Throw error if disk cannot be accessed (#3067)

* test error is thrown and no records in db * Create DiskCannotBeAccessed.php * add transcation * fix ci test * Update DiskCannotBeAccessed.php * remove database transactions * minor test updates as requested Co-authored-by: Freek Van der Herten <freek@spatie.be> Co-authored-by: Patrick Organ <patinthehat@users.noreply.github.com>
spatie · Nov 27, 2022 · 4acff99 · 4acff99
1 parent c7af276
commit 4acff99
Show file tree

Hide file tree

Showing 4 changed files with 66 additions and 9 deletions.
diff --git a/src/MediaCollections/Exceptions/DiskCannotBeAccessed.php b/src/MediaCollections/Exceptions/DiskCannotBeAccessed.php
@@ -0,0 +1,11 @@
+<?php
+
+namespace Spatie\MediaLibrary\MediaCollections\Exceptions;
+
+class DiskCannotBeAccessed extends FileCannotBeAdded
+{
+    public static function create(string $diskName): self
+    {
+        return new static("Disk named `{$diskName}` cannot be accessed");
+    }
+}
diff --git a/src/MediaCollections/FileAdder.php b/src/MediaCollections/FileAdder.php
@@ -8,6 +8,7 @@
 use Illuminate\Support\Traits\Macroable;
 use Spatie\MediaLibrary\Conversions\ImageGenerators\Image as ImageGenerator;
 use Spatie\MediaLibrary\HasMedia;
+use Spatie\MediaLibrary\MediaCollections\Exceptions\DiskCannotBeAccessed;
 use Spatie\MediaLibrary\MediaCollections\Exceptions\DiskDoesNotExist;
 use Spatie\MediaLibrary\MediaCollections\Exceptions\FileDoesNotExist;
 use Spatie\MediaLibrary\MediaCollections\Exceptions\FileIsTooBig;
@@ -440,9 +441,15 @@ protected function processMediaItem(HasMedia $model, Media $media, self $fileAdd
         $model->media()->save($media);
 
         if ($fileAdder->file instanceof RemoteFile) {
-            $this->filesystem->addRemote($fileAdder->file, $media, $fileAdder->fileName);
+            $addedMediaSuccessfully = $this->filesystem->addRemote($fileAdder->file, $media, $fileAdder->fileName);
         } else {
-            $this->filesystem->add($fileAdder->pathToFile, $media, $fileAdder->fileName);
+            $addedMediaSuccessfully = $this->filesystem->add($fileAdder->pathToFile, $media, $fileAdder->fileName);
+        }
+
+        if (! $addedMediaSuccessfully) {
+            $model->media()->delete($media->id);
+
+            throw DiskCannotBeAccessed::create($media->disk);
         }
 
         if (! $fileAdder->preserveOriginal) {
@@ -539,4 +546,4 @@ protected function appendExtension(string $file, ?string $extension): string
             ? $file . '.' . $extension
             : $file;
     }
-}
+}
diff --git a/src/MediaCollections/Filesystem.php b/src/MediaCollections/Filesystem.php
@@ -9,6 +9,7 @@
 use Spatie\MediaLibrary\Conversions\ConversionCollection;
 use Spatie\MediaLibrary\Conversions\FileManipulator;
 use Spatie\MediaLibrary\MediaCollections\Events\MediaHasBeenAdded;
+use Spatie\MediaLibrary\MediaCollections\Exceptions\DiskCannotBeAccessed;
 use Spatie\MediaLibrary\MediaCollections\Models\Media;
 use Spatie\MediaLibrary\Support\File;
 use Spatie\MediaLibrary\Support\PathGenerator\PathGeneratorFactory;
@@ -23,22 +24,34 @@ public function __construct(
     ) {
     }
 
-    public function add(string $file, Media $media, ?string $targetFileName = null): void
+    public function add(string $file, Media $media, ?string $targetFileName = null): bool
     {
-        $this->copyToMediaLibrary($file, $media, null, $targetFileName);
+        try {
+            $this->copyToMediaLibrary($file, $media, null, $targetFileName);
+        } catch(DiskCannotBeAccessed $exception) {
+            return false;
+        }
 
         event(new MediaHasBeenAdded($media));
 
         app(FileManipulator::class)->createDerivedFiles($media);
+
+        return true;
     }
 
-    public function addRemote(RemoteFile $file, Media $media, ?string $targetFileName = null): void
+    public function addRemote(RemoteFile $file, Media $media, ?string $targetFileName = null): bool
     {
-        $this->copyToMediaLibraryFromRemote($file, $media, null, $targetFileName);
+        try {
+            $this->copyToMediaLibraryFromRemote($file, $media, null, $targetFileName);
+        } catch(DiskCannotBeAccessed $exception) {
+            return false;
+        }
 
         event(new MediaHasBeenAdded($media));
 
         app(FileManipulator::class)->createDerivedFiles($media);
+
+        return true;
     }
 
     public function copyToMediaLibraryFromRemote(RemoteFile $file, Media $media, ?string $type = null, ?string $targetFileName = null): void
@@ -129,16 +142,20 @@ public function copyToMediaLibrary(string $pathToFile, Media $media, ?string $ty
             : $media->getDiskDriverName();
 
         if ($diskDriverName === 'local') {
-            $this->filesystem
+            $success = $this->filesystem
                 ->disk($diskName)
                 ->put($destination, $file);
 
             fclose($file);
 
+            if (! $success) {
+                throw DiskCannotBeAccessed::create($diskName);
+            }
+
             return;
         }
 
-        $this->filesystem
+        $success = $this->filesystem
             ->disk($diskName)
             ->put(
                 $destination,
@@ -149,6 +166,10 @@ public function copyToMediaLibrary(string $pathToFile, Media $media, ?string $ty
         if (is_resource($file)) {
             fclose($file);
         }
+
+        if (! $success) {
+            throw DiskCannotBeAccessed::create($diskName);
+        }
     }
 
     public function addCustomRemoteHeaders(array $customRemoteHeaders): void

diff --git a/tests/Feature/FileAdder/IntegrationTest.php b/tests/Feature/FileAdder/IntegrationTest.php
@@ -2,6 +2,7 @@
 
 use Illuminate\Support\Facades\Storage;
 use Spatie\MediaLibrary\MediaCollections\Exceptions\DiskDoesNotExist;
+use Spatie\MediaLibrary\MediaCollections\Exceptions\DiskCannotBeAccessed;
 use Spatie\MediaLibrary\MediaCollections\Exceptions\FileDoesNotExist;
 use Spatie\MediaLibrary\MediaCollections\Exceptions\FileIsTooBig;
 use Spatie\MediaLibrary\MediaCollections\Exceptions\InvalidBase64Data;
@@ -10,6 +11,7 @@
 use Spatie\MediaLibrary\MediaCollections\Exceptions\RequestDoesNotHaveFile;
 use Spatie\MediaLibrary\MediaCollections\Exceptions\UnknownType;
 use Spatie\MediaLibrary\MediaCollections\Exceptions\UnreachableUrl;
+use Spatie\MediaLibrary\MediaCollections\Models\Media;
 use Spatie\MediaLibrary\Tests\TestSupport\RenameOriginalFileNamer;
 use Symfony\Component\HttpFoundation\File\UploadedFile;
 
@@ -594,3 +596,19 @@
 
     expect($result->getStatusCode())->toEqual(200);
 });
+
+it('will throw and exception and not create a record in database if file cannot be added', function () {
+    config()->set('filesystems.disks.invalid_disk', [
+        'driver' => 's3',
+        'secret' => 'test',
+        'key' => 'test',
+        'region' => 'test',
+        'bucket' => 'test',
+    ]);
+
+    $this->testModel
+        ->addMedia($this->getTestJpg())
+        ->toMediaCollection('default', 'invalid_disk');
+
+    expect(Media::count())->toBe(0);
+})->throws(DiskCannotBeAccessed::class);