pkg/http_server: return future for VirtualDirectory serveRequest

pkg/http_server/lib/src/virtual_directory.dart

 // Copyright (c) 2013, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 part of http_server;
 
+typedef dynamic _DirCallback(Directory dir, HttpRequest request);
+typedef dynamic _ErrorCallback(HttpRequest request);
+
 /**
  * A [VirtualDirectory] can serve files and directory-listing from a root path,
  * to [HttpRequest]s.
  *
  * The [VirtualDirectory] providing secure handling of request uris and
  * file-system links, correct mime-types and custom error pages.
  */
 class VirtualDirectory {
   final String root;
 
@@ skipping 10 matching lines @@
   bool followLinks = true;
 
   /**
    * Set or get if the [VirtualDirectory] should jail the root. When the root is
    * not jailed, links can be followed to outside the [root] directory.
    */
   bool jailRoot = true;
 
   final RegExp _invalidPathRegExp = new RegExp("[\\\/\x00]");
 
-  Function _errorCallback;
-  Function _dirCallback;
+  _ErrorCallback _errorCallback;
+  _DirCallback _dirCallback;
 
   /*
    * Create a new [VirtualDirectory] for serving static file content of
    * the path [root].
    *
    * The [root] is not required to exist. If the [root] doesn't exist at time of
    * a request, a 404 is generated.
    */
   VirtualDirectory(this.root);
 
   /**
    * Serve a [Stream] of [HttpRequest]s, in this [VirtualDirectory].
    */
-  void serve(Stream<HttpRequest> requests) {
-    requests.listen(serveRequest);
-  }
+  StreamSubscription<HttpRequest> serve(Stream<HttpRequest> requests) =>
+      requests.listen(serveRequest);
 
   /**
    * Serve a single [HttpRequest], in this [VirtualDirectory].
    */
-  void serveRequest(HttpRequest request) {
-    _locateResource('.', request.uri.pathSegments.iterator..moveNext())
-        .then((entity) {
-          if (entity == null) {
-            _serveErrorPage(HttpStatus.NOT_FOUND, request);
-            return;
-          }
+  Future serveRequest(HttpRequest request) {
+    return _locateResource('.', request.uri.pathSegments.iterator..moveNext())
+        .then((FileSystemEntity entity) {
           if (entity is File) {
             serveFile(entity, request);
           } else if (entity is Directory) {
             _serveDirectory(entity, request);
           } else {
             _serveErrorPage(HttpStatus.NOT_FOUND, request);
           }
+          return request.response.done;
         });
   }
 
   /**
    * Set the [callback] to override the default directory listing. The
    * [callback] will be called with the [Directory] to be listed and the
    * [HttpRequest].
    */
   void set directoryHandler(void callback(Directory dir, HttpRequest request)) {
     _dirCallback = callback;
@@ skipping 68 matching lines @@
    * This is usefull when e.g. overriding [directoryHandler] to redirect to
    * some index file.
    *
    * In the request contains the [HttpStatus.IF_MODIFIED_SINCE] header,
    * [serveFile] will send a [HttpStatus.NOT_MODIFIED] response if the file
    * was not changed.
    *
    * Note that if it was unabled to read from [file], the [request]s response
    * is closed with error-code [HttpStatus.NOT_FOUND].
    */
   void serveFile(File file, HttpRequest request) {

[sethladd, 2014/01/09 05:27:48]

why not return the future from here?

[Anders Johnsen, 2014/01/09 06:12:23]

response.done is the right way of doing it. Also, it'll make it easier for the
user to override dir-listing, etc., and make the future return value to work.

     var response = request.response;
     // TODO(ajohnsen): Set up Zone support for these errors.
     file.lastModified().then((lastModified) {
       if (request.headers.ifModifiedSince != null &&
           !lastModified.isAfter(request.headers.ifModifiedSince)) {
         response.statusCode = HttpStatus.NOT_MODIFIED;
         response.close();
-        return;
+        return null;
       }
 
       response.headers.set(HttpHeaders.LAST_MODIFIED, lastModified);
       response.headers.set(HttpHeaders.ACCEPT_RANGES, "bytes");
 
       if (request.method == 'HEAD') {
         response.close();
-        return;
+        return null;
       }
 
       return file.length().then((length) {
         String range = request.headers.value("range");
         if (range != null) {
           // We only support one range, where the standard support several.
           Match matches = new RegExp(r"^bytes=(\d*)\-(\d*)$").firstMatch(range);
           // If the range header have the right format, handle it.
           if (matches != null) {
             // Serve sub-range.
@@ skipping 88 matching lines @@
     <td>$modified</td>
     <td style="text-align: right">$size</td>
   </tr>''';
         response.write(entry);
       }
 
       if (path != '/') {
         add('../', null, null);
       }
 
-      dir.list(followLinks: true).listen((entity) {
-        // TODO(ajohnsen): Consider async dir listing.
+      dir.list(followLinks: true).forEach((entity) {

[Anders Johnsen, 2014/01/09 06:12:23]

This is not handling errors (they can happen if FS is modified while listing).

[kevmoo, 2014/01/10 18:12:31]

I think I've addressed this, with TODOs for logging errors

         if (entity is File) {
           var stat = entity.statSync();
           add(basename(entity.path),
               stat.modified.toString(),
               stat.size);
         } else if (entity is Directory) {
           add(basename(entity.path) + '/',
               entity.statSync().modified.toString(),
               null);
         }
-      }, onError: (e) {
-      }, onDone: () {
+      })
+      .then((_) {
         response.write(footer);
-        response.close();
+      })
+      .whenComplete(() {

[sethladd, 2014/01/09 05:27:48]

can this be a one-line function?

[kevmoo, 2014/01/10 05:02:00]

Done.

+        return response.close();

[Anders Johnsen, 2014/01/09 06:12:23]

Why return here? This value is not kept.

[kevmoo, 2014/01/10 05:02:00]

Done.

       });
-    }, onError: (e) => response.close());
+    });
   }
 
   void _serveErrorPage(int error, HttpRequest request) {
     var response = request.response;
     response.statusCode = error;
     if (_errorCallback != null) {
       _errorCallback(request);
       return;
     }
     // Default error page.
@@ skipping 67 matching lines @@
 
   Future close() => new Future.value();
 
   void setMimeType(var bytes) {
     var mimeType = lookupMimeType(path, headerBytes: bytes);
     if (mimeType != null) {
       response.headers.contentType = ContentType.parse(mimeType);
     }
   }
 }