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/tag/helpers/tag_rest.php | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) (limited to 'modules/tag/helpers') diff --git a/modules/tag/helpers/tag_rest.php b/modules/tag/helpers/tag_rest.php index cd1ca6c6..0c06587b 100644 --- a/modules/tag/helpers/tag_rest.php +++ b/modules/tag/helpers/tag_rest.php @@ -55,12 +55,12 @@ class tag_rest_Core { } } - return rest::success($resources); + return rest::reply($resources); } static function post($request) { if (empty($request->arguments) || count($request->arguments) != 1 || empty($request->path)) { - throw new Rest_Exception(400, "Bad request"); + throw new Rest_Exception("Bad request", 400); } $path = $request->path; $tags = explode(",", $request->arguments[0]); @@ -80,12 +80,12 @@ class tag_rest_Core { foreach ($tags as $tag) { tag::add($item, $tag); } - return rest::success(); + return rest::reply(); } static function put($request) { if (empty($request->arguments[0]) || empty($request->new_name)) { - throw new Rest_Exception(400, "Bad request"); + throw new Rest_Exception("Bad request", 400); } $name = $request->arguments[0]; @@ -100,12 +100,12 @@ class tag_rest_Core { $tag->name = $request->new_name; $tag->save(); - return rest::success(); + return rest::reply(); } static function delete($request) { if (empty($request->arguments[0])) { - throw new Rest_Exception(400, "Bad request"); + throw new Rest_Exception("Bad request", 400); } $tags = explode(",", $request->arguments[0]); if (!empty($request->path)) { @@ -127,7 +127,7 @@ class tag_rest_Core { }; tag::compact(); - return rest::success(); + return rest::reply(); } private static function _get_items($request) { -- 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/tag/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/tag/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 bb79a1455a3f692c813f8dad6300fd7ecc6fd583 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Sun, 17 Jan 2010 16:55:11 -0800 Subject: Remove unnecessary comment. --- modules/tag/helpers/tag_rest.php | 4 ---- 1 file changed, 4 deletions(-) (limited to 'modules/tag/helpers') diff --git a/modules/tag/helpers/tag_rest.php b/modules/tag/helpers/tag_rest.php index c1bbf4fb..0aac5291 100644 --- a/modules/tag/helpers/tag_rest.php +++ b/modules/tag/helpers/tag_rest.php @@ -44,10 +44,6 @@ class tag_rest_Core { static function put($request) { $tag = rest::resolve($request->url); - // @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); -- cgit v1.2.3 From 9e9c5397b7602107afdda2894d2bf2bbd8ce652c Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Sun, 17 Jan 2010 16:59:25 -0800 Subject: Qualify the Bad Request output when the name is missing --- modules/tag/helpers/tags_rest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'modules/tag/helpers') diff --git a/modules/tag/helpers/tags_rest.php b/modules/tag/helpers/tags_rest.php index 3ef897fd..7f0ed66a 100644 --- a/modules/tag/helpers/tags_rest.php +++ b/modules/tag/helpers/tags_rest.php @@ -33,7 +33,7 @@ class tags_rest_Core { access::required("edit", item::root()); if (empty($request->params->name)) { - throw new Rest_Exception("Bad Request", 400); + throw new Rest_Exception("Bad Request: missing name", 400); } $tag = ORM::factory("tag")->where("name", "=", $request->params->name)->find(); -- 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/tag/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 549b88643acd571367a3b21f6dce6d42af7e7edf Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Tue, 19 Jan 2010 01:35:59 -0800 Subject: Fix a typo "tags" -> "tag" --- modules/tag/helpers/tags_rest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'modules/tag/helpers') diff --git a/modules/tag/helpers/tags_rest.php b/modules/tag/helpers/tags_rest.php index dd23e97f..41317ecd 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("tags", $tag); + $tags[$tag->name] = rest::url("tag", $tag); } return array("members" => $tags); } -- cgit v1.2.3 From 0b5ce9dbc1c5571ad80d5f4a1df3f21ab29429b4 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Tue, 19 Jan 2010 01:36:15 -0800 Subject: Fix a typo: $item -> $tag. --- modules/tag/helpers/tag_rest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'modules/tag/helpers') diff --git a/modules/tag/helpers/tag_rest.php b/modules/tag/helpers/tag_rest.php index a4eaee90..e7be9afa 100644 --- a/modules/tag/helpers/tag_rest.php +++ b/modules/tag/helpers/tag_rest.php @@ -90,7 +90,7 @@ class tag_rest_Core { return $tag; } - static function url($item) { + static function url($tag) { return url::abs_site("rest/tag/" . rawurlencode($tag->name)); } } -- cgit v1.2.3 From 3d4a6d6336a2eaa212878f8af788825256be9e9c Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Wed, 20 Jan 2010 21:14:40 -0800 Subject: Remove unused untagging code from put() Add access permissions to delete() Remove unnecessary return value from delete() --- modules/tag/helpers/tag_rest.php | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) (limited to 'modules/tag/helpers') diff --git a/modules/tag/helpers/tag_rest.php b/modules/tag/helpers/tag_rest.php index e7be9afa..0226c6d3 100644 --- a/modules/tag/helpers/tag_rest.php +++ b/modules/tag/helpers/tag_rest.php @@ -43,19 +43,6 @@ class tag_rest_Core { static function put($request) { $tag = rest::resolve($request->url); - - if (isset($request->params->remove)) { - if (!is_array($request->params->remove)) { - throw new Exception("Bad request", 400); - } - - foreach ($request->params->remove as $item_url) { - $item = rest::resolve($item_url); - access::required("edit", $item); - $tag->remove($item); - } - } - if (isset($request->params->name)) { $tag->name = $request->params->name; } @@ -73,11 +60,10 @@ class tag_rest_Core { } else { // Remove an item from the tag $item = rest::resolve($request->params->url); + access::required("edit", $item); $tag->remove($item); $tag->save(); - tag::compact(); - return array("url" => rest::url("tag", $tag)); } } -- cgit v1.2.3 From 3665391f8bbf40aa27acc816dc546237461e1cba Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Thu, 21 Jan 2010 21:29:42 -0800 Subject: Guard against division by zero. --- modules/tag/helpers/tag.php | 3 +++ 1 file changed, 3 insertions(+) (limited to 'modules/tag/helpers') diff --git a/modules/tag/helpers/tag.php b/modules/tag/helpers/tag.php index d895e08f..c49a2d0f 100644 --- a/modules/tag/helpers/tag.php +++ b/modules/tag/helpers/tag.php @@ -73,6 +73,9 @@ class tag_Core { if ($tags) { $cloud = new View("tag_cloud.html"); $cloud->max_count = $tags[0]->count; + if (!$cloud->max_count) { + return; + } usort($tags, array("tag", "sort_by_name")); $cloud->tags = $tags; return $cloud; -- 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/tag/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 a445a64bcbfc5df45f255e02d3295156e199832d Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Fri, 22 Jan 2010 01:10:00 -0800 Subject: Return the url of the newly created item from post(). --- modules/tag/helpers/tag_rest.php | 1 + 1 file changed, 1 insertion(+) (limited to 'modules/tag/helpers') diff --git a/modules/tag/helpers/tag_rest.php b/modules/tag/helpers/tag_rest.php index d68cb73d..5feeb756 100644 --- a/modules/tag/helpers/tag_rest.php +++ b/modules/tag/helpers/tag_rest.php @@ -44,6 +44,7 @@ class tag_rest_Core { access::required("edit", $item); tag::add($item, $tag->name); + return array("url" => rest::url("tag_item", $tag, $item)); } static function put($request) { -- cgit v1.2.3 From eea27c39f1cc7edb9ad0f9682bedfb2770661a7f Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Fri, 22 Jan 2010 09:43:02 -0800 Subject: Create symmetrical relationship collections: item_tags and tag_items Now when we represent a relationship collection we can refer to it in proper semantic terms. --- modules/tag/helpers/item_tags_rest.php | 51 ++++++++++++++++++++++++++++++++++ modules/tag/helpers/tag_items_rest.php | 48 ++++++++++++++++++++++++++++++++ modules/tag/helpers/tag_rest.php | 6 +++- 3 files changed, 104 insertions(+), 1 deletion(-) create mode 100644 modules/tag/helpers/item_tags_rest.php create mode 100644 modules/tag/helpers/tag_items_rest.php (limited to 'modules/tag/helpers') diff --git a/modules/tag/helpers/item_tags_rest.php b/modules/tag/helpers/item_tags_rest.php new file mode 100644 index 00000000..385d06e8 --- /dev/null +++ b/modules/tag/helpers/item_tags_rest.php @@ -0,0 +1,51 @@ +url); + $tags = array(); + foreach (tag::item_tags($item) as $tag) { + $tags[] = rest::url("tag_item", $tag, $item); + } + + return array( + "url" => $request->url, + "members" => $tags); + } + + static function delete($request) { + list ($tag, $item) = rest::resolve($request->url); + $tag->remove($item); + $tag->save(); + } + + 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_tags/{$item->id}"); + } +} diff --git a/modules/tag/helpers/tag_items_rest.php b/modules/tag/helpers/tag_items_rest.php new file mode 100644 index 00000000..2a00c565 --- /dev/null +++ b/modules/tag/helpers/tag_items_rest.php @@ -0,0 +1,48 @@ +url); + $items = array(); + foreach ($tag->items() as $item) { + if (access::can("view", $item)) { + $items[] = rest::url("tag_item", $tag, $item); + } + } + + return array( + "url" => $request->url, + "members" => $items); + } + + static function delete($request) { + list ($tag, $item) = rest::resolve($request->url); + $tag->remove($item); + $tag->save(); + } + + static function resolve($id) { + return ORM::factory("tag")->where("id", "=", $id)->find(); + } + + static function url($tag) { + return url::abs_site("rest/tag_items/{$tag->id}"); + } +} diff --git a/modules/tag/helpers/tag_rest.php b/modules/tag/helpers/tag_rest.php index 5feeb756..4fe9bef9 100644 --- a/modules/tag/helpers/tag_rest.php +++ b/modules/tag/helpers/tag_rest.php @@ -31,6 +31,7 @@ class tag_rest_Core { "url" => $request->url, "resource" => $tag->as_array(), "relationships" => array( + "url" => rest::url("tag_items", $tag), "items" => $tag_items)); } @@ -67,7 +68,10 @@ class tag_rest_Core { foreach (tag::item_tags($resource) as $tag) { $tags[] = rest::url("tag_item", $tag, $resource); } - return array("tags" => $tags); + return array( + "tags" => array( + "url" => rest::url("item_tags", $resource), + "members" => $tags)); } } -- cgit v1.2.3 From a60969401852ddda878bd2a3444d6378899d4dcc Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Sat, 23 Jan 2010 12:13:14 -0800 Subject: Update tests for recent REST changes. --- modules/gallery/tests/Item_Rest_Helper_Test.php | 20 +++++++++++++++----- modules/rest/tests/Rest_Controller_Test.php | 2 +- modules/tag/helpers/tag_rest.php | 5 +++-- modules/tag/tests/Tag_Rest_Helper_Test.php | 8 ++++++-- 4 files changed, 25 insertions(+), 10 deletions(-) (limited to 'modules/tag/helpers') diff --git a/modules/gallery/tests/Item_Rest_Helper_Test.php b/modules/gallery/tests/Item_Rest_Helper_Test.php index 115d3b1b..8ce6bc43 100644 --- a/modules/gallery/tests/Item_Rest_Helper_Test.php +++ b/modules/gallery/tests/Item_Rest_Helper_Test.php @@ -41,7 +41,9 @@ class Item_Rest_Helper_Test extends Gallery_Unit_Test_Case { rest::url("item", $photo1), rest::url("item", $album2)), "relationships" => array( - "tags" => array())), + "tags" => array( + "url" => rest::url("item_tags", $album1), + "members" => array()))), item_rest::get($request)); $request->url = rest::url("item", $album1); @@ -53,7 +55,9 @@ class Item_Rest_Helper_Test extends Gallery_Unit_Test_Case { rest::url("item", $photo1), rest::url("item", $album2)), "relationships" => array( - "tags" => array())), + "tags" => array( + "url" => rest::url("item_tags", $album1), + "members" => array()))), item_rest::get($request)); $request->url = rest::url("item", $album1); @@ -66,7 +70,9 @@ class Item_Rest_Helper_Test extends Gallery_Unit_Test_Case { rest::url("item", $album2), rest::url("item", $photo2)), "relationships" => array( - "tags" => array())), + "tags" => array( + "url" => rest::url("item_tags", $album1), + "members" => array()))), item_rest::get($request)); } @@ -86,7 +92,9 @@ class Item_Rest_Helper_Test extends Gallery_Unit_Test_Case { "members" => array( rest::url("item", $photo2)), "relationships" => array( - "tags" => array())), + "tags" => array( + "url" => rest::url("item_tags", $album1), + "members" => array()))), item_rest::get($request)); } @@ -104,7 +112,9 @@ class Item_Rest_Helper_Test extends Gallery_Unit_Test_Case { "members" => array( rest::url("item", $album2)), "relationships" => array( - "tags" => array())), + "tags" => array( + "url" => rest::url("item_tags", $album1), + "members" => array() ))), item_rest::get($request)); } diff --git a/modules/rest/tests/Rest_Controller_Test.php b/modules/rest/tests/Rest_Controller_Test.php index 377f5334..5e624112 100644 --- a/modules/rest/tests/Rest_Controller_Test.php +++ b/modules/rest/tests/Rest_Controller_Test.php @@ -130,7 +130,7 @@ class Rest_Controller_Test extends Gallery_Unit_Test_Case { try { test::call_and_capture(array(new Rest_Controller(), "mock")); } catch (Exception $e) { - $this->assert_equal(403, $e->getCode()); + $this->assert_equal(400, $e->getCode()); return; } $this->assert_true(false, "Shouldn't get here"); diff --git a/modules/tag/helpers/tag_rest.php b/modules/tag/helpers/tag_rest.php index 4fe9bef9..4b2a4b46 100644 --- a/modules/tag/helpers/tag_rest.php +++ b/modules/tag/helpers/tag_rest.php @@ -31,8 +31,9 @@ class tag_rest_Core { "url" => $request->url, "resource" => $tag->as_array(), "relationships" => array( - "url" => rest::url("tag_items", $tag), - "items" => $tag_items)); + "items" => array( + "url" => rest::url("tag_items", $tag), + "members" => $tag_items))); } static function post($request) { diff --git a/modules/tag/tests/Tag_Rest_Helper_Test.php b/modules/tag/tests/Tag_Rest_Helper_Test.php index cbd7b6cd..eacf91b3 100644 --- a/modules/tag/tests/Tag_Rest_Helper_Test.php +++ b/modules/tag/tests/Tag_Rest_Helper_Test.php @@ -34,7 +34,9 @@ class Tag_Rest_Helper_Test extends Gallery_Unit_Test_Case { "resource" => $tag->as_array(), "relationships" => array( "items" => array( - rest::url("tag_item", $tag, item::root())))), + "url" => rest::url("tag_items", $tag), + "members" => array( + rest::url("tag_item", $tag, item::root()))))), tag_rest::get($request)); } @@ -56,7 +58,9 @@ class Tag_Rest_Helper_Test extends Gallery_Unit_Test_Case { array("url" => rest::url("tag", $tag), "resource" => $tag->as_array(), "relationships" => array( - "items" => array())), + "items" => array( + "url" => rest::url("tag_items", $tag), + "members" => array()))), tag_rest::get($request)); } -- cgit v1.2.3 From cc79abd0af83fdde471031b0629e8c7b0318790d Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Sun, 24 Jan 2010 14:00:07 -0800 Subject: Simplify tag::add(). --- modules/tag/helpers/tag.php | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) (limited to 'modules/tag/helpers') diff --git a/modules/tag/helpers/tag.php b/modules/tag/helpers/tag.php index 9e59b527..a500be58 100644 --- a/modules/tag/helpers/tag.php +++ b/modules/tag/helpers/tag.php @@ -37,17 +37,11 @@ class tag_Core { if (!$tag->loaded()) { $tag->name = $tag_name; $tag->count = 0; - $tag->save(); } - if (!$tag->has($item)) { - if (!$tag->add($item)) { - throw new Exception("@todo {$tag->name} WAS_NOT_ADDED_TO {$item->id}"); - } - $tag->count++; - $tag->save(); - } - return $tag; + $tag->add($item); + $tag->count++; + return $tag->save(); } /** -- cgit v1.2.3 From f268128043a56888ef35d07ee5d02012b8349c74 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Tue, 26 Jan 2010 00:24:10 -0800 Subject: Add support for adding tags in the item_tags and tag_items relationship collections. --- modules/tag/helpers/item_tags_rest.php | 13 +++++++++++++ modules/tag/helpers/tag_items_rest.php | 13 +++++++++++++ modules/tag/helpers/tags_rest.php | 7 ++++++- 3 files changed, 32 insertions(+), 1 deletion(-) (limited to 'modules/tag/helpers') diff --git a/modules/tag/helpers/item_tags_rest.php b/modules/tag/helpers/item_tags_rest.php index 385d06e8..ce814f77 100644 --- a/modules/tag/helpers/item_tags_rest.php +++ b/modules/tag/helpers/item_tags_rest.php @@ -30,6 +30,19 @@ class item_tags_rest_Core { "members" => $tags); } + static function post($request) { + $tag = rest::resolve($request->params->tag); + $item = rest::resolve($request->params->item); + access::required("view", $item); + + tag::add($item, $tag->name); + return array( + "url" => rest::url("tag_item", $tag, $item), + "members" => array( + rest::url("tag", $tag), + rest::url("item", $item))); + } + static function delete($request) { list ($tag, $item) = rest::resolve($request->url); $tag->remove($item); diff --git a/modules/tag/helpers/tag_items_rest.php b/modules/tag/helpers/tag_items_rest.php index 2a00c565..369a8d83 100644 --- a/modules/tag/helpers/tag_items_rest.php +++ b/modules/tag/helpers/tag_items_rest.php @@ -32,6 +32,19 @@ class tag_items_rest_Core { "members" => $items); } + static function post($request) { + $tag = rest::resolve($request->params->tag); + $item = rest::resolve($request->params->item); + access::required("view", $item); + + tag::add($item, $tag->name); + return array( + "url" => rest::url("tag_item", $tag, $item), + "members" => array( + rest::url("tag", $tag), + rest::url("item", $item))); + } + static function delete($request) { list ($tag, $item) = rest::resolve($request->url); $tag->remove($item); diff --git a/modules/tag/helpers/tags_rest.php b/modules/tag/helpers/tags_rest.php index 57461125..89ff0f21 100644 --- a/modules/tag/helpers/tags_rest.php +++ b/modules/tag/helpers/tags_rest.php @@ -23,7 +23,8 @@ class tags_rest_Core { foreach (ORM::factory("tag")->find_all() as $tag) { $tags[] = rest::url("tag", $tag); } - return array("members" => $tags); + return array("url" => rest::url("tags"), + "members" => $tags); } static function post($request) { @@ -45,4 +46,8 @@ class tags_rest_Core { return array("url" => rest::url("tag", $tag)); } + + static function url() { + return url::abs_site("rest/tags"); + } } -- 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/tag/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 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/tag/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 a04d0d278964c93b4829ec2e77f5f315abcba392 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Fri, 29 Jan 2010 19:42:38 -0800 Subject: Add missing permission checks. Make the tag relationship an associative array. --- modules/tag/helpers/tag_item_rest.php | 6 +++--- modules/tag/helpers/tag_items_rest.php | 8 ++++++-- modules/tag/tests/Tag_Item_Rest_Helper_Test.php | 4 ++-- 3 files changed, 11 insertions(+), 7 deletions(-) (limited to 'modules/tag/helpers') diff --git a/modules/tag/helpers/tag_item_rest.php b/modules/tag/helpers/tag_item_rest.php index 60d37437..672cec53 100644 --- a/modules/tag/helpers/tag_item_rest.php +++ b/modules/tag/helpers/tag_item_rest.php @@ -23,8 +23,8 @@ class tag_item_rest_Core { return array( "url" => $request->url, "members" => array( - rest::url("tag", $tag), - rest::url("item", $item))); + "tag" => rest::url("tag", $tag), + "item" => rest::url("item", $item))); } static function delete($request) { @@ -37,7 +37,7 @@ class tag_item_rest_Core { list ($tag_id, $item_id) = split(",", $tuple); $tag = ORM::factory("tag", $tag_id); $item = ORM::factory("item", $item_id); - if (!$tag->loaded() || !$item->loaded() || !$tag->has($item)) { + if (!$tag->loaded() || !$item->loaded() || !$tag->has($item) || !access::can("view", $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 ef563ac6..18973ebb 100644 --- a/modules/tag/helpers/tag_items_rest.php +++ b/modules/tag/helpers/tag_items_rest.php @@ -37,12 +37,16 @@ class tag_items_rest_Core { $item = rest::resolve($request->params->item); access::required("view", $item); + if (!$tag->loaded()) { + throw new Kohana_404_Exception(); + } + tag::add($item, $tag->name); return array( "url" => rest::url("tag_item", $tag, $item), "members" => array( - rest::url("tag", $tag), - rest::url("item", $item))); + "tag" => rest::url("tag", $tag), + "item" => rest::url("item", $item))); } static function delete($request) { diff --git a/modules/tag/tests/Tag_Item_Rest_Helper_Test.php b/modules/tag/tests/Tag_Item_Rest_Helper_Test.php index 6c49ad67..69c580f1 100644 --- a/modules/tag/tests/Tag_Item_Rest_Helper_Test.php +++ b/modules/tag/tests/Tag_Item_Rest_Helper_Test.php @@ -32,8 +32,8 @@ class Tag_Item_Rest_Helper_Test extends Gallery_Unit_Test_Case { $this->assert_equal_array( array("url" => rest::url("tag_item", $tag, item::root()), "members" => array( - rest::url("tag", $tag), - rest::url("item", item::root()))), + "tag" => rest::url("tag", $tag), + "item" => rest::url("item", item::root()))), tag_item_rest::get($request)); } -- cgit v1.2.3 From 923a515ffb33d0ba15730ef4716f8ffe45cf19fa Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Sat, 30 Jan 2010 11:48:43 -0800 Subject: The user must have some edit permission somewhere to create a tag --- modules/tag/helpers/tags_rest.php | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) (limited to 'modules/tag/helpers') diff --git a/modules/tag/helpers/tags_rest.php b/modules/tag/helpers/tags_rest.php index 89ff0f21..ac0eb81d 100644 --- a/modules/tag/helpers/tags_rest.php +++ b/modules/tag/helpers/tags_rest.php @@ -28,10 +28,18 @@ class tags_rest_Core { } 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()); + // The user must have some edit permission somewhere to create a tag. + if (!identity::active_user()->admin) { + $query = db::build()->from("access_caches")->and_open(); + foreach (identity::active_user()->groups() as $group) { + $query->or_where("edit_{$group->id}", "=", access::ALLOW); + } + $has_any_edit_perm = $query->close()->count_records(); + + if (!$has_any_edit_perm) { + access::forbidden(); + } + } if (empty($request->params->name)) { throw new Rest_Exception("Bad Request", 400); -- cgit v1.2.3 From c050acf30a7351bf0ef5b8ee206704c073e881c7 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Sun, 31 Jan 2010 16:07:41 -0800 Subject: Fix lots of warnings that pop up when we're in E_STRICT mode. They're mostly issues around uninitialized variables, calling non-static functions in a static context, calling Session functions directly instead of on its singleton, passing non-variables by reference, and subclasses not using the same interface as the parent class. --- modules/comment/controllers/admin_comments.php | 1 + modules/comment/helpers/comment_rss.php | 1 + modules/comment/models/comment.php | 3 ++- modules/digibug/controllers/digibug.php | 2 +- modules/forge/libraries/Form_Group.php | 2 +- modules/g2_import/controllers/admin_g2_import.php | 5 +++++ modules/g2_import/helpers/g2_import.php | 11 +++++++++++ modules/g2_import/helpers/g2_import_task.php | 10 ++++++++-- modules/g2_import/views/admin_g2_import.html.php | 2 +- modules/gallery/controllers/admin_modules.php | 1 + modules/gallery/controllers/combined.php | 2 +- modules/gallery/controllers/file_proxy.php | 2 +- modules/gallery/helpers/gallery_block.php | 2 +- modules/gallery/helpers/gallery_rss.php | 1 + modules/gallery/helpers/gallery_task.php | 11 ++++++++--- modules/gallery/helpers/graphics.php | 3 +++ modules/gallery/helpers/l10n_client.php | 1 + modules/gallery/helpers/module.php | 3 ++- modules/gallery/libraries/Form_Script.php | 4 ++-- modules/gallery/libraries/MY_Database.php | 2 +- modules/gallery/libraries/MY_View.php | 2 +- modules/gallery/libraries/ORM_MPTT.php | 2 +- modules/gallery/models/item.php | 2 +- modules/gallery/models/task.php | 4 ++-- modules/gallery/tests/Database_Test.php | 8 ++++---- modules/gallery/tests/Item_Rest_Helper_Test.php | 17 +++++++++++++++++ .../gallery_unit_test/controllers/gallery_unit_test.php | 6 +++++- modules/rest/controllers/rest.php | 1 + modules/rest/helpers/rest.php | 2 +- modules/rest/tests/Rest_Controller_Test.php | 8 ++++---- modules/search/helpers/search.php | 3 ++- modules/tag/helpers/tag_rss.php | 2 ++ modules/tag/helpers/tags_rest.php | 1 - modules/tag/models/tag.php | 2 +- modules/tag/tests/Tag_Item_Rest_Helper_Test.php | 4 +++- modules/tag/tests/Tag_Rest_Helper_Test.php | 8 ++++++++ modules/tag/tests/Tags_Rest_Helper_Test.php | 4 ++++ modules/user/controllers/admin_users.php | 2 +- 38 files changed, 111 insertions(+), 36 deletions(-) (limited to 'modules/tag/helpers') diff --git a/modules/comment/controllers/admin_comments.php b/modules/comment/controllers/admin_comments.php index b7dc5fb3..3dd45919 100644 --- a/modules/comment/controllers/admin_comments.php +++ b/modules/comment/controllers/admin_comments.php @@ -92,6 +92,7 @@ class Admin_Comments_Controller extends Admin_Controller { } private function _counts() { + $counts = new stdClass(); $counts->unpublished = 0; $counts->published = 0; $counts->spam = 0; diff --git a/modules/comment/helpers/comment_rss.php b/modules/comment/helpers/comment_rss.php index 77044884..79fa07df 100644 --- a/modules/comment/helpers/comment_rss.php +++ b/modules/comment/helpers/comment_rss.php @@ -42,6 +42,7 @@ class comment_rss_Core { $comments->where("item_id", "=", $id); } + $feed = new stdClass(); $feed->view = "comment.mrss"; $feed->children = array(); foreach ($comments->find_all($limit, $offset) as $comment) { diff --git a/modules/comment/models/comment.php b/modules/comment/models/comment.php index add15ce8..d9d05995 100644 --- a/modules/comment/models/comment.php +++ b/modules/comment/models/comment.php @@ -116,7 +116,8 @@ class Comment_Model extends ORM { // We only notify on the related items if we're making a visible change. if ($visible_change) { - module::event("item_related_update", $this->item()); + $item = $this->item(); + module::event("item_related_update", $item); } return $this; diff --git a/modules/digibug/controllers/digibug.php b/modules/digibug/controllers/digibug.php index e3b06196..c98ae20c 100644 --- a/modules/digibug/controllers/digibug.php +++ b/modules/digibug/controllers/digibug.php @@ -91,7 +91,7 @@ class Digibug_Controller extends Controller { } // We don't need to save the session for this request - Session::abort_save(); + Session::instance()->abort_save(); if (!TEST_MODE) { // Dump out the image diff --git a/modules/forge/libraries/Form_Group.php b/modules/forge/libraries/Form_Group.php index e0601321..0a04912b 100644 --- a/modules/forge/libraries/Form_Group.php +++ b/modules/forge/libraries/Form_Group.php @@ -80,7 +80,7 @@ class Form_Group_Core extends Forge { } } - public function render() + public function render($template = 'forge_template', $custom = FALSE) { // No Sir, we don't want any html today thank you return; diff --git a/modules/g2_import/controllers/admin_g2_import.php b/modules/g2_import/controllers/admin_g2_import.php index 1c65f482..6dd155b9 100644 --- a/modules/g2_import/controllers/admin_g2_import.php +++ b/modules/g2_import/controllers/admin_g2_import.php @@ -19,6 +19,7 @@ */ class Admin_g2_import_Controller extends Admin_Controller { public function index() { + g2_import::lower_error_reporting(); if (g2_import::is_configured()) { g2_import::init(); } @@ -31,6 +32,7 @@ class Admin_g2_import_Controller extends Admin_Controller { $view = new Admin_View("admin.html"); $view->content = new View("admin_g2_import.html"); $view->content->form = $this->_get_import_form(); + $view->content->version = g2_import::version(); if (g2_import::is_initialized()) { $view->content->g2_stats = $g2_stats; @@ -38,11 +40,13 @@ class Admin_g2_import_Controller extends Admin_Controller { $view->content->thumb_size = module::get_var("gallery", "thumb_size"); $view->content->resize_size = module::get_var("gallery", "resize_size"); } + g2_import::restore_error_reporting(); print $view; } public function save() { access::verify_csrf(); + g2_import::lower_error_reporting(); $form = $this->_get_import_form(); if ($form->validate()) { @@ -63,6 +67,7 @@ class Admin_g2_import_Controller extends Admin_Controller { $view = new Admin_View("admin.html"); $view->content = new View("admin_g2_import.html"); $view->content->form = $form; + g2_import::restore_error_reporting(); print $view; } diff --git a/modules/g2_import/helpers/g2_import.php b/modules/g2_import/helpers/g2_import.php index fa95e547..0fcc0539 100644 --- a/modules/g2_import/helpers/g2_import.php +++ b/modules/g2_import/helpers/g2_import.php @@ -24,6 +24,7 @@ class g2_import_Core { public static $g2_base_url = null; private static $current_g2_item = null; + private static $error_reporting = null; static function is_configured() { return module::get_var("g2_import", "embed_path"); @@ -931,6 +932,16 @@ class g2_import_Core { "useAuthToken" => false)); return str_replace(self::$g2_base_url, "", $url); } + + static function lower_error_reporting() { + // Gallery 2 was not designed to run in E_STRICT mode and will barf out errors. So dial down + // the error reporting when we make G2 calls. + self::$error_reporting = error_reporting(error_reporting() & ~E_STRICT); + } + + static function restore_error_reporting() { + error_reporting(self::$error_reporting); + } } /** diff --git a/modules/g2_import/helpers/g2_import_task.php b/modules/g2_import/helpers/g2_import_task.php index e80b88b9..21ba4c3a 100644 --- a/modules/g2_import/helpers/g2_import_task.php +++ b/modules/g2_import/helpers/g2_import_task.php @@ -19,17 +19,19 @@ */ class g2_import_task_Core { static function available_tasks() { + g2_import::lower_error_reporting(); if (g2_import::is_configured()) { g2_import::init(); } - + $version = g2_import::version(); + g2_import::restore_error_reporting(); if (class_exists("GalleryCoreApi")) { return array(Task_Definition::factory() ->callback("g2_import_task::import") ->name(t("Import from Gallery 2")) ->description( - t("Gallery %version detected", array("version" => g2_import::version()))) + t("Gallery %version detected", array("version" => $version))) ->severity(log::SUCCESS)); } @@ -37,6 +39,8 @@ class g2_import_task_Core { } static function import($task) { + g2_import::lower_error_reporting(); + $start = microtime(true); g2_import::init(); @@ -207,5 +211,7 @@ class g2_import_task_Core { $task->set("mode", $mode); $task->set("queue", $queue); $task->set("done", $done); + + g2_import::restore_error_reporting(); } } diff --git a/modules/g2_import/views/admin_g2_import.html.php b/modules/g2_import/views/admin_g2_import.html.php index 0875e7f7..6a5214a3 100644 --- a/modules/g2_import/views/admin_g2_import.html.php +++ b/modules/g2_import/views/admin_g2_import.html.php @@ -34,7 +34,7 @@

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