diff --git a/services/preserve_service.go b/services/preserve_service.go index 4e73c33..78222db 100644 --- a/services/preserve_service.go +++ b/services/preserve_service.go @@ -68,8 +68,14 @@ func setYAMLPreserving(data []byte, path string, value interface{}) ([]byte, err if out, ok := tryYAMLScalarSplice(data, parts, value, path); ok { return out, nil } - // Fallback (new keys, nested creation, complex values, multiline scalars): - // node re-encode. This normalizes formatting but always produces valid YAML. + // Surgical insert: a new scalar key added to an existing block mapping → + // splice one line, leaving every other byte (including multi-line flow style + // and blank lines elsewhere) untouched. + if out, ok := tryYAMLInsert(data, parts, value, path); ok { + return out, nil + } + // Fallback (nested creation, complex values, multiline scalars): node + // re-encode. This normalizes formatting but always produces valid YAML. return setYAMLNodeEncode(data, parts, value) } @@ -124,6 +130,167 @@ func tryYAMLScalarSplice(data []byte, parts []string, value interface{}, path st return out, true } +// tryYAMLInsert adds a new value by splicing freshly-built lines into the +// original bytes. It anchors at the deepest existing block-style mapping along +// the path and appends the missing tail (a single leaf key, or a nested block +// when intermediate maps are missing too). Unlike the node re-encode fallback it +// never reformats the rest of the document, so multi-line flow collections, +// blank lines, comments and quoting elsewhere stay byte-for-byte identical. +// +// Returns false (defer to fallback) for anything it cannot do surgically: +// non-scalar values, multi-line scalars, or no existing block-mapping anchor +// (e.g. a flow-style or empty parent, or indexing into a scalar). +func tryYAMLInsert(data []byte, parts []string, value interface{}, path string) ([]byte, bool) { + if !isScalarValue(value) { + return nil, false + } + valText, ok := yamlScalarToken(value) + if !ok { + return nil, false + } + top, ok := yamlTopNode(data) + if !ok { + return nil, false + } + + // Find the deepest existing block mapping whose next path segment is absent; + // the remaining segments (>=1) become the inserted tail. Searching deepest + // first means a present leaf-parent yields a one-line insert. + anchor, tail, indentCol, ok := yamlInsertAnchor(top, parts) + if !ok { + return nil, false + } + + firstKey := anchor.Content[0] + starts := lineStarts(data) + if firstKey.Line < 1 || firstKey.Line > len(starts) { + return nil, false + } + // Walk the mapping's lines to find the last one that belongs to it: blank + // and deeper-indented lines belong; the first line dedented past the key + // column ends the block. We insert right after the last belonging content + // line, so trailing blank lines stay below the new entry. + lastContent := 0 + for ln := firstKey.Line; ln <= len(starts); ln++ { + s := starts[ln-1] + e := len(data) + if ln < len(starts) { + e = starts[ln] + } + lineCol, blank := firstNonSpaceCol(data[s:e]) + if blank { + continue + } + if lineCol < indentCol { + break // dedent: belongs to an ancestor mapping + } + lastContent = ln + } + if lastContent == 0 { + return nil, false + } + + insertOff := len(data) + if lastContent < len(starts) { + insertOff = starts[lastContent] + } + prefix := "" + if insertOff == len(data) && len(data) > 0 && data[len(data)-1] != '\n' { + prefix = "\n" + } + entry := prefix + buildYAMLBlock(tail, indentCol, valText) + out := splice(data, insertOff, insertOff, []byte(entry)) + // Validate: a wrong insertion point would put the value at the wrong path (or + // break parsing); confirm the value now resolves at the requested path. + if !yamlPathHasValue(out, path, value) { + return nil, false + } + return out, true +} + +// yamlInsertAnchor returns the deepest block-style mapping along parts whose +// next segment is missing, the remaining (missing) path segments, and the +// 1-based column at which that mapping's keys sit. ok is false when no such +// anchor exists (so the surgical insert is impossible). +func yamlInsertAnchor(top *yaml.Node, parts []string) (anchor *yaml.Node, tail []string, indentCol int, ok bool) { + for d := len(parts) - 1; d >= 0; d-- { + node, found := findYAMLValueNode(top, parts[:d]) + if !found { + continue + } + if node.Kind != yaml.MappingNode || node.Style == yaml.FlowStyle || len(node.Content) == 0 { + continue + } + if mappingHasKey(node, parts[d]) { + // The next segment already exists; we can't insert it here. A deeper + // anchor was already tried, so this path can't be done surgically. + return nil, nil, 0, false + } + return node, parts[d:], node.Content[0].Column, true + } + return nil, nil, 0, false +} + +func mappingHasKey(node *yaml.Node, key string) bool { + for i := 0; i+1 < len(node.Content); i += 2 { + if node.Content[i].Value == key { + return true + } + } + return false +} + +// buildYAMLBlock renders tail (>=1 map keys) as a block-style YAML fragment +// indented to baseCol (1-based), with valText as the innermost scalar. Nested +// levels add 2 spaces each, matching the encoder's default indent. +func buildYAMLBlock(tail []string, baseCol int, valText string) string { + var b strings.Builder + for i, seg := range tail { + b.WriteString(strings.Repeat(" ", baseCol-1+i*2)) + b.WriteString(seg) + if i == len(tail)-1 { + b.WriteString(": ") + b.WriteString(valText) + } else { + b.WriteString(":") + } + b.WriteString("\n") + } + return b.String() +} + +// yamlScalarToken renders value as a single-line YAML scalar token (with the +// minimal quoting yaml.v3 would choose), or false if it cannot be expressed on +// one line. +func yamlScalarToken(value interface{}) (string, bool) { + b, err := yaml.Marshal(value) + if err != nil { + return "", false + } + s := strings.TrimRight(string(b), "\n") + if strings.ContainsAny(s, "\n\r") { + return "", false // block scalar / multi-line: not a single-line insert + } + return s, true +} + +// firstNonSpaceCol returns the 1-based column of the first non-space character +// on a line, and whether the line is blank (only spaces/tabs, ignoring the +// trailing newline). +func firstNonSpaceCol(line []byte) (int, bool) { + for i := 0; i < len(line); i++ { + switch line[i] { + case ' ', '\t': + continue + case '\n', '\r': + return 0, true + default: + return i + 1, false + } + } + return 0, true +} + func tryYAMLLeafDelete(data []byte, parts []string, path string) ([]byte, bool) { top, ok := yamlTopNode(data) if !ok { diff --git a/services/preserve_service_test.go b/services/preserve_service_test.go index cbad437..ba27c3d 100644 --- a/services/preserve_service_test.go +++ b/services/preserve_service_test.go @@ -94,6 +94,182 @@ func TestSetYAMLPreserving_AddsNewKey(t *testing.T) { } } +// TestSetYAMLPreserving_InsertKeepsMultilineFlowStyle reproduces the original +// regression: inserting a new scalar key must NOT collapse multi-line flow-style +// collections ([ ... ] / { ... } spread across lines) elsewhere in the document. +// The whole file outside the single inserted line stays byte-for-byte identical. +func TestSetYAMLPreserving_InsertKeepsMultilineFlowStyle(t *testing.T) { + in := []byte("deployment:\n" + + " args: [\n" + + " '--config',\n" + + " '/etc/app.yaml',\n" + + " '--verbose'\n" + + " ]\n" + + " annotations: {\n" + + " team: platform,\n" + + " tier: backend\n" + + " }\n" + + "image:\n" + + " repository: myapp\n") + want := "deployment:\n" + + " args: [\n" + + " '--config',\n" + + " '/etc/app.yaml',\n" + + " '--verbose'\n" + + " ]\n" + + " annotations: {\n" + + " team: platform,\n" + + " tier: backend\n" + + " }\n" + + "image:\n" + + " repository: myapp\n" + + " tag: v1.4.0\n" + out, err := setYAMLPreserving(in, ".image.tag", "v1.4.0") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if string(out) != want { + t.Errorf("multi-line flow style not preserved on insert:\ngot:\n%q\nwant:\n%q", out, want) + } +} + +// TestSetYAMLPreserving_InsertByteForByte covers blank lines, comment column +// alignment and quoting around the insertion point. +func TestSetYAMLPreserving_InsertByteForByte(t *testing.T) { + in := []byte("# Chart values\n" + + "image:\n" + + " repository: myapp # registry path\n" + + "\n" + + "replicas: 3\n") + want := "# Chart values\n" + + "image:\n" + + " repository: myapp # registry path\n" + + " tag: v2.0.0\n" + + "\n" + + "replicas: 3\n" + out, err := setYAMLPreserving(in, ".image.tag", "v2.0.0") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if string(out) != want { + t.Errorf("insert not byte-for-byte:\ngot:\n%q\nwant:\n%q", out, want) + } +} + +func TestSetYAMLPreserving_InsertTopLevelKey(t *testing.T) { + in := []byte("a: 1\nb: 2\n") + want := "a: 1\nb: 2\nc: 3\n" + out, err := setYAMLPreserving(in, ".c", float64(3)) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if string(out) != want { + t.Errorf("got %q want %q", out, want) + } +} + +func TestSetYAMLPreserving_InsertNoTrailingNewline(t *testing.T) { + in := []byte("image:\n repository: myapp") + want := "image:\n repository: myapp\n tag: v1\n" + out, err := setYAMLPreserving(in, ".image.tag", "v1") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if string(out) != want { + t.Errorf("got %q want %q", out, want) + } +} + +// A string that YAML would otherwise read as a bool/number must be quoted on +// insert so it round-trips as a string. +func TestSetYAMLPreserving_InsertQuotesAmbiguousString(t *testing.T) { + in := []byte("flags:\n verbose: true\n") + out, err := setYAMLPreserving(in, ".flags.enabled", "yes") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + got := string(out) + if !strings.Contains(got, "enabled: \"yes\"") { + t.Errorf("ambiguous string not quoted on insert: %q", got) + } + if !yamlPathHasValue(out, ".flags.enabled", "yes") { + t.Errorf("inserted value does not round-trip as string: %q", got) + } +} + +// Inserting into a deeper block must not disturb a multi-line flow sibling that +// comes after it in the document. +func TestSetYAMLPreserving_InsertBeforeFlowSibling(t *testing.T) { + in := []byte("image:\n" + + " repository: myapp\n" + + "ports: [\n" + + " 8080,\n" + + " 9090\n" + + "]\n") + want := "image:\n" + + " repository: myapp\n" + + " tag: v1\n" + + "ports: [\n" + + " 8080,\n" + + " 9090\n" + + "]\n" + out, err := setYAMLPreserving(in, ".image.tag", "v1") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if string(out) != want { + t.Errorf("got:\n%q\nwant:\n%q", out, want) + } +} + +// When an intermediate map is missing too, the whole missing tail is created as +// a nested block — and unrelated multi-line flow style is still left untouched. +func TestSetYAMLPreserving_InsertCreatesNestedBlock(t *testing.T) { + in := []byte("service:\n" + + " ports: [\n" + + " 80,\n" + + " 443\n" + + " ]\n") + want := "service:\n" + + " ports: [\n" + + " 80,\n" + + " 443\n" + + " ]\n" + + "image:\n" + + " tag: v1.4.0\n" + out, err := setYAMLPreserving(in, ".image.tag", "v1.4.0") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if string(out) != want { + t.Errorf("nested create not surgical:\ngot:\n%q\nwant:\n%q", out, want) + } +} + +func TestSetYAMLPreserving_InsertCreatesDeepNestedBlock(t *testing.T) { + in := []byte("name: app\n") + want := "name: app\n" + + "a:\n" + + " b:\n" + + " c: 5\n" + out, err := setYAMLPreserving(in, ".a.b.c", float64(5)) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if string(out) != want { + t.Errorf("deep nested create not surgical:\ngot:\n%q\nwant:\n%q", out, want) + } +} + +// Indexing through a scalar is impossible; the surgical path must defer to the +// fallback, which surfaces the error rather than producing wrong output. +func TestSetYAMLPreserving_InsertUnderScalarErrors(t *testing.T) { + in := []byte("image: myapp\n") + if _, err := setYAMLPreserving(in, ".image.tag", "v1"); err == nil { + t.Fatal("expected error inserting under a scalar value") + } +} + func TestSetYAMLPreserving_RejectsIndexIntoScalar(t *testing.T) { in := []byte("tag: v1.0.0\n") if _, err := setYAMLPreserving(in, ".tag.inner", "x"); err == nil {