Avoid nested entity creation if column value is null.

We now no longer attempt to create instances of nested entities if the column value is null. Previously the null check happened after checking registered custom conversions which has lead to potential object creation for columns containing null values.

Closes #670

Author: mp911de

src/main/java/org/springframework/data/r2dbc/convert/MappingR2dbcConverter.java

@@ -167,18 +167,18 @@ private Object readFrom(Row row, @Nullable RowMetadata metadata, RelationalPersi
 				value = row.get(identifier);
 			}
 
-			if (value != null && getConversions().hasCustomReadTarget(value.getClass(), property.getType())) {
+			if (value == null) {
+				return null;
+			}
+
+			if (getConversions().hasCustomReadTarget(value.getClass(), property.getType())) {
 				return readValue(value, property.getTypeInformation());
 			}
 
 			if (property.isEntity()) {
 				return readEntityFrom(row, metadata, property);
 			}
 
-			if (value == null) {
-				return null;
-			}
-
 			return readValue(value, property.getTypeInformation());
 
 		} catch (Exception o_O) {
@@ -613,8 +613,8 @@ public <T> BiFunction<Row, RowMetadata, T> populateIdIfNecessary(T object) {
 
 			if (idPropertyUpdateNeeded) {
 				return potentiallySetId(row, metadata, propertyAccessor, idProperty) //
-					? (T) propertyAccessor.getBean() //
-					: object;
+						? (T) propertyAccessor.getBean() //
+						: object;
 			}
 
 			return object;

src/test/java/org/springframework/data/r2dbc/convert/MappingR2dbcConverterUnitTests.java

@@ -64,7 +64,8 @@ void before() {
 
 		R2dbcCustomConversions conversions = new R2dbcCustomConversions(
 				Arrays.asList(StringToMapConverter.INSTANCE, MapToStringConverter.INSTANCE,
-						CustomConversionPersonToOutboundRowConverter.INSTANCE, RowToCustomConversionPerson.INSTANCE));
+						CustomConversionPersonToOutboundRowConverter.INSTANCE, RowToCustomConversionPerson.INSTANCE,
+						StringToSimplePersonConverter.INSTANCE));
 
 		mappingContext.setSimpleTypeHolder(conversions.getSimpleTypeHolder());
 
@@ -81,8 +82,7 @@ void shouldIncludeAllPropertiesInOutboundRow() {
 		converter.write(new Person("id", "Walter", "White", instant, localDateTime), row);
 
 		assertThat(row).containsEntry(SqlIdentifier.unquoted("id"), Parameter.fromOrEmpty("id", String.class));
-		assertThat(row).containsEntry(SqlIdentifier.unquoted("firstname"),
-				Parameter.fromOrEmpty("Walter", String.class));
+		assertThat(row).containsEntry(SqlIdentifier.unquoted("firstname"), Parameter.fromOrEmpty("Walter", String.class));
 		assertThat(row).containsEntry(SqlIdentifier.unquoted("lastname"), Parameter.fromOrEmpty("White", String.class));
 		assertThat(row).containsEntry(SqlIdentifier.unquoted("instant"), Parameter.from(instant));
 		assertThat(row).containsEntry(SqlIdentifier.unquoted("local_date_time"), Parameter.from(localDateTime));
@@ -236,6 +236,19 @@ void shouldEvaluateSpelExpression() {
 		assertThat(result.world).isEqualTo("No, universe");
 	}
 
+	@Test // GH-670
+	void considersConverterBeforeEntityConstruction() {
+
+		MockRow row = MockRow.builder().identified("id", Object.class, 42).identified("person", Object.class, null).build();
+		MockRowMetadata metadata = MockRowMetadata.builder().columnMetadata(MockColumnMetadata.builder().name("id").build())
+				.columnMetadata(MockColumnMetadata.builder().name("person").build()).build();
+
+		WithSimplePersonConstructor result = converter.read(WithSimplePersonConstructor.class, row, metadata);
+
+		assertThat(result.id).isEqualTo(42);
+		assertThat(result.person).isNull();
+	}
+
 	@AllArgsConstructor
 	static class Person {
 		@Id String id;
@@ -349,6 +362,17 @@ public CustomConversionPerson convert(Row source) {
 		}
 	}
 
+	@ReadingConverter
+	enum StringToSimplePersonConverter implements Converter<String, SimplePerson> {
+
+		INSTANCE;
+
+		@Override
+		public SimplePerson convert(String source) {
+			return new SimplePerson(source);
+		}
+	}
+
 	static class WithSpelExpression {
 
 		private final long id;
@@ -361,4 +385,24 @@ public WithSpelExpression(long id, @Value("null") String hello, @Value("#root.wo
 			this.world = world;
 		}
 	}
+
+	static class WithSimplePersonConstructor {
+
+		private final long id;
+		private final SimplePerson person;
+
+		public WithSimplePersonConstructor(long id, SimplePerson person) {
+			this.id = id;
+			this.person = person;
+		}
+	}
+
+	static class SimplePerson {
+
+		private final String name;
+
+		SimplePerson(String name) {
+			this.name = name;
+		}
+	}
 }