diff options
Diffstat (limited to 'modules/gallery/helpers')
| -rw-r--r-- | modules/gallery/helpers/gallery_installer.php | 43 | ||||
| -rw-r--r-- | modules/gallery/helpers/gallery_task.php | 113 | ||||
| -rw-r--r-- | modules/gallery/helpers/item.php | 63 |
3 files changed, 163 insertions, 56 deletions
diff --git a/modules/gallery/helpers/gallery_installer.php b/modules/gallery/helpers/gallery_installer.php index fe651a4e..86ff7ce5 100644 --- a/modules/gallery/helpers/gallery_installer.php +++ b/modules/gallery/helpers/gallery_installer.php @@ -315,7 +315,7 @@ class gallery_installer { module::set_var("gallery", "lock_timeout", 1); module::set_var("gallery", "movie_extract_frame_time", 3); - module::set_version("gallery", 53); + module::set_version("gallery", 54); } static function upgrade($version) { @@ -743,6 +743,47 @@ class gallery_installer { module::set_var("gallery", "movie_extract_frame_time", 3); module::set_version("gallery", $version = 53); } + + if ($version == 53) { + // In v54, we changed how we check for name and slug conflicts in Item_Model. Previously, + // we checked the whole filename. As a result, "foo.jpg" and "foo.png" were not considered + // conflicting if their slugs were different (a rare case in practice since server_add and + // uploader would give them both the same slug "foo"). Now, we check the filename without its + // extension. This upgrade stanza fixes any conflicts where they were previously allowed. + + // This might be slow, but if it times out it can just pick up where it left off. + + // Find and loop through each conflict (e.g. "foo.jpg", "foo.png", and "foo.flv" are one + // conflict; "bar.jpg", "bar.png", and "bar.flv" are another) + foreach (db::build() + ->select_distinct(array("parent_base_name" => + db::expr("CONCAT(`parent_id`, ':', LOWER(SUBSTR(`name`, 1, LOCATE('.', `name`) - 1)))"))) + ->select(array("C" => "COUNT(\"*\")")) + ->from("items") + ->where("type", "<>", "album") + ->having("C", ">", 1) + ->group_by("parent_base_name") + ->execute() as $conflict) { + list ($parent_id, $base_name) = explode(":", $conflict->parent_base_name, 2); + $base_name_escaped = Database::escape_for_like($base_name); + // Loop through the items for each conflict + foreach (db::build() + ->from("items") + ->select("id") + ->where("type", "<>", "album") + ->where("parent_id", "=", $parent_id) + ->where("name", "LIKE", "{$base_name_escaped}.%") + ->limit(1000000) // required to satisfy SQL syntax (no offset without limit) + ->offset(1) // skips the 0th item + ->execute() as $row) { + set_time_limit(30); + $item = ORM::factory("item", $row->id); + $item->name = $item->name; // this will force Item_Model to check for conflicts on save + $item->save(); + } + } + module::set_version("gallery", $version = 54); + } } static function uninstall() { diff --git a/modules/gallery/helpers/gallery_task.php b/modules/gallery/helpers/gallery_task.php index 82ca9ffc..e986b822 100644 --- a/modules/gallery/helpers/gallery_task.php +++ b/modules/gallery/helpers/gallery_task.php @@ -26,11 +26,13 @@ class gallery_task_Core { const FIX_STATE_RUN_DUPE_SLUGS = 5; const FIX_STATE_START_DUPE_NAMES = 6; const FIX_STATE_RUN_DUPE_NAMES = 7; - const FIX_STATE_START_REBUILD_ITEM_CACHES = 8; - const FIX_STATE_RUN_REBUILD_ITEM_CACHES = 9; - const FIX_STATE_START_MISSING_ACCESS_CACHES = 10; - const FIX_STATE_RUN_MISSING_ACCESS_CACHES = 11; - const FIX_STATE_DONE = 12; + const FIX_STATE_START_DUPE_BASE_NAMES = 8; + const FIX_STATE_RUN_DUPE_BASE_NAMES = 9; + const FIX_STATE_START_REBUILD_ITEM_CACHES = 10; + const FIX_STATE_RUN_REBUILD_ITEM_CACHES = 11; + const FIX_STATE_START_MISSING_ACCESS_CACHES = 12; + const FIX_STATE_RUN_MISSING_ACCESS_CACHES = 13; + const FIX_STATE_DONE = 14; static function available_tasks() { $dirty_count = graphics::find_dirty_images_query()->count_records(); @@ -348,8 +350,9 @@ class gallery_task_Core { // album audit (permissions and bogus album covers): 1 operation for every album $total += db::build()->where("type", "=", "album")->count_records("items"); - // one operation for each missing slug, name and access cache - foreach (array("find_dupe_slugs", "find_dupe_names", "find_missing_access_caches") as $func) { + // one operation for each dupe slug, dupe name, dupe base name, and missing access cache + foreach (array("find_dupe_slugs", "find_dupe_names", "find_dupe_base_names", + "find_missing_access_caches") as $func) { foreach (self::$func() as $row) { $total++; } @@ -489,11 +492,12 @@ class gallery_task_Core { $task->set("stack", implode(" ", $stack)); $state = self::FIX_STATE_RUN_DUPE_NAMES; } else { - $state = self::FIX_STATE_START_ALBUMS; + $state = self::FIX_STATE_START_DUPE_BASE_NAMES; } break; case self::FIX_STATE_RUN_DUPE_NAMES: + // NOTE: This does *not* attempt to fix the file system! $stack = explode(" ", $task->get("stack")); list ($parent_id, $name) = explode(":", array_pop($stack)); @@ -505,9 +509,16 @@ class gallery_task_Core { ->find_all(1, 1); if ($conflicts->count() && $conflict = $conflicts->current()) { $task->log("Fixing conflicting name for item id {$conflict->id}"); + if (!$conflict->is_album() && preg_match("/^(.*)(\.[^\.\/]*?)$/", $conflict->name, $matches)) { + $item_base_name = $matches[1]; + $item_extension = $matches[2]; // includes a leading dot + } else { + $item_base_name = $conflict->name; + $item_extension = ""; + } db::build() ->update("items") - ->set("name", $name . "-" . (string)rand(1000, 9999)) + ->set("name", $item_base_name . "-" . (string)rand(1000, 9999) . $item_extension) ->where("id", "=", $conflict->id) ->execute(); @@ -522,6 +533,74 @@ class gallery_task_Core { $completed++; if (empty($stack)) { + $state = self::FIX_STATE_START_DUPE_BASE_NAMES; + } + break; + + case self::FIX_STATE_START_DUPE_BASE_NAMES: + $stack = array(); + foreach (self::find_dupe_base_names() as $row) { + list ($parent_id, $base_name) = explode(":", $row->parent_base_name, 2); + $stack[] = join(":", array($parent_id, $base_name)); + } + if ($stack) { + $task->set("stack", implode(" ", $stack)); + $state = self::FIX_STATE_RUN_DUPE_BASE_NAMES; + } else { + $state = self::FIX_STATE_START_ALBUMS; + } + break; + + case self::FIX_STATE_RUN_DUPE_BASE_NAMES: + // NOTE: This *does* attempt to fix the file system! So, it must go *after* run_dupe_names. + $stack = explode(" ", $task->get("stack")); + list ($parent_id, $base_name) = explode(":", array_pop($stack)); + $base_name_escaped = Database::escape_for_like($base_name); + + $fixed = 0; + // We want to leave the first one alone and update all conflicts to be random values. + $conflicts = ORM::factory("item") + ->where("parent_id", "=", $parent_id) + ->where("name", "LIKE", "{$base_name_escaped}.%") + ->where("type", "<>", "album") + ->find_all(1, 1); + if ($conflicts->count() && $conflict = $conflicts->current()) { + $task->log("Fixing conflicting name for item id {$conflict->id}"); + if (preg_match("/^(.*)(\.[^\.\/]*?)$/", $conflict->name, $matches)) { + $item_base_name = $matches[1]; // unlike $base_name, this always maintains capitalization + $item_extension = $matches[2]; // includes a leading dot + } else { + $item_base_name = $conflict->name; + $item_extension = ""; + } + // Unlike conflicts found in run_dupe_names, these items are likely to have an intact + // file system. Let's use the item save logic to rebuild the paths and rename the files + // if possible. + try { + $conflict->name = $item_base_name . "-" . (string)rand(1000, 9999) . $item_extension; + $conflict->validate(); + // If we get here, we're safe to proceed with save + $conflict->save(); + } catch (Exception $e) { + // Didn't work. Edit database directly without fixing file system. + db::build() + ->update("items") + ->set("name", $item_base_name . "-" . (string)rand(1000, 9999) . $item_extension) + ->where("id", "=", $conflict->id) + ->execute(); + } + + // We fixed one conflict, but there might be more so put this parent back on the stack + // and try again. We won't consider it completed when we don't fix a conflict. This + // guarantees that we won't spend too long fixing one set of conflicts, and that we + // won't stop before all are fixed. + $stack[] = "$parent_id:$base_name"; + break; + } + $task->set("stack", implode(" ", $stack)); + $completed++; + + if (empty($stack)) { $state = self::FIX_STATE_START_ALBUMS; } break; @@ -669,18 +748,32 @@ class gallery_task_Core { } static function find_dupe_names() { + // looking for photos, movies, and albums return db::build() ->select_distinct( array("parent_name" => db::expr("CONCAT(`parent_id`, ':', LOWER(`name`))"))) ->select("id") ->select(array("C" => "COUNT(\"*\")")) ->from("items") - ->where("type", "<>", "album") ->having("C", ">", 1) ->group_by("parent_name") ->execute(); } + static function find_dupe_base_names() { + // looking for photos or movies, not albums + return db::build() + ->select_distinct( + array("parent_base_name" => db::expr("CONCAT(`parent_id`, ':', LOWER(SUBSTR(`name`, 1, LOCATE('.', `name`) - 1)))"))) + ->select("id") + ->select(array("C" => "COUNT(\"*\")")) + ->from("items") + ->where("type", "<>", "album") + ->having("C", ">", 1) + ->group_by("parent_base_name") + ->execute(); + } + static function find_empty_item_caches($limit) { return db::build() ->select("items.id") diff --git a/modules/gallery/helpers/item.php b/modules/gallery/helpers/item.php index b2c4cadf..386eeb08 100644 --- a/modules/gallery/helpers/item.php +++ b/modules/gallery/helpers/item.php @@ -39,55 +39,28 @@ class item_Core { } } - $source->parent_id = $target->id; - - // Moving may result in name or slug conflicts. If that happens, try up to 5 times to pick a - // random name (or slug) to avoid the conflict. $orig_name = $source->name; - $orig_name_filename = pathinfo($source->name, PATHINFO_FILENAME); - $orig_name_extension = pathinfo($source->name, PATHINFO_EXTENSION); - $orig_slug = $source->slug; - for ($i = 0; $i < 5; $i++) { - try { - $source->save(); - if ($orig_name != $source->name) { - switch ($source->type) { - case "album": - message::info( - t("Album <b>%old_name</b> renamed to <b>%new_name</b> to avoid a conflict", - array("old_name" => $orig_name, "new_name" => $source->name))); - break; - - case "photo": - message::info( - t("Photo <b>%old_name</b> renamed to <b>%new_name</b> to avoid a conflict", - array("old_name" => $orig_name, "new_name" => $source->name))); - break; + $source->parent_id = $target->id; + $source->save(); + if ($orig_name != $source->name) { + switch ($source->type) { + case "album": + message::info( + t("Album <b>%old_name</b> renamed to <b>%new_name</b> to avoid a conflict", + array("old_name" => $orig_name, "new_name" => $source->name))); + break; - case "movie": - message::info( - t("Movie <b>%old_name</b> renamed to <b>%new_name</b> to avoid a conflict", - array("old_name" => $orig_name, "new_name" => $source->name))); - break; - } - } + case "photo": + message::info( + t("Photo <b>%old_name</b> renamed to <b>%new_name</b> to avoid a conflict", + array("old_name" => $orig_name, "new_name" => $source->name))); break; - } catch (ORM_Validation_Exception $e) { - $rand = rand(10, 99); - $errors = $e->validation->errors(); - if (isset($errors["name"])) { - $source->name = $orig_name_filename . "-{$rand}." . $orig_name_extension; - unset($errors["name"]); - } - if (isset($errors["slug"])) { - $source->slug = $orig_slug . "-{$rand}"; - unset($errors["slug"]); - } - if ($errors) { - // There were other validation issues-- we don't know how to handle those - throw $e; - } + case "movie": + message::info( + t("Movie <b>%old_name</b> renamed to <b>%new_name</b> to avoid a conflict", + array("old_name" => $orig_name, "new_name" => $source->name))); + break; } } |