BUGFIX: ProxyBuilding minor refactor and fixes

[kitsunet]

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: https://github.com/neos/flow-development-collection/pull/3406

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: https://github.com/neos/flow-development-collection/issues/3493 Related: https://github.com/neos/flow-development-collection/issues/3212 Related: https://github.com/neos/flow-development-collection/issues/3076

[kitsunet]

Interesting test failures... because that did work fine... Investigating...

[lorenzulrich]

Thanks @kitsunet. This is "above my pay grade", so thanks for taking care and finding the reason why my code works ;-).

[kitsunet]

Ok funny, now this PR shows one of the errors I wanted to fix in the functional tests. Why? Well because I broke something (that is if an object already has a sleep method we do not proxy anything) but intererstingly this scenario is AFAIK not happening in Flow nor Neos, nor have I seen it in any of the projects I checked. Anyways, will fix, seems like a good idea.

[robertlemke]

Some documentation for the new flag - I don't know where to put it, maybe into this PR's description? Or the manual?

When Flow generates proxy classes, it automatically detects if your class contains entity properties (properties typed with classes annotated as @Flow\Entity) or other framework-managed objects that require special handling during serialization. In such cases, Flow automatically generates __sleep() and __wakeup() methods that:

• Convert entity references to metadata (class name and persistence identifier) before serialization
• Remove injected and transient properties
• Restore entity references after deserialization

This detection is automatic and works in most cases. However, in rare edge cases where the automatic detection fails (e.g., with complex generic types, deeply nested entity structures, or unusual type declarations), you can force the generation of serialization code using the @Flow\Proxy annotation::

	use Neos\Flow\Annotations as Flow;

	/**
	 * @Flow\Proxy(forceSerializationCode=true)
	 */
	class ComplexObjectWithEntities {

		/**
		 * @var ComplexGenericType<SomeEntity>
		 */
		protected $complexProperty;

	}

You should rarely need to use forceSerializationCode. If you find yourself needing it for standard entity properties or injected dependencies, this indicates a bug in Flow's automatic detection that should be reported.

[mhsdesign]

Okay i found a bug last minute

Regression:

During testing i had Neos Debug installed and the proxies generated failed at php compile time:

Fatal error: Type of Flowpack\Neos\Debug\DataCollector\MessagesCollector::$dataFormatter must be ?Flowpack\Neos\Debug\DataFormatter\DataFormatterInterface (as in class Flowpack\Neos\Debug\DataCollector\AbstractDataCollector) in Data/Temporary/Development/Cache/Code/Flow_Object_Classes/Flowpack_Neos_Debug_DataCollector_MessagesCollector.php on line 40

The reason for this is the constructor in the abstract class: https://github.com/Flowpack/neos-debug/blob/9c82cb345cb1b786b88f07dee73824882479a21d/Classes/DataCollector/AbstractDataCollector.php#L12

public function __construct(
    protected ?DataFormatterInterface $dataFormatter = null,
) {
}

Now the object extending the AbstractDataCollector got in 9.x a proxy like

public function __construct()
{
    $arguments = func_get_args();
    if (get_class($this) === 'Flowpack\Neos\Debug\DataCollector\MessagesCollector') \Neos\Flow\Core\Bootstrap::$staticObjectManager->setInstance('Flowpack\Neos\Debug\DataCollector\MessagesCollector', $this);
    parent::__construct(...$arguments);
}

But with this change (prototypeAutowiring untouched!) the flow proxy gets generated like this by default:

public function __construct(protected \Flowpack\Neos\Debug\DataFormatter\DataFormatterInterface $dataFormatter)
{
    $arguments = func_get_args();
    if (get_class($this) === 'Flowpack\Neos\Debug\DataCollector\MessagesCollector') \Neos\Flow\Core\Bootstrap::$staticObjectManager->setInstance('Flowpack\Neos\Debug\DataCollector\MessagesCollector', $this);
    parent::__construct(...$arguments);
}

Where the default value and the nullability is missing.

EDIT: Found a possibly hack fix https://github.com/neos/flow-development-collection/pull/3494/commits/c7e76f9f07fe2d05369f69d2602d9ef0e0c5e111 but it seems we might need https://github.com/laminas/laminas-code/issues/182 in reality?

[mhsdesign]

and also another oddity during the depreciation logging - there are much more entries than i would have anticipated in which the user of Neos and Flow cant do anything. Even Flow itself seems to raise the deprecation for its own code. I fear that we cannot distinguish between needing the auto-wiring and that Flow just gave that gratis until now. We should probably specify autowiring off for the respective constructors.

The list shows the prototype first entry where it is used in the constructor ... nested entries.

• Neos\ContentRepositoryRegistry\Migration\Factory\MigrationFactory (prototype):

  • Neos\ContentRepositoryRegistry\Command\NodeMigrationCommandController

• Neos\ContentRepositoryRegistry\Service\NodeMigrationGeneratorService (prototype):

  • Neos\ContentRepositoryRegistry\Command\NodeMigrationCommandController

• Neos\ContentRepository\LegacyNodeMigration\RootNodeTypeMapping (prototype):

  • Neos\ContentRepository\LegacyNodeMigration\LegacyExportService
  • Neos\ContentRepository\LegacyNodeMigration\LegacyExportServiceFactory
  • Neos\ContentRepository\LegacyNodeMigration\Processors\EventExportProcessor

• Neos\Flow\Aop\JoinPoint (prototype):

  • Neos\Flow\Security\Authorization\Privilege\Method\MethodPrivilegeSubject

• Neos\Flow\I18n\Locale (prototype):

  • Neos\Flow\I18n\Xliff\Model\FileAdapter

• Neos\Flow\Mvc\ActionRequest (prototype):

  • Neos\Flow\Mvc\Controller\ControllerContext
  • Neos\Form\Core\Runtime\FormRuntime
  • Neos\Fusion\Core\LegacyFusionControllerContext

• Neos\Flow\Mvc\ActionResponse (prototype):

  • Neos\Flow\Mvc\Controller\ControllerContext
  • Neos\Form\Core\Runtime\FormRuntime
  • Neos\Fusion\Core\LegacyFusionControllerContext

• Neos\Flow\Mvc\Controller\Arguments (prototype):

  • Neos\Flow\Mvc\Controller\ControllerContext

• Neos\Flow\Mvc\Routing\UriBuilder (prototype):

  • Neos\Flow\Mvc\Controller\ControllerContext

• Neos\Flow\Persistence\Doctrine\Query (prototype):

  • Neos\Flow\Persistence\Doctrine\QueryResult
  • Neos\Flow\Persistence\EmptyQueryResult
  • Neos\Media\Domain\Model\AssetSource\Neos\NeosAssetProxyQuery

• Neos\Flow\ResourceManagement\PersistentResource (prototype):

  • Neos\Media\Domain\Model\Asset
  • Neos\Media\Domain\Model\Audio
  • Neos\Media\Domain\Model\Document
  • Neos\Media\Domain\Model\Image
  • Neos\Media\Domain\Model\Video

• Neos\Flow\Security\Authorization\Privilege\PrivilegeTarget (prototype):

  • Neos\Media\Security\Authorization\Privilege\ReadAssetCollectionPrivilege
  • Neos\Media\Security\Authorization\Privilege\ReadAssetPrivilege
  • Neos\Media\Security\Authorization\Privilege\ReadTagPrivilege
  • Neos\Neos\Security\Authorization\Privilege\EditNodePrivilege
  • Neos\Neos\Security\Authorization\Privilege\ModulePrivilege
  • Neos\Neos\Security\Authorization\Privilege\ReadNodePrivilege

• Neos\Form\Core\Model\FormDefinition (prototype):

  • Neos\Form\Core\Runtime\FormRuntime

• Neos\Form\Core\Runtime\FormRuntime (prototype):

  • Neos\Form\Core\Model\FinisherContext

• Neos\Fusion\Core\Cache\FusionContextSerializer (prototype):

  • Neos\Neos\Fusion\Cache\NeosFusionContextSerializer

• Neos\Fusion\Core\FusionConfiguration (prototype):

  • Neos\Fusion\Core\Runtime

• Neos\Fusion\Core\FusionGlobals (prototype):

  • Neos\Fusion\Core\Runtime

• Neos\Fusion\Core\ObjectTreeParser\Lexer (prototype):

  • Neos\Fusion\Core\ObjectTreeParser\ObjectTreeParser

• Neos\Fusion\Core\ObjectTreeParser\MergedArrayTree (prototype):

  • Neos\Fusion\Core\ObjectTreeParser\MergedArrayTreeVisitor

• Neos\Fusion\Core\Runtime (prototype):

  • Neos\Fusion\Core\Cache\RuntimeContentCache
  • all objects of type AbstractFusionObject because of the shared constructor

• Neos\Fusion\FusionObjects\AbstractFusionObject (prototype):

  • Neos\Fusion\FusionObjects\Helpers\FluidView

• Neos\Fusion\FusionObjects\TemplateImplementation (prototype):

  • Neos\Fusion\FusionObjects\Helpers\FusionPathProxy

• Neos\Http\Factories\UriFactory (prototype):

  • Neos\Http\Factories\PsrHttpFactory
  • Neos\Http\Factories\ServerRequestFactory

• Neos\Media\Domain\Model\AssetSource\Neos\NeosAssetSource (prototype):

  • Neos\Media\Domain\Model\AssetSource\Neos\NeosAssetProxy
  • Neos\Media\Domain\Model\AssetSource\Neos\NeosAssetProxyQuery
  • Neos\Media\Domain\Model\AssetSource\Neos\NeosAssetProxyRepository

• Neos\Media\Domain\Model\Image (prototype):

  • Neos\Media\Browser\Domain\ImageMapper
  • Neos\Media\Domain\Model\ImageVariant

• Neos\Media\Domain\Model\ThumbnailConfiguration (prototype):

  • Neos\Media\Domain\Model\Thumbnail

• Neos\Media\Domain\ValueObject\Configuration\Label (prototype):

  • Neos\Media\Domain\ValueObject\Configuration\VariantPreset

• Neos\Neos\AssetUsage\AssetUsageIndexingProcessor (prototype):

  • Neos\Neos\AssetUsage\Command\AssetUsageCommandController

• Neos\Neos\Domain\SubtreeTagging\SoftRemoval\SoftRemovalGarbageCollector (prototype):

  • Neos\Neos\Domain\Service\WorkspacePublishingService
  • Neos\Neos\Domain\Service\WorkspaceService

• Neos\Neos\FrontendRouting\DimensionResolution\Resolver\UriPathResolver\Segments (prototype):

  • Neos\Neos\FrontendRouting\DimensionResolution\Resolver\UriPathResolver

• Neos\Neos\FrontendRouting\Projection\DocumentUriPathFinder (prototype):

  • Neos\Neos\FrontendRouting\CatchUpHook\RouterCacheHook
  • Neos\RedirectHandler\NeosAdapter\CatchUpHook\DocumentUriPathProjectionHook

• Neos\Neos\Setup\Infrastructure\ImageHandler\ImageHandlerService (prototype):

  • Neos\Neos\Setup\Infrastructure\Healthcheck\ImageHandlerHealthcheck

• Neos\Setup\Domain\HealthcheckEnvironment (prototype):

  • Neos\Setup\Infrastructure\HealthChecker

• Neos\TimeableNodeVisibility\Domain\ChangedVisibilityType (prototype):

  • Neos\TimeableNodeVisibility\Domain\ChangedVisibility

• TYPO3Fluid\Fluid\Core\Compiler\TemplateCompiler (prototype):

  • TYPO3Fluid\Fluid\Core\Compiler\NodeConverter

• TYPO3Fluid\Fluid\Core\Parser\ParsingState (prototype):

  • TYPO3Fluid\Fluid\Core\Parser\SyntaxTree\ViewHelperNode

[kitsunet]

Mmm, technically I think the constructor change doesn't have to live in here, I guess I can extract it and try to put it in the runtime constructor injection change, probably more thematic there anyways. Then this becomes a bit simpler.