fix issue with null config value

engcom-Charlie · engcom-Charlie · commit a6049d55f20f · 2020-06-09T16:38:53.000+03:00
diff --git a/app/code/Magento/Config/Console/Command/ConfigShowCommand.php b/app/code/Magento/Config/Console/Command/ConfigShowCommand.php
@@ -16,6 +16,8 @@
 use Symfony\Component\Console\Input\InputInterface;
 use Symfony\Component\Console\Input\InputOption;
 use Symfony\Component\Console\Output\OutputInterface;
+use Magento\Framework\App\ObjectManager;
+use Magento\Config\Model\Config\PathValidatorFactory;
 
 /**
  * Command provides possibility to show saved system configuration.
@@ -78,24 +80,42 @@ class ConfigShowCommand extends Command
      */
     private $inputPath;
 
+    /**
+     * @var PathValidatorFactory
+     */
+    private $pathValidatorFactory;
+
+    /**
+     * @var EmulatedAdminhtmlAreaProcessor
+     */
+    private $emulatedAreaProcessor;
+
     /**
      * @param ValidatorInterface $scopeValidator
      * @param ConfigSourceInterface $configSource
      * @param ConfigPathResolver $pathResolver
      * @param ValueProcessor $valueProcessor
+     * @param PathValidatorFactory|null $pathValidatorFactory
+     * @param EmulatedAdminhtmlAreaProcessor|null $emulatedAreaProcessor
      * @internal param ScopeConfigInterface $appConfig
      */
     public function __construct(
         ValidatorInterface $scopeValidator,
         ConfigSourceInterface $configSource,
         ConfigPathResolver $pathResolver,
-        ValueProcessor $valueProcessor
+        ValueProcessor $valueProcessor,
+        ?PathValidatorFactory $pathValidatorFactory = null,
+        ?EmulatedAdminhtmlAreaProcessor $emulatedAreaProcessor = null
     ) {
         parent::__construct();
         $this->scopeValidator = $scopeValidator;
         $this->configSource = $configSource;
         $this->pathResolver = $pathResolver;
         $this->valueProcessor = $valueProcessor;
+        $this->pathValidatorFactory = $pathValidatorFactory
+            ?: ObjectManager::getInstance()->get(PathValidatorFactory::class);
+        $this->emulatedAreaProcessor = $emulatedAreaProcessor
+            ?: ObjectManager::getInstance()->get(EmulatedAdminhtmlAreaProcessor::class);
     }
 
     /**
@@ -146,17 +166,17 @@ protected function execute(InputInterface $input, OutputInterface $output)
             $this->scopeCode = $input->getOption(self::INPUT_OPTION_SCOPE_CODE);
             $this->inputPath = trim($input->getArgument(self::INPUT_ARGUMENT_PATH), '/');
 
-            $this->scopeValidator->isValid($this->scope, $this->scopeCode);
-            $configPath = $this->pathResolver->resolve($this->inputPath, $this->scope, $this->scopeCode);
-            $configValue = $this->configSource->get($configPath);
+            $configValue = $this->emulatedAreaProcessor->process(function () {
+                $this->scopeValidator->isValid($this->scope, $this->scopeCode);
+                if ($this->inputPath) {
+                    $pathValidator = $this->pathValidatorFactory->create();
+                    $pathValidator->validate($this->inputPath);
+                }
 
-            if ($configValue === null) {
-                $output->writeln(sprintf(
-                    '<error>%s</error>',
-                    __('Configuration for path: "%1" doesn\'t exist', $this->inputPath)->render()
-                ));
-                return Cli::RETURN_FAILURE;
-            }
+                $configPath = $this->pathResolver->resolve($this->inputPath, $this->scope, $this->scopeCode);
+
+                return $this->configSource->get($configPath);
+            });
 
             $this->outputResult($output, $configValue, $this->inputPath);
             return Cli::RETURN_SUCCESS;
diff --git a/app/code/Magento/Config/Test/Unit/App/Config/Source/RuntimeConfigSourceTest.php b/app/code/Magento/Config/Test/Unit/App/Config/Source/RuntimeConfigSourceTest.php
@@ -262,6 +262,7 @@ public function configDataProvider(): array
         return [
             'config value 0' => ['default/test/option', ['test' => ['option' => 0]], '0'],
             'config value blank' => ['default/test/option', ['test' => ['option' => '']], ''],
+            'config value null' => ['default/test/option', ['test' => ['option' => null]], ''],
         ];
     }
 }
diff --git a/app/code/Magento/Config/Test/Unit/Console/Command/ConfigShowCommandTest.php b/app/code/Magento/Config/Test/Unit/Console/Command/ConfigShowCommandTest.php
@@ -3,23 +3,41 @@
  * Copyright © Magento, Inc. All rights reserved.
  * See COPYING.txt for license details.
  */
+
 declare(strict_types=1);
 
 namespace Magento\Config\Test\Unit\Console\Command;
 
 use Magento\Config\Console\Command\ConfigShow\ValueProcessor;
 use Magento\Config\Console\Command\ConfigShowCommand;
+use Magento\Config\Console\Command\EmulatedAdminhtmlAreaProcessor;
 use Magento\Framework\App\Config\ConfigPathResolver;
 use Magento\Framework\App\Config\ConfigSourceInterface;
 use Magento\Framework\App\Scope\ValidatorInterface;
 use Magento\Framework\Console\Cli;
 use Magento\Framework\Exception\LocalizedException;
+use Magento\Config\Model\Config\PathValidatorFactory;
+use Magento\Config\Model\Config\PathValidator;
+use Magento\Framework\TestFramework\Unit\Helper\ObjectManager;
 use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
 use Symfony\Component\Console\Tester\CommandTester;
 
+/**
+ * Test for \Magento\Config\Console\Command\ConfigShowCommand.
+ * @SuppressWarnings(PHPMD.CouplingBetweenObjects)
+ */
 class ConfigShowCommandTest extends TestCase
 {
+    private const CONFIG_PATH = 'some/config/path';
+    private const SCOPE = 'some/config/path';
+    private const SCOPE_CODE = 'someScopeCode';
+
+    /**
+     * @var ConfigShowCommand
+     */
+    private $model;
+
     /**
      * @var ValidatorInterface|MockObject
      */
@@ -41,12 +59,22 @@ class ConfigShowCommandTest extends TestCase
     private $pathResolverMock;
 
     /**
-     * @var ConfigShowCommand
+     * @var EmulatedAdminhtmlAreaProcessor|MockObject
+     */
+    private $emulatedAreProcessorMock;
+
+    /**
+     * @var PathValidator|MockObject
      */
-    private $command;
+    private $pathValidatorMock;
 
+    /**
+     * @inheritdoc
+     */
     protected function setUp(): void
     {
+        $objectManager = new ObjectManager($this);
+
         $this->valueProcessorMock = $this->getMockBuilder(ValueProcessor::class)
             ->disableOriginalConstructor()
             ->getMock();
@@ -57,40 +85,69 @@ protected function setUp(): void
             ->getMockForAbstractClass();
         $this->configSourceMock = $this->getMockBuilder(ConfigSourceInterface::class)
             ->getMockForAbstractClass();
+        $this->pathValidatorMock = $this->getMockBuilder(PathValidator::class)
+            ->disableOriginalConstructor()
+            ->getMock();
+        $pathValidatorFactoryMock = $this->getMockBuilder(PathValidatorFactory::class)
+            ->disableOriginalConstructor()
+            ->getMock();
+        $pathValidatorFactoryMock->expects($this->atMost(1))
+            ->method('create')
+            ->willReturn($this->pathValidatorMock);
 
-        $this->command = new ConfigShowCommand(
-            $this->scopeValidatorMock,
-            $this->configSourceMock,
-            $this->pathResolverMock,
-            $this->valueProcessorMock
+        $this->emulatedAreProcessorMock = $this->getMockBuilder(EmulatedAdminhtmlAreaProcessor::class)
+            ->disableOriginalConstructor()
+            ->getMock();
+
+        $this->model = $objectManager->getObject(
+            ConfigShowCommand::class,
+            [
+                'scopeValidator' => $this->scopeValidatorMock,
+                'configSource' => $this->configSourceMock,
+                'pathResolver' => $this->pathResolverMock,
+                'valueProcessor' => $this->valueProcessorMock,
+                'pathValidatorFactory' => $pathValidatorFactoryMock,
+                'emulatedAreaProcessor' => $this->emulatedAreProcessorMock,
+            ]
         );
     }
 
-    public function testExecute()
+    /**
+     * Test get config value
+     *
+     * @return void
+     */
+    public function testExecute(): void
     {
-        $configPath = 'some/config/path';
         $resolvedConfigPath = 'someScope/someScopeCode/some/config/path';
-        $scope = 'someScope';
-        $scopeCode = 'someScopeCode';
 
         $this->scopeValidatorMock->expects($this->once())
             ->method('isValid')
-            ->with($scope, $scopeCode)
+            ->with(self::SCOPE, self::SCOPE_CODE)
             ->willReturn(true);
         $this->pathResolverMock->expects($this->once())
             ->method('resolve')
-            ->with($configPath, $scope, $scopeCode)
+            ->with(self::CONFIG_PATH, self::SCOPE, self::SCOPE_CODE)
             ->willReturn($resolvedConfigPath);
         $this->configSourceMock->expects($this->once())
             ->method('get')
             ->with($resolvedConfigPath)
             ->willReturn('someValue');
         $this->valueProcessorMock->expects($this->once())
             ->method('process')
-            ->with($scope, $scopeCode, 'someValue', $configPath)
+            ->with(self::SCOPE, self::SCOPE_CODE, 'someValue', self::CONFIG_PATH)
             ->willReturn('someProcessedValue');
-
-        $tester = $this->getConfigShowCommandTester($configPath, $scope, $scopeCode);
+        $this->emulatedAreProcessorMock->expects($this->once())
+            ->method('process')
+            ->willReturnCallback(function ($function) {
+                return $function();
+            });
+
+        $tester = $this->getConfigShowCommandTester(
+            self::CONFIG_PATH,
+            self::SCOPE,
+            self::SCOPE_CODE
+        );
 
         $this->assertEquals(
             Cli::RETURN_SUCCESS,
@@ -102,18 +159,28 @@ public function testExecute()
         );
     }
 
-    public function testNotValidScopeOrScopeCode()
+    /**
+     * Test not valid scope or scope code
+     *
+     * @return void
+     */
+    public function testNotValidScopeOrScopeCode(): void
     {
-        $configPath = 'some/config/path';
-        $scope = 'someScope';
-        $scopeCode = 'someScopeCode';
-
         $this->scopeValidatorMock->expects($this->once())
             ->method('isValid')
-            ->with($scope, $scopeCode)
+            ->with(self::SCOPE, self::SCOPE_CODE)
             ->willThrowException(new LocalizedException(__('error message')));
-
-        $tester = $this->getConfigShowCommandTester($configPath, $scope, $scopeCode);
+        $this->emulatedAreProcessorMock->expects($this->once())
+            ->method('process')
+            ->willReturnCallback(function ($function) {
+                return $function();
+            });
+
+        $tester = $this->getConfigShowCommandTester(
+            self::CONFIG_PATH,
+            self::SCOPE,
+            self::SCOPE_CODE
+        );
 
         $this->assertEquals(
             Cli::RETURN_FAILURE,
@@ -125,17 +192,35 @@ public function testNotValidScopeOrScopeCode()
         );
     }
 
-    public function testConfigPathNotExist()
+    /**
+     * Test get config value for not existed path.
+     *
+     * @return void
+     */
+    public function testConfigPathNotExist(): void
     {
-        $configPath = 'some/path';
-        $tester = $this->getConfigShowCommandTester($configPath);
+        $exception = new LocalizedException(
+            __('The  "%1" path doesn\'t exist. Verify and try again.', self::CONFIG_PATH)
+        );
+
+        $this->pathValidatorMock->expects($this->once())
+            ->method('validate')
+            ->with(self::CONFIG_PATH)
+            ->willThrowException($exception);
+        $this->emulatedAreProcessorMock->expects($this->once())
+            ->method('process')
+            ->willReturnCallback(function ($function) {
+                return $function();
+            });
+
+        $tester = $this->getConfigShowCommandTester(self::CONFIG_PATH);
 
         $this->assertEquals(
             Cli::RETURN_FAILURE,
             $tester->getStatusCode()
         );
         $this->assertStringContainsString(
-            __('Configuration for path: "%1" doesn\'t exist', $configPath)->render(),
+            __('The  "%1" path doesn\'t exist. Verify and try again.', self::CONFIG_PATH)->render(),
             $tester->getDisplay()
         );
     }
@@ -159,7 +244,7 @@ private function getConfigShowCommandTester($configPath, $scope = null, $scopeCo
             $arguments['--' . ConfigShowCommand::INPUT_OPTION_SCOPE_CODE] = $scopeCode;
         }
 
-        $tester = new CommandTester($this->command);
+        $tester = new CommandTester($this->model);
         $tester->execute($arguments);
 
         return $tester;

Original file line number	Diff line number	Diff line change
`@@ -262,6 +262,7 @@ public function configDataProvider(): array`
`262`	`262`	`return [`
`263`	`263`	`'config value 0' => ['default/test/option', ['test' => ['option' => 0]], '0'],`
`264`	`264`	`'config value blank' => ['default/test/option', ['test' => ['option' => '']], ''],`
	`265`	`+ 'config value null' => ['default/test/option', ['test' => ['option' => null]], ''],`
`265`	`266`	`];`
`266`	`267`	`}`
`267`	`268`	`}`