From 0ad751ec5cf9385099c0df3c11e179adcf92baff Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Mon, 25 Oct 2021 15:43:26 +0200 Subject: [PATCH] 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 --- .../r2dbc/convert/MappingR2dbcConverter.java | 14 +++--- .../MappingR2dbcConverterUnitTests.java | 50 +++++++++++++++++-- 2 files changed, 54 insertions(+), 10 deletions(-) diff --git a/src/main/java/org/springframework/data/r2dbc/convert/MappingR2dbcConverter.java b/src/main/java/org/springframework/data/r2dbc/convert/MappingR2dbcConverter.java index d103f9e0..5e55a849 100644 --- a/src/main/java/org/springframework/data/r2dbc/convert/MappingR2dbcConverter.java +++ b/src/main/java/org/springframework/data/r2dbc/convert/MappingR2dbcConverter.java @@ -167,7 +167,11 @@ public class MappingR2dbcConverter extends BasicRelationalConverter implements R 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()); } @@ -175,10 +179,6 @@ public class MappingR2dbcConverter extends BasicRelationalConverter implements R return readEntityFrom(row, metadata, property); } - if (value == null) { - return null; - } - return readValue(value, property.getTypeInformation()); } catch (Exception o_O) { @@ -613,8 +613,8 @@ public class MappingR2dbcConverter extends BasicRelationalConverter implements R if (idPropertyUpdateNeeded) { return potentiallySetId(row, metadata, propertyAccessor, idProperty) // - ? (T) propertyAccessor.getBean() // - : object; + ? (T) propertyAccessor.getBean() // + : object; } return object; diff --git a/src/test/java/org/springframework/data/r2dbc/convert/MappingR2dbcConverterUnitTests.java b/src/test/java/org/springframework/data/r2dbc/convert/MappingR2dbcConverterUnitTests.java index 596aa956..ef253b25 100644 --- a/src/test/java/org/springframework/data/r2dbc/convert/MappingR2dbcConverterUnitTests.java +++ b/src/test/java/org/springframework/data/r2dbc/convert/MappingR2dbcConverterUnitTests.java @@ -65,7 +65,8 @@ public class MappingR2dbcConverterUnitTests { R2dbcCustomConversions conversions = R2dbcCustomConversions.of(PostgresDialect.INSTANCE, Arrays.asList(StringToMapConverter.INSTANCE, MapToStringConverter.INSTANCE, - CustomConversionPersonToOutboundRowConverter.INSTANCE, RowToCustomConversionPerson.INSTANCE)); + CustomConversionPersonToOutboundRowConverter.INSTANCE, RowToCustomConversionPerson.INSTANCE, + StringToSimplePersonConverter.INSTANCE)); mappingContext.setSimpleTypeHolder(conversions.getSimpleTypeHolder()); @@ -82,8 +83,7 @@ public class MappingR2dbcConverterUnitTests { 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)); @@ -237,6 +237,19 @@ public class MappingR2dbcConverterUnitTests { 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; @@ -350,6 +363,17 @@ public class MappingR2dbcConverterUnitTests { } } + @ReadingConverter + enum StringToSimplePersonConverter implements Converter { + + INSTANCE; + + @Override + public SimplePerson convert(String source) { + return new SimplePerson(source); + } + } + static class WithSpelExpression { private final long id; @@ -362,4 +386,24 @@ public class MappingR2dbcConverterUnitTests { 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; + } + } }