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

Generated HTTP transport can't cope with different HTTP verbs on the same URI #209

Closed
pauln opened this issue Jul 26, 2017 · 2 comments
Closed

Comments

@pauln
Copy link
Contributor

pauln commented Jul 26, 2017

The HTTP transport generated by Truss doesn't appear to handle using the same URI with different HTTP verbs (i.e. GET vs POST) for different RPC methods. This means that it's not able to generate a clean RESTful API, as you'd normally have something along the lines of:

service Foobar {
  rpc GetFoos (GetFoosRequest) returns (GetFoosResponse) {
    option (google.api.http) = {
      get: "/foo"
    };
  }

  rpc CreateFoo (CreateFooRequest) returns (CreateFooResponse) {
    option (google.api.http) = {
      post: "/foo"
      body: "*"
    };
  }
}

However, this results in the following error:

panic: http: multiple registrations for /foo

goroutine 23 [running]:
net/http.(*ServeMux).Handle(0xc04216e1e0, 0x99d904, 0x14, 0xbf60a0, 0xc042168200)
	C:/Go/src/net/http/server.go:2254 +0x617
foobar/foobar-service/svc.MakeHTTPHandler(0xf8c438, 0xc0420384a0, 0xc042137c40, 0xc042137c60, 0xc042137c80, 0xc042137ca0, 0xbf5fe0, 0xc04216e180, 0x0, 0x0)
	C:/Users/Paul/go/src/foobar/foobar-service/svc/transport_http.go:68 +0x2a8
foobar/foobar-service/svc/server.Run.func2(0xbf5fe0, 0xc04213e630, 0xf8c438, 0xc0420384a0, 0xc042137c40, 0xc042137c60, 0xc042137c80, 0xc042137ca0, 0x992e0a, 0x5, ...)
	C:/Users/Paul/go/src/foobar/foobar-service/svc/server/run.go:91 +0x1fa
created by foobar/foobar-service/svc/server.Run
	C:/Users/Paul/go/src/foobar/foobar-service/svc/server/run.go:94 +0x4f6
@matthewhartstonge
Copy link
Contributor

matthewhartstonge commented Jul 26, 2017

Having a think on this a cleaner approach would be to use Gorilla Mux.

Currently any sub-resource will collide:

/v1/cats
/v1/cats/{catId} // ded
/v1/cats/{catId}/kittens // ded
/v1/cats/{catId}/kittens/{kittenId} // ded (Not ded kittens! D:)

If continuing to use stdlib you get into the issue of having to deal with generating code based on r.Method() whereas with gorilla mux you can continue to generate code how you are, but add a .Methods("GET") to the end of the generated HTTP handler.

For example:

	m := http.NewServeMux()

	m.Handle("/v1/cats", httptransport.NewServer(
		ctx,
		endpoints.CreateCatEndpoint,
		DecodeHTTPCreateCatZeroRequest,
		EncodeHTTPGenericResponse,
		serverOptions...,
	))

	m.Handle("/v1/cats", httptransport.NewServer(
		ctx,
		endpoints.ReadCatsEndpoint,
		DecodeHTTPReadCatsZeroRequest,
		EncodeHTTPGenericResponse,
		serverOptions...,
	))

Becomes:

	m := mux.NewRouter()

	m.Handle("/v1/cats", httptransport.NewServer(
		ctx,
		endpoints.CreateCatEndpoint,
		DecodeHTTPCreateCatZeroRequest,
		EncodeHTTPGenericResponse,
		serverOptions...,
	)).Methods("POST")

	m.Handle("/v1/cats", httptransport.NewServer(
		ctx,
		endpoints.ReadCatsEndpoint,
		DecodeHTTPReadCatsZeroRequest,
		EncodeHTTPGenericResponse,
		serverOptions...,
	)).Methods("GET")

Also means pb route notation like this would be supported as well:

r.HandleFunc("/articles/{category}/", ArticlesCategoryHandler)
r.HandleFunc("/articles/{category}/{id:[0-9]+}", ArticleHandler)

And route params would be accessible via:

vars := mux.Vars(request)
category := vars["category"]

@adamryman
Copy link
Member

@pauln we are aware of this issue though we have not had the resource to solve it and our use case of truss does not require it.

We would love community support in adding this feature though.

@matthewhartstonge tooks the words out of my mouth when offering his solution to this problem.

Gorilla mux was going to be our approach for fixing this. Though it could be a semi large undertaking.

You can see how a go-kit service would use gorilla with this example https://github.com/go-kit/kit/blob/master/examples/profilesvc/transport.go#L14

Thanks for all the feedback @pauln, it has been refreshing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants