From ca2d2182d2030f5b486b131d2ae56dbe8a51bb8f Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Fri, 29 Nov 2019 13:45:50 +0100 Subject: [PATCH] DATAJDBC-219 - Polishing. DeleteWithVersion doesn't require an entity anymore. Added the `@author` and `@since` tags where they were missing. Formatting. Added documentation. Original pull request: #166. --- .../jdbc/core/AggregateChangeExecutor.java | 9 ++- .../jdbc/core/DefaultJdbcInterpreter.java | 54 +++++++------- .../convert/CascadingDataAccessStrategy.java | 5 +- .../jdbc/core/convert/DataAccessStrategy.java | 10 ++- .../convert/DefaultDataAccessStrategy.java | 16 ++--- .../convert/DelegatingDataAccessStrategy.java | 7 +- .../data/jdbc/core/convert/SqlContext.java | 6 +- .../data/jdbc/core/convert/SqlGenerator.java | 72 ++++++------------- .../mybatis/MyBatisDataAccessStrategy.java | 8 +-- .../core/DefaultJdbcInterpreterUnitTests.java | 1 + ...JdbcAggregateTemplateIntegrationTests.java | 51 +++++++------ .../core/convert/SqlGeneratorUnitTests.java | 13 ++-- ...tomizingNamespaceHsqlIntegrationTests.java | 1 + .../MyBatisDataAccessStrategyUnitTests.java | 1 + .../relational/core/conversion/DbAction.java | 29 ++++---- .../RelationalEntityDeleteWriter.java | 23 +++++- .../RelationalEntityVersionUtils.java | 20 +++++- src/main/asciidoc/jdbc.adoc | 15 ++++ src/main/asciidoc/new-features.adoc | 6 ++ 19 files changed, 190 insertions(+), 157 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 c394d0dc..4687cd27 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 @@ -80,7 +80,6 @@ class AggregateChangeExecutor { } @SuppressWarnings("unchecked") - @Nullable private T populateRootVersionIfNecessary(T newRoot, List> actions) { // Does the root entity have a version attribute? @@ -98,12 +97,13 @@ class AggregateChangeExecutor { // This really should never happen. return newRoot; } - DbAction.WithVersion versionAction = (DbAction.WithVersion) rootAction.get(); + DbAction.WithVersion versionAction = (DbAction.WithVersion) rootAction.get(); - return RelationalEntityVersionUtils.setVersionNumberOnEntity(newRoot, - versionAction.getNextVersion(), persistentEntity, converter); + return RelationalEntityVersionUtils.setVersionNumberOnEntity(newRoot, versionAction.getNextVersion(), + persistentEntity, converter); } + @SuppressWarnings("unchecked") @Nullable private T populateIdsIfNecessary(List> actions) { @@ -145,7 +145,6 @@ class AggregateChangeExecutor { return newRoot; } - @SuppressWarnings("unchecked") private Object setIdAndCascadingProperties(DbAction.WithGeneratedId action, @Nullable Object generatedId, AggregateChangeExecutor.StagedValues cascadingValues) { diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/DefaultJdbcInterpreter.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/DefaultJdbcInterpreter.java index 5d7ed699..8b1c3ebe 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/DefaultJdbcInterpreter.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/DefaultJdbcInterpreter.java @@ -20,6 +20,7 @@ import lombok.RequiredArgsConstructor; import java.util.Map; import org.springframework.dao.IncorrectUpdateSemanticsDataAccessException; +import org.springframework.dao.OptimisticLockingFailureException; import org.springframework.data.jdbc.core.convert.DataAccessStrategy; import org.springframework.data.mapping.PersistentPropertyPath; import org.springframework.data.relational.core.conversion.DbAction; @@ -55,6 +56,7 @@ import org.springframework.util.Assert; class DefaultJdbcInterpreter implements Interpreter { public static final String UPDATE_FAILED = "Failed to update entity [%s]. Id [%s] not found in database."; + public static final String UPDATE_FAILED_OPTIMISTIC_LOCKING = "Failed to update entity [%s]. The entity was updated since it was rea or it isn't in the database at all."; private final RelationalConverter converter; private final RelationalMappingContext context; private final DataAccessStrategy accessStrategy; @@ -65,15 +67,11 @@ class DefaultJdbcInterpreter implements Interpreter { */ @Override public void interpret(Insert insert) { + Object id = accessStrategy.insert(insert.getEntity(), insert.getEntityType(), getParentKeys(insert)); insert.setGeneratedId(id); } - @SuppressWarnings("unchecked") - private RelationalPersistentEntity getRequiredPersistentEntity(Class type) { - return (RelationalPersistentEntity) context.getRequiredPersistentEntity(type); - } - /* * (non-Javadoc) * @see org.springframework.data.relational.core.conversion.Interpreter#interpret(org.springframework.data.relational.core.conversion.DbAction.InsertRoot) @@ -83,22 +81,17 @@ class DefaultJdbcInterpreter implements Interpreter { RelationalPersistentEntity persistentEntity = getRequiredPersistentEntity(insert.getEntityType()); + Object id; if (persistentEntity.hasVersionProperty()) { - // The interpreter is responsible for setting the initial version on the entity prior to calling insert. - Number version = RelationalEntityVersionUtils.getVersionNumberFromEntity(insert.getEntity(), persistentEntity, - converter); - if (version != null && version.longValue() > 0) { - throw new IllegalArgumentException("The entity cannot be inserted because it already has a version."); - } + T rootEntity = RelationalEntityVersionUtils.setVersionNumberOnEntity(insert.getEntity(), 1, persistentEntity, converter); - Object id = accessStrategy.insert(rootEntity, insert.getEntityType(), Identifier.empty()); + id = accessStrategy.insert(rootEntity, insert.getEntityType(), Identifier.empty()); insert.setNextVersion(1); - insert.setGeneratedId(id); } else { - Object id = accessStrategy.insert(insert.getEntity(), insert.getEntityType(), Identifier.empty()); - insert.setGeneratedId(id); + id = accessStrategy.insert(insert.getEntity(), insert.getEntityType(), Identifier.empty()); } + insert.setGeneratedId(id); } /* @@ -125,23 +118,25 @@ class DefaultJdbcInterpreter implements Interpreter { RelationalPersistentEntity persistentEntity = getRequiredPersistentEntity(update.getEntityType()); if (persistentEntity.hasVersionProperty()) { + // If the root aggregate has a version property, increment it. Number previousVersion = RelationalEntityVersionUtils.getVersionNumberFromEntity(update.getEntity(), persistentEntity, converter); + Assert.notNull(previousVersion, "The root aggregate cannot be updated because the version property is null."); T rootEntity = RelationalEntityVersionUtils.setVersionNumberOnEntity(update.getEntity(), - previousVersion.longValue() + 1, persistentEntity, - converter); + previousVersion.longValue() + 1, persistentEntity, converter); - if (accessStrategy.updateWithVersion(rootEntity, update.getEntityType(), previousVersion)) { - // Successful update, set the in-memory version on the action. - update.setNextVersion(previousVersion); - } else { - throw new IncorrectUpdateSemanticsDataAccessException( - String.format(UPDATE_FAILED, update.getEntity(), getIdFrom(update))); - } + if (accessStrategy.updateWithVersion(rootEntity, update.getEntityType(), previousVersion)) { + // Successful update, set the in-memory version on the action. + update.setNextVersion(previousVersion); + } else { + throw new OptimisticLockingFailureException( + String.format(UPDATE_FAILED_OPTIMISTIC_LOCKING, update.getEntity())); + } } else { + if (!accessStrategy.update(update.getEntity(), update.getEntityType())) { throw new IncorrectUpdateSemanticsDataAccessException( @@ -179,10 +174,12 @@ class DefaultJdbcInterpreter implements Interpreter { @Override public void interpret(DeleteRoot delete) { - if (delete.getEntity() != null) { + if (delete.getPreviousVersion() != null) { + RelationalPersistentEntity persistentEntity = getRequiredPersistentEntity(delete.getEntityType()); if (persistentEntity.hasVersionProperty()) { - accessStrategy.deleteWithVersion(delete.getEntity(), delete.getEntityType()); + + accessStrategy.deleteWithVersion(delete.getId(), delete.getEntityType(), delete.getPreviousVersion()); return; } } @@ -274,4 +271,9 @@ class DefaultJdbcInterpreter implements Interpreter { return identifier; } + @SuppressWarnings("unchecked") + private RelationalPersistentEntity getRequiredPersistentEntity(Class type) { + return (RelationalPersistentEntity) context.getRequiredPersistentEntity(type); + } + } diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/CascadingDataAccessStrategy.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/CascadingDataAccessStrategy.java index 782face5..53ae324e 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/CascadingDataAccessStrategy.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/CascadingDataAccessStrategy.java @@ -31,6 +31,7 @@ import org.springframework.data.relational.domain.Identifier; * * @author Jens Schauder * @author Mark Paluch + * @author Tyler Van Gorder * @since 1.1 */ public class CascadingDataAccessStrategy implements DataAccessStrategy { @@ -91,8 +92,8 @@ public class CascadingDataAccessStrategy implements DataAccessStrategy { * @see org.springframework.data.jdbc.core.DataAccessStrategy#deleteInstance(java.lang.Object, java.lang.Class) */ @Override - public void deleteWithVersion(T instance, Class domainType) { - collectVoid(das -> das.deleteWithVersion(instance, domainType)); + public void deleteWithVersion(Object id, Class domainType, Number previousVersion) { + collectVoid(das -> das.deleteWithVersion(id, domainType, previousVersion)); } /* diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DataAccessStrategy.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DataAccessStrategy.java index e22baaf8..4284aa4d 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DataAccessStrategy.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DataAccessStrategy.java @@ -17,6 +17,7 @@ package org.springframework.data.jdbc.core.convert; import java.util.Map; +import org.springframework.dao.OptimisticLockingFailureException; import org.springframework.data.jdbc.core.JdbcAggregateOperations; import org.springframework.data.mapping.PersistentPropertyPath; import org.springframework.data.relational.core.mapping.RelationalPersistentProperty; @@ -75,7 +76,7 @@ public interface DataAccessStrategy extends RelationResolver { boolean update(T instance, Class domainType); /** - * Updates the data of a single entity in the database and enforce optimistic record locking using the previousVersion + * Updates the data of a single entity in the database and enforce optimistic record locking using the {@code previousVersion} * property. Referenced entities don't get handled. *

* The statement will be of the form : {@code UPDATE … SET … WHERE ID = :id and VERSION_COLUMN = :previousVersion } @@ -86,6 +87,8 @@ public interface DataAccessStrategy extends RelationResolver { * @param previousVersion The previous version assigned to the instance being saved. * @param the type of the instance to save. * @return whether the update actually updated a row. + * @throws OptimisticLockingFailureException if the update fails to update at least one row assuming the the optimistic locking version check failed. + * @since 2.0 */ boolean updateWithVersion(T instance, Class domainType, Number previousVersion); @@ -109,8 +112,11 @@ public interface DataAccessStrategy extends RelationResolver { * @param id the id of the row to be deleted. Must not be {@code null}. * @param domainType the type of entity to be deleted. Implicitly determines the table to operate on. Must not be * {@code null}. + * @param previousVersion The previous version assigned to the instance being saved. + * @throws OptimisticLockingFailureException if the update fails to update at least one row assuming the the optimistic locking version check failed. + * @since 2.0 */ - void deleteWithVersion(T instance, Class domainType); + void deleteWithVersion(Object id, Class domainType, Number previousVersion); /** * Deletes all entities reachable via {@literal propertyPath} from the instance identified by {@literal rootId}. diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DefaultDataAccessStrategy.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DefaultDataAccessStrategy.java index 7237c179..04ae24ea 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DefaultDataAccessStrategy.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DefaultDataAccessStrategy.java @@ -184,23 +184,17 @@ public class DefaultDataAccessStrategy implements DataAccessStrategy { /* * (non-Javadoc) - * @see org.springframework.data.jdbc.core.DataAccessStrategy#deleteInstance(java.lang.Object, java.lang.Class) + * @see org.springframework.data.jdbc.core.DataAccessStrategy#deleteInstance(java.lang.Object, java.lang.Class, java.lang.Number) */ @Override - public void deleteWithVersion(T instance, Class domainType) { + public void deleteWithVersion(Object id, Class domainType, Number previousVersion) { + + Assert.notNull(id, "Id must not be null."); RelationalPersistentEntity persistentEntity = getRequiredPersistentEntity(domainType); - Object id = getIdValueOrNull(instance, persistentEntity); - Assert.notNull(id, "Cannot delete an instance without it's ID being populated."); - if (!persistentEntity.hasVersionProperty()) { - delete(id, domainType); - return; - } - - Number oldVersion = RelationalEntityVersionUtils.getVersionNumberFromEntity(instance, persistentEntity, converter); MapSqlParameterSource parameterSource = createIdParameterSource(id, domainType); - parameterSource.addValue(VERSION_SQL_PARAMETER_NAME, oldVersion); + parameterSource.addValue(VERSION_SQL_PARAMETER_NAME, previousVersion); int affectedRows = operations.update(sql(domainType).getDeleteByIdAndVersion(), parameterSource); if (affectedRows == 0) { diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DelegatingDataAccessStrategy.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DelegatingDataAccessStrategy.java index 5f89080a..7996ac4b 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DelegatingDataAccessStrategy.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DelegatingDataAccessStrategy.java @@ -27,6 +27,7 @@ import org.springframework.util.Assert; * cyclic dependencies. * * @author Jens Schauder + * @author Tyler Van Gorder * @since 1.1 */ public class DelegatingDataAccessStrategy implements DataAccessStrategy { @@ -89,11 +90,11 @@ public class DelegatingDataAccessStrategy implements DataAccessStrategy { /* * (non-Javadoc) - * @see org.springframework.data.jdbc.core.DataAccessStrategy#deleteWithVersion(java.lang.Object, java.lang.Class) + * @see org.springframework.data.jdbc.core.DataAccessStrategy#deleteWithVersion(java.lang.Object, java.lang.Class, Number) */ @Override - public void deleteWithVersion(T instance, Class domainType) { - delegate.deleteWithVersion(instance, domainType); + public void deleteWithVersion(Object id, Class domainType, Number previousVersion) { + delegate.deleteWithVersion(id, domainType, previousVersion); } /* diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlContext.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlContext.java index cecd3432..005da109 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlContext.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlContext.java @@ -26,6 +26,7 @@ import org.springframework.data.relational.core.sql.Table; * * @author Jens Schauder * @author Mark Paluch + * @author Tyler Van Gorder * @since 1.1 */ class SqlContext { @@ -43,10 +44,7 @@ class SqlContext { } Column getVersionColumn() { - if (!entity.hasVersionProperty()) { - return null; - } - return table.column(entity.getVersionProperty().getColumnName()); + return table.column(entity.getRequiredVersionProperty().getColumnName()); } Table getTable() { diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlGenerator.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlGenerator.java index 8d7bd741..3dc1bcd5 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlGenerator.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlGenerator.java @@ -37,24 +37,7 @@ import org.springframework.data.relational.core.mapping.PersistentPropertyPathEx 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.sql.AssignValue; -import org.springframework.data.relational.core.sql.Assignments; -import org.springframework.data.relational.core.sql.BindMarker; -import org.springframework.data.relational.core.sql.Column; -import org.springframework.data.relational.core.sql.Condition; -import org.springframework.data.relational.core.sql.Delete; -import org.springframework.data.relational.core.sql.DeleteBuilder; -import org.springframework.data.relational.core.sql.Expression; -import org.springframework.data.relational.core.sql.Expressions; -import org.springframework.data.relational.core.sql.Functions; -import org.springframework.data.relational.core.sql.Insert; -import org.springframework.data.relational.core.sql.InsertBuilder; -import org.springframework.data.relational.core.sql.SQL; -import org.springframework.data.relational.core.sql.Select; -import org.springframework.data.relational.core.sql.SelectBuilder; -import org.springframework.data.relational.core.sql.StatementBuilder; -import org.springframework.data.relational.core.sql.Table; -import org.springframework.data.relational.core.sql.Update; +import org.springframework.data.relational.core.sql.*; import org.springframework.data.relational.core.sql.render.SqlRenderer; import org.springframework.data.relational.domain.Identifier; import org.springframework.data.util.Lazy; @@ -501,25 +484,20 @@ class SqlGenerator { } private String createUpdateSql() { - Table table = getTable(); - - List assignments = columns.getUpdateableColumns() // - .stream() // - .map(columnName -> Assignments.value( // - table.column(columnName), // - getBindMarker(columnName))) // - .collect(Collectors.toList()); - - Update update = Update.builder() // - .table(table) // - .set(assignments) // - .where(getIdColumn().isEqualTo(getBindMarker(entity.getIdColumn()))).build(); - - return render(update); + return render(createBaseUpdate().build()); } private String createUpdateWithVersionSql() { + Update update = createBaseUpdate() // + .and(getVersionColumn().isEqualTo(SQL.bindMarker(":" + VERSION_SQL_PARAMETER_NAME))) // + .build(); + + return render(update); + } + + private UpdateBuilder.UpdateWhereAndOr createBaseUpdate() { + Table table = getTable(); List assignments = columns.getUpdateableColumns() // @@ -529,37 +507,27 @@ class SqlGenerator { getBindMarker(columnName))) // .collect(Collectors.toList()); - Update update = null; - update = Update.builder() // + return Update.builder() // .table(table) // .set(assignments) // - .where(getIdColumn().isEqualTo(getBindMarker(entity.getIdColumn()))) // - .and(getVersionColumn().isEqualTo(SQL.bindMarker(":" + VERSION_SQL_PARAMETER_NAME))) // - .build(); - - return render(update); + .where(getIdColumn().isEqualTo(getBindMarker(entity.getIdColumn()))); } private String createDeleteSql() { + return render(createBaseDeleteById(getTable()).build()); + } - Table table = getTable(); + private String createDeleteByIdAndVersionSql() { - Delete delete = Delete.builder().from(table).where(getIdColumn().isEqualTo(SQL.bindMarker(":id"))) // + Delete delete = createBaseDeleteById(getTable()) // + .and(getVersionColumn().isEqualTo(SQL.bindMarker(":" + VERSION_SQL_PARAMETER_NAME))) // .build(); return render(delete); } - private String createDeleteByIdAndVersionSql() { - Table table = getTable(); - - Delete delete = Delete.builder() // - .from(table) // - .where(getIdColumn().isEqualTo(SQL.bindMarker(":id"))) // - .and(getVersionColumn().isEqualTo(SQL.bindMarker(":" + VERSION_SQL_PARAMETER_NAME))) // - .build(); - - return render(delete); + private DeleteBuilder.DeleteWhereAndOr createBaseDeleteById(Table table) { + return Delete.builder().from(table).where(getIdColumn().isEqualTo(SQL.bindMarker(":id"))); } private String createDeleteByPathAndCriteria(PersistentPropertyPathExtension path, diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/mybatis/MyBatisDataAccessStrategy.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/mybatis/MyBatisDataAccessStrategy.java index d10f00ad..c798a6e8 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/mybatis/MyBatisDataAccessStrategy.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/mybatis/MyBatisDataAccessStrategy.java @@ -174,11 +174,9 @@ public class MyBatisDataAccessStrategy implements DataAccessStrategy { @Override public boolean updateWithVersion(S instance, Class domainType, Number previousVersion) { - Map additionalValues = new HashMap<>(); - additionalValues.put(VERSION_SQL_PARAMETER_NAME_OLD, previousVersion); return sqlSession().update(namespace(domainType) + ".updateWithVersion", - new MyBatisContext(null, instance, domainType, additionalValues)) != 0; + new MyBatisContext(null, instance, domainType, Collections.singletonMap(VERSION_SQL_PARAMETER_NAME_OLD, previousVersion))) != 0; } /* @@ -197,10 +195,10 @@ public class MyBatisDataAccessStrategy implements DataAccessStrategy { * @see org.springframework.data.jdbc.core.DataAccessStrategy#deleteInstance(java.lang.Object, java.lang.Class) */ @Override - public void deleteWithVersion(T instance, Class domainType) { + public void deleteWithVersion(Object id, Class domainType, Number previousVersion) { sqlSession().delete(namespace(domainType) + ".deleteWithVersion", - new MyBatisContext(null, instance, domainType, Collections.emptyMap())); + new MyBatisContext(id, null, domainType, Collections.singletonMap(VERSION_SQL_PARAMETER_NAME_OLD, previousVersion))); } /* diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/DefaultJdbcInterpreterUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/DefaultJdbcInterpreterUnitTests.java index 7b423600..542b0c93 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/DefaultJdbcInterpreterUnitTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/DefaultJdbcInterpreterUnitTests.java @@ -48,6 +48,7 @@ import org.springframework.data.relational.domain.Identifier; * @author Jens Schauder * @author Mark Paluch * @author Myeonghyeon Lee + * @author Tyler Van Gorder */ public class DefaultJdbcInterpreterUnitTests { 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 f1168f2e..1f04951a 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 @@ -15,12 +15,8 @@ */ package org.springframework.data.jdbc.core; -import static java.util.Collections.emptyMap; -import static java.util.Collections.singletonList; -import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatExceptionOfType; -import static org.assertj.core.api.Assertions.assertThatThrownBy; -import static org.assertj.core.api.Assertions.tuple; +import static java.util.Collections.*; +import static org.assertj.core.api.Assertions.*; import lombok.Data; import lombok.EqualsAndHashCode; @@ -642,27 +638,29 @@ public class JdbcAggregateTemplateIntegrationTests { @Test // DATAJDBC-219 Test that immutable version attribute works as expected. public void saveAndUpdateAggregateWithImmutableVersion() { + AggregateWithImmutableVersion aggregate = new AggregateWithImmutableVersion(null, null); aggregate = template.save(aggregate); Long id = aggregate.getId(); AggregateWithImmutableVersion reloadedAggregate = template.findById(id, aggregate.getClass()); - assertThat(reloadedAggregate.getVersion()).isEqualTo(1L) - .withFailMessage("version field should initially have the value 1"); - reloadedAggregate = template.save(reloadedAggregate); + assertThat(reloadedAggregate.getVersion()).describedAs("version field should initially have the value 1") + .isEqualTo(1L); + + template.save(reloadedAggregate); AggregateWithImmutableVersion updatedAggregate = template.findById(id, aggregate.getClass()); - assertThat(updatedAggregate.getVersion()).isEqualTo(2L) - .withFailMessage("version field should increment by one with each save"); + assertThat(updatedAggregate.getVersion()).describedAs("version field should increment by one with each save") + .isEqualTo(2L); assertThatThrownBy(() -> template.save(new AggregateWithImmutableVersion(id, 1L))) - .hasRootCauseInstanceOf(OptimisticLockingFailureException.class) - .withFailMessage("saving an aggregate with an outdated version should raise an exception"); + .describedAs("saving an aggregate with an outdated version should raise an exception") + .hasRootCauseInstanceOf(OptimisticLockingFailureException.class); assertThatThrownBy(() -> template.save(new AggregateWithImmutableVersion(id, 3L))) - .hasRootCauseInstanceOf(OptimisticLockingFailureException.class) - .withFailMessage("saving an aggregate with a future version should raise an exception"); + .describedAs("saving an aggregate with a future version should raise an exception") + .hasRootCauseInstanceOf(OptimisticLockingFailureException.class); } @Test // DATAJDBC-219 Test that a delete with a version attribute works as expected. @@ -671,26 +669,26 @@ public class JdbcAggregateTemplateIntegrationTests { AggregateWithImmutableVersion aggregate = new AggregateWithImmutableVersion(null, null); aggregate = template.save(aggregate); - //Should have an ID and a version of 1. + // Should have an ID and a version of 1. final Long id = aggregate.getId(); - assertThatThrownBy(() -> template.delete(new AggregateWithImmutableVersion(id, 0L), AggregateWithImmutableVersion.class)) - .hasRootCauseInstanceOf(OptimisticLockingFailureException.class) - .withFailMessage("deleting an aggregate with an outdated version should raise an exception"); + assertThatThrownBy( + () -> template.delete(new AggregateWithImmutableVersion(id, 0L), AggregateWithImmutableVersion.class)) + .describedAs("deleting an aggregate with an outdated version should raise an exception") + .hasRootCauseInstanceOf(OptimisticLockingFailureException.class); - assertThatThrownBy(() -> template.delete(new AggregateWithImmutableVersion(id, 3L), AggregateWithImmutableVersion.class)) - .hasRootCauseInstanceOf(OptimisticLockingFailureException.class) - .withFailMessage("deleting an aggregate with a future version should raise an exception"); + assertThatThrownBy( + () -> template.delete(new AggregateWithImmutableVersion(id, 3L), AggregateWithImmutableVersion.class)) + .describedAs("deleting an aggregate with a future version should raise an exception") + .hasRootCauseInstanceOf(OptimisticLockingFailureException.class); - - //This should succeed + // This should succeed template.delete(aggregate, AggregateWithImmutableVersion.class); - aggregate = new AggregateWithImmutableVersion(null, null); aggregate = template.save(aggregate); - //This should succeed, as version will not be used. + // This should succeed, as version will not be used. template.deleteById(aggregate.getId(), AggregateWithImmutableVersion.class); } @@ -1013,6 +1011,7 @@ public class JdbcAggregateTemplateIntegrationTests { String name; @ReadOnlyProperty String readOnly; } + @Data static abstract class VersionedAggregate { diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorUnitTests.java index d85d7df1..71578b5f 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorUnitTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorUnitTests.java @@ -15,8 +15,8 @@ */ package org.springframework.data.jdbc.core.convert; -import static java.util.Collections.emptySet; -import static org.assertj.core.api.Assertions.assertThat; +import static java.util.Collections.*; +import static org.assertj.core.api.Assertions.*; import java.util.Map; import java.util.Set; @@ -174,9 +174,9 @@ public class SqlGeneratorUnitTests { public void findAllByPropertyWithMultipartIdentifier() { // this would get called when ListParent is the element type of a Set - String sql = sqlGenerator.getFindAllByProperty( - Identifier.of("backref", "some-value", String.class).withPart("backref_key", "key-value", Object.class), null, - false); + Identifier parentIdentifier = Identifier.of("backref", "some-value", String.class) // + .withPart("backref_key", "key-value", Object.class); + String sql = sqlGenerator.getFindAllByProperty(parentIdentifier, null, false); assertThat(sql).contains("SELECT", // "dummy_entity.id1 AS id1", // @@ -189,7 +189,8 @@ public class SqlGeneratorUnitTests { "FROM dummy_entity ", // "LEFT OUTER JOIN referenced_entity AS ref ON ref.dummy_entity = dummy_entity.id1", // "LEFT OUTER JOIN second_level_referenced_entity AS ref_further ON ref_further.referenced_entity = ref.x_l1id", // - "dummy_entity.backref = :backref", "dummy_entity.backref_key = :backref_key"); + "dummy_entity.backref = :backref", // + "dummy_entity.backref_key = :backref_key"); } @Test // DATAJDBC-131, DATAJDBC-111 diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/mybatis/MyBatisCustomizingNamespaceHsqlIntegrationTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/mybatis/MyBatisCustomizingNamespaceHsqlIntegrationTests.java index 81dcd040..62e41dd9 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/mybatis/MyBatisCustomizingNamespaceHsqlIntegrationTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/mybatis/MyBatisCustomizingNamespaceHsqlIntegrationTests.java @@ -53,6 +53,7 @@ import org.springframework.transaction.annotation.Transactional; * * @author Kazuki Shimizu * @author Jens Schauder + * @author Tyler Van Gorder */ @ContextConfiguration @ActiveProfiles("hsql") diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/mybatis/MyBatisDataAccessStrategyUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/mybatis/MyBatisDataAccessStrategyUnitTests.java index f7431d12..8ad76012 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/mybatis/MyBatisDataAccessStrategyUnitTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/mybatis/MyBatisDataAccessStrategyUnitTests.java @@ -50,6 +50,7 @@ import org.springframework.data.relational.domain.Identifier; * * @author Jens Schauder * @author Mark Paluch + * @author Tyler Van Gorder */ public class MyBatisDataAccessStrategyUnitTests { 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 139a83cf..97e5fdbf 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 @@ -15,6 +15,7 @@ */ package org.springframework.data.relational.core.conversion; +import lombok.AllArgsConstructor; import lombok.Data; import lombok.NonNull; import lombok.RequiredArgsConstructor; @@ -93,15 +94,16 @@ public interface DbAction { } /** - * Represents an insert statement for the root of an aggregate. Upon a successful insert, the initial version and generated ids are populated. + * Represents an insert statement for the root of an aggregate. Upon a successful insert, the initial version and + * generated ids are populated. * * @param type of the entity for which this represents a database interaction. */ @Data @RequiredArgsConstructor - class InsertRoot implements WithVersion, WithGeneratedId { + class InsertRoot implements WithVersion, WithGeneratedId { - @NonNull private T entity; + @NonNull final T entity; private Number nextVersion; private Object generatedId; @@ -134,9 +136,9 @@ public interface DbAction { * @param type of the entity for which this represents a database interaction. */ @Data - class UpdateRoot implements WithVersion { + class UpdateRoot implements WithEntity, WithVersion { - @NonNull private T entity; + @NonNull final T entity; @Nullable Number nextVersion; @Override @@ -150,7 +152,7 @@ public interface DbAction { * * @param type of the entity for which this represents a database interaction. */ - @Data + @Value class Merge implements WithDependingOn, WithPropertyPath { @NonNull T entity; @@ -191,11 +193,11 @@ public interface DbAction { * @param type of the entity for which this represents a database interaction. */ @Value - class DeleteRoot implements WithEntity { + class DeleteRoot implements DbAction{ - @NonNull Object id; - @Nullable T entity; - @NonNull Class entityType; + @NonNull final Object id; + @NonNull final Class entityType; + @Nullable final Number previousVersion; @Override public void doExecuteWith(Interpreter interpreter) { @@ -284,7 +286,7 @@ public interface DbAction { return null; } return Pair.of(entry.getKey(), entry.getValue()); - }; + } @Override default Class getEntityType() { @@ -350,8 +352,11 @@ public interface DbAction { return (Class) getPropertyPath().getRequiredLeafProperty().getActualType(); } } - interface WithVersion extends WithEntity { + + interface WithVersion { + Number getNextVersion(); + void setNextVersion(Number nextVersion); } } 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 090b3ef3..fde82615 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 @@ -22,6 +22,7 @@ import java.util.List; import org.springframework.data.convert.EntityWriter; import org.springframework.data.mapping.PersistentProperty; import org.springframework.data.relational.core.mapping.RelationalMappingContext; +import org.springframework.data.relational.core.mapping.RelationalPersistentEntity; import org.springframework.lang.Nullable; import org.springframework.util.Assert; @@ -34,6 +35,7 @@ import org.springframework.util.Assert; * @author Jens Schauder * @author Mark Paluch * @author Bastian Wilhelm + * @author Tyler Van Gorder */ public class RelationalEntityDeleteWriter implements EntityWriter> { @@ -82,7 +84,7 @@ public class RelationalEntityDeleteWriter implements EntityWriter List> deleteRoot(Object id, AggregateChange aggregateChange) { List> actions = new ArrayList<>(deleteReferencedEntities(id, aggregateChange)); - actions.add(new DbAction.DeleteRoot<>(id, aggregateChange.getEntity(), aggregateChange.getEntityType())); + actions.add(new DbAction.DeleteRoot<>(id, aggregateChange.getEntityType(), getVersion(aggregateChange))); return actions; } @@ -104,4 +106,23 @@ public class RelationalEntityDeleteWriter implements EntityWriter 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/RelationalEntityVersionUtils.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/RelationalEntityVersionUtils.java index cd507f0e..96463449 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/RelationalEntityVersionUtils.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/RelationalEntityVersionUtils.java @@ -1,3 +1,18 @@ +/* + * Copyright 2019 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ package org.springframework.data.relational.core.conversion; import org.springframework.data.mapping.PersistentPropertyAccessor; @@ -10,6 +25,7 @@ import org.springframework.lang.Nullable; * Utilities commonly used to set/get properties for instances of RelationalPersistentEntities. * * @author Tyler Van Gorder + * @since 2.0 */ public class RelationalEntityVersionUtils { @@ -31,8 +47,8 @@ public class RelationalEntityVersionUtils { throw new IllegalArgumentException("The entity does not have a version property."); } - ConvertingPropertyAccessor convertingPropertyAccessor = new ConvertingPropertyAccessor<>(persistentEntity.getPropertyAccessor(instance), - converter.getConversionService()); + ConvertingPropertyAccessor convertingPropertyAccessor = new ConvertingPropertyAccessor<>( + persistentEntity.getPropertyAccessor(instance), converter.getConversionService()); return convertingPropertyAccessor.getProperty(persistentEntity.getRequiredVersionProperty(), Number.class); } diff --git a/src/main/asciidoc/jdbc.adoc b/src/main/asciidoc/jdbc.adoc index c862a926..b3b8cf0c 100644 --- a/src/main/asciidoc/jdbc.adoc +++ b/src/main/asciidoc/jdbc.adoc @@ -365,6 +365,21 @@ Note that whether an entity is new is part of the entity's state. With auto-increment columns, this happens automatically, because the ID gets set by Spring Data with the value from the ID column. If you are not using auto-increment columns, you can use a `BeforeSave` listener, which sets the ID of the entity (covered later in this document). + +[[jdbc.entity-persistence.optimistic-locking]] +=== Optimistic Locking + +Spring Data JDBC supports optimistic locking by means of a numeric attribute that is annotated with + https://docs.spring.io/spring-data/commons/docs/current/api/org/springframework/data/annotation/Version.html[`@Version`] on the aggregate root. +Whenever Spring Data JDBC saves an aggregate with such a version attribute two things happen: +The update statement for the aggregate root will contain a where clause checking that the version stored in the database is actually unchanged. +If this isn't the case an `OptimisticLockingFailureException` will be thrown. +Also the version attribute gets increased both in the entity and in the database so a concurrent action will notice the change and throw an `OptimisticLockingFailureException` if applicable as described above. + +This process also applies to inserting new aggregates, where a `null` or `0` version indicates a new instance and the increased instance afterwards marks the instance as not new anymore, making this work rather nicely with cases where the id is generated during object construction for example when UUIDs are used. + +During deletes the version check also applies but no version is increased. + [[jdbc.query-methods]] == Query Methods diff --git a/src/main/asciidoc/new-features.adoc b/src/main/asciidoc/new-features.adoc index 19bff45f..8e1f586b 100644 --- a/src/main/asciidoc/new-features.adoc +++ b/src/main/asciidoc/new-features.adoc @@ -3,6 +3,11 @@ This section covers the significant changes for each version. +[[new-features.2-0-0]] +== What's New in Spring Data JDBC 2.0 + +* Optimistic Locking support. + [[new-features.1-0-0]] == What's New in Spring Data JDBC 1.0 @@ -13,3 +18,4 @@ This section covers the significant changes for each version. * Event support. * Auditing. * `CustomConversions`. +