[BUGFIX] Streamline site configuration import in distribution packages 64/61864/5
authorOliver Hader <oliver@typo3.org>
Mon, 30 Sep 2019 12:19:19 +0000 (14:19 +0200)
committerOliver Hader <oliver.hader@typo3.org>
Mon, 30 Sep 2019 18:50:06 +0000 (20:50 +0200)
Site configuration shipped in distribution packages had a couple of flaws
during import process which are tackled with this change:

* existing site configurations for same identifier are not overridden on
  the file system level anymore (according warning is logged)
* site configuration is now updated and mapped to for imported pages,
  which did not work before due to hard-coded rootPageId in config.yaml
  (warning is logged in case root page id cannot be mapped)

Resolves: #89314
Releases: master
Change-Id: I856024afb50186eb9f6cc73ef13f1961c948c784
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/61864
Reviewed-by: Andreas Fernandez <a.fernandez@scripting-base.de>
Reviewed-by: Oliver Hader <oliver.hader@typo3.org>
Tested-by: TYPO3com <noreply@typo3.com>
Tested-by: Andreas Fernandez <a.fernandez@scripting-base.de>
Tested-by: Oliver Hader <oliver.hader@typo3.org>
typo3/sysext/core/Classes/Configuration/SiteConfiguration.php
typo3/sysext/extensionmanager/Classes/Utility/InstallUtility.php
typo3/sysext/extensionmanager/Tests/Unit/Utility/InstallUtilityTest.php
typo3/sysext/impexp/Classes/Utility/ImportExportUtility.php

index 9baac8c..26d16a7 100644 (file)
@@ -119,12 +119,13 @@ class SiteConfiguration implements SingletonInterface
     /**
      * Resolve all site objects which have been found in the filesystem.
      *
+     * @param bool $useCache
      * @return Site[]
      */
-    public function resolveAllExistingSites(): array
+    public function resolveAllExistingSites(bool $useCache = true): array
     {
         $sites = [];
-        $siteConfiguration = $this->getAllSiteConfigurationFromFiles();
+        $siteConfiguration = $this->getAllSiteConfigurationFromFiles($useCache);
         foreach ($siteConfiguration as $identifier => $configuration) {
             $rootPageId = (int)($configuration['rootPageId'] ?? 0);
             if ($rootPageId > 0) {
@@ -138,13 +139,14 @@ class SiteConfiguration implements SingletonInterface
     /**
      * Read the site configuration from config files.
      *
+     * @param bool $useCache
      * @return array
      * @throws \TYPO3\CMS\Core\Cache\Exception\NoSuchCacheException
      */
-    protected function getAllSiteConfigurationFromFiles(): array
+    protected function getAllSiteConfigurationFromFiles(bool $useCache = true): array
     {
         // Check if the data is already cached
-        if ($siteConfiguration = $this->getCache()->get($this->cacheIdentifier)) {
+        if ($useCache && $siteConfiguration = $this->getCache()->get($this->cacheIdentifier)) {
             // Due to the nature of PhpFrontend, the `<?php` and `#` wraps have to be removed
             $siteConfiguration = preg_replace('/^<\?php\s*|\s*#$/', '', $siteConfiguration);
             $siteConfiguration = json_decode($siteConfiguration, true);
index ec33de8..bd21405 100644 (file)
@@ -17,13 +17,19 @@ namespace TYPO3\CMS\Extensionmanager\Utility;
 
 use Symfony\Component\Finder\Finder;
 use TYPO3\CMS\Core\Configuration\ExtensionConfiguration;
+use TYPO3\CMS\Core\Configuration\SiteConfiguration;
 use TYPO3\CMS\Core\Core\Environment;
 use TYPO3\CMS\Core\Database\Schema\SchemaMigrator;
 use TYPO3\CMS\Core\Database\Schema\SqlReader;
+use TYPO3\CMS\Core\Log\LogLevel;
+use TYPO3\CMS\Core\Log\LogManager;
 use TYPO3\CMS\Core\Service\OpcodeCacheService;
+use TYPO3\CMS\Core\Site\Entity\Site;
 use TYPO3\CMS\Core\Utility\GeneralUtility;
+use TYPO3\CMS\Extbase\SignalSlot\Dispatcher;
 use TYPO3\CMS\Extensionmanager\Domain\Model\Extension;
 use TYPO3\CMS\Extensionmanager\Exception\ExtensionManagerException;
+use TYPO3\CMS\Impexp\Import;
 use TYPO3\CMS\Impexp\Utility\ImportExportUtility;
 
 /**
@@ -68,7 +74,7 @@ class InstallUtility implements \TYPO3\CMS\Core\SingletonInterface
     protected $cacheManager;
 
     /**
-     * @var \TYPO3\CMS\Extbase\SignalSlot\Dispatcher
+     * @var Dispatcher
      */
     protected $signalSlotDispatcher;
 
@@ -134,9 +140,9 @@ class InstallUtility implements \TYPO3\CMS\Core\SingletonInterface
     }
 
     /**
-     * @param \TYPO3\CMS\Extbase\SignalSlot\Dispatcher $signalSlotDispatcher
+     * @param Dispatcher $signalSlotDispatcher
      */
-    public function injectSignalSlotDispatcher(\TYPO3\CMS\Extbase\SignalSlot\Dispatcher $signalSlotDispatcher)
+    public function injectSignalSlotDispatcher(Dispatcher $signalSlotDispatcher)
     {
         $this->signalSlotDispatcher = $signalSlotDispatcher;
     }
@@ -190,8 +196,8 @@ class InstallUtility implements \TYPO3\CMS\Core\SingletonInterface
         $extension = $this->enrichExtensionWithDetails($extensionKey, false);
         $this->importInitialFiles($extension['siteRelPath'] ?? '', $extensionKey);
         $this->importStaticSqlFile($extension['siteRelPath']);
-        $this->importT3DFile($extension['siteRelPath']);
-        $this->importSiteConfiguration($extension['siteRelPath']);
+        $import = $this->importT3DFile($extension['siteRelPath']);
+        $this->importSiteConfiguration($extension['siteRelPath'], $import);
     }
 
     /**
@@ -493,8 +499,9 @@ class InstallUtility implements \TYPO3\CMS\Core\SingletonInterface
      * Execution state is saved in the this->registry, so it only happens once
      *
      * @param string $extensionSiteRelPath
+     * @return Import|null
      */
-    protected function importT3DFile($extensionSiteRelPath)
+    protected function importT3DFile($extensionSiteRelPath): ?Import
     {
         $registryKeysToCheck = [
             $extensionSiteRelPath . 'Initialisation/data.t3d',
@@ -503,7 +510,7 @@ class InstallUtility implements \TYPO3\CMS\Core\SingletonInterface
         foreach ($registryKeysToCheck as $registryKeyToCheck) {
             if ($this->registry->get('extensionDataImport', $registryKeyToCheck)) {
                 // Data was imported before => early return
-                return;
+                return null;
             }
         }
         $importFileToUse = null;
@@ -524,11 +531,13 @@ class InstallUtility implements \TYPO3\CMS\Core\SingletonInterface
                 $importResult = $importExportUtility->importT3DFile(Environment::getPublicPath() . '/' . $importFileToUse, 0);
                 $this->registry->set('extensionDataImport', $extensionSiteRelPath . 'Initialisation/dataImported', 1);
                 $this->emitAfterExtensionT3DImportSignal($importFileToUse, $importResult);
+                return $importExportUtility->getImport();
             } catch (\ErrorException $e) {
-                $logger = $this->objectManager->get(\TYPO3\CMS\Core\Log\LogManager::class)->getLogger(__CLASS__);
-                $logger->log(\TYPO3\CMS\Core\Log\LogLevel::WARNING, $e->getMessage());
+                $logger = $this->objectManager->get(LogManager::class)->getLogger(__CLASS__);
+                $logger->log(LogLevel::WARNING, $e->getMessage());
             }
         }
+        return null;
     }
 
     /**
@@ -613,8 +622,9 @@ class InstallUtility implements \TYPO3\CMS\Core\SingletonInterface
 
     /**
      * @param string $extensionSiteRelPath
+     * @param Import|null $import
      */
-    protected function importSiteConfiguration(string $extensionSiteRelPath): void
+    protected function importSiteConfiguration(string $extensionSiteRelPath, Import $import = null): void
     {
         $importRelFolder = $extensionSiteRelPath . 'Initialisation/Site';
         $importAbsFolder = Environment::getPublicPath() . '/' . $importRelFolder;
@@ -624,18 +634,59 @@ class InstallUtility implements \TYPO3\CMS\Core\SingletonInterface
             return;
         }
 
+        $logger = $this->objectManager->get(LogManager::class)->getLogger(__CLASS__);
+        $siteConfiguration = GeneralUtility::makeInstance(
+            SiteConfiguration::class,
+            $destinationFolder
+        );
+        $existingSites = $siteConfiguration->resolveAllExistingSites(false);
+
         GeneralUtility::mkdir($destinationFolder);
         $finder = GeneralUtility::makeInstance(Finder::class);
         $finder->directories()->in($importAbsFolder);
         if ($finder->hasResults()) {
             foreach ($finder as $siteConfigDirectory) {
-                $targetDir = $destinationFolder . '/' . $siteConfigDirectory->getBasename();
-                if (!$this->registry->get('siteConfigImport', $targetDir) && !is_dir($targetDir)) {
+                $siteIdentifier = $siteConfigDirectory->getBasename();
+                if (isset($existingSites[$siteIdentifier])) {
+                    $logger->log(
+                        LogLevel::WARNING,
+                        sprintf(
+                            'Skipped importing site configuration from %s due to existing site identifier %s',
+                            $extensionSiteRelPath,
+                            $siteIdentifier
+                        )
+                    );
+                    continue;
+                }
+                $targetDir = $destinationFolder . '/' . $siteIdentifier;
+                if (!$this->registry->get('siteConfigImport', $siteIdentifier) && !is_dir($targetDir)) {
                     GeneralUtility::mkdir($targetDir);
                     GeneralUtility::copyDirectory($siteConfigDirectory->getPathname(), $targetDir);
-                    $this->registry->set('siteConfigImport', $targetDir, 1);
+                    $this->registry->set('siteConfigImport', $siteIdentifier, 1);
                 }
             }
         }
+
+        /** @var Site[] $newSites */
+        $newSites = array_diff_key($siteConfiguration->resolveAllExistingSites(false), $existingSites);
+        $importedPages = $import->import_mapId['pages'] ?? null;
+
+        foreach ($newSites as $newSite) {
+            $exportedPageId = $newSite->getRootPageId();
+            $importedPageId = $importedPages[$exportedPageId] ?? null;
+            if ($importedPageId === null) {
+                $logger->log(
+                    LogLevel::WARNING,
+                    sprintf(
+                        'Imported site configuration with identifier %s could not be mapped to imported page id',
+                        $newSite->getIdentifier()
+                    )
+                );
+                continue;
+            }
+            $configuration = $siteConfiguration->load($newSite->getIdentifier());
+            $configuration['rootPageId'] = $importedPageId;
+            $siteConfiguration->write($newSite->getIdentifier(), $configuration);
+        }
     }
 }
index 236b325..c83cb45 100644 (file)
@@ -16,10 +16,15 @@ namespace TYPO3\CMS\Extensionmanager\Tests\Unit\Utility;
  */
 
 use Prophecy\Argument;
+use Psr\Log\NullLogger;
+use Symfony\Component\Yaml\Yaml;
 use TYPO3\CMS\Core\Cache\CacheManager;
+use TYPO3\CMS\Core\Cache\Frontend\NullFrontend;
 use TYPO3\CMS\Core\Core\Environment;
+use TYPO3\CMS\Core\Log\LogManager;
 use TYPO3\CMS\Core\Registry;
 use TYPO3\CMS\Core\Utility\GeneralUtility;
+use TYPO3\CMS\Extbase\Object\ObjectManager;
 use TYPO3\CMS\Extensionmanager\Utility\DependencyUtility;
 use TYPO3\CMS\Extensionmanager\Utility\InstallUtility;
 use TYPO3\CMS\Extensionmanager\Utility\ListUtility;
@@ -47,6 +52,8 @@ class InstallUtilityTest extends UnitTestCase
 
     protected $backupEnvironment = true;
 
+    protected $resetSingletonInstances = true;
+
     /**
      * @var \PHPUnit_Framework_MockObject_MockObject|InstallUtility|\TYPO3\TestingFramework\Core\AccessibleObjectInterface
      */
@@ -91,6 +98,10 @@ class InstallUtilityTest extends UnitTestCase
             ->method('enrichExtensionWithDetails')
             ->with($this->extensionKey)
             ->will($this->returnCallback([$this, 'getExtensionData']));
+
+        $cacheManagerProphecy = $this->prophesize(CacheManager::class);
+        $cacheManagerProphecy->getCache('core')->willReturn(new NullFrontend('core'));
+        GeneralUtility::setSingletonInstance(CacheManager::class, $cacheManagerProphecy->reveal());
     }
 
     protected function tearDown(): void
@@ -288,13 +299,20 @@ class InstallUtilityTest extends UnitTestCase
         // prepare an extension with a shipped site config
         $extKey = $this->createFakeExtension();
         $absPath = Environment::getProjectPath() . '/' . $this->fakedExtensions[$extKey]['siteRelPath'];
+        $config = Yaml::dump(['dummy' => true]);
         $siteIdentifier = 'site_identifier';
         GeneralUtility::mkdir_deep($absPath . 'Initialisation/Site/' . $siteIdentifier);
-        file_put_contents($absPath . 'Initialisation/Site/' . $siteIdentifier . '/config.yaml', 'DUMMY');
+        file_put_contents($absPath . 'Initialisation/Site/' . $siteIdentifier . '/config.yaml', $config);
 
         $subject = new InstallUtility();
         $listUtility = $this->prophesize(ListUtility::class);
         $subject->injectListUtility($listUtility->reveal());
+        $logManagerProphecy = $this->prophesize(LogManager::class);
+        $logManagerProphecy->getLogger(InstallUtility::class)->willReturn(new NullLogger());
+        $objectManagerProphecy = $this->prophesize(ObjectManager::class);
+        $objectManagerProphecy->get(LogManager::class)->willReturn($logManagerProphecy->reveal());
+        $subject->injectObjectManager($objectManagerProphecy->reveal());
+
         $availableExtensions = [
             $extKey => [
                 'siteRelPath' => $this->fakedExtensions[$extKey]['siteRelPath'],
@@ -327,10 +345,10 @@ class InstallUtilityTest extends UnitTestCase
         );
         $subject->processExtensionSetup($extKey);
 
-        $registry->set('siteConfigImport', $configDir . '/sites/' . $siteIdentifier, 1)->shouldHaveBeenCalled();
+        $registry->set('siteConfigImport', $siteIdentifier, 1)->shouldHaveBeenCalled();
         $siteConfigFile = $configDir . '/sites/' . $siteIdentifier . '/config.yaml';
         self::assertFileExists($siteConfigFile);
-        self::assertSame(file_get_contents($siteConfigFile), 'DUMMY');
+        self::assertStringEqualsFile($siteConfigFile, $config);
     }
 
     /**
@@ -343,20 +361,27 @@ class InstallUtilityTest extends UnitTestCase
         $absPath = Environment::getProjectPath() . '/' . $this->fakedExtensions[$extKey]['siteRelPath'];
         $siteIdentifier = 'site_identifier';
         GeneralUtility::mkdir_deep($absPath . 'Initialisation/Site/' . $siteIdentifier);
-        file_put_contents($absPath . 'Initialisation/Site/' . $siteIdentifier . '/config.yaml', 'DUMMY');
+        file_put_contents($absPath . 'Initialisation/Site/' . $siteIdentifier . '/config.yaml', Yaml::dump(['dummy' => true]));
 
         // fake an already existing site config in test output folder
         $configDir = $absPath . 'Result/config';
         if (!file_exists($configDir)) {
             GeneralUtility::mkdir_deep($configDir);
         }
+        $config = Yaml::dump(['foo' => 'bar']);
         $existingSiteConfig = 'sites/' . $siteIdentifier . '/config.yaml';
         GeneralUtility::mkdir_deep($configDir . '/sites/' . $siteIdentifier);
-        file_put_contents($configDir . '/' . $existingSiteConfig, 'config data already exists. Don\'t touch!');
+        file_put_contents($configDir . '/' . $existingSiteConfig, $config);
 
         $subject = new InstallUtility();
         $listUtility = $this->prophesize(ListUtility::class);
         $subject->injectListUtility($listUtility->reveal());
+        $logManagerProphecy = $this->prophesize(LogManager::class);
+        $logManagerProphecy->getLogger(InstallUtility::class)->willReturn(new NullLogger());
+        $objectManagerProphecy = $this->prophesize(ObjectManager::class);
+        $objectManagerProphecy->get(LogManager::class)->willReturn($logManagerProphecy->reveal());
+        $subject->injectObjectManager($objectManagerProphecy->reveal());
+
         $availableExtensions = [
             $extKey => [
                 'siteRelPath' => $this->fakedExtensions[$extKey]['siteRelPath'],
@@ -387,6 +412,6 @@ class InstallUtilityTest extends UnitTestCase
 
         $siteConfigFile = $configDir . '/sites/' . $siteIdentifier . '/config.yaml';
         self::assertFileExists($siteConfigFile);
-        self::assertSame(file_get_contents($siteConfigFile), 'config data already exists. Don\'t touch!');
+        self::assertStringEqualsFile($siteConfigFile, $config);
     }
 }
index 5385613..485e41b 100644 (file)
@@ -27,6 +27,19 @@ use TYPO3\CMS\Impexp\Import;
 class ImportExportUtility
 {
     /**
+     * @var Import|null
+     */
+    protected $import;
+
+    /**
+     * @return Import|null
+     */
+    public function getImport(): ?Import
+    {
+        return $this->import;
+    }
+
+    /**
      * Import a T3D file directly
      *
      * @param string $file The full absolute path to the file
@@ -43,25 +56,24 @@ class ImportExportUtility
         if (!is_int($pid)) {
             throw new \InvalidArgumentException('Input parameter $int has to be of type integer', 1377625646);
         }
-        /** @var Import $import */
-        $import = GeneralUtility::makeInstance(Import::class);
-        $import->init();
+        $this->import = GeneralUtility::makeInstance(Import::class);
+        $this->import->init();
 
-        $this->emitAfterImportExportInitialisationSignal($import);
+        $this->emitAfterImportExportInitialisationSignal($this->import);
 
         $importResponse = 0;
         if ($file && @is_file($file)) {
-            if ($import->loadFile($file, 1)) {
+            if ($this->import->loadFile($file, 1)) {
                 // Import to root page:
-                $import->importData($pid);
+                $this->import->importData($pid);
                 // Get id of first created page:
-                $newPages = $import->import_mapId['pages'];
+                $newPages = $this->import->import_mapId['pages'];
                 $importResponse = (int)reset($newPages);
             }
         }
 
         // Check for errors during the import process:
-        $errors = $import->printErrorLog();
+        $errors = $this->import->printErrorLog();
         if ($errors !== '') {
             $logger = GeneralUtility::makeInstance(LogManager::class)->getLogger(__CLASS__);
             $logger->warning($errors);