From 513c42ec58b66f7ebd9dd6798553295915d691ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Mu=CC=88ller?= Date: Mon, 8 Sep 2025 22:29:25 +0200 Subject: [PATCH 1/9] BUGFIX: Correctly check advices in parent's parent classes Props to @lorenzulrich for this correct fix, which always gets the parent in relation to the method being called and not the parent of the instance at hand. Fixes: #3406 --- Neos.Flow/Classes/Aop/Builder/ProxyClassBuilder.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Neos.Flow/Classes/Aop/Builder/ProxyClassBuilder.php b/Neos.Flow/Classes/Aop/Builder/ProxyClassBuilder.php index a175e0cf8d..22a4231644 100644 --- a/Neos.Flow/Classes/Aop/Builder/ProxyClassBuilder.php +++ b/Neos.Flow/Classes/Aop/Builder/ProxyClassBuilder.php @@ -409,7 +409,7 @@ public function buildProxyClass(string $targetClassName, array $aspectContainers $proxyClass->addProperty($propertyName, var_export($propertyIntroduction->getInitialValue(), true), $propertyIntroduction->getPropertyVisibility(), $propertyIntroduction->getPropertyDocComment()); } - $proxyClass->getMethod('Flow_Aop_Proxy_buildMethodsAndAdvicesArray')->addPreParentCallCode(" if (method_exists(get_parent_class(\$this), 'Flow_Aop_Proxy_buildMethodsAndAdvicesArray') && is_callable([parent::class, 'Flow_Aop_Proxy_buildMethodsAndAdvicesArray'])) parent::Flow_Aop_Proxy_buildMethodsAndAdvicesArray();\n"); + $proxyClass->getMethod('Flow_Aop_Proxy_buildMethodsAndAdvicesArray')->addPreParentCallCode(" if (method_exists(parent::class, 'Flow_Aop_Proxy_buildMethodsAndAdvicesArray') && is_callable([parent::class, 'Flow_Aop_Proxy_buildMethodsAndAdvicesArray'])) parent::Flow_Aop_Proxy_buildMethodsAndAdvicesArray();\n"); $proxyClass->getMethod('Flow_Aop_Proxy_buildMethodsAndAdvicesArray')->addPreParentCallCode($this->buildMethodsAndAdvicesArrayCode($interceptedMethods)); $proxyClass->getMethod('Flow_Aop_Proxy_buildMethodsAndAdvicesArray')->setVisibility(ProxyMethodGenerator::VISIBILITY_PROTECTED); @@ -424,6 +424,7 @@ public function buildProxyClass(string $targetClassName, array $aspectContainers PHP); } $proxyClass->addTraits(['\\' . AdvicesTrait::class]); + $proxyClass->addInterfaces(['\\' . Aop\ProxyInterface::class]); $this->buildMethodsInterceptorCode($targetClassName, $interceptedMethods); From d058ac6029b47f81d714eb2c0a82554575710218 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Mu=CC=88ller?= Date: Mon, 8 Sep 2025 22:31:12 +0200 Subject: [PATCH 2/9] BUGFIX:ProxyConstructor from reflection This allows classes without constructor injection to have a proper constructor signature. --- .../Proxy/ProxyConstructorGenerator.php | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/Neos.Flow/Classes/ObjectManagement/Proxy/ProxyConstructorGenerator.php b/Neos.Flow/Classes/ObjectManagement/Proxy/ProxyConstructorGenerator.php index 4697909129..c6405db34b 100644 --- a/Neos.Flow/Classes/ObjectManagement/Proxy/ProxyConstructorGenerator.php +++ b/Neos.Flow/Classes/ObjectManagement/Proxy/ProxyConstructorGenerator.php @@ -12,6 +12,8 @@ */ use Laminas\Code\Generator\DocBlockGenerator; +use Laminas\Code\Generator\ParameterGenerator; +use Laminas\Code\Generator\PromotedParameterGenerator; use Neos\Flow\ObjectManagement\DependencyInjection\ProxyClassBuilder; final class ProxyConstructorGenerator extends ProxyMethodGenerator @@ -30,7 +32,7 @@ public function __construct($name = null, array $parameters = [], $flags = self: parent::__construct('__construct', $parameters, $flags, $body, $docBlock); } - public static function fromReflection(\Laminas\Code\Reflection\MethodReflection $reflectionMethod): static + public static function fromReflection(\Laminas\Code\Reflection\MethodReflection $reflectionMethod, bool $withOriginalArgumentSignature = false): static { $method = new static('__construct'); $declaringClass = $reflectionMethod->getDeclaringClass(); @@ -56,6 +58,17 @@ public static function fromReflection(\Laminas\Code\Reflection\MethodReflection $docBlock->setWordWrap(false); $docBlock->setSourceDirty(false); $method->setDocBlock($docBlock); + + if ($withOriginalArgumentSignature) { + foreach ($reflectionMethod->getParameters() as $reflectionParameter) { + $method->setParameter( + $reflectionParameter->isPromoted() + ? PromotedParameterGenerator::fromReflection($reflectionParameter) + : ParameterGenerator::fromReflection($reflectionParameter) + ); + } + } + return $method; } From 27d42630286be56c7b0ae7fddd97b1ccd9f22a18 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Mu=CC=88ller?= Date: Tue, 9 Sep 2025 10:00:59 +0200 Subject: [PATCH 3/9] BUGFIX: ObjectSerialization proxy fixes This is an overhaul of how object serialization is prepared in proxies. Proxies can skip object serialization code if there is nothing to serialize, that is, if there are no entity properties, no injected, or transient properties. We were too eager prior to this patch with not using the serialization code, the checks are now way more detailed. Additionally the "Proxy" Annotation now allows to force serialization code for a class if the checks still fail to detect correctly, this should be rarely needed. This fix however broke some code in Neos that should have gotten the serialization code previously but didn't. Since the class in question is readonly, injecting a mutable property via trait resulted in PHP errors. Therefore we now use a mutable object to hold related entities for serialization purposes which is declared readonly in the proxy to avoid errors with readonly classes should they need serialization code. Other mutable properties were removed as they are not strictly needed. We should do the same refactoring for AOP as well. Proxies can use the original constructor argument signature if no constructor injection is used. Finally prototype autowiring is now a choice via setting, currently default enabled to not change behavior, in the future we should plan a breaking change to disable it and then remove the option altogether. Fixes: #3493 Related: #3212 Related: #3076 --- Neos.Flow/Classes/Annotations/Proxy.php | 26 +++- .../DependencyInjection/ProxyClassBuilder.php | 145 ++++++++++++++---- .../Proxy/ObjectSerializationTrait.php | 71 +++------ .../ObjectManagement/Proxy/ProxyClass.php | 19 ++- .../Proxy/RelatedEntitiesContainer.php | 51 ++++++ Neos.Flow/Configuration/Objects.yaml | 2 + Neos.Flow/Configuration/Settings.Object.yaml | 10 ++ 7 files changed, 245 insertions(+), 79 deletions(-) create mode 100644 Neos.Flow/Classes/ObjectManagement/Proxy/RelatedEntitiesContainer.php diff --git a/Neos.Flow/Classes/Annotations/Proxy.php b/Neos.Flow/Classes/Annotations/Proxy.php index e7e709d811..0f049053a9 100644 --- a/Neos.Flow/Classes/Annotations/Proxy.php +++ b/Neos.Flow/Classes/Annotations/Proxy.php @@ -12,6 +12,7 @@ */ use Doctrine\Common\Annotations\Annotation\NamedArgumentConstructor; +use Neos\Flow\ObjectManagement\Exception\ProxyCompilerException; /** * Used to disable proxy building for an object. @@ -32,8 +33,31 @@ final class Proxy */ public $enabled = true; - public function __construct(bool $enabled = true) + /** + * Whether you need serialization code build in the proxy, this might be needed if you + * build a PHP object you need to serialize that includes entities for example, as in that + * case the entities should be converted to metadata (class & persistence identifier) before + * serialization. + * The serialization code also removes injected/otherwise internal framework properties + * introduced by the proxy building but these situations should be correctly detected by + * proxy building and create the serialization code anyway so you should never have to + * set this for these cases and it would be a bug if you have to. + * + * At this point it wouldn't make much sense to allow a forced disabling of the serialization + * code as that would most certainly run into problems if there was AOP, injections or other reasons. + * Rather disable the proxy completely then. + * + * @var bool + */ + public $forceSerializationCode = false; + + public function __construct(bool $enabled = true, bool $forceSerializationCode = false) { + if ($enabled === false && $forceSerializationCode === true) { + throw new ProxyCompilerException('Cannot disable a Proxy but forceSerializationCode at the same time.', 1756813222); + } + $this->enabled = $enabled; + $this->forceSerializationCode = $forceSerializationCode; } } diff --git a/Neos.Flow/Classes/ObjectManagement/DependencyInjection/ProxyClassBuilder.php b/Neos.Flow/Classes/ObjectManagement/DependencyInjection/ProxyClassBuilder.php index 62165d5fcf..5db37a34de 100644 --- a/Neos.Flow/Classes/ObjectManagement/DependencyInjection/ProxyClassBuilder.php +++ b/Neos.Flow/Classes/ObjectManagement/DependencyInjection/ProxyClassBuilder.php @@ -12,6 +12,7 @@ */ use Neos\Flow\Annotations as Flow; +use Neos\Flow\Aop\ProxyInterface as AopProxyInterface; use Neos\Flow\Cache\CacheManager; use Neos\Flow\Configuration\ConfigurationManager; use Neos\Flow\Configuration\Exception\InvalidConfigurationTypeException; @@ -31,6 +32,7 @@ use Neos\Flow\Reflection\MethodReflection; use Neos\Flow\Reflection\ReflectionService; use Neos\Utility\Arrays; +use Neos\Utility\TypeHandling; use Psr\Log\LoggerInterface; /** @@ -113,9 +115,13 @@ public function build(): void } $this->logger->debug(sprintf('Building dependency injection proxy for "%s"', $className), LogEnvironment::fromMethodName(__METHOD__)); - $constructor = $proxyClass->getConstructor(); + $constructorInjectionCode = $this->buildConstructorInjectionCode($objectConfiguration); + $injectionCodeWasIntroduced = $constructorInjectionCode !== ''; + + $constructor = $proxyClass->getConstructor($injectionCodeWasIntroduced === false); $constructor->addPreParentCallCode($this->buildSetInstanceCode($objectConfiguration)); - $constructor->addPreParentCallCode($this->buildConstructorInjectionCode($objectConfiguration)); + + $constructor->addPreParentCallCode($constructorInjectionCode); $sleepMethod = $proxyClass->getMethod('__sleep'); $sleepMethod->setDocBlock(self::AUTOGENERATED_PROXY_METHOD_COMMENT); @@ -123,21 +129,15 @@ public function build(): void $wakeupMethod = $proxyClass->getMethod('__wakeup'); $wakeupMethod->setDocBlock(self::AUTOGENERATED_PROXY_METHOD_COMMENT); + $wakeupMethod->setReturnType('void'); $wakeupMethod->addPreParentCallCode($this->buildSetInstanceCode($objectConfiguration)); - - $serializeRelatedEntitiesCode = $this->buildSerializeRelatedEntitiesCode($objectConfiguration); - if ($serializeRelatedEntitiesCode !== '') { - $proxyClass->addTraits(['\\' . ObjectSerializationTrait::class]); - $sleepMethod->addPostParentCallCode($serializeRelatedEntitiesCode); - $wakeupMethod->addPreParentCallCode($this->buildSetRelatedEntitiesCode()); - } - $wakeupMethod->addPostParentCallCode($this->buildLifecycleInitializationCode($objectConfiguration, ObjectManagerInterface::INITIALIZATIONCAUSE_RECREATED)); $wakeupMethod->addPostParentCallCode($this->buildLifecycleShutdownCode($objectConfiguration, ObjectManagerInterface::INITIALIZATIONCAUSE_RECREATED)); $injectPropertiesCode = $this->buildPropertyInjectionCode($objectConfiguration); if ($injectPropertiesCode !== '') { + $injectionCodeWasIntroduced = true; $proxyClass->addTraits(['\\' . PropertyInjectionTrait::class]); $injectPropertiesMethod = $proxyClass->getMethod('Flow_Proxy_injectProperties'); $injectPropertiesMethod->addPreParentCallCode($injectPropertiesCode); @@ -158,6 +158,19 @@ public function build(): void ); } + $couldHaveEntityRelations = $this->couldHaveEntityRelations($objectConfiguration); + $isProxyWithAop = in_array('\\' . AopProxyInterface::class, $proxyClass->getInterfaces()); + $serializeRelatedEntitiesCode = $this->buildSerializeRelatedEntitiesCode($objectConfiguration, $isProxyWithAop || $injectionCodeWasIntroduced); + if ($serializeRelatedEntitiesCode !== '') { + $proxyClass->addTraits(['\\' . ObjectSerializationTrait::class]); + if ($couldHaveEntityRelations) { + $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); + $wakeupMethod->addPreParentCallCode($this->buildSetRelatedEntitiesCode()); + } + $constructor->addPostParentCallCode($this->buildLifecycleInitializationCode($objectConfiguration, ObjectManagerInterface::INITIALIZATIONCAUSE_CREATED)); $constructor->addPostParentCallCode($this->buildLifecycleShutdownCode($objectConfiguration, ObjectManagerInterface::INITIALIZATIONCAUSE_CREATED)); @@ -208,11 +221,12 @@ protected function buildSetInstanceCode(Configuration $objectConfiguration): str * NOTE: Even though the method name suggests that it is only dealing with related entities code, it is currently also * used for removing injected properties before serialization. This should be refactored in the future. */ - protected function buildSerializeRelatedEntitiesCode(Configuration $objectConfiguration): string + protected function buildSerializeRelatedEntitiesCode(Configuration $objectConfiguration, bool $forceSerializationCode): string { + /** @var class-string $className */ $className = $objectConfiguration->getClassName(); - - if ($this->reflectionService->hasMethod($className, '__sleep')) { + $forceSerializationCode = $forceSerializationCode === false ? ($this->reflectionService->getClassAnnotation($className, Flow\Proxy::class)?->forceSerializationCode ?? false) : true; + if (!$forceSerializationCode && $this->reflectionService->hasMethod($className, '__sleep')) { return ''; } @@ -225,7 +239,7 @@ protected function buildSerializeRelatedEntitiesCode(Configuration $objectConfig $doBuildCode = $doBuildCode || (count($injectedProperties) > 0); $doBuildCode = $doBuildCode || ($scopeAnnotation->value ?? 'prototype') === 'session'; - if ($doBuildCode === false) { + if (!$forceSerializationCode && $doBuildCode === false) { return ''; } @@ -235,7 +249,7 @@ protected function buildSerializeRelatedEntitiesCode(Configuration $objectConfig $propertyVarTags[$propertyName] = $varTagValues[0] ?? null; } - if (count($transientProperties) === 0 && count($propertyVarTags) === 0) { + if (!$forceSerializationCode && count($transientProperties) === 0 && count($propertyVarTags) === 0) { return ''; } @@ -246,8 +260,6 @@ protected function buildSerializeRelatedEntitiesCode(Configuration $objectConfig var_export($propertyVarTags, true) ], <<<'PHP' - $this->Flow_Object_PropertiesToSerialize = []; - $this->Flow_Persistence_RelatedEntities = null; $result = $this->Flow_serializeRelatedEntities({{transientPropertiesArrayCode}}, {{propertyVarTagsArrayCode}}); PHP ); @@ -255,7 +267,7 @@ protected function buildSerializeRelatedEntitiesCode(Configuration $objectConfig protected function buildSetRelatedEntitiesCode(): string { - return "\n " . '$this->Flow_setRelatedEntities();' . "\n"; + return "\n" . '$this->Flow_setRelatedEntities();' . "\n"; } /** @@ -313,9 +325,20 @@ protected function buildConstructorInjectionCode(Configuration $objectConfigurat $settings = $this->configurationManager->getConfiguration(ConfigurationManager::CONFIGURATION_TYPE_SETTINGS, array_shift($settingPath)); $argumentValue = Arrays::getValueByPath($settings, $settingPath); } - if (!isset($this->objectConfigurations[$argumentValue])) { + $argumentObjectConfiguration = $this->objectConfigurations[$argumentValue] ?? null; + if ($argumentObjectConfiguration === null) { throw new UnknownObjectException('The object "' . $argumentValue . '" which was specified as an argument in the object configuration of object "' . $objectConfiguration->getObjectName() . '" does not exist.', 1264669967); } + + $prototypeAutowiring = $this->configurationManager->getConfiguration(ConfigurationManager::CONFIGURATION_TYPE_SETTINGS, 'Neos.Flow.object.dependencyInjection.prototypeAutowiring') ?? true; + if ($prototypeAutowiring === false && $argumentObjectConfiguration->getScope() === Configuration::SCOPE_PROTOTYPE) { + break; + } + + if ($argumentObjectConfiguration->getScope() === Configuration::SCOPE_PROTOTYPE) { + $this->logger->debug(sprintf('Class "%s" will get prototype "%s" injected via constructor, this is deprecated.', $objectConfiguration->getClassName(), $argumentObjectConfiguration->getClassName())); + } + $assignments[$argumentPosition] = $assignmentPrologue . '\Neos\Flow\Core\Bootstrap::$staticObjectManager->get(\'' . $argumentValue . '\')'; $doReturnCode = true; } @@ -340,6 +363,10 @@ protected function buildConstructorInjectionCode(Configuration $objectConfigurat unset($assignments[$argumentCounter]); } + if ($argumentCounter < 0) { + $doReturnCode = false; + } + $code = $argumentCounter >= 0 ? "\n" . implode(";\n", $assignments) . ";\n" : ''; $index = 0; @@ -610,17 +637,17 @@ protected function buildLifecycleInitializationCode(Configuration $objectConfigu return ''; } $className = $objectConfiguration->getClassName(); - $code = "\n" . ' $isSameClass = get_class($this) === \'' . $className . '\';'; + $code = "\n" . '$isSameClass = get_class($this) === \'' . $className . '\';'; if ($cause === ObjectManagerInterface::INITIALIZATIONCAUSE_RECREATED) { - $code .= "\n" . ' $classParents = class_parents($this);'; - $code .= "\n" . ' $classImplements = class_implements($this);'; - $code .= "\n" . ' $isClassProxy = array_search(\'' . $className . '\', $classParents) !== false && array_search(\'Doctrine\Persistence\Proxy\', $classImplements) !== false;' . "\n"; - $code .= "\n" . ' if ($isSameClass || $isClassProxy) {' . "\n"; + $code .= "\n" . '$classParents = class_parents($this);'; + $code .= "\n" . '$classImplements = class_implements($this);'; + $code .= "\n" . '$isClassProxy = array_search(\'' . $className . '\', $classParents) !== false && array_search(\'Doctrine\Persistence\Proxy\', $classImplements) !== false;' . "\n"; + $code .= "\n" . 'if ($isSameClass || $isClassProxy) {' . "\n"; } else { - $code .= "\n" . ' if ($isSameClass) {' . "\n"; + $code .= "\n" . 'if ($isSameClass) {' . "\n"; } - $code .= ' $this->' . $lifecycleInitializationMethodName . '(' . $cause . ');' . "\n"; - $code .= ' }' . "\n"; + $code .= ' $this->' . $lifecycleInitializationMethodName . '(' . $cause . ');' . "\n"; + $code .= '}' . "\n"; return $code; } @@ -739,4 +766,68 @@ protected function compileStaticMethods(string $className, ProxyClass $proxyClas $compiledMethod->setBody('return ' . $compiledResult . ';'); } } + + protected function couldHaveEntityRelations(Configuration $objectConfiguration): bool + { + $result = false; + /** @var class-string $className */ + $className = $objectConfiguration->getClassName(); + $classPropertyNames = $this->reflectionService->getClassPropertyNames($className); + foreach ($classPropertyNames as $propertyName) { + if ( + $this->reflectionService->isPropertyAnnotatedWith($className, $propertyName, Flow\Transient::class) || + $this->reflectionService->isPropertyAnnotatedWith($className, $propertyName, Flow\Inject::class) + ) { + continue; + } + $propertyType = $this->reflectionService->getPropertyType($className, $propertyName); + if ($propertyType === null) { + $propertyType = $this->reflectionService->getPropertyTagValues($className, $propertyName, 'var'); + } + if (isset($propertyType[0])) { + $propertyType = $propertyType[0]; + } + if ($propertyType === null) { + continue; + } + + try { + $typeInformation = TypeHandling::parseType($propertyType); + } catch (\Throwable $throwable) { + // we will skip on unparsable types + continue; + } + if ( + TypeHandling::isSimpleType($typeInformation['type']) || + ($typeInformation['elementType'] !== null && TypeHandling::isSimpleType($typeInformation['elementType'])) + ) { + continue; + } + + if ($typeInformation['type'] === \Closure::class) { + continue; + } + + if (!class_exists($typeInformation['type'])) { + continue; + } + + $isEntity = $this->reflectionService->getClassAnnotation($typeInformation['type'], Flow\Entity::class); + if (!$isEntity) { + continue; + } + + foreach ($objectConfiguration->getProperties() as $propertyConfiguration) { + if ($propertyConfiguration->getName() === $propertyName) { + continue 2; + } + } + + $this->logger->info('Class ' . $className . ' has property ' . $propertyName . ' that might be entity!'); + + $result = true; + } + + return $result; + } } diff --git a/Neos.Flow/Classes/ObjectManagement/Proxy/ObjectSerializationTrait.php b/Neos.Flow/Classes/ObjectManagement/Proxy/ObjectSerializationTrait.php index 06e5607701..d0d43a0daa 100644 --- a/Neos.Flow/Classes/ObjectManagement/Proxy/ObjectSerializationTrait.php +++ b/Neos.Flow/Classes/ObjectManagement/Proxy/ObjectSerializationTrait.php @@ -27,20 +27,16 @@ */ trait ObjectSerializationTrait { - protected array $Flow_Object_PropertiesToSerialize = []; - - protected ?array $Flow_Persistence_RelatedEntities = null; - /** * Code to find and serialize entities on sleep * * @param array $transientProperties * @param array $propertyVarTags * @return array - * @throws PropertyNotAccessibleException */ private function Flow_serializeRelatedEntities(array $transientProperties, array $propertyVarTags): array { + $propertiesToSerialize = []; $reflectedClass = new \ReflectionClass(__CLASS__); $allReflectedProperties = $reflectedClass->getProperties(); foreach ($allReflectedProperties as $reflectionProperty) { @@ -49,8 +45,7 @@ private function Flow_serializeRelatedEntities(array $transientProperties, array 'Flow_Aop_Proxy_targetMethodsAndGroupedAdvices', 'Flow_Aop_Proxy_groupedAdviceChains', 'Flow_Aop_Proxy_methodIsInAdviceMode', - 'Flow_Persistence_RelatedEntities', - 'Flow_Object_PropertiesToSerialize', + 'Flow_Persistence_RelatedEntitiesContainer', 'Flow_Injected_Properties', ])) { continue; @@ -64,7 +59,10 @@ private function Flow_serializeRelatedEntities(array $transientProperties, array if (is_array($this->$propertyName) || ($this->$propertyName instanceof \ArrayObject || $this->$propertyName instanceof \SplObjectStorage || $this->$propertyName instanceof Collection)) { if (count($this->$propertyName) > 0) { foreach ($this->$propertyName as $key => $value) { - $this->Flow_searchForEntitiesAndStoreIdentifierArray((string)$key, $value, $propertyName); + $entityWasFound = $this->Flow_searchForEntitiesAndStoreIdentifierArray((string)$key, $value, $propertyName); + if ($entityWasFound) { + $propertiesToSerialize[] = 'Flow_Persistence_RelatedEntitiesContainer'; + } } } } @@ -82,29 +80,20 @@ private function Flow_serializeRelatedEntities(array $transientProperties, array } } if ($this->$propertyName instanceof DoctrineProxy || ($this->$propertyName instanceof PersistenceMagicInterface && !Bootstrap::$staticObjectManager->get(PersistenceManagerInterface::class)->isNewObject($this->$propertyName))) { - if (!isset($this->Flow_Persistence_RelatedEntities) || !is_array($this->Flow_Persistence_RelatedEntities)) { - $this->Flow_Persistence_RelatedEntities = []; - $this->Flow_Object_PropertiesToSerialize[] = 'Flow_Persistence_RelatedEntities'; + $entityWasFound = $this->Flow_searchForEntitiesAndStoreIdentifierArray('', $this->$propertyName, $propertyName); + if ($entityWasFound) { + $propertiesToSerialize[] = 'Flow_Persistence_RelatedEntitiesContainer'; } - $identifier = Bootstrap::$staticObjectManager->get(PersistenceManagerInterface::class)->getIdentifierByObject($this->$propertyName); - if (!$identifier && $this->$propertyName instanceof DoctrineProxy) { - $identifier = current(ObjectAccess::getProperty($this->$propertyName, '_identifier', true)); - } - $this->Flow_Persistence_RelatedEntities[$propertyName] = [ - 'propertyName' => $propertyName, - 'entityType' => $className, - 'identifier' => $identifier - ]; continue; } if ($className !== false && (Bootstrap::$staticObjectManager->getScope($className) === Configuration::SCOPE_SINGLETON || $className === DependencyProxy::class)) { continue; } } - $this->Flow_Object_PropertiesToSerialize[] = $propertyName; + $propertiesToSerialize[] = $propertyName; } - return $this->Flow_Object_PropertiesToSerialize; + return $propertiesToSerialize; } /** @@ -113,36 +102,25 @@ private function Flow_serializeRelatedEntities(array $transientProperties, array * @param string $path * @param mixed $propertyValue * @param string $originalPropertyName - * @return void + * @return bool if an entity was found */ - private function Flow_searchForEntitiesAndStoreIdentifierArray(string $path, mixed $propertyValue, string $originalPropertyName): void + private function Flow_searchForEntitiesAndStoreIdentifierArray(string $path, mixed $propertyValue, string $originalPropertyName): bool { + $foundEntity = false; if (is_array($propertyValue) || ($propertyValue instanceof \ArrayObject || $propertyValue instanceof \SplObjectStorage)) { foreach ($propertyValue as $key => $value) { - $this->Flow_searchForEntitiesAndStoreIdentifierArray($path . '.' . $key, $value, $originalPropertyName); + $foundEntity = $foundEntity || $this->Flow_searchForEntitiesAndStoreIdentifierArray($path . '.' . $key, $value, $originalPropertyName); } } elseif ($propertyValue instanceof DoctrineProxy || ($propertyValue instanceof PersistenceMagicInterface && !Bootstrap::$staticObjectManager->get(PersistenceManagerInterface::class)->isNewObject($propertyValue))) { - if (!isset($this->Flow_Persistence_RelatedEntities) || !is_array($this->Flow_Persistence_RelatedEntities)) { - $this->Flow_Persistence_RelatedEntities = []; - $this->Flow_Object_PropertiesToSerialize[] = 'Flow_Persistence_RelatedEntities'; + if (!isset($this->Flow_Persistence_RelatedEntitiesContainer)) { + 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)); } - if ($propertyValue instanceof DoctrineProxy) { - $className = get_parent_class($propertyValue); - } else { - $className = Bootstrap::$staticObjectManager->getObjectNameByClassName(get_class($propertyValue)); - } - $identifier = Bootstrap::$staticObjectManager->get(PersistenceManagerInterface::class)->getIdentifierByObject($propertyValue); - if (!$identifier && $propertyValue instanceof DoctrineProxy) { - $identifier = current(ObjectAccess::getProperty($propertyValue, '_identifier', true)); - } - $this->Flow_Persistence_RelatedEntities[$originalPropertyName . '.' . $path] = [ - 'propertyName' => $originalPropertyName, - 'entityType' => $className, - 'identifier' => $identifier, - 'entityPath' => $path - ]; + $this->Flow_Persistence_RelatedEntitiesContainer->appendRelatedEntity($originalPropertyName, $path, $propertyValue); $this->$originalPropertyName = Arrays::setValueByPath($this->$originalPropertyName, $path, null); + $foundEntity = true; } + + return $foundEntity; } /** @@ -153,9 +131,9 @@ private function Flow_searchForEntitiesAndStoreIdentifierArray(string $path, mix */ private function Flow_setRelatedEntities(): void { - if (isset($this->Flow_Persistence_RelatedEntities)) { + if (isset($this->Flow_Persistence_RelatedEntitiesContainer)) { $persistenceManager = Bootstrap::$staticObjectManager->get(PersistenceManagerInterface::class); - foreach ($this->Flow_Persistence_RelatedEntities as $entityInformation) { + 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); @@ -163,7 +141,8 @@ private function Flow_setRelatedEntities(): void $this->{$entityInformation['propertyName']} = $entity; } } - unset($this->Flow_Persistence_RelatedEntities); + + isset($this->Flow_Persistence_RelatedEntitiesContainer) && $this->Flow_Persistence_RelatedEntitiesContainer->reset(); } } } diff --git a/Neos.Flow/Classes/ObjectManagement/Proxy/ProxyClass.php b/Neos.Flow/Classes/ObjectManagement/Proxy/ProxyClass.php index d414df4d4d..0fd1d0efaf 100644 --- a/Neos.Flow/Classes/ObjectManagement/Proxy/ProxyClass.php +++ b/Neos.Flow/Classes/ObjectManagement/Proxy/ProxyClass.php @@ -115,11 +115,11 @@ public function injectReflectionService(ReflectionService $reflectionService): v * @throws \ReflectionException * @throws CannotBuildObjectException */ - public function getConstructor(): ProxyConstructorGenerator + public function getConstructor(bool $withOriginalArgumentSignature = false): ProxyConstructorGenerator { if (!isset($this->constructor)) { if (method_exists($this->fullOriginalClassName, '__construct')) { - $this->constructor = ProxyConstructorGenerator::fromReflection(new MethodReflection($this->fullOriginalClassName, '__construct')); + $this->constructor = ProxyConstructorGenerator::fromReflection(new MethodReflection($this->fullOriginalClassName, '__construct'), $withOriginalArgumentSignature); } else { $this->constructor = new ProxyConstructorGenerator(); $this->constructor->setFullOriginalClassName($this->fullOriginalClassName); @@ -165,12 +165,12 @@ public function addConstant(string $name, string $valueCode): void * Adds a class property to this proxy class * * @param string $name Name of the property - * @param string $initialValueCode PHP code of the initial value assignment + * @param string|null $initialValueCode PHP code of the initial value assignment * @param string $visibility * @param string $docComment * @return void */ - public function addProperty(string $name, string $initialValueCode, string $visibility = 'private', string $docComment = ''): void + public function addProperty(string $name, string|null $initialValueCode, string $visibility = 'private', string $docComment = ''): void { // TODO: Add support for PHP attributes? $this->properties[$name] = [ @@ -194,6 +194,15 @@ public function addInterfaces(array $interfaceNames): void $this->interfaces = array_merge($this->interfaces, $interfaceNames); } + /** + * Inspect currently added interfaces for this proxy class + * @return array|string[] + */ + public function getInterfaces(): array + { + return $this->interfaces; + } + /** * Adds one or more traits to the class definition. * @@ -308,7 +317,7 @@ protected function renderPropertiesCode(): string if (!empty($attributes['docComment'])) { $code .= ' ' . $attributes['docComment'] . "\n"; } - $code .= ' ' . $attributes['visibility'] . ' $' . $name . ' = ' . $attributes['initialValueCode'] . ";\n"; + $code .= ' ' . $attributes['visibility'] . ' $' . $name . ($attributes['initialValueCode'] ? (' = ' . $attributes['initialValueCode']) : '') . ";\n"; } return $code; } diff --git a/Neos.Flow/Classes/ObjectManagement/Proxy/RelatedEntitiesContainer.php b/Neos.Flow/Classes/ObjectManagement/Proxy/RelatedEntitiesContainer.php new file mode 100644 index 0000000000..63e69527bc --- /dev/null +++ b/Neos.Flow/Classes/ObjectManagement/Proxy/RelatedEntitiesContainer.php @@ -0,0 +1,51 @@ +relatedEntities as $entityInformation) { + yield $entityInformation; + } + } + + public function reset(): void + { + $this->relatedEntities = []; + } + + public function appendRelatedEntity(string $originalPropertyName, string $path, object $propertyValue): void + { + if ($propertyValue instanceof DoctrineProxy) { + $className = get_parent_class($propertyValue); + } else { + $className = Bootstrap::$staticObjectManager->getObjectNameByClassName(get_class($propertyValue)); + } + $identifier = Bootstrap::$staticObjectManager->get(PersistenceManagerInterface::class)->getIdentifierByObject($propertyValue); + if (!$identifier && $propertyValue instanceof DoctrineProxy) { + $identifier = current(ObjectAccess::getProperty($propertyValue, '_identifier', true)); + } + + $this->relatedEntities[$originalPropertyName . '.' . $path] = [ + 'propertyName' => $originalPropertyName, + 'entityType' => $className, + 'identifier' => $identifier, + 'entityPath' => $path + ]; + } +} diff --git a/Neos.Flow/Configuration/Objects.yaml b/Neos.Flow/Configuration/Objects.yaml index daae5adee2..13b2bfa8b5 100644 --- a/Neos.Flow/Configuration/Objects.yaml +++ b/Neos.Flow/Configuration/Objects.yaml @@ -467,6 +467,7 @@ Neos\Flow\Session\SessionInterface: factoryMethodName: getCurrentSession Neos\Flow\Session\Data\SessionKeyValueStore: + scope: singleton properties: cache: object: @@ -477,6 +478,7 @@ Neos\Flow\Session\Data\SessionKeyValueStore: value: Flow_Session_Storage Neos\Flow\Session\Data\SessionMetaDataStore: + scope: singleton properties: cache: object: diff --git a/Neos.Flow/Configuration/Settings.Object.yaml b/Neos.Flow/Configuration/Settings.Object.yaml index 4e6efb37e2..e328a49d65 100644 --- a/Neos.Flow/Configuration/Settings.Object.yaml +++ b/Neos.Flow/Configuration/Settings.Object.yaml @@ -51,3 +51,13 @@ Neos: # - '^Neos\\SomePackage\\ValueObjects\\SomeSpecificValueObject$' # excludeClassesFromConstructorAutowiring: [] + + # This is a forward compatibility setting, preventing constructor arguments to be autowired + # if they are not declared singleton or session when this setting is false. + # + # BEWARE of dragons, Flow code should be fine, but Neos might still contain such constructs. + # Manual full cache flush is necessary after changing this setting. + # + # In the future autowiring of prototypes should no longer be an option as it makes no sense. + # Use a factory class if you need this otherwise. + prototypeAutowiring: true From 907d5571714b78b7b20261f39db2aaadc23f7536 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Mu=CC=88ller?= Date: Thu, 11 Sep 2025 12:47:19 +0200 Subject: [PATCH 4/9] TASK: Fixes and style updates --- .../DependencyInjection/ProxyClassBuilder.php | 2 -- .../ObjectManagement/Proxy/ObjectSerializationTrait.php | 2 -- .../ObjectManagement/Proxy/RelatedEntitiesContainer.php | 7 ++++--- .../Private/Schema/Settings/Neos.Flow.object.schema.yaml | 2 ++ 4 files changed, 6 insertions(+), 7 deletions(-) diff --git a/Neos.Flow/Classes/ObjectManagement/DependencyInjection/ProxyClassBuilder.php b/Neos.Flow/Classes/ObjectManagement/DependencyInjection/ProxyClassBuilder.php index 5db37a34de..0f3e468010 100644 --- a/Neos.Flow/Classes/ObjectManagement/DependencyInjection/ProxyClassBuilder.php +++ b/Neos.Flow/Classes/ObjectManagement/DependencyInjection/ProxyClassBuilder.php @@ -125,11 +125,9 @@ public function build(): void $sleepMethod = $proxyClass->getMethod('__sleep'); $sleepMethod->setDocBlock(self::AUTOGENERATED_PROXY_METHOD_COMMENT); - $sleepMethod->setReturnType('array'); $wakeupMethod = $proxyClass->getMethod('__wakeup'); $wakeupMethod->setDocBlock(self::AUTOGENERATED_PROXY_METHOD_COMMENT); - $wakeupMethod->setReturnType('void'); $wakeupMethod->addPreParentCallCode($this->buildSetInstanceCode($objectConfiguration)); $wakeupMethod->addPostParentCallCode($this->buildLifecycleInitializationCode($objectConfiguration, ObjectManagerInterface::INITIALIZATIONCAUSE_RECREATED)); diff --git a/Neos.Flow/Classes/ObjectManagement/Proxy/ObjectSerializationTrait.php b/Neos.Flow/Classes/ObjectManagement/Proxy/ObjectSerializationTrait.php index d0d43a0daa..6e9cab1f76 100644 --- a/Neos.Flow/Classes/ObjectManagement/Proxy/ObjectSerializationTrait.php +++ b/Neos.Flow/Classes/ObjectManagement/Proxy/ObjectSerializationTrait.php @@ -19,8 +19,6 @@ use Neos\Flow\Persistence\Aspect\PersistenceMagicInterface; use Neos\Flow\Persistence\PersistenceManagerInterface; use Neos\Utility\Arrays; -use Neos\Utility\Exception\PropertyNotAccessibleException; -use Neos\Utility\ObjectAccess; /** * Methods used to serialize objects used by proxy classes. diff --git a/Neos.Flow/Classes/ObjectManagement/Proxy/RelatedEntitiesContainer.php b/Neos.Flow/Classes/ObjectManagement/Proxy/RelatedEntitiesContainer.php index 63e69527bc..ebc35bef64 100644 --- a/Neos.Flow/Classes/ObjectManagement/Proxy/RelatedEntitiesContainer.php +++ b/Neos.Flow/Classes/ObjectManagement/Proxy/RelatedEntitiesContainer.php @@ -2,15 +2,16 @@ namespace Neos\Flow\ObjectManagement\Proxy; use Doctrine\Persistence\Proxy as DoctrineProxy; -use Exception; use Neos\Flow\Annotations as Flow; use Neos\Flow\Core\Bootstrap; use Neos\Flow\Persistence\PersistenceManagerInterface; use Neos\Utility\ObjectAccess; -use Traversable; /** - * + * This is a mutable container to hold references to entities (class & identifier) for serialization. + * Userland code should never (have to) interact with this, it is used in proxy classes only. You might + * see references to it in serialized object strings. + * @internal */ #[Flow\Proxy(false)] final class RelatedEntitiesContainer implements \IteratorAggregate diff --git a/Neos.Flow/Resources/Private/Schema/Settings/Neos.Flow.object.schema.yaml b/Neos.Flow/Resources/Private/Schema/Settings/Neos.Flow.object.schema.yaml index 8e325db365..df10bc94b6 100644 --- a/Neos.Flow/Resources/Private/Schema/Settings/Neos.Flow.object.schema.yaml +++ b/Neos.Flow/Resources/Private/Schema/Settings/Neos.Flow.object.schema.yaml @@ -14,3 +14,5 @@ properties: excludeClassesFromConstructorAutowiring: type: array items: { type: string, required: true } + prototypeAutowiring: + type: boolean From dfcda74cdbc236d54c1187a2ae7c66cccc47f7f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Mu=CC=88ller?= Date: Fri, 12 Sep 2025 09:23:27 +0200 Subject: [PATCH 5/9] Some further logic fixes to proxy serialization --- .../DependencyInjection/ProxyClassBuilder.php | 15 +++++----- .../Proxy/ObjectSerializationTrait.php | 28 +++++++++++++++---- .../Proxy/RelatedEntitiesContainer.php | 16 +++++------ 3 files changed, 37 insertions(+), 22 deletions(-) diff --git a/Neos.Flow/Classes/ObjectManagement/DependencyInjection/ProxyClassBuilder.php b/Neos.Flow/Classes/ObjectManagement/DependencyInjection/ProxyClassBuilder.php index 0f3e468010..7ab0f8f388 100644 --- a/Neos.Flow/Classes/ObjectManagement/DependencyInjection/ProxyClassBuilder.php +++ b/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); diff --git a/Neos.Flow/Classes/ObjectManagement/Proxy/ObjectSerializationTrait.php b/Neos.Flow/Classes/ObjectManagement/Proxy/ObjectSerializationTrait.php index 6e9cab1f76..4088950615 100644 --- a/Neos.Flow/Classes/ObjectManagement/Proxy/ObjectSerializationTrait.php +++ b/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; } } diff --git a/Neos.Flow/Classes/ObjectManagement/Proxy/RelatedEntitiesContainer.php b/Neos.Flow/Classes/ObjectManagement/Proxy/RelatedEntitiesContainer.php index ebc35bef64..934029b23c 100644 --- a/Neos.Flow/Classes/ObjectManagement/Proxy/RelatedEntitiesContainer.php +++ b/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 ]; } } From a4fae90d08e2b3792b72384bec1d069a5c9f6f3e Mon Sep 17 00:00:00 2001 From: Robert Lemke Date: Mon, 20 Oct 2025 16:55:40 +0200 Subject: [PATCH 6/9] Add functional test for proxy serialization bug --- .../Fixtures/ClassWithEntityProperty.php | 40 +++++++++++++++++++ .../Fixtures/SimpleEntity.php | 37 +++++++++++++++++ .../ProxySerializationTest.php | 39 ++++++++++++++++++ 3 files changed, 116 insertions(+) create mode 100644 Neos.Flow/Tests/Functional/ObjectManagement/Fixtures/ClassWithEntityProperty.php create mode 100644 Neos.Flow/Tests/Functional/ObjectManagement/Fixtures/SimpleEntity.php create mode 100644 Neos.Flow/Tests/Functional/ObjectManagement/ProxySerializationTest.php diff --git a/Neos.Flow/Tests/Functional/ObjectManagement/Fixtures/ClassWithEntityProperty.php b/Neos.Flow/Tests/Functional/ObjectManagement/Fixtures/ClassWithEntityProperty.php new file mode 100644 index 0000000000..e5921f6e07 --- /dev/null +++ b/Neos.Flow/Tests/Functional/ObjectManagement/Fixtures/ClassWithEntityProperty.php @@ -0,0 +1,40 @@ +entity = $entity; + $this->someValue = $someValue; + } +} diff --git a/Neos.Flow/Tests/Functional/ObjectManagement/Fixtures/SimpleEntity.php b/Neos.Flow/Tests/Functional/ObjectManagement/Fixtures/SimpleEntity.php new file mode 100644 index 0000000000..d71d813733 --- /dev/null +++ b/Neos.Flow/Tests/Functional/ObjectManagement/Fixtures/SimpleEntity.php @@ -0,0 +1,37 @@ +name = $name; + } + + public function getName(): string + { + return $this->name; + } +} diff --git a/Neos.Flow/Tests/Functional/ObjectManagement/ProxySerializationTest.php b/Neos.Flow/Tests/Functional/ObjectManagement/ProxySerializationTest.php new file mode 100644 index 0000000000..284e64c3a0 --- /dev/null +++ b/Neos.Flow/Tests/Functional/ObjectManagement/ProxySerializationTest.php @@ -0,0 +1,39 @@ +someValue); + } +} From 024bd10e432caf673fd93dd7af0517c4d323710e Mon Sep 17 00:00:00 2001 From: Robert Lemke Date: Mon, 20 Oct 2025 17:04:02 +0200 Subject: [PATCH 7/9] Improve documentation for Proxy annotation --- Neos.Flow/Classes/Annotations/Proxy.php | 77 ++++++++++++++++++------- 1 file changed, 56 insertions(+), 21 deletions(-) diff --git a/Neos.Flow/Classes/Annotations/Proxy.php b/Neos.Flow/Classes/Annotations/Proxy.php index 0f049053a9..e5d9edb025 100644 --- a/Neos.Flow/Classes/Annotations/Proxy.php +++ b/Neos.Flow/Classes/Annotations/Proxy.php @@ -15,10 +15,17 @@ use Neos\Flow\ObjectManagement\Exception\ProxyCompilerException; /** - * Used to disable proxy building for an object. + * Controls proxy class generation behavior for a class. * - * If disabled, neither Dependency Injection nor AOP can be used - * on the object. + * This annotation allows you to: + * - Disable proxy building entirely (enabled=false) - useful for value objects, DTOs, + * or classes that should not use Dependency Injection or AOP + * - Force generation of serialization code (forceSerializationCode=true) - rarely needed + * escape hatch for edge cases where automatic detection of entity relationships fails + * + * When proxy building is disabled (enabled=false), neither Dependency Injection nor AOP + * can be used on the object. The class will be instantiated directly without any + * framework enhancements. * * @Annotation * @NamedArgumentConstructor @@ -28,28 +35,56 @@ final class Proxy { /** - * Whether proxy building for the target is disabled. (Can be given as anonymous argument.) - * @var boolean + * Whether proxy building is enabled for this class. + * + * When set to false, Flow will not generate a proxy class, meaning: + * - No Dependency Injection (no Flow\Inject annotations) + * - No Aspect-Oriented Programming (no AOP advices) + * - No automatic serialization handling + * - The class is instantiated directly without any framework enhancements + * + * This is useful for simple value objects, DTOs, or utility classes that don't need + * framework features and where you want to avoid the minimal overhead of proxy classes. + * + * (Can be given as anonymous argument.) */ - public $enabled = true; + public bool $enabled = true; /** - * Whether you need serialization code build in the proxy, this might be needed if you - * build a PHP object you need to serialize that includes entities for example, as in that - * case the entities should be converted to metadata (class & persistence identifier) before - * serialization. - * The serialization code also removes injected/otherwise internal framework properties - * introduced by the proxy building but these situations should be correctly detected by - * proxy building and create the serialization code anyway so you should never have to - * set this for these cases and it would be a bug if you have to. - * - * At this point it wouldn't make much sense to allow a forced disabling of the serialization - * code as that would most certainly run into problems if there was AOP, injections or other reasons. - * Rather disable the proxy completely then. - * - * @var bool + * Force the generation of serialization code (__sleep/__wakeup methods) in the proxy class. + * + * Flow automatically detects when serialization code is needed (e.g., when a class has entity + * properties, injected dependencies, or transient properties) and generates the appropriate + * __sleep() and __wakeup() methods. These methods handle: + * - Converting entity references to metadata (class name + persistence identifier) + * - Removing injected and framework-internal properties before serialization + * - Restoring entity references and re-injecting dependencies after deserialization + * + * This flag serves as an **escape hatch for rare edge cases** where automatic detection fails, + * such as: + * - Complex generic/template types that aren't fully parsed (e.g., ComplexType) + * - Deeply nested entity structures where type hints don't reveal the entity relationship + * - Union or intersection types with entities that the reflection system cannot fully analyze + * - Properties with dynamic types where documentation hints are non-standard + * + * IMPORTANT: You should rarely need this flag. Flow's automatic detection handles: + * - Properties typed with Flow\Entity classes + * - Properties with Flow\Inject annotations + * - Properties with Flow\Transient annotations + * - Classes with AOP advices + * - Session-scoped objects + * + * If you find yourself needing this flag for standard entity properties, injected dependencies, + * or other common cases, this indicates a bug in Flow's detection logic that should be reported + * at https://github.com/neos/flow-development-collection/issues + * + * Note: Disabling serialization code (not possible via this flag) would break classes with + * AOP, injections, or entity relationships. To completely opt out of proxy features, use + * enabled=false instead. + * + * @see https://flowframework.readthedocs.io/ for more information on object serialization */ - public $forceSerializationCode = false; + public bool $forceSerializationCode = false; public function __construct(bool $enabled = true, bool $forceSerializationCode = false) { From 8aed3247e7e38a1ec136ad1adf0b8e4b6df8d39f Mon Sep 17 00:00:00 2001 From: Robert Lemke Date: Mon, 20 Oct 2025 22:05:08 +0200 Subject: [PATCH 8/9] BUGFIX: Support PHP 8+ named arguments in proxy constructors This change enables Flow proxy classes to support PHP 8+ named argument syntax by including the original constructor parameter signature in generated proxies, fixing compatibility issues with modern PHP code and reflection-based serializers. The proxy constructor now always includes parameters matching the original constructor signature, with all parameters made nullable and given null default values to maintain dependency injection flexibility. This allows both named and positional argument syntax while preserving backward compatibility with existing DI patterns. Resolves #3076 --- .../DependencyInjection/ProxyClassBuilder.php | 2 +- .../ObjectManagement/Proxy/ProxyClass.php | 4 +- .../Proxy/ProxyConstructorGenerator.php | 52 +++++++++++++---- .../DependencyInjectionTest.php | 58 +++++++++++++++++++ .../ClassWithNamedConstructorArguments.php | 38 ++++++++++++ .../SingletonWithConstructorInjection.php | 43 ++++++++++++++ 6 files changed, 184 insertions(+), 13 deletions(-) create mode 100644 Neos.Flow/Tests/Functional/ObjectManagement/Fixtures/ClassWithNamedConstructorArguments.php create mode 100644 Neos.Flow/Tests/Functional/ObjectManagement/Fixtures/SingletonWithConstructorInjection.php diff --git a/Neos.Flow/Classes/ObjectManagement/DependencyInjection/ProxyClassBuilder.php b/Neos.Flow/Classes/ObjectManagement/DependencyInjection/ProxyClassBuilder.php index 7ab0f8f388..7932d98b72 100644 --- a/Neos.Flow/Classes/ObjectManagement/DependencyInjection/ProxyClassBuilder.php +++ b/Neos.Flow/Classes/ObjectManagement/DependencyInjection/ProxyClassBuilder.php @@ -118,7 +118,7 @@ public function build(): void $constructorInjectionCode = $this->buildConstructorInjectionCode($objectConfiguration); $injectionCodeWasIntroduced = $constructorInjectionCode !== ''; - $constructor = $proxyClass->getConstructor($injectionCodeWasIntroduced === false); + $constructor = $proxyClass->getConstructor(); $constructor->addPreParentCallCode($this->buildSetInstanceCode($objectConfiguration)); $constructor->addPreParentCallCode($constructorInjectionCode); diff --git a/Neos.Flow/Classes/ObjectManagement/Proxy/ProxyClass.php b/Neos.Flow/Classes/ObjectManagement/Proxy/ProxyClass.php index 0fd1d0efaf..139d3836b4 100644 --- a/Neos.Flow/Classes/ObjectManagement/Proxy/ProxyClass.php +++ b/Neos.Flow/Classes/ObjectManagement/Proxy/ProxyClass.php @@ -115,11 +115,11 @@ public function injectReflectionService(ReflectionService $reflectionService): v * @throws \ReflectionException * @throws CannotBuildObjectException */ - public function getConstructor(bool $withOriginalArgumentSignature = false): ProxyConstructorGenerator + public function getConstructor(): ProxyConstructorGenerator { if (!isset($this->constructor)) { if (method_exists($this->fullOriginalClassName, '__construct')) { - $this->constructor = ProxyConstructorGenerator::fromReflection(new MethodReflection($this->fullOriginalClassName, '__construct'), $withOriginalArgumentSignature); + $this->constructor = ProxyConstructorGenerator::fromReflection(new MethodReflection($this->fullOriginalClassName, '__construct')); } else { $this->constructor = new ProxyConstructorGenerator(); $this->constructor->setFullOriginalClassName($this->fullOriginalClassName); diff --git a/Neos.Flow/Classes/ObjectManagement/Proxy/ProxyConstructorGenerator.php b/Neos.Flow/Classes/ObjectManagement/Proxy/ProxyConstructorGenerator.php index c6405db34b..723d4bb9b6 100644 --- a/Neos.Flow/Classes/ObjectManagement/Proxy/ProxyConstructorGenerator.php +++ b/Neos.Flow/Classes/ObjectManagement/Proxy/ProxyConstructorGenerator.php @@ -13,7 +13,6 @@ use Laminas\Code\Generator\DocBlockGenerator; use Laminas\Code\Generator\ParameterGenerator; -use Laminas\Code\Generator\PromotedParameterGenerator; use Neos\Flow\ObjectManagement\DependencyInjection\ProxyClassBuilder; final class ProxyConstructorGenerator extends ProxyMethodGenerator @@ -32,7 +31,7 @@ public function __construct($name = null, array $parameters = [], $flags = self: parent::__construct('__construct', $parameters, $flags, $body, $docBlock); } - public static function fromReflection(\Laminas\Code\Reflection\MethodReflection $reflectionMethod, bool $withOriginalArgumentSignature = false): static + public static function fromReflection(\Laminas\Code\Reflection\MethodReflection $reflectionMethod): static { $method = new static('__construct'); $declaringClass = $reflectionMethod->getDeclaringClass(); @@ -59,14 +58,26 @@ public static function fromReflection(\Laminas\Code\Reflection\MethodReflection $docBlock->setSourceDirty(false); $method->setDocBlock($docBlock); - if ($withOriginalArgumentSignature) { - foreach ($reflectionMethod->getParameters() as $reflectionParameter) { - $method->setParameter( - $reflectionParameter->isPromoted() - ? PromotedParameterGenerator::fromReflection($reflectionParameter) - : ParameterGenerator::fromReflection($reflectionParameter) - ); + # Always include original parameters to support named arguments (issue #3076) + foreach ($reflectionMethod->getParameters() as $reflectionParameter) { + $parameter = ParameterGenerator::fromReflection($reflectionParameter); + + $parameterType = $parameter->getType(); + if ($parameterType !== null && $parameterType !== 'mixed' && !str_starts_with($parameterType, '?') && !str_contains($parameterType, 'null')) { + # For union types, add |null instead of ? prefix + if (str_contains($parameterType, '|')) { + $parameter->setType($parameterType . '|null'); + } else { + # For simple types, use ? prefix + $parameter->setType('?' . ltrim($parameterType, '\\')); + } } + + if (!$reflectionParameter->isDefaultValueAvailable()) { + $parameter->setDefaultValue(null); + } + + $method->setParameter($parameter); } return $method; @@ -105,7 +116,28 @@ public function renderBodyCode(): string protected function buildAssignMethodArgumentsCode(): string { - return '$arguments = func_get_args();' . PHP_EOL; + $parameters = $this->getParameters(); + if (empty($parameters)) { + // No parameters, use func_get_args() for backward compatibility + return '$arguments = func_get_args();' . PHP_EOL; + } + + # Build arguments array from actual parameters to support both named and positional arguments + # Only include arguments that were actually provided (to allow DI to fill in missing ones) + # Use a unique variable name to avoid conflicts if a parameter is named $arguments + $code = '$methodArguments = func_get_args();' . PHP_EOL; + $code .= '$arguments = [];' . PHP_EOL; + $index = 0; + foreach ($parameters as $parameter) { + // Use func_num_args() to check if this parameter was actually provided + // This allows DI to inject values for parameters that weren't explicitly passed + $code .= 'if (func_num_args() > ' . $index . ') {' . PHP_EOL; + $code .= ' $arguments[' . $index . '] = $methodArguments[' . $index . '];' . PHP_EOL; + $code .= '}' . PHP_EOL; + $index++; + } + + return $code; } /** diff --git a/Neos.Flow/Tests/Functional/ObjectManagement/DependencyInjectionTest.php b/Neos.Flow/Tests/Functional/ObjectManagement/DependencyInjectionTest.php index 712c5accae..d8e6e0a505 100644 --- a/Neos.Flow/Tests/Functional/ObjectManagement/DependencyInjectionTest.php +++ b/Neos.Flow/Tests/Functional/ObjectManagement/DependencyInjectionTest.php @@ -363,4 +363,62 @@ public function constructorSettingsInjectionViaInjectAnnotation(): void $object = new PrototypeClassL('override'); self::assertSame('override', $object->value); } + + /** + * Test that proxy constructors support PHP 8+ named arguments (issue #3076) + * + * @test + */ + public function prototypeObjectsCanBeInstantiatedWithNamedArguments(): void + { + $valueObject = new Fixtures\ValueObjectClassA('test-value'); + $object = new Fixtures\ClassWithNamedConstructorArguments( + valueObject: $valueObject, + stringValue: 'named-value' + ); + + self::assertSame($valueObject, $object->getValueObject()); + self::assertSame('named-value', $object->getStringValue()); + } + + /** + * Test that named arguments work with positional arguments + * + * @test + */ + public function prototypeObjectsCanBeInstantiatedWithPositionalArguments(): void + { + $valueObject = new Fixtures\ValueObjectClassA('test-value'); + $object = new Fixtures\ClassWithNamedConstructorArguments($valueObject, 'positional-value'); + + self::assertSame($valueObject, $object->getValueObject()); + self::assertSame('positional-value', $object->getStringValue()); + } + + /** + * Test that named arguments work with default values + * + * @test + */ + public function prototypeObjectsCanBeInstantiatedWithNamedArgumentsAndDefaults(): void + { + $valueObject = new Fixtures\ValueObjectClassA('test-value'); + $object = new Fixtures\ClassWithNamedConstructorArguments(valueObject: $valueObject); + + self::assertSame($valueObject, $object->getValueObject()); + self::assertSame('default', $object->getStringValue()); + } + + /** + * Test that constructor injection still works with named argument support + * + * @test + */ + public function singletonObjectsWithConstructorInjectionStillWorkWithNamedArgumentSupport(): void + { + $singleton = $this->objectManager->get(Fixtures\SingletonWithConstructorInjection::class); + + self::assertInstanceOf(Fixtures\SingletonClassA::class, $singleton->getSingletonA()); + self::assertNull($singleton->getOptionalValue()); + } } diff --git a/Neos.Flow/Tests/Functional/ObjectManagement/Fixtures/ClassWithNamedConstructorArguments.php b/Neos.Flow/Tests/Functional/ObjectManagement/Fixtures/ClassWithNamedConstructorArguments.php new file mode 100644 index 0000000000..d7b5493e52 --- /dev/null +++ b/Neos.Flow/Tests/Functional/ObjectManagement/Fixtures/ClassWithNamedConstructorArguments.php @@ -0,0 +1,38 @@ +valueObject; + } + + public function getStringValue(): string + { + return $this->stringValue; + } +} diff --git a/Neos.Flow/Tests/Functional/ObjectManagement/Fixtures/SingletonWithConstructorInjection.php b/Neos.Flow/Tests/Functional/ObjectManagement/Fixtures/SingletonWithConstructorInjection.php new file mode 100644 index 0000000000..2fe96b1183 --- /dev/null +++ b/Neos.Flow/Tests/Functional/ObjectManagement/Fixtures/SingletonWithConstructorInjection.php @@ -0,0 +1,43 @@ +singletonA = $singletonA; + $this->optionalValue = $optionalValue; + } + + public function getSingletonA(): SingletonClassA + { + return $this->singletonA; + } + + public function getOptionalValue(): ?string + { + return $this->optionalValue; + } +} From be00fee631592da30cd172f3bdf452ffd8cbde7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Mu=CC=88ller?= Date: Wed, 22 Oct 2025 08:00:10 +0200 Subject: [PATCH 9/9] Streamline code for constructor injection even more This also avoids a bug with constructors accepting variadic arguments as they exist in Neos.Neos. --- .../DependencyInjection/ProxyClassBuilder.php | 4 +-- .../ObjectManagement/Proxy/ProxyClass.php | 30 +++++++++---------- .../Proxy/ProxyConstructorGenerator.php | 28 ++++------------- 3 files changed, 23 insertions(+), 39 deletions(-) diff --git a/Neos.Flow/Classes/ObjectManagement/DependencyInjection/ProxyClassBuilder.php b/Neos.Flow/Classes/ObjectManagement/DependencyInjection/ProxyClassBuilder.php index 7932d98b72..21caa8cf31 100644 --- a/Neos.Flow/Classes/ObjectManagement/DependencyInjection/ProxyClassBuilder.php +++ b/Neos.Flow/Classes/ObjectManagement/DependencyInjection/ProxyClassBuilder.php @@ -314,7 +314,7 @@ protected function buildConstructorInjectionCode(Configuration $objectConfigurat } elseif ($this->objectConfigurations[$argumentValueObjectName]->getScope() === Configuration::SCOPE_PROTOTYPE) { $assignments[$argumentPosition] = $assignmentPrologue . 'new \\' . $argumentValueObjectName . '(' . $this->buildMethodParametersCode($argumentValue->getArguments()) . ')'; } else { - $assignments[$argumentPosition] = $assignmentPrologue . '\Neos\Flow\Core\Bootstrap::$staticObjectManager->get(\'' . $argumentValueObjectName . '\')'; + $assignments[$argumentPosition] = $assignmentPrologue . '\Neos\Flow\Core\Bootstrap::$staticObjectManager->get(\\' . $argumentValueObjectName . '::class)'; } } else { if (str_contains($argumentValue, '.')) { @@ -336,7 +336,7 @@ protected function buildConstructorInjectionCode(Configuration $objectConfigurat $this->logger->debug(sprintf('Class "%s" will get prototype "%s" injected via constructor, this is deprecated.', $objectConfiguration->getClassName(), $argumentObjectConfiguration->getClassName())); } - $assignments[$argumentPosition] = $assignmentPrologue . '\Neos\Flow\Core\Bootstrap::$staticObjectManager->get(\'' . $argumentValue . '\')'; + $assignments[$argumentPosition] = $assignmentPrologue . '\Neos\Flow\Core\Bootstrap::$staticObjectManager->get(\\' . $argumentValue . '::class)'; $doReturnCode = true; } break; diff --git a/Neos.Flow/Classes/ObjectManagement/Proxy/ProxyClass.php b/Neos.Flow/Classes/ObjectManagement/Proxy/ProxyClass.php index 139d3836b4..5d359015d5 100644 --- a/Neos.Flow/Classes/ObjectManagement/Proxy/ProxyClass.php +++ b/Neos.Flow/Classes/ObjectManagement/Proxy/ProxyClass.php @@ -28,36 +28,36 @@ class ProxyClass * * @var string */ - protected $namespace = ''; + protected string $namespace = ''; /** * The original class name * * @var string */ - protected $originalClassName; + protected string $originalClassName; /** * Fully qualified class name of the original class * * @var class-string */ - protected $fullOriginalClassName; + protected string $fullOriginalClassName; /** - * @var ProxyConstructorGenerator + * @var ProxyConstructorGenerator|null */ - protected $constructor; + protected ?ProxyConstructorGenerator $constructor = null; /** - * @var array + * @var array */ - protected $methods = []; + protected array $methods = []; /** - * @var array + * @var array */ - protected $constants = []; + protected array $constants = []; /** * Note: Not using ProxyInterface::class here, since the interface names must have a leading backslash. @@ -67,19 +67,19 @@ class ProxyClass protected $interfaces = ['\Neos\Flow\ObjectManagement\Proxy\ProxyInterface']; /** - * @var array + * @var array */ - protected $traits = []; + protected array $traits = []; /** - * @var array + * @var array */ - protected $properties = []; + protected array $properties = []; /** * @var ReflectionService */ - protected $reflectionService; + protected ReflectionService $reflectionService; /** * Creates a new ProxyClass instance. @@ -186,7 +186,7 @@ public function addProperty(string $name, string|null $initialValueCode, string * Note that the passed interface names must already have a leading backslash, * for example "\Neos\Flow\Foo\BarInterface". * - * @param array $interfaceNames Fully qualified names of the interfaces to introduce + * @param array $interfaceNames Fully qualified names of the interfaces to introduce * @return void */ public function addInterfaces(array $interfaceNames): void diff --git a/Neos.Flow/Classes/ObjectManagement/Proxy/ProxyConstructorGenerator.php b/Neos.Flow/Classes/ObjectManagement/Proxy/ProxyConstructorGenerator.php index 723d4bb9b6..76892f4ba4 100644 --- a/Neos.Flow/Classes/ObjectManagement/Proxy/ProxyConstructorGenerator.php +++ b/Neos.Flow/Classes/ObjectManagement/Proxy/ProxyConstructorGenerator.php @@ -61,6 +61,11 @@ public static function fromReflection(\Laminas\Code\Reflection\MethodReflection # Always include original parameters to support named arguments (issue #3076) foreach ($reflectionMethod->getParameters() as $reflectionParameter) { $parameter = ParameterGenerator::fromReflection($reflectionParameter); + // workaround necessary for variadic parameters, which cannot be optional/with default + if ($reflectionParameter->isVariadic()) { + $method->setParameter($parameter); + continue; + } $parameterType = $parameter->getType(); if ($parameterType !== null && $parameterType !== 'mixed' && !str_starts_with($parameterType, '?') && !str_contains($parameterType, 'null')) { @@ -116,28 +121,7 @@ public function renderBodyCode(): string protected function buildAssignMethodArgumentsCode(): string { - $parameters = $this->getParameters(); - if (empty($parameters)) { - // No parameters, use func_get_args() for backward compatibility - return '$arguments = func_get_args();' . PHP_EOL; - } - - # Build arguments array from actual parameters to support both named and positional arguments - # Only include arguments that were actually provided (to allow DI to fill in missing ones) - # Use a unique variable name to avoid conflicts if a parameter is named $arguments - $code = '$methodArguments = func_get_args();' . PHP_EOL; - $code .= '$arguments = [];' . PHP_EOL; - $index = 0; - foreach ($parameters as $parameter) { - // Use func_num_args() to check if this parameter was actually provided - // This allows DI to inject values for parameters that weren't explicitly passed - $code .= 'if (func_num_args() > ' . $index . ') {' . PHP_EOL; - $code .= ' $arguments[' . $index . '] = $methodArguments[' . $index . '];' . PHP_EOL; - $code .= '}' . PHP_EOL; - $index++; - } - - return $code; + return '$arguments = func_get_args();' . PHP_EOL; } /**