From 174fb0b748366d048acb74719eac3d2cb78f627a Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 6 Jun 2026 12:38:06 +0200 Subject: [PATCH 1/2] Define asymetric visibility in some ShortUrl props --- module/Core/src/ShortUrl/Entity/ShortUrl.php | 32 ++++++++++++-------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/module/Core/src/ShortUrl/Entity/ShortUrl.php b/module/Core/src/ShortUrl/Entity/ShortUrl.php index 7dbde838..53d01824 100644 --- a/module/Core/src/ShortUrl/Entity/ShortUrl.php +++ b/module/Core/src/ShortUrl/Entity/ShortUrl.php @@ -36,28 +36,28 @@ class ShortUrl extends AbstractEntity { /** * @param Collection $tags - * @param Collection & Selectable $visits - * @param Collection & Selectable $visitsCounts + * @param Collection&Selectable $visits + * @param Collection&Selectable $visitsCounts * @param Collection $redirectRules */ private function __construct( - private string $longUrl, - private string $shortCode, - private Chronos $dateCreated = new Chronos(), - private Collection $tags = new ArrayCollection(), + private(set) string $longUrl, + private(set) string $shortCode, + private(set) Chronos $dateCreated = new Chronos(), + private(set) Collection $tags = new ArrayCollection(), private Collection&Selectable $visits = new ArrayCollection(), private Collection&Selectable $visitsCounts = new ArrayCollection(), - private Chronos|null $validSince = null, - private Chronos|null $validUntil = null, - private int|null $maxVisits = null, - private Domain|null $domain = null, + private(set) Chronos|null $validSince = null, + private(set) Chronos|null $validUntil = null, + private(set) int|null $maxVisits = null, + private(set) Domain|null $domain = null, private bool $customSlugWasProvided = false, private int $shortCodeLength = 0, public readonly ApiKey|null $authorApiKey = null, - private string|null $title = null, + private(set) string|null $title = null, private bool $titleWasAutoResolved = false, - private bool $crawlable = false, - private bool $forwardQuery = true, + private(set) bool $crawlable = false, + private(set) bool $forwardQuery = true, private string|null $importSource = null, private string|null $importOriginalShortCode = null, private Collection $redirectRules = new ArrayCollection(), @@ -170,31 +170,37 @@ class ShortUrl extends AbstractEntity } } + /** @deprecated Use property directly */ public function getLongUrl(): string { return $this->longUrl; } + /** @deprecated Use property directly */ public function getShortCode(): string { return $this->shortCode; } + /** @deprecated Use property directly */ public function getDomain(): Domain|null { return $this->domain; } + /** @deprecated Use property directly */ public function forwardQuery(): bool { return $this->forwardQuery; } + /** @deprecated Use property directly */ public function title(): string|null { return $this->title; } + /** @deprecated Use property directly */ public function dateCreated(): Chronos { return $this->dateCreated; From 2446d9b372f3ac55d287a768b416541f8e85585c Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 7 Jun 2026 09:15:45 +0200 Subject: [PATCH 2/2] Remove unnecessary getters from ShortUrl entity --- .../ShortUrl/CreateShortUrlCommand.php | 2 +- .../Command/ShortUrl/ListShortUrlsCommand.php | 2 +- .../Command/ShortUrl/ResolveUrlCommand.php | 2 +- .../ShortUrl/CreateShortUrlCommandTest.php | 2 +- .../ShortUrl/ListShortUrlsCommandTest.php | 4 +- .../PublishingUpdatesGenerator.php | 2 +- module/Core/src/Matomo/MatomoVisitSender.php | 2 +- .../ShortUrlRedirectionResolver.php | 2 +- module/Core/src/ShortUrl/Entity/ShortUrl.php | 38 ----------------- .../Helper/ShortUrlRedirectionBuilder.php | 2 +- .../src/ShortUrl/Model/ShortUrlIdentifier.php | 4 +- .../src/ShortUrl/Model/ShortUrlWithDeps.php | 4 +- module/Core/src/ShortUrl/UrlShortener.php | 6 +-- .../CrawlableShortCodesQueryTest.php | 10 ++--- .../Repository/ShortUrlListRepositoryTest.php | 8 ++-- .../Repository/ShortUrlRepositoryTest.php | 14 +++---- .../Visit/Repository/VisitRepositoryTest.php | 2 +- .../PublishingUpdatesGeneratorTest.php | 8 ++-- .../Importer/ImportedLinksProcessorTest.php | 2 +- .../test/Matomo/MatomoVisitSenderTest.php | 2 +- .../ShortUrl/DeleteShortUrlServiceTest.php | 2 +- .../test/ShortUrl/Entity/ShortUrlTest.php | 10 ++--- .../Helper/ShortCodeUniquenessHelperTest.php | 42 +++++++++---------- .../Helper/ShortUrlRedirectionBuilderTest.php | 4 +- .../test/ShortUrl/ShortUrlResolverTest.php | 6 +-- .../test/ShortUrl/ShortUrlServiceTest.php | 2 +- .../Core/test/ShortUrl/UrlShortenerTest.php | 2 +- .../RedirectRule/ListRedirectRulesAction.php | 2 +- .../RedirectRule/SetRedirectRulesAction.php | 2 +- .../ListRedirectRulesActionTest.php | 2 +- .../SetRedirectRulesActionTest.php | 2 +- 31 files changed, 76 insertions(+), 118 deletions(-) diff --git a/module/CLI/src/Command/ShortUrl/CreateShortUrlCommand.php b/module/CLI/src/Command/ShortUrl/CreateShortUrlCommand.php index 4e684abc..855371bf 100644 --- a/module/CLI/src/Command/ShortUrl/CreateShortUrlCommand.php +++ b/module/CLI/src/Command/ShortUrl/CreateShortUrlCommand.php @@ -51,7 +51,7 @@ class CreateShortUrlCommand extends Command ); $io->writeln([ - sprintf('Processed long URL: %s', $result->shortUrl->getLongUrl()), + sprintf('Processed long URL: %s', $result->shortUrl->longUrl), sprintf('Generated short URL: %s', $this->stringifier->stringify($result->shortUrl)), ]); return self::SUCCESS; diff --git a/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php b/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php index 32ae7780..36b8520e 100644 --- a/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php +++ b/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php @@ -107,7 +107,7 @@ class ListShortUrlsCommand extends Command } if ($params->showDomain) { $columnsMap['Domain'] = static fn (array $_, ShortUrl $shortUrl): string => ( - $shortUrl->getDomain()->authority ?? Domain::DEFAULT_AUTHORITY + $shortUrl->domain->authority ?? Domain::DEFAULT_AUTHORITY ); } if ($params->showApiKey) { diff --git a/module/CLI/src/Command/ShortUrl/ResolveUrlCommand.php b/module/CLI/src/Command/ShortUrl/ResolveUrlCommand.php index b70ba70e..0fef7691 100644 --- a/module/CLI/src/Command/ShortUrl/ResolveUrlCommand.php +++ b/module/CLI/src/Command/ShortUrl/ResolveUrlCommand.php @@ -39,7 +39,7 @@ class ResolveUrlCommand extends Command try { $url = $this->urlResolver->resolveShortUrl($identifier); - $io->writeln(sprintf('Long URL: %s', $url->getLongUrl())); + $io->writeln(sprintf('Long URL: %s', $url->longUrl)); return self::SUCCESS; } catch (ShortUrlNotFoundException $e) { $io->error($e->getMessage()); diff --git a/module/CLI/test/Command/ShortUrl/CreateShortUrlCommandTest.php b/module/CLI/test/Command/ShortUrl/CreateShortUrlCommandTest.php index f926635f..e3660afc 100644 --- a/module/CLI/test/Command/ShortUrl/CreateShortUrlCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/CreateShortUrlCommandTest.php @@ -82,7 +82,7 @@ class CreateShortUrlCommandTest extends TestCase ); $this->stringifier->method('stringify')->willReturnMap([[$shortUrl, 'stringified_short_url']]); - $this->commandTester->setInputs([$shortUrl->getLongUrl()]); + $this->commandTester->setInputs([$shortUrl->longUrl]); $this->commandTester->execute([]); } diff --git a/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php b/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php index b98daaa3..cf74eb23 100644 --- a/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php @@ -144,8 +144,8 @@ class ListShortUrlsCommandTest extends TestCase apiKey: ApiKey::fromMeta(ApiKeyMeta::fromParams(name: 'my api key')), tags: ['foo', 'bar', 'baz'], )); - $shortCode = $shortUrl->getShortCode(); - $created = $shortUrl->dateCreated()->toAtomString(); + $shortCode = $shortUrl->shortCode; + $created = $shortUrl->dateCreated->toAtomString(); yield 'tags only' => [ ['--show-tags' => true], diff --git a/module/Core/src/EventDispatcher/PublishingUpdatesGenerator.php b/module/Core/src/EventDispatcher/PublishingUpdatesGenerator.php index dfb1d34a..d428a066 100644 --- a/module/Core/src/EventDispatcher/PublishingUpdatesGenerator.php +++ b/module/Core/src/EventDispatcher/PublishingUpdatesGenerator.php @@ -31,7 +31,7 @@ final readonly class PublishingUpdatesGenerator implements PublishingUpdatesGene public function newShortUrlVisitUpdate(Visit $visit): Update { $shortUrl = $visit->shortUrl; - $topic = Topic::newShortUrlVisit($shortUrl?->getShortCode()); + $topic = Topic::newShortUrlVisit($shortUrl?->shortCode); return Update::forTopicAndPayload($topic, [ 'shortUrl' => $this->transformShortUrl($shortUrl), diff --git a/module/Core/src/Matomo/MatomoVisitSender.php b/module/Core/src/Matomo/MatomoVisitSender.php index f77c20c1..e9e5fd9f 100644 --- a/module/Core/src/Matomo/MatomoVisitSender.php +++ b/module/Core/src/Matomo/MatomoVisitSender.php @@ -77,7 +77,7 @@ readonly class MatomoVisitSender implements MatomoVisitSenderInterface } // Send the short URL title or an empty document title to avoid different actions to be created by matomo - $tracker->doTrackPageView($visit->shortUrl?->title() ?? ''); + $tracker->doTrackPageView($visit->shortUrl->title ?? ''); } private function resolveUrlToTrack(Visit $visit): string diff --git a/module/Core/src/RedirectRule/ShortUrlRedirectionResolver.php b/module/Core/src/RedirectRule/ShortUrlRedirectionResolver.php index d6e8b99b..12328d8b 100644 --- a/module/Core/src/RedirectRule/ShortUrlRedirectionResolver.php +++ b/module/Core/src/RedirectRule/ShortUrlRedirectionResolver.php @@ -21,6 +21,6 @@ readonly class ShortUrlRedirectionResolver implements ShortUrlRedirectionResolve } } - return $shortUrl->getLongUrl(); + return $shortUrl->longUrl; } } diff --git a/module/Core/src/ShortUrl/Entity/ShortUrl.php b/module/Core/src/ShortUrl/Entity/ShortUrl.php index 53d01824..f9e16f18 100644 --- a/module/Core/src/ShortUrl/Entity/ShortUrl.php +++ b/module/Core/src/ShortUrl/Entity/ShortUrl.php @@ -91,8 +91,6 @@ class ShortUrl extends AbstractEntity shortCode: sprintf( '%s%s', $creation->pathPrefix ?? '', - // TODO Encapsulate Generating the random short code into ShortUrlCreation, when custom slug is not set, - // then expose it as shortCode or something generic $creation->customSlug ?? generateRandomShortCode($shortCodeLength, $creation->shortUrlMode), ), tags: $relationResolver->resolveTags($creation->tags), @@ -170,42 +168,6 @@ class ShortUrl extends AbstractEntity } } - /** @deprecated Use property directly */ - public function getLongUrl(): string - { - return $this->longUrl; - } - - /** @deprecated Use property directly */ - public function getShortCode(): string - { - return $this->shortCode; - } - - /** @deprecated Use property directly */ - public function getDomain(): Domain|null - { - return $this->domain; - } - - /** @deprecated Use property directly */ - public function forwardQuery(): bool - { - return $this->forwardQuery; - } - - /** @deprecated Use property directly */ - public function title(): string|null - { - return $this->title; - } - - /** @deprecated Use property directly */ - public function dateCreated(): Chronos - { - return $this->dateCreated; - } - public function reachedVisits(int $visitsAmount): bool { return count($this->visits) >= $visitsAmount; diff --git a/module/Core/src/ShortUrl/Helper/ShortUrlRedirectionBuilder.php b/module/Core/src/ShortUrl/Helper/ShortUrlRedirectionBuilder.php index d513012c..4e9e6bab 100644 --- a/module/Core/src/ShortUrl/Helper/ShortUrlRedirectionBuilder.php +++ b/module/Core/src/ShortUrl/Helper/ShortUrlRedirectionBuilder.php @@ -27,7 +27,7 @@ readonly class ShortUrlRedirectionBuilder implements ShortUrlRedirectionBuilderI string|null $extraPath = null, ): string { $uri = new Uri($this->redirectionResolver->resolveLongUrl($shortUrl, $request)); - $shouldForwardQuery = $shortUrl->forwardQuery(); + $shouldForwardQuery = $shortUrl->forwardQuery; $baseQueryString = $uri->getQuery(); $basePath = $uri->getPath(); diff --git a/module/Core/src/ShortUrl/Model/ShortUrlIdentifier.php b/module/Core/src/ShortUrl/Model/ShortUrlIdentifier.php index 50802b17..5ab9b823 100644 --- a/module/Core/src/ShortUrl/Model/ShortUrlIdentifier.php +++ b/module/Core/src/ShortUrl/Model/ShortUrlIdentifier.php @@ -31,8 +31,8 @@ final readonly class ShortUrlIdentifier public static function fromShortUrl(ShortUrl $shortUrl): self { - $domain = $shortUrl->getDomain()?->authority; - return new self($shortUrl->getShortCode(), $domain); + $domain = $shortUrl->domain?->authority; + return new self($shortUrl->shortCode, $domain); } public static function fromShortCodeAndDomain(string $shortCode, string|null $domain = null): self diff --git a/module/Core/src/ShortUrl/Model/ShortUrlWithDeps.php b/module/Core/src/ShortUrl/Model/ShortUrlWithDeps.php index 15fbb22a..45c70970 100644 --- a/module/Core/src/ShortUrl/Model/ShortUrlWithDeps.php +++ b/module/Core/src/ShortUrl/Model/ShortUrlWithDeps.php @@ -32,12 +32,12 @@ final readonly class ShortUrlWithDeps public static function fromShortUrl(ShortUrl $shortUrl): self { - return new self($shortUrl, authority: $shortUrl->getDomain()?->authority); + return new self($shortUrl, authority: $shortUrl->domain?->authority); } public function toIdentifier(): ShortUrlIdentifier { - return ShortUrlIdentifier::fromShortCodeAndDomain($this->shortUrl->getShortCode(), $this->authority); + return ShortUrlIdentifier::fromShortCodeAndDomain($this->shortUrl->shortCode, $this->authority); } public function toArray(): array diff --git a/module/Core/src/ShortUrl/UrlShortener.php b/module/Core/src/ShortUrl/UrlShortener.php index f04e417d..0a52a39a 100644 --- a/module/Core/src/ShortUrl/UrlShortener.php +++ b/module/Core/src/ShortUrl/UrlShortener.php @@ -81,10 +81,8 @@ readonly class UrlShortener implements UrlShortenerInterface ); if (!$couldBeMadeUnique) { - $domain = $shortUrlToBeCreated->getDomain(); - $domainAuthority = $domain?->authority; - - throw NonUniqueSlugException::fromSlug($shortUrlToBeCreated->getShortCode(), $domainAuthority); + $domainAuthority = $shortUrlToBeCreated->domain?->authority; + throw NonUniqueSlugException::fromSlug($shortUrlToBeCreated->shortCode, $domainAuthority); } } } diff --git a/module/Core/test-db/ShortUrl/Repository/CrawlableShortCodesQueryTest.php b/module/Core/test-db/ShortUrl/Repository/CrawlableShortCodesQueryTest.php index f56f774c..dbc58159 100644 --- a/module/Core/test-db/ShortUrl/Repository/CrawlableShortCodesQueryTest.php +++ b/module/Core/test-db/ShortUrl/Repository/CrawlableShortCodesQueryTest.php @@ -41,10 +41,10 @@ class CrawlableShortCodesQueryTest extends DatabaseTestCase $results = [...($this->query)()]; self::assertCount(3, $results); - self::assertContains($shortUrl1->getShortCode(), $results); - self::assertContains($shortUrl3->getShortCode(), $results); - self::assertContains($shortUrl4->getShortCode(), $results); - self::assertNotContains($shortUrl2->getShortCode(), $results); - self::assertNotContains($shortUrl5->getShortCode(), $results); + self::assertContains($shortUrl1->shortCode, $results); + self::assertContains($shortUrl3->shortCode, $results); + self::assertContains($shortUrl4->shortCode, $results); + self::assertNotContains($shortUrl2->shortCode, $results); + self::assertNotContains($shortUrl5->shortCode, $results); } } diff --git a/module/Core/test-db/ShortUrl/Repository/ShortUrlListRepositoryTest.php b/module/Core/test-db/ShortUrl/Repository/ShortUrlListRepositoryTest.php index 3a233974..ada651c6 100644 --- a/module/Core/test-db/ShortUrl/Repository/ShortUrlListRepositoryTest.php +++ b/module/Core/test-db/ShortUrl/Repository/ShortUrlListRepositoryTest.php @@ -166,10 +166,10 @@ class ShortUrlListRepositoryTest extends DatabaseTestCase $result = $this->repo->findList(new ShortUrlsListFiltering(orderBy: Ordering::fromFieldAsc('longUrl'))); self::assertCount(count($urls), $result); - self::assertEquals('https://a', $result[0]->shortUrl->getLongUrl()); - self::assertEquals('https://b', $result[1]->shortUrl->getLongUrl()); - self::assertEquals('https://c', $result[2]->shortUrl->getLongUrl()); - self::assertEquals('https://z', $result[3]->shortUrl->getLongUrl()); + self::assertEquals('https://a', $result[0]->shortUrl->longUrl); + self::assertEquals('https://b', $result[1]->shortUrl->longUrl); + self::assertEquals('https://c', $result[2]->shortUrl->longUrl); + self::assertEquals('https://z', $result[3]->shortUrl->longUrl); } #[Test] diff --git a/module/Core/test-db/ShortUrl/Repository/ShortUrlRepositoryTest.php b/module/Core/test-db/ShortUrl/Repository/ShortUrlRepositoryTest.php index 1b0a226b..3cd94e67 100644 --- a/module/Core/test-db/ShortUrl/Repository/ShortUrlRepositoryTest.php +++ b/module/Core/test-db/ShortUrl/Repository/ShortUrlRepositoryTest.php @@ -54,7 +54,7 @@ class ShortUrlRepositoryTest extends DatabaseTestCase $this->getEntityManager()->flush(); self::assertSame($regularOne, $this->repo->findOneWithDomainFallback( - ShortUrlIdentifier::fromShortCodeAndDomain($regularOne->getShortCode()), + ShortUrlIdentifier::fromShortCodeAndDomain($regularOne->shortCode), ShortUrlMode::STRICT, )); self::assertSame($regularOne, $this->repo->findOneWithDomainFallback( @@ -70,22 +70,22 @@ class ShortUrlRepositoryTest extends DatabaseTestCase ShortUrlMode::STRICT, )); self::assertSame($regularOne, $this->repo->findOneWithDomainFallback( - ShortUrlIdentifier::fromShortCodeAndDomain($withDomainDuplicatingRegular->getShortCode()), + ShortUrlIdentifier::fromShortCodeAndDomain($withDomainDuplicatingRegular->shortCode), ShortUrlMode::STRICT, )); self::assertSame($withDomain, $this->repo->findOneWithDomainFallback( - ShortUrlIdentifier::fromShortCodeAndDomain($withDomain->getShortCode(), 'example.com'), + ShortUrlIdentifier::fromShortCodeAndDomain($withDomain->shortCode, 'example.com'), ShortUrlMode::STRICT, )); self::assertSame( $withDomainDuplicatingRegular, $this->repo->findOneWithDomainFallback( - ShortUrlIdentifier::fromShortCodeAndDomain($withDomainDuplicatingRegular->getShortCode(), 's.test'), + ShortUrlIdentifier::fromShortCodeAndDomain($withDomainDuplicatingRegular->shortCode, 's.test'), ShortUrlMode::STRICT, ), ); self::assertSame($regularOne, $this->repo->findOneWithDomainFallback(ShortUrlIdentifier::fromShortCodeAndDomain( - $withDomainDuplicatingRegular->getShortCode(), + $withDomainDuplicatingRegular->shortCode, 'other-domain.com', ), ShortUrlMode::STRICT)); self::assertNull($this->repo->findOneWithDomainFallback( @@ -93,11 +93,11 @@ class ShortUrlRepositoryTest extends DatabaseTestCase ShortUrlMode::STRICT, )); self::assertNull($this->repo->findOneWithDomainFallback( - ShortUrlIdentifier::fromShortCodeAndDomain($withDomain->getShortCode()), + ShortUrlIdentifier::fromShortCodeAndDomain($withDomain->shortCode), ShortUrlMode::STRICT, )); self::assertNull($this->repo->findOneWithDomainFallback( - ShortUrlIdentifier::fromShortCodeAndDomain($withDomain->getShortCode(), 'other-domain.com'), + ShortUrlIdentifier::fromShortCodeAndDomain($withDomain->shortCode, 'other-domain.com'), ShortUrlMode::STRICT, )); } diff --git a/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php b/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php index be8f30ce..f5c757ba 100644 --- a/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php +++ b/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php @@ -666,7 +666,7 @@ class VisitRepositoryTest extends DatabaseTestCase tags: $tags, ), $this->relationResolver); $domain = is_string($withDomain) ? $withDomain : 'example.com'; - $shortCode = $shortUrl->getShortCode(); + $shortCode = $shortUrl->shortCode; $this->getEntityManager()->persist($shortUrl); $this->createVisitsForShortUrl($shortUrl, $visitsAmount); diff --git a/module/Core/test/EventDispatcher/PublishingUpdatesGeneratorTest.php b/module/Core/test/EventDispatcher/PublishingUpdatesGeneratorTest.php index ee24321d..6bd71162 100644 --- a/module/Core/test/EventDispatcher/PublishingUpdatesGeneratorTest.php +++ b/module/Core/test/EventDispatcher/PublishingUpdatesGeneratorTest.php @@ -57,8 +57,8 @@ class PublishingUpdatesGeneratorTest extends TestCase self::assertEquals( [ 'shortUrl' => [ - 'shortCode' => $shortUrl->getShortCode(), - 'shortUrl' => 'http:/' . $shortUrl->getShortCode(), + 'shortCode' => $shortUrl->shortCode, + 'shortUrl' => 'http:/' . $shortUrl->shortCode, 'longUrl' => 'https://longUrl', 'dateCreated' => $this->now->toAtomString(), 'tags' => [], @@ -141,8 +141,8 @@ class PublishingUpdatesGeneratorTest extends TestCase self::assertEquals( [ 'shortUrl' => [ - 'shortCode' => $shortUrl->getShortCode(), - 'shortUrl' => 'http:/' . $shortUrl->getShortCode(), + 'shortCode' => $shortUrl->shortCode, + 'shortUrl' => 'http:/' . $shortUrl->shortCode, 'longUrl' => 'https://longUrl', 'dateCreated' => $this->now->toAtomString(), 'tags' => [], diff --git a/module/Core/test/Importer/ImportedLinksProcessorTest.php b/module/Core/test/Importer/ImportedLinksProcessorTest.php index 2988910f..c0b4766d 100644 --- a/module/Core/test/Importer/ImportedLinksProcessorTest.php +++ b/module/Core/test/Importer/ImportedLinksProcessorTest.php @@ -157,7 +157,7 @@ class ImportedLinksProcessorTest extends TestCase $this->isInstanceOf(ShortUrl::class), ) ->willReturnCallback(static function (ShortUrl $shortUrl): void { - if ($shortUrl->getShortCode() === 'baz') { + if ($shortUrl->shortCode === 'baz') { throw new RuntimeException('Whatever error'); } }); diff --git a/module/Core/test/Matomo/MatomoVisitSenderTest.php b/module/Core/test/Matomo/MatomoVisitSenderTest.php index 33e077f4..f1cb98f3 100644 --- a/module/Core/test/Matomo/MatomoVisitSenderTest.php +++ b/module/Core/test/Matomo/MatomoVisitSenderTest.php @@ -57,7 +57,7 @@ class MatomoVisitSenderTest extends TestCase $tracker ->expects($this->once()) ->method('doTrackPageView') - ->with($visit->shortUrl?->title() ?? ''); + ->with($visit->shortUrl->title ?? ''); $tracker ->expects($this->once()) ->method('setForceVisitDateTime') diff --git a/module/Core/test/ShortUrl/DeleteShortUrlServiceTest.php b/module/Core/test/ShortUrl/DeleteShortUrlServiceTest.php index 779f2667..88f3ad82 100644 --- a/module/Core/test/ShortUrl/DeleteShortUrlServiceTest.php +++ b/module/Core/test/ShortUrl/DeleteShortUrlServiceTest.php @@ -39,7 +39,7 @@ class DeleteShortUrlServiceTest extends TestCase $shortUrl = ShortUrl::createFake()->setVisits(new ArrayCollection( array_map(static fn () => Visit::forValidShortUrl(ShortUrl::createFake(), Visitor::empty()), range(0, 10)), )); - $this->shortCode = $shortUrl->getShortCode(); + $this->shortCode = $shortUrl->shortCode; $this->em = $this->createMock(EntityManagerInterface::class); diff --git a/module/Core/test/ShortUrl/Entity/ShortUrlTest.php b/module/Core/test/ShortUrl/Entity/ShortUrlTest.php index aa316897..6f959e00 100644 --- a/module/Core/test/ShortUrl/Entity/ShortUrlTest.php +++ b/module/Core/test/ShortUrl/Entity/ShortUrlTest.php @@ -53,10 +53,10 @@ class ShortUrlTest extends TestCase public function regenerateShortCodeProperlyChangesTheValueOnValidShortUrls( ShortUrl $shortUrl, ): void { - $firstShortCode = $shortUrl->getShortCode(); + $firstShortCode = $shortUrl->shortCode; $shortUrl->regenerateShortCode(ShortUrlMode::STRICT); - $secondShortCode = $shortUrl->getShortCode(); + $secondShortCode = $shortUrl->shortCode; self::assertNotEquals($firstShortCode, $secondShortCode); } @@ -80,7 +80,7 @@ class ShortUrlTest extends TestCase new ShortUrlCreation('https://longUrl', shortCodeLength: $length ?? DEFAULT_SHORT_CODES_LENGTH), ); - self::assertEquals($expectedLength, strlen($shortUrl->getShortCode())); + self::assertEquals($expectedLength, strlen($shortUrl->shortCode)); } public static function provideLengths(): iterable @@ -102,7 +102,7 @@ class ShortUrlTest extends TestCase pathPrefix: $pathPrefix, shortCodeLength: 5, )); - $shortCode = $shortUrl->getShortCode(); + $shortCode = $shortUrl->shortCode; if (strlen($expectedPrefix) > 0) { self::assertStringStartsWith($expectedPrefix, $shortCode); @@ -116,7 +116,7 @@ class ShortUrlTest extends TestCase $range = range(1, 1000); // Use a "big" number to reduce false negatives $allFor = static fn (ShortUrlMode $mode): bool => every($range, static function () use ($mode): bool { $shortUrl = ShortUrl::create(new ShortUrlCreation('https://foo', shortUrlMode: $mode)); - $shortCode = $shortUrl->getShortCode(); + $shortCode = $shortUrl->shortCode; return $shortCode === strtolower($shortCode); }); diff --git a/module/Core/test/ShortUrl/Helper/ShortCodeUniquenessHelperTest.php b/module/Core/test/ShortUrl/Helper/ShortCodeUniquenessHelperTest.php index 6aa321c2..eb14d756 100644 --- a/module/Core/test/ShortUrl/Helper/ShortCodeUniquenessHelperTest.php +++ b/module/Core/test/ShortUrl/Helper/ShortCodeUniquenessHelperTest.php @@ -9,9 +9,9 @@ use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\Core\Config\Options\UrlShortenerOptions; -use Shlinkio\Shlink\Core\Domain\Entity\Domain; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortCodeUniquenessHelper; +use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlCreation; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\ShortUrl\Repository\ShortUrlRepositoryInterface; @@ -19,47 +19,42 @@ class ShortCodeUniquenessHelperTest extends TestCase { private ShortCodeUniquenessHelper $helper; private MockObject&ShortUrlRepositoryInterface $repo; - private MockObject&ShortUrl $shortUrl; protected function setUp(): void { $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'); } #[Test, DataProvider('provideDomains')] - public function shortCodeIsRegeneratedIfAlreadyInUse(Domain|null $domain, string|null $expectedAuthority): void + public function shortCodeIsRegeneratedIfAlreadyInUse(string|null $domain, string|null $expectedAuthority): void { + $shortUrl = $this->shortUrl($domain); + $initialShortCode = $shortUrl->shortCode; + $callIndex = 0; $expectedCalls = 3; $this->repo ->expects($this->exactly($expectedCalls)) ->method('shortCodeIsInUseWithLock') - ->with( - ShortUrlIdentifier::fromShortCodeAndDomain('abc123', $expectedAuthority), - ) + ->with($this->callback( + static fn (ShortUrlIdentifier $identifier) => $identifier->domain === $expectedAuthority, + )) ->willReturnCallback(static function () use (&$callIndex, $expectedCalls) { $callIndex++; return $callIndex < $expectedCalls; }); - $this->shortUrl->method('getDomain')->willReturn($domain); - $this->shortUrl - ->expects($this->exactly($expectedCalls - 1)) - ->method('regenerateShortCode') - ->with(); - $result = $this->helper->ensureShortCodeUniqueness($this->shortUrl, false); + $result = $this->helper->ensureShortCodeUniqueness($shortUrl, hasCustomSlug: false); self::assertTrue($result); + self::assertNotEquals($initialShortCode, $shortUrl->shortCode); } public static function provideDomains(): iterable { yield 'no domain' => [null, null]; - yield 'domain' => [Domain::withAuthority($authority = 's.test'), $authority]; + yield 'domain' => [$authority = 's.test', $authority]; } #[Test] @@ -68,15 +63,18 @@ class ShortCodeUniquenessHelperTest extends TestCase $this->repo ->expects($this->once()) ->method('shortCodeIsInUseWithLock') - ->with( - ShortUrlIdentifier::fromShortCodeAndDomain('abc123'), - ) ->willReturn(true); - $this->shortUrl->method('getDomain')->willReturn(null); - $this->shortUrl->expects($this->never())->method('regenerateShortCode'); - $result = $this->helper->ensureShortCodeUniqueness($this->shortUrl, true); + $shortUrl = $this->shortUrl(); + $initialShortCode = $shortUrl->shortCode; + $result = $this->helper->ensureShortCodeUniqueness($shortUrl, hasCustomSlug: true); self::assertFalse($result); + self::assertEquals($initialShortCode, $shortUrl->shortCode); + } + + private function shortUrl(string|null $domain = null): ShortUrl + { + return ShortUrl::create(new ShortUrlCreation('https://example.com', domain: $domain)); } } diff --git a/module/Core/test/ShortUrl/Helper/ShortUrlRedirectionBuilderTest.php b/module/Core/test/ShortUrl/Helper/ShortUrlRedirectionBuilderTest.php index 98eccb80..f5c0254f 100644 --- a/module/Core/test/ShortUrl/Helper/ShortUrlRedirectionBuilderTest.php +++ b/module/Core/test/ShortUrl/Helper/ShortUrlRedirectionBuilderTest.php @@ -49,7 +49,7 @@ class ShortUrlRedirectionBuilderTest extends TestCase $shortUrl, $request, ) - ->willReturn($shortUrl->getLongUrl()); + ->willReturn($shortUrl->longUrl); $result = $this->redirectionBuilder->buildShortUrlRedirect($shortUrl, $request, $extraPath); @@ -143,7 +143,7 @@ class ShortUrlRedirectionBuilderTest extends TestCase $shortUrl, $request, ) - ->willReturn($shortUrl->getLongUrl()); + ->willReturn($shortUrl->longUrl); $result = $this->redirectionBuilder->buildShortUrlRedirect($shortUrl, $request); diff --git a/module/Core/test/ShortUrl/ShortUrlResolverTest.php b/module/Core/test/ShortUrl/ShortUrlResolverTest.php index 85fabc7f..6f31872e 100644 --- a/module/Core/test/ShortUrl/ShortUrlResolverTest.php +++ b/module/Core/test/ShortUrl/ShortUrlResolverTest.php @@ -42,7 +42,7 @@ class ShortUrlResolverTest extends TestCase public function shortCodeIsProperlyParsed(ApiKey|null $apiKey): void { $shortUrl = ShortUrl::withLongUrl('https://expected_url'); - $shortCode = $shortUrl->getShortCode(); + $shortCode = $shortUrl->shortCode; $identifier = ShortUrlIdentifier::fromShortCodeAndDomain($shortCode); $this->repo @@ -75,7 +75,7 @@ class ShortUrlResolverTest extends TestCase public function resolveEnabledShortUrlProperlyParsesShortCode(): void { $shortUrl = ShortUrl::withLongUrl('https://expected_url'); - $shortCode = $shortUrl->getShortCode(); + $shortCode = $shortUrl->shortCode; $this->repo ->expects($this->once()) @@ -119,7 +119,7 @@ class ShortUrlResolverTest extends TestCase #[Test, DataProvider('provideDisabledShortUrls')] public function resolveEnabledShortUrlThrowsExceptionIfUrlIsNotEnabled(ShortUrl $shortUrl): void { - $shortCode = $shortUrl->getShortCode(); + $shortCode = $shortUrl->shortCode; $this->repo ->expects($this->once()) diff --git a/module/Core/test/ShortUrl/ShortUrlServiceTest.php b/module/Core/test/ShortUrl/ShortUrlServiceTest.php index 5759eff5..0e19822f 100644 --- a/module/Core/test/ShortUrl/ShortUrlServiceTest.php +++ b/module/Core/test/ShortUrl/ShortUrlServiceTest.php @@ -82,7 +82,7 @@ class ShortUrlServiceTest extends TestCase $until, ); self::assertEquals($shortUrlEdit->maxVisits, $maxVisits); - self::assertEquals($shortUrlEdit->longUrl ?? $originalLongUrl, $shortUrl->getLongUrl()); + self::assertEquals($shortUrlEdit->longUrl ?? $originalLongUrl, $shortUrl->longUrl); } public static function provideShortUrlEdits(): iterable diff --git a/module/Core/test/ShortUrl/UrlShortenerTest.php b/module/Core/test/ShortUrl/UrlShortenerTest.php index 6f8e2b30..2a7ff1be 100644 --- a/module/Core/test/ShortUrl/UrlShortenerTest.php +++ b/module/Core/test/ShortUrl/UrlShortenerTest.php @@ -74,7 +74,7 @@ class UrlShortenerTest extends TestCase $thereIsError = true; }); - self::assertEquals($longUrl, $result->shortUrl->getLongUrl()); + self::assertEquals($longUrl, $result->shortUrl->longUrl); self::assertEquals($expectDispatchError, $thereIsError); } diff --git a/module/Rest/src/Action/RedirectRule/ListRedirectRulesAction.php b/module/Rest/src/Action/RedirectRule/ListRedirectRulesAction.php index 2a3d882e..3551f7f3 100644 --- a/module/Rest/src/Action/RedirectRule/ListRedirectRulesAction.php +++ b/module/Rest/src/Action/RedirectRule/ListRedirectRulesAction.php @@ -32,7 +32,7 @@ class ListRedirectRulesAction extends AbstractRestAction $rules = $this->ruleService->rulesForShortUrl($shortUrl); return new JsonResponse([ - 'defaultLongUrl' => $shortUrl->getLongUrl(), + 'defaultLongUrl' => $shortUrl->longUrl, 'redirectRules' => $rules, ]); } diff --git a/module/Rest/src/Action/RedirectRule/SetRedirectRulesAction.php b/module/Rest/src/Action/RedirectRule/SetRedirectRulesAction.php index 2dfc26db..798c1445 100644 --- a/module/Rest/src/Action/RedirectRule/SetRedirectRulesAction.php +++ b/module/Rest/src/Action/RedirectRule/SetRedirectRulesAction.php @@ -37,7 +37,7 @@ class SetRedirectRulesAction extends AbstractRestAction $result = $this->ruleService->setRulesForShortUrl($shortUrl, $data); return new JsonResponse([ - 'defaultLongUrl' => $shortUrl->getLongUrl(), + 'defaultLongUrl' => $shortUrl->longUrl, 'redirectRules' => $result, ]); } diff --git a/module/Rest/test/Action/RedirectRule/ListRedirectRulesActionTest.php b/module/Rest/test/Action/RedirectRule/ListRedirectRulesActionTest.php index 55ad5564..aba293c6 100644 --- a/module/Rest/test/Action/RedirectRule/ListRedirectRulesActionTest.php +++ b/module/Rest/test/Action/RedirectRule/ListRedirectRulesActionTest.php @@ -52,7 +52,7 @@ class ListRedirectRulesActionTest extends TestCase self::assertEquals( [ - 'defaultLongUrl' => $shortUrl->getLongUrl(), + 'defaultLongUrl' => $shortUrl->longUrl, 'redirectRules' => $redirectRules, ], $payload, diff --git a/module/Rest/test/Action/RedirectRule/SetRedirectRulesActionTest.php b/module/Rest/test/Action/RedirectRule/SetRedirectRulesActionTest.php index 7db4d7e5..2b27d068 100644 --- a/module/Rest/test/Action/RedirectRule/SetRedirectRulesActionTest.php +++ b/module/Rest/test/Action/RedirectRule/SetRedirectRulesActionTest.php @@ -57,7 +57,7 @@ class SetRedirectRulesActionTest extends TestCase self::assertEquals( [ - 'defaultLongUrl' => $shortUrl->getLongUrl(), + 'defaultLongUrl' => $shortUrl->longUrl, 'redirectRules' => $redirectRules, ], $payload,