hook_image_style_flush doesn't get the $path passed to Drupal\image\Entity\ImageStyle::flush() method

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.

Think this could use a test case to show the issue.

Also since a new parameter is being added to the hook it will need a change record

thanks

Added a test. Adding a test-only for $path being passed is non-sensical since we wouldn't expect that to pass currently, but we can test the change where if you pass a $path to flush(), currently the hook is never called at all.
So added a test for that.

Test only is the interdiff.

The last submitted patch, 29: 2833129-29-TEST_ONLY.patch, failed testing. View results →

Test coverage looks solid.

Added an attempt at a CR, https://www.drupal.org/node/3373248 →

Would be great to get confirmation from the release managers that the behavior change here is acceptable for a minor release.

I think this is sensible for a minor release - more likely it fixes a bug for someone than breaks it, and easy mitigation if there's a problem.

Updated the IS, added a release note

The last submitted patch, 29: 2833129-29.patch, failed testing. View results →

Test failure was unrelated to this change, and tests are back to green, so reverting to RTBC.

These seems a surprising change for me. Currently the hook is only invoked when a whole style is being flushed and not a single path. Therefore all existing implementations are going to assume that the everything is being flushed and not just a single path and therefore act accordingly. That said I can only find 2 instances of this hook in contrib and one already requires this patch so I think I agree with @catch here - okay for a minor release.

Committed 996fb53 and pushed to 11.x. Thanks!

Did a couple of improvements on commit...

Ensured that $path is the expected value in the test

diff --git a/core/modules/image/tests/modules/image_module_test/image_module_test.module b/core/modules/image/tests/modules/image_module_test/image_module_test.module
index 7ff821971f1..3955e44b69e 100644
--- a/core/modules/image/tests/modules/image_module_test/image_module_test.module
+++ b/core/modules/image/tests/modules/image_module_test/image_module_test.module
@@ -44,5 +44,5 @@ function image_module_test_image_style_presave(ImageStyleInterface $style) {
  */
 function image_module_test_image_style_flush($style, $path = NULL) {
   $state = \Drupal::state();
-  $state->set('image_module_test_image_style_flush.called', TRUE);
+  $state->set('image_module_test_image_style_flush.called', $path);
 }
diff --git a/core/modules/image/tests/src/Functional/ImageStyleFlushTest.php b/core/modules/image/tests/src/Functional/ImageStyleFlushTest.php
index 337f3827cc6..13334291953 100644
--- a/core/modules/image/tests/src/Functional/ImageStyleFlushTest.php
+++ b/core/modules/image/tests/src/Functional/ImageStyleFlushTest.php
@@ -129,10 +129,9 @@ public function testFlush() {
     $state = \Drupal::state();
     $state->set('image_module_test_image_style_flush.called', FALSE);
     $style->flush();
-    $this->assertTrue($state->get('image_module_test_image_style_flush.called'));
-    $state->set('image_module_test_image_style_flush.called', FALSE);
+    $this->assertNull($state->get('image_module_test_image_style_flush.called'));
     $style->flush('/made/up/path');
-    $this->assertTrue($state->get('image_module_test_image_style_flush.called'));
+    $this->assertSame('/made/up/path', $state->get('image_module_test_image_style_flush.called'));
   }
 
 }

Added correct typehinit on commit to hook docs.

diff --git a/core/modules/image/image.api.php b/core/modules/image/image.api.php
index 83fde6a69c0..2f786fc318c 100644
--- a/core/modules/image/image.api.php
+++ b/core/modules/image/image.api.php
@@ -32,7 +32,7 @@ function hook_image_effect_info_alter(&$effects) {
  *
  * @param \Drupal\image\ImageStyleInterface $style
  *   The image style object that is being flushed.
- * @param $path
+ * @param string|null $path
  *   (optional) The original image path or URI. If it's supplied, only this
  *   image derivative will be flushed.
  */

Ran the tests locally prior to commit.

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

It seems this caused some regressions:

• 🐛 Uploading a file to media library flushes theme registry Active
• 🐛 With 10.2 image styles might get wrongly flushed Active