Fix setup upgrade command re enables caches

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

@@ -144,7 +144,7 @@ protected function execute(InputInterface $input, OutputInterface $output)
             $searchConfig->validateSearchEngine();
             $installer->removeUnusedTriggers();
             $installer->installSchema($request);
-            $installer->installDataFixtures($request);
+            $installer->installDataFixtures($request, true);
 
             if ($this->deploymentConfig->isAvailable()) {
                 $importConfigCommand = $this->getApplication()->find(ConfigImportCommand::COMMAND_NAME);

setup/src/Magento/Setup/Model/Installer.php

@@ -8,6 +8,7 @@
 
 use Magento\Backend\Setup\ConfigOptionsList as BackendConfigOptionsList;
 use Magento\Framework\App\Cache\Manager;
+use Magento\Framework\App\Cache\Manager as CacheManager;
 use Magento\Framework\App\Cache\Type\Block as BlockCache;
 use Magento\Framework\App\Cache\Type\Config as ConfigCache;
 use Magento\Framework\App\Cache\Type\Layout as LayoutCache;
@@ -920,18 +921,25 @@ private function convertationOfOldScriptsIsAllowed(array $request)
      * Installs data fixtures
      *
      * @param array $request
+     * @param boolean $keepCacheStatuses
      * @return void
      * @throws Exception
      * @throws \Magento\Framework\Setup\Exception
      */
-    public function installDataFixtures(array $request = [])
+    public function installDataFixtures(array $request = [], $keepCacheStatuses = false)
     {
         $frontendCaches = [
             PageCache::TYPE_IDENTIFIER,
             BlockCache::TYPE_IDENTIFIER,
             LayoutCache::TYPE_IDENTIFIER,
         ];
 
+        if ($keepCacheStatuses) {
+            $disabledCaches = $this->getDisabledCacheTypes($frontendCaches);
+
+            $frontendCaches = array_diff($frontendCaches, $disabledCaches);
+        }
+
         /** @var \Magento\Framework\Registry $registry */
         $registry = $this->objectManagerProvider->get()->get(\Magento\Framework\Registry::class);
         //For backward compatibility in install and upgrade scripts with enabled parallelization.
@@ -942,11 +950,20 @@ public function installDataFixtures(array $request = [])
         $setup = $this->dataSetupFactory->create();
         $this->checkFilePermissionsForDbUpgrade();
         $this->log->log('Data install/update:');
-        $this->log->log('Disabling caches:');
-        $this->updateCaches(false, $frontendCaches);
-        $this->handleDBSchemaData($setup, 'data', $request);
-        $this->log->log('Enabling caches:');
-        $this->updateCaches(true, $frontendCaches);
+
+        if ($frontendCaches) {
+            $this->log->log('Disabling caches:');
+            $this->updateCaches(false, $frontendCaches);
+        }
+
+        try {
+            $this->handleDBSchemaData($setup, 'data', $request);
+        } finally {
+            if ($frontendCaches) {
+                $this->log->log('Enabling caches:');
+                $this->updateCaches(true, $frontendCaches);
+            }
+        }
 
         $registry->unregister('setup-mode-enabled');
     }
@@ -995,7 +1012,7 @@ private function throwExceptionForNotWritablePaths(array $paths)
      */
     private function handleDBSchemaData($setup, $type, array $request)
     {
-        if (!($type === 'schema' || $type === 'data')) {
+        if ($type !== 'schema' && $type !== 'data') {
             // phpcs:ignore Magento2.Exceptions.DirectThrow
             throw  new Exception("Unsupported operation type $type is requested");
         }
@@ -1014,17 +1031,13 @@ private function handleDBSchemaData($setup, $type, array $request)
                 'objectManager' => $this->objectManagerProvider->get()
             ]
         );
+
+        $patchApplierParams = $type === 'schema' ?
+            ['schemaSetup' => $setup] :
+            ['moduleDataSetup' => $setup, 'objectManager' => $this->objectManagerProvider->get()];
+
         /** @var PatchApplier $patchApplier */
-        if ($type === 'schema') {
-            $patchApplier = $this->patchApplierFactory->create(['schemaSetup' => $setup]);
-        } elseif ($type === 'data') {
-            $patchApplier = $this->patchApplierFactory->create(
-                [
-                    'moduleDataSetup' => $setup,
-                    'objectManager' => $this->objectManagerProvider->get()
-                ]
-            );
-        }
+        $patchApplier = $this->patchApplierFactory->create($patchApplierParams);
 
         foreach ($moduleNames as $moduleName) {
             if ($this->isDryRun($request)) {
@@ -1086,11 +1099,11 @@ private function handleDBSchemaData($setup, $type, array $request)
 
         if ($type === 'schema') {
             $this->log->log('Schema post-updates:');
-            $handlerType = 'schema-recurring';
         } elseif ($type === 'data') {
             $this->log->log('Data post-updates:');
-            $handlerType = 'data-recurring';
         }
+        $handlerType = $type === 'schema' ? 'schema-recurring' : 'data-recurring';
+
         foreach ($moduleNames as $moduleName) {
             if ($this->isDryRun($request)) {
                 $this->log->log("Module '{$moduleName}':");
@@ -1726,4 +1739,27 @@ public function removeUnusedTriggers(): void
         $this->triggerCleaner->removeTriggers();
         $this->cleanCaches();
     }
+
+    /**
+     * Returns list of disabled cache types
+     *
+     * @param array $cacheTypesToCheck
+     * @return array
+     */
+    private function getDisabledCacheTypes(array $cacheTypesToCheck): array
+    {
+        $disabledCaches = [];
+
+        /** @var CacheManager $cacheManager */
+        $cacheManager = $this->objectManagerProvider->get()->create(CacheManager::class);
+        $cacheStatus = $cacheManager->getStatus();
+
+        foreach ($cacheTypesToCheck as $cacheType) {
+            if (isset($cacheStatus[$cacheType]) && $cacheStatus[$cacheType] === 0) {
+                $disabledCaches[] = $cacheType;
+            }
+        }
+
+        return $disabledCaches;
+    }
 }

setup/src/Magento/Setup/Test/Unit/Model/InstallerTest.php

@@ -62,7 +62,18 @@
     class InstallerTest extends TestCase
     {
         /**
-         * @var \Magento\Setup\Model\Installer
+         * @var array
+         */
+        private $request = [
+            ConfigOptionsListConstants::INPUT_KEY_DB_HOST => '127.0.0.1',
+            ConfigOptionsListConstants::INPUT_KEY_DB_NAME => 'magento',
+            ConfigOptionsListConstants::INPUT_KEY_DB_USER => 'magento',
+            ConfigOptionsListConstants::INPUT_KEY_ENCRYPTION_KEY => 'encryption_key',
+            ConfigOptionsList::INPUT_KEY_BACKEND_FRONTNAME => 'backend',
+        ];
+
+        /**
+         * @var Installer
          */
         private $object;
 
@@ -426,13 +437,7 @@ public function installDataProvider()
         {
             return [
                 [
-                    'request' => [
-                        ConfigOptionsListConstants::INPUT_KEY_DB_HOST => '127.0.0.1',
-                        ConfigOptionsListConstants::INPUT_KEY_DB_NAME => 'magento',
-                        ConfigOptionsListConstants::INPUT_KEY_DB_USER => 'magento',
-                        ConfigOptionsListConstants::INPUT_KEY_ENCRYPTION_KEY => 'encryption_key',
-                        ConfigOptionsList::INPUT_KEY_BACKEND_FRONTNAME => 'backend',
-                    ],
+                    'request' => $this->request,
                     'logMessages' => [
                         ['Starting Magento installation:'],
                         ['File permissions check...'],
@@ -526,6 +531,100 @@ public function installDataProvider()
             ];
         }
 
+        /**
+         * Test for InstallDataFixtures
+         *
+         * @dataProvider testInstallDataFixturesDataProvider
+         *
+         * @param bool $keepCache
+         * @param array $expectedToEnableCacheTypes
+         * @return void
+         */
+        public function testInstallDataFixtures(bool $keepCache, array $expectedToEnableCacheTypes): void
+        {
+            $cacheManagerMock = $this->createMock(Manager::class);
+            //simulate disabled layout cache type
+            $cacheManagerMock->expects($this->atLeastOnce())
+                ->method('getStatus')
+                ->willReturn(['layout' => 0]);
+            $cacheManagerMock->expects($this->atLeastOnce())
+                ->method('getAvailableTypes')
+                ->willReturn(['block_html', 'full_page', 'layout' , 'config', 'collections']);
+            $cacheManagerMock->expects($this->exactly(2))
+                ->method('setEnabled')
+                ->withConsecutive([$expectedToEnableCacheTypes, false], [$expectedToEnableCacheTypes, true])
+                ->willReturn([]);
+
+            $this->objectManager->expects($this->atLeastOnce())
+                ->method('create')
+                ->willReturnMap([
+                    [Manager::class, [], $cacheManagerMock],
+                    [
+                        PatchApplierFactory::class,
+                        ['objectManager' => $this->objectManager],
+                        $this->patchApplierFactoryMock
+                    ],
+                ]);
+
+            $registryMock = $this->createMock(Registry::class);
+            $this->objectManager->expects($this->atLeastOnce())
+                ->method('get')
+                ->with(Registry::class)
+                ->willReturn($registryMock);
+
+            $this->config->expects($this->atLeastOnce())
+                ->method('get')
+                ->willReturn(true);
+
+            $this->filePermissions->expects($this->atLeastOnce())
+                ->method('getMissingWritableDirectoriesForDbUpgrade')
+                ->willReturn([]);
+
+            $connection = $this->getMockBuilder(AdapterInterface::class)
+                ->addMethods(['getSchemaListener'])
+                ->getMockForAbstractClass();
+            $connection->expects($this->once())
+                ->method('getSchemaListener')
+                ->willReturn($this->schemaListenerMock);
+
+            $resource = $this->createMock(ResourceConnection::class);
+            $resource->expects($this->atLeastOnce())
+                ->method('getConnection')
+                ->willReturn($connection);
+            $this->contextMock->expects($this->once())
+                ->method('getResources')
+                ->willReturn($resource);
+
+            $dataSetup = $this->createMock(DataSetup::class);
+            $dataSetup->expects($this->once())
+                ->method('getConnection')
+                ->willReturn($connection);
+
+            $this->dataSetupFactory->expects($this->atLeastOnce())
+                ->method('create')
+                ->willReturn($dataSetup);
+
+            $this->object->installDataFixtures($this->request, $keepCache);
+        }
+
+        /**
+         * DataProvider for testInstallDataFixtures
+         *
+         * @return array
+         */
+        public function testInstallDataFixturesDataProvider(): array
+        {
+            return [
+                'keep cache' => [
+                    true, ['block_html', 'full_page']
+                ],
+                'do not keep a cache' => [
+                    false,
+                    ['block_html', 'full_page', 'layout']
+                ],
+            ];
+        }
+
         public function testCheckInstallationFilePermissions()
         {
             $this->filePermissions