From 9d60cddc775281a47b277a7b3c52573e4596b681 Mon Sep 17 00:00:00 2001 From: khushijain21 Date: Sat, 2 Nov 2024 16:58:46 +0530 Subject: [PATCH 1/4] Do not throw error if alterfunc does not change the key --- mapstr/mapstr.go | 18 +++++++++++------- mapstr/mapstr_test.go | 16 ++++++++++++++++ 2 files changed, 27 insertions(+), 7 deletions(-) diff --git a/mapstr/mapstr.go b/mapstr/mapstr.go index 9d92b71..ea77d29 100644 --- a/mapstr/mapstr.go +++ b/mapstr/mapstr.go @@ -218,12 +218,16 @@ func (m M) AlterPath(path string, mode TraversalMode, alterFunc AlterFunc) (err if newKey == "" { return fmt.Errorf("replacement key for %q cannot be empty", key) } - _, exists := level[newKey] - if exists { - return fmt.Errorf("replacement key %q already exists: %w", newKey, ErrKeyCollision) + + // if altered key is equal to the original key, skip below delete/put func + if newKey != key { + _, exists := level[newKey] + if exists { + return fmt.Errorf("replacement key %q already exists: %w", newKey, ErrKeyCollision) + } + delete(level, key) + level[newKey] = val } - delete(level, key) - level[newKey] = val return nil }) @@ -271,7 +275,7 @@ func (m M) Traverse(path string, mode TraversalMode, visitor TraversalVisitor) ( for i, segment := range segments { if !found { - return ErrKeyNotFound + return fmt.Errorf("could not fetch value for key: %s, Error: %w ", path, ErrKeyNotFound) } found = false @@ -316,7 +320,7 @@ func (m M) Traverse(path string, mode TraversalMode, visitor TraversalVisitor) ( } if !found { - return ErrKeyNotFound + return fmt.Errorf("could not fetch value for key: %s, Error: %w", path, ErrKeyNotFound) } return nil diff --git a/mapstr/mapstr_test.go b/mapstr/mapstr_test.go index 2d3a791..65af644 100644 --- a/mapstr/mapstr_test.go +++ b/mapstr/mapstr_test.go @@ -1264,6 +1264,22 @@ func TestAlterPath(t *testing.T) { }, }, }, + { + name: "alters keys on second level with case-insensitive matching", + from: "level1_field1.key", + mode: CaseInsensitiveMode, + alterFunc: lower, + m: M{ + "level1_field1": M{ + "Key": "value1", + }, + }, + exp: M{ + "level1_field1": M{ + "key": "value1", + }, + }, + }, { name: "alters keys on all nested levels with case-insensitive matching", from: "level1_field1.level2_field1.level3_field1", From 9e9ebbb2556c4af6966037eea99e5bf1bbe7ac32 Mon Sep 17 00:00:00 2001 From: khushijain21 Date: Sat, 2 Nov 2024 17:03:41 +0530 Subject: [PATCH 2/4] test case explanation --- mapstr/mapstr_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mapstr/mapstr_test.go b/mapstr/mapstr_test.go index 65af644..f0020ca 100644 --- a/mapstr/mapstr_test.go +++ b/mapstr/mapstr_test.go @@ -1275,7 +1275,7 @@ func TestAlterPath(t *testing.T) { }, }, exp: M{ - "level1_field1": M{ + "level1_field1": M{ //first level is unchanged - already in lowercase "key": "value1", }, }, From ae9efea4c30388ec8b6a0f19ad4ecced4160fb5b Mon Sep 17 00:00:00 2001 From: Khushi Jain Date: Mon, 4 Nov 2024 14:57:19 +0530 Subject: [PATCH 3/4] Update mapstr/mapstr_test.go Co-authored-by: Denis --- mapstr/mapstr_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mapstr/mapstr_test.go b/mapstr/mapstr_test.go index f0020ca..8fbbf5d 100644 --- a/mapstr/mapstr_test.go +++ b/mapstr/mapstr_test.go @@ -1265,7 +1265,7 @@ func TestAlterPath(t *testing.T) { }, }, { - name: "alters keys on second level with case-insensitive matching", + name: "does not return an error if alterFunc returns the key unchanged", from: "level1_field1.key", mode: CaseInsensitiveMode, alterFunc: lower, From b0f2d618476aa3fda4284b7c25257006eaafd9e8 Mon Sep 17 00:00:00 2001 From: khushijain21 Date: Mon, 4 Nov 2024 15:06:06 +0530 Subject: [PATCH 4/4] update --- mapstr/mapstr.go | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/mapstr/mapstr.go b/mapstr/mapstr.go index ea77d29..b3e16a1 100644 --- a/mapstr/mapstr.go +++ b/mapstr/mapstr.go @@ -220,14 +220,16 @@ func (m M) AlterPath(path string, mode TraversalMode, alterFunc AlterFunc) (err } // if altered key is equal to the original key, skip below delete/put func - if newKey != key { - _, exists := level[newKey] - if exists { - return fmt.Errorf("replacement key %q already exists: %w", newKey, ErrKeyCollision) - } - delete(level, key) - level[newKey] = val + if newKey == key { + return nil + } + + _, exists := level[newKey] + if exists { + return fmt.Errorf("replacement key %q already exists: %w", newKey, ErrKeyCollision) } + delete(level, key) + level[newKey] = val return nil })