From f00600a283617286c813dc902fe3a2d66938b5fc Mon Sep 17 00:00:00 2001 From: ArthurHoaro Date: Thu, 17 Dec 2020 15:43:33 +0100 Subject: 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 --- application/feed/CachedPage.php | 45 ++++++++--- .../front/controller/visitor/DailyController.php | 5 +- application/helper/DailyPageHelper.php | 66 ++++++++++----- application/render/PageCacheManager.php | 14 +++- tests/feed/CachedPageTest.php | 57 ++++++++++--- tests/helper/DailyPageHelperTest.php | 94 +++++++++++++++++----- 6 files changed, 213 insertions(+), 68 deletions(-) diff --git a/application/feed/CachedPage.php b/application/feed/CachedPage.php index d809bdd9..c23c200f 100644 --- a/application/feed/CachedPage.php +++ b/application/feed/CachedPage.php @@ -1,34 +1,43 @@ 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; + } + if ($this->validityPeriod !== null) { + $cacheDate = \DateTime::createFromFormat('U', (string) filemtime($this->filename)); + if ( + $cacheDate < $this->validityPeriod->getStartDate() + || $cacheDate > $this->validityPeriod->getEndDate() + ) { + return null; + } } - return null; + + return file_get_contents($this->filename); } /** diff --git a/application/front/controller/visitor/DailyController.php b/application/front/controller/visitor/DailyController.php index 5ae89299..29492a5f 100644 --- 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) { diff --git a/application/helper/DailyPageHelper.php b/application/helper/DailyPageHelper.php index 9bdb7ba5..05f95812 100644 --- 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) + ); + } } diff --git a/application/render/PageCacheManager.php b/application/render/PageCacheManager.php index 97805c35..fe74bf27 100644 --- 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 ); } } diff --git a/tests/feed/CachedPageTest.php b/tests/feed/CachedPageTest.php index 904db9dc..1decfaf3 100644 --- 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('

Some content

'); @@ -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('

Some content

'); @@ -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('

Some content

'); @@ -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('

Some content

'); + $this->assertFileExists(self::$filename); + $this->assertEquals( + '

Some content

', + $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('

Some content

'); + $this->assertFileExists(self::$filename); + $this->assertNull($page->cachedVersion()); + } } diff --git a/tests/helper/DailyPageHelperTest.php b/tests/helper/DailyPageHelperTest.php index 6238e648..2d745800 100644 --- 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'), + ], + ]; + } } -- cgit v1.2.3