fix map deletes

r3labs · Jun 30, 2021 · 69da8b8 · 69da8b8
1 parent bad6782
commit 69da8b8
Show file tree

Hide file tree

Showing 3 changed files with 57 additions and 26 deletions.
diff --git a/patch.go b/patch.go
@@ -164,7 +164,6 @@ func NewChangeValue(d *Differ, c Change, target interface{}) (ret *ChangeValue)
 //renderChangeValue applies 'path' in change to target. nil check is foregone
 //                  here as we control usage
 func (d *Differ) renderChangeTarget(c *ChangeValue) {
-
 	//This particular change element may potentially have the immutable flag
 	if c.HasFlag(OptionImmutable) {
 		c.AddError(NewError("Option immutable set, cannot apply change"))
@@ -181,7 +180,7 @@ func (d *Differ) renderChangeTarget(c *ChangeValue) {
 		//map prior to editing the value, it will fail to stick. To fix this, we
 		//defer the safe until the stack unwinds
 		m, k, v := d.renderMap(c)
-		defer d.deleteMapEntry(c, m, k, v)
+		defer d.updateMapEntry(c, m, k, v)
 
 	//path element that is a slice
 	case reflect.Slice:

diff --git a/patch_map.go b/patch_map.go
@@ -9,7 +9,6 @@ import (
 
 //renderMap - handle map rendering for patch
 func (d *Differ) renderMap(c *ChangeValue) (m, k, v *reflect.Value) {
-
 	//we must tease out the type of the key, we use the msgpack from diff to recreate the key
 	kt := c.target.Type().Key()
 	field := reflect.New(kt)
@@ -68,34 +67,41 @@ func (d *Differ) renderMap(c *ChangeValue) (m, k, v *reflect.Value) {
 
 }
 
-//deleteMapEntry - deletes are special, they are handled differently based on options
+// updateMapEntry - deletes are special, they are handled differently based on options
 //            container type etc. We have to have special handling for each
 //            type. Set values are more generic even if they must be instanced
-func (d *Differ) deleteMapEntry(c *ChangeValue, m, k, v *reflect.Value) {
-	if k == nil {
+func (d *Differ) updateMapEntry(c *ChangeValue, m, k, v *reflect.Value) {
+	if k == nil || m == nil {
 		return
 	}
 
-	if m != nil && m.CanSet() && v.IsValid() && v.Kind() == reflect.Struct {
-		for x := 0; x < v.NumField(); x++ {
-			if !v.Field(x).IsZero() {
-				m.SetMapIndex(*k, *v)
-				return
-			}
-		} //if all the fields are zero, remove from map
+	switch c.change.Type {
+	case DELETE:
+		if c.HasFlag(FlagDeleted) {
+			return
+		}
+
+		if !m.CanSet() && v.IsValid() && v.Kind() == reflect.Struct {
+			for x := 0; x < v.NumField(); x++ {
+				if !v.Field(x).IsZero() {
+					m.SetMapIndex(*k, *v)
+					return
+				}
+			} //if all the fields are zero, remove from map
+		}
+
 		m.SetMapIndex(*k, reflect.Value{})
 		c.SetFlag(FlagDeleted)
-	} else {
-		switch c.change.Type {
-		case DELETE:
-			m.SetMapIndex(*k, reflect.Value{})
-			c.SetFlag(FlagDeleted)
-		case CREATE:
-			m.SetMapIndex(*k, *v)
-			c.SetFlag(FlagCreated)
-		case UPDATE:
-			m.SetMapIndex(*k, *v)
-			c.SetFlag(FlagUpdated)
-		}
+
+	case CREATE:
+		m.SetMapIndex(*k, *v)
+		c.SetFlag(FlagCreated)
+
+	case UPDATE:
+		m.SetMapIndex(*k, *v)
+		c.SetFlag(FlagUpdated)
+
+	default:
+		panic("NO TYPE SPECIFIED")
 	}
 }
diff --git a/patch_test.go b/patch_test.go
@@ -193,7 +193,6 @@ func TestPatch(t *testing.T) {
 			},
 			nil,
 		},
-
 		{
 			"map",
 			map[string]interface{}{"1": "one", "3": "three"},
@@ -205,6 +204,33 @@ func TestPatch(t *testing.T) {
 			},
 			nil,
 		},
+		{
+			"map-nested-create",
+			map[string]interface{}{"firstName": "John", "lastName": "Michael", "createdBy": "TS", "details": map[string]interface{}{"status": "active", "attributes": map[string]interface{}{"attrA": "A", "attrB": "B"}}},
+			map[string]interface{}{"firstName": "John", "lastName": "Michael", "createdBy": "TS", "details": map[string]interface{}{"status": "active", "attributes": map[string]interface{}{"attrA": "A", "attrB": "B"}, "secondary-attributes": map[string]interface{}{"attrA": "A", "attrB": "B"}}},
+			Changelog{
+				Change{Type: "delete", Path: []string{"details", "secondary-attributes"}, From: nil, To: map[string]interface{}{"attrA": "A", "attrB": "B"}},
+			},
+			nil,
+		},
+		{
+			"map-nested-update",
+			map[string]interface{}{"firstName": "John", "lastName": "Michael", "createdBy": "TS", "details": map[string]interface{}{"status": "active", "attributes": map[string]interface{}{"attrA": "A", "attrB": "B"}}},
+			map[string]interface{}{"firstName": "John", "lastName": "Michael", "createdBy": "TS", "details": map[string]interface{}{"status": "active", "attributes": map[string]interface{}{"attrA": "C", "attrD": "X"}}},
+			Changelog{
+				Change{Type: "delete", Path: []string{"details", "attributes"}, From: map[string]interface{}{"attrA": "A", "attrB": "B"}, To: map[string]interface{}{"attrA": "C", "attrD": "X"}},
+			},
+			nil,
+		},
+		{
+			"map-nested-delete",
+			map[string]interface{}{"firstName": "John", "lastName": "Michael", "createdBy": "TS", "details": map[string]interface{}{"status": "active", "attributes": map[string]interface{}{"attrA": "A", "attrB": "B"}}},
+			map[string]interface{}{"firstName": "John", "lastName": "Michael", "createdBy": "TS", "details": map[string]interface{}{"status": "active"}},
+			Changelog{
+				Change{Type: "delete", Path: []string{"details", "attributes"}, From: map[string]interface{}{"attrA": "A", "attrB": "B"}, To: nil},
+			},
+			nil,
+		},
 	}
 
 	for _, tc := range cases {