DATAJDBC-252 - Optimize population of (immutable) entities to be created.

The entity creation not skips the property population entirely if the metamodel indicates that the instantiation already creates a complete entity. If there's need to actively populate properties, we now correctly skip the ones already consumed by the constructor.

Original pull request: #86.

Author: schauder

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;
@@ -69,12 +70,27 @@ public T mapRow(ResultSet resultSet, int rowNumber) {
 
 		T result = createInstance(entity, resultSet, "");
 
+		if (entity.requiresPropertyPopulation()) {
+			return populateProperties(result, resultSet);
+		}
+
+		return result;
+	}
+
+	private T populateProperties(T result, ResultSet resultSet) {
+
 		PersistentPropertyAccessor<T> propertyAccessor = converter.getPropertyAccessor(entity, result);
 
 		Object id = idProperty == null ? null : readFrom(resultSet, idProperty, "");
 
+		PreferredConstructor<T, RelationalPersistentProperty> 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) {
@@ -86,7 +102,7 @@ public T mapRow(ResultSet resultSet, int rowNumber) {
 			}
 		}
 
-		return result;
+		return propertyAccessor.getBean();
 	}
 
 	/**

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,18 +36,19 @@
 
 import javax.naming.OperationNotSupportedException;
 
-import lombok.experimental.Wither;
 import org.junit.Test;
 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;
 import org.springframework.data.relational.core.mapping.NamingStrategy;
 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 +174,33 @@ 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");
+	}
+
+	@Test // DATAJDBC-252
+	public void handlesMixedProperties() throws SQLException {
+
+		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 <T> EntityRowMapper<T> createRowMapper(Class<T> type) {
 		return createRowMapper(type, NamingStrategy.INSTANCE);
 	}
@@ -189,12 +218,14 @@ private <T> EntityRowMapper<T> createRowMapper(Class<T> 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());
 
@@ -345,4 +376,36 @@ static class OneToList {
 		String name;
 		List<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);
+		}
+	}
 }

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<DummyEntity, Long
 		@Query("INSERT INTO DUMMY_ENTITY (name) VALUES(:name)")
 		void insert(@Param("name") String name);
 
+		// DATAJDBC-252
+		@Query("SELECT 'one' one, 'two' two, 3 three FROM (VALUES (0))")
+		ImmutableTuple immutableTuple();
+
+
+		@Value
+		class ImmutableTuple {
+			String one;
+			String two;
+			int three;
+		}
 	}
 }