From f9615d072dad95a51cc3383a3787c21ffaa489be Mon Sep 17 00:00:00 2001 From: guqing <38999863+guqing@users.noreply.github.com> Date: Thu, 22 Aug 2024 16:26:55 +0800 Subject: [PATCH] chore: remove transactional annotation (#6492) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #### What type of PR is this? /kind cleanup /area core /milestone 2.19.x #### What this PR does / why we need it: 移除事务注解避免对索引创建产生影响，原因参考改动中的方法注释 **其中一点特别注意：** 在执行 `client.create(name, data)` 方法后，会尝试进行 `indexer.indexRecord` 操作。但 indexRecord 可能会因唯一索引中存在重复键而导致 indexRecord 失败，索引创建也会随之失败。为确保索引与数据的一致性，此时应回滚由 `client.create(name, data)` 对数据产生的影响，因此除非找到更佳的一致性问题解决方案，否则暂时不能移除此处的手动事务操作。 ```java return client.create(name, version, data) .map(updated -> converter.convertFrom(type, updated)) .doOnNext(extension -> indexer.indexRecord(convertToRealExtension(extension))) .as(transactionalOperator::transactional); ``` 将变更传递给 extension watcher 是在 `doCreate` 或 `doUpdate` 成功之后才会被处理，因此这里的事务回滚不会对 watcher 造成影响 #### Does this PR introduce a user-facing change? ```release-note None ``` --- .../extension/service/UserServiceImpl.java | 2 - .../ReactiveExtensionClientImpl.java | 41 +++++++++++++++++++ .../ReactiveExtensionStoreClientImpl.java | 2 - .../migration/impl/MigrationServiceImpl.java | 2 - 4 files changed, 41 insertions(+), 6 deletions(-) diff --git a/application/src/main/java/run/halo/app/core/extension/service/UserServiceImpl.java b/application/src/main/java/run/halo/app/core/extension/service/UserServiceImpl.java index 1924b37b2..eb5594637 100644 --- a/application/src/main/java/run/halo/app/core/extension/service/UserServiceImpl.java +++ b/application/src/main/java/run/halo/app/core/extension/service/UserServiceImpl.java @@ -18,7 +18,6 @@ import org.springframework.dao.OptimisticLockingFailureException; import org.springframework.data.domain.Sort; import org.springframework.security.crypto.password.PasswordEncoder; import org.springframework.stereotype.Service; -import org.springframework.transaction.annotation.Transactional; import org.springframework.util.Assert; import org.springframework.util.CollectionUtils; import org.springframework.util.StringUtils; @@ -147,7 +146,6 @@ public class UserServiceImpl implements UserService { } @Override - @Transactional public Mono signUp(User user, String password) { if (!StringUtils.hasText(password)) { throw new IllegalArgumentException("Password must not be blank"); diff --git a/application/src/main/java/run/halo/app/extension/ReactiveExtensionClientImpl.java b/application/src/main/java/run/halo/app/extension/ReactiveExtensionClientImpl.java index 5096bb1de..7187a96b9 100644 --- a/application/src/main/java/run/halo/app/extension/ReactiveExtensionClientImpl.java +++ b/application/src/main/java/run/halo/app/extension/ReactiveExtensionClientImpl.java @@ -312,6 +312,44 @@ public class ReactiveExtensionClientImpl implements ReactiveExtensionClient { return this.indexedQueryEngine; } + /** + *

Note of transactional:

+ *

doSomething does not have a transaction, but methodOne and methodTwo have their own + * transactions.

+ *

If methodTwo fails and throws an exception, the transaction in methodTwo will be rolled + * back, but the transaction in methodOne will not.

+ * 
+     * public void doSomething() {
+     *     // with manual transaction
+     *     methodOne();
+     *     // with manual transaction
+     *     methodTwo();
+     * }
+     *
+ *

If doSomething is annotated with @Transactional, both methodOne and methodTwo will be + * executed within the same transaction context.

+ *

If methodTwo fails and throws an exception, the entire transaction will be rolled back, + * including any changes made by methodOne.

+ *

This ensures that all operations within the doSomething method either succeed or fail + * together.

+ *

This example advises against adding transaction annotations to the outer method that + * invokes {@link #update(Extension)} or {@link #create(Extension)}, as doing so could + * undermine the intended transactional integrity of this method.

+ * + *

Note another point:

+ * After executing the {@code client.create(name, data)} method, an attempt is made to + * indexRecord. However, indexRecord might fail due to duplicate keys in the unique index, + * causing the index creation to fail. In such cases, the data created by {@code client + * .create(name, data)} should be rolled back to maintain consistency between the index and + * the data. + *

Until a better solution is found for this consistency problem, do not remove the + * manual transaction here.

+ * 
+     * client.create(name, data)
+     *  .doOnNext(extension -> indexer.indexRecord(convertToRealExtension(extension)))
+     *  .as(transactionalOperator::transactional);
+     *
+ */ @SuppressWarnings("unchecked") Mono doCreate(E oldExtension, String name, byte[] data) { return Mono.defer(() -> { @@ -325,6 +363,9 @@ public class ReactiveExtensionClientImpl implements ReactiveExtensionClient { }); } + /** + * see also {@link #doCreate(Extension, String, byte[])}. + */ @SuppressWarnings("unchecked") Mono doUpdate(E oldExtension, String name, Long version, byte[] data) { return Mono.defer(() -> { diff --git a/application/src/main/java/run/halo/app/extension/store/ReactiveExtensionStoreClientImpl.java b/application/src/main/java/run/halo/app/extension/store/ReactiveExtensionStoreClientImpl.java index 8e7b4ce8a..465245e28 100644 --- a/application/src/main/java/run/halo/app/extension/store/ReactiveExtensionStoreClientImpl.java +++ b/application/src/main/java/run/halo/app/extension/store/ReactiveExtensionStoreClientImpl.java @@ -8,7 +8,6 @@ import org.springframework.data.domain.Page; import org.springframework.data.domain.PageImpl; import org.springframework.data.domain.Pageable; import org.springframework.stereotype.Component; -import org.springframework.transaction.annotation.Transactional; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; import run.halo.app.infra.exception.DuplicateNameException; @@ -61,7 +60,6 @@ public class ReactiveExtensionStoreClientImpl implements ReactiveExtensionStoreC } @Override - @Transactional public Mono delete(String name, Long version) { return repository.findById(name) .flatMap(extensionStore -> { diff --git a/application/src/main/java/run/halo/app/migration/impl/MigrationServiceImpl.java b/application/src/main/java/run/halo/app/migration/impl/MigrationServiceImpl.java index 6d21b50ba..700bbd076 100644 --- a/application/src/main/java/run/halo/app/migration/impl/MigrationServiceImpl.java +++ b/application/src/main/java/run/halo/app/migration/impl/MigrationServiceImpl.java @@ -29,7 +29,6 @@ import org.springframework.core.io.FileSystemResource; import org.springframework.core.io.Resource; import org.springframework.core.io.buffer.DataBuffer; import org.springframework.stereotype.Service; -import org.springframework.transaction.annotation.Transactional; import org.springframework.util.StringUtils; import org.springframework.web.server.ServerWebInputException; import reactor.core.Exceptions; @@ -136,7 +135,6 @@ public class MigrationServiceImpl implements MigrationService, InitializingBean } @Override - @Transactional public Mono restore(Publisher content) { return Mono.usingWhen( createTempDir("halo-restore-", scheduler),