Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backward compatibility: initDefaultStrategies() is no longer invoked on subclasses of "MessageListenerAdapter" [SPR-15043] #19609

Closed
spring-issuemaster opened this issue Dec 22, 2016 · 3 comments

Comments

@spring-issuemaster
Copy link
Collaborator

commented Dec 22, 2016

Gordon Daugherty opened SPR-15043 and commented

Tl;dr - This comment more succinctly states the concern.


We have an extension of "MessageListenerAdapter" in a widely-shared component. That component doesn't work with Spring versions 4.1.0 and higher due to it no longer invoking "initDefaultStrategies()". As apps that use that component upgrade past Spring v4.0.9 they're experiencing breakages.

Before we patch around the issue I wanted to check with the Spring team to make sure that this change was a deliberate design evolution choice and not a mistake.

As you can see in this API analysis report v4.1.0 of library "spring-jms" removed the protected method "initDefaultStrategies()" from MessageListenerAdapter. This commit was made in support of delivering #14515 and made MessageListenerAdapter extend "AbstractAdaptableMessageListener" and moved the initDefaultStrategies() call to that class. So far so good / I don't think this commit is an issue. This later commit was made in support of
#15500 which removed the initDefaultStrategies() call from "AbstractAdaptableMessageListener".

Neither of the Jira tickets appears to speak to an intentional change of this behavior. If it was intentional your response(s) to this ticket will serve to document for others the "why" behind this backward-incompatible change.

If this change wasn't intentional and you fix it in a later version of Spring that simplifies my process and my software as I can just enable controls to prevent people from using the versions that lack the necessary code.

This comment explains why I think this change was unintentional.


Affects: 4.1.9

Issue Links:

  • #14515 Annotation-driven JMS endpoints
  • #15500 Applying the spring-messaging module to JMS

Referenced from: commits afe0228

1 votes, 4 watchers

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Dec 22, 2016

Juergen Hoeller commented

Hmm, in that immediate commit, it was only intentional to get rid of it in the abstract base class. I missed that it was originally declared in the (old) MessageListenerAdapter subclass, and now isn't being invoked for custom subclasses of that old variant anymore.

That said, the original reason for that initDefaultStrategies template method was JMS 1.0.2 support, which is no longer valid. We didn't really expect anybody to override it for other reasons since it is generally not too nice to call such an overridable method from a base class constructor.

Couldn't you simply call your method implementation from your subclass constructor? Or embed the body of that method right into the constructor? I'd recommend that in general. That said, you have a point about backwards compatibility, so I don't mind re-introducing that callback either... but probably just in deprecated form.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Dec 22, 2016

Gordon Daugherty commented

My preference is to re-introduce the callback, mark it as deprecated, and I'll change my component as you described. This way those affected have time to eliminate their usage of "initDefaultStrategies" between now and when the deprecated behavior is removed (again).

If I had a smaller team and/or was only running into this is 10 places I'd just roll out a new shared component but our portfolio is large enough that it takes a while to get everything updated. Until we do that we'll have individuals upgrade Spring and encounter this issue. If we choose not to patch the compatibility issue they'll find this ticket while researching their unsuccessful upgrade and will probably be irritated that we didn't choose to fix the source of the problem.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Dec 22, 2016

Juergen Hoeller commented

Alright, I'll reintroduce it for 4.3.6 in deprecated form but as of 5.0 it remains gone.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.