Skip to content

Commit

Permalink
fix(gengapic): regapic fix missing return statement (#1054)
Browse files Browse the repository at this point in the history
  • Loading branch information
noahdietz authored Jun 28, 2022
1 parent 2108164 commit 7d08d1b
Show file tree
Hide file tree
Showing 6 changed files with 125 additions and 13 deletions.
1 change: 1 addition & 0 deletions internal/gengapic/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ go_test(
"@com_github_google_go_cmp//cmp",
"@go_googleapis//gapic/metadata:metadata_go_proto",
"@go_googleapis//google/api:annotations_go_proto",
"@go_googleapis//google/api:httpbody_go_proto",
"@go_googleapis//google/api:serviceconfig_go_proto",
"@go_googleapis//google/cloud:extended_operations_go_proto",
"@go_googleapis//google/longrunning:longrunning_go_proto",
Expand Down
1 change: 1 addition & 0 deletions internal/gengapic/gengapic.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ const (
// protoc puts a dot in front of name, signaling that the name is fully qualified.
emptyType = "." + emptyValue
lroType = ".google.longrunning.Operation"
httpBodyType = ".google.api.HttpBody"
alpha = "alpha"
beta = "beta"
disableDeadlinesVar = "GOOGLE_API_GO_EXPERIMENTAL_DISABLE_DEFAULT_DEADLINE"
Expand Down
9 changes: 5 additions & 4 deletions internal/gengapic/genrest.go
Original file line number Diff line number Diff line change
Expand Up @@ -1045,8 +1045,8 @@ func (g *generator) unaryRESTCall(servName string, m *descriptor.MethodDescripto
if err != nil {
return err
}
outFqn := fmt.Sprintf("%s.%s", g.descInfo.ParentFile[outType].GetPackage(), outType.GetName())
isHTTPBodyMessage := outFqn == "google.api.HttpBody"
outFqn := fmt.Sprintf(".%s.%s", g.descInfo.ParentFile[outType].GetPackage(), outType.GetName())
isHTTPBodyMessage := outFqn == httpBodyType

// Ignore error because the only possible error would be from looking up
// the ImportSpec for the OutputType, which has already happened above.
Expand Down Expand Up @@ -1133,9 +1133,10 @@ func (g *generator) unaryRESTCall(servName string, m *descriptor.MethodDescripto
p("if err := unm.Unmarshal(buf, resp); err != nil {")
p(" return maybeUnknownEnum(err)")
p("}")
p("")
p("return nil")
}

p("")
p(" return nil")
p("}, opts...)")
p("if e != nil {")
p(" return nil, e")
Expand Down
58 changes: 49 additions & 9 deletions internal/gengapic/genrest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"github.com/googleapis/gapic-generator-go/internal/pbinfo"
"github.com/googleapis/gapic-generator-go/internal/txtdiff"
"google.golang.org/genproto/googleapis/api/annotations"
"google.golang.org/genproto/googleapis/api/httpbody"
"google.golang.org/genproto/googleapis/api/serviceconfig"
"google.golang.org/genproto/googleapis/cloud/extendedops"
"google.golang.org/genproto/googleapis/longrunning"
Expand Down Expand Up @@ -570,6 +571,26 @@ func TestGenRestMethod(t *testing.T) {
Options: lroRPCOpt,
}

httpBodyDesc := protodesc.ToDescriptorProto((&httpbody.HttpBody{}).ProtoReflect().Descriptor())
httpBodyRPCOpt := &descriptor.MethodOptions{}
proto.SetExtension(httpBodyRPCOpt, annotations.E_Http, &annotations.HttpRule{
Pattern: &annotations.HttpRule_Post{
Post: "/v1/foo",
},
Body: "*",
})
proto.SetExtension(httpBodyRPCOpt, annotations.E_Routing, &annotations.RoutingRule{
RoutingParameters: []*annotations.RoutingParameter{
{Field: "other"},
},
})
httpBodyRPC := &descriptor.MethodDescriptorProto{
Name: proto.String("HttpBodyRPC"),
InputType: proto.String(foofqn),
OutputType: proto.String(httpBodyType),
Options: httpBodyRPCOpt,
}

s := &descriptor.ServiceDescriptorProto{
Name: proto.String("FooService"),
}
Expand Down Expand Up @@ -599,15 +620,16 @@ func TestGenRestMethod(t *testing.T) {
},
descInfo: pbinfo.Info{
ParentFile: map[protoiface.MessageV1]*descriptor.FileDescriptorProto{
op: f,
opS: f,
opRPC: f,
lroRPC: f,
foo: f,
s: f,
pagedFooReq: f,
pagedFooRes: f,
lroDesc: protodesc.ToFileDescriptorProto(longrunning.File_google_longrunning_operations_proto),
op: f,
opS: f,
opRPC: f,
lroRPC: f,
foo: f,
s: f,
pagedFooReq: f,
pagedFooRes: f,
lroDesc: protodesc.ToFileDescriptorProto(longrunning.File_google_longrunning_operations_proto),
httpBodyDesc: protodesc.ToFileDescriptorProto(httpbody.File_google_api_httpbody_proto),
},
ParentElement: map[pbinfo.ProtoType]pbinfo.ProtoType{
opRPC: s,
Expand All @@ -616,6 +638,7 @@ func TestGenRestMethod(t *testing.T) {
pagingRPC: s,
serverStreamRPC: s,
lroRPC: s,
httpBodyRPC: s,
nameField: op,
sizeField: foo,
otherField: foo,
Expand All @@ -627,6 +650,7 @@ func TestGenRestMethod(t *testing.T) {
pagedFooReqFQN: pagedFooReq,
pagedFooResFQN: pagedFooRes,
lroType: lroDesc,
httpBodyType: httpBodyDesc,
},
},
}
Expand Down Expand Up @@ -718,6 +742,22 @@ func TestGenRestMethod(t *testing.T) {
{Name: "longrunningpb", Path: "google.golang.org/genproto/googleapis/longrunning"}: true,
},
},
{
name: "http_body_rpc",
method: httpBodyRPC,
options: &options{},
imports: map[pbinfo.ImportSpec]bool{
{Path: "bytes"}: true,
{Path: "fmt"}: true,
{Path: "google.golang.org/protobuf/encoding/protojson"}: true,
{Path: "google.golang.org/api/googleapi"}: true,
{Path: "net/url"}: true,
{Path: "regexp"}: true,
{Path: "strings"}: true,
{Name: "foopb", Path: "google.golang.org/genproto/cloud/foo/v1"}: true,
{Name: "httpbodypb", Path: "google.golang.org/genproto/googleapis/api/httpbody"}: true,
},
},
} {
s.Method = []*descriptor.MethodDescriptorProto{tst.method}
g.opts = tst.options
Expand Down
66 changes: 66 additions & 0 deletions internal/gengapic/testdata/rest_HttpBodyRPC.want
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
func (c *fooRESTClient) HttpBodyRPC(ctx context.Context, req *foopb.Foo, opts ...gax.CallOption) (*httpbodypb.HttpBody, error) {
m := protojson.MarshalOptions{AllowPartial: true, UseEnumNumbers: true}
jsonReq, err := m.Marshal(req)
if err != nil {
return nil, err
}

baseUrl, err := url.Parse(c.endpoint)
if err != nil {
return nil, err
}
baseUrl.Path += fmt.Sprintf("/v1/foo")

// Build HTTP headers from client and context metadata.
routingHeaders := ""
routingHeadersMap := make(map[string]string)
if reg := regexp.MustCompile("(.*)"); reg.MatchString(req.GetOther()) && len(url.QueryEscape(reg.FindStringSubmatch(req.GetOther())[1])) > 0 {
routingHeadersMap["other"] = url.QueryEscape(reg.FindStringSubmatch(req.GetOther())[1])
}
for headerName, headerValue := range routingHeadersMap {
routingHeaders = fmt.Sprintf("%s%s=%s&", routingHeaders, headerName, headerValue)
}
routingHeaders = strings.TrimSuffix(routingHeaders, "&")
md := metadata.Pairs("x-goog-request-params", routingHeaders)

headers := buildHeaders(ctx, c.xGoogMetadata, md, metadata.Pairs("Content-Type", "application/json"))
opts = append((*c.CallOptions).HttpBodyRPC[0:len((*c.CallOptions).HttpBodyRPC):len((*c.CallOptions).HttpBodyRPC)], opts...)
resp := &httpbodypb.HttpBody{}
e := gax.Invoke(ctx, func(ctx context.Context, settings gax.CallSettings) error {
if settings.Path != "" {
baseUrl.Path = settings.Path
}
httpReq, err := http.NewRequest("POST", baseUrl.String(), bytes.NewReader(jsonReq))
if err != nil {
return err
}
httpReq = httpReq.WithContext(ctx)
httpReq.Header = headers

httpRsp, err := c.httpClient.Do(httpReq)
if err != nil{
return err
}
defer httpRsp.Body.Close()

if err = googleapi.CheckResponse(httpRsp); err != nil {
return err
}

buf, err := ioutil.ReadAll(httpRsp.Body)
if err != nil {
return err
}

resp.Data = buf
if headers := httpRsp.Header; len(headers["Content-Type"]) > 0 {
resp.ContentType = headers["Content-Type"][0]
}

return nil
}, opts...)
if e != nil {
return nil, e
}
return resp, nil
}
3 changes: 3 additions & 0 deletions test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ generate --go_gapic_opt 'go-gapic-package=cloud.google.com/go/texttospeech/apiv1
echo "Generating Cloud Storage v2"
generate --go_gapic_opt 'go-gapic-package=cloud.google.com/go/storage/internal/apiv2;storage,transport=grpc' $GOOGLEAPIS/google/storage/v2/*.proto

echo "Generating Cloud Retail v2"
generate --go_gapic_opt 'go-gapic-package=cloud.google.com/go/retail/apiv2;retail,transport=rest' $GOOGLEAPIS/google/cloud/retail/v2/*.proto

echo "Generation complete"

echo "Running gofmt to check for syntax errors"
Expand Down

0 comments on commit 7d08d1b

Please sign in to comment.