Daily RSS Cache: invalidate cache base on the date

Currently the cache is only invalidated when the datastore changes, while it should rely on selected period of time. Fixes #1659
2020-12-17 15:43:33 +01:00 · 2020-12-17 15:43:33 +01:00 · f00600a283
commit f00600a283
parent ab4c170672
6 changed files with 213 additions and 68 deletions
--- a/application/feed/CachedPage.php
+++ b/application/feed/CachedPage.php
@ -1,34 +1,43 @@
 <?php

+declare(strict_types=1);
+
 namespace Shaarli\Feed;

+use DatePeriod;
+
 /**
 * Simple cache system, mainly for the RSS/ATOM feeds
 */
 class CachedPage
 {
-    // Directory containing page caches
-    private $cacheDir;
+    /** Directory containing page caches */
+    protected $cacheDir;

-    // Should this URL be cached (boolean)?
-    private $shouldBeCached;
+    /** Should this URL be cached (boolean)? */
+    protected $shouldBeCached;

-    // Name of the cache file for this URL
-    private $filename;
+    /** Name of the cache file for this URL */
+    protected $filename;
+
+    /** @var DatePeriod|null Optionally specify a period of time for cache validity */
+    protected $validityPeriod;

    /**
     * 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
+     * @param string      $cacheDir       page cache directory
+     * @param string      $url            page URL
+     * @param bool        $shouldBeCached whether this page needs to be cached
+     * @param ?DatePeriod $validityPeriod Optionally specify a time limit on requested cache
     */
-    public function __construct($cacheDir, $url, $shouldBeCached)
+    public function __construct($cacheDir, $url, $shouldBeCached, ?DatePeriod $validityPeriod)
    {
        // TODO: check write access to the cache directory
        $this->cacheDir = $cacheDir;
        $this->filename = $this->cacheDir . '/' . sha1($url) . '.cache';
        $this->shouldBeCached = $shouldBeCached;
+        $this->validityPeriod = $validityPeriod;
    }

    /**
@ -41,10 +50,20 @@ class CachedPage
        if (!$this->shouldBeCached) {
            return null;
        }
-        if (is_file($this->filename)) {
-            return file_get_contents($this->filename);
+        if (!is_file($this->filename)) {
+            return null;
        }
-        return null;
+        if ($this->validityPeriod !== null) {
+            $cacheDate = \DateTime::createFromFormat('U', (string) filemtime($this->filename));
+            if (
+                $cacheDate < $this->validityPeriod->getStartDate()
+                || $cacheDate > $this->validityPeriod->getEndDate()
+            ) {
+                return null;
+            }
+        }
+
+        return file_get_contents($this->filename);
    }

    /**
--- a/application/front/controller/visitor/DailyController.php
+++ b/application/front/controller/visitor/DailyController.php
@ -86,9 +86,11 @@ class DailyController extends ShaarliVisitorController
    public function rss(Request $request, Response $response): Response
    {
        $response = $response->withHeader('Content-Type', 'application/rss+xml; charset=utf-8');
+        $type = DailyPageHelper::extractRequestedType($request);
+        $cacheDuration = DailyPageHelper::getCacheDatePeriodByType($type);

        $pageUrl = page_url($this->container->environment);
-        $cache = $this->container->pageCacheManager->getCachePage($pageUrl);
+        $cache = $this->container->pageCacheManager->getCachePage($pageUrl, $cacheDuration);

        $cached = $cache->cachedVersion();
        if (!empty($cached)) {
@ -96,7 +98,6 @@ class DailyController extends ShaarliVisitorController
        }

        $days = [];
-        $type = DailyPageHelper::extractRequestedType($request);
        $format = DailyPageHelper::getFormatByType($type);
        $length = DailyPageHelper::getRssLengthByType($type);
        foreach ($this->container->bookmarkService->search() as $bookmark) {
--- a/application/helper/DailyPageHelper.php
+++ b/application/helper/DailyPageHelper.php
@ -4,6 +4,9 @@ declare(strict_types=1);

 namespace Shaarli\Helper;

+use DatePeriod;
+use DateTimeImmutable;
+use Exception;
 use Shaarli\Bookmark\Bookmark;
 use Slim\Http\Request;

@ -40,31 +43,31 @@ class DailyPageHelper
     * @param string|null   $requestedDate  Input string extracted from the request
     * @param Bookmark|null $latestBookmark Latest bookmark found in the datastore (by date)
     *
-     * @return \DateTimeImmutable from input or latest bookmark.
+     * @return DateTimeImmutable from input or latest bookmark.
     *
-     * @throws \Exception Type not supported.
+     * @throws Exception Type not supported.
     */
    public static function extractRequestedDateTime(
        string $type,
        ?string $requestedDate,
        Bookmark $latestBookmark = null
-    ): \DateTimeImmutable {
+    ): DateTimeImmutable {
        $format = static::getFormatByType($type);
        if (empty($requestedDate)) {
            return $latestBookmark instanceof Bookmark
-                ? new \DateTimeImmutable($latestBookmark->getCreated()->format(\DateTime::ATOM))
-                : new \DateTimeImmutable()
+                ? new DateTimeImmutable($latestBookmark->getCreated()->format(\DateTime::ATOM))
+                : new DateTimeImmutable()
            ;
        }

        // W is not supported by createFromFormat...
        if ($type === static::WEEK) {
-            return (new \DateTimeImmutable())
+            return (new DateTimeImmutable())
                ->setISODate((int) substr($requestedDate, 0, 4), (int) substr($requestedDate, 4, 2))
            ;
        }

-        return \DateTimeImmutable::createFromFormat($format, $requestedDate);
+        return DateTimeImmutable::createFromFormat($format, $requestedDate);
    }

    /**
@ -80,7 +83,7 @@ class DailyPageHelper
     *
     * @see https://www.php.net/manual/en/datetime.format.php
     *
-     * @throws \Exception Type not supported.
+     * @throws Exception Type not supported.
     */
    public static function getFormatByType(string $type): string
    {
@ -92,7 +95,7 @@ class DailyPageHelper
            case static::DAY:
                return 'Ymd';
            default:
-                throw new \Exception('Unsupported daily format type');
+                throw new Exception('Unsupported daily format type');
        }
    }

@ -102,14 +105,14 @@ class DailyPageHelper
     *       and we don't want to alter original datetime.
     *
     * @param string             $type      month/week/day
-     * @param \DateTimeImmutable $requested DateTime extracted from request input
+     * @param DateTimeImmutable $requested DateTime extracted from request input
     *                                      (should come from extractRequestedDateTime)
     *
     * @return \DateTimeInterface First DateTime of the time period
     *
-     * @throws \Exception Type not supported.
+     * @throws Exception Type not supported.
     */
-    public static function getStartDateTimeByType(string $type, \DateTimeImmutable $requested): \DateTimeInterface
+    public static function getStartDateTimeByType(string $type, DateTimeImmutable $requested): \DateTimeInterface
    {
        switch ($type) {
            case static::MONTH:
@ -119,7 +122,7 @@ class DailyPageHelper
            case static::DAY:
                return $requested->modify('Today midnight');
            default:
-                throw new \Exception('Unsupported daily format type');
+                throw new Exception('Unsupported daily format type');
        }
    }

@ -129,14 +132,14 @@ class DailyPageHelper
     *       and we don't want to alter original datetime.
     *
     * @param string             $type      month/week/day
-     * @param \DateTimeImmutable $requested DateTime extracted from request input
+     * @param DateTimeImmutable $requested DateTime extracted from request input
     *                                      (should come from extractRequestedDateTime)
     *
     * @return \DateTimeInterface Last DateTime of the time period
     *
-     * @throws \Exception Type not supported.
+     * @throws Exception Type not supported.
     */
-    public static function getEndDateTimeByType(string $type, \DateTimeImmutable $requested): \DateTimeInterface
+    public static function getEndDateTimeByType(string $type, DateTimeImmutable $requested): \DateTimeInterface
    {
        switch ($type) {
            case static::MONTH:
@ -146,7 +149,7 @@ class DailyPageHelper
            case static::DAY:
                return $requested->modify('Today 23:59:59');
            default:
-                throw new \Exception('Unsupported daily format type');
+                throw new Exception('Unsupported daily format type');
        }
    }

@ -161,7 +164,7 @@ class DailyPageHelper
     *
     * @return string Localized time period description
     *
-     * @throws \Exception Type not supported.
+     * @throws Exception Type not supported.
     */
    public static function getDescriptionByType(
        string $type,
@ -183,7 +186,7 @@ class DailyPageHelper
                }
                return $out . format_date($requested, false);
            default:
-                throw new \Exception('Unsupported daily format type');
+                throw new Exception('Unsupported daily format type');
        }
    }

@ -194,7 +197,7 @@ class DailyPageHelper
     *
     * @return int number of elements
     *
-     * @throws \Exception Type not supported.
+     * @throws Exception Type not supported.
     */
    public static function getRssLengthByType(string $type): int
    {
@ -206,7 +209,28 @@ class DailyPageHelper
            case static::DAY:
                return 30; // ~1 month
            default:
-                throw new \Exception('Unsupported daily format type');
+                throw new Exception('Unsupported daily format type');
        }
    }
+
+    /**
+     * Get the number of items to display in the RSS feed depending on the given type.
+     *
+     * @param string             $type      month/week/day
+     * @param ?DateTimeImmutable $requested Currently only used for UT
+     *
+     * @return DatePeriod number of elements
+     *
+     * @throws Exception Type not supported.
+     */
+    public static function getCacheDatePeriodByType(string $type, DateTimeImmutable $requested = null): DatePeriod
+    {
+        $requested = $requested ?? new DateTimeImmutable();
+
+        return new DatePeriod(
+            static::getStartDateTimeByType($type, $requested),
+            new \DateInterval('P1D'),
+            static::getEndDateTimeByType($type, $requested)
+        );
+    }
 }
--- a/application/render/PageCacheManager.php
+++ b/application/render/PageCacheManager.php
@ -2,6 +2,7 @@

 namespace Shaarli\Render;

+use DatePeriod;
 use Shaarli\Feed\CachedPage;

 /**
@ -49,12 +50,21 @@ class PageCacheManager
        $this->purgeCachedPages();
    }

-    public function getCachePage(string $pageUrl): CachedPage
+    /**
+     * Get CachedPage instance for provided URL.
+     *
+     * @param string      $pageUrl
+     * @param ?DatePeriod $validityPeriod Optionally specify a time limit on requested cache
+     *
+     * @return CachedPage
+     */
+    public function getCachePage(string $pageUrl, DatePeriod $validityPeriod = null): CachedPage
    {
        return new CachedPage(
            $this->pageCacheDir,
            $pageUrl,
-            false === $this->isLoggedIn
+            false === $this->isLoggedIn,
+            $validityPeriod
        );
    }
 }
--- a/tests/feed/CachedPageTest.php
+++ b/tests/feed/CachedPageTest.php
@ -40,10 +40,10 @@ class CachedPageTest extends \Shaarli\TestCase
     */
    public function testConstruct()
    {
-        new CachedPage(self::$testCacheDir, '', true);
-        new CachedPage(self::$testCacheDir, '', false);
-        new CachedPage(self::$testCacheDir, 'http://shaar.li/feed/rss', true);
-        new CachedPage(self::$testCacheDir, 'http://shaar.li/feed/atom', false);
+        new CachedPage(self::$testCacheDir, '', true, null);
+        new CachedPage(self::$testCacheDir, '', false, null);
+        new CachedPage(self::$testCacheDir, 'http://shaar.li/feed/rss', true, null);
+        new CachedPage(self::$testCacheDir, 'http://shaar.li/feed/atom', false, null);
        $this->addToAssertionCount(1);
    }

@ -52,7 +52,7 @@ class CachedPageTest extends \Shaarli\TestCase
     */
    public function testCache()
    {
-        $page = new CachedPage(self::$testCacheDir, self::$url, true);
+        $page = new CachedPage(self::$testCacheDir, self::$url, true, null);

        $this->assertFileNotExists(self::$filename);
        $page->cache('<p>Some content</p>');
@ -68,7 +68,7 @@ class CachedPageTest extends \Shaarli\TestCase
     */
    public function testShouldNotCache()
    {
-        $page = new CachedPage(self::$testCacheDir, self::$url, false);
+        $page = new CachedPage(self::$testCacheDir, self::$url, false, null);

        $this->assertFileNotExists(self::$filename);
        $page->cache('<p>Some content</p>');
@ -80,7 +80,7 @@ class CachedPageTest extends \Shaarli\TestCase
     */
    public function testCachedVersion()
    {
-        $page = new CachedPage(self::$testCacheDir, self::$url, true);
+        $page = new CachedPage(self::$testCacheDir, self::$url, true, null);

        $this->assertFileNotExists(self::$filename);
        $page->cache('<p>Some content</p>');
@ -96,7 +96,7 @@ class CachedPageTest extends \Shaarli\TestCase
     */
    public function testCachedVersionNoFile()
    {
-        $page = new CachedPage(self::$testCacheDir, self::$url, true);
+        $page = new CachedPage(self::$testCacheDir, self::$url, true, null);

        $this->assertFileNotExists(self::$filename);
        $this->assertEquals(
@ -110,7 +110,7 @@ class CachedPageTest extends \Shaarli\TestCase
     */
    public function testNoCachedVersion()
    {
-        $page = new CachedPage(self::$testCacheDir, self::$url, false);
+        $page = new CachedPage(self::$testCacheDir, self::$url, false, null);

        $this->assertFileNotExists(self::$filename);
        $this->assertEquals(
@ -118,4 +118,43 @@ class CachedPageTest extends \Shaarli\TestCase
            $page->cachedVersion()
        );
    }
+
+    /**
+     * Return a page's cached content within date period
+     */
+    public function testCachedVersionInDatePeriod()
+    {
+        $period = new \DatePeriod(
+            new \DateTime('yesterday'),
+            new \DateInterval('P1D'),
+            new \DateTime('tomorrow')
+        );
+        $page = new CachedPage(self::$testCacheDir, self::$url, true, $period);
+
+        $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 outside of date period
+     */
+    public function testCachedVersionNotInDatePeriod()
+    {
+        $period = new \DatePeriod(
+            new \DateTime('yesterday noon'),
+            new \DateInterval('P1D'),
+            new \DateTime('yesterday midnight')
+        );
+        $page = new CachedPage(self::$testCacheDir, self::$url, true, $period);
+
+        $this->assertFileNotExists(self::$filename);
+        $page->cache('<p>Some content</p>');
+        $this->assertFileExists(self::$filename);
+        $this->assertNull($page->cachedVersion());
+    }
 }
--- a/tests/helper/DailyPageHelperTest.php
+++ b/tests/helper/DailyPageHelperTest.php
@ -4,6 +4,8 @@ declare(strict_types=1);

 namespace Shaarli\Helper;

+use DateTimeImmutable;
+use DateTimeInterface;
 use Shaarli\Bookmark\Bookmark;
 use Shaarli\TestCase;
 use Slim\Http\Request;
@ -32,7 +34,7 @@ class DailyPageHelperTest extends TestCase
        string $type,
        string $input,
        ?Bookmark $bookmark,
-        \DateTimeInterface $expectedDateTime,
+        DateTimeInterface $expectedDateTime,
        string $compareFormat = 'Ymd'
    ): void {
        $dateTime = DailyPageHelper::extractRequestedDateTime($type, $input, $bookmark);
@ -71,8 +73,8 @@ class DailyPageHelperTest extends TestCase
     */
    public function testGetStartDatesByType(
        string $type,
-        \DateTimeImmutable $dateTime,
-        \DateTimeInterface $expectedDateTime
+        DateTimeImmutable $dateTime,
+        DateTimeInterface $expectedDateTime
    ): void {
        $startDateTime = DailyPageHelper::getStartDateTimeByType($type, $dateTime);

@ -84,7 +86,7 @@ class DailyPageHelperTest extends TestCase
        $this->expectException(\Exception::class);
        $this->expectExceptionMessage('Unsupported daily format type');

-        DailyPageHelper::getStartDateTimeByType('nope', new \DateTimeImmutable());
+        DailyPageHelper::getStartDateTimeByType('nope', new DateTimeImmutable());
    }

    /**
@ -92,8 +94,8 @@ class DailyPageHelperTest extends TestCase
     */
    public function testGetEndDatesByType(
        string $type,
-        \DateTimeImmutable $dateTime,
-        \DateTimeInterface $expectedDateTime
+        DateTimeImmutable $dateTime,
+        DateTimeInterface $expectedDateTime
    ): void {
        $endDateTime = DailyPageHelper::getEndDateTimeByType($type, $dateTime);

@ -105,7 +107,7 @@ class DailyPageHelperTest extends TestCase
        $this->expectException(\Exception::class);
        $this->expectExceptionMessage('Unsupported daily format type');

-        DailyPageHelper::getEndDateTimeByType('nope', new \DateTimeImmutable());
+        DailyPageHelper::getEndDateTimeByType('nope', new DateTimeImmutable());
    }

    /**
@ -113,7 +115,7 @@ class DailyPageHelperTest extends TestCase
     */
    public function testGeDescriptionsByType(
        string $type,
-        \DateTimeImmutable $dateTime,
+        DateTimeImmutable $dateTime,
        string $expectedDescription
    ): void {
        $description = DailyPageHelper::getDescriptionByType($type, $dateTime);
@ -139,7 +141,7 @@ class DailyPageHelperTest extends TestCase
        $this->expectException(\Exception::class);
        $this->expectExceptionMessage('Unsupported daily format type');

-        DailyPageHelper::getDescriptionByType('nope', new \DateTimeImmutable());
+        DailyPageHelper::getDescriptionByType('nope', new DateTimeImmutable());
    }

    /**
@ -159,6 +161,29 @@ class DailyPageHelperTest extends TestCase
        DailyPageHelper::getRssLengthByType('nope');
    }

+    /**
+     * @dataProvider getCacheDatePeriodByType
+     */
+    public function testGetCacheDatePeriodByType(
+        string $type,
+        DateTimeImmutable $requested,
+        DateTimeInterface $start,
+        DateTimeInterface $end
+    ): void {
+        $period = DailyPageHelper::getCacheDatePeriodByType($type, $requested);
+
+        static::assertEquals($start, $period->getStartDate());
+        static::assertEquals($end, $period->getEndDate());
+    }
+
+    public function testGetCacheDatePeriodByTypeExceptionUnknownType(): void
+    {
+        $this->expectException(\Exception::class);
+        $this->expectExceptionMessage('Unsupported daily format type');
+
+        DailyPageHelper::getCacheDatePeriodByType('nope');
+    }
+
    /**
     * Data provider for testExtractRequestedType() test method.
     */
@ -229,9 +254,9 @@ class DailyPageHelperTest extends TestCase
    public function getStartDatesByType(): array
    {
        return [
-            [DailyPageHelper::DAY, new \DateTimeImmutable('2020-10-09 04:05:06'), new \DateTime('2020-10-09 00:00:00')],
-            [DailyPageHelper::WEEK, new \DateTimeImmutable('2020-10-09 04:05:06'), new \DateTime('2020-10-05 00:00:00')],
-            [DailyPageHelper::MONTH, new \DateTimeImmutable('2020-10-09 04:05:06'), new \DateTime('2020-10-01 00:00:00')],
+            [DailyPageHelper::DAY, new DateTimeImmutable('2020-10-09 04:05:06'), new \DateTime('2020-10-09 00:00:00')],
+            [DailyPageHelper::WEEK, new DateTimeImmutable('2020-10-09 04:05:06'), new \DateTime('2020-10-05 00:00:00')],
+            [DailyPageHelper::MONTH, new DateTimeImmutable('2020-10-09 04:05:06'), new \DateTime('2020-10-01 00:00:00')],
        ];
    }

@ -241,9 +266,9 @@ class DailyPageHelperTest extends TestCase
    public function getEndDatesByType(): array
    {
        return [
-            [DailyPageHelper::DAY, new \DateTimeImmutable('2020-10-09 04:05:06'), new \DateTime('2020-10-09 23:59:59')],
-            [DailyPageHelper::WEEK, new \DateTimeImmutable('2020-10-09 04:05:06'), new \DateTime('2020-10-11 23:59:59')],
-            [DailyPageHelper::MONTH, new \DateTimeImmutable('2020-10-09 04:05:06'), new \DateTime('2020-10-31 23:59:59')],
+            [DailyPageHelper::DAY, new DateTimeImmutable('2020-10-09 04:05:06'), new \DateTime('2020-10-09 23:59:59')],
+            [DailyPageHelper::WEEK, new DateTimeImmutable('2020-10-09 04:05:06'), new \DateTime('2020-10-11 23:59:59')],
+            [DailyPageHelper::MONTH, new DateTimeImmutable('2020-10-09 04:05:06'), new \DateTime('2020-10-31 23:59:59')],
        ];
    }

@ -253,11 +278,11 @@ class DailyPageHelperTest extends TestCase
    public function getDescriptionsByType(): array
    {
        return [
-            [DailyPageHelper::DAY, $date = new \DateTimeImmutable(), 'Today - ' . $date->format('F j, Y')],
-            [DailyPageHelper::DAY, $date = new \DateTimeImmutable('-1 day'), 'Yesterday - ' . $date->format('F j, Y')],
-            [DailyPageHelper::DAY, new \DateTimeImmutable('2020-10-09 04:05:06'), 'October 9, 2020'],
-            [DailyPageHelper::WEEK, new \DateTimeImmutable('2020-10-09 04:05:06'), 'Week 41 (October 5, 2020)'],
-            [DailyPageHelper::MONTH, new \DateTimeImmutable('2020-10-09 04:05:06'), 'October, 2020'],
+            [DailyPageHelper::DAY, $date = new DateTimeImmutable(), 'Today - ' . $date->format('F j, Y')],
+            [DailyPageHelper::DAY, $date = new DateTimeImmutable('-1 day'), 'Yesterday - ' . $date->format('F j, Y')],
+            [DailyPageHelper::DAY, new DateTimeImmutable('2020-10-09 04:05:06'), 'October 9, 2020'],
+            [DailyPageHelper::WEEK, new DateTimeImmutable('2020-10-09 04:05:06'), 'Week 41 (October 5, 2020)'],
+            [DailyPageHelper::MONTH, new DateTimeImmutable('2020-10-09 04:05:06'), 'October, 2020'],
        ];
    }

@ -276,7 +301,7 @@ class DailyPageHelperTest extends TestCase
    }

    /**
-     * Data provider for testGetDescriptionsByType() test method.
+     * Data provider for testGetRssLengthsByType() test method.
     */
    public function getRssLengthsByType(): array
    {
@ -286,4 +311,31 @@ class DailyPageHelperTest extends TestCase
            [DailyPageHelper::MONTH],
        ];
    }
+
+    /**
+     * Data provider for testGetCacheDatePeriodByType() test method.
+     */
+    public function getCacheDatePeriodByType(): array
+    {
+        return [
+            [
+                DailyPageHelper::DAY,
+                new DateTimeImmutable('2020-10-09 04:05:06'),
+                new \DateTime('2020-10-09 00:00:00'),
+                new \DateTime('2020-10-09 23:59:59'),
+            ],
+            [
+                DailyPageHelper::WEEK,
+                new DateTimeImmutable('2020-10-09 04:05:06'),
+                new \DateTime('2020-10-05 00:00:00'),
+                new \DateTime('2020-10-11 23:59:59'),
+            ],
+            [
+                DailyPageHelper::MONTH,
+                new DateTimeImmutable('2020-10-09 04:05:06'),
+                new \DateTime('2020-10-01 00:00:00'),
+                new \DateTime('2020-10-31 23:59:59'),
+            ],
+        ];
+    }
 }