From dad927e637392a05fa5b7fc56108d28b7015fcb9 Mon Sep 17 00:00:00 2001 From: Michael Lindahl Date: Thu, 18 Aug 2022 21:05:56 -0700 Subject: [PATCH 1/3] Update CleanCommand to delete depreciated responsive images When running `php artisan media-library:clean` we should be clearing responsive images for conversions that are no longer used, as well as for active conversions that are no longer using responsive images. Fixes #2397 --- src/MediaCollections/Commands/CleanCommand.php | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/MediaCollections/Commands/CleanCommand.php b/src/MediaCollections/Commands/CleanCommand.php index b4e95f14e..fc79d9640 100644 --- a/src/MediaCollections/Commands/CleanCommand.php +++ b/src/MediaCollections/Commands/CleanCommand.php @@ -93,7 +93,7 @@ protected function deleteFilesGeneratedForDeprecatedConversions(): void $this->deleteConversionFilesForDeprecatedConversions($media); if ($media->responsive_images) { - $this->deleteResponsiveImagesForDeprecatedConversions($media); + $this->deleteDeprecatedResponsiveImages($media); } if ($this->rateLimit) { @@ -123,9 +123,10 @@ protected function deleteConversionFilesForDeprecatedConversions(Media $media): }); } - protected function deleteResponsiveImagesForDeprecatedConversions(Media $media): void + protected function deleteDeprecatedResponsiveImages(Media $media): void { - $conversionNames = ConversionCollection::createForMedia($media) + $conversionNamesWithResponsiveImages = ConversionCollection::createForMedia($media) + ->filter(fn (Conversion $conversion) => $conversion->shouldGenerateResponsiveImages()) ->map(fn (Conversion $conversion) => $conversion->getName()) ->push('media_library_original'); @@ -134,7 +135,7 @@ protected function deleteResponsiveImagesForDeprecatedConversions(Media $media): collect($responsiveImagesGeneratedFor) ->map(fn (string $generatedFor) => $media->responsiveImages($generatedFor)) - ->reject(fn (RegisteredResponsiveImages $responsiveImages) => $conversionNames->contains($responsiveImages->generatedFor)) + ->reject(fn (RegisteredResponsiveImages $responsiveImages) => $conversionNamesWithResponsiveImages->contains($responsiveImages->generatedFor)) ->each(function (RegisteredResponsiveImages $responsiveImages) { if (! $this->isDryRun) { $responsiveImages->delete(); From a99b27a48b7615cdaf839c2ece566978eabf46aa Mon Sep 17 00:00:00 2001 From: Michael Lindahl Date: Fri, 19 Aug 2022 10:40:14 -0700 Subject: [PATCH 2/3] Add unit test for active conversions without responsive images --- .../CleanResponsiveImagesTest.php | 30 +++++++++++++++++-- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/tests/Conversions/Commands/CleanCommandTest/CleanResponsiveImagesTest.php b/tests/Conversions/Commands/CleanCommandTest/CleanResponsiveImagesTest.php index 43ba77165..454825171 100644 --- a/tests/Conversions/Commands/CleanCommandTest/CleanResponsiveImagesTest.php +++ b/tests/Conversions/Commands/CleanCommandTest/CleanResponsiveImagesTest.php @@ -1,8 +1,6 @@ testModelWithResponsiveImages ->addMedia($this->getTestJpg()) ->preservingOriginal() @@ -26,3 +24,29 @@ expect($media->responsive_images)->toEqual($originalResponsiveImagesContent); $this->assertFileDoesNotExist($deprecatedReponsiveImagesPath); }); + +it('can clean responsive images for active conversions without responsive images', function () { + $media = $this->testModelWithConversion + ->addMedia($this->getTestJpg()) + ->preservingOriginal() + ->toMediaCollection(); + + $thumbResponsiveImageFileName = "{$media->file_name}___thumb_340_280.jpg"; + $thumbReponsiveImagesPath = $this->getMediaDirectory("{$media->id}/responsive-images/{$thumbResponsiveImageFileName}"); + mkdir($this->getMediaDirectory("{$media->id}/responsive-images")); + touch($thumbReponsiveImagesPath); + + $originalResponsiveImagesContent = $media->responsive_images; + $newResponsiveImages = $originalResponsiveImagesContent; + $newResponsiveImages['thumb']['base64svg'] = "data:image/svg+xml;base64,PCPg=="; + $newResponsiveImages['thumb']['urls'][0] = $thumbResponsiveImageFileName; + $media->responsive_images = $newResponsiveImages; + $media->save(); + + $this->artisan('media-library:clean'); + + $media->refresh(); + + expect($media->responsive_images)->toEqual($originalResponsiveImagesContent); + $this->assertFileDoesNotExist($thumbReponsiveImagesPath); +}); \ No newline at end of file From ed2466415c8f1e4110237f552ad670194403d37f Mon Sep 17 00:00:00 2001 From: Michael Lindahl Date: Fri, 19 Aug 2022 10:45:33 -0700 Subject: [PATCH 3/3] Remove duplicate test and file --- .../CleanResponsiveImagesTest.php | 52 ------------------- .../Commands/CleanConversionsTest.php | 32 ++++++++++-- 2 files changed, 29 insertions(+), 55 deletions(-) delete mode 100644 tests/Conversions/Commands/CleanCommandTest/CleanResponsiveImagesTest.php diff --git a/tests/Conversions/Commands/CleanCommandTest/CleanResponsiveImagesTest.php b/tests/Conversions/Commands/CleanCommandTest/CleanResponsiveImagesTest.php deleted file mode 100644 index 454825171..000000000 --- a/tests/Conversions/Commands/CleanCommandTest/CleanResponsiveImagesTest.php +++ /dev/null @@ -1,52 +0,0 @@ -testModelWithResponsiveImages - ->addMedia($this->getTestJpg()) - ->preservingOriginal() - ->toMediaCollection(); - - $deprecatedResponsiveImageFileName = 'test___deprecatedConversion_50_41.jpg'; - $deprecatedReponsiveImagesPath = $this->getMediaDirectory("1/responsive-images/{$deprecatedResponsiveImageFileName}"); - touch($deprecatedReponsiveImagesPath); - - $originalResponsiveImagesContent = $media->responsive_images; - $newResponsiveImages = $originalResponsiveImagesContent; - $newResponsiveImages['deprecatedConversion'] = $originalResponsiveImagesContent['thumb']; - $newResponsiveImages['deprecatedConversion']['urls'][0] = $deprecatedResponsiveImageFileName; - $media->responsive_images = $newResponsiveImages; - $media->save(); - - $this->artisan('media-library:clean'); - - $media->refresh(); - - expect($media->responsive_images)->toEqual($originalResponsiveImagesContent); - $this->assertFileDoesNotExist($deprecatedReponsiveImagesPath); -}); - -it('can clean responsive images for active conversions without responsive images', function () { - $media = $this->testModelWithConversion - ->addMedia($this->getTestJpg()) - ->preservingOriginal() - ->toMediaCollection(); - - $thumbResponsiveImageFileName = "{$media->file_name}___thumb_340_280.jpg"; - $thumbReponsiveImagesPath = $this->getMediaDirectory("{$media->id}/responsive-images/{$thumbResponsiveImageFileName}"); - mkdir($this->getMediaDirectory("{$media->id}/responsive-images")); - touch($thumbReponsiveImagesPath); - - $originalResponsiveImagesContent = $media->responsive_images; - $newResponsiveImages = $originalResponsiveImagesContent; - $newResponsiveImages['thumb']['base64svg'] = "data:image/svg+xml;base64,PCPg=="; - $newResponsiveImages['thumb']['urls'][0] = $thumbResponsiveImageFileName; - $media->responsive_images = $newResponsiveImages; - $media->save(); - - $this->artisan('media-library:clean'); - - $media->refresh(); - - expect($media->responsive_images)->toEqual($originalResponsiveImagesContent); - $this->assertFileDoesNotExist($thumbReponsiveImagesPath); -}); \ No newline at end of file diff --git a/tests/Conversions/Commands/CleanConversionsTest.php b/tests/Conversions/Commands/CleanConversionsTest.php index 9685b6bc0..0d336eb7f 100644 --- a/tests/Conversions/Commands/CleanConversionsTest.php +++ b/tests/Conversions/Commands/CleanConversionsTest.php @@ -142,14 +142,14 @@ expect($this->getMediaDirectory("{$this->media['model1']['collection2']->id}/test.jpg"))->toBeFile(); }); -it('can clean responsive images', function () { +it('can clean responsive images for deprecated conversions', function () { $media = $this->testModelWithResponsiveImages ->addMedia($this->getTestJpg()) ->preservingOriginal() ->toMediaCollection(); - $deprecatedResponsiveImageFileName = 'test___deprecatedConversion_50_41.jpg'; - $deprecatedReponsiveImagesPath = $this->getMediaDirectory("5/responsive-images/{$deprecatedResponsiveImageFileName}"); + $deprecatedResponsiveImageFileName = "{$media->file_name}___deprecatedConversion_50_41.jpg"; + $deprecatedReponsiveImagesPath = $this->getMediaDirectory("{$media->id}/responsive-images/{$deprecatedResponsiveImageFileName}"); touch($deprecatedReponsiveImagesPath); $originalResponsiveImagesContent = $media->responsive_images; @@ -167,6 +167,32 @@ $this->assertFileDoesNotExist($deprecatedReponsiveImagesPath); }); +it('can clean responsive images for active conversions without responsive images', function () { + $media = $this->testModelWithConversion + ->addMedia($this->getTestJpg()) + ->preservingOriginal() + ->toMediaCollection(); + + $thumbResponsiveImageFileName = "{$media->file_name}___thumb_340_280.jpg"; + $thumbReponsiveImagesPath = $this->getMediaDirectory("{$media->id}/responsive-images/{$thumbResponsiveImageFileName}"); + mkdir($this->getMediaDirectory("{$media->id}/responsive-images")); + touch($thumbReponsiveImagesPath); + + $originalResponsiveImagesContent = $media->responsive_images; + $newResponsiveImages = $originalResponsiveImagesContent; + $newResponsiveImages['thumb']['base64svg'] = "data:image/svg+xml;base64,PCPg=="; + $newResponsiveImages['thumb']['urls'][0] = $thumbResponsiveImageFileName; + $media->responsive_images = $newResponsiveImages; + $media->save(); + + $this->artisan('media-library:clean'); + + $media->refresh(); + + expect($media->responsive_images)->toEqual($originalResponsiveImagesContent); + $this->assertFileDoesNotExist($thumbReponsiveImagesPath); +}); + it('will throw an exception when using a non existing disk', function () { $this->expectException(DiskDoesNotExist::class);