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 From c33b24c9faf5d83e4f1bfc6d3778da6c37139b3f Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Sun, 1 Aug 2010 21:00:30 -0700 Subject: Make maintenance mode a variable instead of a config. Then create links on the Admin > Maintenance page to allow you to turn it on and off. This should be efficient since we cache all vars and look them up on every request anyway. This also allows us to have the Fix task enable maintenance mode while it's running which greatly reduces the chances that somebody will come along and hork the database while we're tinkering with MPTT pointers. Fixes ticket #1259. --- application/config/config.php | 7 ------ modules/gallery/controllers/admin_maintenance.php | 6 ++++++ modules/gallery/helpers/gallery.php | 6 +++--- modules/gallery/helpers/gallery_installer.php | 10 +++++++-- modules/gallery/helpers/gallery_task.php | 9 ++++++-- modules/gallery/libraries/Theme_View.php | 8 ++++--- modules/gallery/module.info | 2 +- modules/gallery/views/admin_maintenance.html.php | 26 ++++++++++++++++++----- 8 files changed, 51 insertions(+), 23 deletions(-) (limited to 'modules/gallery/helpers/gallery_task.php') diff --git a/application/config/config.php b/application/config/config.php index 9a8f4299..3b92ac73 100644 --- a/application/config/config.php +++ b/application/config/config.php @@ -151,10 +151,3 @@ if (TEST_MODE) { array(MODPATH . "gallery_unit_test", MODPATH . "unit_test")); } - -/** - * Setting the maintenance_mode to block all non administrative access. In - * this mode a user can attempt to logon, but will be unable to access anything. - * The application will behave normally if an adminstrator logs on. - */ -//$config["maintenance_mode"] = true; diff --git a/modules/gallery/controllers/admin_maintenance.php b/modules/gallery/controllers/admin_maintenance.php index 3567b4f0..a9cc933c 100644 --- a/modules/gallery/controllers/admin_maintenance.php +++ b/modules/gallery/controllers/admin_maintenance.php @@ -226,4 +226,10 @@ class Admin_Maintenance_Controller extends Admin_Controller { "done" => (bool) $task->done))); } } + + public function maintenance_mode($value) { + access::verify_csrf(); + module::set_var("gallery", "maintenance_mode", $value); + url::redirect("admin/maintenance"); + } } diff --git a/modules/gallery/helpers/gallery.php b/modules/gallery/helpers/gallery.php index d4078209..33a6830c 100644 --- a/modules/gallery/helpers/gallery.php +++ b/modules/gallery/helpers/gallery.php @@ -25,9 +25,9 @@ class gallery_Core { * down for maintenance" page. */ static function maintenance_mode() { - $maintenance_mode = Kohana::config("core.maintenance_mode", false, false); - - if (Router::$controller != "login" && !empty($maintenance_mode) && !identity::active_user()->admin) { + if (Router::$controller != "login" && + module::get_var("gallery", "maintenance_mode", false) && + !identity::active_user()->admin) { Router::$controller = "maintenance"; Router::$controller_path = MODPATH . "gallery/controllers/maintenance.php"; Router::$method = "index"; diff --git a/modules/gallery/helpers/gallery_installer.php b/modules/gallery/helpers/gallery_installer.php index 39c35711..f5589618 100644 --- a/modules/gallery/helpers/gallery_installer.php +++ b/modules/gallery/helpers/gallery_installer.php @@ -295,7 +295,8 @@ class gallery_installer { module::set_var("gallery", "credits", (string) $powered_by_string); module::set_var("gallery", "simultaneous_upload_limit", 5); module::set_var("gallery", "admin_area_timeout", 90 * 60); - module::set_version("gallery", 30); + module::set_var("gallery", "maintenance_mode", 0); + module::set_version("gallery", 31); } static function upgrade($version) { @@ -554,7 +555,12 @@ class gallery_installer { if ($version == 29) { $db->query("ALTER TABLE {caches} ADD KEY (`key`);"); module::set_version("gallery", $version = 30); - } + } + + if ($version == 30) { + module::set_var("gallery", "maintenance_mode", 0); + module::set_version("gallery", $version = 31); + } } static function uninstall() { diff --git a/modules/gallery/helpers/gallery_task.php b/modules/gallery/helpers/gallery_task.php index da9fba49..f5c25ddc 100644 --- a/modules/gallery/helpers/gallery_task.php +++ b/modules/gallery/helpers/gallery_task.php @@ -56,8 +56,8 @@ class gallery_task_Core { $tasks[] = Task_Definition::factory() ->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")) + ->description(t("Fix a variety of problems that might cause your Gallery to act " . + "strangely. Requires maintenance mode.")) ->severity(log::SUCCESS); return $tasks; @@ -343,6 +343,10 @@ class gallery_task_Core { $completed = $task->get("completed"); $state = $task->get("state"); + if (!module::get_var("gallery", "maintenance_mode")) { + module::set_var("gallery", "maintenance_mode", 1); + } + // This is a state machine that checks each item in the database. It verifies the following // attributes for an item. // 1. Left and right MPTT pointers are correct @@ -543,6 +547,7 @@ class gallery_task_Core { $task->done = true; $task->state = "success"; $task->percent_complete = 100; + module::set_var("gallery", "maintenance_mode", 0); } else { $task->percent_complete = round(100 * $completed / $total); } diff --git a/modules/gallery/libraries/Theme_View.php b/modules/gallery/libraries/Theme_View.php index 6246c6f1..7b90c034 100644 --- a/modules/gallery/libraries/Theme_View.php +++ b/modules/gallery/libraries/Theme_View.php @@ -46,9 +46,11 @@ class Theme_View_Core extends Gallery_View { $this->set_global("thumb_proportion", $this->thumb_proportion()); } - $maintenance_mode = Kohana::config("core.maintenance_mode", false, false); - if ($maintenance_mode) { - message::warning(t("This site is currently in maintenance mode")); + if (module::get_var("gallery", "maintenance_mode", false)) { + if (identity::active_user()->admin) { + message::warning(t("This site is currently in maintenance mode. Visit the maintenance page", array("maintenance_url" => url::site("admin/maintenance")))); + } else + message::warning(t("This site is currently in maintenance mode.")); } } diff --git a/modules/gallery/module.info b/modules/gallery/module.info index df2be978..7d28a7c1 100644 --- a/modules/gallery/module.info +++ b/modules/gallery/module.info @@ -1,3 +1,3 @@ name = "Gallery 3" description = "Gallery core application" -version = 30 +version = 31 diff --git a/modules/gallery/views/admin_maintenance.html.php b/modules/gallery/views/admin_maintenance.html.php index ad0e2f55..4bfc57f0 100644 --- a/modules/gallery/views/admin_maintenance.html.php +++ b/modules/gallery/views/admin_maintenance.html.php @@ -1,13 +1,29 @@
-

-

- -

+

+
+
+ maintenance mode which prevents any non-admin from accessing your Gallery. Some of the tasks below will automatically put your Gallery in maintenance mode for you.") ?> +
    + +
  • + on. Non admins cannot access your Gallery. Turn off maintenance mode", array("enable_maintenance_mode_url" => url::site("admin/maintenance/maintenance_mode/0?csrf=$csrf"))) ?> +
    • + +
  • + Turn on maintenance mode", array("enable_maintenance_mode_url" => url::site("admin/maintenance/maintenance_mode/1?csrf=$csrf"))) ?> +
    • + +
+
+
-

+

+

+ +

-- cgit v1.2.3 From ca54cdd6448ec919efe3853992dc652b7c671d97 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Sun, 1 Aug 2010 21:44:24 -0700 Subject: While we're cleaning up albums, also find any cases where we have an album_cover_item_id that points to an invalid item. --- modules/gallery/helpers/gallery_task.php | 32 ++++++++++++++++++-------------- 1 file changed, 18 insertions(+), 14 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 f5c25ddc..abfa9e8f 100644 --- a/modules/gallery/helpers/gallery_task.php +++ b/modules/gallery/helpers/gallery_task.php @@ -20,8 +20,8 @@ class gallery_task_Core { 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_ALBUMS = 2; + const FIX_STATE_RUN_ALBUMS = 3; const FIX_STATE_START_DUPE_SLUGS = 4; const FIX_STATE_RUN_DUPE_SLUGS = 5; const FIX_STATE_START_DUPE_NAMES = 6; @@ -323,7 +323,7 @@ class gallery_task_Core { $total = $task->get("total"); if (empty($total)) { // mptt: 2 operations for every item - // permissions: 1 operation for every album + // album audit (permissions and bogus album covers): 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"); @@ -352,6 +352,7 @@ class gallery_task_Core { // 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. + // 4. there are no album_cover_item_ids pointing to missing items // // 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 @@ -460,7 +461,7 @@ class gallery_task_Core { $task->set("stack", implode(" ", $stack)); $state = self::FIX_STATE_RUN_DUPE_NAMES; } else { - $state = self::FIX_STATE_START_PERMISSIONS; + $state = self::FIX_STATE_START_ALBUMS; } break; @@ -493,11 +494,11 @@ class gallery_task_Core { $completed++; if (empty($stack)) { - $state = self::FIX_STATE_START_PERMISSIONS; + $state = self::FIX_STATE_START_ALBUMS; } break; - case self::FIX_STATE_START_PERMISSIONS: + case self::FIX_STATE_START_ALBUMS: $stack = array(); foreach (db::build() ->select("id") @@ -507,23 +508,26 @@ class gallery_task_Core { $stack[] = $row->id; } $task->set("stack", implode(" ", $stack)); - $state = self::FIX_STATE_RUN_PERMISSIONS; + $state = self::FIX_STATE_RUN_ALBUMS; break; - case self::FIX_STATE_RUN_PERMISSIONS: + case self::FIX_STATE_RUN_ALBUMS: $stack = explode(" ", $task->get("stack")); $id = array_pop($stack); + $item = ORM::factory("item", $id); + if ($item->album_cover_item_id) { + $album_cover_item = ORM::factory("item", $item->album_cover_item_id); + if (!$album_cover_item->loaded()) { + $item->album_cover_item_id = null; + $item->save(); + } + } + $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); } -- cgit v1.2.3 From 8559cdb5b6bfa87864941f726521660023779fa7 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Wed, 4 Aug 2010 21:30:48 -0700 Subject: Add docs reflecting that we may skip some items that have invalid parent_ids --- modules/gallery/helpers/gallery_task.php | 4 ++++ 1 file changed, 4 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 abfa9e8f..bf1355b8 100644 --- a/modules/gallery/helpers/gallery_task.php +++ b/modules/gallery/helpers/gallery_task.php @@ -357,6 +357,10 @@ class gallery_task_Core { // 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. + // + // NOTE: the MPTT check will only traverse items that have valid parents. It's possible that + // we have some tree corruption where there are items with parent ids to non-existent items. + // We should probably do something about that. while ($state != self::FIX_STATE_DONE && microtime(true) - $start < 1.5) { switch ($state) { case self::FIX_STATE_START_MPTT: -- cgit v1.2.3