From 7bf035f3a5d7b7a30fcc6db8b34700c5a1f27744 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Tue, 10 Oct 2023 14:06:46 +0200 Subject: [PATCH] Allow `@EnabledOnRedisDriver` usage with `@ParameterizedRedisTest`. We now enable tests without an instance in EnabledOnRedisDriverCondition assuming that we only use DriverQualifier on instance fields and not static ones. Closes #2734 Original pull request: #2717 --- .../cache/DefaultRedisCacheWriterTests.java | 16 +++---- .../data/redis/cache/RedisCacheTests.java | 44 +++++-------------- .../EnabledOnRedisDriverCondition.java | 20 ++++++++- 3 files changed, 37 insertions(+), 43 deletions(-) diff --git a/src/test/java/org/springframework/data/redis/cache/DefaultRedisCacheWriterTests.java b/src/test/java/org/springframework/data/redis/cache/DefaultRedisCacheWriterTests.java index b2eec5b2de..f8c98b6f45 100644 --- a/src/test/java/org/springframework/data/redis/cache/DefaultRedisCacheWriterTests.java +++ b/src/test/java/org/springframework/data/redis/cache/DefaultRedisCacheWriterTests.java @@ -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; @@ -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; @@ -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; @@ -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) @@ -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()); @@ -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()); diff --git a/src/test/java/org/springframework/data/redis/cache/RedisCacheTests.java b/src/test/java/org/springframework/data/redis/cache/RedisCacheTests.java index 977c199cd6..2d0a1bce1f 100644 --- a/src/test/java/org/springframework/data/redis/cache/RedisCacheTests.java +++ b/src/test/java/org/springframework/data/redis/cache/RedisCacheTests.java @@ -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; @@ -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; @@ -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; @@ -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))); @@ -573,11 +569,9 @@ void cacheGetWithTimeToIdleExpirationAfterEntryExpiresShouldReturnNull() { } @ParameterizedRedisTest // GH-2650 + @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"); @@ -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()); @@ -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); @@ -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())); @@ -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> valueLoaderSupplier = () -> CompletableFuture.completedFuture(jon); @@ -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()); @@ -733,11 +716,8 @@ private Function withTtiExpira } void doWithConnection(Consumer callback) { - RedisConnection connection = connectionFactory.getConnection(); - try { + try (RedisConnection connection = connectionFactory.getConnection()) { callback.accept(connection); - } finally { - connection.close(); } } diff --git a/src/test/java/org/springframework/data/redis/test/condition/EnabledOnRedisDriverCondition.java b/src/test/java/org/springframework/data/redis/test/condition/EnabledOnRedisDriverCondition.java index c7143f46f4..fb85bfe3bc 100644 --- a/src/test/java/org/springframework/data/redis/test/condition/EnabledOnRedisDriverCondition.java +++ b/src/test/java/org/springframework/data/redis/test/condition/EnabledOnRedisDriverCondition.java @@ -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 fieldValue = ReflectionUtils.tryToReadFieldValue(field, context.getRequiredTestInstance()); RedisConnectionFactory value = (RedisConnectionFactory) fieldValue @@ -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()))); } } @@ -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(); + } }