From 5854922ae82e675036dc33e4a6fc6fc99a074c0f Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Wed, 8 May 2019 10:07:57 +0200 Subject: [PATCH] DATAJDBC-370 - Fixed handling of entities with no withers. We tried to set all the properties, even when they were already set via constructor. Fixed it by unifying the three instances where we created and populated instances. Original Pull Request: #151 --- .../jdbc/core/convert/BasicJdbcConverter.java | 54 +--- .../convert/EntityRowMapperUnitTests.java | 249 +++++++++--------- 2 files changed, 140 insertions(+), 163 deletions(-) diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/BasicJdbcConverter.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/BasicJdbcConverter.java index 7b9a48f3..003d10fd 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/BasicJdbcConverter.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/BasicJdbcConverter.java @@ -30,7 +30,6 @@ import org.springframework.data.convert.CustomConversions; import org.springframework.data.jdbc.core.mapping.AggregateReference; import org.springframework.data.jdbc.support.JdbcUtil; import org.springframework.data.mapping.MappingException; -import org.springframework.data.mapping.PersistentEntity; import org.springframework.data.mapping.PersistentPropertyAccessor; import org.springframework.data.mapping.PreferredConstructor; import org.springframework.data.mapping.context.MappingContext; @@ -285,30 +284,22 @@ public class BasicJdbcConverter extends BasicRelationalConverter implements Jdbc } private ReadingContext extendBy(RelationalPersistentProperty property) { - return new ReadingContext<>(entity, accessStrategy, resultSet, path.extendBy(property)); + return new ReadingContext(getMappingContext().getRequiredPersistentEntity(property.getActualType()), + accessStrategy, resultSet, path.extendBy(property)); } T mapRow() { RelationalPersistentProperty idProperty = entity.getIdProperty(); - Object idValue = null; - if (idProperty != null) { - idValue = readFrom(idProperty); - } + Object idValue = idProperty == null ? null : readFrom(idProperty); - T result = createInstanceInternal(entity, idValue); - - return entity.requiresPropertyPopulation() // - ? populateProperties(result) // - : result; + return createInstanceInternal(idValue); } - private T populateProperties(T result) { + private T populateProperties(T instance, @Nullable Object idValue) { - PersistentPropertyAccessor propertyAccessor = getPropertyAccessor(entity, result); - - Object id = idProperty == null ? null : readFrom(idProperty); + PersistentPropertyAccessor propertyAccessor = getPropertyAccessor(entity, instance); PreferredConstructor persistenceConstructor = entity.getPersistenceConstructor(); @@ -318,7 +309,7 @@ public class BasicJdbcConverter extends BasicRelationalConverter implements Jdbc continue; } - propertyAccessor.setProperty(property, readOrLoadProperty(id, property)); + propertyAccessor.setProperty(property, readOrLoadProperty(idValue, property)); } return propertyAccessor.getBean(); @@ -358,27 +349,17 @@ public class BasicJdbcConverter extends BasicRelationalConverter implements Jdbc } @SuppressWarnings("unchecked") - private Object readEmbeddedEntityFrom(@Nullable Object id, RelationalPersistentProperty property) { + private Object readEmbeddedEntityFrom(@Nullable Object idValue, RelationalPersistentProperty property) { ReadingContext newContext = extendBy(property); - RelationalPersistentEntity entity = getMappingContext().getRequiredPersistentEntity(property.getActualType()); - - Object instance = newContext.createInstanceInternal(entity, null); - - PersistentPropertyAccessor accessor = getPropertyAccessor((PersistentEntity) entity, instance); - - for (RelationalPersistentProperty p : entity) { - accessor.setProperty(p, newContext.readOrLoadProperty(id, p)); - } - - return instance; + return newContext.createInstanceInternal(idValue); } @Nullable private S readEntityFrom(RelationalPersistentProperty property, PersistentPropertyPathExtension path) { - ReadingContext newContext = extendBy(property); + ReadingContext newContext = (ReadingContext) extendBy(property); RelationalPersistentEntity entity = (RelationalPersistentEntity) getMappingContext() .getRequiredPersistentEntity(property.getActualType()); @@ -398,15 +379,7 @@ public class BasicJdbcConverter extends BasicRelationalConverter implements Jdbc return null; } - S instance = newContext.createInstanceInternal(entity, idValue); - - PersistentPropertyAccessor accessor = getPropertyAccessor(entity, instance); - - for (RelationalPersistentProperty p : entity) { - accessor.setProperty(p, newContext.readOrLoadProperty(idValue, p)); - } - - return instance; + return newContext.createInstanceInternal(idValue); } @Nullable @@ -419,9 +392,9 @@ public class BasicJdbcConverter extends BasicRelationalConverter implements Jdbc } } - private S createInstanceInternal(RelationalPersistentEntity entity, @Nullable Object idValue) { + private T createInstanceInternal(@Nullable Object idValue) { - return createInstance(entity, parameter -> { + T instance = createInstance(entity, parameter -> { String parameterName = parameter.getName(); @@ -431,6 +404,7 @@ public class BasicJdbcConverter extends BasicRelationalConverter implements Jdbc return readOrLoadProperty(idValue, property); }); + return populateProperties(instance, idValue); } } diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/EntityRowMapperUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/EntityRowMapperUnitTests.java index 2f3a6237..a8c43e83 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/EntityRowMapperUnitTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/EntityRowMapperUnitTests.java @@ -284,6 +284,131 @@ public class EntityRowMapperUnitTests { fixture.assertOn(extracted); } + // Model classes to be used in tests + + @RequiredArgsConstructor + static class TrivialImmutable { + + @Id private final Long id; + private final String name; + } + + static class Trivial { + + @Id Long id; + String name; + } + + static class OneToOne { + + @Id Long id; + String name; + Trivial child; + } + + @RequiredArgsConstructor + static class OneToOneImmutable { + + private final @Id Long id; + private final String name; + private final TrivialImmutable child; + } + + static class OneToSet { + + @Id Long id; + String name; + Set children; + } + + static class OneToMap { + + @Id Long id; + String name; + Map children; + } + + static class OneToList { + + @Id Long id; + String name; + List children; + } + + static class EmbeddedEntity { + + @Id Long id; + String name; + @Embedded("prefix_") Trivial children; + } + + private static class DontUseSetter { + String value; + + DontUseSetter(@Param("value") String value) { + this.value = "setThroughConstructor:" + value; + } + } + + static class MixedProperties { + + final String one; + String two; + final String three; + + @PersistenceConstructor + MixedProperties(String one) { + this.one = one; + this.three = "unset"; + } + + private MixedProperties(String one, String two, String three) { + + this.one = one; + this.two = two; + this.three = three; + } + + MixedProperties withThree(String three) { + return new MixedProperties(one, two, three); + } + } + + @AllArgsConstructor + static class EntityWithListInConstructor { + + @Id final Long id; + + final List content; + } + + static class NoIdChain0 { + String zeroValue; + } + + static class NoIdChain1 { + String oneValue; + NoIdChain0 chain0; + } + + static class NoIdChain2 { + String twoValue; + NoIdChain1 chain1; + } + + static class NoIdChain3 { + String threeValue; + NoIdChain2 chain2; + } + + static class NoIdChain4 { + @Id Long four; + String fourValue; + NoIdChain3 chain3; + } + + // Infrastructure for assertions and constructing mocks + private FixtureBuilder buildFixture() { return new FixtureBuilder<>(); } @@ -418,129 +543,6 @@ public class EntityRowMapperUnitTests { } } - @RequiredArgsConstructor - @Wither - static class TrivialImmutable { - - @Id private final Long id; - private final String name; - } - - static class Trivial { - - @Id Long id; - String name; - } - - static class OneToOne { - - @Id Long id; - String name; - Trivial child; - } - - @RequiredArgsConstructor - @Wither - static class OneToOneImmutable { - - private final @Id Long id; - private final String name; - private final TrivialImmutable child; - } - - static class OneToSet { - - @Id Long id; - String name; - Set children; - } - - static class OneToMap { - - @Id Long id; - String name; - Map children; - } - - static class OneToList { - - @Id Long id; - String name; - List children; - } - - static class EmbeddedEntity { - - @Id Long id; - String name; - @Embedded("prefix_") Trivial children; - } - - private static class DontUseSetter { - String value; - - DontUseSetter(@Param("value") String value) { - this.value = "setThroughConstructor:" + value; - } - } - - static class MixedProperties { - - final String one; - String two; - final String three; - - @PersistenceConstructor - MixedProperties(String one) { - this.one = one; - this.three = "unset"; - } - - private MixedProperties(String one, String two, String three) { - - this.one = one; - this.two = two; - this.three = three; - } - - MixedProperties withThree(String three) { - return new MixedProperties(one, two, three); - } - } - - @AllArgsConstructor - static class EntityWithListInConstructor { - - @Id final Long id; - - final List content; - } - - static class NoIdChain0 { - String zeroValue; - } - - static class NoIdChain1 { - String oneValue; - NoIdChain0 chain0; - } - - static class NoIdChain2 { - String twoValue; - NoIdChain1 chain1; - } - - static class NoIdChain3 { - String threeValue; - NoIdChain2 chain2; - } - - static class NoIdChain4 { - @Id Long four; - String fourValue; - NoIdChain3 chain3; - } - private interface SetValue { SetColumns value(Object value); @@ -609,6 +611,7 @@ public class EntityRowMapperUnitTests { @AllArgsConstructor private static class Fixture { + final ResultSet resultSet; final List> expectations;