[core] have ImageReplace replace image->file with a new file, not replace a whole Image object

core/extension.php

@@ -309,26 +309,16 @@ abstract class DataHandlerExtension extends Extension
                 throw new UploadException("Invalid or corrupted file");
             }
 
-            $this->move_upload_to_archive($event);
-
             /* Check if we are replacing an image */
             if (!is_null($event->replace_id)) {
-                /* hax: This seems like such a dirty way to do this.. */
-
-                /* Check to make sure the image exists. */
                 $existing = Image::by_id($event->replace_id);
-
                 if (is_null($existing)) {
                     throw new UploadException("Post to replace does not exist!");
                 }
-                if ($existing->hash === $event->hash) {
+                send_event(new ImageReplaceEvent($existing, $event->tmpname));
-                    throw new UploadException("The uploaded post is the same as the one to replace.");
+                $event->images[] = $existing;
-                }
-
-                $replacement = $this->create_image_from_data(warehouse_path(Image::IMAGE_DIR, $event->hash), $event->metadata);
-                send_event(new ImageReplaceEvent($existing, $replacement, $event->metadata));
-                $event->images[] = $replacement;
             } else {
+                $this->move_upload_to_archive($event);
                 $image = $this->create_image_from_data(warehouse_path(Image::IMAGE_DIR, $event->hash), $event->metadata);
 
                 $existing = Image::by_hash($image->hash);

core/imageboard/event.php

@@ -52,21 +52,23 @@ class ImageDeletionEvent extends Event
  */
 class ImageReplaceEvent extends Event
 {
+    public string $original_hash;
+    public string $new_hash;
+
     /**
-     * Replaces an image.
+     * Replaces an image file.
      *
      * Updates an existing ID in the database to use a new image
      * file, leaving the tags and such unchanged. Also removes
      * the old image file and thumbnail from the disk.
-     *
-     * @param mixed[] $metadata
      */
     public function __construct(
-        public Image $original,
+        public Image $image,
-        public Image $replacement,
+        public string $tmp_filename,
-        public array $metadata = [],
     ) {
         parent::__construct();
+        $this->original_hash = $image->hash;
+        $this->new_hash = md5_file($tmp_filename);
     }
 }
 

ext/image/main.php

@@ -143,41 +143,36 @@ class ImageIO extends Extension
 
     public function onImageReplace(ImageReplaceEvent $event)
     {
-        $original = $event->original;
+        $image = $event->image;
-        $replacement = $event->replacement;
 
         try {
-            $duplicate = Image::by_hash($replacement->hash);
+            $duplicate = Image::by_hash($event->new_hash);
-            if (!is_null($duplicate) && $duplicate->id != $original->id) {
+            if (!is_null($duplicate) && $duplicate->id != $image->id) {
-                throw new ImageReplaceException(">>{$duplicate->id} already has hash {$replacement->hash}");
+                throw new ImageReplaceException("A different post >>{$duplicate->id} already has hash {$duplicate->hash}");
             }
 
-            $replacement->set_mime(
+            $image->remove_image_only(); // Actually delete the old image file from disk
-                MimeType::get_for_file($replacement->get_image_filename())
+
+            $target = warehouse_path(Image::IMAGE_DIR, $event->new_hash);
+            if (!@copy($event->tmp_filename, $target)) {
+                $errors = error_get_last();
+                throw new UploadException(
+                    "Failed to copy file from uploads ({$event->tmp_filename}) to archive ($target): ".
+                    "{$errors['type']} / {$errors['message']}"
                 );
-
-            // Update the data in the database.
-            if (empty($replacement->source)) {
-                $replacement->source = $original->get_source();
             }
-            $replacement->posted = $original->posted;
+            unlink($event->tmp_filename);
-            $replacement->id = $original->id;
-            send_event(new MediaCheckPropertiesEvent($replacement));
-            $replacement->save_to_db();
 
-            /*
+            // update metadata and save metadata to DB
-                This step could be optional, ie: perhaps move the image somewhere
+            $event->image->hash = $event->new_hash;
-                and have it stored in a 'replaced images' list that could be
+            $event->image->filesize = filesize($target);
-                inspected later by an admin?
+            $event->image->set_mime(MimeType::get_for_file($target));
-            */
+            send_event(new MediaCheckPropertiesEvent($image));
+            $image->save_to_db();
 
-            log_debug("image", "Removing image with hash " . $original->hash);
+            send_event(new ThumbnailGenerationEvent($image));
-            $original->remove_image_only(); // Actually delete the old image file from disk
 
-            /* Generate new thumbnail */
+            log_info("image", "Replaced >>{$image->id} {$event->original_hash} with {$event->new_hash}");
-            send_event(new ThumbnailGenerationEvent($replacement));
-
-            log_info("image", "Replaced >>{$original->id} with ({$replacement->hash})");
         } catch (ImageReplaceException $e) {
             throw new UploadException($e->error);
         }

ext/resize/main.php

@@ -263,25 +263,9 @@ class ResizeImage extends Extension
             Media::RESIZE_TYPE_STRETCH
         ));
 
-        $new_image = new Image();
+        send_event(new ImageReplaceEvent($image_obj, $tmp_filename));
-        $new_image->hash = md5_file($tmp_filename);
-        $new_image->filesize = filesize($tmp_filename);
-        $new_image->filename = 'resized-'.$image_obj->filename;
-        $new_image->width = $new_width;
-        $new_image->height = $new_height;
 
-        /* Move the new image into the main storage location */
+        log_info("resize", "Resized >>{$image_obj->id} - New hash: {$image_obj->hash}");
-        $target = warehouse_path(Image::IMAGE_DIR, $new_image->hash);
-        if (!@copy($tmp_filename, $target)) {
-            throw new ImageResizeException("Failed to copy new image file from temporary location ({$tmp_filename}) to archive ($target)");
-        }
-
-        /* Remove temporary file */
-        @unlink($tmp_filename);
-
-        send_event(new ImageReplaceEvent($image_obj, $new_image));
-
-        log_info("resize", "Resized >>{$image_obj->id} - New hash: {$new_image->hash}");
     }
 
     /**

ext/rotate/main.php

@@ -176,27 +176,15 @@ class RotateImage extends Extension
             throw new ImageRotateException("Could not save image: ".$tmp_filename);
         }
 
-        list($new_width, $new_height) = getimagesize($tmp_filename);
+        $new_hash = md5_file($tmp_filename);
-
-        $new_image = new Image();
-        $new_image->hash = md5_file($tmp_filename);
-        $new_image->filesize = filesize($tmp_filename);
-        $new_image->filename = 'rotated-'.$image_obj->filename;
-        $new_image->width = $new_width;
-        $new_image->height = $new_height;
-        $new_image->posted = $image_obj->posted;
-
         /* Move the new image into the main storage location */
-        $target = warehouse_path(Image::IMAGE_DIR, $new_image->hash);
+        $target = warehouse_path(Image::IMAGE_DIR, $new_hash);
         if (!@copy($tmp_filename, $target)) {
             throw new ImageRotateException("Failed to copy new image file from temporary location ({$tmp_filename}) to archive ($target)");
         }
 
-        /* Remove temporary file */
+        send_event(new ImageReplaceEvent($image_obj, $tmp_filename));
-        @unlink($tmp_filename);
 
-        send_event(new ImageReplaceEvent($image_obj, $new_image));
+        log_info("rotate", "Rotated >>{$image_id} - New hash: {$new_hash}");
-
-        log_info("rotate", "Rotated >>{$image_id} - New hash: {$new_image->hash}");
     }
 }

ext/s3/main.php

@@ -105,8 +105,8 @@ class S3 extends Extension
 
     public function onImageReplace(ImageReplaceEvent $event)
     {
-        $this->remove_file($event->original->hash);
+        $this->remove_file($event->original_hash);
-        $this->sync_post($event->replacement, $event->original->get_tag_array());
+        $this->sync_post($event->image);
     }
 
     // utils

ext/tag_edit/main.php

@@ -184,8 +184,8 @@ class TagEdit extends Extension
 
     public function onImageReplace(ImageReplaceEvent $event)
     {
-        if(!empty($event->metadata['source'])) {
+        if(!empty($_POST['source'])) {
-            send_event(new SourceSetEvent($event->replacement, $event->metadata['source']));
+            send_event(new SourceSetEvent($event->image, $_POST['source']));
         }
     }
 

ext/transcode/main.php

@@ -298,13 +298,13 @@ class TranscodeImage extends Extension
 
                             $before_size =  $image->filesize;
 
-                            $new_image = $this->transcode_and_replace_image($image, $mime);
+                            $this->transcode_and_replace_image($image, $mime);
                             // If a subsequent transcode fails, the database needs to have everything about the previous
                             // transcodes recorded already, otherwise the image entries will be stuck pointing to
                             // missing image files
                             $database->commit();
                             $total++;
-                            $size_difference += ($before_size - $new_image->filesize);
+                            $size_difference += ($before_size - $image->filesize);
                         } catch (\Exception $e) {
                             log_error("transcode", "Error while bulk transcode on item {$image->id} to $mime: ".$e->getMessage());
                             try {
@@ -353,31 +353,11 @@ class TranscodeImage extends Extension
 
 
 
-    private function transcode_and_replace_image(Image $image_obj, string $target_mime): Image
+    private function transcode_and_replace_image(Image $image, string $target_mime): void
     {
-        $original_file = warehouse_path(Image::IMAGE_DIR, $image_obj->hash);
+        $original_file = warehouse_path(Image::IMAGE_DIR, $image->hash);
-
+        $tmp_filename = $this->transcode_image($original_file, $image->get_mime(), $target_mime);
-        $tmp_filename = $this->transcode_image($original_file, $image_obj->get_mime(), $target_mime);
+        send_event(new ImageReplaceEvent($image, $tmp_filename));
-
-        $new_image = new Image();
-        $new_image->hash = md5_file($tmp_filename);
-        $new_image->filesize = filesize($tmp_filename);
-        $new_image->filename = $image_obj->filename;
-        $new_image->width = $image_obj->width;
-        $new_image->height = $image_obj->height;
-
-        /* Move the new image into the main storage location */
-        $target = warehouse_path(Image::IMAGE_DIR, $new_image->hash);
-        if (!@copy($tmp_filename, $target)) {
-            throw new ImageTranscodeException("Failed to copy new image file from temporary location ({$tmp_filename}) to archive ($target)");
-        }
-
-        /* Remove temporary file */
-        @unlink($tmp_filename);
-
-        send_event(new ImageReplaceEvent($image_obj, $new_image));
-
-        return $new_image;
     }
 
 
@@ -391,8 +371,6 @@ class TranscodeImage extends Extension
 
         $engine = $config->get_string(TranscodeConfig::ENGINE);
 
-
-
         if (!$this->can_convert_mime($engine, $source_mime)) {
             throw new ImageTranscodeException("Engine $engine does not support input MIME $source_mime");
         }

ext/transcode_video/main.php

@@ -157,12 +157,12 @@ class TranscodeVideo extends Extension
                         try {
                             $database->begin_transaction();
 
-                            $output_image = $this->transcode_and_replace_video($image, $format);
+                            $transcoded = $this->transcode_and_replace_video($image, $format);
                             // If a subsequent transcode fails, the database needs to have everything about the previous
                             // transcodes recorded already, otherwise the image entries will be stuck pointing to
                             // missing image files
                             $database->commit();
-                            if ($output_image != $image) {
+                            if ($transcoded) {
                                 $total++;
                             }
                         } catch (\Exception $e) {
@@ -199,10 +199,10 @@ class TranscodeVideo extends Extension
         return $output;
     }
 
-    private function transcode_and_replace_video(Image $image, string $target_mime): Image
+    private function transcode_and_replace_video(Image $image, string $target_mime): bool
     {
         if ($image->get_mime() == $target_mime) {
-            return $image;
+            return false;
         }
 
         if ($image->video == null || ($image->video === true && empty($image->video_codec))) {
@@ -215,31 +215,10 @@ class TranscodeVideo extends Extension
         }
 
         $original_file = warehouse_path(Image::IMAGE_DIR, $image->hash);
-
         $tmp_filename = tempnam(sys_get_temp_dir(), "shimmie_transcode_video");
-        try {
         $tmp_filename = $this->transcode_video($original_file, $image->video_codec, $target_mime, $tmp_filename);
-
+        send_event(new ImageReplaceEvent($image, $tmp_filename));
-            $new_image = new Image();
+        return true;
-            $new_image->hash = md5_file($tmp_filename);
-            $new_image->filesize = filesize($tmp_filename);
-            $new_image->filename = $image->filename;
-            $new_image->width = $image->width;
-            $new_image->height = $image->height;
-
-            /* Move the new image into the main storage location */
-            $target = warehouse_path(Image::IMAGE_DIR, $new_image->hash);
-            if (!@copy($tmp_filename, $target)) {
-                throw new VideoTranscodeException("Failed to copy new post file from temporary location ({$tmp_filename}) to archive ($target)");
-            }
-
-            send_event(new ImageReplaceEvent($image, $new_image));
-
-            return $new_image;
-        } finally {
-            /* Remove temporary file */
-            @unlink($tmp_filename);
-        }
     }
 
 

ext/upload/test.php

@@ -60,11 +60,15 @@ class UploadTest extends ShimmiePHPUnitTestCase
 
         sleep(1); // make sure the timestamp changes (see bug #903)
 
+        // create a copy because the file is deleted after upload
+        $tmpfile = tempnam(sys_get_temp_dir(), "shimmie_test");
+        copy("tests/bedroom_workshop.jpg", $tmpfile);
+
         $_FILES = [
             'data' => [
                 'name' => ['puppy-hugs.jpg'],
                 'type' => ['image/jpeg'],
-                'tmp_name' => ['tests/bedroom_workshop.jpg'],
+                'tmp_name' => [$tmpfile],
                 'error' => [0],
                 'size' => [271386],
             ]