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

Improvements to the schema for the key_value processor. #5051

Merged
merged 2 commits into from
Oct 15, 2024

Conversation

dlvenable
Copy link
Member

Description

This adds a none option to the whitespace enum while still supporting empty string. It includes default values to prevent fields from displaying that they are required. It also provides a solution for the schema generation which removes any property with a defaultValue from the list of required fields.

Issues Resolved

N/A

Check List

  • New functionality includes testing.
  • New functionality has a documentation issue. Please link to it in this PR.
    • New functionality has javadoc added
  • Commits are signed with a real name per the DCO

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

This adds a none option to the whitespace enum while still supporting empty string. It includes default values to prevent fields from displaying that they are required. It also provides a solution for the schema generation which removes any property with a defaultValue from the list of required fields.

Signed-off-by: David Venable <[email protected]>
protected boolean isRequired(final MemberScope<?, ?> member) {
final JsonProperty jsonPropertyAnnotation = member.getAnnotationConsideringFieldAndGetter(JsonProperty.class);
if(jsonPropertyAnnotation != null) {
if(jsonPropertyAnnotation.defaultValue() != null && !jsonPropertyAnnotation.defaultValue().isEmpty()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we have any cases where null value is allowed but defaultValue is not null?

Copy link
Member Author

Choose a reason for hiding this comment

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

Technically you cannot provide null to the the annotation's defaultValue field. So this should never be reached. I added the != null as a defensive programming approach.

Copy link
Member Author

Choose a reason for hiding this comment

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

For a more general point-of-view, anytime that a defaultValue is provided, regardless of the value, then the field is not required.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sorry my misread. I thought this condition means required.

@@ -44,6 +45,8 @@ Function<String, String> getTransformFunction() {

@JsonCreator
public static TransformOption fromTransformName(final String transformName) {
if(Objects.equals(transformName, ""))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: space

Signed-off-by: David Venable <[email protected]>
@graytaylor0 graytaylor0 merged commit 729c66f into opensearch-project:main Oct 15, 2024
46 of 47 checks passed
san81 pushed a commit to san81/data-prepper that referenced this pull request Oct 17, 2024
@dlvenable dlvenable deleted the key_value-schema-fix branch October 22, 2024 21:49
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.

3 participants