Problem/Motivation

Sometimes we need to respond on new image derivative creation.

Steps to reproduce

  • Create an image style
  • Upload a new image
  • Call the uri of the new style
  • The derivative image is generated, but there's no way to react to this, like sending some HTTP request to another system to inform about the new derivative image creation

Proposed resolution

Introduce a new hook_image_derivative_created(string $original_uri, string $style, string $derivative_uri) that can be implemented by other modules that will handle the derivative image creation.

Remaining tasks

Accept and commit.

User interface changes

None.

Introduced terminology

None.

API changes

Introduces new hook_image_derivative_created(string $original_uri, string $style, string $derivative_uri).

Data model changes

None.

Release notes snippet

Introduce new hook_image_derivative_created(string $original_uri, string $style, string $derivative_uri).

CommentFileSizeAuthor
#16 add-hook_image_derivative_created--2940016--16.patch1.2 KBefpapado
#15 add-hook_image_derivative_created--2940016--15.patch1.2 KBefpapado
#13 2940016-nr-bot.txt1.74 KBneeds-review-queue-bot
add-hook_image_derivative_created.patch1.92 KBtikaszvince

Issue fork drupal-2940016

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

tikaszvince created an issue. See original summary.

tikaszvince’s picture

tikaszvince commented
Status: Active » Needs review

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

mondrake’s picture

mondrake
Location 🇮🇹
commented
Related issues: +#2986669: Split ImageStyle into the config entity and a separate event-based image processing service

Related, #2986669: Split ImageStyle into the config entity and a separate event-based image processing service

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Needs review » Needs work
StatusFileSize
new 2940016-nr-bot.txt1.74 KB

The Needs Review Queue Bot tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

efpapado’s picture

efpapado commented
Status: Needs work » Needs review

The patch does apply on 10.1.x. Setting to Needs review and retesting.

efpapado’s picture

efpapado commented
StatusFileSize
new add-hook_image_derivative_created--2940016--15.patch1.2 KB

I'm proposing another patch, which has (almost) the same code as yours, but on a different class: Instead of invoking the hook on ImageStyleDownloadController::deliver(), I invoke it on ImageStyle::createDerivative() to make sure that the hook is invoked also when the derivative is created by other methods (for example through the image_style_warmer module).

efpapado’s picture

efpapado commented
StatusFileSize
new add-hook_image_derivative_created--2940016--16.patch1.2 KB

Fixed PHPCS problems.

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work
Issue tags: +Needs Review Queue Initiative, +Needs change record, +Needs issue summary update, +Needs tests

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.

Adding a new api will require a change record. An example is usually very helpful.

Issue summary should be updated to more clearly define the proposed solution and api change.

Will also need test coverage of the api working

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

efpapado opened merge request !9257

efpapado’s picture

efpapado commented
Status: Needs work » Needs review

Updated the patch for the 11.x branch, created a merge request, and created a draft change record.

efpapado’s picture

efpapado commented

After some thought, I added another argument in the hook, which is the id of the image style. Although by default it can be derived from the derivative image uri, there can be cases where this is not possible (for example a custom implementation of an external file system like s3fs.)

Updated also the change record.

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

Sending to NW for the open tags

Issue summary update and test coverage

Looking at the MR new api should be typehinted also.

efpapado’s picture

efpapado commented
Status: Needs work » Needs review

Added type hints to api, added a test. Not sure about the issue summary update, isn't the change record enough?

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

No issue summary is separate from a CR. Recommend using default issue template

efpapado’s picture

efpapado commented
Issue summary: View changes
Status: Needs work » Needs review

Is it ok now?

efpapado’s picture

efpapado commented
Issue summary: View changes
smustgrave’s picture

smustgrave commented
Issue tags: -Needs issue summary update

Summary yes

But tests are failing too btw.

efpapado’s picture

efpapado commented

Removed a useless assertion that was breaking the tests.

efpapado’s picture

efpapado commented
Status: Needs review » Needs work

Setting it back to needs work, the tests are failing, although they succeed locally. I will investigate further...

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.