From f2369aed7c6e66224a480f9ee98401c714dd1819 Mon Sep 17 00:00:00 2001 From: Alexis Guyomar Date: Mon, 21 Oct 2024 11:35:40 +0200 Subject: [PATCH] fix: remove php tags --- classes/Task/Update/UpdateModules.php | 2 +- .../UpgradeTools/Module/ModuleMigration.php | 35 ++++++++++--------- .../Module/ModuleMigrationTest.php | 28 ++++++--------- 3 files changed, 29 insertions(+), 36 deletions(-) diff --git a/classes/Task/Update/UpdateModules.php b/classes/Task/Update/UpdateModules.php index 20a6b33991..770ae15bef 100644 --- a/classes/Task/Update/UpdateModules.php +++ b/classes/Task/Update/UpdateModules.php @@ -69,7 +69,7 @@ public function run(): int $moduleSourceList = new ModuleSourceAggregate($this->container->getModuleSourceProviders()); $moduleDownloader = new ModuleDownloader($this->translator, $this->logger, $this->container->getProperty(UpgradeContainer::TMP_PATH)); $moduleUnzipper = new ModuleUnzipper($this->translator, $this->container->getZipAction(), $modulesPath); - $moduleMigration = new ModuleMigration($this->translator, $this->logger); + $moduleMigration = new ModuleMigration($this->translator, $this->logger, $this->container->getProperty(UpgradeContainer::TMP_PATH)); if ($listModules->getRemainingTotal()) { $moduleInfos = $listModules->getNext(); diff --git a/classes/UpgradeTools/Module/ModuleMigration.php b/classes/UpgradeTools/Module/ModuleMigration.php index 9ee3894f22..48211b86e5 100644 --- a/classes/UpgradeTools/Module/ModuleMigration.php +++ b/classes/UpgradeTools/Module/ModuleMigration.php @@ -41,10 +41,14 @@ class ModuleMigration /** @var Logger */ private $logger; - public function __construct(Translator $translator, Logger $logger) + /** @var string */ + private $sandboxFolder; + + public function __construct(Translator $translator, Logger $logger, string $sandboxFolder) { $this->translator = $translator; $this->logger = $logger; + $this->sandboxFolder = $sandboxFolder; } public function needMigration(ModuleMigrationContext $moduleMigrationContext): bool @@ -106,13 +110,15 @@ public function runMigration(ModuleMigrationContext $moduleMigrationContext): vo $methodName = $this->getUpgradeMethodName($migrationFilePath); try { - $this->loadAndCallFunction($migrationFilePath, $methodName, $moduleMigrationContext); + if (!$this->loadAndCallFunction($migrationFilePath, $methodName, $moduleMigrationContext)) { + throw (new UpgradeException($this->translator->trans('[WARNING] Migration failed while running the file %s. Module %s disabled.', [basename($migrationFilePath), $moduleMigrationContext->getModuleName()])))->setSeverity(UpgradeException::SEVERITY_WARNING); + } } catch (UpgradeException $e) { $moduleMigrationContext->getModuleInstance()->disable(); throw $e; } catch (Throwable $t) { $moduleMigrationContext->getModuleInstance()->disable(); - throw new UpgradeException($this->translator->trans('[WARNING] Unexpected error when trying to upgrade module %s. Module %s disabled.', [$moduleMigrationContext->getModuleName(), $moduleMigrationContext->getModuleName()]), 0, $t); + throw (new UpgradeException($this->translator->trans('[WARNING] Unexpected error when trying to upgrade module %s. Module %s disabled.', [$moduleMigrationContext->getModuleName(), $moduleMigrationContext->getModuleName()]), 0, $t))->setSeverity(UpgradeException::SEVERITY_WARNING); } } } @@ -147,23 +153,18 @@ private function loadAndCallFunction(string $filePath, string $methodName, Modul { $uniqueMethodName = $moduleMigrationContext->getModuleName() . '_' . $methodName; - $fileContent = file_get_contents($filePath); + $sandboxedFilePath = $this->sandboxFolder . DIRECTORY_SEPARATOR . $uniqueMethodName . '.php'; + $pushedFileContents = file_put_contents($sandboxedFilePath, str_replace($methodName, $uniqueMethodName, file_get_contents($filePath))); - if ($fileContent === false) { - throw new UpgradeException(sprintf('[WARNING] Could not read file %s.', $filePath)); + if ($pushedFileContents === false) { + throw (new UpgradeException($this->translator->trans('[WARNING] Could not write temporary file %s.', [$sandboxedFilePath])))->setSeverity(UpgradeException::SEVERITY_WARNING); } - $fileContent = str_replace($methodName, $uniqueMethodName, $fileContent); - $fileContent = str_replace([''], '', $fileContent); - - return (function () use ($fileContent, $uniqueMethodName, $moduleMigrationContext) { - eval($fileContent); - - if (!function_exists($uniqueMethodName)) { - throw new UpgradeException(sprintf('[WARNING] Method %s does not exist in evaluated file.', $uniqueMethodName)); - } + require_once $sandboxedFilePath; + if (!function_exists($uniqueMethodName)) { + throw (new UpgradeException($this->translator->trans('[WARNING] Method %s does not exist. Module %s disabled.', [$uniqueMethodName, $moduleMigrationContext->getModuleName()])))->setSeverity(UpgradeException::SEVERITY_WARNING); + } - return call_user_func($uniqueMethodName, $moduleMigrationContext->getModuleInstance()); - })(); + return call_user_func($uniqueMethodName, $moduleMigrationContext->getModuleInstance()); } } diff --git a/tests/unit/UpgradeTools/Module/ModuleMigrationTest.php b/tests/unit/UpgradeTools/Module/ModuleMigrationTest.php index 5cae1c6187..9e58504c5e 100644 --- a/tests/unit/UpgradeTools/Module/ModuleMigrationTest.php +++ b/tests/unit/UpgradeTools/Module/ModuleMigrationTest.php @@ -42,6 +42,14 @@ class ModuleMigrationTest extends TestCase */ private $moduleMigration; + private static $fixtureFolder; + + public static function setUpBeforeClass() + { + self::$fixtureFolder = sys_get_temp_dir() . '/fakeMigrationFilesDestination'; + @mkdir(self::$fixtureFolder); + } + protected function setUp(): void { if (!defined('_PS_MODULE_DIR_')) { @@ -58,7 +66,7 @@ protected function setUp(): void }); $this->logger = $this->createMock(Logger::class); - $this->moduleMigration = new ModuleMigration($translator, $this->logger); + $this->moduleMigration = new ModuleMigration($translator, $this->logger, self::$fixtureFolder); } public function testNeedMigrationWithSameVersion() @@ -179,22 +187,6 @@ public function testRunMigrationWithXYZDifferentFiles() $this->moduleMigration->runMigration($moduleMigrationContext); } - public function testRunMigrationWithSameInstanceThrowDuplicateMethod() - { - $mymodule = new \fixtures\mymodule\mymodule(); - $mymodule->version = '1.1.1'; - $dbVersion = '0.0.9'; - - $moduleMigrationContext = new ModuleMigrationContext($mymodule, $dbVersion); - - $this->moduleMigration->needMigration($moduleMigrationContext); - - $this->expectException(\PrestaShop\Module\AutoUpgrade\Exceptions\UpgradeException::class); - $this->expectExceptionMessage('[WARNING] Method upgrade_module_1 already exists. Migration for module mymodule aborted, you can try again later on the module manager. Module mymodule disabled.'); - - $this->moduleMigration->runMigration($moduleMigrationContext); - } - public function testRunMigrationWithBadUpgradeMethodName() { $mymodule = new \fixtures\mymodule\mymodule(); @@ -206,7 +198,7 @@ public function testRunMigrationWithBadUpgradeMethodName() $this->moduleMigration->needMigration($moduleMigrationContext); $this->expectException(\PrestaShop\Module\AutoUpgrade\Exceptions\UpgradeException::class); - $this->expectExceptionMessage('[WARNING] Method upgrade_module_1_2_0 does not exist. Module mymodule disabled.'); + $this->expectExceptionMessage('[WARNING] Method mymodule_upgrade_module_1_2_0 does not exist. Module mymodule disabled.'); $this->moduleMigration->runMigration($moduleMigrationContext); }