From c8a479945dbf52e1b67e1c7ad9598867b1528757 Mon Sep 17 00:00:00 2001 From: quobix Date: Sat, 31 Oct 2026 10:49:07 -0400 Subject: [PATCH 01/33] fix(generator): preserve untyped error response precedence --- generator/sdk/gosdk/coverage_test.go | 3 +- .../gosdk/error_response_precedence_test.go | 134 ++++++++++++++++++ generator/sdk/gosdk/generator.go | 13 +- generator/sdk/gosdk/templates/resources.tmpl | 2 + .../testdata/people_match_resources.golden.go | 1 + 5 files changed, 149 insertions(+), 4 deletions(-) create mode 100644 generator/sdk/gosdk/error_response_precedence_test.go diff --git a/generator/sdk/gosdk/coverage_test.go b/generator/sdk/gosdk/coverage_test.go index 5347c16e..a4f17b5e 100644 --- a/generator/sdk/gosdk/coverage_test.go +++ b/generator/sdk/gosdk/coverage_test.go @@ -337,7 +337,8 @@ func TestPrepareResponsesContracts(t *testing.T) { {Status: "default", Content: jsonContent(schema("object", ""))}, }}, "Read", "[ThingsResource.Read]", &view) if err != nil || view.ResponseType != "string" || !reflect.DeepEqual(view.SuccessStatuses, []string{"200"}) || !view.DecodeAll || - len(view.ErrorCases) != 1 || !view.ErrorCases[0].Default || view.ErrorCases[0].Type != "ReadError" || view.ErrorSwitch != "response.StatusCode" { + len(view.ErrorCases) != 2 || view.ErrorCases[0].Case != "401" || view.ErrorCases[0].Type != "" || + !view.ErrorCases[1].Default || view.ErrorCases[1].Type != "ReadError" || view.ErrorSwitch != "response.StatusCode" { t.Fatalf("unexpected prepared responses: %#v %v", view, err) } view = operationView{} diff --git a/generator/sdk/gosdk/error_response_precedence_test.go b/generator/sdk/gosdk/error_response_precedence_test.go new file mode 100644 index 00000000..c616e8ad --- /dev/null +++ b/generator/sdk/gosdk/error_response_precedence_test.go @@ -0,0 +1,134 @@ +// Copyright 2026 Princess B33f Heavy Industries / Dave Shanley +// SPDX-License-Identifier: MIT + +package gosdk + +import ( + "os" + "os/exec" + "path/filepath" + "testing" + + "github.com/pb33f/libopenapi" + "github.com/pb33f/libopenapi/generator/sdk" +) + +func TestGeneratedErrorResponsesPreserveUntypedOverrides(t *testing.T) { + document, err := libopenapi.NewDocument([]byte(errorResponsePrecedenceSpec)) + if err != nil { + t.Fatal(err) + } + model, err := document.BuildV3Model() + if err != nil { + t.Fatal(err) + } + result, err := Generate(&model.Model, Options{PackageName: "example", Prepare: sdk.PrepareOptions{ + Names: map[string]sdk.OperationName{"getWidgets": {Resource: "Widgets", Method: "Get"}}, + }}) + if err != nil { + t.Fatal(err) + } + directory := t.TempDir() + for _, file := range result.Files { + if err := os.WriteFile(filepath.Join(directory, file.Path), file.Content, 0o600); err != nil { + t.Fatal(err) + } + } + for path, content := range map[string]string{ + "go.mod": "module generated.example/response-precedence\n\ngo 1.26.0\n", + "client_test.go": errorResponsePrecedenceRuntime, + } { + if err := os.WriteFile(filepath.Join(directory, path), []byte(content), 0o600); err != nil { + t.Fatal(err) + } + } + command := exec.Command("go", "test", "./...") + command.Dir = directory + command.Env = append(os.Environ(), "GOWORK=off") + if output, err := command.CombinedOutput(); err != nil { + t.Fatalf("generated error handling failed: %v\n%s", err, output) + } +} + +const errorResponsePrecedenceSpec = `openapi: 3.1.0 +info: {title: Widget service, version: '1'} +paths: + /widgets: + get: + operationId: getWidgets + responses: + '200': {description: OK} + '400': + description: Invalid request + content: + application/json: + schema: {type: string} + '404': + description: Missing widget + content: + text/plain: + schema: {type: string} + '405': {description: Method not allowed} + '4XX': + description: Other client error + content: + application/json: + schema: {type: integer} + '5XX': + description: Server error + content: + text/plain: + schema: {type: string} + '503': + description: Service unavailable + content: + application/json: + schema: {type: boolean} + default: + description: Other error + content: + application/json: + schema: {type: boolean} +` + +const errorResponsePrecedenceRuntime = `package example + +import ( + "context" + "errors" + "io" + "net/http" + "strings" + "testing" +) + +type responseDoer struct { status int; body string } + +func (d responseDoer) Do(*http.Request) (*http.Response, error) { + return &http.Response{StatusCode: d.status, Header: make(http.Header), Body: io.NopCloser(strings.NewReader(d.body))}, nil +} + +func TestErrorResponsePrecedence(t *testing.T) { + for _, test := range []struct { name string; status int; body string; typed bool }{ + {"typed exact over typed range", 400, "\"invalid\"", true}, + {"untyped exact over typed range", 404, "missing", false}, + {"bodyless exact over typed range", 405, "", false}, + {"typed range over default", 418, "42", true}, + {"untyped range over default", 502, "unavailable", false}, + {"typed exact over untyped range", 503, "true", true}, + {"typed default", 302, "false", true}, + } { + t.Run(test.name, func(t *testing.T) { + client, err := NewClient("https://example.com", WithHTTPClient(responseDoer{test.status, test.body})) + if err != nil { t.Fatal(err) } + _, err = client.Widgets.Get(context.Background()) + var apiError *APIError + if !errors.As(err, &apiError) { t.Fatalf("error = %v", err) } + if apiError.StatusCode != test.status || string(apiError.Body) != test.body || apiError.Cause != nil { + t.Fatalf("response evidence = %#v", apiError) + } + if (apiError.Value != nil) != test.typed { t.Fatalf("decoded value = %#v", apiError.Value) } + }) + } +} +` diff --git a/generator/sdk/gosdk/generator.go b/generator/sdk/gosdk/generator.go index e93d3125..e73ab4d3 100644 --- a/generator/sdk/gosdk/generator.go +++ b/generator/sdk/gosdk/generator.go @@ -697,9 +697,8 @@ func (e *emitter) prepareResponses(operation *sdk.Operation, base, method string if _, err := statusCondition([]string{status}); err != nil { return fmt.Errorf("gosdk: operation %q error response %s: %w", operation.ID, status, err) } - if typeName != "" { - errorCases = append(errorCases, errorResponseView{Statuses: []string{status}, Type: typeName}) - } + // Untyped responses still override a typed range or default response. + errorCases = append(errorCases, errorResponseView{Statuses: []string{status}, Type: typeName}) } if len(view.SuccessStatuses) == 0 { return fmt.Errorf("gosdk: operation %q has no declared 2xx response", operation.ID) @@ -720,6 +719,9 @@ func (e *emitter) prepareResponses(operation *sdk.Operation, base, method string view.DecodeAll = view.DecodeCondition == view.SuccessCondition view.ErrorCases, view.ErrorSwitch = errorSwitch(errorCases) for _, errorCase := range view.ErrorCases { + if errorCase.Type == "" { + continue + } statuses := strings.Join(errorCase.Statuses, ", ") if errorCase.Default { statuses = "default" @@ -735,6 +737,7 @@ func (e *emitter) prepareResponses(operation *sdk.Operation, base, method string // ranges the switch is on the status code itself. func errorSwitch(responses []errorResponseView) ([]errorResponseView, string) { var exact, ranges, fallback []errorResponseView + hasTypedResponse := false group := func(cases []errorResponseView, response errorResponseView) []errorResponseView { for index := range cases { if cases[index].Type == response.Type { @@ -745,6 +748,7 @@ func errorSwitch(responses []errorResponseView) ([]errorResponseView, string) { return append(cases, response) } for _, response := range responses { + hasTypedResponse = hasTypedResponse || response.Type != "" switch status := response.Statuses[0]; { case status == "default": fallback = []errorResponseView{{Type: response.Type, Default: true}} @@ -754,6 +758,9 @@ func errorSwitch(responses []errorResponseView) ([]errorResponseView, string) { exact = group(exact, response) } } + if !hasTypedResponse { + return nil, "" + } for _, cases := range [][]errorResponseView{exact, ranges} { for index := range cases { sort.Strings(cases[index].Statuses) diff --git a/generator/sdk/gosdk/templates/resources.tmpl b/generator/sdk/gosdk/templates/resources.tmpl index f0e5d809..3eddb05f 100644 --- a/generator/sdk/gosdk/templates/resources.tmpl +++ b/generator/sdk/gosdk/templates/resources.tmpl @@ -118,7 +118,9 @@ var security{{$resource.Name}}{{.MethodName}} = [][]securityRequirement{ {{- else}} case {{.Case}}: {{- end}} +{{- if .Type}} decodeErrorValue[{{.Type}}](apiError) +{{- end}} {{- end}} } {{- end}} diff --git a/generator/sdk/gosdk/testdata/people_match_resources.golden.go b/generator/sdk/gosdk/testdata/people_match_resources.golden.go index d14f3334..90396d09 100644 --- a/generator/sdk/gosdk/testdata/people_match_resources.golden.go +++ b/generator/sdk/gosdk/testdata/people_match_resources.golden.go @@ -211,6 +211,7 @@ func (resource *PeopleResource) Match(ctx context.Context, input *PeopleMatchPar switch response.StatusCode { case 400: decodeErrorValue[PeopleMatchBadRequest](apiError) + case 401: case 429: decodeErrorValue[PeopleMatchTooManyRequests](apiError) } From 303df324e1e6d276987ae6fa04fdd5030d77d36b Mon Sep 17 00:00:00 2001 From: quobix Date: Sat, 31 Oct 2026 10:55:01 -0400 Subject: [PATCH 02/33] fix(v2): resolve external path items in their source context --- datamodel/high/v2/path_item.go | 1 + datamodel/low/v2/paths.go | 19 ++++++++++- document_external_regression_test.go | 23 +++++++++++++ document_v2_path_reference_test.go | 51 ++++++++++++++++++++++++++++ 4 files changed, 93 insertions(+), 1 deletion(-) create mode 100644 document_external_regression_test.go create mode 100644 document_v2_path_reference_test.go diff --git a/datamodel/high/v2/path_item.go b/datamodel/high/v2/path_item.go index 619dcb34..375ab700 100644 --- a/datamodel/high/v2/path_item.go +++ b/datamodel/high/v2/path_item.go @@ -39,6 +39,7 @@ type PathItem struct { func NewPathItem(pathItem *lowV2.PathItem) *PathItem { p := new(PathItem) p.low = pathItem + p.Ref = pathItem.Ref.Value p.Extensions = high.ExtractExtensions(pathItem.Extensions) if !pathItem.Parameters.IsEmpty() { var params []*Parameter diff --git a/datamodel/low/v2/paths.go b/datamodel/low/v2/paths.go index 7d68b443..c209c33a 100644 --- a/datamodel/low/v2/paths.go +++ b/datamodel/low/v2/paths.go @@ -5,6 +5,7 @@ package v2 import ( "context" + "fmt" "hash/maphash" "sync" @@ -125,12 +126,28 @@ func (p *Paths) Build(ctx context.Context, _, root *yaml.Node, idx *index.SpecIn translateFunc := func(value buildInput) (pathBuildResult, error) { pNode := value.pathNode cNode := value.currentNode + pathCtx, pathIdx := ctx, idx + var originalRef low.NodeReference[string] + if ok, refKey, ref := utils.IsNodeRefValue(pNode); ok { + originalRef = low.NodeReference[string]{Value: ref, KeyNode: refKey, ValueNode: utils.GetRefValueNode(pNode)} + resolved, foundIdx, err, foundCtx := low.LocateRefEnd(ctx, pNode, idx, 0) + if err != nil { + return pathBuildResult{}, fmt.Errorf("path item build failed: %w", err) + } + if resolved == nil { + return pathBuildResult{}, fmt.Errorf("path item build failed: cannot find reference: %q", ref) + } + pNode, pathIdx, pathCtx = resolved, foundIdx, foundCtx + } path := new(PathItem) _ = low.BuildModel(pNode, path) - err := path.Build(ctx, cNode, pNode, idx) + err := path.Build(pathCtx, cNode, pNode, pathIdx) if err != nil { return pathBuildResult{}, err } + if originalRef.Value != "" { + path.Ref = originalRef + } return pathBuildResult{ key: low.KeyReference[string]{ Value: cNode.Value, diff --git a/document_external_regression_test.go b/document_external_regression_test.go new file mode 100644 index 00000000..e08ae955 --- /dev/null +++ b/document_external_regression_test.go @@ -0,0 +1,23 @@ +// Copyright 2026 Dave Shanley / Quobix +// SPDX-License-Identifier: MIT + +package libopenapi + +import ( + "github.com/pb33f/libopenapi/datamodel" + "github.com/pb33f/testify/require" + "os" + "path/filepath" + "testing" +) + +func externalRegressionConfig(t *testing.T, files map[string]string) *datamodel.DocumentConfiguration { + t.Helper() + dir := t.TempDir() + for name, content := range files { + path := filepath.Join(dir, name) + require.NoError(t, os.MkdirAll(filepath.Dir(path), 0755)) + require.NoError(t, os.WriteFile(path, []byte(content), 0600)) + } + return &datamodel.DocumentConfiguration{BasePath: dir, AllowFileReferences: true} +} diff --git a/document_v2_path_reference_test.go b/document_v2_path_reference_test.go new file mode 100644 index 00000000..d239e86e --- /dev/null +++ b/document_v2_path_reference_test.go @@ -0,0 +1,51 @@ +// Copyright 2026 Dave Shanley / Quobix +// SPDX-License-Identifier: MIT + +package libopenapi + +import ( + "github.com/pb33f/testify/require" + "testing" +) + +func TestBuildV2Model_ExternalPathItems(t *testing.T) { + config := externalRegressionConfig(t, map[string]string{ + "paths/list.yaml": "get:\n responses:\n '200': {description: ok}\n", + "paths/item.yaml": "parameters:\n - {$ref: ./id.yaml}\nget:\n responses:\n '200': {description: ok}\n", + "paths/id.yaml": "name: id\nin: path\nrequired: true\ntype: string\n", + }) + doc, err := NewDocumentWithConfiguration([]byte(`swagger: "2.0" +info: {title: External paths, version: "1"} +paths: + /items: + $ref: ./paths/list.yaml + /items/{id}: + $ref: ./paths/item.yaml +`), config) + require.NoError(t, err) + model, err := doc.BuildV2Model() + require.NoError(t, err) + list := model.Model.Paths.PathItems.GetOrZero("/items") + require.NotNil(t, list.Get) + require.Empty(t, list.Parameters) + require.Equal(t, "./paths/list.yaml", list.Ref) + item := model.Model.Paths.PathItems.GetOrZero("/items/{id}") + require.NotNil(t, item.Get) + require.Len(t, item.Parameters, 1) + require.Equal(t, "id", item.Parameters[0].Name) + require.Equal(t, "./paths/item.yaml", item.Ref) +} + +func TestBuildV2Model_MissingExternalPathItem(t *testing.T) { + config := externalRegressionConfig(t, map[string]string{ + "paths.yaml": "paths: {}\n", + }) + doc, err := NewDocumentWithConfiguration([]byte(`swagger: "2.0" +info: {title: Missing path, version: "1"} +paths: + /items: {$ref: './paths.yaml#/paths/missing'} +`), config) + require.NoError(t, err) + _, err = doc.BuildV2Model() + require.Error(t, err) +} From ac4988e625a318eae000279d6247eb7cdcc3d9a1 Mon Sep 17 00:00:00 2001 From: quobix Date: Sat, 31 Oct 2026 10:55:02 -0400 Subject: [PATCH 03/33] fix(schema): follow external alias chains to their target --- datamodel/low/base/schema_build.go | 2 +- document_schema_alias_test.go | 41 ++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) create mode 100644 document_schema_alias_test.go diff --git a/datamodel/low/base/schema_build.go b/datamodel/low/base/schema_build.go index a414f25f..dcbd2a9e 100644 --- a/datamodel/low/base/schema_build.go +++ b/datamodel/low/base/schema_build.go @@ -68,7 +68,7 @@ func (s *Schema) Build(ctx context.Context, root *yaml.Node, idx *index.SpecInde if !isTransformed { if h, _, _ := utils.IsNodeRefValue(root); h { - ref, foundIdx, err, fctx := low.LocateRefNodeWithContext(ctx, root, idx) + ref, foundIdx, err, fctx := low.LocateRefEnd(ctx, root, idx, 0) if ref != nil { root = utils.ResolveMergeNodes(ref) if fctx != nil { diff --git a/document_schema_alias_test.go b/document_schema_alias_test.go new file mode 100644 index 00000000..f616e0c8 --- /dev/null +++ b/document_schema_alias_test.go @@ -0,0 +1,41 @@ +// Copyright 2026 Dave Shanley / Quobix +// SPDX-License-Identifier: MIT + +package libopenapi + +import ( + "fmt" + "github.com/pb33f/testify/require" + "testing" +) + +func TestBuildV3Model_ExternalSchemaAliasChain(t *testing.T) { + for _, aliases := range []int{0, 1, 2, 5} { + t.Run(fmt.Sprint(aliases), func(t *testing.T) { + files := map[string]string{"schemas/target.yaml": "type: object\nproperties:\n id: {type: string}\n"} + ref := "./target.yaml" + for i := aliases; i > 0; i-- { + name := fmt.Sprintf("alias%d.yaml", i) + files["schemas/"+name] = "$ref: " + ref + "\n" + ref = "./" + name + } + config := externalRegressionConfig(t, files) + doc, err := NewDocumentWithConfiguration([]byte(fmt.Sprintf(`openapi: 3.1.0 +info: {title: Alias chains, version: "1"} +paths: {} +components: + schemas: + Item: {$ref: ./schemas/%s} +`, ref[2:])), config) + require.NoError(t, err) + model, err := doc.BuildV3Model() + require.NoError(t, err) + proxy := model.Model.Components.Schemas.GetOrZero("Item") + schema := proxy.Schema() + require.NoError(t, proxy.GetBuildError()) + require.NotNil(t, schema) + require.Equal(t, []string{"object"}, schema.Type) + require.NotNil(t, schema.Properties.GetOrZero("id")) + }) + } +} From 7c70f25e3905376c550b14b270870208a32712b8 Mon Sep 17 00:00:00 2001 From: quobix Date: Sat, 31 Oct 2026 10:55:31 -0400 Subject: [PATCH 04/33] fix(index): preserve required alias targets and circular journeys --- document_resolver_regression_test.go | 113 ++++++++++++++++++++++++++ index/resolver_circular.go | 6 +- index/resolver_polymorphic.go | 2 +- index/resolver_relatives.go | 19 ++++- index/resolver_required_alias_test.go | 40 +++++++++ index/resolver_walk.go | 1 + index/utility_methods.go | 15 ++-- 7 files changed, 186 insertions(+), 10 deletions(-) create mode 100644 document_resolver_regression_test.go create mode 100644 index/resolver_required_alias_test.go diff --git a/document_resolver_regression_test.go b/document_resolver_regression_test.go new file mode 100644 index 00000000..713ac33d --- /dev/null +++ b/document_resolver_regression_test.go @@ -0,0 +1,113 @@ +// Copyright 2026 Dave Shanley / Quobix +// SPDX-License-Identifier: MIT + +package libopenapi + +import ( + "fmt" + "github.com/pb33f/testify/require" + "testing" +) + +func TestBuildV3Model_RecursiveRequiredAlias(t *testing.T) { + config := externalRegressionConfig(t, map[string]string{ + "schemas/id.yaml": "$ref: ./uuid.yaml\n", + "schemas/uuid.yaml": "type: string\nformat: uuid\n", + }) + doc, err := NewDocumentWithConfiguration([]byte(`openapi: 3.1.0 +info: {title: Recursive required alias, version: "1"} +paths: {} +components: + schemas: + Item: + type: object + required: [id] + properties: + id: {$ref: ./schemas/id.yaml} + children: + type: array + items: {$ref: '#/components/schemas/Item'} +`), config) + require.NoError(t, err) + model, err := doc.BuildV3Model() + require.NoError(t, err) + item := model.Model.Components.Schemas.GetOrZero("Item").Schema() + require.NotNil(t, item) + require.Equal(t, "uuid", item.Properties.GetOrZero("id").Schema().Format) +} + +func TestBuildV3Model_ReferencedExamplePayload(t *testing.T) { + doc, err := NewDocument([]byte(`openapi: 3.1.0 +info: {title: Example payload, version: "1"} +paths: + /items: + get: + responses: + '200': + description: ok + content: + application/json: + schema: {$ref: '#/components/schemas/Item'} +components: + schemas: + Item: + type: object + properties: + schema: {type: object} + example: + schema: + properties: + id: {$ref: '#/$defs/id'} + $defs: + id: {type: string} +`)) + require.NoError(t, err) + model, err := doc.BuildV3Model() + require.NoError(t, err) + require.NotNil(t, model) +} + +func TestBuildV3Model_ReferencedExampleObjects(t *testing.T) { + for _, version := range []string{"3.0.3", "3.1.0"} { + for _, field := range []string{"value", "dataValue"} { + t.Run(version+"/"+field, func(t *testing.T) { + response := fmt.Sprintf(`description: ok +content: + application/json: + schema: + type: object + properties: + example: {$ref: ./string.yaml} + examples: {$ref: ./string.yaml} + examples: + payload: + %s: + $ref: '#/payload-only' + nested: {$ref: '#/also-payload-only'} +`, field) + config := externalRegressionConfig(t, map[string]string{ + "responses/ok.yaml": response, + "responses/string.yaml": "type: string\n", + }) + root := fmt.Sprintf(`openapi: %s +info: {title: External example payload, version: "1"} +paths: + /items: + get: + responses: + '200': {$ref: ./responses/ok.yaml} +`, version) + doc, err := NewDocumentWithConfiguration([]byte(root), config) + require.NoError(t, err) + model, err := doc.BuildV3Model() + require.NoError(t, err) + media := model.Model.Paths.PathItems.GetOrZero("/items").Get.Responses.Codes.GetOrZero("200").Content.GetOrZero("application/json") + for _, name := range []string{"example", "examples"} { + proxy := media.Schema.Schema().Properties.GetOrZero(name) + require.Equal(t, []string{"string"}, proxy.Schema().Type) + require.NoError(t, proxy.GetBuildError()) + } + }) + } + } +} diff --git a/index/resolver_circular.go b/index/resolver_circular.go index 836f4f42..72c7a227 100644 --- a/index/resolver_circular.go +++ b/index/resolver_circular.go @@ -30,7 +30,8 @@ func (resolver *Resolver) buildCircularReferenceResult( journey []*Reference, loopIndex int, ) *CircularReferenceResult { - loop := append(journey, foundDup) + // Keep the recorded journey independent of later sibling traversal. + loop := append(journey[:len(journey):len(journey)], foundDup) visitedDefinitions := make(map[string]bool) isInfiniteLoop, _ := resolver.isInfiniteCircularDependency(foundDup, visitedDefinitions, nil) @@ -99,6 +100,9 @@ func (resolver *Resolver) isInfiniteCircularDependency( } for refDefinition := range ref.RequiredRefProperties { r, _ := resolver.specIndex.SearchIndexForReference(refDefinition) + if r == nil { + continue // Unresolved targets are reported by reference resolution. + } // Direct loop back to the original starting reference — infinite cycle. if initialRef != nil && initialRef.FullDefinition == r.FullDefinition { diff --git a/index/resolver_polymorphic.go b/index/resolver_polymorphic.go index dc5adbad..e7e3dd3f 100644 --- a/index/resolver_polymorphic.go +++ b/index/resolver_polymorphic.go @@ -73,7 +73,7 @@ func (resolver *Resolver) visitPolymorphicReference( return } - loop := append(state.journey, mappedRefs) + loop := append(state.journey[:len(state.journey):len(state.journey)], mappedRefs) circRef := &CircularReferenceResult{ ParentNode: parentNode, Journey: loop, diff --git a/index/resolver_relatives.go b/index/resolver_relatives.go index bf0dd0f5..a02635e3 100644 --- a/index/resolver_relatives.go +++ b/index/resolver_relatives.go @@ -24,6 +24,12 @@ func (resolver *Resolver) extractRelatives( schemaIDBase string, ) []*Reference { state := newRelativeWalkState(foundRelatives, journey, resolve, depth, schemaIDBase) + if ref != nil { + state.path = ref.SourcePath + if strings.HasPrefix(ref.Definition, "#/") { + state.path = strings.Split(strings.TrimPrefix(ref.Definition, "#/"), "/") + } + } return resolver.extractRelativesWithState(ref, node, parent, state) } @@ -32,6 +38,9 @@ func (resolver *Resolver) extractRelativesWithState( node, parent *yaml.Node, state relativeWalkState, ) []*Reference { + if underOpenAPIExamplePayloadPath(state.path) { + return nil + } // Guard against stack overflow from deeply nested or circular specs. // Journey tracks the reference chain (100 max); depth tracks recursive calls (500 max). if len(state.journey) > 100 { @@ -53,7 +62,13 @@ func (resolver *Resolver) extractRelativesWithState( } if utils.IsNodeMap(n) || utils.IsNodeArray(n) { - found = append(found, resolver.extractNestedRelatives(ref, node, n, state)...) + childState := state + if node.Kind == yaml.MappingNode && i%2 == 1 { + childState.path = append(state.path, node.Content[i-1].Value) + } else if node.Kind == yaml.SequenceNode { + childState.path = append(state.path, "[]") + } + found = append(found, resolver.extractNestedRelatives(ref, node, n, childState)...) } if i%2 == 0 && n.Value == "$ref" && len(node.Content) > i%2+1 { @@ -88,7 +103,7 @@ func (resolver *Resolver) handleRelativeDepthLimit(ref *Reference, state relativ "reference", def) } - loop := append(state.journey, ref) + loop := append(state.journey[:len(state.journey):len(state.journey)], ref) circRef := &CircularReferenceResult{ Journey: loop, Start: ref, diff --git a/index/resolver_required_alias_test.go b/index/resolver_required_alias_test.go new file mode 100644 index 00000000..bfaec693 --- /dev/null +++ b/index/resolver_required_alias_test.go @@ -0,0 +1,40 @@ +package index + +import ( + "testing" + + "github.com/pb33f/go-yaml" + "github.com/pb33f/testify/require" +) + +func TestResolver_RequiredReferenceMissingTarget(t *testing.T) { + var node yaml.Node + require.NoError(t, yaml.Unmarshal([]byte("type: object\n"), &node)) + idx := NewSpecIndex(&node) + resolver := NewResolver(idx) + infinite, _ := resolver.isInfiniteCircularDependency(&Reference{ + FullDefinition: "#/Item", + RequiredRefProperties: map[string][]string{"#/Missing": {"id"}}, + }, make(map[string]bool), nil) + require.False(t, infinite) +} + +func TestResolver_CircularJourneySnapshot(t *testing.T) { + original := &Reference{FullDefinition: "#/Loop"} + journey := make([]*Reference, 1, 8) + journey[0] = original + resolver := &Resolver{} + result := resolver.buildCircularReferenceResult(original, original, journey, 0) + journey[0] = &Reference{FullDefinition: "#/Sibling"} + journey = append(journey, &Reference{FullDefinition: "#/Child"}) + require.Equal(t, []*Reference{original, original}, result.Journey) +} + +func TestRequiredAliasPreservesLocalFragment(t *testing.T) { + var node yaml.Node + require.NoError(t, yaml.Unmarshal([]byte("$ref: '#/$defs/ID'\n$defs:\n ID: {type: string}\n"), &node)) + for _, location := range []string{"/spec/schemas/alias.yaml", "https://example.com/schemas/alias.yaml"} { + targets := extractDefinitionRequiredRefProperties(node.Content[0], map[string][]string{}, location, nil) + require.Contains(t, targets, location+"#/$defs/ID") + } +} diff --git a/index/resolver_walk.go b/index/resolver_walk.go index a573ec57..a2db964d 100644 --- a/index/resolver_walk.go +++ b/index/resolver_walk.go @@ -11,6 +11,7 @@ type relativeWalkState struct { resolve bool depth int schemaIDBase string + path []string } func newRelativeWalkState( diff --git a/index/utility_methods.go b/index/utility_methods.go index 19360ebf..b6c1a1de 100644 --- a/index/utility_methods.go +++ b/index/utility_methods.go @@ -53,11 +53,9 @@ func extractDefinitionRequiredRefProperties(schemaNode *yaml.Node, reqRefProps m } // If the node we're looking at is a direct ref to another model without any properties, mark it as required, but still continue to look for required properties - isRef, _, defPath := utils.IsNodeRefValue(schemaNode) + isRef, _, _ := utils.IsNodeRefValue(schemaNode) if isRef { - if _, ok := reqRefProps[defPath]; !ok { - reqRefProps[defPath] = []string{} - } + reqRefProps = extractRequiredReferenceProperties(fulldef, schemaNode, "", reqRefProps) } // Check for a required parameters list, and return if none exists, as any properties will be optional @@ -125,7 +123,10 @@ func extractRequiredReferenceProperties(fulldef string, requiredPropDefNode *yam defPath := fulldef - if strings.HasPrefix(refName, "http") || filepath.IsAbs(refName) { + if strings.HasPrefix(refName, "#") { + base, _ := SplitRefFragment(fulldef) + defPath = base + refName + } else if strings.HasPrefix(refName, "http") || filepath.IsAbs(refName) { defPath = refName } else { exp := strings.Split(fulldef, "#/") @@ -199,7 +200,9 @@ func extractRequiredReferenceProperties(fulldef string, requiredPropDefNode *yam if _, ok := reqRefProps[defPath]; !ok { reqRefProps[defPath] = []string{} } - reqRefProps[defPath] = append(reqRefProps[defPath], propName) + if propName != "" { + reqRefProps[defPath] = append(reqRefProps[defPath], propName) + } return reqRefProps } From a3b787bb4d9a78e17707ea3a2696af8cb05620ad Mon Sep 17 00:00:00 2001 From: Joseph Axisa Date: Tue, 20 Oct 2026 14:46:17 -0700 Subject: [PATCH 05/33] fix: compare the schema behind a changed $ref When a schema $ref was replaced by an inline schema, an inline schema by a $ref, or a $ref pointed somewhere else, CompareSchemas recorded a non-breaking $ref change and returned, so changes in the new schema were never reported. A $ref to a schema with type: int becoming an inline type: string was a single non-breaking change. Keep the $ref change, then compare the two schemas as it would compare two inline schemas. It still stops at the $ref change, now keeping that change instead of returning nil, when either side doesn't resolve, is on a reference loop, or comes from a document that was not checked for circular references (a loop can't be recognised there), and when a 3.1 $ref with sibling keywords is involved, since its internal allOf form doesn't line up with the referenced schema field by field. Rolodex gains HasCheckedForCircularReferences for that check. The helpers that recognise equivalent forms now look through a reference, so a change made through a $ref is reported like the same inline change: the object composition handling from #650 ($ref: Thing becoming anyOf: [$ref: Thing, type: 'null']), the simple allOf view, and a type array matching an anyOf of scalar types. Fixes #663 --- index/rolodex.go | 5 + index/rolodex_test.go | 14 + what-changed/model/schema.go | 77 ++-- what-changed/model/schema_composition_wrap.go | 5 +- .../model/schema_composition_wrap_test.go | 59 +++ what-changed/model/schema_test.go | 423 +++++++++++++++++- what-changed/what_changed_test.go | 9 +- 7 files changed, 548 insertions(+), 44 deletions(-) diff --git a/index/rolodex.go b/index/rolodex.go index 335427b1..632a2bb9 100644 --- a/index/rolodex.go +++ b/index/rolodex.go @@ -247,6 +247,11 @@ func (r *Rolodex) GetIndexingDuration() time.Duration { return r.indexingDuration } +// HasCheckedForCircularReferences returns true once the rolodex has been checked for circular references. +func (r *Rolodex) HasCheckedForCircularReferences() bool { + return r.circChecked +} + // GetRootIndex returns the root index of the rolodex (the entry point, the main document) func (r *Rolodex) GetRootIndex() *SpecIndex { return r.rootIndex diff --git a/index/rolodex_test.go b/index/rolodex_test.go index 8eb3eb3f..85450412 100644 --- a/index/rolodex_test.go +++ b/index/rolodex_test.go @@ -3049,3 +3049,17 @@ func TestRolodex_Open_HttpPrefixedLocationWithoutScheme(t *testing.T) { assert.Nil(t, rf) assert.EqualError(t, err, "remote URL 'httpdocs/pet.yaml' has no scheme, unable to fetch it") } + +func TestRolodex_HasCheckedForCircularReferences(t *testing.T) { + var rootNode yaml.Node + _ = yaml.Unmarshal([]byte("openapi: 3.1.0"), &rootNode) + config := CreateOpenAPIIndexConfig() + config.AvoidCircularReferenceCheck = true + rolo := NewRolodex(config) + rolo.SetRootNode(&rootNode) + assert.NoError(t, rolo.IndexTheRolodex(context.Background())) + assert.False(t, rolo.HasCheckedForCircularReferences()) + + rolo.CheckForCircularReferences() + assert.True(t, rolo.HasCheckedForCircularReferences()) +} diff --git a/what-changed/model/schema.go b/what-changed/model/schema.go index 5ff42aa6..317815db 100644 --- a/what-changed/model/schema.go +++ b/what-changed/model/schema.go @@ -16,6 +16,7 @@ import ( "github.com/pb33f/libopenapi/datamodel/low/base" v3 "github.com/pb33f/libopenapi/datamodel/low/v3" "github.com/pb33f/libopenapi/orderedmap" + "github.com/pb33f/libopenapi/utils" ) // SchemaChanges represent all changes to a base.Schema OpenAPI object. These changes are represented @@ -396,6 +397,24 @@ func (s *SchemaChanges) TotalBreakingChanges() int { return t } +// canCompareBehindRefChange reports whether the schemas either side of a changed $ref can be compared like any +// other two schemas. Both must resolve, and neither may be on a reference loop, which can only be known once the +// document has been checked for circular references. A 3.1 $ref with sibling keywords is held in an internal allOf +// form that doesn't line up with the referenced schema field by field, so it stops at the $ref change too. +func canCompareBehindRefChange(l, r *base.SchemaProxy) bool { + for _, proxy := range []*base.SchemaProxy{l, r} { + if proxy.Schema() == nil || proxy.IsTransformedRefWithSiblings() { + return false + } + idx := proxy.GetIndex() + if idx == nil || idx.GetRolodex() == nil || !idx.GetRolodex().HasCheckedForCircularReferences() || + base.CheckSchemaProxyForCircularRefs(proxy) { + return false + } + } + return true +} + // CompareSchemas accepts a left and right SchemaProxy and checks for changes. If anything is found, returns // a pointer to SchemaChanges, otherwise returns nil func CompareSchemas(l, r *base.SchemaProxy) *SchemaChanges { @@ -439,18 +458,15 @@ func CompareSchemas(l, r *base.SchemaProxy) *SchemaChanges { // continue on because the external references are the same and we need to check things going forward. } else { - // references are different, that's all we care to know. + // the reference points somewhere else: record that, then compare the schema it now points to. CreateChange(&changes, Modified, v3.RefLabel, l.GetValueNode().Content[1], r.GetValueNode().Content[1], BreakingModified(CompSchema, PropRef), l.GetReference(), r.GetReference()) - sc.PropertyChanges = NewPropertyChanges(changes) - // check if this is a circular ref. - if base.CheckSchemaProxyForCircularRefs(l) || base.CheckSchemaProxyForCircularRefs(r) { - // if we have a circular reference, we can't do any more work here. - return nil + if !canCompareBehindRefChange(l, r) || low.AreEqual(l.Schema(), r.Schema()) { + sc.PropertyChanges = NewPropertyChanges(changes) + return sc } - return sc } } @@ -463,14 +479,11 @@ func CompareSchemas(l, r *base.SchemaProxy) *SchemaChanges { if lHash != rHash { CreateChange(&changes, Modified, v3.RefLabel, l.GetValueNode(), r.GetValueNode().Content[1], BreakingModified(CompSchema, PropRef), l, r.GetReference()) - sc.PropertyChanges = NewPropertyChanges(changes) - // check if this is a circular ref. - if base.CheckSchemaProxyForCircularRefs(r) { - // if we have a circular reference, we can't do any more work here. - return nil + if !canCompareBehindRefChange(l, r) { + sc.PropertyChanges = NewPropertyChanges(changes) + return sc } - return sc } } @@ -483,14 +496,11 @@ func CompareSchemas(l, r *base.SchemaProxy) *SchemaChanges { if lHash != rHash { CreateChange(&changes, Modified, v3.RefLabel, l.GetValueNode().Content[1], r.GetValueNode(), BreakingModified(CompSchema, PropRef), l.GetReference(), r) - sc.PropertyChanges = NewPropertyChanges(changes) - // check if this is a circular ref. - if base.CheckSchemaProxyForCircularRefs(l) { - // if we have a circular reference, we can't do any more work here. - return nil + if !canCompareBehindRefChange(l, r) { + sc.PropertyChanges = NewPropertyChanges(changes) + return sc } - return sc } } @@ -1804,7 +1814,7 @@ func schemaComparisonViewForSimpleAllOfObject(proxy *base.SchemaProxy, schema *b } func isSimpleAllOfObjectSchema(proxy *base.SchemaProxy, schema *base.Schema) bool { - if proxy == nil || schema == nil || proxy.IsReference() || len(schema.AllOf.Value) == 0 { + if proxy == nil || schema == nil || len(schema.AllOf.Value) == 0 { return false } if len(schema.OneOf.Value) > 0 || len(schema.AnyOf.Value) > 0 || len(schema.PrefixItems.Value) > 0 || schema.Not.Value != nil { @@ -2049,21 +2059,37 @@ func schemaPairUsesEquivalentSimpleScalarUnion(typeProxy, anyOfProxy *base.Schem } func isPureTypeArraySchema(proxy *base.SchemaProxy) bool { - if proxy == nil || proxy.IsReference() || !schemaNodeHasSingleKey(proxy.GetValueNode(), v3.TypeLabel) { + node := schemaDefinitionNode(proxy) + if !schemaNodeHasSingleKey(node, v3.TypeLabel) { return false } - typeNode := proxy.GetValueNode().Content[1] + typeNode := node.Content[1] return typeNode.Kind == yaml.SequenceNode && len(typeNode.Content) > 0 } func isPureAnyOfUnionSchema(proxy *base.SchemaProxy) bool { - if proxy == nil || proxy.IsReference() || !schemaNodeHasSingleKey(proxy.GetValueNode(), v3.AnyOfLabel) { + if !schemaNodeHasSingleKey(schemaDefinitionNode(proxy), v3.AnyOfLabel) { return false } schema := proxy.Schema() return schema != nil && len(schema.AnyOf.Value) > 0 } +// schemaDefinitionNode returns the node a schema is defined by, following a reference to the schema it points to. +func schemaDefinitionNode(proxy *base.SchemaProxy) *yaml.Node { + if proxy == nil { + return nil + } + node := proxy.GetValueNode() + if isRef, _, _ := utils.IsNodeRefValue(node); isRef { + if proxy.GetIndex() == nil { + return nil + } + node, _, _, _ = low.LocateRefNodeWithContext(proxy.GetContext(), node, proxy.GetIndex()) + } + return node +} + func schemaNodeHasSingleKey(node *yaml.Node, key string) bool { return node != nil && node.Kind == yaml.MappingNode && len(node.Content) == 2 && node.Content[0].Value == key } @@ -2081,10 +2107,11 @@ func schemaNodeHasOnlyAllowedKeys(node *yaml.Node, allowed map[string]struct{}) } func extractTypeArraySet(proxy *base.SchemaProxy) (map[string]struct{}, bool) { - if proxy == nil || proxy.IsReference() || !schemaNodeHasSingleKey(proxy.GetValueNode(), v3.TypeLabel) { + node := schemaDefinitionNode(proxy) + if !schemaNodeHasSingleKey(node, v3.TypeLabel) { return nil, false } - typeNode := proxy.GetValueNode().Content[1] + typeNode := node.Content[1] if typeNode.Kind != yaml.SequenceNode || len(typeNode.Content) == 0 { return nil, false } diff --git a/what-changed/model/schema_composition_wrap.go b/what-changed/model/schema_composition_wrap.go index 00d6e7b5..b847c194 100644 --- a/what-changed/model/schema_composition_wrap.go +++ b/what-changed/model/schema_composition_wrap.go @@ -10,9 +10,10 @@ import ( ) // preservedObjectBranch returns a comparison view for an object moved into a -// composition. Uncertain wrappers retain the normal structural comparison. +// composition. Uncertain wrappers retain the normal structural comparison. The +// old object may be a reference, which is compared as the schema it resolves to. func preservedObjectBranch(l, r *base.SchemaProxy, old, wrapper *base.Schema) *base.Schema { - if l.IsReference() || r.IsReference() || old == nil || wrapper == nil || + if r.IsReference() || old == nil || wrapper == nil || old.Properties.Value == nil || old.Properties.Value.Len() == 0 || hasObjectComposition(old) { return nil } diff --git a/what-changed/model/schema_composition_wrap_test.go b/what-changed/model/schema_composition_wrap_test.go index ac08df46..2dd3639c 100644 --- a/what-changed/model/schema_composition_wrap_test.go +++ b/what-changed/model/schema_composition_wrap_test.go @@ -10,6 +10,7 @@ import ( "github.com/pb33f/go-yaml" "github.com/pb33f/libopenapi/datamodel" "github.com/pb33f/libopenapi/datamodel/low" + "github.com/pb33f/libopenapi/datamodel/low/base" "github.com/pb33f/libopenapi/datamodel/low/v3" "github.com/pb33f/testify/assert" "github.com/pb33f/testify/require" @@ -498,3 +499,61 @@ components: CompareSchemas(lSchemaProxy, rSchemaProxy) }) } + +// https://github.com/pb33f/libopenapi/issues/663 +// A referenced object moved into a nullable composition is reported like the same inline refactor, plus the +// non-breaking $ref change. +func TestCompareSchemas_ReferencedObjectWrappedMatchesInline(t *testing.T) { + left := `openapi: 3.1.0 +components: + schemas: + Thing: + type: object + required: [id] + properties: + id: + type: string + Entry: + $ref: '#/components/schemas/Thing' + Inline: + type: object + required: [id] + properties: + id: + type: string` + + right := `openapi: 3.1.0 +components: + schemas: + Thing: + type: object + required: [id] + properties: + id: + type: string + Entry: + anyOf: + - $ref: '#/components/schemas/Thing' + - type: 'null' + Inline: + anyOf: + - type: object + required: [id] + properties: + id: + type: string + - type: 'null'` + + leftDoc, rightDoc := test_BuildDoc(left, right) + find := func(doc *v3.Document, name string) *base.SchemaProxy { + return doc.Components.Value.FindSchema(name).Value + } + + changes := CompareSchemas(find(leftDoc, "Entry"), find(rightDoc, "Entry")) + inline := CompareSchemas(find(leftDoc, "Inline"), find(rightDoc, "Inline")) + require.NotNil(t, changes) + require.NotNil(t, inline) + assert.Zero(t, changes.TotalBreakingChanges()) + assert.Equal(t, inline.TotalChanges()+1, changes.TotalChanges()) + assert.Equal(t, changesOtherThanRef(inline), changesOtherThanRef(changes)) +} diff --git a/what-changed/model/schema_test.go b/what-changed/model/schema_test.go index f61968f9..2db0dbf4 100644 --- a/what-changed/model/schema_test.go +++ b/what-changed/model/schema_test.go @@ -7,6 +7,7 @@ import ( "context" "fmt" "reflect" + "sort" "strings" "sync" "sync/atomic" @@ -24,6 +25,7 @@ import ( "github.com/pb33f/libopenapi/index" "github.com/pb33f/libopenapi/orderedmap" "github.com/pb33f/testify/assert" + "github.com/pb33f/testify/require" ) // These tests require full documents to be tested properly. schemas are perhaps the most complex @@ -336,10 +338,12 @@ components: changes := CompareSchemas(lSchemaProxy, rSchemaProxy) assert.NotNil(t, changes) - assert.Len(t, changes.Changes, 1) - assert.Len(t, changes.GetAllChanges(), 1) + assert.Len(t, changes.Changes, 2) + assert.Len(t, changes.GetAllChanges(), 2) assert.Equal(t, Modified, changes.Changes[0].ChangeType) assert.Equal(t, "#/components/schemas/Yo", changes.Changes[0].New) + assert.Equal(t, v3.TypeLabel, changes.Changes[1].Property) + assert.True(t, changes.Changes[1].Breaking) // the schema behind the reference is compared too. } func TestCompareSchemas_RefToInline(t *testing.T) { @@ -369,11 +373,13 @@ components: changes := CompareSchemas(lSchemaProxy, rSchemaProxy) assert.NotNil(t, changes) - assert.Len(t, changes.Changes, 1) - assert.Len(t, changes.GetAllChanges(), 1) + assert.Len(t, changes.Changes, 2) + assert.Len(t, changes.GetAllChanges(), 2) assert.Equal(t, Modified, changes.Changes[0].ChangeType) assert.Equal(t, v3.RefLabel, changes.Changes[0].Property) assert.Equal(t, "#/components/schemas/Yo", changes.Changes[0].Original) + assert.Equal(t, v3.TypeLabel, changes.Changes[1].Property) + assert.True(t, changes.Changes[1].Breaking) // the schema behind the reference is compared too. } func TestCompareSchemas_InlineToRef(t *testing.T) { @@ -403,11 +409,13 @@ components: changes := CompareSchemas(rSchemaProxy, lSchemaProxy) assert.NotNil(t, changes) - assert.Len(t, changes.Changes, 1) - assert.Len(t, changes.GetAllChanges(), 1) + assert.Len(t, changes.Changes, 2) + assert.Len(t, changes.GetAllChanges(), 2) assert.Equal(t, Modified, changes.Changes[0].ChangeType) assert.Equal(t, v3.RefLabel, changes.Changes[0].Property) assert.Equal(t, "#/components/schemas/Yo", changes.Changes[0].New) + assert.Equal(t, v3.TypeLabel, changes.Changes[1].Property) + assert.True(t, changes.Changes[1].Breaking) // the schema behind the reference is compared too. } func TestCompareSchemas_Identical(t *testing.T) { @@ -3452,7 +3460,8 @@ components: changes := CompareSchemas(lSchemaProxy, rSchemaProxy) assert.NotNil(t, changes) - assert.Equal(t, 1, changes.TotalChanges()) + assert.Equal(t, 2, changes.TotalChanges()) // the $ref change, and int becoming string behind it. + assert.Equal(t, 1, changes.TotalBreakingChanges()) } // https://github.com/pb33f/openapi-changes/issues/104 @@ -5405,7 +5414,9 @@ components: rSchemaProxy := rightDoc.Components.Value.FindSchema("OK").Value changes := CompareSchemas(lSchemaProxy, rSchemaProxy) - assert.Nil(t, changes) + // the reference is circular, so it isn't followed, but the $ref change itself is kept. + assert.Len(t, changes.GetAllChanges(), 1) + assert.Equal(t, v3.RefLabel, changes.Changes[0].Property) } func TestCompareSchemas_CheckRefChangeCircular_HackIndex(t *testing.T) { @@ -5472,15 +5483,15 @@ components: //rSchemaProxy.GetIndex().SetAbsolutePath("/something/else.yaml") changes := CompareSchemas(lSchemaProxy, rSchemaProxy) - assert.Len(t, changes.GetAllChanges(), 1) + assert.Len(t, changes.GetAllChanges(), 2) rSchemaProxy.GetIndex().SetAbsolutePath("/something/else.yaml") changes = CompareSchemas(lSchemaProxy, rSchemaProxy) - assert.Len(t, changes.GetAllChanges(), 1) + assert.Len(t, changes.GetAllChanges(), 2) rSchemaProxy.GetIndex().SetAbsolutePath("") changes = CompareSchemas(rSchemaProxy, lSchemaProxy) // flip them back - assert.Len(t, changes.GetAllChanges(), 1) + assert.Len(t, changes.GetAllChanges(), 2) } @@ -5512,7 +5523,9 @@ components: //rSchemaProxy.GetIndex().SetAbsolutePath("/something/else.yaml") changes := CompareSchemas(lSchemaProxy, rSchemaProxy) - assert.Nil(t, changes) + // the reference is circular, so it isn't followed, but the $ref change itself is kept. + assert.Len(t, changes.GetAllChanges(), 1) + assert.Equal(t, v3.RefLabel, changes.Changes[0].Property) } func TestCompareSchemas_CheckRefChange_HackIndex_RightToLeft(t *testing.T) { @@ -5543,11 +5556,11 @@ components: //rSchemaProxy.GetIndex().SetAbsolutePath("/something/else.yaml") changes := CompareSchemas(lSchemaProxy, rSchemaProxy) - assert.Len(t, changes.GetAllChanges(), 1) + assert.Len(t, changes.GetAllChanges(), 2) rSchemaProxy.GetIndex().SetAbsolutePath("/something/else.yaml") changes = CompareSchemas(lSchemaProxy, rSchemaProxy) - assert.Len(t, changes.GetAllChanges(), 1) + assert.Len(t, changes.GetAllChanges(), 2) } func TestCompareSchemas_CheckRefChangeCircular_HackIndex_RightToLeft(t *testing.T) { @@ -5578,7 +5591,9 @@ components: //rSchemaProxy.GetIndex().SetAbsolutePath("/something/else.yaml") changes := CompareSchemas(lSchemaProxy, rSchemaProxy) - assert.Nil(t, changes) + // the reference is circular, so it isn't followed, but the $ref change itself is kept. + assert.Len(t, changes.GetAllChanges(), 1) + assert.Equal(t, v3.RefLabel, changes.Changes[0].Property) } @@ -7423,6 +7438,384 @@ components: assert.Equal(t, 1, changes.TotalBreakingChanges()) } +// changesOtherThanRef summarises every change except the $ref change, for comparing a change made behind a +// reference with the same change made inline. +func changesOtherThanRef(changes *SchemaChanges) []string { + var summary []string + for _, c := range changes.GetAllChanges() { + if c.Property != v3.RefLabel { + summary = append(summary, fmt.Sprintf("%s %d %v", c.Property, c.ChangeType, c.Breaking)) + } + } + sort.Strings(summary) + return summary +} + +// https://github.com/pb33f/libopenapi/issues/663 +func TestCompareSchemas_RefToInline_RequiredAdded(t *testing.T) { + low.ClearHashCache() + left := `openapi: 3.1.0 +components: + schemas: + Pet: + type: object + properties: + name: + type: string + OK: + $ref: '#/components/schemas/Pet' + Inline: + type: object + properties: + name: + type: string` + + right := `openapi: 3.1.0 +components: + schemas: + Pet: + type: object + properties: + name: + type: string + OK: + type: object + properties: + name: + type: string + required: [name] + Inline: + type: object + properties: + name: + type: string + required: [name]` + + leftDoc, rightDoc := test_BuildDoc(left, right) + find := func(doc *v3.Document, name string) *base.SchemaProxy { + return doc.Components.Value.FindSchema(name).Value + } + + changes := CompareSchemas(find(leftDoc, "OK"), find(rightDoc, "OK")) + assert.NotNil(t, changes) + assert.Equal(t, 2, changes.TotalChanges()) + assert.Equal(t, 1, changes.TotalBreakingChanges()) + assert.Equal(t, v3.RefLabel, changes.Changes[0].Property) + assert.False(t, changes.Changes[0].Breaking) + + // reported the way the same change is reported between two inline schemas. + inline := CompareSchemas(find(leftDoc, "Inline"), find(rightDoc, "Inline")) + assert.Equal(t, changesOtherThanRef(inline), changesOtherThanRef(changes)) + + // and in the other direction. + changes = CompareSchemas(find(rightDoc, "OK"), find(leftDoc, "OK")) + inline = CompareSchemas(find(rightDoc, "Inline"), find(leftDoc, "Inline")) + assert.Equal(t, 2, changes.TotalChanges()) + assert.Equal(t, changesOtherThanRef(inline), changesOtherThanRef(changes)) +} + +// https://github.com/pb33f/libopenapi/issues/663 +func TestCompareSchemas_RefRetargeted_RequiredAdded(t *testing.T) { + low.ClearHashCache() + spec := `openapi: 3.1.0 +components: + schemas: + Pet: + type: object + properties: + name: + type: string + NewPet: + type: object + properties: + name: + type: string + required: [name] + OK: + $ref: '#/components/schemas/Pet'` + + leftDoc, rightDoc := test_BuildDoc(spec, strings.Replace(spec, "$ref: '#/components/schemas/Pet'", "$ref: '#/components/schemas/NewPet'", 1)) + + changes := CompareSchemas(leftDoc.Components.Value.FindSchema("OK").Value, rightDoc.Components.Value.FindSchema("OK").Value) + assert.NotNil(t, changes) + assert.Equal(t, 2, changes.TotalChanges()) + assert.Equal(t, 1, changes.TotalBreakingChanges()) + assert.Equal(t, "#/components/schemas/NewPet", changes.Changes[0].New) + assert.Equal(t, v3.RequiredLabel, changes.Changes[1].Property) +} + +// https://github.com/pb33f/libopenapi/issues/663 +func TestCompareSchemas_RefToInline_IdenticalCopy(t *testing.T) { + low.ClearHashCache() + left := `openapi: 3.1.0 +components: + schemas: + Pet: + type: object + properties: + name: + type: string + OK: + $ref: '#/components/schemas/Pet'` + + right := `openapi: 3.1.0 +components: + schemas: + Pet: + type: object + properties: + name: + type: string + OK: + type: object + properties: + name: + type: string` + + leftDoc, rightDoc := test_BuildDoc(left, right) + assert.Nil(t, CompareSchemas(leftDoc.Components.Value.FindSchema("OK").Value, rightDoc.Components.Value.FindSchema("OK").Value)) +} + +// https://github.com/pb33f/libopenapi/issues/663 +func TestCompareSchemas_RefToInline_SimpleAllOf(t *testing.T) { + low.ClearHashCache() + left := `openapi: 3.1.0 +components: + schemas: + Pet: + allOf: + - type: object + properties: + name: + type: string + OK: + $ref: '#/components/schemas/Pet' + Inline: + allOf: + - type: object + properties: + name: + type: string` + + right := `openapi: 3.1.0 +components: + schemas: + OK: + allOf: + - type: object + properties: + name: + type: string + required: [name] + Inline: + allOf: + - type: object + properties: + name: + type: string + required: [name]` + + leftDoc, rightDoc := test_BuildDoc(left, right) + find := func(doc *v3.Document, name string) *base.SchemaProxy { + return doc.Components.Value.FindSchema(name).Value + } + + // the referenced allOf is compared through the same merged view as an inline one. + changes := CompareSchemas(find(leftDoc, "OK"), find(rightDoc, "OK")) + inline := CompareSchemas(find(leftDoc, "Inline"), find(rightDoc, "Inline")) + assert.NotNil(t, changes) + assert.NotEmpty(t, changesOtherThanRef(changes)) + assert.Equal(t, changesOtherThanRef(inline), changesOtherThanRef(changes)) +} + +// https://github.com/pb33f/libopenapi/issues/663 +func TestCompareSchemas_RefWithSiblings_StopsAtRef(t *testing.T) { + low.ClearHashCache() + left := `openapi: 3.1.0 +components: + schemas: + Pet: + type: object + properties: + name: + type: string + OK: + $ref: '#/components/schemas/Pet'` + + right := `openapi: 3.1.0 +components: + schemas: + Pet: + type: object + properties: + name: + type: string + OK: + $ref: '#/components/schemas/Pet' + required: [name]` + + leftDoc, rightDoc := test_BuildDoc(left, right) + l := leftDoc.Components.Value.FindSchema("OK").Value + r := rightDoc.Components.Value.FindSchema("OK").Value + assert.True(t, r.IsTransformedRefWithSiblings()) + + // a $ref with siblings is held as an allOf, which doesn't line up with the referenced schema field by field. + changes := CompareSchemas(l, r) + assert.Len(t, changes.GetAllChanges(), 1) + assert.Equal(t, v3.RefLabel, changes.Changes[0].Property) + + changes = CompareSchemas(r, l) + assert.Len(t, changes.GetAllChanges(), 1) + assert.Equal(t, v3.RefLabel, changes.Changes[0].Property) +} + +// https://github.com/pb33f/libopenapi/issues/663 +func TestCompareSchemas_RefToInline_CircularStopsAtRef(t *testing.T) { + low.ClearHashCache() + left := `openapi: 3.1.0 +components: + schemas: + Node: + type: object + properties: + next: + $ref: '#/components/schemas/Node' + OK: + $ref: '#/components/schemas/Node'` + + right := `openapi: 3.1.0 +components: + schemas: + Node: + type: object + properties: + next: + $ref: '#/components/schemas/Node' + OK: + type: object + required: [next]` + + leftDoc, rightDoc := test_BuildDoc(left, right) + l := leftDoc.Components.Value.FindSchema("OK").Value + r := rightDoc.Components.Value.FindSchema("OK").Value + + changes := CompareSchemas(l, r) + require.NotNil(t, changes) + assert.Len(t, changes.GetAllChanges(), 1) + assert.Equal(t, v3.RefLabel, changes.Changes[0].Property) + + changes = CompareSchemas(r, l) + require.NotNil(t, changes) + assert.Len(t, changes.GetAllChanges(), 1) + assert.Equal(t, v3.RefLabel, changes.Changes[0].Property) +} + +// https://github.com/pb33f/libopenapi/issues/663 +func TestCompareSchemas_RefRetargeted_Unresolved(t *testing.T) { + low.ClearHashCache() + spec := `openapi: 3.1.0 +components: + schemas: + Pet: + type: object + required: [name] + properties: + name: + type: string + OK: + $ref: '#/components/schemas/Pet'` + + cases := map[string]struct { + ref string + config *datamodel.DocumentConfiguration + }{ + "missing": {"#/components/schemas/Missing", datamodel.NewDocumentConfiguration()}, + "external, not resolved": {"other.yaml#/Pet", &datamodel.DocumentConfiguration{SkipExternalRefResolution: true}}, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + leftInfo, _ := datamodel.ExtractSpecInfo([]byte(spec)) + rightInfo, _ := datamodel.ExtractSpecInfo([]byte(strings.Replace(spec, "#/components/schemas/Pet'", tc.ref+"'", 1))) + leftDoc, _ := v3.CreateDocumentFromConfig(leftInfo, tc.config) + rightDoc, _ := v3.CreateDocumentFromConfig(rightInfo, tc.config) + + // there's nothing to compare the old schema with, so only the $ref change is reported. + changes := CompareSchemas(leftDoc.Components.Value.FindSchema("OK").Value, rightDoc.Components.Value.FindSchema("OK").Value) + require.NotNil(t, changes) + assert.Len(t, changes.GetAllChanges(), 1) + assert.Equal(t, v3.RefLabel, changes.Changes[0].Property) + }) + } +} + +// https://github.com/pb33f/libopenapi/issues/663 +func TestCompareSchemas_RefRetargeted_CircularCheckSkipped(t *testing.T) { + low.ClearHashCache() + left := `openapi: 3.1.0 +components: + schemas: + A: + type: object + properties: + b: + $ref: '#/components/schemas/B' + B: + type: object + properties: + a: + $ref: '#/components/schemas/A' + OK: + $ref: '#/components/schemas/A'` + right := strings.NewReplacer("schemas/A", "schemas/X", "schemas/B", "schemas/Y", " A:", " X:", " B:", " Y:").Replace(left) + + config := datamodel.NewDocumentConfiguration() + config.SkipCircularReferenceCheck = true + leftInfo, _ := datamodel.ExtractSpecInfo([]byte(left)) + rightInfo, _ := datamodel.ExtractSpecInfo([]byte(strings.Replace(right, "type: object\n properties:\n b:", "type: object\n description: changed\n properties:\n b:", 1))) + leftDoc, _ := v3.CreateDocumentFromConfig(leftInfo, config) + rightDoc, _ := v3.CreateDocumentFromConfig(rightInfo, config) + + // without the circular check a loop can't be recognised, so the reference is not followed. + changes := CompareSchemas(leftDoc.Components.Value.FindSchema("OK").Value, rightDoc.Components.Value.FindSchema("OK").Value) + require.NotNil(t, changes) + assert.Len(t, changes.GetAllChanges(), 1) + assert.Equal(t, v3.RefLabel, changes.Changes[0].Property) +} + +// https://github.com/pb33f/libopenapi/issues/663 +func TestCompareSchemas_RefToInline_EquivalentScalarUnion(t *testing.T) { + low.ClearHashCache() + left := `openapi: 3.1.0 +components: + schemas: + NullableString: + type: [string, 'null'] + OK: + $ref: '#/components/schemas/NullableString'` + + right := `openapi: 3.1.0 +components: + schemas: + OK: + anyOf: + - type: string + - type: 'null'` + + leftDoc, rightDoc := test_BuildDoc(left, right) + l := leftDoc.Components.Value.FindSchema("OK").Value + r := rightDoc.Components.Value.FindSchema("OK").Value + + // a type array and the equivalent anyOf are the same schema, whichever side is a reference. + changes := CompareSchemas(l, r) + require.NotNil(t, changes) + assert.Len(t, changes.GetAllChanges(), 1) + assert.Equal(t, v3.RefLabel, changes.Changes[0].Property) + + changes = CompareSchemas(r, l) + require.NotNil(t, changes) + assert.Len(t, changes.GetAllChanges(), 1) +} + // https://github.com/pb33f/libopenapi/issues/662 func TestCompareSchemas_RefToInline_UnrelatedCircularReference(t *testing.T) { low.ClearHashCache() diff --git a/what-changed/what_changed_test.go b/what-changed/what_changed_test.go index 2654c678..da5cfb2c 100644 --- a/what-changed/what_changed_test.go +++ b/what-changed/what_changed_test.go @@ -279,13 +279,18 @@ func TestCheckExplodedFileCheck(t *testing.T) { modDoc, _ := v3.CreateDocumentFromConfig(infoMod, config) changes := CompareOpenAPIDocuments(origDoc, modDoc) - assert.Equal(t, 1, changes.TotalChanges()) + assert.Equal(t, 3, changes.TotalChanges()) assert.Equal(t, 0, changes.TotalBreakingChanges()) + // both references changed, and the files they now point to are compared: c-alt.yaml has a new description. allChanges := changes.GetAllChanges() - assert.Len(t, allChanges, 1) + assert.Len(t, allChanges, 3) assert.Equal(t, "b.yaml#/components/schemas/SchemaB", allChanges[0].OriginalObject) assert.Equal(t, "b-alt.yaml#/components/schemas/SchemaB", allChanges[0].NewObject) + assert.Equal(t, "c.yaml", allChanges[1].OriginalObject) + assert.Equal(t, "c-alt.yaml", allChanges[1].NewObject) + assert.Equal(t, "description", allChanges[2].Property) + assert.Equal(t, "object C, but this has a changed description", allChanges[2].New) } From 675572155f6d8249d3fef1edd72ee528cda66006 Mon Sep 17 00:00:00 2001 From: quobix Date: Sat, 31 Oct 2026 10:50:06 -0400 Subject: [PATCH 06/33] fix: preserve exact decimal count constraints --- datamodel/low/integer_decimal.go | 95 +++++++++++++++++++++++++++ datamodel/low/integer_decimal_test.go | 53 +++++++++++++++ datamodel/low/model_builder.go | 28 +++++--- 3 files changed, 168 insertions(+), 8 deletions(-) create mode 100644 datamodel/low/integer_decimal.go create mode 100644 datamodel/low/integer_decimal_test.go diff --git a/datamodel/low/integer_decimal.go b/datamodel/low/integer_decimal.go new file mode 100644 index 00000000..88dea2fc --- /dev/null +++ b/datamodel/low/integer_decimal.go @@ -0,0 +1,95 @@ +package low + +import ( + "math" + "strconv" + "strings" +) + +// parseDecimalInt64 converts a syntactically valid decimal without rounding. +// It scans the mantissa once and retains at most 19 digits, regardless of exponent. +func parseDecimalInt64(value string) (int64, bool) { + value = strings.ReplaceAll(value, "_", "") + negative := strings.HasPrefix(value, "-") + value = strings.TrimLeft(value, "+-") + mantissa := value + exponent := int64(0) + if i := strings.IndexAny(value, "eE"); i >= 0 { + mantissa = value[:i] + exponent, _ = strconv.ParseInt(value[i+1:], 10, 64) + } + var digits, leading, trailing, fractional int64 + nonzero, afterPoint := false, false + for _, digit := range mantissa { + if digit == '.' { + afterPoint = true + continue + } + digits++ + if afterPoint { + fractional++ + } + if digit == '0' { + trailing++ + if !nonzero { + leading++ + } + } else { + nonzero = true + trailing = 0 + } + } + if !nonzero { + return 0, true + } + limit := int64(math.MaxInt64) + if negative { + limit = math.MinInt64 + } + // Bound the exponent before subtracting the mantissa's decimal places. + if exponent > digits+19 { + return limit, true + } + if exponent < -digits { + return 0, false + } + scale := exponent - fractional + if scale < -trailing { + return 0, false + } + integerDigits := digits - leading + scale + if integerDigits > 19 { + return limit, true + } + max := uint64(math.MaxInt64) + if negative { + max++ + } + var result uint64 + remaining := integerDigits + for _, digit := range mantissa { + if digit == '.' { + continue + } + if leading > 0 { + leading-- + continue + } + if remaining == 0 { + break + } + result = result*10 + uint64(digit-'0') + remaining-- + } + for remaining > 0 { + result *= 10 + remaining-- + } + if result > max { + return limit, true + } + if negative { + return -int64(result), true + } + return int64(result), true +} diff --git a/datamodel/low/integer_decimal_test.go b/datamodel/low/integer_decimal_test.go new file mode 100644 index 00000000..44ebbc84 --- /dev/null +++ b/datamodel/low/integer_decimal_test.go @@ -0,0 +1,53 @@ +package low + +import ( + "math" + "strconv" + "testing" + + "github.com/pb33f/go-yaml" + "github.com/pb33f/testify/require" +) + +func TestBuildModel_ExactDecimalIntegers(t *testing.T) { + for _, tc := range []struct { + value string + want int64 + set bool + }{ + {"9007199254740993.0", 9007199254740993, true}, + {"9.007199254740993e15", 9007199254740993, true}, + {"-9.007199254740993e15", -9007199254740993, true}, + {"9007199254740992.5", 0, false}, + {"1.0000000000000001", 0, false}, + {"1e-999999999999999999999", 0, false}, + {"0e-999999999999999999999", 0, true}, + {"1e999999999999999999999", math.MaxInt64, true}, + {"-1e999999999999999999999", math.MinInt64, true}, + {"9223372036854775807.0", math.MaxInt64, true}, + {"-9223372036854775808.0", math.MinInt64, true}, + {"9223372036854775808.0", math.MaxInt64, true}, + {"-9223372036854775809.0", math.MinInt64, true}, + {"001.2300e2", 123, true}, + {"1_000.00", 1000, true}, + {"100.0e-2", 1, true}, + {"100.0e-3", 0, false}, + {".0", 0, true}, + } { + t.Run(tc.value, func(t *testing.T) { + // Explicit tags also exercise overflow literals that YAML's implicit + // float resolver classifies as strings. + var root yaml.Node + require.NoError(t, yaml.Unmarshal([]byte("maxLength: !!float "+tc.value), &root)) + var model struct{ MaxLength NodeReference[int64] } + require.NoError(t, BuildModel(root.Content[0], &model)) + require.Equal(t, tc.set, !model.MaxLength.IsEmpty()) + require.Equal(t, tc.want, model.MaxLength.Value) + got, set := parseIntNode(tc.value) + require.Equal(t, tc.set, set) + if tc.set && strconv.IntSize == 64 { + require.Equal(t, tc.want, int64(got)) + } + }) + } +} diff --git a/datamodel/low/model_builder.go b/datamodel/low/model_builder.go index 5fbe332c..1e6bc7ad 100644 --- a/datamodel/low/model_builder.go +++ b/datamodel/low/model_builder.go @@ -544,17 +544,29 @@ func parseInt64Node(value string) (int64, bool) { return math.MinInt64, true } f, err := strconv.ParseFloat(value, 64) - switch { - case err != nil || math.IsNaN(f): + if (err != nil && !errors.Is(err, strconv.ErrRange)) || math.IsNaN(f) { return 0, false - case f >= math.MaxInt64: + } + if err == nil && math.IsInf(f, 0) { + if f < 0 { + return math.MinInt64, true + } return math.MaxInt64, true - case f <= math.MinInt64: - return math.MinInt64, true - case f != math.Trunc(f): - return 0, false } - return int64(f), true + // Keep the existing hexadecimal-float handling; decimal constraints must not + // pass through float64, which loses integer and fractional digits above 2^53. + if strings.ContainsAny(value, "xX") { + switch { + case f >= math.MaxInt64: + return math.MaxInt64, true + case f <= math.MinInt64: + return math.MinInt64, true + case f != math.Trunc(f): + return 0, false + } + return int64(f), true + } + return parseDecimalInt64(value) } // parseIntNode is parseInt64Node for an int field. Where int is 32 bits, strconv.Atoi clamps the From 7690dbb894da5cdbed47f49353dfe1d0c716b16b Mon Sep 17 00:00:00 2001 From: quobix Date: Sat, 31 Oct 2026 10:53:35 -0400 Subject: [PATCH 07/33] fix: bound retargeted schema comparisons by resolved pairs --- what-changed/model/schema.go | 150 ++++++++----------- what-changed/model/schema_comparison.go | 28 ++++ what-changed/model/schema_comparison_test.go | 105 +++++++++++++ what-changed/model/schema_test.go | 8 +- 4 files changed, 199 insertions(+), 92 deletions(-) create mode 100644 what-changed/model/schema_comparison.go create mode 100644 what-changed/model/schema_comparison_test.go diff --git a/what-changed/model/schema.go b/what-changed/model/schema.go index 317815db..3200031c 100644 --- a/what-changed/model/schema.go +++ b/what-changed/model/schema.go @@ -9,7 +9,6 @@ import ( "slices" "sort" "strings" - "sync" "github.com/pb33f/go-yaml" "github.com/pb33f/libopenapi/datamodel/low" @@ -416,8 +415,13 @@ func canCompareBehindRefChange(l, r *base.SchemaProxy) bool { } // CompareSchemas accepts a left and right SchemaProxy and checks for changes. If anything is found, returns -// a pointer to SchemaChanges, otherwise returns nil +// a pointer to SchemaChanges, otherwise returns nil. Within one call, a retargeted reference's schema pair +// is compared once; later occurrences retain their reference change. Separate calls produce independent reports. func CompareSchemas(l, r *base.SchemaProxy) *SchemaChanges { + return new(schemaComparison).compareSchemas(l, r) +} + +func (c *schemaComparison) compareSchemas(l, r *base.SchemaProxy) *SchemaChanges { sc := new(SchemaChanges) var changes []*Change @@ -504,6 +508,11 @@ func CompareSchemas(l, r *base.SchemaProxy) *SchemaChanges { } } + if len(changes) > 0 && !c.visit(l, r) { + sc.PropertyChanges = NewPropertyChanges(changes) + return sc + } + lSchema := l.Schema() rSchema := r.Schema() preserved := preservedObjectBranch(l, r, lSchema, rSchema) @@ -528,7 +537,7 @@ func CompareSchemas(l, r *base.SchemaProxy) *SchemaChanges { skipSimpleScalarUnionDiff := schemasUseEquivalentSimpleScalarUnion(l, r) // check schema core properties for changes. - checkSchemaPropertyChanges(comparisonLSchema, comparisonRSchema, l, r, &changes, sc, + c.checkSchemaPropertyChanges(comparisonLSchema, comparisonRSchema, l, r, &changes, sc, skipSimpleScalarUnionDiff || preserved != nil) // now for the confusing part, there is also a schema's 'properties' property to parse. @@ -562,10 +571,10 @@ func CompareSchemas(l, r *base.SchemaProxy) *SchemaChanges { ranyOf = nil } - props := checkMappedSchemaOfASchema(lProperties, rProperties, &changes) + props := c.checkMappedSchemaOfASchema(lProperties, rProperties, &changes) sc.SchemaPropertyChanges = props - deps := checkMappedSchemaOfASchema(lDepSchemas, rDepSchemas, &changes) + deps := c.checkMappedSchemaOfASchema(lDepSchemas, rDepSchemas, &changes) sc.DependentSchemasChanges = deps // Check dependent required changes @@ -582,32 +591,14 @@ func CompareSchemas(l, r *base.SchemaProxy) *SchemaChanges { sc.DependentRequiredChanges = depRequiredChanges } - patterns := checkMappedSchemaOfASchema(lPattProp, rPattProp, &changes) + patterns := c.checkMappedSchemaOfASchema(lPattProp, rPattProp, &changes) sc.PatternPropertiesChanges = patterns - var wg sync.WaitGroup - wg.Add(4) - go func() { - extractSchemaChanges(loneOf, roneOf, v3.OneOfLabel, - &sc.OneOfChanges, &changes) - wg.Done() - }() - go func() { - extractSchemaChanges(lallOf, rallOf, v3.AllOfLabel, - &sc.AllOfChanges, &changes) - wg.Done() - }() - go func() { - extractSchemaChanges(lanyOf, ranyOf, v3.AnyOfLabel, - &sc.AnyOfChanges, &changes) - wg.Done() - }() - go func() { - extractSchemaChanges(lprefix, rprefix, v3.PrefixItemsLabel, - &sc.PrefixItemsChanges, &changes) - wg.Done() - }() - wg.Wait() + // Stable traversal gives each resolved schema pair one consistent report location. + c.extractSchemaChanges(loneOf, roneOf, v3.OneOfLabel, &sc.OneOfChanges, &changes) + c.extractSchemaChanges(lallOf, rallOf, v3.AllOfLabel, &sc.AllOfChanges, &changes) + c.extractSchemaChanges(lanyOf, ranyOf, v3.AnyOfLabel, &sc.AnyOfChanges, &changes) + c.extractSchemaChanges(lprefix, rprefix, v3.PrefixItemsLabel, &sc.PrefixItemsChanges, &changes) } // done @@ -645,12 +636,12 @@ func checkSchemaXML(lSchema *base.Schema, rSchema *base.Schema, changes *[]*Chan } } -func checkMappedSchemaOfASchema( +func (c *schemaComparison) checkMappedSchemaOfASchema( lSchema, rSchema *orderedmap.Map[low.KeyReference[string], low.ValueReference[*base.SchemaProxy]], changes *[]*Change, ) map[string]*SchemaChanges { - var syncPropChanges sync.Map // concurrent-safe map + propChanges := make(map[string]*SchemaChanges) var lProps []string lEntities := make(map[string]*base.SchemaProxy) lKeyNodes := make(map[string]*yaml.Node) @@ -670,28 +661,23 @@ func checkMappedSchemaOfASchema( } sort.Strings(lProps) sort.Strings(rProps) - buildProperty(lProps, rProps, lEntities, rEntities, &syncPropChanges, changes, rKeyNodes, lKeyNodes) + c.buildProperty(lProps, rProps, lEntities, rEntities, propChanges, changes, rKeyNodes, lKeyNodes) - // Convert the sync.Map into a regular map[string]*SchemaChanges. - propChanges := make(map[string]*SchemaChanges) - syncPropChanges.Range(func(key, value interface{}) bool { - propChanges[key.(string)] = value.(*SchemaChanges) - return true - }) return propChanges } -func buildProperty(lProps, rProps []string, lEntities, rEntities map[string]*base.SchemaProxy, - propChanges *sync.Map, changes *[]*Change, rKeyNodes, lKeyNodes map[string]*yaml.Node, +func (c *schemaComparison) buildProperty(lProps, rProps []string, lEntities, rEntities map[string]*base.SchemaProxy, + propChanges map[string]*SchemaChanges, changes *[]*Change, rKeyNodes, lKeyNodes map[string]*yaml.Node, ) { - var wg sync.WaitGroup checkProperty := func(key string, lp, rp *base.SchemaProxy) { - defer wg.Done() + if _, compared := propChanges[key]; compared { + return + } if low.AreEqual(lp, rp) { return } - s := CompareSchemas(lp, rp) - propChanges.Store(key, s) + s := c.compareSchemas(lp, rp) + propChanges[key] = s } // left and right equal. @@ -700,8 +686,7 @@ func buildProperty(lProps, rProps []string, lEntities, rEntities map[string]*bas lp := lEntities[lProps[w]] rp := rEntities[rProps[w]] if lProps[w] == rProps[w] && lp != nil && rp != nil { - wg.Add(1) - go checkProperty(lProps[w], lp, rp) + checkProperty(lProps[w], lp, rp) } // Handle keys that do not match. if lProps[w] != rProps[w] { @@ -718,8 +703,7 @@ func buildProperty(lProps, rProps []string, lEntities, rEntities map[string]*bas h := slices.Index(lProps, rProps[w]) lp = lEntities[lProps[h]] rp = rEntities[rProps[w]] - wg.Add(1) - go checkProperty(lProps[h], lp, rp) + checkProperty(lProps[h], lp, rp) } } } @@ -729,8 +713,7 @@ func buildProperty(lProps, rProps []string, lEntities, rEntities map[string]*bas if len(lProps) > len(rProps) { for w := range lProps { if rEntities[lProps[w]] != nil { - wg.Add(1) - go checkProperty(lProps[w], lEntities[lProps[w]], rEntities[lProps[w]]) + checkProperty(lProps[w], lEntities[lProps[w]], rEntities[lProps[w]]) } else { CreateChange(changes, ObjectRemoved, v3.PropertiesLabel, lKeyNodes[lProps[w]], nil, BreakingRemoved(CompSchema, PropProperties), lEntities[lProps[w]], nil) @@ -738,8 +721,7 @@ func buildProperty(lProps, rProps []string, lEntities, rEntities map[string]*bas } for w := range rProps { if lEntities[rProps[w]] != nil { - wg.Add(1) - go checkProperty(rProps[w], lEntities[rProps[w]], rEntities[rProps[w]]) + checkProperty(rProps[w], lEntities[rProps[w]], rEntities[rProps[w]]) } else { CreateChange(changes, ObjectAdded, v3.PropertiesLabel, nil, rKeyNodes[rProps[w]], BreakingAdded(CompSchema, PropProperties), nil, rEntities[rProps[w]]) @@ -751,8 +733,7 @@ func buildProperty(lProps, rProps []string, lEntities, rEntities map[string]*bas if len(rProps) > len(lProps) { for _, propName := range rProps { if lEntities[propName] != nil { - wg.Add(1) - go checkProperty(propName, lEntities[propName], rEntities[propName]) + checkProperty(propName, lEntities[propName], rEntities[propName]) } else { CreateChange(changes, ObjectAdded, v3.PropertiesLabel, nil, rKeyNodes[propName], BreakingAdded(CompSchema, PropProperties), nil, rEntities[propName]) @@ -760,20 +741,16 @@ func buildProperty(lProps, rProps []string, lEntities, rEntities map[string]*bas } for _, propName := range lProps { if rEntities[propName] != nil { - wg.Add(1) - go checkProperty(propName, lEntities[propName], rEntities[propName]) + checkProperty(propName, lEntities[propName], rEntities[propName]) } else { CreateChange(changes, ObjectRemoved, v3.PropertiesLabel, nil, lKeyNodes[propName], BreakingRemoved(CompSchema, PropProperties), lEntities[propName], nil) } } } - - // Wait for all property comparisons to finish. - wg.Wait() } -func checkSchemaPropertyChanges( +func (c *schemaComparison) checkSchemaPropertyChanges( lSchema *base.Schema, rSchema *base.Schema, lProxy *base.SchemaProxy, @@ -1151,7 +1128,7 @@ func checkSchemaPropertyChanges( if lSchema != nil && lSchema.AdditionalProperties.Value != nil && rSchema != nil && rSchema.AdditionalProperties.Value != nil { if lSchema.AdditionalProperties.Value.IsA() && rSchema.AdditionalProperties.Value.IsA() { if !low.AreEqual(lSchema.AdditionalProperties.Value.A, rSchema.AdditionalProperties.Value.A) { - sc.AdditionalPropertiesChanges = CompareSchemas(lSchema.AdditionalProperties.Value.A, rSchema.AdditionalProperties.Value.A) + sc.AdditionalPropertiesChanges = c.compareSchemas(lSchema.AdditionalProperties.Value.A, rSchema.AdditionalProperties.Value.A) } } else { if lSchema.AdditionalProperties.Value.IsB() && rSchema.AdditionalProperties.Value.IsB() { @@ -1476,7 +1453,7 @@ func checkSchemaPropertyChanges( // If if (lSchema != nil && lSchema.If.Value != nil) && (rSchema != nil && rSchema.If.Value != nil) { if !low.AreEqual(lSchema.If.Value, rSchema.If.Value) { - sc.IfChanges = CompareSchemas(lSchema.If.Value, rSchema.If.Value) + sc.IfChanges = c.compareSchemas(lSchema.If.Value, rSchema.If.Value) } } // added If @@ -1492,7 +1469,7 @@ func checkSchemaPropertyChanges( // Else if (lSchema != nil && lSchema.Else.Value != nil) && (rSchema == nil || rSchema.Else.Value != nil) { if !low.AreEqual(lSchema.Else.Value, rSchema.Else.Value) { - sc.ElseChanges = CompareSchemas(lSchema.Else.Value, rSchema.Else.Value) + sc.ElseChanges = c.compareSchemas(lSchema.Else.Value, rSchema.Else.Value) } } // added Else @@ -1508,7 +1485,7 @@ func checkSchemaPropertyChanges( // Then if (lSchema != nil && lSchema.Then.Value != nil) && (rSchema != nil && rSchema.Then.Value != nil) { if !low.AreEqual(lSchema.Then.Value, rSchema.Then.Value) { - sc.ThenChanges = CompareSchemas(lSchema.Then.Value, rSchema.Then.Value) + sc.ThenChanges = c.compareSchemas(lSchema.Then.Value, rSchema.Then.Value) } } // added Then @@ -1524,7 +1501,7 @@ func checkSchemaPropertyChanges( // PropertyNames if (lSchema != nil && lSchema.PropertyNames.Value != nil) && (rSchema != nil && rSchema.PropertyNames.Value != nil) { if !low.AreEqual(lSchema.PropertyNames.Value, rSchema.PropertyNames.Value) { - sc.PropertyNamesChanges = CompareSchemas(lSchema.PropertyNames.Value, rSchema.PropertyNames.Value) + sc.PropertyNamesChanges = c.compareSchemas(lSchema.PropertyNames.Value, rSchema.PropertyNames.Value) } } // added PropertyNames @@ -1540,7 +1517,7 @@ func checkSchemaPropertyChanges( // Contains if (lSchema != nil && lSchema.Contains.Value != nil) && (rSchema != nil && rSchema.Contains.Value != nil) { if !low.AreEqual(lSchema.Contains.Value, rSchema.Contains.Value) { - sc.ContainsChanges = CompareSchemas(lSchema.Contains.Value, rSchema.Contains.Value) + sc.ContainsChanges = c.compareSchemas(lSchema.Contains.Value, rSchema.Contains.Value) } } // added Contains @@ -1556,7 +1533,7 @@ func checkSchemaPropertyChanges( // UnevaluatedItems if (lSchema != nil && lSchema.UnevaluatedItems.Value != nil) && (rSchema != nil && rSchema.UnevaluatedItems.Value != nil) { if !low.AreEqual(lSchema.UnevaluatedItems.Value, rSchema.UnevaluatedItems.Value) { - sc.UnevaluatedItemsChanges = CompareSchemas(lSchema.UnevaluatedItems.Value, rSchema.UnevaluatedItems.Value) + sc.UnevaluatedItemsChanges = c.compareSchemas(lSchema.UnevaluatedItems.Value, rSchema.UnevaluatedItems.Value) } } // added UnevaluatedItems @@ -1574,7 +1551,7 @@ func checkSchemaPropertyChanges( if (lSchema != nil && lSchema.UnevaluatedProperties.Value != nil) && (rSchema != nil && rSchema.UnevaluatedProperties.Value != nil) { if lSchema.UnevaluatedProperties.Value.IsA() && rSchema.UnevaluatedProperties.Value.IsA() { if !low.AreEqual(lSchema.UnevaluatedProperties.Value.A, rSchema.UnevaluatedProperties.Value.A) { - sc.UnevaluatedPropertiesChanges = CompareSchemas(lSchema.UnevaluatedProperties.Value.A, rSchema.UnevaluatedProperties.Value.A) + sc.UnevaluatedPropertiesChanges = c.compareSchemas(lSchema.UnevaluatedProperties.Value.A, rSchema.UnevaluatedProperties.Value.A) } } else { if lSchema.UnevaluatedProperties.Value.IsB() && rSchema.UnevaluatedProperties.Value.IsB() { @@ -1605,7 +1582,7 @@ func checkSchemaPropertyChanges( // Not if (lSchema != nil && lSchema.Not.Value != nil) && (rSchema != nil && rSchema.Not.Value != nil) { if !low.AreEqual(lSchema.Not.Value, rSchema.Not.Value) { - sc.NotChanges = CompareSchemas(lSchema.Not.Value, rSchema.Not.Value) + sc.NotChanges = c.compareSchemas(lSchema.Not.Value, rSchema.Not.Value) } } // added Not @@ -1623,7 +1600,7 @@ func checkSchemaPropertyChanges( if (lSchema != nil && lSchema.Items.Value != nil) && (rSchema != nil && rSchema.Items.Value != nil) { if lSchema.Items.Value.IsA() && rSchema.Items.Value.IsA() { if !low.AreEqual(lSchema.Items.Value.A, rSchema.Items.Value.A) { - sc.ItemsChanges = CompareSchemas(lSchema.Items.Value.A, rSchema.Items.Value.A) + sc.ItemsChanges = c.compareSchemas(lSchema.Items.Value.A, rSchema.Items.Value.A) } } else { CreateChange(changes, Modified, v3.ItemsLabel, @@ -1729,7 +1706,7 @@ func checkSchemaPropertyChanges( // contentSchema (JSON Schema 2020-12) - recursive schema comparison if lSchema != nil && !lSchema.ContentSchema.IsEmpty() && rSchema != nil && !rSchema.ContentSchema.IsEmpty() { - sc.ContentSchemaChanges = CompareSchemas(lSchema.ContentSchema.Value, rSchema.ContentSchema.Value) + sc.ContentSchemaChanges = c.compareSchemas(lSchema.ContentSchema.Value, rSchema.ContentSchema.Value) } if lSchema != nil && !lSchema.ContentSchema.IsEmpty() && (rSchema == nil || rSchema.ContentSchema.IsEmpty()) { CreateChange(changes, PropertyRemoved, base.ContentSchemaLabel, @@ -2231,7 +2208,7 @@ func checkExamples(lSchema *base.Schema, rSchema *base.Schema, changes *[]*Chang } } -func extractSchemaChanges( +func (c *schemaComparison) extractSchemaChanges( lSchema []low.ValueReference[*base.SchemaProxy], rSchema []low.ValueReference[*base.SchemaProxy], label string, @@ -2244,7 +2221,7 @@ func extractSchemaChanges( } if isOrderInsensitiveSchemaCompositionLabel(label) { - extractOrderInsensitiveSchemaChanges(lSchema, rSchema, label, sc, changes) + c.extractOrderInsensitiveSchemaChanges(lSchema, rSchema, label, sc, changes) return } @@ -2273,7 +2250,7 @@ func extractSchemaChanges( for w := range lKeys { // keys are different, which means there are changes. if lKeys[w] != rKeys[w] { - *sc = append(*sc, CompareSchemas(lEntities[lKeys[w]], rEntities[rKeys[w]])) + *sc = append(*sc, c.compareSchemas(lEntities[lKeys[w]], rEntities[rKeys[w]])) } } } @@ -2282,7 +2259,7 @@ func extractSchemaChanges( if len(lKeys) > len(rKeys) { for w := range lKeys { if w < len(rKeys) && lKeys[w] != rKeys[w] { - *sc = append(*sc, CompareSchemas(lEntities[lKeys[w]], rEntities[rKeys[w]])) + *sc = append(*sc, c.compareSchemas(lEntities[lKeys[w]], rEntities[rKeys[w]])) } if w >= len(rKeys) { breaking := schemaCompositionChangeBreaking(label, ObjectRemoved) @@ -2296,7 +2273,7 @@ func extractSchemaChanges( if len(rKeys) > len(lKeys) { for w := range rKeys { if w < len(lKeys) && rKeys[w] != lKeys[w] { - *sc = append(*sc, CompareSchemas(lEntities[lKeys[w]], rEntities[rKeys[w]])) + *sc = append(*sc, c.compareSchemas(lEntities[lKeys[w]], rEntities[rKeys[w]])) } if w >= len(lKeys) { breaking := schemaCompositionChangeBreaking(label, ObjectAdded) @@ -2315,15 +2292,13 @@ type schemaCompositionEntry struct { } type schemaCompositionPair struct { - left schemaCompositionEntry - right schemaCompositionEntry - changes *SchemaChanges + left schemaCompositionEntry + right schemaCompositionEntry } type schemaCompositionPairCandidate struct { left schemaCompositionEntry right schemaCompositionEntry - changes *SchemaChanges totalChanges int breakingChanges int stableKeyMatch bool @@ -2466,7 +2441,6 @@ func buildSchemaCompositionPairCandidateMatrix( row[j] = schemaCompositionPairCandidate{ left: leftEntry, right: rightEntry, - changes: changes, totalChanges: totalChanges, breakingChanges: breakingChanges, stableKeyMatch: leftEntry.stableKey == rightEntry.stableKey, @@ -2592,9 +2566,8 @@ func pairUnmatchedSchemaCompositionEntries( pairs := make([]schemaCompositionPair, 0, len(selected)) for i := range selected { pairs = append(pairs, schemaCompositionPair{ - left: selected[i].left, - right: selected[i].right, - changes: selected[i].changes, + left: selected[i].left, + right: selected[i].right, }) } @@ -2634,7 +2607,7 @@ func schemaCompositionChangeBreaking(label string, changeType int) bool { } } -func extractOrderInsensitiveSchemaChanges( +func (c *schemaComparison) extractOrderInsensitiveSchemaChanges( lSchema []low.ValueReference[*base.SchemaProxy], rSchema []low.ValueReference[*base.SchemaProxy], label string, @@ -2648,13 +2621,14 @@ func extractOrderInsensitiveSchemaChanges( unmatchedPairs := pairUnmatchedSchemaCompositionEntries(leftUnmatched, rightUnmatched) for i := range exactPairs { - if changes := CompareSchemas(exactPairs[i].left.proxy, exactPairs[i].right.proxy); changes != nil { + if changes := c.compareSchemas(exactPairs[i].left.proxy, exactPairs[i].right.proxy); changes != nil { *sc = append(*sc, changes) } } for i := range unmatchedPairs { - if unmatchedPairs[i].changes != nil { - *sc = append(*sc, unmatchedPairs[i].changes) + pair := unmatchedPairs[i] + if change := c.compareSchemas(pair.left.proxy, pair.right.proxy); change != nil { + *sc = append(*sc, change) } } diff --git a/what-changed/model/schema_comparison.go b/what-changed/model/schema_comparison.go new file mode 100644 index 00000000..ec2fa7c3 --- /dev/null +++ b/what-changed/model/schema_comparison.go @@ -0,0 +1,28 @@ +package model + +import ( + "github.com/pb33f/go-yaml" + "github.com/pb33f/libopenapi/datamodel/low/base" +) + +// A comparison owns its visited pairs. No state is shared with other callers or +// union candidate scoring. Properties and compositions visit pairs in stable order. +type schemaComparison struct { + visited map[schemaComparisonPair]struct{} +} + +type schemaComparisonPair struct { + left, right *yaml.Node +} + +func (c *schemaComparison) visit(left, right *base.SchemaProxy) bool { + pair := schemaComparisonPair{schemaDefinitionNode(left), schemaDefinitionNode(right)} + if _, exists := c.visited[pair]; exists { + return false + } + if c.visited == nil { + c.visited = make(map[schemaComparisonPair]struct{}) + } + c.visited[pair] = struct{}{} + return true +} diff --git a/what-changed/model/schema_comparison_test.go b/what-changed/model/schema_comparison_test.go new file mode 100644 index 00000000..be314d91 --- /dev/null +++ b/what-changed/model/schema_comparison_test.go @@ -0,0 +1,105 @@ +package model + +import ( + "fmt" + "strings" + "sync" + "testing" + + "github.com/pb33f/testify/require" +) + +func diamondComparisonSpec(prefix string, depth int) string { + var spec strings.Builder + spec.WriteString("openapi: 3.0.3\ninfo: {title: test, version: '1'}\npaths: {}\ncomponents:\n schemas:\n Root:\n type: object\n properties:\n entry:\n $ref: '#/components/schemas/" + prefix + "0'\n") + for i := 0; i < depth; i++ { + fmt.Fprintf(&spec, " %s%d:\n type: object\n properties:\n a: {$ref: '#/components/schemas/%s%d'}\n b: {$ref: '#/components/schemas/%s%d'}\n", prefix, i, prefix, i+1, prefix, i+1) + } + fmt.Fprintf(&spec, " %s%d: {type: string}\n", prefix, depth) + return spec.String() +} + +func TestCompareSchemas_RetargetedDiamondIsBounded(t *testing.T) { + for _, depth := range []int{8, 16, 24} { + t.Run(fmt.Sprint(depth), func(t *testing.T) { + left, right := test_BuildDoc(diamondComparisonSpec("L", depth), diamondComparisonSpec("R", depth)) + l, r := left.Components.Value.FindSchema("Root").Value, right.Components.Value.FindSchema("Root").Value + for i := 0; i < 3; i++ { + changes := CompareSchemas(l, r) + require.Equal(t, 2*depth+1, changes.TotalChanges()) + current := changes.SchemaPropertyChanges["entry"] + for level := 0; level < depth; level++ { + require.Equal(t, 1, current.SchemaPropertyChanges["b"].TotalChanges()) + current = current.SchemaPropertyChanges["a"] + } + } + var wg sync.WaitGroup + counts := make([]int, 4) + for i := range counts { + wg.Add(1) + go func() { defer wg.Done(); counts[i] = CompareSchemas(l, r).TotalChanges() }() + } + wg.Wait() + for _, count := range counts { + require.Equal(t, 2*depth+1, count) + } + }) + } +} + +func TestCompareSchemas_CandidateScoringHasIndependentPairs(t *testing.T) { + left := `openapi: 3.0.3 +components: + schemas: + Root: + type: object + properties: + first: + anyOf: + - {$ref: '#/components/schemas/A'} + - {$ref: '#/components/schemas/B'} + - {$ref: '#/components/schemas/X'} + second: + anyOf: + - {$ref: '#/components/schemas/A'} + - {$ref: '#/components/schemas/B'} + - {$ref: '#/components/schemas/X'} + A: {type: object, properties: {a: {type: string}}} + B: {type: object, properties: {b: {type: integer}}} + X: {type: object, properties: {x: {type: boolean}}} +` + right := `openapi: 3.0.3 +components: + schemas: + Root: + type: object + properties: + first: + anyOf: + - {$ref: '#/components/schemas/B2'} + - {$ref: '#/components/schemas/A2'} + second: + anyOf: + - {$ref: '#/components/schemas/B2'} + - {$ref: '#/components/schemas/A2'} + A2: {type: object, properties: {a: {type: integer}}} + B2: {type: object, properties: {b: {type: string}}} +` + l, r := test_BuildDoc(left, right) + for i := 0; i < 3; i++ { + changes := CompareSchemas(l.Components.Value.FindSchema("Root").Value, r.Components.Value.FindSchema("Root").Value) + require.Equal(t, 8, changes.TotalChanges()) + for _, name := range []string{"first", "second"} { + union := changes.SchemaPropertyChanges[name] + require.Len(t, union.AnyOfChanges, 2) + for _, branch := range union.AnyOfChanges { + for _, property := range branch.SchemaPropertyChanges { + require.Len(t, property.Changes, 1) + require.Equal(t, "type", property.Changes[0].Property) + } + } + } + require.Equal(t, 5, changes.SchemaPropertyChanges["first"].TotalChanges()) + require.Equal(t, 3, changes.SchemaPropertyChanges["second"].TotalChanges()) + } +} diff --git a/what-changed/model/schema_test.go b/what-changed/model/schema_test.go index 2db0dbf4..06d2ff55 100644 --- a/what-changed/model/schema_test.go +++ b/what-changed/model/schema_test.go @@ -4204,7 +4204,7 @@ func TestExtractOrderInsensitiveSchemaChanges_ExactRefPairsCanProduceChanges(t * var schemaChanges []*SchemaChanges var changes []*Change - extractOrderInsensitiveSchemaChanges( + new(schemaComparison).extractOrderInsensitiveSchemaChanges( []low.ValueReference[*base.SchemaProxy]{{Value: leftProxy}}, []low.ValueReference[*base.SchemaProxy]{{Value: rightProxy}}, v3.OneOfLabel, @@ -4961,7 +4961,7 @@ func TestExtractSchemaChanges_PrefixItemsRemovedAndModified(t *testing.T) { var schemaChanges []*SchemaChanges var changes []*Change - extractSchemaChanges(left, right, v3.PrefixItemsLabel, &schemaChanges, &changes) + new(schemaComparison).extractSchemaChanges(left, right, v3.PrefixItemsLabel, &schemaChanges, &changes) assert.Len(t, schemaChanges, 1) assert.Len(t, changes, 1) @@ -4983,7 +4983,7 @@ func TestExtractSchemaChanges_PrefixItemsAddedAndModified(t *testing.T) { var schemaChanges []*SchemaChanges var changes []*Change - extractSchemaChanges(left, right, v3.PrefixItemsLabel, &schemaChanges, &changes) + new(schemaComparison).extractSchemaChanges(left, right, v3.PrefixItemsLabel, &schemaChanges, &changes) assert.Len(t, schemaChanges, 1) assert.Len(t, changes, 1) @@ -5063,7 +5063,7 @@ func TestCompareSchemas_fireNilCheck(t *testing.T) { // Clear hash cache to ensure deterministic results in concurrent test environments low.ClearHashCache() checkSchemaXML(nil, nil, nil, nil) - checkSchemaPropertyChanges(nil, nil, nil, nil, nil, nil, false) + new(schemaComparison).checkSchemaPropertyChanges(nil, nil, nil, nil, nil, nil, false) checkExamples(nil, nil, nil) } From 93ef837164a75db34f7787d37d51344e8e0aadb9 Mon Sep 17 00:00:00 2001 From: quobix Date: Sat, 31 Oct 2026 10:56:39 -0400 Subject: [PATCH 08/33] fix: count allOf breaking changes once --- what-changed/model/schema.go | 5 ---- .../model/schema_breaking_count_test.go | 25 +++++++++++++++++++ what-changed/model/schema_test.go | 2 +- 3 files changed, 26 insertions(+), 6 deletions(-) create mode 100644 what-changed/model/schema_breaking_count_test.go diff --git a/what-changed/model/schema.go b/what-changed/model/schema.go index 3200031c..2ae69523 100644 --- a/what-changed/model/schema.go +++ b/what-changed/model/schema.go @@ -307,11 +307,6 @@ func (s *SchemaChanges) TotalBreakingChanges() int { t += s.AllOfChanges[n].TotalBreakingChanges() } } - if len(s.AllOfChanges) > 0 { - for n := range s.AllOfChanges { - t += s.AllOfChanges[n].TotalBreakingChanges() - } - } if len(s.AnyOfChanges) > 0 { for n := range s.AnyOfChanges { t += s.AnyOfChanges[n].TotalBreakingChanges() diff --git a/what-changed/model/schema_breaking_count_test.go b/what-changed/model/schema_breaking_count_test.go new file mode 100644 index 00000000..b7a81053 --- /dev/null +++ b/what-changed/model/schema_breaking_count_test.go @@ -0,0 +1,25 @@ +package model + +import ( + "strings" + "testing" + + "github.com/pb33f/testify/require" +) + +func TestCompareSchemas_AllOfBreakingChangesCountedOnce(t *testing.T) { + left := `openapi: 3.0.3 +components: + schemas: + Root: + allOf: + - type: string +` + right := strings.Replace(left, "type: string", "type: integer", 1) + l, r := test_BuildDoc(left, right) + changes := CompareSchemas(l.Components.Value.FindSchema("Root").Value, r.Components.Value.FindSchema("Root").Value) + require.Equal(t, 1, changes.TotalChanges()) + require.Equal(t, 1, changes.TotalBreakingChanges()) + require.Len(t, changes.GetAllChanges(), 1) + require.True(t, changes.GetAllChanges()[0].Breaking) +} diff --git a/what-changed/model/schema_test.go b/what-changed/model/schema_test.go index 06d2ff55..44bb1765 100644 --- a/what-changed/model/schema_test.go +++ b/what-changed/model/schema_test.go @@ -2118,7 +2118,7 @@ components: assert.NotNil(t, changes) assert.Equal(t, 2, changes.TotalChanges()) assert.Len(t, changes.GetAllChanges(), 2) - assert.Equal(t, 2, changes.TotalBreakingChanges()) + assert.Equal(t, 1, changes.TotalBreakingChanges()) assert.Equal(t, ObjectAdded, changes.Changes[0].ChangeType) assert.Equal(t, v3.AllOfLabel, changes.Changes[0].Property) assert.Equal(t, Modified, changes.AllOfChanges[0].Changes[0].ChangeType) From ea25a558e1aae4f16b71719ea5cd4388f370b9b4 Mon Sep 17 00:00:00 2001 From: quobix Date: Sat, 31 Oct 2026 10:57:14 -0400 Subject: [PATCH 09/33] test: cover shared schema graphs with property changes --- what-changed/model/schema_comparison_test.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/what-changed/model/schema_comparison_test.go b/what-changed/model/schema_comparison_test.go index be314d91..6187cdd7 100644 --- a/what-changed/model/schema_comparison_test.go +++ b/what-changed/model/schema_comparison_test.go @@ -47,6 +47,22 @@ func TestCompareSchemas_RetargetedDiamondIsBounded(t *testing.T) { } } +func TestCompareSchemas_RetargetedDiamondWithPropertyAddRemove(t *testing.T) { + const depth = 8 + left := diamondComparisonSpec("L", depth) + right := strings.Replace(diamondComparisonSpec("R", depth), " entry:", " extra: {type: string}\n entry:", 1) + l, r := test_BuildDoc(left, right) + for _, reverse := range []bool{false, true} { + lp, rp := l.Components.Value.FindSchema("Root").Value, r.Components.Value.FindSchema("Root").Value + if reverse { + lp, rp = rp, lp + } + changes := CompareSchemas(lp, rp) + require.Equal(t, 2*depth+2, changes.TotalChanges()) + require.Equal(t, 2*depth+1, changes.SchemaPropertyChanges["entry"].TotalChanges()) + } +} + func TestCompareSchemas_CandidateScoringHasIndependentPairs(t *testing.T) { left := `openapi: 3.0.3 components: From f3d00d2ed4e504e4c6f2f593fe0e35d2fc8cc022 Mon Sep 17 00:00:00 2001 From: Luca Candela Date: Tue, 27 Oct 2026 14:05:31 -0700 Subject: [PATCH 10/33] fix(index): resolve effective YAML component mappings --- datamodel/low/v3/components.go | 1 + document_effective_index_test.go | 65 +++++++++++ index/effective_yaml_test.go | 178 +++++++++++++++++++++++++++++++ index/extract_refs.go | 20 +++- index/extract_refs_inline.go | 8 +- index/extract_refs_walk.go | 4 +- index/schema_id_resolve.go | 6 +- index/spec_index_counts.go | 62 ++++++----- utils/merge_view_test.go | 18 ++++ utils/utils.go | 5 + 10 files changed, 335 insertions(+), 32 deletions(-) create mode 100644 document_effective_index_test.go create mode 100644 index/effective_yaml_test.go diff --git a/datamodel/low/v3/components.go b/datamodel/low/v3/components.go index 12ac3c97..343791e9 100644 --- a/datamodel/low/v3/components.go +++ b/datamodel/low/v3/components.go @@ -278,6 +278,7 @@ func (co *Components) Build(ctx context.Context, root *yaml.Node, idx *index.Spe func extractComponentValues[T low.Buildable[N], N any](ctx context.Context, label string, root *yaml.Node, idx *index.SpecIndex, co *Components) (low.NodeReference[*orderedmap.Map[low.KeyReference[string], low.ValueReference[T]]], error) { var emptyResult low.NodeReference[*orderedmap.Map[low.KeyReference[string], low.ValueReference[T]]] _, nodeLabel, nodeValue := utils.FindKeyNodeFullTop(label, root.Content) + nodeValue = utils.ResolveMergeNodes(nodeValue) if nodeValue == nil { return emptyResult, nil } diff --git a/document_effective_index_test.go b/document_effective_index_test.go new file mode 100644 index 00000000..e7f48c11 --- /dev/null +++ b/document_effective_index_test.go @@ -0,0 +1,65 @@ +package libopenapi + +import ( + "testing" + + "github.com/pb33f/go-yaml" + "github.com/pb33f/libopenapi/utils" + "github.com/pb33f/testify/assert" + "github.com/pb33f/testify/require" +) + +func TestBuildV3ModelEffectiveComponentIndex(t *testing.T) { + const prefix = `openapi: 3.1.1 +info: {title: Index, version: '1'} +paths: + /bound: + get: + responses: + '200': + description: Bound + content: + application/json: + schema: {$ref: '#/components/schemas/Bound'} +` + for _, tc := range []struct{ name, components string }{ + {"plain", "components:\n schemas:\n Bound: {type: integer, minimum: 5}\n"}, + {"components merge", "x-base: &base\n schemas:\n Bound: {type: integer, minimum: 5}\ncomponents:\n <<: *base\n"}, + {"schemas merge", "x-base: &base\n Bound: {type: integer, minimum: 5}\ncomponents:\n schemas:\n <<: *base\n"}, + {"components alias", "x-base: &base\n schemas:\n Bound: {type: integer, minimum: 5}\ncomponents: *base\n"}, + {"schemas alias", "x-base: &base\n Bound: {type: integer, minimum: 5}\ncomponents:\n schemas: *base\n"}, + {"schema value merge", "x-base: &base {type: integer, minimum: 5}\ncomponents:\n schemas:\n Bound:\n <<: *base\n"}, + } { + t.Run(tc.name, func(t *testing.T) { + doc, err := NewDocument([]byte(prefix + tc.components)) + require.NoError(t, err) + root := doc.GetSpecInfo().RootNode + before, err := yaml.Marshal(root) + require.NoError(t, err) + model, err := doc.BuildV3Model() + require.NoError(t, err) + proxy, ok := model.Model.Components.Schemas.Get("Bound") + require.True(t, ok) + schema := proxy.Schema() + require.NotNil(t, schema) + require.NotNil(t, schema.Minimum) + assert.Equal(t, float64(5), *schema.Minimum) + refs := model.Index.GetAllComponentSchemas() + require.Len(t, refs, 1) + ref := refs["#/components/schemas/Bound"] + require.NotNil(t, ref) + assert.Equal(t, "Bound", ref.KeyNode.Value) + _, min := utils.FindKeyNodeTop("minimum", utils.ResolveMergeNodes(ref.Node).Content) + assert.Same(t, min, schema.GoLow().Minimum.ValueNode) + after, err := yaml.Marshal(root) + require.NoError(t, err) + assert.Equal(t, before, after) + rendered, err := doc.Render() + require.NoError(t, err) + reloaded, err := NewDocument(rendered) + require.NoError(t, err) + _, err = reloaded.BuildV3Model() + require.NoError(t, err) + }) + } +} diff --git a/index/effective_yaml_test.go b/index/effective_yaml_test.go new file mode 100644 index 00000000..43a761a2 --- /dev/null +++ b/index/effective_yaml_test.go @@ -0,0 +1,178 @@ +package index + +import ( + "context" + "strings" + "testing" + + "github.com/pb33f/go-yaml" + "github.com/pb33f/testify/assert" + "github.com/pb33f/testify/require" +) + +func TestEffectiveReferenceOccurrences(t *testing.T) { + const source = `openapi: 3.1.1 +info: {title: Index, version: '1'} +paths: {} +x-ref: &ref {$ref: '#/components/schemas/Target'} +x-props: &props + nested: *ref +components: + schemas: + Target: {type: string} + First: {type: object, properties: *props} + Second: {type: object, properties: {<<: *props}} + Example: + type: object + example: {nested: *ref} + examples: + Named: + value: {nested: *ref} +` + var root yaml.Node + require.NoError(t, yaml.Unmarshal([]byte(source), &root)) + before, err := yaml.Marshal(&root) + require.NoError(t, err) + idx := NewSpecIndex(&root) + paths := map[string]bool{} + for _, ref := range idx.rawSequencedRefs { + paths[strings.Join(ref.SourcePath, "/")] = true + } + assert.Contains(t, paths, "components/schemas/First/properties/nested") + assert.Contains(t, paths, "components/schemas/Second/properties/nested") + for path := range paths { + assert.NotContains(t, path, "/Example/") + assert.NotContains(t, path, "/examples/Named/value/") + } + assert.Empty(t, idx.GetReferenceIndexErrors()) + after, err := yaml.Marshal(&root) + require.NoError(t, err) + assert.Equal(t, before, after) +} + +func TestEffectiveFragmentTraversal(t *testing.T) { + const source = `base: &base + escaped/key: &value {type: string} +components: {schemas: {<<: *base}} +array: [*value] +` + var root yaml.Node + require.NoError(t, yaml.Unmarshal([]byte(source), &root)) + for _, path := range []string{"#/components/schemas/escaped~1key/type", "#/array/0/type"} { + got := navigateToFragment(&root, path) + require.NotNil(t, got, path) + assert.Equal(t, "string", got.Value) + } + assert.Nil(t, navigateToFragment(&root, "#/components/schemas/missing")) + assert.Nil(t, navigateToFragment(&root, "#/array/1")) +} + +func TestEffectiveTraversalCycles(t *testing.T) { + for _, source := range []string{ + "x-map: &mapping {<<: *mapping, nested: *mapping, local: value}\n", + "x-sequence: &sequence [*sequence]\n", + } { + var root yaml.Node + require.NoError(t, yaml.Unmarshal([]byte(source), &root)) + idx := NewSpecIndex(&root) + assert.Empty(t, idx.GetReferenceIndexErrors()) + } +} + +func TestEffectiveComponentCollections(t *testing.T) { + const source = `openapi: 3.1.1 +info: {title: Index, version: '1'} +paths: {} +x-entry: &entry + Named: {description: inherited} +x-collections: &collections + schemas: {<<: *entry} + parameters: {<<: *entry} + requestBodies: {<<: *entry} + responses: {<<: *entry} + securitySchemes: {<<: *entry} + headers: {<<: *entry} + examples: {<<: *entry} + links: {<<: *entry} + callbacks: {<<: *entry} + pathItems: {<<: *entry} +components: *collections +` + var root yaml.Node + require.NoError(t, yaml.Unmarshal([]byte(source), &root)) + idx := NewSpecIndex(&root) + assert.Equal(t, 1, idx.GetComponentSchemaCount()) + assert.Equal(t, 1, idx.GetComponentParameterCount()) + for _, kind := range []string{"schemas", "parameters", "requestBodies", "responses", "securitySchemes", "headers", "examples", "links", "callbacks", "pathItems"} { + ref := idx.FindComponent(context.Background(), "#/components/"+kind+"/Named") + require.NotNil(t, ref, kind) + assert.Equal(t, "Named", ref.KeyNode.Value, kind) + assert.Nil(t, idx.FindComponent(context.Background(), "#/components/"+kind+"/<<"), kind) + } +} + +func TestEffectiveInlineSchemaCollections(t *testing.T) { + const source = `openapi: 3.1.1 +info: {title: Index, version: '1'} +paths: {} +x-schema: &schema {type: object, properties: {name: {type: string}}} +x-map: &map {child: *schema} +x-array: &array [*schema] +components: + schemas: + Single: {type: array, items: *schema} + Mapping: {type: object, properties: {<<: *map}} + Array: {allOf: *array} +` + var root yaml.Node + require.NoError(t, yaml.Unmarshal([]byte(source), &root)) + idx := NewSpecIndex(&root) + definitions := map[string]*Reference{} + for _, ref := range idx.allInlineSchemaObjectDefinitions { + definitions[ref.Definition] = ref + } + for _, definition := range []string{ + "#/components/schemas/Single/items", + "#/components/schemas/Mapping/properties/child", + "#/components/schemas/Array/allOf/0", + } { + assert.Contains(t, definitions, definition) + } +} + +func TestEffectiveSchemaIDScope(t *testing.T) { + const source = `openapi: 3.1.1 +info: {title: Index, version: '1'} +paths: {} +x-scope: &scope {$id: 'https://example.test/scoped', type: object} +x-property: &property {$ref: '#/components/schemas/Target'} +components: + schemas: + Target: {type: string} + Scoped: + <<: *scope + properties: {value: *property} + Ordinary: + type: object + properties: {value: *property} +` + var root yaml.Node + require.NoError(t, yaml.Unmarshal([]byte(source), &root)) + idx := NewSpecIndex(&root) + scopes := map[string]string{} + for _, ref := range idx.rawSequencedRefs { + scopes[strings.Join(ref.SourcePath, "/")] = ref.SchemaIdBase + } + assert.Equal(t, "https://example.test/scoped", scopes["components/schemas/Scoped/properties/value"]) + assert.Equal(t, "", scopes["components/schemas/Ordinary/properties/value"]) +} + +func TestEffectiveLookupWithAbsentNodes(t *testing.T) { + t.Run("missing collection mapping", func(t *testing.T) { + require.Nil(t, effectiveMappingValue(nil, "schemas")) + }) + t.Run("document with absent content node", func(t *testing.T) { + root := &yaml.Node{Kind: yaml.DocumentNode, Content: []*yaml.Node{nil}} + require.Nil(t, navigateToFragment(root, "#/type")) + }) +} diff --git a/index/extract_refs.go b/index/extract_refs.go index 0d0b642c..e466bb76 100644 --- a/index/extract_refs.go +++ b/index/extract_refs.go @@ -7,6 +7,7 @@ import ( "context" "github.com/pb33f/go-yaml" + "github.com/pb33f/libopenapi/utils" ) func isSchemaContainingNode(v string) bool { @@ -99,12 +100,25 @@ func isDirectOpenAPIExampleValuePath(seenPath []string) bool { // ExtractRefs will return a deduplicated slice of references for every unique ref found in the document. // The total number of refs, will generally be much higher, you can extract those from GetRawReferenceCount() func (index *SpecIndex) ExtractRefs(ctx context.Context, node, parent *yaml.Node, seenPath []string, level int, poly bool, pName string) []*Reference { + return index.extractRefs(ctx, node, parent, seenPath, level, poly, pName, make(map[*yaml.Node]struct{})) +} + +// Ancestry tracks canonical source nodes, not the shallow views created during +// merge expansion. A shared anchor is visited again at each distinct path. +func (index *SpecIndex) extractRefs(ctx context.Context, node, parent *yaml.Node, seenPath []string, level int, poly bool, pName string, ancestry map[*yaml.Node]struct{}) []*Reference { + node = utils.NodeAlias(node) if node == nil { return nil } - - state := index.initializeExtractRefsState(ctx, node, seenPath, level, poly, pName) - found := index.walkExtractRefs(node, parent, &state) + if _, active := ancestry[node]; active { + return nil + } + ancestry[node] = struct{}{} + defer delete(ancestry, node) + view := utils.ResolveMergeNodes(node) + state := index.initializeExtractRefsState(ctx, view, seenPath, level, poly, pName) + state.ancestry = ancestry + found := index.walkExtractRefs(view, parent, &state) index.refCount = len(index.allRefs) return found } diff --git a/index/extract_refs_inline.go b/index/extract_refs_inline.go index 2cc2f35d..fe61707a 100644 --- a/index/extract_refs_inline.go +++ b/index/extract_refs_inline.go @@ -50,7 +50,7 @@ func (index *SpecIndex) collectInlineSchemaDefinition(parent, node *yaml.Node, s } keyNode := node.Content[keyIndex] - valueNode := node.Content[keyIndex+1] + valueNode := utils.ResolveMergeNodes(node.Content[keyIndex+1]) var jsonPath, definitionPath, fullDefinitionPath string if len(seenPath) > 0 || keyNode.Value != "" { @@ -90,7 +90,7 @@ func (index *SpecIndex) collectMapSchemaDefinitions(parent, node *yaml.Node, see } keyNode := node.Content[keyIndex] - propertiesNode := node.Content[keyIndex+1] + propertiesNode := utils.ResolveMergeNodes(node.Content[keyIndex+1]) if len(seenPath) > 0 { for _, p := range seenPath { @@ -108,6 +108,7 @@ func (index *SpecIndex) collectMapSchemaDefinitions(parent, node *yaml.Node, see continue } + prop = utils.ResolveMergeNodes(prop) var jsonPath, definitionPath, fullDefinitionPath string if len(seenPath) > 0 || keyNode.Value != "" && label != "" { if prefix == "" { @@ -146,10 +147,11 @@ func (index *SpecIndex) collectArraySchemaDefinitions(parent, node *yaml.Node, s } keyNode := node.Content[keyIndex] - arrayNode := node.Content[keyIndex+1] + arrayNode := utils.ResolveMergeNodes(node.Content[keyIndex+1]) prefix := "" for h, element := range arrayNode.Content { + element = utils.ResolveMergeNodes(element) var jsonPath, definitionPath, fullDefinitionPath string if len(seenPath) > 0 { if prefix == "" { diff --git a/index/extract_refs_walk.go b/index/extract_refs_walk.go index a9eb1717..18558736 100644 --- a/index/extract_refs_walk.go +++ b/index/extract_refs_walk.go @@ -12,6 +12,7 @@ import ( ) type extractRefsState struct { + ancestry map[*yaml.Node]struct{} ctx context.Context scope *SchemaIdScope parentBaseURI string @@ -100,10 +101,11 @@ func (index *SpecIndex) walkChildExtractRefs(node, parent *yaml.Node, state *ext state.polyName = state.prev } } - return index.ExtractRefs(state.ctx, node, parent, state.seenPath, state.level, state.poly, state.polyName) + return index.extractRefs(state.ctx, node, parent, state.seenPath, state.level, state.poly, state.polyName, state.ancestry) } func isDirectOpenAPIExampleRefNode(node *yaml.Node) bool { + node = utils.ResolveMergeNodes(node) return utils.IsNodeMap(node) && utils.GetRefValueNode(node) != nil && len(node.Content) == 2 } diff --git a/index/schema_id_resolve.go b/index/schema_id_resolve.go index 505806a7..3ae0cdca 100644 --- a/index/schema_id_resolve.go +++ b/index/schema_id_resolve.go @@ -284,6 +284,10 @@ func navigateToFragment(root *yaml.Node, fragment string) *yaml.Node { segment = strings.ReplaceAll(segment, "~1", "/") segment = strings.ReplaceAll(segment, "~0", "~") + current = utils.ResolveMergeNodes(current) + if current == nil { + return nil + } found := false if current.Kind == yaml.MappingNode { for i := 0; i < len(current.Content)-1; i += 2 { @@ -310,5 +314,5 @@ func navigateToFragment(root *yaml.Node, fragment string) *yaml.Node { } } - return current + return utils.ResolveMergeNodes(current) } diff --git a/index/spec_index_counts.go b/index/spec_index_counts.go index 0793299b..2d8ca803 100644 --- a/index/spec_index_counts.go +++ b/index/spec_index_counts.go @@ -337,14 +337,15 @@ func (index *SpecIndex) GetComponentSchemaCount() int { return index.schemaCount } - for i, n := range index.root.Content[0].Content { + root := utils.ResolveMergeNodes(index.root.Content[0]) + for i, n := range root.Content { if i%2 != 0 { continue } if n.Value == "servers" { - index.rootServersNode = index.root.Content[0].Content[i+1] - if i+1 < len(index.root.Content[0].Content) { - serverDefinitions := index.root.Content[0].Content[i+1] + index.rootServersNode = root.Content[i+1] + if i+1 < len(root.Content) { + serverDefinitions := root.Content[i+1] for x, def := range serverDefinitions.Content { index.serversRefs = append(index.serversRefs, &Reference{ Definition: "servers", @@ -357,9 +358,9 @@ func (index *SpecIndex) GetComponentSchemaCount() int { } } if n.Value == "security" { - index.rootSecurityNode = index.root.Content[0].Content[i+1] - if i+1 < len(index.root.Content[0].Content) { - securityDefinitions := index.root.Content[0].Content[i+1] + index.rootSecurityNode = root.Content[i+1] + if i+1 < len(root.Content) { + securityDefinitions := root.Content[i+1] for x, def := range securityDefinitions.Content { if len(def.Content) > 0 { name := def.Content[0] @@ -374,16 +375,16 @@ func (index *SpecIndex) GetComponentSchemaCount() int { } } if n.Value == "components" { - _, schemasNode := utils.FindKeyNode("schemas", index.root.Content[0].Content[i+1].Content) - _, parametersNode := utils.FindKeyNode("parameters", index.root.Content[0].Content[i+1].Content) - _, requestBodiesNode := utils.FindKeyNode("requestBodies", index.root.Content[0].Content[i+1].Content) - _, responsesNode := utils.FindKeyNode("responses", index.root.Content[0].Content[i+1].Content) - _, securitySchemesNode := utils.FindKeyNode("securitySchemes", index.root.Content[0].Content[i+1].Content) - _, headersNode := utils.FindKeyNode("headers", index.root.Content[0].Content[i+1].Content) - _, examplesNode := utils.FindKeyNode("examples", index.root.Content[0].Content[i+1].Content) - _, linksNode := utils.FindKeyNode("links", index.root.Content[0].Content[i+1].Content) - _, callbacksNode := utils.FindKeyNode("callbacks", index.root.Content[0].Content[i+1].Content) - _, pathItemsNode := utils.FindKeyNode("pathItems", index.root.Content[0].Content[i+1].Content) + schemasNode := effectiveMappingValue(root.Content[i+1], "schemas") + parametersNode := effectiveMappingValue(root.Content[i+1], "parameters") + requestBodiesNode := effectiveMappingValue(root.Content[i+1], "requestBodies") + responsesNode := effectiveMappingValue(root.Content[i+1], "responses") + securitySchemesNode := effectiveMappingValue(root.Content[i+1], "securitySchemes") + headersNode := effectiveMappingValue(root.Content[i+1], "headers") + examplesNode := effectiveMappingValue(root.Content[i+1], "examples") + linksNode := effectiveMappingValue(root.Content[i+1], "links") + callbacksNode := effectiveMappingValue(root.Content[i+1], "callbacks") + pathItemsNode := effectiveMappingValue(root.Content[i+1], "pathItems") if schemasNode != nil { index.extractDefinitionsAndSchemas(schemasNode, "#/components/schemas/") @@ -430,7 +431,7 @@ func (index *SpecIndex) GetComponentSchemaCount() int { } } if n.Value == "definitions" { - schemasNode := index.root.Content[0].Content[i+1] + schemasNode := utils.ResolveMergeNodes(root.Content[i+1]) if schemasNode != nil { index.extractDefinitionsAndSchemas(schemasNode, "#/definitions/") index.schemasNode = schemasNode @@ -438,7 +439,7 @@ func (index *SpecIndex) GetComponentSchemaCount() int { } } if n.Value == "parameters" { - parametersNode := index.root.Content[0].Content[i+1] + parametersNode := utils.ResolveMergeNodes(root.Content[i+1]) if parametersNode != nil { index.extractComponentParameters(parametersNode, "#/parameters/") index.componentLock.Lock() @@ -447,14 +448,14 @@ func (index *SpecIndex) GetComponentSchemaCount() int { } } if n.Value == "responses" { - responsesNode := index.root.Content[0].Content[i+1] + responsesNode := utils.ResolveMergeNodes(root.Content[i+1]) if responsesNode != nil { index.extractComponentResponses(responsesNode, "#/responses/") index.responsesNode = responsesNode } } if n.Value == "securityDefinitions" { - securityDefinitionsNode := index.root.Content[0].Content[i+1] + securityDefinitionsNode := utils.ResolveMergeNodes(root.Content[i+1]) if securityDefinitionsNode != nil { index.extractComponentSecuritySchemes(securityDefinitionsNode, "#/securityDefinitions/") index.securitySchemesNode = securityDefinitionsNode @@ -472,10 +473,11 @@ func (index *SpecIndex) GetComponentParameterCount() int { if index.componentParamCount > 0 { return index.componentParamCount } - for i, n := range index.root.Content[0].Content { + root := utils.ResolveMergeNodes(index.root.Content[0]) + for i, n := range root.Content { if i%2 == 0 { if n.Value == "components" { - _, parametersNode := utils.FindKeyNode("parameters", index.root.Content[0].Content[i+1].Content) + parametersNode := effectiveMappingValue(root.Content[i+1], "parameters") if parametersNode != nil { index.componentLock.Lock() index.parametersNode = parametersNode @@ -484,7 +486,7 @@ func (index *SpecIndex) GetComponentParameterCount() int { } } if n.Value == "parameters" { - parametersNode := index.root.Content[0].Content[i+1] + parametersNode := utils.ResolveMergeNodes(root.Content[i+1]) if parametersNode != nil { index.componentLock.Lock() index.parametersNode = parametersNode @@ -720,3 +722,15 @@ func (index *SpecIndex) GetAllDescriptionsCount() int { return len(index.allDesc // GetAllSummariesCount returns the total number of summary nodes found during indexing. func (index *SpecIndex) GetAllSummariesCount() int { return len(index.allSummaries) } + +// effectiveMappingValue reads a named field and resolves its collection view. +// Both the containing mapping and the field may independently use YAML merges +// or aliases; callers enumerate the same effective entries as model builders. +func effectiveMappingValue(node *yaml.Node, key string) *yaml.Node { + node = utils.ResolveMergeNodes(node) + if node == nil { + return nil + } + _, value := utils.FindKeyNodeTop(key, node.Content) + return utils.ResolveMergeNodes(value) +} diff --git a/utils/merge_view_test.go b/utils/merge_view_test.go index 80fe343d..7772aed1 100644 --- a/utils/merge_view_test.go +++ b/utils/merge_view_test.go @@ -64,3 +64,21 @@ func TestResolvedMergeNodeMappingCycleTerminates(t *testing.T) { assert.Equal(t, []*yaml.Node{key, value}, view.Content) assert.Len(t, node.Content, 4) } + +func TestResolvedMergeNodeSequenceCycleTerminates(t *testing.T) { + const source = `base: &base {inherited: first} +sequence: &sequence [*sequence, *base] +merged: {<<: *sequence, local: retained} +` + var document yaml.Node + require.NoError(t, yaml.Unmarshal([]byte(source), &document)) + _, original := FindKeyNodeTop("merged", document.Content[0].Content) + view := ResolveMergeNodes(original) + _, inherited := FindKeyNodeTop("inherited", view.Content) + require.NotNil(t, inherited) + assert.Equal(t, "first", inherited.Value) + _, local := FindKeyNodeTop("local", view.Content) + require.NotNil(t, local) + assert.Equal(t, "retained", local.Value) + assert.True(t, hasMergeKeys(original.Content)) +} diff --git a/utils/utils.go b/utils/utils.go index aa0a5759..8f8e9363 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -411,6 +411,11 @@ func appendExpandedMergeContent( } if mergeValue.Kind == yaml.SequenceNode { + if _, active := visited[mergeValue]; active { + return target + } + visited[mergeValue] = struct{}{} + defer delete(visited, mergeValue) for _, item := range mergeValue.Content { target = appendExpandedMergeContent(target, seenKeys, item, visited) } From 68e6f5de97e53ccb50db836cf5a5b8a447115a38 Mon Sep 17 00:00:00 2001 From: quobix Date: Sat, 31 Oct 2026 10:58:59 -0400 Subject: [PATCH 11/33] fix(index): bound YAML alias traversal and report incomplete indexes --- document_alias_limit_test.go | 53 +++++++++++++++++++++ index/extract_refs.go | 54 +++++++++++++++++++--- index/extract_refs_alias_limit_test.go | 64 ++++++++++++++++++++++++++ index/extract_refs_walk.go | 7 ++- index/rolodex.go | 10 ++++ 5 files changed, 179 insertions(+), 9 deletions(-) create mode 100644 document_alias_limit_test.go create mode 100644 index/extract_refs_alias_limit_test.go diff --git a/document_alias_limit_test.go b/document_alias_limit_test.go new file mode 100644 index 00000000..5e6ef3ec --- /dev/null +++ b/document_alias_limit_test.go @@ -0,0 +1,53 @@ +package libopenapi + +import ( + "os" + "path/filepath" + "testing" + + "github.com/pb33f/libopenapi/datamodel" + "github.com/pb33f/testify/require" +) + +func TestBuildV3ModelReportsAliasLimit(t *testing.T) { + // A small shared graph crosses the parser's alias ratio limit during indexing. + // It has no references, so only the traversal limit can report this error. + const source = `openapi: 3.1.1 +info: {title: Aliases, version: '1'} +paths: {} +x-a: &a [value] +x-b: &b [*a, *a] +x-c: &c [*b, *b] +x-d: &d [*c, *c] +x-e: &e [*d, *d] +x-f: &f [*e, *e] +x-g: &g [*f, *f] +x-h: &h [*g, *g] +x-i: &i [*h, *h] +x-j: &j [*i, *i] +x-k: &k [*j, *j] +` + for _, location := range []string{"root", "external"} { + t.Run(location, func(t *testing.T) { + for _, skipCircular := range []bool{false, true} { + config := datamodel.NewDocumentConfiguration() + config.SkipCircularReferenceCheck = skipCircular + input := source + if location == "external" { + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "shared.yaml"), []byte(source+"components: {schemas: {Target: {type: string}}}\n"), 0600)) + config.BasePath = dir + config.AllowFileReferences = true + input = "openapi: 3.1.1\ninfo: {title: Aliases, version: '1'}\npaths: {}\ncomponents: {schemas: {Target: {$ref: './shared.yaml#/components/schemas/Target'}}}\n" + } + doc, err := NewDocumentWithConfiguration([]byte(input), config) + require.NoError(t, err) + for range 2 { + model, buildErr := doc.BuildV3Model() + require.ErrorContains(t, buildErr, "document contains excessive aliasing") + require.Nil(t, model) + } + } + }) + } +} diff --git a/index/extract_refs.go b/index/extract_refs.go index e466bb76..2b764ad1 100644 --- a/index/extract_refs.go +++ b/index/extract_refs.go @@ -5,11 +5,16 @@ package index import ( "context" + "strings" "github.com/pb33f/go-yaml" + "github.com/pb33f/go-yaml/plugin/limit" "github.com/pb33f/libopenapi/utils" ) +// Use the parser's existing alias policy when traversing its unexpanded node tree. +var extractRefsAliasLimit limit.Plugin + func isSchemaContainingNode(v string) bool { switch v { case "schema", "items", "additionalProperties", "contains", "not", @@ -99,25 +104,60 @@ func isDirectOpenAPIExampleValuePath(seenPath []string) bool { // ExtractRefs will return a deduplicated slice of references for every unique ref found in the document. // The total number of refs, will generally be much higher, you can extract those from GetRawReferenceCount() +// Excessive YAML alias expansion stops extraction and is reported by GetReferenceIndexErrors. func (index *SpecIndex) ExtractRefs(ctx context.Context, node, parent *yaml.Node, seenPath []string, level int, poly bool, pName string) []*Reference { - return index.extractRefs(ctx, node, parent, seenPath, level, poly, pName, make(map[*yaml.Node]struct{})) + walk := &extractRefsTraversal{ancestry: make(map[*yaml.Node]struct{})} + return index.extractRefs(ctx, node, parent, seenPath, level, poly, pName, walk) +} + +type extractRefsTraversal struct { + ancestry map[*yaml.Node]struct{} + nodes int + aliasNodes int + aliasDepth int + stopped bool +} + +func (walk *extractRefsTraversal) visit() error { + walk.nodes++ + if walk.aliasDepth > 0 { + walk.aliasNodes++ + } + return extractRefsAliasLimit.CheckAlias(walk.aliasNodes, walk.nodes) } // Ancestry tracks canonical source nodes, not the shallow views created during // merge expansion. A shared anchor is visited again at each distinct path. -func (index *SpecIndex) extractRefs(ctx context.Context, node, parent *yaml.Node, seenPath []string, level int, poly bool, pName string, ancestry map[*yaml.Node]struct{}) []*Reference { - node = utils.NodeAlias(node) +func (index *SpecIndex) extractRefs(ctx context.Context, node, parent *yaml.Node, seenPath []string, level int, poly bool, pName string, walk *extractRefsTraversal) []*Reference { + if node == nil || walk.stopped { + return nil + } + err := walk.visit() + if node.Kind == yaml.AliasNode && err == nil { + walk.aliasDepth++ + defer func() { walk.aliasDepth-- }() + node = utils.NodeAlias(node) + err = walk.visit() + } + if err != nil { + walk.stopped = true + // A stopped traversal cannot provide a complete model. + index.refErrors = append(index.refErrors, &ResolvingError{ + ErrorRef: err, Node: node, Path: "$." + strings.Join(seenPath, "."), + }) + return nil + } if node == nil { return nil } - if _, active := ancestry[node]; active { + if _, active := walk.ancestry[node]; active { return nil } - ancestry[node] = struct{}{} - defer delete(ancestry, node) + walk.ancestry[node] = struct{}{} + defer delete(walk.ancestry, node) view := utils.ResolveMergeNodes(node) state := index.initializeExtractRefsState(ctx, view, seenPath, level, poly, pName) - state.ancestry = ancestry + state.traversal = walk found := index.walkExtractRefs(view, parent, &state) index.refCount = len(index.allRefs) return found diff --git a/index/extract_refs_alias_limit_test.go b/index/extract_refs_alias_limit_test.go new file mode 100644 index 00000000..6a1e8202 --- /dev/null +++ b/index/extract_refs_alias_limit_test.go @@ -0,0 +1,64 @@ +package index + +import ( + "context" + "strings" + "testing" + + "github.com/pb33f/go-yaml" + "github.com/pb33f/testify/require" +) + +func TestExtractRefsStopsAtAliasLimit(t *testing.T) { + var root yaml.Node + require.NoError(t, yaml.Unmarshal([]byte("type: string\n"), &root)) + idx := NewSpecIndex(&root) + // The parser starts checking the alias ratio after 1,000 node visits. + walk := &extractRefsTraversal{ + ancestry: make(map[*yaml.Node]struct{}), + nodes: 1000, aliasNodes: 1000, aliasDepth: 1, + } + require.Nil(t, idx.extractRefs(context.Background(), root.Content[0], &root, []string{"x-shared"}, 0, false, "", walk)) + require.True(t, walk.stopped) + require.Len(t, idx.GetReferenceIndexErrors(), 1) + err := idx.GetReferenceIndexErrors()[0].(*ResolvingError) + require.ErrorContains(t, err, "document contains excessive aliasing") + require.Same(t, root.Content[0], err.Node) + require.Equal(t, "$.x-shared", err.Path) + // Once stopped, later siblings do not continue work or add duplicate errors. + require.Nil(t, idx.extractRefs(context.Background(), root.Content[0], &root, nil, 0, false, "", walk)) + require.Len(t, idx.GetReferenceIndexErrors(), 1) + require.Equal(t, 1001, walk.nodes) +} + +func TestExtractRefsAbsentAliasTarget(t *testing.T) { + idx := NewSpecIndex(nil) + for _, node := range []*yaml.Node{nil, {Kind: yaml.AliasNode}} { + require.Nil(t, idx.ExtractRefs(context.Background(), node, nil, nil, 0, false, "")) + } + require.Empty(t, idx.GetReferenceIndexErrors()) +} + +func TestExtractRefsAllowsOrdinaryAliases(t *testing.T) { + const source = `openapi: 3.1.1 +info: {title: Aliases, version: '1'} +paths: {} +x-meta: &meta {description: shared metadata} +x-ref: &ref {$ref: '#/components/schemas/Target'} +components: + schemas: + Target: {type: string} + First: {type: object, properties: {value: *ref}, x-meta: *meta} + Second: {type: object, properties: {value: *ref}, x-meta: *meta} +` + var root yaml.Node + require.NoError(t, yaml.Unmarshal([]byte(source), &root)) + idx := NewSpecIndex(&root) + require.Empty(t, idx.GetReferenceIndexErrors()) + paths := make(map[string]bool) + for _, ref := range idx.GetRawReferencesSequenced() { + paths[strings.Join(ref.SourcePath, "/")] = true + } + require.Contains(t, paths, "components/schemas/First/properties/value") + require.Contains(t, paths, "components/schemas/Second/properties/value") +} diff --git a/index/extract_refs_walk.go b/index/extract_refs_walk.go index 18558736..54da4afd 100644 --- a/index/extract_refs_walk.go +++ b/index/extract_refs_walk.go @@ -12,7 +12,7 @@ import ( ) type extractRefsState struct { - ancestry map[*yaml.Node]struct{} + traversal *extractRefsTraversal ctx context.Context scope *SchemaIdScope parentBaseURI string @@ -69,6 +69,9 @@ func (index *SpecIndex) walkExtractRefs(node, parent *yaml.Node, state *extractR var found []*Reference for i, n := range node.Content { + if state.traversal.stopped { + break + } // In YAML mapping nodes, Content alternates key-value: even indices (0, 2, 4...) // are keys, odd indices (1, 3, 5...) are values. if i%2 == 0 { @@ -101,7 +104,7 @@ func (index *SpecIndex) walkChildExtractRefs(node, parent *yaml.Node, state *ext state.polyName = state.prev } } - return index.extractRefs(state.ctx, node, parent, state.seenPath, state.level, state.poly, state.polyName, state.ancestry) + return index.extractRefs(state.ctx, node, parent, state.seenPath, state.level, state.poly, state.polyName, state.traversal) } func isDirectOpenAPIExampleRefNode(node *yaml.Node) bool { diff --git a/index/rolodex.go b/index/rolodex.go index 632a2bb9..68dbb8f4 100644 --- a/index/rolodex.go +++ b/index/rolodex.go @@ -557,6 +557,16 @@ func (r *Rolodex) IndexTheRolodex(ctx context.Context) error { caughtErrors = append(caughtErrors, index.refErrors...) } } + // Root reference lookup can load additional files after the initial build queue. + // Include fatal extraction errors from those indexes before returning a model. + for _, idx := range r.GetIndexes() { + for _, err := range idx.refErrors { + var resolvingErr *ResolvingError + if errors.As(err, &resolvingErr) { + caughtErrors = append(caughtErrors, err) + } + } + } r.indexingDuration = time.Since(started) r.indexed = true r.caughtErrors = caughtErrors From dd8ee6aee487753a6c26c4d9cc610e03bde672b4 Mon Sep 17 00:00:00 2001 From: quobix Date: Sat, 31 Oct 2026 10:59:07 -0400 Subject: [PATCH 12/33] fix(utils): expand each shared YAML merge source once --- utils/merge_view_test.go | 34 ++++++++++++++++++++++++++++++++++ utils/utils.go | 6 +++--- 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/utils/merge_view_test.go b/utils/merge_view_test.go index 7772aed1..01e57784 100644 --- a/utils/merge_view_test.go +++ b/utils/merge_view_test.go @@ -82,3 +82,37 @@ merged: {<<: *sequence, local: retained} assert.Equal(t, "retained", local.Value) assert.True(t, hasMergeKeys(original.Content)) } + +func TestResolvedMergeNodeSharedSourcesKeepPrecedence(t *testing.T) { + const source = `base: &base {inherited: base, local: base} +first: &first {<<: *base, inherited: first} +second: &second {<<: *base, inherited: second, extra: retained} +shared: &shared [*first, *second] +repeated: &repeated [*shared, *shared] +merged: {<<: *repeated, local: chosen} +` + var document yaml.Node + require.NoError(t, yaml.Unmarshal([]byte(source), &document)) + before, err := yaml.Marshal(&document) + require.NoError(t, err) + _, original := FindKeyNodeTop("merged", document.Content[0].Content) + visited := make(map[*yaml.Node]struct{}) + content := expandMergeContent(original, visited) + for key, expected := range map[string]string{ + "inherited": "first", "local": "chosen", "extra": "retained", + } { + _, value := FindKeyNodeTop(key, content) + require.NotNil(t, value) + require.Equal(t, expected, value.Value) + } + require.Len(t, content, 6) + // Reaching a completed source again in this expansion does no further work. + require.Empty(t, expandMergeContent(original, visited)) + _, repeated := FindKeyNodeTop("repeated", document.Content[0].Content) + require.Empty(t, appendExpandedMergeContent(nil, make(map[string]struct{}), repeated, visited)) + // A new expansion still gives the same effective mapping. + require.Equal(t, content, ResolveMergeNodes(original).Content) + after, err := yaml.Marshal(&document) + require.NoError(t, err) + require.Equal(t, before, after) +} diff --git a/utils/utils.go b/utils/utils.go index 8f8e9363..8e61bbdb 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -364,7 +364,8 @@ func expandMergeContent(node *yaml.Node, visited map[*yaml.Node]struct{}) []*yam return nil } visited[node] = struct{}{} - defer delete(visited, node) + // Earlier merge sources win. Revisiting this mapping cannot add a key + // that was not already supplied or overridden by its first expansion. if !hasMergeKeys(node.Content) { return node.Content @@ -411,11 +412,10 @@ func appendExpandedMergeContent( } if mergeValue.Kind == yaml.SequenceNode { - if _, active := visited[mergeValue]; active { + if _, seen := visited[mergeValue]; seen { return target } visited[mergeValue] = struct{}{} - defer delete(visited, mergeValue) for _, item := range mergeValue.Content { target = appendExpandedMergeContent(target, seenKeys, item, visited) } From 737e6654cf5e653f2c730798f42d6126f4e96f32 Mon Sep 17 00:00:00 2001 From: quobix Date: Sat, 31 Oct 2026 11:02:34 -0400 Subject: [PATCH 13/33] fix(index): skip polymorphic fields inside example payloads --- document_resolver_regression_test.go | 19 +++++++++++++++++++ index/resolver_relatives.go | 3 +++ 2 files changed, 22 insertions(+) diff --git a/document_resolver_regression_test.go b/document_resolver_regression_test.go index 713ac33d..5e74fd7d 100644 --- a/document_resolver_regression_test.go +++ b/document_resolver_regression_test.go @@ -111,3 +111,22 @@ paths: } } } + +func TestBuildV3Model_ExamplePolymorphicPayload(t *testing.T) { + doc, err := NewDocument([]byte(`openapi: 3.1.0 +info: {title: Examples, version: '1'} +paths: {} +components: + schemas: + A: + type: object + example: + oneOf: + - {$ref: '#/components/schemas/A'} + Ref: {$ref: '#/components/schemas/A'} +`)) + require.NoError(t, err) + model, err := doc.BuildV3Model() + require.NoError(t, err) + require.Empty(t, model.Index.GetCircularReferences()) +} diff --git a/index/resolver_relatives.go b/index/resolver_relatives.go index a02635e3..dfaf1d9a 100644 --- a/index/resolver_relatives.go +++ b/index/resolver_relatives.go @@ -41,6 +41,9 @@ func (resolver *Resolver) extractRelativesWithState( if underOpenAPIExamplePayloadPath(state.path) { return nil } + if isDirectOpenAPIExampleValuePath(state.path) && !isDirectOpenAPIExampleRefNode(node) { + return nil + } // Guard against stack overflow from deeply nested or circular specs. // Journey tracks the reference chain (100 max); depth tracks recursive calls (500 max). if len(state.journey) > 100 { From 9543c8f96e3af37bbb49fd2ea6730b3dd9a1d870 Mon Sep 17 00:00:00 2001 From: quobix Date: Sat, 31 Oct 2026 11:02:58 -0400 Subject: [PATCH 14/33] fix(bundler): preserve encoded paths and shared alias targets --- bundler/bundler_composer.go | 18 +++- bundler/composer_functions.go | 10 +- bundler/external_regression_test.go | 152 ++++++++++++++++++++++++++++ 3 files changed, 177 insertions(+), 3 deletions(-) create mode 100644 bundler/external_regression_test.go diff --git a/bundler/bundler_composer.go b/bundler/bundler_composer.go index eeb65fa7..cb5f7ef4 100644 --- a/bundler/bundler_composer.go +++ b/bundler/bundler_composer.go @@ -68,6 +68,9 @@ func handleIndex(c *handleIndexConfig) error { continue } mappedReference := mappedReferences[sequenced.FullDefinition] + if mappedReference == nil { + mappedReference, _ = c.idx.SearchIndexForReference(sequenced.FullDefinition) + } // Check for invalid sibling properties if strict validation is enabled if c.compositionConfig.StrictValidation && @@ -188,6 +191,14 @@ func processReference(model *v3.Document, pr *processRef, cf *handleIndexConfig) } model.Components = components } + // An alias contributes no component of its own. Inline its reference after + // the target has been composed, so both paths use the same component. + isAlias, _, _ := utils.IsNodeRefValue(pr.ref.Node) + isAlias = isAlias && len(pr.ref.Node.Content) == 2 + if !pr.fromDiscriminator && isAlias { + cf.inlineRequired = append(cf.inlineRequired, pr) + return nil + } var location []string if strings.Contains(pr.ref.FullDefinition, "#/") { @@ -198,7 +209,9 @@ func processReference(model *v3.Document, pr *processRef, cf *handleIndexConfig) // Bare-file imports need the sequenced absolute definition so composition // keys and later rewrites point at the same target. pr.ref.FullDefinition = pr.seqRef.FullDefinition - if importType, ok := inferComponentTypeFromSourcePath(pr.seqRef.SourcePath); ok && + if pr.fromDiscriminator && isAlias { + _, location = fileImportLocationForType(v3low.SchemasLabel, components, pr, cf) + } else if importType, ok := inferComponentTypeFromSourcePath(pr.seqRef.SourcePath); ok && canComposeContextualReference(importType, pr.ref.Node, true) { _, location = fileImportLocationForType(importType, components, pr, cf) } else if importType, ok := DetectOpenAPIComponentType(pr.ref.Node); ok { @@ -383,7 +396,8 @@ func enqueueDiscriminatorMappingTargets( mapKey := processRefMapKeyForComponent(ref, v3low.SchemasLabel) // Skip targets already queued for composition. - if cf.refMap.GetOrZero(mapKey) != nil { + if existing := cf.refMap.GetOrZero(mapKey); existing != nil { + existing.fromDiscriminator = true continue } diff --git a/bundler/composer_functions.go b/bundler/composer_functions.go index 432b551a..fb720a2c 100644 --- a/bundler/composer_functions.go +++ b/bundler/composer_functions.go @@ -52,7 +52,15 @@ func processRefMapKey(target, source *index.Reference) string { if fullDefinition == "" && source != nil { fullDefinition = source.FullDefinition } - return contextualProcessRefKey(fullDefinition, source) + key := contextualProcessRefKey(fullDefinition, source) + if key == fullDefinition && target != nil && !isExplicitComponentDefinition(fullDefinition) { + // A whole-file alias has no source path. Use the same detected bucket + // that composition uses, so it shares the target with contextual refs. + if componentType, ok := DetectOpenAPIComponentType(target.Node); ok { + return fullDefinition + contextualRefKeySeparator + componentType + } + } + return key } // processRefMapKeyForComponent scopes a target ref by an already-known component bucket. diff --git a/bundler/external_regression_test.go b/bundler/external_regression_test.go new file mode 100644 index 00000000..2b869089 --- /dev/null +++ b/bundler/external_regression_test.go @@ -0,0 +1,152 @@ +package bundler + +import ( + "fmt" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/pb33f/go-yaml" + "github.com/pb33f/libopenapi" + "github.com/pb33f/libopenapi/datamodel" + "github.com/pb33f/libopenapi/utils" + "github.com/pb33f/testify/require" +) + +func externalRegressionConfig(t *testing.T, files map[string]string) *datamodel.DocumentConfiguration { + t.Helper() + dir := t.TempDir() + for name, content := range files { + path := filepath.Join(dir, name) + require.NoError(t, os.MkdirAll(filepath.Dir(path), 0755)) + require.NoError(t, os.WriteFile(path, []byte(content), 0600)) + } + return &datamodel.DocumentConfiguration{BasePath: dir, AllowFileReferences: true} +} + +func TestBundleBytesComposed_EncodedPathItem(t *testing.T) { + config := externalRegressionConfig(t, map[string]string{"paths.yaml": `openapi: 3.1.0 +info: {title: Paths, version: "1"} +paths: + /items/{id}: + get: + parameters: [{name: id, in: path, required: true, schema: {type: string}}] + responses: + '200': {description: ok} +`}) + root := []byte(`openapi: 3.1.0 +info: {title: Encoded paths, version: "1"} +paths: + /items/{id}: {$ref: './paths.yaml#/paths/~1items~1%7Bid%7D'} +`) + out, err := BundleBytesComposed(root, config, nil) + require.NoError(t, err) + var bundled yaml.Node + require.NoError(t, yaml.Unmarshal(out, &bundled)) + _, paths := utils.FindKeyNodeTop("paths", bundled.Content[0].Content) + require.NotNil(t, paths) + _, pathItem := utils.FindKeyNodeTop("/items/{id}", paths.Content) + require.NotNil(t, pathItem) + require.Nil(t, utils.GetRefValueNode(pathItem), "path item must contain the operation, not a self-reference") + doc, err := libopenapi.NewDocument(out) + require.NoError(t, err) + model, err := doc.BuildV3Model() + require.NoError(t, err) + item := model.Model.Paths.PathItems.GetOrZero("/items/{id}") + require.NotNil(t, item.Get) + require.Len(t, item.Get.Parameters, 1) +} + +func TestBundleBytesComposed_SharedAliasTarget(t *testing.T) { + config := externalRegressionConfig(t, map[string]string{ + "schemas/List.yaml": "type: object\nproperties:\n data:\n type: array\n items: {$ref: ./Item.yaml}\n", + "schemas/Alias.yaml": "$ref: ./Item.yaml\n", + "schemas/Item.yaml": "type: object\nproperties:\n id: {type: string}\n", + }) + root := []byte(`openapi: 3.1.0 +info: {title: Shared aliases, version: "1"} +paths: + /items: + get: + responses: + '200': + description: ok + content: {application/json: {schema: {$ref: ./schemas/List.yaml}}} + /item: + get: + responses: + '200': + description: ok + content: {application/json: {schema: {$ref: ./schemas/Alias.yaml}}} +`) + out, err := BundleBytesComposed(root, config, nil) + require.NoError(t, err) + doc, err := libopenapi.NewDocument(out) + require.NoError(t, err) + model, err := doc.BuildV3Model() + require.NoError(t, err) + list := model.Model.Components.Schemas.GetOrZero("List").Schema() + require.NotNil(t, list.Properties.GetOrZero("data"), string(out)) + require.Equal(t, 2, model.Model.Components.Schemas.Len(), string(out)) + item := list.Properties.GetOrZero("data").Schema().Items.A.Schema() + require.Equal(t, []string{"object"}, item.Type) + require.NotNil(t, item.Properties.GetOrZero("id")) +} + +func TestBundleBytes_RecursiveSchemaAtDifferentDepths(t *testing.T) { + for depth := 0; depth <= 5; depth++ { + t.Run(fmt.Sprint(depth), func(t *testing.T) { + files := map[string]string{ + "schemas/Step.yaml": "properties:\n steps:\n items: {$ref: ./Step.yaml}\n", + "schemas/V.yaml": "properties:\n steps:\n items: {$ref: ./Step.yaml}\n styles: {$ref: ./W.yaml}\n", + "schemas/W.yaml": "properties:\n c: {$ref: ./E.yaml}\n", + "schemas/E.yaml": "{}\n", + } + ref := "V.yaml" + for i := depth; i > 0; i-- { + name := fmt.Sprintf("A%d.yaml", i) + files["schemas/"+name] = "properties:\n p: {$ref: ./" + ref + "}\n" + ref = name + } + files["paths/item.yaml"] = "post:\n requestBody:\n content:\n application/json:\n schema: {$ref: ../schemas/" + ref + "}\n responses:\n '200': {description: ok}\n" + config := externalRegressionConfig(t, files) + root := []byte("openapi: 3.0.3\ninfo: {title: Nested recursion, version: '1'}\npaths:\n /items: {$ref: ./paths/item.yaml}\n") + out, err := BundleBytes(root, config) + require.NoError(t, err) + doc, err := libopenapi.NewDocument(out) + require.NoError(t, err) + model, err := doc.BuildV3Model() + require.NoError(t, err) + step := model.Model.Components.Schemas.GetOrZero("Step") + require.NotNil(t, step, string(out)) + require.Equal(t, "#/components/schemas/Step", step.Schema().Properties.GetOrZero("steps").Schema().Items.A.GetReference()) + }) + } +} + +func TestBundleBytesComposed_AliasDiscriminatorMapping(t *testing.T) { + cfg := externalRegressionConfig(t, map[string]string{"schemas/Alias.yaml": "$ref: ./Dog.yaml\n", "schemas/Dog.yaml": "type: object\nproperties:\n kind: {type: string, enum: [dog]}\n"}) + out, err := BundleBytesComposed([]byte(`openapi: 3.1.0 +info: {title: Aliases, version: '1'} +paths: {} +components: + schemas: + Animal: + oneOf: + - {$ref: ./schemas/Alias.yaml} + discriminator: + propertyName: kind + mapping: + dog: ./schemas/Alias.yaml +`), cfg, nil) + require.NoError(t, err) + + doc, err := libopenapi.NewDocument(out) + require.NoError(t, err) + model, err := doc.BuildV3Model() + require.NoError(t, err) + mapping := model.Model.Components.Schemas.GetOrZero("Animal").Schema().Discriminator.Mapping.GetOrZero("dog") + require.True(t, strings.HasPrefix(mapping, "#/components/schemas/"), mapping) + require.NotNil(t, model.Model.Components.Schemas.GetOrZero(strings.TrimPrefix(mapping, "#/components/schemas/")), mapping) +} From d3746fb4b48847c7c323ccd1fb6afde1c8526b42 Mon Sep 17 00:00:00 2001 From: quobix Date: Sat, 31 Oct 2026 11:06:57 -0400 Subject: [PATCH 15/33] test: preserve integer parsing compatibility and simplify path errors --- datamodel/low/integer_decimal_test.go | 6 ++++++ datamodel/low/v2/paths.go | 3 --- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/datamodel/low/integer_decimal_test.go b/datamodel/low/integer_decimal_test.go index 44ebbc84..f0a215de 100644 --- a/datamodel/low/integer_decimal_test.go +++ b/datamodel/low/integer_decimal_test.go @@ -33,6 +33,12 @@ func TestBuildModel_ExactDecimalIntegers(t *testing.T) { {"100.0e-2", 1, true}, {"100.0e-3", 0, false}, {".0", 0, true}, + {"Inf", math.MaxInt64, true}, + {"-Inf", math.MinInt64, true}, + {"0x1p4", 16, true}, + {"0x1p64", math.MaxInt64, true}, + {"-0x1p64", math.MinInt64, true}, + {"0x1.8p0", 0, false}, } { t.Run(tc.value, func(t *testing.T) { // Explicit tags also exercise overflow literals that YAML's implicit diff --git a/datamodel/low/v2/paths.go b/datamodel/low/v2/paths.go index c209c33a..742822a0 100644 --- a/datamodel/low/v2/paths.go +++ b/datamodel/low/v2/paths.go @@ -134,9 +134,6 @@ func (p *Paths) Build(ctx context.Context, _, root *yaml.Node, idx *index.SpecIn if err != nil { return pathBuildResult{}, fmt.Errorf("path item build failed: %w", err) } - if resolved == nil { - return pathBuildResult{}, fmt.Errorf("path item build failed: cannot find reference: %q", ref) - } pNode, pathIdx, pathCtx = resolved, foundIdx, foundCtx } path := new(PathItem) From ee09a3a829d78bcf3af2d6e8cc876af5f010676f Mon Sep 17 00:00:00 2001 From: quobix Date: Sat, 31 Oct 2026 11:10:14 -0400 Subject: [PATCH 16/33] fix(index): count inherited merge traversal against alias limit --- document_alias_limit_test.go | 25 +++++++++++++++++ index/extract_refs.go | 11 ++++++++ index/extract_refs_alias_limit_test.go | 38 ++++++++++++++++++++++++++ index/extract_refs_walk.go | 8 ++++++ utils/utils.go | 1 + 5 files changed, 83 insertions(+) diff --git a/document_alias_limit_test.go b/document_alias_limit_test.go index 5e6ef3ec..6ea315a6 100644 --- a/document_alias_limit_test.go +++ b/document_alias_limit_test.go @@ -51,3 +51,28 @@ x-k: &k [*j, *j] }) } } + +func TestBuildV3ModelReportsMergeAliasLimit(t *testing.T) { + const source = `openapi: 3.1.1 +info: {title: Merge aliases, version: '1'} +paths: {} +x-a: &a {leaf: value} +x-b: &b {left: {<<: *a}, right: {<<: *a}} +x-c: &c {left: {<<: *b}, right: {<<: *b}} +x-d: &d {left: {<<: *c}, right: {<<: *c}} +x-e: &e {left: {<<: *d}, right: {<<: *d}} +x-f: &f {left: {<<: *e}, right: {<<: *e}} +x-g: &g {left: {<<: *f}, right: {<<: *f}} +x-h: &h {left: {<<: *g}, right: {<<: *g}} +x-i: &i {left: {<<: *h}, right: {<<: *h}} +x-j: &j {left: {<<: *i}, right: {<<: *i}} +x-k: &k {left: {<<: *j}, right: {<<: *j}} +` + doc, err := NewDocument([]byte(source)) + require.NoError(t, err) + for range 2 { + model, buildErr := doc.BuildV3Model() + require.ErrorContains(t, buildErr, "document contains excessive aliasing") + require.Nil(t, model) + } +} diff --git a/index/extract_refs.go b/index/extract_refs.go index 2b764ad1..2dde8a08 100644 --- a/index/extract_refs.go +++ b/index/extract_refs.go @@ -158,6 +158,17 @@ func (index *SpecIndex) extractRefs(ctx context.Context, node, parent *yaml.Node view := utils.ResolveMergeNodes(node) state := index.initializeExtractRefsState(ctx, view, seenPath, level, poly, pName) state.traversal = walk + state.mergeStart = len(view.Content) + if view != node { + // Merge views retain explicit pairs first, then append inherited pairs. + // Only inherited descendants consume the alias expansion budget. + state.mergeStart = 0 + for i := 0; i < len(node.Content); i += 2 { + if key := node.Content[i]; key != nil && key.Tag != "!!merge" { + state.mergeStart += 2 + } + } + } found := index.walkExtractRefs(view, parent, &state) index.refCount = len(index.allRefs) return found diff --git a/index/extract_refs_alias_limit_test.go b/index/extract_refs_alias_limit_test.go index 6a1e8202..f2229f03 100644 --- a/index/extract_refs_alias_limit_test.go +++ b/index/extract_refs_alias_limit_test.go @@ -39,17 +39,53 @@ func TestExtractRefsAbsentAliasTarget(t *testing.T) { require.Empty(t, idx.GetReferenceIndexErrors()) } +func TestExtractRefsCountsInheritedMergeDescendants(t *testing.T) { + const source = `shared: &shared {inherited: {nested: {type: string}}} +target: + before: {type: string} + <<: *shared + after: {type: integer} +` + var root yaml.Node + require.NoError(t, yaml.Unmarshal([]byte(source), &root)) + target := root.Content[0].Content[3] + t.Run("only inherited descendants", func(t *testing.T) { + idx := NewSpecIndex(nil) + walk := &extractRefsTraversal{ancestry: make(map[*yaml.Node]struct{}), nodes: 1000} + idx.extractRefs(context.Background(), target, nil, nil, 0, false, "", walk) + require.False(t, walk.stopped) + require.Empty(t, idx.GetReferenceIndexErrors()) + require.Equal(t, 1005, walk.nodes) + require.Equal(t, 2, walk.aliasNodes) + require.Zero(t, walk.aliasDepth) + }) + t.Run("inherited descendants reach limit", func(t *testing.T) { + idx := NewSpecIndex(nil) + walk := &extractRefsTraversal{ancestry: make(map[*yaml.Node]struct{}), nodes: 997, aliasNodes: 997} + idx.extractRefs(context.Background(), target, nil, []string{"target"}, 0, false, "", walk) + require.True(t, walk.stopped) + require.Len(t, idx.GetReferenceIndexErrors(), 1) + require.ErrorContains(t, idx.GetReferenceIndexErrors()[0], "document contains excessive aliasing") + require.Equal(t, 1001, walk.nodes) + require.Equal(t, 998, walk.aliasNodes) + require.Zero(t, walk.aliasDepth) + }) +} + func TestExtractRefsAllowsOrdinaryAliases(t *testing.T) { const source = `openapi: 3.1.1 info: {title: Aliases, version: '1'} paths: {} x-meta: &meta {description: shared metadata} x-ref: &ref {$ref: '#/components/schemas/Target'} +x-props: &props {value: *ref} components: schemas: Target: {type: string} First: {type: object, properties: {value: *ref}, x-meta: *meta} Second: {type: object, properties: {value: *ref}, x-meta: *meta} + MergedFirst: {type: object, properties: {<<: *props}} + MergedSecond: {type: object, properties: {<<: *props}} ` var root yaml.Node require.NoError(t, yaml.Unmarshal([]byte(source), &root)) @@ -61,4 +97,6 @@ components: } require.Contains(t, paths, "components/schemas/First/properties/value") require.Contains(t, paths, "components/schemas/Second/properties/value") + require.Contains(t, paths, "components/schemas/MergedFirst/properties/value") + require.Contains(t, paths, "components/schemas/MergedSecond/properties/value") } diff --git a/index/extract_refs_walk.go b/index/extract_refs_walk.go index 54da4afd..0cb00eba 100644 --- a/index/extract_refs_walk.go +++ b/index/extract_refs_walk.go @@ -13,6 +13,7 @@ import ( type extractRefsState struct { traversal *extractRefsTraversal + mergeStart int ctx context.Context scope *SchemaIdScope parentBaseURI string @@ -81,7 +82,14 @@ func (index *SpecIndex) walkExtractRefs(node, parent *yaml.Node, state *extractR } if utils.IsNodeMap(n) || utils.IsNodeArray(n) { + inherited := i >= state.mergeStart + if inherited { + state.traversal.aliasDepth++ + } found = append(found, index.walkChildExtractRefs(n, node, state)...) + if inherited { + state.traversal.aliasDepth-- + } } index.unwindExtractRefsPath(node, state, i) diff --git a/utils/utils.go b/utils/utils.go index 8e61bbdb..7c1eaa20 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -710,6 +710,7 @@ func NodeMerge(nodes []*yaml.Node) *yaml.Node { // precedence over later sources. Aliases are resolved before expansion. // A merged mapping is shallow-copied: source nodes are not modified, and original // key/value nodes and their location/comment metadata are retained. Mappings +// place explicit key/value pairs first, followed by inherited pairs. Mappings // without merges retain their identity. Callers must not mutate shared children // while readers or model builders use them. func ResolveMergeNodes(node *yaml.Node) *yaml.Node { From 88d82182829893cc40dc6663c9dedba08d49038c Mon Sep 17 00:00:00 2001 From: quobix Date: Sat, 31 Oct 2026 11:16:05 -0400 Subject: [PATCH 17/33] fix: reuse candidate scores for nested schema unions --- what-changed/model/schema.go | 24 +++---- what-changed/model/schema_comparison.go | 47 ++++++++++++- what-changed/model/schema_comparison_test.go | 72 ++++++++++++++++++++ what-changed/model/schema_test.go | 6 +- 4 files changed, 130 insertions(+), 19 deletions(-) diff --git a/what-changed/model/schema.go b/what-changed/model/schema.go index 2ae69523..199487b5 100644 --- a/what-changed/model/schema.go +++ b/what-changed/model/schema.go @@ -2412,7 +2412,7 @@ func pairExactSchemaCompositionEntries( return pairs, leftUnmatched, rightUnmatched } -func buildSchemaCompositionPairCandidateMatrix( +func (c *schemaComparison) buildSchemaCompositionPairCandidateMatrix( rowEntries, columnEntries []schemaCompositionEntry, swapPairDirection bool, ) [][]schemaCompositionPairCandidate { @@ -2426,18 +2426,12 @@ func buildSchemaCompositionPairCandidateMatrix( leftEntry = columnEntries[j] rightEntry = rowEntries[i] } - changes := CompareSchemas(leftEntry.proxy, rightEntry.proxy) - totalChanges := 0 - breakingChanges := 0 - if changes != nil { - totalChanges = changes.TotalChanges() - breakingChanges = changes.TotalBreakingChanges() - } + score := c.score(leftEntry.proxy, rightEntry.proxy) row[j] = schemaCompositionPairCandidate{ left: leftEntry, right: rightEntry, - totalChanges: totalChanges, - breakingChanges: breakingChanges, + totalChanges: score.total, + breakingChanges: score.breaking, stableKeyMatch: leftEntry.stableKey == rightEntry.stableKey, } } @@ -2545,16 +2539,18 @@ func selectBestSchemaCompositionPairCandidates( return best } -func pairUnmatchedSchemaCompositionEntries( +func (c *schemaComparison) pairUnmatchedSchemaCompositionEntries( leftEntries, rightEntries []schemaCompositionEntry, ) []schemaCompositionPair { if len(leftEntries) == 0 || len(rightEntries) == 0 { return nil } - matrix := buildSchemaCompositionPairCandidateMatrix(leftEntries, rightEntries, false) + var matrix [][]schemaCompositionPairCandidate if len(leftEntries) > len(rightEntries) { - matrix = buildSchemaCompositionPairCandidateMatrix(rightEntries, leftEntries, true) + matrix = c.buildSchemaCompositionPairCandidateMatrix(rightEntries, leftEntries, true) + } else { + matrix = c.buildSchemaCompositionPairCandidateMatrix(leftEntries, rightEntries, false) } selected := selectBestSchemaCompositionPairCandidates(matrix) @@ -2613,7 +2609,7 @@ func (c *schemaComparison) extractOrderInsensitiveSchemaChanges( rightEntries := buildSchemaCompositionEntries(rSchema) exactPairs, leftUnmatched, rightUnmatched := pairExactSchemaCompositionEntries(leftEntries, rightEntries) - unmatchedPairs := pairUnmatchedSchemaCompositionEntries(leftUnmatched, rightUnmatched) + unmatchedPairs := c.pairUnmatchedSchemaCompositionEntries(leftUnmatched, rightUnmatched) for i := range exactPairs { if changes := c.compareSchemas(exactPairs[i].left.proxy, exactPairs[i].right.proxy); changes != nil { diff --git a/what-changed/model/schema_comparison.go b/what-changed/model/schema_comparison.go index ec2fa7c3..7eeef7dd 100644 --- a/what-changed/model/schema_comparison.go +++ b/what-changed/model/schema_comparison.go @@ -3,18 +3,61 @@ package model import ( "github.com/pb33f/go-yaml" "github.com/pb33f/libopenapi/datamodel/low/base" + "github.com/pb33f/libopenapi/index" ) -// A comparison owns its visited pairs. No state is shared with other callers or -// union candidate scoring. Properties and compositions visit pairs in stable order. +// Each report owns its visited pairs. Candidate scores are shared within one +// public comparison, but scoring never marks a pair visited in the report. type schemaComparison struct { visited map[schemaComparisonPair]struct{} + scores map[schemaScorePair]schemaScore } type schemaComparisonPair struct { left, right *yaml.Node } +type schemaScoreProxy struct { + node *yaml.Node + index *index.SpecIndex + reference string + isRef bool +} + +type schemaScorePair struct { + left, right schemaScoreProxy +} + +type schemaScore struct { + total, breaking int +} + +func schemaScoreIdentity(proxy *base.SchemaProxy) schemaScoreProxy { + if proxy == nil { + return schemaScoreProxy{} + } + return schemaScoreProxy{schemaDefinitionNode(proxy), proxy.GetIndex(), proxy.GetReference(), proxy.IsReference()} +} + +func (c *schemaComparison) score(left, right *base.SchemaProxy) schemaScore { + pair := schemaScorePair{schemaScoreIdentity(left), schemaScoreIdentity(right)} + if score, exists := c.scores[pair]; exists { + return score + } + if c.scores == nil { + c.scores = make(map[schemaScorePair]schemaScore) + } + // Each candidate must see a complete comparison, independent of earlier + // candidates and report placement. Keep only its counts, not its report nodes. + candidate := schemaComparison{scores: c.scores} + var score schemaScore + if changes := candidate.compareSchemas(left, right); changes != nil { + score = schemaScore{changes.TotalChanges(), changes.TotalBreakingChanges()} + } + c.scores[pair] = score + return score +} + func (c *schemaComparison) visit(left, right *base.SchemaProxy) bool { pair := schemaComparisonPair{schemaDefinitionNode(left), schemaDefinitionNode(right)} if _, exists := c.visited[pair]; exists { diff --git a/what-changed/model/schema_comparison_test.go b/what-changed/model/schema_comparison_test.go index 6187cdd7..bf7605d6 100644 --- a/what-changed/model/schema_comparison_test.go +++ b/what-changed/model/schema_comparison_test.go @@ -6,6 +6,7 @@ import ( "sync" "testing" + "github.com/pb33f/libopenapi/datamodel/low/base" "github.com/pb33f/testify/require" ) @@ -19,6 +20,77 @@ func diamondComparisonSpec(prefix string, depth int) string { return spec.String() } +func TestCompareSchemas_NestedUnionScoresAreShared(t *testing.T) { + for _, union := range []string{"anyOf", "oneOf"} { + for _, depth := range []int{0, 1, 2, 3, 12} { + t.Run(fmt.Sprintf("%s/%d", union, depth), func(t *testing.T) { + makeSpec := func(prefix, kind string) string { + var spec strings.Builder + spec.WriteString("openapi: 3.0.3\ninfo: {title: Union, version: '1'}\npaths: {}\ncomponents:\n schemas:\n") + fmt.Fprintf(&spec, " Root: {$ref: '#/components/schemas/%s0'}\n", prefix) + for i := 0; i < depth; i++ { + fmt.Fprintf(&spec, " %s%d:\n %s:\n - {$ref: '#/components/schemas/%s%d'}\n - {$ref: '#/components/schemas/%s%d'}\n", prefix, i, union, prefix, i+1, prefix, i+1) + } + fmt.Fprintf(&spec, " %s%d: {type: %s}\n", prefix, depth, kind) + return spec.String() + } + l, r := test_BuildDoc(makeSpec("L", "string"), makeSpec("R", "integer")) + left, right := l.Components.Value.FindSchema("Root").Value, r.Components.Value.FindSchema("Root").Value + comparison := new(schemaComparison) + changes := comparison.compareSchemas(left, right) + require.Equal(t, 2*depth+2, changes.TotalChanges()) + // Four candidates at each level share one complete score. Nested + // candidates reuse the same cache instead of starting four more trees. + require.Len(t, comparison.scores, depth) + require.Equal(t, changes.TotalChanges(), CompareSchemas(left, right).TotalChanges()) + if depth == 12 { + var wg sync.WaitGroup + counts := make([]int, 4) + for i := range counts { + wg.Add(1) + go func() { defer wg.Done(); counts[i] = CompareSchemas(left, right).TotalChanges() }() + } + wg.Wait() + for _, count := range counts { + require.Equal(t, 2*depth+2, count) + } + } + }) + } + } +} + +func TestCompareSchemas_CandidateScoresPreserveReferenceIdentity(t *testing.T) { + makeSpec := func(kind string) string { + return fmt.Sprintf(`openapi: 3.0.3 +components: + schemas: + Target: {type: %s} + Alias: {$ref: '#/components/schemas/Target'} + Ref: {$ref: '#/components/schemas/Target'} + AliasRef: {$ref: '#/components/schemas/Alias'} +`, kind) + } + l, r := test_BuildDoc(makeSpec("string"), makeSpec("integer")) + left := l.Components.Value.FindSchema("Ref").Value + right := r.Components.Value.FindSchema("Ref").Value + alias := r.Components.Value.FindSchema("AliasRef").Value + inlineLeft := l.Components.Value.FindSchema("Target").Value + inlineRight := r.Components.Value.FindSchema("Target").Value + require.Same(t, schemaDefinitionNode(right), schemaDefinitionNode(alias)) + require.Same(t, schemaDefinitionNode(right), schemaDefinitionNode(inlineRight)) + comparison := new(schemaComparison) + for _, pair := range [][2]*base.SchemaProxy{{left, right}, {left, alias}, {inlineLeft, right}, {left, inlineRight}} { + var expected schemaScore + if changes := CompareSchemas(pair[0], pair[1]); changes != nil { + expected = schemaScore{changes.TotalChanges(), changes.TotalBreakingChanges()} + } + require.Equal(t, expected, comparison.score(pair[0], pair[1])) + require.Equal(t, expected, comparison.score(pair[0], pair[1])) + } + require.Len(t, comparison.scores, 4) +} + func TestCompareSchemas_RetargetedDiamondIsBounded(t *testing.T) { for _, depth := range []int{8, 16, 24} { t.Run(fmt.Sprint(depth), func(t *testing.T) { diff --git a/what-changed/model/schema_test.go b/what-changed/model/schema_test.go index 44bb1765..19f4b88c 100644 --- a/what-changed/model/schema_test.go +++ b/what-changed/model/schema_test.go @@ -4415,9 +4415,9 @@ func TestSchemaCompositionPairingHelpers(t *testing.T) { []schemaCompositionPairCandidate{{left: schemaCompositionEntry{position: 0}, right: schemaCompositionEntry{position: 0}, totalChanges: 1, stableKeyMatch: true, breakingChanges: 0}}, )) - assert.Nil(t, pairUnmatchedSchemaCompositionEntries(nil, []schemaCompositionEntry{{position: 0}})) - assert.Nil(t, pairUnmatchedSchemaCompositionEntries([]schemaCompositionEntry{{position: 0}}, nil)) - pairs := pairUnmatchedSchemaCompositionEntries( + assert.Nil(t, new(schemaComparison).pairUnmatchedSchemaCompositionEntries(nil, []schemaCompositionEntry{{position: 0}})) + assert.Nil(t, new(schemaComparison).pairUnmatchedSchemaCompositionEntries([]schemaCompositionEntry{{position: 0}}, nil)) + pairs := new(schemaComparison).pairUnmatchedSchemaCompositionEntries( []schemaCompositionEntry{{position: 0}, {position: 0}}, []schemaCompositionEntry{{position: 1}, {position: 0}}, ) From b496dc7c27dc97a276305064f4b7826be36e1c13 Mon Sep 17 00:00:00 2001 From: quobix Date: Sat, 31 Oct 2026 11:16:58 -0400 Subject: [PATCH 18/33] test: cover recursive schema union candidate scoring --- what-changed/model/schema_comparison_test.go | 40 ++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/what-changed/model/schema_comparison_test.go b/what-changed/model/schema_comparison_test.go index bf7605d6..c64907dd 100644 --- a/what-changed/model/schema_comparison_test.go +++ b/what-changed/model/schema_comparison_test.go @@ -91,6 +91,46 @@ components: require.Len(t, comparison.scores, 4) } +func TestCompareSchemas_CircularUnionCandidatesStopAtReferences(t *testing.T) { + for _, union := range []string{"anyOf", "oneOf"} { + for _, mutual := range []bool{false, true} { + t.Run(fmt.Sprintf("%s/mutual=%t", union, mutual), func(t *testing.T) { + makeSpec := func(prefix string) string { + next := prefix + "A" + if mutual { + next = prefix + "B" + } + return fmt.Sprintf(`openapi: 3.0.3 +components: + schemas: + Root: + %s: + - {$ref: '#/components/schemas/%sA'} + - {$ref: '#/components/schemas/%sA'} + %sA: + %s: + - {$ref: '#/components/schemas/%s'} + - {type: string} + %sB: + %s: + - {$ref: '#/components/schemas/%sA'} + - {type: integer} +`, union, prefix, prefix, prefix, union, next, prefix, union, prefix) + } + l, r := test_BuildDoc(makeSpec("L"), makeSpec("R")) + left, right := l.Components.Value.FindSchema("Root").Value, r.Components.Value.FindSchema("Root").Value + comparison := new(schemaComparison) + changes := comparison.compareSchemas(left, right) + require.Equal(t, 2, changes.TotalChanges()) + require.Len(t, comparison.scores, 1) + for _, score := range comparison.scores { + require.Equal(t, 1, score.total) + } + }) + } + } +} + func TestCompareSchemas_RetargetedDiamondIsBounded(t *testing.T) { for _, depth := range []int{8, 16, 24} { t.Run(fmt.Sprint(depth), func(t *testing.T) { From 3c26804b942f1bffcce833450d8f485171898d7f Mon Sep 17 00:00:00 2001 From: quobix Date: Sat, 31 Oct 2026 14:42:30 -0400 Subject: [PATCH 19/33] test: close reference repair coverage gaps --- datamodel/low/v2/path_item_test.go | 29 ++++++++++ index/resolver_required_alias_test.go | 23 ++++++++ index/rolodex_alias_limit_test.go | 79 +++++++++++++++++++++++++++ index/utility_methods.go | 21 ++----- 4 files changed, 135 insertions(+), 17 deletions(-) create mode 100644 index/rolodex_alias_limit_test.go diff --git a/datamodel/low/v2/path_item_test.go b/datamodel/low/v2/path_item_test.go index 702a0a6c..846dd2c2 100644 --- a/datamodel/low/v2/path_item_test.go +++ b/datamodel/low/v2/path_item_test.go @@ -12,7 +12,9 @@ import ( "github.com/pb33f/libopenapi/datamodel/low" "github.com/pb33f/libopenapi/index" "github.com/pb33f/libopenapi/orderedmap" + "github.com/pb33f/libopenapi/utils" "github.com/pb33f/testify/assert" + "github.com/pb33f/testify/require" ) func TestPathItem_Build_Params(t *testing.T) { @@ -32,6 +34,33 @@ func TestPathItem_Build_Params(t *testing.T) { assert.Error(t, err) } +func TestPathItem_Build_SkippedExternalReferenceAndSiblingOperation(t *testing.T) { + var root yaml.Node + require.NoError(t, yaml.Unmarshal([]byte(`swagger: '2.0' +info: {title: Pets, version: '1'} +paths: + /pets: + $ref: './shared.yaml#/paths/~1pets' + get: + operationId: listPets + responses: + '200': {description: Pet list} +`), &root)) + cfg := index.CreateOpenAPIIndexConfig() + cfg.SkipExternalRefResolution = true + idx := index.NewSpecIndexWithConfig(&root, cfg) + _, paths := utils.FindKeyNodeTop("paths", root.Content[0].Content) + _, source := utils.FindKeyNodeTop("/pets", paths.Content) + var item PathItem + require.NoError(t, item.Build(context.Background(), nil, source, idx)) + assert.Equal(t, "./shared.yaml#/paths/~1pets", item.Ref.Value) + assert.Same(t, source.Content[0], item.Ref.KeyNode) + assert.Same(t, source.Content[1], item.Ref.ValueNode) + assert.Empty(t, item.Parameters.Value) + require.NotNil(t, item.Get.Value) + assert.Equal(t, "listPets", item.Get.Value.OperationId.Value) +} + func TestPathItem_Build_MethodFail(t *testing.T) { yml := `post: $ref: break` diff --git a/index/resolver_required_alias_test.go b/index/resolver_required_alias_test.go index bfaec693..faeae6a4 100644 --- a/index/resolver_required_alias_test.go +++ b/index/resolver_required_alias_test.go @@ -38,3 +38,26 @@ func TestRequiredAliasPreservesLocalFragment(t *testing.T) { require.Contains(t, targets, location+"#/$defs/ID") } } + +func TestRequiredPropertyWithoutDefinition(t *testing.T) { + var node yaml.Node + require.NoError(t, yaml.Unmarshal([]byte(`type: object +required: [missing, id] +properties: + id: {$ref: '#/$defs/ID'} +`), &node)) + targets := extractDefinitionRequiredRefProperties(node.Content[0], map[string][]string{}, "https://example.com/schema.yaml", nil) + require.Equal(t, map[string][]string{"https://example.com/schema.yaml#/$defs/ID": {"id"}}, targets) +} + +func TestRelativeSchemaFileUsesRemoteOwner(t *testing.T) { + resolver := NewResolver(NewSpecIndex(nil)) + for _, ref := range []*Reference{ + {FullDefinition: "https://example.com/schemas/owner.yaml#/components/schemas/Pet"}, + {FullDefinition: "#/components/schemas/Pet", RemoteLocation: "https://example.com/schemas/owner.yaml"}, + } { + definition, full := resolver.buildRelativeLookupDefinitions(ref, "child.yaml", "") + require.Equal(t, "child.yaml", definition) + require.Equal(t, "https://example.com/schemas/child.yaml", full) + } +} diff --git a/index/rolodex_alias_limit_test.go b/index/rolodex_alias_limit_test.go new file mode 100644 index 00000000..f97bc81e --- /dev/null +++ b/index/rolodex_alias_limit_test.go @@ -0,0 +1,79 @@ +package index + +import ( + "context" + "fmt" + "io" + "net/http" + "net/url" + "strings" + "testing" + + "github.com/pb33f/go-yaml" + "github.com/pb33f/testify/require" +) + +const excessiveAliasDocument = `openapi: 3.1.1 +info: {title: Alias limit, version: '1'} +paths: {} +components: {schemas: {Target: {type: string}}} +x-a: &a [value] +x-b: &b [*a, *a] +x-c: &c [*b, *b] +x-d: &d [*c, *c] +x-e: &e [*d, *d] +x-f: &f [*e, *e] +x-g: &g [*f, *f] +x-h: &h [*g, *g] +x-i: &i [*h, *h] +x-j: &j [*i, *i] +x-k: &k [*j, *j] +x-tail: {$ref: './unvisited.yaml'} +` + +func TestExtractRefsAliasLimitSkipsRemainingSiblings(t *testing.T) { + var root yaml.Node + require.NoError(t, yaml.Unmarshal([]byte(excessiveAliasDocument), &root)) + idx := NewSpecIndex(nil) + require.Empty(t, idx.ExtractRefs(context.Background(), &root, nil, nil, 0, false, "")) + require.Len(t, idx.GetReferenceIndexErrors(), 1) + require.ErrorContains(t, idx.GetReferenceIndexErrors()[0], "document contains excessive aliasing") + require.Empty(t, idx.GetRawReferencesSequenced(), "the tail reference must not be visited after the limit") +} + +func TestRolodexReportsAliasLimitFromFileLoadedByRoot(t *testing.T) { + for _, skipCircular := range []bool{false, true} { + t.Run(fmt.Sprintf("skipCircular=%t", skipCircular), func(t *testing.T) { + cfg := CreateOpenAPIIndexConfig() + cfg.AvoidCircularReferenceCheck = skipCircular + cfg.BaseURL, _ = url.Parse("https://example.com/") + rolo := NewRolodex(cfg) + remote, err := NewRemoteFSWithConfig(cfg) + require.NoError(t, err) + remote.RemoteHandlerFunc = func(location string) (*http.Response, error) { + if location != "https://example.com/shared.yaml" { + return nil, fmt.Errorf("unexpected remote lookup: %s", location) + } + return &http.Response{StatusCode: http.StatusOK, Body: io.NopCloser(strings.NewReader(excessiveAliasDocument))}, nil + } + rolo.AddRemoteFS("https://example.com/", remote) + require.Empty(t, remote.GetFiles(), "the external index must be loaded after the initial build queue") + var root yaml.Node + require.NoError(t, yaml.Unmarshal([]byte(`openapi: 3.1.1 +info: {title: Root, version: '1'} +paths: {} +components: + schemas: + Target: {$ref: 'https://example.com/shared.yaml#/components/schemas/Target'} +`), &root)) + rolo.SetRootNode(&root) + err = rolo.IndexTheRolodex(context.Background()) + require.ErrorContains(t, err, "document contains excessive aliasing") + require.Len(t, rolo.GetIndexes(), 1) + var resolvingErr *ResolvingError + require.ErrorAs(t, err, &resolvingErr) + require.NotNil(t, resolvingErr.Node) + require.Contains(t, rolo.GetCaughtErrors(), resolvingErr) + }) + } +} diff --git a/index/utility_methods.go b/index/utility_methods.go index b6c1a1de..d342fbe0 100644 --- a/index/utility_methods.go +++ b/index/utility_methods.go @@ -136,13 +136,8 @@ func extractRequiredReferenceProperties(fulldef string, requiredPropDefNode *yam u, _ := url.Parse(exp[0]) r := strings.Split(refName, "#/") if len(r) == 2 { - var abs string - if r[0] == "" { - abs = u.Path - } else { - abs, _ = filepath.Abs(utils.CheckPathOverlap(filepath.Dir(u.Path), r[0], - string(os.PathSeparator))) - } + abs, _ := filepath.Abs(utils.CheckPathOverlap(filepath.Dir(u.Path), r[0], + string(os.PathSeparator))) u.Path = utils.ReplaceWindowsDriveWithLinuxPath(abs) u.Fragment = "" @@ -156,16 +151,8 @@ func extractRequiredReferenceProperties(fulldef string, requiredPropDefNode *yam } else { r := strings.Split(refName, "#/") if len(r) == 2 { - var abs string - if r[0] == "" { - abs, _ = filepath.Abs(exp[0]) - } else { - abs, _ = filepath.Abs(utils.CheckPathOverlap(filepath.Dir(exp[0]), r[0], - string(os.PathSeparator))) - - // abs, _ = filepath.Abs(filepath.Join(filepath.Dir(exp[0]), r[0], - // string('J'))) - } + abs, _ := filepath.Abs(utils.CheckPathOverlap(filepath.Dir(exp[0]), r[0], + string(os.PathSeparator))) defPath = fmt.Sprintf("%s#/%s", abs, r[1]) } else { From 9144f75d6a634315456a874ac3ee616dfc82dc38 Mon Sep 17 00:00:00 2001 From: quobix Date: Sat, 31 Oct 2026 16:44:16 -0400 Subject: [PATCH 20/33] fix(index): distinguish named schemas from example payloads --- document_named_example_test.go | 75 ++++++++++++++++++++++++++++++++ index/extract_refs.go | 12 +++-- index/find_component_external.go | 5 ++- index/resolver_relatives.go | 16 +++++++ 4 files changed, 104 insertions(+), 4 deletions(-) create mode 100644 document_named_example_test.go diff --git a/document_named_example_test.go b/document_named_example_test.go new file mode 100644 index 00000000..b1486fc2 --- /dev/null +++ b/document_named_example_test.go @@ -0,0 +1,75 @@ +package libopenapi + +import ( + "fmt" + "strings" + "testing" + + "github.com/pb33f/libopenapi/datamodel" + "github.com/pb33f/testify/require" +) + +func TestBuildV3Model_NamedExampleSchemaCycles(t *testing.T) { + for _, name := range []string{"example", "examples"} { + for _, container := range []string{"components/schemas", "definitions", "$defs", "dependentSchemas", "external"} { + t.Run(container+"/"+name, func(t *testing.T) { + prefix := "#/" + container + "/" + if container == "external" { + prefix = "#/" + } + schemas := fmt.Sprintf(`%s: + type: object + required: [child] + properties: + child: {$ref: '%sNode'} +Node: + type: object + required: [back] + properties: + back: {$ref: '%s%s'} +`, name, prefix, prefix, name) + header := "openapi: 3.1.1\ninfo: {title: Named schema cycles, version: '1'}\npaths: {}\n" + config := datamodel.NewDocumentConfiguration() + var source string + switch container { + case "external": + config = externalRegressionConfig(t, map[string]string{"schemas.yaml": schemas}) + source = header + "components:\n schemas:\n Root: {$ref: './schemas.yaml#/" + name + "'}\n" + case "components/schemas": + source = header + "components:\n schemas:\n" + indentNamedSchema(schemas, 4) + default: + source = header + "components:\n schemas:\n Root: {$ref: '" + prefix + name + "'}\n" + container + ":\n" + indentNamedSchema(schemas, 2) + } + doc, err := NewDocumentWithConfiguration([]byte(source), config) + require.NoError(t, err) + _, err = doc.BuildV3Model() + require.ErrorContains(t, err, "infinite circular reference detected") + }) + } + } +} + +func indentNamedSchema(source string, spaces int) string { + indent := strings.Repeat(" ", spaces) + return indent + strings.ReplaceAll(strings.TrimSuffix(source, "\n"), "\n", "\n"+indent) + "\n" +} + +func TestBuildV3Model_ExternalNamedExampleObject(t *testing.T) { + config := externalRegressionConfig(t, map[string]string{ + "examples.yaml": "example:\n value:\n $ref: '#/payload-only'\n", + }) + doc, err := NewDocumentWithConfiguration([]byte(`openapi: 3.1.1 +info: {title: External example, version: '1'} +paths: {} +components: + examples: + External: {$ref: './examples.yaml#/example'} +`), config) + require.NoError(t, err) + model, err := doc.BuildV3Model() + require.NoError(t, err) + require.NotNil(t, model) + value := model.Model.Components.Examples.GetOrZero("External").Value + require.Equal(t, "$ref", value.Content[0].Value) + require.Equal(t, "#/payload-only", value.Content[1].Value) +} diff --git a/index/extract_refs.go b/index/extract_refs.go index 2dde8a08..d43be715 100644 --- a/index/extract_refs.go +++ b/index/extract_refs.go @@ -41,8 +41,8 @@ func isArrayOfSchemaContainingNode(v string) bool { } // underOpenAPIExamplePath reports whether seenPath is under an OpenAPI example or examples -// keyword (sample data, not schema). A segment named "example" or "examples" that is preceded -// by "properties" or "patternProperties" is a schema property name, not an OpenAPI keyword. +// keyword (sample data, not schema). Entries in maps of named schemas are names, +// even when the name is "example" or "examples". func underOpenAPIExamplePath(seenPath []string) bool { for i := range seenPath { if isOpenAPIExampleKeywordSegment(seenPath, i) { @@ -58,7 +58,13 @@ func isOpenAPIExampleKeywordSegment(seenPath []string, idx int) bool { } switch seenPath[idx] { case "example", "examples": - return idx == 0 || (seenPath[idx-1] != "properties" && seenPath[idx-1] != "patternProperties") + if idx > 0 { + switch seenPath[idx-1] { + case "properties", "patternProperties", "schemas", "definitions", "$defs", "dependentSchemas": + return false + } + } + return true default: return false } diff --git a/index/find_component_external.go b/index/find_component_external.go index 80b464f9..172e220c 100644 --- a/index/find_component_external.go +++ b/index/find_component_external.go @@ -89,10 +89,13 @@ func (index *SpecIndex) lookupRolodex(ctx context.Context, uri []string) *Refere } } - foundRef := FindComponent(ctx, parsedDocument, query, absoluteFileLocation, index) + foundRef := FindComponent(ctx, parsedDocument, query, absoluteFileLocation, idx) if foundRef != nil { foundRef.IsRemote = true foundRef.RemoteLocation = absoluteFileLocation + if source := index.allRefs[absoluteFileLocation+query]; source != nil { + foundRef.SourcePath = append([]string(nil), source.SourcePath...) + } return foundRef } index.logger.Debug("[lookupRolodex] FindComponent returned nil", diff --git a/index/resolver_relatives.go b/index/resolver_relatives.go index dfaf1d9a..eef42224 100644 --- a/index/resolver_relatives.go +++ b/index/resolver_relatives.go @@ -29,6 +29,22 @@ func (resolver *Resolver) extractRelatives( if strings.HasPrefix(ref.Definition, "#/") { state.path = strings.Split(strings.TrimPrefix(ref.Definition, "#/"), "/") } + if len(state.path) > 0 && (state.path[0] == "example" || state.path[0] == "examples") && ref.Index != nil { + root := ref.Index.GetRootNode() + if root != nil { + if root.Kind == yaml.DocumentNode && len(root.Content) > 0 { + root = root.Content[0] + } + _, openapi := utils.FindKeyNodeTop("openapi", root.Content) + _, swagger := utils.FindKeyNodeTop("swagger", root.Content) + if openapi == nil && swagger == nil { + // An external fragment identifies the object being resolved. Its + // top-level name is not a keyword in the referencing document. + // Example fields encountered inside that object still mark payloads. + state.path = ref.SourcePath + } + } + } } return resolver.extractRelativesWithState(ref, node, parent, state) } From acacfa88a1b6d343741480b9e87ef12e4de0631a Mon Sep 17 00:00:00 2001 From: quobix Date: Sat, 31 Oct 2026 16:46:23 -0400 Subject: [PATCH 21/33] fix: stop recursive union scoring without caching partial results --- what-changed/model/schema_comparison.go | 24 +++++++- what-changed/model/schema_comparison_test.go | 62 ++++++++++++++++++++ 2 files changed, 84 insertions(+), 2 deletions(-) diff --git a/what-changed/model/schema_comparison.go b/what-changed/model/schema_comparison.go index 7eeef7dd..c5922886 100644 --- a/what-changed/model/schema_comparison.go +++ b/what-changed/model/schema_comparison.go @@ -11,6 +11,14 @@ import ( type schemaComparison struct { visited map[schemaComparisonPair]struct{} scores map[schemaScorePair]schemaScore + scoring *schemaScoring +} + +// Active scores belong to one traversal. A recursion cut must not publish a +// context-dependent partial score for another candidate to reuse. +type schemaScoring struct { + active map[schemaScorePair]struct{} + cycles uint64 } type schemaComparisonPair struct { @@ -44,17 +52,29 @@ func (c *schemaComparison) score(left, right *base.SchemaProxy) schemaScore { if score, exists := c.scores[pair]; exists { return score } + if c.scoring == nil { + c.scoring = &schemaScoring{active: make(map[schemaScorePair]struct{})} + } + if _, active := c.scoring.active[pair]; active { + c.scoring.cycles++ + return schemaScore{} + } + c.scoring.active[pair] = struct{}{} + defer delete(c.scoring.active, pair) + cycles := c.scoring.cycles if c.scores == nil { c.scores = make(map[schemaScorePair]schemaScore) } // Each candidate must see a complete comparison, independent of earlier // candidates and report placement. Keep only its counts, not its report nodes. - candidate := schemaComparison{scores: c.scores} + candidate := schemaComparison{scores: c.scores, scoring: c.scoring} var score schemaScore if changes := candidate.compareSchemas(left, right); changes != nil { score = schemaScore{changes.TotalChanges(), changes.TotalBreakingChanges()} } - c.scores[pair] = score + if c.scoring.cycles == cycles { + c.scores[pair] = score + } return score } diff --git a/what-changed/model/schema_comparison_test.go b/what-changed/model/schema_comparison_test.go index c64907dd..7953cea6 100644 --- a/what-changed/model/schema_comparison_test.go +++ b/what-changed/model/schema_comparison_test.go @@ -1,6 +1,7 @@ package model import ( + "encoding/json" "fmt" "strings" "sync" @@ -10,6 +11,67 @@ import ( "github.com/pb33f/testify/require" ) +func TestCompareSchemas_RecursiveScoresWithoutCircularMetadata(t *testing.T) { + for _, union := range []string{"anyOf", "oneOf"} { + t.Run(union, func(t *testing.T) { + makeSpec := func(name, kind string) string { + return fmt.Sprintf(`openapi: 3.1.1 +info: {title: Recursive scores, version: '1'} +paths: {} +components: + schemas: + Holder: + properties: + p: {$ref: '#/components/schemas/%s'} + %s: + %s: + - {$ref: '#/components/schemas/example/properties/next%s'} + - {type: %s} + example: + type: object + properties: + nextA: + %s: + - {$ref: '#/components/schemas/example/properties/nextB'} + - {type: %s} + nextB: + %s: + - {$ref: '#/components/schemas/example/properties/nextA'} + - {type: %s} +`, name, name, union, name[len(name)-1:], kind, union, kind, union, kind) + } + left, right := test_BuildDoc(makeSpec("exampleA", "string"), makeSpec("exampleB", "integer")) + l := left.Components.Value.FindSchema("Holder").Value + r := right.Components.Value.FindSchema("Holder").Value + for _, proxy := range []*base.SchemaProxy{l, r} { + idx := proxy.GetIndex() + // Model mutation or incomplete metadata must not make scoring fatal. + idx.SetCircularReferences(nil) + idx.GetRolodex().SetSafeCircularReferences(nil) + } + var previous []byte + for range 3 { + comparison := new(schemaComparison) + changes := comparison.compareSchemas(l, r) + require.NotNil(t, changes) + require.Greater(t, changes.TotalChanges(), 0) + require.NotNil(t, comparison.scoring) + require.Greater(t, comparison.scoring.cycles, uint64(0)) + require.Empty(t, comparison.scoring.active) + comparison.score(l, r) + _, cached := comparison.scores[schemaScorePair{schemaScoreIdentity(l), schemaScoreIdentity(r)}] + require.False(t, cached, "a score cut short by recursion must not enter the completed-score cache") + encoded, err := json.Marshal(changes) + require.NoError(t, err) + if previous != nil { + require.Equal(t, string(previous), string(encoded)) + } + previous = encoded + } + }) + } +} + func diamondComparisonSpec(prefix string, depth int) string { var spec strings.Builder spec.WriteString("openapi: 3.0.3\ninfo: {title: test, version: '1'}\npaths: {}\ncomponents:\n schemas:\n Root:\n type: object\n properties:\n entry:\n $ref: '#/components/schemas/" + prefix + "0'\n") From a435203ba2d8f40756cabc2c72665eff6d18cb2a Mon Sep 17 00:00:00 2001 From: quobix Date: Sat, 31 Oct 2026 16:47:25 -0400 Subject: [PATCH 22/33] perf(index): track ancestry only for aliases and merges --- index/extract_refs.go | 25 ++++++++++++++-------- index/extract_refs_alias_limit_test.go | 29 ++++++++++++++++++++++++++ utils/utils.go | 3 +++ 3 files changed, 48 insertions(+), 9 deletions(-) diff --git a/index/extract_refs.go b/index/extract_refs.go index d43be715..ad0e90d0 100644 --- a/index/extract_refs.go +++ b/index/extract_refs.go @@ -112,7 +112,7 @@ func isDirectOpenAPIExampleValuePath(seenPath []string) bool { // The total number of refs, will generally be much higher, you can extract those from GetRawReferenceCount() // Excessive YAML alias expansion stops extraction and is reported by GetReferenceIndexErrors. func (index *SpecIndex) ExtractRefs(ctx context.Context, node, parent *yaml.Node, seenPath []string, level int, poly bool, pName string) []*Reference { - walk := &extractRefsTraversal{ancestry: make(map[*yaml.Node]struct{})} + walk := &extractRefsTraversal{} return index.extractRefs(ctx, node, parent, seenPath, level, poly, pName, walk) } @@ -126,9 +126,10 @@ type extractRefsTraversal struct { func (walk *extractRefsTraversal) visit() error { walk.nodes++ - if walk.aliasDepth > 0 { - walk.aliasNodes++ + if walk.aliasDepth == 0 { + return nil } + walk.aliasNodes++ return extractRefsAliasLimit.CheckAlias(walk.aliasNodes, walk.nodes) } @@ -139,7 +140,8 @@ func (index *SpecIndex) extractRefs(ctx context.Context, node, parent *yaml.Node return nil } err := walk.visit() - if node.Kind == yaml.AliasNode && err == nil { + aliased := node.Kind == yaml.AliasNode + if aliased && err == nil { walk.aliasDepth++ defer func() { walk.aliasDepth-- }() node = utils.NodeAlias(node) @@ -156,12 +158,17 @@ func (index *SpecIndex) extractRefs(ctx context.Context, node, parent *yaml.Node if node == nil { return nil } - if _, active := walk.ancestry[node]; active { - return nil - } - walk.ancestry[node] = struct{}{} - defer delete(walk.ancestry, node) view := utils.ResolveMergeNodes(node) + if aliased || view != node { + if _, active := walk.ancestry[node]; active { + return nil + } + if walk.ancestry == nil { + walk.ancestry = make(map[*yaml.Node]struct{}) + } + walk.ancestry[node] = struct{}{} + defer delete(walk.ancestry, node) + } state := index.initializeExtractRefsState(ctx, view, seenPath, level, poly, pName) state.traversal = walk state.mergeStart = len(view.Content) diff --git a/index/extract_refs_alias_limit_test.go b/index/extract_refs_alias_limit_test.go index f2229f03..d64cc2dc 100644 --- a/index/extract_refs_alias_limit_test.go +++ b/index/extract_refs_alias_limit_test.go @@ -39,6 +39,35 @@ func TestExtractRefsAbsentAliasTarget(t *testing.T) { require.Empty(t, idx.GetReferenceIndexErrors()) } +func TestExtractRefsMergeCycleWithoutAliasInWalk(t *testing.T) { + var root yaml.Node + require.NoError(t, yaml.Unmarshal([]byte("x: &x {b: {<<: *x}}\n"), &root)) + idx := NewSpecIndex(nil) + walk := &extractRefsTraversal{} + idx.extractRefs(context.Background(), &root, nil, nil, 0, false, "", walk) + require.False(t, walk.stopped) + require.Empty(t, idx.GetReferenceIndexErrors()) + require.Empty(t, walk.ancestry) + require.Zero(t, walk.aliasDepth) + require.Less(t, walk.nodes, 10) +} + +func TestExtractRefsPlainDocumentNeedsNoAncestry(t *testing.T) { + var root yaml.Node + require.NoError(t, yaml.Unmarshal([]byte(`openapi: 3.1.1 +components: + schemas: + Item: {type: object, properties: {id: {type: string}}} + Ref: {$ref: '#/components/schemas/Item'} +`), &root)) + idx := NewSpecIndex(nil) + walk := &extractRefsTraversal{} + refs := idx.extractRefs(context.Background(), &root, nil, nil, 0, false, "", walk) + require.Len(t, refs, 1) + require.Nil(t, walk.ancestry) + require.Zero(t, walk.aliasNodes) +} + func TestExtractRefsCountsInheritedMergeDescendants(t *testing.T) { const source = `shared: &shared {inherited: {nested: {type: string}}} target: diff --git a/utils/utils.go b/utils/utils.go index 7c1eaa20..b210ce0d 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -718,6 +718,9 @@ func ResolveMergeNodes(node *yaml.Node) *yaml.Node { if node == nil { return nil } + if node.Kind != yaml.MappingNode && node.Tag != "!!map" { + return node + } if !hasMergeKeys(node.Content) { return node } From 3d18ca3cbf01860fe41215994eb1e112346e41f3 Mon Sep 17 00:00:00 2001 From: quobix Date: Sat, 31 Oct 2026 16:59:39 -0400 Subject: [PATCH 23/33] feat: allow trusted documents to bypass alias expansion limits --- datamodel/document_config.go | 5 +++ datamodel/low/v2/swagger.go | 1 + datamodel/low/v3/create_document.go | 1 + document_alias_limit_test.go | 66 +++++++++++++++++++++++++++++ index/extract_refs.go | 12 +++--- index/index_model.go | 4 ++ 6 files changed, 84 insertions(+), 5 deletions(-) diff --git a/datamodel/document_config.go b/datamodel/document_config.go index 3a431005..4f12dfb0 100644 --- a/datamodel/document_config.go +++ b/datamodel/document_config.go @@ -81,6 +81,11 @@ type DocumentConfiguration struct { // FileFilter should be used to limit the scope of the rolodex. AllowFileReferences bool + // AllowExcessiveAliases disables the YAML alias expansion limit during indexing. + // Use it for trusted documents with large amounts of shared YAML content. + // The limit remains enabled by default; cycle detection is always enabled. + AllowExcessiveAliases bool + // SkipExternalRefResolution will skip resolving external $ref references (those not starting with #). // When enabled, external references will be left as-is during model building. Schema proxies will // report IsReference()=true and GetReference() will return the ref string, but Schema() will return nil. diff --git a/datamodel/low/v2/swagger.go b/datamodel/low/v2/swagger.go index 33656b3b..c1529ed9 100644 --- a/datamodel/low/v2/swagger.go +++ b/datamodel/low/v2/swagger.go @@ -146,6 +146,7 @@ func createDocument(info *datamodel.SpecInfo, config *datamodel.DocumentConfigur idxConfig.IgnorePolymorphicCircularReferences = config.IgnorePolymorphicCircularReferences idxConfig.AllowUnknownExtensionContentDetection = config.AllowUnknownExtensionContentDetection idxConfig.SkipExternalRefResolution = config.SkipExternalRefResolution + idxConfig.AllowExcessiveAliases = config.AllowExcessiveAliases idxConfig.ResolveNestedRefsWithDocumentContext = config.ResolveNestedRefsWithDocumentContext idxConfig.AvoidCircularReferenceCheck = true idxConfig.BaseURL = config.BaseURL diff --git a/datamodel/low/v3/create_document.go b/datamodel/low/v3/create_document.go index 153cbb01..57e01d3d 100644 --- a/datamodel/low/v3/create_document.go +++ b/datamodel/low/v3/create_document.go @@ -157,6 +157,7 @@ func createDocument(info *datamodel.SpecInfo, config *datamodel.DocumentConfigur idxConfig.AllowUnknownExtensionContentDetection = config.AllowUnknownExtensionContentDetection idxConfig.TransformSiblingRefs = config.TransformSiblingRefs idxConfig.SkipExternalRefResolution = config.SkipExternalRefResolution + idxConfig.AllowExcessiveAliases = config.AllowExcessiveAliases idxConfig.ResolveNestedRefsWithDocumentContext = config.ResolveNestedRefsWithDocumentContext idxConfig.AvoidCircularReferenceCheck = true diff --git a/document_alias_limit_test.go b/document_alias_limit_test.go index 6ea315a6..3b1264c4 100644 --- a/document_alias_limit_test.go +++ b/document_alias_limit_test.go @@ -1,14 +1,80 @@ package libopenapi import ( + "fmt" "os" "path/filepath" + "strings" "testing" "github.com/pb33f/libopenapi/datamodel" "github.com/pb33f/testify/require" ) +func sharedResponseDocument() string { + var source strings.Builder + source.WriteString("openapi: 3.1.1\ninfo: {title: Shared responses, version: '1'}\nx-common:\n errors: &errors\n") + for i := range 20 { + fmt.Fprintf(&source, " '%d':\n description: Error %d\n content:\n application/json:\n schema:\n type: object\n properties:\n code: {type: integer}\n message: {type: string}\n", 400+i, i) + } + source.WriteString("paths:\n") + for i := range 5000 { + fmt.Fprintf(&source, " /r%d:\n get:\n operationId: op%d\n responses:\n '200': {description: ok}\n <<: *errors\n", i, i) + } + return source.String() +} + +func TestBuildV3Model_AllowsTrustedSharedResponses(t *testing.T) { + source := sharedResponseDocument() + for _, external := range []bool{false, true} { + for _, allow := range []bool{false, true} { + t.Run(fmt.Sprintf("external=%t/allow=%t", external, allow), func(t *testing.T) { + config := datamodel.NewDocumentConfiguration() + input := source + if external { + config = externalRegressionConfig(t, map[string]string{"routes.yaml": source}) + input = "openapi: 3.1.1\ninfo: {title: Linked response, version: '1'}\npaths:\n /linked: {$ref: './routes.yaml#/paths/~1r4999'}\n" + } + config.AllowExcessiveAliases = allow + doc, err := NewDocumentWithConfiguration([]byte(input), config) + require.NoError(t, err) + model, err := doc.BuildV3Model() + if !allow { + require.ErrorContains(t, err, "AllowExcessiveAliases") + require.Nil(t, model) + return + } + require.NoError(t, err) + require.NotNil(t, model) + require.True(t, model.Index.GetConfig().AllowExcessiveAliases) + if external { + require.Equal(t, 1, model.Model.Paths.PathItems.Len()) + require.Equal(t, "op4999", model.Model.Paths.PathItems.GetOrZero("/linked").Get.OperationId) + require.Len(t, model.Index.GetRolodex().GetIndexes(), 1) + require.Equal(t, 5000, model.Index.GetRolodex().GetIndexes()[0].GetOperationCount()) + } else { + require.Equal(t, 5000, model.Model.Paths.PathItems.Len()) + } + for item := range model.Model.Paths.PathItems.ValuesFromOldest() { + require.Equal(t, 21, item.Get.Responses.Codes.Len()) + require.Equal(t, "Error 19", item.Get.Responses.Codes.GetOrZero("419").Description) + } + }) + } + } +} + +func TestBuildV2Model_PropagatesAliasPolicy(t *testing.T) { + config := datamodel.NewDocumentConfiguration() + config.AllowExcessiveAliases = true + doc, err := NewDocumentWithConfiguration([]byte("swagger: '2.0'\ninfo: {title: Alias policy, version: '1'}\npaths: {}\n"), config) + require.NoError(t, err) + model, err := doc.BuildV2Model() + require.NoError(t, err) + require.True(t, model.Index.GetConfig().AllowExcessiveAliases) + require.True(t, model.Index.GetConfig().ToDocumentConfiguration().AllowExcessiveAliases) +} + func TestBuildV3ModelReportsAliasLimit(t *testing.T) { // A small shared graph crosses the parser's alias ratio limit during indexing. // It has no references, so only the traversal limit can report this error. diff --git a/index/extract_refs.go b/index/extract_refs.go index ad0e90d0..32e06f5e 100644 --- a/index/extract_refs.go +++ b/index/extract_refs.go @@ -12,9 +12,6 @@ import ( "github.com/pb33f/libopenapi/utils" ) -// Use the parser's existing alias policy when traversing its unexpanded node tree. -var extractRefsAliasLimit limit.Plugin - func isSchemaContainingNode(v string) bool { switch v { case "schema", "items", "additionalProperties", "contains", "not", @@ -121,16 +118,20 @@ type extractRefsTraversal struct { nodes int aliasNodes int aliasDepth int + if index.config != nil && index.config.AllowExcessiveAliases { + walk.limits = *limit.New(limit.AliasNone()) + } stopped bool } func (walk *extractRefsTraversal) visit() error { + limits limit.Plugin walk.nodes++ if walk.aliasDepth == 0 { return nil } walk.aliasNodes++ - return extractRefsAliasLimit.CheckAlias(walk.aliasNodes, walk.nodes) + return walk.limits.CheckAlias(walk.aliasNodes, walk.nodes) } // Ancestry tracks canonical source nodes, not the shallow views created during @@ -151,7 +152,8 @@ func (index *SpecIndex) extractRefs(ctx context.Context, node, parent *yaml.Node walk.stopped = true // A stopped traversal cannot provide a complete model. index.refErrors = append(index.refErrors, &ResolvingError{ - ErrorRef: err, Node: node, Path: "$." + strings.Join(seenPath, "."), + ErrorRef: fmt.Errorf("%w; set AllowExcessiveAliases for trusted documents", err), + Node: node, Path: "$." + strings.Join(seenPath, "."), }) return nil } diff --git a/index/index_model.go b/index/index_model.go index f10c2d0b..b0a81c01 100644 --- a/index/index_model.go +++ b/index/index_model.go @@ -85,6 +85,9 @@ func (rm *ReferenceMapped) MarshalJSON() ([]byte, error) { // allowing or disallowing remote or local file lookups. // - https://github.com/pb33f/libopenapi/issues/73 type SpecIndexConfig struct { + // AllowExcessiveAliases disables the YAML alias expansion limit for trusted + // documents with extensive shared content. Cycle detection remains enabled. + AllowExcessiveAliases bool // The BaseURL will be the root from which relative references will be resolved from if they can't be found locally. // // For example: @@ -282,6 +285,7 @@ func (s *SpecIndexConfig) ToDocumentConfiguration() *datamodel.DocumentConfigura BasePath: s.BasePath, SpecFilePath: s.SpecFilePath, AllowFileReferences: s.AllowFileLookup, + AllowExcessiveAliases: s.AllowExcessiveAliases, AllowRemoteReferences: s.AllowRemoteLookup, BypassDocumentCheck: s.SkipDocumentCheck, IgnorePolymorphicCircularReferences: s.IgnorePolymorphicCircularReferences, From 23bce344fe905c11914ffe3a23d23a5a86a51788 Mon Sep 17 00:00:00 2001 From: quobix Date: Sat, 31 Oct 2026 16:59:48 -0400 Subject: [PATCH 24/33] fix: reject path reference cycles before model construction --- datamodel/low/v2/swagger.go | 6 ++++ datamodel/low/v2/swagger_test.go | 17 +++++++++++ datamodel/low/v3/create_document.go | 6 ++++ datamodel/low/v3/create_document_test.go | 17 +++++++++++ document_path_cycle_test.go | 39 ++++++++++++++++++++++++ index/resolver_entry.go | 3 ++ index/utility_methods.go | 39 +++++++++++++++++++----- index/utility_methods_test.go | 12 ++++++++ 8 files changed, 132 insertions(+), 7 deletions(-) create mode 100644 document_path_cycle_test.go diff --git a/datamodel/low/v2/swagger.go b/datamodel/low/v2/swagger.go index c1529ed9..c7dc6f3b 100644 --- a/datamodel/low/v2/swagger.go +++ b/datamodel/low/v2/swagger.go @@ -237,6 +237,12 @@ func createDocument(info *datamodel.SpecInfo, config *datamodel.DocumentConfigur // set the index on the document. doc.Index = rolodex.GetRootIndex() doc.SpecInfo = info + for _, err := range roloErrs { + var resolvingErr *index.ResolvingError + if errors.As(err, &resolvingErr) && errors.Is(resolvingErr.ErrorRef, index.ErrReferenceChainCycle) { + return &doc, errors.Join(errs...) + } + } // build out swagger scalar variables. _ = low.BuildModel(info.RootNode.Content[0], &doc) diff --git a/datamodel/low/v2/swagger_test.go b/datamodel/low/v2/swagger_test.go index f66475ff..f9321ed8 100644 --- a/datamodel/low/v2/swagger_test.go +++ b/datamodel/low/v2/swagger_test.go @@ -519,3 +519,20 @@ paths: {} assert.NotNil(t, doc.Index.GetConfig()) assert.True(t, doc.Index.GetConfig().ResolveNestedRefsWithDocumentContext) } + +func TestCreateDocument_PathReferenceCycle(t *testing.T) { + info, err := datamodel.ExtractSpecInfo([]byte(`swagger: '2.0' +info: {title: Path cycle, version: '1'} +x-a: {$ref: '#/x-b'} +x-b: {$ref: '#/x-a'} +paths: + /a: {$ref: '#/x-a'} +`)) + require.NoError(t, err) + config := datamodel.NewDocumentConfiguration() + config.SkipCircularReferenceCheck = true + doc, err := CreateDocumentFromConfig(info, config) + require.ErrorContains(t, err, "circular reference chain") + require.NotNil(t, doc.Index) + require.Nil(t, doc.Paths.Value) +} diff --git a/datamodel/low/v3/create_document.go b/datamodel/low/v3/create_document.go index 57e01d3d..126dedc3 100644 --- a/datamodel/low/v3/create_document.go +++ b/datamodel/low/v3/create_document.go @@ -302,6 +302,12 @@ func createDocument(info *datamodel.SpecInfo, config *datamodel.DocumentConfigur // set root index. doc.Index = rolodex.GetRootIndex() + for _, err := range roloErrs { + var resolvingErr *index.ResolvingError + if errors.As(err, &resolvingErr) && errors.Is(resolvingErr.ErrorRef, index.ErrReferenceChainCycle) { + return &doc, errors.Join(errs...) + } + } var wg sync.WaitGroup var cacheMap sync.Map diff --git a/datamodel/low/v3/create_document_test.go b/datamodel/low/v3/create_document_test.go index 5ffe00d2..bc67e32f 100644 --- a/datamodel/low/v3/create_document_test.go +++ b/datamodel/low/v3/create_document_test.go @@ -1483,3 +1483,20 @@ paths: {} assert.NotNil(t, doc.Index.GetConfig()) assert.True(t, doc.Index.GetConfig().ResolveNestedRefsWithDocumentContext) } + +func TestCreateDocument_PathReferenceCycle(t *testing.T) { + info, err := datamodel.ExtractSpecInfo([]byte(`openapi: 3.1.1 +info: {title: Path cycle, version: '1'} +x-a: {$ref: '#/x-b'} +x-b: {$ref: '#/x-a'} +paths: + /a: {$ref: '#/x-a'} +`)) + require.NoError(t, err) + config := datamodel.NewDocumentConfiguration() + config.SkipCircularReferenceCheck = true + doc, err := CreateDocumentFromConfig(info, config) + require.ErrorContains(t, err, "circular reference chain") + require.NotNil(t, doc.Index) + require.Nil(t, doc.Paths.Value) +} diff --git a/document_path_cycle_test.go b/document_path_cycle_test.go new file mode 100644 index 00000000..d3273c2d --- /dev/null +++ b/document_path_cycle_test.go @@ -0,0 +1,39 @@ +package libopenapi + +import ( + "fmt" + "testing" + + "github.com/pb33f/libopenapi/datamodel" + "github.com/pb33f/testify/require" +) + +func TestBuildModel_PathReferenceCycleReturnsError(t *testing.T) { + for _, version := range []string{"swagger: '2.0'", "openapi: 3.1.1"} { + for _, skipCircular := range []bool{false, true} { + t.Run(fmt.Sprintf("%s/skipCircular=%t", version, skipCircular), func(t *testing.T) { + config := datamodel.NewDocumentConfiguration() + config.SkipCircularReferenceCheck = skipCircular + doc, err := NewDocumentWithConfiguration([]byte(version+` +info: {title: Path cycle, version: '1'} +x-a: {$ref: '#/x-b'} +x-b: {$ref: '#/x-a'} +paths: + /a: {$ref: '#/x-a'} +`), config) + require.NoError(t, err) + for range 2 { + if version == "swagger: '2.0'" { + model, buildErr := doc.BuildV2Model() + require.Nil(t, model) + require.ErrorContains(t, buildErr, "circular reference chain") + } else { + model, buildErr := doc.BuildV3Model() + require.Nil(t, model) + require.ErrorContains(t, buildErr, "circular reference chain") + } + } + }) + } + } +} diff --git a/index/resolver_entry.go b/index/resolver_entry.go index a604c7ed..4c094005 100644 --- a/index/resolver_entry.go +++ b/index/resolver_entry.go @@ -12,6 +12,9 @@ import ( "github.com/pb33f/libopenapi/utils" ) +// ErrReferenceChainCycle indicates a reference-only chain with no terminal value. +var ErrReferenceChainCycle = errors.New("circular reference chain detected") + // ResolvingError represents an issue the resolver had trying to stitch the tree together. type ResolvingError struct { ErrorRef error diff --git a/index/utility_methods.go b/index/utility_methods.go index d342fbe0..a68799cf 100644 --- a/index/utility_methods.go +++ b/index/utility_methods.go @@ -395,13 +395,38 @@ func (index *SpecIndex) countUniqueInlineDuplicates() int { } func seekRefEnd(ctx context.Context, index *SpecIndex, refName string) *Reference { - ref, _, nCtx := index.SearchIndexForReferenceWithContext(ctx, refName) - if ref != nil { - if ok, _, v := utils.IsNodeRefValue(ref.Node); ok { - return seekRefEnd(nCtx, ref.Index, v) - } - } - return ref + owner := index + type target struct { + index *SpecIndex + node *yaml.Node + } + var seen map[target]struct{} + for index != nil { + ref, nextIndex, nextContext := index.SearchIndexForReferenceWithContext(ctx, refName) + if ref == nil { + return nil + } + ok, _, next := utils.IsNodeRefValue(ref.Node) + if !ok { + return ref + } + key := target{nextIndex, ref.Node} + if _, repeated := seen[key]; repeated { + owner.errorLock.Lock() + owner.refErrors = append(owner.refErrors, &ResolvingError{ + ErrorRef: fmt.Errorf("%w while resolving '%s'", ErrReferenceChainCycle, refName), + Node: ref.Node, Path: ref.Path, + }) + owner.errorLock.Unlock() + return nil + } + if seen == nil { + seen = make(map[target]struct{}) + } + seen[key] = struct{}{} + ctx, index, refName = nextContext, nextIndex, next + } + return nil } // formatParameterPath creates a consistent JSON path for parameter error messages diff --git a/index/utility_methods_test.go b/index/utility_methods_test.go index dbf47a5b..052bb063 100644 --- a/index/utility_methods_test.go +++ b/index/utility_methods_test.go @@ -383,3 +383,15 @@ func Test_WriteIntToHash(t *testing.T) { func Test_Empty_HashNode(t *testing.T) { assert.Equal(t, emptyNodeHash, HashNode(nil)) } + +func TestSeekRefEndCycleAndMissingTarget(t *testing.T) { + var root yaml.Node + assert.NoError(t, yaml.Unmarshal([]byte("a: {$ref: '#/b'}\nb: {$ref: '#/a'}"), &root)) + config := CreateClosedAPIIndexConfig() + config.AvoidCircularReferenceCheck = true + idx := NewSpecIndexWithConfig(&root, config) + assert.Nil(t, seekRefEnd(context.Background(), idx, "#/a")) + assert.ErrorContains(t, idx.GetReferenceIndexErrors()[0], "circular reference chain") + assert.Nil(t, seekRefEnd(context.Background(), idx, "#/missing")) + assert.Nil(t, seekRefEnd(context.Background(), nil, "#/a")) +} From f5664c34f2262792d681622c9566f530b8623c96 Mon Sep 17 00:00:00 2001 From: quobix Date: Sat, 31 Oct 2026 16:59:48 -0400 Subject: [PATCH 25/33] fix(index): preserve example payloads under schema keyword names --- index/extract_refs.go | 36 ++++++++++++++++++++++-------------- index/extract_refs_test.go | 20 ++++++++++++++++++++ 2 files changed, 42 insertions(+), 14 deletions(-) diff --git a/index/extract_refs.go b/index/extract_refs.go index 32e06f5e..7850c2d6 100644 --- a/index/extract_refs.go +++ b/index/extract_refs.go @@ -5,6 +5,7 @@ package index import ( "context" + "fmt" "strings" "github.com/pb33f/go-yaml" @@ -53,18 +54,25 @@ func isOpenAPIExampleKeywordSegment(seenPath []string, idx int) bool { if idx < 0 || idx >= len(seenPath) { return false } - switch seenPath[idx] { - case "example", "examples": - if idx > 0 { - switch seenPath[idx-1] { - case "properties", "patternProperties", "schemas", "definitions", "$defs", "dependentSchemas": - return false - } - } - return true - default: + if seenPath[idx] != "example" && seenPath[idx] != "examples" { return false } + // A name in a schema map cannot itself introduce another schema map. + // For example, properties/definitions/example ends at a payload keyword. + nameSlot := false + for i, segment := range seenPath[:idx] { + if nameSlot { + nameSlot = false + continue + } + switch segment { + case "properties", "patternProperties", "definitions", "$defs", "dependentSchemas": + nameSlot = true + case "schemas": + nameSlot = i == 1 && seenPath[0] == "components" + } + } + return !nameSlot } // underOpenAPIExamplePayloadPath reports whether seenPath points to raw example payload content. @@ -110,22 +118,22 @@ func isDirectOpenAPIExampleValuePath(seenPath []string) bool { // Excessive YAML alias expansion stops extraction and is reported by GetReferenceIndexErrors. func (index *SpecIndex) ExtractRefs(ctx context.Context, node, parent *yaml.Node, seenPath []string, level int, poly bool, pName string) []*Reference { walk := &extractRefsTraversal{} + if index.config != nil && index.config.AllowExcessiveAliases { + walk.limits = *limit.New(limit.AliasNone()) + } return index.extractRefs(ctx, node, parent, seenPath, level, poly, pName, walk) } type extractRefsTraversal struct { + limits limit.Plugin ancestry map[*yaml.Node]struct{} nodes int aliasNodes int aliasDepth int - if index.config != nil && index.config.AllowExcessiveAliases { - walk.limits = *limit.New(limit.AliasNone()) - } stopped bool } func (walk *extractRefsTraversal) visit() error { - limits limit.Plugin walk.nodes++ if walk.aliasDepth == 0 { return nil diff --git a/index/extract_refs_test.go b/index/extract_refs_test.go index 43e888ab..71b481d3 100644 --- a/index/extract_refs_test.go +++ b/index/extract_refs_test.go @@ -5,6 +5,7 @@ package index import ( "context" + "fmt" "runtime" "strings" "testing" @@ -1226,3 +1227,22 @@ components: assert.True(t, rawRefs["#/components/examples/ReusableExample"]) assert.False(t, rawRefs["#/components/schemas/ShouldNotIndex"]) } + +// Copyright 2023 Princess B33f Heavy Industries / Dave Shanley +// SPDX-License-Identifier: MIT + +func TestExtractRefs_ExampleUnderSchemaKeywordName(t *testing.T) { + for _, name := range []string{"schemas", "definitions", "$defs", "dependentSchemas", "properties", "patternProperties"} { + t.Run(name, func(t *testing.T) { + source := fmt.Sprintf("openapi: 3.1.1\ninfo: {title: Payload, version: '1'}\npaths: {}\ncomponents:\n schemas:\n Thing:\n type: object\n properties:\n %s:\n type: object\n example:\n nested: {$ref: '#/not-a-schema'}\n", name) + var root yaml.Node + if err := yaml.Unmarshal([]byte(source), &root); err != nil { + t.Fatal(err) + } + idx := NewSpecIndex(&root) + if refs := idx.GetRawReferencesSequenced(); len(refs) != 0 { + t.Fatalf("example payload indexed: refs=%d first=%s path=%v errors=%v", len(refs), refs[0].Definition, refs[0].SourcePath, idx.GetReferenceIndexErrors()) + } + }) + } +} From eed0290139cd7a49e1c2555d90bfc4c85c60c545 Mon Sep 17 00:00:00 2001 From: quobix Date: Sat, 31 Oct 2026 16:59:48 -0400 Subject: [PATCH 26/33] refactor(low): clarify the integer magnitude limit --- datamodel/low/integer_decimal.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/datamodel/low/integer_decimal.go b/datamodel/low/integer_decimal.go index 88dea2fc..6ba94eca 100644 --- a/datamodel/low/integer_decimal.go +++ b/datamodel/low/integer_decimal.go @@ -61,9 +61,9 @@ func parseDecimalInt64(value string) (int64, bool) { if integerDigits > 19 { return limit, true } - max := uint64(math.MaxInt64) + maxMagnitude := uint64(math.MaxInt64) if negative { - max++ + maxMagnitude++ } var result uint64 remaining := integerDigits @@ -85,7 +85,7 @@ func parseDecimalInt64(value string) (int64, bool) { result *= 10 remaining-- } - if result > max { + if result > maxMagnitude { return limit, true } if negative { From 835d1d8844e874485b112b50ddbd26a689743626 Mon Sep 17 00:00:00 2001 From: quobix Date: Sat, 31 Oct 2026 17:03:40 -0400 Subject: [PATCH 27/33] perf(what-changed): bound parallel comparisons and avoid absent-field checks --- what-changed/model/schema.go | 414 ++++--------------- what-changed/model/schema_comparison.go | 133 +++++- what-changed/model/schema_comparison_test.go | 90 +++- 3 files changed, 285 insertions(+), 352 deletions(-) diff --git a/what-changed/model/schema.go b/what-changed/model/schema.go index 199487b5..b87bb0b3 100644 --- a/what-changed/model/schema.go +++ b/what-changed/model/schema.go @@ -589,11 +589,25 @@ func (c *schemaComparison) compareSchemas(l, r *base.SchemaProxy) *SchemaChanges patterns := c.checkMappedSchemaOfASchema(lPattProp, rPattProp, &changes) sc.PatternPropertiesChanges = patterns - // Stable traversal gives each resolved schema pair one consistent report location. - c.extractSchemaChanges(loneOf, roneOf, v3.OneOfLabel, &sc.OneOfChanges, &changes) - c.extractSchemaChanges(lallOf, rallOf, v3.AllOfLabel, &sc.AllOfChanges, &changes) - c.extractSchemaChanges(lanyOf, ranyOf, v3.AnyOfLabel, &sc.AnyOfChanges, &changes) - c.extractSchemaChanges(lprefix, rprefix, v3.PrefixItemsLabel, &sc.PrefixItemsChanges, &changes) + compositionCount := 0 + for _, size := range [4]int{len(loneOf) + len(roneOf), len(lallOf) + len(rallOf), len(lanyOf) + len(ranyOf), len(lprefix) + len(rprefix)} { + if size > 0 { + compositionCount++ + } + } + if c.serial || compositionCount < 2 { + c.extractSchemaChanges(loneOf, roneOf, v3.OneOfLabel, &sc.OneOfChanges, &changes) + c.extractSchemaChanges(lallOf, rallOf, v3.AllOfLabel, &sc.AllOfChanges, &changes) + c.extractSchemaChanges(lanyOf, ranyOf, v3.AnyOfLabel, &sc.AnyOfChanges, &changes) + c.extractSchemaChanges(lprefix, rprefix, v3.PrefixItemsLabel, &sc.PrefixItemsChanges, &changes) + } else { + c.compareCompositions([]schemaCompositionTask{ + {left: loneOf, right: roneOf, label: v3.OneOfLabel, result: &sc.OneOfChanges}, + {left: lallOf, right: rallOf, label: v3.AllOfLabel, result: &sc.AllOfChanges}, + {left: lanyOf, right: ranyOf, label: v3.AnyOfLabel, result: &sc.AnyOfChanges}, + {left: lprefix, right: rprefix, label: v3.PrefixItemsLabel, result: &sc.PrefixItemsChanges}, + }, &changes) + } } // done @@ -636,6 +650,9 @@ func (c *schemaComparison) checkMappedSchemaOfASchema( rSchema *orderedmap.Map[low.KeyReference[string], low.ValueReference[*base.SchemaProxy]], changes *[]*Change, ) map[string]*SchemaChanges { + if orderedmap.Len(lSchema) == 0 && orderedmap.Len(rSchema) == 0 { + return nil + } propChanges := make(map[string]*SchemaChanges) var lProps []string lEntities := make(map[string]*base.SchemaProxy) @@ -664,6 +681,12 @@ func (c *schemaComparison) checkMappedSchemaOfASchema( func (c *schemaComparison) buildProperty(lProps, rProps []string, lEntities, rEntities map[string]*base.SchemaProxy, propChanges map[string]*SchemaChanges, changes *[]*Change, rKeyNodes, lKeyNodes map[string]*yaml.Node, ) { + type propertyTask struct { + key string + left, right *base.SchemaProxy + result *SchemaChanges + } + var tasks []propertyTask checkProperty := func(key string, lp, rp *base.SchemaProxy) { if _, compared := propChanges[key]; compared { return @@ -671,8 +694,12 @@ func (c *schemaComparison) buildProperty(lProps, rProps []string, lEntities, rEn if low.AreEqual(lp, rp) { return } - s := c.compareSchemas(lp, rp) - propChanges[key] = s + if c.serial || min(len(lProps), len(rProps)) < 2 { + propChanges[key] = c.compareSchemas(lp, rp) + return + } + propChanges[key] = nil + tasks = append(tasks, propertyTask{key: key, left: lp, right: rp}) } // left and right equal. @@ -743,6 +770,12 @@ func (c *schemaComparison) buildProperty(lProps, rProps []string, lEntities, rEn } } } + c.branch(len(tasks), func(i int, branch *schemaComparison) { + tasks[i].result = branch.compareSchemas(tasks[i].left, tasks[i].right) + }) + for _, task := range tasks { + propChanges[task.key] = task.result + } } func (c *schemaComparison) checkSchemaPropertyChanges( @@ -753,6 +786,13 @@ func (c *schemaComparison) checkSchemaPropertyChanges( changes *[]*Change, sc *SchemaChanges, skipSimpleScalarUnionDiff bool, ) { var props []*PropertyCheck + addCheck := func(left, right *yaml.Node, label, property string) { + if left == nil && right == nil { + return + } + addPropertyCheck(&props, left, right, lSchema, rSchema, changes, label, + BreakingModified(CompSchema, property), CompSchema, property) + } // $schema (breaking change) var lnv, rnv *yaml.Node @@ -763,17 +803,7 @@ func (c *schemaComparison) checkSchemaPropertyChanges( rnv = rSchema.SchemaTypeRef.ValueNode } - props = append(props, &PropertyCheck{ - LeftNode: lnv, - RightNode: rnv, - Label: v3.SchemaDialectLabel, - Changes: changes, - Breaking: BreakingModified(CompSchema, PropSchemaDialect), - Component: CompSchema, - Property: PropSchemaDialect, - Original: lSchema, - New: rSchema, - }) + addCheck(lnv, rnv, v3.SchemaDialectLabel, PropSchemaDialect) lnv = nil rnv = nil @@ -784,17 +814,7 @@ func (c *schemaComparison) checkSchemaPropertyChanges( rnv = rSchema.ExclusiveMaximum.ValueNode } // ExclusiveMaximum - props = append(props, &PropertyCheck{ - LeftNode: lnv, - RightNode: rnv, - Label: v3.ExclusiveMaximumLabel, - Changes: changes, - Breaking: BreakingModified(CompSchema, PropExclusiveMaximum), - Component: CompSchema, - Property: PropExclusiveMaximum, - Original: lSchema, - New: rSchema, - }) + addCheck(lnv, rnv, v3.ExclusiveMaximumLabel, PropExclusiveMaximum) lnv = nil rnv = nil @@ -806,17 +826,7 @@ func (c *schemaComparison) checkSchemaPropertyChanges( } // ExclusiveMinimum - props = append(props, &PropertyCheck{ - LeftNode: lnv, - RightNode: rnv, - Label: v3.ExclusiveMinimumLabel, - Changes: changes, - Breaking: BreakingModified(CompSchema, PropExclusiveMinimum), - Component: CompSchema, - Property: PropExclusiveMinimum, - Original: lSchema, - New: rSchema, - }) + addCheck(lnv, rnv, v3.ExclusiveMinimumLabel, PropExclusiveMinimum) lnv = nil rnv = nil @@ -828,17 +838,7 @@ func (c *schemaComparison) checkSchemaPropertyChanges( rnv = rSchema.Type.ValueNode } // Type - props = append(props, &PropertyCheck{ - LeftNode: lnv, - RightNode: rnv, - Label: v3.TypeLabel, - Changes: changes, - Breaking: BreakingModified(CompSchema, PropType), - Component: CompSchema, - Property: PropType, - Original: lSchema, - New: rSchema, - }) + addCheck(lnv, rnv, v3.TypeLabel, PropType) lnv = nil rnv = nil } @@ -850,17 +850,7 @@ func (c *schemaComparison) checkSchemaPropertyChanges( rnv = rSchema.Title.ValueNode } // Title - props = append(props, &PropertyCheck{ - LeftNode: lnv, - RightNode: rnv, - Label: v3.TitleLabel, - Changes: changes, - Breaking: BreakingModified(CompSchema, PropTitle), - Component: CompSchema, - Property: PropTitle, - Original: lSchema, - New: rSchema, - }) + addCheck(lnv, rnv, v3.TitleLabel, PropTitle) lnv = nil rnv = nil @@ -872,17 +862,7 @@ func (c *schemaComparison) checkSchemaPropertyChanges( } // MultipleOf - props = append(props, &PropertyCheck{ - LeftNode: lnv, - RightNode: rnv, - Label: v3.MultipleOfLabel, - Changes: changes, - Breaking: BreakingModified(CompSchema, PropMultipleOf), - Component: CompSchema, - Property: PropMultipleOf, - Original: lSchema, - New: rSchema, - }) + addCheck(lnv, rnv, v3.MultipleOfLabel, PropMultipleOf) lnv = nil rnv = nil @@ -893,17 +873,7 @@ func (c *schemaComparison) checkSchemaPropertyChanges( rnv = rSchema.Maximum.ValueNode } // Maximum - props = append(props, &PropertyCheck{ - LeftNode: lnv, - RightNode: rnv, - Label: v3.MaximumLabel, - Changes: changes, - Breaking: BreakingModified(CompSchema, PropMaximum), - Component: CompSchema, - Property: PropMaximum, - Original: lSchema, - New: rSchema, - }) + addCheck(lnv, rnv, v3.MaximumLabel, PropMaximum) lnv = nil rnv = nil @@ -914,17 +884,7 @@ func (c *schemaComparison) checkSchemaPropertyChanges( rnv = rSchema.Minimum.ValueNode } // Minimum - props = append(props, &PropertyCheck{ - LeftNode: lnv, - RightNode: rnv, - Label: v3.MinimumLabel, - Changes: changes, - Breaking: BreakingModified(CompSchema, PropMinimum), - Component: CompSchema, - Property: PropMinimum, - Original: lSchema, - New: rSchema, - }) + addCheck(lnv, rnv, v3.MinimumLabel, PropMinimum) lnv = nil rnv = nil @@ -935,17 +895,7 @@ func (c *schemaComparison) checkSchemaPropertyChanges( rnv = rSchema.MaxLength.ValueNode } // MaxLength - props = append(props, &PropertyCheck{ - LeftNode: lnv, - RightNode: rnv, - Label: v3.MaxLengthLabel, - Changes: changes, - Breaking: BreakingModified(CompSchema, PropMaxLength), - Component: CompSchema, - Property: PropMaxLength, - Original: lSchema, - New: rSchema, - }) + addCheck(lnv, rnv, v3.MaxLengthLabel, PropMaxLength) lnv = nil rnv = nil @@ -956,17 +906,7 @@ func (c *schemaComparison) checkSchemaPropertyChanges( rnv = rSchema.MinLength.ValueNode } // MinLength - props = append(props, &PropertyCheck{ - LeftNode: lnv, - RightNode: rnv, - Label: v3.MinLengthLabel, - Changes: changes, - Breaking: BreakingModified(CompSchema, PropMinLength), - Component: CompSchema, - Property: PropMinLength, - Original: lSchema, - New: rSchema, - }) + addCheck(lnv, rnv, v3.MinLengthLabel, PropMinLength) lnv = nil rnv = nil @@ -977,17 +917,7 @@ func (c *schemaComparison) checkSchemaPropertyChanges( rnv = rSchema.Pattern.ValueNode } // Pattern - props = append(props, &PropertyCheck{ - LeftNode: lnv, - RightNode: rnv, - Label: v3.PatternLabel, - Changes: changes, - Breaking: BreakingModified(CompSchema, PropPattern), - Component: CompSchema, - Property: PropPattern, - Original: lSchema, - New: rSchema, - }) + addCheck(lnv, rnv, v3.PatternLabel, PropPattern) lnv = nil rnv = nil @@ -998,17 +928,7 @@ func (c *schemaComparison) checkSchemaPropertyChanges( rnv = rSchema.Format.ValueNode } // Format - props = append(props, &PropertyCheck{ - LeftNode: lnv, - RightNode: rnv, - Label: v3.FormatLabel, - Changes: changes, - Breaking: BreakingModified(CompSchema, PropFormat), - Component: CompSchema, - Property: PropFormat, - Original: lSchema, - New: rSchema, - }) + addCheck(lnv, rnv, v3.FormatLabel, PropFormat) lnv = nil rnv = nil @@ -1019,17 +939,7 @@ func (c *schemaComparison) checkSchemaPropertyChanges( rnv = rSchema.MaxItems.ValueNode } // MaxItems - props = append(props, &PropertyCheck{ - LeftNode: lnv, - RightNode: rnv, - Label: v3.MaxItemsLabel, - Changes: changes, - Breaking: BreakingModified(CompSchema, PropMaxItems), - Component: CompSchema, - Property: PropMaxItems, - Original: lSchema, - New: rSchema, - }) + addCheck(lnv, rnv, v3.MaxItemsLabel, PropMaxItems) lnv = nil rnv = nil @@ -1040,17 +950,7 @@ func (c *schemaComparison) checkSchemaPropertyChanges( rnv = rSchema.MinItems.ValueNode } // MinItems - props = append(props, &PropertyCheck{ - LeftNode: lnv, - RightNode: rnv, - Label: v3.MinItemsLabel, - Changes: changes, - Breaking: BreakingModified(CompSchema, PropMinItems), - Component: CompSchema, - Property: PropMinItems, - Original: lSchema, - New: rSchema, - }) + addCheck(lnv, rnv, v3.MinItemsLabel, PropMinItems) lnv = nil rnv = nil @@ -1061,17 +961,7 @@ func (c *schemaComparison) checkSchemaPropertyChanges( rnv = rSchema.MaxProperties.ValueNode } // MaxProperties - props = append(props, &PropertyCheck{ - LeftNode: lnv, - RightNode: rnv, - Label: v3.MaxPropertiesLabel, - Changes: changes, - Breaking: BreakingModified(CompSchema, PropMaxProperties), - Component: CompSchema, - Property: PropMaxProperties, - Original: lSchema, - New: rSchema, - }) + addCheck(lnv, rnv, v3.MaxPropertiesLabel, PropMaxProperties) lnv = nil rnv = nil @@ -1083,17 +973,7 @@ func (c *schemaComparison) checkSchemaPropertyChanges( } // MinProperties - props = append(props, &PropertyCheck{ - LeftNode: lnv, - RightNode: rnv, - Label: v3.MinPropertiesLabel, - Changes: changes, - Breaking: BreakingModified(CompSchema, PropMinProperties), - Component: CompSchema, - Property: PropMinProperties, - Original: lSchema, - New: rSchema, - }) + addCheck(lnv, rnv, v3.MinPropertiesLabel, PropMinProperties) lnv = nil rnv = nil @@ -1104,17 +984,7 @@ func (c *schemaComparison) checkSchemaPropertyChanges( rnv = rSchema.UniqueItems.ValueNode } // UniqueItems - props = append(props, &PropertyCheck{ - LeftNode: lnv, - RightNode: rnv, - Label: v3.UniqueItemsLabel, - Changes: changes, - Breaking: BreakingModified(CompSchema, PropUniqueItems), - Component: CompSchema, - Property: PropUniqueItems, - Original: lSchema, - New: rSchema, - }) + addCheck(lnv, rnv, v3.UniqueItemsLabel, PropUniqueItems) lnv = nil rnv = nil @@ -1158,17 +1028,7 @@ func (c *schemaComparison) checkSchemaPropertyChanges( rnv = rSchema.Description.ValueNode } // Description - props = append(props, &PropertyCheck{ - LeftNode: lnv, - RightNode: rnv, - Label: v3.DescriptionLabel, - Changes: changes, - Breaking: BreakingModified(CompSchema, PropDescription), - Component: CompSchema, - Property: PropDescription, - Original: lSchema, - New: rSchema, - }) + addCheck(lnv, rnv, v3.DescriptionLabel, PropDescription) lnv = nil rnv = nil @@ -1179,17 +1039,7 @@ func (c *schemaComparison) checkSchemaPropertyChanges( rnv = rSchema.ContentEncoding.ValueNode } // ContentEncoding - props = append(props, &PropertyCheck{ - LeftNode: lnv, - RightNode: rnv, - Label: v3.ContentEncodingLabel, - Changes: changes, - Breaking: BreakingModified(CompSchema, PropContentEncoding), - Component: CompSchema, - Property: PropContentEncoding, - Original: lSchema, - New: rSchema, - }) + addCheck(lnv, rnv, v3.ContentEncodingLabel, PropContentEncoding) lnv = nil rnv = nil @@ -1200,17 +1050,7 @@ func (c *schemaComparison) checkSchemaPropertyChanges( rnv = rSchema.ContentMediaType.ValueNode } // ContentMediaType - props = append(props, &PropertyCheck{ - LeftNode: lnv, - RightNode: rnv, - Label: v3.ContentMediaType, - Changes: changes, - Breaking: BreakingModified(CompSchema, PropContentMediaType), - Component: CompSchema, - Property: PropContentMediaType, - Original: lSchema, - New: rSchema, - }) + addCheck(lnv, rnv, v3.ContentMediaType, PropContentMediaType) lnv = nil rnv = nil @@ -1221,17 +1061,7 @@ func (c *schemaComparison) checkSchemaPropertyChanges( rnv = rSchema.Default.ValueNode } // Default - props = append(props, &PropertyCheck{ - LeftNode: lnv, - RightNode: rnv, - Label: v3.DefaultLabel, - Changes: changes, - Breaking: BreakingModified(CompSchema, PropDefault), - Component: CompSchema, - Property: PropDefault, - Original: lSchema, - New: rSchema, - }) + addCheck(lnv, rnv, v3.DefaultLabel, PropDefault) lnv = nil rnv = nil @@ -1242,17 +1072,7 @@ func (c *schemaComparison) checkSchemaPropertyChanges( rnv = rSchema.Const.ValueNode } // Const - props = append(props, &PropertyCheck{ - LeftNode: lnv, - RightNode: rnv, - Label: v3.ConstLabel, - Changes: changes, - Breaking: BreakingModified(CompSchema, PropConst), - Component: CompSchema, - Property: PropConst, - Original: lSchema, - New: rSchema, - }) + addCheck(lnv, rnv, v3.ConstLabel, PropConst) lnv = nil rnv = nil @@ -1263,17 +1083,7 @@ func (c *schemaComparison) checkSchemaPropertyChanges( rnv = rSchema.Nullable.ValueNode } // Nullable - props = append(props, &PropertyCheck{ - LeftNode: lnv, - RightNode: rnv, - Label: v3.NullableLabel, - Changes: changes, - Breaking: BreakingModified(CompSchema, PropNullable), - Component: CompSchema, - Property: PropNullable, - Original: lSchema, - New: rSchema, - }) + addCheck(lnv, rnv, v3.NullableLabel, PropNullable) lnv = nil rnv = nil @@ -1284,17 +1094,7 @@ func (c *schemaComparison) checkSchemaPropertyChanges( rnv = rSchema.ReadOnly.ValueNode } // ReadOnly - props = append(props, &PropertyCheck{ - LeftNode: lnv, - RightNode: rnv, - Label: v3.ReadOnlyLabel, - Changes: changes, - Breaking: BreakingModified(CompSchema, PropReadOnly), - Component: CompSchema, - Property: PropReadOnly, - Original: lSchema, - New: rSchema, - }) + addCheck(lnv, rnv, v3.ReadOnlyLabel, PropReadOnly) lnv = nil rnv = nil @@ -1305,17 +1105,7 @@ func (c *schemaComparison) checkSchemaPropertyChanges( rnv = rSchema.WriteOnly.ValueNode } // WriteOnly - props = append(props, &PropertyCheck{ - LeftNode: lnv, - RightNode: rnv, - Label: v3.WriteOnlyLabel, - Changes: changes, - Breaking: BreakingModified(CompSchema, PropWriteOnly), - Component: CompSchema, - Property: PropWriteOnly, - Original: lSchema, - New: rSchema, - }) + addCheck(lnv, rnv, v3.WriteOnlyLabel, PropWriteOnly) lnv = nil rnv = nil @@ -1340,17 +1130,7 @@ func (c *schemaComparison) checkSchemaPropertyChanges( rnv = rSchema.Deprecated.ValueNode } // Deprecated - props = append(props, &PropertyCheck{ - LeftNode: lnv, - RightNode: rnv, - Label: v3.DeprecatedLabel, - Changes: changes, - Breaking: BreakingModified(CompSchema, PropDeprecated), - Component: CompSchema, - Property: PropDeprecated, - Original: lSchema, - New: rSchema, - }) + addCheck(lnv, rnv, v3.DeprecatedLabel, PropDeprecated) // Required j := make(map[string]int) @@ -1622,17 +1402,7 @@ func (c *schemaComparison) checkSchemaPropertyChanges( if rSchema != nil && rSchema.DynamicAnchor.ValueNode != nil { rnv = rSchema.DynamicAnchor.ValueNode } - props = append(props, &PropertyCheck{ - LeftNode: lnv, - RightNode: rnv, - Label: v3.DynamicAnchorLabel, - Changes: changes, - Breaking: BreakingModified(CompSchema, PropDynamicAnchor), - Component: CompSchema, - Property: PropDynamicAnchor, - Original: lSchema, - New: rSchema, - }) + addCheck(lnv, rnv, v3.DynamicAnchorLabel, PropDynamicAnchor) lnv = nil rnv = nil @@ -1643,17 +1413,7 @@ func (c *schemaComparison) checkSchemaPropertyChanges( if rSchema != nil && rSchema.DynamicRef.ValueNode != nil { rnv = rSchema.DynamicRef.ValueNode } - props = append(props, &PropertyCheck{ - LeftNode: lnv, - RightNode: rnv, - Label: v3.DynamicRefLabel, - Changes: changes, - Breaking: BreakingModified(CompSchema, PropDynamicRef), - Component: CompSchema, - Property: PropDynamicRef, - Original: lSchema, - New: rSchema, - }) + addCheck(lnv, rnv, v3.DynamicRefLabel, PropDynamicRef) lnv = nil rnv = nil @@ -1664,17 +1424,7 @@ func (c *schemaComparison) checkSchemaPropertyChanges( if rSchema != nil && rSchema.Id.ValueNode != nil { rnv = rSchema.Id.ValueNode } - props = append(props, &PropertyCheck{ - LeftNode: lnv, - RightNode: rnv, - Label: base.IdLabel, - Changes: changes, - Breaking: BreakingModified(CompSchema, PropId), - Component: CompSchema, - Property: PropId, - Original: lSchema, - New: rSchema, - }) + addCheck(lnv, rnv, base.IdLabel, PropId) lnv = nil rnv = nil @@ -1685,17 +1435,7 @@ func (c *schemaComparison) checkSchemaPropertyChanges( if rSchema != nil && rSchema.Comment.ValueNode != nil { rnv = rSchema.Comment.ValueNode } - props = append(props, &PropertyCheck{ - LeftNode: lnv, - RightNode: rnv, - Label: base.CommentLabel, - Changes: changes, - Breaking: BreakingModified(CompSchema, PropComment), - Component: CompSchema, - Property: PropComment, - Original: lSchema, - New: rSchema, - }) + addCheck(lnv, rnv, base.CommentLabel, PropComment) lnv = nil rnv = nil diff --git a/what-changed/model/schema_comparison.go b/what-changed/model/schema_comparison.go index c5922886..5e5cb8cc 100644 --- a/what-changed/model/schema_comparison.go +++ b/what-changed/model/schema_comparison.go @@ -2,16 +2,92 @@ package model import ( "github.com/pb33f/go-yaml" + "github.com/pb33f/libopenapi/datamodel/low" "github.com/pb33f/libopenapi/datamodel/low/base" "github.com/pb33f/libopenapi/index" + "runtime" + "sync" ) // Each report owns its visited pairs. Candidate scores are shared within one // public comparison, but scoring never marks a pair visited in the report. type schemaComparison struct { - visited map[schemaComparisonPair]struct{} - scores map[schemaScorePair]schemaScore - scoring *schemaScoring + visited map[schemaComparisonPair]struct{} + inherited map[schemaComparisonPair]struct{} + previous map[schemaComparisonPair]struct{} + scores *schemaScores + scoring *schemaScoring + serial bool +} + +type schemaScores struct { + sync.RWMutex + values map[schemaScorePair]schemaScore +} + +// branch compares independent siblings with bounded workers. Forks read the +// parent's fixed visited set and keep private deltas. Reconcile in report order; +// replay an overlapping fork serially to retain one stable location per pair. +// Each worker remembers its earlier siblings so shared subtrees are expanded +// at most once per worker, rather than once per sibling. +func (c *schemaComparison) branch(count int, task func(int, *schemaComparison)) { + if c.serial || count < 2 || runtime.GOMAXPROCS(0) == 1 { + for i := range count { + task(i, c) + } + return + } + if c.scores == nil { + c.scores = new(schemaScores) + } + deltas := make([]map[schemaComparisonPair]struct{}, count) + workers := min(count, runtime.GOMAXPROCS(0), 4) + var wg sync.WaitGroup + for worker := range workers { + wg.Add(1) + go func() { + defer wg.Done() + fork := schemaComparison{inherited: c.visited, scores: c.scores, serial: true} + for i := worker; i < count; i += workers { + fork.visited = nil + task(i, &fork) + deltas[i] = fork.visited + if len(fork.visited) > 0 { + if fork.previous == nil { + fork.previous = make(map[schemaComparisonPair]struct{}) + } + for pair := range fork.visited { + fork.previous[pair] = struct{}{} + } + } + } + }() + } + wg.Wait() + for i, delta := range deltas { + overlap := false + for pair := range delta { + if _, exists := c.visited[pair]; exists { + overlap = true + break + } + } + if overlap { + c.serial = true + task(i, c) + c.serial = false + continue + } + if len(delta) > 0 { + if c.visited == nil { + c.visited = delta + } else { + for pair := range delta { + c.visited[pair] = struct{}{} + } + } + } + } } // Active scores belong to one traversal. A recursion cut must not publish a @@ -40,6 +116,31 @@ type schemaScore struct { total, breaking int } +type schemaCompositionTask struct { + left, right []low.ValueReference[*base.SchemaProxy] + label string + result *[]*SchemaChanges + changes []*Change +} + +func (c *schemaComparison) compareCompositions(tasks []schemaCompositionTask, changes *[]*Change) { + active := tasks[:0] + for _, task := range tasks { + if len(task.left) > 0 || len(task.right) > 0 { + active = append(active, task) + } + } + c.branch(len(active), func(i int, branch *schemaComparison) { + task := &active[i] + *task.result = nil + task.changes = nil + branch.extractSchemaChanges(task.left, task.right, task.label, task.result, &task.changes) + }) + for _, task := range active { + *changes = append(*changes, task.changes...) + } +} + func schemaScoreIdentity(proxy *base.SchemaProxy) schemaScoreProxy { if proxy == nil { return schemaScoreProxy{} @@ -49,8 +150,13 @@ func schemaScoreIdentity(proxy *base.SchemaProxy) schemaScoreProxy { func (c *schemaComparison) score(left, right *base.SchemaProxy) schemaScore { pair := schemaScorePair{schemaScoreIdentity(left), schemaScoreIdentity(right)} - if score, exists := c.scores[pair]; exists { - return score + if c.scores != nil { + c.scores.RLock() + score, exists := c.scores.values[pair] + c.scores.RUnlock() + if exists { + return score + } } if c.scoring == nil { c.scoring = &schemaScoring{active: make(map[schemaScorePair]struct{})} @@ -63,17 +169,22 @@ func (c *schemaComparison) score(left, right *base.SchemaProxy) schemaScore { defer delete(c.scoring.active, pair) cycles := c.scoring.cycles if c.scores == nil { - c.scores = make(map[schemaScorePair]schemaScore) + c.scores = new(schemaScores) } // Each candidate must see a complete comparison, independent of earlier // candidates and report placement. Keep only its counts, not its report nodes. - candidate := schemaComparison{scores: c.scores, scoring: c.scoring} + candidate := schemaComparison{scores: c.scores, scoring: c.scoring, serial: true} var score schemaScore if changes := candidate.compareSchemas(left, right); changes != nil { score = schemaScore{changes.TotalChanges(), changes.TotalBreakingChanges()} } if c.scoring.cycles == cycles { - c.scores[pair] = score + c.scores.Lock() + if c.scores.values == nil { + c.scores.values = make(map[schemaScorePair]schemaScore) + } + c.scores.values[pair] = score + c.scores.Unlock() } return score } @@ -83,6 +194,12 @@ func (c *schemaComparison) visit(left, right *base.SchemaProxy) bool { if _, exists := c.visited[pair]; exists { return false } + if _, exists := c.inherited[pair]; exists { + return false + } + if _, exists := c.previous[pair]; exists { + return false + } if c.visited == nil { c.visited = make(map[schemaComparisonPair]struct{}) } diff --git a/what-changed/model/schema_comparison_test.go b/what-changed/model/schema_comparison_test.go index 7953cea6..0af3f7cd 100644 --- a/what-changed/model/schema_comparison_test.go +++ b/what-changed/model/schema_comparison_test.go @@ -3,8 +3,10 @@ package model import ( "encoding/json" "fmt" + "runtime" "strings" "sync" + "sync/atomic" "testing" "github.com/pb33f/libopenapi/datamodel/low/base" @@ -15,7 +17,7 @@ func TestCompareSchemas_RecursiveScoresWithoutCircularMetadata(t *testing.T) { for _, union := range []string{"anyOf", "oneOf"} { t.Run(union, func(t *testing.T) { makeSpec := func(name, kind string) string { - return fmt.Sprintf(`openapi: 3.1.1 + spec := fmt.Sprintf(`openapi: 3.1.1 info: {title: Recursive scores, version: '1'} paths: {} components: @@ -39,6 +41,7 @@ components: - {$ref: '#/components/schemas/example/properties/nextA'} - {type: %s} `, name, name, union, name[len(name)-1:], kind, union, kind, union, kind) + return strings.Replace(spec, " p:", fmt.Sprintf(" q: {$ref: '#/components/schemas/%s'}\n p:", name), 1) } left, right := test_BuildDoc(makeSpec("exampleA", "string"), makeSpec("exampleB", "integer")) l := left.Components.Value.FindSchema("Holder").Value @@ -49,20 +52,23 @@ components: idx.SetCircularReferences(nil) idx.GetRolodex().SetSafeCircularReferences(nil) } + serial, err := json.Marshal((&schemaComparison{serial: true}).compareSchemas(l, r)) + require.NoError(t, err) var previous []byte for range 3 { comparison := new(schemaComparison) changes := comparison.compareSchemas(l, r) require.NotNil(t, changes) require.Greater(t, changes.TotalChanges(), 0) + comparison.score(l, r) require.NotNil(t, comparison.scoring) require.Greater(t, comparison.scoring.cycles, uint64(0)) require.Empty(t, comparison.scoring.active) - comparison.score(l, r) - _, cached := comparison.scores[schemaScorePair{schemaScoreIdentity(l), schemaScoreIdentity(r)}] + _, cached := comparison.scores.values[schemaScorePair{schemaScoreIdentity(l), schemaScoreIdentity(r)}] require.False(t, cached, "a score cut short by recursion must not enter the completed-score cache") encoded, err := json.Marshal(changes) require.NoError(t, err) + require.Equal(t, string(serial), string(encoded)) if previous != nil { require.Equal(t, string(previous), string(encoded)) } @@ -103,7 +109,11 @@ func TestCompareSchemas_NestedUnionScoresAreShared(t *testing.T) { require.Equal(t, 2*depth+2, changes.TotalChanges()) // Four candidates at each level share one complete score. Nested // candidates reuse the same cache instead of starting four more trees. - require.Len(t, comparison.scores, depth) + if depth == 0 { + require.Nil(t, comparison.scores) + } else { + require.Len(t, comparison.scores.values, depth) + } require.Equal(t, changes.TotalChanges(), CompareSchemas(left, right).TotalChanges()) if depth == 12 { var wg sync.WaitGroup @@ -150,7 +160,7 @@ components: require.Equal(t, expected, comparison.score(pair[0], pair[1])) require.Equal(t, expected, comparison.score(pair[0], pair[1])) } - require.Len(t, comparison.scores, 4) + require.Len(t, comparison.scores.values, 4) } func TestCompareSchemas_CircularUnionCandidatesStopAtReferences(t *testing.T) { @@ -184,8 +194,8 @@ components: comparison := new(schemaComparison) changes := comparison.compareSchemas(left, right) require.Equal(t, 2, changes.TotalChanges()) - require.Len(t, comparison.scores, 1) - for _, score := range comparison.scores { + require.Len(t, comparison.scores.values, 1) + for _, score := range comparison.scores.values { require.Equal(t, 1, score.total) } }) @@ -293,3 +303,69 @@ components: require.Equal(t, 3, changes.SchemaPropertyChanges["second"].TotalChanges()) } } + +func TestCompareSchemas_ParallelPlacementMatchesSerial(t *testing.T) { + for _, composition := range []bool{false, true} { + t.Run(fmt.Sprintf("composition=%t", composition), func(t *testing.T) { + makeSpec := func(prefix, kind string) string { + var spec strings.Builder + spec.WriteString("openapi: 3.1.0\ninfo: {title: Parallel comparison, version: '1'}\npaths: {}\ncomponents:\n schemas:\n Root:\n") + if composition { + fmt.Fprintf(&spec, " oneOf: [{$ref: '#/components/schemas/%s'}]\n anyOf: [{$ref: '#/components/schemas/%s'}]\n", prefix, prefix) + } else { + spec.WriteString(" properties:\n") + for i := range 8 { + fmt.Fprintf(&spec, " p%d: {$ref: '#/components/schemas/%s'}\n", i, prefix) + } + } + fmt.Fprintf(&spec, " %s:\n type: object\n properties:\n value: {type: %s}\n", prefix, kind) + return spec.String() + } + l, r := test_BuildDoc(makeSpec("Before", "string"), makeSpec("After", "integer")) + left, right := l.Components.Value.FindSchema("Root").Value, r.Components.Value.FindSchema("Root").Value + serial := (&schemaComparison{serial: true}).compareSchemas(left, right) + require.NotNil(t, serial) + expected, err := json.Marshal(serial) + require.NoError(t, err) + for range 20 { + parallel := new(schemaComparison).compareSchemas(left, right) + actual, err := json.Marshal(parallel) + require.NoError(t, err) + require.Equal(t, string(expected), string(actual)) + if composition { + require.Greater(t, parallel.OneOfChanges[0].TotalChanges(), 1) + require.Equal(t, 1, parallel.AnyOfChanges[0].TotalChanges()) + } else { + require.Greater(t, parallel.SchemaPropertyChanges["p0"].TotalChanges(), 1) + require.Equal(t, 1, parallel.SchemaPropertyChanges["p7"].TotalChanges()) + } + } + }) + } +} + +func TestSchemaComparisonBranchBoundsSharedWork(t *testing.T) { + l, r := test_BuildDoc(diamondComparisonSpec("L", 12), diamondComparisonSpec("R", 12)) + left, right := l.Components.Value.FindSchema("Root").Value, r.Components.Value.FindSchema("Root").Value + comparison := new(schemaComparison) + var visits atomic.Int32 + results := make([]bool, 40) + comparison.branch(len(results), func(i int, branch *schemaComparison) { + results[i] = branch.visit(left, right) + if results[i] { + visits.Add(1) + } + }) + require.LessOrEqual(t, visits.Load(), int32(min(runtime.GOMAXPROCS(0), 4))) + require.True(t, results[0]) + for _, result := range results[1:] { + require.False(t, result) + } + require.Len(t, comparison.visited, 1) + comparison.branch(len(results), func(i int, branch *schemaComparison) { + results[i] = branch.visit(left, right) + }) + for _, result := range results { + require.False(t, result, "forks inherit already reported pairs") + } +} From 90e6e074724a937e939c1ad4501bced3619264d0 Mon Sep 17 00:00:00 2001 From: quobix Date: Sat, 31 Oct 2026 17:03:56 -0400 Subject: [PATCH 28/33] test: cover alias policy and external example boundaries --- index/extract_refs_alias_limit_test.go | 15 +++++++++++++++ index/resolver_test.go | 25 +++++++++++++++++++++++++ utils/merge_view_test.go | 9 +++++++++ 3 files changed, 49 insertions(+) diff --git a/index/extract_refs_alias_limit_test.go b/index/extract_refs_alias_limit_test.go index d64cc2dc..84a2cd07 100644 --- a/index/extract_refs_alias_limit_test.go +++ b/index/extract_refs_alias_limit_test.go @@ -129,3 +129,18 @@ components: require.Contains(t, paths, "components/schemas/MergedFirst/properties/value") require.Contains(t, paths, "components/schemas/MergedSecond/properties/value") } + +func TestExtractRefsTrustedAliasesStillStopCycles(t *testing.T) { + config := CreateClosedAPIIndexConfig() + config.AllowExcessiveAliases = true + idx := NewSpecIndexWithConfig(nil, config) + var root yaml.Node + require.NoError(t, yaml.Unmarshal([]byte(strings.ReplaceAll(excessiveAliasDocument, "./unvisited.yaml", "#/components/schemas/Target")), &root)) + require.NotEmpty(t, idx.ExtractRefs(context.Background(), &root, nil, nil, 0, false, "")) + require.Empty(t, idx.GetReferenceIndexErrors()) + require.True(t, idx.GetConfig().ToDocumentConfiguration().AllowExcessiveAliases) + var cycle yaml.Node + require.NoError(t, yaml.Unmarshal([]byte("x: &x {b: {<<: *x}}"), &cycle)) + require.Empty(t, idx.ExtractRefs(context.Background(), &cycle, nil, nil, 0, false, "")) + require.Empty(t, idx.GetReferenceIndexErrors()) +} diff --git a/index/resolver_test.go b/index/resolver_test.go index 7f55b8df..b3f4e171 100644 --- a/index/resolver_test.go +++ b/index/resolver_test.go @@ -4,12 +4,14 @@ import ( "bytes" "errors" "fmt" + "github.com/pb33f/testify/require" "log/slog" "net/http" "net/http/httptest" "net/url" "os" "path/filepath" + "strings" "testing" "github.com/pb33f/jsonpath/pkg/jsonpath" @@ -2042,3 +2044,26 @@ func TestResolver_ResolveRelativeReference(t *testing.T) { assert.True(t, relative.Resolved) assert.Equal(t, foundNode.Content, relative.Node.Content) } + +func TestResolverNamedExternalExampleFragment(t *testing.T) { + for _, source := range []string{ + "example: {properties: {value: {$ref: '#/components/schemas/Target'}}}\ncomponents: {schemas: {Target: {type: string}}}", + "openapi: 3.1.1\nexample: {properties: {value: {$ref: '#/components/schemas/Target'}}}\ncomponents: {schemas: {Target: {type: string}}}", + } { + var root yaml.Node + require.NoError(t, yaml.Unmarshal([]byte(source), &root)) + idx := NewSpecIndex(&root) + _, node := utils.FindKeyNodeTop("example", root.Content[0].Content) + ref := &Reference{Definition: "#/example", FullDefinition: "#/example", Index: idx, Node: node} + resolver := NewResolver(idx) + found := resolver.extractRelatives(ref, node, nil, map[string]bool{}, nil, false, 0, "") + require.Empty(t, resolver.resolvingErrors) + require.NotNil(t, node) + if strings.HasPrefix(source, "openapi:") { + require.Empty(t, found, "a document example remains payload") + } else { + require.Len(t, found, 1, "the external fragment name does not turn its schema into payload") + require.Equal(t, "#/components/schemas/Target", found[0].Definition) + } + } +} diff --git a/utils/merge_view_test.go b/utils/merge_view_test.go index 01e57784..cbb366a6 100644 --- a/utils/merge_view_test.go +++ b/utils/merge_view_test.go @@ -116,3 +116,12 @@ merged: {<<: *repeated, local: chosen} require.NoError(t, err) require.Equal(t, before, after) } + +func TestResolveMergeNodesPreservesSequence(t *testing.T) { + var root yaml.Node + require.NoError(t, yaml.Unmarshal([]byte("[one, two]"), &root)) + node := root.Content[0] + require.Same(t, node, ResolveMergeNodes(node)) + require.Equal(t, "one", node.Content[0].Value) + require.Equal(t, "two", node.Content[1].Value) +} From 3a1fdd5544544012cb8fea26779dc190ef8408bf Mon Sep 17 00:00:00 2001 From: quobix Date: Sat, 31 Oct 2026 17:12:45 -0400 Subject: [PATCH 29/33] fix(index): recognize named components before example keywords --- index/extract_refs.go | 11 +++++++---- index/extract_refs_test.go | 24 ++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/index/extract_refs.go b/index/extract_refs.go index 7850c2d6..6046c6ed 100644 --- a/index/extract_refs.go +++ b/index/extract_refs.go @@ -57,7 +57,7 @@ func isOpenAPIExampleKeywordSegment(seenPath []string, idx int) bool { if seenPath[idx] != "example" && seenPath[idx] != "examples" { return false } - // A name in a schema map cannot itself introduce another schema map. + // A component or schema name cannot itself introduce another named map. // For example, properties/definitions/example ends at a payload keyword. nameSlot := false for i, segment := range seenPath[:idx] { @@ -65,11 +65,14 @@ func isOpenAPIExampleKeywordSegment(seenPath []string, idx int) bool { nameSlot = false continue } + if i == 1 && seenPath[0] == "components" { + nameSlot = true + continue + } switch segment { - case "properties", "patternProperties", "definitions", "$defs", "dependentSchemas": + case "properties", "patternProperties", "definitions", "$defs", "dependentSchemas", + "headers", "links", "callbacks", "encoding", "webhooks": nameSlot = true - case "schemas": - nameSlot = i == 1 && seenPath[0] == "components" } } return !nameSlot diff --git a/index/extract_refs_test.go b/index/extract_refs_test.go index 71b481d3..b758400a 100644 --- a/index/extract_refs_test.go +++ b/index/extract_refs_test.go @@ -1246,3 +1246,27 @@ func TestExtractRefs_ExampleUnderSchemaKeywordName(t *testing.T) { }) } } + +func TestExtractRefs_ExampleUnderComponentKeywordName(t *testing.T) { + for _, container := range []string{"parameters", "headers"} { + for _, name := range []string{"definitions", "$defs", "dependentSchemas", "properties", "patternProperties"} { + source := fmt.Sprintf(`openapi: 3.1.1 +info: {title: Component examples, version: '1'} +paths: {} +components: + %s: + %s: + name: test + in: query + schema: {type: object} + example: + nested: {$ref: '#/payload-only'} +`, container, name) + var root yaml.Node + require.NoError(t, yaml.Unmarshal([]byte(source), &root)) + idx := NewSpecIndex(&root) + require.Empty(t, idx.GetRawReferencesSequenced(), "%s/%s example is payload", container, name) + require.Empty(t, idx.GetReferenceIndexErrors()) + } + } +} From 1fed1e088b699458b24d88c65f547861218edb05 Mon Sep 17 00:00:00 2001 From: quobix Date: Sat, 31 Oct 2026 17:26:20 -0400 Subject: [PATCH 30/33] perf(index): keep transient resolver paths on the stack --- index/resolver_polymorphic.go | 3 ++- index/resolver_relatives.go | 36 ++++++++++++++++++++++------------- index/resolver_test.go | 22 +++++++++++++++++++++ index/resolver_walk.go | 1 - 4 files changed, 47 insertions(+), 15 deletions(-) diff --git a/index/resolver_polymorphic.go b/index/resolver_polymorphic.go index e7e3dd3f..1ccf4f5f 100644 --- a/index/resolver_polymorphic.go +++ b/index/resolver_polymorphic.go @@ -13,6 +13,7 @@ func (resolver *Resolver) extractPolymorphicRelatives( node, keywordNode *yaml.Node, state relativeWalkState, index int, + walkPath []string, ) []*Reference { var found []*Reference @@ -40,7 +41,7 @@ func (resolver *Resolver) extractPolymorphicRelatives( if d, _, l := utils.IsNodeRefValue(v); d { resolver.visitPolymorphicReference(ref, keywordNode.Value, node.Content[index], l, state, index) } else { - found = append(found, resolver.extractRelativesWithState(ref, v, keywordNode, state.descend())...) + found = append(found, resolver.extractRelativesWithState(ref, v, keywordNode, state.descend(), walkPath)...) } } } diff --git a/index/resolver_relatives.go b/index/resolver_relatives.go index eef42224..de75550c 100644 --- a/index/resolver_relatives.go +++ b/index/resolver_relatives.go @@ -24,12 +24,19 @@ func (resolver *Resolver) extractRelatives( schemaIDBase string, ) []*Reference { state := newRelativeWalkState(foundRelatives, journey, resolve, depth, schemaIDBase) + // Keep ordinary paths on this walk's stack. Child walks reuse capacity; + // unusually deep paths grow normally without changing the traversal limit. + var pathBuffer [32]string + walkPath := pathBuffer[:0] if ref != nil { - state.path = ref.SourcePath if strings.HasPrefix(ref.Definition, "#/") { - state.path = strings.Split(strings.TrimPrefix(ref.Definition, "#/"), "/") + for segment := range strings.SplitSeq(strings.TrimPrefix(ref.Definition, "#/"), "/") { + walkPath = append(walkPath, segment) + } + } else { + walkPath = append(walkPath, ref.SourcePath...) } - if len(state.path) > 0 && (state.path[0] == "example" || state.path[0] == "examples") && ref.Index != nil { + if len(walkPath) > 0 && (walkPath[0] == "example" || walkPath[0] == "examples") && ref.Index != nil { root := ref.Index.GetRootNode() if root != nil { if root.Kind == yaml.DocumentNode && len(root.Content) > 0 { @@ -41,23 +48,24 @@ func (resolver *Resolver) extractRelatives( // An external fragment identifies the object being resolved. Its // top-level name is not a keyword in the referencing document. // Example fields encountered inside that object still mark payloads. - state.path = ref.SourcePath + walkPath = append(pathBuffer[:0], ref.SourcePath...) } } } } - return resolver.extractRelativesWithState(ref, node, parent, state) + return resolver.extractRelativesWithState(ref, node, parent, state, walkPath) } func (resolver *Resolver) extractRelativesWithState( ref *Reference, node, parent *yaml.Node, state relativeWalkState, + walkPath []string, ) []*Reference { - if underOpenAPIExamplePayloadPath(state.path) { + if underOpenAPIExamplePayloadPath(walkPath) { return nil } - if isDirectOpenAPIExampleValuePath(state.path) && !isDirectOpenAPIExampleRefNode(node) { + if isDirectOpenAPIExampleValuePath(walkPath) && !isDirectOpenAPIExampleRefNode(node) { return nil } // Guard against stack overflow from deeply nested or circular specs. @@ -82,12 +90,13 @@ func (resolver *Resolver) extractRelativesWithState( if utils.IsNodeMap(n) || utils.IsNodeArray(n) { childState := state + childPath := walkPath if node.Kind == yaml.MappingNode && i%2 == 1 { - childState.path = append(state.path, node.Content[i-1].Value) + childPath = append(walkPath, node.Content[i-1].Value) } else if node.Kind == yaml.SequenceNode { - childState.path = append(state.path, "[]") + childPath = append(walkPath, "[]") } - found = append(found, resolver.extractNestedRelatives(ref, node, n, childState)...) + found = append(found, resolver.extractNestedRelatives(ref, node, n, childState, childPath)...) } if i%2 == 0 && n.Value == "$ref" && len(node.Content) > i%2+1 { @@ -101,7 +110,7 @@ func (resolver *Resolver) extractRelativesWithState( } if i%2 == 0 && shouldExtractPolymorphicRelatives(parent, n) { - found = append(found, resolver.extractPolymorphicRelatives(ref, node, n, state, i)...) + found = append(found, resolver.extractPolymorphicRelatives(ref, node, n, state, i, walkPath)...) skip = true } } @@ -141,6 +150,7 @@ func (resolver *Resolver) extractNestedRelatives( ref *Reference, parent, node *yaml.Node, state relativeWalkState, + walkPath []string, ) []*Reference { childState := state.descend() foundRef, _, _ := resolver.searchReferenceWithContext(ref, ref) @@ -148,9 +158,9 @@ func (resolver *Resolver) extractNestedRelatives( if foundRef.Circular { return nil } - return resolver.extractRelativesWithState(foundRef, node, parent, childState) + return resolver.extractRelativesWithState(foundRef, node, parent, childState, walkPath) } - return resolver.extractRelativesWithState(ref, node, parent, childState) + return resolver.extractRelativesWithState(ref, node, parent, childState, walkPath) } func (resolver *Resolver) extractRelativeReference( diff --git a/index/resolver_test.go b/index/resolver_test.go index b3f4e171..04a41a93 100644 --- a/index/resolver_test.go +++ b/index/resolver_test.go @@ -2067,3 +2067,25 @@ func TestResolverNamedExternalExampleFragment(t *testing.T) { } } } + +func TestResolverDeepExamplePaths(t *testing.T) { + var source strings.Builder + source.WriteString("openapi: 3.1.1\ncomponents:\n schemas:\n Target: {type: string}\n Root:\n") + indent := " " + for range 24 { + source.WriteString(indent + "properties:\n" + indent + " item:\n") + indent += " " + } + source.WriteString(indent + "example: {nested: {$ref: '#/payload-only'}}\n") + source.WriteString(indent + "properties: {child: {$ref: '#/components/schemas/Target'}}\n") + var root yaml.Node + require.NoError(t, yaml.Unmarshal([]byte(source.String()), &root)) + idx := NewSpecIndex(&root) + ref, _ := idx.SearchIndexForReference("#/components/schemas/Root") + require.NotNil(t, ref) + resolver := NewResolver(idx) + found := resolver.extractRelatives(ref, ref.Node, nil, map[string]bool{}, nil, false, 0, "") + require.Empty(t, resolver.resolvingErrors) + require.Len(t, found, 1) + require.Equal(t, "#/components/schemas/Target", found[0].Definition) +} diff --git a/index/resolver_walk.go b/index/resolver_walk.go index a2db964d..a573ec57 100644 --- a/index/resolver_walk.go +++ b/index/resolver_walk.go @@ -11,7 +11,6 @@ type relativeWalkState struct { resolve bool depth int schemaIDBase string - path []string } func newRelativeWalkState( From 990ddea74bbfa850602fbadf703fbedc0848fe46 Mon Sep 17 00:00:00 2001 From: quobix Date: Sat, 31 Oct 2026 17:43:51 -0400 Subject: [PATCH 31/33] perf(index): combine node scans and keep traversal state on the stack --- index/extract_refs.go | 71 +++++++++++++++++++++----- index/extract_refs_alias_limit_test.go | 27 ++++++++++ index/extract_refs_test.go | 2 +- index/extract_refs_walk.go | 37 ++++++-------- 4 files changed, 104 insertions(+), 33 deletions(-) diff --git a/index/extract_refs.go b/index/extract_refs.go index 6046c6ed..984bff0b 100644 --- a/index/extract_refs.go +++ b/index/extract_refs.go @@ -116,19 +116,23 @@ func isDirectOpenAPIExampleValuePath(seenPath []string) bool { return false } +// Limit plugins are immutable after construction and shared by extraction walks. +var defaultExtractRefsLimits limit.Plugin +var unlimitedExtractRefsLimits = limit.New(limit.AliasNone()) + // ExtractRefs will return a deduplicated slice of references for every unique ref found in the document. // The total number of refs, will generally be much higher, you can extract those from GetRawReferenceCount() // Excessive YAML alias expansion stops extraction and is reported by GetReferenceIndexErrors. func (index *SpecIndex) ExtractRefs(ctx context.Context, node, parent *yaml.Node, seenPath []string, level int, poly bool, pName string) []*Reference { walk := &extractRefsTraversal{} if index.config != nil && index.config.AllowExcessiveAliases { - walk.limits = *limit.New(limit.AliasNone()) + walk.limits = unlimitedExtractRefsLimits } return index.extractRefs(ctx, node, parent, seenPath, level, poly, pName, walk) } type extractRefsTraversal struct { - limits limit.Plugin + limits *limit.Plugin ancestry map[*yaml.Node]struct{} nodes int aliasNodes int @@ -142,7 +146,11 @@ func (walk *extractRefsTraversal) visit() error { return nil } walk.aliasNodes++ - return walk.limits.CheckAlias(walk.aliasNodes, walk.nodes) + limits := walk.limits + if limits == nil { + limits = &defaultExtractRefsLimits + } + return limits.CheckAlias(walk.aliasNodes, walk.nodes) } // Ancestry tracks canonical source nodes, not the shallow views created during @@ -152,10 +160,9 @@ func (index *SpecIndex) extractRefs(ctx context.Context, node, parent *yaml.Node return nil } err := walk.visit() - aliased := node.Kind == yaml.AliasNode - if aliased && err == nil { + aliased := node.Kind == yaml.AliasNode && err == nil + if aliased { walk.aliasDepth++ - defer func() { walk.aliasDepth-- }() node = utils.NodeAlias(node) err = walk.visit() } @@ -166,24 +173,32 @@ func (index *SpecIndex) extractRefs(ctx context.Context, node, parent *yaml.Node ErrorRef: fmt.Errorf("%w; set AllowExcessiveAliases for trusted documents", err), Node: node, Path: "$." + strings.Join(seenPath, "."), }) + if aliased { + walk.aliasDepth-- + } return nil } if node == nil { + if aliased { + walk.aliasDepth-- + } return nil } - view := utils.ResolveMergeNodes(node) - if aliased || view != node { + view, nodeID := resolveExtractRefsNode(node) + tracked := aliased || view != node + if tracked { if _, active := walk.ancestry[node]; active { + if aliased { + walk.aliasDepth-- + } return nil } if walk.ancestry == nil { walk.ancestry = make(map[*yaml.Node]struct{}) } walk.ancestry[node] = struct{}{} - defer delete(walk.ancestry, node) } - state := index.initializeExtractRefsState(ctx, view, seenPath, level, poly, pName) - state.traversal = walk + state := index.initializeExtractRefsState(ctx, nodeID, seenPath, level, poly, pName) state.mergeStart = len(view.Content) if view != node { // Merge views retain explicit pairs first, then append inherited pairs. @@ -195,7 +210,39 @@ func (index *SpecIndex) extractRefs(ctx context.Context, node, parent *yaml.Node } } } - found := index.walkExtractRefs(view, parent, &state) + found := index.walkExtractRefs(view, parent, &state, walk) + if tracked { + delete(walk.ancestry, node) + } + if aliased { + walk.aliasDepth-- + } index.refCount = len(index.allRefs) return found } + +// resolveExtractRefsNode finds merge keys and the schema ID in one scan. Ordinary +// mappings retain their identity; merged mappings use the existing read-only view. +func resolveExtractRefsNode(node *yaml.Node) (*yaml.Node, string) { + if node.Kind != yaml.MappingNode && node.Tag != "!!map" { + return node, "" + } + var idNode *yaml.Node + for i := 0; i < len(node.Content); i += 2 { + key := node.Content[i] + if key == nil { + continue + } + if key.Tag == "!!merge" { + view := utils.ResolveMergeNodes(node) + return view, FindSchemaIdInNode(view) + } + if idNode == nil && node.Kind == yaml.MappingNode && key.Value == "$id" && i+1 < len(node.Content) && utils.IsNodeStringValue(node.Content[i+1]) { + idNode = node.Content[i+1] + } + } + if idNode != nil { + return node, idNode.Value + } + return node, "" +} diff --git a/index/extract_refs_alias_limit_test.go b/index/extract_refs_alias_limit_test.go index 84a2cd07..956fceee 100644 --- a/index/extract_refs_alias_limit_test.go +++ b/index/extract_refs_alias_limit_test.go @@ -144,3 +144,30 @@ func TestExtractRefsTrustedAliasesStillStopCycles(t *testing.T) { require.Empty(t, idx.ExtractRefs(context.Background(), &cycle, nil, nil, 0, false, "")) require.Empty(t, idx.GetReferenceIndexErrors()) } + +func TestExtractRefsMergeViewPreservesSchemaID(t *testing.T) { + var root yaml.Node + require.NoError(t, yaml.Unmarshal([]byte(`shared: &shared {$id: 'https://example.com/shared', type: object} +withID: {$id: 'https://example.com/local', <<: *shared} +inherited: {<<: *shared} +`), &root)) + mapping := root.Content[0] + view, id := resolveExtractRefsNode(mapping.Content[3]) + require.Equal(t, "https://example.com/local", id) + require.NotSame(t, mapping.Content[3], view) + _, inherited := resolveExtractRefsNode(mapping.Content[5]) + require.Equal(t, "https://example.com/shared", inherited) + _, documentID := resolveExtractRefsNode(&root) + require.Empty(t, documentID) +} + +func TestExtractRefsSchemaIDKeepsFirstEmptyValue(t *testing.T) { + mapping := &yaml.Node{Kind: yaml.MappingNode, Content: []*yaml.Node{ + nil, {Kind: yaml.ScalarNode, Value: "ignored"}, + {Kind: yaml.ScalarNode, Value: "$id"}, {Kind: yaml.ScalarNode, Tag: "!!str"}, + {Kind: yaml.ScalarNode, Value: "$id"}, {Kind: yaml.ScalarNode, Tag: "!!str", Value: "https://example.com/later"}, + }} + view, id := resolveExtractRefsNode(mapping) + require.Same(t, mapping, view) + require.Empty(t, id) +} diff --git a/index/extract_refs_test.go b/index/extract_refs_test.go index b758400a..271064bb 100644 --- a/index/extract_refs_test.go +++ b/index/extract_refs_test.go @@ -1088,7 +1088,7 @@ properties: func TestExtractRefs_WalkHelpers(t *testing.T) { idx := NewTestSpecIndex().Load().(*SpecIndex) - state := idx.initializeExtractRefsState(context.Background(), nil, nil, 0, false, "") + state := idx.initializeExtractRefsState(context.Background(), "", nil, 0, false, "") node := &yaml.Node{ Kind: yaml.MappingNode, diff --git a/index/extract_refs_walk.go b/index/extract_refs_walk.go index 0cb00eba..02fd53c9 100644 --- a/index/extract_refs_walk.go +++ b/index/extract_refs_walk.go @@ -12,7 +12,6 @@ import ( ) type extractRefsState struct { - traversal *extractRefsTraversal mergeStart int ctx context.Context scope *SchemaIdScope @@ -27,7 +26,7 @@ type extractRefsState struct { func (index *SpecIndex) initializeExtractRefsState( ctx context.Context, - node *yaml.Node, + nodeID string, seenPath []string, level int, poly bool, @@ -40,16 +39,14 @@ func (index *SpecIndex) initializeExtractRefsState( } parentBaseURI := scope.BaseUri - if node != nil && node.Kind == yaml.MappingNode && !underOpenAPIExamplePath(seenPath) { - if nodeID := FindSchemaIdInNode(node); nodeID != "" { - resolvedNodeID, _ := ResolveSchemaId(nodeID, parentBaseURI) - if resolvedNodeID == "" { - resolvedNodeID = nodeID - } - scope = scope.Copy() - scope.PushId(resolvedNodeID) - ctx = WithSchemaIdScope(ctx, scope) + if nodeID != "" && !underOpenAPIExamplePath(seenPath) { + resolvedNodeID, _ := ResolveSchemaId(nodeID, parentBaseURI) + if resolvedNodeID == "" { + resolvedNodeID = nodeID } + scope = scope.Copy() + scope.PushId(resolvedNodeID) + ctx = WithSchemaIdScope(ctx, scope) } return extractRefsState{ @@ -63,16 +60,13 @@ func (index *SpecIndex) initializeExtractRefsState( } } -func (index *SpecIndex) walkExtractRefs(node, parent *yaml.Node, state *extractRefsState) []*Reference { +func (index *SpecIndex) walkExtractRefs(node, parent *yaml.Node, state *extractRefsState, walk *extractRefsTraversal) []*Reference { if node == nil || len(node.Content) == 0 { return nil } var found []*Reference for i, n := range node.Content { - if state.traversal.stopped { - break - } // In YAML mapping nodes, Content alternates key-value: even indices (0, 2, 4...) // are keys, odd indices (1, 3, 5...) are values. if i%2 == 0 { @@ -84,11 +78,14 @@ func (index *SpecIndex) walkExtractRefs(node, parent *yaml.Node, state *extractR if utils.IsNodeMap(n) || utils.IsNodeArray(n) { inherited := i >= state.mergeStart if inherited { - state.traversal.aliasDepth++ + walk.aliasDepth++ } - found = append(found, index.walkChildExtractRefs(n, node, state)...) + found = append(found, index.walkChildExtractRefs(n, node, state, walk)...) if inherited { - state.traversal.aliasDepth-- + walk.aliasDepth-- + } + if walk.stopped { + break } } @@ -98,7 +95,7 @@ func (index *SpecIndex) walkExtractRefs(node, parent *yaml.Node, state *extractR return found } -func (index *SpecIndex) walkChildExtractRefs(node, parent *yaml.Node, state *extractRefsState) []*Reference { +func (index *SpecIndex) walkChildExtractRefs(node, parent *yaml.Node, state *extractRefsState, walk *extractRefsTraversal) []*Reference { if underOpenAPIExamplePayloadPath(state.seenPath) { return nil } @@ -112,7 +109,7 @@ func (index *SpecIndex) walkChildExtractRefs(node, parent *yaml.Node, state *ext state.polyName = state.prev } } - return index.extractRefs(state.ctx, node, parent, state.seenPath, state.level, state.poly, state.polyName, state.traversal) + return index.extractRefs(state.ctx, node, parent, state.seenPath, state.level, state.poly, state.polyName, walk) } func isDirectOpenAPIExampleRefNode(node *yaml.Node) bool { From c339b4bc3832ec5f9aa26daa2ec12b79bc75fe26 Mon Sep 17 00:00:00 2001 From: quobix Date: Sat, 31 Oct 2026 17:43:51 -0400 Subject: [PATCH 32/33] perf(utils): validate JSONPath names with an ASCII lookup --- utils/utils.go | 37 +++++++++++++++++++------------------ utils/utils_test.go | 3 +++ 2 files changed, 22 insertions(+), 18 deletions(-) diff --git a/utils/utils.go b/utils/utils.go index b210ce0d..8c83cd3c 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -943,36 +943,37 @@ var ( bracketNameExp = regexp.MustCompile(`^(\w+)\['?([\w/]+)'?]$`) ) +var jsonPathDotChars = func() [256]bool { + var allowed [256]bool + for _, c := range []byte("ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789_\\") { + allowed[c] = true + } + return allowed +}() + // isPathChar checks if a string is valid for JSONPath dot notation. // returns true only if the string contains only alphanumeric, underscore, or backslash characters // and does not start with a digit (unless it's a pure integer, which is handled separately). // jsonPath requires bracket notation for property names starting with digits like "403_permission_denied". // this is an optimized replacement for the pathCharExp regex. + func isPathChar(s string) bool { if len(s) == 0 { return false } - - firstChar := s[0] - startsWithDigit := firstChar >= '0' && firstChar <= '9' - allDigits := startsWithDigit - - // single pass: validate characters and track if all are digits - for _, r := range s { - if !((r >= 'A' && r <= 'Z') || (r >= 'a' && r <= 'z') || (r >= '0' && r <= '9') || r == '_' || r == '\\') { - return false - } - if allDigits && (r < '0' || r > '9') { - allDigits = false + if s[0] >= '0' && s[0] <= '9' { + for i := 1; i < len(s); i++ { + if s[i] < '0' || s[i] > '9' { + return false + } } + return true } - - // if starts with digit but not pure integer, requires bracket notation - // property names like "403_permission_denied" must use bracket notation - if startsWithDigit && !allDigits { - return false + for i := range len(s) { + if !jsonPathDotChars[s[i]] { + return false + } } - return true } diff --git a/utils/utils_test.go b/utils/utils_test.go index fbaf8378..efb14ca7 100644 --- a/utils/utils_test.go +++ b/utils/utils_test.go @@ -2164,6 +2164,9 @@ func TestConvertComponentIdIntoFriendlyPathSearch_DigitStartingSegments(t *testi // Test isPathChar function directly for comprehensive coverage func TestIsPathChar(t *testing.T) { + for _, name := range []string{"naïve", "東京", string([]byte{'a', 0xff})} { + assert.False(t, isPathChar(name), "non-ASCII names require bracket notation") + } // Valid path characters (letters, numbers not at start, underscore, backslash) assert.True(t, isPathChar("validName")) assert.True(t, isPathChar("Valid123")) From 9d96a62c0c0524313c7866de8ae1435c28ba6f2e Mon Sep 17 00:00:00 2001 From: quobix Date: Sat, 31 Oct 2026 19:04:28 -0400 Subject: [PATCH 33/33] fix(index): isolate cached files across document owners --- index/rolodex.go | 68 +++++-- index/rolodex_file_loader.go | 6 +- index/rolodex_owned_file.go | 68 +++++++ index/rolodex_owned_file_test.go | 317 +++++++++++++++++++++++++++++++ index/rolodex_test.go | 3 + 5 files changed, 444 insertions(+), 18 deletions(-) create mode 100644 index/rolodex_owned_file.go create mode 100644 index/rolodex_owned_file_test.go diff --git a/index/rolodex.go b/index/rolodex.go index 68dbb8f4..46249e01 100644 --- a/index/rolodex.go +++ b/index/rolodex.go @@ -75,6 +75,7 @@ type Rolodex struct { indexingDuration time.Duration indexes []*SpecIndex indexMap map[string]*SpecIndex + ownedFiles map[string]*ownedRolodexFile // private copies of files indexed by another rolodex indexLock sync.Mutex rootIndex *SpecIndex rootNode *yaml.Node @@ -103,6 +104,7 @@ func (r *Rolodex) Release() { r.remoteFS = nil r.indexes = nil r.indexMap = nil + r.ownedFiles = nil r.rootIndex = nil r.indexLock.Unlock() @@ -359,21 +361,41 @@ func (r *Rolodex) IndexTheRolodex(ctx context.Context) error { copiedConfig.Rolodex = r copiedConfig.SpecAbsolutePath = fullPath copiedConfig.AvoidBuildIndex = true // we will build out everything in two steps. - idx, err := idxFile.Index(&copiedConfig) - - if err == nil { // Index() does not throw an error anymore. - // for each index, we need a resolver - resolver := NewResolver(idx) - - // check if the config has been set to ignore circular references in arrays and polymorphic schemas - if copiedConfig.IgnoreArrayCircularReferences { - resolver.IgnoreArrayCircularReferences() + file := idxFile.(RolodexFile) + idx := file.GetIndex() + var err error + if idx == nil || idx.rolodex == r { + idx, err = idxFile.Index(&copiedConfig) + } + if err != nil || idx == nil { + if err == nil { + err = fmt.Errorf("indexing '%s' returned no index", fullPath) } - if copiedConfig.IgnorePolymorphicCircularReferences { - resolver.IgnorePolymorphicCircularReferences() + errChan <- err + return + } + if idx.rolodex != r { + owned, ownErr := r.ownIndexedFile(file) + if ownErr != nil { + errChan <- ownErr + return } - indexChan <- idx + // Only the batch worker waits. Recursive opens must remain non-blocking. + owned.WaitForIndexing() + idx = owned.GetIndex() + } + + resolver := idx.GetResolver() + if resolver == nil { + resolver = NewResolver(idx) + } + if copiedConfig.IgnoreArrayCircularReferences { + resolver.IgnoreArrayCircularReferences() + } + if copiedConfig.IgnorePolymorphicCircularReferences { + resolver.IgnorePolymorphicCircularReferences() } + indexChan <- idx } if lfs, ok := fs.(RolodexFS); ok { @@ -417,7 +439,6 @@ func (r *Rolodex) IndexTheRolodex(ctx context.Context) error { case <-doneChan: indexingCompleted++ case err := <-errChan: - indexingCompleted++ caughtErrors = append(caughtErrors, err) case idx := <-indexChan: indexBuildQueue = append(indexBuildQueue, idx) @@ -432,9 +453,11 @@ func (r *Rolodex) IndexTheRolodex(ctx context.Context) error { ) r.indexLock.Lock() - r.indexes = indexBuildQueue for _, idx := range indexBuildQueue { - r.indexMap[idx.specAbsolutePath] = idx + if r.indexMap[idx.specAbsolutePath] == nil { + r.indexes = append(r.indexes, idx) + r.indexMap[idx.specAbsolutePath] = idx + } } r.indexLock.Unlock() @@ -783,6 +806,18 @@ func (r *Rolodex) localPathForOpen(baseDir, fileLookup string, fileSystem fs.FS) func (r *Rolodex) asLocalFile(file fs.File, fileLookup string) (*LocalFile, []error) { var errorStack []error + if localFile, ok := file.(*LocalFile); ok && localFile == nil { + return nil, nil + } + if indexed, ok := file.(RolodexFile); ok { + if idx := indexed.GetIndex(); idx != nil && idx.rolodex != r { + owned, err := r.ownIndexedFile(indexed) + if err != nil { + return nil, []error{err} + } + return owned, nil + } + } if localFile, ok := file.(*LocalFile); ok { return localFile, nil } @@ -910,6 +945,9 @@ func (r *Rolodex) wrapLocalRolodexFile(localFile *LocalFile) (RolodexFile, error } func (r *Rolodex) wrapRemoteRolodexFile(remoteFile *RemoteFile) (RolodexFile, error) { + if idx := remoteFile.GetIndex(); idx != nil && idx.rolodex != r { + return r.ownIndexedFile(remoteFile) + } return &rolodexFile{ rolodex: r, location: remoteFile.fullPath, diff --git a/index/rolodex_file_loader.go b/index/rolodex_file_loader.go index 29a528f2..63e5cb1b 100644 --- a/index/rolodex_file_loader.go +++ b/index/rolodex_file_loader.go @@ -153,15 +153,15 @@ func (l *LocalFS) OpenWithContext(ctx context.Context, name string) (fs.File, er copiedCfg.SpecAbsolutePath = name copiedCfg.AvoidBuildIndex = true copiedCfg.SpecInfo = nil + if l.rolodex != nil { + copiedCfg.Rolodex = l.rolodex + } // Add this file to the context's indexing set to prevent deadlocks // when circular references cause the same file to be looked up recursively. indexingCtx := AddIndexingFile(ctx, name) idx, _ := extractedFile.IndexWithContext(indexingCtx, &copiedCfg) - if idx != nil && l.rolodex != nil { - idx.rolodex = l.rolodex - } if idx != nil { NewResolver(idx) diff --git a/index/rolodex_owned_file.go b/index/rolodex_owned_file.go new file mode 100644 index 00000000..247e8d7c --- /dev/null +++ b/index/rolodex_owned_file.go @@ -0,0 +1,68 @@ +package index + +import ( + "context" + + "github.com/pb33f/libopenapi/datamodel" +) + +type ownedRolodexFile struct { + file *LocalFile + err error + parsed chan struct{} +} + +// ownIndexedFile isolates a cached file from another document's mutable index. +// A pending file exposes its parsed nodes without waiting for indexing, so +// mutually referencing files cannot deadlock during recursive lookups. +func (r *Rolodex) ownIndexedFile(source RolodexFile) (*LocalFile, error) { + location := source.GetFullPath() + r.indexLock.Lock() + owned := r.ownedFiles[location] + if owned != nil { + r.indexLock.Unlock() + <-owned.parsed + return owned.file, owned.err + } + owned = &ownedRolodexFile{parsed: make(chan struct{})} + if r.ownedFiles == nil { + r.ownedFiles = make(map[string]*ownedRolodexFile) + } + r.ownedFiles[location] = owned + r.indexLock.Unlock() + + data := []byte(source.GetContent()) + info, err := datamodel.ExtractSpecInfoWithDocumentCheckSync(data, true) + if err != nil { + owned.err = err + close(owned.parsed) + return nil, err + } + file := &LocalFile{ + filename: source.Name(), name: source.Name(), fullPath: location, + extension: source.GetFileExtension(), lastModified: source.ModTime(), + data: data, parsed: info.RootNode, indexingComplete: make(chan struct{}), + } + owned.file = file + // Parsing cannot follow references. Release its waiters before indexing, + // where cross-file lookups must be able to return the pending tree. + close(owned.parsed) + + file.indexOnce.Do(func() { + cfg := *r.indexConfig + cfg.Rolodex = r + cfg.SpecAbsolutePath = location + cfg.SpecInfo = info + cfg.AvoidBuildIndex = true + ctx := AddIndexingFile(context.Background(), location) + idx := NewSpecIndexWithConfigAndContext(ctx, info.RootNode, &cfg) + resolver := NewResolver(idx) + resolver.IgnoreArray = cfg.IgnoreArrayCircularReferences + resolver.IgnorePoly = cfg.IgnorePolymorphicCircularReferences + idx.BuildIndex() + file.index.Store(idx) + r.AddIndex(idx) + }) + file.signalIndexingComplete() + return file, nil +} diff --git a/index/rolodex_owned_file_test.go b/index/rolodex_owned_file_test.go new file mode 100644 index 00000000..b66a4b82 --- /dev/null +++ b/index/rolodex_owned_file_test.go @@ -0,0 +1,317 @@ +package index + +import ( + "context" + "errors" + "io/fs" + "os" + "path/filepath" + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/pb33f/go-yaml" + "github.com/pb33f/testify/require" +) + +type sharedIndexedFS struct { + files map[string]RolodexFile + lazy bool +} + +func TestRolodexMissingLocalFile(t *testing.T) { + // LocalFS can return a typed nil for an unavailable discriminator target. + var missing *LocalFile + rolo := NewRolodex(CreateOpenAPIIndexConfig()) + file, errs := rolo.asLocalFile(missing, "missing.yaml") + require.Nil(t, file) + require.Empty(t, errs) +} + +type controlledIndexedFile struct { + *LocalFile + result *SpecIndex + err error + calls int +} + +func (f *controlledIndexedFile) Index(*SpecIndexConfig) (*SpecIndex, error) { + f.calls++ + if f.result != nil { + f.index.Store(f.result) + } + return f.result, f.err +} + +func standaloneIndexedFile(t *testing.T, content string) *LocalFile { + t.Helper() + var root yaml.Node + require.NoError(t, yaml.Unmarshal([]byte("type: object\n"), &root)) + file := &LocalFile{data: []byte(content), name: "schema.yaml", filename: "schema.yaml", fullPath: filepath.Join(t.TempDir(), "schema.yaml"), extension: YAML} + file.index.Store(NewSpecIndex(&root)) + return file +} + +func TestRolodexOwnsStandaloneAndLateIndexes(t *testing.T) { + for _, late := range []bool{false, true} { + t.Run(map[bool]string{false: "cached", true: "returned-by-index"}[late], func(t *testing.T) { + original := standaloneIndexedFile(t, "type: object\nproperties:\n id: {type: string}\n") + file := &controlledIndexedFile{LocalFile: original, result: original.GetIndex()} + if late { + file.LocalFile = &LocalFile{data: original.data, name: original.name, filename: original.filename, fullPath: original.fullPath, extension: YAML} + } + files := &sharedIndexedFS{files: map[string]RolodexFile{"schema.yaml": file}} + cfg := CreateOpenAPIIndexConfig() + cfg.BasePath = filepath.Dir(original.fullPath) + rolo := NewRolodex(cfg) + rolo.AddLocalFS(cfg.BasePath, files) + require.NoError(t, rolo.IndexTheRolodex(context.Background())) + owned, err := rolo.Open(original.fullPath) + require.NoError(t, err) + require.Same(t, rolo, owned.GetIndex().GetRolodex()) + require.NotSame(t, original.GetIndex(), owned.GetIndex()) + if late { + require.Equal(t, 1, file.calls) + } else { + require.Zero(t, file.calls, "do not ask a cached foreign file to index under another owner") + } + }) + } +} + +func TestRolodexReportsIndexAndOwnedParseErrors(t *testing.T) { + for _, failure := range []string{"index-error", "nil-index", "empty-content"} { + t.Run(failure, func(t *testing.T) { + file := &controlledIndexedFile{LocalFile: &LocalFile{name: "schema.yaml", fullPath: filepath.Join(t.TempDir(), "schema.yaml"), data: []byte("type: string\n")}} + switch failure { + case "index-error": + file.err = errors.New("custom index failed") + case "empty-content": + file.LocalFile = standaloneIndexedFile(t, "") + } + cfg := CreateOpenAPIIndexConfig() + cfg.BasePath = filepath.Dir(file.fullPath) + rolo := NewRolodex(cfg) + rolo.AddLocalFS(cfg.BasePath, &sharedIndexedFS{files: map[string]RolodexFile{"schema.yaml": file}}) + require.Error(t, rolo.IndexTheRolodex(context.Background())) + if failure == "empty-content" { + opened, err := rolo.Open(file.fullPath) + require.Error(t, err) + require.Nil(t, opened) + } + }) + } +} + +type synchronizedContentFile struct { + *LocalFile + reads atomic.Int32 +} + +func (f *synchronizedContentFile) GetContent() string { + f.reads.Add(1) + return f.LocalFile.GetContent() +} + +func TestRolodexConcurrentForeignFileCopiesShareOneIndex(t *testing.T) { + const callers = 8 + var workers sync.WaitGroup + source := &synchronizedContentFile{LocalFile: standaloneIndexedFile(t, "type: object\nproperties:\n id: {type: string}\n")} + cfg := CreateOpenAPIIndexConfig() + cfg.IgnoreArrayCircularReferences = true + cfg.IgnorePolymorphicCircularReferences = true + rolo := NewRolodex(cfg) + results := make([]*LocalFile, callers) + errs := make([]error, callers) + start := make(chan struct{}) + workers.Add(callers) + for i := range callers { + go func() { defer workers.Done(); <-start; results[i], errs[i] = rolo.ownIndexedFile(source) }() + } + close(start) + workers.Wait() + require.Equal(t, int32(1), source.reads.Load(), "concurrent opens must parse the source only once") + for i := range callers { + require.NoError(t, errs[i]) + results[i].WaitForIndexing() + require.Same(t, results[0], results[i]) + } + require.Len(t, rolo.GetIndexes(), 1) + idx := results[0].GetIndex() + require.Same(t, rolo, idx.GetRolodex()) + require.True(t, idx.GetResolver().IgnoreArray) + require.True(t, idx.GetResolver().IgnorePoly) + again, err := rolo.ownIndexedFile(source) + require.NoError(t, err) + require.Same(t, results[0], again) +} + +func TestRolodexOwnsCachedRemoteFile(t *testing.T) { + source := standaloneIndexedFile(t, "type: string\n") + remote := &RemoteFile{data: source.data, filename: "schema.yaml", name: "schema.yaml", fullPath: "https://example.com/schema.yaml", extension: YAML} + remote.index.Store(source.GetIndex()) + rolo := NewRolodex(CreateOpenAPIIndexConfig()) + owned, err := rolo.wrapRemoteRolodexFile(remote) + require.NoError(t, err) + require.Same(t, rolo, owned.GetIndex().GetRolodex()) + require.NotSame(t, remote.GetIndex().GetRootNode(), owned.GetIndex().GetRootNode()) + require.Equal(t, remote.GetFullPath(), owned.GetFullPath()) +} + +func (s *sharedIndexedFS) Open(name string) (fs.File, error) { + if file := s.files[filepath.Base(name)]; file != nil { + return file.(fs.File), nil + } + return nil, fs.ErrNotExist +} + +func (s *sharedIndexedFS) GetFiles() map[string]RolodexFile { + if s.lazy { + return nil + } + return s.files +} + +// customIndexedFile preserves the same cached index as LocalFile through a +// custom RolodexFile implementation, as Vacuum's lazy filesystem does. +type customIndexedFile struct{ *LocalFile } + +type blockedContentFile struct { + RolodexFile + fs.File + entered chan struct{} + resume chan struct{} +} + +func (f *blockedContentFile) GetContent() string { + close(f.entered) + <-f.resume + return f.RolodexFile.GetContent() +} + +func TestRolodexPendingOwnedFileRetainsNodeIdentity(t *testing.T) { + files, dir, root := sharedCircularFiles(t, true) + indexSharedFiles(t, files, dir, root) + blocked := &blockedContentFile{RolodexFile: files.files["b.yaml"], File: files.files["b.yaml"].(fs.File), entered: make(chan struct{}), resume: make(chan struct{})} + files.files["b.yaml"] = blocked + files.lazy = true + cfg := CreateOpenAPIIndexConfig() + cfg.BasePath = dir + cfg.SpecAbsolutePath = filepath.Join(dir, "root.yaml") + rolo := NewRolodex(cfg) + rolo.AddLocalFS(dir, files) + var callerRoot yaml.Node + require.NoError(t, yaml.Unmarshal([]byte("components: {}\n"), &callerRoot)) + caller := NewSpecIndexWithConfig(&callerRoot, cfg) + finished := make(chan error, 1) + go func() { _, err := rolo.Open(filepath.Join(dir, "a.yaml")); finished <- err }() + resume := sync.OnceFunc(func() { close(blocked.resume) }) + defer resume() + select { + case <-blocked.entered: + case <-time.After(time.Second): + t.Fatal("foreign indexing did not reach the second file") + } + // The recursive lookup must expose the owned parsed tree without waiting + // for the A -> B -> A index construction to finish. + ref := caller.lookupRolodex(context.Background(), []string{filepath.Join(dir, "a.yaml"), "A"}) + require.NotNil(t, ref) + require.Nil(t, ref.Index) + resume() + select { + case err := <-finished: + require.NoError(t, err) + case <-time.After(time.Second): + t.Fatal("cross-file ownership indexing deadlocked") + } + file, err := rolo.Open(filepath.Join(dir, "a.yaml")) + require.NoError(t, err) + ready := file.GetIndex().FindComponent(context.Background(), "#/A") + require.NotNil(t, ready) + require.Same(t, ref.Node, ready.Node, "pending and completed lookups must use the same YAML nodes") +} + +func sharedCircularFiles(t *testing.T, custom bool) (*sharedIndexedFS, string, []byte) { + t.Helper() + dir, err := filepath.Abs("../test_specs/circular_cross_file/orig") + require.NoError(t, err) + files := &sharedIndexedFS{files: make(map[string]RolodexFile)} + for _, name := range []string{"a.yaml", "b.yaml"} { + data, err := os.ReadFile(filepath.Join(dir, name)) + require.NoError(t, err) + file := &LocalFile{data: data, name: name, filename: name, fullPath: filepath.Join(dir, name), extension: YAML} + if custom { + files.files[name] = &customIndexedFile{file} + } else { + files.files[name] = file + } + } + root, err := os.ReadFile(filepath.Join(dir, "root.yaml")) + require.NoError(t, err) + return files, dir, root +} + +func indexSharedFiles(t *testing.T, files *sharedIndexedFS, dir string, root []byte) *Rolodex { + t.Helper() + cfg := CreateOpenAPIIndexConfig() + cfg.BasePath = dir + cfg.SpecFilePath = "root.yaml" + r := NewRolodex(cfg) + r.AddLocalFS(dir, files) + var node yaml.Node + require.NoError(t, yaml.Unmarshal(root, &node)) + r.SetRootNode(&node) + require.NoError(t, r.IndexTheRolodex(context.Background())) + return r +} + +func TestRolodexSharedIndexedFilesKeepDocumentOwnership(t *testing.T) { + for _, custom := range []bool{false, true} { + for _, lazy := range []bool{false, true} { + name := "local" + if custom { + name = "custom" + } + if lazy { + name += "-lazy" + } + t.Run(name, func(t *testing.T) { + files, dir, root := sharedCircularFiles(t, custom) + first := indexSharedFiles(t, files, dir, root) + firstA := files.files["a.yaml"].GetIndex() + firstB := files.files["b.yaml"].GetIndex() + firstResolver := firstA.GetResolver() + files.lazy = lazy + second := indexSharedFiles(t, files, dir, root) + require.Same(t, firstA, files.files["a.yaml"].GetIndex()) + require.Same(t, firstResolver, firstA.GetResolver(), "building another document must not reset the original resolver") + for _, name := range []string{"a.yaml", "b.yaml"} { + file, err := second.Open(filepath.Join(dir, name)) + require.NoError(t, err) + file.WaitForIndexing() + idx := file.GetIndex() + require.NotSame(t, files.files[name].GetIndex(), idx) + require.Same(t, second, idx.GetRolodex()) + node, err := file.GetContentAsYAMLNode() + require.NoError(t, err) + require.Same(t, idx.GetRootNode(), node) + require.NotSame(t, files.files[name].GetIndex().GetRootNode(), node) + } + require.Len(t, second.GetIndexes(), 2, "lazy indexes must survive batch registration") + var wg sync.WaitGroup + wg.Add(1) + go func() { defer wg.Done(); first.Resolve() }() + for range 100 { + second.GetRootIndex().GetCircularReferences() + second.GetSafeCircularReferences() + second.GetIgnoredCircularReferences() + } + wg.Wait() + require.Same(t, first, firstB.GetRolodex()) + second.Release() + require.Nil(t, second.ownedFiles) + }) + } + } +} diff --git a/index/rolodex_test.go b/index/rolodex_test.go index 85450412..fc4ccbc0 100644 --- a/index/rolodex_test.go +++ b/index/rolodex_test.go @@ -2067,6 +2067,9 @@ func (trf *testRolodexFile) GetContentAsYAMLNode() (*yaml.Node, error) { return &yaml.Node{}, nil } func (trf *testRolodexFile) GetIndex() *SpecIndex { + if trf.errorYaml { + return nil // a file that failed parsing has no cached index + } return &SpecIndex{ specAbsolutePath: "/test/path/spec.yaml", }