From ea57b46e54161e2b61503092f18f208d2bb07899 Mon Sep 17 00:00:00 2001 From: Oleh Posyniak Date: Tue, 23 May 2017 12:28:47 +0300 Subject: [PATCH 01/10] MAGETWO-68928: Fatal error after disabling module --- .../Magento/Framework/Console/Cli.php | 81 +++---------------- .../GenerationDirectoryAccessException.php | 45 +++++++++++ .../Console/GenerationDirectoryAccess.php | 61 ++++++++++---- .../Setup/Console/CompilerPreparation.php | 33 ++++++-- 4 files changed, 128 insertions(+), 92 deletions(-) create mode 100644 lib/internal/Magento/Framework/Console/Exception/GenerationDirectoryAccessException.php diff --git a/lib/internal/Magento/Framework/Console/Cli.php b/lib/internal/Magento/Framework/Console/Cli.php index 27a247107fc33..a7ece717476ad 100644 --- a/lib/internal/Magento/Framework/Console/Cli.php +++ b/lib/internal/Magento/Framework/Console/Cli.php @@ -11,7 +11,7 @@ use Magento\Framework\App\ProductMetadata; use Magento\Framework\App\State; use Magento\Framework\Composer\ComposerJsonFinder; -use Magento\Framework\Exception\FileSystemException; +use Magento\Framework\Console\Exception\GenerationDirectoryAccessException; use Magento\Framework\Filesystem\Driver\File; use Magento\Framework\ObjectManagerInterface; use Magento\Framework\Shell\ComplexParameter; @@ -91,20 +91,13 @@ public function __construct($name = 'UNKNOWN', $version = 'UNKNOWN') /** * {@inheritdoc} * - * @throws \Exception the exception in case of unexpected error + * @throws \Exception The exception in case of unexpected error */ public function doRun(Console\Input\InputInterface $input, Console\Output\OutputInterface $output) { $exitCode = parent::doRun($input, $output); if ($this->initException) { - $output->writeln( - "We're sorry, an error occurred. Try clearing the cache and code generation directories. " - . "By default, they are: " . $this->getDefaultDirectoryPath(DirectoryList::CACHE) . ", " - . $this->getDefaultDirectoryPath(DirectoryList::GENERATED_METADATA) . ", " - . $this->getDefaultDirectoryPath(DirectoryList::GENERATED_CODE) . ", and var/page_cache." - ); - throw $this->initException; } @@ -154,24 +147,17 @@ protected function getApplicationCommands() * Object Manager initialization. * * @return void - * @SuppressWarnings(PHPMD.ExitExpression) */ private function initObjectManager() { - try { - $params = (new ComplexParameter(self::INPUT_KEY_BOOTSTRAP))->mergeFromArgv($_SERVER, $_SERVER); - $params[Bootstrap::PARAM_REQUIRE_MAINTENANCE] = null; - - $this->objectManager = Bootstrap::create(BP, $params)->getObjectManager(); + $params = (new ComplexParameter(self::INPUT_KEY_BOOTSTRAP))->mergeFromArgv($_SERVER, $_SERVER); + $params[Bootstrap::PARAM_REQUIRE_MAINTENANCE] = null; - /** @var ObjectManagerProvider $omProvider */ - $omProvider = $this->serviceManager->get(ObjectManagerProvider::class); - $omProvider->setObjectManager($this->objectManager); - } catch (FileSystemException $exception) { - $this->writeGenerationDirectoryReadError(); + $this->objectManager = Bootstrap::create(BP, $params)->getObjectManager(); - exit(static::RETURN_FAILURE); - } + /** @var ObjectManagerProvider $omProvider */ + $omProvider = $this->serviceManager->get(ObjectManagerProvider::class); + $omProvider->setObjectManager($this->objectManager); } /** @@ -182,7 +168,7 @@ private function initObjectManager() * developer - application will be terminated * * @return void - * @SuppressWarnings(PHPMD.ExitExpression) + * @throws GenerationDirectoryAccessException If generation directory is read-only in developer mode */ private function assertGenerationPermissions() { @@ -197,9 +183,7 @@ private function assertGenerationPermissions() if ($state->getMode() !== State::MODE_PRODUCTION && !$generationDirectoryAccess->check() ) { - $this->writeGenerationDirectoryReadError(); - - exit(static::RETURN_FAILURE); + throw new GenerationDirectoryAccessException(); } } @@ -207,7 +191,7 @@ private function assertGenerationPermissions() * Checks whether compiler is being prepared. * * @return void - * @SuppressWarnings(PHPMD.ExitExpression) + * @throws GenerationDirectoryAccessException If generation directory is read-only */ private function assertCompilerPreparation() { @@ -222,33 +206,10 @@ private function assertCompilerPreparation() new File() ); - try { - $compilerPreparation->handleCompilerEnvironment(); - } catch (FileSystemException $e) { - $this->writeGenerationDirectoryReadError(); - - exit(static::RETURN_FAILURE); - } + $compilerPreparation->handleCompilerEnvironment(); } } - /** - * Writes read error to console. - * - * @return void - */ - private function writeGenerationDirectoryReadError() - { - $output = new \Symfony\Component\Console\Output\ConsoleOutput(); - $output->writeln( - '' - . 'Command line user does not have read and write permissions on ' - . $this->getDefaultDirectoryPath(DirectoryList::GENERATED_CODE) . ' directory. ' - . 'Please address this issue before using Magento command line.' - . '' - ); - } - /** * Retrieves vendor commands. * @@ -270,22 +231,4 @@ protected function getVendorCommands($objectManager) return $commands; } - - /** - * Get default directory path by code - * - * @param string $code - * @return string - */ - private function getDefaultDirectoryPath($code) - { - $config = DirectoryList::getDefaultConfig(); - $result = ''; - - if (isset($config[$code][DirectoryList::PATH])) { - $result = $config[$code][DirectoryList::PATH]; - } - - return $result; - } } diff --git a/lib/internal/Magento/Framework/Console/Exception/GenerationDirectoryAccessException.php b/lib/internal/Magento/Framework/Console/Exception/GenerationDirectoryAccessException.php new file mode 100644 index 0000000000000..90450d65ee7bc --- /dev/null +++ b/lib/internal/Magento/Framework/Console/Exception/GenerationDirectoryAccessException.php @@ -0,0 +1,45 @@ +getDefaultDirectoryPath(DirectoryList::GENERATED_CODE) . ' directory. ' + . 'Please address this issue before using Magento command line.' + ); + + parent::__construct($phrase, $cause, $code); + } + + /** + * Get default directory path by code + * + * @param string $code + * @return string + */ + private function getDefaultDirectoryPath($code) + { + $config = DirectoryList::getDefaultConfig(); + $result = ''; + + if (isset($config[$code][DirectoryList::PATH])) { + $result = $config[$code][DirectoryList::PATH]; + } + + return $result; + } +} diff --git a/lib/internal/Magento/Framework/Console/GenerationDirectoryAccess.php b/lib/internal/Magento/Framework/Console/GenerationDirectoryAccess.php index 7349872ff4ac1..37e1484a7d0cb 100644 --- a/lib/internal/Magento/Framework/Console/GenerationDirectoryAccess.php +++ b/lib/internal/Magento/Framework/Console/GenerationDirectoryAccess.php @@ -7,6 +7,7 @@ use Magento\Framework\App\Bootstrap; use Magento\Framework\App\Filesystem\DirectoryList; +use Magento\Framework\Filesystem\DriverInterface; use Magento\Framework\Filesystem\DriverPool; use Magento\Framework\Filesystem\File\WriteFactory; use Magento\Framework\Filesystem\Directory\Write; @@ -44,33 +45,63 @@ public function check() ? $initParams[Bootstrap::INIT_PARAM_FILESYSTEM_DIR_PATHS] : []; $directoryList = new DirectoryList(BP, $filesystemDirPaths); - $generationDirectoryPath = $directoryList->getPath(DirectoryList::GENERATED_CODE); $driverPool = new DriverPool(); $fileWriteFactory = new WriteFactory($driverPool); /** @var \Magento\Framework\Filesystem\DriverInterface $driver */ $driver = $driverPool->getDriver(DriverPool::FILE); - $directoryWrite = new Write($fileWriteFactory, $driver, $generationDirectoryPath); - if ($directoryWrite->isExist()) { - if ($directoryWrite->isDirectory() - || $directoryWrite->isReadable() - ) { - try { - $probeFilePath = $generationDirectoryPath . DIRECTORY_SEPARATOR . uniqid(mt_rand()).'tmp'; - $fileWriteFactory->create($probeFilePath, DriverPool::FILE, 'w'); - $driver->deleteFile($probeFilePath); - } catch (\Exception $e) { - return false; - } - } else { + + $generationDirs = [ + DirectoryList::GENERATED, + DirectoryList::GENERATED_CODE, + DirectoryList::GENERATED_METADATA + ]; + + foreach ($generationDirs as $generationDirectory) { + $directoryPath = $directoryList->getPath($generationDirectory); + + if (!$this->checkDirectory($fileWriteFactory, $driver, $directoryPath)) { return false; } - } else { + } + + return true; + } + + /** + * Checks the permissions to specific directory + * + * @param WriteFactory $fileWriteFactory The factory of file writers + * @param DriverInterface $driver The driver + * @param string $directoryPath The directory path + * @return bool + */ + private function checkDirectory( + WriteFactory $fileWriteFactory, + DriverInterface $driver, + $directoryPath + ) { + $directoryWrite = new Write($fileWriteFactory, $driver, $directoryPath); + + if (!$directoryWrite->isExist()) { try { $directoryWrite->create(); } catch (\Exception $e) { return false; } } + + if (!$directoryWrite->isDirectory() || !$directoryWrite->isReadable()) { + return false; + } + + try { + $probeFilePath = $directoryPath . DIRECTORY_SEPARATOR . uniqid(mt_rand()) . 'tmp'; + $fileWriteFactory->create($probeFilePath, DriverPool::FILE, 'w'); + $driver->deleteFile($probeFilePath); + } catch (\Exception $e) { + return false; + } + return true; } } diff --git a/setup/src/Magento/Setup/Console/CompilerPreparation.php b/setup/src/Magento/Setup/Console/CompilerPreparation.php index 6bdddfe0053a8..c043095263f2f 100644 --- a/setup/src/Magento/Setup/Console/CompilerPreparation.php +++ b/setup/src/Magento/Setup/Console/CompilerPreparation.php @@ -7,6 +7,7 @@ use Magento\Framework\App\Bootstrap; use Magento\Framework\App\Filesystem\DirectoryList; +use Magento\Framework\Console\Exception\GenerationDirectoryAccessException; use Magento\Framework\Console\GenerationDirectoryAccess; use Magento\Framework\Exception\FileSystemException; use Magento\Framework\Filesystem\Driver\File; @@ -59,30 +60,30 @@ public function __construct( /** * Determine whether a CLI command is for compilation, and if so, clear the directory. * - * @throws FileSystemException if generation directory is read-only + * @throws GenerationDirectoryAccessException If generation directory is read-only * @return void */ public function handleCompilerEnvironment() { - $compilationCommands = [DiCompileCommand::NAME]; + $compilationCommands = $this->getCompilerInvalidationCommands(); $cmdName = $this->input->getFirstArgument(); $isHelpOption = $this->input->hasParameterOption('--help') || $this->input->hasParameterOption('-h'); if (!in_array($cmdName, $compilationCommands) || $isHelpOption) { return; } - $compileDirList = []; + $mageInitParams = $this->serviceManager->get(InitParamListener::BOOTSTRAP_PARAM); $mageDirs = isset($mageInitParams[Bootstrap::INIT_PARAM_FILESYSTEM_DIR_PATHS]) ? $mageInitParams[Bootstrap::INIT_PARAM_FILESYSTEM_DIR_PATHS] : []; $directoryList = new DirectoryList(BP, $mageDirs); - $compileDirList[] = $directoryList->getPath(DirectoryList::GENERATED_CODE); - $compileDirList[] = $directoryList->getPath(DirectoryList::GENERATED_METADATA); + $compileDirList = [ + $directoryList->getPath(DirectoryList::GENERATED_CODE), + $directoryList->getPath(DirectoryList::GENERATED_METADATA), + ]; if (!$this->getGenerationDirectoryAccess()->check()) { - throw new FileSystemException( - new Phrase('Generation directory can not be written.') - ); + throw new GenerationDirectoryAccessException(); } foreach ($compileDirList as $compileDir) { @@ -92,6 +93,22 @@ public function handleCompilerEnvironment() } } + /** + * Retrieves command list with commands which invalidates compiler + * + * @return array + */ + private function getCompilerInvalidationCommands() + { + return [ + DiCompileCommand::NAME, + 'module:disable', + 'module:enable', + 'module:uninstall', + 'deploy:mode:set' + ]; + } + /** * Retrieves generation directory access checker. * From 5b868df1e5e26cf08f1ef80a14f48371a99bfe7c Mon Sep 17 00:00:00 2001 From: Oleh Posyniak Date: Wed, 24 May 2017 14:11:48 +0300 Subject: [PATCH 02/10] MAGETWO-69430: Magento allows to save not valid data using config:set command --- .../Command/ConfigSet/LockProcessor.php | 31 ++++++++++++------- .../Command/ConfigSet/LockProcessorTest.php | 4 +-- 2 files changed, 22 insertions(+), 13 deletions(-) diff --git a/app/code/Magento/Config/Console/Command/ConfigSet/LockProcessor.php b/app/code/Magento/Config/Console/Command/ConfigSet/LockProcessor.php index b8770482f0324..0bd28f0f78d96 100644 --- a/app/code/Magento/Config/Console/Command/ConfigSet/LockProcessor.php +++ b/app/code/Magento/Config/Console/Command/ConfigSet/LockProcessor.php @@ -8,6 +8,7 @@ use Magento\Config\App\Config\Type\System; use Magento\Config\Model\PreparedValueFactory; use Magento\Framework\App\Config\ConfigPathResolver; +use Magento\Framework\App\Config\Value; use Magento\Framework\App\DeploymentConfig; use Magento\Framework\Config\File\ConfigFilePool; use Magento\Framework\Exception\CouldNotSaveException; @@ -79,19 +80,27 @@ public function process($path, $value, $scope, $scopeCode) $configPath = $this->configPathResolver->resolve($path, $scope, $scopeCode, System::CONFIG_TYPE); $backendModel = $this->preparedValueFactory->create($path, $value, $scope, $scopeCode); - /** - * Temporary solution until Magento introduce unified interface - * for storing system configuration into database and configuration files. - */ - $backendModel->validateBeforeSave(); - $backendModel->beforeSave(); + if ($backendModel instanceof Value) { + /** + * Temporary solution until Magento introduce unified interface + * for storing system configuration into database and configuration files. + */ + $backendModel->validateBeforeSave(); + $backendModel->beforeSave(); - $this->deploymentConfigWriter->saveConfig( - [ConfigFilePool::APP_ENV => $this->arrayManager->set($configPath, [], $backendModel->getValue())], - false - ); + $value = $backendModel->getValue(); - $backendModel->afterSave(); + $backendModel->afterSave(); + + /** + * Because FS does not support transactions, + * we'll write value just after all validations are triggered. + */ + $this->deploymentConfigWriter->saveConfig( + [ConfigFilePool::APP_ENV => $this->arrayManager->set($configPath, [], $value)], + false + ); + } } catch (\Exception $exception) { throw new CouldNotSaveException(__('%1', $exception->getMessage()), $exception); } diff --git a/app/code/Magento/Config/Test/Unit/Console/Command/ConfigSet/LockProcessorTest.php b/app/code/Magento/Config/Test/Unit/Console/Command/ConfigSet/LockProcessorTest.php index e37214411ff51..62028eb789230 100644 --- a/app/code/Magento/Config/Test/Unit/Console/Command/ConfigSet/LockProcessorTest.php +++ b/app/code/Magento/Config/Test/Unit/Console/Command/ConfigSet/LockProcessorTest.php @@ -206,10 +206,10 @@ public function testCustomException() ->willReturn($this->valueMock); $this->arrayManagerMock->expects($this->never()) ->method('set'); - $this->valueMock->expects($this->never()) + $this->valueMock->expects($this->once()) ->method('getValue'); $this->valueMock->expects($this->once()) - ->method('validateBeforeSave') + ->method('afterSave') ->willThrowException(new \Exception('Invalid values')); $this->deploymentConfigWriterMock->expects($this->never()) ->method('saveConfig'); From 03802527f166bca56081e87edc91b4d3d9cb9594 Mon Sep 17 00:00:00 2001 From: Oleh Posyniak Date: Thu, 25 May 2017 11:52:12 +0300 Subject: [PATCH 03/10] MAGETWO-68928: Fatal error after disabling module --- .../Magento/Framework/Console/Cli.php | 38 ++++++++++++------- .../GenerationDirectoryAccessException.php | 5 ++- 2 files changed, 28 insertions(+), 15 deletions(-) diff --git a/lib/internal/Magento/Framework/Console/Cli.php b/lib/internal/Magento/Framework/Console/Cli.php index a7ece717476ad..da60ea0e5fe7d 100644 --- a/lib/internal/Magento/Framework/Console/Cli.php +++ b/lib/internal/Magento/Framework/Console/Cli.php @@ -66,23 +66,33 @@ class Cli extends Console\Application /** * @param string $name the application name * @param string $version the application version + * @SuppressWarnings(PHPMD.ExitExpression) */ public function __construct($name = 'UNKNOWN', $version = 'UNKNOWN') { - $configuration = require BP . '/setup/config/application.config.php'; - $bootstrapApplication = new Application(); - $application = $bootstrapApplication->bootstrap($configuration); - $this->serviceManager = $application->getServiceManager(); - - $this->assertCompilerPreparation(); - $this->initObjectManager(); - $this->assertGenerationPermissions(); - - if ($version == 'UNKNOWN') { - $directoryList = new DirectoryList(BP); - $composerJsonFinder = new ComposerJsonFinder($directoryList); - $productMetadata = new ProductMetadata($composerJsonFinder); - $version = $productMetadata->getVersion(); + try { + $configuration = require BP . '/setup/config/application.config.php'; + $bootstrapApplication = new Application(); + $application = $bootstrapApplication->bootstrap($configuration); + $this->serviceManager = $application->getServiceManager(); + + $this->assertCompilerPreparation(); + $this->initObjectManager(); + $this->assertGenerationPermissions(); + + if ($version == 'UNKNOWN') { + $directoryList = new DirectoryList(BP); + $composerJsonFinder = new ComposerJsonFinder($directoryList); + $productMetadata = new ProductMetadata($composerJsonFinder); + $version = $productMetadata->getVersion(); + } + } catch (\Exception $exception) { + $output = new Console\Output\ConsoleOutput(); + $output->writeln( + '' . $exception->getMessage() . '' + ); + + exit(static::RETURN_FAILURE); } parent::__construct($name, $version); diff --git a/lib/internal/Magento/Framework/Console/Exception/GenerationDirectoryAccessException.php b/lib/internal/Magento/Framework/Console/Exception/GenerationDirectoryAccessException.php index 90450d65ee7bc..fc65cc0362e24 100644 --- a/lib/internal/Magento/Framework/Console/Exception/GenerationDirectoryAccessException.php +++ b/lib/internal/Magento/Framework/Console/Exception/GenerationDirectoryAccessException.php @@ -9,6 +9,9 @@ use Magento\Framework\Exception\FileSystemException; use Magento\Framework\Phrase; +/** + * The default exception for missing write permissions on compilation generated folder. + */ class GenerationDirectoryAccessException extends FileSystemException { /** @@ -18,7 +21,7 @@ public function __construct(Phrase $phrase = null, \Exception $cause = null, $co { $phrase = $phrase ?: new Phrase( 'Command line user does not have read and write permissions on ' - . $this->getDefaultDirectoryPath(DirectoryList::GENERATED_CODE) . ' directory. ' + . $this->getDefaultDirectoryPath(DirectoryList::GENERATED) . ' directory. ' . 'Please address this issue before using Magento command line.' ); From 5fb3c26b69ace7fedb86787befc382c2fd0275fe Mon Sep 17 00:00:00 2001 From: Oleh Posyniak Date: Thu, 25 May 2017 12:52:16 +0300 Subject: [PATCH 04/10] MAGETWO-68928: Fatal error after disabling module --- .../Magento/Framework/Console/Cli.php | 16 +++++++------- ...GenerationDirectoryAccessExceptionTest.php | 21 +++++++++++++++++++ 2 files changed, 29 insertions(+), 8 deletions(-) create mode 100644 lib/internal/Magento/Framework/Console/Test/Unit/Exception/GenerationDirectoryAccessExceptionTest.php diff --git a/lib/internal/Magento/Framework/Console/Cli.php b/lib/internal/Magento/Framework/Console/Cli.php index da60ea0e5fe7d..fb8d3e3f28c3c 100644 --- a/lib/internal/Magento/Framework/Console/Cli.php +++ b/lib/internal/Magento/Framework/Console/Cli.php @@ -79,15 +79,8 @@ public function __construct($name = 'UNKNOWN', $version = 'UNKNOWN') $this->assertCompilerPreparation(); $this->initObjectManager(); $this->assertGenerationPermissions(); - - if ($version == 'UNKNOWN') { - $directoryList = new DirectoryList(BP); - $composerJsonFinder = new ComposerJsonFinder($directoryList); - $productMetadata = new ProductMetadata($composerJsonFinder); - $version = $productMetadata->getVersion(); - } } catch (\Exception $exception) { - $output = new Console\Output\ConsoleOutput(); + $output = new \Symfony\Component\Console\Output\ConsoleOutput(); $output->writeln( '' . $exception->getMessage() . '' ); @@ -95,6 +88,13 @@ public function __construct($name = 'UNKNOWN', $version = 'UNKNOWN') exit(static::RETURN_FAILURE); } + if ($version == 'UNKNOWN') { + $directoryList = new DirectoryList(BP); + $composerJsonFinder = new ComposerJsonFinder($directoryList); + $productMetadata = new ProductMetadata($composerJsonFinder); + $version = $productMetadata->getVersion(); + } + parent::__construct($name, $version); } diff --git a/lib/internal/Magento/Framework/Console/Test/Unit/Exception/GenerationDirectoryAccessExceptionTest.php b/lib/internal/Magento/Framework/Console/Test/Unit/Exception/GenerationDirectoryAccessExceptionTest.php new file mode 100644 index 0000000000000..fcc12c8a76bd3 --- /dev/null +++ b/lib/internal/Magento/Framework/Console/Test/Unit/Exception/GenerationDirectoryAccessExceptionTest.php @@ -0,0 +1,21 @@ +assertContains( + 'Command line user does not have read and write permissions on generated directory.', + $exception->getMessage() + ); + } +} From 743fce0d0f4e81618843ee11a8a54b6064e7e221 Mon Sep 17 00:00:00 2001 From: Oleh Posyniak Date: Thu, 25 May 2017 15:04:15 +0300 Subject: [PATCH 05/10] MAGETWO-68928: Fatal error after disabling module --- .../Console/GenerationDirectoryAccess.php | 53 ++++--------------- .../Setup/Console/CompilerPreparation.php | 8 +-- 2 files changed, 14 insertions(+), 47 deletions(-) diff --git a/lib/internal/Magento/Framework/Console/GenerationDirectoryAccess.php b/lib/internal/Magento/Framework/Console/GenerationDirectoryAccess.php index 37e1484a7d0cb..b8978c9ef7181 100644 --- a/lib/internal/Magento/Framework/Console/GenerationDirectoryAccess.php +++ b/lib/internal/Magento/Framework/Console/GenerationDirectoryAccess.php @@ -7,10 +7,8 @@ use Magento\Framework\App\Bootstrap; use Magento\Framework\App\Filesystem\DirectoryList; -use Magento\Framework\Filesystem\DriverInterface; +use Magento\Framework\Filesystem\Directory\WriteFactory; use Magento\Framework\Filesystem\DriverPool; -use Magento\Framework\Filesystem\File\WriteFactory; -use Magento\Framework\Filesystem\Directory\Write; use Zend\ServiceManager\ServiceManager; use Magento\Setup\Mvc\Bootstrap\InitParamListener; @@ -34,7 +32,7 @@ public function __construct( } /** - * Check generated/code read and write access + * Check write permissions to generation folders * * @return bool */ @@ -47,8 +45,6 @@ public function check() $directoryList = new DirectoryList(BP, $filesystemDirPaths); $driverPool = new DriverPool(); $fileWriteFactory = new WriteFactory($driverPool); - /** @var \Magento\Framework\Filesystem\DriverInterface $driver */ - $driver = $driverPool->getDriver(DriverPool::FILE); $generationDirs = [ DirectoryList::GENERATED, @@ -58,50 +54,21 @@ public function check() foreach ($generationDirs as $generationDirectory) { $directoryPath = $directoryList->getPath($generationDirectory); + $directoryWrite = $fileWriteFactory->create($directoryPath); - if (!$this->checkDirectory($fileWriteFactory, $driver, $directoryPath)) { - return false; + if (!$directoryWrite->isExist()) { + try { + $directoryWrite->create(); + } catch (\Exception $e) { + return false; + } } - } - - return true; - } - /** - * Checks the permissions to specific directory - * - * @param WriteFactory $fileWriteFactory The factory of file writers - * @param DriverInterface $driver The driver - * @param string $directoryPath The directory path - * @return bool - */ - private function checkDirectory( - WriteFactory $fileWriteFactory, - DriverInterface $driver, - $directoryPath - ) { - $directoryWrite = new Write($fileWriteFactory, $driver, $directoryPath); - - if (!$directoryWrite->isExist()) { - try { - $directoryWrite->create(); - } catch (\Exception $e) { + if (!$directoryWrite->isWritable()) { return false; } } - if (!$directoryWrite->isDirectory() || !$directoryWrite->isReadable()) { - return false; - } - - try { - $probeFilePath = $directoryPath . DIRECTORY_SEPARATOR . uniqid(mt_rand()) . 'tmp'; - $fileWriteFactory->create($probeFilePath, DriverPool::FILE, 'w'); - $driver->deleteFile($probeFilePath); - } catch (\Exception $e) { - return false; - } - return true; } } diff --git a/setup/src/Magento/Setup/Console/CompilerPreparation.php b/setup/src/Magento/Setup/Console/CompilerPreparation.php index c043095263f2f..9ea938d51fb37 100644 --- a/setup/src/Magento/Setup/Console/CompilerPreparation.php +++ b/setup/src/Magento/Setup/Console/CompilerPreparation.php @@ -72,6 +72,10 @@ public function handleCompilerEnvironment() return; } + if (!$this->getGenerationDirectoryAccess()->check()) { + throw new GenerationDirectoryAccessException(); + } + $mageInitParams = $this->serviceManager->get(InitParamListener::BOOTSTRAP_PARAM); $mageDirs = isset($mageInitParams[Bootstrap::INIT_PARAM_FILESYSTEM_DIR_PATHS]) ? $mageInitParams[Bootstrap::INIT_PARAM_FILESYSTEM_DIR_PATHS] @@ -82,10 +86,6 @@ public function handleCompilerEnvironment() $directoryList->getPath(DirectoryList::GENERATED_METADATA), ]; - if (!$this->getGenerationDirectoryAccess()->check()) { - throw new GenerationDirectoryAccessException(); - } - foreach ($compileDirList as $compileDir) { if ($this->filesystemDriver->isExists($compileDir)) { $this->filesystemDriver->deleteDirectory($compileDir); From 6df7cc3c57c3c4d34119fc8e628937680c0e8756 Mon Sep 17 00:00:00 2001 From: Oleh Posyniak Date: Thu, 1 Jun 2017 15:58:33 +0300 Subject: [PATCH 06/10] MAGETWO-69430: Magento allows to save not valid data using config:set command --- .../Command/ConfigSet/LockProcessor.php | 17 ++------- .../Command/ConfigSet/LockProcessorTest.php | 37 ++++++++++++++----- 2 files changed, 30 insertions(+), 24 deletions(-) diff --git a/app/code/Magento/Config/Console/Command/ConfigSet/LockProcessor.php b/app/code/Magento/Config/Console/Command/ConfigSet/LockProcessor.php index 0bd28f0f78d96..ff91dc31887b1 100644 --- a/app/code/Magento/Config/Console/Command/ConfigSet/LockProcessor.php +++ b/app/code/Magento/Config/Console/Command/ConfigSet/LockProcessor.php @@ -81,24 +81,13 @@ public function process($path, $value, $scope, $scopeCode) $backendModel = $this->preparedValueFactory->create($path, $value, $scope, $scopeCode); if ($backendModel instanceof Value) { - /** - * Temporary solution until Magento introduce unified interface - * for storing system configuration into database and configuration files. - */ - $backendModel->validateBeforeSave(); - $backendModel->beforeSave(); + $resourceModel = $backendModel->getResource(); + $resourceModel->save($backendModel); $value = $backendModel->getValue(); - $backendModel->afterSave(); - - /** - * Because FS does not support transactions, - * we'll write value just after all validations are triggered. - */ $this->deploymentConfigWriter->saveConfig( - [ConfigFilePool::APP_ENV => $this->arrayManager->set($configPath, [], $value)], - false + [ConfigFilePool::APP_ENV => $this->arrayManager->set($configPath, [], $value)] ); } } catch (\Exception $exception) { diff --git a/app/code/Magento/Config/Test/Unit/Console/Command/ConfigSet/LockProcessorTest.php b/app/code/Magento/Config/Test/Unit/Console/Command/ConfigSet/LockProcessorTest.php index 62028eb789230..8bf19b9eeacf7 100644 --- a/app/code/Magento/Config/Test/Unit/Console/Command/ConfigSet/LockProcessorTest.php +++ b/app/code/Magento/Config/Test/Unit/Console/Command/ConfigSet/LockProcessorTest.php @@ -13,6 +13,7 @@ use Magento\Framework\App\DeploymentConfig; use Magento\Framework\Config\File\ConfigFilePool; use Magento\Framework\Exception\FileSystemException; +use Magento\Framework\Model\ResourceModel\AbstractResource; use Magento\Framework\Stdlib\ArrayManager; use Magento\Store\Model\ScopeInterface; use PHPUnit_Framework_MockObject_MockObject as Mock; @@ -55,6 +56,11 @@ class LockProcessorTest extends \PHPUnit_Framework_TestCase */ private $valueMock; + /** + * @var AbstractResource|Mock + */ + private $resourceMock; + /** * @inheritdoc */ @@ -73,9 +79,13 @@ protected function setUp() ->disableOriginalConstructor() ->getMock(); $this->valueMock = $this->getMockBuilder(Value::class) - ->setMethods(['validateBeforeSave', 'beforeSave', 'setValue', 'getValue', 'afterSave']) + ->setMethods(['save', 'getResource', 'setValue', 'getValue', 'afterSave']) ->disableOriginalConstructor() ->getMock(); + $this->resourceMock = $this->getMockBuilder(AbstractResource::class) + ->setMethods(['save']) + ->disableOriginalConstructor() + ->getMockForAbstractClass(); $this->model = new LockProcessor( $this->preparedValueFactory, @@ -120,6 +130,12 @@ public function testProcess($path, $value, $scope, $scopeCode) $this->valueMock->expects($this->once()) ->method('getValue') ->willReturn($value); + $this->valueMock->expects($this->once()) + ->method('getResource') + ->willReturn($this->resourceMock); + $this->resourceMock->expects($this->once()) + ->method('save') + ->with($this->valueMock); $this->deploymentConfigWriterMock->expects($this->once()) ->method('saveConfig') ->with( @@ -138,12 +154,6 @@ public function testProcess($path, $value, $scope, $scopeCode) ], false ); - $this->valueMock->expects($this->once()) - ->method('validateBeforeSave'); - $this->valueMock->expects($this->once()) - ->method('beforeSave'); - $this->valueMock->expects($this->once()) - ->method('afterSave'); $this->model->process($path, $value, $scope, $scopeCode); } @@ -175,6 +185,12 @@ public function testProcessNotReadableFs() $this->valueMock->expects($this->once()) ->method('getValue') ->willReturn($value); + $this->valueMock->expects($this->once()) + ->method('getResource') + ->willReturn($this->resourceMock); + $this->resourceMock->expects($this->once()) + ->method('save') + ->with($this->valueMock); $this->configPathResolver->expects($this->once()) ->method('resolve') ->willReturn('system/default/test/test/test'); @@ -207,9 +223,10 @@ public function testCustomException() $this->arrayManagerMock->expects($this->never()) ->method('set'); $this->valueMock->expects($this->once()) - ->method('getValue'); - $this->valueMock->expects($this->once()) - ->method('afterSave') + ->method('getResource') + ->willReturn($this->resourceMock); + $this->resourceMock->expects($this->once()) + ->method('save') ->willThrowException(new \Exception('Invalid values')); $this->deploymentConfigWriterMock->expects($this->never()) ->method('saveConfig'); From 3599386cfa2aad2c910d00f7421f97f5e3df1ce6 Mon Sep 17 00:00:00 2001 From: Oleh Posyniak Date: Thu, 1 Jun 2017 16:49:16 +0300 Subject: [PATCH 07/10] MAGETWO-69347: Integration plan L4 random fail --- .../ResourceModel/Catalog/ProductTest.php | 20 ++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/dev/tests/integration/testsuite/Magento/Sitemap/Model/ResourceModel/Catalog/ProductTest.php b/dev/tests/integration/testsuite/Magento/Sitemap/Model/ResourceModel/Catalog/ProductTest.php index 9d1ad1095b11a..180fd5df17b80 100644 --- a/dev/tests/integration/testsuite/Magento/Sitemap/Model/ResourceModel/Catalog/ProductTest.php +++ b/dev/tests/integration/testsuite/Magento/Sitemap/Model/ResourceModel/Catalog/ProductTest.php @@ -5,15 +5,33 @@ */ namespace Magento\Sitemap\Model\ResourceModel\Catalog; +use Magento\Indexer\Model\Indexer; +use Magento\TestFramework\Helper\Bootstrap; + /** * Test class for \Magento\Sitemap\Model\ResourceModel\Catalog\Product. * - test products collection generation for sitemap * - * @magentoDataFixtureBeforeTransaction Magento/CatalogSearch/_files/full_reindex.php * @magentoDataFixture Magento/Sitemap/_files/sitemap_products.php */ class ProductTest extends \PHPUnit_Framework_TestCase { + /** + * @var Indexer + */ + private $indexer; + + /** + * @inheritdoc + */ + protected function setUp() + { + /** @var Indexer indexer */ + $this->indexer = Bootstrap::getObjectManager()->create(Indexer::class); + $this->indexer->load('catalogsearch_fulltext'); + $this->indexer->reindexAll(); + } + /** * Test getCollection None images * 1) Check that image attributes were not loaded From c3aa3190b8f34ebc8e8a7cf825f1b15d3d2d183a Mon Sep 17 00:00:00 2001 From: Oleh Posyniak Date: Fri, 2 Jun 2017 12:09:40 +0300 Subject: [PATCH 08/10] MAGETWO-69347: Integration plan L4 random fail --- .../Model/ResourceModel/Catalog/ProductTest.php | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/dev/tests/integration/testsuite/Magento/Sitemap/Model/ResourceModel/Catalog/ProductTest.php b/dev/tests/integration/testsuite/Magento/Sitemap/Model/ResourceModel/Catalog/ProductTest.php index 180fd5df17b80..70c3ba9a40aba 100644 --- a/dev/tests/integration/testsuite/Magento/Sitemap/Model/ResourceModel/Catalog/ProductTest.php +++ b/dev/tests/integration/testsuite/Magento/Sitemap/Model/ResourceModel/Catalog/ProductTest.php @@ -12,26 +12,11 @@ * Test class for \Magento\Sitemap\Model\ResourceModel\Catalog\Product. * - test products collection generation for sitemap * + * @magentoDataFixtureBeforeTransaction Magento/Catalog/_files/enable_reindex_schedule.php * @magentoDataFixture Magento/Sitemap/_files/sitemap_products.php */ class ProductTest extends \PHPUnit_Framework_TestCase { - /** - * @var Indexer - */ - private $indexer; - - /** - * @inheritdoc - */ - protected function setUp() - { - /** @var Indexer indexer */ - $this->indexer = Bootstrap::getObjectManager()->create(Indexer::class); - $this->indexer->load('catalogsearch_fulltext'); - $this->indexer->reindexAll(); - } - /** * Test getCollection None images * 1) Check that image attributes were not loaded From 0b3b161ce35e6a2c11bcab5582dfa316d140b71a Mon Sep 17 00:00:00 2001 From: Oleh Posyniak Date: Tue, 6 Jun 2017 10:51:19 +0300 Subject: [PATCH 09/10] MAGETWO-69347: Integration plan L4 random fail --- .../Sitemap/Model/ResourceModel/Catalog/ProductTest.php | 3 --- 1 file changed, 3 deletions(-) diff --git a/dev/tests/integration/testsuite/Magento/Sitemap/Model/ResourceModel/Catalog/ProductTest.php b/dev/tests/integration/testsuite/Magento/Sitemap/Model/ResourceModel/Catalog/ProductTest.php index 70c3ba9a40aba..62b97e54bf0f0 100644 --- a/dev/tests/integration/testsuite/Magento/Sitemap/Model/ResourceModel/Catalog/ProductTest.php +++ b/dev/tests/integration/testsuite/Magento/Sitemap/Model/ResourceModel/Catalog/ProductTest.php @@ -5,9 +5,6 @@ */ namespace Magento\Sitemap\Model\ResourceModel\Catalog; -use Magento\Indexer\Model\Indexer; -use Magento\TestFramework\Helper\Bootstrap; - /** * Test class for \Magento\Sitemap\Model\ResourceModel\Catalog\Product. * - test products collection generation for sitemap From a9f545cc56051adeb6bee6922eef8cc999d92a06 Mon Sep 17 00:00:00 2001 From: Oleh Posyniak Date: Tue, 6 Jun 2017 14:31:03 +0300 Subject: [PATCH 10/10] MAGETWO-69430: Magento allows to save not valid data using config:set command --- .../Command/ConfigSet/LockProcessor.php | 17 +++++++-- .../Command/ConfigSet/LockProcessorTest.php | 37 +++++-------------- 2 files changed, 24 insertions(+), 30 deletions(-) diff --git a/app/code/Magento/Config/Console/Command/ConfigSet/LockProcessor.php b/app/code/Magento/Config/Console/Command/ConfigSet/LockProcessor.php index ff91dc31887b1..0bd28f0f78d96 100644 --- a/app/code/Magento/Config/Console/Command/ConfigSet/LockProcessor.php +++ b/app/code/Magento/Config/Console/Command/ConfigSet/LockProcessor.php @@ -81,13 +81,24 @@ public function process($path, $value, $scope, $scopeCode) $backendModel = $this->preparedValueFactory->create($path, $value, $scope, $scopeCode); if ($backendModel instanceof Value) { - $resourceModel = $backendModel->getResource(); - $resourceModel->save($backendModel); + /** + * Temporary solution until Magento introduce unified interface + * for storing system configuration into database and configuration files. + */ + $backendModel->validateBeforeSave(); + $backendModel->beforeSave(); $value = $backendModel->getValue(); + $backendModel->afterSave(); + + /** + * Because FS does not support transactions, + * we'll write value just after all validations are triggered. + */ $this->deploymentConfigWriter->saveConfig( - [ConfigFilePool::APP_ENV => $this->arrayManager->set($configPath, [], $value)] + [ConfigFilePool::APP_ENV => $this->arrayManager->set($configPath, [], $value)], + false ); } } catch (\Exception $exception) { diff --git a/app/code/Magento/Config/Test/Unit/Console/Command/ConfigSet/LockProcessorTest.php b/app/code/Magento/Config/Test/Unit/Console/Command/ConfigSet/LockProcessorTest.php index 8bf19b9eeacf7..62028eb789230 100644 --- a/app/code/Magento/Config/Test/Unit/Console/Command/ConfigSet/LockProcessorTest.php +++ b/app/code/Magento/Config/Test/Unit/Console/Command/ConfigSet/LockProcessorTest.php @@ -13,7 +13,6 @@ use Magento\Framework\App\DeploymentConfig; use Magento\Framework\Config\File\ConfigFilePool; use Magento\Framework\Exception\FileSystemException; -use Magento\Framework\Model\ResourceModel\AbstractResource; use Magento\Framework\Stdlib\ArrayManager; use Magento\Store\Model\ScopeInterface; use PHPUnit_Framework_MockObject_MockObject as Mock; @@ -56,11 +55,6 @@ class LockProcessorTest extends \PHPUnit_Framework_TestCase */ private $valueMock; - /** - * @var AbstractResource|Mock - */ - private $resourceMock; - /** * @inheritdoc */ @@ -79,13 +73,9 @@ protected function setUp() ->disableOriginalConstructor() ->getMock(); $this->valueMock = $this->getMockBuilder(Value::class) - ->setMethods(['save', 'getResource', 'setValue', 'getValue', 'afterSave']) + ->setMethods(['validateBeforeSave', 'beforeSave', 'setValue', 'getValue', 'afterSave']) ->disableOriginalConstructor() ->getMock(); - $this->resourceMock = $this->getMockBuilder(AbstractResource::class) - ->setMethods(['save']) - ->disableOriginalConstructor() - ->getMockForAbstractClass(); $this->model = new LockProcessor( $this->preparedValueFactory, @@ -130,12 +120,6 @@ public function testProcess($path, $value, $scope, $scopeCode) $this->valueMock->expects($this->once()) ->method('getValue') ->willReturn($value); - $this->valueMock->expects($this->once()) - ->method('getResource') - ->willReturn($this->resourceMock); - $this->resourceMock->expects($this->once()) - ->method('save') - ->with($this->valueMock); $this->deploymentConfigWriterMock->expects($this->once()) ->method('saveConfig') ->with( @@ -154,6 +138,12 @@ public function testProcess($path, $value, $scope, $scopeCode) ], false ); + $this->valueMock->expects($this->once()) + ->method('validateBeforeSave'); + $this->valueMock->expects($this->once()) + ->method('beforeSave'); + $this->valueMock->expects($this->once()) + ->method('afterSave'); $this->model->process($path, $value, $scope, $scopeCode); } @@ -185,12 +175,6 @@ public function testProcessNotReadableFs() $this->valueMock->expects($this->once()) ->method('getValue') ->willReturn($value); - $this->valueMock->expects($this->once()) - ->method('getResource') - ->willReturn($this->resourceMock); - $this->resourceMock->expects($this->once()) - ->method('save') - ->with($this->valueMock); $this->configPathResolver->expects($this->once()) ->method('resolve') ->willReturn('system/default/test/test/test'); @@ -223,10 +207,9 @@ public function testCustomException() $this->arrayManagerMock->expects($this->never()) ->method('set'); $this->valueMock->expects($this->once()) - ->method('getResource') - ->willReturn($this->resourceMock); - $this->resourceMock->expects($this->once()) - ->method('save') + ->method('getValue'); + $this->valueMock->expects($this->once()) + ->method('afterSave') ->willThrowException(new \Exception('Invalid values')); $this->deploymentConfigWriterMock->expects($this->never()) ->method('saveConfig');