Commit 7622238b authored by Mahmoud Aglan's avatar Mahmoud Aglan

fix(expenses): read the upload's metadata before store() moves it away

This is the actual reason no receipt could be attached — 1b62a44f-… and
every other support code from the expense pages:

    UnableToRetrieveMetadata: Unable to retrieve the file_size for file
    at location: livewire-tmp/adbO73…jpg
    ExpenseShow.php(93): TemporaryUploadedFile->getSize()

store() moves the file out of livewire-tmp. The attachment array put
'path' => $file->store(…) on its first line, and PHP evaluates array
literals in order, so getMimeType() and getSize() on the lines below ran
against a path that had just stopped existing. Validation passed, the
vanished-upload guard passed, the file was even written to its final
home — and then the request died with a 500 on the way to the row.
Nothing was ever saved.

Both handlers now read name, mime and size into locals first and store
last. Reproduced against a real local disk before and after; Livewire's
test harness swaps in a temp disk that does not move the file, which is
why a component test would have gone green on the broken code, so the
regression guard asserts the source ordering instead.
Co-Authored-By: 's avatarClaude Opus 5 (1M context) <noreply@anthropic.com>
parent 32e52a04
...@@ -86,12 +86,19 @@ public function save(ExpenseService $service): void ...@@ -86,12 +86,19 @@ public function save(ExpenseService $service): void
$attachment = []; $attachment = [];
if ($this->attachment) { if ($this->attachment) {
// Metadata first: store() moves the file out of livewire-tmp,
// so any getter called after it reads a path that is gone and
// throws UnableToRetrieveMetadata. See ExpenseShow.
$name = $this->attachment->getClientOriginalName();
$mime = $this->attachment->getMimeType();
$size = $this->attachment->getSize();
$attachment = [ $attachment = [
'attachment_path' => $this->attachment->store('expenses/receipts', 'local'), 'attachment_path' => $this->attachment->store('expenses/receipts', 'local'),
'attachment_name' => $this->attachment->getClientOriginalName(), 'attachment_name' => $name,
'attachment_disk' => 'local', 'attachment_disk' => 'local',
'attachment_mime' => $this->attachment->getMimeType(), 'attachment_mime' => $mime,
'attachment_size' => $this->attachment->getSize(), 'attachment_size' => $size,
]; ];
} }
......
...@@ -85,12 +85,22 @@ public function uploadReceipt(ExpenseService $service): void ...@@ -85,12 +85,22 @@ public function uploadReceipt(ExpenseService $service): void
$this->validate(); $this->validate();
try { try {
// Read the metadata BEFORE storing. store() moves the file out of
// livewire-tmp, and every getter after it goes back to a path that
// no longer exists — getSize() threw Flysystem's
// UnableToRetrieveMetadata and the desk got an error page on every
// single receipt (1b62a44f-…, 2026-09-03). Array literals evaluate
// in order, so 'path' first was the whole bug.
$name = $this->receipt->getClientOriginalName();
$mime = $this->receipt->getMimeType();
$size = $this->receipt->getSize();
$service->attachReceipt($this->expense(), [ $service->attachReceipt($this->expense(), [
'path' => $this->receipt->store('expenses/receipts', 'local'), 'path' => $this->receipt->store('expenses/receipts', 'local'),
'name' => $this->receipt->getClientOriginalName(), 'name' => $name,
'disk' => 'local', 'disk' => 'local',
'mime' => $this->receipt->getMimeType(), 'mime' => $mime,
'size' => $this->receipt->getSize(), 'size' => $size,
], auth()->user()); ], auth()->user());
$this->receipt = null; $this->receipt = null;
......
<?php
namespace Tests\Feature;
use PHPUnit\Framework\Attributes\DataProvider;
use Tests\TestCase;
/**
* store() moves a Livewire temporary upload out of livewire-tmp, so anything
* that reads the file afterwards reads a path that no longer exists.
*
* This is what made every expense receipt fail in production (1b62a44f-…,
* 2026-09-03): the attachment array had `'path' => …->store(…)` on its first
* line, PHP evaluates array literals in order, and getMimeType()/getSize() on
* the lines below then ran against a file that had just been moved — throwing
* Flysystem's UnableToRetrieveMetadata all the way to the browser.
*
* Livewire's test harness swaps the temporary disk for one that does not move
* the file, so a component test goes green on the broken code. The invariant
* that actually holds is about the source: every metadata read comes before
* the store() that invalidates it.
*/
class TemporaryUploadMetadataOrderTest extends TestCase
{
public static function uploadHandlers(): array
{
return [
// Literal paths, not app_path(): the provider runs before the
// application container is booted.
'expense create form' => [dirname(__DIR__, 2) . '/app/Livewire/Financial/ExpenseForm.php'],
'expense receipt upload' => [dirname(__DIR__, 2) . '/app/Livewire/Financial/ExpenseShow.php'],
];
}
#[DataProvider('uploadHandlers')]
public function test_upload_metadata_is_read_before_the_file_is_stored(string $file): void
{
$source = file_get_contents($file);
$store = strpos($source, "->store('expenses/receipts'");
$this->assertNotFalse($store, "No receipt store() found in {$file}");
foreach (['getSize()', 'getMimeType()', 'getClientOriginalName()'] as $read) {
$at = strpos($source, $read);
$this->assertNotFalse($at, "{$read} missing from {$file}");
$this->assertLessThan(
$store,
$at,
"{$read} runs after store() in {$file} — the temporary file is already gone by then."
);
}
}
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment