From 7c1f031f1224a4be33ee8d6d4ad8d31ff727a455 Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Mon, 20 Aug 2018 15:24:17 +0200 Subject: [PATCH 1/4] DATAJDBC-252 - Prepare branch. --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 9b63bcc527..50c6c83b4f 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-jdbc - 1.0.0.BUILD-SNAPSHOT + 1.0.0.DATAJDBC-252-SNAPSHOT Spring Data JDBC Spring Data module for JDBC repositories. From cb1df95d23b081f84d19e00806ccdf59dfa6f6f9 Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Mon, 20 Aug 2018 15:38:30 +0200 Subject: [PATCH 2/4] DATAJDBC-252 - Properties only get set if they are not part of the constructor. For mutable entities this avoids superfluous setting of attributes. For imutables without "Wither"s this avoids failures due to not being able to set a property. --- .../data/jdbc/core/EntityRowMapper.java | 7 +++++ .../jdbc/core/EntityRowMapperUnitTests.java | 30 +++++++++++++++++-- .../QueryAnnotationHsqlIntegrationTests.java | 18 +++++++++++ 3 files changed, 52 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/springframework/data/jdbc/core/EntityRowMapper.java b/src/main/java/org/springframework/data/jdbc/core/EntityRowMapper.java index 1da0c3ba74..b904944a60 100644 --- a/src/main/java/org/springframework/data/jdbc/core/EntityRowMapper.java +++ b/src/main/java/org/springframework/data/jdbc/core/EntityRowMapper.java @@ -23,6 +23,7 @@ import org.springframework.data.mapping.MappingException; import org.springframework.data.mapping.PersistentProperty; import org.springframework.data.mapping.PersistentPropertyAccessor; +import org.springframework.data.mapping.PreferredConstructor; import org.springframework.data.relational.core.conversion.RelationalConverter; import org.springframework.data.relational.core.mapping.RelationalMappingContext; import org.springframework.data.relational.core.mapping.RelationalPersistentEntity; @@ -73,8 +74,14 @@ public T mapRow(ResultSet resultSet, int rowNumber) { Object id = idProperty == null ? null : readFrom(resultSet, idProperty, ""); + PreferredConstructor persistenceConstructor = entity.getPersistenceConstructor(); + for (RelationalPersistentProperty property : entity) { + if (persistenceConstructor != null && persistenceConstructor.isConstructorParameter(property)) { + continue; + } + if (property.isCollectionLike() && id != null) { propertyAccessor.setProperty(property, accessStrategy.findAllByProperty(id, property)); } else if (property.isMap() && id != null) { diff --git a/src/test/java/org/springframework/data/jdbc/core/EntityRowMapperUnitTests.java b/src/test/java/org/springframework/data/jdbc/core/EntityRowMapperUnitTests.java index 47d5bbb4aa..77f5920bda 100644 --- a/src/test/java/org/springframework/data/jdbc/core/EntityRowMapperUnitTests.java +++ b/src/test/java/org/springframework/data/jdbc/core/EntityRowMapperUnitTests.java @@ -22,6 +22,7 @@ import static org.mockito.Mockito.*; import lombok.RequiredArgsConstructor; +import lombok.experimental.Wither; import java.sql.ResultSet; import java.sql.SQLException; @@ -35,7 +36,6 @@ import javax.naming.OperationNotSupportedException; -import lombok.experimental.Wither; import org.junit.Test; import org.mockito.invocation.InvocationOnMock; import org.mockito.stubbing.Answer; @@ -47,6 +47,7 @@ import org.springframework.data.relational.core.mapping.RelationalMappingContext; import org.springframework.data.relational.core.mapping.RelationalPersistentEntity; import org.springframework.data.relational.core.mapping.RelationalPersistentProperty; +import org.springframework.data.repository.query.Param; import org.springframework.util.Assert; /** @@ -172,6 +173,27 @@ public void listReferenceGetsLoadedWithAdditionalSelect() throws SQLException { .containsExactly(ID_FOR_ENTITY_REFERENCING_LIST, "alpha", 2); } + @Test // DATAJDBC-252 + public void doesNotTryToSetPropertiesThatAreSetViaConstructor() throws SQLException { + + ResultSet rs = mockResultSet(asList("value"), // + "value-from-resultSet"); + rs.next(); + + DontUseSetter extracted = createRowMapper(DontUseSetter.class).mapRow(rs, 1); + + assertThat(extracted.value) // + .isEqualTo("setThroughConstructor:value-from-resultSet"); + } + + private static class DontUseSetter { + String value; + + DontUseSetter(@Param("value") String value) { + this.value = "setThroughConstructor:" + value; + } + } + private EntityRowMapper createRowMapper(Class type) { return createRowMapper(type, NamingStrategy.INSTANCE); } @@ -189,12 +211,14 @@ private EntityRowMapper createRowMapper(Class type, NamingStrategy nam doReturn(new HashSet<>(asList( // new SimpleEntry<>("one", new Trivial()), // new SimpleEntry<>("two", new Trivial()) // - ))).when(accessStrategy).findAllByProperty(eq(ID_FOR_ENTITY_REFERENCING_MAP), any(RelationalPersistentProperty.class)); + ))).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()) // - ))).when(accessStrategy).findAllByProperty(eq(ID_FOR_ENTITY_REFERENCING_LIST), any(RelationalPersistentProperty.class)); + ))).when(accessStrategy).findAllByProperty(eq(ID_FOR_ENTITY_REFERENCING_LIST), + any(RelationalPersistentProperty.class)); RelationalConverter converter = new BasicRelationalConverter(context, new JdbcCustomConversions()); diff --git a/src/test/java/org/springframework/data/jdbc/repository/query/QueryAnnotationHsqlIntegrationTests.java b/src/test/java/org/springframework/data/jdbc/repository/query/QueryAnnotationHsqlIntegrationTests.java index 2c5df7db48..bec15dd5bf 100644 --- a/src/test/java/org/springframework/data/jdbc/repository/query/QueryAnnotationHsqlIntegrationTests.java +++ b/src/test/java/org/springframework/data/jdbc/repository/query/QueryAnnotationHsqlIntegrationTests.java @@ -24,6 +24,7 @@ import java.util.Optional; import java.util.stream.Stream; +import lombok.Value; import org.junit.ClassRule; import org.junit.Rule; import org.junit.Test; @@ -251,6 +252,12 @@ public void executeCustomModifyingQueryWithReturnTypeVoid() { assertThat(repository.findByNameAsEntity("Spring Data JDBC")).isNotNull(); } + @Test // DATAJDBC-175 + public void executeCustomQueryWithImmutableResultType() { + + assertThat(repository.immutableTuple()).isEqualTo(new DummyEntityRepository.ImmutableTuple("one", "two", 3)); + } + private DummyEntity dummyEntity(String name) { DummyEntity entity = new DummyEntity(); @@ -329,5 +336,16 @@ private interface DummyEntityRepository extends CrudRepository Date: Tue, 21 Aug 2018 07:28:30 +0200 Subject: [PATCH 3/4] DATAJDBC-252 - Checks if any population of properties is necessary at all. This avoids needless iterating over the properties for instances fully constructed by their constructor. --- .../data/jdbc/core/EntityRowMapper.java | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/springframework/data/jdbc/core/EntityRowMapper.java b/src/main/java/org/springframework/data/jdbc/core/EntityRowMapper.java index b904944a60..35ce65fcbe 100644 --- a/src/main/java/org/springframework/data/jdbc/core/EntityRowMapper.java +++ b/src/main/java/org/springframework/data/jdbc/core/EntityRowMapper.java @@ -70,6 +70,15 @@ public T mapRow(ResultSet resultSet, int rowNumber) { T result = createInstance(entity, resultSet, ""); + if (entity.requiresPropertyPopulation()) { + populateProperties(result, resultSet); + } + + return result; + } + + private void populateProperties(T result, ResultSet resultSet) { + PersistentPropertyAccessor propertyAccessor = converter.getPropertyAccessor(entity, result); Object id = idProperty == null ? null : readFrom(resultSet, idProperty, ""); @@ -92,8 +101,6 @@ public T mapRow(ResultSet resultSet, int rowNumber) { propertyAccessor.setProperty(property, readFrom(resultSet, property, "")); } } - - return result; } /** From abf6f9f52f0b16c13355391a55b9b544efef6dc1 Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Tue, 21 Aug 2018 10:39:57 +0200 Subject: [PATCH 4/4] DATAJDBC-252 - Population of properties handles entities with incomplete constructors correctly. --- .../data/jdbc/core/EntityRowMapper.java | 6 ++- .../jdbc/core/EntityRowMapperUnitTests.java | 49 +++++++++++++++++-- 2 files changed, 48 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/springframework/data/jdbc/core/EntityRowMapper.java b/src/main/java/org/springframework/data/jdbc/core/EntityRowMapper.java index 35ce65fcbe..9ff594c416 100644 --- a/src/main/java/org/springframework/data/jdbc/core/EntityRowMapper.java +++ b/src/main/java/org/springframework/data/jdbc/core/EntityRowMapper.java @@ -71,13 +71,13 @@ public T mapRow(ResultSet resultSet, int rowNumber) { T result = createInstance(entity, resultSet, ""); if (entity.requiresPropertyPopulation()) { - populateProperties(result, resultSet); + return populateProperties(result, resultSet); } return result; } - private void populateProperties(T result, ResultSet resultSet) { + private T populateProperties(T result, ResultSet resultSet) { PersistentPropertyAccessor propertyAccessor = converter.getPropertyAccessor(entity, result); @@ -101,6 +101,8 @@ private void populateProperties(T result, ResultSet resultSet) { propertyAccessor.setProperty(property, readFrom(resultSet, property, "")); } } + + return propertyAccessor.getBean(); } /** diff --git a/src/test/java/org/springframework/data/jdbc/core/EntityRowMapperUnitTests.java b/src/test/java/org/springframework/data/jdbc/core/EntityRowMapperUnitTests.java index 77f5920bda..b1bfb3efd2 100644 --- a/src/test/java/org/springframework/data/jdbc/core/EntityRowMapperUnitTests.java +++ b/src/test/java/org/springframework/data/jdbc/core/EntityRowMapperUnitTests.java @@ -40,6 +40,7 @@ import org.mockito.invocation.InvocationOnMock; import org.mockito.stubbing.Answer; import org.springframework.data.annotation.Id; +import org.springframework.data.annotation.PersistenceConstructor; import org.springframework.data.jdbc.core.convert.JdbcCustomConversions; import org.springframework.data.relational.core.conversion.BasicRelationalConverter; import org.springframework.data.relational.core.conversion.RelationalConverter; @@ -186,12 +187,18 @@ public void doesNotTryToSetPropertiesThatAreSetViaConstructor() throws SQLExcept .isEqualTo("setThroughConstructor:value-from-resultSet"); } - private static class DontUseSetter { - String value; + @Test // DATAJDBC-252 + public void handlesMixedProperties() throws SQLException { - DontUseSetter(@Param("value") String value) { - this.value = "setThroughConstructor:" + value; - } + ResultSet rs = mockResultSet(asList("one", "two", "three"), // + "111", "222", "333"); + rs.next(); + + MixedProperties extracted = createRowMapper(MixedProperties.class).mapRow(rs, 1); + + assertThat(extracted) // + .extracting(e -> e.one, e -> e.two, e -> e.three) // + .isEqualTo(new String[] { "111", "222", "333" }); } private EntityRowMapper createRowMapper(Class type) { @@ -369,4 +376,36 @@ static class OneToList { String name; List 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); + } + } }