Rethink the Transport boxing

[algesten]

Quoting @sosthene-nitrokey in #870:

There would be benefits to not boxing the connectors

In the connector traits, the transports are boxed, preventing chained connectors to make use of specificities of the previous transport. Ideally for example, it would be possible to use the specific type of the previous transport. For example a TCP keepalive connector could just wrap or come after a TcpTransport, get its TCP stream, and enable Keepalive on it.

The way I would do it would use associated types for the transports, and make chained and not chained connectors part of the trait.

pub trait Connector: Debug + Send + Sync + 'static {
    type Transport: Transport;

    fn boxed(self) -> AnyConnector
    where
        Self: Sized,
    {
        Box::new(self)
    }
    fn connect(&self, details: &ConnectionDetails) -> Result<Self::Transport, Error>;
}

pub trait AnyConnectorTrait: Debug + Send + Sync + 'static {
    fn connect(&self, details: &ConnectionDetails) -> Result<Box<dyn Transport>, Error>;
}

impl<C: Connector> AnyConnectorTrait for C {
    fn connect(&self, details: &ConnectionDetails) -> Result<Box<dyn Transport>, Error> {
        Ok(Box::new(self.connect(details)?))
    }
}

pub type AnyConnector = Box<dyn AnyConnectorTrait>;

impl Connector for Box<dyn AnyConnectorTrait> {
    type Transport = Box<dyn Transport>;
    fn connect(&self, details: &ConnectionDetails) -> Result<Self::Transport, Error> {
        (**self).connect(details)
    }
}

pub type AnyTransport = Box<dyn Transport>;

pub trait MiddleConnector<T: Transport>: Debug + Send + Sync + 'static {
    type Transport: Transport;

    fn connect(&self, details: &ConnectionDetails, chained: T) -> Result<Self::Transport, Error>;
}

With an implementation on tuples of many sizes through a macro:

impl<C, M1, M2> Connector for (C, M1, M2)
where
    C: Connector,
    M1: MiddleConnector<C::Transport>,
    M2: MiddleConnector<M1::Transport>,
{
    type Transport = M2::Transport;

    fn connect(&self, details: &ConnectionDetails) -> Result<Self::Transport, Error> {
        self.2
            .connect(details, self.1.connect(details, self.0.connect(details)?)?)
    }
}

You could still keep the ChainedConnector for one Connector + a chain of MiddleConnector<AnyTransport> that could also be boxed.

For connectors that may or may not "chain" like the SocksConnector followed by the TcpConnector, I would instead implement the SocksConnector as a:

struct SocksConnector<C: Connector<Transport = TcpStream>> {
    inner: C,
}

impl<C: Connector<Transport = TcpStream>> Connector for SocksConnector<C> { 
    type Transport = TcpStream;
    fn connect(&self, details: &ConnectionDetails) -> Result<Self::Transport, Error> {
        if !details.use_socks {
            return self.inner.connect(details);
        }
        // Current socks connector logic
    }
}

This type safety would allow for example a keepalive connector, which is much simpler that re-implementing all the functionality of the TcpConnector just to add that. It makes connectors much more composable.

struct KeepaliveConnector;

impl MiddleConnector<TcpTransport> for KeepaliveConnector {
    type Transport = TcpTransport;
    fn connect(
        &self,
        details: &ConnectionDetails,
        chained: TcpTransport,
    ) -> Result<Self::Transport, Error> {
        let stream: &std::net::TcpStream = chained.tcp_stream();
        // Configure Keepalive on the TcpStream;
        Ok(chained)
    }
}

[sosthene-nitrokey]

Thinking a bit about the unversionned module.

Maybe it should just be made #[doc(hidden)], and have another crate that depends on a specific version of ureq (with =3.1.0 for example), and re-exports the items of this module. That way breaking changes in ureq are possible, and this ureq-transports?  crate can have more frequent semver bumps.

[algesten]

That's an interesting thought!

Need to process that tomorrow when I'm awake :D