Cleaned up some warnings in bulk import/export extension

Added transactions to bulk import
Renamed beginTransaction to begin_transaction for naming consistency
Updated cron uploader to handle bulk import transactions

core/database.php

@@ -60,7 +60,7 @@ class Database
         $this->connect_engine();
         $this->engine->init($this->db);
 
-        $this->beginTransaction();
+        $this->begin_transaction();
     }
 
     private function connect_engine(): void
@@ -82,7 +82,7 @@ class Database
         }
     }
 
-    public function beginTransaction(): void
+    public function begin_transaction(): void
     {
         if ($this->transaction === false) {
             $this->db->beginTransaction();
@@ -90,9 +90,14 @@ class Database
         }
     }
 
+    public function is_transaction_open(): bool
+    {
+        return !is_null($this->db) && $this->transaction === true;
+    }
+
     public function commit(): bool
     {
-        if (!is_null($this->db) && $this->transaction === true) {
+        if ($this->is_transaction_open()) {
             $this->transaction = false;
             return $this->db->commit();
         } else {
@@ -102,7 +107,7 @@ class Database
 
     public function rollback(): bool
     {
-        if (!is_null($this->db) && $this->transaction === true) {
+        if ($this->is_transaction_open()) {
             $this->transaction = false;
             return $this->db->rollback();
         } else {

ext/bulk_import_export/events.php

@@ -15,6 +15,7 @@ class BulkExportEvent extends Event
 class BulkImportEvent extends Event
 {
     public $image;
+    public $fields = [];
 
     public function __construct(Image $image, $fields)
     {

ext/bulk_import_export/main.php

@@ -10,7 +10,7 @@ class BulkImportExport extends DataHandlerExtension
 
     public function onDataUpload(DataUploadEvent $event)
     {
-        global $user;
+        global $user, $database;
 
         if ($this->supported_ext($event->type) &&
             $user->can(Permissions::BULK_IMPORT)) {
@@ -32,23 +32,26 @@ class BulkImportExport extends DataHandlerExtension
                 $total = 0;
                 $skipped = 0;
 
+                $database->commit();
+
                 while (!empty($json_data)) {
                     $item = array_pop($json_data);
-                    $image = Image::by_hash($item->hash);
+                    $database->begin_transaction();
-                    if ($image!=null) {
-                        $skipped++;
-                        log_info("BulkImportExport", "Image $item->hash already present, skipping");
-                        continue;
-                    }
-
-                    $tmpfile = tempnam("/tmp", "shimmie_bulk_import");
-                    $stream = $zip->getStream($item->hash);
-                    if ($zip === false) {
-                        log_error("BulkImportExport", "Could not import " . $item->hash . ": File not in zip", "Could not import " . $item->hash . ": File not in zip");
-                        continue;
-                    }
-
                     try {
+                        $image = Image::by_hash($item->hash);
+                        if ($image!=null) {
+                            $skipped++;
+                            log_info(BulkImportExportInfo::KEY, "Image $item->hash already present, skipping");
+                            $database->commit();
+                            continue;
+                        }
+
+                        $tmpfile = tempnam("/tmp", "shimmie_bulk_import");
+                        $stream = $zip->getStream($item->hash);
+                        if ($zip === false) {
+                            throw new SCoreException("Could not import " . $item->hash . ": File not in zip");
+                        }
+
                         file_put_contents($tmpfile, $stream);
 
                         $id = add_image($tmpfile, $item->filename, Tag::implode($item->tags));
@@ -68,20 +71,25 @@ class BulkImportExport extends DataHandlerExtension
                         }
                         send_event(new BulkImportEvent($image, $item));
 
+                        $database->commit();
                         $total++;
                     } catch (Exception $ex) {
-                        throw new SCoreException("Could not import " . $item->hash . ": " . $ex->getMessage());
+                        try {
-                        //log_error("BulkImportExport", "Could not import " . $item->hash . ": " . $ex->getMessage(), "Could not import " . $item->hash . ": " . $ex->getMessage());
+                            $database->rollBack();
-                        //continue;
+                        } catch (Exception $ex2) {
+                            log_error(BulkImportExportInfo::KEY, "Could not roll back transaction: " . $ex2->getMessage(), "Could not import " . $item->hash . ": " . $ex->getMessage());
+                        }
+                        log_error(BulkImportExportInfo::KEY, "Could not import " . $item->hash . ": " . $ex->getMessage(), "Could not import " . $item->hash . ": " . $ex->getMessage());
+                        continue;
                     } finally {
-                        if (is_file($tmpfile)) {
+                        if (!empty($tmpfile) && is_file($tmpfile)) {
                             unlink($tmpfile);
                         }
                     }
                 }
                 $event->image_id = -2; // default -1 = upload wasn't handled
 
-                log_info("BulkImportExport", "Imported $total items, skipped $skipped", "Imported $total items, skipped $skipped");
+                log_info(BulkImportExportInfo::KEY, "Imported $total items, skipped $skipped", "Imported $total items, skipped $skipped");
             } else {
                 throw new SCoreException("Could not open zip archive");
             }
@@ -135,8 +143,6 @@ class BulkImportExport extends DataHandlerExtension
                 $page->set_mode(PageMode::FILE);
                 $page->set_file($zip_filename, true);
                 $page->set_filename(basename($zip_filename));
-
-                $event->redirect = false;
             }
         }
     }

ext/cron_uploader/main.php

@@ -351,7 +351,7 @@ class CronUploader extends Extension
                     break;
                 }
                 try {
-                    $database->beginTransaction();
+                    $database->begin_transaction();
                     $this->log_message(SCORE_LOG_INFO, "Adding file: {$img[0]} - tags: {$img[2]}");
                     $result = $this->add_image($img[0], $img[1], $img[2]);
                     $database->commit();
@@ -363,7 +363,9 @@ class CronUploader extends Extension
                     }
                 } catch (Exception $e) {
                     try {
-                        $database->rollback();
+                        if ($database->is_transaction_open()) {
+                            $database->rollback();
+                        }
                     } catch (Exception $e) {
                     }
 

ext/media/main.php

@@ -1050,7 +1050,7 @@ class Media extends Extension
 
             $this->set_version(MediaConfig::VERSION, 2);
 
-            $database->beginTransaction();
+            $database->begin_transaction();
         }
     }
 }

ext/transcode/main.php

@@ -182,7 +182,7 @@ class TranscodeImage extends Extension
                     $size_difference = 0;
                     foreach ($event->items as $image) {
                         try {
-                            $database->beginTransaction();
+                            $database->begin_transaction();
 
                             $before_size =  $image->filesize;