Make image style system flexible

Nice initiative. I've fielded support requests for this for a few years.
I'll see if I can find time to trial it. Takes a bit to set up the manual test case ...

It was in my queue at #1863720: Image styles needs selectable file system path that led to the very-well-stated feature request #1903190: Allow image style derivatives of private images to be stored on the public file system that deserves a name-check here - potatosauce did a fine job of helping us improve the system. ... since we are dropping related links into this thread :-)

Updated issue summary.

+++ b/core/modules/image/image.moduleundefined
@@ -474,10 +469,7 @@ function image_field_update_instance($instance, $prior_instance) {
 function image_path_flush($path) {
   $styles = entity_load_multiple('image_style');
   foreach ($styles as $style) {
-    $image_path = image_style_path($style->id(), $path);
-    if (file_exists($image_path)) {
-      file_unmanaged_delete($image_path);
-    }
+    $style->flushImage($path);

@@ -521,303 +513,18 @@ function image_style_options($include_empty = TRUE) {
+ * @todo: Remove this wrapper when convering the menu entry to a Symfony menu
+ *   router. Figure then out how to pass arguments to the ImageStyle::deliver()
+ *   method.
  */
 function image_style_deliver($style, $scheme) {

Let's mark this @deprecated with link to new implementation

+++ b/core/modules/image/image.moduleundefined
@@ -521,303 +513,18 @@ function image_style_options($include_empty = TRUE) {
  * After generating an image, transfer it to the requesting agent.

+++ b/core/modules/image/lib/Drupal/image/Plugin/Core/Entity/ImageStyle.phpundefined
@@ -141,4 +147,258 @@ protected static function replaceImageStyle(ImageStyle $style) {
+  public function deliver($scheme, $target) {

awesomeness++ this requires controller and follow up issue # for unittests

+++ b/core/modules/image/lib/Drupal/image/ImageStyleInterface.phpundefined
@@ -14,4 +14,99 @@
+   * Returns the URI of this image when using a style.
...
+  public function path($uri);
...
+   * Returns the URL this image derivative given an image path.
...
+  public function url($path, $clean_urls = NULL);

getUri() and getUrl() or maybe buildURI() and buildURL()

+++ b/core/modules/image/lib/Drupal/image/ImageStyleInterface.phpundefined
@@ -14,4 +14,99 @@
+   * Flushes a specific file in this style.
...
+  public function flushImage($uri);

file|image -> flashMedia()

Also I think better to merge this into one method

flush($uri = NULL) seems better

+++ b/core/modules/image/lib/Drupal/image/ImageStyleInterface.phpundefined
@@ -14,4 +14,99 @@
+   * Creates a new image derivative based on this image style.
...
+   * @param string $source
+   *   Path of the source file.
+   * @param string $destination
+   *   Path or URI of the destination file.
...
+  public function createDerivative($source, $destination);

this is a file uri or url?
same time $destination is URI

+++ b/core/modules/image/lib/Drupal/image/ImageStyleInterface.phpundefined
@@ -14,4 +14,99 @@
+   * Determines the dimensions of this styled image.
...
+   * @param array $dimensions
+   *   Dimensions to be modified - an array with components width and height, in
...
+  public function transformDimensions(array &$dimensions);

This needs explanation, implementation of this method have to provide at least:
- width
- height

+++ b/core/modules/image/lib/Drupal/image/Tests/FileMoveTest.phpundefined
@@ -39,8 +39,8 @@ function testNormal() {
-    $derivative_uri = image_style_path($style->id(), $file->getFileUri());
-    image_style_create_derivative($style, $file->getFileUri(), $derivative_uri);
+    $derivative_uri = $style->path($file->getFileUri());
+    $style->createDerivative($file->getFileUri(), $derivative_uri);

path() makes no sense, also why not use file-object itself?
createDerivate() operating on urls? so what about access checking comment!

+++ b/core/modules/image/lib/Drupal/image/Tests/ImageAdminStylesTest.phpundefined
@@ -36,7 +36,7 @@ function createSampleImage($style) {
-    return image_style_url($style->id(), $file_path) ? $file_path : FALSE;
+    return $style->url($file_path) ? $file_path : FALSE;

disagree on that, getUrl() proper name that does not have collision with entity method

+++ b/core/modules/image/lib/Drupal/image/Tests/ImageAdminStylesTest.phpundefined
@@ -285,7 +285,10 @@ function testStyleReplacement() {
-    $this->assertRaw(image_style_url($new_style_name, file_load($node->{$field_name}[Language::LANGCODE_NOT_SPECIFIED][0]['fid'])->getFileUri()), 'Image displayed using style replacement style.');
+
+    // Reload the image style using the new name.
+    $style = entity_load('image_style', $new_style_name);
+    $this->assertRaw($style->url(file_load($node->{$field_name}[Language::LANGCODE_NOT_SPECIFIED][0]['fid'])->getFileUri()), 'Image displayed using style replacement style.');

@@ -295,8 +298,9 @@ function testStyleReplacement() {
+    $replacement_style = entity_load('image_style', 'thumbnail');
     $this->drupalGet('node/' . $nid);
-    $this->assertRaw(image_style_url('thumbnail', file_load($node->{$field_name}[Language::LANGCODE_NOT_SPECIFIED][0]['fid'])->getFileUri()), 'Image displayed using style replacement style.');
+    $this->assertRaw($replacement_style->url(file_load($node->{$field_name}[Language::LANGCODE_NOT_SPECIFIED][0]['fid'])->getFileUri()), 'Image displayed using style replacement style.');

@@ -362,7 +366,7 @@ function testConfigImport() {
-    $this->assertRaw(image_style_url($style_name, file_load($node->{$field_name}[Language::LANGCODE_NOT_SPECIFIED][0]['fid'])->getFileUri()), format_string('Image displayed using style @style.', array('@style' => $style_name)));
+    $this->assertRaw($style->url(file_load($node->{$field_name}[Language::LANGCODE_NOT_SPECIFIED][0]['fid'])->getFileUri()), format_string('Image displayed using style @style.', array('@style' => $style_name)));

$node->get($field_name)->value probably now

+++ b/core/modules/image/lib/Drupal/image/Tests/ImageStylesPathAndUrlTest.phpundefined
@@ -37,55 +38,59 @@ public static function getInfo() {
-    $this->style_name = 'style_foo';
-    $style = entity_create('image_style', array('name' => $this->style_name, 'label' => $this->randomString()));
-    $style->save();
+    $this->style = entity_create('image_style', array('name' => 'style_foo', 'label' => $this->randomString()));
+    $this->style->save();

Yay!

+++ b/core/modules/picture/picture.moduleundefined
@@ -225,7 +225,7 @@ function theme_picture($variables) {
-    'src' => image_style_url($variables['style_name'], $variables['uri']),
+    'src' => entity_load('image_style', $variables['style_name'])->url($variables['uri']),

@@ -244,7 +244,7 @@ function theme_picture($variables) {
-          'src' => image_style_url($new_sources[0]['style_name'], $new_sources[0]['uri']),
+          'src' => entity_load('image_style', $new_sources[0]['style_name'])->url($new_sources[0]['uri']),

@@ -253,7 +253,7 @@ function theme_picture($variables) {
-          $srcset[] = image_style_url($new_source['style_name'], $new_source['uri']) . ' ' . $new_source['#multiplier'];
+          $srcset[] = entity_load('image_style', $new_source['style_name'])->url($new_source['uri']) . ' ' . $new_source['#multiplier'];

@@ -338,7 +338,7 @@ function picture_get_image_dimensions($variables) {
-  image_style_transform_dimensions($variables['style_name'], $dimensions);
+  entity_load('image_style', $variables['style_name'])->transformDimensions($dimensions);

+++ b/core/modules/rdf/lib/Drupal/rdf/Tests/ImageFieldAttributesTest.phpundefined
@@ -101,7 +101,7 @@ function testNodeTeaser() {
-    $image_uri = image_style_url('medium', $this->file->getFileUri());
+    $image_uri = entity_load('image_style', 'medium')->url($this->file->getFileUri());

+++ b/core/modules/rdf/rdf.moduleundefined
@@ -400,7 +400,7 @@ function rdf_preprocess_field(&$variables) {
-          $variables['item_attributes'][$delta]['resource'] = image_style_url($element[$delta]['#image_style'], $item['entity']->getFileUri());
+          $variables['item_attributes'][$delta]['resource'] = entity_load('image_style', $element[$delta]['#image_style'])->url($item['entity']->getFileUri());

this entity_load() scares me :) let's make follow-up to move this to preprocess

Great!

I'm now following this thread too. I was building an image gallery site in late 2012, and I just couldn't get things going on certain way, what is obviously getting here more attention.

If it helps anything, you should read the descriptions (and comments) in 1863720 and 1903190 so you can pretty much see why there is a problem. There is also bit more wider perspective in this problem in comments, although I believe the solution can't hug all the world :-) Anyways, just to give some idea, how things could work, if they could.

Really glad this thing is getting forward!

This is really great clean-up so any takers?

Updated issue summary.

$directory_path = file_stream_wrapper_get_instance_by_scheme('public')->getDirectoryPath();
  $items[$directory_path . '/styles/%image_style'] = array(
    'title' => 'Generate image style',
    'page callback' => 'image_style_deliver',
    'page arguments' => array(count(explode('/', $directory_path)) + 1),
    'access callback' => TRUE,
    'type' => MENU_CALLBACK,
  );

Should be converted #1987712: Convert file_download() to a new style controller

PS added to summary

@claudiu.cristea I dont think we need to wait, no time left... This tasks could be done in-parallel

Suppose deliver() should be proxied to #1987712: Convert file_download() to a new style controller
Other methods just needs rename

I think only injection of needed services need some work. Other nitpicks are less critical

+++ b/core/modules/image/image.moduleundefined
@@ -404,303 +396,18 @@ function image_style_options($include_empty = TRUE) {
+ * @todo: Remove this wrapper when convering the menu entry to a Symfony menu
+ *   router. Figure then out how to pass arguments to the ImageStyle::deliver()
+ *   method.
  */
 function image_style_deliver($style, $scheme) {

this needs link to the issue

+++ b/core/modules/image/image.moduleundefined
@@ -852,7 +559,7 @@ function image_effect_save($style, &$effect) {
 function image_effect_delete($style, $effect) {
   unset($style->effects[$effect['ieid']]);
   $style->save();
-  image_style_flush($style);
+  $style->flush();

suppose save() should call flush() internally

+++ b/core/modules/image/lib/Drupal/image/ImageStyleInterface.phpundefined
@@ -14,4 +14,102 @@
+   * Determines the dimensions of this styled image.
...
+   * - width: Integer with the derivative image width. ¶
+   * - height: Integer with the derivative image height. ¶

trailing white-space

+++ b/core/modules/image/lib/Drupal/image/Plugin/Core/Entity/ImageStyle.phpundefined
@@ -148,4 +154,255 @@ protected static function replaceImageStyle(ImageStyle $style) {
+      $image_derivative_token = Drupal::request()->query->get(IMAGE_DERIVATIVE_TOKEN);
...
+        $headers = module_invoke_all('file_download', $original_uri);

this needs injection of $request & $moduleHandler services

+++ b/core/modules/image/lib/Drupal/image/Plugin/Core/Entity/ImageStyle.phpundefined
@@ -148,4 +154,255 @@ protected static function replaceImageStyle(ImageStyle $style) {
+      lock()->release($lock_name);

And $lock service

+++ b/core/modules/image/lib/Drupal/image/Plugin/Core/Entity/ImageStyle.phpundefined
@@ -148,4 +154,255 @@ protected static function replaceImageStyle(ImageStyle $style) {
+    if (!config('image.settings')->get('suppress_itok_output')) {
...
+    if ($clean_urls === NULL) {
+      // Assume clean URLs unless the request tells us otherwise.
+      $clean_urls = TRUE;
+      try {
+        $request = Drupal::request();
+        $clean_urls = $request->attributes->get('clean_urls');
+      }
+      catch (ServiceNotFoundException $e) {
+      }

do we really need $config injection here?

And clean URLs should be deprecated

+++ b/core/modules/image/lib/Drupal/image/Plugin/Core/Entity/ImageStyle.phpundefined
@@ -148,4 +154,255 @@ protected static function replaceImageStyle(ImageStyle $style) {
+    // Let other modules update as necessary on flush.
+    module_invoke_all('image_style_flush', $this);
...
+    field_info_cache_clear();
+    drupal_theme_rebuild();
+
+    // Clear page caches when flushing.
+    if (module_exists('block')) {
+      cache('block')->deleteAll();
+    }
+    cache('page')->deleteAll();

this needs injection too

Actually the issue depends on conversion #1987712: Convert file_download() to a new style controller

Fix deprecated functions. Once we move code around it needs to be cleaned up!
Filed #2030207: Do not load all field instances when replacing image style

PS: Injection of services is possible for plugins https://drupal.org/node/2012118 but is not needed because deliver() should live in separate controller.

+++ b/core/modules/image/image.moduleundefined
@@ -404,303 +396,18 @@ function image_style_options($include_empty = TRUE) {
 function image_style_deliver($style, $scheme) {
...
+  return $style->deliver($scheme, $target);

+++ b/core/modules/image/lib/Drupal/image/ImageStyleInterface.phpundefined
@@ -14,4 +14,102 @@
+   * @todo: This will be transformed into a simple wrapper when
+   *   \Drupal\image\Controller\ImageStyleController::deliver() arrives.
...
+  public function deliver($scheme, $target);

+++ b/core/modules/image/lib/Drupal/image/Plugin/Core/Entity/ImageStyle.phpundefined
@@ -148,4 +154,255 @@ protected static function replaceImageStyle(ImageStyle $style) {
+  public function deliver($scheme, $target) {
...
+            drupal_add_http_header($name, $value);
...
+      $headers = array(
+        'Content-Type' => $image->info['mime_type'],
+        'Content-Length' => $image->info['file_size'],
+      );
+      return new BinaryFileResponse($uri, 200, $headers);

Delivery depends on conversion #1987712: Convert file_download() to a new style controller

Which have right implementation.

+++ b/core/modules/image/image.moduleundefined
@@ -404,303 +396,18 @@ function image_style_options($include_empty = TRUE) {
-function image_style_flush($style) {

+++ b/core/modules/image/lib/Drupal/image/ImageStyleInterface.phpundefined
@@ -14,4 +14,102 @@
+   * Flushes cached media for this style.
+   *
+   * @param string $path
+   *   (optional) The original image path or URI. If it's supplied, only this
+   *   image derivative will be flushed.
+   */
+  public function flush($path = NULL);

+++ b/core/modules/image/lib/Drupal/image/Plugin/Core/Entity/ImageStyle.phpundefined
@@ -148,4 +154,255 @@ protected static function replaceImageStyle(ImageStyle $style) {
+  public function flush($path = NULL) {
+    // A specific image path has been provided. Flush only that derivative.
+    if (isset($path)) {
+      $original_uri = $this->buildUri($path);
+      if (file_exists($original_uri)) {
+        file_unmanaged_delete($original_uri);
+      }
+      return;

This is API change should be done before July 1 and issue summary needs to point about it

Awesome! Now we need to get delivery in and write tests for #2030207: Do not load all field instances when replacing image style

Some minor quickies, documentation related. I will continue my review later today and then review the core of this patch (ImageStyle.php and image.module).

+++ b/core/modules/image/image.admin.incundefined
+++ b/core/modules/image/image.admin.incundefined
@@ -634,9 +634,9 @@ function theme_image_style_preview($variables) {

comment says 'style'entry is an array, but it now is an object (though it already was, not due to this patch).

+++ b/core/modules/image/lib/Drupal/image/ImageStyleInterface.phpundefined
@@ -14,4 +14,101 @@
+   * implement this method to set different serve different image derivatives
...
+   *   The image to be deliverd or the apropiate exception.
...
+  public function deliver($scheme, $target);

- incorrect English
- 2 typos, BTW: an exception is not returned but thrown, so I should make it "The image to be delivered.".

+++ b/core/modules/image/lib/Drupal/image/ImageStyleInterface.phpundefined
+   * Returns the URI of this image when using a style.
...
   *   The URI to an image style image.
   */
+  public function buildUri($uri);

I propose to change this to:
- Returns the URI of this image when using this style.
- The URI to the image derivative for this style.

+++ b/core/modules/image/lib/Drupal/image/ImageStyleInterface.phpundefined
+  public function buildUrl($path, $clean_urls = NULL);

- i'm a bit lost. What is the difference between URI and URL? "Server path (or file system path)" versus "extenal path"
- Why is $clean_urls mixed? I guess it should be bool(ean).
- @see refers to the actual base class method, shouldn't it refer to the interface method?

+++ b/core/modules/image/lib/Drupal/image/ImageStyleInterface.phpundefined
   * Generates an image derivative by creating the destination folder (if it does
...
   * and saving a cached version of the resulting image.
...
   *   TRUE if an image derivative was generated, or FALSE if the image derivative
...
  public function createDerivative($original_uri, $derivative_uri);

- lines are to long.
- does it really save a cached version? I guess it saves the image for caching.
- can you explicitly document what happens if the image derivative already exists?

+++ b/core/modules/image/lib/Drupal/image/ImageStyleInterface.phpundefined
   * Applies all effects from this image style and store the resulting width and
...
  public function transformDimensions(array &$dimensions);

- Effects are not really applied but queried/asked about how the dimensions would be affected if it would be applied. Moreover this is an implementation detail, the style should return the resulting dimensions, how it computes these is not relevant here.

+++ b/core/modules/image/lib/Drupal/image/Plugin/Core/Entity/ImageStyle.phpundefined
@@ -148,4 +159,257 @@ protected static function replaceImageStyle(ImageStyle $style) {
+   * @todo: Move to controller after https://drupal.org/node/1987712.
+   */
+  public function deliver($scheme, $target) {

Quite a big method. Are there any functional differences with the one in issue 1987712? This because I did not really review this method, that is better done (has already been done) in issue 1987712, assuming that they are functionally equivalent.

+++ b/core/modules/image/lib/Drupal/image/Plugin/Core/Entity/ImageStyle.phpundefined
@@ -148,4 +159,257 @@ protected static function replaceImageStyle(ImageStyle $style) {
+        // Tell client to retry again in 3 seconds. Currently no browsers are known

Comment line to long.

+++ b/core/modules/image/lib/Drupal/image/Plugin/Core/Entity/ImageStyle.phpundefined
@@ -148,4 +159,257 @@ protected static function replaceImageStyle(ImageStyle $style) {
+    // Try to generate the image, unless another thread just did it while we were

Comment line to long.

+++ b/core/modules/image/lib/Drupal/image/Plugin/Core/Entity/ImageStyle.phpundefined
@@ -148,4 +159,257 @@ protected static function replaceImageStyle(ImageStyle $style) {
+    // 'image.settings:allow_insecure_derivatives' configuration is TRUE, so that
+    // the emitted links remain valid if it is changed back to the default FALSE.
+    // However, sites which need to prevent the token query from being emitted at
+    // all can additionally set the 'image.settings:suppress_itok_output'
+    // configuration to TRUE to achieve that (if both are set, the security token

Comment lines to long.

+++ b/core/modules/image/lib/Drupal/image/Plugin/Core/Entity/ImageStyle.phpundefined
+  public function flush($path = NULL) {
+      $original_uri = $this->buildUri($path);

$original_uri seems to be a misleading name (derivative_uri?).

+++ b/core/modules/image/lib/Drupal/image/Plugin/Core/Entity/ImageStyle.phpundefined
+  public function buildUrl($path, $clean_urls = NULL) {
+      catch (ServiceNotFoundException $e) {
+      }

Is this the fail silent design pattern :) ?

Furthermore:
- I also reviewed the changes (most are deletes) in image.module. They are all OK. Also the removals of the flushes, they are now part of the postSave and postDelete methods().

- I did not review the tests.

- I can confirm that none of the removed functions is still called anywhere.

API changes:
- a number of image_style_...() functions are removed (_create_derivative, transform_dimensions, _flush, _url, _path_token, _path). not sure if those are internal or real API functions.
- the parameter of hook_image_style_flush is now an object. image.api.php is not completely correct in this, the parameter type is correct, but the text still mentions array (as does api.drupal.org).

So the part I reviewed is functionally RTBC, though documentation, naming and coding standards (comment lines to long) need some attention.

+++ b/core/modules/image/image.api.phpundefined
@@ -74,8 +74,8 @@ function hook_image_effect_info_alter(&$effects) {
- * @param Drupal\image\Plugin\Core\Entity\ImageStyle $style
...
+ * @param \Drupal\image\Plugin\Core\Entity\ImageStyleInterface $style

+++ b/core/modules/image/lib/Drupal/image/ImageStyleInterface.phpundefined
@@ -14,4 +14,99 @@
+   * @see \Drupal\image\Plugin\Core\Entity\ImageStyleInterface::deliver()

+++ b/core/modules/image/lib/Drupal/image/Plugin/Core/Entity/ImageStyle.phpundefined
@@ -148,4 +160,258 @@ protected static function replaceImageStyle(ImageStyle $style) {
+    // \Drupal\image\Plugin\Core\Entity\ImageStyle::deliver()).
...
+   *   \Drupal\image\Plugin\Core\Entity\ImageStyle::path().

+++ b/core/modules/image/lib/Drupal/image/Tests/ImageFieldTestBase.phpundefined
@@ -15,11 +15,11 @@
+ *   \Drupal\image\Plugin\Core\Entity\ImageStyle::createDerivative()
...
+ *   \Drupal\image\Plugin\Core\Entity\ImageStyle::flush()

+++ b/core/modules/image/lib/Drupal/image/Tests/ImageStylesPathAndUrlTest.phpundefined
@@ -37,55 +38,59 @@ public static function getInfo() {
+   * Test \Drupal\image\Plugin\Core\Entity\ImageStyle::buildUrl() with a file
...
+   * Test \Drupal\image\Plugin\Core\Entity\ImageStyle::buildUrl() with a file
...
+   * Test \Drupal\image\Plugin\Core\Entity\ImageStyle::buildUrl() with the
...
+   * Tests \Drupal\image\Plugin\Core\Entity\ImageStyle::buildUrl() with a file

@@ -96,13 +101,13 @@ function testImageStyleUrlExtraSlash() {
+   * Tests \Drupal\image\Plugin\Core\Entity\ImageStyle::buildUrl().

\Drupal\image\ImageStyleInterface

+++ b/core/modules/image/lib/Drupal/image/Plugin/Core/Entity/ImageStyle.phpundefined
@@ -148,4 +160,258 @@ protected static function replaceImageStyle(ImageStyle $style) {
+  public function buildUrl($path, $clean_urls = NULL) {
+    $uri = $this->buildUri($path);
+    // The token query is added even if the ¶

trailing whitespace

t's a security issue. I wouldn't touch that.

I was just referring to the documentation, there it should be documented as bool, not a mixed (especially as we explicitly check on FALSE.

This shouldn't be documented in the interface being an implementation issue. Maybe some implementations will always have existing derivatives (imagine keeping derivatives on Flickr).

IMO, the interface defines the (base) contract and thus should specify preconditions that can be assumed and postconditions that should be met. Thus it should specify that as a precondition the directory does not have to exist or that the derivative can already exist and (as a postcondition) does not absolutely have to be recreated in that case.

+++ b/core/modules/image/lib/Drupal/image/Plugin/Core/Entity/ImageStyle.phpundefined
+  public function buildUrl($path, $clean_urls = NULL) {
+      catch (ServiceNotFoundException $e) {
+      }

I prefer to have some documentation if exceptions are caught and ignored because that is an anti-pattern. In this case i can imagine that we don't care if that service cannot be found, it is not absolutely needed for our task.

Thanks for changing the other remarks.

There's no consensus on #1987712: Convert file_download() to a new style controller
So let's get this in first

This issue was RTBC and passing tests on July 1, the beginning of API freeze.

See also:

#2033669: Image file objects should be classed
#1821854: Convert image effects into plugins

Updated issue summary.

I've rerolled my other two issues on top of this one since it is already RTBC.
Let's get this in!

That big RDF issue added an extra call to image_style_url(). Fixed, leaving RTBC.

Doh, bad copy/paste and random failure.
While I was in there, I cleaned up a couple instances of bad docs.

PHPStorm reported it as an unused variable, but $request = $this->prepareRequestForGenerator($clean_url); was needed. I've deleted the assignment so no one else makes the same mistake.

Also, I've attached an interdiff from the last RTBC in #39.

Looks great!

+++ b/core/modules/image/image.moduleundefined
@@ -835,10 +547,6 @@ function image_effect_save($style, &$effect) {
   $style->save();
-
-  // Flush all derivatives that exist for this style, so they are regenerated
-  // with the new or updated effect.
-  image_style_flush($style);

@@ -852,7 +560,6 @@ function image_effect_save($style, &$effect) {
   $style->save();
-  image_style_flush($style);

Note for reviewers, this call to flush is removed since saving the style as a whole triggers flushing now.

Committed 27f8cd4 and pushed to 8.x. Thanks!

Lets get #1987712: Convert file_download() to a new style controller this done ASAP... to me the amount application level services that are being accessed in the new deliver method on the ImageStyle config entity is ugly.

+++ b/core/modules/image/image.moduleundefined
@@ -599,6 +599,8 @@ function image_effect_apply($image, $effect) {
+  // @todo Image style loading will be moved outside theme in
+  // https://drupal.org/node/2029649

Fixed code style in commit...

  // @todo Image style loading will be moved outside theme in
  //   https://drupal.org/node/2029649

Updated issue summary.

Regarding the interdiff of #53:

+++ b/core/modules/image/lib/Drupal/image/Plugin/Core/Entity/ImageStyle.phpundefined
@@ -173,15 +175,15 @@ public function deliver($scheme, $target) {
-      $valid = $valid && isset($image_derivative_token) && $image_derivative_token === $this->getPathToken($scheme . '://' . $target);
+      $valid &= isset($image_derivative_token) && $image_derivative_token === $this->getPathToken($original_uri);

I don't like it that $valid is changed into an integer here. &= is NOT the same as = ... && ... , and &&= does not exist. So the original notation was correct: we want a boolean, so use operators that return a boolean, that's way more readable.

Change notices are critical.

Awesome!

Will this patch be ported to D7?

Modules will have now the opportunity to intercept the derivative creation and place each derivative in custom locations, under custom streams by extending \Drupal\image\Plugin\Core\Entity\ImageStyle or directly implementing \Drupal\image\ImageStyleInterface interface.

Is there an example of how two different stream wrappers (provided by two different contrib modules) would be able to accomplish this from their own module code, and separate from each other?

Ok, then I'm definitely feeling better about re-opening #1358896: Flexible scheme and URI for image derivatives because the actual need is for the individual stream wrappers to be able to control where image style URIs get generated for their own paths, without having to worry about which module wins.