Skip to content

Commit

Permalink
Allow @EnabledOnRedisDriver usage with @ParameterizedRedisTest.
Browse files Browse the repository at this point in the history
We now enable tests without an instance in EnabledOnRedisDriverCondition assuming that we only use DriverQualifier on instance fields and not static ones.

Closes #2734
  • Loading branch information
mp911de authored and jxblum committed Oct 12, 2023
1 parent 10a21bb commit 19b59e2
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 43 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
package org.springframework.data.redis.cache;

import static org.assertj.core.api.Assertions.*;
import static org.assertj.core.api.Assumptions.*;
import static org.springframework.data.redis.cache.RedisCacheWriter.*;

import java.nio.charset.Charset;
Expand All @@ -33,8 +32,10 @@
import org.springframework.data.redis.connection.RedisConnection;
import org.springframework.data.redis.connection.RedisConnectionFactory;
import org.springframework.data.redis.connection.RedisStringCommands.SetOption;
import org.springframework.data.redis.connection.lettuce.LettuceConnectionFactory;
import org.springframework.data.redis.core.types.Expiration;
import org.springframework.data.redis.test.condition.EnabledOnRedisDriver;
import org.springframework.data.redis.test.condition.EnabledOnRedisDriver.DriverQualifier;
import org.springframework.data.redis.test.condition.RedisDriver;
import org.springframework.data.redis.test.extension.parametrized.MethodSource;
import org.springframework.data.redis.test.extension.parametrized.ParameterizedRedisTest;

Expand All @@ -55,7 +56,7 @@ public class DefaultRedisCacheWriterTests {
private byte[] binaryCacheKey = cacheKey.getBytes(StandardCharsets.UTF_8);
private byte[] binaryCacheValue = "value".getBytes(StandardCharsets.UTF_8);

private RedisConnectionFactory connectionFactory;
private final @DriverQualifier RedisConnectionFactory connectionFactory;

public DefaultRedisCacheWriterTests(RedisConnectionFactory connectionFactory) {
this.connectionFactory = connectionFactory;
Expand Down Expand Up @@ -147,10 +148,9 @@ void getShouldReturnNullWhenKeyDoesNotExist() {
}

@ParameterizedRedisTest // GH-2650
@EnabledOnRedisDriver(RedisDriver.LETTUCE)
void cacheHitRetrieveShouldIncrementStatistics() throws ExecutionException, InterruptedException {

assumeThat(connectionFactory).isInstanceOf(LettuceConnectionFactory.class);

doWithConnection(connection -> connection.set(binaryCacheKey, binaryCacheValue));

RedisCacheWriter writer = nonLockingRedisCacheWriter(connectionFactory)
Expand All @@ -163,10 +163,9 @@ void cacheHitRetrieveShouldIncrementStatistics() throws ExecutionException, Inte
}

@ParameterizedRedisTest // GH-2650
@EnabledOnRedisDriver(RedisDriver.LETTUCE)
void storeShouldIncrementStatistics() throws ExecutionException, InterruptedException {

assumeThat(connectionFactory).isInstanceOf(LettuceConnectionFactory.class);

RedisCacheWriter writer = nonLockingRedisCacheWriter(connectionFactory)
.withStatisticsCollector(CacheStatisticsCollector.create());

Expand All @@ -176,10 +175,9 @@ void storeShouldIncrementStatistics() throws ExecutionException, InterruptedExce
}

@ParameterizedRedisTest // GH-2650
@EnabledOnRedisDriver(RedisDriver.LETTUCE)
void cacheMissRetrieveWithLoaderAsyncShouldIncrementStatistics() throws ExecutionException, InterruptedException {

assumeThat(connectionFactory).isInstanceOf(LettuceConnectionFactory.class);

RedisCacheWriter writer = nonLockingRedisCacheWriter(connectionFactory)
.withStatisticsCollector(CacheStatisticsCollector.create());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
package org.springframework.data.redis.cache;

import static org.assertj.core.api.Assertions.*;
import static org.assertj.core.api.Assumptions.*;
import static org.awaitility.Awaitility.*;

import io.netty.util.concurrent.DefaultThreadFactory;
Expand Down Expand Up @@ -49,11 +48,12 @@
import org.springframework.cache.support.NullValue;
import org.springframework.data.redis.connection.RedisConnection;
import org.springframework.data.redis.connection.RedisConnectionFactory;
import org.springframework.data.redis.connection.jedis.JedisConnectionFactory;
import org.springframework.data.redis.connection.lettuce.LettuceConnectionFactory;
import org.springframework.data.redis.serializer.RedisSerializationContext.SerializationPair;
import org.springframework.data.redis.serializer.RedisSerializer;
import org.springframework.data.redis.test.condition.EnabledOnCommand;
import org.springframework.data.redis.test.condition.EnabledOnRedisDriver;
import org.springframework.data.redis.test.condition.EnabledOnRedisDriver.DriverQualifier;
import org.springframework.data.redis.test.condition.RedisDriver;
import org.springframework.data.redis.test.extension.parametrized.MethodSource;
import org.springframework.data.redis.test.extension.parametrized.ParameterizedRedisTest;
import org.springframework.lang.Nullable;
Expand All @@ -80,7 +80,7 @@ public class RedisCacheTests {

private byte[] binaryNullValue = RedisSerializer.java().serialize(NullValue.INSTANCE);

private RedisConnectionFactory connectionFactory;
private final @DriverQualifier RedisConnectionFactory connectionFactory;
private RedisSerializer serializer;
private RedisCache cache;

Expand Down Expand Up @@ -284,13 +284,9 @@ void clearShouldClearCache() {
}

@ParameterizedRedisTest // GH-1721
@EnabledOnRedisDriver(RedisDriver.LETTUCE) // SCAN not supported via Jedis Cluster.
void clearWithScanShouldClearCache() {

// SCAN not supported via Jedis Cluster.
if (connectionFactory instanceof JedisConnectionFactory) {
assumeThat(((JedisConnectionFactory) connectionFactory).isRedisClusterAware()).isFalse();
}

RedisCache cache = new RedisCache("cache",
RedisCacheWriter.nonLockingRedisCacheWriter(connectionFactory, BatchStrategies.scan(25)),
RedisCacheConfiguration.defaultCacheConfig().serializeValuesWith(SerializationPair.fromSerializer(serializer)));
Expand Down Expand Up @@ -573,11 +569,9 @@ void cacheGetWithTimeToIdleExpirationAfterEntryExpiresShouldReturnNull() {
}

@ParameterizedRedisTest
@EnabledOnRedisDriver(RedisDriver.JEDIS)
void retrieveCacheValueUsingJedis() {

// TODO: Is there a better way to do this? @EnableOnRedisDriver(RedisDriver.JEDIS) does not work!
assumeThat(this.connectionFactory instanceof JedisConnectionFactory).isTrue();

assertThatExceptionOfType(UnsupportedOperationException.class)
.isThrownBy(() -> this.cache.retrieve(this.binaryCacheKey)).withMessageContaining("RedisCache");

Expand All @@ -587,12 +581,10 @@ void retrieveCacheValueUsingJedis() {
}

@ParameterizedRedisTest // GH-2650
@EnabledOnRedisDriver(RedisDriver.LETTUCE)
@SuppressWarnings("unchecked")
void retrieveReturnsCachedValue() throws Exception {

// TODO: Is there a better way to do this? @EnableOnRedisDriver(RedisDriver.LETTUCE) does not work!
assumeThat(this.connectionFactory instanceof LettuceConnectionFactory).isTrue();

doWithConnection(connection -> connection.stringCommands().set(this.binaryCacheKey, this.binarySample));

RedisCache cache = new RedisCache("cache", usingLockingRedisCacheWriter(), usingRedisCacheConfiguration());
Expand All @@ -605,13 +597,10 @@ void retrieveReturnsCachedValue() throws Exception {
}

@ParameterizedRedisTest // GH-2650
@EnabledOnRedisDriver(RedisDriver.LETTUCE)
@SuppressWarnings("unchecked")
void retrieveReturnsCachedValueWhenLockIsReleased() throws Exception {

// TODO: Is there a better way to do this? @EnableOnRedisDriver(RedisDriver.LETTUCE) does not work!
assumeThat(this.connectionFactory instanceof LettuceConnectionFactory).isTrue();

String mockValue = "MockValue";
String testValue = "TestValue";

byte[] binaryCacheValue = this.serializer.serialize(testValue);
Expand All @@ -634,11 +623,9 @@ void retrieveReturnsCachedValueWhenLockIsReleased() throws Exception {
}

@ParameterizedRedisTest // GH-2650
@EnabledOnRedisDriver(RedisDriver.LETTUCE)
void retrieveReturnsLoadedValue() throws Exception {

// TODO: Is there a better way to do this? @EnableOnRedisDriver(RedisDriver.LETTUCE) does not work!
assumeThat(this.connectionFactory instanceof LettuceConnectionFactory).isTrue();

RedisCache cache = new RedisCache("cache", usingLockingRedisCacheWriter(), usingRedisCacheConfiguration());
AtomicBoolean loaded = new AtomicBoolean(false);
Person jon = new Person("Jon", Date.from(Instant.now()));
Expand All @@ -658,11 +645,9 @@ void retrieveReturnsLoadedValue() throws Exception {
}

@ParameterizedRedisTest // GH-2650
@EnabledOnRedisDriver(RedisDriver.LETTUCE)
void retrieveStoresLoadedValue() throws Exception {

// TODO: Is there a better way to do this? @EnableOnRedisDriver(RedisDriver.LETTUCE) does not work!
assumeThat(this.connectionFactory instanceof LettuceConnectionFactory).isTrue();

RedisCache cache = new RedisCache("cache", usingLockingRedisCacheWriter(), usingRedisCacheConfiguration());
Person jon = new Person("Jon", Date.from(Instant.now()));
Supplier<CompletableFuture<Person>> valueLoaderSupplier = () -> CompletableFuture.completedFuture(jon);
Expand All @@ -675,11 +660,9 @@ void retrieveStoresLoadedValue() throws Exception {
}

@ParameterizedRedisTest // GH-2650
@EnabledOnRedisDriver(RedisDriver.LETTUCE)
void retrieveReturnsNull() throws Exception {

// TODO: Is there a better way to do this? @EnableOnRedisDriver(RedisDriver.LETTUCE) does not work!
assumeThat(this.connectionFactory instanceof LettuceConnectionFactory).isTrue();

doWithConnection(connection -> connection.stringCommands().set(this.binaryCacheKey, this.binaryNullValue));

RedisCache cache = new RedisCache("cache", usingLockingRedisCacheWriter(), usingRedisCacheConfiguration());
Expand Down Expand Up @@ -733,11 +716,8 @@ private Function<RedisCacheConfiguration, RedisCacheConfiguration> withTtiExpira
}

void doWithConnection(Consumer<RedisConnection> callback) {
RedisConnection connection = connectionFactory.getConnection();
try {
try (RedisConnection connection = connectionFactory.getConnection()) {
callback.accept(connection);
} finally {
connection.close();
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,15 @@ public ConditionEvaluationResult evaluateExecutionCondition(ExtensionContext con

if (annotatedFields.isEmpty()) {
throw new IllegalStateException(
"@EnabledOnRedisDriver requires a field of type RedisConnectionFactory annotated with @DriverQualifier");
"@EnabledOnRedisDriver requires a field of type \"RedisConnectionFactory\" annotated with @DriverQualifier");
}

if (context.getTestInstance().isEmpty()) {
return ENABLED_BY_DEFAULT;
}

for (Field field : annotatedFields) {

Try<Object> fieldValue = ReflectionUtils.tryToReadFieldValue(field, context.getRequiredTestInstance());

RedisConnectionFactory value = (RedisConnectionFactory) fieldValue
Expand All @@ -76,7 +81,8 @@ public ConditionEvaluationResult evaluateExecutionCondition(ExtensionContext con
}

if (!foundMatch) {
return disabled(String.format("Driver %s not supported; Supported driver(s): %s", value,
return disabled(String.format("Driver %s not supported; Supported driver(s): %s",
formatUnsupportedDriver(value),
Arrays.toString(annotation.value())));
}
}
Expand All @@ -85,4 +91,14 @@ public ConditionEvaluationResult evaluateExecutionCondition(ExtensionContext con

}

private static String formatUnsupportedDriver(RedisConnectionFactory value) {

for (RedisDriver redisDriver : RedisDriver.values()) {
if (redisDriver.matches(value)) {
return redisDriver.toString();
}
}

return value.toString();
}
}

0 comments on commit 19b59e2

Please sign in to comment.