Merge pull request #974 from M0rgan01/improve-php-script-errors

Improve php script error checking
PrestaShop · Nov 7, 2024 · 14efc3b · 14efc3b
2 parents 2213114 + 52bae7a
commit 14efc3b
Show file tree

Hide file tree

Showing 34 changed files with 579 additions and 156 deletions.
diff --git a/classes/DbWrapper.php b/classes/DbWrapper.php
@@ -0,0 +1,149 @@
+<?php
+
+/**
+ * Copyright since 2007 PrestaShop SA and Contributors
+ * PrestaShop is an International Registered Trademark & Property of PrestaShop SA
+ *
+ * NOTICE OF LICENSE
+ *
+ * This source file is subject to the Academic Free License 3.0 (AFL-3.0)
+ * that is bundled with this package in the file LICENSE.md.
+ * It is also available through the world-wide-web at this URL:
+ * https://opensource.org/licenses/AFL-3.0
+ * If you did not receive a copy of the license and are unable to
+ * obtain it through the world-wide-web, please send an email
+ * to [email protected] so we can send you a copy immediately.
+ *
+ * DISCLAIMER
+ *
+ * Do not edit or add to this file if you wish to upgrade PrestaShop to newer
+ * versions in the future. If you wish to customize PrestaShop for your
+ * needs please refer to https://devdocs.prestashop.com/ for more information.
+ *
+ * @author    PrestaShop SA and Contributors <[email protected]>
+ * @copyright Since 2007 PrestaShop SA and Contributors
+ * @license   https://opensource.org/licenses/AFL-3.0 Academic Free License 3.0 (AFL-3.0)
+ */
+
+namespace PrestaShop\Module\AutoUpgrade;
+
+use Db;
+use mysqli_result;
+use PDOStatement;
+use PrestaShop\Module\AutoUpgrade\Exceptions\UpdateDatabaseException;
+
+class DbWrapper
+{
+    /**
+     * @param array<mixed> $data
+     *
+     * @throws UpdateDatabaseException
+     */
+    public static function update(string $table, array $data, string $where = '', int $limit = 0, bool $null_values = false, bool $use_cache = true, bool $add_prefix = true): bool
+    {
+        $result = Db::getInstance()->update($table, $data, $where, $limit, $null_values, $use_cache, $add_prefix);
+        self::validateDBQuerySuccess();
+
+        return $result;
+    }
+
+    /**
+     * @throws UpdateDatabaseException
+     */
+    public static function execute(string $sql, bool $use_cache = true): bool
+    {
+        $result = Db::getInstance()->execute($sql, $use_cache);
+        self::validateDBQuerySuccess();
+
+        return $result;
+    }
+
+    /**
+     * @throws UpdateDatabaseException
+     *
+     * @return array<mixed>|bool|mysqli_result|PDOStatement|resource|null
+     */
+    public static function executeS(string $sql, bool $array = true, bool $use_cache = true)
+    {
+        $result = Db::getInstance()->executeS($sql, $array, $use_cache);
+        self::validateDBQuerySuccess();
+
+        return $result;
+    }
+
+    /**
+     * @throws UpdateDatabaseException
+     *
+     * @return string|false|null Returns false if no results
+     */
+    public static function getValue(string $sql, bool $use_cache = true)
+    {
+        $result = Db::getInstance()->getValue($sql, $use_cache);
+        self::validateDBQuerySuccess();
+
+        return $result;
+    }
+
+    /**
+     * @param array<mixed> $data
+     *
+     * @throws UpdateDatabaseException
+     */
+    public static function insert(string $table, array $data, bool $null_values = false, bool $use_cache = true, int $type = Db::INSERT, bool $add_prefix = true): bool
+    {
+        $result = Db::getInstance()->insert($table, $data, $null_values, $use_cache, $type, $add_prefix);
+        self::validateDBQuerySuccess();
+
+        return $result;
+    }
+
+    /**
+     * @return int|string
+     *
+     * @throws UpdateDatabaseException
+     */
+    public static function Insert_ID()
+    {
+        $result = Db::getInstance()->Insert_ID();
+        self::validateDBQuerySuccess();
+
+        return $result;
+    }
+
+    /**
+     * @throws UpdateDatabaseException
+     */
+    public static function delete(string $table, string $where = '', int $limit = 0, bool $use_cache = true, bool $add_prefix = true): bool
+    {
+        $result = Db::getInstance()->delete($table, $where, $limit, $use_cache, $add_prefix);
+        self::validateDBQuerySuccess();
+
+        return $result;
+    }
+
+    /**
+     * @throws UpdateDatabaseException
+     *
+     * @return array<mixed>|bool|object|null
+     */
+    public static function getRow(string $sql, bool $use_cache = true)
+    {
+        $result = Db::getInstance()->getRow($sql, $use_cache);
+        self::validateDBQuerySuccess();
+
+        return $result;
+    }
+
+    /**
+     * @throws UpdateDatabaseException
+     */
+    public static function validateDBQuerySuccess(): void
+    {
+        $previousQueryError = Db::getInstance()->getMsgError();
+        $previousQueryErrorCode = Db::getInstance()->getNumberError();
+
+        if (!empty($previousQueryError) || !empty($previousQueryErrorCode)) {
+            throw new UpdateDatabaseException($previousQueryError, $previousQueryErrorCode);
+        }
+    }
+}
diff --git a/classes/Exceptions/UpdateDatabaseException.php b/classes/Exceptions/UpdateDatabaseException.php
@@ -0,0 +1,34 @@
+<?php
+
+/**
+ * Copyright since 2007 PrestaShop SA and Contributors
+ * PrestaShop is an International Registered Trademark & Property of PrestaShop SA
+ *
+ * NOTICE OF LICENSE
+ *
+ * This source file is subject to the Academic Free License 3.0 (AFL-3.0)
+ * that is bundled with this package in the file LICENSE.md.
+ * It is also available through the world-wide-web at this URL:
+ * https://opensource.org/licenses/AFL-3.0
+ * If you did not receive a copy of the license and are unable to
+ * obtain it through the world-wide-web, please send an email
+ * to [email protected] so we can send you a copy immediately.
+ *
+ * DISCLAIMER
+ *
+ * Do not edit or add to this file if you wish to upgrade PrestaShop to newer
+ * versions in the future. If you wish to customize PrestaShop for your
+ * needs please refer to https://devdocs.prestashop.com/ for more information.
+ *
+ * @author    PrestaShop SA and Contributors <[email protected]>
+ * @copyright Since 2007 PrestaShop SA and Contributors
+ * @license   https://opensource.org/licenses/AFL-3.0 Academic Free License 3.0 (AFL-3.0)
+ */
+
+namespace PrestaShop\Module\AutoUpgrade\Exceptions;
+
+use Exception;
+
+class UpdateDatabaseException extends Exception
+{
+}
diff --git a/classes/UpgradeTools/CoreUpgrader/CoreUpgrader.php b/classes/UpgradeTools/CoreUpgrader/CoreUpgrader.php
@@ -32,6 +32,7 @@
 use InvalidArgumentException;
 use Language;
 use ParseError;
+use PrestaShop\Module\AutoUpgrade\Exceptions\UpdateDatabaseException;
 use PrestaShop\Module\AutoUpgrade\Exceptions\UpgradeException;
 use PrestaShop\Module\AutoUpgrade\Log\LoggerInterface;
 use PrestaShop\Module\AutoUpgrade\Parameters\UpgradeConfiguration;
@@ -430,7 +431,11 @@ protected function runPhpQuery(string $upgrade_file, string $query): void
             }
 
             require_once $pathToPhpDirectory . strtolower($func_name) . '.php';
-            $phpRes = call_user_func_array($func_name, $parameters);
+            try {
+                $phpRes = call_user_func_array($func_name, $parameters);
+            } catch (UpdateDatabaseException $exception) {
+                $this->logPhpError($upgrade_file, $query, $exception->getMessage());
+            }
         }
         // Or an object method
         else {
@@ -440,7 +445,9 @@ protected function runPhpQuery(string $upgrade_file, string $query): void
         }
 
         if ($this->hasPhpError($phpRes)) {
-            $this->logPhpError($upgrade_file, $query, $phpRes);
+            $message = (empty($phpRes['error']) ? '' : $phpRes['error'] . "\n") .
+                (empty($phpRes['msg']) ? '' : ' - ' . $phpRes['msg'] . "\n");
+            $this->logPhpError($upgrade_file, $query, $message);
         } else {
             $this->logger->debug('Migration file: ' . $upgrade_file . ', Query: ' . $query);
         }
@@ -456,32 +463,21 @@ private function hasPhpError($phpRes): bool
         return isset($phpRes) && (is_array($phpRes) && !empty($phpRes['error'])) || $phpRes === false;
     }
 
-    /**
-     * @param string $upgrade_file
-     * @param string $query
-     * @param mixed $phpRes
-     *
-     * @return void
-     */
-    private function logPhpError(string $upgrade_file, string $query, $phpRes): void
+    private function logPhpError(string $upgrade_file, string $query, string $message): void
     {
-        $this->logger->error(
-            '[ERROR] PHP ' . $upgrade_file . ' ' . $query . "\n" .
-            (empty($phpRes['error']) ? '' : $phpRes['error'] . "\n") .
-            (empty($phpRes['msg']) ? '' : ' - ' . $phpRes['msg'] . "\n")
-        );
+        $this->logger->error('PHP ' . $upgrade_file . ' ' . $query . ": \n" . $message);
         $this->container->getState()->setWarningExists(true);
     }
 
     private function logMissingFileError(string $path, string $func_name, string $query): void
     {
-        $this->logger->error('[ERROR] ' . $path . strtolower($func_name) . ' PHP - missing file ' . $query);
+        $this->logger->error($path . strtolower($func_name) . ' PHP - missing file ' . $query);
         $this->container->getState()->setWarningExists(true);
     }
 
     private function logForbiddenObjectMethodError(string $phpString, string $upgrade_file): void
     {
-        $this->logger->error('[ERROR] ' . $upgrade_file . ' PHP - Object Method call is forbidden (' . $phpString . ')');
+        $this->logger->error($upgrade_file . ' PHP - Object Method call is forbidden (' . $phpString . ')');
         $this->container->getState()->setWarningExists(true);
     }
 
@@ -821,7 +817,7 @@ protected function updateRTLFiles(): void
                 $commandBus->handle($adaptThemeToTRLLanguages);
             } catch (CoreException $e) {
                 $this->logger->error('
-                    [ERROR] PHP Impossible to generate RTL files for theme' . $theme['name'] . "\n" .
+                    PHP Impossible to generate RTL files for theme' . $theme['name'] . "\n" .
                     $e->getMessage()
                 );
 

diff --git a/upgrade/php/add_column.php b/upgrade/php/add_column.php
@@ -1,4 +1,7 @@
 <?php
+
+use PrestaShop\Module\AutoUpgrade\DbWrapper;
+
 /**
  * Copyright since 2007 PrestaShop SA and Contributors
  * PrestaShop is an International Registered Trademark & Property of PrestaShop SA
@@ -19,19 +22,25 @@
  * versions in the future. If you wish to customize PrestaShop for your
  * needs please refer to https://devdocs.prestashop.com/ for more information.
  *
- * @author    PrestaShop SA and Contributors <[email protected]>
  * @copyright Since 2007 PrestaShop SA and Contributors
  * @license   https://opensource.org/licenses/OSL-3.0 Open Software License (OSL 3.0)
+ * @author    PrestaShop SA and Contributors <[email protected]>
+ */
+
+/**
+ * @return void
+ *
+ * @throws \PrestaShop\Module\AutoUpgrade\Exceptions\UpdateDatabaseException
  */
 function add_column($table, $column, $parameters)
 {
-    $column_exists = Db::getInstance()->executeS('SHOW COLUMNS FROM `' . _DB_PREFIX_ . $table . "` WHERE Field = '" . $column . "'");
+    $column_exists = DbWrapper::executeS('SHOW COLUMNS FROM `' . _DB_PREFIX_ . $table . "` WHERE Field = '" . $column . "'");
 
     if (!empty($column_exists)) {
         // If it already exists, we will modify the structure
-        Db::getInstance()->execute('ALTER TABLE `' . _DB_PREFIX_ . $table . '` CHANGE `' . $column . '` `' . $column . '` ' . $parameters);
+        DbWrapper::execute('ALTER TABLE `' . _DB_PREFIX_ . $table . '` CHANGE `' . $column . '` `' . $column . '` ' . $parameters);
     } else {
         // Otherwise, we add it
-        Db::getInstance()->execute('ALTER TABLE `' . _DB_PREFIX_ . $table . '` ADD `' . $column . '` ' . $parameters);
+        DbWrapper::execute('ALTER TABLE `' . _DB_PREFIX_ . $table . '` ADD `' . $column . '` ' . $parameters);
     }
 }
diff --git a/upgrade/php/add_hook.php b/upgrade/php/add_hook.php
@@ -1,4 +1,7 @@
 <?php
+
+use PrestaShop\Module\AutoUpgrade\DbWrapper;
+
 /**
  * Copyright since 2007 PrestaShop SA and Contributors
  * PrestaShop is an International Registered Trademark & Property of PrestaShop SA
@@ -19,13 +22,19 @@
  * versions in the future. If you wish to customize PrestaShop for your
  * needs please refer to https://devdocs.prestashop.com/ for more information.
  *
- * @author    PrestaShop SA and Contributors <[email protected]>
  * @copyright Since 2007 PrestaShop SA and Contributors
  * @license   https://opensource.org/licenses/OSL-3.0 Open Software License (OSL 3.0)
+ * @author    PrestaShop SA and Contributors <[email protected]>
+ */
+
+/**
+ * @return bool
+ *
+ * @throws \PrestaShop\Module\AutoUpgrade\Exceptions\UpdateDatabaseException
  */
 function add_hook($hook, $title, $description, $position = 1)
 {
-    return (bool) Db::getInstance()->execute('
+    return (bool) DbWrapper::execute('
         INSERT INTO `' . _DB_PREFIX_ . 'hook` (`name`, `title`, `description`, `position`)
         VALUES ("' . pSQL($hook) . '", "' . pSQL($title) . '", "' . pSQL($description) . '", ' . (int) $position . ')
         ON DUPLICATE KEY UPDATE `title` = VALUES(`title`), `description` = VALUES(`description`)    

diff --git a/upgrade/php/add_missing_unique_key_from_authorization_role.php b/upgrade/php/add_missing_unique_key_from_authorization_role.php
@@ -1,4 +1,7 @@
 <?php
+
+use PrestaShop\Module\AutoUpgrade\DbWrapper;
+
 /**
  * Copyright since 2007 PrestaShop SA and Contributors
  * PrestaShop is an International Registered Trademark & Property of PrestaShop SA
@@ -19,16 +22,22 @@
  * versions in the future. If you wish to customize PrestaShop for your
  * needs please refer to https://devdocs.prestashop.com/ for more information.
  *
- * @author    PrestaShop SA and Contributors <[email protected]>
  * @copyright Since 2007 PrestaShop SA and Contributors
  * @license   https://opensource.org/licenses/OSL-3.0 Open Software License (OSL 3.0)
+ * @author    PrestaShop SA and Contributors <[email protected]>
  */
 
-// Allows you to catch up on a forgotten uniqueness constraint on the roles
+/**
+ * Allows you to catch up on a forgotten uniqueness constraint on the roles
+ *
+ * @return void
+ *
+ * @throws \PrestaShop\Module\AutoUpgrade\Exceptions\UpdateDatabaseException
+ */
 function add_missing_unique_key_from_authorization_role()
 {
     // Verify if we need to create unique key
-    $keys = Db::getInstance()->executeS(
+    $keys = DbWrapper::executeS(
         'SHOW KEYS FROM ' . _DB_PREFIX_ . "authorization_role WHERE Key_name='slug'"
     );
 
@@ -37,7 +46,7 @@ function add_missing_unique_key_from_authorization_role()
     }
 
     // We recover the duplicates that we want to keep
-    $duplicates = Db::getInstance()->executeS(
+    $duplicates = DbWrapper::executeS(
         'SELECT MIN(id_authorization_role) AS keep_ID, slug FROM ' . _DB_PREFIX_ . 'authorization_role GROUP BY slug HAVING COUNT(*) > 1'
     );
 
@@ -47,21 +56,21 @@ function add_missing_unique_key_from_authorization_role()
 
     foreach ($duplicates as $duplicate) {
         // We recover the duplicates that we want to remove
-        $elementsToRemoves = Db::getInstance()->executeS(
+        $elementsToRemoves = DbWrapper::executeS(
             'SELECT id_authorization_role FROM ' . _DB_PREFIX_ . "authorization_role WHERE slug = '" . $duplicate['slug'] . "' AND id_authorization_role != " . $duplicate['keep_ID']
         );
 
         foreach ($elementsToRemoves as $elementToRemove) {
             // We update the access table which may have used a duplicate role
-            Db::getInstance()->execute(
+            DbWrapper::execute(
                 'UPDATE ' . _DB_PREFIX_ . "access SET id_authorization_role = '" . $duplicate['keep_ID'] . "' WHERE id_authorization_role = " . $elementToRemove['id_authorization_role']
             );
             // We remove the role
-            Db::getInstance()->delete('authorization_role', '`id_authorization_role` = ' . (int) $elementToRemove['id_authorization_role']);
+            DbWrapper::delete('authorization_role', '`id_authorization_role` = ' . (int) $elementToRemove['id_authorization_role']);
         }
     }
 
-    Db::getInstance()->execute(
+    DbWrapper::execute(
         'ALTER TABLE ' . _DB_PREFIX_ . 'authorization_role ADD UNIQUE KEY `slug` (`slug`)'
     );
 }