From a93edf158ea621627f837b4583e51b9cd45aabd6 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 15 Jan 2023 13:08:21 +0100 Subject: [PATCH] Added logic to persist device long URLs while creating/editing a short URL --- ...o.Shlink.Core.ShortUrl.Entity.ShortUrl.php | 5 + module/Core/src/Domain/DomainService.php | 2 +- module/Core/src/Domain/Entity/Domain.php | 4 +- module/Core/src/Domain/Model/DomainItem.php | 2 +- .../src/ShortUrl/Entity/DeviceLongUrl.php | 7 +- module/Core/src/ShortUrl/Entity/ShortUrl.php | 101 +++++++++++------- .../ShortUrl/Helper/ShortUrlStringifier.php | 4 +- .../src/ShortUrl/Model/ShortUrlEdition.php | 1 + .../src/ShortUrl/Model/ShortUrlIdentifier.php | 2 +- .../SimpleShortUrlRelationResolver.php | 3 +- module/Core/src/ShortUrl/UrlShortener.php | 2 +- module/Core/src/Visit/VisitsTracker.php | 9 +- .../Repository/DomainRepositoryTest.php | 2 +- .../Repository/ShortUrlRepositoryTest.php | 10 +- .../Tag/Repository/TagRepositoryTest.php | 2 +- .../Visit/Repository/VisitRepositoryTest.php | 4 +- ...ersistenceShortUrlRelationResolverTest.php | 2 +- .../SimpleShortUrlRelationResolverTest.php | 2 +- .../test/ShortUrl/ShortUrlServiceTest.php | 63 ++++++----- .../Rest/src/ApiKey/Model/RoleDefinition.php | 2 +- module/Rest/src/Entity/ApiKey.php | 7 +- .../ShortUrl/OverrideDomainMiddleware.php | 4 +- 22 files changed, 142 insertions(+), 98 deletions(-) diff --git a/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.ShortUrl.Entity.ShortUrl.php b/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.ShortUrl.Entity.ShortUrl.php index b67996ae..13aa36f6 100644 --- a/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.ShortUrl.Entity.ShortUrl.php +++ b/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.ShortUrl.Entity.ShortUrl.php @@ -67,6 +67,11 @@ return static function (ClassMetadata $metadata, array $emConfig): void { ->fetchExtraLazy() ->build(); + $builder->createOneToMany('deviceLongUrls', ShortUrl\Entity\DeviceLongUrl::class) + ->mappedBy('shortUrl') + ->cascadePersist() + ->build(); + $builder->createManyToMany('tags', Tag\Entity\Tag::class) ->setJoinTable(determineTableName('short_urls_in_tags', $emConfig)) ->addInverseJoinColumn('tag_id', 'id', true, false, 'CASCADE') diff --git a/module/Core/src/Domain/DomainService.php b/module/Core/src/Domain/DomainService.php index 3ecc9f03..29afa110 100644 --- a/module/Core/src/Domain/DomainService.php +++ b/module/Core/src/Domain/DomainService.php @@ -51,7 +51,7 @@ class DomainService implements DomainServiceInterface $repo = $this->em->getRepository(Domain::class); $groups = group( $repo->findDomains($apiKey), - fn (Domain $domain) => $domain->getAuthority() === $this->defaultDomain ? 'default' : 'domains', + fn (Domain $domain) => $domain->authority() === $this->defaultDomain ? 'default' : 'domains', ); return [first($groups['default'] ?? []), $groups['domains'] ?? []]; diff --git a/module/Core/src/Domain/Entity/Domain.php b/module/Core/src/Domain/Entity/Domain.php index ab33ae17..b9b5c334 100644 --- a/module/Core/src/Domain/Entity/Domain.php +++ b/module/Core/src/Domain/Entity/Domain.php @@ -24,14 +24,14 @@ class Domain extends AbstractEntity implements JsonSerializable, NotFoundRedirec return new self($authority); } - public function getAuthority(): string + public function authority(): string { return $this->authority; } public function jsonSerialize(): string { - return $this->getAuthority(); + return $this->authority; } public function invalidShortUrlRedirect(): ?string diff --git a/module/Core/src/Domain/Model/DomainItem.php b/module/Core/src/Domain/Model/DomainItem.php index 72ea3e1f..2a1c7fcf 100644 --- a/module/Core/src/Domain/Model/DomainItem.php +++ b/module/Core/src/Domain/Model/DomainItem.php @@ -20,7 +20,7 @@ final class DomainItem implements JsonSerializable public static function forNonDefaultDomain(Domain $domain): self { - return new self($domain->getAuthority(), $domain, false); + return new self($domain->authority(), $domain, false); } public static function forDefaultDomain(string $defaultDomain, NotFoundRedirectConfigInterface $config): self diff --git a/module/Core/src/ShortUrl/Entity/DeviceLongUrl.php b/module/Core/src/ShortUrl/Entity/DeviceLongUrl.php index b3db666d..668741e8 100644 --- a/module/Core/src/ShortUrl/Entity/DeviceLongUrl.php +++ b/module/Core/src/ShortUrl/Entity/DeviceLongUrl.php @@ -10,17 +10,16 @@ use Shlinkio\Shlink\Core\ShortUrl\Model\DeviceLongUrlPair; class DeviceLongUrl extends AbstractEntity { - private ShortUrl $shortUrl; // @phpstan-ignore-line - private function __construct( + private readonly ShortUrl $shortUrl, // No need to read this field. It's used by doctrine public readonly DeviceType $deviceType, private string $longUrl, ) { } - public static function fromPair(DeviceLongUrlPair $pair): self + public static function fromShortUrlAndPair(ShortUrl $shortUrl, DeviceLongUrlPair $pair): self { - return new self($pair->deviceType, $pair->longUrl); + return new self($shortUrl, $pair->deviceType, $pair->longUrl); } public function longUrl(): string diff --git a/module/Core/src/ShortUrl/Entity/ShortUrl.php b/module/Core/src/ShortUrl/Entity/ShortUrl.php index 51bd09ea..644c52eb 100644 --- a/module/Core/src/ShortUrl/Entity/ShortUrl.php +++ b/module/Core/src/ShortUrl/Entity/ShortUrl.php @@ -12,6 +12,7 @@ use Doctrine\Common\Collections\Selectable; use Shlinkio\Shlink\Common\Entity\AbstractEntity; use Shlinkio\Shlink\Core\Domain\Entity\Domain; use Shlinkio\Shlink\Core\Exception\ShortCodeCannotBeRegeneratedException; +use Shlinkio\Shlink\Core\ShortUrl\Model\DeviceLongUrlPair; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlCreation; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlEdition; use Shlinkio\Shlink\Core\ShortUrl\Model\Validation\ShortUrlInputFilter; @@ -24,6 +25,7 @@ use Shlinkio\Shlink\Importer\Model\ImportedShlinkUrl; use Shlinkio\Shlink\Rest\Entity\ApiKey; use function count; +use function Functional\map; use function Shlinkio\Shlink\Core\generateRandomShortCode; use function Shlinkio\Shlink\Core\normalizeDate; use function Shlinkio\Shlink\Core\normalizeOptionalDate; @@ -35,6 +37,8 @@ class ShortUrl extends AbstractEntity private Chronos $dateCreated; /** @var Collection */ private Collection $visits; + /** @var Collection */ + private Collection $deviceLongUrls; /** @var Collection */ private Collection $tags; private ?Chronos $validSince = null; @@ -81,6 +85,10 @@ class ShortUrl extends AbstractEntity $instance->longUrl = $creation->getLongUrl(); $instance->dateCreated = Chronos::now(); $instance->visits = new ArrayCollection(); + $instance->deviceLongUrls = new ArrayCollection(map( + $creation->deviceLongUrls, + fn (DeviceLongUrlPair $pair) => DeviceLongUrl::fromShortUrlAndPair($instance, $pair), + )); $instance->tags = $relationResolver->resolveTags($creation->tags); $instance->validSince = $creation->validSince; $instance->validUntil = $creation->validUntil; @@ -126,6 +134,53 @@ class ShortUrl extends AbstractEntity return $instance; } + public function update( + ShortUrlEdition $shortUrlEdit, + ?ShortUrlRelationResolverInterface $relationResolver = null, + ): void { + if ($shortUrlEdit->validSinceWasProvided()) { + $this->validSince = $shortUrlEdit->validSince; + } + if ($shortUrlEdit->validUntilWasProvided()) { + $this->validUntil = $shortUrlEdit->validUntil; + } + if ($shortUrlEdit->maxVisitsWasProvided()) { + $this->maxVisits = $shortUrlEdit->maxVisits; + } + if ($shortUrlEdit->longUrlWasProvided()) { + $this->longUrl = $shortUrlEdit->longUrl ?? $this->longUrl; + } + if ($shortUrlEdit->tagsWereProvided()) { + $relationResolver = $relationResolver ?? new SimpleShortUrlRelationResolver(); + $this->tags = $relationResolver->resolveTags($shortUrlEdit->tags); + } + if ($shortUrlEdit->crawlableWasProvided()) { + $this->crawlable = $shortUrlEdit->crawlable; + } + if ( + $this->title === null + || $shortUrlEdit->titleWasProvided() + || ($this->titleWasAutoResolved && $shortUrlEdit->titleWasAutoResolved()) + ) { + $this->title = $shortUrlEdit->title; + $this->titleWasAutoResolved = $shortUrlEdit->titleWasAutoResolved(); + } + if ($shortUrlEdit->forwardQueryWasProvided()) { + $this->forwardQuery = $shortUrlEdit->forwardQuery; + } + foreach ($shortUrlEdit->deviceLongUrls as $deviceLongUrlPair) { + $deviceLongUrl = $this->deviceLongUrls->findFirst( + fn ($_, DeviceLongUrl $d) => $d->deviceType === $deviceLongUrlPair->deviceType, + ); + + if ($deviceLongUrl !== null) { + $deviceLongUrl->updateLongUrl($deviceLongUrlPair->longUrl); + } else { + $this->deviceLongUrls->add(DeviceLongUrl::fromShortUrlAndPair($this, $deviceLongUrlPair)); + } + } + } + public function getLongUrl(): string { return $this->longUrl; @@ -224,42 +279,6 @@ class ShortUrl extends AbstractEntity return $this->forwardQuery; } - public function update( - ShortUrlEdition $shortUrlEdit, - ?ShortUrlRelationResolverInterface $relationResolver = null, - ): void { - if ($shortUrlEdit->validSinceWasProvided()) { - $this->validSince = $shortUrlEdit->validSince; - } - if ($shortUrlEdit->validUntilWasProvided()) { - $this->validUntil = $shortUrlEdit->validUntil; - } - if ($shortUrlEdit->maxVisitsWasProvided()) { - $this->maxVisits = $shortUrlEdit->maxVisits; - } - if ($shortUrlEdit->longUrlWasProvided()) { - $this->longUrl = $shortUrlEdit->longUrl ?? $this->longUrl; - } - if ($shortUrlEdit->tagsWereProvided()) { - $relationResolver = $relationResolver ?? new SimpleShortUrlRelationResolver(); - $this->tags = $relationResolver->resolveTags($shortUrlEdit->tags); - } - if ($shortUrlEdit->crawlableWasProvided()) { - $this->crawlable = $shortUrlEdit->crawlable; - } - if ( - $this->title === null - || $shortUrlEdit->titleWasProvided() - || ($this->titleWasAutoResolved && $shortUrlEdit->titleWasAutoResolved()) - ) { - $this->title = $shortUrlEdit->title; - $this->titleWasAutoResolved = $shortUrlEdit->titleWasAutoResolved(); - } - if ($shortUrlEdit->forwardQueryWasProvided()) { - $this->forwardQuery = $shortUrlEdit->forwardQuery; - } - } - /** * @throws ShortCodeCannotBeRegeneratedException */ @@ -298,4 +317,14 @@ class ShortUrl extends AbstractEntity return true; } + + public function deviceLongUrls(): array + { + $data = []; + foreach ($this->deviceLongUrls as $deviceUrl) { + $data[$deviceUrl->deviceType->value] = $deviceUrl->longUrl(); + } + + return $data; + } } diff --git a/module/Core/src/ShortUrl/Helper/ShortUrlStringifier.php b/module/Core/src/ShortUrl/Helper/ShortUrlStringifier.php index 719f82b8..795b2490 100644 --- a/module/Core/src/ShortUrl/Helper/ShortUrlStringifier.php +++ b/module/Core/src/ShortUrl/Helper/ShortUrlStringifier.php @@ -11,7 +11,7 @@ use function sprintf; class ShortUrlStringifier implements ShortUrlStringifierInterface { - public function __construct(private array $domainConfig, private string $basePath = '') + public function __construct(private readonly array $domainConfig, private readonly string $basePath = '') { } @@ -28,6 +28,6 @@ class ShortUrlStringifier implements ShortUrlStringifierInterface private function resolveDomain(ShortUrl $shortUrl): string { - return $shortUrl->getDomain()?->getAuthority() ?? $this->domainConfig['hostname'] ?? ''; + return $shortUrl->getDomain()?->authority() ?? $this->domainConfig['hostname'] ?? ''; } } diff --git a/module/Core/src/ShortUrl/Model/ShortUrlEdition.php b/module/Core/src/ShortUrl/Model/ShortUrlEdition.php index fb0f9bb0..25645437 100644 --- a/module/Core/src/ShortUrl/Model/ShortUrlEdition.php +++ b/module/Core/src/ShortUrl/Model/ShortUrlEdition.php @@ -18,6 +18,7 @@ final class ShortUrlEdition implements TitleResolutionModelInterface { /** * @param string[] $tags + * @param DeviceLongUrlPair[] $deviceLongUrls */ private function __construct( private readonly bool $longUrlPropWasProvided = false, diff --git a/module/Core/src/ShortUrl/Model/ShortUrlIdentifier.php b/module/Core/src/ShortUrl/Model/ShortUrlIdentifier.php index d7b49c68..fc930de5 100644 --- a/module/Core/src/ShortUrl/Model/ShortUrlIdentifier.php +++ b/module/Core/src/ShortUrl/Model/ShortUrlIdentifier.php @@ -45,7 +45,7 @@ final class ShortUrlIdentifier public static function fromShortUrl(ShortUrl $shortUrl): self { $domain = $shortUrl->getDomain(); - $domainAuthority = $domain?->getAuthority(); + $domainAuthority = $domain?->authority(); return new self($shortUrl->getShortCode(), $domainAuthority); } diff --git a/module/Core/src/ShortUrl/Resolver/SimpleShortUrlRelationResolver.php b/module/Core/src/ShortUrl/Resolver/SimpleShortUrlRelationResolver.php index 2048aba9..609a300c 100644 --- a/module/Core/src/ShortUrl/Resolver/SimpleShortUrlRelationResolver.php +++ b/module/Core/src/ShortUrl/Resolver/SimpleShortUrlRelationResolver.php @@ -5,7 +5,6 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\ShortUrl\Resolver; use Doctrine\Common\Collections; -use Doctrine\Common\Collections\Collection; use Shlinkio\Shlink\Core\Domain\Entity\Domain; use Shlinkio\Shlink\Core\Tag\Entity\Tag; @@ -20,7 +19,7 @@ class SimpleShortUrlRelationResolver implements ShortUrlRelationResolverInterfac /** * @param string[] $tags - * @return Collection + * @return Collections\Collection */ public function resolveTags(array $tags): Collections\Collection { diff --git a/module/Core/src/ShortUrl/UrlShortener.php b/module/Core/src/ShortUrl/UrlShortener.php index 27e96262..4720809e 100644 --- a/module/Core/src/ShortUrl/UrlShortener.php +++ b/module/Core/src/ShortUrl/UrlShortener.php @@ -76,7 +76,7 @@ class UrlShortener implements UrlShortenerInterface if (! $couldBeMadeUnique) { $domain = $shortUrlToBeCreated->getDomain(); - $domainAuthority = $domain?->getAuthority(); + $domainAuthority = $domain?->authority(); throw NonUniqueSlugException::fromSlug($shortUrlToBeCreated->getShortCode(), $domainAuthority); } diff --git a/module/Core/src/Visit/VisitsTracker.php b/module/Core/src/Visit/VisitsTracker.php index f97fc618..dd5fff91 100644 --- a/module/Core/src/Visit/VisitsTracker.php +++ b/module/Core/src/Visit/VisitsTracker.php @@ -15,9 +15,9 @@ use Shlinkio\Shlink\Core\Visit\Model\Visitor; class VisitsTracker implements VisitsTrackerInterface { public function __construct( - private ORM\EntityManagerInterface $em, - private EventDispatcherInterface $eventDispatcher, - private TrackingOptions $options, + private readonly ORM\EntityManagerInterface $em, + private readonly EventDispatcherInterface $eventDispatcher, + private readonly TrackingOptions $options, ) { } @@ -62,6 +62,9 @@ class VisitsTracker implements VisitsTrackerInterface $this->trackVisit($createVisit, $visitor); } + /** + * @param callable(Visitor $visitor): Visit $createVisit + */ private function trackVisit(callable $createVisit, Visitor $visitor): void { if ($this->options->disableTracking) { diff --git a/module/Core/test-db/Domain/Repository/DomainRepositoryTest.php b/module/Core/test-db/Domain/Repository/DomainRepositoryTest.php index c96d70ff..0db35974 100644 --- a/module/Core/test-db/Domain/Repository/DomainRepositoryTest.php +++ b/module/Core/test-db/Domain/Repository/DomainRepositoryTest.php @@ -131,7 +131,7 @@ class DomainRepositoryTest extends DatabaseTestCase { return ShortUrl::create( ShortUrlCreation::fromRawData( - ['domain' => $domain->getAuthority(), 'apiKey' => $apiKey, 'longUrl' => 'foo'], + ['domain' => $domain->authority(), 'apiKey' => $apiKey, 'longUrl' => 'foo'], ), new class ($domain) implements ShortUrlRelationResolverInterface { public function __construct(private Domain $domain) diff --git a/module/Core/test-db/ShortUrl/Repository/ShortUrlRepositoryTest.php b/module/Core/test-db/ShortUrl/Repository/ShortUrlRepositoryTest.php index ed500349..01c6c326 100644 --- a/module/Core/test-db/ShortUrl/Repository/ShortUrlRepositoryTest.php +++ b/module/Core/test-db/ShortUrl/Repository/ShortUrlRepositoryTest.php @@ -270,7 +270,7 @@ class ShortUrlRepositoryTest extends DatabaseTestCase $shortUrl = ShortUrl::create(ShortUrlCreation::fromRawData([ 'validSince' => $start, 'apiKey' => $apiKey, - 'domain' => $rightDomain->getAuthority(), + 'domain' => $rightDomain->authority(), 'longUrl' => 'foo', 'tags' => ['foo', 'bar'], ]), $this->relationResolver); @@ -313,7 +313,7 @@ class ShortUrlRepositoryTest extends DatabaseTestCase $shortUrl, $this->repo->findOneMatching(ShortUrlCreation::fromRawData([ 'validSince' => $start, - 'domain' => $rightDomain->getAuthority(), + 'domain' => $rightDomain->authority(), 'longUrl' => 'foo', 'tags' => ['foo', 'bar'], ])), @@ -322,7 +322,7 @@ class ShortUrlRepositoryTest extends DatabaseTestCase $shortUrl, $this->repo->findOneMatching(ShortUrlCreation::fromRawData([ 'validSince' => $start, - 'domain' => $rightDomain->getAuthority(), + 'domain' => $rightDomain->authority(), 'apiKey' => $rightDomainApiKey, 'longUrl' => 'foo', 'tags' => ['foo', 'bar'], @@ -332,7 +332,7 @@ class ShortUrlRepositoryTest extends DatabaseTestCase $shortUrl, $this->repo->findOneMatching(ShortUrlCreation::fromRawData([ 'validSince' => $start, - 'domain' => $rightDomain->getAuthority(), + 'domain' => $rightDomain->authority(), 'apiKey' => $apiKey, 'longUrl' => 'foo', 'tags' => ['foo', 'bar'], @@ -341,7 +341,7 @@ class ShortUrlRepositoryTest extends DatabaseTestCase self::assertNull( $this->repo->findOneMatching(ShortUrlCreation::fromRawData([ 'validSince' => $start, - 'domain' => $rightDomain->getAuthority(), + 'domain' => $rightDomain->authority(), 'apiKey' => $wrongDomainApiKey, 'longUrl' => 'foo', 'tags' => ['foo', 'bar'], diff --git a/module/Core/test-db/Tag/Repository/TagRepositoryTest.php b/module/Core/test-db/Tag/Repository/TagRepositoryTest.php index 4365731a..24a8f516 100644 --- a/module/Core/test-db/Tag/Repository/TagRepositoryTest.php +++ b/module/Core/test-db/Tag/Repository/TagRepositoryTest.php @@ -249,7 +249,7 @@ class TagRepositoryTest extends DatabaseTestCase $shortUrl2 = ShortUrl::create( ShortUrlCreation::fromRawData( - ['domain' => $domain->getAuthority(), 'longUrl' => 'longUrl', 'tags' => $secondUrlTags], + ['domain' => $domain->authority(), 'longUrl' => 'longUrl', 'tags' => $secondUrlTags], ), $this->relationResolver, ); diff --git a/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php b/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php index df4c5334..01c8e590 100644 --- a/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php +++ b/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php @@ -265,7 +265,7 @@ class VisitRepositoryTest extends DatabaseTestCase $this->getEntityManager()->persist($apiKey1); $shortUrl = ShortUrl::create( ShortUrlCreation::fromRawData( - ['apiKey' => $apiKey1, 'domain' => $domain->getAuthority(), 'longUrl' => 'longUrl'], + ['apiKey' => $apiKey1, 'domain' => $domain->authority(), 'longUrl' => 'longUrl'], ), $this->relationResolver, ); @@ -280,7 +280,7 @@ class VisitRepositoryTest extends DatabaseTestCase $shortUrl3 = ShortUrl::create( ShortUrlCreation::fromRawData( - ['apiKey' => $apiKey2, 'domain' => $domain->getAuthority(), 'longUrl' => 'longUrl'], + ['apiKey' => $apiKey2, 'domain' => $domain->authority(), 'longUrl' => 'longUrl'], ), $this->relationResolver, ); diff --git a/module/Core/test/ShortUrl/Resolver/PersistenceShortUrlRelationResolverTest.php b/module/Core/test/ShortUrl/Resolver/PersistenceShortUrlRelationResolverTest.php index 8734b95c..fed61862 100644 --- a/module/Core/test/ShortUrl/Resolver/PersistenceShortUrlRelationResolverTest.php +++ b/module/Core/test/ShortUrl/Resolver/PersistenceShortUrlRelationResolverTest.php @@ -52,7 +52,7 @@ class PersistenceShortUrlRelationResolverTest extends TestCase self::assertSame($result, $foundDomain); } self::assertInstanceOf(Domain::class, $result); - self::assertEquals($authority, $result->getAuthority()); + self::assertEquals($authority, $result->authority()); } public function provideFoundDomains(): iterable diff --git a/module/Core/test/ShortUrl/Resolver/SimpleShortUrlRelationResolverTest.php b/module/Core/test/ShortUrl/Resolver/SimpleShortUrlRelationResolverTest.php index f0cc7023..f1925c68 100644 --- a/module/Core/test/ShortUrl/Resolver/SimpleShortUrlRelationResolverTest.php +++ b/module/Core/test/ShortUrl/Resolver/SimpleShortUrlRelationResolverTest.php @@ -30,7 +30,7 @@ class SimpleShortUrlRelationResolverTest extends TestCase self::assertNull($result); } else { self::assertInstanceOf(Domain::class, $result); - self::assertEquals($domain, $result->getAuthority()); + self::assertEquals($domain, $result->authority()); } } diff --git a/module/Core/test/ShortUrl/ShortUrlServiceTest.php b/module/Core/test/ShortUrl/ShortUrlServiceTest.php index 96e0c9f5..903c3d3d 100644 --- a/module/Core/test/ShortUrl/ShortUrlServiceTest.php +++ b/module/Core/test/ShortUrl/ShortUrlServiceTest.php @@ -7,7 +7,9 @@ namespace ShlinkioTest\Shlink\Core\ShortUrl; use Cake\Chronos\Chronos; use Doctrine\ORM\EntityManagerInterface; use PHPUnit\Framework\MockObject\MockObject; +use PHPUnit\Framework\MockObject\Rule\InvocationOrder; use PHPUnit\Framework\TestCase; +use Shlinkio\Shlink\Core\Model\DeviceType; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlTitleResolutionHelperInterface; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlEdition; @@ -23,21 +25,20 @@ class ShortUrlServiceTest extends TestCase use ApiKeyHelpersTrait; private ShortUrlService $service; - private MockObject & EntityManagerInterface $em; private MockObject & ShortUrlResolverInterface $urlResolver; private MockObject & ShortUrlTitleResolutionHelperInterface $titleResolutionHelper; protected function setUp(): void { - $this->em = $this->createMock(EntityManagerInterface::class); - $this->em->method('persist')->willReturn(null); - $this->em->method('flush')->willReturn(null); + $em = $this->createMock(EntityManagerInterface::class); + $em->method('persist')->willReturn(null); + $em->method('flush')->willReturn(null); $this->urlResolver = $this->createMock(ShortUrlResolverInterface::class); $this->titleResolutionHelper = $this->createMock(ShortUrlTitleResolutionHelperInterface::class); $this->service = new ShortUrlService( - $this->em, + $em, $this->urlResolver, $this->titleResolutionHelper, new SimpleShortUrlRelationResolver(), @@ -49,7 +50,7 @@ class ShortUrlServiceTest extends TestCase * @dataProvider provideShortUrlEdits */ public function updateShortUrlUpdatesProvidedData( - int $expectedValidateCalls, + InvocationOrder $expectedValidateCalls, ShortUrlEdition $shortUrlEdit, ?ApiKey $apiKey, ): void { @@ -61,7 +62,7 @@ class ShortUrlServiceTest extends TestCase $apiKey, )->willReturn($shortUrl); - $this->titleResolutionHelper->expects($this->exactly($expectedValidateCalls)) + $this->titleResolutionHelper->expects($expectedValidateCalls) ->method('processTitleAndValidateUrl') ->with($shortUrlEdit) ->willReturn($shortUrlEdit); @@ -72,34 +73,44 @@ class ShortUrlServiceTest extends TestCase $apiKey, ); + $resolveDeviceLongUrls = function () use ($shortUrlEdit): array { + $result = []; + foreach ($shortUrlEdit->deviceLongUrls ?? [] as $longUrl) { + $result[$longUrl->deviceType->value] = $longUrl->longUrl; + } + + return $result; + }; + self::assertSame($shortUrl, $result); self::assertEquals($shortUrlEdit->validSince, $shortUrl->getValidSince()); self::assertEquals($shortUrlEdit->validUntil, $shortUrl->getValidUntil()); self::assertEquals($shortUrlEdit->maxVisits, $shortUrl->getMaxVisits()); self::assertEquals($shortUrlEdit->longUrl ?? $originalLongUrl, $shortUrl->getLongUrl()); + self::assertEquals($resolveDeviceLongUrls(), $shortUrl->deviceLongUrls()); } public function provideShortUrlEdits(): iterable { - yield 'no long URL' => [0, ShortUrlEdition::fromRawData( - [ - 'validSince' => Chronos::parse('2017-01-01 00:00:00')->toAtomString(), - 'validUntil' => Chronos::parse('2017-01-05 00:00:00')->toAtomString(), - 'maxVisits' => 5, + yield 'no long URL' => [$this->never(), ShortUrlEdition::fromRawData([ + 'validSince' => Chronos::parse('2017-01-01 00:00:00')->toAtomString(), + 'validUntil' => Chronos::parse('2017-01-05 00:00:00')->toAtomString(), + 'maxVisits' => 5, + ]), null]; + yield 'long URL and API key' => [$this->once(), ShortUrlEdition::fromRawData([ + 'validSince' => Chronos::parse('2017-01-01 00:00:00')->toAtomString(), + 'maxVisits' => 10, + 'longUrl' => 'modifiedLongUrl', + ]), ApiKey::create()]; + yield 'long URL with validation' => [$this->once(), ShortUrlEdition::fromRawData([ + 'longUrl' => 'modifiedLongUrl', + 'validateUrl' => true, + ]), null]; + yield 'device redirects' => [$this->never(), ShortUrlEdition::fromRawData([ + 'deviceLongUrls' => [ + DeviceType::IOS->value => 'iosLongUrl', + DeviceType::ANDROID->value => 'androidLongUrl', ], - ), null]; - yield 'long URL' => [1, ShortUrlEdition::fromRawData( - [ - 'validSince' => Chronos::parse('2017-01-01 00:00:00')->toAtomString(), - 'maxVisits' => 10, - 'longUrl' => 'modifiedLongUrl', - ], - ), ApiKey::create()]; - yield 'long URL with validation' => [1, ShortUrlEdition::fromRawData( - [ - 'longUrl' => 'modifiedLongUrl', - 'validateUrl' => true, - ], - ), null]; + ]), null]; } } diff --git a/module/Rest/src/ApiKey/Model/RoleDefinition.php b/module/Rest/src/ApiKey/Model/RoleDefinition.php index 6132772a..bc868f41 100644 --- a/module/Rest/src/ApiKey/Model/RoleDefinition.php +++ b/module/Rest/src/ApiKey/Model/RoleDefinition.php @@ -22,7 +22,7 @@ final class RoleDefinition { return new self( Role::DOMAIN_SPECIFIC, - ['domain_id' => $domain->getId(), 'authority' => $domain->getAuthority()], + ['domain_id' => $domain->getId(), 'authority' => $domain->authority()], ); } } diff --git a/module/Rest/src/Entity/ApiKey.php b/module/Rest/src/Entity/ApiKey.php index 297cdb45..57fecdd0 100644 --- a/module/Rest/src/Entity/ApiKey.php +++ b/module/Rest/src/Entity/ApiKey.php @@ -147,12 +147,9 @@ class ApiKey extends AbstractEntity $meta = $roleDefinition->meta; if ($this->hasRole($role)) { - /** @var ApiKeyRole $apiKeyRole */ - $apiKeyRole = $this->roles->get($role->value); - $apiKeyRole->updateMeta($meta); + $this->roles->get($role->value)?->updateMeta($meta); } else { - $apiKeyRole = new ApiKeyRole($roleDefinition->role, $roleDefinition->meta, $this); - $this->roles[$role->value] = $apiKeyRole; + $this->roles->set($role->value, new ApiKeyRole($role, $meta, $this)); } } } diff --git a/module/Rest/src/Middleware/ShortUrl/OverrideDomainMiddleware.php b/module/Rest/src/Middleware/ShortUrl/OverrideDomainMiddleware.php index f4d01e97..ab92c77a 100644 --- a/module/Rest/src/Middleware/ShortUrl/OverrideDomainMiddleware.php +++ b/module/Rest/src/Middleware/ShortUrl/OverrideDomainMiddleware.php @@ -34,11 +34,11 @@ class OverrideDomainMiddleware implements MiddlewareInterface if ($requestMethod === RequestMethodInterface::METHOD_POST) { /** @var array $payload */ $payload = $request->getParsedBody(); - $payload[ShortUrlInputFilter::DOMAIN] = $domain->getAuthority(); + $payload[ShortUrlInputFilter::DOMAIN] = $domain->authority(); return $handler->handle($request->withParsedBody($payload)); } - return $handler->handle($request->withAttribute(ShortUrlInputFilter::DOMAIN, $domain->getAuthority())); + return $handler->handle($request->withAttribute(ShortUrlInputFilter::DOMAIN, $domain->authority())); } }