OpenAPIv3: count different validation rules as different types #3640
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This replicates an issue we're seeing in the OpenAPIv3 generator, where if two request bodies are structurally similar (i.e. same primitive types), but not identical (in this example both have a single string attribute with the same name, but with different enum validations), the request body for those two methods points at the same type schema. There's a (previously-failing) test, where: - two types which are 'similar' (both have a string-type attr called `my_attr`), but not identical (they are different enums) - two different services with the same method, each referencing different types The resulting OpenAPIv3 schema generates a single `MethodEnumRequestBody` type, and uses it for the `requestBody` type of both methods. This is incorrect, since the two methods use a different enum. The fix here is to update the `hashAttribute` method to hash the `.Validation` of the attribute, if it is set 🎉
raphael
reviewed
Jan 30, 2025
| var _ = Service("svc_enum_1", func() { | ||
| Method("method_enum", func() { | ||
| Payload(func() { | ||
| Reference(T1) |
There was a problem hiding this comment.
Is the Reference required to repro the bug? would it also happen by simply doing Payload(T1)?
| if val.Pattern != "" { | ||
| parts = append(parts, hashString(val.Pattern, h)) | ||
| } | ||
| if val.MinLength != nil { |
There was a problem hiding this comment.
This algo may require a bit of tweaking, as it stands the following two validations would result in the same hash:
Pattern("1")and
MinLength(1)
Obviously there are many other combinations that would result in the same hash being generated. Maybe we need to use something like https://github.com/gohugoio/hashstructure,
Might also be good to add the example above (and maybe others) to tests.
|
Superseded by #3642 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This replicates an issue we're seeing in the OpenAPIv3 generator, where if two request bodies are structurally similar (i.e. same primitive types), but not identical (in this example both have a single string attribute with the same name, but with different enum validations), the request body for those two methods points at the same type schema.
There's a (previously-failing) test, where:
my_attr), but not identical (they are different enums)The resulting OpenAPIv3 schema generates a single
MethodEnumRequestBodytype, and uses it for therequestBodytype of both methods.This is incorrect, since the two methods use a different enum!
The fix here is to update the
hashAttributemethod to hash the.Validationof the attribute, if it is set 🎉