Introduce UploadedFile class bridge implementation with tests.
#160
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
WalkthroughAdds a PSR‑7-aware UploadedFile bridge class and wiring, refines PHPStan annotations, moves uploaded-file reset into StatelessApplication::reset(), updates Request to propagate the PSR‑7 adapter and attach stream resources to created UploadedFile instances, and adds tests plus test-helper stream support. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant App as Yii App
participant Req as Request
participant Adp as ServerRequestAdapter
participant UF as UploadedFile (bridge)
participant Ctl as Controller
Client->>App: POST multipart/form-data
App->>Req: bootstrap request
Req->>Adp: setPsr7Request(psr7Req)
Req->>UF: UploadedFile::setPsr7Adapter(Adp)
Note over Req,UF: adapter bound for this request
Ctl->>UF: getInstancesByName("files")
alt Adapter present
UF->>Adp: getUploadedFiles()
Adp-->>UF: PSR‑7 UploadedFile tree
UF->>UF: convertPsr7FileToLegacyFormat(..., tempResource)
else Legacy SAPI
UF->>UF: normalize `$_FILES` recursively
end
UF-->>Ctl: array of \yii\web\UploadedFile
Ctl-->>Client: Response
sequenceDiagram
autonumber
participant T as Test Runner
participant App as Stateless Yii App
participant UF as UploadedFile (bridge)
T->>App: Request #1 (upload A)
App->>UF: getInstancesByName("upload")
UF-->>App: [A]
T->>App: Request #2 (no files)
App->>UF: reset() called during lifecycle
App->>UF: getInstancesByName("upload")
UF-->>App: []
T->>App: Request #3 (upload B)
App->>UF: getInstancesByName("upload")
UF-->>App: [B]
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type 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 #160 +/- ##
============================================
Coverage 100.00% 100.00%
- Complexity 317 362 +45
============================================
Files 12 13 +1
Lines 801 910 +109
============================================
+ Hits 801 910 +109 ☔ 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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/http/Request.php (3)
686-687: Fix phpstan return type for getUploadedFiles().Current generic is malformed. Align with defensive typing per project guidance.
- * @phpstan-return array<array<array-key, UploadedFile>, mixed> + * @phpstan-return array<array-key, mixed>
815-818: Fix phpstan return type for convertPsr7ToUploadedFiles().Malformed generic; also prefer defensive typing as input is untrusted.
- * @phpstan-return array<array<array-key, UploadedFile>, mixed> + * @phpstan-return array<array-key, mixed>
847-858: Unknown property + potential FD leak: passing tempResource and detaching the stream.
- new UploadedFile([... 'tempResource' => ...]) will set an unknown property unless the subclass declares it.
- Detaching the PSR-7 stream transfers ownership; without explicit close, FDs can leak across requests.
Two safe options (pick one):
Option A (simplest): stop passing tempResource here.
return new UploadedFile( [ 'error' => $psrFile->getError(), 'name' => $psrFile->getClientFilename() ?? '', 'size' => $psrFile->getSize(), 'tempName' => $psrFile->getStream()->getMetadata('uri') ?? '', 'type' => $psrFile->getClientMediaType() ?? '', - 'tempResource' => $psrFile->getStream()->detach(), ], );Option B (keep tempResource): declare property + ensure lifecycle cleanup in UploadedFile (see comments in src/http/UploadedFile.php below).
🧹 Nitpick comments (6)
src/adapter/ServerRequestAdapter.php (1)
314-317: Tighten phpstan type for getUploadedFiles()Use a stricter return type to reflect PSR-7 shape and aid static analysis.
- * @phpstan-return array<array-key, mixed> + * @phpstan-return array<string, UploadedFileInterface|mixed[]>tests/http/stateless/StatelessApplicationUploadedTest.php (3)
22-26: Fix assertion to match message (or adjust message).Currently it only checks type, but the message states “greater than zero.”
- self::assertIsInt( - $size1, - 'Temporary file should have a valid size greater than zero.', - ); + self::assertIsInt($size1); + self::assertGreaterThanOrEqual( + 0, + $size1, + 'Temporary file should have a valid size (>= 0).', + );
70-84: Correct test message: this request is GET, not POST.Message should not say POST.
- 'Expected PSR-7 Response body to be empty for POST request with no uploaded files.', + 'Expected PSR-7 Response body to be empty for GET request with no uploaded files.',
91-111: Nit: sequential variable naming improves readability.Use $tmpFile2/$tmpPath2/$size2 and a matching filename like test2.txt.
- $tmpFile3 = $this->createTmpFile(); + $tmpFile2 = $this->createTmpFile(); - $tmpPath3 = stream_get_meta_data($tmpFile3)['uri']; - $size3 = filesize($tmpPath3); + $tmpPath2 = stream_get_meta_data($tmpFile2)['uri']; + $size2 = filesize($tmpPath2); ... - 'file2' => FactoryHelper::createUploadedFile( - 'test3.txt', + 'file2' => FactoryHelper::createUploadedFile( + 'test2.txt', 'text/plain', - $tmpPath3, - size: $size3, + $tmpPath2, + size: $size2,src/http/UploadedFile.php (2)
57-59: Remove unnecessary phpstan ignore.CI indicates no such error; drop the suppression.
- * @phpstan-ignore property.phpDocType
260-274: Optionally avoid detach if not strictly needed.If you keep detaching, the above resource-closing changes are required. Alternatively, skip detaching and drop tempResource entirely from the cache structure.
- $stream = $psr7File->getStream(); - $uri = $stream->getMetadata('uri'); + $stream = $psr7File->getStream(); + $uri = $stream->getMetadata('uri'); @@ - 'tempResource' => $stream->detach(), + // 'tempResource' can be omitted if not needed: + // 'tempResource' => $stream->detach(),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
src/adapter/ServerRequestAdapter.php(1 hunks)src/http/Request.php(3 hunks)src/http/UploadedFile.php(1 hunks)tests/http/stateless/StatelessApplicationUploadedTest.php(1 hunks)
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#78
File: src/http/Request.php:0-0
Timestamp: 2025-08-10T13:17:04.265Z
Learning: In the yii2-extensions/psr-bridge project, PHPStan type annotations should be defensive when dealing with external input from PSR-7 sources. For methods like `getUploadedFiles()` and `convertPsr7ToUploadedFiles()`, use `mixed` in the return type annotation to account for untrusted input values, even if the implementation filters them.
📚 Learning: 2025-08-10T13:17:04.265Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#78
File: src/http/Request.php:0-0
Timestamp: 2025-08-10T13:17:04.265Z
Learning: In the yii2-extensions/psr-bridge project, PHPStan type annotations should be defensive when dealing with external input from PSR-7 sources. For methods like `getUploadedFiles()` and `convertPsr7ToUploadedFiles()`, use `mixed` in the return type annotation to account for untrusted input values, even if the implementation filters them.
Applied to files:
src/adapter/ServerRequestAdapter.phpsrc/http/Request.phpsrc/http/UploadedFile.php
📚 Learning: 2025-08-10T20:39:09.333Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#82
File: tests/adapter/UploadedFilesPsr7Test.php:197-248
Timestamp: 2025-08-10T20:39:09.333Z
Learning: In the yii2-extensions/psr-bridge project, the TestCase base class (tests/TestCase.php) automatically handles temporary file cleanup. The `createTmpFile()` method tracks files in `$this->tmpFiles[]`, and the `tearDown()` method calls `closeTmpFile()` to clean them up. Tests extending TestCase don't need manual file cleanup when using these methods.
Applied to files:
tests/http/stateless/StatelessApplicationUploadedTest.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/stateless/StatelessApplicationUploadedTest.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/stateless/StatelessApplicationUploadedTest.php
📚 Learning: 2025-08-24T11:52:50.563Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#141
File: tests/http/stateless/ApplicationRoutingTest.php:1-164
Timestamp: 2025-08-24T11:52:50.563Z
Learning: In yii2-extensions/psr-bridge, tests that manipulate PHP superglobals ($_POST, $_GET, $_SERVER) in the http group do not require process isolation and work fine with the current PHPUnit configuration.
Applied to files:
tests/http/stateless/StatelessApplicationUploadedTest.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/stateless/StatelessApplicationUploadedTest.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:
src/http/Request.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:
src/http/Request.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:
src/http/Request.php
🧬 Code graph analysis (3)
tests/http/stateless/StatelessApplicationUploadedTest.php (4)
src/http/UploadedFile.php (2)
UploadedFile(39-454)getInstancesByName(174-191)tests/support/FactoryHelper.php (2)
FactoryHelper(46-268)createRequest(68-89)tests/TestCase.php (3)
TestCase(23-227)createTmpFile(101-112)statelessApplication(134-197)src/http/StatelessApplication.php (1)
handle(269-295)
src/http/Request.php (1)
src/http/UploadedFile.php (2)
UploadedFile(39-454)setPsr7Adapter(230-235)
src/http/UploadedFile.php (2)
src/adapter/ServerRequestAdapter.php (2)
ServerRequestAdapter(47-441)getUploadedFiles(323-326)src/http/Request.php (1)
getUploadedFiles(693-700)
🪛 GitHub Check: mutation / PHP 8.3-ubuntu-latest
src/http/UploadedFile.php
[failure] 59-59:
No error with identifier property.phpDocType is reported on line 59.
🪛 GitHub Check: phpstan / PHP 8.4-ubuntu-latest
src/http/UploadedFile.php
[failure] 59-59:
No error with identifier property.phpDocType is reported on line 59.
🔇 Additional comments (5)
tests/http/stateless/StatelessApplicationUploadedTest.php (1)
132-139: LGTM: verifies per-request isolation.Asserts for presence/absence are correct and validate the bridge behavior.
src/http/Request.php (2)
791-798: Good: bridge wiring ensures per-request isolation.Calling UploadedFile::setPsr7Adapter($this->adapter) on each PSR-7 request setup is correct.
693-700: Delegate to parent for BC or document change
Returning an empty array when no PSR-7 adapter is set alters legacy behavior (formerly pulled from $_FILES). Either:
- call
parent::getUploadedFiles()instead of[]- or clearly document that
getUploadedFiles()now only works with an adaptersrc/http/UploadedFile.php (2)
300-313: LGTM: lazy, single-load cache with PSR-7/legacy branching.Initialization and branching behavior look correct.
315-365: Consistency with legacy: tempResource in cache may reach constructor.Because getInstance[s] pass the entire cached entry to the constructor, having tempResource requires the declared property (above) to avoid exceptions. With that in place plus closeResources(), this is safe.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (3)
src/http/UploadedFile.php (3)
113-118: Prevent UnknownPropertyException when constructing with tempResource.Instances are created via
new self($files[$name])using arrays that includetempResource; without declaring it, BaseObject will throw. Add the property.final class UploadedFile extends \yii\web\UploadedFile { @@ - public static $_files = []; + public static $_files = []; + + /** + * Resource handle for the detached PSR-7 stream (if any). + * + * @var resource|null + */ + public $tempResource = null;Also applies to: 176-189
203-208: Close detached resources during reset() to prevent leaks.Detached streams are never closed; close them before clearing caches.
public static function reset(): void { + self::closeResources(); self::$_files = []; self::$psr7Adapter = null; self::$psr7FilesLoaded = false; } + + /** + * Closes any open tempResource handles in the cache. + */ + private static function closeResources(): void + { + foreach (self::$_files as $k => $info) { + $h = $info['tempResource'] ?? null; + if (is_resource($h)) { + @fclose($h); + self::$_files[$k]['tempResource'] = null; + } + } + }
228-233: Also close resources when swapping adapters.Avoid leaking descriptors across requests.
public static function setPsr7Adapter(ServerRequestAdapter $adapter): void { + self::closeResources(); self::$_files = []; self::$psr7Adapter = $adapter; self::$psr7FilesLoaded = false; }
🧹 Nitpick comments (1)
src/http/UploadedFile.php (1)
41-58: Make $_files private to protect invariants.Prevent external mutation of cache.
- public static $_files = []; + private static array $_files = [];
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/http/UploadedFile.php(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#78
File: src/http/Request.php:0-0
Timestamp: 2025-08-10T13:17:04.265Z
Learning: In the yii2-extensions/psr-bridge project, PHPStan type annotations should be defensive when dealing with external input from PSR-7 sources. For methods like `getUploadedFiles()` and `convertPsr7ToUploadedFiles()`, use `mixed` in the return type annotation to account for untrusted input values, even if the implementation filters them.
📚 Learning: 2025-08-10T13:17:04.265Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#78
File: src/http/Request.php:0-0
Timestamp: 2025-08-10T13:17:04.265Z
Learning: In the yii2-extensions/psr-bridge project, PHPStan type annotations should be defensive when dealing with external input from PSR-7 sources. For methods like `getUploadedFiles()` and `convertPsr7ToUploadedFiles()`, use `mixed` in the return type annotation to account for untrusted input values, even if the implementation filters them.
Applied to files:
src/http/UploadedFile.php
🧬 Code graph analysis (1)
src/http/UploadedFile.php (2)
src/adapter/ServerRequestAdapter.php (2)
ServerRequestAdapter(47-441)getUploadedFiles(323-326)src/http/Request.php (1)
getUploadedFiles(693-700)
🪛 GitHub Check: mutation / PHP 8.3-ubuntu-latest
src/http/UploadedFile.php
[warning] 188-188:
Escaped Mutant for Mutator "ArrayOneItem":
@@ @@
$results[] = new self($file);
}
}
-
return $results;
-
}
return count($results) > 1 ? array_slice($results, 0, 1, true) : $results;
/**
* Resets the internal uploaded files cache and PSR-7 adapter state.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/http/StatelessApplication.php (2)
342-345: Harden ini_get() return typing to avoid TypeError.
ini_get()may return false; method is declared to return string. Cast or fallback to a safe default.Apply:
- protected function getSystemMemoryLimit(): string - { - return ini_get('memory_limit'); - } + protected function getSystemMemoryLimit(): string + { + $value = ini_get('memory_limit'); + // Fallback to '-1' (unlimited) if option lookup fails. + return is_string($value) ? $value : '-1'; + }
483-496: Cleanup detaches all sender handlers indiscriminately; risks nuking third‑party/core listeners.
$event->sender->off($event->name)removes every handler for that event on that sender, not only ones attached in this request. This can break subsequent requests/components. Prefer detaching only what you attached (the global tracker) and, if required, clear global static handlers via a supported API.Apply:
private function cleanupEvents(): void { Event::off('*', '*', $this->eventHandler); - - foreach (array_reverse($this->registeredEvents) as $event) { - if ($event->sender !== null && method_exists($event->sender, 'off')) { - $event->sender->off($event->name); - } - } - $this->registeredEvents = []; - - Event::offAll(); + // Clear global static handlers if the framework provides a dedicated API. + if (is_callable([Event::class, 'offAll'])) { + Event::offAll(); + } }If you need per-sender cleanup, record concrete handlers at attach time and call
$sender->off($name, $handler)precisely. I can sketch a lightweight registry if helpful.
🧹 Nitpick comments (1)
src/http/StatelessApplication.php (1)
456-466: Optional: Make bridge intent explicit for UploadedFile reset.Current call resolves to the namespaced bridge class; consider FQN for clarity to avoid future import regressions.
- UploadedFile::reset(); + \yii2\extensions\psrbridge\http\UploadedFile::reset();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/http/StatelessApplication.php(1 hunks)src/http/UploadedFile.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/http/UploadedFile.php
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#78
File: src/http/Request.php:0-0
Timestamp: 2025-08-10T13:17:04.265Z
Learning: In the yii2-extensions/psr-bridge project, PHPStan type annotations should be defensive when dealing with external input from PSR-7 sources. For methods like `getUploadedFiles()` and `convertPsr7ToUploadedFiles()`, use `mixed` in the return type annotation to account for untrusted input values, even if the implementation filters them.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: mutation / PHP 8.3-ubuntu-latest
- GitHub Check: mutation / PHP 8.3-ubuntu-latest
🔇 Additional comments (3)
src/http/StatelessApplication.php (3)
12-12: Import cleanup correctly switches resolution of UploadedFile to the bridge.Keeping only
use yii\web\Application;ensuresUploadedFile::reset()resolves toyii2\extensions\psrbridge\http\UploadedFilein this namespace. Good change.
406-409: Re-entering parent constructor at reset: confirm no double-registration or leaks.Calling
parent::__construct($this->config)per request is intentional here; ensure no global state (handlers, singletons) is duplicated. This ties back to the event cleanup concerns.If you want, I can provide a script to scan for duplicate singleton registrations across requests in tests.
548-551: Verify wildcard Event APIs are supported in your Yii version
Yii 2.x does not includeEvent::on('*','*',…)orEvent::off('*','*',…)by default—confirm your framework (or a polyfill) supports true wildcard binding. If not, replace with explicit listener registrations or add a small dispatcher that proxies wildcard handlers.
… finalization documentation.
…ion and legacy file loading.
…ge into feature-9
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
tests/http/UploadedFileTest.php (1)
77-99: Avoid truncating temp files when creating streams.
FactoryHelper::createStream($tmpPathFileX)opens the path in 'wb+' by default, which truncates the file. Use read mode.- FactoryHelper::createStream($tmpPathFile1), + FactoryHelper::createStream($tmpPathFile1, 'rb'), ... - FactoryHelper::createStream($tmpPathFile2), + FactoryHelper::createStream($tmpPathFile2, 'rb'),tests/http/stateless/StatelessApplicationUploadedTest.php (2)
111-115: Assertion message doesn’t match the check.You only assert type, but the message says “greater than zero.” Either adjust the message or assert the size.
- self::assertIsInt( - $size1, - 'Temporary file should have a valid size greater than zero.', - ); + self::assertIsInt($size1); + self::assertGreaterThanOrEqual(0, $size1, 'Temporary file should have a non-negative size.');
169-173: Fix message: this is a GET request.The message says “POST request,” but this assertion is for the GET request.
- 'Expected PSR-7 Response body to be empty for POST request with no uploaded files.', + 'Expected PSR-7 Response body to be empty for GET request with no uploaded files.',
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
tests/http/UploadedFileTest.php(1 hunks)tests/http/stateless/StatelessApplicationUploadedTest.php(1 hunks)tests/support/FactoryHelper.php(2 hunks)
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#78
File: src/http/Request.php:0-0
Timestamp: 2025-08-10T13:17:04.265Z
Learning: In the yii2-extensions/psr-bridge project, PHPStan type annotations should be defensive when dealing with external input from PSR-7 sources. For methods like `getUploadedFiles()` and `convertPsr7ToUploadedFiles()`, use `mixed` in the return type annotation to account for untrusted input values, even if the implementation filters them.
📚 Learning: 2025-08-10T20:39:09.333Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#82
File: tests/adapter/UploadedFilesPsr7Test.php:197-248
Timestamp: 2025-08-10T20:39:09.333Z
Learning: In the yii2-extensions/psr-bridge project, the TestCase base class (tests/TestCase.php) automatically handles temporary file cleanup. The `createTmpFile()` method tracks files in `$this->tmpFiles[]`, and the `tearDown()` method calls `closeTmpFile()` to clean them up. Tests extending TestCase don't need manual file cleanup when using these methods.
Applied to files:
tests/http/UploadedFileTest.phptests/http/stateless/StatelessApplicationUploadedTest.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/UploadedFileTest.phptests/http/stateless/StatelessApplicationUploadedTest.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/http/UploadedFileTest.phptests/http/stateless/StatelessApplicationUploadedTest.php
📚 Learning: 2025-08-24T11:52:50.563Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#141
File: tests/http/stateless/ApplicationRoutingTest.php:1-164
Timestamp: 2025-08-24T11:52:50.563Z
Learning: In yii2-extensions/psr-bridge, tests that manipulate PHP superglobals ($_POST, $_GET, $_SERVER) in the http group do not require process isolation and work fine with the current PHPUnit configuration.
Applied to files:
tests/http/UploadedFileTest.phptests/http/stateless/StatelessApplicationUploadedTest.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/UploadedFileTest.phptests/http/stateless/StatelessApplicationUploadedTest.php
📚 Learning: 2025-08-10T13:17:04.265Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#78
File: src/http/Request.php:0-0
Timestamp: 2025-08-10T13:17:04.265Z
Learning: In the yii2-extensions/psr-bridge project, PHPStan type annotations should be defensive when dealing with external input from PSR-7 sources. For methods like `getUploadedFiles()` and `convertPsr7ToUploadedFiles()`, use `mixed` in the return type annotation to account for untrusted input values, even if the implementation filters them.
Applied to files:
tests/http/UploadedFileTest.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/stateless/StatelessApplicationUploadedTest.php
🧬 Code graph analysis (2)
tests/http/UploadedFileTest.php (4)
src/adapter/ServerRequestAdapter.php (1)
ServerRequestAdapter(47-441)src/http/UploadedFile.php (3)
UploadedFile(39-433)getInstanceByName(113-118)setPsr7Adapter(209-214)tests/support/FactoryHelper.php (4)
FactoryHelper(46-268)createRequest(68-89)createUploadedFile(227-235)createStream(191-194)tests/TestCase.php (2)
TestCase(23-227)createTmpFile(101-112)
tests/http/stateless/StatelessApplicationUploadedTest.php (6)
src/creator/ServerRequestCreator.php (1)
ServerRequestCreator(56-209)src/http/UploadedFile.php (2)
UploadedFile(39-433)getInstancesByName(172-189)tests/support/FactoryHelper.php (6)
FactoryHelper(46-268)createServerRequestFactory(173-176)createStreamFactory(206-209)createUploadedFileFactory(247-250)createRequest(68-89)createUploadedFile(227-235)tests/TestCase.php (3)
TestCase(23-227)createTmpFile(101-112)statelessApplication(134-197)src/http/StatelessApplication.php (1)
handle(269-295)src/http/Request.php (1)
createUploadedFile(847-859)
🪛 GitHub Actions: build
tests/http/UploadedFileTest.php
[error] 33-33: PHPUnit test failed: Should return an instance of UploadedFile when a single file is uploaded. Failed asserting that null is an instance of class yii2\extensions\psrbridge\http\UploadedFile.
🪛 GitHub Actions: mutation test
tests/http/UploadedFileTest.php
[error] 33-33: UploadedFileTest::testLegacyFilesLoadingWhenNotPsr7Adapter failed: Expected an UploadedFile instance but got null.
🔇 Additional comments (2)
tests/support/FactoryHelper.php (1)
216-216: Union type for $tmpName looks good and aligns with PSR-7 constructors.Accepting
string|StreamInterfacefor$tmpNameis appropriate and matchesHttpSoft\Message\UploadedFileexpectations.Also applies to: 230-235
tests/http/stateless/StatelessApplicationUploadedTest.php (1)
20-99: PSR-7 adapter wiring verified UploadedFile::setPsr7Adapter is invoked per request in Request.php (line 797) and no legacy UploadedFile::reset calls remain.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (6)
src/http/UploadedFile.php (6)
39-41: Declare tempResource to prevent UnknownPropertyException when constructing from arrays.Instances are built via
new self($files[$name])includingtempResource, but the class doesn’t declare it.Apply:
final class UploadedFile extends \yii\web\UploadedFile { @@ public static $_files = []; + /** + * Resource handle for the temporary stream when sourced from PSR-7. + * + * @var resource|null + */ + public $tempResource = null;Also applies to: 57-57
69-96: Override saveAs() to support PSR‑7 streams (php://temp/memory).Parent uses move_uploaded_file/copy on paths; it won’t handle detached resources. Copy from
$this->tempResourcewhen present and close on success.private static bool $psr7FilesLoaded = false; + /** + * Saves the uploaded file, supporting PSR-7 detached streams. + */ + public function saveAs($file, $deleteTempFile = true): bool + { + if ($this->error !== UPLOAD_ERR_OK) { + return false; + } + if (is_resource($this->tempResource)) { + @rewind($this->tempResource); + $out = @fopen($file, 'wb'); + if ($out === false) { + return false; + } + $ok = stream_copy_to_stream($this->tempResource, $out) !== false; + fclose($out); + if ($deleteTempFile) { + @fclose($this->tempResource); + $this->tempResource = null; + } + return $ok; + } + return parent::saveAs($file, $deleteTempFile); + }
172-189: Return all matches and normalize 'name[]' in getInstancesByName().
- Trim trailing
[]for array inputs.- Keep returning all matches (add tests to kill “ArrayOneItem” mutant).
public static function getInstancesByName($name): array { $files = self::loadFiles(); + if (is_string($name) && substr($name, -2) === '[]') { + $name = substr($name, 0, -2); + } + if (isset($files[$name])) { return [new self($files[$name])]; } @@ return $results; }To verify coverage gaps locally:
#!/bin/bash # Expect: at least one test asserting count > 1 from getInstancesByName rg -nP --type=php -C2 "getInstancesByName\(['\"]\w+['\"]\)" tests | rg -nP "assertCount\(\s*[2-9]"
203-208: Close detached resources in reset() before clearing caches.Avoid leaks in workers/stateless runtimes.
public static function reset(): void { + self::closeResources(); self::$_files = []; self::$psr7Adapter = null; self::$psr7FilesLoaded = false; }
228-233: Also close resources when swapping PSR‑7 adapters.Prevents descriptor leaks between requests.
public static function setPsr7Adapter(ServerRequestAdapter $adapter): void { + self::closeResources(); self::$_files = []; self::$psr7Adapter = $adapter; self::$psr7FilesLoaded = false; }
258-272: PSR‑7 contract: don’t call getStream() when upload has errors.Guard on
$psr7File->getError()first; many implementations throw otherwise.private static function convertPsr7FileToLegacyFormat(UploadedFileInterface $psr7File): array { - $stream = $psr7File->getStream(); - $uri = $stream->getMetadata('uri'); + $error = $psr7File->getError(); + if ($error !== UPLOAD_ERR_OK) { + return [ + 'name' => $psr7File->getClientFilename() ?? '', + 'tempName' => '', + 'tempResource' => null, + 'type' => '', + 'size' => 0, + 'error' => $error, + 'fullPath' => null, + ]; + } + + $stream = $psr7File->getStream(); + $uri = $stream->getMetadata('uri'); return [ 'name' => $psr7File->getClientFilename() ?? '', 'tempName' => is_string($uri) ? $uri : '', 'tempResource' => $stream->detach(), 'type' => $psr7File->getClientMediaType() ?? '', 'size' => $psr7File->getSize() ?? 0, - 'error' => $psr7File->getError(), + 'error' => $error, 'fullPath' => null, ]; }
🧹 Nitpick comments (2)
src/http/UploadedFile.php (2)
27-33: Doc nit: “Immutable, type‑safe access” is misleading.Instances are mutable (public props incl.
tempResource). Rephrase to avoid overpromising.
300-308: Add test to verify PSR-7 adapter reload after reset()
Write a test that:
- Calls
UploadedFile::setPsr7Adapter($adapterA)and invokes a getter to load files from A- Calls
UploadedFile::reset()- Calls
UploadedFile::setPsr7Adapter($adapterB)and asserts that subsequent getters return files from B (not A)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/http/UploadedFile.php(1 hunks)tests/http/UploadedFileTest.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/http/UploadedFileTest.php
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#78
File: src/http/Request.php:0-0
Timestamp: 2025-08-10T13:17:04.265Z
Learning: In the yii2-extensions/psr-bridge project, PHPStan type annotations should be defensive when dealing with external input from PSR-7 sources. For methods like `getUploadedFiles()` and `convertPsr7ToUploadedFiles()`, use `mixed` in the return type annotation to account for untrusted input values, even if the implementation filters them.
📚 Learning: 2025-08-10T13:17:04.265Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#78
File: src/http/Request.php:0-0
Timestamp: 2025-08-10T13:17:04.265Z
Learning: In the yii2-extensions/psr-bridge project, PHPStan type annotations should be defensive when dealing with external input from PSR-7 sources. For methods like `getUploadedFiles()` and `convertPsr7ToUploadedFiles()`, use `mixed` in the return type annotation to account for untrusted input values, even if the implementation filters them.
Applied to files:
src/http/UploadedFile.php
🧬 Code graph analysis (1)
src/http/UploadedFile.php (3)
src/adapter/ServerRequestAdapter.php (2)
ServerRequestAdapter(47-441)getUploadedFiles(323-326)src/http/StatelessApplication.php (1)
reset(401-439)src/http/Request.php (1)
getUploadedFiles(693-700)
🪛 GitHub Check: mutation / PHP 8.3-ubuntu-latest
src/http/UploadedFile.php
[warning] 207-207:
Escaped Mutant for Mutator "FalseValue":
@@ @@
{
self::$_files = [];
self::$psr7Adapter = null;
-
self::$psr7FilesLoaded = false;
-
}
self::$psr7FilesLoaded = true;
/**
* Sets the PSR-7 ServerRequestAdapter for file handling.
[warning] 188-188:
Escaped Mutant for Mutator "ArrayOneItem":
@@ @@
$results[] = new self($file);
}
}
-
return $results;
-
}
return count($results) > 1 ? array_slice($results, 0, 1, true) : $results;
/**
* Resets the internal uploaded files cache and PSR-7 adapter state.
🔇 Additional comments (2)
src/http/UploadedFile.php (2)
113-118: Normalize 'name[]' alias in getInstanceByName().Common callers pass
'field[]'; strip trailing[]before lookup.
[Suggest_essential_refactor]public static function getInstanceByName($name): self|null { $files = self::loadFiles(); + if (is_string($name) && substr($name, -2) === '[]') { + $name = substr($name, 0, -2); + } + return isset($files[$name]) ? new self($files[$name]) : null; }
234-234: Add helper to close any open tempResource handles.
[Suggest_essential_refactor]public static function setPsr7Adapter(ServerRequestAdapter $adapter): void { self::closeResources(); self::$_files = []; self::$psr7Adapter = $adapter; self::$psr7FilesLoaded = false; } + /** + * Closes any open tempResource handles stored in the internal cache. + */ + private static function closeResources(): void + { + foreach (self::$_files as $k => &$info) { + if (is_array($info) && isset($info['tempResource']) && is_resource($info['tempResource'])) { + @fclose($info['tempResource']); + $info['tempResource'] = null; + } + } + unset($info); + }
…nd trailing brackets.
…oaded files cache.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
src/http/UploadedFile.php (4)
115-121: Normalize trailing [] in getInstanceByName() for parity with getInstancesByName().Common callers pass
'name[]'; align behavior.- public static function getInstanceByName($name): self|null + public static function getInstanceByName(string $name): self|null { $files = self::loadFiles(); + if (str_ends_with($name, '[]')) { + $name = substr($name, 0, -2); + } + return isset($files[$name]) ? new self($files[$name]) : null; }
59-69: Declare tempResource to prevent UnknownPropertyException when constructing from array configs.Instances are created via
new self($files[$name])and arrays includetempResource; without a declared property, Yii will throw at runtime.public static $_files = []; + /** + * Resource handle for the temporary file stream (PSR-7 bridge). + * + * @var resource|null + * @phpstan-var resource|null + */ + public $tempResource = null; + /** * PSR-7 ServerRequestAdapter for bridging PSR-7 UploadedFileInterface. */ private static ServerRequestAdapter|null $psr7Adapter = null;
263-277: PSR‑7 contract: don’t call getStream() when getError() != UPLOAD_ERR_OK.Unconditional
getStream()may throw per PSR‑7 for errored uploads.- private static function convertPsr7FileToLegacyFormat(UploadedFileInterface $psr7File): array - { - $stream = $psr7File->getStream(); - $uri = $stream->getMetadata('uri'); - - return [ - 'name' => $psr7File->getClientFilename() ?? '', - 'tempName' => is_string($uri) ? $uri : '', - 'tempResource' => $stream->detach(), - 'type' => $psr7File->getClientMediaType() ?? '', - 'size' => $psr7File->getSize() ?? 0, - 'error' => $psr7File->getError(), - 'fullPath' => null, - ]; - } + private static function convertPsr7FileToLegacyFormat(UploadedFileInterface $psr7File): array + { + $error = $psr7File->getError(); + if ($error !== UPLOAD_ERR_OK) { + return [ + 'name' => $psr7File->getClientFilename() ?? '', + 'tempName' => '', + 'tempResource' => null, + 'type' => '', + 'size' => 0, + 'error' => $error, + 'fullPath' => null, + ]; + } + + $stream = $psr7File->getStream(); + $uri = $stream->getMetadata('uri'); + + return [ + 'name' => $psr7File->getClientFilename() ?? '', + 'tempName' => is_string($uri) ? $uri : '', + 'tempResource' => $stream->detach(), + 'type' => $psr7File->getClientMediaType() ?? '', + 'size' => $psr7File->getSize() ?? 0, + 'error' => $error, + 'fullPath' => null, + ]; + }
41-60: Override saveAs() to support stream-backed uploads (php://temp, memory).Parent implementation expects a filesystem path; PSR‑7 streams won’t save without this override.
final class UploadedFile extends \yii\web\UploadedFile { + /** + * Saves the uploaded file. Falls back to copying from $this->tempResource for non-path streams. + */ + public function saveAs($file, $deleteTempFile = true): bool + { + if (is_resource($this->tempResource)) { + @rewind($this->tempResource); + $out = @fopen($file, 'wb'); + if ($out === false) { + return false; + } + try { + $copied = stream_copy_to_stream($this->tempResource, $out); + } finally { + fclose($out); + } + $ok = ($copied !== false); + if ($ok && $deleteTempFile) { + @fclose($this->tempResource); + $this->tempResource = null; + } + return $ok; + } + return parent::saveAs($file, $deleteTempFile); + }
🧹 Nitpick comments (3)
src/http/UploadedFile.php (3)
174-195: Add scalar type-hints for $name and tighten return annotations.Improves static analysis and aligns with strict_types.
- public static function getInstancesByName($name): array + public static function getInstancesByName(string $name): array { $files = self::loadFiles();
305-313: Set psr7FilesLoaded after successful load to avoid stale true on exceptions.Small robustness tweak.
- if (self::$psr7Adapter !== null && self::$psr7FilesLoaded === false) { - self::$psr7FilesLoaded = true; - - self::loadPsr7Files(); + if (self::$psr7Adapter !== null && self::$psr7FilesLoaded === false) { + self::loadPsr7Files(); + self::$psr7FilesLoaded = true;
29-35: Docs nit: “Immutable” claim is inaccurate.The class exposes mutable static cache; consider rewording.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/http/UploadedFile.php(1 hunks)tests/http/UploadedFileTest.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/http/UploadedFileTest.php
🧰 Additional context used
🧠 Learnings (11)
📓 Common learnings
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#78
File: src/http/Request.php:0-0
Timestamp: 2025-08-10T13:17:04.265Z
Learning: In the yii2-extensions/psr-bridge project, PHPStan type annotations should be defensive when dealing with external input from PSR-7 sources. For methods like `getUploadedFiles()` and `convertPsr7ToUploadedFiles()`, use `mixed` in the return type annotation to account for untrusted input values, even if the implementation filters them.
📚 Learning: 2025-08-10T13:17:04.265Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#78
File: src/http/Request.php:0-0
Timestamp: 2025-08-10T13:17:04.265Z
Learning: In the yii2-extensions/psr-bridge project, PHPStan type annotations should be defensive when dealing with external input from PSR-7 sources. For methods like `getUploadedFiles()` and `convertPsr7ToUploadedFiles()`, use `mixed` in the return type annotation to account for untrusted input values, even if the implementation filters them.
Applied to files:
src/http/UploadedFile.php
📚 Learning: 2025-08-10T20:39:09.333Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#82
File: tests/adapter/UploadedFilesPsr7Test.php:197-248
Timestamp: 2025-08-10T20:39:09.333Z
Learning: In the yii2-extensions/psr-bridge project, the TestCase base class (tests/TestCase.php) automatically handles temporary file cleanup. The `createTmpFile()` method tracks files in `$this->tmpFiles[]`, and the `tearDown()` method calls `closeTmpFile()` to clean them up. Tests extending TestCase don't need manual file cleanup when using these methods.
Applied to files:
src/http/UploadedFile.php
📚 Learning: 2025-08-24T11:52:50.563Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#141
File: tests/http/stateless/ApplicationRoutingTest.php:1-164
Timestamp: 2025-08-24T11:52:50.563Z
Learning: In yii2-extensions/psr-bridge, tests that manipulate PHP superglobals ($_POST, $_GET, $_SERVER) in the http group do not require process isolation and work fine with the current PHPUnit configuration.
Applied to files:
src/http/UploadedFile.php
📚 Learning: 2025-08-10T20:39:09.333Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#82
File: tests/adapter/UploadedFilesPsr7Test.php:197-248
Timestamp: 2025-08-10T20:39:09.333Z
Learning: In the yii2-extensions/psr-bridge project, the TestCase base class configures the runtime path as `dirname(__DIR__) . '/runtime'` in the application configuration. Tests that need to save files to the runtime directory can rely on this pre-configured path.
Applied to files:
src/http/UploadedFile.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:
src/http/UploadedFile.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:
src/http/UploadedFile.php
📚 Learning: 2025-08-26T15:37:52.160Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#155
File: tests/http/stateless/ApplicationMemoryTest.php:198-211
Timestamp: 2025-08-26T15:37:52.160Z
Learning: In yii2-extensions/psr-bridge, tests can use the TestSupport trait from php-forge/support package which provides an invokeMethod() helper for calling protected/private methods during testing, eliminating the need for custom reflection code.
Applied to files:
src/http/UploadedFile.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:
src/http/UploadedFile.php
📚 Learning: 2025-08-08T15:24:06.085Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#71
File: tests/TestCase.php:23-27
Timestamp: 2025-08-08T15:24:06.085Z
Learning: In yii2-extensions/psr-bridge (tests/TestCase.php), maintainer preference: it’s acceptable to use random-looking strings for test-only constants like COOKIE_VALIDATION_KEY; no need to replace with an obviously non-secret value unless CI/secret scanners become problematic.
Applied to files:
src/http/UploadedFile.php
📚 Learning: 2025-08-25T16:08:54.379Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#149
File: tests/http/stateless/ApplicationCoreTest.php:24-27
Timestamp: 2025-08-25T16:08:54.379Z
Learning: In the yii2-extensions/psr-bridge project, avoid importing functions that are not actually used in the code, even if they might seem logically related to the functionality being tested.
Applied to files:
src/http/UploadedFile.php
🧬 Code graph analysis (1)
src/http/UploadedFile.php (2)
src/adapter/ServerRequestAdapter.php (2)
ServerRequestAdapter(47-441)getUploadedFiles(323-326)src/http/Request.php (1)
getUploadedFiles(693-700)
⏰ 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.2-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
…ce management and enhance reset method functionality.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
tests/http/UploadedFileTest.php (1)
17-18: Reset UploadedFile static state in setUp() to prevent cross-test pollution.Tests currently depend on ad-hoc resets; make isolation explicit and uniform.
final class UploadedFileTest extends TestCase { + protected function setUp(): void + { + parent::setUp(); + UploadedFile::reset(); + }
🧹 Nitpick comments (4)
tests/http/UploadedFileTest.php (4)
12-16: Import missing UPLOAD_ERR_CANT_WRITE constant for consistency and static analysis clarity.You already import UPLOAD_ERR_OK; import the error constant too to avoid reliance on fallback resolution and to keep imports consistent.
use function file_put_contents; use function stream_get_meta_data; use const UPLOAD_ERR_OK; +use const UPLOAD_ERR_CANT_WRITE;
74-120: Remove redundant inline reset; rely on setUp().With the proposed setUp() reset, this explicit reset is unnecessary and can hide flaws in isolation if added later elsewhere.
- UploadedFile::reset();
181-285: LGTM: Multiple-file PSR-7 flow is well covered.Good coverage of base-name retrieval, []-suffix handling, and indexed access; distinct instances are asserted. Consider also asserting size/type for completeness.
181-285: Optional: Add a regression test for legacy fallback when PSR-7 has no files.This guards the new fallback and prevents future regressions.
@@ final class UploadedFileTest extends TestCase public function testReturnUploadedFileInstanceWhenMultipleFilesAreUploadedViaPsr7(): void { // ... } + + public function testLegacyFallbackWhenPsr7AdapterProvidesNoFiles(): void + { + $_FILES = [ + 'upload' => [ + 'name' => 'legacy.txt', + 'type' => 'text/plain', + 'tmp_name' => '/tmp/legacy', + 'error' => UPLOAD_ERR_OK, + 'size' => 10, + ], + ]; + + // Adapter set but with no uploaded files. + UploadedFile::setPsr7Adapter( + new ServerRequestAdapter( + FactoryHelper::createRequest('POST', 'site/post')->withUploadedFiles([]) + ) + ); + + $file = UploadedFile::getInstanceByName('upload'); + self::assertInstanceOf(UploadedFile::class, $file); + self::assertSame('legacy.txt', $file->name); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
src/http/Request.php(4 hunks)src/http/UploadedFile.php(1 hunks)tests/http/UploadedFileTest.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/http/Request.php
- src/http/UploadedFile.php
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#78
File: src/http/Request.php:0-0
Timestamp: 2025-08-10T13:17:04.265Z
Learning: In the yii2-extensions/psr-bridge project, PHPStan type annotations should be defensive when dealing with external input from PSR-7 sources. For methods like `getUploadedFiles()` and `convertPsr7ToUploadedFiles()`, use `mixed` in the return type annotation to account for untrusted input values, even if the implementation filters them.
📚 Learning: 2025-08-10T20:39:09.333Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#82
File: tests/adapter/UploadedFilesPsr7Test.php:197-248
Timestamp: 2025-08-10T20:39:09.333Z
Learning: In the yii2-extensions/psr-bridge project, the TestCase base class (tests/TestCase.php) automatically handles temporary file cleanup. The `createTmpFile()` method tracks files in `$this->tmpFiles[]`, and the `tearDown()` method calls `closeTmpFile()` to clean them up. Tests extending TestCase don't need manual file cleanup when using these methods.
Applied to files:
tests/http/UploadedFileTest.php
📚 Learning: 2025-08-10T13:17:04.265Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#78
File: src/http/Request.php:0-0
Timestamp: 2025-08-10T13:17:04.265Z
Learning: In the yii2-extensions/psr-bridge project, PHPStan type annotations should be defensive when dealing with external input from PSR-7 sources. For methods like `getUploadedFiles()` and `convertPsr7ToUploadedFiles()`, use `mixed` in the return type annotation to account for untrusted input values, even if the implementation filters them.
Applied to files:
tests/http/UploadedFileTest.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/UploadedFileTest.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/http/UploadedFileTest.php
📚 Learning: 2025-08-24T11:52:50.563Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#141
File: tests/http/stateless/ApplicationRoutingTest.php:1-164
Timestamp: 2025-08-24T11:52:50.563Z
Learning: In yii2-extensions/psr-bridge, tests that manipulate PHP superglobals ($_POST, $_GET, $_SERVER) in the http group do not require process isolation and work fine with the current PHPUnit configuration.
Applied to files:
tests/http/UploadedFileTest.php
🧬 Code graph analysis (1)
tests/http/UploadedFileTest.php (4)
src/adapter/ServerRequestAdapter.php (1)
ServerRequestAdapter(47-441)src/http/UploadedFile.php (5)
UploadedFile(41-487)setPsr7Adapter(235-242)getInstanceByName(115-120)reset(209-215)getInstancesByName(174-195)tests/support/FactoryHelper.php (4)
FactoryHelper(46-268)createUploadedFile(227-235)createStream(191-194)createRequest(68-89)tests/TestCase.php (2)
TestCase(23-227)createTmpFile(101-112)
⏰ 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: mutation / PHP 8.3-ubuntu-latest
- GitHub Check: phpunit / PHP 8.4-windows-latest
- GitHub Check: mutation / PHP 8.3-ubuntu-latest
🔇 Additional comments (1)
tests/http/UploadedFileTest.php (1)
19-72: LGTM: PSR-7 error-path conversion behavior is covered well.Asserts correctly verify name/type/size propagation and empty tempName on error.
… method to maintain test isolation.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/http/UploadedFile.php (4)
110-115: Optional: use late static binding for instances.Using
new static(...)mirrors Yii’s implementation and future‑proofs in case the class stops being final.- return isset($files[$name]) ? new self($files[$name]) : null; + return isset($files[$name]) ? new static($files[$name]) : null; @@ - $results[] = new self($file); + $results[] = new static($file);Also applies to: 169-190
238-248: Nit: defensive isset before is_resource.While all entries currently include
tempResource, anisset()guard avoids notices if shapes ever vary.- foreach (self::$_files as $entry) { - if (is_resource($entry['tempResource'])) { + foreach (self::$_files as $entry) { + if (isset($entry['tempResource']) && is_resource($entry['tempResource'])) { @fclose($entry['tempResource']); } }
327-338: Nit: simplify conditional.
elseif (self::$psr7Adapter === null)can be a simpleelse.- if (self::$psr7Adapter !== null) { - self::loadPsr7Files(); - } elseif (self::$psr7Adapter === null) { - self::loadLegacyFiles(); - } + if (self::$psr7Adapter !== null) { + self::loadPsr7Files(); + } else { + self::loadLegacyFiles(); + }
204-210: Optional: also sync parent cache.Calling
parent::reset();keeps the parent’s private static cache in sync if mixed usage occurs.public static function reset() { self::closeResources(); self::$_files = []; self::$psr7Adapter = null; + // Keep Yii parent cache consistent if used elsewhere. + parent::reset(); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/http/UploadedFile.php(1 hunks)tests/http/UploadedFileTest.php(1 hunks)
🧰 Additional context used
🧠 Learnings (11)
📓 Common learnings
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#160
File: src/http/UploadedFile.php:41-41
Timestamp: 2025-08-28T22:07:08.430Z
Learning: In yii2-extensions/psr-bridge, the UploadedFile class extends \yii\web\UploadedFile which already handles saving from detached streams/resources in its saveAs() method. The parent class checks is_resource($this->_tempResource) and uses copyTempFile() with stream_copy_to_stream(). The PSR-7 bridge correctly passes detached streams via tempResource, so no override of saveAs() is needed.
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#160
File: src/http/UploadedFile.php:41-41
Timestamp: 2025-08-28T22:07:08.430Z
Learning: In yii2-extensions/psr-bridge, the UploadedFile class extends \yii\web\UploadedFile which already handles saving from detached streams/resources in its saveAs() method. The parent class checks is_resource($this->_tempResource) and uses copyTempFile() with stream_copy_to_stream(). The PSR-7 bridge correctly passes detached streams via tempResource, so no override of saveAs() is needed.
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#160
File: tests/http/UploadedFileTest.php:122-179
Timestamp: 2025-08-28T22:08:51.791Z
Learning: In yii2-extensions/psr-bridge, when the PSR-7 adapter is set in UploadedFile, it indicates worker mode (like RoadRunner, Swoole), where $_FILES is not meaningful and should not be checked as a fallback. The PSR-7 adapter becomes the authoritative source for uploaded files in this mode.
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#78
File: src/http/Request.php:0-0
Timestamp: 2025-08-10T13:17:04.265Z
Learning: In the yii2-extensions/psr-bridge project, PHPStan type annotations should be defensive when dealing with external input from PSR-7 sources. For methods like `getUploadedFiles()` and `convertPsr7ToUploadedFiles()`, use `mixed` in the return type annotation to account for untrusted input values, even if the implementation filters them.
📚 Learning: 2025-08-28T22:08:51.791Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#160
File: tests/http/UploadedFileTest.php:122-179
Timestamp: 2025-08-28T22:08:51.791Z
Learning: In yii2-extensions/psr-bridge, when the PSR-7 adapter is set in UploadedFile, it indicates worker mode (like RoadRunner, Swoole), where $_FILES is not meaningful and should not be checked as a fallback. The PSR-7 adapter becomes the authoritative source for uploaded files in this mode.
Applied to files:
tests/http/UploadedFileTest.phpsrc/http/UploadedFile.php
📚 Learning: 2025-08-28T22:07:08.430Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#160
File: src/http/UploadedFile.php:41-41
Timestamp: 2025-08-28T22:07:08.430Z
Learning: In yii2-extensions/psr-bridge, the UploadedFile class extends \yii\web\UploadedFile which already handles saving from detached streams/resources in its saveAs() method. The parent class checks is_resource($this->_tempResource) and uses copyTempFile() with stream_copy_to_stream(). The PSR-7 bridge correctly passes detached streams via tempResource, so no override of saveAs() is needed.
Applied to files:
tests/http/UploadedFileTest.phpsrc/http/UploadedFile.php
📚 Learning: 2025-08-10T20:39:09.333Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#82
File: tests/adapter/UploadedFilesPsr7Test.php:197-248
Timestamp: 2025-08-10T20:39:09.333Z
Learning: In the yii2-extensions/psr-bridge project, the TestCase base class (tests/TestCase.php) automatically handles temporary file cleanup. The `createTmpFile()` method tracks files in `$this->tmpFiles[]`, and the `tearDown()` method calls `closeTmpFile()` to clean them up. Tests extending TestCase don't need manual file cleanup when using these methods.
Applied to files:
tests/http/UploadedFileTest.phpsrc/http/UploadedFile.php
📚 Learning: 2025-08-10T13:17:04.265Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#78
File: src/http/Request.php:0-0
Timestamp: 2025-08-10T13:17:04.265Z
Learning: In the yii2-extensions/psr-bridge project, PHPStan type annotations should be defensive when dealing with external input from PSR-7 sources. For methods like `getUploadedFiles()` and `convertPsr7ToUploadedFiles()`, use `mixed` in the return type annotation to account for untrusted input values, even if the implementation filters them.
Applied to files:
tests/http/UploadedFileTest.phpsrc/http/UploadedFile.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/UploadedFileTest.phpsrc/http/UploadedFile.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/http/UploadedFileTest.phpsrc/http/UploadedFile.php
📚 Learning: 2025-08-24T11:52:50.563Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#141
File: tests/http/stateless/ApplicationRoutingTest.php:1-164
Timestamp: 2025-08-24T11:52:50.563Z
Learning: In yii2-extensions/psr-bridge, tests that manipulate PHP superglobals ($_POST, $_GET, $_SERVER) in the http group do not require process isolation and work fine with the current PHPUnit configuration.
Applied to files:
tests/http/UploadedFileTest.phpsrc/http/UploadedFile.php
📚 Learning: 2025-08-10T20:39:09.333Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#82
File: tests/adapter/UploadedFilesPsr7Test.php:197-248
Timestamp: 2025-08-10T20:39:09.333Z
Learning: In the yii2-extensions/psr-bridge project, the TestCase base class configures the runtime path as `dirname(__DIR__) . '/runtime'` in the application configuration. Tests that need to save files to the runtime directory can rely on this pre-configured path.
Applied to files:
src/http/UploadedFile.php
📚 Learning: 2025-08-26T15:37:52.160Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#155
File: tests/http/stateless/ApplicationMemoryTest.php:198-211
Timestamp: 2025-08-26T15:37:52.160Z
Learning: In yii2-extensions/psr-bridge, tests can use the TestSupport trait from php-forge/support package which provides an invokeMethod() helper for calling protected/private methods during testing, eliminating the need for custom reflection code.
Applied to files:
src/http/UploadedFile.php
📚 Learning: 2025-08-08T15:24:06.085Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#71
File: tests/TestCase.php:23-27
Timestamp: 2025-08-08T15:24:06.085Z
Learning: In yii2-extensions/psr-bridge (tests/TestCase.php), maintainer preference: it’s acceptable to use random-looking strings for test-only constants like COOKIE_VALIDATION_KEY; no need to replace with an obviously non-secret value unless CI/secret scanners become problematic.
Applied to files:
src/http/UploadedFile.php
🧬 Code graph analysis (2)
tests/http/UploadedFileTest.php (4)
src/adapter/ServerRequestAdapter.php (1)
ServerRequestAdapter(47-441)src/http/UploadedFile.php (5)
UploadedFile(41-479)reset(204-210)setPsr7Adapter(230-236)getInstanceByName(110-115)getInstancesByName(169-190)tests/support/FactoryHelper.php (5)
FactoryHelper(46-268)createUploadedFile(227-235)createStream(191-194)createRequest(68-89)createUploadedFileFactory(247-250)tests/TestCase.php (2)
TestCase(23-227)createTmpFile(101-112)
src/http/UploadedFile.php (2)
src/adapter/ServerRequestAdapter.php (2)
ServerRequestAdapter(47-441)getUploadedFiles(323-326)src/http/Request.php (1)
getUploadedFiles(693-700)
⏰ 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.4-windows-latest
- GitHub Check: mutation / PHP 8.3-ubuntu-latest
- GitHub Check: mutation / PHP 8.3-ubuntu-latest
🔇 Additional comments (12)
src/http/UploadedFile.php (7)
173-176: Nice: handles trailing [] notation.Normalization for
name[]is correct and aligns with common form conventions.
273-301: Good: PSR‑7 compliance by skipping getStream() on error.Avoiding
getStream()when$error !== UPLOAD_ERR_OKmatches the PSR‑7 contract; mappingsize|null → 0is also correct.
141-146: Fix: method signature incompatible with parent (return type).Parent
getInstances()has no return type. Remove: array.- public static function getInstances($model, $attribute): array + public static function getInstances($model, $attribute)Likely an incorrect or invalid review comment.
110-115: Fix: method signature incompatible with parent (return type).Parent
getInstanceByName()has no return type. Remove: self|null.- public static function getInstanceByName($name): self|null + public static function getInstanceByName($name)Likely an incorrect or invalid review comment.
87-93: Fix: method signature incompatible with parent (return type).Parent yii\web\UploadedFile methods have no return types. Declaring
: self|nullhere will trigger a fatal “must be compatible” error. Remove the return type to match the parent.- public static function getInstance($model, $attribute): self|null + public static function getInstance($model, $attribute)Likely an incorrect or invalid review comment.
169-190: Fix: method signature incompatible with parent (return type).Parent
getInstancesByName()has no return type. Remove: array.- public static function getInstancesByName($name): array + public static function getInstancesByName($name)Likely an incorrect or invalid review comment.
204-210: Fix: method signature incompatible with parent (return type).Parent
reset()declares no return type. Remove: voidfor compatibility.- public static function reset(): void + public static function reset()Likely an incorrect or invalid review comment.
tests/http/UploadedFileTest.php (5)
26-79: Good: asserts PSR‑7 error-path mapping without calling getStream().Covers filename/type/size/error, preventing regressions.
81-114: Good: size null → zero behavior validated.
116-160: Good: legacy $_FILES path covered and isolated via setUp reset.
162-219: Good: enforces resource cleanup on reset().White-box inspection is acceptable here; assertions are tight.
221-325: Good: multi-file retrieval incl. '[]' normalization and indexed access.Covers order and distinct instances, killing common mutations.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
tests/http/UploadedFileTest.php (1)
163-220: Strengthen reset() assertions by also verifying cache is cleared.
Add an assertion that the cache array is empty after reset.UploadedFile::reset(); $stillOpenAfterReset = 0; @@ self::assertSame( 0, $stillOpenAfterReset, "All resources should be closed after 'reset()' method.", ); + + self::assertSame( + [], + UploadedFile::$_files, + 'Cache must be cleared after reset().', + );src/http/UploadedFile.php (4)
241-248: Guard against missing key in closeResources().
Avoid potential notices by checking the key before is_resource().- foreach (self::$_files as $entry) { - if (is_resource($entry['tempResource'])) { + foreach (self::$_files as $entry) { + if (isset($entry['tempResource']) && is_resource($entry['tempResource'])) { @fclose($entry['tempResource']); } }
327-334: Minor: simplify redundant branch.
Theelseif (self::$psr7Adapter === null)can be a plainelse.- if (self::$psr7Adapter !== null) { - self::loadPsr7Files(); - } elseif (self::$psr7Adapter === null) { - self::loadLegacyFiles(); - } + if (self::$psr7Adapter !== null) { + self::loadPsr7Files(); + } else { + self::loadLegacyFiles(); + }
30-35: Doc nit: colon instead of period in “Key features”.
Minor wording polish.- * Key features. + * Key features:
59-65: Encapsulation: consider making $_files private and exposing a test helper.
Public static caches are fragile APIs. You can keep tests green by adding a debug accessor (e.g., debugFiles()) under test-only condition.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/http/UploadedFile.php(1 hunks)tests/http/UploadedFileTest.php(1 hunks)
🧰 Additional context used
🧠 Learnings (11)
📓 Common learnings
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#160
File: src/http/UploadedFile.php:41-41
Timestamp: 2025-08-28T22:07:08.430Z
Learning: In yii2-extensions/psr-bridge, the UploadedFile class extends \yii\web\UploadedFile which already handles saving from detached streams/resources in its saveAs() method. The parent class checks is_resource($this->_tempResource) and uses copyTempFile() with stream_copy_to_stream(). The PSR-7 bridge correctly passes detached streams via tempResource, so no override of saveAs() is needed.
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#160
File: src/http/UploadedFile.php:41-41
Timestamp: 2025-08-28T22:07:08.430Z
Learning: In yii2-extensions/psr-bridge, the UploadedFile class extends \yii\web\UploadedFile which already handles saving from detached streams/resources in its saveAs() method. The parent class checks is_resource($this->_tempResource) and uses copyTempFile() with stream_copy_to_stream(). The PSR-7 bridge correctly passes detached streams via tempResource, so no override of saveAs() is needed.
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#160
File: tests/http/UploadedFileTest.php:122-179
Timestamp: 2025-08-28T22:08:51.791Z
Learning: In yii2-extensions/psr-bridge, when the PSR-7 adapter is set in UploadedFile, it indicates worker mode (like RoadRunner, Swoole), where $_FILES is not meaningful and should not be checked as a fallback. The PSR-7 adapter becomes the authoritative source for uploaded files in this mode.
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#78
File: src/http/Request.php:0-0
Timestamp: 2025-08-10T13:17:04.265Z
Learning: In the yii2-extensions/psr-bridge project, PHPStan type annotations should be defensive when dealing with external input from PSR-7 sources. For methods like `getUploadedFiles()` and `convertPsr7ToUploadedFiles()`, use `mixed` in the return type annotation to account for untrusted input values, even if the implementation filters them.
📚 Learning: 2025-08-28T22:07:08.430Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#160
File: src/http/UploadedFile.php:41-41
Timestamp: 2025-08-28T22:07:08.430Z
Learning: In yii2-extensions/psr-bridge, the UploadedFile class extends \yii\web\UploadedFile which already handles saving from detached streams/resources in its saveAs() method. The parent class checks is_resource($this->_tempResource) and uses copyTempFile() with stream_copy_to_stream(). The PSR-7 bridge correctly passes detached streams via tempResource, so no override of saveAs() is needed.
Applied to files:
tests/http/UploadedFileTest.phpsrc/http/UploadedFile.php
📚 Learning: 2025-08-28T22:08:51.791Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#160
File: tests/http/UploadedFileTest.php:122-179
Timestamp: 2025-08-28T22:08:51.791Z
Learning: In yii2-extensions/psr-bridge, when the PSR-7 adapter is set in UploadedFile, it indicates worker mode (like RoadRunner, Swoole), where $_FILES is not meaningful and should not be checked as a fallback. The PSR-7 adapter becomes the authoritative source for uploaded files in this mode.
Applied to files:
tests/http/UploadedFileTest.phpsrc/http/UploadedFile.php
📚 Learning: 2025-08-10T20:39:09.333Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#82
File: tests/adapter/UploadedFilesPsr7Test.php:197-248
Timestamp: 2025-08-10T20:39:09.333Z
Learning: In the yii2-extensions/psr-bridge project, the TestCase base class (tests/TestCase.php) automatically handles temporary file cleanup. The `createTmpFile()` method tracks files in `$this->tmpFiles[]`, and the `tearDown()` method calls `closeTmpFile()` to clean them up. Tests extending TestCase don't need manual file cleanup when using these methods.
Applied to files:
tests/http/UploadedFileTest.phpsrc/http/UploadedFile.php
📚 Learning: 2025-08-10T13:17:04.265Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#78
File: src/http/Request.php:0-0
Timestamp: 2025-08-10T13:17:04.265Z
Learning: In the yii2-extensions/psr-bridge project, PHPStan type annotations should be defensive when dealing with external input from PSR-7 sources. For methods like `getUploadedFiles()` and `convertPsr7ToUploadedFiles()`, use `mixed` in the return type annotation to account for untrusted input values, even if the implementation filters them.
Applied to files:
tests/http/UploadedFileTest.phpsrc/http/UploadedFile.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/UploadedFileTest.phpsrc/http/UploadedFile.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/http/UploadedFileTest.phpsrc/http/UploadedFile.php
📚 Learning: 2025-08-24T11:52:50.563Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#141
File: tests/http/stateless/ApplicationRoutingTest.php:1-164
Timestamp: 2025-08-24T11:52:50.563Z
Learning: In yii2-extensions/psr-bridge, tests that manipulate PHP superglobals ($_POST, $_GET, $_SERVER) in the http group do not require process isolation and work fine with the current PHPUnit configuration.
Applied to files:
tests/http/UploadedFileTest.phpsrc/http/UploadedFile.php
📚 Learning: 2025-08-10T20:39:09.333Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#82
File: tests/adapter/UploadedFilesPsr7Test.php:197-248
Timestamp: 2025-08-10T20:39:09.333Z
Learning: In the yii2-extensions/psr-bridge project, the TestCase base class configures the runtime path as `dirname(__DIR__) . '/runtime'` in the application configuration. Tests that need to save files to the runtime directory can rely on this pre-configured path.
Applied to files:
src/http/UploadedFile.php
📚 Learning: 2025-08-26T15:37:52.160Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#155
File: tests/http/stateless/ApplicationMemoryTest.php:198-211
Timestamp: 2025-08-26T15:37:52.160Z
Learning: In yii2-extensions/psr-bridge, tests can use the TestSupport trait from php-forge/support package which provides an invokeMethod() helper for calling protected/private methods during testing, eliminating the need for custom reflection code.
Applied to files:
src/http/UploadedFile.php
📚 Learning: 2025-08-08T15:24:06.085Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#71
File: tests/TestCase.php:23-27
Timestamp: 2025-08-08T15:24:06.085Z
Learning: In yii2-extensions/psr-bridge (tests/TestCase.php), maintainer preference: it’s acceptable to use random-looking strings for test-only constants like COOKIE_VALIDATION_KEY; no need to replace with an obviously non-secret value unless CI/secret scanners become problematic.
Applied to files:
src/http/UploadedFile.php
🧬 Code graph analysis (2)
tests/http/UploadedFileTest.php (4)
src/adapter/ServerRequestAdapter.php (1)
ServerRequestAdapter(47-441)src/http/UploadedFile.php (5)
UploadedFile(41-479)reset(204-210)setPsr7Adapter(230-236)getInstanceByName(110-115)getInstancesByName(169-190)tests/support/FactoryHelper.php (4)
FactoryHelper(46-268)createUploadedFile(227-235)createStream(191-194)createRequest(68-89)tests/TestCase.php (2)
TestCase(23-227)createTmpFile(101-112)
src/http/UploadedFile.php (3)
src/adapter/ServerRequestAdapter.php (2)
ServerRequestAdapter(47-441)getUploadedFiles(323-326)src/http/StatelessApplication.php (1)
reset(401-439)src/http/Request.php (1)
getUploadedFiles(693-700)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: mutation / PHP 8.3-ubuntu-latest
- GitHub Check: mutation / PHP 8.3-ubuntu-latest
🔇 Additional comments (8)
tests/http/UploadedFileTest.php (5)
20-25: Good: clearing static state in setUp avoids cross-test bleed.
27-80: PSR-7 error-case mapping is well covered.
Covers name/type preservation, empty tempName, error code, and size passthrough.
82-115: Null size → 0 behavior validated.
This guards against PSR-7 getSize() returning null.
117-161: Legacy $_FILES path test looks solid.
Accurately asserts field mappings without invoking filesystem ops.
222-326: Comprehensive multi-file PSR-7 coverage.
Validates base name, trailing [] normalization, and indexed access returning distinct instances.src/http/UploadedFile.php (3)
273-301: PSR-7 error path avoids getStream(); OK.
Correctly preserves metadata and defaults size to int when null.
169-190: Array-name normalization and prefix matching look correct.
Coversname[], direct key, andname[index]forms; matches tests.
1-1: Ignore tempResource UnknownPropertyException concern
The parent class yii\web\UploadedFile already definespublic function __construct($config = []) { $this->_tempResource = ArrayHelper::remove($config, 'tempResource'); parent::__construct($config); }so ‘tempResource’ is removed before BaseObject handles config, preventing any UnknownPropertyException (yiiframework.com).
Likely an incorrect or invalid review comment.
…ent cross-request contamination.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/http/UploadedFile.php (1)
230-233: Close resources and clear cache when swapping PSR-7 adapter.If setPsr7Adapter() is called outside the app reset path, old detached handles persist and cache stales. Close and clear.
Apply:
- public static function setPsr7Adapter(ServerRequestAdapter $adapter): void - { - self::$psr7Adapter = $adapter; - } + public static function setPsr7Adapter(ServerRequestAdapter $adapter): void + { + self::closeResources(); + self::$_files = []; + self::$psr7Adapter = $adapter; + }
🧹 Nitpick comments (4)
src/http/UploadedFile.php (4)
238-245: Guard access to tempResource to avoid notices.Use isset() before is_resource() for robustness.
Apply:
- foreach (self::$_files as $entry) { - if (is_resource($entry['tempResource'])) { + foreach (self::$_files as $entry) { + if (isset($entry['tempResource']) && is_resource($entry['tempResource'])) { @fclose($entry['tempResource']); } }
324-335: Minor: simplify conditional.elseif (self::$psr7Adapter === null) is redundant after the previous branch.
Apply:
- if (self::$_files === []) { - if (self::$psr7Adapter !== null) { - self::loadPsr7Files(); - } elseif (self::$psr7Adapter === null) { - self::loadLegacyFiles(); - } - } + if (self::$_files === []) { + if (self::$psr7Adapter !== null) { + self::loadPsr7Files(); + } else { + self::loadLegacyFiles(); + } + }
353-387: Type safety: cast numeric fields.Ensure size/error are ints as per phpdoc and Yii expectations.
Apply:
- } elseif ($errors !== UPLOAD_ERR_NO_FILE) { + } elseif ($errors !== UPLOAD_ERR_NO_FILE) { self::$_files[$key] = [ 'name' => $names, 'tempName' => is_array($tempNames) ? '' : $tempNames, 'tempResource' => is_resource($tempResources) ? $tempResources : null, - 'type' => is_array($types) ? '' : $types, - 'size' => is_array($sizes) ? 0 : $sizes, - 'error' => is_array($errors) ? UPLOAD_ERR_NO_FILE : $errors, + 'type' => is_array($types) ? '' : $types, + 'size' => is_array($sizes) ? 0 : (int) $sizes, + 'error' => is_array($errors) ? UPLOAD_ERR_NO_FILE : (int) $errors, 'fullPath' => is_string($fullPaths) ? $fullPaths : null, ]; }
59-60: Encapsulate static cache and update tests
Make$_filesprivate insrc/http/UploadedFile.php(line 59) and expose it via an accessor (e.g.getFiles()/resetFiles()); update tests intests/http/UploadedFileTest.php(around line 194) to call the new accessor instead ofUploadedFile::$_files.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/http/StatelessApplication.php(4 hunks)src/http/UploadedFile.php(1 hunks)
🧰 Additional context used
🧠 Learnings (13)
📓 Common learnings
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#160
File: src/http/UploadedFile.php:41-41
Timestamp: 2025-08-28T22:07:08.430Z
Learning: In yii2-extensions/psr-bridge, the UploadedFile class extends \yii\web\UploadedFile which already handles saving from detached streams/resources in its saveAs() method. The parent class checks is_resource($this->_tempResource) and uses copyTempFile() with stream_copy_to_stream(). The PSR-7 bridge correctly passes detached streams via tempResource, so no override of saveAs() is needed.
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#160
File: src/http/UploadedFile.php:41-41
Timestamp: 2025-08-28T22:07:08.430Z
Learning: In yii2-extensions/psr-bridge, the UploadedFile class extends \yii\web\UploadedFile which already handles saving from detached streams/resources in its saveAs() method. The parent class checks is_resource($this->_tempResource) and uses copyTempFile() with stream_copy_to_stream(). The PSR-7 bridge correctly passes detached streams via tempResource, so no override of saveAs() is needed.
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#160
File: tests/http/UploadedFileTest.php:122-179
Timestamp: 2025-08-28T22:08:51.791Z
Learning: In yii2-extensions/psr-bridge, when the PSR-7 adapter is set in UploadedFile, it indicates worker mode (like RoadRunner, Swoole), where $_FILES is not meaningful and should not be checked as a fallback. The PSR-7 adapter becomes the authoritative source for uploaded files in this mode.
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#78
File: src/http/Request.php:0-0
Timestamp: 2025-08-10T13:17:04.265Z
Learning: In the yii2-extensions/psr-bridge project, PHPStan type annotations should be defensive when dealing with external input from PSR-7 sources. For methods like `getUploadedFiles()` and `convertPsr7ToUploadedFiles()`, use `mixed` in the return type annotation to account for untrusted input values, even if the implementation filters them.
📚 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:
src/http/StatelessApplication.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:
src/http/StatelessApplication.php
📚 Learning: 2025-08-28T22:08:51.791Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#160
File: tests/http/UploadedFileTest.php:122-179
Timestamp: 2025-08-28T22:08:51.791Z
Learning: In yii2-extensions/psr-bridge, when the PSR-7 adapter is set in UploadedFile, it indicates worker mode (like RoadRunner, Swoole), where $_FILES is not meaningful and should not be checked as a fallback. The PSR-7 adapter becomes the authoritative source for uploaded files in this mode.
Applied to files:
src/http/UploadedFile.php
📚 Learning: 2025-08-28T22:07:08.430Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#160
File: src/http/UploadedFile.php:41-41
Timestamp: 2025-08-28T22:07:08.430Z
Learning: In yii2-extensions/psr-bridge, the UploadedFile class extends \yii\web\UploadedFile which already handles saving from detached streams/resources in its saveAs() method. The parent class checks is_resource($this->_tempResource) and uses copyTempFile() with stream_copy_to_stream(). The PSR-7 bridge correctly passes detached streams via tempResource, so no override of saveAs() is needed.
Applied to files:
src/http/UploadedFile.php
📚 Learning: 2025-08-10T13:17:04.265Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#78
File: src/http/Request.php:0-0
Timestamp: 2025-08-10T13:17:04.265Z
Learning: In the yii2-extensions/psr-bridge project, PHPStan type annotations should be defensive when dealing with external input from PSR-7 sources. For methods like `getUploadedFiles()` and `convertPsr7ToUploadedFiles()`, use `mixed` in the return type annotation to account for untrusted input values, even if the implementation filters them.
Applied to files:
src/http/UploadedFile.php
📚 Learning: 2025-08-10T20:39:09.333Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#82
File: tests/adapter/UploadedFilesPsr7Test.php:197-248
Timestamp: 2025-08-10T20:39:09.333Z
Learning: In the yii2-extensions/psr-bridge project, the TestCase base class (tests/TestCase.php) automatically handles temporary file cleanup. The `createTmpFile()` method tracks files in `$this->tmpFiles[]`, and the `tearDown()` method calls `closeTmpFile()` to clean them up. Tests extending TestCase don't need manual file cleanup when using these methods.
Applied to files:
src/http/UploadedFile.php
📚 Learning: 2025-08-24T11:52:50.563Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#141
File: tests/http/stateless/ApplicationRoutingTest.php:1-164
Timestamp: 2025-08-24T11:52:50.563Z
Learning: In yii2-extensions/psr-bridge, tests that manipulate PHP superglobals ($_POST, $_GET, $_SERVER) in the http group do not require process isolation and work fine with the current PHPUnit configuration.
Applied to files:
src/http/UploadedFile.php
📚 Learning: 2025-08-10T20:39:09.333Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#82
File: tests/adapter/UploadedFilesPsr7Test.php:197-248
Timestamp: 2025-08-10T20:39:09.333Z
Learning: In the yii2-extensions/psr-bridge project, the TestCase base class configures the runtime path as `dirname(__DIR__) . '/runtime'` in the application configuration. Tests that need to save files to the runtime directory can rely on this pre-configured path.
Applied to files:
src/http/UploadedFile.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:
src/http/UploadedFile.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:
src/http/UploadedFile.php
📚 Learning: 2025-08-26T15:37:52.160Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#155
File: tests/http/stateless/ApplicationMemoryTest.php:198-211
Timestamp: 2025-08-26T15:37:52.160Z
Learning: In yii2-extensions/psr-bridge, tests can use the TestSupport trait from php-forge/support package which provides an invokeMethod() helper for calling protected/private methods during testing, eliminating the need for custom reflection code.
Applied to files:
src/http/UploadedFile.php
📚 Learning: 2025-08-08T15:24:06.085Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#71
File: tests/TestCase.php:23-27
Timestamp: 2025-08-08T15:24:06.085Z
Learning: In yii2-extensions/psr-bridge (tests/TestCase.php), maintainer preference: it’s acceptable to use random-looking strings for test-only constants like COOKIE_VALIDATION_KEY; no need to replace with an obviously non-secret value unless CI/secret scanners become problematic.
Applied to files:
src/http/UploadedFile.php
🧬 Code graph analysis (2)
src/http/StatelessApplication.php (1)
src/http/UploadedFile.php (2)
UploadedFile(41-476)reset(204-210)
src/http/UploadedFile.php (3)
src/adapter/ServerRequestAdapter.php (2)
ServerRequestAdapter(47-441)getUploadedFiles(323-326)src/http/StatelessApplication.php (1)
reset(401-442)src/http/Request.php (1)
getUploadedFiles(693-700)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: mutation / PHP 8.3-ubuntu-latest
- GitHub Check: mutation / PHP 8.3-ubuntu-latest
🔇 Additional comments (7)
src/http/StatelessApplication.php (4)
390-392: Doc update aligns with behavior.Reset now explicitly includes uploaded-file state. Good.
405-407: Right place to reset UploadedFile state.Calling UploadedFile::reset() before parent::__construct() prevents cross-request leakage in workers. LGTM.
447-447: Terminate() doc now matches implementation.Removal of upload reset note is correct since reset happens earlier.
12-12: Import is minimal and correct.Using the namespaced UploadedFile directly avoids an extra import.
src/http/UploadedFile.php (3)
270-298: PSR-7 error path handling is correct.Avoiding getStream() when error != UPLOAD_ERR_OK follows PSR-7. LGTM.
173-176: Nice: supports 'field[]' normalization.Trimming trailing [] matches common form naming.
200-210: Reset sequence is sound.Closing resources before clearing and nulling adapter prevents leaks. Good.
…names and array attributes.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
tests/http/UploadedFileTest.php(1 hunks)tests/support/stub/ComplexUploadedFileModel.php(1 hunks)tests/support/stub/UploadedFileModel.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/http/UploadedFileTest.php
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#160
File: src/http/UploadedFile.php:41-41
Timestamp: 2025-08-28T22:07:08.430Z
Learning: In yii2-extensions/psr-bridge, the UploadedFile class extends \yii\web\UploadedFile which already handles saving from detached streams/resources in its saveAs() method. The parent class checks is_resource($this->_tempResource) and uses copyTempFile() with stream_copy_to_stream(). The PSR-7 bridge correctly passes detached streams via tempResource, so no override of saveAs() is needed.
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#160
File: src/http/UploadedFile.php:41-41
Timestamp: 2025-08-28T22:07:08.430Z
Learning: In yii2-extensions/psr-bridge, the UploadedFile class extends \yii\web\UploadedFile which already handles saving from detached streams/resources in its saveAs() method. The parent class checks is_resource($this->_tempResource) and uses copyTempFile() with stream_copy_to_stream(). The PSR-7 bridge correctly passes detached streams via tempResource, so no override of saveAs() is needed.
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#160
File: tests/http/UploadedFileTest.php:122-179
Timestamp: 2025-08-28T22:08:51.791Z
Learning: In yii2-extensions/psr-bridge, when the PSR-7 adapter is set in UploadedFile, it indicates worker mode (like RoadRunner, Swoole), where $_FILES is not meaningful and should not be checked as a fallback. The PSR-7 adapter becomes the authoritative source for uploaded files in this mode.
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#78
File: src/http/Request.php:0-0
Timestamp: 2025-08-10T13:17:04.265Z
Learning: In the yii2-extensions/psr-bridge project, PHPStan type annotations should be defensive when dealing with external input from PSR-7 sources. For methods like `getUploadedFiles()` and `convertPsr7ToUploadedFiles()`, use `mixed` in the return type annotation to account for untrusted input values, even if the implementation filters them.
📚 Learning: 2025-08-28T22:08:51.791Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#160
File: tests/http/UploadedFileTest.php:122-179
Timestamp: 2025-08-28T22:08:51.791Z
Learning: In yii2-extensions/psr-bridge, when the PSR-7 adapter is set in UploadedFile, it indicates worker mode (like RoadRunner, Swoole), where $_FILES is not meaningful and should not be checked as a fallback. The PSR-7 adapter becomes the authoritative source for uploaded files in this mode.
Applied to files:
tests/support/stub/UploadedFileModel.php
📚 Learning: 2025-08-29T09:44:57.449Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#160
File: src/http/UploadedFile.php:110-115
Timestamp: 2025-08-29T09:44:57.449Z
Learning: In yii2-extensions/psr-bridge, the official Yii2 \yii\web\UploadedFile constructor already handles the 'tempResource' key in the config array and properly maps it to the $_tempResource protected property. Therefore, passing 'tempResource' in the config array to the UploadedFile constructor will not cause UnknownPropertyException.
Applied to files:
tests/support/stub/UploadedFileModel.php
📚 Learning: 2025-08-10T20:39:09.333Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#82
File: tests/adapter/UploadedFilesPsr7Test.php:197-248
Timestamp: 2025-08-10T20:39:09.333Z
Learning: In the yii2-extensions/psr-bridge project, the TestCase base class (tests/TestCase.php) automatically handles temporary file cleanup. The `createTmpFile()` method tracks files in `$this->tmpFiles[]`, and the `tearDown()` method calls `closeTmpFile()` to clean them up. Tests extending TestCase don't need manual file cleanup when using these methods.
Applied to files:
tests/support/stub/UploadedFileModel.php
📚 Learning: 2025-08-28T22:07:08.430Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#160
File: src/http/UploadedFile.php:41-41
Timestamp: 2025-08-28T22:07:08.430Z
Learning: In yii2-extensions/psr-bridge, the UploadedFile class extends \yii\web\UploadedFile which already handles saving from detached streams/resources in its saveAs() method. The parent class checks is_resource($this->_tempResource) and uses copyTempFile() with stream_copy_to_stream(). The PSR-7 bridge correctly passes detached streams via tempResource, so no override of saveAs() is needed.
Applied to files:
tests/support/stub/UploadedFileModel.php
🧬 Code graph analysis (2)
tests/support/stub/UploadedFileModel.php (1)
tests/support/stub/ComplexUploadedFileModel.php (1)
formName(13-16)
tests/support/stub/ComplexUploadedFileModel.php (1)
tests/support/stub/UploadedFileModel.php (1)
formName(13-16)
⏰ 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.3-ubuntu-latest
- GitHub Check: phpunit / PHP 8.4-windows-latest
- GitHub Check: mutation / PHP 8.3-ubuntu-latest
🔇 Additional comments (2)
tests/support/stub/UploadedFileModel.php (1)
13-16: formName() is appropriate and predictable for input-name mapping.The fixed return ensures stable
Html::getInputName()resolution. LGTM.tests/support/stub/ComplexUploadedFileModel.php (1)
13-16: formName() with special chars is good for edge‑case coverage.Useful to validate input-name canonicalization with underscores and hyphens. LGTM.
…to use `UploadedFile` type hinting for file attributes.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
tests/support/stub/UploadedFileModel.php (1)
8-15: Widen property type to base yii\web\UploadedFile for flexibilityTyping the property to the bridge subclass forces all callers/tests to use it exclusively. Since the bridge extends yii\web\UploadedFile, typing to the base accepts both legacy and bridge instances without losing functionality. This prevents accidental TypeError in tests that may still produce yii\web\UploadedFile. Based on the retrieved learnings, no bridge-only APIs are needed on this model.
Apply:
-use yii2\extensions\psrbridge\http\UploadedFile; +use yii\web\UploadedFile; @@ - /** - * @phpstan-var UploadedFile|UploadedFile[]|null - */ - public UploadedFile|array|null $file = null; + /** + * @phpstan-var UploadedFile|array<UploadedFile>|null + */ + public UploadedFile|array|null $file = null;
🧹 Nitpick comments (2)
tests/support/stub/UploadedFileModel.php (1)
12-15: Tighten phpstan annotation to generic array formUse array for element typing; it improves static analysis and matches the declared union.
Apply:
- * @phpstan-var UploadedFile|UploadedFile[]|null + * @phpstan-var UploadedFile|array<UploadedFile>|nulltests/support/stub/ComplexUploadedFileModel.php (1)
12-16: Property union is appropriate; minor docblock polish.
Runtime union is coarse (array) while docblock narrows element type—good. For broader IDE/tooling support, prefer@varwith generics over@phpstan-var.Apply:
- /** - * @phpstan-var UploadedFile|UploadedFile[]|null - */ + /** + * @var UploadedFile|array<int, UploadedFile>|null + */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
tests/http/UploadedFileTest.php(1 hunks)tests/support/stub/ComplexUploadedFileModel.php(1 hunks)tests/support/stub/UploadedFileModel.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/http/UploadedFileTest.php
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#160
File: src/http/UploadedFile.php:41-41
Timestamp: 2025-08-28T22:07:08.430Z
Learning: In yii2-extensions/psr-bridge, the UploadedFile class extends \yii\web\UploadedFile which already handles saving from detached streams/resources in its saveAs() method. The parent class checks is_resource($this->_tempResource) and uses copyTempFile() with stream_copy_to_stream(). The PSR-7 bridge correctly passes detached streams via tempResource, so no override of saveAs() is needed.
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#160
File: src/http/UploadedFile.php:41-41
Timestamp: 2025-08-28T22:07:08.430Z
Learning: In yii2-extensions/psr-bridge, the UploadedFile class extends \yii\web\UploadedFile which already handles saving from detached streams/resources in its saveAs() method. The parent class checks is_resource($this->_tempResource) and uses copyTempFile() with stream_copy_to_stream(). The PSR-7 bridge correctly passes detached streams via tempResource, so no override of saveAs() is needed.
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#160
File: tests/http/UploadedFileTest.php:122-179
Timestamp: 2025-08-28T22:08:51.791Z
Learning: In yii2-extensions/psr-bridge, when the PSR-7 adapter is set in UploadedFile, it indicates worker mode (like RoadRunner, Swoole), where $_FILES is not meaningful and should not be checked as a fallback. The PSR-7 adapter becomes the authoritative source for uploaded files in this mode.
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#78
File: src/http/Request.php:0-0
Timestamp: 2025-08-10T13:17:04.265Z
Learning: In the yii2-extensions/psr-bridge project, PHPStan type annotations should be defensive when dealing with external input from PSR-7 sources. For methods like `getUploadedFiles()` and `convertPsr7ToUploadedFiles()`, use `mixed` in the return type annotation to account for untrusted input values, even if the implementation filters them.
📚 Learning: 2025-08-28T22:08:51.791Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#160
File: tests/http/UploadedFileTest.php:122-179
Timestamp: 2025-08-28T22:08:51.791Z
Learning: In yii2-extensions/psr-bridge, when the PSR-7 adapter is set in UploadedFile, it indicates worker mode (like RoadRunner, Swoole), where $_FILES is not meaningful and should not be checked as a fallback. The PSR-7 adapter becomes the authoritative source for uploaded files in this mode.
Applied to files:
tests/support/stub/UploadedFileModel.phptests/support/stub/ComplexUploadedFileModel.php
📚 Learning: 2025-08-10T13:17:04.265Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#78
File: src/http/Request.php:0-0
Timestamp: 2025-08-10T13:17:04.265Z
Learning: In the yii2-extensions/psr-bridge project, PHPStan type annotations should be defensive when dealing with external input from PSR-7 sources. For methods like `getUploadedFiles()` and `convertPsr7ToUploadedFiles()`, use `mixed` in the return type annotation to account for untrusted input values, even if the implementation filters them.
Applied to files:
tests/support/stub/UploadedFileModel.phptests/support/stub/ComplexUploadedFileModel.php
📚 Learning: 2025-08-29T09:44:57.449Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#160
File: src/http/UploadedFile.php:110-115
Timestamp: 2025-08-29T09:44:57.449Z
Learning: In yii2-extensions/psr-bridge, the official Yii2 \yii\web\UploadedFile constructor already handles the 'tempResource' key in the config array and properly maps it to the $_tempResource protected property. Therefore, passing 'tempResource' in the config array to the UploadedFile constructor will not cause UnknownPropertyException.
Applied to files:
tests/support/stub/UploadedFileModel.php
📚 Learning: 2025-08-28T22:07:08.430Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#160
File: src/http/UploadedFile.php:41-41
Timestamp: 2025-08-28T22:07:08.430Z
Learning: In yii2-extensions/psr-bridge, the UploadedFile class extends \yii\web\UploadedFile which already handles saving from detached streams/resources in its saveAs() method. The parent class checks is_resource($this->_tempResource) and uses copyTempFile() with stream_copy_to_stream(). The PSR-7 bridge correctly passes detached streams via tempResource, so no override of saveAs() is needed.
Applied to files:
tests/support/stub/UploadedFileModel.php
📚 Learning: 2025-08-10T20:39:09.333Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#82
File: tests/adapter/UploadedFilesPsr7Test.php:197-248
Timestamp: 2025-08-10T20:39:09.333Z
Learning: In the yii2-extensions/psr-bridge project, the TestCase base class (tests/TestCase.php) automatically handles temporary file cleanup. The `createTmpFile()` method tracks files in `$this->tmpFiles[]`, and the `tearDown()` method calls `closeTmpFile()` to clean them up. Tests extending TestCase don't need manual file cleanup when using these methods.
Applied to files:
tests/support/stub/UploadedFileModel.php
🧬 Code graph analysis (2)
tests/support/stub/UploadedFileModel.php (2)
src/http/UploadedFile.php (1)
UploadedFile(41-476)tests/support/stub/ComplexUploadedFileModel.php (1)
formName(17-20)
tests/support/stub/ComplexUploadedFileModel.php (2)
src/http/UploadedFile.php (1)
UploadedFile(41-476)tests/support/stub/UploadedFileModel.php (1)
formName(17-20)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: mutation / PHP 8.3-ubuntu-latest
- GitHub Check: mutation / PHP 8.3-ubuntu-latest
🔇 Additional comments (3)
tests/support/stub/UploadedFileModel.php (1)
17-20: LGTM: Stable form nameExplicit formName is concise and predictable for tests.
tests/support/stub/ComplexUploadedFileModel.php (2)
3-6: Good: strict types and clear test-stub namespace.
Consistent with project style and avoids accidental scalar coercions.
7-9: Correct import: using the bridge UploadedFile.
Matches the PSR-7 bridge design; avoids mixing with yii\web\UploadedFile directly.
… with `ComplexUploadedFileModel` using PSR-7 adapter.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
tests/http/UploadedFileTest.php (3)
21-26: Also reset $_FILES to avoid cross-test contamination.A previous test may leave $_FILES populated; clearing it here prevents subtle bleed between legacy-mode tests.
protected function setUp(): void { parent::setUp(); - - UploadedFile::reset(); + // Ensure clean global/file state per test. + $_FILES = []; + UploadedFile::reset(); }
1494-1553: Avoid reaching into UploadedFile internals in this test.Directly reading UploadedFile::$_files couples the test to private state and can break with refactors. Prefer a small debug accessor or asserting via public API.
Minimal test change (uses a public debug helper):
- $resourcesBeforeReset = []; - - foreach (UploadedFile::$_files as $fileData) { - if (isset($fileData['tempResource']) && is_resource($fileData['tempResource'])) { - $resourcesBeforeReset[] = $fileData['tempResource']; - } - } + $resourcesBeforeReset = UploadedFile::debugOpenResources(); @@ - $stillOpenAfterReset = 0; - - foreach ($resourcesBeforeReset as $resource) { - if (is_resource($resource)) { - $stillOpenAfterReset++; - } - } + $stillOpenAfterReset = array_reduce( + $resourcesBeforeReset, + static fn (int $c, $r): int => $c + (is_resource($r) ? 1 : 0), + 0, + ); @@ self::assertSame( 0, $stillOpenAfterReset, "All resources should be closed after 'reset()' method.", ); + // Bonus: state should be cleared. + self::assertEmpty(UploadedFile::getInstancesByName('reset-test'));Add this test-only helper to src/http/UploadedFile.php:
// In UploadedFile class (public, but documented as test-only) public static function debugOpenResources(): array { $out = []; foreach (self::loadFiles() as $f) { if (isset($f['tempResource']) && is_resource($f['tempResource'])) { $out[] = $f['tempResource']; } } return $out; }I can open a follow-up PR with the helper guarded by @internal phpdoc if preferred.
19-20: Optional: add @Covers to tighten code coverage mapping.Helps mutation/coverage tools focus on the bridge surface.
+/** + * @covers \yii2\extensions\psrbridge\http\UploadedFile + * @covers \yii2\extensions\psrbridge\adapter\ServerRequestAdapter + */ final class UploadedFileTest extends TestCase
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
tests/http/UploadedFileTest.php(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#160
File: src/http/UploadedFile.php:41-41
Timestamp: 2025-08-28T22:07:08.430Z
Learning: In yii2-extensions/psr-bridge, the UploadedFile class extends \yii\web\UploadedFile which already handles saving from detached streams/resources in its saveAs() method. The parent class checks is_resource($this->_tempResource) and uses copyTempFile() with stream_copy_to_stream(). The PSR-7 bridge correctly passes detached streams via tempResource, so no override of saveAs() is needed.
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#160
File: src/http/UploadedFile.php:41-41
Timestamp: 2025-08-28T22:07:08.430Z
Learning: In yii2-extensions/psr-bridge, the UploadedFile class extends \yii\web\UploadedFile which already handles saving from detached streams/resources in its saveAs() method. The parent class checks is_resource($this->_tempResource) and uses copyTempFile() with stream_copy_to_stream(). The PSR-7 bridge correctly passes detached streams via tempResource, so no override of saveAs() is needed.
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#160
File: tests/http/UploadedFileTest.php:122-179
Timestamp: 2025-08-28T22:08:51.791Z
Learning: In yii2-extensions/psr-bridge, when the PSR-7 adapter is set in UploadedFile, it indicates worker mode (like RoadRunner, Swoole), where $_FILES is not meaningful and should not be checked as a fallback. The PSR-7 adapter becomes the authoritative source for uploaded files in this mode.
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#78
File: src/http/Request.php:0-0
Timestamp: 2025-08-10T13:17:04.265Z
Learning: In the yii2-extensions/psr-bridge project, PHPStan type annotations should be defensive when dealing with external input from PSR-7 sources. For methods like `getUploadedFiles()` and `convertPsr7ToUploadedFiles()`, use `mixed` in the return type annotation to account for untrusted input values, even if the implementation filters them.
📚 Learning: 2025-08-28T22:07:08.430Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#160
File: src/http/UploadedFile.php:41-41
Timestamp: 2025-08-28T22:07:08.430Z
Learning: In yii2-extensions/psr-bridge, the UploadedFile class extends \yii\web\UploadedFile which already handles saving from detached streams/resources in its saveAs() method. The parent class checks is_resource($this->_tempResource) and uses copyTempFile() with stream_copy_to_stream(). The PSR-7 bridge correctly passes detached streams via tempResource, so no override of saveAs() is needed.
Applied to files:
tests/http/UploadedFileTest.php
📚 Learning: 2025-08-28T22:08:51.791Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#160
File: tests/http/UploadedFileTest.php:122-179
Timestamp: 2025-08-28T22:08:51.791Z
Learning: In yii2-extensions/psr-bridge, when the PSR-7 adapter is set in UploadedFile, it indicates worker mode (like RoadRunner, Swoole), where $_FILES is not meaningful and should not be checked as a fallback. The PSR-7 adapter becomes the authoritative source for uploaded files in this mode.
Applied to files:
tests/http/UploadedFileTest.php
📚 Learning: 2025-08-10T20:39:09.333Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#82
File: tests/adapter/UploadedFilesPsr7Test.php:197-248
Timestamp: 2025-08-10T20:39:09.333Z
Learning: In the yii2-extensions/psr-bridge project, the TestCase base class (tests/TestCase.php) automatically handles temporary file cleanup. The `createTmpFile()` method tracks files in `$this->tmpFiles[]`, and the `tearDown()` method calls `closeTmpFile()` to clean them up. Tests extending TestCase don't need manual file cleanup when using these methods.
Applied to files:
tests/http/UploadedFileTest.php
📚 Learning: 2025-08-10T13:17:04.265Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#78
File: src/http/Request.php:0-0
Timestamp: 2025-08-10T13:17:04.265Z
Learning: In the yii2-extensions/psr-bridge project, PHPStan type annotations should be defensive when dealing with external input from PSR-7 sources. For methods like `getUploadedFiles()` and `convertPsr7ToUploadedFiles()`, use `mixed` in the return type annotation to account for untrusted input values, even if the implementation filters them.
Applied to files:
tests/http/UploadedFileTest.php
🧬 Code graph analysis (1)
tests/http/UploadedFileTest.php (3)
src/adapter/ServerRequestAdapter.php (1)
ServerRequestAdapter(47-441)src/http/UploadedFile.php (7)
UploadedFile(41-476)reset(204-210)setPsr7Adapter(230-233)getInstanceByName(110-115)getInstances(141-146)getInstance(87-92)getInstancesByName(169-190)tests/support/FactoryHelper.php (3)
FactoryHelper(46-268)createUploadedFile(227-235)createStream(191-194)
⏰ 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.3-ubuntu-latest
- GitHub Check: phpunit / PHP 8.1-windows-latest
- GitHub Check: phpunit / PHP 8.4-windows-latest
- GitHub Check: mutation / PHP 8.3-ubuntu-latest
🔇 Additional comments (3)
tests/http/UploadedFileTest.php (3)
1306-1350: LGTM: clear separation of legacy mode.Good assertion that, with no PSR-7 adapter set, legacy $_FILES loading returns a proper UploadedFile instance.
1606-1710: LGTM: bracket/addressing semantics well covered.Solid coverage for PSR‑7 multiple files: base name, trailing [] notation, and indexed access with distinct instances.
1352-1492: LGTM: robust coverage for ragged arrays.Nice validation of defaults when types/sizes arrays have missing indices and mixed errors.
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
New Features
Bug Fixes
Tests
Chores