From 0b25d0f0d9d92fdda94d8f519072ff545f0a9fe3 Mon Sep 17 00:00:00 2001
From: Alexander Schnitzler <git@alexanderschnitzler.de>
Date: Fri, 3 Mar 2023 08:31:17 +0100
Subject: [PATCH] [TASK] Streamline ObjectMonitoring- and DomainObjectInterface
 usage

ObjectMonitoringInterface is a signaling interface for extbase
internals to signal that an object state can be monitored. Its
main purpose is to tell the persistence manager if an object or
an object collection is so called dirty, i.e. if changes need to
be persisted or not.

This change lets DomainObjectInterface extend ObjectMonitoringInterface
instead of AbstractDomainObject. Abstract classes must never be external
api, they should just contain boilerplate/repetative code. With this
change extbase can perform actions based on implemented interfaces, i.e.
guaranteed functionality instead of checking object instances.

Releases: main
Resolves: #100069
Change-Id: I65548f3fe3f6a51c2b87e67c35642c431e2d7bb2
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/78012
Tested-by: Benni Mack <benni@typo3.org>
Reviewed-by: Benni Mack <benni@typo3.org>
Tested-by: core-ci <typo3@b13.com>
Tested-by: Oliver Klee <typo3-coding@oliverklee.de>
Reviewed-by: Oliver Klee <typo3-coding@oliverklee.de>
---
 Build/phpstan/phpstan-baseline.neon           | 30 -------------------
 .../DomainObject/AbstractDomainObject.php     | 14 ++++-----
 .../DomainObject/DomainObjectInterface.php    | 17 ++++++-----
 .../Classes/Mvc/Web/Routing/UriBuilder.php    |  8 ++---
 .../Classes/Persistence/Generic/Backend.php   |  4 +--
 .../Persistence/Generic/LazyLoadingProxy.php  |  5 ++--
 .../Persistence/Generic/Mapper/DataMapper.php |  5 ----
 .../Generic/Storage/Typo3DbQueryParser.php    |  3 +-
 .../Persistence/ObjectMonitoringInterface.php | 10 +++++--
 .../Classes/Persistence/ObjectStorage.php     |  8 +++--
 .../Classes/Utility/DebuggerUtility.php       |  5 ++--
 11 files changed, 40 insertions(+), 69 deletions(-)

diff --git a/Build/phpstan/phpstan-baseline.neon b/Build/phpstan/phpstan-baseline.neon
index b9c1b98485c5..7c8eb08f4a41 100644
--- a/Build/phpstan/phpstan-baseline.neon
+++ b/Build/phpstan/phpstan-baseline.neon
@@ -1640,41 +1640,11 @@ parameters:
 			count: 1
 			path: ../../typo3/sysext/extbase/Classes/Mvc/Request.php
 
-		-
-			message: "#^Call to an undefined method TYPO3\\\\CMS\\\\Extbase\\\\DomainObject\\\\DomainObjectInterface\\:\\:_isDirty\\(\\)\\.$#"
-			count: 1
-			path: ../../typo3/sysext/extbase/Classes/Persistence/Generic/Backend.php
-
-		-
-			message: "#^Call to an undefined method TYPO3\\\\CMS\\\\Extbase\\\\DomainObject\\\\DomainObjectInterface\\:\\:_memorizeCleanState\\(\\)\\.$#"
-			count: 1
-			path: ../../typo3/sysext/extbase/Classes/Persistence/Generic/Backend.php
-
-		-
-			message: "#^Method TYPO3\\\\CMS\\\\Extbase\\\\Persistence\\\\ObjectMonitoringInterface\\:\\:_isDirty\\(\\) invoked with 1 parameter, 0 required\\.$#"
-			count: 1
-			path: ../../typo3/sysext/extbase/Classes/Persistence/Generic/Backend.php
-
-		-
-			message: "#^Call to an undefined method TYPO3\\\\CMS\\\\Extbase\\\\DomainObject\\\\DomainObjectInterface\\:\\:_memorizeCleanState\\(\\)\\.$#"
-			count: 1
-			path: ../../typo3/sysext/extbase/Classes/Persistence/Generic/LazyObjectStorage.php
-
 		-
 			message: "#^Property TYPO3\\\\CMS\\\\Extbase\\\\Persistence\\\\Generic\\\\Mapper\\\\ColumnMap\\:\\:\\$relationTableWhereStatement is unused\\.$#"
 			count: 1
 			path: ../../typo3/sysext/extbase/Classes/Persistence/Generic/Mapper/ColumnMap.php
 
-		-
-			message: "#^Call to an undefined method T of TYPO3\\\\CMS\\\\Extbase\\\\DomainObject\\\\DomainObjectInterface\\:\\:_memorizeCleanState\\(\\)\\.$#"
-			count: 1
-			path: ../../typo3/sysext/extbase/Classes/Persistence/Generic/Mapper/DataMapper.php
-
-		-
-			message: "#^Call to an undefined method TYPO3\\\\CMS\\\\Extbase\\\\DomainObject\\\\DomainObjectInterface\\:\\:_hasProperty\\(\\)\\.$#"
-			count: 3
-			path: ../../typo3/sysext/extbase/Classes/Persistence/Generic/Mapper/DataMapper.php
-
 		-
 			message: "#^Call to an undefined method TYPO3\\\\CMS\\\\Extbase\\\\Persistence\\\\Generic\\\\Qom\\\\JoinConditionInterface\\:\\:getProperty1Name\\(\\)\\.$#"
 			count: 1
diff --git a/typo3/sysext/extbase/Classes/DomainObject/AbstractDomainObject.php b/typo3/sysext/extbase/Classes/DomainObject/AbstractDomainObject.php
index c90cc731c9ea..03a09eb4f274 100644
--- a/typo3/sysext/extbase/Classes/DomainObject/AbstractDomainObject.php
+++ b/typo3/sysext/extbase/Classes/DomainObject/AbstractDomainObject.php
@@ -27,7 +27,7 @@ use TYPO3\CMS\Extbase\Persistence\ObjectMonitoringInterface;
  * All Model domain objects need to inherit from either AbstractEntity or AbstractValueObject, as this provides important framework information.
  * @internal only to be used within Extbase, not part of TYPO3 Core API.
  */
-abstract class AbstractDomainObject implements DomainObjectInterface, ObjectMonitoringInterface
+abstract class AbstractDomainObject implements DomainObjectInterface
 {
     public const PROPERTY_UID = 'uid';
     public const PROPERTY_PID = 'pid';
@@ -153,11 +153,11 @@ abstract class AbstractDomainObject implements DomainObjectInterface, ObjectMoni
     /**
      * Returns the property value of the given property name. Only for internal use.
      *
-     * @param string $propertyName
+     * @param non-empty-string $propertyName
      * @return bool TRUE bool true if the property exists, FALSE if it doesn't exist or NULL in case of an error.
      * @internal
      */
-    public function _hasProperty($propertyName)
+    public function _hasProperty(string $propertyName): bool
     {
         return property_exists($this, $propertyName);
     }
@@ -176,9 +176,9 @@ abstract class AbstractDomainObject implements DomainObjectInterface, ObjectMoni
      * Register an object's clean state, e.g. after it has been reconstituted
      * from the database.
      *
-     * @param string $propertyName The name of the property to be memorized. If omitted all persistable properties are memorized.
+     * @param non-empty-string|null $propertyName The name of the property to be memorized. If omitted all persistable properties are memorized.
      */
-    public function _memorizeCleanState($propertyName = null)
+    public function _memorizeCleanState(string|null $propertyName = null): void
     {
         if ($propertyName !== null) {
             $this->_memorizePropertyCleanState($propertyName);
@@ -246,11 +246,11 @@ abstract class AbstractDomainObject implements DomainObjectInterface, ObjectMoni
     /**
      * Returns TRUE if the properties were modified after reconstitution
      *
-     * @param string $propertyName An optional name of a property to be checked if its value is dirty
+     * @param non-empty-string|null $propertyName An optional name of a property to be checked if its value is dirty
      * @throws \TYPO3\CMS\Extbase\Persistence\Generic\Exception\TooDirtyException
      * @return bool
      */
-    public function _isDirty($propertyName = null)
+    public function _isDirty(string|null $propertyName = null): bool
     {
         if ($this->uid !== null && $this->_getCleanProperty(self::PROPERTY_UID) !== null && $this->uid != $this->_getCleanProperty(self::PROPERTY_UID)) {
             throw new TooDirtyException('The ' . self::PROPERTY_UID . ' "' . $this->uid . '" has been modified, that is simply too much.', 1222871239);
diff --git a/typo3/sysext/extbase/Classes/DomainObject/DomainObjectInterface.php b/typo3/sysext/extbase/Classes/DomainObject/DomainObjectInterface.php
index 4a2403e4f936..0a5cdfae4470 100644
--- a/typo3/sysext/extbase/Classes/DomainObject/DomainObjectInterface.php
+++ b/typo3/sysext/extbase/Classes/DomainObject/DomainObjectInterface.php
@@ -17,6 +17,8 @@ declare(strict_types=1);
 
 namespace TYPO3\CMS\Extbase\DomainObject;
 
+use TYPO3\CMS\Extbase\Persistence\ObjectMonitoringInterface;
+
 /**
  * A Domain Object Interface. All domain objects which should be persisted need to implement the below interface.
  * Usually you will need to subclass \TYPO3\CMS\Extbase\DomainObject\AbstractEntity and \TYPO3\CMS\Extbase\DomainObject\AbstractValueObject
@@ -24,8 +26,10 @@ namespace TYPO3\CMS\Extbase\DomainObject;
  *
  * @see \TYPO3\CMS\Extbase\DomainObject\AbstractEntity
  * @see \TYPO3\CMS\Extbase\DomainObject\AbstractValueObject
+ *
+ * @internal only to be used within Extbase, not part of TYPO3 Core API.
  */
-interface DomainObjectInterface
+interface DomainObjectInterface extends ObjectMonitoringInterface
 {
     /**
      * Getter for uid.
@@ -48,16 +52,18 @@ interface DomainObjectInterface
 
     /**
      * Returns TRUE if the object is new (the uid was not set, yet). Only for internal use
-     *
-     * @internal
      */
     public function _isNew(): bool;
 
+    /**
+     * @param non-empty-string $propertyName
+     */
+    public function _hasProperty(string $propertyName): bool;
+
     /**
      * Reconstitutes a property. Only for internal use.
      *
      * @param mixed $value
-     * @internal
      */
     public function _setProperty(string $propertyName, $value);
 
@@ -65,7 +71,6 @@ interface DomainObjectInterface
      * Returns the property value of the given property name. Only for internal use.
      *
      * @return mixed The propertyValue
-     * @internal
      */
     public function _getProperty(string $propertyName);
 
@@ -73,7 +78,6 @@ interface DomainObjectInterface
      * Returns a hash map of property names and property values
      *
      * @return array The properties
-     * @internal
      */
     public function _getProperties(): array;
 
@@ -83,7 +87,6 @@ interface DomainObjectInterface
      *
      * @param string $propertyName The name of the property to be memorized.
      * @return mixed The clean property value or NULL
-     * @internal
      */
     public function _getCleanProperty(string $propertyName);
 }
diff --git a/typo3/sysext/extbase/Classes/Mvc/Web/Routing/UriBuilder.php b/typo3/sysext/extbase/Classes/Mvc/Web/Routing/UriBuilder.php
index 4afe07e1aea1..62a8aa94f1f1 100644
--- a/typo3/sysext/extbase/Classes/Mvc/Web/Routing/UriBuilder.php
+++ b/typo3/sysext/extbase/Classes/Mvc/Web/Routing/UriBuilder.php
@@ -25,8 +25,8 @@ use TYPO3\CMS\Core\Utility\ArrayUtility;
 use TYPO3\CMS\Core\Utility\GeneralUtility;
 use TYPO3\CMS\Core\Utility\HttpUtility;
 use TYPO3\CMS\Extbase\Configuration\ConfigurationManagerInterface;
-use TYPO3\CMS\Extbase\DomainObject\AbstractDomainObject;
 use TYPO3\CMS\Extbase\DomainObject\AbstractValueObject;
+use TYPO3\CMS\Extbase\DomainObject\DomainObjectInterface;
 use TYPO3\CMS\Extbase\Mvc\Exception\InvalidArgumentValueException;
 use TYPO3\CMS\Extbase\Mvc\ExtbaseRequestParameters;
 use TYPO3\CMS\Extbase\Mvc\Request;
@@ -743,7 +743,7 @@ class UriBuilder
             if ($argumentValue instanceof \Iterator) {
                 $argumentValue = $this->convertIteratorToArray($argumentValue);
             }
-            if ($argumentValue instanceof AbstractDomainObject) {
+            if ($argumentValue instanceof DomainObjectInterface) {
                 if ($argumentValue->getUid() !== null) {
                     $arguments[$argumentKey] = $argumentValue->getUid();
                 } elseif ($argumentValue instanceof AbstractValueObject) {
@@ -774,14 +774,14 @@ class UriBuilder
      * @todo Refactor this into convertDomainObjectsToIdentityArrays()
      * @internal only to be used within Extbase, not part of TYPO3 Core API.
      */
-    public function convertTransientObjectToArray(AbstractDomainObject $object): array
+    public function convertTransientObjectToArray(DomainObjectInterface $object): array
     {
         $result = [];
         foreach ($object->_getProperties() as $propertyName => $propertyValue) {
             if ($propertyValue instanceof \Iterator) {
                 $propertyValue = $this->convertIteratorToArray($propertyValue);
             }
-            if ($propertyValue instanceof AbstractDomainObject) {
+            if ($propertyValue instanceof DomainObjectInterface) {
                 if ($propertyValue->getUid() !== null) {
                     $result[$propertyName] = $propertyValue->getUid();
                 } else {
diff --git a/typo3/sysext/extbase/Classes/Persistence/Generic/Backend.php b/typo3/sysext/extbase/Classes/Persistence/Generic/Backend.php
index e357eb356062..85911aa29b24 100644
--- a/typo3/sysext/extbase/Classes/Persistence/Generic/Backend.php
+++ b/typo3/sysext/extbase/Classes/Persistence/Generic/Backend.php
@@ -35,7 +35,6 @@ use TYPO3\CMS\Extbase\Persistence\Exception\IllegalRelationTypeException;
 use TYPO3\CMS\Extbase\Persistence\Generic\Mapper\ColumnMap;
 use TYPO3\CMS\Extbase\Persistence\Generic\Mapper\DataMapFactory;
 use TYPO3\CMS\Extbase\Persistence\Generic\Mapper\DataMapper;
-use TYPO3\CMS\Extbase\Persistence\ObjectMonitoringInterface;
 use TYPO3\CMS\Extbase\Persistence\ObjectStorage;
 use TYPO3\CMS\Extbase\Persistence\PersistenceManagerInterface;
 use TYPO3\CMS\Extbase\Persistence\QueryInterface;
@@ -314,8 +313,7 @@ class Backend implements BackendInterface, SingletonInterface
                         $queue[] = $containedObject;
                     }
                 }
-            } elseif ($propertyValue instanceof DomainObjectInterface
-                && $object instanceof ObjectMonitoringInterface) {
+            } elseif ($propertyValue instanceof DomainObjectInterface) {
                 if ($object->_isDirty($propertyName)) {
                     if ($propertyValue->_isNew()) {
                         $this->insertObject($propertyValue, $object, $propertyName);
diff --git a/typo3/sysext/extbase/Classes/Persistence/Generic/LazyLoadingProxy.php b/typo3/sysext/extbase/Classes/Persistence/Generic/LazyLoadingProxy.php
index c810e1c2b55c..5d3835170458 100644
--- a/typo3/sysext/extbase/Classes/Persistence/Generic/LazyLoadingProxy.php
+++ b/typo3/sysext/extbase/Classes/Persistence/Generic/LazyLoadingProxy.php
@@ -16,7 +16,6 @@
 namespace TYPO3\CMS\Extbase\Persistence\Generic;
 
 use TYPO3\CMS\Core\Utility\GeneralUtility;
-use TYPO3\CMS\Extbase\DomainObject\AbstractDomainObject;
 use TYPO3\CMS\Extbase\DomainObject\DomainObjectInterface;
 use TYPO3\CMS\Extbase\Persistence\Generic\Mapper\DataMapper;
 
@@ -78,7 +77,7 @@ class LazyLoadingProxy implements \Iterator, LoadingStrategyInterface
         // this check safeguards against a proxy being activated multiple times
         // usually that does not happen, but if the proxy is held from outside
         // its parent ... the result would be weird.
-        if ($this->parentObject instanceof AbstractDomainObject
+        if ($this->parentObject instanceof DomainObjectInterface
             && $this->parentObject->_getProperty($this->propertyName) instanceof LazyLoadingProxy
             && $this->dataMapper
         ) {
@@ -133,7 +132,7 @@ class LazyLoadingProxy implements \Iterator, LoadingStrategyInterface
     {
         $realInstance = $this->_loadRealInstance();
 
-        if ($realInstance instanceof AbstractDomainObject) {
+        if ($realInstance instanceof DomainObjectInterface) {
             return $realInstance->_getProperty($propertyName);
         }
         return $realInstance->{$propertyName};
diff --git a/typo3/sysext/extbase/Classes/Persistence/Generic/Mapper/DataMapper.php b/typo3/sysext/extbase/Classes/Persistence/Generic/Mapper/DataMapper.php
index 8e75b1aa8df5..d3e0b0264529 100644
--- a/typo3/sysext/extbase/Classes/Persistence/Generic/Mapper/DataMapper.php
+++ b/typo3/sysext/extbase/Classes/Persistence/Generic/Mapper/DataMapper.php
@@ -150,11 +150,6 @@ class DataMapper
             $this->thawProperties($object, $row);
             $event = new AfterObjectThawedEvent($object, $row);
             $this->eventDispatcher->dispatch($event);
-            // @todo: technically, we cannot be sure to have an object that supports _memorizeCleanState() here
-            //        because _memorizeCleanState() is a contract method of ObjectMonitoringInterface, and not of
-            //        DomainObjectInterface. The easiest solution would be to have DomainObjectInterface extend
-            //        ObjectMonitoringInterface. That way, ObjectMonitoringInterface can also be kept for use with
-            //        the ObjectStorage class.
             $object->_memorizeCleanState();
             $this->persistenceSession->registerReconstitutedEntity($object);
         }
diff --git a/typo3/sysext/extbase/Classes/Persistence/Generic/Storage/Typo3DbQueryParser.php b/typo3/sysext/extbase/Classes/Persistence/Generic/Storage/Typo3DbQueryParser.php
index ede80d06988a..f8f1897b6d9e 100644
--- a/typo3/sysext/extbase/Classes/Persistence/Generic/Storage/Typo3DbQueryParser.php
+++ b/typo3/sysext/extbase/Classes/Persistence/Generic/Storage/Typo3DbQueryParser.php
@@ -28,6 +28,7 @@ use TYPO3\CMS\Core\Domain\Repository\PageRepository;
 use TYPO3\CMS\Core\Http\ApplicationType;
 use TYPO3\CMS\Core\Utility\GeneralUtility;
 use TYPO3\CMS\Extbase\DomainObject\AbstractDomainObject;
+use TYPO3\CMS\Extbase\DomainObject\DomainObjectInterface;
 use TYPO3\CMS\Extbase\Persistence\Generic\Exception;
 use TYPO3\CMS\Extbase\Persistence\Generic\Exception\InconsistentQuerySettingsException;
 use TYPO3\CMS\Extbase\Persistence\Generic\Exception\InvalidRelationConfigurationException;
@@ -508,7 +509,7 @@ class Typo3DbQueryParser
      */
     protected function createTypedNamedParameter($value, int $forceType = null): string
     {
-        if ($value instanceof AbstractDomainObject
+        if ($value instanceof DomainObjectInterface
             && $value->_hasProperty(AbstractDomainObject::PROPERTY_LOCALIZED_UID)
             && $value->_getProperty(AbstractDomainObject::PROPERTY_LOCALIZED_UID) > 0
         ) {
diff --git a/typo3/sysext/extbase/Classes/Persistence/ObjectMonitoringInterface.php b/typo3/sysext/extbase/Classes/Persistence/ObjectMonitoringInterface.php
index 2c114e701a69..ce1576628c7b 100644
--- a/typo3/sysext/extbase/Classes/Persistence/ObjectMonitoringInterface.php
+++ b/typo3/sysext/extbase/Classes/Persistence/ObjectMonitoringInterface.php
@@ -1,5 +1,7 @@
 <?php
 
+declare(strict_types=1);
+
 /*
  * This file is part of the TYPO3 CMS project.
  *
@@ -26,13 +28,15 @@ interface ObjectMonitoringInterface
     /**
      * Register an object's clean state, e.g. after it has been reconstituted
      * from the database
+     *
+     * @param non-empty-string|null $propertyName
      */
-    public function _memorizeCleanState();
+    public function _memorizeCleanState(string|null $propertyName = null): void;
 
     /**
      * Returns TRUE if the properties were modified after reconstitution
      *
-     * @return bool
+     * @param non-empty-string|null $propertyName
      */
-    public function _isDirty();
+    public function _isDirty(string|null $propertyName = null): bool;
 }
diff --git a/typo3/sysext/extbase/Classes/Persistence/ObjectStorage.php b/typo3/sysext/extbase/Classes/Persistence/ObjectStorage.php
index 71d8e1db10b2..807bec0a5f8f 100644
--- a/typo3/sysext/extbase/Classes/Persistence/ObjectStorage.php
+++ b/typo3/sysext/extbase/Classes/Persistence/ObjectStorage.php
@@ -331,8 +331,10 @@ class ObjectStorage implements \Countable, \Iterator, \ArrayAccess, ObjectMonito
 
     /**
      * Register the storage's clean state, e.g., after it has been reconstituted from the database.
+     *
+     * @param non-empty-string|null $propertyName
      */
-    public function _memorizeCleanState()
+    public function _memorizeCleanState(string|null $propertyName = null): void
     {
         $this->isModified = false;
     }
@@ -340,9 +342,9 @@ class ObjectStorage implements \Countable, \Iterator, \ArrayAccess, ObjectMonito
     /**
      * Returns `true` if the storage was modified after reconstitution.
      *
-     * @return bool
+     * @param non-empty-string|null $propertyName
      */
-    public function _isDirty()
+    public function _isDirty(string|null $propertyName = null): bool
     {
         return $this->isModified;
     }
diff --git a/typo3/sysext/extbase/Classes/Utility/DebuggerUtility.php b/typo3/sysext/extbase/Classes/Utility/DebuggerUtility.php
index 21889f5acba2..ab6e3d5c9b94 100644
--- a/typo3/sysext/extbase/Classes/Utility/DebuggerUtility.php
+++ b/typo3/sysext/extbase/Classes/Utility/DebuggerUtility.php
@@ -18,7 +18,6 @@ declare(strict_types=1);
 namespace TYPO3\CMS\Extbase\Utility;
 
 use TYPO3\CMS\Core\SingletonInterface;
-use TYPO3\CMS\Extbase\DomainObject\AbstractDomainObject;
 use TYPO3\CMS\Extbase\DomainObject\AbstractEntity;
 use TYPO3\CMS\Extbase\DomainObject\AbstractValueObject;
 use TYPO3\CMS\Extbase\DomainObject\DomainObjectInterface;
@@ -253,7 +252,7 @@ class DebuggerUtility
             } else {
                 $dump .= '<span class="extbase-debug-scope">' . $scope . '</span>';
             }
-            if ($object instanceof AbstractDomainObject) {
+            if ($object instanceof DomainObjectInterface) {
                 if ($object->_isDirty()) {
                     $persistenceType = 'modified';
                 } elseif ($object->_isNew()) {
@@ -430,7 +429,7 @@ class DebuggerUtility
                         continue;
                     }
                     $dump .= self::renderDump($property->getValue($object), $level, $plainText, $ansiColors);
-                    if ($object instanceof AbstractDomainObject && !$object->_isNew() && $object->_isDirty($property->getName())) {
+                    if ($object instanceof DomainObjectInterface && !$object->_isNew() && $object->_isDirty($property->getName())) {
                         if ($plainText) {
                             $dump .= ' ' . self::ansiEscapeWrap('modified', '43;30', $ansiColors);
                         } else {
-- 
GitLab