Polishing.

Remove Jetbrains annotation usage.

Simplify code, remove code that is commented out, extract methods, pass PersistentEntity as argument instead of creating instances that hold PersistentEntity as field to align the class lifecycle with its contextual usage.

Refactor AggregateReader lifecycle, use a single instance as there is no entity-specific state attached to AggregateReader.

Add Javadoc.

See #1448
Original pull request: #1622

Author: mp911de

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

@@ -48,30 +48,24 @@
  * intermediate {@link RowDocumentResultSetExtractor RowDocument} and mapped via
  * {@link org.springframework.data.relational.core.conversion.RelationalConverter#read(Class, RowDocument)}.
  *
- * @param <T> the type of aggregate produced by this reader.
  * @author Jens Schauder
  * @author Mark Paluch
  * @since 3.2
  */
-class AggregateReader<T> implements PathToColumnMapping {
+class AggregateReader implements PathToColumnMapping {
 
-	private final RelationalPersistentEntity<T> aggregate;
-	private final Table table;
+	private final AliasFactory aliasFactory;
 	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,
-			NamedParameterJdbcOperations jdbcTemplate, RelationalPersistentEntity<T> aggregate) {
+	AggregateReader(Dialect dialect, JdbcConverter converter, NamedParameterJdbcOperations jdbcTemplate) {
 
+		this.aliasFactory = new AliasFactory();
 		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.sqlGenerator = new SingleQuerySqlGenerator(converter.getMappingContext(), aliasFactory, dialect);
 		this.extractor = new RowDocumentResultSetExtractor(converter.getMappingContext(), this);
 	}
 
@@ -92,55 +86,96 @@ public String keyColumn(AggregatePath path) {
 		return aliasFactory.getKeyAlias(path);
 	}
 
+	/**
+	 * Select a single aggregate by its identifier.
+	 *
+	 * @param id the identifier, must not be {@literal null}.
+	 * @param entity the persistent entity type must not be {@literal null}.
+	 * @return the found aggregate root, or {@literal null} if not found.
+	 * @param <T> aggregator type.
+	 */
 	@Nullable
-	public T findById(Object id) {
+	public <T> T findById(Object id, RelationalPersistentEntity<T> entity) {
 
-		Query query = Query.query(Criteria.where(aggregate.getRequiredIdProperty().getName()).is(id)).limit(1);
+		Query query = Query.query(Criteria.where(entity.getRequiredIdProperty().getName()).is(id)).limit(1);
 
-		return findOne(query);
+		return findOne(query, entity);
 	}
 
+	/**
+	 * Select a single aggregate by a {@link Query}.
+	 *
+	 * @param query the query to run, must not be {@literal null}.
+	 * @param entity the persistent entity type must not be {@literal null}.
+	 * @return the found aggregate root, or {@literal null} if not found.
+	 * @param <T> aggregator type.
+	 */
 	@Nullable
-	public T findOne(Query query) {
-		return doFind(query, this::extractZeroOrOne);
+	public <T> T findOne(Query query, RelationalPersistentEntity<T> entity) {
+		return doFind(query, entity, rs -> extractZeroOrOne(rs, entity));
 	}
 
-	public List<T> findAllById(Iterable<?> ids) {
+	/**
+	 * Select aggregates by their identifiers.
+	 *
+	 * @param ids the identifiers, must not be {@literal null}.
+	 * @param entity the persistent entity type must not be {@literal null}.
+	 * @return the found aggregate roots. The resulting list can be empty or may not contain objects that correspond to
+	 *         the identifiers when the objects are not found in the database.
+	 * @param <T> aggregator type.
+	 */
+	public <T> List<T> findAllById(Iterable<?> ids, RelationalPersistentEntity<T> entity) {
 
 		Collection<?> identifiers = ids instanceof Collection<?> idl ? idl : Streamable.of(ids).toList();
-		Query query = Query.query(Criteria.where(aggregate.getRequiredIdProperty().getName()).in(identifiers));
+		Query query = Query.query(Criteria.where(entity.getRequiredIdProperty().getName()).in(identifiers));
 
-		return findAll(query);
+		return findAll(query, entity);
 	}
 
+	/**
+	 * Select all aggregates by type.
+	 *
+	 * @param entity the persistent entity type must not be {@literal null}.
+	 * @return the found aggregate roots.
+	 * @param <T> aggregator type.
+	 */
 	@SuppressWarnings("ConstantConditions")
-	public List<T> findAll() {
-		return jdbcTemplate.query(sqlGenerator.findAll(), this::extractAll);
+	public <T> List<T> findAll(RelationalPersistentEntity<T> entity) {
+		return jdbcTemplate.query(sqlGenerator.findAll(entity),
+				(ResultSetExtractor<? extends List<T>>) rs -> extractAll(rs, entity));
 	}
 
-	public List<T> findAll(Query query) {
-		return doFind(query, this::extractAll);
+	/**
+	 * Select all aggregates by query.
+	 *
+	 * @param query the query to run, must not be {@literal null}.
+	 * @param entity the persistent entity type must not be {@literal null}.
+	 * @return the found aggregate roots.
+	 * @param <T> aggregator type.
+	 */
+	public <T> List<T> findAll(Query query, RelationalPersistentEntity<T> entity) {
+		return doFind(query, entity, rs -> extractAll(rs, entity));
 	}
 
 	@SuppressWarnings("ConstantConditions")
-	private <R> R doFind(Query query, ResultSetExtractor<R> extractor) {
+	private <T, R> R doFind(Query query, RelationalPersistentEntity<T> entity, ResultSetExtractor<R> extractor) {
 
 		MapSqlParameterSource parameterSource = new MapSqlParameterSource();
-		Condition condition = createCondition(query, parameterSource);
-		String sql = sqlGenerator.findAll(condition);
+		Condition condition = createCondition(query, parameterSource, entity);
+		String sql = sqlGenerator.findAll(entity, condition);
 
 		return jdbcTemplate.query(sql, parameterSource, extractor);
 	}
 
 	@Nullable
-	private Condition createCondition(Query query, MapSqlParameterSource parameterSource) {
+	private Condition createCondition(Query query, MapSqlParameterSource parameterSource,
+			RelationalPersistentEntity<?> entity) {
 
 		QueryMapper queryMapper = new QueryMapper(converter);
 
 		Optional<CriteriaDefinition> criteria = query.getCriteria();
-		return criteria
-				.map(criteriaDefinition -> queryMapper.getMappedObject(parameterSource, criteriaDefinition, table, aggregate))
-				.orElse(null);
+		return criteria.map(criteriaDefinition -> queryMapper.getMappedObject(parameterSource, criteriaDefinition,
+				Table.create(entity.getQualifiedTableName()), entity)).orElse(null);
 	}
 
 	/**
@@ -152,12 +187,13 @@ private Condition createCondition(Query query, MapSqlParameterSource parameterSo
 	 * @return a {@code List} of aggregates, fully converted.
 	 * @throws SQLException on underlying JDBC errors.
 	 */
-	private List<T> extractAll(ResultSet rs) throws SQLException {
+	private <T> List<T> extractAll(ResultSet rs, RelationalPersistentEntity<T> entity) throws SQLException {
 
-		Iterator<RowDocument> iterate = extractor.iterate(aggregate, rs);
+		Iterator<RowDocument> iterate = extractor.iterate(entity, rs);
 		List<T> resultList = new ArrayList<>();
+
 		while (iterate.hasNext()) {
-			resultList.add(converter.read(aggregate.getType(), iterate.next()));
+			resultList.add(converter.read(entity.getType(), iterate.next()));
 		}
 
 		return resultList;
@@ -175,17 +211,19 @@ private List<T> extractAll(ResultSet rs) throws SQLException {
 	 * @throws IncorrectResultSizeDataAccessException when the conversion yields more than one instance.
 	 */
 	@Nullable
-	private T extractZeroOrOne(ResultSet rs) throws SQLException {
+	private <T> T extractZeroOrOne(ResultSet rs, RelationalPersistentEntity<T> entity) throws SQLException {
+
+		Iterator<RowDocument> iterate = extractor.iterate(entity, rs);
 
-		Iterator<RowDocument> iterate = extractor.iterate(aggregate, rs);
 		if (iterate.hasNext()) {
 
 			RowDocument object = iterate.next();
 			if (iterate.hasNext()) {
 				throw new IncorrectResultSizeDataAccessException(1);
 			}
-			return converter.read(aggregate.getType(), object);
+			return converter.read(entity.getType(), object);
 		}
+
 		return null;
 	}
 

Diff for: spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SingleQueryDataAccessStrategy.java

@@ -25,9 +25,7 @@
 import org.springframework.data.relational.core.mapping.RelationalMappingContext;
 import org.springframework.data.relational.core.mapping.RelationalPersistentEntity;
 import org.springframework.data.relational.core.query.Query;
-import org.springframework.data.relational.core.sqlgeneration.AliasFactory;
 import org.springframework.jdbc.core.namedparam.NamedParameterJdbcOperations;
-import org.springframework.util.ConcurrentLruCache;
 
 /**
  * A {@link ReadingDataAccessStrategy} that uses an {@link AggregateReader} to load entities with a single query.
@@ -39,31 +37,28 @@
 class SingleQueryDataAccessStrategy implements ReadingDataAccessStrategy {
 
 	private final RelationalMappingContext mappingContext;
-	private final AliasFactory aliasFactory;
-	private final ConcurrentLruCache<RelationalPersistentEntity<?>, AggregateReader<?>> readerCache;
+	private final AggregateReader aggregateReader;
 
 	public SingleQueryDataAccessStrategy(Dialect dialect, JdbcConverter converter,
 			NamedParameterJdbcOperations jdbcTemplate) {
 
 		this.mappingContext = converter.getMappingContext();
-		this.aliasFactory = new AliasFactory();
-		this.readerCache = new ConcurrentLruCache<>(256,
-				entity -> new AggregateReader<>(dialect, converter, aliasFactory, jdbcTemplate, entity));
+		this.aggregateReader = new AggregateReader(dialect, converter, jdbcTemplate);
 	}
 
 	@Override
 	public <T> T findById(Object id, Class<T> domainType) {
-		return getReader(domainType).findById(id);
+		return aggregateReader.findById(id, getPersistentEntity(domainType));
 	}
 
 	@Override
 	public <T> List<T> findAll(Class<T> domainType) {
-		return getReader(domainType).findAll();
+		return aggregateReader.findAll(getPersistentEntity(domainType));
 	}
 
 	@Override
 	public <T> List<T> findAllById(Iterable<?> ids, Class<T> domainType) {
-		return getReader(domainType).findAllById(ids);
+		return aggregateReader.findAllById(ids, getPersistentEntity(domainType));
 	}
 
 	@Override
@@ -78,12 +73,12 @@ public <T> List<T> findAll(Class<T> domainType, Pageable pageable) {
 
 	@Override
 	public <T> Optional<T> findOne(Query query, Class<T> domainType) {
-		return Optional.ofNullable(getReader(domainType).findOne(query));
+		return Optional.ofNullable(aggregateReader.findOne(query, getPersistentEntity(domainType)));
 	}
 
 	@Override
 	public <T> List<T> findAll(Query query, Class<T> domainType) {
-		return getReader(domainType).findAll(query);
+		return aggregateReader.findAll(query, getPersistentEntity(domainType));
 	}
 
 	@Override
@@ -92,11 +87,7 @@ public <T> List<T> findAll(Query query, Class<T> domainType, Pageable pageable)
 	}
 
 	@SuppressWarnings("unchecked")
-	private <T> AggregateReader<T> getReader(Class<T> domainType) {
-
-		RelationalPersistentEntity<T> persistentEntity = (RelationalPersistentEntity<T>) mappingContext
-				.getRequiredPersistentEntity(domainType);
-
-		return (AggregateReader<T>) readerCache.get(persistentEntity);
+	private <T> RelationalPersistentEntity<T> getPersistentEntity(Class<T> domainType) {
+		return (RelationalPersistentEntity<T>) mappingContext.getRequiredPersistentEntity(domainType);
 	}
 }