From 8b5d0900bb09a35f453025549d1693b4c4dfc8fc Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Tue, 5 Apr 2022 10:32:53 +0200 Subject: [PATCH] Remove package cycle between mapping and convert. Remove accessor methods to obtain annotated converters on PersistentProperty. Closes #2576 --- ...notatedPropertyValueConverterAccessor.java | 60 +++++++++++++++++++ .../PropertyValueConverterFactories.java | 13 +--- .../PropertyValueConverterFactory.java | 6 +- .../data/mapping/PersistentProperty.java | 33 ---------- .../AnnotationBasedPersistentProperty.java | 14 ----- ...ropertyValueConverterFactoryUnitTests.java | 11 ++-- ...ationBasedPersistentPropertyUnitTests.java | 35 +---------- 7 files changed, 77 insertions(+), 95 deletions(-) create mode 100644 src/main/java/org/springframework/data/convert/AnnotatedPropertyValueConverterAccessor.java diff --git a/src/main/java/org/springframework/data/convert/AnnotatedPropertyValueConverterAccessor.java b/src/main/java/org/springframework/data/convert/AnnotatedPropertyValueConverterAccessor.java new file mode 100644 index 000000000..ca818f412 --- /dev/null +++ b/src/main/java/org/springframework/data/convert/AnnotatedPropertyValueConverterAccessor.java @@ -0,0 +1,60 @@ +/* + * Copyright 2022 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.convert; + +import org.springframework.data.mapping.PersistentProperty; +import org.springframework.lang.Nullable; +import org.springframework.util.Assert; + +/** + * Accessor to obtain metadata for {@link PropertyValueConverter} from an annotated {@link PersistentProperty}. + * + * @author Mark Paluch + * @since 2.7 + */ +class AnnotatedPropertyValueConverterAccessor { + + private final ValueConverter annotation; + + public AnnotatedPropertyValueConverterAccessor(PersistentProperty property) { + + Assert.notNull(property, "PersistentProperty must not be null"); + annotation = property.findAnnotation(ValueConverter.class); + } + + /** + * Obtain the {@link PropertyValueConverter converter type} to be used for reading and writing property values. Uses + * the {@link ValueConverter} annotation and extracts its {@link ValueConverter#value() value} attribute. + * + * @return {@literal null} if none defined. Check {@link #hasValueConverter()} to check if the annotation is present + * at all. + */ + @Nullable + @SuppressWarnings({ "unchecked", "rawtypes" }) + public Class>>> getValueConverterType() { + return annotation != null ? (Class) annotation.value() : null; + } + + /** + * Return whether a value converter is configured. Uses {@link ValueConverter} as annotation type. + * + * @return {@literal true} if a value converter is configured. + */ + public boolean hasValueConverter() { + return annotation != null; + } + +} diff --git a/src/main/java/org/springframework/data/convert/PropertyValueConverterFactories.java b/src/main/java/org/springframework/data/convert/PropertyValueConverterFactories.java index 719e3de8c..91f5686c9 100644 --- a/src/main/java/org/springframework/data/convert/PropertyValueConverterFactories.java +++ b/src/main/java/org/springframework/data/convert/PropertyValueConverterFactories.java @@ -15,7 +15,6 @@ */ package org.springframework.data.convert; -import java.util.Arrays; import java.util.Collections; import java.util.EnumSet; import java.util.List; @@ -53,10 +52,6 @@ final class PropertyValueConverterFactories { private List delegates; - ChainedPropertyValueConverterFactory(PropertyValueConverterFactory... delegates) { - this(Arrays.asList(delegates)); - } - ChainedPropertyValueConverterFactory(List delegates) { this.delegates = Collections.unmodifiableList(delegates); } @@ -75,10 +70,6 @@ final class PropertyValueConverterFactories { return delegates.stream().filter(it -> it.getConverter(converterType) != null).findFirst() .map(it -> it.getConverter(converterType)).orElse(null); } - - public List converterFactories() { - return delegates; - } } /** @@ -221,9 +212,11 @@ final class PropertyValueConverterFactories { > PropertyValueConverter cache(PersistentProperty property, @Nullable PropertyValueConverter converter) { + perPropertyCache.putIfAbsent(property, Optional.ofNullable(converter)); - Class>>> valueConverterType = property + AnnotatedPropertyValueConverterAccessor accessor = new AnnotatedPropertyValueConverterAccessor(property); + Class>>> valueConverterType = accessor .getValueConverterType(); if (valueConverterType != null) { cache(valueConverterType, converter); diff --git a/src/main/java/org/springframework/data/convert/PropertyValueConverterFactory.java b/src/main/java/org/springframework/data/convert/PropertyValueConverterFactory.java index e969952db..c87fa5216 100644 --- a/src/main/java/org/springframework/data/convert/PropertyValueConverterFactory.java +++ b/src/main/java/org/springframework/data/convert/PropertyValueConverterFactory.java @@ -54,11 +54,13 @@ public interface PropertyValueConverterFactory { default > PropertyValueConverter getConverter( PersistentProperty property) { - if (!property.hasValueConverter()) { + AnnotatedPropertyValueConverterAccessor accessor = new AnnotatedPropertyValueConverterAccessor(property); + + if (!accessor.hasValueConverter()) { return null; } - return getConverter((Class>) property.getValueConverterType()); + return getConverter((Class>) accessor.getValueConverterType()); } /** diff --git a/src/main/java/org/springframework/data/mapping/PersistentProperty.java b/src/main/java/org/springframework/data/mapping/PersistentProperty.java index ff2babc20..b49db09fb 100644 --- a/src/main/java/org/springframework/data/mapping/PersistentProperty.java +++ b/src/main/java/org/springframework/data/mapping/PersistentProperty.java @@ -23,9 +23,6 @@ import java.util.Map; import org.springframework.core.annotation.AnnotatedElementUtils; import org.springframework.core.annotation.AnnotationUtils; -import org.springframework.data.convert.PropertyValueConverter; -import org.springframework.data.convert.ValueConversionContext; -import org.springframework.data.convert.ValueConverter; import org.springframework.data.util.TypeInformation; import org.springframework.lang.Nullable; import org.springframework.util.Assert; @@ -438,34 +435,4 @@ public interface PersistentProperty

> { return getOwner().getPropertyAccessor(owner); } - /** - * Obtain the {@link PropertyValueConverter converter type} to be used for reading and writing property values. Uses - * the {@link ValueConverter} annotation and extracts its {@link ValueConverter#value() value} attribute. - *

- * Store implementations may override the default and resort to a more specific annotation type. - * - * @return {@literal null} if none defined. Check {@link #hasValueConverter()} to check if the annotation is present - * at all. - * @since 2.7 - */ - @Nullable - @SuppressWarnings({ "unchecked", "rawtypes" }) - default Class>>> getValueConverterType() { - - ValueConverter annotation = findAnnotation(ValueConverter.class); - - return annotation == null ? null : (Class) annotation.value(); - } - - /** - * Return whether a value converter is configured. Uses {@link ValueConverter} as annotation type. - *

- * Store implementations may override the default and resort to a more specific annotation type. - * - * @return {@literal true} if a value converter is configured. - * @since 2.7 - */ - default boolean hasValueConverter() { - return isAnnotationPresent(ValueConverter.class); - } } diff --git a/src/main/java/org/springframework/data/mapping/model/AnnotationBasedPersistentProperty.java b/src/main/java/org/springframework/data/mapping/model/AnnotationBasedPersistentProperty.java index 1db387a15..a1cd511df 100644 --- a/src/main/java/org/springframework/data/mapping/model/AnnotationBasedPersistentProperty.java +++ b/src/main/java/org/springframework/data/mapping/model/AnnotationBasedPersistentProperty.java @@ -33,9 +33,6 @@ import org.springframework.data.annotation.ReadOnlyProperty; import org.springframework.data.annotation.Reference; import org.springframework.data.annotation.Transient; import org.springframework.data.annotation.Version; -import org.springframework.data.convert.ValueConverter; -import org.springframework.data.convert.PropertyValueConverter; -import org.springframework.data.convert.ValueConversionContext; import org.springframework.data.mapping.Association; import org.springframework.data.mapping.MappingException; import org.springframework.data.mapping.PersistentEntity; @@ -309,17 +306,6 @@ public abstract class AnnotationBasedPersistentProperty

>>> getValueConverterType() { - - return doFindAnnotation(ValueConverter.class) // - .map(ValueConverter::value) // - .map(Class.class::cast) // - .orElse(null); - } - /* * (non-Javadoc) * @see org.springframework.data.mapping.model.AbstractPersistentProperty#toString() diff --git a/src/test/java/org/springframework/data/convert/PropertyValueConverterFactoryUnitTests.java b/src/test/java/org/springframework/data/convert/PropertyValueConverterFactoryUnitTests.java index dad7e2273..89961a326 100644 --- a/src/test/java/org/springframework/data/convert/PropertyValueConverterFactoryUnitTests.java +++ b/src/test/java/org/springframework/data/convert/PropertyValueConverterFactoryUnitTests.java @@ -34,6 +34,7 @@ import org.springframework.lang.Nullable; * Unit tests for {@link PropertyValueConverterFactory}. * * @author Christoph Strobl + * @author Mark Paluch */ public class PropertyValueConverterFactoryUnitTests { @@ -48,8 +49,9 @@ public class PropertyValueConverterFactoryUnitTests { void simpleConverterFactoryReadsConverterFromAnnotation() { PersistentProperty property = mock(PersistentProperty.class); - when(property.hasValueConverter()).thenReturn(true); - when(property.getValueConverterType()).thenReturn(ConverterWithDefaultCtor.class); + ValueConverter annotation = mock(ValueConverter.class); + when(annotation.value()).thenReturn((Class) ConverterWithDefaultCtor.class); + when(property.findAnnotation(ValueConverter.class)).thenReturn(annotation); assertThat(PropertyValueConverterFactory.simple().getConverter(property)) .isInstanceOf(ConverterWithDefaultCtor.class); @@ -191,8 +193,9 @@ public class PropertyValueConverterFactoryUnitTests { void cachingConverterFactoryServesCachedInstanceForProperty() { PersistentProperty property = mock(PersistentProperty.class); - when(property.hasValueConverter()).thenReturn(true); - when(property.getValueConverterType()).thenReturn(ConverterWithDefaultCtor.class); + ValueConverter annotation = mock(ValueConverter.class); + when(annotation.value()).thenReturn((Class) ConverterWithDefaultCtor.class); + when(property.findAnnotation(ValueConverter.class)).thenReturn(annotation); PropertyValueConverterFactory factory = PropertyValueConverterFactory .caching(PropertyValueConverterFactory.simple()); diff --git a/src/test/java/org/springframework/data/mapping/model/AnnotationBasedPersistentPropertyUnitTests.java b/src/test/java/org/springframework/data/mapping/model/AnnotationBasedPersistentPropertyUnitTests.java index c2acacfb1..929d5cdbd 100755 --- a/src/test/java/org/springframework/data/mapping/model/AnnotationBasedPersistentPropertyUnitTests.java +++ b/src/test/java/org/springframework/data/mapping/model/AnnotationBasedPersistentPropertyUnitTests.java @@ -36,6 +36,7 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DynamicTest; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestFactory; + import org.springframework.core.annotation.AliasFor; import org.springframework.core.annotation.AnnotationUtils; import org.springframework.data.annotation.AccessType; @@ -44,9 +45,6 @@ import org.springframework.data.annotation.Id; import org.springframework.data.annotation.ReadOnlyProperty; import org.springframework.data.annotation.Reference; import org.springframework.data.annotation.Transient; -import org.springframework.data.convert.PropertyValueConverter; -import org.springframework.data.convert.ValueConversionContext; -import org.springframework.data.convert.ValueConverter; import org.springframework.data.mapping.MappingException; import org.springframework.data.mapping.PersistentProperty; import org.springframework.data.mapping.context.SampleMappingContext; @@ -341,15 +339,6 @@ public class AnnotationBasedPersistentPropertyUnitTests

, Annotation> getAnnotationCache(SamplePersistentProperty property) { return (Map, Annotation>) ReflectionTestUtils.getField(property, "annotationCache"); @@ -472,7 +461,8 @@ public class AnnotationBasedPersistentPropertyUnitTests

{} - static class WithPropertyConverter { - - @ValueConverter(MyPropertyConverter.class) - String value; - } - - static class MyPropertyConverter - implements PropertyValueConverter> { - - @Override - public Object read(Object value, ValueConversionContext context) { - return null; - } - - @Override - public Object write(Object value, ValueConversionContext context) { - return null; - } - } }