image = $new; // unconditional * * If put returned false (silent-fail mode on the default local disk), the * old file was destroyed and the model row ended up referencing a file * that never landed on disk. The fix captures the put return, only * proceeds to delete + reassign when the write succeeded, and mirrors the * same check inside deleteExistingImage so a failed delete does not null * out the model reference either. * * We exercise the fix through the manufacturers update endpoint because it * has a straightforward image field and the fewest confounding variables * of the many handleImages callers, but the fix applies to every model * whose controller ultimately routes through ImageUploadRequest. */ class ImageUploadRequestFailurePathsTest extends TestCase { public function test_failed_new_image_write_preserves_existing_image_reference(): void { Storage::fake('public'); $publicDisk = Storage::disk('public'); // Seed a real "existing image" file so the test can assert it still // exists on disk after the failed write. $publicDisk->put('manufacturers/manufacturer-image-pre-existing.png', 'original bytes'); $manufacturer = Manufacturer::factory()->create([ 'image' => 'manufacturer-image-pre-existing.png', ]); // Proxy the public disk so put() returns false (simulating a write // failure that Storage does not throw on) while every other call // still passes through to the real fake disk. $proxy = Mockery::mock($publicDisk); $proxy->shouldReceive('put')->andReturn(false); $proxy->shouldReceive('exists')->andReturnUsing(fn (...$a) => $publicDisk->exists(...$a)); $proxy->shouldReceive('delete')->andReturnUsing(fn (...$a) => $publicDisk->delete(...$a)); $proxy->shouldReceive('makeDirectory')->andReturnUsing(fn (...$a) => $publicDisk->makeDirectory(...$a)); Storage::shouldReceive('disk')->with('public')->andReturn($proxy); $this->actingAs(User::factory()->superuser()->create()) ->put(route('manufacturers.update', $manufacturer), [ 'name' => $manufacturer->name, 'image' => UploadedFile::fake()->image('replacement.png'), ]) ->assertStatus(302) ->assertSessionHasNoErrors(); $manufacturer->refresh(); $this->assertEquals( 'manufacturer-image-pre-existing.png', $manufacturer->image, 'A failed new-image write must not clear or replace the existing image reference on the model.', ); $this->assertTrue( $publicDisk->exists('manufacturers/manufacturer-image-pre-existing.png'), 'A failed new-image write must not delete the existing image from disk.', ); } public function test_failed_delete_preserves_model_reference(): void { // Directly exercises ImageUploadRequest::deleteExistingImage in // isolation because the ManufacturersController path nulls the // model reference itself before handing off to handleImages (see // ManufacturersController::update line 153). The controller-level // pre-null defeats the deleteExistingImage guard for that specific // caller. Every other caller (ProfileController::update for the // user avatar, and any future callsite that relies on // deleteExistingImage to null the field) benefits from the guard. Storage::fake('public'); $publicDisk = Storage::disk('public'); $publicDisk->put('manufacturers/manufacturer-image-still-there.png', 'still here'); $manufacturer = Manufacturer::factory()->create([ 'image' => 'manufacturer-image-still-there.png', ]); $proxy = Mockery::mock($publicDisk); $proxy->shouldReceive('delete')->andReturn(false); $proxy->shouldReceive('exists')->andReturnUsing(fn (...$a) => $publicDisk->exists(...$a)); $proxy->shouldReceive('put')->andReturnUsing(fn (...$a) => $publicDisk->put(...$a)); $proxy->shouldReceive('makeDirectory')->andReturnUsing(fn (...$a) => $publicDisk->makeDirectory(...$a)); Storage::shouldReceive('disk')->with('public')->andReturn($proxy); $request = app(\App\Http\Requests\ImageUploadRequest::class); $result = $request->deleteExistingImage($manufacturer, 'manufacturers', 'image'); $this->assertEquals( 'manufacturer-image-still-there.png', $result->image, 'A failed delete must not null the model image reference - the file may still be on disk and orphaning it is worse than leaving the reference intact.', ); } public function test_successful_write_still_replaces_existing_image(): void { // Sanity: the fix must not regress the happy path. Successful writes // continue to delete the old file and update the model reference. Storage::fake('public'); $publicDisk = Storage::disk('public'); $publicDisk->put('manufacturers/manufacturer-image-old.png', 'old bytes'); $manufacturer = Manufacturer::factory()->create([ 'image' => 'manufacturer-image-old.png', ]); $this->actingAs(User::factory()->superuser()->create()) ->put(route('manufacturers.update', $manufacturer), [ 'name' => $manufacturer->name, 'image' => UploadedFile::fake()->image('new.png'), ]) ->assertStatus(302) ->assertSessionHasNoErrors(); $manufacturer->refresh(); $this->assertNotEquals('manufacturer-image-old.png', $manufacturer->image); $this->assertNotNull($manufacturer->image); $this->assertStringContainsString('Manufacturer-image', $manufacturer->image); $this->assertFalse( $publicDisk->exists('manufacturers/manufacturer-image-old.png'), 'On a successful new write, the old image file is deleted from disk.', ); $this->assertTrue( $publicDisk->exists('manufacturers/'.$manufacturer->image), 'On a successful new write, the new file lands at the expected path.', ); } }