Skip to content

Commit a4a07dc

Browse files
committed
Apply feedback
1 parent cc58872 commit a4a07dc

File tree

5 files changed

+133
-38
lines changed

5 files changed

+133
-38
lines changed

typed/parser.go

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ func (p ParseableType) FromYAML(object YAMLObject) (*TypedValue, error) {
103103
}
104104

105105
// FromUnstructured converts a go "interface{}" type, typically an
106-
// unstructured object in Kubernetes world. to a TypedValue. It returns an
106+
// unstructured object in Kubernetes world, to a TypedValue. It returns an
107107
// error if the resulting object fails schema validation.
108108
// The provided interface{} must be one of: map[string]interface{},
109109
// map[interface{}]interface{}, []interface{}, int types, float types,
@@ -113,10 +113,9 @@ func (p ParseableType) FromUnstructured(in interface{}) (*TypedValue, error) {
113113
}
114114

115115
// FromStructured converts a go "interface{}" type, typically an structured object in
116-
// Kubernetes, to a TypedValue. It will return an
117-
// error if the resulting object fails schema validation.
118-
// The provided "interface{}" may contain structs and types that are converted to Values
119-
// // by the jsonMarshaler interface.
116+
// Kubernetes, to a TypedValue. It will return an error if the resulting object fails
117+
// schema validation. The provided "interface{}" may contain structs and types that are
118+
// converted to Values by the jsonMarshaler interface.
120119
func (p ParseableType) FromStructured(in interface{}) (*TypedValue, error) {
121120
v, err := value.NewValueReflect(in)
122121
if err != nil {

typed/validate_test.go

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -288,3 +288,98 @@ func TestSchemaSchema(t *testing.T) {
288288
t.Fatalf("failed to create schemaschema: %v", err)
289289
}
290290
}
291+
292+
func BenchmarkValidateStructured(b *testing.B) {
293+
type Primitives struct {
294+
s string
295+
i int64
296+
f float64
297+
b bool
298+
}
299+
300+
primitive1 := Primitives{s: "string1"}
301+
primitive2 := Primitives{i: 100}
302+
primitive3 := Primitives{f: 3.14}
303+
primitive4 := Primitives{b: true}
304+
305+
type Example struct {
306+
listOfPrimitives []Primitives
307+
mapOfPrimitives map[string]Primitives
308+
mapOfLists map[string][]Primitives
309+
}
310+
311+
tests := []struct {
312+
name string
313+
rootTypeName string
314+
schema typed.YAMLObject
315+
object interface{}
316+
}{
317+
{
318+
name: "struct",
319+
rootTypeName: "example",
320+
schema: `types:
321+
- name: example
322+
map:
323+
fields:
324+
- name: listOfPrimitives
325+
type:
326+
list:
327+
elementType:
328+
namedType: primitives
329+
- name: mapOfPrimitives
330+
type:
331+
map:
332+
elementType:
333+
namedType: primitives
334+
- name: mapOfLists
335+
type:
336+
map:
337+
elementType:
338+
list:
339+
elementType:
340+
namedType: primitives
341+
- name: primitives
342+
map:
343+
fields:
344+
- name: s
345+
type:
346+
scalar: string
347+
- name: i
348+
type:
349+
scalar: numeric
350+
- name: f
351+
type:
352+
scalar: numeric
353+
- name: b
354+
type:
355+
scalar: boolean
356+
`,
357+
object: Example{
358+
listOfPrimitives: []Primitives{primitive1, primitive2, primitive3, primitive4},
359+
mapOfPrimitives: map[string]Primitives{"1": primitive1, "2": primitive2, "3": primitive3, "4": primitive4},
360+
mapOfLists: map[string][]Primitives{
361+
"1": {primitive1, primitive2, primitive3, primitive4},
362+
"2": {primitive1, primitive2, primitive3, primitive4},
363+
},
364+
},
365+
},
366+
}
367+
368+
for _, test := range tests {
369+
b.Run(test.name, func(b *testing.B) {
370+
parser, err := typed.NewParser(test.schema)
371+
if err != nil {
372+
b.Fatalf("failed to create schema: %v", err)
373+
}
374+
pt := parser.Type(test.rootTypeName)
375+
376+
b.ReportAllocs()
377+
for n := 0; n < b.N; n++ {
378+
tv, err := pt.FromStructured(test.object)
379+
if err != nil {
380+
b.Errorf("failed to parse/validate yaml: %v\n%v", err, tv)
381+
}
382+
}
383+
})
384+
}
385+
}

value/mapreflect.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,7 @@ func (r mapReflect) Length() int {
2828
}
2929

3030
func (r mapReflect) Get(key string) (Value, bool) {
31-
var val reflect.Value
32-
val = r.Value.MapIndex(r.toMapKey(key))
31+
val := r.Value.MapIndex(r.toMapKey(key))
3332
if !val.IsValid() {
3433
return nil, false
3534
}
@@ -95,6 +94,8 @@ func (r mapReflect) Equals(m Map) bool {
9594
if r.Length() != m.Length() {
9695
return false
9796
}
97+
98+
// TODO: Optimize to avoid Iterate looping here by using r.Value.MapRange or similar if it improves performance.
9899
return m.Iterate(func(key string, value Value) bool {
99100
lhsVal, ok := r.Get(key)
100101
if !ok {

value/valuereflect.go

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ func safeIsNil(v reflect.Value) bool {
131131
return false
132132
}
133133

134-
func (r valueReflect) Map() Map {
134+
func (r valueReflect) AsMap() Map {
135135
val := r.Value
136136
switch val.Kind() {
137137
case reflect.Struct:
@@ -147,21 +147,21 @@ func (r *valueReflect) Recycle() {
147147
reflectPool.Put(r)
148148
}
149149

150-
func (r valueReflect) List() List {
150+
func (r valueReflect) AsList() List {
151151
if r.IsList() {
152152
return listReflect{r.Value}
153153
}
154154
panic("value is not a list")
155155
}
156156

157-
func (r valueReflect) Bool() bool {
157+
func (r valueReflect) AsBool() bool {
158158
if r.IsBool() {
159159
return r.Value.Bool()
160160
}
161161
panic("value is not a bool")
162162
}
163163

164-
func (r valueReflect) Int() int64 {
164+
func (r valueReflect) AsInt() int64 {
165165
if r.isKind(reflect.Int, reflect.Int64, reflect.Int32, reflect.Int16, reflect.Int8) {
166166
return r.Value.Int()
167167
}
@@ -172,14 +172,14 @@ func (r valueReflect) Int() int64 {
172172
panic("value is not an int")
173173
}
174174

175-
func (r valueReflect) Float() float64 {
175+
func (r valueReflect) AsFloat() float64 {
176176
if r.IsFloat() {
177177
return r.Value.Float()
178178
}
179179
panic("value is not a float")
180180
}
181181

182-
func (r valueReflect) String() string {
182+
func (r valueReflect) AsString() string {
183183
kind := r.Value.Kind()
184184
if kind == reflect.String {
185185
return r.Value.String()
@@ -202,13 +202,13 @@ func (r valueReflect) Unstructured() interface{} {
202202
case r.IsList():
203203
return listReflect{Value: r.Value}.Unstructured()
204204
case r.IsString():
205-
return r.String()
205+
return r.AsString()
206206
case r.IsInt():
207-
return r.Int()
207+
return r.AsInt()
208208
case r.IsBool():
209-
return r.Bool()
209+
return r.AsBool()
210210
case r.IsFloat():
211-
return r.Float()
211+
return r.AsFloat()
212212
default:
213213
panic(fmt.Sprintf("value of type %s is not a supported by value reflector", val.Type()))
214214
}

value/valuereflect_test.go

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ func TestReflectPrimitives(t *testing.T) {
3737
if !rv.IsString() {
3838
t.Error("expected IsString to be true")
3939
}
40-
if rv.String() != "string" {
40+
if rv.AsString() != "string" {
4141
t.Errorf("expected rv.String to be 'string' but got %v", rv.Unstructured())
4242
}
4343

@@ -49,39 +49,39 @@ func TestReflectPrimitives(t *testing.T) {
4949
t.Error("expected IsList to be false ([]byte is represented as a base64 encoded string)")
5050
}
5151
encoded := base64.StdEncoding.EncodeToString([]byte("string"))
52-
if rv.String() != encoded {
52+
if rv.AsString() != encoded {
5353
t.Errorf("expected rv.String to be %v but got %v", []byte(encoded), rv.Unstructured())
5454
}
5555

5656
rv = MustReflect(1)
5757
if !rv.IsInt() {
5858
t.Error("expected IsInt to be true")
5959
}
60-
if rv.Int() != 1 {
60+
if rv.AsInt() != 1 {
6161
t.Errorf("expected rv.Int to be 1 but got %v", rv.Unstructured())
6262
}
6363

6464
rv = MustReflect(uint32(3000000000))
6565
if !rv.IsInt() {
6666
t.Error("expected IsInt to be true")
6767
}
68-
if rv.Int() != 3000000000 {
68+
if rv.AsInt() != 3000000000 {
6969
t.Errorf("expected rv.Int to be 3000000000 but got %v", rv.Unstructured())
7070
}
7171

7272
rv = MustReflect(1.5)
7373
if !rv.IsFloat() {
7474
t.Error("expected IsFloat to be true")
7575
}
76-
if rv.Float() != 1.5 {
76+
if rv.AsFloat() != 1.5 {
7777
t.Errorf("expected rv.Float to be 1.1 but got %v", rv.Unstructured())
7878
}
7979

8080
rv = MustReflect(true)
8181
if !rv.IsBool() {
8282
t.Error("expected IsBool to be true")
8383
}
84-
if rv.Bool() != true {
84+
if rv.AsBool() != true {
8585
t.Errorf("expected rv.Bool to be true but got %v", rv.Unstructured())
8686
}
8787

@@ -152,8 +152,8 @@ func TestReflectCustomStringConversion(t *testing.T) {
152152
if !rv.IsString() {
153153
t.Fatalf("expected IsString to be true, but kind is: %T", rv.Unstructured())
154154
}
155-
if rv.String() != tc.expected {
156-
t.Errorf("expected rv.String to be %v but got %s", tc.expected, rv.String())
155+
if rv.AsString() != tc.expected {
156+
t.Errorf("expected rv.String to be %v but got %s", tc.expected, rv.AsString())
157157
}
158158
})
159159
}
@@ -165,8 +165,8 @@ func TestReflectPointers(t *testing.T) {
165165
if !rv.IsString() {
166166
t.Error("expected IsString to be true")
167167
}
168-
if rv.String() != "string" {
169-
t.Errorf("expected rv.String to be 'string' but got %s", rv.String())
168+
if rv.AsString() != "string" {
169+
t.Errorf("expected rv.String to be 'string' but got %s", rv.AsString())
170170
}
171171
}
172172

@@ -233,7 +233,7 @@ func TestReflectStruct(t *testing.T) {
233233
if !rv.IsMap() {
234234
t.Error("expected IsMap to be true")
235235
}
236-
m := rv.Map()
236+
m := rv.AsMap()
237237
if m.Length() != len(tc.expectedMap) {
238238
t.Errorf("expected map to be of length %d but got %d", len(tc.expectedMap), m.Length())
239239
}
@@ -266,12 +266,12 @@ func TestReflectStructMutate(t *testing.T) {
266266
if !rv.IsMap() {
267267
t.Error("expected IsMap to be true")
268268
}
269-
m := rv.Map()
269+
m := rv.AsMap()
270270
atKey1, ok := m.Get("key1")
271271
if !ok {
272272
t.Errorf("expected map.Get(key1) to be 1 but got !ok")
273273
}
274-
if atKey1.Int() != 1 {
274+
if atKey1.AsInt() != 1 {
275275
t.Errorf("expected map.Get(key1) to be 1 but got: %v", atKey1)
276276
}
277277
m.Set("key1", NewValueInterface(int64(2)))
@@ -323,13 +323,13 @@ func TestReflectMap(t *testing.T) {
323323
if !rv.IsMap() {
324324
t.Error("expected IsMap to be true")
325325
}
326-
m := rv.Map()
326+
m := rv.AsMap()
327327
if m.Length() != tc.length {
328328
t.Errorf("expected map to be of length %d but got %d", tc.length, m.Length())
329329
}
330330
iterateResult := map[string]interface{}{}
331331
m.Iterate(func(s string, value Value) bool {
332-
iterateResult[s] = value.String()
332+
iterateResult[s] = value.AsString()
333333
return true
334334
})
335335
if !reflect.DeepEqual(iterateResult, tc.expectedMap) {
@@ -348,12 +348,12 @@ func TestReflectMapMutate(t *testing.T) {
348348
if !rv.IsMap() {
349349
t.Error("expected IsMap to be true")
350350
}
351-
m := rv.Map()
351+
m := rv.AsMap()
352352
atKey1, ok := m.Get("key1")
353353
if !ok {
354354
t.Errorf("expected map.Get(key1) to be 'value1' but got !ok")
355355
}
356-
if atKey1.String() != "value1" {
356+
if atKey1.AsString() != "value1" {
357357
t.Errorf("expected map.Get(key1) to be 'value1' but got: %v", atKey1)
358358
}
359359
m.Set("key1", NewValueInterface("replacement"))
@@ -405,14 +405,14 @@ func TestReflectList(t *testing.T) {
405405
if !rv.IsList() {
406406
t.Error("expected IsList to be true")
407407
}
408-
m := rv.List()
408+
m := rv.AsList()
409409
if m.Length() != tc.length {
410410
t.Errorf("expected list to be of length %d but got %d", tc.length, m.Length())
411411
}
412412
l := m.Length()
413413
iterateResult := make([]interface{}, l)
414414
for i := 0; i < l; i++ {
415-
iterateResult[i] = m.At(i).String()
415+
iterateResult[i] = m.At(i).AsString()
416416
}
417417
if !reflect.DeepEqual(iterateResult, tc.expectedIterate) {
418418
t.Errorf("expected iterate to produce %#v but got %#v", tc.expectedIterate, iterateResult)
@@ -430,9 +430,9 @@ func TestReflectListAt(t *testing.T) {
430430
if !rv.IsList() {
431431
t.Error("expected IsList to be true")
432432
}
433-
list := rv.List()
433+
list := rv.AsList()
434434
atOne := list.At(1)
435-
if atOne.String() != "two" {
435+
if atOne.AsString() != "two" {
436436
t.Errorf("expected list.At(1) to be 'two' but got: %v", atOne)
437437
}
438438
}

0 commit comments

Comments
 (0)