CLI improvement before and during install

.github/workflows/lint.yml

@@ -79,8 +79,8 @@ jobs:
       - name: Run Lint Yaml on `app`
         run: php bin/console lint:yaml app
 
-      - name: Run Lint Yaml on `src`
-        run: php bin/console lint:yaml src
+      - name: Run Lint Yaml on `src` tags are allowed
+        run: php bin/console lint:yaml src --parse-tags
 
       - name: Run Lint Yaml on `.t9n.yml`
         run: php bin/console lint:yaml .t9n.yml

.github/workflows/symfony-console.yml

@@ -0,0 +1,66 @@
+name: Symfony Console
+on:
+  push:
+  pull_request:
+
+permissions:
+  contents: read
+concurrency:
+  group: ${{ github.event_name }}-${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }}
+  cancel-in-progress: true
+jobs:
+  symfony-console:
+    permissions:
+      contents: read  # for actions/checkout to fetch code
+    name: Symfony Console
+    runs-on: ubuntu-latest
+    steps:
+      - name: Setup PHP
+        uses: shivammathur/setup-php@v2
+        with:
+          php-version: 8.1
+          extensions: mbstring, intl, gd, xml, dom, json, fileinfo, curl, zip, iconv
+        env:
+          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+      - uses: actions/checkout@v3
+
+      - name: Get Composer Cache Directory
+        id: composer-cache
+        run: echo "dir=$(composer config cache-files-dir)" >> $GITHUB_OUTPUT
+
+      - name: Cache Composer Directory
+        uses: actions/cache@v3
+        with:
+          path: ${{ steps.composer-cache.outputs.dir }}
+          key: ${{ runner.os }}-composer-${{ hashFiles('**/composer.lock') }}
+          restore-keys: ${{ runner.os }}-composer-
+
+      - name: Composer Install
+        run: composer install --ansi --prefer-dist --no-interaction --no-progress
+
+      - name: Check bin/console can run without shop install and cache clear
+        run: |
+          ./bin/console
+          ./bin/console cache:clear --env=dev
+          ./bin/console cache:clear --env=prod
+          ./bin/console cache:clear --env=dev --no-warmup
+          ./bin/console cache:clear --env=prod --no-warmup
+
+      # Now install the shop
+      - name: Setup Environment
+        timeout-minutes: 15
+        uses: ./.github/actions/setup-env
+        with:
+          PHP_VERSION: 8.1
+          ENABLE_SSL: 'true'
+          INSTALL_AUTO: 'false'
+
+      # Retest the console after shop install
+      - name: Check bin/console and cache:clear can run after shop install
+        run: |
+          docker ps
+          docker exec prestashop_prestashop-git_1 php ./bin/console
+          docker exec prestashop_prestashop-git_1 php -d memory_limit=-1 ./bin/console cache:clear --env=dev
+          docker exec prestashop_prestashop-git_1 php -d memory_limit=-1 ./bin/console cache:clear --env=prod
+          docker exec prestashop_prestashop-git_1 php ./bin/console cache:clear --env=dev --no-warmup
+          docker exec prestashop_prestashop-git_1 php ./bin/console cache:clear --env=prod --no-warmup

app/config/doctrine.yml

@@ -6,6 +6,7 @@ doctrine:
     connections:
       default:
         driver: pdo_mysql
+        wrapper_class: PrestaShopBundle\Doctrine\DatabaseConnection
         host: "%database_host%"
         port: "%database_port%"
         dbname: "%database_name%"

app/config/set_parameters.php

@@ -26,8 +26,29 @@
 
 use PrestaShopBundle\Install\Upgrade;
 
-$parametersFilepath = __DIR__  . '/parameters.php';
-$parameters = require $parametersFilepath;
+$parametersFilepath = __DIR__ . '/parameters.php';
+if (file_exists($parametersFilepath)) {
+    $parameters = require $parametersFilepath;
+} else {
+    // When no parameters file is present (before install) we define null values for mandatory parameters to avoid breaking the CI
+    $parameters = [
+        'parameters' => [
+            'secret' => 'secret',
+            'locale' => 'en',
+            'database_host' => '',
+            'database_port' => null,
+            'database_name' => '',
+            'database_user' => '',
+            'database_password' => '',
+            'database_prefix' => 'ps_',
+            'api_private_key' => null,
+            'api_public_key' => null,
+            'new_cookie_key' => null,
+            'ps_cache_enable' => null,
+            'ps_caching' => null,
+        ],
+    ];
+}
 
 if (!array_key_exists('parameters', $parameters)) {
     throw new \Exception('Missing "parameters" key in "parameters.php" configuration file');
@@ -55,7 +76,7 @@
     $adapters = [
         'array' => 'cache.adapter.array',
         'memcached' => 'cache.adapter.memcached',
-        'apcu' => 'cache.adapter.apcu'
+        'apcu' => 'cache.adapter.apcu',
     ];
 
     if (isset(

bin/console

@@ -13,7 +13,11 @@ use Symfony\Component\ErrorHandler\Debug;
 
 set_time_limit(0);
 
-require_once __DIR__ . '/../config/config.inc.php';
+try {
+  require_once __DIR__ . '/../config/config.inc.php';
+} catch (PrestaShopException) {
+  // Prevent breaking all CLI command when the shop is not installed
+}
 
 $input = new ArgvInput();
 $env = $input->getParameterOption(['--env', '-e'], getenv('SYMFONY_ENV') ?: 'dev');

classes/Tools.php

@@ -3784,6 +3784,11 @@ public static function getPath($url_base, $id_category, $path = '', $highlight =
 
     public static function redirectToInstall()
     {
+        // No redirection in CLI mode
+        if (Tools::isPHPCLI()) {
+            return;
+        }
+
         if (file_exists(__DIR__ . '/../install')) {
             if (defined('_PS_ADMIN_DIR_')) {
                 header('Location: ../install/');

classes/db/Db.php

@@ -214,7 +214,7 @@ public static function getInstance($master = true)
         static $id = 0;
 
         // This MUST not be declared with the class members because some defines (like _DB_SERVER_) may not exist yet (the constructor can be called directly with params)
-        if (!self::$_servers) {
+        if (!self::$_servers && defined('_DB_SERVER_') && defined('_DB_USER_') && defined('_DB_PASSWD_') && defined('_DB_NAME_')) {
             self::$_servers = [
                 ['server' => _DB_SERVER_, 'user' => _DB_USER_, 'password' => _DB_PASSWD_, 'database' => _DB_NAME_], /* MySQL Master server */
             ];
@@ -233,6 +233,10 @@ public static function getInstance($master = true)
         }
 
         if (!isset(self::$instance[$id_server])) {
+            if (!isset(self::$_servers[$id_server])) {
+                throw new PrestaShopException('Database server configuration not found');
+            }
+
             $class = Db::getClass();
             self::$instance[$id_server] = new $class(
                 self::$_servers[$id_server]['server'],

classes/exception/PrestaShopException.php

@@ -32,7 +32,7 @@ class PrestaShopExceptionCore extends Exception
     /**
      * This method acts like an error handler, if dev mode is on, display the error else use a better silent way.
      */
-    public function displayMessage()
+    public function displayMessage(bool $dieAfterDisplay = true)
     {
         if (getenv('kernel.environment') === 'test') {
             throw $this;
@@ -96,8 +96,11 @@ public function displayMessage()
         }
         // Log the error in the disk
         $this->logError();
-        //We only need the error code 1 in cli context
-        exit((int) ToolsCore::isPHPCLI());
+
+        if ($dieAfterDisplay) {
+            //We only need the error code 1 in cli context
+            exit((int) ToolsCore::isPHPCLI());
+        }
     }
 
     /**
@@ -203,7 +206,14 @@ protected function logError()
     {
         $logger = new FileLogger();
         $logger->setFilename(_PS_ROOT_DIR_ . '/var/logs/' . date('Ymd') . '_exception.log');
-        $logger->logError($this->getExtendedMessage(false));
+
+        try {
+            $logger->logError($this->getExtendedMessage(false));
+        } catch (PrestaShopException) {
+            // Catch exception because there is a hook executed in the AbstractLogger that is bound to fail when the DB
+            // is not accessible, there is no point adding some potential error in this method that is already logging
+            // a previous error, it would only confuse the error messages and cause an unwanted fatal error
+        }
     }
 
     /**

classes/shop/Shop.php

@@ -117,7 +117,7 @@ class ShopCore extends ObjectModel
     /** @var bool|null is multistore activated */
     protected static $feature_active;
 
-    /** @var Theme * */
+    /** @var Theme|null * */
     public $theme;
 
     /**

classes/shop/ShopUrl.php

@@ -197,13 +197,13 @@ public static function getMainShopDomain($id_shop = null)
     {
         ShopUrl::cacheMainDomainForShop($id_shop);
 
-        return self::$main_domain[(int) $id_shop];
+        return self::$main_domain[(int) $id_shop] ?? null;
     }
 
     public static function getMainShopDomainSSL($id_shop = null)
     {
         ShopUrl::cacheMainDomainForShop($id_shop);
 
-        return self::$main_domain_ssl[(int) $id_shop];
+        return self::$main_domain_ssl[(int) $id_shop] ?? null;
     }
 }

config/config.inc.php

@@ -55,6 +55,11 @@
 }
 
 require_once __DIR__ . DIRECTORY_SEPARATOR . 'bootstrap.php';
+// If this const is not defined others are likely to be absent but this one is the most likely to cause a fatal error,
+// the following initialization is going to fail, so we throw an exception early
+if (!defined('_DB_PREFIX_')) {
+    throw new PrestaShopException('Constant _DB_PREFIX_ not defined');
+}
 
 if (defined('_PS_CREATION_DATE_')) {
     $creationDate = _PS_CREATION_DATE_;
@@ -116,12 +121,25 @@
 try {
     $context->shop = Shop::initialize();
 } catch (PrestaShopException $e) {
-    $e->displayMessage();
+    // In CLI command the Shop initialization is bound to fail when the shop is not installed, but we don't want to stop
+    // the process or this will break any Symfony command even a simple ./bin/console)
+    $e->displayMessage(!ToolsCore::isPHPCLI());
+}
+
+if ($context->shop) {
+    define('__PS_BASE_URI__', $context->shop->getBaseURI());
+} else {
+    define('__PS_BASE_URI__', '/');
 }
-define('_THEME_NAME_', $context->shop->theme->getName());
-define('_PARENT_THEME_NAME_', $context->shop->theme->get('parent') ?: '');
 
-define('__PS_BASE_URI__', $context->shop->getBaseURI());
+if ($context->shop && $context->shop->theme) {
+    define('_THEME_NAME_', $context->shop->theme->getName());
+    define('_PARENT_THEME_NAME_', $context->shop->theme->get('parent') ?: '');
+} else {
+    // Not ideal fallback but on install when nothing else is available it does the job, better than not having these const at all
+    define('_THEME_NAME_', 'classic');
+    define('_PARENT_THEME_NAME_', '');
+}
 
 /* Include all defines related to base uri and theme name */
 require_once __DIR__ . '/defines_uri.inc.php';

install-dev/controllers/console/process.php

@@ -26,6 +26,7 @@
 
 use PrestaShopBundle\Install\Database;
 use PrestaShopBundle\Install\Install;
+use Symfony\Component\Console\Output\ConsoleOutput;
 use Symfony\Component\Filesystem\Filesystem;
 
 class InstallControllerConsoleProcess extends InstallControllerConsole implements HttpConfigureInterface
@@ -45,10 +46,13 @@ class InstallControllerConsoleProcess extends InstallControllerConsole implement
 
     public function init()
     {
-        $this->model_install = new Install();
+        $output = new ConsoleOutput();
+        $logger = new SymfonyConsoleLogger($output, PrestaShopLoggerInterface::DEBUG);
+
+        $this->model_install = new Install(null, null, $logger);
         $this->model_install->setTranslator($this->translator);
 
-        $this->model_database = new Database();
+        $this->model_database = new Database($logger);
         $this->model_database->setTranslator($this->translator);
     }
 

src/PrestaShopBundle/Doctrine/DatabaseConnection.php

@@ -0,0 +1,71 @@
+<?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 Open Software License (OSL 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/OSL-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 license@prestashop.com 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 <contact@prestashop.com>
+ * @copyright Since 2007 PrestaShop SA and Contributors
+ * @license   https://opensource.org/licenses/OSL-3.0 Open Software License (OSL 3.0)
+ */
+
+declare(strict_types=1);
+
+namespace PrestaShopBundle\Doctrine;
+
+use Doctrine\DBAL\Connection;
+use Doctrine\DBAL\Driver\Exception;
+use Doctrine\DBAL\Platforms\MySQL57Platform;
+
+/**
+ * This wrapper connection class is used to prevent a bug in CLI when the shop is not configured yet.
+ * The problem is that Doctrine warm up automatically tries to get the database driver version vy connecting
+ * to it, but when no configuration is set yet the connection obviously fails.
+ *
+ * There is a possible solution to force server_version in the doctrine DBAL configuration but since PrestaShop
+ * is a framework that can be installed with different database versions we can't hard-code it, besides forcing
+ * the version disables this automatic version detection feature.
+ *
+ * (See https://github.com/doctrine/DoctrineBundle/issues/673 for more details)
+ *
+ * So this class acts as a fallback when the connection tries to get the database platform but fails we catch the
+ * exception, we check if the parameters file has been configured. If it doesn't exist then we are in a case where
+ * the automatic detection failure can be ignored, in other case we throw the caught exception and change nothing to
+ * the original behaviour thus reducing the impact of this wrapper as much as possible.
+ *
+ * In the case we handle a fallback we use MySQL 5.7 as the default platform.
+ */
+class DatabaseConnection extends Connection
+{
+    private const PARAMETERS_FILE = __DIR__ . '/../../../app/config/parameters.php';
+
+    public function getDatabasePlatform()
+    {
+        try {
+            $detectedVersion = parent::getDatabasePlatform();
+        } catch (Exception $e) {
+            if (!file_exists(self::PARAMETERS_FILE)) {
+                return new MySQL57Platform();
+            }
+
+            throw $e;
+        }
+
+        return $detectedVersion;
+    }
+}

src/PrestaShopBundle/Install/AbstractInstall.php

@@ -47,7 +47,7 @@ abstract class AbstractInstall
     protected $errors = [];
 
     /**
-     * @var PrestaShopLoggerInterface
+     * @var PrestaShopLoggerInterface|null
      */
     protected $logger;