Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
crescentrose opened this issue Jul 13, 2024 · 2 comments · Fixed by #848
Closed

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

crescentrose opened this issue Jul 13, 2024 · 2 comments · Fixed by #848
Labels
enhancement New feature or request

Comments

@crescentrose
Copy link

crescentrose commented Jul 13, 2024

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
Copy link
Contributor

thinety commented Jul 17, 2024

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

#848 fixes this.

@crescentrose
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants