Fix for issue #1277 (#1279)

* Fix for issue #1277 * Fix CS * Refactor fix for issue #1277 * Add missing return types * Add withoutManipulations() helper method for Conversion.php * Add test for the withoutManipulations method * Add test for performManipulations early return * Fix style
spatie · Nov 19, 2018 · 089ee07 · 089ee07
1 parent b375d35
commit 089ee07
Show file tree

Hide file tree

Showing 4 changed files with 69 additions and 11 deletions.
diff --git a/src/Conversion/Conversion.php b/src/Conversion/Conversion.php
@@ -81,13 +81,20 @@ public function getManipulations(): Manipulations
         return $this->manipulations;
     }
 
-    public function removeManipulation(string $manipulationName)
+    public function removeManipulation(string $manipulationName) : self
     {
         $this->manipulations->removeManipulation($manipulationName);
 
         return $this;
     }
 
+    public function withoutManipulations() : self
+    {
+        $this->manipulations = new Manipulations();
+
+        return $this;
+    }
+
     public function __call($name, $arguments)
     {
         if (! method_exists($this->manipulations, $name)) {
@@ -106,7 +113,7 @@ public function __call($name, $arguments)
      *
      * @return $this
      */
-    public function setManipulations($manipulations)
+    public function setManipulations($manipulations) : self
     {
         if ($manipulations instanceof Manipulations) {
             $this->manipulations = $this->manipulations->mergeManipulations($manipulations);
@@ -126,7 +133,7 @@ public function setManipulations($manipulations)
      *
      * @return $this
      */
-    public function addAsFirstManipulations(Manipulations $manipulations)
+    public function addAsFirstManipulations(Manipulations $manipulations) : self
     {
         $manipulationSequence = $manipulations->getManipulationSequence()->toArray();
 
@@ -144,7 +151,7 @@ public function addAsFirstManipulations(Manipulations $manipulations)
      *
      * @return $this
      */
-    public function performOnCollections(...$collectionNames)
+    public function performOnCollections(...$collectionNames) : self
     {
         $this->performOnCollections = $collectionNames;
 
@@ -174,7 +181,7 @@ public function shouldBePerformedOn(string $collectionName): bool
      *
      * @return $this
      */
-    public function queued()
+    public function queued() : self
     {
         $this->performOnQueue = true;
 
@@ -186,7 +193,7 @@ public function queued()
      *
      * @return $this
      */
-    public function nonQueued()
+    public function nonQueued() : self
     {
         $this->performOnQueue = false;
 
@@ -198,7 +205,7 @@ public function nonQueued()
      *
      * @return $this
      */
-    public function nonOptimized()
+    public function nonOptimized() : self
     {
         $this->removeManipulation('optimize');
 
@@ -208,7 +215,7 @@ public function nonOptimized()
     /**
      * When creating the converted image, responsive images will be created as well.
      */
-    public function withResponsiveImages()
+    public function withResponsiveImages() : self
     {
         $this->generateResponsiveImages = true;
 

diff --git a/src/FileManipulator.php b/src/FileManipulator.php
@@ -93,13 +93,13 @@ public function performConversions(ConversionCollection $conversions, Media $med
 
                 $copiedOriginalFile = $imageGenerator->convert($copiedOriginalFile, $conversion);
 
-                $conversionResult = $this->performConversion($media, $conversion, $copiedOriginalFile);
+                $manipulationResult = $this->performManipulations($media, $conversion, $copiedOriginalFile);
 
                 $newFileName = pathinfo($media->file_name, PATHINFO_FILENAME).
                     '-'.$conversion->getName().
                     '.'.$conversion->getResultExtension(pathinfo($copiedOriginalFile, PATHINFO_EXTENSION));
 
-                $renamedFile = MediaLibraryFileHelper::renameInDirectory($conversionResult, $newFileName);
+                $renamedFile = MediaLibraryFileHelper::renameInDirectory($manipulationResult, $newFileName);
 
                 if ($conversion->shouldGenerateResponsiveImages()) {
                     app(ResponsiveImageGenerator::class)->generateResponsiveImagesForConversion(
@@ -119,8 +119,12 @@ public function performConversions(ConversionCollection $conversions, Media $med
         $temporaryDirectory->delete();
     }
 
-    public function performConversion(Media $media, Conversion $conversion, string $imageFile): string
+    public function performManipulations(Media $media, Conversion $conversion, string $imageFile): string
     {
+        if ($conversion->getManipulations()->isEmpty()) {
+            return $imageFile;
+        }
+
         $conversionTempFile = pathinfo($imageFile, PATHINFO_DIRNAME).'/'.str_random(16)
             .$conversion->getName()
             .'.'

diff --git a/tests/Unit/Conversion/ConversionTest.php b/tests/Unit/Conversion/ConversionTest.php
@@ -108,6 +108,16 @@ public function it_can_remove_a_previously_set_manipulation()
         $this->assertNull($this->conversion->getManipulations()->getManipulationArgument('format'));
     }
 
+    /** @test */
+    public function it_can_remove_all_previously_set_manipulations()
+    {
+        $this->assertFalse($this->conversion->getManipulations()->isEmpty());
+
+        $this->conversion->withoutManipulations();
+
+        $this->assertTrue($this->conversion->getManipulations()->isEmpty());
+    }
+
     /** @test */
     public function it_will_use_the_extract_duration_parameter_if_it_was_given()
     {

diff --git a/tests/Unit/FileManipulatorTest.php b/tests/Unit/FileManipulatorTest.php
@@ -0,0 +1,37 @@
+<?php
+
+namespace Spatie\MediaLibrary\Tests\Unit;
+
+use Spatie\MediaLibrary\Tests\TestCase;
+use Spatie\MediaLibrary\FileManipulator;
+use Spatie\MediaLibrary\Conversion\Conversion;
+
+class FileManipulatorTest extends TestCase
+{
+    protected $conversionName = 'test';
+
+    /** @var \Spatie\MediaLibrary\Conversion\Conversion */
+    protected $conversion;
+
+    public function setUp()
+    {
+        parent::setUp();
+
+        $this->conversion = new Conversion($this->conversionName);
+    }
+
+    /** @test */
+    public function it_does_not_perform_manipulations_if_not_necessary()
+    {
+        $imageFile = $this->getTestJpg();
+        $media = $this->testModelWithoutMediaConversions->addMedia($this->getTestJpg())->toMediaCollection();
+
+        $conversionTempFile = (new FileManipulator)->performManipulations(
+            $media,
+            $this->conversion->withoutManipulations(),
+            $imageFile
+        );
+
+        $this->assertEquals($imageFile, $conversionTempFile);
+    }
+}