diff --git a/AGENTS.md b/AGENTS.md index 850761ea..c354ac0e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -90,7 +90,7 @@ This repo is a library, not an app. The root package exposes the public entry po - **Hash contract**: every schema field must appear in `Schema.hash()` (`datamodel/low/base/schema_hash.go`). A missing field means equality and diff silently ignore it. Call `ClearSchemaQuickHashMap()` between document lifecycles or the global `sync.Map` cache returns stale hashes. - **Circular refs**: `resolver_circular.go` detects loops by comparing `FullDefinition` strings. If ref rewriting (bundler, resolver) changes these inconsistently, loops go undetected and the resolver hangs or overflows the depth limit (500). - **Reference cache staleness**: `index.cache` (`sync.Map`) is never cleared after bundler mutations. Lookups after bundling can return stale pre-rewrite refs pointing to external files that no longer apply. -- **Bundler irreversibility**: `BundleDocument` / `BundleDocumentComposed` mutates the model in-place permanently. Never compare, re-bundle, or re-index a document after bundling — parse fresh from the rendered output instead. +- **Bundler mutation**: `BundleDocument` / `BundleDocumentComposed` mutate the model in place. Inline reference preparation is idempotent, so repeating `BundleDocument` on the same model must not duplicate lifted components. Comparing or re-indexing a mutated model, and repeating composed bundling, remain unsupported; parse fresh from rendered output for those workflows. - **Sibling ref idempotency**: `CreateAllOfStructure()` in `datamodel/low/base/sibling_ref_transformer.go` is not idempotent. Running it twice (e.g., bundle then re-index) produces nested `allOf` wrappers that break schema validity. - **Resolver state leak**: `IgnorePoly` and `IgnoreArray` flags on the resolver persist between parses. Reusing a resolver across documents causes the second document's polymorphic circular refs to be silently missed. diff --git a/bundler/bundler.go b/bundler/bundler.go index 6000c46b..8bdadc5f 100644 --- a/bundler/bundler.go +++ b/bundler/bundler.go @@ -847,19 +847,17 @@ func bundleWithConfig(model *v3.Document, config *BundleInlineConfig, docConfig } 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) + renderCtx := highbase.NewInlineRenderContext() + renderCtx.PreserveLocalComponentRefs = !inlineLocalRefs + renderCtx.StrictCircularReferenceIdentity = true + if model.Rolodex != nil { + renderCtx.RootIndex = model.Rolodex.GetRootIndex() } if model.Rolodex != nil { - // 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) + resolveDiscriminatorTargets := config != nil && config.ResolveDiscriminatorExternalRefs + if err := prepareInlineReferences(model, renderCtx, resolveDiscriminatorTargets); err != nil { + return nil, err } // resolve extension refs before rendering (mutates model's extension nodes in-place) @@ -868,7 +866,7 @@ func bundleWithConfig(model *v3.Document, config *BundleInlineConfig, docConfig } // render inline - discriminator mappings and circular refs are preserved via SchemaProxy.MarshalYAMLInline() - return model.RenderInline() + return model.RenderInlineWithContext(renderCtx) } // externalSchemaRef represents an external schema that needs to be copied to the root document's components. @@ -880,75 +878,6 @@ type externalSchemaRef struct { originalRef string // The original reference string (e.g., "#/components/schemas/Cat") } -// resolveDiscriminatorExternalRefs handles copying external schemas referenced by discriminators -// to the root document's components section and rewrites the references. -func resolveDiscriminatorExternalRefs(model *v3.Document) { - if model == nil || model.Rolodex == nil { - return - } - - rolodex := model.Rolodex - rootIdx := rolodex.GetRootIndex() - - // Collect all external schemas referenced by discriminators - externalSchemas := collectExternalDiscriminatorSchemas(rolodex, rootIdx) - if len(externalSchemas) == 0 { - return - } - - // Ensure model has Components (buildComponents always succeeds with valid rootIdx, - // and rootIdx must be valid since collectExternalDiscriminatorSchemas would panic otherwise) - if model.Components == nil { - model.Components, _ = buildComponents(rootIdx) - } - - // Build existing names map from current components for collision detection - existingNames := make(map[string]bool) - for pair := model.Components.Schemas.First(); pair != nil; pair = pair.Next() { - existingNames[pair.Key()] = true - } - - // Copy schemas to components and build ref mapping - // We need to map both local refs (like #/components/schemas/Cat) and - // external refs (like ./external.yaml#/components/schemas/Cat) to the new location - refMapping := make(map[string]string) - for _, extSchema := range externalSchemas { - // externalSchemas has unique fullDef values (from map iteration in collectExternalDiscriminatorSchemas) - newRef := copySchemaToComponents(model, extSchema, existingNames) - - // Map the local ref format (used in external files) - refMapping[extSchema.originalRef] = newRef - - // Also map external ref formats that might be used in the root document - // e.g., "./vehicles/car.yaml#/components/schemas/Car" - // The external ref format is: relative path from root + JSON pointer - if extSchema.idx != nil { - rootPath := rootIdx.GetSpecAbsolutePath() - extPath := extSchema.idx.GetSpecAbsolutePath() - if rootPath != "" && extPath != "" { - // Calculate relative path from root to external file - relPath, err := filepath.Rel(filepath.Dir(rootPath), extPath) - if err == nil { - // Normalize path separators to forward slashes for cross-platform compatibility - // OpenAPI refs always use forward slashes regardless of OS - relPath = filepath.ToSlash(relPath) - - // Build external ref format: ./relpath#/components/schemas/Name - externalRefFormat := relPath + extSchema.originalRef - refMapping[externalRefFormat] = newRef - // Also try with "./" prefix - if !strings.HasPrefix(relPath, ".") && !strings.HasPrefix(relPath, "/") { - refMapping["./"+externalRefFormat] = newRef - } - } - } - } - } - - // Rewrite discriminator mapping refs and oneOf/anyOf refs - rewriteInlineDiscriminatorRefs(rolodex, refMapping) -} - // collectExternalDiscriminatorSchemas identifies external schemas referenced by discriminators // that need to be copied to the root document's components section. func collectExternalDiscriminatorSchemas(rolodex *index.Rolodex, rootIdx *index.SpecIndex) []*externalSchemaRef { @@ -1011,29 +940,6 @@ func collectExternalDiscriminatorSchemas(rolodex *index.Rolodex, rootIdx *index. return result } -// copySchemaToComponents copies an external schema to the root document's components section. -// Returns the new reference string (e.g., "#/components/schemas/Cat"). -// existingNames is updated with the new name to track collisions across multiple calls. -func copySchemaToComponents(model *v3.Document, extSchema *externalSchemaRef, existingNames map[string]bool) string { - // Build the schema from the YAML node - // extSchema.ref.Node is always valid (validated when collecting external schemas) - schema, _ := buildSchema(extSchema.ref.Node, extSchema.idx) - - // Check for naming collisions and get unique name - finalName := extSchema.schemaName - if existingNames[finalName] { - finalName = calculateCollisionNameInline(finalName, extSchema.fullDef, "__", existingNames) - } - - // Track this name to prevent future collisions - existingNames[finalName] = true - - // Add to components - model.Components.Schemas.Set(finalName, schema) - - return fmt.Sprintf("#/components/schemas/%s", finalName) -} - // calculateCollisionNameInline generates a unique name for a schema to avoid collisions. // It first tries appending the source filename, then falls back to numeric suffixes. func calculateCollisionNameInline(name, fullDef, delimiter string, existingNames map[string]bool) string { @@ -1059,100 +965,6 @@ func calculateCollisionNameInline(name, fullDef, delimiter string, existingNames } } -// rewriteInlineDiscriminatorRefs updates discriminator mapping refs and oneOf/anyOf refs -// to point to the newly copied component locations. -func rewriteInlineDiscriminatorRefs(rolodex *index.Rolodex, refMapping map[string]string) { - if len(refMapping) == 0 { - return - } - - // Collect all discriminator mapping nodes - mappingNodes := collectDiscriminatorMappingNodes(rolodex) - - // Update discriminator mapping values - for _, mappingNode := range mappingNodes { - originalValue := mappingNode.Value - if newRef, ok := refMapping[originalValue]; ok { - mappingNode.Value = newRef - } - } - - // Also update oneOf/anyOf $ref values in all indexes - allIndexes := append(rolodex.GetIndexes(), rolodex.GetRootIndex()) - for _, idx := range allIndexes { - updateOneOfAnyOfRefs(idx.GetRootNode(), refMapping) - } -} - -// updateOneOfAnyOfRefs recursively walks a YAML node tree to update oneOf/anyOf $ref values. -func updateOneOfAnyOfRefs(n *yaml.Node, refMapping map[string]string) { - if n == nil { - return - } - - if n.Kind == yaml.DocumentNode && len(n.Content) > 0 { - n = n.Content[0] - } - - switch n.Kind { - case yaml.SequenceNode: - for _, c := range n.Content { - updateOneOfAnyOfRefs(c, refMapping) - } - return - case yaml.MappingNode: - default: - return - } - - var hasDiscriminator bool - var oneOfNode, anyOfNode *yaml.Node - - // First pass: check for discriminator and find oneOf/anyOf - for i := 0; i < len(n.Content); i += 2 { - k, v := n.Content[i], n.Content[i+1] - switch k.Value { - case "discriminator": - hasDiscriminator = true - case "oneOf": - oneOfNode = v - case "anyOf": - anyOfNode = v - } - } - - // Update refs in oneOf/anyOf if this schema has a discriminator - if hasDiscriminator { - updateUnionRefs(oneOfNode, refMapping) - updateUnionRefs(anyOfNode, refMapping) - } - - // Recursively process all children - for i := 0; i < len(n.Content); i += 2 { - updateOneOfAnyOfRefs(n.Content[i+1], refMapping) - } -} - -// updateUnionRefs updates $ref values in a oneOf or anyOf sequence. -func updateUnionRefs(seq *yaml.Node, refMapping map[string]string) { - if seq == nil || seq.Kind != yaml.SequenceNode { - return - } - for _, item := range seq.Content { - if item.Kind != yaml.MappingNode { - continue - } - for i := 0; i < len(item.Content); i += 2 { - k, v := item.Content[i], item.Content[i+1] - if k.Value == "$ref" && v.Kind == yaml.ScalarNode { - if newRef, ok := refMapping[v.Value]; ok { - v.Value = newRef - } - } - } - } -} - func collectDiscriminatorMappingValues(idx *index.SpecIndex, n *yaml.Node, pinned map[string]struct{}) { if n.Kind == yaml.DocumentNode && len(n.Content) > 0 { n = n.Content[0] @@ -1236,18 +1048,6 @@ func walkUnionRefs(idx *index.SpecIndex, seq *yaml.Node, pinned map[string]struc } } -// collectDiscriminatorMappingNodes gathers all discriminator mapping value nodes from the document tree. -func collectDiscriminatorMappingNodes(rolodex *index.Rolodex) []*yaml.Node { - var mappingNodes []*yaml.Node - - collectDiscriminatorMappingNodesFromIndex(rolodex.GetRootIndex(), rolodex.GetRootIndex().GetRootNode(), &mappingNodes) - for _, idx := range rolodex.GetIndexes() { - collectDiscriminatorMappingNodesFromIndex(idx, idx.GetRootNode(), &mappingNodes) - } - - 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 { @@ -1308,49 +1108,6 @@ func collectDiscriminatorMappingNodesFromIndexWithContext(idx *index.SpecIndex, } } -// 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 { - n = n.Content[0] - } - - switch n.Kind { - case yaml.SequenceNode: - for _, c := range n.Content { - collectDiscriminatorMappingNodesFromIndex(idx, c, mappingNodes) - } - 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 - } - collectDiscriminatorMappingNodesFromIndex(idx, v, mappingNodes) - } - - 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] - if mappingNode.Kind != yaml.MappingNode { - continue - } - for j := 0; j < len(mappingNode.Content); j += 2 { - *mappingNodes = append(*mappingNodes, mappingNode.Content[j+1]) - } - } - } - } -} - // updateDiscriminatorMappingsComposed updates discriminator mapping references to point to composed component locations. func updateDiscriminatorMappingsComposed(mappings []*discriminatorMappingWithContext, processedNodes *orderedmap.Map[string, *processRef], rolodex *index.Rolodex) { for _, mapping := range mappings { diff --git a/bundler/bundler_issue873_test.go b/bundler/bundler_issue873_test.go index 744ac871..95daaac8 100644 --- a/bundler/bundler_issue873_test.go +++ b/bundler/bundler_issue873_test.go @@ -547,9 +547,6 @@ func TestDiscriminatorMappingCollectorsIgnoreMalformedMappingNode(t *testing.T) collectDiscriminatorMappingNodesFromIndexWithContext(nil, parseYAMLNode(t, spec), &mappings) assert.Empty(t, mappings) - var mappingNodes []*yaml.Node - collectDiscriminatorMappingNodesFromIndex(nil, parseYAMLNode(t, spec), &mappingNodes) - assert.Empty(t, mappingNodes) } func lookupMappingNode(t *testing.T, data []byte, path ...string) *yaml.Node { diff --git a/bundler/bundler_test.go b/bundler/bundler_test.go index ed67eefd..8b15df2d 100644 --- a/bundler/bundler_test.go +++ b/bundler/bundler_test.go @@ -25,7 +25,6 @@ import ( "github.com/pb33f/libopenapi/datamodel/high/base" v3high "github.com/pb33f/libopenapi/datamodel/high/v3" "github.com/pb33f/libopenapi/index" - "github.com/pb33f/libopenapi/orderedmap" "github.com/pb33f/libopenapi/utils" "github.com/pb33f/testify/assert" "github.com/pb33f/testify/require" @@ -2262,11 +2261,6 @@ func TestErrorHandlingOnBundleDocument(t *testing.T) { assert.Nil(t, b) assert.Error(t, err) - // resolveDiscriminatorExternalRefs handles nil gracefully (no return value) - resolveDiscriminatorExternalRefs(nil) - - rewriteInlineDiscriminatorRefs(nil, nil) - updateOneOfAnyOfRefs(nil, nil) walkDiscriminatorMapping(nil, &yaml.Node{Kind: yaml.ScalarNode}, nil) // walkUnionRefs: hit first continue (item.Kind != yaml.MappingNode) @@ -2291,27 +2285,6 @@ func TestErrorHandlingOnBundleDocument(t *testing.T) { }, }, nil) - // updateUnionRefs: hit continue (item.Kind != yaml.MappingNode) - updateUnionRefs(&yaml.Node{ - Kind: yaml.SequenceNode, - Content: []*yaml.Node{ - {Kind: yaml.ScalarNode, Value: "not-a-mapping"}, - }, - }, nil) - - // updateUnionRefs: MappingNode but key != "$ref" (skips inner if) - updateUnionRefs(&yaml.Node{ - Kind: yaml.SequenceNode, - Content: []*yaml.Node{ - { - Kind: yaml.MappingNode, - Content: []*yaml.Node{ - {Kind: yaml.ScalarNode, Value: "notRef"}, - {Kind: yaml.ScalarNode, Value: "someValue"}, - }, - }, - }, - }, nil) } func TestResolveDiscriminatorExternalRefs_NoExternalSchemas(t *testing.T) { @@ -2515,42 +2488,6 @@ paths: {}` 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{ - "Cat": true, // Simulate existing schema named "Cat" - } - - // Create a mock external schema ref - extSchema := &externalSchemaRef{ - schemaName: "Cat", - fullDef: "/some/path/external.yaml#/components/schemas/Cat", - ref: &index.Reference{ - Node: &yaml.Node{ - Kind: yaml.MappingNode, - Content: []*yaml.Node{ - {Kind: yaml.ScalarNode, Value: "type"}, - {Kind: yaml.ScalarNode, Value: "object"}, - }, - }, - }, - } - - // Create a minimal document with components - doc := &v3high.Document{ - Components: &v3high.Components{ - Schemas: orderedmap.New[string, *base.SchemaProxy](), - }, - } - - // Copy should handle collision by appending filename - newRef := copySchemaToComponents(doc, extSchema, existingNames) - - // Should have created a collision-avoidance name - assert.Equal(t, "#/components/schemas/Cat__external", newRef) - assert.True(t, existingNames["Cat__external"], "Should track the new name") -} - func TestCalculateCollisionNameInline_NumericSuffix(t *testing.T) { // Test: When filename-based name also collides, use numeric suffix existingNames := map[string]bool{ @@ -3264,7 +3201,9 @@ paths: require.NoError(t, err) bundledStr := string(bundledBytes) - assert.Contains(t, bundledStr, "external.yaml#/components/schemas/Tree") + assert.NotContains(t, bundledStr, "external.yaml") + assert.Contains(t, bundledStr, "$ref: '#/components/schemas/Tree'") + assert.Contains(t, bundledStr, "Tree:") composedBytes, err := BundleBytesComposed(bundledBytes, cfg, nil) require.NoError(t, err) @@ -3308,13 +3247,65 @@ paths: require.NoError(t, err) bundledStr := string(bundledBytes) - assert.Contains(t, bundledStr, "node.yaml") + assert.NotContains(t, bundledStr, "node.yaml") + assert.Contains(t, bundledStr, "$ref: '#/components/schemas/node'") + assert.Equal(t, 1, strings.Count(bundledStr, " node:")) composedBytes, err := BundleBytesComposed(bundledBytes, cfg, nil) require.NoError(t, err) require.NotNil(t, composedBytes) } +func TestIssue928_ExternalSchemaMapCircularRefsAreSelfContained(t *testing.T) { + fixtureDir := filepath.Join("test", "specs", "issue-928") + spec, err := os.ReadFile(filepath.Join(fixtureDir, "api.yaml")) + require.NoError(t, err) + + config := &datamodel.DocumentConfiguration{ + BasePath: fixtureDir, + AllowFileReferences: true, + } + bundled, err := BundleBytes(spec, config) + require.NoError(t, err) + + output := string(bundled) + for _, dangling := range []string{"\"#/Code2Map\"", "'#/Code2Map'", "\"#/Code3Map\"", "'#/Code3Map'", "\"#/CodeUp4Map\"", "'#/CodeUp4Map'"} { + assert.NotContains(t, output, dangling) + } + assert.NotContains(t, output, "CodeXMap.yaml") + + freshConfig := datamodel.NewDocumentConfiguration() + freshConfig.AllowFileReferences = false + fresh, err := libopenapi.NewDocumentWithConfiguration(bundled, freshConfig) + require.NoError(t, err) + freshModel, buildErr := fresh.BuildV3Model() + require.NoError(t, buildErr) + require.NotNil(t, freshModel.Model.Components) + + for _, name := range []string{"Code2Map", "Code3Map", "CodeUp4Map"} { + require.NotNil(t, freshModel.Model.Components.Schemas.GetOrZero(name), "missing lifted schema %s", name) + assert.Equal(t, 1, strings.Count(output, " "+name+":"), "schema %s was lifted more than once", name) + } + + rootIndex := freshModel.Model.Rolodex.GetRootIndex() + for _, ref := range rootIndex.GetRawReferencesSequenced() { + authored := ref.RawRef + if authored == "" { + authored = ref.Definition + } + assert.True(t, strings.HasPrefix(authored, "#/"), "bundle contains non-local ref %q", authored) + resolved, _ := rootIndex.SearchIndexForReference(authored) + require.NotNil(t, resolved, "bundle contains unresolved ref %q", authored) + } + + // Preparation is deterministic across independent parses. + for i := 0; i < 5; i++ { + again, bundleErr := BundleBytes(spec, config) + require.NoError(t, bundleErr) + assert.Equal(t, bundled, again) + } +} + func TestBundleBytes_ExternalPathItemRef_WithLocalComponents(t *testing.T) { tmp := t.TempDir() diff --git a/bundler/inline_references.go b/bundler/inline_references.go new file mode 100644 index 00000000..c3308bf7 --- /dev/null +++ b/bundler/inline_references.go @@ -0,0 +1,313 @@ +// Copyright 2026 Princess B33f Heavy Industries / Dave Shanley +// SPDX-License-Identifier: MIT + +package bundler + +import ( + "fmt" + "net/url" + "path/filepath" + "sort" + "strings" + + highbase "github.com/pb33f/libopenapi/datamodel/high/base" + v3 "github.com/pb33f/libopenapi/datamodel/high/v3" + "github.com/pb33f/libopenapi/index" + "github.com/pb33f/libopenapi/orderedmap" + "github.com/pb33f/libopenapi/utils" + "go.yaml.in/yaml/v4" +) + +type inlineReferenceTarget struct { + fullDefinition string + definition string + index *index.SpecIndex + node *yaml.Node + name string +} + +// prepareInlineReferences lifts external cycle and discriminator targets before +// rendering and records source-qualified preservation and rewrite policy in ctx. +func prepareInlineReferences(model *v3.Document, ctx *highbase.InlineRenderContext, includeDiscriminatorTargets bool) error { + if model == nil || model.Rolodex == nil || ctx == nil { + return nil + } + + targets, err := collectInlineReferenceTargets(model.Rolodex, includeDiscriminatorTargets) + return prepareCollectedInlineReferences(model, ctx, includeDiscriminatorTargets, targets, err) +} + +func prepareCollectedInlineReferences(model *v3.Document, ctx *highbase.InlineRenderContext, includeDiscriminatorTargets bool, targets []*inlineReferenceTarget, collectErr error) error { + if collectErr != nil { + return collectErr + } + if len(targets) == 0 { + if includeDiscriminatorTargets { + return populateInlineDiscriminatorMappingRewrites(model.Rolodex, nil, ctx) + } + return nil + } + + if model.Components == nil { + model.Components, _ = buildComponents(model.Rolodex.GetRootIndex()) + } + if model.Components.Schemas == nil { + model.Components.Schemas = orderedmap.New[string, *highbase.SchemaProxy]() + } + + existingNames := make(map[string]bool, orderedmap.Len(model.Components.Schemas)+len(targets)) + for pair := model.Components.Schemas.First(); pair != nil; pair = pair.Next() { + existingNames[pair.Key()] = true + } + + rewritesByTarget := make(map[string]string, len(targets)) + for _, target := range targets { + componentName := findInlineTargetComponent(model.Components.Schemas, target) + if componentName == "" { + componentName = target.name + if existingNames[componentName] { + componentName = calculateCollisionNameInline(componentName, target.fullDefinition, "__", existingNames) + } + schema, _ := buildSchema(target.node, target.index) + model.Components.Schemas.Set(componentName, schema) + existingNames[componentName] = true + } + rewritesByTarget[target.fullDefinition] = "#/components/schemas/" + encodeJSONPointerSegment(componentName) + } + + populateInlineReferenceContext(model.Rolodex, targets, rewritesByTarget, ctx) + if includeDiscriminatorTargets { + if err := populateInlineDiscriminatorMappingRewrites(model.Rolodex, rewritesByTarget, ctx); err != nil { + return err + } + } + return nil +} + +func collectInlineReferenceTargets(rolodex *index.Rolodex, includeDiscriminatorTargets bool) ([]*inlineReferenceTarget, error) { + if rolodex == nil || rolodex.GetRootIndex() == nil { + return nil, nil + } + + rootPath := index.CanonicalReferenceIdentity(rolodex.GetRootIndex().GetSpecAbsolutePath()) + indexes := append(append([]*index.SpecIndex{}, rolodex.GetIndexes()...), rolodex.GetRootIndex()) + indexesByPath := make(map[string]*index.SpecIndex, len(indexes)) + schemaTargets := make(map[string]struct{}) + var circular []*index.CircularReferenceResult + for _, idx := range indexes { + indexesByPath[index.CanonicalReferenceIdentity(idx.GetSpecAbsolutePath())] = idx + circular = append(circular, idx.GetCircularReferences()...) + schemaReferenceNodes := make(map[*yaml.Node]struct{}) + for _, schemaRef := range idx.GetAllReferenceSchemas() { + if schemaRef != nil && schemaRef.Node != nil { + schemaReferenceNodes[schemaRef.Node] = struct{}{} + } + } + for _, mapped := range idx.GetMappedReferencesSequenced() { + addInlineSchemaTarget(schemaTargets, schemaReferenceNodes, mapped) + } + } + circular = append(circular, rolodex.GetIgnoredCircularReferences()...) + circular = append(circular, rolodex.GetSafeCircularReferences()...) + + byDefinition := make(map[string]*inlineReferenceTarget) + for _, result := range circular { + if result == nil || len(result.Journey) == 0 { + continue + } + start := circularJourneyStart(result) + for _, ref := range result.Journey[start:] { + if ref == nil || ref.FullDefinition == "" { + continue + } + fullDefinition := index.CanonicalReferenceIdentity(ref.FullDefinition) + if _, isSchema := schemaTargets[fullDefinition]; !isSchema { + continue + } + filePath, definition := index.SplitRefFragment(fullDefinition) + if filePath == "" || filePath == rootPath { + continue + } + if _, exists := byDefinition[fullDefinition]; exists { + continue + } + target, err := buildCircularInlineTarget(ref, fullDefinition, filePath, definition, indexesByPath) + if err != nil { + return nil, err + } + byDefinition[fullDefinition] = target + } + } + + if includeDiscriminatorTargets { + for _, schema := range collectExternalDiscriminatorSchemas(rolodex, rolodex.GetRootIndex()) { + fullDefinition := index.CanonicalReferenceIdentity(schema.fullDef) + if _, exists := byDefinition[fullDefinition]; exists { + continue + } + node := schema.ref.Node + byDefinition[fullDefinition] = &inlineReferenceTarget{ + fullDefinition: fullDefinition, + definition: schema.originalRef, + index: schema.idx, + node: node, + name: schema.schemaName, + } + } + } + + targets := make([]*inlineReferenceTarget, 0, len(byDefinition)) + for _, target := range byDefinition { + targets = append(targets, target) + } + sort.Slice(targets, func(i, j int) bool { + return targets[i].fullDefinition < targets[j].fullDefinition + }) + return targets, nil +} + +func addInlineSchemaTarget(targets map[string]struct{}, schemaNodes map[*yaml.Node]struct{}, mapped *index.ReferenceMapped) { + if mapped == nil || mapped.OriginalReference == nil { + return + } + if _, isSchema := schemaNodes[mapped.OriginalReference.Node]; isSchema { + targets[index.CanonicalReferenceIdentity(mapped.FullDefinition)] = struct{}{} + } +} + +func buildCircularInlineTarget(ref *index.Reference, fullDefinition, filePath, definition string, indexesByPath map[string]*index.SpecIndex) (*inlineReferenceTarget, error) { + sourceIndex := indexesByPath[filePath] + if sourceIndex == nil { + return nil, fmt.Errorf("external circular reference %q has no source index", ref.FullDefinition) + } + node := ref.Node + if node != nil && node.Kind == yaml.DocumentNode && len(node.Content) > 0 { + node = node.Content[0] + } + if node == nil { + return nil, fmt.Errorf("external circular reference %q has no schema node", ref.FullDefinition) + } + return &inlineReferenceTarget{ + fullDefinition: fullDefinition, + definition: definition, + index: sourceIndex, + node: node, + name: inlineTargetName(filePath, definition), + }, nil +} + +func circularJourneyStart(result *index.CircularReferenceResult) int { + if result.LoopIndex >= 0 && result.LoopIndex < len(result.Journey) { + return result.LoopIndex + } + if result.LoopPoint != nil { + loopPoint := index.CanonicalReferenceIdentity(result.LoopPoint.FullDefinition) + for i := len(result.Journey) - 1; i >= 0; i-- { + if result.Journey[i] != nil && index.CanonicalReferenceIdentity(result.Journey[i].FullDefinition) == loopPoint { + return i + } + } + } + return 0 +} + +func populateInlineDiscriminatorMappingRewrites(rolodex *index.Rolodex, rewrites map[string]string, ctx *highbase.InlineRenderContext) error { + for _, mapping := range collectDiscriminatorMappingNodesWithContext(rolodex) { + ref, targetIndex := mapping.sourceIdx.SearchIndexForReference(mapping.node.Value) + if ref == nil || targetIndex == nil { + if utils.IsExternalRef(mapping.node.Value) { + return fmt.Errorf("unable to resolve external discriminator mapping %q", mapping.node.Value) + } + continue + } + if targetIndex == rolodex.GetRootIndex() { + continue + } + canonical := index.CanonicalReferenceIdentity(targetIndex.GetSpecAbsolutePath() + ref.Definition) + replacement := rewrites[canonical] + if replacement == "" { + return fmt.Errorf("external discriminator mapping %q was not lifted", mapping.node.Value) + } + ctx.SetMappingRewrite(mapping.node, replacement) + } + return nil +} + +func inlineTargetName(filePath, definition string) string { + if definition != "" { + segment := definition[strings.LastIndex(definition, "/")+1:] + segment = strings.ReplaceAll(strings.ReplaceAll(segment, "~1", "/"), "~0", "~") + if decoded, err := url.PathUnescape(segment); err == nil { + segment = decoded + } + if segment != "" { + return segment + } + } + base := filepath.Base(filePath) + name := strings.TrimSuffix(base, filepath.Ext(base)) + if name == "" || name == "." { + return "Schema" + } + return name +} + +func findInlineTargetComponent(schemas *orderedmap.Map[string, *highbase.SchemaProxy], target *inlineReferenceTarget) string { + if schemas == nil || target == nil { + return "" + } + for pair := schemas.First(); pair != nil; pair = pair.Next() { + proxy := pair.Value() + if proxy == nil || proxy.GoLow() == nil { + continue + } + lowProxy := proxy.GoLow() + if lowProxy.GetIndex() == target.index && lowProxy.GetValueNode() == target.node { + return pair.Key() + } + } + return "" +} + +func populateInlineReferenceContext(rolodex *index.Rolodex, targets []*inlineReferenceTarget, rewrites map[string]string, ctx *highbase.InlineRenderContext) { + targetsByDefinition := make(map[string]*inlineReferenceTarget, len(targets)) + for _, target := range targets { + targetsByDefinition[target.fullDefinition] = target + } + indexes := append(append([]*index.SpecIndex{}, rolodex.GetIndexes()...), rolodex.GetRootIndex()) + for _, idx := range indexes { + for _, mapped := range idx.GetMappedReferencesSequenced() { + canonicalTarget := index.CanonicalReferenceIdentity(mapped.FullDefinition) + ctx.SetReferenceNodeIdentity(mapped.OriginalReference.Node, mapped.OriginalReference.Index, canonicalTarget) + replacement, ok := rewrites[canonicalTarget] + if !ok { + continue + } + authored := inlineAuthoredReference(mapped.OriginalReference) + ctx.SetReferenceNodeRewrite(mapped.OriginalReference.Node, replacement) + ctx.MarkReferenceNodeAsPreserved(mapped.OriginalReference.Node) + ctx.SetReferenceRewrite(mapped.OriginalReference.Index, authored, replacement) + ctx.MarkScopedRefAsPreserved(mapped.OriginalReference.Index, authored) + // Resolved high-level proxies are backed by the target index even though + // they retain the authored reference string. Record that representation + // as an alias without dropping target qualification. + if target := targetsByDefinition[canonicalTarget]; target != nil { + ctx.SetReferenceRewrite(target.index, authored, replacement) + ctx.MarkScopedRefAsPreserved(target.index, authored) + } + } + } + + for _, target := range targets { + replacement := rewrites[target.fullDefinition] + ctx.SetReferenceRewrite(target.index, target.definition, replacement) + ctx.MarkScopedRefAsPreserved(target.index, target.definition) + } +} + +func inlineAuthoredReference(ref *index.Reference) string { + if ref.RawRef != "" { + return ref.RawRef + } + return ref.Definition +} diff --git a/bundler/inline_references_test.go b/bundler/inline_references_test.go new file mode 100644 index 00000000..2d509ecf --- /dev/null +++ b/bundler/inline_references_test.go @@ -0,0 +1,434 @@ +// Copyright 2026 Princess B33f Heavy Industries / Dave Shanley +// SPDX-License-Identifier: MIT + +package bundler + +import ( + "errors" + "os" + "path/filepath" + "strings" + "sync" + "testing" + + "github.com/pb33f/libopenapi" + "github.com/pb33f/libopenapi/datamodel" + highbase "github.com/pb33f/libopenapi/datamodel/high/base" + v3high "github.com/pb33f/libopenapi/datamodel/high/v3" + "github.com/pb33f/libopenapi/index" + "github.com/pb33f/libopenapi/orderedmap" + "github.com/pb33f/libopenapi/utils" + "github.com/pb33f/testify/assert" + "github.com/pb33f/testify/require" + "go.yaml.in/yaml/v4" +) + +func TestPrepareInlineReferences_NoModelState(t *testing.T) { + ctx := highbase.NewInlineRenderContext() + require.NoError(t, prepareInlineReferences(nil, ctx, false)) + require.NoError(t, prepareInlineReferences(nil, nil, false)) + targets, err := collectInlineReferenceTargets(nil, false) + require.NoError(t, err) + assert.Empty(t, targets) + require.ErrorContains(t, prepareCollectedInlineReferences(nil, nil, false, nil, errors.New("collect failed")), "collect failed") +} + +func TestInlineReferenceTargetHelpers(t *testing.T) { + var document yaml.Node + require.NoError(t, yaml.Unmarshal([]byte("Thing: {type: object}"), &document)) + idx := index.NewSpecIndexWithConfig(&document, index.CreateOpenAPIIndexConfig()) + idx.SetAbsolutePath(filepath.Join(t.TempDir(), "external.yaml")) + fullDefinition := idx.GetSpecAbsolutePath() + "#/Thing" + ref := &index.Reference{FullDefinition: fullDefinition, Node: &document} + + target, err := buildCircularInlineTarget(ref, fullDefinition, idx.GetSpecAbsolutePath(), "#/Thing", map[string]*index.SpecIndex{idx.GetSpecAbsolutePath(): idx}) + require.NoError(t, err) + assert.Equal(t, document.Content[0], target.node) + _, err = buildCircularInlineTarget(ref, fullDefinition, idx.GetSpecAbsolutePath(), "#/Thing", nil) + require.ErrorContains(t, err, "no source index") + ref.Node = nil + _, err = buildCircularInlineTarget(ref, fullDefinition, idx.GetSpecAbsolutePath(), "#/Thing", map[string]*index.SpecIndex{idx.GetSpecAbsolutePath(): idx}) + require.ErrorContains(t, err, "no schema node") + + schemaNode := utils.CreateRefNode("#/Thing") + targets := make(map[string]struct{}) + schemaNodes := map[*yaml.Node]struct{}{schemaNode: {}} + addInlineSchemaTarget(targets, schemaNodes, nil) + addInlineSchemaTarget(targets, schemaNodes, &index.ReferenceMapped{}) + addInlineSchemaTarget(targets, schemaNodes, &index.ReferenceMapped{OriginalReference: &index.Reference{Node: utils.CreateRefNode("#/Other")}}) + addInlineSchemaTarget(targets, schemaNodes, &index.ReferenceMapped{OriginalReference: &index.Reference{Node: schemaNode}, FullDefinition: fullDefinition}) + assert.Contains(t, targets, index.CanonicalReferenceIdentity(fullDefinition)) +} + +func TestPrepareInlineReferences_InitializesSchemaMap(t *testing.T) { + tmp := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(tmp, "node.yaml"), []byte("type: object\nproperties:\n next:\n $ref: './node.yaml'\n"), 0o600)) + root := []byte("openapi: 3.1.0\ninfo: {title: t, version: v}\npaths:\n /node:\n get:\n responses:\n '200':\n description: ok\n content:\n application/json:\n schema:\n $ref: './node.yaml'\n") + config := &datamodel.DocumentConfiguration{BasePath: tmp, AllowFileReferences: true} + document, err := libopenapi.NewDocumentWithConfiguration(root, config) + require.NoError(t, err) + model, err := document.BuildV3Model() + require.NoError(t, err) + model.Model.Components = &v3high.Components{} + require.NoError(t, prepareInlineReferences(&model.Model, highbase.NewInlineRenderContext(), false)) + assert.NotNil(t, model.Model.Components.Schemas) +} + +func TestInlineTargetNameAndLookupEdges(t *testing.T) { + assert.Equal(t, "A/B C", inlineTargetName("schema.yaml", "#/A~1B%20C")) + assert.Equal(t, "node", inlineTargetName("/schemas/node.yaml", "")) + assert.Equal(t, "Schema", inlineTargetName(".", "")) + assert.Empty(t, findInlineTargetComponent(nil, nil)) + + schemas := orderedmap.New[string, *highbase.SchemaProxy]() + schemas.Set("Nil", nil) + assert.Empty(t, findInlineTargetComponent(schemas, &inlineReferenceTarget{})) + assert.Equal(t, "raw", inlineAuthoredReference(&index.Reference{RawRef: "raw", Definition: "definition"})) + assert.Equal(t, "definition", inlineAuthoredReference(&index.Reference{Definition: "definition"})) +} + +func TestCollectInlineReferenceTargets_CircularMetadataEdges(t *testing.T) { + var rootNode, externalNode yaml.Node + require.NoError(t, yaml.Unmarshal([]byte("openapi: 3.1.0\ninfo: {title: t, version: v}\npaths: {}"), &rootNode)) + require.NoError(t, yaml.Unmarshal([]byte("Thing: {type: object}"), &externalNode)) + rootIndex := index.NewSpecIndexWithConfig(&rootNode, index.CreateOpenAPIIndexConfig()) + externalIndex := index.NewSpecIndexWithConfig(&externalNode, index.CreateOpenAPIIndexConfig()) + rootIndex.SetAbsolutePath(filepath.Join(t.TempDir(), "root.yaml")) + externalIndex.SetAbsolutePath(filepath.Join(t.TempDir(), "external.yaml")) + rolodex := index.NewRolodex(index.CreateOpenAPIIndexConfig()) + rolodex.SetRootIndex(rootIndex) + rolodex.AddIndex(externalIndex) + rootIndex.SetRolodex(rolodex) + externalIndex.SetRolodex(rolodex) + + externalDefinition := externalIndex.GetSpecAbsolutePath() + "#/Thing" + rootIndex.SetCircularReferences([]*index.CircularReferenceResult{ + nil, + {}, + {LoopIndex: -2, Journey: []*index.Reference{ + nil, + {}, + {FullDefinition: "#/NoFile", Node: externalNode.Content[0]}, + {FullDefinition: rootIndex.GetSpecAbsolutePath() + "#/Root", Node: rootNode.Content[0]}, + {FullDefinition: externalDefinition, Node: &externalNode}, + {FullDefinition: externalDefinition, Node: externalNode.Content[0]}, + }}, + }) + targets, err := collectInlineReferenceTargets(rolodex, false) + require.NoError(t, err) + assert.Empty(t, targets, "non-schema circular metadata must not be lifted into components.schemas") + + deep := &index.CircularReferenceResult{LoopIndex: 500, LoopPoint: &index.Reference{FullDefinition: externalDefinition}, Journey: []*index.Reference{ + {FullDefinition: rootIndex.GetSpecAbsolutePath() + "#/Root", Node: rootNode.Content[0]}, + {FullDefinition: externalDefinition, Node: externalNode.Content[0]}, + }} + assert.Equal(t, 1, circularJourneyStart(deep), "depth-limit LoopIndex must fall back to the actual loop point") + assert.Zero(t, circularJourneyStart(&index.CircularReferenceResult{LoopIndex: -1, Journey: deep.Journey})) +} + +func TestPopulateInlineDiscriminatorMappingRewrites_SkipsUnresolvedMapping(t *testing.T) { + var root yaml.Node + require.NoError(t, yaml.Unmarshal([]byte(`openapi: 3.1.0 +info: {title: t, version: v} +paths: {} +components: + schemas: + Animal: + discriminator: + propertyName: kind + mapping: + missing: '#/components/schemas/Missing' +`), &root)) + rootIndex := index.NewSpecIndexWithConfig(&root, index.CreateOpenAPIIndexConfig()) + rootIndex.SetAbsolutePath(filepath.Join(t.TempDir(), "root.yaml")) + rolodex := index.NewRolodex(index.CreateOpenAPIIndexConfig()) + rolodex.SetRootIndex(rootIndex) + rootIndex.SetRolodex(rolodex) + require.NoError(t, populateInlineDiscriminatorMappingRewrites(rolodex, nil, highbase.NewInlineRenderContext())) +} + +func TestPopulateInlineDiscriminatorMappingRewrites_RejectsUnliftedExternalTarget(t *testing.T) { + tmp := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(tmp, "cat.yaml"), []byte("Cat: {type: object}\n"), 0o600)) + root := []byte(`openapi: 3.1.0 +info: {title: t, version: v} +paths: {} +components: + schemas: + Animal: + discriminator: + propertyName: kind + mapping: + cat: './cat.yaml#/Cat' +`) + doc, err := libopenapi.NewDocumentWithConfiguration(root, &datamodel.DocumentConfiguration{BasePath: tmp, AllowFileReferences: true}) + require.NoError(t, err) + model, err := doc.BuildV3Model() + require.NoError(t, err) + + err = populateInlineDiscriminatorMappingRewrites(model.Model.Rolodex, nil, highbase.NewInlineRenderContext()) + require.ErrorContains(t, err, "was not lifted") + external := model.Model.Rolodex.GetIndexes()[0] + externalRoot := external.GetRootNode() + if externalRoot.Kind == yaml.DocumentNode { + externalRoot = externalRoot.Content[0] + } + err = prepareCollectedInlineReferences(&model.Model, highbase.NewInlineRenderContext(), true, []*inlineReferenceTarget{{ + fullDefinition: external.GetSpecAbsolutePath() + "#/Other", + definition: "#/Other", + index: external, + node: externalRoot.Content[1], + name: "Other", + }}, nil) + require.ErrorContains(t, err, "was not lifted") +} + +func TestInlineCircularReferences_CollisionsAndIdempotency(t *testing.T) { + tmp := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(tmp, "external.yaml"), []byte(`Thing: + type: object + properties: + next: + $ref: '#/Thing' +`), 0o600)) + root := []byte(`openapi: 3.1.0 +info: {title: collision, version: 1.0.0} +paths: + /thing: + get: + responses: + '200': + description: ok + content: + application/json: + schema: + $ref: './external.yaml#/Thing' +components: + schemas: + Thing: + type: string +`) + config := &datamodel.DocumentConfiguration{BasePath: tmp, AllowFileReferences: true} + doc, err := libopenapi.NewDocumentWithConfiguration(root, config) + require.NoError(t, err) + model, err := doc.BuildV3Model() + require.NoError(t, err) + + first, err := BundleDocument(&model.Model) + require.NoError(t, err) + second, err := BundleDocument(&model.Model) + require.NoError(t, err) + assert.Equal(t, first, second) + assert.Contains(t, string(first), "$ref: '#/components/schemas/Thing__external'") + assert.Equal(t, 1, strings.Count(string(first), " Thing__external:")) +} + +func TestCollectInlineReferenceTargets_NormalizesJourneyPaths(t *testing.T) { + tmp := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(tmp, "external.yaml"), []byte("Thing:\n type: object\n properties:\n next:\n $ref: '#/Thing'\n"), 0o600)) + root := []byte("openapi: 3.1.0\ninfo: {title: normalized, version: v}\npaths: {}\ncomponents:\n schemas:\n Thing:\n $ref: './external.yaml#/Thing'\n") + config := &datamodel.DocumentConfiguration{BasePath: tmp, AllowFileReferences: true} + doc, err := libopenapi.NewDocumentWithConfiguration(root, config) + require.NoError(t, err) + model, err := doc.BuildV3Model() + require.NoError(t, err) + + allCircular := append([]*index.CircularReferenceResult{}, model.Model.Rolodex.GetRootIndex().GetCircularReferences()...) + for _, idx := range model.Model.Rolodex.GetIndexes() { + allCircular = append(allCircular, idx.GetCircularReferences()...) + } + require.NotEmpty(t, allCircular) + var externalJourneyRef *index.Reference + for _, result := range allCircular { + for _, ref := range result.Journey { + if ref == nil { + continue + } + _, fragment := index.SplitRefFragment(ref.FullDefinition) + if strings.Contains(ref.FullDefinition, "external.yaml") { + ref.FullDefinition = filepath.ToSlash(tmp) + "/nested/../external.yaml" + fragment + externalJourneyRef = ref + } + } + } + + targets, err := collectInlineReferenceTargets(model.Model.Rolodex, false) + require.NoError(t, err) + require.Len(t, targets, 1) + assert.Equal(t, index.CanonicalReferenceIdentity(filepath.Join(tmp, "external.yaml")+"#/Thing"), targets[0].fullDefinition) + + require.NotNil(t, externalJourneyRef) + originalNode := externalJourneyRef.Node + externalJourneyRef.Node = nil + _, err = collectInlineReferenceTargets(model.Model.Rolodex, false) + require.ErrorContains(t, err, "no schema node") + externalJourneyRef.Node = originalNode + model.Model.Rolodex.GetIndexes()[0].SetAbsolutePath(filepath.Join(tmp, "renamed.yaml")) + _, err = collectInlineReferenceTargets(model.Model.Rolodex, false) + require.ErrorContains(t, err, "no source index") +} + +func TestBundleBytes_SkipCircularReferenceCheckFailsClosed(t *testing.T) { + tmp := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(tmp, "node.yaml"), []byte("type: object\nproperties:\n next:\n $ref: './node.yaml'\n"), 0o600)) + root := []byte("openapi: 3.1.0\ninfo: {title: skipped, version: v}\npaths: {}\ncomponents:\n schemas:\n Node:\n $ref: './node.yaml'\n") + config := &datamodel.DocumentConfiguration{BasePath: tmp, AllowFileReferences: true, SkipCircularReferenceCheck: true} + + bundled, err := BundleBytes(root, config) + assert.Nil(t, bundled) + require.ErrorContains(t, err, "circular reference") +} + +func TestBundleBytes_UnresolvedExternalDiscriminatorMappingFails(t *testing.T) { + root := []byte(`openapi: 3.1.0 +info: {title: discriminator, version: v} +paths: {} +components: + schemas: + Animal: + discriminator: + propertyName: kind + mapping: + missing: './missing.yaml#/Missing' +`) + config := &datamodel.DocumentConfiguration{BasePath: t.TempDir(), AllowFileReferences: true} + + bundled, err := BundleBytesWithConfig(root, config, &BundleInlineConfig{ResolveDiscriminatorExternalRefs: true}) + assert.Nil(t, bundled) + require.ErrorContains(t, err, "unable to resolve external discriminator mapping") +} + +func TestInlineCircularReferences_SameFragmentInDifferentFiles(t *testing.T) { + tmp := t.TempDir() + for _, dir := range []string{"a", "b"} { + require.NoError(t, os.MkdirAll(filepath.Join(tmp, dir), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(tmp, dir, "node.yaml"), []byte(`Thing: + type: object + properties: + next: + $ref: '#/Thing' +`), 0o600)) + } + root := []byte(`openapi: 3.1.0 +info: {title: scoped refs, version: 1.0.0} +paths: {} +components: + schemas: + A: + $ref: './a/node.yaml#/Thing' + B: + $ref: './b/node.yaml#/Thing' +`) + config := &datamodel.DocumentConfiguration{BasePath: tmp, AllowFileReferences: true} + bundled, err := BundleBytes(root, config) + require.NoError(t, err) + output := string(bundled) + assert.NotContains(t, output, "node.yaml") + assert.Contains(t, output, "$ref: '#/components/schemas/Thing'") + assert.Contains(t, output, "$ref: '#/components/schemas/Thing__node'") + assert.Equal(t, 1, strings.Count(output, " Thing:")) + assert.Equal(t, 1, strings.Count(output, " Thing__node:")) +} + +func TestInlineDiscriminatorPreparationDoesNotMutateIndexedNodes(t *testing.T) { + tmp := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(tmp, "cat.yaml"), []byte(`components: + schemas: + Cat: + type: object + properties: + next: + $ref: '#/components/schemas/Cat' +`), 0o600)) + root := []byte(`openapi: 3.1.0 +info: {title: discriminator, version: 1.0.0} +paths: {} +components: + schemas: + Animal: + type: object + discriminator: + propertyName: kind + mapping: + cat: './cat.yaml#/components/schemas/Cat' + oneOf: + - $ref: './cat.yaml#/components/schemas/Cat' +`) + config := &datamodel.DocumentConfiguration{BasePath: tmp, AllowFileReferences: true} + doc, err := libopenapi.NewDocumentWithConfiguration(root, config) + require.NoError(t, err) + model, err := doc.BuildV3Model() + require.NoError(t, err) + mapping := collectDiscriminatorMappingNodesWithContext(model.Model.Rolodex)[0].node + originalMapping := mapping.Value + unionRef := model.Model.Components.Schemas.GetOrZero("Animal").Schema().OneOf[0].GetReferenceNode() + originalUnion := unionRef.Content[1].Value + + bundled, err := BundleDocumentWithConfig(&model.Model, &BundleInlineConfig{ResolveDiscriminatorExternalRefs: true}) + require.NoError(t, err) + assert.Contains(t, string(bundled), "cat: '#/components/schemas/Cat'") + assert.Equal(t, originalMapping, mapping.Value) + assert.Equal(t, originalUnion, unionRef.Content[1].Value) +} + +func TestBundleInlineLocalRefsPolicyIsPerRender(t *testing.T) { + spec := []byte(`openapi: 3.1.0 +info: {title: concurrent, version: 1.0.0} +paths: + /pet: + get: + responses: + '200': + description: ok + content: + application/json: + schema: + $ref: '#/components/schemas/Pet' +components: + schemas: + Pet: + type: object +`) + var wg sync.WaitGroup + for i := 0; i < 20; i++ { + for _, inline := range []bool{false, true} { + inline := inline + wg.Add(1) + go func() { + defer wg.Done() + out, err := BundleBytesWithConfig(spec, datamodel.NewDocumentConfiguration(), &BundleInlineConfig{InlineLocalRefs: &inline}) + require.NoError(t, err) + if inline { + assert.NotContains(t, string(out), "$ref: '#/components/schemas/Pet'") + } else { + assert.Contains(t, string(out), "$ref: '#/components/schemas/Pet'") + } + }() + } + } + wg.Wait() +} + +func BenchmarkBundleBytes_InlineReferencePreparation(b *testing.B) { + for _, recursive := range []bool{false, true} { + name := "acyclic" + external := "type: object\nproperties:\n value: {type: string}\n" + if recursive { + name = "external-circular" + external = "type: object\nproperties:\n next:\n $ref: './node.yaml'\n" + } + b.Run(name, func(b *testing.B) { + tmp := b.TempDir() + require.NoError(b, os.WriteFile(filepath.Join(tmp, "node.yaml"), []byte(external), 0o600)) + root := []byte("openapi: 3.1.0\ninfo: {title: benchmark, version: v}\npaths: {}\ncomponents:\n schemas:\n Node:\n $ref: './node.yaml'\n") + config := &datamodel.DocumentConfiguration{BasePath: tmp, AllowFileReferences: true} + b.ReportAllocs() + b.ResetTimer() + for i := 0; i < b.N; i++ { + if _, err := BundleBytes(root, config); err != nil { + b.Fatal(err) + } + } + }) + } +} diff --git a/bundler/test/specs/issue-928/api.yaml b/bundler/test/specs/issue-928/api.yaml new file mode 100644 index 00000000..00cf1efe --- /dev/null +++ b/bundler/test/specs/issue-928/api.yaml @@ -0,0 +1,28 @@ +openapi: 3.1.0 +info: + title: vacuum bundle bug reproducer + version: 1.0.0 +paths: + /bug-0-29-9: + post: + requestBody: + description: Тело запроса + required: true + content: + application/json: + schema: + $ref: '#/components/schemas/BugDto' + responses: + '200': + description: Ответ не предусмотрен +components: + schemas: + BugDto: + type: object + propertyNames: + type: string + minLength: 3 + maxLength: 3 + maxProperties: 500 + additionalProperties: + $ref: './components/schemas/CodeXMap.yaml#/CodeStringOrMapDto' diff --git a/bundler/test/specs/issue-928/components/schemas/CodeXMap.yaml b/bundler/test/specs/issue-928/components/schemas/CodeXMap.yaml new file mode 100644 index 00000000..6522b3d5 --- /dev/null +++ b/bundler/test/specs/issue-928/components/schemas/CodeXMap.yaml @@ -0,0 +1,74 @@ +CodeStringOrMapDto: + description: Значение элемента данных, строка или вложенная структура + oneOf: + - type: string + minLength: 1 + maxLength: 255 + pattern: '^[\s\S]+$' + example: '5502039191' + - $ref: '#/Code2Map' + - $ref: '#/Code3Map' + - $ref: '#/CodeUp4Map' +Code2Map: + description: Мапа с двузначными атрибутами + type: object + minProperties: 1 + propertyNames: + description: Двузначный атрибут + type: string + minLength: 2 + maxLength: 2 + pattern: '^[0-9a-zA-Z]+$' + example: SB + maxProperties: 500 + additionalProperties: + description: Значение элемента данных, строка или вложенная структура + oneOf: + - type: string + minLength: 1 + maxLength: 255 + - $ref: '#/Code2Map' + - $ref: '#/Code3Map' + - $ref: '#/CodeUp4Map' +Code3Map: + description: Мапа с трехзначными атрибутами + type: object + minProperties: 1 + propertyNames: + description: Трехзначный атрибут + type: string + minLength: 3 + maxLength: 3 + pattern: '^[0-9a-zA-Z]+$' + example: '048' + maxProperties: 500 + additionalProperties: + description: Значение элемента данных, строка или вложенная структура + oneOf: + - type: string + minLength: 1 + maxLength: 255 + - $ref: '#/Code2Map' + - $ref: '#/Code3Map' + - $ref: '#/CodeUp4Map' +CodeUp4Map: + description: Мапа с двух-трех-четырех значными атрибутами + type: object + minProperties: 1 + propertyNames: + description: Двух-трех-четырех значными атрибуты + type: string + minLength: 2 + maxLength: 4 + pattern: '^[0-9a-zA-Z]+$' + example: 5F2A + maxProperties: 500 + additionalProperties: + description: Значение элемента данных, строка или вложенная структура + oneOf: + - type: string + minLength: 1 + maxLength: 255 + - $ref: '#/Code2Map' + - $ref: '#/Code3Map' + - $ref: '#/CodeUp4Map' diff --git a/datamodel/high/base/discriminator.go b/datamodel/high/base/discriminator.go index df2ebd65..2df43521 100644 --- a/datamodel/high/base/discriminator.go +++ b/datamodel/high/base/discriminator.go @@ -4,6 +4,8 @@ package base import ( + "errors" + "github.com/pb33f/libopenapi/datamodel/high" "github.com/pb33f/libopenapi/datamodel/low" lowBase "github.com/pb33f/libopenapi/datamodel/low/base" @@ -57,3 +59,47 @@ func (d *Discriminator) MarshalYAML() (interface{}, error) { nb := high.NewNodeBuilder(d, d.low) return nb.Render(), nil } + +// MarshalYAMLInlineWithContext renders discriminator mappings with replacements +// prepared for this render operation, without mutating their indexed YAML nodes. +func (d *Discriminator) MarshalYAMLInlineWithContext(ctx any) (interface{}, error) { + nb := high.NewNodeBuilder(d, d.low) + rendered := nb.Render() + renderCtx, ok := ctx.(*InlineRenderContext) + if !ok || renderCtx == nil || d.low == nil || d.Mapping == nil { + return rendered, errors.Join(nb.Errors...) + } + + mappingNode := discriminatorMappingNode(rendered) + if mappingNode != nil { + for pair := d.Mapping.First(); pair != nil; pair = pair.Next() { + lowValue := d.low.FindMappingValue(pair.Key()) + if lowValue == nil { + continue + } + replacement, exists := renderCtx.MappingRewrite(lowValue.ValueNode) + if !exists { + continue + } + for i := 0; i+1 < len(mappingNode.Content); i += 2 { + if mappingNode.Content[i].Value == pair.Key() { + mappingNode.Content[i+1].Value = replacement + break + } + } + } + } + return rendered, errors.Join(nb.Errors...) +} + +func discriminatorMappingNode(node *yaml.Node) *yaml.Node { + if node == nil || node.Kind != yaml.MappingNode { + return nil + } + for i := 0; i+1 < len(node.Content); i += 2 { + if node.Content[i].Value == "mapping" && node.Content[i+1].Kind == yaml.MappingNode { + return node.Content[i+1] + } + } + return nil +} diff --git a/datamodel/high/base/discriminator_test.go b/datamodel/high/base/discriminator_test.go index f4b0ab23..e5007d6d 100644 --- a/datamodel/high/base/discriminator_test.go +++ b/datamodel/high/base/discriminator_test.go @@ -10,7 +10,9 @@ import ( lowmodel "github.com/pb33f/libopenapi/datamodel/low" lowbase "github.com/pb33f/libopenapi/datamodel/low/base" + "github.com/pb33f/libopenapi/utils" "github.com/pb33f/testify/assert" + "github.com/pb33f/testify/require" "go.yaml.in/yaml/v4" ) @@ -140,3 +142,33 @@ defaultMapping: '#/components/schemas/UnknownReptile'` assert.NoError(t, err) assert.NotNil(t, marshaled) } + +func TestDiscriminator_MarshalYAMLInlineWithContext(t *testing.T) { + var node yaml.Node + require.NoError(t, yaml.Unmarshal([]byte(`propertyName: kind +mapping: + cat: './cat.yaml#/Cat' + dog: '#/Dog' +`), &node)) + var lowDiscriminator lowbase.Discriminator + lowmodel.BuildModel(node.Content[0], &lowDiscriminator) + highDiscriminator := NewDiscriminator(&lowDiscriminator) + highDiscriminator.Mapping.Set("bird", "#/Bird") + ctx := NewInlineRenderContext() + ctx.SetMappingRewrite(lowDiscriminator.FindMappingValue("cat").ValueNode, "#/components/schemas/Cat") + + rendered, err := highDiscriminator.MarshalYAMLInlineWithContext(ctx) + require.NoError(t, err) + data, err := yaml.Marshal(rendered) + require.NoError(t, err) + assert.Contains(t, string(data), "cat: '#/components/schemas/Cat'") + assert.Contains(t, string(data), "dog: '#/Dog'") + assert.Equal(t, "./cat.yaml#/Cat", lowDiscriminator.FindMappingValue("cat").ValueNode.Value) + + plain, err := highDiscriminator.MarshalYAMLInlineWithContext("wrong context") + require.NoError(t, err) + assert.NotNil(t, plain) + assert.Nil(t, discriminatorMappingNode(nil)) + assert.Nil(t, discriminatorMappingNode(&yaml.Node{Kind: yaml.SequenceNode})) + assert.Nil(t, discriminatorMappingNode(utils.CreateRefNode("#/Thing"))) +} diff --git a/datamodel/high/base/schema.go b/datamodel/high/base/schema.go index 58dec858..3a7059c5 100644 --- a/datamodel/high/base/schema.go +++ b/datamodel/high/base/schema.go @@ -597,12 +597,12 @@ func (s *Schema) MarshalYAMLInlineWithContext(ctx any) (interface{}, error) { // this avoids mutating shared state and prevents race conditions. for _, sp := range s.OneOf { if sp != nil && sp.IsReference() { - renderCtx.MarkRefAsPreserved(sp.GetReference()) + markDiscriminatorReferenceAsPreserved(renderCtx, sp) } } for _, sp := range s.AnyOf { if sp != nil && sp.IsReference() { - renderCtx.MarkRefAsPreserved(sp.GetReference()) + markDiscriminatorReferenceAsPreserved(renderCtx, sp) } } } @@ -620,6 +620,14 @@ func (s *Schema) MarshalYAMLInlineWithContext(ctx any) (interface{}, error) { return nb.Render(), errors.Join(nb.Errors...) } +func markDiscriminatorReferenceAsPreserved(ctx *InlineRenderContext, sp *SchemaProxy) { + if sp.GoLow() == nil { + ctx.MarkScopedRefAsPreserved(nil, sp.GetReference()) + return + } + ctx.MarkReferenceNodeAsPreserved(sp.GetReferenceNode()) +} + // MarshalYAMLInline will render out the Schema pointer as YAML, and all refs will be inlined fully. // This method creates a fresh InlineRenderContext internally. func (s *Schema) MarshalYAMLInline() (interface{}, error) { diff --git a/datamodel/high/base/schema_proxy.go b/datamodel/high/base/schema_proxy.go index 94642b94..edee4df7 100644 --- a/datamodel/high/base/schema_proxy.go +++ b/datamodel/high/base/schema_proxy.go @@ -44,17 +44,15 @@ func ClearInlineRenderingTracker() { inlineRenderingTracker.Clear() } -// bundlingModeCount tracks the number of active bundling operations. -// Uses reference counting to support concurrent BundleDocument calls safely. -// -// NOTE: This is process-wide. Any RenderInline() call made while bundling is active -// (count > 0) will also preserve local component refs. This is intentional - the bundler -// uses RenderInline internally, and concurrent bundles must all see consistent behavior. -// Direct RenderInline() calls outside of bundling are unaffected when no bundles are running. +// bundlingModeCount is retained for source compatibility with callers that use +// SetBundlingMode and IsBundlingMode directly. New legacy render contexts +// snapshot this state, while bundle policy is explicitly per operation. var bundlingModeCount atomic.Int32 // SetBundlingMode increments or decrements the bundling mode reference count. // Bundling mode is active when count > 0, supporting concurrent bundle operations. +// +// Deprecated: configure InlineRenderContext.PreserveLocalComponentRefs instead. func SetBundlingMode(enabled bool) { if enabled { bundlingModeCount.Add(1) @@ -64,6 +62,8 @@ func SetBundlingMode(enabled bool) { } // IsBundlingMode returns whether any bundling operation is active. +// +// Deprecated: bundle rendering no longer consults process-global bundling mode. func IsBundlingMode() bool { return bundlingModeCount.Load() > 0 } @@ -86,14 +86,31 @@ const ( // Each render call-chain should use its own context to prevent false positive // cycle detection when multiple goroutines render the same schemas concurrently. type InlineRenderContext struct { - tracker sync.Map - Mode RenderingMode - preservedRefs sync.Map // tracks refs that should be preserved in this render + tracker sync.Map + Mode RenderingMode + // PreserveLocalComponentRefs keeps root-local component references intact + // while external references continue through inline rendering. + PreserveLocalComponentRefs bool + // RootIndex identifies the authored root document for local-reference policy. + RootIndex *index.SpecIndex + // StrictCircularReferenceIdentity requires circular metadata to match the + // prepared canonical target instead of a bare authored reference string. + StrictCircularReferenceIdentity bool + preservedRefs sync.Map // scoped reference key -> struct{} + referenceRewrites sync.Map // scoped reference key -> root component ref + mappingRewrites sync.Map // discriminator mapping value node -> root component ref + referenceNodeSources sync.Map // authored reference node -> source index + referenceNodeTargets sync.Map // authored reference node -> canonical target + preservedReferenceNodes sync.Map // authored reference node -> struct{} + referenceNodeRewrites sync.Map // authored reference node -> root component ref } // NewInlineRenderContext creates a new isolated rendering context with default bundle mode. func NewInlineRenderContext() *InlineRenderContext { - return &InlineRenderContext{Mode: RenderingModeBundle} + return &InlineRenderContext{ + Mode: RenderingModeBundle, + PreserveLocalComponentRefs: IsBundlingMode(), + } } // NewInlineRenderContextForValidation creates a context that fully inlines @@ -103,6 +120,26 @@ func NewInlineRenderContextForValidation() *InlineRenderContext { return &InlineRenderContext{Mode: RenderingModeValidation} } +// ScopedReferenceKey identifies an authored reference in the document where it +// appears. The source qualification prevents equal local fragments in different +// external documents from colliding during one render. +func ScopedReferenceKey(source *index.SpecIndex, ref string) string { + if source == nil { + return scopedReferenceKey("", ref) + } + return scopedReferenceKey(source.GetSpecAbsolutePath(), ref) +} + +func scopedReferenceKey(sourcePath, ref string) string { + if ref == "" { + return "" + } + if sourcePath == "" { + return "\x00" + ref + } + return sourcePath + "\x00" + ref +} + // StartRendering marks a key as being rendered. Returns true if already rendering (cycle detected). // The key should be stable and unique per schema instance (e.g., filePath:$ref). func (ctx *InlineRenderContext) StartRendering(key string) bool { @@ -122,21 +159,177 @@ func (ctx *InlineRenderContext) StopRendering(key string) { // MarkRefAsPreserved marks a reference as one that should be preserved (not inlined) in this render. // used by discriminator handling to track which refs need preservation without mutating shared state. +// +// Deprecated: use MarkScopedRefAsPreserved to qualify the authored reference by source document. func (ctx *InlineRenderContext) MarkRefAsPreserved(ref string) { - if ref != "" { - ctx.preservedRefs.Store(ref, true) - } + ctx.MarkScopedRefAsPreserved(nil, ref) } // ShouldPreserveRef returns true if the given reference was marked for preservation. +// +// Deprecated: use ShouldPreserveScopedRef to qualify the authored reference by source document. func (ctx *InlineRenderContext) ShouldPreserveRef(ref string) bool { - if ref == "" { + return ctx.ShouldPreserveScopedRef(nil, ref) +} + +// MarkScopedRefAsPreserved marks an authored reference in a specific source +// document as preserved for this render operation. +func (ctx *InlineRenderContext) MarkScopedRefAsPreserved(source *index.SpecIndex, ref string) { + ctx.markRefAsPreserved(scopedReferencePath(source), ref) +} + +func scopedReferencePath(source *index.SpecIndex) string { + if source == nil { + return "" + } + return source.GetSpecAbsolutePath() +} + +func (ctx *InlineRenderContext) markRefAsPreserved(sourcePath, ref string) { + if key := scopedReferenceKey(sourcePath, ref); key != "" { + ctx.preservedRefs.Store(key, struct{}{}) + } +} + +// ShouldPreserveScopedRef reports whether the source-qualified authored +// reference must remain a reference during this render operation. +func (ctx *InlineRenderContext) ShouldPreserveScopedRef(source *index.SpecIndex, ref string) bool { + return ctx.shouldPreserveRef(scopedReferencePath(source), ref) +} + +func (ctx *InlineRenderContext) shouldPreserveRef(sourcePath, ref string) bool { + key := scopedReferenceKey(sourcePath, ref) + if key == "" { return false } - _, ok := ctx.preservedRefs.Load(ref) + _, ok := ctx.preservedRefs.Load(key) return ok } +// SetReferenceRewrite maps a source-qualified authored reference to a +// self-contained root component reference for this render operation. +func (ctx *InlineRenderContext) SetReferenceRewrite(source *index.SpecIndex, ref, replacement string) { + ctx.setReferenceRewrite(scopedReferencePath(source), ref, replacement) +} + +func (ctx *InlineRenderContext) setReferenceRewrite(sourcePath, ref, replacement string) { + if key := scopedReferenceKey(sourcePath, ref); key != "" && replacement != "" { + ctx.referenceRewrites.Store(key, replacement) + } +} + +// ReferenceRewrite returns the root component reference allocated for a +// source-qualified authored reference. +func (ctx *InlineRenderContext) ReferenceRewrite(source *index.SpecIndex, ref string) (string, bool) { + return ctx.referenceRewrite(scopedReferencePath(source), ref) +} + +func (ctx *InlineRenderContext) referenceRewrite(sourcePath, ref string) (string, bool) { + key := scopedReferenceKey(sourcePath, ref) + if key == "" { + return "", false + } + replacement, ok := ctx.referenceRewrites.Load(key) + if !ok { + return "", false + } + return replacement.(string), true +} + +// SetMappingRewrite records a replacement for one raw discriminator mapping +// value without mutating the indexed YAML node. +func (ctx *InlineRenderContext) SetMappingRewrite(node *yaml.Node, replacement string) { + if node != nil && replacement != "" { + ctx.mappingRewrites.Store(node, replacement) + } +} + +// MappingRewrite returns a prepared discriminator mapping replacement. +func (ctx *InlineRenderContext) MappingRewrite(node *yaml.Node) (string, bool) { + if node == nil { + return "", false + } + replacement, ok := ctx.mappingRewrites.Load(node) + if !ok { + return "", false + } + return replacement.(string), true +} + +// SetReferenceNodeIdentity records the authored source and canonical target for +// a reference node. Bundle preparation calls this once before rendering. +func (ctx *InlineRenderContext) SetReferenceNodeIdentity(node *yaml.Node, source *index.SpecIndex, target string) { + if node == nil { + return + } + if source != nil { + ctx.referenceNodeSources.Store(node, source) + } + if target != "" { + ctx.referenceNodeTargets.Store(node, target) + } +} + +// ReferenceNodeSource returns the authored source index prepared for node. +func (ctx *InlineRenderContext) ReferenceNodeSource(node *yaml.Node) (*index.SpecIndex, bool) { + if node == nil { + return nil, false + } + source, ok := ctx.referenceNodeSources.Load(node) + if !ok { + return nil, false + } + return source.(*index.SpecIndex), true +} + +// ReferenceNodeTarget returns the canonical resolved target prepared for node. +func (ctx *InlineRenderContext) ReferenceNodeTarget(node *yaml.Node) (string, bool) { + if node == nil { + return "", false + } + target, ok := ctx.referenceNodeTargets.Load(node) + if !ok { + return "", false + } + return target.(string), true +} + +// MarkReferenceNodeAsPreserved marks one authored reference occurrence for +// preservation without relying on an ambiguous bare reference string. +func (ctx *InlineRenderContext) MarkReferenceNodeAsPreserved(node *yaml.Node) { + if node != nil { + ctx.preservedReferenceNodes.Store(node, struct{}{}) + } +} + +// ShouldPreserveReferenceNode reports whether an authored occurrence is preserved. +func (ctx *InlineRenderContext) ShouldPreserveReferenceNode(node *yaml.Node) bool { + if node == nil { + return false + } + _, ok := ctx.preservedReferenceNodes.Load(node) + return ok +} + +// SetReferenceNodeRewrite records the replacement for one authored reference occurrence. +func (ctx *InlineRenderContext) SetReferenceNodeRewrite(node *yaml.Node, replacement string) { + if node != nil && replacement != "" { + ctx.referenceNodeRewrites.Store(node, replacement) + } +} + +// ReferenceNodeRewrite returns the replacement for one authored reference occurrence. +func (ctx *InlineRenderContext) ReferenceNodeRewrite(node *yaml.Node) (string, bool) { + if node == nil { + return "", false + } + replacement, ok := ctx.referenceNodeRewrites.Load(node) + if !ok { + return "", false + } + return replacement.(string), true +} + // SchemaProxy exists as a stub that will create a Schema once (and only once) the Schema() method is called. An // underlying low-level SchemaProxy backs this high-level one. // @@ -640,6 +833,89 @@ func (sp *SchemaProxy) referenceYAMLNode() (*yaml.Node, error) { return sp.referenceYAMLNodeForSchema(nil) } +func (sp *SchemaProxy) sourceIndex() *index.SpecIndex { + if sp == nil || sp.schema == nil || sp.schema.Value == nil { + return nil + } + return sp.schema.Value.GetIndex() +} + +func (sp *SchemaProxy) referenceSourceIndex(ctx *InlineRenderContext) *index.SpecIndex { + if ctx != nil { + if source, ok := ctx.ReferenceNodeSource(sp.GetReferenceNode()); ok { + return source + } + } + return sp.sourceIndex() +} + +func (sp *SchemaProxy) referenceSourcePath(ctx *InlineRenderContext) string { + if source := sp.referenceSourceIndex(ctx); source != nil { + return source.GetSpecAbsolutePath() + } + return "" +} + +func (sp *SchemaProxy) isRootLocalReference(ctx *InlineRenderContext) bool { + if sp == nil || !strings.HasPrefix(sp.GetReference(), "#/") { + return false + } + source := sp.referenceSourceIndex(ctx) + if source == nil { + return false + } + if ctx != nil && ctx.RootIndex != nil { + return source == ctx.RootIndex + } + return source.GetRolodex() != nil && source.GetRolodex().GetRootIndex() == source +} + +func (sp *SchemaProxy) referenceTargetIdentity(ctx *InlineRenderContext) string { + if ctx != nil { + if target, ok := ctx.ReferenceNodeTarget(sp.GetReferenceNode()); ok { + return index.CanonicalReferenceIdentity(target) + } + } + ref := sp.GetReference() + if strings.HasPrefix(ref, "#") { + if source := sp.sourceIndex(); source != nil { + return index.CanonicalReferenceIdentity(source.GetSpecAbsolutePath() + ref) + } + } + return "" +} + +// rewrittenReferenceNode clones the authored reference node and applies the +// source-qualified replacement for this render, if one was prepared. Cloning +// preserves $ref siblings and source metadata without mutating indexed nodes. +func (sp *SchemaProxy) rewrittenReferenceNode(ctx *InlineRenderContext) (*yaml.Node, bool, error) { + node, err := sp.referenceYAMLNode() + if node == nil { + return nil, false, err + } + replacement, ok := ctx.ReferenceNodeRewrite(node) + if !ok { + replacement, ok = ctx.referenceRewrite(sp.referenceSourcePath(ctx), sp.GetReference()) + } + if !ok { + return node, false, err + } + cloned := utils.CloneYAMLNode(node) + refValue := utils.GetRefValueNode(cloned) + if refValue == nil { + return nil, false, errors.Join(err, errors.New("unable to rewrite schema reference: $ref value node is missing")) + } + refValue.Value = replacement + return cloned, true, err +} + +func (sp *SchemaProxy) circularReferenceResult(ctx *InlineRenderContext, node *yaml.Node, rewritten bool, refErr, circularErr error) (*yaml.Node, error) { + if rewritten || sp.isRootLocalReference(ctx) { + return node, refErr + } + return node, errors.Join(refErr, circularErr) +} + func (sp *SchemaProxy) referenceYAMLNodeForSchema(s *Schema) (*yaml.Node, error) { if sp.isRefWithSiblings() { return sp.renderRefWithSiblings(), nil @@ -746,21 +1022,30 @@ func (sp *SchemaProxy) marshalYAMLInlineInternal(ctx *InlineRenderContext) (inte refNode := func() (*yaml.Node, error) { return sp.referenceYAMLNode() } + rewrittenRefNode := func() (*yaml.Node, bool, error) { + return sp.rewrittenReferenceNode(ctx) + } // check if this reference should be preserved (set via context by discriminator handling). // this avoids mutating shared SchemaProxy state and prevents race conditions. // need to guard against nil schema.Value which can happen with bad/incomplete proxies. if sp.IsReference() { ref := sp.GetReference() - if ref != "" && ctx.ShouldPreserveRef(ref) { - return refNode() + refNode := sp.GetReferenceNode() + preserve := ctx.ShouldPreserveReferenceNode(refNode) + if !preserve { + preserve = ctx.shouldPreserveRef(sp.referenceSourcePath(ctx), ref) || ctx.ShouldPreserveRef(ref) + } + if ref != "" && preserve { + node, _, err := rewrittenRefNode() + return node, err } } // In bundling mode, preserve local component refs that point to schemas in the SAME document. // Only inline refs that point to schemas from EXTERNAL files. // Outside of bundling mode (direct MarshalYAMLInline calls), inline everything. - if IsBundlingMode() && sp.IsReference() { + if ctx.PreserveLocalComponentRefs && sp.IsReference() { ref := sp.GetReference() if strings.HasPrefix(ref, "#/components/") { // Check if this ref points to a schema in the same root document. @@ -773,7 +1058,7 @@ func (sp *SchemaProxy) marshalYAMLInlineInternal(ctx *InlineRenderContext) (inte if rolodex != nil { rootIdx := rolodex.GetRootIndex() // If the schema is in the root index, preserve the ref - if rootIdx != nil && schemaIdx == rootIdx { + if rootIdx != nil && sp.isRootLocalReference(ctx) { return refNode() } } @@ -790,8 +1075,8 @@ func (sp *SchemaProxy) marshalYAMLInlineInternal(ctx *InlineRenderContext) (inte if ctx.StartRendering(renderKey) { // We're already rendering this schema in THIS call chain - return ref to break the cycle if sp.IsReference() { - node, refErr := refNode() - return node, errors.Join(refErr, + node, rewritten, refErr := rewrittenRefNode() + return sp.circularReferenceResult(ctx, node, rewritten, refErr, fmt.Errorf("schema render failure, circular reference: `%s`", sp.GetReference())) } // For inline schemas, return an empty map to avoid infinite recursion @@ -822,9 +1107,20 @@ func (sp *SchemaProxy) marshalYAMLInlineInternal(ctx *InlineRenderContext) (inte for _, c := range circ { if sp.IsReference() { + if c == nil || c.LoopPoint == nil { + continue + } + if ctx.StrictCircularReferenceIdentity { + target := sp.referenceTargetIdentity(ctx) + if target == "" || target != index.CanonicalReferenceIdentity(c.LoopPoint.FullDefinition) { + continue + } + node, rewritten, refErr := rewrittenRefNode() + return sp.circularReferenceResult(ctx, node, rewritten, refErr, cirError(c.LoopPoint.Definition)) + } if sp.GetReference() == c.LoopPoint.Definition { - node, refErr := refNode() - return node, errors.Join(refErr, cirError(c.LoopPoint.Definition)) + node, rewritten, refErr := rewrittenRefNode() + return sp.circularReferenceResult(ctx, node, rewritten, refErr, cirError(c.LoopPoint.Definition)) } basePath := idx.GetSpecAbsolutePath() @@ -833,8 +1129,8 @@ func (sp *SchemaProxy) marshalYAMLInlineInternal(ctx *InlineRenderContext) (inte } if basePath == c.LoopPoint.FullDefinition { - node, refErr := refNode() - return node, errors.Join(refErr, cirError(c.LoopPoint.Definition)) + node, rewritten, refErr := rewrittenRefNode() + return sp.circularReferenceResult(ctx, node, rewritten, refErr, cirError(c.LoopPoint.Definition)) } a := utils.ReplaceWindowsDriveWithLinuxPath(strings.Replace(c.LoopPoint.FullDefinition, basePath, "", 1)) b := sp.GetReference() @@ -856,16 +1152,16 @@ func (sp *SchemaProxy) marshalYAMLInlineInternal(ctx *InlineRenderContext) (inte bBase, bFragment := index.SplitRefFragment(b) if aFragment != "" && bFragment != "" && aFragment == bFragment { - node, refErr := refNode() - return node, errors.Join(refErr, cirError(c.LoopPoint.Definition)) + node, rewritten, refErr := rewrittenRefNode() + return sp.circularReferenceResult(ctx, node, rewritten, refErr, cirError(c.LoopPoint.Definition)) } if aFragment == "" && bFragment == "" { aNorm := strings.TrimPrefix(strings.TrimPrefix(aBase, "./"), "/") bNorm := strings.TrimPrefix(strings.TrimPrefix(bBase, "./"), "/") if aNorm != "" && bNorm != "" && aNorm == bNorm { - node, refErr := refNode() - return node, errors.Join(refErr, cirError(c.LoopPoint.Definition)) + node, rewritten, refErr := rewrittenRefNode() + return sp.circularReferenceResult(ctx, node, rewritten, refErr, cirError(c.LoopPoint.Definition)) } } } diff --git a/datamodel/high/base/schema_proxy_test.go b/datamodel/high/base/schema_proxy_test.go index 244699a2..e9682d15 100644 --- a/datamodel/high/base/schema_proxy_test.go +++ b/datamodel/high/base/schema_proxy_test.go @@ -684,6 +684,167 @@ func TestInlineRenderContext_ShouldPreserveRef_EmptyString(t *testing.T) { assert.False(t, ctx.ShouldPreserveRef("")) } +func TestInlineRenderContext_SourceQualifiedRewrites(t *testing.T) { + var root yaml.Node + require.NoError(t, yaml.Unmarshal([]byte("type: object"), &root)) + left := index.NewSpecIndexWithConfig(&root, index.CreateOpenAPIIndexConfig()) + right := index.NewSpecIndexWithConfig(&root, index.CreateOpenAPIIndexConfig()) + left.SetAbsolutePath(filepath.Join(t.TempDir(), "left.yaml")) + right.SetAbsolutePath(filepath.Join(t.TempDir(), "right.yaml")) + + ctx := NewInlineRenderContext() + ctx.SetReferenceRewrite(left, "#/Thing", "#/components/schemas/LeftThing") + ctx.SetReferenceRewrite(right, "#/Thing", "#/components/schemas/RightThing") + ctx.MarkScopedRefAsPreserved(left, "#/Thing") + ctx.MarkScopedRefAsPreserved(right, "#/Thing") + + leftRewrite, ok := ctx.ReferenceRewrite(left, "#/Thing") + require.True(t, ok) + assert.Equal(t, "#/components/schemas/LeftThing", leftRewrite) + rightRewrite, ok := ctx.ReferenceRewrite(right, "#/Thing") + require.True(t, ok) + assert.Equal(t, "#/components/schemas/RightThing", rightRewrite) + assert.NotEqual(t, ScopedReferenceKey(left, "#/Thing"), ScopedReferenceKey(right, "#/Thing")) + assert.Equal(t, "\x00#/Thing", ScopedReferenceKey(nil, "#/Thing")) + assert.True(t, ctx.ShouldPreserveScopedRef(left, "#/Thing")) + assert.False(t, ctx.ShouldPreserveScopedRef(left, "")) + _, ok = ctx.ReferenceRewrite(left, "#/Missing") + assert.False(t, ok) + _, ok = ctx.ReferenceRewrite(nil, "") + assert.False(t, ok) +} + +func TestInlineRenderContext_RewriteClonesRefWithSiblings(t *testing.T) { + proxy := CreateSchemaProxyRefWithSchema("#/Thing", &Schema{Description: "keep me"}) + original := proxy.GetReferenceNode() + ctx := NewInlineRenderContext() + ctx.SetReferenceRewrite(nil, "#/Thing", "#/components/schemas/Thing") + ctx.MarkRefAsPreserved("#/Thing") + + rendered, err := proxy.MarshalYAMLInlineWithContext(ctx) + require.NoError(t, err) + node := rendered.(*yaml.Node) + assert.NotSame(t, original, node) + assert.Equal(t, "#/components/schemas/Thing", utils.GetRefValueNode(node).Value) + assert.Equal(t, "#/Thing", utils.GetRefValueNode(original).Value) + assert.Contains(t, string(mustMarshalYAML(t, node)), "description: keep me") +} + +func TestSchemaProxy_RewrittenReferenceNodeErrors(t *testing.T) { + ctx := NewInlineRenderContext() + node, rewritten, err := (&SchemaProxy{}).rewrittenReferenceNode(ctx) + assert.Nil(t, node) + assert.False(t, rewritten) + assert.NoError(t, err) + + invalid := utils.CreateEmptyMapNode() + lowProxy := &lowbase.SchemaProxy{} + lowProxy.SetReference("#/Thing", invalid) + proxy := NewSchemaProxy(&low.NodeReference[*lowbase.SchemaProxy]{Value: lowProxy, ValueNode: invalid}) + ctx.SetReferenceRewrite(nil, "#/Thing", "#/components/schemas/Thing") + node, rewritten, err = proxy.rewrittenReferenceNode(ctx) + assert.Nil(t, node) + assert.False(t, rewritten) + assert.ErrorContains(t, err, "$ref value node is missing") +} + +func TestSchemaProxy_RewrittenReferenceNodeClonesOnlyForRewrite(t *testing.T) { + original := utils.CreateRefNode("#/Thing") + lowProxy := &lowbase.SchemaProxy{} + lowProxy.SetReference("#/Thing", original) + proxy := NewSchemaProxy(&low.NodeReference[*lowbase.SchemaProxy]{Value: lowProxy, ValueNode: original}) + ctx := NewInlineRenderContext() + + node, rewritten, err := proxy.rewrittenReferenceNode(ctx) + require.NoError(t, err) + assert.False(t, rewritten) + assert.Same(t, original, node) + + ctx.SetReferenceRewrite(nil, "#/Thing", "#/components/schemas/Thing") + node, rewritten, err = proxy.rewrittenReferenceNode(ctx) + require.NoError(t, err) + assert.True(t, rewritten) + assert.NotSame(t, original, node) + assert.Equal(t, "#/Thing", utils.GetRefValueNode(original).Value) + assert.Equal(t, "#/components/schemas/Thing", utils.GetRefValueNode(node).Value) +} + +func TestSchemaProxy_CycleTrackerUsesPreparedRewrite(t *testing.T) { + proxy := CreateSchemaProxyRefWithSchema("#/Thing", &Schema{Description: "cycle"}) + ctx := NewInlineRenderContext() + ctx.SetReferenceRewrite(nil, "#/Thing", "#/components/schemas/Thing") + ctx.StartRendering("#/Thing") + + rendered, err := proxy.MarshalYAMLInlineWithContext(ctx) + require.NoError(t, err) + assert.Equal(t, "#/components/schemas/Thing", utils.GetRefValueNode(rendered.(*yaml.Node)).Value) +} + +func TestInlineRenderContext_MappingRewrites(t *testing.T) { + ctx := NewInlineRenderContext() + node := utils.CreateStringNode("./cat.yaml#/Cat") + ctx.SetMappingRewrite(node, "#/components/schemas/Cat") + rewrite, ok := ctx.MappingRewrite(node) + require.True(t, ok) + assert.Equal(t, "#/components/schemas/Cat", rewrite) + _, ok = ctx.MappingRewrite(nil) + assert.False(t, ok) + _, ok = ctx.MappingRewrite(utils.CreateStringNode("missing")) + assert.False(t, ok) + ctx.SetMappingRewrite(nil, "ignored") + ctx.SetMappingRewrite(node, "") +} + +func TestInlineRenderContext_ReferenceNodePolicy(t *testing.T) { + var root yaml.Node + require.NoError(t, yaml.Unmarshal([]byte("type: object"), &root)) + source := index.NewSpecIndexWithConfig(&root, index.CreateOpenAPIIndexConfig()) + node := utils.CreateRefNode("#/Thing") + ctx := NewInlineRenderContext() + + ctx.SetReferenceNodeIdentity(node, source, "/schemas/thing.yaml#/Thing") + actualSource, ok := ctx.ReferenceNodeSource(node) + require.True(t, ok) + assert.Same(t, source, actualSource) + target, ok := ctx.ReferenceNodeTarget(node) + require.True(t, ok) + assert.Equal(t, "/schemas/thing.yaml#/Thing", target) + + ctx.MarkReferenceNodeAsPreserved(node) + assert.True(t, ctx.ShouldPreserveReferenceNode(node)) + ctx.SetReferenceNodeRewrite(node, "#/components/schemas/Thing") + rewrite, ok := ctx.ReferenceNodeRewrite(node) + require.True(t, ok) + assert.Equal(t, "#/components/schemas/Thing", rewrite) + + ctx.SetReferenceNodeIdentity(nil, source, "ignored") + ctx.SetReferenceNodeIdentity(utils.CreateEmptyMapNode(), nil, "") + ctx.MarkReferenceNodeAsPreserved(nil) + ctx.SetReferenceNodeRewrite(nil, "ignored") + ctx.SetReferenceNodeRewrite(node, "") + _, ok = ctx.ReferenceNodeSource(nil) + assert.False(t, ok) + _, ok = ctx.ReferenceNodeSource(utils.CreateEmptyMapNode()) + assert.False(t, ok) + _, ok = ctx.ReferenceNodeTarget(nil) + assert.False(t, ok) + _, ok = ctx.ReferenceNodeTarget(utils.CreateEmptyMapNode()) + assert.False(t, ok) + assert.False(t, ctx.ShouldPreserveReferenceNode(nil)) + assert.False(t, ctx.ShouldPreserveReferenceNode(utils.CreateEmptyMapNode())) + _, ok = ctx.ReferenceNodeRewrite(nil) + assert.False(t, ok) + _, ok = ctx.ReferenceNodeRewrite(utils.CreateEmptyMapNode()) + assert.False(t, ok) +} + +func mustMarshalYAML(t *testing.T, value any) []byte { + t.Helper() + data, err := yaml.Marshal(value) + require.NoError(t, err) + return data +} + func TestInlineRenderContext_PreservedRefs_Concurrent(t *testing.T) { ctx := NewInlineRenderContext() @@ -785,7 +946,7 @@ func TestMarkRefAsPreserved_RefNotMarked(t *testing.T) { assert.Nil(t, result) } -func TestMarshalYAMLInline_BundlingMode_PreservesLocalComponentRefs(t *testing.T) { +func TestMarshalYAMLInline_ContextPreservesLocalComponentRefs(t *testing.T) { // Test that in bundling mode, local #/components/ refs pointing to schemas // in the same root document are preserved (not inlined). // This covers lines 356-373 in schema_proxy.go @@ -840,12 +1001,42 @@ func TestMarshalYAMLInline_BundlingMode_PreservesLocalComponentRefs(t *testing.T sp := NewSchemaProxy(&lowRef) - // Enable bundling mode - SetBundlingMode(true) - defer SetBundlingMode(false) - - // MarshalYAMLInline should preserve the ref since the schema is in the root index - result, err := sp.MarshalYAMLInline() + ctx := NewInlineRenderContext() + ctx.PreserveLocalComponentRefs = true + ctx.RootIndex = idx + assert.True(t, sp.isRootLocalReference(ctx)) + ctx.SetReferenceNodeIdentity(sp.GetReferenceNode(), idx, idx.GetSpecAbsolutePath()+ref) + assert.Same(t, idx, sp.referenceSourceIndex(ctx)) + assert.Equal(t, idx.GetSpecAbsolutePath(), sp.referenceSourcePath(ctx)) + assert.Equal(t, index.CanonicalReferenceIdentity(idx.GetSpecAbsolutePath()+ref), sp.referenceTargetIdentity(ctx)) + + fallbackCtx := NewInlineRenderContext() + fallbackCtx.RootIndex = idx + fallbackCtx.StrictCircularReferenceIdentity = true + assert.Same(t, idx, sp.referenceSourceIndex(fallbackCtx)) + assert.True(t, sp.isRootLocalReference(fallbackCtx)) + assert.Equal(t, index.CanonicalReferenceIdentity(idx.GetSpecAbsolutePath()+ref), sp.referenceTargetIdentity(fallbackCtx)) + assert.False(t, (&SchemaProxy{}).isRootLocalReference(fallbackCtx)) + programmatic := CreateSchemaProxyRef("external.yaml#/Thing") + assert.Empty(t, programmatic.referenceTargetIdentity(fallbackCtx)) + assert.Empty(t, programmatic.referenceSourcePath(NewInlineRenderContext())) + assert.Empty(t, programmatic.referenceSourcePath(fallbackCtx)) + idx.SetCircularReferences([]*index.CircularReferenceResult{ + nil, + {}, + {LoopPoint: &index.Reference{Definition: "#/Other", FullDefinition: idx.GetSpecAbsolutePath() + "#/Other"}}, + {LoopPoint: &index.Reference{Definition: ref, FullDefinition: idx.GetSpecAbsolutePath() + ref}}, + }) + strictCtx := NewInlineRenderContext() + strictCtx.StrictCircularReferenceIdentity = true + strictCtx.RootIndex = idx + strictCtx.SetReferenceNodeIdentity(sp.GetReferenceNode(), idx, idx.GetSpecAbsolutePath()+ref) + strictRendered, strictErr := sp.MarshalYAMLInlineWithContext(strictCtx) + require.NoError(t, strictErr) + assert.Equal(t, ref, utils.GetRefValueNode(strictRendered.(*yaml.Node)).Value) + + // Context-aware rendering preserves the ref because its schema is in the root index. + result, err := sp.MarshalYAMLInlineWithContext(ctx) require.NoError(t, err) resultNode, ok := result.(*yaml.Node) @@ -1092,7 +1283,7 @@ func TestMarshalYAMLInlineWithContext_PreserveReference_ViaLowLevel(t *testing.T assert.Equal(t, "#/components/schemas/TestRef", node.Content[1].Value) } -func TestMarshalYAMLInline_BundlingMode_ViaLowLevelRef(t *testing.T) { +func TestMarshalYAMLInline_ContextPreservesLocalComponentRefViaLowLevelRef(t *testing.T) { // Test bundling mode preserves refs when schema is in root index // Reference is set via low-level proxy (not refStr) @@ -1138,11 +1329,9 @@ func TestMarshalYAMLInline_BundlingMode_ViaLowLevelRef(t *testing.T) { schema: &lowRef, } - // Enable bundling mode - SetBundlingMode(true) - defer SetBundlingMode(false) - - result, err := proxy.MarshalYAMLInline() + ctx := NewInlineRenderContext() + ctx.PreserveLocalComponentRefs = true + result, err := proxy.MarshalYAMLInlineWithContext(ctx) require.NoError(t, err) node, ok := result.(*yaml.Node) diff --git a/datamodel/high/base/schema_test.go b/datamodel/high/base/schema_test.go index 3bf381fa..38b14b8b 100644 --- a/datamodel/high/base/schema_test.go +++ b/datamodel/high/base/schema_test.go @@ -2141,6 +2141,25 @@ oneOf: assert.NotContains(t, output, "meow:") } +func TestSchema_MarshalYAMLInlineWithContext_PreservesProgrammaticDiscriminatorRefs(t *testing.T) { + var node yaml.Node + require.NoError(t, yaml.Unmarshal([]byte("discriminator:\n propertyName: type\n"), &node)) + idx := index.NewSpecIndexWithConfig(&node, index.CreateOpenAPIIndexConfig()) + var lowSchema lowbase.Schema + require.NoError(t, lowSchema.Build(context.Background(), node.Content[0], idx)) + schema := NewSchema(&lowSchema) + schema.OneOf = []*SchemaProxy{CreateSchemaProxyRef("#/components/schemas/ProgrammaticOne")} + schema.AnyOf = []*SchemaProxy{CreateSchemaProxyRef("#/components/schemas/ProgrammaticAny")} + + result, err := schema.MarshalYAMLInlineWithContext(NewInlineRenderContext()) + require.NoError(t, err) + yamlBytes, err := yaml.Marshal(result) + require.NoError(t, err) + output := string(yamlBytes) + assert.Contains(t, output, "$ref: '#/components/schemas/ProgrammaticOne'") + assert.Contains(t, output, "$ref: '#/components/schemas/ProgrammaticAny'") +} + func TestSchema_MarshalYAMLInlineWithContext_NilContext_PreservesDiscriminatorRefs(t *testing.T) { // Test that with nil context (backward compatibility), discriminator refs are preserved diff --git a/datamodel/high/shared.go b/datamodel/high/shared.go index 4e5ee01b..8fca606c 100644 --- a/datamodel/high/shared.go +++ b/datamodel/high/shared.go @@ -15,6 +15,7 @@ package high import ( "context" + "errors" "fmt" "github.com/pb33f/libopenapi/datamodel/low" @@ -61,7 +62,7 @@ func RenderInlineWithContext(high, low, ctx any) (interface{}, error) { nb := NewNodeBuilder(high, low) nb.Resolve = true nb.RenderContext = ctx - return nb.Render(), nil + return nb.Render(), errors.Join(nb.Errors...) } // UnpackExtensions is a convenience function that makes it easy and simple to unpack an objects extensions diff --git a/datamodel/high/v3/callback.go b/datamodel/high/v3/callback.go index 8b9d8f2a..f5c79f9a 100644 --- a/datamodel/high/v3/callback.go +++ b/datamodel/high/v3/callback.go @@ -5,6 +5,7 @@ package v3 import ( "context" + "fmt" "sort" "github.com/pb33f/libopenapi/datamodel/high" @@ -241,10 +242,14 @@ func (c *Callback) marshalYAMLInlineInternal(ctx any) (interface{}, error) { for _, mp := range mapped { if mp.pi != nil { var rendered interface{} + var err error if ctx != nil { - rendered, _ = mp.pi.MarshalYAMLInlineWithContext(ctx) + rendered, err = mp.pi.MarshalYAMLInlineWithContext(ctx) } else { - rendered, _ = mp.pi.MarshalYAMLInline() + rendered, err = mp.pi.MarshalYAMLInline() + } + if err != nil { + return nil, fmt.Errorf("failed to render callback path %q inline: %w", mp.path, err) } kn := utils.CreateStringNode(mp.path) diff --git a/datamodel/high/v3/callback_test.go b/datamodel/high/v3/callback_test.go index d7a8ccef..3cc4cce4 100644 --- a/datamodel/high/v3/callback_test.go +++ b/datamodel/high/v3/callback_test.go @@ -15,6 +15,7 @@ import ( "github.com/pb33f/libopenapi/orderedmap" "github.com/pb33f/libopenapi/utils" "github.com/pb33f/testify/assert" + "github.com/pb33f/testify/require" "go.yaml.in/yaml/v4" ) @@ -86,6 +87,23 @@ func TestCallback_MarshalYAML(t *testing.T) { assert.Equal(t, k, strings.TrimSpace(string(rend))) } +func TestCallback_MarshalYAMLInlineWithContext_PropagatesPathItemError(t *testing.T) { + proxy := base.CreateSchemaProxyRefWithSchema("#/Cycle", &base.Schema{Description: "cycle"}) + ctx := base.NewInlineRenderContext() + require.False(t, ctx.StartRendering("#/Cycle")) + content := orderedmap.New[string, *MediaType]() + content.Set("application/json", &MediaType{Schema: proxy}) + responses := orderedmap.New[string, *Response]() + responses.Set("200", &Response{Description: "ok", Content: content}) + callback := &Callback{Expression: orderedmap.ToOrderedMap(map[string]*PathItem{ + "{$request.body#/callbackUrl}": {Get: &Operation{Responses: &Responses{Codes: responses}}}, + })} + + _, err := callback.MarshalYAMLInlineWithContext(ctx) + require.ErrorContains(t, err, "failed to render callback path") + require.ErrorContains(t, err, "circular reference") +} + func TestCallback_RenderInline(t *testing.T) { ext := orderedmap.New[string, *yaml.Node]() ext.Set("x-burgers", utils.CreateStringNode("why not?")) diff --git a/datamodel/high/v3/components.go b/datamodel/high/v3/components.go index 846f867e..3d8b8c56 100644 --- a/datamodel/high/v3/components.go +++ b/datamodel/high/v3/components.go @@ -4,6 +4,7 @@ package v3 import ( + "errors" "sync" "github.com/pb33f/libopenapi/datamodel" @@ -197,6 +198,17 @@ func (c *Components) MarshalYAMLInline() (interface{}, error) { return rendered, nil } +// MarshalYAMLInlineWithContext renders components with a shared inline render context. +func (c *Components) MarshalYAMLInlineWithContext(ctx any) (interface{}, error) { + c.warnPreservedComponentMapRefs() + nb := high.NewNodeBuilder(c, c.low) + nb.Resolve = true + nb.RenderContext = ctx + rendered := nb.Render() + c.preserveInvalidComponentMapRefs(rendered) + return rendered, errors.Join(nb.Errors...) +} + func (c *Components) warnPreservedComponentMapRefs() { if c == nil || c.low == nil { return diff --git a/datamodel/high/v3/document.go b/datamodel/high/v3/document.go index 3cea0862..efd42782 100644 --- a/datamodel/high/v3/document.go +++ b/datamodel/high/v3/document.go @@ -11,6 +11,7 @@ package v3 import ( "bytes" + "errors" "github.com/pb33f/libopenapi/datamodel/high" "github.com/pb33f/libopenapi/datamodel/high/base" @@ -196,6 +197,16 @@ func (d *Document) RenderInline() ([]byte, error) { return yaml.Marshal(di) } +// RenderInlineWithContext renders the document using one shared inline render +// context and propagates every NodeBuilder error before marshaling output. +func (d *Document) RenderInlineWithContext(ctx *base.InlineRenderContext) ([]byte, error) { + di, err := d.MarshalYAMLInlineWithContext(ctx) + if err != nil { + return nil, err + } + return yaml.Marshal(di) +} + // MarshalYAML will create a ready to render YAML representation of the Document object. func (d *Document) MarshalYAML() (interface{}, error) { nb := high.NewNodeBuilder(d, d.low) @@ -208,6 +219,24 @@ func (d *Document) MarshalYAMLInline() (interface{}, error) { return nb.Render(), nil } +// MarshalYAMLInlineWithContext creates the inline YAML node graph using one +// context for the complete document render and returns accumulated builder +// errors instead of silently emitting a partial document. +func (d *Document) MarshalYAMLInlineWithContext(ctx any) (interface{}, error) { + renderCtx, ok := ctx.(*base.InlineRenderContext) + if !ok || renderCtx == nil { + renderCtx = base.NewInlineRenderContext() + } + nb := high.NewNodeBuilder(d, d.low) + nb.Resolve = true + nb.RenderContext = renderCtx + node := nb.Render() + if err := errors.Join(nb.Errors...); err != nil { + return nil, err + } + return node, nil +} + func (d *Document) GetIndex() *index.SpecIndex { return d.Index } diff --git a/datamodel/high/v3/document_test.go b/datamodel/high/v3/document_test.go index c86c1333..c24715f9 100644 --- a/datamodel/high/v3/document_test.go +++ b/datamodel/high/v3/document_test.go @@ -18,6 +18,7 @@ import ( "time" "github.com/pb33f/libopenapi/datamodel" + highbase "github.com/pb33f/libopenapi/datamodel/high/base" v2 "github.com/pb33f/libopenapi/datamodel/high/v2" lowv2 "github.com/pb33f/libopenapi/datamodel/low/v2" lowv3 "github.com/pb33f/libopenapi/datamodel/low/v3" @@ -59,6 +60,31 @@ func TestNewDocument_Extensions(t *testing.T) { assert.Equal(t, "darkside", xSomethingSomething) } +func TestDocument_RenderInlineWithContext(t *testing.T) { + initTest() + document := NewDocument(lowDoc) + ctx := highbase.NewInlineRenderContext() + rendered, err := document.RenderInlineWithContext(ctx) + assert.NoError(t, err) + assert.Contains(t, string(rendered), "openapi:") + + node, err := document.MarshalYAMLInlineWithContext("wrong context") + assert.NoError(t, err) + assert.NotNil(t, node) + + broken := &Document{ + Version: "3.1.0", + Components: &Components{ + Schemas: orderedmap.ToOrderedMap(map[string]*highbase.SchemaProxy{ + "Broken": highbase.CreateSchemaProxyRef("#/Missing"), + }), + }, + } + _, err = broken.RenderInlineWithContext(highbase.NewInlineRenderContext()) + assert.Error(t, err) + +} + func TestNewDocument_ExternalDocs(t *testing.T) { initTest() h := NewDocument(lowDoc) diff --git a/datamodel/high/v3/media_type.go b/datamodel/high/v3/media_type.go index 1d90efa1..46090698 100644 --- a/datamodel/high/v3/media_type.go +++ b/datamodel/high/v3/media_type.go @@ -4,6 +4,8 @@ package v3 import ( + "errors" + "github.com/pb33f/libopenapi/datamodel" "github.com/pb33f/libopenapi/datamodel/high" "github.com/pb33f/libopenapi/datamodel/high/base" @@ -88,7 +90,7 @@ func (m *MediaType) MarshalYAMLInlineWithContext(ctx any) (interface{}, error) { nb := high.NewNodeBuilder(m, m.low) nb.Resolve = true nb.RenderContext = ctx - return nb.Render(), nil + return nb.Render(), errors.Join(nb.Errors...) } // ExtractContent takes in a complex and hard to navigate low-level content map, and converts it in to a much simpler diff --git a/datamodel/high/v3/media_type_test.go b/datamodel/high/v3/media_type_test.go index ba47d9d4..037f1766 100644 --- a/datamodel/high/v3/media_type_test.go +++ b/datamodel/high/v3/media_type_test.go @@ -17,6 +17,7 @@ import ( "github.com/pb33f/libopenapi/orderedmap" "github.com/pb33f/libopenapi/utils" "github.com/pb33f/testify/assert" + "github.com/pb33f/testify/require" "go.yaml.in/yaml/v4" ) @@ -108,6 +109,15 @@ example: testing a nice mutation` assert.Equal(t, op, strings.TrimSpace(string(yml))) } +func TestMediaType_MarshalYAMLInlineWithContext_PropagatesSchemaError(t *testing.T) { + proxy := base.CreateSchemaProxyRefWithSchema("#/Cycle", &base.Schema{Description: "cycle"}) + ctx := base.NewInlineRenderContext() + require.False(t, ctx.StartRendering("#/Cycle")) + + _, err := (&MediaType{Schema: proxy}).MarshalYAMLInlineWithContext(ctx) + require.ErrorContains(t, err, "circular reference") +} + func TestMediaType_MarshalYAML(t *testing.T) { // load the petstore spec data, _ := os.ReadFile("../../../test_specs/petstorev3.json") diff --git a/datamodel/high/v3/operation.go b/datamodel/high/v3/operation.go index 5f831a4e..25cc1e38 100644 --- a/datamodel/high/v3/operation.go +++ b/datamodel/high/v3/operation.go @@ -121,3 +121,8 @@ func (o *Operation) MarshalYAMLInline() (interface{}, error) { nb.Resolve = true return nb.Render(), nil } + +// MarshalYAMLInlineWithContext renders the operation with a shared inline render context. +func (o *Operation) MarshalYAMLInlineWithContext(ctx any) (interface{}, error) { + return high.RenderInlineWithContext(o, o.low, ctx) +} diff --git a/datamodel/high/v3/paths.go b/datamodel/high/v3/paths.go index c07edd5e..b50f80da 100644 --- a/datamodel/high/v3/paths.go +++ b/datamodel/high/v3/paths.go @@ -145,6 +145,15 @@ func (p *Paths) MarshalYAML() (interface{}, error) { } func (p *Paths) MarshalYAMLInline() (interface{}, error) { + return p.marshalYAMLInlineWithContext(nil) +} + +// MarshalYAMLInlineWithContext renders paths with a shared inline render context. +func (p *Paths) MarshalYAMLInlineWithContext(ctx any) (interface{}, error) { + return p.marshalYAMLInlineWithContext(ctx) +} + +func (p *Paths) marshalYAMLInlineWithContext(ctx any) (interface{}, error) { // map keys correctly. m := utils.CreateEmptyMapNode() type pathItem struct { @@ -177,6 +186,7 @@ func (p *Paths) MarshalYAMLInline() (interface{}, error) { nb := high.NewNodeBuilder(p, p.low) nb.Resolve = true + nb.RenderContext = ctx extNode := nb.Render() if extNode != nil && extNode.Content != nil { var label string @@ -197,7 +207,13 @@ func (p *Paths) MarshalYAMLInline() (interface{}, error) { }) for _, mp := range mapped { if mp.pi != nil { - rendered, err := mp.pi.MarshalYAMLInline() + var rendered interface{} + var err error + if ctx != nil { + rendered, err = mp.pi.MarshalYAMLInlineWithContext(ctx) + } else { + rendered, err = mp.pi.MarshalYAMLInline() + } if err != nil { return nil, fmt.Errorf("failed to render path '%s' inline: %w", mp.path, err) } diff --git a/datamodel/high/v3/responses.go b/datamodel/high/v3/responses.go index cdef1dca..ff482974 100644 --- a/datamodel/high/v3/responses.go +++ b/datamodel/high/v3/responses.go @@ -4,6 +4,7 @@ package v3 import ( + "errors" "fmt" "sort" @@ -155,6 +156,15 @@ func (r *Responses) MarshalYAML() (interface{}, error) { } func (r *Responses) MarshalYAMLInline() (interface{}, error) { + return r.marshalYAMLInlineWithContext(nil) +} + +// MarshalYAMLInlineWithContext renders responses with a shared inline render context. +func (r *Responses) MarshalYAMLInlineWithContext(ctx any) (interface{}, error) { + return r.marshalYAMLInlineWithContext(ctx) +} + +func (r *Responses) marshalYAMLInlineWithContext(ctx any) (interface{}, error) { // map keys correctly. m := utils.CreateEmptyMapNode() type responseItem struct { @@ -183,7 +193,11 @@ func (r *Responses) MarshalYAMLInline() (interface{}, error) { // extract extensions nb := high.NewNodeBuilder(r, r.low) nb.Resolve = true + nb.RenderContext = ctx extNode := nb.Render() + if err := errors.Join(nb.Errors...); err != nil { + return nil, err + } if extNode != nil && extNode.Content != nil { var label string for u := range extNode.Content { @@ -203,7 +217,16 @@ func (r *Responses) MarshalYAMLInline() (interface{}, error) { }) for _, mp := range mapped { if mp.resp != nil { - rendered, _ := mp.resp.MarshalYAMLInline() + var rendered interface{} + var err error + if ctx != nil { + rendered, err = mp.resp.MarshalYAMLInlineWithContext(ctx) + } else { + rendered, err = mp.resp.MarshalYAMLInline() + } + if err != nil { + return nil, fmt.Errorf("failed to render response '%s' inline: %w", mp.code, err) + } kn := utils.CreateStringNode(mp.code) kn.Style = mp.style diff --git a/datamodel/high/v3/responses_test.go b/datamodel/high/v3/responses_test.go index a2ea8688..d166af25 100644 --- a/datamodel/high/v3/responses_test.go +++ b/datamodel/high/v3/responses_test.go @@ -8,13 +8,28 @@ import ( "strings" "testing" + "github.com/pb33f/libopenapi/datamodel/high/base" "github.com/pb33f/libopenapi/datamodel/low" v3 "github.com/pb33f/libopenapi/datamodel/low/v3" "github.com/pb33f/libopenapi/index" + "github.com/pb33f/libopenapi/orderedmap" "github.com/pb33f/testify/assert" + "github.com/pb33f/testify/require" "go.yaml.in/yaml/v4" ) +func TestResponses_MarshalYAMLInlineWithContext_PropagatesDefaultError(t *testing.T) { + proxy := base.CreateSchemaProxyRefWithSchema("#/Cycle", &base.Schema{Description: "cycle"}) + ctx := base.NewInlineRenderContext() + require.False(t, ctx.StartRendering("#/Cycle")) + content := orderedmap.New[string, *MediaType]() + content.Set("application/json", &MediaType{Schema: proxy}) + responses := &Responses{Codes: orderedmap.New[string, *Response](), Default: &Response{Description: "default", Content: content}} + + _, err := responses.MarshalYAMLInlineWithContext(ctx) + require.ErrorContains(t, err, "circular reference") +} + // this test exists because the sample contract doesn't contain a // responses with *everything* populated, I had already written a ton of tests // with hard coded line and column numbers in them, changing the spec above the bottom will diff --git a/index/reference_identity.go b/index/reference_identity.go new file mode 100644 index 00000000..6240e412 --- /dev/null +++ b/index/reference_identity.go @@ -0,0 +1,31 @@ +// Copyright 2026 Princess B33f Heavy Industries / Dave Shanley +// SPDX-License-Identifier: MIT + +package index + +import ( + "net/url" + "path" + "path/filepath" + "strings" + + "github.com/pb33f/libopenapi/utils" +) + +// CanonicalReferenceIdentity normalizes the document portion of a resolved +// reference while preserving its JSON Pointer fragment. It is intended for +// identity comparisons, not for rewriting authored reference values. +func CanonicalReferenceIdentity(ref string) string { + base, fragment := SplitRefFragment(ref) + if base == "" { + return fragment + } + if parsed, err := url.Parse(base); err == nil && parsed.IsAbs() && parsed.Host != "" { + parsed.Path = path.Clean(parsed.Path) + return parsed.String() + fragment + } + base = utils.ReplaceWindowsDriveWithLinuxPath(base) + base = filepath.ToSlash(filepath.Clean(base)) + base = strings.TrimSuffix(base, "/.") + return base + fragment +} diff --git a/index/reference_identity_test.go b/index/reference_identity_test.go new file mode 100644 index 00000000..8a0cde82 --- /dev/null +++ b/index/reference_identity_test.go @@ -0,0 +1,18 @@ +// Copyright 2026 Princess B33f Heavy Industries / Dave Shanley +// SPDX-License-Identifier: MIT + +package index + +import ( + "testing" + + "github.com/pb33f/testify/assert" +) + +func TestCanonicalReferenceIdentity(t *testing.T) { + assert.Equal(t, "#/Thing", CanonicalReferenceIdentity("#/Thing")) + assert.Equal(t, "/schemas/thing.yaml#/Thing", CanonicalReferenceIdentity("/schemas/./thing.yaml#/Thing")) + assert.Equal(t, "/schemas/thing.yaml#/Thing", CanonicalReferenceIdentity(`C:\schemas\thing.yaml#/Thing`)) + assert.Equal(t, "https://example.com/schemas/thing.yaml#/Thing", + CanonicalReferenceIdentity("https://example.com/schemas/other/../thing.yaml#/Thing")) +}