From 012c507eace498e1bfe7a73ff4f5d0cf6b216d30 Mon Sep 17 00:00:00 2001 From: quobix Date: Mon, 29 Jun 2026 08:53:39 -0400 Subject: [PATCH] Address #597 added a `MarshalYAML` to the ordered map struct wrapper to prevent mutations. --- issue597_test.go | 82 +++++++++++++++++ orderedmap/builder.go | 60 ++++++++++++ orderedmap/orderedmap_test.go | 169 ++++++++++++++++++++++++++++++++++ 3 files changed, 311 insertions(+) create mode 100644 issue597_test.go diff --git a/issue597_test.go b/issue597_test.go new file mode 100644 index 000000000..d37375dfa --- /dev/null +++ b/issue597_test.go @@ -0,0 +1,82 @@ +// Copyright 2026 Princess Beef Heavy Industries / Dave Shanley +// https://pb33f.io +// SPDX-License-Identifier: MIT +package libopenapi + +import ( + "testing" + + "github.com/pb33f/libopenapi/datamodel" + "github.com/pb33f/libopenapi/datamodel/low" + "github.com/pb33f/libopenapi/orderedmap" + "github.com/pb33f/testify/require" + "go.yaml.in/yaml/v4" +) + +const issue597Spec = `openapi: 3.1.0 +info: + title: Common schemas + version: 0.0.1 +servers: + - url: http://localhost:8080/ +components: + schemas: + foo: + type: object + properties: + hello: + type: string + world: + type: string + x-custom: true +` + +func TestIssue597SchemaExtensionMarshalYAMLDoesNotChangeLowHash(t *testing.T) { + tests := []struct { + name string + marshal func(*orderedmap.Map[string, *yaml.Node]) error + }{ + { + name: "direct MarshalYAML", + marshal: func(extensions *orderedmap.Map[string, *yaml.Node]) error { + _, err := extensions.MarshalYAML() + return err + }, + }, + { + name: "yaml Marshal", + marshal: func(extensions *orderedmap.Map[string, *yaml.Node]) error { + _, err := yaml.Marshal(extensions) + return err + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + doc, err := NewDocumentWithConfiguration([]byte(issue597Spec), &datamodel.DocumentConfiguration{ + ExtractRefsSequentially: true, + }) + require.NoError(t, err) + + v3Model, err := doc.BuildV3Model() + require.NoError(t, err) + + schema := v3Model.Model.Components.Schemas.Value("foo").Schema() + lowSchema := schema.GoLow() + lowExtension := low.FindItemInOrderedMap[*yaml.Node]("x-custom", lowSchema.GetExtensions()) + require.NotNil(t, lowExtension) + require.Equal(t, "!!bool", lowExtension.Value.Tag) + + initialHash := lowSchema.Hash() + low.ClearHashCache() + + require.NoError(t, tt.marshal(schema.Extensions)) + require.Equal(t, "!!bool", lowExtension.Value.Tag) + require.Equal(t, "true", lowExtension.Value.Value) + + low.ClearHashCache() + require.Equal(t, initialHash, lowSchema.Hash()) + }) + } +} diff --git a/orderedmap/builder.go b/orderedmap/builder.go index 240314589..057a965a8 100644 --- a/orderedmap/builder.go +++ b/orderedmap/builder.go @@ -2,6 +2,7 @@ package orderedmap import ( "fmt" + "reflect" "strings" "github.com/pb33f/libopenapi/datamodel/high/nodes" @@ -33,6 +34,65 @@ type findValueUntyped interface { FindValueUntyped(k string) any } +// MarshalYAML implements yaml.Marshaler for libopenapi's ordered map wrapper. +func (o *Map[K, V]) MarshalYAML() (interface{}, error) { + if o == nil { + return nil, nil + } + + node := yaml.Node{Kind: yaml.MappingNode} + for pair := First(o); pair != nil; pair = pair.Next() { + keyNode := &yaml.Node{} + keyValue, err := encodeMarshalYAMLValue(pair.Key()) + if err != nil { + return nil, err + } + if err = keyNode.Encode(keyValue); err != nil { + return nil, err + } + + valueNode := &yaml.Node{} + value, err := encodeMarshalYAMLValue(pair.Value()) + if err != nil { + return nil, err + } + if err = valueNode.Encode(value); err != nil { + return nil, err + } + + node.Content = append(node.Content, keyNode, valueNode) + } + + return &node, nil +} + +func encodeMarshalYAMLValue(value any) (any, error) { + for { + if value == nil { + return nil, nil + } + if node, ok := value.(*yaml.Node); ok { + return utils.CloneYAMLNode(node), nil + } + + rv := reflect.ValueOf(value) + if rv.Kind() == reflect.Ptr && rv.IsNil() { + return value, nil + } + + m, ok := value.(marshaler) + if !ok { + return value, nil + } + + marshaled, err := m.MarshalYAML() + if err != nil { + return nil, err + } + value = marshaled + } +} + // ToYamlNode converts the ordered map to a yaml node ready for marshalling. func (o *Map[K, V]) ToYamlNode(n NodeBuilder, l any) *yaml.Node { p := utils.CreateEmptyMapNode() diff --git a/orderedmap/orderedmap_test.go b/orderedmap/orderedmap_test.go index 9935c898a..0ad03f5b5 100644 --- a/orderedmap/orderedmap_test.go +++ b/orderedmap/orderedmap_test.go @@ -10,11 +10,27 @@ import ( "time" "github.com/pb33f/libopenapi/datamodel" + "github.com/pb33f/libopenapi/datamodel/low" "github.com/pb33f/libopenapi/orderedmap" "github.com/pb33f/testify/assert" "github.com/pb33f/testify/require" + "go.yaml.in/yaml/v4" ) +type yamlErrorMarshaler struct{} + +func (yamlErrorMarshaler) MarshalYAML() (interface{}, error) { + return nil, errors.New("yaml marshal failed") +} + +type yamlNodeMarshaler struct { + node *yaml.Node +} + +func (y yamlNodeMarshaler) MarshalYAML() (interface{}, error) { + return y.node, nil +} + func TestOrderedMap(t *testing.T) { t.Run("Empty", func(t *testing.T) { m := orderedmap.New[string, int]() @@ -265,6 +281,159 @@ func TestMap(t *testing.T) { }) } +func TestMapMarshalYAMLDoesNotMutateYAMLNodeValues(t *testing.T) { + t.Run("nil map marshals as null", func(t *testing.T) { + var m *orderedmap.Map[string, *yaml.Node] + + node, err := m.MarshalYAML() + require.NoError(t, err) + require.Nil(t, node) + + rendered, err := yaml.Marshal(m) + require.NoError(t, err) + require.Equal(t, "null\n", string(rendered)) + }) + + t.Run("nil value marshals as null", func(t *testing.T) { + m := orderedmap.New[string, any]() + m.Set("x-null", nil) + + rendered, err := yaml.Marshal(m) + require.NoError(t, err) + require.Equal(t, "x-null: null\n", string(rendered)) + }) + + t.Run("nil pointer value marshals as null", func(t *testing.T) { + var value *yamlErrorMarshaler + m := orderedmap.New[string, *yamlErrorMarshaler]() + m.Set("x-null", value) + + rendered, err := yaml.Marshal(m) + require.NoError(t, err) + require.Equal(t, "x-null: null\n", string(rendered)) + }) + + t.Run("key marshaler error", func(t *testing.T) { + m := orderedmap.New[yamlErrorMarshaler, string]() + m.Set(yamlErrorMarshaler{}, "value") + + _, err := m.MarshalYAML() + require.ErrorContains(t, err, "yaml marshal failed") + }) + + t.Run("value marshaler error", func(t *testing.T) { + m := orderedmap.New[string, yamlErrorMarshaler]() + m.Set("x-error", yamlErrorMarshaler{}) + + _, err := m.MarshalYAML() + require.ErrorContains(t, err, "yaml marshal failed") + }) + + t.Run("key encode error", func(t *testing.T) { + m := orderedmap.New[chan int, string]() + m.Set(make(chan int), "value") + + _, err := m.MarshalYAML() + require.Error(t, err) + }) + + t.Run("value encode error", func(t *testing.T) { + m := orderedmap.New[string, chan int]() + m.Set("x-error", make(chan int)) + + _, err := m.MarshalYAML() + require.Error(t, err) + }) + + t.Run("direct scalar node", func(t *testing.T) { + valueNode := &yaml.Node{ + Kind: yaml.ScalarNode, + Tag: "!!bool", + Value: "true", + } + + m := orderedmap.New[string, *yaml.Node]() + m.Set("x-custom", valueNode) + + _, err := m.MarshalYAML() + require.NoError(t, err) + + require.Equal(t, yaml.ScalarNode, valueNode.Kind) + require.Equal(t, "!!bool", valueNode.Tag) + require.Equal(t, "true", valueNode.Value) + require.Equal(t, yaml.Style(0), valueNode.Style) + }) + + t.Run("nested node tree", func(t *testing.T) { + valueNode := &yaml.Node{ + Kind: yaml.MappingNode, + Tag: "!!map", + Content: []*yaml.Node{ + {Kind: yaml.ScalarNode, Tag: "!!str", Value: "enabled"}, + {Kind: yaml.ScalarNode, Tag: "!!bool", Value: "true"}, + {Kind: yaml.ScalarNode, Tag: "!!str", Value: "levels"}, + { + Kind: yaml.SequenceNode, + Tag: "!!seq", + Content: []*yaml.Node{ + {Kind: yaml.ScalarNode, Tag: "!!int", Value: "1"}, + {Kind: yaml.ScalarNode, Tag: "!!int", Value: "2"}, + }, + }, + }, + } + + m := orderedmap.New[string, *yaml.Node]() + m.Set("x-nested", valueNode) + + _, err := yaml.Marshal(m) + require.NoError(t, err) + + require.Equal(t, "!!map", valueNode.Tag) + require.Equal(t, "!!str", valueNode.Content[0].Tag) + require.Equal(t, "!!bool", valueNode.Content[1].Tag) + require.Equal(t, "!!str", valueNode.Content[2].Tag) + require.Equal(t, "!!seq", valueNode.Content[3].Tag) + require.Equal(t, "!!int", valueNode.Content[3].Content[0].Tag) + require.Equal(t, "!!int", valueNode.Content[3].Content[1].Tag) + }) + + t.Run("marshaler key returning node", func(t *testing.T) { + keyNode := &yaml.Node{ + Kind: yaml.ScalarNode, + Tag: "!!str", + Value: "x-custom", + } + m := orderedmap.New[low.KeyReference[string], string]() + m.Set(low.KeyReference[string]{ + Value: "x-custom", + KeyNode: keyNode, + }, "true") + + _, err := yaml.Marshal(m) + require.NoError(t, err) + + require.Equal(t, "!!str", keyNode.Tag) + require.Equal(t, "x-custom", keyNode.Value) + }) + + t.Run("recursive marshaler returning node", func(t *testing.T) { + valueNode := &yaml.Node{ + Kind: yaml.ScalarNode, + Tag: "!!bool", + Value: "true", + } + m := orderedmap.New[string, yamlNodeMarshaler]() + m.Set("x-custom", yamlNodeMarshaler{node: valueNode}) + + _, err := yaml.Marshal(m) + require.NoError(t, err) + + require.Equal(t, "!!bool", valueNode.Tag) + require.Equal(t, "true", valueNode.Value) + }) +} + func TestFirst(t *testing.T) { t.Run("Nil", func(t *testing.T) { pair := orderedmap.First[string, int](nil)