From cb235f8a6ce44872fc2a9c1d3c92b5b1ed755314 Mon Sep 17 00:00:00 2001 From: quobix Date: Mon, 28 Sep 2026 11:22:27 -0400 Subject: [PATCH] fix(bundler): compose a file once when a sequence item references it The composed bundler keys each external reference by its target plus the component bucket it infers from the $ref's source path, so one file is composed once per bucket. The index records no position for sequence items, so a $ref directly in one ends its source path with the key that holds the sequence: [allOf] for a schema file whose root is an allOf, or [paths, /a, get, parameters] for an operation's parameter list. The inference expected an item index after those keys, found nothing, and left the key unscoped. The same file referenced from a mapping slot (a property, or a components/parameters entry) got a scoped key, so it was composed twice: Base and Base__schemas, or Limit and Limit__params. Every $ref pointed at the copy, and the schema holding the property ref was dropped from the bundle with "unable to locate reference anywhere in the rolodex". A $ref whose source path ends in allOf, anyOf, oneOf or prefixItems now infers schemas, and one ending in a parameters list infers parameters. components/parameters is a map, so it is left alone, and a component named after one of these keywords still takes its bucket from the components path. The issue-928 fixture hit the same bug through a oneOf: its composed bundle lifted Code2Map, Code3Map and CodeUp4Map twice, as X and X__CodeXMap. It is now composed once, and a test pins that. Fixes #644 Co-Authored-By: Claude Opus 5.5 --- bundler/issue644_test.go | 189 +++++++++++++++++++++++++++++++++ bundler/source_context.go | 24 +++++ bundler/source_context_test.go | 49 +++++++++ 3 files changed, 262 insertions(+) create mode 100644 bundler/issue644_test.go diff --git a/bundler/issue644_test.go b/bundler/issue644_test.go new file mode 100644 index 00000000..14801398 --- /dev/null +++ b/bundler/issue644_test.go @@ -0,0 +1,189 @@ +// Copyright 2026 Princess Beef Heavy Industries / Dave Shanley +// SPDX-License-Identifier: MIT + +package bundler + +import ( + "bytes" + "log/slog" + "os" + "path/filepath" + "testing" + + "github.com/pb33f/go-yaml" + "github.com/pb33f/libopenapi" + "github.com/pb33f/libopenapi/datamodel" + "github.com/pb33f/testify/assert" + "github.com/pb33f/testify/require" +) + +// issue644Bundle writes files into a temporary directory, bundles root with BundleDocumentComposed and +// returns the bundled document along with everything that was logged. +func issue644Bundle(t *testing.T, root string, files map[string]string, sequential bool) (*yaml.Node, string) { + t.Helper() + dir := t.TempDir() + for name, content := range files { + path := filepath.Join(dir, name) + require.NoError(t, os.MkdirAll(filepath.Dir(path), 0o755)) + require.NoError(t, os.WriteFile(path, []byte(content), 0o644)) + } + + var logs bytes.Buffer + config := datamodel.NewDocumentConfiguration() + config.BasePath = dir + config.ExtractRefsSequentially = sequential + config.Logger = slog.New(slog.NewTextHandler(&logs, nil)) + + doc, err := libopenapi.NewDocumentWithConfiguration([]byte(root), config) + require.NoError(t, err) + v3Doc, err := doc.BuildV3Model() + require.NoError(t, err) + + bundled, err := BundleDocumentComposed(&v3Doc.Model, nil) + require.NoError(t, err) + + var node yaml.Node + require.NoError(t, yaml.Unmarshal(bundled, &node)) + return node.Content[0], logs.String() +} + +// issue644Keys returns the keys of a mapping node in order. +func issue644Keys(node *yaml.Node) []string { + var keys []string + for i := 0; i < len(node.Content); i += 2 { + keys = append(keys, node.Content[i].Value) + } + return keys +} + +const issue644Root = `openapi: 3.1.0 +info: + title: repro + version: 1.0.0 +paths: + /a: + get: + responses: + "200": + description: ok + content: + application/json: + schema: + $ref: "./schemas/Wrapper.yaml" + /b: + get: + responses: + "400": + description: bad + content: + application/json: + schema: + $ref: "./schemas/Extended.yaml" +` + +// https://github.com/pb33f/libopenapi/issues/644 +// A schema file referenced from a property and from an item of a top-level allOf (anyOf, oneOf, prefixItems) +// was composed twice: once as Base and once as Base__schemas, with every $ref pointing at the copy and the +// schema holding the property missing from the bundle. +func TestBundleDocumentComposed_Issue644_SchemaReferencedFromSequenceItemAndProperty(t *testing.T) { + extended := map[string]string{ + "allOf": "allOf:\n - $ref: \"./Base.yaml\"\n - type: object\n properties:\n errors:\n type: array\n items:\n type: string\n", + "anyOf": "anyOf:\n - $ref: \"./Base.yaml\"\n - type: string\n", + "oneOf": "oneOf:\n - type: string\n - $ref: \"./Base.yaml\"\n", + "prefixItems": "type: array\nprefixItems:\n - $ref: \"./Base.yaml\"\n", + } + itemIndex := map[string]int{"allOf": 0, "anyOf": 0, "oneOf": 1, "prefixItems": 0} + + for _, keyword := range []string{"allOf", "anyOf", "oneOf", "prefixItems"} { + for _, sequential := range []bool{true, false} { + name := keyword + "/concurrent" + if sequential { + name = keyword + "/sequential" + } + t.Run(name, func(t *testing.T) { + root, logs := issue644Bundle(t, issue644Root, map[string]string{ + "schemas/Base.yaml": "type: object\nproperties:\n title:\n type: string\n", + "schemas/Wrapper.yaml": "type: object\nproperties:\n error:\n $ref: \"./Base.yaml\"\n", + "schemas/Extended.yaml": extended[keyword], + }, sequential) + + assert.NotContains(t, logs, "unable to locate reference") + + schemas := issue607Value(t, root, "components", "schemas") + assert.ElementsMatch(t, []string{"Wrapper", "Extended", "Base"}, issue644Keys(schemas)) + + assert.Equal(t, "#/components/schemas/Base", + issue607Value(t, schemas, "Wrapper", "properties", "error", "$ref").Value) + + items := issue607Value(t, schemas, "Extended", keyword) + require.Equal(t, yaml.SequenceNode, items.Kind) + assert.Equal(t, "#/components/schemas/Base", + issue607Value(t, items.Content[itemIndex[keyword]], "$ref").Value) + }) + } + } +} + +// The same double composition hit parameters: a parameter file referenced from an operation's parameter list +// and from a components/parameters entry in another file was composed twice, as Limit and Limit__params. +func TestBundleDocumentComposed_Issue644_ParameterReferencedFromListAndComponents(t *testing.T) { + root, logs := issue644Bundle(t, `openapi: 3.1.0 +info: + title: repro + version: 1.0.0 +paths: + /a: + get: + parameters: + - $ref: "./params/Limit.yaml" + responses: + "200": + description: ok + /b: + get: + parameters: + - $ref: "./lib.yaml#/components/parameters/PageLimit" + responses: + "200": + description: ok +`, map[string]string{ + "params/Limit.yaml": "name: limit\nin: query\nschema:\n type: integer\n", + "lib.yaml": "components:\n parameters:\n PageLimit:\n $ref: \"./params/Limit.yaml\"\n", + }, true) + + assert.NotContains(t, logs, "unable to locate reference") + + parameters := issue607Value(t, root, "components", "parameters") + assert.Equal(t, []string{"Limit", "PageLimit"}, issue644Keys(parameters)) + assert.Equal(t, "limit", issue607Value(t, parameters, "Limit", "name").Value) + assert.Equal(t, "#/components/parameters/Limit", issue607Value(t, parameters, "PageLimit", "$ref").Value) + + listRef := func(path string) string { + list := issue607Value(t, root, "paths", path, "get", "parameters") + require.Len(t, list.Content, 1) + return issue607Value(t, list.Content[0], "$ref").Value + } + assert.Equal(t, "#/components/parameters/Limit", listRef("/a")) + assert.Equal(t, "#/components/parameters/PageLimit", listRef("/b")) +} + +// The issue-928 fixture references Code2Map, Code3Map and CodeUp4Map from a oneOf inside CodeXMap.yaml, and +// from mapping slots elsewhere. Composed bundling lifted each of them twice, as X and X__CodeXMap. +func TestBundleBytesComposed_Issue644_OneOfReferencesComposeOnce(t *testing.T) { + fixtureDir := filepath.Join("test", "specs", "issue-928") + spec, err := os.ReadFile(filepath.Join(fixtureDir, "api.yaml")) + require.NoError(t, err) + + config := &datamodel.DocumentConfiguration{ + BasePath: fixtureDir, + AllowFileReferences: true, + } + bundled, err := BundleBytesComposed(spec, config, nil) + require.NoError(t, err) + assert.NotContains(t, string(bundled), "__CodeXMap") + + var node yaml.Node + require.NoError(t, yaml.Unmarshal(bundled, &node)) + schemas := issue607Value(t, node.Content[0], "components", "schemas") + assert.Equal(t, []string{"BugDto", "CodeStringOrMapDto", "Code2Map", "Code3Map", "CodeUp4Map"}, issue644Keys(schemas)) +} diff --git a/bundler/source_context.go b/bundler/source_context.go index c4b5b3d3..68611e16 100644 --- a/bundler/source_context.go +++ b/bundler/source_context.go @@ -62,6 +62,12 @@ func inferComponentTypeFromSourcePath(sourcePath []string) (string, bool) { if segment == v3.RequestBodyLabel { return v3.RequestBodiesLabel, true } + + if i == len(sourcePath)-1 { + if componentType, ok := sequenceItemComponentType(segment, previous); ok { + return componentType, true + } + } } if pathContains(sourcePath, v3.CallbacksLabel) { @@ -76,6 +82,24 @@ func inferComponentTypeFromSourcePath(sourcePath []string) (string, bool) { return "", false } +// sequenceItemComponentType classifies a $ref that sits directly in a sequence item. The index records no +// position for sequence items, so the source path of such a ref ends with the key that holds the sequence: +// a schema in an allOf is [..., allOf], and a parameter in an operation's list is [..., get, parameters]. +// Without this, the same file referenced from a sequence item and from a mapping slot gets two processed-ref +// keys and is composed twice (https://github.com/pb33f/libopenapi/issues/644). +func sequenceItemComponentType(segment, previous string) (string, bool) { + switch segment { + case "allOf", "anyOf", "oneOf", "prefixItems": + return v3.SchemasLabel, true + case v3.ParametersLabel: + // components/parameters is a map of named parameters, never a list. + if previous != v3.ComponentsLabel { + return v3.ParametersLabel, true + } + } + return "", false +} + // canComposeContextualReference reports whether a source-slot inference is safe // for the referenced node. JSON Pointer refs already identify a specific node, // so source context can classify sparse but valid targets. Bare-file refs need a diff --git a/bundler/source_context_test.go b/bundler/source_context_test.go index 84e1bab1..90b930bc 100644 --- a/bundler/source_context_test.go +++ b/bundler/source_context_test.go @@ -143,6 +143,55 @@ func TestInferComponentTypeFromSourcePath(t *testing.T) { sourcePath: []string{"x-private", "thing"}, wantOK: false, }, + // The index records no position for sequence items, so a $ref directly in a sequence item has the key + // holding the sequence as its last segment (https://github.com/pb33f/libopenapi/issues/644). + { + name: "top-level allOf item", + sourcePath: []string{"allOf"}, + wantType: v3.SchemasLabel, + wantOK: true, + }, + { + name: "top-level anyOf item", + sourcePath: []string{"anyOf"}, + wantType: v3.SchemasLabel, + wantOK: true, + }, + { + name: "top-level oneOf item", + sourcePath: []string{"oneOf"}, + wantType: v3.SchemasLabel, + wantOK: true, + }, + { + name: "top-level prefixItems item", + sourcePath: []string{"prefixItems"}, + wantType: v3.SchemasLabel, + wantOK: true, + }, + { + name: "operation parameter list item", + sourcePath: []string{"paths", "~1pets", "get", "parameters"}, + wantType: v3.ParametersLabel, + wantOK: true, + }, + { + name: "path item parameter list item", + sourcePath: []string{"paths", "~1pets", "parameters"}, + wantType: v3.ParametersLabel, + wantOK: true, + }, + { + name: "components parameters map is not a list", + sourcePath: []string{"components", "parameters"}, + wantOK: false, + }, + { + name: "parameter component named like a schema keyword", + sourcePath: []string{"components", "parameters", "allOf"}, + wantType: v3.ParametersLabel, + wantOK: true, + }, } for _, tt := range tests {