Inlcuded tags as part of the ShortUrlMeta

This commit is contained in:
Alejandro Celaya 2021-01-30 19:17:12 +01:00
parent 3f2bd657e1
commit 063ee9c195
14 changed files with 60 additions and 99 deletions

View File

@ -145,7 +145,7 @@ class GenerateShortUrlCommand extends BaseCommand
$doValidateUrl = $this->doValidateUrl($input);
try {
$shortUrl = $this->urlShortener->shorten($tags, ShortUrlMeta::fromRawData([
$shortUrl = $this->urlShortener->shorten(ShortUrlMeta::fromRawData([
ShortUrlMetaInputFilter::LONG_URL => $longUrl,
ShortUrlMetaInputFilter::VALID_SINCE => $this->getOptionWithDeprecatedFallback($input, 'valid-since'),
ShortUrlMetaInputFilter::VALID_UNTIL => $this->getOptionWithDeprecatedFallback($input, 'valid-until'),
@ -158,6 +158,7 @@ class GenerateShortUrlCommand extends BaseCommand
ShortUrlMetaInputFilter::DOMAIN => $input->getOption('domain'),
ShortUrlMetaInputFilter::SHORT_CODE_LENGTH => $shortCodeLength,
ShortUrlMetaInputFilter::VALIDATE_URL => $doValidateUrl,
ShortUrlMetaInputFilter::TAGS => $tags,
]));
$io->writeln([

View File

@ -91,11 +91,11 @@ class GenerateShortUrlCommandTest extends TestCase
{
$shortUrl = ShortUrl::createEmpty();
$urlToShortCode = $this->urlShortener->shorten(
Argument::that(function (array $tags) {
Argument::that(function (ShortUrlMeta $meta) {
$tags = $meta->getTags();
Assert::assertEquals(['foo', 'bar', 'baz', 'boo', 'zar'], $tags);
return $tags;
return true;
}),
Argument::cetera(),
)->willReturn($shortUrl);
$this->commandTester->execute([
@ -117,7 +117,6 @@ class GenerateShortUrlCommandTest extends TestCase
{
$shortUrl = ShortUrl::createEmpty();
$urlToShortCode = $this->urlShortener->shorten(
Argument::type('array'),
Argument::that(function (ShortUrlMeta $meta) use ($expectedValidateUrl) {
Assert::assertEquals($expectedValidateUrl, $meta->doValidateUrl());
return $meta;

View File

@ -1,30 +0,0 @@
<?php
declare(strict_types=1);
namespace Shlinkio\Shlink\Core\Model;
final class CreateShortUrlData
{
private array $tags;
private ShortUrlMeta $meta;
public function __construct(array $tags = [], ?ShortUrlMeta $meta = null)
{
$this->tags = $tags;
$this->meta = $meta ?? ShortUrlMeta::createEmpty();
}
/**
* @return string[]
*/
public function getTags(): array
{
return $this->tags;
}
public function getMeta(): ShortUrlMeta
{
return $this->meta;
}
}

View File

@ -201,7 +201,7 @@ class ShortUrlRepository extends EntitySpecificationRepository implements ShortU
return $qb;
}
public function findOneMatching(array $tags, ShortUrlMeta $meta): ?ShortUrl
public function findOneMatching(ShortUrlMeta $meta): ?ShortUrl
{
$qb = $this->getEntityManager()->createQueryBuilder();
@ -239,6 +239,7 @@ class ShortUrlRepository extends EntitySpecificationRepository implements ShortU
$this->applySpecification($qb, $apiKey->spec(), 's');
}
$tags = $meta->getTags();
$tagsAmount = count($tags);
if ($tagsAmount === 0) {
return $qb->getQuery()->getOneOrNullResult();

View File

@ -38,7 +38,7 @@ interface ShortUrlRepositoryInterface extends ObjectRepository, EntitySpecificat
public function shortCodeIsInUse(string $slug, ?string $domain, ?Specification $spec = null): bool;
public function findOneMatching(array $tags, ShortUrlMeta $meta): ?ShortUrl;
public function findOneMatching(ShortUrlMeta $meta): ?ShortUrl;
public function importedUrlExists(ImportedShlinkUrl $url): bool;
}

View File

@ -38,24 +38,23 @@ class UrlShortener implements UrlShortenerInterface
}
/**
* @param string[] $tags
* @throws NonUniqueSlugException
* @throws InvalidUrlException
* @throws Throwable
*/
public function shorten(array $tags, ShortUrlMeta $meta): ShortUrl
public function shorten(ShortUrlMeta $meta): ShortUrl
{
// First, check if a short URL exists for all provided params
$existingShortUrl = $this->findExistingShortUrlIfExists($tags, $meta);
$existingShortUrl = $this->findExistingShortUrlIfExists($meta);
if ($existingShortUrl !== null) {
return $existingShortUrl;
}
$this->urlValidator->validateUrl($meta->getLongUrl(), $meta->doValidateUrl());
return $this->em->transactional(function () use ($tags, $meta) {
return $this->em->transactional(function () use ($meta) {
$shortUrl = ShortUrl::fromMeta($meta, $this->relationResolver);
$shortUrl->setTags($this->tagNamesToEntities($this->em, $tags));
$shortUrl->setTags($this->tagNamesToEntities($this->em, $meta->getTags()));
$this->verifyShortCodeUniqueness($meta, $shortUrl);
$this->em->persist($shortUrl);
@ -64,7 +63,7 @@ class UrlShortener implements UrlShortenerInterface
});
}
private function findExistingShortUrlIfExists(array $tags, ShortUrlMeta $meta): ?ShortUrl
private function findExistingShortUrlIfExists(ShortUrlMeta $meta): ?ShortUrl
{
if (! $meta->findIfExists()) {
return null;
@ -72,7 +71,7 @@ class UrlShortener implements UrlShortenerInterface
/** @var ShortUrlRepositoryInterface $repo */
$repo = $this->em->getRepository(ShortUrl::class);
return $repo->findOneMatching($tags, $meta);
return $repo->findOneMatching($meta);
}
private function verifyShortCodeUniqueness(ShortUrlMeta $meta, ShortUrl $shortUrlToBeCreated): void

View File

@ -12,9 +12,8 @@ use Shlinkio\Shlink\Core\Model\ShortUrlMeta;
interface UrlShortenerInterface
{
/**
* @param string[] $tags
* @throws NonUniqueSlugException
* @throws InvalidUrlException
*/
public function shorten(array $tags, ShortUrlMeta $meta): ShortUrl;
public function shorten(ShortUrlMeta $meta): ShortUrl;
}

View File

@ -216,16 +216,16 @@ class ShortUrlRepositoryTest extends DatabaseTestCase
/** @test */
public function findOneMatchingReturnsNullForNonExistingShortUrls(): void
{
self::assertNull($this->repo->findOneMatching([], ShortUrlMeta::createEmpty()));
self::assertNull($this->repo->findOneMatching([], ShortUrlMeta::fromRawData(['longUrl' => 'foobar'])));
self::assertNull($this->repo->findOneMatching(ShortUrlMeta::createEmpty()));
self::assertNull($this->repo->findOneMatching(ShortUrlMeta::fromRawData(['longUrl' => 'foobar'])));
self::assertNull($this->repo->findOneMatching(
['foo', 'bar'],
ShortUrlMeta::fromRawData(['longUrl' => 'foobar']),
ShortUrlMeta::fromRawData(['longUrl' => 'foobar', 'tags' => ['foo', 'bar']]),
));
self::assertNull($this->repo->findOneMatching(['foo', 'bar'], ShortUrlMeta::fromRawData([
self::assertNull($this->repo->findOneMatching(ShortUrlMeta::fromRawData([
'validSince' => Chronos::parse('2020-03-05 20:18:30'),
'customSlug' => 'this_slug_does_not_exist',
'longUrl' => 'foobar',
'tags' => ['foo', 'bar'],
])));
}
@ -263,17 +263,16 @@ class ShortUrlRepositoryTest extends DatabaseTestCase
self::assertSame(
$shortUrl,
$this->repo->findOneMatching(
['foo', 'bar'],
ShortUrlMeta::fromRawData(['validSince' => $start, 'longUrl' => 'foo']),
ShortUrlMeta::fromRawData(['validSince' => $start, 'longUrl' => 'foo', 'tags' => ['foo', 'bar']]),
),
);
self::assertSame(
$shortUrl2,
$this->repo->findOneMatching([], ShortUrlMeta::fromRawData(['validUntil' => $end, 'longUrl' => 'bar'])),
$this->repo->findOneMatching(ShortUrlMeta::fromRawData(['validUntil' => $end, 'longUrl' => 'bar'])),
);
self::assertSame(
$shortUrl3,
$this->repo->findOneMatching([], ShortUrlMeta::fromRawData([
$this->repo->findOneMatching(ShortUrlMeta::fromRawData([
'validSince' => $start,
'validUntil' => $end,
'longUrl' => 'baz',
@ -281,7 +280,7 @@ class ShortUrlRepositoryTest extends DatabaseTestCase
);
self::assertSame(
$shortUrl4,
$this->repo->findOneMatching([], ShortUrlMeta::fromRawData([
$this->repo->findOneMatching(ShortUrlMeta::fromRawData([
'customSlug' => 'custom',
'validUntil' => $end,
'longUrl' => 'foo',
@ -289,11 +288,11 @@ class ShortUrlRepositoryTest extends DatabaseTestCase
);
self::assertSame(
$shortUrl5,
$this->repo->findOneMatching([], ShortUrlMeta::fromRawData(['maxVisits' => 3, 'longUrl' => 'foo'])),
$this->repo->findOneMatching(ShortUrlMeta::fromRawData(['maxVisits' => 3, 'longUrl' => 'foo'])),
);
self::assertSame(
$shortUrl6,
$this->repo->findOneMatching([], ShortUrlMeta::fromRawData(['domain' => 'doma.in', 'longUrl' => 'foo'])),
$this->repo->findOneMatching(ShortUrlMeta::fromRawData(['domain' => 'doma.in', 'longUrl' => 'foo'])),
);
}
@ -304,6 +303,9 @@ class ShortUrlRepositoryTest extends DatabaseTestCase
$meta = ShortUrlMeta::fromRawData(['validSince' => $start, 'maxVisits' => 50, 'longUrl' => 'foo']);
$tags = ['foo', 'bar'];
$tagEntities = $this->tagNamesToEntities($this->getEntityManager(), $tags);
$metaWithTags = ShortUrlMeta::fromRawData(
['validSince' => $start, 'maxVisits' => 50, 'longUrl' => 'foo', 'tags' => $tags],
);
$shortUrl1 = ShortUrl::fromMeta($meta);
$shortUrl1->setTags($tagEntities);
@ -319,7 +321,7 @@ class ShortUrlRepositoryTest extends DatabaseTestCase
$this->getEntityManager()->flush();
$result = $this->repo->findOneMatching($tags, $meta);
$result = $this->repo->findOneMatching($metaWithTags);
self::assertSame($shortUrl1, $result);
self::assertNotSame($shortUrl2, $result);
@ -358,53 +360,58 @@ class ShortUrlRepositoryTest extends DatabaseTestCase
self::assertSame(
$shortUrl,
$this->repo->findOneMatching(
['foo', 'bar'],
ShortUrlMeta::fromRawData(['validSince' => $start, 'longUrl' => 'foo']),
ShortUrlMeta::fromRawData(['validSince' => $start, 'longUrl' => 'foo', 'tags' => ['foo', 'bar']]),
),
);
self::assertSame($shortUrl, $this->repo->findOneMatching(['foo', 'bar'], ShortUrlMeta::fromRawData([
self::assertSame($shortUrl, $this->repo->findOneMatching(ShortUrlMeta::fromRawData([
'validSince' => $start,
'apiKey' => $apiKey,
'longUrl' => 'foo',
'tags' => ['foo', 'bar'],
])));
self::assertNull($this->repo->findOneMatching(['foo', 'bar'], ShortUrlMeta::fromRawData([
self::assertNull($this->repo->findOneMatching(ShortUrlMeta::fromRawData([
'validSince' => $start,
'apiKey' => $otherApiKey,
'longUrl' => 'foo',
'tags' => ['foo', 'bar'],
])));
self::assertSame(
$shortUrl,
$this->repo->findOneMatching(['foo', 'bar'], ShortUrlMeta::fromRawData([
$this->repo->findOneMatching(ShortUrlMeta::fromRawData([
'validSince' => $start,
'domain' => $rightDomain->getAuthority(),
'longUrl' => 'foo',
'tags' => ['foo', 'bar'],
])),
);
self::assertSame(
$shortUrl,
$this->repo->findOneMatching(['foo', 'bar'], ShortUrlMeta::fromRawData([
$this->repo->findOneMatching(ShortUrlMeta::fromRawData([
'validSince' => $start,
'domain' => $rightDomain->getAuthority(),
'apiKey' => $rightDomainApiKey,
'longUrl' => 'foo',
'tags' => ['foo', 'bar'],
])),
);
self::assertSame(
$shortUrl,
$this->repo->findOneMatching(['foo', 'bar'], ShortUrlMeta::fromRawData([
$this->repo->findOneMatching(ShortUrlMeta::fromRawData([
'validSince' => $start,
'domain' => $rightDomain->getAuthority(),
'apiKey' => $apiKey,
'longUrl' => 'foo',
'tags' => ['foo', 'bar'],
])),
);
self::assertNull(
$this->repo->findOneMatching(['foo', 'bar'], ShortUrlMeta::fromRawData([
$this->repo->findOneMatching(ShortUrlMeta::fromRawData([
'validSince' => $start,
'domain' => $rightDomain->getAuthority(),
'apiKey' => $wrongDomainApiKey,
'longUrl' => 'foo',
'tags' => ['foo', 'bar'],
])),
);
}

View File

@ -69,7 +69,6 @@ class UrlShortenerTest extends TestCase
public function urlIsProperlyShortened(): void
{
$shortUrl = $this->urlShortener->shorten(
[],
ShortUrlMeta::fromRawData(['longUrl' => 'http://foobar.com/12345/hello?foo=bar']),
);
@ -84,7 +83,7 @@ class UrlShortenerTest extends TestCase
$ensureUniqueness->shouldBeCalledOnce();
$this->expectException(NonUniqueSlugException::class);
$this->urlShortener->shorten([], ShortUrlMeta::fromRawData(
$this->urlShortener->shorten(ShortUrlMeta::fromRawData(
['customSlug' => 'custom-slug', 'longUrl' => 'http://foobar.com/12345/hello?foo=bar'],
));
}
@ -93,13 +92,13 @@ class UrlShortenerTest extends TestCase
* @test
* @dataProvider provideExistingShortUrls
*/
public function existingShortUrlIsReturnedWhenRequested(array $tags, ShortUrlMeta $meta, ShortUrl $expected): void
public function existingShortUrlIsReturnedWhenRequested(ShortUrlMeta $meta, ShortUrl $expected): void
{
$repo = $this->prophesize(ShortUrlRepository::class);
$findExisting = $repo->findOneMatching(Argument::cetera())->willReturn($expected);
$getRepo = $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal());
$result = $this->urlShortener->shorten($tags, $meta);
$result = $this->urlShortener->shorten($meta);
$findExisting->shouldHaveBeenCalledOnce();
$getRepo->shouldHaveBeenCalledOnce();
@ -112,24 +111,21 @@ class UrlShortenerTest extends TestCase
{
$url = 'http://foo.com';
yield [[], ShortUrlMeta::fromRawData(['findIfExists' => true, 'longUrl' => $url]), ShortUrl::withLongUrl(
yield [ShortUrlMeta::fromRawData(['findIfExists' => true, 'longUrl' => $url]), ShortUrl::withLongUrl(
$url,
)];
yield [[], ShortUrlMeta::fromRawData(
yield [ShortUrlMeta::fromRawData(
['findIfExists' => true, 'customSlug' => 'foo', 'longUrl' => $url],
), ShortUrl::withLongUrl($url)];
yield [
['foo', 'bar'],
ShortUrlMeta::fromRawData(['findIfExists' => true, 'longUrl' => $url]),
ShortUrlMeta::fromRawData(['findIfExists' => true, 'longUrl' => $url, 'tags' => ['foo', 'bar']]),
ShortUrl::withLongUrl($url)->setTags(new ArrayCollection([new Tag('bar'), new Tag('foo')])),
];
yield [
[],
ShortUrlMeta::fromRawData(['findIfExists' => true, 'maxVisits' => 3, 'longUrl' => $url]),
ShortUrl::fromMeta(ShortUrlMeta::fromRawData(['maxVisits' => 3, 'longUrl' => $url])),
];
yield [
[],
ShortUrlMeta::fromRawData(
['findIfExists' => true, 'validSince' => Chronos::parse('2017-01-01'), 'longUrl' => $url],
),
@ -138,7 +134,6 @@ class UrlShortenerTest extends TestCase
),
];
yield [
[],
ShortUrlMeta::fromRawData(
['findIfExists' => true, 'validUntil' => Chronos::parse('2017-01-01'), 'longUrl' => $url],
),
@ -147,17 +142,16 @@ class UrlShortenerTest extends TestCase
),
];
yield [
[],
ShortUrlMeta::fromRawData(['findIfExists' => true, 'domain' => 'example.com', 'longUrl' => $url]),
ShortUrl::fromMeta(ShortUrlMeta::fromRawData(['domain' => 'example.com', 'longUrl' => $url])),
];
yield [
['baz', 'foo', 'bar'],
ShortUrlMeta::fromRawData([
'findIfExists' => true,
'validUntil' => Chronos::parse('2017-01-01'),
'maxVisits' => 4,
'longUrl' => $url,
'tags' => ['baz', 'foo', 'bar'],
]),
ShortUrl::fromMeta(ShortUrlMeta::fromRawData([
'validUntil' => Chronos::parse('2017-01-01'),

View File

@ -8,7 +8,7 @@ use Laminas\Diactoros\Response\JsonResponse;
use Psr\Http\Message\ResponseInterface as Response;
use Psr\Http\Message\ServerRequestInterface as Request;
use Shlinkio\Shlink\Core\Exception\ValidationException;
use Shlinkio\Shlink\Core\Model\CreateShortUrlData;
use Shlinkio\Shlink\Core\Model\ShortUrlMeta;
use Shlinkio\Shlink\Core\Service\UrlShortenerInterface;
use Shlinkio\Shlink\Core\Transformer\ShortUrlDataTransformer;
use Shlinkio\Shlink\Rest\Action\AbstractRestAction;
@ -26,11 +26,9 @@ abstract class AbstractCreateShortUrlAction extends AbstractRestAction
public function handle(Request $request): Response
{
$shortUrlData = $this->buildShortUrlData($request);
$tags = $shortUrlData->getTags();
$shortUrlMeta = $shortUrlData->getMeta();
$shortUrlMeta = $this->buildShortUrlData($request);
$shortUrl = $this->urlShortener->shorten($tags, $shortUrlMeta);
$shortUrl = $this->urlShortener->shorten($shortUrlMeta);
$transformer = new ShortUrlDataTransformer($this->domainConfig);
return new JsonResponse($transformer->transform($shortUrl));
@ -39,5 +37,5 @@ abstract class AbstractCreateShortUrlAction extends AbstractRestAction
/**
* @throws ValidationException
*/
abstract protected function buildShortUrlData(Request $request): CreateShortUrlData;
abstract protected function buildShortUrlData(Request $request): ShortUrlMeta;
}

View File

@ -6,7 +6,6 @@ namespace Shlinkio\Shlink\Rest\Action\ShortUrl;
use Psr\Http\Message\ServerRequestInterface as Request;
use Shlinkio\Shlink\Core\Exception\ValidationException;
use Shlinkio\Shlink\Core\Model\CreateShortUrlData;
use Shlinkio\Shlink\Core\Model\ShortUrlMeta;
use Shlinkio\Shlink\Core\Validation\ShortUrlMetaInputFilter;
use Shlinkio\Shlink\Rest\Middleware\AuthenticationMiddleware;
@ -19,12 +18,11 @@ class CreateShortUrlAction extends AbstractCreateShortUrlAction
/**
* @throws ValidationException
*/
protected function buildShortUrlData(Request $request): CreateShortUrlData
protected function buildShortUrlData(Request $request): ShortUrlMeta
{
$payload = (array) $request->getParsedBody();
$payload[ShortUrlMetaInputFilter::API_KEY] = AuthenticationMiddleware::apiKeyFromRequest($request);
$meta = ShortUrlMeta::fromRawData($payload);
return new CreateShortUrlData((array) ($payload['tags'] ?? []), $meta);
return ShortUrlMeta::fromRawData($payload);
}
}

View File

@ -5,7 +5,6 @@ declare(strict_types=1);
namespace Shlinkio\Shlink\Rest\Action\ShortUrl;
use Psr\Http\Message\ServerRequestInterface as Request;
use Shlinkio\Shlink\Core\Model\CreateShortUrlData;
use Shlinkio\Shlink\Core\Model\ShortUrlMeta;
use Shlinkio\Shlink\Core\Validation\ShortUrlMetaInputFilter;
use Shlinkio\Shlink\Rest\Middleware\AuthenticationMiddleware;
@ -15,17 +14,17 @@ class SingleStepCreateShortUrlAction extends AbstractCreateShortUrlAction
protected const ROUTE_PATH = '/short-urls/shorten';
protected const ROUTE_ALLOWED_METHODS = [self::METHOD_GET];
protected function buildShortUrlData(Request $request): CreateShortUrlData
protected function buildShortUrlData(Request $request): ShortUrlMeta
{
$query = $request->getQueryParams();
$longUrl = $query['longUrl'] ?? null;
$apiKey = AuthenticationMiddleware::apiKeyFromRequest($request);
return new CreateShortUrlData([], ShortUrlMeta::fromRawData([
return ShortUrlMeta::fromRawData([
ShortUrlMetaInputFilter::LONG_URL => $longUrl,
ShortUrlMetaInputFilter::API_KEY => $apiKey,
// This will usually be null, unless this API key enforces one specific domain
ShortUrlMetaInputFilter::DOMAIN => $request->getAttribute(ShortUrlMetaInputFilter::DOMAIN),
]));
]);
}
}

View File

@ -54,10 +54,7 @@ class CreateShortUrlActionTest extends TestCase
];
$expectedMeta['apiKey'] = $apiKey;
$shorten = $this->urlShortener->shorten(
Argument::type('array'),
ShortUrlMeta::fromRawData($expectedMeta),
)->willReturn($shortUrl);
$shorten = $this->urlShortener->shorten(ShortUrlMeta::fromRawData($expectedMeta))->willReturn($shortUrl);
$request = ServerRequestFactory::fromGlobals()->withParsedBody($body)->withAttribute(ApiKey::class, $apiKey);

View File

@ -44,7 +44,6 @@ class SingleStepCreateShortUrlActionTest extends TestCase
'longUrl' => 'http://foobar.com',
])->withAttribute(ApiKey::class, $apiKey);
$generateShortCode = $this->urlShortener->shorten(
[],
ShortUrlMeta::fromRawData(['apiKey' => $apiKey, 'longUrl' => 'http://foobar.com']),
)->willReturn(ShortUrl::createEmpty());