Skip to content

Commit 46c6aa2

Browse files
toumorokoshitoumorokoshi
committed
feat(proto): adding json names
A new aep has merged that is trying to enforce consistency across proto and json field names (aep-dev/aeps#309). If not overwritten, the json field names will use camelCase, which violates this new guidance. Adding specific json names to ensure spec compliance.
1 parent 03de496 commit 46c6aa2

File tree

2 files changed

+104
-8
lines changed

2 files changed

+104
-8
lines changed

‎pkg/proto/proto_test.go‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
package proto
1414

1515
import (
16+
"fmt"
1617
"strings"
1718
"testing"
1819

@@ -211,3 +212,78 @@ func TestAddCustomMethodWithNilResponse(t *testing.T) {
211212
assert.NotNil(t, method.Options, "Method options should not be nil")
212213
assert.Equal(t, method.RespType.GetTypeName(), "google.protobuf.Empty", "Response type should be google.protobuf.Empty")
213214
}
215+
216+
func TestJsonNameLabels(t *testing.T) {
217+
// Create test API
218+
exampleAPI := api.ExampleAPI()
219+
220+
// Generate proto string
221+
protoString, err := APIToProtoString(exampleAPI, "example/testapi/v1")
222+
assert.NoError(t, err)
223+
assert.NotEmpty(t, protoString)
224+
225+
protoContent := string(protoString)
226+
t.Logf("Proto content: \n---\n%s\n---", protoContent)
227+
228+
// Test cases for expected json_name labels
229+
testCases := []struct {
230+
name string
231+
fieldName string
232+
expectedJsonName string
233+
}{
234+
{
235+
name: "Publisher title field",
236+
fieldName: "title",
237+
expectedJsonName: "title",
238+
},
239+
{
240+
name: "Publisher id field",
241+
fieldName: "id",
242+
expectedJsonName: "id",
243+
},
244+
{
245+
name: "Book name field",
246+
fieldName: "name",
247+
expectedJsonName: "name",
248+
},
249+
{
250+
name: "Book id field",
251+
fieldName: "id",
252+
expectedJsonName: "id",
253+
},
254+
{
255+
name: "Account name field",
256+
fieldName: "name",
257+
expectedJsonName: "name",
258+
},
259+
}
260+
261+
// Check that each field has the correct json_name label
262+
for _, tc := range testCases {
263+
t.Run(tc.name, func(t *testing.T) {
264+
// In protobuf, json_name is only shown when it differs from the field name
265+
// Since we're setting json_name to match the field name, it won't appear in the output
266+
// Instead, we should verify that the field exists
267+
assert.True(t,
268+
strings.Contains(protoContent, tc.fieldName),
269+
"Expected field %s not found in proto content", tc.fieldName)
270+
})
271+
}
272+
273+
// Test that fields that should have json_name labels actually have them
274+
// Based on the proto output, these fields have explicit json_name labels
275+
fieldsWithJsonName := []string{
276+
"page_token",
277+
"next_page_token",
278+
"max_page_size",
279+
"update_mask",
280+
}
281+
282+
for _, fieldName := range fieldsWithJsonName {
283+
t.Run(fmt.Sprintf("Field with json_name %s", fieldName), func(t *testing.T) {
284+
assert.True(t,
285+
strings.Contains(protoContent, fmt.Sprintf(`json_name = "%s"`, fieldName)),
286+
"Expected json_name label for field %s not found in proto content", fieldName)
287+
})
288+
}
289+
}

‎pkg/proto/resource.go‎

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,8 @@ func GenerateMessage(name string, s *openapi.Schema, a *api.API, m *MessageStora
177177
proto.SetExtension(o, annotations.E_FieldBehavior, []annotations.FieldBehavior{annotations.FieldBehavior_REQUIRED})
178178
f.SetOptions(o)
179179
}
180+
// Set the JSON name after any options are set to ensure it's not overwritten
181+
f.SetJsonName(name)
180182
mb.AddField(f)
181183
}
182184
return mb, nil
@@ -195,6 +197,7 @@ func protoField(name string, number int, s openapi.Schema, a *api.API, m *Messag
195197
if s.Type == "array" {
196198
f.SetRepeated()
197199
}
200+
198201
return f, nil
199202
}
200203

@@ -319,11 +322,13 @@ func AddUpdate(a *api.API, r *api.Resource, resMsg Message, fb *builder.FileBuil
319322
// TODO: find a way to get the actual field mask proto descriptor type, without
320323
// querying the global registry.
321324
fieldMaskDescriptor, _ := desc.LoadMessageDescriptorForType(reflect.TypeOf(fieldmaskpb.FieldMask{}))
322-
mb.AddField(builder.NewField(constants.FIELD_UPDATE_MASK_NAME, builder.FieldTypeImportedMessage(fieldMaskDescriptor)).
325+
updateMaskField := builder.NewField(constants.FIELD_UPDATE_MASK_NAME, builder.FieldTypeImportedMessage(fieldMaskDescriptor)).
323326
SetNumber(constants.FIELD_UPDATE_MASK_NUMBER).
324327
SetComments(builder.Comments{
325328
LeadingComment: "The update mask for the resource",
326-
}))
329+
})
330+
updateMaskField.SetJsonName(constants.FIELD_UPDATE_MASK_NAME)
331+
mb.AddField(updateMaskField)
327332
fb.AddMessage(mb)
328333
method := buildMethod(
329334
"Update"+toMessageName(r.Singular),
@@ -394,24 +399,30 @@ func AddList(r *api.Resource, resMsg Message, fb *builder.FileBuilder, sb *build
394399
})
395400
addParentField(r, reqMb)
396401
addPageToken(r, reqMb)
397-
reqMb.AddField(builder.NewField(constants.FIELD_MAX_PAGE_SIZE_NAME, builder.FieldTypeInt32()).
402+
maxPageSizeField := builder.NewField(constants.FIELD_MAX_PAGE_SIZE_NAME, builder.FieldTypeInt32()).
398403
SetNumber(constants.FIELD_MAX_PAGE_SIZE_NUMBER).
399404
SetComments(builder.Comments{
400405
LeadingComment: "The maximum number of resources to return in a single page.",
401-
}))
406+
})
407+
maxPageSizeField.SetJsonName(constants.FIELD_MAX_PAGE_SIZE_NAME)
408+
reqMb.AddField(maxPageSizeField)
402409
if r.Methods.List.SupportsSkip {
403-
reqMb.AddField(builder.NewField(constants.FIELD_SKIP_NAME, builder.FieldTypeInt32()).
410+
skipField := builder.NewField(constants.FIELD_SKIP_NAME, builder.FieldTypeInt32()).
404411
SetNumber(constants.FIELD_SKIP_NUMBER).
405412
SetComments(builder.Comments{
406413
LeadingComment: "The number of resources to skip before returning the first resource in the page.",
407-
}))
414+
})
415+
skipField.SetJsonName(constants.FIELD_SKIP_NAME)
416+
reqMb.AddField(skipField)
408417
}
409418
if r.Methods.List.SupportsFilter {
410-
reqMb.AddField(builder.NewField(constants.FIELD_FILTER_NAME, builder.FieldTypeString()).
419+
filterField := builder.NewField(constants.FIELD_FILTER_NAME, builder.FieldTypeString()).
411420
SetNumber(constants.FIELD_FILTER_NUMBER).
412421
SetComments(builder.Comments{
413422
LeadingComment: "The filter to apply to the list.",
414-
}))
423+
})
424+
filterField.SetJsonName(constants.FIELD_FILTER_NAME)
425+
reqMb.AddField(filterField)
415426
}
416427
fb.AddMessage(reqMb)
417428
respMb := builder.NewMessage("List" + toMessageName(r.Plural) + "Response")
@@ -424,6 +435,7 @@ func AddList(r *api.Resource, resMsg Message, fb *builder.FileBuilder, sb *build
424435
f := builder.NewField(constants.FIELD_UNREACHABLE_NAME, resMsg.FieldType()).SetNumber(constants.FIELD_UNREACHABLE_NUMBER).SetComments(builder.Comments{
425436
LeadingComment: fmt.Sprintf("A list of %v that were not reachable.", r.Plural),
426437
}).SetRepeated()
438+
f.SetJsonName(constants.FIELD_UNREACHABLE_NAME)
427439
respMb.AddField(f)
428440
}
429441
fb.AddMessage(respMb)
@@ -610,13 +622,15 @@ func addParentField(r *api.Resource, mb *builder.MessageBuilder) {
610622
LeadingComment: fmt.Sprintf("A field for the parent of %v", r.Singular),
611623
}).
612624
SetOptions(o)
625+
f.SetJsonName(constants.FIELD_PARENT_NAME)
613626
mb.AddField(f)
614627
}
615628

616629
func addIdField(_ *api.Resource, mb *builder.MessageBuilder) {
617630
f := builder.NewField(constants.FIELD_ID_NAME, builder.FieldTypeString()).SetNumber(constants.FIELD_ID_NUMBER).SetComments(builder.Comments{
618631
LeadingComment: "An id that uniquely identifies the resource within the collection",
619632
})
633+
f.SetJsonName(constants.FIELD_ID_NAME)
620634
mb.AddField(f)
621635
}
622636

@@ -632,6 +646,7 @@ func addPathField(a *api.API, r *api.Resource, mb *builder.MessageBuilder) {
632646
LeadingComment: "The globally unique identifier for the resource",
633647
}).
634648
SetOptions(o)
649+
f.SetJsonName(constants.FIELD_PATH_NAME)
635650
mb.AddField(f)
636651
}
637652

@@ -644,27 +659,31 @@ func addResourceField(r *api.Resource, resMsg Message, mb *builder.MessageBuilde
644659
LeadingComment: "The resource to perform the operation on.",
645660
}).
646661
SetOptions(o)
662+
f.SetJsonName(cases.KebabToSnakeCase(r.Singular))
647663
mb.AddField(f)
648664
}
649665

650666
func addResourcesField(r *api.Resource, resMsg Message, mb *builder.MessageBuilder) {
651667
f := builder.NewField(constants.FIELD_RESULTS_NAME, resMsg.FieldType()).SetNumber(constants.FIELD_RESULTS_NUMBER).SetComments(builder.Comments{
652668
LeadingComment: fmt.Sprintf("A list of %v", r.Plural),
653669
}).SetRepeated()
670+
f.SetJsonName(constants.FIELD_RESULTS_NAME)
654671
mb.AddField(f)
655672
}
656673

657674
func addPageToken(_ *api.Resource, mb *builder.MessageBuilder) {
658675
f := builder.NewField(constants.FIELD_PAGE_TOKEN_NAME, builder.FieldTypeString()).SetNumber(constants.FIELD_PAGE_TOKEN_NUMBER).SetComments(builder.Comments{
659676
LeadingComment: "The page token indicating the starting point of the page",
660677
})
678+
f.SetJsonName(constants.FIELD_PAGE_TOKEN_NAME)
661679
mb.AddField(f)
662680
}
663681

664682
func addNextPageToken(_ *api.Resource, mb *builder.MessageBuilder) {
665683
f := builder.NewField(constants.FIELD_NEXT_PAGE_TOKEN_NAME, builder.FieldTypeString()).SetNumber(constants.FIELD_NEXT_PAGE_TOKEN_NUMBER).SetComments(builder.Comments{
666684
LeadingComment: "The page token indicating the ending point of this response.",
667685
})
686+
f.SetJsonName(constants.FIELD_NEXT_PAGE_TOKEN_NAME)
668687
mb.AddField(f)
669688
}
670689

@@ -677,6 +696,7 @@ func addForceField(_ *api.API, _ *api.Resource, mb *builder.MessageBuilder) {
677696
LeadingComment: "If true, the resource will be deleted, even if children still exist.",
678697
}).
679698
SetOptions(o)
699+
f.SetJsonName(constants.FIELD_FORCE_NAME)
680700
mb.AddField(f)
681701
}
682702

0 commit comments

Comments
 (0)