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

fix: ozzo validation #134

Merged
merged 8 commits into from
Feb 16, 2024
Merged

Conversation

injeniero
Copy link
Contributor

OpenAPI required, nullable, min, max, minLength and maxLength influence how OZZO validation should be done.

This changes are in the direction described in the OpenAPI 3.0.3 clarification on how nullable property should work.

The following table summarize this PR interpretation of the spec on how it should be done using ozzo validations.

# ozzo validation type required nullable min max minLength format pattern
1 Required array x >0
2 Required enum x
3 Required integer x >0
4 Required integer x <0
5 Required map x >0
6 Required number x >0
7 Required number x <0
8 Required string x >0
9 Required string x x
10 Required string x x
11 NotNil array x
12 NotNil map x
13 NilOrNotEmpty array >0
14 NilOrNotEmpty enum x x
15 NilOrNotEmpty enum x
16 NilOrNotEmpty integer x >0
17 NilOrNotEmpty integer x <0
18 NilOrNotEmpty map >0
19 NilOrNotEmpty number x >0
20 NilOrNotEmpty number x <0
21 NilOrNotEmpty string x >0
22 NilOrNotEmpty string x x x
23 NilOrNotEmpty string x x x

How Has This Been Verified?

I run it against our live service code and also checked each generated test case.

Checklist:

  • The change works as expected.
  • New code can be debugged via logs.
  • I have added tests to cover my changes.
  • I have locally run the tests and all new and existing tests passed.
  • Requires updates to the documentation.
  • I have made the required changes to the documents.

@injeniero injeniero requested a review from a team as a code owner February 2, 2024 22:30
@injeniero injeniero requested review from megaflo and removed request for a team February 2, 2024 22:30
OpenAPI required, nullable, min and minLength influence how OZZO validation should be done.
@injeniero injeniero changed the title Fix Validation fix: ozzo validation Feb 2, 2024
pkg/generators/models/templates/model.gotpl Outdated Show resolved Hide resolved
pkg/generators/models/models.go Outdated Show resolved Hide resolved
@injeniero
Copy link
Contributor Author

The linter is failing with an internal error

level=error msg="Running error: 1 error occurred:\n\t* can't run linter goanalysis_metalinter: inspect: failed to load package : could not load export data: no export data for "slices"\n\n"

@injeniero
Copy link
Contributor Author

The linter is failing with an internal error

level=error msg="Running error: 1 error occurred:\n\t* can't run linter goanalysis_metalinter: inspect: failed to load package : could not load export data: no export data for "slices"\n\n"

This is fixed now

@injeniero
Copy link
Contributor Author

Please let me know what else is needed to get it accepted.

thanks!

@LucasRoesler
Copy link
Member

@injeniero overall it looks good, but I need a bit more time to go through all of the changes and test it out locally, I am aiming that we have this merged no later than end of week

@LucasRoesler LucasRoesler merged commit d48b64b into contiamo:master Feb 16, 2024
2 checks passed
@contiamo-ci contiamo-ci mentioned this pull request Feb 16, 2024
LucasRoesler pushed a commit that referenced this pull request Feb 16, 2024
🤖 I have created a release *beep* *boop*
---


##
[2.1.1](v2.1.0...v2.1.1)
(2024-02-16)


### Bug Fixes

* ozzo validation
([#134](#134))
([d48b64b](d48b64b))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
@injeniero injeniero deleted the fix/required-validation branch February 18, 2024 23:03
@injeniero
Copy link
Contributor Author

Thanks!!!

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