diff --git a/module/Core/src/Domain/Repository/DomainRepository.php b/module/Core/src/Domain/Repository/DomainRepository.php index 2e4f3bb2..99a7a927 100644 --- a/module/Core/src/Domain/Repository/DomainRepository.php +++ b/module/Core/src/Domain/Repository/DomainRepository.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Domain\Repository; +use Doctrine\DBAL\LockMode; use Doctrine\ORM\Query\Expr\Join; use Happyr\DoctrineSpecification\Repository\EntitySpecificationRepository; use Shlinkio\Shlink\Core\Entity\Domain; @@ -32,4 +33,16 @@ class DomainRepository extends EntitySpecificationRepository implements DomainRe return $qb->getQuery()->getResult(); } + + public function findOneByAuthorityWithLock(string $authority): ?Domain + { + $qb = $this->createQueryBuilder('d'); + $qb->where($qb->expr()->eq('d.authority', ':authority')) + ->setParameter('authority', $authority) + ->setMaxResults(1); + + $query = $qb->getQuery()->setLockMode(LockMode::PESSIMISTIC_WRITE); + + return $query->getOneOrNullResult(); + } } diff --git a/module/Core/src/Domain/Repository/DomainRepositoryInterface.php b/module/Core/src/Domain/Repository/DomainRepositoryInterface.php index 1d201520..5dc2f873 100644 --- a/module/Core/src/Domain/Repository/DomainRepositoryInterface.php +++ b/module/Core/src/Domain/Repository/DomainRepositoryInterface.php @@ -15,4 +15,6 @@ interface DomainRepositoryInterface extends ObjectRepository, EntitySpecificatio * @return Domain[] */ public function findDomainsWithout(?string $excludedAuthority, ?ApiKey $apiKey = null): array; + + public function findOneByAuthorityWithLock(string $authority): ?Domain; } diff --git a/module/Core/src/Repository/TagRepository.php b/module/Core/src/Repository/TagRepository.php index d21122d0..38aae1dc 100644 --- a/module/Core/src/Repository/TagRepository.php +++ b/module/Core/src/Repository/TagRepository.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Repository; +use Doctrine\DBAL\LockMode; use Happyr\DoctrineSpecification\Repository\EntitySpecificationRepository; use Happyr\DoctrineSpecification\Spec; use Shlinkio\Shlink\Core\Entity\Tag; @@ -62,4 +63,16 @@ class TagRepository extends EntitySpecificationRepository implements TagReposito return $result > 0; } + + public function findOneByNameWithLock(string $name): ?Tag + { + $qb = $this->createQueryBuilder('t'); + $qb->where($qb->expr()->eq('t.name', ':name')) + ->setParameter('name', $name) + ->setMaxResults(1); + + $query = $qb->getQuery()->setLockMode(LockMode::PESSIMISTIC_WRITE); + + return $query->getOneOrNullResult(); + } } diff --git a/module/Core/src/Repository/TagRepositoryInterface.php b/module/Core/src/Repository/TagRepositoryInterface.php index 924706ff..b340c1fc 100644 --- a/module/Core/src/Repository/TagRepositoryInterface.php +++ b/module/Core/src/Repository/TagRepositoryInterface.php @@ -6,6 +6,7 @@ namespace Shlinkio\Shlink\Core\Repository; use Doctrine\Persistence\ObjectRepository; use Happyr\DoctrineSpecification\Repository\EntitySpecificationRepositoryInterface; +use Shlinkio\Shlink\Core\Entity\Tag; use Shlinkio\Shlink\Core\Tag\Model\TagInfo; use Shlinkio\Shlink\Rest\Entity\ApiKey; @@ -19,4 +20,6 @@ interface TagRepositoryInterface extends ObjectRepository, EntitySpecificationRe public function findTagsWithInfo(?ApiKey $apiKey = null): array; public function tagExists(string $tag, ?ApiKey $apiKey = null): bool; + + public function findOneByNameWithLock(string $name): ?Tag; } diff --git a/module/Core/src/ShortUrl/Resolver/PersistenceShortUrlRelationResolver.php b/module/Core/src/ShortUrl/Resolver/PersistenceShortUrlRelationResolver.php index eb7fddad..b7e27a82 100644 --- a/module/Core/src/ShortUrl/Resolver/PersistenceShortUrlRelationResolver.php +++ b/module/Core/src/ShortUrl/Resolver/PersistenceShortUrlRelationResolver.php @@ -8,8 +8,10 @@ use Doctrine\Common\Collections; use Doctrine\Common\Collections\Collection; use Doctrine\ORM\EntityManagerInterface; use Doctrine\ORM\Events; +use Shlinkio\Shlink\Core\Domain\Repository\DomainRepositoryInterface; use Shlinkio\Shlink\Core\Entity\Domain; use Shlinkio\Shlink\Core\Entity\Tag; +use Shlinkio\Shlink\Core\Repository\TagRepositoryInterface; use function Functional\map; use function Functional\unique; @@ -35,8 +37,9 @@ class PersistenceShortUrlRelationResolver implements ShortUrlRelationResolverInt return null; } - /** @var Domain|null $existingDomain */ - $existingDomain = $this->em->getRepository(Domain::class)->findOneBy(['authority' => $domain]); + /** @var DomainRepositoryInterface $repo */ + $repo = $this->em->getRepository(Domain::class); + $existingDomain = $repo->findOneByAuthorityWithLock($domain); // Memoize only new domains, and let doctrine handle objects hydrated from persistence return $existingDomain ?? $this->memoizeNewDomain($domain); @@ -58,11 +61,12 @@ class PersistenceShortUrlRelationResolver implements ShortUrlRelationResolverInt } $tags = unique($tags); + /** @var TagRepositoryInterface $repo */ $repo = $this->em->getRepository(Tag::class); return new Collections\ArrayCollection(map($tags, function (string $tagName) use ($repo): Tag { // Memoize only new tags, and let doctrine handle objects hydrated from persistence - $tag = $repo->findOneBy(['name' => $tagName]) ?? $this->memoizeNewTag($tagName); + $tag = $repo->findOneByNameWithLock($tagName) ?? $this->memoizeNewTag($tagName); $this->em->persist($tag); return $tag; diff --git a/module/Core/test/ShortUrl/Resolver/PersistenceShortUrlRelationResolverTest.php b/module/Core/test/ShortUrl/Resolver/PersistenceShortUrlRelationResolverTest.php index 8660099c..8698bec0 100644 --- a/module/Core/test/ShortUrl/Resolver/PersistenceShortUrlRelationResolverTest.php +++ b/module/Core/test/ShortUrl/Resolver/PersistenceShortUrlRelationResolverTest.php @@ -6,11 +6,11 @@ namespace ShlinkioTest\Shlink\Core\ShortUrl\Resolver; use Doctrine\Common\EventManager; use Doctrine\ORM\EntityManagerInterface; -use Doctrine\Persistence\ObjectRepository; use PHPUnit\Framework\TestCase; use Prophecy\Argument; use Prophecy\PhpUnit\ProphecyTrait; use Prophecy\Prophecy\ObjectProphecy; +use Shlinkio\Shlink\Core\Domain\Repository\DomainRepositoryInterface; use Shlinkio\Shlink\Core\Entity\Domain; use Shlinkio\Shlink\Core\Entity\Tag; use Shlinkio\Shlink\Core\Repository\TagRepositoryInterface; @@ -48,8 +48,8 @@ class PersistenceShortUrlRelationResolverTest extends TestCase */ public function findsOrCreatesDomainWhenValueIsProvided(?Domain $foundDomain, string $authority): void { - $repo = $this->prophesize(ObjectRepository::class); - $findDomain = $repo->findOneBy(['authority' => $authority])->willReturn($foundDomain); + $repo = $this->prophesize(DomainRepositoryInterface::class); + $findDomain = $repo->findOneByAuthorityWithLock($authority)->willReturn($foundDomain); $getRepository = $this->em->getRepository(Domain::class)->willReturn($repo->reveal()); $result = $this->resolver->resolveDomain($authority); @@ -80,7 +80,7 @@ class PersistenceShortUrlRelationResolverTest extends TestCase $expectedPersistedTags = count($expectedTags); $tagRepo = $this->prophesize(TagRepositoryInterface::class); - $findTag = $tagRepo->findOneBy(Argument::type('array'))->will(function (array $args): ?Tag { + $findTag = $tagRepo->findOneByNameWithLock(Argument::type('string'))->will(function (array $args): ?Tag { ['name' => $name] = $args[0]; return $name === 'foo' ? new Tag($name) : null; }); @@ -106,7 +106,7 @@ class PersistenceShortUrlRelationResolverTest extends TestCase public function returnsEmptyCollectionWhenProvidingEmptyListOfTags(): void { $tagRepo = $this->prophesize(TagRepositoryInterface::class); - $findTag = $tagRepo->findOneBy(Argument::type('array'))->willReturn(null); + $findTag = $tagRepo->findOneByNameWithLock(Argument::type('string'))->willReturn(null); $getRepo = $this->em->getRepository(Tag::class)->willReturn($tagRepo->reveal()); $persist = $this->em->persist(Argument::type(Tag::class)); @@ -121,8 +121,8 @@ class PersistenceShortUrlRelationResolverTest extends TestCase /** @test */ public function newDomainsAreMemoizedUntilStateIsCleared(): void { - $repo = $this->prophesize(ObjectRepository::class); - $repo->findOneBy(Argument::type('array'))->willReturn(null); + $repo = $this->prophesize(DomainRepositoryInterface::class); + $repo->findOneByAuthorityWithLock(Argument::type('string'))->willReturn(null); $this->em->getRepository(Domain::class)->willReturn($repo->reveal()); $authority = 'foo.com'; @@ -141,7 +141,7 @@ class PersistenceShortUrlRelationResolverTest extends TestCase public function newTagsAreMemoizedUntilStateIsCleared(): void { $tagRepo = $this->prophesize(TagRepositoryInterface::class); - $tagRepo->findOneBy(Argument::type('array'))->willReturn(null); + $tagRepo->findOneByNameWithLock(Argument::type('string'))->willReturn(null); $this->em->getRepository(Tag::class)->willReturn($tagRepo->reveal()); $this->em->persist(Argument::type(Tag::class))->will(function (): void { });