-
Notifications
You must be signed in to change notification settings - Fork 5.1k
feat(api): add exchange property and variable helpers #21095
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
base: main
Are you sure you want to change the base?
Conversation
Closes CAMEL-22910
| List<T> result = new ArrayList<>(); | ||
| for (Object element : iterable) { | ||
| if (!elementType.isInstance(element)) { | ||
| return null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we return null, skip the item or throw the exception? I had followed the original API, for which any error on the cast of the property would return null. But maybe we can change it in these implementations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what about skipping and LOG.warn the skipped element?
| } | ||
|
|
||
| if (!valueType.isInstance(rawValue)) { | ||
| return null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same question about the casting posed above.
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🤖 CI automation will test this PR automatically. 🐫 Apache Camel Committers, please review the following items:
|
|
Beware the Exchange API is already big and its a central API for Camel end users. End users should use variables and not exchange properties. The latter is more for Camel internally and component developers and old school before variables was added to Camel. We have ExchangeHelper where extra methods is better suited that are not commonly in use, or would rarely need to be. So if you have some internal API cleanup where you want to remove those suppress warning then IMHO its not new methods on Exchange we should add, but on ExchangeHelper and then use that internally in Camel. |
|
|
||
| List<T> result = new ArrayList<>(); | ||
| for (Object element : iterable) { | ||
| if (!elementType.isInstance(element)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please note that this isIstance check within a loop is very likely to hit the JVM secondary super issue which triggers a false sharing problem and affects pretty much everything older than Java 22.
|
Thanks for the reviews, the goal of this PR is more kind of experimental to provide some simplification to both final users and internal development as well. More than just skipping the warnings it is an attempt to bring some easier API for the final user. I will move them into the ExchangeHelper. I was also thinking to do some similar upper layer API for making it simpler to extract the body. I'll keep it as a draft while I continue refining these possibilities. |
Closes CAMEL-22910
This one could be a good helper for final users to spare time on casting and using raw types. The new utility methods allow the user passing from the following:
to:
There are also utilities for Map, Collections, Set and Optional on
getPropertyandgetVariable.Description
Target
mainbranch)Tracking
Apache Camel coding standards and style
mvn clean install -DskipTestslocally from root folder and I have committed all auto-generated changes.