Skip to content

Commit a6bdc32

Browse files
dvikandvikan
authored
refactor: extract exception and cache middleware (RSS-Bridge#4248)
1 parent 36fd72c commit a6bdc32

8 files changed

+99
-56
lines changed

‎actions/DisplayAction.php

+2-34
Original file line numberDiff line numberDiff line change
@@ -22,25 +22,6 @@ public function __invoke(Request $request): Response
2222
$format = $request->get('format');
2323
$noproxy = $request->get('_noproxy');
2424

25-
$cacheKey = 'http_' . json_encode($request->toArray());
26-
/** @var Response $cachedResponse */
27-
$cachedResponse = $this->cache->get($cacheKey);
28-
if ($cachedResponse) {
29-
$ifModifiedSince = $request->server('HTTP_IF_MODIFIED_SINCE');
30-
$lastModified = $cachedResponse->getHeader('last-modified');
31-
if ($ifModifiedSince && $lastModified) {
32-
$lastModified = new \DateTimeImmutable($lastModified);
33-
$lastModifiedTimestamp = $lastModified->getTimestamp();
34-
$modifiedSince = strtotime($ifModifiedSince);
35-
// TODO: \DateTimeImmutable can be compared directly
36-
if ($lastModifiedTimestamp <= $modifiedSince) {
37-
$modificationTimeGMT = gmdate('D, d M Y H:i:s ', $lastModifiedTimestamp);
38-
return new Response('', 304, ['last-modified' => $modificationTimeGMT . 'GMT']);
39-
}
40-
}
41-
return $cachedResponse->withHeader('rss-bridge', 'This is a cached response');
42-
}
43-
4425
if (!$bridgeName) {
4526
return new Response(render(__DIR__ . '/../templates/error.html.php', ['message' => 'Missing bridge parameter']), 400);
4627
}
@@ -66,6 +47,8 @@ public function __invoke(Request $request): Response
6647
define('NOPROXY', true);
6748
}
6849

50+
$cacheKey = 'http_' . json_encode($request->toArray());
51+
6952
$bridge = $this->bridgeFactory->create($bridgeClassName);
7053

7154
$response = $this->createResponse($request, $bridge, $format);
@@ -80,21 +63,6 @@ public function __invoke(Request $request): Response
8063
$this->cache->set($cacheKey, $response, $ttl);
8164
}
8265

83-
if (in_array($response->getCode(), [403, 429, 503])) {
84-
// Cache these responses for about ~20 mins on average
85-
$this->cache->set($cacheKey, $response, 60 * 15 + rand(1, 60 * 10));
86-
}
87-
88-
if ($response->getCode() === 500) {
89-
$this->cache->set($cacheKey, $response, 60 * 15);
90-
}
91-
92-
// For 1% of requests, prune cache
93-
if (rand(1, 100) === 1) {
94-
// This might be resource intensive!
95-
$this->cache->prune();
96-
}
97-
9866
return $response;
9967
}
10068

‎index.php

+5-10
Original file line numberDiff line numberDiff line change
@@ -63,13 +63,8 @@
6363
$request = Request::fromGlobals();
6464
}
6565

66-
try {
67-
$rssBridge = new RssBridge($container);
68-
$response = $rssBridge->main($request);
69-
$response->send();
70-
} catch (\Throwable $e) {
71-
// Probably an exception inside an action
72-
$logger->error('Exception in RssBridge::main()', ['e' => $e]);
73-
$response = new Response(render(__DIR__ . '/templates/exception.html.php', ['e' => $e]), 500);
74-
$response->send();
75-
}
66+
$rssBridge = new RssBridge($container);
67+
68+
$response = $rssBridge->main($request);
69+
70+
$response->send();

‎lib/Configuration.php

+4
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,10 @@ public static function loadConfiguration(array $customConfig = [], array $env =
8282
}
8383
}
8484

85+
if (Debug::isEnabled()) {
86+
self::setConfig('cache', 'type', 'array');
87+
}
88+
8589
if (!is_array(self::getConfig('system', 'enabled_bridges'))) {
8690
self::throwConfigError('system', 'enabled_bridges', 'Is not an array');
8791
}

‎lib/RssBridge.php

+3-1
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ public function main(Request $request): Response
2323
$handler = $this->container[$actionName];
2424

2525
$middlewares = [
26+
new CacheMiddleware($this->container['cache']),
27+
new ExceptionMiddleware($this->container['logger']),
2628
new SecurityMiddleware(),
2729
new MaintenanceMiddleware(),
2830
new BasicAuthMiddleware(),
@@ -34,6 +36,6 @@ public function main(Request $request): Response
3436
foreach (array_reverse($middlewares) as $middleware) {
3537
$action = fn ($req) => $middleware($req, $action);
3638
}
37-
return $action($request);
39+
return $action($request->withAttribute('action', $actionName));
3840
}
3941
}

‎lib/dependencies.php

+2-10
Original file line numberDiff line numberDiff line change
@@ -56,22 +56,14 @@
5656
// $logger->addHandler(new StreamHandler('/tmp/rss-bridge.txt', Logger::INFO));
5757

5858
// Uncomment this for debug logging to fs
59-
//$logger->addHandler(new StreamHandler('/tmp/rss-bridge-debug.txt', Logger::DEBUG));
59+
// $logger->addHandler(new StreamHandler('/tmp/rss-bridge-debug.txt', Logger::DEBUG));
6060
return $logger;
6161
};
6262

6363
$container['cache'] = function ($c) {
6464
/** @var CacheFactory $cacheFactory */
6565
$cacheFactory = $c['cache_factory'];
66-
$type = Configuration::getConfig('cache', 'type');
67-
if (!$type) {
68-
throw new \Exception('No cache type configured');
69-
}
70-
if (Debug::isEnabled()) {
71-
$cache = $cacheFactory->create('array');
72-
} else {
73-
$cache = $cacheFactory->create($type);
74-
}
66+
$cache = $cacheFactory->create(Configuration::getConfig('cache', 'type'));
7567
return $cache;
7668
};
7769

‎middlewares/CacheMiddleware.php

+59
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
class CacheMiddleware implements Middleware
6+
{
7+
private CacheInterface $cache;
8+
9+
public function __construct(CacheInterface $cache)
10+
{
11+
$this->cache = $cache;
12+
}
13+
14+
public function __invoke(Request $request, $next): Response
15+
{
16+
$action = $request->attribute('action');
17+
18+
if ($action !== 'DisplayAction') {
19+
// We only cache DisplayAction (for now)
20+
return $next($request);
21+
}
22+
23+
// TODO: might want to remove som params from query
24+
$cacheKey = 'http_' . json_encode($request->toArray());
25+
$cachedResponse = $this->cache->get($cacheKey);
26+
27+
if ($cachedResponse) {
28+
$ifModifiedSince = $request->server('HTTP_IF_MODIFIED_SINCE');
29+
$lastModified = $cachedResponse->getHeader('last-modified');
30+
if ($ifModifiedSince && $lastModified) {
31+
$lastModified = new \DateTimeImmutable($lastModified);
32+
$lastModifiedTimestamp = $lastModified->getTimestamp();
33+
$modifiedSince = strtotime($ifModifiedSince);
34+
// TODO: \DateTimeImmutable can be compared directly
35+
if ($lastModifiedTimestamp <= $modifiedSince) {
36+
$modificationTimeGMT = gmdate('D, d M Y H:i:s ', $lastModifiedTimestamp);
37+
return new Response('', 304, ['last-modified' => $modificationTimeGMT . 'GMT']);
38+
}
39+
}
40+
return $cachedResponse;
41+
}
42+
43+
/** @var Response $response */
44+
$response = $next($request);
45+
46+
if (in_array($response->getCode(), [403, 429, 500, 503])) {
47+
// Cache these responses for about ~20 mins on average
48+
$this->cache->set($cacheKey, $response, 60 * 15 + rand(1, 60 * 10));
49+
}
50+
51+
// For 1% of requests, prune cache
52+
if (rand(1, 100) === 1) {
53+
// This might be resource intensive!
54+
$this->cache->prune();
55+
}
56+
57+
return $response;
58+
}
59+
}

‎middlewares/ExceptionMiddleware.php

+24
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
class ExceptionMiddleware implements Middleware
6+
{
7+
private Logger $logger;
8+
9+
public function __construct(Logger $logger)
10+
{
11+
$this->logger = $logger;
12+
}
13+
14+
public function __invoke(Request $request, $next): Response
15+
{
16+
try {
17+
return $next($request);
18+
} catch (\Throwable $e) {
19+
$this->logger->error('Exception in ExceptionMiddleware', ['e' => $e]);
20+
21+
return new Response(render(__DIR__ . '/../templates/exception.html.php', ['e' => $e]), 500);
22+
}
23+
}
24+
}

‎templates/html-format.html.php

-1
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
</head>
2323

2424
<body>
25-
2625
<div class="container">
2726

2827
<h1 class="pagetitle">

0 commit comments

Comments
 (0)