diff --git a/core/modules/image/image.admin.inc b/core/modules/image/image.admin.inc index b3d4a6e..3b080af 100644 --- a/core/modules/image/image.admin.inc +++ b/core/modules/image/image.admin.inc @@ -32,11 +32,11 @@ function template_preprocess_image_style_preview(&$variables) { // Set up original file information. $original_path = \Drupal::config('image.settings')->get('preview_image'); - $original_image = $image_factory->get($original_path); + $image = $image_factory->get($original_path); $variables['original'] = array( 'url' => file_create_url($original_path), - 'width' => $original_image->getWidth(), - 'height' => $original_image->getHeight(), + 'width' => $image->getWidth(), + 'height' => $image->getHeight(), ); if ($variables['original']['width'] > $variables['original']['height']) { $variables['preview']['original']['width'] = min($variables['original']['width'], $sample_width); @@ -51,13 +51,16 @@ function template_preprocess_image_style_preview(&$variables) { $preview_file = $style->buildUri($original_path); // Create derivative if necessary. if (!file_exists($preview_file)) { - $style->createDerivative($original_path, $preview_file); + $style->applyAndSave($image, $preview_file); } - $preview_image = $image_factory->get($preview_file); + else { + $image = $image_factory->get($preview_file); + } + $image = $image_factory->get($preview_file); $variables['derivative'] = array( 'url' => file_create_url($preview_file), - 'width' => $preview_image->getWidth(), - 'height' => $preview_image->getHeight(), + 'width' => $image->getWidth(), + 'height' => $image->getHeight(), ); if ($variables['derivative']['width'] > $variables['derivative']['height']) { $variables['preview']['derivative']['width'] = min($variables['derivative']['width'], $sample_width); diff --git a/core/modules/image/src/Controller/ImageStyleDownloadController.php b/core/modules/image/src/Controller/ImageStyleDownloadController.php index 1407298..35e8235 100644 --- a/core/modules/image/src/Controller/ImageStyleDownloadController.php +++ b/core/modules/image/src/Controller/ImageStyleDownloadController.php @@ -163,16 +163,23 @@ public function deliver(Request $request, $scheme, ImageStyleInterface $image_st } } - // Try to generate the image, unless another thread just did it while we - // were acquiring the lock. - $success = file_exists($derivative_uri) || $image_style->createDerivative($image_uri, $derivative_uri); + if (file_exists($derivative_uri)) { + // Another thread just generated the derivative image while we were + // acquiring the lock. + $image = $this->imageFactory->get($derivative_uri); + $success = $image->isValid(); + } + else { + // Generate the derivative image. + $image = $this->imageFactory->get($image_uri); + $success = $image_style->applyAndSave($image, $derivative_uri); + } if (!empty($lock_acquired)) { $this->lock->release($lock_name); } if ($success) { - $image = $this->imageFactory->get($derivative_uri); $uri = $image->getSource(); $headers += array( 'Content-Type' => $image->getMimeType(), diff --git a/core/modules/image/src/Entity/ImageStyle.php b/core/modules/image/src/Entity/ImageStyle.php index 7d97d59..e7eb019 100644 --- a/core/modules/image/src/Entity/ImageStyle.php +++ b/core/modules/image/src/Entity/ImageStyle.php @@ -7,12 +7,18 @@ namespace Drupal\image\Entity; +use Drupal\Component\Plugin\PluginManagerInterface; use Drupal\Core\Cache\Cache; use Drupal\Core\Config\Entity\ConfigEntityBase; use Drupal\Core\Entity\EntityStorageInterface; use Drupal\Core\Entity\EntityWithPluginCollectionInterface; +use Drupal\Core\File\FileSystemInterface; +use Drupal\Core\Image\ImageFactory; +use Drupal\Core\Image\ImageInterface; +use Drupal\Core\PrivateKey; use Drupal\Core\Routing\RequestHelper; use Drupal\Core\Site\Settings; +use Drupal\Core\StreamWrapper\StreamWrapperManagerInterface; use Drupal\Core\Url; use Drupal\image\ImageEffectPluginCollection; use Drupal\image\ImageEffectInterface; @@ -94,6 +100,68 @@ class ImageStyle extends ConfigEntityBase implements ImageStyleInterface, Entity protected $effectsCollection; /** + * The file system service. + * + * @var \Drupal\Core\File\FileSystemInterface + */ + protected $fileSystem; + + /** + * The image factory. + * + * @var \Drupal\Core\Image\ImageFactory + */ + protected $imageFactory; + + /** + * The image effect plugin manager. + * + * @var \Drupal\Component\Plugin\PluginManagerInterface + */ + protected $effectManager; + + /** + * The stream wrapper manager. + * + * @var \Drupal\Core\StreamWrapper\StreamWrapperManagerInterface + */ + protected $streamWrapperManager; + + /** + * The private key service. + * + * @var \Drupal\Core\PrivateKey + */ + protected $privateKey; + + /** + * Constructs an ImageStyle object. + * + * @param array $values + * An array of values to set, keyed by property name. + * @param string $entity_type + * The type of the entity to create. + * @param \Drupal\Core\File\FileSystemInterface $file_system + * The file system service. + * @param \Drupal\Core\Image\ImageFactory $image_factory + * The image factory. + * @param \Drupal\Component\Plugin\PluginManagerInterface $image_effect_plugin_manager + * The image effect plugin manager. + * @param \Drupal\Core\StreamWrapper\StreamWrapperManagerInterface $stream_wrapper_manager + * The stream wrapper manager. + * @param \Drupal\Core\PrivateKey $private_key + * The private key service. + */ + public function __construct(array $values, $entity_type, FileSystemInterface $file_system = NULL, ImageFactory $image_factory = NULL, PluginManagerInterface $image_effect_plugin_manager = NULL, StreamWrapperManagerInterface $stream_wrapper_manager = NULL, PrivateKey $private_key = NULL) { + parent::__construct($values, $entity_type); + $this->fileSystem = $file_system ?: \Drupal::service('file_system'); + $this->imageFactory = $image_factory ?: \Drupal::service('image.factory'); + $this->effectManager = $image_effect_plugin_manager ?: \Drupal::service('plugin.manager.image.effect'); + $this->streamWrapperManager = $stream_wrapper_manager ?: \Drupal::service('stream_wrapper_manager'); + $this->privateKey = $private_key ?: \Drupal::service('private_key'); + } + + /** * Overrides Drupal\Core\Entity\Entity::id(). */ public function id() { @@ -177,7 +245,7 @@ protected static function replaceImageStyle(ImageStyleInterface $style) { * {@inheritdoc} */ public function buildUri($uri) { - $scheme = $this->fileUriScheme($uri); + $scheme = $this->fileSystem->uriScheme($uri); if ($scheme) { $path = $this->fileUriTarget($uri); } @@ -205,7 +273,7 @@ public function buildUrl($path, $clean_urls = NULL) { $token_query = array(); if (!\Drupal::config('image.settings')->get('suppress_itok_output')) { // The passed $path variable can be either a relative path or a full URI. - $original_uri = file_uri_scheme($path) ? file_stream_wrapper_uri_normalize($path) : file_build_uri($path); + $original_uri = $this->fileSystem->uriScheme($path) ? file_stream_wrapper_uri_normalize($path) : file_build_uri($path); $token_query = array(IMAGE_DERIVATIVE_TOKEN => $this->getPathToken($original_uri)); } @@ -225,8 +293,8 @@ public function buildUrl($path, $clean_urls = NULL) { // ensure that it is included. Once the file exists it's fine to fall back // to the actual file path, this avoids bootstrapping PHP once the files are // built. - if ($clean_urls === FALSE && file_uri_scheme($uri) == 'public' && !file_exists($uri)) { - $directory_path = \Drupal::service('stream_wrapper_manager')->getViaUri($uri)->getDirectoryPath(); + if ($clean_urls === FALSE && $this->fileSystem->uriScheme($uri) == 'public' && !file_exists($uri)) { + $directory_path = $this->streamWrapperManager->getViaUri($uri)->getDirectoryPath(); return Url::fromUri('base:' . $directory_path . '/' . file_uri_target($uri), array('absolute' => TRUE, 'query' => $token_query))->toString(); } @@ -253,7 +321,7 @@ public function flush($path = NULL) { } // Delete the style directory in each registered wrapper. - $wrappers = \Drupal::service('stream_wrapper_manager')->getWrappers(StreamWrapperInterface::WRITE_VISIBLE); + $wrappers = $this->streamWrapperManager->getWrappers(StreamWrapperInterface::WRITE_VISIBLE); foreach ($wrappers as $wrapper => $wrapper_data) { if (file_exists($directory = $wrapper . '://styles/' . $this->id())) { file_unmanaged_delete_recursive($directory); @@ -276,15 +344,27 @@ public function flush($path = NULL) { * {@inheritdoc} */ public function createDerivative($original_uri, $derivative_uri) { + return $this->applyAndSave($this->imageFactory->get($original_uri), $derivative_uri); + } - // If the source file doesn't exist, return FALSE without creating folders. - $image = \Drupal::service('image.factory')->get($original_uri); + /** + * {@inheritdoc} + */ + public function applyAndSave(ImageInterface $image, $derivative_uri) { + // If the image doesn't exist or is invalid, return FALSE without creating + // folders. if (!$image->isValid()) { return FALSE; } - // Get the folder for the final location of this style. - $directory = drupal_dirname($derivative_uri); + // Apply the style to the image object. Return FALSE without creating + // folders in case of failure. + if (!$this->apply($image)) { + return FALSE; + } + + // Get the folder for the final location of this derivative image. + $directory = $this->fileSystem->dirname($derivative_uri); // Build the destination folder tree if it doesn't already exist. if (!file_prepare_directory($directory, FILE_CREATE_DIRECTORY | FILE_MODIFY_PERMISSIONS)) { @@ -292,10 +372,7 @@ public function createDerivative($original_uri, $derivative_uri) { return FALSE; } - foreach ($this->getEffects() as $effect) { - $effect->applyEffect($image); - } - + // Save image file at destination. if (!$image->save($derivative_uri)) { if (file_exists($derivative_uri)) { \Drupal::logger('image')->error('Cached image file %destination already exists. There may be an issue with your rewrite configuration.', array('%destination' => $derivative_uri)); @@ -309,6 +386,23 @@ public function createDerivative($original_uri, $derivative_uri) { /** * {@inheritdoc} */ + public function apply(ImageInterface $image) { + // If the image doesn't exist or is invalid, return FALSE immediately. + if (!$image->isValid()) { + return FALSE; + } + + // Apply the effects to the image object. + foreach ($this->getEffects() as $effect) { + $effect->applyEffect($image); + } + + return TRUE; + } + + /** + * {@inheritdoc} + */ public function transformDimensions(array &$dimensions, $uri) { foreach ($this->getEffects() as $effect) { $effect->transformDimensions($dimensions, $uri); @@ -330,7 +424,7 @@ public function getDerivativeExtension($extension) { */ public function getPathToken($uri) { // Return the first 8 characters. - return substr(Crypt::hmacBase64($this->id() . ':' . $this->addExtension($uri), $this->getPrivateKey() . $this->getHashSalt()), 0, 8); + return substr(Crypt::hmacBase64($this->id() . ':' . $this->addExtension($uri), $this->privateKey->get() . $this->getHashSalt()), 0, 8); } /** @@ -354,7 +448,7 @@ public function getEffect($effect) { */ public function getEffects() { if (!$this->effectsCollection) { - $this->effectsCollection = new ImageEffectPluginCollection($this->getImageEffectPluginManager(), $this->effects); + $this->effectsCollection = new ImageEffectPluginCollection($this->effectManager, $this->effects); $this->effectsCollection->sort(); } return $this->effectsCollection; @@ -399,26 +493,6 @@ public function setName($name) { } /** - * 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 @@ -454,26 +528,6 @@ protected function addExtension($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. diff --git a/core/modules/image/src/ImageStyleInterface.php b/core/modules/image/src/ImageStyleInterface.php index 9507291..26d9f9d 100644 --- a/core/modules/image/src/ImageStyleInterface.php +++ b/core/modules/image/src/ImageStyleInterface.php @@ -8,6 +8,7 @@ namespace Drupal\image; use Drupal\Core\Config\Entity\ConfigEntityInterface; +use Drupal\Core\Image\ImageInterface; /** * Provides an interface defining an image style entity. @@ -100,7 +101,7 @@ public function getPathToken($uri); public function flush($path = NULL); /** - * Creates a new image derivative based on this image style. + * Creates a new image derivative from a source image file. * * Generates an image derivative applying all image effects and saving the * resulting image. @@ -113,10 +114,51 @@ public function flush($path = NULL); * @return bool * TRUE if an image derivative was generated, or FALSE if the image * derivative could not be generated. + * + * @deprecated as of Drupal 8.1.0, will be removed in Drupal 9.0.0. + * Instead, get an Image object from the factory and use the + * ::applyAndSave() method on it. + * @code + * $image = \Drupal::service('image.factory')->get($original_uri); + * $image_style->applyAndSave($image, $derivative_uri); + * @endcode + * + * @see \Drupal\image\ImageStyleInterface::applyAndSave() */ public function createDerivative($original_uri, $derivative_uri); /** + * Applies the style to an Image object and saves image to file. + * + * Alter the source image applying all image effects and saving the resulting + * image. + * + * @param \Drupal\Core\Image\ImageInterface $image + * An Image object. + * @param string $derivative_uri + * Derivative image file URI. + * + * @return bool + * TRUE if the image derivative was saved, FALSE otherwise. + * + * @see \Drupal\image\ImageStyleInterface::apply() + */ + public function applyAndSave(ImageInterface $image, $derivative_uri); + + /** + * Applies the style to an Image object. + * + * Alter the source image applying all image effects. + * + * @param \Drupal\Core\Image\ImageInterface $image + * An Image object. + * + * @return bool + * TRUE if an the image was successfully altered, FALSE otherwise. + */ + public function apply(ImageInterface $image); + + /** * Determines the dimensions of this image style. * * Stores the dimensions of this image style into $dimensions associative diff --git a/core/modules/image/src/Tests/ImageAdminStylesTest.php b/core/modules/image/src/Tests/ImageAdminStylesTest.php index ed3854f..44b45e1 100644 --- a/core/modules/image/src/Tests/ImageAdminStylesTest.php +++ b/core/modules/image/src/Tests/ImageAdminStylesTest.php @@ -394,7 +394,7 @@ public function testFlushUserInterface() { $files = $this->drupalGetTestFiles('image'); $image_uri = $files[0]->uri; $derivative_uri = $style->buildUri($image_uri); - $this->assertTrue($style->createDerivative($image_uri, $derivative_uri)); + $this->assertTrue($style->applyAndSave(\Drupal::service('image.factory')->get($image_uri), $derivative_uri)); $this->assertEqual($this->getImageCount($style), 1); // Go to image styles list page and check if the flush operation link diff --git a/core/modules/image/tests/src/Unit/ImageStyleTest.php b/core/modules/image/tests/src/Unit/ImageStyleTest.php index 80ce713..1a3b972 100644 --- a/core/modules/image/tests/src/Unit/ImageStyleTest.php +++ b/core/modules/image/tests/src/Unit/ImageStyleTest.php @@ -18,6 +18,13 @@ class ImageStyleTest extends UnitTestCase { /** + * The ID of the type of the entity under test. + * + * @var string + */ + protected $entityTypeId; + + /** * The entity type used for testing. * * @var \Drupal\Core\Entity\EntityTypeInterface|\PHPUnit_Framework_MockObject_MockObject @@ -32,11 +39,32 @@ class ImageStyleTest extends UnitTestCase { protected $entityManager; /** - * The ID of the type of the entity under test. + * The file system used for testing. * - * @var string + * @var \Drupal\Core\File\FileSystemInterface|\PHPUnit_Framework_MockObject_MockObject */ - protected $entityTypeId; + protected $fileSystem; + + /** + * The image factory used for testing. + * + * @var \Drupal\Core\Image\ImageFactory|\PHPUnit_Framework_MockObject_MockObject + */ + protected $imageFactory; + + /** + * The stream wrapper manager used for testing. + * + * @var \Drupal\Core\StreamWrapper\StreamWrapperManagerInterface|\PHPUnit_Framework_MockObject_MockObject + */ + protected $streamWrapperManager; + + /** + * The private key service used for testing. + * + * @var \Drupal\Core\PrivateKey|\PHPUnit_Framework_MockObject_MockObject + */ + protected $privateKey; /** * Gets a mocked image style for testing. @@ -57,27 +85,25 @@ protected function getImageStyleMock($image_effect_id, $image_effect, $stubs = a ->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( + ->setConstructorArgs([ array('effects' => array($image_effect_id => array('id' => $image_effect_id))), $this->entityTypeId, - )) + $this->fileSystem, + $this->imageFactory, + $effectManager, + $this->streamWrapperManager, + $this->privateKey, + ]) ->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()) @@ -97,11 +123,33 @@ public function setUp() { $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)); + + $mock_builder = $this->getMockBuilder('\Drupal\Core\File\FileSystem'); + $this->fileSystem = $mock_builder + ->disableOriginalConstructor() + ->setMethods(null) + ->getMock(); + + $mock_builder = $this->getMockBuilder('\Drupal\Core\Image\ImageFactory'); + $this->imageFactory = $mock_builder + ->disableOriginalConstructor() + ->getMock(); + + $mock_builder = $this->getMockBuilder('\Drupal\Core\StreamWrapper\StreamWrapperManagerInterface'); + $this->streamWrapperManager = $mock_builder + ->disableOriginalConstructor() + ->getMock(); + + $mock_builder = $this->getMockBuilder('\Drupal\Core\PrivateKey'); + $this->privateKey = $mock_builder + ->disableOriginalConstructor() + ->getMock(); } /** @@ -173,9 +221,9 @@ public function testGetPathToken() { ->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') + $image_style = $this->getImageStyleMock($image_effect_id, $image_effect, array('getHashSalt')); + $this->privateKey->expects($this->any()) + ->method('get') ->will($this->returnValue($private_key)); $image_style->expects($this->any()) ->method('getHashSalt') @@ -195,9 +243,9 @@ public function testGetPathToken() { ->method('getDerivativeExtension') ->will($this->returnArgument(0)); - $image_style = $this->getImageStyleMock($image_effect_id, $image_effect, array('getPrivateKey', 'getHashSalt')); - $image_style->expects($this->any()) - ->method('getPrivateKey') + $image_style = $this->getImageStyleMock($image_effect_id, $image_effect, array('getHashSalt')); + $this->privateKey->expects($this->any()) + ->method('get') ->will($this->returnValue($private_key)); $image_style->expects($this->any()) ->method('getHashSalt') @@ -209,18 +257,6 @@ public function testGetPathToken() { } /** - * Mock function for ImageStyle::fileUriScheme(). - */ - public function fileUriScheme($uri) { - if (preg_match('/^([\w\-]+):\/\/|^(data):/', $uri, $matches)) { - // The scheme will always be the last element in the matches array. - return array_pop($matches); - } - - return FALSE; - } - - /** * Mock function for ImageStyle::fileUriTarget(). */ public function fileUriTarget($uri) {