[core] only move file to warehouse if all the rest of the upload is ok, fixes #1004

core/extension.php

@@ -313,19 +313,8 @@ abstract class DataHandlerExtension extends Extension
                 }
             }
 
+            // Create a new Image object
             $filename = $event->tmpname;
-            // FIXME: this should happen after ImageAdditionEvent, but the thumbnail
-            // code assumes the file is in the archive already instead of using
-            // the image->get_image_filename() function
-            $filename = warehouse_path(Image::IMAGE_DIR, $event->hash);
-            if (!@copy($event->tmpname, $filename)) {
-                $errors = error_get_last();
-                throw new UploadException(
-                    "Failed to copy file from uploads ({$event->tmpname}) to archive ($filename): ".
-                    "{$errors['type']} / {$errors['message']}"
-                );
-            }
-
             assert(is_readable($filename));
             $image = new Image();
             $image->tmp_file = $filename;
@@ -341,13 +330,21 @@ abstract class DataHandlerExtension extends Extension
             } catch (MediaException $e) {
                 throw new UploadException("Unable to scan media properties $filename / $image->filename / $image->hash: ".$e->getMessage());
             }
+            $image->save_to_db(); // Ensure the image has a DB-assigned ID
 
-            // ensure $image has a database-assigned ID number
+            // Let everybody else know, so that TagEdit can set tags, Ratings can set ratings, etc
-            // before anything else happens
-            $image->save_to_db();
-
             $iae = send_event(new ImageAdditionEvent($image, $event->metadata));
 
+            // If everything is OK, then move the file to the archive
+            $filename = warehouse_path(Image::IMAGE_DIR, $event->hash);
+            if (!@copy($event->tmpname, $filename)) {
+                $errors = error_get_last();
+                throw new UploadException(
+                    "Failed to copy file from uploads ({$event->tmpname}) to archive ($filename): ".
+                    "{$errors['type']} / {$errors['message']}"
+                );
+            }
+
             $event->images[] = $iae->image;
         }
     }
@@ -357,13 +354,13 @@ abstract class DataHandlerExtension extends Extension
         $result = false;
         if ($this->supported_mime($event->image->get_mime())) {
             if ($event->force) {
-                $result = $this->create_thumb($event->image->hash, $event->image->get_mime());
+                $result = $this->create_thumb($event->image);
             } else {
                 $outname = $event->image->get_thumb_filename();
                 if (file_exists($outname)) {
                     return;
                 }
-                $result = $this->create_thumb($event->image->hash, $event->image->get_mime());
+                $result = $this->create_thumb($event->image);
             }
         }
         if ($result) {
@@ -392,7 +389,7 @@ abstract class DataHandlerExtension extends Extension
 
     abstract protected function media_check_properties(MediaCheckPropertiesEvent $event): void;
     abstract protected function check_contents(string $tmpname): bool;
-    abstract protected function create_thumb(string $hash, string $mime): bool;
+    abstract protected function create_thumb(Image $image): bool;
 
     protected function supported_mime(string $mime): bool
     {

core/imageboard/misc.php

@@ -135,18 +135,14 @@ function get_thumbnail_max_size_scaled(): array
 }
 
 
-function create_image_thumb(string $hash, string $mime, string $engine = null)
+function create_image_thumb(Image $image, string $engine = null)
 {
     global $config;
-
-    $inname = warehouse_path(Image::IMAGE_DIR, $hash);
-    $outname = warehouse_path(Image::THUMBNAIL_DIR, $hash);
-    $tsize = get_thumbnail_max_size_scaled();
     create_scaled_image(
-        $inname,
+        $image->get_image_filename(),
-        $outname,
+        $image->get_thumb_filename(),
-        $tsize,
+        get_thumbnail_max_size_scaled(),
-        $mime,
+        $image->get_mime(),
         $engine,
         $config->get_string(ImageConfig::THUMB_FIT)
     );

ext/bulk_import_export/main.php

@@ -156,7 +156,7 @@ class BulkImportExport extends DataHandlerExtension
         return false;
     }
 
-    protected function create_thumb(string $hash, string $mime): bool
+    protected function create_thumb(Image $image): bool
     {
         return false;
     }

ext/handle_archive/main.php

@@ -64,7 +64,7 @@ class ArchiveFileHandler extends DataHandlerExtension
         return false;
     }
 
-    protected function create_thumb(string $hash, string $mime): bool
+    protected function create_thumb(Image $image): bool
     {
         return false;
     }

ext/handle_cbz/main.php

@@ -24,12 +24,12 @@ class CBZFileHandler extends DataHandlerExtension
         unlink($tmp);
     }
 
-    protected function create_thumb(string $hash, string $mime): bool
+    protected function create_thumb(Image $image): bool
     {
-        $cover = $this->get_representative_image(warehouse_path(Image::IMAGE_DIR, $hash));
+        $cover = $this->get_representative_image($image->get_image_filename());
         create_scaled_image(
             $cover,
-            warehouse_path(Image::THUMBNAIL_DIR, $hash),
+            $image->get_thumb_filename(),
             get_thumbnail_max_size_scaled(),
             MimeType::get_for_file($cover),
             null

ext/handle_ico/main.php

@@ -28,10 +28,10 @@ class IcoFileHandler extends DataHandlerExtension
         }
     }
 
-    protected function create_thumb(string $hash, string $mime): bool
+    protected function create_thumb(Image $image): bool
     {
         try {
-            create_image_thumb($hash, $mime, MediaEngine::IMAGICK);
+            create_image_thumb($image, MediaEngine::IMAGICK);
             return true;
         } catch (MediaException $e) {
             log_warning("handle_ico", "Could not generate thumbnail. " . $e->getMessage());

ext/handle_mp3/main.php

@@ -22,9 +22,9 @@ class MP3FileHandler extends DataHandlerExtension
         // TODO: ->length = ???
     }
 
-    protected function create_thumb(string $hash, string $mime): bool
+    protected function create_thumb(Image $image): bool
     {
-        copy("ext/handle_mp3/thumb.jpg", warehouse_path(Image::THUMBNAIL_DIR, $hash));
+        copy("ext/handle_mp3/thumb.jpg", $image->get_thumb_filename());
         return true;
     }
 

ext/handle_pixel/main.php

@@ -42,19 +42,20 @@ class PixelFileHandler extends DataHandlerExtension
         return $info && in_array($info[2], $valid);
     }
 
-    protected function create_thumb(string $hash, string $mime): bool
+    protected function create_thumb(Image $image): bool
     {
         try {
-            create_image_thumb($hash, $mime);
+            create_image_thumb($image);
             return true;
         } catch (InsufficientMemoryException $e) {
+            log_warning("handle_pixel", "Insufficient memory while creating thumbnail: ".$e->getMessage());
             $tsize = get_thumbnail_max_size_scaled();
             $thumb = imagecreatetruecolor($tsize[0], min($tsize[1], 64));
             $white = imagecolorallocate($thumb, 255, 255, 255);
             $black = imagecolorallocate($thumb, 0, 0, 0);
             imagefill($thumb, 0, 0, $white);
-            log_warning("handle_pixel", "Insufficient memory while creating thumbnail: ".$e->getMessage());
             imagestring($thumb, 5, 10, 24, "Image Too Large :(", $black);
+            // FIXME: write the image to disk??
             return true;
         } catch (\Exception $e) {
             log_error("handle_pixel", "Error while creating thumbnail: ".$e->getMessage());

ext/handle_svg/main.php

@@ -64,19 +64,19 @@ class SVGFileHandler extends DataHandlerExtension
         $event->image->height = $msp->height;
     }
 
-    protected function create_thumb(string $hash, string $mime): bool
+    protected function create_thumb(Image $image): bool
     {
         try {
             // Normally we require imagemagick, but for unit tests we can use a no-op engine
             if (defined('UNITTEST')) {
-                create_image_thumb($hash, $mime);
+                create_image_thumb($image);
             } else {
-                create_image_thumb($hash, $mime, MediaEngine::IMAGICK);
+                create_image_thumb($image, MediaEngine::IMAGICK);
             }
             return true;
         } catch (MediaException $e) {
             log_warning("handle_svg", "Could not generate thumbnail. " . $e->getMessage());
-            copy("ext/handle_svg/thumb.jpg", warehouse_path(Image::THUMBNAIL_DIR, $hash));
+            copy("ext/handle_svg/thumb.jpg", $image->get_thumb_filename());
             return false;
         }
     }

ext/handle_video/main.php

@@ -121,9 +121,9 @@ class VideoFileHandler extends DataHandlerExtension
         return MimeType::matches_array($mime, $enabled_formats, true);
     }
 
-    protected function create_thumb(string $hash, string $mime): bool
+    protected function create_thumb(Image $image): bool
     {
-        return Media::create_thumbnail_ffmpeg($hash);
+        return Media::create_thumbnail_ffmpeg($image);
     }
 
     protected function check_contents(string $tmpname): bool

ext/media/main.php

@@ -317,7 +317,7 @@ class Media extends Extension
      * @return bool true if successful, false if not.
      * @throws MediaException
      */
-    public static function create_thumbnail_ffmpeg($hash): bool
+    public static function create_thumbnail_ffmpeg(Image $image): bool
     {
         global $config;
 
@@ -327,10 +327,10 @@ class Media extends Extension
         }
 
         $ok = false;
-        $inname = warehouse_path(Image::IMAGE_DIR, $hash);
+        $inname = $image->get_image_filename();
         $tmpname = tempnam(sys_get_temp_dir(), "shimmie_ffmpeg_thumb");
         try {
-            $outname = warehouse_path(Image::THUMBNAIL_DIR, $hash);
+            $outname = $image->get_thumb_filename();
 
             $orig_size = self::video_size($inname);
             $scaled_size = get_thumbnail_size($orig_size[0], $orig_size[1], true);