diff --git a/.github/docs/openapi3.txt b/.github/docs/openapi3.txt index c04d34ec2..725e1a1b8 100644 --- a/.github/docs/openapi3.txt +++ b/.github/docs/openapi3.txt @@ -3343,6 +3343,18 @@ func (doc *T) ValidateSchemaJSON(schema *Schema, value any, opts ...SchemaValida format validators. This is a convenience method that automatically applies the document's format validators. +func (doc *T) WalkParameters(fn WalkParametersFunc) error + WalkParameters visits every parameter reachable from the document exactly + once, invoking fn for each. It follows resolved $ref targets (param.Value), + so each distinct *Parameter is visited a single time regardless of how + many references point at it. Maps are visited in sorted key order, so the + traversal is deterministic. + + It covers components.parameters, path items, operations, callbacks, + and webhooks. It is the parameter counterpart of WalkSchemas: useful for + validation, linting, parameter transformation, and documentation, without + re-deriving the (easy to get wrong) traversal. + func (doc *T) WalkSchemas(fn WalkSchemasFunc) error WalkSchemas visits every schema reachable from the document exactly once, invoking fn for each. It follows resolved $ref targets (schema.Value) and @@ -3691,6 +3703,20 @@ type ValidationOptions struct { } ValidationOptions provides configuration for validating OpenAPI documents. +type WalkParametersFunc func(jsonPointer string, param *ParameterRef) error + WalkParametersFunc is called once for each parameter visited by + WalkParameters. + + jsonPointer is the RFC 6901 JSON Pointer of the parameter + within the document, e.g. "/components/parameters/Limit" or + "/paths/~1pets/get/parameters/0". For a parameter referenced from several + places it is the pointer of the first visit, and components are visited + first, so a shared parameter is reported at its definition. param is non-nil + and param.Value is non-nil; the callback may modify param.Value in place, + so WalkParameters serves transformers and not only read-only inspection. + + Returning a non-nil error aborts the walk and is returned by WalkParameters. + type WalkSchemasFunc func(jsonPointer string, schema *SchemaRef) error WalkSchemasFunc is called once for each schema visited by WalkSchemas. diff --git a/openapi3/loader.go b/openapi3/loader.go index 4c2ebe288..2f2480c07 100644 --- a/openapi3/loader.go +++ b/openapi3/loader.go @@ -64,6 +64,13 @@ type Loader struct { visitedDocuments map[string]*T + // originTrees retains each loaded document's origin tree, keyed by the + // document itself so insert and lookup cannot disagree, populated when + // IncludeOrigin is set. resolveComponent uses it to re-attach origins to + // components that lose them in the generic-map path, without re-reading + // or re-parsing the file. + originTrees map[*T]*originTree + visitedRefs map[string]struct{} visitedPath []string backtrack map[string][]func(value any) @@ -133,13 +140,25 @@ func (loader *Loader) loadSingleElementFromURI(ref string, rootPath *url.URL, el if err != nil { return nil, err } - if err := unmarshal(data, element, loader.IncludeOrigin, resolvedPath); err != nil { + if _, err := unmarshal(data, element, loader.IncludeOrigin, resolvedPath); err != nil { return nil, err } return resolvedPath, nil } +// rememberOriginTree retains doc's origin tree for attachOriginToResolved. +// tree is nil when IncludeOrigin is off or the data took the json path. +func (loader *Loader) rememberOriginTree(doc *T, tree *originTree) { + if tree == nil { + return + } + if loader.originTrees == nil { + loader.originTrees = make(map[*T]*originTree) + } + loader.originTrees[doc] = tree +} + func (loader *Loader) readURL(location *url.URL) ([]byte, error) { if f := loader.ReadFromURIFunc; f != nil { return f(loader, location) @@ -169,9 +188,11 @@ func (loader *Loader) LoadFromIoReader(reader io.Reader) (*T, error) { func (loader *Loader) LoadFromData(data []byte) (*T, error) { loader.resetVisitedPathItemRefs() doc := &T{} - if err := unmarshal(data, doc, loader.IncludeOrigin, nil); err != nil { + tree, err := unmarshal(data, doc, loader.IncludeOrigin, nil) + if err != nil { return nil, err } + loader.rememberOriginTree(doc, tree) if err := loader.ResolveRefsIn(doc, nil); err != nil { return nil, err } @@ -198,9 +219,11 @@ func (loader *Loader) loadFromDataWithPathInternal(data []byte, location *url.UR doc := &T{} loader.visitedDocuments[uri] = doc - if err := unmarshal(data, doc, loader.IncludeOrigin, location); err != nil { + tree, err := unmarshal(data, doc, loader.IncludeOrigin, location) + if err != nil { return nil, err } + loader.rememberOriginTree(doc, tree) doc.url = copyURI(location) @@ -468,7 +491,7 @@ func (loader *Loader) resolveComponent(doc *T, ref string, path *url.URL, resolv if err2 != nil { return nil, nil, err } - if err2 = unmarshal(data, &cursor, loader.IncludeOrigin, path); err2 != nil { + if _, err2 = unmarshal(data, &cursor, loader.IncludeOrigin, path); err2 != nil { return nil, nil, err } if cursor, err2 = drill(cursor); err2 != nil || cursor == nil { @@ -516,6 +539,10 @@ func (loader *Loader) resolveComponent(doc *T, ref string, path *url.URL, resolv if err := codec(cursor, resolved); err != nil { return nil, nil, fmt.Errorf("bad data in %q (expecting %s)", ref, readableType(resolved)) } + // The value came from a generic map in T.Extensions (a $ref to an + // arbitrary top-level key), so the json round-trip above stripped its + // origins. Re-attach them from the file, best-effort. + loader.attachOriginToResolved(resolved, componentDoc, fragment) return componentDoc, componentPath, nil default: @@ -523,6 +550,33 @@ func (loader *Loader) resolveComponent(doc *T, ref string, path *url.URL, resolv } } +// attachOriginToResolved re-attaches source origins to a component resolved +// through the generic-map path: a $ref to a schema under an arbitrary top-level +// key lands in T.Extensions, and the json round-trip in resolveComponent strips +// its origin. It walks the document's retained origin tree (see originTrees, +// populated when the document was first unmarshaled: no re-read, no re-parse) +// down to the ref fragment and applies that subtree, so the object carries the +// same origins a typed resolution would, with the original file's line numbers. +// Best-effort: a missing tree or fragment leaves the object without origins. +func (loader *Loader) attachOriginToResolved(resolved any, componentDoc *T, fragment string) { + if !loader.IncludeOrigin { + return + } + tree := loader.originTrees[componentDoc] + if tree == nil { + return + } + for part := range strings.SplitSeq(strings.Trim(fragment, "/"), "/") { + if part == "" { + continue + } + if tree = tree.Fields[unescapeRefString(part)]; tree == nil { + return + } + } + applyOrigins(resolved, tree) +} + func readableType(x any) string { switch x.(type) { case *Callback: diff --git a/openapi3/marsh.go b/openapi3/marsh.go index 6483fa9a4..f895e6e40 100644 --- a/openapi3/marsh.go +++ b/openapi3/marsh.go @@ -17,12 +17,15 @@ func unmarshalError(jsonUnmarshalErr error) error { return jsonUnmarshalErr } -func unmarshal(data []byte, v any, includeOrigin bool, location *url.URL) error { +// unmarshal decodes data into v. It returns the document origin tree when +// includeOrigin is set and the data took the yaml path (json input carries no +// origins), so the caller can retain it (see Loader.originTrees). +func unmarshal(data []byte, v any, includeOrigin bool, location *url.URL) (*originTree, error) { var jsonErr, yamlErr error // See https://github.com/getkin/kin-openapi/issues/680 if jsonErr = json.Unmarshal(data, v); jsonErr == nil { - return nil + return nil, nil } // UnmarshalStrict(data, v) TODO: investigate how ymlv3 handles duplicate map keys @@ -35,11 +38,11 @@ func unmarshal(data []byte, v any, includeOrigin bool, location *url.URL) error DisableTimestamps: true, }); err == nil { applyOrigins(v, tree) - return nil + return tree, nil } else { yamlErr = err } // If both unmarshaling attempts fail, return a new error that includes both errors - return fmt.Errorf("failed to unmarshal data: json error: %v, yaml error: %v", jsonErr, yamlErr) + return nil, fmt.Errorf("failed to unmarshal data: json error: %v, yaml error: %v", jsonErr, yamlErr) } diff --git a/openapi3/origin.go b/openapi3/origin.go index e55695a5b..75e1e70ad 100644 --- a/openapi3/origin.go +++ b/openapi3/origin.go @@ -324,3 +324,7 @@ func jsonTagName(f reflect.StructField) string { name, _, _ := strings.Cut(tag, ",") return name } + +// originTree aliases the decoder-side origin tree, so the loader and marsh can +// carry it without referencing the yaml package directly. +type originTree = yaml.OriginTree diff --git a/openapi3/origin_external_ref_test.go b/openapi3/origin_external_ref_test.go new file mode 100644 index 000000000..83f7ea019 --- /dev/null +++ b/openapi3/origin_external_ref_test.go @@ -0,0 +1,106 @@ +package openapi3 + +import ( + "net/url" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" +) + +// A $ref to a schema stored under an arbitrary top-level key in another file +// (e.g. "./schemas.yaml#/User", the Swagger-2-era "definitions bag") must carry +// the origin of the file it lives in, like a $ref into /components/schemas does. +// The key is not a typed field of T, so the loader reaches it through T.Extensions +// (a generic map); the origin must survive that path. +func TestOrigin_ExternalRefToArbitraryTopLevelKey(t *testing.T) { + loader := NewLoader() + loader.IncludeOrigin = true + loader.IsExternalRefsAllowed = true + + doc, err := loader.LoadFromFile("testdata/origin/arbitrary_key.yaml") + require.NoError(t, err) + + user := doc.Paths.Value("/users").Get.Responses.Value("200").Value.Content["application/json"].Schema.Value + require.NotNil(t, user, "the User schema resolves") + + require.NotNil(t, user.Origin, "a schema $ref'd under an arbitrary top-level key must carry an origin") + require.NotNil(t, user.Origin.Key, "the origin has a key location") + require.Equal(t, "arbitrary_key_schemas.yaml", filepath.Base(user.Origin.Key.File), + "origin points at the file the schema lives in, not the referencing document") + require.Equal(t, Location{ + File: user.Origin.Key.File, + Line: 1, + Column: 1, + Name: "User", + EndLine: 7, EndColumn: 19, + }, *user.Origin.Key, "the key origin spans the whole User block in arbitrary_key_schemas.yaml") + require.Equal(t, 2, user.Origin.Fields["type"].Line, "field origins are attached too") + + // the subtree gets origins as well, with the same file + id := user.Properties["id"].Value + require.NotNil(t, id.Origin) + require.Equal(t, user.Origin.Key.File, id.Origin.Key.File) + require.Equal(t, 4, id.Origin.Key.Line, "the id property's own line") + require.Equal(t, 5, id.Origin.Fields["type"].Line) +} + +// Re-attaching origins reuses the origin tree retained at load time: resolving +// the arbitrary-key $ref must not read any file a second time. +func TestOrigin_ExternalRefToArbitraryTopLevelKey_NoRereads(t *testing.T) { + reads := map[string]int{} + loader := NewLoader() + loader.IncludeOrigin = true + loader.ReadFromURIFunc = func(l *Loader, location *url.URL) ([]byte, error) { + reads[location.String()]++ + return DefaultReadFromURI(l, location) + } + + doc, err := loader.LoadFromFile("testdata/origin/arbitrary_key.yaml") + require.NoError(t, err) + + user := doc.Paths.Value("/users").Get.Responses.Value("200").Value.Content["application/json"].Schema.Value + require.NotNil(t, user.Origin, "origins are attached") + + require.Len(t, reads, 2, "the root and the $ref'd file") + for location, n := range reads { + require.Equalf(t, 1, n, "%s must be read exactly once", location) + } +} + +// Keying the retained trees by document also serves documents loaded from +// memory (no location): a $ref to an arbitrary top-level key in the same +// document gets its origin attached too. +func TestOrigin_InternalRefToArbitraryTopLevelKey_FromData(t *testing.T) { + const spec = ` +openapi: 3.0.0 +info: { title: t, version: "1" } +paths: + /users: + get: + responses: + "200": + description: OK + content: + application/json: + schema: + $ref: '#/User' +User: + type: object + properties: + id: { type: string } +` + loader := NewLoader() + loader.IncludeOrigin = true + doc, err := loader.LoadFromData([]byte(spec)) + require.NoError(t, err) + + user := doc.Paths.Value("/users").Get.Responses.Value("200").Value.Content["application/json"].Schema.Value + require.NotNil(t, user.Origin, "a same-document arbitrary-key $ref carries an origin") + require.NotNil(t, user.Origin.Key) + require.Equal(t, "User", user.Origin.Key.Name) + require.Equal(t, 14, user.Origin.Key.Line, "the User: line in the document above") + require.Equal(t, 17, user.Origin.Key.EndLine, "the block's last line") + require.Equal(t, 15, user.Origin.Fields["type"].Line) + require.Empty(t, user.Origin.Key.File, "a document loaded from data has no file") +} diff --git a/openapi3/testdata/origin/arbitrary_key.yaml b/openapi3/testdata/origin/arbitrary_key.yaml new file mode 100644 index 000000000..11cf195fe --- /dev/null +++ b/openapi3/testdata/origin/arbitrary_key.yaml @@ -0,0 +1,14 @@ +openapi: 3.0.0 +info: + title: Arbitrary top-level key ref + version: 1.0.0 +paths: + /users: + get: + responses: + "200": + description: OK + content: + application/json: + schema: + $ref: "./arbitrary_key_schemas.yaml#/User" diff --git a/openapi3/testdata/origin/arbitrary_key_schemas.yaml b/openapi3/testdata/origin/arbitrary_key_schemas.yaml new file mode 100644 index 000000000..8474e357b --- /dev/null +++ b/openapi3/testdata/origin/arbitrary_key_schemas.yaml @@ -0,0 +1,7 @@ +User: + type: object + properties: + id: + type: string + name: + type: string diff --git a/openapi3/walk_parameters.go b/openapi3/walk_parameters.go new file mode 100644 index 000000000..d60edb6c7 --- /dev/null +++ b/openapi3/walk_parameters.go @@ -0,0 +1,128 @@ +package openapi3 + +import ( + "maps" + "slices" + "strconv" + "strings" +) + +// WalkParametersFunc is called once for each parameter visited by +// WalkParameters. +// +// jsonPointer is the RFC 6901 JSON Pointer of the parameter within the +// document, e.g. "/components/parameters/Limit" or +// "/paths/~1pets/get/parameters/0". For a parameter referenced from several +// places it is the pointer of the first visit, and components are visited +// first, so a shared parameter is reported at its definition. param is +// non-nil and param.Value is non-nil; the callback may modify param.Value in +// place, so WalkParameters serves transformers and not only read-only +// inspection. +// +// Returning a non-nil error aborts the walk and is returned by +// WalkParameters. +type WalkParametersFunc func(jsonPointer string, param *ParameterRef) error + +// WalkParameters visits every parameter reachable from the document exactly +// once, invoking fn for each. It follows resolved $ref targets (param.Value), +// so each distinct *Parameter is visited a single time regardless of how many +// references point at it. Maps are visited in sorted key order, so the +// traversal is deterministic. +// +// It covers components.parameters, path items, operations, callbacks, and +// webhooks. It is the parameter counterpart of WalkSchemas: useful for +// validation, linting, parameter transformation, and documentation, without +// re-deriving the (easy to get wrong) traversal. +func (doc *T) WalkParameters(fn WalkParametersFunc) error { + if doc == nil { + return nil + } + w := parameterWalker{fn: fn, seen: make(map[*Parameter]struct{})} + return w.document(doc) +} + +type parameterWalker struct { + fn WalkParametersFunc + seen map[*Parameter]struct{} +} + +func (w *parameterWalker) document(doc *T) error { + if c := doc.Components; c != nil { + for _, name := range slices.Sorted(maps.Keys(c.Parameters)) { + if err := w.parameter("/components/parameters/"+escapeRefString(name), c.Parameters[name]); err != nil { + return err + } + } + for _, name := range slices.Sorted(maps.Keys(c.Callbacks)) { + if cbr := c.Callbacks[name]; cbr != nil && cbr.Value != nil { + if err := w.callback("/components/callbacks/"+escapeRefString(name), cbr.Value); err != nil { + return err + } + } + } + } + if doc.Paths != nil { + items := doc.Paths.Map() + for _, path := range slices.Sorted(maps.Keys(items)) { + if err := w.pathItem("/paths/"+escapeRefString(path), items[path]); err != nil { + return err + } + } + } + for _, name := range slices.Sorted(maps.Keys(doc.Webhooks)) { + if err := w.pathItem("/webhooks/"+escapeRefString(name), doc.Webhooks[name]); err != nil { + return err + } + } + return nil +} + +func (w *parameterWalker) pathItem(ptr string, item *PathItem) error { + if item == nil { + return nil + } + for i, pr := range item.Parameters { + if err := w.parameter(ptr+"/parameters/"+strconv.Itoa(i), pr); err != nil { + return err + } + } + ops := item.Operations() + for _, method := range slices.Sorted(maps.Keys(ops)) { + op := ops[method] + opPtr := ptr + "/" + strings.ToLower(method) + for i, pr := range op.Parameters { + if err := w.parameter(opPtr+"/parameters/"+strconv.Itoa(i), pr); err != nil { + return err + } + } + for _, name := range slices.Sorted(maps.Keys(op.Callbacks)) { + if cbr := op.Callbacks[name]; cbr != nil && cbr.Value != nil { + if err := w.callback(opPtr+"/callbacks/"+escapeRefString(name), cbr.Value); err != nil { + return err + } + } + } + } + return nil +} + +func (w *parameterWalker) callback(ptr string, cb *Callback) error { + items := cb.Map() + for _, expr := range slices.Sorted(maps.Keys(items)) { + if err := w.pathItem(ptr+"/"+escapeRefString(expr), items[expr]); err != nil { + return err + } + } + return nil +} + +func (w *parameterWalker) parameter(ptr string, pr *ParameterRef) error { + if pr == nil || pr.Value == nil { + return nil + } + if _, ok := w.seen[pr.Value]; ok { + return nil + } + w.seen[pr.Value] = struct{}{} + return w.fn(ptr, pr) +} diff --git a/openapi3/walk_parameters_test.go b/openapi3/walk_parameters_test.go new file mode 100644 index 000000000..951509850 --- /dev/null +++ b/openapi3/walk_parameters_test.go @@ -0,0 +1,108 @@ +package openapi3_test + +import ( + "errors" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/getkin/kin-openapi/openapi3" +) + +// walkParametersSpec exercises parameters in components (shared via $ref from +// two operations: dedup), path items, operations, a callback, and a webhook. +var walkParametersSpec = []byte(` +openapi: 3.1.0 +info: {title: t, version: "1"} +paths: + /pets: + parameters: + - name: tenant + in: header + schema: {type: string} + get: + parameters: + - $ref: '#/components/parameters/Limit' + - name: filter + in: query + schema: {type: string} + responses: + "200": {description: ok} + post: + parameters: + - $ref: '#/components/parameters/Limit' + callbacks: + onEvent: + '{$request.body#/url}': + post: + parameters: + - name: attempt + in: query + schema: {type: integer} + responses: + "200": {description: ok} + responses: + "201": {description: created} +webhooks: + newPet: + post: + parameters: + - name: signature + in: header + schema: {type: string} + responses: + "200": {description: ok} +components: + parameters: + Limit: + name: limit + in: query + schema: {type: integer} +`) + +func TestWalkParameters(t *testing.T) { + loader := openapi3.NewLoader() + doc, err := loader.LoadFromData(walkParametersSpec) + require.NoError(t, err) + + visited := map[string]string{} // pointer -> parameter name + require.NoError(t, doc.WalkParameters(func(jsonPointer string, param *openapi3.ParameterRef) error { + require.NotNil(t, param) + require.NotNil(t, param.Value) + _, dup := visited[jsonPointer] + require.Falsef(t, dup, "pointer %q visited twice", jsonPointer) + visited[jsonPointer] = param.Value.Name + return nil + })) + + require.Equal(t, map[string]string{ + // the shared Limit parameter is visited once, at its definition + "/components/parameters/Limit": "limit", + "/paths/~1pets/parameters/0": "tenant", + "/paths/~1pets/get/parameters/1": "filter", + "/paths/~1pets/post/callbacks/onEvent/{$request.body#~1url}/post/parameters/0": "attempt", + "/webhooks/newPet/post/parameters/0": "signature", + }, visited) +} + +func TestWalkParameters_ErrorAborts(t *testing.T) { + loader := openapi3.NewLoader() + doc, err := loader.LoadFromData(walkParametersSpec) + require.NoError(t, err) + + boom := errors.New("boom") + calls := 0 + require.ErrorIs(t, doc.WalkParameters(func(string, *openapi3.ParameterRef) error { + calls++ + return boom + }), boom) + require.Equal(t, 1, calls, "the walk stops at the first error") +} + +func TestWalkParameters_NilDoc(t *testing.T) { + var doc *openapi3.T + require.NoError(t, doc.WalkParameters(func(string, *openapi3.ParameterRef) error { + t.Fatal("no visits on a nil document") + return nil + })) +} diff --git a/openapi3/walk.go b/openapi3/walk_schemas.go similarity index 100% rename from openapi3/walk.go rename to openapi3/walk_schemas.go diff --git a/openapi3/walk_test.go b/openapi3/walk_schemas_test.go similarity index 100% rename from openapi3/walk_test.go rename to openapi3/walk_schemas_test.go