发现Spring事务的一个实锤bug,官方还拒不承认?你来评评理...
你好呀,我是歪歪。
事情是这样的,上周我正在全神贯注的摸鱼,然后有个小伙伴给我发来微信消息,提出了自己关于事务的一个疑问,并配上两段代码:
先说结论:我认为这是 Spring 事务的一个 bug。但是官方说这只能算是文档上的缺陷,不能算是代码的 bug。
(好吧,我这篇文章写了好几天,所以我写到上面这一句的时候,官方还不承认是 bug,但是写完之后他们也承认确实是代码缺陷。不影响,接着往下看。)
好家伙,我懂了,一切解释权归官方所有。
在开始刨根问底之前,我想先就关于如何提问这个问题掰扯几句。
我把上面这个读者的问题截出来,是因为我觉得这个提问简直就是模板方法般的提问。
给了一段示例代码、给了一段源码、说明自己的问题、并表明自己已经查询过但是没有找到合适的答案。
我读完他的文字,我很快就能 get 到他的问题是什么,所以我们之间的交流就非常的高效。
最终我们并没有讨论出一个合理的解释,于是他去提了一个 issues,希望能得到官方的比较权威的回答。
所以我们的故事就围绕着这个 issues 开始吧。
舞台搭建
在正戏开始之前,我先给你把舞台搭建出来,也就是把 Demo 搞出来。
因为是关于 Spring 事务的问题嘛,所以这个 Demo 主要就是体现出“事务”的应用就行了嘛。
所以 Demo 里面最核心的东西就是这个部分:
其中涉及到的两个异常就是简单的自定义异常:
假设这里有一个只允许 10-18 岁的用户使用奇怪的网站,这个部分就代表这个网站的用户注册功能。
接着我们往 user_info 表里面插入一条数据,然后判断年龄如果大于 18 岁,那么抛出 AgeExceptionOver18 异常,表示这个用户不是目标用户。
但是你注意,我 @Transactional 注解里面的 rollbackFor 是 AgeException.class,意思是我并不想回滚这个大于 18 岁的用户,我还是想把他的注册信息保存下来,只是抛出一个异常来表示他不是我的目标用户,
而当我们插入一个年龄小于 10 岁的用户的时候,会抛出 AgeException.class,应该把刚刚执行的插入语句给回滚掉,我并不想保存这部分用户的信息。
好的,那么现在就会有小伙伴问了:小于 10 岁的用户既然不想保存,那么为什么不在插入之前判断呢?
很好的问题,实际开发中肯定是要在插入之前判断的,但是我这里只是为了演示事务功能,所以我就这样写了,咋地了吧!
上面的代码,我来搞个接口触发一下,也就是弄个 Controller 出来:
上面的四个类,就是最关键的几个类,所以我单独拿出来说一下。
整个项目结构也非常的简单:
其他的类不关键,就不拿出来说了,都是你最拿手的 crud。花五分钟搭一个这个项目出来不过分吧?中间还能摸两分钟的鱼。
我把日志级别调整为 debug 级别,接着把项目跑起来验证一下功能。
然后调用这个链接:
http://127.0.0.1:8085/insertUser?age=8
对应的日志是这样的:
可以看到我框起来的部分,首先确认执行了 insert 语句,且 age 确实是为 8。但是最后 Rolling back 了,即回滚了。
为什么回滚,我们心里也是门清,因为这里呼应上了:
接下来试一下 age 为 18 岁的用户:
http://127.0.0.1:8085/insertUser?age=18
对应的日志是这样的:
这没啥说的,事务成功提交,数据插入成功。是我们预期的结果。
数据库数据也没毛病:
然后试一下 age 为 28 岁的用户。
这个用户我们的预期是抛出 AgeExceptionOver18 异常,但是数据得插入成功。
来走一个:
http://127.0.0.1:8085/insertUser?age=28
对应的日志是这样的:
首先数据居然回滚了???
异常倒是抛出来了,但是这也没呼应上啊!
先不管到底啥原理吧,从我的认知来说,首先我的 @Transactional 注解用法绝对没有错,事务配置没有绝对没有错,我的异常也没有乱抛,你凭什么给我回滚了?
你还说这不是 bug?
just 改改 documentation 就行了?
言外之意是要“抵赖”,强行从文档上找补吗?
这不是欺负老实人吗?
额...等等,我写这段的时候情况是这样的。
但是等我写完这段,第二天再次进 issues 里面去看,发现事情发生了变化,官方又承认这是一个 bug 了,会在 5.3.x 版本里面修改文档上的描述,会在 6.0 版本里面进行代码上的修复。
但是我前面已经铺垫了这么多,已经写好了,我就不改了,就当在这里留下一个创作痕迹吧,哈哈。
我们接着往下看。
戏剧冲突
一部戏,肯定有它的戏剧冲突,这是它的核心部分。那么我们 Demo 里面的核心冲突是什么呢?
这一小节就先告诉你“戏剧冲突”在哪。
我先问你一个问题:
Spring 管理的事务,默认回滚的异常是什么呢?
我们带着这个问题去看源码,找到了这个问题的答案,你就能丝滑入戏。
先搞个断点,把程序跑起来,然后看调用栈:
可以看到调用栈里面和事务相关的有这样一个方法:
org.springframework.transaction.interceptor.TransactionAspectSupport#invokeWithinTransaction
这就是我们的突破口。
什么,你问我怎么一下就找到了这里来的?
我只能说:熟能生巧而已。
好吧,其实是有技巧的,你可以自己试着去找一下,因为这不是本文重点,所以我就不多说了。
方法执行异常之后,会走到 catch 代码块里面,下面这一行代码就是异常相关处理的入口:
在我们 age=28 的这个场景下,这个方法进来之后,首先 ex 参数就是我们自定义的 AgeExceptionOver18 异常:
我还框起来了一行代码:
txInfo.transactionAttribute.rollbackOn(ex)
这一行代码你看名字 rollbackOn 也知道是判断 ex 参数是否匹配指定的回滚异常。
如果匹配呢?如果不匹配呢?
如果匹配,rollback。
如果不匹配,commit。
好了,我们接着往下看。
你会走到这里来:
org.springframework.transaction.interceptor.RuleBasedTransactionAttribute#rollbackOn
而这个方法,当 winner 为 null 的时候,上面有个注释,是说没有匹配到对应的规则。
也就是我们什么都不配置,默认情况下,winner 就是 null。
那么下面这行代码里面就藏着我们要找的问题的答案:
return super.rollbackOn(ex);
所以 Spring 管理的事务,默认回滚的异常是什么呢?
源码告诉我:如果当前抛出的异常属于 RuntimeException 或者 Error 都会回滚。
前面都是我在铺路,只是为了把你引到 rollbackOn 方法这个地方来,甚至 super.rollbackOn(ex) 这行代码都是烟雾弹,本文中我们完全不必关注。
我们需要关注的,是这个部分:
首先我们明确一下这个 rollbackRules 是啥玩意。
在我们的 Demo 里面,它就是我们配置在 @Transactional 注解上 rollbackFor 属性中的 AgeException.class 对象的全路径名称:
好,接下来就关键了,你一定要打起精神来。
重点关注这一行代码:
int depth = rule.getDepth(ex);
来,看一下 rule 和 ex 分别是什么东西:
rule 里面的 exceptionName 是我们配置的 AgeException 对象的全路径名称。
ex 是我们程序抛出的 AgeExceptionOver18 异常。
这场戏的核心冲突,就藏在这里的这个方法里面:
org.springframework.transaction.interceptor.RollbackRuleAttribute#getDepth(java.lang.Throwable)
好,至此,舞台搭建完成,核心冲突已经暴露出来。
好戏准备开场。
大幕拉开
你仔细看我框起来的代码。
前面的 exceptionClass.getName() 是啥玩意?
它长这样:
后面的 this.exceptionName 是啥玩意?
它是这个玩意:
接下来,神奇的事情就要发生了,铁子。
com.example.transactional.exception.AgeExceptionOver18
com.example.transactional.exception.AgeException
虽然这是两个不同的异常,但是这两个字符串进行 contains 操作,你说是不是返回 true?
所以,这里的 depth 会返回 0。
那么这里的 winner 不会为空
因此这个方法的返回值就会是 true:
还记得我前面说的吗,这里返回 true 会执行什么代码?
是不是就 rollback 回滚了?
所以,万恶之源就是我们大幕拉开的时候就提到的这一段代码:
org.springframework.transaction.interceptor.RollbackRuleAttribute#getDepth(java.lang.Class<?>, int)
到这里,我觉得已经非常明确了:这难道不是 bug 吗?你强如 Spring 难道还想狡辩?
但是,如果下面这两个字符串进行 equals 操作,你说是不是返回 false,问题就得到解决了?
com.example.transactional.exception.AgeExceptionOver18
com.example.transactional.exception.AgeException
道理是这么个道理,但是我觉得问题肯定没这么简单。
首先我觉得这里用 contains 肯定是故意的,但是具体出于什么目的,我还真不确定。
于是和我讨论的读者提出一个看法,会不会是为了满足 rollbackForClassName 这个属性:
因为当我们用 rollbackForClassName 的时候可以用字符串数组的形式去配置多个需要回滚的异常名称,比如我搞个 NullPointerException:
在正常使用的场景下,我们是可以完成回滚操作的。
对应地方的代码的值是这样的;
java.lang.NullPointerException 字符串当然包含了 NullPointerException 字符串。所以我们进行回滚嘛。没毛病。
但是如果我们用 equals 操作,那么就匹配不上,导致 rollbackForClassName 属性失效了。
所以把 contains 修改为 equals 属于拆西墙,补东墙的措施,不可取。
但是 rollbackForClassName 属性在我们的 Demo 下,也是没有效果的。
比如我把程序改成这样,你说,是不是就乱套了?
同样的道理嘛。
com.example.transactional.exception.AgeExceptionOver18 字符串当然包含了 AgeException 字符串了。
但是我并不想回滚啊,哥,你好好看看,我抛出来的异常是 AgeExceptionOver18 呀。
到这里,我想问题我应该已经描述的非常清楚了,要是你还是没明白问题是什么,那你不用往下看了,再看一下“大幕拉开”这一节。
不然后面你很难入戏。
铺垫一波
为了把真正的问题更好的抛出来,我必须得先把另外一个相关的问题引出来,作为铺垫。
首先,我们去 Spring 项目的 issues 里面搜一下 getDepth 方法所在的 RollbackRuleAttribute 这个类。
看看有没有相关的蛛丝马迹,结果如下:
经过分析,对我有帮助的也只有第一条内容。
https://github.com/spring-projects/spring-framework/pull/24682
题目叫做:
Improve javadoc in RollbackRuleAttribute regarding nested classes。
改进 RollbackRuleAttribute 中关于嵌套类的 javadoc。
从题目我们知道这是一次对于文档的改进。
那么具体是啥改进呢?
可以看到他的描述中也提到了我们前面分析的那一个“万恶之源”的方法。
关于他具体说了什么,其实我也不用给你翻译,直接给你看他提交的代码就一目了然了:
他主要说个了内部类的问题,而且他这个问题和我们的还有点不一样。
他的两个异常类,一个叫 EnclosingException,另一个叫做 EnclosedException,这两个字符串是不存在 contains 关系的。
那么在内部类的场景下,问题是什么呢?
我也给你演示一个,你只需要看一眼就明白了,示例代码如下:
需要注意的是,我现在的两个异常是 AgeException 和 AgeOver18Exception,这二者并不存在包含关系。
前面做 Demo 的时候是 AgeExceptionOver18。
AgeOver18Exception
AgeExceptionOver18
别看花眼了。
你看,内部类的时候抛出异常是这样的:
throw new AgeException.AgeOver18Exception();
你要是没回过味儿来,没关系,断点一打,代码一跑就恍然大悟:
看明白没,铁子。内部类抛出的异常的全路径名称是这样的:
xxx.UserInfoService\$ AgeException$AgeOver18Exception
这不就包含 AgeException 了吗,不就匹配上了吗,不就回滚了吗?
所以,虽然他这个问题的触发方式和我前面提到的还不一样,但是“万恶之源”是一样的。
那么解决方案是什么呢?
仅仅是修改了一下文档,从文档的角度表明了这个情况是会被回滚的:
对应到源码,也就是这个地方的注释:
org.springframework.transaction.interceptor.RollbackRuleAttribute#RollbackRuleAttribute(java.lang.Class<?>)
好,我们现在冷静的思考一下,这里仅仅是从文档的角度来修复这个问题,在文档里面明确说明指定异常的内部类也会被回滚,这个做法对不对?
我认为勉强是可以接受的。
比如,我们知道某个异常类被标记为应该被回滚,那么这个异常类的子类应该被回滚,这是没问题的。
我认为内部类和子类应该保存同样的逻辑,毕竟它们之前确实存在代码上的关联关系,从这个角度上也说的过去。
毕竟一切解释权归官方所有嘛。
到这里你要记住:
RollbackRuleAttribute 类已经因为在回滚异常的判断上使用 contains 爆出过内部类的问题。 这个问题通过修改 javadoc 描述的方式进行了修复,没有修改任何代码。 这个解决方案勉强说的过去。
好了,铺垫完成了。
好戏上演
我再次在 issues 里面搜索 RollbackRuleAttribute,会发现多了一条内容:
前面其实我刻意把这一条内容给隐藏了,因为这个 issues 就是和我聊天的读者提的。
这里的示例代码就是文章最开头出现的代码。
好戏就藏在这个 issues 里面的,一起看一下官方是怎么“反复横跳”。
https://github.com/spring-projects/spring-framework/issues/28098
首先,是一个叫做 snicoll 的哥们把这个 issues 的标题改了一下:
去掉了开头的“Bug”,这也很正常,属于提问不标准,现阶段只是提问者认为是一个 bug,你这样取名字,个人主观意识太过强烈,这样不好。
主要是你知道修改题目的 snicoll 是谁吗?
别问,问就是大佬,Spring 和 Spring Boot项目核心维护人员。
然后隔了几天,这个问题的标题又被另外一个大佬,简单的修改了一下:
仅仅是把 use contains
修改为了 uses contains()
,把 equals
修改为了 equals()
。
这个小细节完美的体现了 Spring 框架的严谨之处,可以说是非常的严谨了。
还没进入到问题解答环节,先把问题的“错别字”给修改了。
接着就进入了官方答疑环节。
说了下面这么大一堆内容,但是根本不要慌,你知道的我的 English level 是非常的 high 的。这一堆内容分为三大部分,我会一点点的给你说明白:
首先是第一部分:
一上来就是个英语长句,但是根本不要怕。
你看他先是简明扼要的提到了一个短语“by design”,也就是“设计如此”。
整个翻译过来大概就是这样的。
这个地方我们要用 contains()
方法呢?这其实是经过考虑的。
那么是基于什么考虑呢?
在 XML 配置文件中,用户通常指定自定义异常类型的简单名称,而不是全路径类名。
啥意思呢?
他给了一个文档中的 xml 配置示例:
那我现在基于我们的 Demo 也搞一个 xml 配置嘛,回到若干年前的基于 xml 配置的方式:
这里我也不得不感慨一句:以前基于 xml 开发的时候是真的麻烦,每次都要去系统项目里面拷一份配置出来,所以我还是很感谢 SpringBoot 的出现的。
这里他想表达一个什么意思呢?
在我的 xml 配置中,关于 rollback-for 属性。他提到的 simple name 就是 AgeException。而 fully qualified class name 就是 com.example.transactional.exception.AgeException。
就是说这里是不限制用户填什么的。
如果用户填的是 simple name,我们也应该让其生效,所以必须要使用 contains()
方法。
以我理解,这个地方和 @Transactional 注解里面的 rollbackForClassName 属性的用法是一样,而这是一个历史遗留问题,是当年一个不好的设计。
但是我认为不能说考虑不周,毕竟别人也很难想到你会按照那么奇怪的方式去命名异常类啊!
总之这一段话他解释了为什么会用 contains()
方法,为什么不能用 equals()
方法。
和我们前面分析的基本一致,只是我们没有想到 XML 的配置方式。
第二段,他开始从文档的角度来解释这个问题。
叫我们关注一下 RollbackRuleAttribute 上的 Javadoc 描述。
这里有一个“NB”,不是我们常常说的牛逼,而是一个缩写:
你看,又在我这里学到一个用不上的英文知识。
我们接着看,主要关注我划线的两句。
第一句是说:由于使用的 contains()
方法,“Exception” 几乎可以匹配任何规则,并且可能会隐藏其他规则。但是“java.lang.Exception”这个全路径的字符串,那么匹配范围就小了很多了。
第二句是说:由于使用的 contains()
方法,对于 “BaseBusinessException” 等不寻常的异常名称,不需要使用类的全路径名称。
所以,第二段他想表达的是:文档上我们已经说过了,对于匹配规则,要仔细思考,要非常的小心:
u1s1,他确实写了,但是你觉得你会看吗?
第三段就很简单了:
看到 its subclasses, and its nested classes.
就知道这是我们前面“铺垫一波”小节说过的部分。
所以你现在知道我为什么给你铺垫了吧?
如果不给你铺垫一波,你突然看到一个内部类的单词 nested classes
,你说你一下反应得过来吗?
你要永远相信我的行文结构。
好了,现在看另外一句我标注的地方,翻译过来是说:在当前的实现中最后一句话并没有遵守。
这里的“最后一句话”就是指 RollbackRuleAttribute 的 Javadoc 的最后一句,也就是 ... its subclasses, and its nested classes
这句。
当然没遵守了。
我写的 Demo 里面的两个异常即不存在子类父类的关系,也不存在内部类的关系。
所以我觉得很纳闷:这个 Javadoc 和我的问题之间并不存在关系,或者说并不冲突啊。前面我也说了,关于这部分的 Javadoc 我觉得是没有毛病的。如果你想要从修改文档的角度来解决这个问题,也不应扯到子类,内部类啥的,应该是完全另起一行才对。
但是具体怎么解决,他并没有立即表态,而是把这个 issues 放到了 Triage Queue 里面:
Queue,队列,你肯定都知道。
Triage 是个啥?
我也不知道,于是我也学到了一个新单词:
也就是说官方把这个 issues 放到了“待分类的”一个队列里面,说明他目前是了解到了问题的所在,但是具体应该怎么解决,还没有定论,有待商榷。
隔了一天这个老哥又来表态了,开始“横跳”:
他说他又想了一下,需要更正他之前的说法:RollbackRuleAttribute(Class) 构造函数的 Javadoc 是 mostly correct,也就是基本没毛病的。需要改进的是关于回滚规则上的描述。
总之他还是想从文档的角度来修复这个问题。
但是解释了我前面的疑惑:即使从修改文档的角度来解决这个问题,也不应扯到子类,内部类啥的,应该是完全另起一行才对。
他这里的“回滚规则”也就是“另起一行”。
接着,他对任务的状态进行了流转:
从“待分类”移动到了“文档”的标签下。
然后表示在 5.3.17 这个里程碑版本中会进行修复:
同时,再次修改了 issues 的标题:
Transaction rollback rules may result in unintentional matches for similarly named exceptions
事务回滚规则可能会导致无意中匹配到名称相似的例外情况
其实如果让我来处理这个问题,我大概率也是会从文档的角度入手,并且最多加一点提醒日志,毕竟这是你使用不规范导致的。
而且我文档上已经说明有“坑”了,你自己没看踩进去了,这怪不得我呀。
但是在和这个读者表达了我的观点之后,他提出的不一样的看法:
他觉得使用者大多并不关注日志,主张抛出异常的方式进行强提醒:
于是他在 issues 上表达了自己的看法:
他觉得需要更精准的匹配规则,大多数人是不看文档的。
接着官方采纳了他的意见,并把该需求移动到了 6.0.0-M3 这个里程碑的版本中去实现:
他的具体回复如下:
他说:老铁,我同意你关于“需要更精准的匹配规则”的观点。
我们会修复 5.3.x 的文档描述。
然后在 6.0 版本中,我们会改进一版代码。
具体来说是这样的:
如果异常模式是以字符串形式提供的。例如,在 XML 配置中或通过 @Transactional(rollbackForClassName = "example.CustomException") 配置,那么现有的 contains()
逻辑将继续被使用。如果一个具体的异常类型是以类引用的形式提供的。例如,通过 @Transactional(rollbackFor = example.CustomException.class),将会有新的逻辑。它完全采用配置上提供的类型信息,从而避免了在 example.CustomException(没有2)被提供为异常类型时,与 example.CustomException2 的意外匹配。
他这里提到的 CustomException 和 CustomException2,其实是他的测试用例里面的代码。类比于我们前面的 AgeException 和 AgeExceptionOver18 这两个异常。
接着,他对这个 issues 进行了重新分类,从“文档”类型,修改为了“enhancement”类型:
enhancement,是个四级词语,背一下,会考:
表示这个 issues 需要通过修改代码来使健壮性更强。
然后再次修改了标题:
对于事务回滚规则,应该使用异常的类型信息,而不是用模式匹配。
本来故事到这里都已经是大结局了,我写到这里的时候就准备收尾了。
想着收尾不着急,先睡一觉再说。
结果...
第二天早上起来,他!又!更!新!了!
我还得补一段内容。
最后一集
早上起来,我一刷新页面,发现官方针对这个 issues 进行了最后一次提交:
这次 issues 的标题,最后定格为:
Support type-safe transaction rollback rules
支持类型安全的事务回滚规则
而这次对应的代码提交链接是这样的:
https://github.com/spring-projects/spring-framework/commit/c1033dbfb3609f3b3fe002d7b582b3302620c05a
里面写了很长一段的内容,来描述这次提交的背景,但是基本上都是我前面写过的东西的总结:
结合我前面写的东西,我给你翻译翻译:
首先我觉得是在事务模块里面创造一个新的概念:type-safe rollback rules,类型安全的回滚规则。
在这次提交之前,只有一种事务回滚机制, Pattern-based rollback rules,即基于匹配模式的回滚规则。
而官方说基于匹配模式的回滚规则,会带来三种意料之外的匹配情况:
不同包中的相同命名的异常类,会被意外匹配上。比如,example.client.WebException 和 example.server.WebException 都会与 “WebException” 模式匹配。 在同一个包中有类似命名的异常,这里说的相似是指当一个给定的异常名称是以另一个异常的名称开头时。例如:example.BusinessException 和 example.BusinessExceptionWithDetails 都与 “example.BusinessException”模式匹配。 嵌套异常,也就是当一个异常类被声明在另一个异常类里面的时候。例如:example.BusinessException 和 example.BusinessException$NestedException 都会与 “example.BusinessException” 匹配上。
第一种没啥说的,请使用全路径名称去避免。
第二种就是我们文章中的例子,需要通过修改代码解决。
第三种内部类的情况我也在前面铺垫过了。但是当时的解决方案是仅增加文档中对应的描述。
但是现在,你看他怎么说的:
这次的提交可以防止后两种情况的意料之外的匹配。也就是说这次提交不仅修复了我们的问题,还修复了内部类的问题。
那么怎么修复的呢?
首先是在 RollbackRuleAttribute 类里面新增了一个 exceptionType 字段:
然后在构造方法里面对其进行赋值:
核心代码变成了这样:
当 exceptionType 字段,即全路径名称不为空的时候,使用 equals()
方法,也就是type-safe rollback rules。否则使用 contains()
方法,也就是 Pattern-based rollback rules。
另外,关于 Javadoc 上的很多描述也发生了变化,我就不一一举例了,强烈建议你自己去看看这次提交。
我只特别拿出一处变化来给你看看:
去掉了“内部类”,改成了“类型安全”。
至此,问题得到解决。
但是在 XML 里面或者用 @Transactional 注解里面的 rollbackForClassName 属性,也就是使用匹配模式的时候,还是会有意料之外的匹配情况。
官方:反正我在文档上说清楚了,你要是还踩坑,那就怪得不我了?
最后,再插一个关于编程规范的事儿。
你想这次这个问题完全是因为你有两个这样的异常类名称引起的:
AgeException
AgeExceptionOver18
而对于异常类,我们都约定成俗的要求必须以“Exception”结尾。
包括阿里巴巴 Java 开发手册在命名风格里面也特意提到了这一点,且是强制要求:
所以,如果我们都遵守这个规则,大家就相安无事。
那么,这个故事最后告诉我们一个什么道理呢?
它告诉我们...
它告诉我们规则就是拿来打破的,如果你不打破规则,永远也踩不到这个坑,也就不会推动 Spring 的改动。
打破规则,这是你的一小步,却是开源世界的一大步。
所以,兄弟们,铁子们,不要墨守成规,要打破...
最后,文章首发于公众号[why技术],欢迎关注,第一时间接收最新文章。