From 6f86a18459589eec93fbaa305001a023652f8cef Mon Sep 17 00:00:00 2001 From: quobix Date: Wed, 25 Feb 2026 10:48:01 -0500 Subject: [PATCH] Address issue #533 Adds a new `CreateSchemaProxyRefWithSchema` function. --- datamodel/high/base/schema_proxy.go | 78 ++++++-- datamodel/high/base/schema_proxy_test.go | 241 +++++++++++++++++++++++ 2 files changed, 298 insertions(+), 21 deletions(-) diff --git a/datamodel/high/base/schema_proxy.go b/datamodel/high/base/schema_proxy.go index db8edf23..0240b9ee 100644 --- a/datamodel/high/base/schema_proxy.go +++ b/datamodel/high/base/schema_proxy.go @@ -202,6 +202,15 @@ func CreateSchemaProxyRef(ref string) *SchemaProxy { return &SchemaProxy{refStr: ref, lock: &sync.Mutex{}} } +// CreateSchemaProxyRefWithSchema creates a SchemaProxy that carries both a $ref and sibling schema +// properties. This supports JSON Schema 2020-12 section 7.7.1.1 where $ref can coexist with other +// keywords. When rendered, $ref appears first followed by the schema's sibling properties. +// +// If schema is nil, the result behaves identically to CreateSchemaProxyRef. +func CreateSchemaProxyRefWithSchema(ref string, schema *Schema) *SchemaProxy { + return &SchemaProxy{refStr: ref, rendered: schema, lock: &sync.Mutex{}} +} + // GetValueNode returns the value node of the SchemaProxy. func (sp *SchemaProxy) GetValueNode() *yaml.Node { if sp.schema != nil { @@ -377,6 +386,27 @@ func (sp *SchemaProxy) GoLowUntyped() any { return sp.schema.Value } +// isRefWithSiblings returns true when this is a programmatically-created proxy +// that carries both a $ref and sibling schema properties. +func (sp *SchemaProxy) isRefWithSiblings() bool { + return sp.refStr != "" && sp.rendered != nil && sp.schema == nil +} + +// renderRefWithSiblings builds a YAML mapping node containing $ref as the +// first key followed by all rendered schema sibling properties. +func (sp *SchemaProxy) renderRefWithSiblings() *yaml.Node { + nb := high.NewNodeBuilder(sp.rendered, nil) + node := nb.Render() + refKey := utils.CreateStringNode("$ref") + refVal := utils.CreateStringNode(sp.refStr) + refVal.Style = yaml.SingleQuotedStyle + content := make([]*yaml.Node, 0, len(node.Content)+2) + content = append(content, refKey, refVal) + content = append(content, node.Content...) + node.Content = content + return node +} + // Render will return a YAML representation of the Schema object as a byte slice. func (sp *SchemaProxy) Render() ([]byte, error) { return yaml.Marshal(sp) @@ -384,19 +414,18 @@ func (sp *SchemaProxy) Render() ([]byte, error) { // MarshalYAML will create a ready to render YAML representation of the SchemaProxy object. func (sp *SchemaProxy) MarshalYAML() (interface{}, error) { - var s *Schema - var err error - // if this schema isn't a reference, then build it out. if !sp.IsReference() { - s, err = sp.BuildSchema() + s, err := sp.BuildSchema() if err != nil { return nil, err } nb := high.NewNodeBuilder(s, s.low) return nb.Render(), nil - } else { - return sp.GetReferenceNode(), nil } + if sp.isRefWithSiblings() { + return sp.renderRefWithSiblings(), nil + } + return sp.GetReferenceNode(), nil } // getInlineRenderKey generates a unique key for tracking this schema during inline rendering. @@ -467,13 +496,22 @@ func (sp *SchemaProxy) MarshalYAMLInline() (interface{}, error) { } func (sp *SchemaProxy) marshalYAMLInlineInternal(ctx *InlineRenderContext) (interface{}, error) { + // refNode returns the correct reference YAML node — with sibling + // properties when this proxy carries both a $ref and schema data. + refNode := func() *yaml.Node { + if sp.isRefWithSiblings() { + return sp.renderRefWithSiblings() + } + return sp.GetReferenceNode() + } + // 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 sp.GetReferenceNode(), nil + return refNode(), nil } } @@ -494,7 +532,7 @@ func (sp *SchemaProxy) marshalYAMLInlineInternal(ctx *InlineRenderContext) (inte rootIdx := rolodex.GetRootIndex() // If the schema is in the root index, preserve the ref if rootIdx != nil && schemaIdx == rootIdx { - return sp.GetReferenceNode(), nil + return refNode(), nil } } } @@ -510,7 +548,7 @@ 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() { - return sp.GetReferenceNode(), + return refNode(), fmt.Errorf("schema render failure, circular reference: `%s`", sp.GetReference()) } // For inline schemas, return an empty map to avoid infinite recursion @@ -542,20 +580,16 @@ func (sp *SchemaProxy) marshalYAMLInlineInternal(ctx *InlineRenderContext) (inte for _, c := range circ { if sp.IsReference() { if sp.GetReference() == c.LoopPoint.Definition { - // nope - return sp.GetReferenceNode(), - cirError((c.LoopPoint.Definition)) + return refNode(), cirError(c.LoopPoint.Definition) } - basePath := sp.GoLow().GetIndex().GetSpecAbsolutePath() + basePath := idx.GetSpecAbsolutePath() if !filepath.IsAbs(basePath) && !strings.HasPrefix(basePath, "http") { basePath, _ = filepath.Abs(basePath) } if basePath == c.LoopPoint.FullDefinition { - // we loop on our-self - return sp.GetReferenceNode(), - cirError((c.LoopPoint.Definition)) + return refNode(), cirError(c.LoopPoint.Definition) } a := utils.ReplaceWindowsDriveWithLinuxPath(strings.Replace(c.LoopPoint.FullDefinition, basePath, "", 1)) b := sp.GetReference() @@ -577,17 +611,14 @@ func (sp *SchemaProxy) marshalYAMLInlineInternal(ctx *InlineRenderContext) (inte bBase, bFragment := index.SplitRefFragment(b) if aFragment != "" && bFragment != "" && aFragment == bFragment { - return sp.GetReferenceNode(), - cirError((c.LoopPoint.Definition)) + return refNode(), cirError(c.LoopPoint.Definition) } if aFragment == "" && bFragment == "" { aNorm := strings.TrimPrefix(strings.TrimPrefix(aBase, "./"), "/") bNorm := strings.TrimPrefix(strings.TrimPrefix(bBase, "./"), "/") if aNorm != "" && bNorm != "" && aNorm == bNorm { - // nope - return sp.GetReferenceNode(), - cirError((c.LoopPoint.Definition)) + return refNode(), cirError(c.LoopPoint.Definition) } } } @@ -598,6 +629,11 @@ func (sp *SchemaProxy) marshalYAMLInlineInternal(ctx *InlineRenderContext) (inte return nil, err } if s != nil { + // For programmatic ref+siblings proxies, render directly to avoid nil-deref + // in Schema.MarshalYAMLInlineWithContext which assumes s.GoLow() is non-nil. + if sp.isRefWithSiblings() { + return sp.renderRefWithSiblings(), nil + } // Delegate to Schema.MarshalYAMLInlineWithContext to ensure discriminator handling is applied // and cycle detection context is propagated. // Schema.MarshalYAMLInlineWithContext sets preserveReference on OneOf/AnyOf items when diff --git a/datamodel/high/base/schema_proxy_test.go b/datamodel/high/base/schema_proxy_test.go index 9c0ff8a1..e23ec6a0 100644 --- a/datamodel/high/base/schema_proxy_test.go +++ b/datamodel/high/base/schema_proxy_test.go @@ -1406,3 +1406,244 @@ func TestClearInlineRenderingTracker(t *testing.T) { // Idempotent: clearing an empty map should not panic. ClearInlineRenderingTracker() } + +// --- Tests for CreateSchemaProxyRefWithSchema --- + +func TestCreateSchemaProxyRefWithSchema(t *testing.T) { + schema := &Schema{Description: "A pet with extra context"} + sp := CreateSchemaProxyRefWithSchema("#/components/schemas/Pet", schema) + + assert.True(t, sp.IsReference()) + assert.Equal(t, "#/components/schemas/Pet", sp.GetReference()) + assert.Equal(t, schema, sp.Schema()) + assert.NotNil(t, sp.GetReferenceNode()) + assert.Nil(t, sp.GoLow()) +} + +func TestCreateSchemaProxyRefWithSchema_Render(t *testing.T) { + schema := &Schema{ + Description: "A pet with extra context", + Type: []string{"object"}, + } + sp := CreateSchemaProxyRefWithSchema("#/components/schemas/Pet", schema) + + result, err := sp.MarshalYAML() + require.NoError(t, err) + + node, ok := result.(*yaml.Node) + require.True(t, ok) + assert.Equal(t, yaml.MappingNode, node.Kind) + + // $ref should be the first key + require.GreaterOrEqual(t, len(node.Content), 2) + assert.Equal(t, "$ref", node.Content[0].Value) + assert.Equal(t, "#/components/schemas/Pet", node.Content[1].Value) + + // Render to YAML bytes to verify full output + rendered, err := sp.Render() + require.NoError(t, err) + renderedStr := string(rendered) + assert.Contains(t, renderedStr, "$ref") + assert.Contains(t, renderedStr, "#/components/schemas/Pet") + assert.Contains(t, renderedStr, "description: A pet with extra context") +} + +func TestCreateSchemaProxyRefWithSchema_NilSchema(t *testing.T) { + // When schema is nil, behaves like CreateSchemaProxyRef + sp := CreateSchemaProxyRefWithSchema("#/components/schemas/Pet", nil) + + assert.True(t, sp.IsReference()) + assert.Equal(t, "#/components/schemas/Pet", sp.GetReference()) + assert.Nil(t, sp.Schema()) + assert.False(t, sp.isRefWithSiblings()) + + // MarshalYAML should produce ref-only output + result, err := sp.MarshalYAML() + require.NoError(t, err) + + node, ok := result.(*yaml.Node) + require.True(t, ok) + assert.Equal(t, 2, len(node.Content)) + assert.Equal(t, "$ref", node.Content[0].Value) +} + +func TestCreateSchemaProxyRefWithSchema_RoundTrip(t *testing.T) { + schema := &Schema{ + Description: "Round trip test", + } + sp := CreateSchemaProxyRefWithSchema("#/components/schemas/Example", schema) + + rendered, err := sp.Render() + require.NoError(t, err) + + // Unmarshal and verify both $ref and sibling properties are present + var result map[string]interface{} + err = yaml.Unmarshal(rendered, &result) + require.NoError(t, err) + assert.Equal(t, "#/components/schemas/Example", result["$ref"]) + assert.Equal(t, "Round trip test", result["description"]) +} + +func TestCreateSchemaProxyRefWithSchema_InlineRender(t *testing.T) { + schema := &Schema{ + Description: "Inline test", + } + sp := CreateSchemaProxyRefWithSchema("#/components/schemas/Pet", schema) + + result, err := sp.MarshalYAMLInline() + require.NoError(t, err) + + node, ok := result.(*yaml.Node) + require.True(t, ok) + assert.Equal(t, yaml.MappingNode, node.Kind) + + // Should have $ref as first key and description + require.GreaterOrEqual(t, len(node.Content), 4) + assert.Equal(t, "$ref", node.Content[0].Value) + assert.Equal(t, "#/components/schemas/Pet", node.Content[1].Value) +} + +func TestCreateSchemaProxyRefWithSchema_InlinePreservedRef(t *testing.T) { + schema := &Schema{ + Description: "Preserved ref test", + } + sp := CreateSchemaProxyRefWithSchema("#/components/schemas/Pet", schema) + + ctx := NewInlineRenderContext() + ctx.MarkRefAsPreserved("#/components/schemas/Pet") + + result, err := sp.MarshalYAMLInlineWithContext(ctx) + require.NoError(t, err) + + node, ok := result.(*yaml.Node) + require.True(t, ok) + assert.Equal(t, yaml.MappingNode, node.Kind) + + // Even with preservation, siblings should still be present because + // refNode() returns renderRefWithSiblings() for ref+siblings proxies + assert.Equal(t, "$ref", node.Content[0].Value) + assert.Equal(t, "#/components/schemas/Pet", node.Content[1].Value) + require.GreaterOrEqual(t, len(node.Content), 4) // $ref key+val + description key+val +} + +func TestCreateSchemaProxyRefWithSchema_CircularRefSafe(t *testing.T) { + // Verify inline rendering doesn't panic when rendered Schema has a non-nil low-level + const ymlComponents = `components: + schemas: + Pet: + type: object + properties: + name: + type: string` + + var idxNode yaml.Node + err := yaml.Unmarshal([]byte(ymlComponents), &idxNode) + require.NoError(t, err) + idx := index.NewSpecIndexWithConfig(&idxNode, index.CreateOpenAPIIndexConfig()) + + // Build a real schema from the spec + const ymlSchema = `type: object +properties: + name: + type: string` + var node yaml.Node + _ = yaml.Unmarshal([]byte(ymlSchema), &node) + + lowProxy := new(lowbase.SchemaProxy) + err = lowProxy.Build(context.Background(), nil, node.Content[0], idx) + require.NoError(t, err) + + parsedProxy := NewSchemaProxy(&low.NodeReference[*lowbase.SchemaProxy]{ + Value: lowProxy, + ValueNode: node.Content[0], + }) + parsedSchema := parsedProxy.Schema() + require.NotNil(t, parsedSchema) + + // Create ref+siblings proxy using the parsed schema (which has non-nil low) + sp := CreateSchemaProxyRefWithSchema("#/components/schemas/Pet", parsedSchema) + + // MarshalYAML should not panic + result, err := sp.MarshalYAML() + require.NoError(t, err) + yamlNode := result.(*yaml.Node) + assert.Equal(t, "$ref", yamlNode.Content[0].Value) +} + +func TestCreateSchemaProxyRefWithSchema_NilLowInline(t *testing.T) { + // Plain high-level schema with low == nil must not panic during inline rendering + sp := CreateSchemaProxyRefWithSchema("#/components/schemas/Test", &Schema{Description: "test"}) + + // MarshalYAMLInline must not panic (the schema.go:628 nil-deref path is avoided) + result, err := sp.MarshalYAMLInline() + require.NoError(t, err) + + node, ok := result.(*yaml.Node) + require.True(t, ok) + assert.Equal(t, yaml.MappingNode, node.Kind) + assert.Equal(t, "$ref", node.Content[0].Value) + assert.Equal(t, "#/components/schemas/Test", node.Content[1].Value) +} + +func TestCreateSchemaProxyRefWithSchema_CycleDetection(t *testing.T) { + // Verify the refNode() closure is used when a cycle is detected for a ref+siblings proxy + schema := &Schema{Description: "cyclic sibling"} + sp := CreateSchemaProxyRefWithSchema("#/components/schemas/Pet", schema) + + ctx := NewInlineRenderContext() + // Pre-mark the ref as being rendered to simulate a cycle + ctx.StartRendering("#/components/schemas/Pet") + + result, err := sp.MarshalYAMLInlineWithContext(ctx) + require.Error(t, err) + assert.Contains(t, err.Error(), "circular reference") + + node, ok := result.(*yaml.Node) + require.True(t, ok) + // Should include siblings in the ref node via refNode() -> renderRefWithSiblings() + assert.Equal(t, "$ref", node.Content[0].Value) + assert.Equal(t, "#/components/schemas/Pet", node.Content[1].Value) + // Description sibling must be present + rendered, renderErr := yaml.Marshal(node) + require.NoError(t, renderErr) + assert.Contains(t, string(rendered), "description: cyclic sibling") +} + +func TestCreateSchemaProxyRefWithSchema_BundlingMode(t *testing.T) { + // Verify ref+siblings proxy works in bundling mode without panic. + // Since there is no low-level backing, the bundling mode check passes through. + for IsBundlingMode() { + SetBundlingMode(false) + } + SetBundlingMode(true) + defer SetBundlingMode(false) + + schema := &Schema{Description: "bundled sibling"} + sp := CreateSchemaProxyRefWithSchema("#/components/schemas/Pet", schema) + + result, err := sp.MarshalYAMLInline() + require.NoError(t, err) + + node, ok := result.(*yaml.Node) + require.True(t, ok) + assert.Equal(t, "$ref", node.Content[0].Value) + assert.Equal(t, "#/components/schemas/Pet", node.Content[1].Value) + rendered, renderErr := yaml.Marshal(node) + require.NoError(t, renderErr) + assert.Contains(t, string(rendered), "description: bundled sibling") +} + +func TestCreateSchemaProxyRefWithSchema_IsRefWithSiblings(t *testing.T) { + // Verify isRefWithSiblings() returns correct values for all factory types + refOnly := CreateSchemaProxyRef("#/components/schemas/Pet") + assert.False(t, refOnly.isRefWithSiblings()) + + schemaOnly := CreateSchemaProxy(&Schema{Description: "test"}) + assert.False(t, schemaOnly.isRefWithSiblings()) + + refWithSchema := CreateSchemaProxyRefWithSchema("#/components/schemas/Pet", &Schema{Description: "test"}) + assert.True(t, refWithSchema.isRefWithSiblings()) + + refWithNilSchema := CreateSchemaProxyRefWithSchema("#/components/schemas/Pet", nil) + assert.False(t, refWithNilSchema.isRefWithSiblings()) +}