Skip to content

Commit ec9c827

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #587 from aramase/aramase/f/fix_generators_inline
fix(generators): detect inline fields structurally instead of matching ",inline" tag
2 parents 16be699 + 4d3c1d1 commit ec9c827

File tree

4 files changed

+170
-23
lines changed

4 files changed

+170
-23
lines changed

‎pkg/generators/openapi.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -223,8 +223,8 @@ func getReferableName(m *types.Member) string {
223223
}
224224

225225
func shouldInlineMembers(m *types.Member) bool {
226-
jsonTags := getJsonTags(m)
227-
return len(jsonTags) > 1 && jsonTags[1] == "inline"
226+
jsonTag, jsonTagExists := reflect.StructTag(m.Tags).Lookup("json")
227+
return m.Embedded && jsonTagExists && (jsonTag == "" || strings.HasPrefix(jsonTag, ","))
228228
}
229229

230230
type openAPITypeWriter struct {

‎pkg/generators/openapi_test.go‎

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -810,6 +810,65 @@ Required: []string{"String"},
810810
})
811811
}
812812

813+
func TestEmbeddedInlineStructWithEmptyJSONTag(t *testing.T) {
814+
inputFile := `
815+
package foo
816+
817+
// Nested is used as embedded inline struct field
818+
type Nested struct {
819+
// A simple string
820+
String string
821+
}
822+
823+
// Blah demonstrate a struct with embedded inline struct field using empty json tag.
824+
type Blah struct {
825+
// An embedded inline struct field with empty json tag
826+
Nested ` + "`" + `json:""` + "`" + `
827+
}`
828+
829+
packagestest.TestAll(t, func(t *testing.T, x packagestest.Exporter) {
830+
e := packagestest.Export(t, x, []packagestest.Module{{
831+
Name: "example.com/base/foo",
832+
Files: map[string]interface{}{
833+
"foo.go": inputFile,
834+
},
835+
}})
836+
defer e.Cleanup()
837+
838+
callErr, funcErr, callBuffer, funcBuffer, _ := testOpenAPITypeWriter(t, e.Config)
839+
if callErr != nil {
840+
t.Fatal(callErr)
841+
}
842+
if funcErr != nil {
843+
t.Fatal(funcErr)
844+
}
845+
assertEqual(t, callBuffer.String(),
846+
`"example.com/base/foo.Blah": schema_examplecom_base_foo_Blah(ref),`)
847+
assertEqual(t, funcBuffer.String(),
848+
`func schema_examplecom_base_foo_Blah(ref common.ReferenceCallback) common.OpenAPIDefinition {
849+
return common.OpenAPIDefinition{
850+
Schema: spec.Schema{
851+
SchemaProps: spec.SchemaProps{
852+
Description: "Blah demonstrate a struct with embedded inline struct field using empty json tag.",
853+
Type: []string{"object"},
854+
Properties: map[string]spec.Schema{
855+
"String": {
856+
SchemaProps: spec.SchemaProps{
857+
Description: "A simple string",
858+
Default: "",
859+
Type: []string{"string"},
860+
Format: "",
861+
},
862+
},
863+
},
864+
Required: []string{"String"},
865+
},
866+
},
867+
}
868+
}`)
869+
})
870+
}
871+
813872
func TestNestedMapString(t *testing.T) {
814873
inputFile := `
815874
package foo
@@ -3102,3 +3161,55 @@ func TestNestedMarkers(t *testing.T) {
31023161
})
31033162

31043163
}
3164+
3165+
func TestShouldInlineMembers(t *testing.T) {
3166+
tests := []struct {
3167+
name string
3168+
member types.Member
3169+
expected bool
3170+
}{
3171+
{
3172+
name: "embedded with ,inline tag",
3173+
member: types.Member{Name: "TypeMeta", Embedded: true, Tags: `json:",inline"`},
3174+
expected: true,
3175+
},
3176+
{
3177+
name: "embedded with empty json tag",
3178+
member: types.Member{Name: "TypeMeta", Embedded: true, Tags: `json:""`},
3179+
expected: true,
3180+
},
3181+
{
3182+
name: "embedded with ,inline,omitempty tag",
3183+
member: types.Member{Name: "ObjectMeta", Embedded: true, Tags: `json:",inline,omitempty"`},
3184+
expected: true,
3185+
},
3186+
{
3187+
name: "embedded with no json tag",
3188+
member: types.Member{Name: "TypeMeta", Embedded: true, Tags: ``},
3189+
expected: false,
3190+
},
3191+
{
3192+
name: "embedded with explicit name",
3193+
member: types.Member{Name: "TypeMeta", Embedded: true, Tags: `json:"typemeta"`},
3194+
expected: false,
3195+
},
3196+
{
3197+
name: "non-embedded with ,inline tag",
3198+
member: types.Member{Name: "TypeMeta", Embedded: false, Tags: `json:",inline"`},
3199+
expected: false,
3200+
},
3201+
{
3202+
name: "non-embedded with empty json tag",
3203+
member: types.Member{Name: "TypeMeta", Embedded: false, Tags: `json:""`},
3204+
expected: false,
3205+
},
3206+
}
3207+
3208+
for _, tc := range tests {
3209+
t.Run(tc.name, func(t *testing.T) {
3210+
if got := shouldInlineMembers(&tc.member); got != tc.expected {
3211+
t.Errorf("shouldInlineMembers(%+v) = %v, want %v", tc.member, got, tc.expected)
3212+
}
3213+
})
3214+
}
3215+
}

‎pkg/generators/rules/list_type_streaming_tags.go‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,14 @@ func (l *StreamingListTypeJSONTags) Validate(t *types.Type) ([]string, error) {
6262
for _, m := range t.Members {
6363
switch m.Name {
6464
case "TypeMeta":
65-
if reflect.StructTag(m.Tags).Get("json") != ",inline" {
65+
if !m.Embedded {
66+
// field must be embedded to inline
67+
fields = append(fields, "TypeMeta")
68+
} else if jsonTag, jsonTagExists := reflect.StructTag(m.Tags).Lookup("json"); !jsonTagExists {
69+
// field should declare a json tag to indicate it is serialized
70+
fields = append(fields, "TypeMeta")
71+
} else if jsonTag != "" && jsonTag != ",inline" {
72+
// expect a completely empty json tag (preferred) or an empty name segment and only an ,inline directive (previously used)
6673
fields = append(fields, "TypeMeta")
6774
}
6875
case "ListMeta":

‎pkg/generators/rules/list_type_streaming_tags_test.go‎

Lines changed: 49 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -119,13 +119,35 @@ func TestStreamingListTypeJSONTags(t *testing.T) {
119119
expectedFields []string
120120
}{
121121
{
122-
name: "simple list",
122+
name: "simple list with inline tag",
123123
t: &types.Type{
124124
Kind: types.Struct,
125125
Members: []types.Member{
126126
{
127-
Name: "TypeMeta",
128-
Tags: `json:",inline"`,
127+
Name: "TypeMeta",
128+
Embedded: true,
129+
Tags: `json:",inline"`,
130+
},
131+
{
132+
Name: "ListMeta",
133+
Tags: `json:"metadata,omitempty" protobuf:"bytes,1,opt,name=metadata"`,
134+
},
135+
{
136+
Name: "Items",
137+
Tags: `json:"items" protobuf:"bytes,2,rep,name=items"`,
138+
},
139+
},
140+
},
141+
},
142+
{
143+
name: "simple list with empty json tag",
144+
t: &types.Type{
145+
Kind: types.Struct,
146+
Members: []types.Member{
147+
{
148+
Name: "TypeMeta",
149+
Embedded: true,
150+
Tags: `json:""`,
129151
},
130152
{
131153
Name: "ListMeta",
@@ -175,8 +197,9 @@ func TestStreamingListTypeJSONTags(t *testing.T) {
175197
Kind: types.Struct,
176198
Members: []types.Member{
177199
{
178-
Name: "TypeMeta",
179-
Tags: `json:"typemeta"`, // subfield typemeta instead of inline
200+
Name: "TypeMeta",
201+
Embedded: true,
202+
Tags: `json:"typemeta"`, // subfield typemeta instead of inline
180203
},
181204
{
182205
Name: "ListMeta",
@@ -196,8 +219,9 @@ func TestStreamingListTypeJSONTags(t *testing.T) {
196219
Kind: types.Struct,
197220
Members: []types.Member{
198221
{
199-
Name: "TypeMeta",
200-
Tags: `json:",inline"`,
222+
Name: "TypeMeta",
223+
Embedded: true,
224+
Tags: `json:",inline"`,
201225
},
202226
{
203227
Name: "ListMeta",
@@ -212,13 +236,14 @@ func TestStreamingListTypeJSONTags(t *testing.T) {
212236
expectedFields: []string{"ListMeta"},
213237
},
214238
{
215-
name: "bad listmeta json tag",
239+
name: "bad items json tag",
216240
t: &types.Type{
217241
Kind: types.Struct,
218242
Members: []types.Member{
219243
{
220-
Name: "TypeMeta",
221-
Tags: `json:",inline"`,
244+
Name: "TypeMeta",
245+
Embedded: true,
246+
Tags: `json:",inline"`,
222247
},
223248
{
224249
Name: "ListMeta",
@@ -258,8 +283,9 @@ func TestStreamingListTypeProtoTags(t *testing.T) {
258283
Kind: types.Struct,
259284
Members: []types.Member{
260285
{
261-
Name: "TypeMeta",
262-
Tags: `json:",inline"`,
286+
Name: "TypeMeta",
287+
Embedded: true,
288+
Tags: `json:",inline"`,
263289
},
264290
{
265291
Name: "ListMeta",
@@ -309,8 +335,9 @@ func TestStreamingListTypeProtoTags(t *testing.T) {
309335
Kind: types.Struct,
310336
Members: []types.Member{
311337
{
312-
Name: "TypeMeta",
313-
Tags: `json:",inline" protobuf:"bytes,3,opt,name=typemeta"`, // Added protobuf tag
338+
Name: "TypeMeta",
339+
Embedded: true,
340+
Tags: `json:",inline" protobuf:"bytes,3,opt,name=typemeta"`, // Added protobuf tag
314341
},
315342
{
316343
Name: "ListMeta",
@@ -325,13 +352,14 @@ func TestStreamingListTypeProtoTags(t *testing.T) {
325352
expectedFields: []string{"TypeMeta"},
326353
},
327354
{
328-
name: "bad listmeta json tag",
355+
name: "bad listmeta proto tag",
329356
t: &types.Type{
330357
Kind: types.Struct,
331358
Members: []types.Member{
332359
{
333-
Name: "TypeMeta",
334-
Tags: `json:",inline"`,
360+
Name: "TypeMeta",
361+
Embedded: true,
362+
Tags: `json:",inline"`,
335363
},
336364
{
337365
Name: "ListMeta",
@@ -346,13 +374,14 @@ func TestStreamingListTypeProtoTags(t *testing.T) {
346374
expectedFields: []string{"ListMeta"},
347375
},
348376
{
349-
name: "bad listmeta json tag",
377+
name: "bad items proto tag",
350378
t: &types.Type{
351379
Kind: types.Struct,
352380
Members: []types.Member{
353381
{
354-
Name: "TypeMeta",
355-
Tags: `json:",inline"`,
382+
Name: "TypeMeta",
383+
Embedded: true,
384+
Tags: `json:",inline"`,
356385
},
357386
{
358387
Name: "ListMeta",

0 commit comments

Comments
 (0)