Ogeon / rustful

[OUTDATED] A light HTTP framework for Rust
https://docs.rs/rustful
Apache License 2.0
862 stars 52 forks source link

Implement Handler for Box and Arc #70

Closed Ogeon closed 8 years ago

Ogeon commented 8 years ago

There are definitely some situations where you may want to package your handlers in a Box or Arc, instead of creating your own wrapper type. We should, therefore, spare the users the effort implement Handler for those types.

Bonus quest: Investigate if some variant of impl<T> Handler for T where T: Deref, T::Target: Handler is a viable alternative or if it would conflict too much. A more generic implementation will conflict with T: Fn(...).

kvikas commented 8 years ago

@Ogeon can you take a look at the pull request. Is this in line with what you were planning? If this looks Ok I'll commit for Box<T> as well. I'm working on a generic way to do this like you suggested. But current implementation is conflicting.

I've a questions. Can you suggest a good example where Arc<T> handler feels like an obvious choice? In my example the usage seems forced.

Ogeon commented 8 years ago

Your implementation for Arc<T> looks good. I also realized that a more generic implementation would conflict with the implementation for Fn, so it won't work, but that's ok. It was just a thought I had.

Your example is close enough, with the difference that it would have to depend on the path to be more "obvious":

fn my_handler(context: Context, response: Response) {
    if let Some(var) = context.variables.get("var") {
        //do stuff with var
    } else {
        //do something else
    }
}

//...

let handler = Arc::new(my_handler);
insert_routes! {
    router => {
        Get: handler.clone(),
        ":var" => Get: handler
    }
}

This example is essentially the hello_world example, but with Arc instead of just a copy of the function pointer. A more "real" usecase would be if the handler had some non-copy data that had to be available at both / and /:var. This would require either the data or the handler itself to be wrapped in an Arc. It's hard to make examples that doesn't feel forced.

Ogeon commented 8 years ago

I'm not 100% sure if an example for this is really necessary. It's quite basic, but on the other hand, why not?

kvikas commented 8 years ago

Thanks for feedback @Ogeon. Still getting hang of Rust and quite liking it so far.

As you said, the example that I've added doesn't add anything to what we already have. It was more of a proof of concept for me to see if everything is working fine. We don't have an example that uses all the http verbs. So I was thinking I can add an example that holds a resource. Get, Post and Delete methods get, update and remove resource respectively. I'm new to Rust and as far as I can understand I need a mutable reference of handler to do it. But I don't know how to do it in current setup. Can you help me with that.

Ogeon commented 8 years ago

I see :smile: It's nice of you to consider adding new examples. Some of the current examples can be extended to show more aspects of the library and it may even be a good idea to replace some of them. The todo example covers multiple http verbs in general, and it's also an example of how mutable state can be implemented (together with handler_storage), so that may already be covered. This is, however, a bit of a side topic and this is not really the right place. We can continue the discussion in a separate issue or pull request if you would like to go through and improve the examples.

kvikas commented 8 years ago

Cool thanks. I removed example and updated PR. This should take care or Arced handlers.

Now I need help with Boxed handlers. Implementing it the same way as Arc results in compiler error:

src/handler.rs:14:1: 18:2 error: conflicting implementations for trait `handler::Handler` [E0119]
src/handler.rs:14 impl<F: Fn(Context, Response) + Send + Sync + 'static> Handler for F {
src/handler.rs:15     fn handle_request(&self, context: Context, response: Response) {
src/handler.rs:16         self(context, response);
src/handler.rs:17     }
src/handler.rs:18 }
src/handler.rs:14:1: 18:2 help: run `rustc --explain E0119` to see a detailed explanation
src/handler.rs:27:1: 31:2 note: note conflicting implementation here
src/handler.rs:27 impl<T: Handler> Handler for Box<T> {
src/handler.rs:28     fn handle_request(&self, context: Context, response: Response) {
src/handler.rs:29         (**self).handle_request(context, response);
src/handler.rs:30     }
src/handler.rs:31 }
error: aborting due to previous error
Could not compile `rustful`.

Appreciate your help on this.

Ogeon commented 8 years ago

That's really strange... Oh, wait, Box<FnOnce> implements FnOnce and so does anything that implements Fn, meaning that T: Fn may be a Box... It should at least be possible to implement Handler for Box<Handler>, I think, since Handler doesn't require Fn. That's the main usecase for boxed handlers, anyway.