Skip to content

Commit

Permalink
fix(fsrepo): deep merge when setting config
Browse files Browse the repository at this point in the history
  • Loading branch information
kylehuntsman committed Mar 3, 2022
1 parent 3ea5631 commit e1d1444
Show file tree
Hide file tree
Showing 4 changed files with 165 additions and 5 deletions.
30 changes: 30 additions & 0 deletions repo/common/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,3 +54,33 @@ func MapSetKV(v map[string]interface{}, key string, value interface{}) error {
}
return nil
}

// Merges the right map into the left map, recursively traversing child maps
// until a non-map value is found
func MapMergeDeep(left, right map[string]interface{}) map[string]interface{} {
// We want to alter a copy of the map, not the original
result := make(map[string]interface{})
for k, v := range left {
result[k] = v
}

for key, rightVal := range right {
// If right value is a map
if rightMap, ok := rightVal.(map[string]interface{}); ok {
// If key is in left
if leftVal, found := result[key]; found {
// If left value is also a map
if leftMap, ok := leftVal.(map[string]interface{}); ok {
// Merge nested map
result[key] = MapMergeDeep(leftMap, rightMap)
continue
}
}
}

// Otherwise set new value to result
result[key] = rightVal
}

return result
}
132 changes: 132 additions & 0 deletions repo/common/common_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
package common

import (
"testing"

"github.com/ipfs/go-ipfs/thirdparty/assert"
)

func TestMapMergeDeepReturnsNew(t *testing.T) {
leftMap := make(map[string]interface{})
leftMap["A"] = "Hello World"

rightMap := make(map[string]interface{})
rightMap["A"] = "Foo"

MapMergeDeep(leftMap, rightMap)

assert.True(leftMap["A"] == "Hello World", t, "MapMergeDeep should return a new map instance")
}

func TestMapMergeDeepNewKey(t *testing.T) {
leftMap := make(map[string]interface{})
leftMap["A"] = "Hello World"
/*
leftMap
{
A: "Hello World"
}
*/

rightMap := make(map[string]interface{})
rightMap["B"] = "Bar"
/*
rightMap
{
B: "Bar"
}
*/

result := MapMergeDeep(leftMap, rightMap)
/*
expected
{
A: "Hello World"
B: "Bar"
}
*/

assert.True(result["B"] == "Bar", t, "New keys in right map should exist in resulting map")
}

func TestMapMergeDeepRecursesOnMaps(t *testing.T) {
leftMapA := make(map[string]interface{})
leftMapA["B"] = "A value!"
leftMapA["C"] = "Another value!"

leftMap := make(map[string]interface{})
leftMap["A"] = leftMapA
/*
leftMap
{
A: {
B: "A value!"
C: "Another value!"
}
}
*/

rightMapA := make(map[string]interface{})
rightMapA["C"] = "A different value!"

rightMap := make(map[string]interface{})
rightMap["A"] = rightMapA
/*
rightMap
{
A: {
C: "A different value!"
}
}
*/

result := MapMergeDeep(leftMap, rightMap)
/*
expected
{
A: {
B: "A value!"
C: "A different value!"
}
}
*/

resultA := result["A"].(map[string]interface{})
assert.True(resultA["B"] == "A value!", t, "Unaltered values should not change")
assert.True(resultA["C"] == "A different value!", t, "Nested values should be altered")
}

func TestMapMergeDeepRightNotAMap(t *testing.T) {
leftMapA := make(map[string]interface{})
leftMapA["B"] = "A value!"

leftMap := make(map[string]interface{})
leftMap["A"] = leftMapA
/*
origMap
{
A: {
B: "A value!"
}
}
*/

rightMap := make(map[string]interface{})
rightMap["A"] = "Not a map!"
/*
newMap
{
A: "Not a map!"
}
*/

result := MapMergeDeep(leftMap, rightMap)
/*
expected
{
A: "Not a map!"
}
*/

assert.True(result["A"] == "Not a map!", t, "Right values that are not a map should be set on the result")
}
6 changes: 2 additions & 4 deletions repo/fsrepo/fsrepo.go
Original file line number Diff line number Diff line change
Expand Up @@ -559,10 +559,8 @@ func (r *FSRepo) setConfigUnsynced(updated *config.Config) error {
if err != nil {
return err
}
for k, v := range m {
mapconf[k] = v
}
if err := serialize.WriteConfigFile(configFilename, mapconf); err != nil {
mergedMap := common.MapMergeDeep(mapconf, m)
if err := serialize.WriteConfigFile(configFilename, mergedMap); err != nil {
return err
}
// Do not use `*r.config = ...`. This will modify the *shared* config
Expand Down
2 changes: 1 addition & 1 deletion test/sharness/t0250-files-api.sh
Original file line number Diff line number Diff line change
Expand Up @@ -876,7 +876,7 @@ test_expect_success "set up automatic sharding/unsharding data" '
'

# TODO: This does not need to report an error https://github.com/ipfs/go-ipfs/issues/8088
test_expect_failure "reset automatic sharding" '
test_expect_success "reset automatic sharding" '
ipfs config --json Internal.UnixFSShardingSizeThreshold null
'

Expand Down

0 comments on commit e1d1444

Please sign in to comment.