From d45a73777935c86fc5131955831833d7465b5e9d Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Mon, 21 Jan 2013 01:22:01 -0500 Subject: Update copyright to 2013. Fixes #1953. --- modules/gallery/helpers/gallery_installer.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'modules/gallery/helpers/gallery_installer.php') diff --git a/modules/gallery/helpers/gallery_installer.php b/modules/gallery/helpers/gallery_installer.php index 597771f3..91c785f6 100644 --- a/modules/gallery/helpers/gallery_installer.php +++ b/modules/gallery/helpers/gallery_installer.php @@ -1,7 +1,7 @@ Date: Thu, 24 Jan 2013 12:03:05 +0100 Subject: #1960 - Add unit test to look for extra spaces at end of line - Added no_extra_spaces_at_end_of_line_test to File_Structure_Test. - Updated Gallery_Filters to exclude testing code that isn't ours. - Removed existing extra spaces. New test now passes. --- modules/gallery/helpers/gallery_installer.php | 6 +++--- modules/gallery/helpers/movie.php | 6 +++--- modules/gallery/tests/File_Structure_Test.php | 18 +++++++++++++++++- modules/gallery/tests/Gallery_Filters.php | 4 ++++ modules/gallery/views/movieplayer.html.php | 2 +- modules/info/helpers/info_block.php | 2 +- modules/organize/views/organize_frame.html.php | 6 +++--- themes/admin_wind/css/fix-ie.css | 2 +- 8 files changed, 33 insertions(+), 13 deletions(-) (limited to 'modules/gallery/helpers/gallery_installer.php') diff --git a/modules/gallery/helpers/gallery_installer.php b/modules/gallery/helpers/gallery_installer.php index 91c785f6..fe651a4e 100644 --- a/modules/gallery/helpers/gallery_installer.php +++ b/modules/gallery/helpers/gallery_installer.php @@ -718,14 +718,14 @@ class gallery_installer { if ($version == 50) { // In v51, we added a lock_timeout variable so that administrators could edit the time out - // from 1 second to a higher variable if their system runs concurrent parallel uploads for + // from 1 second to a higher variable if their system runs concurrent parallel uploads for // instance. module::set_var("gallery", "lock_timeout", 1); module::set_version("gallery", $version = 51); } if ($version == 51) { - // In v52, we added functions to the legal_file helper that map photo and movie file + // In v52, we added functions to the legal_file helper that map photo and movie file // extensions to their mime types (and allow extension of the list by other modules). During // this process, we correctly mapped m4v files to video/x-m4v, correcting a previous error // where they were mapped to video/mp4. This corrects the existing items. @@ -736,7 +736,7 @@ class gallery_installer { ->execute(); module::set_version("gallery", $version = 52); } - + if ($version == 52) { // In v53, we added the ability to change the default time used when extracting frames from // movies. Previously we hard-coded this at 3 seconds, so we use that as the default. diff --git a/modules/gallery/helpers/movie.php b/modules/gallery/helpers/movie.php index fb3fab80..7e6a2e55 100644 --- a/modules/gallery/helpers/movie.php +++ b/modules/gallery/helpers/movie.php @@ -82,7 +82,7 @@ class movie_Core { // extract frame at start_time, unless movie is too short $start_time_arg = ($duration >= $start_time + 0.1) ? "-ss " . movie::seconds_to_hhmmssdd($start_time) : ""; - + $input_args = isset($movie_options["input_args"]) ? $movie_options["input_args"] : ""; $output_args = isset($movie_options["output_args"]) ? $movie_options["output_args"] : ""; @@ -164,7 +164,7 @@ class movie_Core { * Return the time/duration formatted in hh:mm:ss.dd from a number of seconds. * Useful for inputs to ffmpeg. * - * Note that this is similar to date("H:i:s", mktime(0,0,$seconds,0,0,0,0)), but unlike this + * Note that this is similar to date("H:i:s", mktime(0,0,$seconds,0,0,0,0)), but unlike this * approach avoids potential issues with time zone and DST mismatch and/or using deprecated * features (the last argument of mkdate above, which disables DST, is deprecated as of PHP 5.3). */ @@ -172,7 +172,7 @@ class movie_Core { return sprintf("%02d:%02d:%05.2f", floor($seconds / 3600), floor(($seconds % 3600) / 60), floor(100 * $seconds % 6000) / 100); } - + /** * Return the number of seconds from a time/duration formatted in hh:mm:ss.dd. * Useful for outputs from ffmpeg. diff --git a/modules/gallery/tests/File_Structure_Test.php b/modules/gallery/tests/File_Structure_Test.php index 7ad865e0..ce75ea13 100644 --- a/modules/gallery/tests/File_Structure_Test.php +++ b/modules/gallery/tests/File_Structure_Test.php @@ -285,7 +285,7 @@ class File_Structure_Test extends Gallery_Unit_Test_Case { } public function all_public_functions_in_test_files_end_in_test() { - // Who tests the tests? :-) + // Who tests the tests? :-) (ref: http://www.xkcd.com/1163) $dir = new PhpCodeFilterIterator( new GalleryCodeFilterIterator( new RecursiveIteratorIterator( @@ -315,4 +315,20 @@ class File_Structure_Test extends Gallery_Unit_Test_Case { } } } + + public function no_extra_spaces_at_end_of_line_test() { + $dir = new GalleryCodeFilterIterator( + new RecursiveIteratorIterator(new RecursiveDirectoryIterator(DOCROOT))); + $errors = ""; + foreach ($dir as $file) { + if (preg_match("/\.(php|css|html|js)$/", $file)) { + foreach (file($file) as $line_num => $line) { + if ((substr($line, -2) == " \n") || (substr($line, -1) == " ")) { + $errors .= "$file at line " . ($line_num + 1) . "\n"; + } + } + } + } + $this->assert_true(empty($errors), "Extra spaces at end of line found at:\n$errors"); + } } diff --git a/modules/gallery/tests/Gallery_Filters.php b/modules/gallery/tests/Gallery_Filters.php index 83dbd53f..6c2a6aa3 100644 --- a/modules/gallery/tests/Gallery_Filters.php +++ b/modules/gallery/tests/Gallery_Filters.php @@ -47,6 +47,10 @@ class GalleryCodeFilterIterator extends FilterIterator { strpos($path_name, SYSPATH) !== false || strpos($path_name, MODPATH . "gallery/libraries/HTMLPurifier") !== false || strpos($path_name, MODPATH . "gallery/vendor/joomla") !== false || + strpos($path_name, MODPATH . "organize/vendor/ext") !== false || + strpos($path_name, DOCROOT . "lib") !== false || + strpos($path_name, DOCROOT . "themes/admin_wind/css/themeroller") !== false || + strpos($path_name, DOCROOT . "themes/wind/css/themeroller") !== false || substr($path_name, -1, 1) == "~"); } } diff --git a/modules/gallery/views/movieplayer.html.php b/modules/gallery/views/movieplayer.html.php index 25cb9f58..edb5184c 100644 --- a/modules/gallery/views/movieplayer.html.php +++ b/modules/gallery/views/movieplayer.html.php @@ -18,7 +18,7 @@ $("#" + id).css({width: width, height: height}); }; // setup flowplayer - flowplayer(id, + flowplayer(id, $.extend(true, { "src": "", "wmode": "transparent", diff --git a/modules/info/helpers/info_block.php b/modules/info/helpers/info_block.php index 4079fe55..62aa0746 100644 --- a/modules/info/helpers/info_block.php +++ b/modules/info/helpers/info_block.php @@ -29,7 +29,7 @@ class info_block_Core { if ($theme->item()) { $block = new Block(); $block->css_id = "g-metadata"; - $block->title = $theme->item()->is_album() ? t("Album info") : + $block->title = $theme->item()->is_album() ? t("Album info") : ($theme->item()->is_movie() ? t("Movie info") : t("Photo info")); $block->content = new View("info_block.html"); if ($theme->item->title && module::get_var("info", "show_title")) { diff --git a/modules/organize/views/organize_frame.html.php b/modules/organize/views/organize_frame.html.php index c33b7607..b1d92675 100644 --- a/modules/organize/views/organize_frame.html.php +++ b/modules/organize/views/organize_frame.html.php @@ -365,7 +365,7 @@ } } }); - + var tag_button = new Ext.Button({ flex: 2, text: for_js() ?>, @@ -425,11 +425,11 @@ sort_column_combobox, sort_order_combobox ] - }, + }, { xtype: "spacer", - flex: 3 + flex: 3 }, tag_textfield, tag_button, diff --git a/themes/admin_wind/css/fix-ie.css b/themes/admin_wind/css/fix-ie.css index 5475cb79..4546f9fa 100644 --- a/themes/admin_wind/css/fix-ie.css +++ b/themes/admin_wind/css/fix-ie.css @@ -15,4 +15,4 @@ tr.g-info td, tr.g-success td, tr.g-warning td { background: none !important; -} +} -- cgit v1.2.3 From cc2aed656c7289881ba23e8ec700a4daf7889688 Mon Sep 17 00:00:00 2001 From: shadlaws Date: Sat, 26 Jan 2013 08:38:31 +0100 Subject: #1946, 1947 - Make altered names/slugs more user-friendly, make conflict-finding code check filenames with no extensions - Reduced from four places that alter names/slugs to two (one to populate empty slugs, one to handle conflicting names/slugs). - For empty slugs, fill with generic human-readable name (e.g. "photo") instead of random value. - For conflicting names/slugs, add suffix that's sequential (e.g. "-01"), only using random after 99 conflicts. - Made conflict-finding code check filenames with no extensions. - Renamed _randomize_name_or_slug_on_conflict to _check_and_fix_conflicts. - Added unit tests. Also cleaned up existing unit tests to reflect new logic and removed a redundant test. - Added installer logic to correct existing items now considered in conflict. - Revised gallery_task to look for duplicate names based on new criteria. --- installer/install.sql | 2 +- modules/gallery/helpers/gallery_installer.php | 43 +++++- modules/gallery/helpers/gallery_task.php | 113 +++++++++++++-- modules/gallery/helpers/item.php | 63 +++------ modules/gallery/models/item.php | 127 +++++++++++------ modules/gallery/module.info | 2 +- modules/gallery/tests/Item_Model_Test.php | 194 ++++++++++++++++++++++---- 7 files changed, 418 insertions(+), 126 deletions(-) (limited to 'modules/gallery/helpers/gallery_installer.php') diff --git a/installer/install.sql b/installer/install.sql index b01c5a7c..70cf0c86 100644 --- a/installer/install.sql +++ b/installer/install.sql @@ -244,7 +244,7 @@ CREATE TABLE {modules} ( KEY `weight` (`weight`) ) AUTO_INCREMENT=10 DEFAULT CHARSET=utf8; /*!40101 SET character_set_client = @saved_cs_client */; -INSERT INTO {modules} VALUES (1,1,'gallery',53,1); +INSERT INTO {modules} VALUES (1,1,'gallery',54,1); INSERT INTO {modules} VALUES (2,1,'user',4,2); INSERT INTO {modules} VALUES (3,1,'comment',7,3); INSERT INTO {modules} VALUES (4,1,'organize',4,4); diff --git a/modules/gallery/helpers/gallery_installer.php b/modules/gallery/helpers/gallery_installer.php index fe651a4e..86ff7ce5 100644 --- a/modules/gallery/helpers/gallery_installer.php +++ b/modules/gallery/helpers/gallery_installer.php @@ -315,7 +315,7 @@ class gallery_installer { module::set_var("gallery", "lock_timeout", 1); module::set_var("gallery", "movie_extract_frame_time", 3); - module::set_version("gallery", 53); + module::set_version("gallery", 54); } static function upgrade($version) { @@ -743,6 +743,47 @@ class gallery_installer { module::set_var("gallery", "movie_extract_frame_time", 3); module::set_version("gallery", $version = 53); } + + if ($version == 53) { + // In v54, we changed how we check for name and slug conflicts in Item_Model. Previously, + // we checked the whole filename. As a result, "foo.jpg" and "foo.png" were not considered + // conflicting if their slugs were different (a rare case in practice since server_add and + // uploader would give them both the same slug "foo"). Now, we check the filename without its + // extension. This upgrade stanza fixes any conflicts where they were previously allowed. + + // This might be slow, but if it times out it can just pick up where it left off. + + // Find and loop through each conflict (e.g. "foo.jpg", "foo.png", and "foo.flv" are one + // conflict; "bar.jpg", "bar.png", and "bar.flv" are another) + foreach (db::build() + ->select_distinct(array("parent_base_name" => + db::expr("CONCAT(`parent_id`, ':', LOWER(SUBSTR(`name`, 1, LOCATE('.', `name`) - 1)))"))) + ->select(array("C" => "COUNT(\"*\")")) + ->from("items") + ->where("type", "<>", "album") + ->having("C", ">", 1) + ->group_by("parent_base_name") + ->execute() as $conflict) { + list ($parent_id, $base_name) = explode(":", $conflict->parent_base_name, 2); + $base_name_escaped = Database::escape_for_like($base_name); + // Loop through the items for each conflict + foreach (db::build() + ->from("items") + ->select("id") + ->where("type", "<>", "album") + ->where("parent_id", "=", $parent_id) + ->where("name", "LIKE", "{$base_name_escaped}.%") + ->limit(1000000) // required to satisfy SQL syntax (no offset without limit) + ->offset(1) // skips the 0th item + ->execute() as $row) { + set_time_limit(30); + $item = ORM::factory("item", $row->id); + $item->name = $item->name; // this will force Item_Model to check for conflicts on save + $item->save(); + } + } + module::set_version("gallery", $version = 54); + } } static function uninstall() { diff --git a/modules/gallery/helpers/gallery_task.php b/modules/gallery/helpers/gallery_task.php index 82ca9ffc..e986b822 100644 --- a/modules/gallery/helpers/gallery_task.php +++ b/modules/gallery/helpers/gallery_task.php @@ -26,11 +26,13 @@ class gallery_task_Core { const FIX_STATE_RUN_DUPE_SLUGS = 5; const FIX_STATE_START_DUPE_NAMES = 6; const FIX_STATE_RUN_DUPE_NAMES = 7; - const FIX_STATE_START_REBUILD_ITEM_CACHES = 8; - const FIX_STATE_RUN_REBUILD_ITEM_CACHES = 9; - const FIX_STATE_START_MISSING_ACCESS_CACHES = 10; - const FIX_STATE_RUN_MISSING_ACCESS_CACHES = 11; - const FIX_STATE_DONE = 12; + const FIX_STATE_START_DUPE_BASE_NAMES = 8; + const FIX_STATE_RUN_DUPE_BASE_NAMES = 9; + const FIX_STATE_START_REBUILD_ITEM_CACHES = 10; + const FIX_STATE_RUN_REBUILD_ITEM_CACHES = 11; + const FIX_STATE_START_MISSING_ACCESS_CACHES = 12; + const FIX_STATE_RUN_MISSING_ACCESS_CACHES = 13; + const FIX_STATE_DONE = 14; static function available_tasks() { $dirty_count = graphics::find_dirty_images_query()->count_records(); @@ -348,8 +350,9 @@ class gallery_task_Core { // album audit (permissions and bogus album covers): 1 operation for every album $total += db::build()->where("type", "=", "album")->count_records("items"); - // one operation for each missing slug, name and access cache - foreach (array("find_dupe_slugs", "find_dupe_names", "find_missing_access_caches") as $func) { + // one operation for each dupe slug, dupe name, dupe base name, and missing access cache + foreach (array("find_dupe_slugs", "find_dupe_names", "find_dupe_base_names", + "find_missing_access_caches") as $func) { foreach (self::$func() as $row) { $total++; } @@ -489,11 +492,12 @@ class gallery_task_Core { $task->set("stack", implode(" ", $stack)); $state = self::FIX_STATE_RUN_DUPE_NAMES; } else { - $state = self::FIX_STATE_START_ALBUMS; + $state = self::FIX_STATE_START_DUPE_BASE_NAMES; } break; case self::FIX_STATE_RUN_DUPE_NAMES: + // NOTE: This does *not* attempt to fix the file system! $stack = explode(" ", $task->get("stack")); list ($parent_id, $name) = explode(":", array_pop($stack)); @@ -505,9 +509,16 @@ class gallery_task_Core { ->find_all(1, 1); if ($conflicts->count() && $conflict = $conflicts->current()) { $task->log("Fixing conflicting name for item id {$conflict->id}"); + if (!$conflict->is_album() && preg_match("/^(.*)(\.[^\.\/]*?)$/", $conflict->name, $matches)) { + $item_base_name = $matches[1]; + $item_extension = $matches[2]; // includes a leading dot + } else { + $item_base_name = $conflict->name; + $item_extension = ""; + } db::build() ->update("items") - ->set("name", $name . "-" . (string)rand(1000, 9999)) + ->set("name", $item_base_name . "-" . (string)rand(1000, 9999) . $item_extension) ->where("id", "=", $conflict->id) ->execute(); @@ -521,6 +532,74 @@ class gallery_task_Core { $task->set("stack", implode(" ", $stack)); $completed++; + if (empty($stack)) { + $state = self::FIX_STATE_START_DUPE_BASE_NAMES; + } + break; + + case self::FIX_STATE_START_DUPE_BASE_NAMES: + $stack = array(); + foreach (self::find_dupe_base_names() as $row) { + list ($parent_id, $base_name) = explode(":", $row->parent_base_name, 2); + $stack[] = join(":", array($parent_id, $base_name)); + } + if ($stack) { + $task->set("stack", implode(" ", $stack)); + $state = self::FIX_STATE_RUN_DUPE_BASE_NAMES; + } else { + $state = self::FIX_STATE_START_ALBUMS; + } + break; + + case self::FIX_STATE_RUN_DUPE_BASE_NAMES: + // NOTE: This *does* attempt to fix the file system! So, it must go *after* run_dupe_names. + $stack = explode(" ", $task->get("stack")); + list ($parent_id, $base_name) = explode(":", array_pop($stack)); + $base_name_escaped = Database::escape_for_like($base_name); + + $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", "LIKE", "{$base_name_escaped}.%") + ->where("type", "<>", "album") + ->find_all(1, 1); + if ($conflicts->count() && $conflict = $conflicts->current()) { + $task->log("Fixing conflicting name for item id {$conflict->id}"); + if (preg_match("/^(.*)(\.[^\.\/]*?)$/", $conflict->name, $matches)) { + $item_base_name = $matches[1]; // unlike $base_name, this always maintains capitalization + $item_extension = $matches[2]; // includes a leading dot + } else { + $item_base_name = $conflict->name; + $item_extension = ""; + } + // Unlike conflicts found in run_dupe_names, these items are likely to have an intact + // file system. Let's use the item save logic to rebuild the paths and rename the files + // if possible. + try { + $conflict->name = $item_base_name . "-" . (string)rand(1000, 9999) . $item_extension; + $conflict->validate(); + // If we get here, we're safe to proceed with save + $conflict->save(); + } catch (Exception $e) { + // Didn't work. Edit database directly without fixing file system. + db::build() + ->update("items") + ->set("name", $item_base_name . "-" . (string)rand(1000, 9999) . $item_extension) + ->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:$base_name"; + break; + } + $task->set("stack", implode(" ", $stack)); + $completed++; + if (empty($stack)) { $state = self::FIX_STATE_START_ALBUMS; } @@ -669,18 +748,32 @@ class gallery_task_Core { } static function find_dupe_names() { + // looking for photos, movies, and albums return db::build() ->select_distinct( array("parent_name" => db::expr("CONCAT(`parent_id`, ':', LOWER(`name`))"))) ->select("id") ->select(array("C" => "COUNT(\"*\")")) ->from("items") - ->where("type", "<>", "album") ->having("C", ">", 1) ->group_by("parent_name") ->execute(); } + static function find_dupe_base_names() { + // looking for photos or movies, not albums + return db::build() + ->select_distinct( + array("parent_base_name" => db::expr("CONCAT(`parent_id`, ':', LOWER(SUBSTR(`name`, 1, LOCATE('.', `name`) - 1)))"))) + ->select("id") + ->select(array("C" => "COUNT(\"*\")")) + ->from("items") + ->where("type", "<>", "album") + ->having("C", ">", 1) + ->group_by("parent_base_name") + ->execute(); + } + static function find_empty_item_caches($limit) { return db::build() ->select("items.id") diff --git a/modules/gallery/helpers/item.php b/modules/gallery/helpers/item.php index b2c4cadf..386eeb08 100644 --- a/modules/gallery/helpers/item.php +++ b/modules/gallery/helpers/item.php @@ -39,55 +39,28 @@ class item_Core { } } - $source->parent_id = $target->id; - - // Moving may result in name or slug conflicts. If that happens, try up to 5 times to pick a - // random name (or slug) to avoid the conflict. $orig_name = $source->name; - $orig_name_filename = pathinfo($source->name, PATHINFO_FILENAME); - $orig_name_extension = pathinfo($source->name, PATHINFO_EXTENSION); - $orig_slug = $source->slug; - for ($i = 0; $i < 5; $i++) { - try { - $source->save(); - if ($orig_name != $source->name) { - switch ($source->type) { - case "album": - message::info( - t("Album %old_name renamed to %new_name to avoid a conflict", - array("old_name" => $orig_name, "new_name" => $source->name))); - break; - - case "photo": - message::info( - t("Photo %old_name renamed to %new_name to avoid a conflict", - array("old_name" => $orig_name, "new_name" => $source->name))); - break; + $source->parent_id = $target->id; + $source->save(); + if ($orig_name != $source->name) { + switch ($source->type) { + case "album": + message::info( + t("Album %old_name renamed to %new_name to avoid a conflict", + array("old_name" => $orig_name, "new_name" => $source->name))); + break; - case "movie": - message::info( - t("Movie %old_name renamed to %new_name to avoid a conflict", - array("old_name" => $orig_name, "new_name" => $source->name))); - break; - } - } + case "photo": + message::info( + t("Photo %old_name renamed to %new_name to avoid a conflict", + array("old_name" => $orig_name, "new_name" => $source->name))); break; - } catch (ORM_Validation_Exception $e) { - $rand = rand(10, 99); - $errors = $e->validation->errors(); - if (isset($errors["name"])) { - $source->name = $orig_name_filename . "-{$rand}." . $orig_name_extension; - unset($errors["name"]); - } - if (isset($errors["slug"])) { - $source->slug = $orig_slug . "-{$rand}"; - unset($errors["slug"]); - } - if ($errors) { - // There were other validation issues-- we don't know how to handle those - throw $e; - } + case "movie": + message::info( + t("Movie %old_name renamed to %new_name to avoid a conflict", + array("old_name" => $orig_name, "new_name" => $source->name))); + break; } } diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php index bbfeee47..e4b997aa 100644 --- a/modules/gallery/models/item.php +++ b/modules/gallery/models/item.php @@ -338,10 +338,11 @@ class Item_Model_Core extends ORM_MPTT { if (empty($this->slug)) { $this->slug = item::convert_filename_to_slug(pathinfo($this->name, PATHINFO_FILENAME)); - // If the filename is all invalid characters, then the slug may be empty here. Pick a - // random value. + // If the filename is all invalid characters, then the slug may be empty here. We set a + // generic name ("photo", "movie", or "album") based on its type, then rely on + // check_and_fix_conflicts to ensure it doesn't conflict with another name. if (empty($this->slug)) { - $this->slug = (string)rand(1000, 9999); + $this->slug = $this->type; } } @@ -362,7 +363,7 @@ class Item_Model_Core extends ORM_MPTT { } } - $this->_randomize_name_or_slug_on_conflict(); + $this->_check_and_fix_conflicts(); parent::save(); @@ -382,16 +383,6 @@ class Item_Model_Core extends ORM_MPTT { case "photo": case "movie": - // The thumb or resize may already exist in the case where a movie and a photo generate - // a thumbnail of the same name (eg, foo.flv movie and foo.jpg photo will generate - // foo.jpg thumbnail). If that happens, randomize and save again. - if (file_exists($this->resize_path()) || - file_exists($this->thumb_path())) { - $pi = pathinfo($this->name); - $this->name = $pi["filename"] . "-" . random::int() . "." . $pi["extension"]; - parent::save(); - } - copy($this->data_file, $this->file_path()); break; } @@ -435,7 +426,7 @@ class Item_Model_Core extends ORM_MPTT { $this->relative_url_cache = null; } - $this->_randomize_name_or_slug_on_conflict(); + $this->_check_and_fix_conflicts(); parent::save(); @@ -528,30 +519,60 @@ class Item_Model_Core extends ORM_MPTT { /** * Check to see if there's another item that occupies the same name or slug that this item - * intends to use, and if so choose a new name/slug while preserving the extension. - * @todo Improve this. Random numbers are not user friendly + * intends to use, and if so choose a new name/slug while preserving the extension. Since this + * checks the name without its extension, it covers possible collisions with thumbs and resizes + * as well (e.g. between the thumbs of movie "foo.flv" and photo "foo.jpg"). */ - private function _randomize_name_or_slug_on_conflict() { - $base_name = pathinfo($this->name, PATHINFO_FILENAME); - $base_ext = pathinfo($this->name, PATHINFO_EXTENSION); - $base_slug = $this->slug; - while (ORM::factory("item") - ->where("parent_id", "=", $this->parent_id) - ->where("id", $this->id ? "<>" : "IS NOT", $this->id) - ->and_open() - ->where("name", "=", $this->name) - ->or_where("slug", "=", $this->slug) - ->close() - ->find()->id) { - $rand = random::int(); - if ($base_ext) { - $this->name = "$base_name-$rand.$base_ext"; + private function _check_and_fix_conflicts() { + $suffix_num = 1; + $suffix = ""; + if ($this->is_album()) { + while (db::build() + ->from("items") + ->where("parent_id", "=", $this->parent_id) + ->where("id", $this->id ? "<>" : "IS NOT", $this->id) + ->and_open() + ->where("name", "=", "{$this->name}{$suffix}") + ->or_where("slug", "=", "{$this->slug}{$suffix}") + ->close() + ->count_records()) { + $suffix = "-" . (($suffix_num <= 99) ? sprintf("%02d", $suffix_num++) : random::int()); + } + if ($suffix) { + $this->name = "{$this->name}{$suffix}"; + $this->slug = "{$this->slug}{$suffix}"; + $this->relative_path_cache = null; + $this->relative_url_cache = null; + } + } else { + // Split the filename into its base and extension. This uses a regexp similar to + // legal_file::change_extension (which isn't always the same as pathinfo). + if (preg_match("/^(.*)(\.[^\.\/]*?)$/", $this->name, $matches)) { + $base_name = $matches[1]; + $extension = $matches[2]; // includes a leading dot } else { - $this->name = "$base_name-$rand"; + $base_name = $this->name; + $extension = ""; + } + $base_name_escaped = Database::escape_for_like($base_name); + // Note: below query uses LIKE with wildcard % at end, which is still sargable (i.e. quick) + while (db::build() + ->from("items") + ->where("parent_id", "=", $this->parent_id) + ->where("id", $this->id ? "<>" : "IS NOT", $this->id) + ->and_open() + ->where("name", "LIKE", "{$base_name_escaped}{$suffix}.%") + ->or_where("slug", "=", "{$this->slug}{$suffix}") + ->close() + ->count_records()) { + $suffix = "-" . (($suffix_num <= 99) ? sprintf("%02d", $suffix_num++) : random::int()); + } + if ($suffix) { + $this->name = "{$base_name}{$suffix}{$extension}"; + $this->slug = "{$this->slug}{$suffix}"; + $this->relative_path_cache = null; + $this->relative_url_cache = null; } - $this->slug = "$base_slug-$rand"; - $this->relative_path_cache = null; - $this->relative_url_cache = null; } } @@ -871,14 +892,32 @@ class Item_Model_Core extends ORM_MPTT { } } - if (db::build() - ->from("items") - ->where("parent_id", "=", $this->parent_id) - ->where("name", "=", $this->name) - ->merge_where($this->id ? array(array("id", "<>", $this->id)) : null) - ->count_records()) { - $v->add_error("name", "conflict"); - return; + if ($this->is_album()) { + if (db::build() + ->from("items") + ->where("parent_id", "=", $this->parent_id) + ->where("name", "=", $this->name) + ->merge_where($this->id ? array(array("id", "<>", $this->id)) : null) + ->count_records()) { + $v->add_error("name", "conflict"); + return; + } + } else { + if (preg_match("/^(.*)(\.[^\.\/]*?)$/", $this->name, $matches)) { + $base_name = $matches[1]; + } else { + $base_name = $this->name; + } + $base_name_escaped = Database::escape_for_like($base_name); + if (db::build() + ->from("items") + ->where("parent_id", "=", $this->parent_id) + ->where("name", "LIKE", "{$base_name_escaped}.%") + ->merge_where($this->id ? array(array("id", "<>", $this->id)) : null) + ->count_records()) { + $v->add_error("name", "conflict"); + return; + } } if ($this->parent_id == 1 && Kohana::auto_load("{$this->slug}_Controller")) { diff --git a/modules/gallery/module.info b/modules/gallery/module.info index 566ca2eb..b6cefe39 100644 --- a/modules/gallery/module.info +++ b/modules/gallery/module.info @@ -1,6 +1,6 @@ name = "Gallery 3" description = "Gallery core application" -version = 53 +version = 54 author_name = "Gallery Team" author_url = "http://codex.galleryproject.org/Gallery:Team" info_url = "http://codex.galleryproject.org/Gallery3:Modules:gallery" diff --git a/modules/gallery/tests/Item_Model_Test.php b/modules/gallery/tests/Item_Model_Test.php index c45bbc3c..41361b32 100644 --- a/modules/gallery/tests/Item_Model_Test.php +++ b/modules/gallery/tests/Item_Model_Test.php @@ -136,19 +136,6 @@ class Item_Model_Test extends Gallery_Unit_Test_Case { $this->assert_true(false, "Shouldn't get here"); } - public function item_rename_over_existing_name_gets_uniqified_test() { - // Create a test photo - $item = test::random_photo(); - $item2 = test::random_photo(); - - $item->name = $item2->name; - $item->save(); - - // foo.jpg should become foo-####.jpg - $this->assert_true( - preg_match("/" . str_replace(".jpg", "", $item2->name) . "-\d+\.jpg/", $item->name)); - } - public function move_album_test() { $album2 = test::random_album(); $album1 = test::random_album($album2); @@ -205,7 +192,7 @@ class Item_Model_Test extends Gallery_Unit_Test_Case { $this->assert_equal($fullsize_file, file_get_contents($photo->file_path())); } - public function move_album_with_conflicting_target_gets_uniqified_test() { + public function move_album_with_conflicting_target_gets_uniquified_test() { $album = test::random_album(); $source = test::random_album_unsaved($album); $source->name = $album->name; @@ -217,9 +204,9 @@ class Item_Model_Test extends Gallery_Unit_Test_Case { $source->parent_id = item::root()->id; $source->save(); - // foo should become foo-#### - $this->assert_true(preg_match("/{$album->name}-\d+/", $source->name)); - $this->assert_true(preg_match("/{$album->slug}-\d+/", $source->slug)); + // foo should become foo-01 + $this->assert_same("{$album->name}-01", $source->name); + $this->assert_same("{$album->slug}-01", $source->slug); } public function move_album_fails_wrong_target_type_test() { @@ -239,7 +226,7 @@ class Item_Model_Test extends Gallery_Unit_Test_Case { $this->assert_true(false, "Shouldn't get here"); } - public function move_photo_with_conflicting_target_gets_uniqified_test() { + public function move_photo_with_conflicting_target_gets_uniquified_test() { $photo1 = test::random_photo(); $album = test::random_album(); $photo2 = test::random_photo_unsaved($album); @@ -247,17 +234,16 @@ class Item_Model_Test extends Gallery_Unit_Test_Case { $photo2->save(); // $photo1 and $photo2 have the same name, so if we move $photo1 into the root they should - // conflict and get uniqified. + // conflict and get uniquified. $photo2->parent_id = item::root()->id; $photo2->save(); - // foo.jpg should become foo-####.jpg - $this->assert_true( - preg_match("/" . str_replace(".jpg", "", $photo1->name) . "-\d+\.jpg/", $photo2->name)); + // foo.jpg should become foo-01.jpg + $this->assert_same(pathinfo($photo1->name, PATHINFO_FILENAME) . "-01.jpg", $photo2->name); - // foo should become foo - $this->assert_true(preg_match("/{$photo1->slug}/", $photo2->name)); + // foo should become foo-01 + $this->assert_same("{$photo1->slug}-01", $photo2->slug); } public function move_album_inside_descendent_fails_test() { @@ -535,4 +521,164 @@ class Item_Model_Test extends Gallery_Unit_Test_Case { $album->name = $album->name . ".foo.bar"; $album->save(); } + + public function no_conflict_when_parents_different_test() { + $parent1 = test::random_album(); + $parent2 = test::random_album(); + $photo1 = test::random_photo($parent1); + $photo2 = test::random_photo($parent2); + + $photo2->name = $photo1->name; + $photo2->slug = $photo1->slug; + $photo2->save(); + + // photo2 has same name and slug as photo1 but different parent - no conflict. + $this->assert_same($photo1->name, $photo2->name); + $this->assert_same($photo1->slug, $photo2->slug); + } + + public function fix_conflict_when_names_identical_test() { + $parent = test::random_album(); + $photo1 = test::random_photo($parent); + $photo2 = test::random_photo($parent); + + $photo1_orig_base = pathinfo($photo1->name, PATHINFO_FILENAME); + $photo2_orig_slug = $photo2->slug; + + $photo2->name = $photo1->name; + $photo2->save(); + + // photo2 has same name as photo1 - conflict resolved by renaming with -01. + $this->assert_same("{$photo1_orig_base}-01.jpg", $photo2->name); + $this->assert_same("{$photo2_orig_slug}-01", $photo2->slug); + } + + public function fix_conflict_when_slugs_identical_test() { + $parent = test::random_album(); + $photo1 = test::random_photo($parent); + $photo2 = test::random_photo($parent); + + $photo2_orig_base = pathinfo($photo2->name, PATHINFO_FILENAME); + + $photo2->slug = $photo1->slug; + $photo2->save(); + + // photo2 has same slug as photo1 - conflict resolved by renaming with -01. + $this->assert_same("{$photo2_orig_base}-01.jpg", $photo2->name); + $this->assert_same("{$photo1->slug}-01", $photo2->slug); + } + + public function no_conflict_when_parents_different_for_albums_test() { + $parent1 = test::random_album(); + $parent2 = test::random_album(); + $album1 = test::random_album($parent1); + $album2 = test::random_album($parent2); + + $album2->name = $album1->name; + $album2->slug = $album1->slug; + $album2->save(); + + // album2 has same name and slug as album1 but different parent - no conflict. + $this->assert_same($album1->name, $album2->name); + $this->assert_same($album1->slug, $album2->slug); + } + + public function fix_conflict_when_names_identical_for_albums_test() { + $parent = test::random_album(); + $album1 = test::random_album($parent); + $album2 = test::random_album($parent); + + $album2_orig_slug = $album2->slug; + + $album2->name = $album1->name; + $album2->save(); + + // album2 has same name as album1 - conflict resolved by renaming with -01. + $this->assert_same("{$album1->name}-01", $album2->name); + $this->assert_same("{$album2_orig_slug}-01", $album2->slug); + } + + public function fix_conflict_when_slugs_identical_for_albums_test() { + $parent = test::random_album(); + $album1 = test::random_album($parent); + $album2 = test::random_album($parent); + + $album2_orig_name = $album2->name; + + $album2->slug = $album1->slug; + $album2->save(); + + // album2 has same slug as album1 - conflict resolved by renaming with -01. + $this->assert_same("{$album2_orig_name}-01", $album2->name); + $this->assert_same("{$album1->slug}-01", $album2->slug); + } + + public function no_conflict_when_base_names_identical_between_album_and_photo_test() { + $parent = test::random_album(); + $album = test::random_album($parent); + $photo = test::random_photo($parent); + + $photo_orig_slug = $photo->slug; + + $photo->name = "{$album->name}.jpg"; + $photo->save(); + + // photo has same base name as album - no conflict. + $this->assert_same("{$album->name}.jpg", $photo->name); + $this->assert_same($photo_orig_slug, $photo->slug); + } + + public function fix_conflict_when_full_names_identical_between_album_and_photo_test() { + $parent = test::random_album(); + $photo = test::random_photo($parent); + $album = test::random_album($parent); + + $album_orig_slug = $album->slug; + + $album->name = $photo->name; + $album->save(); + + // album has same full name as album - conflict resolved by renaming with -01. + $this->assert_same("{$photo->name}-01", $album->name); + $this->assert_same("{$album_orig_slug}-01", $album->slug); + } + + public function fix_conflict_when_slugs_identical_between_album_and_photo_test() { + $parent = test::random_album(); + $album = test::random_album($parent); + $photo = test::random_photo($parent); + + $photo_orig_base = pathinfo($photo->name, PATHINFO_FILENAME); + + $photo->slug = $album->slug; + $photo->save(); + + // photo has same slug as album - conflict resolved by renaming with -01. + $this->assert_same("{$photo_orig_base}-01.jpg", $photo->name); + $this->assert_same("{$album->slug}-01", $photo->slug); + } + + public function fix_conflict_when_base_names_identical_between_jpg_png_flv_test() { + $parent = test::random_album(); + $item1 = test::random_photo($parent); + $item2 = test::random_photo($parent); + $item3 = test::random_movie($parent); + + $item1_orig_base = pathinfo($item1->name, PATHINFO_FILENAME); + $item2_orig_slug = $item2->slug; + $item3_orig_slug = $item3->slug; + + $item2->set_data_file(MODPATH . "gallery/images/graphicsmagick.png"); + $item2->name = "{$item1_orig_base}.png"; + $item2->save(); + + $item3->name = "{$item1_orig_base}.flv"; + $item3->save(); + + // item2 and item3 have same base name as item1 - conflict resolved by renaming with -01 and -02. + $this->assert_same("{$item1_orig_base}-01.png", $item2->name); + $this->assert_same("{$item2_orig_slug}-01", $item2->slug); + $this->assert_same("{$item1_orig_base}-02.flv", $item3->name); + $this->assert_same("{$item3_orig_slug}-02", $item3->slug); + } } -- cgit v1.2.3 From e957c97c947427093d03de3302f4d98909d1ab1a Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Sun, 27 Jan 2013 15:35:42 -0500 Subject: Add a key on relative_path_cache in the items table to improve performance on installs that use File_Proxy heavily. Fixes #1920. --- installer/install.sql | 5 +++-- modules/gallery/helpers/gallery_installer.php | 11 +++++++++-- modules/gallery/module.info | 2 +- 3 files changed, 13 insertions(+), 5 deletions(-) (limited to 'modules/gallery/helpers/gallery_installer.php') diff --git a/installer/install.sql b/installer/install.sql index 70cf0c86..4097d51e 100644 --- a/installer/install.sql +++ b/installer/install.sql @@ -186,7 +186,8 @@ CREATE TABLE {items} ( KEY `type` (`type`), KEY `random` (`rand_key`), KEY `weight` (`weight`), - KEY `left_ptr` (`left_ptr`) + KEY `left_ptr` (`left_ptr`), + KEY `relative_path_cache` (`relative_path_cache`) ) AUTO_INCREMENT=2 DEFAULT CHARSET=utf8; /*!40101 SET character_set_client = @saved_cs_client */; INSERT INTO {items} VALUES (1,NULL,NULL,UNIX_TIMESTAMP(),'',NULL,1,1,NULL,NULL,2,0,NULL,'','',1,NULL,NULL,2,NULL,'weight','ASC',1,NULL,NULL,'Gallery','album',UNIX_TIMESTAMP(),0,1,NULL,'1','1'); @@ -244,7 +245,7 @@ CREATE TABLE {modules} ( KEY `weight` (`weight`) ) AUTO_INCREMENT=10 DEFAULT CHARSET=utf8; /*!40101 SET character_set_client = @saved_cs_client */; -INSERT INTO {modules} VALUES (1,1,'gallery',54,1); +INSERT INTO {modules} VALUES (1,1,'gallery',55,1); INSERT INTO {modules} VALUES (2,1,'user',4,2); INSERT INTO {modules} VALUES (3,1,'comment',7,3); INSERT INTO {modules} VALUES (4,1,'organize',4,4); diff --git a/modules/gallery/helpers/gallery_installer.php b/modules/gallery/helpers/gallery_installer.php index 86ff7ce5..d4c4de14 100644 --- a/modules/gallery/helpers/gallery_installer.php +++ b/modules/gallery/helpers/gallery_installer.php @@ -116,7 +116,8 @@ class gallery_installer { KEY `type` (`type`), KEY `random` (`rand_key`), KEY `weight` (`weight` DESC), - KEY `left_ptr` (`left_ptr`)) + KEY `left_ptr` (`left_ptr`), + KEY `relative_path_cache` (`relative_path_cache`)) DEFAULT CHARSET=utf8;"); $db->query("CREATE TABLE {logs} ( @@ -315,7 +316,7 @@ class gallery_installer { module::set_var("gallery", "lock_timeout", 1); module::set_var("gallery", "movie_extract_frame_time", 3); - module::set_version("gallery", 54); + module::set_version("gallery", 55); } static function upgrade($version) { @@ -784,6 +785,12 @@ class gallery_installer { } module::set_version("gallery", $version = 54); } + + if ($version == 54) { + $db->query("ALTER TABLE {items} ADD KEY `relative_path_cache` (`relative_path_cache`)"); + module::set_version("gallery", $version = 55); + } + } static function uninstall() { diff --git a/modules/gallery/module.info b/modules/gallery/module.info index b6cefe39..d79a5077 100644 --- a/modules/gallery/module.info +++ b/modules/gallery/module.info @@ -1,6 +1,6 @@ name = "Gallery 3" description = "Gallery core application" -version = 54 +version = 55 author_name = "Gallery Team" author_url = "http://codex.galleryproject.org/Gallery:Team" info_url = "http://codex.galleryproject.org/Gallery3:Modules:gallery" -- cgit v1.2.3 From 93963422505ecc790af62ae0503f301145debac3 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Thu, 31 Jan 2013 19:55:53 -0500 Subject: Drop the requirement to have the install() function set the module version. It's redundant. Fixes #1985. --- modules/comment/helpers/comment_installer.php | 1 - modules/digibug/helpers/digibug_installer.php | 1 - modules/exif/helpers/exif_installer.php | 1 - modules/g2_import/helpers/g2_import_installer.php | 1 - modules/gallery/helpers/gallery_installer.php | 2 -- modules/gallery/helpers/module.php | 3 +-- modules/image_block/helpers/image_block_installer.php | 1 - modules/info/helpers/info_installer.php | 1 - modules/notification/helpers/notification_installer.php | 2 -- modules/rest/helpers/rest_installer.php | 1 - modules/search/helpers/search_installer.php | 1 - modules/server_add/helpers/server_add_installer.php | 1 - modules/slideshow/helpers/slideshow_installer.php | 1 - modules/tag/helpers/tag_installer.php | 1 - modules/user/helpers/user_installer.php | 1 - modules/watermark/helpers/watermark_installer.php | 1 - 16 files changed, 1 insertion(+), 19 deletions(-) (limited to 'modules/gallery/helpers/gallery_installer.php') diff --git a/modules/comment/helpers/comment_installer.php b/modules/comment/helpers/comment_installer.php index 6dbd31cf..136f96ef 100644 --- a/modules/comment/helpers/comment_installer.php +++ b/modules/comment/helpers/comment_installer.php @@ -49,7 +49,6 @@ class comment_installer { module::set_var("comment", "spam_caught", 0); module::set_var("comment", "access_permissions", "everybody"); module::set_var("comment", "rss_visible", "all"); - module::set_version("comment", 7); } static function upgrade($version) { diff --git a/modules/digibug/helpers/digibug_installer.php b/modules/digibug/helpers/digibug_installer.php index b2e529d7..be88b5ec 100644 --- a/modules/digibug/helpers/digibug_installer.php +++ b/modules/digibug/helpers/digibug_installer.php @@ -30,7 +30,6 @@ class digibug_installer { module::set_var("digibug", "company_id", "3153"); module::set_var("digibug", "event_id", "8491"); - module::set_version("digibug", 2); } static function upgrade($version) { diff --git a/modules/exif/helpers/exif_installer.php b/modules/exif/helpers/exif_installer.php index f4c2aa3b..75d0f835 100644 --- a/modules/exif/helpers/exif_installer.php +++ b/modules/exif/helpers/exif_installer.php @@ -29,7 +29,6 @@ class exif_installer { PRIMARY KEY (`id`), KEY(`item_id`)) DEFAULT CHARSET=utf8;"); - module::set_version("exif", 1); } static function activate() { diff --git a/modules/g2_import/helpers/g2_import_installer.php b/modules/g2_import/helpers/g2_import_installer.php index b0c14425..c7569819 100644 --- a/modules/g2_import/helpers/g2_import_installer.php +++ b/modules/g2_import/helpers/g2_import_installer.php @@ -31,7 +31,6 @@ class g2_import_installer { KEY `g2_id` (`g2_id`)) DEFAULT CHARSET=utf8;"); - module::set_version("g2_import", 2); mkdir(VARPATH . "modules/g2_import"); } diff --git a/modules/gallery/helpers/gallery_installer.php b/modules/gallery/helpers/gallery_installer.php index d4c4de14..7f10cdee 100644 --- a/modules/gallery/helpers/gallery_installer.php +++ b/modules/gallery/helpers/gallery_installer.php @@ -315,8 +315,6 @@ class gallery_installer { module::set_var("gallery", "timezone", null); module::set_var("gallery", "lock_timeout", 1); module::set_var("gallery", "movie_extract_frame_time", 3); - - module::set_version("gallery", 55); } static function upgrade($version) { diff --git a/modules/gallery/helpers/module.php b/modules/gallery/helpers/module.php index f4ab5571..df258e87 100644 --- a/modules/gallery/helpers/module.php +++ b/modules/gallery/helpers/module.php @@ -175,9 +175,8 @@ class module_Core { $installer_class = "{$module_name}_installer"; if (method_exists($installer_class, "install")) { call_user_func_array(array($installer_class, "install"), array()); - } else { - module::set_version($module_name, module::available()->$module_name->code_version); } + module::set_version($module_name, module::available()->$module_name->code_version); // Set the weight of the new module, which controls the order in which the modules are // loaded. By default, new modules are installed at the end of the priority list. Since the diff --git a/modules/image_block/helpers/image_block_installer.php b/modules/image_block/helpers/image_block_installer.php index 8558fe51..b177b971 100644 --- a/modules/image_block/helpers/image_block_installer.php +++ b/modules/image_block/helpers/image_block_installer.php @@ -21,7 +21,6 @@ class image_block_installer { static function install() { module::set_var("image_block", "image_count", "1"); - module::set_version("image_block", $version = 3); } static function upgrade($version) { diff --git a/modules/info/helpers/info_installer.php b/modules/info/helpers/info_installer.php index 560af15c..43c216dc 100644 --- a/modules/info/helpers/info_installer.php +++ b/modules/info/helpers/info_installer.php @@ -25,7 +25,6 @@ class info_installer { module::set_var("info", "show_owner", 1); module::set_var("info", "show_name", 1); module::set_var("info", "show_captured", 1); - module::set_version("info", 2); } static function upgrade($version) { diff --git a/modules/notification/helpers/notification_installer.php b/modules/notification/helpers/notification_installer.php index 58435641..f6b05c18 100644 --- a/modules/notification/helpers/notification_installer.php +++ b/modules/notification/helpers/notification_installer.php @@ -36,8 +36,6 @@ class notification_installer { `text` text, PRIMARY KEY (`id`)) DEFAULT CHARSET=utf8;"); - - module::set_version("notification", 2); } static function upgrade($version) { diff --git a/modules/rest/helpers/rest_installer.php b/modules/rest/helpers/rest_installer.php index df7484fe..96f8acfa 100644 --- a/modules/rest/helpers/rest_installer.php +++ b/modules/rest/helpers/rest_installer.php @@ -29,7 +29,6 @@ class rest_installer { UNIQUE KEY(`user_id`)) DEFAULT CHARSET=utf8;"); module::set_var("rest", "allow_guest_access", false); - module::set_version("rest", 3); } static function upgrade($version) { diff --git a/modules/search/helpers/search_installer.php b/modules/search/helpers/search_installer.php index 78dbce38..c9e8f26c 100644 --- a/modules/search/helpers/search_installer.php +++ b/modules/search/helpers/search_installer.php @@ -30,7 +30,6 @@ class search_installer { FULLTEXT INDEX (`data`)) ENGINE=MyISAM DEFAULT CHARSET=utf8;"); - module::set_version("search", 1); } static function activate() { diff --git a/modules/server_add/helpers/server_add_installer.php b/modules/server_add/helpers/server_add_installer.php index e843fc79..b62bbcfa 100644 --- a/modules/server_add/helpers/server_add_installer.php +++ b/modules/server_add/helpers/server_add_installer.php @@ -30,7 +30,6 @@ class server_add_installer { `task_id` int(9) NOT NULL, PRIMARY KEY (`id`)) DEFAULT CHARSET=utf8;"); - module::set_version("server_add", 4); server_add::check_config(); } diff --git a/modules/slideshow/helpers/slideshow_installer.php b/modules/slideshow/helpers/slideshow_installer.php index d283487d..22bd9534 100644 --- a/modules/slideshow/helpers/slideshow_installer.php +++ b/modules/slideshow/helpers/slideshow_installer.php @@ -20,7 +20,6 @@ class slideshow_installer { static function install() { module::set_var("slideshow", "max_scale", 0); - module::set_version("slideshow", 2); } static function upgrade($version) { diff --git a/modules/tag/helpers/tag_installer.php b/modules/tag/helpers/tag_installer.php index f80a9de3..1fd18f3e 100644 --- a/modules/tag/helpers/tag_installer.php +++ b/modules/tag/helpers/tag_installer.php @@ -37,7 +37,6 @@ class tag_installer { KEY(`item_id`, `id`)) DEFAULT CHARSET=utf8;"); module::set_var("tag", "tag_cloud_size", 30); - module::set_version("tag", 3); } static function upgrade($version) { diff --git a/modules/user/helpers/user_installer.php b/modules/user/helpers/user_installer.php index e28af69a..67f6a3d5 100644 --- a/modules/user/helpers/user_installer.php +++ b/modules/user/helpers/user_installer.php @@ -138,6 +138,5 @@ class user_installer { access::allow($registered, "view_full", $root); module::set_var("user", "minimum_password_length", 5); - module::set_version("user", 4); } } \ No newline at end of file diff --git a/modules/watermark/helpers/watermark_installer.php b/modules/watermark/helpers/watermark_installer.php index 5df780a1..13338912 100644 --- a/modules/watermark/helpers/watermark_installer.php +++ b/modules/watermark/helpers/watermark_installer.php @@ -33,7 +33,6 @@ class watermark_installer { DEFAULT CHARSET=utf8;"); @mkdir(VARPATH . "modules/watermark"); - module::set_version("watermark", 2); } static function uninstall() { -- cgit v1.2.3 From 0a2670a19ab121fe6970f2fcdf1864cb452a76c1 Mon Sep 17 00:00:00 2001 From: shadlaws Date: Tue, 12 Feb 2013 00:30:30 +0100 Subject: #1988 - Add movie_allow_uploads option ("always", "never", or "autodetect"). - gallery_installer, module.info, install.sql - add movie_allow_uploads variable - movie::allow_uploads (new) - return true if movie_allow_uploads is "always" or "autodetect" and FFmpeg found, false otherwise - legal_file - use movie::allow_uploads instead of movie::find_ffmpeg - Form_Uploadify - use movie::allow_uploads instead of movie::find_ffmpeg --- installer/install.sql | 5 +++-- modules/gallery/helpers/gallery_installer.php | 8 ++++++++ modules/gallery/helpers/legal_file.php | 4 ++-- modules/gallery/helpers/movie.php | 25 +++++++++++++++++++++++++ modules/gallery/libraries/Form_Uploadify.php | 2 +- modules/gallery/module.info | 2 +- 6 files changed, 40 insertions(+), 6 deletions(-) (limited to 'modules/gallery/helpers/gallery_installer.php') diff --git a/installer/install.sql b/installer/install.sql index 4097d51e..b89d6b9b 100644 --- a/installer/install.sql +++ b/installer/install.sql @@ -245,7 +245,7 @@ CREATE TABLE {modules} ( KEY `weight` (`weight`) ) AUTO_INCREMENT=10 DEFAULT CHARSET=utf8; /*!40101 SET character_set_client = @saved_cs_client */; -INSERT INTO {modules} VALUES (1,1,'gallery',55,1); +INSERT INTO {modules} VALUES (1,1,'gallery',56,1); INSERT INTO {modules} VALUES (2,1,'user',4,2); INSERT INTO {modules} VALUES (3,1,'comment',7,3); INSERT INTO {modules} VALUES (4,1,'organize',4,4); @@ -383,7 +383,7 @@ CREATE TABLE {vars} ( `value` text, PRIMARY KEY (`id`), UNIQUE KEY `module_name` (`module_name`,`name`) -) AUTO_INCREMENT=46 DEFAULT CHARSET=utf8; +) AUTO_INCREMENT=47 DEFAULT CHARSET=utf8; /*!40101 SET character_set_client = @saved_cs_client */; INSERT INTO {vars} VALUES (NULL,'gallery','active_site_theme','wind'); INSERT INTO {vars} VALUES (NULL,'gallery','active_admin_theme','admin_wind'); @@ -417,6 +417,7 @@ INSERT INTO {vars} VALUES (NULL,'gallery','extra_binary_paths','/usr/local/bin:/ INSERT INTO {vars} VALUES (NULL,'gallery','timezone',NULL); INSERT INTO {vars} VALUES (NULL,'gallery','lock_timeout','1'); INSERT INTO {vars} VALUES (NULL,'gallery','movie_extract_frame_time','3'); +INSERT INTO {vars} VALUES (NULL,'gallery','movie_allow_uploads','autodetect'); INSERT INTO {vars} VALUES (NULL,'gallery','blocks_site_sidebar','a:4:{i:10;a:2:{i:0;s:7:\"gallery\";i:1;s:8:\"language\";}i:11;a:2:{i:0;s:4:\"info\";i:1;s:8:\"metadata\";}i:12;a:2:{i:0;s:3:\"rss\";i:1;s:9:\"rss_feeds\";}i:13;a:2:{i:0;s:3:\"tag\";i:1;s:3:\"tag\";}}'); INSERT INTO {vars} VALUES (NULL,'gallery','identity_provider','user'); INSERT INTO {vars} VALUES (NULL,'user','minimum_password_length','5'); diff --git a/modules/gallery/helpers/gallery_installer.php b/modules/gallery/helpers/gallery_installer.php index 7f10cdee..051a66cf 100644 --- a/modules/gallery/helpers/gallery_installer.php +++ b/modules/gallery/helpers/gallery_installer.php @@ -315,6 +315,7 @@ class gallery_installer { module::set_var("gallery", "timezone", null); module::set_var("gallery", "lock_timeout", 1); module::set_var("gallery", "movie_extract_frame_time", 3); + module::set_var("gallery", "movie_allow_uploads", "autodetect"); } static function upgrade($version) { @@ -789,6 +790,13 @@ class gallery_installer { module::set_version("gallery", $version = 55); } + if ($version == 55) { + // In v56, we added the ability to change the default behavior regarding movie uploads. It + // can be set to "always", "never", or "autodetect" to match the previous behavior where they + // are allowed only if FFmpeg is found. + module::set_var("gallery", "movie_allow_uploads", "autodetect"); + module::set_version("gallery", $version = 56); + } } static function uninstall() { diff --git a/modules/gallery/helpers/legal_file.php b/modules/gallery/helpers/legal_file.php index ef588ceb..debd1e6d 100644 --- a/modules/gallery/helpers/legal_file.php +++ b/modules/gallery/helpers/legal_file.php @@ -98,7 +98,7 @@ class legal_file_Core { */ static function get_types_by_extension($extension=null) { $types_by_extension = legal_file::get_photo_types_by_extension(); - if (movie::find_ffmpeg()) { + if (movie::allow_uploads()) { $types_by_extension = array_merge($types_by_extension, legal_file::get_movie_types_by_extension()); } @@ -165,7 +165,7 @@ class legal_file_Core { */ static function get_extensions($extension=null) { $extensions = legal_file::get_photo_extensions(); - if (movie::find_ffmpeg()) { + if (movie::allow_uploads()) { $extensions = array_merge($extensions, legal_file::get_movie_extensions()); } if ($extension) { diff --git a/modules/gallery/helpers/movie.php b/modules/gallery/helpers/movie.php index d4b907a2..eda478c7 100644 --- a/modules/gallery/helpers/movie.php +++ b/modules/gallery/helpers/movie.php @@ -24,6 +24,8 @@ * Note: by design, this class does not do any permission checking. */ class movie_Core { + private static $allow_uploads; + static function get_edit_form($movie) { $form = new Forge("movies/update/$movie->id", "", "post", array("id" => "g-edit-movie-form")); $form->hidden("from_id")->value($movie->id); @@ -109,6 +111,29 @@ class movie_Core { } } + /** + * Return true if movie uploads are allowed, false if not. This is based on the + * "movie_allow_uploads" Gallery variable as well as whether or not ffmpeg is found. + */ + static function allow_uploads() { + if (empty(self::$allow_uploads)) { + // Refresh ffmpeg settings + $ffmpeg = movie::find_ffmpeg(); + switch (module::get_var("gallery", "movie_allow_uploads", "autodetect")) { + case "always": + self::$allow_uploads = true; + break; + case "never": + self::$allow_uploads = false; + break; + default: + self::$allow_uploads = !empty($ffmpeg); + break; + } + } + return self::$allow_uploads; + } + /** * Return the path to the ffmpeg binary if one exists and is executable, or null. */ diff --git a/modules/gallery/libraries/Form_Uploadify.php b/modules/gallery/libraries/Form_Uploadify.php index 56793c69..1e58018d 100644 --- a/modules/gallery/libraries/Form_Uploadify.php +++ b/modules/gallery/libraries/Form_Uploadify.php @@ -46,7 +46,7 @@ class Form_Uploadify_Core extends Form_Input { $v->album = $this->data["album"]; $v->script_data = $this->data["script_data"]; $v->simultaneous_upload_limit = module::get_var("gallery", "simultaneous_upload_limit"); - $v->movies_allowed = (bool) movie::find_ffmpeg(); + $v->movies_allowed = movie::allow_uploads(); $v->extensions = legal_file::get_filters(); $v->suhosin_session_encrypt = (bool) ini_get("suhosin.session.encrypt"); diff --git a/modules/gallery/module.info b/modules/gallery/module.info index d79a5077..2383ec3c 100644 --- a/modules/gallery/module.info +++ b/modules/gallery/module.info @@ -1,6 +1,6 @@ name = "Gallery 3" description = "Gallery core application" -version = 55 +version = 56 author_name = "Gallery Team" author_url = "http://codex.galleryproject.org/Gallery:Team" info_url = "http://codex.galleryproject.org/Gallery3:Modules:gallery" -- cgit v1.2.3