From 7c20658b143f12419bbbecc9bfb4a3fd59985917 Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Tue, 22 Mar 2022 16:51:19 +0100 Subject: [PATCH] Polishing. This change extracts entity modifying behaviour into separate methods, so it doesn't appear as an unexpected side effect of the creation of aggregate changes. Also some formatting. Original pull request #1196 See #1137 --- .../data/jdbc/core/JdbcAggregateTemplate.java | 53 +++++++++++++------ ...JdbcAggregateTemplateIntegrationTests.java | 13 +++-- .../core/JdbcAggregateTemplateUnitTests.java | 29 ++++++---- 3 files changed, 64 insertions(+), 31 deletions(-) diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateTemplate.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateTemplate.java index f42849fbb..5f7f8a2fb 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateTemplate.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateTemplate.java @@ -150,8 +150,9 @@ public class JdbcAggregateTemplate implements JdbcAggregateOperations { RelationalPersistentEntity persistentEntity = context.getRequiredPersistentEntity(instance.getClass()); - Function> changeCreator = persistentEntity.isNew(instance) ? this::createInsertChange - : this::createUpdateChange; + Function> changeCreator = persistentEntity.isNew(instance) + ? entity -> createInsertChange(prepareVersionForInsert(entity)) + : entity -> createUpdateChange(prepareVersionForUpdate(entity)); return store(instance, changeCreator, persistentEntity); } @@ -170,7 +171,7 @@ public class JdbcAggregateTemplate implements JdbcAggregateOperations { RelationalPersistentEntity persistentEntity = context.getRequiredPersistentEntity(instance.getClass()); - return store(instance, this::createInsertChange, persistentEntity); + return store(instance, entity -> createInsertChange(prepareVersionForInsert(entity)), persistentEntity); } /** @@ -187,7 +188,7 @@ public class JdbcAggregateTemplate implements JdbcAggregateOperations { RelationalPersistentEntity persistentEntity = context.getRequiredPersistentEntity(instance.getClass()); - return store(instance, this::createUpdateChange, persistentEntity); + return store(instance, entity -> createUpdateChange(prepareVersionForUpdate(entity)), persistentEntity); } /* @@ -365,6 +366,21 @@ public class JdbcAggregateTemplate implements JdbcAggregateOperations { private MutableAggregateChange createInsertChange(T instance) { + MutableAggregateChange aggregateChange = MutableAggregateChange.forSave(instance); + jdbcEntityInsertWriter.write(instance, aggregateChange); + return aggregateChange; + } + + private MutableAggregateChange createUpdateChange(EntityAndPreviousVersion entityAndVersion) { + + MutableAggregateChange aggregateChange = MutableAggregateChange.forSave(entityAndVersion.entity, + entityAndVersion.version); + jdbcEntityUpdateWriter.write(entityAndVersion.entity, aggregateChange); + return aggregateChange; + } + + private T prepareVersionForInsert(T instance) { + RelationalPersistentEntity persistentEntity = getRequiredPersistentEntity(instance); T preparedInstance = instance; if (persistentEntity.hasVersionProperty()) { @@ -375,31 +391,26 @@ public class JdbcAggregateTemplate implements JdbcAggregateOperations { preparedInstance = RelationalEntityVersionUtils.setVersionNumberOnEntity( // instance, initialVersion, persistentEntity, converter); } - MutableAggregateChange aggregateChange = MutableAggregateChange.forSave(preparedInstance); - jdbcEntityInsertWriter.write(preparedInstance, aggregateChange); - return aggregateChange; + return preparedInstance; } - private MutableAggregateChange createUpdateChange(T instance) { + private EntityAndPreviousVersion prepareVersionForUpdate(T instance) { RelationalPersistentEntity persistentEntity = getRequiredPersistentEntity(instance); T preparedInstance = instance; Number previousVersion = null; if (persistentEntity.hasVersionProperty()) { // If the root aggregate has a version property, increment it. - previousVersion = RelationalEntityVersionUtils.getVersionNumberFromEntity(instance, - persistentEntity, converter); + previousVersion = RelationalEntityVersionUtils.getVersionNumberFromEntity(instance, persistentEntity, converter); Assert.notNull(previousVersion, "The root aggregate cannot be updated because the version property is null."); long newVersion = previousVersion.longValue() + 1; - preparedInstance = RelationalEntityVersionUtils.setVersionNumberOnEntity(instance, newVersion, - persistentEntity, converter); + preparedInstance = RelationalEntityVersionUtils.setVersionNumberOnEntity(instance, newVersion, persistentEntity, + converter); } - MutableAggregateChange aggregateChange = MutableAggregateChange.forSave(preparedInstance, previousVersion); - jdbcEntityUpdateWriter.write(preparedInstance, aggregateChange); - return aggregateChange; + return new EntityAndPreviousVersion<>(preparedInstance, previousVersion); } @SuppressWarnings("unchecked") @@ -489,4 +500,16 @@ public class JdbcAggregateTemplate implements JdbcAggregateOperations { return null; } + + private static class EntityAndPreviousVersion { + + private final T entity; + private final Number version; + + EntityAndPreviousVersion(T entity, @Nullable Number version) { + + this.entity = entity; + this.version = version; + } + } } diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateTemplateIntegrationTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateTemplateIntegrationTests.java index 07450a7eb..206820bfe 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateTemplateIntegrationTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateTemplateIntegrationTests.java @@ -821,9 +821,10 @@ class JdbcAggregateTemplateIntegrationTests { @Test // GH-1137 void testUpdateEntityWithVersionDoesNotTriggerAnewConstructorInvocation() { + AggregateWithImmutableVersion aggregateWithImmutableVersion = new AggregateWithImmutableVersion(null, null); - final AggregateWithImmutableVersion savedRoot = template.save(aggregateWithImmutableVersion); + AggregateWithImmutableVersion savedRoot = template.save(aggregateWithImmutableVersion); assertThat(savedRoot).isNotNull(); assertThat(savedRoot.version).isEqualTo(0L); @@ -836,12 +837,12 @@ class JdbcAggregateTemplateIntegrationTests { AggregateWithImmutableVersion.clearConstructorInvocationData(); - final AggregateWithImmutableVersion updatedRoot = template.save(savedRoot); + AggregateWithImmutableVersion updatedRoot = template.save(savedRoot); assertThat(updatedRoot).isNotNull(); assertThat(updatedRoot.version).isEqualTo(1L); - // Expect only one assignnment of the version to AggregateWithImmutableVersion + // Expect only one assignment of the version to AggregateWithImmutableVersion assertThat(AggregateWithImmutableVersion.constructorInvocations).containsOnly(new ConstructorInvocation(savedRoot.id, updatedRoot.version)); } @@ -1261,6 +1262,7 @@ class JdbcAggregateTemplateIntegrationTests { } public AggregateWithImmutableVersion(Long id, Long version) { + constructorInvocations.add(new ConstructorInvocation(id, version)); this.id = id; this.version = version; @@ -1270,8 +1272,9 @@ class JdbcAggregateTemplateIntegrationTests { @Value @EqualsAndHashCode private static class ConstructorInvocation { - private Long id; - private Long version; + + Long id; + Long version; } @Data diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateTemplateUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateTemplateUnitTests.java index d9e837b64..e12bd84a6 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateTemplateUnitTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateTemplateUnitTests.java @@ -22,8 +22,8 @@ import static org.mockito.Mockito.*; import lombok.AllArgsConstructor; import lombok.Data; - import lombok.RequiredArgsConstructor; + import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -115,7 +115,7 @@ public class JdbcAggregateTemplateUnitTests { assertThat(last).isEqualTo(third); } - @Test + @Test // GH-1137 void savePreparesInstanceWithInitialVersion_onInsert() { EntityWithVersion entity = new EntityWithVersion(1L); @@ -125,11 +125,12 @@ public class JdbcAggregateTemplateUnitTests { ArgumentCaptor aggregateRootCaptor = ArgumentCaptor.forClass(Object.class); verify(callbacks).callback(eq(BeforeSaveCallback.class), aggregateRootCaptor.capture(), any()); + EntityWithVersion afterConvert = (EntityWithVersion) aggregateRootCaptor.getValue(); assertThat(afterConvert.getVersion()).isEqualTo(0L); } - @Test + @Test // GH-1137 void savePreparesInstanceWithInitialVersion_onInsert_whenVersionPropertyIsImmutable() { EntityWithImmutableVersion entity = new EntityWithImmutableVersion(1L, null); @@ -139,11 +140,12 @@ public class JdbcAggregateTemplateUnitTests { ArgumentCaptor aggregateRootCaptor = ArgumentCaptor.forClass(Object.class); verify(callbacks).callback(eq(BeforeSaveCallback.class), aggregateRootCaptor.capture(), any()); + EntityWithImmutableVersion afterConvert = (EntityWithImmutableVersion) aggregateRootCaptor.getValue(); assertThat(afterConvert.getVersion()).isEqualTo(0L); } - @Test // DATAJDBC-507 + @Test // GH-1137 void savePreparesInstanceWithInitialVersion_onInsert_whenVersionPropertyIsPrimitiveType() { EntityWithPrimitiveVersion entity = new EntityWithPrimitiveVersion(1L); @@ -153,11 +155,12 @@ public class JdbcAggregateTemplateUnitTests { ArgumentCaptor aggregateRootCaptor = ArgumentCaptor.forClass(Object.class); verify(callbacks).callback(eq(BeforeSaveCallback.class), aggregateRootCaptor.capture(), any()); + EntityWithPrimitiveVersion afterConvert = (EntityWithPrimitiveVersion) aggregateRootCaptor.getValue(); assertThat(afterConvert.getVersion()).isEqualTo(1L); } - @Test // DATAJDBC-507 + @Test // GH-1137 void savePreparesInstanceWithInitialVersion_onInsert__whenVersionPropertyIsImmutableAndPrimitiveType() { EntityWithImmutablePrimitiveVersion entity = new EntityWithImmutablePrimitiveVersion(1L, 0L); @@ -167,11 +170,13 @@ public class JdbcAggregateTemplateUnitTests { ArgumentCaptor aggregateRootCaptor = ArgumentCaptor.forClass(Object.class); verify(callbacks).callback(eq(BeforeSaveCallback.class), aggregateRootCaptor.capture(), any()); - EntityWithImmutablePrimitiveVersion afterConvert = (EntityWithImmutablePrimitiveVersion) aggregateRootCaptor.getValue(); + + EntityWithImmutablePrimitiveVersion afterConvert = (EntityWithImmutablePrimitiveVersion) aggregateRootCaptor + .getValue(); assertThat(afterConvert.getVersion()).isEqualTo(1L); } - @Test + @Test // GH-1137 void savePreparesChangeWithPreviousVersion_onUpdate() { when(dataAccessStrategy.updateWithVersion(any(), any(), any())).thenReturn(true); @@ -183,11 +188,12 @@ public class JdbcAggregateTemplateUnitTests { ArgumentCaptor aggregateChangeCaptor = ArgumentCaptor.forClass(Object.class); verify(callbacks).callback(eq(BeforeSaveCallback.class), any(), aggregateChangeCaptor.capture()); + MutableAggregateChange aggregateChange = (MutableAggregateChange) aggregateChangeCaptor.getValue(); assertThat(aggregateChange.getPreviousVersion()).isEqualTo(1L); } - @Test + @Test // GH-1137 void savePreparesInstanceWithNextVersion_onUpdate() { when(dataAccessStrategy.updateWithVersion(any(), any(), any())).thenReturn(true); @@ -199,11 +205,12 @@ public class JdbcAggregateTemplateUnitTests { ArgumentCaptor aggregateRootCaptor = ArgumentCaptor.forClass(Object.class); verify(callbacks).callback(eq(BeforeSaveCallback.class), aggregateRootCaptor.capture(), any()); + EntityWithVersion afterConvert = (EntityWithVersion) aggregateRootCaptor.getValue(); assertThat(afterConvert.getVersion()).isEqualTo(2L); } - @Test + @Test // GH-1137 void savePreparesInstanceWithNextVersion_onUpdate_whenVersionPropertyIsImmutable() { when(dataAccessStrategy.updateWithVersion(any(), any(), any())).thenReturn(true); @@ -218,7 +225,7 @@ public class JdbcAggregateTemplateUnitTests { assertThat(afterConvert.getVersion()).isEqualTo(2L); } - @Test + @Test // GH-1137 void deletePreparesChangeWithPreviousVersion_onDeleteByInstance() { EntityWithImmutableVersion entity = new EntityWithImmutableVersion(1L, 1L); @@ -228,11 +235,11 @@ public class JdbcAggregateTemplateUnitTests { ArgumentCaptor aggregateChangeCaptor = ArgumentCaptor.forClass(Object.class); verify(callbacks).callback(eq(BeforeDeleteCallback.class), any(), aggregateChangeCaptor.capture()); + MutableAggregateChange aggregateChange = (MutableAggregateChange) aggregateChangeCaptor.getValue(); assertThat(aggregateChange.getPreviousVersion()).isEqualTo(1L); } - @Test // DATAJDBC-393 public void callbackOnDelete() {