From 67eeca87ac56314cbae89e21ff999802c69418bf Mon Sep 17 00:00:00 2001 From: Tom Hombergs Date: Fri, 15 Mar 2019 18:12:28 +0100 Subject: [PATCH] DATAJDBC-219 - Add support for optimistic locking. The @Version annotation is now evaluated properly and an OptimisticLockingFailureException is thrown on updates when the version has been incremented in the meantime. The @Version field can be of type Long, Integer or Short (or their primitive counterparts). --- .../jdbc/core/DefaultDataAccessStrategy.java | 74 ++++++++- .../data/jdbc/core/SqlGenerator.java | 26 ++- ...JdbcAggregateTemplateIntegrationTests.java | 155 ++++++++++++++++++ .../data/jdbc/core/SqlGeneratorUnitTests.java | 21 +++ ...AggregateTemplateIntegrationTests-hsql.sql | 1 + ...regateTemplateIntegrationTests-mariadb.sql | 4 +- ...ggregateTemplateIntegrationTests-mssql.sql | 4 +- ...ggregateTemplateIntegrationTests-mysql.sql | 4 +- ...egateTemplateIntegrationTests-postgres.sql | 4 +- 9 files changed, 285 insertions(+), 8 deletions(-) diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/DefaultDataAccessStrategy.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/DefaultDataAccessStrategy.java index 4a868735d1..f5392eae91 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/DefaultDataAccessStrategy.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/DefaultDataAccessStrategy.java @@ -26,9 +26,11 @@ import java.util.Map; import java.util.function.Predicate; +import org.springframework.core.convert.ConversionService; import org.springframework.dao.DataRetrievalFailureException; import org.springframework.dao.EmptyResultDataAccessException; import org.springframework.dao.InvalidDataAccessApiUsageException; +import org.springframework.dao.OptimisticLockingFailureException; import org.springframework.data.jdbc.core.convert.JdbcConverter; import org.springframework.data.jdbc.core.convert.JdbcValue; import org.springframework.data.jdbc.support.JdbcUtil; @@ -36,6 +38,8 @@ import org.springframework.data.mapping.PersistentPropertyAccessor; import org.springframework.data.mapping.PersistentPropertyPath; import org.springframework.data.mapping.PropertyHandler; +import org.springframework.data.mapping.model.ConvertingPropertyAccessor; +import org.springframework.data.relational.core.conversion.RelationalConverter; import org.springframework.data.relational.core.mapping.RelationalMappingContext; import org.springframework.data.relational.core.mapping.RelationalPersistentEntity; import org.springframework.data.relational.core.mapping.RelationalPersistentProperty; @@ -48,6 +52,7 @@ import org.springframework.jdbc.support.KeyHolder; import org.springframework.lang.Nullable; import org.springframework.util.Assert; +import static org.springframework.data.jdbc.core.SqlGenerator.*; /** * The default {@link DataAccessStrategy} is to generate SQL statements based on meta data from the entity. @@ -56,6 +61,7 @@ * @author Mark Paluch * @author Thomas Lang * @author Bastian Wilhelm + * @author Tom Hombergs */ public class DefaultDataAccessStrategy implements DataAccessStrategy { @@ -124,6 +130,12 @@ public Object insert(T instance, Class domainType, Identifier identifier) KeyHolder holder = new GeneratedKeyHolder(); RelationalPersistentEntity persistentEntity = getRequiredPersistentEntity(domainType); + if (persistentEntity.hasVersionProperty()) { + + Number newVersion = getNextVersion(instance, persistentEntity, converter.getConversionService()); + setVersion(instance, persistentEntity, newVersion); + } + MapSqlParameterSource parameterSource = getParameterSource(instance, persistentEntity, "", PersistentProperty::isIdProperty); @@ -151,6 +163,16 @@ public Object insert(T instance, Class domainType, Identifier identifier) */ @Override public boolean update(S instance, Class domainType) { + RelationalPersistentEntity persistentEntity = getRequiredPersistentEntity(domainType); + + if (persistentEntity.hasVersionProperty()) { + return updateWithVersion(instance, domainType); + } else { + return updateWithoutVersion(instance, domainType); + } + } + + private boolean updateWithoutVersion(S instance, Class domainType) { RelationalPersistentEntity persistentEntity = getRequiredPersistentEntity(domainType); @@ -158,6 +180,29 @@ public boolean update(S instance, Class domainType) { getParameterSource(instance, persistentEntity, "", Predicates.includeAll())) != 0; } + private boolean updateWithVersion(S instance, Class domainType) { + + RelationalPersistentEntity persistentEntity = getRequiredPersistentEntity(domainType); + + Number oldVersion = getVersion(instance, persistentEntity, converter.getConversionService()); + Number newVersion = getNextVersion(instance, persistentEntity, converter.getConversionService()); + setVersion(instance, persistentEntity, newVersion); + + MapSqlParameterSource parameterSource = getParameterSource(instance, persistentEntity, "", Predicates.includeAll()); + parameterSource.addValue(VERSION_PARAMETER, oldVersion); + int affectedRows = operations.update(sql(domainType).getUpdateWithVersion(), + parameterSource); + + if (affectedRows == 0) { + // reverting version update on entity + setVersion(instance, persistentEntity, oldVersion); + throw new OptimisticLockingFailureException( + String.format("Optimistic lock exception on saving entity of type %s.", persistentEntity.getName())); + } + + return true; + } + /* * (non-Javadoc) * @see org.springframework.data.jdbc.core.DataAccessStrategy#delete(java.lang.Object, java.lang.Class) @@ -354,7 +399,7 @@ private ID getIdValueOrNull(S instance, RelationalPersistentEntity pe } private static boolean isIdPropertyNullOrScalarZero(@Nullable ID idValue, - RelationalPersistentEntity persistentEntity) { + RelationalPersistentEntity persistentEntity) { RelationalPersistentProperty idProperty = persistentEntity.getIdProperty(); return idValue == null // @@ -481,4 +526,31 @@ static Predicate includeAll() { return it -> false; } } + @Nullable + private Number getVersion(T instance, RelationalPersistentEntity entity, ConversionService conversionService) { + RelationalPersistentProperty versionProperty = entity.getRequiredVersionProperty(); + PersistentPropertyAccessor propertyAccessor = entity.getPropertyAccessor(instance); + ConvertingPropertyAccessor convertingPropertyAccessor = new ConvertingPropertyAccessor<>(propertyAccessor, conversionService); + return convertingPropertyAccessor.getProperty(versionProperty, Number.class); + } + + private Number getNextVersion(T instance, RelationalPersistentEntity entity, ConversionService conversionService) { + Number version = getVersion(instance, entity, conversionService); + Class versionType = entity.getRequiredVersionProperty().getType(); + if (versionType == Integer.class || versionType == int.class) { + return version == null ? 1 : version.intValue() + 1; + } else if (versionType == Long.class || versionType == long.class) { + return version == null ? 1L : version.longValue() + 1; + } else if (versionType == Short.class || versionType == short.class) { + return version == null ? (short) 1 : (short) (version.shortValue() + 1); + } + throw new IllegalStateException(String.format("Entity '%s' has version property of invalid type '%s'.", entity.getType().getName(), entity.getVersionProperty().getType().getName())); + } + + private void setVersion(T instance, RelationalPersistentEntity entity, Number newVersion) { + RelationalPersistentProperty versionProperty = entity.getRequiredVersionProperty(); + PersistentPropertyAccessor accessor = versionProperty.getOwner().getPropertyAccessor(instance); + accessor.setProperty(versionProperty, newVersion); + } + } diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/SqlGenerator.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/SqlGenerator.java index 6f9d23bb22..5e7db405b9 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/SqlGenerator.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/SqlGenerator.java @@ -45,9 +45,12 @@ * @author Yoichi Imai * @author Bastian Wilhelm * @author Oleksandr Kucher + * @author Tom Hombergs */ class SqlGenerator { + static final String VERSION_PARAMETER = "___oldOptimisticLockingVersion"; + private final RelationalPersistentEntity entity; private final RelationalMappingContext context; private final List columnNames = new ArrayList<>(); @@ -62,6 +65,7 @@ class SqlGenerator { private final Lazy countSql = Lazy.of(this::createCountSql); private final Lazy updateSql = Lazy.of(this::createUpdateSql); + private final Lazy updateWithVersionSql = Lazy.of(this::createUpdateWithVersionSql); private final Lazy deleteByIdSql = Lazy.of(this::createDeleteSql); private final Lazy deleteByListSql = Lazy.of(this::createDeleteByListSql); @@ -176,6 +180,10 @@ String getUpdate() { return updateSql.get(); } + String getUpdateWithVersion() { + return updateWithVersionSql.get(); + } + String getCount() { return countSql.get(); } @@ -343,8 +351,8 @@ private String createInsertSql(Set additionalColumns) { String tableColumns = String.join(", ", columnNamesForInsert); String parameterNames = columnNamesForInsert.stream()// - .map(this::columnNameToParameterName) - .map(n -> String.format(":%s", n))// + .map(this::columnNameToParameterName) // + .map(n -> String.format(":%s", n)) // .collect(Collectors.joining(", ")); return String.format(insertTemplate, entity.getTableName(), tableColumns, parameterNames); @@ -369,6 +377,18 @@ private String createUpdateSql() { ); } + private String createUpdateWithVersionSql() { + String whereConditionTemplate = " AND %s = :%s"; + + String whereCondition = String.format( // + whereConditionTemplate, // + entity.getVersionProperty().getColumnName(), // + VERSION_PARAMETER // + ); + + return createUpdateSql() + whereCondition; + } + private String createDeleteSql() { return String.format("DELETE FROM %s WHERE %s = :id", entity.getTableName(), entity.getIdColumn()); } @@ -458,7 +478,7 @@ private String cascadeConditions(String innerCondition, PersistentPropertyPath void saveAndUpdateAggregateWithVersion(VersionedAggregate aggregate, Function 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"); + template.save(reloadedAggregate); + + VersionedAggregate updatedAggregate = template.findById(aggregate.getId(), aggregate.getClass()); + assertThat(updatedAggregate.getVersion()).isEqualTo(toConcreteNumber.apply(2)) + .withFailMessage("version field should increment by one with each save"); + + reloadedAggregate.setVersion(toConcreteNumber.apply(1)); + assertThatThrownBy(() -> template.save(reloadedAggregate)) + .hasRootCauseInstanceOf(OptimisticLockingFailureException.class) + .withFailMessage("saving an aggregate with an outdated version should raise an exception"); + + reloadedAggregate.setVersion(toConcreteNumber.apply(3)); + assertThatThrownBy(() -> template.save(reloadedAggregate)) + .hasRootCauseInstanceOf(OptimisticLockingFailureException.class) + .withFailMessage("saving an aggregate with a future version should raise an exception"); + } + private static void assumeNot(String dbProfileName) { Assume.assumeTrue("true" @@ -522,6 +580,103 @@ static class ElementNoId { private String content; } + @Data + static abstract class VersionedAggregate { + + @Id private Long id; + + abstract Number getVersion(); + + abstract void setVersion(Number newVersion); + } + + @Data + @Table("VERSIONED_AGGREGATE") + static class AggregateWithLongVersion extends VersionedAggregate { + + @Version private Long version; + + @Override + void setVersion(Number newVersion) { + this.version = (Long) newVersion; + } + } + + @Table("VERSIONED_AGGREGATE") + static class AggregateWithPrimitiveLongVersion extends VersionedAggregate { + + @Version private long version; + + @Override + void setVersion(Number newVersion) { + this.version = (long) newVersion; + } + + @Override + Number getVersion(){ + return this.version; + } + } + + @Data + @Table("VERSIONED_AGGREGATE") + static class AggregateWithIntegerVersion extends VersionedAggregate { + + @Version private Integer version; + + @Override + void setVersion(Number newVersion) { + this.version = (Integer) newVersion; + } + } + + @Table("VERSIONED_AGGREGATE") + static class AggregateWithPrimitiveIntegerVersion extends VersionedAggregate { + + @Version private int version; + + @Override + void setVersion(Number newVersion) { + this.version = (int) newVersion; + } + + @Override + Number getVersion(){ + return this.version; + } + } + + @Data + @Table("VERSIONED_AGGREGATE") + static class AggregateWithShortVersion extends VersionedAggregate { + + @Version private Short version; + + @Override + void setVersion(Number newVersion) { + this.version = (Short) newVersion; + } + } + + @Table("VERSIONED_AGGREGATE") + static class AggregateWithPrimitiveShortVersion extends VersionedAggregate { + + @Version + private short version; + + @Override + void setVersion(Number newVersion) { + this.version = (short) newVersion; + } + + @Override + Number getVersion(){ + return this.version; + } + + } + + @Configuration @Import(TestConfiguration.class) static class Config { diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/SqlGeneratorUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/SqlGeneratorUnitTests.java index 3e082a8e0d..5d0f646e02 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/SqlGeneratorUnitTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/SqlGeneratorUnitTests.java @@ -26,6 +26,7 @@ import org.junit.Test; import org.springframework.data.annotation.Id; import org.springframework.data.annotation.ReadOnlyProperty; +import org.springframework.data.annotation.Version; import org.springframework.data.jdbc.core.mapping.AggregateReference; import org.springframework.data.jdbc.core.mapping.JdbcMappingContext; import org.springframework.data.jdbc.core.mapping.PersistentPropertyPathTestUtils; @@ -43,6 +44,7 @@ * @author Greg Turnquist * @author Oleksandr Kucher * @author Bastian Wilhelm + * @author Tom Hombergs */ public class SqlGeneratorUnitTests { @@ -241,6 +243,21 @@ public void update() { "id1 = :id"); } + @Test // DATAJDBC-219 + public void updateWithVersion() { + + SqlGenerator sqlGenerator = createSqlGenerator(VersionedEntity.class); + + assertThat(sqlGenerator.getUpdateWithVersion()).containsSequence( // + "UPDATE", // + "versioned_entity", // + "SET", // + "WHERE", // + "id1 = :id", // + "AND", // + "version = :___oldOptimisticLockingVersion"); + } + @Test // DATAJDBC-324 public void readOnlyPropertyExcludedFromQuery_when_generateUpdateSql() { @@ -350,6 +367,10 @@ static class DummyEntity { AggregateReference other; } + static class VersionedEntity extends DummyEntity { + @Version Integer version; + } + @SuppressWarnings("unused") static class ReferencedEntity { diff --git a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-hsql.sql b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-hsql.sql index 1699ad6c50..dbb0b9d928 100644 --- a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-hsql.sql +++ b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-hsql.sql @@ -18,3 +18,4 @@ CREATE TABLE ARRAY_OWNER (ID BIGINT GENERATED BY DEFAULT AS IDENTITY (START WITH CREATE TABLE BYTE_ARRAY_OWNER (ID BIGINT GENERATED BY DEFAULT AS IDENTITY (START WITH 1) PRIMARY KEY, BINARY_DATA VARBINARY(20) NOT NULL); +CREATE TABLE VERSIONED_AGGREGATE (ID BIGINT GENERATED BY DEFAULT AS IDENTITY (START WITH 1) PRIMARY KEY, VERSION BIGINT); \ No newline at end of file diff --git a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-mariadb.sql b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-mariadb.sql index 38957bcc61..7cad9fa58b 100644 --- a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-mariadb.sql +++ b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-mariadb.sql @@ -10,4 +10,6 @@ CREATE TABLE Child_No_Id (ONE_TO_ONE_PARENT INTEGER PRIMARY KEY, content VARCHAR CREATE TABLE LIST_PARENT ( id4 BIGINT AUTO_INCREMENT PRIMARY KEY, NAME VARCHAR(100)); CREATE TABLE element_no_id ( content VARCHAR(100), LIST_PARENT_key BIGINT, LIST_PARENT BIGINT); -CREATE TABLE BYTE_ARRAY_OWNER (ID BIGINT AUTO_INCREMENT PRIMARY KEY, BINARY_DATA VARBINARY(20) NOT NULL) \ No newline at end of file +CREATE TABLE BYTE_ARRAY_OWNER (ID BIGINT AUTO_INCREMENT PRIMARY KEY, BINARY_DATA VARBINARY(20) NOT NULL) + +CREATE TABLE VERSIONED_AGGREGATE (ID BIGINT AUTO_INCREMENT PRIMARY KEY, VERSION BIGINT); \ No newline at end of file diff --git a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-mssql.sql b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-mssql.sql index b507634c35..800ab91361 100644 --- a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-mssql.sql +++ b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-mssql.sql @@ -15,4 +15,6 @@ CREATE TABLE LIST_PARENT ( id4 BIGINT IDENTITY PRIMARY KEY, NAME VARCHAR(100)); CREATE TABLE element_no_id ( content VARCHAR(100), LIST_PARENT_key BIGINT, LIST_PARENT BIGINT); DROP TABLE IF EXISTS BYTE_ARRAY_OWNER; -CREATE TABLE BYTE_ARRAY_OWNER (ID BIGINT IDENTITY PRIMARY KEY, BINARY_DATA VARBINARY(20) NOT NULL) \ No newline at end of file +CREATE TABLE BYTE_ARRAY_OWNER (ID BIGINT IDENTITY PRIMARY KEY, BINARY_DATA VARBINARY(20) NOT NULL) + +CREATE TABLE VERSIONED_AGGREGATE (ID BIGINT IDENTITY PRIMARY KEY, VERSION BIGINT); \ No newline at end of file diff --git a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-mysql.sql b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-mysql.sql index 38957bcc61..7cad9fa58b 100644 --- a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-mysql.sql +++ b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-mysql.sql @@ -10,4 +10,6 @@ CREATE TABLE Child_No_Id (ONE_TO_ONE_PARENT INTEGER PRIMARY KEY, content VARCHAR CREATE TABLE LIST_PARENT ( id4 BIGINT AUTO_INCREMENT PRIMARY KEY, NAME VARCHAR(100)); CREATE TABLE element_no_id ( content VARCHAR(100), LIST_PARENT_key BIGINT, LIST_PARENT BIGINT); -CREATE TABLE BYTE_ARRAY_OWNER (ID BIGINT AUTO_INCREMENT PRIMARY KEY, BINARY_DATA VARBINARY(20) NOT NULL) \ No newline at end of file +CREATE TABLE BYTE_ARRAY_OWNER (ID BIGINT AUTO_INCREMENT PRIMARY KEY, BINARY_DATA VARBINARY(20) NOT NULL) + +CREATE TABLE VERSIONED_AGGREGATE (ID BIGINT AUTO_INCREMENT PRIMARY KEY, VERSION BIGINT); \ No newline at end of file diff --git a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-postgres.sql b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-postgres.sql index 4f39362347..01448ec205 100644 --- a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-postgres.sql +++ b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-postgres.sql @@ -21,4 +21,6 @@ CREATE TABLE element_no_id ( content VARCHAR(100), LIST_PARENT_key BIGINT, LIST_ CREATE TABLE ARRAY_OWNER (ID SERIAL PRIMARY KEY, DIGITS VARCHAR(20)[10], MULTIDIMENSIONAL VARCHAR(20)[10][10]); -CREATE TABLE BYTE_ARRAY_OWNER (ID SERIAL PRIMARY KEY, BINARY_DATA BYTEA NOT NULL) \ No newline at end of file +CREATE TABLE BYTE_ARRAY_OWNER (ID SERIAL PRIMARY KEY, BINARY_DATA BYTEA NOT NULL) + +CREATE TABLE VERSIONED_AGGREGATE (ID SERIAL PRIMARY KEY, VERSION BIGINT); \ No newline at end of file