ReactiveX / RxNetty

Reactive Extension (Rx) Adaptor for Netty
Apache License 2.0
1.38k stars 255 forks source link

HTTP routing (and filtering) #286

Open pk11 opened 9 years ago

pk11 commented 9 years ago

Hi,

I am not sure whether you guys are interested in having an HTTP router/filter in rxnetty-core or that should stay in userland. In any case, I created a library that attempts to address this issue:

https://github.com/pk11/rxnetty-router

Here is an HttpServer using the router:

HttpServer<ByteBuf, ByteBuf> server = RxNetty.createHttpServer(0, using(
                    new Router<ByteBuf, ByteBuf>() 
                    .GET("/hello", new HelloHandler())
                    .GET("/article/:id", withParams( (params, request, response)->{
                        response.setStatus(HttpResponseStatus.OK);
                        response.writeString("params:"+ params.get("id"));
                        return response.close();
                    }))
                    .GET("/public/:*", new ClassPathFileRequestHandler("www"))
                    .notFound(new Handler404())
                    )).start();

As the next step, I will implement filter support.

Potential issues for rxnetty-core:

Please let me know your thoughts.

NiteshKant commented 9 years ago

Thanks @pk11 for sharing your work with us!

We certainly need an application level router like you have created as well as the filter support you intend to add. Currently, karyon provides an interceptor (filter) support which I am intending to fold into RxNetty to be used by both server and client. I am happy to collaborate on that feature too.

I looked at your implementation (jauter in general) and a few comments/questions:

Overall I like the usage and it is similar to what I was thinking of implementing (discussed in this PR #254)

If you can elaborate what you are thinking about implementing a filter, we can discuss it here.

About the usage of jauter, I think we will have to implement it by ourselves as rxnetty-core should not have any dependency apart from netty & rxjava!

pk11 commented 9 years ago

It looks like the starting point for a route definition is always starting with a method, so If I have to route for all methods on a URI, I would have to specify a route for every HTTP method.

That's correct, if that's a requirement then that alone justifies the rewrite

Is the only need for the existence of Route is the need for passing parsed parameters? If yes, we may be able to add that to HttpServerRequest

Yes, that's correct. If you could add an extra field to HttpServerRequest or RequestHandler to store matched URL params then that interface + the java8 requirement would go away.

For filter support, I was thinking about a similar concept to webbit

ngocdaothanh commented 9 years ago

If I have to route for all methods on a URI, I would have to specify a route for every HTTP method.

Author of Jauter here.

The README of Jauter shows that you can use method ANY, like this:

router.ANY("/form_or_create", MyFormOrCreate.class)

rxnetty-core should not have any dependency apart from netty & rxjava!

I'd like to contribute. How about moving Jauter somewhere more convenient for RxNetty?

NiteshKant commented 9 years ago

@ngocdaothanh thanks for pitching in!

The README of Jauter shows that you can use method ANY, like this:

Cool, thanks for the pointer.

How about moving Jauter somewhere more convenient for RxNetty?

We have a very strict objective of not having any other dependency apart from netty and rxjava. So, the only way it could work is if the code is inside rxnetty-core or a separate module. Since, routing is a very core functionality, I do not think having a separate module makes sense. I am totally fine if you want to contribute the router implementation to rxnetty.

I haven't yet read all about Jauter but this is what I am expecting from the router and interceptors:

I would think of it as something like:

router.forUri("/")
         .withMethod(GET)
         .withQueryParam("foo").value("bar")
         .route(requestHandler);

router.forMethod(GET)
         .withUri("/")
         .withQueryParam("foo").value("bar")
         .route(requestHandler);

router.forUri("/")
         .withMethod(GET)
         .withQueryParam("foo").value("bar")
         .intercept(interceptor);
pk11 commented 9 years ago

I'd like to contribute.

@ngocdaothanh awesome news! My project was a proof of concept anyway, so it would be great if you could take over.

@NiteshKant Great write up - I like where this is going! My 2c:

I suppose it's a matter of taste how rich you want to make your DSL but I found the proposed syntax a bit too ambiguous and fluid (i.e. withMethod vs forMethod).

Personally as a user I'd prefer something closer to the current jauter syntax, that is:

router#<verb> ("<url regex (including query params)>", withFilter( (params,req,res)->{...} )

or

router#<verb> ("<url regex (including query params)>", addFilter((params,req,res)->{...}))

and the non-filter option could be simply:

router#<verb> ("<url regex>", (params,req,res)->{...})

or

router#<verb> ("<url regex>", with(params,req,res)->{...})

(and no other option.)

pk11 commented 9 years ago

...or writing these as fluid APIs:

router#<verb> ("<url regex (including query params)>").withFilter( (params,req,res)->{...} )

and

router#<verb> ("<url regex (including query params)>").with( (params,req,res)->{...} )

NiteshKant commented 9 years ago

@pk11 Good points.

The issue with providing regex as the only way to route with specifying QP is that means the regex would have to be evaluated for every URI which is costly. That is the reason why I extracted the QP based routing outside the regex (you can ofcourse still do regex matching for QPs).

I personally like the .route() and .intercept() method names in the DSL as opposed to withFilter() and with() as they are explicit in what they are doing, specifically while using lambdas.

router#<verb> makes sense if we can provide multiple verbs, eg:

router.PUT("/create").orPOST().route( (req, res) -> {...} )
elandau commented 9 years ago

As RxNetty gains more adoption I imagine people will want different DSLs. As such we should make sure to provide a robust low level route wiring mechanism that can be configured via any DSL. Let's take a step back and look at some requirements for low level route wiring mechanism,

For example, we can create a RouteBuilder likes this,

RouteBuilder builder = Routes.newRouteBuild()
   .withIoCContainer(new GuiceContainer(injector))
;

And configure routes likes this,

builder
   .matches(SimpleMatcher.get("/foo/:id"))
   .produces("application/json")
   .serveWith(FooHandler.class)); 

Any DSL ends up being syntactic sugar on top of this mechanism.

pk11 commented 9 years ago

@NiteshKant re: query params: I guess jauter's approach of not matching on query params was coming from a similar consideration. Personally, I tend to prefer smaller APIs with limited functionality, so I would be fine with not being able to match on query params or headers at all (some of those issues can be addressed via filters anyway).

router.PUT("/create").orPOST().route( (req, res) -> {...} ) this looks great!

As RxNetty gains more adoption I imagine people will want different DSLs.

@elandau I suppose it's a question where you'd want to draw the line. For me the low level framework here is netty. So having a very generic API sounds like optimizing for the advanced use case but ultimately it's your call. Having this functionality in core (in any shape or form) would be great for the users.

ngocdaothanh commented 9 years ago

We have a very strict objective of not having any other dependency apart from netty and rxjava. So, the only way it could work is if the code is inside rxnetty-core or a separate module.

The Netty project is also adding the router feature: https://github.com/netty/netty/issues/3023

So maybe to avoid work duplication, RxNetty should wait a little more for the next version of Netty, then use the router feature in it. Please also discuss about the router at the above issue.

gorzell commented 9 years ago

Another great feature in this area is the ability to nest routers/filters by basically having them comply with the Handler API. This allows you to create a top level router than can carve up your high level API URI space, but then delegate the specifics of how to handle routing and filtering (including handling request methods) within that space to a child class.

I think that Finagle is a pretty good example of an API that lets you compose pipelines in this way (although it does leverage a lot of scala language features to do so).

NiteshKant commented 9 years ago

337 is where we are discussing interceptors (filters)