From 2cf51983535ffa498c38b02328e30fe307ec6827 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Wed, 21 Jul 2010 11:34:19 -0700 Subject: Null out relative_path_cache and relative_url_cache after we update the pointers in case the hierarchy was adversely affected by actions when the MPTT pointers were desynced. Fixes ticket #1235. --- modules/gallery/helpers/gallery_task.php | 2 ++ 1 file changed, 2 insertions(+) (limited to 'modules/gallery/helpers/gallery_task.php') diff --git a/modules/gallery/helpers/gallery_task.php b/modules/gallery/helpers/gallery_task.php index bc128b3e..96ea7c0d 100644 --- a/modules/gallery/helpers/gallery_task.php +++ b/modules/gallery/helpers/gallery_task.php @@ -381,6 +381,8 @@ class gallery_task_Core { db::build() ->update("items") ->set("right_ptr", $value) + ->set("relative_path_cache", null) + ->set("relative_url_cache", null) ->where("id", "=", $id) ->execute(); } -- cgit v1.2.3 From 5be9ae3250fab24631c0fc6b900ffccd9b1755f2 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Sun, 25 Jul 2010 11:10:42 -0700 Subject: Add a new maintenance task that resyncs album .htaccess files with database access intents. Use this to fix up .htaccess files after you relocate your Gallery. Fixes ticket #1252. --- modules/gallery/helpers/access.php | 14 +++++--- modules/gallery/helpers/gallery_task.php | 59 +++++++++++++++++++++++++++++++- 2 files changed, 68 insertions(+), 5 deletions(-) (limited to 'modules/gallery/helpers/gallery_task.php') diff --git a/modules/gallery/helpers/access.php b/modules/gallery/helpers/access.php index d3f680d2..b1384c19 100644 --- a/modules/gallery/helpers/access.php +++ b/modules/gallery/helpers/access.php @@ -222,7 +222,7 @@ class access_Core { self::_update_access_non_view_cache($group, $perm_name, $album); } - self::_update_htaccess_files($album, $group, $perm_name, $value); + self::update_htaccess_files($album, $group, $perm_name, $value); model_cache::clear(); } @@ -623,10 +623,16 @@ class access_Core { } /** - * Maintain .htacccess files to prevent direct access to albums, resizes and thumbnails when we - * apply the view and view_full permissions to guest users. + * Rebuild the .htaccess files that prevent direct access to albums, resizes and thumbnails. We + * call this internally any time we change the view or view_full permissions for guest users. + * This function is only public because we use it in maintenance tasks. + * + * @param Item_Model the album + * @param Group_Model the group whose permission is changing + * @param string the permission name + * @param string the new permission value (eg access::DENY) */ - private static function _update_htaccess_files($album, $group, $perm_name, $value) { + public static function update_htaccess_files($album, $group, $perm_name, $value) { if ($group->id != identity::everybody()->id || !($perm_name == "view" || $perm_name == "view_full")) { return; diff --git a/modules/gallery/helpers/gallery_task.php b/modules/gallery/helpers/gallery_task.php index 96ea7c0d..4b5e9e93 100644 --- a/modules/gallery/helpers/gallery_task.php +++ b/modules/gallery/helpers/gallery_task.php @@ -50,7 +50,14 @@ class gallery_task_Core { ->callback("gallery_task::fix_mptt") ->name(t("Fix Album/Photo hierarchy")) ->description(t("Fix problems where your album/photo breadcrumbs are out of " . - "sync with your actual hierarchy.")) + "sync with your actual hierarchy")) + ->severity(log::SUCCESS); + + $tasks[] = Task_Definition::factory() + ->callback("gallery_task::fix_permissions") + ->name(t("Fix permissions")) + ->description(t("Resynchronize database permissions with the .htaccess " . + "files in your gallery3/var directory")) ->severity(log::SUCCESS); return $tasks; @@ -386,4 +393,54 @@ class gallery_task_Core { ->where("id", "=", $id) ->execute(); } + + static function fix_permissions($task) { + $start = microtime(true); + + $total = $task->get("total"); + if (empty($total)) { + $everybody_id = identity::everybody()->id; + $stack = array(); + foreach (db::build() + ->select("id") + ->from("access_intents") + ->where("view_{$everybody_id}", "=", 0) + ->or_where("view_full_{$everybody_id}", "=", 0) + ->execute() as $row) { + $stack[] = $row->id; + } + + $task->set("total", $total = count($stack)); + $task->set("stack", implode(" ", $stack)); + $task->set("completed", 0); + } + + $stack = explode(" ", $task->get("stack")); + $completed = $task->get("completed"); + + while ($stack && microtime(true) - $start < 1.5) { + $album = ORM::factory("item", array_pop($stack)); + $everybody = identity::everybody(); + if (!access::group_can($everybody, "view", $album)) { + access::update_htaccess_files($album, identity::everybody(), "view", access::DENY); + } else { + // It's one or the other, so if they have view then they don't have view_full + access::update_htaccess_files($album, identity::everybody(), "view_full", access::DENY); + } + $completed++; + } + + $task->set("stack", implode(" ", $stack)); + $task->set("completed", $completed); + + if ($total == $completed) { + $task->done = true; + $task->state = "success"; + $task->percent_complete = 100; + } else { + $task->percent_complete = round(100 * $completed / $total); + } + $task->status = t2("One album updated", "%count / %total albums updated", $completed, + array("total" => $total)); + } } \ No newline at end of file -- cgit v1.2.3 From b3c1b4633c55b14bfed607336ca213cb2fda4447 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Wed, 28 Jul 2010 22:42:04 -0700 Subject: Combine the Fix MPTT and Fix Permissions tasks into one magical fix-it task. --- modules/gallery/helpers/gallery_task.php | 166 ++++++++++++------------------- 1 file changed, 65 insertions(+), 101 deletions(-) (limited to 'modules/gallery/helpers/gallery_task.php') diff --git a/modules/gallery/helpers/gallery_task.php b/modules/gallery/helpers/gallery_task.php index 4b5e9e93..54acd8c8 100644 --- a/modules/gallery/helpers/gallery_task.php +++ b/modules/gallery/helpers/gallery_task.php @@ -18,8 +18,9 @@ * Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, MA 02110-1301, USA. */ class gallery_task_Core { - const MPTT_LEFT = 0; - const MPTT_RIGHT = 1; + const FIX_STATE_MPTT_LEFT = 0; + const FIX_STATE_MPTT_RIGHT = 1; + const FIX_STATE_ALBUM_PERMISSIONS = 2; static function available_tasks() { $dirty_count = graphics::find_dirty_images_query()->count_records(); @@ -47,17 +48,10 @@ class gallery_task_Core { ->severity(log::SUCCESS); $tasks[] = Task_Definition::factory() - ->callback("gallery_task::fix_mptt") - ->name(t("Fix Album/Photo hierarchy")) - ->description(t("Fix problems where your album/photo breadcrumbs are out of " . - "sync with your actual hierarchy")) - ->severity(log::SUCCESS); - - $tasks[] = Task_Definition::factory() - ->callback("gallery_task::fix_permissions") - ->name(t("Fix permissions")) - ->description(t("Resynchronize database permissions with the .htaccess " . - "files in your gallery3/var directory")) + ->callback("gallery_task::fix") + ->name(t("Fix your Gallery")) + ->description(t("Fix up a variety of little problems that might be causing " . + "your Gallery to act a little weird")) ->severity(log::SUCCESS); return $tasks; @@ -317,13 +311,13 @@ class gallery_task_Core { } } - static function fix_mptt($task) { + static function fix($task) { $start = microtime(true); $total = $task->get("total"); if (empty($total)) { $task->set("total", $total = db::build()->count_records("items")); - $task->set("stack", "1:" . self::MPTT_LEFT); + $task->set("stack", item::root()->id . ":" . self::FIX_STATE_ALBUM_PERMISSIONS); $task->set("ptr", 1); $task->set("completed", 0); } @@ -332,21 +326,68 @@ class gallery_task_Core { $stack = explode(" ", $task->get("stack")); $completed = $task->get("completed"); - // Implement a depth-first tree walk using a stack. Not the most efficient, but it's simple. + // This is a state machine that checks each item in the database. It verifies the following + // attributes for an item. + // 1. The .htaccess permission files for restricted items exist and are well formed. + // 2. Left and right MPTT pointers are correct + // 3. The relative_path_cache and relative_url_cache values are set to null. + // + // We'll do a depth-first tree walk over our hierarchy using only the adjacency data because + // we don't trust MPTT here (that might be what we're here to fix!). Avoid avoid using ORM + // calls as much as possible since they're expensive. while ($stack && microtime(true) - $start < 1.5) { list($id, $state) = explode(":", array_pop($stack)); switch ($state) { - case self::MPTT_LEFT: - self::fix_mptt_set_left($id, $ptr++); - $item = ORM::factory("item", $id); - array_push($stack, $id . ":" . self::MPTT_RIGHT); - foreach (self::fix_mptt_children($id) as $child) { - array_push($stack, $child->id . ":" . self::MPTT_LEFT); + case self::FIX_STATE_ALBUM_PERMISSIONS: + $everybody = identity::everybody(); + $view_col = "view_{$everybody->id}"; + $view_full_col = "view_full_{$everybody->id}"; + $intent = ORM::factory("access_intent")->where("item_id", "=", $id)->find(); + + // Only load the item if we're going to use it below + if ($intent->$view_col === access::DENY || + $intent->$view_full_col === access::DENY) { + $item = ORM::factory("item", $id); + } + if ($intent->$view_col === access::DENY) { + access::update_htaccess_files($item, $everybody, "view", access::DENY); + } + if ($intent->$view_full_col === access::DENY) { + access::update_htaccess_files($item, $everybody, "view_full", access::DENY); + } + array_push($stack, "$id:" . self::FIX_STATE_MPTT_LEFT); + break; + + case self::FIX_STATE_MPTT_LEFT: + db::build() + ->update("items") + ->set("left_ptr", $ptr++) + ->where("id", "=", $id) + ->execute(); + array_push($stack, "$id:" . self::FIX_STATE_MPTT_RIGHT); + + foreach (db::build() + ->select(array("id", "type")) + ->from("items") + ->where("parent_id", "=", $id) + ->order_by("left_ptr", "ASC") + ->execute() as $child) { + if ($child->type == "album") { + array_push($stack, "{$child->id}:" . self::FIX_STATE_ALBUM_PERMISSIONS); + } else { + array_push($stack, "{$child->id}:" . self::FIX_STATE_MPTT_LEFT); + } } break; - case self::MPTT_RIGHT: - self::fix_mptt_set_right($id, $ptr++); + case self::FIX_STATE_MPTT_RIGHT: + db::build() + ->update("items") + ->set("right_ptr", $ptr++) + ->set("relative_path_cache", null) + ->set("relative_url_cache", null) + ->where("id", "=", $id) + ->execute(); $completed++; break; } @@ -366,81 +407,4 @@ class gallery_task_Core { $task->status = t2("One row updated", "%count / %total rows updated", $completed, array("total" => $total)); } - - static function fix_mptt_children($parent_id) { - return db::build() - ->select("id") - ->from("items") - ->where("parent_id", "=", $parent_id) - ->order_by("left_ptr", "ASC") - ->execute(); - } - - static function fix_mptt_set_left($id, $value) { - db::build() - ->update("items") - ->set("left_ptr", $value) - ->where("id", "=", $id) - ->execute(); - } - - static function fix_mptt_set_right($id, $value) { - db::build() - ->update("items") - ->set("right_ptr", $value) - ->set("relative_path_cache", null) - ->set("relative_url_cache", null) - ->where("id", "=", $id) - ->execute(); - } - - static function fix_permissions($task) { - $start = microtime(true); - - $total = $task->get("total"); - if (empty($total)) { - $everybody_id = identity::everybody()->id; - $stack = array(); - foreach (db::build() - ->select("id") - ->from("access_intents") - ->where("view_{$everybody_id}", "=", 0) - ->or_where("view_full_{$everybody_id}", "=", 0) - ->execute() as $row) { - $stack[] = $row->id; - } - - $task->set("total", $total = count($stack)); - $task->set("stack", implode(" ", $stack)); - $task->set("completed", 0); - } - - $stack = explode(" ", $task->get("stack")); - $completed = $task->get("completed"); - - while ($stack && microtime(true) - $start < 1.5) { - $album = ORM::factory("item", array_pop($stack)); - $everybody = identity::everybody(); - if (!access::group_can($everybody, "view", $album)) { - access::update_htaccess_files($album, identity::everybody(), "view", access::DENY); - } else { - // It's one or the other, so if they have view then they don't have view_full - access::update_htaccess_files($album, identity::everybody(), "view_full", access::DENY); - } - $completed++; - } - - $task->set("stack", implode(" ", $stack)); - $task->set("completed", $completed); - - if ($total == $completed) { - $task->done = true; - $task->state = "success"; - $task->percent_complete = 100; - } else { - $task->percent_complete = round(100 * $completed / $total); - } - $task->status = t2("One album updated", "%count / %total albums updated", $completed, - array("total" => $total)); - } } \ No newline at end of file -- cgit v1.2.3 From 7b8ed1079624eefae8d83fac89caa0eaa3fc9ef4 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Thu, 29 Jul 2010 21:47:52 -0700 Subject: Add recovery code for dupe slugs and dupe names to the general purpose Fix task. Fixes ticket #1260. --- modules/gallery/helpers/gallery_task.php | 263 +++++++++++++++++++++++++------ 1 file changed, 215 insertions(+), 48 deletions(-) (limited to 'modules/gallery/helpers/gallery_task.php') diff --git a/modules/gallery/helpers/gallery_task.php b/modules/gallery/helpers/gallery_task.php index 54acd8c8..da9fba49 100644 --- a/modules/gallery/helpers/gallery_task.php +++ b/modules/gallery/helpers/gallery_task.php @@ -18,9 +18,15 @@ * Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, MA 02110-1301, USA. */ class gallery_task_Core { - const FIX_STATE_MPTT_LEFT = 0; - const FIX_STATE_MPTT_RIGHT = 1; - const FIX_STATE_ALBUM_PERMISSIONS = 2; + const FIX_STATE_START_MPTT = 0; + const FIX_STATE_RUN_MPTT = 1; + const FIX_STATE_START_PERMISSIONS = 2; + const FIX_STATE_RUN_PERMISSIONS = 3; + const FIX_STATE_START_DUPE_SLUGS = 4; + const FIX_STATE_RUN_DUPE_SLUGS = 5; + const FIX_STATE_START_DUPE_NAMES = 6; + const FIX_STATE_RUN_DUPE_NAMES = 7; + const FIX_STATE_DONE = 8; static function available_tasks() { $dirty_count = graphics::find_dirty_images_query()->count_records(); @@ -316,29 +322,194 @@ class gallery_task_Core { $total = $task->get("total"); if (empty($total)) { - $task->set("total", $total = db::build()->count_records("items")); - $task->set("stack", item::root()->id . ":" . self::FIX_STATE_ALBUM_PERMISSIONS); + // mptt: 2 operations for every item + // permissions: 1 operation for every album + // dupe slugs: 1 operation for each unique conflicted slug + $total = 2 * db::build()->count_records("items"); + $total += db::build()->where("type", "=", "album")->count_records("items"); + foreach (self::find_dupe_slugs() as $row) { + $total++; + } + foreach (self::find_dupe_names() as $row) { + $total++; + } + + $task->set("total", $total); + $task->set("state", $state = self::FIX_STATE_START_MPTT); $task->set("ptr", 1); $task->set("completed", 0); } - $ptr = $task->get("ptr"); - $stack = explode(" ", $task->get("stack")); $completed = $task->get("completed"); + $state = $task->get("state"); // This is a state machine that checks each item in the database. It verifies the following // attributes for an item. - // 1. The .htaccess permission files for restricted items exist and are well formed. - // 2. Left and right MPTT pointers are correct + // 1. Left and right MPTT pointers are correct + // 2. The .htaccess permission files for restricted items exist and are well formed. // 3. The relative_path_cache and relative_url_cache values are set to null. // // We'll do a depth-first tree walk over our hierarchy using only the adjacency data because // we don't trust MPTT here (that might be what we're here to fix!). Avoid avoid using ORM // calls as much as possible since they're expensive. - while ($stack && microtime(true) - $start < 1.5) { - list($id, $state) = explode(":", array_pop($stack)); + while ($state != self::FIX_STATE_DONE && microtime(true) - $start < 1.5) { switch ($state) { - case self::FIX_STATE_ALBUM_PERMISSIONS: + case self::FIX_STATE_START_MPTT: + $task->set("ptr", $ptr = 1); + $task->set("stack", item::root()->id . ":L"); + $state = self::FIX_STATE_RUN_MPTT; + break; + + case self::FIX_STATE_RUN_MPTT: + $ptr = $task->get("ptr"); + $stack = explode(" ", $task->get("stack")); + list ($id, $ptr_mode) = explode(":", array_pop($stack)); + if ($ptr_mode == "L") { + $stack[] = "$id:R"; + db::build() + ->update("items") + ->set("left_ptr", $ptr++) + ->where("id", "=", $id) + ->execute(); + + foreach (db::build() + ->select(array("id")) + ->from("items") + ->where("parent_id", "=", $id) + ->order_by("left_ptr", "ASC") + ->execute() as $child) { + array_push($stack, "{$child->id}:L"); + } + } else if ($ptr_mode == "R") { + db::build() + ->update("items") + ->set("right_ptr", $ptr++) + ->set("relative_path_cache", null) + ->set("relative_url_cache", null) + ->where("id", "=", $id) + ->execute(); + } + $task->set("ptr", $ptr); + $task->set("stack", implode(" ", $stack)); + $completed++; + + if (empty($stack)) { + $state = self::FIX_STATE_START_DUPE_SLUGS; + } + break; + + + case self::FIX_STATE_START_DUPE_SLUGS: + $stack = array(); + foreach (self::find_dupe_slugs() as $row) { + list ($parent_id, $slug) = explode(":", $row->parent_slug, 2); + $stack[] = join(":", array($parent_id, $slug)); + } + if ($stack) { + $task->set("stack", implode(" ", $stack)); + $state = self::FIX_STATE_RUN_DUPE_SLUGS; + } else { + $state = self::FIX_STATE_START_DUPE_NAMES; + } + break; + + case self::FIX_STATE_RUN_DUPE_SLUGS: + $stack = explode(" ", $task->get("stack")); + list ($parent_id, $slug) = explode(":", array_pop($stack)); + + // We want to leave the first one alone and update all conflicts to be random values. + $fixed = 0; + $conflicts = ORM::factory("item") + ->where("parent_id", "=", $parent_id) + ->where("slug", "=", $slug) + ->find_all(1, 1); + if ($conflicts->count() && $conflict = $conflicts->current()) { + $task->log("Fixing conflicting slug for item id {$conflict->id}"); + db::build() + ->update("items") + ->set("slug", $slug . "-" . (string)rand(1000, 9999)) + ->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:$slug"; + break; + } + $task->set("stack", implode(" ", $stack)); + $completed++; + + if (empty($stack)) { + $state = self::FIX_STATE_START_DUPE_NAMES; + } + break; + + case self::FIX_STATE_START_DUPE_NAMES: + $stack = array(); + foreach (self::find_dupe_names() as $row) { + list ($parent_id, $name) = explode(":", $row->parent_name, 2); + $stack[] = join(":", array($parent_id, $name)); + } + if ($stack) { + $task->set("stack", implode(" ", $stack)); + $state = self::FIX_STATE_RUN_DUPE_NAMES; + } else { + $state = self::FIX_STATE_START_PERMISSIONS; + } + break; + + case self::FIX_STATE_RUN_DUPE_NAMES: + $stack = explode(" ", $task->get("stack")); + list ($parent_id, $name) = explode(":", array_pop($stack)); + + $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", "=", $name) + ->find_all(1, 1); + if ($conflicts->count() && $conflict = $conflicts->current()) { + $task->log("Fixing conflicting name for item id {$conflict->id}"); + db::build() + ->update("items") + ->set("name", $name . "-" . (string)rand(1000, 9999)) + ->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:$name"; + break; + } + $task->set("stack", implode(" ", $stack)); + $completed++; + + if (empty($stack)) { + $state = self::FIX_STATE_START_PERMISSIONS; + } + break; + + case self::FIX_STATE_START_PERMISSIONS: + $stack = array(); + foreach (db::build() + ->select("id") + ->from("items") + ->where("type", "=", "album") + ->execute() as $row) { + $stack[] = $row->id; + } + $task->set("stack", implode(" ", $stack)); + $state = self::FIX_STATE_RUN_PERMISSIONS; + break; + + case self::FIX_STATE_RUN_PERMISSIONS: + $stack = explode(" ", $task->get("stack")); + $id = array_pop($stack); + $everybody = identity::everybody(); $view_col = "view_{$everybody->id}"; $view_full_col = "view_full_{$everybody->id}"; @@ -355,56 +526,52 @@ class gallery_task_Core { if ($intent->$view_full_col === access::DENY) { access::update_htaccess_files($item, $everybody, "view_full", access::DENY); } - array_push($stack, "$id:" . self::FIX_STATE_MPTT_LEFT); - break; - - case self::FIX_STATE_MPTT_LEFT: - db::build() - ->update("items") - ->set("left_ptr", $ptr++) - ->where("id", "=", $id) - ->execute(); - array_push($stack, "$id:" . self::FIX_STATE_MPTT_RIGHT); + $task->set("stack", implode(" ", $stack)); + $completed++; - foreach (db::build() - ->select(array("id", "type")) - ->from("items") - ->where("parent_id", "=", $id) - ->order_by("left_ptr", "ASC") - ->execute() as $child) { - if ($child->type == "album") { - array_push($stack, "{$child->id}:" . self::FIX_STATE_ALBUM_PERMISSIONS); - } else { - array_push($stack, "{$child->id}:" . self::FIX_STATE_MPTT_LEFT); - } + if (empty($stack)) { + $state = self::FIX_STATE_DONE; } break; - - case self::FIX_STATE_MPTT_RIGHT: - db::build() - ->update("items") - ->set("right_ptr", $ptr++) - ->set("relative_path_cache", null) - ->set("relative_url_cache", null) - ->where("id", "=", $id) - ->execute(); - $completed++; - break; } } - $task->set("stack", implode(" ", $stack)); - $task->set("ptr", $ptr); + $task->set("state", $state); $task->set("completed", $completed); - if ($total == $completed) { + if ($state == self::FIX_STATE_DONE) { $task->done = true; $task->state = "success"; $task->percent_complete = 100; } else { $task->percent_complete = round(100 * $completed / $total); } - $task->status = t2("One row updated", "%count / %total rows updated", $completed, + $task->status = t2("One operation complete", "%count / %total operations complete", $completed, array("total" => $total)); } + + static function find_dupe_slugs() { + return db::build() + ->select_distinct( + array("parent_slug" => new Database_Expression("CONCAT(`parent_id`, ':', `slug`)"))) + ->select("id") + ->select(array("C" => "COUNT(\"*\")")) + ->from("items") + ->having("C", ">", 1) + ->group_by("parent_slug") + ->execute(); + } + + static function find_dupe_names() { + return db::build() + ->select_distinct( + array("parent_name" => new Database_Expression("CONCAT(`parent_id`, ':', `name`)"))) + ->select("id") + ->select(array("C" => "COUNT(\"*\")")) + ->from("items") + ->where("type", "<>", "album") + ->having("C", ">", 1) + ->group_by("parent_name") + ->execute(); + } } \ No newline at end of file -- cgit v1.2.3