diff --git a/datamodel/high/v3/document.go b/datamodel/high/v3/document.go index 5bbd231e..3cea0862 100644 --- a/datamodel/high/v3/document.go +++ b/datamodel/high/v3/document.go @@ -173,9 +173,10 @@ func (d *Document) Render() ([]byte, error) { // the rendering will use the original indention of the document. func (d *Document) RenderWithIndention(indent int) []byte { var buf bytes.Buffer - yamlEncoder := yaml.NewEncoder(&buf) - yamlEncoder.SetIndent(indent) - _ = yamlEncoder.Encode(d) + yamlDumper, _ := yaml.NewDumper(&buf, yaml.WithV3Defaults(), yaml.WithLineWidth(-1)) + yamlDumper.SetIndent(indent) + _ = yamlDumper.Dump(d) + _ = yamlDumper.Close() return buf.Bytes() } diff --git a/datamodel/low/extraction_functions_test.go b/datamodel/low/extraction_functions_test.go index 1f2875ab..edfc49df 100644 --- a/datamodel/low/extraction_functions_test.go +++ b/datamodel/low/extraction_functions_test.go @@ -2379,13 +2379,10 @@ func TestLocateRefEnd_Empty(t *testing.T) { } func TestArray_NotRefNotArray(t *testing.T) { - yml := `` - var idxNode yaml.Node - mErr := yaml.Unmarshal([]byte(yml), &idxNode) - assert.NoError(t, mErr) + idxNode := yaml.Node{Kind: yaml.DocumentNode} idx := index.NewSpecIndexWithConfig(&idxNode, index.CreateClosedAPIIndexConfig()) - yml = `limes: + yml := `limes: not: array` var cNode yaml.Node diff --git a/datamodel/spec_info.go b/datamodel/spec_info.go index bfb51f3a..ab947fd0 100644 --- a/datamodel/spec_info.go +++ b/datamodel/spec_info.go @@ -26,14 +26,14 @@ const ( // SpecInfo represents a 'ready-to-process' OpenAPI Document. The RootNode is the most important property // used by the library, this contains the top of the document tree that every single low model is based off. type SpecInfo struct { - SpecType string `json:"type"` - NumLines int `json:"numLines"` - Version string `json:"version"` - VersionNumeric float32 `json:"versionNumeric"` - SpecFormat string `json:"format"` - SpecFileType string `json:"fileType"` - SpecBytes *[]byte `json:"bytes"` // the original byte array - RootNode *yaml.Node `json:"-"` // reference to the root node of the spec. + SpecType string `json:"type"` + NumLines int `json:"numLines"` + Version string `json:"version"` + VersionNumeric float32 `json:"versionNumeric"` + SpecFormat string `json:"format"` + SpecFileType string `json:"fileType"` + SpecBytes *[]byte `json:"bytes"` // the original byte array + RootNode *yaml.Node `json:"-"` // reference to the root node of the spec. // SpecJSONBytes is the original document converted to JSON. It is populated lazily. // @@ -231,6 +231,9 @@ func extractSpecInfoInternal(spec []byte, bypass bool, skipJSON bool) (*SpecInfo if err := parsedNode.Decode(&jsonSpec); err != nil { return fmt.Errorf("failed to decode YAML to JSON: %w", err) } + if jsonSpec == nil { + return fmt.Errorf("failed to decode YAML to JSON: YAML document root is %v, not a mapping", root.Kind) + } } if err := checkDuplicateMappingKeys(parsedNode); err != nil { return fmt.Errorf("failed to decode YAML to JSON: %w", err) @@ -387,10 +390,8 @@ func extractSpecInfoInternal(spec []byte, bypass bool, skipJSON bool) (*SpecInfo // parseJSON(spec, specInfo, &parsedSpec) //} - if !parsed && !skipJSON { - if err := parseJSON(spec, specInfo, &parsedSpec); err != nil && !bypass { - return nil, err - } + if !parsed && !skipJSON && bypass { + _ = parseJSON(spec, specInfo, &parsedSpec) } // detect the original whitespace indentation @@ -411,9 +412,9 @@ func ExtractSpecInfo(spec []byte) (*SpecInfo, error) { // checkDuplicateMappingKeys walks a parsed node tree and reports duplicate mapping // keys using the exact equality semantics of the go.yaml.in/yaml/v4 decoder: two keys // in the same mapping collide when their node Kind and raw Value match (no tag or -// alias resolution). Error text matches the decoder's construct errors byte for byte, -// and children of an offending mapping are not descended into, mirroring the decoder -// halting construction of that mapping. +// alias resolution). The collected construct errors are normalized onto the +// public one-line error shape, and children of an offending mapping are not +// descended into, mirroring the decoder halting construction of that mapping. // // Known divergence: an anchored mapping with duplicate keys that is aliased // elsewhere is reported ONCE here, while the decoder re-reports it on every @@ -428,13 +429,7 @@ func checkDuplicateMappingKeys(node *yaml.Node) error { if len(errs) == 0 { return nil } - var b strings.Builder - b.WriteString("yaml: construct errors:") - for _, e := range errs { - b.WriteString("\n ") - b.WriteString(e) - } - return errors.New(b.String()) + return errors.New("yaml: construct errors: " + strings.Join(errs, "; ")) } func walkDuplicateMappingKeys(node *yaml.Node, errs *[]string) { diff --git a/datamodel/spec_info_test.go b/datamodel/spec_info_test.go index 56ec42a5..7decb119 100644 --- a/datamodel/spec_info_test.go +++ b/datamodel/spec_info_test.go @@ -228,6 +228,14 @@ func TestExtractSpecInfo_InvalidYAML(t *testing.T) { assert.Error(t, e) } +func TestExtractSpecInfo_BareMergeRootReturnsDecodeError(t *testing.T) { + _, err := ExtractSpecInfo([]byte("<<\n")) + + assert.Error(t, err) + assert.Contains(t, err.Error(), "failed to decode YAML to JSON") + assert.Contains(t, err.Error(), "YAML document root is") +} + // TestExtractSpecInfo_InvalidYAML_DuplicateKey tests issue #355 // Malformed YAML with duplicate keys should return an error when bypass=false func TestExtractSpecInfo_InvalidYAML_DuplicateKey(t *testing.T) { diff --git a/generator/golang/metadata.go b/generator/golang/metadata.go index 3c8274bc..4c376e3f 100644 --- a/generator/golang/metadata.go +++ b/generator/golang/metadata.go @@ -226,12 +226,70 @@ func (g *Generator) applyOpenAPIMetadata(ir *SchemaIR, meta openAPIMetadata) { schema.MaxProperties = &meta.MaxProperties } if len(meta.Enum) > 0 { - ir.Enum = meta.Enum - schema.Enum = meta.Enum + if len(schema.Enum) > 0 && equivalentMetadataYAMLNodeSlices(schema.Enum, meta.Enum) { + ir.Enum = schema.Enum + } else { + ir.Enum = meta.Enum + schema.Enum = meta.Enum + } } if meta.Const != nil { - ir.Const = meta.Const - schema.Const = meta.Const + if schema.Const != nil && equivalentMetadataYAMLNodes(schema.Const, meta.Const) { + ir.Const = schema.Const + } else { + ir.Const = meta.Const + schema.Const = meta.Const + } + } +} + +func equivalentMetadataYAMLNodeSlices(left, right []*yaml.Node) bool { + if len(left) != len(right) { + return false + } + for i := range left { + if !equivalentMetadataYAMLNodes(left[i], right[i]) { + return false + } + } + return true +} + +func equivalentMetadataYAMLNodes(left, right *yaml.Node) bool { + if left == nil || right == nil { + return left == right + } + if left.Kind != right.Kind || + normalizeMetadataYAMLTag(left.Kind, left.Tag) != normalizeMetadataYAMLTag(right.Kind, right.Tag) || + left.Value != right.Value || + left.Anchor != right.Anchor || + len(left.Content) != len(right.Content) { + return false + } + if !equivalentMetadataYAMLNodes(left.Alias, right.Alias) { + return false + } + for i := range left.Content { + if !equivalentMetadataYAMLNodes(left.Content[i], right.Content[i]) { + return false + } + } + return true +} + +func normalizeMetadataYAMLTag(kind yaml.Kind, tag string) string { + if tag != "" { + return tag + } + switch kind { + case yaml.SequenceNode: + return "!!seq" + case yaml.MappingNode: + return "!!map" + case yaml.ScalarNode: + return "!!str" + default: + return tag } } diff --git a/generator/golang/metadata_test.go b/generator/golang/metadata_test.go index c17fc11b..7a275981 100644 --- a/generator/golang/metadata_test.go +++ b/generator/golang/metadata_test.go @@ -263,6 +263,39 @@ func TestGeneratedQuotedOpenAPITag(t *testing.T) { } } +func TestApplyOpenAPIMetadataKeepsEquivalentSourceYAMLNodes(t *testing.T) { + enum := []*yaml.Node{{Kind: yaml.ScalarNode, Tag: "!!str", Value: "bam"}} + constValue := &yaml.Node{Kind: yaml.ScalarNode, Tag: "!!str", Value: "card"} + schema := &highbase.Schema{Enum: enum, Const: constValue} + ir := &SchemaIR{Enum: enum, Const: constValue, SourceSchema: schema} + + NewGenerator().applyOpenAPIMetadata(ir, openAPIMetadata{ + Present: true, + Enum: []*yaml.Node{{Kind: yaml.ScalarNode, Value: "bam"}}, + Const: &yaml.Node{Kind: yaml.ScalarNode, Value: "card"}, + }) + + if ir.Enum[0].Tag != "!!str" || schema.Enum[0].Tag != "!!str" { + t.Fatalf("equivalent enum metadata stripped source tag: %#v", ir.Enum[0]) + } + if ir.Const.Tag != "!!str" || schema.Const.Tag != "!!str" { + t.Fatalf("equivalent const metadata stripped source tag: %#v", ir.Const) + } + + NewGenerator().applyOpenAPIMetadata(ir, openAPIMetadata{ + Present: true, + Enum: []*yaml.Node{{Kind: yaml.ScalarNode, Tag: "!!str", Value: "bgn"}}, + Const: &yaml.Node{Kind: yaml.ScalarNode, Tag: "!!str", Value: "bank_account"}, + }) + + if ir.Enum[0].Value != "bgn" || schema.Enum[0].Value != "bgn" { + t.Fatalf("changed enum metadata was not applied: %#v", ir.Enum[0]) + } + if ir.Const.Value != "bank_account" || schema.Const.Value != "bank_account" { + t.Fatalf("changed const metadata was not applied: %#v", ir.Const) + } +} + func TestFieldSchemaOverridesAndSchemaYAMLProvider(t *testing.T) { sourceSchema := schemaProxyFromYAML(t, ` oneOf: @@ -1005,6 +1038,54 @@ func TestMetadataHelpersCoverage(t *testing.T) { if got := metadataYAMLNodeLiteral(nil, 0); got != "nil" { t.Fatalf("nil yaml node literal mismatch: %q", got) } + if got := metadataYAMLNodeLiteral(&yaml.Node{Kind: yaml.ScalarNode, Value: "1"}, 0); !strings.Contains(got, `Tag: "!!int"`) { + t.Fatalf("empty integer scalar tag should infer numeric metadata literal: %s", got) + } + if got := metadataYAMLNodeLiteral(&yaml.Node{Kind: yaml.ScalarNode, Value: "true"}, 0); !strings.Contains(got, `Tag: "!!bool"`) { + t.Fatalf("empty boolean scalar tag should infer boolean metadata literal: %s", got) + } + if got := metadataYAMLNodeLiteral(&yaml.Node{Kind: yaml.ScalarNode, Style: yaml.SingleQuotedStyle, Value: "1"}, 0); !strings.Contains(got, `Tag: "!!str"`) { + t.Fatalf("styled scalar tag should stay string in metadata literal: %s", got) + } + if got := metadataYAMLNodeLiteral(&yaml.Node{Kind: yaml.MappingNode}, 0); !strings.Contains(got, `Tag: "!!map"`) { + t.Fatalf("empty mapping tag should normalize to default in metadata literal: %s", got) + } + if got := metadataYAMLNodeLiteral(&yaml.Node{Kind: yaml.SequenceNode}, 0); !strings.Contains(got, `Tag: "!!seq"`) { + t.Fatalf("empty sequence tag should normalize to default in metadata literal: %s", got) + } + if got := metadataYAMLNodeLiteralTag(nil); got != "" { + t.Fatalf("nil yaml node tag should be empty: %q", got) + } + if got := inferMetadataYAMLScalarTag(&yaml.Node{Kind: yaml.ScalarNode, Value: "["}); got != "!!str" { + t.Fatalf("invalid scalar should fall back to string tag: %q", got) + } + if got := inferMetadataYAMLScalarTag(&yaml.Node{Kind: yaml.ScalarNode, Value: "[]"}); got != "!!str" { + t.Fatalf("non-scalar parsed value should fall back to string tag: %q", got) + } + if equivalentMetadataYAMLNodeSlices([]*yaml.Node{stringNode("a")}, nil) { + t.Fatal("metadata yaml node slices with different lengths should not match") + } + if equivalentMetadataYAMLNodes( + &yaml.Node{Kind: yaml.AliasNode, Alias: stringNode("a")}, + &yaml.Node{Kind: yaml.AliasNode, Alias: stringNode("b")}, + ) { + t.Fatal("metadata yaml alias nodes with different targets should not match") + } + if equivalentMetadataYAMLNodes( + &yaml.Node{Kind: yaml.SequenceNode, Content: []*yaml.Node{stringNode("a")}}, + &yaml.Node{Kind: yaml.SequenceNode, Content: []*yaml.Node{stringNode("b")}}, + ) { + t.Fatal("metadata yaml nodes with different children should not match") + } + if got := normalizeMetadataYAMLTag(yaml.SequenceNode, ""); got != "!!seq" { + t.Fatalf("empty sequence tag should normalize to !!seq: %q", got) + } + if got := normalizeMetadataYAMLTag(yaml.MappingNode, ""); got != "!!map" { + t.Fatalf("empty mapping tag should normalize to !!map: %q", got) + } + if got := normalizeMetadataYAMLTag(yaml.DocumentNode, ""); got != "" { + t.Fatalf("unknown empty yaml tag should stay empty: %q", got) + } } func schemaProxyFromRefDocumentYAML(t *testing.T, sampleYAML string) *highbase.SchemaProxy { diff --git a/generator/golang/provider_methods.go b/generator/golang/provider_methods.go index 21648eeb..044a4c14 100644 --- a/generator/golang/provider_methods.go +++ b/generator/golang/provider_methods.go @@ -480,7 +480,7 @@ func metadataYAMLNodeLiteral(node *yaml.Node, depth int) string { b.WriteString("&openAPIYAMLNode{\n") writeMetadataField(&b, depth+1, "Kind", strconv.Quote(metadataYAMLKind(node.Kind))) writeMetadataField(&b, depth+1, "Style", metadataPlainIntLiteral(int64(node.Style))) - writeMetadataField(&b, depth+1, "Tag", metadataStringLiteral(node.Tag)) + writeMetadataField(&b, depth+1, "Tag", metadataStringLiteral(metadataYAMLNodeLiteralTag(node))) writeMetadataField(&b, depth+1, "Value", metadataStringLiteral(node.Value)) writeMetadataField(&b, depth+1, "Anchor", metadataStringLiteral(node.Anchor)) writeMetadataField(&b, depth+1, "Content", metadataYAMLNodeContentLiteral(node.Content, depth+1)) @@ -490,6 +490,40 @@ func metadataYAMLNodeLiteral(node *yaml.Node, depth int) string { return b.String() } +func metadataYAMLNodeLiteralTag(node *yaml.Node) string { + if node == nil { + return "" + } + if node.Tag != "" { + return node.Tag + } + switch node.Kind { + case yaml.SequenceNode: + return "!!seq" + case yaml.MappingNode: + return "!!map" + case yaml.ScalarNode: + return inferMetadataYAMLScalarTag(node) + default: + return node.Tag + } +} + +func inferMetadataYAMLScalarTag(node *yaml.Node) string { + if node.Style&(yaml.SingleQuotedStyle|yaml.DoubleQuotedStyle|yaml.LiteralStyle|yaml.FoldedStyle) != 0 { + return "!!str" + } + var parsed yaml.Node + if err := yaml.Unmarshal([]byte(node.Value+"\n"), &parsed); err != nil || len(parsed.Content) == 0 { + return "!!str" + } + scalar := parsed.Content[0] + if scalar.Kind != yaml.ScalarNode || scalar.Tag == "" { + return "!!str" + } + return scalar.Tag +} + func optionalMetadataYAMLNodeLiteral(node *yaml.Node, depth int) string { if node == nil { return "" diff --git a/go.mod b/go.mod index d91e81fc..255b225a 100644 --- a/go.mod +++ b/go.mod @@ -1,13 +1,13 @@ module github.com/pb33f/libopenapi -go 1.25.0 +go 1.25.7 require ( github.com/lucasjones/reggen v0.0.0-20200904144131-37ba4fa293bb github.com/pb33f/jsonpath v0.8.2 github.com/pb33f/ordered-map/v2 v2.3.1 github.com/stretchr/testify v1.11.1 - go.yaml.in/yaml/v4 v4.0.0-rc.4 + go.yaml.in/yaml/v4 v4.0.0-rc.5 golang.org/x/sync v0.21.0 ) diff --git a/go.sum b/go.sum index d0e598a0..a3bebefd 100644 --- a/go.sum +++ b/go.sum @@ -21,8 +21,8 @@ github.com/rogpeppe/go-internal v1.9.0 h1:73kH8U+JUqXU8lRuOHeVHaa/SZPifC7BkcraZV github.com/rogpeppe/go-internal v1.9.0/go.mod h1:WtVeX8xhTBvf0smdhujwtBcq4Qrzq/fJaraNFVN+nFs= github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= -go.yaml.in/yaml/v4 v4.0.0-rc.4 h1:UP4+v6fFrBIb1l934bDl//mmnoIZEDK0idg1+AIvX5U= -go.yaml.in/yaml/v4 v4.0.0-rc.4/go.mod h1:aZqd9kCMsGL7AuUv/m/PvWLdg5sjJsZ4oHDEnfPPfY0= +go.yaml.in/yaml/v4 v4.0.0-rc.5 h1:JVliQq9EGOYaTgMi+k8BhUJyqcGk4ZqeuiN1Cirba9c= +go.yaml.in/yaml/v4 v4.0.0-rc.5/go.mod h1:aZqd9kCMsGL7AuUv/m/PvWLdg5sjJsZ4oHDEnfPPfY0= golang.org/x/sync v0.21.0 h1:HLII4xRRTtCRkxYp4HNFF0Js/Og6q2i++KXbg0gHCwM= golang.org/x/sync v0.21.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= diff --git a/overlay.go b/overlay.go index 2f475f0b..ed37aff7 100644 --- a/overlay.go +++ b/overlay.go @@ -4,6 +4,7 @@ package libopenapi import ( + "bytes" gocontext "context" "github.com/pb33f/libopenapi/datamodel" @@ -32,6 +33,10 @@ type OverlayResult struct { // NewOverlayDocument creates a new overlay document from the provided bytes. // The overlay document can then be applied to a target OpenAPI document using ApplyOverlay. func NewOverlayDocument(overlayBytes []byte) (*highoverlay.Overlay, error) { + if len(bytes.TrimSpace(overlayBytes)) == 0 { + return nil, overlay.ErrInvalidOverlay + } + var node yaml.Node if err := yaml.Unmarshal(overlayBytes, &node); err != nil { return nil, err diff --git a/overlay_test.go b/overlay_test.go index 06667f0a..a52587b4 100644 --- a/overlay_test.go +++ b/overlay_test.go @@ -57,6 +57,18 @@ func TestNewOverlayDocument_InvalidOverlay(t *testing.T) { assert.NotNil(t, ov) } +func TestNewOverlayDocument_InvalidInfoObject(t *testing.T) { + overlayYAML := `overlay: 1.0.0 +info: + $ref: "" +actions: []` + + ov, err := NewOverlayDocument([]byte(overlayYAML)) + + assert.Error(t, err) + assert.Nil(t, ov) +} + func TestNewOverlayDocument_SequenceRoot(t *testing.T) { // Sequence at root - BuildModel is lenient and returns empty overlay overlayYAML := `- item1 diff --git a/what-changed/model/breaking_rules_config.go b/what-changed/model/breaking_rules_config.go index b1883471..e673d418 100644 --- a/what-changed/model/breaking_rules_config.go +++ b/what-changed/model/breaking_rules_config.go @@ -246,6 +246,10 @@ func buildValidComponentSet() map[string]bool { // just "discriminator" at the top level) and returns all validation errors found. // Returns nil if the configuration is valid. func ValidateBreakingRulesConfigYAML(yamlBytes []byte) *ConfigValidationResult { + if len(strings.TrimSpace(string(yamlBytes))) == 0 { + return nil + } + var rootNode yaml.Node if err := yaml.Unmarshal(yamlBytes, &rootNode); err != nil { return &ConfigValidationResult{ diff --git a/what-changed/model/breaking_rules_config_test.go b/what-changed/model/breaking_rules_config_test.go index 9b6a311a..24b608a0 100644 --- a/what-changed/model/breaking_rules_config_test.go +++ b/what-changed/model/breaking_rules_config_test.go @@ -188,6 +188,12 @@ func TestValidateBreakingRulesConfigYAML_EmptyConfig(t *testing.T) { assert.Nil(t, result, "Empty config should be valid") } +func TestValidateBreakingRulesConfigYAML_NonMappingRoot(t *testing.T) { + yaml := `- schema` + result := ValidateBreakingRulesConfigYAML([]byte(yaml)) + assert.Nil(t, result, "Non-mapping config roots should be ignored") +} + func TestValidateBreakingRulesConfigYAML_OnlyValidTopLevel(t *testing.T) { yaml := ` openapi: diff --git a/what-changed/model/comparison_functions.go b/what-changed/model/comparison_functions.go index 824e1384..30361154 100644 --- a/what-changed/model/comparison_functions.go +++ b/what-changed/model/comparison_functions.go @@ -4,6 +4,7 @@ package model import ( + "bytes" "fmt" "reflect" "strings" @@ -412,10 +413,9 @@ func checkForAdditionInternal[T any](l, r *yaml.Node, label string, changes *[]* if withEncoding { createFn = CreateChangeWithEncoding } - // left doesn't exist if: nil OR (empty scalar AND not a map/array) OR (empty map/array) + // left doesn't exist if: nil OR an empty scalar. Empty maps and arrays are real values. leftDoesNotExist := l == nil || - (l.Value == EMPTY_STR && !utils.IsNodeMap(l) && !utils.IsNodeArray(l)) || - ((utils.IsNodeMap(l) || utils.IsNodeArray(l)) && len(l.Content) == 0) + (l.Value == EMPTY_STR && !utils.IsNodeMap(l) && !utils.IsNodeArray(l)) // right exists if: not nil AND (has value OR is array OR is map) rightExists := r != nil && (r.Value != EMPTY_STR || utils.IsNodeArray(r) || utils.IsNodeMap(r)) @@ -441,7 +441,7 @@ func checkForModificationInternal[T any](l, r *yaml.Node, label string, changes createFn = CreateChangeWithEncoding } if l != nil && l.Value != EMPTY_STR && r != nil && r.Value != EMPTY_STR { - if !low.CompareYAMLNodes(l, r) { + if !compareYAMLNodesForChanges(l, r) { createFn(changes, Modified, label, l, r, breaking, orig, new) } return @@ -469,20 +469,60 @@ func checkForModificationInternal[T any](l, r *yaml.Node, label string, changes } // Compare the YAML node trees directly without marshaling - if !low.CompareYAMLNodes(l, r) { + if !compareYAMLNodesForChanges(l, r) { createFn(changes, Modified, label, l, r, breaking, orig, new) } return } if l != nil && utils.IsNodeMap(l) && r != nil && utils.IsNodeMap(r) { // Compare the YAML node trees directly without marshaling - if !low.CompareYAMLNodes(l, r) { + if !compareYAMLNodesForChanges(l, r) { createFn(changes, Modified, label, l, r, breaking, orig, new) } return } } +func compareYAMLNodesForChanges(left, right *yaml.Node) bool { + if low.CompareYAMLNodes(left, right) { + return true + } + leftClone := cloneYAMLNodeWithoutAnchors(left, nil) + rightClone := cloneYAMLNodeWithoutAnchors(right, nil) + if low.CompareYAMLNodes(leftClone, rightClone) { + return true + } + + leftBytes, leftErr := yaml.Marshal(leftClone) + rightBytes, rightErr := yaml.Marshal(rightClone) + return leftErr == nil && rightErr == nil && bytes.Equal(leftBytes, rightBytes) +} + +func cloneYAMLNodeWithoutAnchors(node *yaml.Node, seen map[*yaml.Node]*yaml.Node) *yaml.Node { + if node == nil { + return nil + } + if seen == nil { + seen = make(map[*yaml.Node]*yaml.Node) + } + if clone, ok := seen[node]; ok { + return clone + } + + clone := *node + clone.Anchor = "" + seen[node] = &clone + + if len(node.Content) > 0 { + clone.Content = make([]*yaml.Node, len(node.Content)) + for i, child := range node.Content { + clone.Content[i] = cloneYAMLNodeWithoutAnchors(child, seen) + } + } + clone.Alias = cloneYAMLNodeWithoutAnchors(node.Alias, seen) + return &clone +} + // CheckForModification will check left and right yaml.Node instances for changes. Anything that is found in both // sides, but vary in value is considered a modification. // diff --git a/what-changed/model/comparison_functions_test.go b/what-changed/model/comparison_functions_test.go index d1ec5a80..28589da4 100644 --- a/what-changed/model/comparison_functions_test.go +++ b/what-changed/model/comparison_functions_test.go @@ -159,7 +159,7 @@ func Test_CheckMapsAddition(t *testing.T) { {"Same map", mapA, mapB, false, false}, {"Different map", mapA, mapB, false, false}, {"Add map", nil, mapA, true, false}, - {"Add map value", nil, mapB, true, true}, + {"Empty map exists", nil, mapB, false, true}, {"Remove map", mapA, nil, false, false}, } @@ -611,6 +611,28 @@ func TestCheckForAdditionWithEncoding(t *testing.T) { assert.Equal(t, PropertyAdded, changes[0].ChangeType) assert.NotEmpty(t, changes[0].NewEncoded) }) + + t.Run("does not add empty array already present", func(t *testing.T) { + var lNode, rNode yaml.Node + _ = yaml.Unmarshal([]byte(`[]`), &lNode) + _ = yaml.Unmarshal([]byte(`[]`), &rNode) + + changes := []*Change{} + CheckForAdditionWithEncoding[string](lNode.Content[0], rNode.Content[0], "test", &changes, false, "", "new") + + assert.Empty(t, changes) + }) + + t.Run("does not add empty map already present", func(t *testing.T) { + var lNode, rNode yaml.Node + _ = yaml.Unmarshal([]byte(`{}`), &lNode) + _ = yaml.Unmarshal([]byte(`{}`), &rNode) + + changes := []*Change{} + CheckForAdditionWithEncoding[string](lNode.Content[0], rNode.Content[0], "test", &changes, false, "", "new") + + assert.Empty(t, changes) + }) } // TestCheckForModificationWithEncoding tests the modification check with encoding @@ -664,6 +686,26 @@ func TestCheckForModificationWithEncoding_NumericScalarEquivalence(t *testing.T) assert.Empty(t, changes) } +func TestCheckForModificationWithEncoding_IgnoresAnchorOnlyDifference(t *testing.T) { + left := &yaml.Node{Kind: yaml.ScalarNode, Tag: "!!str", Value: "same", Anchor: "leftAnchor"} + right := &yaml.Node{Kind: yaml.ScalarNode, Tag: "!!str", Value: "same", Anchor: "rightAnchor"} + + changes := []*Change{} + CheckForModificationWithEncoding(left, right, "anchored", &changes, false, "old", "new") + + assert.Empty(t, changes) +} + +func TestCloneYAMLNodeWithoutAnchors_ReusesSeenNode(t *testing.T) { + target := &yaml.Node{Kind: yaml.ScalarNode, Tag: "!!str", Value: "same", Anchor: "target"} + node := &yaml.Node{Kind: yaml.SequenceNode, Tag: "!!seq", Content: []*yaml.Node{target, target}} + + clone := cloneYAMLNodeWithoutAnchors(node, nil) + + assert.Empty(t, clone.Content[0].Anchor) + assert.Same(t, clone.Content[0], clone.Content[1]) +} + // TestCheckMapForChangesWithComp tests the deprecated CheckMapForChangesWithComp function func TestCheckMapForChangesWithComp(t *testing.T) { t.Run("detects removal", func(t *testing.T) { diff --git a/what-changed/model/example.go b/what-changed/model/example.go index 8e6ea730..35a8cf22 100644 --- a/what-changed/model/example.go +++ b/what-changed/model/example.go @@ -84,61 +84,63 @@ func CompareExamples(l, r *base.Example) *ExampleChanges { // Value if utils.IsNodeMap(l.Value.ValueNode) && utils.IsNodeMap(r.Value.ValueNode) { - lKeys := make([]string, len(l.Value.ValueNode.Content)/2) - rKeys := make([]string, len(r.Value.ValueNode.Content)/2) - z := 0 - for k := range l.Value.ValueNode.Content { - if k%2 == 0 { - // if there is no value (value is another map or something else), render the node into yaml and hash it. - // https://github.com/pb33f/libopenapi/issues/61 - val := l.Value.ValueNode.Content[k+1].Value - if val == "" { - val = low.HashYAMLNodeSlice(l.Value.ValueNode.Content[k+1].Content) + if !compareYAMLNodesForChanges(l.Value.ValueNode, r.Value.ValueNode) { + lKeys := make([]string, len(l.Value.ValueNode.Content)/2) + rKeys := make([]string, len(r.Value.ValueNode.Content)/2) + z := 0 + for k := range l.Value.ValueNode.Content { + if k%2 == 0 { + // if there is no value (value is another map or something else), render the node into yaml and hash it. + // https://github.com/pb33f/libopenapi/issues/61 + val := l.Value.ValueNode.Content[k+1].Value + if val == "" { + val = low.HashYAMLNodeSlice(l.Value.ValueNode.Content[k+1].Content) + } + lKeys[z] = fmt.Sprintf("%v-%v-%v", + l.Value.ValueNode.Content[k].Value, + l.Value.ValueNode.Content[k+1].Tag, + fmt.Sprintf("%x", val)) + z++ + } else { + continue } - lKeys[z] = fmt.Sprintf("%v-%v-%v", - l.Value.ValueNode.Content[k].Value, - l.Value.ValueNode.Content[k+1].Tag, - fmt.Sprintf("%x", val)) - z++ - } else { - continue } - } - z = 0 - for k := range r.Value.ValueNode.Content { - if k%2 == 0 { - // if there is no value (value is another map or something else), render the node into yaml and hash it. - // https://github.com/pb33f/libopenapi/issues/61 - val := r.Value.ValueNode.Content[k+1].Value - if val == "" { - val = low.HashYAMLNodeSlice(r.Value.ValueNode.Content[k+1].Content) + z = 0 + for k := range r.Value.ValueNode.Content { + if k%2 == 0 { + // if there is no value (value is another map or something else), render the node into yaml and hash it. + // https://github.com/pb33f/libopenapi/issues/61 + val := r.Value.ValueNode.Content[k+1].Value + if val == "" { + val = low.HashYAMLNodeSlice(r.Value.ValueNode.Content[k+1].Content) + } + rKeys[z] = fmt.Sprintf("%v-%v-%v", + r.Value.ValueNode.Content[k].Value, + r.Value.ValueNode.Content[k+1].Tag, + fmt.Sprintf("%x", val)) + z++ + } else { + continue } - rKeys[z] = fmt.Sprintf("%v-%v-%v", - r.Value.ValueNode.Content[k].Value, - r.Value.ValueNode.Content[k+1].Tag, - fmt.Sprintf("%x", val)) - z++ - } else { - continue - } - } - sort.Strings(lKeys) - sort.Strings(rKeys) - for k := range lKeys { - if k < len(rKeys) && lKeys[k] != rKeys[k] { - CreateChangeWithEncoding(&changes, Modified, v3.ValueLabel, - l.Value.GetValueNode(), r.Value.GetValueNode(), BreakingModified(CompExample, PropValue), l.Value.GetValue(), r.Value.GetValue()) - continue } - if k >= len(rKeys) { - CreateChangeWithEncoding(&changes, PropertyRemoved, v3.ValueLabel, - l.Value.ValueNode, r.Value.ValueNode, BreakingRemoved(CompExample, PropValue), l.Value.Value, r.Value.Value) + sort.Strings(lKeys) + sort.Strings(rKeys) + for k := range lKeys { + if k < len(rKeys) && lKeys[k] != rKeys[k] { + CreateChangeWithEncoding(&changes, Modified, v3.ValueLabel, + l.Value.GetValueNode(), r.Value.GetValueNode(), BreakingModified(CompExample, PropValue), l.Value.GetValue(), r.Value.GetValue()) + continue + } + if k >= len(rKeys) { + CreateChangeWithEncoding(&changes, PropertyRemoved, v3.ValueLabel, + l.Value.ValueNode, r.Value.ValueNode, BreakingRemoved(CompExample, PropValue), l.Value.Value, r.Value.Value) + } } - } - for k := range rKeys { - if k >= len(lKeys) { - CreateChangeWithEncoding(&changes, PropertyAdded, v3.ValueLabel, - l.Value.ValueNode, r.Value.ValueNode, BreakingAdded(CompExample, PropValue), l.Value.Value, r.Value.Value) + for k := range rKeys { + if k >= len(lKeys) { + CreateChangeWithEncoding(&changes, PropertyAdded, v3.ValueLabel, + l.Value.ValueNode, r.Value.ValueNode, BreakingAdded(CompExample, PropValue), l.Value.Value, r.Value.Value) + } } } } else {