Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions datamodel/high/node_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,9 @@ func (n *NodeBuilder) AddYAMLNode(parent *yaml.Node, entry *nodes.NodeEntry) *ya
sl := utils.CreateEmptySequenceNode()
skip := false
for i := 0; i < m.Len(); i++ {
// Reset skip at the start of each iteration to handle items without low-level models
// (e.g., newly created high-level objects appended to an existing slice)
skip = false
sqi := m.Index(i).Interface()
// check if this is a reference.
if glu, ok := sqi.(GoesLowUntyped); ok {
Expand All @@ -406,11 +409,7 @@ func (n *NodeBuilder) AddYAMLNode(parent *yaml.Node, entry *nodes.NodeEntry) *ya
if !n.Resolve {
sl.Content = append(sl.Content, n.renderReference(glu.GoLowUntyped().(low.IsReferenced)))
skip = true
} else {
skip = false
}
} else {
skip = false
}
}
}
Expand Down
192 changes: 192 additions & 0 deletions datamodel/high/node_builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1406,3 +1406,195 @@ func TestNodeBuilder_RenderContext_FallbackToMarshalYAML_Slice(t *testing.T) {
data, _ := yaml.Marshal(node)
assert.Contains(t, string(data), "basic-item1")
}

// nilLowRenderable implements GoesLowUntyped but returns nil for GoLowUntyped()
// This simulates a newly created high-level object without a low-level model
type nilLowRenderable struct {
Value string
}

func (n *nilLowRenderable) MarshalYAML() (interface{}, error) {
return utils.CreateStringNode("nillow-" + n.Value), nil
}

func (n *nilLowRenderable) GoLowUntyped() any {
return nil // Simulates newly created high-level object without low-level model
}

type testWithMixedSlice struct {
Items []*valueReferenceStruct `yaml:"items,omitempty"`
}

type testWithNilLowSlice struct {
Items []*nilLowRenderable `yaml:"items,omitempty"`
}

// TestNodeBuilder_SliceRefFollowedByNilLow tests the bug fix for rendering slices
// where a $ref item is followed by a newly created item without a low-level model.
// Before the fix, the 'skip' variable was not reset between iterations, causing
// items following a $ref to be incorrectly skipped when they had no low-level model.
func TestNodeBuilder_SliceRefFollowedByNilLow(t *testing.T) {
// Create a slice with a $ref followed by a new item without low-level model
refItem := &valueReferenceStruct{
ref: true,
refStr: "#/components/parameters/RefId",
Value: "refValue",
}
newItem := &valueReferenceStruct{
ref: false, // Not a reference
Value: "newValue",
}

// The new item's GoLowUntyped returns a pointer to itself (not nil),
// but it's not a reference, so it should be rendered normally.
// However, to truly test the nil case, we need a different approach.

ty := []*valueReferenceStruct{refItem, newItem}
t1 := test1{
Throg: ty,
}

nb := NewNodeBuilder(&t1, &t1)
node := nb.Render()

data, _ := yaml.Marshal(node)

// Both items should be rendered - the ref as $ref and the new item normally
assert.Contains(t, string(data), "$ref: '#/components/parameters/RefId'")
assert.Contains(t, string(data), "pizza") // newItem renders as "pizza" via MarshalYAML
}

// testMixedItem can have either a reference low-level model or nil low-level model
type testMixedItem struct {
ref bool
refStr string
Value string
hasLowRef bool // if true, GoLowUntyped returns a reference; if false, returns nil
}

func (t *testMixedItem) MarshalYAML() (interface{}, error) {
return utils.CreateStringNode("mixed-" + t.Value), nil
}

func (t *testMixedItem) GoLowUntyped() any {
if t.hasLowRef {
return t
}
return nil
}

func (t *testMixedItem) IsReference() bool {
return t.ref
}

func (t *testMixedItem) GetReference() string {
return t.refStr
}

func (t *testMixedItem) SetReference(ref string, _ *yaml.Node) {
t.refStr = ref
}

func (t *testMixedItem) GetReferenceNode() *yaml.Node {
return nil
}

type testWithMixedItems struct {
Items []*testMixedItem `yaml:"items,omitempty"`
}

// TestNodeBuilder_SliceRefFollowedByNilLowItem tests the specific bug case:
// a $ref item followed by an item with nil GoLowUntyped() (newly created high-level object)
func TestNodeBuilder_SliceRefFollowedByNilLowItem(t *testing.T) {
// Create a slice where:
// 1. First item IS a reference with a low-level model
// 2. Second item has NO low-level model (GoLowUntyped returns nil)
refItem := &testMixedItem{
ref: true,
refStr: "#/components/parameters/RefId",
Value: "refValue",
hasLowRef: true, // Has a low-level reference
}
newItem := &testMixedItem{
ref: false,
Value: "newValue",
hasLowRef: false, // NO low-level model (simulates newly created item)
}

ty := []*testMixedItem{refItem, newItem}
t1 := testWithMixedItems{
Items: ty,
}

nb := NewNodeBuilder(&t1, nil)
node := nb.Render()

data, _ := yaml.Marshal(node)
result := strings.TrimSpace(string(data))

// The bug was that 'newItem' would be skipped because 'skip' wasn't reset
// after processing the $ref item, and newItem's GoLowUntyped() returns nil.
// After the fix, both items should be rendered.
assert.Contains(t, result, "$ref: '#/components/parameters/RefId'", "Reference item should be rendered")
assert.Contains(t, result, "mixed-newValue", "New item without low-level model should also be rendered")
}

// TestNodeBuilder_SliceNilLowFollowedByRef tests the reverse case:
// an item with nil GoLowUntyped() followed by a $ref (should work in both cases)
func TestNodeBuilder_SliceNilLowFollowedByRef(t *testing.T) {
// Create a slice where:
// 1. First item has NO low-level model
// 2. Second item IS a reference with a low-level model
newItem := &testMixedItem{
ref: false,
Value: "newValue",
hasLowRef: false, // NO low-level model
}
refItem := &testMixedItem{
ref: true,
refStr: "#/components/parameters/RefId",
Value: "refValue",
hasLowRef: true, // Has a low-level reference
}

ty := []*testMixedItem{newItem, refItem}
t1 := testWithMixedItems{
Items: ty,
}

nb := NewNodeBuilder(&t1, nil)
node := nb.Render()

data, _ := yaml.Marshal(node)
result := strings.TrimSpace(string(data))

// Both items should be rendered
assert.Contains(t, result, "mixed-newValue", "New item without low-level model should be rendered")
assert.Contains(t, result, "$ref: '#/components/parameters/RefId'", "Reference item should be rendered")
}

// TestNodeBuilder_SliceMultipleRefsAndNilLow tests multiple refs interspersed with nil-low items
func TestNodeBuilder_SliceMultipleRefsAndNilLow(t *testing.T) {
items := []*testMixedItem{
{ref: true, refStr: "#/ref1", Value: "ref1", hasLowRef: true},
{ref: false, Value: "new1", hasLowRef: false}, // nil low
{ref: true, refStr: "#/ref2", Value: "ref2", hasLowRef: true},
{ref: false, Value: "new2", hasLowRef: false}, // nil low
}

t1 := testWithMixedItems{
Items: items,
}

nb := NewNodeBuilder(&t1, nil)
node := nb.Render()

data, _ := yaml.Marshal(node)
result := strings.TrimSpace(string(data))

// All items should be rendered
assert.Contains(t, result, "$ref: '#/ref1'")
assert.Contains(t, result, "mixed-new1")
assert.Contains(t, result, "$ref: '#/ref2'")
assert.Contains(t, result, "mixed-new2")
}
143 changes: 143 additions & 0 deletions document_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -887,6 +887,149 @@ func TestDocument_InputAsJSON_LargeIndent(t *testing.T) {
assert.Equal(t, d, strings.TrimSpace(string(rend)))
}

// TestDocument_AppendParameterAfterRef tests the bug fix for appending new parameters
// to an operation that has $ref parameters. Before the fix, if a $ref was the last
// parameter in the original spec, any appended parameters would be silently dropped
// during rendering because the 'skip' flag wasn't reset between slice iterations.
func TestDocument_AppendParameterAfterRef(t *testing.T) {
spec := `openapi: "3.1.0"
info:
title: Test
version: 1.0.0
paths:
/test/{id}:
get:
parameters:
- $ref: '#/components/parameters/IdParam'
responses:
"200":
description: ok
components:
parameters:
IdParam:
name: id
in: path
required: true
schema:
type: string`

doc, err := NewDocument([]byte(spec))
if err != nil {
t.Fatalf("failed to create document: %v", err)
}

result, errs := doc.BuildV3Model()
assert.NoError(t, errs)

// Get the operation and append a new parameter
pathItem := result.Model.Paths.PathItems.GetOrZero("/test/{id}")
assert.NotNil(t, pathItem)
assert.NotNil(t, pathItem.Get)

// Verify we have the original $ref parameter
assert.Len(t, pathItem.Get.Parameters, 1)

// Append a new parameter (simulating what the user does)
newParam := &v3high.Parameter{
Name: "Host",
In: "header",
}
pathItem.Get.Parameters = append(pathItem.Get.Parameters, newParam)

// Render and reload
_, newDoc, _, err := doc.RenderAndReload()
assert.NoError(t, err)

// Build the new model
newResult, errs := newDoc.BuildV3Model()
assert.NoError(t, errs)

// Check that the new parameter is present
newPathItem := newResult.Model.Paths.PathItems.GetOrZero("/test/{id}")
assert.NotNil(t, newPathItem)
assert.NotNil(t, newPathItem.Get)

// This was the bug: the new "Host" parameter was being dropped
assert.Len(t, newPathItem.Get.Parameters, 2, "Expected 2 parameters (original $ref + appended Host)")

// Verify the Host parameter is present
foundHost := false
for _, p := range newPathItem.Get.Parameters {
if p.Name == "Host" && p.In == "header" {
foundHost = true
break
}
}
assert.True(t, foundHost, "Expected to find the appended 'Host' header parameter")
}

// TestDocument_AppendParameterBeforeRef tests that appending works when $ref is not last
func TestDocument_AppendParameterBeforeRef(t *testing.T) {
spec := `openapi: "3.1.0"
info:
title: Test
version: 1.0.0
paths:
/test/{id}:
get:
parameters:
- name: existing
in: query
schema:
type: string
- $ref: '#/components/parameters/IdParam'
responses:
"200":
description: ok
components:
parameters:
IdParam:
name: id
in: path
required: true
schema:
type: string`

doc, err := NewDocument([]byte(spec))
if err != nil {
t.Fatalf("failed to create document: %v", err)
}

result, errs := doc.BuildV3Model()
assert.NoError(t, errs)

// Get the operation and append a new parameter
pathItem := result.Model.Paths.PathItems.GetOrZero("/test/{id}")

// Append a new parameter
newParam := &v3high.Parameter{
Name: "Host",
In: "header",
}
pathItem.Get.Parameters = append(pathItem.Get.Parameters, newParam)

// Render and reload
_, newDoc, _, err := doc.RenderAndReload()
assert.NoError(t, err)

newResult, errs := newDoc.BuildV3Model()
assert.NoError(t, errs)

newPathItem := newResult.Model.Paths.PathItems.GetOrZero("/test/{id}")

// Should have 3 parameters: existing + $ref + Host
assert.Len(t, newPathItem.Get.Parameters, 3, "Expected 3 parameters")

foundHost := false
for _, p := range newPathItem.Get.Parameters {
if p.Name == "Host" && p.In == "header" {
foundHost = true
break
}
}
assert.True(t, foundHost, "Expected to find the appended 'Host' header parameter")
}

func TestDocument_RenderWithIndention(t *testing.T) {
spec := `openapi: "3.1.0"
info:
Expand Down
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,6 @@ require (
github.com/kr/text v0.2.0 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
github.com/rogpeppe/go-internal v1.9.0 // indirect
golang.org/x/sync v0.19.0 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
)
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu
github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U=
go.yaml.in/yaml/v4 v4.0.0-rc.3 h1:3h1fjsh1CTAPjW7q/EMe+C8shx5d8ctzZTrLcs/j8Go=
go.yaml.in/yaml/v4 v4.0.0-rc.3/go.mod h1:aZqd9kCMsGL7AuUv/m/PvWLdg5sjJsZ4oHDEnfPPfY0=
golang.org/x/sync v0.19.0 h1:vV+1eWNmZ5geRlYjzm2adRgW2/mcpevXNg50YZtPCE4=
golang.org/x/sync v0.19.0/go.mod h1:9KTHXmSnoGruLpwFjVSX0lNNA75CykiMECbovNTZqGI=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 h1:YR8cESwS4TdDjEe65xsg0ogRM/Nc3DYOhEAlW+xobZo=
gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
Expand Down
Loading