Refine API and property-converter documentation.

See #1484
Original pull request: #2566.

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

@@ -21,10 +21,11 @@ import static org.mockito.Mockito.*;
 import java.util.UUID;
 
 import org.junit.jupiter.api.Test;
+import org.mockito.Mockito;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.beans.factory.support.BeanDefinitionBuilder;
 import org.springframework.beans.factory.support.DefaultListableBeanFactory;
-import org.springframework.data.convert.PropertyValueConverter.ValueConversionContext;
+import org.springframework.data.mapping.PersistentEntity;
 import org.springframework.data.mapping.PersistentProperty;
 import org.springframework.data.mapping.context.SamplePersistentProperty;
 import org.springframework.lang.Nullable;
@@ -108,7 +109,7 @@ public class PropertyValueConverterFactoryUnitTests {
 	}
 
 	@Test // GH-1484
-	void compositeConverterFactoryIteratesFactories() {
+	void chainedConverterFactoryIteratesFactories() {
 
 		PropertyValueConverter expected = mock(PropertyValueConverter.class);
 
@@ -132,7 +133,7 @@ public class PropertyValueConverterFactoryUnitTests {
 	}
 
 	@Test // GH-1484
-	void compositeConverterFactoryFailsOnException() {
+	void chainedConverterFactoryFailsOnException() {
 
 		PropertyValueConverterFactory factory = PropertyValueConverterFactory.chained(new PropertyValueConverterFactory() {
 			@Nullable
@@ -163,6 +164,27 @@ public class PropertyValueConverterFactoryUnitTests {
 				.isSameAs(factory.getConverter(ConverterWithDefaultCtor.class));
 	}
 
+	@Test // GH-1484
+	void cachingConverterFactoryAlsoCachesAbsenceOfConverter() {
+
+		PropertyValueConverterFactory source = Mockito.spy(PropertyValueConverterFactory.simple());
+		PropertyValueConverterFactory factory = PropertyValueConverterFactory.caching(source);
+
+		PersistentEntity entity = mock(PersistentEntity.class);
+		PersistentProperty property = mock(PersistentProperty.class);
+		when(property.getOwner()).thenReturn(entity);
+		when(entity.getType()).thenReturn(Person.class);
+		when(property.getName()).thenReturn("firstname");
+
+		// fill the cache
+		assertThat(factory.getConverter(property)).isNull();
+		verify(source).getConverter(any(PersistentProperty.class));
+
+		// now get the cached null value
+		assertThat(factory.getConverter(property)).isNull();
+		verify(source).getConverter(any(PersistentProperty.class));
+	}
+
 	@Test // GH-1484
 	void cachingConverterFactoryServesCachedInstanceForProperty() {
 
@@ -182,14 +204,14 @@ public class PropertyValueConverterFactoryUnitTests {
 
 		@Nullable
 		@Override
-		public String nativeToDomain(@Nullable UUID nativeValue, ValueConversionContext<SamplePersistentProperty> context) {
-			return nativeValue.toString();
+		public String read(@Nullable UUID value, ValueConversionContext<SamplePersistentProperty> context) {
+			return value.toString();
 		}
 
 		@Nullable
 		@Override
-		public UUID domainToNative(@Nullable String domainValue, ValueConversionContext<SamplePersistentProperty> context) {
-			return UUID.fromString(domainValue);
+		public UUID write(@Nullable String value, ValueConversionContext<SamplePersistentProperty> context) {
+			return UUID.fromString(value);
 		}
 	}
 
@@ -199,14 +221,14 @@ public class PropertyValueConverterFactoryUnitTests {
 
 		@Nullable
 		@Override
-		public String nativeToDomain(@Nullable UUID nativeValue, ValueConversionContext context) {
-			return nativeValue.toString();
+		public String read(@Nullable UUID value, ValueConversionContext context) {
+			return value.toString();
 		}
 
 		@Nullable
 		@Override
-		public UUID domainToNative(@Nullable String domainValue, ValueConversionContext context) {
-			return UUID.fromString(domainValue);
+		public UUID write(@Nullable String value, ValueConversionContext context) {
+			return UUID.fromString(value);
 		}
 	}
 
@@ -221,22 +243,44 @@ public class PropertyValueConverterFactoryUnitTests {
 
 		@Nullable
 		@Override
-		public String nativeToDomain(@Nullable UUID nativeValue, ValueConversionContext<SamplePersistentProperty> context) {
+		public String read(@Nullable UUID value, ValueConversionContext<SamplePersistentProperty> context) {
 
 			assertThat(someDependency).isNotNull();
-			return nativeValue.toString();
+			return value.toString();
 		}
 
 		@Nullable
 		@Override
-		public UUID domainToNative(@Nullable String domainValue, ValueConversionContext<SamplePersistentProperty> context) {
+		public UUID write(@Nullable String value, ValueConversionContext<SamplePersistentProperty> context) {
 
 			assertThat(someDependency).isNotNull();
-			return UUID.fromString(domainValue);
+			return UUID.fromString(value);
 		}
 	}
 
 	static class SomeDependency {
 
 	}
+
+	static class Person {
+		String name;
+		Address address;
+
+		public String getName() {
+			return name;
+		}
+
+		public Address getAddress() {
+			return address;
+		}
+	}
+
+	static class Address {
+		String street;
+		ZipCode zipCode;
+	}
+
+	static class ZipCode {
+
+	}
 }

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

@@ -0,0 +1,107 @@
+/*
+ * 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 static org.assertj.core.api.Assertions.*;
+
+import org.junit.jupiter.api.Test;
+import org.springframework.data.convert.PropertyValueConverterFactoryUnitTests.Person;
+import org.springframework.data.mapping.context.SamplePersistentProperty;
+import org.springframework.lang.Nullable;
+
+/**
+ * @author Christoph Strobl
+ */
+public class PropertyValueConverterRegistrarUnitTests {
+
+	@Test // GH-1484
+	void buildsRegistryCorrectly() {
+
+		ValueConverterRegistry registry = new PropertyValueConverterRegistrar<>() //
+				.registerConverter(Person.class, "name", new ReversingPropertyValueConverter()) //
+				.buildRegistry(); //
+
+		assertThat(registry.containsConverterFor(Person.class, "name")).isTrue();
+		assertThat(registry.containsConverterFor(Person.class, "not-a-property")).isFalse();
+	}
+
+	@Test // GH-1484
+	void registersConvertersInRegistryCorrectly() {
+
+		ValueConverterRegistry registry = ValueConverterRegistry.simple();
+
+		new PropertyValueConverterRegistrar<>() //
+				.registerConverter(Person.class, "name", new ReversingPropertyValueConverter()) //
+				.registerConvertersIn(registry); //
+
+		assertThat(registry.containsConverterFor(Person.class, "name")).isTrue();
+		assertThat(registry.containsConverterFor(Person.class, "not-a-property")).isFalse();
+	}
+
+	@Test // GH-1484
+	void allowsTypeSafeConverterRegistration() {
+
+		PropertyValueConverterRegistrar<SamplePersistentProperty> registrar = new PropertyValueConverterRegistrar<>();
+		registrar.registerConverter(Person.class, "name", String.class) //
+				.writing(PropertyValueConverterRegistrarUnitTests::reverse) //
+				.readingAsIs(); //
+
+		PropertyValueConverter<String, String, ? extends ValueConversionContext<SamplePersistentProperty>> name = registrar
+				.buildRegistry().getConverter(Person.class, "name");
+		assertThat(name.write("foo", null)).isEqualTo("oof");
+		assertThat(name.read("off", null)).isEqualTo("off");
+	}
+
+	@Test // GH-1484
+	void allowsTypeSafeConverterRegistrationViaRecordedProperty() {
+
+		PropertyValueConverterRegistrar<SamplePersistentProperty> registrar = new PropertyValueConverterRegistrar<>();
+		registrar.registerConverter(Person.class, Person::getName) //
+				.writing(PropertyValueConverterRegistrarUnitTests::reverse) //
+				.readingAsIs();
+
+		PropertyValueConverter<String, String, ? extends ValueConversionContext<SamplePersistentProperty>> name = registrar
+				.buildRegistry().getConverter(Person.class, "name");
+		assertThat(name.write("foo", null)).isEqualTo("oof");
+		assertThat(name.read("мир", null)).isEqualTo("мир");
+	}
+
+	static class ReversingPropertyValueConverter
+			implements PropertyValueConverter<String, String, ValueConversionContext<?>> {
+
+		@Nullable
+		@Override
+		public String read(@Nullable String value, ValueConversionContext<?> context) {
+			return PropertyValueConverterRegistrarUnitTests.reverse(value);
+		}
+
+		@Nullable
+		@Override
+		public String write(@Nullable String value, ValueConversionContext<?> context) {
+			return PropertyValueConverterRegistrarUnitTests.reverse(value);
+		}
+	}
+
+	@Nullable
+	static String reverse(@Nullable String source) {
+
+		if (source == null) {
+			return null;
+		}
+
+		return new StringBuilder(source).reverse().toString();
+	}
+}

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

@@ -0,0 +1,69 @@
+/*
+ * 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 static org.assertj.core.api.Assertions.*;
+import static org.mockito.Mockito.*;
+
+import org.junit.jupiter.api.Test;
+import org.springframework.beans.factory.BeanFactory;
+import org.springframework.data.convert.PropertyValueConverterFactories.CachingPropertyValueConverterFactory;
+import org.springframework.data.convert.PropertyValueConverterFactories.ChainedPropertyValueConverterFactory;
+
+/**
+ * @author Christoph Strobl
+ */
+class SimplePropertyValueConversionsUnitTests {
+
+	@Test // GH-1484
+	void decoratesTargetFactoryWithCacheWhenCachingIsEnabled() {
+
+		SimplePropertyValueConversions conversions = new SimplePropertyValueConversions();
+		conversions.setConverterFactory(PropertyValueConverterFactory.beanFactoryAware(mock(BeanFactory.class)));
+		conversions.setConverterCacheEnabled(true);
+
+		conversions.init();
+		assertThat(conversions.getConverterFactory()).isInstanceOf(CachingPropertyValueConverterFactory.class);
+	}
+
+	@Test // GH-1484
+	void doesNotDecorateTargetFactoryWithCacheWhenCachingIsDisabled() {
+
+		PropertyValueConverterFactory factory = PropertyValueConverterFactory.beanFactoryAware(mock(BeanFactory.class));
+
+		SimplePropertyValueConversions conversions = new SimplePropertyValueConversions();
+		conversions.setConverterFactory(factory);
+		conversions.setConverterCacheEnabled(false);
+
+		conversions.init();
+		assertThat(conversions.getConverterFactory()).isSameAs(factory);
+	}
+
+	@Test // GH-1484
+	void chainsFactoriesIfConverterRegistryPresent() {
+
+		ValueConverterRegistry<?> registry = mock(ValueConverterRegistry.class);
+		PropertyValueConverterFactory factory = PropertyValueConverterFactory.beanFactoryAware(mock(BeanFactory.class));
+
+		SimplePropertyValueConversions conversions = new SimplePropertyValueConversions();
+		conversions.setConverterFactory(factory);
+		conversions.setValueConverterRegistry(registry);
+		conversions.setConverterCacheEnabled(false);
+
+		conversions.init();
+		assertThat(conversions.getConverterFactory()).isInstanceOf(ChainedPropertyValueConverterFactory.class);
+	}
+}

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

@@ -0,0 +1,88 @@
+/*
+ * 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 static org.assertj.core.api.Assertions.*;
+import static org.mockito.Mockito.*;
+
+import org.junit.jupiter.api.Test;
+import org.springframework.data.mapping.Person;
+
+/**
+ * @author Christoph Strobl
+ */
+class SimplePropertyValueConverterRegistryUnitTests {
+
+	@Test // GH-1484
+	void emptyRegistryDoesNotServeConverters() {
+
+		SimplePropertyValueConverterRegistry registry = new SimplePropertyValueConverterRegistry();
+
+		assertThat(registry.isEmpty()).isTrue();
+		assertThat(registry.size()).isZero();
+		assertThat(registry.containsConverterFor(Person.class, "name")).isFalse();
+		assertThat(registry.getConverter(Person.class, "name")).isNull();
+	}
+
+	@Test // GH-1484
+	void registryCopiesOverConverters() {
+
+		SimplePropertyValueConverterRegistry sourceRegistry = new SimplePropertyValueConverterRegistry();
+		sourceRegistry.registerConverter(Person.class, "name", mock(PropertyValueConverter.class));
+
+		SimplePropertyValueConverterRegistry targetRegistry = new SimplePropertyValueConverterRegistry(sourceRegistry);
+		assertThat(targetRegistry.size()).isOne();
+
+		sourceRegistry.registerConverter(Address.class, "street", mock(PropertyValueConverter.class));
+		assertThat(sourceRegistry.size()).isEqualTo(2);
+		assertThat(targetRegistry.size()).isOne();
+	}
+
+	@Test // GH-1484
+	void registryServesMatchingConverter() {
+
+		SimplePropertyValueConverterRegistry registry = new SimplePropertyValueConverterRegistry();
+		registry.registerConverter(Person.class, "name", mock(PropertyValueConverter.class));
+
+		assertThat(registry.isEmpty()).isFalse();
+		assertThat(registry.size()).isOne();
+
+		assertThat(registry.containsConverterFor(Person.class, "name")).isTrue();
+		assertThat(registry.getConverter(Person.class, "name")).isNotNull();
+
+		assertThat(registry.containsConverterFor(Person.class, "age")).isFalse();
+		assertThat(registry.getConverter(Person.class, "age")).isNull();
+
+		assertThat(registry.getConverter(Address.class, "name")).isNull();
+	}
+
+	@Test // GH-1484
+	void registryMayHoldConvertersForDifferentPropertiesOfSameType() {
+
+		PropertyValueConverter nameConverter = mock(PropertyValueConverter.class);
+		PropertyValueConverter ageConverter = mock(PropertyValueConverter.class);
+
+		SimplePropertyValueConverterRegistry registry = new SimplePropertyValueConverterRegistry();
+		registry.registerConverter(Person.class, "name", nameConverter);
+		registry.registerConverter(Person.class, "age", ageConverter);
+
+		assertThat(registry.getConverter(Person.class, "name")).isSameAs(nameConverter);
+		assertThat(registry.getConverter(Person.class, "age")).isSameAs(ageConverter);
+	}
+
+	static class Address {}
+
+}

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

@@ -1,19 +1,3 @@
-/*
- * 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
- *
- *      http://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.
- */
-
 /*
  * Copyright 2022 the original author or authors.
  *
@@ -21,7 +5,7 @@
  * you may not use this file except in compliance with the License.
  * You may obtain a copy of the License at
  *
- *      http://www.apache.org/licenses/LICENSE-2.0
+ *      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,
@@ -39,7 +23,6 @@ import java.util.function.Function;
 import java.util.function.Predicate;
 
 import org.junit.jupiter.api.Test;
-import org.springframework.data.convert.PropertyValueConverter.ValueConversionContext;
 import org.springframework.data.mapping.PersistentProperty;
 import org.springframework.data.mapping.PropertyPath;
 import org.springframework.data.mapping.context.SamplePersistentProperty;
@@ -87,13 +70,13 @@ public class WhatWeWant {
 
 			@Nullable
 			@Override
-			public Object nativeToDomain(@Nullable Object nativeValue, ValueConversionContext context) {
+			public Object read(@Nullable Object value, ValueConversionContext context) {
 				return null;
 			}
 
 			@Nullable
 			@Override
-			public Object domainToNative(@Nullable Object domainValue, ValueConversionContext context) {
+			public Object write(@Nullable Object value, ValueConversionContext context) {
 				return null;
 			}
 		});
@@ -238,13 +221,13 @@ public class WhatWeWant {
 				}
 
 				@Override
-				public B domainToNative(A domainValue, ValueConversionContext<P> context) {
-					return writer.apply(domainValue, context);
+				public B write(A value, ValueConversionContext<P> context) {
+					return writer.apply(value, context);
 				}
 
 				@Override
-				public A nativeToDomain(B nativeValue, ValueConversionContext<P> context) {
-					return reader.apply(nativeValue, context);
+				public A read(B value, ValueConversionContext<P> context) {
+					return reader.apply(value, context);
 				}
 			}
 		}

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

@@ -44,7 +44,6 @@ import org.springframework.data.mapping.Person;
 import org.springframework.data.util.ClassTypeInformation;
 import org.springframework.data.util.Optionals;
 import org.springframework.data.util.TypeInformation;
-import org.springframework.lang.Nullable;
 import org.springframework.util.ReflectionUtils;
 
 /**

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

@@ -36,7 +36,6 @@ 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.beans.factory.annotation.Autowired;
 import org.springframework.core.annotation.AliasFor;
 import org.springframework.core.annotation.AnnotationUtils;
 import org.springframework.data.annotation.AccessType;
@@ -45,9 +44,9 @@ 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.PropertyConverter;
 import org.springframework.data.convert.PropertyValueConverter;
-import org.springframework.data.convert.PropertyValueConverter.ValueConversionContext;
+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;
@@ -342,6 +341,15 @@ public class AnnotationBasedPersistentPropertyUnitTests<P extends AnnotationBase
 		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");
@@ -536,48 +544,21 @@ public class AnnotationBasedPersistentPropertyUnitTests<P extends AnnotationBase
 
 	static class WithPropertyConverter {
 
-		@PropertyConverter(MyPropertyConverter.class)
+		@ValueConverter(MyPropertyConverter.class)
 		String value;
-
-		@PropertyConverter(MyPropertyConverterThatRequiresComponents.class)
-		String value2;
 	}
 
 	static class MyPropertyConverter
 			implements PropertyValueConverter<Object, Object, ValueConversionContext<SamplePersistentProperty>> {
 
 		@Override
-		public Object nativeToDomain(Object value, ValueConversionContext context) {
+		public Object read(Object value, ValueConversionContext context) {
 			return null;
 		}
 
 		@Override
-		public Object domainToNative(Object value, ValueConversionContext context) {
+		public Object write(Object value, ValueConversionContext context) {
 			return null;
 		}
 	}
-
-	static class MyPropertyConverterThatRequiresComponents
-			implements PropertyValueConverter<Object, Object, ValueConversionContext<SamplePersistentProperty>> {
-
-		private final SomeDependency someDependency;
-
-		public MyPropertyConverterThatRequiresComponents(@Autowired SomeDependency someDependency) {
-			this.someDependency = someDependency;
-		}
-
-		@Override
-		public Object nativeToDomain(Object value, ValueConversionContext context) {
-			return null;
-		}
-
-		@Override
-		public Object domainToNative(Object value, ValueConversionContext context) {
-			return null;
-		}
-	}
-
-	static class SomeDependency {
-
-	}
 }