GH-2690: Add MessageHandlerMethodFactory beans #2798
Conversation
Fixes spring-projects#2690 For more end-user flexibility and for simplicity of the `MessagingMethodInvokerHelper` logic, add `IntegrationContextUtils.MESSAGE_HANDLER_FACTORY_BEAN_NAME` and `IntegrationContextUtils.LIST_MESSAGE_HANDLER_FACTORY_BEAN_NAME` infrastructure beans * Use mentioned beans in the `MessagingMethodInvokerHelper` if any, otherwise fallback to the locally configured `MessageHandlerMethodFactory`, although this is necessary only for our non-Spring-based unit tests * Relax `BeanFactory` requirement for the `EvaluationContext` since it is going to be there in the Spring-based environment, but might not be available in the plain unit tests * Provide some optimization and refactoring for the `MessagingMethodInvokerHelper` * Remove deprecated `HandlerMethodArgumentResolversHolder` and `ARGUMENT_RESOLVERS_BEAN_NAME` & `LIST_ARGUMENT_RESOLVERS_BEAN_NAME` bean definitions and constants for their names
|
I haven't reviewed yet but the reason we log a WARN with a missing BF is because we had several components in the framework which failed to propagate the BF to the evaluator. The warn helps us find any new errors of that kind. I am not so keen on disabling it. |
Oh! I missed that. Sure, will revert. |
* Fix `MessagingMethodInvokerHelper` to propagate `BeanFactory` to argument resolvers if any
| this.targetObject.getClass()); | ||
| if (LOGGER.isDebugEnabled()) { | ||
| LOGGER.debug(this.targetObject.getClass() + | ||
| ": Ambiguous fallback methods; using RequestReplyExchanger.exchange()"); |
There was a problem hiding this comment.
Why do we lose this debug log?
There was a problem hiding this comment.
I don’t understand why we need it at all even with the current master. Looks like we don’t do any fall backs, but just choose a first one for the exchange and that’s all. My feelings that it is a left over after some radical change here. So, since it doesn’t bring any value , remove it altogether!
|
|
||
| @Test | ||
| public void simplePayloadConfiguredWithMethodReference() throws Exception { | ||
| void simplePayloadConfiguredWithMethodReference() throws Exception { |
There was a problem hiding this comment.
Why change from public?
There was a problem hiding this comment.
Because it is now JUnit 5 test and that one doesn’t require public on methods. My point is that eventually we have to come fully only to JUnit 6 or already 6. The 4th one might just go EOL some day...
There was a problem hiding this comment.
Just because public is not required, I don't see why it's worth the effort to change it all over the place; seems like unnecessary work to me.
|
The problem with Travis is fully related to the story about unavailability if our repo. So, we might need to hold merge of this off until resolution of that problem. |
|
So, the Travis build is green. |
Fixes #2690
For more end-user flexibility and for simplicity of the
MessagingMethodInvokerHelperlogic, add
IntegrationContextUtils.MESSAGE_HANDLER_FACTORY_BEAN_NAMEandIntegrationContextUtils.LIST_MESSAGE_HANDLER_FACTORY_BEAN_NAMEinfrastructure beans
MessagingMethodInvokerHelperif any,otherwise fallback to the locally configured
MessageHandlerMethodFactory,although this is necessary only for our non-Spring-based unit tests
BeanFactoryrequirement for theEvaluationContextsince itis going to be there in the Spring-based environment, but might not be
available in the plain unit tests
MessagingMethodInvokerHelperHandlerMethodArgumentResolversHolderandARGUMENT_RESOLVERS_BEAN_NAME&LIST_ARGUMENT_RESOLVERS_BEAN_NAMEbean definitions and constants for their names