DATAJDBC-507 - Optimistic locking version for non primitives start with 0.

This makes the behaviour consistent with that of other Spring Data modules.

Original pull request: #208.

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

@@ -47,6 +47,7 @@ import org.springframework.util.Assert;
 
 /**
  * @author Jens Schauder
+ * @author Umut Erturk
  */
 class JdbcAggregateChangeExecutionContext {
 
@@ -71,12 +72,14 @@ class JdbcAggregateChangeExecutionContext {
 		RelationalPersistentEntity<T> persistentEntity = getRequiredPersistentEntity(insert.getEntityType());
 
 		Object id;
-		if (persistentEntity.hasVersionProperty()) {
+		RelationalPersistentProperty versionProperty = persistentEntity.getVersionProperty();
+		if (versionProperty != null) {
+			long initialVersion = versionProperty.getActualType().isPrimitive() ? 1L : 0;
 
-			T rootEntity = RelationalEntityVersionUtils.setVersionNumberOnEntity(insert.getEntity(), 1, persistentEntity,
+			T rootEntity = RelationalEntityVersionUtils
-					converter);
+					.setVersionNumberOnEntity(insert.getEntity(), initialVersion, persistentEntity, converter);
 			id = accessStrategy.insert(rootEntity, insert.getEntityType(), Identifier.empty());
-			setNewVersion(1);
+			setNewVersion(initialVersion);
 		} else {
 			id = accessStrategy.insert(insert.getEntity(), insert.getEntityType(), Identifier.empty());
 		}

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

@@ -42,6 +42,7 @@ import org.springframework.lang.Nullable;
  * Unit tests for {@link JdbcAggregateChangeExecutionContext}.
  *
  * @author Jens Schauder
+ * @author Umut Erturk
  */
 public class JdbcAggregateChangeExecutorContextUnitTests {
 
@@ -82,6 +83,25 @@ public class JdbcAggregateChangeExecutorContextUnitTests {
 		assertThat(root.version).isEqualTo(1);
 	}
 
+	@Test // DATAJDBC-507
+	public void afterInsertPrimitiveVersionShouldBe1() {
+		DummyEntityNonPrimitiveVersion dummyEntityNonPrimitiveVersion = new DummyEntityNonPrimitiveVersion();
+		when(
+				accessStrategy.insert(dummyEntityNonPrimitiveVersion, DummyEntityNonPrimitiveVersion.class, Identifier.empty()))
+				.thenReturn(23L);
+
+		executionContext.executeInsertRoot(new DbAction.InsertRoot<>(dummyEntityNonPrimitiveVersion));
+
+		DummyEntity newRoot = executionContext.populateIdsIfNecessary();
+
+		assertThat(newRoot).isNull();
+		assertThat(dummyEntityNonPrimitiveVersion.id).isEqualTo(23L);
+
+		executionContext.populateRootVersionIfNecessary(dummyEntityNonPrimitiveVersion);
+
+		assertThat(dummyEntityNonPrimitiveVersion.version).isEqualTo(0);
+	}
+
 	@Test // DATAJDBC-453
 	public void idGenerationOfChild() {
 
@@ -164,6 +184,12 @@ public class JdbcAggregateChangeExecutorContextUnitTests {
 		List<Content> list = new ArrayList<>();
 	}
 
+	private static class DummyEntityNonPrimitiveVersion {
+
+		@Id Long id;
+		@Version Long version;
+	}
+
 	private static class Content {
 		@Id Long id;
 	}

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

@@ -748,28 +748,28 @@ public class JdbcAggregateTemplateIntegrationTests {
 
 		AggregateWithImmutableVersion aggregate = new AggregateWithImmutableVersion(null, null);
 		aggregate = template.save(aggregate);
-		assertThat(aggregate.version).isEqualTo(1L);
+		assertThat(aggregate.version).isEqualTo(0L);
 
 		Long id = aggregate.getId();
 
 		AggregateWithImmutableVersion reloadedAggregate = template.findById(id, aggregate.getClass());
-		assertThat(reloadedAggregate.getVersion()).describedAs("version field should initially have the value 1")
+		assertThat(reloadedAggregate.getVersion()).describedAs("version field should initially have the value 0")
-				.isEqualTo(1L);
+				.isEqualTo(0L);
 
 		AggregateWithImmutableVersion savedAgain = template.save(reloadedAggregate);
 		AggregateWithImmutableVersion reloadedAgain = template.findById(id, aggregate.getClass());
 
 		assertThat(savedAgain.version).describedAs("The object returned by save should have an increased version")
-				.isEqualTo(2L);
+				.isEqualTo(1L);
 
 		assertThat(reloadedAgain.getVersion()).describedAs("version field should increment by one with each save")
-				.isEqualTo(2L);
+				.isEqualTo(1L);
 
-		assertThatThrownBy(() -> template.save(new AggregateWithImmutableVersion(id, 1L)))
+		assertThatThrownBy(() -> template.save(new AggregateWithImmutableVersion(id, 0L)))
 				.describedAs("saving an aggregate with an outdated version should raise an exception")
 				.hasRootCauseInstanceOf(OptimisticLockingFailureException.class);
 
-		assertThatThrownBy(() -> template.save(new AggregateWithImmutableVersion(id, 3L)))
+		assertThatThrownBy(() -> template.save(new AggregateWithImmutableVersion(id, 2L)))
 				.describedAs("saving an aggregate with a future version should raise an exception")
 				.hasRootCauseInstanceOf(OptimisticLockingFailureException.class);
 	}
@@ -779,6 +779,8 @@ public class JdbcAggregateTemplateIntegrationTests {
 
 		AggregateWithImmutableVersion aggregate = new AggregateWithImmutableVersion(null, null);
 		aggregate = template.save(aggregate);
+		// as non-primitive versions start from 0, we need to save one more time to make version equal 1
+		aggregate = template.save(aggregate);
 
 		// Should have an ID and a version of 1.
 		final Long id = aggregate.getId();
@@ -789,7 +791,7 @@ public class JdbcAggregateTemplateIntegrationTests {
 						.hasRootCauseInstanceOf(OptimisticLockingFailureException.class);
 
 		assertThatThrownBy(
-				() -> template.delete(new AggregateWithImmutableVersion(id, 3L), AggregateWithImmutableVersion.class))
+				() -> template.delete(new AggregateWithImmutableVersion(id, 2L), AggregateWithImmutableVersion.class))
 						.describedAs("deleting an aggregate with a future version should raise an exception")
 						.hasRootCauseInstanceOf(OptimisticLockingFailureException.class);
 
@@ -811,7 +813,7 @@ public class JdbcAggregateTemplateIntegrationTests {
 
 	@Test // DATAJDBC-219
 	public void saveAndUpdateAggregateWithPrimitiveLongVersion() {
-		saveAndUpdateAggregateWithVersion(new AggregateWithPrimitiveLongVersion(), Number::longValue);
+		saveAndUpdateAggregateWithPrimitiveVersion(new AggregateWithPrimitiveLongVersion(), Number::longValue);
 	}
 
 	@Test // DATAJDBC-219
@@ -821,7 +823,7 @@ public class JdbcAggregateTemplateIntegrationTests {
 
 	@Test // DATAJDBC-219
 	public void saveAndUpdateAggregateWithPrimitiveIntegerVersion() {
-		saveAndUpdateAggregateWithVersion(new AggregateWithPrimitiveIntegerVersion(), Number::intValue);
+		saveAndUpdateAggregateWithPrimitiveVersion(new AggregateWithPrimitiveIntegerVersion(), Number::intValue);
 	}
 
 	@Test // DATAJDBC-219
@@ -831,7 +833,7 @@ public class JdbcAggregateTemplateIntegrationTests {
 
 	@Test // DATAJDBC-219
 	public void saveAndUpdateAggregateWithPrimitiveShortVersion() {
-		saveAndUpdateAggregateWithVersion(new AggregateWithPrimitiveShortVersion(), Number::shortValue);
+		saveAndUpdateAggregateWithPrimitiveVersion(new AggregateWithPrimitiveShortVersion(), Number::shortValue);
 	}
 
 	@Test // DATAJDBC-462
@@ -849,6 +851,31 @@ public class JdbcAggregateTemplateIntegrationTests {
 
 		template.save(aggregate);
 
+		VersionedAggregate reloadedAggregate = template.findById(aggregate.getId(), aggregate.getClass());
+		assertThat(reloadedAggregate.getVersion()).isEqualTo(toConcreteNumber.apply(0))
+				.withFailMessage("version field should initially have the value 0");
+		template.save(reloadedAggregate);
+
+		VersionedAggregate updatedAggregate = template.findById(aggregate.getId(), aggregate.getClass());
+		assertThat(updatedAggregate.getVersion()).isEqualTo(toConcreteNumber.apply(1))
+				.withFailMessage("version field should increment by one with each save");
+
+		reloadedAggregate.setVersion(toConcreteNumber.apply(0));
+		assertThatThrownBy(() -> template.save(reloadedAggregate))
+				.hasRootCauseInstanceOf(OptimisticLockingFailureException.class)
+				.withFailMessage("saving an aggregate with an outdated version should raise an exception");
+
+		reloadedAggregate.setVersion(toConcreteNumber.apply(2));
+		assertThatThrownBy(() -> template.save(reloadedAggregate))
+				.hasRootCauseInstanceOf(OptimisticLockingFailureException.class)
+				.withFailMessage("saving an aggregate with a future version should raise an exception");
+	}
+
+	private <T extends Number> void saveAndUpdateAggregateWithPrimitiveVersion(VersionedAggregate aggregate,
+			Function<Number, T> toConcreteNumber) {
+
+		template.save(aggregate);
+
 		VersionedAggregate reloadedAggregate = template.findById(aggregate.getId(), aggregate.getClass());
 		assertThat(reloadedAggregate.getVersion()).isEqualTo(toConcreteNumber.apply(1))
 				.withFailMessage("version field should initially have the value 1");