From 5f66703b8152c9691f6b3aa556a954055c649fc2 Mon Sep 17 00:00:00 2001 From: Alexandre Alapetite Date: Sun, 23 Jun 2024 23:31:44 +0200 Subject: [PATCH 01/16] Hash-based caching https://github.com/simplepie/simplepie/pull/401 https://github.com/FreshRSS/FreshRSS/commit/9aab83af115de3b210ea41caae49b70a0f82492a https://github.com/FreshRSS/FreshRSS/commit/00127f07c5fc784130d658e3f26519b0279fc6b8 --- src/Cache/BaseDataCache.php | 20 ++++++++++++ src/Cache/DataCache.php | 14 ++++++++ src/SimplePie.php | 65 ++++++++++++++++++++++++++++++++++--- 3 files changed, 94 insertions(+), 5 deletions(-) diff --git a/src/Cache/BaseDataCache.php b/src/Cache/BaseDataCache.php index 60207b075..1b8fd13fd 100644 --- a/src/Cache/BaseDataCache.php +++ b/src/Cache/BaseDataCache.php @@ -116,4 +116,24 @@ public function delete_data(string $key): bool { return $this->cache->unlink(); } + + /** + * Retrieve the last modified time for the cache + * + * @return int Timestamp + */ + public function mtime(): int + { + return $this->cache->mtime(); + } + + /** + * Set the last modified time to the current time + * + * @return bool Success status + */ + public function touch(): bool + { + return $this->cache->touch(); + } } diff --git a/src/Cache/DataCache.php b/src/Cache/DataCache.php index 02ec59e9c..facf2b179 100644 --- a/src/Cache/DataCache.php +++ b/src/Cache/DataCache.php @@ -77,4 +77,18 @@ public function set_data(string $key, array $value, ?int $ttl = null): bool; * MUST be thrown if the $key string is not a legal value. */ public function delete_data(string $key): bool; + + /** + * Retrieve the last modified time for the cache + * + * @return int Timestamp + */ + public function mtime(): int; + + /** + * Set the last modified time to the current time + * + * @return bool Success status + */ + public function touch(): bool; } diff --git a/src/SimplePie.php b/src/SimplePie.php index 9cbba0087..277c9ece5 100644 --- a/src/SimplePie.php +++ b/src/SimplePie.php @@ -1634,6 +1634,40 @@ public function enable_exceptions(bool $enable = true) $this->enable_exceptions = $enable; } + /** + * Computes a hash of the raw feed content, + * after having cleaned it from noisy elements such as statistics or comments. + * FreshRSS + * @return string $rss A hash of the cleaned content, or empty string in case of error. + */ + function clean_hash(string $rss): string + { + if ($rss === '') { + return ''; + } + //Process by chunks not to use too much memory + if (($stream = fopen('php://temp', 'r+')) + && fwrite($stream, $rss) + && rewind($stream) + ) { + $ctx = hash_init('sha1'); + while ($stream_data = fread($stream, 1048576)) { + hash_update( + $ctx, preg_replace( + [ + '#<(lastBuildDate|pubDate|updated|feedDate|dc:date|slash:comments)>[^<]+#', + '#<(media:starRating|media:statistics) [^/<>]+/>#', + '##s', + ], '', $stream_data + ) + ); + } + fclose($stream); + return hash_final($ctx); + } + return ''; + } + /** * Initialize the feed object * @@ -1641,7 +1675,7 @@ public function enable_exceptions(bool $enable = true) * configuration options get processed, feeds are fetched, cached, and * parsed, and all of that other good stuff. * - * @return bool True if successful, false otherwise + * @return bool|int positive integer with modification time if using cache, boolean true if otherwise successful, false otherwise // FreshRSS */ public function init() { @@ -1718,6 +1752,7 @@ public function init() $this->check_modified = false; $this->multifeed_objects = []; $cache = false; + $hash = ''; if ($this->feed_url !== null) { $parsed_feed_url = $this->registry->call(Misc::class, 'parse_url', [$this->feed_url]); @@ -1729,12 +1764,15 @@ public function init() // Fetch the data into $this->raw_data if (($fetched = $this->fetch_data($cache)) === true) { - return true; + return empty($this->data['mtime']) ? false : $this->data['mtime']; // FreshRSS } elseif ($fetched === false) { return false; } [$headers, $sniffed] = $fetched; + if (isset($this->data['hash']) && is_string($this->data['hash'])) { // FreshRSS + $hash = $this->data['hash']; + } } // Empty response check @@ -1803,6 +1841,8 @@ public function init() $this->data['headers'] = $headers; } $this->data['build'] = Misc::get_build(); + $this->data['hash'] = $hash === '' ? $this->clean_hash($this->raw_data) : $hash; // FreshRSS + $this->data['mtime'] = time(); // FreshRSS // Cache the file if caching is enabled $this->data['cache_expiration_time'] = $this->cache_duration + time(); @@ -1874,7 +1914,10 @@ protected function fetch_data(&$cache) // Load the Cache $this->data = $cache->get_data($cacheKey, []); - if (!empty($this->data)) { + if ($cache->mtime() + $this->cache_duration > time()) { // FreshRSS + $this->raw_data = false; + return true; // If the cache is still valid, just return true + } elseif (!empty($this->data)) { // If the cache is for an outdated build of SimplePie if (!isset($this->data['build']) || $this->data['build'] !== Misc::get_build()) { $cache->delete_data($cacheKey); @@ -1906,7 +1949,7 @@ protected function fetch_data(&$cache) // when requesting this file. (Note that it's up to the file to // support this, but we don't always send the headers either.) $this->check_modified = true; - if (isset($this->data['headers']['last-modified']) || isset($this->data['headers']['etag'])) { + { // if (isset($this->data['headers']['last-modified']) || isset($this->data['headers']['etag'])) { // FreshRSS removed $headers = [ 'Accept' => SimplePie::DEFAULT_HTTP_ACCEPT_HEADER, ]; @@ -1942,6 +1985,17 @@ protected function fetch_data(&$cache) return true; } } + { // FreshRSS + $hash = $this->clean_hash($file->get_body_content()); + if ($this->data['hash'] === $hash) { + syslog(LOG_DEBUG, 'SimplePie hash cache match for ' . $this->feed_url); + $cache->touch(); + return true; // Content unchanged even though server did not send a 304 + } else { + syslog(LOG_DEBUG, 'SimplePie hash cache no match for ' . $this->feed_url); + $this->data['hash'] = $hash; + } + } } // If the cache is still valid, just return true else { @@ -2069,6 +2123,8 @@ protected function fetch_data(&$cache) 'feed_url' => $file->get_final_requested_uri(), 'build' => Misc::get_build(), 'cache_expiration_time' => $this->cache_duration + time(), + 'hash' => $hash === '' ? $this->clean_hash($file->get_body_content()) : $hash, // FreshRSS + 'mtime' => time(), // FreshRSS ]; if (!$cache->set_data($cacheKey, $this->data, $this->cache_duration)) { @@ -2081,7 +2137,6 @@ protected function fetch_data(&$cache) } $this->raw_data = $file->get_body_content(); - $this->raw_data = trim($this->raw_data); $this->permanent_url = $file->get_permanent_uri(); $headers = []; From f1a5e126c17bfdf1d4c071df3acbe877b39a6dc3 Mon Sep 17 00:00:00 2001 From: Alexandre Alapetite Date: Mon, 19 Aug 2024 23:28:23 +0200 Subject: [PATCH 02/16] Backport fix https://github.com/FreshRSS/FreshRSS/pull/6723 --- src/SimplePie.php | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/SimplePie.php b/src/SimplePie.php index 277c9ece5..bfa4644c8 100644 --- a/src/SimplePie.php +++ b/src/SimplePie.php @@ -1988,11 +1988,12 @@ protected function fetch_data(&$cache) { // FreshRSS $hash = $this->clean_hash($file->get_body_content()); if ($this->data['hash'] === $hash) { - syslog(LOG_DEBUG, 'SimplePie hash cache match for ' . $this->feed_url); - $cache->touch(); + syslog(LOG_DEBUG, 'SimplePie hash cache match for ' . SimplePie_Misc::url_remove_credentials($this->feed_url)); + $this->data['headers'] = $file->get_headers(); + $cache->set_data($cacheKey, $this->data, $this->cache_duration); return true; // Content unchanged even though server did not send a 304 } else { - syslog(LOG_DEBUG, 'SimplePie hash cache no match for ' . $this->feed_url); + syslog(LOG_DEBUG, 'SimplePie hash cache no match for ' . SimplePie_Misc::url_remove_credentials($this->feed_url)); $this->data['hash'] = $hash; } } From 76807b5e2bd016d6198a2bdcc4b33161e7942f29 Mon Sep 17 00:00:00 2001 From: Alexandre Alapetite Date: Mon, 19 Aug 2024 23:57:09 +0200 Subject: [PATCH 03/16] A few fixes --- src/Cache/DataCache.php | 14 -------------- src/SimplePie.php | 14 ++++++++------ 2 files changed, 8 insertions(+), 20 deletions(-) diff --git a/src/Cache/DataCache.php b/src/Cache/DataCache.php index facf2b179..02ec59e9c 100644 --- a/src/Cache/DataCache.php +++ b/src/Cache/DataCache.php @@ -77,18 +77,4 @@ public function set_data(string $key, array $value, ?int $ttl = null): bool; * MUST be thrown if the $key string is not a legal value. */ public function delete_data(string $key): bool; - - /** - * Retrieve the last modified time for the cache - * - * @return int Timestamp - */ - public function mtime(): int; - - /** - * Set the last modified time to the current time - * - * @return bool Success status - */ - public function touch(): bool; } diff --git a/src/SimplePie.php b/src/SimplePie.php index bfa4644c8..6fa79ba7f 100644 --- a/src/SimplePie.php +++ b/src/SimplePie.php @@ -1890,6 +1890,7 @@ public function init() * * @param Base|DataCache|false $cache Cache handler, or false to not load from the cache * @return array{array, string}|array{}|bool Returns true if the data was loaded from the cache, or an array of HTTP headers and sniffed type + * @phpstan-impure */ protected function fetch_data(&$cache) { @@ -1914,7 +1915,7 @@ protected function fetch_data(&$cache) // Load the Cache $this->data = $cache->get_data($cacheKey, []); - if ($cache->mtime() + $this->cache_duration > time()) { // FreshRSS + if (isset($this->data['mtime']) && $this->data['mtime'] + $this->cache_duration > time()) { // FreshRSS $this->raw_data = false; return true; // If the cache is still valid, just return true } elseif (!empty($this->data)) { @@ -1949,7 +1950,8 @@ protected function fetch_data(&$cache) // when requesting this file. (Note that it's up to the file to // support this, but we don't always send the headers either.) $this->check_modified = true; - { // if (isset($this->data['headers']['last-modified']) || isset($this->data['headers']['etag'])) { // FreshRSS removed + // @phpstan-ignore if.alwaysTrue + if (true) { // if (isset($this->data['headers']['last-modified']) || isset($this->data['headers']['etag'])) { // FreshRSS $headers = [ 'Accept' => SimplePie::DEFAULT_HTTP_ACCEPT_HEADER, ]; @@ -1985,15 +1987,15 @@ protected function fetch_data(&$cache) return true; } } - { // FreshRSS + if (isset($file)) { // FreshRSS $hash = $this->clean_hash($file->get_body_content()); if ($this->data['hash'] === $hash) { - syslog(LOG_DEBUG, 'SimplePie hash cache match for ' . SimplePie_Misc::url_remove_credentials($this->feed_url)); + syslog(LOG_DEBUG, 'SimplePie hash cache match for ' . Misc::url_remove_credentials($this->feed_url)); $this->data['headers'] = $file->get_headers(); $cache->set_data($cacheKey, $this->data, $this->cache_duration); return true; // Content unchanged even though server did not send a 304 } else { - syslog(LOG_DEBUG, 'SimplePie hash cache no match for ' . SimplePie_Misc::url_remove_credentials($this->feed_url)); + syslog(LOG_DEBUG, 'SimplePie hash cache no match for ' . Misc::url_remove_credentials($this->feed_url)); $this->data['hash'] = $hash; } } @@ -2124,7 +2126,7 @@ protected function fetch_data(&$cache) 'feed_url' => $file->get_final_requested_uri(), 'build' => Misc::get_build(), 'cache_expiration_time' => $this->cache_duration + time(), - 'hash' => $hash === '' ? $this->clean_hash($file->get_body_content()) : $hash, // FreshRSS + 'hash' => empty($hash) ? $this->clean_hash($file->get_body_content()) : $hash, // FreshRSS 'mtime' => time(), // FreshRSS ]; From 1ade02b4d2d4fd103c724af5750e653cc75b4344 Mon Sep 17 00:00:00 2001 From: Alexandre Alapetite Date: Mon, 19 Aug 2024 23:59:11 +0200 Subject: [PATCH 04/16] Reduce changes --- src/Cache/BaseDataCache.php | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/src/Cache/BaseDataCache.php b/src/Cache/BaseDataCache.php index 1b8fd13fd..60207b075 100644 --- a/src/Cache/BaseDataCache.php +++ b/src/Cache/BaseDataCache.php @@ -116,24 +116,4 @@ public function delete_data(string $key): bool { return $this->cache->unlink(); } - - /** - * Retrieve the last modified time for the cache - * - * @return int Timestamp - */ - public function mtime(): int - { - return $this->cache->mtime(); - } - - /** - * Set the last modified time to the current time - * - * @return bool Success status - */ - public function touch(): bool - { - return $this->cache->touch(); - } } From 9475d0f6240aa5a074b7dd7dff72b3892e8f92d3 Mon Sep 17 00:00:00 2001 From: Alexandre Alapetite Date: Tue, 20 Aug 2024 00:16:52 +0200 Subject: [PATCH 05/16] Reduce changes --- src/SimplePie.php | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/SimplePie.php b/src/SimplePie.php index 6fa79ba7f..ead343035 100644 --- a/src/SimplePie.php +++ b/src/SimplePie.php @@ -1770,9 +1770,6 @@ public function init() } [$headers, $sniffed] = $fetched; - if (isset($this->data['hash']) && is_string($this->data['hash'])) { // FreshRSS - $hash = $this->data['hash']; - } } // Empty response check @@ -1841,7 +1838,7 @@ public function init() $this->data['headers'] = $headers; } $this->data['build'] = Misc::get_build(); - $this->data['hash'] = $hash === '' ? $this->clean_hash($this->raw_data) : $hash; // FreshRSS + $this->data['hash'] = $this->data['hash'] ?? $this->clean_hash($this->raw_data); // FreshRSS $this->data['mtime'] = time(); // FreshRSS // Cache the file if caching is enabled From fcd29cd05b9182b3b9fe24bf09a688e84ec03919 Mon Sep 17 00:00:00 2001 From: Alexandre Alapetite Date: Tue, 20 Aug 2024 00:39:13 +0200 Subject: [PATCH 06/16] Relax some tests --- src/SimplePie.php | 1 - tests/Integration/CachingTest.php | 3 ++- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/SimplePie.php b/src/SimplePie.php index ead343035..0fa324063 100644 --- a/src/SimplePie.php +++ b/src/SimplePie.php @@ -1752,7 +1752,6 @@ public function init() $this->check_modified = false; $this->multifeed_objects = []; $cache = false; - $hash = ''; if ($this->feed_url !== null) { $parsed_feed_url = $this->registry->call(Misc::class, 'parse_url', [$this->feed_url]); diff --git a/tests/Integration/CachingTest.php b/tests/Integration/CachingTest.php index 350410061..a5d3134e5 100644 --- a/tests/Integration/CachingTest.php +++ b/tests/Integration/CachingTest.php @@ -112,7 +112,8 @@ public function testInitWithDifferentCacheStateCallsCacheCorrectly( $expectedDataWritten['cache_expiration_time'] = $writtenData['cache_expiration_time']; } - $this->assertSame($expectedDataWritten, $writtenData); + // Test that $writtenData is a subset of $expectedDataWritten + $this->assertEmpty(array_diff_assoc($expectedDataWritten, $writtenData)); // FreshRSS } /** From 4237d4236bf7fe501a6f9b2e65878a1bcca72c02 Mon Sep 17 00:00:00 2001 From: Alexandre Alapetite Date: Tue, 20 Aug 2024 00:41:00 +0200 Subject: [PATCH 07/16] Fix comment --- tests/Integration/CachingTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Integration/CachingTest.php b/tests/Integration/CachingTest.php index a5d3134e5..33e7f4a1e 100644 --- a/tests/Integration/CachingTest.php +++ b/tests/Integration/CachingTest.php @@ -112,7 +112,7 @@ public function testInitWithDifferentCacheStateCallsCacheCorrectly( $expectedDataWritten['cache_expiration_time'] = $writtenData['cache_expiration_time']; } - // Test that $writtenData is a subset of $expectedDataWritten + // Test that $expectedDataWritten is a subset of $writtenData $this->assertEmpty(array_diff_assoc($expectedDataWritten, $writtenData)); // FreshRSS } From 121ac17f63d568915f0d2465e499c8b7d3f135d9 Mon Sep 17 00:00:00 2001 From: Alexandre Alapetite Date: Tue, 20 Aug 2024 16:29:33 +0200 Subject: [PATCH 08/16] Fix a few tests --- src/SimplePie.php | 4 ++-- tests/Integration/CachingTest.php | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/SimplePie.php b/src/SimplePie.php index 0fa324063..9ecfde432 100644 --- a/src/SimplePie.php +++ b/src/SimplePie.php @@ -1763,7 +1763,7 @@ public function init() // Fetch the data into $this->raw_data if (($fetched = $this->fetch_data($cache)) === true) { - return empty($this->data['mtime']) ? false : $this->data['mtime']; // FreshRSS + return $this->data['mtime'] ?? true; // FreshRSS } elseif ($fetched === false) { return false; } @@ -1985,7 +1985,7 @@ protected function fetch_data(&$cache) } if (isset($file)) { // FreshRSS $hash = $this->clean_hash($file->get_body_content()); - if ($this->data['hash'] === $hash) { + if (($this->data['hash'] ?? null) === $hash) { syslog(LOG_DEBUG, 'SimplePie hash cache match for ' . Misc::url_remove_credentials($this->feed_url)); $this->data['headers'] = $file->get_headers(); $cache->set_data($cacheKey, $this->data, $this->cache_duration); diff --git a/tests/Integration/CachingTest.php b/tests/Integration/CachingTest.php index 33e7f4a1e..b151ad6af 100644 --- a/tests/Integration/CachingTest.php +++ b/tests/Integration/CachingTest.php @@ -113,7 +113,7 @@ public function testInitWithDifferentCacheStateCallsCacheCorrectly( } // Test that $expectedDataWritten is a subset of $writtenData - $this->assertEmpty(array_diff_assoc($expectedDataWritten, $writtenData)); // FreshRSS + $this->assertEmpty(array_udiff_assoc($expectedDataWritten, $writtenData, fn ($a, $b) => json_encode($a) <=> json_encode($b))); // FreshRSS } /** From b722e7693229b338c31e61c9ec4b98ccbcc6c39b Mon Sep 17 00:00:00 2001 From: Alexandre Alapetite Date: Tue, 20 Aug 2024 16:33:11 +0200 Subject: [PATCH 09/16] PHP 7.2 compatibility --- tests/Integration/CachingTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Integration/CachingTest.php b/tests/Integration/CachingTest.php index b151ad6af..1caa0c6b8 100644 --- a/tests/Integration/CachingTest.php +++ b/tests/Integration/CachingTest.php @@ -113,7 +113,7 @@ public function testInitWithDifferentCacheStateCallsCacheCorrectly( } // Test that $expectedDataWritten is a subset of $writtenData - $this->assertEmpty(array_udiff_assoc($expectedDataWritten, $writtenData, fn ($a, $b) => json_encode($a) <=> json_encode($b))); // FreshRSS + $this->assertEmpty(array_udiff_assoc($expectedDataWritten, $writtenData, function ($a, $b) { return json_encode($a) <=> json_encode($b); })); // FreshRSS } /** From 15f2de22800621abb4762d1af8477733c984a494 Mon Sep 17 00:00:00 2001 From: Alexandre Alapetite Date: Wed, 21 Aug 2024 00:25:19 +0200 Subject: [PATCH 10/16] Behaviour fixes --- src/Cache/BaseDataCache.php | 19 ++++++++++--------- src/SimplePie.php | 16 +++++++++------- tests/Unit/Cache/BaseDataCacheTest.php | 21 +++++++++++---------- 3 files changed, 30 insertions(+), 26 deletions(-) diff --git a/src/Cache/BaseDataCache.php b/src/Cache/BaseDataCache.php index 60207b075..effa040ed 100644 --- a/src/Cache/BaseDataCache.php +++ b/src/Cache/BaseDataCache.php @@ -50,15 +50,16 @@ public function get_data(string $key, $default = null) return $default; } - // ignore data if internal cache expiration time is not set - if (!array_key_exists('__cache_expiration_time', $data)) { - return $default; - } - - // ignore data if internal cache expiration time is expired - if ($data['__cache_expiration_time'] < time()) { - return $default; - } + // FreshRSS commented out + // // ignore data if internal cache expiration time is not set + // if (!array_key_exists('__cache_expiration_time', $data)) { + // return $default; + // } + + // // ignore data if internal cache expiration time is expired + // if ($data['__cache_expiration_time'] < time()) { + // return $default; + // } // remove internal cache expiration time unset($data['__cache_expiration_time']); diff --git a/src/SimplePie.php b/src/SimplePie.php index 9ecfde432..959779e2b 100644 --- a/src/SimplePie.php +++ b/src/SimplePie.php @@ -1911,10 +1911,7 @@ protected function fetch_data(&$cache) // Load the Cache $this->data = $cache->get_data($cacheKey, []); - if (isset($this->data['mtime']) && $this->data['mtime'] + $this->cache_duration > time()) { // FreshRSS - $this->raw_data = false; - return true; // If the cache is still valid, just return true - } elseif (!empty($this->data)) { + if (!empty($this->data)) { // If the cache is for an outdated build of SimplePie if (!isset($this->data['build']) || $this->data['build'] !== Misc::get_build()) { $cache->delete_data($cacheKey); @@ -1940,14 +1937,19 @@ protected function fetch_data(&$cache) $cache->delete_data($this->get_cache_filename($this->feed_url)); $this->data = []; } + // If the cache is still valid, just return true // FreshRSS + elseif (isset($this->data['mtime']) && $this->data['mtime'] + $this->cache_duration > time()) { + $this->raw_data = false; + return true; + } // Check if the cache has been updated - elseif (isset($this->data['cache_expiration_time']) && $this->data['cache_expiration_time'] > time()) { + // @phpstan-ignore elseif.alwaysTrue + elseif (true) { // isset($this->data['cache_expiration_time']) && $this->data['cache_expiration_time'] > time()) { // FreshRSS // Want to know if we tried to send last-modified and/or etag headers // when requesting this file. (Note that it's up to the file to // support this, but we don't always send the headers either.) $this->check_modified = true; - // @phpstan-ignore if.alwaysTrue - if (true) { // if (isset($this->data['headers']['last-modified']) || isset($this->data['headers']['etag'])) { // FreshRSS + if (isset($this->data['headers']['last-modified']) || isset($this->data['headers']['etag'])) { $headers = [ 'Accept' => SimplePie::DEFAULT_HTTP_ACCEPT_HEADER, ]; diff --git a/tests/Unit/Cache/BaseDataCacheTest.php b/tests/Unit/Cache/BaseDataCacheTest.php index e38c92f7d..24a5e8182 100644 --- a/tests/Unit/Cache/BaseDataCacheTest.php +++ b/tests/Unit/Cache/BaseDataCacheTest.php @@ -69,19 +69,20 @@ public function testGetDataWithCacheMissReturnsDefault(): void $this->assertSame($default, $cache->get_data($key, $default)); } - public function testGetDataWithCacheExpiredReturnsDefault(): void - { - $key = 'name'; - $cachedValue = ['__cache_expiration_time' => 0]; - $default = new stdClass(); + // FreshRSS commented out + // public function testGetDataWithCacheExpiredReturnsDefault(): void + // { + // $key = 'name'; + // $cachedValue = ['__cache_expiration_time' => 0]; + // $default = new stdClass(); - $baseCache = $this->createMock(Base::class); - $baseCache->expects($this->once())->method('load')->willReturn($cachedValue); + // $baseCache = $this->createMock(Base::class); + // $baseCache->expects($this->once())->method('load')->willReturn($cachedValue); - $cache = new BaseDataCache($baseCache); + // $cache = new BaseDataCache($baseCache); - $this->assertSame($default, $cache->get_data($key, $default)); - } + // $this->assertSame($default, $cache->get_data($key, $default)); + // } public function testGetDataWithCacheCorruptionReturnsDefault(): void { From 0b931d4249684ff85febaefa7c0a35e6a8442751 Mon Sep 17 00:00:00 2001 From: Alexandre Alapetite Date: Wed, 21 Aug 2024 00:29:49 +0200 Subject: [PATCH 11/16] Simplification --- src/SimplePie.php | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/SimplePie.php b/src/SimplePie.php index 959779e2b..d952cd6fc 100644 --- a/src/SimplePie.php +++ b/src/SimplePie.php @@ -1937,14 +1937,9 @@ protected function fetch_data(&$cache) $cache->delete_data($this->get_cache_filename($this->feed_url)); $this->data = []; } - // If the cache is still valid, just return true // FreshRSS - elseif (isset($this->data['mtime']) && $this->data['mtime'] + $this->cache_duration > time()) { - $this->raw_data = false; - return true; - } // Check if the cache has been updated - // @phpstan-ignore elseif.alwaysTrue - elseif (true) { // isset($this->data['cache_expiration_time']) && $this->data['cache_expiration_time'] > time()) { // FreshRSS + // elseif (isset($this->data['cache_expiration_time']) && $this->data['cache_expiration_time'] > time()) { // FreshRSS + elseif (empty($this->data['mtime']) || $this->data['mtime'] + $this->cache_duration <= time()) { // Want to know if we tried to send last-modified and/or etag headers // when requesting this file. (Note that it's up to the file to // support this, but we don't always send the headers either.) From 1400fc7ce35bf79e1061219d433c9a2d7f431b7a Mon Sep 17 00:00:00 2001 From: Alexandre Alapetite Date: Wed, 21 Aug 2024 00:36:18 +0200 Subject: [PATCH 12/16] Comment --- src/Cache/BaseDataCache.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Cache/BaseDataCache.php b/src/Cache/BaseDataCache.php index effa040ed..a287e78f1 100644 --- a/src/Cache/BaseDataCache.php +++ b/src/Cache/BaseDataCache.php @@ -50,7 +50,7 @@ public function get_data(string $key, $default = null) return $default; } - // FreshRSS commented out + // FreshRSS commented out, to allow HTTP 304 // // ignore data if internal cache expiration time is not set // if (!array_key_exists('__cache_expiration_time', $data)) { // return $default; From b1504de68ab207a9e15862cc03a863ccefb5bf7c Mon Sep 17 00:00:00 2001 From: Alexandre Alapetite Date: Wed, 21 Aug 2024 22:34:09 +0200 Subject: [PATCH 13/16] Fix tests --- src/Cache/BaseDataCache.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Cache/BaseDataCache.php b/src/Cache/BaseDataCache.php index a287e78f1..bb1b8c5fa 100644 --- a/src/Cache/BaseDataCache.php +++ b/src/Cache/BaseDataCache.php @@ -50,12 +50,12 @@ public function get_data(string $key, $default = null) return $default; } - // FreshRSS commented out, to allow HTTP 304 - // // ignore data if internal cache expiration time is not set - // if (!array_key_exists('__cache_expiration_time', $data)) { - // return $default; - // } + // ignore data if internal cache expiration time is not set + if (!array_key_exists('__cache_expiration_time', $data)) { + return $default; + } + // FreshRSS commented out, to allow HTTP 304 // // ignore data if internal cache expiration time is expired // if ($data['__cache_expiration_time'] < time()) { // return $default; From 82ae104433c0974fbcec1746e28fc017f146d18b Mon Sep 17 00:00:00 2001 From: Alexandre Alapetite Date: Thu, 22 Aug 2024 09:24:43 +0200 Subject: [PATCH 14/16] Remove debug logs --- src/SimplePie.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/SimplePie.php b/src/SimplePie.php index d952cd6fc..72b70eeb9 100644 --- a/src/SimplePie.php +++ b/src/SimplePie.php @@ -1983,12 +1983,10 @@ protected function fetch_data(&$cache) if (isset($file)) { // FreshRSS $hash = $this->clean_hash($file->get_body_content()); if (($this->data['hash'] ?? null) === $hash) { - syslog(LOG_DEBUG, 'SimplePie hash cache match for ' . Misc::url_remove_credentials($this->feed_url)); $this->data['headers'] = $file->get_headers(); $cache->set_data($cacheKey, $this->data, $this->cache_duration); return true; // Content unchanged even though server did not send a 304 } else { - syslog(LOG_DEBUG, 'SimplePie hash cache no match for ' . Misc::url_remove_credentials($this->feed_url)); $this->data['hash'] = $hash; } } From 9f09ec3f8cdfb68c7ecbc47c39b7e8c12eced947 Mon Sep 17 00:00:00 2001 From: Alexandre Alapetite Date: Thu, 22 Aug 2024 16:13:43 +0200 Subject: [PATCH 15/16] Minor comment --- tests/Unit/Cache/BaseDataCacheTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Unit/Cache/BaseDataCacheTest.php b/tests/Unit/Cache/BaseDataCacheTest.php index 24a5e8182..718a1b5f2 100644 --- a/tests/Unit/Cache/BaseDataCacheTest.php +++ b/tests/Unit/Cache/BaseDataCacheTest.php @@ -69,7 +69,7 @@ public function testGetDataWithCacheMissReturnsDefault(): void $this->assertSame($default, $cache->get_data($key, $default)); } - // FreshRSS commented out + // FreshRSS commented out, to allow HTTP 304 // public function testGetDataWithCacheExpiredReturnsDefault(): void // { // $key = 'name'; From 04e66e0ef75ceb256c361f5614f4c2d27e09bdb3 Mon Sep 17 00:00:00 2001 From: Alexandre Alapetite Date: Sun, 8 Sep 2024 18:28:23 +0200 Subject: [PATCH 16/16] Fix type mess with $this->data['headers'] --- src/SimplePie.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/SimplePie.php b/src/SimplePie.php index 72b70eeb9..72bde2b38 100644 --- a/src/SimplePie.php +++ b/src/SimplePie.php @@ -1983,7 +1983,9 @@ protected function fetch_data(&$cache) if (isset($file)) { // FreshRSS $hash = $this->clean_hash($file->get_body_content()); if (($this->data['hash'] ?? null) === $hash) { - $this->data['headers'] = $file->get_headers(); + $this->data['headers'] = array_map(function (array $values): string { + return implode(',', $values); + }, $file->get_headers()); $cache->set_data($cacheKey, $this->data, $this->cache_duration); return true; // Content unchanged even though server did not send a 304 } else {