From 2cbe770599cf27a8a623d2e4768d4b9f9971d113 Mon Sep 17 00:00:00 2001
From: Oliver Hader <oliver@typo3.org>
Date: Sat, 3 Dec 2016 16:44:24 +0100
Subject: [PATCH] [BUGFIX] Skip swapping/publishing of deleted records

In case a content element and the accordant page have been deleted in
separate actions and get published together, the workspace process will
trigger an error message since the removed content element cannot be
published anymore (since it has been processed already with the page).

To avoid this behavior deleted records are collected and checked in the
workspace swapping/publishing process.

Change-Id: If04a198abf81efdc88e75da79da0c01cfaa361ff
Resolves: #47384
Releases: master, 7.6
Reviewed-on: https://review.typo3.org/23129
Tested-by: TYPO3com <no-reply@typo3.com>
Reviewed-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
Tested-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
Reviewed-by: Susanne Moog <susanne.moog@typo3.org>
Tested-by: Susanne Moog <susanne.moog@typo3.org>
---
 .../core/Classes/DataHandling/DataHandler.php | 25 +++++++++++++++++++
 .../version/Classes/Hook/DataHandlerHook.php  |  5 ++++
 .../Regular/AbstractActionTestCase.php        |  9 +++++++
 .../Regular/Modify/ActionTest.php             | 13 ++++++++++
 .../Modify/DataSet/deleteContentAndPage.csv   | 16 ++++++++++++
 .../Regular/Publish/ActionTest.php            | 14 +++++++++++
 .../Publish/DataSet/deleteContentAndPage.csv  | 17 +++++++++++++
 .../Regular/PublishAll/ActionTest.php         | 18 ++++++++++---
 .../DataSet/deleteContentAndPage.csv          | 17 +++++++++++++
 9 files changed, 130 insertions(+), 4 deletions(-)
 create mode 100644 typo3/sysext/workspaces/Tests/Functional/DataHandling/Regular/Modify/DataSet/deleteContentAndPage.csv
 create mode 100644 typo3/sysext/workspaces/Tests/Functional/DataHandling/Regular/Publish/DataSet/deleteContentAndPage.csv
 create mode 100644 typo3/sysext/workspaces/Tests/Functional/DataHandling/Regular/PublishAll/DataSet/deleteContentAndPage.csv

diff --git a/typo3/sysext/core/Classes/DataHandling/DataHandler.php b/typo3/sysext/core/Classes/DataHandling/DataHandler.php
index 3430d2c842b6..4b6858337c72 100644
--- a/typo3/sysext/core/Classes/DataHandling/DataHandler.php
+++ b/typo3/sysext/core/Classes/DataHandling/DataHandler.php
@@ -297,6 +297,13 @@ class DataHandler
      */
     public $copyMappingArray_merged = [];
 
+    /**
+     * Per-table array with UIDs that have been deleted.
+     *
+     * @var array
+     */
+    protected $deletedRecords = [];
+
     /**
      * A map between input file name and final destination for files being attached to records.
      *
@@ -4978,6 +4985,7 @@ class DataHandler
                     ->update($table, $updateFields, ['uid' => (int)$uid]);
                 // Delete all l10n records as well, impossible during undelete because it might bring too many records back to life
                 if (!$undeleteRecord) {
+                    $this->deletedRecords[$table][] = (int)$uid;
                     $this->deleteL10nOverlayRecords($table, $uid);
                 }
             } catch (DBALException $e) {
@@ -5020,6 +5028,7 @@ class DataHandler
                 GeneralUtility::makeInstance(ConnectionPool::class)
                     ->getConnectionForTable($table)
                     ->delete($table, ['uid' => (int)$uid]);
+                $this->deletedRecords[$table][] = (int)$uid;
                 $this->deleteL10nOverlayRecords($table, $uid);
             } catch (DBALException $e) {
                 $databaseErrorMessage = $e->getPrevious()->getMessage();
@@ -8517,6 +8526,22 @@ class DataHandler
         return $result;
     }
 
+    /**
+     * Determines whether a particular record has been deleted
+     * using DataHandler::deleteRecord() in this instance.
+     *
+     * @param string $tableName
+     * @param string $uid
+     * @return bool
+     */
+    public function hasDeletedRecord($tableName, $uid)
+    {
+        return
+            !empty($this->deletedRecords[$tableName])
+            && in_array($uid, $this->deletedRecords[$tableName])
+        ;
+    }
+
     /**
      * Gets the automatically versionized id of a record.
      *
diff --git a/typo3/sysext/version/Classes/Hook/DataHandlerHook.php b/typo3/sysext/version/Classes/Hook/DataHandlerHook.php
index 00cc08901de7..5d68d9d51687 100644
--- a/typo3/sysext/version/Classes/Hook/DataHandlerHook.php
+++ b/typo3/sysext/version/Classes/Hook/DataHandlerHook.php
@@ -765,6 +765,11 @@ class DataHandlerHook
 
         // Check prerequisites before start swapping
 
+        // Skip records that have been deleted during the current execution
+        if ($dataHandler->hasDeletedRecord($table, $id)) {
+            return;
+        }
+
         // First, check if we may actually edit the online record
         if (!$dataHandler->checkRecordUpdateAccess($table, $id)) {
             $dataHandler->newlog('Error: You cannot swap versions for a record you do not have access to edit!', 1);
diff --git a/typo3/sysext/workspaces/Tests/Functional/DataHandling/Regular/AbstractActionTestCase.php b/typo3/sysext/workspaces/Tests/Functional/DataHandling/Regular/AbstractActionTestCase.php
index 801d98e709b6..2ecbe792d4f4 100644
--- a/typo3/sysext/workspaces/Tests/Functional/DataHandling/Regular/AbstractActionTestCase.php
+++ b/typo3/sysext/workspaces/Tests/Functional/DataHandling/Regular/AbstractActionTestCase.php
@@ -230,6 +230,15 @@ abstract class AbstractActionTestCase extends \TYPO3\CMS\Core\Tests\Functional\D
         $this->actionService->deleteRecord(self::TABLE_Page, self::VALUE_PageId);
     }
 
+    /**
+     * @see DataSet/Assertion/deleteContentAndPage.csv
+     */
+    public function deleteContentAndPage()
+    {
+        $this->actionService->deleteRecord(self::TABLE_Content, self::VALUE_ContentIdSecond);
+        $this->actionService->deleteRecord(self::TABLE_Page, self::VALUE_PageId);
+    }
+
     /**
      * @see DataSet/Assertion/copyPageRecord.csv
      */
diff --git a/typo3/sysext/workspaces/Tests/Functional/DataHandling/Regular/Modify/ActionTest.php b/typo3/sysext/workspaces/Tests/Functional/DataHandling/Regular/Modify/ActionTest.php
index c9ed47898224..6b3102ac9b08 100644
--- a/typo3/sysext/workspaces/Tests/Functional/DataHandling/Regular/Modify/ActionTest.php
+++ b/typo3/sysext/workspaces/Tests/Functional/DataHandling/Regular/Modify/ActionTest.php
@@ -279,6 +279,19 @@ class ActionTest extends \TYPO3\CMS\Workspaces\Tests\Functional\DataHandling\Reg
         $this->assertContains('RuntimeException', $response->getError());
     }
 
+    /**
+     * @test
+     * @see DataSet/Assertion/deleteContentAndPage.csv
+     */
+    public function deleteContentAndPage()
+    {
+        parent::deleteContentAndPage();
+        $this->assertAssertionDataSet('deleteContentAndPage');
+
+        $response = $this->getFrontendResponse(self::VALUE_PageId, 0, self::VALUE_BackendUserId, self::VALUE_WorkspaceId, false);
+        $this->assertContains('RuntimeException', $response->getError());
+    }
+
     /**
      * @test
      * @see DataSet/Assertion/copyPageRecord.csv
diff --git a/typo3/sysext/workspaces/Tests/Functional/DataHandling/Regular/Modify/DataSet/deleteContentAndPage.csv b/typo3/sysext/workspaces/Tests/Functional/DataHandling/Regular/Modify/DataSet/deleteContentAndPage.csv
new file mode 100644
index 000000000000..cfaf4359d752
--- /dev/null
+++ b/typo3/sysext/workspaces/Tests/Functional/DataHandling/Regular/Modify/DataSet/deleteContentAndPage.csv
@@ -0,0 +1,16 @@
+"pages",,,,,,,,,,,,,
+,"uid","pid","sorting","deleted","t3_origuid","t3ver_wsid","t3ver_state","t3ver_stage","t3ver_oid","t3ver_move_id","title",,
+,1,0,256,0,0,0,0,0,0,0,"FunctionalTest",,
+,88,1,256,0,0,0,0,0,0,0,"DataHandlerTest",,
+,89,88,256,0,0,0,0,0,0,0,"Relations",,
+,90,88,512,0,0,0,0,0,0,0,"Target",,
+,91,-1,256,0,89,1,2,0,89,0,"Relations",,
+"tt_content",,,,,,,,,,,,,
+,"uid","pid","sorting","deleted","sys_language_uid","l18n_parent","t3_origuid","t3ver_wsid","t3ver_state","t3ver_stage","t3ver_oid","t3ver_move_id","header"
+,296,88,256,0,0,0,0,0,0,0,0,0,"Regular Element #0"
+,297,89,256,0,0,0,0,0,0,0,0,0,"Regular Element #1"
+,298,89,512,0,0,0,0,0,0,0,0,0,"Regular Element #2"
+,299,89,768,0,0,0,0,0,0,0,0,0,"Regular Element #3"
+,300,89,1024,0,1,299,299,0,0,0,0,0,"[Translate to Dansk:] Regular Element #3"
+,301,-1,512,0,0,0,298,2,2,0,298,0,"Regular Element #2"
+,302,-1,512,0,0,0,298,1,2,0,298,0,"Regular Element #2"
diff --git a/typo3/sysext/workspaces/Tests/Functional/DataHandling/Regular/Publish/ActionTest.php b/typo3/sysext/workspaces/Tests/Functional/DataHandling/Regular/Publish/ActionTest.php
index ba656ecc51c5..ba609c0d519d 100644
--- a/typo3/sysext/workspaces/Tests/Functional/DataHandling/Regular/Publish/ActionTest.php
+++ b/typo3/sysext/workspaces/Tests/Functional/DataHandling/Regular/Publish/ActionTest.php
@@ -278,6 +278,20 @@ class ActionTest extends \TYPO3\CMS\Workspaces\Tests\Functional\DataHandling\Reg
         $this->assertContains('PageNotFoundException', $response->getError());
     }
 
+    /**
+     * @test
+     * @see DataSet/Assertion/deleteContentAndPage.csv
+     */
+    public function deleteContentAndPage()
+    {
+        parent::deleteContentAndPage();
+        $this->actionService->publishRecord(self::TABLE_Page, self::VALUE_PageId);
+        $this->assertAssertionDataSet('deleteContentAndPage');
+
+        $response = $this->getFrontendResponse(self::VALUE_PageId, 0, 0, 0, false);
+        $this->assertContains('PageNotFoundException', $response->getError());
+    }
+
     /**
      * @test
      * @see DataSet/Assertion/copyPageRecord.csv
diff --git a/typo3/sysext/workspaces/Tests/Functional/DataHandling/Regular/Publish/DataSet/deleteContentAndPage.csv b/typo3/sysext/workspaces/Tests/Functional/DataHandling/Regular/Publish/DataSet/deleteContentAndPage.csv
new file mode 100644
index 000000000000..58d61185d7cf
--- /dev/null
+++ b/typo3/sysext/workspaces/Tests/Functional/DataHandling/Regular/Publish/DataSet/deleteContentAndPage.csv
@@ -0,0 +1,17 @@
+"pages",,,,,,,,,,,,,
+,"uid","pid","sorting","deleted","t3_origuid","t3ver_wsid","t3ver_state","t3ver_stage","t3ver_oid","t3ver_move_id","title",,
+,1,0,256,0,0,0,0,0,0,0,"FunctionalTest",,
+,88,1,256,0,0,0,0,0,0,0,"DataHandlerTest",,
+,89,88,1000000000,1,89,0,0,0,0,0,"Relations",,
+,90,88,512,0,0,0,0,0,0,0,"Target",,
+,91,-1,1000000000,1,0,0,0,0,89,0,"Relations",,
+"tt_content",,,,,,,,,,,,,
+,"uid","pid","sorting","deleted","sys_language_uid","l18n_parent","t3_origuid","t3ver_wsid","t3ver_state","t3ver_stage","t3ver_oid","t3ver_move_id","header"
+,296,88,256,0,0,0,0,0,0,0,0,0,"Regular Element #0"
+,297,89,1000000000,1,0,0,0,0,0,0,0,0,"Regular Element #1"
+,298,89,1000000000,1,0,0,0,0,0,0,0,0,"Regular Element #2"
+,299,89,1000000000,1,0,0,0,0,0,0,0,0,"Regular Element #3"
+,300,89,1000000000,1,1,299,299,0,0,0,0,0,"[Translate to Dansk:] Regular Element #3"
+,301,-1,512,0,0,0,298,2,2,0,298,0,"Regular Element #2"
+,302,-1,1000000000,1,0,0,298,1,2,0,298,0,"Regular Element #2"
+,303,-1,1000000000,1,1,299,300,1,2,0,300,0,"[Translate to Dansk:] Regular Element #3"
diff --git a/typo3/sysext/workspaces/Tests/Functional/DataHandling/Regular/PublishAll/ActionTest.php b/typo3/sysext/workspaces/Tests/Functional/DataHandling/Regular/PublishAll/ActionTest.php
index 6a4b0c13c761..813439d8b307 100644
--- a/typo3/sysext/workspaces/Tests/Functional/DataHandling/Regular/PublishAll/ActionTest.php
+++ b/typo3/sysext/workspaces/Tests/Functional/DataHandling/Regular/PublishAll/ActionTest.php
@@ -263,6 +263,20 @@ class ActionTest extends \TYPO3\CMS\Workspaces\Tests\Functional\DataHandling\Reg
         $this->assertContains('PageNotFoundException', $response->getError());
     }
 
+    /**
+     * @test
+     * @see DataSet/Assertion/deleteContentAndPage.csv
+     */
+    public function deleteContentAndPage()
+    {
+        parent::deleteContentAndPage();
+        $this->actionService->publishWorkspace(self::VALUE_WorkspaceId);
+        $this->assertAssertionDataSet('deleteContentAndPage');
+
+        $response = $this->getFrontendResponse(self::VALUE_PageId, 0, 0, 0, false);
+        $this->assertContains('PageNotFoundException', $response->getError());
+    }
+
     /**
      * @test
      * @see DataSet/Assertion/copyPageRecord.csv
@@ -372,10 +386,6 @@ class ActionTest extends \TYPO3\CMS\Workspaces\Tests\Functional\DataHandling\Reg
      */
     public function createPlaceholdersAndDeleteDraftParentPage()
     {
-        // @todo These two log entries could be avoided in DataHandlerHook, but are expected
-        // "[newlog()] Error: You cannot swap versions for a record you do not have access to edit!"
-        $this->expectedErrorLogEntries = 2;
-
         parent::createPlaceholdersAndDeleteDraftParentPage();
         $this->actionService->publishWorkspace(self::VALUE_WorkspaceId);
         $this->assertAssertionDataSet('createPlaceholdersAndDeleteDraftParentPage');
diff --git a/typo3/sysext/workspaces/Tests/Functional/DataHandling/Regular/PublishAll/DataSet/deleteContentAndPage.csv b/typo3/sysext/workspaces/Tests/Functional/DataHandling/Regular/PublishAll/DataSet/deleteContentAndPage.csv
new file mode 100644
index 000000000000..58d61185d7cf
--- /dev/null
+++ b/typo3/sysext/workspaces/Tests/Functional/DataHandling/Regular/PublishAll/DataSet/deleteContentAndPage.csv
@@ -0,0 +1,17 @@
+"pages",,,,,,,,,,,,,
+,"uid","pid","sorting","deleted","t3_origuid","t3ver_wsid","t3ver_state","t3ver_stage","t3ver_oid","t3ver_move_id","title",,
+,1,0,256,0,0,0,0,0,0,0,"FunctionalTest",,
+,88,1,256,0,0,0,0,0,0,0,"DataHandlerTest",,
+,89,88,1000000000,1,89,0,0,0,0,0,"Relations",,
+,90,88,512,0,0,0,0,0,0,0,"Target",,
+,91,-1,1000000000,1,0,0,0,0,89,0,"Relations",,
+"tt_content",,,,,,,,,,,,,
+,"uid","pid","sorting","deleted","sys_language_uid","l18n_parent","t3_origuid","t3ver_wsid","t3ver_state","t3ver_stage","t3ver_oid","t3ver_move_id","header"
+,296,88,256,0,0,0,0,0,0,0,0,0,"Regular Element #0"
+,297,89,1000000000,1,0,0,0,0,0,0,0,0,"Regular Element #1"
+,298,89,1000000000,1,0,0,0,0,0,0,0,0,"Regular Element #2"
+,299,89,1000000000,1,0,0,0,0,0,0,0,0,"Regular Element #3"
+,300,89,1000000000,1,1,299,299,0,0,0,0,0,"[Translate to Dansk:] Regular Element #3"
+,301,-1,512,0,0,0,298,2,2,0,298,0,"Regular Element #2"
+,302,-1,1000000000,1,0,0,298,1,2,0,298,0,"Regular Element #2"
+,303,-1,1000000000,1,1,299,300,1,2,0,300,0,"[Translate to Dansk:] Regular Element #3"
-- 
GitLab