Issue #2330899 by attiks, Jelle_S, fietserwin: Allow image effects to change...

Issue #2330899 by attiks, Jelle_S, fietserwin: Allow image effects to change the MIME type + extension, add a convert image effect.

core/lib/Drupal/Core/Image/Image.php

@@ -148,6 +148,13 @@ public function apply($operation, array $arguments = array()) {
     return $this->getToolkit()->apply($operation, $arguments);
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  public function convert($extension) {
+    return $this->apply('convert', array('extension' => $extension));
+  }
+
   /**
    * {@inheritdoc}
    */

core/lib/Drupal/Core/Image/ImageInterface.php

@@ -148,6 +148,21 @@ public function scale($width, $height = NULL, $upscale = FALSE);
    */
   public function scaleAndCrop($width, $height);
 
+  /**
+   * Instructs the toolkit to save the image in the format specified by the
+   * extension.
+   *
+   * @param string $extension
+   *   The extension to convert to (e.g. 'jpeg' or 'png'). Allowed values depend
+   *   on the current image toolkit.
+   *
+   * @return bool
+   *   TRUE on success, FALSE on failure.
+   *
+   * @see \Drupal\Core\ImageToolkit\ImageToolkitInterface::getSupportedExtensions()
+   */
+  public function convert($extension);
+
   /**
    * Crops an image to a rectangle specified by the given dimensions.
    *

core/modules/image/src/Controller/ImageStyleDownloadController.php

@@ -130,8 +130,20 @@ public function deliver(Request $request, $scheme, ImageStyleInterface $image_st
 
     // Don't try to generate file if source is missing.
     if (!file_exists($image_uri)) {
-      $this->logger->notice('Source image at %source_image_path not found while trying to generate derivative image at %derivative_path.',  array('%source_image_path' => $image_uri, '%derivative_path' => $derivative_uri));
-      return new Response($this->t('Error generating image, missing source file.'), 404);
+      // If the image style converted the extension, it has been added to the
+      // original file, resulting in filenames like image.png.jpeg. So to find
+      // the actual source image, we remove the extension and check if that
+      // image exists.
+      $path_info = pathinfo($image_uri);
+      $converted_image_uri = $path_info['dirname'] . DIRECTORY_SEPARATOR . $path_info['filename'];
+      if (!file_exists($converted_image_uri)) {
+        $this->logger->notice('Source image at %source_image_path not found while trying to generate derivative image at %derivative_path.',  array('%source_image_path' => $image_uri, '%derivative_path' => $derivative_uri));
+        return new Response($this->t('Error generating image, missing source file.'), 404);
+      }
+      else {
+        // The converted file does exist, use it as the source.
+        $image_uri = $converted_image_uri;
+      }
     }
 
     // Don't start generating the image if the derivative already exists or if

core/modules/image/src/Entity/ImageStyle.php

@@ -172,15 +172,15 @@ protected static function replaceImageStyle(ImageStyleInterface $style) {
    * {@inheritdoc}
    */
   public function buildUri($uri) {
-    $scheme = file_uri_scheme($uri);
+    $scheme = $this->fileUriScheme($uri);
     if ($scheme) {
-      $path = file_uri_target($uri);
+      $path = $this->fileUriTarget($uri);
     }
     else {
       $path = $uri;
-      $scheme = file_default_scheme();
+      $scheme = $this->fileDefaultScheme();
     }
-    return $scheme . '://styles/' . $this->id() . '/' . $scheme . '/' . $path;
+    return $scheme . '://styles/' . $this->id() . '/' . $scheme . '/' . $this->addExtension($path);
   }
 
   /**
@@ -307,12 +307,22 @@ public function transformDimensions(array &$dimensions) {
     }
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  public function getDerivativeExtension($extension) {
+    foreach ($this->getEffects() as $effect) {
+      $extension = $effect->getDerivativeExtension($extension);
+    }
+    return $extension;
+  }
+
   /**
    * {@inheritdoc}
    */
   public function getPathToken($uri) {
     // Return the first 8 characters.
-    return substr(Crypt::hmacBase64($this->id() . ':' . $uri, \Drupal::service('private_key')->get() . Settings::getHashSalt()), 0, 8);
+    return substr(Crypt::hmacBase64($this->id() . ':' . $this->addExtension($uri), $this->getPrivateKey() . $this->getHashSalt()), 0, 8);
   }
 
   /**
@@ -336,7 +346,7 @@ public function getEffect($effect) {
    */
   public function getEffects() {
     if (!$this->effectsBag) {
-      $this->effectsBag = new ImageEffectBag(\Drupal::service('plugin.manager.image.effect'), $this->effects);
+      $this->effectsBag = new ImageEffectBag($this->getImageEffectPluginManager(), $this->effects);
       $this->effectsBag->sort();
     }
     return $this->effectsBag;
@@ -380,4 +390,114 @@ public function setName($name) {
     return $this;
   }
 
+  /**
+   * Returns the image effect plugin manager.
+   *
+   * @return \Drupal\Component\Plugin\PluginManagerInterface
+   *   The image effect plugin manager.
+   */
+  protected function getImageEffectPluginManager() {
+    return \Drupal::service('plugin.manager.image.effect');
+  }
+
+  /**
+   * Gets the Drupal private key.
+   *
+   * @return string
+   *   The Drupal private key.
+   */
+  protected function getPrivateKey() {
+    return \Drupal::service('private_key')->get();
+  }
+
+  /**
+   * Gets a salt useful for hardening against SQL injection.
+   *
+   * @return string
+   *   A salt based on information in settings.php, not in the database.
+   *
+   * @throws \RuntimeException
+   */
+  protected function getHashSalt() {
+    return Settings::getHashSalt();
+  }
+
+  /**
+   * Adds an extension to a path.
+   *
+   * If this image style changes the extension of the derivative, this method
+   * adds the new extension to the given path. This way we avoid filename
+   * clashes while still allowing us to find the source image.
+   *
+   * @param string $path
+   *   The path to add the extension to.
+   *
+   * @return string
+   *   The given path if this image style doesn't change its extension, or the
+   *   path with the added extension if it does.
+   */
+  protected function addExtension($path) {
+    $original_extension = pathinfo($path, PATHINFO_EXTENSION);
+    $extension = $this->getDerivativeExtension($original_extension);
+    if ($original_extension !== $extension) {
+      $path .= '.' . $extension;
+    }
+    return $path;
+  }
+
+  /**
+   * Provides a wrapper for file_uri_scheme() to allow unit testing.
+   *
+   * Returns the scheme of a URI (e.g. a stream).
+   *
+   * @param string $uri
+   *   A stream, referenced as "scheme://target"  or "data:target".
+   *
+   * @see file_uri_target()
+   *
+   * @todo: Remove when https://www.drupal.org/node/2050759 is in.
+   *
+   * @return string
+   *   A string containing the name of the scheme, or FALSE if none. For
+   *   example, the URI "public://example.txt" would return "public".
+   */
+  protected function fileUriScheme($uri) {
+    return file_uri_scheme($uri);
+  }
+
+  /**
+   * Provides a wrapper for file_uri_target() to allow unit testing.
+   *
+   * Returns the part of a URI after the schema.
+   *
+   * @param string $uri
+   *   A stream, referenced as "scheme://target" or "data:target".
+   *
+   * @see file_uri_scheme()
+   *
+   * @todo: Convert file_uri_target() into a proper injectable service.
+   *
+   * @return string|bool
+   *   A string containing the target (path), or FALSE if none.
+   *   For example, the URI "public://sample/test.txt" would return
+   *   "sample/test.txt".
+   */
+  protected function fileUriTarget($uri) {
+    return file_uri_target($uri);
+  }
+
+  /**
+   * Provides a wrapper for file_default_scheme() to allow unit testing.
+   *
+   * Gets the default file stream implementation.
+   *
+   * @todo: Convert file_default_scheme() into a proper injectable service.
+   *
+   * @return string
+   *   'public', 'private' or any other file scheme defined as the default.
+   */
+  protected function fileDefaultScheme() {
+    return file_default_scheme();
+  }
+
 }

core/modules/image/src/ImageEffectBase.php

@@ -74,6 +74,16 @@ public function transformDimensions(array &$dimensions) {
     $dimensions['width'] = $dimensions['height'] = NULL;
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  public function getDerivativeExtension($extension) {
+    // Most image effects will not change the extension. This base
+    // implementation represents this behavior. Override this method if your
+    // image effect does change the extension.
+    return $extension;
+  }
+
   /**
    * {@inheritdoc}
    */

core/modules/image/src/ImageEffectInterface.php

@@ -43,6 +43,18 @@ public function applyEffect(ImageInterface $image);
    */
   public function transformDimensions(array &$dimensions);
 
+  /**
+   * Returns the extension the derivative would have have after applying this
+   * image effect.
+   *
+   * @param string $extension
+   *   The file extension the derivative has before applying.
+   *
+   * @return string
+   *   The file extension after applying.
+   */
+  public function getDerivativeExtension($extension);
+
   /**
    * Returns a render array summarizing the configuration of the image effect.
    *

core/modules/image/src/ImageStyleInterface.php

@@ -131,6 +131,18 @@ public function createDerivative($original_uri, $derivative_uri);
    */
   public function transformDimensions(array &$dimensions);
 
+  /**
+   * Determines the extension of the derivative without generating it.
+   *
+   * @param string $extension
+   *   The file extension of the original image.
+   *
+   * @return string
+   *   The extension the derivative image will have, given the extension of the
+   *   original.
+   */
+  public function getDerivativeExtension($extension);
+
   /**
    * Returns a specific image effect.
    *

core/modules/image/src/Plugin/ImageEffect/ConvertImageEffect.php

+<?php
+
+/**
+ * @file
+ * Contains \Drupal\image\Plugin\ImageEffect\ConvertImageEffect.
+ */
+
+namespace Drupal\image\Plugin\ImageEffect;
+
+use Drupal\Component\Utility\Unicode;
+use Drupal\Core\Form\FormStateInterface;
+use Drupal\Core\Image\ImageInterface;
+use Drupal\image\ConfigurableImageEffectBase;
+
+/**
+ * Converts an image resource.
+ *
+ * @ImageEffect(
+ *   id = "image_convert",
+ *   label = @Translation("Convert"),
+ *   description = @Translation("Converts an image between extensions (e.g. from PNG to JPEG).")
+ * )
+ */
+class ConvertImageEffect extends ConfigurableImageEffectBase {
+
+  /**
+   * {@inheritdoc}
+   */
+  public function transformDimensions(array &$dimensions) {
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function applyEffect(ImageInterface $image) {
+    if (!$image->convert($this->configuration['extension'])) {
+      $this->logger->error('Image convert failed using the %toolkit toolkit on %path (%mimetype)', array('%toolkit' => $image->getToolkitId(), '%path' => $image->getSource(), '%mimetype' => $image->getMimeType()));
+      return FALSE;
+    }
+    return TRUE;
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function getDerivativeExtension($extension) {
+    return $this->configuration['extension'];
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function getSummary() {
+    $summary = array(
+      '#markup' => Unicode::strtoupper($this->configuration['extension']),
+    );
+    $summary += parent::getSummary();
+
+    return $summary;
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function defaultConfiguration() {
+    return array(
+      'extension' => NULL,
+    );
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function buildConfigurationForm(array $form, FormStateInterface $form_state) {
+    $extensions = \Drupal::service('image.toolkit.manager')->getDefaultToolkit()->getSupportedExtensions();
+    $options = array_combine(
+      $extensions,
+      array_map(array('\Drupal\Component\Utility\Unicode', 'strtoupper'), $extensions)
+    );
+    $form['extension'] = array(
+      '#type' => 'select',
+      '#title' => t('Extension'),
+      '#default_value' => $this->configuration['extension'],
+      '#required' => TRUE,
+      '#options' => $options,
+    );
+    return $form;
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function submitConfigurationForm(array &$form, FormStateInterface $form_state) {
+    parent::submitConfigurationForm($form, $form_state);
+    $this->configuration['extension'] = $form_state->getValue('extension');
+  }
+
+}

core/modules/image/src/Tests/ImageEffectsTest.php

@@ -88,6 +88,21 @@ function testCropEffect() {
     $this->assertEqual($calls['crop'][0][3], 4, 'Height was passed correctly');
   }
 
+  /**
+   * Tests the ConvertImageEffect plugin.
+   */
+  function testConvertEffect() {
+    // Test jpeg.
+    $this->assertImageEffect('image_convert', array(
+      'extension' => 'jpeg',
+    ));
+    $this->assertToolkitOperationsCalled(array('convert'));
+
+    // Check the parameters.
+    $calls = $this->imageTestGetAllCalls();
+    $this->assertEqual($calls['convert'][0][0], 'jpeg', 'Extension was passed correctly');
+  }
+
   /**
    * Test the image_scale_and_crop_effect() function.
    */

core/modules/image/src/Tests/ImageStyleTest.php

+<?php
+
+/**
+ * @file
+ * Contains \Drupal\image\Tests\ImageStyleTest.
+ */
+
+namespace Drupal\image\Tests;
+
+use Drupal\Tests\UnitTestCase;
+use Drupal\Component\Utility\Crypt;
+
+/**
+ * @coversDefaultClass \Drupal\image\Entity\ImageStyle
+ *
+ * @group Image
+ */
+class ImageStyleTest extends UnitTestCase {
+
+  /**
+   * The entity type used for testing.
+   *
+   * @var \Drupal\Core\Entity\EntityTypeInterface|\PHPUnit_Framework_MockObject_MockObject
+   */
+  protected $entityType;
+
+  /**
+   * The entity manager used for testing.
+   *
+   * @var \Drupal\Core\Entity\EntityManagerInterface|\PHPUnit_Framework_MockObject_MockObject
+   */
+  protected $entityManager;
+
+  /**
+   * The ID of the type of the entity under test.
+   *
+   * @var string
+   */
+  protected $entityTypeId;
+
+  /**
+   * Gets a mocked image style for testing.
+   *
+   * @param string $image_effect_id
+   *   The image effect ID.
+   * @param \Drupal\image\ImageEffectInterface|\PHPUnit_Framework_MockObject_MockObject $image_effect
+   *   The image effect used for testing.
+   *
+   * @return \Drupal\image\ImageStyleInterface|\Drupal\image\ImageStyleInterface
+   *   The mocked image style.
+   */
+  protected function getImageStyleMock($image_effect_id, $image_effect, $stubs = array()) {
+    $effectManager = $this->getMockBuilder('\Drupal\image\ImageEffectManager')
+      ->disableOriginalConstructor()
+      ->getMock();
+    $effectManager->expects($this->any())
+      ->method('createInstance')
+      ->with($image_effect_id)
+      ->will($this->returnValue($image_effect));
+    $default_stubs = array(
+      'getImageEffectPluginManager',
+      'fileUriScheme',
+      'fileUriTarget',
+      'fileDefaultScheme',
+    );
+    $image_style = $this->getMockBuilder('\Drupal\image\Entity\ImageStyle')
+      ->setConstructorArgs(array(
+        array('effects' => array($image_effect_id => array('id' => $image_effect_id))),
+        $this->entityTypeId,
+      ))
+      ->setMethods(array_merge($default_stubs, $stubs))
+      ->getMock();
+
+    $image_style->expects($this->any())
+      ->method('getImageEffectPluginManager')
+      ->will($this->returnValue($effectManager));
+    $image_style->expects($this->any())
+      ->method('fileUriScheme')
+      ->will($this->returnCallback(array($this, 'fileUriScheme')));
+    $image_style->expects($this->any())
+      ->method('fileUriTarget')
+      ->will($this->returnCallback(array($this, 'fileUriTarget')));
+    $image_style->expects($this->any())
+      ->method('fileDefaultScheme')
+      ->will($this->returnCallback(array($this, 'fileDefaultScheme')));
+
+    return $image_style;
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function setUp() {
+    $this->entityTypeId = $this->randomMachineName();
+    $this->provider = $this->randomMachineName();
+    $this->entityType = $this->getMock('\Drupal\Core\Entity\EntityTypeInterface');
+    $this->entityType->expects($this->any())
+      ->method('getProvider')
+      ->will($this->returnValue($this->provider));
+    $this->entityManager = $this->getMock('\Drupal\Core\Entity\EntityManagerInterface');
+    $this->entityManager->expects($this->any())
+      ->method('getDefinition')
+      ->with($this->entityTypeId)
+      ->will($this->returnValue($this->entityType));
+  }
+
+  /**
+   * @covers ::getDerivativeExtension
+   */
+  public function testGetDerivativeExtension() {
+    $image_effect_id = $this->randomMachineName();
+    $logger = $this->getMockBuilder('\Psr\Log\LoggerInterface')->getMock();
+    $image_effect = $this->getMockBuilder('\Drupal\image\ImageEffectBase')
+      ->setConstructorArgs(array(array(), $image_effect_id, array(), $logger))
+      ->getMock();
+    $image_effect->expects($this->any())
+      ->method('getDerivativeExtension')
+      ->will($this->returnValue('png'));
+
+    $image_style = $this->getImageStyleMock($image_effect_id, $image_effect);
+
+    $extensions = array('jpeg', 'gif', 'png');
+    foreach ($extensions as $extension) {
+      $extensionReturned = $image_style->getDerivativeExtension($extension);
+      $this->assertEquals($extensionReturned, 'png');
+    }
+  }
+
+  /**
+   * @covers ::buildUri
+   */
+  public function testBuildUri() {
+    // Image style that changes the extension.
+    $image_effect_id = $this->randomMachineName();
+    $logger = $this->getMockBuilder('\Psr\Log\LoggerInterface')->getMock();
+    $image_effect = $this->getMockBuilder('\Drupal\image\ImageEffectBase')
+      ->setConstructorArgs(array(array(), $image_effect_id, array(), $logger))
+      ->getMock();
+    $image_effect->expects($this->any())
+      ->method('getDerivativeExtension')
+      ->will($this->returnValue('png'));
+
+    $image_style = $this->getImageStyleMock($image_effect_id, $image_effect);
+    $this->assertEquals($image_style->buildUri('public://test.jpeg'), 'public://styles/' . $image_style->id() . '/public/test.jpeg.png');
+
+    // Image style that doesn't change the extension.
+    $image_effect_id = $this->randomMachineName();
+    $image_effect = $this->getMockBuilder('\Drupal\image\ImageEffectBase')
+      ->setConstructorArgs(array(array(), $image_effect_id, array(), $logger))
+      ->getMock();
+    $image_effect->expects($this->any())
+      ->method('getDerivativeExtension')
+      ->will($this->returnArgument(0));
+
+    $image_style = $this->getImageStyleMock($image_effect_id, $image_effect);
+    $this->assertEquals($image_style->buildUri('public://test.jpeg'), 'public://styles/' . $image_style->id() . '/public/test.jpeg');
+  }
+
+  /**
+   * @covers ::getPathToken
+   */
+  public function testGetPathToken() {
+    $logger = $this->getMockBuilder('\Psr\Log\LoggerInterface')->getMock();
+    $private_key = $this->randomMachineName();
+    $hash_salt = $this->randomMachineName();
+
+    // Image style that changes the extension.
+    $image_effect_id = $this->randomMachineName();
+    $image_effect = $this->getMockBuilder('\Drupal\image\ImageEffectBase')
+      ->setConstructorArgs(array(array(), $image_effect_id, array(), $logger))
+      ->getMock();
+    $image_effect->expects($this->any())
+      ->method('getDerivativeExtension')
+      ->will($this->returnValue('png'));
+
+    $image_style = $this->getImageStyleMock($image_effect_id, $image_effect, array('getPrivateKey', 'getHashSalt'));
+    $image_style->expects($this->any())
+        ->method('getPrivateKey')
+        ->will($this->returnValue($private_key));
+    $image_style->expects($this->any())
+        ->method('getHashSalt')
+        ->will($this->returnValue($hash_salt));
+
+    // Assert the extension has been added to the URI before creating the token.
+    $this->assertEquals($image_style->getPathToken('public://test.jpeg.png'), $image_style->getPathToken('public://test.jpeg'));
+    $this->assertEquals(substr(Crypt::hmacBase64($image_style->id() . ':' . 'public://test.jpeg.png', $private_key . $hash_salt), 0, 8), $image_style->getPathToken('public://test.jpeg'));
+    $this->assertNotEquals(substr(Crypt::hmacBase64($image_style->id() . ':' . 'public://test.jpeg', $private_key . $hash_salt), 0, 8), $image_style->getPathToken('public://test.jpeg'));
+
+    // Image style that doesn't change the extension.
+    $image_effect_id = $this->randomMachineName();
+    $image_effect = $this->getMockBuilder('\Drupal\image\ImageEffectBase')
+      ->setConstructorArgs(array(array(), $image_effect_id, array(), $logger))