From 546590b163786ac18b145ca729f00bf6e85db4e1 Mon Sep 17 00:00:00 2001 From: ShouheiNishi <96609867+ShouheiNishi@users.noreply.github.com> Date: Wed, 1 Feb 2023 22:19:54 +0900 Subject: [PATCH] openapi3: fix resolving Callbacks (#757) Co-authored-by: Pierre Fenoll fix https://github.com/getkin/kin-openapi/issues/341 --- openapi3/internalize_refs.go | 90 +++++++++++++++++++------ openapi3/issue341_test.go | 39 ++++++++++- openapi3/issue753_test.go | 20 ++++++ openapi3/loader.go | 118 +++++++++++---------------------- openapi3/testdata/issue753.yml | 53 +++++++++++++++ 5 files changed, 217 insertions(+), 103 deletions(-) create mode 100644 openapi3/issue753_test.go create mode 100644 openapi3/testdata/issue753.yml diff --git a/openapi3/internalize_refs.go b/openapi3/internalize_refs.go index acb83cd0c..b8506535e 100644 --- a/openapi3/internalize_refs.go +++ b/openapi3/internalize_refs.go @@ -78,11 +78,16 @@ func (doc *T) addParameterToSpec(p *ParameterRef, refNameResolver RefNameResolve return false } name := refNameResolver(p.Ref) - if _, ok := doc.Components.Parameters[name]; ok { - p.Ref = "#/components/parameters/" + name - return true + if doc.Components != nil { + if _, ok := doc.Components.Parameters[name]; ok { + p.Ref = "#/components/parameters/" + name + return true + } } + if doc.Components == nil { + doc.Components = &Components{} + } if doc.Components.Parameters == nil { doc.Components.Parameters = make(ParametersMap) } @@ -96,9 +101,15 @@ func (doc *T) addHeaderToSpec(h *HeaderRef, refNameResolver RefNameResolver, par return false } name := refNameResolver(h.Ref) - if _, ok := doc.Components.Headers[name]; ok { - h.Ref = "#/components/headers/" + name - return true + if doc.Components != nil { + if _, ok := doc.Components.Headers[name]; ok { + h.Ref = "#/components/headers/" + name + return true + } + } + + if doc.Components == nil { + doc.Components = &Components{} } if doc.Components.Headers == nil { doc.Components.Headers = make(Headers) @@ -113,9 +124,15 @@ func (doc *T) addRequestBodyToSpec(r *RequestBodyRef, refNameResolver RefNameRes return false } name := refNameResolver(r.Ref) - if _, ok := doc.Components.RequestBodies[name]; ok { - r.Ref = "#/components/requestBodies/" + name - return true + if doc.Components != nil { + if _, ok := doc.Components.RequestBodies[name]; ok { + r.Ref = "#/components/requestBodies/" + name + return true + } + } + + if doc.Components == nil { + doc.Components = &Components{} } if doc.Components.RequestBodies == nil { doc.Components.RequestBodies = make(RequestBodies) @@ -130,9 +147,15 @@ func (doc *T) addResponseToSpec(r *ResponseRef, refNameResolver RefNameResolver, return false } name := refNameResolver(r.Ref) - if _, ok := doc.Components.Responses[name]; ok { - r.Ref = "#/components/responses/" + name - return true + if doc.Components != nil { + if _, ok := doc.Components.Responses[name]; ok { + r.Ref = "#/components/responses/" + name + return true + } + } + + if doc.Components == nil { + doc.Components = &Components{} } if doc.Components.Responses == nil { doc.Components.Responses = make(Responses) @@ -147,9 +170,15 @@ func (doc *T) addSecuritySchemeToSpec(ss *SecuritySchemeRef, refNameResolver Ref return } name := refNameResolver(ss.Ref) - if _, ok := doc.Components.SecuritySchemes[name]; ok { - ss.Ref = "#/components/securitySchemes/" + name - return + if doc.Components != nil { + if _, ok := doc.Components.SecuritySchemes[name]; ok { + ss.Ref = "#/components/securitySchemes/" + name + return + } + } + + if doc.Components == nil { + doc.Components = &Components{} } if doc.Components.SecuritySchemes == nil { doc.Components.SecuritySchemes = make(SecuritySchemes) @@ -164,9 +193,15 @@ func (doc *T) addExampleToSpec(e *ExampleRef, refNameResolver RefNameResolver, p return } name := refNameResolver(e.Ref) - if _, ok := doc.Components.Examples[name]; ok { - e.Ref = "#/components/examples/" + name - return + if doc.Components != nil { + if _, ok := doc.Components.Examples[name]; ok { + e.Ref = "#/components/examples/" + name + return + } + } + + if doc.Components == nil { + doc.Components = &Components{} } if doc.Components.Examples == nil { doc.Components.Examples = make(Examples) @@ -181,9 +216,15 @@ func (doc *T) addLinkToSpec(l *LinkRef, refNameResolver RefNameResolver, parentI return } name := refNameResolver(l.Ref) - if _, ok := doc.Components.Links[name]; ok { - l.Ref = "#/components/links/" + name - return + if doc.Components != nil { + if _, ok := doc.Components.Links[name]; ok { + l.Ref = "#/components/links/" + name + return + } + } + + if doc.Components == nil { + doc.Components = &Components{} } if doc.Components.Links == nil { doc.Components.Links = make(Links) @@ -198,6 +239,10 @@ func (doc *T) addCallbackToSpec(c *CallbackRef, refNameResolver RefNameResolver, return false } name := refNameResolver(c.Ref) + + if doc.Components == nil { + doc.Components = &Components{} + } if doc.Components.Callbacks == nil { doc.Components.Callbacks = make(Callbacks) } @@ -293,6 +338,9 @@ func (doc *T) derefRequestBody(r RequestBody, refNameResolver RefNameResolver, p func (doc *T) derefPaths(paths map[string]*PathItem, refNameResolver RefNameResolver, parentIsExternal bool) { for _, ops := range paths { + if isExternalRef(ops.Ref, parentIsExternal) { + parentIsExternal = true + } // inline full operations ops.Ref = "" diff --git a/openapi3/issue341_test.go b/openapi3/issue341_test.go index 15ea9d48c..ba9bed76b 100644 --- a/openapi3/issue341_test.go +++ b/openapi3/issue341_test.go @@ -1,6 +1,7 @@ package openapi3 import ( + "context" "testing" "github.com/stretchr/testify/require" @@ -20,7 +21,43 @@ func TestIssue341(t *testing.T) { bs, err := doc.MarshalJSON() require.NoError(t, err) - require.JSONEq(t, `{"info":{"title":"test file","version":"n/a"},"openapi":"3.0.0","paths":{"/testpath":{"get":{"responses":{"200":{"$ref":"#/components/responses/testpath_200_response"}}}}}}`, string(bs)) + require.JSONEq(t, `{"info":{"title":"test file","version":"n/a"},"openapi":"3.0.0","paths":{"/testpath":{"$ref":"testpath.yaml#/paths/~1testpath"}}}`, string(bs)) require.Equal(t, "string", doc.Paths["/testpath"].Get.Responses["200"].Value.Content["application/json"].Schema.Value.Type) + + doc.InternalizeRefs(context.Background(), nil) + bs, err = doc.MarshalJSON() + require.NoError(t, err) + require.JSONEq(t, `{ + "components": { + "responses": { + "testpath_200_response": { + "content": { + "application/json": { + "schema": { + "type": "string" + } + } + }, + "description": "a custom response" + } + } + }, + "info": { + "title": "test file", + "version": "n/a" + }, + "openapi": "3.0.0", + "paths": { + "/testpath": { + "get": { + "responses": { + "200": { + "$ref": "#/components/responses/testpath_200_response" + } + } + } + } + } + }`, string(bs)) } diff --git a/openapi3/issue753_test.go b/openapi3/issue753_test.go new file mode 100644 index 000000000..4390641a4 --- /dev/null +++ b/openapi3/issue753_test.go @@ -0,0 +1,20 @@ +package openapi3 + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestIssue753(t *testing.T) { + loader := NewLoader() + + doc, err := loader.LoadFromFile("testdata/issue753.yml") + require.NoError(t, err) + + err = doc.Validate(loader.Context) + require.NoError(t, err) + + require.NotNil(t, (*doc.Paths["/test1"].Post.Callbacks["callback1"].Value)["{$request.body#/callback}"].Post.RequestBody.Value.Content["application/json"].Schema.Value) + require.NotNil(t, (*doc.Paths["/test2"].Post.Callbacks["callback2"].Value)["{$request.body#/callback}"].Post.RequestBody.Value.Content["application/json"].Schema.Value) +} diff --git a/openapi3/loader.go b/openapi3/loader.go index 72ab8c46a..03d45ff7f 100644 --- a/openapi3/loader.go +++ b/openapi3/loader.go @@ -44,6 +44,7 @@ type Loader struct { visitedDocuments map[string]*T + visitedCallback map[*Callback]struct{} visitedExample map[*Example]struct{} visitedHeader map[*Header]struct{} visitedLink map[*Link]struct{} @@ -243,11 +244,11 @@ func (loader *Loader) ResolveRefsIn(doc *T, location *url.URL) (err error) { } // Visit all operations - for entrypoint, pathItem := range doc.Paths { + for _, pathItem := range doc.Paths { if pathItem == nil { continue } - if err = loader.resolvePathItemRef(doc, entrypoint, pathItem, location); err != nil { + if err = loader.resolvePathItemRef(doc, pathItem, location); err != nil { return } } @@ -850,6 +851,16 @@ func (loader *Loader) resolveExampleRef(doc *T, component *ExampleRef, documentP } func (loader *Loader) resolveCallbackRef(doc *T, component *CallbackRef, documentPath *url.URL) (err error) { + if component != nil && component.Value != nil { + if loader.visitedCallback == nil { + loader.visitedCallback = make(map[*Callback]struct{}) + } + if _, ok := loader.visitedCallback[component.Value]; ok { + return nil + } + loader.visitedCallback[component.Value] = struct{}{} + } + if component == nil { return errors.New("invalid callback: value MUST be an object") } @@ -878,57 +889,8 @@ func (loader *Loader) resolveCallbackRef(doc *T, component *CallbackRef, documen return nil } - for entrypoint, pathItem := range *value { - entrypoint, pathItem := entrypoint, pathItem - err = func() (err error) { - key := "-" - if documentPath != nil { - key = documentPath.EscapedPath() - } - key += entrypoint - if _, ok := loader.visitedPathItemRefs[key]; ok { - return nil - } - loader.visitedPathItemRefs[key] = struct{}{} - - if pathItem == nil { - return errors.New("invalid path item: value MUST be an object") - } - ref := pathItem.Ref - if ref != "" { - if isSingleRefElement(ref) { - var p PathItem - if documentPath, err = loader.loadSingleElementFromURI(ref, documentPath, &p); err != nil { - return err - } - *pathItem = p - } else { - if doc, ref, documentPath, err = loader.resolveRef(doc, ref, documentPath); err != nil { - return - } - - rest := strings.TrimPrefix(ref, "#/components/callbacks/") - if rest == ref { - return fmt.Errorf(`expected prefix "#/components/callbacks/" in URI %q`, ref) - } - id := unescapeRefString(rest) - - if doc.Components == nil || doc.Components.Callbacks == nil { - return failedToResolveRefFragmentPart(ref, "callbacks") - } - resolved := doc.Components.Callbacks[id] - if resolved == nil { - return failedToResolveRefFragmentPart(ref, id) - } - - for _, p := range *resolved.Value { - *pathItem = *p - break - } - } - } - return loader.resolvePathItemRefContinued(doc, pathItem, documentPath) - }() + for _, pathItem := range *value { + err := loader.resolvePathItemRef(doc, pathItem, documentPath) if err != nil { return err } @@ -973,22 +935,27 @@ func (loader *Loader) resolveLinkRef(doc *T, component *LinkRef, documentPath *u return nil } -func (loader *Loader) resolvePathItemRef(doc *T, entrypoint string, pathItem *PathItem, documentPath *url.URL) (err error) { - key := "_" - if documentPath != nil { - key = documentPath.EscapedPath() - } - key += entrypoint - if _, ok := loader.visitedPathItemRefs[key]; ok { - return nil - } - loader.visitedPathItemRefs[key] = struct{}{} - +func (loader *Loader) resolvePathItemRef(doc *T, pathItem *PathItem, documentPath *url.URL) (err error) { if pathItem == nil { return errors.New("invalid path item: value MUST be an object") } ref := pathItem.Ref if ref != "" { + if pathItem.Summary != "" || + pathItem.Description != "" || + pathItem.Connect != nil || + pathItem.Delete != nil || + pathItem.Get != nil || + pathItem.Head != nil || + pathItem.Options != nil || + pathItem.Patch != nil || + pathItem.Post != nil || + pathItem.Put != nil || + pathItem.Trace != nil || + len(pathItem.Servers) != 0 || + len(pathItem.Parameters) != 0 { + return nil + } if isSingleRefElement(ref) { var p PathItem if documentPath, err = loader.loadSingleElementFromURI(ref, documentPath, &p); err != nil { @@ -996,25 +963,14 @@ func (loader *Loader) resolvePathItemRef(doc *T, entrypoint string, pathItem *Pa } *pathItem = p } else { - if doc, ref, documentPath, err = loader.resolveRef(doc, ref, documentPath); err != nil { - return - } - - rest := strings.TrimPrefix(ref, "#/paths/") - if rest == ref { - return fmt.Errorf(`expected prefix "#/paths/" in URI %q`, ref) - } - id := unescapeRefString(rest) - - if doc.Paths == nil { - return failedToResolveRefFragmentPart(ref, "paths") - } - resolved := doc.Paths[id] - if resolved == nil { - return failedToResolveRefFragmentPart(ref, id) + var resolved PathItem + doc, documentPath, err = loader.resolveComponent(doc, ref, documentPath, &resolved) + if err != nil { + return err } - *pathItem = *resolved + *pathItem = resolved } + pathItem.Ref = ref } return loader.resolvePathItemRefContinued(doc, pathItem, documentPath) } diff --git a/openapi3/testdata/issue753.yml b/openapi3/testdata/issue753.yml new file mode 100644 index 000000000..2123a6dbd --- /dev/null +++ b/openapi3/testdata/issue753.yml @@ -0,0 +1,53 @@ +openapi: '3' +info: + version: 0.0.1 + title: 'test' +paths: + /test1: + post: + requestBody: + content: + application/json: + schema: + type: object + responses: + '200': + description: 'test' + callbacks: + callback1: + '{$request.body#/callback}': + post: + requestBody: + content: + application/json: + schema: + $ref: '#/components/schemas/test' + responses: + '200': + description: 'test' + /test2: + post: + requestBody: + content: + application/json: + schema: + type: object + responses: + '200': + description: 'test' + callbacks: + callback2: + '{$request.body#/callback}': + post: + requestBody: + content: + application/json: + schema: + $ref: '#/components/schemas/test' + responses: + '200': + description: 'test' +components: + schemas: + test: + type: string