Consider an IntoRequest trait

[hawkw]

Feature Request

Crates

• tonic

Motivation

Currently, Tonic's Client API requires users to write code like this:

    let request = tonic::Request::new(HelloRequest {
        name: "hello".into(),
    });

    let response = client.say_hello(request).await?;

The need to wrap the request struct in a tonic::Request is a minor ergonomics issue that could be surprising. This is admittedly a pretty minor papercut, but it seems like it would reduce some friction in the APIs that I suspect a new user is most likely to use (and make the basic examples seem simpler!)...

Proposal

What do you think about changing the client API so that users could just write:

    let request = HelloRequest {
        name: "hello".into(),
    };

    let response = client.say_hello(request).await?;

We could add a trait like this:

pub trait IntoRequest {
    type ReqMessage;
    fn into_request(self) -> tonic::Request<Self::ReqMessage>
    where
        Self: Sized,
    ;
}

and generate impls for all the generated request messages, like

impl tonic::IntoRequest for #request {
    type ReqMessage = Self;
    fn into_request(self) -> tonic::Request<Self>
    where
        Self: Sized,
    {
        tonic::Request::new(self)
    }
}

Then, we could change the generated RPC methods on the client to be like this:

async fn #ident(&mut self, request: impl tonic::IntoRequest<ReqMessage = #request>) 

We could add an impl of IntoRequest for Request<T> like this:

impl<T> IntoRequest for Request<T> {
    type ReqMessage = T;
    fn into_request(self) ->Request<T>
    where
        Self: Sized,
    {
         self
    }
}

which would allow users could still construct tonic::Request types if needed, such as when manually adding headers etc.

Something similar could probably be done for response messages.

Alternatives

We could also consider using From and Into here, rather than defining a new IntoRequest trait. This is what I originally suggested in #1 (comment). Ideally, I think it would be better to use the stdlib traits where possible, rather than defining a new one. However, @LucioFranco says that this causes some issues with type inference which a new trait could possibly avoid.

[ChetanBhasin]

Hey! I'm fairly new to this project, but the library seems fantastic so far!

This looks like a nice first issue. I was hoping to take a crack at it sometime this week or over the weekend if a PR is welcome. 😃

[hawkw]

@ChetanBhasin I'm not sure if this is actually going to be easy, or even possible — I took a crack at it earlier, and ran into some issues with type inference. If you want to take a look at it, you may have more success than I did, but I just wanted to warn you.

[carllerche]

For this to work out, there would need to be two traits. IntoRequest and IntoStreamingRequest. Otherwise, you get into coherence problems.

Sketch:

struct Request<T>(T);

trait Message: std::fmt::Display {}

impl Message for &'static str {}

trait IntoRequest {
    type Message: Message;
    
    fn into_request(self) -> Request<Self::Message>;
}

trait IntoStreamingRequest {
    type Stream: Iterator<Item = Self::Message>;
    type Message: Message;
    
    fn into_streaming_request(self) -> Request<Self::Stream>;
}

impl<T: Message> IntoRequest for Request<T> {
    type Message = T;
    
    fn into_request(self) -> Self {
        self
    }
}

impl<T: Iterator> IntoStreamingRequest for Request<T> where T::Item: Message {
    type Stream = T;
    type Message = T::Item;
    
    fn into_streaming_request(self) -> Self {
        self
    }
}

impl<T: Iterator> IntoStreamingRequest for T where T::Item: Message {
    type Stream = T;
    type Message = T::Item;
    
    fn into_streaming_request(self) -> Request<Self> {
        Request(self)
    }
}

fn say_hello<T: IntoStreamingRequest<Message = &'static str>>(t: T) {
    println!("== say hello");
    for msg in t.into_streaming_request().0 {
        println!(" + {}", msg);
    }
}

fn main() {
    say_hello(vec!["hello", "world"].into_iter());
    say_hello(Request(vec!["hello", "world"].into_iter()));
}

[hawkw]

Oh, the solution of adding a separate trait for streaming requests is obvious in hindsight, nice!