Ergonomics issue with 0.16

[ufoscout]

Hi,

while upgrading a project from rust_postgres 0.15 to 0.17 and r2d2_postgres 0.14 to 0.16, we have some troubles with the new definition of PostgresConnectionManager<T> that contains a new generic parameter <T>. The problem is that the r2d2 pool returns a PooledConnection that now has the very same generic parameter <T> that, as a consequence, has to be redeclared in every place where the connection is used.

For example, our project structure follows the Domain Driven Design principle, then:

• we have a set of Repositories that use a PooledConnection, so they all have to declare the <T> generic param
• we have a set of Services that use the Repositories, so all Services need to declare <T> too
• we have a set of Controllers that use the Services, so all Controllers need to declare <T>
• and so on...

In the end, this <T> generic is declared literally everywhere.

In addition, the rust_postgres Client has no generic parameters at all so it is not clear why the PooledConnection requires it.

I don't know if it is feasible, but the issue could be fixed by changing the signature in the r2d2 pool from:

    pub fn get(&self) -> Result<PooledConnection<M>, Error>;

to

    pub fn get(&self) -> Result<PooledConnection<M::Connection>, Error>;

Or, maybe, the <T> param can be boxed in the PostgresConnectionManager struct:

#[derive(Debug)]
pub struct PostgresConnectionManager {
    config: Config,
    tls_connector: Box<MakeTlsConnect<Socket, TlsConnect=Send, Stream=Send, ... >>,
}

[sfackler]

Do the Repositories explicitly need a PooledConnection or can they just work with a &mut Client?

Changing the type of PooledConnection would need to happen on r2d2, not r2d2-postgres. You could also make a type alias for the pooled connection type to avoid having to spell the full type out everywhere.

[ufoscout]

@sfackler It's a good proposal and, indeed, it solves the issue with the repositories. Nevertheless, we still have the issue with the services that own a copy of the r2d2 pool.

As a temporary workaround, we created our own implementation of the ManageConnection that needs no generic parameters:

use r2d2::ManageConnection;
use postgres::{Config, Socket, Client};
use postgres::tls::MakeTlsConnect;
use tokio_postgres::Error;

pub struct PostgresConnectionManager {
    config: Config,
    tls_connector: Box<dyn Fn(&Config) -> Result<Client, Error> + Send + Sync>,
}

impl PostgresConnectionManager {
    pub fn new(config: Config, tls_connector: Box<dyn Fn(&Config) -> Result<Client, Error> + Send + Sync>) -> PostgresConnectionManager {
        PostgresConnectionManager {
            config,
            tls_connector,
        }
    }
}

impl ManageConnection for PostgresConnectionManager {
    type Connection = Client;
    type Error = Error;

    fn connect(&self) -> Result<Client, Error> {
        (self.tls_connector)(&self.config)
    }

    fn is_valid(&self, client: &mut Client) -> Result<(), Error> {
        client.simple_query("").map(|_| ())
    }

    fn has_broken(&self, client: &mut Client) -> bool {
        client.is_closed()
    }
}

#[cfg(test)]
mod test {

    use super::*;
    use tokio_postgres::NoTls;

    #[test]
    fn new_connection() {

        let tls = NoTls;
        let manager = PostgresConnectionManager::new(
            "host=localhost user=postgres".parse().unwrap(),
            Box::new(move |config| config.connect(tls.clone())),
        );

        let _pool = r2d2::Pool::new(manager).unwrap();
    }

}