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.

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

Improve generated default name for @JmsListener subscription #29790

Closed
wants to merge 4 commits into from
Closed

Improve generated default name for @JmsListener subscription #29790

wants to merge 4 commits into from

Conversation

fml2
Copy link
Contributor

@fml2 fml2 commented Jan 10, 2023 •
edited

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
@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 Marks an issue as a candidate 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 Marks an issue as a candidate 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
mdeinum pushed a commit to mdeinum/spring-framework that referenced this pull request Jun 29, 2023
@fml2 @mdeinum
Improve generated default name for a @JmsListener subscription
38bde34 
Prior to this commit, when using durable subscribers with @JmsListener
methods that do not specify a custom subscription name the generated
default subscription name was always
org.springframework.jms.listener.adapter.MessagingMessageListenerAdapter.
Consequently, multiple such @JmsListener methods were assigned the
same subscription name which violates the uniqueness requirement.

To address this, MessagingMessageListenerAdapter now implements
SubscriptionNameProvider and generates the subscription name based on
the following rules.

- if the InvocableHandlerMethod is present, the subscription name will
  take the form of handlerMethod.getBeanType().getName() + "#" +
  handlerMethod.getMethod().getName().
- otherwise, getClass().getName() is used, which is analogous to the
  previous behavior.

Closes spring-projectsgh-29790
mdeinum pushed a commit to mdeinum/spring-framework that referenced this pull request Jun 29, 2023
@sbrannen @mdeinum
Revise generated default name for @JmsListener subscription
3d6a70c 
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 spring-projectsgh-29790
mdeinum pushed a commit to mdeinum/spring-framework that referenced this pull request Jun 29, 2023
@sbrannen @mdeinum
Update release version due to backport
c55cccc 
See spring-projectsgh-29790
See spring-projectsgh-29902
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