[WIP] feat: Add support for 3.2.0 openapi spec #54
Conversation
BlakeTheAwesome
force-pushed
the
blake/spec-3.2
branch
from
3ee25f8 to
f5724d6
Compare
| Major int | ||
| Minor int | ||
| Patch int | ||
| } |
There was a problem hiding this comment.
| } | |
| } | |
| var _ fmt.Stringer = (*Version)(nil) |
this just adds a compile time check that you implement the Stringer interface correctly with your String() method
| return &PathItem{ | ||
| Map: *sequencedmap.New[string, *Operation](), | ||
| } | ||
| } |
There was a problem hiding this comment.
nit: add whitespace
| } | |
| } | |
| - name: "admin" | ||
| summary: "Admin" | ||
| description: "Administrative operations" | ||
| parent: "users" | ||
| kind: "nav" | ||
| - name: "beta-features" | ||
| summary: "Beta" | ||
| description: "Experimental features" | ||
| kind: "badge" |
There was a problem hiding this comment.
We probably don't want to add these, as a bootstrapped document is meant to be less example and more a starting point for a real document. Adding the summary and kind to the users tag is good but I would avoid adding more tags for now (especially if they aren't being used).
| assert.Equal(t, string(expectedBytes), string(actualYAML), "Bundled document should match expected output") | ||
| } | ||
|
|
||
| func TestBundle_AdditionalOperations_Success(t *testing.T) { |
There was a problem hiding this comment.
if possible I would try and add the additionalOperations test in to the original test above and update the expected doc etc, to make sure everything is working together.
Generally also with tests like this its better to be asserting the exact shape of the document like we do in the test above rather than all the assertions you have below to catch where we might be introducing unintended changes etc, but that is just another reason to integrate it into the test above (which will then also mean we have the inline tests also testing it)
| assert.Equal(t, string(expectedBytes), string(actualYAML), "Inlined document should match expected output") | ||
| } | ||
|
|
||
| func TestInline_AdditionalOperations_Success(t *testing.T) { |
There was a problem hiding this comment.
Same comments as the bundle test
|
|
||
| o := validation.NewOptions(opts...) | ||
|
|
||
| openapi := validation.GetContextObject[OpenAPI](o) |
There was a problem hiding this comment.
I was actually thinking we could maybe be a bit less strict about needing the OpenAPI object to be passed in here. Instead of panic we can just assume when we don't receive the OpenAPI we are working with a document of the latest version.
Was thinking through our future use cases for a linter and more robust validation in the future and we will prob want to double down on support people being able to call the Validate methods on each object manually even when they don't have a full document object yet.
|
|
||
| // ValidateWithTags validates the Tag object in the context of all tags to check for parent relationships. | ||
| // This should be called during document-level validation where all tags are available. | ||
| func (t *Tag) ValidateWithTags(ctx context.Context, allTags []*Tag, opts ...validation.Option) []error { |
There was a problem hiding this comment.
Instead of having a different Validate function we probably want to take the OpenAPI object like we did in the paths Validate function and get the list of tags from that and then validate the tag against that list. Thoughts? We haven't really yet introduce alternate Validate funcs because they then won't be called automatically when you call Validate on the document.
| // TagKind represents commonly used values for the Tag.Kind field. | ||
| // These values are registered in the OpenAPI Initiative's Tag Kind Registry | ||
| // at https://spec.openapis.org/registry/tag-kind/ | ||
| type TagKind string |
There was a problem hiding this comment.
| type TagKind string | |
| type TagKind string | |
| var _ fmt.Stringer = (*TagKind)(nil) |
| } | ||
| } | ||
|
|
||
| func WithUpgradeTargetVersion(version string) Option[UpgradeOptions] { |
There was a problem hiding this comment.
one thing we will need to be aware of and probably proactive about, is that having this be optional will be a breaking change to Gram in that when it upgrades docs it will no longer just upgrade to 3.1.1 but all the way to 3.2.0 which may not yet be supported by MCP/AI yet.
So we just need to make sure once this is released we go and update Gram with the new version (before that team does) and make sure the Upgrade call uses this and set the version to 3.1.1
WIP
TODO:
kinds in registry)itemSchemafield in mediaType and relevant validation and support in other APIs/methodsitemEncoding&prefixEncodingfields in mediaType and relevant validation and support in other APIs/methodsoauth2MetadataUrlanddeprecatedfields in security schema object and relevant validationdefaultMappingfield in discriminator and relevant validationdataValue,serializedValue, and new deprecation ofvalueand the relevant validationnamefield in server object$selffield in OpenAPI object and the relevant validation and update reference resolution to use this as the target location when present instead of parents locaiton