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'); 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..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 @@ -9,7 +9,7 @@ * Test class for \Magento\Sitemap\Model\ResourceModel\Catalog\Product. * - test products collection generation for sitemap * - * @magentoDataFixtureBeforeTransaction Magento/CatalogSearch/_files/full_reindex.php + * @magentoDataFixtureBeforeTransaction Magento/Catalog/_files/enable_reindex_schedule.php * @magentoDataFixture Magento/Sitemap/_files/sitemap_products.php */ class ProductTest extends \PHPUnit_Framework_TestCase diff --git a/lib/internal/Magento/Framework/Console/Cli.php b/lib/internal/Magento/Framework/Console/Cli.php index 27a247107fc33..fb8d3e3f28c3c 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; @@ -66,17 +66,27 @@ 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(); + 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(); + } catch (\Exception $exception) { + $output = new \Symfony\Component\Console\Output\ConsoleOutput(); + $output->writeln( + '' . $exception->getMessage() . '' + ); - $this->assertCompilerPreparation(); - $this->initObjectManager(); - $this->assertGenerationPermissions(); + exit(static::RETURN_FAILURE); + } if ($version == 'UNKNOWN') { $directoryList = new DirectoryList(BP); @@ -91,20 +101,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 +157,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 +178,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 +193,7 @@ private function assertGenerationPermissions() if ($state->getMode() !== State::MODE_PRODUCTION && !$generationDirectoryAccess->check() ) { - $this->writeGenerationDirectoryReadError(); - - exit(static::RETURN_FAILURE); + throw new GenerationDirectoryAccessException(); } } @@ -207,7 +201,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 +216,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 +241,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..fc65cc0362e24 --- /dev/null +++ b/lib/internal/Magento/Framework/Console/Exception/GenerationDirectoryAccessException.php @@ -0,0 +1,48 @@ +getDefaultDirectoryPath(DirectoryList::GENERATED) . ' 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..b8978c9ef7181 100644 --- a/lib/internal/Magento/Framework/Console/GenerationDirectoryAccess.php +++ b/lib/internal/Magento/Framework/Console/GenerationDirectoryAccess.php @@ -7,9 +7,8 @@ use Magento\Framework\App\Bootstrap; use Magento\Framework\App\Filesystem\DirectoryList; +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; @@ -33,7 +32,7 @@ public function __construct( } /** - * Check generated/code read and write access + * Check write permissions to generation folders * * @return bool */ @@ -44,33 +43,32 @@ 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() - ) { + + $generationDirs = [ + DirectoryList::GENERATED, + DirectoryList::GENERATED_CODE, + DirectoryList::GENERATED_METADATA + ]; + + foreach ($generationDirs as $generationDirectory) { + $directoryPath = $directoryList->getPath($generationDirectory); + $directoryWrite = $fileWriteFactory->create($directoryPath); + + if (!$directoryWrite->isExist()) { try { - $probeFilePath = $generationDirectoryPath . DIRECTORY_SEPARATOR . uniqid(mt_rand()).'tmp'; - $fileWriteFactory->create($probeFilePath, DriverPool::FILE, 'w'); - $driver->deleteFile($probeFilePath); + $directoryWrite->create(); } catch (\Exception $e) { return false; } - } else { - return false; } - } else { - try { - $directoryWrite->create(); - } catch (\Exception $e) { + + if (!$directoryWrite->isWritable()) { return false; } } + return true; } } 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() + ); + } +} diff --git a/lib/internal/Magento/Framework/Session/SaveHandler/Redis/Config.php b/lib/internal/Magento/Framework/Session/SaveHandler/Redis/Config.php index 449457830ef17..6b7ab238c30ca 100644 --- a/lib/internal/Magento/Framework/Session/SaveHandler/Redis/Config.php +++ b/lib/internal/Magento/Framework/Session/SaveHandler/Redis/Config.php @@ -48,12 +48,12 @@ class Config implements \Cm\RedisSession\Handler\ConfigInterface /** * Configuration path for persistent identifier */ - const PARAM_PERSISTENT_IDENTIFIER = 'session/redis/param_persistent_identifier'; + const PARAM_PERSISTENT_IDENTIFIER = 'session/redis/persistent_identifier'; /** * Configuration path for compression threshold */ - const PARAM_COMPRESSION_THRESHOLD = 'session/redis/param_compression_threshold'; + const PARAM_COMPRESSION_THRESHOLD = 'session/redis/compression_threshold'; /** * Configuration path for compression library diff --git a/setup/src/Magento/Setup/Console/CompilerPreparation.php b/setup/src/Magento/Setup/Console/CompilerPreparation.php index 6bdddfe0053a8..9ea938d51fb37 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,31 +60,31 @@ 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 = []; + + 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] : []; $directoryList = new DirectoryList(BP, $mageDirs); - $compileDirList[] = $directoryList->getPath(DirectoryList::GENERATED_CODE); - $compileDirList[] = $directoryList->getPath(DirectoryList::GENERATED_METADATA); - - if (!$this->getGenerationDirectoryAccess()->check()) { - throw new FileSystemException( - new Phrase('Generation directory can not be written.') - ); - } + $compileDirList = [ + $directoryList->getPath(DirectoryList::GENERATED_CODE), + $directoryList->getPath(DirectoryList::GENERATED_METADATA), + ]; foreach ($compileDirList as $compileDir) { if ($this->filesystemDriver->isExists($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. *