[BUGFIX] Respect uniqueInSite when moving records 34/60534/10
authorBenni Mack <benni@typo3.org>
Wed, 24 Apr 2019 19:54:33 +0000 (21:54 +0200)
committerGeorg Ringer <georg.ringer@gmail.com>
Tue, 7 May 2019 03:42:44 +0000 (05:42 +0200)
When a page is moved from one site to another site
(two pagetrees with different sites), currently a duplicate
slug is possible. This should never happen, so the same
logic as for "uniqueInPid" is applied when moving records.

Resolves: #87884
Releases: master, 9.5
Change-Id: I9a2d5756958e09fa89fbbc384d03c0503b70bf8c
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/60534
Tested-by: TYPO3com <noreply@typo3.com>
Tested-by: Susanne Moog <look@susi.dev>
Tested-by: Georg Ringer <georg.ringer@gmail.com>
Reviewed-by: Susanne Moog <look@susi.dev>
Reviewed-by: Georg Ringer <georg.ringer@gmail.com>
typo3/sysext/core/Classes/DataHandling/DataHandler.php
typo3/sysext/core/Classes/DataHandling/SlugHelper.php
typo3/sysext/core/Classes/Routing/SiteMatcher.php
typo3/sysext/core/Classes/Site/PseudoSiteFinder.php
typo3/sysext/core/Tests/Functional/DataHandling/Regular/DataSet/LiveDefaultMultiSitePages.csv [new file with mode: 0644]
typo3/sysext/core/Tests/Functional/DataHandling/Regular/MultiSite/DataSet/moveRootPageToDifferentPageTree.csv [new file with mode: 0644]
typo3/sysext/core/Tests/Functional/DataHandling/Regular/MultiSiteTest.php [new file with mode: 0644]

index 6dd58f1..a3b20ae 100644 (file)
@@ -4038,6 +4038,10 @@ class DataHandler implements LoggerAwareInterface
                 // Clear cache after moving
                 $this->registerRecordIdForPageCacheClearing($table, $uid);
                 $this->fixUniqueInPid($table, $uid);
+                $this->fixUniqueInSite($table, (int)$uid);
+                if ($table === 'pages') {
+                    $this->fixUniqueInSiteForSubpages((int)$uid);
+                }
                 // fixCopyAfterDuplFields
                 if ($origDestPid < 0) {
                     $this->fixCopyAfterDuplFields($table, $uid, abs($origDestPid), 1);
@@ -4096,8 +4100,11 @@ class DataHandler implements LoggerAwareInterface
                     }
                     // Clear cache after moving
                     $this->registerRecordIdForPageCacheClearing($table, $uid);
-                    // fixUniqueInPid
                     $this->fixUniqueInPid($table, $uid);
+                    $this->fixUniqueInSite($table, (int)$uid);
+                    if ($table === 'pages') {
+                        $this->fixUniqueInSiteForSubpages((int)$uid);
+                    }
                     // fixCopyAfterDuplFields
                     if ($origDestPid < 0) {
                         $this->fixCopyAfterDuplFields($table, $uid, abs($origDestPid), 1);
@@ -7492,6 +7499,57 @@ class DataHandler implements LoggerAwareInterface
     }
 
     /**
+     * Checks if any uniqueInSite eval fields are in the record and if so, they are re-written to be correct.
+     *
+     * @param string $table Table name
+     * @param int $uid Record UID
+     * @return bool whether the record had to be fixed or not
+     */
+    protected function fixUniqueInSite(string $table, int $uid): bool
+    {
+        $curData = $this->recordInfo($table, $uid, '*');
+        $workspaceId = $this->BE_USER->workspace;
+        $newData = [];
+        foreach ($GLOBALS['TCA'][$table]['columns'] as $field => $conf) {
+            if ($conf['config']['type'] === 'slug' && (string)$curData[$field] !== '') {
+                $evalCodesArray = GeneralUtility::trimExplode(',', $conf['config']['eval'], true);
+                if (in_array('uniqueInSite', $evalCodesArray, true)) {
+                    $helper = GeneralUtility::makeInstance(SlugHelper::class, $table, $field, $conf['config'], $workspaceId);
+                    $state = RecordStateFactory::forName($table)->fromArray($curData);
+                    $newValue = $helper->buildSlugForUniqueInSite($curData[$field], $state);
+                    if ((string)$newValue !== (string)$curData[$field]) {
+                        $newData[$field] = $newValue;
+                    }
+                }
+            }
+        }
+        // IF there are changed fields, then update the database
+        if (!empty($newData)) {
+            $this->updateDB($table, $uid, $newData);
+            return true;
+        }
+        return false;
+    }
+
+    /**
+     * Check if there are subpages that need an adoption as well
+     * @param int $pageId
+     * @param int $pid
+     */
+    protected function fixUniqueInSiteForSubpages(int $pageId)
+    {
+        // Get ALL subpages to update - read-permissions are respected
+        $subPages = $this->int_pageTreeInfo([], $pageId, 99, $pageId);
+        // Now fix uniqueInSite for subpages
+        foreach ($subPages as $thePageUid => $thePagePid) {
+            $recordWasModified = $this->fixUniqueInSite('pages', $thePageUid);
+            if ($recordWasModified) {
+                // @todo: Add logging and history - but how? we don't know the data that was in the system before
+            }
+        }
+    }
+
+    /**
      * When er record is copied you can specify fields from the previous record which should be copied into the new one
      * This function is also called with new elements. But then $update must be set to zero and $newData containing the data array. In that case data in the incoming array is NOT overridden. (250202)
      *
index 4356ea3..7994c3e 100644 (file)
@@ -301,6 +301,7 @@ class SlugHelper
         // The installation contains at least ONE other record with the same slug
         // Now find out if it is the same root page ID
         $siteMatcher = GeneralUtility::makeInstance(SiteMatcher::class);
+        $siteMatcher->refresh();
         $siteOfCurrentRecord = $siteMatcher->matchByPageId($pageId);
         foreach ($records as $record) {
             try {
index 89dacc7..ffdd10a 100644 (file)
@@ -21,6 +21,7 @@ use Symfony\Component\Routing\Exception\NoConfigurationException;
 use Symfony\Component\Routing\Exception\ResourceNotFoundException;
 use Symfony\Component\Routing\Matcher\UrlMatcher;
 use Symfony\Component\Routing\RequestContext;
+use TYPO3\CMS\Core\Cache\CacheManager;
 use TYPO3\CMS\Core\Exception\SiteNotFoundException;
 use TYPO3\CMS\Core\SingletonInterface;
 use TYPO3\CMS\Core\Site\Entity\PseudoSite;
@@ -29,6 +30,7 @@ use TYPO3\CMS\Core\Site\Entity\SiteLanguage;
 use TYPO3\CMS\Core\Site\PseudoSiteFinder;
 use TYPO3\CMS\Core\Site\SiteFinder;
 use TYPO3\CMS\Core\Utility\GeneralUtility;
+use TYPO3\CMS\Core\Utility\RootlineUtility;
 
 /**
  * Returns a site or pseudo-site (with sys_domain records) based on a given request.
@@ -68,6 +70,22 @@ class SiteMatcher implements SingletonInterface
     }
 
     /**
+     * Only used when a page is moved but the pseudo site caches has this information hard-coded, so the caches
+     * need to be flushed.
+     *
+     * @internal
+     * @throws \TYPO3\CMS\Core\Cache\Exception\NoSuchCacheException
+     */
+    public function refresh()
+    {
+        /** Ensure root line caches are flushed */
+        RootlineUtility::purgeCaches();
+        GeneralUtility::makeInstance(CacheManager::class)->getCache('cache_rootline')->flush();
+        $this->pseudoSiteFinder = GeneralUtility::makeInstance(PseudoSiteFinder::class);
+        $this->pseudoSiteFinder->refresh();
+    }
+
+    /**
      * First, it is checked, if a "id" GET/POST parameter is found.
      * If it is, we check for a valid site mounted there.
      *
index 1a072af..66c7274 100644 (file)
@@ -60,10 +60,10 @@ class PseudoSiteFinder
      * Fetches all site root pages, all sys_language records and forms pseudo-sites,
      * but only for the pagetree's that do not have a site configuration available.
      */
-    protected function populate()
+    protected function populate(bool $allowCaching = true)
     {
         $data = $this->cache->get($this->cacheIdentifier);
-        if (empty($data)) {
+        if (empty($data) || $allowCaching === false) {
             $allLanguages = $this->getAllLanguageRecords();
             $availablePages = $this->getAllRootPagesWithoutSiteConfiguration();
             $this->cache->set($this->cacheIdentifier, json_encode([$allLanguages, $availablePages]));
@@ -101,6 +101,16 @@ class PseudoSiteFinder
     }
 
     /**
+     * Rebuild the cache information from the database information.
+     *
+     * @internal
+     */
+    public function refresh()
+    {
+        $this->populate(false);
+    }
+
+    /**
      * Fetches all "sys_language" records.
      *
      * @return array
diff --git a/typo3/sysext/core/Tests/Functional/DataHandling/Regular/DataSet/LiveDefaultMultiSitePages.csv b/typo3/sysext/core/Tests/Functional/DataHandling/Regular/DataSet/LiveDefaultMultiSitePages.csv
new file mode 100644 (file)
index 0000000..73f3e29
--- /dev/null
@@ -0,0 +1,9 @@
+"pages",,,,,,,,,,,,,,
+,"uid","pid","sorting","deleted","sys_language_uid","l10n_parent","t3_origuid","t3ver_wsid","t3ver_state","t3ver_stage","t3ver_oid","t3ver_move_id","title","slug"
+,1,0,256,0,0,0,0,0,0,0,0,0,"FunctionalTest","/"
+,50,0,512,0,0,0,0,0,0,0,0,0,"Second Root Page","/"
+,51,50,128,0,0,0,0,0,0,0,0,0,"DataHandlerTest in second tree","/data-handler"
+,52,51,128,0,0,0,0,0,0,0,0,0,"Relations in second tree","/data-handler/relations"
+,88,1,256,0,0,0,0,0,0,0,0,0,"DataHandlerTest","/data-handler"
+,89,88,256,0,0,0,0,0,0,0,0,0,"Relations","/data-handler/relations"
+,90,88,512,0,0,0,0,0,0,0,0,0,"Target","/data-handler/target"
diff --git a/typo3/sysext/core/Tests/Functional/DataHandling/Regular/MultiSite/DataSet/moveRootPageToDifferentPageTree.csv b/typo3/sysext/core/Tests/Functional/DataHandling/Regular/MultiSite/DataSet/moveRootPageToDifferentPageTree.csv
new file mode 100644 (file)
index 0000000..625a9ee
--- /dev/null
@@ -0,0 +1,9 @@
+"pages",,,,,,,,,,,,
+,"uid","pid","sorting","deleted","t3_origuid","t3ver_wsid","t3ver_state","t3ver_stage","t3ver_oid","t3ver_move_id","title","slug"
+,1,0,256,0,0,0,0,0,0,0,"FunctionalTest","/"
+,50,1,128,0,0,0,0,0,0,0,"Second Root Page","/1"
+,51,50,128,0,0,0,0,0,0,0,"DataHandlerTest in second tree","/data-handler-1"
+,52,51,128,0,0,0,0,0,0,0,"Relations in second tree","/data-handler/relations-1"
+,88,1,256,0,0,0,0,0,0,0,"DataHandlerTest","/data-handler"
+,89,88,256,0,0,0,0,0,0,0,"Relations","/data-handler/relations"
+,90,88,512,0,0,0,0,0,0,0,"Target","/data-handler/target"
diff --git a/typo3/sysext/core/Tests/Functional/DataHandling/Regular/MultiSiteTest.php b/typo3/sysext/core/Tests/Functional/DataHandling/Regular/MultiSiteTest.php
new file mode 100644 (file)
index 0000000..925e48d
--- /dev/null
@@ -0,0 +1,71 @@
+<?php
+declare(strict_types = 1);
+
+namespace TYPO3\CMS\Core\Tests\Functional\DataHandling\Regular;
+
+/*
+ * This file is part of the TYPO3 CMS project.
+ *
+ * It is free software; you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License, either version 2
+ * of the License, or any later version.
+ *
+ * For the full copyright and license information, please read the
+ * LICENSE.txt file that was distributed with this source code.
+ *
+ * The TYPO3 project - inspiring people to share!
+ */
+
+use TYPO3\CMS\Core\Routing\SiteMatcher;
+use TYPO3\CMS\Core\Tests\Functional\DataHandling\AbstractDataHandlerActionTestCase;
+use TYPO3\CMS\Core\Utility\GeneralUtility;
+
+/**
+ * Functional test for the DataHandler when handling multiple pagetrees
+ */
+class MultiSiteTest extends AbstractDataHandlerActionTestCase
+{
+    const VALUE_PageIdWebsite = 1;
+    const VALUE_PageIdSecondSite = 50;
+
+    const TABLE_Page = 'pages';
+
+    /**
+     * @var string
+     */
+    protected $assertionDataSetDirectory = 'typo3/sysext/core/Tests/Functional/DataHandling/Regular/MultiSite/DataSet/';
+
+    /**
+     * @var string
+     */
+    protected $scenarioDataSetDirectory = 'typo3/sysext/core/Tests/Functional/DataHandling/Regular/DataSet/';
+
+    protected function setUp()
+    {
+        parent::setUp();
+
+        $this->importScenarioDataSet('LiveDefaultMultiSitePages');
+        $this->importScenarioDataSet('LiveDefaultElements');
+
+        $this->setUpFrontendRootPage(1, ['typo3/sysext/core/Tests/Functional/Fixtures/Frontend/JsonRenderer.typoscript']);
+    }
+
+    /**
+     * @test
+     * @see DataSet/moveRootPageToDifferentPageTree.csv
+     */
+    public function moveRootPageToDifferentPageTree()
+    {
+        // Warm up caches for the root line utility to identify side effects
+        GeneralUtility::makeInstance(SiteMatcher::class)->matchByPageId(self::VALUE_PageIdWebsite);
+        GeneralUtility::makeInstance(SiteMatcher::class)->matchByPageId(self::VALUE_PageIdSecondSite);
+
+        // URL is now "/1" for the second site
+        $this->actionService->moveRecord(self::TABLE_Page, self::VALUE_PageIdSecondSite, self::VALUE_PageIdWebsite);
+        $this->assertAssertionDataSet('moveRootPageToDifferentPageTree');
+
+        $responseSections = $this->getFrontendResponse(self::VALUE_PageIdSecondSite)->getResponseSections();
+        $this->assertThat($responseSections, $this->getRequestSectionHasRecordConstraint()
+            ->setTable(self::TABLE_Page)->setField('title')->setValues('Second Root Page'));
+    }
+}