Apply feedback

Author: jpbetz

internal/fixture/state.go

@@ -100,7 +100,7 @@ func (s *State) UpdateObject(tv *typed.TypedValue, version fieldpath.APIVersion,
 	return nil
 }
 
-// Set the current state with the passed in object
+// Update the current state with the passed in object
 func (s *State) Update(obj typed.YAMLObject, version fieldpath.APIVersion, manager string) error {
 	tv, err := s.Parser.FromYAML(FixTabsOrDie(obj))
 	if err != nil {
@@ -308,7 +308,7 @@ func (f ForceApplyObject) preprocess(parser typed.ParseableType) (Operation, err
 	return f, nil
 }
 
-// Set is a type of operation. It is a controller type of
+// Update is a type of operation. It is a controller type of
 // update. Errors are passed along.
 type Update struct {
 	Manager    string

merge/real_test.go

@@ -101,7 +101,7 @@ func BenchmarkOperations(b *testing.B) {
 					},
 				},
 				{
-					name: "Set",
+					name: "Update",
 					ops: []Operation{
 						Update{
 							Manager:    "controller",

merge/update.go

@@ -114,7 +114,7 @@ func (s *Updater) update(oldObject, newObject *typed.TypedValue, version fieldpa
 	return managers, compare, nil
 }
 
-// Set is the method you should call once you've merged your final
+// Update is the method you should call once you've merged your final
 // object on CREATE/UPDATE/PATCH verbs. newObject must be the object
 // that you intend to persist (after applying the patch if this is for a
 // PATCH call), and liveObject must be the original object (empty if

typed/parser.go

@@ -114,8 +114,9 @@ func (p ParseableType) FromUnstructured(in interface{}) (*TypedValue, error) {
 
 // FromStructured converts a go "interface{}" type, typically an structured object in
 // Kubernetes, to a TypedValue. It will return an error if the resulting object fails
-// schema validation. The provided "interface{}" may contain structs and types that are
-// converted to Values by the jsonMarshaler interface.
+// schema validation. The provided "interface{}" value must be a pointer so that the
+// value can be modified via reflection. The provided "interface{}" may contain structs
+// and types that are converted to Values by the jsonMarshaler interface.
 func (p ParseableType) FromStructured(in interface{}) (*TypedValue, error) {
 	v, err := value.NewValueReflect(in)
 	if err != nil {

typed/union.go

@@ -265,7 +265,7 @@ func (u *union) NormalizeApply(applied, merged, out value.Map) error {
 		return fmt.Errorf("applied discriminator (%v) doesn't match applied field (%v)", u.d.Get(applied), *as.One())
 	}
 
-	// Set discriminiator if needed
+	// Update discriminiator if needed
 	if u.deduceInvalidDiscriminator {
 		u.d.Set(out, u.dn.toDiscriminated(*as.One()))
 	}

value/fields.go

@@ -27,7 +27,7 @@ type Field struct {
 	Value Value
 }
 
-// FieldList is a list of key-value pairs. Each field is expected to
+// FieldList is a list of key-value pairs. Each field is expectUpdated to
 // have a different name.
 type FieldList []Field
 

value/structreflect.go

@@ -85,13 +85,13 @@ type fieldCacheEntry struct {
 	isOmitEmpty bool
 	// fieldPath is the field indices (see FieldByIndex) to lookup the value of
 	// a field in a reflect.Value struct. A path of field indices is used
-	// to support traversing to a field nested in struct fields that have the 'inline'
+	// to support traversing to a field field in struct fields that have the 'inline'
 	// json tag.
 	fieldPath [][]int
 }
 
 func (f *fieldCacheEntry) getFieldFromStruct(structVal reflect.Value) reflect.Value {
-	// field might be nested within 'inline' structs
+	// field might be field within 'inline' structs
 	for _, elem := range f.fieldPath {
 		structVal = structVal.FieldByIndex(elem)
 	}

value/valuereflect_test.go

@@ -299,72 +299,133 @@ func TestReflectStructMutate(t *testing.T) {
 	}
 }
 
-// TestReflectMutateNestedStruct ensures a structs nested within various typed can be modified.
+// TestReflectMutateNestedStruct ensures a structs field within various typed can be modified.
 func TestReflectMutateNestedStruct(t *testing.T) {
-	type nested struct {
+	type field struct {
 		S string `json:"s,omitempty"`
 	}
-	type root struct {
-		Field      nested                       `json:"field"`
-		List       []nested                     `json:"list"`
-		Map        map[string]nested            `json:"map"`
-		MapOfMaps  map[string]map[string]nested `json:"mapOfMaps"`
-		MapOfLists map[string][]nested          `json:"mapOfLists"`
-	}
-	rv := MustReflect(&root{
-		Field:      nested{S: "field"},
-		List:       []nested{{S: "listItem"}},
-		Map:        map[string]nested{"mapKey": {S: "mapItem"}},
-		MapOfMaps:  map[string]map[string]nested{"outer": {"inner": {S: "mapOfMapItem"}}},
-		MapOfLists: map[string][]nested{"outer": {{S: "mapOfListsItem"}}},
-	})
-	rootMap := rv.AsMap()
-	field, _ := rootMap.Get("field")
-	list, _ := rootMap.Get("list")
-	listItem := list.AsList().At(0)
-	m, _ := rootMap.Get("map")
-	mapItem, _ := m.AsMap().Get("mapKey")
-	mapOfMaps, _ := rootMap.Get("mapOfMaps")
-	innerMap, _ := mapOfMaps.AsMap().Get("outer")
-	mapOfMapsItem, _ := innerMap.AsMap().Get("inner")
-	mapOfLists, _ := rootMap.Get("mapOfLists")
-	innerList, _ := mapOfLists.AsMap().Get("outer")
-	mapOfListsItem := innerList.AsList().At(0)
-
-	field.AsMap().Set("s", NewValueInterface("field2"))
-	listItem.AsMap().Set("s", NewValueInterface("listItem2"))
-	mapItem.AsMap().Set("s", NewValueInterface("mapItem2"))
-	mapOfMapsItem.AsMap().Set("s", NewValueInterface("mapOfMapItem2"))
-	mapOfListsItem.AsMap().Set("s", NewValueInterface("mapOfListsItem2"))
 
-	unstructured := rv.Unstructured()
-	expectedMap := map[string]interface{}{
-		"field":      map[string]interface{}{"s": "field2"},
-		"list":       []interface{}{map[string]interface{}{"s": "listItem2"}},
-		"map":        map[string]interface{}{"mapKey": map[string]interface{}{"s": "mapItem2"}},
-		"mapOfMaps":  map[string]interface{}{"outer": map[string]interface{}{"inner": map[string]interface{}{"s": "mapOfMapItem2"}}},
-		"mapOfLists": map[string]interface{}{"outer": []interface{}{map[string]interface{}{"s": "mapOfListsItem2"}}},
-	}
-	if !reflect.DeepEqual(unstructured, expectedMap) {
-		t.Errorf("expected %v but got: %v", expectedMap, unstructured)
+	cases := []struct {
+		fieldName     string
+		root          Value
+		lookupField   func(root Value) Value
+		expectUpdated interface{}
+		expectDeleted interface{}
+	}{
+		{
+			fieldName: "field",
+			root: MustReflect(&struct {
+				Field field `json:"field,omitempty"`
+			}{
+				Field: field{S: "field"},
+			}),
+			lookupField: func(rv Value) Value {
+				field, _ := rv.AsMap().Get("field")
+				return field
+			},
+			expectUpdated: map[string]interface{}{
+				"field": map[string]interface{}{"s": "updatedValue"},
+			},
+			expectDeleted: map[string]interface{}{
+				"field": map[string]interface{}{},
+			},
+		},
+		{
+			fieldName: "map",
+			root: MustReflect(&struct {
+				Map map[string]field `json:"map,omitempty"`
+			}{
+				Map: map[string]field{"mapKey": {S: "mapItem"}},
+			}),
+			lookupField: func(rv Value) Value {
+				m, _ := rv.AsMap().Get("map")
+				mapItem, _ := m.AsMap().Get("mapKey")
+				return mapItem
+			},
+			expectUpdated: map[string]interface{}{
+				"map": map[string]interface{}{"mapKey": map[string]interface{}{"s": "updatedValue"}},
+			},
+			expectDeleted: map[string]interface{}{
+				"map": map[string]interface{}{"mapKey": map[string]interface{}{}},
+			},
+		},
+		{
+			fieldName: "list",
+			root: MustReflect(&struct {
+				List []field `json:"list,omitempty"`
+			}{
+				List: []field{{S: "listItem"}},
+			}),
+			lookupField: func(rv Value) Value {
+				list, _ := rv.AsMap().Get("list")
+				return list.AsList().At(0)
+			},
+			expectUpdated: map[string]interface{}{
+				"list": []interface{}{map[string]interface{}{"s": "updatedValue"}},
+			},
+			expectDeleted: map[string]interface{}{
+				"list": []interface{}{map[string]interface{}{}},
+			},
+		},
+		{
+			fieldName: "mapOfMaps",
+			root: MustReflect(&struct {
+				MapOfMaps map[string]map[string]field `json:"mapOfMaps,omitempty"`
+			}{
+				MapOfMaps: map[string]map[string]field{"outer": {"inner": {S: "mapOfMapItem"}}},
+			}),
+			lookupField: func(rv Value) Value {
+				mapOfMaps, _ := rv.AsMap().Get("mapOfMaps")
+				innerMap, _ := mapOfMaps.AsMap().Get("outer")
+				mapOfMapsItem, _ := innerMap.AsMap().Get("inner")
+				return mapOfMapsItem
+			},
+			expectUpdated: map[string]interface{}{
+				"mapOfMaps": map[string]interface{}{"outer": map[string]interface{}{"inner": map[string]interface{}{"s": "updatedValue"}}},
+			},
+			expectDeleted: map[string]interface{}{
+				"mapOfMaps": map[string]interface{}{"outer": map[string]interface{}{"inner": map[string]interface{}{}}},
+			},
+		},
+		{
+			fieldName: "mapOfLists",
+			root: MustReflect(&struct {
+				MapOfLists map[string][]field `json:"mapOfLists,omitempty"`
+			}{
+				MapOfLists: map[string][]field{"outer": {{S: "mapOfListsItem"}}},
+			}),
+			lookupField: func(rv Value) Value {
+				mapOfLists, _ := rv.AsMap().Get("mapOfLists")
+				innerList, _ := mapOfLists.AsMap().Get("outer")
+				mapOfListsItem := innerList.AsList().At(0)
+				return mapOfListsItem
+			},
+
+			expectUpdated: map[string]interface{}{
+				"mapOfLists": map[string]interface{}{"outer": []interface{}{map[string]interface{}{"s": "updatedValue"}}},
+			},
+			expectDeleted: map[string]interface{}{
+				"mapOfLists": map[string]interface{}{"outer": []interface{}{map[string]interface{}{}}},
+			},
+		},
 	}
 
-	field.AsMap().Delete("s")
-	listItem.AsMap().Delete("s")
-	mapItem.AsMap().Delete("s")
-	mapOfMapsItem.AsMap().Delete("s")
-	mapOfListsItem.AsMap().Delete("s")
+	for _, tc := range cases {
+		t.Run(tc.fieldName, func(t *testing.T) {
+			root := tc.root
+			field := tc.lookupField(root)
+			field.AsMap().Set("s", NewValueInterface("updatedValue"))
+			unstructured := root.Unstructured()
+			if !reflect.DeepEqual(unstructured, tc.expectUpdated) {
+				t.Errorf("expected %v but got: %v", tc.expectUpdated, unstructured)
+			}
 
-	unstructured = rv.Unstructured()
-	expectedMap = map[string]interface{}{
-		"field":      map[string]interface{}{},
-		"list":       []interface{}{map[string]interface{}{}},
-		"map":        map[string]interface{}{"mapKey": map[string]interface{}{}},
-		"mapOfMaps":  map[string]interface{}{"outer": map[string]interface{}{"inner": map[string]interface{}{}}},
-		"mapOfLists": map[string]interface{}{"outer": []interface{}{map[string]interface{}{}}},
-	}
-	if !reflect.DeepEqual(unstructured, expectedMap) {
-		t.Errorf("expected %v but got: %v", expectedMap, unstructured)
+			field.AsMap().Delete("s")
+			unstructured = root.Unstructured()
+			if !reflect.DeepEqual(unstructured, tc.expectDeleted) {
+				t.Errorf("expected %v but got: %v", tc.expectDeleted, unstructured)
+			}
+		})
 	}
 }