From f6526d9bda616c796c2fa06b30af41bb7ebecf0d Mon Sep 17 00:00:00 2001 From: ivixvi Date: Sun, 9 Feb 2025 23:45:46 +0900 Subject: [PATCH 1/6] add: utils --- utils.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/utils.go b/utils.go index 42d5673..dc2a345 100644 --- a/utils.go +++ b/utils.go @@ -63,3 +63,21 @@ func eqMap(m1 map[string]interface{}, m2 map[string]interface{}) bool { } return true } + +func containsMap(slice []map[string]interface{}, item map[string]interface{}) bool { + for _, v := range slice { + if eqMap(v, item) { + return true + } + } + return false +} + +func containsItem(slice []interface{}, item interface{}) bool { + for _, v := range slice { + if v == item { + return true + } + } + return false +} From 12d650e52ac2d1b120a2c8d613eb2710da97c39c Mon Sep 17 00:00:00 2001 From: ivixvi Date: Sun, 9 Feb 2025 23:46:00 +0900 Subject: [PATCH 2/6] refactor: operators --- adder.go | 150 +++++++++++++++++++++++++--------------------------- remover.go | 7 +-- replacer.go | 131 +++++++++++++++++++++------------------------ 3 files changed, 133 insertions(+), 155 deletions(-) diff --git a/adder.go b/adder.go index fcf88e8..46ad96e 100644 --- a/adder.go +++ b/adder.go @@ -11,98 +11,92 @@ var adderInstance *adder func (r *adder) Direct(scopedMap map[string]interface{}, scopedAttr string, value interface{}) (map[string]interface{}, bool) { switch newValue := value.(type) { case []map[string]interface{}: - oldSlice, ok := scopedMap[scopedAttr] - if !ok { - scopedMap[scopedAttr] = newValue - return scopedMap, true - } - oldMaps, ok := areEveryItemsMap(oldSlice) - if !ok { - // WARN: unexpected current value - scopedMap[scopedAttr] = newValue - return scopedMap, true - } - changed := false - for _, newMap := range newValue { - found := false - for _, oldMap := range oldMaps { - if eqMap(newMap, oldMap) { - found = true - break - } - } - if !found { - oldMaps = append(oldMaps, newMap) - changed = true - } - } - if changed { - scopedMap[scopedAttr] = oldMaps - } - return scopedMap, changed + return r.addMapSlice(scopedMap, scopedAttr, newValue) case map[string]interface{}: - oldMap, ok := scopedMap[scopedAttr].(map[string]interface{}) - if ok { - changed := false - scopedMap[scopedAttr], changed = mergeMap(oldMap, newValue) - return scopedMap, changed - } - scopedMap[scopedAttr] = value - return scopedMap, true + return r.addMap(scopedMap, scopedAttr, newValue) case []interface{}: - oldSlice, ok := scopedMap[scopedAttr].([]interface{}) - if !ok { - scopedMap[scopedAttr] = newValue - return scopedMap, true + return r.addSlice(scopedMap, scopedAttr, newValue) + case interface{}: + return r.addValue(scopedMap, scopedAttr, newValue) + } + return scopedMap, false +} + +func (r *adder) addMapSlice(scopedMap map[string]interface{}, scopedAttr string, newValue []map[string]interface{}) (map[string]interface{}, bool) { + oldSlice, ok := scopedMap[scopedAttr] + if !ok { + scopedMap[scopedAttr] = newValue + return scopedMap, true + } + oldMaps, ok := areEveryItemsMap(oldSlice) + if !ok { + // WARN: unexpected current value + scopedMap[scopedAttr] = newValue + return scopedMap, true + } + changed := false + for _, newMap := range newValue { + if !containsMap(oldMaps, newMap) { + oldMaps = append(oldMaps, newMap) + changed = true } + } + if changed { + scopedMap[scopedAttr] = oldMaps + } + return scopedMap, changed +} + +func (r *adder) addMap(scopedMap map[string]interface{}, scopedAttr string, newValue map[string]interface{}) (map[string]interface{}, bool) { + oldMap, ok := scopedMap[scopedAttr].(map[string]interface{}) + if ok { changed := false - if oldMaps, ok := areEveryItemsMap(oldSlice); ok { - if newMaps, ok := areEveryItemsMap(newValue); ok { - for _, newMap := range newMaps { - found := false - for _, oldMap := range oldMaps { - if eqMap(newMap, oldMap) { - found = true - break - } - } - if !found { - oldMaps = append(oldMaps, newMap) - changed = true - } - } - if changed { - scopedMap[scopedAttr] = oldMaps - } + scopedMap[scopedAttr], changed = mergeMap(oldMap, newValue) + return scopedMap, changed + } + scopedMap[scopedAttr] = newValue + return scopedMap, true +} - return scopedMap, changed - } - } else { - for _, newItem := range newValue { - found := false - for _, oldItem := range oldSlice { - if newItem == oldItem { - found = true - break - } - } - if !found { +func (r *adder) addSlice(scopedMap map[string]interface{}, scopedAttr string, newValue []interface{}) (map[string]interface{}, bool) { + oldSlice, ok := scopedMap[scopedAttr].([]interface{}) + if !ok { + scopedMap[scopedAttr] = newValue + return scopedMap, true + } + changed := false + if oldMaps, ok := areEveryItemsMap(oldSlice); ok { + if newMaps, ok := areEveryItemsMap(newValue); ok { + for _, newMap := range newMaps { + if !containsMap(oldMaps, newMap) { + oldMaps = append(oldMaps, newMap) changed = true - oldSlice = append(oldSlice, newItem) } } if changed { - scopedMap[scopedAttr] = oldSlice + scopedMap[scopedAttr] = oldMaps } + return scopedMap, changed } - return scopedMap, changed - case interface{}: - if oldValue, ok := scopedMap[scopedAttr]; !ok || oldValue != newValue { - scopedMap[scopedAttr] = value - return scopedMap, true + } else { + for _, newItem := range newValue { + if !containsItem(oldSlice, newItem) { + oldSlice = append(oldSlice, newItem) + changed = true + } + } + if changed { + scopedMap[scopedAttr] = oldSlice } } + return scopedMap, changed +} +func (r *adder) addValue(scopedMap map[string]interface{}, scopedAttr string, newValue interface{}) (map[string]interface{}, bool) { + if oldValue, ok := scopedMap[scopedAttr]; !ok || oldValue != newValue { + scopedMap[scopedAttr] = newValue + return scopedMap, true + } return scopedMap, false } diff --git a/remover.go b/remover.go index 9ab2d53..7a55e88 100644 --- a/remover.go +++ b/remover.go @@ -47,14 +47,11 @@ func (r *remover) ByValueExpressionForAttribute(scopedMaps []map[string]interfac if !isMatchExpression(oldValue, expr) { newValues = append(newValues, oldValue) } else { - _, ok := oldValue[subAttr] - if ok { + if _, ok := oldValue[subAttr]; ok { changed = true delete(oldValue, subAttr) - newValues = append(newValues, oldValue) - } else { - newValues = append(newValues, oldValue) } + newValues = append(newValues, oldValue) } } return newValues, changed diff --git a/replacer.go b/replacer.go index 5196a33..f683794 100644 --- a/replacer.go +++ b/replacer.go @@ -11,91 +11,78 @@ var replacerInstance *replacer func (r *replacer) Direct(scopedMap map[string]interface{}, scopedAttr string, value interface{}) (map[string]interface{}, bool) { switch newValue := value.(type) { case []map[string]interface{}: - oldSlice, ok := scopedMap[scopedAttr] - if !ok { - scopedMap[scopedAttr] = newValue - return scopedMap, true - } - oldMaps, ok := areEveryItemsMap(oldSlice) - if !ok { - // WARN: unexpected current value - scopedMap[scopedAttr] = newValue - return scopedMap, true - } - if len(oldMaps) != len(newValue) { - scopedMap[scopedAttr] = newValue - return scopedMap, true - } - for _, newMap := range newValue { - found := false - for _, oldMap := range oldMaps { - if eqMap(newMap, oldMap) { - found = true - break - } - } - if !found { - scopedMap[scopedAttr] = newValue - return scopedMap, true - } - } - return scopedMap, false + return r.replaceMapSlice(scopedMap, scopedAttr, newValue) case map[string]interface{}: - oldMap, ok := scopedMap[scopedAttr].(map[string]interface{}) - if ok && eqMap(newValue, oldMap) { - return scopedMap, false - } - scopedMap[scopedAttr] = value - return scopedMap, true + return r.replaceMap(scopedMap, scopedAttr, newValue) case []interface{}: - oldSlice, ok := scopedMap[scopedAttr].([]interface{}) - if !ok { - scopedMap[scopedAttr] = newValue - return scopedMap, true - } - if len(oldSlice) != len(newValue) { + return r.replaceSlice(scopedMap, scopedAttr, newValue) + case interface{}: + return r.replaceValue(scopedMap, scopedAttr, newValue) + } + return scopedMap, false +} + +func (r *replacer) replaceMapSlice(scopedMap map[string]interface{}, scopedAttr string, newValue []map[string]interface{}) (map[string]interface{}, bool) { + oldSlice, ok := scopedMap[scopedAttr] + if !ok { + scopedMap[scopedAttr] = newValue + return scopedMap, true + } + oldMaps, ok := areEveryItemsMap(oldSlice) + // WARN: !ok -> unexpected current value + if !ok || len(oldMaps) != len(newValue) { + scopedMap[scopedAttr] = newValue + return scopedMap, true + } + for _, newMap := range newValue { + if !containsMap(oldMaps, newMap) { scopedMap[scopedAttr] = newValue return scopedMap, true } - if oldMaps, ok := areEveryItemsMap(oldSlice); ok { - if newMaps, ok := areEveryItemsMap(newValue); ok { - for _, newMap := range newMaps { - found := false - for _, oldMap := range oldMaps { - if eqMap(newMap, oldMap) { - found = true - break - } - } - if !found { - scopedMap[scopedAttr] = newValue - return scopedMap, true - } - } - } - } else { - for _, newItem := range newValue { - found := false - for _, oldItem := range oldSlice { - if newItem == oldItem { - found = true - break - } - } - if !found { + } + return scopedMap, false +} + +func (r *replacer) replaceMap(scopedMap map[string]interface{}, scopedAttr string, newValue map[string]interface{}) (map[string]interface{}, bool) { + oldMap, ok := scopedMap[scopedAttr].(map[string]interface{}) + if ok && eqMap(newValue, oldMap) { + return scopedMap, false + } + scopedMap[scopedAttr] = newValue + return scopedMap, true +} + +func (r *replacer) replaceSlice(scopedMap map[string]interface{}, scopedAttr string, newValue []interface{}) (map[string]interface{}, bool) { + oldSlice, ok := scopedMap[scopedAttr].([]interface{}) + if !ok || len(oldSlice) != len(newValue) { + scopedMap[scopedAttr] = newValue + return scopedMap, true + } + if oldMaps, ok := areEveryItemsMap(oldSlice); ok { + if newMaps, ok := areEveryItemsMap(newValue); ok { + for _, newMap := range newMaps { + if !containsMap(oldMaps, newMap) { scopedMap[scopedAttr] = newValue return scopedMap, true } } } - return scopedMap, false - case interface{}: - if oldValue, ok := scopedMap[scopedAttr]; !ok || oldValue != newValue { - scopedMap[scopedAttr] = value - return scopedMap, true + } else { + for _, newItem := range newValue { + if !containsItem(oldSlice, newItem) { + scopedMap[scopedAttr] = newValue + return scopedMap, true + } } } + return scopedMap, false +} +func (r *replacer) replaceValue(scopedMap map[string]interface{}, scopedAttr string, newValue interface{}) (map[string]interface{}, bool) { + if oldValue, ok := scopedMap[scopedAttr]; !ok || oldValue != newValue { + scopedMap[scopedAttr] = newValue + return scopedMap, true + } return scopedMap, false } From b2c8f3b9eae9d52e7755343e9f663928cf6f1083 Mon Sep 17 00:00:00 2001 From: ivixvi Date: Sun, 9 Feb 2025 23:46:08 +0900 Subject: [PATCH 3/6] fix: typo --- path_specified_replace_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/path_specified_replace_test.go b/path_specified_replace_test.go index 9b87d57..c210ec0 100644 --- a/path_specified_replace_test.go +++ b/path_specified_replace_test.go @@ -25,13 +25,13 @@ func TestPathSpecifiedReplace(t *testing.T) { op: scim.PatchOperation{ Op: "replace", Path: path(`externalId`), - Value: "galice", + Value: "g-alice", }, data: map[string]interface{}{ - "externalId": "gbob", + "externalId": "g-bob", }, expected: map[string]interface{}{ - "externalId": "galice", + "externalId": "g-alice", }, expectedChanged: true, }, From 8a79ef9b3014c8cb233b9d64eb4698bd5acba1d5 Mon Sep 17 00:00:00 2001 From: ivixvi Date: Sun, 9 Feb 2025 23:46:24 +0900 Subject: [PATCH 4/6] chore: fix --- utils_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/utils_test.go b/utils_test.go index f1fa44a..b4b64a3 100644 --- a/utils_test.go +++ b/utils_test.go @@ -30,14 +30,14 @@ func TestAreEveryItemsMap(t *testing.T) { }, expectedOk: true, expectedSlice: []map[string]interface{}{ - map[string]interface{}{}, - map[string]interface{}{}, + {}, + {}, }, }, { name: "failed", target: []interface{}{ - "hoge", 31, + "value", 31, }, expectedOk: false, expectedSlice: nil, From 39bd30dd3006849261d3f2c31752421ef71301fc Mon Sep 17 00:00:00 2001 From: ivixvi Date: Sun, 9 Feb 2025 23:48:20 +0900 Subject: [PATCH 5/6] fix: empty row --- scimpatch.go | 1 - 1 file changed, 1 deletion(-) diff --git a/scimpatch.go b/scimpatch.go index 5d526d7..52e2cbb 100644 --- a/scimpatch.go +++ b/scimpatch.go @@ -204,5 +204,4 @@ func (p *Patcher) pathUnspecifiedOperate( // unexpected input return data, false, nil } - } From 27ca64547b9a34b292b501a4d2811579885bc080 Mon Sep 17 00:00:00 2001 From: ivixvi Date: Sun, 9 Feb 2025 23:53:30 +0900 Subject: [PATCH 6/6] tests: add utils test --- export_test.go | 10 ++++++ utils_test.go | 98 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 108 insertions(+) diff --git a/export_test.go b/export_test.go index 2e0b35c..124744a 100644 --- a/export_test.go +++ b/export_test.go @@ -15,3 +15,13 @@ func EqMap(m1 map[string]interface{}, m2 map[string]interface{}) bool { func MergeMap(m1 map[string]interface{}, m2 map[string]interface{}) (map[string]interface{}, bool) { return mergeMap(m1, m2) } + +// ContainsMap is export containsMap for testing +func ContainsMap(slice []map[string]interface{}, item map[string]interface{}) bool { + return containsMap(slice, item) +} + +// ContainsItem is export containsItem for testing +func ContainsItem(slice []interface{}, item interface{}) bool { + return containsItem(slice, item) +} diff --git a/utils_test.go b/utils_test.go index b4b64a3..9d1df18 100644 --- a/utils_test.go +++ b/utils_test.go @@ -213,3 +213,101 @@ func TestMergeMap(t *testing.T) { }) } } + +// TestContainsMap は containsMap をテストします +func TestContainsMap(t *testing.T) { + // Define the test cases + testCases := []struct { + name string + slice []map[string]interface{} + item map[string]interface{} + expected bool + }{ + { + name: "empty slice", + slice: []map[string]interface{}{}, + item: map[string]interface{}{"key": "value"}, + expected: false, + }, + { + name: "item not in slice", + slice: []map[string]interface{}{ + {"key1": "value1"}, + {"key2": "value2"}, + }, + item: map[string]interface{}{"key": "value"}, + expected: false, + }, + { + name: "item in slice", + slice: []map[string]interface{}{ + {"key1": "value1"}, + {"key": "value"}, + }, + item: map[string]interface{}{"key": "value"}, + expected: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + t.Log(tc.name) + // call ContainsMap + ok := scimpatch.ContainsMap(tc.slice, tc.item) + + // Check if the result matches the expected data + if !(tc.expected == ok) { + t.Fatalf("ContainsMap() not returned Expected: %v, %v", tc.expected, ok) + } + }) + } +} + +// TestContainsItem は containsItem をテストします +func TestContainsItem(t *testing.T) { + // Define the test cases + testCases := []struct { + name string + slice []interface{} + item interface{} + expected bool + }{ + { + name: "empty slice", + slice: []interface{}{}, + item: "value", + expected: false, + }, + { + name: "item not in slice", + slice: []interface{}{ + "value1", + "value2", + }, + item: "value", + expected: false, + }, + { + name: "item in slice", + slice: []interface{}{ + "value1", + "value", + }, + item: "value", + expected: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + t.Log(tc.name) + // call ContainsItem + ok := scimpatch.ContainsItem(tc.slice, tc.item) + + // Check if the result matches the expected data + if !(tc.expected == ok) { + t.Fatalf("ContainsItem() not returned Expected: %v, %v", tc.expected, ok) + } + }) + } +}