Make it impossible to create a short URL with an empty long URL

This commit is contained in:
Alejandro Celaya 2023-01-22 11:27:16 +01:00
parent 13e443880a
commit 39adef8ab8
25 changed files with 49 additions and 61 deletions

View File

@ -40,7 +40,7 @@ class GetDomainVisitsCommandTest extends TestCase
/** @test */
public function outputIsProperlyGenerated(): void
{
$shortUrl = ShortUrl::createEmpty();
$shortUrl = ShortUrl::createFake();
$visit = Visit::forValidShortUrl($shortUrl, new Visitor('bar', 'foo', '', ''))->locate(
VisitLocation::fromGeolocation(new Location('', 'Spain', '', 'Madrid', 0, 0, '')),
);

View File

@ -48,7 +48,7 @@ class CreateShortUrlCommandTest extends TestCase
/** @test */
public function properShortCodeIsCreatedIfLongUrlIsCorrect(): void
{
$shortUrl = ShortUrl::createEmpty();
$shortUrl = ShortUrl::createFake();
$this->urlShortener->expects($this->once())->method('shorten')->withAnyParameters()->willReturn($shortUrl);
$this->stringifier->expects($this->once())->method('stringify')->with($shortUrl)->willReturn(
'stringified_short_url',
@ -98,7 +98,7 @@ class CreateShortUrlCommandTest extends TestCase
/** @test */
public function properlyProcessesProvidedTags(): void
{
$shortUrl = ShortUrl::createEmpty();
$shortUrl = ShortUrl::createFake();
$this->urlShortener->expects($this->once())->method('shorten')->with(
$this->callback(function (ShortUrlCreation $creation) {
Assert::assertEquals(['foo', 'bar', 'baz', 'boo', 'zar'], $creation->tags);
@ -130,7 +130,7 @@ class CreateShortUrlCommandTest extends TestCase
Assert::assertEquals($expectedDomain, $meta->domain);
return true;
}),
)->willReturn(ShortUrl::createEmpty());
)->willReturn(ShortUrl::createFake());
$this->stringifier->method('stringify')->with($this->isInstanceOf(ShortUrl::class))->willReturn('');
$input['longUrl'] = 'http://domain.com/foo/bar';
@ -153,7 +153,7 @@ class CreateShortUrlCommandTest extends TestCase
*/
public function urlValidationHasExpectedValueBasedOnProvidedFlags(array $options, ?bool $expectedValidateUrl): void
{
$shortUrl = ShortUrl::createEmpty();
$shortUrl = ShortUrl::createFake();
$this->urlShortener->expects($this->once())->method('shorten')->with(
$this->callback(function (ShortUrlCreation $meta) use ($expectedValidateUrl) {
Assert::assertEquals($expectedValidateUrl, $meta->doValidateUrl());

View File

@ -94,7 +94,7 @@ class GetShortUrlVisitsCommandTest extends TestCase
/** @test */
public function outputIsProperlyGenerated(): void
{
$visit = Visit::forValidShortUrl(ShortUrl::createEmpty(), new Visitor('bar', 'foo', '', ''))->locate(
$visit = Visit::forValidShortUrl(ShortUrl::createFake(), new Visitor('bar', 'foo', '', ''))->locate(
VisitLocation::fromGeolocation(new Location('', 'Spain', '', 'Madrid', 0, 0, '')),
);
$shortCode = 'abc123';

View File

@ -40,7 +40,7 @@ class GetTagVisitsCommandTest extends TestCase
/** @test */
public function outputIsProperlyGenerated(): void
{
$shortUrl = ShortUrl::createEmpty();
$shortUrl = ShortUrl::createFake();
$visit = Visit::forValidShortUrl($shortUrl, new Visitor('bar', 'foo', '', ''))->locate(
VisitLocation::fromGeolocation(new Location('', 'Spain', '', 'Madrid', 0, 0, '')),
);

View File

@ -40,7 +40,7 @@ class GetNonOrphanVisitsCommandTest extends TestCase
/** @test */
public function outputIsProperlyGenerated(): void
{
$shortUrl = ShortUrl::createEmpty();
$shortUrl = ShortUrl::createFake();
$visit = Visit::forValidShortUrl($shortUrl, new Visitor('bar', 'foo', '', ''))->locate(
VisitLocation::fromGeolocation(new Location('', 'Spain', '', 'Madrid', 0, 0, '')),
);

View File

@ -66,7 +66,7 @@ class LocateVisitsCommandTest extends TestCase
bool $expectWarningPrint,
array $args,
): void {
$visit = Visit::forValidShortUrl(ShortUrl::createEmpty(), new Visitor('', '', '1.2.3.4', ''));
$visit = Visit::forValidShortUrl(ShortUrl::createFake(), new Visitor('', '', '1.2.3.4', ''));
$location = VisitLocation::fromGeolocation(Location::emptyInstance());
$mockMethodBehavior = $this->invokeHelperMethods($visit, $location);
@ -113,7 +113,7 @@ class LocateVisitsCommandTest extends TestCase
*/
public function localhostAndEmptyAddressesAreIgnored(IpCannotBeLocatedException $e, string $message): void
{
$visit = Visit::forValidShortUrl(ShortUrl::createEmpty(), Visitor::emptyInstance());
$visit = Visit::forValidShortUrl(ShortUrl::createFake(), Visitor::emptyInstance());
$location = VisitLocation::fromGeolocation(Location::emptyInstance());
$this->lock->method('acquire')->with($this->isFalse())->willReturn(true);
@ -140,7 +140,7 @@ class LocateVisitsCommandTest extends TestCase
/** @test */
public function errorWhileLocatingIpIsDisplayed(): void
{
$visit = Visit::forValidShortUrl(ShortUrl::createEmpty(), new Visitor('', '', '1.2.3.4', ''));
$visit = Visit::forValidShortUrl(ShortUrl::createFake(), new Visitor('', '', '1.2.3.4', ''));
$location = VisitLocation::fromGeolocation(Location::emptyInstance());
$this->lock->method('acquire')->with($this->isFalse())->willReturn(true);

View File

@ -62,12 +62,9 @@ class ShortUrl extends AbstractEntity
{
}
/**
* @deprecated This should not be allowed
*/
public static function createEmpty(): self
public static function createFake(): self
{
return self::create(ShortUrlCreation::createEmpty());
return self::withLongUrl('foo');
}
/**

View File

@ -43,14 +43,6 @@ final class ShortUrlCreation implements TitleResolutionModelInterface
) {
}
/**
* @deprecated This should not be allowed
*/
public static function createEmpty(): self
{
return new self('');
}
/**
* @throws ValidationException
*/

View File

@ -134,7 +134,6 @@ class ShortUrlRepositoryTest extends DatabaseTestCase
/** @test */
public function findOneMatchingReturnsNullForNonExistingShortUrls(): void
{
self::assertNull($this->repo->findOneMatching(ShortUrlCreation::createEmpty()));
self::assertNull($this->repo->findOneMatching(ShortUrlCreation::fromRawData(['longUrl' => 'foobar'])));
self::assertNull($this->repo->findOneMatching(
ShortUrlCreation::fromRawData(['longUrl' => 'foobar', 'tags' => ['foo', 'bar']]),

View File

@ -31,7 +31,7 @@ class VisitLocationRepositoryTest extends DatabaseTestCase
*/
public function findVisitsReturnsProperVisits(int $blockSize): void
{
$shortUrl = ShortUrl::createEmpty();
$shortUrl = ShortUrl::createFake();
$this->getEntityManager()->persist($shortUrl);
for ($i = 0; $i < 6; $i++) {

View File

@ -56,7 +56,7 @@ class QrCodeActionTest extends TestCase
$shortCode = 'abc123';
$this->urlResolver->expects($this->once())->method('resolveEnabledShortUrl')->with(
ShortUrlIdentifier::fromShortCodeAndDomain($shortCode, ''),
)->willReturn(ShortUrl::createEmpty());
)->willReturn(ShortUrl::createFake());
$delegate = $this->createMock(RequestHandlerInterface::class);
$delegate->expects($this->never())->method('handle');
@ -78,7 +78,7 @@ class QrCodeActionTest extends TestCase
$code = 'abc123';
$this->urlResolver->method('resolveEnabledShortUrl')->with(
ShortUrlIdentifier::fromShortCodeAndDomain($code, ''),
)->willReturn(ShortUrl::createEmpty());
)->willReturn(ShortUrl::createFake());
$delegate = $this->createMock(RequestHandlerInterface::class);
$req = (new ServerRequest())->withAttribute('shortCode', $code)->withQueryParams($query);
@ -111,7 +111,7 @@ class QrCodeActionTest extends TestCase
$code = 'abc123';
$this->urlResolver->method('resolveEnabledShortUrl')->with(
ShortUrlIdentifier::fromShortCodeAndDomain($code, ''),
)->willReturn(ShortUrl::createEmpty());
)->willReturn(ShortUrl::createFake());
$delegate = $this->createMock(RequestHandlerInterface::class);
$resp = $this->action($defaultOptions)->process($req->withAttribute('shortCode', $code), $delegate);

View File

@ -70,7 +70,7 @@ class LocateVisitTest extends TestCase
{
$event = new UrlVisited('123');
$this->em->expects($this->once())->method('find')->with(Visit::class, '123')->willReturn(
Visit::forValidShortUrl(ShortUrl::createEmpty(), new Visitor('', '', '1.2.3.4', '')),
Visit::forValidShortUrl(ShortUrl::createFake(), new Visitor('', '', '1.2.3.4', '')),
);
$this->em->expects($this->never())->method('flush');
$this->dbUpdater->expects($this->once())->method('databaseFileExists')->withAnyParameters()->willReturn(false);
@ -89,7 +89,7 @@ class LocateVisitTest extends TestCase
{
$event = new UrlVisited('123');
$this->em->expects($this->once())->method('find')->with(Visit::class, '123')->willReturn(
Visit::forValidShortUrl(ShortUrl::createEmpty(), new Visitor('', '', '1.2.3.4', '')),
Visit::forValidShortUrl(ShortUrl::createFake(), new Visitor('', '', '1.2.3.4', '')),
);
$this->em->expects($this->never())->method('flush');
$this->dbUpdater->expects($this->once())->method('databaseFileExists')->withAnyParameters()->willReturn(true);
@ -110,7 +110,7 @@ class LocateVisitTest extends TestCase
{
$event = new UrlVisited('123');
$this->em->expects($this->once())->method('find')->with(Visit::class, '123')->willReturn(
Visit::forValidShortUrl(ShortUrl::createEmpty(), new Visitor('', '', '1.2.3.4', '')),
Visit::forValidShortUrl(ShortUrl::createFake(), new Visitor('', '', '1.2.3.4', '')),
);
$this->em->expects($this->never())->method('flush');
$this->dbUpdater->expects($this->once())->method('databaseFileExists')->withAnyParameters()->willReturn(true);
@ -148,7 +148,7 @@ class LocateVisitTest extends TestCase
public function provideNonLocatableVisits(): iterable
{
$shortUrl = ShortUrl::createEmpty();
$shortUrl = ShortUrl::createFake();
yield 'null IP' => [Visit::forValidShortUrl($shortUrl, new Visitor('', '', null, ''))];
yield 'empty IP' => [Visit::forValidShortUrl($shortUrl, new Visitor('', '', '', ''))];
@ -183,11 +183,11 @@ class LocateVisitTest extends TestCase
public function provideIpAddresses(): iterable
{
yield 'no original IP address' => [
Visit::forValidShortUrl(ShortUrl::createEmpty(), new Visitor('', '', '1.2.3.4', '')),
Visit::forValidShortUrl(ShortUrl::createFake(), new Visitor('', '', '1.2.3.4', '')),
null,
];
yield 'original IP address' => [
Visit::forValidShortUrl(ShortUrl::createEmpty(), new Visitor('', '', '1.2.3.4', '')),
Visit::forValidShortUrl(ShortUrl::createFake(), new Visitor('', '', '1.2.3.4', '')),
'1.2.3.4',
];
yield 'base url' => [Visit::forBasePath(new Visitor('', '', '1.2.3.4', '')), '1.2.3.4'];

View File

@ -59,7 +59,7 @@ class NotifyVisitToMercureTest extends TestCase
public function notificationsAreSentWhenVisitIsFound(): void
{
$visitId = '123';
$visit = Visit::forValidShortUrl(ShortUrl::createEmpty(), Visitor::emptyInstance());
$visit = Visit::forValidShortUrl(ShortUrl::createFake(), Visitor::emptyInstance());
$update = Update::forTopicAndPayload('', []);
$this->em->expects($this->once())->method('find')->with(Visit::class, $visitId)->willReturn($visit);
@ -79,7 +79,7 @@ class NotifyVisitToMercureTest extends TestCase
public function debugIsLoggedWhenExceptionIsThrown(): void
{
$visitId = '123';
$visit = Visit::forValidShortUrl(ShortUrl::createEmpty(), Visitor::emptyInstance());
$visit = Visit::forValidShortUrl(ShortUrl::createFake(), Visitor::emptyInstance());
$update = Update::forTopicAndPayload('', []);
$e = new RuntimeException('Error');

View File

@ -123,7 +123,7 @@ class NotifyVisitToWebHooksTest extends TestCase
public function provideVisits(): iterable
{
yield 'regular visit' => [
Visit::forValidShortUrl(ShortUrl::createEmpty(), Visitor::emptyInstance()),
Visit::forValidShortUrl(ShortUrl::createFake(), Visitor::emptyInstance()),
['shortUrl', 'visit'],
];
yield 'orphan visit' => [Visit::forBasePath(Visitor::emptyInstance()), ['visit'],];

View File

@ -210,7 +210,7 @@ class ImportedLinksProcessorTest extends TestCase
]),
'<comment>Skipped</comment>. Imported <info>4</info> visits',
4,
ShortUrl::createEmpty(),
ShortUrl::createFake(),
];
yield 'existing short URL with previous imported visits' => [
$createImportedUrl([
@ -222,8 +222,8 @@ class ImportedLinksProcessorTest extends TestCase
]),
'<comment>Skipped</comment>. Imported <info>2</info> visits',
2,
ShortUrl::createEmpty()->setVisits(new ArrayCollection([
Visit::fromImport(ShortUrl::createEmpty(), new ImportedShlinkVisit('', '', $now, null)),
ShortUrl::createFake()->setVisits(new ArrayCollection([
Visit::fromImport(ShortUrl::createFake(), new ImportedShlinkVisit('', '', $now, null)),
])),
];
}

View File

@ -29,8 +29,8 @@ class DeleteShortUrlServiceTest extends TestCase
protected function setUp(): void
{
$shortUrl = ShortUrl::createEmpty()->setVisits(new ArrayCollection(
map(range(0, 10), fn () => Visit::forValidShortUrl(ShortUrl::createEmpty(), Visitor::emptyInstance())),
$shortUrl = ShortUrl::createFake()->setVisits(new ArrayCollection(
map(range(0, 10), fn () => Visit::forValidShortUrl(ShortUrl::createFake(), Visitor::emptyInstance())),
));
$this->shortCode = $shortUrl->getShortCode();

View File

@ -42,7 +42,7 @@ class ShortUrlTest extends TestCase
'The short code cannot be regenerated on ShortUrls where a custom slug was provided.',
];
yield 'already persisted' => [
ShortUrl::createEmpty()->setId('1'),
ShortUrl::createFake()->setId('1'),
'The short code can be regenerated only on new ShortUrls which have not been persisted yet.',
];
}
@ -64,7 +64,7 @@ class ShortUrlTest extends TestCase
public function provideValidShortUrls(): iterable
{
yield 'no custom slug' => [ShortUrl::createEmpty()];
yield 'no custom slug' => [ShortUrl::createFake()];
yield 'imported with custom slug' => [ShortUrl::fromImport(
new ImportedShlinkUrl(ImportSource::BITLY, 'longUrl', [], Chronos::now(), null, 'custom-slug', null),
true,

View File

@ -36,10 +36,10 @@ class ShortUrlListServiceTest extends TestCase
public function listedUrlsAreReturnedFromEntityManager(?ApiKey $apiKey): void
{
$list = [
ShortUrl::createEmpty(),
ShortUrl::createEmpty(),
ShortUrl::createEmpty(),
ShortUrl::createEmpty(),
ShortUrl::createFake(),
ShortUrl::createFake(),
ShortUrl::createFake(),
ShortUrl::createFake(),
];
$this->repo->expects($this->once())->method('findList')->willReturn($list);

View File

@ -38,7 +38,7 @@ class ShortUrlDataTransformerTest extends TestCase
$maxVisits = random_int(1, 1000);
$now = Chronos::now();
yield 'no metadata' => [ShortUrl::createEmpty(), [
yield 'no metadata' => [ShortUrl::createFake(), [
'validSince' => null,
'validUntil' => null,
'maxVisits' => null,

View File

@ -18,7 +18,7 @@ class VisitTest extends TestCase
*/
public function isProperlyJsonSerialized(string $userAgent, bool $expectedToBePotentialBot): void
{
$visit = Visit::forValidShortUrl(ShortUrl::createEmpty(), new Visitor($userAgent, 'some site', '1.2.3.4', ''));
$visit = Visit::forValidShortUrl(ShortUrl::createFake(), new Visitor($userAgent, 'some site', '1.2.3.4', ''));
self::assertEquals([
'referer' => 'some site',
@ -48,7 +48,7 @@ class VisitTest extends TestCase
public function addressIsAnonymizedWhenRequested(bool $anonymize, ?string $address, ?string $expectedAddress): void
{
$visit = Visit::forValidShortUrl(
ShortUrl::createEmpty(),
ShortUrl::createFake(),
new Visitor('Chrome', 'some site', $address, ''),
$anonymize,
);

View File

@ -86,7 +86,7 @@ class VisitsStatsHelperTest extends TestCase
$repo = $this->createMock(ShortUrlRepositoryInterface::class);
$repo->expects($this->once())->method('shortCodeIsInUse')->with($identifier, $spec)->willReturn(true);
$list = map(range(0, 1), fn () => Visit::forValidShortUrl(ShortUrl::createEmpty(), Visitor::emptyInstance()));
$list = map(range(0, 1), fn () => Visit::forValidShortUrl(ShortUrl::createFake(), Visitor::emptyInstance()));
$repo2 = $this->createMock(VisitRepository::class);
$repo2->method('findVisitsByShortCode')->with(
$identifier,
@ -146,7 +146,7 @@ class VisitsStatsHelperTest extends TestCase
$repo = $this->createMock(TagRepository::class);
$repo->expects($this->once())->method('tagExists')->with($tag, $apiKey)->willReturn(true);
$list = map(range(0, 1), fn () => Visit::forValidShortUrl(ShortUrl::createEmpty(), Visitor::emptyInstance()));
$list = map(range(0, 1), fn () => Visit::forValidShortUrl(ShortUrl::createFake(), Visitor::emptyInstance()));
$repo2 = $this->createMock(VisitRepository::class);
$repo2->method('findVisitsByTag')->with($tag, $this->isInstanceOf(VisitsListFiltering::class))->willReturn(
$list,
@ -187,7 +187,7 @@ class VisitsStatsHelperTest extends TestCase
$repo = $this->createMock(DomainRepository::class);
$repo->expects($this->once())->method('domainExists')->with($domain, $apiKey)->willReturn(true);
$list = map(range(0, 1), fn () => Visit::forValidShortUrl(ShortUrl::createEmpty(), Visitor::emptyInstance()));
$list = map(range(0, 1), fn () => Visit::forValidShortUrl(ShortUrl::createFake(), Visitor::emptyInstance()));
$repo2 = $this->createMock(VisitRepository::class);
$repo2->method('findVisitsByDomain')->with(
$domain,
@ -217,7 +217,7 @@ class VisitsStatsHelperTest extends TestCase
$repo = $this->createMock(DomainRepository::class);
$repo->expects($this->never())->method('domainExists');
$list = map(range(0, 1), fn () => Visit::forValidShortUrl(ShortUrl::createEmpty(), Visitor::emptyInstance()));
$list = map(range(0, 1), fn () => Visit::forValidShortUrl(ShortUrl::createFake(), Visitor::emptyInstance()));
$repo2 = $this->createMock(VisitRepository::class);
$repo2->method('findVisitsByDomain')->with(
'DEFAULT',
@ -259,7 +259,7 @@ class VisitsStatsHelperTest extends TestCase
/** @test */
public function nonOrphanVisitsAreReturnedAsExpected(): void
{
$list = map(range(0, 3), fn () => Visit::forValidShortUrl(ShortUrl::createEmpty(), Visitor::emptyInstance()));
$list = map(range(0, 3), fn () => Visit::forValidShortUrl(ShortUrl::createFake(), Visitor::emptyInstance()));
$repo = $this->createMock(VisitRepository::class);
$repo->expects($this->once())->method('countNonOrphanVisits')->with(
$this->isInstanceOf(VisitsCountFiltering::class),

View File

@ -58,7 +58,7 @@ class VisitsTrackerTest extends TestCase
public function provideTrackingMethodNames(): iterable
{
yield 'track' => ['track', [ShortUrl::createEmpty(), Visitor::emptyInstance()]];
yield 'track' => ['track', [ShortUrl::createFake(), Visitor::emptyInstance()]];
yield 'trackInvalidShortUrlVisit' => ['trackInvalidShortUrlVisit', [Visitor::emptyInstance()]];
yield 'trackBaseUrlVisit' => ['trackBaseUrlVisit', [Visitor::emptyInstance()]];
yield 'trackRegularNotFoundVisit' => ['trackRegularNotFoundVisit', [Visitor::emptyInstance()]];

View File

@ -37,7 +37,7 @@ class CreateShortUrlActionTest extends TestCase
public function properShortcodeConversionReturnsData(): void
{
$apiKey = ApiKey::create();
$shortUrl = ShortUrl::createEmpty();
$shortUrl = ShortUrl::createFake();
$expectedMeta = $body = [
'longUrl' => 'http://www.domain.com/foo/bar',
'validSince' => Chronos::now()->toAtomString(),

View File

@ -49,7 +49,7 @@ class EditShortUrlActionTest extends TestCase
->withParsedBody([
'maxVisits' => 5,
]);
$this->shortUrlService->expects($this->once())->method('updateShortUrl')->willReturn(ShortUrl::createEmpty());
$this->shortUrlService->expects($this->once())->method('updateShortUrl')->willReturn(ShortUrl::createFake());
$resp = $this->action->handle($request);

View File

@ -43,7 +43,7 @@ class SingleStepCreateShortUrlActionTest extends TestCase
])->withAttribute(ApiKey::class, $apiKey);
$this->urlShortener->expects($this->once())->method('shorten')->with(
ShortUrlCreation::fromRawData(['apiKey' => $apiKey, 'longUrl' => 'http://foobar.com']),
)->willReturn(ShortUrl::createEmpty());
)->willReturn(ShortUrl::createFake());
$resp = $this->action->handle($request);