Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Throw error if disk cannot be accessed #3067

Merged
merged 7 commits into from
Nov 27, 2022
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions src/MediaCollections/Exceptions/DiskCannotBeAccessed.php
Original file line number Diff line number Diff line change
@@ -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");
}
}
13 changes: 10 additions & 3 deletions src/MediaCollections/FileAdder.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -438,9 +439,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) {
Expand Down Expand Up @@ -533,4 +540,4 @@ protected function appendExtension(string $file, ?string $extension): string
? $file . '.' . $extension
: $file;
}
}
}
33 changes: 27 additions & 6 deletions src/MediaCollections/Filesystem.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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
Expand Down
21 changes: 21 additions & 0 deletions tests/Feature/FileAdder/IntegrationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -594,3 +596,22 @@

expect($result->getStatusCode())->toEqual(200);
});

it('will throw and exception and not create a record in database if file cannot be added', function () {

$this->expectException(DiskCannotBeAccessed::class);
patinthehat marked this conversation as resolved.
Show resolved Hide resolved

config()->set('filesystems.disks.minio_disk', [
patinthehat marked this conversation as resolved.
Show resolved Hide resolved
'driver' => 's3',
'secret' => 'test',
'key' => 'test',
'region' => 'test',
'bucket' => 'test',
]);

$this->testModel
->addMedia($this->getTestJpg())
->toMediaCollection('default', 'minio_disk');

expect(Media::count())->toBe(0);
});