Merge pull request #3717 from StyxUA/fix/remove-files-with-custom-fil…

…e-namer FileNamer should be used while removing conversions and/or responsive images
spatie · Nov 8, 2024 · 68d74ba · 68d74ba
2 parents cc46eb8 + ddc4a75
commit 68d74ba
Show file tree

Hide file tree

Showing 6 changed files with 81 additions and 41 deletions.
diff --git 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
@@ -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
@@ -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/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);

diff --git 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
@@ -16,15 +16,17 @@ public function registerMediaConversions(?Media $media = null): void
 
         $this
             ->addMediaConversion('otherImageConversion')
-            ->greyscale();
+            ->greyscale()
+            ->nonQueued();
 
         $this
             ->addMediaConversion('pngtojpg')
             ->width(700)
             ->quality(1)
             ->background('#ff00ff')
             ->format('jpg')
-            ->withResponsiveImages();
+            ->withResponsiveImages()
+            ->nonQueued();
 
         $this
             ->addMediaConversion('lowerQuality')