From f1fbfdf6d08bdea90f4ac29d158442b1fee0f184 Mon Sep 17 00:00:00 2001 From: Sam Coe Date: Mon, 22 Nov 2021 09:24:03 -0800 Subject: [PATCH] Fix up bug in RemoveEntry and add tests for config_map --- internal/config/config_map.go | 67 ++++++++------- internal/config/config_map_test.go | 126 ++++++++++++++++++++++++++++- 2 files changed, 162 insertions(+), 31 deletions(-) diff --git a/internal/config/config_map.go b/internal/config/config_map.go index 8afaf3a4c..c391bc486 100644 --- a/internal/config/config_map.go +++ b/internal/config/config_map.go @@ -6,7 +6,7 @@ import ( "gopkg.in/yaml.v3" ) -// This type implements a low-level get/set config that is backed by an in-memory tree of Yaml +// This type implements a low-level get/set config that is backed by an in-memory tree of yaml // nodes. It allows us to interact with a yaml-based config programmatically, preserving any // comments that were present when the yaml was parsed. type ConfigMap struct { @@ -37,41 +37,41 @@ func (cm *ConfigMap) GetStringValue(key string) (string, error) { func (cm *ConfigMap) SetStringValue(key, value string) error { entry, err := cm.FindEntry(key) + if err == nil { + entry.ValueNode.Value = value + return nil + } var notFound *NotFoundError - - valueNode := entry.ValueNode - - if err != nil && errors.As(err, ¬Found) { - keyNode := &yaml.Node{ - Kind: yaml.ScalarNode, - Value: key, - } - valueNode = &yaml.Node{ - Kind: yaml.ScalarNode, - Tag: "!!str", - Value: "", - } - - cm.Root.Content = append(cm.Root.Content, keyNode, valueNode) - } else if err != nil { + if err != nil && !errors.As(err, ¬Found) { return err } - valueNode.Value = value + keyNode := &yaml.Node{ + Kind: yaml.ScalarNode, + Value: key, + } + valueNode := &yaml.Node{ + Kind: yaml.ScalarNode, + Tag: "!!str", + Value: value, + } + cm.Root.Content = append(cm.Root.Content, keyNode, valueNode) return nil } -func (cm *ConfigMap) FindEntry(key string) (ce *ConfigEntry, err error) { - err = nil +func (cm *ConfigMap) FindEntry(key string) (*ConfigEntry, error) { + ce := &ConfigEntry{} - ce = &ConfigEntry{} + if cm.Empty() { + return ce, &NotFoundError{errors.New("not found")} + } - // Content slice goes [key1, value1, key2, value2, ...] + // Content slice goes [key1, value1, key2, value2, ...]. topLevelPairs := cm.Root.Content for i, v := range topLevelPairs { - // Skip every other slice item since we only want to check against keys + // Skip every other slice item since we only want to check against keys. if i%2 != 0 { continue } @@ -81,7 +81,7 @@ func (cm *ConfigMap) FindEntry(key string) (ce *ConfigEntry, err error) { if i+1 < len(topLevelPairs) { ce.ValueNode = topLevelPairs[i+1] } - return + return ce, nil } } @@ -89,14 +89,23 @@ func (cm *ConfigMap) FindEntry(key string) (ce *ConfigEntry, err error) { } func (cm *ConfigMap) RemoveEntry(key string) { + if cm.Empty() { + return + } + newContent := []*yaml.Node{} - content := cm.Root.Content - for i := 0; i < len(content); i++ { - if content[i].Value == key { - i++ // skip the next node which is this key's value + var skipNext bool + for i, v := range cm.Root.Content { + if skipNext { + skipNext = false + continue + } + if i%2 != 0 || v.Value != key { + newContent = append(newContent, v) } else { - newContent = append(newContent, content[i]) + // Don't append current node and skip the next which is this key's value. + skipNext = true } } diff --git a/internal/config/config_map_test.go b/internal/config/config_map_test.go index c504e4cbc..4dc49d01b 100644 --- a/internal/config/config_map_test.go +++ b/internal/config/config_map_test.go @@ -1,7 +1,6 @@ package config import ( - "fmt" "testing" "github.com/stretchr/testify/assert" @@ -46,12 +45,135 @@ func TestFindEntry(t *testing.T) { return } assert.NoError(t, err) - fmt.Println(out) assert.Equal(t, tt.output, out.ValueNode.Value) }) } } +func TestEmpty(t *testing.T) { + cm := ConfigMap{} + assert.Equal(t, true, cm.Empty()) + cm.Root = &yaml.Node{ + Content: []*yaml.Node{ + { + Value: "test", + }, + }, + } + assert.Equal(t, false, cm.Empty()) +} + +func TestGetStringValue(t *testing.T) { + tests := []struct { + name string + key string + wantValue string + wantErr bool + }{ + { + name: "get key", + key: "valid", + wantValue: "present", + }, + { + name: "get key that is not present", + key: "invalid", + wantErr: true, + }, + { + name: "get key that has same content as a value", + key: "same", + wantValue: "logical", + }, + } + + for _, tt := range tests { + cm := ConfigMap{Root: testYaml()} + t.Run(tt.name, func(t *testing.T) { + val, err := cm.GetStringValue(tt.key) + if tt.wantErr { + assert.EqualError(t, err, "not found") + return + } + assert.Equal(t, tt.wantValue, val) + }) + } +} + +func TestSetStringValue(t *testing.T) { + tests := []struct { + name string + key string + value string + }{ + { + name: "set key that is not present", + key: "notPresent", + value: "test1", + }, + { + name: "set key that is present", + key: "erroneous", + value: "test2", + }, + { + name: "set key that is blank", + key: "blank", + value: "test3", + }, + { + name: "set key that has same content as a value", + key: "present", + value: "test4", + }, + } + + for _, tt := range tests { + cm := ConfigMap{Root: testYaml()} + t.Run(tt.name, func(t *testing.T) { + err := cm.SetStringValue(tt.key, tt.value) + assert.NoError(t, err) + val, err := cm.GetStringValue(tt.key) + assert.NoError(t, err) + assert.Equal(t, tt.value, val) + }) + } +} + +func TestRemoveEntry(t *testing.T) { + tests := []struct { + name string + key string + wantLength int + }{ + { + name: "remove key", + key: "erroneous", + wantLength: 6, + }, + { + name: "remove key that is not present", + key: "invalid", + wantLength: 8, + }, + { + name: "remove key that has same content as a value", + key: "same", + wantLength: 6, + }, + } + + for _, tt := range tests { + cm := ConfigMap{Root: testYaml()} + t.Run(tt.name, func(t *testing.T) { + cm.RemoveEntry(tt.key) + assert.Equal(t, tt.wantLength, len(cm.Root.Content)) + _, err := cm.FindEntry(tt.key) + assert.EqualError(t, err, "not found") + }) + } +} + func testYaml() *yaml.Node { var root yaml.Node var data = `