Merge pull request #129 from apelisse/perf/new-value-rebased

Perf/new value rebased

Author: k8s-ci-robot

fieldpath/element.go

@@ -34,7 +34,7 @@ type PathElement struct {
 
 	// Key selects the list element which has fields matching those given.
 	// The containing object must be an associative list with map typed
-	// elements.
+	// elements. They are sorted alphabetically.
 	Key *value.FieldList
 
 	// Value selects the list element with the given value. The containing
@@ -71,7 +71,7 @@ func (e PathElement) Less(rhs PathElement) bool {
 		if rhs.Value == nil {
 			return true
 		}
-		return e.Value.Less(*rhs.Value)
+		return value.Less(*e.Value, *rhs.Value)
 	} else if rhs.Value != nil {
 		return false
 	}
@@ -103,12 +103,12 @@ func (e PathElement) String() string {
 	case e.Key != nil:
 		strs := make([]string, len(*e.Key))
 		for i, k := range *e.Key {
-			strs[i] = fmt.Sprintf("%v=%v", k.Name, k.Value)
+			strs[i] = fmt.Sprintf("%v=%v", k.Name, value.ToString(k.Value))
 		}
 		// Keys are supposed to be sorted.
 		return "[" + strings.Join(strs, ",") + "]"
 	case e.Value != nil:
-		return fmt.Sprintf("[=%v]", e.Value)
+		return fmt.Sprintf("[=%v]", value.ToString(*e.Value))
 	case e.Index != nil:
 		return fmt.Sprintf("[%v]", *e.Index)
 	default:
@@ -127,10 +127,7 @@ func KeyByFields(nameValues ...interface{}) *value.FieldList {
 	}
 	out := value.FieldList{}
 	for i := 0; i < len(nameValues)-1; i += 2 {
-		out = append(out, value.Field{
-			Name:  nameValues[i].(string),
-			Value: nameValues[i+1].(value.Value),
-		})
+		out = append(out, value.Field{Name: nameValues[i].(string), Value: value.NewValueInterface(nameValues[i+1])})
 	}
 	out.Sort()
 	return &out

fieldpath/element_test.go

@@ -62,9 +62,12 @@ func TestPathElementSet(t *testing.T) {
 	})
 }
 
-func strptr(s string) *string   { return &s }
-func intptr(i int) *int         { return &i }
-func valptr(i int) *value.Value { v := value.IntValue(i); return &v }
+func strptr(s string) *string { return &s }
+func intptr(i int) *int       { return &i }
+func valptr(i interface{}) *value.Value {
+	v := value.NewValueInterface(i)
+	return &v
+}
 
 func TestPathElementLess(t *testing.T) {
 	table := []struct {
@@ -91,7 +94,7 @@ func TestPathElementLess(t *testing.T) {
 		}, {
 			name: "FieldName-3",
 			a:    PathElement{FieldName: strptr("capybara")},
-			b:    PathElement{Key: KeyByFields("dog", value.IntValue(3))},
+			b:    PathElement{Key: KeyByFields("dog", 3)},
 		}, {
 			name: "FieldName-4",
 			a:    PathElement{FieldName: strptr("elephant")},
@@ -102,28 +105,28 @@ func TestPathElementLess(t *testing.T) {
 			b:    PathElement{Index: intptr(5)},
 		}, {
 			name: "Key-1",
-			a:    PathElement{Key: KeyByFields("goat", value.IntValue(1))},
-			b:    PathElement{Key: KeyByFields("goat", value.IntValue(1))},
+			a:    PathElement{Key: KeyByFields("goat", 1)},
+			b:    PathElement{Key: KeyByFields("goat", 1)},
 			eq:   true,
 		}, {
 			name: "Key-2",
-			a:    PathElement{Key: KeyByFields("horse", value.IntValue(1))},
-			b:    PathElement{Key: KeyByFields("horse", value.IntValue(2))},
+			a:    PathElement{Key: KeyByFields("horse", 1)},
+			b:    PathElement{Key: KeyByFields("horse", 2)},
 		}, {
 			name: "Key-3",
-			a:    PathElement{Key: KeyByFields("ibex", value.IntValue(1))},
-			b:    PathElement{Key: KeyByFields("jay", value.IntValue(1))},
+			a:    PathElement{Key: KeyByFields("ibex", 1)},
+			b:    PathElement{Key: KeyByFields("jay", 1)},
 		}, {
 			name: "Key-4",
-			a:    PathElement{Key: KeyByFields("kite", value.IntValue(1))},
-			b:    PathElement{Key: KeyByFields("kite", value.IntValue(1), "kite-2", value.IntValue(1))},
+			a:    PathElement{Key: KeyByFields("kite", 1)},
+			b:    PathElement{Key: KeyByFields("kite", 1, "kite-2", 1)},
 		}, {
 			name: "Key-5",
-			a:    PathElement{Key: KeyByFields("kite", value.IntValue(1))},
+			a:    PathElement{Key: KeyByFields("kite", 1)},
 			b:    PathElement{Value: valptr(1)},
 		}, {
 			name: "Key-6",
-			a:    PathElement{Key: KeyByFields("kite", value.IntValue(1))},
+			a:    PathElement{Key: KeyByFields("kite", 1)},
 			b:    PathElement{Index: intptr(5)},
 		}, {
 			name: "Value-1",
@@ -156,15 +159,15 @@ func TestPathElementLess(t *testing.T) {
 			tt := table[i]
 			if tt.eq {
 				if tt.a.Less(tt.b) {
-					t.Errorf("oops, a < b: %#v, %#v", tt.a, tt.b)
+					t.Errorf("oops, a < b: %#v (%v), %#v (%v)", tt.a, tt.a, tt.b, tt.b)
 				}
 			} else {
 				if !tt.a.Less(tt.b) {
-					t.Errorf("oops, a >= b: %#v, %#v", tt.a, tt.b)
+					t.Errorf("oops, a >= b: %#v (%v), %#v (%v)", tt.a, tt.a, tt.b, tt.b)
 				}
 			}
 			if tt.b.Less(tt.b) {
-				t.Errorf("oops, b < a: %#v, %#v", tt.b, tt.a)
+				t.Errorf("oops, b < a: %#v (%v), %#v (%v)", tt.b, tt.b, tt.a, tt.a)
 			}
 		})
 	}

fieldpath/fromvalue.go

@@ -43,37 +43,37 @@ type objectWalker struct {
 
 func (w *objectWalker) walk() {
 	switch {
-	case w.value.Null:
-	case w.value.FloatValue != nil:
-	case w.value.IntValue != nil:
-	case w.value.StringValue != nil:
-	case w.value.BooleanValue != nil:
+	case w.value.IsNull():
+	case w.value.IsFloat():
+	case w.value.IsInt():
+	case w.value.IsString():
+	case w.value.IsBool():
 		// All leaf fields handled the same way (after the switch
 		// statement).
 
 	// Descend
-	case w.value.ListValue != nil:
+	case w.value.IsList():
 		// If the list were atomic, we'd break here, but we don't have
 		// a schema, so we can't tell.
-
-		for i, child := range w.value.ListValue.Items {
+		list := w.value.List()
+		for i := 0; i < list.Length(); i++ {
 			w2 := *w
-			w2.path = append(w.path, GuessBestListPathElement(i, child))
-			w2.value = child
+			w2.path = append(w.path, GuessBestListPathElement(i, list.At(i)))
+			w2.value = list.At(i)
 			w2.walk()
 		}
 		return
-	case w.value.MapValue != nil:
+	case w.value.IsMap():
 		// If the map/struct were atomic, we'd break here, but we don't
 		// have a schema, so we can't tell.
 
-		for i := range w.value.MapValue.Items {
-			child := w.value.MapValue.Items[i]
+		w.value.Map().Iterate(func(k string, val value.Value) bool {
 			w2 := *w
-			w2.path = append(w.path, PathElement{FieldName: &child.Name})
-			w2.value = child.Value
+			w2.path = append(w.path, PathElement{FieldName: &k})
+			w2.value = val
 			w2.walk()
-		}
+			return true
+		})
 		return
 	}
 
@@ -97,7 +97,7 @@ var AssociativeListCandidateFieldNames = []string{
 // whether item has any of the fields listed in
 // AssociativeListCandidateFieldNames which have scalar values.
 func GuessBestListPathElement(index int, item value.Value) PathElement {
-	if item.MapValue == nil {
+	if !item.IsMap() {
 		// Non map items could be parts of sets or regular "atomic"
 		// lists. We won't try to guess whether something should be a
 		// set or not.
@@ -106,15 +106,15 @@ func GuessBestListPathElement(index int, item value.Value) PathElement {
 
 	var keys value.FieldList
 	for _, name := range AssociativeListCandidateFieldNames {
-		f, ok := item.MapValue.Get(name)
+		f, ok := item.Map().Get(name)
 		if !ok {
 			continue
 		}
 		// only accept primitive/scalar types as keys.
-		if f.Value.Null || f.Value.MapValue != nil || f.Value.ListValue != nil {
+		if f.IsNull() || f.IsMap() || f.IsList() {
 			continue
 		}
-		keys = append(keys, *f)
+		keys = append(keys, value.Field{Name: name, Value: f})
 	}
 	if len(keys) > 0 {
 		keys.Sort()

fieldpath/fromvalue_test.go

@@ -19,6 +19,7 @@ package fieldpath
 import (
 	"testing"
 
+	"gopkg.in/yaml.v2"
 	"sigs.k8s.io/structured-merge-diff/value"
 )
 
@@ -31,19 +32,19 @@ func TestFromValue(t *testing.T) {
 		{`{"a": [{"a": null}]}`, NewSet(
 			MakePathOrDie("a", 0, "a"),
 		)}, {`{"a": [{"id": a}]}`, NewSet(
-			MakePathOrDie("a", KeyByFields("id", value.StringValue("a")), "id"),
+			MakePathOrDie("a", KeyByFields("id", "a"), "id"),
 		)}, {`{"a": [{"name": a}]}`, NewSet(
-			MakePathOrDie("a", KeyByFields("name", value.StringValue("a")), "name"),
+			MakePathOrDie("a", KeyByFields("name", "a"), "name"),
 		)}, {`{"a": [{"key": a}]}`, NewSet(
-			MakePathOrDie("a", KeyByFields("key", value.StringValue("a")), "key"),
+			MakePathOrDie("a", KeyByFields("key", "a"), "key"),
 		)}, {`{"a": [{"name": "a", "key": "b"}]}`, NewSet(
 			MakePathOrDie("a", KeyByFields(
-				"key", value.StringValue("b"),
-				"name", value.StringValue("a"),
+				"key", "b",
+				"name", "a",
 			), "key"),
 			MakePathOrDie("a", KeyByFields(
-				"key", value.StringValue("b"),
-				"name", value.StringValue("a"),
+				"key", "b",
+				"name", "a",
 			), "name"),
 		)}, {`{"a": [5]}`, NewSet(
 			MakePathOrDie("a", 0),
@@ -65,11 +66,12 @@ func TestFromValue(t *testing.T) {
 		tt := tt
 		t.Run(tt.objYAML, func(t *testing.T) {
 			t.Parallel()
-			v, err := value.FromYAML([]byte(tt.objYAML))
+			var v interface{}
+			err := yaml.Unmarshal([]byte(tt.objYAML), &v)
 			if err != nil {
 				t.Fatalf("couldn't parse: %v", err)
 			}
-			got := SetFromValue(v)
+			got := SetFromValue(value.NewValueInterface(v))
 			if !got.Equals(tt.set) {
 				t.Errorf("wanted\n%s\nbut got\n%s\n", tt.set, got)
 			}

fieldpath/managers.go

@@ -13,6 +13,11 @@ limitations under the License.
 
 package fieldpath
 
+import (
+	"fmt"
+	"strings"
+)
+
 // APIVersion describes the version of an object or of a fieldset.
 type APIVersion string
 
@@ -95,3 +100,14 @@ func (lhs ManagedFields) Difference(rhs ManagedFields) ManagedFields {
 
 	return diff
 }
+
+func (lhs ManagedFields) String() string {
+	s := strings.Builder{}
+	for k, v := range lhs {
+		fmt.Fprintf(&s, "%s:\n", k)
+		fmt.Fprintf(&s, "- Applied: %v\n", v.Applied())
+		fmt.Fprintf(&s, "- APIVersion: %v\n", v.APIVersion())
+		fmt.Fprintf(&s, "- Set: %v\n", v.Set())
+	}
+	return s.String()
+}

fieldpath/path_test.go

@@ -22,6 +22,10 @@ import (
 	"sigs.k8s.io/structured-merge-diff/value"
 )
 
+var (
+	_V = value.NewValueInterface
+)
+
 func TestPathString(t *testing.T) {
 	table := []struct {
 		name   string
@@ -31,16 +35,20 @@ func TestPathString(t *testing.T) {
 		{"basic1", MakePathOrDie("foo", 1), ".foo[1]"},
 		{"basic2", MakePathOrDie("foo", "bar", 1, "baz"), ".foo.bar[1].baz"},
 		{"associative-list-ref", MakePathOrDie("foo", KeyByFields(
-			"a", value.StringValue("b"),
-			"c", value.IntValue(1),
-			"d", value.FloatValue(1.5),
-			"e", value.BooleanValue(true),
+			// This makes sure we test all types: string,
+			// floats, integers and booleans.
+			"a", "b",
+			"c", 1,
+			"d", 1.5,
+			"e", true,
 		)), `.foo[a="b",c=1,d=1.5,e=true]`},
 		{"sets", MakePathOrDie("foo",
-			value.StringValue("b"),
-			value.IntValue(5),
-			value.BooleanValue(false),
-			value.FloatValue(3.14159),
+			// This makes sure we test all types: string,
+			// floats, integers and booleans.
+			_V("b"),
+			_V(5),
+			_V(false),
+			_V(3.14159),
 		), `.foo[="b"][=5][=false][=3.14159]`},
 	}
 	for _, tt := range table {

fieldpath/pathelementmap.go

@@ -76,10 +76,10 @@ func (s *PathElementValueMap) Get(pe PathElement) (value.Value, bool) {
 		return !s.members[i].PathElement.Less(pe)
 	})
 	if loc == len(s.members) {
-		return value.Value{}, false
+		return nil, false
 	}
 	if s.members[loc].PathElement.Equals(pe) {
 		return s.members[loc].Value, true
 	}
-	return value.Value{}, false
+	return nil, false
 }

fieldpath/pathelementmap_test.go

@@ -29,22 +29,22 @@ func TestPathElementValueMap(t *testing.T) {
 		t.Fatal("Unexpected path-element found in empty map")
 	}
 
-	m.Insert(PathElement{FieldName: strptr("carrot")}, value.StringValue("knife"))
-	m.Insert(PathElement{FieldName: strptr("chive")}, value.IntValue(2))
+	m.Insert(PathElement{FieldName: strptr("carrot")}, value.NewValueInterface("knife"))
+	m.Insert(PathElement{FieldName: strptr("chive")}, value.NewValueInterface(2))
 
 	if _, ok := m.Get(PathElement{FieldName: strptr("onion")}); ok {
 		t.Fatal("Unexpected path-element in map")
 	}
 
 	if val, ok := m.Get(PathElement{FieldName: strptr("carrot")}); !ok {
 		t.Fatal("Missing path-element in map")
-	} else if !val.Equals(value.StringValue("knife")) {
+	} else if !value.Equals(val, value.NewValueInterface("knife")) {
 		t.Fatalf("Unexpected value found: %#v", val)
 	}
 
 	if val, ok := m.Get(PathElement{FieldName: strptr("chive")}); !ok {
 		t.Fatal("Missing path-element in map")
-	} else if !val.Equals(value.IntValue(2)) {
+	} else if !value.Equals(val, value.NewValueInterface(2)) {
 		t.Fatalf("Unexpected value found: %#v", val)
 	}
 }

fieldpath/serialize-pe.go

@@ -84,6 +84,7 @@ func DeserializePathElement(s string) (PathElement, error) {
 		iter := readPool.BorrowIterator(b)
 		defer readPool.ReturnIterator(iter)
 		fields := value.FieldList{}
+
 		iter.ReadObjectCB(func(iter *jsoniter.Iterator, key string) bool {
 			v, err := value.ReadJSONIter(iter)
 			if err != nil {
@@ -134,19 +135,20 @@ func serializePathElementToWriter(w io.Writer, pe PathElement) error {
 			return err
 		}
 		stream.WriteObjectStart()
+
 		for i, field := range *pe.Key {
 			if i > 0 {
 				stream.WriteMore()
 			}
 			stream.WriteObjectField(field.Name)
-			field.Value.WriteJSONStream(stream)
+			value.WriteJSONStream(field.Value, stream)
 		}
 		stream.WriteObjectEnd()
 	case pe.Value != nil:
 		if _, err := stream.Write(peValueSepBytes); err != nil {
 			return err
 		}
-		pe.Value.WriteJSONStream(stream)
+		value.WriteJSONStream(*pe.Value, stream)
 	case pe.Index != nil:
 		if _, err := stream.Write(peIndexSepBytes); err != nil {
 			return err