Difficulty to implement a wrapper of ManageConnection due to is_valid signature

[arnodb]

Hello,

Problem

Recently, with r2d2, I implemented a wrapper of a ManageConnection which delegates everything to its inner connection manager and adds a bit of instrumentation. The pattern looks like this:

pub struct WrappedConnection<M: ManageConnection> {
    conn: M::Connection,
    ...
}

pub struct WrappedConnectionManager<M: ManageConnection> {
    manager: M,
    ...
}

impl<M: ManageConnection> ManageConnection for WrappedConnectionManager<M> {
    type Connection = WrappedConnection<M>;
    type Error = M::Error;

    fn connect(&self) -> Result<Self::Connection, Self::Error> {
        let conn = self.manager.connect()?;
        Ok(WrappedConnection {
            conn,
            ...
        })
    }

    fn is_valid(&self, conn: &mut Self::Connection) -> Result<(), Self::Error> {
        let result = self.manager.is_valid(&mut conn.conn);
        ...
        result
    }

    fn has_broken(&self, conn: &mut Self::Connection) -> bool {
        self.manager.has_broken(&mut conn.conn)
    }
}

I am trying to switch to another pool crate like bb8 for multiple reasons, async is one of them. But it appears impossible to implement such a wrapper with bb8 because is_valid takes a &mut PoolConnection<'_, Self> instead of &mut Self::Connection.

I understand the why of this difference with r2d2 (commit 0125284, superseded by some others), but if a few things are modified, the implementation of the wrapper would be possible.

Proposed solution

The change in the signature of ManageConnection::is_valid can be reverted as it appears to be, in the various implementations I saw, not necessary: the pooled connection can just be dereferenced when calling is_valid.

I tried to imagine other solutions, e.g. using temporary PooledConnection instances, be that one suffers from severe flaws: it has no pool to be attached to, and PooledConnection::drop_invalid does not return the value it takes.

What do you think?

[djc]

Your proposed solution seems reasonable, please submit a PR.

[arnodb]

Please see #116.