From aa05b792c36341cea2ef48a0068ba726bb8270bb Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Wed, 6 May 2020 13:54:55 +0200 Subject: [PATCH] DATAJDBC-493 - Polishing. Using `execute` instead of `query` since we are not interested in the results. Refactoring of the concurrency tests. Make the concurrency tests run with all databases. Added support for DB2. Moved AnsiDialect to the main sources so it can be referenced in other Dialects. Original pull request: #196. --- .../data/jdbc/core/JdbcAggregateTemplate.java | 1 - .../jdbc/core/convert/DataAccessStrategy.java | 4 +- .../convert/DefaultDataAccessStrategy.java | 24 +- .../mybatis/MyBatisDataAccessStrategy.java | 7 +- ...GeneratorFixedNamingStrategyUnitTests.java | 3 +- .../core/convert/SqlGeneratorUnitTests.java | 56 +++-- ...RepositoryConcurrencyIntegrationTests.java | 210 +++++++----------- .../JdbcRepositoryIntegrationTests.java | 1 - ...positoryConcurrencyIntegrationTests-h2.sql | 2 + ...sitoryConcurrencyIntegrationTests-hsql.sql | 2 + ...oryConcurrencyIntegrationTests-mariadb.sql | 2 + ...itoryConcurrencyIntegrationTests-mssql.sql | 2 + ...ryConcurrencyIntegrationTests-postgres.sql | 4 + .../relational/core/conversion/DbAction.java | 9 +- .../core/dialect/AbstractDialect.java | 44 +--- .../relational/core/dialect}/AnsiDialect.java | 8 +- .../relational/core/dialect/Db2Dialect.java | 6 + .../relational/core/dialect/H2Dialect.java | 24 +- .../core/dialect/HsqlDbDialect.java | 17 +- .../core/dialect/PostgresDialect.java | 1 + .../dialect/SqlServerSelectRenderContext.java | 3 +- .../core/sql/DefaultSelectBuilder.java | 2 + .../data/relational/core/sql/LockMode.java | 1 + .../data/relational/core/sql/LockOptions.java | 2 + 24 files changed, 174 insertions(+), 261 deletions(-) create mode 100644 spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryConcurrencyIntegrationTests-h2.sql create mode 100644 spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryConcurrencyIntegrationTests-hsql.sql create mode 100644 spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryConcurrencyIntegrationTests-mariadb.sql create mode 100644 spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryConcurrencyIntegrationTests-mssql.sql create mode 100644 spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryConcurrencyIntegrationTests-postgres.sql rename {spring-data-jdbc/src/test/java/org/springframework/data/jdbc/testing => spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect}/AnsiDialect.java (90%) 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 8b1c28fdd..396515a17 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 @@ -50,7 +50,6 @@ import org.springframework.util.Assert; * @author Thomas Lang * @author Christoph Strobl * @author Milan Milanov - * @author Myeonghyeon Lee */ public class JdbcAggregateTemplate implements JdbcAggregateOperations { 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 c69ee14a7..8b301383c 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 @@ -132,7 +132,7 @@ public interface DataAccessStrategy extends RelationResolver { void deleteAll(PersistentPropertyPath propertyPath); /** - * Acquire Lock + * Acquire a lock on the aggregate specified by id. * * @param id the id of the entity to load. Must not be {@code null}. * @param lockMode the lock mode for select. Must not be {@code null}. @@ -141,7 +141,7 @@ public interface DataAccessStrategy extends RelationResolver { void acquireLockById(Object id, LockMode lockMode, Class domainType); /** - * Acquire Lock entities of the given domain type. + * Acquire a lock on all aggregates of the given domain type. * * @param lockMode the lock mode for select. Must not be {@code null}. * @param domainType the domain type of the entity. Must not be {@code null}. 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 80c6fe4b0..80b7623db 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 @@ -18,6 +18,7 @@ package org.springframework.data.jdbc.core.convert; import static org.springframework.data.jdbc.core.convert.SqlGenerator.*; import java.sql.JDBCType; +import java.sql.PreparedStatement; import java.sql.ResultSet; import java.sql.SQLException; import java.util.ArrayList; @@ -27,7 +28,11 @@ import java.util.List; import java.util.Map; import java.util.function.Predicate; -import org.springframework.dao.*; +import org.springframework.dao.DataAccessException; +import org.springframework.dao.DataRetrievalFailureException; +import org.springframework.dao.EmptyResultDataAccessException; +import org.springframework.dao.InvalidDataAccessApiUsageException; +import org.springframework.dao.OptimisticLockingFailureException; import org.springframework.data.domain.Pageable; import org.springframework.data.domain.Sort; import org.springframework.data.jdbc.support.JdbcUtil; @@ -42,6 +47,7 @@ import org.springframework.data.relational.core.mapping.RelationalPersistentProp import org.springframework.data.relational.core.sql.IdentifierProcessing; import org.springframework.data.relational.core.sql.LockMode; import org.springframework.data.relational.core.sql.SqlIdentifier; +import org.springframework.jdbc.core.PreparedStatementCallback; import org.springframework.jdbc.core.ResultSetExtractor; import org.springframework.jdbc.core.RowMapper; import org.springframework.jdbc.core.namedparam.NamedParameterJdbcOperations; @@ -245,9 +251,10 @@ public class DefaultDataAccessStrategy implements DataAccessStrategy { */ @Override public void acquireLockById(Object id, LockMode lockMode, Class domainType) { + String acquireLockByIdSql = sql(domainType).getAcquireLockById(lockMode); SqlIdentifierParameterSource parameter = createIdParameterSource(id, domainType); - operations.queryForObject(acquireLockByIdSql, parameter, Object.class); + operations.execute(acquireLockByIdSql, parameter, ps -> {ps.execute(); return null;}); } /* @@ -256,8 +263,9 @@ public class DefaultDataAccessStrategy implements DataAccessStrategy { */ @Override public void acquireLockAll(LockMode lockMode, Class domainType) { + String acquireLockAllSql = sql(domainType).getAcquireLockAll(lockMode); - operations.query(acquireLockAllSql, Collections.emptyMap(), new NoMappingResultSetExtractor()); + operations.getJdbcOperations().execute(acquireLockAllSql); } /* @@ -605,14 +613,4 @@ public class DefaultDataAccessStrategy implements DataAccessStrategy { return null; } } - - /** - * The type No mapping result set extractor. - */ - static class NoMappingResultSetExtractor implements ResultSetExtractor { - @Override - public Object extractData(ResultSet resultSet) throws SQLException, DataAccessException { - return null; - } - } } 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 0ac9e2aa1..ea918b24a 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 @@ -257,13 +257,15 @@ public class MyBatisDataAccessStrategy implements DataAccessStrategy { */ @Override public void acquireLockById(Object id, LockMode lockMode, Class domainType) { + String statement = namespace(domainType) + ".acquireLockById"; MyBatisContext parameter = new MyBatisContext(id, null, domainType, Collections.emptyMap()); long result = sqlSession().selectOne(statement, parameter); if (result < 1) { - throw new EmptyResultDataAccessException( - String.format("The lock target does not exist. id: %s, statement: %s", id, statement), 1); + + String message = String.format("The lock target does not exist. id: %s, statement: %s", id, statement); + throw new EmptyResultDataAccessException(message, 1); } } @@ -273,6 +275,7 @@ public class MyBatisDataAccessStrategy implements DataAccessStrategy { */ @Override public void acquireLockAll(LockMode lockMode, Class domainType) { + String statement = namespace(domainType) + ".acquireLockAll"; MyBatisContext parameter = new MyBatisContext(null, null, domainType, Collections.emptyMap()); diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorFixedNamingStrategyUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorFixedNamingStrategyUnitTests.java index be3928932..5fc67d100 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorFixedNamingStrategyUnitTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorFixedNamingStrategyUnitTests.java @@ -23,9 +23,8 @@ import org.junit.Test; import org.springframework.data.annotation.Id; import org.springframework.data.jdbc.core.mapping.JdbcMappingContext; import org.springframework.data.jdbc.core.mapping.PersistentPropertyPathTestUtils; -import org.springframework.data.jdbc.testing.AnsiDialect; +import org.springframework.data.relational.core.dialect.AnsiDialect; import org.springframework.data.mapping.PersistentPropertyPath; -import org.springframework.data.relational.core.dialect.HsqlDbDialect; import org.springframework.data.relational.core.mapping.NamingStrategy; import org.springframework.data.relational.core.mapping.RelationalMappingContext; import org.springframework.data.relational.core.mapping.RelationalPersistentEntity; 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 1599b428e..ef030fce9 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 @@ -25,7 +25,6 @@ import java.util.Set; import org.assertj.core.api.SoftAssertions; import org.junit.Before; import org.junit.Test; - import org.springframework.data.annotation.Id; import org.springframework.data.annotation.ReadOnlyProperty; import org.springframework.data.annotation.Version; @@ -36,7 +35,7 @@ import org.springframework.data.jdbc.core.PropertyPathTestingUtils; import org.springframework.data.jdbc.core.mapping.AggregateReference; import org.springframework.data.jdbc.core.mapping.JdbcMappingContext; import org.springframework.data.jdbc.core.mapping.PersistentPropertyPathTestUtils; -import org.springframework.data.jdbc.testing.AnsiDialect; +import org.springframework.data.relational.core.dialect.AnsiDialect; import org.springframework.data.mapping.PersistentPropertyPath; import org.springframework.data.relational.core.dialect.Dialect; import org.springframework.data.relational.core.mapping.Column; @@ -95,19 +94,18 @@ public class SqlGeneratorUnitTests { String sql = sqlGenerator.getFindOne(); - SoftAssertions softAssertions = new SoftAssertions(); - softAssertions.assertThat(sql) // - .startsWith("SELECT") // - .contains("dummy_entity.id1 AS id1,") // - .contains("dummy_entity.x_name AS x_name,") // - .contains("dummy_entity.x_other AS x_other,") // - .contains("ref.x_l1id AS ref_x_l1id") // - .contains("ref.x_content AS ref_x_content").contains(" FROM dummy_entity") // - .contains("ON ref.dummy_entity = dummy_entity.id1") // - .contains("WHERE dummy_entity.id1 = :id") // - // 1-N relationships do not get loaded via join - .doesNotContain("Element AS elements"); - softAssertions.assertAll(); + SoftAssertions.assertSoftly(softly -> softly // + .assertThat(sql) // + .startsWith("SELECT") // + .contains("dummy_entity.id1 AS id1,") // + .contains("dummy_entity.x_name AS x_name,") // + .contains("dummy_entity.x_other AS x_other,") // + .contains("ref.x_l1id AS ref_x_l1id") // + .contains("ref.x_content AS ref_x_content").contains(" FROM dummy_entity") // + .contains("ON ref.dummy_entity = dummy_entity.id1") // + .contains("WHERE dummy_entity.id1 = :id") // + // 1-N relationships do not get loaded via join + .doesNotContain("Element AS elements")); } @Test // DATAJDBC-493 @@ -115,14 +113,13 @@ public class SqlGeneratorUnitTests { String sql = sqlGenerator.getAcquireLockById(LockMode.PESSIMISTIC_WRITE); - SoftAssertions softAssertions = new SoftAssertions(); - softAssertions.assertThat(sql) // - .startsWith("SELECT") // - .contains("dummy_entity.id1") // - .contains("WHERE dummy_entity.id1 = :id") // - .contains("FOR UPDATE") // - .doesNotContain("Element AS elements"); - softAssertions.assertAll(); + SoftAssertions.assertSoftly(softly -> softly // + .assertThat(sql) // + .startsWith("SELECT") // + .contains("dummy_entity.id1") // + .contains("WHERE dummy_entity.id1 = :id") // + .contains("FOR UPDATE") // + .doesNotContain("Element AS elements")); } @Test // DATAJDBC-493 @@ -130,13 +127,12 @@ public class SqlGeneratorUnitTests { String sql = sqlGenerator.getAcquireLockAll(LockMode.PESSIMISTIC_WRITE); - SoftAssertions softAssertions = new SoftAssertions(); - softAssertions.assertThat(sql) // - .startsWith("SELECT") // - .contains("dummy_entity.id1") // - .contains("FOR UPDATE") // - .doesNotContain("Element AS elements"); - softAssertions.assertAll(); + SoftAssertions.assertSoftly(softly -> softly // + .assertThat(sql) // + .startsWith("SELECT") // + .contains("dummy_entity.id1") // + .contains("FOR UPDATE") // + .doesNotContain("Element AS elements")); } @Test // DATAJDBC-112 diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/JdbcRepositoryConcurrencyIntegrationTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/JdbcRepositoryConcurrencyIntegrationTests.java index e12a762bd..ac56aabd5 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/JdbcRepositoryConcurrencyIntegrationTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/JdbcRepositoryConcurrencyIntegrationTests.java @@ -15,10 +15,21 @@ */ package org.springframework.data.jdbc.repository; +import static org.assertj.core.api.Assertions.*; + import junit.framework.AssertionFailedError; import lombok.AllArgsConstructor; import lombok.Getter; import lombok.With; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.CountDownLatch; +import java.util.function.UnaryOperator; + +import org.junit.Before; import org.junit.ClassRule; import org.junit.Rule; import org.junit.Test; @@ -29,34 +40,20 @@ import org.springframework.context.annotation.Import; import org.springframework.dao.IncorrectUpdateSemanticsDataAccessException; import org.springframework.data.annotation.Id; import org.springframework.data.jdbc.repository.support.JdbcRepositoryFactory; -import org.springframework.data.jdbc.testing.DatabaseProfileValueSource; import org.springframework.data.jdbc.testing.TestConfiguration; import org.springframework.data.repository.CrudRepository; import org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate; -import org.springframework.test.annotation.IfProfileValue; -import org.springframework.test.annotation.ProfileValueSourceConfiguration; -import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.junit4.rules.SpringClassRule; import org.springframework.test.context.junit4.rules.SpringMethodRule; import org.springframework.transaction.PlatformTransactionManager; import org.springframework.transaction.support.TransactionTemplate; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.List; -import java.util.concurrent.CopyOnWriteArrayList; -import java.util.concurrent.CountDownLatch; - -import static org.assertj.core.api.Assertions.assertThat; - -/** Tests that highly concurrent update operations of an entity don't cause deadlocks. +/** + * Tests that highly concurrent update operations of an entity don't cause deadlocks. * * @author Myeonghyeon Lee * @author Jens Schauder */ -@ContextConfiguration -@ProfileValueSourceConfiguration(DatabaseProfileValueSource.class) -@IfProfileValue(name = "current.database.is.not.mysql", value = "false") public class JdbcRepositoryConcurrencyIntegrationTests { @Configuration @@ -83,38 +80,37 @@ public class JdbcRepositoryConcurrencyIntegrationTests { @Autowired DummyEntityRepository repository; @Autowired PlatformTransactionManager transactionManager; - @Test // DATAJDBC-488 - public void updateConcurrencyWithEmptyReferences() throws Exception { + List concurrencyEntities; + DummyEntity entity; - DummyEntity entity = createDummyEntity(); - entity = repository.save(entity); + TransactionTemplate transactionTemplate; + List exceptions; + + @Before + public void before() { + + entity = repository.save(createDummyEntity()); assertThat(entity.getId()).isNotNull(); - List concurrencyEntities = createEntityStates(entity); + concurrencyEntities = createEntityStates(entity); - TransactionTemplate transactionTemplate = new TransactionTemplate(this.transactionManager); + transactionTemplate = new TransactionTemplate(this.transactionManager); - List exceptions = new CopyOnWriteArrayList<>(); - CountDownLatch startLatch = new CountDownLatch(concurrencyEntities.size()); // latch for all threads to wait on. - CountDownLatch doneLatch = new CountDownLatch(concurrencyEntities.size()); // latch for main thread to wait on until all threads are done. + exceptions = new CopyOnWriteArrayList<>(); + } - concurrencyEntities.stream() // - .map(e -> new Thread(() -> { + @Test // DATAJDBC-488 + public void updateConcurrencyWithEmptyReferences() throws Exception { - try { + // latch for all threads to wait on. + CountDownLatch startLatch = new CountDownLatch(concurrencyEntities.size()); + // latch for main thread to wait on until all threads are done. + CountDownLatch doneLatch = new CountDownLatch(concurrencyEntities.size()); - startLatch.countDown(); - startLatch.await(); + UnaryOperator action = e -> repository.save(e); - transactionTemplate.execute(status -> repository.save(e)); - } catch (Exception ex) { - exceptions.add(ex); - } finally { - doneLatch.countDown(); - } - })) // - .forEach(Thread::start); + concurrencyEntities.forEach(e -> executeInParallel(startLatch, doneLatch, action, e)); doneLatch.await(); @@ -124,62 +120,31 @@ public class JdbcRepositoryConcurrencyIntegrationTests { } @Test // DATAJDBC-493 - public void updateConcurrencyWithDelete() throws Exception { - - DummyEntity entity = createDummyEntity(); - entity = repository.save(entity); - - Long targetId = entity.getId(); - assertThat(targetId).isNotNull(); - - List concurrencyEntities = createEntityStates(entity); - - TransactionTemplate transactionTemplate = new TransactionTemplate(this.transactionManager); + public void concurrentUpdateAndDelete() throws Exception { - List exceptions = new CopyOnWriteArrayList<>(); CountDownLatch startLatch = new CountDownLatch(concurrencyEntities.size() + 1); // latch for all threads to wait on. - CountDownLatch doneLatch = new CountDownLatch(concurrencyEntities.size() + 1); // latch for main thread to wait on until all threads are done. - - // update - concurrencyEntities.stream() // - .map(e -> new Thread(() -> { - - try { - - startLatch.countDown(); - startLatch.await(); - - transactionTemplate.execute(status -> repository.save(e)); - } catch (Exception ex) { - // When the delete execution is complete, the Update execution throws an IncorrectUpdateSemanticsDataAccessException. - if (ex.getCause() instanceof IncorrectUpdateSemanticsDataAccessException) { - return; - } - - exceptions.add(ex); - } finally { - doneLatch.countDown(); - } - })) // - .forEach(Thread::start); - - // delete - new Thread(() -> { + CountDownLatch doneLatch = new CountDownLatch(concurrencyEntities.size() + 1); // latch for main thread to wait on + // until all threads are done. + UnaryOperator updateAction = e -> { try { - - startLatch.countDown(); - startLatch.await(); - - transactionTemplate.execute(status -> { - repository.deleteById(targetId); - return null; - }); + return repository.save(e); } catch (Exception ex) { - exceptions.add(ex); - } finally { - doneLatch.countDown(); + // When the delete execution is complete, the Update execution throws an + // IncorrectUpdateSemanticsDataAccessException. + if (ex.getCause() instanceof IncorrectUpdateSemanticsDataAccessException) { + return null; + } + throw ex; } - }).start(); + }; + + UnaryOperator deleteAction = e -> { + repository.deleteById(entity.id); + return null; + }; + + concurrencyEntities.forEach(e -> executeInParallel(startLatch, doneLatch, updateAction, e)); + executeInParallel(startLatch, doneLatch, deleteAction, entity); doneLatch.await(); @@ -188,42 +153,41 @@ public class JdbcRepositoryConcurrencyIntegrationTests { } @Test // DATAJDBC-493 - public void updateConcurrencyWithDeleteAll() throws Exception { - - DummyEntity entity = createDummyEntity(); - entity = repository.save(entity); - - List concurrencyEntities = createEntityStates(entity); - - TransactionTemplate transactionTemplate = new TransactionTemplate(this.transactionManager); + public void concurrentUpdateAndDeleteAll() throws Exception { - List exceptions = new CopyOnWriteArrayList<>(); CountDownLatch startLatch = new CountDownLatch(concurrencyEntities.size() + 1); // latch for all threads to wait on. - CountDownLatch doneLatch = new CountDownLatch(concurrencyEntities.size() + 1); // latch for main thread to wait on until all threads are done. + CountDownLatch doneLatch = new CountDownLatch(concurrencyEntities.size() + 1); // latch for main thread to wait on + // until all threads are done. - // update - concurrencyEntities.stream() // - .map(e -> new Thread(() -> { + UnaryOperator updateAction = e -> { + try { + return repository.save(e); + } catch (Exception ex) { + // When the delete execution is complete, the Update execution throws an + // IncorrectUpdateSemanticsDataAccessException. + if (ex.getCause() instanceof IncorrectUpdateSemanticsDataAccessException) { + return null; + } + throw ex; + } + }; - try { + UnaryOperator deleteAction = e -> { + repository.deleteAll(); + return null; + }; - startLatch.countDown(); - startLatch.await(); + concurrencyEntities.forEach(e -> executeInParallel(startLatch, doneLatch, updateAction, e)); + executeInParallel(startLatch, doneLatch, deleteAction, entity); - transactionTemplate.execute(status -> repository.save(e)); - } catch (Exception ex) { - // When the delete execution is complete, the Update execution throws an IncorrectUpdateSemanticsDataAccessException. - if (ex.getCause() instanceof IncorrectUpdateSemanticsDataAccessException) { - return; - } + doneLatch.await(); - exceptions.add(ex); - } finally { - doneLatch.countDown(); - } - })) // - .forEach(Thread::start); + assertThat(exceptions).isEmpty(); + assertThat(repository.count()).isEqualTo(0); + } + private void executeInParallel(CountDownLatch startLatch, CountDownLatch doneLatch, + UnaryOperator deleteAction, DummyEntity entity) { // delete new Thread(() -> { try { @@ -231,21 +195,13 @@ public class JdbcRepositoryConcurrencyIntegrationTests { startLatch.countDown(); startLatch.await(); - transactionTemplate.execute(status -> { - repository.deleteAll(); - return null; - }); + transactionTemplate.execute(status -> deleteAction.apply(entity)); } catch (Exception ex) { exceptions.add(ex); } finally { doneLatch.countDown(); } }).start(); - - doneLatch.await(); - - assertThat(exceptions).isEmpty(); - assertThat(repository.count()).isEqualTo(0); } private List createEntityStates(DummyEntity entity) { @@ -254,7 +210,7 @@ public class JdbcRepositoryConcurrencyIntegrationTests { Element element1 = new Element(null, 1L); Element element2 = new Element(null, 2L); - for (int i = 0; i < 100; i++) { + for (int i = 0; i < 50; i++) { List newContent = Arrays.asList(element1.withContent(element1.content + i + 2), element2.withContent(element2.content + i + 2)); diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/JdbcRepositoryIntegrationTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/JdbcRepositoryIntegrationTests.java index 12cf1bedf..b76287754 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/JdbcRepositoryIntegrationTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/JdbcRepositoryIntegrationTests.java @@ -64,7 +64,6 @@ import org.springframework.transaction.annotation.Transactional; * @author Jens Schauder * @author Mark Paluch */ -@ContextConfiguration @Transactional public class JdbcRepositoryIntegrationTests { diff --git a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryConcurrencyIntegrationTests-h2.sql b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryConcurrencyIntegrationTests-h2.sql new file mode 100644 index 000000000..942dd36cf --- /dev/null +++ b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryConcurrencyIntegrationTests-h2.sql @@ -0,0 +1,2 @@ +CREATE TABLE dummy_entity ( id BIGINT GENERATED BY DEFAULT AS IDENTITY ( START WITH 1 ) PRIMARY KEY, NAME VARCHAR(100)); +CREATE TABLE element (id BIGINT GENERATED BY DEFAULT AS IDENTITY ( START WITH 1 ) PRIMARY KEY, content BIGINT, Dummy_Entity_key BIGINT,dummy_entity BIGINT); diff --git a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryConcurrencyIntegrationTests-hsql.sql b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryConcurrencyIntegrationTests-hsql.sql new file mode 100644 index 000000000..942dd36cf --- /dev/null +++ b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryConcurrencyIntegrationTests-hsql.sql @@ -0,0 +1,2 @@ +CREATE TABLE dummy_entity ( id BIGINT GENERATED BY DEFAULT AS IDENTITY ( START WITH 1 ) PRIMARY KEY, NAME VARCHAR(100)); +CREATE TABLE element (id BIGINT GENERATED BY DEFAULT AS IDENTITY ( START WITH 1 ) PRIMARY KEY, content BIGINT, Dummy_Entity_key BIGINT,dummy_entity BIGINT); diff --git a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryConcurrencyIntegrationTests-mariadb.sql b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryConcurrencyIntegrationTests-mariadb.sql new file mode 100644 index 000000000..e0a8a767c --- /dev/null +++ b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryConcurrencyIntegrationTests-mariadb.sql @@ -0,0 +1,2 @@ +CREATE TABLE dummy_entity ( id BIGINT AUTO_INCREMENT PRIMARY KEY, NAME VARCHAR(100)); +CREATE TABLE element (id BIGINT AUTO_INCREMENT PRIMARY KEY, content BIGINT, Dummy_Entity_key BIGINT,dummy_entity BIGINT); diff --git a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryConcurrencyIntegrationTests-mssql.sql b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryConcurrencyIntegrationTests-mssql.sql new file mode 100644 index 000000000..e0a8a767c --- /dev/null +++ b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryConcurrencyIntegrationTests-mssql.sql @@ -0,0 +1,2 @@ +CREATE TABLE dummy_entity ( id BIGINT AUTO_INCREMENT PRIMARY KEY, NAME VARCHAR(100)); +CREATE TABLE element (id BIGINT AUTO_INCREMENT PRIMARY KEY, content BIGINT, Dummy_Entity_key BIGINT,dummy_entity BIGINT); diff --git a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryConcurrencyIntegrationTests-postgres.sql b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryConcurrencyIntegrationTests-postgres.sql new file mode 100644 index 000000000..ed7a483c9 --- /dev/null +++ b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryConcurrencyIntegrationTests-postgres.sql @@ -0,0 +1,4 @@ +DROP TABLE dummy_entity; +DROP TABLE element; +CREATE TABLE dummy_entity ( id SERIAL PRIMARY KEY, NAME VARCHAR(100)); +CREATE TABLE element (id SERIAL PRIMARY KEY, content BIGINT, Dummy_Entity_key BIGINT,dummy_entity BIGINT); 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 142277d10..7ddbf06e9 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 @@ -327,10 +327,11 @@ public interface DbAction { * @param type of the entity for which this represents a database interaction. */ final class AcquireLockRoot implements DbAction { + private final Object id; private final Class entityType; - public AcquireLockRoot(Object id, Class entityType) { + AcquireLockRoot(Object id, Class entityType) { this.id = id; this.entityType = entityType; } @@ -349,15 +350,15 @@ public interface DbAction { } /** - * Represents an acquire lock statement for all entities that that a reachable via a give path from any aggregate root of a - * given type. + * Represents an acquire lock statement for all aggregate roots of a given type. * * @param type of the entity for which this represents a database interaction. */ final class AcquireLockAllRoot implements DbAction { + private final Class entityType; - public AcquireLockAllRoot(Class entityType) { + AcquireLockAllRoot(Class entityType) { this.entityType = entityType; } diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/AbstractDialect.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/AbstractDialect.java index 35f1b39c9..cfde945f4 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/AbstractDialect.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/AbstractDialect.java @@ -57,12 +57,8 @@ public abstract class AbstractDialect implements Dialect { Function afterFromTable = select -> ""; LockClause lockClause = lock(); - switch (lockClause.getClausePosition()) { - - case AFTER_FROM_TABLE: - afterFromTable = new LockRenderFunction(lockClause); - - default: + if (lockClause.getClausePosition() == LockClause.Position.AFTER_FROM_TABLE) { + afterFromTable = new LockRenderFunction(lockClause); } return afterFromTable.andThen(PrependWithLeadingWhitespace.INSTANCE); @@ -78,31 +74,19 @@ public abstract class AbstractDialect implements Dialect { Function afterOrderByLimit = getAfterOrderByLimit(); Function afterOrderByLock = getAfterOrderByLock(); - return select -> { - - StringBuilder afterOrderByBuilder = new StringBuilder(); - afterOrderByBuilder.append(afterOrderByLimit.apply(select)); - afterOrderByBuilder.append(afterOrderByLock.apply(select)); - return afterOrderByBuilder.toString(); - }; + return select -> String.valueOf(afterOrderByLimit.apply(select)) + + afterOrderByLock.apply(select); } private Function getAfterOrderByLimit() { LimitClause limit = limit(); - Function afterOrderByLimit = select -> ""; - - switch (limit.getClausePosition()) { - - case AFTER_ORDER_BY: - afterOrderByLimit = new AfterOrderByLimitRenderFunction(limit); - break; - - default: - throw new UnsupportedOperationException(String.format("Clause position %s not supported!", limit)); + if (limit.getClausePosition() == LimitClause.Position.AFTER_ORDER_BY) { + return new AfterOrderByLimitRenderFunction(limit) // + .andThen(PrependWithLeadingWhitespace.INSTANCE); + } else { + throw new UnsupportedOperationException(String.format("Clause position %s not supported!", limit)); } - - return afterOrderByLimit.andThen(PrependWithLeadingWhitespace.INSTANCE); } private Function getAfterOrderByLock() { @@ -110,12 +94,8 @@ public abstract class AbstractDialect implements Dialect { Function afterOrderByLock = select -> ""; - switch (lock.getClausePosition()) { - - case AFTER_ORDER_BY: - afterOrderByLock = new LockRenderFunction(lock); - - default: + if (lock.getClausePosition() == LockClause.Position.AFTER_ORDER_BY) { + afterOrderByLock = new LockRenderFunction(lock); } return afterOrderByLock.andThen(PrependWithLeadingWhitespace.INSTANCE); @@ -124,7 +104,7 @@ public abstract class AbstractDialect implements Dialect { /** * {@link SelectRenderContext} derived from {@link Dialect} specifics. */ - class DialectSelectRenderContext implements SelectRenderContext { + static class DialectSelectRenderContext implements SelectRenderContext { private final Function afterFromTable; private final Function afterOrderBy; diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/testing/AnsiDialect.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/AnsiDialect.java similarity index 90% rename from spring-data-jdbc/src/test/java/org/springframework/data/jdbc/testing/AnsiDialect.java rename to spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/AnsiDialect.java index 2d111648b..cd6c6db92 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/testing/AnsiDialect.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/AnsiDialect.java @@ -13,14 +13,10 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.springframework.data.jdbc.testing; +package org.springframework.data.relational.core.dialect; import lombok.RequiredArgsConstructor; -import org.springframework.data.relational.core.dialect.AbstractDialect; -import org.springframework.data.relational.core.dialect.ArrayColumns; -import org.springframework.data.relational.core.dialect.LimitClause; -import org.springframework.data.relational.core.dialect.LockClause; import org.springframework.data.relational.core.sql.IdentifierProcessing; import org.springframework.data.relational.core.sql.LockOptions; import org.springframework.util.Assert; @@ -81,7 +77,7 @@ public class AnsiDialect extends AbstractDialect { } }; - private static final LockClause LOCK_CLAUSE = new LockClause() { + static final LockClause LOCK_CLAUSE = new LockClause() { /* * (non-Javadoc) diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/Db2Dialect.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/Db2Dialect.java index 486d49fb1..a646885f8 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/Db2Dialect.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/Db2Dialect.java @@ -16,6 +16,7 @@ package org.springframework.data.relational.core.dialect; import org.springframework.data.relational.core.sql.IdentifierProcessing; +import org.springframework.data.relational.core.sql.LockOptions; /** * An SQL dialect for DB2. @@ -80,6 +81,11 @@ public class Db2Dialect extends AbstractDialect { return LIMIT_CLAUSE; } + @Override + public LockClause lock() { + return AnsiDialect.LOCK_CLAUSE; + } + /* * (non-Javadoc) * @see org.springframework.data.relational.core.dialect.Dialect#getIdentifierProcessing() diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/H2Dialect.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/H2Dialect.java index f055befc2..829179d2c 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/H2Dialect.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/H2Dialect.java @@ -20,7 +20,6 @@ import lombok.RequiredArgsConstructor; import org.springframework.data.relational.core.sql.IdentifierProcessing; import org.springframework.data.relational.core.sql.IdentifierProcessing.LetterCasing; import org.springframework.data.relational.core.sql.IdentifierProcessing.Quoting; -import org.springframework.data.relational.core.sql.LockOptions; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; @@ -79,27 +78,6 @@ public class H2Dialect extends AbstractDialect { } }; - private static final LockClause LOCK_CLAUSE = new LockClause() { - - /* - * (non-Javadoc) - * @see org.springframework.data.relational.core.dialect.LockClause#getLock(LockOptions) - */ - @Override - public String getLock(LockOptions lockOptions) { - return "FOR UPDATE"; - } - - /* - * (non-Javadoc) - * @see org.springframework.data.relational.core.dialect.LockClause#getClausePosition() - */ - @Override - public Position getClausePosition() { - return Position.AFTER_ORDER_BY; - } - }; - private final H2ArrayColumns ARRAY_COLUMNS = new H2ArrayColumns(); /* @@ -117,7 +95,7 @@ public class H2Dialect extends AbstractDialect { */ @Override public LockClause lock() { - return LOCK_CLAUSE; + return AnsiDialect.LOCK_CLAUSE; } /* diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/HsqlDbDialect.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/HsqlDbDialect.java index b97dbfdf4..e5aaa6b24 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/HsqlDbDialect.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/HsqlDbDialect.java @@ -15,8 +15,6 @@ */ package org.springframework.data.relational.core.dialect; -import org.springframework.data.relational.core.sql.LockOptions; - /** * A {@link Dialect} for HsqlDb. * @@ -36,7 +34,7 @@ public class HsqlDbDialect extends AbstractDialect { @Override public LockClause lock() { - return LOCK_CLAUSE; + return AnsiDialect.LOCK_CLAUSE; } private static final LimitClause LIMIT_CLAUSE = new LimitClause() { @@ -61,17 +59,4 @@ public class HsqlDbDialect extends AbstractDialect { return Position.AFTER_ORDER_BY; } }; - - private static final LockClause LOCK_CLAUSE = new LockClause() { - - @Override - public String getLock(LockOptions lockOptions) { - return "FOR UPDATE"; - } - - @Override - public Position getClausePosition() { - return Position.AFTER_ORDER_BY; - } - }; } diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/PostgresDialect.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/PostgresDialect.java index c0f786798..22d2bb34c 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/PostgresDialect.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/PostgresDialect.java @@ -115,6 +115,7 @@ public class PostgresDialect extends AbstractDialect { } static class PostgresLockClause implements LockClause { + private final IdentifierProcessing identifierProcessing; PostgresLockClause(IdentifierProcessing identifierProcessing) { diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/SqlServerSelectRenderContext.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/SqlServerSelectRenderContext.java index 76f2be162..a328c26cd 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/SqlServerSelectRenderContext.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/SqlServerSelectRenderContext.java @@ -46,8 +46,7 @@ public class SqlServerSelectRenderContext implements SelectRenderContext { * @param afterFromTable the delegate {@code afterFromTable} function. * @param afterOrderBy the delegate {@code afterOrderBy} function. */ - protected SqlServerSelectRenderContext( - Function afterFromTable, + protected SqlServerSelectRenderContext(Function afterFromTable, Function afterOrderBy) { this.afterFromTable = afterFromTable; diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/DefaultSelectBuilder.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/DefaultSelectBuilder.java index a9c20c3f9..c1883ef3a 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/DefaultSelectBuilder.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/DefaultSelectBuilder.java @@ -273,6 +273,7 @@ class DefaultSelectBuilder implements SelectBuilder, SelectAndFrom, SelectFromAn */ @Override public SelectLock lock(LockMode lockMode) { + this.lockMode = lockMode; return this; } @@ -283,6 +284,7 @@ class DefaultSelectBuilder implements SelectBuilder, SelectAndFrom, SelectFromAn */ @Override public Select build() { + DefaultSelect select = new DefaultSelect(distinct, selectList, from, limit, offset, joins, where, orderBy, lockMode); SelectValidator.validate(select); return select; diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/LockMode.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/LockMode.java index af6118473..4a5f9c049 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/LockMode.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/LockMode.java @@ -22,6 +22,7 @@ package org.springframework.data.relational.core.sql; * @since 2.0 */ public enum LockMode { + PESSIMISTIC_READ, PESSIMISTIC_WRITE } diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/LockOptions.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/LockOptions.java index 25f37e24e..f20b632c8 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/LockOptions.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/LockOptions.java @@ -22,10 +22,12 @@ package org.springframework.data.relational.core.sql; * @since 2.0 */ public class LockOptions { + private final LockMode lockMode; private final From from; public LockOptions(LockMode lockMode, From from) { + this.lockMode = lockMode; this.from = from; }