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-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..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
@@ -51,10 +51,8 @@ T execute(AggregateChange aggregateChange) {
aggregateChange.forEachAction(action -> execute(action, executionContext));
T root = executionContext.populateIdsIfNecessary();
- root = root == null ? aggregateChange.getEntity() : root;
-
- if (root != null) {
- root = executionContext.populateRootVersionIfNecessary(root);
+ if (root == null) {
+ root = aggregateChange.getEntity();
}
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 778083db11..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);
}
}
@@ -147,16 +122,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) {
@@ -258,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() {
@@ -378,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..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.
@@ -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;
@@ -51,6 +53,7 @@
* @author Christoph Strobl
* @author Milan Milanov
* @author Myeonghyeon Lee
+ * @author Chirag Tailor
*/
public class JdbcAggregateTemplate implements JdbcAggregateOperations {
@@ -63,6 +66,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 +90,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 +120,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 +338,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 +365,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/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
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..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.
@@ -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;
@@ -55,6 +58,7 @@
* @author Christoph Strobl
* @author Mark Paluch
* @author Milan Milanov
+ * @author Chirag Tailor
*/
@ExtendWith(MockitoExtension.class)
public class JdbcAggregateTemplateUnitTests {
@@ -111,6 +115,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