Added logic to persist device long URLs while creating/editing a short URL

This commit is contained in:
Alejandro Celaya 2023-01-15 13:08:21 +01:00
parent fdadf3ba07
commit a93edf158e
22 changed files with 142 additions and 98 deletions

View file

@ -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')

View file

@ -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'] ?? []];

View file

@ -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

View file

@ -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

View file

@ -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

View file

@ -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<int, Visit> */
private Collection $visits;
/** @var Collection<int, DeviceLongUrl> */
private Collection $deviceLongUrls;
/** @var Collection<int, Tag> */
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;
}
}

View file

@ -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'] ?? '';
}
}

View file

@ -18,6 +18,7 @@ final class ShortUrlEdition implements TitleResolutionModelInterface
{
/**
* @param string[] $tags
* @param DeviceLongUrlPair[] $deviceLongUrls
*/
private function __construct(
private readonly bool $longUrlPropWasProvided = false,

View file

@ -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);
}

View file

@ -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<int, Tag>
* @return Collections\Collection<int, Tag>
*/
public function resolveTags(array $tags): Collections\Collection
{

View file

@ -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);
}

View file

@ -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) {

View file

@ -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)

View file

@ -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'],

View file

@ -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,
);

View file

@ -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,
);

View file

@ -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

View file

@ -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());
}
}

View file

@ -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];
}
}

View file

@ -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()],
);
}
}

View file

@ -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));
}
}
}

View file

@ -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()));
}
}