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

Handle custom JMS acknowledgment modes as client acknowledge #30619

Merged
merged 2 commits into from Jun 12, 2023

Conversation

vpavic
Copy link
Contributor

@vpavic vpavic commented Jun 8, 2023

This commit updates JmsAccessor to handle custom JMS acknowledgment
modes as client acknowledge, which is useful when working with JMS
providers that provide non-standard variations of CLIENT_ACKNOWLEDGE,
such as AWS SQS and its UNORDERED_ACKNOWLEDGE mode.


The workaround (a quite tedious one) that I'm using at the moment is to subclass DefaultJmsListenerContainerFactory and DefaultMessageListenerContainer and override #isClientAcknowledge using something like this:

class SqsJmsListenerContainerFactory extends DefaultJmsListenerContainerFactory {

    @Override
    protected DefaultMessageListenerContainer createContainerInstance() {
        return new DefaultMessageListenerContainer() {

            @Override
            protected boolean isClientAcknowledge(Session session) throws JMSException {
                return (session.getAcknowledgeMode() == SQSSession.UNORDERED_ACKNOWLEDGE);
            }

        };
    }

}

For reference, SQS UNORDERED_ACKNOWLEDGE is described as:

Non standard acknowledge mode. This is a variation of CLIENT_ACKNOWLEDGE where Clients need to remember to call acknowledge on message. Difference is that calling acknowledge on a message only acknowledge the message being called.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jun 8, 2023
@jhoeller
Copy link
Contributor

jhoeller commented Jun 8, 2023

We could turn this around and implement JmsAccessor.isClientAcknowledge() through exclusion of all other common JMS modes (TRANSACTED, AUTO, DUPS_OK): treating everything else as client acknowledge, leading to an explicit Message.acknowledge() call in JmsTemplate and AbstractMessageListenerContainer. Since that explicit acknowledge method is defined to be ignored on implicit acknowledge setups, it won't ever hurt to call it.

	protected boolean isClientAcknowledge(Session session) throws JMSException {
		int mode = session.getAcknowledgeMode();
		return (mode != Session.SESSION_TRANSACTED &&
				mode != Session.AUTO_ACKNOWLEDGE &&
				mode != Session.DUPS_OK_ACKNOWLEDGE);
	}

No specific configuration necessary then, so it would work out of the box for your scenario, I suppose? If that sounds feasible and you got some cycles left, feel free to update the PR accordingly and rebase it onto the 6.0.x branch for inclusion in 6.0.10. Otherwise we can try to implement that out-of-the-box approach ourselves next week.

@jhoeller jhoeller added in: messaging Issues in messaging modules (jms, messaging) type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Jun 8, 2023
@jhoeller jhoeller self-assigned this Jun 8, 2023
@jhoeller jhoeller added this to the 6.0.10 milestone Jun 8, 2023
@vpavic
Copy link
Contributor Author

vpavic commented Jun 8, 2023

Thanks for the quick feedback Juergen.

No specific configuration necessary then, so it would work out of the box for your scenario, I suppose?

Yes indeed, that would be perfect.

Since that explicit acknowledge method is defined to be ignored on implicit acknowledge setups, it won't ever hurt to call it.

I missed this, and was wary about changing the default behavior in any way, but if there's no risk in doing explicit acknowledge for vendor specific acknowledge modes that not variants of client acknowledge then I'm all for it.

I can update the PR according to your guidance tomorrow morning.

This commit updates JmsAccessor to handle custom JMS acknowledgment
modes as client acknowledge, which is useful when working with JMS
providers that provide non-standard variations of CLIENT_ACKNOWLEDGE,
such as AWS SQS and its UNORDERED_ACKNOWLEDGE mode.
This commit reduces the visibility of JmsAccessorTests and its test
methods to package-private and removes unnecessary throws declarations.
@vpavic vpavic force-pushed the jms-custom-client-ack-modes branch from 7fb27d6 to db2f1d6 Compare June 9, 2023 08:02
@vpavic vpavic changed the base branch from main to 6.0.x June 9, 2023 08:02
@vpavic vpavic changed the title Add support for custom JMS client acknowledge mode Handle custom JMS acknowledgment modes as client acknowledge Jun 9, 2023
@vpavic
Copy link
Contributor Author

vpavic commented Jun 9, 2023

PR updated according to feedback and rebased on 6.0.x.

@sbrannen sbrannen requested a review from jhoeller June 9, 2023 08:14
@jhoeller jhoeller merged commit 3d61d9e into spring-projects:6.0.x Jun 12, 2023
2 checks passed
@vpavic vpavic deleted the jms-custom-client-ack-modes branch June 12, 2023 08:52
mdeinum pushed a commit to mdeinum/spring-framework that referenced this pull request Jun 29, 2023
…projects#30619)

This commit updates JmsAccessor to handle custom JMS acknowledgment
modes as client acknowledge, which is useful when working with JMS
providers that provide non-standard variations of CLIENT_ACKNOWLEDGE,
such as AWS SQS and its UNORDERED_ACKNOWLEDGE mode.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: messaging Issues in messaging modules (jms, messaging) type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants