Merge pull request #309 from virtualtam/refactor/PageCache

CachedPage: move to a proper file, add tests

8 files changed, 342 insertions, 88 deletions

diff --git a/application/Cache.php b/application/Cache.php
new file mode 100644
index 00000000..5d050165
--- /dev/null
+++ b/application/Cache.php
@@ -0,0 +1,38 @@
+<?php
+/**
+ * Cache utilities
+ */
+
+/**
+ * Purges all cached pages
+ *
+ * @param string $pageCacheDir page cache directory
+ *
+ * @return mixed an error string if the directory is missing
+ */
+function purgeCachedPages($pageCacheDir)
+{
+    if (! is_dir($pageCacheDir)) {
+        $error = 'Cannot purge '.$pageCacheDir.': no directory';
+        error_log($error);
+        return $error;
+    }
+
+    array_map('unlink', glob($pageCacheDir.'/*.cache'));
+}
+
+/**
+ * Invalidates caches when the database is changed or the user logs out.
+ *
+ * @param string $pageCacheDir page cache directory
+ */
+function invalidateCaches($pageCacheDir)
+{
+    // Purge cache attached to session.
+    if (isset($_SESSION['tags'])) {
+        unset($_SESSION['tags']);
+    }
+
+    // Purge page cache shared by sessions.
+    purgeCachedPages($pageCacheDir);
+}
diff --git a/application/CachedPage.php b/application/CachedPage.php
new file mode 100644
index 00000000..50cfa9ac
--- /dev/null
+++ b/application/CachedPage.php
@@ -0,0 +1,63 @@
+<?php
+/**
+ * Simple cache system, mainly for the RSS/ATOM feeds
+ */
+class CachedPage
+{
+    // Directory containing page caches
+    private $cacheDir;
+
+    // Full URL of the page to cache -typically the value returned by pageUrl()
+    private $url;
+
+    // Should this URL be cached (boolean)?
+    private $shouldBeCached;
+
+    // Name of the cache file for this URL
+    private $filename;
+
+    /**
+     * Creates a new CachedPage
+     *
+     * @param string $cacheDir       page cache directory
+     * @param string $url            page URL
+     * @param bool   $shouldBeCached whether this page needs to be cached
+     */
+    public function __construct($cacheDir, $url, $shouldBeCached)
+    {
+        // TODO: check write access to the cache directory
+        $this->cacheDir = $cacheDir;
+        $this->url = $url;
+        $this->filename = $this->cacheDir.'/'.sha1($url).'.cache';
+        $this->shouldBeCached = $shouldBeCached;
+    }
+
+    /**
+     * Returns the cached version of a page, if it exists and should be cached
+     *
+     * @return a cached version of the page if it exists, null otherwise
+     */
+    public function cachedVersion()
+    {
+        if (!$this->shouldBeCached) {
+            return null;
+        }
+        if (is_file($this->filename)) {
+            return file_get_contents($this->filename);
+        }
+        return null;
+    }
+
+    /**
+     * Puts a page in the cache
+     *
+     * @param string $pageContent XML content to cache
+     */
+    public function cache($pageContent)
+    {
+        if (!$this->shouldBeCached) {
+            return;
+        }
+        file_put_contents($this->filename, $pageContent);
+    }
+}
diff --git a/application/LinkDB.php b/application/LinkDB.php
index 1e16fef1..463aa47e 100644
--- a/application/LinkDB.php
+++ b/application/LinkDB.php
@@ -269,8 +269,10 @@ You use the community supported version of the original Shaarli project, by Seba
 
     /**
      * Saves the database from memory to disk
+     *
+     * @param string $pageCacheDir page cache directory
      */
-    public function savedb()
+    public function savedb($pageCacheDir)
     {
         if (!$this->_loggedIn) {
             // TODO: raise an Exception instead
@@ -280,7 +282,7 @@ You use the community supported version of the original Shaarli project, by Seba
             $this->_datastore,
             self::$phpPrefix.base64_encode(gzdeflate(serialize($this->_links))).self::$phpSuffix
         );
-        invalidateCaches();
+        invalidateCaches($pageCacheDir);
     }
 
     /**
@@ -439,4 +441,3 @@ You use the community supported version of the original Shaarli project, by Seba
         return $linkDays;
     }
 }
-?>
diff --git a/index.php b/index.php
index 2c731e9f..84b8f015 100755
--- a/index.php
+++ b/index.php
@@ -70,6 +70,8 @@ if (is_file($GLOBALS['config']['CONFIG_FILE'])) {
 }
 
 // Shaarli library
+require_once 'application/Cache.php';
+require_once 'application/CachedPage.php';
 require_once 'application/LinkDB.php';
 require_once 'application/TimeZone.php';
 require_once 'application/Utils.php';
@@ -203,63 +205,6 @@ function checkUpdate()
 
 
 // -----------------------------------------------------------------------------------------------
-// Simple cache system (mainly for the RSS/ATOM feeds).
-
-class pageCache
-{
-    private $url; // Full URL of the page to cache (typically the value returned by pageUrl())
-    private $shouldBeCached; // boolean: Should this url be cached?
-    private $filename; // Name of the cache file for this url.
-
-    /*
-         $url = URL (typically the value returned by pageUrl())
-         $shouldBeCached = boolean. If false, the cache will be disabled.
-    */
-    public function __construct($url,$shouldBeCached)
-    {
-        $this->url = $url;
-        $this->filename = $GLOBALS['config']['PAGECACHE'].'/'.sha1($url).'.cache';
-        $this->shouldBeCached = $shouldBeCached;
-    }
-
-    // If the page should be cached and a cached version exists,
-    // returns the cached version (otherwise, return null).
-    public function cachedVersion()
-    {
-        if (!$this->shouldBeCached) return null;
-        if (is_file($this->filename)) { return file_get_contents($this->filename); exit; }
-        return null;
-    }
-
-    // Put a page in the cache.
-    public function cache($page)
-    {
-        if (!$this->shouldBeCached) return;
-        file_put_contents($this->filename,$page);
-    }
-
-    // Purge the whole cache.
-    // (call with pageCache::purgeCache())
-    public static function purgeCache()
-    {
-        if (is_dir($GLOBALS['config']['PAGECACHE']))
-        {
-            $handler = opendir($GLOBALS['config']['PAGECACHE']);
-            if ($handler!==false)
-            {
-                while (($filename = readdir($handler))!==false)
-                {
-                    if (endsWith($filename,'.cache')) { unlink($GLOBALS['config']['PAGECACHE'].'/'.$filename); }
-                }
-                closedir($handler);
-            }
-        }
-    }
-
-}
-
-
-// -----------------------------------------------------------------------------------------------
 // Log to text file
 function logm($message)
 {
@@ -718,8 +663,16 @@ function showRSS()
 
     // Cache system
     $query = $_SERVER["QUERY_STRING"];
-    $cache = new pageCache(pageUrl(),startsWith($query,'do=rss') && !isLoggedIn());
-    $cached = $cache->cachedVersion(); if (!empty($cached)) { echo $cached; exit; }
+    $cache = new CachedPage(
+        $GLOBALS['config']['PAGECACHE'],
+        pageUrl(),
+        startsWith($query,'do=rss') && !isLoggedIn()
+    );
+    $cached = $cache->cachedVersion();
+    if (! empty($cached)) {
+        echo $cached;
+        exit;
+    }
 
     // If cached was not found (or not usable), then read the database and build the response:
     $LINKSDB = new LinkDB(
@@ -798,11 +751,19 @@ function showATOM()
 
     // Cache system
     $query = $_SERVER["QUERY_STRING"];
-    $cache = new pageCache(pageUrl(),startsWith($query,'do=atom') && !isLoggedIn());
-    $cached = $cache->cachedVersion(); if (!empty($cached)) { echo $cached; exit; }
-    // If cached was not found (or not usable), then read the database and build the response:
+    $cache = new CachedPage(
+        $GLOBALS['config']['PAGECACHE'],
+        pageUrl(),
+        startsWith($query,'do=atom') && !isLoggedIn()
+    );
+    $cached = $cache->cachedVersion();
+    if (!empty($cached)) {
+        echo $cached;
+        exit;
+    }
 
-// Read links from database (and filter private links if used it not logged in).
+    // If cached was not found (or not usable), then read the database and build the response:
+    // Read links from database (and filter private links if used it not logged in).
     $LINKSDB = new LinkDB(
         $GLOBALS['config']['DATASTORE'],
         isLoggedIn() || $GLOBALS['config']['OPEN_SHAARLI'],
@@ -884,7 +845,11 @@ function showATOM()
 function showDailyRSS() {
     // Cache system
     $query = $_SERVER["QUERY_STRING"];
-    $cache = new pageCache(pageUrl(), startsWith($query, 'do=dailyrss') && !isLoggedIn());
+    $cache = new CachedPage(
+        $GLOBALS['config']['PAGECACHE'],
+        pageUrl(),
+        startsWith($query,'do=dailyrss') && !isLoggedIn()
+    );
     $cached = $cache->cachedVersion();
     if (!empty($cached)) {
         echo $cached;
@@ -1076,7 +1041,7 @@ function renderPage()
     // -------- User wants to logout.
     if (isset($_SERVER["QUERY_STRING"]) && startswith($_SERVER["QUERY_STRING"],'do=logout'))
     {
-        invalidateCaches();
+        invalidateCaches($GLOBALS['config']['PAGECACHE']);
         logout();
         header('Location: ?');
         exit;
@@ -1383,7 +1348,7 @@ function renderPage()
                 $value['tags']=trim(implode(' ',$tags));
                 $LINKSDB[$key]=$value;
             }
-            $LINKSDB->savedb(); // Save to disk.
+            $LINKSDB->savedb($GLOBALS['config']['PAGECACHE']); // Save to disk.
             echo '<script>alert("Tag was removed from '.count($linksToAlter).' links.");document.location=\'?\';</script>';
             exit;
         }
@@ -1400,7 +1365,7 @@ function renderPage()
                 $value['tags']=trim(implode(' ',$tags));
                 $LINKSDB[$key]=$value;
             }
-            $LINKSDB->savedb(); // Save to disk.
+            $LINKSDB->savedb($GLOBALS['config']['PAGECACHE']); // Save to disk.
             echo '<script>alert("Tag was renamed in '.count($linksToAlter).' links.");document.location=\'?searchtags='.urlencode($_POST['totag']).'\';</script>';
             exit;
         }
@@ -1429,7 +1394,7 @@ function renderPage()
                       'linkdate'=>$linkdate,'tags'=>str_replace(',',' ',$tags));
         if ($link['title']=='') $link['title']=$link['url']; // If title is empty, use the URL as title.
         $LINKSDB[$linkdate] = $link;
-        $LINKSDB->savedb(); // Save to disk.
+        $LINKSDB->savedb($GLOBALS['config']['PAGECACHE']); // Save to disk.
         pubsubhub();
 
         // If we are called from the bookmarklet, we must close the popup:
@@ -1462,7 +1427,7 @@ function renderPage()
         // - we are protected from XSRF by the token.
         $linkdate=$_POST['lf_linkdate'];
         unset($LINKSDB[$linkdate]);
-        $LINKSDB->savedb(); // save to disk
+        $LINKSDB->savedb($GLOBALS['config']['PAGECACHE']); // save to disk
 
         // If we are called from the bookmarklet, we must close the popup:
         if (isset($_GET['source']) && ($_GET['source']=='bookmarklet' || $_GET['source']=='firefoxsocialapi')) { echo '<script>self.close();</script>'; exit; }
@@ -1751,7 +1716,7 @@ function importFile()
                 }
             }
         }
-        $LINKSDB->savedb();
+        $LINKSDB->savedb($GLOBALS['config']['PAGECACHE']);
 
         echo '<script>alert("File '.json_encode($filename).' ('.$filesize.' bytes) was successfully processed: '.$import_count.' links imported.");document.location=\'?\';</script>';
     }
@@ -2386,14 +2351,6 @@ function resizeImage($filepath)
     return true;
 }
 
-// Invalidate caches when the database is changed or the user logs out.
-// (e.g. tags cache).
-function invalidateCaches()
-{
-    unset($_SESSION['tags']);  // Purge cache attached to session.
-    pageCache::purgeCache();   // Purge page cache shared by sessions.
-}
-
 try {
     mergeDeprecatedConfig($GLOBALS, isLoggedIn());
 } catch(Exception $e) {
diff --git a/tests/CacheTest.php b/tests/CacheTest.php
new file mode 100644
index 00000000..aa5395b0
--- /dev/null
+++ b/tests/CacheTest.php
@@ -0,0 +1,79 @@
+<?php
+/**
+ * Cache tests
+ */
+
+// required to access $_SESSION array
+session_start();
+
+require_once 'application/Cache.php';
+
+/**
+ * Unitary tests for cached pages
+ */
+class CachedTest extends PHPUnit_Framework_TestCase
+{
+    // test cache directory
+    protected static $testCacheDir = 'tests/dummycache';
+
+    // dummy cached file names / content
+    protected static $pages = array('a', 'toto', 'd7b59c');
+
+
+    /**
+     * Populate the cache with dummy files
+     */
+    public function setUp()
+    {
+        if (! is_dir(self::$testCacheDir)) {
+            mkdir(self::$testCacheDir);
+        } else {
+            array_map('unlink', glob(self::$testCacheDir.'/*'));
+        }
+        
+        foreach (self::$pages as $page) {
+            file_put_contents(self::$testCacheDir.'/'.$page.'.cache', $page);
+        }
+        file_put_contents(self::$testCacheDir.'/intru.der', 'ShouldNotBeThere');
+    }
+
+    /**
+     * Purge cached pages
+     */
+    public function testPurgeCachedPages()
+    {
+        purgeCachedPages(self::$testCacheDir);
+        foreach (self::$pages as $page) {
+            $this->assertFileNotExists(self::$testCacheDir.'/'.$page.'.cache');
+        }
+
+        $this->assertFileExists(self::$testCacheDir.'/intru.der');
+    }
+
+    /**
+     * Purge cached pages - missing directory
+     */
+    public function testPurgeCachedPagesMissingDir()
+    {
+        $this->assertEquals(
+            'Cannot purge tests/dummycache_missing: no directory',
+            purgeCachedPages(self::$testCacheDir.'_missing')
+        );
+    }
+
+    /**
+     * Purge cached pages and session cache
+     */
+    public function testInvalidateCaches()
+    {
+        $this->assertArrayNotHasKey('tags', $_SESSION);
+        $_SESSION['tags'] = array('goodbye', 'cruel', 'world');
+
+        invalidateCaches(self::$testCacheDir);
+        foreach (self::$pages as $page) {
+            $this->assertFileNotExists(self::$testCacheDir.'/'.$page.'.cache');
+        }        
+
+        $this->assertArrayNotHasKey('tags', $_SESSION);
+    }
+}
diff --git a/tests/CachedPageTest.php b/tests/CachedPageTest.php
new file mode 100644
index 00000000..e97af030
--- /dev/null
+++ b/tests/CachedPageTest.php
@@ -0,0 +1,121 @@
+<?php
+/**
+ * PageCache tests
+ */
+
+require_once 'application/CachedPage.php';
+
+/**
+ * Unitary tests for cached pages
+ */
+class CachedPageTest extends PHPUnit_Framework_TestCase
+{
+    // test cache directory
+    protected static $testCacheDir = 'tests/pagecache';
+    protected static $url = 'http://shaar.li/?do=atom';
+    protected static $filename;
+
+    /**
+     * Create the cache directory if needed
+     */
+    public static function setUpBeforeClass()
+    {
+        if (! is_dir(self::$testCacheDir)) {
+            mkdir(self::$testCacheDir);
+        }
+        self::$filename = self::$testCacheDir.'/'.sha1(self::$url).'.cache';
+    }
+
+    /**
+     * Reset the page cache
+     */
+    public function setUp()
+    {
+        if (file_exists(self::$filename)) {
+            unlink(self::$filename);
+        }
+    }
+
+    /**
+     * Create a new cached page
+     */
+    public function testConstruct()
+    {
+        new CachedPage(self::$testCacheDir, '', true);
+        new CachedPage(self::$testCacheDir, '', false);
+        new CachedPage(self::$testCacheDir, 'http://shaar.li/?do=rss', true);
+        new CachedPage(self::$testCacheDir, 'http://shaar.li/?do=atom', false);
+    }
+
+    /**
+     * Cache a page's content
+     */
+    public function testCache()
+    {
+        $page = new CachedPage(self::$testCacheDir, self::$url, true);
+
+        $this->assertFileNotExists(self::$filename);
+        $page->cache('<p>Some content</p>');
+        $this->assertFileExists(self::$filename);
+        $this->assertEquals(
+            '<p>Some content</p>',
+            file_get_contents(self::$filename)
+        );
+    }
+
+    /**
+     * "Cache" a page's content - the page is not to be cached
+     */
+    public function testShouldNotCache()
+    {
+        $page = new CachedPage(self::$testCacheDir, self::$url, false);
+
+        $this->assertFileNotExists(self::$filename);
+        $page->cache('<p>Some content</p>');
+        $this->assertFileNotExists(self::$filename);
+    }
+
+    /**
+     * Return a page's cached content
+     */
+    public function testCachedVersion()
+    {
+        $page = new CachedPage(self::$testCacheDir, self::$url, true);
+
+        $this->assertFileNotExists(self::$filename);
+        $page->cache('<p>Some content</p>');
+        $this->assertFileExists(self::$filename);
+        $this->assertEquals(
+            '<p>Some content</p>',
+            $page->cachedVersion()
+        );
+    }
+
+    /**
+     * Return a page's cached content - the file does not exist
+     */
+    public function testCachedVersionNoFile()
+    {
+        $page = new CachedPage(self::$testCacheDir, self::$url, true);
+
+        $this->assertFileNotExists(self::$filename);
+        $this->assertEquals(
+            null,
+            $page->cachedVersion()
+        );
+    }
+
+    /**
+     * Return a page's cached content - the page is not to be cached
+     */
+    public function testNoCachedVersion()
+    {
+        $page = new CachedPage(self::$testCacheDir, self::$url, false);
+
+        $this->assertFileNotExists(self::$filename);
+        $this->assertEquals(
+            null,
+            $page->cachedVersion()
+        );
+    }
+}
diff --git a/tests/LinkDBTest.php b/tests/LinkDBTest.php
index 504c8190..451f1d6f 100644
--- a/tests/LinkDBTest.php
+++ b/tests/LinkDBTest.php
@@ -3,6 +3,7 @@
  * Link datastore tests
  */
 
+require_once 'application/Cache.php';
 require_once 'application/LinkDB.php';
 require_once 'application/Utils.php';
 require_once 'tests/utils/ReferenceLinkDB.php';
@@ -180,11 +181,7 @@ class LinkDBTest extends PHPUnit_Framework_TestCase
             'tags'=>'unit test'
         );
         $testDB[$link['linkdate']] = $link;
-
-        // TODO: move PageCache to a proper class/file
-        function invalidateCaches() {}
-
-        $testDB->savedb();
+        $testDB->savedb('tests');
 
         $testDB = new LinkDB(self::$testDatastore, true, false);
         $this->assertEquals($dbSize + 1, sizeof($testDB));
@@ -514,4 +511,3 @@ class LinkDBTest extends PHPUnit_Framework_TestCase
         );
     }
 }
-?>
diff --git a/tests/utils/ReferenceLinkDB.php b/tests/utils/ReferenceLinkDB.php
index 0b225720..47b51829 100644
--- a/tests/utils/ReferenceLinkDB.php
+++ b/tests/utils/ReferenceLinkDB.php
@@ -125,4 +125,3 @@ class ReferenceLinkDB
         return $this->_privateCount;
     }
 }
-?>