From 29ded9d537b0fc2f9f62b66859cbb607726741fc Mon Sep 17 00:00:00 2001 From: Jaime Soriano Pastor Date: Thu, 5 Oct 2023 09:04:57 +0200 Subject: [PATCH] Automatically format nested objects in YAML files (#1485) Package Spec v3 doesn't allow to include names with dots in YAMLs, these cases need to be migrated to nested objects. Automate this migration step for this format version. --- docs/howto/update_major_package_spec.md | 6 +- internal/builder/dynamic_mappings.go | 3 +- internal/formatter/formatter.go | 2 +- internal/formatter/yaml_formatter.go | 93 ++++++++++++++++- internal/formatter/yaml_formatter_test.go | 118 ++++++++++++++++++++++ internal/packages/changelog/yaml.go | 3 +- 6 files changed, 217 insertions(+), 8 deletions(-) create mode 100644 internal/formatter/yaml_formatter_test.go diff --git a/docs/howto/update_major_package_spec.md b/docs/howto/update_major_package_spec.md index e69ff7e40..039cfb9ce 100644 --- a/docs/howto/update_major_package_spec.md +++ b/docs/howto/update_major_package_spec.md @@ -42,8 +42,10 @@ setting was included with and withoud dotted notation. This is commonly found in `conditions` or in `elasticsearch` settings. -To solve this, please use nested dotations. So if for example your package has -something like the following: +`elastic-package` `check` and `format` subcommands will try to fix this +automatically. If you are still finding this issue, you will need to fix it +manually. For that, please use nested dotations. So if for example your package +has something like the following: ``` conditions: elastic.subscription: basic diff --git a/internal/builder/dynamic_mappings.go b/internal/builder/dynamic_mappings.go index 7f0c458f8..da0026e50 100644 --- a/internal/builder/dynamic_mappings.go +++ b/internal/builder/dynamic_mappings.go @@ -240,7 +240,8 @@ func formatResult(result interface{}) ([]byte, error) { if err != nil { return nil, errors.New("failed to encode") } - d, _, err = formatter.YAMLFormatter(d) + yamlFormatter := &formatter.YAMLFormatter{} + d, _, err = yamlFormatter.Format(d) if err != nil { return nil, errors.New("failed to format") } diff --git a/internal/formatter/formatter.go b/internal/formatter/formatter.go index eda77e11b..ed16e8658 100644 --- a/internal/formatter/formatter.go +++ b/internal/formatter/formatter.go @@ -21,7 +21,7 @@ func newFormatter(specVersion semver.Version, ext string) formatter { case ".json": return JSONFormatterBuilder(specVersion).Format case ".yaml", ".yml": - return YAMLFormatter + return NewYAMLFormatter(specVersion).Format default: return nil } diff --git a/internal/formatter/yaml_formatter.go b/internal/formatter/yaml_formatter.go index 7fbcca936..28e3d8ab2 100644 --- a/internal/formatter/yaml_formatter.go +++ b/internal/formatter/yaml_formatter.go @@ -7,13 +7,24 @@ package formatter import ( "bytes" "fmt" + "strings" + "github.com/Masterminds/semver/v3" "gopkg.in/yaml.v3" ) -// YAMLFormatter function is responsible for formatting the given YAML input. -// The function is exposed, so it can be used by other internal packages. -func YAMLFormatter(content []byte) ([]byte, bool, error) { +// YAMLFormatter is responsible for formatting the given YAML input. +type YAMLFormatter struct { + specVersion semver.Version +} + +func NewYAMLFormatter(specVersion semver.Version) *YAMLFormatter { + return &YAMLFormatter{ + specVersion: specVersion, + } +} + +func (f *YAMLFormatter) Format(content []byte) ([]byte, bool, error) { // yaml.Unmarshal() requires `yaml.Node` to be passed instead of generic `interface{}`. // Otherwise it can't detect any comments and fields are considered as normal map. var node yaml.Node @@ -22,6 +33,10 @@ func YAMLFormatter(content []byte) ([]byte, bool, error) { return nil, false, fmt.Errorf("unmarshalling YAML file failed: %w", err) } + if !f.specVersion.LessThan(semver.MustParse("3.0.0")) { + extendNestedObjects(&node) + } + var b bytes.Buffer encoder := yaml.NewEncoder(&b) encoder.SetIndent(2) @@ -39,3 +54,75 @@ func YAMLFormatter(content []byte) ([]byte, bool, error) { return formatted, string(content) == string(formatted), nil } + +func extendNestedObjects(node *yaml.Node) { + if node.Kind == yaml.MappingNode { + extendMapNode(node) + } + for _, child := range node.Content { + extendNestedObjects(child) + } +} + +func extendMapNode(node *yaml.Node) { + for i := 0; i < len(node.Content); i += 2 { + key := node.Content[i] + value := node.Content[i+1] + + base, rest, found := strings.Cut(key.Value, ".") + + // Insert nested objects only when the key has a dot, and is not quoted. + if found && key.Style == 0 { + // Copy key to create the new parent with the first part of the path. + newKey := *key + newKey.Value = base + newKey.FootComment = "" + newKey.HeadComment = "" + newKey.LineComment = "" + + // Copy key also to create the key of the child value. + newChildKey := *key + newChildKey.Value = rest + + // Copy the parent node to create the nested object, that contains the new + // child key and the original value. + newNode := *node + newNode.Content = []*yaml.Node{ + &newChildKey, + value, + } + + // Replace current key and value. + node.Content[i] = &newKey + node.Content[i+1] = &newNode + } + + // Recurse on the current value. + extendNestedObjects(node.Content[i+1]) + } + + mergeNodes(node) +} + +// mergeNodes merges the contents of keys with the same name. +func mergeNodes(node *yaml.Node) { + keys := make(map[string]*yaml.Node) + k := 0 + for i := 0; i < len(node.Content); i += 2 { + key := node.Content[i] + value := node.Content[i+1] + + merged, found := keys[key.Value] + if !found { + keys[key.Value] = value + node.Content[k] = key + node.Content[k+1] = value + k += 2 + continue + } + + merged.Content = append(merged.Content, value.Content...) + } + + node.Content = node.Content[:k] +} diff --git a/internal/formatter/yaml_formatter_test.go b/internal/formatter/yaml_formatter_test.go new file mode 100644 index 000000000..d714e2c92 --- /dev/null +++ b/internal/formatter/yaml_formatter_test.go @@ -0,0 +1,118 @@ +// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one +// or more contributor license agreements. Licensed under the Elastic License; +// you may not use this file except in compliance with the Elastic License. + +package formatter + +import ( + "testing" + + "github.com/Masterminds/semver/v3" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestYAMLFormatterNestedObjects(t *testing.T) { + cases := []struct { + title string + doc string + expected string + }{ + { + title: "one-level nested setting", + doc: `foo.bar: 3`, + expected: `foo: + bar: 3 +`, + }, + { + title: "two-level nested setting", + doc: `foo.bar.baz: 3`, + expected: `foo: + bar: + baz: 3 +`, + }, + { + title: "nested setting at second level", + doc: `foo: + bar.baz: 3`, + expected: `foo: + bar: + baz: 3 +`, + }, + { + title: "two two-level nested settings", + doc: `foo.bar.baz: 3 +a.b.c: 42`, + expected: `foo: + bar: + baz: 3 +a: + b: + c: 42 +`, + }, + { + title: "keep comments with the leaf value", + doc: `foo.bar.baz: 3 # baz +# Mistery of life and everything else. +a.b.c: 42`, + expected: `foo: + bar: + baz: 3 # baz +a: + b: + # Mistery of life and everything else. + c: 42 +`, + }, + { + title: "keep double-quoted keys", + doc: `"foo.bar.baz": 3`, + expected: "\"foo.bar.baz\": 3\n", + }, + { + title: "keep single-quoted keys", + doc: `"foo.bar.baz": 3`, + expected: "\"foo.bar.baz\": 3\n", + }, + { + title: "array of maps", + doc: `foo: + - foo.bar: 1 + - foo.bar: 2`, + expected: `foo: + - foo: + bar: 1 + - foo: + bar: 2 +`, + }, + { + title: "merge keys", + doc: `es.something: true +es.other.thing: false +es.other.level: 13`, + expected: `es: + something: true + other: + thing: false + level: 13 +`, + }, + } + + sv := semver.MustParse("3.0.0") + formatter := NewYAMLFormatter(*sv).Format + + for _, c := range cases { + t.Run(c.title, func(t *testing.T) { + result, _, err := formatter([]byte(c.doc)) + require.NoError(t, err) + assert.Equal(t, c.expected, string(result)) + }) + } + +} diff --git a/internal/packages/changelog/yaml.go b/internal/packages/changelog/yaml.go index 31f0ec620..53a8eb36b 100644 --- a/internal/packages/changelog/yaml.go +++ b/internal/packages/changelog/yaml.go @@ -125,7 +125,8 @@ func formatResult(result interface{}) ([]byte, error) { if err != nil { return nil, errors.New("failed to encode") } - d, _, err = formatter.YAMLFormatter(d) + yamlFormatter := &formatter.YAMLFormatter{} + d, _, err = yamlFormatter.Format(d) if err != nil { return nil, errors.New("failed to format") }