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;
 
 
 // Used for signal a directory redirecting, where a tailing slash is missing.
 class _DirectoryRedirect {
   const _DirectoryRedirect();
 }
 
+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())
+  Future serveRequest(HttpRequest request) {
+    return _locateResource('.', request.uri.pathSegments.iterator..moveNext())
         .then((entity) {
-          if (entity == null) {
-            _serveErrorPage(HttpStatus.NOT_FOUND, request);
-            return;
-          }
           if (entity is File) {
             serveFile(entity, request);
           } else if (entity is Directory) {
             if (allowDirectoryListing) {
               _serveDirectory(entity, request);
             } else {
               _serveErrorPage(HttpStatus.NOT_FOUND, request);
             }
           } else if (entity is _DirectoryRedirect) {
             // TODO(ajohnsen): Use HttpRequest.requestedUri once 1.2 is out.
             request.response.redirect(Uri.parse('${request.uri}/'),
                                       status: HttpStatus.MOVED_PERMANENTLY);
           } else {
+            assert(entity == null);
             _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 75 matching lines @@
    * is closed with error-code [HttpStatus.NOT_FOUND].
    */
   void serveFile(File file, HttpRequest request) {
     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 13 matching lines @@
             response.headers.set(HttpHeaders.CONTENT_LENGTH, end - start);
 
             // Set 'Partial Content' status code.
             response.statusCode = HttpStatus.PARTIAL_CONTENT;
             response.headers.set(HttpHeaders.CONTENT_RANGE,
                                  "bytes $start-${end - 1}/$length");
 
             // Pipe the 'range' of the file.
             file.openRead(start, end)
                 .pipe(new _VirtualDirectoryFileStream(response, file.path))
-                .catchError((_) {});
+                .catchError((_) {
+                  // TODO(kevmoo): log errors
+                });
             return;
           }
         }
 
         file.openRead()
             .pipe(new _VirtualDirectoryFileStream(response, file.path))
-            .catchError((_) {});
+            .catchError((_) {
+              // TODO(kevmoo): log errors
+            });
       });
     }).catchError((_) {
       response.statusCode = HttpStatus.NOT_FOUND;
       response.close();
     });
   }
 
   void _serveDirectory(Directory dir, HttpRequest request) {
     if (_dirCallback != null) {
       _dirCallback(dir, request);
       return;
     }
     var response = request.response;
     dir.stat().then((stats) {
       if (request.headers.ifModifiedSince != null &&
           !stats.modified.isAfter(request.headers.ifModifiedSince)) {
         response.statusCode = HttpStatus.NOT_MODIFIED;
-        response.close();
-        return;
+        return null;
       }
 
       response.headers.set(HttpHeaders.LAST_MODIFIED, stats.modified);
       var path = request.uri.path;
       var header =
 '''<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN"
 http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
 <html xmlns="http://www.w3.org/1999/xhtml">
 <head>
 <title>Index of $path</title>
@@ skipping 28 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.
-        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: () {
-        response.write(footer);
-        response.close();
-      });
-    }, onError: (e) => response.close());
+      return dir.list(followLinks: true)
+        .handleError((error) {

[Anders Johnsen, 2014/01/10 18:19:45]

Why is this better than the old version. Its longer and harder to  follow. Also,
please remove the old todo.

[kevmoo, 2014/01/10 18:51:24]

Went back to old impl...it was just personal preference.

Can the comment still be removed?

[Anders Johnsen, 2014/01/10 18:53:35]

Yep :)

+          // TODO(kevmoo): log errors;
+        })
+        .forEach((entity) {
+          // TODO(ajohnsen): Consider async dir listing.
+          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);
+          }
+        })
+        .then((_) {
+          response.write(footer);
+        });
+    })
+    .catchError((error) {
+      // TODO(kevmoo): log errors
+    })
+    .whenComplete(() {
+      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 15 matching lines @@
 </body>
 </html>''';
     response.write(page);
     response.close();
   }
 }
 
 class _VirtualDirectoryFileStream extends StreamConsumer<List<int>> {
   final HttpResponse response;
   final String path;
-  var buffer = [];
+  List<int> buffer = [];
 
   _VirtualDirectoryFileStream(HttpResponse this.response, String this.path);
 
   Future addStream(Stream<List<int>> stream) {
     stream.listen(
         (data) {
           if (buffer == null) {
             response.add(data);
             return;
           }
@@ skipping 24 matching lines @@
             }
           }
           response.close();
         },
         onError: response.addError);
     return response.done;
   }
 
   Future close() => new Future.value();
 
-  void setMimeType(var bytes) {
+  void setMimeType(List<int> bytes) {
     var mimeType = lookupMimeType(path, headerBytes: bytes);
     if (mimeType != null) {
       response.headers.contentType = ContentType.parse(mimeType);
     }
   }
 }