From b3e328c9ff4c3e19df4b6d18da947b759fe0c201 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Thu, 14 Jan 2010 21:04:09 -0800 Subject: Begin the process of converting to model based validation. Right now only Albums_Controller::update() supports the pattern. All form and controller based validation happening when editing an album has been moved over. Model based validation means that our REST controllers share the same validation as web controllers. We'll have consistency enforced at the model level, which is a Good Thing. The basic pattern is now: 1) Rules are in the model 2) ORM::validate() (which is called by ORM::save() but you can call it directly, too) checks the model for all the rules and throws an ORM_Validation_Exception if there are failures 3) Actions are no longer taken when you call Item_Model::__set(). Instead, they're all queued up and executed when you call Item_Model::save(). Notes: - item::validate_xxx() functions are now in Item_Model:: - We still call $form->validate() because the form can have rules (and forms triggered by events will likely continue to have rules. --- modules/gallery/controllers/albums.php | 51 ++++++++++++---------------------- 1 file changed, 17 insertions(+), 34 deletions(-) (limited to 'modules/gallery/controllers') diff --git a/modules/gallery/controllers/albums.php b/modules/gallery/controllers/albums.php index 2eeefdf1..8ad3ff72 100644 --- a/modules/gallery/controllers/albums.php +++ b/modules/gallery/controllers/albums.php @@ -129,42 +129,27 @@ class Albums_Controller extends Items_Controller { access::required("edit", $album); $form = album::get_edit_form($album); - if ($valid = $form->validate()) { - if ($album->id != 1 && - $form->edit_item->dirname->value != $album->name || - $form->edit_item->slug->value != $album->slug) { - // Make sure that there's not a conflict - if ($row = db::build() - ->select(array("name", "slug")) - ->from("items") - ->where("parent_id", "=", $album->parent_id) - ->where("id", "<>", $album->id) - ->and_open() - ->where("name", "=", $form->edit_item->dirname->value) - ->or_where("slug", "=", $form->edit_item->slug->value) - ->close() - ->execute() - ->current()) { - if ($row->name == $form->edit_item->dirname->value) { - $form->edit_item->dirname->add_error("name_conflict", 1); - } - if ($row->slug == $form->edit_item->slug->value) { - $form->edit_item->slug->add_error("slug_conflict", 1); - } - $valid = false; - } - } - } - - if ($valid) { + try { + $valid = $form->validate(); $album->title = $form->edit_item->title->value; $album->description = $form->edit_item->description->value; $album->sort_column = $form->edit_item->sort_order->column->value; $album->sort_order = $form->edit_item->sort_order->direction->value; - if ($album->id != 1) { - $album->rename($form->edit_item->dirname->value); - } + $album->name = $form->edit_item->dirname->value; $album->slug = $form->edit_item->slug->value; + $album->validate(); + } catch (ORM_Validation_Exception $e) { + // Translate ORM validation errors into form error messages + foreach ($e->validation->errors() as $key => $error) { + if ($key == "name") { + $key = "dirname"; + } + $form->edit_item->inputs[$key]->add_error($error, 1); + } + $valid = false; + } + + if ($valid) { $album->save(); module::event("item_edit_form_completed", $album, $form); @@ -180,9 +165,7 @@ class Albums_Controller extends Items_Controller { print json_encode(array("result" => "success")); } } else { - print json_encode( - array("result" => "error", - "form" => $form->__toString())); + print json_encode(array("result" => "error", "form" => (string) $form)); } } -- cgit v1.2.3 From 5809949ae8ff87cd5acf56c528e6dc2af6619513 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Fri, 15 Jan 2010 11:28:05 -0800 Subject: Don't use Input directly to get album names, etc. Use the form fields. --- modules/gallery/controllers/albums.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'modules/gallery/controllers') diff --git a/modules/gallery/controllers/albums.php b/modules/gallery/controllers/albums.php index 8ad3ff72..9f8c16ea 100644 --- a/modules/gallery/controllers/albums.php +++ b/modules/gallery/controllers/albums.php @@ -95,16 +95,16 @@ class Albums_Controller extends Items_Controller { access::required("view", $album); access::required("add", $album); - $input = Input::instance(); $form = album::get_add_form($album); if ($form->validate()) { $new_album = album::create( $album, - $input->post("name"), - $input->post("title", $input->post("name")), - $input->post("description"), + $form->add_album->inputs["name"]->value, + $form->add_album->title->value ? + $form->add_album->title->value : $form->add_album->inputs["name"]->value, + $form->add_album->description->value, identity::active_user()->id, - $input->post("slug")); + $form->add_album->slug->value); log::success("content", "Created an album", html::anchor("albums/$new_album->id", "view album")); -- cgit v1.2.3 From 50e3cc5837df7b0ae8e2d43a3dacee7500ba6db8 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Fri, 15 Jan 2010 12:15:20 -0800 Subject: Move model business logic out of album::create() and into Item_Model::save(). This makes creating albums similar to editing them and makes it difficult to create an album poorly. I expect to be able to remove a lot of code from the photo and movie helper because it's duplicated here. In order to do this, I refactored ORM_MPTT::add_to_parent() into ORM_MPTT::save() so we now add it to the parent when we do save. This allows us to call save() only once which saves a database call per add. The Albums_Controller logic is roughly the same as before. Haven't updated the tests yet, they're going to fail miserably since many of them depend on album::create() which is now gone. --- modules/gallery/controllers/albums.php | 42 +++++++++++-------- modules/gallery/helpers/album.php | 67 ------------------------------ modules/gallery/libraries/ORM_MPTT.php | 59 +++++++++++++------------- modules/gallery/models/item.php | 75 +++++++++++++++++++++++++++++----- 4 files changed, 118 insertions(+), 125 deletions(-) (limited to 'modules/gallery/controllers') diff --git a/modules/gallery/controllers/albums.php b/modules/gallery/controllers/albums.php index 9f8c16ea..7658a913 100644 --- a/modules/gallery/controllers/albums.php +++ b/modules/gallery/controllers/albums.php @@ -96,29 +96,35 @@ class Albums_Controller extends Items_Controller { access::required("add", $album); $form = album::get_add_form($album); - if ($form->validate()) { - $new_album = album::create( - $album, - $form->add_album->inputs["name"]->value, - $form->add_album->title->value ? - $form->add_album->title->value : $form->add_album->inputs["name"]->value, - $form->add_album->description->value, - identity::active_user()->id, - $form->add_album->slug->value); + try { + $valid = $form->validate(); + $album = ORM::factory("item"); + $album->type = "album"; + $album->parent_id = $parent_id; + $album->name = $form->add_album->inputs["name"]->value; + $album->title = $form->add_album->title->value ? + $form->add_album->title->value : $form->add_album->inputs["name"]->value; + $album->description = $form->add_album->description->value; + $album->slug = $form->add_album->slug->value; + $album->validate(); + } catch (ORM_Validation_Exception $e) { + // Translate ORM validation errors into form error messages + foreach ($e->validation->errors() as $key => $error) { + $form->add_album->inputs[$key]->add_error($error, 1); + } + $valid = false; + } + if ($valid) { + $album->save(); log::success("content", "Created an album", - html::anchor("albums/$new_album->id", "view album")); + html::anchor("albums/$album->id", "view album")); message::success(t("Created album %album_title", - array("album_title" => html::purify($new_album->title)))); + array("album_title" => html::purify($album->title)))); - print json_encode( - array("result" => "success", - "location" => $new_album->url())); + print json_encode(array("result" => "success", "location" => $album->url())); } else { - print json_encode( - array( - "result" => "error", - "form" => $form->__toString())); + print json_encode(array("result" => "error", "form" => (string) $form)); } } diff --git a/modules/gallery/helpers/album.php b/modules/gallery/helpers/album.php index 52759414..e99770e9 100644 --- a/modules/gallery/helpers/album.php +++ b/modules/gallery/helpers/album.php @@ -24,71 +24,6 @@ * Note: by design, this class does not do any permission checking. */ class album_Core { - /** - * Create a new album. - * @param integer $parent_id id of parent album - * @param string $name the name of this new album (it will become the directory name on disk) - * @param integer $title the title of the new album - * @param string $description (optional) the longer description of this album - * @param string $slug (optional) the url component for this photo - * @return Item_Model - */ - static function create($parent, $name, $title, $description=null, $owner_id=null, $slug=null) { - if (!$parent->loaded() || !$parent->is_album()) { - throw new Exception("@todo INVALID_PARENT"); - } - - if (strpos($name, "/")) { - throw new Exception("@todo NAME_CANNOT_CONTAIN_SLASH"); - } - - // We don't allow trailing periods as a security measure - // ref: http://dev.kohanaphp.com/issues/684 - if (rtrim($name, ".") != $name) { - throw new Exception("@todo NAME_CANNOT_END_IN_PERIOD"); - } - - if (empty($slug)) { - $slug = item::convert_filename_to_slug($name); - } - - $album = ORM::factory("item"); - $album->type = "album"; - $album->title = $title; - $album->description = $description; - $album->name = $name; - $album->owner_id = $owner_id; - $album->thumb_dirty = 1; - $album->resize_dirty = 1; - $album->slug = $slug; - $album->sort_column = "created"; - $album->sort_order = "ASC"; - - // Randomize the name or slug if there's a conflict - // @todo Improve this. Random numbers are not user friendly - while (ORM::factory("item") - ->where("parent_id", "=", $parent->id) - ->and_open() - ->where("name", "=", $album->name) - ->or_where("slug", "=", $album->slug) - ->close() - ->find()->id) { - $rand = rand(); - $album->name = "{$name}-$rand"; - $album->slug = "{$slug}-$rand"; - } - - $album = $album->add_to_parent($parent); - mkdir($album->file_path()); - mkdir(dirname($album->thumb_path())); - mkdir(dirname($album->resize_path())); - - // @todo: publish this from inside Item_Model::save() when we refactor to the point where - // there's only one save() happening here. - module::event("item_created", $album); - - return $album; - } static function get_add_form($parent) { $form = new Forge("albums/create/{$parent->id}", "", "post", array("id" => "g-add-album-form")); @@ -97,10 +32,8 @@ class album_Core { $group->input("title")->label(t("Title")); $group->textarea("description")->label(t("Description")); $group->input("name")->label(t("Directory name")) - ->callback("item::validate_no_slashes") ->error_messages("no_slashes", t("The directory name can't contain the \"/\" character")); $group->input("slug")->label(t("Internet Address")) - ->callback("item::validate_url_safe") ->error_messages( "not_url_safe", t("The internet address should contain only letters, numbers, hyphens and underscores")); diff --git a/modules/gallery/libraries/ORM_MPTT.php b/modules/gallery/libraries/ORM_MPTT.php index ed77cac9..46ae0af8 100644 --- a/modules/gallery/libraries/ORM_MPTT.php +++ b/modules/gallery/libraries/ORM_MPTT.php @@ -40,44 +40,43 @@ class ORM_MPTT_Core extends ORM { } /** - * Add this node as a child of the parent provided. + * Overload ORM::save() to update the MPTT tree when we add new items to the hierarchy. * * @chainable - * @param integer $parent_id the id of the parent node - * @return ORM + * @return ORM */ - function add_to_parent($parent) { - $this->lock(); - $parent->reload(); // Assume that the prior lock holder may have changed the parent + function save() { + if (!$this->loaded()) { + $this->lock(); + $parent = ORM::factory("item")->where("id", "=", $this->parent_id)->find(); - try { - // Make a hole in the parent for this new item - $this->db_builder - ->update($this->table_name) - ->set("left_ptr", new Database_Expression("`left_ptr` + 2")) - ->where("left_ptr", ">=", $parent->right_ptr) - ->execute(); - $this->db_builder - ->update($this->table_name) - ->set("right_ptr", new Database_Expression("`right_ptr` + 2")) - ->where("right_ptr", ">=", $parent->right_ptr) - ->execute(); - $parent->right_ptr += 2; + try { + // Make a hole in the parent for this new item + $this->db_builder + ->update($this->table_name) + ->set("left_ptr", new Database_Expression("`left_ptr` + 2")) + ->where("left_ptr", ">=", $parent->right_ptr) + ->execute(); + $this->db_builder + ->update($this->table_name) + ->set("right_ptr", new Database_Expression("`right_ptr` + 2")) + ->where("right_ptr", ">=", $parent->right_ptr) + ->execute(); + $parent->right_ptr += 2; - // Insert this item into the hole - $this->left_ptr = $parent->right_ptr - 2; - $this->right_ptr = $parent->right_ptr - 1; - $this->parent_id = $parent->id; - $this->level = $parent->level + 1; - $this->save(); - $parent->reload(); - } catch (Exception $e) { + // Insert this item into the hole + $this->left_ptr = $parent->right_ptr - 2; + $this->right_ptr = $parent->right_ptr - 1; + $this->parent_id = $parent->id; + $this->level = $parent->level + 1; + } catch (Exception $e) { + $this->unlock(); + throw $e; + } $this->unlock(); - throw $e; } - $this->unlock(); - return $this; + return parent::save(); } /** diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php index 33b36ff1..e929f30d 100644 --- a/modules/gallery/models/item.php +++ b/modules/gallery/models/item.php @@ -25,7 +25,8 @@ class Item_Model extends ORM_MPTT { "name" => array("rules" => array("length[0,255]", "required")), "title" => array("rules" => array("length[0,255]", "required")), "slug" => array("rules" => array("length[0,255]", "required")), - "description" => array("rules" => array("length[0,65535]")) + "description" => array("rules" => array("length[0,65535]")), + "parent_id" => array("rules" => array("Item_Model::valid_parent")) ); /** @@ -355,7 +356,10 @@ class Item_Model extends ORM_MPTT { } /** + * Handle any business logic necessary to create an item. * @see ORM::save() + * + * @return ORM Item_Model */ public function save() { $significant_changes = $this->changed; @@ -366,12 +370,55 @@ class Item_Model extends ORM_MPTT { if (!empty($this->changed) && $significant_changes) { $this->updated = time(); if (!$this->loaded()) { + // Create a new item. Use whatever fields are set, and specify defaults for the rest. $this->created = $this->updated; $this->weight = item::get_max_weight(); $this->rand_key = ((float)mt_rand()) / (float)mt_getrandmax(); - } else { - $send_event = 1; + $this->thumb_dirty = 1; + $this->resize_dirty = 1; + if (empty($this->sort_column)) { + $this->sort_column = "created"; + } + if (empty($this->sort_order)) { + $this->sort_order = "ASC"; + } + if (empty($this->owner_id)) { + $this->owner_id = identity::active_user()->id; + } + if (empty($this->slug)) { + $tmp = pathinfo($this->name, PATHINFO_FILENAME); + $tmp = preg_replace("/[^A-Za-z0-9-_]+/", "-", $tmp); + $this->slug = trim($tmp, "-"); + } + + // Randomize the name or slug if there's a conflict + // @todo Improve this. Random numbers are not user friendly + $base_name = $this->name; + $base_slug = $this->slug; + while (ORM::factory("item") + ->where("parent_id", "=", $this->parent_id) + ->and_open() + ->where("name", "=", $this->name) + ->or_where("slug", "=", $this->slug) + ->close() + ->find()->id) { + $rand = rand(); + $this->name = "$base_name-$rand"; + $this->slug = "$base_slug-$rand"; + } + + parent::save(); + // Call this after we finish saving so that the paths are correct. + if ($this->is_album()) { + mkdir($this->file_path()); + mkdir(dirname($this->thumb_path())); + mkdir(dirname($this->resize_path())); + } + + module::event("item_created", $this); + } else { + // Update an existing item if ($this->original()->name != $this->name) { $this->rename($this->name); $this->relative_path_cache = null; @@ -394,14 +441,11 @@ class Item_Model extends ORM_MPTT { ->where("right_ptr", "<", $this->right_ptr) ->execute(); } + parent::save(); + module::event("item_updated", $this->original(), $this); } } - parent::save(); - - if (isset($send_event)) { - module::event("item_updated", $this->original(), $this); - } return $this; } @@ -663,7 +707,7 @@ class Item_Model extends ORM_MPTT { public function valid_slug(Validation $v, $value) { if (preg_match("/[^A-Za-z0-9-_]/", $value)) { $v->add_error("slug", "not_url_safe"); - } else if ($row = db::build() + } else if (db::build() ->from("items") ->where("parent_id", "=", $this->parent_id) ->where("id", "<>", $this->id) @@ -682,7 +726,7 @@ class Item_Model extends ORM_MPTT { $v->add_error("name", "no_slashes"); } else if (rtrim($value, ".") !== $value) { $v->add_error("name", "no_trailing_period"); - } else if ($row = db::build() + } else if (db::build() ->from("items") ->where("parent_id", "=", $this->parent_id) ->where("id", "<>", $this->id) @@ -691,4 +735,15 @@ class Item_Model extends ORM_MPTT { $v->add_error("name", "conflict"); } } + + /** + * Make sure that the parent id refers to an album. + */ + static function valid_parent($value) { + return db::build() + ->from("items") + ->where("id", "=", $value) + ->where("type", "=", "album") + ->count_records() == 1; + } } -- cgit v1.2.3 From bf085a1a176f32546f86988049e0c3f809842ce7 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Sat, 16 Jan 2010 00:51:31 -0800 Subject: Convert photo uploading over to the new model based validation approach. - Rearrange Simple_Uploader_Controller::add_photo() to validate the form early in the process, and switch to using model based validation. - Move thumbnail generation into gallery_event::item_created() so that it's decoupled from the model. - Delete photo::create() and move all of its logic into Item_Model::save(). - Add Item_Model::$data_file to track the data file associated with new movies and photos. - Do some cleanup on the validation callbacks -- it turns out the 2nd argument is the field name not the value. --- modules/gallery/controllers/simple_uploader.php | 40 ++++--- modules/gallery/helpers/gallery_event.php | 16 +++ modules/gallery/helpers/photo.php | 112 ------------------- modules/gallery/models/item.php | 139 +++++++++++++++++++----- 4 files changed, 151 insertions(+), 156 deletions(-) (limited to 'modules/gallery/controllers') diff --git a/modules/gallery/controllers/simple_uploader.php b/modules/gallery/controllers/simple_uploader.php index 5d32e35f..7a7e7557 100644 --- a/modules/gallery/controllers/simple_uploader.php +++ b/modules/gallery/controllers/simple_uploader.php @@ -40,39 +40,45 @@ class Simple_Uploader_Controller extends Controller { access::required("add", $album); access::verify_csrf(); + // The Flash uploader not call /start directly, so simulate it here for now. + if (!batch::in_progress()) { + batch::start(); + } + + $form = $this->_get_add_form($album); + + // Uploadify adds its own field to the form, so validate that separately. $file_validation = new Validation($_FILES); $file_validation->add_rules( "Filedata", "upload::valid", "upload::required", "upload::type[gif,jpg,jpeg,png,flv,mp4]"); - if ($file_validation->validate()) { - // SimpleUploader.swf does not yet call /start directly, so simulate it here for now. - if (!batch::in_progress()) { - batch::start(); - } + if ($form->validate() && $file_validation->validate()) { $temp_filename = upload::save("Filedata"); try { - $name = substr(basename($temp_filename), 10); // Skip unique identifier Kohana adds - $title = item::convert_filename_to_title($name); + $item = ORM::factory("item"); + $item->name = substr(basename($temp_filename), 10); // Skip unique identifier Kohana adds + $item->title = item::convert_filename_to_title($item->name); + $item->parent_id = $album->id; + $item->set_data_file($temp_filename); + $path_info = @pathinfo($temp_filename); if (array_key_exists("extension", $path_info) && in_array(strtolower($path_info["extension"]), array("flv", "mp4"))) { - $item = movie::create($album, $temp_filename, $name, $title); + $item->type = "movie"; + $item->save(); log::success("content", t("Added a movie"), html::anchor("movies/$item->id", t("view movie"))); } else { - $item = photo::create($album, $temp_filename, $name, $title); + $item->type = "photo"; + $item->save(); log::success("content", t("Added a photo"), html::anchor("photos/$item->id", t("view photo"))); } - // We currently have no way of showing errors if validation fails, so only call our event - // handlers if validation passes. - $form = $this->_get_add_form($album); - if ($form->validate()) { - module::event("add_photos_form_completed", $item, $form); - } + module::event("add_photos_form_completed", $item, $form); } catch (Exception $e) { - Kohana_Log::add("alert", $e->__toString()); + // The Flash uploader has no good way of reporting complex errors, so just keep it simple. + Kohana_Log::add("error", $e->getMessage() . "\n" . $e->getTraceAsString()); if (file_exists($temp_filename)) { unlink($temp_filename); } @@ -84,7 +90,7 @@ class Simple_Uploader_Controller extends Controller { print "FILEID: $item->id"; } else { header("HTTP/1.1 400 Bad Request"); - print "ERROR: " . t("Invalid Upload"); + print "ERROR: " . t("Invalid upload"); } } diff --git a/modules/gallery/helpers/gallery_event.php b/modules/gallery/helpers/gallery_event.php index 679d65c2..9452e855 100644 --- a/modules/gallery/helpers/gallery_event.php +++ b/modules/gallery/helpers/gallery_event.php @@ -73,6 +73,22 @@ class gallery_event_Core { static function item_created($item) { access::add_item($item); + + if ($item->is_photo() || $item->is_movie()) { + // Build our thumbnail/resizes. + try { + graphics::generate($item); + } catch (Exception $e) { + log::failure("Unable to create a thumbnail for item id {$item->id}"); + Kohana_Log::add("error", $e->getMessage() . "\n" . $e->getTraceAsString()); + } + + // If the parent has no cover item, make this it. + $parent = $item->parent(); + if (access::can("edit", $parent) && $parent->album_cover_item_id == null) { + item::make_album_cover($item); + } + } } static function item_deleted($item) { diff --git a/modules/gallery/helpers/photo.php b/modules/gallery/helpers/photo.php index aeae7f56..74e30409 100644 --- a/modules/gallery/helpers/photo.php +++ b/modules/gallery/helpers/photo.php @@ -24,118 +24,6 @@ * Note: by design, this class does not do any permission checking. */ class photo_Core { - /** - * Create a new photo. - * @param integer $parent parent album - * @param string $filename path to the photo file on disk - * @param string $name the filename to use for this photo in the album - * @param integer $title the title of the new photo - * @param string $description (optional) the longer description of this photo - * @param string $slug (optional) the url component for this photo - * @return Item_Model - */ - static function create($parent, $filename, $name, $title, - $description=null, $owner_id=null, $slug=null) { - if (!$parent->loaded() || !$parent->is_album()) { - throw new Exception("@todo INVALID_PARENT"); - } - - if (!is_file($filename)) { - throw new Exception("@todo MISSING_IMAGE_FILE"); - } - - if (strpos($name, "/")) { - throw new Exception("@todo NAME_CANNOT_CONTAIN_SLASH"); - } - - // We don't allow trailing periods as a security measure - // ref: http://dev.kohanaphp.com/issues/684 - if (rtrim($name, ".") != $name) { - throw new Exception("@todo NAME_CANNOT_END_IN_PERIOD"); - } - - if (filesize($filename) == 0) { - throw new Exception("@todo EMPTY_INPUT_FILE"); - } - - $image_info = getimagesize($filename); - - // Force an extension onto the name - $pi = pathinfo($filename); - if (empty($pi["extension"])) { - $pi["extension"] = image_type_to_extension($image_info[2], false); - $name .= "." . $pi["extension"]; - } - - if (empty($slug)) { - $slug = item::convert_filename_to_slug($name); - } - - $photo = ORM::factory("item"); - $photo->type = "photo"; - $photo->title = $title; - $photo->description = $description; - $photo->name = $name; - $photo->owner_id = $owner_id ? $owner_id : identity::active_user()->id; - $photo->width = $image_info[0]; - $photo->height = $image_info[1]; - $photo->mime_type = empty($image_info['mime']) ? "application/unknown" : $image_info['mime']; - $photo->thumb_dirty = 1; - $photo->resize_dirty = 1; - $photo->sort_column = "weight"; - $photo->slug = $slug; - - // Randomize the name or slug if there's a conflict - // @todo Improve this. Random numbers are not user friendly - while (ORM::factory("item") - ->where("parent_id", "=", $parent->id) - ->and_open() - ->where("name", "=", $photo->name) - ->or_where("slug", "=", $photo->slug) - ->close() - ->find()->id) { - $rand = rand(); - $photo->name = "{$name}.$rand.{$pi['extension']}"; - $photo->slug = "{$slug}-$rand"; - } - - // This saves the photo - $photo->add_to_parent($parent); - - /* - * If the thumb or resize already exists then rename it. We need to do this after the save - * because the resize_path and thumb_path both call relative_path which caches the - * path. Before add_to_parent the relative path will be incorrect. - */ - if (file_exists($photo->resize_path()) || - file_exists($photo->thumb_path())) { - $photo->name = $pi["filename"] . "-" . rand() . "." . $pi["extension"]; - $photo->save(); - } - - copy($filename, $photo->file_path()); - - // @todo: publish this from inside Item_Model::save() when we refactor to the point where - // there's only one save() happening here. - module::event("item_created", $photo); - - // Build our thumbnail/resizes. If we fail to build thumbnail/resize we assume that the image - // is bad in some way and discard it. - try { - graphics::generate($photo); - } catch (Exception $e) { - $photo->delete(); - throw $e; - } - - // If the parent has no cover item, make this it. - if (access::can("edit", $parent) && $parent->album_cover_item_id == null) { - item::make_album_cover($photo); - } - - return $photo; - } - static function get_edit_form($photo) { $form = new Forge("photos/update/$photo->id", "", "post", array("id" => "g-edit-photo-form")); $form->hidden("from_id"); diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php index 46b0304e..977b9771 100644 --- a/modules/gallery/models/item.php +++ b/modules/gallery/models/item.php @@ -20,13 +20,13 @@ class Item_Model extends ORM_MPTT { protected $children = 'items'; protected $sorting = array(); + protected $data_file = null; var $rules = array( "name" => array("rules" => array("length[0,255]", "required")), "title" => array("rules" => array("length[0,255]", "required")), "slug" => array("rules" => array("length[0,255]", "required")), "description" => array("rules" => array("length[0,65535]")), - "parent_id" => array("rules" => array("Item_Model::valid_parent")), "type" => array("rules" => array("Item_Model::valid_type")), ); @@ -174,6 +174,14 @@ class Item_Model extends ORM_MPTT { return $this; } + /** + * Specify the path to the data file associated with this item. To actually associate it, + * you still have to call save(). + */ + public function set_data_file($data_file) { + $this->data_file = $data_file; + } + /** * Return the server-relative url to this item, eg: * /gallery3/index.php/BobsWedding?page=2 @@ -304,7 +312,7 @@ class Item_Model extends ORM_MPTT { } $this->relative_path_cache = implode($names, "/"); $this->relative_url_cache = implode($slugs, "/"); - $this->save(); + return $this; } /** @@ -319,7 +327,7 @@ class Item_Model extends ORM_MPTT { } if (!isset($this->relative_path_cache)) { - $this->_build_relative_caches(); + $this->_build_relative_caches()->save(); } return $this->relative_path_cache; } @@ -334,7 +342,7 @@ class Item_Model extends ORM_MPTT { } if (!isset($this->relative_url_cache)) { - $this->_build_relative_caches(); + $this->_build_relative_caches()->save(); } return $this->relative_url_cache; } @@ -368,6 +376,7 @@ class Item_Model extends ORM_MPTT { unset($significant_changes["relative_url_cache"]); unset($significant_changes["relative_path_cache"]); + if (!empty($this->changed) && $significant_changes) { $this->updated = time(); if (!$this->loaded()) { @@ -386,15 +395,37 @@ class Item_Model extends ORM_MPTT { if (empty($this->owner_id)) { $this->owner_id = identity::active_user()->id; } + + // Make an url friendly slug from the name, if necessary if (empty($this->slug)) { $tmp = pathinfo($this->name, PATHINFO_FILENAME); $tmp = preg_replace("/[^A-Za-z0-9-_]+/", "-", $tmp); $this->slug = trim($tmp, "-"); } - // Randomize the name or slug if there's a conflict + if ($this->is_movie() || $this->is_photo()) { + $image_info = getimagesize($this->data_file); + + if ($this->is_photo()) { + $this->width = $image_info[0]; + $this->height = $image_info[1]; + $this->mime_type = + empty($image_info['mime']) ? "application/unknown" : $image_info['mime']; + } + + // Force an extension onto the name if necessary + $pi = pathinfo($this->data_file); + if (empty($pi["extension"])) { + $pi["extension"] = image_type_to_extension($image_info[2], false); + $this->name .= "." . $pi["extension"]; + } + + } + + // Randomize the name or slug if there's a conflict. Preserve the extension. // @todo Improve this. Random numbers are not user friendly - $base_name = $this->name; + $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) @@ -404,19 +435,46 @@ class Item_Model extends ORM_MPTT { ->close() ->find()->id) { $rand = rand(); - $this->name = "$base_name-$rand"; + if ($base_ext) { + $this->name = "$base_name-$rand.$base_ext"; + } else { + $this->name = "$base_name-$rand"; + } $this->slug = "$base_slug-$rand"; } parent::save(); - // Call this after we finish saving so that the paths are correct. - if ($this->is_album()) { + // Build our url caches and save again. If we could depend on a save happening later we + // could defer this 2nd save. + $this->_build_relative_caches(); + parent::save(); + + // Take any actions that we can only do once all our paths are set correctly after saving. + switch ($this->type) { + case "album": mkdir($this->file_path()); mkdir(dirname($this->thumb_path())); mkdir(dirname($this->resize_path())); + break; + + case "photo": + // 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"] . "-" . rand() . "." . $pi["extension"]; + parent::save(); + } + + copy($this->data_file, $this->file_path()); + break; } + // This will almost definitely trigger another save, so put it at the end so that we're + // tail recursive. module::event("item_created", $this); } else { // Update an existing item @@ -691,8 +749,8 @@ class Item_Model extends ORM_MPTT { if (!$array) { // The root item has different rules for the name and slug. if ($this->id == 1) { - $this->rules["name"]["rules"][] = "length[0]"; - $this->rules["slug"]["rules"][] = "length[0]"; + $this->rules["name"] = array("rules" => array("length[0]")); + $this->rules["slug"] = array("rules" => array("length[0]")); } // Names and slugs can't conflict @@ -700,20 +758,28 @@ class Item_Model extends ORM_MPTT { $this->rules["slug"]["callbacks"][] = array($this, "valid_slug"); } + // Movies and photos must have data files + if ($this->is_photo() || $this->is_movie() && !$this->loaded()) { + $this->rules["name"]["callbacks"][] = array($this, "valid_data_file"); + } + + // All items must have a legal parent + $this->rules["parent_id"]["callbacks"][] = array($this, "valid_parent"); + parent::validate($array); } /** * Validate that the desired slug does not conflict. */ - public function valid_slug(Validation $v, $value) { - if (preg_match("/[^A-Za-z0-9-_]/", $value)) { + public function valid_slug(Validation $v, $field) { + if (preg_match("/[^A-Za-z0-9-_]/", $this->slug)) { $v->add_error("slug", "not_url_safe"); } else if (db::build() ->from("items") ->where("parent_id", "=", $this->parent_id) ->where("id", "<>", $this->id) - ->where("slug", "=", $value) + ->where("slug", "=", $this->slug) ->count_records()) { $v->add_error("slug", "conflict"); } @@ -723,36 +789,55 @@ class Item_Model extends ORM_MPTT { * Validate the item name. It can't conflict with other names, can't contain slashes or * trailing periods. */ - public function valid_name(Validation $v, $value) { - if (strpos($value, "/") !== false) { + public function valid_name(Validation $v, $field) { + if (strpos($this->name, "/") !== false) { $v->add_error("name", "no_slashes"); - } else if (rtrim($value, ".") !== $value) { + } else if (rtrim($this->name, ".") !== $this->name) { $v->add_error("name", "no_trailing_period"); } else if (db::build() ->from("items") ->where("parent_id", "=", $this->parent_id) ->where("id", "<>", $this->id) - ->where("name", "=", $value) + ->where("name", "=", $this->name) ->count_records()) { $v->add_error("name", "conflict"); } } /** - * Make sure that the type is valid. + * Make sure that the data file is well formed (it exists and isn't empty). */ - static function valid_type($value) { - return in_array($value, array("album", "photo", "movie")); + public function valid_data_file(Validation $v, $field) { + if (!is_file($this->data_file)) { + $v->add_error("file", "bad_path"); + } else if (filesize($this->data_file) == 0) { + $v->add_error("file", "empty_file"); + } } /** * Make sure that the parent id refers to an album. */ - static function valid_parent($value) { - return db::build() - ->from("items") - ->where("id", "=", $value) - ->where("type", "=", "album") - ->count_records() == 1; + public function valid_parent(Validation $v, $field) { + if ($this->id == 1) { + if ($this->parent_id != 0) { + $v->add_error("parent_id", "invalid"); + } + } else { + if (db::build() + ->from("items") + ->where("id", "=", $this->parent_id) + ->where("type", "=", "album") + ->count_records() != 1) { + $v->add_error("parent_id", "invalid"); + } + } + } + + /** + * Make sure that the type is valid. + */ + static function valid_type($value) { + return in_array($value, array("album", "photo", "movie")); } } -- cgit v1.2.3 From 5a8449f16d3c0db8fb47acf515d319d6eb9e87f4 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Sat, 16 Jan 2010 11:12:27 -0800 Subject: Convert Photos_Controller::update() to use model based validation. --- modules/gallery/controllers/photos.php | 55 ++++++++++------------------------ modules/gallery/helpers/photo.php | 10 ++----- 2 files changed, 17 insertions(+), 48 deletions(-) (limited to 'modules/gallery/controllers') diff --git a/modules/gallery/controllers/photos.php b/modules/gallery/controllers/photos.php index 56b454ce..98f2126d 100644 --- a/modules/gallery/controllers/photos.php +++ b/modules/gallery/controllers/photos.php @@ -61,48 +61,25 @@ class Photos_Controller extends Items_Controller { access::required("edit", $photo); $form = photo::get_edit_form($photo); - $valid = $form->validate(); - - if ($valid) { - $new_ext = pathinfo($form->edit_item->filename->value, PATHINFO_EXTENSION); - $old_ext = pathinfo($photo->name, PATHINFO_EXTENSION); - if (strcasecmp($new_ext, $old_ext)) { - $form->edit_item->filename->add_error("illegal_extension", 1); - $valid = false; - } - } - - if ($valid) { - if ($form->edit_item->filename->value != $photo->name || - $form->edit_item->slug->value != $photo->slug) { - // Make sure that there's not a name or slug conflict - if ($row = db::build() - ->select(array("name", "slug")) - ->from("items") - ->where("parent_id", "=", $photo->parent_id) - ->where("id", "<>", $photo->id) - ->and_open() - ->where("name", "=", $form->edit_item->filename->value) - ->or_where("slug", "=", $form->edit_item->slug->value) - ->close() - ->execute() - ->current()) { - if ($row->name == $form->edit_item->filename->value) { - $form->edit_item->filename->add_error("name_conflict", 1); - } - if ($row->slug == $form->edit_item->slug->value) { - $form->edit_item->slug->add_error("slug_conflict", 1); - } - $valid = false; + try { + $valid = $form->validate(); + $photo->title = $form->edit_item->title->value; + $photo->description = $form->edit_item->description->value; + $photo->slug = $form->edit_item->slug->value; + $photo->name = $form->edit_item->filename->value; + $photo->validate(); + } catch (ORM_Validation_Exception $e) { + // Translate ORM validation errors into form error messages + foreach ($e->validation->errors() as $key => $error) { + if ($key == "name") { + $key = "filename"; } + $form->edit_item->inputs[$key]->add_error($error, 1); } + $valid = false; } if ($valid) { - $photo->title = $form->edit_item->title->value; - $photo->description = $form->edit_item->description->value; - $photo->slug = $form->edit_item->slug->value; - $photo->rename($form->edit_item->filename->value); $photo->save(); module::event("item_edit_form_completed", $photo, $form); @@ -118,9 +95,7 @@ class Photos_Controller extends Items_Controller { print json_encode(array("result" => "success")); } } else { - print json_encode( - array("result" => "error", - "form" => $form->__toString())); + print json_encode(array("result" => "error", "form" => (string) $form)); } } diff --git a/modules/gallery/helpers/photo.php b/modules/gallery/helpers/photo.php index 74e30409..cb94772e 100644 --- a/modules/gallery/helpers/photo.php +++ b/modules/gallery/helpers/photo.php @@ -31,18 +31,13 @@ class photo_Core { $group->input("title")->label(t("Title"))->value($photo->title); $group->textarea("description")->label(t("Description"))->value($photo->description); $group->input("filename")->label(t("Filename"))->value($photo->name) - ->rules("required") - ->error_messages( - "name_conflict", t("There is already a movie, photo or album with this name")) - ->callback("item::validate_no_slashes") + ->error_messages("conflict", t("There is already a movie, photo or album with this name")) ->error_messages("no_slashes", t("The photo name can't contain a \"/\"")) - ->callback("item::validate_no_trailing_period") ->error_messages("no_trailing_period", t("The photo name can't end in \".\"")) ->error_messages("illegal_extension", t("You cannot change the filename extension")); $group->input("slug")->label(t("Internet Address"))->value($photo->slug) - ->callback("item::validate_url_safe") ->error_messages( - "slug_conflict", t("There is already a movie, photo or album with this internet address")) + "conflict", t("There is already a movie, photo or album with this internet address")) ->error_messages( "not_url_safe", t("The internet address should contain only letters, numbers, hyphens and underscores")); @@ -51,7 +46,6 @@ class photo_Core { $group = $form->group("buttons")->label(""); $group->submit("")->value(t("Modify")); - $form->add_rules_from(ORM::factory("item")); return $form; } -- cgit v1.2.3 From efdb73cb986002806dfe3c9241f792652e4b56fa Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Sat, 16 Jan 2010 12:00:50 -0800 Subject: Make movie creation use model based validation. Move movie related logic from movie::create() into Item_Model --- modules/gallery/controllers/simple_uploader.php | 7 ++ modules/gallery/helpers/movie.php | 103 ------------------------ modules/gallery/models/item.php | 77 +++++++++++------- 3 files changed, 55 insertions(+), 132 deletions(-) (limited to 'modules/gallery/controllers') diff --git a/modules/gallery/controllers/simple_uploader.php b/modules/gallery/controllers/simple_uploader.php index 7a7e7557..16d1d241 100644 --- a/modules/gallery/controllers/simple_uploader.php +++ b/modules/gallery/controllers/simple_uploader.php @@ -79,6 +79,13 @@ class Simple_Uploader_Controller extends Controller { } catch (Exception $e) { // The Flash uploader has no good way of reporting complex errors, so just keep it simple. Kohana_Log::add("error", $e->getMessage() . "\n" . $e->getTraceAsString()); + + // Ugh. I hate to use instanceof, But this beats catching the exception separately since + // we mostly want to treat it the same way as all other exceptions + if ($e instanceof ORM_Validation_Exception) { + Kohana_Log::add("error", "Validation errors: " . print_r($e->validation->errors(), 1)); + } + if (file_exists($temp_filename)) { unlink($temp_filename); } diff --git a/modules/gallery/helpers/movie.php b/modules/gallery/helpers/movie.php index b0d24f68..0a27ac94 100644 --- a/modules/gallery/helpers/movie.php +++ b/modules/gallery/helpers/movie.php @@ -24,109 +24,6 @@ * Note: by design, this class does not do any permission checking. */ class movie_Core { - /** - * Create a new movie. - * @param integer $parent_id id of parent album - * @param string $filename path to the photo file on disk - * @param string $name the filename to use for this photo in the album - * @param integer $title the title of the new photo - * @param string $description (optional) the longer description of this photo - * @param string $slug (optional) the url component for this photo - * @return Item_Model - */ - static function create($parent, $filename, $name, $title, - $description=null, $owner_id=null, $slug=null) { - if (!$parent->loaded() || !$parent->is_album()) { - throw new Exception("@todo INVALID_PARENT"); - } - - if (!is_file($filename)) { - throw new Exception("@todo MISSING_MOVIE_FILE"); - } - - if (strpos($name, "/")) { - throw new Exception("@todo NAME_CANNOT_CONTAIN_SLASH"); - } - - // We don't allow trailing periods as a security measure - // ref: http://dev.kohanaphp.com/issues/684 - if (rtrim($name, ".") != $name) { - throw new Exception("@todo NAME_CANNOT_END_IN_PERIOD"); - } - - try { - $movie_info = movie::getmoviesize($filename); - } catch (Exception $e) { - // Assuming this is MISSING_FFMPEG for now - $movie_info = getimagesize(MODPATH . "gallery/images/missing_movie.png"); - } - - // Force an extension onto the name - $pi = pathinfo($filename); - if (empty($pi["extension"])) { - $pi["extension"] = image_type_to_extension($movie_info[2], false); - $name .= "." . $pi["extension"]; - } - - if (empty($slug)) { - $slug = item::convert_filename_to_slug($name); - } - - $movie = ORM::factory("item"); - $movie->type = "movie"; - $movie->title = $title; - $movie->description = $description; - $movie->name = $name; - $movie->owner_id = $owner_id ? $owner_id : identity::active_user()->id; - $movie->width = $movie_info[0]; - $movie->height = $movie_info[1]; - $movie->mime_type = strtolower($pi["extension"]) == "mp4" ? "video/mp4" : "video/x-flv"; - $movie->thumb_dirty = 1; - $movie->resize_dirty = 1; - $movie->sort_column = "weight"; - $movie->slug = $slug; - - // Randomize the name if there's a conflict - // @todo Improve this. Random numbers are not user friendly - while (ORM::factory("item") - ->where("parent_id", "=", $parent->id) - ->and_open() - ->where("name", "=", $movie->name) - ->or_where("slug", "=", $movie->slug) - ->close() - ->find()->id) { - $rand = rand(); - $movie->name = "{$name}.$rand.{$pi['extension']}"; - $movie->slug = "{$slug}-$rand"; - } - - // This saves the photo - $movie->add_to_parent($parent); - - // If the thumb or resize already exists then rename it - if (file_exists($movie->resize_path()) || - file_exists($movie->thumb_path())) { - $movie->name = $pi["filename"] . "-" . rand() . "." . $pi["extension"]; - $movie->save(); - } - - copy($filename, $movie->file_path()); - - // @todo: publish this from inside Item_Model::save() when we refactor to the point where - // there's only one save() happening here. - module::event("item_created", $movie); - - // Build our thumbnail - graphics::generate($movie); - - // If the parent has no cover item, make this it. - if (access::can("edit", $parent) && $parent->album_cover_item_id == null) { - item::make_album_cover($movie); - } - - return $movie; - } - static function get_edit_form($movie) { $form = new Forge("movies/update/$movie->id", "", "post", array("id" => "g-edit-movie-form")); $form->hidden("from_id"); diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php index a9607699..c007afeb 100644 --- a/modules/gallery/models/item.php +++ b/modules/gallery/models/item.php @@ -376,7 +376,6 @@ class Item_Model extends ORM_MPTT { unset($significant_changes["relative_url_cache"]); unset($significant_changes["relative_path_cache"]); - if (!empty($this->changed) && $significant_changes) { $this->updated = time(); if (!$this->loaded()) { @@ -403,23 +402,32 @@ class Item_Model extends ORM_MPTT { $this->slug = trim($tmp, "-"); } + // Get the width, height and mime type from our data file for photos and movies. if ($this->is_movie() || $this->is_photo()) { - $image_info = getimagesize($this->data_file); + $pi = pathinfo($this->data_file); if ($this->is_photo()) { + $image_info = getimagesize($this->data_file); $this->width = $image_info[0]; $this->height = $image_info[1]; - $this->mime_type = - empty($image_info['mime']) ? "application/unknown" : $image_info['mime']; - } + $this->mime_type = $image_info["mime"]; - // Force an extension onto the name if necessary - $pi = pathinfo($this->data_file); - if (empty($pi["extension"])) { - $pi["extension"] = image_type_to_extension($image_info[2], false); - $this->name .= "." . $pi["extension"]; - } + // Force an extension onto the name if necessary + if (empty($pi["extension"])) { + $pi["extension"] = image_type_to_extension($image_info[2], false); + $this->name .= "." . $pi["extension"]; + } + } else { + list ($this->width, $this->height) = movie::getmoviesize($this->data_file); + // No extension? Assume FLV. + if (empty($pi["extension"])) { + $pi["extension"] = "flv"; + $this->name .= "." . $pi["extension"]; + } + + $this->mime_type = strtolower($pi["extension"]) == "mp4" ? "video/mp4" : "video/x-flv"; + } } // Randomize the name or slug if there's a conflict. Preserve the extension. @@ -445,8 +453,9 @@ class Item_Model extends ORM_MPTT { parent::save(); - // Build our url caches and save again. If we could depend on a save happening later we - // could defer this 2nd save. + // Build our url caches, then save again. We have to do this after it's already been + // saved once because we use only information from the database to build the paths. If we + // could depend on a save happening later we could defer this 2nd save. $this->_build_relative_caches(); parent::save(); @@ -459,6 +468,7 @@ class Item_Model extends ORM_MPTT { break; 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. @@ -746,26 +756,27 @@ class Item_Model extends ORM_MPTT { * Add some custom per-instance rules. */ public function validate($array=null) { + // validate() is recursive, only modify the rules on the outermost call. if (!$array) { - // The root item has different rules for the name and slug. if ($this->id == 1) { + // Root album can't have a name or slug $this->rules["name"] = array("rules" => array("length[0]")); $this->rules["slug"] = array("rules" => array("length[0]")); + } else { + // Layer some callbacks on top of the existing rules + $this->rules["name"]["callbacks"] = array(array($this, "valid_name")); + $this->rules["slug"]["callbacks"] = array(array($this, "valid_slug")); } - // Names and slugs can't conflict - $this->rules["name"]["callbacks"][] = array($this, "valid_name"); - $this->rules["slug"]["callbacks"][] = array($this, "valid_slug"); - } + // Movies and photos must have data files + if (($this->is_photo() || $this->is_movie()) && !$this->loaded()) { + $this->rules["name"]["callbacks"][] = array($this, "valid_data_file"); + } - // Movies and photos must have data files - if (($this->is_photo() || $this->is_movie()) && !$this->loaded()) { - $this->rules["name"]["callbacks"][] = array($this, "valid_data_file"); + // All items must have a legal parent + $this->rules["parent_id"]["callbacks"] = array(array($this, "valid_parent")); } - // All items must have a legal parent - $this->rules["parent_id"]["callbacks"][] = array($this, "valid_parent"); - parent::validate($array); } @@ -801,11 +812,19 @@ class Item_Model extends ORM_MPTT { } if ($this->is_movie() || $this->is_photo()) { - $new_ext = pathinfo($this->name, PATHINFO_EXTENSION); - $old_ext = pathinfo($this->original()->name, PATHINFO_EXTENSION); - if (strcasecmp($new_ext, $old_ext)) { - $v->add_error("name", "illegal_extension"); - return; + if ($this->loaded()) { + // Existing items can't change their extension + $new_ext = pathinfo($this->name, PATHINFO_EXTENSION); + $old_ext = pathinfo($this->original()->name, PATHINFO_EXTENSION); + if (strcasecmp($new_ext, $old_ext)) { + $v->add_error("name", "illegal_extension"); + return; + } + } else { + // New items must have an extension + if (!pathinfo($this->name, PATHINFO_EXTENSION)) { + $v->add_error("name", "illegal_extension"); + } } } -- cgit v1.2.3 From 8ce11ac97062f63eb6d0c5ef261bdf9ff6727ed2 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Sat, 16 Jan 2010 12:07:36 -0800 Subject: Convert Movies_Controller::update() over to model based validation. --- modules/gallery/controllers/movies.php | 55 ++++++++++------------------------ modules/gallery/helpers/movie.php | 10 ++----- 2 files changed, 18 insertions(+), 47 deletions(-) (limited to 'modules/gallery/controllers') diff --git a/modules/gallery/controllers/movies.php b/modules/gallery/controllers/movies.php index 7a8e4d2a..0908e281 100644 --- a/modules/gallery/controllers/movies.php +++ b/modules/gallery/controllers/movies.php @@ -61,48 +61,25 @@ class Movies_Controller extends Items_Controller { access::required("edit", $movie); $form = movie::get_edit_form($movie); - $valid = $form->validate(); - - if ($valid) { - $new_ext = pathinfo($form->edit_item->filename->value, PATHINFO_EXTENSION); - $old_ext = pathinfo($movie->name, PATHINFO_EXTENSION); - if (strcasecmp($new_ext, $old_ext)) { - $form->edit_item->filename->add_error("illegal_extension", 1); - $valid = false; - } - } - - if ($valid) { - if ($form->edit_item->filename->value != $movie->name || - $form->edit_item->slug->value != $movie->slug) { - // Make sure that there's not a name or slug conflict - if ($row = db::build() - ->select(array("name", "slug")) - ->from("items") - ->where("parent_id", "=", $movie->parent_id) - ->where("id", "<>", $movie->id) - ->and_open() - ->where("name", "=", $form->edit_item->filename->value) - ->or_where("slug", "=", $form->edit_item->slug->value) - ->close() - ->execute() - ->current()) { - if ($row->name == $form->edit_item->filename->value) { - $form->edit_item->filename->add_error("name_conflict", 1); - } - if ($row->slug == $form->edit_item->slug->value) { - $form->edit_item->slug->add_error("slug_conflict", 1); - } - $valid = false; + try { + $valid = $form->validate(); + $movie->title = $form->edit_item->title->value; + $movie->description = $form->edit_item->description->value; + $movie->slug = $form->edit_item->slug->value; + $movie->name = $form->edit_item->filename->value; + $movie->validate(); + } catch (ORM_Validation_Exception $e) { + // Translate ORM validation errors into form error messages + foreach ($e->validation->errors() as $key => $error) { + if ($key == "name") { + $key = "filename"; } + $form->edit_item->inputs[$key]->add_error($error, 1); } + $valid = false; } if ($valid) { - $movie->title = $form->edit_item->title->value; - $movie->description = $form->edit_item->description->value; - $movie->slug = $form->edit_item->slug->value; - $movie->rename($form->edit_item->filename->value); $movie->save(); module::event("item_edit_form_completed", $movie, $form); @@ -118,9 +95,7 @@ class Movies_Controller extends Items_Controller { print json_encode(array("result" => "success")); } } else { - print json_encode( - array("result" => "error", - "form" => $form->__toString())); + print json_encode(array("result" => "error", "form" => (string) $form)); } } diff --git a/modules/gallery/helpers/movie.php b/modules/gallery/helpers/movie.php index 0a27ac94..b2e846d3 100644 --- a/modules/gallery/helpers/movie.php +++ b/modules/gallery/helpers/movie.php @@ -31,18 +31,14 @@ class movie_Core { $group->input("title")->label(t("Title"))->value($movie->title); $group->textarea("description")->label(t("Description"))->value($movie->description); $group->input("filename")->label(t("Filename"))->value($movie->name) - ->rules("required") ->error_messages( - "name_conflict", t("There is already a movie, photo or album with this name")) - ->callback("item::validate_no_slashes") + "conflict", t("There is already a movie, photo or album with this name")) ->error_messages("no_slashes", t("The movie name can't contain a \"/\"")) - ->callback("item::validate_no_trailing_period") ->error_messages("no_trailing_period", t("The movie name can't end in \".\"")) ->error_messages("illegal_extension", t("You cannot change the filename extension")); $group->input("slug")->label(t("Internet Address"))->value($movie->slug) - ->callback("item::validate_url_safe") ->error_messages( - "slug_conflict", t("There is already a movie, photo or album with this internet address")) + "conflict", t("There is already a movie, photo or album with this internet address")) ->error_messages( "not_url_safe", t("The internet address should contain only letters, numbers, hyphens and underscores")); @@ -51,7 +47,7 @@ class movie_Core { $group = $form->group("buttons")->label(""); $group->submit("")->value(t("Modify")); - $form->add_rules_from(ORM::factory("item")); + return $form; } -- cgit v1.2.3 From 5c49c041e740b8bb8eb6afae8563731ab858aa97 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Sat, 16 Jan 2010 22:42:02 -0800 Subject: Use "(string) $form" instead of "$form->__toString()" --- modules/gallery/controllers/login.php | 4 +--- modules/tag/controllers/admin_tags.php | 4 +--- modules/tag/controllers/tags.php | 4 +--- modules/user/controllers/admin_users.php | 6 ++---- modules/watermark/controllers/admin_watermarks.php | 12 +++--------- 5 files changed, 8 insertions(+), 22 deletions(-) (limited to 'modules/gallery/controllers') diff --git a/modules/gallery/controllers/login.php b/modules/gallery/controllers/login.php index 75ee6b9c..464db491 100644 --- a/modules/gallery/controllers/login.php +++ b/modules/gallery/controllers/login.php @@ -33,9 +33,7 @@ class Login_Controller extends Controller { print json_encode( array("result" => "success")); } else { - print json_encode( - array("result" => "error", - "form" => $form->__toString())); + print json_encode(array("result" => "error", "form" => (string) $form)); } } diff --git a/modules/tag/controllers/admin_tags.php b/modules/tag/controllers/admin_tags.php index e20b8ac8..3b605a4e 100644 --- a/modules/tag/controllers/admin_tags.php +++ b/modules/tag/controllers/admin_tags.php @@ -60,9 +60,7 @@ class Admin_Tags_Controller extends Admin_Controller { array("result" => "success", "location" => url::site("admin/tags"))); } else { - print json_encode( - array("result" => "error", - "form" => $form->__toString())); + print json_encode(array("result" => "error", "form" => (string) $form)); } } diff --git a/modules/tag/controllers/tags.php b/modules/tag/controllers/tags.php index 992c7411..e28b7a83 100644 --- a/modules/tag/controllers/tags.php +++ b/modules/tag/controllers/tags.php @@ -71,9 +71,7 @@ class Tags_Controller extends Controller { array("result" => "success", "cloud" => tag::cloud(30)->__toString())); } else { - print json_encode( - array("result" => "error", - "form" => $form->__toString())); + print json_encode(array("result" => "error", "form" => (string) $form)); } } diff --git a/modules/user/controllers/admin_users.php b/modules/user/controllers/admin_users.php index c35eba73..91468250 100644 --- a/modules/user/controllers/admin_users.php +++ b/modules/user/controllers/admin_users.php @@ -82,8 +82,7 @@ class Admin_Users_Controller extends Admin_Controller { $name = $user->name; $user->delete(); } else { - print json_encode(array("result" => "error", - "form" => $form->__toString())); + print json_encode(array("result" => "error", "form" => (string) $form)); } $message = t("Deleted user %user_name", array("user_name" => $name)); @@ -221,8 +220,7 @@ class Admin_Users_Controller extends Admin_Controller { $name = $group->name; $group->delete(); } else { - print json_encode(array("result" => "error", - "form" => $form->__toString())); + print json_encode(array("result" => "error", "form" => (string) $form)); } $message = t("Deleted group %group_name", array("group_name" => $name)); diff --git a/modules/watermark/controllers/admin_watermarks.php b/modules/watermark/controllers/admin_watermarks.php index 2a1d5f60..f535ad08 100644 --- a/modules/watermark/controllers/admin_watermarks.php +++ b/modules/watermark/controllers/admin_watermarks.php @@ -52,9 +52,7 @@ class Admin_Watermarks_Controller extends Admin_Controller { array("result" => "success", "location" => url::site("admin/watermarks"))); } else { - print json_encode( - array("result" => "error", - "form" => $form->__toString())); + print json_encode(array("result" => "error", "form" => (string) $form)); } } @@ -84,9 +82,7 @@ class Admin_Watermarks_Controller extends Admin_Controller { array("result" => "success", "location" => url::site("admin/watermarks"))); } else { - print json_encode( - array("result" => "error", - "form" => $form->__toString())); + print json_encode(array("result" => "error", "form" => (string) $form)); } } @@ -127,9 +123,7 @@ class Admin_Watermarks_Controller extends Admin_Controller { array("result" => "success", "location" => url::site("admin/watermarks"))); } else { - print json_encode( - array("result" => "error", - "form" => $form->__toString())); + print json_encode(array("result" => "error", "form" => (string) $form)); } } -- cgit v1.2.3 From 512910962d62a2011d7770da1b6e68bd6bbad983 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Tue, 19 Jan 2010 19:24:46 -0800 Subject: Change "dirname" to "name" in the edit album form. I'd rather have consistency between field names than deal with underlying issues with Forge bitching about the "name" property. --- modules/gallery/controllers/albums.php | 5 +---- modules/gallery/helpers/album.php | 4 ++-- modules/gallery/tests/Albums_Controller_Test.php | 6 +++--- 3 files changed, 6 insertions(+), 9 deletions(-) (limited to 'modules/gallery/controllers') diff --git a/modules/gallery/controllers/albums.php b/modules/gallery/controllers/albums.php index 7658a913..a378f3ee 100644 --- a/modules/gallery/controllers/albums.php +++ b/modules/gallery/controllers/albums.php @@ -141,15 +141,12 @@ class Albums_Controller extends Items_Controller { $album->description = $form->edit_item->description->value; $album->sort_column = $form->edit_item->sort_order->column->value; $album->sort_order = $form->edit_item->sort_order->direction->value; - $album->name = $form->edit_item->dirname->value; + $album->name = $form->edit_item->inputs["name"]->value; $album->slug = $form->edit_item->slug->value; $album->validate(); } catch (ORM_Validation_Exception $e) { // Translate ORM validation errors into form error messages foreach ($e->validation->errors() as $key => $error) { - if ($key == "name") { - $key = "dirname"; - } $form->edit_item->inputs[$key]->add_error($error, 1); } $valid = false; diff --git a/modules/gallery/helpers/album.php b/modules/gallery/helpers/album.php index e99770e9..55282252 100644 --- a/modules/gallery/helpers/album.php +++ b/modules/gallery/helpers/album.php @@ -52,7 +52,7 @@ class album_Core { $group->input("title")->label(t("Title"))->value($parent->title); $group->textarea("description")->label(t("Description"))->value($parent->description); if ($parent->id != 1) { - $group->input("dirname")->label(t("Directory Name"))->value($parent->name) + $group->input("name")->label(t("Directory Name"))->value($parent->name) ->rules("required") ->error_messages( "conflict", t("There is already a movie, photo or album with this name")) @@ -65,7 +65,7 @@ class album_Core { "not_url_safe", t("The internet address should contain only letters, numbers, hyphens and underscores")); } else { - $group->hidden("dirname")->value($parent->name); + $group->hidden("name")->value($parent->name); $group->hidden("slug")->value($parent->slug); } diff --git a/modules/gallery/tests/Albums_Controller_Test.php b/modules/gallery/tests/Albums_Controller_Test.php index 8ba2c7bc..26dc2571 100644 --- a/modules/gallery/tests/Albums_Controller_Test.php +++ b/modules/gallery/tests/Albums_Controller_Test.php @@ -31,9 +31,9 @@ class Albums_Controller_Test extends Unit_Test_Case { $album = test::random_album(); // Randomize to avoid conflicts. - $new_dirname = "new_name_" . rand(); + $new_name = "new_name_" . rand(); - $_POST["dirname"] = $new_dirname; + $_POST["name"] = $new_name; $_POST["title"] = "new title"; $_POST["description"] = "new description"; $_POST["column"] = "weight"; @@ -49,7 +49,7 @@ class Albums_Controller_Test extends Unit_Test_Case { ob_end_clean(); $this->assert_equal(json_encode(array("result" => "success")), $results); - $this->assert_equal($new_dirname, $album->name); + $this->assert_equal($new_name, $album->name); $this->assert_equal("new title", $album->title); $this->assert_equal("new description", $album->description); } -- cgit v1.2.3 From e02675b730fb814105e1cb9dceb0e25fdcbd3e27 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Tue, 19 Jan 2010 19:31:01 -0800 Subject: Change "filename" to "name" in the edit album form. I'd rather have consistency between field names than deal with underlying issues with Forge bitching about the "name" property. --- modules/gallery/controllers/movies.php | 5 +---- modules/gallery/controllers/photos.php | 5 +---- modules/gallery/helpers/movie.php | 2 +- modules/gallery/helpers/photo.php | 2 +- modules/gallery/tests/Photos_Controller_Test.php | 4 ++-- 5 files changed, 6 insertions(+), 12 deletions(-) (limited to 'modules/gallery/controllers') diff --git a/modules/gallery/controllers/movies.php b/modules/gallery/controllers/movies.php index 0908e281..b51282b3 100644 --- a/modules/gallery/controllers/movies.php +++ b/modules/gallery/controllers/movies.php @@ -66,14 +66,11 @@ class Movies_Controller extends Items_Controller { $movie->title = $form->edit_item->title->value; $movie->description = $form->edit_item->description->value; $movie->slug = $form->edit_item->slug->value; - $movie->name = $form->edit_item->filename->value; + $movie->name = $form->edit_item->inputs["name"]->value; $movie->validate(); } catch (ORM_Validation_Exception $e) { // Translate ORM validation errors into form error messages foreach ($e->validation->errors() as $key => $error) { - if ($key == "name") { - $key = "filename"; - } $form->edit_item->inputs[$key]->add_error($error, 1); } $valid = false; diff --git a/modules/gallery/controllers/photos.php b/modules/gallery/controllers/photos.php index 98f2126d..b5da3884 100644 --- a/modules/gallery/controllers/photos.php +++ b/modules/gallery/controllers/photos.php @@ -66,14 +66,11 @@ class Photos_Controller extends Items_Controller { $photo->title = $form->edit_item->title->value; $photo->description = $form->edit_item->description->value; $photo->slug = $form->edit_item->slug->value; - $photo->name = $form->edit_item->filename->value; + $photo->name = $form->edit_item->inputs["name"]->value; $photo->validate(); } catch (ORM_Validation_Exception $e) { // Translate ORM validation errors into form error messages foreach ($e->validation->errors() as $key => $error) { - if ($key == "name") { - $key = "filename"; - } $form->edit_item->inputs[$key]->add_error($error, 1); } $valid = false; diff --git a/modules/gallery/helpers/movie.php b/modules/gallery/helpers/movie.php index b2e846d3..b07a9e69 100644 --- a/modules/gallery/helpers/movie.php +++ b/modules/gallery/helpers/movie.php @@ -30,7 +30,7 @@ class movie_Core { $group = $form->group("edit_item")->label(t("Edit Movie")); $group->input("title")->label(t("Title"))->value($movie->title); $group->textarea("description")->label(t("Description"))->value($movie->description); - $group->input("filename")->label(t("Filename"))->value($movie->name) + $group->input("name")->label(t("Filename"))->value($movie->name) ->error_messages( "conflict", t("There is already a movie, photo or album with this name")) ->error_messages("no_slashes", t("The movie name can't contain a \"/\"")) diff --git a/modules/gallery/helpers/photo.php b/modules/gallery/helpers/photo.php index cb94772e..9bd277bc 100644 --- a/modules/gallery/helpers/photo.php +++ b/modules/gallery/helpers/photo.php @@ -30,7 +30,7 @@ class photo_Core { $group = $form->group("edit_item")->label(t("Edit Photo")); $group->input("title")->label(t("Title"))->value($photo->title); $group->textarea("description")->label(t("Description"))->value($photo->description); - $group->input("filename")->label(t("Filename"))->value($photo->name) + $group->input("name")->label(t("Filename"))->value($photo->name) ->error_messages("conflict", t("There is already a movie, photo or album with this name")) ->error_messages("no_slashes", t("The photo name can't contain a \"/\"")) ->error_messages("no_trailing_period", t("The photo name can't end in \".\"")) diff --git a/modules/gallery/tests/Photos_Controller_Test.php b/modules/gallery/tests/Photos_Controller_Test.php index 31e0bc21..f548b40d 100644 --- a/modules/gallery/tests/Photos_Controller_Test.php +++ b/modules/gallery/tests/Photos_Controller_Test.php @@ -31,7 +31,7 @@ class Photos_Controller_Test extends Unit_Test_Case { $controller = new Photos_Controller(); $photo = test::random_photo(); - $_POST["filename"] = "new name.jpg"; + $_POST["name"] = "new name.jpg"; $_POST["title"] = "new title"; $_POST["description"] = "new description"; $_POST["slug"] = "new-slug"; @@ -55,7 +55,7 @@ class Photos_Controller_Test extends Unit_Test_Case { $controller = new Photos_Controller(); $photo = test::random_photo(); - $_POST["filename"] = "new name.jpg"; + $_POST["name"] = "new name.jpg"; $_POST["title"] = "new title"; $_POST["description"] = "new description"; $_POST["slug"] = "new slug"; -- cgit v1.2.3 From cedbc82dccaf74a983f1f92846735b69391fdf10 Mon Sep 17 00:00:00 2001 From: Tim Almdal Date: Thu, 28 Jan 2010 07:44:58 -0800 Subject: Do all the html::clean|purify calls in the views and not the controller. Also clean the subject line and email message body of the contact user email. --- modules/gallery/controllers/user_profile.php | 4 ++-- modules/gallery/helpers/gallery_event.php | 2 +- modules/gallery/views/user_profile.html.php | 2 +- modules/gallery/views/user_profile_info.html.php | 2 +- modules/rest/views/user_profile_rest.html.php | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) (limited to 'modules/gallery/controllers') diff --git a/modules/gallery/controllers/user_profile.php b/modules/gallery/controllers/user_profile.php index a0e6619e..327d2ff1 100644 --- a/modules/gallery/controllers/user_profile.php +++ b/modules/gallery/controllers/user_profile.php @@ -53,11 +53,11 @@ class User_Profile_Controller extends Controller { if ($form->validate()) { Sendmail::factory() ->to($user->email) - ->subject($form->message->subject->value) + ->subject(html::clean($form->message->subject->value)) ->header("Mime-Version", "1.0") ->header("Content-type", "text/html; charset=iso-8859-1") ->reply_to($form->message->reply_to->value) - ->message($form->message->message->value) + ->message(html::purify($form->message->message->value)) ->send(); message::success(t("Sent message to %user_name", array("user_name" => $user->display_name()))); print json_encode(array("result" => "success")); diff --git a/modules/gallery/helpers/gallery_event.php b/modules/gallery/helpers/gallery_event.php index 70c6de4a..9b252f61 100644 --- a/modules/gallery/helpers/gallery_event.php +++ b/modules/gallery/helpers/gallery_event.php @@ -411,7 +411,7 @@ class gallery_event_Core { if ($field == "locale") { $value = locales::display_name($value); } - $v->fields[(string) $label] = html::clean($value); + $v->fields[(string) $label] = $value; } } $data->content[] = (object) array("title" => t("User information"), "view" => $v); diff --git a/modules/gallery/views/user_profile.html.php b/modules/gallery/views/user_profile.html.php index 708b1613..7dc9d13e 100644 --- a/modules/gallery/views/user_profile.html.php +++ b/modules/gallery/views/user_profile.html.php @@ -41,7 +41,7 @@
- +
view ?>
diff --git a/modules/gallery/views/user_profile_info.html.php b/modules/gallery/views/user_profile_info.html.php index 2a2549c8..2f2d68d3 100644 --- a/modules/gallery/views/user_profile_info.html.php +++ b/modules/gallery/views/user_profile_info.html.php @@ -3,7 +3,7 @@ $value): ?> - + diff --git a/modules/rest/views/user_profile_rest.html.php b/modules/rest/views/user_profile_rest.html.php index 3807817e..397afa89 100644 --- a/modules/rest/views/user_profile_rest.html.php +++ b/modules/rest/views/user_profile_rest.html.php @@ -2,7 +2,7 @@
  • -

    :

    +

    :

-- cgit v1.2.3 From 3ed81869cbbf021a11868f612ffc79c031df42f2 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Thu, 28 Jan 2010 20:44:10 -0800 Subject: Cast the SafeString $task->status to (string) so that it doesn't come down to the JS as an object. --- modules/gallery/controllers/admin_maintenance.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'modules/gallery/controllers') diff --git a/modules/gallery/controllers/admin_maintenance.php b/modules/gallery/controllers/admin_maintenance.php index aa4fb29f..3062ea09 100644 --- a/modules/gallery/controllers/admin_maintenance.php +++ b/modules/gallery/controllers/admin_maintenance.php @@ -215,7 +215,7 @@ class Admin_Maintenance_Controller extends Admin_Controller { print json_encode(array("result" => "success", "task" => array( "percent_complete" => $task->percent_complete, - "status" => $task->status, + "status" => (string) $task->status, "done" => (bool) $task->done), "location" => url::site("admin/maintenance"))); @@ -223,7 +223,7 @@ class Admin_Maintenance_Controller extends Admin_Controller { print json_encode(array("result" => "in_progress", "task" => array( "percent_complete" => $task->percent_complete, - "status" => $task->status, + "status" => (string) $task->status, "done" => (bool) $task->done))); } } -- cgit v1.2.3 From aacafaaf35d8cc41c9d6374412c8cf1e2544dad7 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Thu, 28 Jan 2010 23:17:32 -0800 Subject: Add @todo. --- modules/gallery/controllers/quick.php | 1 + 1 file changed, 1 insertion(+) (limited to 'modules/gallery/controllers') diff --git a/modules/gallery/controllers/quick.php b/modules/gallery/controllers/quick.php index 7f9a9826..04aab8dc 100644 --- a/modules/gallery/controllers/quick.php +++ b/modules/gallery/controllers/quick.php @@ -47,6 +47,7 @@ class Quick_Controller extends Controller { graphics::generate($item); $parent = $item->parent(); + // @todo: this is an inadequate way to regenerate the parent's thumbnail after rotation. if ($parent->album_cover_item_id == $item->id) { copy($item->thumb_path(), $parent->thumb_path()); $parent->thumb_width = $item->thumb_width; -- cgit v1.2.3 From 2bfcec9620814a6f3d0163a174d7ba90efef369d Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Sat, 30 Jan 2010 19:48:57 -0800 Subject: Prevent brute force login attacks by reducing login attempts to 1 per minute after there have been 5 consecutive failed login attempts. Fix for ticket #589. --- modules/gallery/controllers/login.php | 7 ++--- modules/gallery/helpers/auth.php | 45 ++++++++++++++++++++++++++- modules/gallery/helpers/gallery_event.php | 5 +++ modules/gallery/helpers/gallery_installer.php | 22 ++++++++++++- modules/gallery/models/failed_login.php | 20 ++++++++++++ modules/gallery/module.info | 3 +- 6 files changed, 94 insertions(+), 8 deletions(-) create mode 100644 modules/gallery/models/failed_login.php (limited to 'modules/gallery/controllers') diff --git a/modules/gallery/controllers/login.php b/modules/gallery/controllers/login.php index cfe86cfb..1426f0d8 100644 --- a/modules/gallery/controllers/login.php +++ b/modules/gallery/controllers/login.php @@ -62,11 +62,10 @@ class Login_Controller extends Controller { if ($valid) { $user = identity::lookup_user_by_name($form->login->inputs["name"]->value); if (empty($user) || !identity::is_correct_password($user, $form->login->password->value)) { - log::warning( - "user", - t("Failed login for %name", - array("name" => $form->login->inputs["name"]->value))); $form->login->inputs["name"]->add_error("invalid_login", 1); + $name = $form->login->inputs["name"]->value; + log::warning("user", t("Failed login for %name", array("name" => $name))); + module::event("user_login_failed", $name); $valid = false; } } diff --git a/modules/gallery/helpers/auth.php b/modules/gallery/helpers/auth.php index f7d4f7e8..e112f127 100644 --- a/modules/gallery/helpers/auth.php +++ b/modules/gallery/helpers/auth.php @@ -22,7 +22,10 @@ class auth_Core { $form = new Forge($url, "", "post", array("id" => "g-login-form")); $form->set_attr('class', "g-narrow"); $group = $form->group("login")->label(t("Login")); - $group->input("name")->label(t("Username"))->id("g-username")->class(null); + $group->input("name")->label(t("Username"))->id("g-username")->class(null) + ->callback("auth::validate_too_many_failed_logins") + ->error_messages( + "too_many_failed_logins", t("Too many failed login attempts. Try again later")); $group->password("password")->label(t("Password"))->id("g-password")->class(null); $group->inputs["name"]->error_messages("invalid_login", t("Invalid name or password")); $group->submit("")->value(t("Login")); @@ -55,4 +58,44 @@ class auth_Core { array("url" => user_profile::url($user->id), "user_name" => html::clean($user->name)))); } + + /** + * After there have been 5 failed login attempts, any failure leads to getting locked out for a + * minute. + */ + static function validate_too_many_failed_logins($name_input) { + $failed_login = ORM::factory("failed_login") + ->where("name", "=", $name_input->value) + ->find(); + if ($failed_login->loaded() && + $failed_login->count > 5 && + (time() - $failed_login->time < 60)) { + $name_input->add_error("too_many_failed_logins", 1); + } + } + + /** + * Record a failed login for this user + */ + static function record_failed_login($name) { + $failed_login = ORM::factory("failed_login") + ->where("name", "=", $name) + ->find(); + if (!$failed_login->loaded()) { + $failed_login->name = $name; + } + $failed_login->time = time(); + $failed_login->count++; + $failed_login->save(); + } + + /** + * Clear any failed logins for this user + */ + static function record_successful_login($user) { + db::build() + ->delete("failed_logins") + ->where("name", "=", $user->name) + ->execute(); + } } \ No newline at end of file diff --git a/modules/gallery/helpers/gallery_event.php b/modules/gallery/helpers/gallery_event.php index 80452276..6479e2c3 100644 --- a/modules/gallery/helpers/gallery_event.php +++ b/modules/gallery/helpers/gallery_event.php @@ -110,6 +110,11 @@ class gallery_event_Core { graphics::choose_default_toolkit(); module::clear_var("gallery", "choose_default_tookit"); } + auth::record_successful_login($user); + } + + static function user_login_failed($name) { + auth::record_failed_login($name); } static function item_index_data($item, $data) { diff --git a/modules/gallery/helpers/gallery_installer.php b/modules/gallery/helpers/gallery_installer.php index d2378d64..cf701ed4 100644 --- a/modules/gallery/helpers/gallery_installer.php +++ b/modules/gallery/helpers/gallery_installer.php @@ -42,6 +42,14 @@ class gallery_installer { KEY (`tags`)) DEFAULT CHARSET=utf8;"); + $db->query("CREATE TABLE {failed_logins} ( + `id` int(9) NOT NULL auto_increment, + `count` int(9) NOT NULL, + `name` varchar(255) NOT NULL, + `time` int(9) NOT NULL, + PRIMARY KEY (`id`)) + DEFAULT CHARSET=utf8;"); + $db->query("CREATE TABLE {graphics_rules} ( `id` int(9) NOT NULL auto_increment, `active` BOOLEAN default 0, @@ -276,7 +284,7 @@ class gallery_installer { // @todo this string needs to be picked up by l10n_scanner module::set_var("gallery", "credits", "Powered by Gallery %version"); module::set_var("gallery", "simultaneous_upload_limit", 5); - module::set_version("gallery", 22); + module::set_version("gallery", 23); } static function upgrade($version) { @@ -485,6 +493,17 @@ class gallery_installer { } module::set_version("gallery", $version = 23); } + + if ($version = 23) { + $db->query("CREATE TABLE {failed_logins} ( + `id` int(9) NOT NULL auto_increment, + `count` int(9) NOT NULL, + `name` varchar(255) NOT NULL, + `time` int(9) NOT NULL, + PRIMARY KEY (`id`)) + DEFAULT CHARSET=utf8;"); + module::set_version("gallery", $version = 24); + } } static function uninstall() { @@ -493,6 +512,7 @@ class gallery_installer { $db->query("DROP TABLE IF EXISTS {access_intents}"); $db->query("DROP TABLE IF EXISTS {graphics_rules}"); $db->query("DROP TABLE IF EXISTS {incoming_translations}"); + $db->query("DROP TABLE IF EXISTS {failed_logins}"); $db->query("DROP TABLE IF EXISTS {items}"); $db->query("DROP TABLE IF EXISTS {logs}"); $db->query("DROP TABLE IF EXISTS {modules}"); diff --git a/modules/gallery/models/failed_login.php b/modules/gallery/models/failed_login.php new file mode 100644 index 00000000..0b84c295 --- /dev/null +++ b/modules/gallery/models/failed_login.php @@ -0,0 +1,20 @@ + Date: Sat, 30 Jan 2010 21:16:47 -0800 Subject: Update install.sql -- gallery version jumps from 23 to 25 due to a mistake in the version 24 upgrade code. Update packager to serialize files so that we can serialize the new .htaccess files Update init_var.php to include the newly serialized .htaccess files. Fixes ticket #587. --- installer/init_var.php | 3 +++ installer/install.sql | 2 +- modules/gallery/controllers/packager.php | 9 ++++++--- 3 files changed, 10 insertions(+), 4 deletions(-) (limited to 'modules/gallery/controllers') diff --git a/installer/init_var.php b/installer/init_var.php index c010888c..353665a3 100755 --- a/installer/init_var.php +++ b/installer/init_var.php @@ -7,3 +7,6 @@ !file_exists(VARPATH . "thumbs") && mkdir(VARPATH . "thumbs"); !file_exists(VARPATH . "tmp") && mkdir(VARPATH . "tmp"); !file_exists(VARPATH . "uploads") && mkdir(VARPATH . "uploads"); +file_put_contents(VARPATH . "logs/.htaccess", base64_decode("RGlyZWN0b3J5SW5kZXggLmh0YWNjZXNzClNldEhhbmRsZXIgR2FsbGVyeV9TZWN1cml0eV9Eb19Ob3RfUmVtb3ZlCk9wdGlvbnMgTm9uZQo8SWZNb2R1bGUgbW9kX3Jld3JpdGUuYz4KUmV3cml0ZUVuZ2luZSBvZmYKPC9JZk1vZHVsZT4KT3JkZXIgYWxsb3csZGVueQpEZW55IGZyb20gYWxsCg==")); +file_put_contents(VARPATH . "tmp/.htaccess", base64_decode("RGlyZWN0b3J5SW5kZXggLmh0YWNjZXNzClNldEhhbmRsZXIgR2FsbGVyeV9TZWN1cml0eV9Eb19Ob3RfUmVtb3ZlCk9wdGlvbnMgTm9uZQo8SWZNb2R1bGUgbW9kX3Jld3JpdGUuYz4KUmV3cml0ZUVuZ2luZSBvZmYKPC9JZk1vZHVsZT4KT3JkZXIgYWxsb3csZGVueQpEZW55IGZyb20gYWxsCg==")); +file_put_contents(VARPATH . "uploads/.htaccess", base64_decode("RGlyZWN0b3J5SW5kZXggLmh0YWNjZXNzClNldEhhbmRsZXIgR2FsbGVyeV9TZWN1cml0eV9Eb19Ob3RfUmVtb3ZlCk9wdGlvbnMgTm9uZQo8SWZNb2R1bGUgbW9kX3Jld3JpdGUuYz4KUmV3cml0ZUVuZ2luZSBvZmYKPC9JZk1vZHVsZT4KT3JkZXIgYWxsb3csZGVueQpEZW55IGZyb20gYWxsCg==")); diff --git a/installer/install.sql b/installer/install.sql index e64a961e..20b632fa 100644 --- a/installer/install.sql +++ b/installer/install.sql @@ -239,7 +239,7 @@ CREATE TABLE {modules} ( UNIQUE KEY `name` (`name`) ) AUTO_INCREMENT=10 DEFAULT CHARSET=utf8; SET character_set_client = @saved_cs_client; -INSERT INTO {modules} VALUES (1,1,'gallery',23); +INSERT INTO {modules} VALUES (1,1,'gallery',25); INSERT INTO {modules} VALUES (2,1,'user',2); INSERT INTO {modules} VALUES (3,1,'comment',2); INSERT INTO {modules} VALUES (4,1,'organize',1); diff --git a/modules/gallery/controllers/packager.php b/modules/gallery/controllers/packager.php index cb64f1bf..66626483 100644 --- a/modules/gallery/controllers/packager.php +++ b/modules/gallery/controllers/packager.php @@ -164,7 +164,7 @@ class Packager_Controller extends Controller { foreach($objects as $name => $file){ if ($file->getBasename() == "database.php") { continue; - } else if (basename($file->getPath()) == "logs") { + } else if (basename($file->getPath()) == "logs" && $file->getBasename() != ".htaccess") { continue; } @@ -172,8 +172,8 @@ class Packager_Controller extends Controller { $paths[] = "VARPATH . \"" . substr($name, strlen(VARPATH)) . "\""; } else { // @todo: serialize non-directories - print "IGNORING FILE: $name\n"; - return; + $files["VARPATH . \"" . substr($name, strlen(VARPATH)) . "\""] = + base64_encode(file_get_contents($name)); } } // Sort the paths so that the var file is stable @@ -185,6 +185,9 @@ class Packager_Controller extends Controller { foreach ($paths as $path) { fwrite($fd, "!file_exists($path) && mkdir($path);\n"); } + foreach ($files as $file => $contents) { + fwrite($fd, "file_put_contents($file, base64_decode(\"$contents\"));\n"); + } fclose($fd); } } \ No newline at end of file -- cgit v1.2.3 From c6676dd455d070aaded25f048269eee07248100a Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Sun, 31 Jan 2010 15:23:37 -0800 Subject: Remove obsolete call to _force_block_adder() which has been broken for over a year. --- modules/gallery/controllers/admin_dashboard.php | 2 -- 1 file changed, 2 deletions(-) (limited to 'modules/gallery/controllers') diff --git a/modules/gallery/controllers/admin_dashboard.php b/modules/gallery/controllers/admin_dashboard.php index 5f2cb41d..42a3c081 100644 --- a/modules/gallery/controllers/admin_dashboard.php +++ b/modules/gallery/controllers/admin_dashboard.php @@ -91,7 +91,5 @@ class Admin_Dashboard_Controller extends Admin_Controller { } block_manager::set_active($location, $new_blocks); } - - $this->_force_block_adder(); } } -- cgit v1.2.3 From c050acf30a7351bf0ef5b8ee206704c073e881c7 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Sun, 31 Jan 2010 16:07:41 -0800 Subject: Fix lots of warnings that pop up when we're in E_STRICT mode. They're mostly issues around uninitialized variables, calling non-static functions in a static context, calling Session functions directly instead of on its singleton, passing non-variables by reference, and subclasses not using the same interface as the parent class. --- modules/comment/controllers/admin_comments.php | 1 + modules/comment/helpers/comment_rss.php | 1 + modules/comment/models/comment.php | 3 ++- modules/digibug/controllers/digibug.php | 2 +- modules/forge/libraries/Form_Group.php | 2 +- modules/g2_import/controllers/admin_g2_import.php | 5 +++++ modules/g2_import/helpers/g2_import.php | 11 +++++++++++ modules/g2_import/helpers/g2_import_task.php | 10 ++++++++-- modules/g2_import/views/admin_g2_import.html.php | 2 +- modules/gallery/controllers/admin_modules.php | 1 + modules/gallery/controllers/combined.php | 2 +- modules/gallery/controllers/file_proxy.php | 2 +- modules/gallery/helpers/gallery_block.php | 2 +- modules/gallery/helpers/gallery_rss.php | 1 + modules/gallery/helpers/gallery_task.php | 11 ++++++++--- modules/gallery/helpers/graphics.php | 3 +++ modules/gallery/helpers/l10n_client.php | 1 + modules/gallery/helpers/module.php | 3 ++- modules/gallery/libraries/Form_Script.php | 4 ++-- modules/gallery/libraries/MY_Database.php | 2 +- modules/gallery/libraries/MY_View.php | 2 +- modules/gallery/libraries/ORM_MPTT.php | 2 +- modules/gallery/models/item.php | 2 +- modules/gallery/models/task.php | 4 ++-- modules/gallery/tests/Database_Test.php | 8 ++++---- modules/gallery/tests/Item_Rest_Helper_Test.php | 17 +++++++++++++++++ .../gallery_unit_test/controllers/gallery_unit_test.php | 6 +++++- modules/rest/controllers/rest.php | 1 + modules/rest/helpers/rest.php | 2 +- modules/rest/tests/Rest_Controller_Test.php | 8 ++++---- modules/search/helpers/search.php | 3 ++- modules/tag/helpers/tag_rss.php | 2 ++ modules/tag/helpers/tags_rest.php | 1 - modules/tag/models/tag.php | 2 +- modules/tag/tests/Tag_Item_Rest_Helper_Test.php | 4 +++- modules/tag/tests/Tag_Rest_Helper_Test.php | 8 ++++++++ modules/tag/tests/Tags_Rest_Helper_Test.php | 4 ++++ modules/user/controllers/admin_users.php | 2 +- 38 files changed, 111 insertions(+), 36 deletions(-) (limited to 'modules/gallery/controllers') diff --git a/modules/comment/controllers/admin_comments.php b/modules/comment/controllers/admin_comments.php index b7dc5fb3..3dd45919 100644 --- a/modules/comment/controllers/admin_comments.php +++ b/modules/comment/controllers/admin_comments.php @@ -92,6 +92,7 @@ class Admin_Comments_Controller extends Admin_Controller { } private function _counts() { + $counts = new stdClass(); $counts->unpublished = 0; $counts->published = 0; $counts->spam = 0; diff --git a/modules/comment/helpers/comment_rss.php b/modules/comment/helpers/comment_rss.php index 77044884..79fa07df 100644 --- a/modules/comment/helpers/comment_rss.php +++ b/modules/comment/helpers/comment_rss.php @@ -42,6 +42,7 @@ class comment_rss_Core { $comments->where("item_id", "=", $id); } + $feed = new stdClass(); $feed->view = "comment.mrss"; $feed->children = array(); foreach ($comments->find_all($limit, $offset) as $comment) { diff --git a/modules/comment/models/comment.php b/modules/comment/models/comment.php index add15ce8..d9d05995 100644 --- a/modules/comment/models/comment.php +++ b/modules/comment/models/comment.php @@ -116,7 +116,8 @@ class Comment_Model extends ORM { // We only notify on the related items if we're making a visible change. if ($visible_change) { - module::event("item_related_update", $this->item()); + $item = $this->item(); + module::event("item_related_update", $item); } return $this; diff --git a/modules/digibug/controllers/digibug.php b/modules/digibug/controllers/digibug.php index e3b06196..c98ae20c 100644 --- a/modules/digibug/controllers/digibug.php +++ b/modules/digibug/controllers/digibug.php @@ -91,7 +91,7 @@ class Digibug_Controller extends Controller { } // We don't need to save the session for this request - Session::abort_save(); + Session::instance()->abort_save(); if (!TEST_MODE) { // Dump out the image diff --git a/modules/forge/libraries/Form_Group.php b/modules/forge/libraries/Form_Group.php index e0601321..0a04912b 100644 --- a/modules/forge/libraries/Form_Group.php +++ b/modules/forge/libraries/Form_Group.php @@ -80,7 +80,7 @@ class Form_Group_Core extends Forge { } } - public function render() + public function render($template = 'forge_template', $custom = FALSE) { // No Sir, we don't want any html today thank you return; diff --git a/modules/g2_import/controllers/admin_g2_import.php b/modules/g2_import/controllers/admin_g2_import.php index 1c65f482..6dd155b9 100644 --- a/modules/g2_import/controllers/admin_g2_import.php +++ b/modules/g2_import/controllers/admin_g2_import.php @@ -19,6 +19,7 @@ */ class Admin_g2_import_Controller extends Admin_Controller { public function index() { + g2_import::lower_error_reporting(); if (g2_import::is_configured()) { g2_import::init(); } @@ -31,6 +32,7 @@ class Admin_g2_import_Controller extends Admin_Controller { $view = new Admin_View("admin.html"); $view->content = new View("admin_g2_import.html"); $view->content->form = $this->_get_import_form(); + $view->content->version = g2_import::version(); if (g2_import::is_initialized()) { $view->content->g2_stats = $g2_stats; @@ -38,11 +40,13 @@ class Admin_g2_import_Controller extends Admin_Controller { $view->content->thumb_size = module::get_var("gallery", "thumb_size"); $view->content->resize_size = module::get_var("gallery", "resize_size"); } + g2_import::restore_error_reporting(); print $view; } public function save() { access::verify_csrf(); + g2_import::lower_error_reporting(); $form = $this->_get_import_form(); if ($form->validate()) { @@ -63,6 +67,7 @@ class Admin_g2_import_Controller extends Admin_Controller { $view = new Admin_View("admin.html"); $view->content = new View("admin_g2_import.html"); $view->content->form = $form; + g2_import::restore_error_reporting(); print $view; } diff --git a/modules/g2_import/helpers/g2_import.php b/modules/g2_import/helpers/g2_import.php index fa95e547..0fcc0539 100644 --- a/modules/g2_import/helpers/g2_import.php +++ b/modules/g2_import/helpers/g2_import.php @@ -24,6 +24,7 @@ class g2_import_Core { public static $g2_base_url = null; private static $current_g2_item = null; + private static $error_reporting = null; static function is_configured() { return module::get_var("g2_import", "embed_path"); @@ -931,6 +932,16 @@ class g2_import_Core { "useAuthToken" => false)); return str_replace(self::$g2_base_url, "", $url); } + + static function lower_error_reporting() { + // Gallery 2 was not designed to run in E_STRICT mode and will barf out errors. So dial down + // the error reporting when we make G2 calls. + self::$error_reporting = error_reporting(error_reporting() & ~E_STRICT); + } + + static function restore_error_reporting() { + error_reporting(self::$error_reporting); + } } /** diff --git a/modules/g2_import/helpers/g2_import_task.php b/modules/g2_import/helpers/g2_import_task.php index e80b88b9..21ba4c3a 100644 --- a/modules/g2_import/helpers/g2_import_task.php +++ b/modules/g2_import/helpers/g2_import_task.php @@ -19,17 +19,19 @@ */ class g2_import_task_Core { static function available_tasks() { + g2_import::lower_error_reporting(); if (g2_import::is_configured()) { g2_import::init(); } - + $version = g2_import::version(); + g2_import::restore_error_reporting(); if (class_exists("GalleryCoreApi")) { return array(Task_Definition::factory() ->callback("g2_import_task::import") ->name(t("Import from Gallery 2")) ->description( - t("Gallery %version detected", array("version" => g2_import::version()))) + t("Gallery %version detected", array("version" => $version))) ->severity(log::SUCCESS)); } @@ -37,6 +39,8 @@ class g2_import_task_Core { } static function import($task) { + g2_import::lower_error_reporting(); + $start = microtime(true); g2_import::init(); @@ -207,5 +211,7 @@ class g2_import_task_Core { $task->set("mode", $mode); $task->set("queue", $queue); $task->set("done", $done); + + g2_import::restore_error_reporting(); } } diff --git a/modules/g2_import/views/admin_g2_import.html.php b/modules/g2_import/views/admin_g2_import.html.php index 0875e7f7..6a5214a3 100644 --- a/modules/g2_import/views/admin_g2_import.html.php +++ b/modules/g2_import/views/admin_g2_import.html.php @@ -34,7 +34,7 @@

  • - g2_import::version())) ?> + $version)) ?>
  • diff --git a/modules/gallery/controllers/admin_modules.php b/modules/gallery/controllers/admin_modules.php index 84fee25d..081b3f12 100644 --- a/modules/gallery/controllers/admin_modules.php +++ b/modules/gallery/controllers/admin_modules.php @@ -67,6 +67,7 @@ class Admin_Modules_Controller extends Admin_Controller { } private function _do_save() { + $changes = new stdClass(); $changes->activate = array(); $changes->deactivate = array(); $activated_names = array(); diff --git a/modules/gallery/controllers/combined.php b/modules/gallery/controllers/combined.php index e90a2f1a..7f3a3c7d 100644 --- a/modules/gallery/controllers/combined.php +++ b/modules/gallery/controllers/combined.php @@ -41,7 +41,7 @@ class Combined_Controller extends Controller { $input = Input::instance(); // We don't need to save the session for this request - Session::abort_save(); + Session::instance()->abort_save(); // Our data is immutable, so if they already have a copy then it needs no updating. if ($input->server("HTTP_IF_MODIFIED_SINCE")) { diff --git a/modules/gallery/controllers/file_proxy.php b/modules/gallery/controllers/file_proxy.php index 646edf17..33952366 100644 --- a/modules/gallery/controllers/file_proxy.php +++ b/modules/gallery/controllers/file_proxy.php @@ -121,7 +121,7 @@ class File_Proxy_Controller extends Controller { expires::check(2592000, $item->updated); // We don't need to save the session for this request - Session::abort_save(); + Session::instance()->abort_save(); expires::set(2592000, $item->updated); // 30 days diff --git a/modules/gallery/helpers/gallery_block.php b/modules/gallery/helpers/gallery_block.php index 9d4e81b6..be0f11b8 100644 --- a/modules/gallery/helpers/gallery_block.php +++ b/modules/gallery/helpers/gallery_block.php @@ -72,7 +72,7 @@ class gallery_block_Core { $block->content = new View("admin_block_platform.html"); if (is_readable("/proc/loadavg")) { $block->content->load_average = - join(" ", array_slice(explode(" ", array_shift(file("/proc/loadavg"))), 0, 3)); + join(" ", array_slice(explode(" ", current(file("/proc/loadavg"))), 0, 3)); } else { $block->content->load_average = t("Unavailable"); } diff --git a/modules/gallery/helpers/gallery_rss.php b/modules/gallery/helpers/gallery_rss.php index d422636f..c1790d28 100644 --- a/modules/gallery/helpers/gallery_rss.php +++ b/modules/gallery/helpers/gallery_rss.php @@ -25,6 +25,7 @@ class gallery_rss_Core { } static function feed($feed_id, $offset, $limit, $id) { + $feed = new stdClass(); switch ($feed_id) { case "latest": $feed->children = ORM::factory("item") diff --git a/modules/gallery/helpers/gallery_task.php b/modules/gallery/helpers/gallery_task.php index c75e050a..b2f18d7c 100644 --- a/modules/gallery/helpers/gallery_task.php +++ b/modules/gallery/helpers/gallery_task.php @@ -111,6 +111,7 @@ class gallery_task_Core { site_status::clear("graphics_dirty"); } } catch (Exception $e) { + Kohana_Log::add("error",(string)$e); $task->done = true; $task->state = "error"; $task->status = $e->getMessage(); @@ -214,6 +215,7 @@ class gallery_task_Core { Cache::instance()->delete("update_l10n_cache:{$task->id}"); } } catch (Exception $e) { + Kohana_Log::add("error",(string)$e); $task->done = true; $task->state = "error"; $task->status = $e->getMessage(); @@ -233,10 +235,10 @@ class gallery_task_Core { try { $start = microtime(true); $data = Cache::instance()->get("file_cleanup_cache:{$task->id}"); - if ($data) { - $files = unserialize($data); - } + $files = $data ? unserialize($data) : array(); $i = 0; + $current = 0; + $total = 0; switch ($task->get("mode", "init")) { case "init": // 0% @@ -262,6 +264,7 @@ class gallery_task_Core { if (count($files) == 0) { break; } + case "delete_files": $current = $task->get("current"); $total = $task->get("total"); @@ -279,8 +282,10 @@ class gallery_task_Core { if ($total == $current) { $task->done = true; $task->state = "success"; + $task->percent_complete = 100; } } catch (Exception $e) { + Kohana_Log::add("error",(string)$e); $task->done = true; $task->state = "error"; $task->status = $e->getMessage(); diff --git a/modules/gallery/helpers/graphics.php b/modules/gallery/helpers/graphics.php index 5a290905..c85c7750 100644 --- a/modules/gallery/helpers/graphics.php +++ b/modules/gallery/helpers/graphics.php @@ -262,6 +262,9 @@ class graphics_Core { */ static function detect_toolkits() { $toolkits = new stdClass(); + $toolkits->gd = new stdClass(); + $toolkits->imagemagick = new stdClass(); + $toolkits->graphicsmagick = new stdClass(); // GD is special, it doesn't use exec() $gd = function_exists("gd_info") ? gd_info() : array(); diff --git a/modules/gallery/helpers/l10n_client.php b/modules/gallery/helpers/l10n_client.php index 086245e8..c27e4e5b 100644 --- a/modules/gallery/helpers/l10n_client.php +++ b/modules/gallery/helpers/l10n_client.php @@ -77,6 +77,7 @@ class l10n_client_Core { * translations for. */ static function fetch_updates(&$num_fetched) { + $request = new stdClass(); $request->locales = array(); $request->messages = new stdClass(); diff --git a/modules/gallery/helpers/module.php b/modules/gallery/helpers/module.php index 95e426c4..9523d1d2 100644 --- a/modules/gallery/helpers/module.php +++ b/modules/gallery/helpers/module.php @@ -430,7 +430,8 @@ class module_Core { // This could happen if there's a race condition continue; } - self::$var_cache->{$row->module_name}->{$row->name} = $row->value; + // Mute the "Creating default object from empty value" warning below + @self::$var_cache->{$row->module_name}->{$row->name} = $row->value; } $cache = ORM::factory("var"); $cache->module_name = "gallery"; diff --git a/modules/gallery/libraries/Form_Script.php b/modules/gallery/libraries/Form_Script.php index e841408d..1f965767 100644 --- a/modules/gallery/libraries/Form_Script.php +++ b/modules/gallery/libraries/Form_Script.php @@ -50,7 +50,7 @@ class Form_Script_Core extends Forge { return $this; } - public function render() { + public function render($template="forge_template", $custom=false) { $script = array(); if (!empty($this->data["url"])) { $script[] = html::script($this->data["url"]); @@ -63,4 +63,4 @@ class Form_Script_Core extends Forge { return implode("\n", $script); } -} // End Form Script \ No newline at end of file +} \ No newline at end of file diff --git a/modules/gallery/libraries/MY_Database.php b/modules/gallery/libraries/MY_Database.php index 61f23fb0..e2ef68cd 100644 --- a/modules/gallery/libraries/MY_Database.php +++ b/modules/gallery/libraries/MY_Database.php @@ -38,7 +38,7 @@ abstract class Database extends Database_Core { * Parse the query string and convert any strings of the form `\([a-zA-Z0-9_]*?)\] * table prefix . $1 */ - public function query($sql = '') { + public function query($sql) { if (!empty($sql)) { $sql = $this->add_table_prefixes($sql); } diff --git a/modules/gallery/libraries/MY_View.php b/modules/gallery/libraries/MY_View.php index cec59ec1..83e0d0be 100644 --- a/modules/gallery/libraries/MY_View.php +++ b/modules/gallery/libraries/MY_View.php @@ -27,7 +27,7 @@ class View extends View_Core { View::$global_data[$key] = $value; } - public function is_set($key) { + public function is_set($key=null) { return parent::is_set($key) ? true : array_key_exists($key, View::$global_data); } diff --git a/modules/gallery/libraries/ORM_MPTT.php b/modules/gallery/libraries/ORM_MPTT.php index 83f9b51e..3668d42d 100644 --- a/modules/gallery/libraries/ORM_MPTT.php +++ b/modules/gallery/libraries/ORM_MPTT.php @@ -85,7 +85,7 @@ class ORM_MPTT_Core extends ORM { /** * Delete this node and all of its children. */ - public function delete() { + public function delete($ignored_id=null) { $children = $this->children(); if ($children) { foreach ($this->children() as $item) { diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php index 083fd06b..dbd56fa2 100644 --- a/modules/gallery/models/item.php +++ b/modules/gallery/models/item.php @@ -70,7 +70,7 @@ class Item_Model extends ORM_MPTT { return $this->type == 'movie'; } - public function delete() { + public function delete($ignored_id=null) { if ($this->id == 1) { $v = new Validation(array("id")); $v->add_error("id", "cant_delete_root_album"); diff --git a/modules/gallery/models/task.php b/modules/gallery/models/task.php index f40be492..24d909cb 100644 --- a/modules/gallery/models/task.php +++ b/modules/gallery/models/task.php @@ -27,7 +27,7 @@ class Task_Model extends ORM { } } - public function set($key, $value) { + public function set($key, $value=null) { $context = unserialize($this->context); $context[$key] = $value; $this->context = serialize($context); @@ -40,7 +40,7 @@ class Task_Model extends ORM { return parent::save(); } - public function delete() { + public function delete($ignored_id=null) { Cache::instance()->delete($this->_cache_key()); return parent::delete(); } diff --git a/modules/gallery/tests/Database_Test.php b/modules/gallery/tests/Database_Test.php index e58f73eb..861f7bba 100644 --- a/modules/gallery/tests/Database_Test.php +++ b/modules/gallery/tests/Database_Test.php @@ -168,12 +168,12 @@ class Database_Mock extends Database { return array("test"); } - public function quote_column($val) { - return "[$val]"; + public function quote_column($val, $alias=null) { + return $alias ? "[$val,$alias]" : "[$val]"; } - public function quote_table($val) { - return "[$val]"; + public function quote_table($val, $alias=null) { + return $alias ? "[$val,$alias]" : "[$val]"; } public function quote($val) { diff --git a/modules/gallery/tests/Item_Rest_Helper_Test.php b/modules/gallery/tests/Item_Rest_Helper_Test.php index 088b1cbd..6d1dd864 100644 --- a/modules/gallery/tests/Item_Rest_Helper_Test.php +++ b/modules/gallery/tests/Item_Rest_Helper_Test.php @@ -32,6 +32,7 @@ class Item_Rest_Helper_Test extends Gallery_Unit_Test_Case { $album1->reload(); // No scope is the same as "direct" + $request = new stdClass(); $request->url = rest::url("item", $album1); $request->params = new stdClass(); $this->assert_equal_array( @@ -84,7 +85,9 @@ class Item_Rest_Helper_Test extends Gallery_Unit_Test_Case { $photo2->save(); $album1->reload(); + $request = new stdClass(); $request->url = rest::url("item", $album1); + $request->params = new stdClass(); $request->params->name = "foo"; $this->assert_equal_array( array("url" => rest::url("item", $album1), @@ -104,7 +107,9 @@ class Item_Rest_Helper_Test extends Gallery_Unit_Test_Case { $album2 = test::random_album($album1); $album1->reload(); + $request = new stdClass(); $request->url = rest::url("item", $album1); + $request->params = new stdClass(); $request->params->type = "album"; $this->assert_equal_array( array("url" => rest::url("item", $album1), @@ -122,7 +127,9 @@ class Item_Rest_Helper_Test extends Gallery_Unit_Test_Case { $album1 = test::random_album(); access::allow(identity::everybody(), "edit", $album1); + $request = new stdClass(); $request->url = rest::url("item", $album1); + $request->params = new stdClass(); $request->params->title = "my new title"; item_rest::put($request); @@ -133,7 +140,9 @@ class Item_Rest_Helper_Test extends Gallery_Unit_Test_Case { $album1 = test::random_album(); access::allow(identity::everybody(), "edit", $album1); + $request = new stdClass(); $request->url = rest::url("item", $album1); + $request->params = new stdClass(); $request->params->title = "my new title"; $request->params->slug = "not url safe"; @@ -150,7 +159,9 @@ class Item_Rest_Helper_Test extends Gallery_Unit_Test_Case { $album1 = test::random_album(); access::allow(identity::everybody(), "edit", $album1); + $request = new stdClass(); $request->url = rest::url("item", $album1); + $request->params = new stdClass(); $request->params->type = "album"; $request->params->name = "my album"; $request->params->title = "my album"; @@ -165,7 +176,9 @@ class Item_Rest_Helper_Test extends Gallery_Unit_Test_Case { $album1 = test::random_album(); access::allow(identity::everybody(), "edit", $album1); + $request = new stdClass(); $request->url = rest::url("item", $album1); + $request->params = new stdClass(); $request->params->type = "album"; $request->params->name = "my album"; $request->params->title = "my album"; @@ -185,7 +198,9 @@ class Item_Rest_Helper_Test extends Gallery_Unit_Test_Case { $album1 = test::random_album(); access::allow(identity::everybody(), "edit", $album1); + $request = new stdClass(); $request->url = rest::url("item", $album1); + $request->params = new stdClass(); $request->params->type = "photo"; $request->params->name = "my photo.jpg"; $request->file = MODPATH . "gallery/tests/test.jpg"; @@ -200,6 +215,7 @@ class Item_Rest_Helper_Test extends Gallery_Unit_Test_Case { $album1 = test::random_album(); access::allow(identity::everybody(), "edit", $album1); + $request = new stdClass(); $request->url = rest::url("item", $album1); item_rest::delete($request); @@ -212,6 +228,7 @@ class Item_Rest_Helper_Test extends Gallery_Unit_Test_Case { access::deny(identity::everybody(), "edit", $album1); identity::set_active_user(identity::guest()); + $request = new stdClass(); $request->url = rest::url("item", $album1); try { item_rest::delete($request); diff --git a/modules/gallery_unit_test/controllers/gallery_unit_test.php b/modules/gallery_unit_test/controllers/gallery_unit_test.php index e05fcbaa..2690ad24 100644 --- a/modules/gallery_unit_test/controllers/gallery_unit_test.php +++ b/modules/gallery_unit_test/controllers/gallery_unit_test.php @@ -18,11 +18,15 @@ * Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, MA 02110-1301, USA. */ class Gallery_Unit_Test_Controller extends Controller { - function Index() { + function index() { if (!TEST_MODE) { throw new Kohana_404_Exception(); } + // Force strict behavior to flush out bugs early + ini_set("display_errors", true); + error_reporting(-1); + // Jump through some hoops to satisfy the way that we check for the site_domain in // config.php. We structure this such that the code in config will leave us with a // site_domain of "." (for historical reasons) diff --git a/modules/rest/controllers/rest.php b/modules/rest/controllers/rest.php index 9141d6d4..374ae0d2 100644 --- a/modules/rest/controllers/rest.php +++ b/modules/rest/controllers/rest.php @@ -40,6 +40,7 @@ class Rest_Controller extends Controller { public function __call($function, $args) { $input = Input::instance(); + $request = new stdClass(); switch ($method = strtolower($input->server("REQUEST_METHOD"))) { case "get": $request->params = (object) $input->get(); diff --git a/modules/rest/helpers/rest.php b/modules/rest/helpers/rest.php index b3f80a55..a61aba2f 100644 --- a/modules/rest/helpers/rest.php +++ b/modules/rest/helpers/rest.php @@ -19,7 +19,7 @@ */ class rest_Core { static function reply($data=array()) { - Session::abort_save(); + Session::instance()->abort_save(); if ($data) { if (Input::instance()->get("output") == "html") { diff --git a/modules/rest/tests/Rest_Controller_Test.php b/modules/rest/tests/Rest_Controller_Test.php index 5e624112..9f73bed9 100644 --- a/modules/rest/tests/Rest_Controller_Test.php +++ b/modules/rest/tests/Rest_Controller_Test.php @@ -138,8 +138,8 @@ class Rest_Controller_Test extends Gallery_Unit_Test_Case { } class mock_rest { - function get($request) { return $request; } - function post($request) { return $request; } - function put($request) { return $request; } - function delete($request) { return $request; } + static function get($request) { return $request; } + static function post($request) { return $request; } + static function put($request) { return $request; } + static function delete($request) { return $request; } } \ No newline at end of file diff --git a/modules/search/helpers/search.php b/modules/search/helpers/search.php index b2497eae..9018ffa2 100644 --- a/modules/search/helpers/search.php +++ b/modules/search/helpers/search.php @@ -65,7 +65,8 @@ class search_Core { $record->item_id = $item->id; } - module::event("item_index_data", $record->item(), $data); + $item = $record->item(); + module::event("item_index_data", $item, $data); $record->data = join(" ", (array)$data); $record->dirty = 0; $record->save(); diff --git a/modules/tag/helpers/tag_rss.php b/modules/tag/helpers/tag_rss.php index f09a4530..5d42caab 100644 --- a/modules/tag/helpers/tag_rss.php +++ b/modules/tag/helpers/tag_rss.php @@ -34,6 +34,8 @@ class tag_rss_Core { if (!$tag->loaded()) { throw new Kohana_404_Exception(); } + + $feed = new stdClass(); $feed->children = $tag->items($limit, $offset, "photo"); $feed->max_pages = ceil($tag->count / $limit); $feed->title = $tag->name; diff --git a/modules/tag/helpers/tags_rest.php b/modules/tag/helpers/tags_rest.php index ac0eb81d..f28be7b5 100644 --- a/modules/tag/helpers/tags_rest.php +++ b/modules/tag/helpers/tags_rest.php @@ -35,7 +35,6 @@ class tags_rest_Core { $query->or_where("edit_{$group->id}", "=", access::ALLOW); } $has_any_edit_perm = $query->close()->count_records(); - if (!$has_any_edit_perm) { access::forbidden(); } diff --git a/modules/tag/models/tag.php b/modules/tag/models/tag.php index 2b33c30d..38a8ed69 100644 --- a/modules/tag/models/tag.php +++ b/modules/tag/models/tag.php @@ -95,7 +95,7 @@ class Tag_Model extends ORM { * Overload ORM::delete() to trigger an item_related_update event for all items that are * related to this tag. */ - public function delete() { + public function delete($ignored_id=null) { $related_item_ids = array(); foreach (db::build() ->select("item_id") diff --git a/modules/tag/tests/Tag_Item_Rest_Helper_Test.php b/modules/tag/tests/Tag_Item_Rest_Helper_Test.php index 69c580f1..cb368790 100644 --- a/modules/tag/tests/Tag_Item_Rest_Helper_Test.php +++ b/modules/tag/tests/Tag_Item_Rest_Helper_Test.php @@ -28,6 +28,7 @@ class Tag_Item_Rest_Helper_Test extends Gallery_Unit_Test_Case { public function get_test() { $tag = tag::add(item::root(), "tag1")->reload(); + $request = new stdClass(); $request->url = rest::url("tag_item", $tag, item::root()); $this->assert_equal_array( array("url" => rest::url("tag_item", $tag, item::root()), @@ -38,6 +39,7 @@ class Tag_Item_Rest_Helper_Test extends Gallery_Unit_Test_Case { } public function get_with_invalid_url_test() { + $request = new stdClass(); $request->url = "bogus"; try { tag_item_rest::get($request); @@ -50,6 +52,7 @@ class Tag_Item_Rest_Helper_Test extends Gallery_Unit_Test_Case { public function delete_test() { $tag = tag::add(item::root(), "tag1")->reload(); + $request = new stdClass(); $request->url = rest::url("tag_item", $tag, item::root()); tag_item_rest::delete($request); @@ -60,7 +63,6 @@ class Tag_Item_Rest_Helper_Test extends Gallery_Unit_Test_Case { $album = test::random_album(); $tag = tag::add($album, "tag1")->reload(); - $tuple = rest::resolve(rest::url("tag_item", $tag, $album)); $this->assert_equal_array($tag->as_array(), $tuple[0]->as_array()); $this->assert_equal_array($album->as_array(), $tuple[1]->as_array()); diff --git a/modules/tag/tests/Tag_Rest_Helper_Test.php b/modules/tag/tests/Tag_Rest_Helper_Test.php index d3cae0fb..838de975 100644 --- a/modules/tag/tests/Tag_Rest_Helper_Test.php +++ b/modules/tag/tests/Tag_Rest_Helper_Test.php @@ -28,6 +28,7 @@ class Tag_Rest_Helper_Test extends Gallery_Unit_Test_Case { public function get_test() { $tag = tag::add(item::root(), "tag1")->reload(); + $request = new stdClass(); $request->url = rest::url("tag", $tag); $this->assert_equal_array( array("url" => rest::url("tag", $tag), @@ -41,6 +42,7 @@ class Tag_Rest_Helper_Test extends Gallery_Unit_Test_Case { } public function get_with_invalid_url_test() { + $request = new stdClass(); $request->url = "bogus"; try { tag_rest::get($request); @@ -53,6 +55,7 @@ class Tag_Rest_Helper_Test extends Gallery_Unit_Test_Case { public function get_with_no_relationships_test() { $tag = test::random_tag(); + $request = new stdClass(); $request->url = rest::url("tag", $tag); $this->assert_equal_array( array("url" => rest::url("tag", $tag), @@ -72,7 +75,9 @@ class Tag_Rest_Helper_Test extends Gallery_Unit_Test_Case { access::allow(identity::everybody(), "edit", $album); // Add the album to the tag + $request = new stdClass(); $request->url = rest::url("tag", $tag); + $request->params = new stdClass(); $request->params->url = rest::url("item", $album); $this->assert_equal_array( array("url" => rest::url("tag_item", $tag, $album)), @@ -93,7 +98,9 @@ class Tag_Rest_Helper_Test extends Gallery_Unit_Test_Case { public function put_test() { $tag = test::random_tag(); + $request = new stdClass(); $request->url = rest::url("tag", $tag); + $request->params = new stdClass(); $request->params->name = "new name"; tag_rest::put($request); @@ -102,6 +109,7 @@ class Tag_Rest_Helper_Test extends Gallery_Unit_Test_Case { public function delete_tag_test() { $tag = test::random_tag(); + $request = new stdClass(); $request->url = rest::url("tag", $tag); tag_rest::delete($request); diff --git a/modules/tag/tests/Tags_Rest_Helper_Test.php b/modules/tag/tests/Tags_Rest_Helper_Test.php index a1713811..cdf7bfdf 100644 --- a/modules/tag/tests/Tags_Rest_Helper_Test.php +++ b/modules/tag/tests/Tags_Rest_Helper_Test.php @@ -43,6 +43,8 @@ class Tags_Rest_Helper_Test extends Gallery_Unit_Test_Case { public function post_test() { access::allow(identity::everybody(), "edit", item::root()); + $request = new stdClass(); + $request->params = new stdClass(); $request->params->name = "test tag"; $this->assert_equal( array("url" => url::site("rest/tag/1")), @@ -55,6 +57,8 @@ class Tags_Rest_Helper_Test extends Gallery_Unit_Test_Case { identity::set_active_user(identity::guest()); try { + $request = new stdClass(); + $request->params = new stdClass(); $request->params->name = "test tag"; tags_rest::post($request); } catch (Exception $e) { diff --git a/modules/user/controllers/admin_users.php b/modules/user/controllers/admin_users.php index c11b0596..03d9858b 100644 --- a/modules/user/controllers/admin_users.php +++ b/modules/user/controllers/admin_users.php @@ -323,7 +323,7 @@ class Admin_Users_Controller extends Admin_Controller { return $form; } - private function _add_locale_dropdown(&$form, $user=null) { + private static function _add_locale_dropdown(&$form, $user=null) { $locales = locales::installed(); foreach ($locales as $locale => $display_name) { $locales[$locale] = SafeString::of_safe_html($display_name); -- cgit v1.2.3