From 76c1830c570f7eaab2220f881fc2ab892b499155 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Thu, 17 Aug 2023 08:58:50 +0200 Subject: [PATCH] Polishing. Replace qualified class name access of inner classes with simple names and imports. Remove Java 8 guards. Extend supported temporal types in Jsr310Converters. Remove superfluous converter annotations. Simplify tests. See #2677 Original pull request: #2681 --- .../redis/core/convert/BinaryConverters.java | 33 +++---- .../redis/core/convert/Jsr310Converters.java | 55 +++--------- .../core/convert/RedisCustomConversions.java | 2 +- .../core/convert/Jsr310ConvertersTest.java | 61 +++++++++++++ .../RedisRepositoryIntegrationTestBase.java | 85 +++++-------------- 5 files changed, 115 insertions(+), 121 deletions(-) create mode 100644 src/test/java/org/springframework/data/redis/core/convert/Jsr310ConvertersTest.java 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 5f8a16d63d..b4ab86e940 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 @@ -19,10 +19,11 @@ import java.nio.charset.StandardCharsets; import java.text.DateFormat; import java.text.ParseException; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.Date; -import java.util.Set; +import java.util.List; import java.util.UUID; import org.springframework.core.convert.converter.Converter; @@ -50,20 +51,22 @@ 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() - ); + List converters = new ArrayList<>(12); + + converters.add(new StringToBytesConverter()); + converters.add(new BytesToStringConverter()); + converters.add(new NumberToBytesConverter()); + converters.add(new BytesToNumberConverterFactory()); + converters.add(new EnumToBytesConverter()); + converters.add(new BytesToEnumConverterFactory()); + converters.add(new BooleanToBytesConverter()); + converters.add(new BytesToBooleanConverter()); + converters.add(new DateToBytesConverter()); + converters.add(new BytesToDateConverter()); + converters.add(new UuidToBytesConverter()); + converters.add(new BytesToUuidConverter()); + + return converters; } /** 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 cc6274473b..07e34ab7d3 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 @@ -28,39 +28,27 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; -import java.util.Collections; import java.util.List; import org.springframework.core.convert.converter.Converter; -import org.springframework.data.convert.ReadingConverter; -import org.springframework.data.convert.WritingConverter; import org.springframework.data.redis.core.convert.BinaryConverters.StringBasedConverter; -import org.springframework.util.ClassUtils; /** - * Helper class to register JSR-310 specific {@link Converter} implementations in case the we're running on Java 8. + * Helper class to register JSR-310 specific {@link Converter} implementations. * * @author Mark Paluch + * @author John Blum */ public abstract class Jsr310Converters { - private static final boolean JAVA_8_IS_PRESENT = ClassUtils.isPresent("java.time.LocalDateTime", - Jsr310Converters.class.getClassLoader()); - /** * Returns the {@link Converter Converters} to be registered. - *

- * Will only return {@link Converter Converters} in case we're running on Java 8. * * @return the {@link Converter Converters} to be registered. */ public static Collection> getConvertersToRegister() { - if (!JAVA_8_IS_PRESENT) { - return Collections.emptySet(); - } - - List> converters = new ArrayList<>(); + List> converters = new ArrayList<>(20); converters.add(new LocalDateTimeToBytesConverter()); converters.add(new BytesToLocalDateTimeConverter()); @@ -88,19 +76,15 @@ public abstract class Jsr310Converters { public static boolean supports(Class type) { - if (!JAVA_8_IS_PRESENT) { - return false; - } - return Arrays.> asList(LocalDateTime.class, LocalDate.class, LocalTime.class, Instant.class, - ZonedDateTime.class, ZoneId.class, Period.class, Duration.class).contains(type); + ZonedDateTime.class, ZoneId.class, Period.class, Duration.class, OffsetDateTime.class, OffsetTime.class) + .contains(type); } /** * @author Mark Paluch * @since 1.7 */ - @WritingConverter static class LocalDateTimeToBytesConverter extends StringBasedConverter implements Converter { @Override @@ -113,7 +97,6 @@ public byte[] convert(LocalDateTime source) { * @author Mark Paluch * @since 1.7 */ - @ReadingConverter static class BytesToLocalDateTimeConverter extends StringBasedConverter implements Converter { @Override @@ -126,7 +109,6 @@ public LocalDateTime convert(byte[] source) { * @author Mark Paluch * @since 1.7 */ - @WritingConverter static class LocalDateToBytesConverter extends StringBasedConverter implements Converter { @Override @@ -139,7 +121,6 @@ public byte[] convert(LocalDate source) { * @author Mark Paluch * @since 1.7 */ - @ReadingConverter static class BytesToLocalDateConverter extends StringBasedConverter implements Converter { @Override @@ -152,7 +133,6 @@ public LocalDate convert(byte[] source) { * @author Mark Paluch * @since 1.7 */ - @WritingConverter static class LocalTimeToBytesConverter extends StringBasedConverter implements Converter { @Override @@ -165,7 +145,6 @@ public byte[] convert(LocalTime source) { * @author Mark Paluch * @since 1.7 */ - @ReadingConverter static class BytesToLocalTimeConverter extends StringBasedConverter implements Converter { @Override @@ -178,7 +157,6 @@ public LocalTime convert(byte[] source) { * @author Mark Paluch * @since 1.7 */ - @WritingConverter static class ZonedDateTimeToBytesConverter extends StringBasedConverter implements Converter { @Override @@ -191,7 +169,6 @@ public byte[] convert(ZonedDateTime source) { * @author Mark Paluch * @since 1.7 */ - @ReadingConverter static class BytesToZonedDateTimeConverter extends StringBasedConverter implements Converter { @Override @@ -204,7 +181,6 @@ public ZonedDateTime convert(byte[] source) { * @author Mark Paluch * @since 1.7 */ - @WritingConverter static class InstantToBytesConverter extends StringBasedConverter implements Converter { @Override @@ -217,7 +193,6 @@ public byte[] convert(Instant source) { * @author Mark Paluch * @since 1.7 */ - @ReadingConverter static class BytesToInstantConverter extends StringBasedConverter implements Converter { @Override @@ -230,7 +205,6 @@ public Instant convert(byte[] source) { * @author Mark Paluch * @since 1.7 */ - @WritingConverter static class ZoneIdToBytesConverter extends StringBasedConverter implements Converter { @Override @@ -243,7 +217,6 @@ public byte[] convert(ZoneId source) { * @author Mark Paluch * @since 1.7 */ - @ReadingConverter static class BytesToZoneIdConverter extends StringBasedConverter implements Converter { @Override @@ -256,7 +229,6 @@ public ZoneId convert(byte[] source) { * @author Mark Paluch * @since 1.7 */ - @WritingConverter static class PeriodToBytesConverter extends StringBasedConverter implements Converter { @Override @@ -269,7 +241,6 @@ public byte[] convert(Period source) { * @author Mark Paluch * @since 1.7 */ - @ReadingConverter static class BytesToPeriodConverter extends StringBasedConverter implements Converter { @Override @@ -282,7 +253,6 @@ public Period convert(byte[] source) { * @author Mark Paluch * @since 1.7 */ - @WritingConverter static class DurationToBytesConverter extends StringBasedConverter implements Converter { @Override @@ -295,7 +265,6 @@ public byte[] convert(Duration source) { * @author Mark Paluch * @since 1.7 */ - @ReadingConverter static class BytesToDurationConverter extends StringBasedConverter implements Converter { @Override @@ -306,9 +275,10 @@ public Duration convert(byte[] source) { /** * @author John Blum - * @see java.time.OffsetDateTime + * @since 3.1.3 */ - static class OffsetDateTimeToBytesConverter extends StringBasedConverter implements Converter { + static class OffsetDateTimeToBytesConverter extends StringBasedConverter + implements Converter { @Override public byte[] convert(OffsetDateTime source) { @@ -318,9 +288,10 @@ public byte[] convert(OffsetDateTime source) { /** * @author John Blum - * @see java.time.OffsetDateTime + * @since 3.1.3 */ - static class BytesToOffsetDateTimeConverter extends StringBasedConverter implements Converter { + static class BytesToOffsetDateTimeConverter extends StringBasedConverter + implements Converter { @Override public OffsetDateTime convert(byte[] source) { @@ -330,7 +301,7 @@ public OffsetDateTime convert(byte[] source) { /** * @author John Blum - * @see java.time.OffsetTime + * @since 3.1.3 */ static class OffsetTimeToBytesConverter extends StringBasedConverter implements Converter { @@ -342,7 +313,7 @@ public byte[] convert(OffsetTime source) { /** * @author John Blum - * @see java.time.OffsetTime + * @since 3.1.3 */ static class BytesToOffsetTimeConverter extends StringBasedConverter implements Converter { 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 267e331fcf..5c20f42d7b 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 @@ -37,7 +37,7 @@ public class RedisCustomConversions extends org.springframework.data.convert.Cus static { - List converters = new ArrayList<>(); + List converters = new ArrayList<>(35); converters.addAll(BinaryConverters.getConvertersToRegister()); converters.addAll(Jsr310Converters.getConvertersToRegister()); diff --git a/src/test/java/org/springframework/data/redis/core/convert/Jsr310ConvertersTest.java b/src/test/java/org/springframework/data/redis/core/convert/Jsr310ConvertersTest.java new file mode 100644 index 0000000000..d733087bd4 --- /dev/null +++ b/src/test/java/org/springframework/data/redis/core/convert/Jsr310ConvertersTest.java @@ -0,0 +1,61 @@ +/* + * Copyright 2023 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.redis.core.convert; + +import static org.assertj.core.api.Assertions.*; + +import java.time.Duration; +import java.time.Instant; +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; +import java.util.Date; + +import org.junit.jupiter.api.Test; + +/** + * Unit test for {@link Jsr310Converters}. + * + * @author Mark Paluch + */ +class Jsr310ConvertersTest { + + @Test // GH-2677 + void shouldReportSupportedTemporalTypes() { + + assertThat(Jsr310Converters.supports(Object.class)).isFalse(); + assertThat(Jsr310Converters.supports(Date.class)).isFalse(); + + assertThat(Jsr310Converters.supports(Instant.class)).isTrue(); + assertThat(Jsr310Converters.supports(ZoneId.class)).isTrue(); + assertThat(Jsr310Converters.supports(ZonedDateTime.class)).isTrue(); + + assertThat(Jsr310Converters.supports(LocalDateTime.class)).isTrue(); + assertThat(Jsr310Converters.supports(LocalDate.class)).isTrue(); + assertThat(Jsr310Converters.supports(LocalTime.class)).isTrue(); + + assertThat(Jsr310Converters.supports(Duration.class)).isTrue(); + assertThat(Jsr310Converters.supports(Period.class)).isTrue(); + + assertThat(Jsr310Converters.supports(OffsetTime.class)).isTrue(); + assertThat(Jsr310Converters.supports(OffsetDateTime.class)).isTrue(); + } +} 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 0ae4ae7ea9..6fd2ad1341 100644 --- a/src/test/java/org/springframework/data/redis/repository/RedisRepositoryIntegrationTestBase.java +++ b/src/test/java/org/springframework/data/redis/repository/RedisRepositoryIntegrationTestBase.java @@ -15,7 +15,7 @@ */ package org.springframework.data.redis.repository; -import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.*; import java.time.OffsetDateTime; import java.time.OffsetTime; @@ -27,7 +27,6 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; - import org.springframework.beans.factory.annotation.Autowired; import org.springframework.data.annotation.Id; import org.springframework.data.annotation.Reference; @@ -51,9 +50,7 @@ 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. @@ -504,9 +501,7 @@ void shouldProperlyReadNestedImmutableObject() { @Test // GH-2677 void shouldProperlyHandleEntityWithOffsetJavaTimeTypes() { - User jonDoe = User.as("Jon Doe") - .expires(OffsetTime.now().plusMinutes(5)) - .lastAccess(OffsetDateTime.now()); + User jonDoe = User.of("Jon Doe").expires(OffsetTime.now().plusMinutes(5)).lastAccess(OffsetDateTime.now()); this.userRepository.save(jonDoe); @@ -519,8 +514,7 @@ void shouldProperlyHandleEntityWithOffsetJavaTimeTypes() { assertThat(loadedJonDoe.getExpiration()).isEqualTo(jonDoe.getExpiration()); } - public interface PersonRepository - extends PagingAndSortingRepository, CrudRepository, + public interface PersonRepository extends PagingAndSortingRepository, CrudRepository, QueryByExampleExecutor { List findByFirstname(String firstname); @@ -566,7 +560,7 @@ public interface CityRepository extends CrudRepository { public interface ImmutableObjectRepository extends CrudRepository {} - public interface UserRepository extends CrudRepository { } + public interface UserRepository extends CrudRepository {} /** * Custom Redis {@link IndexConfiguration} forcing index of {@link Person#lastname}. @@ -670,12 +664,9 @@ public boolean equals(Object obj) { return false; } - return Objects.equals(this.getId(), that.getId()) - && Objects.equals(this.getFirstname(), that.getFirstname()) - && Objects.equals(this.getLastname(), that.getLastname()) - && Objects.equals(this.getAlive(), that.getAlive()) - && Objects.equals(this.getCity(), that.getCity()) - && Objects.equals(this.getHometown(), that.getHometown()); + return Objects.equals(this.getId(), that.getId()) && Objects.equals(this.getFirstname(), that.getFirstname()) + && Objects.equals(this.getLastname(), that.getLastname()) && Objects.equals(this.getAlive(), that.getAlive()) + && Objects.equals(this.getCity(), that.getCity()) && Objects.equals(this.getHometown(), that.getHometown()); } @Override @@ -725,9 +716,8 @@ public boolean equals(Object obj) { return false; } - return Objects.equals(this.getId(), that.getId()) - && Objects.equals(this.getName(), that.getName()) - && Objects.equals(this.getLocation(), that.getLocation()); + return Objects.equals(this.getId(), that.getId()) && Objects.equals(this.getName(), that.getName()) + && Objects.equals(this.getLocation(), that.getLocation()); } @Override @@ -738,11 +728,7 @@ public int hashCode() { @Override public String toString() { - return "City{" + - "id='" + id + '\'' + - ", name='" + name + '\'' + - ", location=" + location + - '}'; + return "City{" + "id='" + id + '\'' + ", name='" + name + '\'' + ", location=" + location + '}'; } } @@ -781,9 +767,8 @@ public boolean equals(Object obj) { return false; } - return Objects.equals(this.getId(), that.getId()) - && Objects.equals(this.getName(), that.getName()) - && Objects.equals(this.getNested(), that.getNested()); + return Objects.equals(this.getId(), that.getId()) && Objects.equals(this.getName(), that.getName()) + && Objects.equals(this.getNested(), that.getNested()); } @Override @@ -793,9 +778,8 @@ public int hashCode() { public String toString() { - return "RedisRepositoryIntegrationTestBase.Immutable(id=" + this.getId() - + ", name=" + this.getName() - + ", nested=" + this.getNested() + ")"; + return "RedisRepositoryIntegrationTestBase.Immutable(id=" + this.getId() + ", name=" + this.getName() + + ", nested=" + this.getNested() + ")"; } public Immutable withId(String id) { @@ -814,22 +798,20 @@ public Immutable withNested(Immutable 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); - } + @Id private final String name; private OffsetDateTime lastAccessed; private OffsetTime expiration; - @Id - private final String name; - - private User(@NonNull String name) { + private User(String name) { this.name = name; } + static User of(String name) { + return new User(name); + } + @Nullable public OffsetTime getExpiration() { return this.expiration; @@ -844,38 +826,15 @@ public String getName() { return this.name; } - public User lastAccess(@Nullable OffsetDateTime dateTime) { + public User lastAccess(OffsetDateTime dateTime) { this.lastAccessed = dateTime; return this; } - public User expires(@Nullable OffsetTime time) { + public User expires(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(); - } } }