Contrib.social

Add possibility to skip flushing all image style derivatives when an image style configuration changes

Open on Drupal.org →
Created on 1 July 2022, about 3 years ago
Updated 17 February 2023, over 2 years ago

Problem/Motivation

Right now, if an image style configuration changes, it removes all the generated images in this function:

  public function postSave(EntityStorageInterface $storage, $update = TRUE) {
    parent::postSave($storage, $update);

    if ($update) {
      if (!empty($this->original) && $this->id() !== $this->original->id()) {
        // The old image style name needs flushing after a rename.
        $this->original->flush();
        // Update field settings if necessary.
        if (!$this->isSyncing()) {
          static::replaceImageStyle($this);
        }
      }
      else {
        // Flush image style when updating without changing the name.
        $this->flush();
      }
    }
  }

Now, on a huge high-traffic website will gigabytes of images, this means the server must regenerate everything. For example:
- if you consider doing that on a 100 GB dataset, then the server load will be going ultimately high later because of regenerating this much content again.
- Also, if you're using AWS S3 or Google Cloud Storage for files, this means a lot of extra traffic and file writing process. That adds extra cost.

Situations where we found this idea useful:
- We already generated WEBP derivatives with ImageAPI Optimaze WEBP. But when core was able to generate WEBP images we switched to that. We wanted to reuse those existing generated WEBP images after the image style config change, and not regenerate thousands of same ones.
- Newsletters were sent out with embedded JPG derivatives and we didn't want to remove those JPG images when we changed the image style to convert to WEBP always.

Also, I think only a little percent of visitors or even content editors care about the older images. It would be awesome if we could temporarily switch off automatic removing derivatives while we're deploying the new configuration, for example in the settings.php. What do you think?

Steps to reproduce

Proposed resolution

Remaining tasks

Update issue summary
Add test coverage
Code review

User interface changes

API changes

Data model changes

Release notes snippet

✨ Feature request
Status

Needs work

Version

10.1 ✨

Component
Image module  →

Last updated 11 days ago

Created by

🇸🇰Slovakia kaszarobert

Live updates comments and jobs are added and updated live.
  • Needs tests

    The change is currently missing an automated test that fails when run with the original code, and succeeds when the bug has been fixed.

  • Needs issue summary update

    Issue summaries save everyone time if they are kept up-to-date. See Update issue summary task instructions.

Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • Comment over 2 years ago →
    🇺🇸United States smustgrave

    This issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request → as a guide.

    This will need an issue summary update for proposed solution.

    This will need test coverage

  • Comment about 1 year ago →
    🇫🇮Finland ZeiP

    We had another kind of problem related to that, namely that the deployments took quite long as we're using an S3 API for images and deleting the image styles takes a long time during config import. We decided to solve this by adding the image styles to a queue that is handled after the config import has been done. Attaching a patch of the concept here (it's not supposed to be completely working, but just to share our idea of the solution.)

  • Comment about 2 months ago →
    🇨🇦Canada b_sharpe

    Adding the +1 for visibility here as it's getting far more common to use S3-type storage here

  • First commit to issue fork.
  • Merge request !12408Resolve #3293788 "Add possibility to" → (Open) created by oily
  • Comment about 2 months ago →
    🇬🇧United Kingdom oily Greater London

    For test coverage maybe use the existing test: /core/modules/image/tests/src/Functional/ImageStyleFlushTest.php

  • Comment about 2 months ago →
    🇬🇧United Kingdom oily Greater London

    Created ImageStyleEditTest. Needs to be refactored.

  • Pipeline finished with Success
    about 2 months ago
    Total: 395s
    #526046
contrib.social Blog FAQ Discussions
Production build 0.71.5 2024