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.

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

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

Sorry @kevmoo, I added them!

@devoncarew @natebosch – thoughts here?

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

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?

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:

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));
}

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);
    };
  };
}

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);
    };

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.