Add default expressions for ExpEvalReqHAdvice (#2738)

* Add default expressions for ExpEvalReqHAdvice

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.

* * Remove asserts for expression setters

Author: artembilan

spring-integration-core/src/main/java/org/springframework/integration/handler/advice/ExpressionEvaluatingRequestHandlerAdvice.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2018 the original author or authors.
+ * Copyright 2002-2019 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -16,17 +16,21 @@
 
 package org.springframework.integration.handler.advice;
 
+import org.springframework.beans.factory.BeanFactory;
 import org.springframework.expression.EvaluationContext;
 import org.springframework.expression.Expression;
-import org.springframework.expression.spel.standard.SpelExpressionParser;
 import org.springframework.expression.spel.support.StandardEvaluationContext;
 import org.springframework.integration.core.MessagingTemplate;
 import org.springframework.integration.expression.ExpressionUtils;
+import org.springframework.integration.expression.FunctionExpression;
 import org.springframework.integration.message.AdviceMessage;
+import org.springframework.lang.Nullable;
 import org.springframework.messaging.Message;
 import org.springframework.messaging.MessageChannel;
 import org.springframework.messaging.MessagingException;
+import org.springframework.messaging.core.DestinationResolver;
 import org.springframework.messaging.support.ErrorMessage;
+import org.springframework.util.StringUtils;
 
 /**
  * Used to advise {@link org.springframework.messaging.MessageHandler}s.
@@ -36,53 +40,61 @@
  * containing the evaluation result in its payload and the {@code inputMessage} property containing
  * the original message that was sent to the endpoint.
  * The failure expression is NOT evaluated if the success expression throws an exception.
+ * <p>
+ * When expressions are not configured, but channels are, the default expression is evaluated
+ * just into a {@code payload} from the message.
  *
  * @author Gary Russell
  * @author Artem Bilan
+ *
  * @since 2.2
  *
  */
 public class ExpressionEvaluatingRequestHandlerAdvice extends AbstractRequestHandlerAdvice {
 
-	private volatile Expression  onSuccessExpression;
+	private static final Expression DEFAULT_EXPRESSION = new FunctionExpression<Message<?>>(Message::getPayload);
+
+	private final MessagingTemplate messagingTemplate = new MessagingTemplate();
 
-	private volatile MessageChannel successChannel;
+	private Expression onSuccessExpression;
 
-	private volatile String successChannelName;
+	private MessageChannel successChannel;
 
-	private volatile Expression onFailureExpression;
+	private String successChannelName;
 
-	private volatile MessageChannel failureChannel;
+	private Expression onFailureExpression;
 
-	private volatile String failureChannelName;
+	private MessageChannel failureChannel;
 
-	private final MessagingTemplate messagingTemplate = new MessagingTemplate();
+	private String failureChannelName;
 
-	private volatile boolean trapException = false;
+	private boolean trapException = false;
 
-	private volatile boolean returnFailureExpressionResult = false;
+	private boolean returnFailureExpressionResult = false;
 
-	private volatile boolean propagateOnSuccessEvaluationFailures;
+	private boolean propagateOnSuccessEvaluationFailures;
 
-	private volatile EvaluationContext evaluationContext;
+	private EvaluationContext evaluationContext;
 
 	/**
 	 * Set the expression to evaluate against the message after a successful
 	 * handler invocation.
+	 * Defaults to {@code payload}, if {@code successChannel} is configured.
 	 * @param onSuccessExpression the SpEL expression.
 	 * @since 4.3.7
 	 */
 	public void setOnSuccessExpressionString(String onSuccessExpression) {
-		this.onSuccessExpression = new SpelExpressionParser().parseExpression(onSuccessExpression);
+		setOnSuccessExpression(EXPRESSION_PARSER.parseExpression(onSuccessExpression));
 	}
 
 	/**
 	 * Set the expression to evaluate against the message after a successful
 	 * handler invocation.
+	 * Defaults to {@code payload}, if {@code successChannel} is configured.
 	 * @param onSuccessExpression the SpEL expression.
 	 * @since 5.0
 	 */
-	public void setOnSuccessExpression(Expression onSuccessExpression) {
+	public void setOnSuccessExpression(@Nullable Expression onSuccessExpression) {
 		this.onSuccessExpression = onSuccessExpression;
 	}
 
@@ -94,26 +106,28 @@ public void setOnSuccessExpression(Expression onSuccessExpression) {
 	 */
 	@Deprecated
 	public void setExpressionOnSuccess(Expression onSuccessExpression) {
-		this.onSuccessExpression = onSuccessExpression;
+		setOnSuccessExpression(onSuccessExpression);
 	}
 
 	/**
 	 * Set the expression to evaluate against the root message after a failed
-	 * handler invocation. The exception is available as the variable {@code #exception}
+	 * handler invocation. The exception is available as the variable {@code #exception}.
+	 * Defaults to {@code payload}, if {@code failureChannel} is configured.
 	 * @param onFailureExpression the SpEL expression.
 	 * @since 4.3.7
 	 */
 	public void setOnFailureExpressionString(String onFailureExpression) {
-		this.onFailureExpression = new SpelExpressionParser().parseExpression(onFailureExpression);
+		setOnFailureExpression(EXPRESSION_PARSER.parseExpression(onFailureExpression));
 	}
 
 	/**
 	 * Set the expression to evaluate against the root message after a failed
-	 * handler invocation. The exception is available as the variable {@code #exception}
+	 * handler invocation. The exception is available as the variable {@code #exception}.
+	 * Defaults to {@code payload}, if {@code failureChannel} is configured.
 	 * @param onFailureExpression the SpEL expression.
 	 * @since 5.0
 	 */
-	public void setOnFailureExpression(Expression onFailureExpression) {
+	public void setOnFailureExpression(@Nullable Expression onFailureExpression) {
 		this.onFailureExpression = onFailureExpression;
 	}
 
@@ -125,7 +139,7 @@ public void setOnFailureExpression(Expression onFailureExpression) {
 	 */
 	@Deprecated
 	public void setExpressionOnFailure(Expression onFailureExpression) {
-		this.onFailureExpression = onFailureExpression;
+		setOnFailureExpression(onFailureExpression);
 	}
 
 	/**
@@ -178,7 +192,6 @@ public void setTrapException(boolean trapException) {
 	/**
 	 * If true, the result of evaluating the onFailureExpression will
 	 * be returned as the result of AbstractReplyProducingMessageHandler.handleRequestMessage(Message).
-	 *
 	 * @param returnFailureExpressionResult true to return the result of the evaluation.
 	 */
 	public void setReturnFailureExpressionResult(boolean returnFailureExpressionResult) {
@@ -200,24 +213,39 @@ public void setPropagateEvaluationFailures(boolean propagateOnSuccessEvaluationF
 	@Override
 	protected void onInit() {
 		super.onInit();
-		if (this.getBeanFactory() != null) {
-			this.messagingTemplate.setBeanFactory(this.getBeanFactory());
+		BeanFactory beanFactory = getBeanFactory();
+		if (beanFactory != null) {
+			this.messagingTemplate.setBeanFactory(beanFactory);
+		}
+
+		if (this.onSuccessExpression == null
+				&& (this.successChannel != null || StringUtils.hasText(this.successChannelName))) {
+
+			this.onSuccessExpression = DEFAULT_EXPRESSION;
+		}
+
+		if (this.onFailureExpression == null
+				&& (this.failureChannel != null || StringUtils.hasText(this.failureChannelName))) {
+
+			this.onFailureExpression = DEFAULT_EXPRESSION;
 		}
 	}
 
 	@Override
-	protected Object doInvoke(ExecutionCallback callback, Object target, Message<?> message) throws Exception {
+	protected Object doInvoke(ExecutionCallback callback, Object target, Message<?> message)
+			throws Exception { // NOSONAR
+
 		try {
 			Object result = callback.execute();
 			if (this.onSuccessExpression != null) {
-				this.evaluateSuccessExpression(message);
+				evaluateSuccessExpression(message);
 			}
 			return result;
 		}
 		catch (Exception e) {
-			Exception actualException = this.unwrapExceptionIfNecessary(e);
+			Exception actualException = unwrapExceptionIfNecessary(e);
 			if (this.onFailureExpression != null) {
-				Object evalResult = this.evaluateFailureExpression(message, actualException);
+				Object evalResult = evaluateFailureExpression(message, actualException);
 				if (this.returnFailureExpressionResult) {
 					return evalResult;
 				}
@@ -229,68 +257,69 @@ protected Object doInvoke(ExecutionCallback callback, Object target, Message<?>
 		}
 	}
 
-	private void evaluateSuccessExpression(Message<?> message) throws Exception {
+	private void evaluateSuccessExpression(Message<?> message) throws Exception { // NOSONAR
 		Object evalResult;
-		boolean evaluationFailed = false;
 		try {
-			evalResult = this.onSuccessExpression.getValue(this.prepareEvaluationContextToUse(null), message);
+			evalResult = this.onSuccessExpression.getValue(prepareEvaluationContextToUse(null), message);
 		}
 		catch (Exception e) {
 			evalResult = e;
-			evaluationFailed = true;
 		}
-		if (this.successChannel == null && this.successChannelName != null && getChannelResolver() != null) {
-			this.successChannel = getChannelResolver().resolveDestination(this.successChannelName);
+		DestinationResolver<MessageChannel> channelResolver = getChannelResolver();
+		if (this.successChannel == null && this.successChannelName != null && channelResolver != null) {
+			this.successChannel = channelResolver.resolveDestination(this.successChannelName);
 		}
 		if (evalResult != null && this.successChannel != null) {
-			AdviceMessage<?> resultMessage = new AdviceMessage<Object>(evalResult, message);
+			AdviceMessage<?> resultMessage = new AdviceMessage<>(evalResult, message);
 			this.messagingTemplate.send(this.successChannel, resultMessage);
 		}
-		if (evaluationFailed && this.propagateOnSuccessEvaluationFailures) {
+		if (evalResult instanceof Exception && this.propagateOnSuccessEvaluationFailures) {
 			throw (Exception) evalResult;
 		}
 	}
 
-	private Object evaluateFailureExpression(Message<?> message, Exception exception) throws Exception {
+	private Object evaluateFailureExpression(Message<?> message, Exception exception) {
 		Object evalResult;
 		try {
-			evalResult = this.onFailureExpression.getValue(this.prepareEvaluationContextToUse(exception), message);
+			evalResult = this.onFailureExpression.getValue(prepareEvaluationContextToUse(exception), message);
 		}
 		catch (Exception e) {
 			evalResult = e;
 			logger.error("Failure expression evaluation failed for " + message + ": " + e.getMessage());
 		}
-		if (this.failureChannel == null && this.failureChannelName != null && getChannelResolver() != null) {
-			this.failureChannel = getChannelResolver().resolveDestination(this.failureChannelName);
+		DestinationResolver<MessageChannel> channelResolver = getChannelResolver();
+		if (this.failureChannel == null && this.failureChannelName != null && channelResolver != null) {
+			this.failureChannel = channelResolver.resolveDestination(this.failureChannelName);
 		}
 		if (evalResult != null && this.failureChannel != null) {
-			MessagingException messagingException = new MessageHandlingExpressionEvaluatingAdviceException(message,
-					"Handler Failed", this.unwrapThrowableIfNecessary(exception), evalResult);
-			ErrorMessage resultMessage = new ErrorMessage(messagingException);
-			this.messagingTemplate.send(this.failureChannel, resultMessage);
+			MessagingException messagingException =
+					new MessageHandlingExpressionEvaluatingAdviceException(message, "Handler Failed",
+							unwrapThrowableIfNecessary(exception), evalResult);
+			ErrorMessage errorMessage = new ErrorMessage(messagingException);
+			this.messagingTemplate.send(this.failureChannel, errorMessage);
 		}
 		return evalResult;
 	}
 
 	protected StandardEvaluationContext createEvaluationContext() {
-		return ExpressionUtils.createStandardEvaluationContext(this.getBeanFactory());
+		return ExpressionUtils.createStandardEvaluationContext(getBeanFactory());
 	}
 
 	/**
 	 * If we don't need variables (i.e., exception is null)
 	 * we can use a singleton context; otherwise we need a new one each time.
-	 * @param exception
+	 * @param exception the {@link Exception} to use in the context.
 	 * @return The context.
 	 */
 	private EvaluationContext prepareEvaluationContextToUse(Exception exception) {
 		EvaluationContext evaluationContextToUse;
 		if (exception != null) {
-			evaluationContextToUse = this.createEvaluationContext();
+			evaluationContextToUse = createEvaluationContext();
 			evaluationContextToUse.setVariable("exception", exception);
 		}
 		else {
 			if (this.evaluationContext == null) {
-				this.evaluationContext = this.createEvaluationContext();
+				this.evaluationContext = createEvaluationContext();
 			}
 			evaluationContextToUse = this.evaluationContext;
 		}
@@ -305,6 +334,7 @@ public static class MessageHandlingExpressionEvaluatingAdviceException extends M
 
 		public MessageHandlingExpressionEvaluatingAdviceException(Message<?> message, String description,
 				Throwable cause, Object evaluationResult) {
+
 			super(message, description, cause);
 			this.evaluationResult = evaluationResult;
 		}

spring-integration-core/src/test/java/org/springframework/integration/configuration/EnableIntegrationTests.java

@@ -1163,7 +1163,6 @@ public MessageHandler myHandler() {
 		@Bean
 		public Advice myHandlerAdvice() {
 			ExpressionEvaluatingRequestHandlerAdvice advice = new ExpressionEvaluatingRequestHandlerAdvice();
-			advice.setOnSuccessExpressionString("payload");
 			advice.setSuccessChannel(myHandlerSuccessChannel());
 			return advice;
 		}

spring-integration-core/src/test/java/org/springframework/integration/dsl/flows/IntegrationFlowTests.java

@@ -644,7 +644,6 @@ public static class ContextConfiguration2 {
 		@Bean
 		public Advice expressionAdvice() {
 			ExpressionEvaluatingRequestHandlerAdvice advice = new ExpressionEvaluatingRequestHandlerAdvice();
-			advice.setOnSuccessExpressionString("payload");
 			advice.setSuccessChannel(this.successChannel);
 			return advice;
 		}

src/reference/asciidoc/handler-advice.adoc

@@ -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 the `payload` of the message.
+
 When an exception is thrown in the scope of the advice, by default, that exception is thrown to the caller after any `failureExpression` is evaluated.
 If you wish to suppress throwing the exception, set the `trapException` property to `true`.
 The following advice shows how to configure an advice with Java DSL: