Change the way that we do RESTful routing.

1) We now use __call() in REST_Controller to handle any requests to a controller
   that were not already handled.  In the case of RESTful controllers, this should
   be the only entry point (although they're free to break the model and add other
   ones.. nothing stops them).

   This means that we can remove all the catch-all routes in
   routes.php which greatly simplifies it.

2) Move request_method() and output_format() out of REST_Controller and into the REST
   helper in core/helpers/rest.php

3) Experiment with letting the various subclasses check the output_format and deal with
   it themselves.  This simplifies the API, but it might be a bad idea in that it might
   push too much work to the individual controllers.  It's a balancing act, time will tell,
   I'm willing to change it back later.

9 files changed, 92 insertions, 71 deletions

diff --git a/core/config/routes.php b/core/config/routes.php
index 0e2aa887..ac4a8cb0 100644
--- a/core/config/routes.php
+++ b/core/config/routes.php
@@ -18,20 +18,8 @@
  * Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, MA  02110-1301, USA.
  */
 
-// FIXME Temporary workaround to show the welcome page at /welcome.
-// The problem is that we're routing all requests to /{controllername} to Rest_Controller,
-// even requests to controllers that do not implement Rest_Controller.
-$config['^welcome$'] = 'welcome';
-$config['^media_rss/feed/(\d+)'] = 'media_rss/feed/$1';
-
-// REST configuration
-// Any resource requests (eg: album/1 or comment/3) get dispatched to the REST
-// dispatcher, and the abstract REST_Controller is not directly routable.
-$config['^rest'] = null;
-$config['^rest/.*'] = null;
-$config['^(\w+)/(\d+)$'] = '$1/dispatch/$2';
-$config['^(\w+)$'] = '$1/dispatch/0';
-$config['^form/(edit|add)/(\w+)/(.*)$'] = '$2/form_$1/$3';
+// The abstract REST_Controller is not directly routable.
+$config['^rest\b'] = null;
 
 // For now our default page is the scaffolding.
 $config['_default'] = 'welcome';
diff --git a/core/controllers/albums.php b/core/controllers/albums.php
index ead738d9..f9e0be2c 100644
--- a/core/controllers/albums.php
+++ b/core/controllers/albums.php
@@ -20,9 +20,9 @@
 class Albums_Controller extends Items_Controller {
 
   /**
-   *  @see Rest_Controller::_show($resource, $output_format)
+   *  @see Rest_Controller::_show($resource)
    */
-  public function _show($item, $output_format) {
+  public function _show($item) {
     // @todo: these need to be pulled from the database
     $theme_name = "default";
     $page_size = 9;
diff --git a/core/controllers/items.php b/core/controllers/items.php
index 6c202b30..d56ef774 100644
--- a/core/controllers/items.php
+++ b/core/controllers/items.php
@@ -41,7 +41,7 @@ class Items_Controller extends REST_Controller {
     throw new Exception("@todo Comment_Controller::_form NOT IMPLEMENTED");
   }
 
-  public function _show($item, $format) {
+  public function _show($item) {
     // Redirect to the more specific resource type, since it will render
     // differently.  We could also just delegate here, but it feels more appropriate
     // to have a single canonical resource mapping.
diff --git a/core/controllers/photos.php b/core/controllers/photos.php
index a5b75b79..1d995de8 100644
--- a/core/controllers/photos.php
+++ b/core/controllers/photos.php
@@ -20,9 +20,9 @@
 class Photos_Controller extends Items_Controller {
 
   /**
-   *  @see Rest_Controller::_show($resource, $output_format)
+   *  @see Rest_Controller::_show($resource)
    */
-  public function _show($item, $output_format) {
+  public function _show($item) {
     $template = new View("page.html");
 
     // @todo: this needs to be data-driven
diff --git a/core/controllers/rest.php b/core/controllers/rest.php
index 6391c153..25f71299 100644
--- a/core/controllers/rest.php
+++ b/core/controllers/rest.php
@@ -28,19 +28,19 @@
  *     // Handle GET request to /controller
  *   }
  *
- *   public function _show(ORM $comment, $output_format) {
+ *   public function _show(ORM $comment) {
  *     // Handle GET request to /comments/{comment_id}
  *   }
  *
- *   public function _update(ORM $comment, $output_format) {
+ *   public function _update(ORM $comment) {
  *     // Handle PUT request to /comments/{comment_id}
  *   }
  *
- *   public function _create(ORM $comment, $output_format) {
+ *   public function _create(ORM $comment) {
  *     // Handle POST request to /comments
  *   }
  *
- *   public function _delete(ORM $comment, $output_format) {
+ *   public function _delete(ORM $comment) {
  *     // Handle DELETE request to /comments/{comments_id}
  *   }
  *
@@ -62,33 +62,42 @@
 abstract class REST_Controller extends Controller {
   protected $resource_type = null;
 
-  public function dispatch($id) {
+  public function __construct() {
     if ($this->resource_type == null) {
       throw new Exception("@todo ERROR_MISSING_RESOURCE_TYPE");
     }
+    parent::__construct();
+  }
 
-    // A request on /controller gets routed to REST_Controller::dispatch(0).
-    if (!$id && $this->request_method() == "get") {
+  /**
+   * Handle dispatching for all REST controllers.
+   */
+  public function __call($function, $args) {
+    // If no parameter was provided after the controller name (eg "/albums") then $function will
+    // be set to "index".  Otherwise, $function is the first parameter, and $args are all
+    // subsequent parameters.
+    $request_method = rest::request_method();
+    if ($function == "index" && $request_method == "get") {
       return $this->_index();
     }
 
     // @todo this needs security checks
+    $id = $function;
     $resource = ORM::factory($this->resource_type, $id);
-    if (!$resource->loaded && !$this->request_method() == "post") {
+    if (!$resource->loaded && $request_method == "post") {
       return Kohana::show_404();
     }
 
-    if ($this->request_method() == "get") {
-      $this->_show($resource, $this->output_format());
+    switch ($request_method) {
+    case "get":
+      $this->_show($resource);
 
       if (Session::instance()->get("use_profiler", false)) {
         $profiler = new Profiler();
         $profiler->render();
       }
       return;
-    }
 
-    switch ($this->request_method()) {
     case "put":
       return $this->_update($resource);
 
@@ -121,38 +130,6 @@ abstract class REST_Controller extends Controller {
   }
 
   /**
-   * We're expecting to run in an environment that only supports GET/POST, so expect to tunnel
-   * PUT and DELETE through POST.
-   *
-   * Returns the HTTP request method taking into consideration PUT/DELETE tunneling.
-   * @todo Move this to a MY_request helper?
-   * @return string HTTP request method
-   */
-  protected function request_method() {
-    if (request::method() == "get") {
-      return "get";
-    } else {
-      switch ($this->input->post("_method", $this->input->get("_method"))) {
-      case "put":    return "put";
-      case "delete": return "delete";
-      default:       return "post";
-      }
-    }
-  }
-
-  /**
-   * Choose an output format based on what the client prefers to accept.
-   * @return string "html", "xml" or "json"
-   */
-  protected function output_format() {
-    // Pick a format, but let it be overridden.
-    return $this->input->get(
-      "_format", $this->input->post(
-        "_format", request::preferred_accept(
-          array("html", "xml", "json"))));
-  }
-
-  /**
    * Perform a GET request on the controller root
    * (e.g. http://www.example.com/gallery3/comments)
    */
@@ -168,7 +145,7 @@ abstract class REST_Controller extends Controller {
    * Perform a GET request on this resource
    * @param ORM $resource the instance of this resource type
    */
-  abstract public function _show($resource, $output_format);
+  abstract public function _show($resource);
 
   /**
    * Perform a PUT request on this resource
diff --git a/core/helpers/rest.php b/core/helpers/rest.php
new file mode 100644
index 00000000..154ef6f6
--- /dev/null
+++ b/core/helpers/rest.php
@@ -0,0 +1,55 @@
+<?php defined("SYSPATH") or die("No direct script access.");
+/**
+ * Gallery - a web based photo album viewer and editor
+ * Copyright (C) 2000-2008 Bharat Mediratta
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or (at
+ * your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+
+class REST_Core {
+  /**
+   * We're expecting to run in an environment that only supports GET/POST, so expect to tunnel
+   * PUT and DELETE through POST.
+   *
+   * Returns the HTTP request method taking into consideration PUT/DELETE tunneling.
+   * @todo Move this to a MY_request helper?
+   * @return string HTTP request method
+   */
+  public static function request_method() {
+    if (request::method() == "get") {
+      return "get";
+    } else {
+      $input = Input::instance();
+      switch ($input->post("_method", $input->get("_method"))) {
+      case "put":    return "put";
+      case "delete": return "delete";
+      default:       return "post";
+      }
+    }
+  }
+
+  /**
+   * Choose an output format based on what the client prefers to accept.
+   * @return string "html", "xml" or "json"
+   */
+  public static function output_format() {
+    // Pick a format, but let it be overridden.
+      $input = Input::instance();
+    return $input->get(
+      "_format", $input->post(
+        "_format", request::preferred_accept(
+          array("html", "xml", "json"))));
+  }
+}
diff --git a/modules/comment/controllers/comments.php b/modules/comment/controllers/comments.php
index 89e42753..11a9c2a1 100644
--- a/modules/comment/controllers/comments.php
+++ b/modules/comment/controllers/comments.php
@@ -32,7 +32,7 @@ class Comments_Controller extends REST_Controller {
       header("HTTP/1.1 400 Bad Request");
       return;
     }
-    print comment::get_comments($item_id, $this->get_output_format());
+    print comment::get_comments($item_id);
   }
 
   /**
@@ -59,9 +59,10 @@ class Comments_Controller extends REST_Controller {
   /**
    * Display an existing comment.
    *  @todo Set proper Content-Type in a central place (REST_Controller::dispatch?).
-   *  @see Rest_Controller::_show($resource, $output_format)
+   *  @see Rest_Controller::_show($resource)
    */
-  public function _show($comment, $output_format) {
+  public function _show($comment) {
+    $output_format = rest::output_format();
     switch ($output_format) {
     case "xml":
       header("Content-Type: application/xml");
diff --git a/modules/comment/helpers/comment.php b/modules/comment/helpers/comment.php
index e9628a01..1739497c 100644
--- a/modules/comment/helpers/comment.php
+++ b/modules/comment/helpers/comment.php
@@ -106,7 +106,7 @@ class Comment_Core {
   }
 
   // @todo Set proper Content-Type in a central place (REST_Controller::dispatch?).
-  static function get_comments($item_id, $output_format) {
+  static function get_comments($item_id) {
     $comments = ORM::factory('comment')->where('item_id', $item_id)
       ->orderby('datetime', 'asc')
       ->find_all();
@@ -116,7 +116,7 @@ class Comment_Core {
       return;
     }
 
-    switch ($output_format) {
+    switch (rest::output_format()) {
     case "xml":
       header("Content-Type: application/xml");
       return xml::to_xml($comments, array("comments", "comment"));
diff --git a/modules/user/controllers/users.php b/modules/user/controllers/users.php
index 871af942..f6b77d0d 100644
--- a/modules/user/controllers/users.php
+++ b/modules/user/controllers/users.php
@@ -36,9 +36,9 @@ class Users_Controller extends REST_Controller {
   }
 
   /**
-   * @see Rest_Controller::_show($resource, $output_format)
+   * @see Rest_Controller::_show($resource)
    */
-  public function _show($user, $output_format) {
+  public function _show($user) {
     throw new Exception("@todo User_Controller::_show NOT IMPLEMENTED");
   }