Implements image style generation like Drupal 7

1. Testing this change
I am trying this and I'm getting "Error generating image.".

The error comes from

$success = $image_style->createDerivative($image_uri, $derivative_uri) &&
      \Drupal::service('stream_wrapper_manager')->getViaScheme('s3')->waitUntilFileExists($derivative_uri);

which returns false (in my case).
This seems to come from :

\Drupal::service('image.factory')->get($original_uri);

returning an invalid image.

Input is s3://2017-03/tesla_spain.jpg (the files exists on S3, the file upload worked)
Output should be s3://styles/medium/s3/2017-03/tesla_spain.jpg but neither folder nor file exist?
What am I doing wrong here?

2. Cacheability:
Mildly related The TrustedRedirectResponse is cacheable:
Is it ok if I add something like

      $response =  new TrustedRedirectResponse(file_create_url($derivative_uri));
      // Cache the output of this response.
      $ttl = $config = \Drupal::config('s3fs.settings')->get('redirect_ttl');
      $response
        ->addCacheableDependency((new \Drupal\Core\Cache\CacheableMetadata())->setCacheMaxAge($ttl));
      return $response;

So in the config we could set how long the redirects should be cached in a reverse proxy (or at the client)?

Added patch with default setting included.

Added in unwanted code (by accident), so removing that again.

A little formatting added.

Hi wouters_f and jansete

We tried to apply the image_style_generation-2861975-11.patch

We found that the thumbnail generate the correct image and path

After we tried to clear the drupal cache, the thumbnail cannot generate

Please see the attachment

Thanks

Hi Support

We found that different behavior using default browser

If using chrome, the broken thumbnail image is appear, please see the attachment as broken thumbnail image.png

When we tried open the URL of the thumbnail, the following error is appear, please see the attachment

"Error generating image, missing source file."

Thanks a lot

I was also seeing issues with image style derivative generation and was able to work around it using the attached patch. It's similar to the one wouters_f provided in that it changes the cache max-age of the redirect response from S3fsImageStyleDownloadController, but I just hard coded it to zero instead. I did that because if the response is cached at all, then sometimes requests to this controller wouldn't get to it, resulting in no image generation.

For reference, here's what I changed in this patch:

$response = new TrustedRedirectResponse(file_create_url($derivative_uri));

// Don't cache the redirect because doing so breaks style derivative generation.
// On the first request to this controller, the generation works, but returns
// a cacheable response, meaning any subsequent requests for generation won't hit
// this controller, even if it's for a different file. Seems like the cache is
// matching the route prefix '/s3/files/styles/'.
$cache_dependency = new CacheableMetadata();
$cache_dependency->setCacheMaxAge(0);
$response->addCacheableDependency($cache_dependency);

return $response;

If this is a totally bad or weird thing to do, please let me know.

Facing the same issue.
Thumbnails are being generated only after second or third page reload.

Image does not pass GDToolkit isValid() method because read stream returns no data for a while.

I'm also finding that there are weird caching issues with responsive image styles and S3FS, if you have the Dynamic Internal Page Cache enabled. Still trying to figure out what exactly is going wrong, but I've narrowed it down to S3FS for now.

Thumbnails are not getting generated and failed in this method isValid() .Always returns "false" on this isValid method.

/core/modules/image/src/Entity/ImageStyle.php
// If the source file doesn't exist, return FALSE without creating folders.
$image = \Drupal::service('image.factory')->get($original_uri);
if (!$image->isValid()) {
return FALSE;
}

Thanks,
Shankar

Hello Guys,

I have the same problem with image styles. However I think the issue here is more deep and I've made a patch for a small workaround until we have a working solution with Drupal cache.
The problem is that the result of route "/s3/files/styles/{image_style}/{scheme}", should be cached only in combination with $_GET['file'] parameter because this is the unique combination for caching the result.
Ideally, we should not cache the result of this route in my opinion but we should have some kind of cache mechanism.
For example in the local file system using ImageStyleDownloadController::deliver(), nginx will prevent Drupal bootstrap if file exists so we can mark the route as not cacheable because it will be only called when missing file in the file system.
However in the S3 context this is not exactly like this, so not caching the result will cause a big performance impact (actually not tested how is going to work with CNAME) because each file will call Drupal for the redirect to S3.
Attaching the workaround until we brainstorm what is possible here and what will be the best.

Providing a proposal of how to fix the issues and having also Dynamic Page Cache working for redirects.
By default the cache should not work until you setup the TTL in the configuration by extending a little bit the @wouters_f patch.

Hi jansete,

Thanks for the work on the module, but sorry to report that 8.x-3.0-alpha4 doesn't seem to fix the issues around style generated images being stored in S3, at least on my site.

We're using version 8.x-3.0-alpha4 of the module. $settings['s3fs.use_s3_for_public'] is TRUE.

I'm seeing a lot of thumbnails (generated through image styles) loading completely incorrectly, and a lot of 404's for images that I'm certain actually have source images to generate from.

Sorry I can't give real URL's, the site isn't live yet, and the client is sensitive around disclosure of their info. I've search / replaced URL's, image style names and filenames below.

I have a search page (a view backed by search_api_solr) that renders 10 results, each with a thumbnail. The thumbnails use my "my_image_style" image style. Looking at the search page I can see that many of the thumbnails appear to be "wrong" i.e. not the actual image uploaded to the listed node.

Using S3 browser I can see the path s3fs-public/styles/my_image_style/public/import/ contains 23 files.

If I use Chrome network tab with "preserve log" and "disable cache" both checked I can load a thumbnail for one of the ones that is displayed wrong (e.g. http://uat.site.example.com/ s3/files/styles/my_image_style/public/import/ rpc.jpg) and see that I get a 302 redirect to a seemingly unrelated thumbnail (e.g. https://s3-ap-southeast-2.amazonaws.com/ my-s3-bucket/s3fs-public/styles/my_image_style/public/import/ xyz.jpg).

I then 'drush cr' to clear cache and ctrl-F5 to hard refresh the page.

Using S3 browser I can see that the path now contains 24 files - exactly one more - so it seems like the code has managed to generate one styled image, but failed on the others, possibly due to the locking in the code. Many of my images are still displaying the wrong thumbnail. Subsequent refreshes fail to update the other thumbnails.

e.g. http://uat.site.example.com/ s3/files/styles/my_image_style/ public/import/ aipc.jpg 302 redirects to https://s3-ap-southeast-2.amazonaws.com/ my-s3-bucket/s3fs-public/styles/my_image_style/ public/import/ blah.jpg.

(removed some extra info here that was a separate issue due to misconfig my end)

@jansete,
In committed 4cfd9e8 on 8.x-3.x you have added only the change for redirect module, but actually the fix for image styles was not.
If you compare the two patches, https://www.drupal.org/files/issues/image_style_generation-2861975-22.patch and https://www.drupal.org/files/issues/2861975-image-styles-generation.patch

The one that I've provide in #22 is the actual that fix the problem with styling.

@George Bills, you can also check the changes in #22 and provide a feedback if this is fixing the problem.

Hi @skek @jansete, the patch in #22 works for me.

The patch won't apply cleanly - there's conflicts in S3fsImageStyleRoutes.php but those changed lines look just like they're changing syntax from "array()" to "[]" so that's not a big deal.

Process was - apply patch, uninstall and reinstall module (to load in the schema change), config import to restore my S3 settings, image styles now work.

Hi @jansete - I didn't write the code, but I think it's fine? The caching varies by file and itok query params, so at least it won't cache a 302 for a completely different image anymore. I guess the ttl only makes a difference if the base file is reuploaded while the styled image is still cached.

Hi @jansete patch #22 works for me.
To reproduce problem you need to enable cache:
If you have such settings in your settings.php file

<?php
$settings['cache']['bins']['render'] = 'cache.backend.null';
$settings['cache']['bins']['dynamic_page_cache'] = 'cache.backend.null';
?>

just comment it.