Create a Rest_Exception class and use it to convey status to the clie…

…nt instead of calling rest::forbidden and other rest helper error messages.
gallery · Dec 31, 2009 · 1a12a5e · 1a12a5e
1 parent 732047e
commit 1a12a5e
Show file tree

Hide file tree

Showing 6 changed files with 109 additions and 67 deletions.
diff --git a/modules/gallery/helpers/gallery_rest.php b/modules/gallery/helpers/gallery_rest.php
@@ -50,7 +50,7 @@ static function get($request) {
 
   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 @@ static function put($request) {
 
   static function post($request) {
     if (empty($request->arguments)) {
-      return rest::invalid_request();
+      Rest_Exception::trigger(400, "Bad request");
     }
 
     $components = $request->arguments;
@@ -125,14 +125,15 @@ static function post($request) {
 
   static function delete($request) {
     if (empty($request->arguments)) {
+      Rest_Exception::trigger(400, "Bad request", $log_message);
       return rest::invalid_request();
     }
     $path = implode("/", $request->arguments);
 
     $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
@@ -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 @@ public function __call($function, $args) {
         $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 @@ private function _set_active_user($access_token) {
       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
@@ -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 @@ static function validation_error($error_data) {
     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
@@ -0,0 +1,41 @@
+<?php defined('SYSPATH') OR die('No direct access allowed.');
+/**
+ * Creates a "Page Not Found" exception.
+ *
+ * $Id: Kohana_404_Exception.php 4679 2009-11-10 01:45:52Z isaiah $
+ *
+ * @package    Core
+ * @author     Kohana Team
+ * @copyright  (c) 2007-2009 Kohana Team
+ * @license    http://kohanaphp.com/license
+ */
+
+class Rest_Exception_Core extends Exception {
+  /**
+   * Set internal properties.
+   */
+  public function __construct($code, $text) {
+    parent::__construct("$code $text");
+  }
+
+  /**
+   * Throws a new Rest exception.
+   *
+   * @throws  Rest_Exception
+   * @return  void
+   */
+  public static function trigger($code, $text, $log_message=null) {
+    $message = "$code: $text" . (!empty($log_message) ? "\n$log_message" : "");
+    Kohana_Log::add("info", $message);
+    throw new Rest_Exception($code, $text);
+  }
+
+  /**
+   * Sends the headers, to emulate server behavior.
+   *
+   * @return void
+   */
+  public function sendHeaders() {
+    header('HTTP/1.1 {$this->getMessage()}');
+  }
+} // End Rest Exception
diff --git a/modules/rest/tests/Rest_Controller_Test.php b/modules/rest/tests/Rest_Controller_Test.php
@@ -75,26 +75,38 @@ public function rest_access_key_generated_test() {
   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 @@ public function rest_get_resource_invalid_key_test() {
     $_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,19 +142,27 @@ public function rest_get_resource_no_user_for_key_test() {
     $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() {
     $_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", 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
@@ -60,7 +60,7 @@ static function get($request) {
 
   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 @@ static function post($request) {
 
   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 @@ static function put($request) {
 
   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)) {