poem-web / poem

A full-featured and easy-to-use web framework with the Rust programming language.
Apache License 2.0
3.44k stars 281 forks source link

Drop `#[derive(Default)]` from handler macro #845

Closed crescentrose closed 1 month ago

crescentrose commented 1 month ago

Description of the feature

The #[handler] macro currently adds a Debug derive to every endpoint that it generates. This causes some issues when working with handlers that take generic arguments.

In some situations, e.g. services with shared database connections, a Default implementation is not appropriate or even possible. Based on some cursory testing, it seems like the default() method doesn't even get called. But it is very much possible that I missed something, so please let me know if a Default implementation is necessary.

To avoid a breaking change, perhaps the Default derive could only be omitted with an optional parameter to the handler macro?

Code example (if possible)

use poem::{get, handler, middleware::AddData, test::TestClient, web::Data, EndpointExt, Route};

trait MyTrait: Clone + Send + Sync + 'static {
    fn do_something(&self) -> String;
}

#[derive(Clone)]
struct MyService;

impl MyTrait for MyService {
    fn do_something(&self) -> String {
        String::from("hello!")
    }
}

impl Default for MyService { // 👈 I want to get rid of this
    fn default() -> Self {   //    since it does not seem to get called
        unreachable!()       //    and having this `unreachable!` here
    }                        //    makes me slightly uncomfortable.
}

#[handler]
async fn hello<T: MyTrait>(Data(service): Data<&T>) -> String {
    service.do_something()
}

#[tokio::main]
async fn main() {
    let tc = TestClient::new(
        Route::new()
            .at("/", get(hello::<MyService>::default()))
            .with(AddData::new(MyService)),
    );

    tc.get("/").send().await.assert_text("hello!").await;
}
thinety commented 1 month ago

The Default implementation is needed to create the handler (hello::<MyService>::default()), but using #[derive(Default)]` is not ideal.

https://github.com/poem-web/poem/pull/848 fixes this.

crescentrose commented 1 month ago

@thinety this is great, thank you for picking this up!