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

build(deps): bump com.networknt:json-schema-validator from 1.0.87 to 1.4.3 #1674

Merged
merged 4 commits into from
Jul 2, 2024

Conversation

justin-tay
Copy link

Issue #, if available: #1663

Description of changes:

Upgrades com.networknt:json-schema-validator to 1.4.3.

Note that the validation error messages have changed since 1.0.87.

{
  "validationErrors": [
    {
      "type": "enum",
      "code": "1008",
      "message": "/properties/name/type: does not have a value in the enumeration [array, boolean, integer, null, number, object, string]",
      "instanceLocation": "/properties/name/type",
      "evaluationPath": "/properties/properties/additionalProperties/$ref/properties/type/anyOf/0/$ref/enum",
      "schemaLocation": "http://json-schema.org/draft-07/schema#/definitions/simpleTypes/enum",
      "messageKey": "enum",
      "arguments": [
        "[array, boolean, integer, null, number, object, string]"
      ]
    },
    {
      "type": "type",
      "code": "1029",
      "message": "/properties/name/type: string found, array expected",
      "instanceLocation": "/properties/name/type",
      "evaluationPath": "/properties/properties/additionalProperties/$ref/properties/type/anyOf/1/type",
      "schemaLocation": "http://json-schema.org/draft-07/schema#/properties/type/anyOf/1/type",
      "messageKey": "type",
      "arguments": [
        "string",
        "array"
      ]
    }
  ]
}

Checklist

Breaking change checklist

RFC issue #:

  • Migration process documented
  • Implement warnings (if it can live side by side)

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@jeromevdl
Copy link
Contributor

jeromevdl commented Jun 28, 2024

Thanks for submitting this PR. It looks good to me but I have 3 questions.

JsonSchemaFactory jsonSchemaFactory = JsonSchemaFactory.getInstance(VersionFlag.V202012, builder -> 
    builder.schemaMappers(schemaMappers -> schemaMappers.mapPrefix("https://www.example.org/", "classpath:schema/"))
);

@justin-tay
Copy link
Author

justin-tay commented Jun 28, 2024

Thanks for submitting this PR. It looks good to me but I have 3 questions.

  • Is there any breaking change compared to previous version: can we reuse the same schema files? is the validation the same (appart from the output message)? ...?

There were breaking API changes, but given that it doesn't seem like this library provides much options to customize the validator I'm not sure it applies here, like if users had custom keyword or format implementations etc that would require code changes.

This fixes a lot of issues with respect to compliance with the specifications with respect to the validation so if users relied on incorrect behavior that will break. For instance previously certain keywords for 2019-09 and in particular 2020-12 didn't work eg $dynamicRef, $dynamicAnchor.

  • Do we need to update the documentation to reflect this change? If yes, can you do it part of this PR.

I'm not really sure which aspect of the documentation would require updating. The message is likely the most visible change.

JsonSchemaFactory jsonSchemaFactory = JsonSchemaFactory.getInstance(VersionFlag.V202012, builder -> 
    builder.schemaMappers(schemaMappers -> schemaMappers.mapPrefix("https://www.example.org/", "classpath:schema/"))
);

There's actually no need to do that for the standard schema dialects because the MetaSchemaMapper will map the standard dialects to the classpath but in this case this library already maps them to the classpath which I opted to retain since I'm not sure if the intent was to prevent users from pulling other resources from https etc and only allow classpath resources.

@jeromevdl
Copy link
Contributor

@justin-tay, Can you try to run the two end-to-end tests in powertools-e2e-tests ? Then if it works we should be good. Thank you.

@justin-tay
Copy link
Author

Will there be any costs involved running the two end-to-end tests in powertools-e2e-tests?

@jeromevdl
Copy link
Contributor

The ValidationApiGWE2ET is based on API GW / Lambda, and you have free tier with these services so it should not cost anything. Do not test the ALB one if you don't want to expense anything.

@justin-tay
Copy link
Author

I only ran ValidationApiGWE2ET but I expect that ValidationALBE2ET should also pass with the updated messages.

[INFO] Tests run: 3, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 51.41 s -- in software.amazon.lambda.powertools.ValidationApiGWE2ET
[INFO]
[INFO] Results:
[INFO]
[INFO] Tests run: 3, Failures: 0, Errors: 0, Skipped: 0
[INFO]
[INFO]

Copy link
Contributor

@jeromevdl jeromevdl left a comment

Choose a reason for hiding this comment

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

Thank you, LGTM

@jeromevdl jeromevdl merged commit b4f9ee9 into aws-powertools:v2 Jul 2, 2024
8 of 12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants