Skip to content

Commit e8b9d44

Browse files
author
Antoine Pelisse
committed
Create an actual interface rather than interface{}
Problem with interface{} is that we need all the objects to be in their "unstructured" form, and this won't work for built-in types that will still have to be converted to that form. We can further improve the allocations by removing the need for interface{}, and built a real interface instead that can be implemented for built-in types.
1 parent 5dc536e commit e8b9d44

23 files changed

+482
-420
lines changed

fieldpath/element.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +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{Name: nameValues[i].(string), Value: nameValues[i+1]})
130+
out = append(out, value.Field{Name: nameValues[i].(string), Value: value.ValueInterface{Value: nameValues[i+1]}})
131131
}
132132
out.Sort()
133133
return &out

fieldpath/element_test.go

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -62,9 +62,12 @@ 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 value.Value) *value.Value { return &i }
65+
func strptr(s string) *string { return &s }
66+
func intptr(i int) *int { return &i }
67+
func valptr(i interface{}) *value.Value {
68+
v := value.Value(value.ValueInterface{Value: i})
69+
return &v
70+
}
6871

6972
func TestPathElementLess(t *testing.T) {
7073
table := []struct {

fieldpath/fromvalue.go

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -43,30 +43,30 @@ type objectWalker struct {
4343

4444
func (w *objectWalker) walk() {
4545
switch {
46-
case value.IsFloat(w.value):
47-
case value.IsInt(w.value):
48-
case value.IsString(w.value):
49-
case value.IsBool(w.value):
46+
case w.value.IsFloat():
47+
case w.value.IsInt():
48+
case w.value.IsString():
49+
case w.value.IsBool():
5050
// All leaf fields handled the same way (after the switch
5151
// statement).
5252

5353
// Descend
54-
case value.IsList(w.value):
54+
case w.value.IsList():
5555
// If the list were atomic, we'd break here, but we don't have
5656
// a schema, so we can't tell.
5757

58-
for i, child := range value.ValueList(w.value) {
58+
for i := 0; i < w.value.List().Length(); i++ {
5959
w2 := *w
60-
w2.path = append(w.path, GuessBestListPathElement(i, child))
61-
w2.value = child
60+
w2.path = append(w.path, GuessBestListPathElement(i, w.value.List().At(i)))
61+
w2.value = w.value.List().At(i)
6262
w2.walk()
6363
}
6464
return
65-
case value.IsMap(w.value):
65+
case w.value.IsMap():
6666
// If the map/struct were atomic, we'd break here, but we don't
6767
// have a schema, so we can't tell.
6868

69-
value.ValueMap(w.value).Iterate(func(k string, val value.Value) bool {
69+
w.value.Map().Iterate(func(k string, val value.Value) bool {
7070
w2 := *w
7171
w2.path = append(w.path, PathElement{FieldName: &k})
7272
w2.value = val
@@ -96,7 +96,7 @@ var AssociativeListCandidateFieldNames = []string{
9696
// whether item has any of the fields listed in
9797
// AssociativeListCandidateFieldNames which have scalar values.
9898
func GuessBestListPathElement(index int, item value.Value) PathElement {
99-
if !value.IsMap(item) {
99+
if !item.IsMap() {
100100
// Non map items could be parts of sets or regular "atomic"
101101
// lists. We won't try to guess whether something should be a
102102
// set or not.
@@ -105,12 +105,12 @@ func GuessBestListPathElement(index int, item value.Value) PathElement {
105105

106106
var keys value.FieldList
107107
for _, name := range AssociativeListCandidateFieldNames {
108-
f, ok := value.ValueMap(item).Get(name)
108+
f, ok := item.Map().Get(name)
109109
if !ok {
110110
continue
111111
}
112112
// only accept primitive/scalar types as keys.
113-
if f == nil || value.IsMap(f) || value.IsList(f) {
113+
if f.IsNull() || f.IsMap() || f.IsList() {
114114
continue
115115
}
116116
keys = append(keys, value.Field{Name: name, Value: f})

fieldpath/fromvalue_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"testing"
2121

2222
"gopkg.in/yaml.v2"
23+
"sigs.k8s.io/structured-merge-diff/value"
2324
)
2425

2526
func TestFromValue(t *testing.T) {
@@ -70,7 +71,7 @@ func TestFromValue(t *testing.T) {
7071
if err != nil {
7172
t.Fatalf("couldn't parse: %v", err)
7273
}
73-
got := SetFromValue(v)
74+
got := SetFromValue(value.ValueInterface{Value: v})
7475
if !got.Equals(tt.set) {
7576
t.Errorf("wanted\n%s\nbut got\n%s\n", tt.set, got)
7677
}

fieldpath/path_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,8 @@ import (
2323
)
2424

2525
var (
26-
_V = func(v value.Value) *value.Value {
26+
_V = func(i interface{}) *value.Value {
27+
v := value.Value(value.ValueInterface{Value: i})
2728
return &v
2829
}
2930
)

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")}, "knife")
33-
m.Insert(PathElement{FieldName: strptr("chive")}, 2)
32+
m.Insert(PathElement{FieldName: strptr("carrot")}, value.ValueInterface{Value: "knife"})
33+
m.Insert(PathElement{FieldName: strptr("chive")}, value.ValueInterface{Value: 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 !value.Equals(val, "knife") {
41+
} else if !value.Equals(val, value.ValueInterface{Value: "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 !value.Equals(val, 2) {
47+
} else if !value.Equals(val, value.ValueInterface{Value: 2}) {
4848
t.Fatalf("Unexpected value found: %#v", val)
4949
}
5050
}

internal/cli/operation.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ func (m merge) Execute(w io.Writer) error {
114114
return err
115115
}
116116

117-
yaml, err := yaml.Marshal(out.AsValue())
117+
yaml, err := yaml.Marshal(out.AsValue().Interface())
118118
if err != nil {
119119
return err
120120
}

merge/conflict_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,8 @@ var (
2929
_NS = fieldpath.NewSet
3030
_P = fieldpath.MakePathOrDie
3131
_KBF = fieldpath.KeyByFields
32-
_V = func(v value.Value) *value.Value {
32+
_V = func(i interface{}) *value.Value {
33+
v := value.Value(value.ValueInterface{Value: i})
3334
return &v
3435
}
3536
)

merge/multiple_appliers_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -985,7 +985,7 @@ func (r repeatingConverter) Convert(v *typed.TypedValue, version fieldpath.APIVe
985985
if err != nil {
986986
return nil, missingVersionError
987987
}
988-
y, err := yaml.Marshal(v.AsValue())
988+
y, err := yaml.Marshal(v.AsValue().Interface())
989989
if err != nil {
990990
return nil, err
991991
}

typed/helpers.go

Lines changed: 19 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -101,16 +101,16 @@ func resolveSchema(s *schema.Schema, tr schema.TypeRef, v *value.Value, ah atomH
101101
// and validation will fail at a later stage. (with a more useful error)
102102
func deduceAtom(atom schema.Atom, val *value.Value) schema.Atom {
103103
switch {
104-
case val == nil:
105-
case value.IsFloat(*val), value.IsInt(*val), value.IsString(*val), value.IsBool(*val):
104+
case val == nil, *val == nil:
105+
case (*val).IsFloat(), (*val).IsInt(), (*val).IsString(), (*val).IsBool():
106106
if atom.Scalar != nil {
107107
return schema.Atom{Scalar: atom.Scalar}
108108
}
109-
case value.IsList(*val):
109+
case (*val).IsList():
110110
if atom.List != nil {
111111
return schema.Atom{List: atom.List}
112112
}
113-
case value.IsMap(*val):
113+
case (*val).IsMap():
114114
if atom.Map != nil {
115115
return schema.Atom{Map: atom.Map}
116116
}
@@ -137,43 +137,45 @@ func handleAtom(a schema.Atom, tr schema.TypeRef, ah atomHandler) ValidationErro
137137
}
138138

139139
// Returns the list, or an error. Reminder: nil is a valid list and might be returned.
140-
func listValue(val value.Value) ([]interface{}, error) {
141-
if val == nil {
140+
func listValue(val value.Value) (value.List, error) {
141+
if val.IsNull() {
142142
// Null is a valid list.
143143
return nil, nil
144144
}
145-
l, ok := val.([]interface{})
146-
if !ok {
145+
if !val.IsList() {
147146
return nil, fmt.Errorf("expected list, got %v", val)
148147
}
149-
return l, nil
148+
return val.List(), nil
150149
}
151150

152151
// Returns the map, or an error. Reminder: nil is a valid map and might be returned.
153152
func mapValue(val value.Value) (value.Map, error) {
154153
if val == nil {
154+
return nil, fmt.Errorf("expected map, got nil")
155+
}
156+
if val.IsNull() {
155157
// Null is a valid map.
156158
return nil, nil
157159
}
158-
if !value.IsMap(val) {
160+
if !val.IsMap() {
159161
return nil, fmt.Errorf("expected map, got %v", val)
160162
}
161-
return value.ValueMap(val), nil
163+
return val.Map(), nil
162164
}
163165

164166
func keyedAssociativeListItemToPathElement(list *schema.List, index int, child value.Value) (fieldpath.PathElement, error) {
165167
pe := fieldpath.PathElement{}
166-
if child == nil {
168+
if child.IsNull() {
167169
// For now, the keys are required which means that null entries
168170
// are illegal.
169171
return pe, errors.New("associative list with keys may not have a null element")
170172
}
171-
if !value.IsMap(child) {
173+
if !child.IsMap() {
172174
return pe, errors.New("associative list with keys may not have non-map elements")
173175
}
174176
keyMap := value.FieldList{}
175177
for _, fieldName := range list.Keys {
176-
if val, ok := value.ValueMap(child).Get(fieldName); ok {
178+
if val, ok := child.Map().Get(fieldName); ok {
177179
keyMap = append(keyMap, value.Field{Name: fieldName, Value: val})
178180
} else {
179181
return pe, fmt.Errorf("associative list with keys has an element that omits key field %q", fieldName)
@@ -187,15 +189,15 @@ func keyedAssociativeListItemToPathElement(list *schema.List, index int, child v
187189
func setItemToPathElement(list *schema.List, index int, child value.Value) (fieldpath.PathElement, error) {
188190
pe := fieldpath.PathElement{}
189191
switch {
190-
case value.IsMap(child):
192+
case child.IsMap():
191193
// TODO: atomic maps should be acceptable.
192194
return pe, errors.New("associative list without keys has an element that's a map type")
193-
case value.IsList(child):
195+
case child.IsList():
194196
// Should we support a set of lists? For the moment
195197
// let's say we don't.
196198
// TODO: atomic lists should be acceptable.
197199
return pe, errors.New("not supported: associative list with lists as elements")
198-
case child == nil:
200+
case child.IsNull():
199201
return pe, errors.New("associative list without keys has an element that's an explicit null")
200202
default:
201203
// We are a set type.

0 commit comments

Comments
 (0)