Remove imagemagick texttowrapper temporary files on shutdown

Thank you. Can we add tests for this?

1. +++ b/src/Plugin/ImageToolkit/Operation/imagemagick/TextToWrapper.php
   @@ -34,9 +34,25 @@ class TextToWrapper extends ImagemagickImageToolkitOperationBase {
   +      // Remove temporary files after they've been processed
   +      register_shutdown_function([
   +        '\Drupal\image_effects\Plugin\ImageToolkit\Operation\imagemagick\TextToWrapper',
   +        'delete',
   +      ], $gd_wrapper_destination);
   

   I think static::class instead of FQCN should work the same.

2. +++ b/src/Plugin/ImageToolkit/Operation/imagemagick/TextToWrapper.php
   @@ -34,9 +34,25 @@ class TextToWrapper extends ImagemagickImageToolkitOperationBase {
   +  public static function delete($file_path) {
   

   This methods needs a more descriptive name ... deleteTempWrapperFile maybe?

Also:

+++ b/src/Plugin/ImageToolkit/Operation/imagemagick/TextToWrapper.php
@@ -34,9 +34,25 @@ class TextToWrapper extends ImagemagickImageToolkitOperationBase {
+      register_shutdown_function([

I think we should use drupal_register_shutdown_function here.

and some CS issues:

FILE: .../src/Plugin/ImageToolkit/Operation/imagemagick/TextToWrapper.php
----------------------------------------------------------------------
FOUND 5 ERRORS AFFECTING 5 LINES
----------------------------------------------------------------------
 37 | ERROR | [x] Inline comments must end in full-stops, exclamation
    |       |     marks, colons, question marks, or closing
    |       |     parentheses
 48 | ERROR | [x] Doc comment short description must end with a full
    |       |     stop
 51 | ERROR | [x] Parameter comment must end with a full stop
 57 | ERROR | [x] Expected 1 blank line after function; 0 found
 58 | ERROR | [x] The closing brace for the class must have an empty
    |       |     line before it
----------------------------------------------------------------------
PHPCBF CAN FIX THE 5 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------

Perhaps do it in the effect class? This will cover any image toolkit that creates a temporary file on the fly for this effect.

Some notes:
- the needs tests remains
- file_unmanaged_delete() does handle non-existing files but creates a log entry for it, so I also do a check with file_exists() before calling file_unmanaged_delete().

@fietserwin unfortunately #6 won't work: the wrapper image file needs to be available for the ::save method of the Imagemagick toolkit (when the convert executable is invoked), which is only called at the end of the ImageStyle::createDerivative method when all effects have been applied to the source image.

Yes, you're right, like I did in D7 (using a hook_exit). Thu indeed back to the patch of #2.

Here's #2 with a test and a test-only patch. Nothing will fail until #3046619: Introduce DrupalCI configuration to install ImageMagick and GraphicsMagick on testbots is solved, since tests for ImageMagick toolkit are currently skipped on DrupalCI testbots.

Needs work anyway for #3 and #5.

#3046619: Introduce DrupalCI configuration to install ImageMagick and GraphicsMagick on testbots is committed, retesting the patches in #9.

Addressed #3 and #5.

Fine with me, though I would have used different names, see below. Commit either with or without the changes suggested, without a need to set it back to NR.

Aside: where does the name $gd_wrapper[_destination] come from? We are not in the gd toolkit.

1. +++ b/src/Plugin/ImageToolkit/Operation/imagemagick/TextToWrapper.php
   @@ -29,14 +29,29 @@ class TextToWrapper extends ImagemagickImageToolkitOperationBase {
   +   *   Path of the file that's about to be deleted.
   

   that's => that is (like it's was changed to it has).

2. +++ b/src/Plugin/ImageToolkit/Operation/imagemagick/TextToWrapper.php
   @@ -29,14 +29,29 @@ class TextToWrapper extends ImagemagickImageToolkitOperationBase {
   +  public static function deleteTempWrapperFile($file_path) {
   

   deleteTempFile($filename) (Wrapper in the method name does not add any clarity, $filename seems to be used in most php and Drupal file functions, though Drupal also uses $uri/$url or $path (the latter when it might also be a directory))

Thanks @fietserwin.

Changes on commit:

@@ -33,24 +33,24 @@ class TextToWrapper extends ImagemagickImageToolkitOperationBase {
       $tmp_file = \Drupal::service('file_system')->tempnam('temporary://', 'ifx');
       $gd_wrapper_destination = $tmp_file . '.png';
       file_unmanaged_move($tmp_file, $gd_wrapper_destination, FILE_CREATE_DIRECTORY);
       $gd_wrapper->save($gd_wrapper_destination);
       $tmp_wrapper = \Drupal::service('image.factory')->get($gd_wrapper_destination, 'imagemagick');
-      // Remove temporary files after they've been processed.
-      drupal_register_shutdown_function([static::class, 'deleteTempWrapperFile'], $gd_wrapper_destination);
+      // Defer removal of the temporary file to after it has been processed.
+      drupal_register_shutdown_function([static::class, 'deleteTempFile'], $gd_wrapper_destination);
       return $this->getToolkit()->apply('replace_image', ['replacement_image' => $tmp_wrapper]);
     }
     return FALSE;
   }
 
   /**
    * Delete the image effect temporary file after it has been used.
    *
    * @param string $file_path
-   *   Path of the file that's about to be deleted.
+   *   Path of the file that is about to be deleted.
    */
-  public static function deleteTempWrapperFile($file_path) {
+  public static function deleteTempFile($file_path) {
     if (file_exists($file_path)) {
       file_unmanaged_delete($file_path);
     }
   }
 

where does the name $gd_wrapper[_destination] come from? We are not in the gd toolkit.

We are using the GD toolkit here, just to prepare an image with hte text inside (the wrapper) that is then overlaid via IM. Unfortunately it won't be easy to develop a native IM toolkit operation for the same with all the options that this operation has.

Committed. Thanks all.

Oops, a dependency on the gd library (at least not the toolkit) in the IM toolkit... :)

Nono, it depends on the GD toolkit actually :) Same for the Smart Crop effects.