diff --git a/internal/gengapic/BUILD.bazel b/internal/gengapic/BUILD.bazel index 380e18aa..427ba60c 100644 --- a/internal/gengapic/BUILD.bazel +++ b/internal/gengapic/BUILD.bazel @@ -99,6 +99,7 @@ go_test( "@org_golang_google_protobuf//encoding/protojson", "@org_golang_google_protobuf//proto", "@org_golang_google_protobuf//reflect/protodesc", + "@org_golang_google_protobuf//reflect/protoreflect", "@org_golang_google_protobuf//runtime/protoiface", "@org_golang_google_protobuf//types/descriptorpb", "@org_golang_google_protobuf//types/known/apipb", diff --git a/internal/gengapic/client_init.go b/internal/gengapic/client_init.go index ceb936e0..8f35c618 100644 --- a/internal/gengapic/client_init.go +++ b/internal/gengapic/client_init.go @@ -116,9 +116,9 @@ func (g *generator) internalClientIntfInit(serv *descriptor.ServiceDescriptorPro case g.isLRO(m): // Unary call where the return type is a wrapper of // longrunning.Operation and more precise types - lroType := lroTypeName(m.GetName()) + lroType := lroTypeName(m) p("%s(context.Context, *%s.%s, ...gax.CallOption) (*%s, error)", - m.GetName(), inSpec.Name, inType.GetName(), lroTypeName(m.GetName())) + m.GetName(), inSpec.Name, inType.GetName(), lroType) p("%[1]s(name string) *%[1]s", lroType) case m.GetClientStreaming(): @@ -258,7 +258,7 @@ func (g *generator) genClientWrapperMethod(m *descriptor.MethodDescriptorProto, if g.isLRO(m) { reqTyp := fmt.Sprintf("%s.%s", inSpec.Name, inType.GetName()) - lroType := lroTypeName(m.GetName()) + lroType := lroTypeName(m) p("func (c *%s) %s(ctx context.Context, req *%s, opts ...gax.CallOption) (*%s, error) {", clientTypeName, m.GetName(), reqTyp, lroType) p(" return c.internalClient.%s(ctx, req, opts...)", m.GetName()) diff --git a/internal/gengapic/client_init_test.go b/internal/gengapic/client_init_test.go index 8f096d35..9b10a86e 100644 --- a/internal/gengapic/client_init_test.go +++ b/internal/gengapic/client_init_test.go @@ -33,6 +33,7 @@ import ( code "google.golang.org/genproto/googleapis/rpc/code" "google.golang.org/protobuf/encoding/protojson" "google.golang.org/protobuf/proto" + "google.golang.org/protobuf/reflect/protoreflect" "google.golang.org/protobuf/runtime/protoiface" "google.golang.org/protobuf/types/known/apipb" duration "google.golang.org/protobuf/types/known/durationpb" @@ -361,7 +362,6 @@ func TestClientInit(t *testing.T) { } customOpOpts := &descriptor.MethodOptions{} - proto.SetExtension(customOpOpts, extendedops.E_OperationService, opS.GetName()) servCustomOp := &descriptor.ServiceDescriptorProto{ Name: proto.String("Foo"), Method: []*descriptor.MethodDescriptorProto{ @@ -390,6 +390,7 @@ func TestClientInit(t *testing.T) { parameter *string imports map[pbinfo.ImportSpec]bool wantNumSnps int + setExt func() (protoreflect.ExtensionType, interface{}) }{ { tstName: "foo_client_init", @@ -498,8 +499,50 @@ func TestClientInit(t *testing.T) { {Name: "httptransport", Path: "google.golang.org/api/transport/http"}: true, }, wantNumSnps: 1, + setExt: func() (protoreflect.ExtensionType, interface{}) { + return extendedops.E_OperationService, opS.GetName() + }, + }, + { + tstName: "lro_client_conflict", + mixins: mixins{ + "google.longrunning.Operations": operationsMethods(), + }, + servName: "Foo", + serv: servLRO, + parameter: proto.String("go-gapic-package=path;mypackage"), + imports: map[pbinfo.ImportSpec]bool{ + {Name: "gtransport", Path: "google.golang.org/api/transport/grpc"}: true, + {Name: "longrunningpb", Path: "cloud.google.com/go/longrunning/autogen/longrunningpb"}: true, + {Name: "lroauto", Path: "cloud.google.com/go/longrunning/autogen"}: true, + {Name: "mypackagepb", Path: "github.com/googleapis/mypackage"}: true, + {Path: "context"}: true, + {Path: "google.golang.org/api/option"}: true, + {Path: "google.golang.org/grpc"}: true, + }, + wantNumSnps: 6, + setExt: func() (protoreflect.ExtensionType, interface{}) { + return annotations.E_Http, &annotations.HttpRule{ + Pattern: &annotations.HttpRule_Post{ + Post: "/v1beta1/{parent=projects/*/locations/*/featureGroups/*}/features", + }, + } + }, }, } { + setExt := tst.setExt + if setExt == nil { + setExt = func() (protoreflect.ExtensionType, interface{}) { + return annotations.E_Http, &annotations.HttpRule{ + Pattern: &annotations.HttpRule_Get{ + Get: "/zip", + }, + } + } + } + ext, value := setExt() + proto.SetExtension(tst.serv.Method[0].GetOptions(), ext, value) + fds := append(mixinDescriptors(), &descriptor.FileDescriptorProto{ Package: proto.String("mypackage"), Options: &descriptor.FileOptions{ diff --git a/internal/gengapic/genrest.go b/internal/gengapic/genrest.go index 938aa123..a442bac0 100644 --- a/internal/gengapic/genrest.go +++ b/internal/gengapic/genrest.go @@ -909,7 +909,7 @@ func (g *generator) lroRESTCall(servName string, m *descriptor.MethodDescriptorP } g.imports[outSpec] = true - opWrapperType := lroTypeName(m.GetName()) + opWrapperType := lroTypeName(m) p("func (c *%s) %s(ctx context.Context, req *%s.%s, opts ...gax.CallOption) (*%s, error) {", lowcaseServName, m.GetName(), inSpec.Name, inType.GetName(), opWrapperType) diff --git a/internal/gengapic/lro.go b/internal/gengapic/lro.go index de223cac..c0876a5e 100644 --- a/internal/gengapic/lro.go +++ b/internal/gengapic/lro.go @@ -21,6 +21,7 @@ import ( longrunning "cloud.google.com/go/longrunning/autogen/longrunningpb" "github.com/golang/protobuf/protoc-gen-go/descriptor" "github.com/googleapis/gapic-generator-go/internal/pbinfo" + "google.golang.org/genproto/googleapis/api/annotations" "google.golang.org/protobuf/proto" ) @@ -38,7 +39,7 @@ func (g *generator) lroCall(servName string, m *descriptor.MethodDescriptorProto return err } - lroType := lroTypeName(m.GetName()) + lroType := lroTypeName(m) p := g.printf lowcaseServName := lowerFirst(servName + "GRPCClient") @@ -74,7 +75,7 @@ func (g *generator) lroCall(servName string, m *descriptor.MethodDescriptorProto func (g *generator) lroType(servName string, serv *descriptor.ServiceDescriptorProto, m *descriptor.MethodDescriptorProto) error { protoPkg := g.descInfo.ParentFile[serv].GetPackage() mFQN := fmt.Sprintf("%s.%s.%s", protoPkg, serv.GetName(), m.GetName()) - lroType := lroTypeName(m.GetName()) + lroType := lroTypeName(m) p := g.printf hasREST := containsTransport(g.opts.transports, rest) @@ -280,6 +281,17 @@ func (g *generator) lroType(servName string, serv *descriptor.ServiceDescriptorP return nil } -func lroTypeName(methodName string) string { - return methodName + "Operation" +func lroTypeName(m *descriptor.MethodDescriptorProto) string { + // This whole if block is a hack to workaround a operation handler namespace + // collision. We should remove this in the future if the design is fixed for + // the v1 api. This is for aiplatform.featureregistryservice.createfeature. + if eHTTP, ok := proto.GetExtension(m.GetOptions(), annotations.E_Http).(*annotations.HttpRule); ok && eHTTP != nil && eHTTP.Pattern != nil { + switch t := eHTTP.Pattern.(type) { + case *annotations.HttpRule_Post: + if t.Post == "/v1beta1/{parent=projects/*/locations/*/featureGroups/*}/features" { + return m.GetName() + "RegistryOperation" + } + } + } + return m.GetName() + "Operation" } diff --git a/internal/gengapic/testdata/lro_client_conflict.want b/internal/gengapic/testdata/lro_client_conflict.want new file mode 100644 index 00000000..d46e0ac3 --- /dev/null +++ b/internal/gengapic/testdata/lro_client_conflict.want @@ -0,0 +1,178 @@ +// internalFooClient is an interface that defines the methods available from Awesome Foo API. +type internalFooClient interface { + Close() error + setGoogleClientInfo(...string) + Connection() *grpc.ClientConn + Zip(context.Context, *mypackagepb.Bar, ...gax.CallOption) (*ZipRegistryOperation, error) + ZipRegistryOperation(name string) *ZipRegistryOperation + ListOperations(context.Context, *longrunningpb.ListOperationsRequest, ...gax.CallOption) *OperationIterator + GetOperation(context.Context, *longrunningpb.GetOperationRequest, ...gax.CallOption) (*longrunningpb.Operation, error) + DeleteOperation(context.Context, *longrunningpb.DeleteOperationRequest, ...gax.CallOption) error + CancelOperation(context.Context, *longrunningpb.CancelOperationRequest, ...gax.CallOption) error + WaitOperation(context.Context, *longrunningpb.WaitOperationRequest, ...gax.CallOption) (*longrunningpb.Operation, error) +} + +// FooClient is a client for interacting with Awesome Foo API. +// Methods, except Close, may be called concurrently. However, fields must not be modified concurrently with method calls. +// +// Foo service does stuff. +type FooClient struct { + // The internal transport-dependent client. + internalClient internalFooClient + + // The call options for this service. + CallOptions *FooCallOptions + + // LROClient is used internally to handle long-running operations. + // It is exposed so that its CallOptions can be modified if required. + // Users should not Close this client. + LROClient *lroauto.OperationsClient + +} + +// Wrapper methods routed to the internal client. + +// Close closes the connection to the API service. The user should invoke this when +// the client is no longer required. +func (c *FooClient) Close() error { + return c.internalClient.Close() +} + +// setGoogleClientInfo sets the name and version of the application in +// the `x-goog-api-client` header passed on each request. Intended for +// use by Google-written clients. +func (c *FooClient) setGoogleClientInfo(keyval ...string) { + c.internalClient.setGoogleClientInfo(keyval...) +} + +// Connection returns a connection to the API service. +// +// Deprecated: Connections are now pooled so this method does not always +// return the same resource. +func (c *FooClient) Connection() *grpc.ClientConn { + return c.internalClient.Connection() +} + +// Zip does some stuff. +func (c *FooClient) Zip(ctx context.Context, req *mypackagepb.Bar, opts ...gax.CallOption) (*ZipRegistryOperation, error) { + return c.internalClient.Zip(ctx, req, opts...) +} + +// ZipRegistryOperation returns a new ZipRegistryOperation from a given name. +// The name must be that of a previously created ZipRegistryOperation, possibly from a different process. +func (c *FooClient) ZipRegistryOperation(name string) *ZipRegistryOperation { + return c.internalClient.ZipRegistryOperation(name) +} + +func (c *FooClient) ListOperations(ctx context.Context, req *longrunningpb.ListOperationsRequest, opts ...gax.CallOption) *OperationIterator { + return c.internalClient.ListOperations(ctx, req, opts...) +} + +func (c *FooClient) GetOperation(ctx context.Context, req *longrunningpb.GetOperationRequest, opts ...gax.CallOption) (*longrunningpb.Operation, error) { + return c.internalClient.GetOperation(ctx, req, opts...) +} + +func (c *FooClient) DeleteOperation(ctx context.Context, req *longrunningpb.DeleteOperationRequest, opts ...gax.CallOption) error { + return c.internalClient.DeleteOperation(ctx, req, opts...) +} + +func (c *FooClient) CancelOperation(ctx context.Context, req *longrunningpb.CancelOperationRequest, opts ...gax.CallOption) error { + return c.internalClient.CancelOperation(ctx, req, opts...) +} + +func (c *FooClient) WaitOperation(ctx context.Context, req *longrunningpb.WaitOperationRequest, opts ...gax.CallOption) (*longrunningpb.Operation, error) { + return c.internalClient.WaitOperation(ctx, req, opts...) +} + +// fooGRPCClient is a client for interacting with Awesome Foo API over gRPC transport. +// +// Methods, except Close, may be called concurrently. However, fields must not be modified concurrently with method calls. +type fooGRPCClient struct { + // Connection pool of gRPC connections to the service. + connPool gtransport.ConnPool + + // Points back to the CallOptions field of the containing FooClient + CallOptions **FooCallOptions + + // The gRPC API client. + fooClient mypackagepb.FooClient + + // LROClient is used internally to handle long-running operations. + // It is exposed so that its CallOptions can be modified if required. + // Users should not Close this client. + LROClient **lroauto.OperationsClient + + operationsClient longrunningpb.OperationsClient + + // The x-goog-* metadata to be sent with each request. + xGoogHeaders []string +} + +// NewFooClient creates a new foo client based on gRPC. +// The returned client must be Closed when it is done being used to clean up its underlying connections. +// +// Foo service does stuff. +func NewFooClient(ctx context.Context, opts ...option.ClientOption) (*FooClient, error) { + clientOpts := defaultFooGRPCClientOptions() + if newFooClientHook != nil { + hookOpts, err := newFooClientHook(ctx, clientHookParams{}) + if err != nil { + return nil, err + } + clientOpts = append(clientOpts, hookOpts...) + } + + connPool, err := gtransport.DialPool(ctx, append(clientOpts, opts...)...) + if err != nil { + return nil, err + } + client := FooClient{CallOptions: defaultFooCallOptions()} + + c := &fooGRPCClient{ + connPool: connPool, + fooClient: mypackagepb.NewFooClient(connPool), + CallOptions: &client.CallOptions, + operationsClient: longrunningpb.NewOperationsClient(connPool), + + } + c.setGoogleClientInfo() + + client.internalClient = c + + client.LROClient, err = lroauto.NewOperationsClient(ctx, gtransport.WithConnPool(connPool)) + if err != nil { + // This error "should not happen", since we are just reusing old connection pool + // and never actually need to dial. + // If this does happen, we could leak connp. However, we cannot close conn: + // If the user invoked the constructor with option.WithGRPCConn, + // we would close a connection that's still in use. + // TODO: investigate error conditions. + return nil, err + } + c.LROClient = &client.LROClient + return &client, nil +} + +// Connection returns a connection to the API service. +// +// Deprecated: Connections are now pooled so this method does not always +// return the same resource. +func (c *fooGRPCClient) Connection() *grpc.ClientConn { + return c.connPool.Conn() +} + +// setGoogleClientInfo sets the name and version of the application in +// the `x-goog-api-client` header passed on each request. Intended for +// use by Google-written clients. +func (c *fooGRPCClient) setGoogleClientInfo(keyval ...string) { + kv := append([]string{"gl-go", gax.GoVersion}, keyval...) + kv = append(kv, "gapic", getVersionClient(), "gax", gax.Version, "grpc", grpc.Version) + c.xGoogHeaders = []string{"x-goog-api-client", gax.XGoogHeader(kv...)} +} + +// Close closes the connection to the API service. The user should invoke this when +// the client is no longer required. +func (c *fooGRPCClient) Close() error { + return c.connPool.Close() +} +