Polishing.

Introduces the BatchedActions abstraction to encapsulate the different ways singular actions get combined into batched actions.

Original pull request #1230
Original pull request #1229
Original pull request #1228
Original pull request #1211

Author: schauder

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

@@ -50,6 +50,7 @@
  * @author Myeonghyeon Lee
  * @author Chirag Tailor
  */
+@SuppressWarnings("rawtypes")
 class JdbcAggregateChangeExecutionContext {
 
 	private static final String UPDATE_FAILED = "Failed to update entity [%s]. Id [%s] not found in database.";
@@ -192,16 +193,14 @@ private Object getParentId(DbAction.WithDependingOn<?> action) {
 	private DbAction.WithEntity<?> getIdOwningAction(DbAction.WithEntity<?> action,
 			PersistentPropertyPathExtension idPath) {
 
-		if (!(action instanceof DbAction.WithDependingOn)) {
+		if (!(action instanceof DbAction.WithDependingOn<?> withDependingOn)) {
 
 			Assert.state(idPath.getLength() == 0,
 					"When the id path is not empty the id providing action should be of type WithDependingOn");
 
 			return action;
 		}
 
-		DbAction.WithDependingOn<?> withDependingOn = (DbAction.WithDependingOn<?>) action;
-
 		if (idPath.matches(withDependingOn.getPropertyPath())) {
 			return action;
 		}
@@ -257,9 +256,8 @@ <T> List<T> populateIdsIfNecessary() {
 				roots.add((T) newEntity);
 			}
 
-			// the id property was immutable so we have to propagate changes up the tree
-			if (newEntity != action.getEntity() && action instanceof DbAction.Insert) {
-				DbAction.Insert<?> insert = (DbAction.Insert<?>) action;
+			// the id property was immutable, so we have to propagate changes up the tree
+			if (newEntity != action.getEntity() && action instanceof DbAction.Insert<?> insert) {
 
 				Pair<?, ?> qualifier = insert.getQualifier();
 
@@ -463,6 +461,7 @@ public List createEmptyInstance() {
 		public List add(@Nullable List list, @Nullable Object qualifier, Object value) {
 
 			Assert.notNull(list, "List must not be null.");
+			Assert.notNull(qualifier, "ListAggregator can't handle a null qualifier.");
 
 			int index = (int) qualifier;
 			if (index >= list.size()) {

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

@@ -21,7 +21,7 @@
 import org.springframework.lang.Nullable;
 
 /**
- * Specifies a operations one can perform on a database, based on an <em>Domain Type</em>.
+ * Specifies operations one can perform on a database, based on an <em>Domain Type</em>.
  *
  * @author Jens Schauder
  * @author Thomas Lang
@@ -90,7 +90,7 @@ public interface JdbcAggregateOperations {
 	<T> void deleteAllById(Iterable<?> ids, Class<T> domainType);
 
 	/**
-	 * Delete an aggregate identified by it's aggregate root.
+	 * Delete an aggregate identified by its aggregate root.
 	 *
 	 * @param aggregateRoot to delete. Must not be {@code null}.
 	 * @param domainType the type of the aggregate root. Must not be {@code null}.

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

@@ -287,6 +287,7 @@ public <T> void deleteAllById(Iterable<?> ids, Class<T> domainType) {
 				.forDelete(domainType);
 
 		ids.forEach(id -> {
+
 			DeleteAggregateChange<T> change = createDeletingChange(id, null, domainType);
 			triggerBeforeDelete(null, id, change);
 			batchingAggregateChange.add(change);
@@ -316,6 +317,7 @@ public <T> void deleteAll(Iterable<? extends T> instances, Class<T> domainType)
 		Map<Object, T> instancesBeforeExecute = new LinkedHashMap<>();
 
 		instances.forEach(instance -> {
+
 			Object id = context.getRequiredPersistentEntity(domainType).getIdentifierAccessor(instance)
 					.getRequiredIdentifier();
 			DeleteAggregateChange<T> change = createDeletingChange(id, instance, domainType);
@@ -384,6 +386,7 @@ private <T> List<T> performSaveAll(Iterable<T> instances) {
 
 		for (T instance : instances) {
 			if (batchingAggregateChange == null) {
+				// noinspection unchecked
 				batchingAggregateChange = BatchingAggregateChange.forSave((Class<T>) ClassUtils.getUserClass(instance));
 			}
 			batchingAggregateChange.add(beforeExecute(instance, changeCreatorSelectorForSave(instance)));
@@ -540,15 +543,6 @@ private <T> T triggerBeforeDelete(@Nullable T aggregateRoot, Object id, MutableA
 		return null;
 	}
 
-	private static class EntityAndPreviousVersion<T> {
-
-		private final T entity;
-		private final Number version;
-
-		EntityAndPreviousVersion(T entity, @Nullable Number version) {
-
-			this.entity = entity;
-			this.version = version;
-		}
+	private record EntityAndPreviousVersion<T> (T entity, @Nullable Number version) {
 	}
 }

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

@@ -27,6 +27,8 @@
 import org.springframework.data.relational.core.mapping.RelationalPersistentProperty;
 import org.springframework.data.relational.core.sql.LockMode;
 
+import static java.lang.Boolean.*;
+
 /**
  * Delegates each methods to the {@link DataAccessStrategy}s passed to the constructor in turn until the first that does
  * not throw an exception.
@@ -155,15 +157,14 @@ public <T> Iterable<T> findAll(Class<T> domainType, Pageable pageable) {
 
 	private <T> T collect(Function<DataAccessStrategy, T> function) {
 
-		// Keep <T> as Eclipse fails to compile if <> is used.
 		return strategies.stream().collect(new FunctionCollector<>(function));
 	}
 
 	private void collectVoid(Consumer<DataAccessStrategy> consumer) {
 
 		collect(das -> {
 			consumer.accept(das);
-			return null;
+			return TRUE;
 		});
 	}
 }

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

@@ -183,8 +183,7 @@ public <T> void deleteWithVersion(Object id, Class<T> domainType, Number previou
 	@Override
 	public void delete(Object rootId, PersistentPropertyPath<RelationalPersistentProperty> propertyPath) {
 
-		RelationalPersistentEntity<?> rootEntity = context
-				.getRequiredPersistentEntity(propertyPath.getBaseProperty().getOwner().getType());
+		RelationalPersistentEntity<?> rootEntity = context.getRequiredPersistentEntity(getBaseType(propertyPath));
 
 		RelationalPersistentProperty referencingProperty = propertyPath.getLeafProperty();
 		Assert.notNull(referencingProperty, "No property found matching the PropertyPath " + propertyPath);
@@ -199,8 +198,7 @@ public void delete(Object rootId, PersistentPropertyPath<RelationalPersistentPro
 	@Override
 	public void delete(Iterable<Object> rootIds, PersistentPropertyPath<RelationalPersistentProperty> propertyPath) {
 
-		RelationalPersistentEntity<?> rootEntity = context
-				.getRequiredPersistentEntity(propertyPath.getBaseProperty().getOwner().getType());
+		RelationalPersistentEntity<?> rootEntity = context.getRequiredPersistentEntity(getBaseType(propertyPath));
 
 		RelationalPersistentProperty referencingProperty = propertyPath.getLeafProperty();
 
@@ -220,8 +218,7 @@ public <T> void deleteAll(Class<T> domainType) {
 	@Override
 	public void deleteAll(PersistentPropertyPath<RelationalPersistentProperty> propertyPath) {
 
-		operations.getJdbcOperations()
-				.update(sql(propertyPath.getBaseProperty().getOwner().getType()).createDeleteAllSql(propertyPath));
+		operations.getJdbcOperations().update(sql(getBaseType(propertyPath)).createDeleteAllSql(propertyPath));
 	}
 
 	@Override
@@ -365,4 +362,13 @@ private <T> SqlIdentifier getIdColumn(Class<T> domainType) {
 		return Optional.ofNullable(context.getRequiredPersistentEntity(domainType).getIdProperty())
 				.map(RelationalPersistentProperty::getColumnName).orElse(null);
 	}
+
+	private Class<?> getBaseType(PersistentPropertyPath<RelationalPersistentProperty> propertyPath) {
+
+		RelationalPersistentProperty baseProperty = propertyPath.getBaseProperty();
+
+		Assert.notNull(baseProperty, "The base property must not be null");
+
+		return baseProperty.getOwner().getType();
+	}
 }

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

@@ -103,7 +103,7 @@ class SqlGenerator {
 	}
 
 	/**
-	 * Construct a IN-condition based on a {@link Select Sub-Select} which selects the ids (or stand ins for ids) of the
+	 * Construct an IN-condition based on a {@link Select Sub-Select} which selects the ids (or stand-ins for ids) of the
 	 * given {@literal path} to those that reference the root entities specified by the {@literal rootCondition}.
 	 *
 	 * @param path specifies the table and id to select
@@ -135,7 +135,7 @@ private Condition getSubselectCondition(PersistentPropertyPathExtension path,
 			innerCondition = rootCondition.apply(selectFilterColumn);
 		} else {
 
-			// otherwise we need another layer of subselect
+			// otherwise, we need another layer of subselect
 			innerCondition = getSubselectCondition(parentPath, rootCondition, selectFilterColumn);
 		}
 
@@ -502,7 +502,7 @@ private SelectBuilder.SelectOrdered applyPagination(Pageable pageable, SelectBui
 	@Nullable
 	Column getColumn(PersistentPropertyPathExtension path) {
 
-		// an embedded itself doesn't give an column, its members will though.
+		// an embedded itself doesn't give a column, its members will though.
 		// if there is a collection or map on the path it won't get selected at all, but it will get loaded with a separate
 		// select
 		// only the parent path is considered in order to handle arrays that get stored as BINARY properly
@@ -512,7 +512,7 @@ Column getColumn(PersistentPropertyPathExtension path) {
 
 		if (path.isEntity()) {
 
-			// Simple entities without id include there backreference as an synthetic id in order to distinguish null entities
+			// Simple entities without id include there backreference as a synthetic id in order to distinguish null entities
 			// from entities with only null values.
 
 			if (path.isQualified() //
@@ -622,7 +622,7 @@ private UpdateBuilder.UpdateWhereAndOr createBaseUpdate() {
 
 		Table table = getTable();
 
-		List<AssignValue> assignments = columns.getUpdateableColumns() //
+		List<AssignValue> assignments = columns.getUpdatableColumns() //
 				.stream() //
 				.map(columnName -> Assignments.value( //
 						table.column(columnName), //
@@ -807,7 +807,7 @@ static class Columns {
 		private final List<SqlIdentifier> nonIdColumnNames = new ArrayList<>();
 		private final Set<SqlIdentifier> readOnlyColumnNames = new HashSet<>();
 		private final Set<SqlIdentifier> insertableColumns;
-		private final Set<SqlIdentifier> updateableColumns;
+		private final Set<SqlIdentifier> updatableColumns;
 
 		Columns(RelationalPersistentEntity<?> entity,
 				MappingContext<RelationalPersistentEntity<?>, RelationalPersistentProperty> mappingContext,
@@ -823,12 +823,12 @@ static class Columns {
 
 			this.insertableColumns = Collections.unmodifiableSet(insertable);
 
-			Set<SqlIdentifier> updateable = new LinkedHashSet<>(columnNames);
+			Set<SqlIdentifier> updatable = new LinkedHashSet<>(columnNames);
 
-			updateable.removeAll(idColumnNames);
-			updateable.removeAll(readOnlyColumnNames);
+			updatable.removeAll(idColumnNames);
+			updatable.removeAll(readOnlyColumnNames);
 
-			this.updateableColumns = Collections.unmodifiableSet(updateable);
+			this.updatableColumns = Collections.unmodifiableSet(updatable);
 		}
 
 		private void populateColumnNameCache(RelationalPersistentEntity<?> entity, String prefix) {
@@ -881,8 +881,8 @@ Set<SqlIdentifier> getInsertableColumns() {
 		/**
 		 * @return Column names that can be used for {@code UPDATE}.
 		 */
-		Set<SqlIdentifier> getUpdateableColumns() {
-			return updateableColumns;
+		Set<SqlIdentifier> getUpdatableColumns() {
+			return updatableColumns;
 		}
 	}
 }

spring-data-jdbc/src/main/java/org/springframework/data/jdbc/mybatis/MyBatisDataAccessStrategy.java

@@ -21,10 +21,7 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.stream.Collectors;
 
-import org.apache.commons.logging.Log;
-import org.apache.commons.logging.LogFactory;
 import org.apache.ibatis.session.SqlSession;
 import org.mybatis.spring.SqlSessionTemplate;
 import org.springframework.dao.EmptyResultDataAccessException;
@@ -63,11 +60,9 @@
  */
 public class MyBatisDataAccessStrategy implements DataAccessStrategy {
 
-	private static final Log LOG = LogFactory.getLog(MyBatisDataAccessStrategy.class);
 	private static final String VERSION_SQL_PARAMETER_NAME_OLD = "___oldOptimisticLockingVersion";
 
 	private final SqlSession sqlSession;
-	private final IdentifierProcessing identifierProcessing;
 	private NamespaceStrategy namespaceStrategy = NamespaceStrategy.DEFAULT_INSTANCE;
 
 	/**
@@ -133,7 +128,6 @@ public static DataAccessStrategy createCombinedAccessStrategy(RelationalMappingC
 	public MyBatisDataAccessStrategy(SqlSession sqlSession, IdentifierProcessing identifierProcessing) {
 
 		this.sqlSession = sqlSession;
-		this.identifierProcessing = identifierProcessing;
 	}
 
 	/**
@@ -210,7 +204,7 @@ public <T> void deleteWithVersion(Object id, Class<T> domainType, Number previou
 	@Override
 	public void delete(Object rootId, PersistentPropertyPath<RelationalPersistentProperty> propertyPath) {
 
-		Class<?> ownerType = propertyPath.getBaseProperty().getOwner().getType();
+		Class<?> ownerType = getOwnerTyp(propertyPath);
 		String statement = namespace(ownerType) + ".delete-" + toDashPath(propertyPath);
 		Class<?> leafType = propertyPath.getRequiredLeafProperty().getTypeInformation().getType();
 		MyBatisContext parameter = new MyBatisContext(rootId, null, leafType, Collections.emptyMap());
@@ -234,10 +228,9 @@ public <T> void deleteAll(Class<T> domainType) {
 	@Override
 	public void deleteAll(PersistentPropertyPath<RelationalPersistentProperty> propertyPath) {
 
-		Class<?> baseType = propertyPath.getBaseProperty().getOwner().getType();
 		Class<?> leafType = propertyPath.getRequiredLeafProperty().getTypeInformation().getType();
 
-		String statement = namespace(baseType) + ".deleteAll-" + toDashPath(propertyPath);
+		String statement = namespace(getOwnerTyp(propertyPath)) + ".deleteAll-" + toDashPath(propertyPath);
 		MyBatisContext parameter = new MyBatisContext(null, null, leafType, Collections.emptyMap());
 		sqlSession().delete(statement, parameter);
 	}
@@ -291,8 +284,7 @@ public <T> Iterable<T> findAllById(Iterable<?> ids, Class<T> domainType) {
 	public Iterable<Object> findAllByPath(Identifier identifier,
 			PersistentPropertyPath<? extends RelationalPersistentProperty> path) {
 
-		String statementName = namespace(path.getBaseProperty().getOwner().getType()) + ".findAllByPath-"
-				+ path.toDotPath();
+		String statementName = namespace(getOwnerTyp(path)) + ".findAllByPath-" + path.toDotPath();
 
 		return sqlSession().selectList(statementName,
 				new MyBatisContext(identifier, null, path.getRequiredLeafProperty().getType()));
@@ -333,12 +325,6 @@ public long count(Class<?> domainType) {
 		return sqlSession().selectOne(statement, parameter);
 	}
 
-	private Map<String, Object> convertToParameterMap(Map<SqlIdentifier, Object> additionalParameters) {
-
-		return additionalParameters.entrySet().stream() //
-				.collect(Collectors.toMap(e -> e.getKey().toSql(identifierProcessing), Map.Entry::getValue));
-	}
-
 	private String namespace(Class<?> domainType) {
 		return this.namespaceStrategy.getNamespace(domainType);
 	}
@@ -348,6 +334,20 @@ private SqlSession sqlSession() {
 	}
 
 	private static String toDashPath(PersistentPropertyPath<RelationalPersistentProperty> propertyPath) {
-		return propertyPath.toDotPath().replaceAll("\\.", "-");
+
+		String dotPath = propertyPath.toDotPath();
+		if (dotPath == null) {
+			return "";
+		}
+		return dotPath.replaceAll("\\.", "-");
+	}
+
+	private Class<?> getOwnerTyp(PersistentPropertyPath<? extends RelationalPersistentProperty> propertyPath) {
+
+		RelationalPersistentProperty baseProperty = propertyPath.getBaseProperty();
+
+		Assert.notNull(baseProperty, "BaseProperty must not be null.");
+
+		return baseProperty.getOwner().getType();
 	}
 }

spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/AggregateChangeIdGenerationImmutableUnitTests.java

@@ -406,22 +406,19 @@ private static Map<String, Tag> createTagMap(Object... keysAndValues) {
 
 	DbAction.Insert<?> createInsert(String propertyName, Object value, @Nullable Object key) {
 
-		DbAction.Insert<Object> insert = new DbAction.Insert<>(value,
+		return new DbAction.Insert<>(value,
 				context.getPersistentPropertyPath(propertyName, DummyEntity.class), rootInsert,
 				singletonMap(toPath(propertyName), key), IdValueSource.GENERATED);
-
-		return insert;
 	}
 
 	DbAction.Insert<?> createDeepInsert(String propertyName, Object value, Object key,
 			@Nullable DbAction.Insert<?> parentInsert) {
 
 		PersistentPropertyPath<RelationalPersistentProperty> propertyPath = toPath(
 				parentInsert.getPropertyPath().toDotPath() + "." + propertyName);
-		DbAction.Insert<Object> insert = new DbAction.Insert<>(value, propertyPath, parentInsert,
-				singletonMap(propertyPath, key), IdValueSource.GENERATED);
 
-		return insert;
+		return new DbAction.Insert<>(value, propertyPath, parentInsert,
+				singletonMap(propertyPath, key), IdValueSource.GENERATED);
 	}
 
 	PersistentPropertyPath<RelationalPersistentProperty> toPath(String path) {