From 0358ef18b844b7e0423b959db3eaf4214b214d6e Mon Sep 17 00:00:00 2001 From: John Blum Date: Wed, 16 Aug 2023 17:01:56 -0700 Subject: [PATCH 1/2] Prepare issue branch for 2677. --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 1f6ddd552b..6989dc7026 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-redis - 3.2.0-SNAPSHOT + 3.2.0-GH-2677-SNAPSHOT Spring Data Redis Spring Data module for Redis From 426f2ae009e19c9f35b7f6f8084df98c0d6ff1af Mon Sep 17 00:00:00 2001 From: John Blum Date: Wed, 16 Aug 2023 17:35:00 -0700 Subject: [PATCH 2/2] Register Converters for Offset java.time types in JSR310Converters. We now appropriately handle OffsetDateTime and OffsetTime the same as all other java.time types, supported as simple types on Spring application (persistent) entity classes. Closes #2677 --- .../redis/core/convert/BinaryConverters.java | 20 ++++ .../redis/core/convert/Jsr310Converters.java | 61 +++++++++++- .../core/convert/RedisCustomConversions.java | 13 +-- ...edisRepositoryClusterIntegrationTests.java | 4 +- .../RedisRepositoryIntegrationTestBase.java | 94 +++++++++++++++++++ .../RedisRepositoryIntegrationTests.java | 5 +- 6 files changed, 178 insertions(+), 19 deletions(-) diff --git a/src/main/java/org/springframework/data/redis/core/convert/BinaryConverters.java b/src/main/java/org/springframework/data/redis/core/convert/BinaryConverters.java index 905d902f0a..5f8a16d63d 100644 --- a/src/main/java/org/springframework/data/redis/core/convert/BinaryConverters.java +++ b/src/main/java/org/springframework/data/redis/core/convert/BinaryConverters.java @@ -20,7 +20,9 @@ import java.text.DateFormat; import java.text.ParseException; import java.util.Arrays; +import java.util.Collection; import java.util.Date; +import java.util.Set; import java.util.UUID; import org.springframework.core.convert.converter.Converter; @@ -46,6 +48,24 @@ final class BinaryConverters { private BinaryConverters() {} + static Collection getConvertersToRegister() { + + return Set.of( + new BinaryConverters.StringToBytesConverter(), + new BinaryConverters.BytesToStringConverter(), + new BinaryConverters.NumberToBytesConverter(), + new BinaryConverters.BytesToNumberConverterFactory(), + new BinaryConverters.EnumToBytesConverter(), + new BinaryConverters.BytesToEnumConverterFactory(), + new BinaryConverters.BooleanToBytesConverter(), + new BinaryConverters.BytesToBooleanConverter(), + new BinaryConverters.DateToBytesConverter(), + new BinaryConverters.BytesToDateConverter(), + new BinaryConverters.UuidToBytesConverter(), + new BinaryConverters.BytesToUuidConverter() + ); + } + /** * @author Christoph Strobl * @since 1.7 diff --git a/src/main/java/org/springframework/data/redis/core/convert/Jsr310Converters.java b/src/main/java/org/springframework/data/redis/core/convert/Jsr310Converters.java index e7a9a464bf..cc6274473b 100644 --- a/src/main/java/org/springframework/data/redis/core/convert/Jsr310Converters.java +++ b/src/main/java/org/springframework/data/redis/core/convert/Jsr310Converters.java @@ -13,7 +13,6 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - package org.springframework.data.redis.core.convert; import java.time.Duration; @@ -21,6 +20,8 @@ import java.time.LocalDate; import java.time.LocalDateTime; import java.time.LocalTime; +import java.time.OffsetDateTime; +import java.time.OffsetTime; import java.time.Period; import java.time.ZoneId; import java.time.ZonedDateTime; @@ -47,9 +48,11 @@ public abstract class Jsr310Converters { Jsr310Converters.class.getClassLoader()); /** - * Returns the converters to be registered. Will only return converters in case we're running on Java 8. + * Returns the {@link Converter Converters} to be registered. + *

+ * Will only return {@link Converter Converters} in case we're running on Java 8. * - * @return + * @return the {@link Converter Converters} to be registered. */ public static Collection> getConvertersToRegister() { @@ -58,6 +61,7 @@ public abstract class Jsr310Converters { } List> converters = new ArrayList<>(); + converters.add(new LocalDateTimeToBytesConverter()); converters.add(new BytesToLocalDateTimeConverter()); converters.add(new LocalDateToBytesConverter()); @@ -74,6 +78,10 @@ public abstract class Jsr310Converters { converters.add(new BytesToPeriodConverter()); converters.add(new DurationToBytesConverter()); converters.add(new BytesToDurationConverter()); + converters.add(new OffsetDateTimeToBytesConverter()); + converters.add(new BytesToOffsetDateTimeConverter()); + converters.add(new OffsetTimeToBytesConverter()); + converters.add(new BytesToOffsetTimeConverter()); return converters; } @@ -296,4 +304,51 @@ public Duration convert(byte[] source) { } } + /** + * @author John Blum + * @see java.time.OffsetDateTime + */ + static class OffsetDateTimeToBytesConverter extends StringBasedConverter implements Converter { + + @Override + public byte[] convert(OffsetDateTime source) { + return fromString(source.toString()); + } + } + + /** + * @author John Blum + * @see java.time.OffsetDateTime + */ + static class BytesToOffsetDateTimeConverter extends StringBasedConverter implements Converter { + + @Override + public OffsetDateTime convert(byte[] source) { + return OffsetDateTime.parse(toString(source)); + } + } + + /** + * @author John Blum + * @see java.time.OffsetTime + */ + static class OffsetTimeToBytesConverter extends StringBasedConverter implements Converter { + + @Override + public byte[] convert(OffsetTime source) { + return fromString(source.toString()); + } + } + + /** + * @author John Blum + * @see java.time.OffsetTime + */ + static class BytesToOffsetTimeConverter extends StringBasedConverter implements Converter { + + @Override + public OffsetTime convert(byte[] source) { + return OffsetTime.parse(toString(source)); + } + } } diff --git a/src/main/java/org/springframework/data/redis/core/convert/RedisCustomConversions.java b/src/main/java/org/springframework/data/redis/core/convert/RedisCustomConversions.java index f950d9ca64..267e331fcf 100644 --- a/src/main/java/org/springframework/data/redis/core/convert/RedisCustomConversions.java +++ b/src/main/java/org/springframework/data/redis/core/convert/RedisCustomConversions.java @@ -39,18 +39,7 @@ public class RedisCustomConversions extends org.springframework.data.convert.Cus List converters = new ArrayList<>(); - converters.add(new BinaryConverters.StringToBytesConverter()); - converters.add(new BinaryConverters.BytesToStringConverter()); - converters.add(new BinaryConverters.NumberToBytesConverter()); - converters.add(new BinaryConverters.BytesToNumberConverterFactory()); - converters.add(new BinaryConverters.EnumToBytesConverter()); - converters.add(new BinaryConverters.BytesToEnumConverterFactory()); - converters.add(new BinaryConverters.BooleanToBytesConverter()); - converters.add(new BinaryConverters.BytesToBooleanConverter()); - converters.add(new BinaryConverters.DateToBytesConverter()); - converters.add(new BinaryConverters.BytesToDateConverter()); - converters.add(new BinaryConverters.UuidToBytesConverter()); - converters.add(new BinaryConverters.BytesToUuidConverter()); + converters.addAll(BinaryConverters.getConvertersToRegister()); converters.addAll(Jsr310Converters.getConvertersToRegister()); STORE_CONVERTERS = Collections.unmodifiableList(converters); diff --git a/src/test/java/org/springframework/data/redis/repository/RedisRepositoryClusterIntegrationTests.java b/src/test/java/org/springframework/data/redis/repository/RedisRepositoryClusterIntegrationTests.java index 2bf3b60b47..4044f3904d 100644 --- a/src/test/java/org/springframework/data/redis/repository/RedisRepositoryClusterIntegrationTests.java +++ b/src/test/java/org/springframework/data/redis/repository/RedisRepositoryClusterIntegrationTests.java @@ -32,7 +32,6 @@ import org.springframework.data.redis.core.RedisTemplate; import org.springframework.data.redis.repository.configuration.EnableRedisRepositories; import org.springframework.data.redis.test.condition.EnabledOnRedisClusterAvailable; -import org.springframework.lang.NonNullApi; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.junit.jupiter.SpringExtension; @@ -52,13 +51,14 @@ class RedisRepositoryClusterIntegrationTests extends RedisRepositoryIntegrationT @EnableRedisRepositories(considerNestedRepositories = true, indexConfiguration = MyIndexConfiguration.class, keyspaceConfiguration = MyKeyspaceConfiguration.class, includeFilters = { @ComponentScan.Filter(type = FilterType.ASSIGNABLE_TYPE, - classes = { PersonRepository.class, CityRepository.class, ImmutableObjectRepository.class }) }) + classes = { PersonRepository.class, CityRepository.class, ImmutableObjectRepository.class, UserRepository.class }) }) static class Config { @Bean RedisTemplate redisTemplate(RedisConnectionFactory connectionFactory) { RedisTemplate template = new RedisTemplate<>(); + template.setConnectionFactory(connectionFactory); return template; diff --git a/src/test/java/org/springframework/data/redis/repository/RedisRepositoryIntegrationTestBase.java b/src/test/java/org/springframework/data/redis/repository/RedisRepositoryIntegrationTestBase.java index 9f35a99200..0ae4ae7ea9 100644 --- a/src/test/java/org/springframework/data/redis/repository/RedisRepositoryIntegrationTestBase.java +++ b/src/test/java/org/springframework/data/redis/repository/RedisRepositoryIntegrationTestBase.java @@ -17,6 +17,8 @@ import static org.assertj.core.api.Assertions.assertThat; +import java.time.OffsetDateTime; +import java.time.OffsetTime; import java.util.Arrays; import java.util.Collections; import java.util.List; @@ -49,6 +51,9 @@ import org.springframework.data.repository.CrudRepository; import org.springframework.data.repository.PagingAndSortingRepository; import org.springframework.data.repository.query.QueryByExampleExecutor; +import org.springframework.lang.NonNull; +import org.springframework.lang.Nullable; +import org.springframework.util.Assert; /** * Base for testing Redis repository support in different configurations. @@ -62,6 +67,7 @@ public abstract class RedisRepositoryIntegrationTestBase { @Autowired PersonRepository repo; @Autowired CityRepository cityRepo; @Autowired ImmutableObjectRepository immutableObjectRepo; + @Autowired UserRepository userRepository; @Autowired KeyValueTemplate kvTemplate; @BeforeEach @@ -495,6 +501,24 @@ void shouldProperlyReadNestedImmutableObject() { assertThat(loaded.nested).isEqualTo(nested); } + @Test // GH-2677 + void shouldProperlyHandleEntityWithOffsetJavaTimeTypes() { + + User jonDoe = User.as("Jon Doe") + .expires(OffsetTime.now().plusMinutes(5)) + .lastAccess(OffsetDateTime.now()); + + this.userRepository.save(jonDoe); + + User loadedJonDoe = this.userRepository.findById(jonDoe.getName()).orElse(null); + + assertThat(loadedJonDoe).isNotNull(); + assertThat(loadedJonDoe).isNotSameAs(jonDoe); + assertThat(loadedJonDoe.getName()).isEqualTo(jonDoe.getName()); + assertThat(loadedJonDoe.getLastAccessed()).isEqualTo(jonDoe.getLastAccessed()); + assertThat(loadedJonDoe.getExpiration()).isEqualTo(jonDoe.getExpiration()); + } + public interface PersonRepository extends PagingAndSortingRepository, CrudRepository, QueryByExampleExecutor { @@ -542,6 +566,8 @@ public interface CityRepository extends CrudRepository { public interface ImmutableObjectRepository extends CrudRepository {} + public interface UserRepository extends CrudRepository { } + /** * Custom Redis {@link IndexConfiguration} forcing index of {@link Person#lastname}. * @@ -784,4 +810,72 @@ public Immutable withNested(Immutable nested) { return Objects.equals(getNested(), nested) ? this : new Immutable(this.id, this.name, nested); } } + + @RedisHash("Users") + static class User { + + static User as(@NonNull String name) { + Assert.hasText(name, () -> String.format("Name [%s] of User is required", name)); + return new User(name); + } + + private OffsetDateTime lastAccessed; + + private OffsetTime expiration; + + @Id + private final String name; + + private User(@NonNull String name) { + this.name = name; + } + + @Nullable + public OffsetTime getExpiration() { + return this.expiration; + } + + @Nullable + public OffsetDateTime getLastAccessed() { + return this.lastAccessed; + } + + public String getName() { + return this.name; + } + + public User lastAccess(@Nullable OffsetDateTime dateTime) { + this.lastAccessed = dateTime; + return this; + } + + public User expires(@Nullable OffsetTime time) { + this.expiration = time; + return this; + } + + @Override + public boolean equals(Object obj) { + + if (this == obj) { + return true; + } + + if (!(obj instanceof User that)) { + return false; + } + + return this.getName().equals(that.getName()); + } + + @Override + public int hashCode() { + return Objects.hash(getName()); + } + + @Override + public String toString() { + return getName(); + } + } } diff --git a/src/test/java/org/springframework/data/redis/repository/RedisRepositoryIntegrationTests.java b/src/test/java/org/springframework/data/redis/repository/RedisRepositoryIntegrationTests.java index 13c850d02e..3bab07e22f 100644 --- a/src/test/java/org/springframework/data/redis/repository/RedisRepositoryIntegrationTests.java +++ b/src/test/java/org/springframework/data/redis/repository/RedisRepositoryIntegrationTests.java @@ -57,8 +57,7 @@ public class RedisRepositoryIntegrationTests extends RedisRepositoryIntegrationT @EnableRedisRepositories(considerNestedRepositories = true, indexConfiguration = MyIndexConfiguration.class, keyspaceConfiguration = MyKeyspaceConfiguration.class, includeFilters = { @ComponentScan.Filter(type = FilterType.ASSIGNABLE_TYPE, - classes = { PersonRepository.class, CityRepository.class, ImmutableObjectRepository.class }) }) - + classes = { PersonRepository.class, CityRepository.class, ImmutableObjectRepository.class, UserRepository.class }) }) static class Config { @Bean @@ -70,6 +69,7 @@ RedisConnectionFactory connectionFactory() { RedisTemplate redisTemplate(RedisConnectionFactory connectionFactory) { RedisTemplate template = new RedisTemplate<>(); + template.setDefaultSerializer(StringRedisSerializer.UTF_8); template.setConnectionFactory(connectionFactory); @@ -107,6 +107,7 @@ private RedisTypeMapper customTypeMapper() { public void shouldConsiderCustomTypeMapper() { Person rand = new Person(); + rand.id = "rand"; rand.firstname = "rand"; rand.lastname = "al'thor";