From 0e3327bca70623175791ee41085d55d0cb13fe5b Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Sun, 3 Jan 2010 20:30:35 -0800 Subject: Simplify the REST API code. Here's what I did: 1) Simplify gallery_rest to return flat models, no children and do no validation for now. 2) Flatten the REST replies and use HTTP codes to indicate success/failure instead of additional status messages. 3) Use the message and error code support in the base Exception class, instead of brewing our own in Rest_Exception. 4) Get rid of rest::success() and rest::fail() -- we only need rest::reply() since all failures are covered by throwing an exception. 5) Get rid of /rest/access_key and just use /rest for authentication. 6) Inline and simplify rest::normalize_request since we only use it once 7) Change rest::set_active_user to succeed or throw an exception 8) Extract Rest_Exception::sendHeaders into rest::send_headers() Here's what's currently broken: 1) Data validation. There currently is none 2) Logging. That's gone too 3) image block and tag code is broken 4) Tests are broken 5) No movie support --- modules/gallery/helpers/gallery_rest.php | 263 +++++++------------------------ 1 file changed, 57 insertions(+), 206 deletions(-) (limited to 'modules/gallery/helpers') diff --git a/modules/gallery/helpers/gallery_rest.php b/modules/gallery/helpers/gallery_rest.php index a87ebb4e..f1c8d825 100644 --- a/modules/gallery/helpers/gallery_rest.php +++ b/modules/gallery/helpers/gallery_rest.php @@ -17,232 +17,83 @@ * along with this program; if not, write to the Free Software * Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, MA 02110-1301, USA. */ -class gallery_rest_Core { - static function get($request) { - $path = implode("/", $request->arguments); - $item = gallery_rest::_get_item($path); +// @todo Add logging +// @todo VALIDATION + +// Validation questions +// +// We need to be able to properly validate anything we want to enter here. But all of our +// validation currently happens at the controller / form level, and we're not using the same +// controllers or forms. +// +// Possible solutions: +// 1) Move validation into the model and use it both here and in the regular controllers. But +// if we do that, how do we translate validation failures into a user-consumable output which +// we need so that we can return proper error responses to form submissions? +// +// 2) Create some kind of validation helper that can validate every field. Wait, isn't this +// just like #1 except in a helper instead of in the model? - $parent = $item->parent(); - $response_data = array("type" => $item->type, - "name" => $item->name, - "path" => $item->relative_url(), - "parent_path" => empty($parent) ? null : $parent->relative_url(), - "title" => $item->title, - "thumb_url" => $item->thumb_url(true), - "thumb_size" => array("height" => $item->thumb_height, - "width" => $item->thumb_width), - "resize_url" => $item->resize_url(true), - "resize_size" => array("height" => (int)$item->resize_height, - "width" => (int)$item->resize_width), - "url" => $item->file_url(true), - "size" => array("height" => $item->height, - "width" => $item->width), - "description" => $item->description, - "slug" => $item->slug); +class gallery_rest_Core { + static function get($request) { + $item = url::get_item_from_uri($request->path); + access::required("view", $item); - $children = self::_get_children($item, $request); - if (!empty($children) || $item->is_album()) { - $response_data["children"] = $children; - } - return rest::success(array("resource" => $response_data)); + return json_encode($item->as_array()); } static function put($request) { - if (empty($request->arguments)) { - throw new Rest_Exception(400, "Bad request"); - } - $path = implode("/", $request->arguments); - $item = gallery_rest::_get_item($path, "edit"); - - // Validate the request data - $new_values = gallery_rest::_validate($request, $item->parent_id, $item->id); - $errors = $new_values->errors(); - if (empty($errors)) { - $item->title = $new_values->title; - $item->description = $new_values->description; - if ($item->id != 1) { - $item->rename($new_values->name); + $item = url::get_item_from_uri($request->path); + access::required("edit", $item); + + $params = $request->params; + foreach (array("captured", "description", "slug", "sort_column", "sort_order", + "title", "view_count", "weight") as $key) { + if (isset($params->$key)) { + $item->$key = $params->$key; } - $item->slug = $new_values->slug; - $item->save(); - - log::success("content", "Updated $item->type", - "type}s/$item->id\">view"); - - return rest::success(); - } else { - return rest::validation_error($errors); } + $item->save(); + + return rest::reply(array("url" => url::abs_site("/rest/gallery/" . $item->relative_url()))); } static function post($request) { - if (empty($request->arguments)) { - throw new Rest_Exception(400, "Bad request"); - } + $parent = url::get_item_from_uri($request->path); + access::required("edit", $parent); - $components = $request->arguments; - $name = urldecode(array_pop($components)); - - $parent = gallery_rest::_get_item(implode("/", $components), "edit"); - - // Validate the request data - $request->name = $name; - $new_values = gallery_rest::_validate($request, $parent->id); - $errors = $new_values->errors(); - if (!empty($errors)) { - return rest::validation_error($errors); - } + $params = $request->params; + switch ($params->type) { + case "album": + $item = album::create( + $parent, + $params->name, + isset($params->title) ? $params->title : $name, + isset($params->description) ? $params->description : null); + break; - if (empty($new_values["image"])) { - $new_item = album::create( + case "photo": + $item = photo::create( $parent, - $name, - empty($new_values["title"]) ? $name : $new_values["title"], - empty($new_values["description"]) ? null : $new_values["description"], - identity::active_user()->id, - empty($new_values["slug"]) ? $name : $new_values["slug"]); - $log_message = t("Added an album"); - } else { - $temp_filename = upload::save("image"); - $path_info = @pathinfo($temp_filename); - if (array_key_exists("extension", $path_info) && - in_array(strtolower($path_info["extension"]), array("flv", "mp4"))) { - $new_item = - movie::create($parent, $temp_filename, $new_values["name"], $new_values["title"]); - $log_message = t("Added a movie"); - } else { - $new_item = - photo::create($parent, $temp_filename, $new_values["name"], $new_values["title"]); - $log_message = t("Added a photo"); - } - } + $request->file, + $params->name, + isset($params->title) ? $params->title : $name, + isset($params->description) ? $params->description : null); + break; - log::success("content", $log_message, "type}s/$new_item->id\">view"); + default: + throw new Rest_Exception("Invalid type: $args->type", 400); + } - return rest::success(array("path" => $new_item->relative_url())); + return rest::reply(array("url" => url::abs_site("/rest/gallery/" . $item->relative_url()))); } static function delete($request) { - if (empty($request->arguments)) { - throw new Rest_Exception(400, "Bad request"); - } - $path = implode("/", $request->arguments); - - $item = gallery_rest::_get_item($path, "edit"); - - if ($item->id == 1) { - throw new Rest_Exception(400, "Bad request"); - } + $item = url::get_item_from_uri($request->path); + access::required("edit", $item); - $parent = $item->parent(); $item->delete(); - - if ($item->is_album()) { - $msg = t("Deleted album %title", array("title" => html::purify($item->title))); - } else { - $msg = t("Deleted photo %title", array("title" => html::purify($item->title))); - } - log::success("content", $msg); - - return rest::success(array("resource" => array("parent_path" => $parent->relative_url()))); - } - - private static function _get_item($path, $permission="view") { - $item = url::get_item_from_uri($path); - - if (!$item->loaded()) { - throw new Kohana_404_Exception(); - } - - if (!access::can($permission, $item)) { - throw new Kohana_404_Exception(); - } - - return $item; - } - - private static function _get_children($item, $request) { - $children = array(); - $limit = empty($request->limit) ? null : $request->limit; - $offset = empty($request->offset) ? null : $request->offset; - $where = empty($request->filter) ? array() : array("type" => $request->filter); - foreach ($item->viewable()->children($limit, $offset, $where) as $child) { - $children[] = array("type" => $child->type, - "has_children" => $child->children_count() > 0, - "path" => $child->relative_url(), - "thumb_url" => $child->thumb_url(true), - "thumb_dimensions" => array("width" => $child->thumb_width, - "height" => $child->thumb_height), - "has_thumb" => $child->has_thumb(), - "title" => $child->title); - } - - return $children; - } - - private static function _validate($request, $parent_id, $item_id=0) { - $item = ORM::factory("item", $item_id); - - // Normalize the inputs so all fields have a value - $new_values = Validation::factory(array()); - foreach ($item->form_rules as $field => $rule_set) { - if (isset($request->$field)) { - $new_values[$field] = $request->$field; - } else if (isset($item->$field)) { - $new_values[$field] = $item->$field; - } - foreach (explode("|", $rule_set) as $rule) { - $new_values->add_rules($field, $rule); - } - } - $name = $new_values["name"]; - $new_values["title"] = empty($new_values["title"]) ? $name : $new_values["title"]; - $new_values["description"] = - empty($new_values["description"]) ? null : $new_values["description"]; - $new_values["slug"] = empty($new_values["slug"]) ? $name : $new_values["slug"]; - - if (!empty($request->image)) { - $new_values["image"] = $request->image; - $new_values->add_rules( - "image", "upload::valid", "upload::required", "upload::type[gif,jpg,jpeg,png,flv,mp4]"); - } - - if ($new_values->validate() && $item_id != 1) { - $errors = gallery_rest::_check_for_conflicts($parent_id, $item_id, - $new_values["name"], $new_values["slug"]); - if (!empty($errors)) { - !empty($errors["name_conflict"]) OR $new_values->add_error("name", "Duplicate name"); - !empty($errors["slug_conflict"]) OR - $new_values->add_error("slug", "Duplicate Internet address"); - } - } - - return $new_values; - } - - private static function _check_for_conflicts($parent_id, $item_id, $new_name, $new_slug) { - $errors = array(); - - if ($row = db::build() - ->select(array("name", "slug")) - ->from("items") - ->where("parent_id", "=", $parent_id) - ->where("id", "<>", $item_id) - ->and_open() - ->where("name", "=", $new_name) - ->or_where("slug", "=", $new_slug) - ->close() - ->execute() - ->current()) { - if ($row->name == $new_name) { - $errors["name_conflict"] = 1; - } - if ($row->slug == $new_slug) { - $errors["slug_conflict"] = 1; - } - } - - return $errors; + return rest::reply(); } } -- cgit v1.2.3 From 3fffa18e650189e7f846592c9d4c3e7bbfe71c62 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Mon, 4 Jan 2010 21:48:21 -0800 Subject: Further progress on refining the REST server side code. 1) Deal in fully qualified URL resources through the rest interface. All rest methods are now passed the complete url in request->url. 2) Create rest::resolve() which lets individual resource definition code convert a full url into the appropriate matching resource. Implement gallery_rest::resolve() and tag_rest::resolve() 3) Reimplement tag_rest's get() and post() methods. They're much simpler now. 4) Implement the tags_rest helper which supports working with the entire tags collection. --- modules/gallery/helpers/gallery_rest.php | 14 ++++--- modules/rest/controllers/rest.php | 2 +- modules/rest/helpers/rest.php | 20 +++++++++ modules/tag/helpers/tag.php | 2 +- modules/tag/helpers/tag_rest.php | 69 ++++++++------------------------ modules/tag/helpers/tags_rest.php | 48 ++++++++++++++++++++++ 6 files changed, 96 insertions(+), 59 deletions(-) create mode 100644 modules/tag/helpers/tags_rest.php (limited to 'modules/gallery/helpers') diff --git a/modules/gallery/helpers/gallery_rest.php b/modules/gallery/helpers/gallery_rest.php index f1c8d825..858721d0 100644 --- a/modules/gallery/helpers/gallery_rest.php +++ b/modules/gallery/helpers/gallery_rest.php @@ -37,14 +37,14 @@ class gallery_rest_Core { static function get($request) { - $item = url::get_item_from_uri($request->path); + $item = rest::resolve($request->url); access::required("view", $item); - return json_encode($item->as_array()); + return rest::reply($item->as_array()); } static function put($request) { - $item = url::get_item_from_uri($request->path); + $item = rest::resolve($request->url); access::required("edit", $item); $params = $request->params; @@ -60,7 +60,7 @@ class gallery_rest_Core { } static function post($request) { - $parent = url::get_item_from_uri($request->path); + $parent = rest::resolve($request->url); access::required("edit", $parent); $params = $request->params; @@ -90,10 +90,14 @@ class gallery_rest_Core { } static function delete($request) { - $item = url::get_item_from_uri($request->path); + $item = rest::resolve($request->url); access::required("edit", $item); $item->delete(); return rest::reply(); } + + static function resolve($path) { + return url::get_item_from_uri($path); + } } diff --git a/modules/rest/controllers/rest.php b/modules/rest/controllers/rest.php index 0332e5fc..5ef9eb84 100644 --- a/modules/rest/controllers/rest.php +++ b/modules/rest/controllers/rest.php @@ -60,7 +60,7 @@ class Rest_Controller extends Controller { $request->method = strtolower($input->server("HTTP_X_GALLERY_REQUEST_METHOD", $method)); $request->access_token = $input->server("HTTP_X_GALLERY_REQUEST_KEY"); - $request->path = implode("/", $args); + $request->url = url::abs_current(true); try { rest::set_active_user($request->access_token); diff --git a/modules/rest/helpers/rest.php b/modules/rest/helpers/rest.php index f7f3f9fd..b1b83e1b 100644 --- a/modules/rest/helpers/rest.php +++ b/modules/rest/helpers/rest.php @@ -51,4 +51,24 @@ class rest_Core { static function send_headers($exception) { header("HTTP/1.1 " . $exception->getCode() . " " . $exception->getMessage()); } + + /** + * Convert a REST url into an object. + * Eg: "http://example.com/gallery3/index.php/rest/gallery/Family/Wedding" -> Item_Model + * + * @param string the fully qualified REST url + * @return mixed the corresponding object (usually a model of some kind) + */ + static function resolve($url) { + $components = explode("/", substr($url, strlen(url::abs_site("rest"))), 3); + + // The first component will be empty because of the slash between "rest" and the + // resource type. + $class = "$components[1]_rest"; + if (!method_exists($class, "resolve")) { + throw new Kohana_404_Exception($url); + } + + return call_user_func(array($class, "resolve"), !empty($components[2]) ? $components[2] : null); + } } diff --git a/modules/tag/helpers/tag.php b/modules/tag/helpers/tag.php index 8075afe4..d895e08f 100644 --- a/modules/tag/helpers/tag.php +++ b/modules/tag/helpers/tag.php @@ -41,7 +41,7 @@ class tag_Core { } if (!$tag->has($item)) { - if (!$tag->add($item, $tag)) { + if (!$tag->add($item)) { throw new Exception("@todo {$tag->name} WAS_NOT_ADDED_TO {$item->id}"); } $tag->count++; diff --git a/modules/tag/helpers/tag_rest.php b/modules/tag/helpers/tag_rest.php index 0c06587b..4b5103ef 100644 --- a/modules/tag/helpers/tag_rest.php +++ b/modules/tag/helpers/tag_rest.php @@ -18,71 +18,36 @@ * Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, MA 02110-1301, USA. */ class tag_rest_Core { - // If no arguments just return all the tags. If 2 or more then it is a path then - // return the tags for that item. But if its only 1, then is it a path or a tag? - // Assume a tag first, if nothing is found then try finding the item. static function get($request) { - $resources = array(); - switch (count($request->arguments)) { - case 0: - $tags = ORM::factory("tag") - ->select("name", "count") - ->order_by("count", "DESC"); - if (!empty($request->limit)) { - $tags->limit($request->limit); - } - if (!empty($request->offset)) { - $tags->offset($request->offset); - } - $resources = array("tags" => array()); - foreach ($tags->find_all() as $row) { - $resources["tags"][] = array("name" => $row->name, "count" => $row->count); - } - break; - case 1: - $resources = tag_rest::_get_items($request); - if (!empty($resources)) { - $resources = array("resources" => $resources); - break; - } - default: - $item = ORM::factory("item") - ->where("relative_url_cache", "=", implode("/", $request->arguments)) - ->viewable() - ->find(); - if ($item->loaded()) { - $resources = array("tags" => tag::item_tags($item)); - } - } - - return rest::reply($resources); + return rest::reply(rest::resolve($request->url)->as_array()); } static function post($request) { - if (empty($request->arguments) || count($request->arguments) != 1 || empty($request->path)) { + $tag = rest::resolve($request->url); + + if (empty($request->params->url)) { throw new Rest_Exception("Bad request", 400); } - $path = $request->path; - $tags = explode(",", $request->arguments[0]); - $item = ORM::factory("item") - ->where("relative_url_cache", "=", $path) - ->viewable() - ->find(); - if (!$item->loaded()) { - throw new Kohana_404_Exception(); - } + $item = rest::resolve($request->params->url); - if (!access::can("edit", $item)) { + access::required("edit", $item); + tag::add($item, $tag->name); + + return rest::reply(); + } + + static function resolve($tag_name) { + $tag = ORM::factory("tag")->where("name", "=", $tag_name)->find(); + if (!$tag->loaded()) { throw new Kohana_404_Exception(); } - foreach ($tags as $tag) { - tag::add($item, $tag); - } - return rest::reply(); + return $tag; } + // ------------------------------------------------------------ + static function put($request) { if (empty($request->arguments[0]) || empty($request->new_name)) { throw new Rest_Exception("Bad request", 400); diff --git a/modules/tag/helpers/tags_rest.php b/modules/tag/helpers/tags_rest.php new file mode 100644 index 00000000..d2bd28b0 --- /dev/null +++ b/modules/tag/helpers/tags_rest.php @@ -0,0 +1,48 @@ +find_all() as $tag) { + $data[$tag->name] = url::abs_site("rest/tags/" . rawurlencode($tag->name)); + } + return rest::reply($data); + } + + static function post($request) { + // @todo: what permission should be required to create a tag here? + // for now, require edit at the top level. Perhaps later, just require any edit perms, + // anywhere in the gallery? + access::required("edit", item::root()); + + if (empty($request->params->name)) { + throw new Rest_Exception("Bad Request", 400); + } + + $tag = ORM::factory("tag")->where("name", "=", $request->params->name)->find(); + if (!$tag->loaded()) { + $tag->name = $request->params->name; + $tag->count = 0; + $tag->save(); + } + + return rest::reply(array("url" => url::abs_site("rest/tag/" . rawurlencode($tag->name)))); + } +} -- cgit v1.2.3 From 31454d37b3ea02104925f1976609576c5f09c0c6 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Tue, 5 Jan 2010 13:41:06 -0800 Subject: Improve REST tag support. - Add support for retrieving a list of members from a collection - Implement put(), post() and delete() for tags. - Use tag_rest::delete() as a way to remove members from the tag collection --- modules/gallery/helpers/gallery_rest.php | 7 +- modules/tag/helpers/tag_rest.php | 118 ++++++++++++------------------- modules/tag/helpers/tags_rest.php | 6 +- 3 files changed, 54 insertions(+), 77 deletions(-) (limited to 'modules/gallery/helpers') diff --git a/modules/gallery/helpers/gallery_rest.php b/modules/gallery/helpers/gallery_rest.php index 858721d0..c7b32d8c 100644 --- a/modules/gallery/helpers/gallery_rest.php +++ b/modules/gallery/helpers/gallery_rest.php @@ -40,7 +40,12 @@ class gallery_rest_Core { $item = rest::resolve($request->url); access::required("view", $item); - return rest::reply($item->as_array()); + $children = array(); + foreach ($item->children() as $child) { + $children[] = url::abs_site("rest/gallery/" . $child->relative_url()); + } + + return rest::reply(array("resource" => $item->as_array(), "members" => $children)); } static function put($request) { diff --git a/modules/tag/helpers/tag_rest.php b/modules/tag/helpers/tag_rest.php index 4b5103ef..c1bbf4fb 100644 --- a/modules/tag/helpers/tag_rest.php +++ b/modules/tag/helpers/tag_rest.php @@ -19,107 +19,79 @@ */ class tag_rest_Core { static function get($request) { - return rest::reply(rest::resolve($request->url)->as_array()); + $tag = rest::resolve($request->url); + $items = array(); + foreach ($tag->items() as $item) { + $items[] = url::abs_site("rest/gallery/" . $item->relative_url()); + } + + return rest::reply(array("resource" => $tag->as_array(), "members" => $items)); } static function post($request) { - $tag = rest::resolve($request->url); - if (empty($request->params->url)) { throw new Rest_Exception("Bad request", 400); } + $tag = rest::resolve($request->url); $item = rest::resolve($request->params->url); - access::required("edit", $item); - tag::add($item, $tag->name); - return rest::reply(); + tag::add($item, $tag->name); + return rest::reply(array("url" => url::abs_site("rest/tag/" . rawurlencode($tag->name)))); } - static function resolve($tag_name) { - $tag = ORM::factory("tag")->where("name", "=", $tag_name)->find(); - if (!$tag->loaded()) { - throw new Kohana_404_Exception(); - } + static function put($request) { + $tag = rest::resolve($request->url); - return $tag; - } + // @todo: what permission should be required to edit a tag? + // for now, require edit at the top level. Perhaps later, just require any edit perms, + // anywhere in the gallery? - // ------------------------------------------------------------ + if (isset($request->params->remove)) { + if (!is_array($request->params->remove)) { + throw new Exception("Bad request", 400); + } - static function put($request) { - if (empty($request->arguments[0]) || empty($request->new_name)) { - throw new Rest_Exception("Bad request", 400); + foreach ($request->params->remove as $item_url) { + $item = rest::resolve($item_url); + access::required("edit", $item); + $tag->remove($item); + } } - $name = $request->arguments[0]; - - $tag = ORM::factory("tag") - ->where("name", "=", $name) - ->find(); - if (!$tag->loaded()) { - throw new Kohana_404_Exception(); + if (isset($request->params->name)) { + $tag->name = $request->params->name; } - $tag->name = $request->new_name; $tag->save(); - - return rest::reply(); + return rest::reply(array("url" => url::abs_site("rest/tag/" . rawurlencode($tag->name)))); } static function delete($request) { - if (empty($request->arguments[0])) { - throw new Rest_Exception("Bad request", 400); - } - $tags = explode(",", $request->arguments[0]); - if (!empty($request->path)) { - $tag_list = ORM::factory("tag") - ->join("items_tags", "tags.id", "items_tags.tag_id") - ->join("items", "items.id", "items_tags.item_id") - ->where("tags.name", "IN", $tags) - ->where("relative_url_cache", "=", $request->path) - ->viewable() - ->find_all(); - } else { - $tag_list = ORM::factory("tag") - ->where("name", "IN", $tags) - ->find_all(); - } + $tag = rest::resolve($request->url); - foreach ($tag_list as $row) { - $row->delete(); - }; + if (empty($request->params->url)) { + // Delete the tag + $tag->delete(); + return rest::reply(); + } else { + // Remove an item from the tag + $item = rest::resolve($request->params->url); + $tag->remove($item); + $tag->save(); - tag::compact(); - return rest::reply(); + tag::compact(); + return rest::reply(array("url" => url::abs_site("rest/tag/" . rawurlencode($tag->name)))); + } } - private static function _get_items($request) { - $tags = explode(",", $request->arguments[0]); - $items = ORM::factory("item") - ->select_distinct("*") - ->join("items_tags", "items.id", "items_tags.item_id") - ->join("tags", "tags.id", "items_tags.tag_id") - ->where("tags.name", "IN", $tags); - if (!empty($request->limit)) { - $items->limit($request->limit); - } - if (!empty($request->offset)) { - $items->offset($request->offset); - } - $resources = array(); - foreach ($items->find_all() as $item) { - $resources[] = array("type" => $item->type, - "has_children" => $item->children_count() > 0, - "path" => $item->relative_url(), - "thumb_url" => $item->thumb_url(true), - "thumb_dimensions" => array("width" => $item->thumb_width, - "height" => $item->thumb_height), - "has_thumb" => $item->has_thumb(), - "title" => $item->title); + static function resolve($tag_name) { + $tag = ORM::factory("tag")->where("name", "=", $tag_name)->find(); + if (!$tag->loaded()) { + throw new Kohana_404_Exception(); } - return $resources; + return $tag; } } diff --git a/modules/tag/helpers/tags_rest.php b/modules/tag/helpers/tags_rest.php index d2bd28b0..3ef897fd 100644 --- a/modules/tag/helpers/tags_rest.php +++ b/modules/tag/helpers/tags_rest.php @@ -19,11 +19,11 @@ */ class tags_rest_Core { static function get($request) { - $data = array(); + $tags = array(); foreach (ORM::factory("tag")->find_all() as $tag) { - $data[$tag->name] = url::abs_site("rest/tags/" . rawurlencode($tag->name)); + $tags[$tag->name] = url::abs_site("rest/tags/" . rawurlencode($tag->name)); } - return rest::reply($data); + return rest::reply(array("members" => $tags)); } static function post($request) { -- cgit v1.2.3 From 14f6e5f6d3933b958fa61b83c627412282610dee Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Fri, 8 Jan 2010 11:12:02 -0800 Subject: Allow the "name" param in get() so that you can restrict the query to children with a given name. --- modules/gallery/helpers/gallery_rest.php | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'modules/gallery/helpers') diff --git a/modules/gallery/helpers/gallery_rest.php b/modules/gallery/helpers/gallery_rest.php index c7b32d8c..fd18d59a 100644 --- a/modules/gallery/helpers/gallery_rest.php +++ b/modules/gallery/helpers/gallery_rest.php @@ -40,8 +40,14 @@ class gallery_rest_Core { $item = rest::resolve($request->url); access::required("view", $item); + if (isset($request->params->name)) { + $where[] = array("name", "=", $request->params->name); + } else { + $where = array(); + } + $children = array(); - foreach ($item->children() as $child) { + foreach ($item->children($where) as $child) { $children[] = url::abs_site("rest/gallery/" . $child->relative_url()); } -- cgit v1.2.3 From 895ac72e706daf8aead624c4cb8d556a2299f73f Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Fri, 8 Jan 2010 11:49:18 -0800 Subject: Add item::random() to return a random Item_Model. --- modules/gallery/helpers/item.php | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) (limited to 'modules/gallery/helpers') diff --git a/modules/gallery/helpers/item.php b/modules/gallery/helpers/item.php index f6181f8a..8098d1cd 100644 --- a/modules/gallery/helpers/item.php +++ b/modules/gallery/helpers/item.php @@ -173,4 +173,34 @@ class item_Core { static function root() { return model_cache::get("item", 1); } + + /** + * Return a random Item_Model, with optional filters + * + * @param array (optional) where tuple + */ + static function random($where=null) { + $random = ((float)mt_rand()) / (float)mt_getrandmax(); + + // Pick a random number and find the item that's got nearest smaller number. In the rare case + // that we chose the smallest number in the system, choose the item with the smallest number. + // This approach works best when the random numbers in the system are roughly evenly + // distributed so this is going to be more efficient with larger data sets. + $random = 0.0; + $items = ORM::factory("item") + ->viewable() + ->where("rand_key", "<", $random) + ->merge_where($where) + ->order_by("rand_key", "DESC") + ->find_all(1); + + if ($items->count() == 0) { + $items = ORM::factory("item") + ->viewable() + ->merge_where($where) + ->order_by("rand_key", "ASC") + ->find_all(1); + } + return $items; + } } \ No newline at end of file -- cgit v1.2.3 From fb65a0a5854812a9837c770dfbb27a23bee49e3d Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Fri, 8 Jan 2010 14:20:15 -0800 Subject: Remove debug code. --- modules/gallery/helpers/item.php | 1 - 1 file changed, 1 deletion(-) (limited to 'modules/gallery/helpers') diff --git a/modules/gallery/helpers/item.php b/modules/gallery/helpers/item.php index 8098d1cd..eb528f8f 100644 --- a/modules/gallery/helpers/item.php +++ b/modules/gallery/helpers/item.php @@ -186,7 +186,6 @@ class item_Core { // that we chose the smallest number in the system, choose the item with the smallest number. // This approach works best when the random numbers in the system are roughly evenly // distributed so this is going to be more efficient with larger data sets. - $random = 0.0; $items = ORM::factory("item") ->viewable() ->where("rand_key", "<", $random) -- cgit v1.2.3 From 9864ab4b2708ec54c39092a21828403cbbd25e2e Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Fri, 8 Jan 2010 14:56:08 -0800 Subject: Move the random image functionality into the gallery REST helper since choosing a random image is essentially a function on an item collection. Also implemented a bunch of other query filters for item collections. Created item::random_query() as a way of generating a reasonable starting point for random queries. --- modules/gallery/helpers/gallery_rest.php | 55 +++++++++++++++++--- modules/gallery/helpers/item.php | 25 +++------ modules/image_block/helpers/image_block_block.php | 2 +- modules/image_block/helpers/image_block_rest.php | 62 ----------------------- 4 files changed, 55 insertions(+), 89 deletions(-) delete mode 100644 modules/image_block/helpers/image_block_rest.php (limited to 'modules/gallery/helpers') diff --git a/modules/gallery/helpers/gallery_rest.php b/modules/gallery/helpers/gallery_rest.php index fd18d59a..0de5da2b 100644 --- a/modules/gallery/helpers/gallery_rest.php +++ b/modules/gallery/helpers/gallery_rest.php @@ -36,22 +36,63 @@ // just like #1 except in a helper instead of in the model? class gallery_rest_Core { + + /** + * For items that are collections, you can specify the following additional query parameters to + * query the collection. You can specify them in any combination. + * + * scope=direct + * only return items that are immediately under this one + * scope=all + * return items anywhere under this one + * + * name= + * only return items where the name contains this substring + * + * random=true + * return a single random item + * + * type= + * limit the type to types in this list. eg, "type=photo,movie" + */ static function get($request) { $item = rest::resolve($request->url); access::required("view", $item); - if (isset($request->params->name)) { - $where[] = array("name", "=", $request->params->name); + $p = $request->params; + if (isset($p->random)) { + $orm = item::random_query()->offset(0)->limit(1); } else { - $where = array(); + $orm = ORM::factory("item")->viewable(); + } + + if (!empty($p->scope) && !in_array($p->scope, array("direct", "all"))) { + throw new Exception("Bad Request", 400); + } + if (!empty($p->scope)) { + if ($p->scope == "direct") { + $orm->where("parent_id", "=", $item->id); + } else { + $orm->where("left_ptr", ">=", $item->left_ptr); + $orm->where("right_ptr", "<=", $item->left_ptr); + $orm->where("id", "<>", $item->id); + } + } + + if (isset($p->name)) { + $orm->where("name", "LIKE", "%{$p->name}%"); + } + + if (isset($p->type)) { + $orm->where("type", "IN", explode(",", $p->type)); } - $children = array(); - foreach ($item->children($where) as $child) { - $children[] = url::abs_site("rest/gallery/" . $child->relative_url()); + $members = array(); + foreach ($orm->find_all() as $child) { + $members[] = url::abs_site("rest/gallery/" . $child->relative_url()); } - return rest::reply(array("resource" => $item->as_array(), "members" => $children)); + return rest::reply(array("resource" => $item->as_array(), "members" => $members)); } static function put($request) { diff --git a/modules/gallery/helpers/item.php b/modules/gallery/helpers/item.php index eb528f8f..1fd9ef16 100644 --- a/modules/gallery/helpers/item.php +++ b/modules/gallery/helpers/item.php @@ -175,31 +175,18 @@ class item_Core { } /** - * Return a random Item_Model, with optional filters + * Return a query to get a random Item_Model, with optional filters * * @param array (optional) where tuple */ - static function random($where=null) { - $random = ((float)mt_rand()) / (float)mt_getrandmax(); - - // Pick a random number and find the item that's got nearest smaller number. In the rare case - // that we chose the smallest number in the system, choose the item with the smallest number. + static function random_query($where=null) { + // Pick a random number and find the item that's got nearest smaller number. // This approach works best when the random numbers in the system are roughly evenly // distributed so this is going to be more efficient with larger data sets. - $items = ORM::factory("item") + return ORM::factory("item") ->viewable() - ->where("rand_key", "<", $random) + ->where("rand_key", "<", ((float)mt_rand()) / (float)mt_getrandmax()) ->merge_where($where) - ->order_by("rand_key", "DESC") - ->find_all(1); - - if ($items->count() == 0) { - $items = ORM::factory("item") - ->viewable() - ->merge_where($where) - ->order_by("rand_key", "ASC") - ->find_all(1); - } - return $items; + ->order_by("rand_key", "DESC"); } } \ No newline at end of file diff --git a/modules/image_block/helpers/image_block_block.php b/modules/image_block/helpers/image_block_block.php index 5f2bbcb7..f28e775f 100644 --- a/modules/image_block/helpers/image_block_block.php +++ b/modules/image_block/helpers/image_block_block.php @@ -30,7 +30,7 @@ class image_block_block_Core { $block->css_id = "g-image-block"; $block->title = t("Random image"); $block->content = new View("image_block_block.html"); - $block->content->items = item::random(array(array("type", "!=", "album"))); + $block->content->items = item::random_query(array(array("type", "!=", "album")))->find_all(1); if ($block->content->items->count() == 0) { $block = ""; diff --git a/modules/image_block/helpers/image_block_rest.php b/modules/image_block/helpers/image_block_rest.php deleted file mode 100644 index 65eefb21..00000000 --- a/modules/image_block/helpers/image_block_rest.php +++ /dev/null @@ -1,62 +0,0 @@ -type) ? "random" : $request->type; - switch ($type) { - case "random": - $random = ((float)mt_rand()) / (float)mt_getrandmax(); - - $items = ORM::factory("item") - ->viewable() - ->where("type", "!=", "album") - ->where("rand_key", "<", $random) - ->order_by(array("rand_key" => "DESC")) - ->find_all(1); - - if ($items->count() == 0) { - // Try once more. If this fails, just ditch the block altogether - $items = ORM::factory("item") - ->viewable() - ->where("type", "!=", "album") - ->where("rand_key", ">= ", $random) - ->order_by(array("rand_key" => "DESC")) - ->find_all(1); - } - break; - default: - return rest::fail("Unsupported image block type: '{$type}'"); - } - - if ($items->count() > 0) { - $item = $items->current(); - $response_data = array("name" => $item->name, - "path" => $item->relative_url(), - "title" => $item->title, - "thumb_url" => $item->thumb_url(true), - "thumb_size" => array("height" => $item->thumb_height, - "width" => $item->thumb_width)); - - return rest::reply(array("resource" => $response_data)); - } else { - return rest::reply(); - } - } -} -- cgit v1.2.3 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 ++++-------- modules/gallery/helpers/album.php | 9 +-- modules/gallery/helpers/item.php | 18 ----- modules/gallery/models/item.php | 139 +++++++++++++++++++++------------ 4 files changed, 106 insertions(+), 111 deletions(-) (limited to 'modules/gallery/helpers') 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)); } } diff --git a/modules/gallery/helpers/album.php b/modules/gallery/helpers/album.php index feaf74cc..477f1945 100644 --- a/modules/gallery/helpers/album.php +++ b/modules/gallery/helpers/album.php @@ -107,7 +107,6 @@ class album_Core { t("The internet address should contain only letters, numbers, hyphens and underscores")); $group->hidden("type")->value("album"); $group->submit("")->value(t("Create")); - $form->add_rules_from(ORM::factory("item")); $form->script("") ->url(url::abs_file("modules/gallery/js/albums_form_add.js")); return $form; @@ -124,15 +123,12 @@ class album_Core { $group->input("dirname")->label(t("Directory Name"))->value($parent->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 directory name can't contain a \"/\"")) - ->callback("item::validate_no_trailing_period") ->error_messages("no_trailing_period", t("The directory name can't end in \".\"")); $group->input("slug")->label(t("Internet Address"))->value($parent->slug) ->error_messages( - "slug_conflict", t("There is already a movie, photo or album with this internet address")) - ->callback("item::validate_url_safe") + "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")); @@ -159,7 +155,6 @@ class album_Core { $group = $form->group("buttons")->label(""); $group->hidden("type")->value("album"); $group->submit("")->value(t("Modify")); - $form->add_rules_from(ORM::factory("item")); return $form; } diff --git a/modules/gallery/helpers/item.php b/modules/gallery/helpers/item.php index 1fd9ef16..53291ccc 100644 --- a/modules/gallery/helpers/item.php +++ b/modules/gallery/helpers/item.php @@ -78,24 +78,6 @@ class item_Core { graphics::generate($album); } - static function validate_no_slashes($input) { - if (strpos($input->value, "/") !== false) { - $input->add_error("no_slashes", 1); - } - } - - static function validate_no_trailing_period($input) { - if (rtrim($input->value, ".") !== $input->value) { - $input->add_error("no_trailing_period", 1); - } - } - - static function validate_url_safe($input) { - if (preg_match("/[^A-Za-z0-9-_]/", $input->value)) { - $input->add_error("not_url_safe", 1); - } - } - /** * Sanitize a filename into something presentable as an item title * @param string $filename diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php index 4a3d26e9..19bdf655 100644 --- a/modules/gallery/models/item.php +++ b/modules/gallery/models/item.php @@ -21,11 +21,11 @@ class Item_Model extends ORM_MPTT { protected $children = 'items'; protected $sorting = array(); - var $form_rules = array( - "name" => "required|length[0,255]", - "title" => "required|length[0,255]", - "description" => "length[0,65535]", - "slug" => "required|length[0,255]" + 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]")) ); /** @@ -146,21 +146,12 @@ class Item_Model extends ORM_MPTT { } /** - * Rename the underlying file for this item to a new name. Move all the files. This requires a - * save. + * Rename the underlying file for this item to a new name and move all related files. * * @chainable */ - public function rename($new_name) { - if ($new_name == $this->name) { - return; - } - - if (strpos($new_name, "/")) { - throw new Exception("@todo NAME_CANNOT_CONTAIN_SLASH"); - } - - $old_relative_path = urldecode($this->relative_path()); + private function rename($new_name) { + $old_relative_path = urldecode($this->original()->relative_path()); $new_relative_path = dirname($old_relative_path) . "/" . $new_name; if (file_exists(VARPATH . "albums/$new_relative_path")) { throw new Exception("@todo INVALID_RENAME_FILE_EXISTS: $new_relative_path"); @@ -178,18 +169,6 @@ class Item_Model extends ORM_MPTT { @rename(VARPATH . "thumbs/$old_relative_path", VARPATH . "thumbs/$new_relative_path"); } - $this->name = $new_name; - - if ($this->is_album()) { - db::build() - ->update("items") - ->set("relative_url_cache", null) - ->set("relative_path_cache", null) - ->where("left_ptr", ">", $this->left_ptr) - ->where("right_ptr", "<", $this->right_ptr) - ->execute(); - } - return $this; } @@ -375,29 +354,6 @@ class Item_Model extends ORM_MPTT { } } - /** - * @see ORM::__set() - */ - public function __set($column, $value) { - if ($column == "name") { - $this->relative_path_cache = null; - } else if ($column == "slug") { - if ($this->slug != $value) { - // Clear the relative url cache for this item and all children - $this->relative_url_cache = null; - if ($this->is_album()) { - db::build() - ->update("items") - ->set("relative_url_cache", null) - ->where("left_ptr", ">", $this->left_ptr) - ->where("right_ptr", "<", $this->right_ptr) - ->execute(); - } - } - } - parent::__set($column, $value); - } - /** * @see ORM::save() */ @@ -414,9 +370,34 @@ class Item_Model extends ORM_MPTT { $this->weight = item::get_max_weight(); } else { $send_event = 1; + + if ($this->original()->name != $this->name) { + $this->rename($this->name); + $this->relative_path_cache = null; + } + + if ($this->original()->slug != $this->slug) { + // Clear the relative url cache for this item and all children + $this->relative_url_cache = null; + } + + // Changing the name or the slug ripples downwards + if ($this->is_album() && + ($this->original()->name != $this->name || + $this->original()->slug != $this->slug)) { + db::build() + ->update("items") + ->set("relative_url_cache", null) + ->set("relative_path_cache", null) + ->where("left_ptr", ">", $this->left_ptr) + ->where("right_ptr", "<", $this->right_ptr) + ->execute(); + } } } + parent::save(); + if (isset($send_event)) { module::event("item_updated", $this->original(), $this); } @@ -655,4 +636,58 @@ class Item_Model extends ORM_MPTT { } return parent::descendants($limit, $offset, $where, $order_by); } + + /** + * Add some custom per-instance rules. + */ + public function validate($array=null) { + 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]"; + } + + // Names and slugs can't conflict + $this->rules["name"]["callbacks"][] = array($this, "valid_name"); + $this->rules["slug"]["callbacks"][] = array($this, "valid_slug"); + } + + 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)) { + $v->add_error("slug", "not_url_safe"); + } else if ($row = db::build() + ->from("items") + ->where("parent_id", "=", $this->parent_id) + ->where("id", "<>", $this->id) + ->where("slug", "=", $this->slug) + ->count_records()) { + $v->add_error("slug", "conflict"); + } + } + + /** + * 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) { + $v->add_error("name", "no_slashes"); + } else if (rtrim($value, ".") !== $value) { + $v->add_error("name", "no_trailing_period"); + } else if ($row = db::build() + ->from("items") + ->where("parent_id", "=", $this->parent_id) + ->where("id", "<>", $this->id) + ->where("name", "=", $this->name) + ->count_records()) { + $v->add_error("name", "conflict"); + } + } } -- cgit v1.2.3 From 94f58e8b65b78cafed8f07f70a48b7b271cfc212 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Fri, 15 Jan 2010 10:48:39 -0800 Subject: Move setting Item_Model::rand_key into Item_Model::save() since it's business logic. --- modules/gallery/helpers/album.php | 1 - modules/gallery/helpers/movie.php | 1 - modules/gallery/helpers/photo.php | 1 - modules/gallery/models/item.php | 1 + 4 files changed, 1 insertion(+), 3 deletions(-) (limited to 'modules/gallery/helpers') diff --git a/modules/gallery/helpers/album.php b/modules/gallery/helpers/album.php index 477f1945..52759414 100644 --- a/modules/gallery/helpers/album.php +++ b/modules/gallery/helpers/album.php @@ -61,7 +61,6 @@ class album_Core { $album->thumb_dirty = 1; $album->resize_dirty = 1; $album->slug = $slug; - $album->rand_key = ((float)mt_rand()) / (float)mt_getrandmax(); $album->sort_column = "created"; $album->sort_order = "ASC"; diff --git a/modules/gallery/helpers/movie.php b/modules/gallery/helpers/movie.php index 01859924..b0d24f68 100644 --- a/modules/gallery/helpers/movie.php +++ b/modules/gallery/helpers/movie.php @@ -85,7 +85,6 @@ class movie_Core { $movie->resize_dirty = 1; $movie->sort_column = "weight"; $movie->slug = $slug; - $movie->rand_key = ((float)mt_rand()) / (float)mt_getrandmax(); // Randomize the name if there's a conflict // @todo Improve this. Random numbers are not user friendly diff --git a/modules/gallery/helpers/photo.php b/modules/gallery/helpers/photo.php index 4e20e610..aeae7f56 100644 --- a/modules/gallery/helpers/photo.php +++ b/modules/gallery/helpers/photo.php @@ -84,7 +84,6 @@ class photo_Core { $photo->resize_dirty = 1; $photo->sort_column = "weight"; $photo->slug = $slug; - $photo->rand_key = ((float)mt_rand()) / (float)mt_getrandmax(); // Randomize the name or slug if there's a conflict // @todo Improve this. Random numbers are not user friendly diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php index 9edc65ce..33b36ff1 100644 --- a/modules/gallery/models/item.php +++ b/modules/gallery/models/item.php @@ -368,6 +368,7 @@ class Item_Model extends ORM_MPTT { if (!$this->loaded()) { $this->created = $this->updated; $this->weight = item::get_max_weight(); + $this->rand_key = ((float)mt_rand()) / (float)mt_getrandmax(); } else { $send_event = 1; -- 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/helpers') 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/helpers') 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/helpers') 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 b5a6a6a5d5741f592d504b8a444899964101b6b6 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Sat, 16 Jan 2010 11:44:21 -0800 Subject: Oops, log::failure() doesn't exist. Use log::error(). --- modules/gallery/helpers/gallery_event.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'modules/gallery/helpers') diff --git a/modules/gallery/helpers/gallery_event.php b/modules/gallery/helpers/gallery_event.php index 9452e855..db3b34fe 100644 --- a/modules/gallery/helpers/gallery_event.php +++ b/modules/gallery/helpers/gallery_event.php @@ -79,7 +79,9 @@ class gallery_event_Core { try { graphics::generate($item); } catch (Exception $e) { - log::failure("Unable to create a thumbnail for item id {$item->id}"); + log::error("graphics", t("Couldn't create a thumbnail or resize for %item_title", + array("item_title" => $item->title)), + html::anchor($item->abs_url(), t("details"))); Kohana_Log::add("error", $e->getMessage() . "\n" . $e->getTraceAsString()); } -- 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/helpers') 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/helpers') 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 cfb27dde023e4f4d04fc9de687548501e607d371 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Sun, 17 Jan 2010 13:28:24 -0800 Subject: Adjust installers to work with model based validation. --- modules/gallery/helpers/gallery_installer.php | 33 ++++++++++++++++----------- modules/user/helpers/user_installer.php | 32 ++++++++++++++++++++------ 2 files changed, 45 insertions(+), 20 deletions(-) (limited to 'modules/gallery/helpers') diff --git a/modules/gallery/helpers/gallery_installer.php b/modules/gallery/helpers/gallery_installer.php index 1e0ad28c..aa297236 100644 --- a/modules/gallery/helpers/gallery_installer.php +++ b/modules/gallery/helpers/gallery_installer.php @@ -209,19 +209,26 @@ class gallery_installer { t("Edit"); t("Add"); - $root = ORM::factory("item"); - $root->type = "album"; - $root->title = "Gallery"; - $root->description = ""; - $root->left_ptr = 1; - $root->right_ptr = 2; - $root->parent_id = 0; - $root->level = 1; - $root->thumb_dirty = 1; - $root->resize_dirty = 1; - $root->sort_column = "weight"; - $root->sort_order = "ASC"; - $root->save(); + // Hardcode the first item to sidestep ORM validation rules + $now = time(); + db::build()->insert( + "items", + array("created" => $now, + "description" => "", + "left_ptr" => 1, + "level" => 1, + "parent_id" => 0, + "resize_dirty" => 1, + "right_ptr" => 2, + "sort_column" => "weight", + "sort_order" => "ASC", + "thumb_dirty" => 1, + "title" => "Gallery", + "type" => "album", + "updated" => $now, + "weight" => 1)) + ->execute(); + $root = ORM::factory("item")->where("id", "=", 1)->find(); access::add_item($root); module::set_var("gallery", "active_site_theme", "wind"); diff --git a/modules/user/helpers/user_installer.php b/modules/user/helpers/user_installer.php index 0cba502f..70bee300 100644 --- a/modules/user/helpers/user_installer.php +++ b/modules/user/helpers/user_installer.php @@ -53,21 +53,39 @@ class user_installer { UNIQUE KEY(`user_id`, `group_id`)) DEFAULT CHARSET=utf8;"); - $everybody = group::create("Everybody"); + $everybody = ORM::factory("group"); + $everybody->name = "Everybody"; $everybody->special = true; $everybody->save(); - $registered = group::create("Registered Users"); + $registered = ORM::factory("group"); + $registered->name = "Registered Users"; $registered->special = true; $registered->save(); - $guest = user::create("guest", "Guest User", ""); - $guest->guest = true; - $guest->remove($registered); + // Avoid ORM to sidestep validation. + db::build()->insert( + "users", + array("name" => "guest", + "full_name" => "Guest User", + "guest" => true)) + ->execute(); + + $guest = ORM::factory("user")->where("id", "=", 1)->find(); + $guest->add($everybody); $guest->save(); - $admin = user::create("admin", "Gallery Administrator", "admin"); - $admin->admin = true; + db::build()->insert( + "users", + array("name" => "admin", + "full_name" => "Gallery Administrator", + "password" => "admin", + "admin" => true)) + ->execute(); + + $admin = ORM::factory("user")->where("id", "=", 2)->find(); + $admin->add($everybody); + $admin->add($registered); $admin->save(); $current_provider = module::get_var("gallery", "identity_provider"); -- cgit v1.2.3 From fafa7f277f0591c74bd3d162c4c39a01604b55ae Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Sun, 17 Jan 2010 16:55:48 -0800 Subject: Remove a @todo. --- modules/gallery/helpers/gallery_rest.php | 40 +++++++++++++++++++------------- 1 file changed, 24 insertions(+), 16 deletions(-) (limited to 'modules/gallery/helpers') diff --git a/modules/gallery/helpers/gallery_rest.php b/modules/gallery/helpers/gallery_rest.php index 0de5da2b..24733f20 100644 --- a/modules/gallery/helpers/gallery_rest.php +++ b/modules/gallery/helpers/gallery_rest.php @@ -19,7 +19,6 @@ */ // @todo Add logging -// @todo VALIDATION // Validation questions // @@ -100,10 +99,15 @@ class gallery_rest_Core { access::required("edit", $item); $params = $request->params; - foreach (array("captured", "description", "slug", "sort_column", "sort_order", - "title", "view_count", "weight") as $key) { - if (isset($params->$key)) { - $item->$key = $params->$key; + + // Only change fields from a whitelist. + foreach (array("album_cover_item_id", "captured", "description", + "height", "mime_type", "name", "parent_id", "rand_key", "resize_dirty", + "resize_height", "resize_width", "slug", "sort_column", "sort_order", + "thumb_dirty", "thumb_height", "thumb_width", "title", "view_count", + "weight", "width") as $key) { + if (array_key_exists($key, $request->params)) { + $item->$key = $request->params->$key; } } $item->save(); @@ -116,22 +120,26 @@ class gallery_rest_Core { access::required("edit", $parent); $params = $request->params; + $item = ORM::factory("item"); switch ($params->type) { case "album": - $item = album::create( - $parent, - $params->name, - isset($params->title) ? $params->title : $name, - isset($params->description) ? $params->description : null); + $item->type = "album"; + $item->parent_id = $parent->id; + $item->name = $params->name; + $item->title = isset($params->title) ? $params->title : $name; + $item->description = isset($params->description) ? $params->description : null; + $item->save(); break; case "photo": - $item = photo::create( - $parent, - $request->file, - $params->name, - isset($params->title) ? $params->title : $name, - isset($params->description) ? $params->description : null); + case "movie": + $item->type = $params->type; + $item->parent_id = $parent->id; + $item->set_data_file($request->file); + $item->name = $params->name; + $item->title = isset($params->title) ? $params->title : $name; + $item->description = isset($params->description) ? $params->description : null; + $item->save(); break; default: -- cgit v1.2.3 From f20fa2cfedc42f98a38a77d77186d180bd0c3426 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Sun, 17 Jan 2010 20:37:25 -0800 Subject: Change IdentityProvider::create_user() to take $email as well, since that's a required parameter for the Gallery driver. --- modules/gallery/helpers/identity.php | 4 ++-- modules/gallery/libraries/IdentityProvider.php | 4 ++-- modules/gallery/libraries/drivers/IdentityProvider.php | 3 ++- modules/user/libraries/drivers/IdentityProvider/Gallery.php | 3 ++- 4 files changed, 8 insertions(+), 6 deletions(-) (limited to 'modules/gallery/helpers') diff --git a/modules/gallery/helpers/identity.php b/modules/gallery/helpers/identity.php index eae0ea3e..ef93d72f 100644 --- a/modules/gallery/helpers/identity.php +++ b/modules/gallery/helpers/identity.php @@ -155,8 +155,8 @@ class identity_Core { /** * @see IdentityProvider_Driver::create_user. */ - static function create_user($name, $full_name, $password) { - return IdentityProvider::instance()->create_user($name, $full_name, $password); + static function create_user($name, $full_name, $password, $email) { + return IdentityProvider::instance()->create_user($name, $full_name, $password, $email); } /** diff --git a/modules/gallery/libraries/IdentityProvider.php b/modules/gallery/libraries/IdentityProvider.php index bcb3056a..30d4efa4 100644 --- a/modules/gallery/libraries/IdentityProvider.php +++ b/modules/gallery/libraries/IdentityProvider.php @@ -119,8 +119,8 @@ class IdentityProvider_Core { /** * @see IdentityProvider_Driver::create_user. */ - public function create_user($name, $full_name, $password) { - return $this->driver->create_user($name, $full_name, $password); + public function create_user($name, $full_name, $password, $email) { + return $this->driver->create_user($name, $full_name, $password, $email); } /** diff --git a/modules/gallery/libraries/drivers/IdentityProvider.php b/modules/gallery/libraries/drivers/IdentityProvider.php index a808c7e8..b7b1fbe8 100644 --- a/modules/gallery/libraries/drivers/IdentityProvider.php +++ b/modules/gallery/libraries/drivers/IdentityProvider.php @@ -38,9 +38,10 @@ interface IdentityProvider_Driver { * @param string $name * @param string $full_name * @param string $password + * @param string $email * @return User_Definition the user object */ - public function create_user($name, $full_name, $password); + public function create_user($name, $full_name, $password, $email); /** * Is the password provided correct? diff --git a/modules/user/libraries/drivers/IdentityProvider/Gallery.php b/modules/user/libraries/drivers/IdentityProvider/Gallery.php index 9927ea33..666f185f 100644 --- a/modules/user/libraries/drivers/IdentityProvider/Gallery.php +++ b/modules/user/libraries/drivers/IdentityProvider/Gallery.php @@ -38,11 +38,12 @@ class IdentityProvider_Gallery_Driver implements IdentityProvider_Driver { /** * @see IdentityProvider_Driver::create_user. */ - public function create_user($name, $full_name, $password) { + public function create_user($name, $full_name, $password, $email) { $user = ORM::factory("user"); $user->name = $name; $user->full_name = $full_name; $user->password = $password; + $user->email = $email; return $user->save(); } -- cgit v1.2.3 From cb7c263b470bd1452c47a3e29d67373869150d2c Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Tue, 19 Jan 2010 00:36:40 -0800 Subject: Return arrays instead of calling rest::reply. --- modules/gallery/helpers/gallery_rest.php | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) (limited to 'modules/gallery/helpers') diff --git a/modules/gallery/helpers/gallery_rest.php b/modules/gallery/helpers/gallery_rest.php index 24733f20..7f93bd38 100644 --- a/modules/gallery/helpers/gallery_rest.php +++ b/modules/gallery/helpers/gallery_rest.php @@ -91,7 +91,7 @@ class gallery_rest_Core { $members[] = url::abs_site("rest/gallery/" . $child->relative_url()); } - return rest::reply(array("resource" => $item->as_array(), "members" => $members)); + return array("resource" => $item->as_array(), "members" => $members); } static function put($request) { @@ -112,7 +112,7 @@ class gallery_rest_Core { } $item->save(); - return rest::reply(array("url" => url::abs_site("/rest/gallery/" . $item->relative_url()))); + return array("url" => url::abs_site("/rest/gallery/" . $item->relative_url())); } static function post($request) { @@ -146,7 +146,7 @@ class gallery_rest_Core { throw new Rest_Exception("Invalid type: $args->type", 400); } - return rest::reply(array("url" => url::abs_site("/rest/gallery/" . $item->relative_url()))); + return array("url" => url::abs_site("/rest/gallery/" . $item->relative_url())); } static function delete($request) { @@ -154,7 +154,6 @@ class gallery_rest_Core { access::required("edit", $item); $item->delete(); - return rest::reply(); } static function resolve($path) { -- cgit v1.2.3 From a587426cfda4a0cd4e8f3f53607c8e3ad2305506 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Tue, 19 Jan 2010 01:12:09 -0800 Subject: Don't try to set the album cover for the grandparent if we don't have edit permissions for it. --- modules/gallery/helpers/item.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'modules/gallery/helpers') diff --git a/modules/gallery/helpers/item.php b/modules/gallery/helpers/item.php index 53291ccc..7821e628 100644 --- a/modules/gallery/helpers/item.php +++ b/modules/gallery/helpers/item.php @@ -59,7 +59,7 @@ class item_Core { $parent->save(); graphics::generate($parent); $grand_parent = $parent->parent(); - if ($grand_parent && $grand_parent->album_cover_item_id == null) { + if (access::can("edit", $grand_parent) && $grand_parent->album_cover_item_id == null) { item::make_album_cover($parent); } } -- cgit v1.2.3 From 069a23e81192578f5f02f6a52a07536ceb6c0bcd Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Tue, 19 Jan 2010 01:16:59 -0800 Subject: Make scope default to direct. Add slug to the post params. Fix minor output bug. --- modules/gallery/helpers/gallery_rest.php | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) (limited to 'modules/gallery/helpers') diff --git a/modules/gallery/helpers/gallery_rest.php b/modules/gallery/helpers/gallery_rest.php index 7f93bd38..827da122 100644 --- a/modules/gallery/helpers/gallery_rest.php +++ b/modules/gallery/helpers/gallery_rest.php @@ -65,17 +65,19 @@ class gallery_rest_Core { $orm = ORM::factory("item")->viewable(); } - if (!empty($p->scope) && !in_array($p->scope, array("direct", "all"))) { + if (empty($p->scope)) { + $p->scope = "direct"; + } + + if (!in_array($p->scope, array("direct", "all"))) { throw new Exception("Bad Request", 400); } - if (!empty($p->scope)) { - if ($p->scope == "direct") { - $orm->where("parent_id", "=", $item->id); - } else { - $orm->where("left_ptr", ">=", $item->left_ptr); - $orm->where("right_ptr", "<=", $item->left_ptr); - $orm->where("id", "<>", $item->id); - } + + if ($p->scope == "direct") { + $orm->where("parent_id", "=", $item->id); + } else { + $orm->where("left_ptr", ">", $item->left_ptr); + $orm->where("right_ptr", "<", $item->right_ptr); } if (isset($p->name)) { @@ -128,6 +130,7 @@ class gallery_rest_Core { $item->name = $params->name; $item->title = isset($params->title) ? $params->title : $name; $item->description = isset($params->description) ? $params->description : null; + $item->slug = isset($params->slug) ? $params->slug : null; $item->save(); break; @@ -137,13 +140,14 @@ class gallery_rest_Core { $item->parent_id = $parent->id; $item->set_data_file($request->file); $item->name = $params->name; - $item->title = isset($params->title) ? $params->title : $name; + $item->title = isset($params->title) ? $params->title : $params->name; $item->description = isset($params->description) ? $params->description : null; + $item->slug = isset($params->slug) ? $params->slug : null; $item->save(); break; default: - throw new Rest_Exception("Invalid type: $args->type", 400); + throw new Rest_Exception("Invalid type: $params->type", 400); } return array("url" => url::abs_site("/rest/gallery/" . $item->relative_url())); -- cgit v1.2.3 From c590fed132b07647c38b1d5b4a93ffe30b6ac4bf Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Tue, 19 Jan 2010 01:33:57 -0800 Subject: Change rest::url() to take a module name and a resource. The module does the rest. This function is symmetrical to rest::resolve. --- modules/gallery/helpers/gallery_rest.php | 28 ++++--------- modules/gallery/tests/Gallery_Rest_Helper_Test.php | 46 +++++++++++----------- modules/rest/helpers/rest.php | 12 ++++-- modules/tag/helpers/tag_rest.php | 15 ++++--- modules/tag/helpers/tags_rest.php | 6 +-- 5 files changed, 50 insertions(+), 57 deletions(-) (limited to 'modules/gallery/helpers') diff --git a/modules/gallery/helpers/gallery_rest.php b/modules/gallery/helpers/gallery_rest.php index 827da122..5fd73a2e 100644 --- a/modules/gallery/helpers/gallery_rest.php +++ b/modules/gallery/helpers/gallery_rest.php @@ -17,25 +17,7 @@ * along with this program; if not, write to the Free Software * Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, MA 02110-1301, USA. */ - -// @todo Add logging - -// Validation questions -// -// We need to be able to properly validate anything we want to enter here. But all of our -// validation currently happens at the controller / form level, and we're not using the same -// controllers or forms. -// -// Possible solutions: -// 1) Move validation into the model and use it both here and in the regular controllers. But -// if we do that, how do we translate validation failures into a user-consumable output which -// we need so that we can return proper error responses to form submissions? -// -// 2) Create some kind of validation helper that can validate every field. Wait, isn't this -// just like #1 except in a helper instead of in the model? - class gallery_rest_Core { - /** * For items that are collections, you can specify the following additional query parameters to * query the collection. You can specify them in any combination. @@ -90,7 +72,7 @@ class gallery_rest_Core { $members = array(); foreach ($orm->find_all() as $child) { - $members[] = url::abs_site("rest/gallery/" . $child->relative_url()); + $members[] = rest::url("gallery", $child); } return array("resource" => $item->as_array(), "members" => $members); @@ -114,7 +96,7 @@ class gallery_rest_Core { } $item->save(); - return array("url" => url::abs_site("/rest/gallery/" . $item->relative_url())); + return array("url" => rest::url("gallery", $item)); } static function post($request) { @@ -150,7 +132,7 @@ class gallery_rest_Core { throw new Rest_Exception("Invalid type: $params->type", 400); } - return array("url" => url::abs_site("/rest/gallery/" . $item->relative_url())); + return array("url" => rest::url("gallery", $item)); } static function delete($request) { @@ -163,4 +145,8 @@ class gallery_rest_Core { static function resolve($path) { return url::get_item_from_uri($path); } + + static function url($item) { + return url::abs_site("rest/gallery/" . $item->relative_url()); + } } diff --git a/modules/gallery/tests/Gallery_Rest_Helper_Test.php b/modules/gallery/tests/Gallery_Rest_Helper_Test.php index 35fd0daf..dcd9a9db 100644 --- a/modules/gallery/tests/Gallery_Rest_Helper_Test.php +++ b/modules/gallery/tests/Gallery_Rest_Helper_Test.php @@ -28,7 +28,7 @@ class Gallery_Rest_Helper_Test extends Gallery_Unit_Test_Case { public function resolve_test() { $album = test::random_album(); - $resolved = rest::resolve(rest::url("gallery", $album->relative_url())); + $resolved = rest::resolve(rest::url("gallery", $album)); $this->assert_equal($album->id, $resolved->id); } @@ -40,32 +40,32 @@ class Gallery_Rest_Helper_Test extends Gallery_Unit_Test_Case { $album1->reload(); // No scope is the same as "direct" - $request->url = rest::url("gallery", $album1->relative_url()); + $request->url = rest::url("gallery", $album1); $request->params = new stdClass(); $this->assert_equal_array( array("resource" => $album1->as_array(), "members" => array( - rest::url("gallery", $photo1->relative_url()), - rest::url("gallery", $album2->relative_url()))), + rest::url("gallery", $photo1), + rest::url("gallery", $album2))), gallery_rest::get($request)); - $request->url = rest::url("gallery", $album1->relative_url()); + $request->url = rest::url("gallery", $album1); $request->params->scope = "direct"; $this->assert_equal_array( array("resource" => $album1->as_array(), "members" => array( - rest::url("gallery", $photo1->relative_url()), - rest::url("gallery", $album2->relative_url()))), + rest::url("gallery", $photo1), + rest::url("gallery", $album2))), gallery_rest::get($request)); - $request->url = rest::url("gallery", $album1->relative_url()); + $request->url = rest::url("gallery", $album1); $request->params->scope = "all"; $this->assert_equal_array( array("resource" => $album1->as_array(), "members" => array( - rest::url("gallery", $photo1->relative_url()), - rest::url("gallery", $album2->relative_url()), - rest::url("gallery", $photo2->relative_url()))), + rest::url("gallery", $photo1), + rest::url("gallery", $album2), + rest::url("gallery", $photo2))), gallery_rest::get($request)); } @@ -77,12 +77,12 @@ class Gallery_Rest_Helper_Test extends Gallery_Unit_Test_Case { $photo2->save(); $album1->reload(); - $request->url = rest::url("gallery", $album1->relative_url()); + $request->url = rest::url("gallery", $album1); $request->params->name = "foo"; $this->assert_equal_array( array("resource" => $album1->as_array(), "members" => array( - rest::url("gallery", $photo2->relative_url()))), + rest::url("gallery", $photo2))), gallery_rest::get($request)); } @@ -92,12 +92,12 @@ class Gallery_Rest_Helper_Test extends Gallery_Unit_Test_Case { $album2 = test::random_album($album1); $album1->reload(); - $request->url = rest::url("gallery", $album1->relative_url()); + $request->url = rest::url("gallery", $album1); $request->params->type = "album"; $this->assert_equal_array( array("resource" => $album1->as_array(), "members" => array( - rest::url("gallery", $album2->relative_url()))), + rest::url("gallery", $album2))), gallery_rest::get($request)); } @@ -105,11 +105,11 @@ class Gallery_Rest_Helper_Test extends Gallery_Unit_Test_Case { $album1 = test::random_album(); access::allow(identity::everybody(), "edit", $album1); - $request->url = rest::url("gallery", $album1->relative_url()); + $request->url = rest::url("gallery", $album1); $request->params->title = "my new title"; $this->assert_equal_array( - array("url" => rest::url("gallery", $album1->relative_url())), + array("url" => rest::url("gallery", $album1)), gallery_rest::put($request)); $this->assert_equal("my new title", $album1->reload()->title); } @@ -118,7 +118,7 @@ class Gallery_Rest_Helper_Test extends Gallery_Unit_Test_Case { $album1 = test::random_album(); access::allow(identity::everybody(), "edit", $album1); - $request->url = rest::url("gallery", $album1->relative_url()); + $request->url = rest::url("gallery", $album1); $request->params->title = "my new title"; $request->params->slug = "not url safe"; @@ -135,7 +135,7 @@ class Gallery_Rest_Helper_Test extends Gallery_Unit_Test_Case { $album1 = test::random_album(); access::allow(identity::everybody(), "edit", $album1); - $request->url = rest::url("gallery", $album1->relative_url()); + $request->url = rest::url("gallery", $album1); $request->params->type = "album"; $request->params->name = "my album"; $request->params->title = "my album"; @@ -150,7 +150,7 @@ class Gallery_Rest_Helper_Test extends Gallery_Unit_Test_Case { $album1 = test::random_album(); access::allow(identity::everybody(), "edit", $album1); - $request->url = rest::url("gallery", $album1->relative_url()); + $request->url = rest::url("gallery", $album1); $request->params->type = "album"; $request->params->name = "my album"; $request->params->title = "my album"; @@ -170,7 +170,7 @@ class Gallery_Rest_Helper_Test extends Gallery_Unit_Test_Case { $album1 = test::random_album(); access::allow(identity::everybody(), "edit", $album1); - $request->url = rest::url("gallery", $album1->relative_url()); + $request->url = rest::url("gallery", $album1); $request->params->type = "photo"; $request->params->name = "my photo.jpg"; $request->file = MODPATH . "gallery/tests/test.jpg"; @@ -185,7 +185,7 @@ class Gallery_Rest_Helper_Test extends Gallery_Unit_Test_Case { $album1 = test::random_album(); access::allow(identity::everybody(), "edit", $album1); - $request->url = rest::url("gallery", $album1->relative_url()); + $request->url = rest::url("gallery", $album1); gallery_rest::delete($request); $album1->reload(); @@ -195,7 +195,7 @@ class Gallery_Rest_Helper_Test extends Gallery_Unit_Test_Case { public function delete_album_fails_without_permission_test() { $album1 = test::random_album(); - $request->url = rest::url("gallery", $album1->relative_url()); + $request->url = rest::url("gallery", $album1); try { gallery_rest::delete($request); } catch (Exception $e) { diff --git a/modules/rest/helpers/rest.php b/modules/rest/helpers/rest.php index 423765bb..93ad2bd3 100644 --- a/modules/rest/helpers/rest.php +++ b/modules/rest/helpers/rest.php @@ -75,10 +75,14 @@ class rest_Core { /** * Return an absolute url used for REST resource location. * @param string module name (eg, "gallery", "tags") - * @param string relative path (eg "Family/Weddings.jpg") - * @return string complete url + * @param object resource */ - static function url($module, $path) { - return url::abs_site("rest/$module/$path"); + static function url($module, $resource) { + $class = "{$module}_rest"; + if (!method_exists($class, "url")) { + throw new Exception("@todo MISSING REST CLASS: $class"); + } + + return call_user_func(array($class, "url"), $resource); } } diff --git a/modules/tag/helpers/tag_rest.php b/modules/tag/helpers/tag_rest.php index 0aac5291..a4eaee90 100644 --- a/modules/tag/helpers/tag_rest.php +++ b/modules/tag/helpers/tag_rest.php @@ -22,10 +22,10 @@ class tag_rest_Core { $tag = rest::resolve($request->url); $items = array(); foreach ($tag->items() as $item) { - $items[] = url::abs_site("rest/gallery/" . $item->relative_url()); + $items[] = rest::url("gallery", $item); } - return rest::reply(array("resource" => $tag->as_array(), "members" => $items)); + return array("resource" => $tag->as_array(), "members" => $items); } static function post($request) { @@ -38,7 +38,7 @@ class tag_rest_Core { access::required("edit", $item); tag::add($item, $tag->name); - return rest::reply(array("url" => url::abs_site("rest/tag/" . rawurlencode($tag->name)))); + return array("url" => rest::url("tag", $tag)); } static function put($request) { @@ -61,7 +61,7 @@ class tag_rest_Core { } $tag->save(); - return rest::reply(array("url" => url::abs_site("rest/tag/" . rawurlencode($tag->name)))); + return array("url" => rest::url("tag", $tag)); } static function delete($request) { @@ -70,7 +70,6 @@ class tag_rest_Core { if (empty($request->params->url)) { // Delete the tag $tag->delete(); - return rest::reply(); } else { // Remove an item from the tag $item = rest::resolve($request->params->url); @@ -78,7 +77,7 @@ class tag_rest_Core { $tag->save(); tag::compact(); - return rest::reply(array("url" => url::abs_site("rest/tag/" . rawurlencode($tag->name)))); + return array("url" => rest::url("tag", $tag)); } } @@ -90,4 +89,8 @@ class tag_rest_Core { return $tag; } + + static function url($item) { + return url::abs_site("rest/tag/" . rawurlencode($tag->name)); + } } diff --git a/modules/tag/helpers/tags_rest.php b/modules/tag/helpers/tags_rest.php index 7f0ed66a..dd23e97f 100644 --- a/modules/tag/helpers/tags_rest.php +++ b/modules/tag/helpers/tags_rest.php @@ -21,9 +21,9 @@ class tags_rest_Core { static function get($request) { $tags = array(); foreach (ORM::factory("tag")->find_all() as $tag) { - $tags[$tag->name] = url::abs_site("rest/tags/" . rawurlencode($tag->name)); + $tags[$tag->name] = rest::url("tags", $tag); } - return rest::reply(array("members" => $tags)); + return array("members" => $tags); } static function post($request) { @@ -43,6 +43,6 @@ class tags_rest_Core { $tag->save(); } - return rest::reply(array("url" => url::abs_site("rest/tag/" . rawurlencode($tag->name)))); + return array("url" => rest::url("tag", $tag)); } } -- cgit v1.2.3 From c3ed64fc6c0c1c9001d32191b0f1a5c21ec4b7c5 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Tue, 19 Jan 2010 01:46:45 -0800 Subject: Use property_exists() on our stdClass instead of array_key_exists() --- modules/gallery/helpers/gallery_rest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'modules/gallery/helpers') diff --git a/modules/gallery/helpers/gallery_rest.php b/modules/gallery/helpers/gallery_rest.php index 5fd73a2e..49096100 100644 --- a/modules/gallery/helpers/gallery_rest.php +++ b/modules/gallery/helpers/gallery_rest.php @@ -90,7 +90,7 @@ class gallery_rest_Core { "resize_height", "resize_width", "slug", "sort_column", "sort_order", "thumb_dirty", "thumb_height", "thumb_width", "title", "view_count", "weight", "width") as $key) { - if (array_key_exists($key, $request->params)) { + if (property_exists($request->params, $key)) { $item->$key = $request->params->$key; } } -- 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/helpers') 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/helpers') 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 b5cf24456f4868a0e553af389e1b482984bc8a86 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Wed, 20 Jan 2010 00:51:34 -0800 Subject: Forbidden is a 403, not a 503. --- modules/gallery/helpers/access.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'modules/gallery/helpers') diff --git a/modules/gallery/helpers/access.php b/modules/gallery/helpers/access.php index e0a0e979..2cfaa947 100644 --- a/modules/gallery/helpers/access.php +++ b/modules/gallery/helpers/access.php @@ -183,10 +183,10 @@ class access_Core { } /** - * Terminate immediately with an HTTP 503 Forbidden response. + * Terminate immediately with an HTTP 403 Forbidden response. */ static function forbidden() { - throw new Exception("@todo FORBIDDEN", 503); + throw new Exception("@todo FORBIDDEN", 403); } /** -- cgit v1.2.3 From 210e02f0001489cdfa22da2fb57d6db08954aef3 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Wed, 20 Jan 2010 21:13:58 -0800 Subject: Throw Rest exceptions, not regular exceptions. --- modules/gallery/helpers/gallery_rest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'modules/gallery/helpers') diff --git a/modules/gallery/helpers/gallery_rest.php b/modules/gallery/helpers/gallery_rest.php index 49096100..c5838ea5 100644 --- a/modules/gallery/helpers/gallery_rest.php +++ b/modules/gallery/helpers/gallery_rest.php @@ -52,7 +52,7 @@ class gallery_rest_Core { } if (!in_array($p->scope, array("direct", "all"))) { - throw new Exception("Bad Request", 400); + throw new Rest_Exception("Bad Request", 400); } if ($p->scope == "direct") { -- cgit v1.2.3 From 00957f79bab42b2323b9fe52425b1e0ed51137ac Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Wed, 20 Jan 2010 22:46:46 -0800 Subject: Throw Kohana_Exception instead of Exception on access denied, since that may bubble all the way up --- modules/gallery/helpers/access.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'modules/gallery/helpers') diff --git a/modules/gallery/helpers/access.php b/modules/gallery/helpers/access.php index 2cfaa947..10fa8666 100644 --- a/modules/gallery/helpers/access.php +++ b/modules/gallery/helpers/access.php @@ -186,7 +186,7 @@ class access_Core { * Terminate immediately with an HTTP 403 Forbidden response. */ static function forbidden() { - throw new Exception("@todo FORBIDDEN", 403); + throw new Kohana_Exception("@todo FORBIDDEN", null, 403); } /** -- cgit v1.2.3 From 16ccda0f3d63b5db43c5eaee551ecc62c42becf5 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Wed, 20 Jan 2010 23:49:10 -0800 Subject: Two fixes: 1) Don't call ORM_MPTT::move_to() directly. Use the new model-based-validation approach of changing the parent_id and saving. 2) Item_Model::parent() can return null; check for it. --- modules/gallery/helpers/item.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'modules/gallery/helpers') diff --git a/modules/gallery/helpers/item.php b/modules/gallery/helpers/item.php index 7821e628..41d49ce9 100644 --- a/modules/gallery/helpers/item.php +++ b/modules/gallery/helpers/item.php @@ -39,7 +39,8 @@ class item_Core { } } - $source->move_to($target); + $source->parent_id = $target->id; + $source->save(); // If the target has no cover item, make this it. if ($target->album_cover_item_id == null) { @@ -59,7 +60,8 @@ class item_Core { $parent->save(); graphics::generate($parent); $grand_parent = $parent->parent(); - if (access::can("edit", $grand_parent) && $grand_parent->album_cover_item_id == null) { + if ($grand_parent && access::can("edit", $grand_parent) && + $grand_parent->album_cover_item_id == null) { item::make_album_cover($parent); } } -- cgit v1.2.3 From bcf1caad1459a458a7923335a4a6bc521816de40 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Fri, 22 Jan 2010 00:27:00 -0800 Subject: Reshape the rest code to be more consistent with regards to relationships. Now when you view a resource, it has 4 top level elements: url: the url of this resource resource: array of key value pairs describing the resource members: array of urls to members of this collection relationships: array of array of members. Relationships are a special type of collection that links two different resources together. To remove a relationship, just DELETE its url. To create a relationship, POST to its collection. Individual modules can add their own relationships to any resource via a callback mechanism. Example: Array( [url] => http://g3.com/rest/item/1 [resource] => Array ( [id] => 1 [album_cover_item_id] => 4 [captured] => [created] => 1264056417 [description] => [height] => ... ) [members] => Array( [0] => http://g3.com/rest/item/2 [1] => http://g3.com/rest/item/3 [2] => http://g3.com/rest/item/4 [3] => http://g3.com/rest/item/5 ... ) [relationships] => Array( [tags] => Array ( [0] => http://g3.com/rest/tag_item/2,1 [1] => http://g3.com/rest/tag_item/23,1 ) ) ) --- modules/gallery/helpers/gallery_rest.php | 152 ----------------------------- modules/gallery/helpers/item_rest.php | 158 +++++++++++++++++++++++++++++++ modules/rest/controllers/rest.php | 2 +- modules/rest/helpers/rest.php | 50 ++++++++-- modules/tag/helpers/tag.php | 14 +-- modules/tag/helpers/tag_event.php | 12 ++- modules/tag/helpers/tag_item_rest.php | 50 ++++++++++ modules/tag/helpers/tag_rest.php | 43 +++++---- modules/tag/helpers/tags_rest.php | 4 +- 9 files changed, 289 insertions(+), 196 deletions(-) delete mode 100644 modules/gallery/helpers/gallery_rest.php create mode 100644 modules/gallery/helpers/item_rest.php create mode 100644 modules/tag/helpers/tag_item_rest.php (limited to 'modules/gallery/helpers') diff --git a/modules/gallery/helpers/gallery_rest.php b/modules/gallery/helpers/gallery_rest.php deleted file mode 100644 index c5838ea5..00000000 --- a/modules/gallery/helpers/gallery_rest.php +++ /dev/null @@ -1,152 +0,0 @@ - - * only return items where the name contains this substring - * - * random=true - * return a single random item - * - * type= - * limit the type to types in this list. eg, "type=photo,movie" - */ - static function get($request) { - $item = rest::resolve($request->url); - access::required("view", $item); - - $p = $request->params; - if (isset($p->random)) { - $orm = item::random_query()->offset(0)->limit(1); - } else { - $orm = ORM::factory("item")->viewable(); - } - - if (empty($p->scope)) { - $p->scope = "direct"; - } - - if (!in_array($p->scope, array("direct", "all"))) { - throw new Rest_Exception("Bad Request", 400); - } - - if ($p->scope == "direct") { - $orm->where("parent_id", "=", $item->id); - } else { - $orm->where("left_ptr", ">", $item->left_ptr); - $orm->where("right_ptr", "<", $item->right_ptr); - } - - if (isset($p->name)) { - $orm->where("name", "LIKE", "%{$p->name}%"); - } - - if (isset($p->type)) { - $orm->where("type", "IN", explode(",", $p->type)); - } - - $members = array(); - foreach ($orm->find_all() as $child) { - $members[] = rest::url("gallery", $child); - } - - return array("resource" => $item->as_array(), "members" => $members); - } - - static function put($request) { - $item = rest::resolve($request->url); - access::required("edit", $item); - - $params = $request->params; - - // Only change fields from a whitelist. - foreach (array("album_cover_item_id", "captured", "description", - "height", "mime_type", "name", "parent_id", "rand_key", "resize_dirty", - "resize_height", "resize_width", "slug", "sort_column", "sort_order", - "thumb_dirty", "thumb_height", "thumb_width", "title", "view_count", - "weight", "width") as $key) { - if (property_exists($request->params, $key)) { - $item->$key = $request->params->$key; - } - } - $item->save(); - - return array("url" => rest::url("gallery", $item)); - } - - static function post($request) { - $parent = rest::resolve($request->url); - access::required("edit", $parent); - - $params = $request->params; - $item = ORM::factory("item"); - switch ($params->type) { - case "album": - $item->type = "album"; - $item->parent_id = $parent->id; - $item->name = $params->name; - $item->title = isset($params->title) ? $params->title : $name; - $item->description = isset($params->description) ? $params->description : null; - $item->slug = isset($params->slug) ? $params->slug : null; - $item->save(); - break; - - case "photo": - case "movie": - $item->type = $params->type; - $item->parent_id = $parent->id; - $item->set_data_file($request->file); - $item->name = $params->name; - $item->title = isset($params->title) ? $params->title : $params->name; - $item->description = isset($params->description) ? $params->description : null; - $item->slug = isset($params->slug) ? $params->slug : null; - $item->save(); - break; - - default: - throw new Rest_Exception("Invalid type: $params->type", 400); - } - - return array("url" => rest::url("gallery", $item)); - } - - static function delete($request) { - $item = rest::resolve($request->url); - access::required("edit", $item); - - $item->delete(); - } - - static function resolve($path) { - return url::get_item_from_uri($path); - } - - static function url($item) { - return url::abs_site("rest/gallery/" . $item->relative_url()); - } -} diff --git a/modules/gallery/helpers/item_rest.php b/modules/gallery/helpers/item_rest.php new file mode 100644 index 00000000..edc44c45 --- /dev/null +++ b/modules/gallery/helpers/item_rest.php @@ -0,0 +1,158 @@ + + * only return items where the name contains this substring + * + * random=true + * return a single random item + * + * type= + * limit the type to types in this list. eg, "type=photo,movie" + */ + static function get($request) { + $item = rest::resolve($request->url); + access::required("view", $item); + + $p = $request->params; + if (isset($p->random)) { + $orm = item::random_query()->offset(0)->limit(1); + } else { + $orm = ORM::factory("item")->viewable(); + } + + if (empty($p->scope)) { + $p->scope = "direct"; + } + + if (!in_array($p->scope, array("direct", "all"))) { + throw new Rest_Exception("Bad Request", 400); + } + + if ($p->scope == "direct") { + $orm->where("parent_id", "=", $item->id); + } else { + $orm->where("left_ptr", ">", $item->left_ptr); + $orm->where("right_ptr", "<", $item->right_ptr); + } + + if (isset($p->name)) { + $orm->where("name", "LIKE", "%{$p->name}%"); + } + + if (isset($p->type)) { + $orm->where("type", "IN", explode(",", $p->type)); + } + + $members = array(); + foreach ($orm->find_all() as $child) { + $members[] = rest::url("item", $child); + } + + return array( + "url" => $request->url, + "resource" => $item->as_array(), + "members" => $members, + "relationships" => rest::relationships("item", $item)); + } + + static function put($request) { + $item = rest::resolve($request->url); + access::required("edit", $item); + + $params = $request->params; + + // Only change fields from a whitelist. + foreach (array("album_cover_item_id", "captured", "description", + "height", "mime_type", "name", "parent_id", "rand_key", "resize_dirty", + "resize_height", "resize_width", "slug", "sort_column", "sort_order", + "thumb_dirty", "thumb_height", "thumb_width", "title", "view_count", + "weight", "width") as $key) { + if (property_exists($request->params, $key)) { + $item->$key = $request->params->$key; + } + } + if ($item->changed) { + $item->save(); + } + } + + static function post($request) { + $parent = rest::resolve($request->url); + access::required("edit", $parent); + + $params = $request->params; + $item = ORM::factory("item"); + switch ($params->type) { + case "album": + $item->type = "album"; + $item->parent_id = $parent->id; + $item->name = $params->name; + $item->title = isset($params->title) ? $params->title : $name; + $item->description = isset($params->description) ? $params->description : null; + $item->slug = isset($params->slug) ? $params->slug : null; + $item->save(); + break; + + case "photo": + case "movie": + $item->type = $params->type; + $item->parent_id = $parent->id; + $item->set_data_file($request->file); + $item->name = $params->name; + $item->title = isset($params->title) ? $params->title : $params->name; + $item->description = isset($params->description) ? $params->description : null; + $item->slug = isset($params->slug) ? $params->slug : null; + $item->save(); + break; + + default: + throw new Rest_Exception("Invalid type: $params->type", 400); + } + } + + static function delete($request) { + $item = rest::resolve($request->url); + access::required("edit", $item); + + $item->delete(); + } + + static function resolve($id) { + $item = ORM::factory("item")->where("id", "=", $id)->find(); + if (!access::can("view", $item)) { + throw new Kohana_404_Exception(); + } + return $item; + } + + static function url($item) { + return url::abs_site("rest/item/{$item->id}"); + } +} diff --git a/modules/rest/controllers/rest.php b/modules/rest/controllers/rest.php index 9f0bc5b3..ba996b84 100644 --- a/modules/rest/controllers/rest.php +++ b/modules/rest/controllers/rest.php @@ -56,7 +56,7 @@ class Rest_Controller extends Controller { $handler_method = $request->method; if (!method_exists($handler_class, $handler_method)) { - throw new Rest_Exception("Forbidden", 403); + throw new Rest_Exception("Bad Request", 400); } try { diff --git a/modules/rest/helpers/rest.php b/modules/rest/helpers/rest.php index 85987ca1..fe704a9e 100644 --- a/modules/rest/helpers/rest.php +++ b/modules/rest/helpers/rest.php @@ -22,8 +22,16 @@ class rest_Core { Session::abort_save(); if ($data) { - header("Content-type: application/json"); - print json_encode($data); + if (Input::instance()->get("output_type") == "html") { + header("Content-type: text/html"); + $html = preg_replace( + "#(^|[\n ])([\w]+?://[\w]+[^ \"\n\r\t<]*)#ise", "'\\1\\2'", + print_r($data, 1)); + print "
$html
"; + } else { + header("Content-type: application/json"); + print json_encode($data); + } } } @@ -64,7 +72,10 @@ class rest_Core { /** * Convert a REST url into an object. - * Eg: "http://example.com/gallery3/index.php/rest/gallery/Family/Wedding" -> Item_Model + * Eg: + * http://example.com/gallery3/index.php/rest/item/35 -> Item_Model + * http://example.com/gallery3/index.php/rest/tag/16 -> Tag_Model + * http://example.com/gallery3/index.php/rest/tagged_item/1,16 -> [Tag_Model, Item_Model] * * @param string the fully qualified REST url * @return mixed the corresponding object (usually a model of some kind) @@ -88,15 +99,38 @@ class rest_Core { /** * Return an absolute url used for REST resource location. - * @param string module name (eg, "gallery", "tags") + * @param string resource type (eg, "item", "tag") * @param object resource */ - static function url($module, $resource) { - $class = "{$module}_rest"; + static function url() { + $args = func_get_args(); + $resource_type = array_shift($args); + + $class = "{$resource_type}_rest"; if (!method_exists($class, "url")) { - throw new Exception("@todo MISSING REST CLASS: $class"); + throw new Rest_Exception("Bad Request", 400); + } + + $url = call_user_func_array(array($class, "url"), $args); + if (Input::instance()->get("output_type") == "html") { + $url .= "?output_type=html"; + } + return $url; + } + + static function relationships($resource_type, $resource) { + $results = array(); + foreach (module::active() as $module) { + foreach (glob(MODPATH . "{$module->name}/helpers/*_rest.php") as $filename) { + $class = str_replace(".php", "", basename($filename)); + if (method_exists($class, "relationships")) { + $results = array_merge( + $results, + call_user_func(array($class, "relationships"), $resource_type, $resource)); + } + } } - return call_user_func(array($class, "url"), $resource); + return $results; } } diff --git a/modules/tag/helpers/tag.php b/modules/tag/helpers/tag.php index c49a2d0f..9e59b527 100644 --- a/modules/tag/helpers/tag.php +++ b/modules/tag/helpers/tag.php @@ -91,16 +91,10 @@ class tag_Core { * @return array */ static function item_tags($item) { - $tags = array(); - foreach (db::build() - ->select("name") - ->from("tags") - ->join("items_tags", "tags.id", "items_tags.tag_id", "left") - ->where("items_tags.item_id", "=", $item->id) - ->execute() as $row) { - $tags[] = $row->name; - } - return $tags; + return ORM::factory("tag") + ->join("items_tags", "tags.id", "items_tags.tag_id", "left") + ->where("items_tags.item_id", "=", $item->id) + ->find_all(); } static function get_add_form($item) { diff --git a/modules/tag/helpers/tag_event.php b/modules/tag/helpers/tag_event.php index 6ee8e708..403ccd52 100644 --- a/modules/tag/helpers/tag_event.php +++ b/modules/tag/helpers/tag_event.php @@ -71,9 +71,13 @@ class tag_event_Core { $('form input[id=tags]').autocomplete( '$url', {max: 30, multiple: true, multipleSeparator: ',', cacheLength: 1}); });"); - $tag_value = implode(", ", tag::item_tags($item)); + + $tag_names = array(); + foreach (tag::item_tags($item) as $tag) { + $tag_names[] = $tag->name; + } $form->edit_item->input("tags")->label(t("Tags (comma separated)")) - ->value($tag_value); + ->value(implode(", ", $tag_names)); } static function item_edit_form_completed($item, $form) { @@ -95,7 +99,9 @@ class tag_event_Core { } static function item_index_data($item, $data) { - $data[] = join(" ", tag::item_tags($item)); + foreach (tag::item_tags($item) as $tag) { + $data[] = $tag->name; + } } static function add_photos_form($album, $form) { diff --git a/modules/tag/helpers/tag_item_rest.php b/modules/tag/helpers/tag_item_rest.php new file mode 100644 index 00000000..cd9bb6fe --- /dev/null +++ b/modules/tag/helpers/tag_item_rest.php @@ -0,0 +1,50 @@ +url); + return array( + "url" => $request->url, + "members" => array( + rest::url("tag", $tag), + rest::url("item", $item))); + } + + static function delete($request) { + list ($tag, $item) = rest::resolve($request->url); + $tag->remove($item); + $tag->save(); + } + + static function resolve($tuple) { + list ($tag_id, $item_id) = split(",", $tuple); + $tag = ORM::factory("tag")->where("id", "=", $tag_id)->find(); + $item = ORM::factory("item")->where("id", "=", $item_id)->find(); + if (!$tag->loaded() || !$item->loaded() || !$tag->has($item)) { + throw new Kohana_404_Exception(); + } + + return array($tag, $item); + } + + static function url($tag, $item) { + return url::abs_site("rest/tag_item/{$tag->id},{$item->id}"); + } +} diff --git a/modules/tag/helpers/tag_rest.php b/modules/tag/helpers/tag_rest.php index 0226c6d3..d68cb73d 100644 --- a/modules/tag/helpers/tag_rest.php +++ b/modules/tag/helpers/tag_rest.php @@ -20,12 +20,18 @@ class tag_rest_Core { static function get($request) { $tag = rest::resolve($request->url); - $items = array(); + $tag_items = array(); foreach ($tag->items() as $item) { - $items[] = rest::url("gallery", $item); + if (access::can("view", $item)) { + $tag_items[] = rest::url("tag_item", $tag, $item); + } } - return array("resource" => $tag->as_array(), "members" => $items); + return array( + "url" => $request->url, + "resource" => $tag->as_array(), + "relationships" => array( + "items" => $tag_items)); } static function post($request) { @@ -38,37 +44,34 @@ class tag_rest_Core { access::required("edit", $item); tag::add($item, $tag->name); - return array("url" => rest::url("tag", $tag)); } static function put($request) { $tag = rest::resolve($request->url); if (isset($request->params->name)) { $tag->name = $request->params->name; + $tag->save(); } - - $tag->save(); - return array("url" => rest::url("tag", $tag)); } static function delete($request) { $tag = rest::resolve($request->url); + $tag->delete(); + } - if (empty($request->params->url)) { - // Delete the tag - $tag->delete(); - } else { - // Remove an item from the tag - $item = rest::resolve($request->params->url); - access::required("edit", $item); - $tag->remove($item); - $tag->save(); - tag::compact(); + static function relationships($resource_type, $resource) { + switch ($resource_type) { + case "item": + $tags = array(); + foreach (tag::item_tags($resource) as $tag) { + $tags[] = rest::url("tag_item", $tag, $resource); + } + return array("tags" => $tags); } } - static function resolve($tag_name) { - $tag = ORM::factory("tag")->where("name", "=", $tag_name)->find(); + static function resolve($id) { + $tag = ORM::factory("tag")->where("id", "=", $id)->find(); if (!$tag->loaded()) { throw new Kohana_404_Exception(); } @@ -77,6 +80,6 @@ class tag_rest_Core { } static function url($tag) { - return url::abs_site("rest/tag/" . rawurlencode($tag->name)); + return url::abs_site("rest/tag/{$tag->id}"); } } diff --git a/modules/tag/helpers/tags_rest.php b/modules/tag/helpers/tags_rest.php index 41317ecd..57461125 100644 --- a/modules/tag/helpers/tags_rest.php +++ b/modules/tag/helpers/tags_rest.php @@ -21,7 +21,7 @@ class tags_rest_Core { static function get($request) { $tags = array(); foreach (ORM::factory("tag")->find_all() as $tag) { - $tags[$tag->name] = rest::url("tag", $tag); + $tags[] = rest::url("tag", $tag); } return array("members" => $tags); } @@ -33,7 +33,7 @@ class tags_rest_Core { access::required("edit", item::root()); if (empty($request->params->name)) { - throw new Rest_Exception("Bad Request: missing name", 400); + throw new Rest_Exception("Bad Request", 400); } $tag = ORM::factory("tag")->where("name", "=", $request->params->name)->find(); -- cgit v1.2.3 From 0efbfcbe61e26cb53cb0719ced3c5c436f128386 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Fri, 22 Jan 2010 01:10:31 -0800 Subject: Return the url of the newly created item from post(). Don't try to access ORM::$changed -- it's protected. --- modules/gallery/helpers/item_rest.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'modules/gallery/helpers') diff --git a/modules/gallery/helpers/item_rest.php b/modules/gallery/helpers/item_rest.php index edc44c45..9598b191 100644 --- a/modules/gallery/helpers/item_rest.php +++ b/modules/gallery/helpers/item_rest.php @@ -98,9 +98,7 @@ class item_rest_Core { $item->$key = $request->params->$key; } } - if ($item->changed) { - $item->save(); - } + $item->save(); } static function post($request) { @@ -135,6 +133,8 @@ class item_rest_Core { default: throw new Rest_Exception("Invalid type: $params->type", 400); } + + return array("url" => rest::url("item", $item)); } static function delete($request) { -- cgit v1.2.3 From ca5f625a5e3dc2fc26136b5dea8f27251f92c7c4 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Sun, 24 Jan 2010 11:40:01 -0800 Subject: Log validation errors. --- modules/gallery/helpers/task.php | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'modules/gallery/helpers') diff --git a/modules/gallery/helpers/task.php b/modules/gallery/helpers/task.php index 4aa95f33..ad756ecd 100644 --- a/modules/gallery/helpers/task.php +++ b/modules/gallery/helpers/task.php @@ -85,6 +85,13 @@ class task_Core { $task->save(); } catch (Exception $e) { Kohana_Log::add("error", $e->__toString()); + + // 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)); + } + $task->log($e->__toString()); $task->state = "error"; $task->done = true; -- cgit v1.2.3 From ec0f89f10a58c3c3751387d5c1d1efcbf940bc9a Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Wed, 27 Jan 2010 21:40:48 -0800 Subject: Change "resource" to "entity" in REST responses. They're all resources, but we differentiate resources as collections and entities. --- modules/gallery/helpers/item_rest.php | 2 +- modules/gallery/tests/Item_Rest_Helper_Test.php | 11 ++++++----- modules/tag/helpers/tag_rest.php | 2 +- modules/tag/tests/Tag_Rest_Helper_Test.php | 4 ++-- 4 files changed, 10 insertions(+), 9 deletions(-) (limited to 'modules/gallery/helpers') diff --git a/modules/gallery/helpers/item_rest.php b/modules/gallery/helpers/item_rest.php index 9598b191..2236fbbb 100644 --- a/modules/gallery/helpers/item_rest.php +++ b/modules/gallery/helpers/item_rest.php @@ -77,7 +77,7 @@ class item_rest_Core { return array( "url" => $request->url, - "resource" => $item->as_array(), + "entity" => $item->as_array(), "members" => $members, "relationships" => rest::relationships("item", $item)); } diff --git a/modules/gallery/tests/Item_Rest_Helper_Test.php b/modules/gallery/tests/Item_Rest_Helper_Test.php index 8ce6bc43..d91e0f58 100644 --- a/modules/gallery/tests/Item_Rest_Helper_Test.php +++ b/modules/gallery/tests/Item_Rest_Helper_Test.php @@ -36,7 +36,7 @@ class Item_Rest_Helper_Test extends Gallery_Unit_Test_Case { $request->params = new stdClass(); $this->assert_equal_array( array("url" => rest::url("item", $album1), - "resource" => $album1->as_array(), + "entity" => $album1->as_array(), "members" => array( rest::url("item", $photo1), rest::url("item", $album2)), @@ -50,7 +50,7 @@ class Item_Rest_Helper_Test extends Gallery_Unit_Test_Case { $request->params->scope = "direct"; $this->assert_equal_array( array("url" => rest::url("item", $album1), - "resource" => $album1->as_array(), + "entity" => $album1->as_array(), "members" => array( rest::url("item", $photo1), rest::url("item", $album2)), @@ -64,7 +64,7 @@ class Item_Rest_Helper_Test extends Gallery_Unit_Test_Case { $request->params->scope = "all"; $this->assert_equal_array( array("url" => rest::url("item", $album1), - "resource" => $album1->as_array(), + "entity" => $album1->as_array(), "members" => array( rest::url("item", $photo1), rest::url("item", $album2), @@ -88,7 +88,7 @@ class Item_Rest_Helper_Test extends Gallery_Unit_Test_Case { $request->params->name = "foo"; $this->assert_equal_array( array("url" => rest::url("item", $album1), - "resource" => $album1->as_array(), + "entity" => $album1->as_array(), "members" => array( rest::url("item", $photo2)), "relationships" => array( @@ -108,7 +108,7 @@ class Item_Rest_Helper_Test extends Gallery_Unit_Test_Case { $request->params->type = "album"; $this->assert_equal_array( array("url" => rest::url("item", $album1), - "resource" => $album1->as_array(), + "entity" => $album1->as_array(), "members" => array( rest::url("item", $album2)), "relationships" => array( @@ -210,6 +210,7 @@ class Item_Rest_Helper_Test extends Gallery_Unit_Test_Case { public function delete_album_fails_without_permission_test() { $album1 = test::random_album(); access::deny(identity::everybody(), "edit", $album1); + identity::set_active_user(identity::guest()); $request->url = rest::url("item", $album1); try { diff --git a/modules/tag/helpers/tag_rest.php b/modules/tag/helpers/tag_rest.php index 4b2a4b46..7143daa9 100644 --- a/modules/tag/helpers/tag_rest.php +++ b/modules/tag/helpers/tag_rest.php @@ -29,7 +29,7 @@ class tag_rest_Core { return array( "url" => $request->url, - "resource" => $tag->as_array(), + "entity" => $tag->as_array(), "relationships" => array( "items" => array( "url" => rest::url("tag_items", $tag), diff --git a/modules/tag/tests/Tag_Rest_Helper_Test.php b/modules/tag/tests/Tag_Rest_Helper_Test.php index eacf91b3..d3cae0fb 100644 --- a/modules/tag/tests/Tag_Rest_Helper_Test.php +++ b/modules/tag/tests/Tag_Rest_Helper_Test.php @@ -31,7 +31,7 @@ class Tag_Rest_Helper_Test extends Gallery_Unit_Test_Case { $request->url = rest::url("tag", $tag); $this->assert_equal_array( array("url" => rest::url("tag", $tag), - "resource" => $tag->as_array(), + "entity" => $tag->as_array(), "relationships" => array( "items" => array( "url" => rest::url("tag_items", $tag), @@ -56,7 +56,7 @@ class Tag_Rest_Helper_Test extends Gallery_Unit_Test_Case { $request->url = rest::url("tag", $tag); $this->assert_equal_array( array("url" => rest::url("tag", $tag), - "resource" => $tag->as_array(), + "entity" => $tag->as_array(), "relationships" => array( "items" => array( "url" => rest::url("tag_items", $tag), -- cgit v1.2.3 From cfbbf9ef606094868ccbd25ccf65e1a6f610528b Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Wed, 27 Jan 2010 21:58:06 -0800 Subject: Convert __toString() to use (string) cast instead. --- modules/exif/helpers/exif_task.php | 2 +- modules/g2_import/helpers/g2_import.php | 10 +++++----- modules/gallery/helpers/gallery_task.php | 8 ++++---- modules/gallery/helpers/task.php | 4 ++-- modules/organize/controllers/organize.php | 10 +++++----- modules/tag/controllers/tags.php | 2 +- 6 files changed, 18 insertions(+), 18 deletions(-) (limited to 'modules/gallery/helpers') diff --git a/modules/exif/helpers/exif_task.php b/modules/exif/helpers/exif_task.php index 27352643..90869630 100644 --- a/modules/exif/helpers/exif_task.php +++ b/modules/exif/helpers/exif_task.php @@ -82,7 +82,7 @@ class exif_task_Core { $task->done = true; $task->state = "error"; $task->status = $e->getMessage(); - $task->log($e->__toString()); + $task->log((string)$e); } } } diff --git a/modules/g2_import/helpers/g2_import.php b/modules/g2_import/helpers/g2_import.php index 6f019aa8..74164305 100644 --- a/modules/g2_import/helpers/g2_import.php +++ b/modules/g2_import/helpers/g2_import.php @@ -239,7 +239,7 @@ class g2_import_Core { $g2_group = g2(GalleryCoreApi::loadEntitiesById($g2_group_id)); } catch (Exception $e) { return t("Failed to import Gallery 2 group with id: %id\n%exception", - array("id" => $g2_group_id, "exception" => $e->__toString())); + array("id" => $g2_group_id, "exception" => (string)$e)); } switch ($g2_group->getGroupType()) { @@ -295,7 +295,7 @@ class g2_import_Core { $g2_user = g2(GalleryCoreApi::loadEntitiesById($g2_user_id)); } catch (Exception $e) { return t("Failed to import Gallery 2 user with id: %id\n%exception", - array("id" => $g2_user_id, "exception" => $e->__toString())); + array("id" => $g2_user_id, "exception" => (string)$e)); } $g2_groups = g2(GalleryCoreApi::fetchGroupsForUser($g2_user->getId())); @@ -449,7 +449,7 @@ class g2_import_Core { $g2_path = g2($g2_item->fetchPath()); } catch (Exception $e) { return t("Failed to import Gallery 2 item with id: %id\n%exception", - array("id" => $g2_item_id, "exception" => $e->__toString())); + array("id" => $g2_item_id, "exception" => (string)$e)); } $parent = ORM::factory("item")->where("id", "=", self::map($g2_item->getParentId()))->find(); @@ -596,7 +596,7 @@ class g2_import_Core { $g2_comment = g2(GalleryCoreApi::loadEntitiesById($g2_comment_id)); } catch (Exception $e) { return t("Failed to import Gallery 2 comment with id: %id\%exception", - array("id" => $g2_comment_id, "exception" => $e->__toString())); + array("id" => $g2_comment_id, "exception" => (string)$e)); } $text = $g2_comment->getSubject(); @@ -642,7 +642,7 @@ class g2_import_Core { $tag_names = array_values(g2(TagsHelper::getTagsByItemId($g2_item_id))); } catch (Exception $e) { return t("Failed to import Gallery 2 tags for item with id: %id\n%exception", - array("id" => $g2_item_id, "exception" => $e->__toString())); + array("id" => $g2_item_id, "exception" => (string)$e)); } foreach ($tag_names as $tag_name) { diff --git a/modules/gallery/helpers/gallery_task.php b/modules/gallery/helpers/gallery_task.php index 5402b5d1..c75e050a 100644 --- a/modules/gallery/helpers/gallery_task.php +++ b/modules/gallery/helpers/gallery_task.php @@ -81,7 +81,7 @@ class gallery_task_Core { } catch (Exception $e) { $errors[] = t("Unable to rebuild images for '%title'", array("title" => html::purify($item->title))); - $errors[] = $e->__toString(); + $errors[] = (string)$e; $ignored[$item->id] = 1; } } @@ -114,7 +114,7 @@ class gallery_task_Core { $task->done = true; $task->state = "error"; $task->status = $e->getMessage(); - $errors[] = $e->__toString(); + $errors[] = (string)$e; } if ($errors) { $task->log($errors); @@ -217,7 +217,7 @@ class gallery_task_Core { $task->done = true; $task->state = "error"; $task->status = $e->getMessage(); - $errors[] = $e->__toString(); + $errors[] = (string)$e; } if ($errors) { $task->log($errors); @@ -284,7 +284,7 @@ class gallery_task_Core { $task->done = true; $task->state = "error"; $task->status = $e->getMessage(); - $errors[] = $e->__toString(); + $errors[] = (string)$e; } if ($errors) { $task->log($errors); diff --git a/modules/gallery/helpers/task.php b/modules/gallery/helpers/task.php index ad756ecd..645850d1 100644 --- a/modules/gallery/helpers/task.php +++ b/modules/gallery/helpers/task.php @@ -84,7 +84,7 @@ class task_Core { } $task->save(); } catch (Exception $e) { - Kohana_Log::add("error", $e->__toString()); + Kohana_Log::add("error", (string)$e); // 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 @@ -92,7 +92,7 @@ class task_Core { Kohana_Log::add("error", "Validation errors: " . print_r($e->validation->errors(), 1)); } - $task->log($e->__toString()); + $task->log((string)$e); $task->state = "error"; $task->done = true; $task->status = substr($e->getMessage(), 0, 255); diff --git a/modules/organize/controllers/organize.php b/modules/organize/controllers/organize.php index 201ced30..4a4b9f13 100644 --- a/modules/organize/controllers/organize.php +++ b/modules/organize/controllers/organize.php @@ -36,7 +36,7 @@ class Organize_Controller extends Controller { access::required("edit", $album); print json_encode( - array("grid" => self::_get_micro_thumb_grid($album, $offset)->__toString(), + array("grid" => (string)self::_get_micro_thumb_grid($album, $offset), "sort_column" => $album->sort_column, "sort_order" => $album->sort_order)); } @@ -57,8 +57,8 @@ class Organize_Controller extends Controller { } print json_encode( - array("tree" => self::_expanded_tree(ORM::factory("item", 1), $target_album)->__toString(), - "grid" => self::_get_micro_thumb_grid($target_album, 0)->__toString())); + array("tree" => (string)self::_expanded_tree(ORM::factory("item", 1), $target_album), + "grid" => (string)self::_get_micro_thumb_grid($target_album, 0))); } function rearrange($target_id, $before_or_after) { @@ -114,7 +114,7 @@ class Organize_Controller extends Controller { module::event("album_rearrange", $album); print json_encode( - array("grid" => self::_get_micro_thumb_grid($album, 0)->__toString(), + array("grid" => (string)self::_get_micro_thumb_grid($album, 0), "sort_column" => $album->sort_column, "sort_order" => $album->sort_order)); } @@ -136,7 +136,7 @@ class Organize_Controller extends Controller { $album->save(); print json_encode( - array("grid" => self::_get_micro_thumb_grid($album, 0)->__toString(), + array("grid" => (string)self::_get_micro_thumb_grid($album, 0), "sort_column" => $album->sort_column, "sort_order" => $album->sort_order)); } diff --git a/modules/tag/controllers/tags.php b/modules/tag/controllers/tags.php index e28b7a83..1eede907 100644 --- a/modules/tag/controllers/tags.php +++ b/modules/tag/controllers/tags.php @@ -69,7 +69,7 @@ class Tags_Controller extends Controller { print json_encode( array("result" => "success", - "cloud" => tag::cloud(30)->__toString())); + "cloud" => (string)tag::cloud(30))); } else { print json_encode(array("result" => "error", "form" => (string) $form)); } -- cgit v1.2.3 From 4b32a71afc7650fe7bdd02ba384c8914f60538f3 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Wed, 27 Jan 2010 22:34:11 -0800 Subject: Convert back to using ORM::factory(..., $id) instead of calling where(). --- modules/comment/models/comment.php | 2 +- modules/comment/tests/Comment_Event_Test.php | 2 +- modules/g2_import/controllers/g2.php | 2 +- modules/g2_import/helpers/g2_import.php | 11 +++++------ modules/gallery/helpers/gallery_installer.php | 2 +- modules/gallery/helpers/item_rest.php | 2 +- modules/gallery/libraries/ORM_MPTT.php | 2 +- modules/gallery/models/item.php | 4 ++-- modules/server_add/controllers/server_add.php | 13 ++++++------- modules/tag/helpers/item_tags_rest.php | 2 +- modules/tag/helpers/tag_item_rest.php | 4 ++-- modules/tag/helpers/tag_items_rest.php | 2 +- modules/tag/helpers/tag_rest.php | 2 +- modules/user/models/group.php | 2 +- modules/user/models/user.php | 2 +- 15 files changed, 26 insertions(+), 28 deletions(-) (limited to 'modules/gallery/helpers') diff --git a/modules/comment/models/comment.php b/modules/comment/models/comment.php index 43c4148f..8be022b5 100644 --- a/modules/comment/models/comment.php +++ b/modules/comment/models/comment.php @@ -108,7 +108,7 @@ class Comment_Model extends ORM { module::event("comment_created", $this); } else { // Updated comment - $original = ORM::factory("comment")->where("id", "=", $this->id)->find(); + $original = ORM::factory("comment", $this->id); $visible_change = $original->state == "published" || $this->state == "published"; parent::save(); module::event("comment_updated", $original, $this); diff --git a/modules/comment/tests/Comment_Event_Test.php b/modules/comment/tests/Comment_Event_Test.php index 27272055..08f55b3f 100644 --- a/modules/comment/tests/Comment_Event_Test.php +++ b/modules/comment/tests/Comment_Event_Test.php @@ -30,6 +30,6 @@ class Comment_Event_Test extends Gallery_Unit_Test_Case { $album->delete(); - $this->assert_false(ORM::factory("comment")->where("id", "=", $comment->id)->find()->loaded()); + $this->assert_false(ORM::factory("comment", $comment->id)->loaded()); } } diff --git a/modules/g2_import/controllers/g2.php b/modules/g2_import/controllers/g2.php index 3e002758..5fd4400c 100644 --- a/modules/g2_import/controllers/g2.php +++ b/modules/g2_import/controllers/g2.php @@ -50,7 +50,7 @@ class G2_Controller extends Admin_Controller { throw new Kohana_404_Exception(); } - $item = ORM::factory("item")->where("id", "=", $g2_map->g3_id)->find(); + $item = ORM::factory("item", $g2_map->g3_id); if (!$item->loaded() || !access::can("view", $item)) { throw new Kohana_404_Exception(); } diff --git a/modules/g2_import/helpers/g2_import.php b/modules/g2_import/helpers/g2_import.php index 74164305..fa95e547 100644 --- a/modules/g2_import/helpers/g2_import.php +++ b/modules/g2_import/helpers/g2_import.php @@ -358,8 +358,7 @@ class g2_import_Core { if ($g2_album->getParentId() == null) { return t("Skipping Gallery 2 root album"); } - $parent_album = - ORM::factory("item")->where("id", "=", self::map($g2_album->getParentId()))->find(); + $parent_album = ORM::factory("item", self::map($g2_album->getParentId())); $album = ORM::factory("item"); $album->type = "album"; @@ -423,8 +422,8 @@ class g2_import_Core { } $item_id = self::map($g2_source->getId()); if ($item_id) { - $item = ORM::factory("item")->where("id", "=", $item_id)->find(); - $g2_album = ORM::factory("item")->where("id", "=", $g3_album_id)->find(); + $item = ORM::factory("item", $item_id); + $g2_album = ORM::factory("item", $g3_album_id); $g2_album->album_cover_item_id = $item->id; $g2_album->thumb_dirty = 1; $g2_album->view_count = g2(GalleryCoreApi::fetchItemViewCount($g2_album_id)); @@ -452,7 +451,7 @@ class g2_import_Core { array("id" => $g2_item_id, "exception" => (string)$e)); } - $parent = ORM::factory("item")->where("id", "=", self::map($g2_item->getParentId()))->find(); + $parent = ORM::factory("item", self::map($g2_item->getParentId())); $g2_type = $g2_item->getEntityType(); $corrupt = 0; @@ -633,7 +632,7 @@ class g2_import_Core { GalleryCoreApi::requireOnce("modules/tags/classes/TagsHelper.class"); $g2_item_id = array_shift($queue); - $g3_item = ORM::factory("item")->where("id", "=", self::map($g2_item_id))->find(); + $g3_item = ORM::factory("item", self::map($g2_item_id)); if (!$g3_item->loaded()) { return; } diff --git a/modules/gallery/helpers/gallery_installer.php b/modules/gallery/helpers/gallery_installer.php index aa297236..bfab4645 100644 --- a/modules/gallery/helpers/gallery_installer.php +++ b/modules/gallery/helpers/gallery_installer.php @@ -228,7 +228,7 @@ class gallery_installer { "updated" => $now, "weight" => 1)) ->execute(); - $root = ORM::factory("item")->where("id", "=", 1)->find(); + $root = ORM::factory("item", 1); access::add_item($root); module::set_var("gallery", "active_site_theme", "wind"); diff --git a/modules/gallery/helpers/item_rest.php b/modules/gallery/helpers/item_rest.php index 2236fbbb..d5ca1456 100644 --- a/modules/gallery/helpers/item_rest.php +++ b/modules/gallery/helpers/item_rest.php @@ -145,7 +145,7 @@ class item_rest_Core { } static function resolve($id) { - $item = ORM::factory("item")->where("id", "=", $id)->find(); + $item = ORM::factory("item", $id); if (!access::can("view", $item)) { throw new Kohana_404_Exception(); } diff --git a/modules/gallery/libraries/ORM_MPTT.php b/modules/gallery/libraries/ORM_MPTT.php index a7bb24ea..83f9b51e 100644 --- a/modules/gallery/libraries/ORM_MPTT.php +++ b/modules/gallery/libraries/ORM_MPTT.php @@ -48,7 +48,7 @@ class ORM_MPTT_Core extends ORM { function save() { if (!$this->loaded()) { $this->lock(); - $parent = ORM::factory("item")->where("id", "=", $this->parent_id)->find(); + $parent = ORM::factory("item", $this->parent_id); try { // Make a hole in the parent for this new item diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php index 9706d61f..ae1b6608 100644 --- a/modules/gallery/models/item.php +++ b/modules/gallery/models/item.php @@ -423,7 +423,7 @@ class Item_Model extends ORM_MPTT { // If any significant fields have changed, load up a copy of the original item and // keep it around. - $original = ORM::factory("item")->where("id", "=", $this->id)->find(); + $original = ORM::factory("item", $this->id); if (array_intersect($this->changed, array("parent_id", "name", "slug"))) { $original->_build_relative_caches(); $this->relative_path_cache = null; @@ -787,7 +787,7 @@ class Item_Model extends ORM_MPTT { if ($this->is_movie() || $this->is_photo()) { if ($this->loaded()) { // Existing items can't change their extension - $original = ORM::factory("item")->where("id", "=", $this->id)->find(); + $original = ORM::factory("item", $this->id); $new_ext = pathinfo($this->name, PATHINFO_EXTENSION); $old_ext = pathinfo($original->name, PATHINFO_EXTENSION); if (strcasecmp($new_ext, $old_ext)) { diff --git a/modules/server_add/controllers/server_add.php b/modules/server_add/controllers/server_add.php index 4d6d5dfe..287855b6 100644 --- a/modules/server_add/controllers/server_add.php +++ b/modules/server_add/controllers/server_add.php @@ -24,7 +24,7 @@ class Server_Add_Controller extends Admin_Controller { $files[] = $path; } - $item = ORM::factory("item")->where("id", "=", $id)->find(); + $item = ORM::factory("item", $id); $view = new View("server_add_tree_dialog.html"); $view->item = $item; $view->tree = new View("server_add_tree.html"); @@ -78,7 +78,7 @@ class Server_Add_Controller extends Admin_Controller { */ public function start() { access::verify_csrf(); - $item = ORM::factory("item")->where("id", "=", Input::instance()->get("item_id"))->find(); + $item = ORM::factory("item", Input::instance()->get("item_id")); foreach (Input::instance()->post("paths") as $path) { if (server_add::is_valid_path($path)) { @@ -104,7 +104,7 @@ class Server_Add_Controller extends Admin_Controller { function run($task_id) { access::verify_csrf(); - $task = ORM::factory("task")->where("id", "=", $task_id)->find(); + $task = ORM::factory("task", $task_id); if (!$task->loaded() || $task->owner_id != identity::active_user()->id) { access::forbidden(); } @@ -216,12 +216,11 @@ class Server_Add_Controller extends Admin_Controller { // Look up the parent item for this entry. By now it should exist, but if none was // specified, then this belongs as a child of the current item. - $parent_entry = - ORM::factory("server_add_file")->where("id", "=", $entry->parent_id)->find(); + $parent_entry = ORM::factory("server_add_file", $entry->parent_id); if (!$parent_entry->loaded()) { - $parent = ORM::factory("item")->where("id", "=", $task->get("item_id"))->find(); + $parent = ORM::factory("item", $task->get("item_id")); } else { - $parent = ORM::factory("item")->where("id", "=", $parent_entry->item_id)->find(); + $parent = ORM::factory("item", $parent_entry->item_id); } $name = basename($entry->file); diff --git a/modules/tag/helpers/item_tags_rest.php b/modules/tag/helpers/item_tags_rest.php index ce814f77..43e2cef0 100644 --- a/modules/tag/helpers/item_tags_rest.php +++ b/modules/tag/helpers/item_tags_rest.php @@ -50,7 +50,7 @@ class item_tags_rest_Core { } static function resolve($id) { - $item = ORM::factory("item")->where("id", "=", $id)->find(); + $item = ORM::factory("item", $id); if (!access::can("view", $item)) { throw new Kohana_404_Exception(); } diff --git a/modules/tag/helpers/tag_item_rest.php b/modules/tag/helpers/tag_item_rest.php index cd9bb6fe..60d37437 100644 --- a/modules/tag/helpers/tag_item_rest.php +++ b/modules/tag/helpers/tag_item_rest.php @@ -35,8 +35,8 @@ class tag_item_rest_Core { static function resolve($tuple) { list ($tag_id, $item_id) = split(",", $tuple); - $tag = ORM::factory("tag")->where("id", "=", $tag_id)->find(); - $item = ORM::factory("item")->where("id", "=", $item_id)->find(); + $tag = ORM::factory("tag", $tag_id); + $item = ORM::factory("item", $item_id); if (!$tag->loaded() || !$item->loaded() || !$tag->has($item)) { throw new Kohana_404_Exception(); } diff --git a/modules/tag/helpers/tag_items_rest.php b/modules/tag/helpers/tag_items_rest.php index 369a8d83..ef563ac6 100644 --- a/modules/tag/helpers/tag_items_rest.php +++ b/modules/tag/helpers/tag_items_rest.php @@ -52,7 +52,7 @@ class tag_items_rest_Core { } static function resolve($id) { - return ORM::factory("tag")->where("id", "=", $id)->find(); + return ORM::factory("tag", $id); } static function url($tag) { diff --git a/modules/tag/helpers/tag_rest.php b/modules/tag/helpers/tag_rest.php index 7143daa9..4879cf63 100644 --- a/modules/tag/helpers/tag_rest.php +++ b/modules/tag/helpers/tag_rest.php @@ -77,7 +77,7 @@ class tag_rest_Core { } static function resolve($id) { - $tag = ORM::factory("tag")->where("id", "=", $id)->find(); + $tag = ORM::factory("tag", $id); if (!$tag->loaded()) { throw new Kohana_404_Exception(); } diff --git a/modules/user/models/group.php b/modules/user/models/group.php index 85114ede..851e72e6 100644 --- a/modules/user/models/group.php +++ b/modules/user/models/group.php @@ -55,7 +55,7 @@ class Group_Model extends ORM implements Group_Definition { module::event("group_created", $this); } else { // Updated group - $original = ORM::factory("group")->where("id", "=", $this->id)->find(); + $original = ORM::factory("group", $this->id); parent::save(); module::event("group_updated", $original, $this); } diff --git a/modules/user/models/user.php b/modules/user/models/user.php index 7c97bae7..78c31047 100644 --- a/modules/user/models/user.php +++ b/modules/user/models/user.php @@ -99,7 +99,7 @@ class User_Model extends ORM implements User_Definition { module::event("user_created", $this); } else { // Updated user - $original = ORM::factory("user")->where("id", "=", $this->id)->find(); + $original = ORM::factory("user", $this->id); parent::save(); module::event("user_updated", $original, $this); } -- cgit v1.2.3 From acbb5aac05d41aad89176d28f1d583c8072f2a2a Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Wed, 27 Jan 2010 22:41:09 -0800 Subject: Remove unnecessary rules() in the form. --- modules/gallery/helpers/album.php | 1 - 1 file changed, 1 deletion(-) (limited to 'modules/gallery/helpers') diff --git a/modules/gallery/helpers/album.php b/modules/gallery/helpers/album.php index 55282252..641f0708 100644 --- a/modules/gallery/helpers/album.php +++ b/modules/gallery/helpers/album.php @@ -53,7 +53,6 @@ class album_Core { $group->textarea("description")->label(t("Description"))->value($parent->description); if ($parent->id != 1) { $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")) ->error_messages("no_slashes", t("The directory name can't contain a \"/\"")) -- cgit v1.2.3 From a2fc1d3422dec370dd1c177d6930bbad8c1aa1d8 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Wed, 27 Jan 2010 22:55:54 -0800 Subject: Localize error messages for the built-in rules. --- modules/gallery/helpers/album.php | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) (limited to 'modules/gallery/helpers') diff --git a/modules/gallery/helpers/album.php b/modules/gallery/helpers/album.php index 641f0708..30eeb960 100644 --- a/modules/gallery/helpers/album.php +++ b/modules/gallery/helpers/album.php @@ -29,14 +29,20 @@ class album_Core { $form = new Forge("albums/create/{$parent->id}", "", "post", array("id" => "g-add-album-form")); $group = $form->group("add_album") ->label(t("Add an album to %album_title", array("album_title" => $parent->title))); - $group->input("title")->label(t("Title")); + $group->input("title")->label(t("Title")) + ->error_messages("required", t("You must provide a title")) + ->error_messages("length", t("Your title is too long")); $group->textarea("description")->label(t("Description")); $group->input("name")->label(t("Directory name")) - ->error_messages("no_slashes", t("The directory name can't contain the \"/\" character")); + ->error_messages("no_slashes", t("The directory name can't contain the \"/\" character")) + ->error_messages("required", t("You must provide a directory name")) + ->error_messages("length", t("Your directory name is too long")); $group->input("slug")->label(t("Internet Address")) ->error_messages( "not_url_safe", - t("The internet address should contain only letters, numbers, hyphens and underscores")); + t("The internet address should contain only letters, numbers, hyphens and underscores")) + ->error_messages("required", t("You must provide an Internet Address")) + ->error_messages("length", t("Your Internet Address is too long")); $group->hidden("type")->value("album"); $group->submit("")->value(t("Create")); $form->script("") @@ -49,20 +55,26 @@ class album_Core { $form->hidden("from_id"); $group = $form->group("edit_item")->label(t("Edit Album")); - $group->input("title")->label(t("Title"))->value($parent->title); + $group->input("title")->label(t("Title"))->value($parent->title) + ->error_messages("required", t("You must provide a title")) + ->error_messages("length", t("Your title is too long")); $group->textarea("description")->label(t("Description"))->value($parent->description); if ($parent->id != 1) { $group->input("name")->label(t("Directory Name"))->value($parent->name) ->error_messages( "conflict", t("There is already a movie, photo or album with this name")) ->error_messages("no_slashes", t("The directory name can't contain a \"/\"")) - ->error_messages("no_trailing_period", t("The directory name can't end in \".\"")); + ->error_messages("no_trailing_period", t("The directory name can't end in \".\"")) + ->error_messages("required", t("You must provide a directory name")) + ->error_messages("length", t("Your directory name is too long")); $group->input("slug")->label(t("Internet Address"))->value($parent->slug) ->error_messages( "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")); + t("The internet address should contain only letters, numbers, hyphens and underscores")) + ->error_messages("required", t("You must provide an Internet Address")) + ->error_messages("length", t("Your Internet Address is too long")); } else { $group->hidden("name")->value($parent->name); $group->hidden("slug")->value($parent->slug); -- cgit v1.2.3 From e5b25983a67a53ad209aa4b8c251afa3276853fa Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Wed, 27 Jan 2010 23:00:29 -0800 Subject: Localize all error messages. --- modules/gallery/helpers/photo.php | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) (limited to 'modules/gallery/helpers') diff --git a/modules/gallery/helpers/photo.php b/modules/gallery/helpers/photo.php index 9bd277bc..bbdf2e3b 100644 --- a/modules/gallery/helpers/photo.php +++ b/modules/gallery/helpers/photo.php @@ -28,19 +28,25 @@ class photo_Core { $form = new Forge("photos/update/$photo->id", "", "post", array("id" => "g-edit-photo-form")); $form->hidden("from_id"); $group = $form->group("edit_item")->label(t("Edit Photo")); - $group->input("title")->label(t("Title"))->value($photo->title); + $group->input("title")->label(t("Title"))->value($photo->title) + ->error_messages("required", t("You must provide a title")) + ->error_messages("length", t("Your title is too long")); $group->textarea("description")->label(t("Description"))->value($photo->description); $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 \".\"")) - ->error_messages("illegal_extension", t("You cannot change the filename extension")); + ->error_messages("illegal_data_file_extension", t("You cannot change the photo file extension")) + ->error_messages("required", t("You must provide a photo file name")) + ->error_messages("length", t("Your photo file name is too long")); $group->input("slug")->label(t("Internet Address"))->value($photo->slug) ->error_messages( "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")); + t("The internet address should contain only letters, numbers, hyphens and underscores")) + ->error_messages("required", t("You must provide an internet address")) + ->error_messages("length", t("Your internet address is too long")); module::event("item_edit_form", $photo, $form); -- cgit v1.2.3 From 0e5b5e25595dd3dd75222cf8ead58e32f8d5859f Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Wed, 27 Jan 2010 23:00:49 -0800 Subject: Fix capitalization of "internet address". --- modules/gallery/helpers/album.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'modules/gallery/helpers') diff --git a/modules/gallery/helpers/album.php b/modules/gallery/helpers/album.php index 30eeb960..389f6e48 100644 --- a/modules/gallery/helpers/album.php +++ b/modules/gallery/helpers/album.php @@ -41,8 +41,8 @@ class album_Core { ->error_messages( "not_url_safe", t("The internet address should contain only letters, numbers, hyphens and underscores")) - ->error_messages("required", t("You must provide an Internet Address")) - ->error_messages("length", t("Your Internet Address is too long")); + ->error_messages("required", t("You must provide an internet address")) + ->error_messages("length", t("Your internet address is too long")); $group->hidden("type")->value("album"); $group->submit("")->value(t("Create")); $form->script("") @@ -73,8 +73,8 @@ class album_Core { ->error_messages( "not_url_safe", t("The internet address should contain only letters, numbers, hyphens and underscores")) - ->error_messages("required", t("You must provide an Internet Address")) - ->error_messages("length", t("Your Internet Address is too long")); + ->error_messages("required", t("You must provide an internet address")) + ->error_messages("length", t("Your internet address is too long")); } else { $group->hidden("name")->value($parent->name); $group->hidden("slug")->value($parent->slug); -- cgit v1.2.3 From bbe70119ef99e77a57dbc5354bc348c7adaece46 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Wed, 27 Jan 2010 23:05:57 -0800 Subject: Localize validation messages. --- modules/comment/helpers/comment.php | 3 --- modules/gallery/helpers/movie.php | 12 +++++++++--- 2 files changed, 9 insertions(+), 6 deletions(-) (limited to 'modules/gallery/helpers') diff --git a/modules/comment/helpers/comment.php b/modules/comment/helpers/comment.php index c9c20879..f710ad92 100644 --- a/modules/comment/helpers/comment.php +++ b/modules/comment/helpers/comment.php @@ -50,10 +50,7 @@ class comment_Core { $group->inputs["name"]->value($active->full_name)->disabled("disabled"); $group->email->value($active->email)->disabled("disabled"); $group->url->value($active->url)->disabled("disabled"); - } else { - $group->inputs["name"]->error_messages("missing", t("You must provide a name")); } - $group->text->error_messages("missing", t("You must provide a comment")); return $form; } diff --git a/modules/gallery/helpers/movie.php b/modules/gallery/helpers/movie.php index b07a9e69..7033b7da 100644 --- a/modules/gallery/helpers/movie.php +++ b/modules/gallery/helpers/movie.php @@ -28,20 +28,26 @@ class movie_Core { $form = new Forge("movies/update/$movie->id", "", "post", array("id" => "g-edit-movie-form")); $form->hidden("from_id"); $group = $form->group("edit_item")->label(t("Edit Movie")); - $group->input("title")->label(t("Title"))->value($movie->title); + $group->input("title")->label(t("Title"))->value($movie->title) + ->error_messages("required", t("You must provide a title")) + ->error_messages("length", t("Your title is too long")); $group->textarea("description")->label(t("Description"))->value($movie->description); $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 \"/\"")) ->error_messages("no_trailing_period", t("The movie name can't end in \".\"")) - ->error_messages("illegal_extension", t("You cannot change the filename extension")); + ->error_messages("illegal_data_file_extension", t("You cannot change the movie file extension")) + ->error_messages("required", t("You must provide a movie file name")) + ->error_messages("length", t("Your movie file name is too long")); $group->input("slug")->label(t("Internet Address"))->value($movie->slug) ->error_messages( "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")); + t("The internet address should contain only letters, numbers, hyphens and underscores")) + ->error_messages("required", t("You must provide an internet address")) + ->error_messages("length", t("Your internet address is too long")); module::event("item_edit_form", $movie, $form); -- 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/helpers') 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 c51fe9682075c961972c344f4888a4adceabe3eb Mon Sep 17 00:00:00 2001 From: Tim Almdal Date: Thu, 28 Jan 2010 09:27:27 -0800 Subject: Make the varible for the profile name more descriptive and clean the label --- modules/gallery/helpers/gallery_event.php | 4 ++-- modules/gallery/tests/xss_data.txt | 1 - modules/gallery/views/user_profile_info.html.php | 4 ++-- 3 files changed, 4 insertions(+), 5 deletions(-) (limited to 'modules/gallery/helpers') diff --git a/modules/gallery/helpers/gallery_event.php b/modules/gallery/helpers/gallery_event.php index 9b252f61..b3d4daab 100644 --- a/modules/gallery/helpers/gallery_event.php +++ b/modules/gallery/helpers/gallery_event.php @@ -404,14 +404,14 @@ class gallery_event_Core { if (!$data->display_all) { $fields = array("name" => t("Name"), "full_name" => t("Full name"), "url" => "Web site"); } - $v->fields = array(); + $v->user_profile_data = array(); foreach ($fields as $field => $label) { if (!empty($data->user->$field)) { $value = $data->user->$field; if ($field == "locale") { $value = locales::display_name($value); } - $v->fields[(string) $label] = $value; + $v->user_profile_data[(string) $label] = $value; } } $data->content[] = (object) array("title" => t("User information"), "view" => $v); diff --git a/modules/gallery/tests/xss_data.txt b/modules/gallery/tests/xss_data.txt index 04add4c7..663080a0 100644 --- a/modules/gallery/tests/xss_data.txt +++ b/modules/gallery/tests/xss_data.txt @@ -224,7 +224,6 @@ modules/gallery/views/upgrader.html.php 102 DIRTY_ATTR $don modules/gallery/views/user_languages_block.html.php 2 DIRTY form::dropdown("g-select-session-locale",$installed_locales,$selected) modules/gallery/views/user_profile.html.php 35 DIRTY_ATTR $user->avatar_url(40,$theme->url(,true)) modules/gallery/views/user_profile.html.php 46 DIRTY $info->view -modules/gallery/views/user_profile_info.html.php 5 DIRTY $field modules/image_block/views/image_block_block.html.php 3 DIRTY_JS $item->url() modules/image_block/views/image_block_block.html.php 4 DIRTY $item->thumb_img(array("class"=>"g-thumbnail")) modules/info/views/info_block.html.php 22 DIRTY date("M j, Y H:i:s",$item->captured) diff --git a/modules/gallery/views/user_profile_info.html.php b/modules/gallery/views/user_profile_info.html.php index 2f2d68d3..58e134bb 100644 --- a/modules/gallery/views/user_profile_info.html.php +++ b/modules/gallery/views/user_profile_info.html.php @@ -1,8 +1,8 @@ - $value): ?> + $value): ?> - + -- cgit v1.2.3 From 11fbcfeb25a9da60737807d2e0705993d93d24da Mon Sep 17 00:00:00 2001 From: Tim Almdal Date: Thu, 28 Jan 2010 09:55:41 -0800 Subject: Found another broken link for what should have been the user profile --- modules/gallery/helpers/auth.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'modules/gallery/helpers') diff --git a/modules/gallery/helpers/auth.php b/modules/gallery/helpers/auth.php index 21a39bfb..bdff2f70 100644 --- a/modules/gallery/helpers/auth.php +++ b/modules/gallery/helpers/auth.php @@ -51,6 +51,8 @@ class auth_Core { module::event("user_logout", $user); } log::info("user", t("User %name logged out", array("name" => $user->name)), - html::anchor("user/$user->id", html::clean($user->name))); + t('%user_name', + array("url" => user_profile::url($user->id), + "user_name" => html::clean($user->name)))); } } \ No newline at end of file -- cgit v1.2.3 From 5c527513c688571adcff45f513efff54b9c55e61 Mon Sep 17 00:00:00 2001 From: Andy Staudacher Date: Thu, 28 Jan 2010 19:46:53 -0800 Subject: Fix language preference block / language cookie reading. The preference block must have been broken by a jquery update, and the cookie reading by a Kohana update. --- modules/gallery/helpers/locales.php | 4 +++- modules/gallery/views/user_languages_block.html.php | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) (limited to 'modules/gallery/helpers') diff --git a/modules/gallery/helpers/locales.php b/modules/gallery/helpers/locales.php index 5c8c227a..dc32b12f 100644 --- a/modules/gallery/helpers/locales.php +++ b/modules/gallery/helpers/locales.php @@ -238,7 +238,9 @@ class locales_Core { } static function cookie_locale() { - $cookie_data = Input::instance()->cookie("g_locale"); + // Can't use Input framework for client side cookies since + // they're not signed. + $cookie_data = isset($_COOKIE["g_locale"]) ? $_COOKIE["g_locale"] : null; $locale = null; if ($cookie_data) { if (preg_match("/^([a-z]{2,3}(?:_[A-Z]{2})?)$/", trim($cookie_data), $matches)) { diff --git a/modules/gallery/views/user_languages_block.html.php b/modules/gallery/views/user_languages_block.html.php index 89185967..3776ca13 100644 --- a/modules/gallery/views/user_languages_block.html.php +++ b/modules/gallery/views/user_languages_block.html.php @@ -1,7 +1,7 @@