metaverse / truss

Truss helps you build go-kit microservices without having to worry about writing or maintaining boilerplate code.
Other
736 stars 144 forks source link

gRPC HTTP body mapping works incorrectly #325

Open chebykinn opened 3 years ago

chebykinn commented 3 years ago

Hi, I've noticed some inconsistency with grpc http transcoding.

Here's how to reproduce it:

  1. Take example proto schema from google doc (https://cloud.google.com/endpoints/docs/grpc/transcoding#map_a_create_method) Full schema:
    
    syntax = "proto3";

package shelf;

import "github.com/metaverse/truss/deftree/googlethirdparty/annotations.proto";

service Shelfs {

// Creates a new shelf in the bookstore. rpc CreateShelf(CreateShelfRequest) returns (Shelf) { // Client example: // curl -d '{"theme":"Music"}' http://DOMAIN_NAME/v1/shelves option (google.api.http) = { post: "/v1/shelves" body: "shelf" }; } }

// Request message for CreateShelf method. message CreateShelfRequest { // The shelf resource to create. Shelf shelf = 1; }

// A shelf resource. message Shelf { // A unique shelf id. int64 id = 1; // A theme of the shelf (fiction, poetry, etc). string theme = 2; }

2. Generate and run server
3. Add debug printing of Shelf field to CreateShelf:
```go
func (s shelfsService) CreateShelf(ctx context.Context, in *pb.CreateShelfRequest) (*pb.Shelf, error) {
    fmt.Printf("in: %v\n", in.Shelf)
    var resp pb.Shelf
    return &resp, nil
}
  1. Make curl request as suggested in the example: curl -d '{"theme":"Music"}' 'http://localhost:5050/v1/shelves' The server does not set the Shelf field:
    2021/02/07 00:16:57 transport gRPC addr :5040
    2021/02/07 00:16:57 transport debug addr :5060
    2021/02/07 00:16:57 transport HTTP addr :5050
    in: <nil>

    If you wrap the body with the shelf object, like curl -d '{"shelf": {"theme":"Music"}}' 'http://localhost:5050/v1/shelves' the field will be correctly set to theme:"Music"

chebykinn commented 3 years ago

Just noticed that wildcard mapping does not work as intended as well:

    option (google.api.http) = {
      post: "/v1/shelves/{shelf_id}"
      body: "*"
    };

Still have to send the body with nested object: '{"shelf": {"theme":"Music"}}'

UPD: Nevermind, this is working fine.

zaquestion commented 3 years ago

interesting, ive never looked that closely at how the mapping is supposed to function, it seems like there is a mismatch in what truss does and the spec.

The wildcard case i would expect to need to be wrapped too? youre saying this allowed you to remove the outer "shelf" json?

@adamryman can you shed any insight on this and advise on fixing?

chebykinn commented 3 years ago

No, the wildcard should take the whole request body as is, except fields that are mapped in the request path, so in this case "shelf" object should not be removed.