Skip to content

Improve generated default name for @JmsListener subscription #29790

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 4 commits into from
Closed

Improve generated default name for @JmsListener subscription #29790

wants to merge 4 commits into from
+13 −1

Conversation

fml2
Copy link
Contributor

@fml2 fml2 commented Jan 10, 2023 •
edited
Loading

This should fix #29763.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jan 10, 2023
fml2 added 3 commits January 10, 2023 08:38
@fml2
chore: Added the needed import

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
GPG key ID: 4AEE18F83AFDEB23
Expired
Verified
Learn about vigilant mode
e9ec189
@fml2
chore: Fix style (add '.this')

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
GPG key ID: 4AEE18F83AFDEB23
Expired
Verified
Learn about vigilant mode
2c4ec46
@fml2
chore: Improve style ('else' on a separate line)

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
GPG key ID: 4AEE18F83AFDEB23
Expired
Verified
Learn about vigilant mode
e8def9d
@sbrannen sbrannen added in: messaging Issues in messaging modules (jms, messaging) type: enhancement A general enhancement labels Jan 11, 2023
@sbrannen sbrannen changed the title fix (spring-jms): Better default name for a JMS subscription Improve generated default name for a JMS subscription Jan 11, 2023
@sbrannen sbrannen mentioned this pull request Jan 11, 2023
Closed
@sbrannen sbrannen removed the status: waiting-for-triage An issue we've not yet triaged or decided on label Jan 11, 2023
@sbrannen sbrannen added this to the 6.0.5 milestone Jan 11, 2023
@sbrannen sbrannen self-assigned this Jan 11, 2023
@fml2
Copy link
Contributor Author

fml2 commented Jan 11, 2023

Could this also be backported to the 5.x line?

sbrannen pushed a commit to sbrannen/spring-framework that referenced this pull request Jan 30, 2023
@fml2 @sbrannen
Improve generated default name for a JMS subscription
145efa8 
Closes spring-projectsgh-29790
sbrannen added a commit to sbrannen/spring-framework that referenced this pull request Jan 30, 2023
@sbrannen
Revised generated default name for a JMS subscription
7e09f91 
The previous commit changed the generated default name for a JMS
subscription to <FQCN>#<method name> -- for example:

- org.example.MyListener#myListenerMethod

In order to generate unique subscription names for overloaded listener
methods within a class, this commit retains the <FQCN>#<method name>
format for a method that declares zero parameters but changes the format
to <FQCN>#<method name>(<parameter list>) for methods that declare
parameters, where the parameter list is a comma-separated list of the
canonical names of the parameter types -- for example:

- org.example.MyListener#myListenerMethod
- org.example.MyListener#myListenerMethod(java.lang.String)
- org.example.MyListener#myListenerMethod(byte[])
- org.example.MyListener#myListenerMethod(byte[],java.lang.String)

See spring-projectsgh-29790
@sbrannen sbrannen changed the title Improve generated default name for a JMS subscription Improve generated default name for @JmsListener subscription Jan 30, 2023
@sbrannen
Copy link
Member

As a proof of concept, I pushed changes to main...sbrannen:spring-framework:issues/gh-29790-default-jms-subscription-name that include the method signature in the generated subscription name (in order to generate unique subscription names for overloaded @JmsListener methods).

However, before pushing those changes I decided to verify the requirements of a subscription name in the JMS 3.0 spec. That points out that characters such as (, ), ,, and # are not guaranteed to be supported. In addition, there's only a guarantee of support for up to 128 characters.

In light of that, I have decided to use a <FQCN>.<method name> (FQCN: fully qualified class name) format instead of the <FQCN>#<method name> format in this PR.

@sbrannen sbrannen closed this in ad4e0d9 Jan 30, 2023
sbrannen added a commit that referenced this pull request Jan 30, 2023
@sbrannen
Revise generated default name for @JmsListener subscription
04321d0 
The previous commit changed the generated default name for a JMS
subscription to <FQCN>#<method name> -- for example:

- org.example.MyListener#myListenerMethod

However, the JMS spec does not guarantee that '#' is a supported
character. This commit therefore changes '#' to '.' as the separator
between the class name and method name -- for example:

- org.example.MyListener.myListenerMethod

This commit also introduces tests and documentation for these changes.

See gh-29790
@sbrannen
Copy link
Member

Could this also be backported to the 5.x line?

Yes, we will backport this to 5.3.x.

@sbrannen sbrannen added for: backport-to-5.3.x status: backported An issue that has been backported to maintenance branches and removed for: backport-to-5.3.x labels Jan 30, 2023
@sbrannen sbrannen mentioned this pull request Jan 30, 2023
Closed
@fml2
Copy link
Contributor Author

fml2 commented Jan 30, 2023

In light of that, I have decided to use a <FQCN>.<method name> (FQCN: fully qualified class name) format instead of the <FQCN>#<method name> format in this PR.

This is perfectly OK for me. The main thing is that the name is meaningful and not fixed and is equal to the name of a rather internal Spring class.

sbrannen added a commit that referenced this pull request Jan 31, 2023
@sbrannen
Update release version due to backport
ebe717f 
See gh-29790
See gh-29902
@sbrannen
Copy link
Member

This has been merged into main in ad4e0d9 and revised in 04321d0.

@fml2 fml2 deleted the feat/default-jms-subsr-name branch January 31, 2023 20:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@sbrannen sbrannen

Labels
in: messaging Issues in messaging modules (jms, messaging) status: backported An issue that has been backported to maintenance branches type: enhancement A general enhancement
Projects
None yet
Milestone
6.0.5
Development

Successfully merging this pull request may close these issues.

Wrong default name of the JMS subscription
3 participants
@fml2 @sbrannen @spring-projects-issues