Support Single Query Loading for queries with where clauses

pom.xml

@@ -5,7 +5,7 @@
 
 	<groupId>org.springframework.data</groupId>
 	<artifactId>spring-data-relational-parent</artifactId>
-	<version>3.2.0-SNAPSHOT</version>
+	<version>3.2.0-1601-where-clause-SNAPSHOT</version>
 	<packaging>pom</packaging>
 
 	<name>Spring Data Relational Parent</name>
@@ -47,6 +47,9 @@
 		<!-- test utilities-->
 		<awaitility.version>4.2.0</awaitility.version>
 		<archunit.version>1.0.1</archunit.version>
+
+		<jmh.version>1.37</jmh.version>
+		<mbr.version>0.4.0.BUILD-SNAPSHOT</mbr.version>
 	</properties>
 
 	<inceptionYear>2017</inceptionYear>
@@ -154,6 +157,98 @@
 				</plugins>
 			</build>
 		</profile>
+
+		<profile>
+			<id>jmh</id>
+			<dependencies>
+				<dependency>
+					<groupId>com.github.mp911de.microbenchmark-runner</groupId>
+					<artifactId>microbenchmark-runner-junit5</artifactId>
+					<version>${mbr.version}</version>
+					<scope>test</scope>
+				</dependency>
+				<dependency>
+					<groupId>org.openjdk.jmh</groupId>
+					<artifactId>jmh-core</artifactId>
+					<version>${jmh.version}</version>
+					<scope>test</scope>
+				</dependency>
+				<dependency>
+					<groupId>org.openjdk.jmh</groupId>
+					<artifactId>jmh-generator-annprocess</artifactId>
+					<version>${jmh.version}</version>
+					<scope>test</scope>
+				</dependency>
+			</dependencies>
+			<build>
+				<plugins>
+					<plugin>
+						<groupId>org.codehaus.mojo</groupId>
+						<artifactId>build-helper-maven-plugin</artifactId>
+						<version>3.3.0</version>
+						<executions>
+							<execution>
+								<id>add-source</id>
+								<phase>generate-sources</phase>
+								<goals>
+									<goal>add-test-source</goal>
+								</goals>
+								<configuration>
+									<sources>
+										<source>src/jmh/java</source>
+									</sources>
+								</configuration>
+							</execution>
+						</executions>
+					</plugin>
+					<plugin>
+						<groupId>org.apache.maven.plugins</groupId>
+						<artifactId>maven-surefire-plugin</artifactId>
+						<configuration>
+							<skip>true</skip>
+						</configuration>
+					</plugin>
+
+					<plugin>
+						<groupId>org.apache.maven.plugins</groupId>
+						<artifactId>maven-failsafe-plugin</artifactId>
+						<configuration>
+							<skip>true</skip>
+						</configuration>
+					</plugin>
+					<plugin>
+						<groupId>org.codehaus.mojo</groupId>
+						<artifactId>exec-maven-plugin</artifactId>
+						<version>3.1.0</version>
+						<executions>
+							<execution>
+								<id>run-benchmarks</id>
+								<phase>pre-integration-test</phase>
+								<goals>
+									<goal>exec</goal>
+								</goals>
+								<configuration>
+									<classpathScope>test</classpathScope>
+									<executable>java</executable>
+									<arguments>
+										<argument>-classpath</argument>
+										<classpath/>
+										<argument>org.openjdk.jmh.Main</argument>
+										<argument>.*</argument>
+									</arguments>
+								</configuration>
+							</execution>
+						</executions>
+					</plugin>
+				</plugins>
+			</build>
+			<repositories>
+				<repository>
+					<id>jitpack.io</id>
+					<url>https://jitpack.io</url>
+				</repository>
+			</repositories>
+		</profile>
 	</profiles>
 
 	<build>

spring-data-jdbc-distribution/pom.xml

@@ -14,7 +14,7 @@
 	<parent>
 		<groupId>org.springframework.data</groupId>
 		<artifactId>spring-data-relational-parent</artifactId>
-		<version>3.2.0-SNAPSHOT</version>
+		<version>3.2.0-1601-where-clause-SNAPSHOT</version>
 		<relativePath>../pom.xml</relativePath>
 	</parent>
 

spring-data-jdbc/pom.xml

@@ -6,7 +6,7 @@
 	<modelVersion>4.0.0</modelVersion>
 
 	<artifactId>spring-data-jdbc</artifactId>
-	<version>3.2.0-SNAPSHOT</version>
+	<version>3.2.0-1601-where-clause-SNAPSHOT</version>
 
 	<name>Spring Data JDBC</name>
 	<description>Spring Data module for JDBC repositories.</description>
@@ -15,7 +15,7 @@
 	<parent>
 		<groupId>org.springframework.data</groupId>
 		<artifactId>spring-data-relational-parent</artifactId>
-		<version>3.2.0-SNAPSHOT</version>
+		<version>3.2.0-1601-where-clause-SNAPSHOT</version>
 	</parent>
 
 	<properties>

spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/AggregateReader.java

@@ -18,25 +18,33 @@
 import java.sql.ResultSet;
 import java.sql.SQLException;
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.Iterator;
 import java.util.List;
-import java.util.Map;
+import java.util.Optional;
 
 import org.springframework.dao.IncorrectResultSizeDataAccessException;
 import org.springframework.data.relational.core.dialect.Dialect;
 import org.springframework.data.relational.core.mapping.AggregatePath;
 import org.springframework.data.relational.core.mapping.RelationalPersistentEntity;
+import org.springframework.data.relational.core.query.Criteria;
+import org.springframework.data.relational.core.query.CriteriaDefinition;
+import org.springframework.data.relational.core.query.Query;
+import org.springframework.data.relational.core.sql.Condition;
+import org.springframework.data.relational.core.sql.Table;
 import org.springframework.data.relational.core.sqlgeneration.AliasFactory;
 import org.springframework.data.relational.core.sqlgeneration.SingleQuerySqlGenerator;
 import org.springframework.data.relational.core.sqlgeneration.SqlGenerator;
 import org.springframework.data.relational.domain.RowDocument;
+import org.springframework.data.util.Streamable;
+import org.springframework.jdbc.core.ResultSetExtractor;
+import org.springframework.jdbc.core.namedparam.MapSqlParameterSource;
 import org.springframework.jdbc.core.namedparam.NamedParameterJdbcOperations;
 import org.springframework.lang.Nullable;
-import org.springframework.util.Assert;
 
 /**
  * Reads complete Aggregates from the database, by generating appropriate SQL using a {@link SingleQuerySqlGenerator}
- * through {@link org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate}. Results are converterd into an
+ * through {@link org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate}. Results are converted into an
  * intermediate {@link RowDocumentResultSetExtractor RowDocument} and mapped via
  * {@link org.springframework.data.relational.core.conversion.RelationalConverter#read(Class, RowDocument)}.
  *
@@ -45,12 +53,14 @@
  * @author Mark Paluch
  * @since 3.2
  */
-class AggregateReader<T> {
+class AggregateReader<T> implements PathToColumnMapping {
 
 	private final RelationalPersistentEntity<T> aggregate;
-	private final org.springframework.data.relational.core.sqlgeneration.SqlGenerator sqlGenerator;
+	private final Table table;
+	private final SqlGenerator sqlGenerator;
 	private final JdbcConverter converter;
 	private final NamedParameterJdbcOperations jdbcTemplate;
+	private final AliasFactory aliasFactory;
 	private final RowDocumentResultSetExtractor extractor;
 
 	AggregateReader(Dialect dialect, JdbcConverter converter, AliasFactory aliasFactory,
@@ -59,44 +69,88 @@ class AggregateReader<T> {
 		this.converter = converter;
 		this.aggregate = aggregate;
 		this.jdbcTemplate = jdbcTemplate;
+		this.table = Table.create(aggregate.getQualifiedTableName());
+		this.sqlGenerator = new SingleQuerySqlGenerator(converter.getMappingContext(), aliasFactory, dialect, aggregate);
+		this.aliasFactory = aliasFactory;
+		this.extractor = new RowDocumentResultSetExtractor(converter.getMappingContext(), this);
+	}
+
+	@Override
+	public String column(AggregatePath path) {
 
-		this.sqlGenerator = new CachingSqlGenerator(
-				new SingleQuerySqlGenerator(converter.getMappingContext(), aliasFactory, dialect, aggregate));
+		String alias = aliasFactory.getColumnAlias(path);
+
+		if (alias == null) {
+			throw new IllegalStateException(String.format("Alias for '%s' must not be null", path));
+		}
 
-		this.extractor = new RowDocumentResultSetExtractor(converter.getMappingContext(),
-				createPathToColumnMapping(aliasFactory));
+		return alias;
 	}
 
-	public List<T> findAll() {
-		return jdbcTemplate.query(sqlGenerator.findAll(), this::extractAll);
+	@Override
+	public String keyColumn(AggregatePath path) {
+		return aliasFactory.getKeyAlias(path);
 	}
 
 	@Nullable
 	public T findById(Object id) {
 
-		id = converter.writeValue(id, aggregate.getRequiredIdProperty().getTypeInformation());
+		Query query = Query.query(Criteria.where(aggregate.getRequiredIdProperty().getName()).is(id)).limit(1);
 
-		return jdbcTemplate.query(sqlGenerator.findById(), Map.of("id", id), this::extractZeroOrOne);
+		return findOne(query);
 	}
 
-	public Iterable<T> findAllById(Iterable<?> ids) {
+	@Nullable
+	public T findOne(Query query) {
+		return doFind(query, this::extractZeroOrOne);
+	}
 
-		List<Object> convertedIds = new ArrayList<>();
-		for (Object id : ids) {
-			convertedIds.add(converter.writeValue(id, aggregate.getRequiredIdProperty().getTypeInformation()));
-		}
+	public List<T> findAllById(Iterable<?> ids) {
+
+		Collection<?> identifiers = ids instanceof Collection<?> idl ? idl : Streamable.of(ids).toList();
+		Query query = Query.query(Criteria.where(aggregate.getRequiredIdProperty().getName()).in(identifiers));
+
+		return findAll(query);
+	}
+
+	@SuppressWarnings("ConstantConditions")
+	public List<T> findAll() {
+		return jdbcTemplate.query(sqlGenerator.findAll(), this::extractAll);
+	}
+
+	public List<T> findAll(Query query) {
+		return doFind(query, this::extractAll);
+	}
+
+	@SuppressWarnings("ConstantConditions")
+	private <R> R doFind(Query query, ResultSetExtractor<R> extractor) {
+
+		MapSqlParameterSource parameterSource = new MapSqlParameterSource();
+		Condition condition = createCondition(query, parameterSource);
+		String sql = sqlGenerator.findAll(condition);
 
-		return jdbcTemplate.query(sqlGenerator.findAllById(), Map.of("ids", convertedIds), this::extractAll);
+		return jdbcTemplate.query(sql, parameterSource, extractor);
+	}
+
+	@Nullable
+	private Condition createCondition(Query query, MapSqlParameterSource parameterSource) {
+
+		QueryMapper queryMapper = new QueryMapper(converter);
+
+		Optional<CriteriaDefinition> criteria = query.getCriteria();
+		return criteria
+				.map(criteriaDefinition -> queryMapper.getMappedObject(parameterSource, criteriaDefinition, table, aggregate))
+				.orElse(null);
 	}
 
 	/**
 	 * Extracts a list of aggregates from the given {@link ResultSet} by utilizing the
 	 * {@link RowDocumentResultSetExtractor} and the {@link JdbcConverter}. When used as a method reference this conforms
 	 * to the {@link org.springframework.jdbc.core.ResultSetExtractor} contract.
-	 * 
+	 *
 	 * @param rs the {@link ResultSet} from which to extract the data. Must not be {(}@literal null}.
 	 * @return a {@code List} of aggregates, fully converted.
-	 * @throws SQLException
+	 * @throws SQLException on underlying JDBC errors.
 	 */
 	private List<T> extractAll(ResultSet rs) throws SQLException {
 
@@ -114,9 +168,10 @@ private List<T> extractAll(ResultSet rs) throws SQLException {
 	 * {@link RowDocumentResultSetExtractor} and the {@link JdbcConverter}. When used as a method reference this conforms
 	 * to the {@link org.springframework.jdbc.core.ResultSetExtractor} contract.
 	 *
-	 * @param @param rs the {@link ResultSet} from which to extract the data. Must not be {(}@literal null}.
-	 * @return The single instance when the conversion results in exactly one instance. If the {@literal ResultSet} is empty, null is returned.
-	 * @throws SQLException
+	 * @param rs the {@link ResultSet} from which to extract the data. Must not be {(}@literal null}.
+	 * @return The single instance when the conversion results in exactly one instance. If the {@literal ResultSet} is
+	 *         empty, null is returned.
+	 * @throws SQLException on underlying JDBC errors.
 	 * @throws IncorrectResultSizeDataAccessException when the conversion yields more than one instance.
 	 */
 	@Nullable
@@ -134,65 +189,4 @@ private T extractZeroOrOne(ResultSet rs) throws SQLException {
 		return null;
 	}
 
-	private PathToColumnMapping createPathToColumnMapping(AliasFactory aliasFactory) {
-		return new PathToColumnMapping() {
-			@Override
-			public String column(AggregatePath path) {
-
-				String alias = aliasFactory.getColumnAlias(path);
-				Assert.notNull(alias, () -> "alias for >" + path + "< must not be null");
-				return alias;
-			}
-
-			@Override
-			public String keyColumn(AggregatePath path) {
-				return aliasFactory.getKeyAlias(path);
-			}
-		};
-	}
-
-	/**
-	 * A wrapper for the {@link org.springframework.data.relational.core.sqlgeneration.SqlGenerator} that caches the
-	 * generated statements.
-	 *
-	 * @author Jens Schauder
-	 * @since 3.2
-	 */
-	static class CachingSqlGenerator implements org.springframework.data.relational.core.sqlgeneration.SqlGenerator {
-
-		private final org.springframework.data.relational.core.sqlgeneration.SqlGenerator delegate;
-
-		private final String findAll;
-		private final String findById;
-		private final String findAllById;
-
-		public CachingSqlGenerator(SqlGenerator delegate) {
-
-			this.delegate = delegate;
-
-			findAll = delegate.findAll();
-			findById = delegate.findById();
-			findAllById = delegate.findAllById();
-		}
-
-		@Override
-		public String findAll() {
-			return findAll;
-		}
-
-		@Override
-		public String findById() {
-			return findById;
-		}
-
-		@Override
-		public String findAllById() {
-			return findAllById;
-		}
-
-		@Override
-		public AliasFactory getAliasFactory() {
-			return delegate.getAliasFactory();
-		}
-	}
 }