Introduce ErrorHandler and StatelessApplication implementation with tests.
#32
Conversation
|
""" WalkthroughThis change introduces a new Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant StatelessApplication
participant ErrorHandler
participant YiiRequest
participant YiiResponse
Client->>StatelessApplication: handle(ServerRequestInterface)
StatelessApplication->>StatelessApplication: reset(request)
StatelessApplication->>YiiRequest: Inject PSR-7 request
StatelessApplication->>StatelessApplication: Trigger EVENT_BEFORE_REQUEST
StatelessApplication->>StatelessApplication: Handle request
StatelessApplication->>StatelessApplication: Trigger EVENT_AFTER_REQUEST
StatelessApplication->>YiiResponse: Convert to PSR-7 Response
StatelessApplication->>StatelessApplication: terminate(response)
StatelessApplication-->>Client: PSR-7 Response
alt Exception occurs
StatelessApplication->>ErrorHandler: handleException(exception)
ErrorHandler->>YiiResponse: renderException(exception)
ErrorHandler-->>StatelessApplication: Response
StatelessApplication-->>Client: PSR-7 Error Response
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15–20 minutes Possibly related PRs
Suggested labels
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🧰 Additional context used🧠 Learnings (3)📓 Common learningstests/support/stub/SiteController.php (6)Learnt from: terabytesoftw Learnt from: terabytesoftw Learnt from: terabytesoftw Learnt from: terabytesoftw Learnt from: terabytesoftw Learnt from: terabytesoftw tests/http/StatelessApplicationTest.php (6)Learnt from: terabytesoftw Learnt from: terabytesoftw Learnt from: terabytesoftw Learnt from: terabytesoftw Learnt from: terabytesoftw Learnt from: terabytesoftw ⏰ 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)
🔇 Additional comments (28)
✨ 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. 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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #32 +/- ##
=============================================
- Coverage 100.00% 93.07% -6.93%
- Complexity 220 280 +60
=============================================
Files 10 12 +2
Lines 532 722 +190
=============================================
+ Hits 532 672 +140
- Misses 0 50 +50 ☔ View full report in Codecov by Sentry. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
src/errorhandler/ErrorHandler.php (1)
85-87: Consider removing runtime display_errors modificationSetting
display_errorsat runtime may not work in all environments and is generally discouraged. Error display should be controlled via PHP configuration.Consider removing this runtime setting and rely on proper PHP configuration:
- if (YII_DEBUG) { - ini_set('display_errors', 'true'); - }src/http/StatelessApplication.php (2)
144-146: Document uopz extension requirementThe use of
uopz_redefinerequires the uopz extension which is not commonly available in production environments. Consider documenting this optional dependency.Add a comment explaining the purpose and optional nature:
// override YII_BEGIN_TIME if possible for yii2-debug and other modules that depend on it + // Note: This requires the uopz extension and is optional for debugging purposes if (\function_exists('uopz_redefine')) { \uopz_redefine('YII_BEGIN_TIME', microtime(true)); }
62-72: Consider making memory threshold configurableThe 90% memory threshold is hardcoded. Consider making it configurable for different use cases.
Add a class property for the threshold:
+ private float $memoryThreshold = 0.90; + public function clean(): bool { gc_collect_cycles(); $limit = (int) $this->memoryLimit; - $bound = $limit * .90; + $bound = $limit * $this->memoryThreshold; $usage = memory_get_usage(true); return $usage >= $bound ? true : false; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
composer.json(1 hunks)src/errorhandler/ErrorHandler.php(1 hunks)src/http/StatelessApplication.php(1 hunks)tests/phpstan-config.php(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
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.
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.
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.
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#6
File: src/adapter/ServerRequestAdapter.php:15-15
Timestamp: 2025-07-19T18:46:16.591Z
Learning: In Yii2 framework, direct public property access is commonly used instead of getter/setter methods, which differs from strict OOP encapsulation patterns. This is an established framework convention that should be followed in Yii2 extensions.
tests/phpstan-config.php (2)
Learnt from: terabytesoftw
PR: #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.
Learnt from: terabytesoftw
PR: #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.
composer.json (2)
Learnt from: terabytesoftw
PR: #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.
Learnt from: terabytesoftw
PR: #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.
src/errorhandler/ErrorHandler.php (3)
Learnt from: terabytesoftw
PR: #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.
Learnt from: terabytesoftw
PR: #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.
Learnt from: terabytesoftw
PR: #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.
src/http/StatelessApplication.php (3)
Learnt from: terabytesoftw
PR: #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.
Learnt from: terabytesoftw
PR: #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.
Learnt from: terabytesoftw
PR: #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.
🧬 Code Graph Analysis (1)
tests/phpstan-config.php (3)
src/errorhandler/ErrorHandler.php (1)
ErrorHandler(13-101)src/http/Request.php (1)
Request(44-523)src/http/Response.php (2)
Response(38-102)Response(38-107)
🔇 Additional comments (3)
composer.json (1)
17-17: LGTM!The addition of
psr/http-server-handlerdependency is appropriate for implementing the PSR-15RequestHandlerInterfaceinStatelessApplication.tests/phpstan-config.php (1)
7-21: LGTM!The PHPStan configuration correctly registers the new PSR-7 bridge components for static analysis.
src/http/StatelessApplication.php (1)
202-215: Excellent event cleanup implementationThe event cleanup properly handles deregistration in reverse order (LIFO) and includes comprehensive cleanup with
Event::offAll().
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/http/StatelessApplication.php (2)
1-43: Class structure and imports look good.The class properly extends
yii\web\Applicationand implementsRequestHandlerInterfacefor PSR-7 compatibility. All necessary imports are present and property declarations are well-typed with PHPStan annotations.Note: The version string typo mentioned in previous reviews has been correctly fixed to "0.1.0".
217-242: Memory limit parsing correctly handles unlimited memory case.The method now properly handles the
-1unlimited memory case that was mentioned in previous reviews, setting it toPHP_INT_MAX. The suffix parsing for K/M/G units is also correctly implemented.
🧹 Nitpick comments (1)
src/http/StatelessApplication.php (1)
62-72: Simplify the return statement in clean method.The method logic is sound with appropriate memory monitoring and garbage collection.
- return $usage >= $bound ? true : false; + return $usage >= $bound;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
composer-require-checker.json(1 hunks)src/http/StatelessApplication.php(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- composer-require-checker.json
⏰ 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). (4)
- GitHub Check: mutation / PHP 8.4-ubuntu-latest
- GitHub Check: phpunit / PHP 8.1-windows-latest
- GitHub Check: phpunit / PHP 8.2-windows-latest
- GitHub Check: mutation / PHP 8.4-ubuntu-latest
🔇 Additional comments (8)
src/http/StatelessApplication.php (8)
50-60: Constructor properly implements stateless initialization.The constructor correctly defers parent initialization to support the stateless request handling pattern. Cookie settings are appropriately configured for stateless operation, and the PHPStan ignore annotation is justified.
77-99: Core components configuration looks correct.The method properly merges parent components with PSR-bridge specific implementations, ensuring proper integration of custom ErrorHandler, Request, and Response classes.
101-124: PSR-7 request handling implementation is well-structured.The method properly implements the RequestHandlerInterface with appropriate state management, event triggering, and exception handling. The try-catch ensures errors are properly handled and responses are always PSR-7 compliant.
131-139: Bootstrap method correctly sets up Yii aliases.The method properly configures webroot and web aliases from the request before calling parent bootstrap.
183-200: Terminate method implements proper cleanup sequence.The method correctly handles event cleanup, file reset, logging, request reset, and response validation before returning the PSR-7 response.
202-215: Event cleanup implementation is thorough.The method properly detaches the global event handler, reverses through registered events for cleanup, and calls Event::offAll() for complete cleanup.
244-259: Error handling delegation is properly implemented.The method correctly validates the ErrorHandler instance and delegates exception handling while maintaining proper application state and event triggering.
261-271: Event tracking methods are simple and effective.The initialization and start methods properly set up global event tracking for later cleanup, which is essential for the stateless operation model.
fix(phpstan.neon): ensure yii2 config path is set correctly.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
src/http/StatelessApplication.php (1)
299-300: Consider alternative to parent constructor recallCalling
parent::__construct()after the object is already constructed is unusual and requires suppressing static analysis. This might indicate a design issue.Consider refactoring to use a factory pattern or a separate initialization method instead of reconstructing the parent:
protected function reinitialize(array $config): void { // Reset application components and state without calling parent constructor $this->setComponents($this->coreComponents()); $this->configure($config); // ... other initialization logic }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
composer.json(2 hunks)phpstan.neon(1 hunks)src/http/ErrorHandler.php(1 hunks)src/http/StatelessApplication.php(1 hunks)tests/phpstan-config.php(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- phpstan.neon
- composer.json
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/phpstan-config.php
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
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.
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.
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.
src/http/ErrorHandler.php (3)
Learnt from: terabytesoftw
PR: #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.
Learnt from: terabytesoftw
PR: #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.
Learnt from: terabytesoftw
PR: #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.
src/http/StatelessApplication.php (3)
Learnt from: terabytesoftw
PR: #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.
Learnt from: terabytesoftw
PR: #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.
Learnt from: terabytesoftw
PR: #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.
🧬 Code Graph Analysis (1)
src/http/ErrorHandler.php (6)
src/http/Response.php (2)
Response(38-102)Response(38-107)tests/support/stub/HTTPFunctions.php (1)
http_response_code(143-150)src/exception/HeadersAlreadySentException.php (1)
HeadersAlreadySentException(45-45)src/http/Request.php (1)
Request(44-523)src/exception/OutputAlreadySentException.php (1)
OutputAlreadySentException(45-45)src/adapter/ServerRequestAdapter.php (1)
ServerRequestAdapter(47-451)
⏰ 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.4-ubuntu-latest
- GitHub Check: mutation / PHP 8.4-ubuntu-latest
🔇 Additional comments (5)
src/http/ErrorHandler.php (1)
16-41: Ensure LSP compatibility in handleException return typeYour override of
handleExceptioninsrc/http/ErrorHandler.phpchanges its signature from the parent’svoid handleException($exception)to returning aResponse. Since PHP only allows covariant return types when the parent declares a return type, this may break substitutability if the base class still expects no return value.Please verify the parent signature in
yii\web\ErrorHandler(usually undervendor/yiisoft/yii2/web/ErrorHandler.php) and then choose one of the following approaches:
- Remove the
Responsereturn type and keephandleExceptionasvoid, storing the generated response in a class property instead.- Create a new method (e.g.
handleExceptionWithResponse) that wraps the parent’shandleException, returnsResponse, and leaves the original signature intact.- If you control both classes and can safely add a
: Responsereturn type to the parent, update its signature to allow covariant returns.• Verify that any callers of
handleExceptionaren’t relying on avoidreturn.
• Confirm that the final behavior (HTTP code 500 + rendered response) still works under your chosen approach.src/http/StatelessApplication.php (4)
127-137: Memory management implementation looks goodThe
clean()method correctly implements memory threshold checking with garbage collection. The 90% threshold is reasonable for triggering worker recycling.
203-227: Well-structured PSR-7 request handlingThe
handle()method correctly implements the PSR RequestHandlerInterface with proper state management, event triggering, and exception handling. The implementation ensures a PSR-7 response is always returned.
394-419: Memory limit parsing correctly implementedThe method properly handles all cases including unlimited memory (-1) and various suffixes (K, M, G). The implementation is thorough and efficient with caching.
366-379: Excellent event lifecycle managementThe event tracking and cleanup implementation is well-designed for stateless operation:
- Proper reverse-order cleanup prevents dependency issues
- Global event tracking with wildcards captures all events
- Memory leak prevention for worker environments
…andling logic and remove unnecessary `ini_set` calls for cookie handling in `StatelessApplication` class.
…` for improved testing of cookie and response handling.
…ge into feature-5
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
tests/support/stub/SiteController.php (6)
12-15: Add return type for action404 method.The method should specify a return type for better type safety and to address static analysis warnings.
- public function action404() + public function action404(): never
17-25: Add return type for actionAuth method.The method should specify a return type for better type safety.
- public function actionAuth() + public function actionAuth(): array
49-56: Add return type for actionFile method.The method should specify a return type for better type safety.
- public function actionFile() + public function actionFile(): mixed
58-61: Add return type for actionGeneralException method.The method should specify a return type for better type safety.
- public function actionGeneralException() + public function actionGeneralException(): never
63-68: Add return type for actionGet method.The method should specify a return type for better type safety.
- public function actionGet() + public function actionGet(): array
94-103: Add parameter and return types for actionQuery method.The method should specify parameter and return types for better type safety.
- public function actionQuery($test) + public function actionQuery(string $test): array
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/http/Request.php(1 hunks)tests/TestCase.php(3 hunks)tests/http/StatelessApplicationTest.php(1 hunks)tests/support/FactoryHelper.php(2 hunks)tests/support/stub/SiteController.php(1 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
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.
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.
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.
src/http/Request.php (3)
Learnt from: terabytesoftw
PR: #21
File: src/adapter/ResponseAdapter.php:86-98
Timestamp: 2025-07-21T23:28:20.089Z
Learning: In Yii2, cookies with expire == 1 are treated as delete cookies and cookie validation is skipped for them. The official Yii2 Response::sendCookies() method uses if ($expire != 1 && isset($validationKey)) to determine when to apply validation hashing.
Learnt from: terabytesoftw
PR: #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.
Learnt from: terabytesoftw
PR: #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.
tests/support/FactoryHelper.php (3)
Learnt from: terabytesoftw
PR: #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.
Learnt from: terabytesoftw
PR: #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.
Learnt from: terabytesoftw
PR: #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.
tests/TestCase.php (5)
Learnt from: terabytesoftw
PR: #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.
Learnt from: terabytesoftw
PR: #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.
Learnt from: terabytesoftw
PR: #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.
Learnt from: terabytesoftw
PR: #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.
Learnt from: terabytesoftw
PR: #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.
tests/http/StatelessApplicationTest.php (5)
Learnt from: terabytesoftw
PR: #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.
Learnt from: terabytesoftw
PR: #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.
Learnt from: terabytesoftw
PR: #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.
Learnt from: terabytesoftw
PR: #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.
Learnt from: terabytesoftw
PR: #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.
tests/support/stub/SiteController.php (5)
Learnt from: terabytesoftw
PR: #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.
Learnt from: terabytesoftw
PR: #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.
Learnt from: terabytesoftw
PR: #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.
Learnt from: terabytesoftw
PR: #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.
Learnt from: terabytesoftw
PR: #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.
🧬 Code Graph Analysis (1)
tests/support/FactoryHelper.php (1)
src/creator/ServerRequestCreator.php (1)
ServerRequestCreator(49-139)
🪛 GitHub Check: mutation / PHP 8.4-ubuntu-latest
tests/support/stub/SiteController.php
[failure] 124-124:
Only booleans are allowed in an if condition, resource|false given.
[failure] 120-120:
Method yii2\extensions\psrbridge\tests\support\stub\SiteController::actionStream() has no return type specified.
[failure] 94-94:
Method yii2\extensions\psrbridge\tests\support\stub\SiteController::actionQuery() has parameter $test with no type specified.
[failure] 94-94:
Method yii2\extensions\psrbridge\tests\support\stub\SiteController::actionQuery() has no return type specified.
[failure] 63-63:
Method yii2\extensions\psrbridge\tests\support\stub\SiteController::actionGet() has no return type specified.
[failure] 58-58:
Method yii2\extensions\psrbridge\tests\support\stub\SiteController::actionGeneralException() has no return type specified.
[failure] 49-49:
Method yii2\extensions\psrbridge\tests\support\stub\SiteController::actionFile() has no return type specified.
[failure] 17-17:
Method yii2\extensions\psrbridge\tests\support\stub\SiteController::actionAuth() has no return type specified.
[failure] 12-12:
Method yii2\extensions\psrbridge\tests\support\stub\SiteController::action404() has no return type specified.
🪛 GitHub Check: phpstan / PHP 8.4-ubuntu-latest
tests/support/stub/SiteController.php
[failure] 124-124:
Only booleans are allowed in an if condition, resource|false given.
[failure] 120-120:
Method yii2\extensions\psrbridge\tests\support\stub\SiteController::actionStream() has no return type specified.
[failure] 94-94:
Method yii2\extensions\psrbridge\tests\support\stub\SiteController::actionQuery() has parameter $test with no type specified.
[failure] 94-94:
Method yii2\extensions\psrbridge\tests\support\stub\SiteController::actionQuery() has no return type specified.
[failure] 63-63:
Method yii2\extensions\psrbridge\tests\support\stub\SiteController::actionGet() has no return type specified.
[failure] 58-58:
Method yii2\extensions\psrbridge\tests\support\stub\SiteController::actionGeneralException() has no return type specified.
[failure] 49-49:
Method yii2\extensions\psrbridge\tests\support\stub\SiteController::actionFile() has no return type specified.
[failure] 17-17:
Method yii2\extensions\psrbridge\tests\support\stub\SiteController::actionAuth() has no return type specified.
[failure] 12-12:
Method yii2\extensions\psrbridge\tests\support\stub\SiteController::action404() has no return type specified.
🔇 Additional comments (7)
src/http/Request.php (1)
46-50: LGTM! Cookie validation property follows Yii2 conventions.The
$cookieValidationKeyproperty is properly typed, documented, and follows the standard Yii2 pattern for cookie validation. The integration with the PSR-7 adapter in thegetCookies()method is correctly implemented.tests/support/FactoryHelper.php (2)
28-28: LGTM! Import added for new factory method.The ServerRequestCreator import is properly added to support the new factory method.
144-161: LGTM! Factory method follows established patterns.The
createServerRequestCreator()method properly follows the established pattern in the FactoryHelper class, creating a ServerRequestCreator instance using existing factory methods. The documentation is consistent and the implementation correctly matches the constructor signature.tests/TestCase.php (3)
7-8: LGTM! Imports added for stateless application support.The new imports properly support the PSR-7 factories and StatelessApplication testing infrastructure.
Also applies to: 11-11, 13-15
97-167: LGTM! Comprehensive test fixture for StatelessApplication.The
statelessApplication()method provides a well-structured test fixture with appropriate defaults for testing the new StatelessApplication class. The configuration properly:
- Disables security features (cookies, CSRF) for testing
- Sets up logging and caching components
- Configures PSR-7 container bindings
- Provides flexible URL routing for test scenarios
- Accepts optional config for test customization
187-187: LGTM! Minor repositioning without functional change.The repositioning of the
'isConsoleRequest' => falsesetting doesn't affect functionality.tests/http/StatelessApplicationTest.php (1)
1-294: LGTM! Comprehensive test coverage for StatelessApplication.This test class provides excellent coverage of the StatelessApplication functionality with well-structured tests that:
- Cover various HTTP scenarios (cookies, POST/GET data, redirects, status codes)
- Properly set up global variables and create PSR-7 requests using FactoryHelper
- Include detailed assertions with helpful failure messages
- Use proper tearDown to clean up application state
- Follow established testing patterns from the project
The tests effectively validate the PSR-7 integration and stateless application behavior.
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a StatelessApplication implementation with PSR-7 bridge support and an error handler for modern PHP runtimes. It enables worker-based and SAPI environments to handle HTTP requests in a stateless manner while maintaining Yii2 compatibility.
Key changes:
- Introduces
StatelessApplicationclass implementingRequestHandlerInterfacefor PSR-7 request handling - Adds custom
ErrorHandlerfor enhanced exception processing with PSR-7 response generation - Extends
Requestclass with HTTP Basic authentication credential retrieval and cookie validation key support
Reviewed Changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/http/StatelessApplication.php |
Core stateless application implementation with PSR-7 request handling, memory management, and event cleanup |
src/http/ErrorHandler.php |
Custom error handler that generates PSR-7 responses for exceptions with fallback rendering |
src/http/Request.php |
Extended request class with HTTP Basic auth credential parsing and cookie validation key property |
tests/http/StatelessApplicationTest.php |
Comprehensive test suite covering stateless application functionality including session isolation and memory management |
tests/http/ErrorHandlerTest.php |
Test coverage for error handler behavior across various exception scenarios |
tests/support/stub/SiteController.php |
Test controller providing various HTTP scenarios for testing the stateless application |
tests/support/stub/Identity.php |
User identity stub for authentication testing |
tests/support/FactoryHelper.php |
Factory helper with ServerRequestCreator for PSR-7 request creation |
| Various test files | Updated constant usage and improved test coverage for file upload handling |
…ation in `SiteController` class.
There was a problem hiding this comment.
Pull Request Overview
This PR introduces the StatelessApplication and ErrorHandler implementation with comprehensive test coverage to enable PSR-7 HTTP request handling in modern PHP runtime environments. The implementation provides a worker-safe, stateless application that can handle PSR-7 requests while maintaining compatibility with Yii2's existing component system.
- Implements
StatelessApplicationwith PSR-7RequestHandlerInterfacefor stateless operation - Adds
ErrorHandlerfor PSR-7 response generation from exceptions - Enhances
Requestcomponent with HTTP Basic authentication support and cookie validation key
Reviewed Changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/http/StatelessApplication.php |
Core stateless application implementing PSR-7 RequestHandlerInterface with memory management and event tracking |
src/http/ErrorHandler.php |
PSR-7 compatible error handler for exception processing and response rendering |
src/http/Request.php |
Enhanced request component with HTTP Basic auth credential retrieval and cookie validation key |
tests/http/StatelessApplicationTest.php |
Comprehensive test suite covering stateless application functionality and request handling |
tests/http/ErrorHandlerTest.php |
Extensive tests for error handler behavior across various exception scenarios |
tests/support/stub/SiteController.php |
Test controller stub providing various HTTP scenario endpoints for testing |
tests/support/stub/Identity.php |
User identity stub supporting authentication in test scenarios |
tests/support/FactoryHelper.php |
Added factory method for PSR-7 server request creation |
| Multiple test files | Updated to use constants instead of global constants and enhanced test coverage |
… and implement session data actions in `SiteController`.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
tests/http/StatelessApplicationTest.php (3)
116-117: Fix incorrect hash key extractionThe code extracts 'hash1' from
$captchaData1but 'hash2' from$captchaData2. Both should extract the same key ('hash1') for proper comparison between users.$hash1 = $captchaData1['hash1'] ?? null; -$hash2 = $captchaData2['hash2'] ?? null; +$hash2 = $captchaData2['hash1'] ?? null;
831-835: Fix hardcoded session name in assertionsThe test hardcodes 'PHPSESSID' but the actual session name in the test environment is 'DEFAULTSESS'. Use the configured session name instead.
+ $sessionName = $app->session->getName(); self::assertSame( - "PHPSESSID={$sessionId}; Path=/; HttpOnly; SameSite", + "{$sessionName}={$sessionId}; Path=/; HttpOnly; SameSite", $response1->getHeaders()['Set-Cookie'][0] ?? '', "Response 'Set-Cookie' header should contain '{$sessionId}' for 'site/setsession' route in " . "'StatelessApplication'.", );Apply the same fix to lines 860-864.
Also applies to: 860-864
916-920: Fix hardcoded session name in session isolation testThe test hardcodes 'PHPSESSID' instead of using the actual session name from configuration.
+ $sessionName = $app->session->getName(); self::assertSame( - 'PHPSESSID=session-user-a; Path=/; HttpOnly; SameSite', + $sessionName . '=session-user-a; Path=/; HttpOnly; SameSite', $response1->getHeaders()['Set-Cookie'][0] ?? '', "Response 'Set-Cookie' header should contain 'session-user-a' for 'site/setsession' route in " . "'StatelessApplication'.", );Apply the same fix to lines 946-949.
Also applies to: 946-949
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
tests/http/StatelessApplicationTest.php(1 hunks)tests/support/stub/SiteController.php(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
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.
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.
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.
tests/support/stub/SiteController.php (5)
Learnt from: terabytesoftw
PR: #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.
Learnt from: terabytesoftw
PR: #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.
Learnt from: terabytesoftw
PR: #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.
Learnt from: terabytesoftw
PR: #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.
Learnt from: terabytesoftw
PR: #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.
tests/http/StatelessApplicationTest.php (5)
Learnt from: terabytesoftw
PR: #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.
Learnt from: terabytesoftw
PR: #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.
Learnt from: terabytesoftw
PR: #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.
Learnt from: terabytesoftw
PR: #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.
Learnt from: terabytesoftw
PR: #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.
⏰ 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.4-ubuntu-latest
- GitHub Check: mutation / PHP 8.4-ubuntu-latest
🔇 Additional comments (17)
tests/support/stub/SiteController.php (17)
1-20: LGTM! Clean imports and class structure.The file structure follows best practices with strict types, organized imports, and appropriate use of function imports for built-in PHP functions.
24-32: LGTM! Authentication method implementation is correct.The method properly extracts HTTP Basic auth credentials and returns them in a structured format for testing purposes.
37-48: LGTM! Authentication check implementation is appropriate.The method correctly checks user authentication status and identity. The ternary operator with instanceof check is acceptable for this test context.
50-70: LGTM! Cookie handling for testing is correct.The method properly adds test cookies with appropriate configurations. The explicit
httpOnly => falsesetting is appropriate for testing scenarios where JavaScript access to cookies may be needed.
75-91: LGTM! File handling implementation is robust.The method properly creates a temporary file with error checking, writes content, and sends it as a file response. The resource handling and exception throwing are appropriate.
93-98: LGTM! GET parameter handling is straightforward and correct.The method appropriately returns GET parameters for testing purposes.
110-115: LGTM! Session retrieval implementation is correct.The method properly retrieves and returns session data in a structured format for testing.
120-125: LGTM! Session data retrieval is implemented correctly.The method appropriately retrieves and structures session data for testing purposes.
130-135: LGTM! Simple index action for testing is appropriate.The method provides a basic endpoint returning structured data for testing purposes.
140-160: LGTM! Login implementation follows good authentication practices.The method properly validates credentials, handles authentication flow, and returns appropriate status responses for testing purposes.
162-167: LGTM! POST parameter handling is correct.The method appropriately returns POST parameters for testing purposes.
172-175: LGTM! Redirect implementation is appropriate for testing.The method properly demonstrates redirect functionality with appropriate exception handling annotation.
177-180: LGTM! Refresh implementation is appropriate.The method properly demonstrates refresh functionality with fragment identifier for testing purposes.
182-191: LGTM! External actions configuration is correct.The method properly configures external actions including captcha with appropriate constraints for testing purposes.
193-211: LGTM! Session management methods are implemented correctly.Both
actionSetsessionandactionSetsessiondataproperly handle session data setting with appropriate response formatting for testing purposes.
213-216: LGTM! Status code setting is straightforward and correct.The method appropriately sets HTTP status code for testing purposes.
222-236: LGTM! Stream handling implementation is robust.The method properly creates a temporary file with error checking and streams it as a file response. The resource handling and exception management are appropriate based on previous review fixes.
…ing session data.
… return cookie array in `SiteController` class.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (7)
tests/http/StatelessApplicationTest.php (7)
45-45: Fix hardcoded session name in cookie arraysThe test hardcodes 'PHPSESSID' but the actual session name in the test environment is 'DEFAULTSESS'. Use the configured session name instead.
Also applies to: 90-90, 143-143
177-181: Fix hardcoded session name in assertionThe test hardcodes 'PHPSESSID' in the assertion but should use the actual session name.
222-222: Fix hardcoded session name in worker mode testReplace hardcoded 'PHPSESSID' with the actual session name used by the application.
Also applies to: 237-237
806-806: Fix hardcoded session names throughout the testAll occurrences of 'PHPSESSID' in cookies and assertions should use the actual session name.
Also applies to: 831-834, 837-837, 860-863
892-892: Replace hardcoded 'PHPSESSID' with dynamic session nameUse the actual session name from the application configuration.
Also applies to: 917-920, 923-923, 946-949
1065-1065: Fix hardcoded session names in authentication testReplace hardcoded 'PHPSESSID' in cookie arrays with the actual session name.
Also applies to: 1111-1111
117-117: Fix inconsistent hash key extractionThe variable assignments are inconsistent -
$hash1extracts 'hash1' from$captchaData1but$hash2extracts 'hash2' from$captchaData2. This should extract the same key from both arrays for proper comparison.- $hash2 = $captchaData2['hash2'] ?? null; + $hash2 = $captchaData2['hash1'] ?? null;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
tests/TestCase.php(4 hunks)tests/http/StatelessApplicationTest.php(1 hunks)tests/support/stub/SiteController.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/TestCase.php
- tests/support/stub/SiteController.php
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
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.
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.
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.
tests/http/StatelessApplicationTest.php (5)
Learnt from: terabytesoftw
PR: #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.
Learnt from: terabytesoftw
PR: #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.
Learnt from: terabytesoftw
PR: #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.
Learnt from: terabytesoftw
PR: #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.
Learnt from: terabytesoftw
PR: #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.
🪛 GitHub Actions: build
tests/http/StatelessApplicationTest.php
[error] 176-176: PHPUnit failure: Captcha image response 'Set-Cookie' should contain 'user-a-session' but got 'DEFAULTSESS=7p79qqtpq6cusou9k8jd806dst'. Assertion of string identity failed.
[error] 259-259: PHPUnit failure: Session 'worker-session-1' did not return expected data 'user-1-data'. Assertion failed: null is not identical to expected string.
[error] 830-830: PHPUnit failure: Response 'Set-Cookie' header should contain 'test-session-688939f0bb976' but got 'DEFAULTSESS=qddqt8djbn8fdr64o2lfj38hai'. Assertion of string identity failed.
[error] 916-916: PHPUnit failure: Response 'Set-Cookie' header should contain 'session-user-a' but got 'DEFAULTSESS=ceopgqhnc03ejtcr18r7k7hbtu'. Assertion of string identity failed.
[error] 994-994: PHPUnit failure: Expected exactly one 'PHPSESSID' cookie in response but found none. Assertion on cookie count failed.
[error] 1095-1095: PHPUnit failure: Response 'Set-Cookie' header should contain 'user1-session' but got 'DEFAULTSESS=q6qtsmj1tbncnd2a21gage6ho9'. Assertion of string identity failed.
…amic session names in `Set-Cookie` headers.
…amic session names for cookie management.
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
tests/http/StatelessApplicationTest.php (1)
332-332: Fix typo in commentThe comment mentions 'PHPSESSION' but should be 'PHPSESSID' or better yet, refer to it as "the session cookie".
- // skip the last cookie header (assumed to be 'PHPSESSION'). + // skip the session cookie header
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/http/StatelessApplicationTest.php(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
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.
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.
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.
tests/http/StatelessApplicationTest.php (6)
Learnt from: terabytesoftw
PR: #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.
Learnt from: terabytesoftw
PR: #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.
Learnt from: terabytesoftw
PR: #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.
Learnt from: terabytesoftw
PR: #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.
Learnt from: terabytesoftw
PR: #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.
Learnt from: terabytesoftw
PR: #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.
🪛 GitHub Actions: build
tests/http/StatelessApplicationTest.php
[error] 180-180: PHPUnit failure: Captcha image response 'Set-Cookie' should contain 'user-a-session'. Assertion failed: expected 'DEFAULTSESS=user-a-session; Path=/; HttpOnly; SameSite' but got 'DEFAULTSESS=7ns80ror73oeovlem77ro7177j; Path=/; HttpOnly; SameSite'.
[error] 263-263: PHPUnit failure: Session 'worker-session-1' should return its own data ('user-1-data') without leaking between sessions. Assertion failed: expected 'user-1-data' but got null.
[error] 867-867: PHPUnit failure: Response 'Set-Cookie' header should contain 'test-session-688942dc99097' for 'site/getsession' route. Assertion failed: expected 'DEFAULTSESS=test-session-688942dc99097; Path=/; HttpOnly; SameSite' but got 'DEFAULTSESS=j3do5vnn3erm4s14pvb1fmp56h; Path=/; HttpOnly; SameSite'.
[error] 955-955: PHPUnit failure: Response 'Set-Cookie' header should contain 'session-user-b' for 'site/getsession' route. Assertion failed: expected 'DEFAULTSESS=session-user-b; Path=/; HttpOnly; SameSite' but got 'DEFAULTSESS=uqlgmpt8v8blrc5stfeqt7ci21; Path=/; HttpOnly; SameSite'.
[error] 1140-1140: PHPUnit failure: Response 'Set-Cookie' header should contain 'user2-session' for 'site/checkauth' route. Assertion failed: expected 'DEFAULTSESS=user2-session; Path=/; HttpOnly; SameSite' but got an empty string.
…amic session names for cookie management.
… `StatelessApplicationTest` class.
…pplicationTest` class.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
tests/http/StatelessApplicationTest.php (1)
119-120: Fix inconsistent hash extractionThe hash extraction is inconsistent - extracting 'hash1' from the first response but 'hash2' from the second response. Both should extract the same key for proper comparison.
$hash1 = $captchaData1['hash1'] ?? null; - $hash2 = $captchaData2['hash2'] ?? null; + $hash2 = $captchaData2['hash1'] ?? null;
🧹 Nitpick comments (1)
tests/http/StatelessApplicationTest.php (1)
331-335: Fix comment indentationThe pipeline detected an indentation issue with the comment. The comment should align with the code block.
$cookies = $response->getHeaders()['set-cookie'] ?? []; foreach ($cookies as $cookie) { - // skip the session cookie header + // skip the session cookie header if (str_starts_with($cookie, $app->session->getName()) === false) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/http/StatelessApplicationTest.php(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
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.
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.
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.
tests/http/StatelessApplicationTest.php (6)
Learnt from: terabytesoftw
PR: #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.
Learnt from: terabytesoftw
PR: #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.
Learnt from: terabytesoftw
PR: #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.
Learnt from: terabytesoftw
PR: #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.
Learnt from: terabytesoftw
PR: #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.
Learnt from: terabytesoftw
PR: #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.
🪛 GitHub Actions: ecs
tests/http/StatelessApplicationTest.php
[warning] 331-331: PhpCsFixer whitespace fixer detected an indentation issue: line comment indentation differs. Run 'vendor/bin/ecs check --fix' to apply the fix.
⏰ 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). (3)
- GitHub Check: mutation / PHP 8.4-ubuntu-latest
- GitHub Check: phpunit / PHP 8.4-windows-latest
- GitHub Check: mutation / PHP 8.4-ubuntu-latest
…hod of `StatelessApplicationTest` class.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
…onSetflash` and `actionGetflash` methods.
Summary by CodeRabbit
New Features
Chores