Generalize Connected implementation to all Listeners for SocketAddr

[Voultapher]

The previous implementation implemented the trait Connected for TcpListener and TapIo. This was both needlessly specific and blocked other implementers of the Listener trait from themselves implementing Connected.

This is a revive of #3314 that avoids blocking existing implementations. AFAICT it's 2x2 matrix for (tcplistener, custom listener) x (SocketAddr, custom addr) as shown in this example:

use std::net::SocketAddr;

use tokio::net::{TcpListener, TcpStream};

use axum::extract::connect_info::Connected;
use axum::serve::{IncomingStream, Listener};
use axum::Router;

fn create_router() -> Router {
    todo!()
}

fn tcp_listener() -> TcpListener {
    todo!()
}

#[derive(Clone)]
#[allow(dead_code)]
struct CustomAddr(SocketAddr);

impl Connected<IncomingStream<'_, TcpListener>> for CustomAddr {
    fn connect_info(_stream: IncomingStream<'_, TcpListener>) -> Self {
        todo!()
    }
}

impl Connected<IncomingStream<'_, CustomListener>> for CustomAddr {
    fn connect_info(_stream: IncomingStream<'_, CustomListener>) -> Self {
        todo!()
    }
}

struct CustomListener {}

impl Listener for CustomListener {
    type Io = TcpStream;
    type Addr = SocketAddr;

    async fn accept(&mut self) -> (Self::Io, Self::Addr) {
        todo!()
    }

    fn local_addr(&self) -> tokio::io::Result<Self::Addr> {
        todo!()
    }
}

fn custom_connected() {
    let router = create_router();
    let _ = axum::serve(
        tcp_listener(),
        router.into_make_service_with_connect_info::<CustomAddr>(),
    );
}

fn custom_listener() {
    let router = create_router();
    let _ = axum::serve(CustomListener {}, router.into_make_service());
}

fn custom_listener_with_connect() {
    let router = create_router();
    let _ = axum::serve(
        CustomListener {},
        router.into_make_service_with_connect_info::<SocketAddr>(),
    );
}

fn custom_listener_with_custom_connect() {
    let router = create_router();
    let _ = axum::serve(
        CustomListener {},
        router.into_make_service_with_connect_info::<CustomAddr>(),
    );
}

fn main() {
    custom_connected();
    custom_listener();
    custom_listener_with_connect();
    custom_listener_with_custom_connect();
}

The annoying aspect with types like SocketAddr in this situation is that it's not user controlled so they can't implement traits for it. In contrast user controlled custom address types don't have that limitation. However there is still the situation of third party address types that aren't aware of Connected those will have to be wrapped I fear. One option would be to provide a trait that third party address types can implement to opt into the default connected impl, but I don't think that's a good idea since it creates more traits to keep track of and third party address types are likely not aware of Connected so they won't be aware of such a trait either with high likelihood.

[Voultapher]

@jplatte the CI passes, so please take another look.

[Turbo87]

I'm not too familiar with the code, but the proposal sounds reasonable and if it compiles then I guess it's fine 😅

[mladedav]

This maybe got forgotten a bit, I think we can merge this.

Should we write a changeling for this or not since this should allow strictly more things?

[jplatte]

Well, this removes the ability to use tap_io with listeners that have an Addr type other than SocketAddr. I'm still not really sure about this.

Sorry about not responding for so long though!

[mladedav]

If people implement it the way this PR does for SocketAddr, they can do that for all their types for all Listeners as far as I can see. The only problem I can see would be if they wanted to have non-local type as Listener::Addr (e.g. IpAddr) but they can create a newtype to work around that.

Specifically the following example works so I might be misunderstanding?

Example

use std::future::{ready, Future};

use axum::{
    extract::connect_info::Connected,
    routing::get,
    serve::{IncomingStream, Listener, ListenerExt},
    Router,
};
use tokio::net::TcpStream;

#[tokio::main]
async fn main() {
    let app = Router::new()
        .route("/", get(()))
        .into_make_service_with_connect_info::<Address>();
    let listener = Listen.tap_io(|_| ());
    axum::serve(listener, app).await.unwrap();
}

struct Listen;
#[derive(Debug, Clone)]
struct Address;

impl Listener for Listen {
    type Io = TcpStream;

    type Addr = Address;

    fn accept(&mut self) -> impl Future<Output = (Self::Io, Self::Addr)> + Send {
        ready(todo!())
    }

    fn local_addr(&self) -> tokio::io::Result<Self::Addr> {
        todo!()
    }
}

impl<L> Connected<IncomingStream<'_, L>> for Address
where
    L: Listener<Addr = Address>,
{
    fn connect_info(stream: IncomingStream<'_, L>) -> Self {
        todo!()
    }
}

[jplatte]

Can we have such an impl as a test / in an example (the one that previously stopped compiling, maybe)? If it works, I guess this is fine.

[Voultapher]

In the details part of the PR description there are examples. I'm not aware of a scenario that previously compiled but no longer does. If the addr type implements Connected it should be compatible with tap_io no regression here.

[mladedav]

I think adding what you have as the examples here (without the main) as tests to the file would be ok. If it compiles, it works, I don't think we need anything sophisticated here. Just to make sure this works going forward.

[Voultapher]

I've added the test, not sure if the CI failure is related to this PR.

[jplatte]

This is related to the PR:

error: unnecessary structure name repetition
  --> axum/src/extract/connect_info.rs:91:35
   |
91 |         L: serve::Listener<Addr = SocketAddr>,
   |                                   ^^^^^^^^^^ help: use the applicable keyword: `Self`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#use_self
   = note: `-D clippy::use-self` implied by `-D warnings`
   = help: to override `-D warnings` add `#[allow(clippy::use_self)]`

The rest isn't and will be solved separately.

[mladedav]

Last small things, after that the tests should pass.