Skip to content

Commit a8822f7

Browse files
yongruilink8s-publishing-bot
authored andcommitted
Add slice and map union member support with tests
- Code generator: use len() != 0 for slice/map member extractors instead of != nil, so empty slice/map are treated as "not set" - Add slice and map members to union test types (both discriminated and undiscriminated) - Add test coverage for nil vs empty, ratcheting, and nil oldObj with slice/map members Co-authored-by: Tim Hockin <thockin@google.com> Kubernetes-commit: 24c1eb237d7a01421831d3cff668bf6fe04589ad
1 parent 7dba2d0 commit a8822f7

2 files changed

Lines changed: 115 additions & 11 deletions

File tree

pkg/api/validate/union_test.go

Lines changed: 113 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -190,8 +190,10 @@ func TestDiscriminatedUnion(t *testing.T) {
190190
}
191191

192192
type testStruct struct {
193-
M1 *m1 `json:"m1"`
194-
M2 *m2 `json:"m2"`
193+
M1 *m1 `json:"m1"`
194+
M2 *m2 `json:"m2"`
195+
M3 []string `json:"m3"`
196+
M4 map[string]string `json:"m4"`
195197
}
196198

197199
type m1 struct{}
@@ -210,6 +212,18 @@ var extractors = []ExtractorFn[*testStruct, bool]{
210212
}
211213
return s.M2 != nil
212214
},
215+
func(s *testStruct) bool {
216+
if s == nil {
217+
return false
218+
}
219+
return len(s.M3) != 0
220+
},
221+
func(s *testStruct) bool {
222+
if s == nil {
223+
return false
224+
}
225+
return len(s.M4) != 0
226+
},
213227
}
214228

215229
func TestUnionRatcheting(t *testing.T) {
@@ -224,7 +238,7 @@ func TestUnionRatcheting(t *testing.T) {
224238
oldStruct: nil,
225239
newStruct: &testStruct{},
226240
expected: field.ErrorList{
227-
field.Invalid(nil, "", "must specify one of: `m1`, `m2`"),
241+
field.Invalid(nil, "", "must specify one of: `m1`, `m2`, `m3`, `m4`"),
228242
}.WithOrigin("union"),
229243
},
230244
{
@@ -253,14 +267,40 @@ func TestUnionRatcheting(t *testing.T) {
253267
M2: &m2{},
254268
},
255269
expected: field.ErrorList{
256-
field.Invalid(nil, "{m1, m2}", "must specify exactly one of: `m1`, `m2`"),
270+
field.Invalid(nil, "{m1, m2}", "must specify exactly one of: `m1`, `m2`, `m3`, `m4`"),
271+
}.WithOrigin("union"),
272+
},
273+
{
274+
name: "slice member ratcheting: unchanged membership",
275+
oldStruct: &testStruct{M3: []string{"a"}},
276+
newStruct: &testStruct{M3: []string{"b"}},
277+
},
278+
{
279+
name: "map member ratcheting: unchanged membership",
280+
oldStruct: &testStruct{M4: map[string]string{"k": "v1"}},
281+
newStruct: &testStruct{M4: map[string]string{"k": "v2"}},
282+
},
283+
{
284+
name: "empty slice is not set",
285+
oldStruct: nil,
286+
newStruct: &testStruct{M3: []string{}},
287+
expected: field.ErrorList{
288+
field.Invalid(nil, "", "must specify one of: `m1`, `m2`, `m3`, `m4`"),
289+
}.WithOrigin("union"),
290+
},
291+
{
292+
name: "empty map is not set",
293+
oldStruct: nil,
294+
newStruct: &testStruct{M4: map[string]string{}},
295+
expected: field.ErrorList{
296+
field.Invalid(nil, "", "must specify one of: `m1`, `m2`, `m3`, `m4`"),
257297
}.WithOrigin("union"),
258298
},
259299
}
260300

261301
for _, tc := range testCases {
262302
t.Run(tc.name, func(t *testing.T) {
263-
members := []UnionMember{NewUnionMember("m1"), NewUnionMember("m2")}
303+
members := []UnionMember{NewUnionMember("m1"), NewUnionMember("m2"), NewUnionMember("m3"), NewUnionMember("m4")}
264304
got := Union(context.Background(), operation.Operation{Type: operation.Update}, nil, tc.newStruct, tc.oldStruct,
265305
NewUnionMembership(members...), extractors...)
266306
if !reflect.DeepEqual(got, tc.expected) {
@@ -271,9 +311,11 @@ func TestUnionRatcheting(t *testing.T) {
271311
}
272312

273313
type testDiscriminatedStruct struct {
274-
D string `json:"d"`
275-
M1 *m1 `json:"m1"`
276-
M2 *m2 `json:"m2"`
314+
D string `json:"d"`
315+
M1 *m1 `json:"m1"`
316+
M2 *m2 `json:"m2"`
317+
M3 []string `json:"m3"`
318+
M4 map[string]string `json:"m4"`
277319
}
278320

279321
var testDiscriminatorExtractor = func(s *testDiscriminatedStruct) string {
@@ -295,6 +337,18 @@ var testDiscriminatedExtractors = []ExtractorFn[*testDiscriminatedStruct, bool]{
295337
}
296338
return s.M2 != nil
297339
},
340+
func(s *testDiscriminatedStruct) bool {
341+
if s == nil {
342+
return false
343+
}
344+
return len(s.M3) != 0
345+
},
346+
func(s *testDiscriminatedStruct) bool {
347+
if s == nil {
348+
return false
349+
}
350+
return len(s.M4) != 0
351+
},
298352
}
299353

300354
func TestDiscriminatedUnionRatcheting(t *testing.T) {
@@ -366,11 +420,61 @@ func TestDiscriminatedUnionRatcheting(t *testing.T) {
366420
field.Invalid(field.NewPath("m2"), "", "must be specified when `d` is \"m2\""),
367421
}.WithOrigin("union"),
368422
},
423+
{
424+
name: "slice member ratcheting: unchanged membership",
425+
oldStruct: &testDiscriminatedStruct{
426+
D: "m3",
427+
M3: []string{"a"},
428+
},
429+
newStruct: &testDiscriminatedStruct{
430+
D: "m3",
431+
M3: []string{"b"},
432+
},
433+
},
434+
{
435+
name: "map member ratcheting: unchanged membership",
436+
oldStruct: &testDiscriminatedStruct{
437+
D: "m4",
438+
M4: map[string]string{"k": "v1"},
439+
},
440+
newStruct: &testDiscriminatedStruct{
441+
D: "m4",
442+
M4: map[string]string{"k": "v2"},
443+
},
444+
},
445+
{
446+
name: "empty slice is not set",
447+
oldStruct: &testDiscriminatedStruct{
448+
D: "m3",
449+
M3: []string{"a"},
450+
},
451+
newStruct: &testDiscriminatedStruct{
452+
D: "m3",
453+
M3: []string{},
454+
},
455+
expected: field.ErrorList{
456+
field.Invalid(field.NewPath("m3"), "", "must be specified when `d` is \"m3\""),
457+
}.WithOrigin("union"),
458+
},
459+
{
460+
name: "empty map is not set",
461+
oldStruct: &testDiscriminatedStruct{
462+
D: "m4",
463+
M4: map[string]string{"k": "v"},
464+
},
465+
newStruct: &testDiscriminatedStruct{
466+
D: "m4",
467+
M4: map[string]string{},
468+
},
469+
expected: field.ErrorList{
470+
field.Invalid(field.NewPath("m4"), "", "must be specified when `d` is \"m4\""),
471+
}.WithOrigin("union"),
472+
},
369473
}
370474

371475
for _, tc := range testCases {
372476
t.Run(tc.name, func(t *testing.T) {
373-
members := []UnionMember{NewDiscriminatedUnionMember("m1", "m1"), NewDiscriminatedUnionMember("m2", "m2")}
477+
members := []UnionMember{NewDiscriminatedUnionMember("m1", "m1"), NewDiscriminatedUnionMember("m2", "m2"), NewDiscriminatedUnionMember("m3", "m3"), NewDiscriminatedUnionMember("m4", "m4")}
374478
got := DiscriminatedUnion(context.Background(), operation.Operation{Type: operation.Update}, nil, tc.newStruct, tc.oldStruct,
375479
NewDiscriminatedUnionMembership("d", members...), testDiscriminatorExtractor, testDiscriminatedExtractors...)
376480
if !reflect.DeepEqual(got, tc.expected) {

pkg/api/validate/zeroorone_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ func TestZeroOrOneOfUnionRatcheting(t *testing.T) {
119119
M2: &m2{},
120120
},
121121
expected: field.ErrorList{
122-
field.Invalid(nil, "{m1, m2}", "must specify at most one of: `m1`, `m2`").WithOrigin("zeroOrOneOf"),
122+
field.Invalid(nil, "{m1, m2}", "must specify at most one of: `m1`, `m2`, `m3`, `m4`").WithOrigin("zeroOrOneOf"),
123123
},
124124
},
125125
{
@@ -142,7 +142,7 @@ func TestZeroOrOneOfUnionRatcheting(t *testing.T) {
142142

143143
for _, tc := range testCases {
144144
t.Run(tc.name, func(t *testing.T) {
145-
members := []UnionMember{NewUnionMember("m1"), NewUnionMember("m2")}
145+
members := []UnionMember{NewUnionMember("m1"), NewUnionMember("m2"), NewUnionMember("m3"), NewUnionMember("m4")}
146146
got := ZeroOrOneOfUnion(context.Background(), operation.Operation{Type: operation.Update}, nil, tc.newStruct, tc.oldStruct,
147147
NewUnionMembership(members...), extractors...)
148148
if !reflect.DeepEqual(got, tc.expected) {

0 commit comments

Comments
 (0)