From 4d65223355890cd68adafeb16e33e329e765c56e 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 d103f9e..5e55a84 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 596aa95..ef253b2 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; + } + } }