From 69c2d3ad53f937a9328363c9817f6a3ae5fa3ff7 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Fri, 18 Dec 2020 15:02:40 +0100 Subject: [PATCH] Add "skipDestructor" option to lazy loading proxies --- .../Factory/AbstractBaseFactory.php | 12 ++++-- .../Factory/LazyLoadingValueHolderFactory.php | 1 + .../MethodGenerator/SkipDestructor.php | 29 ++++++++++++++ .../LazyLoadingGhostGenerator.php | 27 ++++++++++--- .../MethodGenerator/SkipDestructor.php | 32 +++++++++++++++ .../LazyLoadingValueHolderGenerator.php | 25 ++++++++++-- .../lazy-loading-ghost-skip-destructor.phpt | 39 ++++++++++++++++++ ...-loading-value-holder-skip-destructor.phpt | 40 +++++++++++++++++++ 8 files changed, 193 insertions(+), 12 deletions(-) create mode 100644 src/ProxyManager/ProxyGenerator/LazyLoadingGhost/MethodGenerator/SkipDestructor.php create mode 100644 src/ProxyManager/ProxyGenerator/LazyLoadingValueHolder/MethodGenerator/SkipDestructor.php create mode 100644 tests/language-feature-scripts/lazy-loading-ghost-skip-destructor.phpt create mode 100644 tests/language-feature-scripts/lazy-loading-value-holder-skip-destructor.phpt diff --git a/src/ProxyManager/Factory/AbstractBaseFactory.php b/src/ProxyManager/Factory/AbstractBaseFactory.php index 005249931..a1c68bec8 100644 --- a/src/ProxyManager/Factory/AbstractBaseFactory.php +++ b/src/ProxyManager/Factory/AbstractBaseFactory.php @@ -17,6 +17,8 @@ use function assert; use function class_exists; use function is_a; +use function serialize; +use function sha1; /** * Base factory common logic @@ -29,7 +31,7 @@ abstract class AbstractBaseFactory * Cached checked class names * * @var array - * @psalm-var array + * @psalm-var array */ private array $checkedClasses = []; @@ -54,8 +56,10 @@ public function __construct(?Configuration $configuration = null) */ protected function generateProxy(string $className, array $proxyOptions = []): string { - if (array_key_exists($className, $this->checkedClasses)) { - $generatedClassName = $this->checkedClasses[$className]; + $cacheKey = $proxyOptions ? sha1(serialize([$className, $proxyOptions])) : $className; + + if (array_key_exists($cacheKey, $this->checkedClasses)) { + $generatedClassName = $this->checkedClasses[$cacheKey]; assert(is_a($generatedClassName, $className, true)); @@ -87,7 +91,7 @@ protected function generateProxy(string $className, array $proxyOptions = []): s ->getSignatureChecker() ->checkSignature(new ReflectionClass($proxyClassName), $proxyParameters); - return $this->checkedClasses[$className] = $proxyClassName; + return $this->checkedClasses[$cacheKey] = $proxyClassName; } abstract protected function getGenerator(): ProxyGeneratorInterface; diff --git a/src/ProxyManager/Factory/LazyLoadingValueHolderFactory.php b/src/ProxyManager/Factory/LazyLoadingValueHolderFactory.php index 54b8c9b1f..616af991a 100644 --- a/src/ProxyManager/Factory/LazyLoadingValueHolderFactory.php +++ b/src/ProxyManager/Factory/LazyLoadingValueHolderFactory.php @@ -35,6 +35,7 @@ public function __construct(?Configuration $configuration = null) * array=, * ?Closure= * ) : bool $initializer + * @psalm-param array{skipDestructor?: bool} $proxyOptions * * @psalm-return RealObjectType&ValueHolderInterface&VirtualProxyInterface * diff --git a/src/ProxyManager/ProxyGenerator/LazyLoadingGhost/MethodGenerator/SkipDestructor.php b/src/ProxyManager/ProxyGenerator/LazyLoadingGhost/MethodGenerator/SkipDestructor.php new file mode 100644 index 000000000..5381de525 --- /dev/null +++ b/src/ProxyManager/ProxyGenerator/LazyLoadingGhost/MethodGenerator/SkipDestructor.php @@ -0,0 +1,29 @@ +setBody( + '$this->' . $initializerProperty->getName() . ' || parent::__destruct();' + ); + } +} diff --git a/src/ProxyManager/ProxyGenerator/LazyLoadingGhostGenerator.php b/src/ProxyManager/ProxyGenerator/LazyLoadingGhostGenerator.php index 7ec491714..a83b7f926 100644 --- a/src/ProxyManager/ProxyGenerator/LazyLoadingGhostGenerator.php +++ b/src/ProxyManager/ProxyGenerator/LazyLoadingGhostGenerator.php @@ -25,6 +25,7 @@ use ProxyManager\ProxyGenerator\LazyLoadingGhost\MethodGenerator\MagicSleep; use ProxyManager\ProxyGenerator\LazyLoadingGhost\MethodGenerator\MagicUnset; use ProxyManager\ProxyGenerator\LazyLoadingGhost\MethodGenerator\SetProxyInitializer; +use ProxyManager\ProxyGenerator\LazyLoadingGhost\MethodGenerator\SkipDestructor; use ProxyManager\ProxyGenerator\LazyLoadingGhost\PropertyGenerator\InitializationTracker; use ProxyManager\ProxyGenerator\LazyLoadingGhost\PropertyGenerator\InitializerProperty; use ProxyManager\ProxyGenerator\LazyLoadingGhost\PropertyGenerator\PrivatePropertiesMap; @@ -48,7 +49,7 @@ class LazyLoadingGhostGenerator implements ProxyGeneratorInterface /** * {@inheritDoc} * - * @psalm-param array{skippedProperties?: array} $proxyOptions + * @psalm-param array{skippedProperties?: array, skipDestructor?: bool} $proxyOptions * * @return void * @@ -65,6 +66,7 @@ public function generate(ReflectionClass $originalClass, ClassGenerator $classGe $publicProperties = new PublicPropertiesMap($filteredProperties); $privateProperties = new PrivatePropertiesMap($filteredProperties); $protectedProperties = new ProtectedPropertiesMap($filteredProperties); + $skipDestructor = ($proxyOptions['skipDestructor'] ?? false) && $originalClass->hasMethod('__destruct'); $classGenerator->setExtendedClass($originalClass->getName()); $classGenerator->setImplementedInterfaces([GhostObjectInterface::class]); @@ -81,7 +83,7 @@ static function (MethodGenerator $generatedMethod) use ($originalClass, $classGe ClassGeneratorUtils::addMethodIfNotFinal($originalClass, $classGenerator, $generatedMethod); }, array_merge( - $this->getAbstractProxiedMethods($originalClass), + $this->getAbstractProxiedMethods($originalClass, $skipDestructor), [ $init, new StaticProxyConstructor($initializer, $filteredProperties), @@ -124,7 +126,8 @@ static function (MethodGenerator $generatedMethod) use ($originalClass, $classGe new GetProxyInitializer($initializer), new InitializeProxy($initializer, $init), new IsProxyInitialized($initializer), - ] + ], + $skipDestructor ? [new SkipDestructor($initializer)] : [] ) ); } @@ -134,8 +137,22 @@ static function (MethodGenerator $generatedMethod) use ($originalClass, $classGe * * @return MethodGenerator[] */ - private function getAbstractProxiedMethods(ReflectionClass $originalClass): array + private function getAbstractProxiedMethods(ReflectionClass $originalClass, bool $skipDestructor): array { + $excludedMethods = [ + '__get', + '__set', + '__isset', + '__unset', + '__clone', + '__sleep', + '__wakeup', + ]; + + if ($skipDestructor) { + $excludedMethods[] = '__destruct'; + } + return array_map( static function (ReflectionMethod $method): ProxyManagerMethodGenerator { $generated = ProxyManagerMethodGenerator::fromReflectionWithoutBodyAndDocBlock( @@ -146,7 +163,7 @@ static function (ReflectionMethod $method): ProxyManagerMethodGenerator { return $generated; }, - ProxiedMethodsFilter::getAbstractProxiedMethods($originalClass) + ProxiedMethodsFilter::getAbstractProxiedMethods($originalClass, $excludedMethods) ); } } diff --git a/src/ProxyManager/ProxyGenerator/LazyLoadingValueHolder/MethodGenerator/SkipDestructor.php b/src/ProxyManager/ProxyGenerator/LazyLoadingValueHolder/MethodGenerator/SkipDestructor.php new file mode 100644 index 000000000..7c9498c0f --- /dev/null +++ b/src/ProxyManager/ProxyGenerator/LazyLoadingValueHolder/MethodGenerator/SkipDestructor.php @@ -0,0 +1,32 @@ +getName(); + $valueHolder = $valueHolderProperty->getName(); + + $this->setBody( + '$this->' . $initializer . ' || $this->' . $valueHolder . '->__destruct();' + ); + } +} diff --git a/src/ProxyManager/ProxyGenerator/LazyLoadingValueHolderGenerator.php b/src/ProxyManager/ProxyGenerator/LazyLoadingValueHolderGenerator.php index e6f2d0416..c6802c90a 100644 --- a/src/ProxyManager/ProxyGenerator/LazyLoadingValueHolderGenerator.php +++ b/src/ProxyManager/ProxyGenerator/LazyLoadingValueHolderGenerator.php @@ -25,6 +25,7 @@ use ProxyManager\ProxyGenerator\LazyLoadingValueHolder\MethodGenerator\MagicSleep; use ProxyManager\ProxyGenerator\LazyLoadingValueHolder\MethodGenerator\MagicUnset; use ProxyManager\ProxyGenerator\LazyLoadingValueHolder\MethodGenerator\SetProxyInitializer; +use ProxyManager\ProxyGenerator\LazyLoadingValueHolder\MethodGenerator\SkipDestructor; use ProxyManager\ProxyGenerator\LazyLoadingValueHolder\PropertyGenerator\InitializerProperty; use ProxyManager\ProxyGenerator\LazyLoadingValueHolder\PropertyGenerator\ValueHolderProperty; use ProxyManager\ProxyGenerator\PropertyGenerator\PublicPropertiesMap; @@ -48,12 +49,14 @@ class LazyLoadingValueHolderGenerator implements ProxyGeneratorInterface /** * {@inheritDoc} * + * @psalm-param array{skipDestructor?: bool} $proxyOptions + * * @return void * * @throws InvalidProxiedClassException * @throws InvalidArgumentException */ - public function generate(ReflectionClass $originalClass, ClassGenerator $classGenerator) + public function generate(ReflectionClass $originalClass, ClassGenerator $classGenerator, array $proxyOptions = []) { CanProxyAssertion::assertClassCanBeProxied($originalClass); @@ -71,6 +74,21 @@ public function generate(ReflectionClass $originalClass, ClassGenerator $classGe $classGenerator->addPropertyFromGenerator($initializer = new InitializerProperty()); $classGenerator->addPropertyFromGenerator($publicProperties); + $skipDestructor = ($proxyOptions['skipDestructor'] ?? false) && $originalClass->hasMethod('__destruct'); + $excludedMethods = [ + '__get', + '__set', + '__isset', + '__unset', + '__clone', + '__sleep', + '__wakeup', + ]; + + if ($skipDestructor) { + $excludedMethods[] = '__destruct'; + } + array_map( static function (MethodGenerator $generatedMethod) use ($originalClass, $classGenerator): void { ClassGeneratorUtils::addMethodIfNotFinal($originalClass, $classGenerator, $generatedMethod); @@ -78,7 +96,7 @@ static function (MethodGenerator $generatedMethod) use ($originalClass, $classGe array_merge( array_map( $this->buildLazyLoadingMethodInterceptor($initializer, $valueHolder), - ProxiedMethodsFilter::getProxiedMethods($originalClass) + ProxiedMethodsFilter::getProxiedMethods($originalClass, $excludedMethods) ), [ new StaticProxyConstructor($initializer, Properties::fromReflectionClass($originalClass)), @@ -95,7 +113,8 @@ static function (MethodGenerator $generatedMethod) use ($originalClass, $classGe new InitializeProxy($initializer, $valueHolder), new IsProxyInitialized($valueHolder), new GetWrappedValueHolderValue($valueHolder), - ] + ], + $skipDestructor ? [new SkipDestructor($initializer, $valueHolder)] : [] ) ); } diff --git a/tests/language-feature-scripts/lazy-loading-ghost-skip-destructor.phpt b/tests/language-feature-scripts/lazy-loading-ghost-skip-destructor.phpt new file mode 100644 index 000000000..dfebd9317 --- /dev/null +++ b/tests/language-feature-scripts/lazy-loading-ghost-skip-destructor.phpt @@ -0,0 +1,39 @@ +--TEST-- +Verifies that generated lazy loading ghost objects can skip calling the proxied destructor +--FILE-- +createProxy(Destructable::class, $init, ['skipDestructor' => true]); +echo "NO __destruct\n"; +unset($proxy); + +$proxy = $factory->createProxy(Destructable::class, $init, ['skipDestructor' => true]); +echo 'DO '; +$proxy->triggerInit = true; +unset($proxy); + +$proxy = $factory->createProxy(Destructable::class, $init); +echo "\nDO "; +unset($proxy); +?> +--EXPECT-- +NO __destruct +DO init__destruct +DO __destruct diff --git a/tests/language-feature-scripts/lazy-loading-value-holder-skip-destructor.phpt b/tests/language-feature-scripts/lazy-loading-value-holder-skip-destructor.phpt new file mode 100644 index 000000000..66e5072c6 --- /dev/null +++ b/tests/language-feature-scripts/lazy-loading-value-holder-skip-destructor.phpt @@ -0,0 +1,40 @@ +--TEST-- +Verifies that generated lazy loading value holders can skip calling the proxied destructor +--FILE-- +createProxy(Destructable::class, $init, ['skipDestructor' => true]); +echo "NO __destruct\n"; +unset($proxy); + +$proxy = $factory->createProxy(Destructable::class, $init, ['skipDestructor' => true]); +echo "DO "; +$proxy->triggerInit = true; +unset($proxy); + +$proxy = $factory->createProxy(Destructable::class, $init); +echo "\nDO "; +unset($proxy); +?> +--EXPECT-- +NO __destruct +DO init__destruct__destruct +DO init__destruct__destruct