Remove package cycle between mapping and convert.

Remove accessor methods to obtain annotated converters on PersistentProperty.

Closes #2576

Author: mp911de

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<? extends PropertyValueConverter<?, ?, ? extends ValueConversionContext<? extends PersistentProperty<?>>>> 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;
+	}
+
+}

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 @@ static class ChainedPropertyValueConverterFactory implements PropertyValueConver
 
 		private List<PropertyValueConverterFactory> delegates;
 
-		ChainedPropertyValueConverterFactory(PropertyValueConverterFactory... delegates) {
-			this(Arrays.asList(delegates));
-		}
-
 		ChainedPropertyValueConverterFactory(List<PropertyValueConverterFactory> delegates) {
 			this.delegates = Collections.unmodifiableList(delegates);
 		}
@@ -75,10 +70,6 @@ public <S, T, C extends ValueConversionContext<?>> PropertyValueConverter<S, T,
 			return delegates.stream().filter(it -> it.getConverter(converterType) != null).findFirst()
 					.map(it -> it.getConverter(converterType)).orElse(null);
 		}
-
-		public List<PropertyValueConverterFactory> converterFactories() {
-			return delegates;
-		}
 	}
 
 	/**
@@ -221,9 +212,11 @@ static class Cache {
 
 			<S, T, C extends ValueConversionContext<?>> PropertyValueConverter<S, T, C> cache(PersistentProperty<?> property,
 					@Nullable PropertyValueConverter<S, T, C> converter) {
+
 				perPropertyCache.putIfAbsent(property, Optional.ofNullable(converter));
 
-				Class<? extends PropertyValueConverter<?, ?, ? extends ValueConversionContext<? extends PersistentProperty<?>>>> valueConverterType = property
+				AnnotatedPropertyValueConverterAccessor accessor = new AnnotatedPropertyValueConverterAccessor(property);
+				Class<? extends PropertyValueConverter<?, ?, ? extends ValueConversionContext<? extends PersistentProperty<?>>>> valueConverterType = accessor
 						.getValueConverterType();
 				if (valueConverterType != null) {
 					cache(valueConverterType, converter);

src/main/java/org/springframework/data/convert/PropertyValueConverterFactory.java

@@ -54,11 +54,13 @@ public interface PropertyValueConverterFactory {
 	default <DV, SV, C extends ValueConversionContext<?>> PropertyValueConverter<DV, SV, C> getConverter(
 			PersistentProperty<?> property) {
 
-		if (!property.hasValueConverter()) {
+		AnnotatedPropertyValueConverterAccessor accessor = new AnnotatedPropertyValueConverterAccessor(property);
+
+		if (!accessor.hasValueConverter()) {
 			return null;
 		}
 
-		return getConverter((Class<PropertyValueConverter<DV, SV, C>>) property.getValueConverterType());
+		return getConverter((Class<PropertyValueConverter<DV, SV, C>>) accessor.getValueConverterType());
 	}
 
 	/**

src/main/java/org/springframework/data/mapping/PersistentProperty.java

@@ -23,9 +23,6 @@
 
 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 @@ default <T> PersistentPropertyAccessor<T> getAccessorForOwner(T owner) {
 		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.
-	 * <p>
-	 * 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<? extends PropertyValueConverter<?, ?, ? extends ValueConversionContext<? extends PersistentProperty<?>>>> 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.
-	 * <p>
-	 * 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);
-	}
 }

src/main/java/org/springframework/data/mapping/model/AnnotationBasedPersistentProperty.java

@@ -33,9 +33,6 @@
 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 TypeInformation<?> getAssociationTargetTypeInformation() {
 		return associationTargetType.getNullable();
 	}
 
-	@Nullable
-	@Override
-	@SuppressWarnings("unchecked")
-	public Class<? extends PropertyValueConverter<?, ?, ? extends ValueConversionContext<? extends PersistentProperty<?>>>> getValueConverterType() {
-
-		return doFindAnnotation(ValueConverter.class) //
-				.map(ValueConverter::value) //
-				.map(Class.class::cast) //
-				.orElse(null);
-	}
-
 	/*
 	 * (non-Javadoc)
 	 * @see org.springframework.data.mapping.model.AbstractPersistentProperty#toString()

src/test/java/org/springframework/data/convert/PropertyValueConverterFactoryUnitTests.java

@@ -34,6 +34,7 @@
  * Unit tests for {@link PropertyValueConverterFactory}.
  *
  * @author Christoph Strobl
+ * @author Mark Paluch
  */
 public class PropertyValueConverterFactoryUnitTests {
 
@@ -48,8 +49,9 @@ void simpleConverterFactoryCanInstantiateFactoryWithDefaultCtor() {
 	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 @@ void cachingConverterFactoryAlsoCachesAbsenceOfConverter() {
 	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());

src/test/java/org/springframework/data/mapping/model/AnnotationBasedPersistentPropertyUnitTests.java

@@ -36,6 +36,7 @@
 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.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 @@ void detectsJMoleculesIdentity() {
 		assertThat(property.isIdProperty()).isTrue();
 	}
 
-	@Test // GH-1484
-	void detectsValueConverter() {
-
-		SamplePersistentProperty property = getProperty(WithPropertyConverter.class, "value");
-
-		assertThat(property.hasValueConverter()).isTrue();
-		assertThat(property.getValueConverterType()).isEqualTo(MyPropertyConverter.class);
-	}
-
 	@SuppressWarnings("unchecked")
 	private Map<Class<? extends Annotation>, Annotation> getAnnotationCache(SamplePersistentProperty property) {
 		return (Map<Class<? extends Annotation>, Annotation>) ReflectionTestUtils.getField(property, "annotationCache");
@@ -472,7 +461,8 @@ public String getProperty() {
 	@Retention(RetentionPolicy.RUNTIME)
 	@Target(value = { FIELD, METHOD, ANNOTATION_TYPE })
 	@Id
-	public @interface MyId {}
+	public @interface MyId {
+	}
 
 	static class FieldAccess {
 		String name;
@@ -542,23 +532,4 @@ static class JMolecules {
 
 	interface JMoleculesAggregate extends AggregateRoot<JMoleculesAggregate, Identifier> {}
 
-	static class WithPropertyConverter {
-
-		@ValueConverter(MyPropertyConverter.class)
-		String value;
-	}
-
-	static class MyPropertyConverter
-			implements PropertyValueConverter<Object, Object, ValueConversionContext<SamplePersistentProperty>> {
-
-		@Override
-		public Object read(Object value, ValueConversionContext context) {
-			return null;
-		}
-
-		@Override
-		public Object write(Object value, ValueConversionContext context) {
-			return null;
-		}
-	}
 }