Skip to content
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

CAMEL-12079: Bean language: support bean::function notation #2156

Merged
merged 1 commit into from
Mar 4, 2018

Conversation

snurmine
Copy link
Contributor

No description provided.

@oscerd
Copy link
Contributor

oscerd commented Jan 3, 2018

Looks good, but not sure if we want to add other stuff to the bean language. @davsclaus WDYT?

@snurmine
Copy link
Contributor Author

snurmine commented Jan 3, 2018

I just took the ticket from JIRA. If not needed, I can close the PR.

@oscerd
Copy link
Contributor

oscerd commented Jan 3, 2018

yeah, my fault. Too much tickets I'm looking at :-)

@oscerd
Copy link
Contributor

oscerd commented Jan 3, 2018

I'll merge tomorrow :-)

Thanks for the PR.

@snurmine
Copy link
Contributor Author

snurmine commented Jan 3, 2018

Feel free to wait comments etc.

@davsclaus
Copy link
Contributor

davsclaus commented Jan 4, 2018

Specifying a method name on a bean is used in a few other places as well. Just search the code for similar pattern

I think we should also get input from @lburgazzoli so lets wait till he get back from PTO

@lburgazzoli
Copy link
Contributor

If there are other places where the . notation is used we should update that too

if (doubleColonIndex > 0) {
beanName = expression.substring(0, doubleColonIndex);
method = expression.substring(doubleColonIndex + 2);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

We should check first for ::`` then for .otherwise expressions likemy.own.Bean::function` would lead to error

@lburgazzoli
Copy link
Contributor

@snurmine do you have any time to complete this PR ?

@snurmine
Copy link
Contributor Author

I try after vacation 8.2

@snurmine
Copy link
Contributor Author

Tried to fix issues. Not sure tough if found every place (I search using ?method=)

@oscerd
Copy link
Contributor

oscerd commented Mar 2, 2018

@lburgazzoli can you review? This looks good to me.

@lburgazzoli
Copy link
Contributor

@snurmine it looks good to me, the only additional think I'd like to have is a test that validate method resolution for cases like "my.company.MyClass::method"

@davsclaus
Copy link
Contributor

We need to check for ?method= first because there is a problem with checking for double colon first, as you can potentially pass in parameter values where its included.

bean("myBean?method=blah('hello::world', 6)")

Also a little bit of a 3rd party problem is that Camel tooling need to be aware of this and also add support for checking the double colon syntax as well - but that should be possible. We need to add this to the release note of this change.

Also we need to educate Camel users that the :: syntax is to refer to a method name, its not the same as the Java :: method reference that does the lambda thing. I dont think in Java you can pass in parameter values as you would be able to do in Camel, eg

bean("myBean::blah('hello::world', 6)")

@snurmine
Copy link
Contributor Author

snurmine commented Mar 2, 2018

Added more test cases to BeanTest. ResourceHelper delegates parsing to BeanLanguage now.
I understand that :: is mean tto support dots in the bean name, I mean my.bean.Method('a.b') is not supposed to work?

@davsclaus davsclaus merged commit 701e033 into apache:master Mar 4, 2018
Croway pushed a commit to Croway/camel that referenced this pull request Mar 14, 2024
[ENTESB-20648] Upgrade to Spring Boot 2.7.9, Spring Batch 4.3.8 and S…
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.

4 participants