refactor: simplified code, fixed code issues

thorsten · Oct 5, 2024 · 7fe4153 · 7fe4153
1 parent bb86f47
commit 7fe4153
Show file tree

Hide file tree

Showing 22 changed files with 185 additions and 169 deletions.
diff --git a/.gitignore b/.gitignore
@@ -92,8 +92,8 @@ tests/fixtures/path-to-delete/foo.bar.baz
 # Jest related stuff
 coverage/*
 
-# Playwright related stuff
-playwright-report/
+# PHPMD
+phpmd.html
 
 # Netbeans project folder
 nbproject/

diff --git a/composer.json b/composer.json
@@ -90,7 +90,7 @@
     "openapi:json": "./phpmyfaq/src/libs/bin/openapi -b ./phpmyfaq/src/libs/autoload.php -f json ./phpmyfaq/src/phpMyFAQ -o docs/openapi.json",
     "openapi:yaml": "./phpmyfaq/src/libs/bin/openapi -b ./phpmyfaq/src/libs/autoload.php ./phpmyfaq/src/phpMyFAQ -o docs/openapi.yaml",
     "phpstan": "./phpmyfaq/src/libs/bin/phpstan analyze --memory-limit=4G",
-    "phpmd": "./phpmyfaq/src/libs/bin/phpmd ./phpmyfaq/src/phpMyFAQ xml rulesets.xml",
+    "phpmd": "./phpmyfaq/src/libs/bin/phpmd ./phpmyfaq/src/phpMyFAQ html rulesets.xml > phpmd.html",
     "refactor": "./phpmyfaq/src/libs/bin/rector",
     "refactor:dryrun": "./phpmyfaq/src/libs/bin/rector --dry-run",
     "test": "./phpmyfaq/src/libs/bin/phpunit --coverage-text"

diff --git a/phpmyfaq/src/phpMyFAQ/Administration/Api.php b/phpmyfaq/src/phpMyFAQ/Administration/Api.php
@@ -83,14 +83,14 @@ public function getVersions(): array
             ) {
                 throw new Core\Exception('phpMyFAQ Version API is not available: ' .  $e->getMessage());
             }
-        } else {
-            return [
-                'installed' => $this->configuration->getVersion(),
-                'stable' => 'n/a',
-                'development' => 'n/a',
-                'nightly' => 'n/a'
-            ];
         }
+
+        return [
+            'installed' => $this->configuration->getVersion(),
+            'stable' => 'n/a',
+            'development' => 'n/a',
+            'nightly' => 'n/a'
+        ];
     }
 
     /**

diff --git a/phpmyfaq/src/phpMyFAQ/Administration/Backup.php b/phpmyfaq/src/phpMyFAQ/Administration/Backup.php
@@ -182,9 +182,9 @@ public function createContentFolderBackup(): string
 
             if ($file->isDir()) {
                 $zipArchive->addEmptyDir($relativePath);
-            } else {
-                $zipArchive->addFile($filePath, $relativePath);
             }
+
+            $zipArchive->addFile($filePath, $relativePath);
         }
 
         $zipArchive->close();

diff --git a/phpmyfaq/src/phpMyFAQ/Administration/Faq.php b/phpmyfaq/src/phpMyFAQ/Administration/Faq.php
@@ -114,8 +114,6 @@ public function getAllFaqsByCategory(int $categoryId, bool $onlyInactive = false
                     'created' => $row->created,
                 ];
             }
-        } else {
-            return $faqData;
         }
 
         return $faqData;

diff --git a/phpmyfaq/src/phpMyFAQ/Administration/RatingData.php b/phpmyfaq/src/phpMyFAQ/Administration/RatingData.php
@@ -36,8 +36,20 @@ public function __construct(private Configuration $configuration)
     public function getAll(): array
     {
         $ratings = [];
+        $query = $this->buildQuery();
+        $result = $this->configuration->getDb()->query($query);
+
+        while ($row = $this->configuration->getDb()->fetchObject($result)) {
+            $ratings[] = $this->mapRowToRating($row);
+        }
+
+        return $ratings;
+    }
 
-        $query = match (Database::getType()) {
+    private function buildQuery(): string
+    {
+        $prefix = Database::getTablePrefix();
+        return match (Database::getType()) {
             'sqlsrv' => sprintf(
                 '
                     SELECT
@@ -68,9 +80,7 @@ public function getAll(): array
                         fv.usr
                     ORDER BY
                         fcr.category_id',
-                Database::getTablePrefix(),
-                Database::getTablePrefix(),
-                Database::getTablePrefix()
+                $prefix, $prefix, $prefix
             ),
             default => sprintf(
                 '
@@ -102,37 +112,37 @@ public function getAll(): array
                         fv.usr
                     ORDER BY
                         fcr.category_id',
-                Database::getTablePrefix(),
-                Database::getTablePrefix(),
-                Database::getTablePrefix()
+                $prefix,
+                $prefix,
+                $prefix
             ),
         };
+    }
 
-        $result = $this->configuration->getDb()->query($query);
-        while ($row = $this->configuration->getDb()->fetchObject($result)) {
-            $question = Strings::htmlspecialchars(trim((string) $row->question));
-            $url = sprintf(
-                '%sindex.php?action=faq&cat=%d&id=%d&artlang=%s',
-                $this->configuration->getDefaultUrl(),
-                $row->category_id,
-                $row->id,
-                $row->lang
-            );
+    private function mapRowToRating(object $row): array
+    {
+        var_dump($this->configuration->getDefaultUrl());
 
-            $link = new Link($url, $this->configuration);
-            $link->itemTitle = $question;
+        $question = Strings::htmlspecialchars(trim((string) $row->question));
+        $url = sprintf(
+            '%sindex.php?action=faq&cat=%d&id=%d&artlang=%s',
+            $this->configuration->getDefaultUrl(),
+            $row->category_id,
+            $row->id,
+            $row->lang
+        );
 
-            $ratings[] = [
-                'id' => $row->id,
-                'lang' => $row->lang,
-                'category_id' => $row->category_id,
-                'question' => $question,
-                'url' => $link->toString(),
-                'number' => $row->num,
-                'user' => $row->usr
-            ];
-        }
+        $link = new Link($url, $this->configuration);
+        $link->itemTitle = $question;
 
-        return $ratings;
+        return [
+            'id' => $row->id,
+            'lang' => $row->lang,
+            'category_id' => $row->category_id,
+            'question' => $question,
+            'url' => $link->toString(),
+            'number' => $row->num,
+            'user' => $row->usr
+        ];
     }
 }
diff --git a/phpmyfaq/src/phpMyFAQ/Administration/Report.php b/phpmyfaq/src/phpMyFAQ/Administration/Report.php
@@ -98,23 +98,23 @@ public function getReportingData(): array
         while ($row = $this->configuration->getDb()->fetchObject($result)) {
             if ($row->id == $lastId) {
                 ++$report[$row->id]['faq_translations'];
-            } else {
-                $report[$row->id] = [
-                    'faq_id' => $row->id,
-                    'faq_language' => $row->lang,
-                    'category_id' => $row->category_id,
-                    'category_parent' => $row->parent_id,
-                    'category_name' => $row->category_name,
-                    'faq_translations' => 0,
-                    'faq_sticky' => $row->sticky,
-                    'faq_question' => $row->question,
-                    'faq_org_author' => $row->original_author,
-                    'faq_updated' => Date::createIsoDate($row->updated),
-                    'faq_visits' => $row->visits,
-                    'faq_last_author' => $row->last_author,
-                ];
             }
 
+            $report[$row->id] = [
+                'faq_id' => $row->id,
+                'faq_language' => $row->lang,
+                'category_id' => $row->category_id,
+                'category_parent' => $row->parent_id,
+                'category_name' => $row->category_name,
+                'faq_translations' => 0,
+                'faq_sticky' => $row->sticky,
+                'faq_question' => $row->question,
+                'faq_org_author' => $row->original_author,
+                'faq_updated' => Date::createIsoDate($row->updated),
+                'faq_visits' => $row->visits,
+                'faq_last_author' => $row->last_author,
+            ];
+
             $lastId = $row->id;
         }
 

diff --git a/phpmyfaq/src/phpMyFAQ/Application.php b/phpmyfaq/src/phpMyFAQ/Application.php
@@ -53,18 +53,17 @@ private function setLanguage(): string
             $language = new Language($this->configuration);
             $currentLanguage = $language->setLanguageByAcceptLanguage();
 
+            require sprintf('%s/language_en.php', PMF_TRANSLATION_DIR);
             if (Language::isASupportedLanguage($currentLanguage)) {
                 require sprintf('%s/language_%s.php', PMF_TRANSLATION_DIR, $currentLanguage);
-            } else {
-                require sprintf('%s/language_en.php', PMF_TRANSLATION_DIR);
             }
 
             $this->configuration->setLanguage($language);
 
             return $currentLanguage;
-        } else {
-            return 'en';
         }
+
+        return 'en';
     }
 
     /**

diff --git a/phpmyfaq/src/phpMyFAQ/Attachment/AttachmentFactory.php b/phpmyfaq/src/phpMyFAQ/Attachment/AttachmentFactory.php
@@ -47,14 +47,14 @@ class AttachmentFactory
     /**
      * Create an attachment exemplar.
      *
-     * @param int|null    $id  ID
+     * @param int|null    $attachmentId  ID
      * @param string|null $key Key
      * @throws AttachmentException
      */
-    public static function create(?int $id = null, ?string $key = null): File
+    public static function create(?int $attachmentId = null, ?string $key = null): File
     {
         $return = match (self::$storageType) {
-            AttachmentStorageType::FILESYSTEM->value => new File($id),
+            AttachmentStorageType::FILESYSTEM->value => new File($attachmentId),
             default => throw new AttachmentException('Unknown attachment storage type'),
         };
 
@@ -63,8 +63,6 @@ public static function create(?int $id = null, ?string $key = null): File
          */
         if (self::$encryptionEnabled) {
             $key ??= self::$defaultKey;
-        } else {
-            $key = null;
         }
 
         $return->setKey($key);

diff --git a/phpmyfaq/src/phpMyFAQ/Attachment/Filesystem/File/EncryptedFile.php b/phpmyfaq/src/phpMyFAQ/Attachment/Filesystem/File/EncryptedFile.php
@@ -18,6 +18,7 @@
 namespace phpMyFAQ\Attachment\Filesystem\File;
 
 use phpMyFAQ\Attachment\File;
+use phpMyFAQ\Attachment\Filesystem\AbstractFile;
 use phpseclib3\Crypt\AES;
 
 /**
@@ -32,14 +33,14 @@ class EncryptedFile extends File
      *
      * @var string
      */
-    private const CHUNKDELIMITER = 'ฒૐᥤ';
+    private const CHUNK_DELIMITER = 'ฒૐᥤ';
 
     /**
      * AES instance.
      *
-     * @var object
+     * @var AES
      */
-    protected $aes;
+    protected AES $aes;
 
     /** @var resource */
     private $handle;
@@ -57,7 +58,7 @@ public function __construct($filepath, string $mode, string $key)
 
     public function putChunk(string $chunk): int|false
     {
-        $content = $this->aes->encrypt($chunk) . self::CHUNKDELIMITER;
+        $content = $this->aes->encrypt($chunk) . self::CHUNK_DELIMITER;
 
         return fwrite($this->handle, $content);
     }
@@ -67,37 +68,34 @@ public function putChunk(string $chunk): int|false
      */
     public function copyTo(string $target): bool
     {
-        $retval = false;
+        $return = false;
 
         if (is_string($target)) {
-            $target = new VanillaFile($target, self::MODE_WRITE);
+            $target = new VanillaFile($target, AbstractFile::MODE_WRITE);
         } else {
-            $target->setMode(self::MODE_WRITE);
+            $target->setMode(AbstractFile::MODE_WRITE);
         }
 
         if ($target->isOk()) {
             while (!$this->eof()) {
                 $target->putChunk($this->getChunk());
             }
 
-            $retval = true;
+            $return = true;
         }
 
-        return $retval;
+        return $return;
     }
 
-    /**
-     * @inheritdoc
-     */
     public function getChunk(): string
     {
         $readEnd = false;
         $chunk = '';
-        $chunkDelimLen = strlen(self::CHUNKDELIMITER);
+        $chunkDelimLen = strlen(self::CHUNK_DELIMITER);
 
         while (!$readEnd && !$this->eof()) {
             $chunk .= fread($this->handle, 1);
-            $readEnd = self::CHUNKDELIMITER == substr($chunk, -$chunkDelimLen);
+            $readEnd = self::CHUNK_DELIMITER == substr($chunk, -$chunkDelimLen);
         }
 
         $chunk = substr($chunk, 0, -$chunkDelimLen);

diff --git a/phpmyfaq/src/phpMyFAQ/Auth/AuthHttp.php b/phpmyfaq/src/phpMyFAQ/Auth/AuthHttp.php
@@ -24,6 +24,7 @@
 use phpMyFAQ\Enums\AuthenticationSourceType;
 use phpMyFAQ\User;
 use SensitiveParameter;
+use Symfony\Component\HttpFoundation\Request;
 
 /**
  * Class AuthHttp
@@ -32,12 +33,16 @@
  */
 class AuthHttp extends Auth implements AuthDriverInterface
 {
+    private Request $request;
+
     /**
      * @inheritDoc
      */
     public function __construct(Configuration $configuration)
     {
         parent::__construct($configuration);
+
+        $this->request = Request::createFromGlobals();
     }
 
     /**
@@ -77,17 +82,21 @@ public function delete($login): bool
      */
     public function checkCredentials(string $login, #[SensitiveParameter] $password, ?array $optionalData = null): bool
     {
-        if (!isset($_SERVER['PHP_AUTH_USER']) && $_SERVER['PHP_AUTH_PW']) {
+        if ($this->request->server->get('PHP_AUTH_USER') === null && $this->request->server->get('PHP_AUTH_PW')) {
             return false;
         }
-        return ($_SERVER['PHP_AUTH_USER'] === $login && $_SERVER['PHP_AUTH_PW'] === $password);
+
+        return (
+            $this->request->server->get('PHP_AUTH_USER') === $login &&
+            $this->request->server->get('PHP_AUTH_PW') === $password
+        );
     }
 
     /**
      * @inheritDoc
      */
     public function isValidLogin($login, ?array $optionalData = null): int
     {
-        return isset($_SERVER['PHP_AUTH_USER']) ? 1 : 0;
+        return $this->request->server->get('PHP_AUTH_USER') !== null ? 1 : 0;
     }
 }