mirrors/shlink
1
0
Fork 0
mirror of https://github.com/shlinkio/shlink.git synced 2025-03-14 04:00:57 +03:00
Code Issues Projects Releases Packages Wiki Activity

Added locking to short URL creation when checking if URL exists

Browse source
This commit is contained in:
Alejandro Celaya 2021-05-02 10:33:27 +02:00
parent bf0c679a48
commit 3ff4ac84c4
9 changed files with 68 additions and 27 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

14
module/Core/src/Model/ShortUrlIdentifier.php
View file

@ -5,6 +5,7 @@ declare(strict_types=1);
namespace Shlinkio\Shlink\Core\Model;
use Psr\Http\Message\ServerRequestInterface;
use Shlinkio\Shlink\Core\Entity\ShortUrl;
use Symfony\Component\Console\Input\InputInterface;
final class ShortUrlIdentifier
@ -42,6 +43,19 @@ final class ShortUrlIdentifier
return new self($shortCode, $domain);
}
public static function fromShortUrl(ShortUrl $shortUrl): self
{
$domain = $shortUrl->getDomain();
$domainAuthority = $domain !== null ? $domain->getAuthority() : null;
return new self($shortUrl->getShortCode(), $domainAuthority);
}
public static function fromShortCodeAndDomain(string $shortCode, ?string $domain = null): self
{
return new self($shortCode, $domain);
}
public function shortCode(): string
{
return $this->shortCode;

22
module/Core/src/Repository/ShortUrlRepository.php
View file

@ -4,6 +4,7 @@ declare(strict_types=1);
namespace Shlinkio\Shlink\Core\Repository;
use Doctrine\DBAL\LockMode;
use Doctrine\ORM\Query\Expr\Join;
use Doctrine\ORM\QueryBuilder;
use Happyr\DoctrineSpecification\Repository\EntitySpecificationRepository;
@ -11,6 +12,7 @@ use Happyr\DoctrineSpecification\Specification\Specification;
use Shlinkio\Shlink\Common\Doctrine\Type\ChronosDateTimeType;
use Shlinkio\Shlink\Common\Util\DateRange;
use Shlinkio\Shlink\Core\Entity\ShortUrl;
use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier;
use Shlinkio\Shlink\Core\Model\ShortUrlMeta;
use Shlinkio\Shlink\Core\Model\ShortUrlsOrdering;
use Shlinkio\Shlink\Importer\Model\ImportedShlinkUrl;
@ -180,12 +182,24 @@ class ShortUrlRepository extends EntitySpecificationRepository implements ShortU
return $qb->getQuery()->getOneOrNullResult();
}
public function shortCodeIsInUse(string $slug, ?string $domain = null, ?Specification $spec = null): bool
public function shortCodeIsInUse(ShortUrlIdentifier $identifier, ?Specification $spec = null): bool
{
$qb = $this->createFindOneQueryBuilder($slug, $domain, $spec);
$qb->select('COUNT(DISTINCT s.id)');
return $this->doShortCodeIsInUse($identifier, $spec, null);
}
return ((int) $qb->getQuery()->getSingleScalarResult()) > 0;
public function shortCodeIsInUseWithLock(ShortUrlIdentifier $identifier, ?Specification $spec = null): bool
{
return $this->doShortCodeIsInUse($identifier, $spec, LockMode::PESSIMISTIC_WRITE);
}
private function doShortCodeIsInUse(ShortUrlIdentifier $identifier, ?Specification $spec, ?int $lockMode): bool
{
$qb = $this->createFindOneQueryBuilder($identifier->shortCode(), $identifier->domain(), $spec);
$qb->select('s.id');
$query = $qb->getQuery()->setLockMode($lockMode);
return $query->getOneOrNullResult() !== null;
}
private function createFindOneQueryBuilder(string $slug, ?string $domain, ?Specification $spec): QueryBuilder

5
module/Core/src/Repository/ShortUrlRepositoryInterface.php
View file

@ -9,6 +9,7 @@ use Happyr\DoctrineSpecification\Repository\EntitySpecificationRepositoryInterfa
use Happyr\DoctrineSpecification\Specification\Specification;
use Shlinkio\Shlink\Common\Util\DateRange;
use Shlinkio\Shlink\Core\Entity\ShortUrl;
use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier;
use Shlinkio\Shlink\Core\Model\ShortUrlMeta;
use Shlinkio\Shlink\Core\Model\ShortUrlsOrdering;
use Shlinkio\Shlink\Importer\Model\ImportedShlinkUrl;
@ -36,7 +37,9 @@ interface ShortUrlRepositoryInterface extends ObjectRepository, EntitySpecificat
public function findOne(string $shortCode, ?string $domain = null, ?Specification $spec = null): ?ShortUrl;
public function shortCodeIsInUse(string $slug, ?string $domain, ?Specification $spec = null): bool;
public function shortCodeIsInUse(ShortUrlIdentifier $identifier, ?Specification $spec = null): bool;
public function shortCodeIsInUseWithLock(ShortUrlIdentifier $identifier, ?Specification $spec = null): bool;
public function findOneMatching(ShortUrlMeta $meta): ?ShortUrl;

7
module/Core/src/Service/ShortUrl/ShortCodeHelper.php
View file

@ -6,6 +6,7 @@ namespace Shlinkio\Shlink\Core\Service\ShortUrl;
use Doctrine\ORM\EntityManagerInterface;
use Shlinkio\Shlink\Core\Entity\ShortUrl;
use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier;
use Shlinkio\Shlink\Core\Repository\ShortUrlRepository;
class ShortCodeHelper implements ShortCodeHelperInterface // TODO Rename to ShortCodeUniquenessHelper
@ -19,13 +20,9 @@ class ShortCodeHelper implements ShortCodeHelperInterface // TODO Rename to Shor
public function ensureShortCodeUniqueness(ShortUrl $shortUrlToBeCreated, bool $hasCustomSlug): bool
{
$shortCode = $shortUrlToBeCreated->getShortCode();
$domain = $shortUrlToBeCreated->getDomain();
$domainAuthority = $domain !== null ? $domain->getAuthority() : null;
/** @var ShortUrlRepository $repo */
$repo = $this->em->getRepository(ShortUrl::class);
$otherShortUrlsExist = $repo->shortCodeIsInUse($shortCode, $domainAuthority);
$otherShortUrlsExist = $repo->shortCodeIsInUseWithLock(ShortUrlIdentifier::fromShortUrl($shortUrlToBeCreated));
if (! $otherShortUrlsExist) {
return true;

2
module/Core/src/Visit/VisitsStatsHelper.php
View file

@ -58,7 +58,7 @@ class VisitsStatsHelper implements VisitsStatsHelperInterface
/** @var ShortUrlRepositoryInterface $repo */
$repo = $this->em->getRepository(ShortUrl::class);
if (! $repo->shortCodeIsInUse($identifier->shortCode(), $identifier->domain(), $spec)) {
if (! $repo->shortCodeIsInUse($identifier, $spec)) {
throw ShortUrlNotFoundException::fromNotFound($identifier);
}

19
module/Core/test-db/Repository/ShortUrlRepositoryTest.php
View file

@ -11,6 +11,7 @@ use Shlinkio\Shlink\Common\Util\DateRange;
use Shlinkio\Shlink\Core\Entity\Domain;
use Shlinkio\Shlink\Core\Entity\ShortUrl;
use Shlinkio\Shlink\Core\Entity\Visit;
use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier;
use Shlinkio\Shlink\Core\Model\ShortUrlMeta;
use Shlinkio\Shlink\Core\Model\ShortUrlsOrdering;
use Shlinkio\Shlink\Core\Model\Visitor;
@ -180,12 +181,18 @@ class ShortUrlRepositoryTest extends DatabaseTestCase
$this->getEntityManager()->flush();
self::assertTrue($this->repo->shortCodeIsInUse('my-cool-slug'));
self::assertFalse($this->repo->shortCodeIsInUse('my-cool-slug', 'doma.in'));
self::assertFalse($this->repo->shortCodeIsInUse('slug-not-in-use'));
self::assertFalse($this->repo->shortCodeIsInUse('another-slug'));
self::assertFalse($this->repo->shortCodeIsInUse('another-slug', 'example.com'));
self::assertTrue($this->repo->shortCodeIsInUse('another-slug', 'doma.in'));
self::assertTrue($this->repo->shortCodeIsInUse(ShortUrlIdentifier::fromShortCodeAndDomain('my-cool-slug')));
self::assertFalse($this->repo->shortCodeIsInUse(
ShortUrlIdentifier::fromShortCodeAndDomain('my-cool-slug', 'doma.in'),
));
self::assertFalse($this->repo->shortCodeIsInUse(ShortUrlIdentifier::fromShortCodeAndDomain('slug-not-in-use')));
self::assertFalse($this->repo->shortCodeIsInUse(ShortUrlIdentifier::fromShortCodeAndDomain('another-slug')));
self::assertFalse($this->repo->shortCodeIsInUse(
ShortUrlIdentifier::fromShortCodeAndDomain('another-slug', 'example.com'),
));
self::assertTrue($this->repo->shortCodeIsInUse(
ShortUrlIdentifier::fromShortCodeAndDomain('another-slug', 'doma.in'),
));
}
/** @test */

17
module/Core/test/Service/ShortUrl/ShortCodeHelperTest.php
View file

@ -10,6 +10,7 @@ use Prophecy\PhpUnit\ProphecyTrait;
use Prophecy\Prophecy\ObjectProphecy;
use Shlinkio\Shlink\Core\Entity\Domain;
use Shlinkio\Shlink\Core\Entity\ShortUrl;
use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier;
use Shlinkio\Shlink\Core\Repository\ShortUrlRepository;
use Shlinkio\Shlink\Core\Service\ShortUrl\ShortCodeHelper;
@ -39,12 +40,12 @@ class ShortCodeHelperTest extends TestCase
$callIndex = 0;
$expectedCalls = 3;
$repo = $this->prophesize(ShortUrlRepository::class);
$shortCodeIsInUse = $repo->shortCodeIsInUse('abc123', $expectedAuthority)->will(
function () use (&$callIndex, $expectedCalls) {
$callIndex++;
return $callIndex < $expectedCalls;
},
);
$shortCodeIsInUse = $repo->shortCodeIsInUseWithLock(
ShortUrlIdentifier::fromShortCodeAndDomain('abc123', $expectedAuthority),
)->will(function () use (&$callIndex, $expectedCalls) {
$callIndex++;
return $callIndex < $expectedCalls;
});
$getRepo = $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal());
$this->shortUrl->getDomain()->willReturn($domain);
@ -66,7 +67,9 @@ class ShortCodeHelperTest extends TestCase
public function inUseSlugReturnsError(): void
{
$repo = $this->prophesize(ShortUrlRepository::class);
$shortCodeIsInUse = $repo->shortCodeIsInUse('abc123', null)->willReturn(true);
$shortCodeIsInUse = $repo->shortCodeIsInUseWithLock(
ShortUrlIdentifier::fromShortCodeAndDomain('abc123'),
)->willReturn(true);
$getRepo = $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal());
$this->shortUrl->getDomain()->willReturn(null);

1
module/Core/test/Service/UrlShortenerTest.php
View file

@ -46,7 +46,6 @@ class UrlShortenerTest extends TestCase
return $callback();
});
$repo = $this->prophesize(ShortUrlRepository::class);
$repo->shortCodeIsInUse(Argument::cetera())->willReturn(false);
$this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal());
$this->shortCodeHelper = $this->prophesize(ShortCodeHelperInterface::class);

8
module/Core/test/Visit/VisitsStatsHelperTest.php
View file

@ -81,7 +81,9 @@ class VisitsStatsHelperTest extends TestCase
$shortCode = '123ABC';
$spec = $apiKey === null ? null : $apiKey->spec();
$repo = $this->prophesize(ShortUrlRepositoryInterface::class);
$count = $repo->shortCodeIsInUse($shortCode, null, $spec)->willReturn(true);
$count = $repo->shortCodeIsInUse(ShortUrlIdentifier::fromShortCodeAndDomain($shortCode), $spec)->willReturn(
true,
);
$this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal())->shouldBeCalledOnce();
$list = map(range(0, 1), fn () => Visit::forValidShortUrl(ShortUrl::createEmpty(), Visitor::emptyInstance()));
@ -101,7 +103,9 @@ class VisitsStatsHelperTest extends TestCase
{
$shortCode = '123ABC';
$repo = $this->prophesize(ShortUrlRepositoryInterface::class);
$count = $repo->shortCodeIsInUse($shortCode, null, null)->willReturn(false);
$count = $repo->shortCodeIsInUse(ShortUrlIdentifier::fromShortCodeAndDomain($shortCode), null)->willReturn(
false,
);
$this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal())->shouldBeCalledOnce();
$this->expectException(ShortUrlNotFoundException::class);