Code to reproduce:

{code}
$style = \Drupal\image\Entity\ImageStyle::load('thumbnail');
$style->flush('http://www.example.com/');
{code}

Error: GuzzleHttp\Exception\ConnectException: cURL error 6: Couldn't resolve host 'styles' (see http://curl.haxx.se/libcurl/c/libcurl-errors.html) in GuzzleHttp\Handler\CurlFactory::createRejection() (line 186 of vendor/guzzlehttp/guzzle/src/Handler/CurlFactory.php).

This is because ImageStyle::flush() just calls $this->buildUri(), which generates a result of http://styles/www.example.com which we don't get a chance to correct like we do when image derivatives are output, which calls file_create_url() and our alter hook gets the chance to execute.

CommentFileSizeAuthor
#5 2785939-cleanup-image-style-handling.interdiff.txt3.02 KBDave Reid
#5 2785939-cleanup-image-style-handling.patch13.74 KBDave Reid
#3 2785939-cleanup-image-style-handling.patch11.74 KBDave Reid
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Dave Reid created an issue. See original summary.

Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid as a volunteer and at Lullabot commented

Ugghhhh, we were at least able to handle this in D7 because image_style_flush() did not return early and allowed hook_image_style_flush() to execute. Execution simply stops in ImageStyle::flush() when the file_exists fails.

We really really could use #1358896: Flexible scheme and URI for image derivatives.

Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid as a volunteer and at Lullabot commented
Status: Active » Needs review
FileSize
2785939-cleanup-image-style-handling.patch11.74 KB

Fuck it, let's just override the ImageStyle class. This simplifies a lot of code.

deviantintegral’s picture

deviantintegral CreditAttribution: deviantintegral at Lullabot commented

Looking good!

  1. +++ b/remote_stream_wrapper.module
@@ -3,6 +3,15 @@
+ * Implements hook_entity_info_alter().
...
+function remote_stream_wrapper_entity_type_alter(&$entity_types) {

    Should we be using https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Entity%21... ?

    Either way, the docblock doesn't match the hook. 

  2. +++ b/src/Entity/ImageStyle.php
@@ -0,0 +1,27 @@
+    if (file_is_uri_remote($uri)) {

    Is there an issue to make this procedural code OOP / testable?

Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid as a volunteer and at Lullabot commented
FileSize
2785939-cleanup-image-style-handling.patch13.74 KB
2785939-cleanup-image-style-handling.interdiff.txt3.02 KB

Either way, the docblock doesn't match the hook.

It looks like hook_entity_type_alter is what I should use to alter existing things rather than adding new things. Also, THANKS FILE ENTITY (where I got the example from).

Is there an issue to make this procedural code OOP / testable?

#2778103: Convert file_is_* helper functions to a service class. For now I've copied what the original ImageStyle class does for unit testability and added a wrapper method.

  • Dave Reid committed 545386f on 8.x-1.x 
    Issue #2785939 by Dave Reid: Cleanup image style handling by overriding...
Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid as a volunteer and at Lullabot commented
Status: Needs review » Fixed

Merged #5 to 8.x-1.x. Thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.