refactor: Add associated types to Service, use protocol enum as Service

[Frando]

Based on #47 (can rebase on main if needed)
Alternative to #41
Fixes #39

This PR simplifies working with irpc:

• Instead of defining a separate FooService struct, the Service trait is implemented on the protocol enum. One type less to create.

• Added an associated types to the Service trait to point to the extended message enum (the one where the variants contain the WithChannels structs). By doing this, we can reduce the generics on the Client from 3 to 1, and on the LocalSender from 2 to 1. I think this is a net benefit for all users.

• The service trait is now implemented by the proc macro if the message argument is provided.

• A new trait RemoteService: Service has a required method to create a aService::Message from the protocol enum, and a provided method to create a Handler for use with the listen function. The RemoteService impl is generated by the proc macro. This saves the tedious manual impl of mapping from msg, rx, tx to the message enum (see the diff in the examples).

• Added no_rpc and no_spans arguments to the macro to omit generating code that depends on the rpc or spans features of irpc

• Expanded and improved the documentation of the rpc_requests macro. Also moved it from irpc_derive to irpc to be able to add doc links to items from irpc.

First and foremost, have a look at the diff to the examples.

[rklaehn]

Instead of defining a separate Service struct, we can simply use the XxxProtocol struct as the service! One type less to define and manage.

That was always possible, since the service struct was nowhere needed by value. But nevertheless, nice simplification.

In general I like this. It creates a closer coupling between the service and the enums, so you can no longer have e.g. an enum that claims to handle service Foo and handles all cases required for Foo and some additonal internal messages. But I guess in tokio actors the best way to handle internal messages is via a second channel anyway, so it might be OK.

[rklaehn]

Currently giving this a spin in a new repo.

One thing I noticed - the docs are not yet updated, still have the many type parameters.

A client to the service S using the local message type M and the remote message type R.
[LocalSender](https://github.com/n0-computer/irpc/pull/struct.LocalSender.html)
A local sender for the service S using the message type M.

[rklaehn]

So the first parameter of the macro is the "full" enum containing each case with channels and span?

Is the service trait implemented for the serializable message enum or for the full enum?

I am playing around with this in a separate repo, and am a bit confused. I got it to work, like this:

#[rpc_requests(TestMessage, alias = "Msg")]
#[derive(Debug, Serialize, Deserialize)]
enum TestProtocol {
    #[rpc(tx = oneshot::Sender<String>)]
    Put(PutRequest),
    #[rpc(tx = mpsc::Sender<String>)]
    Get(GetRequest),
}

But I can't see the type aliases for the full cases:

fn foo(msg: PutRequestMsg) -> String { // does not compile!
    format!("Received Put request with key: {}, value: {}", msg.key, msg.value)
}

And if I point the macro to an explicit service, I get an error as well:

#[rpc_requests(TestMessage, service = TestService, alias = "Msg")]
#[derive(Debug, Serialize, Deserialize)]
enum TestProtocol {
    #[rpc(tx = oneshot::Sender<String>)]
    Put(PutRequest),
    #[rpc(tx = mpsc::Sender<String>)]
    Get(GetRequest),
}

=>

the trait bound `TestProtocol: Service` is not satisfied
the trait `Service` is not implemented for `TestProtocol``

So Service must always be implemented for the Protocol enum? Then what is the service parameter for?

[rklaehn]

Docs for the rpc_requests macro are out of date.

[Frando]

Yeah, docs are not updated yet - will do this once we decide to go forward with this to not do it multiple times.

So the first parameter of the macro is the "full" enum containing each case with channels and span?

Yes.

Is the service trait implemented for the serializable message enum or for the full enum?

It is implemented for the serializable message enum (i.e. the enum people usually write themselves and which has the macro on it).

I am playing around with this in a separate repo, and am a bit confused. I got it to work, like this:

It works but the alias is called PutMsg not PutRequestMsg. I don't think I changed this on this PR, can you check if it's the same on main?

This work for me:

Type aliases

use irpc::{
    channel::{mpsc, oneshot},
    rpc_requests,
};
use serde::{Deserialize, Serialize};

#[derive(Debug, Serialize, Deserialize)]
struct PutRequest {
    key: String,
    value: String,
}

#[derive(Debug, Serialize, Deserialize)]
struct GetRequest {
    key: String,
}

#[rpc_requests(TestMessage, alias = "Msg")]
#[derive(Debug, Serialize, Deserialize)]
enum TestProtocol {
    #[rpc(tx = oneshot::Sender<String>)]
    Put(PutRequest),
    #[rpc(tx = mpsc::Sender<String>)]
    Get(GetRequest),
}

fn foo(msg: PutMsg) -> String {
    // does not compile!
    format!(
        "Received Put request with key: {}, value: {}",
        msg.inner.key, msg.inner.value
    )
}

And if I point the macro to an explicit service, I get an error as well:

Right, there was a bug, this is now fixed. This gist works:

Custom service

use irpc::{
    channel::{mpsc, oneshot},
    rpc_requests, Service,
};
use serde::{Deserialize, Serialize};

#[derive(Debug, Serialize, Deserialize)]
struct PutRequest {
    key: String,
    value: String,
}

#[derive(Debug, Serialize, Deserialize)]
struct GetRequest {
    key: String,
}

#[derive(Debug)]
struct TestService;

impl Service for TestService {
    type Message = TestMessage;
    type WireMessage = TestProtocol;
}

#[rpc_requests(TestMessage, alias = "Msg", service = TestService)]
#[derive(Debug, Serialize, Deserialize)]
enum TestProtocol {
    #[rpc(tx = oneshot::Sender<String>)]
    Put(PutRequest),
    #[rpc(tx = mpsc::Sender<String>)]
    Get(GetRequest),
}

fn foo(msg: PutMsg) -> String {
    // does not compile!
    format!(
        "Received Put request with key: {}, value: {}",
        msg.inner.key, msg.inner.value
    )
}

fn main() {}

So Service must always be implemented for the Protocol enum? Then what is the service parameter for?

It doesn't have to be implemented on the Protocol enum. The default is that the rpc_requests macros adds an impl of Service for the Protocol enum. But you could provide your own with a service parameter. Maybe this is not needed though, I can't really think of situations where you'd want to use the macro but set it to a different service. Not sure?

[rklaehn]

I can't think of a concrete downside of "protocol enum is service", other than some abstract things that are probably best handled at a higher level anyway. So I would vote for just making this the default. Then you would not need any arg on the macro for standard use. It just builds you the second enum and a bunch of from conversions and the like.

[Frando]

Yes, the latest commit impls that.

[rklaehn]

Yes, the latest commit impls that.

Thanks, will give it another test drive today.

[Frando]

Replying to #51 (comment) which is more fitting here:

Generation of the message enum is optional? What exactly does the macro do if you don't specify the message parameter?

If message is not set, it only generates the Channels impls for the request structs and the From impls to convert from request struct to protocol enum (and the type aliases if alias is set).

I tried the no_rpc flag. It seems that the only thing the no_rpc flag controls so far is generating the remote service impl.

Correct. This is the only part of the generated code that depends on the rpc feature flag of irpc being enabled.

I tried the no_spans flag, but there are still spans in the enums. If we want to have no_spans at all, it should make sure that there are no spans in the enum cases.

I don't think the macro has any effect on that. The generated code only does WithChannels::from((msg, tx, rx)). Whether WithChannels includes spans or not depends on whether the spans feature of irpc is enabled or not. Likely your test had it enabled due to feature unification.

[Frando]

I added a commit that improves the docs further. I think this is ready to go from my side.

Maybe it's better to make the message argument required? I'm not sure if there's valid use case for the macro without having it generate the message enum and service impl.

[rklaehn]

I still got a few bikesheds, mostly about naming.

Is rpc_requests still the right name for the macro? Are no_rpc and no_spans good names? E.g. I was a bit confused that no_spans does not mean that you don't have spans, just that it does not generate the associated code using spans.

But we can do this in a subsequent PR. The macro docs are very good now.