hyperium / tonic

A native gRPC client & server implementation with async/await support.
https://docs.rs/tonic
MIT License
10.02k stars 1.02k forks source link

How can I use different codec based on the 'Content-Type' header? #851

Open zbysir opened 2 years ago

zbysir commented 2 years ago

Feature Request

Is there any way I can use different codec implementations based on the 'Content-Type' header?

Motivation

My requirement is to make this grpc service support multiple protocols (codec), which is based on the content-type parameter of the request header.

Proposal

Codec trait is too complicated, I have no idea. Just give me some tips.

zbysir commented 2 years ago

Happily, I seem to have found a way. I modified the source code of tonic-build as follows:

            let ct = req.headers().get("content-type").unwrap();
            if (ct == "application/grpc"){
                let codec = tonic::codec::ProstCodec::default();

                let mut grpc = tonic::server::Grpc::new(codec)
                    .apply_compression_config(accept_compression_encodings, send_compression_encodings);

                let res = grpc.unary(method, req).await;
                Ok(res)
            } else {
                let codec = crate::JsonCodec::default();

                let mut grpc = tonic::server::Grpc::new(codec)
                    .apply_compression_config(accept_compression_encodings, send_compression_encodings);

                let res = grpc.unary(method, req).await;
                Ok(res)
            }

It is worked. Is there a better way to implement it?

davidpdrsn commented 2 years ago

I think this is something thats worth supporting! Feels like a good extension to the whole codec system.

However I've never done anything like this myself so not sure what the best approach would be and don't have the bandwidth to work on it at the moment.

zbysir commented 2 years ago

Happily, I seem to have found a way. I modified the source code of tonic-build as follows:

            let ct = req.headers().get("content-type").unwrap();
            if (ct == "application/grpc"){
                let codec = tonic::codec::ProstCodec::default();

                let mut grpc = tonic::server::Grpc::new(codec)
                    .apply_compression_config(accept_compression_encodings, send_compression_encodings);

                let res = grpc.unary(method, req).await;
                Ok(res)
            } else {
                let codec = crate::JsonCodec::default();

                let mut grpc = tonic::server::Grpc::new(codec)
                    .apply_compression_config(accept_compression_encodings, send_compression_encodings);

                let res = grpc.unary(method, req).await;
                Ok(res)
            }

It is worked. Is there a better way to implement it?

Maybe this feature should be implemented by ‘tonic-build’?

davidpdrsn commented 2 years ago

Sure technically it could. The real question is in designing a good API for it, defining the use cases we want to support, and making sure it fits with where we want tonic to go.

zbysir commented 2 years ago

The implementation of JsonCodec can refer to here: https://github.com/hyperium/tonic/pull/853

zbysir commented 2 years ago

Sure technically it could. The real question is in designing a good API for it, defining the use cases we want to support, and making sure it fits with where we want tonic to go.

Hi, davidpdrsn. I am trying the following API to configuration codec, do you think this is the right way?

fn main() -> Result<(), Box<dyn std::error::Error>> {
    tonic_build::configure()
        .codec("application/grpc | application/grpc+proto", "tonic::codec::ProstCodec")
        .codec("application/json | application/grpc+json", "crate::lib::codec::JsonCodec")
        .type_attribute(".", "#[derive(serde_derive::Deserialize, serde_derive::Serialize)]")
        .compile(&["proto/helloworld.proto"],&["proto"])?;
    Ok(())
}

The generated code is as follows: image

I hope to improve it and contribute to this repository, thank you for your help.

YHM404 commented 6 months ago

Hi, any update on this?