refactor: introduce http Request object

RSS-Bridge · Jan 25, 2024 · 5a69031 · 5a69031
1 parent 9574c17
commit 5a69031
Show file tree

Hide file tree

Showing 19 changed files with 125 additions and 94 deletions.
diff --git a/actions/ConnectivityAction.php b/actions/ConnectivityAction.php
@@ -19,13 +19,13 @@ public function __construct()
         $this->bridgeFactory = new BridgeFactory();
     }
 
-    public function execute(array $request)
+    public function execute(Request $request)
     {
         if (!Debug::isEnabled()) {
             return new Response('This action is only available in debug mode!', 403);
         }
 
-        $bridgeName = $request['bridge'] ?? null;
+        $bridgeName = $request->get('bridge');
         if (!$bridgeName) {
             return render_template('connectivity.html.php');
         }

diff --git a/actions/DetectAction.php b/actions/DetectAction.php
@@ -2,10 +2,10 @@
 
 class DetectAction implements ActionInterface
 {
-    public function execute(array $request)
+    public function execute(Request $request)
     {
-        $targetURL = $request['url'] ?? null;
-        $format = $request['format'] ?? null;
+        $targetURL = $request->get('url');
+        $format = $request->get('format');
 
         if (!$targetURL) {
             throw new \Exception('You must specify a url!');

diff --git a/actions/DisplayAction.php b/actions/DisplayAction.php
@@ -11,9 +11,13 @@ public function __construct()
         $this->logger = RssBridge::getLogger();
     }
 
-    public function execute(array $request)
+    public function execute(Request $request)
     {
-        $cacheKey = 'http_' . json_encode($request);
+        $bridgeName = $request->get('bridge');
+        $format = $request->get('format');
+        $noproxy = $request->get('_noproxy');
+
+        $cacheKey = 'http_' . json_encode($request->toArray());
         /** @var Response $cachedResponse */
         $cachedResponse = $this->cache->get($cacheKey);
         if ($cachedResponse) {
@@ -31,7 +35,6 @@ public function execute(array $request)
             return $cachedResponse;
         }
 
-        $bridgeName = $request['bridge'] ?? null;
         if (!$bridgeName) {
             return new Response(render(__DIR__ . '/../templates/error.html.php', ['message' => 'Missing bridge parameter']), 400);
         }
@@ -40,15 +43,15 @@ public function execute(array $request)
         if (!$bridgeClassName) {
             return new Response(render(__DIR__ . '/../templates/error.html.php', ['message' => 'Bridge not found']), 404);
         }
-        $format = $request['format'] ?? null;
+
         if (!$format) {
             return new Response(render(__DIR__ . '/../templates/error.html.php', ['message' => 'You must specify a format']), 400);
         }
         if (!$bridgeFactory->isEnabled($bridgeClassName)) {
             return new Response(render(__DIR__ . '/../templates/error.html.php', ['message' => 'This bridge is not whitelisted']), 400);
         }
 
-        $noproxy = $request['_noproxy'] ?? null;
+
         if (
             Configuration::getConfig('proxy', 'url')
             && Configuration::getConfig('proxy', 'by_bridge')
@@ -65,7 +68,7 @@ public function execute(array $request)
         $response = $this->createResponse($request, $bridge, $format);
 
         if ($response->getCode() === 200) {
-            $ttl = $request['_cache_timeout'] ?? null;
+            $ttl = $request->get('_cache_timeout');
             if (Configuration::getConfig('cache', 'custom_timeout') && $ttl) {
                 $ttl = (int) $ttl;
             } else {
@@ -90,7 +93,7 @@ public function execute(array $request)
         return $response;
     }
 
-    private function createResponse(array $request, BridgeAbstract $bridge, FormatAbstract $format)
+    private function createResponse(Request $request, BridgeAbstract $bridge, FormatAbstract $format)
     {
         $items = [];
         $feed = [];
@@ -107,7 +110,8 @@ private function createResponse(array $request, BridgeAbstract $bridge, FormatAb
                 '_error_time',
                 '_', // Some RSS readers add a cache-busting parameter (_=<timestamp>) to feed URLs, detect and ignore them.
             ];
-            $input = array_diff_key($request, array_fill_keys($remove, ''));
+            $requestArray = $request->toArray();
+            $input = array_diff_key($requestArray, array_fill_keys($remove, ''));
             $bridge->setInput($input);
             $bridge->collectData();
             $items = $bridge->getItems();

diff --git a/actions/FindfeedAction.php b/actions/FindfeedAction.php
@@ -7,10 +7,10 @@
  */
 class FindfeedAction implements ActionInterface
 {
-    public function execute(array $request)
+    public function execute(Request $request)
     {
-        $targetURL = $request['url'] ?? null;
-        $format = $request['format'] ?? null;
+        $targetURL = $request->get('url');
+        $format = $request->get('format');
 
         if (!$targetURL) {
             return new Response('You must specify a url', 400);

diff --git a/actions/FrontpageAction.php b/actions/FrontpageAction.php
@@ -2,10 +2,11 @@
 
 final class FrontpageAction implements ActionInterface
 {
-    public function execute(array $request)
+    public function execute(Request $request)
     {
+        $showInactive = (bool) $request->get('show_inactive');
+
         $messages = [];
-        $showInactive = (bool) ($request['show_inactive'] ?? null);
         $activeBridges = 0;
 
         $bridgeFactory = new BridgeFactory();
@@ -18,16 +19,13 @@ public function execute(array $request)
             ];
         }
 
-        $formatFactory = new FormatFactory();
-        $formats = $formatFactory->getFormatNames();
-
         $body = '';
         foreach ($bridgeClassNames as $bridgeClassName) {
             if ($bridgeFactory->isEnabled($bridgeClassName)) {
-                $body .= BridgeCard::displayBridgeCard($bridgeClassName);
+                $body .= BridgeCard::render($bridgeClassName);
                 $activeBridges++;
             } elseif ($showInactive) {
-                $body .= BridgeCard::displayBridgeCard($bridgeClassName, false) . "\n";
+                $body .= BridgeCard::render($bridgeClassName, false) . "\n";
             }
         }
 

diff --git a/actions/HealthAction.php b/actions/HealthAction.php
@@ -4,7 +4,7 @@
 
 class HealthAction implements ActionInterface
 {
-    public function execute(array $request)
+    public function execute(Request $request)
     {
         $response = [
             'code' => 200,

diff --git a/actions/ListAction.php b/actions/ListAction.php
@@ -2,7 +2,7 @@
 
 class ListAction implements ActionInterface
 {
-    public function execute(array $request)
+    public function execute(Request $request)
     {
         $list = new \stdClass();
         $list->bridges = [];

diff --git a/actions/SetBridgeCacheAction.php b/actions/SetBridgeCacheAction.php
@@ -9,15 +9,17 @@ public function __construct()
         $this->cache = RssBridge::getCache();
     }
 
-    public function execute(array $request)
+    public function execute(Request $request)
     {
+        $requestArray = $request->toArray();
+
         // Authentication
         $accessTokenInConfig = Configuration::getConfig('authentication', 'access_token');
         if (!$accessTokenInConfig) {
             return new Response('Access token is not set in this instance', 403, ['content-type' => 'text/plain']);
         }
-        if (isset($request['access_token'])) {
-            $accessTokenGiven = $request['access_token'];
+        if (isset($requestArray['access_token'])) {
+            $accessTokenGiven = $requestArray['access_token'];
         } else {
             $header = trim($_SERVER['HTTP_AUTHORIZATION'] ?? '');
             $position = strrpos($header, 'Bearer ');
@@ -35,33 +37,32 @@ public function execute(array $request)
         }
 
         // Begin actual work
-        $key = $request['key'] ?? null;
+        $key = $requestArray['key'] ?? null;
         if (!$key) {
-            returnClientError('You must specify key!');
+            return new Response('You must specify key', 400, ['content-type' => 'text/plain']);
         }
 
         $bridgeFactory = new BridgeFactory();
 
-        $bridgeName = $request['bridge'] ?? null;
+        $bridgeName = $requestArray['bridge'] ?? null;
         $bridgeClassName = $bridgeFactory->createBridgeClassName($bridgeName);
         if (!$bridgeClassName) {
-            throw new \Exception(sprintf('Bridge not found: %s', $bridgeName));
+            return new Response(sprintf('Bridge not found: %s', $bridgeName), 400, ['content-type' => 'text/plain']);
         }
 
         // whitelist control
         if (!$bridgeFactory->isEnabled($bridgeClassName)) {
-            throw new \Exception('This bridge is not whitelisted', 401);
+            return new Response('This bridge is not whitelisted', 401, ['content-type' => 'text/plain']);
         }
 
         $bridge = $bridgeFactory->create($bridgeClassName);
         $bridge->loadConfiguration();
-        $value = $request['value'];
+        $value = $requestArray['value'];
 
         $cacheKey = get_class($bridge) . '_' . $key;
         $ttl = 86400 * 3;
         $this->cache->set($cacheKey, $value, $ttl);
 
-        header('Content-Type: text/plain');
-        echo 'done';
+        return new Response('done', 200, ['Content-Type' => 'text/plain']);
     }
 }
diff --git a/bridges/StreamCzBridge.php b/bridges/StreamCzBridge.php
@@ -63,7 +63,7 @@ public function collectData()
             $imageUrlNode = reset($episode['node']['images']);
             $item = [
                 'title' => $episode['node']['name'],
-                'uri' => "${fixedUrl}/${episodeUrl}",
+                'uri' => $fixedUrl . '/' . $episodeUrl,
                 'content' => $imageUrlNode ? '<img src="' . $imageUrlNode['url'] . '" />' : '',
                 'timestamp' => $episode['node']['publishTime']['timestamp']
             ];

diff --git a/formats/HtmlFormat.php b/formats/HtmlFormat.php
@@ -12,20 +12,21 @@ public function stringify()
         $formatFactory = new FormatFactory();
         $buttons = [];
         $linkTags = [];
-        foreach ($formatFactory->getFormatNames() as $format) {
+        foreach ($formatFactory->getFormatNames() as $formatName) {
             // Dynamically build buttons for all formats (except HTML)
-            if ($format === 'Html') {
+            if ($formatName === 'Html') {
                 continue;
             }
-            $formatUrl = '?' . str_ireplace('format=Html', 'format=' . $format, htmlentities($queryString));
+            $formatUrl = '?' . str_ireplace('format=Html', 'format=' . $formatName, htmlentities($queryString));
             $buttons[] = [
                 'href' => $formatUrl,
-                'value' => $format,
+                'value' => $formatName,
             ];
+            $format = $formatFactory->create($formatName);
             $linkTags[] = [
                 'href' => $formatUrl,
-                'title' => $format,
-                'type' => $formatFactory->create($format)->getMimeType(),
+                'title' => $formatName,
+                'type' => $format->getMimeType(),
             ];
         }
 

diff --git a/lib/ActionInterface.php b/lib/ActionInterface.php
@@ -5,5 +5,5 @@ interface ActionInterface
     /**
      * @return string|Response
      */
-    public function execute(array $request);
+    public function execute(Request $request);
 }
diff --git a/lib/BridgeCard.php b/lib/BridgeCard.php
@@ -3,13 +3,13 @@
 final class BridgeCard
 {
     /**
-     * Gets a single bridge card
+     * Render bridge card
      *
      * @param class-string<BridgeAbstract> $bridgeClassName The bridge name
      * @param bool $isActive Indicates if the bridge is active or not
      * @return string The bridge card
      */
-    public static function displayBridgeCard($bridgeClassName, $isActive = true)
+    public static function render($bridgeClassName, $isActive = true)
     {
         $bridgeFactory = new BridgeFactory();
 
@@ -56,10 +56,10 @@ class="bridge-card"
         // If we don't have any parameter for the bridge, we print a generic form to load it.
         if (count($contexts) === 0) {
             // The bridge has zero parameters
-            $card .= self::getForm($bridgeClassName, $isActive);
+            $card .= self::renderForm($bridgeClassName, $isActive);
         } elseif (count($contexts) === 1 && array_key_exists('global', $contexts)) {
             // The bridge has a single context with key 'global'
-            $card .= self::getForm($bridgeClassName, $isActive, '', $contexts['global']);
+            $card .= self::renderForm($bridgeClassName, $isActive, '', $contexts['global']);
         } else {
             // The bridge has one or more contexts (named or unnamed)
             foreach ($contexts as $contextName => $contextParameters) {
@@ -77,7 +77,7 @@ class="bridge-card"
                     $card .= '<h5>' . $contextName . '</h5>' . PHP_EOL;
                 }
 
-                $card .= self::getForm($bridgeClassName, $isActive, $contextName, $contextParameters);
+                $card .= self::renderForm($bridgeClassName, $isActive, $contextName, $contextParameters);
             }
         }
 
@@ -97,7 +97,7 @@ class="bridge-card"
         return $card;
     }
 
-    private static function getForm(
+    private static function renderForm(
         string $bridgeClassName,
         bool $isActive = false,
         string $contextName = '',

diff --git a/lib/FeedExpander.php b/lib/FeedExpander.php
@@ -26,6 +26,7 @@ public function collectExpandableDatas(string $url, $maxItems = -1)
         $badStrings = [
             '&nbsp;',
             '&raquo;',
+            '&rsquo;',
         ];
         $xmlString = str_replace($badStrings, '', $xmlString);
         $feedParser = new FeedParser();

diff --git a/lib/FormatFactory.php b/lib/FormatFactory.php
@@ -2,32 +2,26 @@
 
 class FormatFactory
 {
-    private $folder;
-    private $formatNames;
+    private array $formatNames = [];
 
-    public function __construct(string $folder = PATH_LIB_FORMATS)
+    public function __construct()
     {
-        $this->folder = $folder;
-
-        // create format names
-        foreach (scandir($this->folder) as $file) {
-            if (preg_match('/^([^.]+)Format\.php$/U', $file, $m)) {
+        $iterator = new \FilesystemIterator(__DIR__ . '/../formats');
+        foreach ($iterator as $file) {
+            if (preg_match('/^([^.]+)Format\.php$/U', $file->getFilename(), $m)) {
                 $this->formatNames[] = $m[1];
             }
         }
+        sort($this->formatNames);
     }
 
-    /**
-     * @throws \InvalidArgumentException
-     * @param string $name The name of the format e.g. "Atom", "Mrss" or "Json"
-     */
     public function create(string $name): FormatAbstract
     {
         if (! preg_match('/^[a-zA-Z0-9-]*$/', $name)) {
             throw new \InvalidArgumentException('Format name invalid!');
         }
-        $sanitizedName = $this->sanitizeFormatName($name);
-        if ($sanitizedName === null) {
+        $sanitizedName = $this->sanitizeName($name);
+        if (!$sanitizedName) {
             throw new \InvalidArgumentException(sprintf('Unknown format given `%s`', $name));
         }
         $className = '\\' . $sanitizedName . 'Format';
@@ -39,15 +33,13 @@ public function getFormatNames(): array
         return $this->formatNames;
     }
 
-    protected function sanitizeFormatName(string $name)
+    protected function sanitizeName(string $name): ?string
     {
         $name = ucfirst(strtolower($name));
-
         // Trim trailing '.php' if exists
         if (preg_match('/(.+)(?:\.php)/', $name, $matches)) {
             $name = $matches[1];
         }
-
         // Trim trailing 'Format' if exists
         if (preg_match('/(.+)(?:Format)/i', $name, $matches)) {
             $name = $matches[1];