From 0ed9f5d122299b343138b169ee343d81b09b5a9a Mon Sep 17 00:00:00 2001 From: percivalalb Date: Wed, 3 Jul 2024 13:54:12 +0100 Subject: [PATCH] openapi3: improve internalization ref naming to avoid collisions (#955) * Add failing test case * Improve default name internalisation to avoid collisions * Set ref path when resolving refs too * Update unit tests Still got some work to do, the recursive test still fails * Make InternalizeRefs deterministic This makes resolving references & internalising references determinstic by sorting map for loops by key. Ensures refs are resolved in the same order, depending on the spec this can result in a different (but equal value) internalised spec. * Ensure root document url is set The unmarshal function was removing the .url value * Ensure internalised names are valid * Update internalized golden files * Maintain first path assigned to each reference This will be the path at the closest point to the actual definition in the reference chain. Also trim . from the start of paths * Tidy up & relocation some functions * Use use OS repsecting file seperator * Check for duplicate references to tidy up internalized spec * Swap condition checks & add comment * Maintain consistent slash, only adjusting for OS specific when needed * Adjust documentation * Internalised -> internalized Excuse my British English --- .github/docs/openapi3.txt | 42 +++- README.md | 2 + openapi3/helpers.go | 35 ++- openapi3/internalize_refs.go | 212 +++++++++++++----- openapi3/internalize_refs_test.go | 1 + openapi3/issue341_test.go | 4 +- openapi3/issue618_test.go | 6 +- openapi3/loader.go | 189 ++++++++++------ openapi3/loader_circular_test.go | 2 +- openapi3/loader_read_from_uri_func_test.go | 2 +- openapi3/loader_uri_reader.go | 2 +- openapi3/refs.go | 99 ++++++++ openapi3/refs.tmpl | 11 + .../interalizationNameCollision/api.yml | 25 +++ .../api.yml.internalized.yml | 75 +++++++ .../schemas/book/record.yml | 8 + .../schemas/cd/record.yml | 8 + .../issue959/openapi.yml.internalized.yml | 6 +- .../recursiveRef/openapi.yml.internalized.yml | 44 +++- openapi3/testdata/spec.yaml.internalized.yml | 8 +- .../testref.openapi.yml.internalized.yml | 2 +- 21 files changed, 614 insertions(+), 169 deletions(-) create mode 100644 openapi3/testdata/interalizationNameCollision/api.yml create mode 100644 openapi3/testdata/interalizationNameCollision/api.yml.internalized.yml create mode 100644 openapi3/testdata/interalizationNameCollision/schemas/book/record.yml create mode 100644 openapi3/testdata/interalizationNameCollision/schemas/cd/record.yml diff --git a/.github/docs/openapi3.txt b/.github/docs/openapi3.txt index 0ada195c8..37791ed53 100644 --- a/.github/docs/openapi3.txt +++ b/.github/docs/openapi3.txt @@ -43,6 +43,16 @@ const ( VARIABLES +var ( + IdentifierRegExp = regexp.MustCompile(`^[` + identifierChars + `]+$`) + InvalidIdentifierCharRegExp = regexp.MustCompile(`[^` + identifierChars + `]`) +) + IdentifierRegExp verifies whether Component object key matches contains just + 'identifierChars', according to OpenAPI v3.x. InvalidIdentifierCharRegExp + matches all characters not contained in 'identifierChars'. However, to be + able supporting legacy OpenAPI v2.x, there is a need to customize above + pattern in order not to fail converted v2-v3 validation + var ( // SchemaErrorDetailsDisabled disables printing of details about schema errors. SchemaErrorDetailsDisabled = false @@ -63,12 +73,6 @@ var ErrURINotSupported = errors.New("unsupported URI") ErrURINotSupported indicates the ReadFromURIFunc does not know how to handle a given URI. -var IdentifierRegExp = regexp.MustCompile(identifierPattern) - IdentifierRegExp verifies whether Component object key matches - 'identifierPattern' pattern, according to OpenAPI v3.x. However, to be able - supporting legacy OpenAPI v2.x, there is a need to customize above pattern - in order not to fail converted v2-v3 validation - var SchemaStringFormats = make(map[string]Format, 4) SchemaStringFormats allows for validating string formats @@ -78,13 +82,22 @@ FUNCTIONS func BoolPtr(value bool) *bool BoolPtr is a helper for defining OpenAPI schemas. -func DefaultRefNameResolver(ref string) string +func DefaultRefNameResolver(doc *T, ref componentRef) string DefaultRefResolver is a default implementation of refNameResolver for the InternalizeRefs function. - If a reference points to an element inside a document, it returns the last - element in the reference using filepath.Base. Otherwise if the reference - points to a file, it returns the file name trimmed of all extensions. + The external reference is internalized to (hopefully) a unique name. + If the external reference matches (by path) to another reference in the root + document then the name of that component is used. + + The transformation involves: + - Cutting the "#/components/" part. + - Cutting the file extensions (.yaml/.json) from documents. + - Trimming the common directory with the root spec. + - Replace invalid characters with with underscores. + + This is an injective mapping over a "reasonable" amount of the possible + openapi spec domain space but is not perfect. There might be edge cases. func DefineIPv4Format() DefineIPv4Format opts in ipv4 format validation on top of OAS 3 spec @@ -1171,7 +1184,12 @@ type Ref struct { Ref is specified by OpenAPI/Swagger 3.0 standard. See https://github.com/OAI/OpenAPI-Specification/blob/main/versions/3.0.3.md#reference-object -type RefNameResolver func(string) string +type RefNameResolver func(*T, componentRef) string + RefNameResolver maps a component to an name that is used as it's + internalized name. + + The function should avoid name collisions (i.e. be a injective mapping). It + must only contain characters valid for fixed field names: IdentifierRegExp. type RequestBodies map[string]*RequestBodyRef @@ -1922,7 +1940,7 @@ func (doc *T) AddServer(server *Server) func (doc *T) AddServers(servers ...*Server) -func (doc *T) InternalizeRefs(ctx context.Context, refNameResolver func(ref string) string) +func (doc *T) InternalizeRefs(ctx context.Context, refNameResolver func(*T, componentRef) string) InternalizeRefs removes all references to external files from the spec and moves them to the components section. diff --git a/README.md b/README.md index d355d0ddd..54e58b7ca 100644 --- a/README.md +++ b/README.md @@ -305,6 +305,8 @@ for _, path := range doc.Paths.InMatchingOrder() { ### v0.126.0 * `openapi3.CircularReferenceError` and `openapi3.CircularReferenceCounter` are removed. `openapi3.Loader` now implements reference backtracking, so any kind of circular references should be properly resolved. +* `InternalizeRefs` now takes a refNameResolver that has access to `openapi3.T` and more properties of the reference needing resolving. +* The `DefaultRefNameResolver` has been updated, choosing names that will be less likely to collide with each other. Because of this internalized specs will likely change slightly. ### v0.125.0 * The `openapi3filter.ErrFunc` and `openapi3filter.LogFunc` func types now take the validated request's context as first argument. diff --git a/openapi3/helpers.go b/openapi3/helpers.go index 25da8a7ca..cb1ed3a9f 100644 --- a/openapi3/helpers.go +++ b/openapi3/helpers.go @@ -6,24 +6,29 @@ import ( "path" "reflect" "regexp" + "sort" "strings" "github.com/go-openapi/jsonpointer" ) -const identifierPattern = `^[a-zA-Z0-9._-]+$` +const identifierChars = `a-zA-Z0-9._-` -// IdentifierRegExp verifies whether Component object key matches 'identifierPattern' pattern, according to OpenAPI v3.x. +// IdentifierRegExp verifies whether Component object key matches contains just 'identifierChars', according to OpenAPI v3.x. +// InvalidIdentifierCharRegExp matches all characters not contained in 'identifierChars'. // However, to be able supporting legacy OpenAPI v2.x, there is a need to customize above pattern in order not to fail // converted v2-v3 validation -var IdentifierRegExp = regexp.MustCompile(identifierPattern) +var ( + IdentifierRegExp = regexp.MustCompile(`^[` + identifierChars + `]+$`) + InvalidIdentifierCharRegExp = regexp.MustCompile(`[^` + identifierChars + `]`) +) -// ValidateIdentifier returns an error if the given component name does not match IdentifierRegExp. +// ValidateIdentifier returns an error if the given component name does not match [IdentifierRegExp]. func ValidateIdentifier(value string) error { if IdentifierRegExp.MatchString(value) { return nil } - return fmt.Errorf("identifier %q is not supported by OpenAPIv3 standard (regexp: %q)", value, identifierPattern) + return fmt.Errorf("identifier %q is not supported by OpenAPIv3 standard (charset: [%q])", value, identifierChars) } // Float64Ptr is a helper for defining OpenAPI schemas. @@ -46,6 +51,26 @@ func Uint64Ptr(value uint64) *uint64 { return &value } +// componentNames returns the map keys in a sorted slice. +func componentNames[E any](s map[string]E) []string { + out := make([]string, 0, len(s)) + for i := range s { + out = append(out, i) + } + sort.Strings(out) + return out +} + +// copyURI makes a copy of the pointer. +func copyURI(u *url.URL) *url.URL { + if u == nil { + return nil + } + + c := *u // shallow-copy + return &c +} + type componentRef interface { RefString() string RefPath() *url.URL diff --git a/openapi3/internalize_refs.go b/openapi3/internalize_refs.go index 191c14f7e..b4742864c 100644 --- a/openapi3/internalize_refs.go +++ b/openapi3/internalize_refs.go @@ -2,47 +2,134 @@ package openapi3 import ( "context" - "path/filepath" + "path" "strings" ) -type RefNameResolver func(string) string +// RefNameResolver maps a component to an name that is used as it's internalized name. +// +// The function should avoid name collisions (i.e. be a injective mapping). +// It must only contain characters valid for fixed field names: [IdentifierRegExp]. +type RefNameResolver func(*T, componentRef) string // DefaultRefResolver is a default implementation of refNameResolver for the // InternalizeRefs function. // -// If a reference points to an element inside a document, it returns the last -// element in the reference using filepath.Base. Otherwise if the reference points -// to a file, it returns the file name trimmed of all extensions. -func DefaultRefNameResolver(ref string) string { - if ref == "" { - return "" - } - split := strings.SplitN(ref, "#", 2) - if len(split) == 2 { - return filepath.Base(split[1]) - } - ref = split[0] - for ext := filepath.Ext(ref); len(ext) > 0; ext = filepath.Ext(ref) { - ref = strings.TrimSuffix(ref, ext) - } - return filepath.Base(ref) -} +// The external reference is internalized to (hopefully) a unique name. If +// the external reference matches (by path) to another reference in the root +// document then the name of that component is used. +// +// The transformation involves: +// - Cutting the "#/components/" part. +// - Cutting the file extensions (.yaml/.json) from documents. +// - Trimming the common directory with the root spec. +// - Replace invalid characters with with underscores. +// +// This is an injective mapping over a "reasonable" amount of the possible openapi +// spec domain space but is not perfect. There might be edge cases. +func DefaultRefNameResolver(doc *T, ref componentRef) string { + if ref.RefString() == "" || ref.RefPath() == nil { + panic("unable to resolve reference to name") + } + + name := ref.RefPath() + + // If refering to a component in the root spec, no need to internalize just use + // the existing component. + // XXX(percivalalb): since this function call is iterating over components behind the + // scenes during an internalization call it actually starts interating over + // new & replaced internalized components. This might caused some edge cases, + // haven't found one yet but this might need to actually be used on a frozen copy + // of doc. + if nameInRoot, found := ReferencesComponentInRootDocument(doc, ref); found { + nameInRoot = strings.TrimPrefix(nameInRoot, "#") + + rootCompURI := copyURI(doc.url) + rootCompURI.Fragment = nameInRoot + name = rootCompURI + } + + filePath, componentPath := name.Path, name.Fragment -func schemaNames(s Schemas) []string { - out := make([]string, 0, len(s)) - for i := range s { - out = append(out, i) + // Cut out the "#/components/" to make the names shorter. + // XXX(percivalalb): This might cause collisions but is worth the brevity. + if b, a, ok := strings.Cut(componentPath, path.Join("components", ref.CollectionName(), "")); ok { + componentPath = path.Join(b, a) } - return out + + if filePath != "" { + // If the path is the same as the root doc, just remove. + if doc.url != nil && filePath == doc.url.Path { + filePath = "" + } + + // Remove the path extentions to make this JSON/YAML agnostic. + for ext := path.Ext(filePath); len(ext) > 0; ext = path.Ext(filePath) { + filePath = strings.TrimSuffix(filePath, ext) + } + + // Trim the common prefix with the root doc path. + if doc.url != nil { + commonDir := path.Dir(doc.url.Path) + for { + if commonDir == "." { // no common prefix + break + } + + if p, found := cutDirectories(filePath, commonDir); found { + filePath = p + break + } + + commonDir = path.Dir(commonDir) + } + } + } + + var internalizedName string + + // Trim .'s & slashes from start e.g. otherwise ./doc.yaml would end up as __doc + if filePath != "" { + internalizedName = strings.TrimLeft(filePath, "./") + } + + if componentPath != "" { + if internalizedName != "" { + internalizedName += "_" + } + + internalizedName += strings.TrimLeft(componentPath, "./") + } + + // Replace invalid characters in component fixed field names. + internalizedName = InvalidIdentifierCharRegExp.ReplaceAllString(internalizedName, "_") + + return internalizedName } -func parametersMapNames(s ParametersMap) []string { - out := make([]string, 0, len(s)) - for i := range s { - out = append(out, i) +// cutDirectories removes the given directories from the start of the path if +// the path is a child. +func cutDirectories(p, dirs string) (string, bool) { + if dirs == "" || p == "" { + return p, false } - return out + + p = strings.TrimRight(p, "/") + dirs = strings.TrimRight(dirs, "/") + + var sb strings.Builder + sb.Grow(len(ParameterInHeader)) + for _, segments := range strings.Split(p, "/") { + sb.WriteString(segments) + + if sb.String() == p { + return strings.TrimPrefix(p, dirs), true + } + + sb.WriteRune('/') + } + + return p, false } func isExternalRef(ref string, parentIsExternal bool) bool { @@ -54,7 +141,7 @@ func (doc *T) addSchemaToSpec(s *SchemaRef, refNameResolver RefNameResolver, par return false } - name := refNameResolver(s.Ref) + name := refNameResolver(doc, s) if doc.Components != nil { if _, ok := doc.Components.Schemas[name]; ok { s.Ref = "#/components/schemas/" + name @@ -77,7 +164,7 @@ func (doc *T) addParameterToSpec(p *ParameterRef, refNameResolver RefNameResolve if p == nil || !isExternalRef(p.Ref, parentIsExternal) { return false } - name := refNameResolver(p.Ref) + name := refNameResolver(doc, p) if doc.Components != nil { if _, ok := doc.Components.Parameters[name]; ok { p.Ref = "#/components/parameters/" + name @@ -100,7 +187,7 @@ func (doc *T) addHeaderToSpec(h *HeaderRef, refNameResolver RefNameResolver, par if h == nil || !isExternalRef(h.Ref, parentIsExternal) { return false } - name := refNameResolver(h.Ref) + name := refNameResolver(doc, h) if doc.Components != nil { if _, ok := doc.Components.Headers[name]; ok { h.Ref = "#/components/headers/" + name @@ -123,7 +210,7 @@ func (doc *T) addRequestBodyToSpec(r *RequestBodyRef, refNameResolver RefNameRes if r == nil || !isExternalRef(r.Ref, parentIsExternal) { return false } - name := refNameResolver(r.Ref) + name := refNameResolver(doc, r) if doc.Components != nil { if _, ok := doc.Components.RequestBodies[name]; ok { r.Ref = "#/components/requestBodies/" + name @@ -146,7 +233,7 @@ func (doc *T) addResponseToSpec(r *ResponseRef, refNameResolver RefNameResolver, if r == nil || !isExternalRef(r.Ref, parentIsExternal) { return false } - name := refNameResolver(r.Ref) + name := refNameResolver(doc, r) if doc.Components != nil { if _, ok := doc.Components.Responses[name]; ok { r.Ref = "#/components/responses/" + name @@ -169,7 +256,7 @@ func (doc *T) addSecuritySchemeToSpec(ss *SecuritySchemeRef, refNameResolver Ref if ss == nil || !isExternalRef(ss.Ref, parentIsExternal) { return } - name := refNameResolver(ss.Ref) + name := refNameResolver(doc, ss) if doc.Components != nil { if _, ok := doc.Components.SecuritySchemes[name]; ok { ss.Ref = "#/components/securitySchemes/" + name @@ -192,7 +279,7 @@ func (doc *T) addExampleToSpec(e *ExampleRef, refNameResolver RefNameResolver, p if e == nil || !isExternalRef(e.Ref, parentIsExternal) { return } - name := refNameResolver(e.Ref) + name := refNameResolver(doc, e) if doc.Components != nil { if _, ok := doc.Components.Examples[name]; ok { e.Ref = "#/components/examples/" + name @@ -215,7 +302,7 @@ func (doc *T) addLinkToSpec(l *LinkRef, refNameResolver RefNameResolver, parentI if l == nil || !isExternalRef(l.Ref, parentIsExternal) { return } - name := refNameResolver(l.Ref) + name := refNameResolver(doc, l) if doc.Components != nil { if _, ok := doc.Components.Links[name]; ok { l.Ref = "#/components/links/" + name @@ -238,7 +325,7 @@ func (doc *T) addCallbackToSpec(c *CallbackRef, refNameResolver RefNameResolver, if c == nil || !isExternalRef(c.Ref, parentIsExternal) { return false } - name := refNameResolver(c.Ref) + name := refNameResolver(doc, c) if doc.Components == nil { doc.Components = &Components{} @@ -264,7 +351,9 @@ func (doc *T) derefSchema(s *Schema, refNameResolver RefNameResolver, parentIsEx } } } - for _, s2 := range s.Properties { + + for _, name := range componentNames(s.Properties) { + s2 := s.Properties[name] isExternal := doc.addSchemaToSpec(s2, refNameResolver, parentIsExternal) if s2 != nil { doc.derefSchema(s2.Value, refNameResolver, isExternal || parentIsExternal) @@ -279,7 +368,8 @@ func (doc *T) derefSchema(s *Schema, refNameResolver RefNameResolver, parentIsEx } func (doc *T) derefHeaders(hs Headers, refNameResolver RefNameResolver, parentIsExternal bool) { - for _, h := range hs { + for _, name := range componentNames(hs) { + h := hs[name] isExternal := doc.addHeaderToSpec(h, refNameResolver, parentIsExternal) if doc.isVisitedHeader(h.Value) { continue @@ -289,26 +379,30 @@ func (doc *T) derefHeaders(hs Headers, refNameResolver RefNameResolver, parentIs } func (doc *T) derefExamples(es Examples, refNameResolver RefNameResolver, parentIsExternal bool) { - for _, e := range es { + for _, name := range componentNames(es) { + e := es[name] doc.addExampleToSpec(e, refNameResolver, parentIsExternal) } } func (doc *T) derefContent(c Content, refNameResolver RefNameResolver, parentIsExternal bool) { - for _, mediatype := range c { + for _, name := range componentNames(c) { + mediatype := c[name] isExternal := doc.addSchemaToSpec(mediatype.Schema, refNameResolver, parentIsExternal) if mediatype.Schema != nil { doc.derefSchema(mediatype.Schema.Value, refNameResolver, isExternal || parentIsExternal) } doc.derefExamples(mediatype.Examples, refNameResolver, parentIsExternal) - for _, e := range mediatype.Encoding { + for _, name := range componentNames(mediatype.Encoding) { + e := mediatype.Encoding[name] doc.derefHeaders(e.Headers, refNameResolver, parentIsExternal) } } } func (doc *T) derefLinks(ls Links, refNameResolver RefNameResolver, parentIsExternal bool) { - for _, l := range ls { + for _, name := range componentNames(ls) { + l := ls[name] doc.addLinkToSpec(l, refNameResolver, parentIsExternal) } } @@ -327,7 +421,8 @@ func (doc *T) derefResponses(rs *Responses, refNameResolver RefNameResolver, par } func (doc *T) derefResponseBodies(es ResponseBodies, refNameResolver RefNameResolver, parentIsExternal bool) { - for _, e := range es { + for _, name := range componentNames(es) { + e := es[name] doc.derefResponse(e, refNameResolver, parentIsExternal) } } @@ -345,7 +440,8 @@ 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 { + for _, name := range componentNames(paths) { + ops := paths[name] pathIsExternal := isExternalRef(ops.Ref, parentIsExternal) // inline full operations ops.Ref = "" @@ -357,12 +453,15 @@ func (doc *T) derefPaths(paths map[string]*PathItem, refNameResolver RefNameReso } } - for _, op := range ops.Operations() { + opsWithMethod := ops.Operations() + for _, name := range componentNames(opsWithMethod) { + op := opsWithMethod[name] isExternal := doc.addRequestBodyToSpec(op.RequestBody, refNameResolver, pathIsExternal) if op.RequestBody != nil && op.RequestBody.Value != nil { doc.derefRequestBody(*op.RequestBody.Value, refNameResolver, pathIsExternal || isExternal) } - for _, cb := range op.Callbacks { + for _, name := range componentNames(op.Callbacks) { + cb := op.Callbacks[name] isExternal := doc.addCallbackToSpec(cb, refNameResolver, pathIsExternal) if cb.Value != nil { cbValue := (*cb.Value).Map() @@ -391,7 +490,7 @@ func (doc *T) derefPaths(paths map[string]*PathItem, refNameResolver RefNameReso // Example: // // doc.InternalizeRefs(context.Background(), nil) -func (doc *T) InternalizeRefs(ctx context.Context, refNameResolver func(ref string) string) { +func (doc *T) InternalizeRefs(ctx context.Context, refNameResolver func(*T, componentRef) string) { doc.resetVisited() if refNameResolver == nil { @@ -399,8 +498,7 @@ func (doc *T) InternalizeRefs(ctx context.Context, refNameResolver func(ref stri } if components := doc.Components; components != nil { - names := schemaNames(components.Schemas) - for _, name := range names { + for _, name := range componentNames(components.Schemas) { schema := components.Schemas[name] isExternal := doc.addSchemaToSpec(schema, refNameResolver, false) if schema != nil { @@ -408,8 +506,7 @@ func (doc *T) InternalizeRefs(ctx context.Context, refNameResolver func(ref stri doc.derefSchema(schema.Value, refNameResolver, isExternal) } } - names = parametersMapNames(components.Parameters) - for _, name := range names { + for _, name := range componentNames(components.Parameters) { p := components.Parameters[name] isExternal := doc.addParameterToSpec(p, refNameResolver, false) if p != nil && p.Value != nil { @@ -418,7 +515,8 @@ func (doc *T) InternalizeRefs(ctx context.Context, refNameResolver func(ref stri } } doc.derefHeaders(components.Headers, refNameResolver, false) - for _, req := range components.RequestBodies { + for _, name := range componentNames(components.RequestBodies) { + req := components.RequestBodies[name] isExternal := doc.addRequestBodyToSpec(req, refNameResolver, false) if req != nil && req.Value != nil { req.Ref = "" // always dereference the top level @@ -426,13 +524,15 @@ func (doc *T) InternalizeRefs(ctx context.Context, refNameResolver func(ref stri } } doc.derefResponseBodies(components.Responses, refNameResolver, false) - for _, ss := range components.SecuritySchemes { + for _, name := range componentNames(components.SecuritySchemes) { + ss := components.SecuritySchemes[name] doc.addSecuritySchemeToSpec(ss, refNameResolver, false) } doc.derefExamples(components.Examples, refNameResolver, false) doc.derefLinks(components.Links, refNameResolver, false) - for _, cb := range components.Callbacks { + for _, name := range componentNames(components.Callbacks) { + cb := components.Callbacks[name] isExternal := doc.addCallbackToSpec(cb, refNameResolver, false) if cb != nil && cb.Value != nil { cb.Ref = "" // always dereference the top level diff --git a/openapi3/internalize_refs_test.go b/openapi3/internalize_refs_test.go index 90e73f234..6e9853acc 100644 --- a/openapi3/internalize_refs_test.go +++ b/openapi3/internalize_refs_test.go @@ -24,6 +24,7 @@ func TestInternalizeRefs(t *testing.T) { {"testdata/callbacks.yml"}, {"testdata/issue831/testref.internalizepath.openapi.yml"}, {"testdata/issue959/openapi.yml"}, + {"testdata/interalizationNameCollision/api.yml"}, } for _, test := range tests { diff --git a/openapi3/issue341_test.go b/openapi3/issue341_test.go index 6154591e9..55f18e54d 100644 --- a/openapi3/issue341_test.go +++ b/openapi3/issue341_test.go @@ -48,7 +48,7 @@ func TestIssue341(t *testing.T) { require.JSONEq(t, `{ "components": { "responses": { - "testpath_200_response": { + "testpath_testpath_200_response": { "content": { "application/json": { "schema": { @@ -70,7 +70,7 @@ func TestIssue341(t *testing.T) { "get": { "responses": { "200": { - "$ref": "#/components/responses/testpath_200_response" + "$ref": "#/components/responses/testpath_testpath_200_response" } } } diff --git a/openapi3/issue618_test.go b/openapi3/issue618_test.go index 2085ca0ee..cd6758895 100644 --- a/openapi3/issue618_test.go +++ b/openapi3/issue618_test.go @@ -33,7 +33,7 @@ paths: doc.InternalizeRefs(ctx, nil) - require.Contains(t, doc.Components.Schemas, "JournalEntry") - require.Contains(t, doc.Components.Schemas, "Record") - require.Contains(t, doc.Components.Schemas, "Account") + require.Contains(t, doc.Components.Schemas, "testdata_schema618_JournalEntry") + require.Contains(t, doc.Components.Schemas, "testdata_schema618_Record") + require.Contains(t, doc.Components.Schemas, "testdata_schema618_Account") } diff --git a/openapi3/loader.go b/openapi3/loader.go index 003ba6c77..567d7394e 100644 --- a/openapi3/loader.go +++ b/openapi3/loader.go @@ -11,7 +11,6 @@ import ( "path" "path/filepath" "reflect" - "sort" "strconv" "strings" ) @@ -167,16 +166,14 @@ func (loader *Loader) loadFromDataWithPathInternal(data []byte, location *url.UR } doc := &T{} - if location != nil { - specURL := *location - doc.url = &specURL // shallow-copy - } - loader.visitedDocuments[uri] = doc if err := unmarshal(data, doc); err != nil { return nil, err } + + doc.url = copyURI(location) + if err := loader.ResolveRefsIn(doc, location); err != nil { return nil, err } @@ -195,50 +192,50 @@ func (loader *Loader) ResolveRefsIn(doc *T, location *url.URL) (err error) { } if components := doc.Components; components != nil { - for _, component := range components.Headers { + for _, name := range componentNames(components.Headers) { + component := components.Headers[name] if err = loader.resolveHeaderRef(doc, component, location); err != nil { return } } - for _, component := range components.Parameters { + for _, name := range componentNames(components.Parameters) { + component := components.Parameters[name] if err = loader.resolveParameterRef(doc, component, location); err != nil { return } } - for _, component := range components.RequestBodies { + for _, name := range componentNames(components.RequestBodies) { + component := components.RequestBodies[name] if err = loader.resolveRequestBodyRef(doc, component, location); err != nil { return } } - for _, component := range components.Responses { + for _, name := range componentNames(components.Responses) { + component := components.Responses[name] if err = loader.resolveResponseRef(doc, component, location); err != nil { return } } - for _, component := range components.Schemas { + for _, name := range componentNames(components.Schemas) { + component := components.Schemas[name] if err = loader.resolveSchemaRef(doc, component, location, []string{}); err != nil { return } } - for _, component := range components.SecuritySchemes { + for _, name := range componentNames(components.SecuritySchemes) { + component := components.SecuritySchemes[name] if err = loader.resolveSecuritySchemeRef(doc, component, location); err != nil { return } } - - examples := make([]string, 0, len(components.Examples)) - for name := range components.Examples { - examples = append(examples, name) - } - sort.Strings(examples) - for _, name := range examples { + for _, name := range componentNames(components.Examples) { component := components.Examples[name] if err = loader.resolveExampleRef(doc, component, location); err != nil { return } } - - for _, component := range components.Callbacks { + for _, name := range componentNames(components.Callbacks) { + component := components.Callbacks[name] if err = loader.resolveCallbackRef(doc, component, location); err != nil { return } @@ -246,7 +243,9 @@ func (loader *Loader) ResolveRefsIn(doc *T, location *url.URL) (err error) { } // Visit all operations - for _, pathItem := range doc.Paths.Map() { + pathItems := doc.Paths.Map() + for _, name := range componentNames(pathItems) { + pathItem := pathItems[name] if pathItem == nil { continue } @@ -270,7 +269,7 @@ func join(basePath *url.URL, relativePath *url.URL) *url.URL { func resolvePath(basePath *url.URL, componentPath *url.URL) *url.URL { if is_file(componentPath) { // support absolute paths - if componentPath.Path[0] == '/' { + if filepath.IsAbs(componentPath.Path) { return componentPath } return join(basePath, componentPath) @@ -407,8 +406,22 @@ func (loader *Loader) resolveComponent(doc *T, ref string, path *url.URL, resolv err = nil } + setComponent := func(target any) { + if componentPath != nil { + if i, ok := target.(interface { + setRefPath(*url.URL) + }); ok { + copy := *componentPath + copy.Fragment = parsedURL.Fragment + i.setRefPath(©) + } + } + } + switch { case reflect.TypeOf(cursor) == reflect.TypeOf(resolved): + setComponent(cursor) + reflect.ValueOf(resolved).Elem().Set(reflect.ValueOf(cursor).Elem()) return componentDoc, componentPath, nil @@ -421,6 +434,8 @@ func (loader *Loader) resolveComponent(doc *T, ref string, path *url.URL, resolv if err = json.Unmarshal(enc, expect); err != nil { return err } + + setComponent(expect) return nil } if err := codec(cursor, resolved); err != nil { @@ -521,16 +536,10 @@ func (loader *Loader) resolveRef(doc *T, ref string, path *url.URL) (*T, string, return doc, ref, path, nil } - if err := loader.allowsExternalRefs(ref); err != nil { - return nil, "", nil, err - } - - resolvedPath, err := resolvePathWithRef(ref, path) + fragment, resolvedPath, err := loader.resolveRefPath(ref, path) if err != nil { return nil, "", nil, err } - fragment := "#" + resolvedPath.Fragment - resolvedPath.Fragment = "" if doc, err = loader.loadFromURIInternal(resolvedPath); err != nil { return nil, "", nil, fmt.Errorf("error resolving reference %q: %w", ref, err) @@ -539,6 +548,25 @@ func (loader *Loader) resolveRef(doc *T, ref string, path *url.URL) (*T, string, return doc, fragment, resolvedPath, nil } +func (loader *Loader) resolveRefPath(ref string, path *url.URL) (string, *url.URL, error) { + if ref != "" && ref[0] == '#' { + return ref, path, nil + } + + if err := loader.allowsExternalRefs(ref); err != nil { + return "", nil, err + } + + resolvedPath, err := resolvePathWithRef(ref, path) + if err != nil { + return "", nil, err + } + + fragment := "#" + resolvedPath.Fragment + resolvedPath.Fragment = "" + return fragment, resolvedPath, nil +} + var ( errMUSTCallback = errors.New("invalid callback: value MUST be an object") errMUSTExample = errors.New("invalid example: value MUST be an object") @@ -563,6 +591,8 @@ func (loader *Loader) resolveHeaderRef(doc *T, component *HeaderRef, documentPat } if !loader.shouldVisitRef(ref, func(value any) { component.Value = value.(*Header) + _, refDocPath, _ := loader.resolveRefPath(ref, documentPath) + component.setRefPath(refDocPath) }) { return nil } @@ -573,7 +603,7 @@ func (loader *Loader) resolveHeaderRef(doc *T, component *HeaderRef, documentPat return err } component.Value = &header - component.refPath = *documentPath + component.setRefPath(documentPath) } else { var resolved HeaderRef doc, componentPath, err := loader.resolveComponent(doc, ref, documentPath, &resolved) @@ -587,7 +617,7 @@ func (loader *Loader) resolveHeaderRef(doc *T, component *HeaderRef, documentPat return err } component.Value = resolved.Value - component.refPath = resolved.refPath + component.setRefPath(resolved.RefPath()) } defer loader.unvisitRef(ref, component.Value) } @@ -615,6 +645,8 @@ func (loader *Loader) resolveParameterRef(doc *T, component *ParameterRef, docum } if !loader.shouldVisitRef(ref, func(value any) { component.Value = value.(*Parameter) + _, refDocPath, _ := loader.resolveRefPath(ref, documentPath) + component.setRefPath(refDocPath) }) { return nil } @@ -625,7 +657,7 @@ func (loader *Loader) resolveParameterRef(doc *T, component *ParameterRef, docum return err } component.Value = ¶m - component.refPath = *documentPath + component.setRefPath(documentPath) } else { var resolved ParameterRef doc, componentPath, err := loader.resolveComponent(doc, ref, documentPath, &resolved) @@ -639,7 +671,7 @@ func (loader *Loader) resolveParameterRef(doc *T, component *ParameterRef, docum return err } component.Value = resolved.Value - component.refPath = resolved.refPath + component.setRefPath(resolved.RefPath()) } defer loader.unvisitRef(ref, component.Value) } @@ -651,7 +683,8 @@ func (loader *Loader) resolveParameterRef(doc *T, component *ParameterRef, docum if value.Content != nil && value.Schema != nil { return errors.New("cannot contain both schema and content in a parameter") } - for _, contentType := range value.Content { + for _, name := range componentNames(value.Content) { + contentType := value.Content[name] if schema := contentType.Schema; schema != nil { if err := loader.resolveSchemaRef(doc, schema, documentPath, []string{}); err != nil { return err @@ -677,6 +710,8 @@ func (loader *Loader) resolveRequestBodyRef(doc *T, component *RequestBodyRef, d } if !loader.shouldVisitRef(ref, func(value any) { component.Value = value.(*RequestBody) + _, refDocPath, _ := loader.resolveRefPath(ref, documentPath) + component.setRefPath(refDocPath) }) { return nil } @@ -687,7 +722,7 @@ func (loader *Loader) resolveRequestBodyRef(doc *T, component *RequestBodyRef, d return err } component.Value = &requestBody - component.refPath = *documentPath + component.setRefPath(documentPath) } else { var resolved RequestBodyRef doc, componentPath, err := loader.resolveComponent(doc, ref, documentPath, &resolved) @@ -701,7 +736,7 @@ func (loader *Loader) resolveRequestBodyRef(doc *T, component *RequestBodyRef, d return err } component.Value = resolved.Value - component.refPath = resolved.refPath + component.setRefPath(resolved.RefPath()) } defer loader.unvisitRef(ref, component.Value) } @@ -710,16 +745,12 @@ func (loader *Loader) resolveRequestBodyRef(doc *T, component *RequestBodyRef, d return nil } - for _, contentType := range value.Content { + for _, name := range componentNames(value.Content) { + contentType := value.Content[name] if contentType == nil { continue } - examples := make([]string, 0, len(contentType.Examples)) - for name := range contentType.Examples { - examples = append(examples, name) - } - sort.Strings(examples) - for _, name := range examples { + for _, name := range componentNames(contentType.Examples) { example := contentType.Examples[name] if err := loader.resolveExampleRef(doc, example, documentPath); err != nil { return err @@ -746,6 +777,8 @@ func (loader *Loader) resolveResponseRef(doc *T, component *ResponseRef, documen } if !loader.shouldVisitRef(ref, func(value any) { component.Value = value.(*Response) + _, refDocPath, _ := loader.resolveRefPath(ref, documentPath) + component.setRefPath(refDocPath) }) { return nil } @@ -756,7 +789,7 @@ func (loader *Loader) resolveResponseRef(doc *T, component *ResponseRef, documen return err } component.Value = &resp - component.refPath = *documentPath + component.setRefPath(documentPath) } else { var resolved ResponseRef doc, componentPath, err := loader.resolveComponent(doc, ref, documentPath, &resolved) @@ -770,7 +803,7 @@ func (loader *Loader) resolveResponseRef(doc *T, component *ResponseRef, documen return err } component.Value = resolved.Value - component.refPath = resolved.refPath + component.setRefPath(resolved.RefPath()) } defer loader.unvisitRef(ref, component.Value) } @@ -779,21 +812,18 @@ func (loader *Loader) resolveResponseRef(doc *T, component *ResponseRef, documen return nil } - for _, header := range value.Headers { + for _, name := range componentNames(value.Headers) { + header := value.Headers[name] if err := loader.resolveHeaderRef(doc, header, documentPath); err != nil { return err } } - for _, contentType := range value.Content { + for _, name := range componentNames(value.Content) { + contentType := value.Content[name] if contentType == nil { continue } - examples := make([]string, 0, len(contentType.Examples)) - for name := range contentType.Examples { - examples = append(examples, name) - } - sort.Strings(examples) - for _, name := range examples { + for _, name := range componentNames(contentType.Examples) { example := contentType.Examples[name] if err := loader.resolveExampleRef(doc, example, documentPath); err != nil { return err @@ -807,7 +837,8 @@ func (loader *Loader) resolveResponseRef(doc *T, component *ResponseRef, documen contentType.Schema = schema } } - for _, link := range value.Links { + for _, name := range componentNames(value.Links) { + link := value.Links[name] if err := loader.resolveLinkRef(doc, link, documentPath); err != nil { return err } @@ -826,6 +857,8 @@ func (loader *Loader) resolveSchemaRef(doc *T, component *SchemaRef, documentPat } if !loader.shouldVisitRef(ref, func(value any) { component.Value = value.(*Schema) + _, refDocPath, _ := loader.resolveRefPath(ref, documentPath) + component.setRefPath(refDocPath) }) { return nil } @@ -836,7 +869,7 @@ func (loader *Loader) resolveSchemaRef(doc *T, component *SchemaRef, documentPat return err } component.Value = &schema - component.refPath = *documentPath + component.setRefPath(documentPath) } else { var resolved SchemaRef doc, componentPath, err := loader.resolveComponent(doc, ref, documentPath, &resolved) @@ -850,7 +883,7 @@ func (loader *Loader) resolveSchemaRef(doc *T, component *SchemaRef, documentPat return err } component.Value = resolved.Value - component.refPath = resolved.refPath + component.setRefPath(resolved.RefPath()) } defer loader.unvisitRef(ref, component.Value) } @@ -865,7 +898,8 @@ func (loader *Loader) resolveSchemaRef(doc *T, component *SchemaRef, documentPat return err } } - for _, v := range value.Properties { + for _, name := range componentNames(value.Properties) { + v := value.Properties[name] if err := loader.resolveSchemaRef(doc, v, documentPath, visited); err != nil { return err } @@ -909,6 +943,8 @@ func (loader *Loader) resolveSecuritySchemeRef(doc *T, component *SecurityScheme } if !loader.shouldVisitRef(ref, func(value any) { component.Value = value.(*SecurityScheme) + _, refDocPath, _ := loader.resolveRefPath(ref, documentPath) + component.setRefPath(refDocPath) }) { return nil } @@ -919,7 +955,7 @@ func (loader *Loader) resolveSecuritySchemeRef(doc *T, component *SecurityScheme return err } component.Value = &scheme - component.refPath = *documentPath + component.setRefPath(documentPath) } else { var resolved SecuritySchemeRef doc, componentPath, err := loader.resolveComponent(doc, ref, documentPath, &resolved) @@ -933,7 +969,7 @@ func (loader *Loader) resolveSecuritySchemeRef(doc *T, component *SecurityScheme return err } component.Value = resolved.Value - component.refPath = resolved.refPath + component.setRefPath(resolved.RefPath()) } defer loader.unvisitRef(ref, component.Value) } @@ -947,6 +983,8 @@ func (loader *Loader) resolveExampleRef(doc *T, component *ExampleRef, documentP } if !loader.shouldVisitRef(ref, func(value any) { component.Value = value.(*Example) + _, refDocPath, _ := loader.resolveRefPath(ref, documentPath) + component.setRefPath(refDocPath) }) { return nil } @@ -957,7 +995,7 @@ func (loader *Loader) resolveExampleRef(doc *T, component *ExampleRef, documentP return err } component.Value = &example - component.refPath = *documentPath + component.setRefPath(documentPath) } else { var resolved ExampleRef doc, componentPath, err := loader.resolveComponent(doc, ref, documentPath, &resolved) @@ -971,7 +1009,7 @@ func (loader *Loader) resolveExampleRef(doc *T, component *ExampleRef, documentP return err } component.Value = resolved.Value - component.refPath = resolved.refPath + component.setRefPath(resolved.RefPath()) } defer loader.unvisitRef(ref, component.Value) } @@ -989,6 +1027,8 @@ func (loader *Loader) resolveCallbackRef(doc *T, component *CallbackRef, documen } if !loader.shouldVisitRef(ref, func(value any) { component.Value = value.(*Callback) + _, refDocPath, _ := loader.resolveRefPath(ref, documentPath) + component.setRefPath(refDocPath) }) { return nil } @@ -999,7 +1039,7 @@ func (loader *Loader) resolveCallbackRef(doc *T, component *CallbackRef, documen return err } component.Value = &resolved - component.refPath = *documentPath + component.setRefPath(documentPath) } else { var resolved CallbackRef doc, componentPath, err := loader.resolveComponent(doc, ref, documentPath, &resolved) @@ -1013,7 +1053,7 @@ func (loader *Loader) resolveCallbackRef(doc *T, component *CallbackRef, documen return err } component.Value = resolved.Value - component.refPath = resolved.refPath + component.setRefPath(resolved.RefPath()) } defer loader.unvisitRef(ref, component.Value) } @@ -1022,7 +1062,9 @@ func (loader *Loader) resolveCallbackRef(doc *T, component *CallbackRef, documen return nil } - for _, pathItem := range value.Map() { + pathItems := value.Map() + for _, name := range componentNames(pathItems) { + pathItem := pathItems[name] if err = loader.resolvePathItemRef(doc, pathItem, documentPath); err != nil { return err } @@ -1041,6 +1083,8 @@ func (loader *Loader) resolveLinkRef(doc *T, component *LinkRef, documentPath *u } if !loader.shouldVisitRef(ref, func(value any) { component.Value = value.(*Link) + _, refDocPath, _ := loader.resolveRefPath(ref, documentPath) + component.setRefPath(refDocPath) }) { return nil } @@ -1051,7 +1095,7 @@ func (loader *Loader) resolveLinkRef(doc *T, component *LinkRef, documentPath *u return err } component.Value = &link - component.refPath = *documentPath + component.setRefPath(documentPath) } else { var resolved LinkRef doc, componentPath, err := loader.resolveComponent(doc, ref, documentPath, &resolved) @@ -1065,7 +1109,7 @@ func (loader *Loader) resolveLinkRef(doc *T, component *LinkRef, documentPath *u return err } component.Value = resolved.Value - component.refPath = resolved.refPath + component.setRefPath(resolved.RefPath()) } defer loader.unvisitRef(ref, component.Value) } @@ -1114,7 +1158,9 @@ func (loader *Loader) resolvePathItemRef(doc *T, pathItem *PathItem, documentPat return } } - for _, operation := range pathItem.Operations() { + operations := pathItem.Operations() + for _, name := range componentNames(operations) { + operation := operations[name] for _, parameter := range operation.Parameters { if err = loader.resolveParameterRef(doc, parameter, documentPath); err != nil { return @@ -1125,12 +1171,15 @@ func (loader *Loader) resolvePathItemRef(doc *T, pathItem *PathItem, documentPat return } } - for _, response := range operation.Responses.Map() { + responses := operation.Responses.Map() + for _, name := range componentNames(responses) { + response := responses[name] if err = loader.resolveResponseRef(doc, response, documentPath); err != nil { return } } - for _, callback := range operation.Callbacks { + for _, name := range componentNames(operation.Callbacks) { + callback := operation.Callbacks[name] if err = loader.resolveCallbackRef(doc, callback, documentPath); err != nil { return } diff --git a/openapi3/loader_circular_test.go b/openapi3/loader_circular_test.go index 864e7708a..f16b469d9 100644 --- a/openapi3/loader_circular_test.go +++ b/openapi3/loader_circular_test.go @@ -22,6 +22,7 @@ func TestLoadCircular(t *testing.T) { Ref: ref, Value: obj, } + arr.Items.setRefPath(&url.URL{Path: "testdata/circularRef2/AwsEnvironmentSettings.yaml"}) obj.Description = "test" obj.Properties = map[string]*SchemaRef{ "children": { @@ -35,7 +36,6 @@ func TestLoadCircular(t *testing.T) { } actual := doc.Paths.Map()["/sample"].Put.RequestBody.Value.Content.Get("application/json").Schema - actual.refPath = url.URL{} require.Equal(t, expected.Value, actual.Value) } diff --git a/openapi3/loader_read_from_uri_func_test.go b/openapi3/loader_read_from_uri_func_test.go index 8269c8ee6..17ea05ef3 100644 --- a/openapi3/loader_read_from_uri_func_test.go +++ b/openapi3/loader_read_from_uri_func_test.go @@ -14,7 +14,7 @@ func TestLoaderReadFromURIFunc(t *testing.T) { loader := NewLoader() loader.IsExternalRefsAllowed = true loader.ReadFromURIFunc = func(loader *Loader, url *url.URL) ([]byte, error) { - return os.ReadFile(filepath.Join("testdata", url.Path)) + return os.ReadFile(filepath.Join("testdata", filepath.FromSlash(url.Path))) } doc, err := loader.LoadFromFile("recursiveRef/openapi.yml") require.NoError(t, err) diff --git a/openapi3/loader_uri_reader.go b/openapi3/loader_uri_reader.go index ba7b5f24a..b023dfb29 100644 --- a/openapi3/loader_uri_reader.go +++ b/openapi3/loader_uri_reader.go @@ -79,7 +79,7 @@ func ReadFromFile(loader *Loader, location *url.URL) ([]byte, error) { if !is_file(location) { return nil, ErrURINotSupported } - return os.ReadFile(location.Path) + return os.ReadFile(filepath.FromSlash(location.Path)) } // URIMapCache returns a ReadFromURIFunc that caches the contents read from URI diff --git a/openapi3/refs.go b/openapi3/refs.go index 9374fb2b2..846dc55a0 100644 --- a/openapi3/refs.go +++ b/openapi3/refs.go @@ -40,6 +40,17 @@ func (x *CallbackRef) CollectionName() string { return "callbacks" } // RefPath returns the path of the $ref relative to the root document. func (x *CallbackRef) RefPath() *url.URL { return &x.refPath } +func (x *CallbackRef) setRefPath(u *url.URL) { + // Do not set to null or override a path already set. + // References can be loaded multiple times not all with access + // to the correct path info. + if u == nil || x.refPath != (url.URL{}) { + return + } + + x.refPath = *u +} + // MarshalYAML returns the YAML encoding of CallbackRef. func (x CallbackRef) MarshalYAML() (any, error) { if ref := x.Ref; ref != "" { @@ -166,6 +177,17 @@ func (x *ExampleRef) CollectionName() string { return "examples" } // RefPath returns the path of the $ref relative to the root document. func (x *ExampleRef) RefPath() *url.URL { return &x.refPath } +func (x *ExampleRef) setRefPath(u *url.URL) { + // Do not set to null or override a path already set. + // References can be loaded multiple times not all with access + // to the correct path info. + if u == nil || x.refPath != (url.URL{}) { + return + } + + x.refPath = *u +} + // MarshalYAML returns the YAML encoding of ExampleRef. func (x ExampleRef) MarshalYAML() (any, error) { if ref := x.Ref; ref != "" { @@ -292,6 +314,17 @@ func (x *HeaderRef) CollectionName() string { return "headers" } // RefPath returns the path of the $ref relative to the root document. func (x *HeaderRef) RefPath() *url.URL { return &x.refPath } +func (x *HeaderRef) setRefPath(u *url.URL) { + // Do not set to null or override a path already set. + // References can be loaded multiple times not all with access + // to the correct path info. + if u == nil || x.refPath != (url.URL{}) { + return + } + + x.refPath = *u +} + // MarshalYAML returns the YAML encoding of HeaderRef. func (x HeaderRef) MarshalYAML() (any, error) { if ref := x.Ref; ref != "" { @@ -418,6 +451,17 @@ func (x *LinkRef) CollectionName() string { return "links" } // RefPath returns the path of the $ref relative to the root document. func (x *LinkRef) RefPath() *url.URL { return &x.refPath } +func (x *LinkRef) setRefPath(u *url.URL) { + // Do not set to null or override a path already set. + // References can be loaded multiple times not all with access + // to the correct path info. + if u == nil || x.refPath != (url.URL{}) { + return + } + + x.refPath = *u +} + // MarshalYAML returns the YAML encoding of LinkRef. func (x LinkRef) MarshalYAML() (any, error) { if ref := x.Ref; ref != "" { @@ -544,6 +588,17 @@ func (x *ParameterRef) CollectionName() string { return "parameters" } // RefPath returns the path of the $ref relative to the root document. func (x *ParameterRef) RefPath() *url.URL { return &x.refPath } +func (x *ParameterRef) setRefPath(u *url.URL) { + // Do not set to null or override a path already set. + // References can be loaded multiple times not all with access + // to the correct path info. + if u == nil || x.refPath != (url.URL{}) { + return + } + + x.refPath = *u +} + // MarshalYAML returns the YAML encoding of ParameterRef. func (x ParameterRef) MarshalYAML() (any, error) { if ref := x.Ref; ref != "" { @@ -670,6 +725,17 @@ func (x *RequestBodyRef) CollectionName() string { return "requestBodies" } // RefPath returns the path of the $ref relative to the root document. func (x *RequestBodyRef) RefPath() *url.URL { return &x.refPath } +func (x *RequestBodyRef) setRefPath(u *url.URL) { + // Do not set to null or override a path already set. + // References can be loaded multiple times not all with access + // to the correct path info. + if u == nil || x.refPath != (url.URL{}) { + return + } + + x.refPath = *u +} + // MarshalYAML returns the YAML encoding of RequestBodyRef. func (x RequestBodyRef) MarshalYAML() (any, error) { if ref := x.Ref; ref != "" { @@ -796,6 +862,17 @@ func (x *ResponseRef) CollectionName() string { return "responses" } // RefPath returns the path of the $ref relative to the root document. func (x *ResponseRef) RefPath() *url.URL { return &x.refPath } +func (x *ResponseRef) setRefPath(u *url.URL) { + // Do not set to null or override a path already set. + // References can be loaded multiple times not all with access + // to the correct path info. + if u == nil || x.refPath != (url.URL{}) { + return + } + + x.refPath = *u +} + // MarshalYAML returns the YAML encoding of ResponseRef. func (x ResponseRef) MarshalYAML() (any, error) { if ref := x.Ref; ref != "" { @@ -922,6 +999,17 @@ func (x *SchemaRef) CollectionName() string { return "schemas" } // RefPath returns the path of the $ref relative to the root document. func (x *SchemaRef) RefPath() *url.URL { return &x.refPath } +func (x *SchemaRef) setRefPath(u *url.URL) { + // Do not set to null or override a path already set. + // References can be loaded multiple times not all with access + // to the correct path info. + if u == nil || x.refPath != (url.URL{}) { + return + } + + x.refPath = *u +} + // MarshalYAML returns the YAML encoding of SchemaRef. func (x SchemaRef) MarshalYAML() (any, error) { if ref := x.Ref; ref != "" { @@ -1048,6 +1136,17 @@ func (x *SecuritySchemeRef) CollectionName() string { return "securitySchemes" } // RefPath returns the path of the $ref relative to the root document. func (x *SecuritySchemeRef) RefPath() *url.URL { return &x.refPath } +func (x *SecuritySchemeRef) setRefPath(u *url.URL) { + // Do not set to null or override a path already set. + // References can be loaded multiple times not all with access + // to the correct path info. + if u == nil || x.refPath != (url.URL{}) { + return + } + + x.refPath = *u +} + // MarshalYAML returns the YAML encoding of SecuritySchemeRef. func (x SecuritySchemeRef) MarshalYAML() (any, error) { if ref := x.Ref; ref != "" { diff --git a/openapi3/refs.tmpl b/openapi3/refs.tmpl index a965bd1e1..f9ed1e6e9 100644 --- a/openapi3/refs.tmpl +++ b/openapi3/refs.tmpl @@ -40,6 +40,17 @@ func (x *{{ $type.Name }}Ref) CollectionName() string { return "{{ $type.Collect // RefPath returns the path of the $ref relative to the root document. func (x *{{ $type.Name }}Ref) RefPath() *url.URL { return &x.refPath } +func (x *{{ $type.Name }}Ref) setRefPath(u *url.URL) { + // Do not set to null or override a path already set. + // References can be loaded multiple times not all with access + // to the correct path info. + if u == nil || x.refPath != (url.URL{}) { + return + } + + x.refPath = *u +} + // MarshalYAML returns the YAML encoding of {{ $type.Name }}Ref. func (x {{ $type.Name }}Ref) MarshalYAML() (any, error) { if ref := x.Ref; ref != "" { diff --git a/openapi3/testdata/interalizationNameCollision/api.yml b/openapi3/testdata/interalizationNameCollision/api.yml new file mode 100644 index 000000000..fe39f2986 --- /dev/null +++ b/openapi3/testdata/interalizationNameCollision/api.yml @@ -0,0 +1,25 @@ +openapi: "3.0.0" +info: + version: 1.0.0 + title: Internalise ref name collision. +paths: + /book/record: + get: + operationId: getBookRecord + responses: + 200: + description: A Book record. + content: + application/json: + schema: + $ref: './schemas/book/record.yml' + /cd/record: + get: + operationId: getCDRecord + responses: + 200: + description: A CD record. + content: + application/json: + schema: + $ref: './schemas/cd/record.yml' diff --git a/openapi3/testdata/interalizationNameCollision/api.yml.internalized.yml b/openapi3/testdata/interalizationNameCollision/api.yml.internalized.yml new file mode 100644 index 000000000..75e52f0fc --- /dev/null +++ b/openapi3/testdata/interalizationNameCollision/api.yml.internalized.yml @@ -0,0 +1,75 @@ +{ + "openapi": "3.0.0", + "info": { + "version": "1.0.0", + "title": "Internalise ref name collision." + }, + "paths": { + "/book/record": { + "get": { + "operationId": "getBookRecord", + "responses": { + "200": { + "description": "A Book record.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/schemas_book_record" + } + } + } + } + } + } + }, + "/cd/record": { + "get": { + "operationId": "getCDRecord", + "responses": { + "200": { + "description": "A CD record.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/schemas_cd_record" + } + } + } + } + } + } + } + }, + "components": { + "schemas": { + "schemas_book_record": { + "type": "object", + "required": [ + "id" + ], + "properties": { + "id": { + "type": "number" + }, + "pages": { + "type": "number" + } + } + }, + "schemas_cd_record": { + "type": "object", + "required": [ + "id" + ], + "properties": { + "id": { + "type": "number" + }, + "tracks": { + "type": "number" + } + } + } + } + } +} diff --git a/openapi3/testdata/interalizationNameCollision/schemas/book/record.yml b/openapi3/testdata/interalizationNameCollision/schemas/book/record.yml new file mode 100644 index 000000000..ef72cc994 --- /dev/null +++ b/openapi3/testdata/interalizationNameCollision/schemas/book/record.yml @@ -0,0 +1,8 @@ +type: object +required: + - id +properties: + id: + type: number + pages: + type: number diff --git a/openapi3/testdata/interalizationNameCollision/schemas/cd/record.yml b/openapi3/testdata/interalizationNameCollision/schemas/cd/record.yml new file mode 100644 index 000000000..0e5ba7acf --- /dev/null +++ b/openapi3/testdata/interalizationNameCollision/schemas/cd/record.yml @@ -0,0 +1,8 @@ +type: object +required: + - id +properties: + id: + type: number + tracks: + type: number diff --git a/openapi3/testdata/issue959/openapi.yml.internalized.yml b/openapi3/testdata/issue959/openapi.yml.internalized.yml index 3cbe674d6..db83681dc 100644 --- a/openapi3/testdata/issue959/openapi.yml.internalized.yml +++ b/openapi3/testdata/issue959/openapi.yml.internalized.yml @@ -1,7 +1,7 @@ { "components": { "schemas": { - "External1": { + "components_External1": { "type": "string" } } @@ -26,10 +26,10 @@ "name": "external1", "required": true, "schema": { - "$ref": "#/components/schemas/External1" + "$ref": "#/components/schemas/components_External1" } } ] } } -} \ No newline at end of file +} diff --git a/openapi3/testdata/recursiveRef/openapi.yml.internalized.yml b/openapi3/testdata/recursiveRef/openapi.yml.internalized.yml index 0d508527a..33387fdf0 100644 --- a/openapi3/testdata/recursiveRef/openapi.yml.internalized.yml +++ b/openapi3/testdata/recursiveRef/openapi.yml.internalized.yml @@ -1,7 +1,7 @@ { "components": { "parameters": { - "number": { + "parameters_number": { "in": "query", "name": "someNumber", "schema": { @@ -22,6 +22,34 @@ } }, "schemas": { + "components_Bar": { + "example": "bar", + "type": "string" + }, + "components_Cat": { + "properties": { + "cat": { + "$ref": "#/components/schemas/components_Cat" + } + }, + "type": "object" + }, + "components_Foo": { + "properties": { + "bar": { + "$ref": "#/components/schemas/components_Bar" + } + }, + "type": "object" + }, + "components_Foo_Foo2": { + "properties": { + "foo": { + "$ref": "#/components/schemas/components_Foo" + } + }, + "type": "object" + }, "Bar": { "example": "bar", "type": "string" @@ -33,7 +61,7 @@ "Foo": { "properties": { "bar": { - "$ref": "#/components/schemas/Bar" + "$ref": "#/components/schemas/components_Bar" } }, "type": "object" @@ -41,19 +69,15 @@ "Foo2": { "properties": { "foo": { - "$ref": "#/components/schemas/Foo" + "$ref": "#/components/schemas/components_Foo" } }, "type": "object" }, - "error":{ - "title":"ErrorDetails", - "type":"object" - }, "Cat": { "properties": { "cat": { - "$ref": "#/components/schemas/Cat" + "$ref": "#/components/schemas/components_Cat" } }, "type": "object" @@ -86,7 +110,7 @@ "schema": { "properties": { "foo2": { - "$ref": "#/components/schemas/Foo2" + "$ref": "#/components/schemas/components_Foo_Foo2" } }, "type": "object" @@ -102,7 +126,7 @@ }, "parameters": [ { - "$ref": "#/components/parameters/number" + "$ref": "#/components/parameters/parameters_number" } ] } diff --git a/openapi3/testdata/spec.yaml.internalized.yml b/openapi3/testdata/spec.yaml.internalized.yml index feca4a00c..bcfd3fe9f 100644 --- a/openapi3/testdata/spec.yaml.internalized.yml +++ b/openapi3/testdata/spec.yaml.internalized.yml @@ -4,19 +4,19 @@ "Test": { "properties": { "test": { - "$ref": "#/components/schemas/b" + "$ref": "#/components/schemas/ext_definitions_b" } }, "type": "object" }, - "a": { + "ext_definitions_a": { "type": "string" }, - "b": { + "ext_definitions_b": { "description": "I use a local reference.", "properties": { "name": { - "$ref": "#/components/schemas/a" + "$ref": "#/components/schemas/ext_definitions_a" } }, "type": "object" diff --git a/openapi3/testdata/testref.openapi.yml.internalized.yml b/openapi3/testdata/testref.openapi.yml.internalized.yml index e35a50041..f270941d8 100644 --- a/openapi3/testdata/testref.openapi.yml.internalized.yml +++ b/openapi3/testdata/testref.openapi.yml.internalized.yml @@ -4,7 +4,7 @@ "AnotherTestSchema": { "type": "string" }, - "CustomTestSchema": { + "components_Name": { "type": "string" } }