Add getParams method to router.dart

[CLNMR]

This PR adds a getParams method to the handler. This allows to extract the params of a URI from a request without actually handling the request in a middleware.

As far as I am aware, this was not possible in any other way before, as the routes are private in a Router.

---

• I’ve reviewed the contributor guide and applied the relevant portions to this PR.

Contribution guidelines:

• See our contributor guide for general expectations for PRs.
• Larger or significant changes should be discussed in an issue before creating a PR.
• Contributions to our repos should follow the Dart style guide and use dart format.
• Most changes should add an entry to the changelog and may need to rev the pubspec package version.
• Changes to packages require corresponding tests.

Note that many Dart repos have a weekly cadence for reviewing PRs - please allow for some latency before initial review feedback.

[google-cla]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[kevmoo]

@CLNMR – add tests and an entry to the changelog, please!

[CLNMR]

@CLNMR – add tests and an entry to the changelog, please!

Sorry @kevmoo, I added them!

[kevmoo]

@devoncarew @natebosch – thoughts here?

[kevmoo]

@CLNMR – need to change the pubspec version to match the changelog!

[natebosch]

If we decide to land this, how about the name urlParameters.

https://dart.dev/effective-dart/design#avoid-starting-a-method-name-with-get

[natebosch]

I think @jonasfj might have the most context on this package and whether this is a good solution.

I'm not sure I fully understand the problem. What is the use case where URL parameters must be read without handling the request?

[jonasfj]

Functionality is already there:

https://pub.dev/documentation/shelf_router/latest/shelf_router/params.html

update, I missed this: 🤣

This allows to extract the params of a URI from a request without actually handling the request in a middleware.

[jonasfj]

It's kind of intentional that internals of this package are not public. It makes it possible to refactor the implementation, add new features without doing breaking changes.

We could make _routes public, but if you want fast moving APIs that can do everything, perhaps it better to build it as a package in the community.

What is the intended use case for this?

[jonasfj]

Technically, the route is not necessarily matched.
The Handler for the route may return Router.routeNotFound, in which case the Router will try all subsequent routes.

See documentation for Router.routeNotFound.

It's kind of weird to look at matched parameters from different routes and mounted routers.

[CLNMR]

Yes, that's true, but only when a route was matched and found are the params returned. Therefore you can deduce whether the router found a route or not (see also my comment below).

[CLNMR]

Hi, thanks for the feedback.

To expand a little on our use case:
We have several routers, each with a number of different routes.
Now, a lot of those routes (but not all!) have a workspaceId in their path. Currently, in each function that handles such a route, we must look up (and verify) the workspace using this id.
We would love to move this logic into a middleware, that populates the request with the workspace. BUT: This logic should only be used when <workspaceId> is part of the path!

This is where this feature comes in: Using this, we can give the list of routers to the middleware that parses the workspace. In this middleware, we can then call the proposed function (via null-aware assignment) so that we get the extracted parameters from the router that will later match the requested route. Then, we can run the initialisation logic for the workspace iff the workspaceId was extracted.

Here are the important parts of our current code using this feature:

server.dart

void main() async {
  final routers = [
    RecordApiService().router,
    CalendarApiService().router,
    SwaggerService().router,
    FlowApiService().router,
    BillingAccountApiService().router,
  ];

  final pipeline = Pipeline()
      .addMiddleware(logRequests())
      .addMiddleware(exceptionHandler)
      .addMiddleware(corsHeaders)
      .addMiddleware(**routeInspectorMiddleware**(routers))
      .addMiddleware(sessionControllerMiddleware)
      .addMiddleware(recordSpecMiddleware)
      .addMiddleware(recordMiddleware)
      .addMiddleware(flowSpecMiddleware)
      .addMiddleware(stripeCustomerMiddleware)
      .addHandler(routers.fold<Cascade>(Cascade(), (c, r) => c.add(r)).handler);

  await io.serve(pipeline, '0.0.0.0', int.parse(portStr));
}

route_inspector_middleware.dart

Middleware routeInspectorMiddleware(List<Router> routers) {
  return (Handler handler) {
    return (Request request) {
      for (final router in routers) {
        final params = router.getParams(request);
        if (params != null) {
          // Attach the matched route to the request context
          final updatedRequest = request.change(context: {'params': params});
          return handler(updatedRequest);
        }
      }
      return handler(request);
    };
  };
}

session_controller_middleware.dart

FutureOr<Response> Function(Request) sessionControllerMiddleware(
  Handler handler,
) =>
    (request) async {
      final workspaceId =
          (request.context['params'] as Map<String, dynamic>?)?['workspaceId'];

      if (workspaceId is String) {
        // do initialisation logic to get stateSnap
        final updatedRequest =
            request.change(context: {'stateSnap': stateSnap});
        return handler(updatedRequest);
      }
      return handler(request);
    };

[jonasfj]

Have you considered being more explicit, this is a bit scary because suddenly <workspaceId> will have all sorts of side effects. You can afaik do:

Suppose you have:

final router = Router();

router.get('/api/<workspaceId>/info', (Request request, String workspaceId) async {
  final workspace = await loadWorkspace(workspaceId);
  ...
});
router.get('/api/<workspaceId>/file/<id>', (Request request, String workspaceId, String id) async {
  final workspace = await loadWorkspace(workspaceId);
  ...
});
router.put('/api/<workspaceId>/file/<id>', (Request request, String workspaceId, String id) async {
  final workspace = await loadWorkspace(workspaceId);
  ...
});

IMO: This is not a bad setup at all, having to call loadWorkspace in every handler is annoying, but it works.

If you wanted smarter you could also do an extension method:

extension WorkspaceRequest on Request {
  Future<Workspace> get workspace {
    final workspaceId = params['workspaceId'];
    if (workspaceId == null) {
      throw StateError('Router for this handler does contain /<workspaceId>/');
    }
    return loadWorkspace(workspaceId);
  }
}

final router = Router();

router.get('/api/<workspaceId>/info', (Request request, String _) async {
  final workspace = await request.workspace;
  ...
});
router.get('/api/<workspaceId>/file/<id>', (Request request, String _, String id) async {
  final workspace = await request.workspace;
  ...
});
router.put('/api/<workspaceId>/file/<id>', (Request request, String _, String id) async {
  final workspace = await request.workspace;
  ...
});

Of course accessing request.workspace outside a handler for a route with /<workspaceId>/ in the route will throw.

---

If we should add more features to Router I think we should consider support for middleware.

extension WorkspaceRequest on Request {
  Workspace get workspace => context['workspace'] as Workspace;
}

final router = Router();

// NOTE: middlewares are not supported at the moment.
// But you can almost do it by returning Router.routeNotFound, you just can modify
// the request when doing this.
router.middleware('/api/<workspaceId>', (Handler handler, String workspaceId) async {
  final workspace = await loadWorkspace(workspaceId);
  return (request) {
    return handler(request.change(context: {'workspace': workspace}));
  }
});

router.get('/api/<workspaceId>/info', (Request request, String _) async {
  final workspace = await request.workspace;
  ...
});
router.get('/api/<workspaceId>/file/<id>', (Request request, String _, String id) async {
  final workspace = await request.workspace;
  ...
});
router.put('/api/<workspaceId>/file/<id>', (Request request, String _, String id) async {
  final workspace = await request.workspace;
  ...
});

Yes, you might need to add this middleware to all the different route patterns that contain /<workspaceId>/, but probably you have a limited number of such patterns -- much fewer than you have routes. And now it's all more explicit.

---

Just some quick ideas off the top of my head.