From 46dc8fb9a0ce86836fd9f91d0ca4761c7f99f256 Mon Sep 17 00:00:00 2001 From: Styx Date: Mon, 21 Oct 2024 23:43:58 +0200 Subject: [PATCH 1/2] Removed unnecessary interface implementation and constructor from `FileBaseFileRemover` (already declared in parent class) --- src/Support/FileRemover/FileBaseFileRemover.php | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/Support/FileRemover/FileBaseFileRemover.php b/src/Support/FileRemover/FileBaseFileRemover.php index 4e365398d..9afcd998e 100644 --- a/src/Support/FileRemover/FileBaseFileRemover.php +++ b/src/Support/FileRemover/FileBaseFileRemover.php @@ -2,14 +2,10 @@ namespace Spatie\MediaLibrary\Support\FileRemover; -use Illuminate\Contracts\Filesystem\Factory; -use Spatie\MediaLibrary\MediaCollections\Filesystem; use Spatie\MediaLibrary\MediaCollections\Models\Media; -class FileBaseFileRemover extends DefaultFileRemover implements FileRemover +class FileBaseFileRemover extends DefaultFileRemover { - public function __construct(protected Filesystem $mediaFileSystem, protected Factory $filesystem) {} - public function removeAllFiles(Media $media): void { $this->removeFile($this->mediaFileSystem->getMediaDirectory($media).$media->file_name, $media->disk); From ddc4a75e739f64fa95ab63fc44f143239581aded Mon Sep 17 00:00:00 2001 From: Styx Date: Wed, 23 Oct 2024 18:56:47 +0200 Subject: [PATCH 2/2] Default file remover should honor custom file namer. --- src/MediaCollections/Filesystem.php | 8 ++- .../RegisteredResponsiveImages.php | 5 ++ .../FileRemover/DefaultFileRemover.php | 59 +++++++++---------- tests/Feature/Media/DeleteTest.php | 38 ++++++++++++ .../TestModelWithResponsiveImages.php | 6 +- 5 files changed, 80 insertions(+), 36 deletions(-) diff --git a/src/MediaCollections/Filesystem.php b/src/MediaCollections/Filesystem.php index a3aed27ab..dbe3830cf 100644 --- a/src/MediaCollections/Filesystem.php +++ b/src/MediaCollections/Filesystem.php @@ -11,6 +11,7 @@ use Spatie\MediaLibrary\MediaCollections\Exceptions\DiskCannotBeAccessed; use Spatie\MediaLibrary\MediaCollections\Models\Media; use Spatie\MediaLibrary\Support\File; +use Spatie\MediaLibrary\Support\FileNamer\FileNamer; use Spatie\MediaLibrary\Support\FileRemover\FileRemoverFactory; use Spatie\MediaLibrary\Support\PathGenerator\PathGeneratorFactory; use Spatie\MediaLibrary\Support\RemoteFile; @@ -235,14 +236,17 @@ public function removeFile(Media $media, string $path): void public function removeResponsiveImages(Media $media, string $conversionName = 'media_library_original'): void { + /** @var FileNamer $fileNamer */ + $fileNamer = app(config('media-library.file_namer')); + $mediaFilename = $fileNamer->responsiveFileName($media->name); + $responsiveImagesDirectory = $this->getResponsiveImagesDirectory($media); $allFilePaths = $this->filesystem->disk($media->disk)->allFiles($responsiveImagesDirectory); $responsiveImagePaths = array_filter( $allFilePaths, - fn (string $path) => Str::contains($path, $media->name.'___'.$conversionName) - + static fn (string $path) => Str::contains($path, $mediaFilename.'___'.$conversionName) ); $this->filesystem->disk($media->disk)->delete($responsiveImagePaths); diff --git a/src/ResponsiveImages/RegisteredResponsiveImages.php b/src/ResponsiveImages/RegisteredResponsiveImages.php index cadb0c31c..a628c5322 100644 --- a/src/ResponsiveImages/RegisteredResponsiveImages.php +++ b/src/ResponsiveImages/RegisteredResponsiveImages.php @@ -30,6 +30,11 @@ public function getUrls(): array ->toArray(); } + public function getFilenames(): array + { + return $this->files->pluck('fileName')->toArray(); + } + public function getSrcset(): string { $filesSrcset = $this->files diff --git a/src/Support/FileRemover/DefaultFileRemover.php b/src/Support/FileRemover/DefaultFileRemover.php index b743cfd94..5d4b5d51c 100644 --- a/src/Support/FileRemover/DefaultFileRemover.php +++ b/src/Support/FileRemover/DefaultFileRemover.php @@ -7,6 +7,9 @@ use Illuminate\Support\Str; use Spatie\MediaLibrary\MediaCollections\Filesystem; use Spatie\MediaLibrary\MediaCollections\Models\Media; +use Spatie\MediaLibrary\Support\FileNamer\FileNamer; +use Spatie\MediaLibrary\Support\PathGenerator\PathGeneratorFactory; +use Spatie\MediaLibrary\Support\UrlGenerator\UrlGeneratorFactory; class DefaultFileRemover implements FileRemover { @@ -36,9 +39,7 @@ public function removeFromMediaDirectory(Media $media, string $disk): void $allFilePaths = $this->filesystem->disk($disk)->allFiles($directory); $imagePaths = array_filter( $allFilePaths, - function (string $path) use ($media) { - return Str::afterLast($path, '/') === $media->file_name; - } + static fn (string $path) => Str::afterLast($path, '/') === $media->file_name ); foreach ($imagePaths as $imagePath) { $this->filesystem->disk($disk)->delete($imagePath); @@ -62,21 +63,12 @@ public function removeFromConversionsDirectory(Media $media, string $disk): void ->each(function (string $directory) use ($media, $disk) { try { $allFilePaths = $this->filesystem->disk($disk)->allFiles($directory); - - $conversions = array_keys($media->generated_conversions ?? []); - - $imagePaths = array_filter( - $allFilePaths, - function (string $path) use ($conversions, $media) { - foreach ($conversions as $conversion) { - if (Str::contains($path, pathinfo($media->file_name, PATHINFO_FILENAME).'-'.$conversion)) { - return true; - } - } - - return false; - } + $conversions = $media->getMediaConversionNames() ?: []; + $conversionsFilePaths = array_map( + static fn (string $conversion) => $media->getPathRelativeToRoot($conversion), + $conversions, ); + $imagePaths = array_intersect($allFilePaths, $conversionsFilePaths); foreach ($imagePaths as $imagePath) { $this->filesystem->disk($disk)->delete($imagePath); } @@ -93,28 +85,31 @@ function (string $path) use ($conversions, $media) { public function removeFromResponsiveImagesDirectory(Media $media, string $disk): void { $responsiveImagesDirectory = $this->mediaFileSystem->getMediaDirectory($media, 'responsiveImages'); + $mediaRoot = PathGeneratorFactory::create($media)->getPathForResponsiveImages($media); + /** @var FileNamer $fileNamer */ + $fileNamer = app(config('media-library.file_namer')); + $mediaFilename = $fileNamer->responsiveFileName($media->file_name); collect([$responsiveImagesDirectory]) ->unique() - ->each(function (string $directory) use ($media, $disk) { + ->each(function (string $directory) use ($media, $disk, $mediaRoot, $mediaFilename) { try { $allFilePaths = $this->filesystem->disk($disk)->allFiles($directory); - $conversions = array_keys($media->generated_conversions ?? []); - $conversions[] = 'media_library_original'; - - $imagePaths = array_filter( - $allFilePaths, - function (string $path) use ($conversions, $media) { - foreach ($conversions as $conversion) { - if (Str::contains($path, pathinfo($media->file_name, PATHINFO_FILENAME).'___'.$conversion)) { - return true; - } - } - - return false; - } + $conversions = $media->getMediaConversionNames() ?: []; + $responsiveImagesFilePaths = collect($conversions) + ->flatMap(static fn (string $conversion) => $media->responsiveImages($conversion)->getFilenames()) + ->map(static fn (string $imagePath) => $mediaRoot.$imagePath) + ->toArray(); + + $imagePaths = array_merge( + array_intersect($allFilePaths, $responsiveImagesFilePaths), + array_filter( + $allFilePaths, + static fn (string $path) => Str::startsWith($path, $mediaRoot.$mediaFilename.'___media_library_original_'), + ), ); + foreach ($imagePaths as $imagePath) { $this->filesystem->disk($disk)->delete($imagePath); } diff --git a/tests/Feature/Media/DeleteTest.php b/tests/Feature/Media/DeleteTest.php index 55df0b142..d2faf29fe 100644 --- a/tests/Feature/Media/DeleteTest.php +++ b/tests/Feature/Media/DeleteTest.php @@ -5,8 +5,10 @@ use Illuminate\Support\Facades\Storage; use Spatie\MediaLibrary\MediaCollections\Models\Media; use Spatie\MediaLibrary\Support\FileRemover\FileBaseFileRemover; +use Spatie\MediaLibrary\Support\PathGenerator\DefaultPathGenerator; use Spatie\MediaLibrary\Tests\Support\PathGenerator\CustomDirectoryStructurePathGenerator; use Spatie\MediaLibrary\Tests\TestSupport\TestCustomPathGenerator; +use Spatie\MediaLibrary\Tests\TestSupport\TestFileNamer; use Spatie\MediaLibrary\Tests\TestSupport\TestModels\TestModel; use Spatie\MediaLibrary\Tests\TestSupport\TestPathGenerator; @@ -140,6 +142,42 @@ expect(File::exists($media2->getPath()))->toBeTrue(); }); +it('will remove conversion files when using custom file namer', function () { + config(['media-library.path_generator' => DefaultPathGenerator::class]); + config(['media-library.file_namer' => TestFileNamer::class]); + + $media = $this->testModelWithConversion->addMedia($this->getTestJpg())->toMediaCollection('images'); + + expect(File::exists($media->getPath('thumb')))->toBeTrue(); + expect(File::exists($media->getPath('keep_original_format')))->toBeTrue(); + + $media->delete(); + + expect(File::exists($media->getPath()))->toBeFalse(); + expect(File::exists($media->getPath('thumb')))->toBeFalse(); + expect(File::exists($media->getPath('keep_original_format')))->toBeFalse(); + expect(File::exists($this->getMediaDirectory($media->getKey()).'/conversions'))->toBeFalse(); +}); + +it('will remove responsive images when using custom file namer', function () { + config(['media-library.path_generator' => DefaultPathGenerator::class]); + config(['media-library.file_namer' => TestFileNamer::class]); + + $media = $this->testModelWithResponsiveImages->addMedia($this->getTestJpg())->toMediaCollection('images'); + + expect(File::exists($this->getMediaDirectory($media->getKey()).'/conversions'))->toBeTrue(); + expect(File::exists($this->getMediaDirectory($media->getKey()).'/responsive-images'))->toBeTrue(); + expect(File::exists($this->getMediaDirectory($media->getKey())))->toBeTrue(); + + + $media->delete(); + + expect(File::exists($media->getPath()))->toBeFalse(); + expect(File::exists($this->getMediaDirectory($media->getKey()).'/conversions'))->toBeFalse(); + expect(File::exists($this->getMediaDirectory($media->getKey()).'/responsive-images'))->toBeFalse(); + expect(File::exists($this->getMediaDirectory($media->getKey())))->toBeFalse(); +}); + it('will not remove the files when should delete preserving media returns true', function () { $testModelClass = new class extends TestModel { diff --git a/tests/TestSupport/TestModels/TestModelWithResponsiveImages.php b/tests/TestSupport/TestModels/TestModelWithResponsiveImages.php index 40e574db8..313bc5deb 100644 --- a/tests/TestSupport/TestModels/TestModelWithResponsiveImages.php +++ b/tests/TestSupport/TestModels/TestModelWithResponsiveImages.php @@ -16,7 +16,8 @@ public function registerMediaConversions(?Media $media = null): void $this ->addMediaConversion('otherImageConversion') - ->greyscale(); + ->greyscale() + ->nonQueued(); $this ->addMediaConversion('pngtojpg') @@ -24,7 +25,8 @@ public function registerMediaConversions(?Media $media = null): void ->quality(1) ->background('#ff00ff') ->format('jpg') - ->withResponsiveImages(); + ->withResponsiveImages() + ->nonQueued(); $this ->addMediaConversion('lowerQuality')