Skip to content

Commit

Permalink
Merge pull request #2251 from acelaya-forks/feature/inject-repos
Browse files Browse the repository at this point in the history
Feature/inject repos
  • Loading branch information
acelaya authored Nov 9, 2024
2 parents fcd8252 + fca3891 commit d6b103d
Show file tree
Hide file tree
Showing 10 changed files with 75 additions and 83 deletions.
22 changes: 19 additions & 3 deletions module/Core/config/dependencies.config.php
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,10 @@
ShortUrl\Transformer\ShortUrlDataTransformer::class => ConfigAbstractFactory::class,
ShortUrl\Middleware\ExtraPathRedirectMiddleware::class => ConfigAbstractFactory::class,
ShortUrl\Middleware\TrimTrailingSlashMiddleware::class => ConfigAbstractFactory::class,
ShortUrl\Repository\ShortUrlRepository::class => [
EntityRepositoryFactory::class,
ShortUrl\Entity\ShortUrl::class,
],
ShortUrl\Repository\ShortUrlListRepository::class => [
EntityRepositoryFactory::class,
ShortUrl\Entity\ShortUrl::class,
Expand All @@ -67,6 +71,7 @@
Tag\Repository\TagRepository::class => [EntityRepositoryFactory::class, Tag\Entity\Tag::class],

Domain\DomainService::class => ConfigAbstractFactory::class,
Domain\Repository\DomainRepository::class => [EntityRepositoryFactory::class, Domain\Entity\Domain::class],

Visit\VisitsTracker::class => ConfigAbstractFactory::class,
Visit\RequestTracker::class => ConfigAbstractFactory::class,
Expand Down Expand Up @@ -133,6 +138,7 @@
ShortUrl\Resolver\PersistenceShortUrlRelationResolver::class,
ShortUrl\Helper\ShortCodeUniquenessHelper::class,
EventDispatcherInterface::class,
ShortUrl\Repository\ShortUrlRepository::class,
],
Visit\VisitsTracker::class => [
'em',
Expand Down Expand Up @@ -161,13 +167,23 @@
ShortUrl\ShortUrlResolver::class,
ShortUrl\Repository\ExpiredShortUrlsRepository::class,
],
ShortUrl\ShortUrlResolver::class => ['em', Config\Options\UrlShortenerOptions::class],
ShortUrl\ShortUrlResolver::class => [
ShortUrl\Repository\ShortUrlRepository::class,
Config\Options\UrlShortenerOptions::class,
],
ShortUrl\ShortUrlVisitsDeleter::class => [
Visit\Repository\VisitDeleterRepository::class,
ShortUrl\ShortUrlResolver::class,
],
ShortUrl\Helper\ShortCodeUniquenessHelper::class => ['em', Config\Options\UrlShortenerOptions::class],
Domain\DomainService::class => ['em', Config\Options\UrlShortenerOptions::class],
ShortUrl\Helper\ShortCodeUniquenessHelper::class => [
ShortUrl\Repository\ShortUrlRepository::class,
Config\Options\UrlShortenerOptions::class,
],
Domain\DomainService::class => [
'em',
Config\Options\UrlShortenerOptions::class,
Domain\Repository\DomainRepository::class,
],

Util\DoctrineBatchHelper::class => ['em'],
Util\RedirectResponseHelper::class => [Config\Options\RedirectOptions::class],
Expand Down
14 changes: 7 additions & 7 deletions module/Core/src/Domain/DomainService.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,11 @@

readonly class DomainService implements DomainServiceInterface
{
public function __construct(private EntityManagerInterface $em, private UrlShortenerOptions $urlShortenerOptions)
{
public function __construct(
private EntityManagerInterface $em,
private UrlShortenerOptions $urlShortenerOptions,
private DomainRepositoryInterface $repo,
) {
}

/**
Expand Down Expand Up @@ -49,9 +52,7 @@ public function listDomains(ApiKey|null $apiKey = null): array
*/
private function defaultDomainAndRest(ApiKey|null $apiKey): array
{
/** @var DomainRepositoryInterface $repo */
$repo = $this->em->getRepository(Domain::class);
$allDomains = $repo->findDomains($apiKey);
$allDomains = $this->repo->findDomains($apiKey);
$defaultDomain = null;
$restOfDomains = [];

Expand All @@ -71,7 +72,6 @@ private function defaultDomainAndRest(ApiKey|null $apiKey): array
*/
public function getDomain(string $domainId): Domain
{
/** @var Domain|null $domain */
$domain = $this->em->find(Domain::class, $domainId);
if ($domain === null) {
throw DomainNotFoundException::fromId($domainId);
Expand All @@ -82,7 +82,7 @@ public function getDomain(string $domainId): Domain

public function findByAuthority(string $authority, ApiKey|null $apiKey = null): Domain|null
{
return $this->em->getRepository(Domain::class)->findOneByAuthority($authority, $apiKey);
return $this->repo->findOneByAuthority($authority, $apiKey);
}

/**
Expand Down
10 changes: 5 additions & 5 deletions module/Core/src/Importer/ImportedLinksProcessor.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,13 @@
use function Shlinkio\Shlink\Core\normalizeDate;
use function sprintf;

class ImportedLinksProcessor implements ImportedLinksProcessorInterface
readonly class ImportedLinksProcessor implements ImportedLinksProcessorInterface
{
public function __construct(
private readonly EntityManagerInterface $em,
private readonly ShortUrlRelationResolverInterface $relationResolver,
private readonly ShortCodeUniquenessHelperInterface $shortCodeHelper,
private readonly DoctrineBatchHelperInterface $batchHelper,
private EntityManagerInterface $em,
private ShortUrlRelationResolverInterface $relationResolver,
private ShortCodeUniquenessHelperInterface $shortCodeHelper,
private DoctrineBatchHelperInterface $batchHelper,
) {
}

Expand Down
16 changes: 6 additions & 10 deletions module/Core/src/ShortUrl/Helper/ShortCodeUniquenessHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,25 +4,21 @@

namespace Shlinkio\Shlink\Core\ShortUrl\Helper;

use Doctrine\ORM\EntityManagerInterface;
use Shlinkio\Shlink\Core\Config\Options\UrlShortenerOptions;
use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl;
use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlIdentifier;
use Shlinkio\Shlink\Core\ShortUrl\Repository\ShortUrlRepository;
use Shlinkio\Shlink\Core\ShortUrl\Repository\ShortUrlRepositoryInterface;

class ShortCodeUniquenessHelper implements ShortCodeUniquenessHelperInterface
readonly class ShortCodeUniquenessHelper implements ShortCodeUniquenessHelperInterface
{
public function __construct(
private readonly EntityManagerInterface $em,
private readonly UrlShortenerOptions $options,
) {
public function __construct(private ShortUrlRepositoryInterface $repo, private UrlShortenerOptions $options)
{
}

public function ensureShortCodeUniqueness(ShortUrl $shortUrlToBeCreated, bool $hasCustomSlug): bool
{
/** @var ShortUrlRepository $repo */
$repo = $this->em->getRepository(ShortUrl::class);
$otherShortUrlsExist = $repo->shortCodeIsInUseWithLock(ShortUrlIdentifier::fromShortUrl($shortUrlToBeCreated));
$identifier = ShortUrlIdentifier::fromShortUrl($shortUrlToBeCreated);
$otherShortUrlsExist = $this->repo->shortCodeIsInUseWithLock($identifier);

if (! $otherShortUrlsExist) {
return true;
Expand Down
13 changes: 4 additions & 9 deletions module/Core/src/ShortUrl/ShortUrlResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,17 @@

namespace Shlinkio\Shlink\Core\ShortUrl;

use Doctrine\ORM\EntityManagerInterface;
use Shlinkio\Shlink\Core\Config\Options\UrlShortenerOptions;
use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException;
use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl;
use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlIdentifier;
use Shlinkio\Shlink\Core\ShortUrl\Repository\ShortUrlRepository;
use Shlinkio\Shlink\Core\ShortUrl\Repository\ShortUrlRepositoryInterface;
use Shlinkio\Shlink\Rest\Entity\ApiKey;

readonly class ShortUrlResolver implements ShortUrlResolverInterface
{
public function __construct(
private EntityManagerInterface $em,
private ShortUrlRepositoryInterface $repo,
private UrlShortenerOptions $urlShortenerOptions,
) {
}
Expand All @@ -25,9 +24,7 @@ public function __construct(
*/
public function resolveShortUrl(ShortUrlIdentifier $identifier, ApiKey|null $apiKey = null): ShortUrl
{
/** @var ShortUrlRepository $shortUrlRepo */
$shortUrlRepo = $this->em->getRepository(ShortUrl::class);
$shortUrl = $shortUrlRepo->findOne($identifier, $apiKey?->spec());
$shortUrl = $this->repo->findOne($identifier, $apiKey?->spec());
if ($shortUrl === null) {
throw ShortUrlNotFoundException::fromNotFound($identifier);
}
Expand All @@ -53,9 +50,7 @@ public function resolveEnabledShortUrl(ShortUrlIdentifier $identifier): ShortUrl
*/
public function resolvePublicShortUrl(ShortUrlIdentifier $identifier): ShortUrl
{
/** @var ShortUrlRepository $shortUrlRepo */
$shortUrlRepo = $this->em->getRepository(ShortUrl::class);
$shortUrl = $shortUrlRepo->findOneWithDomainFallback($identifier, $this->urlShortenerOptions->mode);
$shortUrl = $this->repo->findOneWithDomainFallback($identifier, $this->urlShortenerOptions->mode);
if ($shortUrl === null) {
throw ShortUrlNotFoundException::fromNotFound($identifier);
}
Expand Down
17 changes: 8 additions & 9 deletions module/Core/src/ShortUrl/UrlShortener.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,15 @@
use Shlinkio\Shlink\Core\ShortUrl\Repository\ShortUrlRepositoryInterface;
use Shlinkio\Shlink\Core\ShortUrl\Resolver\ShortUrlRelationResolverInterface;

class UrlShortener implements UrlShortenerInterface
readonly class UrlShortener implements UrlShortenerInterface
{
public function __construct(
private readonly ShortUrlTitleResolutionHelperInterface $titleResolutionHelper,
private readonly EntityManagerInterface $em,
private readonly ShortUrlRelationResolverInterface $relationResolver,
private readonly ShortCodeUniquenessHelperInterface $shortCodeHelper,
private readonly EventDispatcherInterface $eventDispatcher,
private ShortUrlTitleResolutionHelperInterface $titleResolutionHelper,
private EntityManagerInterface $em,
private ShortUrlRelationResolverInterface $relationResolver,
private ShortCodeUniquenessHelperInterface $shortCodeHelper,
private EventDispatcherInterface $eventDispatcher,
private ShortUrlRepositoryInterface $repo,
) {
}

Expand Down Expand Up @@ -70,9 +71,7 @@ private function findExistingShortUrlIfExists(ShortUrlCreation $creation): Short
return null;
}

/** @var ShortUrlRepositoryInterface $repo */
$repo = $this->em->getRepository(ShortUrl::class);
return $repo->findOneMatching($creation);
return $this->repo->findOneMatching($creation);
}

private function verifyShortCodeUniqueness(ShortUrlCreation $meta, ShortUrl $shortUrlToBeCreated): void
Expand Down
28 changes: 14 additions & 14 deletions module/Core/test/Domain/DomainServiceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
use Shlinkio\Shlink\Core\Domain\DomainService;
use Shlinkio\Shlink\Core\Domain\Entity\Domain;
use Shlinkio\Shlink\Core\Domain\Model\DomainItem;
use Shlinkio\Shlink\Core\Domain\Repository\DomainRepository;
use Shlinkio\Shlink\Core\Domain\Repository\DomainRepositoryInterface;
use Shlinkio\Shlink\Core\Exception\DomainNotFoundException;
use Shlinkio\Shlink\Rest\ApiKey\Model\ApiKeyMeta;
use Shlinkio\Shlink\Rest\ApiKey\Model\RoleDefinition;
Expand All @@ -25,19 +25,23 @@ class DomainServiceTest extends TestCase
{
private DomainService $domainService;
private MockObject & EntityManagerInterface $em;
private MockObject & DomainRepositoryInterface $repo;

protected function setUp(): void
{
$this->em = $this->createMock(EntityManagerInterface::class);
$this->domainService = new DomainService($this->em, new UrlShortenerOptions(defaultDomain: 'default.com'));
$this->repo = $this->createMock(DomainRepositoryInterface::class);
$this->domainService = new DomainService(
$this->em,
new UrlShortenerOptions(defaultDomain: 'default.com'),
$this->repo,
);
}

#[Test, DataProvider('provideExcludedDomains')]
public function listDomainsDelegatesIntoRepository(array $domains, array $expectedResult, ApiKey|null $apiKey): void
{
$repo = $this->createMock(DomainRepository::class);
$repo->expects($this->once())->method('findDomains')->with($apiKey)->willReturn($domains);
$this->em->expects($this->once())->method('getRepository')->with(Domain::class)->willReturn($repo);
$this->repo->expects($this->once())->method('findDomains')->with($apiKey)->willReturn($domains);

$result = $this->domainService->listDomains($apiKey);

Expand Down Expand Up @@ -127,11 +131,9 @@ public function getDomainReturnsEntityWhenFound(): void
public function getOrCreateAlwaysPersistsDomain(Domain|null $foundDomain, ApiKey|null $apiKey): void
{
$authority = 'example.com';
$repo = $this->createMock(DomainRepository::class);
$repo->method('findOneByAuthority')->with($authority, $apiKey)->willReturn(
$this->repo->expects($this->once())->method('findOneByAuthority')->with($authority, $apiKey)->willReturn(
$foundDomain,
);
$this->em->expects($this->once())->method('getRepository')->with(Domain::class)->willReturn($repo);
$this->em->expects($this->once())->method('persist')->with($foundDomain ?? $this->isInstanceOf(Domain::class));
$this->em->expects($this->once())->method('flush');

Expand All @@ -149,9 +151,7 @@ public function getOrCreateThrowsExceptionForApiKeysWithDomainRole(): void
$domain = Domain::withAuthority($authority);
$domain->setId('1');
$apiKey = ApiKey::fromMeta(ApiKeyMeta::withRoles(RoleDefinition::forDomain($domain)));
$repo = $this->createMock(DomainRepository::class);
$repo->method('findOneByAuthority')->with($authority, $apiKey)->willReturn(null);
$this->em->expects($this->once())->method('getRepository')->with(Domain::class)->willReturn($repo);
$this->repo->expects($this->once())->method('findOneByAuthority')->with($authority, $apiKey)->willReturn(null);
$this->em->expects($this->never())->method('persist');
$this->em->expects($this->never())->method('flush');

Expand All @@ -166,9 +166,9 @@ public function configureNotFoundRedirectsConfiguresFetchedDomain(
ApiKey|null $apiKey,
): void {
$authority = 'example.com';
$repo = $this->createMock(DomainRepository::class);
$repo->method('findOneByAuthority')->with($authority, $apiKey)->willReturn($foundDomain);
$this->em->expects($this->once())->method('getRepository')->with(Domain::class)->willReturn($repo);
$this->repo->expects($this->once())->method('findOneByAuthority')->with($authority, $apiKey)->willReturn(
$foundDomain,
);
$this->em->expects($this->once())->method('persist')->with($foundDomain ?? $this->isInstanceOf(Domain::class));
$this->em->expects($this->once())->method('flush');

Expand Down
19 changes: 6 additions & 13 deletions module/Core/test/ShortUrl/Helper/ShortCodeUniquenessHelperTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

namespace ShlinkioTest\Shlink\Core\ShortUrl\Helper;

use Doctrine\ORM\EntityManagerInterface;
use PHPUnit\Framework\Attributes\DataProvider;
use PHPUnit\Framework\Attributes\Test;
use PHPUnit\Framework\MockObject\MockObject;
Expand All @@ -14,18 +13,18 @@
use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl;
use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortCodeUniquenessHelper;
use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlIdentifier;
use Shlinkio\Shlink\Core\ShortUrl\Repository\ShortUrlRepository;
use Shlinkio\Shlink\Core\ShortUrl\Repository\ShortUrlRepositoryInterface;

class ShortCodeUniquenessHelperTest extends TestCase
{
private ShortCodeUniquenessHelper $helper;
private MockObject & EntityManagerInterface $em;
private MockObject & ShortUrlRepositoryInterface $repo;
private MockObject & ShortUrl $shortUrl;

protected function setUp(): void
{
$this->em = $this->createMock(EntityManagerInterface::class);
$this->helper = new ShortCodeUniquenessHelper($this->em, new UrlShortenerOptions());
$this->repo = $this->createMock(ShortUrlRepositoryInterface::class);
$this->helper = new ShortCodeUniquenessHelper($this->repo, new UrlShortenerOptions());

$this->shortUrl = $this->createMock(ShortUrl::class);
$this->shortUrl->method('getShortCode')->willReturn('abc123');
Expand All @@ -36,16 +35,12 @@ public function shortCodeIsRegeneratedIfAlreadyInUse(Domain|null $domain, string
{
$callIndex = 0;
$expectedCalls = 3;
$repo = $this->createMock(ShortUrlRepository::class);
$repo->expects($this->exactly($expectedCalls))->method('shortCodeIsInUseWithLock')->with(
$this->repo->expects($this->exactly($expectedCalls))->method('shortCodeIsInUseWithLock')->with(
ShortUrlIdentifier::fromShortCodeAndDomain('abc123', $expectedAuthority),
)->willReturnCallback(function () use (&$callIndex, $expectedCalls) {
$callIndex++;
return $callIndex < $expectedCalls;
});
$this->em->expects($this->exactly($expectedCalls))->method('getRepository')->with(ShortUrl::class)->willReturn(
$repo,
);
$this->shortUrl->method('getDomain')->willReturn($domain);
$this->shortUrl->expects($this->exactly($expectedCalls - 1))->method('regenerateShortCode')->with();

Expand All @@ -63,11 +58,9 @@ public static function provideDomains(): iterable
#[Test]
public function inUseSlugReturnsError(): void
{
$repo = $this->createMock(ShortUrlRepository::class);
$repo->expects($this->once())->method('shortCodeIsInUseWithLock')->with(
$this->repo->expects($this->once())->method('shortCodeIsInUseWithLock')->with(
ShortUrlIdentifier::fromShortCodeAndDomain('abc123'),
)->willReturn(true);
$this->em->expects($this->once())->method('getRepository')->with(ShortUrl::class)->willReturn($repo);
$this->shortUrl->method('getDomain')->willReturn(null);
$this->shortUrl->expects($this->never())->method('regenerateShortCode');

Expand Down
Loading

0 comments on commit d6b103d

Please sign in to comment.