From 98241f27c31c74477fc0dd66368f49f3205cb367 Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Wed, 8 May 2019 07:47:54 +0200 Subject: [PATCH 1/6] DATAJDBC-370 - Prepare branch --- pom.xml | 2 +- spring-data-jdbc-distribution/pom.xml | 2 +- spring-data-jdbc/pom.xml | 4 ++-- spring-data-relational/pom.xml | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/pom.xml b/pom.xml index 680dddf736..d147ba7d53 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-relational-parent - 1.1.0.BUILD-SNAPSHOT + 1.1.0.DATAJDBC-370-SNAPSHOT pom Spring Data Relational Parent diff --git a/spring-data-jdbc-distribution/pom.xml b/spring-data-jdbc-distribution/pom.xml index f6d4373844..8945528d0d 100644 --- a/spring-data-jdbc-distribution/pom.xml +++ b/spring-data-jdbc-distribution/pom.xml @@ -14,7 +14,7 @@ org.springframework.data spring-data-relational-parent - 1.1.0.BUILD-SNAPSHOT + 1.1.0.DATAJDBC-370-SNAPSHOT ../pom.xml diff --git a/spring-data-jdbc/pom.xml b/spring-data-jdbc/pom.xml index cfb1e9443b..9ed7427d84 100644 --- a/spring-data-jdbc/pom.xml +++ b/spring-data-jdbc/pom.xml @@ -5,7 +5,7 @@ 4.0.0 spring-data-jdbc - 1.1.0.BUILD-SNAPSHOT + 1.1.0.DATAJDBC-370-SNAPSHOT Spring Data JDBC Spring Data module for JDBC repositories. @@ -14,7 +14,7 @@ org.springframework.data spring-data-relational-parent - 1.1.0.BUILD-SNAPSHOT + 1.1.0.DATAJDBC-370-SNAPSHOT diff --git a/spring-data-relational/pom.xml b/spring-data-relational/pom.xml index ba0ffea143..ea6fbff70b 100644 --- a/spring-data-relational/pom.xml +++ b/spring-data-relational/pom.xml @@ -5,7 +5,7 @@ 4.0.0 spring-data-relational - 1.1.0.BUILD-SNAPSHOT + 1.1.0.DATAJDBC-370-SNAPSHOT Spring Data Relational Spring Data Relational support @@ -13,7 +13,7 @@ org.springframework.data spring-data-relational-parent - 1.1.0.BUILD-SNAPSHOT + 1.1.0.DATAJDBC-370-SNAPSHOT From feb6b5410236282aa546f05ba10b609c15a7ee0a Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Wed, 8 May 2019 09:46:43 +0200 Subject: [PATCH 2/6] DATAJDBC-370 - Polishing. Adding a missing issue reference to a test. --- .../data/jdbc/core/JdbcAggregateTemplateIntegrationTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateTemplateIntegrationTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateTemplateIntegrationTests.java index f2f910fa34..ffe40093c9 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateTemplateIntegrationTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateTemplateIntegrationTests.java @@ -436,7 +436,7 @@ public void saveAndLoadAnEntityWithByteArray() { assertThat(reloaded.binaryData).isEqualTo(new byte[] { 1, 23, 42 }); } - @Test + @Test // DATAJDBC-340 public void saveAndLoadLongChain() { Chain4 chain4 = new Chain4(); From 654c5157e71d1300b9f46febe28744e1dbe105cd Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Wed, 8 May 2019 10:07:57 +0200 Subject: [PATCH 3/6] 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. --- .../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 7b9a48f33f..003d10fd03 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.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 ReadingContext(RelationalPersistentEntity entity, DataAccessStrategy a } 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); - } - - T result = createInstanceInternal(entity, idValue); + Object idValue = idProperty == null ? null : readFrom(idProperty); - 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 @@ private T populateProperties(T result) { continue; } - propertyAccessor.setProperty(property, readOrLoadProperty(id, property)); + propertyAccessor.setProperty(property, readOrLoadProperty(idValue, property)); } return propertyAccessor.getBean(); @@ -358,27 +349,17 @@ private Object readFrom(RelationalPersistentProperty property) { } @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 @@ private S readEntityFrom(RelationalPersistentProperty property, PersistentPr 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 @@ private Object getObjectFromResultSet(String backreferenceName) { } } - 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 @@ private S createInstanceInternal(RelationalPersistentEntity entity, @Null 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 2f3a62377b..a8c43e830b 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 void chainedEntitiesWithoutId() throws SQLException { 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 @@ private boolean next() { } } - @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 SetValue endUpIn(Function extractor) { @AllArgsConstructor private static class Fixture { + final ResultSet resultSet; final List> expectations; From 9cc2b446367fd62110aaefdb272e9bd717e02e97 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Thu, 9 May 2019 11:03:02 +0200 Subject: [PATCH 4/6] DATAJDBC-370 - Polishing. Reade wither method and add explicit test for immutable value. --- .../convert/EntityRowMapperUnitTests.java | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) 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 a8c43e830b..0e6447a414 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 @@ -23,6 +23,7 @@ import lombok.AllArgsConstructor; import lombok.RequiredArgsConstructor; +import lombok.Value; import lombok.experimental.Wither; import java.sql.ResultSet; @@ -60,6 +61,7 @@ * @author Mark Paluch * @author Maciej Walkowiak * @author Bastian Wilhelm + * @author Christoph Strobl */ public class EntityRowMapperUnitTests { @@ -284,8 +286,24 @@ public void chainedEntitiesWithoutId() throws SQLException { fixture.assertOn(extracted); } + @Test // DATAJDBC-370 + public void simpleImmutableEmbeddedGetsProperlyExtracted() throws SQLException { + + ResultSet rs = mockResultSet(asList("id", "value"), // + ID_FOR_ENTITY_NOT_REFERENCING_MAP, "ru'Ha'"); + rs.next(); + + WithImmutableValue extracted = createRowMapper(WithImmutableValue.class).mapRow(rs, 1); + + assertThat(extracted) // + .isNotNull() // + .extracting(e -> e.id, e -> e.embeddedImmutableValue) // + .containsExactly(ID_FOR_ENTITY_NOT_REFERENCING_MAP, new ImmutableValue("ru'Ha'")); + } + // Model classes to be used in tests + @Wither @RequiredArgsConstructor static class TrivialImmutable { @@ -306,6 +324,7 @@ static class OneToOne { Trivial child; } + @Wither @RequiredArgsConstructor static class OneToOneImmutable { @@ -407,6 +426,17 @@ static class NoIdChain4 { NoIdChain3 chain3; } + static class WithImmutableValue { + + @Id Long id; + @Embedded ImmutableValue embeddedImmutableValue; + } + + @Value + static class ImmutableValue { + Object value; + } + // Infrastructure for assertions and constructing mocks private FixtureBuilder buildFixture() { From 23bee70aa5a2134e5101d03644cb6215114e0d86 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Thu, 9 May 2019 15:25:37 +0200 Subject: [PATCH 5/6] DATAJDBC-370 - Fix read/write of nulled embedded objects. We now allow read and write of Objects annotated with Embedded that are actually null. When writing all contained fields will be nulled. Reading back the entity considers an embedded object to be null itself if all contained properties are null within the backing result. Relates to DATAJDBC-364 --- .../jdbc/core/convert/BasicJdbcConverter.java | 34 ++++- .../convert/DefaultDataAccessStrategy.java | 33 +++- .../convert/EntityRowMapperUnitTests.java | 141 +++++++++++++++++- ...dbcRepositoryEmbeddedIntegrationTests.java | 10 +- 4 files changed, 205 insertions(+), 13 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 003d10fd03..4ae014c391 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 @@ -52,6 +52,7 @@ * * @author Mark Paluch * @author Jens Schauder + * @author Christoph Strobl * @see MappingContext * @see SimpleTypeHolder * @see CustomConversions @@ -345,15 +346,40 @@ private Object readFrom(RelationalPersistentProperty property) { Object value = getObjectFromResultSet(path.extendBy(property).getColumnAlias()); return readValue(value, property.getTypeInformation()); - } - @SuppressWarnings("unchecked") + @Nullable private Object readEmbeddedEntityFrom(@Nullable Object idValue, RelationalPersistentProperty property) { - ReadingContext newContext = extendBy(property); + ReadingContext ctx = extendBy(property); + return hasInstanceValues(property, ctx) ? ctx.createInstanceInternal(idValue) : null; + } - return newContext.createInstanceInternal(idValue); + private boolean hasInstanceValues(RelationalPersistentProperty property, ReadingContext ctx) { + + RelationalPersistentEntity persistentEntity = getMappingContext() + .getPersistentEntity(property.getTypeInformation()); + + PersistentPropertyPathExtension extension = ctx.path; + + for (RelationalPersistentProperty embeddedProperty : persistentEntity) { + + if (embeddedProperty.isQualified() || embeddedProperty.isReference()) { + return true; + } + + try { + if (ctx.getObjectFromResultSet(extension.extendBy(embeddedProperty).getColumnName()) != null) { + return true; + } + } catch (MappingException e) { + if (ctx.getObjectFromResultSet(extension.extendBy(embeddedProperty).getReverseColumnNameAlias()) != null) { + return true; + } + } + } + + return false; } @Nullable diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DefaultDataAccessStrategy.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DefaultDataAccessStrategy.java index 685f5f80a8..4b66d647ad 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DefaultDataAccessStrategy.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DefaultDataAccessStrategy.java @@ -54,6 +54,7 @@ * @author Mark Paluch * @author Thomas Lang * @author Bastian Wilhelm + * @author Christoph Strobl * @since 1.1 */ public class DefaultDataAccessStrategy implements DataAccessStrategy { @@ -313,7 +314,8 @@ private MapSqlParameterSource getParameterSource(S instance, RelationalPe MapSqlParameterSource parameters = new MapSqlParameterSource(); - PersistentPropertyAccessor propertyAccessor = persistentEntity.getPropertyAccessor(instance); + PersistentPropertyAccessor propertyAccessor = instance != null ? persistentEntity.getPropertyAccessor(instance) + : NoValuePropertyAccessor.instance(); persistentEntity.doWithProperties((PropertyHandler) property -> { @@ -480,4 +482,33 @@ static Predicate includeAll() { return it -> false; } } + + /** + * A {@link PersistentPropertyAccessor} implementation always returning null + * + * @param + */ + static class NoValuePropertyAccessor implements PersistentPropertyAccessor { + + private static final NoValuePropertyAccessor INSTANCE = new NoValuePropertyAccessor(); + + static NoValuePropertyAccessor instance() { + return INSTANCE; + } + + @Override + public void setProperty(PersistentProperty property, Object value) { + throw new UnsupportedOperationException("Cannot set value on 'null' target object."); + } + + @Override + public Object getProperty(PersistentProperty property) { + return null; + } + + @Override + public T getBean() { + return null; + } + } } 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 0e6447a414..2004a3357c 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 @@ -18,11 +18,15 @@ import static java.util.Arrays.*; import static java.util.Collections.*; import static org.assertj.core.api.Assertions.*; -import static org.mockito.ArgumentMatchers.*; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.*; import lombok.AllArgsConstructor; +import lombok.EqualsAndHashCode; +import lombok.NoArgsConstructor; import lombok.RequiredArgsConstructor; +import lombok.SneakyThrows; import lombok.Value; import lombok.experimental.Wither; @@ -301,6 +305,101 @@ public void simpleImmutableEmbeddedGetsProperlyExtracted() throws SQLException { .containsExactly(ID_FOR_ENTITY_NOT_REFERENCING_MAP, new ImmutableValue("ru'Ha'")); } + @Test // DATAJDBC-370 + @SneakyThrows + public void simplePrimitiveImmutableEmbeddedGetsProperlyExtracted() { + + ResultSet rs = mockResultSet(asList("id", "value"), // + ID_FOR_ENTITY_NOT_REFERENCING_MAP, 24); + rs.next(); + + WithPrimitiveImmutableValue extracted = createRowMapper(WithPrimitiveImmutableValue.class).mapRow(rs, 1); + + assertThat(extracted) // + .isNotNull() // + .extracting(e -> e.id, e -> e.embeddedImmutablePrimitiveValue) // + .containsExactly(ID_FOR_ENTITY_NOT_REFERENCING_MAP, new ImmutablePrimitiveValue(24)); + } + + @Test // DATAJDBC-370 + public void simpleImmutableEmbeddedShouldBeNullIfAllOfTheEmbeddableAreNull() throws SQLException { + + ResultSet rs = mockResultSet(asList("id", "value"), // + ID_FOR_ENTITY_NOT_REFERENCING_MAP, null); + rs.next(); + + WithImmutableValue extracted = createRowMapper(WithImmutableValue.class).mapRow(rs, 1); + + assertThat(extracted) // + .isNotNull() // + .extracting(e -> e.id, e -> e.embeddedImmutableValue) // + .containsExactly(ID_FOR_ENTITY_NOT_REFERENCING_MAP, null); + } + + @Test // DATAJDBC-370 + @SneakyThrows + public void embeddedShouldBeNullWhenFieldsAreNull() { + + ResultSet rs = mockResultSet(asList("id", "name", "prefix_id", "prefix_name"), // + ID_FOR_ENTITY_NOT_REFERENCING_MAP, "alpha", null, null); + rs.next(); + + EmbeddedEntity extracted = createRowMapper(EmbeddedEntity.class).mapRow(rs, 1); + + assertThat(extracted) // + .isNotNull() // + .extracting(e -> e.id, e -> e.name, e -> e.children) // + .containsExactly(ID_FOR_ENTITY_NOT_REFERENCING_MAP, "alpha", null); + } + + @Test // DATAJDBC-370 + @SneakyThrows + public void embeddedShouldNotBeNullWhenAtLeastOneFieldIsNotNull() { + + ResultSet rs = mockResultSet(asList("id", "name", "prefix_id", "prefix_name"), // + ID_FOR_ENTITY_NOT_REFERENCING_MAP, "alpha", 24, null); + rs.next(); + + EmbeddedEntity extracted = createRowMapper(EmbeddedEntity.class).mapRow(rs, 1); + + assertThat(extracted) // + .isNotNull() // + .extracting(e -> e.id, e -> e.name, e -> e.children) // + .containsExactly(ID_FOR_ENTITY_NOT_REFERENCING_MAP, "alpha", new Trivial(24L, null)); + } + + @Test // DATAJDBC-370 + @SneakyThrows + public void primitiveEmbeddedShouldBeNullWhenNoValuePresent() { + + ResultSet rs = mockResultSet(asList("id", "value"), // + ID_FOR_ENTITY_NOT_REFERENCING_MAP, null); + rs.next(); + + WithPrimitiveImmutableValue extracted = createRowMapper(WithPrimitiveImmutableValue.class).mapRow(rs, 1); + + assertThat(extracted) // + .isNotNull() // + .extracting(e -> e.id, e -> e.embeddedImmutablePrimitiveValue) // + .containsExactly(ID_FOR_ENTITY_NOT_REFERENCING_MAP, null); + } + + @Test // DATAJDBC-370 + @SneakyThrows + public void deepNestedEmbeddable() { + + ResultSet rs = mockResultSet(asList("id", "level0", "level1_value", "level1_level2_value"), // + ID_FOR_ENTITY_NOT_REFERENCING_MAP, "0", "1", "2"); + rs.next(); + + WithDeepNestedEmbeddable extracted = createRowMapper(WithDeepNestedEmbeddable.class).mapRow(rs, 1); + + assertThat(extracted) // + .isNotNull() // + .extracting(e -> e.id, e -> extracted.level0, e -> e.level1.value, e -> e.level1.level2.value) // + .containsExactly(ID_FOR_ENTITY_NOT_REFERENCING_MAP, "0", "1", "2"); + } + // Model classes to be used in tests @Wither @@ -311,6 +410,9 @@ static class TrivialImmutable { private final String name; } + @EqualsAndHashCode + @NoArgsConstructor + @AllArgsConstructor static class Trivial { @Id Long id; @@ -432,11 +534,35 @@ static class WithImmutableValue { @Embedded ImmutableValue embeddedImmutableValue; } + static class WithPrimitiveImmutableValue { + + @Id Long id; + @Embedded ImmutablePrimitiveValue embeddedImmutablePrimitiveValue; + } + @Value static class ImmutableValue { Object value; } + @Value + static class ImmutablePrimitiveValue { + int value; + } + + static class WithDeepNestedEmbeddable { + + @Id Long id; + String level0; + @Embedded("level1_") EmbeddedWithEmbedded level1; + } + + static class EmbeddedWithEmbedded { + + Object value; + @Embedded("level2_") ImmutableValue level2; + } + // Infrastructure for assertions and constructing mocks private FixtureBuilder buildFixture() { @@ -454,22 +580,23 @@ private EntityRowMapper createRowMapper(Class type, NamingStrategy nam DataAccessStrategy accessStrategy = mock(DataAccessStrategy.class); // the ID of the entity is used to determine what kind of ResultSet is needed for subsequent selects. - doReturn(new HashSet<>(asList(new Trivial(), new Trivial()))).when(accessStrategy) + doReturn(new HashSet<>(asList(new Trivial(1L, "one"), new Trivial(2L, "two")))).when(accessStrategy) .findAllByProperty(eq(ID_FOR_ENTITY_NOT_REFERENCING_MAP), any(RelationalPersistentProperty.class)); doReturn(new HashSet<>(asList( // - new SimpleEntry<>("one", new Trivial()), // - new SimpleEntry<>("two", new Trivial()) // + new SimpleEntry<>("one", new Trivial(1L, "one")), // + new SimpleEntry<>("two", new Trivial(2L, "two")) // ))).when(accessStrategy).findAllByProperty(eq(ID_FOR_ENTITY_REFERENCING_MAP), any(RelationalPersistentProperty.class)); doReturn(new HashSet<>(asList( // - new SimpleEntry<>(1, new Trivial()), // - new SimpleEntry<>(2, new Trivial()) // + new SimpleEntry<>(1, new Trivial(1L, "one")), // + new SimpleEntry<>(2, new Trivial(2L, "tow")) // ))).when(accessStrategy).findAllByProperty(eq(ID_FOR_ENTITY_REFERENCING_LIST), any(RelationalPersistentProperty.class)); - JdbcConverter converter = new BasicJdbcConverter(context, new JdbcCustomConversions()); + JdbcConverter converter = new BasicJdbcConverter(context, new JdbcCustomConversions(), + JdbcTypeFactory.unsupported()); return new EntityRowMapper<>( // (RelationalPersistentEntity) context.getRequiredPersistentEntity(type), // diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/JdbcRepositoryEmbeddedIntegrationTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/JdbcRepositoryEmbeddedIntegrationTests.java index 5d0ac7609f..924c728972 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/JdbcRepositoryEmbeddedIntegrationTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/JdbcRepositoryEmbeddedIntegrationTests.java @@ -44,6 +44,7 @@ * Very simple use cases for creation and usage of JdbcRepositories with test {@link Embedded} annotation in Entities. * * @author Bastian Wilhelm + * @author Christoph Strobl */ @ContextConfiguration @Transactional @@ -209,6 +210,14 @@ public void deleteAll() { assertThat(repository.findAll()).isEmpty(); } + @Test // DATAJDBC-370 + public void saveWithNullValueEmbeddable() { + + DummyEntity entity = repository.save(new DummyEntity()); + + assertThat(JdbcTestUtils.countRowsInTableWhere((JdbcTemplate) template.getJdbcOperations(), "dummy_entity", + "id = " + entity.getId())).isEqualTo(1); + } private static DummyEntity createDummyEntity() { DummyEntity entity = new DummyEntity(); @@ -222,7 +231,6 @@ private static DummyEntity createDummyEntity() { entity.setPrefixedEmbeddable(prefixedCascadedEmbeddable); - final CascadedEmbeddable cascadedEmbeddable = new CascadedEmbeddable(); cascadedEmbeddable.setTest("c2"); From 876e7790f4de19e650f6d6a03ba81e6c56c28f07 Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Fri, 10 May 2019 13:03:05 +0200 Subject: [PATCH 6/6] DATAJDBC-370 - Polishing. Refactored code to better use existing code. --- .../jdbc/core/convert/BasicJdbcConverter.java | 22 ++++++------------- 1 file changed, 7 insertions(+), 15 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 4ae014c391..70289e008f 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 @@ -351,31 +351,23 @@ private Object readFrom(RelationalPersistentProperty property) { @Nullable private Object readEmbeddedEntityFrom(@Nullable Object idValue, RelationalPersistentProperty property) { - ReadingContext ctx = extendBy(property); - return hasInstanceValues(property, ctx) ? ctx.createInstanceInternal(idValue) : null; + ReadingContext newContext = extendBy(property); + return newContext.hasInstanceValues(idValue) ? newContext.createInstanceInternal(idValue) : null; } - private boolean hasInstanceValues(RelationalPersistentProperty property, ReadingContext ctx) { + private boolean hasInstanceValues(Object idValue) { - RelationalPersistentEntity persistentEntity = getMappingContext() - .getPersistentEntity(property.getTypeInformation()); - - PersistentPropertyPathExtension extension = ctx.path; + RelationalPersistentEntity persistentEntity = path.getLeafEntity(); for (RelationalPersistentProperty embeddedProperty : persistentEntity) { + // if the embedded contains Lists, Sets or Maps we consider it non-empty if (embeddedProperty.isQualified() || embeddedProperty.isReference()) { return true; } - try { - if (ctx.getObjectFromResultSet(extension.extendBy(embeddedProperty).getColumnName()) != null) { - return true; - } - } catch (MappingException e) { - if (ctx.getObjectFromResultSet(extension.extendBy(embeddedProperty).getReverseColumnNameAlias()) != null) { - return true; - } + if (readOrLoadProperty(idValue, embeddedProperty) != null) { + return true; } }