Merge branch 'feat/mount_dynamic_regexp' into feat/mount_dynamic_routes

dart-lang · Sep 11, 2022 · ac228e8 · ac228e8
2 parents 874da11 + 54a2f9a
commit ac228e8
Show file tree

Hide file tree

Showing 3 changed files with 122 additions and 24 deletions.
diff --git a/pkgs/shelf_router/lib/src/router.dart b/pkgs/shelf_router/lib/src/router.dart
@@ -19,7 +19,7 @@ import 'package:http_methods/http_methods.dart';
 import 'package:meta/meta.dart' show sealed;
 import 'package:shelf/shelf.dart';
 
-import 'router_entry.dart' show RouterEntry;
+import 'router_entry.dart' show ParamInfo, RouterEntry;
 
 /// Get a URL parameter captured by the [Router].
 @Deprecated('Use Request.params instead')
@@ -167,7 +167,7 @@ class Router {
         prefix + '<$pathParam|[^]*>',
         (Request request, RouterEntry route) {
           // Remove path param from extracted route params
-          final paramsList = [...route.params]..removeLast();
+          final paramsList = [...route.paramInfos]..removeLast();
           return _invokeMountedHandler(request, handler, path, paramsList);
         },
         mounted: true,
@@ -176,15 +176,16 @@ class Router {
       _all(
         prefix,
         (Request request, RouterEntry route) {
-          return _invokeMountedHandler(request, handler, path, route.params);
+          return _invokeMountedHandler(
+              request, handler, path, route.paramInfos);
         },
         mounted: true,
       );
       _all(
         prefix + '/<$pathParam|[^]*>',
         (Request request, RouterEntry route) {
           // Remove path param from extracted route params
-          final paramsList = [...route.params]..removeLast();
+          final paramsList = [...route.paramInfos]..removeLast();
           return _invokeMountedHandler(
               request, handler, path + '/', paramsList);
         },
@@ -194,13 +195,14 @@ class Router {
   }
 
   Future<Response> _invokeMountedHandler(Request request, Function handler,
-      String path, List<String> paramsList) async {
+      String path, List<ParamInfo> paramInfos) async {
     final params = _getParamsFromRequest(request);
-    final resolvedPath = _replaceParamsInPath(request, path, params);
+    final resolvedPath =
+        _replaceParamsInPath(request, path, params, paramInfos);
 
     return await Function.apply(handler, [
       request.change(path: resolvedPath),
-      ...paramsList.map((n) => params[n]),
+      ...paramInfos.map((info) => params[info.name]),
     ]) as Response;
   }
 
@@ -214,15 +216,37 @@ class Router {
     Request request,
     String path,
     Map<String, String> params,
+    List<ParamInfo> paramInfos,
   ) {
-    // TODO(davidmartos96): Maybe this could be done in a different way
-    // to avoid replacing the path N times, N being the number of params
-    var resolvedPath = path;
-    for (final paramEntry in params.entries) {
-      resolvedPath =
-          resolvedPath.replaceFirst('<${paramEntry.key}>', paramEntry.value);
+    // we iterate the non-resolved path and we write to a StringBuffer
+    // resolving ther parameters along the way
+    final resolvedPathBuff = StringBuffer();
+    var paramIndex = 0;
+    var charIndex = 0;
+    while (charIndex < path.length) {
+      if (paramIndex < paramInfos.length) {
+        final paramInfo = paramInfos[paramIndex];
+        if (charIndex < paramInfo.startIdx - 1) {
+          // Add up until the param slot starts
+          final part = path.substring(charIndex, paramInfo.startIdx - 1);
+          resolvedPathBuff.write(part);
+          charIndex += part.length;
+        } else {
+          // Add the resolved value of the parameter
+          final paramName = paramInfo.name;
+          final paramValue = params[paramName]!;
+          resolvedPathBuff.write(paramValue);
+          charIndex = paramInfo.endIdx - 1;
+          paramIndex++;
+        }
+      } else {
+        // All params looped, so add up until the end of the path
+        final part = path.substring(charIndex, path.length);
+        resolvedPathBuff.write(part);
+        charIndex += part.length;
+      }
     }
-
+    var resolvedPath = resolvedPathBuff.toString();
     return resolvedPath;
   }
 

diff --git a/pkgs/shelf_router/lib/src/router_entry.dart b/pkgs/shelf_router/lib/src/router_entry.dart
@@ -44,13 +44,16 @@ class RouterEntry {
   final RegExp _routePattern;
 
   /// Names for the parameters in the route pattern.
-  final List<String> _params;
+  final List<ParamInfo> _paramInfos;
+
+  List<ParamInfo> get paramInfos => _paramInfos.toList();
 
   /// List of parameter names in the route pattern.
-  List<String> get params => _params.toList(); // exposed for using generator.
+  // exposed for using generator.
+  List<String> get params => _paramInfos.map((p) => p.name).toList();
 
   RouterEntry._(this.verb, this.route, this._handler, this._middleware,
-      this._routePattern, this._params, this._mounted);
+      this._routePattern, this._paramInfos, this._mounted);
 
   factory RouterEntry(
     String verb,
@@ -66,12 +69,26 @@ class RouterEntry {
           route, 'route', 'expected route to start with a slash');
     }
 
-    final params = <String>[];
+    final params = <ParamInfo>[];
     var pattern = '';
+    // Keep the index where the matches are located
+    // so that we can calculate the positioning of
+    // the extracted parameter
+    var prevMatchIndex = 0;
     for (var m in _parser.allMatches(route)) {
-      pattern += RegExp.escape(m[1]!);
+      final firstGroup = m[1]!;
+      pattern += RegExp.escape(firstGroup);
       if (m[2] != null) {
-        params.add(m[2]!);
+        final paramName = m[2]!;
+        final startIdx = prevMatchIndex + firstGroup.length;
+        final paramInfo = ParamInfo(
+          name: paramName,
+          startIdx: startIdx,
+          endIdx: m.end,
+        );
+        params.add(paramInfo);
+        prevMatchIndex = m.end;
+
         if (m[3] != null && !_isNoCapture(m[3]!)) {
           throw ArgumentError.value(
               route, 'route', 'expression for "${m[2]}" is capturing');
@@ -95,9 +112,10 @@ class RouterEntry {
     }
     // Construct map from parameter name to matched value
     var params = <String, String>{};
-    for (var i = 0; i < _params.length; i++) {
+    for (var i = 0; i < _paramInfos.length; i++) {
       // first group is always the full match, we ignore this group.
-      params[_params[i]] = m[i + 1]!;
+      final paramInfo = _paramInfos[i];
+      params[paramInfo.name] = m[i + 1]!;
     }
     return params;
   }
@@ -114,14 +132,37 @@ class RouterEntry {
         return await _handler(request, this) as Response;
       }
 
-      if (_handler is Handler || _params.isEmpty) {
+      if (_handler is Handler || _paramInfos.isEmpty) {
         return await _handler(request) as Response;
       }
 
       return await Function.apply(_handler, [
         request,
-        ..._params.map((n) => params[n]),
+        ..._paramInfos.map((info) => params[info.name]),
       ]) as Response;
     })(request);
   }
 }
+
+/// This class holds information about a parameter extracted
+/// from the route path.
+/// The indexes can by used by the mount logic to resolve the
+/// parametrized path when handling the request.
+class ParamInfo {
+  /// This is the name of the parameter, without <, >
+  final String name;
+
+  /// The index in the route String where the parameter
+  /// expression starts (inclusive)
+  final int startIdx;
+
+  /// The index in the route String where the parameter
+  /// expression ends (exclusive)
+  final int endIdx;
+
+  const ParamInfo({
+    required this.name,
+    required this.startIdx,
+    required this.endIdx,
+  });
+}
diff --git a/pkgs/shelf_router/test/router_test.dart b/pkgs/shelf_router/test/router_test.dart
@@ -259,4 +259,37 @@ void main() {
     expect(await get('/users/jake'), 'jake root');
     expect(await get('/users/david/no-route'), 'catch-all-handler');
   });
+
+  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) {
+      var router = Router();
+      router.get('/', (r) => Response.ok('$second ${int.parse(fourthNum)}'));
+      return router(request);
+    });
+
+    server.mount(app);
+
+    expect(await get('/first/hello/third/12/last'), 'hello 12');
+  });
+
+  test('can mount dynamic routes with regexp', () async {
+    var app = Router();
+
+    app.mount(r'/before/<bookId|\d+>/after', (Request request, String bookId) {
+      var router = Router();
+      router.get('/', (r) => Response.ok('book ${int.parse(bookId)}'));
+      return router(request);
+    });
+
+    app.all('/<_|[^]*>', (Request request) {
+      return Response.ok('catch-all-handler');
+    });
+
+    server.mount(app);
+
+    expect(await get('/before/123/after'), 'book 123');
+    expect(await get('/before/abc/after'), 'catch-all-handler');
+  });
 }