Skip to content

Add default expressions for ExpEvalReqHAdvice #2738

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

Merged
merged 2 commits into from
Feb 6, 2019

Conversation

artembilan
Copy link
Member

https://stackoverflow.com/questions/54546728/how-to-handle-ftpoutboundadapter-connection-exception-in-spring-integration-flow

When channels are configured for the ExpressionEvaluatingRequestHandlerAdvice,
but no expressions, the logic is not performed.
In other words: we can evaluate expressions, when no channels,
but we don't send messages to channels when no expressions.

  • Provide default expressions to be evaluated to payload, when only
    channels are provided.

https://stackoverflow.com/questions/54546728/how-to-handle-ftpoutboundadapter-connection-exception-in-spring-integration-flow

When channels are configured for the `ExpressionEvaluatingRequestHandlerAdvice`,
but no expressions, the logic is not performed.
In other words: we can evaluate expressions, when no channels,
but we  don't send messages to channels when no expressions.

* Provide default expressions to be evaluated to `payload`, when only
channels are provided.
* @param onSuccessExpression the SpEL expression.
* @since 5.0
*/
public void setOnSuccessExpression(Expression onSuccessExpression) {
Assert.notNull(onSuccessExpression, "'onSuccessExpression' must not be null");
Copy link
Contributor

Choose a reason for hiding this comment

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

Breaking change; someone might have...

    advice.setFailureExpression(...);
    advice.setSuccessExpression(null);

Just for self-documentation purposes.

Maybe 5.2 only? Since the work around is simple?

However, why are these asserts even needed? You retained the null checks in doInvoke().

@@ -406,6 +406,8 @@ An additional property, called `inputMessage`, contains the original message sen
A message sent to the `failureChannel` (when the handler throws an exception) is an `ErrorMessage` with a payload of `MessageHandlingExpressionEvaluatingAdviceException`.
Like all `MessagingException` instances, this payload has `failedMessage` and `cause` properties, as well as an additional property called `evaluationResult`, which contains the result of the expression evaluation.

NOTE: Starting with version 5.1.3, if channels are configured, but expressions are not provided, the default expression is used to evaluate to a `payload` of the message.

Copy link
Contributor

Choose a reason for hiding this comment

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

to the payload if the message.

@artembilan
Copy link
Member Author

Pushed according your advises. I think with the current state it is not a breaking change because previously the channels just haven't had any value. They've been ignored.

@garyrussell
Copy link
Contributor

I agree, but it's not necessary to assert not null here; and users might have done it, just for completeness; no need to break such apps.

@garyrussell garyrussell merged commit 71fd60f into spring-projects:master Feb 6, 2019
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.

2 participants