diff --git a/src/User/Keycloak/CachedUserIdentityResolver.php b/src/User/Keycloak/CachedUserIdentityResolver.php index c89b0797b8..26c358fcd0 100644 --- a/src/User/Keycloak/CachedUserIdentityResolver.php +++ b/src/User/Keycloak/CachedUserIdentityResolver.php @@ -7,6 +7,7 @@ use CultuurNet\UDB3\Model\ValueObject\Web\EmailAddress; use CultuurNet\UDB3\User\UserIdentityDetails; use CultuurNet\UDB3\User\UserIdentityResolver; +use InvalidArgumentException; use Symfony\Contracts\Cache\CacheInterface; final class CachedUserIdentityResolver implements UserIdentityResolver @@ -25,37 +26,28 @@ public function __construct( public function getUserById(string $userId): ?UserIdentityDetails { - return $this->deserializeUserIdentityDetails( - $this->cache->get( - $this->createCacheKey($userId, 'user_id'), - function () use ($userId) { - return $this->getUserIdentityDetailsAsArray($this->userIdentityResolver->getUserById($userId)); - } - ) + return $this->getUserFromCache( + $userId, + 'user_id', + fn () => $this->getUserIdentityDetailsAsArray($this->userIdentityResolver->getUserById($userId)) ); } public function getUserByEmail(EmailAddress $email): ?UserIdentityDetails { - return $this->deserializeUserIdentityDetails( - $this->cache->get( - $this->createCacheKey($email->toString(), 'email'), - function () use ($email) { - return $this->getUserIdentityDetailsAsArray($this->userIdentityResolver->getUserByEmail($email)); - } - ) + return $this->getUserFromCache( + $email->toString(), + 'email', + fn () => $this->getUserIdentityDetailsAsArray($this->userIdentityResolver->getUserByEmail($email)) ); } public function getUserByNick(string $nick): ?UserIdentityDetails { - return $this->deserializeUserIdentityDetails( - $this->cache->get( - $this->createCacheKey($nick, 'nick'), - function () use ($nick) { - return $this->getUserIdentityDetailsAsArray($this->userIdentityResolver->getUserByNick($nick)); - } - ) + return $this->getUserFromCache( + $nick, + 'nick', + fn () => $this->getUserIdentityDetailsAsArray($this->userIdentityResolver->getUserByNick($nick)) ); } @@ -64,12 +56,26 @@ private function createCacheKey(string $value, string $property): string return preg_replace('/[{}()\/\\\\@:]/', '_', $value . '_' . $property); } - private function getUserIdentityDetailsAsArray(?UserIdentityDetails $userIdentityDetails): ?array + private function getUserFromCache(string $key, string $type, callable $callback): ?UserIdentityDetails { - if ($userIdentityDetails !== null) { - return $userIdentityDetails->jsonSerialize(); + try { + return $this->deserializeUserIdentityDetails( + $this->cache->get( + $this->createCacheKey($key, $type), + fn () => $callback() + ) + ); + } catch (InvalidArgumentException $exception) { + return null; } - return null; + } + + private function getUserIdentityDetailsAsArray(?UserIdentityDetails $userIdentityDetails): array + { + if ($userIdentityDetails === null) { + throw new InvalidArgumentException(); + } + return $userIdentityDetails->jsonSerialize(); } private function deserializeUserIdentityDetails(?array $cachedUserIdentityDetails): ?UserIdentityDetails diff --git a/tests/User/Keycloak/CachedUserIdentityResolverTest.php b/tests/User/Keycloak/CachedUserIdentityResolverTest.php index c87d22e93d..ebf4025530 100644 --- a/tests/User/Keycloak/CachedUserIdentityResolverTest.php +++ b/tests/User/Keycloak/CachedUserIdentityResolverTest.php @@ -27,7 +27,7 @@ final class CachedUserIdentityResolverTest extends TestCase protected function setUp(): void { $this->fallbackUserIdentityResolver = $this->createMock(UserIdentityResolver::class); - $cache = new ArrayAdapter(); + $cache = new ArrayAdapter(); $this->cachedUserIdentityResolver = new CachedUserIdentityResolver( $this->fallbackUserIdentityResolver, @@ -98,6 +98,20 @@ public function it_can_get_a_cached_user_by_id(): void ); } + /** + * @test + */ + public function it_does_not_cache_when_id_is_null(): void + { + $this->fallbackUserIdentityResolver->expects($this->exactly(2)) + ->method('getUserById') + ->with('null') + ->willReturn(null); + + $this->cachedUserIdentityResolver->getUserById('null'); + $this->cachedUserIdentityResolver->getUserById('null'); + } + /** * @test */ @@ -128,6 +142,21 @@ public function it_can_get_a_cached_user_by_email(): void ); } + /** + * @test + */ + public function it_does_not_cache_when_email_is_null(): void + { + $nonExistingUser = new EmailAddress('null@null.com'); + $this->fallbackUserIdentityResolver->expects($this->exactly(2)) + ->method('getUserByEmail') + ->with($nonExistingUser) + ->willReturn(null); + + $this->cachedUserIdentityResolver->getUserByEmail($nonExistingUser); + $this->cachedUserIdentityResolver->getUserByEmail($nonExistingUser); + } + /** * @test */ @@ -157,4 +186,18 @@ public function it_can_get_a_cached_user_by_nick(): void $this->cachedUserIdentityResolver->getUserByNick('Jane Doe') ); } + + /** + * @test + */ + public function it_does_not_cache_when_nick_is_null(): void + { + $this->fallbackUserIdentityResolver->expects($this->exactly(2)) + ->method('getUserByNick') + ->with('null') + ->willReturn(null); + + $this->cachedUserIdentityResolver->getUserByNick('null'); + $this->cachedUserIdentityResolver->getUserByNick('null'); + } }