From 4d8df97c44f0219b62272c75344b2f5f5946ec2c Mon Sep 17 00:00:00 2001 From: sidux Date: Mon, 8 Jul 2024 12:33:07 +0200 Subject: [PATCH 1/7] chore(cache): improve invalidation logic --- .envrc | 3 + .gitignore | 9 + devenv.lock | 122 ++++++++++ devenv.nix | 26 ++ devenv.yaml | 15 ++ src/Attribute/Cache/Tag.php | 14 ++ .../Contract/Cache/AutoTaggable.php | 1 - src/Interceptor/Impl/CacheInterceptor.php | 128 ++-------- src/Interceptor/Impl/CacheTagsTrait.php | 227 ++++++++++++++++++ .../Impl/InvalidateCacheInterceptor.php | 98 ++------ .../ClassWithInvalidateCacheAttributes.php | 2 +- tests/Interceptor/CacheInterceptorTest.php | 2 +- 12 files changed, 458 insertions(+), 189 deletions(-) create mode 100644 .envrc create mode 100644 devenv.lock create mode 100644 devenv.nix create mode 100644 devenv.yaml create mode 100644 src/Attribute/Cache/Tag.php create mode 100644 src/Interceptor/Impl/CacheTagsTrait.php diff --git a/.envrc b/.envrc new file mode 100644 index 0000000..5bf8fc1 --- /dev/null +++ b/.envrc @@ -0,0 +1,3 @@ +source_url "https://raw.githubusercontent.com/cachix/devenv/95f329d49a8a5289d31e0982652f7058a189bfca/direnvrc" "sha256-d+8cBpDfDBj41inrADaJt+bDWhOktwslgoP5YiGJ1v0=" + +use devenv \ No newline at end of file diff --git a/.gitignore b/.gitignore index 03c660e..817086d 100644 --- a/.gitignore +++ b/.gitignore @@ -3,3 +3,12 @@ build/ vendor/ .phpunit.result.cache tests/cache +# Devenv +.devenv* +devenv.local.nix + +# direnv +.direnv + +# pre-commit +.pre-commit-config.yaml diff --git a/devenv.lock b/devenv.lock new file mode 100644 index 0000000..ddb0086 --- /dev/null +++ b/devenv.lock @@ -0,0 +1,122 @@ +{ + "nodes": { + "devenv": { + "locked": { + "dir": "src/modules", + "lastModified": 1720180409, + "owner": "cachix", + "repo": "devenv", + "rev": "7b3ed618571f0d14655b05f7b1c6ef600904383a", + "treeHash": "14b4b6bc32582a78300257c3b618d821557eb530", + "type": "github" + }, + "original": { + "dir": "src/modules", + "owner": "cachix", + "repo": "devenv", + "type": "github" + } + }, + "flake-compat": { + "flake": false, + "locked": { + "lastModified": 1696426674, + "owner": "edolstra", + "repo": "flake-compat", + "rev": "0f9255e01c2351cc7d116c072cb317785dd33b33", + "treeHash": "2addb7b71a20a25ea74feeaf5c2f6a6b30898ecb", + "type": "github" + }, + "original": { + "owner": "edolstra", + "repo": "flake-compat", + "type": "github" + } + }, + "gitignore": { + "inputs": { + "nixpkgs": [ + "pre-commit-hooks", + "nixpkgs" + ] + }, + "locked": { + "lastModified": 1709087332, + "owner": "hercules-ci", + "repo": "gitignore.nix", + "rev": "637db329424fd7e46cf4185293b9cc8c88c95394", + "treeHash": "ca14199cabdfe1a06a7b1654c76ed49100a689f9", + "type": "github" + }, + "original": { + "owner": "hercules-ci", + "repo": "gitignore.nix", + "type": "github" + } + }, + "nixpkgs": { + "locked": { + "lastModified": 1716977621, + "owner": "cachix", + "repo": "devenv-nixpkgs", + "rev": "4267e705586473d3e5c8d50299e71503f16a6fb6", + "treeHash": "6d9f1f7ca0faf1bc2eeb397c78a49623260d3412", + "type": "github" + }, + "original": { + "owner": "cachix", + "ref": "rolling", + "repo": "devenv-nixpkgs", + "type": "github" + } + }, + "nixpkgs-stable": { + "locked": { + "lastModified": 1719957072, + "owner": "NixOS", + "repo": "nixpkgs", + "rev": "7144d6241f02d171d25fba3edeaf15e0f2592105", + "treeHash": "415bf0e03835797927c1b2cb46a557bcecc36673", + "type": "github" + }, + "original": { + "owner": "NixOS", + "ref": "nixos-23.11", + "repo": "nixpkgs", + "type": "github" + } + }, + "pre-commit-hooks": { + "inputs": { + "flake-compat": "flake-compat", + "gitignore": "gitignore", + "nixpkgs": [ + "nixpkgs" + ], + "nixpkgs-stable": "nixpkgs-stable" + }, + "locked": { + "lastModified": 1719259945, + "owner": "cachix", + "repo": "pre-commit-hooks.nix", + "rev": "0ff4381bbb8f7a52ca4a851660fc7a437a4c6e07", + "treeHash": "1a76ff89a9d4017b48abbb1bad8837b35d604ffc", + "type": "github" + }, + "original": { + "owner": "cachix", + "repo": "pre-commit-hooks.nix", + "type": "github" + } + }, + "root": { + "inputs": { + "devenv": "devenv", + "nixpkgs": "nixpkgs", + "pre-commit-hooks": "pre-commit-hooks" + } + } + }, + "root": "root", + "version": 7 +} diff --git a/devenv.nix b/devenv.nix new file mode 100644 index 0000000..49c6fbe --- /dev/null +++ b/devenv.nix @@ -0,0 +1,26 @@ +{ pkgs, lib, config, inputs, ... }: + +{ + languages.php = { + enable = true; + version = lib.mkDefault "8.2"; + extensions = [ "xdebug" ]; + + ini = '' + memory_limit = -1 + opcache.enable = 1 + opcache.revalidate_freq = 0 + opcache.validate_timestamps = 1 + opcache.max_accelerated_files = 30000 + opcache.memory_consumption = 256M + opcache.interned_strings_buffer = 20 + realpath_cache_ttl = 3600 + xdebug.idekey = "PHPSTORM" + xdebug.start_with_request = "yes" + zend.assertions = 1 + date.timezone = "Europe/Paris" + xdebug.output_dir = ".devenv/state/xdebug" + xdebug.mode = "off" + ''; + }; +} diff --git a/devenv.yaml b/devenv.yaml new file mode 100644 index 0000000..116a2ad --- /dev/null +++ b/devenv.yaml @@ -0,0 +1,15 @@ +# yaml-language-server: $schema=https://devenv.sh/devenv.schema.json +inputs: + nixpkgs: + url: github:cachix/devenv-nixpkgs/rolling + +# If you're using non-OSS software, you can set allowUnfree to true. +# allowUnfree: true + +# If you're willing to use a package that's vulnerable +# permittedInsecurePackages: +# - "openssl-1.1.1w" + +# If you have more than one devenv you can merge them +#imports: +# - ./backend diff --git a/src/Attribute/Cache/Tag.php b/src/Attribute/Cache/Tag.php new file mode 100644 index 0000000..780f165 --- /dev/null +++ b/src/Attribute/Cache/Tag.php @@ -0,0 +1,14 @@ + */ - public static function getHits(?string $poolName = self::DEFAULT_POOL_NAME): array + public static function getHits(?string $poolName = null): array { + $poolName ??= self::DEFAULT_POOL_NAME; return self::$hits[$poolName] ?? []; } /** * @return array */ - public static function getMisses(?string $poolName = self::DEFAULT_POOL_NAME): array + public static function getMisses(?string $poolName = null): array { + $poolName ??= self::DEFAULT_POOL_NAME; return self::$misses[$poolName] ?? []; } @@ -103,18 +105,19 @@ public function prefix(Instance $instance): Response self::$hits[$pool] = self::$hits[$pool] ?? []; self::$hits[$pool][] = $cacheKey; - + $data = $data->get(); + $tags = $this->getTags($instance, $attribute, $data); foreach ($missedPools as $missedPool) { $handler->save( $missedPool, $cacheKey, - $data->get(), + $data, $attribute->ttl ?? $this->config->defaultTtl, - $this->getTags($instance, $attribute, $data) + $tags ); } - return new Response($data->get(), true); + return new Response($data, true); } return new Response(null, false); @@ -272,107 +275,6 @@ private function getInnerCode(\ReflectionMethod|\ReflectionClass $reflection): s return $code; } - /** - * @return array - */ - private function getTags(Instance $instance, Cache $attribute, mixed $response = null): array - { - $parameters = $instance->getMethod() - ->getParameters(); - - $tags = array_map( - static fn (string $expression) => Expression::evaluateToString($expression, $parameters), - $attribute->tags - ); - - if ($response !== null) { - $tags = array_values(array_filter([ - ...$tags, - ...$this->guessObjectsTags( - $response, - $this->config->autoTagsExcludedClasses - ), - ])); - } - - return $tags; - } - - /** - * @param array $excludedClasses - * @param array $registeredTags - * - * @return array - */ - private function guessObjectsTags(mixed $object, array $excludedClasses = [], array $registeredTags = []): array - { - if (!\is_object($object) && !is_iterable($object)) { - return $registeredTags; - } - - foreach ($excludedClasses as $excludedClass) { - if ($object instanceof $excludedClass) { - return $registeredTags; - } - } - - if (is_iterable($object)) { - foreach ($object as $item) { - $registeredTags = $this->guessObjectsTags($item, $excludedClasses, $registeredTags); - } - - return $registeredTags; - } - - if (!$object instanceof AutoTaggable) { - return $registeredTags; - } - - $tag = $this->buildTag($object); - - if (isset($registeredTags[$tag])) { - return $registeredTags; - } - - $registeredTags[$tag] = $tag; - - $ref = new \ReflectionClass($object); - - foreach ($ref->getProperties() as $propRef) { - $subObject = $this->getPropertyValue($ref, $object, $propRef->getName()); - - $registeredTags = $this->guessObjectsTags($subObject, $excludedClasses, $registeredTags); - } - - return $registeredTags; - } - - private function buildTag(AutoTaggable $object): string - { - return str_replace('\\', '.', \get_class($object)) . '.' . $object->getId(); - } - - /** - * @param \ReflectionClass $ref - */ - private function getPropertyValue(\ReflectionClass $ref, object $object, string $propertyName): mixed - { - $getter = 'get' . ucfirst($propertyName); - $refMethod = $ref->hasMethod($getter) ? $ref->getMethod($getter) : null; - if ($refMethod !== null && $refMethod->isPublic() && \count($refMethod->getParameters()) === 0) { - return $refMethod->invoke($object); - } - - $propRef = $ref->getProperty($propertyName); - if (!$propRef->isInitialized($object)) { - return null; - } - - $propRef->setAccessible(true); - - return $propRef->getValue($object); - } - /** * @param array $parameters */ @@ -385,4 +287,12 @@ private function getParametersHash(array $parameters): string return $identifier; } + + /** + * @return array + */ + private function getAutoTagsExcludedClasses(): array + { + return $this->config->autoTagsExcludedClasses ?? []; + } } diff --git a/src/Interceptor/Impl/CacheTagsTrait.php b/src/Interceptor/Impl/CacheTagsTrait.php new file mode 100644 index 0000000..4692a87 --- /dev/null +++ b/src/Interceptor/Impl/CacheTagsTrait.php @@ -0,0 +1,227 @@ + + */ + abstract private function getAutoTagsExcludedClasses(): array; + + /** + * @return array + */ + private function getTags(Instance $instance, Cache|InvalidateCache $attribute, mixed $response = null): array + { + $parameters = $instance->getMethod() + ->getParameters() + ; + + $tags = array_map( + static fn (string $expression) => Expression::evaluateToString($expression, $parameters), + $attribute->tags + ); + + /** @noinspection PhpConditionCheckedByNextConditionInspection */ + if ($response !== null && \is_object($response)) { + $prefix = str_replace( + ['\\', 'SharedResponse', 'Embedded', '_Shared'], + ['.', '', '', ''], + \get_class($response) + ); + } else { + $prefix = str_replace( + '\\', + '.', + $instance->getReflection()->getName() . $instance->getMethod()->getName() + ); + } + + return array_values( + array_filter([ + ...$tags, + ...$this->guessObjectsTags( + $response, + prefix: $prefix, + excludedClasses: $this->getAutoTagsExcludedClasses(), + ), + ...array_merge(...array_map(fn ($param) => $this->guessObjectsTags( + $param, + prefix: $prefix, + excludedClasses: $this->getAutoTagsExcludedClasses(), + ), $parameters)), + ]) + ); + } + + /** + * @param array $excludedClasses + * @param array $registeredTags + * + * @return array + */ + private function guessObjectsTags( + mixed $object, + string $prefix, + array $excludedClasses = [], + array $registeredTags = [] + ): array { + if ($object === null) { + return $registeredTags; + } + + if (!\is_object($object) && !is_iterable($object)) { + return $registeredTags; + } + + foreach ($excludedClasses as $excludedClass) { + if ($object instanceof $excludedClass) { + return $registeredTags; + } + } + + if (is_iterable($object)) { + foreach ($object as $item) { + $registeredTags = $this->guessObjectsTags($item, $prefix, $excludedClasses, $registeredTags); + } + + return $registeredTags; + } + + if (!$object instanceof AutoTaggable) { + return $registeredTags; + } + + $ref = new \ReflectionObject($object); + $tags = $this->buildTags($object, $ref, $prefix); + + foreach ($tags as $tag) { + if (isset($registeredTags[$tag])) { + return $registeredTags; + } + $registeredTags[$tag] = $tag; + } + + foreach ($ref->getProperties() as $propRef) { + $subObject = $this->getPropertyValue($ref, $object, $propRef->getName()); + + $registeredTags = $this->guessObjectsTags($subObject, $prefix, $excludedClasses, $registeredTags); + } + + return $registeredTags; + } + + /** + * @return string[] + */ + private function buildTags(AutoTaggable $object, \ReflectionObject $ref, string $prefix): array + { + $tags = []; + /** @var \ReflectionMethod|\ReflectionProperty $member */ + foreach ([...$ref->getMethods(), ...$ref->getProperties()] as $member) { + $tagsAttributes = $member->getAttributes(Tag::class); + if (\count($tagsAttributes) === 0) { + if (!$member->isPublic()) { + continue; + } + if (\in_array($member->getName(), ['id', 'getId', 'userId', 'getUserId'], true)) { + $tags[] = $this->buildTagName( + $object, + $member, + $prefix, + ); + } + continue; + } + + foreach ($tagsAttributes as $tagAttribute) { + if ($member instanceof \ReflectionMethod) { + if (!$member->isPublic() || \count($member->getParameters()) > 0) { + throw new \LogicException( + sprintf( + 'Method %s::%s must be public and have no parameters to be used as a tag.', + $ref->getName(), + $member->getName() + ) + ); + } + $tags[] = $this->buildTagName( + $object, + $member, + $prefix, + $tagAttribute + ); + } + + if ($member instanceof \ReflectionProperty) { + if (!$member->isInitialized($object)) { + throw new \LogicException( + sprintf( + 'Property %s::%s must be initialized to be used as a tag.', + $ref->getName(), + $member->getName() + ) + ); + } + $tags[] = $this->buildTagName( + $object, + $member, + $prefix, + $tagAttribute + ); + } + } + } + + return array_unique($tags); + } + + /** + * @param \ReflectionAttribute|null $tagAttribute + */ + private function buildTagName( + AutoTaggable $object, + \ReflectionProperty|\ReflectionMethod $member, + ?string $prefix, + ?\ReflectionAttribute $tagAttribute = null + ): string { + $value = $member instanceof \ReflectionProperty + ? $member->getValue() + : $member->invoke($object, []); + + if ($tagAttribute?->newInstance()?->prefix !== null) { + return $tagAttribute->newInstance()->prefix . $value; + } + + return $prefix . '.' . $member->getName() . '.' . $value; + } + + /** + * @param \ReflectionClass $ref + */ + private function getPropertyValue(\ReflectionClass $ref, object $object, string $propertyName): mixed + { + $getter = 'get' . ucfirst($propertyName); + $refMethod = $ref->hasMethod($getter) ? $ref->getMethod($getter) : null; + if ($refMethod !== null && $refMethod->isPublic() && \count($refMethod->getParameters()) === 0) { + return $refMethod->invoke($object); + } + + $propRef = $ref->getProperty($propertyName); + if (!$propRef->isInitialized($object)) { + return null; + } + + return $propRef->getValue($object); + } +} diff --git a/src/Interceptor/Impl/InvalidateCacheInterceptor.php b/src/Interceptor/Impl/InvalidateCacheInterceptor.php index 7e18b27..d54a3b2 100644 --- a/src/Interceptor/Impl/InvalidateCacheInterceptor.php +++ b/src/Interceptor/Impl/InvalidateCacheInterceptor.php @@ -14,6 +14,8 @@ final class InvalidateCacheInterceptor extends AbstractInterceptor implements SuffixInterceptor { + use CacheTagsTrait; + private string $defaultPoolName = 'default'; public function suffix(Instance $instance): Response @@ -28,105 +30,47 @@ public function suffix(Instance $instance): Response $handler = $this->getHandlers(CacheHandler::class, $attribute)[0]; $pools = \count($attribute->pools) === 0 ? [$this->defaultPoolName] : $attribute->pools; - $tags = $this->getTags($instance, $attribute); - - foreach ($pools as $pool) { - $handler->invalidateTags($pool, $tags); - } - - return new Response(); - } - - public function supportsSuffix(Instance $instance): bool - { - return $instance->getMethod() - ->hasAttribute(InvalidateCache::class); - } - - public function getSuffixPriority(): int - { - return 40; - } - - /** - * @return array - */ - private function getTags(Instance $instance, InvalidateCache $attribute): array - { $parameters = $instance->getMethod() ->getParameters() ; + $response = $instance->getMethod()->getResponse(); $tags = array_map( static fn (string $expression) => Expression::evaluateToString($expression, $parameters), $attribute->tags ); - if (\count($tags) > 0) { - return $tags; + if (\count($tags) === 0) { + $tags = $this->getTags($instance, $attribute, $response); } - $guessedTags = array_values( - array_filter( - $this->guessObjectsTags($instance->getMethod()->getResponse()) - ) - ); + if (\count($tags) === 0) { + throw new \RuntimeException('No tags found for invalidation'); + } - if (\count($guessedTags) === 0) { - throw new \RuntimeException('No tags found for method ' . $instance->getMethod()->getName()); + foreach ($pools as $pool) { + $handler->invalidateTags($pool, $tags); } - return $guessedTags; + return new Response(); } - /** - * @return array - */ - private function guessObjectsTags(mixed $objects): array + public function supportsSuffix(Instance $instance): bool { - if (!is_iterable($objects)) { - $objects = [$objects]; - } - - $tags = []; - foreach ($objects as $object) { - if (!\is_object($object)) { - continue; - } - - $ref = new \ReflectionClass($object); - $id = $this->getPropertyValue($ref, $object, 'id'); - if ($id === false || $id === null) { - continue; - } - $tag = $this->getNormalizedNamespace($object) . '.' . $id; - $tags[$tag] = $tag; - } - - return $tags; + return $instance->getMethod() + ->hasAttribute(InvalidateCache::class); } - /** - * @param \ReflectionClass $ref - */ - private function getPropertyValue(\ReflectionClass $ref, object $object, string $propertyName): mixed + public function getSuffixPriority(): int { - $getter = 'get' . ucfirst($propertyName); - $refMethod = $ref->hasMethod($getter) ? $ref->getMethod($getter) : null; - if ($refMethod !== null && $refMethod->isPublic()) { - return $refMethod->invoke($object); - } - $propRef = $ref->getProperty($propertyName); - if (!$propRef->isInitialized($object)) { - return false; - } - $propRef->setAccessible(true); - - return $propRef->getValue($object); + return 40; } - private function getNormalizedNamespace(object $object): string + /** + * @return array + */ + private function getAutoTagsExcludedClasses(): array { - return str_replace('\\', '.', \get_class($object)); + return []; } } diff --git a/tests/Double/Stub/Cache/ClassWithInvalidateCacheAttributes.php b/tests/Double/Stub/Cache/ClassWithInvalidateCacheAttributes.php index 66017f7..c710817 100644 --- a/tests/Double/Stub/Cache/ClassWithInvalidateCacheAttributes.php +++ b/tests/Double/Stub/Cache/ClassWithInvalidateCacheAttributes.php @@ -47,7 +47,7 @@ public function methodWithInvalidateCacheButNoTag(): ResponseStub return new ResponseStub(); } - #[InvalidateCache(tags: ['"OpenClassrooms.ServiceProxy.Tests.Double.Stub.Cache.ResponseStub.12"'])] + #[InvalidateCache(tags: ['"OpenClassrooms.ServiceProxy.Tests.Double.Stub.Cache.ResponseStub.getId.12"'])] public function methodWithInvalidateCacheAndExplicitTag(): ResponseStub { return new ResponseStub(); diff --git a/tests/Interceptor/CacheInterceptorTest.php b/tests/Interceptor/CacheInterceptorTest.php index 2732403..279c72e 100644 --- a/tests/Interceptor/CacheInterceptorTest.php +++ b/tests/Interceptor/CacheInterceptorTest.php @@ -239,7 +239,7 @@ public function testMethodCacheIsAutoTaggedFromResponse(): void $this->assertNotEmpty($this->cacheInterceptor::getHits()); $this->assertEmpty($this->cacheInterceptor::getMisses()); - $tagToInvalidate = str_replace('\\', '.', ResponseStub::class) . '.' . ResponseStub::ID; + $tagToInvalidate = str_replace('\\', '.', ResponseStub::class) . '.getId.' . ResponseStub::ID; $this->cacheHandlerMock->invalidateTags('default', [$tagToInvalidate]); From c1a51c2f14b2af7eecf3dd92ec10a6814225b96e Mon Sep 17 00:00:00 2001 From: sidux Date: Mon, 8 Jul 2024 15:35:25 +0200 Subject: [PATCH 2/7] fix(cache): fix phpstan type extraction compatiblity for generics --- src/Helper/TypesExtractor.php | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/Helper/TypesExtractor.php b/src/Helper/TypesExtractor.php index 66b210f..05238be 100644 --- a/src/Helper/TypesExtractor.php +++ b/src/Helper/TypesExtractor.php @@ -131,10 +131,11 @@ private function getPhpDocTypesNames(string $classContext, string $docComment, a $types = $this->typeHelper->getTypes($tag->value, $nameScope); foreach ($types as $type) { $classNames[] = $type->getClassName() ?? $type->getBuiltinType(); - if ($type->isCollection()) { - foreach ($type->getCollectionValueTypes() as $collectionValueType) { - $classNames[] = $collectionValueType->getClassName() ?? $collectionValueType->getBuiltinType(); - } + foreach ($type->getCollectionValueTypes() as $collectionType) { + $classNames[] = $collectionType->getClassName() ?? $collectionType->getBuiltinType(); + } + foreach ($type->getCollectionKeyTypes() as $collectionType) { + $classNames[] = $collectionType->getClassName() ?? $collectionType->getBuiltinType(); } } } From 6427f80e5da9f123f95390972302a80f2e11173c Mon Sep 17 00:00:00 2001 From: sidux Date: Mon, 8 Jul 2024 15:36:28 +0200 Subject: [PATCH 3/7] chore(cache): force list tags (instead of hashmap) --- src/Interceptor/Impl/CacheTagsTrait.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Interceptor/Impl/CacheTagsTrait.php b/src/Interceptor/Impl/CacheTagsTrait.php index 4692a87..1d790bb 100644 --- a/src/Interceptor/Impl/CacheTagsTrait.php +++ b/src/Interceptor/Impl/CacheTagsTrait.php @@ -55,11 +55,11 @@ private function getTags(Instance $instance, Cache|InvalidateCache $attribute, m prefix: $prefix, excludedClasses: $this->getAutoTagsExcludedClasses(), ), - ...array_merge(...array_map(fn ($param) => $this->guessObjectsTags( + ...array_merge(...array_values(array_map(fn ($param) => $this->guessObjectsTags( $param, prefix: $prefix, excludedClasses: $this->getAutoTagsExcludedClasses(), - ), $parameters)), + ), $parameters))), ]) ); } From a729ffbb91be7249cc1a2e5c36109f2d4b833264 Mon Sep 17 00:00:00 2001 From: sidux Date: Mon, 8 Jul 2024 16:03:40 +0200 Subject: [PATCH 4/7] feat(cache): add cache tags tests --- src/Interceptor/Impl/CacheInterceptor.php | 8 +++- src/Interceptor/Impl/CacheTagsTrait.php | 4 +- .../Stub/Cache/ClassWithCacheAttributes.php | 6 +++ tests/Double/Stub/Cache/Request1Stub.php | 44 +++++++++++++++++++ tests/Interceptor/CacheInterceptorTest.php | 24 ++++++++++ 5 files changed, 82 insertions(+), 4 deletions(-) create mode 100644 tests/Double/Stub/Cache/Request1Stub.php diff --git a/src/Interceptor/Impl/CacheInterceptor.php b/src/Interceptor/Impl/CacheInterceptor.php index 5ce1eee..32f5f96 100644 --- a/src/Interceptor/Impl/CacheInterceptor.php +++ b/src/Interceptor/Impl/CacheInterceptor.php @@ -103,10 +103,14 @@ public function prefix(Instance $instance): Response continue; } - self::$hits[$pool] = self::$hits[$pool] ?? []; - self::$hits[$pool][] = $cacheKey; + $data = $data->get(); $tags = $this->getTags($instance, $attribute, $data); + self::$hits[$pool] = self::$hits[$pool] ?? []; + self::$hits[$pool][] = [ + 'key' => $cacheKey, + 'tags' => $tags, + ]; foreach ($missedPools as $missedPool) { $handler->save( $missedPool, diff --git a/src/Interceptor/Impl/CacheTagsTrait.php b/src/Interceptor/Impl/CacheTagsTrait.php index 1d790bb..db32072 100644 --- a/src/Interceptor/Impl/CacheTagsTrait.php +++ b/src/Interceptor/Impl/CacheTagsTrait.php @@ -196,11 +196,11 @@ private function buildTagName( ?\ReflectionAttribute $tagAttribute = null ): string { $value = $member instanceof \ReflectionProperty - ? $member->getValue() + ? $member->getValue($object) : $member->invoke($object, []); if ($tagAttribute?->newInstance()?->prefix !== null) { - return $tagAttribute->newInstance()->prefix . $value; + return $tagAttribute->newInstance()->prefix . '.' . $value; } return $prefix . '.' . $member->getName() . '.' . $value; diff --git a/tests/Double/Stub/Cache/ClassWithCacheAttributes.php b/tests/Double/Stub/Cache/ClassWithCacheAttributes.php index ab4db92..93faa5b 100644 --- a/tests/Double/Stub/Cache/ClassWithCacheAttributes.php +++ b/tests/Double/Stub/Cache/ClassWithCacheAttributes.php @@ -107,4 +107,10 @@ public function methodWithMultiplePools(): string { return self::DATA; } + + #[Cache] + public function methodWithTaggedRequest(Request1Stub $request1Stub): ResponseStub + { + return new ResponseStub(); + } } diff --git a/tests/Double/Stub/Cache/Request1Stub.php b/tests/Double/Stub/Cache/Request1Stub.php new file mode 100644 index 0000000..058b811 --- /dev/null +++ b/tests/Double/Stub/Cache/Request1Stub.php @@ -0,0 +1,44 @@ +executeAndAssertCacheMiss('ClassWithCache'); $this->executeAndAssertCacheHit('ClassWithCache'); } + + public function testRequestAndTagAttribute(): void + { + $proxy = $this->proxyFactory->createProxy(new ClassWithCacheAttributes()); + $proxy->methodWithTaggedRequest(new Request1Stub()); + + $this->assertEmpty(CacheInterceptor::getHits()); + $this->assertNotEmpty(CacheInterceptor::getMisses()); + + $result = $proxy->methodWithTaggedRequest(new Request1Stub()); + + $this->assertEquals(new ResponseStub(), $result); + $this->assertNotEmpty(CacheInterceptor::getHits()); + $this->assertEquals([ + 'OpenClassrooms.ServiceProxy.Tests.Double.Stub.Cache.ResponseStub.getId.12', + 'OpenClassrooms.ServiceProxy.Tests.Double.Stub.Cache.ResponseStub.getName.test', + 'OpenClassrooms.ServiceProxy.Tests.Double.Stub.Cache.ResponseStub.getUserId.1111', + 'OpenClassrooms.ServiceProxy.Tests.Double.Stub.Cache.ResponseStub.age.10', + 'prefix.paris', + 'OpenClassrooms.ServiceProxy.Tests.Double.Stub.Cache.ResponseStub.id.1', + ], CacheInterceptor::getHits()[0]['tags']); + $this->assertEmpty(CacheInterceptor::getMisses()); + } } From 2d7718702cf88c45ef7f1ddf71ea4e1cde250bd4 Mon Sep 17 00:00:00 2001 From: sidux Date: Mon, 8 Jul 2024 16:27:14 +0200 Subject: [PATCH 5/7] chore(cache): add cache invalidation test --- src/Interceptor/Impl/CacheInterceptor.php | 7 ++--- src/Interceptor/Impl/CacheTagsTrait.php | 30 ++++++++++++------- .../ClassWithInvalidateCacheAttributes.php | 13 +++++++- tests/Double/Stub/Cache/Request2Stub.php | 14 +++++++++ tests/Interceptor/CacheInterceptorTest.php | 16 +++++----- .../InvalidateCacheInterceptorTest.php | 15 ++++++++++ 6 files changed, 72 insertions(+), 23 deletions(-) create mode 100644 tests/Double/Stub/Cache/Request2Stub.php diff --git a/src/Interceptor/Impl/CacheInterceptor.php b/src/Interceptor/Impl/CacheInterceptor.php index 32f5f96..a0a8f3f 100644 --- a/src/Interceptor/Impl/CacheInterceptor.php +++ b/src/Interceptor/Impl/CacheInterceptor.php @@ -23,7 +23,7 @@ final class CacheInterceptor extends AbstractInterceptor implements SuffixInterc private const DEFAULT_POOL_NAME = 'default'; /** - * @var string[][] + * @var array}>> */ private static array $hits = []; @@ -44,7 +44,7 @@ public function __construct( } /** - * @return array + * @return array}> */ public static function getHits(?string $poolName = null): array { @@ -103,12 +103,11 @@ public function prefix(Instance $instance): Response continue; } - $data = $data->get(); $tags = $this->getTags($instance, $attribute, $data); self::$hits[$pool] = self::$hits[$pool] ?? []; self::$hits[$pool][] = [ - 'key' => $cacheKey, + 'key' => $cacheKey, 'tags' => $tags, ]; foreach ($missedPools as $missedPool) { diff --git a/src/Interceptor/Impl/CacheTagsTrait.php b/src/Interceptor/Impl/CacheTagsTrait.php index db32072..3a3d743 100644 --- a/src/Interceptor/Impl/CacheTagsTrait.php +++ b/src/Interceptor/Impl/CacheTagsTrait.php @@ -13,6 +13,15 @@ trait CacheTagsTrait { + private function normalizePrefixName(string $name): string + { + return str_replace( + ['\\', 'SharedResponse', 'Embedded', '_Shared'], + ['.', '', '', ''], + $name, + ); + } + /** * @return array */ @@ -34,15 +43,9 @@ private function getTags(Instance $instance, Cache|InvalidateCache $attribute, m /** @noinspection PhpConditionCheckedByNextConditionInspection */ if ($response !== null && \is_object($response)) { - $prefix = str_replace( - ['\\', 'SharedResponse', 'Embedded', '_Shared'], - ['.', '', '', ''], - \get_class($response) - ); + $prefix = $this->normalizePrefixName(\get_class($response)); } else { - $prefix = str_replace( - '\\', - '.', + $prefix = $this->normalizePrefixName( $instance->getReflection()->getName() . $instance->getMethod()->getName() ); } @@ -199,11 +202,18 @@ private function buildTagName( ? $member->getValue($object) : $member->invoke($object, []); + $memberPrefix = str_replace( + ['get', 'has', 'is'], + ['', '', ''], + mb_strtolower($member->getName()) + ); if ($tagAttribute?->newInstance()?->prefix !== null) { - return $tagAttribute->newInstance()->prefix . '.' . $value; + return $this->normalizePrefixName( + $tagAttribute->newInstance()->prefix + ) . '.' . $memberPrefix . '.' . $value; } - return $prefix . '.' . $member->getName() . '.' . $value; + return $prefix . '.' . $memberPrefix . '.' . $value; } /** diff --git a/tests/Double/Stub/Cache/ClassWithInvalidateCacheAttributes.php b/tests/Double/Stub/Cache/ClassWithInvalidateCacheAttributes.php index c710817..30b0f8c 100644 --- a/tests/Double/Stub/Cache/ClassWithInvalidateCacheAttributes.php +++ b/tests/Double/Stub/Cache/ClassWithInvalidateCacheAttributes.php @@ -47,7 +47,7 @@ public function methodWithInvalidateCacheButNoTag(): ResponseStub return new ResponseStub(); } - #[InvalidateCache(tags: ['"OpenClassrooms.ServiceProxy.Tests.Double.Stub.Cache.ResponseStub.getId.12"'])] + #[InvalidateCache(tags: ['"OpenClassrooms.ServiceProxy.Tests.Double.Stub.Cache.ResponseStub.id.12"'])] public function methodWithInvalidateCacheAndExplicitTag(): ResponseStub { return new ResponseStub(); @@ -64,4 +64,15 @@ public function methodInvalidatingSubResource(): EmbeddedResponseStub { return new EmbeddedResponseStub(); } + + #[Cache] + public function methodWithTaggedRequest(Request1Stub $request1Stub): ResponseStub + { + return new ResponseStub(); + } + + #[InvalidateCache] + public function methodWithInvalidateCacheButNoTagForRequest(Request2Stub $request2Stub): void + { + } } diff --git a/tests/Double/Stub/Cache/Request2Stub.php b/tests/Double/Stub/Cache/Request2Stub.php new file mode 100644 index 0000000..7cf79fb --- /dev/null +++ b/tests/Double/Stub/Cache/Request2Stub.php @@ -0,0 +1,14 @@ +assertNotEmpty($this->cacheInterceptor::getHits()); $this->assertEmpty($this->cacheInterceptor::getMisses()); - $tagToInvalidate = str_replace('\\', '.', ResponseStub::class) . '.getId.' . ResponseStub::ID; + $tagToInvalidate = str_replace('\\', '.', ResponseStub::class) . '.id.' . ResponseStub::ID; $this->cacheHandlerMock->invalidateTags('default', [$tagToInvalidate]); @@ -784,13 +784,13 @@ public function testRequestAndTagAttribute(): void $this->assertEquals(new ResponseStub(), $result); $this->assertNotEmpty(CacheInterceptor::getHits()); $this->assertEquals([ - 'OpenClassrooms.ServiceProxy.Tests.Double.Stub.Cache.ResponseStub.getId.12', - 'OpenClassrooms.ServiceProxy.Tests.Double.Stub.Cache.ResponseStub.getName.test', - 'OpenClassrooms.ServiceProxy.Tests.Double.Stub.Cache.ResponseStub.getUserId.1111', - 'OpenClassrooms.ServiceProxy.Tests.Double.Stub.Cache.ResponseStub.age.10', - 'prefix.paris', - 'OpenClassrooms.ServiceProxy.Tests.Double.Stub.Cache.ResponseStub.id.1', - ], CacheInterceptor::getHits()[0]['tags']); + 'OpenClassrooms.ServiceProxy.Tests.Double.Stub.Cache.ResponseStub.id.12', + 'OpenClassrooms.ServiceProxy.Tests.Double.Stub.Cache.ResponseStub.name.test', + 'OpenClassrooms.ServiceProxy.Tests.Double.Stub.Cache.ResponseStub.userid.1111', + 'OpenClassrooms.ServiceProxy.Tests.Double.Stub.Cache.ResponseStub.age.10', + 'prefix.city.paris', + 'OpenClassrooms.ServiceProxy.Tests.Double.Stub.Cache.ResponseStub.id.1', + ], CacheInterceptor::getHits()[0]['tags']); $this->assertEmpty(CacheInterceptor::getMisses()); } } diff --git a/tests/Interceptor/InvalidateCacheInterceptorTest.php b/tests/Interceptor/InvalidateCacheInterceptorTest.php index 58d0d3f..c7eea2a 100644 --- a/tests/Interceptor/InvalidateCacheInterceptorTest.php +++ b/tests/Interceptor/InvalidateCacheInterceptorTest.php @@ -11,6 +11,8 @@ use OpenClassrooms\ServiceProxy\Tests\CacheTestTrait; use OpenClassrooms\ServiceProxy\Tests\Double\Mock\Cache\CacheHandlerMock; use OpenClassrooms\ServiceProxy\Tests\Double\Stub\Cache\ClassWithInvalidateCacheAttributes; +use OpenClassrooms\ServiceProxy\Tests\Double\Stub\Cache\Request1Stub; +use OpenClassrooms\ServiceProxy\Tests\Double\Stub\Cache\Request2Stub; use OpenClassrooms\ServiceProxy\Tests\ProxyTestTrait; use PHPUnit\Framework\TestCase; @@ -129,4 +131,17 @@ public function testCacheInvalidationWithTagsFromSubResources(): void $this->proxy->methodWithCachedEmbeddedResponse(); $this->assertEmpty($this->cacheInterceptor->getHits()); } + + public function testCacheInvalidationWithRequest(): void + { + $this->proxy->methodWithTaggedRequest(new Request1Stub()); + $this->assertEmpty(CacheInterceptor::getHits()); + + $this->proxy->methodWithTaggedRequest(new Request1Stub()); + $this->assertNotEmpty(CacheInterceptor::getHits()); + + $this->proxy->methodWithInvalidateCacheButNoTagForRequest(new Request2Stub()); + $this->proxy->methodWithTaggedRequest(new Request1Stub()); + $this->assertEmpty(CacheInterceptor::getHits()); + } } From 46b95e3dbe033e0407ec710800d27ca4815d7a45 Mon Sep 17 00:00:00 2001 From: sidux Date: Mon, 8 Jul 2024 18:11:07 +0200 Subject: [PATCH 6/7] fix(cache): fix cache key invalidation when tag changes --- src/Interceptor/Impl/CacheInterceptor.php | 2 +- src/Interceptor/Impl/CacheTagsTrait.php | 18 ++++++++++++++---- .../Impl/InvalidateCacheInterceptor.php | 6 +----- 3 files changed, 16 insertions(+), 10 deletions(-) diff --git a/src/Interceptor/Impl/CacheInterceptor.php b/src/Interceptor/Impl/CacheInterceptor.php index a0a8f3f..55ccc74 100644 --- a/src/Interceptor/Impl/CacheInterceptor.php +++ b/src/Interceptor/Impl/CacheInterceptor.php @@ -189,7 +189,7 @@ private function buildCacheKey(Instance $instance, Cache $attribute): string $instance->getMethod() ->getName(), $this->getParametersHash($instance->getMethod()->getParameters()), - ...$this->getTags($instance, $attribute), + ...$this->getAttributeTags($instance->getMethod()->getParameters(), $attribute), $attribute->ttl, $this->getInnerCode($instance->getMethod()->getReflection()), $this->getTypesInnerCode($instance->getMethod()->getReflection()), diff --git a/src/Interceptor/Impl/CacheTagsTrait.php b/src/Interceptor/Impl/CacheTagsTrait.php index 3a3d743..51104bd 100644 --- a/src/Interceptor/Impl/CacheTagsTrait.php +++ b/src/Interceptor/Impl/CacheTagsTrait.php @@ -13,6 +13,19 @@ trait CacheTagsTrait { + /** + * @param array $parameters + * + * @return array + */ + private function getAttributeTags(array $parameters, Cache|InvalidateCache $attribute): array + { + return array_map( + static fn (string $expression) => Expression::evaluateToString($expression, $parameters), + $attribute->tags + ); + } + private function normalizePrefixName(string $name): string { return str_replace( @@ -36,10 +49,7 @@ private function getTags(Instance $instance, Cache|InvalidateCache $attribute, m ->getParameters() ; - $tags = array_map( - static fn (string $expression) => Expression::evaluateToString($expression, $parameters), - $attribute->tags - ); + $tags = $this->getAttributeTags($parameters, $attribute); /** @noinspection PhpConditionCheckedByNextConditionInspection */ if ($response !== null && \is_object($response)) { diff --git a/src/Interceptor/Impl/InvalidateCacheInterceptor.php b/src/Interceptor/Impl/InvalidateCacheInterceptor.php index d54a3b2..9463f49 100644 --- a/src/Interceptor/Impl/InvalidateCacheInterceptor.php +++ b/src/Interceptor/Impl/InvalidateCacheInterceptor.php @@ -10,7 +10,6 @@ use OpenClassrooms\ServiceProxy\Interceptor\Contract\SuffixInterceptor; use OpenClassrooms\ServiceProxy\Model\Request\Instance; use OpenClassrooms\ServiceProxy\Model\Response\Response; -use OpenClassrooms\ServiceProxy\Util\Expression; final class InvalidateCacheInterceptor extends AbstractInterceptor implements SuffixInterceptor { @@ -35,10 +34,7 @@ public function suffix(Instance $instance): Response ; $response = $instance->getMethod()->getResponse(); - $tags = array_map( - static fn (string $expression) => Expression::evaluateToString($expression, $parameters), - $attribute->tags - ); + $tags = $this->getAttributeTags($parameters, $attribute); if (\count($tags) === 0) { $tags = $this->getTags($instance, $attribute, $response); From e0c90a306cc78c53baf566528445a43a92b7ab45 Mon Sep 17 00:00:00 2001 From: sidux Date: Thu, 18 Jul 2024 19:32:41 +0200 Subject: [PATCH 7/7] fix(cache): fail safe cache --- src/Interceptor/Impl/CacheInterceptor.php | 46 +++++++++++++------ .../Impl/LegacyCacheInterceptor.php | 27 +++++++++-- 2 files changed, 53 insertions(+), 20 deletions(-) diff --git a/src/Interceptor/Impl/CacheInterceptor.php b/src/Interceptor/Impl/CacheInterceptor.php index 55ccc74..737bc77 100644 --- a/src/Interceptor/Impl/CacheInterceptor.php +++ b/src/Interceptor/Impl/CacheInterceptor.php @@ -14,6 +14,8 @@ use OpenClassrooms\ServiceProxy\Interceptor\Exception\InternalCodeRetrievalException; use OpenClassrooms\ServiceProxy\Model\Request\Instance; use OpenClassrooms\ServiceProxy\Model\Response\Response; +use Psr\Log\LoggerInterface; +use Psr\Log\NullLogger; use Symfony\Component\PropertyInfo\Type; final class CacheInterceptor extends AbstractInterceptor implements SuffixInterceptor, PrefixInterceptor @@ -34,10 +36,14 @@ final class CacheInterceptor extends AbstractInterceptor implements SuffixInterc private TypesExtractor $typesExtractor; + private LoggerInterface $logger; + public function __construct( private readonly CacheInterceptorConfig $config, iterable $handlers = [], + ?LoggerInterface $logger = null, ) { + $this->logger = $logger ?? new NullLogger(); parent::__construct($handlers); $this->typesExtractor = new TypesExtractor(); @@ -92,32 +98,42 @@ public function prefix(Instance $instance): Response $missedPools = []; foreach ($pools as $pool) { - $data = $handler->fetch($pool, $cacheKey); + try { + $data = $handler->fetch($pool, $cacheKey); + if (!$data->isHit()) { + $missedPools[] = $pool; - if (!$data->isHit()) { - $missedPools[] = $pool; + self::$misses[$pool] = self::$misses[$pool] ?? []; + self::$misses[$pool][] = $cacheKey; - self::$misses[$pool] = self::$misses[$pool] ?? []; - self::$misses[$pool][] = $cacheKey; + continue; + } + + $data = $data->get(); + $tags = $this->getTags($instance, $attribute, $data); + } catch (\Throwable $e) { + $this->logger->error($e->getMessage(), ['exception' => $e]); + $missedPools[] = $pool; continue; } - - $data = $data->get(); - $tags = $this->getTags($instance, $attribute, $data); self::$hits[$pool] = self::$hits[$pool] ?? []; self::$hits[$pool][] = [ 'key' => $cacheKey, 'tags' => $tags, ]; foreach ($missedPools as $missedPool) { - $handler->save( - $missedPool, - $cacheKey, - $data, - $attribute->ttl ?? $this->config->defaultTtl, - $tags - ); + try { + $handler->save( + $missedPool, + $cacheKey, + $data, + $attribute->ttl ?? $this->config->defaultTtl, + $tags + ); + } catch (\Throwable $e) { + $this->logger->error($e->getMessage(), ['exception' => $e]); + } } return new Response($data, true); diff --git a/src/Interceptor/Impl/LegacyCacheInterceptor.php b/src/Interceptor/Impl/LegacyCacheInterceptor.php index 5227ef4..0ddf61c 100644 --- a/src/Interceptor/Impl/LegacyCacheInterceptor.php +++ b/src/Interceptor/Impl/LegacyCacheInterceptor.php @@ -12,12 +12,25 @@ use OpenClassrooms\ServiceProxy\Model\Request\Instance; use OpenClassrooms\ServiceProxy\Model\Response\Response; use OpenClassrooms\ServiceProxy\Util\Expression; +use Psr\Log\LoggerInterface; +use Psr\Log\NullLogger; /** * @deprecated use CacheHandler instead */ final class LegacyCacheInterceptor extends AbstractInterceptor implements SuffixInterceptor, PrefixInterceptor { + private LoggerInterface $logger; + + public function __construct( + iterable $handlers = [], + ?LoggerInterface $logger = null + ) { + $this->logger = $logger ?? new NullLogger(); + parent::__construct($handlers); + + } + public function prefix(Instance $instance): Response { $annotation = $instance->getMethod() @@ -37,15 +50,19 @@ public function prefix(Instance $instance): Response $handler = $this->getHandlers(CacheHandler::class, $annotation)[0]; array_unshift($tags, $proxyId); - $data = $handler->fetch('default', implode('|', $tags)); + try { + $data = $handler->fetch('default', implode('|', $tags)); + // this is needed to solve a bug (when the false is stored in the cache) + if (!$data->isHit()) { + return new Response(null, false); + } - // this is needed to solve a bug (when the false is stored in the cache) + return new Response($data->get(), true); + } catch (\Throwable $e) { + $this->logger->error($e->getMessage(), ['exception' => $e]); - if (!$data->isHit()) { return new Response(null, false); } - - return new Response($data->get(), true); } public function suffix(Instance $instance): Response