diff --git a/CHANGELOG.md b/CHANGELOG.md index ec17c604..54f146d1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 in https://github.com/sofascore/purgatory-bundle/pull/112 - Ability to pass a static method callable as a `DynamicValues` provider by @HypeMC in https://github.com/sofascore/purgatory-bundle/pull/137 +- Ability to use a closure as the `PurgeOn` `if` condition on PHP 8.5+ by @Brajk19 + in https://github.com/sofascore/purgatory-bundle/pull/116 ### Changed diff --git a/composer.json b/composer.json index ce685df0..7b70daf7 100644 --- a/composer.json +++ b/composer.json @@ -54,6 +54,7 @@ "symfony/filesystem": "^6.4 || ^7.4 || ^8.0", "symfony/http-client": "^6.4 || ^7.4 || ^8.0", "symfony/messenger": "^6.4 || ^7.4 || ^8.0", + "symfony/polyfill-deepclone": "^1.39", "symfony/process": "^6.4 || ^7.4 || ^8.0", "symfony/serializer": "^6.4 || ^7.4 || ^8.0", "symfony/yaml": "^6.4 || ^7.4 || ^8.0" @@ -63,7 +64,9 @@ "symfony/http-client": "To use the Varnish purger", "symfony/messenger": "To purge URLs asynchronously", "symfony/serializer": "To resolve purge targets from serialization groups with `ForGroups`", - "symfony/yaml": "To configure purge subscriptions using YAML files" + "symfony/yaml": "To configure purge subscriptions using YAML files", + "symfony/polyfill-deepclone": "To use a closure as the \"if\" condition of a #[PurgeOn] attribute (PHP 8.5+)", + "symfony/deepclone": "Faster, native PHP extension alternative to symfony/polyfill-deepclone for using a closure as the \"if\" condition of a #[PurgeOn] attribute (PHP 8.5+)" }, "conflict": { "symfony/cache": "<6.4", diff --git a/docs/README.md b/docs/README.md index c5f653bb..f8eec8eb 100644 --- a/docs/README.md +++ b/docs/README.md @@ -378,6 +378,35 @@ In this example, the purge will only occur if the post has more than 3,000 upvot You can also add [custom Expression Language functions](custom-expression-language-functions.md). +### Adding Conditional Logic with Closures + +Starting with [PHP 8.5](https://www.php.net/releases/8.5/en.php), closures can be used in attributes, so the same +condition can be written in plain PHP instead of an expression. The closure receives the entity as its only argument: + +```php +#[Route('/post/{id<\d+>}', name: 'post_details', methods: 'GET')] +#[PurgeOn(Post::class, if: static function (Post $post): bool { + return $post->upvotes > 3000; +})] +public function detailsAction(Post $post) +{ +} +``` + +This feature requires the [`symfony/polyfill-deepclone`](https://github.com/symfony/polyfill-deepclone) package: + +```sh +composer require symfony/polyfill-deepclone +``` + +For better performance you can instead install the [`symfony/deepclone`](https://github.com/symfony/php-ext-deepclone) PHP extension. + +The closure must: + +- have exactly one parameter, typed with the subscribed entity class or one of its parents, +- declare a non-nullable `bool` return type. + + ### Using Purge on Actions with Multiple Routes By default, the attribute generates URLs for all routes associated with the action. You can limit this to one or more diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 2d57336b..4e169a47 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -13,13 +13,13 @@ parameters: path: src/Attribute/RouteParamValue/EnumValues.php - - message: '#^Parameter \#1 \$configuration of class Sofascore\\PurgatoryBundle\\Cache\\Configuration\\Configuration constructor expects array\, optional\?\: true\}\>, if\?\: string, actions\?\: non\-empty\-list\\}\>\>, mixed given\.$#' + message: '#^Parameter \#1 \$configuration of class Sofascore\\PurgatoryBundle\\Cache\\Configuration\\Configuration constructor expects array\, optional\?\: true\}\>, if\?\: array\\|string, actions\?\: non\-empty\-list\\}\>\>, mixed given\.$#' identifier: argument.type count: 1 path: src/Cache/Configuration/CachedConfigurationLoader.php - - message: '#^Method Sofascore\\PurgatoryBundle\\Cache\\Configuration\\Subscriptions\:\:getIterator\(\) should return Traversable\, optional\?\: true\}\>, if\?\: string, actions\?\: non\-empty\-list\\}\> but returns ArrayIterator\, optional\?\: bool\}\>, if\?\: string, actions\?\: non\-empty\-list\\}\>\.$#' + message: '#^Method Sofascore\\PurgatoryBundle\\Cache\\Configuration\\Subscriptions\:\:getIterator\(\) should return Traversable\, optional\?\: true\}\>, if\?\: array\\|string, actions\?\: non\-empty\-list\\}\> but returns ArrayIterator\, optional\?\: bool\}\>, if\?\: array\\|string, actions\?\: non\-empty\-list\\}\>\.$#' identifier: return.type count: 1 path: src/Cache/Configuration/Subscriptions.php diff --git a/src/Attribute/PurgeOn.php b/src/Attribute/PurgeOn.php index 1a40b925..9b023253 100644 --- a/src/Attribute/PurgeOn.php +++ b/src/Attribute/PurgeOn.php @@ -18,7 +18,7 @@ final class PurgeOn public readonly ?TargetInterface $target; /** @var ?non-empty-array */ public readonly ?array $routeParams; - public readonly ?Expression $if; + public readonly \Closure|Expression|null $if; /** @var ?non-empty-list */ public readonly ?array $route; /** @var ?non-empty-list */ @@ -35,10 +35,14 @@ public function __construct( public readonly string $class, string|array|TargetInterface|null $target = null, ?array $routeParams = null, - string|Expression|null $if = null, + \Closure|string|Expression|null $if = null, string|array|null $route = null, string|array|Action|null $actions = null, ) { + if ($if instanceof \Closure && !\function_exists('deepclone_to_array')) { + throw new LogicException('You cannot use a closure for the "if" attribute because deepclone support is not available. Install the "symfony/polyfill-deepclone" package (run "composer require symfony/polyfill-deepclone") or the "symfony/deepclone" PHP extension (https://github.com/symfony/php-ext-deepclone).'); + } + $this->target = \is_array($target) || \is_string($target) ? new ForProperties($target) : $target; $this->routeParams = null !== $routeParams ? self::normalizeRouteParams($routeParams) : null; $this->if = \is_string($if) ? self::normalizeExpression($if) : $if; diff --git a/src/Cache/Configuration/Configuration.php b/src/Cache/Configuration/Configuration.php index e9c0c200..34379d34 100644 --- a/src/Cache/Configuration/Configuration.php +++ b/src/Cache/Configuration/Configuration.php @@ -12,7 +12,7 @@ final class Configuration implements \Countable * @param array, optional?: true}>, - * if?: string, + * if?: string|array, * actions?: non-empty-list, * }>> $configuration */ @@ -56,7 +56,7 @@ public function count(): int * @return array, optional?: true}>, - * if?: string, + * if?: string|array, * actions?: non-empty-list, * }>> */ diff --git a/src/Cache/Configuration/ConfigurationLoader.php b/src/Cache/Configuration/ConfigurationLoader.php index aad9e2d9..f7f30a21 100644 --- a/src/Cache/Configuration/ConfigurationLoader.php +++ b/src/Cache/Configuration/ConfigurationLoader.php @@ -38,7 +38,11 @@ public function load(): Configuration } if (null !== $subscription->if) { - $config['if'] = (string) $subscription->if; + if ($subscription->if instanceof \Closure) { + $config['if'] = deepclone_to_array($subscription->if); + } else { + $config['if'] = (string) $subscription->if; + } } if (null !== $subscription->actions) { diff --git a/src/Cache/Configuration/Subscriptions.php b/src/Cache/Configuration/Subscriptions.php index de145eb4..fd47b54e 100644 --- a/src/Cache/Configuration/Subscriptions.php +++ b/src/Cache/Configuration/Subscriptions.php @@ -10,7 +10,7 @@ * @implements \IteratorAggregate, optional?: true}>, - * if?: string, + * if?: string|array, * actions?: non-empty-list, * }> */ @@ -21,7 +21,7 @@ final class Subscriptions implements \IteratorAggregate, \Countable * @param list, optional?: true}>, - * if?: string, + * if?: string|array, * actions?: non-empty-list, * }> $subscriptions */ @@ -53,7 +53,7 @@ public function key(): string * @return list, optional?: true}>, - * if?: string, + * if?: string|array, * actions?: non-empty-list, * }> */ diff --git a/src/Cache/PropertyResolver/AssociationResolver.php b/src/Cache/PropertyResolver/AssociationResolver.php index 8dc5019c..3839cbe5 100644 --- a/src/Cache/PropertyResolver/AssociationResolver.php +++ b/src/Cache/PropertyResolver/AssociationResolver.php @@ -73,6 +73,11 @@ public function resolveSubscription( } if (null !== $if = $routeMetadata->purgeOn->if) { + if ($if instanceof \Closure) { + // TODO support closures + throw new \RuntimeException('Cannot create inverse subscription with closures'); + } + $if = $this->expressionTransformer->transform($if, $associationClass, $associationTarget, 'false'); } diff --git a/src/Cache/Subscription/PurgeSubscription.php b/src/Cache/Subscription/PurgeSubscription.php index 15b14dd8..db8416db 100644 --- a/src/Cache/Subscription/PurgeSubscription.php +++ b/src/Cache/Subscription/PurgeSubscription.php @@ -23,7 +23,7 @@ public function __construct( public readonly string $routeName, public readonly Route $route, public readonly ?array $actions, - public readonly ?Expression $if = null, + public readonly \Closure|Expression|null $if = null, ) { } } diff --git a/src/Cache/Subscription/PurgeSubscriptionProvider.php b/src/Cache/Subscription/PurgeSubscriptionProvider.php index 58c37f08..390c10f7 100644 --- a/src/Cache/Subscription/PurgeSubscriptionProvider.php +++ b/src/Cache/Subscription/PurgeSubscriptionProvider.php @@ -15,6 +15,7 @@ use Sofascore\PurgatoryBundle\Cache\RouteMetadata\RouteMetadataProviderInterface; use Sofascore\PurgatoryBundle\Cache\TargetResolver\TargetResolverInterface; use Sofascore\PurgatoryBundle\Exception\EntityMetadataNotFoundException; +use Sofascore\PurgatoryBundle\Exception\InvalidIfClosureException; use Sofascore\PurgatoryBundle\Exception\InvalidIfExpressionException; use Sofascore\PurgatoryBundle\Exception\MissingRequiredRouteParametersException; use Sofascore\PurgatoryBundle\Exception\TargetSubscriptionNotResolvableException; @@ -59,7 +60,7 @@ private function provideFromMetadata(RouteMetadataProviderInterface $routeMetada $purgeOn = $routeMetadata->purgeOn; if (null !== $purgeOn->if) { - $this->validateExpression($purgeOn->if, $routeMetadata->routeName); + $this->validateIf($purgeOn->if, $routeMetadata->routeName, $purgeOn->class); } // if route parameters are not specified, they are same as path variables @@ -146,6 +147,52 @@ private function validateRouteParams(array $routeParams, RouteMetadata $routeMet } } + private function validateIf(\Closure|Expression $expression, string $routeName, string $entity): void + { + if ($expression instanceof \Closure) { + $this->validateIfClosure($expression, $routeName, $entity); + + return; + } + + $this->validateExpression($expression, $routeName); + } + + private function validateIfClosure(\Closure $expression, string $routeName, string $entity): void + { + $reflection = new \ReflectionFunction($expression); + + if (null !== $reflection->getClosureThis()) { + throw new InvalidIfClosureException($routeName, 'Closure must be static'); + } + + if ([] !== $reflection->getClosureUsedVariables()) { + throw new InvalidIfClosureException($routeName, 'Closure must not capture variables'); + } + + $returnType = $reflection->getReturnType(); + + if (!$returnType instanceof \ReflectionNamedType + || $returnType->allowsNull() + || !\in_array($returnType->getName(), ['bool', 'true', 'false']) + ) { + throw new InvalidIfClosureException($routeName, 'Return type must be bool'); + } + + if (1 !== $reflection->getNumberOfParameters()) { + throw new InvalidIfClosureException($routeName, 'Closure must have exactly 1 parameter'); + } + + $parameterType = $reflection->getParameters()[0]->getType(); + + if (!$parameterType instanceof \ReflectionNamedType + || $parameterType->allowsNull() + || !is_a($entity, $parameterType->getName(), true) + ) { + throw new InvalidIfClosureException($routeName, "Parameter in closure must be of type $entity"); + } + } + private function validateExpression(Expression $expression, string $routeName): void { try { diff --git a/src/Command/DebugCommand.php b/src/Command/DebugCommand.php index 696d1c1b..bf4ee2f3 100644 --- a/src/Command/DebugCommand.php +++ b/src/Command/DebugCommand.php @@ -204,7 +204,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int * @return array, optional?: true}>, - * if?: string, + * if?: string|array, * actions?: non-empty-list, * }>> */ @@ -227,7 +227,7 @@ private function findSubscriptions(string $subscription, bool $withProperties): * @return array, optional?: true}>, - * if?: string, + * if?: string|array, * actions?: non-empty-list, * }>> */ @@ -248,7 +248,7 @@ private function findSubscriptionsForRoute(string $routeName): array * @param array, optional?: true}>, - * if?: string, + * if?: string|array, * actions?: non-empty-list, * }>> $configuration */ @@ -260,6 +260,12 @@ private function display(SymfonyStyle $io, array $configuration): void $entity = explode('::', $key); foreach ($subscriptions as $subscription) { + if (isset($subscription['if']) && \is_array($subscription['if'])) { + $if = $this->formatClosureCondition($subscription['if']); + } else { + $if = \is_string($subscription['if'] ?? null) ? $subscription['if'] : 'NONE'; + } + $io->table( ['Option', 'Value'], [ @@ -267,7 +273,7 @@ private function display(SymfonyStyle $io, array $configuration): void ['Property', $entity[1] ?? 'ANY'], ['Route Name', $subscription['routeName']], ['Route Params', isset($subscription['routeParams']) ? $this->formatRouteParams($subscription['routeParams']) : 'NONE'], - ['Condition', $subscription['if'] ?? 'NONE'], + ['Condition', $if], ['Actions', isset($subscription['actions']) ? $this->formatActions($subscription['actions']) : 'ANY'], ], ); @@ -275,6 +281,43 @@ private function display(SymfonyStyle $io, array $configuration): void } } + /** + * @param array $serializedClosure + */ + private function formatClosureCondition(array $serializedClosure): string + { + /** @var \Closure $closure */ + $closure = deepclone_from_array($serializedClosure); + $reflection = new \ReflectionFunction($closure); + + $file = $reflection->getFileName(); + $startLine = $reflection->getStartLine(); + $endLine = $reflection->getEndLine(); + + if (false === $file || false === $startLine || false === $endLine || false === $lines = @file($file)) { + return 'CLOSURE'; + } + + $sourceLines = array_map( + static fn (string $line): string => rtrim($line, "\r\n"), + \array_slice($lines, $startLine - 1, $endLine - $startLine + 1), + ); + + $indent = ''; + if (preg_match('/^(\s*).*?(?=(?:static\s+)?(?:function|fn)\b)/', $sourceLines[0], $matches)) { + $indent = $matches[1]; + $sourceLines[0] = substr($sourceLines[0], \strlen($matches[0])); + } + + foreach ($sourceLines as $i => $line) { + if ($i > 0 && '' !== $indent && str_starts_with($line, $indent)) { + $sourceLines[$i] = substr($line, \strlen($indent)); + } + } + + return rtrim(rtrim(implode("\n", $sourceLines)), ','); + } + /** * @param array, optional?: true}> $routeParams */ diff --git a/src/Exception/InvalidIfClosureException.php b/src/Exception/InvalidIfClosureException.php new file mode 100644 index 00000000..646f1f0d --- /dev/null +++ b/src/Exception/InvalidIfClosureException.php @@ -0,0 +1,15 @@ +getExpressionLanguage()->evaluate($subscription['if'], ['obj' => $entity]); - if (!\is_bool($result)) { - throw new InvalidIfExpressionResultException($subscription['routeName'], $subscription['if'], $result); + if (\is_array($subscription['if'])) { + /** @var \Closure $closure */ + $closure = deepclone_from_array($subscription['if']); + + /** @var bool $result */ + $result = $closure($entity); + } else { + $result = $this->getExpressionLanguage()->evaluate($subscription['if'], ['obj' => $entity]); + + if (!\is_bool($result)) { + throw new InvalidIfExpressionResultException($subscription['routeName'], $subscription['if'], $result); + } } if (!$result) { diff --git a/tests/Application/Php85ApplicationTest.php b/tests/Application/Php85ApplicationTest.php new file mode 100644 index 00000000..9e4ccfed --- /dev/null +++ b/tests/Application/Php85ApplicationTest.php @@ -0,0 +1,53 @@ += 8.5.0')] +final class Php85ApplicationTest extends AbstractKernelTestCase +{ + use InteractsWithPurgatory; + + private EntityManagerInterface $entityManager; + + protected function setUp(): void + { + self::initializeApplication(['test_case' => 'Php85TestApplication', 'config' => 'app_config.yaml']); + + $this->entityManager = self::getContainer()->get('doctrine.orm.entity_manager'); + } + + protected function tearDown(): void + { + unset($this->entityManager); + + parent::tearDown(); + } + + /** + * @see PlantController::dryPlantsAction + */ + public function testIfWithClosure(): void + { + $plant = new Plant(waterLevel: 0); + $this->entityManager->persist($plant); + $this->entityManager->flush(); + + self::assertUrlIsPurged('/plants/dry'); + self::clearPurger(); + + $plant = new Plant(waterLevel: 1); + $this->entityManager->persist($plant); + $this->entityManager->flush(); + + self::assertUrlIsNotPurged('/plants/dry'); + } +} diff --git a/tests/Application/Php85ConfigurationTest.php b/tests/Application/Php85ConfigurationTest.php new file mode 100644 index 00000000..3d1a38dc --- /dev/null +++ b/tests/Application/Php85ConfigurationTest.php @@ -0,0 +1,84 @@ += 8.5.0')] +class Php85ConfigurationTest extends AbstractKernelTestCase +{ + private static ?Configuration $configuration; + + public static function setUpBeforeClass(): void + { + parent::setUpBeforeClass(); + + self::initializeApplication(['test_case' => 'Php85TestApplication', 'config' => 'app_config.yaml']); + + self::$configuration = self::getContainer()->get('sofascore.purgatory.configuration_loader')->load(); + + self::ensureKernelShutdown(); + } + + public static function tearDownAfterClass(): void + { + self::$configuration = null; + + parent::tearDownAfterClass(); + } + + #[DataProvider('configurationProvider')] + public function testConfiguration(string $entity, array $subscription): void + { + self::assertSubscriptionExists( + key: $entity, + subscription: $subscription, + ); + } + + public static function configurationProvider(): iterable + { + /* @see PlantController::dryPlantsAction */ + yield [ + 'entity' => Plant::class, + 'subscription' => [ + 'routeName' => 'dry_plants_list', + 'if' => [ + 'classes' => '', + 'objectMeta' => 0, + 'prepared' => [ + 'Sofascore\PurgatoryBundle\Tests\Functional\Php85TestApplication\Controller\PlantController', + 'dryPlantsAction()', + 2, + 0, + 20, + ], + 'mask' => 1, + ], + 'actions' => [Action::Create], + ], + ]; + } + + private static function assertSubscriptionExists(string $key, array $subscription): void + { + self::assertTrue( + condition: self::$configuration->has($key), + message: \sprintf('Failed asserting that the configuration contains a subscription for "%s".', $key), + ); + + self::assertContains( + needle: $subscription, + haystack: self::$configuration->get($key), + message: \sprintf('Failed asserting that the configuration contains the subscription "%s" for the key "%s".', json_encode($subscription), $key), + ); + } +} diff --git a/tests/Attribute/PurgeOnTest.php b/tests/Attribute/PurgeOnTest.php index 17e2ca08..d2fe2f2d 100644 --- a/tests/Attribute/PurgeOnTest.php +++ b/tests/Attribute/PurgeOnTest.php @@ -5,6 +5,7 @@ namespace Sofascore\PurgatoryBundle\Tests\Attribute; use PHPUnit\Framework\Attributes\CoversClass; +use PHPUnit\Framework\Attributes\RequiresFunction; use PHPUnit\Framework\Attributes\TestWith; use PHPUnit\Framework\TestCase; use Sofascore\PurgatoryBundle\Attribute\PurgeOn; @@ -45,4 +46,14 @@ public function testValueNormalization(string $property, mixed $value, mixed $ex self::assertEquals($expectedValue, $purgeOn->$property); } + + #[RequiresFunction('deepclone_to_array')] + public function testIfWithClosure(): void + { + $if = static fn (\stdClass $obj): bool => true; + + $purgeOn = new PurgeOn(\stdClass::class, if: $if); + + self::assertSame($if, $purgeOn->if); + } } diff --git a/tests/Cache/Configuration/ConfigurationLoaderTest.php b/tests/Cache/Configuration/ConfigurationLoaderTest.php index b6584e70..133bbd1f 100644 --- a/tests/Cache/Configuration/ConfigurationLoaderTest.php +++ b/tests/Cache/Configuration/ConfigurationLoaderTest.php @@ -6,6 +6,7 @@ use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\DataProvider; +use PHPUnit\Framework\Attributes\RequiresPhp; use PHPUnit\Framework\TestCase; use Sofascore\PurgatoryBundle\Attribute\RouteParamValue\CompoundValues; use Sofascore\PurgatoryBundle\Attribute\RouteParamValue\EnumValues; @@ -16,6 +17,7 @@ use Sofascore\PurgatoryBundle\Cache\Subscription\PurgeSubscription; use Sofascore\PurgatoryBundle\Cache\Subscription\PurgeSubscriptionProviderInterface; use Sofascore\PurgatoryBundle\Listener\Enum\Action; +use Sofascore\PurgatoryBundle\Tests\Fixtures\ClosureIfHolder; use Sofascore\PurgatoryBundle\Tests\Fixtures\DummyStringEnum; use Symfony\Component\ExpressionLanguage\Expression; use Symfony\Component\Routing\Route; @@ -228,4 +230,36 @@ class: 'Foo', ], ]; } + + #[RequiresPhp('>= 8.5.0')] + public function testSubscriptionsWithClosureIf(): void + { + $purgeSubscriptionProvider = $this->createMock(PurgeSubscriptionProviderInterface::class); + $purgeSubscriptionProvider->expects(self::once()) + ->method('provide') + ->willReturn([ + new PurgeSubscription( + class: \stdClass::class, + property: null, + routeParams: [], + routeName: 'app_route_foo', + route: new Route('/foo'), + actions: Action::cases(), + if: ClosureIfHolder::RETURNS_TRUE, + ), + ]); + + $loader = new ConfigurationLoader($purgeSubscriptionProvider); + + self::assertInstanceOf(Configuration::class, $configuration = $loader->load()); + self::assertSame([ + 'stdClass' => [ + [ + 'routeName' => 'app_route_foo', + 'if' => deepclone_to_array(ClosureIfHolder::RETURNS_TRUE), + 'actions' => Action::cases(), + ], + ], + ], $configuration->toArray()); + } } diff --git a/tests/Cache/Subscription/Fixtures/DummyEntity.php b/tests/Cache/Subscription/Fixtures/DummyEntity.php new file mode 100644 index 00000000..9c849c96 --- /dev/null +++ b/tests/Cache/Subscription/Fixtures/DummyEntity.php @@ -0,0 +1,13 @@ +createMock(RouteMetadataProviderInterface::class); + $routeMetadataProvider->expects(self::once()) + ->method('provide') + ->willReturnCallback(static function () use ($routeMetadata) { + yield $routeMetadata; + }); + + $targetResolverLocator = $this->createMock(ContainerInterface::class); + $targetResolverLocator->expects(self::never())->method('get'); + + $purgeSubscriptionProvider = new PurgeSubscriptionProvider( + subscriptionResolvers: [], + routeMetadataProviders: [$routeMetadataProvider], + managerRegistry: self::createStub(ManagerRegistry::class), + targetResolverLocator: $targetResolverLocator, + expressionLanguage: null, + ); + + /** @var PurgeSubscription[] $propertySubscriptions */ + $propertySubscriptions = [...$purgeSubscriptionProvider->provide()]; + + self::assertCount(\count($expectedSubscriptions), $propertySubscriptions); + self::assertEquals($expectedSubscriptions, $propertySubscriptions); + } + + public static function providerRouteMetadataWithPhp85Features(): iterable + { + $route = new Route('/foo'); + $if = static function (DummyEntity $entity): bool { + return $entity->getData() > 0; + }; + + yield 'PurgeOn with closure' => [ + 'routeMetadata' => new RouteMetadata( + routeName: 'foo', + route: $route, + purgeOn: new PurgeOn( + class: DummyEntity::class, + if: $if, + ), + reflectionMethod: new \ReflectionMethod(DummyController::class, 'barAction'), + ), + 'expectedSubscriptions' => [ + new PurgeSubscription( + class: DummyEntity::class, + property: null, + routeParams: [], + routeName: 'foo', + route: $route, + actions: null, + if: $if, + ), + ], + ]; + } + + #[RequiresFunction('deepclone_to_array')] + #[DataProvider('provideInvalidClosures')] + public function testInvalidClosures(\Closure $if, string $expectedMessage): void + { + $routeMetadataProvider = $this->createMock(RouteMetadataProviderInterface::class); + $routeMetadataProvider->expects(self::once()) + ->method('provide') + ->willReturnCallback(static function () use ($if): iterable { + yield new RouteMetadata( + routeName: 'foo', + route: new Route('/{foo}'), + purgeOn: new PurgeOn( + class: DummyEntity::class, + if: $if, + ), + reflectionMethod: null, + ); + }); + + $purgeSubscriptionProvider = new PurgeSubscriptionProvider( + subscriptionResolvers: [], + routeMetadataProviders: [$routeMetadataProvider], + managerRegistry: self::createStub(ManagerRegistry::class), + targetResolverLocator: self::createStub(ContainerInterface::class), + expressionLanguage: self::createStub(ExpressionLanguage::class), + ); + + $this->expectException(InvalidIfClosureException::class); + $this->expectExceptionMessage($expectedMessage); + + [...$purgeSubscriptionProvider->provide()]; + } + + public static function provideInvalidClosures(): iterable + { + yield 'invalid return type (union)' => [ + 'if' => static function (DummyEntity $entity): int|string { + return $entity->getData(); + }, + 'expectedMessage' => 'Return type must be bool', + ]; + + yield 'nullable return type' => [ + 'if' => static function (DummyEntity $entity): ?bool { + return null; + }, + 'expectedMessage' => 'Return type must be bool', + ]; + + yield 'invalid return type' => [ + 'if' => static function (DummyEntity $entity): int { + return $entity->getData(); + }, + 'expectedMessage' => 'Return type must be bool', + ]; + + yield 'too many parameters' => [ + 'if' => static function (DummyEntity $entity, array $options): bool { + return $entity->getData() > 0; + }, + 'expectedMessage' => 'Closure must have exactly 1 parameter', + ]; + + yield 'invalid parameter type (union)' => [ + 'if' => static function (DummyEntity|int $entity): bool { + return $entity->getData() > 0; + }, + 'expectedMessage' => 'Parameter in closure must be of type '.DummyEntity::class, + ]; + + yield 'nullable parameter type' => [ + 'if' => static function (?DummyEntity $entity): bool { + return $entity?->getData() > 0; + }, + 'expectedMessage' => 'Parameter in closure must be of type '.DummyEntity::class, + ]; + + yield 'invalid parameter type' => [ + 'if' => static function (\stdClass $entity): bool { + return true; + }, + 'expectedMessage' => 'Parameter in closure must be of type '.DummyEntity::class, + ]; + + yield 'closure bound to an instance' => [ + 'if' => (new class { + public function getIf(): \Closure + { + return function (DummyEntity $entity): bool { + return $this instanceof self; + }; + } + })->getIf(), + 'expectedMessage' => 'Closure must be static', + ]; + + $number = 1; + yield 'captured scalar variable' => [ + 'if' => static function (DummyEntity $entity) use ($number): bool { + return $entity->getData() > $number; + }, + 'expectedMessage' => 'Closure must not capture variables', + ]; + + $object = new DummyEntity(); + yield 'captured object variable' => [ + 'if' => static function (DummyEntity $entity) use ($object): bool { + return $entity->getData() > $object->getData(); + }, + 'expectedMessage' => 'Closure must not capture variables', + ]; + } } diff --git a/tests/Command/Php85DebugCommandTest.php b/tests/Command/Php85DebugCommandTest.php new file mode 100644 index 00000000..6f52da9c --- /dev/null +++ b/tests/Command/Php85DebugCommandTest.php @@ -0,0 +1,64 @@ += 8.5.0')] +final class Php85DebugCommandTest extends AbstractKernelTestCase +{ + private string|false $colSize; + private CommandTester $command; + + protected function setUp(): void + { + $this->colSize = getenv('COLUMNS'); + putenv('COLUMNS=300'); + + self::initializeApplication(['test_case' => 'Php85TestApplication', 'config' => 'app_config.yaml']); + + $this->command = new CommandTester( + command: (new Application(self::$kernel))->find('purgatory:debug'), + ); + } + + protected function tearDown(): void + { + putenv($this->colSize ? 'COLUMNS='.$this->colSize : 'COLUMNS'); + + unset( + $this->colSize, + $this->command, + ); + + parent::tearDown(); + } + + public function testClosureIfIsRendered(): void + { + $this->command->execute([ + '--all' => true, + ]); + + $this->command->assertCommandIsSuccessful(); + + $expectedClosure = <<<'PHP' + Condition static function (Plant $plant): bool { + return 0 === $plant->getWaterLevel(); + } + PHP; + + self::assertStringContainsString( + needle: $expectedClosure, + haystack: preg_replace('/ +$/m', '', $this->command->getDisplay()), + ); + } +} diff --git a/tests/Fixtures/ClosureIfHolder.php b/tests/Fixtures/ClosureIfHolder.php new file mode 100644 index 00000000..b18419a8 --- /dev/null +++ b/tests/Fixtures/ClosureIfHolder.php @@ -0,0 +1,24 @@ +getWaterLevel(); + }, + actions: Action::Create, + )] + public function dryPlantsAction(): void + { + } +} diff --git a/tests/Functional/Php85TestApplication/Entity/Plant.php b/tests/Functional/Php85TestApplication/Entity/Plant.php new file mode 100644 index 00000000..17914b26 --- /dev/null +++ b/tests/Functional/Php85TestApplication/Entity/Plant.php @@ -0,0 +1,39 @@ +waterLevel = $waterLevel; + } + + public function getId(): int + { + return $this->id; + } + + public function getWaterLevel(): int + { + return $this->waterLevel; + } + + public function setWaterLevel(int $waterLevel): void + { + $this->waterLevel = $waterLevel; + } +} diff --git a/tests/Functional/Php85TestApplication/config/app_config.yaml b/tests/Functional/Php85TestApplication/config/app_config.yaml new file mode 100644 index 00000000..8c12ff00 --- /dev/null +++ b/tests/Functional/Php85TestApplication/config/app_config.yaml @@ -0,0 +1,7 @@ +services: + _defaults: + autoconfigure: true + autowire: true + + Sofascore\PurgatoryBundle\Tests\Functional\Php85TestApplication\: + resource: '../' diff --git a/tests/RouteProvider/UpdatedEntityRouteProviderTest.php b/tests/RouteProvider/UpdatedEntityRouteProviderTest.php index ddf3532e..07f37fd2 100644 --- a/tests/RouteProvider/UpdatedEntityRouteProviderTest.php +++ b/tests/RouteProvider/UpdatedEntityRouteProviderTest.php @@ -9,6 +9,7 @@ use Doctrine\ORM\Mapping\ClassMetadata; use Doctrine\ORM\PersistentCollection; use PHPUnit\Framework\Attributes\CoversClass; +use PHPUnit\Framework\Attributes\RequiresPhp; use PHPUnit\Framework\Attributes\TestWith; use PHPUnit\Framework\TestCase; use Psr\Container\ContainerInterface; @@ -29,6 +30,7 @@ use Sofascore\PurgatoryBundle\RouteProvider\PropertyAccess\PurgatoryPropertyAccessor; use Sofascore\PurgatoryBundle\RouteProvider\PurgeRoute; use Sofascore\PurgatoryBundle\RouteProvider\UpdatedEntityRouteProvider; +use Sofascore\PurgatoryBundle\Tests\Fixtures\ClosureIfHolder; use Sofascore\PurgatoryBundle\Tests\Fixtures\DummyStringEnum; use Symfony\Component\DependencyInjection\ServiceLocator; use Symfony\Component\ExpressionLanguage\ExpressionLanguage; @@ -448,6 +450,59 @@ public function testExceptionIsThrownOnInvalidIfReturnType(mixed $ifResult, stri [...$routeProvider->provideRoutesFor(Action::Update, new \stdClass(), [])]; } + #[RequiresPhp('>= 8.5.0')] + public function testProvideRoutesToPurgeWithClosureIf(): void + { + $routeProvider = $this->createRouteProvider([ + 'stdClass' => [ + [ + 'routeName' => 'foo_route', + 'if' => deepclone_to_array(ClosureIfHolder::RETURNS_TRUE), + ], + ], + 'stdClass::foo' => [ + [ + 'routeName' => 'bar_route', + 'if' => deepclone_to_array(ClosureIfHolder::RETURNS_TRUE), + ], + [ + 'routeName' => 'baz_route', + 'routeParams' => [ + 'param1' => [ + 'type' => PropertyValues::type(), + 'values' => ['foo', 'bar'], + ], + 'param2' => [ + 'type' => PropertyValues::type(), + 'values' => ['baz'], + ], + ], + 'if' => deepclone_to_array(ClosureIfHolder::RETURNS_FALSE), + ], + ], + ], false); + + $entity = new \stdClass(); + + self::assertTrue($routeProvider->supports(Action::Update, $entity)); + self::assertFalse($routeProvider->supports(Action::Delete, $entity)); + self::assertFalse($routeProvider->supports(Action::Create, $entity)); + + $routes = [...$routeProvider->provideRoutesFor( + action: Action::Update, + entity: $entity, + entityChangeSet: [ + 'foo' => ['old', 'new'], + ], + )]; + + self::assertCount(2, $routes); + self::assertContainsOnlyInstancesOf(PurgeRoute::class, $routes); + + self::assertSame(['name' => 'foo_route', 'params' => []], (array) $routes[0]); + self::assertSame(['name' => 'bar_route', 'params' => []], (array) $routes[1]); + } + private function createRouteProvider(array $configuration, bool $withExpressionLang): UpdatedEntityRouteProvider { $configurationLoader = self::createStub(ConfigurationLoaderInterface::class);