Skip to content

Commit b8f4c05

Browse files
lavalamplavalamp
committed
replace []Field with *Map
1 parent 55d9506 commit b8f4c05

File tree

10 files changed

+53
-36
lines changed

10 files changed

+53
-36
lines changed

‎fieldpath/element.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ type PathElement struct {
3535
// Key selects the list element which has fields matching those given.
3636
// The containing object must be an associative list with map typed
3737
// elements.
38-
Key []value.Field
38+
Key *value.Map
3939

4040
// Value selects the list element with the given value. The containing
4141
// object must be an associative list with a primitive typed element
@@ -58,12 +58,12 @@ func (e PathElement) Less(rhs PathElement) bool {
5858
return false
5959
}
6060

61-
if len(e.Key) != 0 {
62-
if len(rhs.Key) == 0 {
61+
if e.Key != nil {
62+
if rhs.Key == nil {
6363
return true
6464
}
65-
return value.FieldCollectionLess(e.Key, rhs.Key)
66-
} else if len(rhs.Key) != 0 {
65+
return e.Key.Less(rhs.Key)
66+
} else if rhs.Key != nil {
6767
return false
6868
}
6969

@@ -100,9 +100,9 @@ func (e PathElement) String() string {
100100
switch {
101101
case e.FieldName != nil:
102102
return "." + *e.FieldName
103-
case len(e.Key) > 0:
104-
strs := make([]string, len(e.Key))
105-
for i, k := range e.Key {
103+
case e.Key != nil:
104+
strs := make([]string, len(e.Key.Items))
105+
for i, k := range e.Key.Items {
106106
strs[i] = fmt.Sprintf("%v=%v", k.Name, k.Value)
107107
}
108108
// The order must be canonical, since we use the string value

‎fieldpath/element_test.go

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ func TestPathElementLess(t *testing.T) {
9191
}, {
9292
name: "FieldName-3",
9393
a: PathElement{FieldName: strptr("capybara")},
94-
b: PathElement{Key: []value.Field{{Name: "dog", Value: value.IntValue(3)}}},
94+
b: PathElement{Key: &value.Map{Items: []value.Field{{Name: "dog", Value: value.IntValue(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: []value.Field{{Name: "goat", Value: value.IntValue(1)}}},
106-
b: PathElement{Key: []value.Field{{Name: "goat", Value: value.IntValue(1)}}},
105+
a: PathElement{Key: &value.Map{Items: []value.Field{{Name: "goat", Value: value.IntValue(1)}}}},
106+
b: PathElement{Key: &value.Map{Items: []value.Field{{Name: "goat", Value: value.IntValue(1)}}}},
107107
eq: true,
108108
}, {
109109
name: "Key-2",
110-
a: PathElement{Key: []value.Field{{Name: "horse", Value: value.IntValue(1)}}},
111-
b: PathElement{Key: []value.Field{{Name: "horse", Value: value.IntValue(2)}}},
110+
a: PathElement{Key: &value.Map{Items: []value.Field{{Name: "horse", Value: value.IntValue(1)}}}},
111+
b: PathElement{Key: &value.Map{Items: []value.Field{{Name: "horse", Value: value.IntValue(2)}}}},
112112
}, {
113113
name: "Key-3",
114-
a: PathElement{Key: []value.Field{{Name: "ibex", Value: value.IntValue(1)}}},
115-
b: PathElement{Key: []value.Field{{Name: "jay", Value: value.IntValue(1)}}},
114+
a: PathElement{Key: &value.Map{Items: []value.Field{{Name: "ibex", Value: value.IntValue(1)}}}},
115+
b: PathElement{Key: &value.Map{Items: []value.Field{{Name: "jay", Value: value.IntValue(1)}}}},
116116
}, {
117117
name: "Key-4",
118-
a: PathElement{Key: []value.Field{{Name: "kite", Value: value.IntValue(1)}}},
119-
b: PathElement{Key: []value.Field{{Name: "kite", Value: value.IntValue(1)}, {Name: "kite-2", Value: value.IntValue(1)}}},
118+
a: PathElement{Key: &value.Map{Items: []value.Field{{Name: "kite", Value: value.IntValue(1)}}}},
119+
b: PathElement{Key: &value.Map{Items: []value.Field{{Name: "kite", Value: value.IntValue(1)}, {Name: "kite-2", Value: value.IntValue(1)}}}},
120120
}, {
121121
name: "Key-5",
122-
a: PathElement{Key: []value.Field{{Name: "kite", Value: value.IntValue(1)}}},
122+
a: PathElement{Key: &value.Map{Items: []value.Field{{Name: "kite", Value: value.IntValue(1)}}}},
123123
b: PathElement{Value: valptr(1)},
124124
}, {
125125
name: "Key-6",
126-
a: PathElement{Key: []value.Field{{Name: "kite", Value: value.IntValue(1)}}},
126+
a: PathElement{Key: &value.Map{Items: []value.Field{{Name: "kite", Value: value.IntValue(1)}}}},
127127
b: PathElement{Index: intptr(5)},
128128
}, {
129129
name: "Value-1",

‎fieldpath/fromvalue.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ func GuessBestListPathElement(index int, item value.Value) PathElement {
117117
keys = append(keys, *f)
118118
}
119119
if len(keys) > 0 {
120-
return PathElement{Key: keys}
120+
return PathElement{Key: &value.Map{Items: keys}}
121121
}
122122
return PathElement{Index: &index}
123123
}

‎fieldpath/path.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,11 @@ func (fp Path) String() string {
3535
return strings.Join(strs, "")
3636
}
3737

38+
// Equals returns true if the two paths are equivalent.
39+
func (fp Path) Equals(fp2 Path) bool {
40+
return !fp.Less(fp2) && !fp2.Less(fp)
41+
}
42+
3843
// Less provides a lexical order for Paths.
3944
func (fp Path) Less(rhs Path) bool {
4045
i := 0
@@ -87,7 +92,7 @@ func MakePath(parts ...interface{}) (Path, error) {
8792
if len(t) == 0 {
8893
return nil, fmt.Errorf("associative list key type path elements must have at least one key (got zero)")
8994
}
90-
fp = append(fp, PathElement{Key: t})
95+
fp = append(fp, PathElement{Key: &value.Map{Items: t}})
9196
case value.Value:
9297
// TODO: understand schema and verify that this is a set type
9398
// TODO: make a copy of t

‎fieldpath/path_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import (
2222
"sigs.k8s.io/structured-merge-diff/value"
2323
)
2424

25-
func TestPath(t *testing.T) {
25+
func TestPathString(t *testing.T) {
2626
table := []struct {
2727
name string
2828
fp Path

‎fieldpath/set_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ func TestSetInsertHas(t *testing.T) {
181181
}
182182

183183
func TestSetString(t *testing.T) {
184-
p := MakePathOrDie("foo", PathElement{Key: KeyByFields("name", value.StringValue("first"))})
184+
p := MakePathOrDie("foo", PathElement{Key: &value.Map{Items: KeyByFields("name", value.StringValue("first"))}})
185185
s1 := NewSet(p)
186186

187187
if p.String() != s1.String() {

‎internal/fixture/state.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ package fixture
1919
import (
2020
"bytes"
2121
"fmt"
22-
"reflect"
2322

2423
"sigs.k8s.io/structured-merge-diff/fieldpath"
2524
"sigs.k8s.io/structured-merge-diff/merge"
@@ -164,7 +163,7 @@ type Operation interface {
164163

165164
func hasConflict(conflicts merge.Conflicts, conflict merge.Conflict) bool {
166165
for i := range conflicts {
167-
if reflect.DeepEqual(conflict, conflicts[i]) {
166+
if conflict.Equals(conflicts[i]) {
168167
return true
169168
}
170169
}

‎merge/conflict.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,14 @@ func (c Conflict) Error() string {
4040
return fmt.Sprintf("conflict with %q: %v", c.Manager, c.Path)
4141
}
4242

43+
// Equals returns true if c == c2
44+
func (c Conflict) Equals(c2 Conflict) bool {
45+
if c.Manager != c2.Manager {
46+
return false
47+
}
48+
return c.Path.Equals(c2.Path)
49+
}
50+
4351
// Conflicts accumulates multiple conflicts and aggregates them by managers.
4452
type Conflicts []Conflict
4553

@@ -74,6 +82,19 @@ func (conflicts Conflicts) Error() string {
7482
return strings.Join(messages, "\n")
7583
}
7684

85+
// Equals returns true if the lists of conflicts are the same.
86+
func (c Conflicts) Equals(c2 Conflicts) bool {
87+
if len(c) != len(c2) {
88+
return false
89+
}
90+
for i := range c {
91+
if !c[i].Equals(c2[i]) {
92+
return false
93+
}
94+
}
95+
return true
96+
}
97+
7798
// ConflictsFromManagers creates a list of conflicts given Managers sets.
7899
func ConflictsFromManagers(sets fieldpath.ManagedFields) Conflicts {
79100
conflicts := []Conflict{}

‎typed/helpers.go

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,7 @@ func keyedAssociativeListItemToPathElement(list schema.List, index int, child va
197197
if child.MapValue == nil {
198198
return pe, errors.New("associative list with keys may not have non-map elements")
199199
}
200+
keyMap := &value.Map{}
200201
for _, fieldName := range list.Keys {
201202
var fieldValue value.Value
202203
field, ok := child.MapValue.Get(fieldName)
@@ -206,11 +207,9 @@ func keyedAssociativeListItemToPathElement(list schema.List, index int, child va
206207
// Treat keys as required.
207208
return pe, fmt.Errorf("associative list with keys has an element that omits key field %q", fieldName)
208209
}
209-
pe.Key = append(pe.Key, value.Field{
210-
Name: fieldName,
211-
Value: fieldValue,
212-
})
210+
keyMap.Set(fieldName, fieldValue)
213211
}
212+
pe.Key = keyMap
214213
return pe, nil
215214
}
216215

‎value/value.go

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -173,13 +173,6 @@ type Map struct {
173173
order []int
174174
}
175175

176-
// FieldCollectionLess uses the map's Less logic to compare the two collections of fields.
177-
func FieldCollectionLess(lhs, rhs []Field) bool {
178-
m1 := Map{Items: lhs}
179-
m2 := Map{Items: rhs}
180-
return m1.Less(&m2)
181-
}
182-
183176
func (m *Map) computeOrder() {
184177
if len(m.order) != len(m.Items) {
185178
m.order = make([]int, len(m.Items))

0 commit comments

Comments
 (0)