From 060ddd7bf731ff03c7a1a0227fbe759edc189db0 Mon Sep 17 00:00:00 2001 From: quobix Date: Fri, 30 Jan 2026 09:58:14 -0500 Subject: [PATCH 1/6] Fixing TOCTOU issues --- datamodel/low/extraction_functions.go | 37 +++-- datamodel/low/extraction_functions_test.go | 158 +++++++++++++++++++++ 2 files changed, 182 insertions(+), 13 deletions(-) diff --git a/datamodel/low/extraction_functions.go b/datamodel/low/extraction_functions.go index 72302e52..90bbe42b 100644 --- a/datamodel/low/extraction_functions.go +++ b/datamodel/low/extraction_functions.go @@ -17,7 +17,6 @@ import ( "strconv" "strings" "sync" - "unsafe" jsonpathconfig "github.com/pb33f/jsonpath/pkg/jsonpath/config" @@ -1026,9 +1025,9 @@ func hashYamlNodeFast(n *yaml.Node) string { } // Try cache first for complex nodes + // Use pointer directly as key - *yaml.Node pointers are stable and comparable if n.Kind != yaml.ScalarNode { - cacheKey := uintptr(unsafe.Pointer(n)) - if cached, ok := hashCache.Load(cacheKey); ok { + if cached, ok := hashCache.Load(n); ok { return cached.(string) } } @@ -1043,8 +1042,7 @@ func hashYamlNodeFast(n *yaml.Node) string { // Cache complex nodes if n.Kind != yaml.ScalarNode { - cacheKey := uintptr(unsafe.Pointer(n)) - hashCache.Store(cacheKey, result) + hashCache.Store(n, result) } return result @@ -1071,6 +1069,12 @@ func hashNodeTree(h hash.Hash, n *yaml.Node, visited map[*yaml.Node]bool) { h.Write([]byte(n.Anchor)) } + // CRITICAL: Snapshot Content to prevent TOCTOU races + // This captures the slice header (pointer, len, cap) atomically. + // Even if another goroutine reassigns n.Content later, our local + // 'content' variable still refers to the original backing array. + content := n.Content + // Hash based on node type switch n.Kind { case yaml.ScalarNode: @@ -1078,7 +1082,7 @@ func hashNodeTree(h hash.Hash, n *yaml.Node, visited map[*yaml.Node]bool) { case yaml.SequenceNode: h.Write([]byte("[")) - for _, child := range n.Content { + for _, child := range content { hashNodeTree(h, child, visited) h.Write([]byte(",")) } @@ -1086,6 +1090,13 @@ func hashNodeTree(h hash.Hash, n *yaml.Node, visited map[*yaml.Node]bool) { case yaml.MappingNode: h.Write([]byte("{")) + + // Guard against empty mapping nodes + if len(content) == 0 { + h.Write([]byte("}")) + return + } + // For maps, we need consistent ordering // Collect key-value pairs and sort by key hash type kvPair struct { @@ -1093,17 +1104,17 @@ func hashNodeTree(h hash.Hash, n *yaml.Node, visited map[*yaml.Node]bool) { keyNode *yaml.Node valueNode *yaml.Node } - pairs := make([]kvPair, 0, len(n.Content)/2) + pairs := make([]kvPair, 0, len(content)/2) - for i := 0; i < len(n.Content); i += 2 { - if i+1 < len(n.Content) { + for i := 0; i < len(content); i += 2 { + if i+1 < len(content) { // Hash the key for sorting keyH := sha256.New() - hashNodeTree(keyH, n.Content[i], visited) + hashNodeTree(keyH, content[i], visited) pairs = append(pairs, kvPair{ keyHash: fmt.Sprintf("%x", keyH.Sum(nil)), - keyNode: n.Content[i], - valueNode: n.Content[i+1], + keyNode: content[i], + valueNode: content[i+1], }) } } @@ -1124,7 +1135,7 @@ func hashNodeTree(h hash.Hash, n *yaml.Node, visited map[*yaml.Node]bool) { case yaml.DocumentNode: h.Write([]byte("DOC[")) - for _, child := range n.Content { + for _, child := range content { hashNodeTree(h, child, visited) } h.Write([]byte("]")) diff --git a/datamodel/low/extraction_functions_test.go b/datamodel/low/extraction_functions_test.go index f0959d23..c8478f35 100644 --- a/datamodel/low/extraction_functions_test.go +++ b/datamodel/low/extraction_functions_test.go @@ -3276,3 +3276,161 @@ components: _ = labelNode _ = valueNode } + +// TestHashNodeTree_EmptyMappingNode tests hashing of empty MappingNode with nil Content +func TestHashNodeTree_EmptyMappingNode(t *testing.T) { + // Test MappingNode with nil Content + node := &yaml.Node{ + Kind: yaml.MappingNode, + Tag: "!!map", + } + h := sha256.New() + visited := make(map[*yaml.Node]bool) + hashNodeTree(h, node, visited) + result := h.Sum(nil) + assert.NotNil(t, result) + assert.Greater(t, len(result), 0) +} + +// TestHashNodeTree_EmptyMappingNodeEmptySlice tests hashing of empty MappingNode with empty Content slice +func TestHashNodeTree_EmptyMappingNodeEmptySlice(t *testing.T) { + // Test MappingNode with empty Content slice (not nil) + node := &yaml.Node{ + Kind: yaml.MappingNode, + Tag: "!!map", + Content: []*yaml.Node{}, + } + h := sha256.New() + visited := make(map[*yaml.Node]bool) + hashNodeTree(h, node, visited) + result := h.Sum(nil) + assert.NotNil(t, result) + assert.Greater(t, len(result), 0) +} + +// TestHashNodeTree_MappingNodeOddChildren tests hashing of MappingNode with odd number of children +func TestHashNodeTree_MappingNodeOddChildren(t *testing.T) { + // Test MappingNode with odd number of children (orphan key) + node := &yaml.Node{ + Kind: yaml.MappingNode, + Tag: "!!map", + Content: []*yaml.Node{ + {Kind: yaml.ScalarNode, Value: "orphan_key"}, + }, + } + h := sha256.New() + visited := make(map[*yaml.Node]bool) + hashNodeTree(h, node, visited) + result := h.Sum(nil) + assert.NotNil(t, result) +} + +// TestHashYamlNodeFast_EmptyMappingNode tests fast hashing of empty MappingNode +func TestHashYamlNodeFast_EmptyMappingNode(t *testing.T) { + ClearHashCache() + node := &yaml.Node{ + Kind: yaml.MappingNode, + Tag: "!!map", + Content: []*yaml.Node{}, + } + hash := hashYamlNodeFast(node) + assert.NotEmpty(t, hash) +} + +// TestHashNodeTree_ConcurrentAccess tests for race conditions during concurrent hashing +func TestHashNodeTree_ConcurrentAccess(t *testing.T) { + // Test for race conditions during concurrent hashing + ClearHashCache() + + node := &yaml.Node{ + Kind: yaml.MappingNode, + Tag: "!!map", + Content: []*yaml.Node{ + {Kind: yaml.ScalarNode, Value: "key1"}, + {Kind: yaml.ScalarNode, Value: "value1"}, + {Kind: yaml.ScalarNode, Value: "key2"}, + {Kind: yaml.ScalarNode, Value: "value2"}, + }, + } + + var wg sync.WaitGroup + results := make([]string, 50) + + for i := 0; i < 50; i++ { + wg.Add(1) + go func(idx int) { + defer wg.Done() + results[idx] = hashYamlNodeFast(node) + }(i) + } + + wg.Wait() + + // All goroutines should produce the same hash + expected := results[0] + for i := 1; i < len(results); i++ { + assert.Equal(t, expected, results[i], + "Concurrent hash at index %d doesn't match", i) + } +} + +// TestHashNodeTree_ConcurrentModification simulates the race condition scenario +// NOTE: This test intentionally creates a data race to verify the fix prevents panics. +// The race detector will report the race, but the test demonstrates that the snapshot +// pattern prevents index-out-of-bounds panics. In production, callers should synchronize +// access to yaml.Node objects or use them in single-threaded contexts. +func TestHashNodeTree_ConcurrentModification(t *testing.T) { + if testing.Short() { + t.Skip("Skipping race-condition test in short mode") + } + + // Simulate the race condition scenario where Content is being modified + // during hashing - this test verifies the snapshot pattern prevents panics + ClearHashCache() + + node := &yaml.Node{ + Kind: yaml.MappingNode, + Tag: "!!map", + Content: []*yaml.Node{ + {Kind: yaml.ScalarNode, Value: "key"}, + {Kind: yaml.ScalarNode, Value: "value"}, + }, + } + + var wg sync.WaitGroup + done := make(chan bool) + + // Writer goroutine - modifies Content to simulate the race + wg.Add(1) + go func() { + defer wg.Done() + for i := 0; i < 100; i++ { + // Simulate the race by reassigning Content + // This mimics what happens in ExtractMapNoLookupExtensions + node.Content = []*yaml.Node{ + {Kind: yaml.ScalarNode, Value: fmt.Sprintf("key%d", i)}, + {Kind: yaml.ScalarNode, Value: fmt.Sprintf("value%d", i)}, + } + } + close(done) // Signal reader to stop when writer finishes + }() + + // Reader goroutine - hashes repeatedly until writer signals done + wg.Add(1) + go func() { + defer wg.Done() + for { + select { + case <-done: + return + default: + _ = hashYamlNodeFast(node) + } + } + }() + + // Wait for both goroutines to finish + wg.Wait() + + // If we get here without panic, the fix works +} From ca0258bf9573c3c655037ebb45aad491ef529f84 Mon Sep 17 00:00:00 2001 From: quobix Date: Fri, 30 Jan 2026 09:59:59 -0500 Subject: [PATCH 2/6] Address issue #511 `BundleInlineRefs` was not operating correctly, it got refactored out of use. Added some docs and clarification, and resolved the bundler behavior --- bundler/bundler.go | 116 +++++--- bundler/bundler_test.go | 550 +++++++++++++++++++++++++++++++++++ datamodel/document_config.go | 10 +- 3 files changed, 637 insertions(+), 39 deletions(-) diff --git a/bundler/bundler.go b/bundler/bundler.go index 01b6c95d..17c48771 100644 --- a/bundler/bundler.go +++ b/bundler/bundler.go @@ -27,6 +27,23 @@ import ( // ErrInvalidModel is returned when the model is not usable. var ErrInvalidModel = errors.New("invalid model") +// buildV3ModelFromBytes is a helper that parses bytes and builds a v3 model. +// Returns the model and any build errors. The model may be non-nil even when err is non-nil +// (e.g., circular reference warnings), allowing bundling to proceed with warnings. +func buildV3ModelFromBytes(bytes []byte, configuration *datamodel.DocumentConfiguration) (*v3.Document, error) { + doc, err := libopenapi.NewDocumentWithConfiguration(bytes, configuration) + if err != nil { + return nil, err + } + + v3Doc, buildErr := doc.BuildV3Model() + if v3Doc == nil { + return nil, errors.Join(ErrInvalidModel, buildErr) + } + // Return both model and error - caller decides how to handle warnings/errors + return &v3Doc.Model, buildErr +} + // BundleBytes will take a byte slice of an OpenAPI specification and return a bundled version of it. // This is useful for when you want to take a specification with external references, and you want to bundle it // into a single document. @@ -36,17 +53,12 @@ var ErrInvalidModel = errors.New("invalid model") // // Circular references will not be resolved and will be skipped. func BundleBytes(bytes []byte, configuration *datamodel.DocumentConfiguration) ([]byte, error) { - doc, err := libopenapi.NewDocumentWithConfiguration(bytes, configuration) - if err != nil { + model, err := buildV3ModelFromBytes(bytes, configuration) + if model == nil { return nil, err } - v3Doc, err := doc.BuildV3Model() - if v3Doc == nil { - return nil, errors.Join(ErrInvalidModel, err) - } - - bundledBytes, e := bundleWithConfig(&v3Doc.Model, nil) + bundledBytes, e := bundleWithConfig(model, nil, configuration) return bundledBytes, errors.Join(err, e) } @@ -80,7 +92,7 @@ func BundleBytesComposed(bytes []byte, configuration *datamodel.DocumentConfigur // // Circular references will not be resolved and will be skipped. func BundleDocument(model *v3.Document) ([]byte, error) { - return bundleWithConfig(model, nil) + return bundleWithConfig(model, nil, nil) } // BundleBytesWithConfig will take a byte slice of an OpenAPI specification and return a bundled version of it, @@ -89,17 +101,12 @@ func BundleDocument(model *v3.Document) ([]byte, error) { // Use the BundleInlineConfig to enable features like ResolveDiscriminatorExternalRefs which copies external // schemas referenced by discriminator mappings to the root document's components section. func BundleBytesWithConfig(bytes []byte, configuration *datamodel.DocumentConfiguration, bundleConfig *BundleInlineConfig) ([]byte, error) { - doc, err := libopenapi.NewDocumentWithConfiguration(bytes, configuration) - if err != nil { + model, err := buildV3ModelFromBytes(bytes, configuration) + if model == nil { return nil, err } - v3Doc, err := doc.BuildV3Model() - if v3Doc == nil { - return nil, errors.Join(ErrInvalidModel, err) - } - - bundledBytes, e := bundleWithConfig(&v3Doc.Model, bundleConfig) + bundledBytes, e := bundleWithConfig(model, bundleConfig, configuration) return bundledBytes, errors.Join(err, e) } @@ -109,7 +116,7 @@ func BundleBytesWithConfig(bytes []byte, configuration *datamodel.DocumentConfig // Use the BundleInlineConfig to enable features like ResolveDiscriminatorExternalRefs which copies external // schemas referenced by discriminator mappings to the root document's components section. func BundleDocumentWithConfig(model *v3.Document, bundleConfig *BundleInlineConfig) ([]byte, error) { - return bundleWithConfig(model, bundleConfig) + return bundleWithConfig(model, bundleConfig, nil) } // BundleCompositionConfig is used to configure the composition of OpenAPI documents when using BundleDocumentComposed. @@ -119,6 +126,14 @@ type BundleCompositionConfig struct { } // BundleInlineConfig provides configuration options for inline bundling. +// +// Example usage: +// // Inline everything including local refs +// inlineTrue := true +// config := &BundleInlineConfig{ +// InlineLocalRefs: &inlineTrue, +// } +// bundled, err := BundleBytesWithConfig(specBytes, docConfig, config) type BundleInlineConfig struct { // ResolveDiscriminatorExternalRefs when true, copies external schemas referenced // by discriminator mappings to the root document's components section. @@ -126,6 +141,13 @@ type BundleInlineConfig struct { // in external files reference other schemas in those external files. // Default: false (preserves existing behavior of keeping external refs as-is) ResolveDiscriminatorExternalRefs bool + + // InlineLocalRefs controls whether local component references are inlined during bundling. + // When nil, falls back to DocumentConfiguration.BundleInlineRefs. + // - false: preserve local refs like #/components/schemas/Pet (discriminator-safe, default behavior) + // - true: inline all refs including local component refs + // Default: nil (uses DocumentConfiguration.BundleInlineRefs) + InlineLocalRefs *bool } // BundleDocumentComposed will take a v3.Document and return a composed bundled version of it. Composed means @@ -260,30 +282,47 @@ func compose(model *v3.Document, compositionConfig *BundleCompositionConfig) ([] return b, errors.Join(errs...) } -func bundleWithConfig(model *v3.Document, config *BundleInlineConfig) ([]byte, error) { - // Enable bundling mode to preserve local component refs. - // This ensures refs to schemas already in the root document aren't inlined. - highbase.SetBundlingMode(true) - defer highbase.SetBundlingMode(false) +// resolveBundleInlineConfig resolves the inlineLocalRefs setting from the fallback chain: +// 1. BundleInlineConfig.InlineLocalRefs (explicit per-call) +// 2. DocumentConfiguration.BundleInlineRefs (document-wide default) +// 3. false (system default - preserve local refs) +func resolveBundleInlineConfig(bundleConfig *BundleInlineConfig, docConfig *datamodel.DocumentConfiguration) bool { + if bundleConfig != nil && bundleConfig.InlineLocalRefs != nil { + return *bundleConfig.InlineLocalRefs + } + if docConfig != nil { + return docConfig.BundleInlineRefs + } + return false // system default +} + +func bundleWithConfig(model *v3.Document, config *BundleInlineConfig, docConfig *datamodel.DocumentConfiguration) ([]byte, error) { + if model == nil { + return nil, errors.New("model cannot be nil") + } + + inlineLocalRefs := resolveBundleInlineConfig(config, docConfig) + + // enable bundling mode to preserve local component refs during marshalling + // when inlineLocalRefs is true, skip bundling mode to inline everything + if !inlineLocalRefs { + highbase.SetBundlingMode(true) + defer highbase.SetBundlingMode(false) + } if model.Rolodex != nil { - // Handle discriminator external refs if enabled. - // This copies external schemas referenced by discriminator mappings to the root - // document's components section, ensuring the bundled output is valid. + // copy external schemas referenced by discriminator mappings to root components + // ensures bundled output is valid and self-contained if config != nil && config.ResolveDiscriminatorExternalRefs { resolveDiscriminatorExternalRefs(model) } - // Resolve extension refs before rendering. - // Extensions are raw *yaml.Node and bypass MarshalYAMLInline(), so we resolve them separately. - // NOTE: This mutates the model's extension nodes in-place. + // resolve extension refs before rendering (mutates model's extension nodes in-place) + // extensions are raw yaml nodes that bypass MarshalYAMLInline() resolveExtensionRefs(model.Rolodex) } - // Use RenderInline which resolves refs on-the-fly during rendering. - // Discriminator mappings are preserved via Schema.MarshalYAMLInline() which - // marks oneOf/anyOf SchemaProxy items to preserve their references. - // Circular references are handled in SchemaProxy.MarshalYAMLInline(). + // render inline - discriminator mappings and circular refs are preserved via SchemaProxy.MarshalYAMLInline() return model.RenderInline() } @@ -400,11 +439,14 @@ func collectExternalDiscriminatorSchemas(rolodex *index.Rolodex, rootIdx *index. continue } - // Find the index for this file using pre-built map (O(1) lookup) - // All pinned refs come from indexed files, so this lookup always succeeds - sourceIdx := indexByPath[filePath] + // find the index for this file using pre-built map (O(1) lookup) + sourceIdx, ok := indexByPath[filePath] + if !ok { + // defensive: skip if index not found (shouldn't happen with valid specs) + continue + } - // Find the actual reference - this was already found when pinning + // find the actual reference - this was already found when pinning ref, _ := sourceIdx.SearchIndexForReference(jsonPointer) // Extract schema name from the JSON pointer diff --git a/bundler/bundler_test.go b/bundler/bundler_test.go index a06a6b0b..fcb6a75d 100644 --- a/bundler/bundler_test.go +++ b/bundler/bundler_test.go @@ -2282,3 +2282,553 @@ components: assert.Equal(t, "#node", itemsSchema.DynamicRef, "DynamicRef should be '#node'") } +// ============================================================================ +// BundleInlineRefs Configuration Tests +// These tests verify the BundleInlineRefs flag functionality (Issue #511) +// Issue #511: https://github.com/pb33f/libopenapi/issues/511 +// ============================================================================ + +// TestBundleInlineRefs_Default_PreservesLocalRefs verifies the default behavior +// when BundleInlineRefs is not set (defaults to false). +// Local component refs like #/components/schemas/Pet should be preserved. +func TestBundleInlineRefs_Default_PreservesLocalRefs(t *testing.T) { + spec := `openapi: 3.1.0 +info: + title: Test API + version: 1.0.0 +paths: + /pets: + get: + responses: + '200': + description: Success + content: + application/json: + schema: + $ref: '#/components/schemas/Pet' +components: + schemas: + Pet: + type: object + properties: + name: + type: string + tag: + type: string` + + // Default config (BundleInlineRefs not set, defaults to false) + config := datamodel.NewDocumentConfiguration() + + bundled, err := BundleBytes([]byte(spec), config) + require.NoError(t, err) + require.NotNil(t, bundled) + + bundledStr := string(bundled) + + // Local refs should be preserved in default mode + assert.Contains(t, bundledStr, "$ref: '#/components/schemas/Pet'") + assert.Contains(t, bundledStr, "components:") + assert.Contains(t, bundledStr, "schemas:") + assert.Contains(t, bundledStr, "Pet:") +} + +// TestBundleInlineRefs_False_PreservesLocalRefs verifies that explicitly setting +// BundleInlineRefs to false preserves local component refs. +func TestBundleInlineRefs_False_PreservesLocalRefs(t *testing.T) { + spec := `openapi: 3.1.0 +info: + title: Test API + version: 1.0.0 +paths: + /pets: + get: + responses: + '200': + description: Success + content: + application/json: + schema: + $ref: '#/components/schemas/Pet' +components: + schemas: + Pet: + type: object + properties: + name: + type: string` + + config := datamodel.NewDocumentConfiguration() + config.BundleInlineRefs = false // Explicitly set to false + + bundled, err := BundleBytes([]byte(spec), config) + require.NoError(t, err) + require.NotNil(t, bundled) + + bundledStr := string(bundled) + + // Local refs should be preserved + assert.Contains(t, bundledStr, "$ref: '#/components/schemas/Pet'") + assert.Contains(t, bundledStr, "components:") + assert.Contains(t, bundledStr, "Pet:") +} + +// TestBundleInlineRefs_True_InlinesLocalRefs verifies that setting +// BundleInlineRefs to true causes local component refs to be inlined. +// This resolves Issue #511 where the flag had no effect. +func TestBundleInlineRefs_True_InlinesLocalRefs(t *testing.T) { + spec := `openapi: 3.1.0 +info: + title: Test API + version: 1.0.0 +paths: + /pets: + get: + responses: + '200': + description: Success + content: + application/json: + schema: + $ref: '#/components/schemas/Pet' +components: + schemas: + Pet: + type: object + properties: + name: + type: string + tag: + type: string` + + config := datamodel.NewDocumentConfiguration() + config.BundleInlineRefs = true // Enable full inlining + + bundled, err := BundleBytes([]byte(spec), config) + require.NoError(t, err) + require.NotNil(t, bundled) + + bundledStr := string(bundled) + + // Local refs should be inlined (no $ref to Pet) + assert.NotContains(t, bundledStr, "$ref: '#/components/schemas/Pet'") + + // The schema should be inlined directly in the response + assert.Contains(t, bundledStr, "schema:") + assert.Contains(t, bundledStr, "type: object") + assert.Contains(t, bundledStr, "properties:") + assert.Contains(t, bundledStr, "name:") +} + +// TestBundleInlineConfig_OverridesDocConfig verifies that BundleInlineConfig.InlineLocalRefs +// takes precedence over DocumentConfiguration.BundleInlineRefs. +func TestBundleInlineConfig_OverridesDocConfig(t *testing.T) { + spec := `openapi: 3.1.0 +info: + title: Test API + version: 1.0.0 +paths: + /pets: + get: + responses: + '200': + description: Success + content: + application/json: + schema: + $ref: '#/components/schemas/Pet' +components: + schemas: + Pet: + type: object + properties: + name: + type: string` + + // Document config says preserve refs + docConfig := datamodel.NewDocumentConfiguration() + docConfig.BundleInlineRefs = false + + // Bundle config overrides to inline refs + inlineTrue := true + bundleConfig := &BundleInlineConfig{ + InlineLocalRefs: &inlineTrue, + } + + bundled, err := BundleBytesWithConfig([]byte(spec), docConfig, bundleConfig) + require.NoError(t, err) + require.NotNil(t, bundled) + + bundledStr := string(bundled) + + // BundleInlineConfig should win - refs should be inlined + assert.NotContains(t, bundledStr, "$ref: '#/components/schemas/Pet'") + assert.Contains(t, bundledStr, "type: object") +} + +// TestBundleInlineConfig_NilUsesDocConfig verifies that when BundleInlineConfig.InlineLocalRefs +// is nil, it falls back to DocumentConfiguration.BundleInlineRefs. +func TestBundleInlineConfig_NilUsesDocConfig(t *testing.T) { + spec := `openapi: 3.1.0 +info: + title: Test API + version: 1.0.0 +paths: + /pets: + get: + responses: + '200': + description: Success + content: + application/json: + schema: + $ref: '#/components/schemas/Pet' +components: + schemas: + Pet: + type: object + properties: + name: + type: string` + + // Document config says inline refs + docConfig := datamodel.NewDocumentConfiguration() + docConfig.BundleInlineRefs = true + + // Bundle config doesn't override (InlineLocalRefs is nil) + bundleConfig := &BundleInlineConfig{ + InlineLocalRefs: nil, // Not set - should use docConfig + } + + bundled, err := BundleBytesWithConfig([]byte(spec), docConfig, bundleConfig) + require.NoError(t, err) + require.NotNil(t, bundled) + + bundledStr := string(bundled) + + // Should fall back to docConfig - refs should be inlined + assert.NotContains(t, bundledStr, "$ref: '#/components/schemas/Pet'") + assert.Contains(t, bundledStr, "type: object") +} + +// TestBundleInlineConfig_FalseOverridesDocConfigTrue verifies that explicitly +// setting BundleInlineConfig.InlineLocalRefs to false overrides a true value +// in DocumentConfiguration.BundleInlineRefs. +func TestBundleInlineConfig_FalseOverridesDocConfigTrue(t *testing.T) { + spec := `openapi: 3.1.0 +info: + title: Test API + version: 1.0.0 +paths: + /pets: + get: + responses: + '200': + description: Success + content: + application/json: + schema: + $ref: '#/components/schemas/Pet' +components: + schemas: + Pet: + type: object + properties: + name: + type: string` + + // Document config says inline refs + docConfig := datamodel.NewDocumentConfiguration() + docConfig.BundleInlineRefs = true + + // Bundle config explicitly says preserve refs + inlineFalse := false + bundleConfig := &BundleInlineConfig{ + InlineLocalRefs: &inlineFalse, + } + + bundled, err := BundleBytesWithConfig([]byte(spec), docConfig, bundleConfig) + require.NoError(t, err) + require.NotNil(t, bundled) + + bundledStr := string(bundled) + + // BundleInlineConfig should win - refs should be preserved + assert.Contains(t, bundledStr, "$ref: '#/components/schemas/Pet'") + assert.Contains(t, bundledStr, "components:") + assert.Contains(t, bundledStr, "Pet:") +} + +// TestBundleDocument_NoConfigAvailable verifies that BundleDocument (which doesn't +// have access to DocumentConfiguration) uses system defaults (preserve refs). +func TestBundleDocument_NoConfigAvailable(t *testing.T) { + spec := `openapi: 3.1.0 +info: + title: Test API + version: 1.0.0 +paths: + /pets: + get: + responses: + '200': + description: Success + content: + application/json: + schema: + $ref: '#/components/schemas/Pet' +components: + schemas: + Pet: + type: object + properties: + name: + type: string` + + doc, err := libopenapi.NewDocument([]byte(spec)) + require.NoError(t, err) + + v3Doc, err := doc.BuildV3Model() + require.NoError(t, err) + + bundled, err := BundleDocument(&v3Doc.Model) + require.NoError(t, err) + require.NotNil(t, bundled) + + bundledStr := string(bundled) + + // Without config, should use system default (preserve refs) + assert.Contains(t, bundledStr, "$ref: '#/components/schemas/Pet'") + assert.Contains(t, bundledStr, "components:") +} + +// TestBundleWithConfig_NilModel verifies that passing a nil model returns an error. +func TestBundleWithConfig_NilModel(t *testing.T) { + config := datamodel.NewDocumentConfiguration() + + // Call bundleWithConfig directly with nil model + _, err := bundleWithConfig(nil, nil, config) + require.Error(t, err) + assert.Contains(t, err.Error(), "model cannot be nil") +} + +// TestBundleInlineRefs_CircularRefs_AlwaysSkipped verifies that circular references +// are never inlined, even with BundleInlineRefs: true. +func TestBundleInlineRefs_CircularRefs_AlwaysSkipped(t *testing.T) { + spec := `openapi: 3.1.0 +info: + title: Test API + version: 1.0.0 +paths: + /nodes: + get: + responses: + '200': + description: Success + content: + application/json: + schema: + $ref: '#/components/schemas/TreeNode' +components: + schemas: + TreeNode: + type: object + properties: + value: + type: string + children: + type: array + items: + $ref: '#/components/schemas/TreeNode'` + + config := datamodel.NewDocumentConfiguration() + config.BundleInlineRefs = true // Try to inline everything + + bundled, err := BundleBytes([]byte(spec), config) + require.NoError(t, err) + require.NotNil(t, bundled) + + bundledStr := string(bundled) + + // Circular refs should still be preserved (can't inline infinite recursion) + // The ref in the items should remain + assert.Contains(t, bundledStr, "$ref:") + assert.Contains(t, bundledStr, "TreeNode") +} + +// ============================================================================ +// Issue #511: BundleInlineRefs Flag Was Non-Functional +// ============================================================================ +// Previously, setting BundleInlineRefs: true had no effect because the flag +// wasn't wired up to the bundler's SetBundlingMode() mechanism. These tests +// verify the fix. +// Issue #511: https://github.com/pb33f/libopenapi/issues/511 + +// TestIssue511_BundleInlineRefs_WasNonFunctional verifies that Issue #511 is fixed. +func TestIssue511_BundleInlineRefs_WasNonFunctional(t *testing.T) { + // This is the scenario from Issue #511 + spec := `openapi: 3.1.0 +info: + title: Pet Store API + version: 1.0.0 +paths: + /pets: + get: + summary: List all pets + responses: + '200': + description: A list of pets + content: + application/json: + schema: + type: array + items: + $ref: '#/components/schemas/Pet' + /pets/{petId}: + get: + summary: Get a pet by ID + parameters: + - name: petId + in: path + required: true + schema: + type: string + responses: + '200': + description: A pet + content: + application/json: + schema: + $ref: '#/components/schemas/Pet' +components: + schemas: + Pet: + type: object + required: + - id + - name + properties: + id: + type: string + name: + type: string + tag: + type: string` + + // Before the fix: setting BundleInlineRefs: true had NO EFFECT + // After the fix: setting BundleInlineRefs: true DOES inline local refs + + config := &datamodel.DocumentConfiguration{ + BundleInlineRefs: true, // This was broken - didn't actually inline refs + } + + bundled, err := BundleBytes([]byte(spec), config) + require.NoError(t, err) + require.NotNil(t, bundled) + + bundledStr := string(bundled) + + // After the fix: refs should be inlined + assert.NotContains(t, bundledStr, "$ref: '#/components/schemas/Pet'", + "BundleInlineRefs: true should inline local component refs") + + // Verify the schema is actually inlined in both locations + assert.Contains(t, bundledStr, "type: array") + assert.Contains(t, bundledStr, "items:") + assert.Contains(t, bundledStr, "type: object") + assert.Contains(t, bundledStr, "required:") + + // The components section might still exist but shouldn't be referenced + // (or it might be removed entirely during rendering - either is fine) +} + +// TestIssue511_BackwardCompatibility verifies that the default behavior +// (BundleInlineRefs not set or set to false) still preserves local refs +// to maintain backward compatibility after fixing Issue #511. +func TestIssue511_BackwardCompatibility(t *testing.T) { + spec := `openapi: 3.1.0 +info: + title: Pet Store API + version: 1.0.0 +paths: + /pets: + get: + responses: + '200': + description: A list of pets + content: + application/json: + schema: + $ref: '#/components/schemas/Pet' +components: + schemas: + Pet: + type: object + properties: + name: + type: string` + + // Default behavior - don't set BundleInlineRefs (or set to false) + config := datamodel.NewDocumentConfiguration() + // config.BundleInlineRefs defaults to false + + bundled, err := BundleBytes([]byte(spec), config) + require.NoError(t, err) + require.NotNil(t, bundled) + + bundledStr := string(bundled) + + // Default behavior should preserve local refs (backward compatible) + assert.Contains(t, bundledStr, "$ref: '#/components/schemas/Pet'", + "Default behavior should preserve local refs for backward compatibility") + assert.Contains(t, bundledStr, "components:") + assert.Contains(t, bundledStr, "schemas:") + assert.Contains(t, bundledStr, "Pet:") +} + +// TestIssue511_PerCallOverride verifies that BundleInlineConfig.InlineLocalRefs +// can override DocumentConfiguration.BundleInlineRefs on a per-call basis. +// This provides the fine-grained control requested in Issue #511. +func TestIssue511_PerCallOverride(t *testing.T) { + spec := `openapi: 3.1.0 +info: + title: Pet Store API + version: 1.0.0 +paths: + /pets: + get: + responses: + '200': + description: A list of pets + content: + application/json: + schema: + $ref: '#/components/schemas/Pet' +components: + schemas: + Pet: + type: object + properties: + name: + type: string` + + // Document config says preserve refs + docConfig := &datamodel.DocumentConfiguration{ + BundleInlineRefs: false, + } + + // But we want to inline for this specific call + inlineTrue := true + bundleConfig := &BundleInlineConfig{ + InlineLocalRefs: &inlineTrue, + } + + bundled, err := BundleBytesWithConfig([]byte(spec), docConfig, bundleConfig) + require.NoError(t, err) + require.NotNil(t, bundled) + + bundledStr := string(bundled) + + // Per-call config should override document config + assert.NotContains(t, bundledStr, "$ref: '#/components/schemas/Pet'", + "BundleInlineConfig.InlineLocalRefs should override DocumentConfiguration.BundleInlineRefs") + assert.Contains(t, bundledStr, "type: object") +} diff --git a/datamodel/document_config.go b/datamodel/document_config.go index 580961c7..cf3207d0 100644 --- a/datamodel/document_config.go +++ b/datamodel/document_config.go @@ -130,8 +130,14 @@ type DocumentConfiguration struct { // defaults to false (which means extensions will be included) ExcludeExtensionRefs bool - // BundleInlineRefs is used by the bundler module. If set to true, all references will be inlined, including - // local references (to the root document) as well as all external references. This is false by default. + // BundleInlineRefs controls whether local component references are inlined during bundling. + // When false (default): Local refs like #/components/schemas/Pet are preserved + // When true: Local refs are also inlined (may break discriminator mappings) + // + // Note: This setting can be overridden per-call using BundleInlineConfig.InlineLocalRefs + // when calling bundler.BundleBytesWithConfig() + // + // Circular references are ALWAYS preserved regardless of this setting. BundleInlineRefs bool // RecomposeRefs is used by the bundler module. If set to true, all references will be composed into the root document. From dc8be693214c5767f14fac189c08b6ed2df44b91 Mon Sep 17 00:00:00 2001 From: quobix Date: Fri, 30 Jan 2026 12:28:27 -0500 Subject: [PATCH 3/6] coverage update ensure we hit the defensive check --- bundler/bundler_test.go | 116 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 116 insertions(+) diff --git a/bundler/bundler_test.go b/bundler/bundler_test.go index fcb6a75d..14fa093e 100644 --- a/bundler/bundler_test.go +++ b/bundler/bundler_test.go @@ -15,6 +15,7 @@ import ( "os" "os/exec" "path/filepath" + "reflect" "runtime" "strings" "sync" @@ -2168,6 +2169,121 @@ paths: {}` assert.Empty(t, result, "Expected no external schemas when all discriminator refs are internal") } +func TestCollectExternalDiscriminatorSchemas_DefensiveContinue(t *testing.T) { + // Test: defensive check at line 446 - when indexByPath lookup fails + // This test uses reflection to manipulate the rolodex's internal state to create + // a scenario where an index path exists in the pinned map but not in the rolodex's + // index list. This "shouldn't happen with valid specs" but the defensive check + // protects against edge cases like concurrent rolodex modifications, path mismatches, + // or corrupted state. + + tmpDir := t.TempDir() + + // Create main spec with discriminator mapping to external files + mainSpec := `openapi: 3.1.0 +info: + title: Main API + version: 1.0.0 +components: + schemas: + Pet: + type: object + discriminator: + propertyName: petType + mapping: + dog: './external.yaml#/components/schemas/Dog' + cat: './external2.yaml#/components/schemas/Cat' + oneOf: + - $ref: './external.yaml#/components/schemas/Dog' + - $ref: './external2.yaml#/components/schemas/Cat' +paths: {}` + + externalSpec1 := `openapi: 3.1.0 +info: + title: External API 1 + version: 1.0.0 +components: + schemas: + Dog: + type: object + properties: + breed: + type: string +paths: {}` + + externalSpec2 := `openapi: 3.1.0 +info: + title: External API 2 + version: 1.0.0 +components: + schemas: + Cat: + type: object + properties: + color: + type: string +paths: {}` + + mainPath := filepath.Join(tmpDir, "main.yaml") + externalPath1 := filepath.Join(tmpDir, "external.yaml") + externalPath2 := filepath.Join(tmpDir, "external2.yaml") + + err := os.WriteFile(mainPath, []byte(mainSpec), 0644) + require.NoError(t, err) + err = os.WriteFile(externalPath1, []byte(externalSpec1), 0644) + require.NoError(t, err) + err = os.WriteFile(externalPath2, []byte(externalSpec2), 0644) + require.NoError(t, err) + + mainBytes, err := os.ReadFile(mainPath) + require.NoError(t, err) + + config := datamodel.NewDocumentConfiguration() + config.BasePath = tmpDir + + doc, err := libopenapi.NewDocumentWithConfiguration(mainBytes, config) + require.NoError(t, err) + + model, errs := doc.BuildV3Model() + require.Empty(t, errs) + require.NotNil(t, model) + + rolodex := model.Model.Rolodex + rootIdx := rolodex.GetRootIndex() + + // Verify normal operation first + result := collectExternalDiscriminatorSchemas(rolodex, rootIdx) + require.Len(t, result, 2, "Should collect both external schemas initially") + + // Use reflection to manipulate the rolodex's internal indexes slice + // to remove one of the external indexes, creating the scenario where + // an index path exists in the pinned map but not in GetIndexes() + rolodexVal := reflect.ValueOf(rolodex).Elem() + indexesField := rolodexVal.FieldByName("indexes") + + // Make the field writable using reflection + indexesField = reflect.NewAt(indexesField.Type(), indexesField.Addr().UnsafePointer()).Elem() + + // Get the current indexes slice + currentIndexes := indexesField.Interface().([]*index.SpecIndex) + require.GreaterOrEqual(t, len(currentIndexes), 2, "Should have at least 2 external indexes") + + // Remove the last external index from the slice to create a mismatch + // This simulates the edge case where an index was removed or is missing + modifiedIndexes := currentIndexes[:len(currentIndexes)-1] + indexesField.Set(reflect.ValueOf(modifiedIndexes)) + + // Now call collectExternalDiscriminatorSchemas again + // The function should gracefully handle the missing index via the defensive continue + result2 := collectExternalDiscriminatorSchemas(rolodex, rootIdx) + + // The result should have one less schema due to the missing index + // The defensive continue at line 446 prevents a panic or nil pointer dereference + assert.LessOrEqual(t, len(result2), len(result), "Should handle missing index gracefully") + assert.GreaterOrEqual(t, len(result2), 0, "Function should not panic with missing index") + assert.Len(t, result2, 1, "Should have one schema remaining after removing one index") +} + func TestCopySchemaToComponents_NameCollision(t *testing.T) { // Test: existingNames[finalName] collision path existingNames := map[string]bool{ From f007b93d1d994e399e7cbea6bf454a7d59e057ef Mon Sep 17 00:00:00 2001 From: quobix Date: Sat, 31 Jan 2026 17:31:08 -0500 Subject: [PATCH 4/6] Track origins when composed bundling. Now we can track the origins of all the references from the composed bundled doc. --- bundler/bundler.go | 146 +++++ bundler/bundler_composer.go | 118 ++-- bundler/composer_functions.go | 94 +++- bundler/origin.go | 83 +++ bundler/origin_test.go | 981 ++++++++++++++++++++++++++++++++++ 5 files changed, 1351 insertions(+), 71 deletions(-) create mode 100644 bundler/origin.go create mode 100644 bundler/origin_test.go diff --git a/bundler/bundler.go b/bundler/bundler.go index 17c48771..d51f7a78 100644 --- a/bundler/bundler.go +++ b/bundler/bundler.go @@ -82,6 +82,23 @@ func BundleBytesComposed(bytes []byte, configuration *datamodel.DocumentConfigur return bundledBytes, errors.Join(err, e) } +// BundleBytesComposedWithOrigins returns a bundled spec with origin tracking for navigation. +// This enables consumers to map bundled components back to their original file locations. +func BundleBytesComposedWithOrigins(bytes []byte, configuration *datamodel.DocumentConfiguration, compositionConfig *BundleCompositionConfig) (*BundleResult, error) { + doc, err := libopenapi.NewDocumentWithConfiguration(bytes, configuration) + if err != nil { + return nil, err + } + + v3Doc, err := doc.BuildV3Model() + if v3Doc == nil || err != nil { + return nil, errors.Join(ErrInvalidModel, err) + } + + result, e := composeWithOrigins(&v3Doc.Model, compositionConfig) + return result, errors.Join(err, e) +} + // BundleDocument will take a v3.Document and return a bundled version of it. // This is useful for when you want to take a document that has been built // from a specification with external references, and you want to bundle it @@ -161,6 +178,134 @@ func BundleDocumentComposed(model *v3.Document, compositionConfig *BundleComposi return compose(model, compositionConfig) } +// composeWithOrigins performs composed bundling and returns origin tracking information +func composeWithOrigins(model *v3.Document, compositionConfig *BundleCompositionConfig) (*BundleResult, error) { + if compositionConfig == nil { + compositionConfig = &BundleCompositionConfig{ + Delimiter: "__", + } + } else { + if compositionConfig.Delimiter == "" { + compositionConfig.Delimiter = "__" + } + if strings.Contains(compositionConfig.Delimiter, "#") || + strings.Contains(compositionConfig.Delimiter, "/") { + return nil, errors.New("composition delimiter cannot contain '#' or '/' characters") + } + if strings.Contains(compositionConfig.Delimiter, " ") { + return nil, errors.New("composition delimiter cannot contain spaces") + } + } + + if model == nil || model.Rolodex == nil { + return nil, errors.New("model or rolodex is nil") + } + + rolodex := model.Rolodex + indexes := rolodex.GetIndexes() + + discriminatorMappings := collectDiscriminatorMappingNodes(rolodex) + + cf := &handleIndexConfig{ + idx: rolodex.GetRootIndex(), + model: model, + indexes: indexes, + seen: sync.Map{}, + refMap: orderedmap.New[string, *processRef](), + compositionConfig: compositionConfig, + discriminatorMappings: discriminatorMappings, + origins: make(ComponentOriginMap), + } + if err := handleIndex(cf); err != nil { + return nil, err + } + + processedNodes := orderedmap.New[string, *processRef]() + var errs []error + for _, ref := range cf.refMap.FromOldest() { + err := processReference(model, ref, cf) + errs = append(errs, err) + processedNodes.Set(ref.ref.FullDefinition, ref) + } + + slices.SortFunc(indexes, func(i, j *index.SpecIndex) int { + if i.GetSpecAbsolutePath() < j.GetSpecAbsolutePath() { + return 1 + } + return 0 + }) + + rootIndex := rolodex.GetRootIndex() + remapIndex(rootIndex, processedNodes) + + for _, idx := range indexes { + remapIndex(idx, processedNodes) + } + + updateDiscriminatorMappingsComposed(discriminatorMappings, processedNodes, rolodex) + + // anything that could not be recomposed and needs inlining + inlinedPaths := make(map[string]*yaml.Node) + for _, pr := range cf.inlineRequired { + if pr.refPointer != "" { + + // if the ref is a pointer to an external pointer, then we need to stitch it. + uri := strings.Split(pr.refPointer, "#/") + if len(uri) == 2 { + if uri[0] != "" { + if !filepath.IsAbs(uri[0]) && !strings.HasPrefix(uri[0], "http") { + // if the uri is not absolute, then we need to make it absolute. + uri[0] = filepath.Join(filepath.Dir(pr.idx.GetSpecAbsolutePath()), uri[0]) + } + pointerRef := pr.idx.FindComponent(context.Background(), strings.Join(uri, "#/")) + pr.seqRef.Node.Content = pointerRef.Node.Content + // Track this inlined content for reuse + if pr.ref != nil { + inlinedPaths[pr.ref.FullDefinition] = pointerRef.Node + } + continue + } + } + } + pr.seqRef.Node.Content = pr.ref.Node.Content + // Track this inlined content for reuse + if pr.ref != nil { + inlinedPaths[pr.ref.FullDefinition] = pr.ref.Node + } + } + + // Fix any remaining absolute path references that match inlined content + // Also check the root index + allIndexes := append(indexes, rolodex.GetRootIndex()) + for _, idx := range allIndexes { + for _, seqRef := range idx.GetRawReferencesSequenced() { + if isRef, _, refVal := utils.IsNodeRefValue(seqRef.Node); isRef { + // Check if this is an absolute path that should have been inlined + if filepath.IsAbs(refVal) { + // Try to find matching inlined content + for inlinedPath, inlinedNode := range inlinedPaths { + // Match if paths are the same or if they refer to the same file + if refVal == inlinedPath { + seqRef.Node.Content = inlinedNode.Content + break + } + } + } + } + } + } + + b, err := model.Render() + errs = append(errs, err) + + result := &BundleResult{ + Bytes: b, + Origins: cf.origins, + } + + return result, errors.Join(errs...) +} + func compose(model *v3.Document, compositionConfig *BundleCompositionConfig) ([]byte, error) { if compositionConfig == nil { compositionConfig = &BundleCompositionConfig{ @@ -196,6 +341,7 @@ func compose(model *v3.Document, compositionConfig *BundleCompositionConfig) ([] refMap: orderedmap.New[string, *processRef](), compositionConfig: compositionConfig, discriminatorMappings: discriminatorMappings, + origins: make(ComponentOriginMap), } if err := handleIndex(cf); err != nil { return nil, err diff --git a/bundler/bundler_composer.go b/bundler/bundler_composer.go index cc4e201a..65f767a1 100644 --- a/bundler/bundler_composer.go +++ b/bundler/bundler_composer.go @@ -18,12 +18,14 @@ import ( ) type processRef struct { - idx *index.SpecIndex - ref *index.Reference - seqRef *index.Reference - refPointer string - name string - location []string + idx *index.SpecIndex + ref *index.Reference + seqRef *index.Reference + refPointer string + name string + location []string + wasRenamed bool // true when component was renamed due to collision + originalName string // original name before collision renaming } type handleIndexConfig struct { @@ -35,6 +37,7 @@ type handleIndexConfig struct { inlineRequired []*processRef compositionConfig *BundleCompositionConfig discriminatorMappings []*yaml.Node + origins ComponentOriginMap // component origins for navigation } // handleIndex will recursively explore the indexes and their references, building a map of references @@ -207,83 +210,57 @@ func processReference(model *v3.Document, pr *processRef, cf *handleIndexConfig) if len(location) > 1 { switch location[1] { case v3low.SchemasLabel: - if len(location) > 2 { - schemaName := location[2] - if components.Schemas != nil { - return checkReferenceAndBubbleUp(schemaName, cf.compositionConfig.Delimiter, - pr, idx, components.Schemas, buildSchema) - } + if len(location) > 2 && components.Schemas != nil { + return checkReferenceAndCapture(location[2], cf.compositionConfig.Delimiter, + v3low.SchemasLabel, pr, idx, components.Schemas, buildSchema, cf.origins) } case v3low.ResponsesLabel: - if len(location) > 2 { - responseCode := location[2] - if components.Responses != nil { - return checkReferenceAndBubbleUp(responseCode, cf.compositionConfig.Delimiter, - pr, idx, components.Responses, buildResponse) - } + if len(location) > 2 && components.Responses != nil { + return checkReferenceAndCapture(location[2], cf.compositionConfig.Delimiter, + v3low.ResponsesLabel, pr, idx, components.Responses, buildResponse, cf.origins) } case v3low.ParametersLabel: - if len(location) > 2 { - paramName := location[2] - if components.Parameters != nil { - return checkReferenceAndBubbleUp(paramName, cf.compositionConfig.Delimiter, - pr, idx, components.Parameters, buildParameter) - } + if len(location) > 2 && components.Parameters != nil { + return checkReferenceAndCapture(location[2], cf.compositionConfig.Delimiter, + v3low.ParametersLabel, pr, idx, components.Parameters, buildParameter, cf.origins) } case v3low.HeadersLabel: - if len(location) > 2 { - headerName := location[2] - if components.Headers != nil { - return checkReferenceAndBubbleUp(headerName, cf.compositionConfig.Delimiter, - pr, idx, components.Headers, buildHeader) - } + if len(location) > 2 && components.Headers != nil { + return checkReferenceAndCapture(location[2], cf.compositionConfig.Delimiter, + v3low.HeadersLabel, pr, idx, components.Headers, buildHeader, cf.origins) } case v3low.RequestBodiesLabel: - if len(location) > 2 { - requestBodyName := location[2] - if components.RequestBodies != nil { - return checkReferenceAndBubbleUp(requestBodyName, cf.compositionConfig.Delimiter, - pr, idx, components.RequestBodies, buildRequestBody) - } + if len(location) > 2 && components.RequestBodies != nil { + return checkReferenceAndCapture(location[2], cf.compositionConfig.Delimiter, + v3low.RequestBodiesLabel, pr, idx, components.RequestBodies, buildRequestBody, cf.origins) } + case v3low.ExamplesLabel: - if len(location) > 2 { - exampleName := location[2] - if components.Examples != nil { - return checkReferenceAndBubbleUp(exampleName, cf.compositionConfig.Delimiter, - pr, idx, components.Examples, buildExample) - } + if len(location) > 2 && components.Examples != nil { + return checkReferenceAndCapture(location[2], cf.compositionConfig.Delimiter, + v3low.ExamplesLabel, pr, idx, components.Examples, buildExample, cf.origins) } case v3low.LinksLabel: - if len(location) > 2 { - linksName := location[2] - if components.Links != nil { - return checkReferenceAndBubbleUp(linksName, cf.compositionConfig.Delimiter, - pr, idx, components.Links, buildLink) - } + if len(location) > 2 && components.Links != nil { + return checkReferenceAndCapture(location[2], cf.compositionConfig.Delimiter, + v3low.LinksLabel, pr, idx, components.Links, buildLink, cf.origins) } case v3low.CallbacksLabel: - if len(location) > 2 { - callbacks := location[2] - if components.Callbacks != nil { - return checkReferenceAndBubbleUp(callbacks, cf.compositionConfig.Delimiter, - pr, idx, components.Callbacks, buildCallback) - } + if len(location) > 2 && components.Callbacks != nil { + return checkReferenceAndCapture(location[2], cf.compositionConfig.Delimiter, + v3low.CallbacksLabel, pr, idx, components.Callbacks, buildCallback, cf.origins) } case v3low.PathItemsLabel: - if len(location) > 2 { - pathItem := location[2] - if components.PathItems != nil { - return checkReferenceAndBubbleUp(pathItem, cf.compositionConfig.Delimiter, - pr, idx, components.PathItems, buildPathItem) - } + if len(location) > 2 && components.PathItems != nil { + return checkReferenceAndCapture(location[2], cf.compositionConfig.Delimiter, + v3low.PathItemsLabel, pr, idx, components.PathItems, buildPathItem, cf.origins) } } } @@ -308,61 +285,64 @@ func processReference(model *v3.Document, pr *processRef, cf *handleIndexConfig) return nil } + // preserve original name before collision handling + pr.originalName = componentName + if importType, ok := DetectOpenAPIComponentType(pr.ref.Node); ok { switch importType { case v3low.SchemasLabel: if components.Schemas != nil { pr.name = checkForCollision(componentName, delim, pr, components.Schemas) pr.location = []string{v3low.ComponentsLabel, v3low.SchemasLabel, pr.name} - return checkReferenceAndBubbleUp(pr.name, delim, pr, idx, components.Schemas, buildSchema) + return checkReferenceAndCapture(pr.name, delim, v3low.SchemasLabel, pr, idx, components.Schemas, buildSchema, cf.origins) } case v3low.ResponsesLabel: if components.Responses != nil { pr.name = checkForCollision(componentName, delim, pr, components.Responses) pr.location = []string{v3low.ComponentsLabel, v3low.ResponsesLabel, pr.name} - return checkReferenceAndBubbleUp(pr.name, delim, pr, idx, components.Responses, buildResponse) + return checkReferenceAndCapture(pr.name, delim, v3low.ResponsesLabel, pr, idx, components.Responses, buildResponse, cf.origins) } case v3low.ParametersLabel: if components.Parameters != nil { pr.name = checkForCollision(componentName, delim, pr, components.Parameters) pr.location = []string{v3low.ComponentsLabel, v3low.ParametersLabel, pr.name} - return checkReferenceAndBubbleUp(pr.name, delim, pr, idx, components.Parameters, buildParameter) + return checkReferenceAndCapture(pr.name, delim, v3low.ParametersLabel, pr, idx, components.Parameters, buildParameter, cf.origins) } case v3low.HeadersLabel: if components.Headers != nil { pr.name = checkForCollision(componentName, delim, pr, components.Headers) pr.location = []string{v3low.ComponentsLabel, v3low.HeadersLabel, pr.name} - return checkReferenceAndBubbleUp(pr.name, delim, pr, idx, components.Headers, buildHeader) + return checkReferenceAndCapture(pr.name, delim, v3low.HeadersLabel, pr, idx, components.Headers, buildHeader, cf.origins) } case v3low.RequestBodiesLabel: if components.RequestBodies != nil { pr.name = checkForCollision(componentName, delim, pr, components.RequestBodies) pr.location = []string{v3low.ComponentsLabel, v3low.RequestBodiesLabel, pr.name} - return checkReferenceAndBubbleUp(pr.name, delim, pr, idx, components.RequestBodies, buildRequestBody) + return checkReferenceAndCapture(pr.name, delim, v3low.RequestBodiesLabel, pr, idx, components.RequestBodies, buildRequestBody, cf.origins) } case v3low.ExamplesLabel: if components.Examples != nil { pr.name = checkForCollision(componentName, delim, pr, components.Examples) pr.location = []string{v3low.ComponentsLabel, v3low.ExamplesLabel, pr.name} - return checkReferenceAndBubbleUp(pr.name, delim, pr, idx, components.Examples, buildExample) + return checkReferenceAndCapture(pr.name, delim, v3low.ExamplesLabel, pr, idx, components.Examples, buildExample, cf.origins) } case v3low.LinksLabel: if components.Links != nil { pr.name = checkForCollision(componentName, delim, pr, components.Links) pr.location = []string{v3low.ComponentsLabel, v3low.LinksLabel, pr.name} - return checkReferenceAndBubbleUp(pr.name, delim, pr, idx, components.Links, buildLink) + return checkReferenceAndCapture(pr.name, delim, v3low.LinksLabel, pr, idx, components.Links, buildLink, cf.origins) } case v3low.CallbacksLabel: if components.Callbacks != nil { pr.name = checkForCollision(componentName, delim, pr, components.Callbacks) pr.location = []string{v3low.ComponentsLabel, v3low.CallbacksLabel, pr.name} - return checkReferenceAndBubbleUp(pr.name, delim, pr, idx, components.Callbacks, buildCallback) + return checkReferenceAndCapture(pr.name, delim, v3low.CallbacksLabel, pr, idx, components.Callbacks, buildCallback, cf.origins) } case v3low.PathItemsLabel: if components.PathItems != nil { pr.name = checkForCollision(componentName, delim, pr, components.PathItems) pr.location = []string{v3low.ComponentsLabel, v3low.PathItemsLabel, pr.name} - return checkReferenceAndBubbleUp(pr.name, delim, pr, idx, components.PathItems, buildPathItem) + return checkReferenceAndCapture(pr.name, delim, v3low.PathItemsLabel, pr, idx, components.PathItems, buildPathItem, cf.origins) } } } diff --git a/bundler/composer_functions.go b/bundler/composer_functions.go index 1fe0825f..da47f5c6 100644 --- a/bundler/composer_functions.go +++ b/bundler/composer_functions.go @@ -23,6 +23,15 @@ import ( "go.yaml.in/yaml/v4" ) +// extractFragment returns the JSON pointer fragment from a full definition. +// e.g., "file.yaml#/components/schemas/Pet" → "#/components/schemas/Pet" +func extractFragment(fullDef string) string { + if idx := strings.Index(fullDef, "#/"); idx != -1 { + return fullDef[idx:] + } + return "#/" +} + func calculateCollisionName(name, pointer, delimiter string, iteration int) string { jsonPointer := strings.Split(pointer, "#/") if len(jsonPointer) == 2 { @@ -83,23 +92,57 @@ func checkReferenceAndBubbleUp[T any]( componentMap *orderedmap.Map[string, T], buildFunc func(node *yaml.Node, idx *index.SpecIndex) (T, error), ) error { - // Build the component + // preserve original name before collision handling (unless already set) + if pr != nil && pr.originalName == "" { + pr.originalName = name + } + component, err := buildFunc(pr.ref.Node, idx) if err != nil { return err } + wasRenamed := false + // Handle potential collisions and add to the component map if v := componentMap.GetOrZero(name); !isZeroOfType(v) { uniqueName := handleCollision(name, delimiter, pr, componentMap) componentMap.Set(uniqueName, component) + wasRenamed = true + name = uniqueName } else { componentMap.Set(name, component) } + // update final name and renamed flag (preserve existing wasRenamed=true if already set) + if pr != nil { + pr.name = name + // only update wasRenamed if it's being set to true, or if it wasn't already true + if wasRenamed || !pr.wasRenamed { + pr.wasRenamed = wasRenamed + } + } + return nil } +// checkReferenceAndCapture combines reference building and origin tracking. +// eliminates duplication of the check-capture-return pattern used throughout processReference. +func checkReferenceAndCapture[T any]( + name, delimiter, componentType string, + pr *processRef, + idx *index.SpecIndex, + componentMap *orderedmap.Map[string, T], + buildFunc func(node *yaml.Node, idx *index.SpecIndex) (T, error), + origins ComponentOriginMap, +) error { + err := checkReferenceAndBubbleUp(name, delimiter, pr, idx, componentMap, buildFunc) + if err == nil && origins != nil { + captureOrigin(pr, componentType, origins) + } + return err +} + func isZeroOfType[T any](v T) bool { isZero := reflect.ValueOf(v).IsZero() return isZero @@ -118,11 +161,27 @@ func handleCollision[T any](schemaName, delimiter string, pr *processRef, compon } pr.name = uniqueName + pr.wasRenamed = true return uniqueName } func handleFileImport[T any](pr *processRef, importType, delimiter string, components *orderedmap.Map[string, T]) []string { - name := checkForCollision(filepath.Base(strings.Replace(pr.ref.Name, filepath.Ext(pr.ref.Name), "", 1)), delimiter, pr, components) + // extract base name from file before collision handling + baseName := filepath.Base(strings.Replace(pr.ref.Name, filepath.Ext(pr.ref.Name), "", 1)) + + // preserve original name before any renaming + if pr.originalName == "" { + pr.originalName = baseName + } + + // check for collisions and get final name + name := checkForCollision(baseName, delimiter, pr, components) + + // detect if renaming occurred + if name != baseName { + pr.wasRenamed = true + } + pr.name = name pr.ref.Name = name pr.seqRef.Name = name @@ -329,3 +388,34 @@ func buildPathItem(node *yaml.Node, idx *index.SpecIndex) (*v3.PathItem, error) err := pathItem.Build(ctx, &yaml.Node{}, node, idx) return v3.NewPathItem(&pathItem), err } + +// captureOrigin records origin information for a processed reference. +// enables navigation from bundled components back to their source files. +func captureOrigin(pr *processRef, componentType string, origins ComponentOriginMap) { + if pr == nil || pr.ref == nil || pr.idx == nil || origins == nil { + return + } + + originalRef := extractFragment(pr.ref.FullDefinition) + + originalName := pr.originalName + if originalName == "" { + originalName = pr.name + } + + // pr.name is updated by checkReferenceAndBubbleUp after collision handling + bundledRef := "#/components/" + componentType + "/" + pr.name + + origin := &ComponentOrigin{ + OriginalFile: pr.idx.GetSpecAbsolutePath(), + OriginalRef: originalRef, + OriginalName: originalName, + Line: pr.ref.Node.Line, + Column: pr.ref.Node.Column, + WasRenamed: pr.wasRenamed, + BundledRef: bundledRef, + ComponentType: componentType, + } + + origins[bundledRef] = origin +} diff --git a/bundler/origin.go b/bundler/origin.go new file mode 100644 index 00000000..05b8ae49 --- /dev/null +++ b/bundler/origin.go @@ -0,0 +1,83 @@ +// Copyright 2023-2026 Princess Beef Heavy Industries, LLC / Dave Shanley +// https://pb33f.io +// SPDX-License-Identifier: MIT + +package bundler + +// ComponentOrigin tracks the original location of a component that was lifted +// into the bundled document's components section. +type ComponentOrigin struct { + // OriginalFile is the absolute path to the file containing the original definition. + // e.g., "/path/to/models/User.yaml" + OriginalFile string `json:"originalFile" yaml:"originalFile"` + + // OriginalRef is the JSON Pointer within the original file. + // e.g., "#/components/schemas/User" or "#/User" + OriginalRef string `json:"originalRef" yaml:"originalRef"` + + // OriginalName is the component name before any collision renaming. + // e.g., "User" (even if bundled as "User__2") + OriginalName string `json:"originalName" yaml:"originalName"` + + // Line is the 1-based line number in the original file. + Line int `json:"line" yaml:"line"` + + // Column is the 1-based column number in the original file. + Column int `json:"column" yaml:"column"` + + // WasRenamed indicates if the component was renamed due to collision. + WasRenamed bool `json:"wasRenamed" yaml:"wasRenamed"` + + // BundledRef is the final JSON Pointer in the bundled document. + // e.g., "#/components/schemas/User__2" + BundledRef string `json:"bundledRef" yaml:"bundledRef"` + + // ComponentType is the type of component (schemas, responses, parameters, etc.) + ComponentType string `json:"componentType" yaml:"componentType"` +} + +// ComponentOriginMap maps bundled refs to their original locations. +// Key is the bundled JSON Pointer (e.g., "#/components/schemas/User"). +type ComponentOriginMap map[string]*ComponentOrigin + +// BundleResult contains the bundled bytes and origin tracking information. +type BundleResult struct { + // Bytes is the bundled YAML output. + Bytes []byte + + // Origins maps bundled JSON Pointer paths to their original locations. + // This enables navigation from bundled components back to source files. + Origins ComponentOriginMap +} + +// NewBundleResult creates a new BundleResult with initialized maps. +func NewBundleResult() *BundleResult { + return &BundleResult{ + Origins: make(ComponentOriginMap), + } +} + +// AddOrigin adds a component origin to the result. +func (r *BundleResult) AddOrigin(bundledRef string, origin *ComponentOrigin) { + if r.Origins == nil { + r.Origins = make(ComponentOriginMap) + } + origin.BundledRef = bundledRef + r.Origins[bundledRef] = origin +} + +// GetOrigin retrieves the origin for a bundled reference. +func (r *BundleResult) GetOrigin(bundledRef string) *ComponentOrigin { + if r.Origins == nil { + return nil + } + return r.Origins[bundledRef] +} + +// OriginCount returns the number of tracked origins. +func (r *BundleResult) OriginCount() int { + if r.Origins == nil { + return 0 + } + return len(r.Origins) +} diff --git a/bundler/origin_test.go b/bundler/origin_test.go new file mode 100644 index 00000000..4abab909 --- /dev/null +++ b/bundler/origin_test.go @@ -0,0 +1,981 @@ +// Copyright 2023-2026 Princess Beef Heavy Industries, LLC / Dave Shanley +// https://pb33f.io +// SPDX-License-Identifier: MIT + +package bundler + +import ( + "os" + "path/filepath" + "testing" + + "github.com/pb33f/libopenapi" + "github.com/pb33f/libopenapi/datamodel" + "github.com/pb33f/libopenapi/index" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.yaml.in/yaml/v4" +) + +func TestComponentOrigin_Structure(t *testing.T) { + t.Run("initializes with all fields", func(t *testing.T) { + origin := &ComponentOrigin{ + OriginalFile: "/path/to/models/User.yaml", + OriginalRef: "#/components/schemas/User", + OriginalName: "User", + Line: 10, + Column: 2, + WasRenamed: false, + BundledRef: "#/components/schemas/User", + ComponentType: "schemas", + } + + assert.Equal(t, "/path/to/models/User.yaml", origin.OriginalFile) + assert.Equal(t, "#/components/schemas/User", origin.OriginalRef) + assert.Equal(t, "User", origin.OriginalName) + assert.Equal(t, 10, origin.Line) + assert.Equal(t, 2, origin.Column) + assert.False(t, origin.WasRenamed) + assert.Equal(t, "#/components/schemas/User", origin.BundledRef) + assert.Equal(t, "schemas", origin.ComponentType) + }) + + t.Run("handles renamed components", func(t *testing.T) { + origin := &ComponentOrigin{ + OriginalName: "Pet", + WasRenamed: true, + BundledRef: "#/components/schemas/Pet__2", + } + + assert.Equal(t, "Pet", origin.OriginalName) + assert.True(t, origin.WasRenamed) + assert.Equal(t, "#/components/schemas/Pet__2", origin.BundledRef) + }) +} + +func TestBundleResult_NewBundleResult(t *testing.T) { + t.Run("creates result with initialized maps", func(t *testing.T) { + result := NewBundleResult() + + assert.NotNil(t, result) + assert.NotNil(t, result.Origins) + assert.Equal(t, 0, len(result.Origins)) + assert.Nil(t, result.Bytes) + }) +} + +func TestBundleResult_AddOrigin(t *testing.T) { + t.Run("adds origin to map", func(t *testing.T) { + result := NewBundleResult() + origin := &ComponentOrigin{ + OriginalFile: "/models/user.yaml", + OriginalName: "User", + } + + result.AddOrigin("#/components/schemas/User", origin) + + assert.Equal(t, 1, len(result.Origins)) + assert.Equal(t, "#/components/schemas/User", origin.BundledRef) + retrieved := result.Origins["#/components/schemas/User"] + assert.Equal(t, "/models/user.yaml", retrieved.OriginalFile) + }) + + t.Run("initializes origins map if nil", func(t *testing.T) { + result := &BundleResult{} + assert.Nil(t, result.Origins) + + origin := &ComponentOrigin{OriginalName: "Test"} + result.AddOrigin("#/components/schemas/Test", origin) + + assert.NotNil(t, result.Origins) + assert.Equal(t, 1, len(result.Origins)) + }) + + t.Run("overwrites existing origin for same key", func(t *testing.T) { + result := NewBundleResult() + + origin1 := &ComponentOrigin{OriginalFile: "/file1.yaml"} + result.AddOrigin("#/components/schemas/User", origin1) + + origin2 := &ComponentOrigin{OriginalFile: "/file2.yaml"} + result.AddOrigin("#/components/schemas/User", origin2) + + assert.Equal(t, 1, len(result.Origins)) + assert.Equal(t, "/file2.yaml", result.Origins["#/components/schemas/User"].OriginalFile) + }) +} + +func TestBundleResult_GetOrigin(t *testing.T) { + t.Run("retrieves existing origin", func(t *testing.T) { + result := NewBundleResult() + origin := &ComponentOrigin{OriginalFile: "/test.yaml"} + result.AddOrigin("#/components/schemas/Test", origin) + + retrieved := result.GetOrigin("#/components/schemas/Test") + + assert.NotNil(t, retrieved) + assert.Equal(t, "/test.yaml", retrieved.OriginalFile) + }) + + t.Run("returns nil for non-existent key", func(t *testing.T) { + result := NewBundleResult() + + retrieved := result.GetOrigin("#/components/schemas/NonExistent") + + assert.Nil(t, retrieved) + }) + + t.Run("handles nil origins map", func(t *testing.T) { + result := &BundleResult{} + + retrieved := result.GetOrigin("#/components/schemas/Test") + + assert.Nil(t, retrieved) + }) +} + +func TestBundleResult_OriginCount(t *testing.T) { + t.Run("returns zero for empty map", func(t *testing.T) { + result := NewBundleResult() + + assert.Equal(t, 0, result.OriginCount()) + }) + + t.Run("returns correct count", func(t *testing.T) { + result := NewBundleResult() + result.AddOrigin("#/components/schemas/User", &ComponentOrigin{}) + result.AddOrigin("#/components/schemas/Pet", &ComponentOrigin{}) + result.AddOrigin("#/components/responses/Success", &ComponentOrigin{}) + + assert.Equal(t, 3, result.OriginCount()) + }) + + t.Run("handles nil origins map", func(t *testing.T) { + result := &BundleResult{} + + assert.Equal(t, 0, result.OriginCount()) + }) +} + +func TestBundleBytesComposedWithOrigins_SimpleSpec(t *testing.T) { + // create temp directory for test files + tmpDir := t.TempDir() + + // create main spec + mainYAML := `openapi: 3.1.0 +info: + title: Test API + version: 1.0.0 +paths: + /users: + get: + responses: + '200': + $ref: './responses.yaml#/UserListResponse' +components: + schemas: + LocalSchema: + type: object + properties: + id: + type: string` + + // create external responses file + responsesYAML := `UserListResponse: + description: List of users + content: + application/json: + schema: + $ref: './schemas.yaml#/UserList'` + + // create external schemas file + schemasYAML := `UserList: + type: array + items: + $ref: '#/User' +User: + type: object + properties: + name: + type: string` + + // write files + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "main.yaml"), []byte(mainYAML), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "responses.yaml"), []byte(responsesYAML), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "schemas.yaml"), []byte(schemasYAML), 0644)) + + // load and bundle + config := &datamodel.DocumentConfiguration{ + AllowFileReferences: true, + BasePath: tmpDir, + SpecFilePath: filepath.Join(tmpDir, "main.yaml"), + } + + mainBytes, err := os.ReadFile(filepath.Join(tmpDir, "main.yaml")) + require.NoError(t, err) + + result, err := BundleBytesComposedWithOrigins(mainBytes, config, nil) + require.NoError(t, err) + require.NotNil(t, result) + + assert.NotEmpty(t, result.Bytes) + assert.NotNil(t, result.Origins) + assert.Greater(t, len(result.Origins), 0, "should have tracked origins") + assert.Greater(t, result.OriginCount(), 0) + + // verify we can find the User schema origin + var userOrigin *ComponentOrigin + for bundledRef, origin := range result.Origins { + if origin.OriginalName == "User" { + userOrigin = origin + assert.Contains(t, bundledRef, "User") + break + } + } + assert.NotNil(t, userOrigin, "User schema should have tracked origin") + if userOrigin != nil { + assert.Contains(t, userOrigin.OriginalFile, "schemas.yaml") + assert.Equal(t, "User", userOrigin.OriginalName) + assert.Equal(t, "schemas", userOrigin.ComponentType) + assert.Greater(t, userOrigin.Line, 0) + } +} + +func TestBundleBytesComposedWithOrigins_CollisionHandling(t *testing.T) { + tmpDir := t.TempDir() + + // create main spec with a local Pet schema + mainYAML := `openapi: 3.1.0 +info: + title: Test API + version: 1.0.0 +components: + schemas: + Pet: + type: object + properties: + localId: + type: string +paths: + /pets: + get: + responses: + '200': + content: + application/json: + schema: + $ref: './external.yaml#/Pet'` + + // create external file with conflicting Pet schema + externalYAML := `Pet: + type: object + properties: + externalId: + type: integer` + + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "main.yaml"), []byte(mainYAML), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "external.yaml"), []byte(externalYAML), 0644)) + + config := &datamodel.DocumentConfiguration{ + AllowFileReferences: true, + BasePath: tmpDir, + SpecFilePath: filepath.Join(tmpDir, "main.yaml"), + } + + mainBytes, err := os.ReadFile(filepath.Join(tmpDir, "main.yaml")) + require.NoError(t, err) + + result, err := BundleBytesComposedWithOrigins(mainBytes, config, nil) + require.NoError(t, err) + require.NotNil(t, result) + + // debug: print all origins + t.Logf("Total origins tracked: %d", len(result.Origins)) + for bundledRef, origin := range result.Origins { + t.Logf("Origin: bundled=%s, original=%s, file=%s, renamed=%v", + bundledRef, origin.OriginalName, filepath.Base(origin.OriginalFile), origin.WasRenamed) + } + + // find the Pet schema from external file - it will be renamed due to collision with main.yaml's Pet + var externalPetOrigin *ComponentOrigin + for bundledRef, origin := range result.Origins { + if filepath.Base(origin.OriginalFile) == "external.yaml" { + externalPetOrigin = origin + t.Logf("Found external Pet: bundled=%s, original=%s, renamed=%v", + bundledRef, origin.OriginalName, origin.WasRenamed) + break + } + } + + // verify the external Pet was tracked + assert.NotNil(t, externalPetOrigin, "external Pet schema should have origin") + if externalPetOrigin != nil { + assert.Contains(t, externalPetOrigin.OriginalFile, "external.yaml") + assert.Equal(t, "schemas", externalPetOrigin.ComponentType) + // the bundled ref should be different from #/components/schemas/Pet due to collision + assert.NotEqual(t, "#/components/schemas/Pet", externalPetOrigin.BundledRef) + assert.Contains(t, externalPetOrigin.BundledRef, "Pet") + } +} + +func TestBundleBytesComposedWithOrigins_MultipleComponentTypes(t *testing.T) { + tmpDir := t.TempDir() + + mainYAML := `openapi: 3.1.0 +info: + title: Test API + version: 1.0.0 +paths: + /users: + get: + parameters: + - $ref: './params.yaml#/UserId' + responses: + '200': + $ref: './responses.yaml#/UserResponse' + requestBody: + $ref: './bodies.yaml#/UserInput'` + + paramsYAML := `UserId: + name: id + in: query + schema: + type: string` + + responsesYAML := `UserResponse: + description: User data + content: + application/json: + schema: + type: object` + + bodiesYAML := `UserInput: + description: User input + content: + application/json: + schema: + type: object` + + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "main.yaml"), []byte(mainYAML), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "params.yaml"), []byte(paramsYAML), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "responses.yaml"), []byte(responsesYAML), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "bodies.yaml"), []byte(bodiesYAML), 0644)) + + config := &datamodel.DocumentConfiguration{ + AllowFileReferences: true, + BasePath: tmpDir, + SpecFilePath: filepath.Join(tmpDir, "main.yaml"), + } + + mainBytes, err := os.ReadFile(filepath.Join(tmpDir, "main.yaml")) + require.NoError(t, err) + + result, err := BundleBytesComposedWithOrigins(mainBytes, config, nil) + require.NoError(t, err) + require.NotNil(t, result) + + // verify we tracked all component types + foundTypes := make(map[string]bool) + for _, origin := range result.Origins { + foundTypes[origin.ComponentType] = true + } + + // at minimum, we should have tracked different component types + assert.Greater(t, len(foundTypes), 0, "should have multiple component types") + + // debug: print all origins + t.Logf("Total origins tracked: %d", len(result.Origins)) + for bundledRef, origin := range result.Origins { + t.Logf("Origin: bundled=%s, name=%s, type=%s, file=%s", + bundledRef, origin.OriginalName, origin.ComponentType, filepath.Base(origin.OriginalFile)) + } + + // verify specific origins - check what we actually got + foundResponse := false + + for _, origin := range result.Origins { + if origin.OriginalName == "UserResponse" { + foundResponse = true + // check that the origin was tracked, component type may vary + assert.Contains(t, origin.OriginalFile, "responses.yaml") + } + } + + assert.True(t, foundResponse, "should track response origin") + // note: parameters and request bodies may not be tracked if they're inlined + // rather than lifted to components +} + +func TestBundleBytesComposedWithOrigins_SingleFileSpec(t *testing.T) { + simpleSpec := `openapi: 3.1.0 +info: + title: Simple API + version: 1.0.0 +paths: + /test: + get: + responses: + '200': + description: OK +components: + schemas: + Simple: + type: string` + + config := &datamodel.DocumentConfiguration{ + AllowFileReferences: false, + } + + result, err := BundleBytesComposedWithOrigins([]byte(simpleSpec), config, nil) + require.NoError(t, err) + require.NotNil(t, result) + + assert.NotEmpty(t, result.Bytes) + // single-file specs may have no external refs to track + assert.NotNil(t, result.Origins) +} + +func TestBundleBytesComposedWithOrigins_CustomDelimiter(t *testing.T) { + tmpDir := t.TempDir() + + mainYAML := `openapi: 3.1.0 +info: + title: Test + version: 1.0.0 +components: + schemas: + Pet: + type: object +paths: + /pets: + get: + responses: + '200': + content: + application/json: + schema: + $ref: './external.yaml#/Pet'` + + externalYAML := `Pet: + type: string` + + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "main.yaml"), []byte(mainYAML), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "external.yaml"), []byte(externalYAML), 0644)) + + config := &datamodel.DocumentConfiguration{ + AllowFileReferences: true, + BasePath: tmpDir, + } + + mainBytes, err := os.ReadFile(filepath.Join(tmpDir, "main.yaml")) + require.NoError(t, err) + + // use custom delimiter + compositionConfig := &BundleCompositionConfig{ + Delimiter: "@@", + } + + result, err := BundleBytesComposedWithOrigins(mainBytes, config, compositionConfig) + require.NoError(t, err) + + // verify origin tracking works with custom delimiter + assert.NotNil(t, result.Origins) + assert.Greater(t, len(result.Origins), 0, "should track origins") + + // debug log + t.Logf("Origins with custom delimiter: %d", len(result.Origins)) + for bundledRef, origin := range result.Origins { + t.Logf(" bundled=%s, original=%s, renamed=%v", bundledRef, origin.OriginalName, origin.WasRenamed) + if origin.WasRenamed { + assert.Contains(t, bundledRef, "@@", "renamed component should use custom delimiter") + } + } +} + +func TestBundleBytesComposedWithOrigins_ErrorHandling(t *testing.T) { + t.Run("handles invalid spec", func(t *testing.T) { + invalidSpec := []byte("not: valid: yaml:") + + config := &datamodel.DocumentConfiguration{} + result, err := BundleBytesComposedWithOrigins(invalidSpec, config, nil) + + assert.Error(t, err) + assert.Nil(t, result) + }) + + t.Run("handles non-openapi document", func(t *testing.T) { + notOpenAPI := []byte("key: value\nother: data") + + config := &datamodel.DocumentConfiguration{} + result, err := BundleBytesComposedWithOrigins(notOpenAPI, config, nil) + + assert.Error(t, err) + assert.Nil(t, result) + }) +} + +func TestBundleBytesComposedWithOrigins_LineAndColumnTracking(t *testing.T) { + tmpDir := t.TempDir() + + mainYAML := `openapi: 3.1.0 +info: + title: Test + version: 1.0.0 +paths: + /test: + get: + responses: + '200': + content: + application/json: + schema: + $ref: './schemas.yaml#/TestSchema'` + + // schemas file with specific line numbers we can verify + schemasYAML := `# Comment line 1 +# Comment line 2 +TestSchema: + type: object + properties: + field1: + type: string` + + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "main.yaml"), []byte(mainYAML), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "schemas.yaml"), []byte(schemasYAML), 0644)) + + config := &datamodel.DocumentConfiguration{ + AllowFileReferences: true, + BasePath: tmpDir, + } + + mainBytes, err := os.ReadFile(filepath.Join(tmpDir, "main.yaml")) + require.NoError(t, err) + + result, err := BundleBytesComposedWithOrigins(mainBytes, config, nil) + require.NoError(t, err) + + // find TestSchema origin + var testSchemaOrigin *ComponentOrigin + for _, origin := range result.Origins { + if origin.OriginalName == "TestSchema" { + testSchemaOrigin = origin + break + } + } + + require.NotNil(t, testSchemaOrigin, "should track TestSchema origin") + assert.Equal(t, "TestSchema", testSchemaOrigin.OriginalName) + // YAML parsing adds document node, so actual line number may be +1 + assert.Greater(t, testSchemaOrigin.Line, 0, "should have line info") + assert.Greater(t, testSchemaOrigin.Column, 0, "should have column info") +} + +func TestCaptureOrigin_EdgeCases(t *testing.T) { + t.Run("handles nil processRef", func(t *testing.T) { + origins := make(ComponentOriginMap) + + // should not panic + assert.NotPanics(t, func() { + captureOrigin(nil, "schemas", origins) + }) + + assert.Equal(t, 0, len(origins)) + }) + + t.Run("handles nil origins map", func(t *testing.T) { + pr := &processRef{ + name: "Test", + } + + // should not panic + assert.NotPanics(t, func() { + captureOrigin(pr, "schemas", nil) + }) + }) + + t.Run("handles missing ref in processRef", func(t *testing.T) { + origins := make(ComponentOriginMap) + pr := &processRef{ + name: "Test", + ref: nil, // nil ref + } + + captureOrigin(pr, "schemas", origins) + + assert.Equal(t, 0, len(origins), "should not add origin with nil ref") + }) + + t.Run("handles missing idx in processRef", func(t *testing.T) { + origins := make(ComponentOriginMap) + pr := &processRef{ + name: "Test", + idx: nil, // nil idx + } + + captureOrigin(pr, "schemas", origins) + + assert.Equal(t, 0, len(origins), "should not add origin with nil idx") + }) +} + +func TestBundleDocumentComposed_StillWorks(t *testing.T) { + // verify the original BundleDocumentComposed function still works + tmpDir := t.TempDir() + + mainYAML := `openapi: 3.1.0 +info: + title: Test + version: 1.0.0 +paths: + /test: + get: + responses: + '200': + $ref: './response.yaml#/TestResponse'` + + responseYAML := `TestResponse: + description: Test response` + + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "main.yaml"), []byte(mainYAML), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "response.yaml"), []byte(responseYAML), 0644)) + + config := &datamodel.DocumentConfiguration{ + AllowFileReferences: true, + BasePath: tmpDir, + } + + mainBytes, err := os.ReadFile(filepath.Join(tmpDir, "main.yaml")) + require.NoError(t, err) + + doc, err := libopenapi.NewDocumentWithConfiguration(mainBytes, config) + require.NoError(t, err) + + v3Model, errs := doc.BuildV3Model() + require.NoError(t, errs) + + // original function should still work + bundledBytes, err := BundleDocumentComposed(&v3Model.Model, nil) + require.NoError(t, err) + assert.NotEmpty(t, bundledBytes) +} + +func TestBundleBytesComposed_StillWorks(t *testing.T) { + // verify backward compatibility + tmpDir := t.TempDir() + + mainYAML := `openapi: 3.1.0 +info: + title: Test + version: 1.0.0 +paths: + /test: + get: + responses: + '200': + $ref: './response.yaml#/TestResponse'` + + responseYAML := `TestResponse: + description: Test response` + + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "main.yaml"), []byte(mainYAML), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "response.yaml"), []byte(responseYAML), 0644)) + + config := &datamodel.DocumentConfiguration{ + AllowFileReferences: true, + BasePath: tmpDir, + } + + mainBytes, err := os.ReadFile(filepath.Join(tmpDir, "main.yaml")) + require.NoError(t, err) + + // original function should still work + bundledBytes, err := BundleBytesComposed(mainBytes, config, nil) + require.NoError(t, err) + assert.NotEmpty(t, bundledBytes) +} + +func TestBundleBytesComposedWithOrigins_InvalidDelimiter(t *testing.T) { + simpleSpec := `openapi: 3.1.0 +info: + title: Test + version: 1.0.0 +paths: + /test: + get: + responses: + '200': + description: OK` + + t.Run("rejects delimiter with hash", func(t *testing.T) { + config := &datamodel.DocumentConfiguration{} + compositionConfig := &BundleCompositionConfig{ + Delimiter: "#invalid", + } + + result, err := BundleBytesComposedWithOrigins([]byte(simpleSpec), config, compositionConfig) + + assert.Error(t, err) + assert.Contains(t, err.Error(), "cannot contain '#'") + assert.Nil(t, result) + }) + + t.Run("rejects delimiter with slash", func(t *testing.T) { + config := &datamodel.DocumentConfiguration{} + compositionConfig := &BundleCompositionConfig{ + Delimiter: "in/valid", + } + + result, err := BundleBytesComposedWithOrigins([]byte(simpleSpec), config, compositionConfig) + + assert.Error(t, err) + assert.Contains(t, err.Error(), "delimiter cannot contain") + assert.Nil(t, result) + }) + + t.Run("rejects delimiter with space", func(t *testing.T) { + config := &datamodel.DocumentConfiguration{} + compositionConfig := &BundleCompositionConfig{ + Delimiter: "in valid", + } + + result, err := BundleBytesComposedWithOrigins([]byte(simpleSpec), config, compositionConfig) + + assert.Error(t, err) + assert.Contains(t, err.Error(), "cannot contain spaces") + assert.Nil(t, result) + }) + + t.Run("uses default delimiter when empty", func(t *testing.T) { + tmpDir := t.TempDir() + + mainYAML := `openapi: 3.1.0 +info: + title: Test + version: 1.0.0 +paths: + /test: + get: + responses: + '200': + $ref: './response.yaml#/TestResponse'` + + responseYAML := `TestResponse: + description: Test response` + + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "main.yaml"), []byte(mainYAML), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "response.yaml"), []byte(responseYAML), 0644)) + + config := &datamodel.DocumentConfiguration{ + AllowFileReferences: true, + BasePath: tmpDir, + } + + mainBytes, err := os.ReadFile(filepath.Join(tmpDir, "main.yaml")) + require.NoError(t, err) + + // empty delimiter should use default "__" + compositionConfig := &BundleCompositionConfig{ + Delimiter: "", + } + + result, err := BundleBytesComposedWithOrigins(mainBytes, config, compositionConfig) + require.NoError(t, err) + assert.NotNil(t, result) + }) +} + +func TestBundleBytesComposedWithOrigins_NilModel(t *testing.T) { + // this tests an internal error condition that shouldn't happen in practice + // but we need to cover it for completeness + emptySpec := []byte("") + + config := &datamodel.DocumentConfiguration{} + result, err := BundleBytesComposedWithOrigins(emptySpec, config, nil) + + assert.Error(t, err) + assert.Nil(t, result) +} + +func TestBundleBytesComposedWithOrigins_AllComponentTypes(t *testing.T) { + // comprehensive test covering all component types to maximize coverage + tmpDir := t.TempDir() + + mainYAML := `openapi: 3.1.0 +info: + title: Comprehensive Test + version: 1.0.0 +paths: + /test: + $ref: './paths.yaml#/TestPath' +components: + schemas: + LocalSchema: + type: string` + + pathsYAML := `TestPath: + get: + parameters: + - $ref: './components.yaml#/TestParam' + responses: + '200': + $ref: './components.yaml#/TestResponse' + requestBody: + $ref: './components.yaml#/TestRequestBody' + callbacks: + testCallback: + $ref: './components.yaml#/TestCallback'` + + componentsYAML := `TestParam: + name: test + in: query + schema: + type: string +TestResponse: + description: Test response + headers: + X-Test: + $ref: '#/TestHeader' + links: + testLink: + $ref: '#/TestLink' + content: + application/json: + schema: + type: object + examples: + testExample: + $ref: '#/TestExample' +TestRequestBody: + description: Test request body + content: + application/json: + schema: + type: object +TestCallback: + '{$request.body#/callbackUrl}': + post: + responses: + '200': + description: Callback response +TestHeader: + description: Test header + schema: + type: string +TestLink: + operationId: testOp +TestExample: + value: test` + + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "main.yaml"), []byte(mainYAML), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "paths.yaml"), []byte(pathsYAML), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "components.yaml"), []byte(componentsYAML), 0644)) + + config := &datamodel.DocumentConfiguration{ + AllowFileReferences: true, + BasePath: tmpDir, + } + + mainBytes, err := os.ReadFile(filepath.Join(tmpDir, "main.yaml")) + require.NoError(t, err) + + result, err := BundleBytesComposedWithOrigins(mainBytes, config, nil) + // may have errors due to circular refs or other issues, but should still produce output + require.NotNil(t, result) + assert.NotEmpty(t, result.Bytes) + + // verify we tracked multiple component types + componentTypes := make(map[string]bool) + for _, origin := range result.Origins { + componentTypes[origin.ComponentType] = true + } + + t.Logf("Component types tracked: %v", componentTypes) + assert.Greater(t, len(componentTypes), 1, "should track multiple component types") +} + +func TestCaptureOrigin_FullCoverage(t *testing.T) { + t.Run("captures with empty location", func(t *testing.T) { + origins := make(ComponentOriginMap) + pr := &processRef{ + ref: &index.Reference{ + FullDefinition: "test.yaml", + Node: &yaml.Node{Line: 5, Column: 2}, + }, + idx: &index.SpecIndex{}, + originalName: "Test", + name: "Test", + location: []string{}, // empty location + } + + captureOrigin(pr, "schemas", origins) + + assert.Equal(t, 1, len(origins)) + // bundledRef is now built from pr.name and componentType, not pr.location + assert.Equal(t, "#/components/schemas/Test", origins["#/components/schemas/Test"].BundledRef) + }) + + t.Run("captures with complex location", func(t *testing.T) { + origins := make(ComponentOriginMap) + pr := &processRef{ + ref: &index.Reference{ + FullDefinition: "test.yaml#/components/schemas/ComplexName", + Node: &yaml.Node{Line: 10, Column: 4}, + }, + idx: &index.SpecIndex{}, + originalName: "ComplexName", + name: "ComplexName__2", + wasRenamed: true, + location: []string{"components", "schemas", "ComplexName__2"}, + } + + captureOrigin(pr, "schemas", origins) + + require.Equal(t, 1, len(origins)) + origin := origins["#/components/schemas/ComplexName__2"] + require.NotNil(t, origin) + assert.Equal(t, "ComplexName", origin.OriginalName) + assert.True(t, origin.WasRenamed) + assert.Equal(t, "#/components/schemas/ComplexName", origin.OriginalRef) + }) + + t.Run("handles full definition without fragment", func(t *testing.T) { + origins := make(ComponentOriginMap) + pr := &processRef{ + ref: &index.Reference{ + FullDefinition: "test.yaml", + Node: &yaml.Node{Line: 1, Column: 1}, + }, + idx: &index.SpecIndex{}, + originalName: "Root", + name: "Root", + location: []string{"components", "schemas", "Root"}, + } + + captureOrigin(pr, "schemas", origins) + + assert.Equal(t, 1, len(origins)) + origin := origins["#/components/schemas/Root"] + assert.Equal(t, "#/", origin.OriginalRef) + }) + + t.Run("falls back to pr.name when originalName is empty", func(t *testing.T) { + origins := make(ComponentOriginMap) + pr := &processRef{ + ref: &index.Reference{ + FullDefinition: "test.yaml#/components/schemas/Fallback", + Node: &yaml.Node{Line: 1, Column: 1}, + }, + idx: &index.SpecIndex{}, + originalName: "", // empty originalName triggers fallback + name: "Fallback", + location: []string{"components", "schemas", "Fallback"}, + } + + captureOrigin(pr, "schemas", origins) + + assert.Equal(t, 1, len(origins)) + origin := origins["#/components/schemas/Fallback"] + require.NotNil(t, origin) + assert.Equal(t, "Fallback", origin.OriginalName) // should fall back to pr.name + }) +} From 46036a242d82eb39c74c62098ab9580342600e39 Mon Sep 17 00:00:00 2001 From: quobix Date: Mon, 2 Feb 2026 17:45:24 -0500 Subject: [PATCH 5/6] More quirky rolodex issues squashed When using a custom file system, some relative paths were being doubled up in certain scenarios when files link backwards and forwards. Updated a lot of places where this can occurr as it appeared during stress testing of an upsrream feature using the rolodex with a custom file system. We now check for overlap consistently and resolve properly. --- bundler/bundler.go | 243 ++- bundler/bundler_composer.go | 231 ++- bundler/bundler_composer_test.go | 72 + bundler/bundler_ref_rewrite_test.go | 1969 +++++++++++++++++++++ bundler/bundler_strict_validation_test.go | 43 + bundler/bundler_test.go | 42 + bundler/composer_functions.go | 165 +- bundler/composer_functions_test.go | 434 ++--- bundler/detect_type.go | 3 + bundler/detect_type_test.go | 7 + bundler/origin_test.go | 15 + datamodel/low/base/schema_test.go | 3 + datamodel/low/extraction_functions.go | 15 +- index/rolodex.go | 2 +- index/rolodex_file_loader.go | 5 +- index/search_index.go | 7 +- utils/utils.go | 19 + utils/utils_test.go | 78 + utils/windows_drive.go | 41 +- utils/windows_drive_test.go | 108 ++ 20 files changed, 3168 insertions(+), 334 deletions(-) create mode 100644 bundler/bundler_ref_rewrite_test.go diff --git a/bundler/bundler.go b/bundler/bundler.go index d51f7a78..863ac085 100644 --- a/bundler/bundler.go +++ b/bundler/bundler.go @@ -8,6 +8,7 @@ import ( "context" "errors" "fmt" + "os" "path/filepath" "slices" "strings" @@ -74,12 +75,12 @@ func BundleBytesComposed(bytes []byte, configuration *datamodel.DocumentConfigur } v3Doc, err := doc.BuildV3Model() - if v3Doc == nil || err != nil { + if err != nil { return nil, errors.Join(ErrInvalidModel, err) } bundledBytes, e := compose(&v3Doc.Model, compositionConfig) - return bundledBytes, errors.Join(err, e) + return bundledBytes, e } // BundleBytesComposedWithOrigins returns a bundled spec with origin tracking for navigation. @@ -91,12 +92,12 @@ func BundleBytesComposedWithOrigins(bytes []byte, configuration *datamodel.Docum } v3Doc, err := doc.BuildV3Model() - if v3Doc == nil || err != nil { + if err != nil { return nil, errors.Join(ErrInvalidModel, err) } result, e := composeWithOrigins(&v3Doc.Model, compositionConfig) - return result, errors.Join(err, e) + return result, e } // BundleDocument will take a v3.Document and return a bundled version of it. @@ -138,19 +139,20 @@ func BundleDocumentWithConfig(model *v3.Document, bundleConfig *BundleInlineConf // BundleCompositionConfig is used to configure the composition of OpenAPI documents when using BundleDocumentComposed. type BundleCompositionConfig struct { - Delimiter string // Delimiter is used to separate clashing names. Defaults to `__`. - StrictValidation bool // StrictValidation will cause bundling to fail on invalid OpenAPI specs (e.g. $ref with siblings) + Delimiter string // Delimiter is used to separate clashing names. Defaults to `__`. + StrictValidation bool // StrictValidation will cause bundling to fail on invalid OpenAPI specs (e.g. $ref with siblings) } // BundleInlineConfig provides configuration options for inline bundling. // // Example usage: -// // Inline everything including local refs -// inlineTrue := true -// config := &BundleInlineConfig{ -// InlineLocalRefs: &inlineTrue, -// } -// bundled, err := BundleBytesWithConfig(specBytes, docConfig, config) +// +// // Inline everything including local refs +// inlineTrue := true +// config := &BundleInlineConfig{ +// InlineLocalRefs: &inlineTrue, +// } +// bundled, err := BundleBytesWithConfig(specBytes, docConfig, config) type BundleInlineConfig struct { // ResolveDiscriminatorExternalRefs when true, copies external schemas referenced // by discriminator mappings to the root document's components section. @@ -178,6 +180,15 @@ func BundleDocumentComposed(model *v3.Document, compositionConfig *BundleComposi return compose(model, compositionConfig) } +// BundleDocumentComposedWithOrigins will take a v3.Document and return a composed bundled version of it +// along with origin tracking information. This allows consumers to map bundled components back to their +// original file locations. The document model will be mutated permanently. +// +// Circular references will not be resolved and will be skipped. +func BundleDocumentComposedWithOrigins(model *v3.Document, compositionConfig *BundleCompositionConfig) (*BundleResult, error) { + return composeWithOrigins(model, compositionConfig) +} + // composeWithOrigins performs composed bundling and returns origin tracking information func composeWithOrigins(model *v3.Document, compositionConfig *BundleCompositionConfig) (*BundleResult, error) { if compositionConfig == nil { @@ -203,11 +214,14 @@ func composeWithOrigins(model *v3.Document, compositionConfig *BundleComposition rolodex := model.Rolodex indexes := rolodex.GetIndexes() + rootIndex := rolodex.GetRootIndex() - discriminatorMappings := collectDiscriminatorMappingNodes(rolodex) + // Step 1: Collect discriminator mappings WITH context (early) + discriminatorMappings := collectDiscriminatorMappingNodesWithContext(rolodex) cf := &handleIndexConfig{ - idx: rolodex.GetRootIndex(), + idx: rootIndex, + rootIdx: rootIndex, model: model, indexes: indexes, seen: sync.Map{}, @@ -216,9 +230,19 @@ func composeWithOrigins(model *v3.Document, compositionConfig *BundleComposition discriminatorMappings: discriminatorMappings, origins: make(ComponentOriginMap), } + + // Step 2: Enqueue mapping targets for composition (AFTER cf is created, BEFORE handleIndex) + // Pass rootIndex so we can skip root-local #/ refs + enqueueDiscriminatorMappingTargets(discriminatorMappings, cf, rootIndex) + // Refresh indexes in case mapping resolution loaded new ones + cf.indexes = rolodex.GetIndexes() + if err := handleIndex(cf); err != nil { return nil, err } + if err := handleDiscriminatorMappingIndexes(cf, rootIndex, rolodex); err != nil { + return nil, err + } processedNodes := orderedmap.New[string, *processRef]() var errs []error @@ -235,18 +259,26 @@ func composeWithOrigins(model *v3.Document, compositionConfig *BundleComposition return 0 }) - rootIndex := rolodex.GetRootIndex() + // Step 3: Remap indexed refs remapIndex(rootIndex, processedNodes) for _, idx := range indexes { remapIndex(idx, processedNodes) } + // Step 4: Update discriminator mappings (uses renameRef for collision handling) updateDiscriminatorMappingsComposed(discriminatorMappings, processedNodes, rolodex) + // Step 5: Inline handling with guard for synthetic discriminator refs // anything that could not be recomposed and needs inlining inlinedPaths := make(map[string]*yaml.Node) for _, pr := range cf.inlineRequired { + // Skip synthetic refs from discriminator mappings - their seqRef.Node is a + // scalar value node, not a $ref node with Content array + if pr.fromDiscriminator { + continue + } + if pr.refPointer != "" { // if the ref is a pointer to an external pointer, then we need to stitch it. @@ -255,7 +287,7 @@ func composeWithOrigins(model *v3.Document, compositionConfig *BundleComposition if uri[0] != "" { if !filepath.IsAbs(uri[0]) && !strings.HasPrefix(uri[0], "http") { // if the uri is not absolute, then we need to make it absolute. - uri[0] = filepath.Join(filepath.Dir(pr.idx.GetSpecAbsolutePath()), uri[0]) + uri[0] = utils.CheckPathOverlap(filepath.Dir(pr.idx.GetSpecAbsolutePath()), uri[0], string(os.PathSeparator)) } pointerRef := pr.idx.FindComponent(context.Background(), strings.Join(uri, "#/")) pr.seqRef.Node.Content = pointerRef.Node.Content @@ -274,9 +306,18 @@ func composeWithOrigins(model *v3.Document, compositionConfig *BundleComposition } } + // Step 6: Tree walk for any remaining unindexed refs + // Re-fetch indexes since new ones may have been loaded during composition + // (e.g., discriminator mapping targets that weren't indexed initially) + allLoadedIndexes := rolodex.GetIndexes() + rewriteAllRefs(rootIndex, processedNodes, rolodex) + for _, idx := range allLoadedIndexes { + rewriteAllRefs(idx, processedNodes, rolodex) + } + // Fix any remaining absolute path references that match inlined content // Also check the root index - allIndexes := append(indexes, rolodex.GetRootIndex()) + allIndexes := append(allLoadedIndexes, rolodex.GetRootIndex()) for _, idx := range allIndexes { for _, seqRef := range idx.GetRawReferencesSequenced() { if isRef, _, refVal := utils.IsNodeRefValue(seqRef.Node); isRef { @@ -330,11 +371,14 @@ func compose(model *v3.Document, compositionConfig *BundleCompositionConfig) ([] rolodex := model.Rolodex indexes := rolodex.GetIndexes() + rootIndex := rolodex.GetRootIndex() - discriminatorMappings := collectDiscriminatorMappingNodes(rolodex) + // Collect discriminator mappings WITH context (early) + discriminatorMappings := collectDiscriminatorMappingNodesWithContext(rolodex) cf := &handleIndexConfig{ - idx: rolodex.GetRootIndex(), + idx: rootIndex, + rootIdx: rootIndex, model: model, indexes: indexes, seen: sync.Map{}, @@ -343,9 +387,19 @@ func compose(model *v3.Document, compositionConfig *BundleCompositionConfig) ([] discriminatorMappings: discriminatorMappings, origins: make(ComponentOriginMap), } + + // Enqueue mapping targets for composition (AFTER cf is created, BEFORE handleIndex) + // Pass rootIndex so we can skip root-local #/ refs + enqueueDiscriminatorMappingTargets(discriminatorMappings, cf, rootIndex) + // Refresh indexes in case mapping resolution loaded new ones + cf.indexes = rolodex.GetIndexes() + if err := handleIndex(cf); err != nil { return nil, err } + if err := handleDiscriminatorMappingIndexes(cf, rootIndex, rolodex); err != nil { + return nil, err + } processedNodes := orderedmap.New[string, *processRef]() var errs []error @@ -362,18 +416,26 @@ func compose(model *v3.Document, compositionConfig *BundleCompositionConfig) ([] return 0 }) - rootIndex := rolodex.GetRootIndex() + // Remap indexed refs remapIndex(rootIndex, processedNodes) for _, idx := range indexes { remapIndex(idx, processedNodes) } + // Update discriminator mappings (uses renameRef for collision handling) updateDiscriminatorMappingsComposed(discriminatorMappings, processedNodes, rolodex) + // Inline handling with guard for synthetic discriminator refs // anything that could not be recomposed and needs inlining inlinedPaths := make(map[string]*yaml.Node) for _, pr := range cf.inlineRequired { + // Skip synthetic refs from discriminator mappings - their seqRef.Node is a + // scalar value node, not a $ref node with a Content array + if pr.fromDiscriminator { + continue + } + if pr.refPointer != "" { // if the ref is a pointer to an external pointer, then we need to stitch it. @@ -382,7 +444,7 @@ func compose(model *v3.Document, compositionConfig *BundleCompositionConfig) ([] if uri[0] != "" { if !filepath.IsAbs(uri[0]) && !strings.HasPrefix(uri[0], "http") { // if the uri is not absolute, then we need to make it absolute. - uri[0] = filepath.Join(filepath.Dir(pr.idx.GetSpecAbsolutePath()), uri[0]) + uri[0] = utils.CheckPathOverlap(filepath.Dir(pr.idx.GetSpecAbsolutePath()), uri[0], string(os.PathSeparator)) } pointerRef := pr.idx.FindComponent(context.Background(), strings.Join(uri, "#/")) pr.seqRef.Node.Content = pointerRef.Node.Content @@ -401,9 +463,18 @@ func compose(model *v3.Document, compositionConfig *BundleCompositionConfig) ([] } } + // Tree walk for any remaining unindexed refs + // Re-fetch indexes since new ones may have been loaded during composition + // (e.g., discriminator mapping targets that weren't indexed initially) + allLoadedIndexes := rolodex.GetIndexes() + rewriteAllRefs(rootIndex, processedNodes, rolodex) + for _, idx := range allLoadedIndexes { + rewriteAllRefs(idx, processedNodes, rolodex) + } + // Fix any remaining absolute path references that match inlined content // Also check the root index - allIndexes := append(indexes, rolodex.GetRootIndex()) + allIndexes := append(allLoadedIndexes, rolodex.GetRootIndex()) for _, idx := range allIndexes { for _, seqRef := range idx.GetRawReferencesSequenced() { if isRef, _, refVal := utils.IsNodeRefValue(seqRef.Node); isRef { @@ -474,11 +545,11 @@ func bundleWithConfig(model *v3.Document, config *BundleInlineConfig, docConfig // externalSchemaRef represents an external schema that needs to be copied to the root document's components. type externalSchemaRef struct { - idx *index.SpecIndex // Source index where the schema is defined - ref *index.Reference // The reference object - schemaName string // The target name in components - fullDef string // The full definition path (e.g., "/path/to/file.yaml#/components/schemas/Cat") - originalRef string // The original reference string (e.g., "#/components/schemas/Cat") + idx *index.SpecIndex // Source index where the schema is defined + ref *index.Reference // The reference object + schemaName string // The target name in components + fullDef string // The full definition path (e.g., "/path/to/file.yaml#/components/schemas/Cat") + originalRef string // The original reference string (e.g., "#/components/schemas/Cat") } // resolveDiscriminatorExternalRefs handles copying external schemas referenced by discriminators @@ -846,6 +917,63 @@ func collectDiscriminatorMappingNodes(rolodex *index.Rolodex) []*yaml.Node { return mappingNodes } +// collectDiscriminatorMappingNodesWithContext gathers all discriminator mapping value nodes +// along with their source index context for proper relative path resolution. +func collectDiscriminatorMappingNodesWithContext(rolodex *index.Rolodex) []*discriminatorMappingWithContext { + var mappings []*discriminatorMappingWithContext + + collectDiscriminatorMappingNodesFromIndexWithContext(rolodex.GetRootIndex(), rolodex.GetRootIndex().GetRootNode(), &mappings) + for _, idx := range rolodex.GetIndexes() { + collectDiscriminatorMappingNodesFromIndexWithContext(idx, idx.GetRootNode(), &mappings) + } + + return mappings +} + +// collectDiscriminatorMappingNodesFromIndexWithContext recursively walks a YAML node tree +// to find discriminator mapping nodes, preserving the source index context. +func collectDiscriminatorMappingNodesFromIndexWithContext(idx *index.SpecIndex, n *yaml.Node, mappings *[]*discriminatorMappingWithContext) { + if n.Kind == yaml.DocumentNode && len(n.Content) > 0 { + n = n.Content[0] + } + + switch n.Kind { + case yaml.SequenceNode: + for _, c := range n.Content { + collectDiscriminatorMappingNodesFromIndexWithContext(idx, c, mappings) + } + return + case yaml.MappingNode: + default: + return + } + + var discriminator *yaml.Node + + for i := 0; i < len(n.Content); i += 2 { + k, v := n.Content[i], n.Content[i+1] + switch k.Value { + case "discriminator": + discriminator = v + } + collectDiscriminatorMappingNodesFromIndexWithContext(idx, v, mappings) + } + + if discriminator != nil && discriminator.Kind == yaml.MappingNode { + for i := 0; i < len(discriminator.Content); i += 2 { + if discriminator.Content[i].Value == "mapping" { + mappingNode := discriminator.Content[i+1] + for j := 0; j < len(mappingNode.Content); j += 2 { + *mappings = append(*mappings, &discriminatorMappingWithContext{ + node: mappingNode.Content[j+1], + sourceIdx: idx, + }) + } + } + } + } +} + // collectDiscriminatorMappingNodesFromIndex recursively walks a YAML node tree to find discriminator mapping nodes. func collectDiscriminatorMappingNodesFromIndex(idx *index.SpecIndex, n *yaml.Node, mappingNodes *[]*yaml.Node) { if n.Kind == yaml.DocumentNode && len(n.Content) > 0 { @@ -887,34 +1015,53 @@ func collectDiscriminatorMappingNodesFromIndex(idx *index.SpecIndex, n *yaml.Nod } // updateDiscriminatorMappingsComposed updates discriminator mapping references to point to composed component locations. -func updateDiscriminatorMappingsComposed(mappingNodes []*yaml.Node, processedNodes *orderedmap.Map[string, *processRef], rolodex *index.Rolodex) { - for _, mappingNode := range mappingNodes { - originalValue := mappingNode.Value - - if !strings.Contains(originalValue, "#/") { +func updateDiscriminatorMappingsComposed(mappings []*discriminatorMappingWithContext, processedNodes *orderedmap.Map[string, *processRef], rolodex *index.Rolodex) { + for _, mapping := range mappings { + originalValue := mapping.node.Value + if originalValue == "" { continue } - var matchingIdx *index.SpecIndex + // Skip external URLs and URNs - they should never be rewritten + if strings.HasPrefix(originalValue, "http://") || + strings.HasPrefix(originalValue, "https://") || + strings.HasPrefix(originalValue, "urn:") { + continue + } - // Search root index first - if ref, refIdx := rolodex.GetRootIndex().SearchIndexForReference(originalValue); ref != nil { - matchingIdx = refIdx - } else { - // Search all other indexes - for _, idx := range rolodex.GetIndexes() { - if ref, refIdx := idx.SearchIndexForReference(originalValue); ref != nil { - matchingIdx = refIdx - break - } + // Use the cached canonicalKey and targetIdx from enqueue time. + // At enqueue time, we captured ref.FullDefinition BEFORE any mutation. + // Using SearchIndexForReference again here would return a potentially mutated + // ref.FullDefinition that won't match processedNodes keys. + canonicalKey := mapping.canonicalKey + targetIdx := mapping.targetIdx + + // If canonicalKey is empty, the mapping wasn't resolved during enqueue. + // Try to resolve it now as a fallback. + if canonicalKey == "" { + ref, refIdx := mapping.sourceIdx.SearchIndexForReference(originalValue) + if ref == nil { + ref, refIdx = rolodex.GetRootIndex().SearchIndexForReference(originalValue) + } + if ref == nil || refIdx == nil { + continue } + canonicalKey = ref.FullDefinition + targetIdx = refIdx // Use the resolved index, NOT mapping.sourceIdx } - if matchingIdx != nil { - newRef := renameRef(matchingIdx, originalValue, processedNodes) - if newRef != originalValue { - mappingNode.Value = newRef - } + // Gate rewrites on processedNodes presence. + // Only rewrite if the target was actually composed into the bundled output. + // This prevents dangling refs when SearchIndexForReference resolves something + // that never made it into processedNodes (e.g., unprocessed transitive refs). + if processedNodes.GetOrZero(canonicalKey) == nil { + continue + } + + // Use targetIdx (where the ref actually lives), NOT sourceIdx + newRef := renameRef(targetIdx, canonicalKey, processedNodes) + if newRef != originalValue { + mapping.node.Value = newRef } } } diff --git a/bundler/bundler_composer.go b/bundler/bundler_composer.go index 65f767a1..edc8ce61 100644 --- a/bundler/bundler_composer.go +++ b/bundler/bundler_composer.go @@ -7,6 +7,7 @@ import ( "context" "fmt" "net/url" + "path/filepath" "strings" "sync" @@ -14,30 +15,42 @@ import ( v3low "github.com/pb33f/libopenapi/datamodel/low/v3" "github.com/pb33f/libopenapi/index" "github.com/pb33f/libopenapi/orderedmap" + "github.com/pb33f/libopenapi/utils" "go.yaml.in/yaml/v4" ) type processRef struct { - idx *index.SpecIndex - ref *index.Reference - seqRef *index.Reference - refPointer string - name string - location []string - wasRenamed bool // true when component was renamed due to collision - originalName string // original name before collision renaming + idx *index.SpecIndex + ref *index.Reference + seqRef *index.Reference + refPointer string + name string + location []string + wasRenamed bool // true when component was renamed due to collision + originalName string // original name before collision renaming + fromDiscriminator bool // true if created from discriminator mapping (skip inline handling) +} + +// discriminatorMappingWithContext stores a mapping node with its source index +// and the pre-computed canonical key for processedNodes lookup +type discriminatorMappingWithContext struct { + node *yaml.Node // The YAML node containing the mapping value + sourceIdx *index.SpecIndex // The index where the mapping was found + canonicalKey string // Pre-computed key: ref.FullDefinition at enqueue time (before any mutation) + targetIdx *index.SpecIndex // The index where the resolved ref actually lives (may differ from sourceIdx) } type handleIndexConfig struct { idx *index.SpecIndex + rootIdx *index.SpecIndex model *v3.Document indexes []*index.SpecIndex refMap *orderedmap.Map[string, *processRef] seen sync.Map inlineRequired []*processRef compositionConfig *BundleCompositionConfig - discriminatorMappings []*yaml.Node - origins ComponentOriginMap // component origins for navigation + discriminatorMappings []*discriminatorMappingWithContext // mapping nodes with source context + origins ComponentOriginMap // component origins for navigation } // handleIndex will recursively explore the indexes and their references, building a map of references @@ -53,8 +66,8 @@ func handleIndex(c *handleIndexConfig) error { // Check for invalid sibling properties if strict validation is enabled if c.compositionConfig.StrictValidation && - c.idx.GetConfig().SpecInfo.VersionNumeric == 3.0 && - sequenced.HasSiblingProperties { + c.idx.GetConfig().SpecInfo.VersionNumeric == 3.0 && + sequenced.HasSiblingProperties { siblingKeys := make([]string, 0, len(sequenced.SiblingProperties)) for key := range sequenced.SiblingProperties { siblingKeys = append(siblingKeys, key) @@ -89,6 +102,10 @@ func handleIndex(c *handleIndexConfig) error { continue } if foundIndex != nil && mappedReference != nil { + // Avoid recomposing components that resolve back to the root document. + if c.rootIdx != nil && foundIndex.GetSpecAbsolutePath() == c.rootIdx.GetSpecAbsolutePath() { + continue + } // store the reference to be composed in the root. if kk := c.refMap.GetOrZero(mappedReference.FullDefinition); kk == nil { c.refMap.Set(mappedReference.FullDefinition, &processRef{ @@ -118,16 +135,16 @@ func handleIndex(c *handleIndexConfig) error { // recomposed as components. OpenAPI root keys are always lowercase per spec. // Package-level to avoid allocation on each call. var openAPIRootKeys = map[string]bool{ - "openapi": true, - "info": true, + "openapi": true, + "info": true, "jsonSchemaDialect": true, - "servers": true, - "paths": true, - "webhooks": true, - "components": true, - "security": true, - "tags": true, - "externalDocs": true, + "servers": true, + "paths": true, + "webhooks": true, + "components": true, + "security": true, + "tags": true, + "externalDocs": true, } // isOpenAPIRootKey returns true if the key is a known OpenAPI root-level key @@ -355,3 +372,175 @@ func processReference(model *v3.Document, pr *processRef, cf *handleIndexConfig) } return nil } + +// enqueueDiscriminatorMappingTargets ensures mapping targets are composed into components. +// This handles cases where a schema is ONLY referenced via discriminator mapping. +func enqueueDiscriminatorMappingTargets( + mappings []*discriminatorMappingWithContext, + cf *handleIndexConfig, + rootIdx *index.SpecIndex, +) { + for _, mapping := range mappings { + refValue := mapping.node.Value + + // Skip empty values + if refValue == "" { + continue + } + + // Skip external URLs and URNs - they're not local refs to compose + if strings.HasPrefix(refValue, "http://") || + strings.HasPrefix(refValue, "https://") || + strings.HasPrefix(refValue, "urn:") { + continue + } + + // Only skip #/ refs if we're in the ROOT index. + // In external files, #/components/... refers to THAT file's components, + // which must still be composed into the root document. + if strings.HasPrefix(refValue, "#/") && mapping.sourceIdx == rootIdx { + continue + } + + // Resolve using source index context + ref, foundIdx := mapping.sourceIdx.SearchIndexForReference(refValue) + if ref == nil { + ref, foundIdx = resolveDiscriminatorMappingTarget(mapping.sourceIdx, refValue) + } + if ref == nil { + continue // Can't resolve - will be caught later + } + + // Cache the canonical key and target index NOW, before any mutation can occur. + // This key will be used later in updateDiscriminatorMappingsComposed + // to correctly look up the processedNodes entry. + // The targetIdx may differ from sourceIdx if the ref resolves to a different file. + mapping.canonicalKey = ref.FullDefinition + mapping.targetIdx = foundIdx + + // Skip if already in refMap (avoid duplicates) + if cf.refMap.GetOrZero(ref.FullDefinition) != nil { + continue + } + + // Derive name from reference - use ref.Name if set, otherwise extract from FullDefinition + name := ref.Name + if name == "" { + name = deriveNameFromFullDefinition(ref.FullDefinition) + } + + pr := &processRef{ + ref: ref, + seqRef: ref, + idx: foundIdx, + name: name, + fromDiscriminator: true, // NEW FLAG: prevents inline handling + } + cf.refMap.Set(ref.FullDefinition, pr) + } +} + +// resolveDiscriminatorMappingTarget attempts to resolve a mapping value as a whole-file reference. +// This is a fallback for cases where SearchIndexForReference returns nil for bare file refs. +func resolveDiscriminatorMappingTarget( + sourceIdx *index.SpecIndex, + refValue string, +) (*index.Reference, *index.SpecIndex) { + if sourceIdx == nil { + return nil, nil + } + + rolodex := sourceIdx.GetRolodex() + if rolodex == nil { + return nil, nil + } + + absPath := refValue + if !filepath.IsAbs(absPath) && !strings.HasPrefix(absPath, "http") { + base := sourceIdx.GetSpecAbsolutePath() + if base == "" { + if cfg := sourceIdx.GetConfig(); cfg != nil { + base = cfg.BasePath + } + } + if base != "" && filepath.Ext(base) != "" { + base = filepath.Dir(base) + } + if base != "" { + if p, err := filepath.Abs(utils.CheckPathOverlap(base, refValue, string(filepath.Separator))); err == nil { + absPath = p + } + } + } + + rFile, err := rolodex.OpenWithContext(context.Background(), absPath) + if err != nil || rFile == nil { + return nil, nil + } + + if rFile.GetIndex() == nil { + if cfg := sourceIdx.GetConfig(); cfg != nil { + if idxFile, ok := rFile.(index.CanBeIndexed); ok { + _, _ = idxFile.Index(cfg) + } + } + } + + idx := rFile.GetIndex() + node, _ := rFile.GetContentAsYAMLNode() + if node != nil && node.Kind == yaml.DocumentNode && len(node.Content) > 0 { + node = node.Content[0] + } + + ref := &index.Reference{ + FullDefinition: absPath, + Definition: absPath, + Name: filepath.Base(absPath), + Index: idx, + Node: node, + IsRemote: true, + RemoteLocation: absPath, + } + + return ref, idx +} + +// handleDiscriminatorMappingIndexes ensures indexes discovered only via discriminator mappings +// are explored so their internal refs are composed. +func handleDiscriminatorMappingIndexes( + cf *handleIndexConfig, + rootIdx *index.SpecIndex, + rolodex *index.Rolodex, +) error { + for _, mapping := range cf.discriminatorMappings { + if mapping.targetIdx == nil { + continue + } + if mapping.targetIdx == rootIdx { + continue + } + if _, ok := cf.seen.Load(mapping.targetIdx.GetSpecAbsolutePath()); ok { + continue + } + + // Refresh indexes in case new ones were loaded during mapping resolution. + if rolodex != nil { + cf.indexes = rolodex.GetIndexes() + } + + cf.idx = mapping.targetIdx + if err := handleIndex(cf); err != nil { + return err + } + } + cf.idx = rootIdx + return nil +} + +func deriveNameFromFullDefinition(fullDef string) string { + if idx := strings.Index(fullDef, "#"); idx != -1 { + fullDef = fullDef[:idx] + } + baseName := filepath.Base(fullDef) + return strings.TrimSuffix(baseName, filepath.Ext(baseName)) +} diff --git a/bundler/bundler_composer_test.go b/bundler/bundler_composer_test.go index 0c5386fa..1d9e942e 100644 --- a/bundler/bundler_composer_test.go +++ b/bundler/bundler_composer_test.go @@ -17,6 +17,7 @@ import ( "github.com/pb33f/libopenapi/datamodel" v3 "github.com/pb33f/libopenapi/datamodel/high/v3" "github.com/pb33f/libopenapi/index" + "github.com/pb33f/libopenapi/orderedmap" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.yaml.in/yaml/v4" @@ -151,6 +152,77 @@ func TestBundlerComposed_StrangeRefs(t *testing.T) { } } +func TestEnqueueDiscriminatorMappingTargets_StripsFragmentWhenNameMissing(t *testing.T) { + tmpDir := t.TempDir() + + rootSpec := `openapi: 3.1.0 +info: + title: Enqueue Mapping Fragment + version: 1.0.0 +paths: {} +components: + schemas: + Animal: + type: object + discriminator: + propertyName: kind + mapping: + cat: './schemas/Cat.yaml#/components/schemas/Cat'` + + catSpec := `components: + schemas: + Cat: + type: object` + + require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, "schemas"), 0755)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "root.yaml"), []byte(rootSpec), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "schemas", "Cat.yaml"), []byte(catSpec), 0644)) + + specBytes, err := os.ReadFile(filepath.Join(tmpDir, "root.yaml")) + require.NoError(t, err) + + cfg := datamodel.NewDocumentConfiguration() + cfg.BasePath = tmpDir + cfg.AllowFileReferences = true + cfg.SpecFilePath = "root.yaml" + + doc, err := libopenapi.NewDocumentWithConfiguration(specBytes, cfg) + require.NoError(t, err) + + v3Doc, err := doc.BuildV3Model() + require.NoError(t, err) + require.NotNil(t, v3Doc) + + rolodex := v3Doc.Index.GetRolodex() + mappings := collectDiscriminatorMappingNodesWithContext(rolodex) + require.NotEmpty(t, mappings) + + refValue := mappings[0].node.Value + ref, _ := mappings[0].sourceIdx.SearchIndexForReference(refValue) + require.NotNil(t, ref) + + // Force name extraction logic by clearing the name. + ref.Name = "" + if !strings.Contains(ref.FullDefinition, "#") { + ref.FullDefinition = ref.FullDefinition + "#/components/schemas/Cat" + } + + cf := &handleIndexConfig{ + refMap: orderedmap.New[string, *processRef](), + } + + enqueueDiscriminatorMappingTargets(mappings, cf, rolodex.GetRootIndex()) + + pr := cf.refMap.GetOrZero(ref.FullDefinition) + require.NotNil(t, pr) + assert.Equal(t, "Cat", pr.name) +} + +func TestDeriveNameFromFullDefinition_StripsFragment(t *testing.T) { + name := deriveNameFromFullDefinition("/tmp/path/Cat.yaml#/components/schemas/Cat") + assert.Equal(t, "Cat", name) +} + // TestBundleBytesComposed_DiscriminatorMapping tests that composed bundling correctly // updates discriminator mappings when external schemas are moved to components. func TestBundleBytesComposed_DiscriminatorMapping(t *testing.T) { diff --git a/bundler/bundler_ref_rewrite_test.go b/bundler/bundler_ref_rewrite_test.go new file mode 100644 index 00000000..72a30bf8 --- /dev/null +++ b/bundler/bundler_ref_rewrite_test.go @@ -0,0 +1,1969 @@ +// Copyright 2023-2025 Princess Beef Heavy Industries, LLC / Dave Shanley +// https://pb33f.io +// SPDX-License-Identifier: MIT + +package bundler + +import ( + "os" + "path/filepath" + "regexp" + "strings" + "testing" + "testing/fstest" + + "github.com/pb33f/libopenapi/datamodel" + "github.com/pb33f/libopenapi/index" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// assertNoFilePathRefs checks that no file-path refs remain in the bundled output +// (excludes http(s)://, urn: which are legitimate external refs) +func assertNoFilePathRefs(t *testing.T, yamlBytes []byte) { + t.Helper() + content := string(yamlBytes) + + // Find all $ref values + refPattern := regexp.MustCompile(`\$ref:\s*['"]?([^'"}\s\n]+)['"]?`) + matches := refPattern.FindAllStringSubmatch(content, -1) + + for _, match := range matches { + if len(match) < 2 { + continue + } + refValue := match[1] + + // Skip legitimate external refs + if strings.HasPrefix(refValue, "http://") || + strings.HasPrefix(refValue, "https://") || + strings.HasPrefix(refValue, "urn:") { + continue + } + + // Flag file-path patterns that should have been rewritten + if strings.HasPrefix(refValue, "./") || + strings.HasPrefix(refValue, "../") || + strings.Contains(refValue, ".yaml") || + strings.Contains(refValue, ".yml") || + strings.Contains(refValue, ".json") { + // But exclude URLs that happen to contain .yaml/.json + if !strings.Contains(refValue, "://") { + t.Errorf("Found unrewritten file-path ref: %s", refValue) + } + } + } +} + +// TestBundlerComposed_TransitiveExternalRefs verifies that transitive external refs +// (main.yaml -> external.yaml -> definitions.yaml#/SomeSchema) are properly stitched. +// This covers the external pointer stitching code in compose() and composeWithOrigins(). +func TestBundlerComposed_TransitiveExternalRefs(t *testing.T) { + tmpDir := t.TempDir() + + // Root spec references external.yaml which has a pathItem + mainSpec := `openapi: 3.1.0 +info: + title: Transitive Test + version: 1.0.0 +paths: + /test: + $ref: './external.yaml'` + + // External pathItem references definitions.yaml for its schema + externalSpec := `get: + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: './definitions.yaml#/components/schemas/Item'` + + // Definitions file with schemas + definitionsSpec := `components: + schemas: + Item: + type: object + properties: + id: + type: string + nested: + $ref: '#/components/schemas/Nested' + Nested: + type: object + properties: + value: + type: integer` + + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "main.yaml"), []byte(mainSpec), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "external.yaml"), []byte(externalSpec), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "definitions.yaml"), []byte(definitionsSpec), 0644)) + + mainBytes, err := os.ReadFile(filepath.Join(tmpDir, "main.yaml")) + require.NoError(t, err) + + config := datamodel.NewDocumentConfiguration() + config.BasePath = tmpDir + + bundled, err := BundleBytesComposed(mainBytes, config, nil) + require.NoError(t, err) + + bundledStr := string(bundled) + t.Logf("Bundled output:\n%s", bundledStr) + + // Verify transitive ref is resolved - Item schema should be composed + assert.Contains(t, bundledStr, "Item", "Item schema should be composed from definitions.yaml") + assert.Contains(t, bundledStr, "Nested", "Nested schema should be composed from definitions.yaml") + assert.Contains(t, bundledStr, "integer", "Nested value type should be present") + + // Verify no file paths remain + assertNoFilePathRefs(t, bundled) +} + +// TestBundlerComposedWithOrigins_TransitiveExternalRefs verifies transitive refs with origin tracking. +// When schemas use non-standard paths like #/definitions/..., they get inlined rather than composed. +func TestBundlerComposedWithOrigins_TransitiveExternalRefs(t *testing.T) { + tmpDir := t.TempDir() + + mainSpec := `openapi: 3.1.0 +info: + title: Transitive With Origins Test + version: 1.0.0 +paths: + /items: + $ref: './paths/items.yaml'` + + // Path file references schema in another directory + pathsItems := `get: + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: '../schemas/Item.yaml#/definitions/ItemModel'` + + // Schema file with definitions section (not components) + schemaItem := `definitions: + ItemModel: + type: object + properties: + name: + type: string + data: + $ref: '#/definitions/ItemData' + ItemData: + type: object + properties: + value: + type: number` + + require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, "paths"), 0755)) + require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, "schemas"), 0755)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "main.yaml"), []byte(mainSpec), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "paths", "items.yaml"), []byte(pathsItems), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "schemas", "Item.yaml"), []byte(schemaItem), 0644)) + + mainBytes, err := os.ReadFile(filepath.Join(tmpDir, "main.yaml")) + require.NoError(t, err) + + config := datamodel.NewDocumentConfiguration() + config.BasePath = tmpDir + + result, err := BundleBytesComposedWithOrigins(mainBytes, config, nil) + require.NoError(t, err) + require.NotNil(t, result) + + bundledStr := string(result.Bytes) + t.Logf("Bundled output:\n%s", bundledStr) + + // Verify transitive refs resolved - content should be inlined since it uses #/definitions/ + // (non-standard path that can't be composed) + assert.Contains(t, bundledStr, "name:", "ItemModel properties should be inlined") + assert.Contains(t, bundledStr, "type: number", "ItemData properties should be inlined") + + // Verify no file paths remain + assertNoFilePathRefs(t, result.Bytes) +} + +// TestBundlerComposedWithOrigins_InlineRequiredWithRefPointer verifies that composeWithOrigins +// properly handles inline-required refs that have external pointer references. +// This exercises the external pointer stitching code in composeWithOrigins() (lines 266-284). +func TestBundlerComposedWithOrigins_InlineRequiredWithRefPointer(t *testing.T) { + tmpDir := t.TempDir() + + // Root spec with a ref to external schema that has a ref to another file + mainSpec := `openapi: 3.1.0 +info: + title: Inline Pointer Test + version: 1.0.0 +paths: + /data: + get: + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: './schemas/Data.yaml#/definitions/DataObject'` + + // External schema with definitions (non-standard location that triggers inline) + dataSchema := `definitions: + DataObject: + type: object + properties: + nested: + $ref: './nested.yaml#/definitions/NestedObject'` + + // Another external file + nestedSchema := `definitions: + NestedObject: + type: object + properties: + value: + type: string` + + require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, "schemas"), 0755)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "main.yaml"), []byte(mainSpec), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "schemas", "Data.yaml"), []byte(dataSchema), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "schemas", "nested.yaml"), []byte(nestedSchema), 0644)) + + mainBytes, err := os.ReadFile(filepath.Join(tmpDir, "main.yaml")) + require.NoError(t, err) + + config := datamodel.NewDocumentConfiguration() + config.BasePath = tmpDir + + // Use WithOrigins variant to exercise composeWithOrigins code path + result, err := BundleBytesComposedWithOrigins(mainBytes, config, nil) + require.NoError(t, err) + require.NotNil(t, result) + + bundledStr := string(result.Bytes) + t.Logf("Bundled output:\n%s", bundledStr) + + // Verify content was inlined/resolved + assert.Contains(t, bundledStr, "type: object", "Object definition should be present") + assert.Contains(t, bundledStr, "value:", "Nested value property should be present") + + // Verify no file paths remain + assertNoFilePathRefs(t, result.Bytes) +} + +// TestBundlerComposedWithOrigins_InlineRequiredRefPointerChain verifies that inline-required refs +// which themselves point at external refs are stitched into the final output. +func TestBundlerComposedWithOrigins_InlineRequiredRefPointerChain(t *testing.T) { + tmpDir := t.TempDir() + + mainSpec := `openapi: 3.1.0 +info: + title: Inline Pointer Chain + version: 1.0.0 +paths: + /data: + get: + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: './schemas/Data.yaml#/definitions/DataObject'` + + // DataObject is a $ref to another file, which should trigger refPointer stitching. + dataSchema := `definitions: + DataObject: + $ref: './nested.yaml#/definitions/NestedObject'` + + nestedSchema := `definitions: + NestedObject: + type: object + properties: + value: + type: string` + + require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, "schemas"), 0755)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "main.yaml"), []byte(mainSpec), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "schemas", "Data.yaml"), []byte(dataSchema), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "schemas", "nested.yaml"), []byte(nestedSchema), 0644)) + + mainBytes, err := os.ReadFile(filepath.Join(tmpDir, "main.yaml")) + require.NoError(t, err) + + config := datamodel.NewDocumentConfiguration() + config.BasePath = tmpDir + + result, err := BundleBytesComposedWithOrigins(mainBytes, config, nil) + require.NoError(t, err) + require.NotNil(t, result) + + bundledStr := string(result.Bytes) + t.Logf("Bundled output:\n%s", bundledStr) + + assert.Contains(t, bundledStr, "value:", "Nested properties should be present") + assertNoFilePathRefs(t, result.Bytes) +} + +// TestBundlerComposedWithOrigins_AbsolutePathRefReuse ensures absolute-path refs +// that point at inline-required content are replaced with the inlined node content. +func TestBundlerComposedWithOrigins_AbsolutePathRefReuse(t *testing.T) { + tmpDir := t.TempDir() + + require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, "schemas"), 0755)) + + schemaPath := filepath.Join(tmpDir, "schemas", "Shared.yaml") + mainSpec := `openapi: 3.1.0 +info: + title: Absolute Ref Reuse + version: 1.0.0 +paths: + /one: + get: + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: './schemas/Shared.yaml' + /two: + get: + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: './schemas/Shared.yaml'` + + sharedSchema := `openapi: 3.1.0 +info: + title: External Spec + version: 1.0.0 +paths: {}` + + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "main.yaml"), []byte(mainSpec), 0644)) + require.NoError(t, os.WriteFile(schemaPath, []byte(sharedSchema), 0644)) + + mainBytes, err := os.ReadFile(filepath.Join(tmpDir, "main.yaml")) + require.NoError(t, err) + + absSchemaPath, err := filepath.Abs(schemaPath) + require.NoError(t, err) + + config := datamodel.NewDocumentConfiguration() + config.BasePath = tmpDir + config.AllowFileReferences = true + + result, err := BundleBytesComposedWithOrigins(mainBytes, config, nil) + require.NoError(t, err) + require.NotNil(t, result) + + bundledStr := string(result.Bytes) + t.Logf("Bundled output:\n%s", bundledStr) + + assert.Contains(t, bundledStr, "External Spec", "Inlined content should be present") + assert.NotContains(t, bundledStr, absSchemaPath, "Absolute path refs should be replaced") +} + +// TestBundlerComposed_DiscriminatorUnknownTargetSkipped verifies that non-composable +// discriminator mapping targets do not get inlined during composed bundling. +func TestBundlerComposed_DiscriminatorUnknownTargetSkipped(t *testing.T) { + tmpDir := t.TempDir() + + mainSpec := `openapi: 3.1.0 +info: + title: Discriminator Unknown + version: 1.0.0 +paths: {} +components: + schemas: + Item: + type: object + discriminator: + propertyName: kind + mapping: + weird: './schemas/Weird.yaml' + properties: + kind: + type: string` + + unknownSchema := `not: + a: schema` + + require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, "schemas"), 0755)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "main.yaml"), []byte(mainSpec), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "schemas", "Weird.yaml"), []byte(unknownSchema), 0644)) + + mainBytes, err := os.ReadFile(filepath.Join(tmpDir, "main.yaml")) + require.NoError(t, err) + + config := datamodel.NewDocumentConfiguration() + config.BasePath = tmpDir + config.AllowFileReferences = true + + bundled, err := BundleBytesComposed(mainBytes, config, nil) + require.NoError(t, err) + + bundledStr := string(bundled) + assert.Contains(t, bundledStr, "Weird.yaml", "Non-composable mapping target should remain a file ref") +} + +// TestBundlerComposedWithOrigins_DiscriminatorUnknownTargetSkipped verifies the same behavior +// when using the WithOrigins variant. +func TestBundlerComposedWithOrigins_DiscriminatorUnknownTargetSkipped(t *testing.T) { + tmpDir := t.TempDir() + + mainSpec := `openapi: 3.1.0 +info: + title: Discriminator Unknown Origins + version: 1.0.0 +paths: {} +components: + schemas: + Item: + type: object + discriminator: + propertyName: kind + mapping: + weird: './schemas/Weird.yaml' + properties: + kind: + type: string` + + unknownSchema := `not: + a: schema` + + require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, "schemas"), 0755)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "main.yaml"), []byte(mainSpec), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "schemas", "Weird.yaml"), []byte(unknownSchema), 0644)) + + mainBytes, err := os.ReadFile(filepath.Join(tmpDir, "main.yaml")) + require.NoError(t, err) + + config := datamodel.NewDocumentConfiguration() + config.BasePath = tmpDir + config.AllowFileReferences = true + + result, err := BundleBytesComposedWithOrigins(mainBytes, config, nil) + require.NoError(t, err) + require.NotNil(t, result) + + bundledStr := string(result.Bytes) + assert.Contains(t, bundledStr, "Weird.yaml", "Non-composable mapping target should remain a file ref") +} + +// TestBundlerComposedWithOrigins_DiscriminatorWithInlineRequired verifies that +// discriminator mappings that trigger the inlineRequired path are properly handled. +func TestBundlerComposedWithOrigins_DiscriminatorWithInlineRequired(t *testing.T) { + tmpDir := t.TempDir() + + // Root spec with discriminator mapping to external file + mainSpec := `openapi: 3.1.0 +info: + title: Discriminator Inline Test + version: 1.0.0 +paths: + /items: + get: + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: '#/components/schemas/Item' +components: + schemas: + Item: + type: object + discriminator: + propertyName: itemType + mapping: + special: './schemas/Special.yaml' + properties: + itemType: + type: string` + + specialSchema := `type: object +properties: + specialField: + type: string` + + require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, "schemas"), 0755)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "main.yaml"), []byte(mainSpec), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "schemas", "Special.yaml"), []byte(specialSchema), 0644)) + + mainBytes, err := os.ReadFile(filepath.Join(tmpDir, "main.yaml")) + require.NoError(t, err) + + config := datamodel.NewDocumentConfiguration() + config.BasePath = tmpDir + + // Use WithOrigins to exercise composeWithOrigins + result, err := BundleBytesComposedWithOrigins(mainBytes, config, nil) + require.NoError(t, err) + require.NotNil(t, result) + + bundledStr := string(result.Bytes) + t.Logf("Bundled output:\n%s", bundledStr) + + // Verify Special schema was composed + assert.Contains(t, bundledStr, "Special", "Special schema should be composed") + assert.Contains(t, bundledStr, "specialField", "Special properties should be present") + + // Verify no file paths remain + assertNoFilePathRefs(t, result.Bytes) + assertNoFilePathMappings(t, result.Bytes) +} + +// TestBundlerComposed_DiscriminatorMappingWithFragmentExtractsName verifies that +// discriminator mapping targets with a bare file reference (e.g., './Cat.yaml') +// correctly extract the name from the path. +// This tests that the name extraction code works when ref.Name is empty. +func TestBundlerComposed_DiscriminatorMappingWithFragmentExtractsName(t *testing.T) { + tmpDir := t.TempDir() + + // Root spec with discriminator mapping using bare file refs (no #/...) + // These bare file refs will have empty ref.Name, triggering name extraction from path + rootSpec := `openapi: 3.1.0 +info: + title: Fragment Name Test + version: 1.0.0 +paths: + /items: + get: + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: '#/components/schemas/Item' +components: + schemas: + Item: + type: object + discriminator: + propertyName: itemType + mapping: + # Bare file refs - ref.Name will be empty, name extracted from path + cat: './schemas/CatType.yaml' + dog: './schemas/DogType.yaml' + properties: + itemType: + type: string` + + // Cat schema - bare file (no components section) + catSchema := `type: object +properties: + meow: + type: boolean` + + // Dog schema - bare file (no components section) + dogSchema := `type: object +properties: + bark: + type: boolean` + + require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, "schemas"), 0755)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "root.yaml"), []byte(rootSpec), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "schemas", "CatType.yaml"), []byte(catSchema), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "schemas", "DogType.yaml"), []byte(dogSchema), 0644)) + + rootBytes, err := os.ReadFile(filepath.Join(tmpDir, "root.yaml")) + require.NoError(t, err) + + config := datamodel.NewDocumentConfiguration() + config.BasePath = tmpDir + + bundled, err := BundleBytesComposed(rootBytes, config, nil) + require.NoError(t, err) + + bundledStr := string(bundled) + t.Logf("Bundled output:\n%s", bundledStr) + + // Schemas should be composed with names derived from filenames (CatType, DogType) + assert.Contains(t, bundledStr, "CatType", "Cat schema should be composed with name from filename") + assert.Contains(t, bundledStr, "DogType", "Dog schema should be composed with name from filename") + assert.Contains(t, bundledStr, "meow", "Cat properties should be present") + assert.Contains(t, bundledStr, "bark", "Dog properties should be present") + + // Verify no file paths remain in mappings + assertNoFilePathRefs(t, bundled) + assertNoFilePathMappings(t, bundled) +} + +// assertNoFilePathMappings checks that no file-path values remain in discriminator mappings +func assertNoFilePathMappings(t *testing.T, yamlBytes []byte) { + t.Helper() + content := string(yamlBytes) + + // Look for mapping values that look like file paths + // These appear as values in the mapping block (not as $ref values) + mappingPattern := regexp.MustCompile(`mapping:\s*\n((?:\s+\w+:\s*['"]?[^'"}\n]+['"]?\n?)+)`) + matches := mappingPattern.FindAllStringSubmatch(content, -1) + + for _, match := range matches { + if len(match) < 2 { + continue + } + mappingContent := match[1] + + // Check each value in the mapping block + valuePattern := regexp.MustCompile(`:\s*['"]?([^'"}\n]+)['"]?`) + values := valuePattern.FindAllStringSubmatch(mappingContent, -1) + + for _, val := range values { + if len(val) < 2 { + continue + } + refValue := strings.TrimSpace(val[1]) + + // Skip legitimate refs + if strings.HasPrefix(refValue, "#/") || + strings.HasPrefix(refValue, "http://") || + strings.HasPrefix(refValue, "https://") || + strings.HasPrefix(refValue, "urn:") { + continue + } + + // Flag file-path patterns + if strings.HasPrefix(refValue, "./") || + strings.HasPrefix(refValue, "../") || + strings.Contains(refValue, ".yaml") || + strings.Contains(refValue, ".yml") || + strings.Contains(refValue, ".json") { + t.Errorf("Found unrewritten file-path in discriminator mapping: %s", refValue) + } + } + } +} + +// TestBundlerComposed_ComposesDiscriminatorMappingOnlyTargets verifies that schemas +// which are ONLY referenced via discriminator mappings (not via $ref) are still +// composed into the bundled output. +func TestBundlerComposed_ComposesDiscriminatorMappingOnlyTargets(t *testing.T) { + // Create temp directory structure + tmpDir := t.TempDir() + + // Root spec with discriminator mapping pointing to external schemas + rootSpec := `openapi: 3.1.0 +info: + title: Test API + version: 1.0.0 +paths: + /pets: + get: + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: '#/components/schemas/Pet' +components: + schemas: + Pet: + type: object + discriminator: + propertyName: petType + mapping: + cat: './schemas/Cat.yaml' + dog: './schemas/Dog.yaml' + properties: + petType: + type: string + name: + type: string +` + + // Cat schema - only referenced via discriminator mapping + catSchema := `type: object +allOf: + - $ref: '../root.yaml#/components/schemas/Pet' + - type: object + properties: + meow: + type: boolean +` + + // Dog schema - only referenced via discriminator mapping + dogSchema := `type: object +allOf: + - $ref: '../root.yaml#/components/schemas/Pet' + - type: object + properties: + bark: + type: boolean +` + + // Write files + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "root.yaml"), []byte(rootSpec), 0644)) + require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, "schemas"), 0755)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "schemas", "Cat.yaml"), []byte(catSchema), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "schemas", "Dog.yaml"), []byte(dogSchema), 0644)) + + // Read and bundle + rootBytes, err := os.ReadFile(filepath.Join(tmpDir, "root.yaml")) + require.NoError(t, err) + + config := datamodel.NewDocumentConfiguration() + config.BasePath = tmpDir + + bundled, err := BundleBytesComposed(rootBytes, config, nil) + require.NoError(t, err) + + bundledStr := string(bundled) + t.Logf("Bundled output:\n%s", bundledStr) + + // Verify Cat and Dog schemas were composed into components + assert.Contains(t, bundledStr, "Cat", "Cat schema should be composed into components") + assert.Contains(t, bundledStr, "Dog", "Dog schema should be composed into components") + + // Verify discriminator mappings were rewritten to point to composed schemas + assert.Contains(t, bundledStr, "cat: '#/components/schemas/Cat'", "Cat mapping should be rewritten to component ref") + assert.Contains(t, bundledStr, "dog: '#/components/schemas/Dog'", "Dog mapping should be rewritten to component ref") + + // Verify no file paths remain + assertNoFilePathRefs(t, bundled) + assertNoFilePathMappings(t, bundled) +} + +// TestBundlerComposed_DiscriminatorMappingTargets_TransitiveRefs verifies that +// mapping-only targets with their own external refs are fully composed. +func TestBundlerComposed_DiscriminatorMappingTargets_TransitiveRefs(t *testing.T) { + tmpDir := t.TempDir() + + rootSpec := `openapi: 3.1.0 +info: + title: Test API + version: 1.0.0 +paths: + /pets: + get: + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: '#/components/schemas/Pet' +components: + schemas: + Pet: + type: object + discriminator: + propertyName: petType + mapping: + cat: './schemas/Cat.yaml' + properties: + petType: + type: string +` + + // Cat schema is only referenced via discriminator mapping and pulls in Base.yaml + catSchema := `type: object +allOf: + - $ref: './Base.yaml' + - type: object + properties: + meow: + type: boolean +` + + baseSchema := `type: object +properties: + id: + type: string +` + + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "root.yaml"), []byte(rootSpec), 0644)) + require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, "schemas"), 0755)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "schemas", "Cat.yaml"), []byte(catSchema), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "schemas", "Base.yaml"), []byte(baseSchema), 0644)) + + rootBytes, err := os.ReadFile(filepath.Join(tmpDir, "root.yaml")) + require.NoError(t, err) + + config := datamodel.NewDocumentConfiguration() + config.BasePath = tmpDir + + bundled, err := BundleBytesComposed(rootBytes, config, nil) + require.NoError(t, err) + + bundledStr := string(bundled) + t.Logf("Bundled output:\n%s", bundledStr) + + // Base schema should be composed and refs rewritten + assert.Contains(t, bundledStr, "Base", "Base schema should be composed into components") + + // Verify no file paths remain + assertNoFilePathRefs(t, bundled) + assertNoFilePathMappings(t, bundled) +} + +// TestBundlerComposed_HandlesDiscriminatorMappingWithoutFragment tests mappings +// that reference external files without a JSON pointer fragment (e.g., './Admin.yaml') +func TestBundlerComposed_HandlesDiscriminatorMappingWithoutFragment(t *testing.T) { + tmpDir := t.TempDir() + + rootSpec := `openapi: 3.1.0 +info: + title: Test API + version: 1.0.0 +paths: + /users: + get: + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: '#/components/schemas/User' +components: + schemas: + User: + type: object + discriminator: + propertyName: role + mapping: + admin: './Admin.yaml' + guest: './Guest.yaml' + properties: + role: + type: string +` + + adminSchema := `type: object +properties: + permissions: + type: array + items: + type: string +` + + guestSchema := `type: object +properties: + expiresAt: + type: string + format: date-time +` + + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "root.yaml"), []byte(rootSpec), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "Admin.yaml"), []byte(adminSchema), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "Guest.yaml"), []byte(guestSchema), 0644)) + + rootBytes, err := os.ReadFile(filepath.Join(tmpDir, "root.yaml")) + require.NoError(t, err) + + config := datamodel.NewDocumentConfiguration() + config.BasePath = tmpDir + + bundled, err := BundleBytesComposed(rootBytes, config, nil) + require.NoError(t, err) + + bundledStr := string(bundled) + t.Logf("Bundled output:\n%s", bundledStr) + + // Verify schemas were composed + assert.Contains(t, bundledStr, "Admin", "Admin schema should be composed") + assert.Contains(t, bundledStr, "Guest", "Guest schema should be composed") + assert.Contains(t, bundledStr, "permissions", "Admin properties should be present") + assert.Contains(t, bundledStr, "expiresAt", "Guest properties should be present") + + // Verify no file paths remain + assertNoFilePathRefs(t, bundled) + assertNoFilePathMappings(t, bundled) +} + +// TestBundlerComposed_PreservesExternalHttpUrls verifies that http(s) URLs in +// discriminator mappings are NOT rewritten +func TestBundlerComposed_PreservesExternalHttpUrls(t *testing.T) { + spec := `openapi: 3.1.0 +info: + title: Test API + version: 1.0.0 +paths: + /pets: + get: + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: '#/components/schemas/Pet' +components: + schemas: + Pet: + type: object + discriminator: + propertyName: petType + mapping: + external: 'https://example.com/schemas/ExternalPet.yaml' + properties: + petType: + type: string +` + + config := datamodel.NewDocumentConfiguration() + bundled, err := BundleBytesComposed([]byte(spec), config, nil) + require.NoError(t, err) + + bundledStr := string(bundled) + + // Verify external URL is preserved + assert.Contains(t, bundledStr, "https://example.com/schemas/ExternalPet.yaml", + "External HTTP URL should be preserved in discriminator mapping") +} + +// TestBundlerComposed_PreservesUrns verifies that URN references in +// discriminator mappings are NOT rewritten +func TestBundlerComposed_PreservesUrns(t *testing.T) { + spec := `openapi: 3.1.0 +info: + title: Test API + version: 1.0.0 +paths: + /pets: + get: + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: '#/components/schemas/Pet' +components: + schemas: + Pet: + type: object + discriminator: + propertyName: petType + mapping: + special: 'urn:example:pet:special' + properties: + petType: + type: string +` + + config := datamodel.NewDocumentConfiguration() + bundled, err := BundleBytesComposed([]byte(spec), config, nil) + require.NoError(t, err) + + bundledStr := string(bundled) + + // Verify URN is preserved + assert.Contains(t, bundledStr, "urn:example:pet:special", + "URN should be preserved in discriminator mapping") +} + +// TestBundlerComposed_SkipsVendorExtensionRefs verifies that $ref values inside +// vendor extensions (x-*) are NOT rewritten when external refs are processed +func TestBundlerComposed_SkipsVendorExtensionRefs(t *testing.T) { + tmpDir := t.TempDir() + + // Root spec with a vendor extension containing a ref-like value + rootSpec := `openapi: 3.1.0 +info: + title: Test API + version: 1.0.0 +paths: + /users: + get: + x-custom-extension: + ref: './custom-format.yaml' + type: custom + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: './schemas/User.yaml' +` + + userSchema := `type: object +properties: + name: + type: string +` + + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "root.yaml"), []byte(rootSpec), 0644)) + require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, "schemas"), 0755)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "schemas", "User.yaml"), []byte(userSchema), 0644)) + + rootBytes, err := os.ReadFile(filepath.Join(tmpDir, "root.yaml")) + require.NoError(t, err) + + config := datamodel.NewDocumentConfiguration() + config.BasePath = tmpDir + + bundled, err := BundleBytesComposed(rootBytes, config, nil) + require.NoError(t, err) + + bundledStr := string(bundled) + + // The extension should be preserved + assert.Contains(t, bundledStr, "x-custom-extension", + "Vendor extension should be preserved") + // The ref inside the extension should NOT be rewritten (it's custom format, not a $ref) + assert.Contains(t, bundledStr, "./custom-format.yaml", + "Extension internal refs should be preserved as-is") +} + +// TestBundlerComposed_HandlesEmptyMappingValues verifies that empty discriminator +// mapping values don't cause errors +func TestBundlerComposed_HandlesEmptyMappingValues(t *testing.T) { + spec := `openapi: 3.1.0 +info: + title: Test API + version: 1.0.0 +paths: + /pets: + get: + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: '#/components/schemas/Pet' +components: + schemas: + Pet: + type: object + discriminator: + propertyName: petType + mapping: + empty: '' + local: '#/components/schemas/LocalPet' + properties: + petType: + type: string + LocalPet: + type: object +` + + config := datamodel.NewDocumentConfiguration() + bundled, err := BundleBytesComposed([]byte(spec), config, nil) + require.NoError(t, err) + + bundledStr := string(bundled) + + // Verify local ref is preserved and spec is valid + assert.Contains(t, bundledStr, "#/components/schemas/LocalPet", + "Local component ref should be preserved") +} + +// TestBundlerComposed_HandlesExternalFileLocalRefs verifies that #/ refs in external +// files (which refer to THAT file's components) are properly composed +func TestBundlerComposed_HandlesExternalFileLocalRefs(t *testing.T) { + tmpDir := t.TempDir() + + rootSpec := `openapi: 3.1.0 +info: + title: Test API + version: 1.0.0 +paths: + /pets: + get: + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: './external.yaml#/components/schemas/Pet' +` + + // External file has its own components section with internal refs + externalSpec := `components: + schemas: + Pet: + type: object + discriminator: + propertyName: petType + mapping: + cat: '#/components/schemas/Cat' + properties: + petType: + type: string + Cat: + type: object + properties: + meow: + type: boolean +` + + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "root.yaml"), []byte(rootSpec), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "external.yaml"), []byte(externalSpec), 0644)) + + rootBytes, err := os.ReadFile(filepath.Join(tmpDir, "root.yaml")) + require.NoError(t, err) + + config := datamodel.NewDocumentConfiguration() + config.BasePath = tmpDir + + bundled, err := BundleBytesComposed(rootBytes, config, nil) + require.NoError(t, err) + + bundledStr := string(bundled) + t.Logf("Bundled output:\n%s", bundledStr) + + // Verify both Pet and Cat were composed + assert.Contains(t, bundledStr, "Pet", "Pet schema should be composed") + assert.Contains(t, bundledStr, "Cat", "Cat schema should be composed") + assert.Contains(t, bundledStr, "meow", "Cat properties should be present") + + // Verify no file paths remain + assertNoFilePathRefs(t, bundled) + assertNoFilePathMappings(t, bundled) +} + +// TestBundlerComposedWithOrigins_DiscriminatorMappingTargetsHaveOrigins verifies +// that schemas composed from discriminator mapping targets have proper origin tracking +func TestBundlerComposedWithOrigins_DiscriminatorMappingTargetsHaveOrigins(t *testing.T) { + tmpDir := t.TempDir() + + rootSpec := `openapi: 3.1.0 +info: + title: Test API + version: 1.0.0 +paths: + /pets: + get: + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: '#/components/schemas/Pet' +components: + schemas: + Pet: + type: object + discriminator: + propertyName: petType + mapping: + cat: './schemas/Cat.yaml' + properties: + petType: + type: string +` + + catSchema := `type: object +properties: + meow: + type: boolean +` + + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "root.yaml"), []byte(rootSpec), 0644)) + require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, "schemas"), 0755)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "schemas", "Cat.yaml"), []byte(catSchema), 0644)) + + rootBytes, err := os.ReadFile(filepath.Join(tmpDir, "root.yaml")) + require.NoError(t, err) + + config := datamodel.NewDocumentConfiguration() + config.BasePath = tmpDir + + result, err := BundleBytesComposedWithOrigins(rootBytes, config, nil) + require.NoError(t, err) + require.NotNil(t, result) + + t.Logf("Bundled output:\n%s", string(result.Bytes)) + t.Logf("Origins: %+v", result.Origins) + + // Verify Cat schema has origin tracking + // Note: The exact key depends on how the schema was named during composition + foundCatOrigin := false + for key, origin := range result.Origins { + t.Logf("Origin key: %s, file: %s", key, origin.OriginalFile) + if strings.Contains(key, "Cat") { + foundCatOrigin = true + assert.Contains(t, origin.OriginalFile, "Cat.yaml", + "Cat origin should reference Cat.yaml file") + } + } + + // Cat may or may not have an origin depending on how it was processed + // The important thing is that the schema was composed correctly + _ = foundCatOrigin +} + +// TestBundlerComposed_RewritesResponseRefsInPathItems verifies that $ref values +// within path items (like responses) pointing to external files are rewritten +func TestBundlerComposed_RewritesResponseRefsInPathItems(t *testing.T) { + tmpDir := t.TempDir() + + rootSpec := `openapi: 3.1.0 +info: + title: Test API + version: 1.0.0 +paths: + /users: + get: + responses: + '200': + $ref: './responses/Success.yaml' + '404': + $ref: './responses/NotFound.yaml' +` + + successResponse := `description: Success response +content: + application/json: + schema: + type: object + properties: + message: + type: string +` + + notFoundResponse := `description: Not found response +content: + application/json: + schema: + type: object + properties: + error: + type: string +` + + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "root.yaml"), []byte(rootSpec), 0644)) + require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, "responses"), 0755)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "responses", "Success.yaml"), []byte(successResponse), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "responses", "NotFound.yaml"), []byte(notFoundResponse), 0644)) + + rootBytes, err := os.ReadFile(filepath.Join(tmpDir, "root.yaml")) + require.NoError(t, err) + + config := datamodel.NewDocumentConfiguration() + config.BasePath = tmpDir + + bundled, err := BundleBytesComposed(rootBytes, config, nil) + require.NoError(t, err) + + bundledStr := string(bundled) + t.Logf("Bundled output:\n%s", bundledStr) + + // Verify responses were composed and refs were rewritten + assert.Contains(t, bundledStr, "Success response", "Success response content should be present") + assert.Contains(t, bundledStr, "Not found response", "NotFound response content should be present") + + // Verify no file paths remain + assertNoFilePathRefs(t, bundled) +} + +// TestBundlerComposed_RewritesSchemaRefsInLiftedComponents verifies that $ref values +// within components that were lifted from external files are also rewritten +func TestBundlerComposed_RewritesSchemaRefsInLiftedComponents(t *testing.T) { + tmpDir := t.TempDir() + + rootSpec := `openapi: 3.1.0 +info: + title: Test API + version: 1.0.0 +paths: + /users: + get: + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: './schemas/User.yaml' +` + + // User schema references Address schema in same directory + userSchema := `type: object +properties: + name: + type: string + address: + $ref: './Address.yaml' +` + + addressSchema := `type: object +properties: + street: + type: string + city: + type: string +` + + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "root.yaml"), []byte(rootSpec), 0644)) + require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, "schemas"), 0755)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "schemas", "User.yaml"), []byte(userSchema), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "schemas", "Address.yaml"), []byte(addressSchema), 0644)) + + rootBytes, err := os.ReadFile(filepath.Join(tmpDir, "root.yaml")) + require.NoError(t, err) + + config := datamodel.NewDocumentConfiguration() + config.BasePath = tmpDir + + bundled, err := BundleBytesComposed(rootBytes, config, nil) + require.NoError(t, err) + + bundledStr := string(bundled) + t.Logf("Bundled output:\n%s", bundledStr) + + // Verify both schemas were composed + assert.Contains(t, bundledStr, "User", "User schema should be composed") + assert.Contains(t, bundledStr, "Address", "Address schema should be composed") + assert.Contains(t, bundledStr, "street", "Address properties should be present") + + // Verify no file paths remain + assertNoFilePathRefs(t, bundled) +} + +// TestBundlerComposed_CustomFS_NoDoubledPaths verifies that when using a custom fs.FS +// implementation via DocumentConfiguration.LocalFS, the bundler does not generate +// doubled path segments like "components/components/schemas/Admin.yaml". +// +// This test exercises the "last-ditch rolodex lookup" path in SearchIndexForReference +// which is triggered when files aren't pre-indexed (as happens with custom fs.FS). +func TestBundlerComposed_CustomFS_NoDoubledPaths(t *testing.T) { + // Use fstest.MapFS to provide a custom fs.FS + // This ensures files aren't pre-indexed during BuildV3Model, which triggers + // the rolodex lookup path where the path doubling bug occurs. + rootSpec := `openapi: 3.1.0 +info: + title: Test API with Custom FS + version: 1.0.0 +paths: + /users: + $ref: 'paths/users.yaml' +components: + schemas: + BaseUser: + type: object + discriminator: + propertyName: userType + mapping: + admin: './components/schemas/Admin.yaml' + guest: './components/schemas/Guest.yaml' + properties: + userType: + type: string + name: + type: string +` + + pathsUsers := `get: + summary: Get users + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: '../components/schemas/Admin.yaml' +` + + // Admin and Guest schemas without circular back-ref to root + adminSchema := `type: object +properties: + adminLevel: + type: integer + name: + type: string +` + + guestSchema := `type: object +properties: + expiresAt: + type: string + format: date-time + name: + type: string +` + + customFS := fstest.MapFS{ + "openapi.yaml": &fstest.MapFile{Data: []byte(rootSpec)}, + "paths/users.yaml": &fstest.MapFile{Data: []byte(pathsUsers)}, + "components/schemas/Admin.yaml": &fstest.MapFile{Data: []byte(adminSchema)}, + "components/schemas/Guest.yaml": &fstest.MapFile{Data: []byte(guestSchema)}, + } + + // Create a LocalFS using the custom fstest.MapFS via DirFS configuration + localFS, err := index.NewLocalFSWithConfig(&index.LocalFSConfig{ + BaseDirectory: ".", + DirFS: customFS, + }) + require.NoError(t, err) + + config := datamodel.NewDocumentConfiguration() + config.BasePath = "." + config.LocalFS = localFS + config.AllowFileReferences = true + + bundled, err := BundleBytesComposed([]byte(rootSpec), config, nil) + require.NoError(t, err) + + bundledStr := string(bundled) + t.Logf("Bundled output:\n%s", bundledStr) + + // Assert no doubled path segments in output + assert.NotContains(t, bundledStr, "components/components", + "Path should not have doubled 'components' segments") + assert.NotContains(t, bundledStr, "schemas/schemas", + "Path should not have doubled 'schemas' segments") + assert.NotContains(t, bundledStr, "paths/paths", + "Path should not have doubled 'paths' segments") + + // Verify the schemas were actually composed (not silently dropped) + assert.Contains(t, bundledStr, "Admin", "Admin schema should be composed") + assert.Contains(t, bundledStr, "Guest", "Guest schema should be composed") + assert.Contains(t, bundledStr, "adminLevel", "Admin properties should be present") + assert.Contains(t, bundledStr, "expiresAt", "Guest properties should be present") + + // Verify no file paths remain + assertNoFilePathRefs(t, bundled) + assertNoFilePathMappings(t, bundled) +} + +// TestBundlerComposed_CustomFS_CrossDirectoryRefs verifies that refs from paths/ +// to components/ directories work correctly with custom fs.FS and don't produce +// doubled path segments. +func TestBundlerComposed_CustomFS_CrossDirectoryRefs(t *testing.T) { + rootSpec := `openapi: 3.1.0 +info: + title: Cross Directory Refs Test + version: 1.0.0 +paths: + /items: + $ref: 'paths/items.yaml' +` + + pathsItems := `get: + summary: Get items + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: '../components/schemas/Item.yaml' +post: + summary: Create item + requestBody: + content: + application/json: + schema: + $ref: '../components/schemas/CreateItem.yaml' + responses: + '201': + description: Created +` + + itemSchema := `type: object +properties: + id: + type: string + data: + $ref: './ItemData.yaml' +` + + createItemSchema := `type: object +properties: + data: + $ref: './ItemData.yaml' +required: + - data +` + + itemDataSchema := `type: object +properties: + name: + type: string + value: + type: number +` + + customFS := fstest.MapFS{ + "openapi.yaml": &fstest.MapFile{Data: []byte(rootSpec)}, + "paths/items.yaml": &fstest.MapFile{Data: []byte(pathsItems)}, + "components/schemas/Item.yaml": &fstest.MapFile{Data: []byte(itemSchema)}, + "components/schemas/CreateItem.yaml": &fstest.MapFile{Data: []byte(createItemSchema)}, + "components/schemas/ItemData.yaml": &fstest.MapFile{Data: []byte(itemDataSchema)}, + } + + // Create a LocalFS using the custom fstest.MapFS via DirFS configuration + localFS, err := index.NewLocalFSWithConfig(&index.LocalFSConfig{ + BaseDirectory: ".", + DirFS: customFS, + }) + require.NoError(t, err) + + config := datamodel.NewDocumentConfiguration() + config.BasePath = "." + config.LocalFS = localFS + config.AllowFileReferences = true + + bundled, err := BundleBytesComposed([]byte(rootSpec), config, nil) + require.NoError(t, err) + + bundledStr := string(bundled) + t.Logf("Bundled output:\n%s", bundledStr) + + // Assert no doubled path segments + assert.NotContains(t, bundledStr, "components/components", + "Path should not have doubled 'components' segments") + assert.NotContains(t, bundledStr, "schemas/schemas", + "Path should not have doubled 'schemas' segments") + + // Verify schemas were composed + assert.Contains(t, bundledStr, "Item", "Item schema should be composed") + assert.Contains(t, bundledStr, "CreateItem", "CreateItem schema should be composed") + assert.Contains(t, bundledStr, "ItemData", "ItemData schema should be composed") + + // Verify no file paths remain + assertNoFilePathRefs(t, bundled) +} + +// TestBundlerComposed_RewritesResponseRefsFromPathFiles verifies that $ref values +// inside pathItem files pointing to response files are properly rewritten. +// This is a regression test for refs like "../components/responses/Problem.yaml" +// not being rewritten when they're inside composed pathItems. +func TestBundlerComposed_RewritesResponseRefsFromPathFiles(t *testing.T) { + tmpDir := t.TempDir() + + // Root spec with paths pointing to external files + rootSpec := `openapi: 3.1.0 +info: + title: Test API + version: 1.0.0 +paths: + /users: + $ref: 'paths/users.yaml' +components: + schemas: + User: + type: object + properties: + name: + type: string +` + + // Path file with ref to response file using relative path + pathsUsers := `get: + summary: Get users + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: '../components/schemas/UserResponse.yaml' + '404': + $ref: '../components/responses/NotFound.yaml' +` + + userResponseSchema := `type: object +properties: + users: + type: array + items: + type: object + properties: + id: + type: string +` + + notFoundResponse := `description: Not Found +content: + application/json: + schema: + type: object + properties: + error: + type: string +` + + // Create directory structure + require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, "paths"), 0755)) + require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, "components", "schemas"), 0755)) + require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, "components", "responses"), 0755)) + + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "root.yaml"), []byte(rootSpec), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "paths", "users.yaml"), []byte(pathsUsers), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "components", "schemas", "UserResponse.yaml"), []byte(userResponseSchema), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "components", "responses", "NotFound.yaml"), []byte(notFoundResponse), 0644)) + + rootBytes, err := os.ReadFile(filepath.Join(tmpDir, "root.yaml")) + require.NoError(t, err) + + config := datamodel.NewDocumentConfiguration() + config.BasePath = tmpDir + + bundled, err := BundleBytesComposed(rootBytes, config, nil) + require.NoError(t, err) + + bundledStr := string(bundled) + t.Logf("Bundled output:\n%s", bundledStr) + + // Verify response refs were rewritten - should NOT contain file paths + assert.NotContains(t, bundledStr, "../components/responses/NotFound.yaml", + "Response ref should be rewritten to component ref") + assert.NotContains(t, bundledStr, "../components/schemas/UserResponse.yaml", + "Schema ref should be rewritten to component ref") + + // Verify the components were composed + assert.Contains(t, bundledStr, "NotFound", "NotFound response should be composed") + assert.Contains(t, bundledStr, "UserResponse", "UserResponse schema should be composed") + + // Verify no file paths remain + assertNoFilePathRefs(t, bundled) +} + +// TestBundlerComposed_NoDuplicateSchemas verifies that schemas referenced from +// multiple locations (root and external files) are not duplicated. +func TestBundlerComposed_NoDuplicateSchemas(t *testing.T) { + tmpDir := t.TempDir() + + // Root spec with discriminator mapping pointing to external schemas + rootSpec := `openapi: 3.1.0 +info: + title: Test API + version: 1.0.0 +paths: + /users: + $ref: 'paths/users.yaml' +components: + schemas: + User: + type: object + discriminator: + propertyName: userType + mapping: + admin: './components/schemas/Admin.yaml' + basic: './components/schemas/Basic.yaml' + properties: + userType: + type: string + name: + type: string +` + + // Path file that also references Admin schema + pathsUsers := `post: + summary: Create user + requestBody: + content: + application/json: + schema: + discriminator: + propertyName: userType + mapping: + admin: '../components/schemas/Admin.yaml' + basic: '../components/schemas/Basic.yaml' + anyOf: + - $ref: '../components/schemas/Admin.yaml' + - $ref: '../components/schemas/Basic.yaml' + responses: + '201': + description: Created +` + + adminSchema := `type: object +properties: + adminLevel: + type: integer +` + + basicSchema := `type: object +properties: + accessLevel: + type: integer +` + + // Create directory structure + require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, "paths"), 0755)) + require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, "components", "schemas"), 0755)) + + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "root.yaml"), []byte(rootSpec), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "paths", "users.yaml"), []byte(pathsUsers), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "components", "schemas", "Admin.yaml"), []byte(adminSchema), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "components", "schemas", "Basic.yaml"), []byte(basicSchema), 0644)) + + rootBytes, err := os.ReadFile(filepath.Join(tmpDir, "root.yaml")) + require.NoError(t, err) + + config := datamodel.NewDocumentConfiguration() + config.BasePath = tmpDir + + bundled, err := BundleBytesComposed(rootBytes, config, nil) + require.NoError(t, err) + + bundledStr := string(bundled) + t.Logf("Bundled output:\n%s", bundledStr) + + // Count occurrences of Admin in schemas section + // Should appear exactly once, not twice (no Admin__schemas) + assert.NotContains(t, bundledStr, "Admin__", + "Admin schema should not be duplicated with suffix") + assert.NotContains(t, bundledStr, "Basic__", + "Basic schema should not be duplicated with suffix") + + // Verify schemas were composed + assert.Contains(t, bundledStr, "Admin", "Admin schema should be composed") + assert.Contains(t, bundledStr, "Basic", "Basic schema should be composed") + assert.Contains(t, bundledStr, "adminLevel", "Admin properties should be present") + assert.Contains(t, bundledStr, "accessLevel", "Basic properties should be present") + + // Verify no file paths remain + assertNoFilePathRefs(t, bundled) + assertNoFilePathMappings(t, bundled) +} + +// TestBundlerComposed_OA31_SiblingRefProperties tests that OpenAPI 3.1 $ref with +// sibling properties (like description) are handled correctly: +// 1. The sibling properties (description) should be PRESERVED +// 2. The $ref should be REWRITTEN to the composed component location +func TestBundlerComposed_OA31_SiblingRefProperties(t *testing.T) { + tmpDir := t.TempDir() + + // Root spec + rootSpec := `openapi: 3.1.0 +info: + title: Test API + version: 1.0.0 +paths: + '/users/{username}': + $ref: 'paths/users_{username}.yaml' +components: + securitySchemes: + api_key: + type: apiKey + in: header + name: api_key +` + + // Path file with $ref AND sibling description (valid in OA 3.1) + pathsUsers := `get: + summary: Get user by name + operationId: getUserByName + parameters: + - name: username + in: path + required: true + schema: + type: string + responses: + '200': + description: Success + content: + application/json: + schema: + $ref: '../components/schemas/User.yaml' + '403': + description: Forbidden + $ref: '../components/responses/Problem.yaml' + '404': + description: User not found + $ref: '../components/responses/Problem.yaml' +` + + // User schema + userSchema := `type: object +properties: + name: + type: string + email: + type: string +` + + // Problem response + problemResponse := `description: Problem +content: + application/problem+json: + schema: + $ref: '../schemas/Problem.yaml' +` + + // Problem schema + problemSchema := `type: object +properties: + type: + type: string + title: + type: string + status: + type: integer +` + + // Create directory structure + require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, "paths"), 0755)) + require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, "components", "schemas"), 0755)) + require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, "components", "responses"), 0755)) + + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "root.yaml"), []byte(rootSpec), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "paths", "users_{username}.yaml"), []byte(pathsUsers), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "components", "schemas", "User.yaml"), []byte(userSchema), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "components", "responses", "Problem.yaml"), []byte(problemResponse), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "components", "schemas", "Problem.yaml"), []byte(problemSchema), 0644)) + + rootBytes, err := os.ReadFile(filepath.Join(tmpDir, "root.yaml")) + require.NoError(t, err) + + config := datamodel.NewDocumentConfiguration() + config.BasePath = tmpDir + + bundled, err := BundleBytesComposed(rootBytes, config, nil) + require.NoError(t, err) + + bundledStr := string(bundled) + t.Logf("Bundled output:\n%s", bundledStr) + + // Issue 1: Description should be PRESERVED as "Forbidden", NOT changed to "#/components/responses/Problem" + assert.Contains(t, bundledStr, "description: Forbidden", + "Original description 'Forbidden' should be preserved, not overwritten") + assert.NotContains(t, bundledStr, "description: '#/components/responses/Problem'", + "Description should NOT be overwritten with the ref target") + + // Issue 2: The $ref should be REWRITTEN to component ref + assert.NotContains(t, bundledStr, "$ref: ../components/responses/Problem.yaml", + "File path ref should be rewritten") + assert.Contains(t, bundledStr, "$ref: '#/components/responses/Problem'", + "Ref should be rewritten to composed component location") + + // Verify no file paths remain + assertNoFilePathRefs(t, bundled) +} + +// TestBundlerComposed_DuplicateSchemasDifferentPaths tests that the same schema +// referenced via different relative paths is NOT duplicated +func TestBundlerComposed_DuplicateSchemasDifferentPaths(t *testing.T) { + tmpDir := t.TempDir() + + // Root spec - references User.yaml which has discriminator to Admin/Basic + rootSpec := `openapi: 3.1.0 +info: + title: Test API + version: 1.0.0 +paths: + '/users/{username}': + $ref: 'paths/users_{username}.yaml' + '/user': + $ref: 'paths/user.yaml' +` + + // Path file that references User schema + pathsUsersUsername := `get: + summary: Get user + responses: + '200': + description: Success + content: + application/json: + schema: + $ref: '../components/schemas/User.yaml' +` + + // Path file with discriminator mapping using DIFFERENT relative path + pathsUser := `post: + summary: Create user + requestBody: + content: + application/json: + schema: + discriminator: + propertyName: userType + mapping: + admin: '../components/schemas/Admin.yaml' + basic: '../components/schemas/Basic.yaml' + anyOf: + - $ref: '../components/schemas/Admin.yaml' + - $ref: '../components/schemas/Basic.yaml' + responses: + '200': + description: Success +` + + // User schema with discriminator using SAME-DIRECTORY relative path + userSchema := `type: object +discriminator: + propertyName: userType + mapping: + admin: './Admin.yaml' + basic: './Basic.yaml' +properties: + userType: + type: string + name: + type: string +` + + // Admin schema + adminSchema := `description: Admin user +allOf: + - $ref: './User.yaml' + - type: object + properties: + adminLevel: + type: integer +` + + // Basic schema + basicSchema := `description: Basic user +allOf: + - $ref: './User.yaml' + - type: object + properties: + accessLevel: + type: integer +` + + // Create directory structure + require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, "paths"), 0755)) + require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, "components", "schemas"), 0755)) + + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "root.yaml"), []byte(rootSpec), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "paths", "users_{username}.yaml"), []byte(pathsUsersUsername), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "paths", "user.yaml"), []byte(pathsUser), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "components", "schemas", "User.yaml"), []byte(userSchema), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "components", "schemas", "Admin.yaml"), []byte(adminSchema), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "components", "schemas", "Basic.yaml"), []byte(basicSchema), 0644)) + + rootBytes, err := os.ReadFile(filepath.Join(tmpDir, "root.yaml")) + require.NoError(t, err) + + config := datamodel.NewDocumentConfiguration() + config.BasePath = tmpDir + + bundled, err := BundleBytesComposed(rootBytes, config, nil) + require.NoError(t, err) + + bundledStr := string(bundled) + t.Logf("Bundled output:\n%s", bundledStr) + + // Issue: Same schema should NOT be duplicated with collision suffix + assert.NotContains(t, bundledStr, "Admin__", + "Admin schema should not be duplicated with suffix") + assert.NotContains(t, bundledStr, "Basic__", + "Basic schema should not be duplicated with suffix") + + // Count occurrences of "Admin:" in schemas section - should be exactly 1 + adminCount := strings.Count(bundledStr, "Admin:") + assert.Equal(t, 1, adminCount, "Admin schema should appear exactly once, got %d", adminCount) + + basicCount := strings.Count(bundledStr, "Basic:") + assert.Equal(t, 1, basicCount, "Basic schema should appear exactly once, got %d", basicCount) + + // Verify schemas were composed + assert.Contains(t, bundledStr, "Admin", "Admin schema should be present") + assert.Contains(t, bundledStr, "Basic", "Basic schema should be present") + + // Verify no file paths remain + assertNoFilePathRefs(t, bundled) + assertNoFilePathMappings(t, bundled) +} diff --git a/bundler/bundler_strict_validation_test.go b/bundler/bundler_strict_validation_test.go index 73e26a96..09223354 100644 --- a/bundler/bundler_strict_validation_test.go +++ b/bundler/bundler_strict_validation_test.go @@ -55,6 +55,49 @@ components: assert.Contains(t, err.Error(), "column 17") } +func TestStrictValidation_RefWithSiblings_WithOrigins_ShouldError(t *testing.T) { + spec := `openapi: 3.0.0 +info: + title: Test API + version: 1.0.0 +paths: + /test: + get: + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: '#/components/schemas/TestSchema' + description: This is invalid - $ref cannot have siblings +components: + schemas: + TestSchema: + type: object + properties: + name: + type: string` + + config := &BundleCompositionConfig{ + StrictValidation: true, + } + + docConfig := &datamodel.DocumentConfiguration{ + AllowFileReferences: false, + } + + result, err := BundleBytesComposedWithOrigins([]byte(spec), docConfig, config) + + require.Error(t, err, "Strict validation must fail on invalid $ref siblings for 3.0 specs") + assert.Nil(t, result) + assert.Contains( + t, + err.Error(), + "invalid OpenAPI 3.0 specification: $ref cannot have sibling properties", + ) +} + func TestStrictValidation_RefWithoutSiblings_ShouldSucceed(t *testing.T) { spec := `openapi: 3.0.0 info: diff --git a/bundler/bundler_test.go b/bundler/bundler_test.go index 14fa093e..f6cadb77 100644 --- a/bundler/bundler_test.go +++ b/bundler/bundler_test.go @@ -2948,3 +2948,45 @@ components: "BundleInlineConfig.InlineLocalRefs should override DocumentConfiguration.BundleInlineRefs") assert.Contains(t, bundledStr, "type: object") } + +// TestBundleDocumentComposed_NilRolodex verifies that BundleDocumentComposed returns +// an error when the document has a nil Rolodex. This covers the nil check in compose(). +func TestBundleDocumentComposed_NilRolodex(t *testing.T) { + // Create a v3.Document with nil Rolodex + doc := &v3high.Document{ + Info: &base.Info{Title: "Test"}, + // Rolodex intentionally nil + } + _, err := BundleDocumentComposed(doc, nil) + require.Error(t, err) + assert.Contains(t, err.Error(), "model or rolodex is nil") +} + +// TestBundleDocumentComposed_NilModel verifies that BundleDocumentComposed returns +// an error when the model is nil. This covers the nil check in compose(). +func TestBundleDocumentComposed_NilModel(t *testing.T) { + _, err := BundleDocumentComposed(nil, nil) + require.Error(t, err) + assert.Contains(t, err.Error(), "model or rolodex is nil") +} + +// TestBundleDocumentComposedWithOrigins_NilRolodex verifies that BundleDocumentComposedWithOrigins +// returns an error when the document has a nil Rolodex. This covers the nil check in composeWithOrigins(). +func TestBundleDocumentComposedWithOrigins_NilRolodex(t *testing.T) { + // Create a v3.Document with nil Rolodex + doc := &v3high.Document{ + Info: &base.Info{Title: "Test"}, + // Rolodex intentionally nil + } + _, err := BundleDocumentComposedWithOrigins(doc, nil) + require.Error(t, err) + assert.Contains(t, err.Error(), "model or rolodex is nil") +} + +// TestBundleDocumentComposedWithOrigins_NilModel verifies that BundleDocumentComposedWithOrigins +// returns an error when the model is nil. This covers the nil check in composeWithOrigins(). +func TestBundleDocumentComposedWithOrigins_NilModel(t *testing.T) { + _, err := BundleDocumentComposedWithOrigins(nil, nil) + require.Error(t, err) + assert.Contains(t, err.Error(), "model or rolodex is nil") +} diff --git a/bundler/composer_functions.go b/bundler/composer_functions.go index da47f5c6..090316d9 100644 --- a/bundler/composer_functions.go +++ b/bundler/composer_functions.go @@ -167,7 +167,17 @@ func handleCollision[T any](schemaName, delimiter string, pr *processRef, compon func handleFileImport[T any](pr *processRef, importType, delimiter string, components *orderedmap.Map[string, T]) []string { // extract base name from file before collision handling - baseName := filepath.Base(strings.Replace(pr.ref.Name, filepath.Ext(pr.ref.Name), "", 1)) + // First try pr.ref.Name, then fall back to extracting from FullDefinition + refName := pr.ref.Name + if refName == "" { + // For bare file refs, extract name from FullDefinition path + refName = pr.ref.FullDefinition + // Remove any fragment + if idx := strings.Index(refName, "#"); idx != -1 { + refName = refName[:idx] + } + } + baseName := filepath.Base(strings.Replace(refName, filepath.Ext(refName), "", 1)) // preserve original name before any renaming if pr.originalName == "" { @@ -279,16 +289,22 @@ func rewireRef(idx *index.SpecIndex, ref *index.Reference, fullDef string, proce if pr := processedNodes.GetOrZero(fullDef); pr != nil { if kk, _, _ := utils.IsNodeRefValue(pr.ref.Node); kk { if pr.refPointer == "" { - pr.refPointer = pr.ref.Node.Content[1].Value + // Use GetRefValueNode to handle OA 3.1 sibling properties correctly + if refValNode := utils.GetRefValueNode(pr.ref.Node); refValNode != nil { + pr.refPointer = refValNode.Value + } } } } rename := renameRef(idx, fullDef, processedNodes) if isRef { - - if ref.Node.Content[1].Value != rename { - ref.Node.Content[1].Value = rename + // Use GetRefValueNode to find the correct $ref value node + // This handles OA 3.1 sibling properties where $ref may not be at index 0 + if refValNode := utils.GetRefValueNode(ref.Node); refValNode != nil { + if refValNode.Value != rename { + refValNode.Value = rename + } } ref.FullDefinition = rename ref.Definition = rename @@ -419,3 +435,142 @@ func captureOrigin(pr *processRef, componentType string, origins ComponentOrigin origins[bundledRef] = origin } + +// rewriteAllRefs walks an index's document tree and rewrites un-re-written $ref values. +// Must be called as the FINAL step after all other processing. +func rewriteAllRefs( + idx *index.SpecIndex, + processedNodes *orderedmap.Map[string, *processRef], + rolodex *index.Rolodex, +) { + walkAndRewriteRefs(idx.GetRootNode(), idx, processedNodes, rolodex, false) +} + +func walkAndRewriteRefs( + node *yaml.Node, + sourceIdx *index.SpecIndex, + processedNodes *orderedmap.Map[string, *processRef], + rolodex *index.Rolodex, + inExtension bool, // Tracks if we're under an x-* key +) { + if node == nil { + return + } + + switch node.Kind { + case yaml.DocumentNode: + if len(node.Content) > 0 { + walkAndRewriteRefs(node.Content[0], sourceIdx, processedNodes, rolodex, inExtension) + } + return + case yaml.SequenceNode: + for _, child := range node.Content { + walkAndRewriteRefs(child, sourceIdx, processedNodes, rolodex, inExtension) + } + return + case yaml.MappingNode: + // Continue below + default: + return + } + + for i := 0; i < len(node.Content); i += 2 { + keyNode := node.Content[i] + valueNode := node.Content[i+1] + + // Track extension scope + childInExtension := inExtension || strings.HasPrefix(keyNode.Value, "x-") + + if keyNode.Value == "$ref" && valueNode.Kind == yaml.ScalarNode && !inExtension { + newRef := resolveRefToComposed(valueNode.Value, sourceIdx, processedNodes, rolodex) + if newRef != valueNode.Value { + valueNode.Value = newRef + } + } else { + walkAndRewriteRefs(valueNode, sourceIdx, processedNodes, rolodex, childInExtension) + } + } +} + +func resolveRefToComposed( + refValue string, + sourceIdx *index.SpecIndex, + processedNodes *orderedmap.Map[string, *processRef], + rolodex *index.Rolodex, +) string { + // Skip external URLs and URNs + if strings.HasPrefix(refValue, "http://") || + strings.HasPrefix(refValue, "https://") || + strings.HasPrefix(refValue, "urn:") { + return refValue + } + + // Use source index for relative path resolution + ref, refIdx := sourceIdx.SearchIndexForReference(refValue) + if ref == nil { + ref, refIdx = rolodex.GetRootIndex().SearchIndexForReference(refValue) + } + if ref == nil { + for _, idx := range rolodex.GetIndexes() { + if r, i := idx.SearchIndexForReference(refValue); r != nil { + ref, refIdx = r, i + break + } + } + } + + // fallback for unindexed local refs (root cause #4): + // If the ref starts with #/ but SearchIndexForReference couldn't resolve it, + // try renameRef directly - it may match a processedNodes entry. + if ref == nil && strings.HasPrefix(refValue, "#/") { + // Build absolute key for lookup: sourceIdx path + refValue + absKey := sourceIdx.GetSpecAbsolutePath() + refValue + // Gate on processedNodes presence - only rewrite if target was composed + if processedNodes.GetOrZero(absKey) == nil { + return refValue + } + return renameRef(sourceIdx, absKey, processedNodes) + } + + if ref == nil || refIdx == nil { + return refValue + } + + // SearchIndexForReference returns a Reference with a potentially relative FullDefinition. + // But processedNodes keys are absolute paths. We need to construct the absolute key + // using the returned index's path + the fragment from the reference. + // Format: /abs/path/to/file.yaml#/components/schemas/Name + absoluteKey := ref.FullDefinition + fragment := "" + if idx := strings.Index(ref.FullDefinition, "#"); idx != -1 { + fragment = ref.FullDefinition[idx:] + } + + if !filepath.IsAbs(absoluteKey) && refIdx != nil { + // Build absolute key + absoluteKey = refIdx.GetSpecAbsolutePath() + fragment + } + + // If the ref resolves to the ROOT index, and it's a canonical location (#/components/...) ref, + // we should rewrite it to a local component ref. Root document components are NOT in + // processedNodes (only external refs are), but they're valid targets. + rootIdx := rolodex.GetRootIndex() + if refIdx != nil && refIdx.GetSpecAbsolutePath() == rootIdx.GetSpecAbsolutePath() { + // This ref points to the root document + if fragment != "" && strings.HasPrefix(fragment, "#/") { + // Return the fragment as-is - it's already a valid local ref + return fragment + } + } + + // For non-root refs, gate rewrites on processedNodes presence. + // Only rewrite if the target was actually composed into the bundled output. + // This prevents dangling refs when SearchIndexForReference resolves something + // that never made it into processedNodes. + if processedNodes.GetOrZero(absoluteKey) == nil { + return refValue + } + + // Use renameRef() which handles collision renames + return renameRef(refIdx, absoluteKey, processedNodes) +} diff --git a/bundler/composer_functions_test.go b/bundler/composer_functions_test.go index 83ab3cc1..fe0dc119 100644 --- a/bundler/composer_functions_test.go +++ b/bundler/composer_functions_test.go @@ -1,298 +1,238 @@ -// Copyright 2023-2025 Princess Beef Heavy Industries, LLC / Dave Shanley -// https://pb33f.io - package bundler import ( + "os" + "path/filepath" "testing" - "github.com/pb33f/libopenapi/orderedmap" - "github.com/pb33f/libopenapi" + "github.com/pb33f/libopenapi/datamodel" + highbase "github.com/pb33f/libopenapi/datamodel/high/base" + v3low "github.com/pb33f/libopenapi/datamodel/low/v3" "github.com/pb33f/libopenapi/index" + "github.com/pb33f/libopenapi/orderedmap" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) -func TestProcessRef_UnknownLocation(t *testing.T) { - // create an empty doc - doc, _ := libopenapi.NewDocument([]byte("openapi: 3.1.1")) - m, _ := doc.BuildV3Model() - - ref := &processRef{ - idx: m.Index, - ref: &index.Reference{ - FullDefinition: "#/blarp", - }, - seqRef: nil, - name: "test", - location: []string{"unknown"}, +func TestHandleFileImport_StripsFragment(t *testing.T) { + pr := &processRef{ + ref: &index.Reference{FullDefinition: "/tmp/schemas/Cat.yaml#Cat"}, + seqRef: &index.Reference{}, } - config := &handleIndexConfig{ - compositionConfig: &BundleCompositionConfig{ - Delimiter: "__", - }, - idx: m.Index, - } - - err := processReference(&m.Model, ref, config) + components := orderedmap.New[string, *highbase.SchemaProxy]() + location := handleFileImport(pr, v3low.SchemasLabel, "__", components) - assert.NoError(t, err) - assert.Len(t, config.inlineRequired, 1) + assert.Equal(t, []string{v3low.ComponentsLabel, v3low.SchemasLabel, "Cat"}, location) + assert.Equal(t, "Cat", pr.name) + assert.Equal(t, "Cat", pr.ref.Name) + assert.Equal(t, "Cat", pr.seqRef.Name) } -func TestProcessRef_UnknownLocation_TwoStep(t *testing.T) { - // create an empty doc - doc, _ := libopenapi.NewDocument([]byte("openapi: 3.1.1")) - m, _ := doc.BuildV3Model() - - ref := &processRef{ - idx: m.Index, - ref: &index.Reference{ - FullDefinition: "blip.yaml#/blarp/blop", - }, - seqRef: nil, - name: "test", - location: []string{"unknown"}, - } - - config := &handleIndexConfig{ - compositionConfig: &BundleCompositionConfig{ - Delimiter: "__", - }, - idx: m.Index, - } - - err := processReference(&m.Model, ref, config) - - assert.NoError(t, err) - assert.Len(t, config.inlineRequired, 1) +func TestWalkAndRewriteRefs_NilNode(t *testing.T) { + require.NotPanics(t, func() { + walkAndRewriteRefs(nil, nil, nil, nil, false) + }) } -func TestProcessRef_UnknownLocation_ThreeStep(t *testing.T) { - // create an empty doc - doc, _ := libopenapi.NewDocument([]byte("openapi: 3.1.1")) - m, _ := doc.BuildV3Model() - - ref := &processRef{ - idx: m.Index, - ref: &index.Reference{ - FullDefinition: "bleep.yaml#/blarp/blop/blurp", - Definition: "#/blarp/blop/blurp", - }, - seqRef: nil, - name: "test", - location: []string{"unknown"}, - } +func TestResolveRefToComposed_PreservesExternalRefs(t *testing.T) { + assert.Equal(t, "https://example.com/schema.json", resolveRefToComposed("https://example.com/schema.json", nil, nil, nil)) + assert.Equal(t, "urn:example:thing", resolveRefToComposed("urn:example:thing", nil, nil, nil)) +} - config := &handleIndexConfig{ - compositionConfig: &BundleCompositionConfig{ - Delimiter: "__", - }, - idx: m.Index, - } +func TestResolveRefToComposed_RootFallbackFromExternalSource(t *testing.T) { + _, rolodex := buildResolveRefContext(t) - err := processReference(&m.Model, ref, config) + indexes := rolodex.GetIndexes() + require.NotEmpty(t, indexes) - assert.NoError(t, err) - assert.Len(t, config.inlineRequired, 1) + got := resolveRefToComposed("#/components/schemas/RootThing", indexes[0], orderedmap.New[string, *processRef](), rolodex) + assert.Equal(t, "#/components/schemas/RootThing", got) } -// A component key that contains a dot (“asdf.zxcv”) must *not* be shortened to -// “asdf” when we re-wire references. -func TestRenameRef_KeepsDotInComponentName(t *testing.T) { - spec := []byte(` -openapi: 3.1.0 -info: - title: Test - version: 0.0.0 -components: - schemas: - "asdf.zxcv": - type: object - Foo: - allOf: - - $ref: '#/components/schemas/asdf.zxcv' -`) +func TestResolveRefToComposed_UnindexedLocalRef_NoProcessedNode(t *testing.T) { + rootIdx, rolodex := buildResolveRefContext(t) + refValue := "#/components/schemas/Missing" - doc, err := libopenapi.NewDocument(spec) - assert.NoError(t, err) + got := resolveRefToComposed(refValue, rootIdx, orderedmap.New[string, *processRef](), rolodex) + assert.Equal(t, refValue, got) +} - v3doc, errs := doc.BuildV3Model() - assert.NoError(t, errs) +func TestResolveRefToComposed_UnindexedLocalRef_UsesProcessedNode(t *testing.T) { + rootIdx, rolodex := buildResolveRefContext(t) + refValue := "#/components/schemas/Missing" - idx := v3doc.Model.Index + absKey := rootIdx.GetSpecAbsolutePath() + refValue + require.NotEmpty(t, rootIdx.GetSpecAbsolutePath()) processed := orderedmap.New[string, *processRef]() + processed.Set(absKey, &processRef{name: "Renamed"}) - got := renameRef(idx, "#/components/schemas/asdf.zxcv", processed) + got := resolveRefToComposed(refValue, rootIdx, processed, rolodex) + assert.Equal(t, "#/components/schemas/Renamed", got) +} + +func TestResolveRefToComposed_UnresolvedRef_ReturnsOriginal(t *testing.T) { + rootIdx, rolodex := buildResolveRefContext(t) + refValue := "./missing.yaml" - assert.Equal(t, - "#/components/schemas/asdf.zxcv", - got, - "renameRef must not strip the .zxcv part from the component key") + got := resolveRefToComposed(refValue, rootIdx, orderedmap.New[string, *processRef](), rolodex) + assert.Equal(t, refValue, got) } -// A reference that really *is* a filename + JSON-pointer must still have the -// extension stripped -func TestRenameRef_FilePointer_Extensions(t *testing.T) { - exts := []string{".yaml", ".yml", ".json"} +func TestResolveRefToComposed_FindsInOtherIndexes(t *testing.T) { + rootIdx, rolodex, idxA, idxB := buildMultiIndexResolveContext(t) + require.NotNil(t, rootIdx) + require.NotNil(t, idxA) + require.NotNil(t, idxB) - for _, ext := range exts { - def := "schemas/pet" + ext + "#/components/schemas/Pet" - got := renameRef(nil, def, orderedmap.New[string, *processRef]()) - assert.Equal(t, "#/components/schemas/Pet", got, - "extension %s should not affect the pointer rewrite", ext) + refValue := "./B.yaml#/components/schemas/BThing" + if r, _ := idxB.SearchIndexForReference(refValue); r == nil { + t.Fatalf("expected idxB to resolve %s", refValue) + } + if r, _ := rootIdx.SearchIndexForReference(refValue); r != nil { + t.Fatalf("expected root index to NOT resolve %s", refValue) } + + got := resolveRefToComposed(refValue, rootIdx, orderedmap.New[string, *processRef](), rolodex) + assert.Equal(t, refValue, got) } -// If a component name has already been changed during composition, -// renameRef must return that new name. -func TestRenameRef_RespectsAlreadyRenamedComponent(t *testing.T) { - ps := orderedmap.New[string, *processRef]() - ps.Set("#/components/schemas/asdf.zxcv", - &processRef{name: "asdf__1", location: []string{}}) - - got := renameRef(nil, - "#/components/schemas/asdf.zxcv", - ps) - - assert.Equal(t, - "#/components/schemas/asdf__1", - got, - "renameRef should use the name stored in processedNodes") +func TestRenameRef_FallbackKeepsLastSegment(t *testing.T) { + def := "/tmp/spec.yaml#/components/schemas/Thing" + got := renameRef(nil, def, orderedmap.New[string, *processRef]()) + assert.Equal(t, "#/components/schemas/Thing", got) } -func TestRenameRef_RootFileImport(t *testing.T) { - processed := orderedmap.New[string, *processRef]() - processed.Set("schemas/pet.yaml", - &processRef{location: []string{"components", "schemas", "Pet"}}) +func buildResolveRefContext(t *testing.T) (*index.SpecIndex, *index.Rolodex) { + tmpDir := t.TempDir() - got := renameRef(nil, "schemas/pet.yaml", processed) + rootSpec := `openapi: 3.1.0 +info: + title: Ref Context + version: 1.0.0 +paths: + /ext: + get: + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: './schemas/External.yaml' +components: + schemas: + RootThing: + type: object` - assert.Equal(t, "#/components/schemas/Pet", got) -} + extSchema := `type: object +properties: + id: + type: string` -// A JSON-pointer that has only one segment (e.g. "#/Foo") and was NOT processed -// must be returned unchanged -func TestRenameRef_ShortPointerUnprocessedIsReturnedUnchanged(t *testing.T) { - got := renameRef(nil, "#/Foo", orderedmap.New[string, *processRef]()) - assert.Equal(t, "#/Foo", got) -} + require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, "schemas"), 0755)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "root.yaml"), []byte(rootSpec), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "schemas", "External.yaml"), []byte(extSchema), 0644)) -// A JSON-pointer that has only one segment (e.g. "#/Foo") and WAS processed -// must return the new component path -func TestRenameRef_ShortPointerProcessedReturnsComponentPath(t *testing.T) { - processed := orderedmap.New[string, *processRef]() - processed.Set("child.yaml#/NonRequired", - &processRef{ - name: "NonRequired", - location: []string{"components", "schemas", "NonRequired"}, - }) - - got := renameRef(nil, "child.yaml#/NonRequired", processed) - assert.Equal(t, "#/components/schemas/NonRequired", got) -} + specBytes, err := os.ReadFile(filepath.Join(tmpDir, "root.yaml")) + require.NoError(t, err) -// Test renameRef with a single-segment pointer that has a file prefix -func TestRenameRef_SingleSegmentWithFilePrefixProcessed(t *testing.T) { - processed := orderedmap.New[string, *processRef]() - processed.Set("schemas/pet.yaml#/Pet", - &processRef{ - name: "Pet", - location: []string{"components", "schemas", "Pet"}, - }) - - got := renameRef(nil, "schemas/pet.yaml#/Pet", processed) - assert.Equal(t, "#/components/schemas/Pet", got) -} + cfg := datamodel.NewDocumentConfiguration() + cfg.BasePath = tmpDir + cfg.SpecFilePath = "root.yaml" + cfg.AllowFileReferences = true -// Test isOpenAPIRootKey helper function - case-sensitive check -func TestIsOpenAPIRootKey(t *testing.T) { - // Root keys that should return true (exact lowercase match) - rootKeys := []string{ - "openapi", "info", "jsonSchemaDialect", "servers", "paths", - "webhooks", "components", "security", "tags", "externalDocs", - } - for _, key := range rootKeys { - assert.True(t, isOpenAPIRootKey(key), "isOpenAPIRootKey(%q) should return true", key) - } + doc, err := libopenapi.NewDocumentWithConfiguration(specBytes, cfg) + require.NoError(t, err) - // Non-root keys that should return false (including case variations) - // This allows component names like "Paths" or "INFO" to be recomposed - nonRootKeys := []string{ - "Pet", "User", "NonRequired", "MySchema", "Response200", - "foo", "bar", "SomeRandomKey", - // Case variations of root keys - should NOT match (allows component names) - "OPENAPI", "INFO", "PATHS", "Servers", "Components", "Paths", - } - for _, key := range nonRootKeys { - assert.False(t, isOpenAPIRootKey(key), "isOpenAPIRootKey(%q) should return false", key) - } -} + v3Doc, err := doc.BuildV3Model() + require.NoError(t, err) + require.NotNil(t, v3Doc) -// Test JSON Pointer encoding helper functions -func TestEncodeJSONPointerSegment(t *testing.T) { - tests := []struct { - input string - expected string - }{ - {"simple", "simple"}, - {"foo/bar", "foo~1bar"}, - {"foo~bar", "foo~0bar"}, - {"foo~/bar", "foo~0~1bar"}, - {"a/b/c", "a~1b~1c"}, - {"~0~1", "~00~01"}, - {"", ""}, - } - for _, tc := range tests { - got := encodeJSONPointerSegment(tc.input) - assert.Equal(t, tc.expected, got, "encodeJSONPointerSegment(%q)", tc.input) + if v3Doc.Index == nil || v3Doc.Index.GetRolodex() == nil { + t.Fatalf("expected index and rolodex to be initialized") } -} -func TestJoinLocationAsJSONPointer(t *testing.T) { - tests := []struct { - location []string - expected string - }{ - {[]string{"components", "schemas", "Pet"}, "components/schemas/Pet"}, - {[]string{"components", "schemas", "Foo/Bar"}, "components/schemas/Foo~1Bar"}, - {[]string{"components", "schemas", "Foo~Bar"}, "components/schemas/Foo~0Bar"}, - {[]string{"components", "schemas", "a/b~c"}, "components/schemas/a~1b~0c"}, - {[]string{}, ""}, - } - for _, tc := range tests { - got := joinLocationAsJSONPointer(tc.location) - assert.Equal(t, tc.expected, got, "joinLocationAsJSONPointer(%v)", tc.location) - } + return v3Doc.Index, v3Doc.Index.GetRolodex() } -// Test renameRef properly encodes JSON Pointer escapes -func TestRenameRef_JSONPointerEscapeRoundTrip(t *testing.T) { - // Component name contains "/" - must be escaped as ~1 - processed := orderedmap.New[string, *processRef]() - processed.Set("child.yaml#/Foo~1Bar", - &processRef{ - name: "Foo/Bar", // decoded name - location: []string{"components", "schemas", "Foo/Bar"}, - }) - - got := renameRef(nil, "child.yaml#/Foo~1Bar", processed) - assert.Equal(t, "#/components/schemas/Foo~1Bar", got, - "renameRef should re-encode / as ~1 in component name") -} +func buildMultiIndexResolveContext(t *testing.T) (*index.SpecIndex, *index.Rolodex, *index.SpecIndex, *index.SpecIndex) { + tmpDir := t.TempDir() -func TestRenameRef_JSONPointerTildeEscapeRoundTrip(t *testing.T) { - // Component name contains "~" - must be escaped as ~0 - processed := orderedmap.New[string, *processRef]() - processed.Set("child.yaml#/Foo~0Bar", - &processRef{ - name: "Foo~Bar", // decoded name - location: []string{"components", "schemas", "Foo~Bar"}, - }) - - got := renameRef(nil, "child.yaml#/Foo~0Bar", processed) - assert.Equal(t, "#/components/schemas/Foo~0Bar", got, - "renameRef should re-encode ~ as ~0 in component name") + rootSpec := `openapi: 3.1.0 +info: + title: Multi Index + version: 1.0.0 +paths: + /a: + get: + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: './schemas/A.yaml#/components/schemas/AThing' + /b: + get: + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: './schemas/B.yaml#/components/schemas/BThing'` + + aSchema := `openapi: 3.1.0 +info: + title: A + version: 1.0.0 +components: + schemas: + AThing: + type: object` + + bSchema := `openapi: 3.1.0 +info: + title: B + version: 1.0.0 +components: + schemas: + BThing: + type: object` + + require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, "schemas"), 0755)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "root.yaml"), []byte(rootSpec), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "schemas", "A.yaml"), []byte(aSchema), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "schemas", "B.yaml"), []byte(bSchema), 0644)) + + specBytes, err := os.ReadFile(filepath.Join(tmpDir, "root.yaml")) + require.NoError(t, err) + + cfg := datamodel.NewDocumentConfiguration() + cfg.BasePath = tmpDir + cfg.SpecFilePath = "root.yaml" + cfg.AllowFileReferences = true + + doc, err := libopenapi.NewDocumentWithConfiguration(specBytes, cfg) + require.NoError(t, err) + + v3Doc, err := doc.BuildV3Model() + require.NoError(t, err) + require.NotNil(t, v3Doc) + + rolodex := v3Doc.Index.GetRolodex() + var idxA, idxB *index.SpecIndex + for _, idx := range rolodex.GetIndexes() { + switch filepath.Base(idx.GetSpecAbsolutePath()) { + case "A.yaml": + idxA = idx + case "B.yaml": + idxB = idx + } + } + + return v3Doc.Index, rolodex, idxA, idxB } diff --git a/bundler/detect_type.go b/bundler/detect_type.go index f242056c..8fdf2cc9 100644 --- a/bundler/detect_type.go +++ b/bundler/detect_type.go @@ -150,6 +150,9 @@ func hasPathItemProperties(node *yaml.Node) bool { // Helper function to get all keys from a mapping node func getNodeKeys(node *yaml.Node) []string { + if node.Kind == yaml.DocumentNode && len(node.Content) > 0 { + node = node.Content[0] + } if node.Kind != yaml.MappingNode { return nil } diff --git a/bundler/detect_type_test.go b/bundler/detect_type_test.go index dc9a87ca..45e94c0c 100644 --- a/bundler/detect_type_test.go +++ b/bundler/detect_type_test.go @@ -524,6 +524,13 @@ key3: value3 keys := getNodeKeys(node) assert.ElementsMatch(t, []string{"key1", "key2", "key3"}, keys) + // Test with document node + var docNode yaml.Node + err := yaml.Unmarshal([]byte(yamlStr), &docNode) + assert.NoError(t, err) + keys = getNodeKeys(&docNode) + assert.ElementsMatch(t, []string{"key1", "key2", "key3"}, keys) + // Test with sequence node yamlStr = ` - item1 diff --git a/bundler/origin_test.go b/bundler/origin_test.go index 4abab909..69c9eb0a 100644 --- a/bundler/origin_test.go +++ b/bundler/origin_test.go @@ -514,6 +514,21 @@ func TestBundleBytesComposedWithOrigins_ErrorHandling(t *testing.T) { }) } +func TestBundleBytesComposedWithOrigins_ErrorModel(t *testing.T) { + specBytes := []byte(`openapi: 3.1.0 +info: + title: Error Model + version: 1.0.0 +paths: + /cake: + $ref: '#/components/schemas/Cake'`) + + result, err := BundleBytesComposedWithOrigins(specBytes, nil, nil) + + assert.Error(t, err) + assert.Nil(t, result) +} + func TestBundleBytesComposedWithOrigins_LineAndColumnTracking(t *testing.T) { tmpDir := t.TempDir() diff --git a/datamodel/low/base/schema_test.go b/datamodel/low/base/schema_test.go index 6fcc4cb5..80cac525 100644 --- a/datamodel/low/base/schema_test.go +++ b/datamodel/low/base/schema_test.go @@ -1499,6 +1499,7 @@ func TestSchema_Hash_Equal(t *testing.T) { } func TestSchema_Hash_AdditionalPropsSlice(t *testing.T) { + low.ClearHashCache() left := `schema: additionalProperties: - type: string` @@ -1524,6 +1525,7 @@ func TestSchema_Hash_AdditionalPropsSlice(t *testing.T) { } func TestSchema_Hash_AdditionalPropsSliceNoMap(t *testing.T) { + low.ClearHashCache() left := `schema: additionalProperties: - hello` @@ -1549,6 +1551,7 @@ func TestSchema_Hash_AdditionalPropsSliceNoMap(t *testing.T) { } func TestSchema_Hash_NotEqual(t *testing.T) { + low.ClearHashCache() left := `schema: title: an OK message - but different items: true diff --git a/datamodel/low/extraction_functions.go b/datamodel/low/extraction_functions.go index 90bbe42b..04b9455b 100644 --- a/datamodel/low/extraction_functions.go +++ b/datamodel/low/extraction_functions.go @@ -11,6 +11,7 @@ import ( "fmt" "hash" "net/url" + "os" "path/filepath" "reflect" "sort" @@ -180,7 +181,7 @@ func LocateRefNodeWithContext(ctx context.Context, root *yaml.Node, idx *index.S if p != "" && explodedRefValue[0] != "" { // We are resolving the relative URL against the absolute URL of // the spec containing the reference. - u.Path = utils.ReplaceWindowsDriveWithLinuxPath(filepath.Join(p, explodedRefValue[0])) + u.Path = utils.ReplaceWindowsDriveWithLinuxPath(utils.CheckPathOverlap(p, explodedRefValue[0], string(os.PathSeparator))) } u.Fragment = "" // Turn the reference value [rv] into the absolute filepath/URL we @@ -201,13 +202,13 @@ func LocateRefNodeWithContext(ctx context.Context, root *yaml.Node, idx *index.S sp := strings.Split(specPath, "#") // Create a clean (absolute?) path to the file containing the // referenced value. - abs, _ = filepath.Abs(filepath.Join(filepath.Dir(sp[0]), explodedRefValue[0])) + abs, _ = filepath.Abs(utils.CheckPathOverlap(filepath.Dir(sp[0]), explodedRefValue[0], string(os.PathSeparator))) } rv = fmt.Sprintf("%s#%s", abs, explodedRefValue[1]) } else { // We don't have a path for the schema we are trying to resolve // relative references from. This likely happens when the schema - // is the root schema, i.e. the file given to libopenapi as entry. + // is the root schema, i.e., the file given to libopenapi as an entry. // // check for a config BaseURL and use that if it exists. @@ -218,7 +219,7 @@ func LocateRefNodeWithContext(ctx context.Context, root *yaml.Node, idx *index.S p = u.Path } - u.Path = utils.ReplaceWindowsDriveWithLinuxPath(filepath.Join(p, explodedRefValue[0])) + u.Path = utils.ReplaceWindowsDriveWithLinuxPath(utils.CheckPathOverlap(p, explodedRefValue[0], string(os.PathSeparator))) rv = fmt.Sprintf("%s#%s", u.String(), explodedRefValue[1]) } } @@ -231,21 +232,21 @@ func LocateRefNodeWithContext(ctx context.Context, root *yaml.Node, idx *index.S if strings.HasPrefix(specPath, "http") { u, _ := url.Parse(specPath) p := filepath.Dir(u.Path) - abs, _ := filepath.Abs(filepath.Join(p, rv)) + abs, _ := filepath.Abs(utils.CheckPathOverlap(p, rv, string(os.PathSeparator))) u.Path = utils.ReplaceWindowsDriveWithLinuxPath(abs) rv = u.String() } else { if specPath != "" { - abs, _ := filepath.Abs(filepath.Join(filepath.Dir(specPath), rv)) + abs, _ := filepath.Abs(utils.CheckPathOverlap(filepath.Dir(specPath), rv, string(os.PathSeparator))) rv = abs } else { // check for a config baseURL and use that if it exists. if idx.GetConfig().BaseURL != nil { u := *idx.GetConfig().BaseURL - abs, _ := filepath.Abs(filepath.Join(u.Path, rv)) + abs, _ := filepath.Abs(utils.CheckPathOverlap(u.Path, rv, string(os.PathSeparator))) u.Path = utils.ReplaceWindowsDriveWithLinuxPath(abs) rv = u.String() } diff --git a/index/rolodex.go b/index/rolodex.go index 9027d7f0..1e720383 100644 --- a/index/rolodex.go +++ b/index/rolodex.go @@ -596,7 +596,7 @@ func (r *Rolodex) OpenWithContext(ctx context.Context, location string) (Rolodex // check if this is a URL or an abs/rel reference. if !filepath.IsAbs(location) { - fileLookup, _ = filepath.Abs(filepath.Join(k, location)) + fileLookup, _ = filepath.Abs(utils.CheckPathOverlap(k, location, string(os.PathSeparator))) } // For generic fs.FS implementations, we need to use relative paths diff --git a/index/rolodex_file_loader.go b/index/rolodex_file_loader.go index c7126feb..9de01166 100644 --- a/index/rolodex_file_loader.go +++ b/index/rolodex_file_loader.go @@ -19,6 +19,7 @@ import ( "context" "github.com/pb33f/libopenapi/datamodel" + "github.com/pb33f/libopenapi/utils" "go.yaml.in/yaml/v4" ) @@ -85,7 +86,7 @@ func (l *LocalFS) OpenWithContext(ctx context.Context, name string) (fs.File, er } if !filepath.IsAbs(name) { - name, _ = filepath.Abs(filepath.Join(l.baseDirectory, name)) + name, _ = filepath.Abs(utils.CheckPathOverlap(l.baseDirectory, name, string(os.PathSeparator))) } if f, ok := l.Files.Load(name); ok { @@ -483,7 +484,7 @@ func (l *LocalFS) extractFile(p string) (*LocalFile, error) { config := l.fsConfig if !filepath.IsAbs(p) { if config != nil && config.BaseDirectory != "" { - abs, _ = filepath.Abs(filepath.Join(config.BaseDirectory, p)) + abs, _ = filepath.Abs(utils.CheckPathOverlap(config.BaseDirectory, p, string(os.PathSeparator))) } else { abs, _ = filepath.Abs(p) } diff --git a/index/search_index.go b/index/search_index.go index d3ad0e3a..0f49745b 100644 --- a/index/search_index.go +++ b/index/search_index.go @@ -7,8 +7,11 @@ import ( "context" "fmt" "net/url" + "os" "path/filepath" "strings" + + "github.com/pb33f/libopenapi/utils" ) type ContextKey string @@ -133,7 +136,7 @@ func (index *SpecIndex) SearchIndexForReferenceByReferenceWithContext(ctx contex if filepath.Ext(absPath) != "" { absPath = filepath.Dir(absPath) } - roloLookup, _ = filepath.Abs(filepath.Join(absPath, uri[0])) + roloLookup, _ = filepath.Abs(utils.CheckPathOverlap(absPath, uri[0], string(os.PathSeparator))) } } } else { @@ -158,7 +161,7 @@ func (index *SpecIndex) SearchIndexForReferenceByReferenceWithContext(ctx contex if filepath.Ext(absPath) != "" { absPath = filepath.Dir(absPath) } - roloLookup, _ = filepath.Abs(filepath.Join(absPath, uri[0])) + roloLookup, _ = filepath.Abs(utils.CheckPathOverlap(absPath, uri[0], string(os.PathSeparator))) } } ref = uri[0] diff --git a/utils/utils.go b/utils/utils.go index 91c5b4f1..fc8e4094 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -605,6 +605,25 @@ func IsNodeRefValue(node *yaml.Node) (bool, *yaml.Node, string) { return false, nil, "" } +// GetRefValueNode returns the $ref value node from a mapping node. +// Unlike IsNodeRefValue which returns the string value, this returns the actual node +// so it can be modified in place. This correctly handles OA 3.1 sibling properties +// where $ref may not be at position 0. +func GetRefValueNode(node *yaml.Node) *yaml.Node { + if node == nil { + return nil + } + n := NodeAlias(node) + for i, r := range n.Content { + if i%2 == 0 && r.Value == "$ref" { + if i+1 < len(n.Content) { + return n.Content[i + 1] + } + } + } + return nil +} + // FixContext will clean up a JSONpath string to be correctly traversable. func FixContext(context string) string { tokens := strings.Split(context, ".") diff --git a/utils/utils_test.go b/utils/utils_test.go index 31726e48..dc18c3a2 100644 --- a/utils/utils_test.go +++ b/utils/utils_test.go @@ -1756,3 +1756,81 @@ func TestConvertComponentIdIntoFriendlyPathSearch_DefensiveCodeDocumentation(t * // This assertion should always pass, demonstrating why the defensive code is rarely executed assert.True(t, allProperlyFormatted, "All paths should start with $ due to the function's design") } + +func TestGetRefValueNode_NilNode(t *testing.T) { + result := GetRefValueNode(nil) + assert.Nil(t, result) +} + +func TestGetRefValueNode_SimpleRef(t *testing.T) { + // YAML node with $ref at position 1 (standard case) + node := &yaml.Node{ + Kind: yaml.MappingNode, + Content: []*yaml.Node{ + {Kind: yaml.ScalarNode, Value: "$ref"}, + {Kind: yaml.ScalarNode, Value: "#/components/schemas/Pet"}, + }, + } + result := GetRefValueNode(node) + assert.NotNil(t, result) + assert.Equal(t, "#/components/schemas/Pet", result.Value) +} + +func TestGetRefValueNode_RefWithSiblingProperties(t *testing.T) { + // OpenAPI 3.1 style: $ref with sibling properties (description comes before $ref) + node := &yaml.Node{ + Kind: yaml.MappingNode, + Content: []*yaml.Node{ + {Kind: yaml.ScalarNode, Value: "description"}, + {Kind: yaml.ScalarNode, Value: "A pet description"}, + {Kind: yaml.ScalarNode, Value: "$ref"}, + {Kind: yaml.ScalarNode, Value: "#/components/schemas/Pet"}, + }, + } + result := GetRefValueNode(node) + assert.NotNil(t, result) + assert.Equal(t, "#/components/schemas/Pet", result.Value) +} + +func TestGetRefValueNode_RefAtEnd(t *testing.T) { + // $ref at the end of multiple properties + node := &yaml.Node{ + Kind: yaml.MappingNode, + Content: []*yaml.Node{ + {Kind: yaml.ScalarNode, Value: "description"}, + {Kind: yaml.ScalarNode, Value: "Description text"}, + {Kind: yaml.ScalarNode, Value: "summary"}, + {Kind: yaml.ScalarNode, Value: "Summary text"}, + {Kind: yaml.ScalarNode, Value: "$ref"}, + {Kind: yaml.ScalarNode, Value: "./external.yaml#/components/schemas/Item"}, + }, + } + result := GetRefValueNode(node) + assert.NotNil(t, result) + assert.Equal(t, "./external.yaml#/components/schemas/Item", result.Value) +} + +func TestGetRefValueNode_NoRef(t *testing.T) { + // Node without $ref + node := &yaml.Node{ + Kind: yaml.MappingNode, + Content: []*yaml.Node{ + {Kind: yaml.ScalarNode, Value: "type"}, + {Kind: yaml.ScalarNode, Value: "object"}, + {Kind: yaml.ScalarNode, Value: "description"}, + {Kind: yaml.ScalarNode, Value: "A schema without ref"}, + }, + } + result := GetRefValueNode(node) + assert.Nil(t, result) +} + +func TestGetRefValueNode_EmptyNode(t *testing.T) { + // Empty mapping node + node := &yaml.Node{ + Kind: yaml.MappingNode, + Content: []*yaml.Node{}, + } + result := GetRefValueNode(node) + assert.Nil(t, result) +} diff --git a/utils/windows_drive.go b/utils/windows_drive.go index c5b5221b..71ac0df6 100644 --- a/utils/windows_drive.go +++ b/utils/windows_drive.go @@ -15,6 +15,8 @@ func ReplaceWindowsDriveWithLinuxPath(path string) string { // CheckPathOverlap joins pathA and pathB while avoiding duplicated overlapping segments. // It tolerates mixed separators in the inputs and uses OS-specific path comparison rules. +// It also handles leading ".." segments in pathB by first resolving them against pathA +// before checking for overlap, preventing path doubling issues. func CheckPathOverlap(pathA, pathB, sep string) string { if sep == "" { sep = string(filepath.Separator) @@ -27,10 +29,47 @@ func CheckPathOverlap(pathA, pathB, sep string) string { }) } + // preserve path prefix (absolute path marker or Windows drive letter) + prefix := "" + if len(pathA) > 0 && (pathA[0] == '/' || pathA[0] == '\\') { + prefix = string(pathA[0]) + } + if len(pathA) > 2 && pathA[1] == ':' { + prefix = pathA[:2] + sep + } + aParts := split(pathA) bParts := split(pathB) - // Find the longest suffix of aParts that matches the prefix of bParts. + // If pathA has a Windows drive letter, drop it from aParts + // to avoid duplicating it when rebuilding with prefix. + if len(pathA) > 2 && pathA[1] == ':' { + if len(aParts) > 0 && strings.EqualFold(aParts[0], pathA[:2]) { + aParts = aParts[1:] + } + } + + // Handle leading ".." segments in pathB by going up pathA. + // This prevents path doubling when resolving refs like "../components/schemas/X.yaml" + // from a base path like "/path/to/components/schemas". + dotDotResolved := false + for len(bParts) > 0 && bParts[0] == ".." && len(aParts) > 0 { + aParts = aParts[:len(aParts)-1] // Go up one directory + bParts = bParts[1:] // Remove the ".." + dotDotResolved = true + } + + // rebuild pathA if we resolved any ".." segments + if dotDotResolved { + if len(aParts) == 0 { + pathA = prefix + } else { + // Use the OS separator to join parts, then add a prefix + pathA = prefix + strings.Join(aParts, string(filepath.Separator)) + } + } + + // find the longest suffix of aParts that matches the prefix of bParts. overlap := 0 maxCheck := len(aParts) if len(bParts) < maxCheck { diff --git a/utils/windows_drive_test.go b/utils/windows_drive_test.go index dcb42b24..253f54c9 100644 --- a/utils/windows_drive_test.go +++ b/utils/windows_drive_test.go @@ -4,6 +4,7 @@ import ( "os" "path/filepath" "runtime" + "strings" "testing" ) @@ -148,3 +149,110 @@ func TestCheckPathOverlap_CaseInsensitiveWindows(t *testing.T) { t.Errorf("Expected %s, got %s", expected, result) } } + +// TestCheckPathOverlap_DotDotResolution tests that leading ".." segments in pathB +// are resolved against pathA before checking for overlap. This prevents path doubling +// issues when resolving refs like "../components/schemas/X.yaml" from a base path +// like "/path/to/components/schemas". +func TestCheckPathOverlap_DotDotResolution(t *testing.T) { + // This is the key scenario that caused path doubling bugs: + // - Base path is inside components/schemas (e.g., from User.yaml) + // - Ref goes up with ".." then back into components/schemas + // - Without proper ".." handling, we'd get components/components/schemas/... + pathA := filepath.Join("tmp", "spec", "components", "schemas") + pathB := "../components/schemas/Admin.yaml" + expected := filepath.Join("tmp", "spec", "components", "schemas", "Admin.yaml") + result := CheckPathOverlap(pathA, pathB, string(os.PathSeparator)) + if result != expected { + t.Errorf("Expected %s, got %s", expected, result) + } +} + +func TestCheckPathOverlap_DotDotMultipleLevels(t *testing.T) { + // Multiple ".." segments going up multiple levels + pathA := filepath.Join("tmp", "spec", "components", "schemas", "nested") + pathB := "../../responses/Problem.yaml" + expected := filepath.Join("tmp", "spec", "components", "responses", "Problem.yaml") + result := CheckPathOverlap(pathA, pathB, string(os.PathSeparator)) + if result != expected { + t.Errorf("Expected %s, got %s", expected, result) + } +} + +func TestCheckPathOverlap_DotDotWithOverlap(t *testing.T) { + // ".." followed by overlap detection + pathA := filepath.Join("tmp", "spec", "paths") + pathB := "../components/schemas/User.yaml" + expected := filepath.Join("tmp", "spec", "components", "schemas", "User.yaml") + result := CheckPathOverlap(pathA, pathB, string(os.PathSeparator)) + if result != expected { + t.Errorf("Expected %s, got %s", expected, result) + } +} + +func TestCheckPathOverlap_DotDotAbsolutePath(t *testing.T) { + if runtime.GOOS == "windows" { + pathA := `C:\spec\components\schemas` + pathB := `../components/schemas/Admin.yaml` + expected := `C:\spec\components\schemas\Admin.yaml` + result := CheckPathOverlap(pathA, pathB, string(os.PathSeparator)) + if result != expected { + t.Errorf("Expected %s, got %s", expected, result) + } + } else { + pathA := "/spec/components/schemas" + pathB := "../components/schemas/Admin.yaml" + expected := "/spec/components/schemas/Admin.yaml" + result := CheckPathOverlap(pathA, pathB, string(os.PathSeparator)) + if result != expected { + t.Errorf("Expected %s, got %s", expected, result) + } + } +} + +// TestCheckPathOverlap_DotDotConsumesAllPathA verifies that when ".." segments +// consume ALL of pathA, the result is just pathB (without the consumed ".."). +// This covers line 56-58 in windows_drive.go (len(aParts) == 0 branch). +func TestCheckPathOverlap_DotDotConsumesAllPathA(t *testing.T) { + // Single segment pathA consumed by ".." + result := CheckPathOverlap("schemas", "../file.yaml", "/") + expected := filepath.Join("file.yaml") + if result != expected { + t.Errorf("Expected %s, got %s", expected, result) + } +} + +func TestCheckPathOverlap_DotDotConsumesMultipleSegments(t *testing.T) { + // Two segments pathA, two ".." in pathB - consumes all of pathA + result := CheckPathOverlap("a/b", "../../file.yaml", "/") + expected := filepath.Join("file.yaml") + if result != expected { + t.Errorf("Expected %s, got %s", expected, result) + } +} + +func TestCheckPathOverlap_DotDotExceedsPathA(t *testing.T) { + // More ".." than pathA segments - should stop at root + result := CheckPathOverlap("a", "../../file.yaml", "/") + expected := filepath.Join("..", "file.yaml") + if result != expected { + t.Errorf("Expected %s, got %s", expected, result) + } +} + +// TestCheckPathOverlap_WindowsDriveLetter tests Windows drive letter detection. +// This covers lines 37-39 in windows_drive.go. +func TestCheckPathOverlap_WindowsDriveLetter(t *testing.T) { + // Test Windows path parsing with drive letter + // Note: This uses backslash separator to simulate Windows paths + pathA := `C:\Users\test` + pathB := `test\file.yaml` + result := CheckPathOverlap(pathA, pathB, `\`) + // Should preserve C: prefix and handle overlap + if !strings.HasPrefix(result, `C:`) { + t.Errorf("Expected result to start with C:, got %s", result) + } + if !strings.Contains(result, "file.yaml") { + t.Errorf("Expected result to contain file.yaml, got %s", result) + } +} From fb837a73ea7333fb27e20f05c9ba15c3eed2abbb Mon Sep 17 00:00:00 2001 From: quobix Date: Mon, 2 Feb 2026 18:04:28 -0500 Subject: [PATCH 6/6] updated coverage --- bundler/bundler_composer_test.go | 78 ++++++++++++++++ bundler/bundler_ref_rewrite_test.go | 2 +- bundler/bundler_strict_validation_test.go | 107 ++++++++++++++++++++++ 3 files changed, 186 insertions(+), 1 deletion(-) diff --git a/bundler/bundler_composer_test.go b/bundler/bundler_composer_test.go index 1d9e942e..9e22aa71 100644 --- a/bundler/bundler_composer_test.go +++ b/bundler/bundler_composer_test.go @@ -223,6 +223,84 @@ func TestDeriveNameFromFullDefinition_StripsFragment(t *testing.T) { assert.Equal(t, "Cat", name) } +func TestResolveDiscriminatorMappingTarget_NilSourceIdx(t *testing.T) { + ref, idx := resolveDiscriminatorMappingTarget(nil, "schemas/Cat.yaml") + assert.Nil(t, ref) + assert.Nil(t, idx) +} + +func TestResolveDiscriminatorMappingTarget_NoRolodex(t *testing.T) { + var rootNode yaml.Node + require.NoError(t, yaml.Unmarshal([]byte(`openapi: 3.0.0 +info: + title: No Rolodex + version: 1.0.0 +paths: {}`), &rootNode)) + + cfg := index.CreateOpenAPIIndexConfig() + sourceIdx := index.NewSpecIndexWithConfig(&rootNode, cfg) + + ref, idx := resolveDiscriminatorMappingTarget(sourceIdx, "schemas/Cat.yaml") + assert.Nil(t, ref) + assert.Nil(t, idx) +} + +func TestResolveDiscriminatorMappingTarget_IndexesAndReturnsRef(t *testing.T) { + tmpDir := t.TempDir() + + require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, "schemas"), 0755)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "schemas", "Cat.yaml"), []byte("type: object\n"), 0644)) + + var rootNode yaml.Node + require.NoError(t, yaml.Unmarshal([]byte(`openapi: 3.0.0 +info: + title: Root + version: 1.0.0 +paths: {}`), &rootNode)) + + cfg := index.CreateOpenAPIIndexConfig() + cfg.BasePath = tmpDir + cfg.SpecAbsolutePath = "" + + sourceIdx := index.NewSpecIndexWithConfig(&rootNode, cfg) + rolodex := index.NewRolodex(cfg) + + localFS, err := index.NewLocalFSWithConfig(&index.LocalFSConfig{ + BaseDirectory: tmpDir, + IndexConfig: nil, + }) + require.NoError(t, err) + + rolodex.AddLocalFS(tmpDir, localFS) + sourceIdx.SetRolodex(rolodex) + + ref, idx := resolveDiscriminatorMappingTarget(sourceIdx, "schemas/Cat.yaml") + require.NotNil(t, ref) + _ = idx + + expected, err := filepath.Abs(filepath.Join(tmpDir, "schemas", "Cat.yaml")) + require.NoError(t, err) + + assert.Equal(t, expected, ref.FullDefinition) + assert.Equal(t, expected, ref.RemoteLocation) + assert.Equal(t, filepath.Base(expected), ref.Name) + require.NotNil(t, ref.Node) + assert.NotEqual(t, yaml.DocumentNode, ref.Node.Kind) +} + +func TestHandleDiscriminatorMappingIndexes_SkipsRootTarget(t *testing.T) { + rootIdx := &index.SpecIndex{} + cf := &handleIndexConfig{ + idx: rootIdx, + rootIdx: rootIdx, + discriminatorMappings: []*discriminatorMappingWithContext{{targetIdx: rootIdx}}, + } + + err := handleDiscriminatorMappingIndexes(cf, rootIdx, nil) + require.NoError(t, err) + assert.Equal(t, rootIdx, cf.idx) +} + // TestBundleBytesComposed_DiscriminatorMapping tests that composed bundling correctly // updates discriminator mappings when external schemas are moved to components. func TestBundleBytesComposed_DiscriminatorMapping(t *testing.T) { diff --git a/bundler/bundler_ref_rewrite_test.go b/bundler/bundler_ref_rewrite_test.go index 72a30bf8..44206ec1 100644 --- a/bundler/bundler_ref_rewrite_test.go +++ b/bundler/bundler_ref_rewrite_test.go @@ -1,4 +1,4 @@ -// Copyright 2023-2025 Princess Beef Heavy Industries, LLC / Dave Shanley +// Copyright 2023-2026 Princess Beef Heavy Industries, LLC / Dave Shanley // https://pb33f.io // SPDX-License-Identifier: MIT diff --git a/bundler/bundler_strict_validation_test.go b/bundler/bundler_strict_validation_test.go index 09223354..ddf778fa 100644 --- a/bundler/bundler_strict_validation_test.go +++ b/bundler/bundler_strict_validation_test.go @@ -98,6 +98,113 @@ components: ) } +func TestStrictValidation_DiscriminatorMappingTarget_ShouldError(t *testing.T) { + tmpDir := t.TempDir() + + rootSpec := `openapi: 3.0.0 +info: + title: Root + version: 1.0.0 +paths: {} +components: + schemas: + Animal: + type: object + discriminator: + propertyName: kind + mapping: + bad: './bad.yaml#/components/schemas/Bad'` + + badSpec := `openapi: 3.0.0 +info: + title: Bad + version: 1.0.0 +paths: {} +components: + schemas: + Good: + type: object + Bad: + $ref: '#/components/schemas/Good' + description: invalid sibling` + + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "root.yaml"), []byte(rootSpec), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "bad.yaml"), []byte(badSpec), 0644)) + + docConfig := &datamodel.DocumentConfiguration{ + BasePath: tmpDir, + AllowFileReferences: true, + SpecFilePath: "root.yaml", + } + + config := &BundleCompositionConfig{ + StrictValidation: true, + } + + _, err := BundleBytesComposed([]byte(rootSpec), docConfig, config) + + require.Error(t, err) + assert.Contains( + t, + err.Error(), + "invalid OpenAPI 3.0 specification: $ref cannot have sibling properties", + ) +} + +func TestStrictValidation_DiscriminatorMappingTarget_WithOrigins_ShouldError(t *testing.T) { + tmpDir := t.TempDir() + + rootSpec := `openapi: 3.0.0 +info: + title: Root + version: 1.0.0 +paths: {} +components: + schemas: + Animal: + type: object + discriminator: + propertyName: kind + mapping: + bad: './bad.yaml#/components/schemas/Bad'` + + badSpec := `openapi: 3.0.0 +info: + title: Bad + version: 1.0.0 +paths: {} +components: + schemas: + Good: + type: object + Bad: + $ref: '#/components/schemas/Good' + description: invalid sibling` + + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "root.yaml"), []byte(rootSpec), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "bad.yaml"), []byte(badSpec), 0644)) + + docConfig := &datamodel.DocumentConfiguration{ + BasePath: tmpDir, + AllowFileReferences: true, + SpecFilePath: "root.yaml", + } + + config := &BundleCompositionConfig{ + StrictValidation: true, + } + + result, err := BundleBytesComposedWithOrigins([]byte(rootSpec), docConfig, config) + + require.Error(t, err) + assert.Nil(t, result) + assert.Contains( + t, + err.Error(), + "invalid OpenAPI 3.0 specification: $ref cannot have sibling properties", + ) +} + func TestStrictValidation_RefWithoutSiblings_ShouldSucceed(t *testing.T) { spec := `openapi: 3.0.0 info: