Merge pull request #356 from magento-thunder/MAGECLOUD-2729

MAGECLOUD-2729: Reading environment-specific .magento.env.yaml doesn't work on build phase

Author: Yevhen Miroshnychenko

src/Config/Environment.php

@@ -262,14 +262,4 @@ public function isMasterBranch(): bool
         return isset($_ENV['MAGENTO_CLOUD_ENVIRONMENT'])
             && preg_match(self::GIT_MASTER_BRANCH_RE, $_ENV['MAGENTO_CLOUD_ENVIRONMENT']);
     }
-
-    /**
-     * Returns branch name of the current environment.
-     *
-     * @return string
-     */
-    public function getBranchName(): string
-    {
-        return $_ENV['MAGENTO_CLOUD_BRANCH'] ?? '';
-    }
 }

src/Config/Environment/Reader.php

@@ -5,12 +5,10 @@
  */
 namespace Magento\MagentoCloud\Config\Environment;
 
-use Magento\MagentoCloud\Config\Environment;
 use Magento\MagentoCloud\Filesystem\ConfigFileList;
 use Magento\MagentoCloud\Filesystem\FileSystemException;
 use Magento\MagentoCloud\Filesystem\Reader\ReaderInterface;
 use Magento\MagentoCloud\Filesystem\Driver\File;
-use Magento\MagentoCloud\Filesystem\SystemList;
 use Symfony\Component\Yaml\Yaml;
 use Symfony\Component\Yaml\Exception\ParseException;
 
@@ -19,21 +17,6 @@
  */
 class Reader implements ReaderInterface
 {
-    /**
-     * @string
-     */
-    const DIR_ENV_CONFIG = '.magento.env';
-
-    /**
-     * @var SystemList
-     */
-    private $systemList;
-
-    /**
-     * @var Environment
-     */
-    private $environment;
-
     /**
      * @var ConfigFileList
      */
@@ -52,19 +35,11 @@ class Reader implements ReaderInterface
     private $config;
 
     /**
-     * @param SystemList $systemList
-     * @param Environment $environment
      * @param ConfigFileList $configFileList
      * @param File $file
      */
-    public function __construct(
-        SystemList $systemList,
-        Environment $environment,
-        ConfigFileList $configFileList,
-        File $file
-    ) {
-        $this->systemList = $systemList;
-        $this->environment = $environment;
+    public function __construct(ConfigFileList $configFileList, File $file)
+    {
         $this->configFileList = $configFileList;
         $this->file = $file;
     }
@@ -77,73 +52,12 @@ public function __construct(
     public function read(): array
     {
         if ($this->config === null) {
-            $mainConfigPath = $this->configFileList->getEnvConfig();
+            $path = $this->configFileList->getEnvConfig();
 
-            $branchConfigPath = sprintf(
-                '%s/%s/%s.yaml',
-                $this->systemList->getMagentoRoot(),
-                self::DIR_ENV_CONFIG,
-                $this->environment->getBranchName()
-            );
-
-            $this->config = $this->mergeConfigs(
-                $this->parseConfig($mainConfigPath),
-                $this->parseConfig($branchConfigPath)
-            );
+            $this->config = !$this->file->isExists($path) ?
+                [] : (array)Yaml::parse($this->file->fileGetContents($path));
         }
 
         return $this->config;
     }
-
-    /**
-     * Returns parsed yaml config from file.
-     * Returns an empty array if file doesn't exists
-     *
-     * @param string $path
-     * @return array
-     * @throws FileSystemException
-     */
-    private function parseConfig(string $path): array
-    {
-        return !$this->file->isExists($path) ?
-            [] : (array)Yaml::parse($this->file->fileGetContents($path));
-    }
-
-    /**
-     * Merges configuration from $branchConfig into $mainConfig by each stage or by each logger configuration.
-     *
-     * Separately merges each stage and logger configuration to avoid cases when the variable has an array value
-     * and it should be replaced instead of recursive merging.
-     *
-     * @param array $mainConfig
-     * @param array $branchConfig
-     * @return array
-     */
-    private function mergeConfigs(array $mainConfig, array $branchConfig): array
-    {
-        $newConfig = $mainConfig;
-
-        foreach ($branchConfig as $sectionName => $sectionConfig) {
-            if (!is_array($sectionConfig)) {
-                continue;
-            }
-
-            foreach ($sectionConfig as $stageName => $stageConfig) {
-                if (!is_array($stageConfig)) {
-                    continue;
-                }
-
-                if (isset($newConfig[$sectionName][$stageName])) {
-                    $newConfig[$sectionName][$stageName] = array_merge(
-                        $newConfig[$sectionName][$stageName],
-                        $stageConfig
-                    );
-                } else {
-                    $newConfig[$sectionName][$stageName] = $stageConfig;
-                }
-            }
-        }
-
-        return $newConfig;
-    }
 }

src/Test/Unit/Config/Environment/ReaderTest.php

@@ -5,11 +5,9 @@
  */
 namespace Magento\MagentoCloud\Test\Unit\Config\Environment;
 
-use Magento\MagentoCloud\Config\Environment;
 use Magento\MagentoCloud\Config\Environment\Reader;
 use Magento\MagentoCloud\Filesystem\ConfigFileList;
 use Magento\MagentoCloud\Filesystem\Driver\File;
-use Magento\MagentoCloud\Filesystem\SystemList;
 use phpmock\phpunit\PHPMock;
 use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
@@ -21,16 +19,6 @@ class ReaderTest extends TestCase
 {
     use PHPMock;
 
-    /**
-     * @var SystemList|MockObject
-     */
-    private $systemListMock;
-
-    /**
-     * @var Environment|MockObject
-     */
-    private $environmentMock;
-
     /**
      * @var ConfigFileList|MockObject
      */
@@ -57,14 +45,10 @@ protected function setUp()
         self::defineFunctionMock('Magento\MagentoCloud\Filesystem\Driver', 'file_get_contents');
         self::defineFunctionMock('Magento\MagentoCloud\Filesystem\Driver', 'file_exists');
 
-        $this->systemListMock = $this->createMock(SystemList::class);
-        $this->environmentMock = $this->createMock(Environment::class);
         $this->configFileListMock = $this->createMock(ConfigFileList::class);
         $this->fileMock = $this->createPartialMock(File::class, []);
 
         $this->reader = new Reader(
-            $this->systemListMock,
-            $this->environmentMock,
             $this->configFileListMock,
             $this->fileMock
         );
@@ -77,98 +61,18 @@ public function testRead()
         $this->configFileListMock->expects($this->once())
             ->method('getEnvConfig')
             ->willReturn($baseDir . '/.magento.env.yaml');
-        $this->systemListMock->expects($this->once())
-            ->method('getMagentoRoot')
-            ->willReturn($baseDir);
-        $this->environmentMock->expects($this->once())
-            ->method('getBranchName')
-            ->willReturn('test-branch');
 
         $this->reader->read();
         $this->assertEquals(
             [
                 'stage' => [
-                    'global' => ['SCD_ON_DEMAND' => false, 'UPDATE_URLS' => false],
-                    'deploy' => ['DATABASE_CONFIGURATION' => ['host' => 'localhost'], 'SCD_THREADS' => 3],
-                    'build' => ['SCD_THREADS' => 2],
-                ],
-                'log' => [
-                    'gelf' => [
-                        'min_level' => 'info',
-                        'use_default_formatter' => true,
-                        'additional' => ['project' => 'project'],
-                    ],
-                    'syslog' => ['ident' => 'ident-branch', 'facility' => 7],
-                ],
-            ],
-            $this->reader->read()
-        );
-    }
-
-    public function testReadBranchConfigNotExists()
-    {
-        $baseDir = __DIR__ . '/_file/';
-
-        $this->configFileListMock->expects($this->once())
-            ->method('getEnvConfig')
-            ->willReturn($baseDir . '/.magento.env.yaml');
-        $this->systemListMock->expects($this->once())
-            ->method('getMagentoRoot')
-            ->willReturn($baseDir);
-        $this->environmentMock->expects($this->once())
-            ->method('getBranchName')
-            ->willReturn('not-exist');
-
-        $this->assertEquals(
-            [
-                'stage' => [
-                    'global' => ['SCD_ON_DEMAND' => true, 'UPDATE_URLS' => false],
-                    'deploy' => [
-                        'DATABASE_CONFIGURATION' => [
-                            'host' => '127.0.0.1',
-                            'port' => '3306',
-                            'schema' => 'test_schema',
-                        ],
-                        'SCD_THREADS' => 5,
+                    'global' => [
+                        'SCD_ON_DEMAND' => true,
+                        'UPDATE_URLS' => false
                     ],
-                ],
-                'log' => [
-                    'gelf' => [
-                        'min_level' => 'info',
-                        'use_default_formatter' => true,
-                        'additional' => ['project' => 'project', 'app_id' => 'app'],
-                    ],
-                ],
-            ],
-            $this->reader->read()
-        );
-    }
-
-    public function testReadBranchConfigWithEmptySectionAndStage()
-    {
-        $baseDir = __DIR__ . '/_file/';
-
-        $this->configFileListMock->expects($this->once())
-            ->method('getEnvConfig')
-            ->willReturn($baseDir . '/.magento.env.yaml');
-        $this->systemListMock->expects($this->once())
-            ->method('getMagentoRoot')
-            ->willReturn($baseDir);
-        $this->environmentMock->expects($this->once())
-            ->method('getBranchName')
-            ->willReturn('test-branch-emty');
-
-        $this->assertEquals(
-            [
-                'stage' => [
-                    'global' => ['SCD_ON_DEMAND' => true, 'UPDATE_URLS' => false],
                     'deploy' => [
-                        'DATABASE_CONFIGURATION' => [
-                            'host' => '127.0.0.1',
-                            'port' => '3306',
-                            'schema' => 'test_schema',
-                        ],
-                        'SCD_THREADS' => 5,
+                        'DATABASE_CONFIGURATION' => ['host' => '127.0.0.1', 'port' => 3306, 'schema' => 'test_schema'],
+                        'SCD_THREADS' => 5
                     ],
                 ],
                 'log' => [
@@ -190,29 +94,16 @@ public function testReadMainConfigWithEmptySectionAndStage()
         $this->configFileListMock->expects($this->once())
             ->method('getEnvConfig')
             ->willReturn($baseDir . '/.magento-with-empty-sections.env.yaml');
-        $this->systemListMock->expects($this->once())
-            ->method('getMagentoRoot')
-            ->willReturn($baseDir);
-        $this->environmentMock->expects($this->once())
-            ->method('getBranchName')
-            ->willReturn('test-branch');
 
         $this->reader->read();
         $this->assertEquals(
             [
                 'stage' => [
-                    'global' => ['SCD_ON_DEMAND' => false],
-                    'deploy' => ['DATABASE_CONFIGURATION' => ['host' => 'localhost'], 'SCD_THREADS' => 3],
-                    'build' => ['SCD_THREADS' => 2],
-                ],
-                'log' => [
-                    'gelf' => [
-                        'min_level' => 'info',
-                        'use_default_formatter' => true,
-                        'additional' => ['project' => 'project'],
-                    ],
-                    'syslog' => ['ident' => 'ident-branch', 'facility' => 7],
+                    'global' => ['SCD_ON_DEMAND' => true],
+                    'deploy' => null,
+                    'build' => null,
                 ],
+                'log' => null,
             ],
             $this->reader->read()
         );

src/Test/Unit/Config/Environment/_file/.magento.env/test-branch-emty.yaml

@@ -152,16 +152,4 @@ public function isMasterBranchDataProvider(): array
             [true, 'production-lad13m'],
         ];
     }
-
-    public function testGetBranchName()
-    {
-        $_ENV['MAGENTO_CLOUD_BRANCH'] = 'test-branch';
-
-        $this->assertEquals('test-branch', $this->environment->getBranchName());
-    }
-
-    public function testGetBranchNameEmpty()
-    {
-        $this->assertEquals('', $this->environment->getBranchName());
-    }
 }