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.

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 RPCs returning unknown enum values over REST #856

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
102 changes: 102 additions & 0 deletions cmd/gapic-showcase/compliance_custom_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
// 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 (
"fmt"
"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 tests both RepeatWithUnknownEnum and RepeatWithUnknownOptionalEnum
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{}

for idx, variant := range []string{"invalidenum", "invalidoptionalenum"} {
errorPrefix := fmt.Sprintf("[%d %q]", idx, variant)

// 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))
}
}
}

// 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)
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
}
2 changes: 1 addition & 1 deletion cmd/gapic-showcase/compliance_suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
23 changes: 23 additions & 0 deletions schema/google/showcase/v1beta1/compliance.proto
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,29 @@ service Compliance {
};
}

// 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 simply echo the request.
rpc RepeatWithUnknownEnum(RepeatRequest) returns (RepeatResponse) {
option (google.api.http) = {
post: "/v1beta1/repeat:invalidenum"
body: "*"
};
}

// 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 simply echo the request.
rpc RepeatWithUnknownOptionalEnum(RepeatRequest) returns (RepeatResponse) {
Comment on lines +104 to +116
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could these be one request with two fields and maybe a field to toggle behavior? For example, we have the Echo RPC which could return either an EchoResponse or an error status depending on what was sent in the request oneof. Might simplify the implementation a bit?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've been thinking about this (and sidetracked by other issues, sorry). We could keep just the previously existing RPCs and add a top-level field to RepeatRequest to generate the unknown enum. In fact, I would change RepeatRequest.server_verify to be an enum like server_behavior that could be verify, or generate_unknown_enum, or something else in the future.

Not too concerned about that being a breaking change because (a) most generators are not yet using the compliance suite, and (b) the compliance suite is meant to be parsed form this repo and then used to issue requests, without the client code modifying the request.

WDYT?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah that sounds great to me. Thanks for considering this.

option (google.api.http) = {
post: "/v1beta1/repeat:invalidoptionalenum"
body: "*"
};
}
}

message RepeatRequest {
Expand Down
6 changes: 4 additions & 2 deletions server/genrest/README.md
Original file line number Diff line number Diff line change
@@ -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).
84 changes: 84 additions & 0 deletions server/genrest/compliance_custom.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
// 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 (
"bytes"
"context"
"net/http"

genprotopb "github.com/googleapis/gapic-showcase/server/genproto"
"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.
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)
if err != nil {
// TODO: Properly handle error. Is StatusInternalServerError (500) the right response?
backend.Error(w, http.StatusInternalServerError, "server error: %s", err.Error())
return
}

// 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{}
}
if response.Request.Info == nil {
response.Request.Info = &genprotopb.ComplianceData{}
}
mutate(response.Request.Info, sentinelValue)

json, err := marshaler.Marshal(response)
if err != nil {
backend.Error(w, http.StatusInternalServerError, "error json-encoding response: %s", err.Error())
return
}

// Change the sentinel string to an unknown value.
json = bytes.ReplaceAll(json, []byte(sentinelString), []byte("LIFE_KINGDOM_NEW"))

w.Write(json)
}

// enumMutator represents a function that modifies `data` in place using `sentinelValue`.
type enumMutator func(data *genprotopb.ComplianceData, sentinelValue genprotopb.ComplianceData_LifeKingdom)
8 changes: 8 additions & 0 deletions server/services/compliance_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,14 @@ 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)
}

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
Expand Down
2 changes: 2 additions & 0 deletions server/services/compliance_service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
21 changes: 21 additions & 0 deletions util/genrest/goviewcreator.go
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,15 @@ 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]
if len(customHandler) > 0 {
source.P(" backend.%s(w, r, request)", customHandler)
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 {")
Expand Down Expand Up @@ -353,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");
Expand Down