Skip to content

Commit cb430fa

Browse files
coryancoryan
authored
cleanup(sidekick): consistent management of WKT (#2258)
All the current service specifications (Protobuf, OpenAPI, Discovery) use the same WKT (well-known types). In Protobuf the proto descriptors of the WKT are loaded by the `import "google/protobuf/...";` directives. In other specifications, the types need to be defined by the parser. Likewise, in tests we need to define these types ahead of time.
1 parent 2e3e55a commit cb430fa

File tree

10 files changed

+105
-123
lines changed

10 files changed

+105
-123
lines changed

‎internal/sidekick/internal/api/test.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,14 +59,16 @@ func NewTestAPI(messages []*Message, enums []*Enum, services []*Service) *API {
5959
}
6060
}
6161

62-
return &API{
62+
model := &API{
6363
Name: "Test",
6464
PackageName: packageName,
6565
Messages: messages,
6666
Enums: enums,
6767
Services: services,
6868
State: state,
6969
}
70+
model.LoadWellKnownTypes()
71+
return model
7072
}
7173

7274
// parentName returns the parent's name from a fully qualified identifier.

‎internal/sidekick/internal/api/well_known_types.go‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
// Copyright 2025 Google LLC
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// https://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package api
16+
17+
// LoadWellKnownTypes adds well-known types to `state`.
18+
//
19+
// Some source specification formats (Discovery, OpenAPI) must manually add the
20+
// well-known types. In Protobuf these types are automatically defined in the
21+
// protoc output.
22+
func (model *API) LoadWellKnownTypes() {
23+
for _, message := range wellKnownMessages {
24+
model.State.MessageByID[message.ID] = message
25+
}
26+
model.State.EnumByID[".google.protobuf.NullValue"] = &Enum{
27+
Name: "NullValue",
28+
Package: "google.protobuf",
29+
ID: ".google.protobuf.NullValue",
30+
}
31+
}
32+
33+
var wellKnownMessages = []*Message{
34+
{
35+
ID: ".google.protobuf.Any",
36+
Name: "Any",
37+
Package: "google.protobuf",
38+
},
39+
{
40+
ID: ".google.protobuf.Struct",
41+
Name: "Struct",
42+
Package: "google.protobuf",
43+
},
44+
{
45+
ID: ".google.protobuf.Value",
46+
Name: "Value",
47+
Package: "google.protobuf",
48+
},
49+
{
50+
ID: ".google.protobuf.ListValue",
51+
Name: "ListValue",
52+
Package: "google.protobuf",
53+
},
54+
{
55+
ID: ".google.protobuf.Empty",
56+
Name: "Empty",
57+
Package: "google.protobuf",
58+
},
59+
{
60+
ID: ".google.protobuf.FieldMask",
61+
Name: "FieldMask",
62+
Package: "google.protobuf",
63+
Fields: []*Field{
64+
{
65+
Name: "paths",
66+
JSONName: "paths",
67+
Typez: STRING_TYPE,
68+
Repeated: true,
69+
},
70+
},
71+
},
72+
{
73+
ID: ".google.protobuf.Duration",
74+
Name: "Duration",
75+
Package: "google.protobuf",
76+
},
77+
{
78+
ID: ".google.protobuf.Timestamp",
79+
Name: "Timestamp",
80+
Package: "google.protobuf",
81+
},
82+
{ID: ".google.protobuf.BytesValue", Name: "BytesValue", Package: "google.protobuf"},
83+
{ID: ".google.protobuf.UInt64Value", Name: "UInt64Value", Package: "google.protobuf"},
84+
{ID: ".google.protobuf.Int64Value", Name: "Int64Value", Package: "google.protobuf"},
85+
{ID: ".google.protobuf.UInt32Value", Name: "UInt32Value", Package: "google.protobuf"},
86+
{ID: ".google.protobuf.Int32Value", Name: "Int32Value", Package: "google.protobuf"},
87+
{ID: ".google.protobuf.FloatValue", Name: "FloatValue", Package: "google.protobuf"},
88+
{ID: ".google.protobuf.DoubleValue", Name: "DoubleValue", Package: "google.protobuf"},
89+
{ID: ".google.protobuf.BoolValue", Name: "BoolValue", Package: "google.protobuf"},
90+
}

‎internal/sidekick/internal/dart/annotate_test.go‎

Lines changed: 1 addition & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -410,7 +410,7 @@ func TestBuildQueryLinesMessages(t *testing.T) {
410410
payload := sample.SecretPayload()
411411
model := api.NewTestAPI(
412412
[]*api.Message{r, a, sample.CustomerManagedEncryption(), secretVersion,
413-
updateRequest, sample.Secret(), fieldMaskMessage(), payload},
413+
updateRequest, sample.Secret(), payload},
414414
[]*api.Enum{sample.EnumState()},
415415
[]*api.Service{})
416416
model.PackageName = "test"
@@ -465,19 +465,3 @@ func TestBuildQueryLinesMessages(t *testing.T) {
465465
t.Errorf("mismatch in TestBuildQueryLines (-want, +got)\n:%s", diff)
466466
}
467467
}
468-
469-
func fieldMaskMessage() *api.Message {
470-
return &api.Message{
471-
Name: "FieldMask",
472-
ID: ".google.protobuf.FieldMask",
473-
Package: sample.Package,
474-
Fields: []*api.Field{
475-
{
476-
Name: "paths",
477-
JSONName: "paths",
478-
Typez: api.STRING_TYPE,
479-
Repeated: true,
480-
},
481-
},
482-
}
483-
}

‎internal/sidekick/internal/parser/discovery/discovery.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,11 @@ func NewAPI(serviceConfig *serviceconfig.Service, contents []byte) (*api.API, er
4545
EnumByID: make(map[string]*api.Enum),
4646
},
4747
}
48+
// Discovery docs use some well-known types inspired by Protobuf. With
49+
// protoc these types are automatically included via `import` statements.
50+
// In the Discovery docs JSON inputs, these types are not automatically
51+
// included.
52+
result.LoadWellKnownTypes()
4853

4954
// Discovery docs do not define a service name or package name. The service
5055
// config may provide one.

‎internal/sidekick/internal/parser/openapi.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,11 @@ func makeAPIForOpenAPI(serviceConfig *serviceconfig.Service, model *libopenapi.D
7979
EnumByID: make(map[string]*api.Enum),
8080
},
8181
}
82+
// OpenAPI (for Google) uses some well-known types inspired by Protobuf.
83+
// With protoc these types are automatically included via `import`
84+
// statements. In the OpenAPI JSON inputs, these types are not automatically
85+
// included.
86+
result.LoadWellKnownTypes()
8287

8388
if serviceConfig != nil {
8489
result.Name = strings.TrimSuffix(serviceConfig.Name, ".googleapis.com")

‎internal/sidekick/internal/rust/annotate.go‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -450,7 +450,6 @@ type enumValueAnnotation struct {
450450
func annotateModel(model *api.API, codec *codec) *modelAnnotations {
451451
codec.hasServices = len(model.State.ServiceByID) > 0
452452

453-
loadWellKnownTypes(model.State)
454453
resolveUsedPackages(model, codec.extraPackages)
455454
// Only annotate enums and messages that we intend to generate. In the
456455
// process we discover the external dependencies and trim the list of

‎internal/sidekick/internal/rust/annotate_test.go‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,6 @@ func serviceAnnotationsModel() *api.API {
140140
[]*api.Message{request, response},
141141
[]*api.Enum{usedEnum, extraEnum},
142142
[]*api.Service{service})
143-
loadWellKnownTypes(model.State)
144143
api.CrossReference(model)
145144
return model
146145
}
@@ -1141,7 +1140,6 @@ func TestWrapperFieldAnnotations(t *testing.T) {
11411140
Fields: []*api.Field{singular_field},
11421141
}
11431142
model := api.NewTestAPI([]*api.Message{message}, []*api.Enum{}, []*api.Service{})
1144-
loadWellKnownTypes(model.State)
11451143
api.CrossReference(model)
11461144
api.LabelRecursiveFields(model)
11471145
codec := createRustCodec()

‎internal/sidekick/internal/rust/codec.go‎

Lines changed: 0 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -284,68 +284,6 @@ type packagez struct {
284284
usedIf []string
285285
}
286286

287-
var wellKnownMessages = []*api.Message{
288-
{
289-
ID: ".google.protobuf.Any",
290-
Name: "Any",
291-
Package: "google.protobuf",
292-
},
293-
{
294-
ID: ".google.protobuf.Struct",
295-
Name: "Struct",
296-
Package: "google.protobuf",
297-
},
298-
{
299-
ID: ".google.protobuf.Value",
300-
Name: "Value",
301-
Package: "google.protobuf",
302-
},
303-
{
304-
ID: ".google.protobuf.ListValue",
305-
Name: "ListValue",
306-
Package: "google.protobuf",
307-
},
308-
{
309-
ID: ".google.protobuf.Empty",
310-
Name: "Empty",
311-
Package: "google.protobuf",
312-
},
313-
{
314-
ID: ".google.protobuf.FieldMask",
315-
Name: "FieldMask",
316-
Package: "google.protobuf",
317-
},
318-
{
319-
ID: ".google.protobuf.Duration",
320-
Name: "Duration",
321-
Package: "google.protobuf",
322-
},
323-
{
324-
ID: ".google.protobuf.Timestamp",
325-
Name: "Timestamp",
326-
Package: "google.protobuf",
327-
},
328-
{ID: ".google.protobuf.BytesValue", Name: "BytesValue", Package: "google.protobuf"},
329-
{ID: ".google.protobuf.UInt64Value", Name: "UInt64Value", Package: "google.protobuf"},
330-
{ID: ".google.protobuf.Int64Value", Name: "Int64Value", Package: "google.protobuf"},
331-
{ID: ".google.protobuf.UInt32Value", Name: "UInt32Value", Package: "google.protobuf"},
332-
{ID: ".google.protobuf.Int32Value", Name: "Int32Value", Package: "google.protobuf"},
333-
{ID: ".google.protobuf.FloatValue", Name: "FloatValue", Package: "google.protobuf"},
334-
{ID: ".google.protobuf.DoubleValue", Name: "DoubleValue", Package: "google.protobuf"},
335-
{ID: ".google.protobuf.BoolValue", Name: "BoolValue", Package: "google.protobuf"},
336-
}
337-
338-
func loadWellKnownTypes(s *api.APIState) {
339-
for _, message := range wellKnownMessages {
340-
s.MessageByID[message.ID] = message
341-
}
342-
s.EnumByID[".google.protobuf.NullValue"] = &api.Enum{
343-
Name: "NullValue",
344-
Package: "google.protobuf",
345-
ID: ".google.protobuf.NullValue",
346-
}
347-
}
348-
349287
func resolveUsedPackages(model *api.API, extraPackages []*packagez) {
350288
hasServices := len(model.State.ServiceByID) > 0
351289
hasLROs := false

‎internal/sidekick/internal/rust/codec_test.go‎

Lines changed: 1 addition & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -283,7 +283,6 @@ func checkRustPackages(t *testing.T, got *codec, want *codec) {
283283

284284
func TestWellKnownTypesExist(t *testing.T) {
285285
model := api.NewTestAPI([]*api.Message{}, []*api.Enum{}, []*api.Service{})
286-
loadWellKnownTypes(model.State)
287286
for _, name := range []string{"Any", "Duration", "Empty", "FieldMask", "Timestamp"} {
288287
if _, ok := model.State.MessageByID[fmt.Sprintf(".google.protobuf.%s", name)]; !ok {
289288
t.Errorf("cannot find well-known message %s in API", name)
@@ -294,7 +293,6 @@ func TestWellKnownTypesExist(t *testing.T) {
294293
func TestWellKnownTypesAsMethod(t *testing.T) {
295294
model := api.NewTestAPI([]*api.Message{}, []*api.Enum{}, []*api.Service{})
296295
c := createRustCodec()
297-
loadWellKnownTypes(model.State)
298296

299297
want := "wkt::Empty"
300298
got := c.methodInOutTypeName(".google.protobuf.Empty", model.State, model.PackageName)
@@ -371,7 +369,6 @@ func TestMethodInOut(t *testing.T) {
371369
}
372370
model := api.NewTestAPI([]*api.Message{message, nested}, []*api.Enum{}, []*api.Service{})
373371
c := createRustCodec()
374-
loadWellKnownTypes(model.State)
375372

376373
want := "crate::model::Target"
377374
got := c.methodInOutTypeName("..Target", model.State, model.PackageName)
@@ -534,7 +531,6 @@ func TestFieldType(t *testing.T) {
534531
"f_map": "std::collections::HashMap<i32,i32>",
535532
}
536533
c := createRustCodec()
537-
loadWellKnownTypes(model.State)
538534
for _, field := range message.Fields {
539535
want, ok := expectedTypes[field.Name]
540536
if !ok {
@@ -579,7 +575,6 @@ func TestOneOfFieldType(t *testing.T) {
579575
"f_map": "std::collections::HashMap<i32,i32>",
580576
}
581577
c := createRustCodec()
582-
loadWellKnownTypes(model.State)
583578
for _, field := range message.Fields {
584579
want, ok := expectedTypes[field.Name]
585580
if !ok {
@@ -655,7 +650,6 @@ func TestFieldMapTypeValues(t *testing.T) {
655650
model := api.NewTestAPI([]*api.Message{message, other_message, map_thing}, []*api.Enum{}, []*api.Service{})
656651
api.LabelRecursiveFields(model)
657652
c := createRustCodec()
658-
loadWellKnownTypes(model.State)
659653
got := fieldType(field, model.State, false, c.modulePath, model.PackageName, c.packageMapping)
660654
if got != test.want {
661655
t.Errorf("mismatched field type for %s, got=%s, want=%s", field.Name, got, test.want)
@@ -716,7 +710,6 @@ func TestFieldMapTypeKey(t *testing.T) {
716710
model := api.NewTestAPI([]*api.Message{message, map_thing}, []*api.Enum{enum}, []*api.Service{})
717711
api.LabelRecursiveFields(model)
718712
c := createRustCodec()
719-
loadWellKnownTypes(model.State)
720713
got := fieldType(field, model.State, false, c.modulePath, model.PackageName, c.packageMapping)
721714
if got != test.want {
722715
t.Errorf("mismatched field type for %s, got=%s, want=%s", field.Name, got, test.want)
@@ -725,16 +718,11 @@ func TestFieldMapTypeKey(t *testing.T) {
725718
}
726719

727720
func TestAsQueryParameter(t *testing.T) {
728-
options := &api.Message{
729-
Name: "Options",
730-
ID: "..Options",
731-
Fields: []*api.Field{},
732-
}
733721
optionsField := &api.Field{
734722
Name: "options_field",
735723
JSONName: "optionsField",
736724
Typez: api.MESSAGE_TYPE,
737-
TypezID: options.ID,
725+
TypezID: "..Options",
738726
Optional: true,
739727
}
740728
requiredField := &api.Field{
@@ -786,21 +774,6 @@ func TestAsQueryParameter(t *testing.T) {
786774
TypezID: ".google.protobuf.FieldMask",
787775
Optional: true,
788776
}
789-
request := &api.Message{
790-
Name: "TestRequest",
791-
ID: "..TestRequest",
792-
Fields: []*api.Field{
793-
optionsField,
794-
requiredField, optionalField, repeatedField,
795-
requiredEnumField, optionalEnumField, repeatedEnumField,
796-
requiredFieldMaskField, optionalFieldMaskField,
797-
},
798-
}
799-
model := api.NewTestAPI(
800-
[]*api.Message{options, request},
801-
[]*api.Enum{},
802-
[]*api.Service{})
803-
loadWellKnownTypes(model.State)
804777

805778
for _, test := range []struct {
806779
field *api.Field
@@ -901,7 +874,6 @@ func TestOneOfAsQueryParameter(t *testing.T) {
901874
[]*api.Enum{},
902875
[]*api.Service{})
903876
api.CrossReference(model)
904-
loadWellKnownTypes(model.State)
905877

906878
for _, test := range []struct {
907879
field *api.Field
@@ -1315,7 +1287,6 @@ func TestFormatDocCommentsCrossLinks(t *testing.T) {
13151287
// To test the mappings we need a fairly complex model.API instance. Create it
13161288
// in a separate function to make this more readable.
13171289
model := makeApiForRustFormatDocCommentsCrossLinks()
1318-
loadWellKnownTypes(model.State)
13191290

13201291
got := c.formatDocComments(input, "test-only-ID", model.State, []string{"test.v1"})
13211292
if diff := cmp.Diff(want, got); diff != "" {
@@ -1375,7 +1346,6 @@ func TestFormatDocCommentsRelativeCrossLinks(t *testing.T) {
13751346
// To test the mappings we need a fairly complex model.API instance. Create it
13761347
// in a separate function to make this more readable.
13771348
model := makeApiForRustFormatDocCommentsCrossLinks()
1378-
loadWellKnownTypes(model.State)
13791349

13801350
got := c.formatDocComments(input, "test-only-ID", model.State, []string{"test.v1"})
13811351
if diff := cmp.Diff(want, got); diff != "" {
@@ -1435,7 +1405,6 @@ implied enum value reference [SomeMessage.SomeEnum.ENUM_VALUE][]
14351405
// To test the mappings we need a fairly complex model.API instance. Create it
14361406
// in a separate function to make this more readable.
14371407
model := makeApiForRustFormatDocCommentsCrossLinks()
1438-
loadWellKnownTypes(model.State)
14391408

14401409
got := c.formatDocComments(input, "test-only-ID", model.State, []string{"test.v1.Message", "test.v1"})
14411410
if diff := cmp.Diff(want, got); diff != "" {
@@ -1659,7 +1628,6 @@ Hyperlink: <a href="https://hyperlink.com">Content</a>`
16591628
// To test the mappings we need a fairly complex model.API instance. Create it
16601629
// in a separate function to make this more readable.
16611630
model := makeApiForRustFormatDocCommentsCrossLinks()
1662-
loadWellKnownTypes(model.State)
16631631

16641632
got := c.formatDocComments(input, "test-only-ID", model.State, []string{})
16651633
if diff := cmp.Diff(want, got); diff != "" {

0 commit comments

Comments
 (0)