mirror of
https://github.com/shlinkio/shlink.git
synced 2025-02-09 23:15:41 -06:00
Added locks to tag and domain creation during short URL creation
This commit is contained in:
parent
3ff4ac84c4
commit
f82e103bc5
@ -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();
|
||||
}
|
||||
}
|
||||
|
@ -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;
|
||||
}
|
||||
|
@ -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();
|
||||
}
|
||||
}
|
||||
|
@ -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;
|
||||
}
|
||||
|
@ -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;
|
||||
|
@ -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 {
|
||||
});
|
||||
|
Loading…
Reference in New Issue
Block a user