From fa0663d7df0cfcf0818e182f3d2d19fc6be2a5d1 Mon Sep 17 00:00:00 2001 From: Tim Almdal Date: Tue, 8 Dec 2009 09:19:48 -0800 Subject: Rename the backing table from rest_keys to user_access_tokens Implement an api to format the errors and success messages Removed the custom routing... urls are now /rest// --- modules/rest/helpers/rest.php | 69 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) create mode 100644 modules/rest/helpers/rest.php (limited to 'modules/rest/helpers/rest.php') diff --git a/modules/rest/helpers/rest.php b/modules/rest/helpers/rest.php new file mode 100644 index 00000000..34852a9e --- /dev/null +++ b/modules/rest/helpers/rest.php @@ -0,0 +1,69 @@ + "OK"); + if (!empty($message)) { + $response["message"] = (string)$message; + } + // We don't need to save the session for this request + Session::abort_save(); + return json_encode(array_merge($response, $response_data)); + } + + private static function _format_response($message, $log_message) { + if (!empty($log_message)) { + Kohana::log("info", $log_message); + } + // We don't need to save the session for this request + Session::abort_save(); + return json_encode(array("status" => "ERROR", "message" => (string)$message)); + } +} -- cgit v1.2.3 From 6fd04069aec67ff115cac4296c013cb5eea6782b Mon Sep 17 00:00:00 2001 From: Tim Almdal Date: Tue, 8 Dec 2009 12:50:13 -0800 Subject: Add another error handler "not found" to the rest API. Implement the get_album rest request handler. --- modules/gallery/helpers/gallery_rest.php | 64 ++++++++++++++++++ modules/gallery/tests/Gallery_Rest_Helper_Test.php | 75 ++++++++++++++++++++++ modules/rest/helpers/rest.php | 11 +++- 3 files changed, 148 insertions(+), 2 deletions(-) create mode 100644 modules/gallery/helpers/gallery_rest.php create mode 100644 modules/gallery/tests/Gallery_Rest_Helper_Test.php (limited to 'modules/rest/helpers/rest.php') diff --git a/modules/gallery/helpers/gallery_rest.php b/modules/gallery/helpers/gallery_rest.php new file mode 100644 index 00000000..367c2684 --- /dev/null +++ b/modules/gallery/helpers/gallery_rest.php @@ -0,0 +1,64 @@ +path)) { + return rest::invalid_request(); + } + + $album = ORM::factory("item") + ->where("relative_path_cache", $request->path) + ->where("type", "album") + ->viewable() + ->find(); + + if (!$album->loaded) { + return rest::not_found(); + } + + $response_data = array("path" => $album->relative_path(), + "title" => $album->title, + "thumb_url" => $album->thumb_url(), + "url" => $album->abs_url(), + "description" => $album->description, + "internet_address" => $album->slug); + + $children = self::_get_children($album, $request); + if (!empty($children)) { + $response_data["children"] = $children; + } + return rest::success(array("album" => $response_data)); + } + + private static function _get_children($album, $request) { + $children = array(); + $limit = empty($request->limit) ? null : $request->limit; + $offset = empty($request->offset) ? null : $request->offset; + $where = empty($request->filter) ? array() : array("type" => $request->filter); + foreach ($album->children($limit, $offset, $where) as $child) { + $children[] = array("type" => $child->type, + "has_children" => $child->children_count() > 0, + "path" => $child->relative_path(), + "title" => $child->title); + } + + return $children; + } +} diff --git a/modules/gallery/tests/Gallery_Rest_Helper_Test.php b/modules/gallery/tests/Gallery_Rest_Helper_Test.php new file mode 100644 index 00000000..ea1841d0 --- /dev/null +++ b/modules/gallery/tests/Gallery_Rest_Helper_Test.php @@ -0,0 +1,75 @@ +_save = array($_GET, $_POST, $_SERVER); + $this->_saved_active_user = identity::active_user(); + + $this->_user = identity::create_user("access_test", "Access Test", "password"); + $key = ORM::factory("user_access_token"); + $this->_access_key = $key->access_key = md5($this->_user->name . rand()); + $key->user_id = $this->_user->id; + $key->save(); + + $root = ORM::factory("item", 1); + $this->_album = album::create($root, "album", "Test Album", rand()); + $this->_child = album::create($this->_album, "child", "Test Child Album", rand()); + + $filename = MODPATH . "gallery/tests/test.jpg"; + $rand = rand(); + $this->_photo = photo::create($this->_child, $filename, "$rand.jpg", $rand); + + identity::set_active_user($this->_user); + } + + public function teardown() { + list($_GET, $_POST, $_SERVER) = $this->_save; + identity::set_active_user($this->_saved_active_user); + + try { + if (!empty($this->_user)) { + $this->_user->delete(); + } + if (!empty($this->_album)) { + $this->_album->delete(); + } + } catch (Exception $e) { } + } + + public function gallery_rest_get_album_test() { + $request = (object)array("path" => $this->_child->relative_path()); + print Kohana::debug($request) . "\n"; + + $this->assert_equal( + json_encode(array("status" => "OK", + "album" => array("path" => $this->_child->relative_path(), + "title" => $this->_child->title, + "thumb_url" => $this->_child->thumb_url(), + "url" => $this->_child->abs_url(), + "description" => $this->_child->description, + "internet_address" => $this->_child->slug, + "children" => array(array( + "type" => "photo", + "has_children" => false, + "path" => $this->_photo->relative_path(), + "title" => $this->_photo->title))))), + gallery_rest::get_album($request)); + } +} diff --git a/modules/rest/helpers/rest.php b/modules/rest/helpers/rest.php index 34852a9e..64a32d40 100644 --- a/modules/rest/helpers/rest.php +++ b/modules/rest/helpers/rest.php @@ -32,7 +32,7 @@ class rest_Core { } /** - * Not implemented + * Not Implemented */ static function not_implemented($log_message=null) { return self::_format_response(t("Service not implemented"), $log_message); @@ -46,7 +46,14 @@ class rest_Core { } /** - * Not implemented + * Resource Not Found + */ + static function not_found($log_message=null) { + return self::_format_response(t("Internal error"), $log_message); + } + + /** + * Success */ static function success($response_data, $message=null) { $response = array("status" => "OK"); -- cgit v1.2.3 From 837396ca2889b9e4e4a7b33a31409a2cd12a483c Mon Sep 17 00:00:00 2001 From: Tim Almdal Date: Tue, 8 Dec 2009 18:06:16 -0800 Subject: Change the url mapping so that path to the is part of the url The request key is put in the X-Gallery-Request-Key header The HTTP method can be override by using the X-Gallery-Request-Method header Normalize the request data so that it doesn't matter where it comes from (HTTP get or HTTP post request) --- modules/rest/controllers/rest.php | 87 ++++++++++++++++++----------- modules/rest/helpers/rest.php | 5 +- modules/rest/tests/Rest_Controller_Test.php | 70 ++++++----------------- 3 files changed, 73 insertions(+), 89 deletions(-) (limited to 'modules/rest/helpers/rest.php') diff --git a/modules/rest/controllers/rest.php b/modules/rest/controllers/rest.php index 0e5cbe96..0c88877a 100644 --- a/modules/rest/controllers/rest.php +++ b/modules/rest/controllers/rest.php @@ -47,58 +47,77 @@ class Rest_Controller extends Controller { } public function __call($function, $args) { - $access_token = $this->input->get("request_key"); - $request = $this->input->post("request", null); + $request = $this->_normalize_request($args); - if (empty($access_token)) { + if (empty($request->access_token)) { print rest::forbidden("No access token supplied."); return; } try { - $key = ORM::factory("user_access_token") - ->where("access_key", $access_token) - ->find(); + if ($this->_set_active_user($request->access_token)) { + $handler_class = "{$function}_rest"; + $handler_method = "{$request->method}"; - if (!$key->loaded) { - print rest::forbidden("Invalid key: $access_token"); - return; - } + if (!method_exists($handler_class, $handler_method)) { + print rest::not_implemented("$handler_class::$handler_method is not implemented"); + return; + } - $user = identity::lookup_user($key->user_id); - if (empty($user)) { - print rest::forbidden("User not found: {$key->user_id}"); - return; + print call_user_func(array($handler_class, $handler_method), $request); } + } catch (Exception $e) { + print rest::internal_error($e); + } + } - if (!empty($request)) { - $method = strtolower($this->input->server("HTTP_X_HTTP_METHOD_OVERRIDE", "POST")); + private function _normalize_request($args) { + $method = strtolower($this->input->server("REQUEST_METHOD")); + if ($method != "get") { + $request = $this->input->post("request", null); + if ($request) { $request = json_decode($request); } else { - print rest::invalid_request("Empty Request"); - return; + $request = new stdClass(); } - - - if (empty($args[0])) { - print rest::invalid_request("Resource not supplied"); - return; + } else { + $request = new stdClass(); + foreach (array_keys($_GET) as $key) { + if ($key == "request_key") { + continue; + } + $request->$key = $this->input->get($key); } + } - $handler_class = "{$function}_rest"; - $handler_method = "{$method}_{$args[0]}"; + $override_method = strtolower($this->input->server("HTTP_X_GALLERY_REQUEST_METHOD", null)); + $request->method = empty($override_method) ? $method : $override_method; + $request->access_token = $this->input->server("HTTP_X_GALLERY_REQUEST_KEY"); + $request->path = implode("/", $args); - if (!method_exists($handler_class, $handler_method)) { - print rest::not_implemented("$handler_class::$handler_method is not implemented"); - return; - } + return $request; + } - identity::set_active_user($user); + private 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(); - print call_user_func(array($handler_class, $handler_method), $request); - } catch (Exception $e) { - print rest::internal_error($e); + if ($key->loaded) { + $user = identity::lookup_user($key->user_id); + if (empty($user)) { + print rest::forbidden("User not found: {$key->user_id}"); + return false;; + } + } else { + print rest::forbidden("Invalid user access token supplied: {$key->user_id}"); + return false; + } } + identity::set_active_user($user); + return true; } - } \ No newline at end of file diff --git a/modules/rest/helpers/rest.php b/modules/rest/helpers/rest.php index 64a32d40..22c13be9 100644 --- a/modules/rest/helpers/rest.php +++ b/modules/rest/helpers/rest.php @@ -60,9 +60,12 @@ class rest_Core { if (!empty($message)) { $response["message"] = (string)$message; } + if ($response_data) { + $response = array_merge($response, $response_data); + } // We don't need to save the session for this request Session::abort_save(); - return json_encode(array_merge($response, $response_data)); + return json_encode($response); } private static function _format_response($message, $log_message) { diff --git a/modules/rest/tests/Rest_Controller_Test.php b/modules/rest/tests/Rest_Controller_Test.php index afac2d05..1417c315 100644 --- a/modules/rest/tests/Rest_Controller_Test.php +++ b/modules/rest/tests/Rest_Controller_Test.php @@ -72,7 +72,7 @@ class Rest_Controller_Test extends Unit_Test_Case { } public function rest_access_key_no_parameters_test() { - $_SERVER["REQUEST_METHOD"] = "POST"; + $_SERVER["REQUEST_METHOD"] = "GET"; $this->assert_equal( json_encode(array("status" => "ERROR", "message" => (string)t("Authorization failed"))), @@ -90,7 +90,6 @@ class Rest_Controller_Test extends Unit_Test_Case { public function rest_access_key_invalid_password_test() { $_SERVER["REQUEST_METHOD"] = "POST"; - $_POST["request"] = json_encode(array("user" => "access_test", "password" => "invalid")); $this->assert_equal( json_encode(array("status" => "ERROR", "message" => (string)t("Authorization failed"))), @@ -100,31 +99,14 @@ class Rest_Controller_Test extends Unit_Test_Case { public function rest_get_resource_no_request_key_test() { $_SERVER["HTTP_X_HTTP_METHOD_OVERRIDE"] = "GET"; - $_SERVER["REQUEST_METHOD"] = "POST"; - $_POST["request"] = json_encode(array("path" => $this->_path)); - $this->assert_equal( json_encode(array("status" => "ERROR", "message" => (string)t("Authorization failed"))), - $this->_call_controller("rest")); - } - - public function rest_get_resource_no_request_content_test() { - $_SERVER["HTTP_X_HTTP_METHOD_OVERRIDE"] = "GET"; - - $_SERVER["REQUEST_METHOD"] = "POST"; - $_GET["request_key"] = $this->_access_key; - - $this->assert_equal( - json_encode(array("status" => "ERROR", "message" => (string)t("Invalid request"))), - $this->_call_controller("rest")); + $this->_call_controller("rest", explode("/", $this->_photo->relative_path()))); } public function rest_get_resource_invalid_key_test() { - $_SERVER["HTTP_X_HTTP_METHOD_OVERRIDE"] = "GET"; - - $_SERVER["REQUEST_METHOD"] = "POST"; - $_GET["request_key"] = md5($this->_access_key); // screw up the access key - $_POST["request"] = json_encode(array("path" => $this->_path)); + $_SERVER["HTTP_X_GALLERY_REQUEST_KEY"] = md5($this->_access_key); // screw up the access key; + $_SERVER["REQUEST_METHOD"] = "GET"; $this->assert_equal( json_encode(array("status" => "ERROR", "message" => (string)t("Authorization failed"))), @@ -132,50 +114,30 @@ class Rest_Controller_Test extends Unit_Test_Case { } public function rest_get_resource_no_user_for_key_test() { - $_SERVER["HTTP_X_HTTP_METHOD_OVERRIDE"] = "GET"; - $_SERVER["REQUEST_METHOD"] = "POST"; - - $_GET["request_key"] = $this->_access_key; - $_POST["request"] = json_encode(array("path" => $this->_path)); + $_SERVER["REQUEST_METHOD"] = "GET"; + $_SERVER["HTTP_X_GALLERY_REQUEST_KEY"] = $this->_access_key; $this->_user->delete(); unset($this->_user); $this->assert_equal( json_encode(array("status" => "ERROR", "message" => (string)t("Authorization failed"))), - $this->_call_controller("rest")); - } - - public function rest_get_resource_no_resource_test() { - $_SERVER["HTTP_X_HTTP_METHOD_OVERRIDE"] = "GET"; - $_SERVER["REQUEST_METHOD"] = "POST"; - - $_GET["request_key"] = $this->_access_key; - $_POST["request"] = json_encode(array("path" => $this->_path)); - - $this->assert_equal( - json_encode(array("status" => "ERROR", "message" => (string)t("Invalid request"))), - $this->_call_controller("rest")); + $this->_call_controller("rest", explode("/", $this->_photo->relative_path()))); } public function rest_get_resource_no_handler_test() { - $_SERVER["HTTP_X_HTTP_METHOD_OVERRIDE"] = "GET"; - $_SERVER["REQUEST_METHOD"] = "POST"; - - $_GET["request_key"] = $this->_access_key; - $_POST["request"] = json_encode(array("path" => $this->_path)); + $_SERVER["REQUEST_METHOD"] = "GET"; + $_SERVER["HTTP_X_GALLERY_REQUEST_KEY"] = $this->_access_key; + $_SERVER["HTTP_X_GALLERY_REQUEST_METHOD"] = "PUT"; $this->assert_equal( json_encode(array("status" => "ERROR", "message" => (string)t("Service not implemented"))), - $this->_call_controller("rest", "album")); + $this->_call_controller("rest", explode("/", $this->_photo->relative_path()))); } public function rest_get_resource_test() { - $_SERVER["HTTP_X_HTTP_METHOD_OVERRIDE"] = "GET"; - $_SERVER["REQUEST_METHOD"] = "POST"; - - $_GET["request_key"] = $this->_access_key; - $_POST["request"] = json_encode(array("path" => $this->_path)); + $_SERVER["REQUEST_METHOD"] = "GET"; + $_SERVER["HTTP_X_GALLERY_REQUEST_KEY"] = $this->_access_key; $this->assert_equal( json_encode(array("status" => "OK", "message" => (string)t("Processed"), @@ -185,14 +147,14 @@ class Rest_Controller_Test extends Unit_Test_Case { "description" => $this->_photo->description, "internet_address" => $this->_photo->slug, "type" => $this->_photo->type))), - $this->_call_controller("rest", "photo")); + $this->_call_controller("rest", explode("/", $this->_photo->relative_path()))); } private function _call_controller($method="access_key", $arg=null) { $controller = new Rest_Controller(); ob_start(); - call_user_func(array($controller, $method), $arg); + call_user_func_array(array($controller, $method), $arg); $results = ob_get_contents(); ob_end_clean(); @@ -203,7 +165,7 @@ class Rest_Controller_Test extends Unit_Test_Case { class rest_rest { static $request = null; - static function get_photo($request) { + static function get($request) { self::$request = $request; $item = ORM::factory("item") ->where("relative_path_cache", $request->path) -- cgit v1.2.3 From 9319f37c4f157c5b0787df9116889e4e9ea5df78 Mon Sep 17 00:00:00 2001 From: Tim Almdal Date: Tue, 8 Dec 2009 23:27:43 -0800 Subject: Correct the error message when the item is not found; remove the check for no request_key (access_token) as athat is treated as public permissions --- modules/gallery/tests/Gallery_Rest_Helper_Test.php | 17 +++++++++++++++-- modules/rest/controllers/rest.php | 13 ++----------- modules/rest/helpers/rest.php | 2 +- 3 files changed, 18 insertions(+), 14 deletions(-) (limited to 'modules/rest/helpers/rest.php') diff --git a/modules/gallery/tests/Gallery_Rest_Helper_Test.php b/modules/gallery/tests/Gallery_Rest_Helper_Test.php index ea1841d0..1bf0b1ca 100644 --- a/modules/gallery/tests/Gallery_Rest_Helper_Test.php +++ b/modules/gallery/tests/Gallery_Rest_Helper_Test.php @@ -55,7 +55,6 @@ class Gallery_Rest_Helper_Test extends Unit_Test_Case { public function gallery_rest_get_album_test() { $request = (object)array("path" => $this->_child->relative_path()); - print Kohana::debug($request) . "\n"; $this->assert_equal( json_encode(array("status" => "OK", @@ -70,6 +69,20 @@ class Gallery_Rest_Helper_Test extends Unit_Test_Case { "has_children" => false, "path" => $this->_photo->relative_path(), "title" => $this->_photo->title))))), - gallery_rest::get_album($request)); + gallery_rest::get($request)); + } + + public function gallery_rest_get_photo_test() { + $request = (object)array("path" => $this->_photo->relative_path()); + + $this->assert_equal( + json_encode(array("status" => "OK", + "photo" => array("path" => $this->_photo->relative_path(), + "title" => $this->_photo->title, + "thumb_url" => $this->_photo->thumb_url(), + "url" => $this->_photo->abs_url(), + "description" => $this->_photo->description, + "internet_address" => $this->_photo->slug))), + gallery_rest::get($request)); } } diff --git a/modules/rest/controllers/rest.php b/modules/rest/controllers/rest.php index 0c88877a..1ec24493 100644 --- a/modules/rest/controllers/rest.php +++ b/modules/rest/controllers/rest.php @@ -48,16 +48,10 @@ class Rest_Controller extends Controller { public function __call($function, $args) { $request = $this->_normalize_request($args); - - if (empty($request->access_token)) { - print rest::forbidden("No access token supplied."); - return; - } - try { if ($this->_set_active_user($request->access_token)) { $handler_class = "{$function}_rest"; - $handler_method = "{$request->method}"; + $handler_method = $request->method; if (!method_exists($handler_class, $handler_method)) { print rest::not_implemented("$handler_class::$handler_method is not implemented"); @@ -67,7 +61,7 @@ class Rest_Controller extends Controller { print call_user_func(array($handler_class, $handler_method), $request); } } catch (Exception $e) { - print rest::internal_error($e); + print rest::internal_error($e->__toString()); } } @@ -83,9 +77,6 @@ class Rest_Controller extends Controller { } else { $request = new stdClass(); foreach (array_keys($_GET) as $key) { - if ($key == "request_key") { - continue; - } $request->$key = $this->input->get($key); } } diff --git a/modules/rest/helpers/rest.php b/modules/rest/helpers/rest.php index 22c13be9..fbbd6733 100644 --- a/modules/rest/helpers/rest.php +++ b/modules/rest/helpers/rest.php @@ -49,7 +49,7 @@ class rest_Core { * Resource Not Found */ static function not_found($log_message=null) { - return self::_format_response(t("Internal error"), $log_message); + return self::_format_response(t("Resource not found"), $log_message); } /** -- cgit v1.2.3 From dfc556e8a6e2c0636a93d87bc0cdb0f85f588fd4 Mon Sep 17 00:00:00 2001 From: Tim Almdal Date: Wed, 9 Dec 2009 12:06:45 -0800 Subject: Implement the RESTful interface for albums/photos/movies --- modules/gallery/controllers/photos.php | 3 +- modules/gallery/helpers/gallery_rest.php | 51 ++++++++++ modules/gallery/tests/Gallery_Rest_Helper_Test.php | 108 +++++++++++++++++++-- modules/rest/helpers/rest.php | 21 ++-- 4 files changed, 168 insertions(+), 15 deletions(-) (limited to 'modules/rest/helpers/rest.php') diff --git a/modules/gallery/controllers/photos.php b/modules/gallery/controllers/photos.php index f2c0f5dd..455ac25c 100644 --- a/modules/gallery/controllers/photos.php +++ b/modules/gallery/controllers/photos.php @@ -102,8 +102,7 @@ class Photos_Controller extends Items_Controller { log::success("content", "Updated photo", "url()}\">view"); message::success( - t("Saved photo %photo_title", - array("photo_title" => html::purify($photo->title)))); + t("Saved photo %photo_title", array("photo_title" => html::purify($photo->title)))); print json_encode( array("result" => "success", diff --git a/modules/gallery/helpers/gallery_rest.php b/modules/gallery/helpers/gallery_rest.php index 043e17b5..82d1bb5b 100644 --- a/modules/gallery/helpers/gallery_rest.php +++ b/modules/gallery/helpers/gallery_rest.php @@ -46,6 +46,57 @@ class gallery_rest_Core { return rest::success(array($item->type => $response_data)); } + static function put($request) { + if (empty($request->path)) { + return rest::invalid_request(); + } + + $item = ORM::factory("item") + ->where("relative_url_cache", $request->path) + ->viewable() + ->find(); + + if (!$item->loaded) { + return rest::not_found("Resource: {$request->path} missing."); + } + + if (!access::can("edit", $item)) { + return rest::not_found("Resource: {$request->path} permission denied."); + } + + // Normalize the request + $new_values = array(); + $fields = array("title", "description", "name", "slug"); + if ($item->is_album()) { + $fields = array_merge($fields, array("sort_column", "sort_order")); + } + foreach ($fields as $field) { + $new_values[$field] = !empty($request->$field) ? $request->$field : $item->$field; + } + if ($item->id == 1) { + unset($new_values["name"]); + } + if ($item->id != 1 && + ($new_values["name"] != $item->name || $new_values["slug"] != $item->slug)) { + // Make sure that there's not a conflict + $errors = item::check_for_conflicts($item, $new_values["name"], $new_values["slug"]); + if (!empty($errors["name_conflict"])) { + return rest::fail(t("Renaming %path failed: new name exists", + array("path" => $request->path))); + } + if (!empty($errors["slug_conflict"])) { + return rest::fail(t("Renaming %path failed: new internet address exists", + array("path" => $request->path))); + } + } + + item::update($item, $new_values); + + log::success("content", "Updated $item->type", "type}s/$item->id\">view"); + + return rest::success(); + } + private static function _get_children($item, $request) { $children = array(); $limit = empty($request->limit) ? null : $request->limit; diff --git a/modules/gallery/tests/Gallery_Rest_Helper_Test.php b/modules/gallery/tests/Gallery_Rest_Helper_Test.php index b874863f..9c960409 100644 --- a/modules/gallery/tests/Gallery_Rest_Helper_Test.php +++ b/modules/gallery/tests/Gallery_Rest_Helper_Test.php @@ -36,7 +36,9 @@ class Gallery_Rest_Helper_Test extends Unit_Test_Case { $rand = rand(); $this->_photo = photo::create($this->_child, $filename, "$rand.jpg", $rand); - identity::set_active_user($this->_user); + $filename = MODPATH . "gallery/tests/test.jpg"; + $rand = rand(); + $this->_sibling = photo::create($this->_album, $filename, "$rand.jpg", $rand); } public function teardown() { @@ -54,11 +56,11 @@ class Gallery_Rest_Helper_Test extends Unit_Test_Case { } public function gallery_rest_get_album_test() { - $request = (object)array("path" => $this->_child->relative_path()); + $request = (object)array("path" => $this->_child->relative_url()); $this->assert_equal( json_encode(array("status" => "OK", - "album" => array("path" => $this->_child->relative_url_path(), + "album" => array("path" => $this->_child->relative_url(), "title" => $this->_child->title, "thumb_url" => $this->_child->thumb_url(), "url" => $this->_child->abs_url(), @@ -67,17 +69,17 @@ class Gallery_Rest_Helper_Test extends Unit_Test_Case { "children" => array(array( "type" => "photo", "has_children" => false, - "path" => $this->_photo->relative_url_path(), + "path" => $this->_photo->relative_url(), "title" => $this->_photo->title))))), gallery_rest::get($request)); } public function gallery_rest_get_photo_test() { - $request = (object)array("path" => $this->_photo->relative_path()); + $request = (object)array("path" => $this->_photo->relative_url()); $this->assert_equal( json_encode(array("status" => "OK", - "photo" => array("path" => $this->_photo->relative_path(), + "photo" => array("path" => $this->_photo->relative_url(), "title" => $this->_photo->title, "thumb_url" => $this->_photo->thumb_url(), "url" => $this->_photo->abs_url(), @@ -85,4 +87,98 @@ class Gallery_Rest_Helper_Test extends Unit_Test_Case { "internet_address" => $this->_photo->slug))), gallery_rest::get($request)); } + + public function gallery_rest_put_album_no_path_test() { + access::allow(identity::registered_users(), "edit", $this->_child); + + identity::set_active_user($this->_user); + $request = (object)array("description" => "Updated description", + "title" => "Updated Title", + "sort_order" => "DESC", + "sort_column" => "title", + "name" => "new name"); + + $this->assert_equal(json_encode(array("status" => "ERROR", "message" => "Invalid request")), + gallery_rest::put($request)); + } + + public function gallery_rest_put_album_not_found_test() { + access::allow(identity::registered_users(), "edit", $this->_child); + + identity::set_active_user($this->_user); + $request = (object)array("path" => $this->_child->relative_url() . rand(), + "description" => "Updated description", + "title" => "Updated Title", + "sort_order" => "DESC", + "sort_column" => "title", + "name" => "new name"); + + $this->assert_equal(json_encode(array("status" => "ERROR", "message" => "Resource not found")), + gallery_rest::put($request)); + } + + public function gallery_rest_put_album_no_edit_permission_test() { + identity::set_active_user($this->_user); + $request = (object)array("path" => $this->_child->relative_url(), + "description" => "Updated description", + "title" => "Updated Title", + "sort_order" => "DESC", + "sort_column" => "title", + "name" => "new name"); + + $this->assert_equal(json_encode(array("status" => "ERROR", "message" => "Resource not found")), + gallery_rest::put($request)); + } + + public function gallery_rest_put_album_rename_conflict_test() { + access::allow(identity::registered_users(), "edit", $this->_child); + identity::set_active_user($this->_user); + $request = (object)array("path" => $this->_child->relative_url(), + "description" => "Updated description", + "title" => "Updated Title", + "sort_order" => "DESC", + "sort_column" => "title", + "name" => $this->_sibling->name); + + $this->assert_equal( + json_encode(array("status" => "ERROR", + "message" => "Renaming album/child failed: new name exists")), + gallery_rest::put($request)); + } + + public function gallery_rest_put_album_test() { + access::allow(identity::registered_users(), "edit", $this->_child); + + identity::set_active_user($this->_user); + $request = (object)array("path" => $this->_child->relative_url(), + "description" => "Updated description", + "title" => "Updated Title", + "sort_order" => "DESC", + "sort_column" => "title", + "name" => "new name"); + + $this->assert_equal(json_encode(array("status" => "OK")), gallery_rest::put($request)); + $this->_child->reload(); + $this->assert_equal("Updated description", $this->_child->description); + $this->assert_equal("Updated Title", $this->_child->title); + $this->assert_equal("DESC", $this->_child->sort_order); + $this->assert_equal("title", $this->_child->sort_column); + $this->assert_equal("new name", $this->_child->name); + } + + public function gallery_rest_put_photo_test() { + access::allow(identity::registered_users(), "edit", $this->_child); + + identity::set_active_user($this->_user); + $request = (object)array("path" => $this->_photo->relative_url(), + "description" => "Updated description", + "title" => "Updated Title", + "name" => "new name"); + + $this->assert_equal(json_encode(array("status" => "OK")), gallery_rest::put($request)); + $this->_photo->reload(); + $this->assert_equal("Updated description", $this->_photo->description); + $this->assert_equal("Updated Title", $this->_photo->title); + $this->assert_equal("new name", $this->_photo->name); + } } diff --git a/modules/rest/helpers/rest.php b/modules/rest/helpers/rest.php index fbbd6733..2c653f21 100644 --- a/modules/rest/helpers/rest.php +++ b/modules/rest/helpers/rest.php @@ -21,41 +21,48 @@ class rest_Core { * Authorization Failure */ static function forbidden($log_message=null) { - return self::_format_response(t("Authorization failed"), $log_message); + return self::_format_failure_response(t("Authorization failed"), $log_message); } /** * Invalid Failure */ static function invalid_request($log_message=null) { - return self::_format_response(t("Invalid request"), $log_message); + return self::_format_failure_response(t("Invalid request"), $log_message); } /** * Not Implemented */ static function not_implemented($log_message=null) { - return self::_format_response(t("Service not implemented"), $log_message); + return self::_format_failure_response(t("Service not implemented"), $log_message); } /** * Internal Error */ static function internal_error($log_message=null) { - return self::_format_response(t("Internal error"), $log_message); + return self::_format_failure_response(t("Internal error"), $log_message); } /** * Resource Not Found */ static function not_found($log_message=null) { - return self::_format_response(t("Resource not found"), $log_message); + return self::_format_failure_response(t("Resource not found"), $log_message); + } + + /** + * Resource Not Found + */ + static function fail($log_message=null) { + return self::_format_failure_response($log_message, $log_message); } /** * Success */ - static function success($response_data, $message=null) { + static function success($response_data=null, $message=null) { $response = array("status" => "OK"); if (!empty($message)) { $response["message"] = (string)$message; @@ -68,7 +75,7 @@ class rest_Core { return json_encode($response); } - private static function _format_response($message, $log_message) { + private static function _format_failure_response($message, $log_message) { if (!empty($log_message)) { Kohana::log("info", $log_message); } -- cgit v1.2.3 From fd7990735cc73b5b1494190b9c187297e588a9f6 Mon Sep 17 00:00:00 2001 From: Tim Almdal Date: Mon, 21 Dec 2009 11:25:11 -0800 Subject: Added validation to the edit functionality, since we can't trust any input --- modules/gallery/helpers/gallery_rest.php | 80 ++++++++++++++++++-------------- modules/rest/controllers/rest.php | 5 +- modules/rest/helpers/rest.php | 19 ++++++-- 3 files changed, 62 insertions(+), 42 deletions(-) (limited to 'modules/rest/helpers/rest.php') diff --git a/modules/gallery/helpers/gallery_rest.php b/modules/gallery/helpers/gallery_rest.php index 30a37ad1..e31c4252 100644 --- a/modules/gallery/helpers/gallery_rest.php +++ b/modules/gallery/helpers/gallery_rest.php @@ -48,7 +48,7 @@ class gallery_rest_Core { "size" => array("height" => $item->height, "width" => $item->width), "description" => $item->description, - "internet_address" => $item->slug); + "slug" => $item->slug); $children = self::_get_children($item, $request); if (!empty($children) || $item->is_album()) { @@ -58,10 +58,6 @@ class gallery_rest_Core { } static function put($request) { - if (empty($request->path)) { - return rest::invalid_request(); - } - $item = ORM::factory("item") ->where("relative_url_cache", $request->path) ->viewable() @@ -75,37 +71,18 @@ class gallery_rest_Core { return rest::not_found("Resource: {$request->path} permission denied."); } - // Normalize the request - $new_values = array(); - $fields = array("title", "description", "name", "slug"); - if ($item->is_album()) { - $fields = array_merge($fields, array("sort_column", "sort_order")); - } - foreach ($fields as $field) { - $new_values[$field] = !empty($request->$field) ? $request->$field : $item->$field; - } - if ($item->id == 1) { - unset($new_values["name"]); - } - if ($item->id != 1 && - ($new_values["name"] != $item->name || $new_values["slug"] != $item->slug)) { - // Make sure that there's not a conflict - $errors = item::check_for_conflicts($item, $new_values["name"], $new_values["slug"]); - if (!empty($errors["name_conflict"])) { - return rest::fail(t("Renaming %path failed: new name exists", - array("path" => $request->path))); - } - if (!empty($errors["slug_conflict"])) { - return rest::fail(t("Renaming %path failed: new internet address exists", - array("path" => $request->path))); - } - } - - item::update($item, $new_values); + // Validate the request data + $new_values = gallery_rest::_validate($item, $request); + $errors = $new_values->errors(); + if (empty($errors)) { + item::update($item, $new_values->as_array()); - log::success("content", "Updated $item->type", "type}s/$item->id\">view"); + log::success("content", "Updated $item->type", "type}s/$item->id\">view"); - return rest::success(); + return rest::success(); + } else { + return rest::validation_error($errors); + } } static function post($request) { @@ -129,6 +106,8 @@ class gallery_rest_Core { return rest::not_found("Resource: {$request->path} permission denied."); } + // @TODO validate input values (assume nothing about the quality of input) + if (empty($_FILES["image"])) { $new_item = album::create( $parent, @@ -189,6 +168,7 @@ class gallery_rest_Core { return rest::invalid_request("Attempt to delete the root album"); } + $parent = $item->parent(); $item->delete(); if ($item->is_album()) { @@ -198,7 +178,7 @@ class gallery_rest_Core { } log::success("content", $msg); - return rest::success(); + return rest::success(array("resource" => array("parent_path" => $parent->relative_url()))); } private static function _get_children($item, $request) { @@ -219,4 +199,34 @@ class gallery_rest_Core { return $children; } + + private static function _validate($item, $request) { + $new_values = array(); + $fields = array("title", "description", "name", "slug"); + if ($item->id == 1) { + unset($request["name"]); + unset($request["slug"]); + } + foreach ($fields as $field) { + $new_values[$field] = isset($request->$field) ? $request->$field : $item->$field; + } + + $new_values = new Validation($new_values); + foreach ($item->rules as $field => $rules) { + foreach (explode("|", $rules) as $rule) { + $new_values->add_rules($field, $rule); + } + } + + if (($valid = $new_values->validate()) && $item->id != 1) { + $errors = item::check_for_conflicts($item, $new_values["name"], $new_values["slug"]); + if ($valid = empty($errors)) { + !empty($errors["name_conflict"]) OR $new_values->add_error("name", "Duplicate Name"); + !empty($errors["slug_conflict"]) OR + $new_values->add_error("name", "Duplicate Internet Address"); + } + } + + return $new_values; + } } diff --git a/modules/rest/controllers/rest.php b/modules/rest/controllers/rest.php index d1404b29..7a5ab46a 100644 --- a/modules/rest/controllers/rest.php +++ b/modules/rest/controllers/rest.php @@ -67,7 +67,7 @@ class Rest_Controller extends Controller { } private function _normalize_request($args=array()) { - $method = strtolower($this->input->server("REQUEST_METHOD")); + $method = strtolower($this->input->server("REQUEST_METHOD")); $request = new stdClass(); foreach (array_keys($this->input->get()) as $key) { $request->$key = $this->input->get($key); @@ -78,8 +78,7 @@ class Rest_Controller extends Controller { } } - $override_method = strtolower($this->input->server("HTTP_X_GALLERY_REQUEST_METHOD", null)); - $request->method = empty($override_method) ? $method : $override_method; + $request->method = strtolower($this->input->server("HTTP_X_GALLERY_REQUEST_METHOD", $method)); $request->access_token = $this->input->server("HTTP_X_GALLERY_REQUEST_KEY"); $request->path = implode("/", $args); diff --git a/modules/rest/helpers/rest.php b/modules/rest/helpers/rest.php index 2c653f21..ad6ca7c7 100644 --- a/modules/rest/helpers/rest.php +++ b/modules/rest/helpers/rest.php @@ -62,14 +62,25 @@ class rest_Core { /** * Success */ - static function success($response_data=null, $message=null) { + static function success($response_data=array(), $message=null) { $response = array("status" => "OK"); if (!empty($message)) { $response["message"] = (string)$message; } - if ($response_data) { - $response = array_merge($response, $response_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); -- cgit v1.2.3 From bccb6fc02146fb07cd1b472a90092e78e2259e91 Mon Sep 17 00:00:00 2001 From: Tim Almdal Date: Sun, 27 Dec 2009 08:32:12 -0800 Subject: Clean up validation the check for duplicate names or slugs, finish converting the rest API to Kohana 2.4 --- modules/gallery/helpers/gallery_rest.php | 93 ++++++++++++++-------- modules/gallery/helpers/item.php | 8 +- modules/gallery/tests/Gallery_Rest_Helper_Test.php | 8 +- modules/image_block/helpers/image_block_rest.php | 12 +-- modules/rest/controllers/rest.php | 27 ++++--- modules/rest/helpers/rest.php | 2 +- modules/rest/tests/Rest_Controller_Test.php | 2 +- modules/tag/helpers/tag_rest.php | 62 ++++++++------- modules/tag/models/tag.php | 4 +- modules/tag/tests/Tag_Rest_Helper_Test.php | 4 +- 10 files changed, 129 insertions(+), 93 deletions(-) (limited to 'modules/rest/helpers/rest.php') diff --git a/modules/gallery/helpers/gallery_rest.php b/modules/gallery/helpers/gallery_rest.php index 227a6f02..1d790d8c 100644 --- a/modules/gallery/helpers/gallery_rest.php +++ b/modules/gallery/helpers/gallery_rest.php @@ -22,11 +22,11 @@ class gallery_rest_Core { $path = implode("/", $request->arguments); $item = ORM::factory("item") - ->where("relative_url_cache", $path) + ->where("relative_url_cache", "=", $path) ->viewable() ->find(); - if (!$item->loaded) { + if (!$item->loaded()) { return rest::not_found("Resource: {$path} missing."); } @@ -62,11 +62,11 @@ class gallery_rest_Core { $path = implode("/", $request->arguments); $item = ORM::factory("item") - ->where("relative_url_cache", $path) + ->where("relative_url_cache", "=", $path) ->viewable() ->find(); - if (!$item->loaded) { + if (!$item->loaded()) { return rest::not_found("Resource: {$path} missing."); } @@ -75,12 +75,13 @@ class gallery_rest_Core { } // Validate the request data - $new_values = gallery_rest::_validate($request, $item); + $new_values = gallery_rest::_validate($request, $item->parent_id, $item->id); $errors = $new_values->errors(); if (empty($errors)) { item::update($item, $new_values->as_array()); - log::success("content", "Updated $item->type", "type}s/$item->id\">view"); + log::success("content", "Updated $item->type", + "type}s/$item->id\">view"); return rest::success(); } else { @@ -94,15 +95,15 @@ class gallery_rest_Core { } $path = implode("/", $request->arguments); - $components = explode("/", $path); + $components = $request->arguments; $name = urldecode(array_pop($components)); $parent = ORM::factory("item") - ->where("relative_url_cache", implode("/", $components)) + ->where("relative_url_cache", "=", implode("/", $components)) ->viewable() ->find(); - if (!$parent->loaded) { + if (!$parent->loaded()) { return rest::not_found("Resource: {$path} missing."); } @@ -111,7 +112,7 @@ class gallery_rest_Core { } // Validate the request data - $new_values = gallery_rest::_validate($request); + $new_values = gallery_rest::_validate($request, $parent->id); $errors = $new_values->errors(); if (!empty($errors)) { return rest::validation_error($errors); @@ -121,10 +122,10 @@ class gallery_rest_Core { $new_item = album::create( $parent, $name, - empty($request->title) ? $name : $request->title, - empty($request->description) ? null : $request->description, + empty($new_values["title"]) ? $name : $new_values["title"], + empty($new_values["description"]) ? null : $new_values["description"], identity::active_user()->id, - empty($request->slug) ? $name : $request->slug); + empty($new_values["slug"]) ? $name : $new_values["slug"]); $log_message = t("Added an album"); } else { $temp_filename = upload::save("image"); @@ -153,11 +154,11 @@ class gallery_rest_Core { $path = implode("/", $request->arguments); $item = ORM::factory("item") - ->where("relative_url_cache", $path) + ->where("relative_url_cache", "=", $path) ->viewable() ->find(); - if (!$item->loaded) { + if (!$item->loaded()) { return rest::success(); } @@ -193,7 +194,7 @@ class gallery_rest_Core { "path" => $child->relative_url(), "thumb_url" => $child->thumb_url(true), "thumb_dimensions" => array("width" => $child->thumb_width, - "height" => $child->thumb_height), + "height" => $child->thumb_height), "has_thumb" => $child->has_thumb(), "title" => $child->title); } @@ -201,38 +202,40 @@ class gallery_rest_Core { return $children; } - private static function _validate($request, $item=null) { + private static function _validate($request, $parent_id, $item_id=0) { $new_values = array(); - $fields = array("title", "description", "name", "slug", "image"); - if (empty($item)) { - $item = ORM::factory("item"); - $item->id = 0; - } - if ($item->id == 1) { + $fields = array("name" => "length[0,255]", + "title" => "required|length[0,255]", + "description" => "length[0,65535]", + "slug" => "required|length[0,255]"); + if ($item_id == 1) { unset($request["name"]); unset($request["slug"]); } - foreach ($fields as $field) { + foreach (array_keys($fields) as $field) { if (isset($request->$field)) { $new_values[$field] = $request->$field; } else if (isset($item->$field)) { $new_values[$field] = $item->$field; } } - - $new_values = new Validation($new_values); - foreach ($item->rules as $field => $rules) { - foreach (explode("|", $rules) as $rule) { - $new_values->add_rules($field, $rule); - } + if (!empty($request->image)) { + $new_values["image"] = $request->image; } + + $new_values = Validation::factory($new_values) + ->add_rules("name", "length[0,255]") + ->add_rules("title", "length[0,255]") + ->add_rules("description", "length[0,65535]") + ->add_rules("slug", "length[0,255]"); if (isset($new_values["image"])) { $new_values->add_rules( "image", "upload::valid", "upload::required", "upload::type[gif,jpg,jpeg,png,flv,mp4]"); } - if ($new_values->validate() && $item->id != 1) { - $errors = item::check_for_conflicts($item, $new_values["name"], $new_values["slug"]); + if ($new_values->validate() && $item_id != 1) { + $errors = gallery_rest::_check_for_conflicts($parent_id, $item_id, + $new_values["name"], $new_values["slug"]); if (!empty($errors)) { !empty($errors["name_conflict"]) OR $new_values->add_error("name", "Duplicate Name"); !empty($errors["slug_conflict"]) OR @@ -242,4 +245,30 @@ class gallery_rest_Core { return $new_values; } + + private static function _check_for_conflicts($parent_id, $item_id, $new_name, $new_slug) { + $errors = array(); + + if ($row = db::build() + ->select(array("name", "slug")) + ->from("items") + ->where("parent_id", "=", $parent_id) + ->where("id", "<>", $item_id) + ->and_open() + ->where("name", "=", $new_name) + ->or_where("slug", "=", $new_slug) + ->close() + ->execute() + ->current()) { + if ($row->name == $new_name) { + $errors["name_conflict"] = 1; + } + if ($row->slug == $new_slug) { + $errors["slug_conflict"] = 1; + } + } + + return $errors; + } + } diff --git a/modules/gallery/helpers/item.php b/modules/gallery/helpers/item.php index c620ba95..fc390e70 100644 --- a/modules/gallery/helpers/item.php +++ b/modules/gallery/helpers/item.php @@ -122,11 +122,11 @@ class item_Core { if ($row = db::build() ->select(array("name", "slug")) ->from("items") - ->where("parent_id", $item->parent_id) - ->where("id <>", $item->id) + ->where("parent_id", "=", $item->parent_id) + ->where("id", "<>", $item->id) ->and_open() - ->where("name", $new_name) - ->orwhere("slug", $new_slug) + ->where("name", "=", $new_name) + ->or_where("slug", "=", $new_slug) ->close() ->execute() ->current()) { diff --git a/modules/gallery/tests/Gallery_Rest_Helper_Test.php b/modules/gallery/tests/Gallery_Rest_Helper_Test.php index 14c73248..f36f6aaf 100644 --- a/modules/gallery/tests/Gallery_Rest_Helper_Test.php +++ b/modules/gallery/tests/Gallery_Rest_Helper_Test.php @@ -208,7 +208,7 @@ class Gallery_Rest_Helper_Test extends Unit_Test_Case { "parent_path" => $this->_album->relative_url()))), gallery_rest::delete($request)); $this->_child->reload(); - $this->assert_false($this->_child->loaded); + $this->assert_false($this->_child->loaded()); } public function gallery_rest_delete_photo_test() { @@ -222,7 +222,7 @@ class Gallery_Rest_Helper_Test extends Unit_Test_Case { "parent_path" => $this->_album->relative_url()))), gallery_rest::delete($request)); $this->_sibling->reload(); - $this->assert_false($this->_sibling->loaded); + $this->assert_false($this->_sibling->loaded()); } public function gallery_rest_post_album_test() { @@ -235,9 +235,9 @@ class Gallery_Rest_Helper_Test extends Unit_Test_Case { $this->assert_equal(json_encode(array("status" => "OK", "path" => $new_path)), gallery_rest::post($request)); $album = ORM::factory("item") - ->where("relative_url_cache", $new_path) + ->where("relative_url_cache", "=", $new_path) ->find(); - $this->assert_true($album->loaded); + $this->assert_true($album->loaded()); $this->assert_equal("new child", $album->slug); } } diff --git a/modules/image_block/helpers/image_block_rest.php b/modules/image_block/helpers/image_block_rest.php index 45f849b1..7afd974c 100644 --- a/modules/image_block/helpers/image_block_rest.php +++ b/modules/image_block/helpers/image_block_rest.php @@ -26,18 +26,18 @@ class image_block_rest_Core { $items = ORM::factory("item") ->viewable() - ->where("type !=", "album") - ->where("rand_key < ", $random) - ->orderby(array("rand_key" => "DESC")) + ->where("type", "!=", "album") + ->where("rand_key", "<", $random) + ->order_by(array("rand_key" => "DESC")) ->find_all(1); if ($items->count() == 0) { // Try once more. If this fails, just ditch the block altogether $items = ORM::factory("item") ->viewable() - ->where("type !=", "album") - ->where("rand_key >= ", $random) - ->orderby(array("rand_key" => "DESC")) + ->where("type", "!=", "album") + ->where("rand_key", ">= ", $random) + ->order_by(array("rand_key" => "DESC")) ->find_all(1); } break; diff --git a/modules/rest/controllers/rest.php b/modules/rest/controllers/rest.php index 1289d62b..6715bc15 100644 --- a/modules/rest/controllers/rest.php +++ b/modules/rest/controllers/rest.php @@ -18,7 +18,7 @@ */ class Rest_Controller extends Controller { public function access_key() { - $request = (object)$this->input->get(); + $request = (object)Input::instance()->get(); if (empty($request->user) || empty($request->password)) { print rest::forbidden("No user or password supplied"); return; @@ -36,13 +36,13 @@ class Rest_Controller extends Controller { } $key = ORM::factory("user_access_token") - ->where("user_id", $user->id) + ->where("user_id", "=", $user->id) ->find(); - if (!$key->loaded) { + if (!$key->loaded()) { $key->user_id = $user->id; $key->access_key = md5($user->name . rand()); $key->save(); - Kohana::log("alert", Kohana::debug($key->as_array())); + Kohana_Log::add("alert", Kohana::debug($key->as_array())); } print rest::success(array("token" => $key->access_key)); } @@ -67,22 +67,23 @@ class Rest_Controller extends Controller { } private function _normalize_request($args=array()) { - $method = strtolower($this->input->server("REQUEST_METHOD")); + $input = Input::instance(); + $method = strtolower($input->server("REQUEST_METHOD")); $request = new stdClass(); - foreach (array_keys($this->input->get()) as $key) { - $request->$key = $this->input->get($key); + foreach (array_keys($input->get()) as $key) { + $request->$key = $input->get($key); } if ($method != "get") { - foreach (array_keys($this->input->post()) as $key) { - $request->$key = $this->input->post($key); + foreach (array_keys($input->post()) as $key) { + $request->$key = $input->post($key); } foreach (array_keys($_FILES) as $key) { $request->$key = $_FILES[$key]; } } - $request->method = strtolower($this->input->server("HTTP_X_GALLERY_REQUEST_METHOD", $method)); - $request->access_token = $this->input->server("HTTP_X_GALLERY_REQUEST_KEY"); + $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 return $request; @@ -93,10 +94,10 @@ class Rest_Controller extends Controller { $user = identity::guest(); } else { $key = ORM::factory("user_access_token") - ->where("access_key", $access_token) + ->where("access_key", "=", $access_token) ->find(); - if ($key->loaded) { + if ($key->loaded()) { $user = identity::lookup_user($key->user_id); if (empty($user)) { print rest::forbidden("User not found: {$key->user_id}"); diff --git a/modules/rest/helpers/rest.php b/modules/rest/helpers/rest.php index ad6ca7c7..276ff0c2 100644 --- a/modules/rest/helpers/rest.php +++ b/modules/rest/helpers/rest.php @@ -88,7 +88,7 @@ class rest_Core { private static function _format_failure_response($message, $log_message) { if (!empty($log_message)) { - Kohana::log("info", $log_message); + Kohana_Log::add("info", $log_message); } // We don't need to save the session for this request Session::abort_save(); diff --git a/modules/rest/tests/Rest_Controller_Test.php b/modules/rest/tests/Rest_Controller_Test.php index b7fbd5a3..6bebc47d 100644 --- a/modules/rest/tests/Rest_Controller_Test.php +++ b/modules/rest/tests/Rest_Controller_Test.php @@ -173,7 +173,7 @@ class rest_rest { static function get($request) { self::$request = $request; $item = ORM::factory("item") - ->where("relative_url_cache", implode("/", $request->arguments)) + ->where("relative_url_cache", "=", implode("/", $request->arguments)) ->find(); $response["path"] = $item->relative_url(); $response["title"] = $item->title; diff --git a/modules/tag/helpers/tag_rest.php b/modules/tag/helpers/tag_rest.php index d62c0231..cca9a88b 100644 --- a/modules/tag/helpers/tag_rest.php +++ b/modules/tag/helpers/tag_rest.php @@ -18,40 +18,44 @@ * 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) { - if (empty($request->arguments)) { + $resources = array(); + switch (count($request->arguments)) { + case 0: $tags = ORM::factory("tag") ->select("name", "count") - ->orderby("count", "DESC"); + ->order_by("count", "DESC"); if (!empty($request->limit)) { $tags->limit($request->limit); } if (!empty($request->offset)) { $tags->offset($request->offset); } - $response = array("tags" => array()); + $resources = array("tags" => array()); foreach ($tags->find_all() as $row) { - $response["tags"][] = array("name" => $row->name, "count" => $row->count); + $resources["tags"][] = array("name" => $row->name, "count" => $row->count); } - } else { - $path = implode("/", $request->arguments); - if (strpos($path, ",") === false) { - $item = ORM::factory("item") - ->where("relative_url_cache", $path) - ->viewable() - ->find(); - // If we didn't find it and there was only one argument, retry as a tag not a path - if ($item->loaded || count($request->arguments) != 1) { - $response = array("tags" => $item->loaded ? tag::item_tags($item) : array()); - } else { - $response = array("resources" => tag_rest::_get_items($request)); - } - } else { - $response = array("resources" => tag_rest::_get_items($request)); + 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::success($response); + return rest::success($resources); } static function post($request) { @@ -62,10 +66,10 @@ class tag_rest_Core { $tags = explode(",", $request->arguments[0]); $item = ORM::factory("item") - ->where("relative_url_cache", $path) + ->where("relative_url_cache", "=", $path) ->viewable() ->find(); - if (!$item->loaded) { + if (!$item->loaded()) { return rest::not_found("Resource: {$path} missing."); } @@ -87,9 +91,9 @@ class tag_rest_Core { $name = $request->arguments[0]; $tag = ORM::factory("tag") - ->where("name", $name) + ->where("name", "=", $name) ->find(); - if (!$tag->loaded) { + if (!$tag->loaded()) { return rest::not_found("Tag: {$name} not found."); } @@ -108,13 +112,13 @@ class tag_rest_Core { $tag_list = ORM::factory("tag") ->join("items_tags", "tags.id", "items_tags.tag_id") ->join("items", "items.id", "items_tags.item_id") - ->in("tags.name", $tags) - ->where("relative_url_cache", $request->path) + ->where("tags.name", "IN", $tags) + ->where("relative_url_cache", "=", $request->path) ->viewable() ->find_all(); } else { $tag_list = ORM::factory("tag") - ->in("name", $tags) + ->where("name", "IN", $tags) ->find_all(); } @@ -129,10 +133,10 @@ class tag_rest_Core { private static function _get_items($request) { $tags = explode(",", $request->arguments[0]); $items = ORM::factory("item") - ->select("distinct *") + ->select_distinct("*") ->join("items_tags", "items.id", "items_tags.item_id") ->join("tags", "tags.id", "items_tags.tag_id") - ->in("tags.name", $tags); + ->where("tags.name", "IN", $tags); if (!empty($request->limit)) { $items->limit($request->limit); } diff --git a/modules/tag/models/tag.php b/modules/tag/models/tag.php index b2ce9eda..d0d2117c 100644 --- a/modules/tag/models/tag.php +++ b/modules/tag/models/tag.php @@ -108,7 +108,9 @@ class Tag_Model extends ORM { $result = parent::delete(); if ($related_item_ids) { - foreach (ORM::factory("item")->in("id", array_keys($related_item_ids))->find_all() as $item) { + foreach (ORM::factory("item") + ->where("id", "IN", array_keys($related_item_ids)) + ->find_all() as $item) { module::event("item_related_update", $item); } } diff --git a/modules/tag/tests/Tag_Rest_Helper_Test.php b/modules/tag/tests/Tag_Rest_Helper_Test.php index 1c550366..6b1c9a33 100644 --- a/modules/tag/tests/Tag_Rest_Helper_Test.php +++ b/modules/tag/tests/Tag_Rest_Helper_Test.php @@ -210,11 +210,11 @@ class Tag_Rest_Helper_Test extends Unit_Test_Case { $this->assert_equal(json_encode(array("status" => "OK")), tag_rest::delete($request)); $request = (object)array("arguments" => array("T1,P1")); - $this->assert_equal(json_encode(array("status" => "OK", "resources" => array())), + $this->assert_equal(json_encode(array("status" => "OK")), tag_rest::get($request)); } - public function tag_rest_delete_tag_from_item_test() { + public function tag_rest_delete_tagc_from_item_test() { $request = (object)array("arguments" => array("T1,P1"), $this->_photo->relative_url()); -- cgit v1.2.3 From 11792a12bb2002a434217efabe232022dd253b67 Mon Sep 17 00:00:00 2001 From: Tim Almdal Date: Wed, 30 Dec 2009 17:08:01 -0800 Subject: 1) Remove the rest::not_found method and replace it with "throw new Kohana_404_Exception 2) Don't use the input path to lookup the item via relative_path_cache. Instead use url::get_item_from_uri method. --- modules/gallery/helpers/gallery_rest.php | 72 ++++++++-------------- modules/gallery/tests/Gallery_Rest_Helper_Test.php | 16 +++-- modules/rest/helpers/rest.php | 9 +-- modules/tag/helpers/tag_rest.php | 6 +- modules/tag/tests/Tag_Rest_Helper_Test.php | 27 +++++--- 5 files changed, 59 insertions(+), 71 deletions(-) (limited to 'modules/rest/helpers/rest.php') diff --git a/modules/gallery/helpers/gallery_rest.php b/modules/gallery/helpers/gallery_rest.php index 1d790d8c..94c7dc6f 100644 --- a/modules/gallery/helpers/gallery_rest.php +++ b/modules/gallery/helpers/gallery_rest.php @@ -21,14 +21,7 @@ class gallery_rest_Core { static function get($request) { $path = implode("/", $request->arguments); - $item = ORM::factory("item") - ->where("relative_url_cache", "=", $path) - ->viewable() - ->find(); - - if (!$item->loaded()) { - return rest::not_found("Resource: {$path} missing."); - } + $item = gallery_rest::_get_item($path); $parent = $item->parent(); $response_data = array("type" => $item->type, @@ -60,25 +53,19 @@ class gallery_rest_Core { return rest::invalid_request(); } $path = implode("/", $request->arguments); - - $item = ORM::factory("item") - ->where("relative_url_cache", "=", $path) - ->viewable() - ->find(); - - if (!$item->loaded()) { - return rest::not_found("Resource: {$path} missing."); - } - - if (!access::can("edit", $item)) { - return rest::not_found("Resource: {$path} permission denied."); - } + $item = gallery_rest::_get_item($path, "edit"); // Validate the request data $new_values = gallery_rest::_validate($request, $item->parent_id, $item->id); $errors = $new_values->errors(); if (empty($errors)) { - item::update($item, $new_values->as_array()); + $item->title = $new_values->title; + $item->description = $new_values->description; + if ($item->id != 1) { + $item->rename($new_values->name); + } + $item->slug = $new_values->slug; + $item->save(); log::success("content", "Updated $item->type", "type}s/$item->id\">view"); @@ -93,23 +80,11 @@ class gallery_rest_Core { if (empty($request->arguments)) { return rest::invalid_request(); } - $path = implode("/", $request->arguments); $components = $request->arguments; $name = urldecode(array_pop($components)); - $parent = ORM::factory("item") - ->where("relative_url_cache", "=", implode("/", $components)) - ->viewable() - ->find(); - - if (!$parent->loaded()) { - return rest::not_found("Resource: {$path} missing."); - } - - if (!access::can("edit", $parent)) { - return rest::not_found("Resource: {$path} permission denied."); - } + $parent = gallery_rest::_get_item(implode("/", $components), "edit"); // Validate the request data $new_values = gallery_rest::_validate($request, $parent->id); @@ -153,18 +128,7 @@ class gallery_rest_Core { } $path = implode("/", $request->arguments); - $item = ORM::factory("item") - ->where("relative_url_cache", "=", $path) - ->viewable() - ->find(); - - if (!$item->loaded()) { - return rest::success(); - } - - if (!access::can("edit", $item)) { - return rest::not_found("Resource: {$path} permission denied."); - } + $item = gallery_rest::_get_item($path, "edit"); if ($item->id == 1) { return rest::invalid_request("Attempt to delete the root album"); @@ -183,6 +147,20 @@ class gallery_rest_Core { return rest::success(array("resource" => array("parent_path" => $parent->relative_url()))); } + private static function _get_item($path, $permission="view") { + $item = url::get_item_from_uri($path); + + if (!$item->loaded()) { + throw new Kohana_404_Exception(); + } + + if (!access::can($permission, $item)) { + throw new Kohana_404_Exception(); + } + + return $item; + } + private static function _get_children($item, $request) { $children = array(); $limit = empty($request->limit) ? null : $request->limit; diff --git a/modules/gallery/tests/Gallery_Rest_Helper_Test.php b/modules/gallery/tests/Gallery_Rest_Helper_Test.php index f36f6aaf..fba83d47 100644 --- a/modules/gallery/tests/Gallery_Rest_Helper_Test.php +++ b/modules/gallery/tests/Gallery_Rest_Helper_Test.php @@ -136,8 +136,12 @@ class Gallery_Rest_Helper_Test extends Unit_Test_Case { "title" => "Updated Title", "name" => "new name"); - $this->assert_equal(json_encode(array("status" => "ERROR", "message" => "Resource not found")), - gallery_rest::put($request)); + try { + gallery_rest::put($request); + } catch (Kohana_404_Exception $k404) { + } catch (Exception $e) { + $this->assert_false(true, $e->__toString()); + } } public function gallery_rest_put_album_no_edit_permission_test() { @@ -147,8 +151,12 @@ class Gallery_Rest_Helper_Test extends Unit_Test_Case { "title" => "Updated Title", "name" => "new name"); - $this->assert_equal(json_encode(array("status" => "ERROR", "message" => "Resource not found")), - gallery_rest::put($request)); + try { + gallery_rest::put($request); + } catch (Kohana_404_Exception $k404) { + } catch (Exception $e) { + $this->assert_false(true, $e->__toString()); + } } public function gallery_rest_put_album_rename_conflict_test() { diff --git a/modules/rest/helpers/rest.php b/modules/rest/helpers/rest.php index 276ff0c2..4b3166c0 100644 --- a/modules/rest/helpers/rest.php +++ b/modules/rest/helpers/rest.php @@ -46,14 +46,7 @@ class rest_Core { } /** - * Resource Not Found - */ - static function not_found($log_message=null) { - return self::_format_failure_response(t("Resource not found"), $log_message); - } - - /** - * Resource Not Found + * Request failed */ static function fail($log_message=null) { return self::_format_failure_response($log_message, $log_message); diff --git a/modules/tag/helpers/tag_rest.php b/modules/tag/helpers/tag_rest.php index ed6cfc1c..cfcf93b2 100644 --- a/modules/tag/helpers/tag_rest.php +++ b/modules/tag/helpers/tag_rest.php @@ -70,11 +70,11 @@ class tag_rest_Core { ->viewable() ->find(); if (!$item->loaded()) { - return rest::not_found("Resource: {$path} missing."); + throw new Kohana_404_Exception(); } if (!access::can("edit", $item)) { - return rest::not_found("Resource: {$path} permission denied."); + throw new Kohana_404_Exception(); } foreach ($tags as $tag) { @@ -94,7 +94,7 @@ class tag_rest_Core { ->where("name", "=", $name) ->find(); if (!$tag->loaded()) { - return rest::not_found("Tag: {$name} not found."); + throw new Kohana_404_Exception(); } $tag->name = $request->new_name; diff --git a/modules/tag/tests/Tag_Rest_Helper_Test.php b/modules/tag/tests/Tag_Rest_Helper_Test.php index 6b1c9a33..4408bf4b 100644 --- a/modules/tag/tests/Tag_Rest_Helper_Test.php +++ b/modules/tag/tests/Tag_Rest_Helper_Test.php @@ -119,9 +119,12 @@ class Tag_Rest_Helper_Test extends Unit_Test_Case { public function tag_rest_add_tags_for_item_not_found_test() { $request = (object)array("path" => $this->_photo->relative_url() . "b", "arguments" => array("new,one")); - $this->assert_equal( - json_encode(array("status" => "ERROR", "message" => "Resource not found")), - tag_rest::post($request)); + try { + tag_rest::post($request); + } catch (Kohana_404_Exception $k404) { + } catch (Exception $e) { + $this->assert_false(true, $e->__toString()); + } } public function tag_rest_add_tags_for_item_no_access_test() { @@ -129,9 +132,12 @@ class Tag_Rest_Helper_Test extends Unit_Test_Case { $request = (object)array("path" => $this->_photo->relative_url(), "arguments" => array("new,one")); - $this->assert_equal( - json_encode(array("status" => "ERROR", "message" => "Resource not found")), - tag_rest::post($request)); + try { + tag_rest::post($request); + } catch (Kohana_404_Exception $k404) { + } catch (Exception $e) { + $this->assert_false(true, $e->__toString()); + } } public function tag_rest_add_tags_for_item_test() { @@ -175,9 +181,12 @@ class Tag_Rest_Helper_Test extends Unit_Test_Case { public function tag_rest_update_tags_not_found_test() { $request = (object)array("arguments" => array("not"), "new_name" => "found"); - $this->assert_equal( - json_encode(array("status" => "ERROR", "message" => "Resource not found")), - tag_rest::put($request)); + try { + tag_rest::put($request); + } catch (Kohana_404_Exception $k404) { + } catch (Exception $e) { + $this->assert_false(true, $e->__toString()); + } } public function tag_rest_update_tags_test() { -- cgit v1.2.3 From 1a12a5e3c89c41ebd087591c16611fbab4293f5b Mon Sep 17 00:00:00 2001 From: Tim Almdal Date: Thu, 31 Dec 2009 11:51:51 -0800 Subject: Create a Rest_Exception class and use it to convey status to the client instead of calling rest::forbidden and other rest helper error messages. --- modules/gallery/helpers/gallery_rest.php | 7 ++-- modules/rest/controllers/rest.php | 23 +++++------ modules/rest/helpers/rest.php | 39 ++++--------------- modules/rest/libraries/Rest_Exception.php | 41 ++++++++++++++++++++ modules/rest/tests/Rest_Controller_Test.php | 60 ++++++++++++++++++++--------- modules/tag/helpers/tag_rest.php | 6 +-- 6 files changed, 109 insertions(+), 67 deletions(-) create mode 100644 modules/rest/libraries/Rest_Exception.php (limited to 'modules/rest/helpers/rest.php') diff --git a/modules/gallery/helpers/gallery_rest.php b/modules/gallery/helpers/gallery_rest.php index 21e2b939..563a2c7c 100644 --- a/modules/gallery/helpers/gallery_rest.php +++ b/modules/gallery/helpers/gallery_rest.php @@ -50,7 +50,7 @@ class gallery_rest_Core { static function put($request) { if (empty($request->arguments)) { - return rest::invalid_request(); + Rest_Exception::trigger(400, "Bad request"); } $path = implode("/", $request->arguments); $item = gallery_rest::_get_item($path, "edit"); @@ -78,7 +78,7 @@ class gallery_rest_Core { static function post($request) { if (empty($request->arguments)) { - return rest::invalid_request(); + Rest_Exception::trigger(400, "Bad request"); } $components = $request->arguments; @@ -125,6 +125,7 @@ class gallery_rest_Core { static function delete($request) { if (empty($request->arguments)) { + Rest_Exception::trigger(400, "Bad request", $log_message); return rest::invalid_request(); } $path = implode("/", $request->arguments); @@ -132,7 +133,7 @@ class gallery_rest_Core { $item = gallery_rest::_get_item($path, "edit"); if ($item->id == 1) { - return rest::invalid_request("Attempt to delete the root album"); + Rest_Exception::trigger(400, "Bad request", "Attempt to delete the root album"); } $parent = $item->parent(); diff --git a/modules/rest/controllers/rest.php b/modules/rest/controllers/rest.php index 6715bc15..b71e60f5 100644 --- a/modules/rest/controllers/rest.php +++ b/modules/rest/controllers/rest.php @@ -20,18 +20,17 @@ class Rest_Controller extends Controller { public function access_key() { $request = (object)Input::instance()->get(); if (empty($request->user) || empty($request->password)) { - print rest::forbidden("No user or password supplied"); - return; + Rest_Exception::trigger(403, "Forbidden", "No user or password supplied"); } $user = identity::lookup_user_by_name($request->user); if (empty($user)) { - print rest::forbidden("User '{$request->user}' not found"); + Rest_Exception::trigger(403, "Forbidden", "User '{$request->user}' not found"); return; } if (!identity::is_correct_password($user, $request->password)) { - print rest::forbidden("Invalid password for '{$request->user}'."); + Rest_Exception::trigger(403, "Forbidden", "Invalid password for '{$request->user}'."); return; } @@ -55,14 +54,16 @@ class Rest_Controller extends Controller { $handler_method = $request->method; if (!method_exists($handler_class, $handler_method)) { - print rest::not_implemented("$handler_class::$handler_method is not implemented"); - return; + Rest_Exception::trigger(501, "Not implemented", "$handler_class::$handler_method"); } print call_user_func(array($handler_class, $handler_method), $request); } + } catch (Rest_Exception $e) { + $e->sendHeaders(); } catch (Exception $e) { - print rest::internal_error($e->__toString()); + Kohana_Log::add("error", $e->__toString()); + header("HTTP/1.1 500 Internal Error"); } } @@ -100,12 +101,12 @@ class Rest_Controller extends Controller { if ($key->loaded()) { $user = identity::lookup_user($key->user_id); if (empty($user)) { - print rest::forbidden("User not found: {$key->user_id}"); - return false;; + Rest_Exception::trigger(403, "Forbidden", $log_message, + "User not found: {$key->user_id}"); } } else { - print rest::forbidden("Invalid user access token supplied: {$key->user_id}"); - return false; + Rest_Exception::trigger(403, "Forbidden", $log_message, + "Invalid user access token supplied: {$key->user_id}"); } } identity::set_active_user($user); diff --git a/modules/rest/helpers/rest.php b/modules/rest/helpers/rest.php index 4b3166c0..7684567c 100644 --- a/modules/rest/helpers/rest.php +++ b/modules/rest/helpers/rest.php @@ -17,39 +17,23 @@ * Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, MA 02110-1301, USA. */ class rest_Core { - /** - * Authorization Failure - */ - static function forbidden($log_message=null) { - return self::_format_failure_response(t("Authorization failed"), $log_message); - } - - /** - * Invalid Failure - */ - static function invalid_request($log_message=null) { - return self::_format_failure_response(t("Invalid request"), $log_message); - } - /** * Not Implemented */ static function not_implemented($log_message=null) { - return self::_format_failure_response(t("Service not implemented"), $log_message); - } - - /** - * Internal Error - */ - static function internal_error($log_message=null) { - return self::_format_failure_response(t("Internal error"), $log_message); + Rest_Exception::trigger(501, "Not implemented", $log_message); } /** * Request failed */ static function fail($log_message=null) { - return self::_format_failure_response($log_message, $log_message); + if (!empty($log_message)) { + Kohana_Log::add("info", $log_message); + } + // We don't need to save the session for this request + Session::abort_save(); + return json_encode(array("status" => "ERROR", "message" => (string)$message)); } /** @@ -78,13 +62,4 @@ class rest_Core { Session::abort_save(); return json_encode($response); } - - private static function _format_failure_response($message, $log_message) { - if (!empty($log_message)) { - Kohana_Log::add("info", $log_message); - } - // We don't need to save the session for this request - Session::abort_save(); - return json_encode(array("status" => "ERROR", "message" => (string)$message)); - } } diff --git a/modules/rest/libraries/Rest_Exception.php b/modules/rest/libraries/Rest_Exception.php new file mode 100644 index 00000000..acdcb568 --- /dev/null +++ b/modules/rest/libraries/Rest_Exception.php @@ -0,0 +1,41 @@ +getMessage()}'); + } +} // End Rest Exception \ No newline at end of file diff --git a/modules/rest/tests/Rest_Controller_Test.php b/modules/rest/tests/Rest_Controller_Test.php index 6bebc47d..21b83fe6 100644 --- a/modules/rest/tests/Rest_Controller_Test.php +++ b/modules/rest/tests/Rest_Controller_Test.php @@ -75,26 +75,38 @@ class Rest_Controller_Test extends Unit_Test_Case { public function rest_access_key_no_parameters_test() { $_SERVER["REQUEST_METHOD"] = "GET"; - $this->assert_equal( - json_encode(array("status" => "ERROR", "message" => (string)t("Authorization failed"))), - $this->_call_controller()); + try { + $this->_call_controller(); + } catch (Rest_Exception $e) { + $this->assert_equal("403 Forbidden", $e->getMessage()); + } catch (Exception $e) { + $this->assert_false(true, $e->__toString()); + } } public function rest_access_key_user_not_found_test() { $_SERVER["REQUEST_METHOD"] = "POST"; $_POST["request"] = json_encode(array("user" => "access_test2", "password" => "password")); - $this->assert_equal( - json_encode(array("status" => "ERROR", "message" => (string)t("Authorization failed"))), - $this->_call_controller()); + try { + $this->_call_controller(); + } catch (Rest_Exception $e) { + $this->assert_equal("403 Forbidden", $e->getMessage()); + } catch (Exception $e) { + $this->assert_false(true, $e->__toString()); + } } public function rest_access_key_invalid_password_test() { $_SERVER["REQUEST_METHOD"] = "POST"; - $this->assert_equal( - json_encode(array("status" => "ERROR", "message" => (string)t("Authorization failed"))), - $this->_call_controller()); + try { + $this->_call_controller(); + } catch (Rest_Exception $e) { + $this->assert_equal("403 Forbidden", $e->getMessage()); + } catch (Exception $e) { + $this->assert_false(true, $e->__toString()); + } } public function rest_get_resource_no_request_key_test() { @@ -114,9 +126,13 @@ class Rest_Controller_Test extends Unit_Test_Case { $_SERVER["HTTP_X_GALLERY_REQUEST_KEY"] = md5($this->_access_key); // screw up the access key; $_SERVER["REQUEST_METHOD"] = "GET"; - $this->assert_equal( - json_encode(array("status" => "ERROR", "message" => (string)t("Authorization failed"))), - $this->_call_controller()); + try { + $this->_call_controller(); + } catch (Rest_Exception $e) { + $this->assert_equal("403 Forbidden", $e->getMessage()); + } catch (Exception $e) { + $this->assert_false(true, $e->__toString()); + } } public function rest_get_resource_no_user_for_key_test() { @@ -126,9 +142,13 @@ class Rest_Controller_Test extends Unit_Test_Case { $this->_user->delete(); unset($this->_user); - $this->assert_equal( - json_encode(array("status" => "ERROR", "message" => (string)t("Authorization failed"))), - $this->_call_controller("rest", explode("/", $this->_photo->relative_url()))); + try { + $this->_call_controller("rest", explode("/", $this->_photo->relative_url())); + } catch (Rest_Exception $e) { + $this->assert_equal("403 Forbidden", $e->getMessage()); + } catch (Exception $e) { + $this->assert_false(true, $e->__toString()); + } } public function rest_get_resource_no_handler_test() { @@ -136,9 +156,13 @@ class Rest_Controller_Test extends Unit_Test_Case { $_SERVER["HTTP_X_GALLERY_REQUEST_KEY"] = $this->_access_key; $_SERVER["HTTP_X_GALLERY_REQUEST_METHOD"] = "PUT"; - $this->assert_equal( - json_encode(array("status" => "ERROR", "message" => (string)t("Service not implemented"))), - $this->_call_controller("rest", explode("/", $this->_photo->relative_url()))); + try { + $this->_call_controller("rest", explode("/", $this->_photo->relative_url())); + } catch (Rest_Exception $e) { + $this->assert_equal("501 Not Implemented", $e->getMessage()); + } catch (Exception $e) { + $this->assert_false(true, $e->__toString()); + } } public function rest_get_resource_test() { diff --git a/modules/tag/helpers/tag_rest.php b/modules/tag/helpers/tag_rest.php index cfcf93b2..29b74510 100644 --- a/modules/tag/helpers/tag_rest.php +++ b/modules/tag/helpers/tag_rest.php @@ -60,7 +60,7 @@ class tag_rest_Core { static function post($request) { if (empty($request->arguments) || count($request->arguments) != 1 || empty($request->path)) { - return rest::invalid_request(); + Rest_Exception::trigger(400, "Bad request"); } $path = $request->path; $tags = explode(",", $request->arguments[0]); @@ -85,7 +85,7 @@ class tag_rest_Core { static function put($request) { if (empty($request->arguments[0]) || empty($request->new_name)) { - return rest::invalid_request(); + Rest_Exception::trigger(400, "Bad request"); } $name = $request->arguments[0]; @@ -105,7 +105,7 @@ class tag_rest_Core { static function delete($request) { if (empty($request->arguments[0])) { - return rest::invalid_request(); + Rest_Exception::trigger(400, "Bad request"); } $tags = explode(",", $request->arguments[0]); if (!empty($request->path)) { -- cgit v1.2.3 From 4611eb2142b155f1113e35b456fe38e33f0884fb Mon Sep 17 00:00:00 2001 From: Tim Almdal Date: Thu, 31 Dec 2009 12:32:54 -0800 Subject: Move the set_active_user and normalize_request methods to rest.php helper --- modules/rest/controllers/rest.php | 50 ++---------------------------------- modules/rest/helpers/rest.php | 54 ++++++++++++++++++++++++++++++++++----- 2 files changed, 49 insertions(+), 55 deletions(-) (limited to 'modules/rest/helpers/rest.php') diff --git a/modules/rest/controllers/rest.php b/modules/rest/controllers/rest.php index b71e60f5..05935e75 100644 --- a/modules/rest/controllers/rest.php +++ b/modules/rest/controllers/rest.php @@ -47,9 +47,9 @@ class Rest_Controller extends Controller { } public function __call($function, $args) { - $request = $this->_normalize_request($args); + $request = rest::normalize_request($args); try { - if ($this->_set_active_user($request->access_token)) { + if (rest::set_active_user($request->access_token)) { $handler_class = "{$function}_rest"; $handler_method = $request->method; @@ -66,50 +66,4 @@ class Rest_Controller extends Controller { header("HTTP/1.1 500 Internal Error"); } } - - private 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]; - } - } - - $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 - - return $request; - } - - private 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)) { - Rest_Exception::trigger(403, "Forbidden", $log_message, - "User not found: {$key->user_id}"); - } - } else { - Rest_Exception::trigger(403, "Forbidden", $log_message, - "Invalid user access token supplied: {$key->user_id}"); - } - } - identity::set_active_user($user); - return true; - } } \ No newline at end of file diff --git a/modules/rest/helpers/rest.php b/modules/rest/helpers/rest.php index 7684567c..7e2445e4 100644 --- a/modules/rest/helpers/rest.php +++ b/modules/rest/helpers/rest.php @@ -17,13 +17,6 @@ * Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, MA 02110-1301, USA. */ class rest_Core { - /** - * Not Implemented - */ - static function not_implemented($log_message=null) { - Rest_Exception::trigger(501, "Not implemented", $log_message); - } - /** * Request failed */ @@ -62,4 +55,51 @@ class rest_Core { Session::abort_save(); return json_encode($response); } + + + 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]; + } + } + + $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 + + return $request; + } + + 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)) { + Rest_Exception::trigger(403, "Forbidden", $log_message, + "User not found: {$key->user_id}"); + } + } else { + Rest_Exception::trigger(403, "Forbidden", $log_message, + "Invalid user access token supplied: {$key->user_id}"); + } + } + identity::set_active_user($user); + return true; + } } -- cgit v1.2.3 From 28597ba53354537704899e7ad9eb39bbd5718b21 Mon Sep 17 00:00:00 2001 From: Tim Almdal Date: Sat, 2 Jan 2010 14:31:59 -0800 Subject: Correct file structure tests, Have the tests delete the userid they create so as not to impact other tests. --- modules/gallery/tests/Gallery_Rest_Helper_Test.php | 22 ++++++++++------ modules/rest/controllers/rest.php | 3 ++- modules/rest/helpers/rest.php | 3 ++- modules/rest/helpers/rest_event.php | 3 ++- modules/rest/tests/Rest_Controller_Test.php | 29 ++++++++++++++-------- modules/tag/tests/Tag_Rest_Helper_Test.php | 20 +++++++++------ 6 files changed, 52 insertions(+), 28 deletions(-) (limited to 'modules/rest/helpers/rest.php') diff --git a/modules/gallery/tests/Gallery_Rest_Helper_Test.php b/modules/gallery/tests/Gallery_Rest_Helper_Test.php index 4cd3f2a6..605a4f37 100644 --- a/modules/gallery/tests/Gallery_Rest_Helper_Test.php +++ b/modules/gallery/tests/Gallery_Rest_Helper_Test.php @@ -26,17 +26,25 @@ class Gallery_Rest_Helper_Test extends Unit_Test_Case { public function teardown() { list($_GET, $_POST, $_SERVER, $_FILES) = $this->_save; identity::set_active_user($this->_saved_active_user); + if (!empty($this->_user)) { + try { + $this->_user->delete(); + } catch (Exception $e) { } + } } private function _create_user() { - $user = identity::create_user("access_test" . rand(), "Access Test", "password"); - $key = ORM::factory("user_access_token"); - $key->access_key = md5($user->name . rand()); - $key->user_id = $user->id; - $key->save(); - identity::set_active_user($user); - return $user; + if (empty($this->_user)) { + $this->_user = identity::create_user("access_test" . rand(), "Access Test", "password"); + $key = ORM::factory("user_access_token"); + $key->access_key = md5($this->_user->name . rand()); + $key->user_id = $this->_user->id; + $key->save(); + identity::set_active_user($this->_user); + } + return $this->_user; } + private function _create_album($parent=null) { $album_name = "album_" . rand(); if (empty($parent)) { diff --git a/modules/rest/controllers/rest.php b/modules/rest/controllers/rest.php index 446ec7cb..39ca4797 100644 --- a/modules/rest/controllers/rest.php +++ b/modules/rest/controllers/rest.php @@ -1,4 +1,5 @@ -access_key = md5($user->name . rand()); - $key->user_id = $user->id; - $key->save(); - return array($key->access_key, $user); + if (empty($this->_user)) { + $this->_user = identity::create_user("access_test" . rand(), "Access Test", "password"); + $this->_key = ORM::factory("user_access_token"); + $this->_key->access_key = md5($this->_user->name . rand()); + $this->_key->user_id = $this->_user->id; + $this->_key->save(); + identity::set_active_user($this->_user); + } + return array($this->_key->access_key, $this->_user); + } + + public function teardown() { + list($_GET, $_POST, $_SERVER) = $this->_save; + if (!empty($this->_user)) { + try { + $this->_user->delete(); + } catch (Exception $e) { } + } } private function _create_image($parent=null) { @@ -40,11 +52,6 @@ class Rest_Controller_Test extends Unit_Test_Case { return photo::create($parent, $filename, "$image_name.jpg", $image_name); } - - public function teardown() { - list($_GET, $_POST, $_SERVER) = $this->_save; - } - public function rest_access_key_exists_test() { list ($access_key, $user) = $this->_create_user(); $_SERVER["REQUEST_METHOD"] = "GET"; diff --git a/modules/tag/tests/Tag_Rest_Helper_Test.php b/modules/tag/tests/Tag_Rest_Helper_Test.php index ac64470c..055e5cec 100644 --- a/modules/tag/tests/Tag_Rest_Helper_Test.php +++ b/modules/tag/tests/Tag_Rest_Helper_Test.php @@ -31,18 +31,24 @@ class Tag_Rest_Helper_Test extends Unit_Test_Case { Database::instance()->query("TRUNCATE {tags}"); Database::instance()->query("TRUNCATE {items_tags}"); + if (!empty($this->_user)) { + $this->_user->delete(); + } } catch (Exception $e) { } } private function _create_user() { - $user = identity::create_user("access_test" . rand(), "Access Test", "password"); - $key = ORM::factory("user_access_token"); - $key->access_key = md5($user->name . rand()); - $key->user_id = $user->id; - $key->save(); - identity::set_active_user($user); - return $user; + if (empty($this->_user)) { + $this->_user = identity::create_user("access_test" . rand(), "Access Test", "password"); + $key = ORM::factory("user_access_token"); + $key->access_key = md5($this->_user->name . rand()); + $key->user_id = $this->_user->id; + $key->save(); + identity::set_active_user($this->_user); + } + return $this->_user; } + private function _create_album($tags=array(), $parent=null) { $album_name = "album_" . rand(); if (empty($parent)) { -- cgit v1.2.3 From 5b9801092b3c347161f9e3b8069e05945a5010d2 Mon Sep 17 00:00:00 2001 From: Tim Almdal Date: Sat, 2 Jan 2010 16:55:06 -0800 Subject: Remove the Rest_Exception::trigger method. --- modules/gallery/helpers/gallery_rest.php | 9 +++--- modules/rest/controllers/rest.php | 51 +++++++++++++++---------------- modules/rest/helpers/rest.php | 6 ++-- modules/rest/libraries/Rest_Exception.php | 35 +++++++++------------ modules/tag/helpers/tag_rest.php | 6 ++-- 5 files changed, 49 insertions(+), 58 deletions(-) (limited to 'modules/rest/helpers/rest.php') diff --git a/modules/gallery/helpers/gallery_rest.php b/modules/gallery/helpers/gallery_rest.php index 563a2c7c..a87ebb4e 100644 --- a/modules/gallery/helpers/gallery_rest.php +++ b/modules/gallery/helpers/gallery_rest.php @@ -50,7 +50,7 @@ class gallery_rest_Core { static function put($request) { if (empty($request->arguments)) { - Rest_Exception::trigger(400, "Bad request"); + throw new Rest_Exception(400, "Bad request"); } $path = implode("/", $request->arguments); $item = gallery_rest::_get_item($path, "edit"); @@ -78,7 +78,7 @@ class gallery_rest_Core { static function post($request) { if (empty($request->arguments)) { - Rest_Exception::trigger(400, "Bad request"); + throw new Rest_Exception(400, "Bad request"); } $components = $request->arguments; @@ -125,15 +125,14 @@ class gallery_rest_Core { static function delete($request) { if (empty($request->arguments)) { - Rest_Exception::trigger(400, "Bad request", $log_message); - return rest::invalid_request(); + throw new Rest_Exception(400, "Bad request"); } $path = implode("/", $request->arguments); $item = gallery_rest::_get_item($path, "edit"); if ($item->id == 1) { - Rest_Exception::trigger(400, "Bad request", "Attempt to delete the root album"); + throw new Rest_Exception(400, "Bad request"); } $parent = $item->parent(); diff --git a/modules/rest/controllers/rest.php b/modules/rest/controllers/rest.php index 39ca4797..26e5b31a 100644 --- a/modules/rest/controllers/rest.php +++ b/modules/rest/controllers/rest.php @@ -19,32 +19,34 @@ */ class Rest_Controller extends Controller { public function access_key() { - $request = (object)Input::instance()->get(); - if (empty($request->user) || empty($request->password)) { - Rest_Exception::trigger(403, "Forbidden", "No user or password supplied"); - } + try { + $request = (object)Input::instance()->get(); + if (empty($request->user) || empty($request->password)) { + throw new Rest_Exception(403, "Forbidden"); + } - $user = identity::lookup_user_by_name($request->user); - if (empty($user)) { - Rest_Exception::trigger(403, "Forbidden", "User '{$request->user}' not found"); - return; - } + $user = identity::lookup_user_by_name($request->user); + if (empty($user)) { + throw new Rest_Exception(403, "Forbidden"); + } - if (!identity::is_correct_password($user, $request->password)) { - Rest_Exception::trigger(403, "Forbidden", "Invalid password for '{$request->user}'."); - return; - } + if (!identity::is_correct_password($user, $request->password)) { + throw new Rest_Exception(403, "Forbidden"); + } - $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 = 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(); + } + print rest::success(array("token" => $key->access_key)); + } catch (Rest_Exception $e) { + $e->sendHeaders(); } - print rest::success(array("token" => $key->access_key)); - } + } public function __call($function, $args) { $request = rest::normalize_request($args); @@ -54,16 +56,13 @@ class Rest_Controller extends Controller { $handler_method = $request->method; if (!method_exists($handler_class, $handler_method)) { - Rest_Exception::trigger(501, "Not implemented", "$handler_class::$handler_method"); + throw new Rest_Exception(403, "Forbidden"); } print call_user_func(array($handler_class, $handler_method), $request); } } catch (Rest_Exception $e) { $e->sendHeaders(); - } catch (Exception $e) { - Kohana_Log::add("error", $e->__toString()); - header("HTTP/1.1 500 Internal Error"); } } } \ No newline at end of file diff --git a/modules/rest/helpers/rest.php b/modules/rest/helpers/rest.php index 00790e6b..be0644f2 100644 --- a/modules/rest/helpers/rest.php +++ b/modules/rest/helpers/rest.php @@ -92,12 +92,10 @@ class rest_Core { if ($key->loaded()) { $user = identity::lookup_user($key->user_id); if (empty($user)) { - Rest_Exception::trigger(403, "Forbidden", $log_message, - "User not found: {$key->user_id}"); + throw new Rest_Exception(403, "Forbidden"); } } else { - Rest_Exception::trigger(403, "Forbidden", $log_message, - "Invalid user access token supplied: {$key->user_id}"); + throw new Rest_Exception(403, "Forbidden"); } } identity::set_active_user($user); diff --git a/modules/rest/libraries/Rest_Exception.php b/modules/rest/libraries/Rest_Exception.php index acdcb568..905b94a0 100644 --- a/modules/rest/libraries/Rest_Exception.php +++ b/modules/rest/libraries/Rest_Exception.php @@ -1,15 +1,22 @@ -arguments) || count($request->arguments) != 1 || empty($request->path)) { - Rest_Exception::trigger(400, "Bad request"); + throw new Rest_Exception(400, "Bad request"); } $path = $request->path; $tags = explode(",", $request->arguments[0]); @@ -85,7 +85,7 @@ class tag_rest_Core { static function put($request) { if (empty($request->arguments[0]) || empty($request->new_name)) { - Rest_Exception::trigger(400, "Bad request"); + throw new Rest_Exception(400, "Bad request"); } $name = $request->arguments[0]; @@ -105,7 +105,7 @@ class tag_rest_Core { static function delete($request) { if (empty($request->arguments[0])) { - Rest_Exception::trigger(400, "Bad request"); + throw new Rest_Exception(400, "Bad request"); } $tags = explode(",", $request->arguments[0]); if (!empty($request->path)) { -- cgit v1.2.3