From b70132399a4b78cc59e72d3eef10a884e1276595 Mon Sep 17 00:00:00 2001 From: mhyeon-lee Date: Sat, 22 Feb 2020 14:54:20 +0900 Subject: [PATCH 1/8] DATAJDBC-493 Add failing test deadlocks with update and delete. --- ...RepositoryConcurrencyIntegrationTests.java | 67 +++++++++++++++++++ 1 file changed, 67 insertions(+) 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 71520eb1d4..ac38a53b6f 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 @@ -20,12 +20,14 @@ import lombok.Getter; import lombok.With; import org.junit.ClassRule; +import org.junit.Ignore; import org.junit.Rule; import org.junit.Test; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; 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; @@ -123,6 +125,71 @@ public void updateConcurrencyWithEmptyReferences() throws Exception { assertThat(exceptions).isEmpty(); } + @Test // DATAJDBC-493 + @Ignore("failing test") + 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); + + 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(() -> { + try { + + startLatch.countDown(); + startLatch.await(); + + transactionTemplate.execute(status -> { + repository.deleteById(targetId); + return null; + }); + } catch (Exception ex) { + exceptions.add(ex); + } finally { + doneLatch.countDown(); + } + }).start(); + + doneLatch.await(); + + assertThat(exceptions).isEmpty(); + assertThat(repository.findById(entity.id)).isEmpty(); + } + private List createEntityStates(DummyEntity entity) { List concurrencyEntities = new ArrayList<>(); From cbffe9adcd85932d7a778fc22b4d0bebe02a84f6 Mon Sep 17 00:00:00 2001 From: mhyeon-lee Date: Thu, 27 Feb 2020 00:31:40 +0900 Subject: [PATCH 2/8] DATAJDBC-498 Add LockMode for SelectBuilder --- .../relational/core/sql/DefaultSelect.java | 15 ++++- .../core/sql/DefaultSelectBuilder.java | 24 +++++++- .../data/relational/core/sql/LockMode.java | 27 +++++++++ .../data/relational/core/sql/LockOptions.java | 34 +++++++++++ .../data/relational/core/sql/Select.java | 6 ++ .../relational/core/sql/SelectBuilder.java | 42 +++++++++---- .../core/sql/SelectBuilderUnitTests.java | 60 +++++++++++++++++++ 7 files changed, 193 insertions(+), 15 deletions(-) create mode 100644 spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/LockMode.java create mode 100644 spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/LockOptions.java diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/DefaultSelect.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/DefaultSelect.java index 7ffe2aaa3e..1d865dba3a 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/DefaultSelect.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/DefaultSelect.java @@ -27,6 +27,7 @@ * Default {@link Select} implementation. * * @author Mark Paluch + * @author Myeonghyeon Lee * @since 1.1 */ class DefaultSelect implements Select { @@ -39,9 +40,10 @@ class DefaultSelect implements Select { private final List joins; private final @Nullable Where where; private final List orderBy; + private final @Nullable LockMode lockMode; DefaultSelect(boolean distinct, List selectList, List from, long limit, long offset, - List joins, @Nullable Condition where, List orderBy) { + List joins, @Nullable Condition where, List orderBy, @Nullable LockMode lockMode) { this.distinct = distinct; this.selectList = new SelectList(new ArrayList<>(selectList)); @@ -51,6 +53,7 @@ class DefaultSelect implements Select { this.joins = new ArrayList<>(joins); this.orderBy = Collections.unmodifiableList(new ArrayList<>(orderBy)); this.where = where != null ? new Where(where) : null; + this.lockMode = lockMode; } /* @@ -85,6 +88,16 @@ public boolean isDistinct() { return distinct; } + /* + * (non-Javadoc) + * @see org.springframework.data.relational.core.sql.Select#getLockMode() + */ + @Nullable + @Override + public LockMode getLockMode() { + return lockMode; + } + /* * (non-Javadoc) * @see org.springframework.data.relational.core.sql.Visitable#visit(org.springframework.data.relational.core.sql.Visitor) 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 99f8256610..a9c20c3f90 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 @@ -31,6 +31,7 @@ * * @author Mark Paluch * @author Jens Schauder + * @author Myeonghyeon Lee * @since 1.1 */ class DefaultSelectBuilder implements SelectBuilder, SelectAndFrom, SelectFromAndJoin, SelectWhereAndOr { @@ -43,6 +44,7 @@ class DefaultSelectBuilder implements SelectBuilder, SelectAndFrom, SelectFromAn private List joins = new ArrayList<>(); private @Nullable Condition where; private List orderBy = new ArrayList<>(); + private @Nullable LockMode lockMode; /* * (non-Javadoc) @@ -265,13 +267,23 @@ public DefaultSelectBuilder join(Join join) { return this; } + /* + * (non-Javadoc) + * @see org.springframework.data.relational.core.sql.SelectBuilder.SelectLock#lock(org.springframework.data.relational.core.sql.LockMode) + */ + @Override + public SelectLock lock(LockMode lockMode) { + this.lockMode = lockMode; + return this; + } + /* * (non-Javadoc) * @see org.springframework.data.relational.core.sql.SelectBuilder.BuildSelect#build() */ @Override public Select build() { - DefaultSelect select = new DefaultSelect(distinct, selectList, from, limit, offset, joins, where, orderBy); + DefaultSelect select = new DefaultSelect(distinct, selectList, from, limit, offset, joins, where, orderBy, lockMode); SelectValidator.validate(select); return select; } @@ -448,6 +460,16 @@ public SelectFromAndJoin offset(long offset) { return selectBuilder.offset(offset); } + /* + * (non-Javadoc) + * @see org.springframework.data.relational.core.sql.SelectBuilder.SelectLock#lock(org.springframework.data.relational.core.sql.LockMode) + */ + @Override + public SelectLock lock(LockMode lockMode) { + selectBuilder.join(finishJoin()); + return selectBuilder.lock(lockMode); + } + /* * (non-Javadoc) * @see org.springframework.data.relational.core.sql.SelectBuilder.BuildSelect#build() 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 new file mode 100644 index 0000000000..af6118473b --- /dev/null +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/LockMode.java @@ -0,0 +1,27 @@ +/* + * Copyright 2020 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.sql; + +/** + * Lock Mode Types of SELECT statements. + * + * @author Myeonghyeon Lee + * @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 new file mode 100644 index 0000000000..e2184ecaa1 --- /dev/null +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/LockOptions.java @@ -0,0 +1,34 @@ +/* + * Copyright 2020 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.sql; + +/** + * LockOptions has a LOCK option to apply to the Select statement. + * + * @author Myeonghyeon Lee + * @since 2.0 + */ +public class LockOptions { + private final LockMode lockMode; + + public LockOptions(LockMode lockMode) { + this.lockMode = lockMode; + } + + public LockMode getLockMode() { + return this.lockMode; + } +} diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/Select.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/Select.java index 1e0f971e12..ca79419ef5 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/Select.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/Select.java @@ -15,6 +15,8 @@ */ package org.springframework.data.relational.core.sql; +import org.springframework.lang.Nullable; + import java.util.List; import java.util.OptionalLong; @@ -30,6 +32,7 @@ * * * @author Mark Paluch + * @author Myeonghyeon Lee * @since 1.1 * @see StatementBuilder * @see SelectBuilder @@ -71,4 +74,7 @@ static SelectBuilder builder() { * @return */ boolean isDistinct(); + + @Nullable + LockMode getLockMode(); } diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/SelectBuilder.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/SelectBuilder.java index c31f288773..fcaf30f304 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/SelectBuilder.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/SelectBuilder.java @@ -22,6 +22,7 @@ * * @author Mark Paluch * @author Jens Schauder + * @author Myeonghyeon Lee * @since 1.1 * @see StatementBuilder */ @@ -211,9 +212,9 @@ interface SelectFrom extends BuildSelect { } /** - * Builder exposing {@code FROM}, {@code JOIN}, {@code WHERE} and {@code LIMIT/OFFSET} methods. + * Builder exposing {@code FROM}, {@code JOIN}, {@code WHERE}, {@code LIMIT/OFFSET} and {@code LOCK} methods. */ - interface SelectFromAndOrderBy extends SelectFrom, SelectOrdered, SelectLimitOffset, BuildSelect { + interface SelectFromAndOrderBy extends SelectFrom, SelectOrdered, SelectLimitOffset, SelectLock, BuildSelect { @Override SelectFromAndOrderBy limitOffset(long limit, long offset); @@ -247,9 +248,10 @@ interface SelectFromAndOrderBy extends SelectFrom, SelectOrdered, SelectLimitOff } /** - * Builder exposing {@code FROM}, {@code JOIN}, {@code WHERE} and {@code LIMIT/OFFSET} methods. + * Builder exposing {@code FROM}, {@code JOIN}, {@code WHERE}, {@code LIMIT/OFFSET} and {@code LOCK} methods. */ - interface SelectFromAndJoin extends SelectFromAndOrderBy, BuildSelect, SelectJoin, SelectWhere, SelectLimitOffset { + interface SelectFromAndJoin + extends SelectFromAndOrderBy, BuildSelect, SelectJoin, SelectWhere, SelectLimitOffset, SelectLock { /** * Declare a {@link Table} to {@code SELECT … FROM}. Multiple calls to this or other {@code from} methods keep @@ -315,10 +317,10 @@ interface SelectFromAndJoin extends SelectFromAndOrderBy, BuildSelect, SelectJoi } /** - * Builder exposing {@code FROM}, {@code WHERE}, {@code LIMIT/OFFSET}, and JOIN {@code AND} continuation methods. + * Builder exposing {@code FROM}, {@code WHERE}, {@code LIMIT/OFFSET}, JOIN {@code AND} and {@code LOCK} continuation methods. */ interface SelectFromAndJoinCondition - extends BuildSelect, SelectJoin, SelectWhere, SelectOnCondition, SelectLimitOffset { + extends BuildSelect, SelectJoin, SelectWhere, SelectOnCondition, SelectLimitOffset, SelectLock { /** * Apply {@code limit} and {@code offset} parameters to the select statement. To read the first 20 rows from start @@ -380,9 +382,9 @@ interface SelectLimitOffset { } /** - * Builder exposing {@code ORDER BY} methods. + * Builder exposing {@code ORDER BY} and {@code LOCK} methods. */ - interface SelectOrdered extends BuildSelect { + interface SelectOrdered extends SelectLock, BuildSelect { /** * Add one or more {@link Column columns} to order by. @@ -410,9 +412,9 @@ interface SelectOrdered extends BuildSelect { } /** - * Interface exposing {@code WHERE} methods. + * Interface exposing {@code WHERE}, {@code LOCK} methods. */ - interface SelectWhere extends SelectOrdered, BuildSelect { + interface SelectWhere extends SelectOrdered, SelectLock, BuildSelect { /** * Apply a {@code WHERE} clause. @@ -428,7 +430,7 @@ interface SelectWhere extends SelectOrdered, BuildSelect { /** * Interface exposing {@code AND}/{@code OR} combinator methods for {@code WHERE} {@link Condition}s. */ - interface SelectWhereAndOr extends SelectOrdered, BuildSelect { + interface SelectWhereAndOr extends SelectOrdered, SelectLock, BuildSelect { /** * Combine the previous {@code WHERE} {@link Condition} using {@code AND}. @@ -452,7 +454,7 @@ interface SelectWhereAndOr extends SelectOrdered, BuildSelect { /** * Interface exposing {@code JOIN} methods. */ - interface SelectJoin extends BuildSelect { + interface SelectJoin extends SelectLock, BuildSelect { /** * Declare a {@code JOIN} {@code table}. @@ -518,7 +520,7 @@ interface SelectOnConditionComparison { /** * Builder exposing JOIN and {@code JOIN … ON} continuation methods. */ - interface SelectOnCondition extends SelectJoin, BuildSelect { + interface SelectOnCondition extends SelectJoin, SelectLock, BuildSelect { /** * Declare an additional source column in the {@code JOIN}. @@ -530,6 +532,20 @@ interface SelectOnCondition extends SelectJoin, BuildSelect { SelectOnConditionComparison and(Expression column); } + /** + * Lock methods. + */ + interface SelectLock extends BuildSelect { + + /** + * Apply lock to read. + * + * @param lockMode lockMode to read. + * @return {@code this} builder. + */ + SelectLock lock(LockMode lockMode); + } + /** * Interface exposing the {@link Select} build method. */ diff --git a/spring-data-relational/src/test/java/org/springframework/data/relational/core/sql/SelectBuilderUnitTests.java b/spring-data-relational/src/test/java/org/springframework/data/relational/core/sql/SelectBuilderUnitTests.java index a71f92670e..66918f8328 100644 --- a/spring-data-relational/src/test/java/org/springframework/data/relational/core/sql/SelectBuilderUnitTests.java +++ b/spring-data-relational/src/test/java/org/springframework/data/relational/core/sql/SelectBuilderUnitTests.java @@ -27,6 +27,7 @@ * Unit tests for {@link SelectBuilder}. * * @author Mark Paluch + * @author Myeonghyeon Lee */ public class SelectBuilderUnitTests { @@ -147,4 +148,63 @@ public void joins() { assertThat(join.getType()).isEqualTo(JoinType.JOIN); } + @Test // DATAJDBC-498 + public void selectWithLock() { + + SelectBuilder builder = StatementBuilder.select(); + + Table table = SQL.table("mytable"); + Column foo = table.column("foo"); + Column bar = table.column("bar"); + LockMode lockMode = LockMode.PESSIMISTIC_WRITE; + + Select select = builder.select(foo, bar).from(table).lock(lockMode).build(); + + CapturingVisitor visitor = new CapturingVisitor(); + select.visit(visitor); + + assertThat(visitor.enter).containsSequence(foo, table, bar, table, new From(table), table); + assertThat(select.getLockMode()).isEqualTo(lockMode); + } + + @Test // DATAJDBC-498 + public void selectWithWhereWithLock() { + + SelectBuilder builder = StatementBuilder.select(); + + Table table = SQL.table("mytable"); + Column foo = table.column("foo"); + + Comparison condition = foo.isEqualTo(SQL.literalOf("bar")); + LockMode lockMode = LockMode.PESSIMISTIC_WRITE; + + Select select = builder.select(foo).from(table).where(condition).lock(lockMode).build(); + + CapturingVisitor visitor = new CapturingVisitor(); + select.visit(visitor); + + assertThat(visitor.enter).containsSequence(foo, table, new From(table), table, new Where(condition)); + assertThat(select.getLockMode()).isEqualTo(lockMode); + } + + @Test // DATAJDBC-498 + public void orderByWithLock() { + + SelectBuilder builder = StatementBuilder.select(); + + Table table = SQL.table("mytable"); + + Column foo = SQL.column("foo", table).as("foo"); + + OrderByField orderByField = OrderByField.from(foo).asc(); + LockMode lockMode = LockMode.PESSIMISTIC_WRITE; + + Select select = builder.select(foo).from(table).orderBy(orderByField).lock(lockMode).build(); + + CapturingVisitor visitor = new CapturingVisitor(); + select.visit(visitor); + + assertThat(visitor.enter).containsSequence(foo, table, new From(table), table, orderByField, foo); + assertThat(select.getLockMode()).isEqualTo(lockMode); + } } From b95cb065fc840a24c38414c40d3b9be935fa231e Mon Sep 17 00:00:00 2001 From: mhyeon-lee Date: Thu, 27 Feb 2020 00:39:18 +0900 Subject: [PATCH 3/8] DATAJDBC-498 Add afterFromTable method to SelectRenderContext for Postgres lock hint --- .../core/sql/render/SelectRenderContext.java | 11 +++++++++++ .../core/sql/render/SelectStatementVisitor.java | 3 +++ 2 files changed, 14 insertions(+) diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/SelectRenderContext.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/SelectRenderContext.java index ad1d3825b9..878f2bcc94 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/SelectRenderContext.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/SelectRenderContext.java @@ -25,6 +25,7 @@ * element without further whitespace processing. Hooks are responsible for adding required surrounding whitespaces. * * @author Mark Paluch + * @author Myeonghyeon Lee * @since 1.1 */ public interface SelectRenderContext { @@ -39,6 +40,16 @@ public interface SelectRenderContext { return select -> ""; } + /** + * Customization hook: Rendition of a part after {@code FROM} table. + * Renders an empty string by default. + * + * @return render {@link Function} invoked after rendering {@code FROM} table. + */ + default Function afterFromTable() { + return select -> ""; + } + /** * Customization hook: Rendition of a part after {@code ORDER BY}. The rendering function is called always, regardless * whether {@code ORDER BY} exists or not. Renders an empty string by default. diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/SelectStatementVisitor.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/SelectStatementVisitor.java index 935af6120f..825748f940 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/SelectStatementVisitor.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/SelectStatementVisitor.java @@ -28,6 +28,7 @@ * * @author Mark Paluch * @author Jens Schauder + * @author Myeonghyeon Lee * @since 1.1 */ class SelectStatementVisitor extends DelegatingVisitor implements PartRenderer { @@ -125,6 +126,8 @@ public Delegation doLeave(Visitable segment) { builder.append(" FROM ").append(from); } + builder.append(selectRenderContext.afterFromTable().apply(select)); + if (join.length() != 0) { builder.append(' ').append(join); } From 0b5e9181959fe1cb0d4f05a2cd3de1fde88fbafb Mon Sep 17 00:00:00 2001 From: mhyeon-lee Date: Thu, 27 Feb 2020 01:34:08 +0900 Subject: [PATCH 4/8] DATAJDBC-498 Add LockClause to Dialect --- .../jdbc/core/convert/NonQuotingDialect.java | 7 ++ .../data/jdbc/testing/AnsiDialect.java | 33 ++++++ .../core/dialect/AbstractDialect.java | 103 +++++++++++++++++- .../data/relational/core/dialect/Dialect.java | 8 ++ .../relational/core/dialect/H2Dialect.java | 32 ++++++ .../core/dialect/HsqlDbDialect.java | 21 ++++ .../relational/core/dialect/LockClause.java | 55 ++++++++++ .../relational/core/dialect/MySqlDialect.java | 42 +++++++ .../core/dialect/PostgresDialect.java | 42 +++++++ .../core/dialect/SqlServerDialect.java | 44 +++++++- .../dialect/SqlServerSelectRenderContext.java | 15 ++- .../core/dialect/HsqlDbDialectUnitTests.java | 13 +++ .../MySqlDialectRenderingUnitTests.java | 50 +++++++++ .../core/dialect/MySqlDialectUnitTests.java | 13 +++ .../PostgresDialectRenderingUnitTests.java | 50 +++++++++ .../dialect/PostgresDialectUnitTests.java | 13 +++ .../SqlServerDialectRenderingUnitTests.java | 80 ++++++++++++++ .../dialect/SqlServerDialectUnitTests.java | 13 +++ 18 files changed, 627 insertions(+), 7 deletions(-) create mode 100644 spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/LockClause.java diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/NonQuotingDialect.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/NonQuotingDialect.java index b1672e5f86..6185f85f97 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/NonQuotingDialect.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/NonQuotingDialect.java @@ -19,6 +19,7 @@ import org.springframework.data.relational.core.dialect.Dialect; import org.springframework.data.relational.core.dialect.HsqlDbDialect; import org.springframework.data.relational.core.dialect.LimitClause; +import org.springframework.data.relational.core.dialect.LockClause; import org.springframework.data.relational.core.sql.IdentifierProcessing; /** @@ -27,6 +28,7 @@ * @author Mark Paluch * @author Milan Milanov * @author Jens Schauder + * @author Myeonghyeon Lee */ public class NonQuotingDialect extends AbstractDialect implements Dialect { @@ -39,6 +41,11 @@ public LimitClause limit() { return HsqlDbDialect.INSTANCE.limit(); } + @Override + public LockClause lock() { + return HsqlDbDialect.INSTANCE.lock(); + } + @Override public IdentifierProcessing getIdentifierProcessing() { return IdentifierProcessing.create(new IdentifierProcessing.Quoting(""), IdentifierProcessing.LetterCasing.AS_IS); diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/testing/AnsiDialect.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/testing/AnsiDialect.java index abb1c84131..2d111648be 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/testing/AnsiDialect.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/testing/AnsiDialect.java @@ -20,7 +20,9 @@ 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; import org.springframework.util.ClassUtils; @@ -28,6 +30,7 @@ * An SQL dialect for the ANSI SQL standard. * * @author Milan Milanov + * @author Myeonghyeon Lee * @since 2.0 */ public class AnsiDialect extends AbstractDialect { @@ -78,6 +81,27 @@ public Position getClausePosition() { } }; + 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.LimitClause#getClausePosition() + */ + @Override + public Position getClausePosition() { + return Position.AFTER_ORDER_BY; + } + }; + private final AnsiArrayColumns ARRAY_COLUMNS = new AnsiArrayColumns(); /* @@ -89,6 +113,15 @@ public LimitClause limit() { return LIMIT_CLAUSE; } + /* + * (non-Javadoc) + * @see org.springframework.data.relational.core.dialect.Dialect#lock() + */ + @Override + public LockClause lock() { + return LOCK_CLAUSE; + } + /* * (non-Javadoc) * @see org.springframework.data.relational.core.dialect.Dialect#getArraySupport() 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 65671ad073..a14b035fc2 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 @@ -20,6 +20,8 @@ import java.util.OptionalLong; import java.util.function.Function; +import org.springframework.data.relational.core.sql.LockMode; +import org.springframework.data.relational.core.sql.LockOptions; import org.springframework.data.relational.core.sql.Select; import org.springframework.data.relational.core.sql.render.SelectRenderContext; @@ -27,6 +29,7 @@ * Base class for {@link Dialect} implementations. * * @author Mark Paluch + * @author Myeonghyeon Lee * @since 1.1 */ public abstract class AbstractDialect implements Dialect { @@ -38,9 +41,31 @@ public abstract class AbstractDialect implements Dialect { @Override public SelectRenderContext getSelectContext() { + Function afterFromTable = getAfterFromTable(); Function afterOrderBy = getAfterOrderBy(); - return new DialectSelectRenderContext(afterOrderBy); + return new DialectSelectRenderContext(afterFromTable, afterOrderBy); + } + + /** + * Returns a {@link Function afterFromTable Function}. Typically used for table hint for SQL Server. + * + * @return the {@link Function} called on {@code afterFromTable}. + */ + protected Function getAfterFromTable() { + + Function afterFromTable = select -> ""; + + LockClause lockClause = lock(); + switch (lockClause.getClausePosition()) { + + case AFTER_FROM_TABLE: + afterFromTable = new LockRenderFunction(lockClause); + + default: + } + + return afterFromTable.andThen(PrependWithLeadingWhitespace.INSTANCE); } /** @@ -50,21 +75,50 @@ public SelectRenderContext getSelectContext() { */ protected Function getAfterOrderBy() { - Function afterOrderBy; + 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(); + }; + } + + private Function getAfterOrderByLimit() { LimitClause limit = limit(); + Function afterOrderByLimit = select -> ""; + switch (limit.getClausePosition()) { case AFTER_ORDER_BY: - afterOrderBy = new AfterOrderByLimitRenderFunction(limit); + afterOrderByLimit = new AfterOrderByLimitRenderFunction(limit); break; default: throw new UnsupportedOperationException(String.format("Clause position %s not supported!", limit)); } - return afterOrderBy.andThen(PrependWithLeadingWhitespace.INSTANCE); + return afterOrderByLimit.andThen(PrependWithLeadingWhitespace.INSTANCE); + } + + private Function getAfterOrderByLock() { + LockClause lock = lock(); + + Function afterOrderByLock = select -> ""; + + switch (lock.getClausePosition()) { + + case AFTER_ORDER_BY: + afterOrderByLock = new LockRenderFunction(lock); + + default: + } + + return afterOrderByLock.andThen(PrependWithLeadingWhitespace.INSTANCE); } /** @@ -72,12 +126,26 @@ protected Function getAfterOrderBy() { */ class DialectSelectRenderContext implements SelectRenderContext { + private final Function afterFromTable; private final Function afterOrderBy; - DialectSelectRenderContext(Function afterOrderBy) { + DialectSelectRenderContext( + Function afterFromTable, + Function afterOrderBy) { + + this.afterFromTable = afterFromTable; this.afterOrderBy = afterOrderBy; } + /* + * (non-Javadoc) + * @see org.springframework.data.relational.core.sql.render.SelectRenderContext#afterFromTable() + */ + @Override + public Function afterFromTable() { + return afterFromTable; + } + /* * (non-Javadoc) * @see org.springframework.data.relational.core.sql.render.SelectRenderContext#afterOrderBy(boolean) @@ -122,6 +190,31 @@ public CharSequence apply(Select select) { } } + /** + * {@code LOCK} function rendering the {@link LockClause}. + */ + @RequiredArgsConstructor + static class LockRenderFunction implements Function { + + private final LockClause clause; + + /* + * (non-Javadoc) + * @see java.util.function.Function#apply(java.lang.Object) + */ + @Override + public CharSequence apply(Select select) { + + LockMode lockMode = select.getLockMode(); + + if (lockMode == null) { + return ""; + } + + return clause.getLock(new LockOptions(lockMode)); + } + } + /** * Prepends a non-empty rendering result with a leading whitespace, */ diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/Dialect.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/Dialect.java index 81cfa1a318..aa1bad1a38 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/Dialect.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/Dialect.java @@ -26,6 +26,7 @@ * * @author Mark Paluch * @author Jens Schauder + * @author Myeonghyeon Lee * @since 1.1 */ public interface Dialect { @@ -37,6 +38,13 @@ public interface Dialect { */ LimitClause limit(); + /** + * Return the {@link LockClause} used by this dialect. + * + * @return the {@link LockClause} used by this dialect. + */ + LockClause lock(); + /** * Returns the array support object that describes how array-typed columns are supported by this dialect. * 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 f8b261866e..f055befc2f 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,6 +20,7 @@ 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; @@ -27,6 +28,7 @@ * An SQL dialect for H2. * * @author Mark Paluch + * @author Myeonghyeon Lee * @since 2.0 */ public class H2Dialect extends AbstractDialect { @@ -77,6 +79,27 @@ public Position getClausePosition() { } }; + 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(); /* @@ -88,6 +111,15 @@ public LimitClause limit() { return LIMIT_CLAUSE; } + /* + * (non-Javadoc) + * @see org.springframework.data.relational.core.dialect.Dialect#lock() + */ + @Override + public LockClause lock() { + return LOCK_CLAUSE; + } + /* * (non-Javadoc) * @see org.springframework.data.relational.core.dialect.Dialect#getArraySupport() 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 59faf39138..b97dbfdf4e 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,10 +15,13 @@ */ package org.springframework.data.relational.core.dialect; +import org.springframework.data.relational.core.sql.LockOptions; + /** * A {@link Dialect} for HsqlDb. * * @author Jens Schauder + * @author Myeonghyeon Lee */ public class HsqlDbDialect extends AbstractDialect { @@ -31,6 +34,11 @@ public LimitClause limit() { return LIMIT_CLAUSE; } + @Override + public LockClause lock() { + return LOCK_CLAUSE; + } + private static final LimitClause LIMIT_CLAUSE = new LimitClause() { @Override @@ -53,4 +61,17 @@ public Position getClausePosition() { 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/LockClause.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/LockClause.java new file mode 100644 index 0000000000..e2a2880f23 --- /dev/null +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/LockClause.java @@ -0,0 +1,55 @@ +/* + * Copyright 2020 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.dialect; + +import org.springframework.data.relational.core.sql.LockOptions; + +/** + * A clause representing Dialect-specific {@code LOCK}. + * + * @author Myeonghyeon Lee + * @since 2.0 + */ +public interface LockClause { + + /** + * Returns the {@code LOCK} clause to lock results. + * + * @param lockOptions contains the lock mode to apply. + * @return rendered lock clause. + */ + String getLock(LockOptions lockOptions); + + /** + * Returns the {@link Position} where to apply the {@link #getLock(LockOptions) clause}. + */ + Position getClausePosition(); + + /** + * Enumeration of where to render the clause within the SQL statement. + */ + enum Position { + /** + * Append the clause after from table. + */ + AFTER_FROM_TABLE, + + /** + * Append the clause at the end of the statement. + */ + AFTER_ORDER_BY + } +} diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/MySqlDialect.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/MySqlDialect.java index 102f3416f4..6e4f2ed010 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/MySqlDialect.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/MySqlDialect.java @@ -19,12 +19,14 @@ import org.springframework.data.relational.core.sql.IdentifierProcessing.LetterCasing; import org.springframework.data.relational.core.sql.IdentifierProcessing.Quoting; import org.springframework.util.Assert; +import org.springframework.data.relational.core.sql.LockOptions; /** * A SQL dialect for MySQL. * * @author Mark Paluch * @author Jens Schauder + * @author Myeonghyeon Lee * @since 1.1 */ public class MySqlDialect extends AbstractDialect { @@ -102,6 +104,37 @@ public Position getClausePosition() { } }; + 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) { + switch (lockOptions.getLockMode()) { + + case PESSIMISTIC_WRITE: + return "FOR UPDATE"; + + case PESSIMISTIC_READ: + return "LOCK IN SHARE MODE"; + + default: + return ""; + } + } + + /* + * (non-Javadoc) + * @see org.springframework.data.relational.core.dialect.LockClause#getClausePosition() + */ + @Override + public Position getClausePosition() { + return Position.AFTER_ORDER_BY; + } + }; + /* * (non-Javadoc) * @see org.springframework.data.relational.core.dialect.Dialect#limit() @@ -111,6 +144,15 @@ public LimitClause limit() { return LIMIT_CLAUSE; } + /* + * (non-Javadoc) + * @see org.springframework.data.relational.core.dialect.Dialect#lock() + */ + @Override + public LockClause lock() { + return 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/PostgresDialect.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/PostgresDialect.java index 7ab45656fc..7dc730b402 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 @@ -20,6 +20,7 @@ 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; @@ -27,6 +28,7 @@ * An SQL dialect for Postgres. * * @author Mark Paluch + * @author Myeonghyeon Lee * @since 1.1 */ public class PostgresDialect extends AbstractDialect { @@ -77,6 +79,37 @@ public Position getClausePosition() { } }; + 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) { + switch (lockOptions.getLockMode()) { + + case PESSIMISTIC_WRITE: + return "FOR UPDATE"; + + case PESSIMISTIC_READ: + return "FOR SHARE"; + + default: + return ""; + } + } + + /* + * (non-Javadoc) + * @see org.springframework.data.relational.core.dialect.LockClause#getClausePosition() + */ + @Override + public Position getClausePosition() { + return Position.AFTER_ORDER_BY; + } + }; + private final PostgresArrayColumns ARRAY_COLUMNS = new PostgresArrayColumns(); /* @@ -88,6 +121,15 @@ public LimitClause limit() { return LIMIT_CLAUSE; } + /* + * (non-Javadoc) + * @see org.springframework.data.relational.core.dialect.Dialect#lock() + */ + @Override + public LockClause lock() { + return LOCK_CLAUSE; + } + /* * (non-Javadoc) * @see org.springframework.data.relational.core.dialect.Dialect#getArraySupport() diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/SqlServerDialect.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/SqlServerDialect.java index 038a300922..25f2f6712b 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/SqlServerDialect.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/SqlServerDialect.java @@ -15,6 +15,7 @@ */ package org.springframework.data.relational.core.dialect; +import org.springframework.data.relational.core.sql.LockOptions; import org.springframework.data.relational.core.sql.render.SelectRenderContext; import org.springframework.data.util.Lazy; @@ -22,6 +23,7 @@ * An SQL dialect for Microsoft SQL Server. * * @author Mark Paluch + * @author Myeonghyeon Lee * @since 1.1 */ public class SqlServerDialect extends AbstractDialect { @@ -72,8 +74,39 @@ public Position getClausePosition() { } }; + private static final LockClause LOCK_CLAUSE = new LockClause() { + + /* + * (non-Javadoc) + * @see org.springframework.data.relational.core.dialect.LockClause#getLimit(LockOptions) + */ + @Override + public String getLock(LockOptions lockOptions) { + switch (lockOptions.getLockMode()) { + + case PESSIMISTIC_WRITE: + return "WITH (UPDLOCK, ROWLOCK)"; + + case PESSIMISTIC_READ: + return "WITH (HOLDLOCK, ROWLOCK)"; + + default: + return ""; + } + } + + /* + * (non-Javadoc) + * @see org.springframework.data.relational.core.dialect.LimitClause#getClausePosition() + */ + @Override + public Position getClausePosition() { + return Position.AFTER_FROM_TABLE; + } + }; + private final Lazy selectRenderContext = Lazy - .of(() -> new SqlServerSelectRenderContext(getAfterOrderBy())); + .of(() -> new SqlServerSelectRenderContext(getAfterFromTable(), getAfterOrderBy())); /* * (non-Javadoc) @@ -84,6 +117,15 @@ public LimitClause limit() { return LIMIT_CLAUSE; } + /* + * (non-Javadoc) + * @see org.springframework.data.relational.core.dialect.Dialect#lock() + */ + @Override + public LockClause lock() { + return LOCK_CLAUSE; + } + /* * (non-Javadoc) * @see org.springframework.data.relational.core.dialect.Dialect#getLikeEscaper() 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 79f4dfec3a..76f2be1627 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 @@ -28,6 +28,7 @@ * * * @author Mark Paluch + * @author Myeonghyeon Lee */ public class SqlServerSelectRenderContext implements SelectRenderContext { @@ -36,14 +37,20 @@ public class SqlServerSelectRenderContext implements SelectRenderContext { private static final String SYNTHETIC_SELECT_LIST = ", ROW_NUMBER() over (ORDER BY (SELECT 1)) AS " + SYNTHETIC_ORDER_BY_FIELD; + private final Function afterFromTable; private final Function afterOrderBy; /** * Creates a new {@link SqlServerSelectRenderContext}. * + * @param afterFromTable the delegate {@code afterFromTable} function. * @param afterOrderBy the delegate {@code afterOrderBy} function. */ - protected SqlServerSelectRenderContext(Function afterOrderBy) { + protected SqlServerSelectRenderContext( + Function afterFromTable, + Function afterOrderBy) { + + this.afterFromTable = afterFromTable; this.afterOrderBy = afterOrderBy; } @@ -60,6 +67,12 @@ protected SqlServerSelectRenderContext(Function afterOrder }; } + @Override + public Function afterFromTable() { + + return afterFromTable; + } + @Override public Function afterOrderBy(boolean hasOrderBy) { diff --git a/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/HsqlDbDialectUnitTests.java b/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/HsqlDbDialectUnitTests.java index 818bf406ae..ab278a0d12 100644 --- a/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/HsqlDbDialectUnitTests.java +++ b/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/HsqlDbDialectUnitTests.java @@ -18,11 +18,14 @@ import static org.assertj.core.api.Assertions.*; import org.junit.Test; +import org.springframework.data.relational.core.sql.LockMode; +import org.springframework.data.relational.core.sql.LockOptions; /** * Unit tests for the {@link HsqlDbDialect}. * * @author Jens Schauder + * @author Myeonghyeon Lee */ public class HsqlDbDialectUnitTests { @@ -66,4 +69,14 @@ public void shouldQuoteIdentifiersUsingBackticks() { assertThat(abcQuoted).isEqualTo("\"abc\""); } + + @Test // DATAJDBC-498 + public void shouldRenderLock() { + + LockClause limit = HsqlDbDialect.INSTANCE.lock(); + LockOptions lockOptions = new LockOptions(LockMode.PESSIMISTIC_WRITE); + + assertThat(limit.getLock(lockOptions)).isEqualTo("FOR UPDATE"); + assertThat(limit.getClausePosition()).isEqualTo(LockClause.Position.AFTER_ORDER_BY); + } } diff --git a/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/MySqlDialectRenderingUnitTests.java b/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/MySqlDialectRenderingUnitTests.java index f2541ed8b1..9d660ff80f 100644 --- a/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/MySqlDialectRenderingUnitTests.java +++ b/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/MySqlDialectRenderingUnitTests.java @@ -20,6 +20,7 @@ import org.junit.Before; import org.junit.Test; +import org.springframework.data.relational.core.sql.LockMode; import org.springframework.data.relational.core.sql.Select; import org.springframework.data.relational.core.sql.StatementBuilder; import org.springframework.data.relational.core.sql.Table; @@ -31,6 +32,7 @@ * * @author Mark Paluch * @author Jens Schauder + * @author Myeonghyeon Lee */ public class MySqlDialectRenderingUnitTests { @@ -73,4 +75,52 @@ public void shouldRenderSelectWithLimitOffset() { assertThat(sql).isEqualTo("SELECT foo.* FROM foo LIMIT 20, 10"); } + + @Test // DATAJDBC-498 + public void shouldRenderSelectWithLockWrite() { + + Table table = Table.create("foo"); + LockMode lockMode = LockMode.PESSIMISTIC_WRITE; + Select select = StatementBuilder.select(table.asterisk()).from(table).lock(lockMode).build(); + + String sql = SqlRenderer.create(factory.createRenderContext()).render(select); + + assertThat(sql).isEqualTo("SELECT foo.* FROM foo FOR UPDATE"); + } + + @Test // DATAJDBC-498 + public void shouldRenderSelectWithLockRead() { + + Table table = Table.create("foo"); + LockMode lockMode = LockMode.PESSIMISTIC_READ; + Select select = StatementBuilder.select(table.asterisk()).from(table).lock(lockMode).build(); + + String sql = SqlRenderer.create(factory.createRenderContext()).render(select); + + assertThat(sql).isEqualTo("SELECT foo.* FROM foo LOCK IN SHARE MODE"); + } + + @Test // DATAJDBC-498 + public void shouldRenderSelectWithLimitWithLockWrite() { + + Table table = Table.create("foo"); + LockMode lockMode = LockMode.PESSIMISTIC_WRITE; + Select select = StatementBuilder.select(table.asterisk()).from(table).limit(10).lock(lockMode).build(); + + String sql = SqlRenderer.create(factory.createRenderContext()).render(select); + + assertThat(sql).isEqualTo("SELECT foo.* FROM foo LIMIT 10 FOR UPDATE"); + } + + @Test // DATAJDBC-498 + public void shouldRenderSelectWithLimitWithLockRead() { + + Table table = Table.create("foo"); + LockMode lockMode = LockMode.PESSIMISTIC_READ; + Select select = StatementBuilder.select(table.asterisk()).from(table).limit(10).lock(lockMode).build(); + + String sql = SqlRenderer.create(factory.createRenderContext()).render(select); + + assertThat(sql).isEqualTo("SELECT foo.* FROM foo LIMIT 10 LOCK IN SHARE MODE"); + } } diff --git a/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/MySqlDialectUnitTests.java b/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/MySqlDialectUnitTests.java index 5e6d94ac44..5799e57fbd 100644 --- a/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/MySqlDialectUnitTests.java +++ b/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/MySqlDialectUnitTests.java @@ -18,12 +18,15 @@ import static org.assertj.core.api.Assertions.*; import org.junit.Test; +import org.springframework.data.relational.core.sql.LockMode; +import org.springframework.data.relational.core.sql.LockOptions; /** * Unit tests for {@link MySqlDialect}. * * @author Mark Paluch * @author Jens Schauder + * @author Myeonghyeon Lee */ public class MySqlDialectUnitTests { @@ -67,4 +70,14 @@ public void shouldQuoteIdentifiersUsingBackticks() { assertThat(abcQuoted).isEqualTo("`abc`"); } + + @Test // DATAJDBC-498 + public void shouldRenderLock() { + + LockClause lock = MySqlDialect.INSTANCE.lock(); + + assertThat(lock.getLock(new LockOptions(LockMode.PESSIMISTIC_WRITE))).isEqualTo("FOR UPDATE"); + assertThat(lock.getLock(new LockOptions(LockMode.PESSIMISTIC_READ))).isEqualTo("LOCK IN SHARE MODE"); + assertThat(lock.getClausePosition()).isEqualTo(LockClause.Position.AFTER_ORDER_BY); + } } diff --git a/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/PostgresDialectRenderingUnitTests.java b/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/PostgresDialectRenderingUnitTests.java index 3ca3cd7591..73b1692f30 100644 --- a/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/PostgresDialectRenderingUnitTests.java +++ b/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/PostgresDialectRenderingUnitTests.java @@ -20,6 +20,7 @@ import org.junit.Before; import org.junit.Test; +import org.springframework.data.relational.core.sql.LockMode; import org.springframework.data.relational.core.sql.Select; import org.springframework.data.relational.core.sql.StatementBuilder; import org.springframework.data.relational.core.sql.Table; @@ -31,6 +32,7 @@ * * @author Mark Paluch * @author Jens Schauder + * @author Myeonghyeon Lee */ public class PostgresDialectRenderingUnitTests { @@ -97,4 +99,52 @@ public void shouldRenderSelectWithLimitOffset() { assertThat(sql).isEqualTo("SELECT foo.* FROM foo LIMIT 10 OFFSET 20"); } + + @Test // DATAJDBC-498 + public void shouldRenderSelectWithLockWrite() { + + Table table = Table.create("foo"); + LockMode lockMode = LockMode.PESSIMISTIC_WRITE; + Select select = StatementBuilder.select(table.asterisk()).from(table).lock(lockMode).build(); + + String sql = SqlRenderer.create(factory.createRenderContext()).render(select); + + assertThat(sql).isEqualTo("SELECT foo.* FROM foo FOR UPDATE"); + } + + @Test // DATAJDBC-498 + public void shouldRenderSelectWithLockRead() { + + Table table = Table.create("foo"); + LockMode lockMode = LockMode.PESSIMISTIC_READ; + Select select = StatementBuilder.select(table.asterisk()).from(table).lock(lockMode).build(); + + String sql = SqlRenderer.create(factory.createRenderContext()).render(select); + + assertThat(sql).isEqualTo("SELECT foo.* FROM foo FOR SHARE"); + } + + @Test // DATAJDBC-498 + public void shouldRenderSelectWithLimitWithLockWrite() { + + Table table = Table.create("foo"); + LockMode lockMode = LockMode.PESSIMISTIC_WRITE; + Select select = StatementBuilder.select(table.asterisk()).from(table).limit(10).lock(lockMode).build(); + + String sql = SqlRenderer.create(factory.createRenderContext()).render(select); + + assertThat(sql).isEqualTo("SELECT foo.* FROM foo LIMIT 10 FOR UPDATE"); + } + + @Test // DATAJDBC-498 + public void shouldRenderSelectWithLimitWithLockRead() { + + Table table = Table.create("foo"); + LockMode lockMode = LockMode.PESSIMISTIC_READ; + Select select = StatementBuilder.select(table.asterisk()).from(table).limit(10).lock(lockMode).build(); + + String sql = SqlRenderer.create(factory.createRenderContext()).render(select); + + assertThat(sql).isEqualTo("SELECT foo.* FROM foo LIMIT 10 FOR SHARE"); + } } diff --git a/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/PostgresDialectUnitTests.java b/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/PostgresDialectUnitTests.java index b65bad06f2..b843c62d4b 100644 --- a/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/PostgresDialectUnitTests.java +++ b/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/PostgresDialectUnitTests.java @@ -19,11 +19,14 @@ import static org.assertj.core.api.SoftAssertions.*; import org.junit.Test; +import org.springframework.data.relational.core.sql.LockMode; +import org.springframework.data.relational.core.sql.LockOptions; /** * Unit tests for {@link PostgresDialect}. * * @author Mark Paluch + * @author Myeonghyeon Lee */ public class PostgresDialectUnitTests { @@ -71,4 +74,14 @@ public void shouldRenderLimitOffset() { assertThat(limit.getLimitOffset(20, 10)).isEqualTo("LIMIT 20 OFFSET 10"); } + + @Test // DATAJDBC-498 + public void shouldRenderLock() { + + LockClause lock = PostgresDialect.INSTANCE.lock(); + + assertThat(lock.getLock(new LockOptions(LockMode.PESSIMISTIC_WRITE))).isEqualTo("FOR UPDATE"); + assertThat(lock.getLock(new LockOptions(LockMode.PESSIMISTIC_READ))).isEqualTo("FOR SHARE"); + assertThat(lock.getClausePosition()).isEqualTo(LockClause.Position.AFTER_ORDER_BY); + } } diff --git a/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/SqlServerDialectRenderingUnitTests.java b/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/SqlServerDialectRenderingUnitTests.java index e239793ca6..da78c1c0c7 100644 --- a/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/SqlServerDialectRenderingUnitTests.java +++ b/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/SqlServerDialectRenderingUnitTests.java @@ -20,6 +20,7 @@ import org.junit.Before; import org.junit.Test; +import org.springframework.data.relational.core.sql.LockMode; import org.springframework.data.relational.core.sql.Select; import org.springframework.data.relational.core.sql.StatementBuilder; import org.springframework.data.relational.core.sql.Table; @@ -31,6 +32,7 @@ * * @author Mark Paluch * @author Jens Schauder + * @author Myeonghyeon Lee */ public class SqlServerDialectRenderingUnitTests { @@ -112,4 +114,82 @@ public void shouldRenderSelectWithLimitOffsetAndOrderBy() { assertThat(sql).isEqualTo("SELECT foo.* FROM foo ORDER BY column_1 OFFSET 20 ROWS FETCH NEXT 10 ROWS ONLY"); } + + @Test // DATAJDBC-498 + public void shouldRenderSelectWithLockWrite() { + + Table table = Table.create("foo"); + LockMode lockMode = LockMode.PESSIMISTIC_WRITE; + Select select = StatementBuilder.select(table.asterisk()).from(table).lock(lockMode).build(); + + String sql = SqlRenderer.create(factory.createRenderContext()).render(select); + + assertThat(sql).isEqualTo( + "SELECT foo.* FROM foo WITH (UPDLOCK, ROWLOCK)"); + } + + @Test // DATAJDBC-498 + public void shouldRenderSelectWithLockRead() { + + Table table = Table.create("foo"); + LockMode lockMode = LockMode.PESSIMISTIC_READ; + Select select = StatementBuilder.select(table.asterisk()).from(table).lock(lockMode).build(); + + String sql = SqlRenderer.create(factory.createRenderContext()).render(select); + + assertThat(sql).isEqualTo( + "SELECT foo.* FROM foo WITH (HOLDLOCK, ROWLOCK)"); + } + + @Test // DATAJDBC-498 + public void shouldRenderSelectWithLimitOffsetWithLockWrite() { + + Table table = Table.create("foo"); + LockMode lockMode = LockMode.PESSIMISTIC_WRITE; + Select select = StatementBuilder.select(table.asterisk()).from(table).limit(10).offset(20).lock(lockMode).build(); + + String sql = SqlRenderer.create(factory.createRenderContext()).render(select); + + assertThat(sql).isEqualTo( + "SELECT foo.*, ROW_NUMBER() over (ORDER BY (SELECT 1)) AS __relational_row_number__ FROM foo WITH (UPDLOCK, ROWLOCK) ORDER BY __relational_row_number__ OFFSET 20 ROWS FETCH NEXT 10 ROWS ONLY"); + } + + @Test // DATAJDBC-498 + public void shouldRenderSelectWithLimitOffsetWithLockRead() { + + Table table = Table.create("foo"); + LockMode lockMode = LockMode.PESSIMISTIC_READ; + Select select = StatementBuilder.select(table.asterisk()).from(table).limit(10).offset(20).lock(lockMode).build(); + + String sql = SqlRenderer.create(factory.createRenderContext()).render(select); + + assertThat(sql).isEqualTo( + "SELECT foo.*, ROW_NUMBER() over (ORDER BY (SELECT 1)) AS __relational_row_number__ FROM foo WITH (HOLDLOCK, ROWLOCK) ORDER BY __relational_row_number__ OFFSET 20 ROWS FETCH NEXT 10 ROWS ONLY"); + } + + @Test // DATAJDBC-498 + public void shouldRenderSelectWithLimitOffsetAndOrderByWithLockWrite() { + + Table table = Table.create("foo"); + LockMode lockMode = LockMode.PESSIMISTIC_WRITE; + Select select = StatementBuilder.select(table.asterisk()).from(table).orderBy(table.column("column_1")).limit(10) + .offset(20).lock(lockMode).build(); + + String sql = SqlRenderer.create(factory.createRenderContext()).render(select); + + assertThat(sql).isEqualTo("SELECT foo.* FROM foo WITH (UPDLOCK, ROWLOCK) ORDER BY column_1 OFFSET 20 ROWS FETCH NEXT 10 ROWS ONLY"); + } + + @Test // DATAJDBC-498 + public void shouldRenderSelectWithLimitOffsetAndOrderByWithLockRead() { + + Table table = Table.create("foo"); + LockMode lockMode = LockMode.PESSIMISTIC_READ; + Select select = StatementBuilder.select(table.asterisk()).from(table).orderBy(table.column("column_1")).limit(10) + .offset(20).lock(lockMode).build(); + + String sql = SqlRenderer.create(factory.createRenderContext()).render(select); + + assertThat(sql).isEqualTo("SELECT foo.* FROM foo WITH (HOLDLOCK, ROWLOCK) ORDER BY column_1 OFFSET 20 ROWS FETCH NEXT 10 ROWS ONLY"); + } } diff --git a/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/SqlServerDialectUnitTests.java b/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/SqlServerDialectUnitTests.java index ff4c1b03d8..46f2f1554c 100644 --- a/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/SqlServerDialectUnitTests.java +++ b/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/SqlServerDialectUnitTests.java @@ -18,11 +18,14 @@ import static org.assertj.core.api.Assertions.*; import org.junit.Test; +import org.springframework.data.relational.core.sql.LockMode; +import org.springframework.data.relational.core.sql.LockOptions; /** * Unit tests for {@link SqlServerDialect}. * * @author Mark Paluch + * @author Myeonghyeon Lee */ public class SqlServerDialectUnitTests { @@ -59,4 +62,14 @@ public void shouldRenderLimitOffset() { assertThat(limit.getLimitOffset(20, 10)).isEqualTo("OFFSET 10 ROWS FETCH NEXT 20 ROWS ONLY"); } + + @Test // DATAJDBC-498 + public void shouldRenderLock() { + + LockClause lock = SqlServerDialect.INSTANCE.lock(); + + assertThat(lock.getLock(new LockOptions(LockMode.PESSIMISTIC_WRITE))).isEqualTo("WITH (UPDLOCK, ROWLOCK)"); + assertThat(lock.getLock(new LockOptions(LockMode.PESSIMISTIC_READ))).isEqualTo("WITH (HOLDLOCK, ROWLOCK)"); + assertThat(lock.getClausePosition()).isEqualTo(LockClause.Position.AFTER_FROM_TABLE); + } } From 7bb2beb63580b362b6f19f29a52b413694de1fc0 Mon Sep 17 00:00:00 2001 From: mhyeon-lee Date: Fri, 1 May 2020 03:22:17 +0900 Subject: [PATCH 5/8] [DATAJDBC-493] Add findOneWithLock for DataAccessStrategy --- .../convert/CascadingDataAccessStrategy.java | 11 +++++ .../jdbc/core/convert/DataAccessStrategy.java | 14 +++++++ .../convert/DefaultDataAccessStrategy.java | 20 +++++++++ .../convert/DelegatingDataAccessStrategy.java | 14 +++++++ .../data/jdbc/core/convert/SqlGenerator.java | 20 +++++++++ .../mybatis/MyBatisDataAccessStrategy.java | 13 ++++++ .../core/convert/SqlGeneratorUnitTests.java | 42 ++++++++++++++----- 7 files changed, 124 insertions(+), 10 deletions(-) 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 151897dabd..1b9530ddd0 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 @@ -24,6 +24,7 @@ import org.springframework.data.domain.Sort; import org.springframework.data.mapping.PersistentPropertyPath; import org.springframework.data.relational.core.mapping.RelationalPersistentProperty; +import org.springframework.data.relational.core.sql.LockMode; /** * Delegates each methods to the {@link DataAccessStrategy}s passed to the constructor in turn until the first that does @@ -33,6 +34,7 @@ * @author Mark Paluch * @author Tyler Van Gorder * @author Milan Milanov + * @author Myeonghyeon Lee * @since 1.1 */ public class CascadingDataAccessStrategy implements DataAccessStrategy { @@ -133,6 +135,15 @@ public T findById(Object id, Class domainType) { return collect(das -> das.findById(id, domainType)); } + /* + * (non-Javadoc) + * @see org.springframework.data.jdbc.core.DataAccessStrategy#findById(java.lang.Object, org.springframework.data.relational.core.sql.LockMode, java.lang.Class) + */ + @Override + public T findByIdWithLock(Object id, LockMode lockMode, Class domainType) { + return collect(das -> das.findByIdWithLock(id, lockMode, domainType)); + } + /* * (non-Javadoc) * @see org.springframework.data.jdbc.core.DataAccessStrategy#findAll(java.lang.Class) 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 9d66c25939..af00a0317a 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 @@ -23,6 +23,7 @@ import org.springframework.data.jdbc.core.JdbcAggregateOperations; import org.springframework.data.mapping.PersistentPropertyPath; import org.springframework.data.relational.core.mapping.RelationalPersistentProperty; +import org.springframework.data.relational.core.sql.LockMode; import org.springframework.lang.Nullable; /** @@ -33,6 +34,7 @@ * @author Jens Schauder * @author Tyler Van Gorder * @author Milan Milanov + * @author Myeonghyeon Lee */ public interface DataAccessStrategy extends RelationResolver { @@ -148,6 +150,18 @@ public interface DataAccessStrategy extends RelationResolver { @Nullable T findById(Object id, Class domainType); + /** + * Loads a single entity identified by type and id with lock. + * + * @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}. + * @param domainType the domain type of the entity. Must not be {@code null}. + * @param the type of the entity. + * @return Might return {@code null}. + */ + @Nullable + T findByIdWithLock(Object id, LockMode lockMode, Class domainType); + /** * Loads all entities of the given type. * 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 c112dd9bdf..c87fda316f 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 @@ -41,6 +41,7 @@ import org.springframework.data.relational.core.mapping.RelationalPersistentEntity; import org.springframework.data.relational.core.mapping.RelationalPersistentProperty; 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.RowMapper; import org.springframework.jdbc.core.namedparam.NamedParameterJdbcOperations; @@ -62,6 +63,7 @@ * @author Tom Hombergs * @author Tyler Van Gorder * @author Milan Milanov + * @author Myeonghyeon Lee * @since 1.1 */ public class DefaultDataAccessStrategy implements DataAccessStrategy { @@ -269,6 +271,24 @@ public T findById(Object id, Class domainType) { } } + /* + * (non-Javadoc) + * @see org.springframework.data.jdbc.core.DataAccessStrategy#findById(java.lang.Object, org.springframework.data.relational.core.sql.LockMode, java.lang.Class) + */ + @Override + @SuppressWarnings("unchecked") + public T findByIdWithLock(Object id, LockMode lockMode, Class domainType) { + + String findOneWithLockSql = sql(domainType).getFindOneWithLock(lockMode); + SqlIdentifierParameterSource parameter = createIdParameterSource(id, domainType); + + try { + return operations.queryForObject(findOneWithLockSql, parameter, (RowMapper) getEntityRowMapper(domainType)); + } catch (EmptyResultDataAccessException e) { + return null; + } + } + /* * (non-Javadoc) * @see org.springframework.data.jdbc.core.DataAccessStrategy#findAll(java.lang.Class) 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 25c1c92398..31d93fb690 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 @@ -19,6 +19,7 @@ import org.springframework.data.domain.Sort; import org.springframework.data.mapping.PersistentPropertyPath; import org.springframework.data.relational.core.mapping.RelationalPersistentProperty; +import org.springframework.data.relational.core.sql.LockMode; import org.springframework.util.Assert; /** @@ -28,6 +29,7 @@ * @author Jens Schauder * @author Tyler Van Gorder * @author Milan Milanov + * @author Myeonghyeon Lee * @since 1.1 */ public class DelegatingDataAccessStrategy implements DataAccessStrategy { @@ -128,6 +130,18 @@ public T findById(Object id, Class domainType) { return delegate.findById(id, domainType); } + /* + * (non-Javadoc) + * @see org.springframework.data.jdbc.core.DataAccessStrategy#findById(java.lang.Object, org.springframework.data.relational.core.sql.LockMode, java.lang.Class) + */ + @Override + public T findByIdWithLock(Object id, LockMode lockMode, Class domainType) { + + Assert.notNull(delegate, "Delegate is null"); + + return delegate.findByIdWithLock(id, lockMode, domainType); + } + /* * (non-Javadoc) * @see org.springframework.data.jdbc.core.DataAccessStrategy#findAll(java.lang.Class) 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 5578438035..c6bafb438d 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 @@ -52,6 +52,7 @@ * @author Tom Hombergs * @author Tyler Van Gorder * @author Milan Milanov + * @author Myeonghyeon Lee */ class SqlGenerator { @@ -257,6 +258,16 @@ String getFindOne() { return findOneSql.get(); } + /** + * Create a {@code SELECT … FROM … WHERE :id = … (LOCK CLAUSE)} statement. + * + * @param lockMode Lock clause mode. + * @return the statement as a {@link String}. Guaranteed to be not {@literal null}. + */ + String getFindOneWithLock(LockMode lockMode) { + return this.createFindOneWithLockSql(lockMode); + } + /** * Create a {@code INSERT INTO … (…) VALUES(…)} statement. * @@ -358,6 +369,15 @@ private String createFindOneSql() { return render(select); } + private String createFindOneWithLockSql(LockMode lockMode) { + + Select select = selectBuilder().where(getIdColumn().isEqualTo(getBindMarker(ID_SQL_PARAMETER))) // + .lock(lockMode) + .build(); + + return render(select); + } + private String createFindAllSql() { return render(selectBuilder().build()); } 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 1e36ec2083..007b0de472 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 @@ -41,6 +41,7 @@ import org.springframework.data.relational.core.mapping.RelationalMappingContext; import org.springframework.data.relational.core.mapping.RelationalPersistentProperty; 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.namedparam.NamedParameterJdbcOperations; import org.springframework.util.Assert; @@ -60,6 +61,7 @@ * @author Mark Paluch * @author Tyler Van Gorder * @author Milan Milanov + * @author Myeonghyeon Lee */ public class MyBatisDataAccessStrategy implements DataAccessStrategy { @@ -260,6 +262,17 @@ public T findById(Object id, Class domainType) { return sqlSession().selectOne(statement, parameter); } + /* + * (non-Javadoc) + * @see org.springframework.data.jdbc.core.DataAccessStrategy#findById(java.lang.Object, org.springframework.data.relational.core.sql.LockMode, java.lang.Class) + */ + @Override + public T findByIdWithLock(Object id, LockMode lockMode, Class domainType) { + String statement = namespace(domainType) + ".findByIdWithLock"; + MyBatisContext parameter = new MyBatisContext(id, null, domainType, Collections.emptyMap()); + return sqlSession().selectOne(statement, parameter); + } + /* * (non-Javadoc) * @see org.springframework.data.jdbc.core.DataAccessStrategy#findAll(java.lang.Class) 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 0f28329fc3..6b3a0989a8 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 @@ -46,6 +46,7 @@ import org.springframework.data.relational.core.mapping.RelationalPersistentEntity; import org.springframework.data.relational.core.mapping.RelationalPersistentProperty; import org.springframework.data.relational.core.sql.Aliased; +import org.springframework.data.relational.core.sql.LockMode; import org.springframework.data.relational.core.sql.SqlIdentifier; import org.springframework.data.relational.core.sql.Table; @@ -59,6 +60,7 @@ * @author Mark Paluch * @author Tom Hombergs * @author Milan Milanov + * @author Myeonghyeon Lee */ public class SqlGeneratorUnitTests { @@ -95,16 +97,36 @@ public void findOne() { 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"); + .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(); + } + + @Test // DATAJDBC-493 + public void findOneWithLock() { + + String sql = sqlGenerator.getFindOneWithLock(LockMode.PESSIMISTIC_WRITE); + + 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") // + .contains("FOR UPDATE") // + .doesNotContain("Element AS elements"); softAssertions.assertAll(); } From bf77aaa90168db0aee9ec05c363afe7eceb9bdd1 Mon Sep 17 00:00:00 2001 From: mhyeon-lee Date: Fri, 1 May 2020 04:15:20 +0900 Subject: [PATCH 6/8] [DATAJDBC-493] Add acquire lock before execute delete aggregate root --- .../data/jdbc/core/JdbcAggregateTemplate.java | 12 ++++++++++++ .../JdbcRepositoryConcurrencyIntegrationTests.java | 3 --- 2 files changed, 12 insertions(+), 3 deletions(-) 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 396515a170..8207cda20d 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 @@ -39,7 +39,9 @@ import org.springframework.data.relational.core.mapping.RelationalMappingContext; import org.springframework.data.relational.core.mapping.RelationalPersistentEntity; import org.springframework.data.relational.core.mapping.event.*; +import org.springframework.data.relational.core.sql.LockMode; import org.springframework.lang.Nullable; +import org.springframework.transaction.support.TransactionSynchronizationManager; import org.springframework.util.Assert; /** @@ -50,6 +52,7 @@ * @author Thomas Lang * @author Christoph Strobl * @author Milan Milanov + * @author Myeonghyeon Lee */ public class JdbcAggregateTemplate implements JdbcAggregateOperations { @@ -352,6 +355,9 @@ private void deleteTree(Object id, @Nullable T entity, Class domainType) entity = triggerBeforeDelete(entity, id, change); change.setEntity(entity); + // [DATAJDBC-493] Acquire Lock to avoid DeadLock + this.acquireLockIfActualTransactionActive(id, domainType); + executor.execute(change); triggerAfterDelete(entity, id, change); @@ -441,4 +447,10 @@ private T triggerBeforeDelete(@Nullable T aggregateRoot, Object id, MutableA return null; } + + private void acquireLockIfActualTransactionActive(Object id, Class domainType) { + if (TransactionSynchronizationManager.isActualTransactionActive()) { + this.accessStrategy.findByIdWithLock(id, LockMode.PESSIMISTIC_WRITE, domainType); + } + } } 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 ac38a53b6f..075f24cb4d 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 @@ -20,7 +20,6 @@ import lombok.Getter; import lombok.With; import org.junit.ClassRule; -import org.junit.Ignore; import org.junit.Rule; import org.junit.Test; import org.springframework.beans.factory.annotation.Autowired; @@ -36,7 +35,6 @@ import org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate; import org.springframework.test.annotation.IfProfileValue; import org.springframework.test.annotation.ProfileValueSourceConfiguration; -import org.springframework.test.context.ActiveProfiles; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.junit4.rules.SpringClassRule; import org.springframework.test.context.junit4.rules.SpringMethodRule; @@ -126,7 +124,6 @@ public void updateConcurrencyWithEmptyReferences() throws Exception { } @Test // DATAJDBC-493 - @Ignore("failing test") public void updateConcurrencyWithDelete() throws Exception { DummyEntity entity = createDummyEntity(); From 64bf8afbdba2ce4bfd80b03677ea50cb78c8b1f3 Mon Sep 17 00:00:00 2001 From: mhyeon-lee Date: Fri, 1 May 2020 06:06:49 +0900 Subject: [PATCH 7/8] [DATAJDBC-493] Fix PostgresDialect Lock clause with Root table name --- .../core/dialect/AbstractDialect.java | 2 +- .../core/dialect/PostgresDialect.java | 81 ++++++++++++------- .../relational/core/sql/DefaultSelect.java | 9 +++ .../data/relational/core/sql/From.java | 7 +- .../data/relational/core/sql/LockOptions.java | 8 +- .../data/relational/core/sql/Select.java | 2 + .../core/dialect/HsqlDbDialectUnitTests.java | 11 ++- .../core/dialect/MySqlDialectUnitTests.java | 11 ++- .../PostgresDialectRenderingUnitTests.java | 8 +- .../dialect/PostgresDialectUnitTests.java | 11 ++- .../dialect/SqlServerDialectUnitTests.java | 12 ++- 11 files changed, 110 insertions(+), 52 deletions(-) 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 a14b035fc2..35f1b39c9a 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 @@ -211,7 +211,7 @@ public CharSequence apply(Select select) { return ""; } - return clause.getLock(new LockOptions(lockMode)); + return clause.getLock(new LockOptions(lockMode, select.getFrom())); } } 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 7dc730b402..c0f786798c 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 @@ -17,13 +17,17 @@ import lombok.RequiredArgsConstructor; +import org.springframework.data.relational.core.sql.From; 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.data.relational.core.sql.Table; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; +import java.util.List; + /** * An SQL dialect for Postgres. * @@ -79,37 +83,6 @@ public Position getClausePosition() { } }; - 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) { - switch (lockOptions.getLockMode()) { - - case PESSIMISTIC_WRITE: - return "FOR UPDATE"; - - case PESSIMISTIC_READ: - return "FOR SHARE"; - - default: - return ""; - } - } - - /* - * (non-Javadoc) - * @see org.springframework.data.relational.core.dialect.LockClause#getClausePosition() - */ - @Override - public Position getClausePosition() { - return Position.AFTER_ORDER_BY; - } - }; - private final PostgresArrayColumns ARRAY_COLUMNS = new PostgresArrayColumns(); /* @@ -121,6 +94,8 @@ public LimitClause limit() { return LIMIT_CLAUSE; } + private final PostgresLockClause LOCK_CLAUSE = new PostgresLockClause(this.getIdentifierProcessing()); + /* * (non-Javadoc) * @see org.springframework.data.relational.core.dialect.Dialect#lock() @@ -139,6 +114,50 @@ public ArrayColumns getArraySupport() { return ARRAY_COLUMNS; } + static class PostgresLockClause implements LockClause { + private final IdentifierProcessing identifierProcessing; + + PostgresLockClause(IdentifierProcessing identifierProcessing) { + this.identifierProcessing = identifierProcessing; + } + + /* + * (non-Javadoc) + * @see org.springframework.data.relational.core.dialect.LockClause#getLock(LockOptions) + */ + @Override + public String getLock(LockOptions lockOptions) { + + List
tables = lockOptions.getFrom().getTables(); + if (tables.isEmpty()) { + return ""; + } + + String tableName = tables.get(0).getName().toSql(this.identifierProcessing); + + switch (lockOptions.getLockMode()) { + + case PESSIMISTIC_WRITE: + return "FOR UPDATE OF " + tableName; + + case PESSIMISTIC_READ: + return "FOR SHARE OF " + tableName; + + default: + return ""; + } + } + + /* + * (non-Javadoc) + * @see org.springframework.data.relational.core.dialect.LockClause#getClausePosition() + */ + @Override + public Position getClausePosition() { + return Position.AFTER_ORDER_BY; + } + }; + @RequiredArgsConstructor static class PostgresArrayColumns implements ArrayColumns { diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/DefaultSelect.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/DefaultSelect.java index 1d865dba3a..925d15c4f2 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/DefaultSelect.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/DefaultSelect.java @@ -56,6 +56,15 @@ class DefaultSelect implements Select { this.lockMode = lockMode; } + /* + * (non-Javadoc) + * @see org.springframework.data.relational.core.sql.Select#getFrom() + */ + @Override + public From getFrom() { + return this.from; + } + /* * (non-Javadoc) * @see org.springframework.data.relational.core.sql.Select#getOrderBy() diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/From.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/From.java index c8a21128ca..f7928e73fb 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/From.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/From.java @@ -16,6 +16,7 @@ package org.springframework.data.relational.core.sql; import java.util.Arrays; +import java.util.Collections; import java.util.List; import org.springframework.util.StringUtils; @@ -38,7 +39,11 @@ public class From extends AbstractSegment { super(tables.toArray(new Table[] {})); - this.tables = tables; + this.tables = Collections.unmodifiableList(tables); + } + + public List
getTables() { + return this.tables; } /* 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 e2184ecaa1..25f37e24e4 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 @@ -23,12 +23,18 @@ */ public class LockOptions { private final LockMode lockMode; + private final From from; - public LockOptions(LockMode lockMode) { + public LockOptions(LockMode lockMode, From from) { this.lockMode = lockMode; + this.from = from; } public LockMode getLockMode() { return this.lockMode; } + + public From getFrom() { + return this.from; + } } diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/Select.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/Select.java index ca79419ef5..bab2eb1290 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/Select.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/Select.java @@ -49,6 +49,8 @@ static SelectBuilder builder() { return new DefaultSelectBuilder(); } + From getFrom(); + /** * @return the {@link List} of {@link OrderByField ORDER BY} fields. */ diff --git a/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/HsqlDbDialectUnitTests.java b/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/HsqlDbDialectUnitTests.java index ab278a0d12..e7b6cfd4d7 100644 --- a/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/HsqlDbDialectUnitTests.java +++ b/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/HsqlDbDialectUnitTests.java @@ -15,15 +15,17 @@ */ package org.springframework.data.relational.core.dialect; -import static org.assertj.core.api.Assertions.*; - import org.junit.Test; +import org.springframework.data.relational.core.sql.From; import org.springframework.data.relational.core.sql.LockMode; import org.springframework.data.relational.core.sql.LockOptions; +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; + /** * Unit tests for the {@link HsqlDbDialect}. - * + * * @author Jens Schauder * @author Myeonghyeon Lee */ @@ -74,7 +76,8 @@ public void shouldQuoteIdentifiersUsingBackticks() { public void shouldRenderLock() { LockClause limit = HsqlDbDialect.INSTANCE.lock(); - LockOptions lockOptions = new LockOptions(LockMode.PESSIMISTIC_WRITE); + From from = mock(From.class); + LockOptions lockOptions = new LockOptions(LockMode.PESSIMISTIC_WRITE, from); assertThat(limit.getLock(lockOptions)).isEqualTo("FOR UPDATE"); assertThat(limit.getClausePosition()).isEqualTo(LockClause.Position.AFTER_ORDER_BY); diff --git a/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/MySqlDialectUnitTests.java b/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/MySqlDialectUnitTests.java index 5799e57fbd..f8170d5fe4 100644 --- a/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/MySqlDialectUnitTests.java +++ b/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/MySqlDialectUnitTests.java @@ -15,12 +15,14 @@ */ package org.springframework.data.relational.core.dialect; -import static org.assertj.core.api.Assertions.*; - import org.junit.Test; +import org.springframework.data.relational.core.sql.From; import org.springframework.data.relational.core.sql.LockMode; import org.springframework.data.relational.core.sql.LockOptions; +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; + /** * Unit tests for {@link MySqlDialect}. * @@ -75,9 +77,10 @@ public void shouldQuoteIdentifiersUsingBackticks() { public void shouldRenderLock() { LockClause lock = MySqlDialect.INSTANCE.lock(); + From from = mock(From.class); - assertThat(lock.getLock(new LockOptions(LockMode.PESSIMISTIC_WRITE))).isEqualTo("FOR UPDATE"); - assertThat(lock.getLock(new LockOptions(LockMode.PESSIMISTIC_READ))).isEqualTo("LOCK IN SHARE MODE"); + assertThat(lock.getLock(new LockOptions(LockMode.PESSIMISTIC_WRITE, from))).isEqualTo("FOR UPDATE"); + assertThat(lock.getLock(new LockOptions(LockMode.PESSIMISTIC_READ, from))).isEqualTo("LOCK IN SHARE MODE"); assertThat(lock.getClausePosition()).isEqualTo(LockClause.Position.AFTER_ORDER_BY); } } diff --git a/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/PostgresDialectRenderingUnitTests.java b/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/PostgresDialectRenderingUnitTests.java index 73b1692f30..0325a7a0c7 100644 --- a/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/PostgresDialectRenderingUnitTests.java +++ b/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/PostgresDialectRenderingUnitTests.java @@ -109,7 +109,7 @@ public void shouldRenderSelectWithLockWrite() { String sql = SqlRenderer.create(factory.createRenderContext()).render(select); - assertThat(sql).isEqualTo("SELECT foo.* FROM foo FOR UPDATE"); + assertThat(sql).isEqualTo("SELECT foo.* FROM foo FOR UPDATE OF foo"); } @Test // DATAJDBC-498 @@ -121,7 +121,7 @@ public void shouldRenderSelectWithLockRead() { String sql = SqlRenderer.create(factory.createRenderContext()).render(select); - assertThat(sql).isEqualTo("SELECT foo.* FROM foo FOR SHARE"); + assertThat(sql).isEqualTo("SELECT foo.* FROM foo FOR SHARE OF foo"); } @Test // DATAJDBC-498 @@ -133,7 +133,7 @@ public void shouldRenderSelectWithLimitWithLockWrite() { String sql = SqlRenderer.create(factory.createRenderContext()).render(select); - assertThat(sql).isEqualTo("SELECT foo.* FROM foo LIMIT 10 FOR UPDATE"); + assertThat(sql).isEqualTo("SELECT foo.* FROM foo LIMIT 10 FOR UPDATE OF foo"); } @Test // DATAJDBC-498 @@ -145,6 +145,6 @@ public void shouldRenderSelectWithLimitWithLockRead() { String sql = SqlRenderer.create(factory.createRenderContext()).render(select); - assertThat(sql).isEqualTo("SELECT foo.* FROM foo LIMIT 10 FOR SHARE"); + assertThat(sql).isEqualTo("SELECT foo.* FROM foo LIMIT 10 FOR SHARE OF foo"); } } diff --git a/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/PostgresDialectUnitTests.java b/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/PostgresDialectUnitTests.java index b843c62d4b..bd3678b7ad 100644 --- a/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/PostgresDialectUnitTests.java +++ b/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/PostgresDialectUnitTests.java @@ -17,10 +17,15 @@ import static org.assertj.core.api.Assertions.*; import static org.assertj.core.api.SoftAssertions.*; +import static org.mockito.Mockito.*; import org.junit.Test; +import org.springframework.data.relational.core.sql.From; import org.springframework.data.relational.core.sql.LockMode; import org.springframework.data.relational.core.sql.LockOptions; +import org.springframework.data.relational.core.sql.Table; + +import java.util.Collections; /** * Unit tests for {@link PostgresDialect}. @@ -79,9 +84,11 @@ public void shouldRenderLimitOffset() { public void shouldRenderLock() { LockClause lock = PostgresDialect.INSTANCE.lock(); + From from = mock(From.class); + when(from.getTables()).thenReturn(Collections.singletonList(Table.create("dummy_table"))); - assertThat(lock.getLock(new LockOptions(LockMode.PESSIMISTIC_WRITE))).isEqualTo("FOR UPDATE"); - assertThat(lock.getLock(new LockOptions(LockMode.PESSIMISTIC_READ))).isEqualTo("FOR SHARE"); + assertThat(lock.getLock(new LockOptions(LockMode.PESSIMISTIC_WRITE, from))).isEqualTo("FOR UPDATE OF dummy_table"); + assertThat(lock.getLock(new LockOptions(LockMode.PESSIMISTIC_READ, from))).isEqualTo("FOR SHARE OF dummy_table"); assertThat(lock.getClausePosition()).isEqualTo(LockClause.Position.AFTER_ORDER_BY); } } diff --git a/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/SqlServerDialectUnitTests.java b/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/SqlServerDialectUnitTests.java index 46f2f1554c..46452b18a4 100644 --- a/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/SqlServerDialectUnitTests.java +++ b/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/SqlServerDialectUnitTests.java @@ -15,12 +15,15 @@ */ package org.springframework.data.relational.core.dialect; -import static org.assertj.core.api.Assertions.*; - import org.junit.Test; +import org.springframework.data.relational.core.sql.From; import org.springframework.data.relational.core.sql.LockMode; import org.springframework.data.relational.core.sql.LockOptions; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.Mockito.mock; + /** * Unit tests for {@link SqlServerDialect}. * @@ -67,9 +70,10 @@ public void shouldRenderLimitOffset() { public void shouldRenderLock() { LockClause lock = SqlServerDialect.INSTANCE.lock(); + From from = mock(From.class); - assertThat(lock.getLock(new LockOptions(LockMode.PESSIMISTIC_WRITE))).isEqualTo("WITH (UPDLOCK, ROWLOCK)"); - assertThat(lock.getLock(new LockOptions(LockMode.PESSIMISTIC_READ))).isEqualTo("WITH (HOLDLOCK, ROWLOCK)"); + assertThat(lock.getLock(new LockOptions(LockMode.PESSIMISTIC_WRITE, from))).isEqualTo("WITH (UPDLOCK, ROWLOCK)"); + assertThat(lock.getLock(new LockOptions(LockMode.PESSIMISTIC_READ, from))).isEqualTo("WITH (HOLDLOCK, ROWLOCK)"); assertThat(lock.getClausePosition()).isEqualTo(LockClause.Position.AFTER_FROM_TABLE); } } From a2c0e31c18334eec67f4a43317c1c6419cbccf15 Mon Sep 17 00:00:00 2001 From: mhyeon-lee Date: Tue, 5 May 2020 00:25:22 +0900 Subject: [PATCH 8/8] DATAJDBC-493 Add DbAction AcquireLockRoot, AcquireLockAllRoot for delete execution. --- .../jdbc/core/AggregateChangeExecutor.java | 5 ++ .../JdbcAggregateChangeExecutionContext.java | 10 +++ .../data/jdbc/core/JdbcAggregateTemplate.java | 11 ---- .../convert/CascadingDataAccessStrategy.java | 27 +++++--- .../jdbc/core/convert/DataAccessStrategy.java | 29 +++++---- .../convert/DefaultDataAccessStrategy.java | 57 ++++++++++------- .../convert/DelegatingDataAccessStrategy.java | 28 +++++---- .../data/jdbc/core/convert/SqlGenerator.java | 40 ++++++++++-- .../mybatis/MyBatisDataAccessStrategy.java | 34 ++++++++--- .../core/convert/SqlGeneratorUnitTests.java | 25 +++++--- ...RepositoryConcurrencyIntegrationTests.java | 61 +++++++++++++++++++ .../relational/core/conversion/DbAction.java | 51 ++++++++++++++++ .../RelationalEntityDeleteWriter.java | 22 +++++-- ...RelationalEntityDeleteWriterUnitTests.java | 47 +++++++++++--- 14 files changed, 348 insertions(+), 99 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 24376bf899..edfb51edb5 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 @@ -27,6 +27,7 @@ * Executes an {@link MutableAggregateChange}. * * @author Jens Schauder + * @author Myeonghyeon Lee * @since 2.0 */ class AggregateChangeExecutor { @@ -77,6 +78,10 @@ private void execute(DbAction action, JdbcAggregateChangeExecutionContext exe executionContext.executeDeleteRoot((DbAction.DeleteRoot) action); } else if (action instanceof DbAction.DeleteAllRoot) { executionContext.executeDeleteAllRoot((DbAction.DeleteAllRoot) action); + } else if (action instanceof DbAction.AcquireLockRoot) { + executionContext.executeAcquireLock((DbAction.AcquireLockRoot) action); + } else if (action instanceof DbAction.AcquireLockAllRoot) { + executionContext.executeAcquireLockAllRoot((DbAction.AcquireLockAllRoot) action); } else { throw new RuntimeException("unexpected action"); } diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateChangeExecutionContext.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateChangeExecutionContext.java index f74cbdf916..bd67f5c4ba 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateChangeExecutionContext.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateChangeExecutionContext.java @@ -42,6 +42,7 @@ import org.springframework.data.relational.core.mapping.PersistentPropertyPathExtension; import org.springframework.data.relational.core.mapping.RelationalPersistentEntity; import org.springframework.data.relational.core.mapping.RelationalPersistentProperty; +import org.springframework.data.relational.core.sql.LockMode; import org.springframework.data.util.Pair; import org.springframework.lang.Nullable; import org.springframework.util.Assert; @@ -49,6 +50,7 @@ /** * @author Jens Schauder * @author Umut Erturk + * @author Myeonghyeon Lee */ class JdbcAggregateChangeExecutionContext { @@ -164,6 +166,14 @@ void executeMerge(DbAction.Merge merge) { } } + void executeAcquireLock(DbAction.AcquireLockRoot acquireLock) { + accessStrategy.acquireLockById(acquireLock.getId(), LockMode.PESSIMISTIC_WRITE, acquireLock.getEntityType()); + } + + void executeAcquireLockAllRoot(DbAction.AcquireLockAllRoot acquireLock) { + accessStrategy.acquireLockAll(LockMode.PESSIMISTIC_WRITE, acquireLock.getEntityType()); + } + private void add(DbActionExecutionResult result) { results.put(result.getAction(), result); } 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 8207cda20d..8b1c28fdd8 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 @@ -39,9 +39,7 @@ import org.springframework.data.relational.core.mapping.RelationalMappingContext; import org.springframework.data.relational.core.mapping.RelationalPersistentEntity; import org.springframework.data.relational.core.mapping.event.*; -import org.springframework.data.relational.core.sql.LockMode; import org.springframework.lang.Nullable; -import org.springframework.transaction.support.TransactionSynchronizationManager; import org.springframework.util.Assert; /** @@ -355,9 +353,6 @@ private void deleteTree(Object id, @Nullable T entity, Class domainType) entity = triggerBeforeDelete(entity, id, change); change.setEntity(entity); - // [DATAJDBC-493] Acquire Lock to avoid DeadLock - this.acquireLockIfActualTransactionActive(id, domainType); - executor.execute(change); triggerAfterDelete(entity, id, change); @@ -447,10 +442,4 @@ private T triggerBeforeDelete(@Nullable T aggregateRoot, Object id, MutableA return null; } - - private void acquireLockIfActualTransactionActive(Object id, Class domainType) { - if (TransactionSynchronizationManager.isActualTransactionActive()) { - this.accessStrategy.findByIdWithLock(id, LockMode.PESSIMISTIC_WRITE, domainType); - } - } } 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 1b9530ddd0..c9cad5f3e6 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 @@ -119,29 +119,38 @@ public void deleteAll(PersistentPropertyPath prope /* * (non-Javadoc) - * @see org.springframework.data.jdbc.core.DataAccessStrategy#count(java.lang.Class) + * @see org.springframework.data.jdbc.core.DataAccessStrategy#acquireLockById(java.lang.Object, org.springframework.data.relational.core.sql.LockMode, java.lang.Class) */ @Override - public long count(Class domainType) { - return collect(das -> das.count(domainType)); + public void acquireLockById(Object id, LockMode lockMode, Class domainType) { + collectVoid(das -> das.acquireLockById(id, lockMode, domainType)); } /* * (non-Javadoc) - * @see org.springframework.data.jdbc.core.DataAccessStrategy#findById(java.lang.Object, java.lang.Class) + * @see org.springframework.data.jdbc.core.DataAccessStrategy#acquireLockAll(org.springframework.data.relational.core.sql.LockMode, java.lang.Class) */ @Override - public T findById(Object id, Class domainType) { - return collect(das -> das.findById(id, domainType)); + public void acquireLockAll(LockMode lockMode, Class domainType) { + collectVoid(das -> das.acquireLockAll(lockMode, domainType)); + } + + /* + * (non-Javadoc) + * @see org.springframework.data.jdbc.core.DataAccessStrategy#count(java.lang.Class) + */ + @Override + public long count(Class domainType) { + return collect(das -> das.count(domainType)); } /* * (non-Javadoc) - * @see org.springframework.data.jdbc.core.DataAccessStrategy#findById(java.lang.Object, org.springframework.data.relational.core.sql.LockMode, java.lang.Class) + * @see org.springframework.data.jdbc.core.DataAccessStrategy#findById(java.lang.Object, java.lang.Class) */ @Override - public T findByIdWithLock(Object id, LockMode lockMode, Class domainType) { - return collect(das -> das.findByIdWithLock(id, lockMode, domainType)); + public T findById(Object id, Class domainType) { + return collect(das -> das.findById(id, domainType)); } /* 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 af00a0317a..c69ee14a71 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 @@ -131,6 +131,23 @@ public interface DataAccessStrategy extends RelationResolver { */ void deleteAll(PersistentPropertyPath propertyPath); + /** + * Acquire Lock + * + * @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}. + * @param domainType the domain type of the entity. Must not be {@code null}. + */ + void acquireLockById(Object id, LockMode lockMode, Class domainType); + + /** + * Acquire Lock entities 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}. + */ + void acquireLockAll(LockMode lockMode, Class domainType); + /** * Counts the rows in the table representing the given domain type. * @@ -150,18 +167,6 @@ public interface DataAccessStrategy extends RelationResolver { @Nullable T findById(Object id, Class domainType); - /** - * Loads a single entity identified by type and id with lock. - * - * @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}. - * @param domainType the domain type of the entity. Must not be {@code null}. - * @param the type of the entity. - * @return Might return {@code null}. - */ - @Nullable - T findByIdWithLock(Object id, LockMode lockMode, Class domainType); - /** * Loads all entities of the given type. * 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 c87fda316f..80c6fe4b07 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,8 @@ import static org.springframework.data.jdbc.core.convert.SqlGenerator.*; import java.sql.JDBCType; +import java.sql.ResultSet; +import java.sql.SQLException; import java.util.ArrayList; import java.util.Collections; import java.util.HashSet; @@ -25,10 +27,7 @@ import java.util.Map; import java.util.function.Predicate; -import org.springframework.dao.DataRetrievalFailureException; -import org.springframework.dao.EmptyResultDataAccessException; -import org.springframework.dao.InvalidDataAccessApiUsageException; -import org.springframework.dao.OptimisticLockingFailureException; +import org.springframework.dao.*; import org.springframework.data.domain.Pageable; import org.springframework.data.domain.Sort; import org.springframework.data.jdbc.support.JdbcUtil; @@ -43,6 +42,7 @@ 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.ResultSetExtractor; import org.springframework.jdbc.core.RowMapper; import org.springframework.jdbc.core.namedparam.NamedParameterJdbcOperations; import org.springframework.jdbc.core.namedparam.SqlParameterSource; @@ -239,6 +239,27 @@ public void deleteAll(PersistentPropertyPath prope .update(sql(propertyPath.getBaseProperty().getOwner().getType()).createDeleteAllSql(propertyPath)); } + /* + * (non-Javadoc) + * @see org.springframework.data.jdbc.core.DataAccessStrategy#acquireLockById(java.lang.Object, org.springframework.data.relational.core.sql.LockMode, java.lang.Class) + */ + @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); + } + + /* + * (non-Javadoc) + * @see org.springframework.data.jdbc.core.DataAccessStrategy#acquireLockAll(org.springframework.data.relational.core.sql.LockMode, java.lang.Class) + */ + @Override + public void acquireLockAll(LockMode lockMode, Class domainType) { + String acquireLockAllSql = sql(domainType).getAcquireLockAll(lockMode); + operations.query(acquireLockAllSql, Collections.emptyMap(), new NoMappingResultSetExtractor()); + } + /* * (non-Javadoc) * @see org.springframework.data.jdbc.core.DataAccessStrategy#count(java.lang.Class) @@ -271,24 +292,6 @@ public T findById(Object id, Class domainType) { } } - /* - * (non-Javadoc) - * @see org.springframework.data.jdbc.core.DataAccessStrategy#findById(java.lang.Object, org.springframework.data.relational.core.sql.LockMode, java.lang.Class) - */ - @Override - @SuppressWarnings("unchecked") - public T findByIdWithLock(Object id, LockMode lockMode, Class domainType) { - - String findOneWithLockSql = sql(domainType).getFindOneWithLock(lockMode); - SqlIdentifierParameterSource parameter = createIdParameterSource(id, domainType); - - try { - return operations.queryForObject(findOneWithLockSql, parameter, (RowMapper) getEntityRowMapper(domainType)); - } catch (EmptyResultDataAccessException e) { - return null; - } - } - /* * (non-Javadoc) * @see org.springframework.data.jdbc.core.DataAccessStrategy#findAll(java.lang.Class) @@ -602,4 +605,14 @@ public T getBean() { 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/core/convert/DelegatingDataAccessStrategy.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DelegatingDataAccessStrategy.java index 31d93fb690..bb5b7d2558 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 @@ -111,35 +111,41 @@ public void deleteAll(PersistentPropertyPath prope /* * (non-Javadoc) - * @see org.springframework.data.jdbc.core.DataAccessStrategy#count(java.lang.Class) + * @see org.springframework.data.jdbc.core.DataAccessStrategy#acquireLockById(java.lang.Object, org.springframework.data.relational.core.sql.LockMode, java.lang.Class) */ @Override - public long count(Class domainType) { - return delegate.count(domainType); + public void acquireLockById(Object id, LockMode lockMode, Class domainType) { + delegate.acquireLockById(id, lockMode, domainType); } /* * (non-Javadoc) - * @see org.springframework.data.jdbc.core.DataAccessStrategy#findById(java.lang.Object, java.lang.Class) + * @see org.springframework.data.jdbc.core.DataAccessStrategy#acquireLockAll(org.springframework.data.relational.core.sql.LockMode, java.lang.Class) */ @Override - public T findById(Object id, Class domainType) { - - Assert.notNull(delegate, "Delegate is null"); + public void acquireLockAll(LockMode lockMode, Class domainType) { + delegate.acquireLockAll(lockMode, domainType); + } - return delegate.findById(id, domainType); + /* + * (non-Javadoc) + * @see org.springframework.data.jdbc.core.DataAccessStrategy#count(java.lang.Class) + */ + @Override + public long count(Class domainType) { + return delegate.count(domainType); } /* * (non-Javadoc) - * @see org.springframework.data.jdbc.core.DataAccessStrategy#findById(java.lang.Object, org.springframework.data.relational.core.sql.LockMode, java.lang.Class) + * @see org.springframework.data.jdbc.core.DataAccessStrategy#findById(java.lang.Object, java.lang.Class) */ @Override - public T findByIdWithLock(Object id, LockMode lockMode, Class domainType) { + public T findById(Object id, Class domainType) { Assert.notNull(delegate, "Delegate is null"); - return delegate.findByIdWithLock(id, lockMode, domainType); + return delegate.findById(id, domainType); } /* 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 c6bafb438d..7e5a798eaa 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 @@ -259,13 +259,23 @@ String getFindOne() { } /** - * Create a {@code SELECT … FROM … WHERE :id = … (LOCK CLAUSE)} statement. + * Create a {@code SELECT count(id) FROM … WHERE :id = … (LOCK CLAUSE)} statement. * * @param lockMode Lock clause mode. * @return the statement as a {@link String}. Guaranteed to be not {@literal null}. */ - String getFindOneWithLock(LockMode lockMode) { - return this.createFindOneWithLockSql(lockMode); + String getAcquireLockById(LockMode lockMode) { + return this.createAcquireLockById(lockMode); + } + + /** + * Create a {@code SELECT count(id) FROM … (LOCK CLAUSE)} statement. + * + * @param lockMode Lock clause mode. + * @return the statement as a {@link String}. Guaranteed to be not {@literal null}. + */ + String getAcquireLockAll(LockMode lockMode) { + return this.createAcquireLockAll(lockMode); } /** @@ -369,10 +379,28 @@ private String createFindOneSql() { return render(select); } - private String createFindOneWithLockSql(LockMode lockMode) { + private String createAcquireLockById(LockMode lockMode) { - Select select = selectBuilder().where(getIdColumn().isEqualTo(getBindMarker(ID_SQL_PARAMETER))) // - .lock(lockMode) + Table table = this.getTable(); + + Select select = StatementBuilder // + .select(getIdColumn()) // + .from(table) // + .where(getIdColumn().isEqualTo(getBindMarker(ID_SQL_PARAMETER))) // + .lock(lockMode) // + .build(); + + return render(select); + } + + private String createAcquireLockAll(LockMode lockMode) { + + Table table = this.getTable(); + + Select select = StatementBuilder // + .select(getIdColumn()) // + .from(table) // + .lock(lockMode) // .build(); return render(select); 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 007b0de472..0ac9e2aa15 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 @@ -26,6 +26,7 @@ import org.mybatis.spring.SqlSessionTemplate; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.springframework.dao.EmptyResultDataAccessException; import org.springframework.data.domain.Pageable; import org.springframework.data.domain.Sort; import org.springframework.data.jdbc.core.convert.CascadingDataAccessStrategy; @@ -252,23 +253,40 @@ public void deleteAll(PersistentPropertyPath prope /* * (non-Javadoc) - * @see org.springframework.data.jdbc.core.DataAccessStrategy#findById(java.lang.Object, java.lang.Class) + * @see org.springframework.data.jdbc.core.DataAccessStrategy#acquireLockById(java.lang.Object, org.springframework.data.relational.core.sql.LockMode, java.lang.Class) */ @Override - public T findById(Object id, Class domainType) { - - String statement = namespace(domainType) + ".findById"; + public void acquireLockById(Object id, LockMode lockMode, Class domainType) { + String statement = namespace(domainType) + ".acquireLockById"; MyBatisContext parameter = new MyBatisContext(id, null, domainType, Collections.emptyMap()); - return sqlSession().selectOne(statement, parameter); + + 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); + } + } + + /* + * (non-Javadoc) + * @see org.springframework.data.jdbc.core.DataAccessStrategy#acquireLockAll(org.springframework.data.relational.core.sql.LockMode, java.lang.Class) + */ + @Override + public void acquireLockAll(LockMode lockMode, Class domainType) { + String statement = namespace(domainType) + ".acquireLockAll"; + MyBatisContext parameter = new MyBatisContext(null, null, domainType, Collections.emptyMap()); + + sqlSession().selectOne(statement, parameter); } /* * (non-Javadoc) - * @see org.springframework.data.jdbc.core.DataAccessStrategy#findById(java.lang.Object, org.springframework.data.relational.core.sql.LockMode, java.lang.Class) + * @see org.springframework.data.jdbc.core.DataAccessStrategy#findById(java.lang.Object, java.lang.Class) */ @Override - public T findByIdWithLock(Object id, LockMode lockMode, Class domainType) { - String statement = namespace(domainType) + ".findByIdWithLock"; + public T findById(Object id, Class domainType) { + + String statement = namespace(domainType) + ".findById"; MyBatisContext parameter = new MyBatisContext(id, null, domainType, Collections.emptyMap()); return sqlSession().selectOne(statement, parameter); } 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 6b3a0989a8..d3cc9d2508 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 @@ -111,25 +111,34 @@ public void findOne() { } @Test // DATAJDBC-493 - public void findOneWithLock() { + public void getAcquireLockById() { - String sql = sqlGenerator.getFindOneWithLock(LockMode.PESSIMISTIC_WRITE); + String sql = sqlGenerator.getAcquireLockById(LockMode.PESSIMISTIC_WRITE); 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("dummy_entity.id1") // .contains("WHERE dummy_entity.id1 = :id") // .contains("FOR UPDATE") // .doesNotContain("Element AS elements"); softAssertions.assertAll(); } + @Test // DATAJDBC-493 + public void getAcquireLockAll() { + + 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(); + } + @Test // DATAJDBC-112 public void cascadingDeleteFirstLevel() { 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 075f24cb4d..e12a762bdb 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 @@ -187,6 +187,67 @@ public void updateConcurrencyWithDelete() throws Exception { assertThat(repository.findById(entity.id)).isEmpty(); } + @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); + + 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(() -> { + try { + + startLatch.countDown(); + startLatch.await(); + + transactionTemplate.execute(status -> { + repository.deleteAll(); + return null; + }); + } 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) { List concurrencyEntities = new ArrayList<>(); 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 28c3b1e6d1..142277d107 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 @@ -32,6 +32,7 @@ * @author Jens Schauder * @author Mark Paluch * @author Tyler Van Gorder + * @author Myeonghyeon Lee */ public interface DbAction { @@ -319,6 +320,56 @@ public String toString() { } } + /** + * Represents an acquire lock statement for a aggregate root when only the ID is known. + *

+ * + * @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) { + this.id = id; + this.entityType = entityType; + } + + public Object getId() { + return this.id; + } + + public Class getEntityType() { + return this.entityType; + } + + public String toString() { + return "DbAction.AcquireLockRoot(id=" + this.getId() + ", entityType=" + this.getEntityType() + ")"; + } + } + + /** + * Represents an acquire lock statement for all entities that that a reachable via a give path from any aggregate root 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) { + this.entityType = entityType; + } + + public Class getEntityType() { + return this.entityType; + } + + public String toString() { + return "DbAction.AcquireLockAllRoot(entityType=" + this.getEntityType() + ")"; + } + } + /** * An action depending on another action for providing additional information like the id of a parent entity. * 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 64af552a42..138aa01441 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 @@ -36,6 +36,7 @@ * @author Mark Paluch * @author Bastian Wilhelm * @author Tyler Van Gorder + * @author Myeonghyeon Lee */ public class RelationalEntityDeleteWriter implements EntityWriter> { @@ -68,12 +69,18 @@ public void write(@Nullable Object id, MutableAggregateChange aggregateChange private List> deleteAll(Class entityType) { - List> actions = new ArrayList<>(); + List> deleteReferencedActions = new ArrayList<>(); context.findPersistentPropertyPaths(entityType, PersistentProperty::isEntity) - .filter(p -> !p.getRequiredLeafProperty().isEmbedded()).forEach(p -> actions.add(new DbAction.DeleteAll<>(p))); + .filter(p -> !p.getRequiredLeafProperty().isEmbedded()).forEach(p -> deleteReferencedActions.add(new DbAction.DeleteAll<>(p))); - Collections.reverse(actions); + Collections.reverse(deleteReferencedActions); + + List> actions = new ArrayList<>(); + if (!deleteReferencedActions.isEmpty()) { + actions.add(new DbAction.AcquireLockAllRoot<>(entityType)); + } + actions.addAll(deleteReferencedActions); DbAction.DeleteAllRoot result = new DbAction.DeleteAllRoot<>(entityType); actions.add(result); @@ -83,7 +90,14 @@ private List> deleteAll(Class entityType) { private List> deleteRoot(Object id, AggregateChange aggregateChange) { - List> actions = new ArrayList<>(deleteReferencedEntities(id, aggregateChange)); + List> deleteReferencedActions = deleteReferencedEntities(id, aggregateChange); + + List> actions = new ArrayList<>(); + if (!deleteReferencedActions.isEmpty()) { + actions.add(new DbAction.AcquireLockRoot<>(id, aggregateChange.getEntityType())); + } + actions.addAll(deleteReferencedActions); + actions.add(new DbAction.DeleteRoot<>(id, aggregateChange.getEntityType(), getVersion(aggregateChange))); return actions; diff --git a/spring-data-relational/src/test/java/org/springframework/data/relational/core/conversion/RelationalEntityDeleteWriterUnitTests.java b/spring-data-relational/src/test/java/org/springframework/data/relational/core/conversion/RelationalEntityDeleteWriterUnitTests.java index 09e8191619..81fabaad7a 100644 --- a/spring-data-relational/src/test/java/org/springframework/data/relational/core/conversion/RelationalEntityDeleteWriterUnitTests.java +++ b/spring-data-relational/src/test/java/org/springframework/data/relational/core/conversion/RelationalEntityDeleteWriterUnitTests.java @@ -16,26 +16,23 @@ package org.springframework.data.relational.core.conversion; import lombok.Data; - -import java.util.ArrayList; -import java.util.List; - import org.assertj.core.api.Assertions; import org.assertj.core.groups.Tuple; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.junit.MockitoJUnitRunner; import org.springframework.data.annotation.Id; -import org.springframework.data.relational.core.conversion.DbAction.Delete; -import org.springframework.data.relational.core.conversion.DbAction.DeleteAll; -import org.springframework.data.relational.core.conversion.DbAction.DeleteAllRoot; -import org.springframework.data.relational.core.conversion.DbAction.DeleteRoot; +import org.springframework.data.relational.core.conversion.DbAction.*; import org.springframework.data.relational.core.mapping.RelationalMappingContext; +import java.util.ArrayList; +import java.util.List; + /** * Unit tests for the {@link org.springframework.data.relational.core.conversion.RelationalEntityDeleteWriter} * * @author Jens Schauder + * @author Myeonghyeon Lee */ @RunWith(MockitoJUnitRunner.class) public class RelationalEntityDeleteWriterUnitTests { @@ -54,12 +51,27 @@ public void deleteDeletesTheEntityAndReferencedEntities() { Assertions.assertThat(extractActions(aggregateChange)) .extracting(DbAction::getClass, DbAction::getEntityType, DbActionTestSupport::extractPath) // .containsExactly( // + Tuple.tuple(AcquireLockRoot.class, SomeEntity.class, ""), // Tuple.tuple(Delete.class, YetAnother.class, "other.yetAnother"), // Tuple.tuple(Delete.class, OtherEntity.class, "other"), // Tuple.tuple(DeleteRoot.class, SomeEntity.class, "") // ); } + @Test // DATAJDBC-493 + public void deleteDeletesTheEntityAndNoReferencedEntities() { + + SingleEntity entity = new SingleEntity(23L); + + MutableAggregateChange aggregateChange = MutableAggregateChange.forDelete(SingleEntity.class, entity); + + converter.write(entity.id, aggregateChange); + + Assertions.assertThat(extractActions(aggregateChange)) + .extracting(DbAction::getClass, DbAction::getEntityType, DbActionTestSupport::extractPath) // + .containsExactly(Tuple.tuple(DeleteRoot.class, SingleEntity.class, "")); + } + @Test // DATAJDBC-188 public void deleteAllDeletesAllEntitiesAndReferencedEntities() { @@ -70,12 +82,25 @@ public void deleteAllDeletesAllEntitiesAndReferencedEntities() { Assertions.assertThat(extractActions(aggregateChange)) .extracting(DbAction::getClass, DbAction::getEntityType, DbActionTestSupport::extractPath) // .containsExactly( // + Tuple.tuple(AcquireLockAllRoot.class, SomeEntity.class, ""), // Tuple.tuple(DeleteAll.class, YetAnother.class, "other.yetAnother"), // Tuple.tuple(DeleteAll.class, OtherEntity.class, "other"), // Tuple.tuple(DeleteAllRoot.class, SomeEntity.class, "") // ); } + @Test // DATAJDBC-493 + public void deleteAllDeletesAllEntitiesAndNoReferencedEntities() { + + MutableAggregateChange aggregateChange = MutableAggregateChange.forDelete(SingleEntity.class, null); + + converter.write(null, aggregateChange); + + Assertions.assertThat(extractActions(aggregateChange)) + .extracting(DbAction::getClass, DbAction::getEntityType, DbActionTestSupport::extractPath) // + .containsExactly(Tuple.tuple(DeleteAllRoot.class, SingleEntity.class, "")); + } + private List> extractActions(MutableAggregateChange aggregateChange) { List> actions = new ArrayList<>(); @@ -103,4 +128,10 @@ private class OtherEntity { private class YetAnother { @Id final Long id; } + + @Data + private class SingleEntity { + @Id final Long id; + String name; + } }