smartive / zitadel-rust

An implementation of the ZITADEL gRPC API in Rust. Complemented with other useful elements such as ServiceAccount auth.
https://docs.rs/zitadel/latest/zitadel/
Other
42 stars 16 forks source link

Implement Clone for ChainedInterceptor #542

Closed c-thiel closed 2 weeks ago

c-thiel commented 5 months ago

First of all thanks for this great create!

The Problem

Tonic Service Client are designed to be cloned in Multi-Threading/Multiplexing scenarios. There is a section "Multiplexing-Requests" on this in the tonic docs: https://docs.rs/tonic/latest/tonic/transport/struct.Channel.html#multiplexing-requests

As a result a tonic client implements Clone. When using a client build with this crate, a XXXServiceClient<InterceptedService<TonicChannel, ChainedInterceptor>> is returned. Everything except ChainedInterceptor implements Clone.

I need to use the client in a Multiplexing scenario and require the client to implement a lightweight clone to avoid opening a large number of channels.

The Solution

... is not as trivial as I would have though because ServiceAccountInterceptor requires mutability for the token. I changed it to use Interior Mutability instead.

I also had to introduce a new Interceptor trait which I don't like. The only reason for the new interceptor trait is the ChainedInterceptor chain method which runs calls via the original Interceptor trait on &mut self. This breaks my interior mutability approach though.

I would argue that even if you decide to stick to the old implementation of the ChainedInterceptor, the changed ServiceAccountInterceptor and AccessTokenInterceptor are better off using interior mutability and can still be used in clone-able clients even though we couldn't use the ClientBuilder to obtain them.

PR is coming in a second, let me know if you have any better ideas!

buehler commented 2 weeks ago

Done via #566