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-19386: Support object property on template bean #10205

Merged
merged 1 commit into from
May 25, 2023

Conversation

igarashitm
Copy link
Contributor

@igarashitm igarashitm commented May 24, 2023

Description

Fixes: https://issues.apache.org/jira/browse/CAMEL-19386

Also fixed the annotation for beans property on TemplatedRouteDefinitionDeserializer and RouteTemplateDefinitionDeserializer to represent what deserializers actually supports in the generated schema

Target

  • I checked that the commit is targeting the correct branch (note that Camel 3 uses camel-3.x, whereas Camel 4 uses the main branch)

Tracking

  • If this is a large change, bug fix, or code improvement, I checked there is a JIRA issue filed for the change (usually before you start working on it).

Apache Camel coding standards and style

  • I checked that each commit in the pull request has a meaningful subject line and body.
  • I formatted the code using mvn -Pformat,fastinstall install && mvn -Psourcecheck

Cc @lburgazzoli

Hi @grgrzybek , I believe I made it backward compatible for XML DSL for template bean property, while it adds properties to accept object property like non-template bean style. Hopefully this won't bother your current work...

@github-actions
Copy link
Contributor

🌟 Thank you for your contribution to the Apache Camel project! 🌟

🐫 Maintainers, please note that first-time contributors require manual approval for the GitHub Actions to run.

⚠️ Please note that the changes on this PR may be tested automatically if they change components.

If necessary Apache Camel Committers may access logs and test results in the job summaries!

@github-actions
Copy link
Contributor

Components tested:

Total Tested Failed ❌ Passed ✅
1 1 2 0

Also fixed the annotation for `beans` property on TemplatedRouteDefinitionDeserializer and RouteTemplateDefinitionDeserializer to represent what deserializers actually supports in the generated schema
@github-actions
Copy link
Contributor

Components tested:

Total Tested Failed ❌ Passed ✅
1 1 2 0

@igarashitm
Copy link
Contributor Author

hmm components/camel-spring-xml tests doesn't fail for me, can I look at the workflow test log?

@grgrzybek
Copy link
Contributor

I'll check this PR before noon (CEST).

@grgrzybek
Copy link
Contributor

grgrzybek commented May 25, 2023

yes, this is what I was thinking about and it's ok (in org.apache.camel.model.BeanFactoryDefinition):

@XmlElement(name = "properties")
@XmlJavaTypeAdapter(BeanPropertiesAdapter.class)
private Map<String, Object> properties;

@grgrzybek
Copy link
Contributor

This YAML/XML/JAXB consistency is tricky, but finally I got the PR - looks good ;)

Copy link
Contributor

@oscerd oscerd left a comment

Choose a reason for hiding this comment

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

It's there something to know in relation to the work ongoin in apache/camel-kamelets#1475

Copy link
Contributor

@oscerd oscerd left a comment

Choose a reason for hiding this comment

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

It's there something to know in relation to the work ongoing in apache/camel-kamelets#1475 ?

@igarashitm
Copy link
Contributor Author

@oscerd There shouldn't be any problem - this change fixes the schema to allow both property styles, so apache/camel-kamelets#1475 is not mandatory after this. But I think it's still beneficial to do that and align the style with non-template bean on official Kamelets.

@oscerd
Copy link
Contributor

oscerd commented May 25, 2023

I'll complete the work on that side. Thanks

@igarashitm igarashitm merged commit be5dd92 into apache:main May 25, 2023
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants