From ed34451cb32c7ee553ce26c8936f9498e9c44e70 Mon Sep 17 00:00:00 2001 From: Victor Chudnovsky Date: Fri, 20 Aug 2021 18:54:18 -0700 Subject: [PATCH 1/7] Infrastructure to provide REST-specific handlers via hooks NEXT: - clean up init function for go view creator - add custom operation for unknown enum --- .../google/showcase/v1beta1/compliance.proto | 11 +++++ server/genrest/compliance_manual.go | 47 +++++++++++++++++++ server/services/compliance_service.go | 4 ++ util/genrest/goviewcreator.go | 18 +++++++ 4 files changed, 80 insertions(+) create mode 100644 server/genrest/compliance_manual.go diff --git a/schema/google/showcase/v1beta1/compliance.proto b/schema/google/showcase/v1beta1/compliance.proto index 6b55407c6..3905f9a52 100644 --- a/schema/google/showcase/v1beta1/compliance.proto +++ b/schema/google/showcase/v1beta1/compliance.proto @@ -96,6 +96,17 @@ service Compliance { }; } + // This method returns an unknown enum value. Client libraries should either error gracefully (not + // crash), ignore the value, or, depending on the API, set it to the zero default. + // + // This only works over REST currently. Requests over gRPC will echo the request and should + // succeed on the client side + rpc RepeatWithUnknownEnum(RepeatRequest) returns (RepeatResponse) { + option (google.api.http) = { + post: "/v1beta1/repeat:invalidenum" + body: "*" + }; + } } message RepeatRequest { diff --git a/server/genrest/compliance_manual.go b/server/genrest/compliance_manual.go new file mode 100644 index 000000000..376ea2c1e --- /dev/null +++ b/server/genrest/compliance_manual.go @@ -0,0 +1,47 @@ +// Copyright 2021 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package genrest + +import ( + "context" + "net/http" + + genprotopb "github.com/googleapis/gapic-showcase/server/genproto" + "github.com/googleapis/gapic-showcase/util/genrest/resttools" +) + +// .google.showcase.v1beta1.Compliance.HandleRepeatDataBody +func (backend *RESTBackend) customRepeatWithUnknownEnum(w http.ResponseWriter, r *http.Request, request *genprotopb.RepeatRequest) { + marshaler := resttools.ToJSON() + + response, err := backend.ComplianceServer.RepeatWithUnknownEnum(context.Background(), request) + if err != nil { + // TODO: Properly handle error. Is StatusInternalServerError (500) the right response? + backend.Error(w, http.StatusInternalServerError, "server error: %s", err.Error()) + return + } + + // FIXME: change response field to sentinel enum + + json, err := marshaler.Marshal(response) + if err != nil { + backend.Error(w, http.StatusInternalServerError, "error json-encoding response: %s", err.Error()) + return + } + + // FIXME: change sentinel to unknown value + + w.Write(json) +} diff --git a/server/services/compliance_service.go b/server/services/compliance_service.go index e3b40f0e1..fdc85b4f1 100644 --- a/server/services/compliance_service.go +++ b/server/services/compliance_service.go @@ -98,6 +98,10 @@ func (csi *complianceServerImpl) RepeatDataBodyPatch(ctx context.Context, in *pb return csi.Repeat(ctx, in) } +func (csi *complianceServerImpl) RepeatWithUnknownEnum(ctx context.Context, in *pb.RepeatRequest) (*pb.RepeatResponse, error) { + return csi.Repeat(ctx, in) +} + // complianceSuiteBytes contains the contents of the compliance suite JSON file. This requires Go // 1.16. Note that embedding can only be applied to global variables at package scope. //go:embed compliance_suite.json diff --git a/util/genrest/goviewcreator.go b/util/genrest/goviewcreator.go index afc0ff48e..b5bec2ccb 100644 --- a/util/genrest/goviewcreator.go +++ b/util/genrest/goviewcreator.go @@ -24,6 +24,14 @@ import ( "github.com/googleapis/gapic-showcase/util/genrest/resttools" ) +var customFunctions map[string]string + +func init() { + customFunctions = map[string]string{ + ".google.showcase.v1beta1.Compliance.RepeatWithUnknownEnum": "customRepeatWithUnknownEnum", + } +} + // NewView creates a a new goview.View (a series of files to be output) from a gomodel.Model. The // current approach is to generate one file per service, with that file containing all the service's // RPCs. An additional file `genrest.go` is also created to register all these handlers with a @@ -189,6 +197,16 @@ func NewView(model *gomodel.Model) (*goview.View, error) { source.P(" requestJSON, _ := marshaler.Marshal(%s)", handler.RequestVariable) source.P(` backend.StdLog.Printf(" request: %%s", requestJSON)`) source.P("") + + methodId := fmt.Sprintf("%s.%s", service.ProtoPath, handler.GoMethod) + customHandler, _ := customFunctions[methodId] + source.P(" // %s", methodId) + if len(customHandler) > 0 { + source.P(" backend.%s(w, r, request) // %s", customHandler, methodId) + source.P("}") + continue + } + // TODO: In the future, we may want to redirect all REST-endpoint requests to the gRPC endpoint so that the gRPC-registered observers get invoked. source.P(" %s, err := backend.%sServer.%s(context.Background(), %s)", handler.ResponseVariable, service.ShortName, handler.GoMethod, handler.RequestVariable) source.P(" if err != nil {") From 5bfe059185c2b15d364c47b875c4c3c064bdf983 Mon Sep 17 00:00:00 2001 From: Victor Chudnovsky Date: Sat, 21 Aug 2021 12:51:37 -0700 Subject: [PATCH 2/7] Provide an unknown enum NEXT: need tests, clean up code --- server/genrest/README.md | 6 ++++-- .../{compliance_manual.go => compliance_custom.go} | 11 ++++++++--- 2 files changed, 12 insertions(+), 5 deletions(-) rename server/genrest/{compliance_manual.go => compliance_custom.go} (70%) diff --git a/server/genrest/README.md b/server/genrest/README.md index 192b71c78..259e0c624 100644 --- a/server/genrest/README.md +++ b/server/genrest/README.md @@ -1,4 +1,6 @@ # server/genrest -This directory contains auto-generated files used to implement a REST endpoint -for Showcase services. +This directory contains mostly auto-generated files used to implement a REST +endpoint for Showcase services. The `*_custom.go` files contain manually written +REST-specific handlers, which are useful for helping generators test +REST-specific functionality (such as invalid JSON). diff --git a/server/genrest/compliance_manual.go b/server/genrest/compliance_custom.go similarity index 70% rename from server/genrest/compliance_manual.go rename to server/genrest/compliance_custom.go index 376ea2c1e..5214da269 100644 --- a/server/genrest/compliance_manual.go +++ b/server/genrest/compliance_custom.go @@ -15,6 +15,7 @@ package genrest import ( + "bytes" "context" "net/http" @@ -22,7 +23,7 @@ import ( "github.com/googleapis/gapic-showcase/util/genrest/resttools" ) -// .google.showcase.v1beta1.Compliance.HandleRepeatDataBody +// customRepeatWithUnknownEnum provides REST-specific handling for a RepeatWithUnknownEnum request. It returns a JSON response with an unknown enum symbol string in an enum field. func (backend *RESTBackend) customRepeatWithUnknownEnum(w http.ResponseWriter, r *http.Request, request *genprotopb.RepeatRequest) { marshaler := resttools.ToJSON() @@ -33,7 +34,10 @@ func (backend *RESTBackend) customRepeatWithUnknownEnum(w http.ResponseWriter, r return } - // FIXME: change response field to sentinel enum + // Make sure we have at least one sentinel value before serializing properly. + sentinelValue := genprotopb.ComplianceData_LIFE_KINGDOM_UNSPECIFIED + sentinelString := genprotopb.ComplianceData_LifeKingdom_name[int32(sentinelValue)] + response.Request.Info.FKingdom = sentinelValue json, err := marshaler.Marshal(response) if err != nil { @@ -41,7 +45,8 @@ func (backend *RESTBackend) customRepeatWithUnknownEnum(w http.ResponseWriter, r return } - // FIXME: change sentinel to unknown value + // Change the sentinel string to an unknown value. + json = bytes.ReplaceAll(json, []byte(sentinelString), []byte("LIFE_KINGDOM_NEW")) w.Write(json) } From acba666543276ad3e03e51d7c0e6dc75667a8930 Mon Sep 17 00:00:00 2001 From: Victor Chudnovsky Date: Tue, 24 Aug 2021 16:53:57 -0700 Subject: [PATCH 3/7] Added test for TestRepeatWithUnknownEnum Asa result of the test, also fixed NPE by ensuring submessages are present before referencing them. --- cmd/gapic-showcase/compliance_custom_test.go | 101 +++++++++++++++++++ cmd/gapic-showcase/compliance_suite_test.go | 2 +- server/genrest/compliance_custom.go | 6 ++ 3 files changed, 108 insertions(+), 1 deletion(-) create mode 100644 cmd/gapic-showcase/compliance_custom_test.go diff --git a/cmd/gapic-showcase/compliance_custom_test.go b/cmd/gapic-showcase/compliance_custom_test.go new file mode 100644 index 000000000..d71c062aa --- /dev/null +++ b/cmd/gapic-showcase/compliance_custom_test.go @@ -0,0 +1,101 @@ +// Copyright 2021 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package main + +import ( + "io/ioutil" + "net/http" + "strings" + "testing" + + "github.com/googleapis/gapic-showcase/server/genproto" + genprotopb "github.com/googleapis/gapic-showcase/server/genproto" + "github.com/googleapis/gapic-showcase/util/genrest/resttools" + "google.golang.org/protobuf/encoding/protojson" +) + +// TestRepeatWithUnknownEnum +func TestRepeatWithUnknownEnum(t *testing.T) { + _, server, err := complianceSuiteTestSetup() + if err != nil { + t.Fatal(err) + } + server.Start() + defer server.Close() + + resttools.JSONMarshaler.Replace(nil) + defer resttools.JSONMarshaler.Restore() + + request := &genprotopb.RepeatRequest{ + FInt32: 23, + } + + // First ensure the request would be otherwise successful + errorPrefix := `"repeat:body":` + responseBody, requestBody := getJSONResponse(t, request, server.URL+"/v1beta1/repeat:body", errorPrefix) + var response genproto.RepeatResponse + if err := protojson.Unmarshal(responseBody, &response); err != nil { + t.Fatalf("%s could not unmarshal httpResponse body: %s\n response body: %s\n request: %s\n", + errorPrefix, err, string(responseBody), string(requestBody)) + } + + // Then ensure the expected error occurs + errorPrefix = `"repeat:invalidenum":` + responseBody, requestBody = getJSONResponse(t, request, server.URL+"/v1beta1/repeat:invalidenum", errorPrefix) + err = protojson.Unmarshal(responseBody, &response) + if err == nil { + t.Fatalf("%s did not receive an error:\n response body: %s\n request: %s\n", + errorPrefix, string(responseBody), string(requestBody)) + } + if !strings.Contains(err.Error(), "invalid value for enum type") { + t.Fatalf("%s received different error than expected: %s\n response body: %s\n request: %s\n", + errorPrefix, err, string(responseBody), string(requestBody)) + } + +} + +func getJSONResponse(t *testing.T, request *genprotopb.RepeatRequest, uri, errorPrefix string) (responseBody, requestBody []byte) { + verb := "POST" + requestBody, err := resttools.ToJSON().Marshal(request) + if err != nil { + t.Fatalf("%s error encoding request: %s", errorPrefix, err) + } + + httpRequest, err := http.NewRequest(verb, uri, strings.NewReader(string(requestBody))) + if err != nil { + t.Fatalf("%s error creating request: %s", errorPrefix, err) + } + resttools.PopulateRequestHeaders(httpRequest) + + httpResponse, err := http.DefaultClient.Do(httpRequest) + if err != nil { + t.Fatalf("%s error issuing call: %s", errorPrefix, err) + } + + // Check for successful response. + if got, want := httpResponse.StatusCode, http.StatusOK; got != want { + t.Errorf("%s response code: got %d, want %d\n %s %s\n\n", + errorPrefix, got, want, verb, uri) + } + + responseBody, err = ioutil.ReadAll(httpResponse.Body) + httpResponse.Body.Close() + if err != nil { + t.Fatalf("%s could not read httpResponse body: %s", errorPrefix, err) + } + return responseBody, requestBody +} + +// repeat wtih same message for invalidenum; check for error diff --git a/cmd/gapic-showcase/compliance_suite_test.go b/cmd/gapic-showcase/compliance_suite_test.go index 89f4d60ac..b9c30f05d 100644 --- a/cmd/gapic-showcase/compliance_suite_test.go +++ b/cmd/gapic-showcase/compliance_suite_test.go @@ -107,7 +107,7 @@ func TestComplianceSuite(t *testing.T) { } // Unmarshal httpResponse body, interpreted as JSON. - // should do this. + // GAPIC generators should do this in their tests. responseBody, err := ioutil.ReadAll(httpResponse.Body) httpResponse.Body.Close() if err != nil { diff --git a/server/genrest/compliance_custom.go b/server/genrest/compliance_custom.go index 5214da269..b8c50ad50 100644 --- a/server/genrest/compliance_custom.go +++ b/server/genrest/compliance_custom.go @@ -37,6 +37,12 @@ func (backend *RESTBackend) customRepeatWithUnknownEnum(w http.ResponseWriter, r // Make sure we have at least one sentinel value before serializing properly. sentinelValue := genprotopb.ComplianceData_LIFE_KINGDOM_UNSPECIFIED sentinelString := genprotopb.ComplianceData_LifeKingdom_name[int32(sentinelValue)] + if response.Request == nil { + response.Request = &genprotopb.RepeatRequest{} + } + if response.Request.Info == nil { + response.Request.Info = &genprotopb.ComplianceData{} + } response.Request.Info.FKingdom = sentinelValue json, err := marshaler.Marshal(response) From 11434063bc3cd3950e4bb945ae8b69191626891d Mon Sep 17 00:00:00 2001 From: Victor Chudnovsky Date: Tue, 24 Aug 2021 20:10:06 -0700 Subject: [PATCH 4/7] Add and implement RepeatWithUnknownOptionalEnum RPC Need to test this next. --- .../google/showcase/v1beta1/compliance.proto | 12 +++++++++++ server/genrest/compliance_custom.go | 21 ++++++++++++++++++- server/services/compliance_service.go | 4 ++++ util/genrest/goviewcreator.go | 3 ++- 4 files changed, 38 insertions(+), 2 deletions(-) diff --git a/schema/google/showcase/v1beta1/compliance.proto b/schema/google/showcase/v1beta1/compliance.proto index 3905f9a52..3e4ecc8eb 100644 --- a/schema/google/showcase/v1beta1/compliance.proto +++ b/schema/google/showcase/v1beta1/compliance.proto @@ -107,6 +107,18 @@ service Compliance { body: "*" }; } + + // This method returns an unknown enum value. Client libraries should either error gracefully (not + // crash), ignore the value, or, depending on the API, set it to the zero default. + // + // This only works over REST currently. Requests over gRPC will echo the request and should + // succeed on the client side + rpc RepeatWithUnknownOptionalEnum(RepeatRequest) returns (RepeatResponse) { + option (google.api.http) = { + post: "/v1beta1/repeat:invalidoptionalenum" + body: "*" + }; + } } message RepeatRequest { diff --git a/server/genrest/compliance_custom.go b/server/genrest/compliance_custom.go index b8c50ad50..da1d85bc5 100644 --- a/server/genrest/compliance_custom.go +++ b/server/genrest/compliance_custom.go @@ -25,6 +25,22 @@ import ( // customRepeatWithUnknownEnum provides REST-specific handling for a RepeatWithUnknownEnum request. It returns a JSON response with an unknown enum symbol string in an enum field. func (backend *RESTBackend) customRepeatWithUnknownEnum(w http.ResponseWriter, r *http.Request, request *genprotopb.RepeatRequest) { + mutator := func(data *genprotopb.ComplianceData, sentinelValue genprotopb.ComplianceData_LifeKingdom) { + data.FKingdom = sentinelValue + } + backend.customRepeatWithUnknownEnumMethod(w, r, request, mutator) +} + +// customRepeatWithUnknownOptionalEnum provides REST-specific handling for a RepeatWithUnknownOptionalEnum request. It returns a JSON response with an unknown enum symbol string in an enum field. +func (backend *RESTBackend) customRepeatWithUnknownOptionalEnum(w http.ResponseWriter, r *http.Request, request *genprotopb.RepeatRequest) { + mutator := func(data *genprotopb.ComplianceData, sentinelValue genprotopb.ComplianceData_LifeKingdom) { + data.PKingdom = &sentinelValue + } + backend.customRepeatWithUnknownEnumMethod(w, r, request, mutator) +} + +// customRepeatWithUnknownEnumMethod provides REST-specific handling for the RepeatWithUnknown*Enum request. It returns a JSON response with an unknown enum symbol string in an enum field. +func (backend *RESTBackend) customRepeatWithUnknownEnumMethod(w http.ResponseWriter, r *http.Request, request *genprotopb.RepeatRequest, mutate enumMutator) { marshaler := resttools.ToJSON() response, err := backend.ComplianceServer.RepeatWithUnknownEnum(context.Background(), request) @@ -43,7 +59,7 @@ func (backend *RESTBackend) customRepeatWithUnknownEnum(w http.ResponseWriter, r if response.Request.Info == nil { response.Request.Info = &genprotopb.ComplianceData{} } - response.Request.Info.FKingdom = sentinelValue + mutate(response.Request.Info, sentinelValue) json, err := marshaler.Marshal(response) if err != nil { @@ -56,3 +72,6 @@ func (backend *RESTBackend) customRepeatWithUnknownEnum(w http.ResponseWriter, r w.Write(json) } + +// enumMutator represents a function that modifies data in place using sentinelValue. +type enumMutator func(data *genprotopb.ComplianceData, sentinelValue genprotopb.ComplianceData_LifeKingdom) diff --git a/server/services/compliance_service.go b/server/services/compliance_service.go index fdc85b4f1..4fe7f8406 100644 --- a/server/services/compliance_service.go +++ b/server/services/compliance_service.go @@ -102,6 +102,10 @@ func (csi *complianceServerImpl) RepeatWithUnknownEnum(ctx context.Context, in * return csi.Repeat(ctx, in) } +func (csi *complianceServerImpl) RepeatWithUnknownOptionalEnum(ctx context.Context, in *pb.RepeatRequest) (*pb.RepeatResponse, error) { + return csi.Repeat(ctx, in) +} + // complianceSuiteBytes contains the contents of the compliance suite JSON file. This requires Go // 1.16. Note that embedding can only be applied to global variables at package scope. //go:embed compliance_suite.json diff --git a/util/genrest/goviewcreator.go b/util/genrest/goviewcreator.go index b5bec2ccb..43b369bdc 100644 --- a/util/genrest/goviewcreator.go +++ b/util/genrest/goviewcreator.go @@ -28,7 +28,8 @@ var customFunctions map[string]string func init() { customFunctions = map[string]string{ - ".google.showcase.v1beta1.Compliance.RepeatWithUnknownEnum": "customRepeatWithUnknownEnum", + ".google.showcase.v1beta1.Compliance.RepeatWithUnknownEnum": "customRepeatWithUnknownEnum", + ".google.showcase.v1beta1.Compliance.RepeatWithUnknownOptionalEnum": "customRepeatWithUnknownOptionalEnum", } } From 1ee9d48d2bdd582ffa664f7f1ebc2c957203ef21 Mon Sep 17 00:00:00 2001 From: Victor Chudnovsky Date: Tue, 24 Aug 2021 20:24:42 -0700 Subject: [PATCH 5/7] Add test for RepeatWithUnknownOptionalEnum --- cmd/gapic-showcase/compliance_custom_test.go | 43 +++++++++++--------- 1 file changed, 23 insertions(+), 20 deletions(-) diff --git a/cmd/gapic-showcase/compliance_custom_test.go b/cmd/gapic-showcase/compliance_custom_test.go index d71c062aa..c361b2c3c 100644 --- a/cmd/gapic-showcase/compliance_custom_test.go +++ b/cmd/gapic-showcase/compliance_custom_test.go @@ -15,6 +15,7 @@ package main import ( + "fmt" "io/ioutil" "net/http" "strings" @@ -42,28 +43,30 @@ func TestRepeatWithUnknownEnum(t *testing.T) { FInt32: 23, } - // First ensure the request would be otherwise successful - errorPrefix := `"repeat:body":` - responseBody, requestBody := getJSONResponse(t, request, server.URL+"/v1beta1/repeat:body", errorPrefix) - var response genproto.RepeatResponse - if err := protojson.Unmarshal(responseBody, &response); err != nil { - t.Fatalf("%s could not unmarshal httpResponse body: %s\n response body: %s\n request: %s\n", - errorPrefix, err, string(responseBody), string(requestBody)) - } + for idx, variant := range []string{"invalidenum", "invalidoptionalenum"} { + errorPrefix := fmt.Sprintf("[%d %q]", idx, variant) - // Then ensure the expected error occurs - errorPrefix = `"repeat:invalidenum":` - responseBody, requestBody = getJSONResponse(t, request, server.URL+"/v1beta1/repeat:invalidenum", errorPrefix) - err = protojson.Unmarshal(responseBody, &response) - if err == nil { - t.Fatalf("%s did not receive an error:\n response body: %s\n request: %s\n", - errorPrefix, string(responseBody), string(requestBody)) - } - if !strings.Contains(err.Error(), "invalid value for enum type") { - t.Fatalf("%s received different error than expected: %s\n response body: %s\n request: %s\n", - errorPrefix, err, string(responseBody), string(requestBody)) - } + // First ensure the request would be otherwise successful + responseBody, requestBody := getJSONResponse(t, request, server.URL+"/v1beta1/repeat:body", errorPrefix) + var response genproto.RepeatResponse + if err := protojson.Unmarshal(responseBody, &response); err != nil { + t.Fatalf("%s could not unmarshal valid response body: %s\n response body: %s\n request: %s\n", + errorPrefix, err, string(responseBody), string(requestBody)) + } + // Then ensure the expected error occurs + responseBody, requestBody = getJSONResponse(t, request, + fmt.Sprintf("%s/v1beta1/repeat:%s", server.URL, variant), errorPrefix) + err = protojson.Unmarshal(responseBody, &response) + if err == nil { + t.Fatalf("%s did not receive an error:\n response body: %s\n request: %s\n", + errorPrefix, string(responseBody), string(requestBody)) + } + if !strings.Contains(err.Error(), "invalid value for enum type") { + t.Fatalf("%s received different error than expected: %s\n response body: %s\n request: %s\n", + errorPrefix, err, string(responseBody), string(requestBody)) + } + } } func getJSONResponse(t *testing.T, request *genprotopb.RepeatRequest, uri, errorPrefix string) (responseBody, requestBody []byte) { From ea6a882448ca9b8006d219ac2fef60ecf272f190 Mon Sep 17 00:00:00 2001 From: Victor Chudnovsky Date: Wed, 25 Aug 2021 09:16:40 -0700 Subject: [PATCH 6/7] Use non-zero value as a sentinel This allows isolating unknown enum values in specific fields. --- server/genrest/compliance_custom.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/server/genrest/compliance_custom.go b/server/genrest/compliance_custom.go index da1d85bc5..5ae651216 100644 --- a/server/genrest/compliance_custom.go +++ b/server/genrest/compliance_custom.go @@ -50,8 +50,11 @@ func (backend *RESTBackend) customRepeatWithUnknownEnumMethod(w http.ResponseWri return } - // Make sure we have at least one sentinel value before serializing properly. - sentinelValue := genprotopb.ComplianceData_LIFE_KINGDOM_UNSPECIFIED + // Make sure we have at least one sentinel value before serializing properly; we will then + // replace the sentinel value in the JSON with an unknown value. The sentinel value should + // be a non-zero value, since unset non-proto-optional fields will serialize with the zero + // value, which would result in all of these always getting the new, unknown value + sentinelValue := genprotopb.ComplianceData_ANIMALIA sentinelString := genprotopb.ComplianceData_LifeKingdom_name[int32(sentinelValue)] if response.Request == nil { response.Request = &genprotopb.RepeatRequest{} From 3b54211b1f79b3a19c59399ddd20286acf35b063 Mon Sep 17 00:00:00 2001 From: Victor Chudnovsky Date: Wed, 25 Aug 2021 10:27:57 -0700 Subject: [PATCH 7/7] Cosmetic fixes. --- cmd/gapic-showcase/compliance_custom_test.go | 10 ++++---- .../google/showcase/v1beta1/compliance.proto | 16 ++++++------- server/genrest/compliance_custom.go | 12 ++++++---- server/services/compliance_service_test.go | 2 ++ util/genrest/goviewcreator.go | 24 ++++++++++--------- 5 files changed, 35 insertions(+), 29 deletions(-) diff --git a/cmd/gapic-showcase/compliance_custom_test.go b/cmd/gapic-showcase/compliance_custom_test.go index c361b2c3c..d2d35d4c8 100644 --- a/cmd/gapic-showcase/compliance_custom_test.go +++ b/cmd/gapic-showcase/compliance_custom_test.go @@ -27,7 +27,7 @@ import ( "google.golang.org/protobuf/encoding/protojson" ) -// TestRepeatWithUnknownEnum +// TestRepeatWithUnknownEnum tests both RepeatWithUnknownEnum and RepeatWithUnknownOptionalEnum func TestRepeatWithUnknownEnum(t *testing.T) { _, server, err := complianceSuiteTestSetup() if err != nil { @@ -39,9 +39,7 @@ func TestRepeatWithUnknownEnum(t *testing.T) { resttools.JSONMarshaler.Replace(nil) defer resttools.JSONMarshaler.Restore() - request := &genprotopb.RepeatRequest{ - FInt32: 23, - } + request := &genprotopb.RepeatRequest{} for idx, variant := range []string{"invalidenum", "invalidoptionalenum"} { errorPrefix := fmt.Sprintf("[%d %q]", idx, variant) @@ -69,6 +67,8 @@ func TestRepeatWithUnknownEnum(t *testing.T) { } } +// getJSONResponse is a helper function for TestRepeatWithUnknownEnum. It issues the REST request to +// the given URI and returns both the response and request JSON bodies. func getJSONResponse(t *testing.T, request *genprotopb.RepeatRequest, uri, errorPrefix string) (responseBody, requestBody []byte) { verb := "POST" requestBody, err := resttools.ToJSON().Marshal(request) @@ -100,5 +100,3 @@ func getJSONResponse(t *testing.T, request *genprotopb.RepeatRequest, uri, error } return responseBody, requestBody } - -// repeat wtih same message for invalidenum; check for error diff --git a/schema/google/showcase/v1beta1/compliance.proto b/schema/google/showcase/v1beta1/compliance.proto index 3e4ecc8eb..8386f98c2 100644 --- a/schema/google/showcase/v1beta1/compliance.proto +++ b/schema/google/showcase/v1beta1/compliance.proto @@ -96,11 +96,11 @@ service Compliance { }; } - // This method returns an unknown enum value. Client libraries should either error gracefully (not - // crash), ignore the value, or, depending on the API, set it to the zero default. + // This method returns an unknown value for a non-optional enum field. Client libraries should + // either error gracefully (not crash), ignore the value, or, depending on the API, set it to the + // zero default. // - // This only works over REST currently. Requests over gRPC will echo the request and should - // succeed on the client side + // This only works over REST currently. Requests over gRPC will simply echo the request. rpc RepeatWithUnknownEnum(RepeatRequest) returns (RepeatResponse) { option (google.api.http) = { post: "/v1beta1/repeat:invalidenum" @@ -108,11 +108,11 @@ service Compliance { }; } - // This method returns an unknown enum value. Client libraries should either error gracefully (not - // crash), ignore the value, or, depending on the API, set it to the zero default. + // This method returns an unknown value for an optional enum field. Client libraries should either + // error gracefully (not crash), ignore the value, or, depending on the API, set it to the zero + // default. // - // This only works over REST currently. Requests over gRPC will echo the request and should - // succeed on the client side + // This only works over REST currently. Requests over gRPC will simply echo the request. rpc RepeatWithUnknownOptionalEnum(RepeatRequest) returns (RepeatResponse) { option (google.api.http) = { post: "/v1beta1/repeat:invalidoptionalenum" diff --git a/server/genrest/compliance_custom.go b/server/genrest/compliance_custom.go index 5ae651216..a4bd2d7f8 100644 --- a/server/genrest/compliance_custom.go +++ b/server/genrest/compliance_custom.go @@ -23,7 +23,8 @@ import ( "github.com/googleapis/gapic-showcase/util/genrest/resttools" ) -// customRepeatWithUnknownEnum provides REST-specific handling for a RepeatWithUnknownEnum request. It returns a JSON response with an unknown enum symbol string in an enum field. +// customRepeatWithUnknownEnum provides REST-specific handling for a RepeatWithUnknownEnum +// request. It returns a JSON response with an unknown enum symbol string in an enum field. func (backend *RESTBackend) customRepeatWithUnknownEnum(w http.ResponseWriter, r *http.Request, request *genprotopb.RepeatRequest) { mutator := func(data *genprotopb.ComplianceData, sentinelValue genprotopb.ComplianceData_LifeKingdom) { data.FKingdom = sentinelValue @@ -31,7 +32,9 @@ func (backend *RESTBackend) customRepeatWithUnknownEnum(w http.ResponseWriter, r backend.customRepeatWithUnknownEnumMethod(w, r, request, mutator) } -// customRepeatWithUnknownOptionalEnum provides REST-specific handling for a RepeatWithUnknownOptionalEnum request. It returns a JSON response with an unknown enum symbol string in an enum field. +// customRepeatWithUnknownOptionalEnum provides REST-specific handling for a +// RepeatWithUnknownOptionalEnum request. It returns a JSON response with an unknown enum symbol +// string in an enum field. func (backend *RESTBackend) customRepeatWithUnknownOptionalEnum(w http.ResponseWriter, r *http.Request, request *genprotopb.RepeatRequest) { mutator := func(data *genprotopb.ComplianceData, sentinelValue genprotopb.ComplianceData_LifeKingdom) { data.PKingdom = &sentinelValue @@ -39,7 +42,8 @@ func (backend *RESTBackend) customRepeatWithUnknownOptionalEnum(w http.ResponseW backend.customRepeatWithUnknownEnumMethod(w, r, request, mutator) } -// customRepeatWithUnknownEnumMethod provides REST-specific handling for the RepeatWithUnknown*Enum request. It returns a JSON response with an unknown enum symbol string in an enum field. +// customRepeatWithUnknownEnumMethod provides REST-specific handling for the RepeatWithUnknown*Enum +// request. It returns a JSON response with an unknown enum symbol string in an enum field. func (backend *RESTBackend) customRepeatWithUnknownEnumMethod(w http.ResponseWriter, r *http.Request, request *genprotopb.RepeatRequest, mutate enumMutator) { marshaler := resttools.ToJSON() @@ -76,5 +80,5 @@ func (backend *RESTBackend) customRepeatWithUnknownEnumMethod(w http.ResponseWri w.Write(json) } -// enumMutator represents a function that modifies data in place using sentinelValue. +// enumMutator represents a function that modifies `data` in place using `sentinelValue`. type enumMutator func(data *genprotopb.ComplianceData, sentinelValue genprotopb.ComplianceData_LifeKingdom) diff --git a/server/services/compliance_service_test.go b/server/services/compliance_service_test.go index a2e6ce122..670286277 100644 --- a/server/services/compliance_service_test.go +++ b/server/services/compliance_service_test.go @@ -57,6 +57,8 @@ func TestComplianceRepeats(t *testing.T) { server.RepeatDataPathTrailingResource, server.RepeatDataBodyPut, server.RepeatDataBodyPatch, + server.RepeatWithUnknownEnum, + server.RepeatWithUnknownOptionalEnum, } { response, err := rpc(context.Background(), request) if err != nil { diff --git a/util/genrest/goviewcreator.go b/util/genrest/goviewcreator.go index 43b369bdc..8b52e179e 100644 --- a/util/genrest/goviewcreator.go +++ b/util/genrest/goviewcreator.go @@ -24,15 +24,6 @@ import ( "github.com/googleapis/gapic-showcase/util/genrest/resttools" ) -var customFunctions map[string]string - -func init() { - customFunctions = map[string]string{ - ".google.showcase.v1beta1.Compliance.RepeatWithUnknownEnum": "customRepeatWithUnknownEnum", - ".google.showcase.v1beta1.Compliance.RepeatWithUnknownOptionalEnum": "customRepeatWithUnknownOptionalEnum", - } -} - // NewView creates a a new goview.View (a series of files to be output) from a gomodel.Model. The // current approach is to generate one file per service, with that file containing all the service's // RPCs. An additional file `genrest.go` is also created to register all these handlers with a @@ -201,9 +192,8 @@ func NewView(model *gomodel.Model) (*goview.View, error) { methodId := fmt.Sprintf("%s.%s", service.ProtoPath, handler.GoMethod) customHandler, _ := customFunctions[methodId] - source.P(" // %s", methodId) if len(customHandler) > 0 { - source.P(" backend.%s(w, r, request) // %s", customHandler, methodId) + source.P(" backend.%s(w, r, request)", customHandler) source.P("}") continue } @@ -372,7 +362,19 @@ func (namer *Namer) Get(newName string) string { var license string +// customFunctions contains a map of fully qualified RPC names to their manually written REST +// handlers (in package gapic-showcase/server/genrest). For these RPCs, the generated code created +// by this file calls these custom handlers, instead of delegating to the core gRPC server +// implementation as most of the REST handlers do. This allows Showcase to provide REST-specific +// behavior in some scenarios. +var customFunctions map[string]string + func init() { + customFunctions = map[string]string{ + ".google.showcase.v1beta1.Compliance.RepeatWithUnknownEnum": "customRepeatWithUnknownEnum", + ".google.showcase.v1beta1.Compliance.RepeatWithUnknownOptionalEnum": "customRepeatWithUnknownOptionalEnum", + } + license = fmt.Sprintf(`// Copyright %d Google LLC // // Licensed under the Apache License, Version 2.0 (the "License");