don't create Routers when handling the request

dart-lang · davidmartos96 · Sep 10, 2022 · Sep 10, 2022 · Sep 10, 2022 · Sep 10, 2022
commit 1a7d4dcdc7a8156b3275543cca664b002930148d
diff --git a/pkgs/shelf_router/lib/src/router.dart b/pkgs/shelf_router/lib/src/router.dart
@@ -56,6 +56,40 @@ extension RouterParams on Request {
     }
     return _emptyParams;
   }
+
+  /// Get URL parameters captured by the [Router.mount].
+  /// They can be accessed from inside the mounted routes.
+  ///
+  /// **Example**
+  /// ```dart
+  /// Router createUsersRouter() {
+  ///   var router = Router();
+  ///
+  ///   String getUser(Request r) => r.mountedParams['user']!;
+  ///
+  ///   router.get('/self', (Request request) {
+  ///     return Response.ok("I'm ${getUser(request)}");
+  ///   });
+  ///
+  ///   return router;
+  /// }
+  ///
+  /// var app = Router();
+  ///
+  /// final usersRouter = createUsersRouter();
+  /// app.mount('/users/<user>', (Request r, String user) => usersRouter(r));
+  /// ```
+  ///
+  /// If no parameters are captured this returns an empty map.
+  ///
+  /// The returned map is unmodifiable.
+  Map<String, String> get mountedParams {
+    final p = context['shelf_router/mountedParams'];
+    if (p is Map<String, String>) {
+      return UnmodifiableMapView(p);
+    }
+    return _emptyParams;
+  }
 }
 
 /// Middleware to remove body from request.
@@ -148,11 +182,17 @@ class Router {
 
   /// Handle all request to [route] using [handler].
   void all(String route, Function handler) {
-    _all(route, handler, mounted: false);
+    _all(route, handler, applyParamsOnHandle: true);
   }
 
-  void _all(String route, Function handler, {required bool mounted}) {
-    _routes.add(RouterEntry('ALL', route, handler, mounted: mounted));
+  void _all(String route, Function handler,
+      {required bool applyParamsOnHandle}) {
+    _routes.add(RouterEntry(
+      'ALL',
+      route,
+      handler,
+      applyParamsOnHandle: applyParamsOnHandle,
+    ));
   }
 
   /// Mount a handler below a prefix.
@@ -161,7 +201,6 @@ class Router {
       throw ArgumentError.value(prefix, 'prefix', 'must start with a slash');
     }
 
-    // first slash is always in request.handlerPath
     const restPathParam = _kRestPathParam;
 
     if (prefix.endsWith('/')) {
@@ -172,15 +211,15 @@ class Router {
           final paramsList = [...route.params]..removeLast();
           return _invokeMountedHandler(request, handler, paramsList);
         },
-        mounted: true,
+        applyParamsOnHandle: false,
       );
     } else {
       _all(
         prefix,
         (Request request, RouterEntry route) {
           return _invokeMountedHandler(request, handler, route.params);
         },
-        mounted: true,
+        applyParamsOnHandle: false,
       );
       _all(
         prefix + '/<$restPathParam|[^]*>',
@@ -189,7 +228,7 @@ class Router {
           final paramsList = [...route.params]..removeLast();
           return _invokeMountedHandler(request, handler, paramsList);
         },
-        mounted: true,
+        applyParamsOnHandle: false,
       );
     }
   }
@@ -199,8 +238,21 @@ class Router {
     final paramsMap = request.params;
     final effectivePath = _getEffectiveMountPath(request.url.path, paramsMap);
 
+    final modifiedRequest = request.change(
+      path: effectivePath,
+      context: {
+        // Include the parameters captured here as mounted parameters.
+        // We also include previous mounted params in case there is double
+        // nesting of `mount`s
+        'shelf_router/mountedParams': {
+          ...request.mountedParams,
+          ...paramsMap,
+        },
+      },
+    );
+
     return await Function.apply(handler, [
-      request.change(path: effectivePath),
+      modifiedRequest,
       ...pathParams.map((param) => paramsMap[param]),
     ]) as Response;
   }

diff --git a/pkgs/shelf_router/lib/src/router_entry.dart b/pkgs/shelf_router/lib/src/router_entry.dart
@@ -34,9 +34,13 @@ class RouterEntry {
   final Function _handler;
   final Middleware _middleware;
 
-  /// This router entry is used
-  /// as a mount point
-  final bool _mounted;
+  /// If the arguments should be applied or not to the handler function.
+  /// This is useful to have as false when there is
+  /// internal logic that registers routes and the number of expected arguments
+  /// by the user is unknown. i.e: [Router.mount]
+  /// When this is false, this [RouterEntry] is provided as an argument along
+  /// the [Request] so that the caller can read information from the route.
+  final bool _applyParamsOnHandle;
 
   /// Expression that the request path must match.
   ///
@@ -50,14 +54,14 @@ class RouterEntry {
   List<String> get params => _params.toList(); // exposed for using generator.
 
   RouterEntry._(this.verb, this.route, this._handler, this._middleware,
-      this._routePattern, this._params, this._mounted);
+      this._routePattern, this._params, this._applyParamsOnHandle);
 
   factory RouterEntry(
     String verb,
     String route,
     Function handler, {
     Middleware? middleware,
-    bool mounted = false,
+    bool applyParamsOnHandle = true,
   }) {
     middleware = middleware ?? ((Handler fn) => fn);
 
@@ -82,7 +86,14 @@ class RouterEntry {
     final routePattern = RegExp('^$pattern\$');
 
     return RouterEntry._(
-        verb, route, handler, middleware, routePattern, params, mounted);
+      verb,
+      route,
+      handler,
+      middleware,
+      routePattern,
+      params,
+      applyParamsOnHandle,
+    );
   }
 
   /// Returns a map from parameter name to value, if the path matches the
@@ -107,10 +118,8 @@ class RouterEntry {
     request = request.change(context: {'shelf_router/params': params});
 
     return await _middleware((request) async {
-      if (_mounted) {
-        // if this route is mounted, we include
-        // the route itself as a parameter so
-        // that the mount can extract the parameters
+      if (!_applyParamsOnHandle) {
+        // We handle the request just providing this route
         return await _handler(request, this) as Response;
       }
 

diff --git a/pkgs/shelf_router/test/router_test.dart b/pkgs/shelf_router/test/router_test.dart
@@ -204,34 +204,40 @@ void main() {
   });
 
   test('can mount dynamic routes', () async {
-    // routes for an [user] to [other]. This gets nested
-    // parameters from previous mounts
-    Handler createUserToOtherHandler(String user, String other) {
+    // routes for a specific [user]. The user value
+    // is extracted from the mount
+    Router createUsersRouter() {
       var router = Router();
 
-      router.get('/<action>', (Request request, String action) {
-        return Response.ok('$user to $other: $action');
-      });
+      String getUser(Request r) => r.mountedParams['user']!;
 
-      return router;
-    }
+      // Nested mount
+      // Routes for an [user] to [other]. This gets nested
+      // parameters from previous mounts
+      Router createUserToOtherRouter() {
+        var router = Router();
 
-    // routes for a specific [user]. The user value
-    // is extracted from the mount
-    Handler createUserHandler(String user) {
-      var router = Router();
+        String getOtherUser(Request r) => r.mountedParams['other']!;
 
-      router.mount('/to/<other>/', (Request request, String other) {
-        final handler = createUserToOtherHandler(user, other);
-        return handler(request);
-      });
+        router.get('/<action>', (Request request, String action) {
+          return Response.ok(
+            '${getUser(request)} to ${getOtherUser(request)}: $action',
+          );
+        });
+
+        return router;
+      }
+
+      final userToOtherRouter = createUserToOtherRouter();
+      router.mount(
+          '/to/<other>/', (Request r, String other) => userToOtherRouter(r));
 
       router.get('/self', (Request request) {
-        return Response.ok("I'm $user");
+        return Response.ok("I'm ${getUser(request)}");
       });
 
       router.get('/', (Request request) {
-        return Response.ok('$user root');
+        return Response.ok('${getUser(request)} root');
       });
       return router;
     }
@@ -241,10 +247,8 @@ void main() {
       return Response.ok('hello-world');
     });
 
-    app.mount('/users/<user>', (Request request, String user) {
-      final handler = createUserHandler(user);
-      return handler(request);
-    });
+    final usersRouter = createUsersRouter();
+    app.mount('/users/<user>', (Request r, String user) => usersRouter(r));
 
     app.all('/<_|[^]*>', (Request request) {
       return Response.ok('catch-all-handler');
@@ -262,12 +266,24 @@ void main() {
 
   test('can mount dynamic routes with multiple parameters', () async {
     var app = Router();
-    app.mount(r'/first/<second>/third/<fourth|\d+>/last',
-        (Request request, String second, String fourthNum) {
+
+    final mountedRouter = () {
       var router = Router();
-      router.get('/', (r) => Response.ok('$second ${int.parse(fourthNum)}'));
-      return router(request);
-    });
+
+      String getSecond(Request r) => r.mountedParams['second']!;
+      int getFourth(Request r) => int.parse(r.mountedParams['fourth']!);
+
+      router.get(
+        '/',
+        (Request r) => Response.ok('${getSecond(r)} ${getFourth(r)}'),
+      );
+      return router;
+    }();
+
+    app.mount(
+      r'/first/<second>/third/<fourth|\d+>/last',
+      (Request r, String second, String fourth) => mountedRouter(r),
+    );
 
     server.mount(app);
 
@@ -277,11 +293,19 @@ void main() {
   test('can mount dynamic routes with regexp', () async {
     var app = Router();
 
-    app.mount(r'/before/<bookId|\d+>/after', (Request request, String bookId) {
+    final mountedRouter = () {
       var router = Router();
-      router.get('/', (r) => Response.ok('book ${int.parse(bookId)}'));
-      return router(request);
-    });
+
+      int getBookId(Request r) => int.parse(r.mountedParams['bookId']!);
+
+      router.get('/', (Request r) => Response.ok('book ${getBookId(r)}'));
+      return router;
+    }();
+
+    app.mount(
+      r'/before/<bookId|\d+>/after',
+      (Request r, String bookId) => mountedRouter(r),
+    );
 
     app.all('/<_|[^]*>', (Request request) {
       return Response.ok('catch-all-handler');