alibaba / alibaba/p3c

@Transactional 指定 rollbackFor?我认为没必要。

Open
#799 7 comments 0 reactions 0 assignees View on GitHub
Dominant language
Kotlin
Stars
30.8k
Forks
8k
PR merge metrics
No merged PRs in 30d

Description

每当使用`@Transactional`的时候该规约都提示我需要指定rollbackFor,但我认为是没必要的。理由如下:
Spring默认对Runtime和Error回滚无可厚非,毕竟无需显示处理。但是如果是Exception,不是需要人为的做处理吗?既然人为处理,又要分两种情况:
1. catch了不做任何异常抛出,这样显然不用回滚;
2. catch了又抛出Runtime,这种不用说肯定会回滚;
3. catch了但又抛出了Exception,这种默认Spring不回滚;
4. 不catch直接抛出Exception这种也是不回滚,与3同理。

再回到问题本身来看:
1. Runtime和Error的情况Spring已经帮我们做了指定,没有必要重复指定。
2. 而Exception则是根据业务来决定是否需要回滚,如果需要可以转换成Runtime这种Spring默认做了回滚,再去指定rollbackFor纯粹是多余。Spring默认不对Exception处理就是让你自己做选择,而现在到好,不管三七二十一全都需要设置。**当然如果说你抛出Exception,并且还希望回滚,这个时候可以让规约提醒你是否需要回滚,否则不处理。**总之,要不要指定rollbackFor,规约应该只在抛出Exception时候才去提醒,而不是只要用了`@Transactional`就去提醒。

当然,讨论本问题的本质不是说要不要指定rollbackFor,而是我们应该站在自身业务的角度去思考,我们应不应指定。一般来说对于方法内抛出的Exception(如楼下的IOException、SQLException)是不应该再抛到外层让调用方处理,调用方也是无法处理的。

另外我注意到此问题有其它提问#518,并且我也看到回复是针对jdbc 层面的说法,希望该规约(插件也需要更新)能区分Spring-TX的情况,可以删除对该注解的提示,因为该注解本身就是Spring提供的,无需显示指定。
```xml



```

Contributor guide

No contributing guide indexed for this repository

Research direction

Start by locating the rule that emits the `java.exception.TransactionMustHaveRollbackRule.violation.msg.simple` message and review the related discussion in issue #518, especially the Spring-TX distinction. Done means the rule no longer flags every `@Transactional` annotation, while the requested handling for exceptions and any required plugin update are covered.

Written by the indexing model from the issue text.

Assessment

Tech stack
java, spring
Domain
devtools
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.