From a6bdc322b01de91b7a9e27243b30a65caf2d3c5a Mon Sep 17 00:00:00 2001 From: Dag Date: Sun, 1 Sep 2024 21:48:14 +0200 Subject: [PATCH] refactor: extract exception and cache middleware (#4248) --- actions/DisplayAction.php | 36 +----------------- index.php | 15 +++----- lib/Configuration.php | 4 ++ lib/RssBridge.php | 4 +- lib/dependencies.php | 12 +----- middlewares/CacheMiddleware.php | 59 +++++++++++++++++++++++++++++ middlewares/ExceptionMiddleware.php | 24 ++++++++++++ templates/html-format.html.php | 1 - 8 files changed, 99 insertions(+), 56 deletions(-) create mode 100644 middlewares/CacheMiddleware.php create mode 100644 middlewares/ExceptionMiddleware.php diff --git a/actions/DisplayAction.php b/actions/DisplayAction.php index bda45558..845bfd84 100644 --- a/actions/DisplayAction.php +++ b/actions/DisplayAction.php @@ -22,25 +22,6 @@ class DisplayAction implements ActionInterface $format = $request->get('format'); $noproxy = $request->get('_noproxy'); - $cacheKey = 'http_' . json_encode($request->toArray()); - /** @var Response $cachedResponse */ - $cachedResponse = $this->cache->get($cacheKey); - if ($cachedResponse) { - $ifModifiedSince = $request->server('HTTP_IF_MODIFIED_SINCE'); - $lastModified = $cachedResponse->getHeader('last-modified'); - if ($ifModifiedSince && $lastModified) { - $lastModified = new \DateTimeImmutable($lastModified); - $lastModifiedTimestamp = $lastModified->getTimestamp(); - $modifiedSince = strtotime($ifModifiedSince); - // TODO: \DateTimeImmutable can be compared directly - if ($lastModifiedTimestamp <= $modifiedSince) { - $modificationTimeGMT = gmdate('D, d M Y H:i:s ', $lastModifiedTimestamp); - return new Response('', 304, ['last-modified' => $modificationTimeGMT . 'GMT']); - } - } - return $cachedResponse->withHeader('rss-bridge', 'This is a cached response'); - } - if (!$bridgeName) { return new Response(render(__DIR__ . '/../templates/error.html.php', ['message' => 'Missing bridge parameter']), 400); } @@ -66,6 +47,8 @@ class DisplayAction implements ActionInterface define('NOPROXY', true); } + $cacheKey = 'http_' . json_encode($request->toArray()); + $bridge = $this->bridgeFactory->create($bridgeClassName); $response = $this->createResponse($request, $bridge, $format); @@ -80,21 +63,6 @@ class DisplayAction implements ActionInterface $this->cache->set($cacheKey, $response, $ttl); } - if (in_array($response->getCode(), [403, 429, 503])) { - // Cache these responses for about ~20 mins on average - $this->cache->set($cacheKey, $response, 60 * 15 + rand(1, 60 * 10)); - } - - if ($response->getCode() === 500) { - $this->cache->set($cacheKey, $response, 60 * 15); - } - - // For 1% of requests, prune cache - if (rand(1, 100) === 1) { - // This might be resource intensive! - $this->cache->prune(); - } - return $response; } diff --git a/index.php b/index.php index e67b0c9f..ec7490d6 100644 --- a/index.php +++ b/index.php @@ -63,13 +63,8 @@ if ($argv) { $request = Request::fromGlobals(); } -try { - $rssBridge = new RssBridge($container); - $response = $rssBridge->main($request); - $response->send(); -} catch (\Throwable $e) { - // Probably an exception inside an action - $logger->error('Exception in RssBridge::main()', ['e' => $e]); - $response = new Response(render(__DIR__ . '/templates/exception.html.php', ['e' => $e]), 500); - $response->send(); -} +$rssBridge = new RssBridge($container); + +$response = $rssBridge->main($request); + +$response->send(); \ No newline at end of file diff --git a/lib/Configuration.php b/lib/Configuration.php index 44fd3612..187848fb 100644 --- a/lib/Configuration.php +++ b/lib/Configuration.php @@ -82,6 +82,10 @@ final class Configuration } } + if (Debug::isEnabled()) { + self::setConfig('cache', 'type', 'array'); + } + if (!is_array(self::getConfig('system', 'enabled_bridges'))) { self::throwConfigError('system', 'enabled_bridges', 'Is not an array'); } diff --git a/lib/RssBridge.php b/lib/RssBridge.php index 5e90fb13..c7b132d6 100644 --- a/lib/RssBridge.php +++ b/lib/RssBridge.php @@ -23,6 +23,8 @@ final class RssBridge $handler = $this->container[$actionName]; $middlewares = [ + new CacheMiddleware($this->container['cache']), + new ExceptionMiddleware($this->container['logger']), new SecurityMiddleware(), new MaintenanceMiddleware(), new BasicAuthMiddleware(), @@ -34,6 +36,6 @@ final class RssBridge foreach (array_reverse($middlewares) as $middleware) { $action = fn ($req) => $middleware($req, $action); } - return $action($request); + return $action($request->withAttribute('action', $actionName)); } } diff --git a/lib/dependencies.php b/lib/dependencies.php index 227a66f1..45ae5d61 100644 --- a/lib/dependencies.php +++ b/lib/dependencies.php @@ -56,22 +56,14 @@ $container['logger'] = function () { // $logger->addHandler(new StreamHandler('/tmp/rss-bridge.txt', Logger::INFO)); // Uncomment this for debug logging to fs - //$logger->addHandler(new StreamHandler('/tmp/rss-bridge-debug.txt', Logger::DEBUG)); + // $logger->addHandler(new StreamHandler('/tmp/rss-bridge-debug.txt', Logger::DEBUG)); return $logger; }; $container['cache'] = function ($c) { /** @var CacheFactory $cacheFactory */ $cacheFactory = $c['cache_factory']; - $type = Configuration::getConfig('cache', 'type'); - if (!$type) { - throw new \Exception('No cache type configured'); - } - if (Debug::isEnabled()) { - $cache = $cacheFactory->create('array'); - } else { - $cache = $cacheFactory->create($type); - } + $cache = $cacheFactory->create(Configuration::getConfig('cache', 'type')); return $cache; }; diff --git a/middlewares/CacheMiddleware.php b/middlewares/CacheMiddleware.php new file mode 100644 index 00000000..ae0d0d33 --- /dev/null +++ b/middlewares/CacheMiddleware.php @@ -0,0 +1,59 @@ +cache = $cache; + } + + public function __invoke(Request $request, $next): Response + { + $action = $request->attribute('action'); + + if ($action !== 'DisplayAction') { + // We only cache DisplayAction (for now) + return $next($request); + } + + // TODO: might want to remove som params from query + $cacheKey = 'http_' . json_encode($request->toArray()); + $cachedResponse = $this->cache->get($cacheKey); + + if ($cachedResponse) { + $ifModifiedSince = $request->server('HTTP_IF_MODIFIED_SINCE'); + $lastModified = $cachedResponse->getHeader('last-modified'); + if ($ifModifiedSince && $lastModified) { + $lastModified = new \DateTimeImmutable($lastModified); + $lastModifiedTimestamp = $lastModified->getTimestamp(); + $modifiedSince = strtotime($ifModifiedSince); + // TODO: \DateTimeImmutable can be compared directly + if ($lastModifiedTimestamp <= $modifiedSince) { + $modificationTimeGMT = gmdate('D, d M Y H:i:s ', $lastModifiedTimestamp); + return new Response('', 304, ['last-modified' => $modificationTimeGMT . 'GMT']); + } + } + return $cachedResponse; + } + + /** @var Response $response */ + $response = $next($request); + + if (in_array($response->getCode(), [403, 429, 500, 503])) { + // Cache these responses for about ~20 mins on average + $this->cache->set($cacheKey, $response, 60 * 15 + rand(1, 60 * 10)); + } + + // For 1% of requests, prune cache + if (rand(1, 100) === 1) { + // This might be resource intensive! + $this->cache->prune(); + } + + return $response; + } +} \ No newline at end of file diff --git a/middlewares/ExceptionMiddleware.php b/middlewares/ExceptionMiddleware.php new file mode 100644 index 00000000..8bb74713 --- /dev/null +++ b/middlewares/ExceptionMiddleware.php @@ -0,0 +1,24 @@ +logger = $logger; + } + + public function __invoke(Request $request, $next): Response + { + try { + return $next($request); + } catch (\Throwable $e) { + $this->logger->error('Exception in ExceptionMiddleware', ['e' => $e]); + + return new Response(render(__DIR__ . '/../templates/exception.html.php', ['e' => $e]), 500); + } + } +} \ No newline at end of file diff --git a/templates/html-format.html.php b/templates/html-format.html.php index 8e8abfcf..a62bb16a 100644 --- a/templates/html-format.html.php +++ b/templates/html-format.html.php @@ -22,7 +22,6 @@ -