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

Added IHttpRouter interface #177

Merged
merged 3 commits into from Feb 7, 2013

Conversation

Projects
None yet
2 participants
@lclarkmichalek
Contributor

lclarkmichalek commented Feb 6, 2013

I've been implementing my own router class recently, and I've felt the need for an interface for Http Routers in two ways:

  • The UrlRouter public API is 90% convenience functions, with only 2 functions that actually do work (match and handleRequest). By using an interface, the convenience functions need not be explicitly implemented by any other routers, and new routers need only implement match and handleRequest.
  • Having a base type for routers is really useful for creating generic/pluggable site components. I don't know what the standard way of organising vibe.d modules is, but I generally just expose 1 function to register routes. Currently it's not really possible to do this without specifying the type of the router, which isn't really great. Having a base type solves this problem.

This pull request adds a IHttpRouter interface that implements all of the convenience functions from UrlRouter and also modifies UrlRouter slightly to implement IHttpRouter (I changed the return value of match from void to IHttpRouter).

@s-ludwig

This comment has been minimized.

Show comment
Hide comment
@s-ludwig

s-ludwig Feb 6, 2013

Member

Just out of curiosity, what does your router do different/additional?

In general I don't see any issue adding an interface, I'll just make some small inline notes that would be good to have handled before merging.

Member

s-ludwig commented Feb 6, 2013

Just out of curiosity, what does your router do different/additional?

In general I don't see any issue adding an interface, I'll just make some small inline notes that would be good to have handled before merging.

@s-ludwig

View changes

Show outdated Hide outdated source/vibe/http/router.d
@s-ludwig

View changes

Show outdated Hide outdated source/vibe/http/router.d
@s-ludwig

View changes

Show outdated Hide outdated source/vibe/http/router.d
@s-ludwig

View changes

Show outdated Hide outdated source/vibe/http/router.d
@lclarkmichalek

This comment has been minimized.

Show comment
Hide comment
@lclarkmichalek

lclarkmichalek Feb 6, 2013

Contributor

I have 2 different routers. One which uses regex based parsing with named subgroups for parameter selection a la django (this relies on a patch to phobos that has yet to be merged) and another that allows the exposing of another router on a subpath i.e.

auto siterouter = new UrlRouter();
siterouter.get("/test", &testView);

auto forwardedrouter = new ForwardedRouter();
forwardedrouter.add_router("/subsite", siterouter);
forwardedrouter.get("/test", &testView2)
// So forwardedrouter will map GET /subsite/test -> testView, GET /test -> testView2
Contributor

lclarkmichalek commented Feb 6, 2013

I have 2 different routers. One which uses regex based parsing with named subgroups for parameter selection a la django (this relies on a patch to phobos that has yet to be merged) and another that allows the exposing of another router on a subpath i.e.

auto siterouter = new UrlRouter();
siterouter.get("/test", &testView);

auto forwardedrouter = new ForwardedRouter();
forwardedrouter.add_router("/subsite", siterouter);
forwardedrouter.get("/test", &testView2)
// So forwardedrouter will map GET /subsite/test -> testView, GET /test -> testView2
@s-ludwig

This comment has been minimized.

Show comment
Hide comment
@s-ludwig

s-ludwig Feb 7, 2013

Member

Thanks, merging in...

Member

s-ludwig commented Feb 7, 2013

Thanks, merging in...

s-ludwig added a commit that referenced this pull request Feb 7, 2013

@s-ludwig s-ludwig merged commit 544f569 into vibe-d:master Feb 7, 2013

s-ludwig added a commit that referenced this pull request Feb 7, 2013

Renamed IHttpServerRequestHandler to HttpServerRequestHandler to comp…
…ly with the latest style practice.

See also pull #177.

@lclarkmichalek lclarkmichalek deleted the lclarkmichalek:IHttpRouter branch Feb 7, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment