Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add interface abstraction primitives #577

Merged
merged 32 commits into from
Apr 26, 2021
Merged

feat: add interface abstraction primitives #577

Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
5d2ae4c
feat: add interface abstraction primitives
software-dov Dec 14, 2020
bcddb28
Fix errors, lint failures and remove duplicated code
software-dov Mar 31, 2021
fcfc073
Lint fix
software-dov Mar 31, 2021
07ae859
Name changes
software-dov Mar 31, 2021
bc2d75d
Remove changes to go.mod
software-dov Apr 5, 2021
c94aba6
Finish gutting existing client
software-dov Apr 8, 2021
1f6d86c
Update internal/gengapic/client_init.go
software-dov Apr 14, 2021
fe6505e
Integrate reviews
software-dov Apr 14, 2021
043c34e
Merge branch 'pimpl-interface' of https://github.com/software-dov/gap…
software-dov Apr 14, 2021
0fad7d6
fixup! Merge branch 'pimpl-interface' of https://github.com/software-…
software-dov Apr 14, 2021
f4b92cd
Fixes
software-dov Apr 14, 2021
628173e
s/[A,a]bstractClient/internalClient/g
software-dov Apr 14, 2021
6c67c3d
Comment cleanup
software-dov Apr 14, 2021
e2fcc6a
Fix
software-dov Apr 15, 2021
fe8750e
Pass through for internal client
software-dov Apr 16, 2021
c23a4a9
s/GrpcClientOptions/GRPCClientOptions/
software-dov Apr 16, 2021
07637f1
Correct the double pointer bug
software-dov Apr 21, 2021
24621c0
Integrate reviews
software-dov Apr 21, 2021
cd8be14
Fixes
software-dov Apr 22, 2021
c6f57bb
Fix golden tests
software-dov Apr 22, 2021
f897ea5
Final fix
software-dov Apr 22, 2021
2743692
Fix
software-dov Apr 22, 2021
122c73c
Last time, for real, really
software-dov Apr 22, 2021
dedce17
Churn
software-dov Apr 22, 2021
06ef835
setGoogleClientInfo
software-dov Apr 22, 2021
80fef7e
Churn
software-dov Apr 22, 2021
c02df12
Add Operation helper methods to interface
software-dov Apr 23, 2021
4104bed
Mixin methods
software-dov Apr 23, 2021
0e72a64
Tests
software-dov Apr 23, 2021
150c1ae
Integrate reviews
software-dov Apr 26, 2021
8215ad9
Comma comma comma comma comma chameleon!
software-dov Apr 26, 2021
3c7dec8
Merge branch 'master' into pimpl-interface
software-dov Apr 26, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
409 changes: 265 additions & 144 deletions internal/gengapic/client_init.go
View file
Open in desktop

Large diffs are not rendered by default.

52 changes: 39 additions & 13 deletions internal/gengapic/client_init_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (

"github.com/golang/protobuf/proto"
"github.com/golang/protobuf/protoc-gen-go/descriptor"
plugin "github.com/golang/protobuf/protoc-gen-go/plugin"
"github.com/golang/protobuf/ptypes/duration"
wrappers "github.com/golang/protobuf/ptypes/wrappers"
"github.com/google/go-cmp/cmp"
Expand All @@ -39,7 +40,7 @@ func TestClientHook(t *testing.T) {

g.clientHook("Foo")
got := g.pt.String()
want := "var newFooClientHook clientHook\n\n"
want := "var newFooGRPCClientHook clientHook\n\n"

if diff := cmp.Diff(got, want); diff != "" {
t.Errorf("clientHook() (-got,+want): %s", diff)
Expand Down Expand Up @@ -205,25 +206,25 @@ func TestClientInit(t *testing.T) {
var g generator
g.apiName = "Awesome Foo API"
g.imports = map[pbinfo.ImportSpec]bool{}
g.opts = &options{transports: []transport{grpc}}

servPlain := &descriptor.ServiceDescriptorProto{
Name: proto.String("Foo"),
Method: []*descriptor.MethodDescriptorProto{
{Name: proto.String("Zip"), OutputType: proto.String("Foo")},
{Name: proto.String("Zip"), InputType: proto.String(".mypackage.Bar"), OutputType: proto.String(".mypackage.Foo")},
},
}
servLRO := &descriptor.ServiceDescriptorProto{
Name: proto.String("Foo"),
Method: []*descriptor.MethodDescriptorProto{
{Name: proto.String("Zip"), OutputType: proto.String(".google.longrunning.Operation")},
{Name: proto.String("Zip"), InputType: proto.String(".mypackage.Bar"), OutputType: proto.String(".google.longrunning.Operation")},
},
}

for _, tst := range []struct {
tstName string

mixins map[string]bool
tstName string
servName string
mixins map[string]bool
serv *descriptor.ServiceDescriptorProto
}{
{
Expand All @@ -249,13 +250,37 @@ func TestClientInit(t *testing.T) {
serv: servLRO,
},
} {
g.descInfo.ParentFile = map[proto.Message]*descriptor.FileDescriptorProto{
tst.serv: {
Options: &descriptor.FileOptions{
GoPackage: proto.String("mypackage"),
request := plugin.CodeGeneratorRequest{
Parameter: proto.String("go-gapic-package=path;mypackage"),
ProtoFile: []*descriptor.FileDescriptorProto{
&descriptor.FileDescriptorProto{
Package: proto.String("mypackage"),
Options: &descriptor.FileOptions{
GoPackage: proto.String("mypackage"),
},
Service: []*descriptor.ServiceDescriptorProto{tst.serv},
MessageType: []*descriptor.DescriptorProto{
&descriptor.DescriptorProto{
Name: proto.String("Bar"),
},
&descriptor.DescriptorProto{
Name: proto.String("Foo"),
},
},
},
},
}
&descriptor.FileDescriptorProto{
Package: proto.String("google.longrunning"),
Options: &descriptor.FileOptions{
GoPackage: proto.String("google.longrunning"),
},
MessageType: []*descriptor.DescriptorProto{
&descriptor.DescriptorProto{
Name: proto.String("Operation"),
},
},
},
}}
g.init(&request)
g.comments = map[proto.Message]string{
tst.serv: "Foo service does stuff.",
}
Expand All @@ -270,7 +295,8 @@ func TestClientInit(t *testing.T) {
}

g.reset()
g.clientInit(tst.serv, tst.servName)
g.makeClients(tst.serv, tst.servName)

txtdiff.Diff(t, tst.tstName, g.pt.String(), filepath.Join("testdata", tst.tstName+".want"))
}
}
Expand Down
21 changes: 12 additions & 9 deletions internal/gengapic/gengapic.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright 2018 Google LLC
// Copyright 2018 Google L/LC
software-dov marked this conversation as resolved.
Show resolved Hide resolved
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -127,7 +127,7 @@ func (g *generator) gen(serv *descriptor.ServiceDescriptorProto) error {
if err := g.clientOptions(serv, servName); err != nil {
return err
}
if err := g.clientInit(serv, servName); err != nil {
if err := g.makeClients(serv, servName); err != nil {
return err
}

Expand Down Expand Up @@ -238,17 +238,19 @@ func (g *generator) unaryCall(servName string, m *descriptor.MethodDescriptorPro

p := g.printf

p("func (c *%sClient) %s(ctx context.Context, req *%s.%s, opts ...gax.CallOption) (*%s.%s, error) {",
servName, m.GetName(), inSpec.Name, inType.GetName(), outSpec.Name, outType.GetName())
lowcaseServName := lowerFirst(servName)

p("func (c *%sGRPCClient) %s(ctx context.Context, req *%s.%s, opts ...gax.CallOption) (*%s.%s, error) {",
lowcaseServName, m.GetName(), inSpec.Name, inType.GetName(), outSpec.Name, outType.GetName())

g.deadline(sFQN, m.GetName())

err = g.insertMetadata(m)
if err != nil {
return err
}

g.appendCallOpts(m)

p("var resp *%s.%s", outSpec.Name, outType.GetName())
p("err := gax.Invoke(ctx, func(ctx context.Context, settings gax.CallSettings) error {")
p(" var err error")
Expand Down Expand Up @@ -281,16 +283,17 @@ func (g *generator) emptyUnaryCall(servName string, m *descriptor.MethodDescript

p := g.printf

p("func (c *%sClient) %s(ctx context.Context, req *%s.%s, opts ...gax.CallOption) error {",
servName, m.GetName(), inSpec.Name, inType.GetName())
lowcaseServName := lowerFirst(servName)

p("func (c *%sGRPCClient) %s(ctx context.Context, req *%s.%s, opts ...gax.CallOption) error {",
lowcaseServName, m.GetName(), inSpec.Name, inType.GetName())

g.deadline(sFQN, m.GetName())

err = g.insertMetadata(m)
if err != nil {
return err
}

g.appendCallOpts(m)
p("err := gax.Invoke(ctx, func(ctx context.Context, settings gax.CallSettings) error {")
p(" var err error")
Expand Down Expand Up @@ -420,7 +423,7 @@ func (g *generator) lookupFieldType(msgName, field string) descriptor.FieldDescr
}

func (g *generator) appendCallOpts(m *descriptor.MethodDescriptorProto) {
g.printf("opts = append(%[1]s[0:len(%[1]s):len(%[1]s)], opts...)", "c.CallOptions."+*m.Name)
g.printf("opts = append(%[1]s[0:len(%[1]s):len(%[1]s)], opts...)", "(*c.CallOptions)."+*m.Name)
}

func (g *generator) methodDoc(m *descriptor.MethodDescriptorProto) {
Expand Down
16 changes: 10 additions & 6 deletions internal/gengapic/lro.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,17 +43,19 @@ func (g *generator) lroCall(servName string, m *descriptor.MethodDescriptorProto
lroType := lroTypeName(m.GetName())
p := g.printf

p("func (c *%sClient) %s(ctx context.Context, req *%s.%s, opts ...gax.CallOption) (*%s, error) {",
servName, m.GetName(), inSpec.Name, inType.GetName(), lroType)
lowcaseServName := lowerFirst(servName)

p("func (c *%sGrpcClient) %s(ctx context.Context, req *%s.%s, opts ...gax.CallOption) (*%s, error) {",
lowcaseServName, m.GetName(), inSpec.Name, inType.GetName(), lroType)

g.deadline(sFQN, m.GetName())

err = g.insertMetadata(m)
if err != nil {
return err
}

g.appendCallOpts(m)

p(" var resp *%s.%s", outSpec.Name, outType.GetName())
p(" err := gax.Invoke(ctx, func(ctx context.Context, settings gax.CallSettings) error {")
p(" var err error")
Expand All @@ -64,7 +66,7 @@ func (g *generator) lroCall(servName string, m *descriptor.MethodDescriptorProto
p(" return nil, err")
p(" }")
p(" return &%s{", lroType)
p(" lro: longrunning.InternalNewOperation(c.LROClient, resp),")
p(" lro: longrunning.InternalNewOperation(*c.LROClient, resp),")
p(" }, nil")

p("}")
Expand All @@ -91,6 +93,8 @@ func (g *generator) lroType(servName string, serv *descriptor.ServiceDescriptorP
return fmt.Errorf("rpc %q has google.longrunning.operation_info but is missing option google.longrunning.operation_info.response_type", mFQN)
}

lowcaseServName := lowerFirst(servName)

var respType string
{
// eLRO.ResponseType is either fully-qualified or top-level in the same package as the method.
Expand Down Expand Up @@ -152,9 +156,9 @@ func (g *generator) lroType(servName string, serv *descriptor.ServiceDescriptorP
{
p("// %[1]s returns a new %[1]s from a given name.", lroType)
p("// The name must be that of a previously created %s, possibly from a different process.", lroType)
p("func (c *%sClient) %[2]s(name string) *%[2]s {", servName, lroType)
p("func (c *%sGrpcClient) %[2]s(name string) *%[2]s {", lowcaseServName, lroType)
p(" return &%s{", lroType)
p(" lro: longrunning.InternalNewOperation(c.LROClient, &longrunningpb.Operation{Name: name}),")
p(" lro: longrunning.InternalNewOperation(*c.LROClient, &longrunningpb.Operation{Name: name}),")
p(" }")
p("}")
p("")
Expand Down
18 changes: 16 additions & 2 deletions internal/gengapic/options.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package gengapic

import (
"fmt"
"path/filepath"
"strings"

Expand All @@ -26,10 +27,10 @@ type transport int
const (
grpc transport = iota
rest

paramError = "need parameter in format: go-gapic-package=client/import/path;packageName"
)

const paramError = "need parameter in format: go-gapic-package=client/import/path;packageName"

type options struct {
pkgPath string
pkgName string
Expand Down Expand Up @@ -149,3 +150,16 @@ func parseOptions(parameter *string) (*options, error) {

return &opts, nil
}

// Utility function for stringifying the Transport enum
func (t transport) String() string {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: can we colocate methods with the type they belong to for ease of readability? Consider either moving the type def for transport down here or adding a new file transport.go (and a corresponding test file).

switch t {
case grpc:
return "grpc"
case rest:
return "rest"
default:
// Add new transport variants as need be.
return fmt.Sprintf("%d", int(t))
}
}
6 changes: 3 additions & 3 deletions internal/gengapic/options_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,17 +110,17 @@ func TestParseOptions(t *testing.T) {
} {
opts, err := parseOptions(&tst.param)
if tst.expectErr && err == nil {
t.Errorf("ParseOptions(%s) expected error", tst.param)
t.Errorf("parseOptions(%s) expected error", tst.param)
continue
}

if !tst.expectErr && err != nil {
t.Errorf("ParseOptions(%s) got unexpected error: %v", tst.param, err)
t.Errorf("parseOptions(%s) got unexpected error: %v", tst.param, err)
continue
}

if !reflect.DeepEqual(opts, tst.expectedOpts) {
t.Errorf("ParseOptions(%s) = %v, expected %v", tst.param, opts, tst.expectedOpts)
t.Errorf("parseOptions(%s) = %v, expected %v", tst.param, opts, tst.expectedOpts)
continue
}
}
Expand Down
8 changes: 5 additions & 3 deletions internal/gengapic/paging.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,9 @@ func (g *generator) pagingCall(servName string, m *descriptor.MethodDescriptorPr
inType := g.descInfo.Type[m.GetInputType()].(*descriptor.DescriptorProto)
outType := g.descInfo.Type[m.GetOutputType()].(*descriptor.DescriptorProto)

// We DON'T want to export the transport layers.
lowcaseServName := lowerFirst(servName)

inSpec, err := g.descInfo.ImportSpec(inType)
if err != nil {
return err
Expand All @@ -188,14 +191,13 @@ func (g *generator) pagingCall(servName string, m *descriptor.MethodDescriptorPr
}

p := g.printf
p("func (c *%sClient) %s(ctx context.Context, req *%s.%s, opts ...gax.CallOption) *%s {",
servName, *m.Name, inSpec.Name, inType.GetName(), pt.iterTypeName)
p("func (c *%sGrpcClient) %s(ctx context.Context, req *%s.%s, opts ...gax.CallOption) *%s {",
lowcaseServName, *m.Name, inSpec.Name, inType.GetName(), pt.iterTypeName)

err = g.insertMetadata(m)
if err != nil {
return err
}

g.appendCallOpts(m)

p("it := &%s{}", pt.iterTypeName)
Expand Down
16 changes: 10 additions & 6 deletions internal/gengapic/stream.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,16 @@ func (g *generator) noRequestStreamCall(servName string, s *descriptor.ServiceDe
}
g.imports[servSpec] = true

p("func (c *%sClient) %s(ctx context.Context, opts ...gax.CallOption) (%s.%s_%sClient, error) {",
servName, m.GetName(), servSpec.Name, s.GetName(), m.GetName())
// We DON'T want to export the transport layers.
lowcaseServName := lowerFirst(servName)

p("func (c *%sGrpcClient) %s(ctx context.Context, opts ...gax.CallOption) (%s.%s_%sClient, error) {",
lowcaseServName, m.GetName(), servSpec.Name, s.GetName(), m.GetName())
g.insertMetadata(nil)
g.appendCallOpts(m)
p(" var resp %s.%s_%sClient", servSpec.Name, s.GetName(), m.GetName())

g.appendCallOpts(m)

p(" err := gax.Invoke(ctx, func(ctx context.Context, settings gax.CallSettings) error {")
p(" var err error")
p(" resp, err = c.%s.%s(ctx, settings.GRPC...)", grpcClientField(servName), m.GetName())
Expand Down Expand Up @@ -62,16 +66,16 @@ func (g *generator) serverStreamCall(servName string, s *descriptor.ServiceDescr
g.imports[servSpec] = true

p := g.printf
lowcaseServName := lowerFirst(servName)

p("func (c *%sClient) %s(ctx context.Context, req *%s.%s, opts ...gax.CallOption) (%s.%s_%sClient, error) {",
servName, m.GetName(), inSpec.Name, inType.GetName(), servSpec.Name, s.GetName(), m.GetName())
p("func (c *%sGrpcClient) %s(ctx context.Context, req *%s.%s, opts ...gax.CallOption) (%s.%s_%sClient, error) {",
lowcaseServName, m.GetName(), inSpec.Name, inType.GetName(), servSpec.Name, s.GetName(), m.GetName())

err = g.insertMetadata(m)
if err != nil {
return err
}

g.appendCallOpts(m)
p(" var resp %s.%s_%sClient", servSpec.Name, s.GetName(), m.GetName())
p("err := gax.Invoke(ctx, func(ctx context.Context, settings gax.CallSettings) error {")
p(" var err error")
Expand Down
Loading