Hi, I’m crooked.

Last week, I was absorbed in fishing, and then a friend sent me a message on wechat with a question about my business, accompanied by two codes:

Conclusion first: I think this is a bug in Spring transactions. But officials say this is a documentation bug, not a code bug.

(Well, I’ve been working on this article for a few days, so when I wrote the above sentence, the authorities didn’t admit it was a bug, but they did admit it was a bug after I wrote it. No impact, continue to read.)

Boy, I get it. All interpretation is official.

Before I get to the bottom of this, I’d like to say a few words about how to ask questions.

I cut out the above reader’s question because I think it’s a template question.

You give a sample code, you give a source code, you explain your problem, and you say that you’ve looked around and found no good answers.

As SOON as I read his words, I could get to know what his problem was, so our communication was very efficient.

In the end, we did not discuss a reasonable explanation, so he went to raise issues, hoping to get an official authoritative answer.

So let’s start our story on issues.

stage

Before the main act starts, LET me set the stage for you.

Since it is about Spring transactions, this Demo is mainly about the application of “transactions”.

So the core thing in the Demo is this section:

The two exceptions involved are simple custom exceptions:

Let’s say there’s a weird site that only allows users between the ages of 10 and 18 to use. This section represents the user registration function for that site.

Then we insert a piece of data into the user_INFO table. If the user is older than 18, AgeExceptionOver18 is thrown, indicating that the user is not the target user.

But notice that rollbackFor in my @Transactional annotation is ageException. class, which means I don’t want to rollback a user who is older than 18. I want to save his registration information. I’m just throwing an exception to show that he’s not my target user,

When we insert a user under the age of 10, ageException.class will be thrown. We should roll back the insert statement. I don’t want to save this user’s information.

Users younger than 10 don’t want to save, so why not decide before inserting?

Very good question, in the actual development is definitely to determine before inserting, but I am only here to demonstrate the transaction function, so I write it like this, how!

In the code above, I will create an interface to trigger, that is, to create a Controller:

The four classes above are the most critical ones, so I’ll separate them out.

The structure of the project is also very simple:

Other classes are not critical, I will not come out to say, are your best CRUD. Is it too much to spend five minutes building this project? I can touch fish for two minutes in between.

I set the log level to DEBUG and run the project to verify the functionality.

Then call this link:

http://127.0.0.1:8085/insertUser?age=8

The corresponding log looks like this:

As you can see what I’ve boxed, first verify that the INSERT statement was executed and that age is indeed 8. But eventually it rolled back.

Why roll back, our hearts are also clear, because here echoes:

Next, try users whose age is 18:

http://127.0.0.1:8085/insertUser?age=18

The corresponding log looks like this:

This is nothing to say, transaction committed successfully, data inserted successfully. That’s what we expected.

There is nothing wrong with the database data:

Then try a user whose age is 28.

The user is expected to throw AgeExceptionOver18, but the data is inserted successfully.

Come and walk one:

http://127.0.0.1:8085/insertUser?age=28

The corresponding log looks like this:

First the data was rolled back??

Abnormality is thrown out, but this also did not echo on ah!

There is nothing wrong with my @Transactional annotation, there is nothing wrong with transaction configuration, and my exceptions are not thrown around. Why should you roll them back?

And you said it wasn’t a bug?

Just change documentation?

The implication is to “deny”, forcibly from the document to make up?

Isn’t that bullying honest people?

Er… Wait, this is what happened when I wrote this.

However, when I finished writing this paragraph, I checked issues again the next day and found that things had changed. The official admitted that this was a bug again. The description in the document will be modified in version 5.3.x and the code will be fixed in version 6.0.

But I have foreshadowed so much in front, already written, I will not change, as here to leave a creative trace, ha ha.

Let’s move on.

Dramatic conflicts

A play, of course, has its dramatic conflict, which is the core of it. So what’s the core conflict in our Demo?

This section begins by telling you where the “dramatic conflict” is.

Let me ask you a question:

What are the exceptions that Spring manages to roll back by default?

We took this question to the source code, found the answer to this question, you can slide into the play.

Make a breakpoint, run the program, and then look at the call stack:

You can see that there is a transaction-related method in the call stack:

org.springframework.transaction.interceptor.TransactionAspectSupport#invokeWithinTransaction

This is where we break in.

What? How did I find my way here so quickly?

All I can say is: practice makes perfect.

Well, there are tricks, and you can try to find them yourself, because that’s not the point of this article, so I won’t go into them.

After the method executes the exception, it goes inside the catch block. The following line is the entry point for exception handling:

In our age=28 scenario, after this method comes in, the ex argument is our custom AgeExceptionOver18 exception:

I’ve also framed a line of code:

txInfo.transactionAttribute.rollbackOn(ex)
Copy the code

RollbackOn checks whether the ex parameter matches the specified rollback exception.

What if it matches? What if they don’t match?

If yes, rollback.

If they do not match, commit.

All right, let’s move on.

You will come here:

org.springframework.transaction.interceptor.RuleBasedTransactionAttribute#rollbackOn

For this method, when winner is null, there is a comment indicating that no rule was matched.

That is, we configure nothing, and by default, winner is null.

The following line of code contains the answer to our question:

return super.rollbackOn(ex);

So what are the exceptions that Spring manages to roll back by default?

If the current exception is a RuntimeException or Error, it will be rolled back.

I’ve been paving the way just to get you to the rollbackOn method, even the super.rollbackon (ex) line is a smoke screen that we don’t need to focus on in this article.

What we need to focus on is this part:

First let’s clarify what this rollbackRules is.

In our Demo, this is the full path name of the ageException. class object that we configured in the rollbackFor attribute on the @Transactional annotation:

Okay, here’s the thing. You have to keep your spirits up.

Focus on this line of code:

int depth = rule.getDepth(ex);

Let’s see what rule and ex are:

The exceptionName in rule is the full path name of the AgeException object that we configured.

Ex is the AgeExceptionOver18 exception thrown by our program.

The conflict at the heart of the play lies in this approach here:

org.springframework.transaction.interceptor.RollbackRuleAttribute#getDepth(java.lang.Throwable)

Ok, so the stage is set and the core conflict has been exposed.

The show is ready to begin.

The curtain went up

Look carefully at the code I’ve framed.

Exceptionclass.getname ()

It looks like this:

ExceptionName = this.exceptionName = this.exceptionName

It’s this thing:

Now, something amazing is about to happen, Tetsuko.

com.example.transactional.exception.AgeExceptionOver18

com.example.transactional.exception.AgeException

Although these are two different exceptions, these two strings perform the contains operation. Do you think it returns true?

So depth here is going to return 0.

So the winner here won’t be empty

So this method will return true:

Remember what I said when I returned true what code would be executed?

Is it rollback?

So, the root of all evil is this code we mentioned when the curtain went up:

org.springframework.transaction.interceptor.RollbackRuleAttribute#getDepth(java.lang.Class<?>, int)

At this point, I think it’s pretty clear: Isn’t this a bug? Are you better than Spring?

However, if the following two strings were to operate on equals, would you return false and the problem be solved?

com.example.transactional.exception.AgeExceptionOver18

com.example.transactional.exception.AgeException

That’s the point, but I don’t think it’s that simple.

First of all, I think the use of contains is intentional, but for what purpose, I’m not really sure.

The reader I was talking to raised a question about whether it would satisfy the rollbackForClassName property:

Because when we use rollbackForClassName we can use an array of strings to configure multiple exception names that need to be rolled back. For example, if I get a NullPointerException:

In normal use scenarios, we can complete the rollback operation.

The value of the corresponding code is like this;

Java. Lang. NullPointerException string contains a NullPointerException string, of course. So let’s roll back. No problem.

But if we use equals, then there is no match, causing the rollbackForClassName property to be invalidated.

So modifying contains to equals is a measure of demolishing the west wall and repairing the east wall, not an option.

But the rollbackForClassName property doesn’t work in our Demo either.

If I change the program to something like this, do you think it’s messed up?

Same idea.

Com. Example. Transactional. Exception. AgeExceptionOver18 string contains AgeException string, of course.

But I don’t want to roll back ah, elder brother, you take a good look, I throw exception is AgeExceptionOver18 ah.

At this point, I think I’ve described the problem quite clearly, and if you still don’t understand what the problem is, you don’t have to look any further than the section called “The curtain goes up.”

Or you’ll have a hard time getting in later.

A ground wave

In order to better present the real question, I have to introduce another related question to set the stage.

First, go to the Issues of the Spring project and search for the RollbackRuleAttribute class where the getDepth method is located.

To see if there are any relevant clues, the results are as follows:

After analysis, the only thing that helped me was the first one.

Github.com/spring-proj…

The title is:

Improve Javadoc in RollbackRuleAttribute regarding nested classes. Improved javadoc for nested classes in RollbackRuleAttribute.

We know from the title that this is an improvement to the document.

So what exactly are the improvements?

You can see that his description also refers to the “root of all evil” approach we analyzed earlier.

As for what he said in detail, actually I don’t need to translate it for you, but I can show you the code he submitted at a glance:

He mainly talks about the inner class problem, and his problem is a little different from ours.

EnclosingException and EnclosedException are two strings that do not contain

So in the internal scenario, what’s the problem?

I’m going to show you one, too. You just need to look at it. The sample code is as follows:

Note that my current two exceptions are AgeException and AgeOver18Exception, and there is no inclusion relationship between them.

The previous Demo was AgeExceptionOver18.

AgeOver18Exception

AgeExceptionOver18

Don’t look at it.

An inner class throws an exception like this:

throw new AgeException.AgeOver18Exception();

If you don’t come back to it, it doesn’t matter, as soon as the breakpoint hits, the code runs, and suddenly you realize:

See what I mean, Tetsuko. The full path name of an exception thrown by an inner class looks like this:

xxx.UserInfoService\ AgeException\AgeOver18Exception

There’s AgeException, there’s a match, there’s a rollback.

So while the trigger for his question is not quite the same as I mentioned earlier, the “root of all evil” is the same.

So what’s the solution?

Simply modifying the document indicates that the case can be rolled back from the document’s perspective:

Corresponding to the source code, which is a comment in this place:

org.springframework.transaction.interceptor.RollbackRuleAttribute#RollbackRuleAttribute(java.lang.Class<?>)

Ok, let’s think about it for a moment. We’re just fixing this from a documentation point of view. Is it right to specify in the documentation that the inner class that specified the exception will also be rolled back?

I think grudging is acceptable.

For example, if we know that an exception class is marked to be rolled back, it is ok that a subclass of that exception class should be rolled back.

I think it makes sense that inner classes and subclasses should keep the same logic, because they do have previous relationships in code.

After all, the right of interpretation belongs to the authorities.

Here’s something to remember:

  • The RollbackRuleAttribute class has already exposed problems with inner classes by using contains to determine rollback exceptions.
  • This problem is fixed by modifying the Javadoc description without changing any code.
  • The solution makes sense after a fashion.

All right, the setup is done.

Good show

When I search for RollbackRuleAttribute in issues, I find the following:

In fact, I deliberately put this content to hide, because this issues is and I chat with the reader mentioned.

The sample code here is the code that appears at the beginning of this article.

Play is hidden in this issues inside, look at the official is how “repeated horizontal jump”.

Github.com/spring-proj…

First, a guy named Snicoll changed the title of this issue:

The “Bug” at the beginning is removed, which is also very normal and belongs to the non-standard questioning. At the present stage, the questioner only thinks it is a Bug. You choose the name like this, and your personal subjective consciousness is too strong, which is not good.

Basically, do you know who Snicoll was who changed the title?

Don’t ask, ask is the big guy, the core maintainer of Spring and Spring Boot projects.

And then a few days later, the title of the question was simply changed by another big guy:

Just change use contains to uses contains() and equals to equals().

This little detail perfectly illustrates the rigor of the Spring framework, which is very rigour.

Before entering the problem solving segment, I first corrected the “typos” of the question.

Then came the official q&A session.

But don’t panic. As you know, my English level is very high. There are three parts to this pile, and I’ll break it down for you:

First, part one:

It starts out as a long English sentence, but don’t be afraid at all.

You see, he started with the succinct phrase ‘by design’, meaning ‘so designed’.

That’s what the whole translation looks like.

Where do we use the contains() method? This is deliberate.

So on what basis?

In XML configuration files, users typically specify simple names for custom exception types rather than full path class names.

What does that mean?

He gives an example of an XML configuration in a document:

Let me do an XML configuration based on our Demo, going back a few years to xmL-based configuration:

I can’t help but note that in the past, xmL-based development was a real hassle, and every time you had to copy a configuration out of your system project, I’m grateful for SpringBoot.

What does he mean by that?

In my XML configuration, about the rollback-for attribute. The simple name he mentioned is AgeException. The fully qualified class name is com. Example. Transactional. Exception. AgeException.

So there’s no limit to what the user can fill in.

If the user fills in simple Name, we should also let it take effect, so we must use the contains() method.

As I understand it, this is the same as the rollbackForClassName attribute in the @Transactional annotation, which is a legacy of bad design.

But I don’t think it’s unwise to name exception classes in such a strange way.

Anyway, in this paragraph he explains why we use the contains() method, and why we can’t use equals().

Basically the same as what we analyzed earlier, except that we didn’t think about how XML was configured.

In the second paragraph, he begins to explain the problem from a document point of view.

Let’s look at the Javadoc description on the RollbackRuleAttribute.

Here is a “NB”, which is not what we often say, but an abbreviation:

You see, I have learned another useless Knowledge of English.

Let’s go on and focus on the two sentences that I underlined.

Because of the contains() method used, “Exception” can match almost any rule, and may hide other rules. But “java.lang.Exception” is a full path string, so the range of matches is much smaller.

Because of the contains() method used, you do not need to use the class’s full path name for unusual exception names such as “BaseBusinessException.”

As we’ve said in the documentation, think carefully and be very careful about matching rules:

U1s1, he did, but do you think you’ll read it?

The third paragraph is easy:

See its subclasses, and its nested classes. I knew this was the part we talked about in the foreshadowing section.

So now you know why I set you up?

If you suddenly saw a word nested classes without laying a foundation, do you think you would know it?

You must always trust my writing structure.

The last sentence is not observed in the current implementation.

The “last sentence” here refers to the last sentence of the RollbackRuleAttribute Javadoc, which is… Its subclasses, and its nested classes.

Of course not.

The two exceptions in the Demo I wrote do not have the relationship between the parent of the child class and the inner class.

So I wondered: There was no relationship, or conflict, between this Javadoc and my problem. As I said earlier, I think this part of Javadoc is fine. If you want to solve this problem from the point of view of modifying the document, you don’t want to talk about subclasses, inner classes, etc., it should be a whole other line.

He didn’t say how he would address the issues immediately. Instead, he added the issue to the Triage Queue:

Queue, Queue, you know all about it.

What’s a Triage?

I didn’t know, so I also learned a new word:

That is to say, the official has put the issues in a queue of “to be classified”, indicating that he is aware of the problem, but the specific solution, there is no conclusion, to be discussed.

A day later, the elder brother came back and began to “jump” :

He says that on second thought, he needs to correct his earlier statement: the Javadoc of the RollbackRuleAttribute(Class) constructor is mostly correct, which is basically fine. What needs to be improved is the description of the rollback rules.

Anyway, he wanted to fix the problem from a documentation perspective.

But it does explain my previous confusion: even if the problem is solved from the perspective of modifying the documentation, it should not talk about subclasses, inner classes, etc., but it should be a completely different line.

His “rollback rule” here means “start on another line.”

Next, he flipped the status of the task:

Move from “to classify” to the “Documents” TAB.

It then indicates that a fix will be made in milestone 5.3.17:

At the same time, the title of Issues is revised again:

Transaction rollback rules may result in unintentional matches for similarly named exceptions

In fact, if I had to deal with this problem, I would probably also start from a documentation point of view, and at the very least add a little more reminder log, after all, you are using the non-standard result.

And my document has already stated that there is a “pit”, you did not see the step in, this blame me.

But after expressing my opinion to this reader, he offered a different view:

He feels that most users do not pay attention to logs, and advocates throwing exceptions as a strong reminder:

So he expressed his views on issues:

He felt the need for more precise matching rules, because most people don’t read documents.

Then officials took his advice and moved the requirement to the 6.0.0-M3 milestone:

His specific response is as follows:

He said: Tie, I agree with you about the need for more precise matching rules.

We will fix the documentation description for 5.3.x.

Then in version 6.0, we will improve one version of the code.

Here’s what it looks like:

  • If the exception pattern is provided as a string. For example, in an XML configuration or through @transactional (rollbackForClassName = “example.customException “) configuration, then the existingcontains()Logic will continue to be used.
  • If a specific exception type is provided in the form of a class reference. For example, by @ Transactional (rollbackFor = example. CustomException. Class), there will be a new logic. It takes exactly the type information provided in the configuration, avoiding an unexpected match with Example.customException2 when Example.customException (without 2) is provided as an exception type.

The CustomException and CustomException2 he mentions here are actually code from his test case. This is analogous to our previous exceptions AgeException and AgeExceptionOver18.

Next, he reclassifies the issues from the “document” type to the “enhancement” type:

Enhancement.

Indicates that this issue requires code changes to make it more robust.

Then he changed the title again:

For transaction rollback rules, you should use the type information of the exception instead of pattern matching.

This is the end of the story, and I’m ready to end it as I write it.

Don’t worry about finishing. Sleep on it.

The results…

The next morning, he! Again!!! More!!! New! !

I have one more paragraph to fill in.

The last episode

When I woke up this morning, I refreshed the page and found the last official submission for this issue:

The title of issues, finally framed as:

Support type-safe Transaction rollback rules Supports type-safe transaction rollback rules

The corresponding code submission link looks like this:

Github.com/spring-proj…

There’s a long paragraph describing the background to the submission, but it’s basically a summary of what I wrote earlier:

Combined with what I wrote earlier, I will translate for you.

The first thing I think is to create a new concept in the transaction module: type-safe rollback rules.

Prior to this commit, there was only one transaction rollback mechanism, pattern-based rollback Rules.

The rollback rule, based on matching patterns, introduces three unexpected matches:

  • Exception classes with the same name in different packages can be accidentally matched. Such as example. Client. WebException and example. Server WebException with “WebException” pattern matching.
  • There are similarly named exceptions in the same package. Similarity is when a given exception name begins with the name of another exception. For example: example. BusinessException and example. BusinessExceptionWithDetails are linked to “example. BusinessException” pattern matching.
  • Nested exceptions, when an exception class is declared inside another exception class. For example: example. BusinessException and example. BusinessException $NestedException are matching with “example. BusinessException”.

The first is nothing to say, use full path names to avoid.

The second is the example in our article, which requires code modification.

There’s a third inner class case that I’ve already foreshadowed. But the solution was to just add the corresponding descriptions in the documentation.

But now, look what he says:

This commit prevents unexpected matches in the latter two cases. That is, this commit fixes not only our problem, but also the internal class.

So how do you fix it?

ExceptionType is added to the RollbackRuleAttribute class:

Then assign to it in the constructor:

The core code looks like this:

When the exceptionType field, that is, the full path name, is not empty, use the equals() method, that is, type-safe rollback rules. Otherwise use the contains() method, that is, pattern-based rollback rules.

In addition, many of the descriptions on Javadoc have changed, and I’m not going to go through them all, but I strongly encourage you to take a look at this submission for yourself.

I’ll show you just one change in particular:

Removed “inner class” and changed it to “type-safe”.

At this point, the problem is solved.

However, unexpected matches can occur in XML or with the @Transactional annotation rollbackForClassName attribute, that is, when matching patterns are used.

Official: anyway I made it clear on the document, if you still step on pit, that strange get not me?

Finally, one more thing about programming specifications.

You think this problem is entirely caused by the fact that you have two such exception class names:

AgeException

AgeExceptionOver18

In the case of Exception classes, we all agree that requirements must end with “Exception”.

Including Alibaba Java development manual in naming style also specifically mentioned this, and is mandatory:

So, if we all follow this rule, we’ll all be fine.

So what is the final lesson of the story?

It tells us that…

It tells us that rules are made to be broken, and if you don’t break the rules, you’ll never step into the hole, and you won’t push Spring changes.

Break the rules. It’s one small step for you, but one giant leap for the open source world.

So, brothers, iron sons, don’t get stuck in a rut, break it…

Finally, the article was published on the public account [WHY Technology], welcome to pay attention to, the first time to receive the latest article.