From b20f9123dc0cc4bc70597910e12eb18ec0eef2c2 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Sat, 10 Jul 2010 18:25:23 -0700 Subject: [PATCH] Wrap album deletes in a batch so that we can handle lots of cascading deletes in bulk. This lets us avoid the problem where we continually choose and delete album covers which makes deletes really slow. It probably also avoids huge amounts of notification emails (untested). Fixes ticket #1190. --- modules/gallery/controllers/quick.php | 11 +++++++- modules/gallery/helpers/gallery_event.php | 33 ++++++++++++++++++----- 2 files changed, 37 insertions(+), 7 deletions(-) diff --git a/modules/gallery/controllers/quick.php b/modules/gallery/controllers/quick.php index 52f15e7d0f..08a33138dd 100644 --- a/modules/gallery/controllers/quick.php +++ b/modules/gallery/controllers/quick.php @@ -116,7 +116,16 @@ public function delete($id) { } $parent = $item->parent(); - $item->delete(); + + if ($item->is_album()) { + // Album delete will trigger deletes for all children. Do this in a batch so that we can be + // smart about notifications, album cover updates, etc. + batch::start(); + $item->delete(); + batch::stop(); + } else { + $item->delete(); + } message::success($msg); $from_id = Input::instance()->get("from_id"); diff --git a/modules/gallery/helpers/gallery_event.php b/modules/gallery/helpers/gallery_event.php index ba6dd99d1f..60520690e6 100644 --- a/modules/gallery/helpers/gallery_event.php +++ b/modules/gallery/helpers/gallery_event.php @@ -109,14 +109,35 @@ static function item_deleted($item) { $parent = $item->parent(); if (!$parent->album_cover_item_id) { - // Assume we deleted the album cover and pick a new one. Choosing the first photo in the - // album is logical, but it's not the most efficient in the case where we're deleting all - // the photos in the album one at a time since we'll probably delete them in order which - // means that we'll be resetting the album cover each time. - if ($child = $parent->children(1)->current()) { - item::make_album_cover($child); + // Assume that we deleted the album cover + if (batch::in_progress()) { + // Remember that this parent is missing an album cover, for later. + $batch_missing_album_cover = Session::instance()->get("batch_missing_album_cover", array()); + $batch_missing_album_cover[$parent->id] = 1; + Session::instance()->set("batch_missing_album_cover", $batch_missing_album_cover); + } else { + // Choose the first child as the new cover. + if ($child = $parent->children(1)->current()) { + item::make_album_cover($child); + } + } + } + } + + static function batch_complete() { + // Set the album covers for any items that where we probably deleted the album cover during + // this batch. The item may have been deleted, so don't count on it being around. Choose the + // first child as the new album cover. + // NOTE: if the first child doesn't have an album cover, then this won't work. + foreach (array_keys(Session::instance()->get("batch_missing_album_cover", array())) as $id) { + $item = ORM::factory("item", $id); + if ($item->loaded() && !$item->album_cover_item_id) { + if ($child = $item->children(1)->current()) { + item::make_album_cover($child); + } } } + Session::instance()->delete("batch_missing_album_cover"); } static function item_moved($item, $old_parent) {