Skip to content

Commit 990c697

Browse files
author
Antoine Pelisse
committed
Refactor value package to not copy everything
Value is now just a view of the generic interface, so that we don't have to allocate big objects and can directly work on the raw data.
1 parent fda0dd5 commit 990c697

38 files changed

+937
-1564
lines changed

‎fieldpath/element.go

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ type PathElement struct {
3434

3535
// Key selects the list element which has fields matching those given.
3636
// The containing object must be an associative list with map typed
37-
// elements.
37+
// elements. They are sorted alphabetically.
3838
Key *value.FieldList
3939

4040
// Value selects the list element with the given value. The containing
@@ -71,7 +71,7 @@ func (e PathElement) Less(rhs PathElement) bool {
7171
if rhs.Value == nil {
7272
return true
7373
}
74-
return e.Value.Less(*rhs.Value)
74+
return value.Less(*e.Value, *rhs.Value)
7575
} else if rhs.Value != nil {
7676
return false
7777
}
@@ -103,12 +103,12 @@ func (e PathElement) String() string {
103103
case e.Key != nil:
104104
strs := make([]string, len(*e.Key))
105105
for i, k := range *e.Key {
106-
strs[i] = fmt.Sprintf("%v=%v", k.Name, k.Value)
106+
strs[i] = fmt.Sprintf("%v=%v", k.Name, value.ToString(k.Value))
107107
}
108108
// Keys are supposed to be sorted.
109109
return "[" + strings.Join(strs, ",") + "]"
110110
case e.Value != nil:
111-
return fmt.Sprintf("[=%v]", e.Value)
111+
return fmt.Sprintf("[=%v]", value.ToString(*e.Value))
112112
case e.Index != nil:
113113
return fmt.Sprintf("[%v]", *e.Index)
114114
default:
@@ -127,10 +127,7 @@ func KeyByFields(nameValues ...interface{}) *value.FieldList {
127127
}
128128
out := value.FieldList{}
129129
for i := 0; i < len(nameValues)-1; i += 2 {
130-
out = append(out, value.Field{
131-
Name: nameValues[i].(string),
132-
Value: nameValues[i+1].(value.Value),
133-
})
130+
out = append(out, value.Field{Name: nameValues[i].(string), Value: nameValues[i+1]})
134131
}
135132
out.Sort()
136133
return &out

‎fieldpath/element_test.go

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -62,9 +62,9 @@ func TestPathElementSet(t *testing.T) {
6262
})
6363
}
6464

65-
func strptr(s string) *string { return &s }
66-
func intptr(i int) *int { return &i }
67-
func valptr(i int) *value.Value { v := value.IntValue(i); return &v }
65+
func strptr(s string) *string { return &s }
66+
func intptr(i int) *int { return &i }
67+
func valptr(i value.Value) *value.Value { return &i }
6868

6969
func TestPathElementLess(t *testing.T) {
7070
table := []struct {
@@ -91,7 +91,7 @@ func TestPathElementLess(t *testing.T) {
9191
}, {
9292
name: "FieldName-3",
9393
a: PathElement{FieldName: strptr("capybara")},
94-
b: PathElement{Key: KeyByFields("dog", value.IntValue(3))},
94+
b: PathElement{Key: KeyByFields("dog", 3)},
9595
}, {
9696
name: "FieldName-4",
9797
a: PathElement{FieldName: strptr("elephant")},
@@ -102,28 +102,28 @@ func TestPathElementLess(t *testing.T) {
102102
b: PathElement{Index: intptr(5)},
103103
}, {
104104
name: "Key-1",
105-
a: PathElement{Key: KeyByFields("goat", value.IntValue(1))},
106-
b: PathElement{Key: KeyByFields("goat", value.IntValue(1))},
105+
a: PathElement{Key: KeyByFields("goat", 1)},
106+
b: PathElement{Key: KeyByFields("goat", 1)},
107107
eq: true,
108108
}, {
109109
name: "Key-2",
110-
a: PathElement{Key: KeyByFields("horse", value.IntValue(1))},
111-
b: PathElement{Key: KeyByFields("horse", value.IntValue(2))},
110+
a: PathElement{Key: KeyByFields("horse", 1)},
111+
b: PathElement{Key: KeyByFields("horse", 2)},
112112
}, {
113113
name: "Key-3",
114-
a: PathElement{Key: KeyByFields("ibex", value.IntValue(1))},
115-
b: PathElement{Key: KeyByFields("jay", value.IntValue(1))},
114+
a: PathElement{Key: KeyByFields("ibex", 1)},
115+
b: PathElement{Key: KeyByFields("jay", 1)},
116116
}, {
117117
name: "Key-4",
118-
a: PathElement{Key: KeyByFields("kite", value.IntValue(1))},
119-
b: PathElement{Key: KeyByFields("kite", value.IntValue(1), "kite-2", value.IntValue(1))},
118+
a: PathElement{Key: KeyByFields("kite", 1)},
119+
b: PathElement{Key: KeyByFields("kite", 1, "kite-2", 1)},
120120
}, {
121121
name: "Key-5",
122-
a: PathElement{Key: KeyByFields("kite", value.IntValue(1))},
122+
a: PathElement{Key: KeyByFields("kite", 1)},
123123
b: PathElement{Value: valptr(1)},
124124
}, {
125125
name: "Key-6",
126-
a: PathElement{Key: KeyByFields("kite", value.IntValue(1))},
126+
a: PathElement{Key: KeyByFields("kite", 1)},
127127
b: PathElement{Index: intptr(5)},
128128
}, {
129129
name: "Value-1",
@@ -156,15 +156,15 @@ func TestPathElementLess(t *testing.T) {
156156
tt := table[i]
157157
if tt.eq {
158158
if tt.a.Less(tt.b) {
159-
t.Errorf("oops, a < b: %#v, %#v", tt.a, tt.b)
159+
t.Errorf("oops, a < b: %#v (%v), %#v (%v)", tt.a, tt.a, tt.b, tt.b)
160160
}
161161
} else {
162162
if !tt.a.Less(tt.b) {
163-
t.Errorf("oops, a >= b: %#v, %#v", tt.a, tt.b)
163+
t.Errorf("oops, a >= b: %#v (%v), %#v (%v)", tt.a, tt.a, tt.b, tt.b)
164164
}
165165
}
166166
if tt.b.Less(tt.b) {
167-
t.Errorf("oops, b < a: %#v, %#v", tt.b, tt.a)
167+
t.Errorf("oops, b < a: %#v (%v), %#v (%v)", tt.b, tt.b, tt.a, tt.a)
168168
}
169169
})
170170
}

‎fieldpath/fromvalue.go

Lines changed: 15 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -43,35 +43,34 @@ type objectWalker struct {
4343

4444
func (w *objectWalker) walk() {
4545
switch {
46-
case w.value.Null:
47-
case w.value.FloatValue != nil:
48-
case w.value.IntValue != nil:
49-
case w.value.StringValue != nil:
50-
case w.value.BooleanValue != nil:
46+
case value.IsFloat(w.value):
47+
case value.IsInt(w.value):
48+
case value.IsString(w.value):
49+
case value.IsBool(w.value):
5150
// All leaf fields handled the same way (after the switch
5251
// statement).
5352

5453
// Descend
55-
case w.value.ListValue != nil:
54+
case value.IsList(w.value):
5655
// If the list were atomic, we'd break here, but we don't have
5756
// a schema, so we can't tell.
5857

59-
for i, child := range w.value.ListValue.Items {
58+
for i, child := range value.ValueList(w.value) {
6059
w2 := *w
6160
w2.path = append(w.path, GuessBestListPathElement(i, child))
6261
w2.value = child
6362
w2.walk()
6463
}
6564
return
66-
case w.value.MapValue != nil:
65+
case value.IsMap(w.value):
6766
// If the map/struct were atomic, we'd break here, but we don't
6867
// have a schema, so we can't tell.
6968

70-
for i := range w.value.MapValue.Items {
71-
child := w.value.MapValue.Items[i]
69+
for key, val := range value.ValueMap(w.value) {
7270
w2 := *w
73-
w2.path = append(w.path, PathElement{FieldName: &child.Name})
74-
w2.value = child.Value
71+
k := key
72+
w2.path = append(w.path, PathElement{FieldName: &k})
73+
w2.value = val
7574
w2.walk()
7675
}
7776
return
@@ -97,7 +96,7 @@ var AssociativeListCandidateFieldNames = []string{
9796
// whether item has any of the fields listed in
9897
// AssociativeListCandidateFieldNames which have scalar values.
9998
func GuessBestListPathElement(index int, item value.Value) PathElement {
100-
if item.MapValue == nil {
99+
if !value.IsMap(item) {
101100
// Non map items could be parts of sets or regular "atomic"
102101
// lists. We won't try to guess whether something should be a
103102
// set or not.
@@ -106,15 +105,15 @@ func GuessBestListPathElement(index int, item value.Value) PathElement {
106105

107106
var keys value.FieldList
108107
for _, name := range AssociativeListCandidateFieldNames {
109-
f, ok := item.MapValue.Get(name)
108+
f, ok := value.ValueMap(item)[name]
110109
if !ok {
111110
continue
112111
}
113112
// only accept primitive/scalar types as keys.
114-
if f.Value.Null || f.Value.MapValue != nil || f.Value.ListValue != nil {
113+
if f == nil || value.IsMap(f) || value.IsList(f) {
115114
continue
116115
}
117-
keys = append(keys, *f)
116+
keys = append(keys, value.Field{Name: name, Value: f})
118117
}
119118
if len(keys) > 0 {
120119
keys.Sort()

‎fieldpath/fromvalue_test.go

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ package fieldpath
1919
import (
2020
"testing"
2121

22-
"sigs.k8s.io/structured-merge-diff/value"
22+
"gopkg.in/yaml.v2"
2323
)
2424

2525
func TestFromValue(t *testing.T) {
@@ -31,19 +31,19 @@ func TestFromValue(t *testing.T) {
3131
{`{"a": [{"a": null}]}`, NewSet(
3232
MakePathOrDie("a", 0, "a"),
3333
)}, {`{"a": [{"id": a}]}`, NewSet(
34-
MakePathOrDie("a", KeyByFields("id", value.StringValue("a")), "id"),
34+
MakePathOrDie("a", KeyByFields("id", "a"), "id"),
3535
)}, {`{"a": [{"name": a}]}`, NewSet(
36-
MakePathOrDie("a", KeyByFields("name", value.StringValue("a")), "name"),
36+
MakePathOrDie("a", KeyByFields("name", "a"), "name"),
3737
)}, {`{"a": [{"key": a}]}`, NewSet(
38-
MakePathOrDie("a", KeyByFields("key", value.StringValue("a")), "key"),
38+
MakePathOrDie("a", KeyByFields("key", "a"), "key"),
3939
)}, {`{"a": [{"name": "a", "key": "b"}]}`, NewSet(
4040
MakePathOrDie("a", KeyByFields(
41-
"key", value.StringValue("b"),
42-
"name", value.StringValue("a"),
41+
"key", "b",
42+
"name", "a",
4343
), "key"),
4444
MakePathOrDie("a", KeyByFields(
45-
"key", value.StringValue("b"),
46-
"name", value.StringValue("a"),
45+
"key", "b",
46+
"name", "a",
4747
), "name"),
4848
)}, {`{"a": [5]}`, NewSet(
4949
MakePathOrDie("a", 0),
@@ -65,7 +65,8 @@ func TestFromValue(t *testing.T) {
6565
tt := tt
6666
t.Run(tt.objYAML, func(t *testing.T) {
6767
t.Parallel()
68-
v, err := value.FromYAML([]byte(tt.objYAML))
68+
var v interface{}
69+
err := yaml.Unmarshal([]byte(tt.objYAML), &v)
6970
if err != nil {
7071
t.Fatalf("couldn't parse: %v", err)
7172
}

‎fieldpath/managers.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,11 @@ limitations under the License.
1313

1414
package fieldpath
1515

16+
import (
17+
"fmt"
18+
"strings"
19+
)
20+
1621
// APIVersion describes the version of an object or of a fieldset.
1722
type APIVersion string
1823

@@ -95,3 +100,14 @@ func (lhs ManagedFields) Difference(rhs ManagedFields) ManagedFields {
95100

96101
return diff
97102
}
103+
104+
func (lhs ManagedFields) String() string {
105+
s := strings.Builder{}
106+
for k, v := range lhs {
107+
fmt.Fprintf(&s, "%s:\n", k)
108+
fmt.Fprintf(&s, "- Applied: %v\n", v.Applied())
109+
fmt.Fprintf(&s, "- APIVersion: %v\n", v.APIVersion())
110+
fmt.Fprintf(&s, "- Set: %v\n", v.Set())
111+
}
112+
return s.String()
113+
}

‎fieldpath/path.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,10 +93,10 @@ func MakePath(parts ...interface{}) (Path, error) {
9393
return nil, fmt.Errorf("associative list key type path elements must have at least one key (got zero)")
9494
}
9595
fp = append(fp, PathElement{Key: t})
96-
case value.Value:
96+
case *value.Value:
9797
// TODO: understand schema and verify that this is a set type
9898
// TODO: make a copy of t
99-
fp = append(fp, PathElement{Value: &t})
99+
fp = append(fp, PathElement{Value: t})
100100
default:
101101
return nil, fmt.Errorf("unable to make %#v into a path element", p)
102102
}

‎fieldpath/path_test.go

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,12 @@ import (
2222
"sigs.k8s.io/structured-merge-diff/value"
2323
)
2424

25+
var (
26+
_V = func(v value.Value) *value.Value {
27+
return &v
28+
}
29+
)
30+
2531
func TestPathString(t *testing.T) {
2632
table := []struct {
2733
name string
@@ -31,16 +37,20 @@ func TestPathString(t *testing.T) {
3137
{"basic1", MakePathOrDie("foo", 1), ".foo[1]"},
3238
{"basic2", MakePathOrDie("foo", "bar", 1, "baz"), ".foo.bar[1].baz"},
3339
{"associative-list-ref", MakePathOrDie("foo", KeyByFields(
34-
"a", value.StringValue("b"),
35-
"c", value.IntValue(1),
36-
"d", value.FloatValue(1.5),
37-
"e", value.BooleanValue(true),
40+
// This makes sure we test all types: string,
41+
// floats, integers and booleans.
42+
"a", "b",
43+
"c", 1,
44+
"d", 1.5,
45+
"e", true,
3846
)), `.foo[a="b",c=1,d=1.5,e=true]`},
3947
{"sets", MakePathOrDie("foo",
40-
value.StringValue("b"),
41-
value.IntValue(5),
42-
value.BooleanValue(false),
43-
value.FloatValue(3.14159),
48+
// This makes sure we test all types: string,
49+
// floats, integers and booleans.
50+
_V("b"),
51+
_V(5),
52+
_V(false),
53+
_V(3.14159),
4454
), `.foo[="b"][=5][=false][=3.14159]`},
4555
}
4656
for _, tt := range table {

‎fieldpath/pathelementmap.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,10 +76,10 @@ func (s *PathElementValueMap) Get(pe PathElement) (value.Value, bool) {
7676
return !s.members[i].PathElement.Less(pe)
7777
})
7878
if loc == len(s.members) {
79-
return value.Value{}, false
79+
return nil, false
8080
}
8181
if s.members[loc].PathElement.Equals(pe) {
8282
return s.members[loc].Value, true
8383
}
84-
return value.Value{}, false
84+
return nil, false
8585
}

‎fieldpath/pathelementmap_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,22 +29,22 @@ func TestPathElementValueMap(t *testing.T) {
2929
t.Fatal("Unexpected path-element found in empty map")
3030
}
3131

32-
m.Insert(PathElement{FieldName: strptr("carrot")}, value.StringValue("knife"))
33-
m.Insert(PathElement{FieldName: strptr("chive")}, value.IntValue(2))
32+
m.Insert(PathElement{FieldName: strptr("carrot")}, "knife")
33+
m.Insert(PathElement{FieldName: strptr("chive")}, 2)
3434

3535
if _, ok := m.Get(PathElement{FieldName: strptr("onion")}); ok {
3636
t.Fatal("Unexpected path-element in map")
3737
}
3838

3939
if val, ok := m.Get(PathElement{FieldName: strptr("carrot")}); !ok {
4040
t.Fatal("Missing path-element in map")
41-
} else if !val.Equals(value.StringValue("knife")) {
41+
} else if !value.Equals(val, "knife") {
4242
t.Fatalf("Unexpected value found: %#v", val)
4343
}
4444

4545
if val, ok := m.Get(PathElement{FieldName: strptr("chive")}); !ok {
4646
t.Fatal("Missing path-element in map")
47-
} else if !val.Equals(value.IntValue(2)) {
47+
} else if !value.Equals(val, 2) {
4848
t.Fatalf("Unexpected value found: %#v", val)
4949
}
5050
}

‎fieldpath/serialize-pe.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ func DeserializePathElement(s string) (PathElement, error) {
8484
iter := readPool.BorrowIterator(b)
8585
defer readPool.ReturnIterator(iter)
8686
fields := value.FieldList{}
87+
8788
iter.ReadObjectCB(func(iter *jsoniter.Iterator, key string) bool {
8889
v, err := value.ReadJSONIter(iter)
8990
if err != nil {
@@ -134,19 +135,20 @@ func serializePathElementToWriter(w io.Writer, pe PathElement) error {
134135
return err
135136
}
136137
stream.WriteObjectStart()
138+
137139
for i, field := range *pe.Key {
138140
if i > 0 {
139141
stream.WriteMore()
140142
}
141143
stream.WriteObjectField(field.Name)
142-
field.Value.WriteJSONStream(stream)
144+
value.WriteJSONStream(field.Value, stream)
143145
}
144146
stream.WriteObjectEnd()
145147
case pe.Value != nil:
146148
if _, err := stream.Write(peValueSepBytes); err != nil {
147149
return err
148150
}
149-
pe.Value.WriteJSONStream(stream)
151+
value.WriteJSONStream(*pe.Value, stream)
150152
case pe.Index != nil:
151153
if _, err := stream.Write(peIndexSepBytes); err != nil {
152154
return err

0 commit comments

Comments
 (0)