Skip to content

Commit c0c043b

Browse files
coryancoryan
authored
fix(sidekick): track the source service for mixins (#1821)
We need this to generate the right gRPC calls in Rust.
1 parent c590f20 commit c0c043b

File tree

9 files changed

+171
-119
lines changed

9 files changed

+171
-119
lines changed

‎internal/sidekick/internal/api/model.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,12 @@ type Method struct {
242242
// Service is the service this method belongs to, mustache templates use this field to
243243
// navigate the data structure.
244244
Service *Service
245+
// `SourceService` is the original service this method belongs to. For most
246+
// methods `SourceService` and `Service` are the same. For mixins, the
247+
// source service is the mixin, such as longrunning.Operations.
248+
SourceService *Service
249+
// `SourceServiceID` is the original service ID for this method.
250+
SourceServiceID string
245251
// Codec contains language specific annotations.
246252
Codec any
247253
}

‎internal/sidekick/internal/api/skip_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -362,7 +362,7 @@ func TestIncludeMethods(t *testing.T) {
362362
ID: ".test.Service1.Method2",
363363
},
364364
}
365-
if diff := cmp.Diff(wantMethods, s1.Methods, cmpopts.IgnoreFields(Method{}, "Model", "Service", "InputType", "OutputType", "InputTypeID", "OutputTypeID")); diff != "" {
365+
if diff := cmp.Diff(wantMethods, s1.Methods, cmpopts.IgnoreFields(Method{}, "Model", "Service", "SourceService", "InputType", "OutputType", "InputTypeID", "OutputTypeID")); diff != "" {
366366
t.Errorf("mismatch in methods (-want, +got)\n:%s", diff)
367367
}
368368
}

‎internal/sidekick/internal/api/xref.go‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,14 @@ func CrossReference(model *API) error {
5555
for _, m := range s.Methods {
5656
m.Model = model
5757
m.Service = s
58+
source, ok := model.State.ServiceByID[m.SourceServiceID]
59+
if ok {
60+
m.SourceService = source
61+
} else {
62+
// Default to the regular service. OpenAPI does not define the
63+
// services for mixins.
64+
m.SourceService = s
65+
}
5866
}
5967
}
6068
return nil

‎internal/sidekick/internal/api/xref_test.go‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,13 +87,25 @@ func TestCrossReferenceMethod(t *testing.T) {
8787
InputTypeID: ".test.Request",
8888
OutputTypeID: ".test.Response",
8989
}
90+
mixinMethod := &Method{
91+
Name: "GetOperation",
92+
ID: ".test.Service.GetOperation",
93+
SourceServiceID: ".google.longrunning.Operations",
94+
InputTypeID: ".test.Request",
95+
OutputTypeID: ".test.Response",
96+
}
9097
service := &Service{
9198
Name: "Service",
9299
ID: ".test.Service",
93-
Methods: []*Method{method},
100+
Methods: []*Method{method, mixinMethod},
101+
}
102+
mixinService := &Service{
103+
Name: "Operations",
104+
ID: ".google.longrunning.Operations",
105+
Methods: []*Method{},
94106
}
95107

96-
model := NewTestAPI([]*Message{request, response}, []*Enum{}, []*Service{service})
108+
model := NewTestAPI([]*Message{request, response}, []*Enum{}, []*Service{service, mixinService})
97109
if err := CrossReference(model); err != nil {
98110
t.Fatal(err)
99111
}

‎internal/sidekick/internal/parser/protobuf.go‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -262,7 +262,7 @@ func makeAPIForProtobuf(serviceConfig *serviceconfig.Service, req *pluginpb.Code
262262
service := processService(state, s, sFQN, f.GetPackage())
263263
for _, m := range s.Method {
264264
mFQN := sFQN + "." + m.GetName()
265-
if method := processMethod(state, m, mFQN, f.GetPackage()); method != nil {
265+
if method := processMethod(state, m, mFQN, f.GetPackage(), sFQN); method != nil {
266266
service.Methods = append(service.Methods, method)
267267
}
268268
}
@@ -322,7 +322,7 @@ func makeAPIForProtobuf(serviceConfig *serviceconfig.Service, req *pluginpb.Code
322322
// define the mixin method in the code.
323323
continue
324324
}
325-
if method := processMethod(state, m, mFQN, service.Package); method != nil {
325+
if method := processMethod(state, m, mFQN, service.Package, sFQN); method != nil {
326326
applyServiceConfigMethodOverrides(method, originalFQN, serviceConfig, result, mixin)
327327
service.Methods = append(service.Methods, method)
328328
}
@@ -443,7 +443,7 @@ func processService(state *api.APIState, s *descriptorpb.ServiceDescriptorProto,
443443
return service
444444
}
445445

446-
func processMethod(state *api.APIState, m *descriptorpb.MethodDescriptorProto, mFQN, packagez string) *api.Method {
446+
func processMethod(state *api.APIState, m *descriptorpb.MethodDescriptorProto, mFQN, packagez, serviceID string) *api.Method {
447447
pathInfo, err := parsePathInfo(m, state)
448448
if err != nil {
449449
slog.Error("unsupported http method", "method", m, "error", err)
@@ -467,6 +467,7 @@ func processMethod(state *api.APIState, m *descriptorpb.MethodDescriptorProto, m
467467
OperationInfo: parseOperationInfo(packagez, m),
468468
Routing: routing,
469469
ReturnsEmpty: outputTypeID == ".google.protobuf.Empty",
470+
SourceServiceID: serviceID,
470471
}
471472
state.MethodByID[mFQN] = method
472473
return method

‎internal/sidekick/internal/parser/protobuf_mixin_test.go‎

Lines changed: 31 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -66,11 +66,12 @@ func TestProtobuf_LocationMixin(t *testing.T) {
6666
}
6767

6868
checkMethod(t, service, "GetLocation", &api.Method{
69-
Documentation: "Provides the [Locations][google.cloud.location.Locations] service functionality in this service.",
70-
Name: "GetLocation",
71-
ID: ".test.TestService.GetLocation",
72-
InputTypeID: ".google.cloud.location.GetLocationRequest",
73-
OutputTypeID: ".google.cloud.location.Location",
69+
Documentation: "Provides the [Locations][google.cloud.location.Locations] service functionality in this service.",
70+
Name: "GetLocation",
71+
ID: ".test.TestService.GetLocation",
72+
SourceServiceID: ".google.cloud.location.Locations",
73+
InputTypeID: ".google.cloud.location.GetLocationRequest",
74+
OutputTypeID: ".google.cloud.location.Location",
7475
PathInfo: &api.PathInfo{
7576
Bindings: []*api.PathBinding{
7677
{
@@ -133,11 +134,12 @@ func TestProtobuf_IAMMixin(t *testing.T) {
133134
t.Fatal("Cannot find .test.TestService.GetIamPolicy")
134135
}
135136
checkMethod(t, service, "GetIamPolicy", &api.Method{
136-
Documentation: "Provides the [IAMPolicy][google.iam.v1.IAMPolicy] service functionality in this service.",
137-
Name: "GetIamPolicy",
138-
ID: ".test.TestService.GetIamPolicy",
139-
InputTypeID: ".google.iam.v1.GetIamPolicyRequest",
140-
OutputTypeID: ".google.iam.v1.Policy",
137+
Documentation: "Provides the [IAMPolicy][google.iam.v1.IAMPolicy] service functionality in this service.",
138+
Name: "GetIamPolicy",
139+
ID: ".test.TestService.GetIamPolicy",
140+
SourceServiceID: ".google.iam.v1.IAMPolicy",
141+
InputTypeID: ".google.iam.v1.GetIamPolicyRequest",
142+
OutputTypeID: ".google.iam.v1.Policy",
141143
PathInfo: &api.PathInfo{
142144
Bindings: []*api.PathBinding{
143145
{
@@ -206,11 +208,12 @@ func TestProtobuf_OperationMixin(t *testing.T) {
206208
}
207209

208210
checkMethod(t, service, "GetOperation", &api.Method{
209-
Documentation: "Custom docs.",
210-
Name: "GetOperation",
211-
ID: ".test.TestService.GetOperation",
212-
InputTypeID: ".google.longrunning.GetOperationRequest",
213-
OutputTypeID: ".google.longrunning.Operation",
211+
Documentation: "Custom docs.",
212+
Name: "GetOperation",
213+
ID: ".test.TestService.GetOperation",
214+
SourceServiceID: ".google.longrunning.Operations",
215+
InputTypeID: ".google.longrunning.GetOperationRequest",
216+
OutputTypeID: ".google.longrunning.Operation",
214217
PathInfo: &api.PathInfo{
215218
Bindings: []*api.PathBinding{
216219
{
@@ -289,12 +292,13 @@ func TestProtobuf_OperationMixinNoEmpty(t *testing.T) {
289292
}
290293

291294
checkMethod(t, service, "CancelOperation", &api.Method{
292-
Documentation: "Custom docs.",
293-
Name: "CancelOperation",
294-
ID: ".test.TestService.CancelOperation",
295-
InputTypeID: ".google.longrunning.CancelOperationRequest",
296-
OutputTypeID: ".google.protobuf.Empty",
297-
ReturnsEmpty: true,
295+
Documentation: "Custom docs.",
296+
Name: "CancelOperation",
297+
ID: ".test.TestService.CancelOperation",
298+
SourceServiceID: ".google.longrunning.Operations",
299+
InputTypeID: ".google.longrunning.CancelOperationRequest",
300+
OutputTypeID: ".google.protobuf.Empty",
301+
ReturnsEmpty: true,
298302
PathInfo: &api.PathInfo{
299303
Bindings: []*api.PathBinding{
300304
{
@@ -371,11 +375,12 @@ func TestProtobuf_DuplicateMixin(t *testing.T) {
371375
}
372376

373377
checkMethod(t, service, "GetOperation", &api.Method{
374-
Documentation: "Source file docs.",
375-
Name: "GetOperation",
376-
ID: ".test.LroService.GetOperation",
377-
InputTypeID: ".google.longrunning.GetOperationRequest",
378-
OutputTypeID: ".google.longrunning.Operation",
378+
Documentation: "Source file docs.",
379+
Name: "GetOperation",
380+
ID: ".test.LroService.GetOperation",
381+
SourceServiceID: ".test.LroService",
382+
InputTypeID: ".google.longrunning.GetOperationRequest",
383+
OutputTypeID: ".google.longrunning.Operation",
379384
PathInfo: &api.PathInfo{
380385
Bindings: []*api.PathBinding{
381386
{

0 commit comments

Comments
 (0)