Merge pull request #995 from magento-falcons/MAGETWO-66853

Fixed issues:
- MAGETWO-66736 Asymmetric transaction rollback error during stores importing
- MAGETWO-66672 Impossible generate fixture if store_groups>1
- MAGETWO-61431 [GitHub] CLI issue with bin/magento and exit codes for failure #7576

app/code/Magento/Store/Model/Config/Importer.php

@@ -7,12 +7,12 @@
 
 use Magento\Framework\App\CacheInterface;
 use Magento\Framework\App\DeploymentConfig\ImporterInterface;
-use Magento\Framework\App\ResourceConnection;
 use Magento\Framework\Exception\State\InvalidTransitionException;
 use Magento\Store\Model\Config\Importer\DataDifferenceCalculator;
 use Magento\Store\Model\Config\Importer\Processor\ProcessorFactory;
 use Magento\Store\Model\ScopeInterface;
 use Magento\Store\Model\StoreManagerInterface;
+use Magento\Store\Model\ResourceModel\Website;
 
 /**
  * Imports stores, websites and groups from transmitted data.
@@ -43,7 +43,7 @@ class Importer implements ImporterInterface
     /**
      * The resource of transaction.
      *
-     * @var ResourceConnection
+     * @var Website
      */
     private $resource;
 
@@ -59,14 +59,14 @@ class Importer implements ImporterInterface
      * @param ProcessorFactory $processFactory The factory for processes
      * @param StoreManagerInterface $storeManager The manager for operations with store
      * @param CacheInterface $cacheManager The application cache manager
-     * @param ResourceConnection $resource The resource of transaction
+     * @param Website $resource The resource of transaction
      */
     public function __construct(
         DataDifferenceCalculator $dataDifferenceCalculator,
         ProcessorFactory $processFactory,
         StoreManagerInterface $storeManager,
         CacheInterface $cacheManager,
-        ResourceConnection $resource
+        Website $resource
     ) {
         $this->dataDifferenceCalculator = $dataDifferenceCalculator;
         $this->processFactory = $processFactory;
@@ -100,25 +100,35 @@ public function import(array $data)
                 );
             }
 
-            $this->resource->getConnection()->beginTransaction();
+            $this->resource->beginTransaction();
 
             foreach ($actions as $action) {
                 $this->processFactory->create($action)->run($data);
             }
-
-            $this->resource->getConnection()->commit();
         } catch (\Exception $exception) {
-            $this->resource->getConnection()->rollBack();
+            $this->resource->rollBack();
+            $this->reinitStores();
 
             throw new InvalidTransitionException(__('%1', $exception->getMessage()), $exception);
-        } finally {
-            $this->storeManager->reinitStores();
-            $this->cacheManager->clean();
         }
 
+        $this->resource->commit();
+        $this->reinitStores();
+
         return $messages;
     }
 
+    /**
+     * Reinitialize store list.
+     *
+     * @return void
+     */
+    private function reinitStores()
+    {
+        $this->storeManager->reinitStores();
+        $this->cacheManager->clean();
+    }
+
     /**
      * Retrieves message reminder about root category assigning.
      *

app/code/Magento/Store/Test/Unit/Model/Config/ImporterTest.php

@@ -6,9 +6,8 @@
 namespace Magento\Store\Test\Unit\Model\Config;
 
 use Magento\Framework\App\CacheInterface;
-use Magento\Framework\App\ResourceConnection;
-use Magento\Framework\DB\Adapter\AdapterInterface;
 use Magento\Store\Model\Config\Importer;
+use Magento\Store\Model\ResourceModel\Website;
 use Magento\Store\Model\ScopeInterface;
 use Magento\Store\Model\StoreManager;
 use PHPUnit_Framework_MockObject_MockObject as Mock;
@@ -52,15 +51,10 @@ class ImporterTest extends \PHPUnit_Framework_TestCase
     private $cacheManagerMock;
 
     /**
-     * @var ResourceConnection|Mock
+     * @var Website|Mock
      */
     private $resourceMock;
 
-    /**
-     * @var AdapterInterface|Mock
-     */
-    private $connectionMock;
-
     /**
      * @inheritdoc
      */
@@ -79,15 +73,9 @@ protected function setUp()
             ->getMock();
         $this->cacheManagerMock = $this->getMockBuilder(CacheInterface::class)
             ->getMockForAbstractClass();
-        $this->resourceMock = $this->getMockBuilder(ResourceConnection::class)
+        $this->resourceMock = $this->getMockBuilder(Website::class)
             ->disableOriginalConstructor()
             ->getMock();
-        $this->connectionMock = $this->getMockBuilder(AdapterInterface::class)
-            ->getMockForAbstractClass();
-
-        $this->resourceMock->expects($this->any())
-            ->method('getConnection')
-            ->willReturn($this->connectionMock);
         $this->processorFactoryMock->expects($this->any())
             ->method('create')
             ->willReturn($this->processorMock);
@@ -109,12 +97,12 @@ public function testImport()
             ScopeInterface::SCOPE_WEBSITES => ['websites'],
         ];
 
-        $this->connectionMock->expects($this->once())
+        $this->resourceMock->expects($this->once())
             ->method('beginTransaction');
         $this->processorMock->expects($this->exactly(3))
             ->method('run')
             ->with($data);
-        $this->connectionMock->expects($this->once())
+        $this->resourceMock->expects($this->once())
             ->method('commit');
         $this->storeManagerMock->expects($this->once())
             ->method('reinitStores');
@@ -145,12 +133,12 @@ public function testImport()
      */
     public function testImportWithException()
     {
-        $this->connectionMock->expects($this->once())
+        $this->resourceMock->expects($this->once())
             ->method('beginTransaction');
         $this->processorMock->expects($this->any())
             ->method('run')
             ->willThrowException(new \Exception('Some error'));
-        $this->connectionMock->expects($this->never())
+        $this->resourceMock->expects($this->never())
             ->method('commit');
         $this->storeManagerMock->expects($this->once())
             ->method('reinitStores');

setup/src/Magento/Setup/Console/Command/DbStatusCommand.php

@@ -7,6 +7,7 @@
 
 use Composer\Package\Version\VersionParser;
 use Magento\Framework\App\DeploymentConfig;
+use Magento\Framework\Console\Cli;
 use Magento\Framework\Module\DbVersionInfo;
 use Magento\Setup\Model\ObjectManagerProvider;
 use Symfony\Component\Console\Input\InputInterface;
@@ -17,6 +18,11 @@
  */
 class DbStatusCommand extends AbstractSetupCommand
 {
+    /**
+     * Code for error when application upgrade is required.
+     */
+    const EXIT_CODE_UPGRADE_REQUIRED = 2;
+
     /**
      * Object manager provider
      *
@@ -60,8 +66,10 @@ protected function configure()
     protected function execute(InputInterface $input, OutputInterface $output)
     {
         if (!$this->deploymentConfig->isAvailable()) {
-            $output->writeln("<info>No information is available: the Magento application is not installed.</info>");
-            return;
+            $output->writeln(
+                "<info>No information is available: the Magento application is not installed.</info>"
+            );
+            return Cli::RETURN_FAILURE;
         }
         /** @var DbVersionInfo $dbVersionInfo */
         $dbVersionInfo = $this->objectManagerProvider->get()
@@ -93,13 +101,18 @@ protected function execute(InputInterface $input, OutputInterface $output)
                     '<info>Some modules use code versions newer or older than the database. ' .
                     "First update the module code, then run 'setup:upgrade'.</info>"
                 );
-                // we must have an exit code higher than zero to indicate something was wrong
-                return \Magento\Framework\Console\Cli::RETURN_FAILURE;
-            } else {
-                $output->writeln("<info>Run 'setup:upgrade' to update your DB schema and data.</info>");
+                return Cli::RETURN_FAILURE;
             }
-        } else {
-            $output->writeln('<info>All modules are up to date.</info>');
+
+            $output->writeln(
+                "<info>Run 'setup:upgrade' to update your DB schema and data.</info>"
+            );
+            return static::EXIT_CODE_UPGRADE_REQUIRED;
         }
+
+        $output->writeln(
+            '<info>All modules are up to date.</info>'
+        );
+        return Cli::RETURN_SUCCESS;
     }
 }

setup/src/Magento/Setup/Fixtures/StoresFixture.php

@@ -254,13 +254,15 @@ private function generateStoreGroups()
         while ($existedStoreGroupCount < $this->storeGroupsCount) {
             $websiteId = $this->websiteIds[$existedStoreGroupCount % $existedWebsitesCount];
             $storeGroupName = sprintf('Store Group %d - website_id_%d', ++$existedStoreGroupCount, $websiteId);
+            $storeGroupCode = sprintf('store_group_%d', $existedStoreGroupCount);
 
             $storeGroup = clone $this->defaultStoreGroup;
             $storeGroup->addData(
                 [
                     'group_id' => null,
                     'website_id' => $websiteId,
                     'name' => $storeGroupName,
+                    'code' => $storeGroupCode,
                     'root_category_id' => $this->getStoreCategoryId($storeGroupName),
                 ]
             );

setup/src/Magento/Setup/Test/Unit/Console/Command/DbStatusCommandTest.php

@@ -5,19 +5,27 @@
  */
 namespace Magento\Setup\Test\Unit\Console\Command;
 
+use Magento\Framework\Console\Cli;
 use Magento\Framework\Module\DbVersionInfo;
 use Magento\Setup\Console\Command\DbStatusCommand;
+use Magento\Framework\App\DeploymentConfig;
+use Magento\Setup\Model\ObjectManagerProvider;
+use Magento\Framework\ObjectManagerInterface;
+use PHPUnit_Framework_MockObject_MockObject as Mock;
 use Symfony\Component\Console\Tester\CommandTester;
 
+/**
+ * @inheritdoc
+ */
 class DbStatusCommandTest extends \PHPUnit_Framework_TestCase
 {
     /**
-     * @var \Magento\Framework\Module\DbVersionInfo|\PHPUnit_Framework_MockObject_MockObject
+     * @var DbVersionInfo|Mock
      */
     private $dbVersionInfo;
 
     /**
-     * @var \Magento\Framework\App\DeploymentConfig|\PHPUnit_Framework_MockObject_MockObject
+     * @var DeploymentConfig|Mock
      */
     private $deploymentConfig;
 
@@ -26,48 +34,66 @@ class DbStatusCommandTest extends \PHPUnit_Framework_TestCase
      */
     private $command;
 
+    /**
+     * @inheritdoc
+     */
     protected function setUp()
     {
-        $this->dbVersionInfo = $this->getMock(\Magento\Framework\Module\DbVersionInfo::class, [], [], '', false);
-        $objectManagerProvider = $this->getMock(\Magento\Setup\Model\ObjectManagerProvider::class, [], [], '', false);
-        $objectManager = $this->getMockForAbstractClass(\Magento\Framework\ObjectManagerInterface::class);
+        $this->dbVersionInfo = $this->getMockBuilder(DbVersionInfo::class)
+            ->disableOriginalConstructor()
+            ->getMock();
+        /** @var ObjectManagerProvider|Mock $objectManagerProvider */
+        $objectManagerProvider = $this->getMockBuilder(ObjectManagerProvider::class)
+            ->disableOriginalConstructor()
+            ->getMock();
+        /** @var ObjectManagerInterface|Mock $objectManager */
+        $objectManager = $this->getMockForAbstractClass(ObjectManagerInterface::class);
+        $this->deploymentConfig = $this->getMockBuilder(DeploymentConfig::class)
+            ->disableOriginalConstructor()
+            ->getMock();
+
         $objectManagerProvider->expects($this->any())
             ->method('get')
             ->will($this->returnValue($objectManager));
         $objectManager->expects($this->any())
             ->method('get')
             ->will($this->returnValue($this->dbVersionInfo));
-        $this->deploymentConfig = $this->getMock(\Magento\Framework\App\DeploymentConfig::class, [], [], '', false);
+
         $this->command = new DbStatusCommand($objectManagerProvider, $this->deploymentConfig);
     }
 
     /**
      * @param array $outdatedInfo
      * @param string $expectedMessage
+     * @param int $expectedCode
      *
      * @dataProvider executeDataProvider
      */
-    public function testExecute(array $outdatedInfo, $expectedMessage)
+    public function testExecute(array $outdatedInfo, $expectedMessage, $expectedCode)
     {
         $this->deploymentConfig->expects($this->once())
             ->method('isAvailable')
             ->will($this->returnValue(true));
         $this->dbVersionInfo->expects($this->once())
             ->method('getDbVersionErrors')
             ->will($this->returnValue($outdatedInfo));
+
         $tester = new CommandTester($this->command);
         $tester->execute([]);
+
         $this->assertStringMatchesFormat($expectedMessage, $tester->getDisplay());
+        $this->assertSame($expectedCode, $tester->getStatusCode());
     }
 
     public function executeDataProvider()
     {
         return [
             'DB is up to date' => [
                 [],
-                'All modules are up to date%a'
+                'All modules are up to date%a',
+                Cli::RETURN_SUCCESS
             ],
-            'DB is outdated'   => [
+            'DB is outdated' => [
                 [
                     [
                         DbVersionInfo::KEY_MODULE => 'module_a',
@@ -78,6 +104,7 @@ public function executeDataProvider()
                 ],
                 '%amodule_a%aschema%a1%a->%a2'
                 . "%aRun 'setup:upgrade' to update your DB schema and data%a",
+                DbStatusCommand::EXIT_CODE_UPGRADE_REQUIRED,
             ],
             'code is outdated' => [
                 [
@@ -90,6 +117,7 @@ public function executeDataProvider()
                 ],
                 '%amodule_a%adata%a2.0.0%a->%a1.0.0'
                 . '%aSome modules use code versions newer or older than the database%a',
+                Cli::RETURN_FAILURE,
             ],
             'both DB and code is outdated' => [
                 [
@@ -109,6 +137,7 @@ public function executeDataProvider()
                 '%amodule_a%aschema%a1.0.0%a->%a2.0.0'
                 . '%amodule_b%adata%a2.0.0%a->%a1.0.0'
                 . '%aSome modules use code versions newer or older than the database%a',
+                Cli::RETURN_FAILURE,
             ],
         ];
     }
@@ -120,11 +149,14 @@ public function testExecuteNotInstalled()
             ->will($this->returnValue(false));
         $this->dbVersionInfo->expects($this->never())
             ->method('getDbVersionErrors');
+
         $tester = new CommandTester($this->command);
         $tester->execute([]);
+
         $this->assertStringMatchesFormat(
             'No information is available: the Magento application is not installed.%w',
             $tester->getDisplay()
         );
+        $this->assertSame(Cli::RETURN_FAILURE, $tester->getStatusCode());
     }
 }