application: refactor version checks, move to ApplicationUtils

Relates to #372

Modifications:
 - move checkUpdate() to ApplicationUtils
 - reduce file I/O operations during version checks
 - apply coding conventions
 - add test coverage

Tools:
 - create a sandbox directory for tests

Signed-off-by: VirtualTam <virtualtam@flibidi.net>

.gitignore

@@ -19,9 +19,8 @@ composer.lock
 # Ignore development and test resources
 coverage
 doxygen
-tests/datastore.php
-tests/dummycache/
+sandbox
 phpmd.html
 
 # Ignore user plugin configuration
-plugins/*/config.php
+plugins/*/config.php

Makefile

@@ -110,6 +110,7 @@ test:
 	@echo "-------"
 	@echo "PHPUNIT"
 	@echo "-------"
+	@mkdir -p sandbox
 	@$(BIN)/phpunit tests
 
 ##
@@ -119,6 +120,7 @@ test:
 ### remove all unversioned files
 clean:
 	@git clean -df
+	@rm -rf sandbox
 
 ### generate Doxygen documentation
 doxygen: clean

application/ApplicationUtils.php

@@ -4,6 +4,98 @@
  */
 class ApplicationUtils
 {
+    private static $GIT_URL = 'https://raw.githubusercontent.com/shaarli/Shaarli';
+    private static $GIT_BRANCH = 'master';
+    private static $VERSION_FILE = 'shaarli_version.php';
+    private static $VERSION_START_TAG = '<?php /* ';
+    private static $VERSION_END_TAG = ' */ ?>';
+
+    /**
+     * Gets the latest version code from the Git repository
+     *
+     * The code is read from the raw content of the version file on the Git server.
+     *
+     * @return mixed the version code from the repository if available, else 'false'
+     */
+    public static function getLatestGitVersionCode($url, $timeout=2)
+    {
+        list($headers, $data) = get_http_url($url, $timeout);
+
+        if (strpos($headers[0], '200 OK') === false) {
+            error_log('Failed to retrieve ' . $url);
+            return false;
+        }
+
+        return str_replace(
+            array(self::$VERSION_START_TAG, self::$VERSION_END_TAG, PHP_EOL),
+            array('', '', ''),
+            $data
+        );
+    }
+
+    /**
+     * Checks if a new Shaarli version has been published on the Git repository
+     *
+     * Updates checks are run periodically, according to the following criteria:
+     * - the update checks are enabled (install, global config);
+     * - the user is logged in (or this is an open instance);
+     * - the last check is older than a given interval;
+     * - the check is non-blocking if the HTTPS connection to Git fails;
+     * - in case of failure, the update file's modification date is updated,
+     *   to avoid intempestive connection attempts.
+     *
+     * @param string $currentVersion the current version code
+     * @param string $updateFile     the file where to store the latest version code
+     * @param int    $checkInterval  the minimum interval between update checks (in seconds
+     * @param bool   $enableCheck    whether to check for new versions
+     * @param bool   $isLoggedIn     whether the user is logged in
+     *
+     * @return mixed the new version code if available and greater, else 'false'
+     */
+    public static function checkUpdate(
+        $currentVersion, $updateFile, $checkInterval, $enableCheck, $isLoggedIn)
+    {
+        if (! $isLoggedIn) {
+            // Do not check versions for visitors
+            return false;
+        }
+
+        if (empty($enableCheck)) {
+            // Do not check if the user doesn't want to
+            return false;
+        }
+
+        if (is_file($updateFile) && (filemtime($updateFile) > time() - $checkInterval)) {
+            // Shaarli has checked for updates recently - skip HTTP query
+            $latestKnownVersion = file_get_contents($updateFile);
+
+            if (version_compare($latestKnownVersion, $currentVersion) == 1) {
+                return $latestKnownVersion;
+            }
+            return false;
+        }
+
+        // Late Static Binding allows overriding within tests
+        // See http://php.net/manual/en/language.oop5.late-static-bindings.php
+        $latestVersion = static::getLatestGitVersionCode(
+            self::$GIT_URL . '/' . self::$GIT_BRANCH . '/' . self::$VERSION_FILE
+        );
+
+        if (! $latestVersion) {
+            // Only update the file's modification date
+            file_put_contents($updateFile, $currentVersion);
+            return false;
+        }
+
+        // Update the file's content and modification date
+        file_put_contents($updateFile, $latestVersion);
+
+        if (version_compare($latestVersion, $currentVersion) == 1) {
+            return $latestVersion;
+        }
+
+        return false;
+    }
 
     /**
      * Checks the PHP version to ensure Shaarli can run

index.php

@@ -305,32 +305,6 @@ function setup_login_state() {
 }
 $userIsLoggedIn = setup_login_state();
 
-// Checks if an update is available for Shaarli.
-// (at most once a day, and only for registered user.)
-// Output: '' = no new version.
-//         other= the available version.
-function checkUpdate()
-{
-    if (!isLoggedIn()) return ''; // Do not check versions for visitors.
-    if (empty($GLOBALS['config']['ENABLE_UPDATECHECK'])) return ''; // Do not check if the user doesn't want to.
-
-    // Get latest version number at most once a day.
-    if (!is_file($GLOBALS['config']['UPDATECHECK_FILENAME']) || (filemtime($GLOBALS['config']['UPDATECHECK_FILENAME'])<time()-($GLOBALS['config']['UPDATECHECK_INTERVAL'])))
-    {
-        $version = shaarli_version;
-        list($headers, $data) = get_http_url('https://raw.githubusercontent.com/shaarli/Shaarli/master/shaarli_version.php', 2);
-        if (strpos($headers[0], '200 OK') !== false) {
-            $version = str_replace(' */ ?>', '', str_replace('<?php /* ', '', $data));
-        }
-        // If failed, never mind. We don't want to bother the user with that.
-        file_put_contents($GLOBALS['config']['UPDATECHECK_FILENAME'],$version); // touch file date
-    }
-    // Compare versions:
-    $newestversion=file_get_contents($GLOBALS['config']['UPDATECHECK_FILENAME']);
-    if (version_compare($newestversion,shaarli_version)==1) return $newestversion;
-    return '';
-}
-
 
 // -----------------------------------------------------------------------------------------------
 // Log to text file
@@ -657,7 +631,18 @@ class pageBuilder
     private function initialize()
     {
         $this->tpl = new RainTPL;
-        $this->tpl->assign('newversion', escape(checkUpdate()));
+        $this->tpl->assign(
+            'newversion',
+            escape(
+                ApplicationUtils::checkUpdate(
+                    shaarli_version,
+                    $GLOBALS['config']['UPDATECHECK_FILENAME'],
+                    $GLOBALS['config']['UPDATECHECK_INTERVAL'],
+                    $GLOBALS['config']['ENABLE_UPDATECHECK'],
+                    isLoggedIn()
+                )
+            )
+        );
         $this->tpl->assign('feedurl', escape(index_url($_SERVER)));
         $searchcrits = ''; // Search criteria
         if (!empty($_GET['searchtags'])) {

tests/ApplicationUtilsTest.php

@@ -5,12 +5,230 @@
 
 require_once 'application/ApplicationUtils.php';
 
+/**
+ * Fake ApplicationUtils class to avoid HTTP requests
+ */
+class FakeApplicationUtils extends ApplicationUtils
+{
+    public static $VERSION_CODE = '';
+
+    /**
+     * Toggle HTTP requests, allow overriding the version code
+     */
+    public static function getLatestGitVersionCode($url, $timeout=0)
+    {
+        return self::$VERSION_CODE;
+    }
+}
+
 
 /**
  * Unitary tests for Shaarli utilities
  */
 class ApplicationUtilsTest extends PHPUnit_Framework_TestCase
 {
+    protected static $testUpdateFile = 'sandbox/update.txt';
+    protected static $testVersion = '0.5.0';
+    protected static $versionPattern = '/^\d+\.\d+\.\d+$/';
+
+    /**
+     * Reset test data for each test
+     */
+    public function setUp()
+    {
+        FakeApplicationUtils::$VERSION_CODE = '';
+        if (file_exists(self::$testUpdateFile)) {
+            unlink(self::$testUpdateFile);
+        }
+    }
+
+    /**
+     * Retrieve the latest version code available on Git
+     *
+     * Expected format: Semantic Versioning - major.minor.patch
+     */
+    public function testGetLatestGitVersionCode()
+    {
+        $testTimeout = 10;
+
+        $this->assertEquals(
+            '0.5.4',
+            ApplicationUtils::getLatestGitVersionCode(
+                'https://raw.githubusercontent.com/shaarli/Shaarli/'
+               .'v0.5.4/shaarli_version.php',
+                $testTimeout
+            )
+        );
+        $this->assertRegexp(
+            self::$versionPattern,
+            ApplicationUtils::getLatestGitVersionCode(
+                'https://raw.githubusercontent.com/shaarli/Shaarli/'
+               .'master/shaarli_version.php',
+                $testTimeout
+            )
+        );
+    }
+
+    /**
+     * Attempt to retrieve the latest version from an invalid URL
+     */
+    public function testGetLatestGitVersionCodeInvalidUrl()
+    {
+        $this->assertFalse(
+            ApplicationUtils::getLatestGitVersionCode('htttp://null.io', 0)
+        );
+    }
+
+    /**
+     * Test update checks - the user is logged off
+     */
+    public function testCheckUpdateLoggedOff()
+    {
+        $this->assertFalse(
+            ApplicationUtils::checkUpdate(self::$testVersion, 'null', 0, false, false)
+        );
+    }
+
+    /**
+     * Test update checks - the user has disabled updates
+     */
+    public function testCheckUpdateUserDisabled()
+    {
+        $this->assertFalse(
+            ApplicationUtils::checkUpdate(self::$testVersion, 'null', 0, false, true)
+        );
+    }
+
+    /**
+     * A newer version is available
+     */
+    public function testCheckUpdateNewVersionNew()
+    {
+        $newVersion = '1.8.3';
+        FakeApplicationUtils::$VERSION_CODE = $newVersion;
+
+        $version = FakeApplicationUtils::checkUpdate(
+            self::$testVersion,
+            self::$testUpdateFile,
+            100,
+            true,
+            true
+        );
+
+        $this->assertEquals($newVersion, $version);
+    }
+
+    /**
+     * No available information about versions
+     */
+    public function testCheckUpdateNewVersionUnavailable()
+    {
+        $version = FakeApplicationUtils::checkUpdate(
+            self::$testVersion,
+            self::$testUpdateFile,
+            100,
+            true,
+            true
+        );
+
+        $this->assertFalse($version);
+    }
+
+    /**
+     * Shaarli is up-to-date
+     */
+    public function testCheckUpdateNewVersionUpToDate()
+    {
+        FakeApplicationUtils::$VERSION_CODE = self::$testVersion;
+
+        $version = FakeApplicationUtils::checkUpdate(
+            self::$testVersion,
+            self::$testUpdateFile,
+            100,
+            true,
+            true
+        );
+
+        $this->assertFalse($version);
+    }
+
+    /**
+     * Time-traveller's Shaarli
+     */
+    public function testCheckUpdateNewVersionMaartiMcFly()
+    {
+        FakeApplicationUtils::$VERSION_CODE = '0.4.1';
+
+        $version = FakeApplicationUtils::checkUpdate(
+            self::$testVersion,
+            self::$testUpdateFile,
+            100,
+            true,
+            true
+        );
+
+        $this->assertFalse($version);
+    }
+
+    /**
+     * The version has been checked recently and Shaarli is up-to-date
+     */
+    public function testCheckUpdateNewVersionTwiceUpToDate()
+    {
+        FakeApplicationUtils::$VERSION_CODE = self::$testVersion;
+
+        // Create the update file
+        $version = FakeApplicationUtils::checkUpdate(
+            self::$testVersion,
+            self::$testUpdateFile,
+            100,
+            true,
+            true
+        );
+
+        $this->assertFalse($version);
+
+        // Reuse the update file
+        $version = FakeApplicationUtils::checkUpdate(
+            self::$testVersion,
+            self::$testUpdateFile,
+            100,
+            true,
+            true
+        );
+
+        $this->assertFalse($version);
+    }
+
+    /**
+     * The version has been checked recently and Shaarli is outdated
+     */
+    public function testCheckUpdateNewVersionTwiceOutdated()
+    {
+        $newVersion = '1.8.3';
+        FakeApplicationUtils::$VERSION_CODE = $newVersion;
+
+        // Create the update file
+        $version = FakeApplicationUtils::checkUpdate(
+            self::$testVersion,
+            self::$testUpdateFile,
+            100,
+            true,
+            true
+        );
+        $this->assertEquals($newVersion, $version);
+
+        // Reuse the update file
+        $version = FakeApplicationUtils::checkUpdate(
+            self::$testVersion,
+            self::$testUpdateFile,
+            100,
+            true,
+            true
+        );
+        $this->assertEquals($newVersion, $version);
+    }
+
     /**
      * Check supported PHP versions
      */

tests/CacheTest.php

@@ -11,10 +11,10 @@ require_once 'application/Cache.php';
 /**
  * Unitary tests for cached pages
  */
-class CachedTest extends PHPUnit_Framework_TestCase
+class CacheTest extends PHPUnit_Framework_TestCase
 {
     // test cache directory
-    protected static $testCacheDir = 'tests/dummycache';
+    protected static $testCacheDir = 'sandbox/dummycache';
 
     // dummy cached file names / content
     protected static $pages = array('a', 'toto', 'd7b59c');
@@ -56,7 +56,7 @@ class CachedTest extends PHPUnit_Framework_TestCase
     public function testPurgeCachedPagesMissingDir()
     {
         $this->assertEquals(
-            'Cannot purge tests/dummycache_missing: no directory',
+            'Cannot purge sandbox/dummycache_missing: no directory',
             purgeCachedPages(self::$testCacheDir.'_missing')
         );
     }

tests/CachedPageTest.php

@@ -11,7 +11,7 @@ require_once 'application/CachedPage.php';
 class CachedPageTest extends PHPUnit_Framework_TestCase
 {
     // test cache directory
-    protected static $testCacheDir = 'tests/pagecache';
+    protected static $testCacheDir = 'sandbox/pagecache';
     protected static $url = 'http://shaar.li/?do=atom';
     protected static $filename;
 

tests/LinkDBTest.php

@@ -16,7 +16,7 @@ require_once 'tests/utils/ReferenceLinkDB.php';
 class LinkDBTest extends PHPUnit_Framework_TestCase
 {
     // datastore to test write operations
-    protected static $testDatastore = 'tests/datastore.php';
+    protected static $testDatastore = 'sandbox/datastore.php';
     protected static $refDB = null;
     protected static $publicLinkDB = null;
     protected static $privateLinkDB = null;