make context ref mut

[axos88]

** Breaking change **

This PR makes removes the Clone and Copy from context::Context, and passes it by mutable reference to the functions.

Reasoning:

This PR is in preparation to a proper solution to #392 and a reimplementation of #393 based on the discussions there.

Once context::Context is passed around as a mutable reference instead of cloning, we can extend it with an AnyMap and allow the transport sink, transport stream and service communicate by mutating the context itself.
That will be necessary to finally properly implement an mqtt transport.

I tried a few other solutions to walking around the boxing, but unfortunately hit a rust limitation where I couldn't express that Fut: 'a, so the compiler complains that it doesn't live long enough:

That non-compiling code is available in https://github.com/axos88/tarpc/tree/failed-attempt-at-mutable-ref-context

impl<Req, Resp, Fut, F> Serve for ServeFn<Req, Resp, F>
where
    Req: RequestName,
    for<'a> F: FnOnce(&mut context::Context, Req) -> Fut + 'a,
    Fut: Future<Output = Result<Resp, ServerError>>,
{
    type Req = Req;
    type Resp = Resp;
    type Fut<'a> = Fut where Self: 'a, Req: 'a, Resp: 'a;

    fn serve<'a>(self, ctx: &'a mut context::Context, req: Req) -> Self::Fut<'a> where Self: 'a, Req: 'a, Resp: 'a {
        (self.f)(ctx, req)
    }
}

[tikue]

Thanks for working on a solution to MQTT! Do you have a branch with a working MQTT transport? I would like to see how everything fits together before accepting this PR, which will be a large breaking change. Also, it's been a while since I've thought about this problem and forgot most of the details! 😰

One immediate question:

• Is there any way to keep Context: Clone but not Copy and have it still passed by value? For example, could the AnyMap be reference-counted or copy-on-write? It looks like Context is never sent to the dispatch which suggests the additional state will only be used for a quick one-time calculation before sending a request?

[axos88]

Not yet, i will add a another pr adding the anymap to the context, and then redo the transport on top of that, so its still a wip.

The goal of raising this pr early would just like you to take a look if its a good direction, and if all things work out it can be merged later down the road.

The pr for 0.35 was abandoned and doesnt conpile on current rust anymore, so i want to avoid that :)

Once anymap is added i dont think we could easily keep context clone, unless we restrict the contect to contain only clone values.

The main issue with sending by value is that you cannot modify it within a request unless you return the modified version, but thats another can of worms both technically and ergonimically.

Another issue is that you need to keep anymap not only sync, but also send that way.

Tbh not looking at how large a breaking change it is, i believe it is much cleaner to send the context in as &mut, since it expresses intent clearly.

Once RTN is stabilized we could probably get rid of the boxings, but i havent tried that yet.

[tikue]

Thanks for explaining! What kinds of modifications will be performed on the context on a per-request basis?

I think my main concern is this design introduces a bifurcation of the request context: the client stub sees one version, and the client dispatch sees another. It's a little hard for me to reason through the implications right now, but I worry this introduces the possibility of losing important parts of the context that are needed by the dispatch.

Looking forward to seeing the working transport!

[axos88]

For my transport, modification of the context would not be required, however I envision use cases where that would/could be useful, such as error reporting, authn/authz, etc.

Your point is totally valid, I will need some time to reason through it, but I only get a few minutes here and there to work on this, but I believe the context would not necessarily need to be bifurcated, if the modified context is sent back to the client - although the bifurcation is already the case if the server modifies it in a hook isn't it?

Let me work through extending context with the anymap, and see where it leads.

[axos88]

I've added two more PRs, but will need modifications, because the Transport is wired up to Stream and Sink Request and Response, which only contain the SharedContext, so will need some work to allow the transport to access the server / client state.

[axos88]

@tikue, it is now working end to end, but a lot more cleanup work is needed. You can see the working mqtt transport and an example for usage here:

https://github.com/axos88/tarpc-mqtt-transport/tree/master/src

[axos88]

Hey @tikue, any chance you could take a look at this after the holidays?