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: create json schema for user-input recipe #222

Merged

Conversation

nullPointer373
Copy link

@nullPointer373 nullPointer373 commented Aug 7, 2023

Issue #, if available:

Description of changes:
I have added a JSON schema file that defines the user-input recipe format and structure. The schema is based on the existing recipe definition in its Java package and its documentation, which allows for further semantic validation.

Why is this change necessary:
The JSON schema is essential for validating user-input recipes and ensuring they conform to the expected format. By using the schema, we can enforce constraints on the recipe properties and catch any potential errors or invalid input early in the process.

How was this change tested:
I have tested the JSON schema by validating it against various recipes manually. And all unit tests (including newly added tests) and integration tests passed successfully.

Any additional information or context required to review the change:
One important aspect to consider is the naming convention for recipe properties. While the recipe follows the CamelCase naming convention for properties in its documentation, hands-on experimentation has revealed that the recipe properties can be case-insensitive when specified. To accommodate this behavior, I have deliberately made all property names in the JSON schema lowercase. During the validation process, the property names will be converted to lowercase in the data object before applying the schema. This approach ensures that the validation process is case-insensitive and can handle variations in property casing.

Checklist:

  • Updated the README if applicable
  • Updated or added new unit tests
  • Updated or added new integration tests
  • Updated or added new end-to-end tests
  • If your code makes a remote network call, it was tested with a proxy
  • You confirm that the change is backwards compatible

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

@codecov
Copy link

codecov bot commented Aug 7, 2023

Codecov Report

❗ No coverage uploaded for pull request base (Yifan-recipe-validation@62c8382). Click here to learn what that means.
Patch has no changes to coverable lines.

Additional details and impacted files
@@                    Coverage Diff                     @@
##             Yifan-recipe-validation     #222   +/-   ##
==========================================================
  Coverage                           ?   97.55%           
==========================================================
  Files                              ?       42           
  Lines                              ?     1516           
  Branches                           ?        0           
==========================================================
  Hits                               ?     1479           
  Misses                             ?       37           
  Partials                           ?        0           
Flag Coverage Δ
integ 87.20% <0.00%> (?)
uat 75.79% <0.00%> (?)
unit 93.46% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

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

saranyailla
saranyailla previously approved these changes Aug 8, 2023
@nullPointer373 nullPointer373 merged commit b8cdb34 into Yifan-recipe-validation Aug 8, 2023
5 checks passed
@nullPointer373 nullPointer373 deleted the Yifan-recipe-validation-add-schema branch August 8, 2023 20:36
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