Introduce the Updater class which

  * contains methods designed to be run once.
  * is able to upgrade the datastore or the configuration.
  * is based on methods names, stored in a text file with ';' separator (updates.txt).
  * begins with existing function 'mergeDeprecatedConfigFile()' (options.php).

8 files changed, 550 insertions, 86 deletions

diff --git a/application/Config.php b/application/Config.php
index 9af5a535..05a59452 100644
--- a/application/Config.php
+++ b/application/Config.php
@@ -174,33 +174,6 @@ function load_plugin_parameter_values($plugins, $config)
 }
 
 /**
- * Milestone 0.9 - shaarli/Shaarli#41: options.php is not supported anymore.
- * ==> if user is loggedIn, merge its content with config.php, then delete options.php.
- *
- * @param array $config     contains all configuration fields.
- * @param bool  $isLoggedIn true if user is logged in.
- *
- * @return void
- */
-function mergeDeprecatedConfig($config, $isLoggedIn)
-{
-    $config_file = $config['config']['CONFIG_FILE'];
-
-    if (is_file($config['config']['DATADIR'].'/options.php') && $isLoggedIn) {
-        include $config['config']['DATADIR'].'/options.php';
-
-        // Load GLOBALS into config
-        foreach ($GLOBALS as $key => $value) {
-            $config[$key] = $value;
-        }
-        $config['config']['CONFIG_FILE'] = $config_file;
-        writeConfig($config, $isLoggedIn);
-
-        unlink($config['config']['DATADIR'].'/options.php');
-    }
-}
-
-/**
  * Exception used if a mandatory field is missing in given configuration.
  */
 class MissingFieldConfigException extends Exception
diff --git a/application/LinkDB.php b/application/LinkDB.php
index 19ca6435..a95b3f36 100644
--- a/application/LinkDB.php
+++ b/application/LinkDB.php
@@ -260,14 +260,11 @@ You use the community supported version of the original Shaarli project, by Seba
             }
         }
 
-        // Keep the list of the mapping URLs-->linkdate up-to-date.
         $this->_urls = array();
-        foreach ($this->_links as $link) {
+        foreach ($this->_links as &$link) {
+            // Keep the list of the mapping URLs-->linkdate up-to-date.
             $this->_urls[$link['url']] = $link['linkdate'];
-        }
-
-        // Escape links data
-        foreach($this->_links as &$link) { 
+            // Sanitize data fields.
             sanitizeLink($link);
             // Do not use the redirector for internal links (Shaarli note URL starting with a '?').
             if (!empty($this->_redirector) && !startsWith($link['url'], '?')) {
@@ -381,6 +378,7 @@ You use the community supported version of the original Shaarli project, by Seba
         }
         $linkDays = array_keys($linkDays);
         sort($linkDays);
+
         return $linkDays;
     }
 }
diff --git a/application/LinkFilter.php b/application/LinkFilter.php
index b2e6530f..096d3b04 100644
--- a/application/LinkFilter.php
+++ b/application/LinkFilter.php
@@ -260,7 +260,6 @@ class LinkFilter
      * Convert a list of tags (str) to an array. Also
      * - handle case sensitivity.
      * - accepts spaces commas as separator.
-     * - remove private tags for loggedout users.
      *
      * @param string $tags          string containing a list of tags.
      * @param bool   $casesensitive will convert everything to lowercase if false.
diff --git a/application/Updater.php b/application/Updater.php
new file mode 100644
index 00000000..20ae0c4d
--- /dev/null
+++ b/application/Updater.php
@@ -0,0 +1,228 @@
+<?php
+
+/**
+ * Class Updater.
+ * Used to update stuff when a new Shaarli's version is reached.
+ * Update methods are ran only once, and the stored in a JSON file.
+ */
+class Updater
+{
+    /**
+     * @var array Updates which are already done.
+     */
+    protected $doneUpdates;
+
+    /**
+     * @var array Shaarli's configuration array.
+     */
+    protected $config;
+
+    /**
+     * @var LinkDB instance.
+     */
+    protected $linkDB;
+
+    /**
+     * @var bool True if the user is logged in, false otherwise.
+     */
+    protected $isLoggedIn;
+
+    /**
+     * @var ReflectionMethod[] List of current class methods.
+     */
+    protected $methods;
+
+    /**
+     * Object constructor.
+     *
+     * @param array   $doneUpdates Updates which are already done.
+     * @param array   $config      Shaarli's configuration array.
+     * @param LinkDB  $linkDB      LinkDB instance.
+     * @param boolean $isLoggedIn  True if the user is logged in.
+     */
+    public function __construct($doneUpdates, $config, $linkDB, $isLoggedIn)
+    {
+        $this->doneUpdates = $doneUpdates;
+        $this->config = $config;
+        $this->linkDB = $linkDB;
+        $this->isLoggedIn = $isLoggedIn;
+
+        // Retrieve all update methods.
+        $class = new ReflectionClass($this);
+        $this->methods = $class->getMethods();
+    }
+
+    /**
+     * Run all new updates.
+     * Update methods have to start with 'updateMethod' and return true (on success).
+     *
+     * @return array An array containing ran updates.
+     *
+     * @throws UpdaterException If something went wrong.
+     */
+    public function update()
+    {
+        $updatesRan = array();
+
+        // If the user isn't logged in, exit without updating.
+        if ($this->isLoggedIn !== true) {
+            return $updatesRan;
+        }
+
+        if ($this->methods == null) {
+            throw new UpdaterException('Couldn\'t retrieve Updater class methods.');
+        }
+
+        foreach ($this->methods as $method) {
+            // Not an update method or already done, pass.
+            if (! startsWith($method->getName(), 'updateMethod')
+                || in_array($method->getName(), $this->doneUpdates)
+            ) {
+                continue;
+            }
+
+            try {
+                $method->setAccessible(true);
+                $res = $method->invoke($this);
+                // Update method must return true to be considered processed.
+                if ($res === true) {
+                    $updatesRan[] = $method->getName();
+                }
+            } catch (Exception $e) {
+                throw new UpdaterException($method, $e);
+            }
+        }
+
+        $this->doneUpdates = array_merge($this->doneUpdates, $updatesRan);
+
+        return $updatesRan;
+    }
+
+    /**
+     * @return array Updates methods already processed.
+     */
+    public function getDoneUpdates()
+    {
+        return $this->doneUpdates;
+    }
+
+    /**
+     * Move deprecated options.php to config.php.
+     *
+     * Milestone 0.9 (old versioning) - shaarli/Shaarli#41:
+     *    options.php is not supported anymore.
+     */
+    public function updateMethodMergeDeprecatedConfigFile()
+    {
+        $config_file = $this->config['config']['CONFIG_FILE'];
+
+        if (is_file($this->config['config']['DATADIR'].'/options.php')) {
+            include $this->config['config']['DATADIR'].'/options.php';
+
+            // Load GLOBALS into config
+            foreach ($GLOBALS as $key => $value) {
+                $this->config[$key] = $value;
+            }
+            $this->config['config']['CONFIG_FILE'] = $config_file;
+            writeConfig($this->config, $this->isLoggedIn);
+
+            unlink($this->config['config']['DATADIR'].'/options.php');
+        }
+
+        return true;
+    }
+}
+
+/**
+ * Class UpdaterException.
+ */
+class UpdaterException extends Exception
+{
+    /**
+     * @var string Method where the error occurred.
+     */
+    protected $method;
+
+    /**
+     * @var Exception The parent exception.
+     */
+    protected $previous;
+
+    /**
+     * Constructor.
+     *
+     * @param string         $message  Force the error message if set.
+     * @param string         $method   Method where the error occurred.
+     * @param Exception|bool $previous Parent exception.
+     */
+    public function __construct($message = '', $method = '', $previous = false)
+    {
+        $this->method = $method;
+        $this->previous = $previous;
+        $this->message = $this->buildMessage($message);
+    }
+
+    /**
+     * Build the exception error message.
+     *
+     * @param string $message Optional given error message.
+     *
+     * @return string The built error message.
+     */
+    private function buildMessage($message)
+    {
+        $out = '';
+        if (! empty($message)) {
+            $out .= $message . PHP_EOL;
+        }
+
+        if (! empty($this->method)) {
+            $out .= 'An error occurred while running the update '. $this->method . PHP_EOL;
+        }
+
+        if (! empty($this->previous)) {
+            $out .= '  '. $this->previous->getMessage();
+        }
+
+        return $out;
+    }
+}
+
+
+/**
+ * Read the updates file, and return already done updates.
+ *
+ * @param string $updatesFilepath Updates file path.
+ *
+ * @return array Already done update methods.
+ */
+function read_updates_file($updatesFilepath)
+{
+    if (! empty($updatesFilepath) && is_file($updatesFilepath)) {
+        $content = file_get_contents($updatesFilepath);
+        if (! empty($content)) {
+            return explode(';', $content);
+        }
+    }
+    return array();
+}
+
+/**
+ * Write updates file.
+ *
+ * @param string $updatesFilepath Updates file path.
+ * @param array  $updates         Updates array to write.
+ *
+ * @throws Exception Couldn't write version number.
+ */
+function write_updates_file($updatesFilepath, $updates)
+{
+    if (empty($updatesFilepath)) {
+        throw new Exception('Updates file path is not set, can\'t write updates.');
+    }
+
+    $res = file_put_contents($updatesFilepath, implode(';', $updates));
+    if ($res === false) {
+        throw new Exception('Unable to write updates in '. $updatesFilepath . '.');
+    }
+}
diff --git a/index.php b/index.php
index 31dcbf0f..a5d8b7bd 100644
--- a/index.php
+++ b/index.php
@@ -44,6 +44,9 @@ $GLOBALS['config']['DATASTORE'] = $GLOBALS['config']['DATADIR'].'/datastore.php'
 // Banned IPs
 $GLOBALS['config']['IPBANS_FILENAME'] = $GLOBALS['config']['DATADIR'].'/ipbans.php';
 
+// Processed updates file.
+$GLOBALS['config']['UPDATES_FILE'] = $GLOBALS['config']['DATADIR'].'/updates.txt';
+
 // Access log
 $GLOBALS['config']['LOG_FILE'] = $GLOBALS['config']['DATADIR'].'/log.txt';
 
@@ -61,7 +64,6 @@ $GLOBALS['config']['CACHEDIR'] = 'cache';
 // Atom & RSS feed cache directory
 $GLOBALS['config']['PAGECACHE'] = 'pagecache';
 
-
 /*
  * Global configuration
  */
@@ -159,6 +161,7 @@ require_once 'application/Utils.php';
 require_once 'application/Config.php';
 require_once 'application/PluginManager.php';
 require_once 'application/Router.php';
+require_once 'application/Updater.php';
 
 // Ensure the PHP version is supported
 try {
@@ -1110,6 +1113,25 @@ function renderPage()
         $GLOBALS['redirector']
     );
 
+    $updater = new Updater(
+        read_updates_file($GLOBALS['config']['UPDATES_FILE']),
+        $GLOBALS,
+        $LINKSDB,
+        isLoggedIn()
+    );
+    try {
+        $newUpdates = $updater->update();
+        if (! empty($newUpdates)) {
+            write_updates_file(
+                $GLOBALS['config']['UPDATES_FILE'],
+                $updater->getDoneUpdates()
+            );
+        }
+    }
+    catch(Exception $e) {
+        die($e->getMessage());
+    }
+
     $PAGE = new pageBuilder;
 
     // Determine which page will be rendered.
@@ -2515,15 +2537,6 @@ function resizeImage($filepath)
     return true;
 }
 
-try {
-    mergeDeprecatedConfig($GLOBALS, isLoggedIn());
-} catch(Exception $e) {
-    error_log(
-        'ERROR while merging deprecated options.php file.' . PHP_EOL .
-        $e->getMessage()
-    );
-}
-
 if (isset($_SERVER["QUERY_STRING"]) && startswith($_SERVER["QUERY_STRING"],'do=genthumbnail')) { genThumbnail(); exit; }  // Thumbnail generation/cache does not need the link database.
 if (isset($_SERVER["QUERY_STRING"]) && startswith($_SERVER["QUERY_STRING"],'do=rss')) { showRSS(); exit; }
 if (isset($_SERVER["QUERY_STRING"]) && startswith($_SERVER["QUERY_STRING"],'do=atom')) { showATOM(); exit; }
diff --git a/tests/ConfigTest.php b/tests/ConfigTest.php
index 492ddd3b..7200aae6 100644
--- a/tests/ConfigTest.php
+++ b/tests/ConfigTest.php
@@ -134,48 +134,6 @@ class ConfigTest extends PHPUnit_Framework_TestCase
     }
 
     /**
-     * Test mergeDeprecatedConfig while being logged in:
-     *      1. init a config file.
-     *      2. init a options.php file with update value.
-     *      3. merge.
-     *      4. check updated value in config file.
-     */
-    public function testMergeDeprecatedConfig()
-    {
-        // init
-        writeConfig(self::$configFields, true);
-        $configCopy = self::$configFields;
-        $invert = !$configCopy['privateLinkByDefault'];
-        $configCopy['privateLinkByDefault'] = $invert;
-
-        // Use writeConfig to create a options.php
-        $configCopy['config']['CONFIG_FILE'] = 'tests/options.php';
-        writeConfig($configCopy, true);
-
-        $this->assertTrue(is_file($configCopy['config']['CONFIG_FILE']));
-
-        // merge configs
-        mergeDeprecatedConfig(self::$configFields, true);
-
-        // make sure updated field is changed
-        include self::$configFields['config']['CONFIG_FILE'];
-        $this->assertEquals($invert, $GLOBALS['privateLinkByDefault']);
-        $this->assertFalse(is_file($configCopy['config']['CONFIG_FILE']));
-    }
-
-    /**
-     * Test mergeDeprecatedConfig while being logged in without options file.
-     */
-    public function testMergeDeprecatedConfigNoFile()
-    {
-        writeConfig(self::$configFields, true);
-        mergeDeprecatedConfig(self::$configFields, true);
-
-        include self::$configFields['config']['CONFIG_FILE'];
-        $this->assertEquals(self::$configFields['login'], $GLOBALS['login']);
-    }
-
-    /**
      * Test save_plugin_config with valid data.
      *
      * @throws PluginConfigOrderException
diff --git a/tests/Updater/DummyUpdater.php b/tests/Updater/DummyUpdater.php
new file mode 100644
index 00000000..e9ef2aaa
--- /dev/null
+++ b/tests/Updater/DummyUpdater.php
@@ -0,0 +1,68 @@
+<?php
+
+require_once 'application/Updater.php';
+
+/**
+ * Class DummyUpdater.
+ * Extends Updater to add update method designed for unit tests.
+ */
+class DummyUpdater extends Updater
+{
+    /**
+     * Object constructor.
+     *
+     * @param array   $doneUpdates Updates which are already done.
+     * @param array   $config      Shaarli's configuration array.
+     * @param LinkDB  $linkDB      LinkDB instance.
+     * @param boolean $isLoggedIn  True if the user is logged in.
+     */
+    public function __construct($doneUpdates, $config, $linkDB, $isLoggedIn)
+    {
+        parent::__construct($doneUpdates, $config, $linkDB, $isLoggedIn);
+
+        // Retrieve all update methods.
+        // For unit test, only retrieve final methods,
+        $class = new ReflectionClass($this);
+        $this->methods = $class->getMethods(ReflectionMethod::IS_FINAL);
+    }
+
+    /**
+     * Update method 1.
+     *
+     * @return bool true.
+     */
+    private final function updateMethodDummy1()
+    {
+        return true;
+    }
+
+    /**
+     * Update method 2.
+     *
+     * @return bool true.
+     */
+    private final function updateMethodDummy2()
+    {
+        return true;
+    }
+
+    /**
+     * Update method 3.
+     *
+     * @return bool true.
+     */
+    private final function updateMethodDummy3()
+    {
+        return true;
+    }
+
+    /**
+     * Update method 4, raise an exception.
+     *
+     * @throws Exception error.
+     */
+    private final function updateMethodException()
+    {
+        throw new Exception('whatever');
+    }
+}
diff --git a/tests/Updater/UpdaterTest.php b/tests/Updater/UpdaterTest.php
new file mode 100644
index 00000000..63ed5e03
--- /dev/null
+++ b/tests/Updater/UpdaterTest.php
@@ -0,0 +1,227 @@
+<?php
+
+require_once 'tests/Updater/DummyUpdater.php';
+
+/**
+ * Class UpdaterTest.
+ * Runs unit tests against the Updater class.
+ */
+class UpdaterTest extends PHPUnit_Framework_TestCase
+{
+    /**
+     * @var array Configuration input set.
+     */
+    private static $configFields;
+
+    /**
+     * Executed before each test.
+     */
+    public function setUp()
+    {
+        self::$configFields = array(
+            'login' => 'login',
+            'hash' => 'hash',
+            'salt' => 'salt',
+            'timezone' => 'Europe/Paris',
+            'title' => 'title',
+            'titleLink' => 'titleLink',
+            'redirector' => '',
+            'disablesessionprotection' => false,
+            'privateLinkByDefault' => false,
+            'config' => array(
+                'CONFIG_FILE' => 'tests/Updater/config.php',
+                'DATADIR' => 'tests/Updater',
+                'config1' => 'config1data',
+                'config2' => 'config2data',
+            )
+        );
+    }
+
+    /**
+     * Executed after each test.
+     *
+     * @return void
+     */
+    public function tearDown()
+    {
+        if (is_file(self::$configFields['config']['CONFIG_FILE'])) {
+            unlink(self::$configFields['config']['CONFIG_FILE']);
+        }
+
+        if (is_file(self::$configFields['config']['DATADIR'] . '/options.php')) {
+            unlink(self::$configFields['config']['DATADIR'] . '/options.php');
+        }
+
+        if (is_file(self::$configFields['config']['DATADIR'] . '/updates.json')) {
+            unlink(self::$configFields['config']['DATADIR'] . '/updates.json');
+        }
+    }
+
+    /**
+     * Test read_updates_file with an empty/missing file.
+     */
+    public function testReadEmptyUpdatesFile()
+    {
+        $this->assertEquals(array(), read_updates_file(''));
+        $updatesFile = self::$configFields['config']['DATADIR'] . '/updates.json';
+        touch($updatesFile);
+        $this->assertEquals(array(), read_updates_file($updatesFile));
+    }
+
+    /**
+     * Test read/write updates file.
+     */
+    public function testReadWriteUpdatesFile()
+    {
+        $updatesFile = self::$configFields['config']['DATADIR'] . '/updates.json';
+        $updatesMethods = array('m1', 'm2', 'm3');
+
+        write_updates_file($updatesFile, $updatesMethods);
+        $readMethods = read_updates_file($updatesFile);
+        $this->assertEquals($readMethods, $updatesMethods);
+
+        // Update
+        $updatesMethods[] = 'm4';
+        write_updates_file($updatesFile, $updatesMethods);
+        $readMethods = read_updates_file($updatesFile);
+        $this->assertEquals($readMethods, $updatesMethods);
+    }
+
+    /**
+     * Test errors in write_updates_file(): empty updates file.
+     *
+     * @expectedException              Exception
+     * @expectedExceptionMessageRegExp /Updates file path is not set(.*)/
+     */
+    public function testWriteEmptyUpdatesFile()
+    {
+        write_updates_file('', array('test'));
+    }
+
+    /**
+     * Test errors in write_updates_file(): not writable updates file.
+     *
+     * @expectedException              Exception
+     * @expectedExceptionMessageRegExp /Unable to write(.*)/
+     */
+    public function testWriteUpdatesFileNotWritable()
+    {
+        $updatesFile = self::$configFields['config']['DATADIR'] . '/updates.json';
+        touch($updatesFile);
+        chmod($updatesFile, 0444);
+        @write_updates_file($updatesFile, array('test'));
+    }
+
+    /**
+     * Test the update() method, with no update to run.
+     *   1. Everything already run.
+     *   2. User is logged out.
+     */
+    public function testNoUpdates()
+    {
+        $updates = array(
+            'updateMethodDummy1',
+            'updateMethodDummy2',
+            'updateMethodDummy3',
+            'updateMethodException',
+        );
+        $updater = new DummyUpdater($updates, array(), array(), true);
+        $this->assertEquals(array(), $updater->update());
+
+        $updater = new DummyUpdater(array(), array(), array(), false);
+        $this->assertEquals(array(), $updater->update());
+    }
+
+    /**
+     * Test the update() method, with all updates to run (except the failing one).
+     */
+    public function testUpdatesFirstTime()
+    {
+        $updates = array('updateMethodException',);
+        $expectedUpdates = array(
+            'updateMethodDummy1',
+            'updateMethodDummy2',
+            'updateMethodDummy3',
+        );
+        $updater = new DummyUpdater($updates, array(), array(), true);
+        $this->assertEquals($expectedUpdates, $updater->update());
+    }
+
+    /**
+     * Test the update() method, only one update to run.
+     */
+    public function testOneUpdate()
+    {
+        $updates = array(
+            'updateMethodDummy1',
+            'updateMethodDummy3',
+            'updateMethodException',
+        );
+        $expectedUpdate = array('updateMethodDummy2');
+
+        $updater = new DummyUpdater($updates, array(), array(), true);
+        $this->assertEquals($expectedUpdate, $updater->update());
+    }
+
+    /**
+     * Test Update failed.
+     *
+     * @expectedException UpdaterException
+     */
+    public function testUpdateFailed()
+    {
+        $updates = array(
+            'updateMethodDummy1',
+            'updateMethodDummy2',
+            'updateMethodDummy3',
+        );
+
+        $updater = new DummyUpdater($updates, array(), array(), true);
+        $updater->update();
+    }
+
+    /**
+     * Test update mergeDeprecatedConfig:
+     *      1. init a config file.
+     *      2. init a options.php file with update value.
+     *      3. merge.
+     *      4. check updated value in config file.
+     */
+    public function testUpdateMergeDeprecatedConfig()
+    {
+        // init
+        writeConfig(self::$configFields, true);
+        $configCopy = self::$configFields;
+        $invert = !$configCopy['privateLinkByDefault'];
+        $configCopy['privateLinkByDefault'] = $invert;
+
+        // Use writeConfig to create a options.php
+        $configCopy['config']['CONFIG_FILE'] = 'tests/Updater/options.php';
+        writeConfig($configCopy, true);
+
+        $this->assertTrue(is_file($configCopy['config']['CONFIG_FILE']));
+
+        // merge configs
+        $updater = new Updater(array(), self::$configFields, array(), true);
+        $updater->updateMethodMergeDeprecatedConfigFile();
+
+        // make sure updated field is changed
+        include self::$configFields['config']['CONFIG_FILE'];
+        $this->assertEquals($invert, $GLOBALS['privateLinkByDefault']);
+        $this->assertFalse(is_file($configCopy['config']['CONFIG_FILE']));
+    }
+
+    /**
+     * Test mergeDeprecatedConfig in without options file.
+     */
+    public function testMergeDeprecatedConfigNoFile()
+    {
+        writeConfig(self::$configFields, true);
+
+        $updater = new Updater(array(), self::$configFields, array(), true);
+        $updater->updateMethodMergeDeprecatedConfigFile();
+
+        include self::$configFields['config']['CONFIG_FILE'];
+        $this->assertEquals(self::$configFields['login'], $GLOBALS['login']);
+    }
+}