From 6bcce0a41529871780f0e4d150170fca09dfc53a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Letord?= Date: Wed, 21 Feb 2024 14:57:26 +0100 Subject: [PATCH 1/4] Generate Access Interceptor Proxy --- .../Symfony/CacheWarmer/ProxyCacheWarmer.php | 44 ++++++ .../Symfony/Config/services.php | 7 + .../Compiler/ServiceProxyPass.php | 23 +-- .../AccessInterceptorGenerator.php | 73 ++++++++++ .../Factory/AccessInterceptorFactory.php | 58 ++++++++ .../Method/InterceptedMethod.php | 9 +- .../Method/InterceptorGenerator.php | 88 ++++++++++++ .../Method/SetMethodPrefixInterceptors.php | 32 +++++ .../Method/SetMethodSuffixInterceptors.php | 35 +++++ .../AccessInterceptorValueHolderGenerator.php | 133 ------------------ .../AccessInterceptorValueHolderFactory.php | 87 ------------ src/Generator/Method/InterceptorGenerator.php | 99 ------------- src/Model/Request/Instance.php | 21 +-- src/Proxy/AccessInterceptorsInterface.php | 15 ++ src/ProxyFactory.php | 45 ++++-- .../Stub/Cache/ClassWithCacheAttributes.php | 7 + .../ClassWithInvalidateCacheAttributes.php | 2 + .../ClassWithAnnotationOnPrivateMethod.php | 14 ++ tests/Double/Stub/ClassWithFinalMethod.php | 14 ++ tests/Double/Stub/FinalClass.php | 11 ++ tests/Interceptor/AbstractInterceptorTest.php | 2 +- tests/Interceptor/CacheInterceptorTest.php | 47 ++++--- tests/Interceptor/EventInterceptorTest.php | 4 +- .../InvalidateCacheInterceptorTest.php | 2 +- .../LegacyCacheInterceptorTest.php | 4 +- tests/Interceptor/SecurityInterceptorTest.php | 2 +- tests/Interceptor/TestCodeTemplate.php | 5 +- tests/ProxyFactoryTest.php | 49 +++++-- tests/ProxyTestTrait.php | 21 +-- 29 files changed, 539 insertions(+), 414 deletions(-) create mode 100644 src/FrameworkBridge/Symfony/CacheWarmer/ProxyCacheWarmer.php create mode 100644 src/Generator/AccessInterceptorGenerator/AccessInterceptorGenerator.php create mode 100644 src/Generator/AccessInterceptorGenerator/Factory/AccessInterceptorFactory.php rename src/Generator/{ => AccessInterceptorGenerator}/Method/InterceptedMethod.php (78%) create mode 100644 src/Generator/AccessInterceptorGenerator/Method/InterceptorGenerator.php create mode 100644 src/Generator/AccessInterceptorGenerator/Method/SetMethodPrefixInterceptors.php create mode 100644 src/Generator/AccessInterceptorGenerator/Method/SetMethodSuffixInterceptors.php delete mode 100644 src/Generator/AccessInterceptorValueHolderGenerator.php delete mode 100644 src/Generator/Factory/AccessInterceptorValueHolderFactory.php delete mode 100644 src/Generator/Method/InterceptorGenerator.php create mode 100644 src/Proxy/AccessInterceptorsInterface.php create mode 100644 tests/Double/Stub/ClassWithAnnotationOnPrivateMethod.php create mode 100644 tests/Double/Stub/ClassWithFinalMethod.php create mode 100644 tests/Double/Stub/FinalClass.php diff --git a/src/FrameworkBridge/Symfony/CacheWarmer/ProxyCacheWarmer.php b/src/FrameworkBridge/Symfony/CacheWarmer/ProxyCacheWarmer.php new file mode 100644 index 0000000..f099d21 --- /dev/null +++ b/src/FrameworkBridge/Symfony/CacheWarmer/ProxyCacheWarmer.php @@ -0,0 +1,44 @@ +proxies = $proxies; + } + + /** + * {@inheritdoc} + */ + public function warmUp(string $cacheDir) + { + foreach ($this->proxies as $proxy) { + if ($proxy instanceof LazyLoadingInterface && !$proxy->isProxyInitialized()) { + $proxy->initializeProxy(); + } + + if (class_exists(LazyObjectInterface::class) + && $proxy instanceof LazyObjectInterface + && !$proxy->isLazyObjectInitialized()) { + $proxy->initializeLazyObject(); + } + } + } + + /** + * {@inheritdoc} + */ + public function isOptional() + { + return true; + } +} diff --git a/src/FrameworkBridge/Symfony/Config/services.php b/src/FrameworkBridge/Symfony/Config/services.php index 338854d..4a305e3 100644 --- a/src/FrameworkBridge/Symfony/Config/services.php +++ b/src/FrameworkBridge/Symfony/Config/services.php @@ -9,6 +9,7 @@ use OpenClassrooms\ServiceProxy\ProxyFactory; use OpenClassrooms\ServiceProxy\ProxyFactoryConfiguration; use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator; +use OpenClassrooms\ServiceProxy\FrameworkBridge\Symfony\CacheWarmer\ProxyCacheWarmer; use function Symfony\Component\DependencyInjection\Loader\Configurator\inline_service; use function Symfony\Component\DependencyInjection\Loader\Configurator\param; use function Symfony\Component\DependencyInjection\Loader\Configurator\service; @@ -74,4 +75,10 @@ service('annotation_reader')->nullOnInvalid(), ]) ->tag('kernel.event_subscriber'); + + $services->set(ProxyCacheWarmer::class) + ->args([ + tagged_iterator('openclassrooms.service_proxy') + ]) + ->tag('kernel.cache_warmer', ['priority' => 128]); }; diff --git a/src/FrameworkBridge/Symfony/DependencyInjection/Compiler/ServiceProxyPass.php b/src/FrameworkBridge/Symfony/DependencyInjection/Compiler/ServiceProxyPass.php index 4c066c3..ad7bd3b 100644 --- a/src/FrameworkBridge/Symfony/DependencyInjection/Compiler/ServiceProxyPass.php +++ b/src/FrameworkBridge/Symfony/DependencyInjection/Compiler/ServiceProxyPass.php @@ -30,23 +30,26 @@ private function buildServiceProxies(): void $serviceProxyIds = []; $taggedServices = $this->container->findTaggedServiceIds('openclassrooms.service_proxy'); foreach ($taggedServices as $taggedServiceId => $tagParameters) { - $this->buildServiceProxyFactoryDefinition($taggedServiceId); + $this->overrideServiceDefinition($taggedServiceId); $serviceProxyIds[] = $taggedServiceId; - $this->compiler->log($this, "Add proxy for {$taggedServiceId} service."); + $this->compiler->log($this, "Override service definition for {$taggedServiceId} service."); } $this->container->setParameter('openclassrooms.service_proxy.service_proxy_ids', $serviceProxyIds); } - private function buildServiceProxyFactoryDefinition(string $taggedServiceName): void + private function overrideServiceDefinition(string $taggedServiceName): void { $definition = $this->container->findDefinition($taggedServiceName); - $factoryDefinition = new Definition($definition->getClass()); - $factoryDefinition->setFactory([new Reference(ProxyFactory::class), 'createProxy']); - $factoryDefinition->setArguments([$definition]); - $this->container->setDefinition($taggedServiceName, $factoryDefinition); - $factoryDefinition->setPublic($definition->isPublic()); - $factoryDefinition->setLazy($definition->isLazy()); - $factoryDefinition->setTags($definition->getTags()); + + if ($definition->getFactory() !== null) { + $this->compiler->log($this, "Service {$taggedServiceName} is not compatible with service proxy"); + + return; + } + + $definition->setFactory([new Reference(ProxyFactory::class), 'createInstance']); + + $definition->setArguments([$definition->getClass(), ...$definition->getArguments()]); } } diff --git a/src/Generator/AccessInterceptorGenerator/AccessInterceptorGenerator.php b/src/Generator/AccessInterceptorGenerator/AccessInterceptorGenerator.php new file mode 100644 index 0000000..de34a3d --- /dev/null +++ b/src/Generator/AccessInterceptorGenerator/AccessInterceptorGenerator.php @@ -0,0 +1,73 @@ +} $proxyOptions + * + * @return void + * + * @throws \InvalidArgumentException + * @throws InvalidProxiedClassException + */ + public function generate(\ReflectionClass $originalClass, ClassGenerator $classGenerator, array $proxyOptions = []) + { + if (!array_key_exists('methods', $proxyOptions)) { + throw new \InvalidArgumentException(sprintf("Generator %s needs a methods proxyOptions",__CLASS__)); + } + + CanProxyAssertion::assertClassCanBeProxied($originalClass, false); + + $classGenerator->setExtendedClass($originalClass->getName()); + $classGenerator->setImplementedInterfaces([AccessInterceptorsInterface::class]); + $classGenerator->addPropertyFromGenerator($prefixInterceptors = new MethodPrefixInterceptors()); + $classGenerator->addPropertyFromGenerator($suffixInterceptors = new MethodSuffixInterceptors()); + + array_map( + static function (MethodGenerator $generatedMethod) use ($originalClass, $classGenerator): void { + ClassGeneratorUtils::addMethodIfNotFinal($originalClass, $classGenerator, $generatedMethod); + }, + array_merge( + array_map( + $this->buildMethodInterceptor($prefixInterceptors, $suffixInterceptors), + array_map(static fn (string $method) => $originalClass->getMethod($method), $proxyOptions['methods']) + ), + [ + new SetMethodPrefixInterceptors($prefixInterceptors), + new SetMethodSuffixInterceptors($suffixInterceptors), + ] + ) + ); + } + + private function buildMethodInterceptor( + MethodPrefixInterceptors $prefixInterceptors, + MethodSuffixInterceptors $suffixInterceptors + ): callable { + return static function (\ReflectionMethod $method) use ($prefixInterceptors, $suffixInterceptors): InterceptedMethod { + return InterceptedMethod::generateMethod( + new MethodReflection($method->getDeclaringClass()->getName(), $method->getName()), + $prefixInterceptors, + $suffixInterceptors + ); + }; + } +} diff --git a/src/Generator/AccessInterceptorGenerator/Factory/AccessInterceptorFactory.php b/src/Generator/AccessInterceptorGenerator/Factory/AccessInterceptorFactory.php new file mode 100644 index 0000000..882d913 --- /dev/null +++ b/src/Generator/AccessInterceptorGenerator/Factory/AccessInterceptorFactory.php @@ -0,0 +1,58 @@ +generator = new AccessInterceptorGenerator(); + } + + /** + * @template T of object + * + * @param class-string $class + * @param array $args + * @param array $prefixInterceptors an array (indexed by method name) of interceptor closures to be called + * before method logic is executed + * @param array $suffixInterceptors an array (indexed by method name) of interceptor closures to be called + * after method logic is executed + * + * @return T + */ + public function createInstance( + string $class, + array $args, + array $prefixInterceptors = [], + array $suffixInterceptors = [] + ): object { + $methods = array_merge( + array_keys($prefixInterceptors), + array_keys($suffixInterceptors) + ); + $methods = array_unique($methods); + + $proxyClassName = $this->generateProxy($class, ['methods' => $methods]); + + $instance = new $proxyClassName(...$args); + + $instance->setPrefixInterceptors($prefixInterceptors); + $instance->setSuffixInterceptors($suffixInterceptors); + + return $instance; + } + + public function getGenerator(): AccessInterceptorGenerator + { + return $this->generator; + } +} diff --git a/src/Generator/Method/InterceptedMethod.php b/src/Generator/AccessInterceptorGenerator/Method/InterceptedMethod.php similarity index 78% rename from src/Generator/Method/InterceptedMethod.php rename to src/Generator/AccessInterceptorGenerator/Method/InterceptedMethod.php index 33d1964..11772cf 100644 --- a/src/Generator/Method/InterceptedMethod.php +++ b/src/Generator/AccessInterceptorGenerator/Method/InterceptedMethod.php @@ -2,12 +2,13 @@ declare(strict_types=1); -namespace OpenClassrooms\ServiceProxy\Generator\Method; +namespace OpenClassrooms\ServiceProxy\Generator\AccessInterceptorGenerator\Method; use Laminas\Code\Generator\Exception\InvalidArgumentException; use Laminas\Code\Generator\PropertyGenerator; use Laminas\Code\Reflection\MethodReflection; +use OpenClassrooms\ServiceProxy\Generator\AccessInterceptorGenerator\Method\InterceptorGenerator; use ProxyManager\Generator\MethodGenerator; /** @@ -20,11 +21,10 @@ final class InterceptedMethod extends MethodGenerator */ public static function generateMethod( MethodReflection $originalMethod, - PropertyGenerator $valueHolderProperty, PropertyGenerator $prefixInterceptors, PropertyGenerator $suffixInterceptors ): self { - $method = static::fromReflectionWithoutBodyAndDocBlock($originalMethod); + $method = static::fromReflectionWithoutBodyAndDocBlock($originalMethod); $forwardedParams = []; foreach ($originalMethod->getParameters() as $parameter) { @@ -32,10 +32,9 @@ public static function generateMethod( } $method->setBody(InterceptorGenerator::createInterceptedMethodBody( - '$returnValue = $this->' . $valueHolderProperty->getName() . '->' + '$returnValue = parent::' . $originalMethod->getName() . '(' . implode(', ', $forwardedParams) . ');', $method, - $valueHolderProperty, $prefixInterceptors, $suffixInterceptors, $originalMethod diff --git a/src/Generator/AccessInterceptorGenerator/Method/InterceptorGenerator.php b/src/Generator/AccessInterceptorGenerator/Method/InterceptorGenerator.php new file mode 100644 index 0000000..af08101 --- /dev/null +++ b/src/Generator/AccessInterceptorGenerator/Method/InterceptorGenerator.php @@ -0,0 +1,88 @@ +{{$prefixInterceptorsName}}[{{$name}}])) { + $returnEarly = false; + $prefixReturnValue = $this->{{$prefixInterceptorsName}}[{{$name}}]->__invoke($this, $this, {{$name}}, {{$paramsString}}, $returnEarly); + + if ($returnEarly) { + {{$prefixEarlyReturnExpression}} + } + } + + $exception = null; + try { + {{$methodBody}} + } catch (\Exception $e) { + $exception = $e; + } + + if (isset($this->{{$suffixInterceptorsName}}[{{$name}}])) { + $returnEarly = false; + $suffixReturnValue = $this->{{$suffixInterceptorsName}}[{{$name}}]->__invoke($this, $this, {{$name}}, {{$paramsString}}, $exception ?? $returnValue, $returnEarly); + + if ($returnEarly) { + {{$suffixEarlyReturnExpression}} + } + } + + if ($exception) { + throw $exception; + } + + {{$returnExpression}} + PHP; + + /** + * @param string $methodBody the body of the previously generated code. + * It MUST assign the return value to a variable + * `$returnValue` instead of directly returning + */ + public static function createInterceptedMethodBody( + string $methodBody, + MethodGenerator $method, + PropertyGenerator $prefixInterceptors, + PropertyGenerator $suffixInterceptors, + ?\ReflectionMethod $originalMethod + ): string { + $replacements = [ + '{{$name}}' => var_export($method->getName(), true), + '{{$prefixInterceptorsName}}' => $prefixInterceptors->getName(), + '{{$prefixEarlyReturnExpression}}' => ProxiedMethodReturnExpression::generate('$prefixReturnValue', $originalMethod), + '{{$methodBody}}' => $methodBody, + '{{$suffixInterceptorsName}}' => $suffixInterceptors->getName(), + '{{$suffixEarlyReturnExpression}}' => ProxiedMethodReturnExpression::generate('$suffixReturnValue', $originalMethod), + '{{$returnExpression}}' => ProxiedMethodReturnExpression::generate('$returnValue', $originalMethod), + '{{$paramsString}}' => 'array(' . implode(', ', array_map( + static function (ParameterGenerator $parameter): string { + return var_export($parameter->getName(), true) . ' => ' . ($parameter->getPassedByReference() ? '&$' : '$') . $parameter->getName(); + }, + $method->getParameters() + )) + . ')', + ]; + + return str_replace( + array_keys($replacements), + $replacements, + self::TEMPLATE + ); + } +} diff --git a/src/Generator/AccessInterceptorGenerator/Method/SetMethodPrefixInterceptors.php b/src/Generator/AccessInterceptorGenerator/Method/SetMethodPrefixInterceptors.php new file mode 100644 index 0000000..700da7e --- /dev/null +++ b/src/Generator/AccessInterceptorGenerator/Method/SetMethodPrefixInterceptors.php @@ -0,0 +1,32 @@ +setType('array'); + $this->setParameter($interceptors); + $this->setReturnType('void'); + $this->setBody('$this->' . $prefixInterceptor->getName() . ' = $prefixInterceptors;'); + } +} diff --git a/src/Generator/AccessInterceptorGenerator/Method/SetMethodSuffixInterceptors.php b/src/Generator/AccessInterceptorGenerator/Method/SetMethodSuffixInterceptors.php new file mode 100644 index 0000000..24d5551 --- /dev/null +++ b/src/Generator/AccessInterceptorGenerator/Method/SetMethodSuffixInterceptors.php @@ -0,0 +1,35 @@ +setType('array'); + $this->setParameter($interceptors); + $this->setReturnType('void'); + $this->setBody('$this->' . $suffixInterceptor->getName() . ' = $suffixInterceptors;'); + } +} diff --git a/src/Generator/AccessInterceptorValueHolderGenerator.php b/src/Generator/AccessInterceptorValueHolderGenerator.php deleted file mode 100644 index cd47cac..0000000 --- a/src/Generator/AccessInterceptorValueHolderGenerator.php +++ /dev/null @@ -1,133 +0,0 @@ -isInterface()) { - $interfaces[] = $originalClass->getName(); - } else { - $classGenerator->setExtendedClass($originalClass->getName()); - } - - $classGenerator->setImplementedInterfaces($interfaces); - $classGenerator->addPropertyFromGenerator($valueHolder = new ValueHolderProperty($originalClass)); - $classGenerator->addPropertyFromGenerator($prefixInterceptors = new MethodPrefixInterceptors()); - $classGenerator->addPropertyFromGenerator($suffixInterceptors = new MethodSuffixInterceptors()); - $classGenerator->addPropertyFromGenerator($publicProperties); - - array_map( - static function (MethodGenerator $generatedMethod) use ($originalClass, $classGenerator): void { - ClassGeneratorUtils::addMethodIfNotFinal($originalClass, $classGenerator, $generatedMethod); - }, - array_merge( - array_map( - $this->buildMethodInterceptor($prefixInterceptors, $suffixInterceptors, $valueHolder), - ProxiedMethodsFilter::getProxiedMethods($originalClass) - ), - [ - Constructor::generateMethod($originalClass, $valueHolder), - new StaticProxyConstructor($originalClass, $valueHolder, $prefixInterceptors, $suffixInterceptors), - new GetWrappedValueHolderValue($valueHolder), - new SetMethodPrefixInterceptor($prefixInterceptors), - new SetMethodSuffixInterceptor($suffixInterceptors), - new MagicGet( - $originalClass, - $valueHolder, - $prefixInterceptors, - $suffixInterceptors, - $publicProperties - ), - new MagicSet( - $originalClass, - $valueHolder, - $prefixInterceptors, - $suffixInterceptors, - $publicProperties - ), - new MagicIsset( - $originalClass, - $valueHolder, - $prefixInterceptors, - $suffixInterceptors, - $publicProperties - ), - new MagicUnset( - $originalClass, - $valueHolder, - $prefixInterceptors, - $suffixInterceptors, - $publicProperties - ), - new MagicClone($originalClass, $valueHolder, $prefixInterceptors, $suffixInterceptors), - new MagicSleep($originalClass, $valueHolder), - new MagicWakeup($originalClass), - ] - ) - ); - } - - private function buildMethodInterceptor( - MethodPrefixInterceptors $prefixes, - MethodSuffixInterceptors $suffixes, - ValueHolderProperty $valueHolder - ): callable { - return static function (\ReflectionMethod $method) use ($prefixes, $suffixes, $valueHolder): InterceptedMethod { - return InterceptedMethod::generateMethod( - new MethodReflection($method->getDeclaringClass()->getName(), $method->getName()), - $valueHolder, - $prefixes, - $suffixes - ); - }; - } -} diff --git a/src/Generator/Factory/AccessInterceptorValueHolderFactory.php b/src/Generator/Factory/AccessInterceptorValueHolderFactory.php deleted file mode 100644 index 3e10a84..0000000 --- a/src/Generator/Factory/AccessInterceptorValueHolderFactory.php +++ /dev/null @@ -1,87 +0,0 @@ -generator = new AccessInterceptorValueHolderGenerator(); - } - - /** - * @param object $instance the object to be wrapped within the value holder - * @param array $prefixInterceptors an array (indexed by method name) of interceptor closures to be called - * before method logic is executed - * @param array $suffixInterceptors an array (indexed by method name) of interceptor closures to be called - * after method logic is executed - * - * @throws InvalidSignatureException - * @throws MissingSignatureException - * @throws \OutOfBoundsException - * - * @psalm-template RealObjectType of object - * - * @psalm-param RealObjectType $instance - * @psalm-param array=, - * RealObjectType=, - * string=, - * array=, - * bool= - * ) : mixed> $prefixInterceptors - * @psalm-param array=, - * RealObjectType=, - * string=, - * array=, - * mixed=, - * bool= - * ) : mixed> $suffixInterceptors - * - * @psalm-return RealObjectType&AccessInterceptorInterface&ValueHolderInterface&AccessInterceptorValueHolderInterface - * - * @psalm-suppress MixedInferredReturnType We ignore type checks here, since `staticProxyConstructor` is not - * interfaced (by design) - */ - public function createProxy( - object $instance, - array $prefixInterceptors = [], - array $suffixInterceptors = [] - ): AccessInterceptorValueHolderInterface { - $proxyClassName = $this->generateProxy(\get_class($instance)); - - /** - * We ignore type checks here, since `staticProxyConstructor` is not interfaced (by design) - * - * @psalm-suppress MixedMethodCall - * @psalm-suppress MixedReturnStatement - * @noinspection PhpUndefinedMethodInspection - */ - return $proxyClassName::staticProxyConstructor($instance, $prefixInterceptors, $suffixInterceptors); - } - - protected function getGenerator(): ProxyGeneratorInterface - { - return $this->generator; - } -} diff --git a/src/Generator/Method/InterceptorGenerator.php b/src/Generator/Method/InterceptorGenerator.php deleted file mode 100644 index ea06c0a..0000000 --- a/src/Generator/Method/InterceptorGenerator.php +++ /dev/null @@ -1,99 +0,0 @@ -{{$prefixInterceptorsName}}[{{$name}}])) { - $returnEarly = false; - $prefixReturnValue = $this->{{$prefixInterceptorsName}}[{{$name}}]->__invoke($this, $this->{{$valueHolderName}}, {{$name}}, {{$paramsString}}, $returnEarly); - - if ($returnEarly) { - {{$returnEarlyPrefixExpression}} - } -} - -$exception = null; -try { - {{$methodBody}} -} catch (\Exception $e) { - $exception = $e; -} - -if (isset($this->{{$suffixInterceptorsName}}[{{$name}}])) { - $returnEarly = false; - $suffixReturnValue = $this->{{$suffixInterceptorsName}}[{{$name}}]->__invoke($this, $this->{{$valueHolderName}}, {{$name}}, {{$paramsString}}, $exception ?? $returnValue, $returnEarly); - - if ($returnEarly) { - {{$returnEarlySuffixExpression}} - } -} - -if ($exception) { - throw $exception; -} - -{{$returnExpression}} -PHP; - - /** - * @param string $methodBody the body of the previously generated code. - * It MUST assign the return value to a variable - * `$returnValue` instead of directly returning - */ - public static function createInterceptedMethodBody( - string $methodBody, - MethodGenerator $method, - PropertyGenerator $valueHolder, - PropertyGenerator $prefixInterceptors, - PropertyGenerator $suffixInterceptors, - ?\ReflectionMethod $originalMethod - ): string { - $name = var_export($method->getName(), true); - $valueHolderName = $valueHolder->getName(); - $prefixInterceptorsName = $prefixInterceptors->getName(); - $suffixInterceptorsName = $suffixInterceptors->getName(); - $params = []; - - foreach ($method->getParameters() as $parameter) { - $parameterName = $parameter->getName(); - $params[] = var_export($parameterName, true) . ' => $' . $parameter->getName(); - } - - $paramsString = 'array(' . implode(', ', $params) . ')'; - - $replacements = [ - '{{$prefixInterceptorsName}}' => $prefixInterceptorsName, - '{{$name}}' => $name, - '{{$valueHolderName}}' => $valueHolderName, - '{{$paramsString}}' => $paramsString, - '{{$returnEarlyPrefixExpression}}' => ProxiedMethodReturnExpression::generate( - '$prefixReturnValue', - $originalMethod - ), - '{{$methodBody}}' => $methodBody, - '{{$suffixInterceptorsName}}' => $suffixInterceptorsName, - '{{$returnEarlySuffixExpression}}' => ProxiedMethodReturnExpression::generate( - '$suffixReturnValue', - $originalMethod - ), - '{{$returnExpression}}' => ProxiedMethodReturnExpression::generate('$returnValue', $originalMethod), - - ]; - - return str_replace(array_keys($replacements), $replacements, self::TEMPLATE); - } -} diff --git a/src/Model/Request/Instance.php b/src/Model/Request/Instance.php index 8a44678..c0ee782 100644 --- a/src/Model/Request/Instance.php +++ b/src/Model/Request/Instance.php @@ -11,9 +11,7 @@ final class Instance { private Method $method; - private object $object; - - private \ReflectionObject $reflection; + private \ReflectionClass $reflection; private function __construct() { @@ -26,13 +24,13 @@ private function __construct() * @throws AnnotationException */ public static function createFromMethod( - object $object, + string $class, string $methodName, ?array $parameters = null, mixed $response = null ): self { $annotationReader = new AnnotationReader(); - $reflection = new \ReflectionObject($object); + $reflection = new \ReflectionClass($class); $methodRef = $reflection->getMethod($methodName); $annotations = $annotationReader->getMethodAnnotations($methodRef); $method = Method::create($methodRef, $annotations); @@ -43,7 +41,7 @@ public static function createFromMethod( $method->setResponse($response); } - return self::create($object, $reflection, $method); + return self::create($reflection, $method); } public function getMethod(): Method @@ -52,12 +50,10 @@ public function getMethod(): Method } public static function create( - object $object, - \ReflectionObject $reflection, + \ReflectionClass $reflection, Method $method ): self { $self = new self(); - $self->object = $object; $self->reflection = $reflection; $self->method = $method; @@ -81,12 +77,7 @@ public function setResponse(mixed $response): self return $this; } - public function getObject(): object - { - return $this->object; - } - - public function getReflection(): \ReflectionObject + public function getReflection(): \ReflectionClass { return $this->reflection; } diff --git a/src/Proxy/AccessInterceptorsInterface.php b/src/Proxy/AccessInterceptorsInterface.php new file mode 100644 index 0000000..964422c --- /dev/null +++ b/src/Proxy/AccessInterceptorsInterface.php @@ -0,0 +1,15 @@ + $class + * @param mixed ...$agrs * * @return T */ - public function createProxy(object $object): object + public function createInstance(string $class, ...$args): object { - $instanceRef = new \ReflectionObject($object); - $methods = $instanceRef->getMethods(\ReflectionMethod::IS_PUBLIC); + $instanceRef = new \ReflectionClass($class); + + if ($instanceRef->isFinal()) { + throw new \LogicException(sprintf("Unable to proxify final class %s. Hint: replace final keywords with a @final annotation", $instanceRef->getName())); + } + + $methods = $instanceRef->getMethods(); $interceptionClosures = []; foreach ($methods as $methodRef) { $methodAnnotations = $this->annotationReader->getMethodAnnotations($methodRef); + + if (count($methodAnnotations) === 0 && count($methodRef->getAttributes()) === 0) { + continue; + } + $instance = Instance::create( - $object, $instanceRef, Method::create( $methodRef, @@ -80,6 +92,14 @@ public function createProxy(object $object): object foreach ([PrefixInterceptor::PREFIX_TYPE, SuffixInterceptor::SUFFIX_TYPE] as $type) { $interceptors = $this->filterInterceptors($instance, $type); if (\count($interceptors) > 0) { + if ($methodRef->isFinal()) { + throw new \LogicException(sprintf("Unable to proxify a final method %s on class %s. Hint: replace final keywords with a @final annotation", $methodRef->getName(), $methodRef->getDeclaringClass())); + } + + if ($methodRef->isPrivate()) { + throw new \LogicException(sprintf("Unable to attach an interceptor to a private method %s on class %s.", $methodRef->getName(), $methodRef->getDeclaringClass())); + } + $interceptionClosures[$type][$methodRef->getName()] = $this->getInterceptionClosure( $type, $interceptors, @@ -90,12 +110,13 @@ public function createProxy(object $object): object } if (\count($interceptionClosures) === 0) { - return $object; + return new $class(...$args); } return $this->getInterceptorFactory() - ->createProxy( - $object, + ->createInstance( + $class, + $args, $interceptionClosures[PrefixInterceptor::PREFIX_TYPE] ?? [], $interceptionClosures[SuffixInterceptor::SUFFIX_TYPE] ?? [], ) @@ -252,10 +273,10 @@ private function getInterceptionClosure( }; } - private function getInterceptorFactory(): AccessInterceptorValueHolderFactory + private function getInterceptorFactory(): AccessInterceptorFactory { if ($this->configuration->isEval()) { - return new AccessInterceptorValueHolderFactory(); + return new AccessInterceptorFactory(); } $proxiesDir = $this->configuration->getProxiesDir(); @@ -270,6 +291,6 @@ private function getInterceptorFactory(): AccessInterceptorValueHolderFactory // @phpstan-ignore-next-line spl_autoload_register($conf->getProxyAutoloader()); - return new AccessInterceptorValueHolderFactory($conf); + return new AccessInterceptorFactory($conf); } } diff --git a/tests/Double/Stub/Cache/ClassWithCacheAttributes.php b/tests/Double/Stub/Cache/ClassWithCacheAttributes.php index ab4db92..e5461e4 100644 --- a/tests/Double/Stub/Cache/ClassWithCacheAttributes.php +++ b/tests/Double/Stub/Cache/ClassWithCacheAttributes.php @@ -11,6 +11,13 @@ class ClassWithCacheAttributes { public const DATA = 'data'; + public readonly string $data; + + public function __construct(?string $data = null) + { + $this->data = $data ?? 'hello'; + } + public function methodWithoutAttribute(): bool { return true; diff --git a/tests/Double/Stub/Cache/ClassWithInvalidateCacheAttributes.php b/tests/Double/Stub/Cache/ClassWithInvalidateCacheAttributes.php index 66017f7..b3ff466 100644 --- a/tests/Double/Stub/Cache/ClassWithInvalidateCacheAttributes.php +++ b/tests/Double/Stub/Cache/ClassWithInvalidateCacheAttributes.php @@ -11,6 +11,8 @@ class ClassWithInvalidateCacheAttributes { public const DATA = 'data'; + private $data; + #[Cache(tags: ['"my_tag"'])] public function methodWithTaggedCache(): string { diff --git a/tests/Double/Stub/ClassWithAnnotationOnPrivateMethod.php b/tests/Double/Stub/ClassWithAnnotationOnPrivateMethod.php new file mode 100644 index 0000000..895aba3 --- /dev/null +++ b/tests/Double/Stub/ClassWithAnnotationOnPrivateMethod.php @@ -0,0 +1,14 @@ +expectException(HandlerNotFound::class); $this->getProxyFactory( [new CacheInterceptor($config, [new CacheHandlerMock()])] - )->createProxy(new ClassWithCacheAttributes()) + )->createInstance(ClassWithCacheAttributes::class) ->invalidHandler(); } diff --git a/tests/Interceptor/CacheInterceptorTest.php b/tests/Interceptor/CacheInterceptorTest.php index 2732403..3717889 100644 --- a/tests/Interceptor/CacheInterceptorTest.php +++ b/tests/Interceptor/CacheInterceptorTest.php @@ -53,7 +53,7 @@ protected function tearDown(): void public function testSupportsCacheAttribute(): void { $method = Instance::createFromMethod( - new ClassWithCacheAttributes(), + ClassWithCacheAttributes::class, 'methodWithoutAttribute' ); @@ -61,7 +61,7 @@ public function testSupportsCacheAttribute(): void $this->assertFalse($this->cacheInterceptor->supportsSuffix($method)); $method = Instance::createFromMethod( - new ClassWithCacheAttributes(), + ClassWithCacheAttributes::class, 'methodWithAttribute' ); @@ -72,7 +72,7 @@ public function testSupportsCacheAttribute(): void public function testNotSupportsCacheAnnotation(): void { $method = Instance::createFromMethod( - new LegacyCacheAnnotatedClass(), + LegacyCacheAnnotatedClass::class, 'annotatedMethod' ); @@ -82,14 +82,14 @@ public function testNotSupportsCacheAnnotation(): void public function testMethodWithoutCache(): void { - $proxy = $this->proxyFactory->createProxy(new ClassWithCacheAttributes()); + $proxy = $this->proxyFactory->createInstance(ClassWithCacheAttributes::class); $this->assertEquals(ClassWithCacheAttributes::DATA, $proxy->methodWithAttribute()); } public function testNotInCacheReturnData(): void { - $proxy = $this->proxyFactory->createProxy(new ClassWithCacheAttributes()); + $proxy = $this->proxyFactory->createInstance(ClassWithCacheAttributes::class); $this->assertEquals(ClassWithCacheAttributes::DATA, $proxy->methodWithAttribute()); $this->assertEmpty($this->cacheInterceptor->getHits()); @@ -98,7 +98,7 @@ public function testNotInCacheReturnData(): void public function testInCacheReturnData(): void { - $proxy = $this->proxyFactory->createProxy(new ClassWithCacheAttributes()); + $proxy = $this->proxyFactory->createInstance(ClassWithCacheAttributes::class); $proxy->methodWithAttribute(); $this->assertEmpty($this->cacheInterceptor::getHits()); @@ -113,7 +113,7 @@ public function testInCacheReturnData(): void public function testMethodWithVoidReturnIsNotCached(): void { - $proxy = $this->proxyFactory->createProxy(new ClassWithCacheAttributes()); + $proxy = $this->proxyFactory->createInstance(ClassWithCacheAttributes::class); $proxy->methodWithVoidReturn(); $this->assertEmpty($this->cacheInterceptor->getHits()); @@ -128,7 +128,7 @@ public function testMethodWithVoidReturnIsNotCached(): void public function testCachedMethodWithArguments(): void { - $proxy = $this->proxyFactory->createProxy(new ClassWithCacheAttributes()); + $proxy = $this->proxyFactory->createInstance(ClassWithCacheAttributes::class); $proxy->methodWithArguments('value1', 'value2'); $this->assertEmpty($this->cacheInterceptor->getHits()); @@ -152,7 +152,7 @@ public function testCachedMethodWithArguments(): void public function testOnExceptionDontSave(): void { - $proxy = $this->proxyFactory->createProxy(new ClassWithCacheAttributes()); + $proxy = $this->proxyFactory->createInstance(ClassWithCacheAttributes::class); try { $proxy->methodWithException(); @@ -173,7 +173,7 @@ public function testOnExceptionDontSave(): void public function testWithLifeTimeReturnData(): void { - $proxy = $this->proxyFactory->createProxy(new ClassWithCacheAttributes()); + $proxy = $this->proxyFactory->createInstance(ClassWithCacheAttributes::class); $data = $proxy->methodWithLifetime(); @@ -183,7 +183,7 @@ public function testWithLifeTimeReturnData(): void public function testWithTagsReturnDataAndCanBeInvalidated(): void { - $proxy = $this->proxyFactory->createProxy(new ClassWithCacheAttributes()); + $proxy = $this->proxyFactory->createInstance(ClassWithCacheAttributes::class); $proxy->methodWithTaggedCache(); $this->assertEmpty($this->cacheInterceptor->getHits()); @@ -206,7 +206,7 @@ public function testWithTagsReturnDataAndCanBeInvalidated(): void public function testWithTagsAndParameterReturnDataAndCanBeInvalidated(): void { - $proxy = $this->proxyFactory->createProxy(new ClassWithCacheAttributes()); + $proxy = $this->proxyFactory->createInstance(ClassWithCacheAttributes::class); $proxy->methodWithResolvedTag(new ParameterClassStub()); $this->assertEmpty($this->cacheInterceptor->getHits()); @@ -227,7 +227,7 @@ public function testWithTagsAndParameterReturnDataAndCanBeInvalidated(): void public function testMethodCacheIsAutoTaggedFromResponse(): void { - $proxy = $this->proxyFactory->createProxy(new ClassWithCacheAttributes()); + $proxy = $this->proxyFactory->createInstance(ClassWithCacheAttributes::class); $proxy->methodWithAttributeReturningObject(); $this->assertEmpty($this->cacheInterceptor::getHits()); @@ -251,7 +251,7 @@ public function testMethodCacheIsAutoTaggedFromResponse(): void public function testMethodWithPhpDoc(): void { - $proxy = $this->proxyFactory->createProxy(new ClassWithCacheAttributes()); + $proxy = $this->proxyFactory->createInstance(ClassWithCacheAttributes::class); $proxy->methodWithAttributeAndPhpDoc(); $this->assertEmpty($this->cacheInterceptor::getHits()); @@ -266,35 +266,42 @@ public function testMethodWithPhpDoc(): void public function testUnknownHandlerThrowsException(): void { $this->expectException(HandlerNotFound::class); - $proxy = $this->proxyFactory->createProxy(new ClassWithCacheAttributes()); + $proxy = $this->proxyFactory->createInstance(ClassWithCacheAttributes::class); $proxy->invalidHandler(); } public function testUnknownPoolThrowsException(): void { $this->expectException(\InvalidArgumentException::class); - $proxy = $this->proxyFactory->createProxy(new ClassWithCacheAttributes()); + $proxy = $this->proxyFactory->createInstance(ClassWithCacheAttributes::class); $proxy->invalidPool(); } public function testPool(): void { - $proxy = $this->proxyFactory->createProxy(new ClassWithCacheAttributes()); + $proxy = $this->proxyFactory->createInstance(ClassWithCacheAttributes::class); $this->assertEquals(ClassWithCacheAttributes::DATA, $proxy->methodWithPool()); $this->assertEmpty($this->cacheInterceptor::getHits()); $this->assertNotEmpty($this->cacheInterceptor::getMisses()); } + public function testProperties(): void + { + $proxy = $this->proxyFactory->createInstance(ClassWithCacheAttributes::class, 'world'); + + $this->assertEquals('world', $proxy->data); + } + public function testBothHandlerAndPool(): void { - $proxy = $this->proxyFactory->createProxy(new ClassWithCacheAttributes()); + $proxy = $this->proxyFactory->createInstance(ClassWithCacheAttributes::class); $this->assertEquals(ClassWithCacheAttributes::DATA, $proxy->bothHandlerAndPool()); } public function testMethodWithMultiplePools(): void { - $proxy = $this->proxyFactory->createProxy(new ClassWithCacheAttributes()); + $proxy = $this->proxyFactory->createInstance(ClassWithCacheAttributes::class); $proxy->methodWithMultiplePools(); $this->assertEmpty($this->cacheInterceptor->getHits('foo')); @@ -316,7 +323,7 @@ public function testMethodWithMultiplePools(): void public function testMethodWithNoPoolUsesDefaultPool(): void { - $proxy = $this->proxyFactory->createProxy(new ClassWithCacheAttributes()); + $proxy = $this->proxyFactory->createInstance(ClassWithCacheAttributes::class); $proxy->methodWithAttribute(); $this->assertNotEmpty($this->cacheInterceptor->getMisses('default')); diff --git a/tests/Interceptor/EventInterceptorTest.php b/tests/Interceptor/EventInterceptorTest.php index 600139e..c524e45 100644 --- a/tests/Interceptor/EventInterceptorTest.php +++ b/tests/Interceptor/EventInterceptorTest.php @@ -32,13 +32,13 @@ protected function setUp(): void ), ] ); - $this->proxy = $this->proxyFactory->createProxy(new EventAnnotatedClass()); + $this->proxy = $this->proxyFactory->createInstance(EventAnnotatedClass::class); } public function testInvalidMethodEventThrowException(): void { $this->expectException(\InvalidArgumentException::class); - $proxy = $this->proxyFactory->createProxy(new InvalidMethodEventAnnotatedClass()); + $proxy = $this->proxyFactory->createInstance(InvalidMethodEventAnnotatedClass::class); $proxy->eventWithWrongMethods(); } diff --git a/tests/Interceptor/InvalidateCacheInterceptorTest.php b/tests/Interceptor/InvalidateCacheInterceptorTest.php index 58d0d3f..9c97280 100644 --- a/tests/Interceptor/InvalidateCacheInterceptorTest.php +++ b/tests/Interceptor/InvalidateCacheInterceptorTest.php @@ -42,7 +42,7 @@ protected function setUp(): void $this->invalidateCacheInterceptor, ]); - $this->proxy = $this->proxyFactory->createProxy(new ClassWithInvalidateCacheAttributes()); + $this->proxy = $this->proxyFactory->createInstance(ClassWithInvalidateCacheAttributes::class); } protected function tearDown(): void diff --git a/tests/Interceptor/LegacyCacheInterceptorTest.php b/tests/Interceptor/LegacyCacheInterceptorTest.php index 5b5cd4e..96bc48d 100644 --- a/tests/Interceptor/LegacyCacheInterceptorTest.php +++ b/tests/Interceptor/LegacyCacheInterceptorTest.php @@ -34,13 +34,13 @@ protected function setUp(): void $this->proxyFactory = $this->getProxyFactory([ $this->cacheInterceptor, ]); - $this->proxy = $this->proxyFactory->createProxy(new CacheAnnotatedClass()); + $this->proxy = $this->proxyFactory->createInstance(CacheAnnotatedClass::class); } public function testTooLongIdWithIdThrowException(): void { $this->expectException(AnnotationException::class); - $this->proxyFactory->createProxy(new InvalidIdCacheAnnotatedClass()); + $this->proxyFactory->createInstance(InvalidIdCacheAnnotatedClass::class); } public function testTagsInvalidationThrowException(): void diff --git a/tests/Interceptor/SecurityInterceptorTest.php b/tests/Interceptor/SecurityInterceptorTest.php index ea43d67..2876b64 100644 --- a/tests/Interceptor/SecurityInterceptorTest.php +++ b/tests/Interceptor/SecurityInterceptorTest.php @@ -33,7 +33,7 @@ protected function setUp(): void ), ] ); - $this->proxy = $this->proxyFactory->createProxy(new SecurityAnnotatedClass()); + $this->proxy = $this->proxyFactory->createInstance(SecurityAnnotatedClass::class); } public function testOnlyRoleNotAuthorizedThrowException(): void diff --git a/tests/Interceptor/TestCodeTemplate.php b/tests/Interceptor/TestCodeTemplate.php index 921159f..0e766c8 100644 --- a/tests/Interceptor/TestCodeTemplate.php +++ b/tests/Interceptor/TestCodeTemplate.php @@ -33,11 +33,10 @@ ); $classname = '\\OpenClassrooms\\ServiceProxy\\Tests\\tmp\\' . $argv[1]; -$instance = new $classname(); -$proxy = $proxyFactory->createProxy($instance); +$instance = $proxyFactory->createInstance($classname); -$proxy->execute(); +$instance->execute(); $cacheHit = !empty($cacheInterceptor->getHits()); diff --git a/tests/ProxyFactoryTest.php b/tests/ProxyFactoryTest.php index b47ccbc..573591e 100644 --- a/tests/ProxyFactoryTest.php +++ b/tests/ProxyFactoryTest.php @@ -17,6 +17,9 @@ use OpenClassrooms\ServiceProxy\Tests\Double\Mock\Security\SecurityHandlerMock; use OpenClassrooms\ServiceProxy\Tests\Double\Mock\Transaction\TransactionHandlerMock; use OpenClassrooms\ServiceProxy\Tests\Double\Stub\Cache\ClassWithCacheAttributes; +use OpenClassrooms\ServiceProxy\Tests\Double\Stub\ClassWithAnnotationOnPrivateMethod; +use OpenClassrooms\ServiceProxy\Tests\Double\Stub\ClassWithFinalMethod; +use OpenClassrooms\ServiceProxy\Tests\Double\Stub\FinalClass; use OpenClassrooms\ServiceProxy\Tests\Double\Stub\WithConstructorAnnotationClass; use OpenClassrooms\ServiceProxy\Tests\Double\Stub\WithoutAnnotationClass; use PHPUnit\Framework\TestCase; @@ -39,33 +42,49 @@ protected function setUp(): void ); } - public function testWithoutAnnotationReturnServiceProxyInterface(): void + public function testWithoutAnnotationReturnUnmodifiedObject(): void { - $inputClass = new WithoutAnnotationClass(); - $inputClass->field = true; - $proxy = $this->factory->createProxy($inputClass); + $instance = $this->factory->createInstance(WithoutAnnotationClass::class); + $instance->field = true; - $this->assertTrue($proxy->aMethodWithoutAnnotation()); - $this->assertTrue($proxy->aMethodWithoutServiceProxyAnnotation()); - $this->assertNotProxy($inputClass, $proxy); + $this->assertTrue($instance->aMethodWithoutAnnotation()); + $this->assertTrue($instance->aMethodWithoutServiceProxyAnnotation()); + $this->assertNotProxy(WithoutAnnotationClass::class, $instance); + } + + + public function testThrownExceptionWhenCreateAProxyOnFinalClass(): void + { + $this->expectException(\LogicException::class); + $this->factory->createInstance(FinalClass::class); + } + + public function testThrownExceptionOnFinalMethodInterception(): void + { + $this->expectException(\LogicException::class); + $this->factory->createInstance(ClassWithFinalMethod::class); + } + + public function testThrownExceptionOnPrivateMethodInterception(): void + { + $this->expectException(\LogicException::class); + $this->factory->createInstance(ClassWithAnnotationOnPrivateMethod::class); } public function testWithCacheAnnotationReturnServiceProxyCacheInterface(): void { - $inputClass = new ClassWithCacheAttributes(); - $proxy = $this->factory->createProxy($inputClass); + $instance = $this->factory->createInstance(ClassWithCacheAttributes::class); - $this->assertProxy($inputClass, $proxy); - $this->assertTrue($proxy->methodWithoutAttribute()); + $this->assertProxy(ClassWithCacheAttributes::class, $instance); + $this->assertTrue($instance->methodWithoutAttribute()); } public function testWithCacheAnnotationWithConstructorReturnServiceProxyCacheInterface(): void { - $inputClass = new WithConstructorAnnotationClass('test'); - $proxy = $this->factory->createProxy($inputClass); + $instance = $this->factory->createInstance(WithConstructorAnnotationClass::class, 'test'); - $this->assertProxy($inputClass, $proxy); - $this->assertTrue($proxy->aMethodWithoutAnnotation()); + $this->assertProxy(WithConstructorAnnotationClass::class, $instance); + $this->assertTrue($instance->aMethodWithoutAnnotation()); } public function testCheckInterceptorsOrders(): void diff --git a/tests/ProxyTestTrait.php b/tests/ProxyTestTrait.php index 7262cd6..17bb1c8 100644 --- a/tests/ProxyTestTrait.php +++ b/tests/ProxyTestTrait.php @@ -6,6 +6,7 @@ use OpenClassrooms\ServiceProxy\Interceptor\Contract\PrefixInterceptor; use OpenClassrooms\ServiceProxy\Interceptor\Contract\SuffixInterceptor; +use OpenClassrooms\ServiceProxy\Proxy\AccessInterceptorsInterface; use OpenClassrooms\ServiceProxy\ProxyFactory; use OpenClassrooms\ServiceProxy\ProxyFactoryConfiguration; use PHPUnit\Framework\TestCase as Assert; @@ -23,18 +24,22 @@ protected function tearDown(): void $fs->remove(self::$cacheDir); } - protected function assertProxy(object $input, object $proxy): void + /** + * @param class-string $input + */ + protected function assertProxy(string $input, object $proxy): void { - Assert::assertInstanceOf(\get_class($input), $proxy); - Assert::assertInstanceOf(ValueHolderInterface::class, $proxy); - Assert::assertInstanceOf(AccessInterceptorInterface::class, $proxy); + Assert::assertInstanceOf($input, $proxy); + Assert::assertInstanceOf(AccessInterceptorsInterface::class, $proxy); } - protected function assertNotProxy(object $input, object $proxy): void + /** + * @param class-string $class + */ + protected function assertNotProxy(string $class, object $proxy): void { - Assert::assertInstanceOf(\get_class($input), $proxy); - Assert::assertNotInstanceOf(ValueHolderInterface::class, $proxy); - Assert::assertNotInstanceOf(AccessInterceptorInterface::class, $proxy); + Assert::assertInstanceOf($class, $proxy); + Assert::assertNotInstanceOf(AccessInterceptorsInterface::class, $proxy); } /** From 5ac3629253016870a675fbaff797145694eb8999 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Letord?= Date: Mon, 25 Mar 2024 11:40:59 +0100 Subject: [PATCH 2/4] Fix typing and code style --- .../Symfony/CacheWarmer/ProxyCacheWarmer.php | 15 ++++++-- .../Symfony/Config/services.php | 8 +++-- .../Compiler/ServiceProxyPass.php | 1 - .../Subscriber/ServiceProxySubscriber.php | 15 ++------ .../AccessInterceptorGenerator.php | 20 +++++++---- .../Factory/AccessInterceptorFactory.php | 14 +++++--- .../Method/InterceptedMethod.php | 3 +- .../Method/InterceptorGenerator.php | 36 ++++++++++++------- .../Method/SetMethodPrefixInterceptors.php | 4 ++- .../Method/SetMethodSuffixInterceptors.php | 1 - src/Handler/Contract/EventHandler.php | 5 +++ .../SymfonyEventDispatcherEventHandler.php | 7 ++++ .../Contract/PrefixInterceptor.php | 10 ++++++ .../Contract/StartUpInterceptor.php | 10 ++++++ .../Contract/SuffixInterceptor.php | 10 ++++++ src/Interceptor/Impl/CacheInterceptor.php | 9 +++++ .../Impl/InvalidateCacheInterceptor.php | 4 +++ .../Impl/LegacyCacheInterceptor.php | 19 ++++++++++ src/Interceptor/Impl/ListenInterceptor.php | 10 ++++++ src/Interceptor/Impl/SecurityInterceptor.php | 5 +++ .../Impl/TransactionInterceptor.php | 4 +++ src/Invoker/Contract/MethodInvoker.php | 4 +++ src/Invoker/Impl/AggregateMethodInvoker.php | 7 +++- src/Model/Event.php | 5 +++ src/Model/Request/Instance.php | 23 ++++++++++++ src/Proxy/AccessInterceptorsInterface.php | 18 +++++++--- src/ProxyFactory.php | 32 +++++++++++++---- .../ClassWithAnnotationOnPrivateMethod.php | 3 +- tests/Double/Stub/ClassWithFinalMethod.php | 7 ++-- tests/Double/Stub/FinalClass.php | 5 +-- tests/ProxyFactoryTest.php | 1 - tests/ProxyTestTrait.php | 2 -- 32 files changed, 248 insertions(+), 69 deletions(-) diff --git a/src/FrameworkBridge/Symfony/CacheWarmer/ProxyCacheWarmer.php b/src/FrameworkBridge/Symfony/CacheWarmer/ProxyCacheWarmer.php index f099d21..7d106ab 100644 --- a/src/FrameworkBridge/Symfony/CacheWarmer/ProxyCacheWarmer.php +++ b/src/FrameworkBridge/Symfony/CacheWarmer/ProxyCacheWarmer.php @@ -1,16 +1,23 @@ + */ private iterable $proxies; + /** + * @param iterable $proxies + */ public function __construct(iterable $proxies) { $this->proxies = $proxies; @@ -19,7 +26,7 @@ public function __construct(iterable $proxies) /** * {@inheritdoc} */ - public function warmUp(string $cacheDir) + public function warmUp(string $cacheDir): array { foreach ($this->proxies as $proxy) { if ($proxy instanceof LazyLoadingInterface && !$proxy->isProxyInitialized()) { @@ -32,6 +39,8 @@ public function warmUp(string $cacheDir) $proxy->initializeLazyObject(); } } + + return []; } /** diff --git a/src/FrameworkBridge/Symfony/Config/services.php b/src/FrameworkBridge/Symfony/Config/services.php index 4a305e3..454dbb2 100644 --- a/src/FrameworkBridge/Symfony/Config/services.php +++ b/src/FrameworkBridge/Symfony/Config/services.php @@ -2,6 +2,7 @@ declare(strict_types=1); +use OpenClassrooms\ServiceProxy\FrameworkBridge\Symfony\CacheWarmer\ProxyCacheWarmer; use OpenClassrooms\ServiceProxy\FrameworkBridge\Symfony\Messenger\Transport\Serialization\MessageSerializer; use OpenClassrooms\ServiceProxy\FrameworkBridge\Symfony\Subscriber\ServiceProxySubscriber; use OpenClassrooms\ServiceProxy\Handler\Impl\Cache\SymfonyCacheHandler; @@ -9,7 +10,6 @@ use OpenClassrooms\ServiceProxy\ProxyFactory; use OpenClassrooms\ServiceProxy\ProxyFactoryConfiguration; use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator; -use OpenClassrooms\ServiceProxy\FrameworkBridge\Symfony\CacheWarmer\ProxyCacheWarmer; use function Symfony\Component\DependencyInjection\Loader\Configurator\inline_service; use function Symfony\Component\DependencyInjection\Loader\Configurator\param; use function Symfony\Component\DependencyInjection\Loader\Configurator\service; @@ -78,7 +78,9 @@ $services->set(ProxyCacheWarmer::class) ->args([ - tagged_iterator('openclassrooms.service_proxy') + tagged_iterator('openclassrooms.service_proxy'), ]) - ->tag('kernel.cache_warmer', ['priority' => 128]); + ->tag('kernel.cache_warmer', [ + 'priority' => 128, + ]); }; diff --git a/src/FrameworkBridge/Symfony/DependencyInjection/Compiler/ServiceProxyPass.php b/src/FrameworkBridge/Symfony/DependencyInjection/Compiler/ServiceProxyPass.php index ad7bd3b..b45a97d 100644 --- a/src/FrameworkBridge/Symfony/DependencyInjection/Compiler/ServiceProxyPass.php +++ b/src/FrameworkBridge/Symfony/DependencyInjection/Compiler/ServiceProxyPass.php @@ -8,7 +8,6 @@ use Symfony\Component\DependencyInjection\Compiler\Compiler; use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface; use Symfony\Component\DependencyInjection\ContainerBuilder; -use Symfony\Component\DependencyInjection\Definition; use Symfony\Component\DependencyInjection\Reference; final class ServiceProxyPass implements CompilerPassInterface diff --git a/src/FrameworkBridge/Symfony/Subscriber/ServiceProxySubscriber.php b/src/FrameworkBridge/Symfony/Subscriber/ServiceProxySubscriber.php index 1129550..59112d1 100644 --- a/src/FrameworkBridge/Symfony/Subscriber/ServiceProxySubscriber.php +++ b/src/FrameworkBridge/Symfony/Subscriber/ServiceProxySubscriber.php @@ -9,7 +9,6 @@ use OpenClassrooms\ServiceProxy\Interceptor\Contract\StartUpInterceptor; use OpenClassrooms\ServiceProxy\Model\Request\Instance; use OpenClassrooms\ServiceProxy\Model\Request\Method; -use ProxyManager\Proxy\ValueHolderInterface; use Symfony\Component\EventDispatcher\EventSubscriberInterface; use Symfony\Component\HttpKernel\Event\RequestEvent; @@ -73,24 +72,16 @@ public function startUp(RequestEvent $event): void } /** - * @return iterable + * @return iterable> */ public function getInstances(): iterable { foreach ($this->proxies as $proxy) { - $object = $proxy; - if ($proxy instanceof ValueHolderInterface) { - $object = $proxy->getWrappedValueHolderValue(); - if ($object === null) { - continue; - } - } - $instanceRef = new \ReflectionObject($object); - $methods = $instanceRef->getMethods(\ReflectionMethod::IS_PUBLIC); + $instanceRef = new \ReflectionClass($proxy); + $methods = $instanceRef->getMethods(\ReflectionMethod::IS_PUBLIC | \ReflectionMethod::IS_PROTECTED); foreach ($methods as $methodRef) { $methodAnnotations = $this->annotationReader->getMethodAnnotations($methodRef); $instance = Instance::create( - $proxy, $instanceRef, Method::create( $methodRef, diff --git a/src/Generator/AccessInterceptorGenerator/AccessInterceptorGenerator.php b/src/Generator/AccessInterceptorGenerator/AccessInterceptorGenerator.php index de34a3d..18b579f 100644 --- a/src/Generator/AccessInterceptorGenerator/AccessInterceptorGenerator.php +++ b/src/Generator/AccessInterceptorGenerator/AccessInterceptorGenerator.php @@ -1,13 +1,15 @@ } $proxyOptions * - * @return void - * * @throws \InvalidArgumentException * @throws InvalidProxiedClassException */ public function generate(\ReflectionClass $originalClass, ClassGenerator $classGenerator, array $proxyOptions = []) { - if (!array_key_exists('methods', $proxyOptions)) { - throw new \InvalidArgumentException(sprintf("Generator %s needs a methods proxyOptions",__CLASS__)); + if (!\array_key_exists('methods', $proxyOptions)) { + throw new \InvalidArgumentException(sprintf('Generator %s needs a methods proxyOptions', __CLASS__)); } CanProxyAssertion::assertClassCanBeProxied($originalClass, false); @@ -48,7 +48,10 @@ static function (MethodGenerator $generatedMethod) use ($originalClass, $classGe array_merge( array_map( $this->buildMethodInterceptor($prefixInterceptors, $suffixInterceptors), - array_map(static fn (string $method) => $originalClass->getMethod($method), $proxyOptions['methods']) + array_map( + static fn (string $method) => $originalClass->getMethod($method), + $proxyOptions['methods'] + ) ), [ new SetMethodPrefixInterceptors($prefixInterceptors), @@ -62,7 +65,10 @@ private function buildMethodInterceptor( MethodPrefixInterceptors $prefixInterceptors, MethodSuffixInterceptors $suffixInterceptors ): callable { - return static function (\ReflectionMethod $method) use ($prefixInterceptors, $suffixInterceptors): InterceptedMethod { + return static function (\ReflectionMethod $method) use ( + $prefixInterceptors, + $suffixInterceptors + ): InterceptedMethod { return InterceptedMethod::generateMethod( new MethodReflection($method->getDeclaringClass()->getName(), $method->getName()), $prefixInterceptors, diff --git a/src/Generator/AccessInterceptorGenerator/Factory/AccessInterceptorFactory.php b/src/Generator/AccessInterceptorGenerator/Factory/AccessInterceptorFactory.php index 882d913..4efbbf3 100644 --- a/src/Generator/AccessInterceptorGenerator/Factory/AccessInterceptorFactory.php +++ b/src/Generator/AccessInterceptorGenerator/Factory/AccessInterceptorFactory.php @@ -1,4 +1,6 @@ - $class * @param array $args * @param array $prefixInterceptors an array (indexed by method name) of interceptor closures to be called @@ -41,13 +43,15 @@ public function createInstance( ); $methods = array_unique($methods); - $proxyClassName = $this->generateProxy($class, ['methods' => $methods]); + $proxyClassName = $this->generateProxy($class, [ + 'methods' => $methods, + ]); $instance = new $proxyClassName(...$args); - + $instance->setPrefixInterceptors($prefixInterceptors); $instance->setSuffixInterceptors($suffixInterceptors); - + return $instance; } diff --git a/src/Generator/AccessInterceptorGenerator/Method/InterceptedMethod.php b/src/Generator/AccessInterceptorGenerator/Method/InterceptedMethod.php index 11772cf..a07f13b 100644 --- a/src/Generator/AccessInterceptorGenerator/Method/InterceptedMethod.php +++ b/src/Generator/AccessInterceptorGenerator/Method/InterceptedMethod.php @@ -8,7 +8,6 @@ use Laminas\Code\Generator\PropertyGenerator; use Laminas\Code\Reflection\MethodReflection; -use OpenClassrooms\ServiceProxy\Generator\AccessInterceptorGenerator\Method\InterceptorGenerator; use ProxyManager\Generator\MethodGenerator; /** @@ -24,7 +23,7 @@ public static function generateMethod( PropertyGenerator $prefixInterceptors, PropertyGenerator $suffixInterceptors ): self { - $method = static::fromReflectionWithoutBodyAndDocBlock($originalMethod); + $method = static::fromReflectionWithoutBodyAndDocBlock($originalMethod); $forwardedParams = []; foreach ($originalMethod->getParameters() as $parameter) { diff --git a/src/Generator/AccessInterceptorGenerator/Method/InterceptorGenerator.php b/src/Generator/AccessInterceptorGenerator/Method/InterceptorGenerator.php index af08101..a5aadd8 100644 --- a/src/Generator/AccessInterceptorGenerator/Method/InterceptorGenerator.php +++ b/src/Generator/AccessInterceptorGenerator/Method/InterceptorGenerator.php @@ -63,19 +63,29 @@ public static function createInterceptedMethodBody( ?\ReflectionMethod $originalMethod ): string { $replacements = [ - '{{$name}}' => var_export($method->getName(), true), - '{{$prefixInterceptorsName}}' => $prefixInterceptors->getName(), - '{{$prefixEarlyReturnExpression}}' => ProxiedMethodReturnExpression::generate('$prefixReturnValue', $originalMethod), - '{{$methodBody}}' => $methodBody, - '{{$suffixInterceptorsName}}' => $suffixInterceptors->getName(), - '{{$suffixEarlyReturnExpression}}' => ProxiedMethodReturnExpression::generate('$suffixReturnValue', $originalMethod), - '{{$returnExpression}}' => ProxiedMethodReturnExpression::generate('$returnValue', $originalMethod), - '{{$paramsString}}' => 'array(' . implode(', ', array_map( - static function (ParameterGenerator $parameter): string { - return var_export($parameter->getName(), true) . ' => ' . ($parameter->getPassedByReference() ? '&$' : '$') . $parameter->getName(); - }, - $method->getParameters() - )) + '{{$name}}' => var_export($method->getName(), true), + '{{$prefixInterceptorsName}}' => $prefixInterceptors->getName(), + '{{$prefixEarlyReturnExpression}}' => ProxiedMethodReturnExpression::generate( + '$prefixReturnValue', + $originalMethod + ), + '{{$methodBody}}' => $methodBody, + '{{$suffixInterceptorsName}}' => $suffixInterceptors->getName(), + '{{$suffixEarlyReturnExpression}}' => ProxiedMethodReturnExpression::generate( + '$suffixReturnValue', + $originalMethod + ), + '{{$returnExpression}}' => ProxiedMethodReturnExpression::generate( + '$returnValue', + $originalMethod + ), + '{{$paramsString}}' => 'array(' . implode(', ', array_map( + static fn (ParameterGenerator $parameter): string => var_export( + $parameter->getName(), + true + ) . ' => ' . ($parameter->getPassedByReference() ? '&$' : '$') . $parameter->getName(), + $method->getParameters() + )) . ')', ]; diff --git a/src/Generator/AccessInterceptorGenerator/Method/SetMethodPrefixInterceptors.php b/src/Generator/AccessInterceptorGenerator/Method/SetMethodPrefixInterceptors.php index 700da7e..3fd2538 100644 --- a/src/Generator/AccessInterceptorGenerator/Method/SetMethodPrefixInterceptors.php +++ b/src/Generator/AccessInterceptorGenerator/Method/SetMethodPrefixInterceptors.php @@ -1,4 +1,6 @@ - $instance + */ public function listen(Instance $instance, string $name, Transport $transport = null, int $priority = 0): void; } diff --git a/src/Handler/Impl/Event/SymfonyEventDispatcherEventHandler.php b/src/Handler/Impl/Event/SymfonyEventDispatcherEventHandler.php index 62d8e69..cf87831 100644 --- a/src/Handler/Impl/Event/SymfonyEventDispatcherEventHandler.php +++ b/src/Handler/Impl/Event/SymfonyEventDispatcherEventHandler.php @@ -47,6 +47,13 @@ public function listen(Instance $instance, string $name, ?Transport $transport = ); } + /** + * @template T of object + * + * @param Instance $instance + * + * @return callable(object): mixed + */ private function getCallable(Instance $instance): callable { return fn (object $event) => $this->aggregateMethodInvoker->invoke($instance, $event); diff --git a/src/Interceptor/Contract/PrefixInterceptor.php b/src/Interceptor/Contract/PrefixInterceptor.php index 593e79a..574a7c7 100644 --- a/src/Interceptor/Contract/PrefixInterceptor.php +++ b/src/Interceptor/Contract/PrefixInterceptor.php @@ -11,8 +11,18 @@ interface PrefixInterceptor { public const PREFIX_TYPE = 'prefix'; + /** + * @template T of object + * + * @param Instance $instance + */ public function prefix(Instance $instance): Response; + /** + * @template T of object + * + * @param Instance $instance + */ public function supportsPrefix(Instance $instance): bool; public function getPrefixPriority(): int; diff --git a/src/Interceptor/Contract/StartUpInterceptor.php b/src/Interceptor/Contract/StartUpInterceptor.php index 6cc0320..ca30c1b 100644 --- a/src/Interceptor/Contract/StartUpInterceptor.php +++ b/src/Interceptor/Contract/StartUpInterceptor.php @@ -11,8 +11,18 @@ interface StartUpInterceptor { public const PREFIX_TYPE = 'startUp'; + /** + * @template T of object + * + * @param Instance $instance + */ public function startUp(Instance $instance): Response; + /** + * @template T of object + * + * @param Instance $instance + */ public function supportsStartUp(Instance $instance): bool; public function getStartUpPriority(): int; diff --git a/src/Interceptor/Contract/SuffixInterceptor.php b/src/Interceptor/Contract/SuffixInterceptor.php index 33db462..02fac02 100644 --- a/src/Interceptor/Contract/SuffixInterceptor.php +++ b/src/Interceptor/Contract/SuffixInterceptor.php @@ -11,8 +11,18 @@ interface SuffixInterceptor { public const SUFFIX_TYPE = 'suffix'; + /** + * @template T of object + * + * @param Instance $instance + */ public function suffix(Instance $instance): Response; + /** + * @template T of object + * + * @param Instance $instance + */ public function supportsSuffix(Instance $instance): bool; public function getSuffixPriority(): int; diff --git a/src/Interceptor/Impl/CacheInterceptor.php b/src/Interceptor/Impl/CacheInterceptor.php index 96a3fda..3301398 100644 --- a/src/Interceptor/Impl/CacheInterceptor.php +++ b/src/Interceptor/Impl/CacheInterceptor.php @@ -173,6 +173,11 @@ public function getSuffixPriority(): int return 20; } + /** + * @template T of object + * + * @param Instance $instance + */ private function buildCacheKey(Instance $instance, Cache $attribute): string { $identifier = implode( @@ -273,6 +278,10 @@ private function getInnerCode(\ReflectionMethod|\ReflectionClass $reflection): s } /** + * @template T of object + * + * @param Instance $instance + * * @return array */ private function getTags(Instance $instance, Cache $attribute, mixed $response = null): array diff --git a/src/Interceptor/Impl/InvalidateCacheInterceptor.php b/src/Interceptor/Impl/InvalidateCacheInterceptor.php index 7e18b27..68e6e19 100644 --- a/src/Interceptor/Impl/InvalidateCacheInterceptor.php +++ b/src/Interceptor/Impl/InvalidateCacheInterceptor.php @@ -49,6 +49,10 @@ public function getSuffixPriority(): int } /** + * @template T of object + * + * @param Instance $instance + * * @return array */ private function getTags(Instance $instance, InvalidateCache $attribute): array diff --git a/src/Interceptor/Impl/LegacyCacheInterceptor.php b/src/Interceptor/Impl/LegacyCacheInterceptor.php index 5227ef4..ed4ae99 100644 --- a/src/Interceptor/Impl/LegacyCacheInterceptor.php +++ b/src/Interceptor/Impl/LegacyCacheInterceptor.php @@ -18,6 +18,11 @@ */ final class LegacyCacheInterceptor extends AbstractInterceptor implements SuffixInterceptor, PrefixInterceptor { + /** + * @template T of object + * + * @param Instance $instance + */ public function prefix(Instance $instance): Response { $annotation = $instance->getMethod() @@ -98,6 +103,11 @@ public function getSuffixPriority(): int return 20; } + /** + * @template T of object + * + * @param Instance $instance + */ private function getNamespace(Instance $instance, Cache $annotation): ?string { $parameters = $instance->getMethod() @@ -115,6 +125,11 @@ private function getNamespace(Instance $instance, Cache $annotation): ?string return null; } + /** + * @template T of object + * + * @param Instance $instance + */ private function getProxyId(Instance $instance, Cache $annotation): string { $parameters = $instance->getMethod() @@ -139,6 +154,10 @@ private function getProxyId(Instance $instance, Cache $annotation): string } /** + * @template T of object + * + * @param Instance $instance + * * @return array */ private function getTags(Instance $instance, Cache $annotation): array diff --git a/src/Interceptor/Impl/ListenInterceptor.php b/src/Interceptor/Impl/ListenInterceptor.php index 0129557..88e72a2 100644 --- a/src/Interceptor/Impl/ListenInterceptor.php +++ b/src/Interceptor/Impl/ListenInterceptor.php @@ -14,6 +14,11 @@ final class ListenInterceptor extends AbstractInterceptor implements StartUpInterceptor { + /** + * @template T of object + * + * @param Instance $instance + */ public function startUp(Instance $instance): Response { $attributes = $instance->getMethod() @@ -37,6 +42,11 @@ public function startUp(Instance $instance): Response return new Response(); } + /** + * @template T of object + * + * @param Instance $instance + */ public function supportsStartUp(Instance $instance): bool { return $instance->getMethod() diff --git a/src/Interceptor/Impl/SecurityInterceptor.php b/src/Interceptor/Impl/SecurityInterceptor.php index 7bfa178..3ac4518 100644 --- a/src/Interceptor/Impl/SecurityInterceptor.php +++ b/src/Interceptor/Impl/SecurityInterceptor.php @@ -56,6 +56,11 @@ public function supportsPrefix(Instance $instance): bool ; } + /** + * @template T of object + * + * @param Instance $instance + */ private function guessRoleName(Instance $instance): string { $className = $instance->getReflection() diff --git a/src/Interceptor/Impl/TransactionInterceptor.php b/src/Interceptor/Impl/TransactionInterceptor.php index 9116837..35c5e40 100644 --- a/src/Interceptor/Impl/TransactionInterceptor.php +++ b/src/Interceptor/Impl/TransactionInterceptor.php @@ -73,6 +73,10 @@ public function getSuffixPriority(): int } /** + * @template T of object + * + * @param Instance $instance + * * @throws \Exception */ private function handleMappedException(Instance $instance, Transaction $attribute): void diff --git a/src/Invoker/Contract/MethodInvoker.php b/src/Invoker/Contract/MethodInvoker.php index f42f507..c80b17d 100644 --- a/src/Invoker/Contract/MethodInvoker.php +++ b/src/Invoker/Contract/MethodInvoker.php @@ -9,6 +9,10 @@ interface MethodInvoker { /** + * @template T of object + * + * @param Instance $listenerInstance + * * @throws \InvalidArgumentException */ public function invoke(Instance $listenerInstance, ?object $event = null): mixed; diff --git a/src/Invoker/Impl/AggregateMethodInvoker.php b/src/Invoker/Impl/AggregateMethodInvoker.php index a3fec80..d834b73 100644 --- a/src/Invoker/Impl/AggregateMethodInvoker.php +++ b/src/Invoker/Impl/AggregateMethodInvoker.php @@ -10,13 +10,18 @@ final class AggregateMethodInvoker { /** - * @param iterable $invokers + * @param iterable $invokers */ public function __construct( private iterable $invokers ) { } + /** + * @template T of object + * + * @param Instance $instance + */ public function invoke(Instance $instance, ?object $object = null): mixed { if (!\is_array($this->invokers)) { diff --git a/src/Model/Event.php b/src/Model/Event.php index 39a5c9a..de7aac9 100644 --- a/src/Model/Event.php +++ b/src/Model/Event.php @@ -25,6 +25,11 @@ public function __construct( ) { } + /** + * @template T of object + * + * @param Instance $instance + */ public static function createFromSenderInstance( Instance $instance, Moment $moment = Moment::SUFFIX, diff --git a/src/Model/Request/Instance.php b/src/Model/Request/Instance.php index c0ee782..8d4e029 100644 --- a/src/Model/Request/Instance.php +++ b/src/Model/Request/Instance.php @@ -7,10 +7,16 @@ use Doctrine\Common\Annotations\AnnotationException; use Doctrine\Common\Annotations\AnnotationReader; +/** + * @template T of object + */ final class Instance { private Method $method; + /** + * @var \ReflectionClass + */ private \ReflectionClass $reflection; private function __construct() @@ -18,10 +24,13 @@ private function __construct() } /** + * @param class-string $class * @param array|null $parameters * * @throws \ReflectionException * @throws AnnotationException + * + * @return self */ public static function createFromMethod( string $class, @@ -49,10 +58,16 @@ public function getMethod(): Method return $this->method; } + /** + * @param \ReflectionClass $reflection + * + * @return self + */ public static function create( \ReflectionClass $reflection, Method $method ): self { + /** @var Instance $self */ $self = new self(); $self->reflection = $reflection; $self->method = $method; @@ -62,6 +77,8 @@ public static function create( /** * @param array $parameters + * + * @return self */ public function setParameters(array $parameters): self { @@ -70,6 +87,9 @@ public function setParameters(array $parameters): self return $this; } + /** + * @return self + */ public function setResponse(mixed $response): self { $this->method->setResponse($response); @@ -77,6 +97,9 @@ public function setResponse(mixed $response): self return $this; } + /** + * @return \ReflectionClass + */ public function getReflection(): \ReflectionClass { return $this->reflection; diff --git a/src/Proxy/AccessInterceptorsInterface.php b/src/Proxy/AccessInterceptorsInterface.php index 964422c..8e5928a 100644 --- a/src/Proxy/AccessInterceptorsInterface.php +++ b/src/Proxy/AccessInterceptorsInterface.php @@ -1,15 +1,25 @@ - + */ interface AccessInterceptorsInterface extends ProxyInterface { - + /** + * @param array, object, string, array, bool): mixed>> $prefixInterceptors + */ public function setPrefixInterceptors(array $prefixInterceptors): void; + /** + * @param array, object, string, array, bool): mixed>> $suffixInterceptors + */ public function setSuffixInterceptors(array $suffixInterceptors): void; } diff --git a/src/ProxyFactory.php b/src/ProxyFactory.php index e89dd9f..a76de2a 100644 --- a/src/ProxyFactory.php +++ b/src/ProxyFactory.php @@ -8,8 +8,6 @@ use Doctrine\Common\Annotations\AnnotationReader; use Doctrine\Common\Annotations\Reader; use OpenClassrooms\ServiceProxy\Generator\AccessInterceptorGenerator\Factory\AccessInterceptorFactory; -use OpenClassrooms\ServiceProxy\Generator\AccessInterceptorScopeLocalizerGenerator\Factory\AccessInterceptorScopeLocalizerFactory; -use OpenClassrooms\ServiceProxy\Generator\AccessInterceptorValueHolderGenerator\Factory\AccessInterceptorValueHolderFactory; use OpenClassrooms\ServiceProxy\Interceptor\Contract\PrefixInterceptor; use OpenClassrooms\ServiceProxy\Interceptor\Contract\SuffixInterceptor; use OpenClassrooms\ServiceProxy\Model\Request\Instance; @@ -61,7 +59,7 @@ public function __construct( * @template T of object * * @param class-string $class - * @param mixed ...$agrs + * @param mixed ...$args * * @return T */ @@ -70,7 +68,10 @@ public function createInstance(string $class, ...$args): object $instanceRef = new \ReflectionClass($class); if ($instanceRef->isFinal()) { - throw new \LogicException(sprintf("Unable to proxify final class %s. Hint: replace final keywords with a @final annotation", $instanceRef->getName())); + throw new \LogicException(sprintf( + 'Unable to proxify final class %s. Hint: replace final keywords with a @final annotation', + $instanceRef->getName() + )); } $methods = $instanceRef->getMethods(); @@ -78,7 +79,7 @@ public function createInstance(string $class, ...$args): object foreach ($methods as $methodRef) { $methodAnnotations = $this->annotationReader->getMethodAnnotations($methodRef); - if (count($methodAnnotations) === 0 && count($methodRef->getAttributes()) === 0) { + if (\count($methodAnnotations) === 0 && \count($methodRef->getAttributes()) === 0) { continue; } @@ -93,11 +94,19 @@ public function createInstance(string $class, ...$args): object $interceptors = $this->filterInterceptors($instance, $type); if (\count($interceptors) > 0) { if ($methodRef->isFinal()) { - throw new \LogicException(sprintf("Unable to proxify a final method %s on class %s. Hint: replace final keywords with a @final annotation", $methodRef->getName(), $methodRef->getDeclaringClass())); + throw new \LogicException(sprintf( + 'Unable to proxify a final method %s on class %s. Hint: replace final keywords with a @final annotation', + $methodRef->getName(), + $methodRef->getDeclaringClass() + )); } if ($methodRef->isPrivate()) { - throw new \LogicException(sprintf("Unable to attach an interceptor to a private method %s on class %s.", $methodRef->getName(), $methodRef->getDeclaringClass())); + throw new \LogicException(sprintf( + 'Unable to attach an interceptor to a private method %s on class %s.', + $methodRef->getName(), + $methodRef->getDeclaringClass() + )); } $interceptionClosures[$type][$methodRef->getName()] = $this->getInterceptionClosure( @@ -132,8 +141,11 @@ public function getInterceptors(): array } /** + * @template T of object + * * @param PrefixInterceptor::PREFIX_TYPE|SuffixInterceptor::SUFFIX_TYPE $type * @param PrefixInterceptor[]|SuffixInterceptor[] $interceptors + * @param Instance $instance */ private function intercept( string $type, @@ -200,6 +212,10 @@ static function (object $a, object $b) use ($type) { } /** + * @template T of object + * + * @param Instance $instance + * / * @param PrefixInterceptor::PREFIX_TYPE|SuffixInterceptor::SUFFIX_TYPE $type * * @return PrefixInterceptor[]|SuffixInterceptor[] @@ -227,8 +243,10 @@ private function filterInterceptors(Instance $instance, string $type): array } /** + * @template T of object * @param PrefixInterceptor::PREFIX_TYPE|SuffixInterceptor::SUFFIX_TYPE $type * @param SuffixInterceptor[]|PrefixInterceptor[] $interceptors + * @param Instance $instance */ private function getInterceptionClosure( string $type, diff --git a/tests/Double/Stub/ClassWithAnnotationOnPrivateMethod.php b/tests/Double/Stub/ClassWithAnnotationOnPrivateMethod.php index 895aba3..ce6bda3 100644 --- a/tests/Double/Stub/ClassWithAnnotationOnPrivateMethod.php +++ b/tests/Double/Stub/ClassWithAnnotationOnPrivateMethod.php @@ -1,5 +1,7 @@ assertNotProxy(WithoutAnnotationClass::class, $instance); } - public function testThrownExceptionWhenCreateAProxyOnFinalClass(): void { $this->expectException(\LogicException::class); diff --git a/tests/ProxyTestTrait.php b/tests/ProxyTestTrait.php index 17bb1c8..08a1595 100644 --- a/tests/ProxyTestTrait.php +++ b/tests/ProxyTestTrait.php @@ -10,8 +10,6 @@ use OpenClassrooms\ServiceProxy\ProxyFactory; use OpenClassrooms\ServiceProxy\ProxyFactoryConfiguration; use PHPUnit\Framework\TestCase as Assert; -use ProxyManager\Proxy\AccessInterceptorInterface; -use ProxyManager\Proxy\ValueHolderInterface; use Symfony\Component\Filesystem\Filesystem; trait ProxyTestTrait From 4b58492c4375ff9e3abe6541d69ef402bf66f147 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Letord?= Date: Tue, 26 Mar 2024 09:48:20 +0100 Subject: [PATCH 3/4] Clarrify error message --- .../AccessInterceptorGenerator.php | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/Generator/AccessInterceptorGenerator/AccessInterceptorGenerator.php b/src/Generator/AccessInterceptorGenerator/AccessInterceptorGenerator.php index 18b579f..5e5da37 100644 --- a/src/Generator/AccessInterceptorGenerator/AccessInterceptorGenerator.php +++ b/src/Generator/AccessInterceptorGenerator/AccessInterceptorGenerator.php @@ -18,6 +18,10 @@ use ProxyManager\ProxyGenerator\Assertion\CanProxyAssertion; use ProxyManager\ProxyGenerator\ProxyGeneratorInterface; +use InvalidArgumentException; +use ReflectionClass; +use ReflectionMethod; + class AccessInterceptorGenerator implements ProxyGeneratorInterface { /** @@ -25,13 +29,13 @@ class AccessInterceptorGenerator implements ProxyGeneratorInterface * * @param array{'methods'?: array} $proxyOptions * - * @throws \InvalidArgumentException + * @throws InvalidArgumentException * @throws InvalidProxiedClassException */ - public function generate(\ReflectionClass $originalClass, ClassGenerator $classGenerator, array $proxyOptions = []) + public function generate(ReflectionClass $originalClass, ClassGenerator $classGenerator, array $proxyOptions = []) { if (!\array_key_exists('methods', $proxyOptions)) { - throw new \InvalidArgumentException(sprintf('Generator %s needs a methods proxyOptions', __CLASS__)); + throw new InvalidArgumentException(sprintf('Missing methods options for %s.', __CLASS__)); } CanProxyAssertion::assertClassCanBeProxied($originalClass, false); @@ -65,7 +69,7 @@ private function buildMethodInterceptor( MethodPrefixInterceptors $prefixInterceptors, MethodSuffixInterceptors $suffixInterceptors ): callable { - return static function (\ReflectionMethod $method) use ( + return static function (ReflectionMethod $method) use ( $prefixInterceptors, $suffixInterceptors ): InterceptedMethod { From 2ac06aabfb8e09701b27e81290777bc5b0bbf805 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Letord?= Date: Wed, 3 Apr 2024 18:13:56 +0200 Subject: [PATCH 4/4] Small fixes from review --- .../Compiler/ServiceProxyPass.php | 4 +++- .../AccessInterceptorGenerator.php | 12 ++++-------- .../Cache/ClassWithInvalidateCacheAttributes.php | 2 -- tests/Double/Stub/FinalClass.php | 3 +++ 4 files changed, 10 insertions(+), 11 deletions(-) diff --git a/src/FrameworkBridge/Symfony/DependencyInjection/Compiler/ServiceProxyPass.php b/src/FrameworkBridge/Symfony/DependencyInjection/Compiler/ServiceProxyPass.php index b45a97d..7abbf3e 100644 --- a/src/FrameworkBridge/Symfony/DependencyInjection/Compiler/ServiceProxyPass.php +++ b/src/FrameworkBridge/Symfony/DependencyInjection/Compiler/ServiceProxyPass.php @@ -44,7 +44,9 @@ private function overrideServiceDefinition(string $taggedServiceName): void if ($definition->getFactory() !== null) { $this->compiler->log($this, "Service {$taggedServiceName} is not compatible with service proxy"); - return; + throw new \RuntimeException( + "Unable to override {$taggedServiceName}, remove the factory definition or the Interceptable interface." + ); } $definition->setFactory([new Reference(ProxyFactory::class), 'createInstance']); diff --git a/src/Generator/AccessInterceptorGenerator/AccessInterceptorGenerator.php b/src/Generator/AccessInterceptorGenerator/AccessInterceptorGenerator.php index 5e5da37..c163335 100644 --- a/src/Generator/AccessInterceptorGenerator/AccessInterceptorGenerator.php +++ b/src/Generator/AccessInterceptorGenerator/AccessInterceptorGenerator.php @@ -18,10 +18,6 @@ use ProxyManager\ProxyGenerator\Assertion\CanProxyAssertion; use ProxyManager\ProxyGenerator\ProxyGeneratorInterface; -use InvalidArgumentException; -use ReflectionClass; -use ReflectionMethod; - class AccessInterceptorGenerator implements ProxyGeneratorInterface { /** @@ -29,13 +25,13 @@ class AccessInterceptorGenerator implements ProxyGeneratorInterface * * @param array{'methods'?: array} $proxyOptions * - * @throws InvalidArgumentException + * @throws \InvalidArgumentException * @throws InvalidProxiedClassException */ - public function generate(ReflectionClass $originalClass, ClassGenerator $classGenerator, array $proxyOptions = []) + public function generate(\ReflectionClass $originalClass, ClassGenerator $classGenerator, array $proxyOptions = []) { if (!\array_key_exists('methods', $proxyOptions)) { - throw new InvalidArgumentException(sprintf('Missing methods options for %s.', __CLASS__)); + throw new \InvalidArgumentException(sprintf('Missing methods options for %s.', __CLASS__)); } CanProxyAssertion::assertClassCanBeProxied($originalClass, false); @@ -69,7 +65,7 @@ private function buildMethodInterceptor( MethodPrefixInterceptors $prefixInterceptors, MethodSuffixInterceptors $suffixInterceptors ): callable { - return static function (ReflectionMethod $method) use ( + return static function (\ReflectionMethod $method) use ( $prefixInterceptors, $suffixInterceptors ): InterceptedMethod { diff --git a/tests/Double/Stub/Cache/ClassWithInvalidateCacheAttributes.php b/tests/Double/Stub/Cache/ClassWithInvalidateCacheAttributes.php index b3ff466..66017f7 100644 --- a/tests/Double/Stub/Cache/ClassWithInvalidateCacheAttributes.php +++ b/tests/Double/Stub/Cache/ClassWithInvalidateCacheAttributes.php @@ -11,8 +11,6 @@ class ClassWithInvalidateCacheAttributes { public const DATA = 'data'; - private $data; - #[Cache(tags: ['"my_tag"'])] public function methodWithTaggedCache(): string { diff --git a/tests/Double/Stub/FinalClass.php b/tests/Double/Stub/FinalClass.php index afaa3dc..ba55962 100644 --- a/tests/Double/Stub/FinalClass.php +++ b/tests/Double/Stub/FinalClass.php @@ -4,8 +4,11 @@ namespace OpenClassrooms\ServiceProxy\Tests\Double\Stub; +use OpenClassrooms\ServiceProxy\Attribute\Cache; + final class FinalClass { + #[Cache] public function aMethod(): void { }