Skip to content

Short-circuit methods for lambdas from annotation #2823

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

Conversation

artembilan
Copy link
Member

When we have a @ServiceActivator or any other messaging annotations
on Function or Consumer @Beans, there is a restriction when we
can't use lambdas because of target method argument type erasure in Java.

  • Use a @Bean method return to determine the target function argument
    type and wrap the call into the LambdaMessageProcessor.

In this case we call the target method directly after possible payload
conversion according expected generic type for Function or Consumer.
There is just no reason to go a MessagingMethodInvokerHelper route
for this lambda variants

@artembilan
Copy link
Member Author

If the solution is OK, I'll go ahead with other Messaging annotations.
Also need to check with Kotlin Lambdas and, of course, really check if the target classs is lambda...

Copy link
Contributor

@garyrussell garyrussell left a comment

Choose a reason for hiding this comment

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

Nice! Just one comment.


Class<?> expectedPayloadType = methodReturnType.getGeneric(0).toClass();
serviceActivator = new ServiceActivatingHandler(
new LambdaMessageProcessor(target, expectedPayloadType));
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you need to check for wildcard generic types here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure what you mean, but with the methodReturnType.getGeneric(0).toClass() it returns an Object class for a Consumer<?>. So, I think we are good.

@garyrussell garyrussell dismissed their stale review March 19, 2019 21:10

OK; in that case, looks good.

When we have a `@ServiceActivator` or any other messaging annotations
on `Function` or `Consumer` `@Bean`s, there is a restriction when we
can't use lambdas because of target method argument type erasure in Java.

* Use a `@Bean` method return to determine the target function argument
type and wrap the call into the `LambdaMessageProcessor`.

In this case we call the target method directly after possible payload
conversion according expected generic type for `Function` or `Consumer`.
There is just no reason to go a `MessagingMethodInvokerHelper` route
for this lambda variants

* Apply the short-circuit algorithm for Kotlin lambdas as well
* Make some refactoring and improvements to `ClassUtils`
if favor or similar API in the SF `ClassUtils`
* Fix `No beanFactory` warning for the `ExpressionCommandMessageProcessor`
@artembilan artembilan force-pushed the Generics_for_lambda_functions branch from d49d6d7 to 0dbdf0e Compare March 25, 2019 20:22
@artembilan artembilan changed the title [DO NOT MERGE YET] Short-circuit methods for lambdas from annotation Short-circuit methods for lambdas from annotation Mar 25, 2019
@artembilan artembilan requested a review from garyrussell March 25, 2019 20:22
@artembilan
Copy link
Member Author

Ready for review.

Thanks

@garyrussell
Copy link
Contributor

> Task :spring-integration-core:checkstyleMain
[ant:checkstyle] [ERROR] /home/travis/build/spring-projects/spring-integration/spring-integration-core/src/main/java/org/springframework/integration/config/annotation/ServiceActivatorAnnotationPostProcessor.java:23:8: Unused import - java.util.function.Consumer. [UnusedImports]
[ant:checkstyle] [ERROR] /home/travis/build/spring-projects/spring-integration/spring-integration-core/src/main/java/org/springframework/integration/config/annotation/ServiceActivatorAnnotationPostProcessor.java:24:8: Unused import - java.util.function.Function. [UnusedImports]
[ant:checkstyle] [ERROR] /home/travis/build/spring-projects/spring-integration/spring-integration-core/src/main/java/org/springframework/integration/config/annotation/ServiceActivatorAnnotationPostProcessor.java:28:8: Unused import - org.springframework.core.ResolvableType. [UnusedImports]
[ant:checkstyle] [ERROR] /home/travis/build/spring-projects/spring-integration/spring-integration-core/src/main/java/org/springframework/integration/config/annotation/ServiceActivatorAnnotationPostProcessor.java:32:8: Unused import - org.springframework.integration.handler.LambdaMessageProcessor. [UnusedImports]
[ant:checkstyle] [ERROR] /home/travis/build/spring-projects/spring-integration/spring-integration-core/src/main/java/org/springframework/integration/handler/ExpressionCommandMessageProcessor.java:55:17: Reference to instance variable 'methodFilter' needs "this.". [RequireThis]
[ant:checkstyle] [ERROR] /home/travis/build/spring-projects/spring-integration/spring-integration-core/src/main/java/org/springframework/integration/util/ClassUtils.java:20:8: Unused import - java.util.HashMap. [UnusedImports]
[ant:checkstyle] [ERROR] /home/travis/build/spring-projects/spring-integration/spring-integration-core/src/main/java/org/springframework/integration/util/ClassUtils.java:21:8: Unused import - java.util.Map. [UnusedImports]
> Task :spring-integration-core:checkstyleMain FAILED

@artembilan
Copy link
Member Author

Pushed the fix.

Thank you for pointing that out!

There is an AnnotationBeanUtils deprecated in SF, but that should be considered as a separate issue...

Copy link
Contributor

@garyrussell garyrussell left a comment

Choose a reason for hiding this comment

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

Just a couple of minor issues.

@@ -510,9 +515,26 @@ protected void checkMessageHandlerAttributes(String handlerBeanName, List<Annota
}
}

protected boolean resolveAttributeToBoolean(String requiresReply) {
return Boolean.parseBoolean(this.beanFactory.resolveEmbeddedValue(requiresReply));
Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds like a generic method name, but specific parameter name. String attribute ?

@@ -3118,7 +3122,8 @@ protected StandardIntegrationFlow get() {
if (this.integrationFlow == null) {
if (this.currentMessageChannel instanceof FixedSubscriberChannelPrototype) {
throw new BeanCreationException("The 'currentMessageChannel' (" + this.currentMessageChannel
+ ") is a prototype for 'FixedSubscriberChannel' which can't be created without 'MessageHandler' "
+ ") is a prototype for 'FixedSubscriberChannel' which can't be created without " +
"'MessageHandler' "
Copy link
Contributor

Choose a reason for hiding this comment

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

without a 'MessageHandler'.

* Fix typo in the exception message for `IntegrationFlowDefinition.get()`
@garyrussell
Copy link
Contributor

org.springframework.integration.jmx.config.OperationInvokingOutboundGatewayTests > gatewayWithPrimitiveArgs FAILED
    org.springframework.messaging.MessagingException at OperationInvokingOutboundGatewayTests.java:103

@garyrussell garyrussell merged commit 572dc0e into spring-projects:master Mar 28, 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