added request object, added url function that does all the security checks, improved controller object

4 files changed, 237 insertions, 40 deletions

diff --git a/controllers/controller.php b/controllers/controller.php
index 032cb45ee..e1cf3e866 100644
--- a/controllers/controller.php
+++ b/controllers/controller.php
@@ -10,18 +10,38 @@
 *
 */
 
+
+/*
+
+Usage
+
+MyController extends Controller {
+    
+    public function __construct($request=null, $userLoggedInCheck=true, $csrfCheck=true){
+        super($request, $userLoggedInCheck, $csrfCheck);
+    }
+
+    public function myRoute(){
+    
+    }
+
+}
+
+
+*/
+
+
 namespace OCA\News;
 
 class Controller {
 
-    protected $userId;
     protected $trans;
 
-
     public function __construct(){
-        $this->userId = \OCP\USER::getUser();
         $this->trans = \OC_L10N::get('news');
         $this->safeParams = array();
+
+
     }
 
 
@@ -65,36 +85,121 @@ class Controller {
 
 
     /**
+     * Renders a renderer and sets the csrf check and logged in check to true
+     * @param Renderer $renderer: the render which should be used to render the page
+     */
+    protected function render(Renderer $renderer){
+        $renderer->bind('userId', $this->request->userId);
+        $renderer->render();
+        $this->csrfCheck = true;
+        $this->userLoggedInCheck = true;
+    }
+
+
+    /**
      * Binds variables to the template and prints it
-     * The following values are always assigned: userId, trans
+     * @param $templateName the name of the template
      * @param $arguments an array with arguments in $templateVar => $content
-     * @param $template the name of the template
      * @param $safeParams template parameters which should not be escaped
      * @param $fullPage if true, it will render a full page, otherwise only a part
      *                  defaults to true
      */
-    protected function render($template, $arguments=array(), $safeParams=array(),
-                              $fullPage=true){
+    protected function renderTemplate($templateName, $arguments=array(), 
+                                      $safeParams=array(), $fullPage=true){
+        $renderer = new TemplateRenderer($templateName, $fullPage);
+        $renderer->bindSafe($safeParams);
+        $this->render($renderer);
+    }
+
+    /**
+     * Binds variables to a JSON array and prints it
+     * @param $arguments an array with arguments in $key => $value
+     * @param $error: Empty by default. If set, a log message written and the
+     *                $error will be sent to the client
+     */
+    protected function renderJSON($arguments=array(), $error=""){
+        $renderer = new JSONRenderer($error);
+        $this->render($renderer);
+    }
+
+
+}
+
+
 
+
+
+
+interface Renderer {
+    public function render();
+    public function bind($params);
+}
+
+
+
+class TemplateRenderer implements Renderer {
+
+    private $safeParams = array();
+
+    public function __construct($name, $fullPage=true){
         if($fullPage){
-            $template = new \OCP\Template('news', $template, 'user');
+            $this->template = new \OCP\Template('news', $template, 'user');
         } else {
-            $template = new \OCP\Template('news', $template);
+            $this->template = new \OCP\Template('news', $template);
         }
+    }
+
+    public function bindSafe($params){
+        $this->safeParams = $params;
+    }
 
-        foreach($arguments as $key => $value){
-            if(array_key_exists($key, $safeParams)) {
-                $template->assign($key, $value, false);
+
+    public function bind($params){
+        foreach($params as $key => $value){
+            if(array_key_exists($key, $this->safeParams)) {
+                $this->template->assign($key, $value, false);
             } else {
-                $template->assign($key, $value);
+                $this->template->assign($key, $value);
             }
-
         }
+    }
 
-        $template->assign('userId', $this->userId);
-        $template->assign('trans', $this->trans);
-        $template->printPage();
+
+    public function render(){
+        $this->template->printPage();
     }
 
 
 }
+
+
+class JSONRenderer implements Renderer {
+
+    private $params;
+
+    public function __construct($error){
+        $this->error = $error;
+    }
+
+
+    public function bind($params){
+        $this->params = $params;
+    }
+
+
+    public function render(){
+        if($this->error === ""){
+            OCP\JSON::success($this->params);        
+        } else {
+            OCP\JSON::error(array(
+                                'data' => array('message' => $l->t('An error occured: ') . $error)
+                            )
+            );
+            OCP\Util::writeLog('news',$_SERVER['REQUEST_URI'] . 'Error: '. $error, OCP\Util::ERROR);
+            exit();
+        }
+        
+    }
+
+    
+}
\ No newline at end of file
diff --git a/controllers/news.controller.php b/controllers/news.controller.php
index be3cdbf08..3a77d5f7e 100644
--- a/controllers/news.controller.php
+++ b/controllers/news.controller.php
@@ -12,23 +12,27 @@
 
 namespace OCA\News;
 
+require_once \OC_App::getAppPath('news') . '/controllers/controller.php';
+
+
 class NewsController extends Controller {
 
     /**
      * Decides wether to show the feedpage or the firstrun page
      */
-    public function index(){
+    public function index($request){
+        echo "hi";
         $feedMapper = new FeedMapper($this->userId);
 
         if($feedMapper->feedCount() > 0){
-            $this->feedPage();
+            $this->feedPage($request);
         } else {
-            $this->firstRun();
+            $this->firstRun($request);
         }
     }
 
 
-    public function firstRun(){
+    public function firstRun($request){
         $this->addScript('news');
         $this->addScript('firstrun');
         $this->addStyle('firstrun');
@@ -36,7 +40,7 @@ class NewsController extends Controller {
     }
 
 
-    public function feedPage(){
+    public function feedPage($request){
         $this->addScript('main');
         $this->addScript('news');
         $this->addScript('menu');
@@ -51,10 +55,10 @@ class NewsController extends Controller {
         $itemMapper = new ItemMapper($this->userId);
 
         // if no feed id is passed as parameter, then show the last viewed feed on reload
-        $lastViewedFeedId = isset( $_GET['feedid'] ) ? $_GET['feedid'] : (int)$this->getUserValue('lastViewedFeed');
-        $lastViewedFeedType = isset( $_GET['feedid'] ) ? FeedType::FEED : (int)$this->getUserValue('lastViewedFeedType');
+        $lastViewedFeedId = isset( $request->get['feedid'] ) ? $request->get['feedid'] : (int)$this->getUserValue('lastViewedFeed');
+        $lastViewedFeedType = isset( $request->get['feedid'] ) ? FeedType::FEED : (int)$this->getUserValue('lastViewedFeedType');
         
-	$showAll = $this->getUserValue('showAll');
+        $showAll = $this->getUserValue('showAll');
 
         if( $lastViewedFeedId === null || $lastViewedFeedType === null) {
             $lastViewedFeedId = $feedMapper->mostRecent();
@@ -87,7 +91,7 @@ class NewsController extends Controller {
             'items' => $items
         );
 
-        $this->render('main', $params, array('items' => true));
+        $this->renderTemplate('main', $params, array('items' => true));
     }
 
 
@@ -98,7 +102,7 @@ class NewsController extends Controller {
      * @param $showAll if true, it will also include unread items
      * @return an array with all items
      */
-    public function getItems($feedType, $feedId, $showAll){
+    private function getItems($feedType, $feedId, $showAll){
         $items = array();
         $itemMapper = new ItemMapper($this->userId);
 
@@ -148,7 +152,7 @@ class NewsController extends Controller {
      * @param $feedId the id of the feed or folder
      * @return the unread count
      */
-    public function getItemUnreadCount($feedType, $feedId){
+    private function getItemUnreadCount($feedType, $feedId){
         $unreadCount = 0;
         $itemMapper = new ItemMapper($this->userId);
 
diff --git a/index.php b/index.php
index 1fadfc812..45acad4c6 100644
--- a/index.php
+++ b/index.php
@@ -11,18 +11,10 @@
 *
 */
 
-require_once OC_App::getAppPath('news') . '/controllers/controller.php';
-require_once OC_App::getAppPath('news') . '/controllers/news.controller.php';
+namespace OCA\News;
 
-OCP\User::checkLoggedIn();
-OCP\App::checkAppEnabled('news');
-OCP\App::setActiveNavigationEntry('news');
-
-$controller = new OCA\News\NewsController();
+require_once \OC_App::getAppPath('news') . '/lib/url.php';
+require_once \OC_App::getAppPath('news') . '/controllers/news.controller.php';
 
 // routes
-if(isset($_GET['jstest'])){
-	$controller->javascriptTests();
-} else {
-	$controller->index();
-}
\ No newline at end of file
+url(new NewsController(), 'index');
\ No newline at end of file
diff --git a/lib/url.php b/lib/url.php
new file mode 100644
index 000000000..73a7366bf
--- /dev/null
+++ b/lib/url.php
@@ -0,0 +1,96 @@
+<?php
+/**
+* ownCloud - News app
+*
+* @author Bernhard Posselt
+* Copyright (c) 2012 - Bernhard Posselt <nukeawhale@gmail.com>
+*
+* This file is licensed under the Affero General Public License version 3 or later.
+* See the COPYING-README file
+*
+*/
+
+namespace OCA\News;
+
+/**
+ * Used for mapping controllers and doing security checks
+ * @param Controller $controller: a new instance of the controller
+ * @param string $method: the name of the controller method that should be called
+ * @param bool $userLoggedIn: if false, there wont be a logged in check
+ * @param bool $csrfCheck: if false, there wont be a csrf check
+ */
+function url($controller, $method, $userLoggedInCheck=true, $csrfCheck=true){
+    
+	\OCP\App::setActiveNavigationEntry('news');
+
+    if(!\OC_App::isEnabled('news')){
+        \OCP\Util::writeLog('news', 'App news is not enabled!', \OCP\Util::ERROR);
+        exit();
+    }
+
+    if($userLoggedInCheck){
+        if(!\OC_User::isLoggedIn()){
+            \OCP\Util::writeLog('news', 'User is not logged in!', \OCP\Util::ERROR);
+            exit();
+        }
+    } 
+    echo "yodd";
+
+    if($csrfCheck){
+        if(!\OC_Util::isCallRegistered()){
+            \OCP\Util::writeLog('news', 'CSRF check failed', \OCP\Util::ERROR);
+            //exit();
+        }
+    }
+
+    $controller->$method(new Request());
+}
+
+
+
+/**
+ * This class is used to wrap $_GET and $_POST to improve testability of apps
+ */
+class Request {
+    public $get;
+    public $post;
+    public $user = null;
+
+    private $userId;
+
+    /**
+     * All parameters default to the built in $_GET, $_POST and \OCP\USER::getUser()
+     * @param array $get: an array with all get variables
+     * @param array $post: an array with all post variables
+     * @param string $userId: the id fo the user
+     */
+    public function __construct($get=null, $post=null, $userId=null){
+        if($get === null){
+            $get = $_GET;
+        }
+
+        if($post === null){
+            $post = $_POST;
+        }
+
+        if($userId === null){
+            $userId = \OCP\USER::getUser();
+        }
+
+        $this->get = $get;
+        $this->post = $post;
+        $this->userId = $userId;
+    }
+
+
+    /**
+     * This is used to do lazy fetching for user data
+     */
+    public function __get($name){
+        if($name === 'user' && $this->user === null){
+            // FIXME: get a new user instance
+        }
+        return $this->$name;
+    }
+
+}
\ No newline at end of file