Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit 336d337

Browse files
k8s-ci-robotk8s-ci-robot
authoredOct 23, 2019
Merge pull request #118 from apelisse/keyvalue
Simplify PathElement Keys type
2 parents e660f95 + b21daea commit 336d337

File tree

9 files changed

+107
-42
lines changed

9 files changed

+107
-42
lines changed
 

‎fieldpath/element.go

Lines changed: 9 additions & 10 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.Map
38+
Key *value.FieldList
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
@@ -62,7 +62,7 @@ func (e PathElement) Less(rhs PathElement) bool {
6262
if rhs.Key == nil {
6363
return true
6464
}
65-
return e.Key.Less(rhs.Key)
65+
return e.Key.Less(*rhs.Key)
6666
} else if rhs.Key != nil {
6767
return false
6868
}
@@ -101,13 +101,11 @@ func (e PathElement) String() string {
101101
case e.FieldName != nil:
102102
return "." + *e.FieldName
103103
case e.Key != nil:
104-
strs := make([]string, len(e.Key.Items))
105-
for i, k := range e.Key.Items {
104+
strs := make([]string, len(*e.Key))
105+
for i, k := range *e.Key {
106106
strs[i] = fmt.Sprintf("%v=%v", k.Name, k.Value)
107107
}
108-
// The order must be canonical, since we use the string value
109-
// in a set structure.
110-
sort.Strings(strs)
108+
// Keys are supposed to be sorted.
111109
return "[" + strings.Join(strs, ",") + "]"
112110
case e.Value != nil:
113111
return fmt.Sprintf("[=%v]", e.Value)
@@ -123,18 +121,19 @@ func (e PathElement) String() string {
123121
// names (type must be string) with values (type must be value.Value). If these
124122
// conditions are not met, KeyByFields will panic--it's intended for static
125123
// construction and shouldn't have user-produced values passed to it.
126-
func KeyByFields(nameValues ...interface{}) []value.Field {
124+
func KeyByFields(nameValues ...interface{}) *value.FieldList {
127125
if len(nameValues)%2 != 0 {
128126
panic("must have a value for every name")
129127
}
130-
out := []value.Field{}
128+
out := value.FieldList{}
131129
for i := 0; i < len(nameValues)-1; i += 2 {
132130
out = append(out, value.Field{
133131
Name: nameValues[i].(string),
134132
Value: nameValues[i+1].(value.Value),
135133
})
136134
}
137-
return out
135+
out.Sort()
136+
return &out
138137
}
139138

140139
// PathElementSet is a set of path elements.

‎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.Map{Items: []value.Field{{Name: "dog", Value: value.IntValue(3)}}}},
94+
b: PathElement{Key: KeyByFields("dog", 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.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)}}}},
105+
a: PathElement{Key: KeyByFields("goat", value.IntValue(1))},
106+
b: PathElement{Key: KeyByFields("goat", value.IntValue(1))},
107107
eq: true,
108108
}, {
109109
name: "Key-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)}}}},
110+
a: PathElement{Key: KeyByFields("horse", value.IntValue(1))},
111+
b: PathElement{Key: KeyByFields("horse", value.IntValue(2))},
112112
}, {
113113
name: "Key-3",
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)}}}},
114+
a: PathElement{Key: KeyByFields("ibex", value.IntValue(1))},
115+
b: PathElement{Key: KeyByFields("jay", value.IntValue(1))},
116116
}, {
117117
name: "Key-4",
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)}}}},
118+
a: PathElement{Key: KeyByFields("kite", value.IntValue(1))},
119+
b: PathElement{Key: KeyByFields("kite", value.IntValue(1), "kite-2", value.IntValue(1))},
120120
}, {
121121
name: "Key-5",
122-
a: PathElement{Key: &value.Map{Items: []value.Field{{Name: "kite", Value: value.IntValue(1)}}}},
122+
a: PathElement{Key: KeyByFields("kite", value.IntValue(1))},
123123
b: PathElement{Value: valptr(1)},
124124
}, {
125125
name: "Key-6",
126-
a: PathElement{Key: &value.Map{Items: []value.Field{{Name: "kite", Value: value.IntValue(1)}}}},
126+
a: PathElement{Key: KeyByFields("kite", value.IntValue(1))},
127127
b: PathElement{Index: intptr(5)},
128128
}, {
129129
name: "Value-1",

‎fieldpath/fromvalue.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ func GuessBestListPathElement(index int, item value.Value) PathElement {
104104
return PathElement{Index: &index}
105105
}
106106

107-
var keys []value.Field
107+
var keys value.FieldList
108108
for _, name := range AssociativeListCandidateFieldNames {
109109
f, ok := item.MapValue.Get(name)
110110
if !ok {
@@ -117,7 +117,8 @@ func GuessBestListPathElement(index int, item value.Value) PathElement {
117117
keys = append(keys, *f)
118118
}
119119
if len(keys) > 0 {
120-
return PathElement{Key: &value.Map{Items: keys}}
120+
keys.Sort()
121+
return PathElement{Key: &keys}
121122
}
122123
return PathElement{Index: &index}
123124
}

‎fieldpath/fromvalue_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,12 +38,12 @@ func TestFromValue(t *testing.T) {
3838
MakePathOrDie("a", KeyByFields("key", value.StringValue("a")), "key"),
3939
)}, {`{"a": [{"name": "a", "key": "b"}]}`, NewSet(
4040
MakePathOrDie("a", KeyByFields(
41-
"name", value.StringValue("a"),
4241
"key", value.StringValue("b"),
42+
"name", value.StringValue("a"),
4343
), "key"),
4444
MakePathOrDie("a", KeyByFields(
45-
"name", value.StringValue("a"),
4645
"key", value.StringValue("b"),
46+
"name", value.StringValue("a"),
4747
), "name"),
4848
)}, {`{"a": [5]}`, NewSet(
4949
MakePathOrDie("a", 0),

‎fieldpath/path.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -88,11 +88,11 @@ func MakePath(parts ...interface{}) (Path, error) {
8888
fp = append(fp, PathElement{Index: &t})
8989
case string:
9090
fp = append(fp, PathElement{FieldName: &t})
91-
case []value.Field:
92-
if len(t) == 0 {
91+
case *value.FieldList:
92+
if len(*t) == 0 {
9393
return nil, fmt.Errorf("associative list key type path elements must have at least one key (got zero)")
9494
}
95-
fp = append(fp, PathElement{Key: &value.Map{Items: t}})
95+
fp = append(fp, PathElement{Key: t})
9696
case value.Value:
9797
// TODO: understand schema and verify that this is a set type
9898
// TODO: make a copy of t

‎fieldpath/serialize-pe.go

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -83,14 +83,18 @@ func DeserializePathElement(s string) (PathElement, error) {
8383
case peKeySepBytes[0]:
8484
iter := readPool.BorrowIterator(b)
8585
defer readPool.ReturnIterator(iter)
86-
v, err := value.ReadJSONIter(iter)
87-
if err != nil {
88-
return PathElement{}, err
89-
}
90-
if v.MapValue == nil {
91-
return PathElement{}, fmt.Errorf("expected key value pairs but got %#v", v)
92-
}
93-
return PathElement{Key: v.MapValue}, nil
86+
fields := value.FieldList{}
87+
iter.ReadObjectCB(func(iter *jsoniter.Iterator, key string) bool {
88+
v, err := value.ReadJSONIter(iter)
89+
if err != nil {
90+
iter.Error = err
91+
return false
92+
}
93+
fields = append(fields, value.Field{Name: key, Value: v})
94+
return true
95+
})
96+
fields.Sort()
97+
return PathElement{Key: &fields}, iter.Error
9498
case peIndexSepBytes[0]:
9599
i, err := strconv.Atoi(s[2:])
96100
if err != nil {
@@ -129,8 +133,15 @@ func serializePathElementToWriter(w io.Writer, pe PathElement) error {
129133
if _, err := stream.Write(peKeySepBytes); err != nil {
130134
return err
131135
}
132-
v := value.Value{MapValue: pe.Key}
133-
v.WriteJSONStream(stream)
136+
stream.WriteObjectStart()
137+
for i, field := range *pe.Key {
138+
if i > 0 {
139+
stream.WriteMore()
140+
}
141+
stream.WriteObjectField(field.Name)
142+
field.Value.WriteJSONStream(stream)
143+
}
144+
stream.WriteObjectEnd()
134145
case pe.Value != nil:
135146
if _, err := stream.Write(peValueSepBytes); err != nil {
136147
return err

‎fieldpath/set_test.go

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

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

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

‎typed/helpers.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,7 @@ func keyedAssociativeListItemToPathElement(list *schema.List, index int, child v
205205
if child.MapValue == nil {
206206
return pe, errors.New("associative list with keys may not have non-map elements")
207207
}
208-
keyMap := &value.Map{}
208+
keyMap := value.FieldList{}
209209
for _, fieldName := range list.Keys {
210210
var fieldValue value.Value
211211
field, ok := child.MapValue.Get(fieldName)
@@ -215,9 +215,10 @@ func keyedAssociativeListItemToPathElement(list *schema.List, index int, child v
215215
// Treat keys as required.
216216
return pe, fmt.Errorf("associative list with keys has an element that omits key field %q", fieldName)
217217
}
218-
keyMap.Set(fieldName, fieldValue)
218+
keyMap = append(keyMap, value.Field{Name: fieldName, Value: fieldValue})
219219
}
220-
pe.Key = keyMap
220+
keyMap.Sort()
221+
pe.Key = &keyMap
221222
return pe, nil
222223
}
223224

‎value/value.go

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,59 @@ type Field struct {
129129
Value Value
130130
}
131131

132+
// FieldList is a list of key-value pairs. Each field is expected to
133+
// have a different name.
134+
type FieldList []Field
135+
136+
// Sort sorts the field list by Name.
137+
func (f FieldList) Sort() {
138+
if len(f) < 2 {
139+
return
140+
}
141+
if len(f) == 2 {
142+
if f[1].Name < f[0].Name {
143+
f[0], f[1] = f[1], f[0]
144+
}
145+
return
146+
}
147+
sort.SliceStable(f, func(i, j int) bool {
148+
return f[i].Name < f[j].Name
149+
})
150+
}
151+
152+
// Less compares two lists lexically.
153+
func (f FieldList) Less(rhs FieldList) bool {
154+
i := 0
155+
for {
156+
if i >= len(f) && i >= len(rhs) {
157+
// Maps are the same length and all items are equal.
158+
return false
159+
}
160+
if i >= len(f) {
161+
// F is shorter.
162+
return true
163+
}
164+
if i >= len(rhs) {
165+
// RHS is shorter.
166+
return false
167+
}
168+
if f[i].Name != rhs[i].Name {
169+
// the map having the field name that sorts lexically less is "less"
170+
return f[i].Name < rhs[i].Name
171+
}
172+
if f[i].Value.Less(rhs[i].Value) {
173+
// F is less; return
174+
return true
175+
}
176+
if rhs[i].Value.Less(f[i].Value) {
177+
// RHS is less; return
178+
return false
179+
}
180+
// The items are equal; continue.
181+
i++
182+
}
183+
}
184+
132185
// List is a list of items.
133186
type List struct {
134187
Items []Value

0 commit comments

Comments
 (0)
Please sign in to comment.