[TASK] Ensure recursive page update on page movement

If a page is moved, subpages have to be considered, e.g. as rootline
and slug might have changed. Also the MountPagesUpdater has to be
triggered to ensure mounted pages will be updated too.

This commit adds the missing recursive updates, an extension to the
RecordMovedEvent and PageMovedEvent is done allowing that recursive
updates on move operations will only be done if the parent page of
the page has changed.

Resolves: #206

Classes/Domain/Index/Queue/UpdateHandler/DataUpdateHandler.php

@@ -231,12 +231,27 @@ public function handleVersionSwap(int $uid, string $table): void
     /**
      * Handle page move
      *
+     * @param int $uid
+     * @param int|null $previousParentId
      * @throws DBALException
      * @throws UnexpectedTYPO3SiteInitializationException
      */
-    public function handleMovedPage(int $uid): void
+    public function handleMovedPage(int $uid, ?int $previousParentId = null): void
     {
+        $excludedPages = $this->pagesRepository->findAllPagesWithinNoSearchSubEntriesMarkedPages();
+        if (in_array($uid, $excludedPages)) {
+            return;
+        }
+
         $this->applyPageChangesToQueue($uid);
+
+        if ($previousParentId !== null) {
+            $pageRecord = BackendUtility::getRecord('pages', $uid);
+            if ($pageRecord !== null && (int)$pageRecord['pid'] !== $previousParentId) {
+                $treePageIds = $this->getSubPageIds($uid);
+                $this->updatePageIdItems($treePageIds);
+            }
+        }
     }
 
     /**
@@ -483,7 +498,8 @@ protected function getIsTranslationParentRecordEnabled(string $recordTable, int
     protected function updatePageIdItems(array $treePageIds): void
     {
         foreach ($treePageIds as $treePageId) {
-            $this->indexQueue->updateItem('pages', $treePageId);
+            $this->indexQueue->updateItem('pages', $treePageId, time());
+            $this->mountPageUpdater->update($treePageId);
         }
     }
 

Classes/Domain/Index/Queue/UpdateHandler/EventListener/ImmediateProcessingEventListener.php

@@ -104,7 +104,7 @@ protected function handleVersionSwappedEvent(VersionSwappedEvent $event): void
     protected function handleRecordMovedEvent(RecordMovedEvent $event): void
     {
         if ($event->isPageUpdate()) {
-            $this->getDataUpdateHandler()->handleMovedPage($event->getUid());
+            $this->getDataUpdateHandler()->handleMovedPage($event->getUid(), $event->getPreviousParentId());
         } else {
             $this->getDataUpdateHandler()->handleMovedRecord($event->getUid(), $event->getTable());
         }
@@ -150,7 +150,7 @@ protected function handleRecordDeletedEvent(RecordDeletedEvent $event): void
      */
     protected function handlePageMovedEvent(PageMovedEvent $event): void
     {
-        $this->getGarbageHandler()->handlePageMovement($event->getUid());
+        $this->getGarbageHandler()->handlePageMovement($event->getUid(), $event->getPreviousParentId());
     }
 
     /**

Classes/Domain/Index/Queue/UpdateHandler/Events/AbstractRecordMovedEvent.php

@@ -0,0 +1,51 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * 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!
+ */
+
+namespace ApacheSolrForTypo3\Solr\Domain\Index\Queue\UpdateHandler\Events;
+
+/**
+ * Abstract event base for events fired if a record or page is moved
+ */
+abstract class AbstractRecordMovedEvent extends AbstractDataUpdateEvent
+{
+    /**
+     * pid of the record prior moving
+     *
+     * @var int|null
+     */
+    protected ?int $previousParentId = null;
+
+    /**
+     * Sets the record's pid prior moving
+     *
+     * @param int $pid
+     */
+    public function setPreviousParentId(int $pid)
+    {
+        $this->previousParentId = $pid;
+    }
+
+    /**
+     * Returns the record's pid prior moving
+     *
+     * @return int|null
+     */
+    public function getPreviousParentId(): ?int
+    {
+        return $this->previousParentId;
+    }
+}

Classes/Domain/Index/Queue/UpdateHandler/Events/PageMovedEvent.php

@@ -20,7 +20,7 @@
 /**
  * Event fired if a page is moved
  */
-class PageMovedEvent extends AbstractDataUpdateEvent
+class PageMovedEvent extends AbstractRecordMovedEvent
 {
     /**
      * Constructor

Classes/Domain/Index/Queue/UpdateHandler/Events/RecordMovedEvent.php

@@ -20,4 +20,4 @@
 /**
  * Event fired if a record is moved
  */
-class RecordMovedEvent extends AbstractDataUpdateEvent {}
+class RecordMovedEvent extends AbstractRecordMovedEvent {}

Classes/Domain/Index/Queue/UpdateHandler/GarbageHandler.php

@@ -22,6 +22,7 @@
 use Doctrine\DBAL\Exception as DBALException;
 use PDO;
 use Throwable;
+use TYPO3\CMS\Backend\Utility\BackendUtility;
 use TYPO3\CMS\Core\Context\Exception\AspectNotFoundException;
 use TYPO3\CMS\Core\Utility\GeneralUtility;
 use UnexpectedValueException;
@@ -79,20 +80,30 @@ public function collectGarbage(string $table, int $uid): void
     /**
      * Handles moved pages
      *
+     * As rootline and page slug might have changed on page movement,
+     * document have to be removed from Solr. Reindexing is taken
+     * care of by the DataUpdateHandler.
+     *
+     * @param int $uid
+     * @param int|null $previousParentId
      * @throws DBALException
      * @throws UnexpectedTYPO3SiteInitializationException
      */
-    public function handlePageMovement(int $uid): void
+    public function handlePageMovement(int $uid, ?int $previousParentId = null): void
     {
-        // TODO the below comment is not valid anymore, pid has been removed from doc ID
-        // ...still needed?
-
-        // must be removed from index since the pid changes and
-        // is part of the Solr document ID
         $this->collectGarbage('pages', $uid);
 
-        // now re-index with new properties
-        $this->indexQueue->updateItem('pages', $uid);
+        // collect garbage of subpages
+        if ($previousParentId !== null) {
+            $pageRecord = BackendUtility::getRecord('pages', $uid);
+            if ($pageRecord !== null && (int)$pageRecord['pid'] !== $previousParentId) {
+                $subPageIds = $this->getSubPageIds($uid);
+                array_walk(
+                    $subPageIds,
+                    fn (int $subPageId) => $this->collectGarbage('pages', $subPageId)
+                );
+            }
+        }
     }
 
     /**

Classes/GarbageCollector.php

@@ -23,6 +23,7 @@
 use ApacheSolrForTypo3\Solr\Domain\Index\Queue\UpdateHandler\GarbageHandler;
 use ApacheSolrForTypo3\Solr\System\TCA\TCAService;
 use Psr\EventDispatcher\EventDispatcherInterface;
+use TYPO3\CMS\Backend\Utility\BackendUtility;
 use TYPO3\CMS\Core\DataHandling\DataHandler;
 use TYPO3\CMS\Core\SingletonInterface;
 use TYPO3\CMS\Core\Utility\GeneralUtility;
@@ -67,11 +68,18 @@ public function __construct(TCAService $TCAService = null, EventDispatcherInterf
      */
     public function processCmdmap_preProcess($command, $table, $uid, $value, DataHandler $tceMain): void
     {
-        // workspaces: collect garbage only for LIVE workspace
-        if ($command === 'delete' && ($GLOBALS['BE_USER']->workspace ?? null) == 0) {
+        // workspaces: process command map only for LIVE workspace
+        if (($GLOBALS['BE_USER']->workspace ?? null) != 0) {
+            return;
+        }
+
+        if ($command === 'delete') {
             $this->eventDispatcher->dispatch(
                 new RecordDeletedEvent((int)$uid, (string)$table)
             );
+        } elseif ($command === 'move' && $table === 'pages') {
+            $pageRow = BackendUtility::getRecord('pages', $uid);
+            $this->trackedRecords['pages'][$uid] = $pageRow;
         }
     }
 
@@ -104,9 +112,11 @@ public function processCmdmap_postProcess($command, $table, $uid, $value, DataHa
     {
         // workspaces: collect garbage only for LIVE workspace
         if ($command === 'move' && $table === 'pages' && ($GLOBALS['BE_USER']->workspace ?? null) == 0) {
-            $this->eventDispatcher->dispatch(
-                new PageMovedEvent((int)$uid)
-            );
+            $event = new PageMovedEvent((int)$uid);
+            if (($this->trackedRecords['pages'][$uid] ?? null) !== null) {
+                $event->setPreviousParentId((int)$this->trackedRecords['pages'][$uid]['pid']);
+            }
+            $this->eventDispatcher->dispatch($event);
         }
     }
 

Classes/IndexQueue/RecordMonitor.php

@@ -24,6 +24,7 @@
 use ApacheSolrForTypo3\Solr\System\Configuration\ExtensionConfiguration;
 use ApacheSolrForTypo3\Solr\Util;
 use Psr\EventDispatcher\EventDispatcherInterface;
+use TYPO3\CMS\Backend\Utility\BackendUtility;
 use TYPO3\CMS\Core\DataHandling\DataHandler;
 use TYPO3\CMS\Core\Exception\Page\PageNotFoundException;
 use TYPO3\CMS\Core\Utility\GeneralUtility;
@@ -38,6 +39,7 @@
  */
 class RecordMonitor
 {
+    protected array $trackedRecords = [];
     protected EventDispatcherInterface $eventDispatcher;
 
     public function __construct(EventDispatcherInterface $eventDispatcher = null)
@@ -59,10 +61,17 @@ public function processCmdmap_preProcess(
         $table,
         $uid,
     ): void {
-        if ($command === 'delete' && $table === 'tt_content' && ($GLOBALS['BE_USER']->workspace ?? null) == 0) {
+        if (($GLOBALS['BE_USER']->workspace ?? null) != 0) {
+            return;
+        }
+
+        if ($command === 'delete' && $table === 'tt_content') {
             $this->eventDispatcher->dispatch(
                 new ContentElementDeletedEvent($uid)
             );
+        } elseif ($command === 'move' && $table === 'pages') {
+            $pageRow = BackendUtility::getRecord('pages', $uid);
+            $this->trackedRecords['pages'][$uid] = $pageRow;
         }
     }
 
@@ -97,9 +106,11 @@ public function processCmdmap_postProcess(
 
         // moving pages/records in LIVE workspace
         if ($command === 'move' && ($GLOBALS['BE_USER']->workspace ?? null) == 0) {
-            $this->eventDispatcher->dispatch(
-                new RecordMovedEvent($uid, $table)
-            );
+            $event = new RecordMovedEvent($uid, $table);
+            if ($table === 'pages' && ($this->trackedRecords['pages'][$uid] ?? null) !== null) {
+                $event->setPreviousParentId((int)$this->trackedRecords['pages'][$uid]['pid']);
+            }
+            $this->eventDispatcher->dispatch($event);
         }
     }
 

Tests/Integration/Fixtures/can_collect_garbage_if_page_tree_is_moved.csv

@@ -0,0 +1,9 @@
+"pages",
+,"uid","pid","is_siteroot","doktype","sys_language_uid","l10n_parent","slug","title","sorting","no_search_sub_entries"
+,2,1,0,1,0,0,"/1st-subpage-in-root-1","1st subpage in root 1",10,0
+,3,1,0,1,0,0,"/2nd-subpage-in-root 1","2nd subpage in root 1",20,0
+,4,1,0,254,"sys_language_uid","l10n_parent","/sysfolder-with-set-option-no_search_sub_entries","sysfolder with set option no_search_sub_entries",99,1
+,10,1,0,1,0,0,"/root-of-subtree-to-be-moved","root of subtree to be moved",30,0
+,11,10,0,1,0,0,"/root-of-subtree-to-be-moved/first-child-of-subtree-to-be-moved","first child of subtree to be moved",10,0
+,12,10,0,1,0,0,"/root-of-subtree-to-be-moved/2nd-child-of-subtree-to-be-moved","2nd child of subtree to be moved",20,0
+,13,11,0,1,0,0,"/root-of-subtree-to-be-moved/first-child-of-subtree-to-be-moved/3rd-child-of-subtree-to-be-moved","3rd child of subtree to be moved",10,0

Tests/Integration/GarbageCollectorTest.php

@@ -116,7 +116,9 @@ protected function tearDown(): void
             $this->garbageCollector,
             $this->indexer,
             $this->extensionConfiguration,
-            $this->eventQueue
+            $this->eventQueue,
+            $this->backendUser,
+            $GLOBALS['LANG']
         );
         parent::tearDown();
     }
@@ -337,6 +339,83 @@ protected function prepareCanCollectGarbageFromSubPagesWhenPageIsSetToHiddenAndE
         $this->garbageCollector->processDatamap_afterDatabaseOperations('update', 'pages', 2, $changeSet, $dataHandler);
     }
 
+    /**
+     * @test
+     */
+    public function canCollectGarbageIfPageTreeIsMoved(): void
+    {
+        $this->importCSVDataSet(__DIR__ . '/Fixtures/can_collect_garbage_if_page_tree_is_moved.csv');
+
+        $this->assertEmptyIndexQueue();
+        $this->addToQueueAndIndexRecord('pages', 10);
+        $this->addToQueueAndIndexRecord('pages', 11);
+        $this->addToQueueAndIndexRecord('pages', 12);
+        $this->addToQueueAndIndexRecord('pages', 13);
+        $this->waitToBeVisibleInSolr();
+        $this->assertSolrContainsDocumentCount(4);
+
+        $this->dataHandler->start(
+            [],
+            ['pages' => [10 => ['move' => 2]]],
+            $this->backendUser
+        );
+
+        $this->dataHandler->process_cmdmap();
+        $this->assertIndexQueueContainsItemAmount(4);
+        $this->assertSolrContainsDocumentCount(0);
+    }
+
+    /**
+     * @test
+     */
+    public function canCollectGarbageIfPageTreeIsMovedToSysfolderWithDisabledOptionIncludeSubEntriesInSearch(): void
+    {
+        $this->importCSVDataSet(__DIR__ . '/Fixtures/can_collect_garbage_if_page_tree_is_moved.csv');
+
+        $this->assertEmptyIndexQueue();
+        $this->addToQueueAndIndexRecord('pages', 10);
+        $this->addToQueueAndIndexRecord('pages', 11);
+        $this->addToQueueAndIndexRecord('pages', 12);
+        $this->addToQueueAndIndexRecord('pages', 13);
+        $this->waitToBeVisibleInSolr();
+        $this->assertIndexQueueContainsItemAmount(4);
+
+        $this->dataHandler->start(
+            [],
+            ['pages' => [10 => ['move' => 4]]],
+            $this->backendUser
+        );
+        $this->dataHandler->process_cmdmap();
+        $this->assertEmptyIndexQueue();
+        $this->assertSolrContainsDocumentCount(0);
+    }
+
+    /**
+     * @test
+     */
+    public function canCollectGarbageIfPageTreeIsMovedButStaysOnSamePage(): void
+    {
+        $this->importCSVDataSet(__DIR__ . '/Fixtures/can_collect_garbage_if_page_tree_is_moved.csv');
+
+        $this->assertEmptyIndexQueue();
+        $this->addToQueueAndIndexRecord('pages', 10);
+        $this->addToQueueAndIndexRecord('pages', 11);
+        $this->addToQueueAndIndexRecord('pages', 12);
+        $this->addToQueueAndIndexRecord('pages', 13);
+        $this->waitToBeVisibleInSolr();
+        $this->assertSolrContainsDocumentCount(4);
+
+        $this->dataHandler->start(
+            [],
+            ['pages' => [10 => ['move' => -2]]],
+            $this->backendUser
+        );
+
+        $this->dataHandler->process_cmdmap();
+        $this->assertIndexQueueContainsItemAmount(4);
+        $this->assertSolrContainsDocumentCount(3);
+    }
+
     /**
      * @test
      */

Tests/Integration/IndexQueue/Fixtures/can_handle_mounted_page_tree_movement.csv

@@ -0,0 +1,7 @@
+"pages",
+,"uid","pid","is_siteroot","doktype","sys_language_uid","l10n_parent","slug","title","sorting","no_search_sub_entries","mount_pid_ol","mount_pid"
+,2,1,0,1,0,0,"/1st-subpage-in-root-1","1st subpage in root 1",10,0,0,0
+,3,1,0,7,0,0,"/mount-point-for-subtree","mount point for subtree",20,0,0,10
+,4,1,0,254,"sys_language_uid","l10n_parent","/sysfolder-with-set-option-no_search_sub_entries","sysfolder with set option no_search_sub_entries",99,1,0,0
+,10,1,0,1,0,0,"/root-of-subtree-to-be-moved","root of subtree to be moved",30,0,0,0
+,11,10,0,1,0,0,"/root-of-subtree-to-be-moved/first-child-of-subtree-to-be-moved","first child of subtree to be moved",10,0,0,0

Tests/Integration/IndexQueue/Fixtures/can_handle_page_tree_movement.csv

@@ -0,0 +1,9 @@
+"pages",
+,"uid","pid","is_siteroot","doktype","sys_language_uid","l10n_parent","slug","title","sorting","no_search_sub_entries"
+,2,1,0,1,0,0,"/1st-subpage-in-root-1","1st subpage in root 1",10,0
+,3,1,0,1,0,0,"/2nd-subpage-in-root 1","2nd subpage in root 1",20,0
+,4,1,0,254,"sys_language_uid","l10n_parent","/sysfolder-with-set-option-no_search_sub_entries","sysfolder with set option no_search_sub_entries",99,1
+,10,1,0,1,0,0,"/root-of-subtree-to-be-moved","root of subtree to be moved",30,0
+,11,10,0,1,0,0,"/root-of-subtree-to-be-moved/first-child-of-subtree-to-be-moved","first child of subtree to be moved",10,0
+,12,10,0,1,0,0,"/root-of-subtree-to-be-moved/2nd-child-of-subtree-to-be-moved","2nd child of subtree to be moved",20,0
+,13,11,0,1,0,0,"/root-of-subtree-to-be-moved/first-child-of-subtree-to-be-moved/3rd-child-of-subtree-to-be-moved","3rd child of subtree to be moved",10,0