refactor(Request): Add resolve method to handle routing and parameters, throw NotFoundHttpException for unresolved requests.
#64
Conversation
…s, throw `NotFoundHttpException` for unresolved requests.
WalkthroughA new enum case for "Page not found" was added to the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Request
participant ServerRequestAdapter
participant UrlManager
participant Exception
Client->>Request: resolve()
alt PSR-7 adapter set
Request->>ServerRequestAdapter: resolve(Request)
ServerRequestAdapter->>UrlManager: parseRequest(Request)
alt Route found
UrlManager-->>ServerRequestAdapter: [route, params]
ServerRequestAdapter-->>Request: [route, merged params]
Request-->>Client: [route, merged params]
else Route not found
UrlManager-->>ServerRequestAdapter: false
ServerRequestAdapter->>Exception: throw NotFoundHttpException("Page not found.")
end
else No adapter
Request->>UrlManager: resolve()
alt Route found
UrlManager-->>Request: [route, params]
Request-->>Client: [route, params]
else Route not found
UrlManager-->>Request: false
Request->>Exception: throw NotFoundHttpException("Page not found.")
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #64 +/- ##
===========================================
Coverage 100.00% 100.00%
- Complexity 292 296 +4
===========================================
Files 12 12
Lines 746 757 +11
===========================================
+ Hits 746 757 +11 ☔ View full report in Codecov by Sentry. |
…parameters, throw `NotFoundHttpException` class for unresolved requests.
…n` class in error view logic tests.
…idge into fix_mini_51
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/http/StatelessApplicationTest.php (1)
926-928: PHPDoc comment appears incomplete.The PHPDoc comment on line 927 seems incomplete - it should likely be
@throws InvalidConfigException if the configuration is invalid or incomplete.to match the pattern used in other test methods.Apply this fix:
/** - * if the configuration is invalid or incomplete. + * @throws InvalidConfigException if the configuration is invalid or incomplete. */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/adapter/ServerRequestAdapter.php(2 hunks)src/http/Request.php(4 hunks)tests/TestCase.php(1 hunks)tests/http/StatelessApplicationTest.php(8 hunks)tests/support/stub/SiteController.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/http/Request.php
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#53
File: src/http/ErrorHandler.php:258-272
Timestamp: 2025-08-03T16:24:09.241Z
Learning: In yii2-extensions/psr-bridge, the StatelessApplication creates a new Response instance for each request in the reset() method, then passes it to ErrorHandler::setResponse(). This means the template response is not shared across requests, so calling clear() on it in createErrorResponse() is safe and doesn't cause side effects.
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#53
File: src/http/ErrorHandler.php:258-272
Timestamp: 2025-08-03T16:24:09.241Z
Learning: In yii2-extensions/psr-bridge, the StatelessApplication creates a new Response instance for each request in the reset() method, then passes it to ErrorHandler::setResponse(). This means the template response is not shared across requests, so calling clear() on it in createErrorResponse() is safe and doesn't cause side effects.
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#53
File: src/http/ErrorHandler.php:258-272
Timestamp: 2025-08-03T16:24:09.241Z
Learning: In yii2-extensions/psr-bridge, the StatelessApplication creates a new Response instance for each request in the reset() method (line 408: `$this->response = new Response($this->components['response'] ?? []);`), then passes it to ErrorHandler::setResponse(). This means the template response is not shared across requests, so calling clear() on it in createErrorResponse() is safe and doesn't cause side effects.
📚 Learning: in yii2-extensions/psr-bridge, the responseadapter::formatcookieheader() method uses `$expire !== 1`...
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#21
File: tests/http/PSR7ResponseTest.php:0-0
Timestamp: 2025-07-22T00:50:26.546Z
Learning: In yii2-extensions/psr-bridge, the ResponseAdapter::formatCookieHeader() method uses `$expire !== 1` to skip validation for Yii2's special deletion cookies, but this should be extended to handle all expired cookies, not just the special case where expire=1.
Applied to files:
src/adapter/ServerRequestAdapter.php
📚 Learning: in yii2-extensions/psr-bridge, expired cookies should not be hashed/validated because they are delet...
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#21
File: tests/http/PSR7ResponseTest.php:0-0
Timestamp: 2025-07-22T01:01:13.426Z
Learning: In yii2-extensions/psr-bridge, expired cookies should not be hashed/validated because they are deletion cookies meant to remove existing cookies from the client browser. The validation logic should only apply to live cookies (expire=0 or expire >= current time) and skip validation for both the special Yii2 deletion case (expire=1) and regular expired cookies.
Applied to files:
src/adapter/ServerRequestAdapter.php
📚 Learning: in yii2-extensions/psr-bridge, the statelessapplication creates a new response instance for each req...
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#53
File: src/http/ErrorHandler.php:258-272
Timestamp: 2025-08-03T16:24:09.241Z
Learning: In yii2-extensions/psr-bridge, the StatelessApplication creates a new Response instance for each request in the reset() method, then passes it to ErrorHandler::setResponse(). This means the template response is not shared across requests, so calling clear() on it in createErrorResponse() is safe and doesn't cause side effects.
Applied to files:
src/adapter/ServerRequestAdapter.phptests/http/StatelessApplicationTest.php
📚 Learning: in yii2-extensions/psr-bridge cookie validation logic, a cookie is only considered expired when expi...
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#21
File: tests/adapter/ResponseAdapterTest.php:436-450
Timestamp: 2025-07-22T10:56:44.424Z
Learning: In yii2-extensions/psr-bridge cookie validation logic, a cookie is only considered expired when expire < current time (strictly less than). When expire == current time, the cookie is still valid and validation/hashing should apply. Only cookies with expire < current time should skip validation.
Applied to files:
src/adapter/ServerRequestAdapter.php
📚 Learning: in yii2-extensions/psr-bridge, the statelessapplication creates a new response instance for each req...
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#53
File: src/http/ErrorHandler.php:258-272
Timestamp: 2025-08-03T16:24:09.241Z
Learning: In yii2-extensions/psr-bridge, the StatelessApplication creates a new Response instance for each request in the reset() method (line 408: `$this->response = new Response($this->components['response'] ?? []);`), then passes it to ErrorHandler::setResponse(). This means the template response is not shared across requests, so calling clear() on it in createErrorResponse() is safe and doesn't cause side effects.
Applied to files:
src/adapter/ServerRequestAdapter.phptests/http/StatelessApplicationTest.php
📚 Learning: in the yii2-extensions/psr-bridge project, the base testcase class already handles $_server cleanup ...
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#6
File: tests/http/RequestTest.php:1536-1552
Timestamp: 2025-07-20T16:35:15.341Z
Learning: In the yii2-extensions/psr-bridge project, the base TestCase class already handles $_SERVER cleanup in setUp() and tearDown() methods, so individual test methods that extend TestCase don't need manual $_SERVER restoration.
Applied to files:
src/adapter/ServerRequestAdapter.phptests/TestCase.phptests/http/StatelessApplicationTest.php
📚 Learning: in the yii2-extensions/psr-bridge project, the base testcase class already handles $_server cleanup ...
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#6
File: tests/http/RequestTest.php:1536-1552
Timestamp: 2025-07-20T16:35:15.341Z
Learning: In the yii2-extensions/psr-bridge project, the base TestCase class already handles $_SERVER cleanup in setUp() and tearDown() methods (lines 28 and 32), so individual test methods that extend TestCase don't need manual $_SERVER restoration.
Applied to files:
src/adapter/ServerRequestAdapter.phptests/TestCase.phptests/http/StatelessApplicationTest.php
📚 Learning: the testcase class in yii2-extensions/psr-bridge automatically handles $_server superglobal cleanup ...
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#6
File: tests/http/RequestTest.php:1564-1578
Timestamp: 2025-07-20T16:33:57.495Z
Learning: The TestCase class in yii2-extensions/psr-bridge automatically handles $_SERVER superglobal cleanup by saving its original state before each test and restoring it afterward in setUp() and tearDown() methods. Manual $_SERVER cleanup in individual test methods is unnecessary when extending this TestCase.
Applied to files:
tests/TestCase.phptests/http/StatelessApplicationTest.php
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: mutation / PHP 8.3-ubuntu-latest
- GitHub Check: mutation / PHP 8.3-ubuntu-latest
🔇 Additional comments (10)
tests/TestCase.php (1)
152-153: LGTM! URL routing rules updated correctly.The routing configuration has been properly updated to support the new
site/updateaction with numeric ID parameter. Both string-based rule formats are valid in Yii2 and the parameter patterns are appropriately constrained.src/adapter/ServerRequestAdapter.php (2)
12-12: LGTM! Imports added for new functionality.The required imports for
NotFoundHttpExceptionandRequestclasses are correctly added to support the new resolve method.Also applies to: 14-14
369-404: Well-implemented route resolution method.The
resolve()method is well-designed with proper error handling and documentation. The implementation correctly:
- Uses Yii2's
UrlManager::parseRequest()for route parsing- Combines parsed parameters with query parameters (parsed params take precedence)
- Throws
NotFoundHttpExceptionwith localized message for unresolved routes- Includes comprehensive PHPDoc with usage example
The parameter combination using
+operator gives correct precedence to parsed parameters over query parameters.tests/support/stub/SiteController.php (3)
315-318: LGTM! Exception testing method properly implemented.The
actionTriggerException()method is correctly implemented for testing exception handling with proper PHPDoc annotations and return type.
323-326: LGTM! UserException testing method properly implemented.The
actionTriggerUserException()method is correctly implemented for testing user-friendly exception handling with appropriate annotations.
331-336: LGTM! Update action correctly implemented.The
actionUpdate()method is properly implemented with:
- Correct JSON response format setting
- Proper parameter typing with nullable string
- Appropriate return type annotation
- Simple JSON structure that aligns with the new routing rule
This method effectively supports testing of the new route resolution functionality.
tests/http/StatelessApplicationTest.php (4)
13-13: LGTM: Clean import addition.Adding
Exceptionto the grouped import improves maintainability and supports the use ofException::classconstant elsewhere in the file.
697-699: LGTM: Improved exception documentation.The addition of
@throws InvalidConfigExceptionannotations consistently documents potential configuration-related exceptions across test methods. This enhances code documentation and helps developers understand potential failure modes.
1240-1279: LGTM: Well-structured test for route parameter handling.The new test method properly validates:
- Route parameter extraction from URL path (
site/update/123)- JSON response format and content
- Request URI path preservation
- Appropriate HTTP status codes
This test aligns well with the PR's route resolution functionality and follows the established testing patterns in the file.
879-879: LGTM: Improved maintainability with class constant.Replacing the hardcoded string
'yii\base\Exception'withException::classimproves maintainability, provides better IDE support, and eliminates potential typos. This aligns with modern PHP best practices.
… for `InvalidConfigException` class in error view logic tests.
…StatelessApplicationTest` class.
… `NotFoundHttpException` class tests and improve method naming.
…lessApplicationTest` class.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
tests/TestCase.php(1 hunks)tests/http/StatelessApplicationTest.php(12 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/TestCase.php
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#53
File: src/http/ErrorHandler.php:258-272
Timestamp: 2025-08-03T16:24:09.241Z
Learning: In yii2-extensions/psr-bridge, the StatelessApplication creates a new Response instance for each request in the reset() method, then passes it to ErrorHandler::setResponse(). This means the template response is not shared across requests, so calling clear() on it in createErrorResponse() is safe and doesn't cause side effects.
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#53
File: src/http/ErrorHandler.php:258-272
Timestamp: 2025-08-03T16:24:09.241Z
Learning: In yii2-extensions/psr-bridge, the StatelessApplication creates a new Response instance for each request in the reset() method, then passes it to ErrorHandler::setResponse(). This means the template response is not shared across requests, so calling clear() on it in createErrorResponse() is safe and doesn't cause side effects.
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#53
File: src/http/ErrorHandler.php:258-272
Timestamp: 2025-08-03T16:24:09.241Z
Learning: In yii2-extensions/psr-bridge, the StatelessApplication creates a new Response instance for each request in the reset() method (line 408: `$this->response = new Response($this->components['response'] ?? []);`), then passes it to ErrorHandler::setResponse(). This means the template response is not shared across requests, so calling clear() on it in createErrorResponse() is safe and doesn't cause side effects.
📚 Learning: in yii2-extensions/psr-bridge, the statelessapplication creates a new response instance for each req...
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#53
File: src/http/ErrorHandler.php:258-272
Timestamp: 2025-08-03T16:24:09.241Z
Learning: In yii2-extensions/psr-bridge, the StatelessApplication creates a new Response instance for each request in the reset() method, then passes it to ErrorHandler::setResponse(). This means the template response is not shared across requests, so calling clear() on it in createErrorResponse() is safe and doesn't cause side effects.
Applied to files:
tests/http/StatelessApplicationTest.php
📚 Learning: in yii2-extensions/psr-bridge, the statelessapplication creates a new response instance for each req...
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#53
File: src/http/ErrorHandler.php:258-272
Timestamp: 2025-08-03T16:24:09.241Z
Learning: In yii2-extensions/psr-bridge, the StatelessApplication creates a new Response instance for each request in the reset() method (line 408: `$this->response = new Response($this->components['response'] ?? []);`), then passes it to ErrorHandler::setResponse(). This means the template response is not shared across requests, so calling clear() on it in createErrorResponse() is safe and doesn't cause side effects.
Applied to files:
tests/http/StatelessApplicationTest.php
📚 Learning: in the yii2-extensions/psr-bridge project, the base testcase class already handles $_server cleanup ...
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#6
File: tests/http/RequestTest.php:1536-1552
Timestamp: 2025-07-20T16:35:15.341Z
Learning: In the yii2-extensions/psr-bridge project, the base TestCase class already handles $_SERVER cleanup in setUp() and tearDown() methods, so individual test methods that extend TestCase don't need manual $_SERVER restoration.
Applied to files:
tests/http/StatelessApplicationTest.php
📚 Learning: in the yii2-extensions/psr-bridge project, the base testcase class already handles $_server cleanup ...
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#6
File: tests/http/RequestTest.php:1536-1552
Timestamp: 2025-07-20T16:35:15.341Z
Learning: In the yii2-extensions/psr-bridge project, the base TestCase class already handles $_SERVER cleanup in setUp() and tearDown() methods (lines 28 and 32), so individual test methods that extend TestCase don't need manual $_SERVER restoration.
Applied to files:
tests/http/StatelessApplicationTest.php
📚 Learning: the testcase class in yii2-extensions/psr-bridge automatically handles $_server superglobal cleanup ...
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#6
File: tests/http/RequestTest.php:1564-1578
Timestamp: 2025-07-20T16:33:57.495Z
Learning: The TestCase class in yii2-extensions/psr-bridge automatically handles $_SERVER superglobal cleanup by saving its original state before each test and restoring it afterward in setUp() and tearDown() methods. Manual $_SERVER cleanup in individual test methods is unnecessary when extending this TestCase.
Applied to files:
tests/http/StatelessApplicationTest.php
📚 Learning: in yii2-extensions/psr-bridge, expired cookies should not be hashed/validated because they are delet...
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#21
File: tests/http/PSR7ResponseTest.php:0-0
Timestamp: 2025-07-22T01:01:13.426Z
Learning: In yii2-extensions/psr-bridge, expired cookies should not be hashed/validated because they are deletion cookies meant to remove existing cookies from the client browser. The validation logic should only apply to live cookies (expire=0 or expire >= current time) and skip validation for both the special Yii2 deletion case (expire=1) and regular expired cookies.
Applied to files:
tests/http/StatelessApplicationTest.php
🧬 Code Graph Analysis (1)
tests/http/StatelessApplicationTest.php (4)
tests/support/FactoryHelper.php (2)
FactoryHelper(46-268)createServerRequestCreator(154-161)tests/TestCase.php (1)
statelessApplication(106-168)src/adapter/ServerRequestAdapter.php (1)
resolve(390-404)src/exception/Message.php (1)
getMessage(223-226)
🔇 Additional comments (6)
tests/http/StatelessApplicationTest.php (6)
13-19: LGTM! Import statements properly support new functionality.The added imports for
Exception,NotFoundHttpException, andMessageare correctly placed and necessary for the new test methods and updated assertions.
698-700: LGTM! PHPDoc annotations improve documentation.The
@throws InvalidConfigExceptionannotations are correctly added to test methods that callstatelessApplication(), which can throw this exception. The documentation is consistent and helpful for static analysis.Also applies to: 734-736, 790-792, 842-844, 903-905, 939-941, 993-995, 1111-1113, 1165-1167
880-880: LGTM! Class constant usage improves maintainability.Using
Exception::classinstead of a hardcoded string'yii\base\Exception'is a best practice that provides better IDE support and compile-time safety.
1241-1280: LGTM! Comprehensive test for route parameter handling.This test method effectively validates the new route parameter functionality:
- Properly tests the
site/update/<id:\d+>URL rule- Verifies correct JSON response with route parameter extraction
- Includes thorough assertions for status code, content type, response body, and request path
- Follows established testing patterns in the file
1903-1937: LGTM! Well-structured test for missing route handling with strict parsing disabled.This test effectively validates the NotFoundHttpException behavior:
- Correctly tests a non-existent route scenario
- Verifies 404 status code and proper HTML error response
- The expected error message aligns with the new
Message::PAGE_NOT_FOUNDenum case- Supports the PR's objective of proper exception handling for unresolved requests
1241-1243: LGTM! Consistent PHPDoc annotations.The
@throws InvalidConfigExceptionannotations are properly applied to all test methods that callstatelessApplication(), maintaining consistency across the test file.Also applies to: 1903-1905, 1939-1941, 1993-1995, 2052-2054, 2111-2113, 2165-2167
Summary by CodeRabbit
New Features
Documentation