eclipse-vertx / vertx-grpc

Development of the gRPC component for Eclipse Vert.x
Eclipse Public License 2.0
42 stars 23 forks source link

Grpc transcoding #95

Open zZHorizonZz opened 4 months ago

zZHorizonZz commented 4 months ago

Describe the feature

This feature request proposes gRPC transcoding capabilities for the server. This would allow the server to accept json formatted grpc requests and transcode them according to the specifications defined in google.api.http.

Use cases

While grpc web is already implemented, it utilizes a unique format that differs from the transcoding of REST requests into grpc requests. Implementing this feature would be particularly useful when publicly exposing a grpc service, where supporting web based requests or providing users with a REST api option is desired.

Contribution

I am willing to contribute to the implementation of this feature. I have already implemented this in quarkus repository. After discussion in this PR, the maintainers concluded that this should be implemented directly in grpc server implementation.

vietj commented 4 months ago

@zZHorizonZz we would be pleased to have this feature, perhaps we can discuss how the implementation should be crafted for the vertx-grpc project ?

zZHorizonZz commented 4 months ago

@zZHorizonZz we would be pleased to have this feature, perhaps we can discuss how the implementation should be crafted for the vertx-grpc project ?

In my Quarkus PR, I needed to create custom GrpcReadStream and GrpcServerRequest implementations. This was necessary to merge path and query parameters into a JSON request. Also to find the appropriate marshallers, unmarshallers, etc, based on the current path, I needed to implement my own GrpcServer as well.

During debugging, I noticed that the code attempted to read a grpc prefix from the buffer, which doesn't exist in JSON requests. To fix this, I reimplemented several classes, including ReadStreamAdapter, WriteStreamAdapter, MessageDecoder, and MessageEncoder. In a previous conversation (https://github.com/quarkusio/quarkus/discussions/39991#discussioncomment-9073291), it was mentioned that Quarkus uses Vert.x 4. After going trough the code, I found that in newer versions you have added grpc web, which can read JSON from requests. This should resolve the grpc prefix issue. I think?

However, there might still be problems in determining the correct request marshallers, unmarshallers based on its path and merging path and query parameters into the body.

vietj commented 4 months ago

we are planning to backport grpc-web to vertx 4, so we can assume developing your PR against master can be done and then we can backport everything to vertx 4.

zZHorizonZz commented 4 months ago

Before I start implementing it, I will need to go through the code of grpc-web so that I understand it better, as I reckon the transcoding implementation will be using some elements from grpc-web.

vietj commented 4 months ago

sure @zZHorizonZz , there might be some decoupling / refactoring necessary because the current grpc-web integration uses inheritance with calls to super to override the behaviour for grpc-web sometimes

vietj commented 4 months ago

we can also schedule a call to discuss the plans if you are inclined

zZHorizonZz commented 4 months ago

I wouldn't mind getting into a call, although I will be busy until the end of the month, so it would be in 2-3 weeks.