Introduce PropertyValueConversionService and specific null-conversion methods.

PropertyValueConverter read and write methods are never called with null values. Instead, PropertyValueConverter now defines readNull and writeNull to encapsulate null conversion. PropertyValueConversionService is a facade that encapsulates these details to simplify converter usage.

Resolves #2577
Closes #2592

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

@@ -0,0 +1,111 @@
+/*
+ * 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;
+
+/**
+ * Conversion service based on {@link CustomConversions} to convert domain and store values using
+ * {@link PropertyValueConverter property-specific converters}.
+ *
+ * @author Mark Paluch
+ * @since 2.7
+ */
+public class PropertyValueConversionService {
+
+	private final CustomConversions conversions;
+
+	public PropertyValueConversionService(CustomConversions conversions) {
+
+		Assert.notNull(conversions, "CustomConversions must not be null");
+
+		this.conversions = conversions;
+	}
+
+	/**
+	 * Return {@literal true} there is a converter registered for {@link PersistentProperty}.
+	 * <p>
+	 * If this method returns {@literal true}, it means {@link #read(Object, PersistentProperty, ValueConversionContext)}
+	 * and {@link #write(Object, PersistentProperty, ValueConversionContext)} are capable to invoke conversion.
+	 *
+	 * @param property the underlying property.
+	 * @return {@literal true} there is a converter registered for {@link PersistentProperty}.
+	 */
+	public boolean hasConverter(PersistentProperty<?> property) {
+		return conversions.hasPropertyValueConverter(property);
+	}
+
+	/**
+	 * Convert a value from its store-native representation into its domain-specific type.
+	 *
+	 * @param value the value to convert. Can be {@code null}.
+	 * @param property the underlying property.
+	 * @param context the context object.
+	 * @param <P> property type.
+	 * @param <VCC> value conversion context type.
+	 * @return the value to be used in the domain model. Can be {@code null}.
+	 */
+	@Nullable
+	public <P extends PersistentProperty<P>, VCC extends ValueConversionContext<P>> Object read(@Nullable Object value,
+			P property, VCC context) {
+
+		PropertyValueConverter<Object, Object, ValueConversionContext<P>> converter = getRequiredConverter(property);
+
+		if (value == null) {
+			return converter.readNull(context);
+		}
+
+		return converter.read(value, context);
+	}
+
+	/**
+	 * Convert a value from its domain-specific value into its store-native representation.
+	 *
+	 * @param value the value to convert. Can be {@code null}.
+	 * @param property the underlying property.
+	 * @param context the context object.
+	 * @param <P> property type.
+	 * @param <VCC> value conversion context type.
+	 * @return the value to be written to the data store. Can be {@code null}.
+	 */
+	@Nullable
+	public <P extends PersistentProperty<P>, VCC extends ValueConversionContext<P>> Object write(@Nullable Object value,
+			P property, VCC context) {
+
+		PropertyValueConverter<Object, Object, ValueConversionContext<P>> converter = getRequiredConverter(property);
+
+		if (value == null) {
+			return converter.writeNull(context);
+		}
+
+		return converter.write(value, context);
+	}
+
+	private <P extends PersistentProperty<P>> PropertyValueConverter<Object, Object, ValueConversionContext<P>> getRequiredConverter(
+			P property) {
+
+		PropertyValueConverter<Object, Object, ValueConversionContext<P>> converter = conversions
+				.getPropertyValueConverter(property);
+
+		if (converter == null) {
+			throw new IllegalArgumentException(String.format("No converter registered for property %s", property));
+		}
+
+		return converter;
+	}
+}

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

@@ -26,8 +26,13 @@ import org.springframework.lang.Nullable;
  * <p>
  * A {@link PropertyValueConverter} is, other than a {@link ReadingConverter} or {@link WritingConverter}, only applied
  * to special annotated fields which allows a fine-grained conversion of certain values within a specific context.
+ * <p>
+ * Converter methods are called with non-null values only and provide specific hooks for {@code null} value handling.
+ * {@link #readNull(ValueConversionContext)} and {@link #writeNull(ValueConversionContext)} methods are specifically
+ * designated to either retain {@code null} values or return a different value to indicate {@code null} values.
  *
  * @author Christoph Strobl
+ * @author Mark Paluch
  * @param <DV> domain-specific type.
  * @param <SV> store-native type.
  * @param <C> the store specific {@link ValueConversionContext conversion context}.
@@ -39,23 +44,47 @@ public interface PropertyValueConverter<DV, SV, C extends ValueConversionContext
 	 * Convert the given store specific value into it's domain value representation. Typically, a {@literal read}
 	 * operation.
 	 *
-	 * @param value can be {@literal null}.
+	 * @param value the value to read.
 	 * @param context never {@literal null}.
 	 * @return the converted value. Can be {@literal null}.
 	 */
 	@Nullable
-	DV read(@Nullable SV value, C context);
+	DV read(SV value, C context);
+
+	/**
+	 * Convert the given {@code null} value from the store into it's domain value representation. Typically, a
+	 * {@literal read} operation. Returns {@code null} by default.
+	 *
+	 * @param context never {@literal null}.
+	 * @return the converted value. Can be {@literal null}.
+	 */
+	@Nullable
+	default DV readNull(C context) {
+		return null;
+	}
 
 	/**
 	 * Convert the given domain-specific value into it's native store representation. Typically, a {@literal write}
 	 * operation.
 	 *
-	 * @param value can be {@literal null}.
+	 * @param value the value to write.
 	 * @param context never {@literal null}.
 	 * @return the converted value. Can be {@literal null}.
 	 */
 	@Nullable
-	SV write(@Nullable DV value, C context);
+	SV write(DV value, C context);
+
+	/**
+	 * Convert the given {@code null} value from the domain model into it's native store representation. Typically, a
+	 * {@literal write} operation. Returns {@code null} by default.
+	 *
+	 * @param context never {@literal null}.
+	 * @return the converted value. Can be {@literal null}.
+	 */
+	@Nullable
+	default SV writeNull(C context) {
+		return null;
+	}
 
 	/**
 	 * No-op {@link PropertyValueConverter} implementation.
@@ -100,14 +129,24 @@ public interface PropertyValueConverter<DV, SV, C extends ValueConversionContext
 
 		@Nullable
 		@Override
-		public SV write(@Nullable DV value, ValueConversionContext<P> context) {
+		public SV write(DV value, ValueConversionContext<P> context) {
 			return writer.apply(value, context);
 		}
 
+		@Override
+		public SV writeNull(ValueConversionContext<P> context) {
+			return writer.apply(null, context);
+		}
+
 		@Nullable
 		@Override
 		public DV read(@Nullable SV value, ValueConversionContext<P> context) {
 			return reader.apply(value, context);
 		}
+
+		@Override
+		public DV readNull(ValueConversionContext<P> context) {
+			return reader.apply(null, context);
+		}
 	}
 }

src/test/benchmark/org/springframework/data/convert/PropertyValueConversionServiceUnitTests.java

@@ -0,0 +1,138 @@
+/*
+ * 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 java.util.Collections;
+
+import org.junit.jupiter.api.Test;
+
+import org.springframework.data.mapping.Person;
+import org.springframework.data.mapping.context.SampleMappingContext;
+import org.springframework.data.mapping.context.SamplePersistentProperty;
+import org.springframework.data.mapping.model.BasicPersistentEntity;
+import org.springframework.data.util.Predicates;
+
+/**
+ * Unit tests for {@link PropertyValueConversionService}.
+ *
+ * @author Mark Paluch
+ */
+class PropertyValueConversionServiceUnitTests {
+
+	SampleMappingContext mappingContext = new SampleMappingContext();
+
+	PropertyValueConversions conversions = PropertyValueConversions.simple(it -> {
+		it.registerConverter(Person.class, "firstName", String.class).writing(w -> "Writing " + w)
+				.reading(r -> "Reading " + r);
+	});
+	PropertyValueConversionService service = createConversionService(conversions);
+
+	@Test // GH-2557
+	void shouldReportConverter() {
+
+		BasicPersistentEntity<Object, SamplePersistentProperty> entity = mappingContext
+				.getRequiredPersistentEntity(Person.class);
+
+		assertThat(service.hasConverter(entity.getRequiredPersistentProperty("firstName"))).isTrue();
+		assertThat(service.hasConverter(entity.getRequiredPersistentProperty("lastName"))).isFalse();
+	}
+
+	@Test // GH-2557
+	void conversionWithoutConverterShouldFail() {
+
+		BasicPersistentEntity<Object, SamplePersistentProperty> entity = mappingContext
+				.getRequiredPersistentEntity(Person.class);
+
+		SamplePersistentProperty property = entity.getRequiredPersistentProperty("lastName");
+		assertThatIllegalArgumentException().isThrownBy(() -> service.read("foo", property, () -> property));
+		assertThatIllegalArgumentException().isThrownBy(() -> service.write("foo", property, () -> property));
+	}
+
+	@Test // GH-2557
+	void readShouldUseReadConverter() {
+
+		BasicPersistentEntity<Object, SamplePersistentProperty> entity = mappingContext
+				.getRequiredPersistentEntity(Person.class);
+
+		SamplePersistentProperty property = entity.getRequiredPersistentProperty("firstName");
+		assertThat(service.read("Walter", property, () -> property)).isEqualTo("Reading Walter");
+		assertThat(service.read(null, property, () -> property)).isEqualTo("Reading null");
+	}
+
+	@Test // GH-2557
+	void readShouldUseWriteConverter() {
+
+		BasicPersistentEntity<Object, SamplePersistentProperty> entity = mappingContext
+				.getRequiredPersistentEntity(Person.class);
+
+		SamplePersistentProperty property = entity.getRequiredPersistentProperty("firstName");
+		assertThat(service.write("Walter", property, () -> property)).isEqualTo("Writing Walter");
+		assertThat(service.write(null, property, () -> property)).isEqualTo("Writing null");
+	}
+
+	@Test // GH-2557
+	void readShouldUseNullConvertersConverter() {
+
+		PropertyValueConversions conversions = PropertyValueConversions.simple(it -> {
+			it.registerConverter(Person.class, "firstName", WithNullConverters.INSTANCE);
+		});
+
+		PropertyValueConversionService service = createConversionService(conversions);
+
+		BasicPersistentEntity<Object, SamplePersistentProperty> entity = mappingContext
+				.getRequiredPersistentEntity(Person.class);
+
+		SamplePersistentProperty property = entity.getRequiredPersistentProperty("firstName");
+
+		assertThat(service.read(null, property, () -> property)).isEqualTo("readNull");
+		assertThat(service.write(null, property, () -> property)).isEqualTo("writeNull");
+	}
+
+	private static PropertyValueConversionService createConversionService(PropertyValueConversions conversions) {
+
+		CustomConversions.ConverterConfiguration configuration = new CustomConversions.ConverterConfiguration(
+				CustomConversions.StoreConversions.NONE, Collections.emptyList(), Predicates.isTrue(), conversions);
+
+		return new PropertyValueConversionService(new CustomConversions(configuration));
+	}
+
+	enum WithNullConverters implements PropertyValueConverter<String, String, ValueConversionContext<?>> {
+		INSTANCE;
+
+		@Override
+		public String read(String value, ValueConversionContext<?> context) {
+			return value;
+		}
+
+		@Override
+		public String readNull(ValueConversionContext<?> context) {
+			return "readNull";
+		}
+
+		@Override
+		public String write(String value, ValueConversionContext<?> context) {
+			return value;
+		}
+
+		@Override
+		public String writeNull(ValueConversionContext<?> context) {
+			return "writeNull";
+		}
+	}
+
+}

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

@@ -226,13 +226,13 @@ public class PropertyValueConverterFactoryUnitTests {
 
 		@Nullable
 		@Override
-		public String read(@Nullable UUID value, ValueConversionContext context) {
+		public String read(UUID value, ValueConversionContext context) {
 			return value.toString();
 		}
 
 		@Nullable
 		@Override
-		public UUID write(@Nullable String value, ValueConversionContext context) {
+		public UUID write(String value, ValueConversionContext context) {
 			return UUID.fromString(value);
 		}
 	}
@@ -248,7 +248,7 @@ public class PropertyValueConverterFactoryUnitTests {
 
 		@Nullable
 		@Override
-		public String read(@Nullable UUID value, ValueConversionContext<SamplePersistentProperty> context) {
+		public String read(UUID value, ValueConversionContext<SamplePersistentProperty> context) {
 
 			assertThat(someDependency).isNotNull();
 			return value.toString();
@@ -256,7 +256,7 @@ public class PropertyValueConverterFactoryUnitTests {
 
 		@Nullable
 		@Override
-		public UUID write(@Nullable String value, ValueConversionContext<SamplePersistentProperty> context) {
+		public UUID write(String value, ValueConversionContext<SamplePersistentProperty> context) {
 
 			assertThat(someDependency).isNotNull();
 			return UUID.fromString(value);