Skip to content

Refactor routing core, introduce route collection and route finder to reduce router responsibilities#415

Open
armanist wants to merge 8 commits intosoftberg:masterfrom
armanist:412-Refactor-routing-core-introduce-RouteCollection-and-RouteFinder-to-reduce-Router-responsibilities
Open

Refactor routing core, introduce route collection and route finder to reduce router responsibilities#415
armanist wants to merge 8 commits intosoftberg:masterfrom
armanist:412-Refactor-routing-core-introduce-RouteCollection-and-RouteFinder-to-reduce-Router-responsibilities

Conversation

@armanist
Copy link
Member

@armanist armanist commented Feb 3, 2026

Routing Refactor (Breaking Change)

Summary

This PR introduces a breaking refactor of the routing system as part of the 3.0.0 stabilization work.

The focus is correctness, explicit behavior, and test coverage — not redesign.


What changed (high-impact)

  • Routing internals refactored to an object-based architecture:
    • Route
    • RouteCollection
    • MatchedRoute
    • RouteBuilder
    • RouteDispatcher
  • Route matching now produces a MatchedRoute stored on Request
  • Global route helpers (current_*, route_*) now read from MatchedRoute
  • Middleware ordering is strictly prepend-based and enforced by tests
  • Legacy static routing state and implicit controller resolution were removed
  • Extensive unit tests added for routing, helpers, and request integration

⚠️ Breaking changes to review carefully

Contributors should focus review effort on the following areas:

1. Controller resolution

  • Controllers are now instantiated directly from the route handler
  • Legacy RouteController-based resolution is no longer used
  • Verify namespace / FQCN expectations match real applications

2. RouteBuilder behavior

  • Grouping, middleware order, caching, and naming are enforced at build time
  • Route name uniqueness is validated during build

3. Request → routing boundary

  • MatchedRoute is nullable
  • Helpers must safely handle the “no route matched” case
  • Dispatcher is never invoked when no route is matched

What this PR does not do

  • ❌ No DSL changes
  • ❌ No backward-compatibility shims

How to review

Suggested review order:

  1. RouteBuilder and its unit tests
  2. Controller resolution logic in RouteDispatcher
  3. Route helper tests (expected runtime behavior)

This refactor affects core request flow — careful review is appreciated.

Closes #412

@codecov
Copy link

codecov bot commented Feb 3, 2026

Codecov Report

❌ Patch coverage is 86.54244% with 65 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.34%. Comparing base (df8d534) to head (a349d60).

Files with missing lines Patch % Lines
src/Router/RouteBuilder.php 79.38% 27 Missing ⚠️
src/Console/Commands/RouteListCommand.php 0.00% 23 Missing ⚠️
src/Console/Commands/OpenApiCommand.php 0.00% 7 Missing ⚠️
src/Middleware/MiddlewareManager.php 50.00% 4 Missing ⚠️
src/Router/RouteDispatcher.php 94.28% 2 Missing ⚠️
src/Di/Di.php 94.11% 1 Missing ⚠️
src/Router/PatternCompiler.php 98.71% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master     #415      +/-   ##
============================================
- Coverage     81.48%   81.34%   -0.15%     
- Complexity     2691     2716      +25     
============================================
  Files           239      241       +2     
  Lines          7157     7294     +137     
============================================
+ Hits           5832     5933     +101     
- Misses         1325     1361      +36     

☔ 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.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@armanist armanist added enhancement New feature or request help wanted Extra attention is needed labels Feb 3, 2026
@armanist armanist added this to the 3.0.0 milestone Feb 3, 2026
Copy link
Collaborator

@andrey-smaelov andrey-smaelov left a comment

Choose a reason for hiding this comment

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

  1. There are several temp files were pushed (probably by accident) need to be removed
  2. There are several new error messages that appears all over the code, while need to be placed into ExceptionMessages class

return $this->route;
}

/**
Copy link
Collaborator

Choose a reason for hiding this comment

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

Double dock-block to be removed

@@ -0,0 +1,10 @@
Date: Thu, 16 Oct 2025 19:41:01 +0300
Copy link
Collaborator

Choose a reason for hiding this comment

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

temporary generated files appeared to be pushed

if ($callback instanceof \Closure) {
self::callControllerMethod($callback);
if ($closure === null) {
throw new LogicException('Closure route missing closure.');
Copy link
Collaborator

Choose a reason for hiding this comment

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

Error messages should be placed into ExceptionMessages

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request help wanted Extra attention is needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Refactor routing core: introduce RouteCollection and RouteFinder to reduce Router responsibilities

2 participants