diff --git a/composer.json b/composer.json index 41885c62..7b72c92a 100644 --- a/composer.json +++ b/composer.json @@ -46,7 +46,7 @@ "predis/predis": "^1.1", "pugx/shortid-php": "^0.7", "ramsey/uuid": "^3.9", - "shlinkio/shlink-common": "dev-main#3777189 as 3.7", + "shlinkio/shlink-common": "dev-main#554e370 as 3.7", "shlinkio/shlink-config": "^1.0", "shlinkio/shlink-event-dispatcher": "^2.1", "shlinkio/shlink-importer": "^2.2", diff --git a/module/Core/src/ShortUrl/Resolver/PersistenceShortUrlRelationResolver.php b/module/Core/src/ShortUrl/Resolver/PersistenceShortUrlRelationResolver.php index 1b004a95..eb7fddad 100644 --- a/module/Core/src/ShortUrl/Resolver/PersistenceShortUrlRelationResolver.php +++ b/module/Core/src/ShortUrl/Resolver/PersistenceShortUrlRelationResolver.php @@ -7,6 +7,7 @@ namespace Shlinkio\Shlink\Core\ShortUrl\Resolver; use Doctrine\Common\Collections; use Doctrine\Common\Collections\Collection; use Doctrine\ORM\EntityManagerInterface; +use Doctrine\ORM\Events; use Shlinkio\Shlink\Core\Entity\Domain; use Shlinkio\Shlink\Core\Entity\Tag; @@ -17,9 +18,15 @@ class PersistenceShortUrlRelationResolver implements ShortUrlRelationResolverInt { private EntityManagerInterface $em; + /** @var array */ + private array $memoizedNewDomains = []; + /** @var array */ + private array $memoizedNewTags = []; + public function __construct(EntityManagerInterface $em) { $this->em = $em; + $this->em->getEventManager()->addEventListener(Events::postFlush, $this); } public function resolveDomain(?string $domain): ?Domain @@ -30,7 +37,14 @@ class PersistenceShortUrlRelationResolver implements ShortUrlRelationResolverInt /** @var Domain|null $existingDomain */ $existingDomain = $this->em->getRepository(Domain::class)->findOneBy(['authority' => $domain]); - return $existingDomain ?? new Domain($domain); + + // Memoize only new domains, and let doctrine handle objects hydrated from persistence + return $existingDomain ?? $this->memoizeNewDomain($domain); + } + + private function memoizeNewDomain(string $domain): Domain + { + return $this->memoizedNewDomains[$domain] = $this->memoizedNewDomains[$domain] ?? new Domain($domain); } /** @@ -47,10 +61,22 @@ class PersistenceShortUrlRelationResolver implements ShortUrlRelationResolverInt $repo = $this->em->getRepository(Tag::class); return new Collections\ArrayCollection(map($tags, function (string $tagName) use ($repo): Tag { - $tag = $repo->findOneBy(['name' => $tagName]) ?? new Tag($tagName); + // Memoize only new tags, and let doctrine handle objects hydrated from persistence + $tag = $repo->findOneBy(['name' => $tagName]) ?? $this->memoizeNewTag($tagName); $this->em->persist($tag); return $tag; })); } + + private function memoizeNewTag(string $tagName): Tag + { + return $this->memoizedNewTags[$tagName] = $this->memoizedNewTags[$tagName] ?? new Tag($tagName); + } + + public function postFlush(): void + { + $this->memoizedNewDomains = []; + $this->memoizedNewTags = []; + } } diff --git a/module/Core/test/ShortUrl/Resolver/PersistenceShortUrlRelationResolverTest.php b/module/Core/test/ShortUrl/Resolver/PersistenceShortUrlRelationResolverTest.php index 187ccbe6..8660099c 100644 --- a/module/Core/test/ShortUrl/Resolver/PersistenceShortUrlRelationResolverTest.php +++ b/module/Core/test/ShortUrl/Resolver/PersistenceShortUrlRelationResolverTest.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace ShlinkioTest\Shlink\Core\ShortUrl\Resolver; +use Doctrine\Common\EventManager; use Doctrine\ORM\EntityManagerInterface; use Doctrine\Persistence\ObjectRepository; use PHPUnit\Framework\TestCase; @@ -14,6 +15,7 @@ use Shlinkio\Shlink\Core\Entity\Domain; use Shlinkio\Shlink\Core\Entity\Tag; use Shlinkio\Shlink\Core\Repository\TagRepositoryInterface; use Shlinkio\Shlink\Core\ShortUrl\Resolver\PersistenceShortUrlRelationResolver; + use function count; class PersistenceShortUrlRelationResolverTest extends TestCase @@ -26,6 +28,8 @@ class PersistenceShortUrlRelationResolverTest extends TestCase public function setUp(): void { $this->em = $this->prophesize(EntityManagerInterface::class); + $this->em->getEventManager()->willReturn(new EventManager()); + $this->resolver = new PersistenceShortUrlRelationResolver($this->em->reveal()); } @@ -113,4 +117,45 @@ class PersistenceShortUrlRelationResolverTest extends TestCase $getRepo->shouldNotHaveBeenCalled(); $persist->shouldNotHaveBeenCalled(); } + + /** @test */ + public function newDomainsAreMemoizedUntilStateIsCleared(): void + { + $repo = $this->prophesize(ObjectRepository::class); + $repo->findOneBy(Argument::type('array'))->willReturn(null); + $this->em->getRepository(Domain::class)->willReturn($repo->reveal()); + + $authority = 'foo.com'; + $domain1 = $this->resolver->resolveDomain($authority); + $domain2 = $this->resolver->resolveDomain($authority); + + self::assertSame($domain1, $domain2); + + $this->resolver->postFlush(); + $domain3 = $this->resolver->resolveDomain($authority); + + self::assertNotSame($domain1, $domain3); + } + + /** @test */ + public function newTagsAreMemoizedUntilStateIsCleared(): void + { + $tagRepo = $this->prophesize(TagRepositoryInterface::class); + $tagRepo->findOneBy(Argument::type('array'))->willReturn(null); + $this->em->getRepository(Tag::class)->willReturn($tagRepo->reveal()); + $this->em->persist(Argument::type(Tag::class))->will(function (): void { + }); + + $tags = ['foo', 'bar']; + [$foo1, $bar1] = $this->resolver->resolveTags($tags); + [$foo2, $bar2] = $this->resolver->resolveTags($tags); + + self::assertSame($foo1, $foo2); + self::assertSame($bar1, $bar2); + + $this->resolver->postFlush(); + [$foo3, $bar3] = $this->resolver->resolveTags($tags); + self::assertNotSame($foo1, $foo3); + self::assertNotSame($bar1, $bar3); + } }