refactor(Request): Implement getServerName() method with PSR-7 support and fallback to Yii2, including comprehensive tests.
#71
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ort and fallback to Yii2, including comprehensive tests.
WalkthroughA new Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Request
participant PSR7Adapter
participant ParentRequest
Client->>Request: getServerName()
alt PSR-7 adapter is set
Request->>PSR7Adapter: getServerParam('SERVER_NAME')
alt SERVER_NAME is valid string
PSR7Adapter-->>Request: server name (string)
Request-->>Client: server name
else SERVER_NAME missing/invalid
PSR7Adapter-->>Request: null
Request-->>Client: null
end
else No PSR-7 adapter
Request->>ParentRequest: getServerName()
ParentRequest-->>Request: server name or null
Request-->>Client: server name or null
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~18 minutes Possibly related PRs
Suggested labels
Poem
Note 🔌 MCP (Model Context Protocol) integration is now available in Early Access!Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (12)📓 Common learnings📚 Learning: 2025-07-20T16:35:15.341ZApplied to files:
📚 Learning: 2025-07-20T16:33:57.495ZApplied to files:
📚 Learning: 2025-07-20T16:35:15.341ZApplied to files:
📚 Learning: 2025-08-06T22:52:05.608ZApplied to files:
📚 Learning: 2025-08-08T15:28:00.137ZApplied to files:
📚 Learning: 2025-08-08T15:28:00.137ZApplied to files:
📚 Learning: 2025-08-08T15:24:06.045ZApplied to files:
📚 Learning: 2025-08-03T16:24:09.241ZApplied to files:
📚 Learning: 2025-08-03T16:24:09.241ZApplied to files:
📚 Learning: 2025-07-22T00:50:26.546ZApplied to files:
📚 Learning: 2025-07-22T01:01:13.426ZApplied to files:
🧬 Code Graph Analysis (1)tests/http/RequestTest.php (4)
⏰ 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)
✨ 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 #71 +/- ##
===========================================
Coverage 100.00% 100.00%
- Complexity 307 309 +2
===========================================
Files 12 12
Lines 780 784 +4
===========================================
+ Hits 780 784 +4 ☔ 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: 3
🧹 Nitpick comments (6)
tests/adapter/ServerRequestAdapterTest.php (4)
1018-1031: Test name vs. expectation mismatchMethod name says “ReturnNull…” but the assertion expects an empty string. Rename for clarity.
- public function testReturnNullFromHeaderWhenCsrfHeaderEmptyAndAdapterIsSet(): void + public function testReturnEmptyStringFromHeaderWhenCsrfHeaderPresentButEmpty(): void
1048-1106: Server name test coverage is thoroughGood coverage for:
- Present string
- Missing
- Null
- Non-string
- Empty array
- Reset behavior
- Independent instances
Optional: consolidate the “null-ish / invalid types” cases into a data provider to reduce duplication.
Also applies to: 1522-1536, 2122-2197
590-608: Direct $_SERVER manipulation in tests — OK given base TestCase cleanupPer project learnings, TestCase setUp/tearDown handles $_SERVER snapshot/restore, so this is safe. Consider a brief inline comment referencing that to preempt future cleanup code additions.
Also applies to: 1482-1499, 2043-2059
1630-1637: filesize() assertions — pragmatic but consider robustnessAssertions guard against false, good. If flakiness ever appears across environments, consider virtual FS (e.g., vfsStream) to avoid relying on real paths. Not required now.
Also applies to: 804-811
tests/http/RequestTest.php (2)
624-627: Strengthen the assertion – check the actual value, not just non-null
assertNotNull()only proves that some token is returned; it doesn’t guarantee you are falling back to the expected header value (parent-csrf-token-456).
UsingassertSame('parent-csrf-token-456', …)would catch regressions where an unexpected default is returned.
1597-1609: Movereset()after you prepare$_SERVER
reset()clears internal caches; calling it before you populate$_SERVERrisks the object still carrying a stale script URL if anything is cached between the constructor and this test.
Safer sequence:- $request->reset(); - - $_SERVER['SCRIPT_NAME'] = '/test.php'; - $_SERVER['SCRIPT_FILENAME'] = '/path/to/test.php'; + $_SERVER['SCRIPT_NAME'] = '/test.php'; + $_SERVER['SCRIPT_FILENAME'] = '/path/to/test.php'; + + $request->reset();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/http/Request.php(1 hunks)tests/adapter/ServerRequestAdapterTest.php(68 hunks)tests/http/RequestTest.php(50 hunks)
🧰 Additional context used
🧠 Learnings (9)
📓 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#64
File: tests/http/StatelessApplicationTest.php:1939-1967
Timestamp: 2025-08-06T22:52:05.608Z
Learning: In yii2-extensions/psr-bridge tests, when testing specific component methods like Request::resolve(), it's necessary to call $app->handle($request) first to initialize all application components before testing the method in isolation. This ensures proper component lifecycle initialization.
📚 Learning: 2025-07-20T16:33:57.495Z
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/adapter/ServerRequestAdapterTest.phptests/http/RequestTest.php
📚 Learning: 2025-07-20T16:35:15.341Z
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/adapter/ServerRequestAdapterTest.phptests/http/RequestTest.php
📚 Learning: 2025-07-20T16:35:15.341Z
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/adapter/ServerRequestAdapterTest.phptests/http/RequestTest.php
📚 Learning: 2025-08-06T22:52:05.608Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#64
File: tests/http/StatelessApplicationTest.php:1939-1967
Timestamp: 2025-08-06T22:52:05.608Z
Learning: In yii2-extensions/psr-bridge tests, when testing specific component methods like Request::resolve(), it's necessary to call $app->handle($request) first to initialize all application components before testing the method in isolation. This ensures proper component lifecycle initialization.
Applied to files:
tests/adapter/ServerRequestAdapterTest.phptests/http/RequestTest.php
📚 Learning: 2025-07-22T01:01:13.426Z
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/adapter/ServerRequestAdapterTest.phptests/http/RequestTest.php
📚 Learning: 2025-07-21T23:28:20.089Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#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.
Applied to files:
tests/adapter/ServerRequestAdapterTest.php
📚 Learning: 2025-07-22T00:50:26.546Z
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:
tests/adapter/ServerRequestAdapterTest.phptests/http/RequestTest.php
📚 Learning: 2025-07-22T10:56:44.424Z
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:
tests/adapter/ServerRequestAdapterTest.php
🪛 Gitleaks (8.27.2)
tests/adapter/ServerRequestAdapterTest.php
2215-2215: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ 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: phpunit / PHP 8.3-windows-latest
- GitHub Check: phpunit / PHP 8.4-windows-latest
- GitHub Check: mutation / PHP 8.3-ubuntu-latest
- GitHub Check: mutation / PHP 8.3-ubuntu-latest
🔇 Additional comments (11)
src/http/Request.php (1)
527-551: Add getServerName() with PSR-7 support — looks correct
- Uses getServerParam('SERVER_NAME') ensuring type-safety (string|null).
- Falls back cleanly to parent implementation when adapter is null.
- Docblock matches behavior and return type.
No changes requested.
tests/adapter/ServerRequestAdapterTest.php (10)
21-23: Import additions are appropriateAdding is_array and stream_get_meta_data imports is warranted and used below.
100-124: Cookie collection after reset — solid behavior assertionsConfirms a new CookieCollection instance is returned and old state doesn’t leak post-reset. Good.
2201-2232: Read-only CookieCollection mutation test is correctExpecting InvalidCallException on add() against read-only collection is appropriate.
2234-2250: InvalidConfigException when validation enabled but key empty — correctThis enforces configuration correctness early. Good guardrail.
523-525: Invalid cookies with validation enabled — behavior confirmedAsserting an empty collection when cookies fail validation is consistent with the adapter contract.
2082-2120: Secure headers filtering logic — strong negative testProperly filters X-Forwarded-* and related headers when request isn’t from a trusted host, while preserving normal headers. Good assertions.
567-571: Script URL tests are precise
- Empty in worker mode or traditional mode without SCRIPT_NAME.
- Uses SCRIPT_NAME when traditional mode and present.
Matches adapter behavior; messages are clear.
Also applies to: 584-587, 1512-1518
1900-1903: General adapter behavior tests — LGTMQuery params/string, raw body, remote host/IP, and uploaded files conversions are well-covered and assertions are crisp.
Also applies to: 1404-1407, 1358-1389, 1425-1429, 1440-1443, 1454-1458, 1473-1477, 1761-1761, 1846-1847
931-969: Multiple validated cookies — comprehensiveValidates multiple signatures and ensures decrypted values and names match. Solid coverage.
Also applies to: 953-990
1936-1956: Validated cookie(s) happy path — correctEnsures only valid signed cookies pass, name/value integrity preserved, and expire null as expected.
Also applies to: 1961-2015
…t in `TestCase` class and update related tests.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/TestCase.php (1)
193-193: Good centralization of cookieValidationKeyReplacing per-test literals with a single source in TestCase improves consistency and reduces drift across the suite.
Consider adding a brief docblock on the constant noting it’s test-only and intentionally non-secret to document intent (and help future readers/scanners).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
tests/TestCase.php(2 hunks)tests/adapter/ResponseAdapterTest.php(1 hunks)tests/http/RequestTest.php(50 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/http/RequestTest.php
🧰 Additional context used
🧠 Learnings (7)
📓 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#64
File: tests/http/StatelessApplicationTest.php:1939-1967
Timestamp: 2025-08-06T22:52:05.608Z
Learning: In yii2-extensions/psr-bridge tests, when testing specific component methods like Request::resolve(), it's necessary to call $app->handle($request) first to initialize all application components before testing the method in isolation. This ensures proper component lifecycle initialization.
📚 Learning: 2025-07-22T00:50:26.546Z
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:
tests/adapter/ResponseAdapterTest.php
📚 Learning: 2025-07-21T23:28:20.089Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#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.
Applied to files:
tests/adapter/ResponseAdapterTest.php
📚 Learning: 2025-07-22T01:01:13.426Z
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/adapter/ResponseAdapterTest.phptests/TestCase.php
📚 Learning: 2025-07-20T16:33:57.495Z
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.php
📚 Learning: 2025-07-20T16:35:15.341Z
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/TestCase.php
📚 Learning: 2025-07-20T16:35:15.341Z
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/TestCase.php
🪛 Gitleaks (8.27.2)
tests/TestCase.php
26-26: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ 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: phpunit / PHP 8.4-windows-latest
- GitHub Check: phpunit / PHP 8.1-windows-latest
- GitHub Check: mutation / PHP 8.3-ubuntu-latest
- GitHub Check: mutation / PHP 8.3-ubuntu-latest
🔇 Additional comments (1)
tests/adapter/ResponseAdapterTest.php (1)
909-909: LGTM: use inherited test key via self::COOKIE_VALIDATION_KEYClean replacement of hardcoded literal with the shared constant. Visibility is protected in the parent TestCase, so inheritance here is valid.
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (2)
tests/http/RequestTest.php (2)
77-103: CSRF “unsafe” / “safe” method lists are reversed
GETandHEADare safe methods;DELETE,PATCH,PUTare unsafe.
The test currently expects the opposite, so it will fail against the real implementation (and mask regressions).-foreach (['GET', 'HEAD', 'POST'] as $method) { // unsafe +foreach (['POST', 'PUT', 'PATCH', 'DELETE'] as $method) { // unsafe ... -foreach (['DELETE', 'PATCH', 'PUT', 'OPTIONS'] as $method) { // safe +foreach (['GET', 'HEAD', 'OPTIONS'] as $method) { // safe
924-932: Set$_SERVER['SERVER_NAME']before instantiatingRequest
Requestmay snapshot super-globals in its constructor.
Instantiate the object only after populating$_SERVERto avoid false-positives.- $request = new Request(); - - $_SERVER['SERVER_NAME'] = 'servername'; + $_SERVER['SERVER_NAME'] = 'servername'; + $request = new Request();
♻️ Duplicate comments (3)
tests/http/RequestTest.php (2)
1733-1741: Same ordering issue forgetScriptUrl()Clear
$_SERVERprior tonew Request(); otherwiseSCRIPT_NAMEfrom the runtime environment prevents the expected exception.- $request = new Request(); - + $_SERVER = []; + $request = new Request();
1723-1731:getScriptFile()exception test can pass even when it shouldn’tThe environment already contains
SCRIPT_FILENAMEwhen PHPUnit runs, so the call will not throw.
Blank$_SERVERbefore creatingRequest:- $request = new Request(); - + $_SERVER = []; + $request = new Request();tests/adapter/ServerRequestAdapterTest.php (1)
95-97: Centralise the dummycookieValidationKeyconstantThe literal string
'test-validation-key-32-characters'is repeated throughout the test suite. A shared constant already exists inTestCase(see previous PR discussion); use it instead to:• eliminate duplication
• silence generic-API-key scanners (Gitleaks) with a single allow-list entry
• simplify future changes to the valueExample diff:
- $request->cookieValidationKey = 'test-validation-key-32-characters'; + $request->cookieValidationKey = self::COOKIE_VALIDATION_KEY;Apply the same replacement in every occurrence listed above (and any future tests).
Also applies to: 305-307, 378-380, 517-518, 940-964, 1912-1933, 2214-2216
🧹 Nitpick comments (2)
tests/adapter/ServerRequestAdapterTest.php (2)
21-22: Avoid unnecessaryuse functionimport for built-ins
is_array()is a core PHP function and does not need to be imported explicitly. Removing the import keeps the header cleaner without changing behaviour.
95-97: Remove redundant key assignment when validation is disabledIn tests where
$request->enableCookieValidation = false, settingcookieValidationKeyis unnecessary noise and can be removed after the constant refactor.Also applies to: 305-307, 378-380
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
tests/adapter/ServerRequestAdapterTest.php(67 hunks)tests/http/RequestTest.php(50 hunks)
🧰 Additional context used
🧠 Learnings (11)
📓 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#64
File: tests/http/StatelessApplicationTest.php:1939-1967
Timestamp: 2025-08-06T22:52:05.608Z
Learning: In yii2-extensions/psr-bridge tests, when testing specific component methods like Request::resolve(), it's necessary to call $app->handle($request) first to initialize all application components before testing the method in isolation. This ensures proper component lifecycle initialization.
📚 Learning: 2025-07-20T16:35:15.341Z
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/RequestTest.phptests/adapter/ServerRequestAdapterTest.php
📚 Learning: 2025-07-20T16:33:57.495Z
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/RequestTest.phptests/adapter/ServerRequestAdapterTest.php
📚 Learning: 2025-07-20T16:35:15.341Z
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/RequestTest.phptests/adapter/ServerRequestAdapterTest.php
📚 Learning: 2025-08-06T22:52:05.608Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#64
File: tests/http/StatelessApplicationTest.php:1939-1967
Timestamp: 2025-08-06T22:52:05.608Z
Learning: In yii2-extensions/psr-bridge tests, when testing specific component methods like Request::resolve(), it's necessary to call $app->handle($request) first to initialize all application components before testing the method in isolation. This ensures proper component lifecycle initialization.
Applied to files:
tests/http/RequestTest.phptests/adapter/ServerRequestAdapterTest.php
📚 Learning: 2025-08-03T16:24:09.241Z
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/RequestTest.php
📚 Learning: 2025-08-03T16:24:09.241Z
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/RequestTest.php
📚 Learning: 2025-07-22T00:50:26.546Z
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:
tests/http/RequestTest.phptests/adapter/ServerRequestAdapterTest.php
📚 Learning: 2025-07-22T01:01:13.426Z
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/RequestTest.phptests/adapter/ServerRequestAdapterTest.php
📚 Learning: 2025-07-21T23:28:20.089Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#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.
Applied to files:
tests/adapter/ServerRequestAdapterTest.php
📚 Learning: 2025-07-22T10:56:44.424Z
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:
tests/adapter/ServerRequestAdapterTest.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). (3)
- GitHub Check: phpunit / PHP 8.3-windows-latest
- GitHub Check: mutation / PHP 8.3-ubuntu-latest
- GitHub Check: mutation / PHP 8.3-ubuntu-latest
This was referenced Aug 9, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit