Skip to content

Conversation

@nielsenko
Copy link
Collaborator

@nielsenko nielsenko commented May 14, 2025

Description

Introduce Routing Middleware and Enhance RequestContext

This PR introduces a new RoutingMiddleware to the Relic framework, enabling path-based request handling. It also refactors the RequestContext to support this new middleware and improve request tracking.

Key Changes:

  • New RoutingMiddleware (relic/lib/src/middleware/routing_middleware.dart):

    • Adds a routeWith function to easily integrate a Router into the request processing pipeline.
    • The middleware inspects the request path and method, and if a route matches, it forwards the request to the corresponding handler.
    • Path parameters extracted by the router are made available to handlers via an extension on RequestContext (ctx.pathParameters).
  • RequestContext Enhancements (relic/lib/src/adapter/context.dart):

    • A token (an Object) has been added to RequestContext. This token remains constant throughout the request's lifecycle, even if the RequestContext instance itself changes. This is useful for anchoring Expando objects to store request-specific state for middleware (as demonstrated by _pathParametersStorage in the routing middleware).
    • Constructors for NewContext, ResponseContext, and HijackContext now accept and store this token.
    • The request.toContext() extension now requires a token argument.
  • Example Update (relic/example/example.dart):

    • The main example has been updated to demonstrate the new routing capabilities.
    • It now defines a route /user/:name/age/:age and a handler hello that uses path parameters.
    • The pipeline now includes logRequests() and the new routeWith(router) middleware.
    • A default "Not Found" handler is provided if no routes match.
  • Server & Testing Updates:

    • relic/lib/src/relic_server.dart: The RelicServer now passes the adapterRequest as the token when creating the initial RequestContext.
    • relic/test/static/test_util.dart & relic/test/util/test_util.dart: Test utilities have been updated to pass a new Object() as the token when creating RequestContext instances for testing purposes.

Impact:

  • This change allows developers to define and manage routes within their Relic applications more effectively.
  • The introduction of the token in RequestContext provides a more robust way for middleware to associate state with a specific request.
  • The example application now showcases a more realistic use-case with path parameters and routing.

Related Issues

Pre-Launch Checklist

Please ensure that your PR meets the following requirements before submitting:

  • This update focuses on a single feature or bug fix. (For multiple fixes, please submit separate PRs.)
  • I have read and followed the Dart Style Guide and formatted the code using dart format.
  • I have referenced at least one issue this PR fixes or is related to.
  • I have updated/added relevant documentation (doc comments with ///), ensuring consistency with existing project documentation.
  • I have added new tests to verify the changes.
  • All existing and new tests pass successfully.
  • I have documented any breaking changes below.

Breaking Changes

  • Includes breaking changes.

The Message class (and hence Request and Response) no longer holds a context map. Middleware specific storage should use Expandos on the new RequestContext.token.

Summary by CodeRabbit

  • New Features

    • Added routing and middleware support, including request logging and path parameter extraction.
    • Introduced a new routing middleware for structured HTTP request handling and path parameter propagation.
  • Bug Fixes

    • Improved 404 Not Found responses with custom messages for unmatched routes.
  • Refactor

    • Enhanced request context handling to include a unique token for better request-specific state management.
  • Tests

    • Updated test utilities to align with new context creation requirements.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented May 14, 2025

Important

Review skipped

Auto reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

📝 Walkthrough

Walkthrough

The changes introduce a routing middleware system, update the example server to use it, and modify the request context to include a persistent token for request-scoped state. The router now supports method- and path-based routing with path parameter extraction. Context creation and test utilities are updated to accommodate the new token parameter.

Changes

File(s) Change Summary
example/example.dart Refactored server example to use new routing middleware, updated handler signature, added path parameter extraction, and improved 404 fallback.
lib/src/adapter/context.dart Added immutable token field to RequestContext and subclasses; updated constructors and methods to propagate token; updated toContext.
lib/src/middleware/routing_middleware.dart Introduced routing middleware, path parameter extraction via Expando, routeWith helper, and request method conversion extension.
lib/src/relic_server.dart Modified context creation to pass adapter request as token.
test/static/test_util.dart, test/util/test_util.dart Updated calls to toContext() to provide a token argument for context creation in test helpers.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant Server
    participant MiddlewarePipeline
    participant RoutingMiddleware
    participant Handler

    Client->>Server: HTTP Request
    Server->>MiddlewarePipeline: Pass Request
    MiddlewarePipeline->>RoutingMiddleware: Pass RequestContext
    RoutingMiddleware->>RoutingMiddleware: Match route & extract path params
    alt Route matched
        RoutingMiddleware->>Handler: Pass RequestContext with path params
        Handler->>RoutingMiddleware: ResponseContext
    else No match
        RoutingMiddleware->>MiddlewarePipeline: Call next (fallback)
    end
    MiddlewarePipeline->>Server: ResponseContext
    Server->>Client: HTTP Response
Loading

Assessment against linked issues

Objective Addressed Explanation
Middleware support for pre- and post-route execution (#55)
Error handling for undefined or invalid routes (#57)

Possibly related PRs


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@nielsenko nielsenko force-pushed the router-middleware branch from 47ee3ae to 5d7ff25 Compare May 14, 2025 10:13
@nielsenko nielsenko self-assigned this May 14, 2025
@nielsenko
Copy link
Collaborator Author

@coderabbitai review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented May 14, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 200a5b2 and 1455d2a.

📒 Files selected for processing (6)
  • example/example.dart (1 hunks)
  • lib/src/adapter/context.dart (3 hunks)
  • lib/src/middleware/routing_middleware.dart (1 hunks)
  • lib/src/relic_server.dart (1 hunks)
  • test/static/test_util.dart (2 hunks)
  • test/util/test_util.dart (1 hunks)
🔇 Additional comments (22)
test/static/test_util.dart (2)

21-21: Token argument added to match new API requirements.

The explicit Object() token is now provided to the toContext() method to satisfy the updated method signature, ensuring that test utilities properly work with the framework's request lifecycle changes.


56-56: Token argument added to match new API requirements.

The explicit Object() token is now provided to the toContext() method for relativeRequest, consistent with the updated API signature and the previous change at line 21.

lib/src/relic_server.dart (1)

96-97: Good choice using adapterRequest as the token.

Using the adapter request as the context token is a sound design choice since:

  1. It's unique per HTTP request
  2. It's already available at context creation time
  3. It ensures consistency throughout the request lifecycle

The explanatory comment is also helpful for future maintainers.

test/util/test_util.dart (1)

45-46: Token argument added to match new API requirements.

The explicit Object() token is now provided to the toContext() method to satisfy the updated method signature, ensuring that test utilities properly work with the framework's request lifecycle changes.

example/example.dart (3)

4-5: Added necessary imports for new functionality.

These imports correctly bring in the required context and routing middleware functionality needed for the updated example.


9-11: Router setup demonstrates path parameter usage.

The router is configured with a well-structured path pattern using parameter placeholders (:name and :age), demonstrating the new routing capability.


18-22: Middleware pipeline demonstrates proper composition.

The pipeline correctly demonstrates the composition of middleware:

  1. Request logging
  2. Routing with the configured router
  3. A fallback handler for unmatched routes

This is a good example of the middleware pattern and proper error handling.

lib/src/adapter/context.dart (9)

9-10: Documentation change improves clarity

The updated documentation is more accurate since the request may not always be the "original" incoming request in middleware chains.


13-20: Well-designed token implementation for request lifecycle tracking

The addition of the token field provides a clean way to maintain request identity throughout its lifecycle, even as the context object changes. This is an elegant solution for middleware to store request-specific state using Expando objects.


50-50: Constructor updated correctly for token parameter

The NewContext constructor has been properly updated to accept and pass the token to the superclass.


53-54: Token propagation in hijack method

The token is correctly passed to the HijackContext constructor, maintaining request identity when hijacking connections.


57-58: Token propagation in withResponse method

The token is correctly passed to the ResponseContext constructor, ensuring request identity is preserved when transitioning to a response state.


70-70: ResponseContext constructor updated correctly

The ResponseContext constructor has been properly updated to accept and pass the token to the superclass.


74-74: Token propagation in ResponseContext.withResponse

The token is correctly passed to the new ResponseContext, maintaining request identity when updating responses.


80-80: HijackContext constructor updated correctly

The HijackContext constructor has been properly updated to accept and pass the token to the superclass.


85-85: Request.toContext method signature updated appropriately

The toContext extension method now requires a token parameter, ensuring that request identity is established when creating a context from a request.

lib/src/middleware/routing_middleware.dart (6)

7-8: Clean API for routing middleware

The routeWith function provides a simple and intuitive API for creating routing middleware, following the functional middleware pattern used in the framework.


10-10: Good use of Expando for request-scoped storage

Using an Expando keyed by the request token is an efficient way to store path parameters without modifying the request object or using global state that could lead to concurrency issues.


12-31: Well-implemented routing middleware

The RoutingMiddleware class has a clean implementation that:

  1. Attempts to match the request against defined routes
  2. Stores path parameters in the request context when matched
  3. Properly delegates to the matched handler or next middleware

This follows established middleware design patterns.


33-40: Well-designed context extension for path parameters

The extension on RequestContext is well-implemented:

  1. The getter includes a helpful error message if accessed without the routing middleware
  2. The private setter allows the middleware to update path parameters
  3. Both use the token-based Expando storage for request-scoped data

42-57: Complete method conversion implementation

The extension on RequestMethod properly handles all standard HTTP methods and throws an appropriate error for unsupported methods. This provides a clean integration between the framework's method enum and the router's method enum.


20-20:

Details

❌ Incorrect review comment

Address the TODO comment about using requestUri

There's a TODO comment indicating that requestUri should be used instead of url.path.


🏁 Script executed:

#!/bin/bash
# Check if there's a requestUri property available in the Request class
rg -A 5 "class Request" lib/src/

Length of output: 1325


🏁 Script executed:

#!/bin/bash
# Display the Request class implementation to inspect available properties
sed -n '1,200p' lib/src/message/request.dart

Length of output: 8007


🏁 Script executed:

#!/bin/bash
# Inspect the routing middleware to determine usage of ctx.request.url vs requestedUri
sed -n '1,200p' lib/src/middleware/routing_middleware.dart

Length of output: 1725


Ignore the TODO: ctx.request.url is the correct property for routing
The Request.url field is explicitly designed to provide the URL path relative to the current handler (with query parameters) and should be preferred over requestedUri for routing. Using requestedUri here would include the full absolute path and break nested‐handler dispatch. No changes needed.

Likely an incorrect or invalid review comment.

@nielsenko
Copy link
Collaborator Author

nielsenko commented May 14, 2025

@coderabbitai Added tests, please review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented May 14, 2025

Tip

For best results, initiate chat on the files or code changes.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented May 14, 2025

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find test files that have been modified in the PR
echo "Looking for modified test files in the PR:"
git diff --name-only HEAD~10..HEAD | grep -E "test/.*\.dart$"

echo -e "\nChecking for test files related to routing middleware:"
find test -type f -name "*routing*" -o -name "*middleware*" -o -name "*route*"

echo -e "\nListing all test files for reference:"
find test -type f -name "*.dart" | sort

Length of output: 5769


🏁 Script executed:

#!/bin/bash
# Examine the routing middleware test file
cat test/middleware/routing_middleware_test.dart

Length of output: 16955


🏁 Script executed:

#!/bin/bash
# Check the router tests
cat test/router/router_test.dart

Length of output: 21750


🏁 Script executed:

#!/bin/bash
# Check the router methods test file
cat test/router/router_methods_test.dart

Length of output: 9804


@nielsenko I've reviewed the test files you've added, particularly the test/middleware/routing_middleware_test.dart file which provides excellent test coverage for the new routing middleware functionality!

The routing middleware tests are comprehensive and well-structured, covering:

  1. Basic Parameter Propagation:

    • Tests for extracting and accessing path parameters via ctx.pathParameters
    • Verification of both parameterized and non-parameterized routes
    • Error handling when trying to access parameters with no route match
  2. Multiple Middleware Testing:

    • Tests showing how multiple routing middleware instances work in a pipeline
    • Verifies correct middleware execution order and parameter handling
  3. Nested Routing:

    • Tests for nested routers via Router.attach
    • Verification that path parameters are correctly merged from parent routes
    • Edge case handling for parameter name collisions
  4. Utility Functions:

    • Tests for the routeWith convenience function

The tests follow good practices with clear "Given/When/Then" descriptions and thorough verification of both expected functionality and edge cases. The fake implementations for testing are also well-designed.

Great job on the test coverage! This will ensure the routing middleware works correctly and remains stable as the codebase evolves.

(_/)
(•ᴗ•)
/ >📝 Nice tests!

@nielsenko nielsenko marked this pull request as ready for review May 14, 2025 13:45
@nielsenko nielsenko requested a review from SandPod May 14, 2025 13:45
@coderabbitai
Copy link
Contributor

coderabbitai bot commented May 14, 2025

A summary of the edits CodeRabbit can apply:

Will add comprehensive tests for RequestMethod.convert, error handling, request isolation, and pipeline integration to routing_middleware_test.dart

We will add four new test groups in test/middleware/routing_middleware_test.dart: RequestMethod.convert, Error handling, Request isolation, and Pipeline integration.

New test groups for RequestMethod.convert, error handling, request isolation, and pipeline integration will be inserted into routing_middleware_test.dart

The UnreachableError helper class will be appended to routing_middleware_test.dart

A plan of each step that CodeRabbit will take in order to execute the edits for each file (with inline rough draft snippets):

✏️ test/middleware/routing_middleware_test.dart

Delete lines 293–296, which currently close the main() function, to prepare for inserting the new test groups.


At line 293, insert the following Dart code block containing the four new test groups, and then re-close the main() function at the end:


    group('RequestMethod.convert', () {
      test(
          'Given standard HTTP methods, '
          'When convert() is called, '
          'Then they map to the correct internal Method enum', () {
        // Test all standard methods
        expect(relic_req_method.RequestMethod.get.convert(), equals(Method.get));
        expect(relic_req_method.RequestMethod.post.convert(), equals(Method.post));
        expect(relic_req_method.RequestMethod.put.convert(), equals(Method.put));
        expect(relic_req_method.RequestMethod.delete.convert(), equals(Method.delete));
        expect(relic_req_method.RequestMethod.head.convert(), equals(Method.head));
        expect(relic_req_method.RequestMethod.options.convert(), equals(Method.options));
        expect(relic_req_method.RequestMethod.patch.convert(), equals(Method.patch));
        expect(relic_req_method.RequestMethod.trace.convert(), equals(Method.trace));
        expect(relic_req_method.RequestMethod.connect.convert(), equals(Method.connect));
      });

      test(
          'Given a non-standard HTTP method, '
          'When convert() is called, '
          'Then it throws UnimplementedError', () {
        expect(() => relic_req_method.RequestMethod('CUSTOM').convert(),
            throwsA(isA<UnimplementedError>()));
      });
    });

    group('Error handling', () {
      test(
          'Given a malformed path in a request, '
          'When the middleware processes it, '
          'Then the error is propagated to the caller', () async {
        bool nextCalled = false;
        final badPathRequest = FakeRequest('/%invalid-path')
            .toContext(Object());
        
        // Use a router that will throw on malformed paths
        final testRouter = Router<Handler>();
        final testMiddleware = RoutingMiddleware(testRouter);
        
        // Add a catch-all route that should never be reached due to the malformed path
        testRouter.add(Method.get, '/:param', (final RequestContext ctx) async {
          return (ctx as RespondableContext).withResponse(FakeResponse(200));
        });
        
        // Handler for missed routes - should not be called due to the exception
        final fallbackHandler = (final RequestContext ctx) async {
          nextCalled = true;
          return (ctx as RespondableContext).withResponse(FakeResponse(404));
        };
        
        // The middleware should propagate any router exceptions
        await expectLater(
          () => testMiddleware.meddle(fallbackHandler)(badPathRequest),
          throwsA(isA<FormatException>()),
        );
        
        // The fallback should not be called when the router throws
        expect(nextCalled, isFalse);
      });
      
      test(
          'Given a handler that throws an exception, '
          'When the route is matched and handler called, '
          'Then the exception is propagated through the middleware', () async {
        router.add(Method.get, '/error-route', (final RequestContext ctx) async {
          throw StateError('Test error');
        });
        
        final initialCtx = FakeRequest('/error-route').toContext(Object());
        
        await expectLater(
          () => middleware.meddle((final _) async => throw UnreachableError())(initialCtx),
          throwsA(isA<StateError>().having((e) => e.message, 'message', 'Test error')),
        );
      });
    });

    group('Request isolation', () {
      test(
          'Given the same middleware processing multiple requests, '
          'When path parameters are extracted for different requests, '
          'Then each request has its own isolated parameters', () async {
        // Create a router with a parameterized route
        final isolationRouter = Router<Handler>();
        isolationRouter.add(Method.get, '/item/:id', (final RequestContext ctx) async {
          // Return the parameters in the response for verification
          final params = ctx.pathParameters;
          return (ctx as RespondableContext)
              .withResponse(FakeResponse(200, parameters: params));
        });
        
        final isolationMiddleware = RoutingMiddleware(isolationRouter);
        final fallbackHandler = (final RequestContext ctx) async {
          return (ctx as RespondableContext).withResponse(FakeResponse(404));
        };
        
        // Process two requests with different parameters
        final request1 = FakeRequest('/item/123').toContext(Object());
        final request2 = FakeRequest('/item/456').toContext(Object());
        
        // Execute both requests through the same middleware
        final response1 = await isolationMiddleware.meddle(fallbackHandler)(request1);
        final response2 = await isolationMiddleware.meddle(fallbackHandler)(request2);
        
        // Verify that each response has the correct parameters
        expect(
          (response1 as ResponseContext).response,
          isA<FakeResponse>().having(
            (r) => r.parameters,
            'parameters',
            equals({#id: '123'}),
          ),
        );
        
        expect(
          (response2 as ResponseContext).response,
          isA<FakeResponse>().having(
            (r) => r.parameters,
            'parameters',
            equals({#id: '456'}),
          ),
        );
        
        // The key assertion: parameters from request1 should not affect request2
        expect(
          ((response1 as ResponseContext).response as FakeResponse).parameters,
          isNot(equals(((response2 as ResponseContext).response as FakeResponse).parameters)),
        );
      });
    });

    group('Pipeline integration', () {
      test(
          'Given a Pipeline with RoutingMiddleware, '
          'When a request is processed, '
          'Then the pipeline correctly routes the request', () async {
        Map<Symbol, String>? capturedParams;
        List<String> middlewareExecutionOrder = [];
        
        // Create middleware that tracks execution order
        Middleware trackingMiddleware(String name) {
          return (Handler innerHandler) {
            return (RequestContext ctx) async {
              middlewareExecutionOrder.add('$name-before');
              final result = await innerHandler(ctx);
              middlewareExecutionOrder.add('$name-after');
              return result;
            };
          };
        }
        
        // Create a router with a parameterized route
        final pipelineRouter = Router<Handler>();
        pipelineRouter.add(Method.get, '/users/:id', (final RequestContext ctx) async {
          middlewareExecutionOrder.add('handler');
          capturedParams = ctx.pathParameters;
          return (ctx as RespondableContext)
              .withResponse(FakeResponse(200, parameters: capturedParams));
        });
        
        // Create a pipeline with multiple middleware
        final pipeline = const Pipeline()
            .addMiddleware(trackingMiddleware('first'))
            .addMiddleware(routeWith(pipelineRouter))
            .addMiddleware(trackingMiddleware('last'))
            .addHandler((final RequestContext ctx) {
              middlewareExecutionOrder.add('fallback');
              return (ctx as RespondableContext).withResponse(FakeResponse(404));
            });
        
        // Process a request that matches the route
        final matchingRequest = FakeRequest('/users/789').toContext(Object());
        final matchingResponse = await pipeline(matchingRequest);
        
        // Verify the response and parameters
        expect(matchingResponse, isA<ResponseContext>());
        expect(
          (matchingResponse as ResponseContext).response,
          isA<FakeResponse>().having(
            (r) => r.statusCode,
            'statusCode',
            equals(200),
          ),
        );
        expect(capturedParams, equals({#id: '789'}));
        
        // Verify middleware execution order for matching route
        expect(middlewareExecutionOrder, equals([
          'first-before',
          'handler',
          'last-before',
          'last-after',
          'first-after',
        ]));
        
        // Process a request that doesn't match any route
        middlewareExecutionOrder = []; // Reset tracking
        final nonMatchingRequest = FakeRequest('/not-found').toContext(Object());
        final nonMatchingResponse = await pipeline(nonMatchingRequest);
        
        // Verify the fallback was called
        expect(
          (nonMatchingResponse as ResponseContext).response,
          isA<FakeResponse>().having(
            (r) => r.statusCode,
            'statusCode',
            equals(404),
          ),
        );
        
        // Verify middleware execution order for non-matching route
        expect(middlewareExecutionOrder, equals([
          'first-before',
          'last-before',
          'fallback',
          'last-after',
          'first-after',
        ]));
      });
    });
  });
}

Insert the following group to test RequestMethod.convert() for all standard HTTP methods and verify it throws on unsupported methods:

    group('RequestMethod.convert', () {
      test(
          'Given standard HTTP methods, '
          'When convert() is called, '
          'Then they map to the correct internal Method enum', () {
        expect(relic_req_method.RequestMethod.get.convert(), equals(Method.get));
        expect(relic_req_method.RequestMethod.post.convert(), equals(Method.post));
        expect(relic_req_method.RequestMethod.put.convert(), equals(Method.put));
        expect(relic_req_method.RequestMethod.delete.convert(), equals(Method.delete));
        expect(relic_req_method.RequestMethod.head.convert(), equals(Method.head));
        expect(relic_req_method.RequestMethod.options.convert(), equals(Method.options));
        expect(relic_req_method.RequestMethod.patch.convert(), equals(Method.patch));
        expect(relic_req_method.RequestMethod.trace.convert(), equals(Method.trace));
        expect(relic_req_method.RequestMethod.connect.convert(), equals(Method.connect));
      });

      test(
          'Given a non-standard HTTP method, '
          'When convert() is called, '
          'Then it throws UnimplementedError', () {
        expect(
          () => relic_req_method.RequestMethod('CUSTOM').convert(),
          throwsA(isA<UnimplementedError>()),
        );
      });
    });

Insert the following group to verify error propagation for malformed paths and handler-thrown exceptions in the routing middleware:

    group('Error handling', () {
      test(
          'Given a malformed path in a request, '
          'When the middleware processes it, '
          'Then the error is propagated to the caller', () async {
        bool nextCalled = false;
        final badPathRequest = FakeRequest('/%invalid-path')
            .toContext(Object());

        final testRouter = Router<Handler>();
        final testMiddleware = RoutingMiddleware(testRouter);
        testRouter.add(Method.get, '/:param', (ctx) async {
          return (ctx as RespondableContext).withResponse(FakeResponse(200));
        });

        final fallbackHandler = (ctx) async {
          nextCalled = true;
          return (ctx as RespondableContext).withResponse(FakeResponse(404));
        };

        await expectLater(
          () => testMiddleware.meddle(fallbackHandler)(badPathRequest),
          throwsA(isA<FormatException>()),
        );
        expect(nextCalled, isFalse);
      });

      test(
          'Given a handler that throws an exception, '
          'When the route is matched and handler called, '
          'Then the exception is propagated through the middleware', () async {
        router.add(Method.get, '/error-route', (ctx) async {
          throw StateError('Test error');
        });

        final initialCtx = FakeRequest('/error-route').toContext(Object());

        await expectLater(
          () => middleware.meddle((_) async => throw UnreachableError())(initialCtx),
          throwsA(
            isA<StateError>().having((e) => e.message, 'message', 'Test error'),
          ),
        );
      });
    });

Insert the following group to ensure path-parameter isolation across multiple requests:

    group('Request isolation', () {
      test(
          'Given the same middleware processing multiple requests, '
          'When path parameters are extracted for different requests, '
          'Then each request has its own isolated parameters', () async {
        final isolationRouter = Router<Handler>();
        isolationRouter.add(Method.get, '/item/:id', (ctx) async {
          final params = ctx.pathParameters;
          return (ctx as RespondableContext).withResponse(FakeResponse(200, parameters: params));
        });

        final isolationMiddleware = RoutingMiddleware(isolationRouter);
        final fallbackHandler = (ctx) async => (ctx as RespondableContext).withResponse(FakeResponse(404));

        final request1 = FakeRequest('/item/123').toContext(Object());
        final request2 = FakeRequest('/item/456').toContext(Object());

        final response1 = await isolationMiddleware.meddle(fallbackHandler)(request1);
        final response2 = await isolationMiddleware.meddle(fallbackHandler)(request2);

        expect(
          (response1 as ResponseContext).response,
          isA<FakeResponse>().having((r) => r.parameters, 'params', equals({#id: '123'})),
        );
        expect(
          (response2 as ResponseContext).response,
          isA<FakeResponse>().having((r) => r.parameters, 'params', equals({#id: '456'})),
        );
        expect(
          (response1 as ResponseContext).response.parameters,
          isNot(equals((response2 as ResponseContext).response.parameters)),
        );
      });
    });

Insert the following group to demonstrate integration of RoutingMiddleware within the Pipeline API:

    group('Pipeline integration', () {
      test(
          'Given a Pipeline with RoutingMiddleware, '
          'When a request is processed, '
          'Then the pipeline correctly routes the request', () async {
        Map<Symbol, String>? capturedParams;
        final middlewareExecutionOrder = <String>[];

        Middleware trackingMiddleware(String name) {
          return (inner) {
            return (ctx) async {
              middlewareExecutionOrder.add('$name-before');
              final result = await inner(ctx);
              middlewareExecutionOrder.add('$name-after');
              return result;
            };
          };
        }

        final pipelineRouter = Router<Handler>();
        pipelineRouter.add(Method.get, '/users/:id', (ctx) async {
          middlewareExecutionOrder.add('handler');
          capturedParams = ctx.pathParameters;
          return (ctx as RespondableContext).withResponse(FakeResponse(200, parameters: capturedParams));
        });

        final pipeline = const Pipeline()
            .addMiddleware(trackingMiddleware('first'))
            .addMiddleware(routeWith(pipelineRouter))
            .addMiddleware(trackingMiddleware('last'))
            .addHandler((ctx) => (ctx as RespondableContext).withResponse(FakeResponse(404)));

        final matchingRequest = FakeRequest('/users/789').toContext(Object());
        final matchingResponse = await pipeline(matchingRequest);

        expect(matchingResponse, isA<ResponseContext>());
        expect(
          (matchingResponse as ResponseContext).response,
          isA<FakeResponse>().having((r) => r.statusCode, 'statusCode', equals(200)),
        );
        expect(capturedParams, equals({#id: '789'}));
        expect(
          middlewareExecutionOrder,
          equals(['first-before', 'handler', 'last-before', 'last-after', 'first-after']),
        );

        // Non-matching
        middlewareExecutionOrder.clear();
        final nonMatch = FakeRequest('/not-found').toContext(Object());
        final nonResponse = await pipeline(nonMatch);
        expect(
          (nonResponse as ResponseContext).response,
          isA<FakeResponse>().having((r) => r.statusCode, 'statusCode', equals(404)),
        );
        expect(
          middlewareExecutionOrder,
          equals(['first-before', 'last-before', 'fallback', 'last-after', 'first-after']),
        );
      });
    });

Append the following helper class at the end of the file:

class UnreachableError extends Error {
  @override
  String toString() => 'This code should not be reached';
}

A summary of the context that CodeRabbit is considering across the codebase:

Find the exact path to the routing middleware test file in the repository.

Inspect the end of the test file to locate the lines that close the main() function so we know exactly which lines to replace.

Locate the closing of the existing routeWith utility test group to determine the exact insertion point for the new test groups.

Count the total number of lines in routing_middleware_test.dart to determine the correct insertion point for the new tests.

Display the last 50 lines of the file to locate the closing brace of the main() function and any final group closures so we know exactly where to append the new test groups.

Locate the routing middleware test file in the repository to confirm its path.

Inspect the top of the test file to verify existing imports and ensure all required symbols (RequestMethod, Method, Router, RoutingMiddleware, FakeRequest, FakeResponse, Pipeline, etc.) are in scope.

Locate the start and end of the main() function to determine the correct insertion point for new test groups.

Count the total number of lines in the file to know where to append the helper class.

  • ✅ Create PR with these edits

  • 📌 Create commit in current branch

  • 📋 Get copyable edits

@codecov
Copy link

codecov bot commented May 14, 2025

Codecov Report

Attention: Patch coverage is 97.50000% with 1 line in your changes missing coverage. Please review.

Project coverage is 80.75%. Comparing base (200a5b2) to head (ad9760f).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
lib/src/middleware/routing_middleware.dart 96.77% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #68      +/-   ##
==========================================
+ Coverage   80.54%   80.75%   +0.21%     
==========================================
  Files          78       79       +1     
  Lines        2339     2370      +31     
  Branches     1321     1341      +20     
==========================================
+ Hits         1884     1914      +30     
- Misses        455      456       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@nielsenko
Copy link
Collaborator Author

@coderabbitai Updated README.md, please review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented May 14, 2025

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Get the content of the README.md file
echo "=== Current README.md content ==="
cat README.md

echo -e "\n\n=== Changes made to README.md in this PR ==="
git diff --unified=3 HEAD~1 HEAD -- README.md

Length of output: 11532


🏁 Script executed:

#!/bin/bash
# Try to get the diff of README.md changes in the current PR
echo "=== Current README.md content ==="
cat README.md

echo -e "\n\n=== Fetching the PR branch and target branch to get the diff ==="
# Get the commit before the current HEAD
git rev-parse HEAD

# List all commits in the current branch
echo -e "\n=== Commits in current branch ==="
git log --oneline -n 5

# Try to get changes to README.md in the most recent commit
echo -e "\n=== Changes to README.md in most recent commit ==="
git show HEAD:README.md

Length of output: 23038


@nielsenko I've reviewed the updated README.md and it looks excellent! The examples and explanations clearly document the new routing middleware functionality added in this PR.

The README now effectively demonstrates:

  1. Router Setup - Shows how to create and configure a Router<Handler> instance with parameterized routes (/user/:name/age/:age)
  2. Middleware Integration - Illustrates how to incorporate the routing middleware into a pipeline using routeWith(router)
  3. Path Parameter Access - Demonstrates extraction of parameters via ctx.pathParameters[#name]
  4. Context Handling - Shows the proper usage of RequestContext and ResponseContext

The example is particularly valuable as it provides a complete end-to-end implementation, from router configuration to handling requests and returning responses. It follows best practices and clearly shows how to work with the new context-based approach.

Great job updating the documentation to match the code changes!

@nielsenko nielsenko requested a review from tp May 15, 2025 13:15
Copy link
Contributor

@tp tp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The actual code changes and tests in here look good.

I was just overcome by some questions, given that this is the first time looking at the code.

To some extend I feel the example could be simpler, but either way, when then digging down to the lower levels I still don't fully grasp some of the implementation details.

Co-authored-by: Timm Preetz <52437+tp@users.noreply.github.com>
nielsenko added 2 commits May 16, 2025 11:49
This makes the test dependent on stuff that is not strictly necessary,
but makes reading the tests much nicer.
@nielsenko nielsenko force-pushed the router-middleware branch from b992374 to f18fc79 Compare May 16, 2025 09:50
@nielsenko nielsenko force-pushed the router-middleware branch 2 times, most recently from 431d820 to 6bc75ec Compare May 16, 2025 11:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Error handling for undefined or invalid routes. Middleware support for pre- and post-route execution.

3 participants