Should Service require Clone?

[withoutboats]

Context: #6. The question: should Service have Clone as a supertrait?

---

@carllerche: Clone can be added as a decorator around a service IMO that provides some level of configurable buffering.

Does this mean you have an idea for a wrapper which takes any Service type and produces a Service + Clone type? If this works it at least alleviates my concern; users won't be totally out of luck if they need to clone a Service, they just need to apply this wrapper.

---

I don't see how it would be unusable? It's very usable in the case of sequential use. You can only have one in-flight request at the time.

So the model that I'm concerned about is this:

struct Endpoint {
    redis: RedisService,
    postgres: PostgresService,
}

impl Endpoint {
    fn call(&mut self) -> impl Future<Item = Response, Error = Error> {
        self.redis.call(redis_request).and_then(|redis_response| {
            self.postgres.call(postgres_request);
            ...
        }
    }
}

// or someday
impl Foo {
    async fn call(&mut self) -> Result<Response, Error> {
        let redis_response = await!(self.redis.call(redis_request))?;
        await!(self.postgres.call(postgres_request))?;
        ...
    }
}

This results in ownership errors because the future needs to own the postgres handle. If the postgres handle isn't Clone, users simply can't write this code.

If all they need to do is apply some wrapper around the handle, that's not as serious. But even then I suspect tower is going to get a similar reputation issue that tokio has had, so I'm wondering how much the performance impact of requiring every Service to build in its Clone aspect would actually be.

[carllerche]

I pushed an initial pass at a futures aware borrow cell. This would support the described use case nicely without imposing Clone or buffering.

Code: futures-borrow

And, adapting your example to use Borrow.

struct Endpoint {
    redis: RedisService,
    postgres: Borrow<PostgresService>,
}

impl Service for Endpoint {
    // Associated types...

    fn poll_ready(&mut self) -> Poll<(), Self::Error> {
        try_ready!(self.redis.poll_ready());

        // Checks if the `Borrow` is ready to be borrowed (not the actual underlying service
        self.postgres.poll_ready()
    }

    fn call(&mut self) -> impl Future<Item = Response, Error = Error> {
        let mut postgres = try!(self.postgres.try_borrow());

        self.redis.call(redis_request).and_then(move |redis_response| {
            postgres.ready()
                .and_then(move |postgres| postgres.call(postgres_request))
        }
    }

[danburkert]

That's an interesting pattern. If I understand correctly, that service implementation is only allowing a single request to be in-flight at once, right? As a thought experiment, if you'd wrapped the postgres service in an Arc<Borrow<PostgresService>>, would that allow multiple in-flight requests?

[danburkert]

Nevermind, that's not possible due to &/&mut.

[carllerche]

Is this still an unanswered question in peoples mind? This is a "no" or me, but #29 is still TBD.

[carllerche]

Clone bounds are added when needed. Many services cannot implement Clone, so it should not be part of the trait.