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

feat(ConfigForm): Add support for oneOf fields #1551

Merged
merged 1 commit into from
Oct 10, 2024

Conversation

lordrip
Copy link
Member

@lordrip lordrip commented Oct 9, 2024

Context

Currently, we have partial support for the oneOf schemas.

This commit cleans up the generated schemas, by removing the not definition from oneOf and also removing the empty properties like:

{
  property: { }
}

Test the changes live: https://lordrip.github.io/kaoto/

- errorHandler: {}

fix: #1550
fix: #948

@lordrip lordrip requested review from tplevko, shivamG640, igarashitm and apupier and removed request for tplevko October 9, 2024 16:16
Copy link
Member

@apupier apupier left a comment

Choose a reason for hiding this comment

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

I do not understand what to look at and verify exactly. What is modified functionally?
I tried in VS Code and I can still create routes.

Comment on lines 78 to 82
assertFalse(properties.has("langChain4jCharacterTokenizer"));
assertFalse(properties.has("langChain4jLineTokenizer"));
assertFalse(properties.has("langChain4jParagraphTokenizer"));
assertFalse(properties.has("langChain4jSentenceTokenizer"));
assertFalse(properties.has("langChain4jWordTokenizer"));
Copy link
Member

Choose a reason for hiding this comment

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

to help understand error failure, it is convenien tto provide an error message on the reason

Copy link
Member Author

Choose a reason for hiding this comment

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

Added

}

@Test
public void testRemoveEmptyProperties() throws Exception {
Copy link
Member

Choose a reason for hiding this comment

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

in JUnit 5, it is recommended to not use the the public modifier and keep default package
https://junit.org/junit5/docs/current/user-guide/#writing-tests-classes-and-methods

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, I updated all tests to follow that convention

Currently, we have partial support for the `oneOf` schemas.

This commit cleans up the generated schemas, by removing the `not`
definition from `oneOf` and also removing the empty properties like:

'''
{
  property: { }
}
'''

fix: KaotoIO#1550
fix: KaotoIO#948
@lordrip lordrip force-pushed the feat/support-for-oneof-fields branch from 80a26ff to baf4bbd Compare October 10, 2024 10:11
Copy link

sonarcloud bot commented Oct 10, 2024

Copy link

codecov bot commented Oct 10, 2024

Codecov Report

Attention: Patch coverage is 92.15686% with 4 lines in your changes missing coverage. Please review.

Please upload report for BASE (main@f95f50e). Learn more about missing BASE report.

Files with missing lines Patch % Lines
...catalog/generator/CamelYamlDslSchemaProcessor.java 88.57% 3 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1551   +/-   ##
=======================================
  Coverage        ?   80.02%           
  Complexity      ?      272           
=======================================
  Files           ?      276           
  Lines           ?     7825           
  Branches        ?     1496           
=======================================
  Hits            ?     6262           
  Misses          ?     1502           
  Partials        ?       61           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@lordrip
Copy link
Member Author

lordrip commented Oct 10, 2024

I do not understand what to look at and verify exactly. What is modified functionally?

Adding support for oneOf elements in the JSON Schema, used by errorHandler, the new tokenizer EIP, among others. If you check Camel 4.9.0-SNAPSHOT, you'll notice that the tokenizer doesn't show any tokenizer option, just the default id and descrption.

In the past, oneOf was supported at the root level, in a separate code branch, now that code branch was consolidated for all oneOf fields.

I tried in VS Code and I can still create routes.

I wouldn't expect less 😅

@lordrip lordrip merged commit 2d5d990 into KaotoIO:main Oct 10, 2024
12 checks passed
@lordrip lordrip deleted the feat/support-for-oneof-fields branch October 10, 2024 13:11
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.

Support for Tokenizer EIP Support nested anyOf/oneOf schemas
3 participants