Client should implement Clone

[c-thiel]

Currently the signature of the publish_event method of the client is:

    pub async fn publish_event<S>(
        &mut self,
        ...
    ) -> Result<(), Error>

This requires Ownership of the client for every event we emit. This is an extreme and unusual harsh restriction if we comprare the client for example to a MongoDB or other database clients. Is this really neccessary?

In a typical Scenario we will have mutliple sources for events (maybe just the same object multiple times). Due to rust Ownership rules, each object would need it's own client. Alternatively we could work with a central service that get's data via a threadsafe Stream. However even then the central service would be hard to implement due to the required ownership.

It would be much nicer if we could use such methods on a borrowed client instead. This would allow us to simply pass references to one client to all the places that need it.

Is there some dapr specific motivation for this strict implementation? I guess the question is: What do we need to modify on the client side when sending an event?

This issue might be related: https://github.com/dapr/rust-sdk/issues/52

[c-thiel]

I dug a bit deeper: The problem is actually not the &mut self, but a missing clone. The &mut self is required due to the tonic transport channel limitations: https://docs.rs/tonic/latest/tonic/transport/struct.Channel.html

That is fine, because, as it states in the doc, cloning a Channel is cheap. Even further: Also the tonic generated client is based on the same principal (Extract from https://docs.rs/tonic/latest/tonic/client/index.html):

"Upon using the your generated client, you will discover all the functions corresponding to your rpc methods take &mut self, making concurrent usage of the client difficult. The answer is simply to clone the client, which is cheap as all client instances will share the same channel for communication. For more details, see transport::Channel."

Thus, we should simply implement Clone for the Dapr Client, as it is already implemented for the tonic client.

[danielgerlag]

I don't think the API experience should be defined by the limitations of an internal dependency (Tonic). We should really find a way to make all the client method not require a mutable reference to self.

[c-thiel]

@danielgerlag, I agree that the API experience should not be limited by that. From what I can see making that change is a larger effort and a breaking change.

In the meantime it would really be great if we could merge this PR to have a workaround. Also, I wouldn't say implementing clone is "just" a workaround, it can be helpful in a variety of use-cases, i.e. also deriving similar clients from one another.

[cscetbon]

@c-thiel agreed on this, this is extremely limiting at the moment. Why does it take so long to take a decision ? The PR was created in March ...