From 4bf35ba56bb9f06de0cb9ab920b799a39f8eaffc Mon Sep 17 00:00:00 2001 From: VirtualTam Date: Tue, 24 Nov 2015 02:52:22 +0100 Subject: 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 --- .gitignore | 5 +- Makefile | 2 + application/ApplicationUtils.php | 92 +++++++++++++++++ index.php | 39 +++---- tests/ApplicationUtilsTest.php | 218 +++++++++++++++++++++++++++++++++++++++ tests/CacheTest.php | 6 +- tests/CachedPageTest.php | 2 +- tests/LinkDBTest.php | 2 +- 8 files changed, 331 insertions(+), 35 deletions(-) diff --git a/.gitignore b/.gitignore index b98c38b9..75cd3a6b 100644 --- a/.gitignore +++ b/.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 \ No newline at end of file +plugins/*/config.php diff --git a/Makefile b/Makefile index c560d8d1..a86f9aa8 100644 --- a/Makefile +++ b/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 diff --git a/application/ApplicationUtils.php b/application/ApplicationUtils.php index b0e94e24..c7414b77 100644 --- a/application/ApplicationUtils.php +++ b/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 = ''; + + /** + * 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 diff --git a/index.php b/index.php index 3954be97..90045d60 100644 --- a/index.php +++ b/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'])', '', str_replace('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'])) { diff --git a/tests/ApplicationUtilsTest.php b/tests/ApplicationUtilsTest.php index 01301e68..437c21fd 100644 --- a/tests/ApplicationUtilsTest.php +++ b/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 */ diff --git a/tests/CacheTest.php b/tests/CacheTest.php index aa5395b0..eacd4487 100644 --- a/tests/CacheTest.php +++ b/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') ); } diff --git a/tests/CachedPageTest.php b/tests/CachedPageTest.php index e97af030..51565cd6 100644 --- a/tests/CachedPageTest.php +++ b/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; diff --git a/tests/LinkDBTest.php b/tests/LinkDBTest.php index ff917f6d..7b22b270 100644 --- a/tests/LinkDBTest.php +++ b/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; -- cgit v1.2.3