From 9b9f1d03413dad3b11ca8bea0132864f7d8eb521 Mon Sep 17 00:00:00 2001 From: Matthew Barbour Date: Fri, 12 Jun 2020 13:39:57 -0500 Subject: [PATCH] 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 | 13 +++++--- ext/bulk_import_export/events.php | 1 + ext/bulk_import_export/main.php | 50 +++++++++++++++++-------------- ext/cron_uploader/main.php | 6 ++-- ext/media/main.php | 2 +- ext/transcode/main.php | 2 +- 6 files changed, 44 insertions(+), 30 deletions(-) diff --git a/core/database.php b/core/database.php index 56639cf8..59afd839 100644 --- a/core/database.php +++ b/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 { diff --git a/ext/bulk_import_export/events.php b/ext/bulk_import_export/events.php index 8b063bc3..62244709 100644 --- a/ext/bulk_import_export/events.php +++ b/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) { diff --git a/ext/bulk_import_export/main.php b/ext/bulk_import_export/main.php index 1a5d3991..8cf38b20 100644 --- a/ext/bulk_import_export/main.php +++ b/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); - 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; - } - + $database->begin_transaction(); 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()); - //log_error("BulkImportExport", "Could not import " . $item->hash . ": " . $ex->getMessage(), "Could not import " . $item->hash . ": " . $ex->getMessage()); - //continue; + try { + $database->rollBack(); + } 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; } } } diff --git a/ext/cron_uploader/main.php b/ext/cron_uploader/main.php index 40f67930..5e6a1f7e 100644 --- a/ext/cron_uploader/main.php +++ b/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) { } diff --git a/ext/media/main.php b/ext/media/main.php index 50d548ad..0ec336ed 100644 --- a/ext/media/main.php +++ b/ext/media/main.php @@ -1050,7 +1050,7 @@ class Media extends Extension $this->set_version(MediaConfig::VERSION, 2); - $database->beginTransaction(); + $database->begin_transaction(); } } } diff --git a/ext/transcode/main.php b/ext/transcode/main.php index 42f7b8c3..c68d2665 100644 --- a/ext/transcode/main.php +++ b/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;