Skip to content

Commit

Permalink
Pragmatism?
Browse files Browse the repository at this point in the history
  • Loading branch information
@Alkarex
Alkarex committed Sep 16, 2024
1 parent f8e3f92 commit b72dd25
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 29 deletions.
2 changes: 0 additions & 2 deletions src/Cache/BaseDataCache.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,6 @@ public function set_data(string $key, array $value, ?int $ttl = null): bool
{
if ($ttl === null) {
$ttl = 3600;
} elseif ($ttl <= 0) {
return $this->delete_data($key); // FreshRSS
}

// place internal cache expiration time
Expand Down
22 changes: 10 additions & 12 deletions src/HTTP/Utils.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,15 @@ final class Utils
* Negotiate the cache expiration time based on the HTTP response headers.
* Return the cache duration time in number of seconds since the Unix Epoch, accounting for:
* - `Cache-Control: max-age` minus `Age`, bounded by `$cache_duration_min` and `$cache_duration_max`
* - `Cache-Control: must-revalidate` will prevent `$cache_duration_min` from extending past the `max-age`
* - `Cache-Control: no-cache` will return the current time
* - `Cache-Control: no-store` will return `0`
* - `Cache-Control: must-revalidate` will set `$cache_duration` to `$cache_duration_min`
* - `Cache-Control: no-cache` will return `time() + $cache_duration_min`
* - `Cache-Control: no-store` will return `time() + $cache_duration_min - 3`
* - `Expires` like `Cache-Control: max-age` but only if it is absent
*
* @param array<string,mixed> $http_headers HTTP headers of the response
* @param int $cache_duration Desired cache duration in seconds, potentially overridden by HTTP response headers
* @param int $cache_duration_min Minimal cache duration (in seconds), overriding HTTP response headers `Cache-Control: max-age` and `Expires`,
* but without effect on `Cache-Control: no-store` and `Cache-Control: no-cache` and `Cache-Control: must-revalidate`
* @param int $cache_duration_min Minimal cache duration (in seconds), overriding HTTP response headers `Cache-Control` and `Expires`,
* @param int $cache_duration_max Maximal cache duration (in seconds), overriding HTTP response headers `Cache-Control: max-age` and `Expires`,
* but without effect on `Cache-Control: no-store` and `Cache-Control: no-cache`
* @return int The negotiated cache expiration time in seconds since the Unix Epoch
*
* FreshRSS
Expand All @@ -35,30 +33,30 @@ public static function negociate_cache_expiration_time(array $http_headers, int
$cache_control = $http_headers['cache-control'] ?? '';
if ($cache_control !== '') {
if (preg_match('/\bno-store\b/', $cache_control)) {
return 0;
return time() + $cache_duration_min - 3; // -3 to distinguish from no-cache if needed
}
if (preg_match('/\bno-cache\b/', $cache_control)) {
return time();
return time() + $cache_duration_min;
}
if (preg_match('/\bmust-revalidate\b/', $cache_control)) {
$cache_duration_min = 0;
$cache_duration = $cache_duration_min;
}
if (preg_match('/\bs-maxage=(\d+)\b/', $cache_control, $matches) || preg_match('/\bmax-age=(\d+)\b/', $cache_control, $matches)) {
$max_age = (int) $matches[1];
$age = $http_headers['age'] ?? '';
if (is_numeric($age)) {
$max_age -= (int) $age;
}
return time() + min(max($max_age, $cache_duration_min), $cache_duration_max);
return time() + min(max($max_age, $cache_duration), $cache_duration_max);
}
}
$expires = $http_headers['expires'] ?? '';
if ($expires !== '') {
$expire_date = \SimplePie\Misc::parse_date($expires);
if ($expire_date !== false) {
return min(max($expire_date, time() + $cache_duration_min), time() + $cache_duration_max);
return min(max($expire_date, time() + $cache_duration), time() + $cache_duration_max);
}
}
return time() + min(max($cache_duration, $cache_duration_min), $cache_duration_max);
return time() + $cache_duration;
}
}
26 changes: 11 additions & 15 deletions src/SimplePie.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -516,17 +516,15 @@ class SimplePie
public $cache_duration = 3600;

/**
* @var int Minimal cache duration (in seconds), overriding HTTP response headers `Cache-Control: max-age` and `Expires`,
* but without effect on `Cache-Control: no-store` and `Cache-Control: no-cache` and `Cache-Control: must-revalidate`
* @var int Minimal cache duration (in seconds), overriding HTTP response headers `Cache-Control` and `Expires`,
* @see SimplePie::set_cache_duration()
* @access private
* FreshRSS
*/
public $cache_duration_min = 60;

/**
* @var int Maximal cache duration (in seconds), overriding HTTP response headers `Cache-Control: max-age` and `Expires`,
* but without effect on `Cache-Control: no-store` and `Cache-Control: no-cache`
* @var int Maximal cache duration (in seconds), overriding HTTP response headers `Cache-Control` and `Expires`,
* @see SimplePie::set_cache_duration()
* @access private
* FreshRSS
Expand Down Expand Up @@ -1010,22 +1008,20 @@ public function force_cache_fallback(bool $enable = false)
* FreshRSS: The cache is (partially) HTTP compliant, with the following rules:
*
* @param int $seconds The feed content cache duration, which may be overridden by HTTP response headers)
* @param int $min The minimun cache duration (default: 60s), overriding HTTP response headers `Cache-Control: max-age` and `Expires`,
* but without effect on `Cache-Control: no-store` and `Cache-Control: no-cache` and `Cache-Control: must-revalidate`
* @param int $max The maximum cache duration (default: 24h), overriding HTTP response headers `Cache-Control: max-age` and `Expires`,
* but without effect on `Cache-Control: no-store` and `Cache-Control: no-cache`
* @param int $min The minimun cache duration (default: 60s), overriding HTTP response headers `Cache-Control` and `Expires`,

This comment has been minimized.

Sign in to view

Copy link
@Frenzie

Frenzie Sep 16, 2024

Member

typo :)
* @param int $max The maximum cache duration (default: 24h), overriding HTTP response headers `Cache-Control` and `Expires`,
* @return void
*/
public function set_cache_duration(int $seconds = 3600, ?int $min = null, ?int $max = null)
{
$this->cache_duration = $seconds;
$this->cache_duration = max(0, $seconds);
if (is_int($min)) { // FreshRSS
$this->cache_duration_min = $min;
$this->cache_duration_min = min(max(0, $min), $seconds);
} elseif ($this->cache_duration_min > $seconds) {
$this->cache_duration_min = $seconds;
}
if (is_int($max)) { // FreshRSS
$this->cache_duration_max = $max;
$this->cache_duration_max = max($seconds, $max);
} elseif ($this->cache_duration_max < $seconds) {
$this->cache_duration_max = $seconds;
}
Expand Down Expand Up @@ -2007,7 +2003,7 @@ protected function fetch_data(&$cache)

if ($this->force_cache_fallback) {
$this->data['cache_expiration_time'] = \SimplePie\HTTP\Utils::negociate_cache_expiration_time($this->data['headers'] ?? [], $this->cache_duration, $this->cache_duration_min, $this->cache_duration_max); // FreshRSS
if (!$cache->set_data($cacheKey, $this->data, $this->data['cache_expiration_time'] <= 0 ? 0 : $this->cache_duration_max)) { // FreshRSS
if (!$cache->set_data($cacheKey, $this->data, $this->cache_duration)) { // FreshRSS
trigger_error("$this->cache_location is not writable. Make sure you've set the correct relative or absolute path, and that the location is server-writable.", E_USER_WARNING);
}

Expand All @@ -2028,7 +2024,7 @@ protected function fetch_data(&$cache)
return implode(',', $values);
}, $file->get_headers());
}
if (!$cache->set_data($cacheKey, $this->data, ($this->data['cache_expiration_time'] ?? 1) <= 0 ? 0 : $this->cache_duration_max)) { // FreshRSS
if (!$cache->set_data($cacheKey, $this->data, $this->cache_duration)) { // FreshRSS
trigger_error("$this->cache_location is not writable. Make sure you've set the correct relative or absolute path, and that the location is server-writable.", E_USER_WARNING);
}

Expand All @@ -2043,7 +2039,7 @@ protected function fetch_data(&$cache)
$this->data['headers'] = array_map(function (array $values): string {
return implode(',', $values);
}, $file->get_headers());
if (!$cache->set_data($cacheKey, $this->data, $this->data['cache_expiration_time'] <= 0 ? 0 : $this->cache_duration_max)) { // FreshRSS
if (!$cache->set_data($cacheKey, $this->data, $this->cache_duration)) { // FreshRSS
trigger_error("$this->cache_location is not writable. Make sure you've set the correct relative or absolute path, and that the location is server-writable.", E_USER_WARNING);
}

Expand Down Expand Up @@ -2183,7 +2179,7 @@ protected function fetch_data(&$cache)
'hash' => empty($hash) ? $this->clean_hash($file->get_body_content()) : $hash, // FreshRSS
];

if (!$cache->set_data($cacheKey, $this->data, $this->data['cache_expiration_time'] <= 0 ? 0 : $this->cache_duration_max)) { // FreshRSS
if (!$cache->set_data($cacheKey, $this->data, $this->cache_duration)) { // FreshRSS
trigger_error("$this->cache_location is not writable. Make sure you've set the correct relative or absolute path, and that the location is server-writable.", E_USER_WARNING);
}
}
Expand Down

0 comments on commit b72dd25

Please sign in to comment.