Skip to content

Fix spring-integration sample #411

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Sep 25, 2019
Merged

Fix spring-integration sample #411

merged 3 commits into from
Sep 25, 2019

Conversation

olegz
Copy link
Contributor

@olegz olegz commented Sep 24, 2019

  • Added additional proxy checks
  • Added discoverFunctionTypeFromClass() method to FunctionTypeUtils

@olegz
Fix spring-integration sample
65fd059 
- Added additional proxy checks
- Added discoverFunctionTypeFromClass() method to FunctionTypeUtils
@olegz olegz requested a review from artembilan September 24, 2019 21:38
@olegz olegz added this to the 3.0.0.M4 milestone Sep 24, 2019
@olegz olegz added the bug label Sep 24, 2019
artembilan
artembilan suggested changes Sep 24, 2019
View reviewed changes
Copy link
Member

@artembilan artembilan left a comment

Choose a reason for hiding this comment

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

Just only one concern about <T>.

To make your build working properly on Travis you need to add this entry into the .travis.yml:

dist: trusty

...ple-spring-integration/src/main/java/example/FunctionSampleSpringIntegrationApplication.java Outdated
public IntegrationFlow uppercaseFlow() {
return IntegrationFlows.from(MessageFunction.class, "uppercase")
.<String, String>transform(String::toUpperCase)
.logAndReply(LoggingHandler.Level.WARN);
}

public interface MessageFunction extends Function<Message<String>, Message<String>> {
public interface MessageFunction<T> extends Function<Message<String>, Message<String>> {
Copy link
Member

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 <T> here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No reason, just wanted to create a deep hierarchy and validate that the new discover method still finds the function type.

Copy link
Member

Choose a reason for hiding this comment

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

OK, but it must not be as a part of a sample. It is going to confuse end-users.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ohh, i forgot to delete it. Good point

@artembilan
Copy link
Member

@olegz ,

I want to share with you another point of view for this problem.

Instead of assuming about proxies, how about to perform a registry.getBean("&" + actualName, FactoryBean.class).getObjectType() logic in the FunctionContextUtils.findType(), when FactoryBean.class.isAssinableFrom(definition.getBeanClass()) and call the same new FunctionTypeUtils.discoverFunctionTypeFromClass()?

artembilan
artembilan approved these changes Sep 25, 2019
View reviewed changes
Copy link
Member

@artembilan artembilan left a comment

Choose a reason for hiding this comment

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

LGTM. Merging...

@artembilan artembilan merged commit 97bea81 into spring-cloud:master Sep 25, 2019
@olegz olegz deleted the INT-FIX branch December 27, 2022 11:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@artembilan artembilan artembilan approved these changes

Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
3.0.0.RC1
Development

Successfully merging this pull request may close these issues.
2 participants
@olegz @artembilan