refactor: extract exception and cache middleware (#4248)

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;
     }
 

index.php

@@ -63,13 +63,8 @@ if ($argv) {
     $request = Request::fromGlobals();
 }
 
-try {
+$rssBridge = new RssBridge($container);
-    $rssBridge = new RssBridge($container);
+
-    $response = $rssBridge->main($request);
+$response = $rssBridge->main($request);
-    $response->send();
+
-} catch (\Throwable $e) {
+$response->send();
-    // 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();
-}

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');
         }

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));
     }
 }

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');
+    $cache = $cacheFactory->create(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);
-    }
     return $cache;
 };
 

middlewares/CacheMiddleware.php

@@ -0,0 +1,59 @@
+<?php
+
+declare(strict_types=1);
+
+class CacheMiddleware implements Middleware
+{
+    private CacheInterface $cache;
+
+    public function __construct(CacheInterface $cache)
+    {
+        $this->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;
+    }
+}

middlewares/ExceptionMiddleware.php

@@ -0,0 +1,24 @@
+<?php
+
+declare(strict_types=1);
+
+class ExceptionMiddleware implements Middleware
+{
+    private Logger $logger;
+
+    public function __construct(Logger $logger)
+    {
+        $this->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);
+        }
+    }
+}

templates/html-format.html.php

@@ -22,7 +22,6 @@
 </head>
 
 <body>
-
     <div class="container">
 
         <h1 class="pagetitle">