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/rest/helpers/rest.php | 90 ++++++++++--------------------------------- 1 file changed, 20 insertions(+), 70 deletions(-) (limited to 'modules/rest/helpers') diff --git a/modules/rest/helpers/rest.php b/modules/rest/helpers/rest.php index be0644f2..f7f3f9fd 100644 --- a/modules/rest/helpers/rest.php +++ b/modules/rest/helpers/rest.php @@ -18,87 +18,37 @@ * Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, MA 02110-1301, USA. */ class rest_Core { - /** - * Request failed - */ - static function fail($log_message=null) { - if (!empty($log_message)) { - Kohana_Log::add("info", $log_message); - } - // We don't need to save the session for this request + static function reply($data=array()) { Session::abort_save(); - return json_encode(array("status" => "ERROR", "message" => (string)$message)); - } - /** - * Success - */ - static function success($response_data=array(), $message=null) { - $response = array("status" => "OK"); - if (!empty($message)) { - $response["message"] = (string)$message; + if ($data) { + print json_encode($data); } - $response = array_merge($response, $response_data); - - // We don't need to save the session for this request - Session::abort_save(); - return json_encode($response); } - /** - * Validation Error - */ - static function validation_error($error_data) { - $response = array("status" => "VALIDATE_ERROR"); - $response = array_merge($response, array("fields" => $error_data)); - - // We don't need to save the session for this request - Session::abort_save(); - return json_encode($response); - } + static function set_active_user($access_token) { + if (empty($access_token)) { + identity::set_active_user(identity::guest()); + return; + } + $key = ORM::factory("user_access_token") + ->where("access_key", "=", $access_token) + ->find(); - static function normalize_request($args=array()) { - $input = Input::instance(); - $method = strtolower($input->server("REQUEST_METHOD")); - $request = new stdClass(); - foreach (array_keys($input->get()) as $key) { - $request->$key = $input->get($key); - } - if ($method != "get") { - foreach (array_keys($input->post()) as $key) { - $request->$key = $input->post($key); - } - foreach (array_keys($_FILES) as $key) { - $request->$key = $_FILES[$key]; - } + if (!$key->loaded()) { + throw new Rest_Exception("Forbidden", 403); } - $request->method = strtolower($input->server("HTTP_X_GALLERY_REQUEST_METHOD", $method)); - $request->access_token = $input->server("HTTP_X_GALLERY_REQUEST_KEY"); - $request->arguments = $args; // Let the rest handler figure out what the arguments mean + $user = identity::lookup_user($key->user_id); + if (empty($user)) { + throw new Rest_Exception("Forbidden", 403); + } - return $request; + identity::set_active_user($user); } - static function set_active_user($access_token) { - if (empty($access_token)) { - $user = identity::guest(); - } else { - $key = ORM::factory("user_access_token") - ->where("access_key", "=", $access_token) - ->find(); - - if ($key->loaded()) { - $user = identity::lookup_user($key->user_id); - if (empty($user)) { - throw new Rest_Exception(403, "Forbidden"); - } - } else { - throw new Rest_Exception(403, "Forbidden"); - } - } - identity::set_active_user($user); - return true; + static function send_headers($exception) { + header("HTTP/1.1 " . $exception->getCode() . " " . $exception->getMessage()); } } -- 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/rest/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 d43badb4ecac092c897cb52cc54696d1489bc2e0 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Fri, 8 Jan 2010 11:11:38 -0800 Subject: Change url parsing in resolve() to ignore the query string. --- modules/rest/helpers/rest.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'modules/rest/helpers') diff --git a/modules/rest/helpers/rest.php b/modules/rest/helpers/rest.php index b1b83e1b..121191f2 100644 --- a/modules/rest/helpers/rest.php +++ b/modules/rest/helpers/rest.php @@ -60,10 +60,10 @@ class rest_Core { * @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); + $relative_url = substr($url, strlen(url::abs_site("rest"))); + $path = parse_url($relative_url, PHP_URL_PATH); + $components = explode("/", $path, 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); -- cgit v1.2.3 From 2c0b0aaebca6ec1149585853d7d849d50bea4184 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Tue, 19 Jan 2010 00:35:32 -0800 Subject: Add rest::url() for convenience. --- modules/rest/helpers/rest.php | 10 ++++++++++ 1 file changed, 10 insertions(+) (limited to 'modules/rest/helpers') diff --git a/modules/rest/helpers/rest.php b/modules/rest/helpers/rest.php index 121191f2..423765bb 100644 --- a/modules/rest/helpers/rest.php +++ b/modules/rest/helpers/rest.php @@ -71,4 +71,14 @@ class rest_Core { return call_user_func(array($class, "resolve"), !empty($components[2]) ? $components[2] : null); } + + /** + * 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 + */ + static function url($module, $path) { + return url::abs_site("rest/$module/$path"); + } } -- 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/rest/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 b8c09b6d872d00ad74d2f41d7699e12ee10bd361 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Tue, 19 Jan 2010 19:31:24 -0800 Subject: Use an appropriate json content type --- modules/rest/helpers/rest.php | 1 + 1 file changed, 1 insertion(+) (limited to 'modules/rest/helpers') diff --git a/modules/rest/helpers/rest.php b/modules/rest/helpers/rest.php index 93ad2bd3..27bafabd 100644 --- a/modules/rest/helpers/rest.php +++ b/modules/rest/helpers/rest.php @@ -22,6 +22,7 @@ class rest_Core { Session::abort_save(); if ($data) { + header("Content-type: application/json"); print json_encode($data); } } -- cgit v1.2.3 From a774dc5447a1d90cd5c361e9196ab41fd91f92cf Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Tue, 19 Jan 2010 22:38:03 -0800 Subject: Don't send headers if they're already sent. --- modules/rest/helpers/rest.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'modules/rest/helpers') diff --git a/modules/rest/helpers/rest.php b/modules/rest/helpers/rest.php index 27bafabd..76ecef23 100644 --- a/modules/rest/helpers/rest.php +++ b/modules/rest/helpers/rest.php @@ -50,7 +50,9 @@ class rest_Core { } static function send_headers($exception) { - header("HTTP/1.1 " . $exception->getCode() . " " . $exception->getMessage()); + if (!headers_sent()) { + header("HTTP/1.1 " . $exception->getCode() . " " . $exception->getMessage()); + } } /** -- cgit v1.2.3 From c65eca0607572b49ec59a7c387c4b93d15561adb Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Tue, 19 Jan 2010 23:30:09 -0800 Subject: Move rest::send_headers back into Rest_Exception. --- modules/rest/helpers/rest.php | 6 ------ 1 file changed, 6 deletions(-) (limited to 'modules/rest/helpers') diff --git a/modules/rest/helpers/rest.php b/modules/rest/helpers/rest.php index 76ecef23..fccb365e 100644 --- a/modules/rest/helpers/rest.php +++ b/modules/rest/helpers/rest.php @@ -49,12 +49,6 @@ class rest_Core { identity::set_active_user($user); } - static function send_headers($exception) { - if (!headers_sent()) { - 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 -- cgit v1.2.3 From 5119d58e7ff06e1ba922d73bf47aefd242dc888f Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Wed, 20 Jan 2010 00:07:03 -0800 Subject: Move access key creation into a helper function. --- modules/rest/controllers/rest.php | 10 +--------- modules/rest/helpers/rest.php | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 9 deletions(-) (limited to 'modules/rest/helpers') diff --git a/modules/rest/controllers/rest.php b/modules/rest/controllers/rest.php index a6b618e8..9f0bc5b3 100644 --- a/modules/rest/controllers/rest.php +++ b/modules/rest/controllers/rest.php @@ -27,15 +27,7 @@ class Rest_Controller extends Controller { throw new Rest_Exception("Forbidden", 403); } - $key = ORM::factory("user_access_token") - ->where("user_id", "=", $user->id) - ->find(); - if (!$key->loaded()) { - $key->user_id = $user->id; - $key->access_key = md5($user->name . rand()); - $key->save(); - } - + $key = rest::get_access_token($user->id); rest::reply($key->access_key); } diff --git a/modules/rest/helpers/rest.php b/modules/rest/helpers/rest.php index fccb365e..cd3de725 100644 --- a/modules/rest/helpers/rest.php +++ b/modules/rest/helpers/rest.php @@ -49,6 +49,20 @@ class rest_Core { identity::set_active_user($user); } + static function get_access_token($user_id) { + $key = ORM::factory("user_access_token") + ->where("user_id", "=", $user_id) + ->find(); + + if (!$key->loaded()) { + $key->user_id = $user_id; + $key->access_key = md5(rand()); + $key->save(); + } + return $key; + } + + /** * Convert a REST url into an object. * Eg: "http://example.com/gallery3/index.php/rest/gallery/Family/Wedding" -> Item_Model -- cgit v1.2.3 From 3a26ace065476fbc9c2537e97bf7720d3648400f Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Wed, 20 Jan 2010 21:15:44 -0800 Subject: Improve robustness in resolve() against bad urls.. --- modules/rest/helpers/rest.php | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'modules/rest/helpers') diff --git a/modules/rest/helpers/rest.php b/modules/rest/helpers/rest.php index cd3de725..324b7f98 100644 --- a/modules/rest/helpers/rest.php +++ b/modules/rest/helpers/rest.php @@ -75,6 +75,10 @@ class rest_Core { $path = parse_url($relative_url, PHP_URL_PATH); $components = explode("/", $path, 3); + if (count($components) != 3) { + throw new Kohana_404_Exception($url); + } + $class = "$components[1]_rest"; if (!method_exists($class, "resolve")) { throw new Kohana_404_Exception($url); -- cgit v1.2.3 From a0c99794185bbfe227556a4b006b2a6e770b3a78 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Thu, 21 Jan 2010 20:12:28 -0800 Subject: whitespace. --- modules/rest/helpers/rest.php | 1 - 1 file changed, 1 deletion(-) (limited to 'modules/rest/helpers') diff --git a/modules/rest/helpers/rest.php b/modules/rest/helpers/rest.php index 324b7f98..85987ca1 100644 --- a/modules/rest/helpers/rest.php +++ b/modules/rest/helpers/rest.php @@ -62,7 +62,6 @@ class rest_Core { return $key; } - /** * Convert a REST url into an object. * Eg: "http://example.com/gallery3/index.php/rest/gallery/Family/Wedding" -> Item_Model -- 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/rest/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 a0c6d055d1b7259667bebb1b5bf9e2a99c0f8260 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Fri, 22 Jan 2010 00:53:44 -0800 Subject: output_type --> output --- modules/rest/helpers/rest.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'modules/rest/helpers') diff --git a/modules/rest/helpers/rest.php b/modules/rest/helpers/rest.php index fe704a9e..0d2ec9d4 100644 --- a/modules/rest/helpers/rest.php +++ b/modules/rest/helpers/rest.php @@ -22,7 +22,7 @@ class rest_Core { Session::abort_save(); if ($data) { - if (Input::instance()->get("output_type") == "html") { + if (Input::instance()->get("output") == "html") { header("Content-type: text/html"); $html = preg_replace( "#(^|[\n ])([\w]+?://[\w]+[^ \"\n\r\t<]*)#ise", "'\\1\\2'", @@ -112,8 +112,8 @@ class rest_Core { } $url = call_user_func_array(array($class, "url"), $args); - if (Input::instance()->get("output_type") == "html") { - $url .= "?output_type=html"; + if (Input::instance()->get("output") == "html") { + $url .= "?output=html"; } return $url; } -- cgit v1.2.3 From a95609849e42656b35f0110a08cc4436ad56b916 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Fri, 29 Jan 2010 14:53:40 -0800 Subject: Use var_export instead of print_r for better clarity. --- modules/rest/helpers/rest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'modules/rest/helpers') diff --git a/modules/rest/helpers/rest.php b/modules/rest/helpers/rest.php index 0d2ec9d4..1f95a7e7 100644 --- a/modules/rest/helpers/rest.php +++ b/modules/rest/helpers/rest.php @@ -25,8 +25,8 @@ class rest_Core { if (Input::instance()->get("output") == "html") { header("Content-type: text/html"); $html = preg_replace( - "#(^|[\n ])([\w]+?://[\w]+[^ \"\n\r\t<]*)#ise", "'\\1\\2'", - print_r($data, 1)); + "#([\w]+?://[\w]+[^ \'\"\n\r\t<]*)#ise", "'\\1'", + var_export($data, 1)); print "
$html
"; } else { header("Content-type: application/json"); -- cgit v1.2.3 From dcba664f74439e37cc269df0cf549a2fee552aeb Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Fri, 29 Jan 2010 20:37:48 -0800 Subject: Use ? or & as appropriate when appending output=html. --- modules/rest/helpers/rest.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'modules/rest/helpers') diff --git a/modules/rest/helpers/rest.php b/modules/rest/helpers/rest.php index 1f95a7e7..3883794a 100644 --- a/modules/rest/helpers/rest.php +++ b/modules/rest/helpers/rest.php @@ -113,7 +113,11 @@ class rest_Core { $url = call_user_func_array(array($class, "url"), $args); if (Input::instance()->get("output") == "html") { - $url .= "?output=html"; + if (strpos($url, "?") === false) { + $url .= "?output=html"; + } else { + $url .= "&output=html"; + } } return $url; } -- cgit v1.2.3 From 1470b99d1facd07fcb46c0c4e46896d339f5a75a Mon Sep 17 00:00:00 2001 From: Andy Staudacher Date: Sat, 30 Jan 2010 21:42:57 -0800 Subject: Protect REST login controller from brute force attacks too. And make the REST auth token less predictable by using a better source for randomness. --- modules/gallery/helpers/auth.php | 9 +++++++-- modules/rest/controllers/rest.php | 7 +++++++ modules/rest/helpers/rest.php | 2 +- 3 files changed, 15 insertions(+), 3 deletions(-) (limited to 'modules/rest/helpers') diff --git a/modules/gallery/helpers/auth.php b/modules/gallery/helpers/auth.php index e112f127..8c7a0b6d 100644 --- a/modules/gallery/helpers/auth.php +++ b/modules/gallery/helpers/auth.php @@ -64,14 +64,19 @@ class auth_Core { * minute. */ static function validate_too_many_failed_logins($name_input) { + $name = is_object($name_input) ? $name_input->value : $name_input; $failed_login = ORM::factory("failed_login") - ->where("name", "=", $name_input->value) + ->where("name", "=", $name) ->find(); if ($failed_login->loaded() && $failed_login->count > 5 && (time() - $failed_login->time < 60)) { - $name_input->add_error("too_many_failed_logins", 1); + if (is_object($name_input)) { + $name_input->add_error("too_many_failed_logins", 1); + } + return false; } + return true; } /** diff --git a/modules/rest/controllers/rest.php b/modules/rest/controllers/rest.php index ba996b84..64a548d0 100644 --- a/modules/rest/controllers/rest.php +++ b/modules/rest/controllers/rest.php @@ -22,11 +22,18 @@ class Rest_Controller extends Controller { $username = Input::instance()->post("user"); $password = Input::instance()->post("password"); + if (empty($username) || !auth::validate_too_many_failed_logins($username)) { + throw new Rest_Exception("Forbidden", 403); + } + $user = identity::lookup_user_by_name($username); if (empty($user) || !identity::is_correct_password($user, $password)) { + module::event("user_login_failed", $username); throw new Rest_Exception("Forbidden", 403); } + auth::login($user); + $key = rest::get_access_token($user->id); rest::reply($key->access_key); } diff --git a/modules/rest/helpers/rest.php b/modules/rest/helpers/rest.php index 3883794a..b3f80a55 100644 --- a/modules/rest/helpers/rest.php +++ b/modules/rest/helpers/rest.php @@ -64,7 +64,7 @@ class rest_Core { if (!$key->loaded()) { $key->user_id = $user_id; - $key->access_key = md5(rand()); + $key->access_key = md5(md5(uniqid(mt_rand(), true) . access::private_key())); $key->save(); } return $key; -- 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/rest/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