Some further logic fixes to proxy serialization

Author: kitsunet

Neos.Flow/Classes/ObjectManagement/DependencyInjection/ProxyClassBuilder.php

@@ -123,9 +123,6 @@ public function build(): void
 
             $constructor->addPreParentCallCode($constructorInjectionCode);
 
-            $sleepMethod = $proxyClass->getMethod('__sleep');
-            $sleepMethod->setDocBlock(self::AUTOGENERATED_PROXY_METHOD_COMMENT);
-
             $wakeupMethod = $proxyClass->getMethod('__wakeup');
             $wakeupMethod->setDocBlock(self::AUTOGENERATED_PROXY_METHOD_COMMENT);
 
@@ -165,7 +162,13 @@ public function build(): void
                     $proxyClass->addProperty('Flow_Persistence_RelatedEntitiesContainer', null, 'private readonly \Neos\Flow\ObjectManagement\Proxy\RelatedEntitiesContainer');
                     $constructor->addPostParentCallCode('$this->Flow_Persistence_RelatedEntitiesContainer = new \Neos\Flow\ObjectManagement\Proxy\RelatedEntitiesContainer();');
                 }
-                $sleepMethod->addPostParentCallCode($serializeRelatedEntitiesCode);
+
+                $classHasSleepMethod = $this->reflectionService->hasMethod($className, '__sleep');
+                if (!$classHasSleepMethod) {
+                    $sleepMethod = $proxyClass->getMethod('__sleep');
+                    $sleepMethod->setDocBlock(self::AUTOGENERATED_PROXY_METHOD_COMMENT);
+                    $sleepMethod->addPostParentCallCode($serializeRelatedEntitiesCode);
+                }
                 $wakeupMethod->addPreParentCallCode($this->buildSetRelatedEntitiesCode());
             }
 
@@ -224,10 +227,6 @@ protected function buildSerializeRelatedEntitiesCode(Configuration $objectConfig
         /** @var class-string $className */
         $className = $objectConfiguration->getClassName();
         $forceSerializationCode = $forceSerializationCode === false ? ($this->reflectionService->getClassAnnotation($className, Flow\Proxy::class)?->forceSerializationCode ?? false) : true;
-        if (!$forceSerializationCode && $this->reflectionService->hasMethod($className, '__sleep')) {
-            return '';
-        }
-
         $scopeAnnotation = $this->reflectionService->getClassAnnotation($className, Flow\Scope::class);
         $transientProperties = $this->reflectionService->getPropertyNamesByAnnotation($className, Flow\Transient::class);
         $injectedProperties = $this->reflectionService->getPropertyNamesByAnnotation($className, Flow\Inject::class);

Neos.Flow/Classes/ObjectManagement/Proxy/ObjectSerializationTrait.php

@@ -84,7 +84,12 @@ private function Flow_serializeRelatedEntities(array $transientProperties, array
                     }
                     continue;
                 }
-                if ($className !== false && (Bootstrap::$staticObjectManager->getScope($className) === Configuration::SCOPE_SINGLETON || $className === DependencyProxy::class)) {
+                if ($className !== false &&
+                    (
+                        Bootstrap::$staticObjectManager->getScope($className) === Configuration::SCOPE_SINGLETON
+                        || Bootstrap::$staticObjectManager->getScope($className) === Configuration::SCOPE_SESSION
+                        || $className === DependencyProxy::class
+                    )) {
                     continue;
                 }
             }
@@ -114,7 +119,18 @@ private function Flow_searchForEntitiesAndStoreIdentifierArray(string $path, mix
                 throw new \RuntimeException(sprintf('The class "%s" has an entity reference Flow could not detect, please add a Flow\\Proxy annotation with "forceSerializationCode" set to "true".', 1756936954));
             }
             $this->Flow_Persistence_RelatedEntitiesContainer->appendRelatedEntity($originalPropertyName, $path, $propertyValue);
-            $this->$originalPropertyName = Arrays::setValueByPath($this->$originalPropertyName, $path, null);
+            /**
+             * The idea of setting to null here is to prevent serialization after we found an entity, BUT this logic
+             * is heavily flawed in today's PHP world. Type hinting might make null an invalid value. Also
+             * Arrays::setValueByPath() only works on "Array-like" not on objects, therefore
+             * we don't handle direct properties of $this (path empty string) at all here.
+             * They are skipped for serialization in Flow_serializeRelatedEntities so we don't need to unset.
+             * This still leaves the option of types going awry somewhere, but at the moment there
+             * isn't really a better solution at hand and the case should be super rare.
+             */
+            if ($path !== '') {
+                $this->$originalPropertyName = Arrays::setValueByPath($this->$originalPropertyName, $path, null);
+            }
             $foundEntity = true;
         }
 
@@ -132,11 +148,11 @@ private function Flow_setRelatedEntities(): void
         if (isset($this->Flow_Persistence_RelatedEntitiesContainer)) {
             $persistenceManager = Bootstrap::$staticObjectManager->get(PersistenceManagerInterface::class);
             foreach ($this->Flow_Persistence_RelatedEntitiesContainer as $entityInformation) {
-                $entity = $persistenceManager->getObjectByIdentifier($entityInformation['identifier'], $entityInformation['entityType'], true);
-                if (isset($entityInformation['entityPath'])) {
-                    $this->{$entityInformation['propertyName']} = Arrays::setValueByPath($this->{$entityInformation['propertyName']}, $entityInformation['entityPath'], $entity);
+                $entity = $persistenceManager->getObjectByIdentifier($entityInformation['i'], $entityInformation['c'], true);
+                if (isset($entityInformation['p'])) {
+                    $this->{$entityInformation['n']} = Arrays::setValueByPath($this->{$entityInformation['n']}, $entityInformation['p'], $entity);
                 } else {
-                    $this->{$entityInformation['propertyName']} = $entity;
+                    $this->{$entityInformation['n']} = $entity;
                 }
             }
 

Neos.Flow/Classes/ObjectManagement/Proxy/RelatedEntitiesContainer.php

@@ -16,18 +16,18 @@
 #[Flow\Proxy(false)]
 final class RelatedEntitiesContainer implements \IteratorAggregate
 {
-    protected array $relatedEntities = [];
+    protected array $e = [];
 
     public function getIterator(): \Generator
     {
-        foreach ($this->relatedEntities as $entityInformation) {
+        foreach ($this->e as $entityInformation) {
             yield $entityInformation;
         }
     }
 
     public function reset(): void
     {
-        $this->relatedEntities = [];
+        $this->e = [];
     }
 
     public function appendRelatedEntity(string $originalPropertyName, string $path, object $propertyValue): void
@@ -42,11 +42,11 @@ public function appendRelatedEntity(string $originalPropertyName, string $path,
             $identifier = current(ObjectAccess::getProperty($propertyValue, '_identifier', true));
         }
 
-        $this->relatedEntities[$originalPropertyName . '.' . $path] = [
-            'propertyName' => $originalPropertyName,
-            'entityType' => $className,
-            'identifier' => $identifier,
-            'entityPath' => $path
+        $this->e[$originalPropertyName . '.' . $path] = [
+            'n' => $originalPropertyName,
+            'c' => $className,
+            'i' => $identifier,
+            'p' => $path
         ];
     }
 }