Add HTTP handler registration to the HTTP server

Add HTTP handler registration to the HTTP server

NOTE: This is a breaking change.

The onRequest callback on the HTTP server have been removed in favor
of the ability to register handlers. A handler can be either a
function or an instance of a class which implements a specific
interface. Each handler registration has a regular expression which is
matched against the request path.

There is also a default handler which can be set to handle all
requests which is not matched by any of the registered patterns. If
there is no default handler explicitly set there is a builtin one
which will just return 404 status.

Currently the method is not considered in the matching.

R=ager@google.com, ajohnsen@google.com

BUG=none
TEST=tests/standalone/src/io/HttpServerHandlerTest.dart

Committed: https://code.google.com/p/dart/source/detail?r=6683

[Anders Johnsen, 2012-04-18 12:19:09]

LGTM, Very nice, I like!

https://chromiumcodereview.appspot.com/10119005/diff/3001/runtime/bin/http_im...
File runtime/bin/http_impl.dart (right):

https://chromiumcodereview.appspot.com/10119005/diff/3001/runtime/bin/http_im...
runtime/bin/http_impl.dart:799: Object _handler;
The type could be either a Function or a RequestHandler. This would move the
type checking to addRequestHandler/defaultRequestHandler.

https://chromiumcodereview.appspot.com/10119005/diff/3001/runtime/bin/http_im...
runtime/bin/http_impl.dart:865: if
(_handlers[i]._pattern.hasMatch(request.path)) {
What if we have multiple matches? Ignore? Should be documented.

[Mads Ager (google), 2012-04-18 12:22:08]

lgtm

https://chromiumcodereview.appspot.com/10119005/diff/1/runtime/bin/http.dart
File runtime/bin/http.dart (right):

https://chromiumcodereview.appspot.com/10119005/diff/1/runtime/bin/http.dart#...
runtime/bin/http.dart:56: * Interface to implement by HTTP request handler
classes.
to be implemented

https://chromiumcodereview.appspot.com/10119005/diff/1/runtime/bin/http.dart#...
runtime/bin/http.dart:90: * handler for which pattern which matches will be
handed the
for which the pattern matches will be ...

https://chromiumcodereview.appspot.com/10119005/diff/1/runtime/bin/http.dart#...
runtime/bin/http.dart:92: * [RequestHandler] interface or a function taking two
arguments.
Maybe spell out what the arguments are?

https://chromiumcodereview.appspot.com/10119005/diff/1/runtime/bin/http.dart#...
runtime/bin/http.dart:97: * Sets the request handler. This request handler will
be called if
Sets the default request handler.

https://chromiumcodereview.appspot.com/10119005/diff/1/runtime/bin/http.dart#...
runtime/bin/http.dart:99: * matches the current request. If no default request
handler is set
matches -> match

https://chromiumcodereview.appspot.com/10119005/diff/3001/runtime/bin/http.dart
File runtime/bin/http.dart (right):

https://chromiumcodereview.appspot.com/10119005/diff/3001/runtime/bin/http.da...
runtime/bin/http.dart:94: addRequestHandler(RegExp pattern, Object handler);
I'm a little worried about forcing this to be a RegExp. How about using a
function here instead of a regexp pattern:

bool match(String path)

Then for regexps you can do:

addRequestHandler(new RegExp('asdf').hasMatch, ...);

while at the same time allowing cheaper (or more flexible) matching routines.

https://chromiumcodereview.appspot.com/10119005/diff/3001/runtime/bin/http_im...
File runtime/bin/http_impl.dart (right):

https://chromiumcodereview.appspot.com/10119005/diff/3001/runtime/bin/http_im...
runtime/bin/http_impl.dart:878: 
Remove blank line?

https://chromiumcodereview.appspot.com/10119005/diff/3001/tests/standalone/sr...
File tests/standalone/src/io/HttpServerSocketTest.dart (right):

https://chromiumcodereview.appspot.com/10119005/diff/3001/tests/standalone/sr...
tests/standalone/src/io/HttpServerSocketTest.dart:182: };
Slightly funky indentation. I think I prefer to add indentation for the
end-brace here as well?

https://chromiumcodereview.appspot.com/10119005/diff/3001/tests/standalone/sr...
File tests/standalone/src/io/HttpServerTest.dart (right):

https://chromiumcodereview.appspot.com/10119005/diff/3001/tests/standalone/sr...
tests/standalone/src/io/HttpServerTest.dart:26: };
ditto

https://chromiumcodereview.appspot.com/10119005/diff/3001/tests/standalone/sr...
File tests/standalone/src/io/HttpTest.dart (left):

https://chromiumcodereview.appspot.com/10119005/diff/3001/tests/standalone/sr...
tests/standalone/src/io/HttpTest.dart:518: //testStartStop();
Whoops. Good catch.

[Søren Gjesse, 2012-04-18 13:26:56]

https://chromiumcodereview.appspot.com/10119005/diff/3001/runtime/bin/http.dart
File runtime/bin/http.dart (right):

https://chromiumcodereview.appspot.com/10119005/diff/3001/runtime/bin/http.da...
runtime/bin/http.dart:94: addRequestHandler(RegExp pattern, Object handler);
On 2012/04/18 12:22:08, Mads Ager wrote:
> I'm a little worried about forcing this to be a RegExp. How about using a
> function here instead of a regexp pattern:
> 
> bool match(String path)
> 
> Then for regexps you can do:
> 
> addRequestHandler(new RegExp('asdf').hasMatch, ...);
> 
> while at the same time allowing cheaper (or more flexible) matching routines.

Good point changed RegExp to a function that gets the request. Then the matching
can match on all properties of the request including path and method.

bool matcher(HttpRequest request)

https://chromiumcodereview.appspot.com/10119005/diff/3001/runtime/bin/http_im...
File runtime/bin/http_impl.dart (right):

https://chromiumcodereview.appspot.com/10119005/diff/3001/runtime/bin/http_im...
runtime/bin/http_impl.dart:799: Object _handler;
On 2012/04/18 12:19:09, ajohnsen wrote:
> The type could be either a Function or a RequestHandler. This would move the
> type checking to addRequestHandler/defaultRequestHandler.

Good point. Used RequestHandler and added the class _RequestHandlerImpl for
wrapping functions.

https://chromiumcodereview.appspot.com/10119005/diff/3001/runtime/bin/http_im...
runtime/bin/http_impl.dart:865: if
(_handlers[i]._pattern.hasMatch(request.path)) {
On 2012/04/18 12:19:09, ajohnsen wrote:
> What if we have multiple matches? Ignore? Should be documented.

The documentation in http.dart says that the first matching handler will handle
the request.

https://chromiumcodereview.appspot.com/10119005/diff/3001/runtime/bin/http_im...
runtime/bin/http_impl.dart:878: 
On 2012/04/18 12:22:08, Mads Ager wrote:
> Remove blank line?

Done.

https://chromiumcodereview.appspot.com/10119005/diff/3001/tests/standalone/sr...
File tests/standalone/src/io/HttpServerSocketTest.dart (right):

https://chromiumcodereview.appspot.com/10119005/diff/3001/tests/standalone/sr...
tests/standalone/src/io/HttpServerSocketTest.dart:182: };
On 2012/04/18 12:22:08, Mads Ager wrote:
> Slightly funky indentation. I think I prefer to add indentation for the
> end-brace here as well?

Absolutely, done.

https://chromiumcodereview.appspot.com/10119005/diff/3001/tests/standalone/sr...
File tests/standalone/src/io/HttpServerTest.dart (right):

https://chromiumcodereview.appspot.com/10119005/diff/3001/tests/standalone/sr...
tests/standalone/src/io/HttpServerTest.dart:26: };
On 2012/04/18 12:22:08, Mads Ager wrote:
> ditto

Done.