Merge pull request #38 from ivixvi/refactor-re-assign

Refactor re assign
ivixvi · Feb 11, 2025 · e21e9b6 · e21e9b6
2 parents d8b52fa + 37bfbd8
commit e21e9b6
Show file tree

Hide file tree

Showing 6 changed files with 80 additions and 99 deletions.
diff --git a/adder.go b/adder.go
@@ -8,7 +8,7 @@ type adder struct{}
 
 var adderInstance *adder
 
-func (r *adder) Direct(scopedMap map[string]interface{}, scopedAttr string, value interface{}) (map[string]interface{}, bool) {
+func (r *adder) Direct(scopedMap map[string]interface{}, scopedAttr string, value interface{}) bool {
 	switch newValue := value.(type) {
 	case []map[string]interface{}:
 		return r.addMapSlice(scopedMap, scopedAttr, newValue)
@@ -19,19 +19,19 @@ func (r *adder) Direct(scopedMap map[string]interface{}, scopedAttr string, valu
 	case interface{}:
 		return r.addValue(scopedMap, scopedAttr, newValue)
 	}
-	return scopedMap, false
+	return false
 }
 
-func (r *adder) addMapSlice(scopedMap map[string]interface{}, scopedAttr string, newValue []map[string]interface{}) (map[string]interface{}, bool) {
+func (r *adder) addMapSlice(scopedMap map[string]interface{}, scopedAttr string, newValue []map[string]interface{}) bool {
 	oldSlice, ok := scopedMap[scopedAttr]
 	if !ok {
 		scopedMap[scopedAttr] = newValue
-		return scopedMap, true
+		return true
 	}
 	oldMaps, ok := areEveryItemsMap(oldSlice)
 	if !ok {
 		scopedMap[scopedAttr] = newValue
-		return scopedMap, true
+		return true
 	}
 	changed := false
 	for _, newMap := range newValue {
@@ -43,26 +43,26 @@ func (r *adder) addMapSlice(scopedMap map[string]interface{}, scopedAttr string,
 	if changed {
 		scopedMap[scopedAttr] = oldMaps
 	}
-	return scopedMap, changed
+	return changed
 }
 
-func (r *adder) addMap(scopedMap map[string]interface{}, scopedAttr string, newValue map[string]interface{}) (map[string]interface{}, bool) {
+func (r *adder) addMap(scopedMap map[string]interface{}, scopedAttr string, newValue map[string]interface{}) bool {
 	oldMap, ok := scopedMap[scopedAttr].(map[string]interface{})
 	if ok {
 		changed := false
 		scopedMap[scopedAttr], changed = mergeMap(oldMap, newValue)
-		return scopedMap, changed
+		return changed
 	}
 	scopedMap[scopedAttr] = newValue
-	return scopedMap, true
+	return true
 }
 
-func (r *adder) addSlice(scopedMap map[string]interface{}, scopedAttr string, newValue []interface{}) (map[string]interface{}, bool) {
+func (r *adder) addSlice(scopedMap map[string]interface{}, scopedAttr string, newValue []interface{}) bool {
 	oldSlice, ok := scopedMap[scopedAttr].([]interface{})
 	// oldSlice is nil
 	if !ok {
 		scopedMap[scopedAttr] = newValue
-		return scopedMap, true
+		return true
 	}
 
 	// Complex MultiValued
@@ -81,15 +81,15 @@ func (r *adder) addSlice(scopedMap map[string]interface{}, scopedAttr string, ne
 	if changed {
 		scopedMap[scopedAttr] = oldSlice
 	}
-	return scopedMap, changed
+	return changed
 }
 
-func (r *adder) addValue(scopedMap map[string]interface{}, scopedAttr string, newValue interface{}) (map[string]interface{}, bool) {
+func (r *adder) addValue(scopedMap map[string]interface{}, scopedAttr string, newValue interface{}) bool {
 	if oldValue, ok := scopedMap[scopedAttr]; !ok || oldValue != newValue {
 		scopedMap[scopedAttr] = newValue
-		return scopedMap, true
+		return true
 	}
-	return scopedMap, false
+	return false
 }
 
 func (r *adder) ByValueForItem(scopedSlice []interface{}, value interface{}) ([]interface{}, bool) {
@@ -102,46 +102,31 @@ func (r *adder) ByValueForItem(scopedSlice []interface{}, value interface{}) ([]
 }
 
 func (r *adder) ByValueExpressionForItem(scopedMaps []map[string]interface{}, expr filter.Expression, value interface{}) ([]map[string]interface{}, bool) {
-	switch newValue := value.(type) {
-	case map[string]interface{}:
-		changed := false
-		newValues := []map[string]interface{}{}
-		for _, oldValue := range scopedMaps {
-			if isMatchExpression(oldValue, expr) && !eqMap(oldValue, newValue) {
-				var merger map[string]interface{}
-				merger, changed = mergeMap(oldValue, newValue)
-				newValues = append(newValues, merger)
-			} else {
-				newValues = append(newValues, oldValue)
-			}
-		}
-		return newValues, changed
-	default:
-		// unexpected input
+	newValue, ok := value.(map[string]interface{})
+
+	// unexpected input
+	if !ok {
 		return scopedMaps, false
 	}
-}
 
-func (r *adder) ByValueExpressionForAttribute(scopedMaps []map[string]interface{}, expr filter.Expression, subAttr string, value interface{}) ([]map[string]interface{}, bool) {
 	changed := false
-	newValues := []map[string]interface{}{}
-	found := false
-	for _, oldValue := range scopedMaps {
-		if isMatchExpression(oldValue, expr) {
-			found = true
-			oldAttrValue, ok := oldValue[subAttr]
-			if !ok || oldAttrValue != value {
-				changed = true
-				oldValue[subAttr] = value
-			}
+	for i, oldValue := range scopedMaps {
+		if isMatchExpression(oldValue, expr) && !eqMap(oldValue, newValue) {
+			var merger map[string]interface{}
+			merger, changed = mergeMap(oldValue, newValue)
+			scopedMaps[i] = merger
 		}
-		newValues = append(newValues, oldValue)
 	}
+	return scopedMaps, changed
+}
+
+func (r *adder) ByValueExpressionForAttribute(scopedMaps []map[string]interface{}, expr filter.Expression, subAttr string, value interface{}) ([]map[string]interface{}, bool) {
+	scopedMaps, changed, found := replaceByValueExpressionForAttribute(scopedMaps, expr, subAttr, value)
 	if !found {
 		changed = true
 		newMap := toMap(expr)
 		newMap[subAttr] = value
-		newValues = append(newValues, newMap)
+		scopedMaps = append(scopedMaps, newMap)
 	}
-	return newValues, changed
+	return scopedMaps, changed
 }
diff --git a/operator.go b/operator.go
@@ -9,7 +9,7 @@ import "github.com/scim2/filter-parser/v2"
 // ByValueExpressionForItem は 対象の属性が、MultiValuedComplexAttribute で path にて valFilter が指定されているときにそれを受けとって更新後のスライスと変更有無を返却します。
 // ByValueExpressionForAttribute は 対象の属性が、MultiValuedComplexAttribute で path にて valFilter と subAttr が指定されているときにそれを受けとって更新後のスライスと変更有無を返却します。
 type Operator interface {
-	Direct(scopedMap map[string]interface{}, scopedAttr string, value interface{}) (map[string]interface{}, bool)
+	Direct(scopedMap map[string]interface{}, scopedAttr string, value interface{}) bool
 	ByValueForItem(scopedSlice []interface{}, value interface{}) ([]interface{}, bool)
 	ByValueExpressionForItem(scopedMaps []map[string]interface{}, expr filter.Expression, value interface{}) ([]map[string]interface{}, bool)
 	ByValueExpressionForAttribute(scopedMaps []map[string]interface{}, expr filter.Expression, subAttr string, value interface{}) ([]map[string]interface{}, bool)

diff --git a/remover.go b/remover.go
@@ -6,12 +6,12 @@ type remover struct{}
 
 var removerInstance *remover
 
-func (r *remover) Direct(scopedMap map[string]interface{}, scopedAttr string, value interface{}) (map[string]interface{}, bool) {
+func (r *remover) Direct(scopedMap map[string]interface{}, scopedAttr string, value interface{}) bool {
 	if _, ok := scopedMap[scopedAttr]; ok {
 		delete(scopedMap, scopedAttr)
-		return scopedMap, true
+		return true
 	}
-	return scopedMap, false
+	return false
 }
 
 func (r *remover) ByValueForItem(scopedSlice []interface{}, value interface{}) ([]interface{}, bool) {

diff --git a/replacer.go b/replacer.go
@@ -8,7 +8,7 @@ type replacer struct{}
 
 var replacerInstance *replacer
 
-func (r *replacer) Direct(scopedMap map[string]interface{}, scopedAttr string, value interface{}) (map[string]interface{}, bool) {
+func (r *replacer) Direct(scopedMap map[string]interface{}, scopedAttr string, value interface{}) bool {
 	switch newValue := value.(type) {
 	case []map[string]interface{}:
 		return r.replaceMapSlice(scopedMap, scopedAttr, newValue)
@@ -19,44 +19,44 @@ func (r *replacer) Direct(scopedMap map[string]interface{}, scopedAttr string, v
 	case interface{}:
 		return r.replaceValue(scopedMap, scopedAttr, newValue)
 	}
-	return scopedMap, false
+	return false
 }
 
-func (r *replacer) replaceMapSlice(scopedMap map[string]interface{}, scopedAttr string, newValue []map[string]interface{}) (map[string]interface{}, bool) {
+func (r *replacer) replaceMapSlice(scopedMap map[string]interface{}, scopedAttr string, newValue []map[string]interface{}) bool {
 	oldSlice, ok := scopedMap[scopedAttr]
 	if !ok {
 		scopedMap[scopedAttr] = newValue
-		return scopedMap, true
+		return true
 	}
 	oldMaps, ok := areEveryItemsMap(oldSlice)
 	if !ok || len(oldMaps) != len(newValue) {
 		scopedMap[scopedAttr] = newValue
-		return scopedMap, true
+		return true
 	}
 	for _, newMap := range newValue {
 		if !containsMap(oldMaps, newMap) {
 			scopedMap[scopedAttr] = newValue
-			return scopedMap, true
+			return true
 		}
 	}
-	return scopedMap, false
+	return false
 }
 
-func (r *replacer) replaceMap(scopedMap map[string]interface{}, scopedAttr string, newValue map[string]interface{}) (map[string]interface{}, bool) {
+func (r *replacer) replaceMap(scopedMap map[string]interface{}, scopedAttr string, newValue map[string]interface{}) bool {
 	oldMap, ok := scopedMap[scopedAttr].(map[string]interface{})
 	if ok && eqMap(newValue, oldMap) {
-		return scopedMap, false
+		return false
 	}
 	scopedMap[scopedAttr] = newValue
-	return scopedMap, true
+	return true
 }
 
-func (r *replacer) replaceSlice(scopedMap map[string]interface{}, scopedAttr string, newValue []interface{}) (map[string]interface{}, bool) {
+func (r *replacer) replaceSlice(scopedMap map[string]interface{}, scopedAttr string, newValue []interface{}) bool {
 	oldSlice, ok := scopedMap[scopedAttr].([]interface{})
 	// oldSlice is nil
 	if !ok || len(oldSlice) != len(newValue) {
 		scopedMap[scopedAttr] = newValue
-		return scopedMap, true
+		return true
 	}
 
 	// Complex MultiValued
@@ -68,24 +68,24 @@ func (r *replacer) replaceSlice(scopedMap map[string]interface{}, scopedAttr str
 	for _, newItem := range newValue {
 		if !containsItem(oldSlice, newItem) {
 			scopedMap[scopedAttr] = newValue
-			return scopedMap, true
+			return true
 		}
 	}
 
-	return scopedMap, false
+	return false
 }
 
-func (r *replacer) replaceValue(scopedMap map[string]interface{}, scopedAttr string, newValue interface{}) (map[string]interface{}, bool) {
+func (r *replacer) replaceValue(scopedMap map[string]interface{}, scopedAttr string, newValue interface{}) bool {
 	if oldValue, ok := scopedMap[scopedAttr]; !ok || oldValue != newValue {
 		scopedMap[scopedAttr] = newValue
-		return scopedMap, true
+		return true
 	}
-	return scopedMap, false
+	return false
 }
 
 func (r *replacer) ByValueForItem(scopedSlice []interface{}, value interface{}) ([]interface{}, bool) {
 	changed := false
-	if containsItem(scopedSlice, value) {
+	if !containsItem(scopedSlice, value) {
 		changed = true
 		scopedSlice = append(scopedSlice, value)
 	}
@@ -101,32 +101,37 @@ func (r *replacer) ByValueExpressionForItem(scopedMaps []map[string]interface{},
 	}
 
 	changed := false
-	newValues := []map[string]interface{}{}
-	for _, oldValue := range scopedMaps {
+	for i, oldValue := range scopedMaps {
 		if isMatchExpression(oldValue, expr) && !eqMap(oldValue, newValue) {
 			changed = true
-			newValues = append(newValues, newValue)
-		} else {
-			newValues = append(newValues, oldValue)
+			scopedMaps[i] = newValue
 		}
-
 	}
-	return newValues, changed
-
+	return scopedMaps, changed
 }
 
 func (r *replacer) ByValueExpressionForAttribute(scopedMaps []map[string]interface{}, expr filter.Expression, subAttr string, value interface{}) ([]map[string]interface{}, bool) {
+	scopedMaps, changed, _ := replaceByValueExpressionForAttribute(scopedMaps, expr, subAttr, value)
+	return scopedMaps, changed
+}
+
+func replaceByValueExpressionForAttribute(
+	scopedMaps []map[string]interface{},
+	expr filter.Expression,
+	subAttr string,
+	value interface{},
+) ([]map[string]interface{}, bool, bool) {
 	changed := false
-	newValues := []map[string]interface{}{}
+	found := false
 	for _, oldValue := range scopedMaps {
 		if isMatchExpression(oldValue, expr) {
+			found = true
 			oldAttrValue, ok := oldValue[subAttr]
 			if !ok || oldAttrValue != value {
 				changed = true
 				oldValue[subAttr] = value
 			}
 		}
-		newValues = append(newValues, oldValue)
 	}
-	return newValues, changed
+	return scopedMaps, changed, found
 }
diff --git a/scimpatch.go b/scimpatch.go
@@ -153,9 +153,8 @@ func (p *Patcher) pathSpecifiedOperate(
 		n.ApplyScopedMapSlice(newValues)
 	case !attr.MultiValued() || op.Path.ValueExpression == nil:
 		scopedMap, scopedAttr := n.GetScopedMap()
-		scopedMap, changed = operator.Direct(scopedMap, scopedAttr, op.Value)
+		changed = operator.Direct(scopedMap, scopedAttr, op.Value)
 		n.ApplyScopedMap(scopedMap)
-		data = n.GetMap()
 	}
 
 	return data, changed, nil
@@ -183,19 +182,17 @@ func (p *Patcher) pathUnspecifiedOperate(
 						continue
 					}
 					for scopedAttr, scopedValue := range newUriMap {
-						scopedMap, changed_ := operator.Direct(oldMap, scopedAttr, scopedValue)
+						changed_ := operator.Direct(oldMap, scopedAttr, scopedValue)
 						if changed_ {
 							changed = changed_ || changed
-							oldMap = scopedMap
 						}
 					}
 					data[uriPrefix.ID] = oldMap
 				}
 			} else {
-				data_, changed_ := operator.Direct(data, attr, value)
+				changed_ := operator.Direct(data, attr, value)
 				if changed_ {
 					changed = changed_ || changed
-					data = data_
 				}
 			}
 		}