diff options
| -rw-r--r-- | modules/gallery/helpers/gallery_event.php | 5 | ||||
| -rw-r--r-- | modules/gallery/helpers/graphics.php | 78 | ||||
| -rw-r--r-- | modules/gallery/helpers/item.php | 15 | ||||
| -rw-r--r-- | modules/gallery/models/item.php | 15 | ||||
| -rw-r--r-- | modules/gallery/tests/Graphics_Helper_Test.php | 58 |
5 files changed, 117 insertions, 54 deletions
diff --git a/modules/gallery/helpers/gallery_event.php b/modules/gallery/helpers/gallery_event.php index bd9acc1c..ee3c51cc 100644 --- a/modules/gallery/helpers/gallery_event.php +++ b/modules/gallery/helpers/gallery_event.php @@ -141,10 +141,9 @@ class gallery_event_Core { foreach (ORM::factory("item") ->where("album_cover_item_id", "=", $item->id) ->find_all() as $target) { - copy($item->thumb_path(), $target->thumb_path()); - $target->thumb_width = $item->thumb_width; - $target->thumb_height = $item->thumb_height; + $target->thumb_dirty = 1; $target->save(); + graphics::generate($target); } } diff --git a/modules/gallery/helpers/graphics.php b/modules/gallery/helpers/graphics.php index 3f5e2d56..19ae1036 100644 --- a/modules/gallery/helpers/graphics.php +++ b/modules/gallery/helpers/graphics.php @@ -115,36 +115,12 @@ class graphics_Core { * @param Item_Model $item */ static function generate($item) { - if ($item->is_album()) { - if (!$cover = $item->album_cover()) { - // This album has no cover; there's nothing to generate. Because of an old bug, it's - // possible that there's an album cover item id that points to an invalid item. In that - // case, just null out the album cover item id. It's not optimal to do that at this low - // level, but it's not trivial to find these cases quickly in an upgrade script and if we - // don't do this, the album may be permanently marked as "needs rebuilding" - // - // ref: http://sourceforge.net/apps/trac/gallery/ticket/1172 - // http://galleryproject.org/node/96926 - if ($item->album_cover_item_id) { - $item->album_cover_item_id = null; - $item->save(); - } - return; - } - $input_file = $cover->file_path(); - $input_item = $cover; - } else { - $input_file = $item->file_path(); - $input_item = $item; - } - if ($item->thumb_dirty) { $ops["thumb"] = $item->thumb_path(); } - if ($item->resize_dirty && !$item->is_album() && !$item->is_movie()) { + if ($item->resize_dirty && $item->is_photo()) { $ops["resize"] = $item->resize_path(); } - if (empty($ops)) { $item->thumb_dirty = 0; $item->resize_dirty = 0; @@ -154,9 +130,11 @@ class graphics_Core { try { foreach ($ops as $target => $output_file) { + $working_file = $item->file_path(); // Delete anything that might already be there @unlink($output_file); - if ($input_item->is_movie()) { + switch ($item->type) { + case "movie": // Run movie_extract_frame events, which can either: // - generate an output file, bypassing the ffmpeg-based movie::extract_frame // - add to the options sent to movie::extract_frame (e.g. change frame extract time, @@ -164,27 +142,55 @@ class graphics_Core { // Note that the args are similar to those of the events in gallery_graphics $movie_options_wrapper = new stdClass(); $movie_options_wrapper->movie_options = array(); - module::event("movie_extract_frame", $input_file, $output_file, - $movie_options_wrapper, $input_item); + module::event("movie_extract_frame", $working_file, $output_file, + $movie_options_wrapper, $item); // If no output_file generated by events, run movie::extract_frame with movie_options clearstatcache(); if (@filesize($output_file) == 0) { try { - movie::extract_frame($input_file, $output_file, $movie_options_wrapper->movie_options); + movie::extract_frame($working_file, $output_file, $movie_options_wrapper->movie_options); } catch (Exception $e) { // Didn't work, likely because of MISSING_FFMPEG - use placeholder graphics::_replace_image_with_placeholder($item, $target); } } $working_file = $output_file; - } else { - $working_file = $input_file; - } - foreach (self::_get_rules($target) as $rule) { - $args = array($working_file, $output_file, unserialize($rule->args), $item); - call_user_func_array($rule->operation, $args); - $working_file = $output_file; + case "photo": + // Run the graphics rules (for both movies and photos) + foreach (self::_get_rules($target) as $rule) { + $args = array($working_file, $output_file, unserialize($rule->args), $item); + call_user_func_array($rule->operation, $args); + $working_file = $output_file; + } + break; + + case "album": + if (!$cover = $item->album_cover()) { + // This album has no cover; there's nothing to generate. Because of an old bug, it's + // possible that there's an album cover item id that points to an invalid item. In that + // case, just null out the album cover item id. It's not optimal to do that at this low + // level, but it's not trivial to find these cases quickly in an upgrade script and if we + // don't do this, the album may be permanently marked as "needs rebuilding" + // + // ref: http://sourceforge.net/apps/trac/gallery/ticket/1172 + // http://galleryproject.org/node/96926 + if ($item->album_cover_item_id) { + $item->album_cover_item_id = null; + $item->save(); + } + return; + } + if ($cover->thumb_dirty) { + graphics::generate($cover); + } + if (!$cover->thumb_dirty) { + // Make the album cover from the cover item's thumb. Run gallery_graphics::resize with + // null options and it will figure out if this is a direct copy or conversion to jpg. + $working_file = $cover->thumb_path(); + gallery_graphics::resize($working_file, $output_file, null, $item); + } + break; } } diff --git a/modules/gallery/helpers/item.php b/modules/gallery/helpers/item.php index 386eeb08..093feb2d 100644 --- a/modules/gallery/helpers/item.php +++ b/modules/gallery/helpers/item.php @@ -78,15 +78,9 @@ class item_Core { model_cache::clear(); $parent->album_cover_item_id = $item->is_album() ? $item->album_cover_item_id : $item->id; - if ($item->thumb_dirty) { - $parent->thumb_dirty = 1; - graphics::generate($parent); - } else { - copy($item->thumb_path(), $parent->thumb_path()); - $parent->thumb_width = $item->thumb_width; - $parent->thumb_height = $item->thumb_height; - } $parent->save(); + graphics::generate($parent); + $grand_parent = $parent->parent(); if ($grand_parent && access::can("edit", $grand_parent) && $grand_parent->album_cover_item_id == null) { @@ -97,15 +91,10 @@ class item_Core { static function remove_album_cover($album) { access::required("view", $album); access::required("edit", $album); - @unlink($album->thumb_path()); model_cache::clear(); $album->album_cover_item_id = null; - $album->thumb_width = 0; - $album->thumb_height = 0; - $album->thumb_dirty = 1; $album->save(); - graphics::generate($album); } /** diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php index 60318c26..f9edd3c6 100644 --- a/modules/gallery/models/item.php +++ b/modules/gallery/models/item.php @@ -420,6 +420,15 @@ class Item_Model_Core extends ORM_MPTT { } } + // If an album's cover has changed (or been removed), delete any existing album cover, + // reset the thumb metadata, and mark the thumb as dirty. + if (array_key_exists("album_cover_item_id", $this->changed) && $this->is_album()) { + @unlink($original->thumb_path()); + $this->thumb_dirty = 1; + $this->thumb_height = 0; + $this->thumb_width = 0; + } + if (array_intersect($this->changed, array("parent_id", "name", "slug"))) { $original->_build_relative_caches(); $this->relative_path_cache = null; @@ -966,10 +975,12 @@ class Item_Model_Core extends ORM_MPTT { return; } - if ($this->album_cover_item_id && db::build() + if ($this->album_cover_item_id && ($this->is_photo() || $this->is_movie() || + db::build() ->from("items") ->where("id", "=", $this->album_cover_item_id) - ->count_records() != 1) { + ->where("type", "<>", "album") + ->count_records() != 1)) { $v->add_error("album_cover_item_id", "invalid_item"); } } diff --git a/modules/gallery/tests/Graphics_Helper_Test.php b/modules/gallery/tests/Graphics_Helper_Test.php index ddcb9dfd..a68822b0 100644 --- a/modules/gallery/tests/Graphics_Helper_Test.php +++ b/modules/gallery/tests/Graphics_Helper_Test.php @@ -44,6 +44,39 @@ class Graphics_Helper_Test extends Gallery_Unit_Test_Case { $this->assert_equal(0, $movie->thumb_dirty); } + public function generate_album_cover_test() { + $album = test::random_album(); + $photo = test::random_unique_photo($album); + $album->reload(); + // Check that the image was copied directly from item thumb + $this->assert_equal(file_get_contents($photo->thumb_path()), + file_get_contents($album->thumb_path())); + // Check that the items table got updated + $this->assert_equal(array(200, 150), array($album->thumb_width, $album->thumb_height)); + // Check that the image is not marked dirty + $this->assert_equal(0, $album->thumb_dirty); + } + + public function generate_album_cover_from_png_test() { + $input_file = MODPATH . "gallery/tests/test.jpg"; + $output_file = TMPPATH . test::random_name() . ".png"; + gallery_graphics::resize($input_file, $output_file, null, null); + + $album = test::random_album(); + $photo = test::random_photo_unsaved($album); + $photo->set_data_file($output_file); + $photo->name = "album_cover_from_png.png"; + $photo->save(); + $album->reload(); + // Check that the image was correctly resized and converted to jpg + $this->assert_equal(array(200, 150, "image/jpeg", "jpg"), + photo::get_file_metadata($album->thumb_path())); + // Check that the items table got updated + $this->assert_equal(array(200, 150), array($album->thumb_width, $album->thumb_height)); + // Check that the image is not marked dirty + $this->assert_equal(0, $album->thumb_dirty); + } + public function generate_bad_photo_test() { $photo = test::random_photo(); // At this point, the photo is valid and has a valid resize and thumb. Make it garble. @@ -86,4 +119,29 @@ class Graphics_Helper_Test extends Gallery_Unit_Test_Case { // Check that the image is *not* marked as dirty $this->assert_equal(0, $movie->thumb_dirty); } + + public function generate_album_cover_from_bad_photo_test() { + $album = test::random_album(); + $photo = test::random_photo($album); + $album->reload(); + // At this point, the photo is valid and has a valid resize and thumb. Make it garble. + file_put_contents($photo->file_path(), test::lorem_ipsum(200)); + // Regenerate album from garbled photo. + $photo->thumb_dirty = 1; + $photo->save(); + $album->thumb_dirty = 1; + try { + graphics::generate($album); + $this->assert_true(false, "Shouldn't get here"); + } catch (Exception $e) { + // Exception expected + } + // Check that the image got replaced with a missing image placeholder + $this->assert_same(file_get_contents(MODPATH . "gallery/images/missing_photo.jpg"), + file_get_contents($album->thumb_path())); + // Check that the items table got updated with new metadata + $this->assert_equal(array(200, 200), array($album->thumb_width, $album->thumb_height)); + // Check that the images are marked as dirty + $this->assert_equal(1, $album->thumb_dirty); + } }
\ No newline at end of file |