Skip to content

Conversation

artembilan
Copy link
Member

Fixes #3957

Sometimes we cannot use a standard JmsReplyTo property for sending replies from the server. A DestinationResolver API does not have access to the request message.

  • Introduce a ChannelPublishingJmsMessageListener.replyToExpression property to evaluate a reply destination against request JMS Message
  • Use this expression only of no JmsReplyTo property
  • Expose this property on Java DSL level
  • To simplify end-user experience with lambda configuration for this property, introduce a CheckedFunction which essentially re-throws exception "sneaky" way

Comment on lines 272 to 278
/**
* Set a SpEL expression to resolve a 'replyTo' destination from a request {@link jakarta.jms.Message}
* as a root evaluation object if {@link jakarta.jms.Message#getJMSReplyTo()} is null.
* @param replyToExpression the SpEL expression for 'replyTo' destination.
* @since 6.1
*/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Javadocs should wrap at 90 chars max.

Suggested change
/**
* Set a SpEL expression to resolve a 'replyTo' destination from a request {@link jakarta.jms.Message}
* as a root evaluation object if {@link jakarta.jms.Message#getJMSReplyTo()} is null.
* @param replyToExpression the SpEL expression for 'replyTo' destination.
* @since 6.1
*/
/**
* Set a SpEL expression to resolve a 'replyTo' destination from a request
* {@link jakarta.jms.Message} as a root evaluation object if
* {@link jakarta.jms.Message#getJMSReplyTo()} is null.
* @param replyToExpression the SpEL expression for 'replyTo' destination.
* @since 6.1
*/

* {@link #resolveDefaultReplyDestination default reply destination} is returned; if this too is <code>null</code>,
* It will first check the JMS Reply-To {@link Destination} of the supplied request message;
* if that is null, then the configured {@link #replyToExpression} is evaluated (if any), then a
* {@link #resolveDefaultReplyDestination default reply destination} is returned; if this too is null,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto

}

@Nullable
private Destination resolveReplyTo(jakarta.jms.Message request, Session session) throws JMSException {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

protected so subclasses can override?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so. Then it is probably better to make the whole getReplyDestination() as protected.
And then we will dive to a mess what is protected and what is not and can be accessed from the inheritors.
If you wish, we can revise this in a separate issue.

Comment on lines 414 to 418
By default, a `JmsInboundGateway` looks for a `jakarta.jms.Message.getJMSReplyTo()` property in the received message for sending a reply.
Otherwise, it can be configured with a static `defaultReplyDestination`, or `defaultReplyQueueName` or `defaultReplyTopicName`.
In addition, starting with version 6.1, a `replyToExpression` can be configured on a provided `ChannelPublishingJmsMessageListener` to determine a reply destination dynamically if standard `JMSReplyTo` property cannot be on request.
A received `jakarta.jms.Message` is used a root evaluation context object.
The following example demonstrates how to use Java DSL API to configure an inbound JMS gateway for custom reply destination resolved from request message:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
By default, a `JmsInboundGateway` looks for a `jakarta.jms.Message.getJMSReplyTo()` property in the received message for sending a reply.
Otherwise, it can be configured with a static `defaultReplyDestination`, or `defaultReplyQueueName` or `defaultReplyTopicName`.
In addition, starting with version 6.1, a `replyToExpression` can be configured on a provided `ChannelPublishingJmsMessageListener` to determine a reply destination dynamically if standard `JMSReplyTo` property cannot be on request.
A received `jakarta.jms.Message` is used a root evaluation context object.
The following example demonstrates how to use Java DSL API to configure an inbound JMS gateway for custom reply destination resolved from request message:
By default, the `JmsInboundGateway` looks for a `jakarta.jms.Message.getJMSReplyTo()` property in the received message to determine where to send a reply.
Otherwise, it can be configured with a static `defaultReplyDestination`, or `defaultReplyQueueName` or `defaultReplyTopicName`.
In addition, starting with version 6.1, a `replyToExpression` can be configured on a provided `ChannelPublishingJmsMessageListener` to determine the reply destination dynamically, if the standard `JMSReplyTo` property is `null`on the request.
The received `jakarta.jms.Message` is used the root evaluation context object.
The following example demonstrates how to use Java DSL API to configure an inbound JMS gateway with a custom reply destination resolved from the request message:

[[x6.1-jms]]
=== JMS Changes

A `JmsInboundGateway`, via its `ChannelPublishingJmsMessageListener`, can now be configured with a `replyToExpression` to resolve a reply destination against request message at runtime.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
A `JmsInboundGateway`, via its `ChannelPublishingJmsMessageListener`, can now be configured with a `replyToExpression` to resolve a reply destination against request message at runtime.
The `JmsInboundGateway`, via its `ChannelPublishingJmsMessageListener`, can now be configured with a `replyToExpression` to resolve a reply destination against the request message at runtime.

try {
return apply(t1);
}
catch (Throwable t) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sonar will complain about this.

Suggested change
catch (Throwable t) {
catch (Throwable t) { // NOSONAR

return sneakyThrow(t);
}
};
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not really comfortable with institutionalizing this JVM abuse. Is there no alternative?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. Will it be better if there is no that unchecked() then?
Something like plain:

@FunctionalInterface
public interface CheckedFunction<T, R> {

	R apply(T t) throws Throwable;

}

?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right; that's the bit I was objecting to. You probebly need a // NOSONAR here too.

Comment on lines +165 to +167
public S replyToFunction(CheckedFunction<Message, ?> replyToFunction) {
return replyToExpression(new FunctionExpression<>(replyToFunction.unchecked()));
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need this; why not just

Suggested change
public S replyToFunction(CheckedFunction<Message, ?> replyToFunction) {
return replyToExpression(new FunctionExpression<>(replyToFunction.unchecked()));
}
public S replyToFunction(Function<Message, ?> replyToFunction) {
return replyToExpression(new FunctionExpression<>(replyToFunction));
}

??

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, but then .replyToFunction(message -> message.getStringProperty("myReplyTo")) is not going to be clean and end-user has to handle that JMSException himself which is not runtime one.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, you are assuming that the user will use a JMS operation to determine the destination? I guess that's reasonable given that the root object is a Message.

OK.

Fixes spring-projects#3957

Sometimes we cannot use a standard `JmsReplyTo` property for sending replies from the server.
A `DestinationResolver` API does not have access to the request message.

* Introduce a `ChannelPublishingJmsMessageListener.replyToExpression` property to evaluate
a reply destination against request JMS `Message`
* Use this expression only of no `JmsReplyTo` property
* Expose this property on Java DSL level
* To simplify end-user experience with lambda configuration for this property, introduce a `CheckedFunction`
which essentially re-throws exception "sneaky" way
* Fix Javadocs lines length
* Regular `catch` and re-throw in the `CheckedFunction`
@garyrussell garyrussell merged commit acd8a03 into spring-projects:main Feb 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Jms InboundGateway to support Dynamic reply queue
2 participants