Discussion: shaarli controllers

application/PageBuilder.php

@@ -1,5 +1,7 @@
 <?php
 
+require_once 'inc/rain.tpl.class.php';
+
 /**
  * This class is in charge of building the final page.
  * (This is basically a wrapper around RainTPL which pre-fills some fields.)

controllers/Controller.php

@@ -0,0 +1,141 @@
+<?php
+
+/**
+ * Class Controller.
+ * 
+ * This abstract class is design to handle page rendering.
+ * All request to Shaarli must pass through its own controller.
+ */
+abstract class Controller
+{
+    /**
+     * @var PageBuilder
+     */
+    protected $tpl;
+
+    /**
+     * @var ConfigManager
+     */
+    protected $conf;
+
+    /**
+     * @var PluginManager
+     */
+    protected $pluginManager;
+
+    /**
+     * @var LinkDB
+     */
+    protected $linkDB;
+
+    /**
+     * @var array $_SERVER
+     */
+    protected $server;
+
+    /**
+     * @var array
+     */
+    protected $get;
+
+    /**
+     * @var array
+     */
+    protected $post;
+
+    /**
+     * Controller constructor.
+     * @param PageBuilder $tpl
+     * @param ConfigManager $conf
+     * @param PluginManager $pluginManager
+     * @param LinkDB $linkDB
+     * @param array $server
+     * @param array $get
+     * @param array $post
+     */
+    public function __construct(
+        PageBuilder $tpl,
+        ConfigManager $conf,
+        PluginManager $pluginManager,
+        LinkDB $linkDB,
+        array $server,
+        array $get,
+        array $post
+    ) {
+        $this->tpl = $tpl;
+        $this->conf = $conf;
+        $this->pluginManager = $pluginManager;
+        $this->linkDB = $linkDB;
+        $this->server = $server;
+        $this->get = $get;
+        $this->post = $post;
+    }
+
+    /**
+     * Handle headers redirection.
+     * 
+     * @return boolean true if a redirection is necessary, false otherwise.
+     */
+    public abstract function redirect();
+
+    /**
+     * Render the view through the template engine.
+     */
+    public abstract function render();
+
+    /**
+     * @param PageBuilder $tpl
+     */
+    public function setTpl($tpl)
+    {
+        $this->tpl = $tpl;
+    }
+
+    /**
+     * @param ConfigManager $conf
+     */
+    public function setConf($conf)
+    {
+        $this->conf = $conf;
+    }
+
+    /**
+     * @param PluginManager $pluginManager
+     */
+    public function setPluginManager($pluginManager)
+    {
+        $this->pluginManager = $pluginManager;
+    }
+
+    /**
+     * @param LinkDB $linkDB
+     */
+    public function setLinkDB($linkDB)
+    {
+        $this->linkDB = $linkDB;
+    }
+
+    /**
+     * @param array $server
+     */
+    public function setServer($server)
+    {
+        $this->server = $server;
+    }
+
+    /**
+     * @param array $get
+     */
+    public function setGet($get)
+    {
+        $this->get = $get;
+    }
+
+    /**
+     * @param array $post
+     */
+    public function setPost($post)
+    {
+        $this->post = $post;
+    }
+}

controllers/LoginController.php

@@ -0,0 +1,37 @@
+<?php
+
+require_once 'Controller.php';
+
+/**
+ * Class LoginController
+ *
+ * Handle login page.
+ */
+class LoginController extends Controller
+{
+    /**
+     * {@inheritdoc}
+     */
+    public function redirect()
+    {
+        // No need to login for open Shaarli
+        if ($this->conf->get('security.open_shaarli')) {
+            header('Location: ?');
+            return true;
+        }
+        return false;
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function render()
+    {
+        if (isset($this->get['username'])) {
+            $this->tpl->assign('username', escape($this->get['username']));
+        }
+        $referer = isset($this->server['HTTP_REFERER']) ? escape($this->server['HTTP_REFERER']) : '';
+        $this->tpl->assign('returnurl', $referer);
+        $this->tpl->renderPage('loginform');
+    }
+}

index.php

@@ -780,6 +780,15 @@ function renderPage($conf, $pluginManager)
         $PAGE->assign('plugins_' . $name, $plugin_data);
     }
 
+    $viewClass = ucfirst($targetPage) . 'Controller';
+    /** @var Controller $view */
+    $view = new $viewClass($PAGE, $conf, $pluginManager, $LINKSDB, $_SERVER, $_GET, $_POST);
+    if ($view->redirect()) {
+        exit;
+    }
+    $view->render();
+    exit;
+
     // -------- Display login form.
     if ($targetPage == Router::$PAGE_LOGIN)
     {

tests/views/ControllerTest.php

@@ -0,0 +1,91 @@
+<?php
+
+require_once 'application/config/ConfigManager.php';
+require_once 'application/PageBuilder.php';
+require_once 'application/PluginManager.php';
+require_once 'tests/utils/ReferenceLinkDB.php';
+require_once 'application/LinkDB.php';
+
+/**
+ * Class ControllerTest
+ *
+ * Parent class for controller test classes.
+ * It instantiate controllers parameters.
+ */
+abstract class ControllerTest extends PHPUnit_Framework_TestCase
+{
+    /**
+     * @var string path to test datastore.
+     */
+    public static $testDatastore = 'sandbox/datastore.php';
+
+    /**
+     * @var Controller
+     */
+    protected $controller;
+
+    /**
+     * @var PageBuilder
+     */
+    protected $pageBuilder;
+
+    /**
+     * @var ConfigManager
+     */
+    protected $conf;
+
+    /**
+     * @var PluginManager
+     */
+    protected $pluginManager;
+
+    /**
+     * @var LinkDB
+     */
+    protected $linkDB;
+
+    /**
+     * Called before every test.
+     * Instantiate default objects used in the controller.
+     */
+    public function setUp()
+    {
+        $this->conf = new ConfigManager('tests/utils/config/configJson');
+        $this->pageBuilder = new PageBuilder($this->conf);
+        $this->pluginManager = new PluginManager($this->conf);
+
+        $refDB = new ReferenceLinkDB();
+        $refDB->write(self::$testDatastore);
+
+        $this->linkDB = new LinkDB(self::$testDatastore, true, false);
+    }
+
+    /**
+     * Called after every test.
+     * Delete the test datastore.
+     */
+    public function tearDown()
+    {
+        if (file_exists(self::$testDatastore)) {
+            unlink(self::$testDatastore);
+        }
+    }
+
+    /**
+     * Test if a header has been set.
+     *
+     * @param string   $expected Expected header regex, without delimiters.
+     * @param array    $headers  Headers set.
+     *
+     * @return bool true if the header has been set, false otherwise.
+     */
+    public function isHeaderSetRegex($expected, $headers)
+    {
+        foreach ($headers as $header) {
+            if (preg_match('/'. $expected .'/', $header)) {
+                return true;
+            }
+        }
+        return false;
+    }
+}

tests/views/LoginControllerTest.php

@@ -0,0 +1,70 @@
+<?php
+
+require_once 'controllers/LoginController.php';
+
+/**
+ * Class LoginControllerTest
+ *
+ * Test the login view page
+ */
+class LoginControllerTest extends ControllerTest
+{
+    public function setUp()
+    {
+        parent::setUp();
+        $this->controller = new LoginController(
+            $this->pageBuilder,
+            $this->conf,
+            $this->pluginManager,
+            $this->linkDB,
+            array(),
+            array(),
+            array()
+        );
+    }
+
+    /**
+     * Test login redirection: should only redirect if open shaarli is enabled.
+     *
+     * @runInSeparateProcess
+     */
+    public function testLoginRedirect() {
+        $this->assertFalse($this->controller->redirect());
+        $headers = xdebug_get_headers();
+        $this->assertFalse($this->isHeaderSetRegex('Location:.*', $headers));
+
+        $this->conf->set('security.open_shaarli', true);
+        $this->assertTrue($this->controller->redirect());
+        $headers = xdebug_get_headers();
+        $this->assertTrue($this->isHeaderSetRegex('Location: \?', $headers));
+    }
+
+    /**
+     * Test login rendering without any data set.
+     */
+    public function testLoginRenderWithoutLogin() {
+        $this->markTestIncomplete('This test is not doable now due to ban function relying on $_SERVER.');
+        $this->expectOutputRegex('<input type="text" name="login"');
+        $this->expectOutputRegex('<input type="password" name="password"');
+        $this->expectOutputRegex('<input type="checkbox" name="longlastingsession"');
+        $this->controller->render();
+    }
+
+    /**
+     * Test login rendering with a username set in $_GET.
+     */
+    public function testLoginRenderWithLogin() {
+        $this->markTestIncomplete('This test is not doable now due to ban function relying on $_SERVER.');
+        $username = 'John Doe';
+        $this->controller->setGet(array('username' => $username));
+        $this->expectOutputRegex('<input type="text" name="login" value="'. $username .'"');
+        $this->controller->render();
+    }
+
+    /**
+     * Test login rendering with a username set in $_GET.
+     */
+    public function testLoginRenderBanned() {
+        $this->markTestIncomplete('This test is not doable now due to ban function relying on $_SERVER.');
+    }
+}