GH-2064 BeforeConvertCallback should be applied after the RootAggrega…

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

@@ -28,6 +28,7 @@
 import java.util.stream.Stream;
 import java.util.stream.StreamSupport;
 
+import org.jspecify.annotations.Nullable;
 import org.springframework.context.ApplicationContext;
 import org.springframework.context.ApplicationEventPublisher;
 import org.springframework.data.domain.Page;
@@ -50,11 +51,22 @@
 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.relational.core.mapping.event.*;
+import org.springframework.data.relational.core.mapping.event.AbstractRelationalEvent;
+import org.springframework.data.relational.core.mapping.event.AfterConvertCallback;
+import org.springframework.data.relational.core.mapping.event.AfterConvertEvent;
+import org.springframework.data.relational.core.mapping.event.AfterDeleteCallback;
+import org.springframework.data.relational.core.mapping.event.AfterDeleteEvent;
+import org.springframework.data.relational.core.mapping.event.AfterSaveCallback;
+import org.springframework.data.relational.core.mapping.event.AfterSaveEvent;
+import org.springframework.data.relational.core.mapping.event.BeforeConvertCallback;
+import org.springframework.data.relational.core.mapping.event.BeforeConvertEvent;
+import org.springframework.data.relational.core.mapping.event.BeforeDeleteCallback;
+import org.springframework.data.relational.core.mapping.event.BeforeDeleteEvent;
+import org.springframework.data.relational.core.mapping.event.BeforeSaveCallback;
+import org.springframework.data.relational.core.mapping.event.BeforeSaveEvent;
+import org.springframework.data.relational.core.mapping.event.Identifier;
 import org.springframework.data.relational.core.query.Query;
 import org.springframework.data.support.PageableExecutionUtils;
-import org.springframework.data.util.Streamable;
-import org.springframework.lang.Nullable;
 import org.springframework.util.Assert;
 import org.springframework.util.ClassUtils;
 
@@ -175,7 +187,7 @@ public <T> T save(T instance) {
 
 	@Override
 	public <T> List<T> saveAll(Iterable<T> instances) {
-		return doInBatch(instances, (first) -> (second -> changeCreatorSelectorForSave(first).apply(second)));
+		return saveInBatch(instances, instance -> changeCreatorSelectorForSave(instance));
 	}
 
 	/**
@@ -196,7 +208,7 @@ public <T> T insert(T instance) {
 
 	@Override
 	public <T> List<T> insertAll(Iterable<T> instances) {
-		return doInBatch(instances, (__) -> (entity -> createInsertChange(prepareVersionForInsert(entity))));
+		return doInBatch(instances, entity -> createInsertChange(prepareVersionForInsert(entity)));
 	}
 
 	/**
@@ -217,10 +229,28 @@ public <T> T update(T instance) {
 
 	@Override
 	public <T> List<T> updateAll(Iterable<T> instances) {
-		return doInBatch(instances, (__) -> (entity -> createUpdateChange(prepareVersionForUpdate(entity))));
+		return doInBatch(instances, entity -> createUpdateChange(prepareVersionForUpdate(entity)));
 	}
 
-	private <T> List<T> doInBatch(Iterable<T> instances,Function<T, Function<T, RootAggregateChange<T>>> changeCreatorFunction) {
+	private <T> List<T> saveInBatch(Iterable<T> instances, Function<T, AggregateChangeCreator<T>> changes) {
+
+		Assert.notNull(instances, "Aggregate instances must not be null");
+
+		if (!instances.iterator().hasNext()) {
+			return Collections.emptyList();
+		}
+
+		List<EntityAndChangeCreator<T>> entityAndChangeCreators = new ArrayList<>();
+
+		for (T instance : instances) {
+			verifyIdProperty(instance);
+			entityAndChangeCreators.add(new EntityAndChangeCreator<>(instance, changes.apply(instance)));
+		}
+
+		return performSaveAll(entityAndChangeCreators);
+	}
+
+	private <T> List<T> doInBatch(Iterable<T> instances, AggregateChangeCreator<T> changeCreatorFunction) {
 
 		Assert.notNull(instances, "Aggregate instances must not be null");
 
@@ -231,7 +261,7 @@ private <T> List<T> doInBatch(Iterable<T> instances,Function<T, Function<T, Root
 		List<EntityAndChangeCreator<T>> entityAndChangeCreators = new ArrayList<>();
 		for (T instance : instances) {
 			verifyIdProperty(instance);
-			entityAndChangeCreators.add(new EntityAndChangeCreator<T>(instance, changeCreatorFunction.apply(instance)));
+			entityAndChangeCreators.add(new EntityAndChangeCreator<T>(instance, changeCreatorFunction));
 		}
 		return performSaveAll(entityAndChangeCreators);
 	}
@@ -484,7 +514,7 @@ private <T> RootAggregateChange<T> beforeExecute(EntityAndChangeCreator<T> insta
 
 		T aggregateRoot = triggerBeforeConvert(instance.entity);
 
-		RootAggregateChange<T> change = instance.changeCreator.apply(aggregateRoot);
+		RootAggregateChange<T> change = instance.changeCreator.createAggregateChange(aggregateRoot);
 
 		aggregateRoot = triggerBeforeSave(change.getRoot(), change);
 
@@ -542,7 +572,7 @@ private <T> List<T> performSaveAll(Iterable<EntityAndChangeCreator<T>> instances
 		return results;
 	}
 
-	private <T> Function<T, RootAggregateChange<T>> changeCreatorSelectorForSave(T instance) {
+	private <T> AggregateChangeCreator<T> changeCreatorSelectorForSave(T instance) {
 
 		return context.getRequiredPersistentEntity(instance.getClass()).isNew(instance)
 				? entity -> createInsertChange(prepareVersionForInsert(entity))
@@ -681,6 +711,13 @@ private <T> T triggerBeforeDelete(@Nullable T aggregateRoot, Object id, MutableA
 	private record EntityAndPreviousVersion<T> (T entity, @Nullable Number version) {
 	}
 
-	private record EntityAndChangeCreator<T> (T entity, Function<T, RootAggregateChange<T>> changeCreator) {
+	private record EntityAndChangeCreator<T> (T entity, AggregateChangeCreator<T> changeCreator) {
+	}
+
+	private interface AggregateChangeCreator<T> extends Function<T, RootAggregateChange<T>> {
+
+		default RootAggregateChange<T> createAggregateChange(T instance) {
+			return this.apply(instance);
+		}
 	}
 }

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

@@ -29,8 +29,10 @@
 import java.util.stream.IntStream;
 import java.util.stream.Stream;
 
+import org.assertj.core.api.Assertions;
 import org.junit.jupiter.api.Test;
 import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.context.ApplicationContext;
 import org.springframework.context.ApplicationEventPublisher;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
@@ -52,13 +54,15 @@
 import org.springframework.data.jdbc.testing.TestClass;
 import org.springframework.data.jdbc.testing.TestConfiguration;
 import org.springframework.data.jdbc.testing.TestDatabaseFeatures;
+import org.springframework.data.mapping.callback.EntityCallbacks;
 import org.springframework.data.mapping.context.InvalidPersistentPropertyPath;
 import org.springframework.data.relational.core.mapping.Column;
 import org.springframework.data.relational.core.mapping.Embedded;
 import org.springframework.data.relational.core.mapping.InsertOnlyProperty;
 import org.springframework.data.relational.core.mapping.MappedCollection;
 import org.springframework.data.relational.core.mapping.RelationalMappingContext;
 import org.springframework.data.relational.core.mapping.Table;
+import org.springframework.data.relational.core.mapping.event.BeforeConvertCallback;
 import org.springframework.data.relational.core.query.Criteria;
 import org.springframework.data.relational.core.query.CriteriaDefinition;
 import org.springframework.data.relational.core.query.Query;
@@ -1371,6 +1375,22 @@ void mapWithEnumKey() {
 		assertThat(enumMapOwners).containsExactly(enumMapOwner);
 	}
 
+	@Test //GH-2064
+	void saveAllBeforeConvertCallback() {
+		var first = new BeforeConvertCallbackForSaveBatch("first");
+		var second = new BeforeConvertCallbackForSaveBatch("second");
+		var third = new BeforeConvertCallbackForSaveBatch("third");
+
+		template.saveAll(List.of(first, second, third));
+
+		var allEntriesInTable = template.findAll(BeforeConvertCallbackForSaveBatch.class);
+
+		Assertions.assertThat(allEntriesInTable)
+				.hasSize(3)
+				.extracting(BeforeConvertCallbackForSaveBatch::getName)
+				.containsOnly("first", "second", "third");
+	}
+
 	@Test // GH-1684
 	void oneToOneWithIdenticalIdColumnName() {
 
@@ -2182,6 +2202,32 @@ public Short getVersion() {
 		}
 	}
 
+	@Table("BEFORE_CONVERT_CALLBACK_FOR_SAVE_BATCH")
+	static class BeforeConvertCallbackForSaveBatch {
+
+		@Id
+		private String id;
+
+		private String name;
+
+		public BeforeConvertCallbackForSaveBatch(String name) {
+			this.name = name;
+		}
+
+		public String getId() {
+			return id;
+		}
+
+		public BeforeConvertCallbackForSaveBatch setId(String id) {
+			this.id = id;
+			return this;
+		}
+
+		public String getName() {
+			return name;
+		}
+	}
+
 	@Table("VERSIONED_AGGREGATE")
 	static class AggregateWithPrimitiveShortVersion extends VersionedAggregate {
 
@@ -2269,9 +2315,17 @@ TestClass testClass() {
 		}
 
 		@Bean
-		JdbcAggregateOperations operations(ApplicationEventPublisher publisher, RelationalMappingContext context,
+		BeforeConvertCallback<BeforeConvertCallbackForSaveBatch> callback() {
+			return aggregate -> {
+				aggregate.setId(UUID.randomUUID().toString());
+				return aggregate;
+			};
+		}
+
+		@Bean
+		JdbcAggregateOperations operations(ApplicationContext applicationContext, RelationalMappingContext context,
 				DataAccessStrategy dataAccessStrategy, JdbcConverter converter) {
-			return new JdbcAggregateTemplate(publisher, context, converter, dataAccessStrategy);
+			return new JdbcAggregateTemplate(applicationContext, context, converter, dataAccessStrategy);
 		}
 	}
 

spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-db2.sql

@@ -59,6 +59,8 @@ DROP TABLE THIRD;
 DROP TABLE SEC;
 DROP TABLE FIRST;
 
+DROP TABLE BEFORE_CONVERT_CALLBACK_FOR_SAVE_BATCH;
+
 CREATE TABLE LEGO_SET
 (
   "id1"  BIGINT GENERATED BY DEFAULT AS IDENTITY (START WITH 1) PRIMARY KEY,
@@ -467,4 +469,10 @@ CREATE TABLE THIRD
     SEC BIGINT      NOT NULL,
     NAME   VARCHAR(20) NOT NULL,
     FOREIGN KEY (SEC) REFERENCES SEC (ID)
-);
+);
+
+CREATE TABLE BEFORE_CONVERT_CALLBACK_FOR_SAVE_BATCH
+(
+    ID VARCHAR(36) PRIMARY KEY NOT NULL,
+    NAME VARCHAR(20)
+);

spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-h2.sql

@@ -417,4 +417,10 @@ CREATE TABLE THIRD
     SEC BIGINT      NOT NULL,
     NAME   VARCHAR(20) NOT NULL,
     FOREIGN KEY (SEC) REFERENCES SEC (ID)
-);
+);
+
+CREATE TABLE BEFORE_CONVERT_CALLBACK_FOR_SAVE_BATCH
+(
+    ID VARCHAR PRIMARY KEY,
+    NAME VARCHAR
+);

spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-hsql.sql

@@ -419,4 +419,10 @@ CREATE TABLE THIRD
     SEC BIGINT      NOT NULL,
     NAME   VARCHAR(20) NOT NULL,
     FOREIGN KEY (SEC) REFERENCES SEC (ID)
-);
+);
+
+CREATE TABLE BEFORE_CONVERT_CALLBACK_FOR_SAVE_BATCH
+(
+    ID VARCHAR PRIMARY KEY,
+    NAME VARCHAR
+);

spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-mariadb.sql

@@ -391,4 +391,10 @@ CREATE TABLE THIRD
     SEC BIGINT      NOT NULL,
     NAME   VARCHAR(20) NOT NULL,
     FOREIGN KEY (SEC) REFERENCES SEC (ID)
-);
+);
+
+CREATE TABLE BEFORE_CONVERT_CALLBACK_FOR_SAVE_BATCH
+(
+    ID VARCHAR(36) PRIMARY KEY,
+    NAME VARCHAR(20)
+);

spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-mssql.sql

@@ -441,4 +441,12 @@ CREATE TABLE THIRD
     SEC BIGINT      NOT NULL,
     NAME   VARCHAR(20) NOT NULL,
     FOREIGN KEY (SEC) REFERENCES SEC (ID)
-);
+);
+
+DROP TABLE BEFORE_CONVERT_CALLBACK_FOR_SAVE_BATCH;
+
+CREATE TABLE BEFORE_CONVERT_CALLBACK_FOR_SAVE_BATCH
+(
+    ID VARCHAR PRIMARY KEY,
+    NAME VARCHAR
+);

spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-mysql.sql

@@ -397,4 +397,10 @@ CREATE TABLE THIRD
     SEC BIGINT      NOT NULL,
     NAME   VARCHAR(20) NOT NULL,
     FOREIGN KEY (SEC) REFERENCES SEC (ID)
-);
+);
+
+CREATE TABLE BEFORE_CONVERT_CALLBACK_FOR_SAVE_BATCH
+(
+    ID VARCHAR(36) PRIMARY KEY,
+    NAME VARCHAR(20)
+);

spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-oracle.sql

@@ -49,6 +49,8 @@ DROP TABLE THIRD CASCADE CONSTRAINTS PURGE;
 DROP TABLE SEC CASCADE CONSTRAINTS PURGE;
 DROP TABLE FIRST CASCADE CONSTRAINTS PURGE;
 
+DROP TABLE BEFORE_CONVERT_CALLBACK_FOR_SAVE_BATCH CASCADE CONSTRAINTS PURGE;
+
 CREATE TABLE LEGO_SET
 (
     "id1" NUMBER GENERATED by default on null as IDENTITY PRIMARY KEY,
@@ -447,4 +449,10 @@ CREATE TABLE THIRD
     SEC NUMBER      NOT NULL,
     NAME   VARCHAR(20) NOT NULL,
     FOREIGN KEY (SEC) REFERENCES SEC (ID)
-);
+);
+
+CREATE TABLE BEFORE_CONVERT_CALLBACK_FOR_SAVE_BATCH
+(
+    ID VARCHAR PRIMARY KEY,
+    NAME VARCHAR
+);

spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-postgres.sql

@@ -52,6 +52,8 @@ DROP TABLE THIRD;
 DROP TABLE SEC;
 DROP TABLE FIRST;
 
+DROP TABLE "BEFORE_CONVERT_CALLBACK_FOR_SAVE_BATCH";
+
 CREATE TABLE LEGO_SET
 (
     "id1" SERIAL PRIMARY KEY,
@@ -470,4 +472,10 @@ CREATE TABLE THIRD
     SEC BIGINT      NOT NULL,
     NAME   VARCHAR(20) NOT NULL,
     FOREIGN KEY (SEC) REFERENCES SEC (ID)
-);
+);
+
+CREATE TABLE "BEFORE_CONVERT_CALLBACK_FOR_SAVE_BATCH"
+(
+    ID VARCHAR PRIMARY KEY,
+    NAME VARCHAR
+);