From ae21bc14a6a499ab22084de291351e38b6d1dcad Mon Sep 17 00:00:00 2001 From: Chirag Tailor Date: Thu, 17 Mar 2022 13:56:56 -0500 Subject: [PATCH 1/5] 1137-dup-constructor-calls-for-version - Prepare branch --- pom.xml | 2 +- spring-data-jdbc-distribution/pom.xml | 2 +- spring-data-jdbc/pom.xml | 4 ++-- spring-data-relational/pom.xml | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/pom.xml b/pom.xml index 2c64717780..7aef4e3e19 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-relational-parent - 2.4.0-SNAPSHOT + 2.4.0-1137-dup-constructor-calls-for-version-SNAPSHOT pom Spring Data Relational Parent diff --git a/spring-data-jdbc-distribution/pom.xml b/spring-data-jdbc-distribution/pom.xml index 0646c2846d..93cc6a7341 100644 --- a/spring-data-jdbc-distribution/pom.xml +++ b/spring-data-jdbc-distribution/pom.xml @@ -14,7 +14,7 @@ org.springframework.data spring-data-relational-parent - 2.4.0-SNAPSHOT + 2.4.0-1137-dup-constructor-calls-for-version-SNAPSHOT ../pom.xml diff --git a/spring-data-jdbc/pom.xml b/spring-data-jdbc/pom.xml index 11114a795e..f7319d71ea 100644 --- a/spring-data-jdbc/pom.xml +++ b/spring-data-jdbc/pom.xml @@ -6,7 +6,7 @@ 4.0.0 spring-data-jdbc - 2.4.0-SNAPSHOT + 2.4.0-1137-dup-constructor-calls-for-version-SNAPSHOT Spring Data JDBC Spring Data module for JDBC repositories. @@ -15,7 +15,7 @@ org.springframework.data spring-data-relational-parent - 2.4.0-SNAPSHOT + 2.4.0-1137-dup-constructor-calls-for-version-SNAPSHOT diff --git a/spring-data-relational/pom.xml b/spring-data-relational/pom.xml index a6eb48c891..6e1a3bb528 100644 --- a/spring-data-relational/pom.xml +++ b/spring-data-relational/pom.xml @@ -6,7 +6,7 @@ 4.0.0 spring-data-relational - 2.4.0-SNAPSHOT + 2.4.0-1137-dup-constructor-calls-for-version-SNAPSHOT Spring Data Relational Spring Data Relational support @@ -14,7 +14,7 @@ org.springframework.data spring-data-relational-parent - 2.4.0-SNAPSHOT + 2.4.0-1137-dup-constructor-calls-for-version-SNAPSHOT From 9260dc927534f2e7e53faeead8089d5f04f6f0ef Mon Sep 17 00:00:00 2001 From: Chirag Tailor Date: Fri, 11 Mar 2022 10:21:29 -0600 Subject: [PATCH 2/5] Small refactors for simplification --- .../data/jdbc/core/AggregateChangeExecutor.java | 4 +++- .../core/JdbcAggregateChangeExecutionContext.java | 12 +++--------- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/AggregateChangeExecutor.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/AggregateChangeExecutor.java index 85b03407ea..4731cb8572 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/AggregateChangeExecutor.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/AggregateChangeExecutor.java @@ -51,7 +51,9 @@ T execute(AggregateChange aggregateChange) { aggregateChange.forEachAction(action -> execute(action, executionContext)); T root = executionContext.populateIdsIfNecessary(); - root = root == null ? aggregateChange.getEntity() : root; + if (root == null) { + root = aggregateChange.getEntity(); + } if (root != null) { root = executionContext.populateRootVersionIfNecessary(root); diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateChangeExecutionContext.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateChangeExecutionContext.java index 778083db11..ae9224930c 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateChangeExecutionContext.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateChangeExecutionContext.java @@ -147,16 +147,10 @@ void executeUpdate(DbAction.Update update) { void executeDeleteRoot(DbAction.DeleteRoot delete) { if (delete.getPreviousVersion() != null) { - - RelationalPersistentEntity persistentEntity = getRequiredPersistentEntity(delete.getEntityType()); - if (persistentEntity.hasVersionProperty()) { - - accessStrategy.deleteWithVersion(delete.getId(), delete.getEntityType(), delete.getPreviousVersion()); - return; - } + accessStrategy.deleteWithVersion(delete.getId(), delete.getEntityType(), delete.getPreviousVersion()); + } else { + accessStrategy.delete(delete.getId(), delete.getEntityType()); } - - accessStrategy.delete(delete.getId(), delete.getEntityType()); } void executeDelete(DbAction.Delete delete) { From 5ea44080a2a39170fc5a09e41a343687737fe855 Mon Sep 17 00:00:00 2001 From: Chirag Tailor Date: Fri, 11 Mar 2022 14:41:44 -0600 Subject: [PATCH 3/5] Determine and set the value for entity @Version before conversion to DbActions to simplify execution context. --- .../jdbc/core/AggregateChangeExecutor.java | 4 - .../JdbcAggregateChangeExecutionContext.java | 77 +-------- .../data/jdbc/core/JdbcAggregateTemplate.java | 54 +++++- ...angeExecutorContextImmutableUnitTests.java | 6 +- ...gregateChangeExecutorContextUnitTests.java | 25 +-- .../core/JdbcAggregateTemplateUnitTests.java | 157 ++++++++++++++++++ .../relational/core/conversion/DbAction.java | 10 +- .../conversion/DefaultAggregateChange.java | 12 +- .../conversion/MutableAggregateChange.java | 36 +++- .../RelationalEntityDeleteWriter.java | 22 +-- .../core/conversion/WritingContext.java | 6 +- .../RelationalEntityWriterUnitTests.java | 57 +++---- 12 files changed, 302 insertions(+), 164 deletions(-) diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/AggregateChangeExecutor.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/AggregateChangeExecutor.java index 4731cb8572..7b016e4179 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/AggregateChangeExecutor.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/AggregateChangeExecutor.java @@ -55,10 +55,6 @@ T execute(AggregateChange aggregateChange) { root = aggregateChange.getEntity(); } - if (root != null) { - root = executionContext.populateRootVersionIfNecessary(root); - } - return root; } diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateChangeExecutionContext.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateChangeExecutionContext.java index ae9224930c..7b9368d37e 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateChangeExecutionContext.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateChangeExecutionContext.java @@ -40,7 +40,6 @@ import org.springframework.data.mapping.context.MappingContext; import org.springframework.data.relational.core.conversion.DbAction; import org.springframework.data.relational.core.conversion.DbActionExecutionResult; -import org.springframework.data.relational.core.conversion.RelationalEntityVersionUtils; import org.springframework.data.relational.core.mapping.PersistentPropertyPathExtension; import org.springframework.data.relational.core.mapping.RelationalPersistentEntity; import org.springframework.data.relational.core.mapping.RelationalPersistentProperty; @@ -65,7 +64,6 @@ class JdbcAggregateChangeExecutionContext { private final DataAccessStrategy accessStrategy; private final Map, DbActionExecutionResult> results = new LinkedHashMap<>(); - @Nullable private Long version; JdbcAggregateChangeExecutionContext(JdbcConverter converter, DataAccessStrategy accessStrategy) { @@ -76,28 +74,8 @@ class JdbcAggregateChangeExecutionContext { void executeInsertRoot(DbAction.InsertRoot insert) { - RelationalPersistentEntity persistentEntity = getRequiredPersistentEntity(insert.getEntityType()); - - Object id; - if (persistentEntity.hasVersionProperty()) { - - RelationalPersistentProperty versionProperty = persistentEntity.getVersionProperty(); - - Assert.state(versionProperty != null, "Version property must not be null at this stage."); - - long initialVersion = versionProperty.getActualType().isPrimitive() ? 1L : 0; - - T rootEntity = RelationalEntityVersionUtils.setVersionNumberOnEntity( // - insert.getEntity(), initialVersion, persistentEntity, converter); - - id = accessStrategy.insert(rootEntity, insert.getEntityType(), Identifier.empty(), insert.getIdValueSource()); - - setNewVersion(initialVersion); - } else { - id = accessStrategy.insert(insert.getEntity(), insert.getEntityType(), Identifier.empty(), - insert.getIdValueSource()); - } - + Object id = accessStrategy.insert(insert.getEntity(), insert.getEntityType(), Identifier.empty(), + insert.getIdValueSource()); add(new DbActionExecutionResult(insert, id)); } @@ -125,12 +103,9 @@ void executeInsertBatch(DbAction.InsertBatch insertBatch) { void executeUpdateRoot(DbAction.UpdateRoot update) { - RelationalPersistentEntity persistentEntity = getRequiredPersistentEntity(update.getEntityType()); - - if (persistentEntity.hasVersionProperty()) { - updateWithVersion(update, persistentEntity); + if (update.getPreviousVersion() != null) { + updateWithVersion(update); } else { - updateWithoutVersion(update); } } @@ -252,36 +227,6 @@ private Object getIdFrom(DbAction.WithEntity idOwningAction) { return identifier; } - private void setNewVersion(long version) { - - Assert.isNull(this.version, "A new version was set a second time."); - - this.version = version; - } - - private long getNewVersion() { - - Assert.notNull(version, "A new version was requested, but none was set."); - - return version; - } - - private boolean hasNewVersion() { - return version != null; - } - - T populateRootVersionIfNecessary(T newRoot) { - - if (!hasNewVersion()) { - return newRoot; - } - // Does the root entity have a version attribute? - RelationalPersistentEntity persistentEntity = (RelationalPersistentEntity) context - .getRequiredPersistentEntity(newRoot.getClass()); - - return RelationalEntityVersionUtils.setVersionNumberOnEntity(newRoot, getNewVersion(), persistentEntity, converter); - } - @SuppressWarnings("unchecked") @Nullable T populateIdsIfNecessary() { @@ -372,20 +317,12 @@ private void updateWithoutVersion(DbAction.UpdateRoot update) { } } - private void updateWithVersion(DbAction.UpdateRoot update, RelationalPersistentEntity persistentEntity) { - - // If the root aggregate has a version property, increment it. - Number previousVersion = RelationalEntityVersionUtils.getVersionNumberFromEntity(update.getEntity(), - persistentEntity, converter); + private void updateWithVersion(DbAction.UpdateRoot update) { + Number previousVersion = update.getPreviousVersion(); Assert.notNull(previousVersion, "The root aggregate cannot be updated because the version property is null."); - setNewVersion(previousVersion.longValue() + 1); - - T rootEntity = RelationalEntityVersionUtils.setVersionNumberOnEntity(update.getEntity(), getNewVersion(), - persistentEntity, converter); - - if (!accessStrategy.updateWithVersion(rootEntity, update.getEntityType(), previousVersion)) { + if (!accessStrategy.updateWithVersion(update.getEntity(), update.getEntityType(), previousVersion)) { throw new OptimisticLockingFailureException(String.format(UPDATE_FAILED_OPTIMISTIC_LOCKING, update.getEntity())); } 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 eab91a0d97..4d426f0bb1 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 @@ -35,8 +35,10 @@ import org.springframework.data.relational.core.conversion.RelationalEntityDeleteWriter; import org.springframework.data.relational.core.conversion.RelationalEntityInsertWriter; import org.springframework.data.relational.core.conversion.RelationalEntityUpdateWriter; +import org.springframework.data.relational.core.conversion.RelationalEntityVersionUtils; 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.support.PageableExecutionUtils; import org.springframework.lang.Nullable; @@ -63,6 +65,7 @@ public class JdbcAggregateTemplate implements JdbcAggregateOperations { private final DataAccessStrategy accessStrategy; private final AggregateChangeExecutor executor; + private final JdbcConverter converter; private EntityCallbacks entityCallbacks = EntityCallbacks.create(); @@ -86,6 +89,7 @@ public JdbcAggregateTemplate(ApplicationContext publisher, RelationalMappingCont this.publisher = publisher; this.context = context; this.accessStrategy = dataAccessStrategy; + this.converter = converter; this.jdbcEntityInsertWriter = new RelationalEntityInsertWriter(context); this.jdbcEntityUpdateWriter = new RelationalEntityUpdateWriter(context); @@ -115,6 +119,7 @@ public JdbcAggregateTemplate(ApplicationEventPublisher publisher, RelationalMapp this.publisher = publisher; this.context = context; this.accessStrategy = dataAccessStrategy; + this.converter = converter; this.jdbcEntityInsertWriter = new RelationalEntityInsertWriter(context); this.jdbcEntityUpdateWriter = new RelationalEntityUpdateWriter(context); @@ -332,7 +337,7 @@ private T store(T aggregateRoot, Function> chan MutableAggregateChange change = changeCreator.apply(aggregateRoot); - aggregateRoot = triggerBeforeSave(aggregateRoot, change); + aggregateRoot = triggerBeforeSave(change.getEntity(), change); change.setEntity(aggregateRoot); @@ -359,21 +364,58 @@ private void deleteTree(Object id, @Nullable T entity, Class domainType) private MutableAggregateChange createInsertChange(T instance) { - MutableAggregateChange aggregateChange = MutableAggregateChange.forSave(instance); - jdbcEntityInsertWriter.write(instance, aggregateChange); + RelationalPersistentEntity persistentEntity = getRequiredPersistentEntity(instance); + T preparedInstance = instance; + if (persistentEntity.hasVersionProperty()) { + RelationalPersistentProperty versionProperty = persistentEntity.getRequiredVersionProperty(); + + long initialVersion = versionProperty.getActualType().isPrimitive() ? 1L : 0; + + preparedInstance = RelationalEntityVersionUtils.setVersionNumberOnEntity( // + instance, initialVersion, persistentEntity, converter); + } + MutableAggregateChange aggregateChange = MutableAggregateChange.forSave(preparedInstance); + jdbcEntityInsertWriter.write(preparedInstance, aggregateChange); return aggregateChange; } private MutableAggregateChange createUpdateChange(T instance) { - MutableAggregateChange aggregateChange = MutableAggregateChange.forSave(instance); - jdbcEntityUpdateWriter.write(instance, aggregateChange); + 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); + + 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); + } + MutableAggregateChange aggregateChange = MutableAggregateChange.forSave(preparedInstance, previousVersion); + jdbcEntityUpdateWriter.write(preparedInstance, aggregateChange); return aggregateChange; } + @SuppressWarnings("unchecked") + private RelationalPersistentEntity getRequiredPersistentEntity(T instance) { + return (RelationalPersistentEntity) context.getRequiredPersistentEntity(instance.getClass()); + } + private MutableAggregateChange createDeletingChange(Object id, @Nullable T entity, Class domainType) { - MutableAggregateChange aggregateChange = MutableAggregateChange.forDelete(domainType, entity); + Number previousVersion = null; + if (entity != null) { + RelationalPersistentEntity persistentEntity = getRequiredPersistentEntity(entity); + if (persistentEntity.hasVersionProperty()) { + previousVersion = RelationalEntityVersionUtils.getVersionNumberFromEntity(entity, persistentEntity, converter); + } + } + MutableAggregateChange aggregateChange = MutableAggregateChange.forDelete(domainType, entity, previousVersion); jdbcEntityDeleteWriter.write(id, aggregateChange); return aggregateChange; } diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateChangeExecutorContextImmutableUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateChangeExecutorContextImmutableUnitTests.java index 5d36c652e3..9a74fcefbc 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateChangeExecutorContextImmutableUnitTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateChangeExecutorContextImmutableUnitTests.java @@ -64,7 +64,7 @@ public void rootOfEmptySetOfActionsisNull() { } @Test // DATAJDBC-453 - public void afterInsertRootIdAndVersionMaybeUpdated() { + public void afterInsertRootIdMaybeUpdated() { // note that the root entity isn't the original one, but a new instance with the version set. when(accessStrategy.insert(any(DummyEntity.class), eq(DummyEntity.class), eq(Identifier.empty()), @@ -76,10 +76,6 @@ public void afterInsertRootIdAndVersionMaybeUpdated() { assertThat(newRoot).isNotNull(); assertThat(newRoot.id).isEqualTo(23L); - - newRoot = executionContext.populateRootVersionIfNecessary(newRoot); - - assertThat(newRoot.version).isEqualTo(1); } @Test // DATAJDBC-453 diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateChangeExecutorContextUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateChangeExecutorContextUnitTests.java index c3cb96ffb6..9db3d8d92c 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateChangeExecutorContextUnitTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateChangeExecutorContextUnitTests.java @@ -70,7 +70,7 @@ public void rootOfEmptySetOfActionIsNull() { } @Test // DATAJDBC-453 - public void afterInsertRootIdAndVersionMaybeUpdated() { + public void afterInsertRootIdMaybeUpdated() { when(accessStrategy.insert(root, DummyEntity.class, Identifier.empty(), IdValueSource.GENERATED)).thenReturn(23L); @@ -80,22 +80,6 @@ public void afterInsertRootIdAndVersionMaybeUpdated() { assertThat(newRoot).isNull(); assertThat(root.id).isEqualTo(23L); - - executionContext.populateRootVersionIfNecessary(root); - - assertThat(root.version).isEqualTo(1); - } - - @Test // DATAJDBC-507 - public void afterInsertNotPrimitiveVersionShouldBeZero() { - - DummyEntityNonPrimitiveVersion dummyEntityNonPrimitiveVersion = new DummyEntityNonPrimitiveVersion(); - - executionContext - .executeInsertRoot(new DbAction.InsertRoot<>(dummyEntityNonPrimitiveVersion, IdValueSource.GENERATED)); - executionContext.populateRootVersionIfNecessary(dummyEntityNonPrimitiveVersion); - - assertThat(dummyEntityNonPrimitiveVersion.version).isEqualTo(0); } @Test // DATAJDBC-453 @@ -218,19 +202,12 @@ PersistentPropertyPath toPath(String path) { private static class DummyEntity { @Id Long id; - @Version long version; Content content; List list = new ArrayList<>(); } - private static class DummyEntityNonPrimitiveVersion { - - @Id Long id; - @Version Long version; - } - private static class Content { @Id Long id; } 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 2362db6243..092feff304 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 @@ -23,13 +23,16 @@ 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; +import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; import org.springframework.context.ApplicationEventPublisher; import org.springframework.data.annotation.Id; +import org.springframework.data.annotation.Version; import org.springframework.data.domain.PageRequest; import org.springframework.data.domain.Sort; import org.springframework.data.jdbc.core.convert.BasicJdbcConverter; @@ -111,6 +114,124 @@ public void callbackOnSave() { assertThat(last).isEqualTo(third); } + @Test + void savePreparesInstanceWithInitialVersion_onInsert() { + + EntityWithVersion entity = new EntityWithVersion(1L); + when(callbacks.callback(any(), any(), any())).thenReturn(entity, entity); + + template.save(entity); + + 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 + void savePreparesInstanceWithInitialVersion_onInsert_whenVersionPropertyIsImmutable() { + + EntityWithImmutableVersion entity = new EntityWithImmutableVersion(1L, null); + when(callbacks.callback(any(), any(), any())).thenReturn(entity, entity); + + template.save(entity); + + 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 + void savePreparesInstanceWithInitialVersion_onInsert_whenVersionPropertyIsPrimitiveType() { + + EntityWithPrimitiveVersion entity = new EntityWithPrimitiveVersion(1L); + when(callbacks.callback(any(), any(), any())).thenReturn(entity, entity); + + template.save(entity); + + 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 + void savePreparesInstanceWithInitialVersion_onInsert__whenVersionPropertyIsImmutableAndPrimitiveType() { + + EntityWithImmutablePrimitiveVersion entity = new EntityWithImmutablePrimitiveVersion(1L, 0L); + when(callbacks.callback(any(), any(), any())).thenReturn(entity, entity); + + template.save(entity); + + ArgumentCaptor aggregateRootCaptor = ArgumentCaptor.forClass(Object.class); + verify(callbacks).callback(eq(BeforeSaveCallback.class), aggregateRootCaptor.capture(), any()); + EntityWithImmutablePrimitiveVersion afterConvert = (EntityWithImmutablePrimitiveVersion) aggregateRootCaptor.getValue(); + assertThat(afterConvert.getVersion()).isEqualTo(1L); + } + + @Test + void savePreparesChangeWithPreviousVersion_onUpdate() { + + when(dataAccessStrategy.updateWithVersion(any(), any(), any())).thenReturn(true); + EntityWithVersion entity = new EntityWithVersion(1L); + entity.setVersion(1L); + when(callbacks.callback(any(), any(), any())).thenReturn(entity, entity); + + template.save(entity); + + 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 + void savePreparesInstanceWithNextVersion_onUpdate() { + + when(dataAccessStrategy.updateWithVersion(any(), any(), any())).thenReturn(true); + EntityWithVersion entity = new EntityWithVersion(1L); + entity.setVersion(1L); + when(callbacks.callback(any(), any(), any())).thenReturn(entity, entity); + + template.save(entity); + + 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 + void savePreparesInstanceWithNextVersion_onUpdate_whenVersionPropertyIsImmutable() { + + when(dataAccessStrategy.updateWithVersion(any(), any(), any())).thenReturn(true); + EntityWithImmutableVersion entity = new EntityWithImmutableVersion(1L, 1L); + when(callbacks.callback(any(), any(), any())).thenReturn(entity, entity); + + template.save(entity); + + ArgumentCaptor aggregateRootCaptor = ArgumentCaptor.forClass(Object.class); + verify(callbacks).callback(eq(BeforeSaveCallback.class), aggregateRootCaptor.capture(), any()); + EntityWithImmutableVersion afterConvert = (EntityWithImmutableVersion) aggregateRootCaptor.getValue(); + assertThat(afterConvert.getVersion()).isEqualTo(2L); + } + + @Test + void deletePreparesChangeWithPreviousVersion_onDeleteByInstance() { + + EntityWithImmutableVersion entity = new EntityWithImmutableVersion(1L, 1L); + when(callbacks.callback(any(), any(), any())).thenReturn(entity, entity); + + template.delete(entity, EntityWithImmutableVersion.class); + + 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() { @@ -209,4 +330,40 @@ private static class SampleEntity { private String name; } + + @Data + @RequiredArgsConstructor + private static class EntityWithVersion { + + @Column("id1") @Id private final Long id; + + @Version private Long version; + } + + @Data + @RequiredArgsConstructor + private static class EntityWithImmutableVersion { + + @Column("id1") @Id private final Long id; + + @Version private final Long version; + } + + @Data + @RequiredArgsConstructor + private static class EntityWithPrimitiveVersion { + + @Column("id1") @Id private final Long id; + + @Version private long version; + } + + @Data + @RequiredArgsConstructor + private static class EntityWithImmutablePrimitiveVersion { + + @Column("id1") @Id private final Long id; + + @Version private final long version; + } } diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/DbAction.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/DbAction.java index ae7bb1ce1d..d4e66f9fa3 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/DbAction.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/DbAction.java @@ -166,14 +166,22 @@ class UpdateRoot implements WithEntity { private final T entity; - public UpdateRoot(T entity) { + @Nullable private final Number previousVersion; + + public UpdateRoot(T entity, @Nullable Number previousVersion) { this.entity = entity; + this.previousVersion = previousVersion; } public T getEntity() { return this.entity; } + @Nullable + public Number getPreviousVersion() { + return previousVersion; + } + public String toString() { return "DbAction.UpdateRoot(entity=" + this.getEntity() + ")"; } diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/DefaultAggregateChange.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/DefaultAggregateChange.java index 5d84d33e84..b5b9950174 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/DefaultAggregateChange.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/DefaultAggregateChange.java @@ -41,11 +41,15 @@ class DefaultAggregateChange implements MutableAggregateChange { /** Aggregate root, to which the change applies, if available */ private @Nullable T entity; - public DefaultAggregateChange(Kind kind, Class entityType, @Nullable T entity) { + /** The previous version assigned to the instance being changed, if available */ + @Nullable private final Number previousVersion; + + public DefaultAggregateChange(Kind kind, Class entityType, @Nullable T entity, @Nullable Number previousVersion) { this.kind = kind; this.entityType = entityType; this.entity = entity; + this.previousVersion = previousVersion; } /** @@ -95,6 +99,12 @@ public void setEntity(@Nullable T aggregateRoot) { this.entity = aggregateRoot; } + @Nullable + @Override + public Number getPreviousVersion() { + return previousVersion; + } + /* * (non-Javadoc) * @see org.springframework.data.relational.core.conversion.AggregateChange#getEntity() diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/MutableAggregateChange.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/MutableAggregateChange.java index 2df77a0e0c..ff12100d9b 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/MutableAggregateChange.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/MutableAggregateChange.java @@ -36,12 +36,25 @@ public interface MutableAggregateChange extends AggregateChange { * @return the {@link MutableAggregateChange} for saving the root {@code entity}. * @since 1.2 */ - @SuppressWarnings("unchecked") static MutableAggregateChange forSave(T entity) { + return forSave(entity, null); + } + + /** + * Factory method to create an {@link MutableAggregateChange} for saving entities. + * + * @param entity aggregate root to save. + * @param previousVersion the previous version assigned to the instance being saved. May be {@literal null}. + * @param entity type. + * @return the {@link MutableAggregateChange} for saving the root {@code entity}. + * @since 2.4 + */ + @SuppressWarnings("unchecked") + static MutableAggregateChange forSave(T entity, @Nullable Number previousVersion) { Assert.notNull(entity, "Entity must not be null"); - return new DefaultAggregateChange<>(Kind.SAVE, (Class) ClassUtils.getUserClass(entity), entity); + return new DefaultAggregateChange<>(Kind.SAVE, (Class) ClassUtils.getUserClass(entity), entity, previousVersion); } /** @@ -70,10 +83,24 @@ static MutableAggregateChange forDelete(T entity) { * @since 1.2 */ static MutableAggregateChange forDelete(Class entityClass, @Nullable T entity) { + return forDelete(entityClass, entity, null); + } + + /** + * Factory method to create an {@link MutableAggregateChange} for deleting entities. + * + * @param entityClass aggregate root type. + * @param entity aggregate root to delete. + * @param previousVersion the previous version assigned to the instance being saved. May be {@literal null}. + * @param entity type. + * @return the {@link MutableAggregateChange} for deleting the root {@code entity}. + * @since 2.4 + */ + static MutableAggregateChange forDelete(Class entityClass, @Nullable T entity, @Nullable Number previousVersion) { Assert.notNull(entityClass, "Entity class must not be null"); - return new DefaultAggregateChange<>(Kind.DELETE, entityClass, entity); + return new DefaultAggregateChange<>(Kind.DELETE, entityClass, entity, previousVersion); } /** @@ -89,4 +116,7 @@ static MutableAggregateChange forDelete(Class entityClass, @Nullable T * @param aggregateRoot may be {@literal null} if the change refers to a list of aggregates or references it by id. */ void setEntity(@Nullable T aggregateRoot); + + @Nullable + Number getPreviousVersion(); } diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/RelationalEntityDeleteWriter.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/RelationalEntityDeleteWriter.java index 46876968b9..c661e5cfc2 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/RelationalEntityDeleteWriter.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/RelationalEntityDeleteWriter.java @@ -88,7 +88,7 @@ private List> deleteAll(Class entityType) { return actions; } - private List> deleteRoot(Object id, AggregateChange aggregateChange) { + private List> deleteRoot(Object id, MutableAggregateChange aggregateChange) { List> deleteReferencedActions = deleteReferencedEntities(id, aggregateChange); @@ -98,7 +98,7 @@ private List> deleteRoot(Object id, AggregateChange aggregate } actions.addAll(deleteReferencedActions); - actions.add(new DbAction.DeleteRoot<>(id, aggregateChange.getEntityType(), getVersion(aggregateChange))); + actions.add(new DbAction.DeleteRoot<>(id, aggregateChange.getEntityType(), aggregateChange.getPreviousVersion())); return actions; } @@ -121,22 +121,4 @@ private List> deleteReferencedEntities(Object id, AggregateChange return actions; } - @Nullable - private Number getVersion(AggregateChange aggregateChange) { - - RelationalPersistentEntity persistentEntity = context - .getRequiredPersistentEntity(aggregateChange.getEntityType()); - if (!persistentEntity.hasVersionProperty()) { - return null; - } - - Object entity = aggregateChange.getEntity(); - - if (entity == null) { - return null; - } - - return (Number) persistentEntity.getPropertyAccessor(entity) - .getProperty(persistentEntity.getRequiredVersionProperty()); - } } diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/WritingContext.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/WritingContext.java index 86806fee0e..5f936dd751 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/WritingContext.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/WritingContext.java @@ -52,6 +52,7 @@ class WritingContext { private final Map> previousActions = new HashMap<>(); private final Map, List> nodesCache = new HashMap<>(); private final IdValueSource rootIdValueSource; + @Nullable private final Number previousVersion; WritingContext(RelationalMappingContext context, Object root, MutableAggregateChange aggregateChange) { @@ -59,6 +60,7 @@ class WritingContext { this.root = root; this.entity = aggregateChange.getEntity(); this.entityType = aggregateChange.getEntityType(); + this.previousVersion = aggregateChange.getPreviousVersion(); this.rootIdValueSource = IdValueSource.forInstance(root, context.getRequiredPersistentEntity(aggregateChange.getEntityType())); this.paths = context.findPersistentPropertyPaths(entityType, (p) -> p.isEntity() && !p.isEmbedded()); @@ -88,7 +90,7 @@ List> insert() { List> update() { List> actions = new ArrayList<>(); - actions.add(setRootAction(new DbAction.UpdateRoot<>(entity))); + actions.add(setRootAction(new DbAction.UpdateRoot<>(entity, previousVersion))); actions.addAll(deleteReferenced()); actions.addAll(insertReferenced()); return actions; @@ -103,7 +105,7 @@ List> save() { actions.addAll(insertReferenced()); } else { - actions.add(setRootAction(new DbAction.UpdateRoot<>(entity))); + actions.add(setRootAction(new DbAction.UpdateRoot<>(entity, previousVersion))); actions.addAll(deleteReferenced()); actions.addAll(insertReferenced()); } diff --git a/spring-data-relational/src/test/java/org/springframework/data/relational/core/conversion/RelationalEntityWriterUnitTests.java b/spring-data-relational/src/test/java/org/springframework/data/relational/core/conversion/RelationalEntityWriterUnitTests.java index ae6e32c65d..bb449fe4c3 100644 --- a/spring-data-relational/src/test/java/org/springframework/data/relational/core/conversion/RelationalEntityWriterUnitTests.java +++ b/spring-data-relational/src/test/java/org/springframework/data/relational/core/conversion/RelationalEntityWriterUnitTests.java @@ -85,7 +85,7 @@ public void newEntityGetsConvertedToOneInsert() { SingleReferenceEntity entity = new SingleReferenceEntity(null); MutableAggregateChange aggregateChange = // - new DefaultAggregateChange<>(AggregateChange.Kind.SAVE, SingleReferenceEntity.class, entity); + new DefaultAggregateChange<>(AggregateChange.Kind.SAVE, SingleReferenceEntity.class, entity, null); converter.write(entity, aggregateChange); @@ -107,7 +107,7 @@ void newEntityWithPrimitiveLongId_insertDoesNotIncludeId_whenIdValueIsZero() { PrimitiveLongIdEntity entity = new PrimitiveLongIdEntity(); MutableAggregateChange aggregateChange = // - new DefaultAggregateChange<>(AggregateChange.Kind.SAVE, PrimitiveLongIdEntity.class, entity); + new DefaultAggregateChange<>(AggregateChange.Kind.SAVE, PrimitiveLongIdEntity.class, entity, null); converter.write(entity, aggregateChange); @@ -129,7 +129,7 @@ void newEntityWithPrimitiveIntId_insertDoesNotIncludeId_whenIdValueIsZero() { PrimitiveIntIdEntity entity = new PrimitiveIntIdEntity(); MutableAggregateChange aggregateChange = // - new DefaultAggregateChange<>(AggregateChange.Kind.SAVE, PrimitiveIntIdEntity.class, entity); + new DefaultAggregateChange<>(AggregateChange.Kind.SAVE, PrimitiveIntIdEntity.class, entity, null); converter.write(entity, aggregateChange); @@ -153,7 +153,7 @@ public void newEntityGetsConvertedToOneInsertByEmbeddedEntities() { entity.other = new Element(2L); MutableAggregateChange aggregateChange = // - new DefaultAggregateChange<>(AggregateChange.Kind.SAVE, EmbeddedReferenceEntity.class, entity); + new DefaultAggregateChange<>(AggregateChange.Kind.SAVE, EmbeddedReferenceEntity.class, entity, null); converter.write(entity, aggregateChange); @@ -177,7 +177,7 @@ public void newEntityWithReferenceGetsConvertedToTwoInserts() { entity.other = new Element(null); MutableAggregateChange aggregateChange = // - new DefaultAggregateChange<>(AggregateChange.Kind.SAVE, SingleReferenceEntity.class, entity); + new DefaultAggregateChange<>(AggregateChange.Kind.SAVE, SingleReferenceEntity.class, entity, null); converter.write(entity, aggregateChange); @@ -203,7 +203,7 @@ void newEntityWithReference_whenReferenceHasPrimitiveId_insertDoesNotIncludeId_w entity.primitiveIntIdEntity = new PrimitiveIntIdEntity(); MutableAggregateChange aggregateChange = // - new DefaultAggregateChange<>(AggregateChange.Kind.SAVE, EntityWithReferencesToPrimitiveIdEntity.class, entity); + new DefaultAggregateChange<>(AggregateChange.Kind.SAVE, EntityWithReferencesToPrimitiveIdEntity.class, entity, null); converter.write(entity, aggregateChange); @@ -230,7 +230,7 @@ public void existingEntityGetsConvertedToDeletePlusUpdate() { SingleReferenceEntity entity = new SingleReferenceEntity(SOME_ENTITY_ID); MutableAggregateChange aggregateChange = // - new DefaultAggregateChange<>(AggregateChange.Kind.SAVE, SingleReferenceEntity.class, entity); + new DefaultAggregateChange<>(AggregateChange.Kind.SAVE, SingleReferenceEntity.class, entity, 1L); converter.write(entity, aggregateChange); @@ -239,10 +239,11 @@ public void existingEntityGetsConvertedToDeletePlusUpdate() { DbAction::getEntityType, // DbActionTestSupport::extractPath, // DbActionTestSupport::actualEntityType, // - DbActionTestSupport::isWithDependsOn) // + DbActionTestSupport::isWithDependsOn, // + dbAction -> dbAction instanceof UpdateRoot ? ((UpdateRoot) dbAction).getPreviousVersion() : null) // .containsExactly( // - tuple(UpdateRoot.class, SingleReferenceEntity.class, "", SingleReferenceEntity.class, false), // - tuple(Delete.class, Element.class, "other", null, false) // + tuple(UpdateRoot.class, SingleReferenceEntity.class, "", SingleReferenceEntity.class, false, 1L), // + tuple(Delete.class, Element.class, "other", null, false, null) // ); } @@ -253,7 +254,7 @@ public void newReferenceTriggersDeletePlusInsert() { entity.other = new Element(null); MutableAggregateChange aggregateChange = new DefaultAggregateChange<>( - AggregateChange.Kind.SAVE, SingleReferenceEntity.class, entity); + AggregateChange.Kind.SAVE, SingleReferenceEntity.class, entity, 1L); converter.write(entity, aggregateChange); @@ -276,7 +277,7 @@ public void newEntityWithEmptySetResultsInSingleInsert() { SetContainer entity = new SetContainer(null); MutableAggregateChange aggregateChange = new DefaultAggregateChange<>(AggregateChange.Kind.SAVE, - SetContainer.class, entity); + SetContainer.class, entity, null); converter.write(entity, aggregateChange); @@ -299,7 +300,7 @@ public void newEntityWithSetContainingMultipleElementsResultsInAnInsertForTheBat entity.elements.add(new Element(null)); MutableAggregateChange aggregateChange = new DefaultAggregateChange<>(AggregateChange.Kind.SAVE, - SetContainer.class, entity); + SetContainer.class, entity, null); converter.write(entity, aggregateChange); List> actions = extractActions(aggregateChange); @@ -342,7 +343,7 @@ public void cascadingReferencesTriggerCascadingActions() { ); MutableAggregateChange aggregateChange = new DefaultAggregateChange<>( - AggregateChange.Kind.SAVE, CascadingReferenceEntity.class, entity); + AggregateChange.Kind.SAVE, CascadingReferenceEntity.class, entity, null); converter.write(entity, aggregateChange); @@ -404,7 +405,7 @@ public void cascadingReferencesTriggerCascadingActionsForUpdate() { ); MutableAggregateChange aggregateChange = new DefaultAggregateChange<>( - AggregateChange.Kind.SAVE, CascadingReferenceEntity.class, entity); + AggregateChange.Kind.SAVE, CascadingReferenceEntity.class, entity, 1L); converter.write(entity, aggregateChange); @@ -456,7 +457,7 @@ public void newEntityWithEmptyMapResultsInSingleInsert() { MapContainer entity = new MapContainer(null); MutableAggregateChange aggregateChange = new DefaultAggregateChange<>(AggregateChange.Kind.SAVE, - MapContainer.class, entity); + MapContainer.class, entity, null); converter.write(entity, aggregateChange); @@ -476,7 +477,7 @@ public void newEntityWithMapResultsInAdditionalInsertPerElement() { entity.elements.put("two", new Element(null)); MutableAggregateChange aggregateChange = new DefaultAggregateChange<>(AggregateChange.Kind.SAVE, - MapContainer.class, entity); + MapContainer.class, entity, null); converter.write(entity, aggregateChange); List> actions = extractActions(aggregateChange); @@ -520,7 +521,7 @@ public void newEntityWithFullMapResultsInAdditionalInsertPerElement() { entity.elements.put("b", new Element(null)); MutableAggregateChange aggregateChange = new DefaultAggregateChange<>(AggregateChange.Kind.SAVE, - MapContainer.class, entity); + MapContainer.class, entity, null); converter.write(entity, aggregateChange); List> actions = extractActions(aggregateChange); @@ -560,7 +561,7 @@ public void newEntityWithEmptyListResultsInSingleInsert() { ListContainer entity = new ListContainer(null); MutableAggregateChange aggregateChange = new DefaultAggregateChange<>(AggregateChange.Kind.SAVE, - ListContainer.class, entity); + ListContainer.class, entity, null); converter.write(entity, aggregateChange); @@ -580,7 +581,7 @@ public void newEntityWithListResultsInAdditionalInsertPerElement() { entity.elements.add(new Element(null)); MutableAggregateChange aggregateChange = new DefaultAggregateChange<>(AggregateChange.Kind.SAVE, - ListContainer.class, entity); + ListContainer.class, entity, null); converter.write(entity, aggregateChange); List> actions = extractActions(aggregateChange); @@ -612,7 +613,7 @@ public void mapTriggersDeletePlusInsert() { entity.elements.put("one", new Element(null)); MutableAggregateChange aggregateChange = new DefaultAggregateChange<>(AggregateChange.Kind.SAVE, - MapContainer.class, entity); + MapContainer.class, entity, 1L); converter.write(entity, aggregateChange); @@ -636,7 +637,7 @@ public void listTriggersDeletePlusInsert() { entity.elements.add(new Element(null)); MutableAggregateChange aggregateChange = new DefaultAggregateChange<>(AggregateChange.Kind.SAVE, - ListContainer.class, entity); + ListContainer.class, entity, 1L); converter.write(entity, aggregateChange); @@ -661,7 +662,7 @@ public void multiLevelQualifiedReferencesWithId() { listMapContainer.maps.get(0).elements.put("one", new Element(null)); MutableAggregateChange aggregateChange = new DefaultAggregateChange<>(AggregateChange.Kind.SAVE, - ListMapContainer.class, listMapContainer); + ListMapContainer.class, listMapContainer, 1L); converter.write(listMapContainer, aggregateChange); @@ -689,7 +690,7 @@ public void multiLevelQualifiedReferencesWithOutId() { listMapContainer.maps.get(0).elements.put("one", new NoIdElement()); MutableAggregateChange aggregateChange = new DefaultAggregateChange<>( - AggregateChange.Kind.SAVE, NoIdListMapContainer.class, listMapContainer); + AggregateChange.Kind.SAVE, NoIdListMapContainer.class, listMapContainer, 1L); converter.write(listMapContainer, aggregateChange); @@ -716,7 +717,7 @@ public void savingANullEmbeddedWithEntity() { // the embedded is null !!! MutableAggregateChange aggregateChange = // - new DefaultAggregateChange<>(AggregateChange.Kind.SAVE, EmbeddedReferenceChainEntity.class, entity); + new DefaultAggregateChange<>(AggregateChange.Kind.SAVE, EmbeddedReferenceChainEntity.class, entity, null); converter.write(entity, aggregateChange); @@ -741,7 +742,7 @@ public void savingInnerNullEmbeddedWithEntity() { // the embedded is null !!! MutableAggregateChange aggregateChange = // - new DefaultAggregateChange<>(AggregateChange.Kind.SAVE, RootWithEmbeddedReferenceChainEntity.class, root); + new DefaultAggregateChange<>(AggregateChange.Kind.SAVE, RootWithEmbeddedReferenceChainEntity.class, root, null); converter.write(root, aggregateChange); @@ -769,7 +770,7 @@ void newEntityWithCollectionWhereSomeElementsHaveIdSet_producesABatchInsertEachF root.elements.add(new Element(null)); root.elements.add(new Element(2L)); MutableAggregateChange aggregateChange = // - new DefaultAggregateChange<>(AggregateChange.Kind.SAVE, ListContainer.class, root); + new DefaultAggregateChange<>(AggregateChange.Kind.SAVE, ListContainer.class, root, null); converter.write(root, aggregateChange); @@ -817,7 +818,7 @@ void newEntityWithCollection_whenElementHasPrimitiveId_batchInsertDoesNotInclude entity.primitiveIntIdEntities.add(new PrimitiveIntIdEntity()); MutableAggregateChange aggregateChange = // - new DefaultAggregateChange<>(AggregateChange.Kind.SAVE, EntityWithReferencesToPrimitiveIdEntity.class, entity); + new DefaultAggregateChange<>(AggregateChange.Kind.SAVE, EntityWithReferencesToPrimitiveIdEntity.class, entity, null); converter.write(entity, aggregateChange); From 9eb2dcaf92ad7debcbb7bbfd904730c4cb2eaff4 Mon Sep 17 00:00:00 2001 From: Chirag Tailor Date: Thu, 17 Mar 2022 14:44:20 -0500 Subject: [PATCH 4/5] Copy over test from https://github.com/spring-projects/spring-data-relational/pull/1150 --- ...JdbcAggregateTemplateIntegrationTests.java | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) 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 5a8b1fd77c..07450a7eb8 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 @@ -819,6 +819,32 @@ void saveAndUpdateAggregateWithImmutableVersion() { .hasRootCauseInstanceOf(OptimisticLockingFailureException.class); } + @Test // GH-1137 + void testUpdateEntityWithVersionDoesNotTriggerAnewConstructorInvocation() { + AggregateWithImmutableVersion aggregateWithImmutableVersion = new AggregateWithImmutableVersion(null, null); + + final AggregateWithImmutableVersion savedRoot = template.save(aggregateWithImmutableVersion); + + assertThat(savedRoot).isNotNull(); + assertThat(savedRoot.version).isEqualTo(0L); + + assertThat(AggregateWithImmutableVersion.constructorInvocations).containsExactly( + new ConstructorInvocation(null, null), // Initial invocation, done by client + new ConstructorInvocation(null, savedRoot.version), // Assigning the version + new ConstructorInvocation(savedRoot.id, savedRoot.version) // Assigning the id + ); + + AggregateWithImmutableVersion.clearConstructorInvocationData(); + + final AggregateWithImmutableVersion updatedRoot = template.save(savedRoot); + + assertThat(updatedRoot).isNotNull(); + assertThat(updatedRoot.version).isEqualTo(1L); + + // Expect only one assignnment of the version to AggregateWithImmutableVersion + assertThat(AggregateWithImmutableVersion.constructorInvocations).containsOnly(new ConstructorInvocation(savedRoot.id, updatedRoot.version)); + } + @Test // DATAJDBC-219 Test that a delete with a version attribute works as expected. void deleteAggregateWithVersion() { @@ -1227,6 +1253,25 @@ static class AggregateWithImmutableVersion { @Id Long id; @Version Long version; + + private final static List constructorInvocations = new ArrayList<>(); + + public static void clearConstructorInvocationData() { + constructorInvocations.clear(); + } + + public AggregateWithImmutableVersion(Long id, Long version) { + constructorInvocations.add(new ConstructorInvocation(id, version)); + this.id = id; + this.version = version; + } + } + + @Value + @EqualsAndHashCode + private static class ConstructorInvocation { + private Long id; + private Long version; } @Data From 5933fd183ee8bc3883dce71e984a0c3c100545da Mon Sep 17 00:00:00 2001 From: Chirag Tailor Date: Thu, 17 Mar 2022 14:54:08 -0500 Subject: [PATCH 5/5] Update license and class headers. --- .../springframework/data/jdbc/core/JdbcAggregateTemplate.java | 3 ++- .../data/jdbc/core/JdbcAggregateTemplateUnitTests.java | 3 ++- .../relational/core/conversion/DefaultAggregateChange.java | 1 + .../relational/core/conversion/MutableAggregateChange.java | 1 + .../core/conversion/RelationalEntityDeleteWriter.java | 1 + 5 files changed, 7 insertions(+), 2 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 4d426f0bb1..f42849fbbd 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 @@ -1,5 +1,5 @@ /* - * Copyright 2017-2021 the original author or authors. + * Copyright 2017-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -53,6 +53,7 @@ * @author Christoph Strobl * @author Milan Milanov * @author Myeonghyeon Lee + * @author Chirag Tailor */ public class JdbcAggregateTemplate implements JdbcAggregateOperations { 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 092feff304..d9e837b644 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 @@ -1,5 +1,5 @@ /* - * Copyright 2019-2021 the original author or authors. + * Copyright 2019-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -58,6 +58,7 @@ * @author Christoph Strobl * @author Mark Paluch * @author Milan Milanov + * @author Chirag Tailor */ @ExtendWith(MockitoExtension.class) public class JdbcAggregateTemplateUnitTests { diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/DefaultAggregateChange.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/DefaultAggregateChange.java index b5b9950174..ea53f4b0f9 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/DefaultAggregateChange.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/DefaultAggregateChange.java @@ -27,6 +27,7 @@ * * @author Jens Schauder * @author Mark Paluch + * @author Chirag Tailor * @since 2.0 */ class DefaultAggregateChange implements MutableAggregateChange { diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/MutableAggregateChange.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/MutableAggregateChange.java index ff12100d9b..79d1f6fc9b 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/MutableAggregateChange.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/MutableAggregateChange.java @@ -24,6 +24,7 @@ * * @author Jens Schauder * @author Mark Paluch + * @author Chirag Tailor * @since 2.0 */ public interface MutableAggregateChange extends AggregateChange { diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/RelationalEntityDeleteWriter.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/RelationalEntityDeleteWriter.java index c661e5cfc2..1aa436374f 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/RelationalEntityDeleteWriter.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/RelationalEntityDeleteWriter.java @@ -37,6 +37,7 @@ * @author Bastian Wilhelm * @author Tyler Van Gorder * @author Myeonghyeon Lee + * @author Chirag Tailor */ public class RelationalEntityDeleteWriter implements EntityWriter> {