Revert "Fix bug with removing non leaf items"

Author: jennybuckley

fieldpath/set.go

@@ -159,10 +159,7 @@ func (s *Set) iteratePrefix(prefix Path, f func(Path)) {
 func (s *Set) WithPrefix(pe PathElement) *Set {
 	subset, ok := s.Children.Get(pe)
 	if !ok {
-		subset = NewSet()
-	}
-	if s.Members.Has(pe) {
-		subset.Insert(MakePathOrDie(""))
+		return NewSet()
 	}
 	return subset
 }

merge/update.go

@@ -176,5 +176,5 @@ func (s *Updater) removeDisownedItems(merged, applied typed.TypedValue, lastSet
 	if err != nil {
 		return nil, fmt.Errorf("failed to create field set from applied config in last applied version: %v", err)
 	}
-	return merged.RemoveItems(lastSet.Set, appliedSet), nil
+	return merged.RemoveItems(lastSet.Set.Difference(appliedSet)), nil
 }

typed/deduced.go

@@ -173,6 +173,6 @@ func modified(lhs, rhs value.Value, path fieldpath.Path, set *fieldpath.Set) {
 
 // RemoveItems does nothing because all lists in a deducedTypedValue are considered atomic,
 // and there are no maps because it is indistinguishable from a struct.
-func (dv deducedTypedValue) RemoveItems(_ *fieldpath.Set, _ *fieldpath.Set) TypedValue {
+func (dv deducedTypedValue) RemoveItems(_ *fieldpath.Set) TypedValue {
 	return dv
 }

typed/remove.go

@@ -20,18 +20,16 @@ import (
 )
 
 type removingWalker struct {
-	value  *value.Value
-	schema *schema.Schema
-	lhs    *fieldpath.Set
-	rhs    *fieldpath.Set
+	value    *value.Value
+	schema   *schema.Schema
+	toRemove *fieldpath.Set
 }
 
-func removeItemsWithSchema(value *value.Value, lhs *fieldpath.Set, rhs *fieldpath.Set, schema *schema.Schema, typeRef schema.TypeRef) {
+func removeItemsWithSchema(value *value.Value, toRemove *fieldpath.Set, schema *schema.Schema, typeRef schema.TypeRef) {
 	w := &removingWalker{
-		value:  value,
-		schema: schema,
-		lhs:    lhs,
-		rhs:    rhs,
+		value:    value,
+		schema:   schema,
+		toRemove: toRemove,
 	}
 	resolveSchema(schema, typeRef, w)
 }
@@ -58,10 +56,8 @@ func (w *removingWalker) doStruct(t schema.Struct) ValidationErrors {
 	for i, _ := range s.Items {
 		item := s.Items[i]
 		pe := fieldpath.PathElement{FieldName: &item.Name}
-		subsetLHS := w.lhs.WithPrefix(pe)
-		subsetRHS := w.rhs.WithPrefix(pe)
-		if !subsetLHS.Empty() {
-			removeItemsWithSchema(&s.Items[i].Value, subsetLHS, subsetRHS, w.schema, fieldTypes[item.Name])
+		if subset := w.toRemove.WithPrefix(pe); !subset.Empty() {
+			removeItemsWithSchema(&s.Items[i].Value, subset, w.schema, fieldTypes[item.Name])
 		}
 	}
 	return nil
@@ -80,13 +76,12 @@ func (w *removingWalker) doList(t schema.List) (errs ValidationErrors) {
 		item := l.Items[i]
 		// Ignore error because we have already validated this list
 		pe, _ := listItemToPathElement(t, i, item)
-		subsetLHS := w.lhs.WithPrefix(pe)
-		subsetRHS := w.rhs.WithPrefix(pe)
-		if !subsetLHS.Empty() {
-			if subsetRHS.Empty() {
-				continue
-			}
-			removeItemsWithSchema(&l.Items[i], subsetLHS, subsetRHS, w.schema, t.ElementType)
+		path, _ := fieldpath.MakePath(pe)
+		if w.toRemove.Has(path) {
+			continue
+		}
+		if subset := w.toRemove.WithPrefix(pe); !subset.Empty() {
+			removeItemsWithSchema(&l.Items[i], subset, w.schema, t.ElementType)
 		}
 		newItems = append(newItems, l.Items[i])
 	}
@@ -106,13 +101,12 @@ func (w *removingWalker) doMap(t schema.Map) ValidationErrors {
 	for i, _ := range m.Items {
 		item := m.Items[i]
 		pe := fieldpath.PathElement{FieldName: &item.Name}
-		subsetLHS := w.lhs.WithPrefix(pe)
-		subsetRHS := w.rhs.WithPrefix(pe)
-		if !subsetLHS.Empty() {
-			if subsetRHS.Empty() {
-				continue
-			}
-			removeItemsWithSchema(&m.Items[i].Value, subsetLHS, subsetRHS, w.schema, t.ElementType)
+		path, _ := fieldpath.MakePath(pe)
+		if w.toRemove.Has(path) {
+			continue
+		}
+		if subset := w.toRemove.WithPrefix(pe); !subset.Empty() {
+			removeItemsWithSchema(&m.Items[i].Value, subset, w.schema, t.ElementType)
 		}
 		newMap.Set(item.Name, m.Items[i].Value)
 	}

typed/typed.go

@@ -53,8 +53,8 @@ type TypedValue interface {
 	// match), or an error will be returned. Validation errors will be returned if
 	// the objects don't conform to the schema.
 	Compare(rhs TypedValue) (c *Comparison, err error)
-	// RemoveItems removes each list or map item from the value that has children in lhs and not in rhs.
-	RemoveItems(lhs *fieldpath.Set, rhs *fieldpath.Set) TypedValue
+	// RemoveItems removes each provided list or map item from the value.
+	RemoveItems(items *fieldpath.Set) TypedValue
 }
 
 // AsTyped accepts a value and a type and returns a TypedValue. 'v' must have
@@ -161,9 +161,9 @@ func (tv typedValue) Compare(rhs TypedValue) (c *Comparison, err error) {
 	return c, nil
 }
 
-// RemoveItems removes each list or map item from the value that has children in lhs and not in rhs.
-func (tv typedValue) RemoveItems(lhs *fieldpath.Set, rhs *fieldpath.Set) TypedValue {
-	removeItemsWithSchema(&tv.value, lhs, rhs, tv.schema, tv.typeRef)
+// RemoveItems removes each provided list or map item from the value.
+func (tv typedValue) RemoveItems(items *fieldpath.Set) TypedValue {
+	removeItemsWithSchema(&tv.value, items, tv.schema, tv.typeRef)
 	return tv
 }