diff --git a/modules/gallery/helpers/graphics.php b/modules/gallery/helpers/graphics.php index 0c5f8366d3..3f5e2d5671 100644 --- a/modules/gallery/helpers/graphics.php +++ b/modules/gallery/helpers/graphics.php @@ -154,10 +154,9 @@ static function generate($item) { try { foreach ($ops as $target => $output_file) { + // Delete anything that might already be there + @unlink($output_file); if ($input_item->is_movie()) { - // Convert the movie filename to a JPG first, delete anything that might already be there - $output_file = legal_file::change_extension($output_file, "jpg"); - @unlink($output_file); // 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, @@ -173,8 +172,8 @@ static function generate($item) { try { movie::extract_frame($input_file, $output_file, $movie_options_wrapper->movie_options); } catch (Exception $e) { - // Didn't work, likely because of MISSING_FFMPEG - copy missing_movie instead - copy(MODPATH . "gallery/images/missing_movie.jpg", $output_file); + // Didn't work, likely because of MISSING_FFMPEG - use placeholder + graphics::_replace_image_with_placeholder($item, $target); } } $working_file = $output_file; @@ -193,31 +192,82 @@ static function generate($item) { if (file_exists($item->thumb_path())) { $item->thumb_dirty = 0; } else { - copy(MODPATH . "gallery/images/missing_photo.png", $item->thumb_path()); + Kohana_Log::add("error", "Failed to rebuild thumb image: $item->title"); + graphics::_replace_image_with_placeholder($item, "thumb"); } - list ($item->thumb_width, $item->thumb_height) = - photo::get_file_metadata($item->thumb_path()); } if (!empty($ops["resize"])) { if (file_exists($item->resize_path())) { $item->resize_dirty = 0; } else { - copy(MODPATH . "gallery/images/missing_photo.png", $item->resize_path()); + Kohana_Log::add("error", "Failed to rebuild resize image: $item->title"); + graphics::_replace_image_with_placeholder($item, "resize"); } - list ($item->resize_width, $item->resize_height) = - photo::get_file_metadata($item->resize_path()); } + graphics::_update_item_dimensions($item); $item->save(); } catch (Exception $e) { - // Something went wrong rebuilding the image. Leave it dirty and move on. - // @todo we should handle this better. - Kohana_Log::add("error", "Caught exception rebuilding image: {$item->title}\n" . + // Something went wrong rebuilding the image. Replace with the placeholder images, + // leave it dirty and move on. + Kohana_Log::add("error", "Caught exception rebuilding images: {$item->title}\n" . $e->getMessage() . "\n" . $e->getTraceAsString()); + if ($item->is_photo()) { + graphics::_replace_image_with_placeholder($item, "resize"); + } + graphics::_replace_image_with_placeholder($item, "thumb"); + graphics::_update_item_dimensions($item); + $item->save(); throw $e; } } + private static function _update_item_dimensions($item) { + if ($item->is_photo()) { + list ($item->resize_width, $item->resize_height) = + photo::get_file_metadata($item->resize_path()); + } + list ($item->thumb_width, $item->thumb_height) = + photo::get_file_metadata($item->thumb_path()); + } + + private static function _replace_image_with_placeholder($item, $target) { + if ($item->is_movie() || ($item->is_album() && $item->album_cover()->is_movie())) { + $input_path = MODPATH . "gallery/images/missing_movie.jpg"; + } else { + $input_path = MODPATH . "gallery/images/missing_photo.jpg"; + } + + if ($target == "thumb") { + $output_path = $item->thumb_path(); + $size = module::get_var("gallery", "thumb_size", 200); + } else { + $output_path = $item->resize_path(); + $size = module::get_var("gallery", "resize_size", 640); + } + $options = array("width" => $size, "height" => $size, "master" => Image::AUTO); + + try { + // Copy/convert/resize placeholder as needed. + gallery_graphics::resize($input_path, $output_path, $options, null); + } catch (Exception $e) { + // Copy/convert/resize didn't work. Add to the log and copy the jpg version (which could have + // a non-jpg extension). This is less than ideal, but it's better than putting nothing + // there and causing theme views to act strangely because a file is missing. + // @todo we should handle this better. + Kohana_Log::add("error", "Caught exception converting placeholder for missing image: " . + $item->title . "\n" . $e->getMessage() . "\n" . $e->getTraceAsString()); + copy($input_path, $output_path); + } + + if (!file_exists($output_path)) { + // Copy/convert/resize didn't throw an exception, but still didn't work - do the same as above. + // @todo we should handle this better. + Kohana_Log::add("error", "Failed to convert placeholder for missing image: $item->title"); + copy($input_path, $output_path); + } + } + private static function _get_rules($target) { if (empty(self::$_rules_cache[$target])) { $rules = array(); diff --git a/modules/gallery/images/missing_photo.jpg b/modules/gallery/images/missing_photo.jpg new file mode 100644 index 0000000000..a9d176d8a3 Binary files /dev/null and b/modules/gallery/images/missing_photo.jpg differ diff --git a/modules/gallery/images/missing_photo.png b/modules/gallery/images/missing_photo.png deleted file mode 100644 index 677862751f..0000000000 Binary files a/modules/gallery/images/missing_photo.png and /dev/null differ diff --git a/modules/gallery/tests/Graphics_Helper_Test.php b/modules/gallery/tests/Graphics_Helper_Test.php new file mode 100644 index 0000000000..ddcb9dfd5e --- /dev/null +++ b/modules/gallery/tests/Graphics_Helper_Test.php @@ -0,0 +1,89 @@ +assert_equal(array(640, 480, "image/jpeg", "jpg"), + photo::get_file_metadata($photo->resize_path())); + $this->assert_equal(array(200, 150, "image/jpeg", "jpg"), + photo::get_file_metadata($photo->thumb_path())); + // Check that the items table got updated + $this->assert_equal(array(640, 480), array($photo->resize_width, $photo->resize_height)); + $this->assert_equal(array(200, 150), array($photo->thumb_width, $photo->thumb_height)); + // Check that the images are not marked dirty + $this->assert_equal(0, $photo->resize_dirty); + $this->assert_equal(0, $photo->thumb_dirty); + } + + public function generate_movie_test() { + $movie = test::random_movie(); + // Check that the image was correctly resized + $this->assert_equal(array(200, 160, "image/jpeg", "jpg"), + photo::get_file_metadata($movie->thumb_path())); + // Check that the items table got updated + $this->assert_equal(array(200, 160), array($movie->thumb_width, $movie->thumb_height)); + // Check that the image is not marked dirty + $this->assert_equal(0, $movie->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. + file_put_contents($photo->file_path(), test::lorem_ipsum(200)); + // Regenerate + $photo->resize_dirty = 1; + $photo->thumb_dirty = 1; + try { + graphics::generate($photo); + $this->assert_true(false, "Shouldn't get here"); + } catch (Exception $e) { + // Exception expected + } + // Check that the images got replaced with missing image placeholders + $this->assert_same(file_get_contents(MODPATH . "gallery/images/missing_photo.jpg"), + file_get_contents($photo->resize_path())); + $this->assert_same(file_get_contents(MODPATH . "gallery/images/missing_photo.jpg"), + file_get_contents($photo->thumb_path())); + // Check that the items table got updated with new metadata + $this->assert_equal(array(200, 200), array($photo->resize_width, $photo->resize_height)); + $this->assert_equal(array(200, 200), array($photo->thumb_width, $photo->thumb_height)); + // Check that the images are marked as dirty + $this->assert_equal(1, $photo->resize_dirty); + $this->assert_equal(1, $photo->thumb_dirty); + } + + public function generate_bad_movie_test() { + // Unlike photos, its ok to have missing movies - no thrown exceptions, thumb_dirty can be reset. + $movie = test::random_movie(); + // At this point, the movie is valid and has a valid thumb. Make it garble. + file_put_contents($movie->file_path(), test::lorem_ipsum(200)); + // Regenerate + $movie->thumb_dirty = 1; + graphics::generate($movie); + // Check that the image got replaced with a missing image placeholder + $this->assert_same(file_get_contents(MODPATH . "gallery/images/missing_movie.jpg"), + file_get_contents($movie->thumb_path())); + // Check that the items table got updated with new metadata + $this->assert_equal(array(200, 200), array($movie->thumb_width, $movie->thumb_height)); + // Check that the image is *not* marked as dirty + $this->assert_equal(0, $movie->thumb_dirty); + } +} \ No newline at end of file