Merge pull request #506 from acelaya-forks/feature/improved-shortcodes

Feature/improved shortcodes
This commit is contained in:
Alejandro Celaya 2019-10-11 12:51:24 +02:00 committed by GitHub
commit eb17eae781
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
22 changed files with 283 additions and 216 deletions

View file

@ -3,7 +3,6 @@ APP_ENV=
SECRET_KEY=
SHORTENED_URL_SCHEMA=
SHORTENED_URL_HOSTNAME=
SHORTCODE_CHARS=
# Database
DB_USER=

View file

@ -8,7 +8,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this
#### Added
* *Nothing*
* [#491](https://github.com/shlinkio/shlink/issues/491) Added improved short code generation logic.
Now, short codes are truly random, which removes the guessability factor existing in previous versions.
Generated short codes have 5 characters, and shlink makes sure they keep unique, while making it backwards-compatible.
#### Changed

View file

@ -33,6 +33,7 @@
"ocramius/proxy-manager": "~2.2.2",
"phly/phly-event-dispatcher": "^1.0",
"predis/predis": "^1.1",
"pugx/shortid-php": "^0.5",
"shlinkio/shlink-common": "^2.0",
"shlinkio/shlink-event-dispatcher": "^1.0",
"shlinkio/shlink-installer": "^2.1",
@ -77,7 +78,10 @@
"Shlinkio\\Shlink\\Rest\\": "module/Rest/src",
"Shlinkio\\Shlink\\Core\\": "module/Core/src",
"Shlinkio\\Shlink\\PreviewGenerator\\": "module/PreviewGenerator/src/"
}
},
"files": [
"module/Core/functions/functions.php"
]
},
"autoload-dev": {
"psr-4": {

View file

@ -2,8 +2,6 @@
declare(strict_types=1);
use Shlinkio\Shlink\Core\Options\UrlShortenerOptions;
use function Shlinkio\Shlink\Common\env;
return [
@ -13,7 +11,6 @@ return [
'schema' => env('SHORTENED_URL_SCHEMA', 'http'),
'hostname' => env('SHORTENED_URL_HOSTNAME'),
],
'shortcode_chars' => env('SHORTCODE_CHARS', UrlShortenerOptions::DEFAULT_CHARS),
'validate_url' => true,
'not_found_short_url' => [
'enable_redirection' => false,

View file

@ -92,7 +92,6 @@ This is the complete list of supported env vars:
* `SHORT_DOMAIN_HOST`: The custom short domain used for this shlink instance. For example **doma.in**.
* `SHORT_DOMAIN_SCHEMA`: Either **http** or **https**.
* `SHORTCODE_CHARS`: A charset to use when building short codes. Only needed when using more than one shlink instance ([Multi instance considerations](#multi-instance-considerations)).
* `DB_DRIVER`: **sqlite** (which is the default value), **mysql**, **maria** or **postgres**.
* `DB_NAME`: The database name to be used when using an external database driver. Defaults to **shlink**.
* `DB_USER`: The username credential to be used when using an external database driver.
@ -112,6 +111,8 @@ This is the complete list of supported env vars:
In the future, these redis servers could be used for other caching operations performed by shlink.
* `SHORTCODE_CHARS`: **Ignored when using Shlink 1.20 or newer**. A charset to use when building short codes. Only needed when using more than one shlink instance ([Multi instance considerations](#multi-instance-considerations)).
An example using all env vars could look like this:
```bash
@ -178,7 +179,13 @@ docker run --name shlink -p 8080:8080 -v ${PWD}/my/config/dir:/etc/shlink/config
These are some considerations to take into account when running multiple instances of shlink.
* The first time shlink is run, it generates a charset used to generate short codes, which is a shuffled base62 charset.
* Some operations performed by Shlink should never be run more than once at the same time (like creating the database for the first time, or downloading the GeoLite2 database). For this reason, Shlink uses a locking system.
However, these locks are locally scoped to each Shlink instance by default.
You can (and should) make the locks to be shared by all Shlink instances by using a redis server/cluster. Just define the `REDIS_SERVERS` env var with the list of servers.
* **Ignore this if using Shlink 1.20 or newer**. The first time shlink is run, it generates a charset used to generate short codes, which is a shuffled base62 charset.
If you are using several shlink instances, you will probably want all of them to use the same charset.
@ -186,12 +193,6 @@ These are some considerations to take into account when running multiple instanc
If you don't do this, each shlink instance will use a different charset. However this shouldn't be a problem in practice, since the chances to get a collision will be very low.
* Some operations performed by Shlink should never be run more than once at the same time (like creating the database for the first time, or downloading the GeoLite2 database). For this reason, Shlink uses a locking system.
However, these locks are locally scoped to each Shlink instance by default.
You can (and should) make the locks to be shared by all Shlink instances by using a redis server/cluster. Just define the `REDIS_SERVERS` env var with the list of servers.
## Versions
Versions of this image match the shlink version it contains.

View file

@ -32,17 +32,15 @@ $helper = new class {
'postgres' => '5432',
];
/** @var string */
private $charset;
/** @var string */
private $secretKey;
public function __construct()
{
[$this->charset, $this->secretKey] = $this->initShlinkKeys();
[, $this->secretKey] = $this->initShlinkSecretKey();
}
private function initShlinkKeys(): array
private function initShlinkSecretKey(): array
{
$keysFile = sprintf('%s/shlink.keys', sys_get_temp_dir());
if (file_exists($keysFile)) {
@ -50,29 +48,19 @@ $helper = new class {
}
$keys = [
env('SHORTCODE_CHARS', $this->generateShortcodeChars()),
env('SECRET_KEY', $this->generateSecretKey()),
'', // This was the SHORTCODE_CHARS. Kept as empty string for BC
env('SECRET_KEY', $this->generateSecretKey()), // Deprecated
];
file_put_contents($keysFile, implode(',', $keys));
return $keys;
}
private function generateShortcodeChars(): string
{
return str_shuffle(self::BASE62);
}
private function generateSecretKey(): string
{
return substr(str_shuffle(self::BASE62), 0, 32);
}
public function getShortcodeChars(): string
{
return $this->charset;
}
public function getSecretKey(): string
{
return $this->secretKey;
@ -137,7 +125,6 @@ return [
'schema' => env('SHORT_DOMAIN_SCHEMA', 'http'),
'hostname' => env('SHORT_DOMAIN_HOST', ''),
],
'shortcode_chars' => $helper->getShortcodeChars(),
'validate_url' => (bool) env('VALIDATE_URLS', true),
'not_found_short_url' => $helper->getNotFoundConfig(),
],

View file

@ -5,7 +5,6 @@ declare(strict_types=1);
namespace Shlinkio\Shlink\CLI\Command\Config;
use Shlinkio\Shlink\CLI\Util\ExitCodes;
use Shlinkio\Shlink\Core\Options\UrlShortenerOptions;
use Symfony\Component\Console\Command\Command;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Output\OutputInterface;
@ -18,6 +17,7 @@ use function str_shuffle;
class GenerateCharsetCommand extends Command
{
public const NAME = 'config:generate-charset';
private const DEFAULT_CHARS = '0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ';
protected function configure(): void
{
@ -26,14 +26,14 @@ class GenerateCharsetCommand extends Command
->setDescription(sprintf(
'[DEPRECATED] Generates a character set sample just by shuffling the default one, "%s". '
. 'Then it can be set in the SHORTCODE_CHARS environment variable',
UrlShortenerOptions::DEFAULT_CHARS
self::DEFAULT_CHARS
))
->setHelp('<fg=red;options=bold>This command is deprecated. Better leave shlink generate the charset.</>');
}
protected function execute(InputInterface $input, OutputInterface $output): ?int
{
$charSet = str_shuffle(UrlShortenerOptions::DEFAULT_CHARS);
$charSet = str_shuffle(self::DEFAULT_CHARS);
(new SymfonyStyle($input, $output))->success(sprintf('Character set: "%s"', $charSet));
return ExitCodes::EXIT_SUCCESS;
}

View file

@ -20,6 +20,11 @@ use Symfony\Component\Console\Tester\CommandTester;
class GenerateShortUrlCommandTest extends TestCase
{
private const DOMAIN_CONFIG = [
'schema' => 'http',
'hostname' => 'foo.com',
];
/** @var CommandTester */
private $commandTester;
/** @var ObjectProphecy */
@ -28,10 +33,7 @@ class GenerateShortUrlCommandTest extends TestCase
public function setUp(): void
{
$this->urlShortener = $this->prophesize(UrlShortener::class);
$command = new GenerateShortUrlCommand($this->urlShortener->reveal(), [
'schema' => 'http',
'hostname' => 'foo.com',
]);
$command = new GenerateShortUrlCommand($this->urlShortener->reveal(), self::DOMAIN_CONFIG);
$app = new Application();
$app->add($command);
$this->commandTester = new CommandTester($command);
@ -40,9 +42,8 @@ class GenerateShortUrlCommandTest extends TestCase
/** @test */
public function properShortCodeIsCreatedIfLongUrlIsCorrect(): void
{
$urlToShortCode = $this->urlShortener->urlToShortCode(Argument::cetera())->willReturn(
(new ShortUrl(''))->setShortCode('abc123')
);
$shortUrl = new ShortUrl('');
$urlToShortCode = $this->urlShortener->urlToShortCode(Argument::cetera())->willReturn($shortUrl);
$this->commandTester->execute([
'longUrl' => 'http://domain.com/foo/bar',
@ -51,7 +52,7 @@ class GenerateShortUrlCommandTest extends TestCase
$output = $this->commandTester->getDisplay();
$this->assertEquals(ExitCodes::EXIT_SUCCESS, $this->commandTester->getStatusCode());
$this->assertStringContainsString('http://foo.com/abc123', $output);
$this->assertStringContainsString($shortUrl->toString(self::DOMAIN_CONFIG), $output);
$urlToShortCode->shouldHaveBeenCalledOnce();
}
@ -86,6 +87,7 @@ class GenerateShortUrlCommandTest extends TestCase
/** @test */
public function properlyProcessesProvidedTags(): void
{
$shortUrl = new ShortUrl('');
$urlToShortCode = $this->urlShortener->urlToShortCode(
Argument::type(UriInterface::class),
Argument::that(function (array $tags) {
@ -93,7 +95,7 @@ class GenerateShortUrlCommandTest extends TestCase
return $tags;
}),
Argument::cetera()
)->willReturn((new ShortUrl(''))->setShortCode('abc123'));
)->willReturn($shortUrl);
$this->commandTester->execute([
'longUrl' => 'http://domain.com/foo/bar',
@ -102,7 +104,7 @@ class GenerateShortUrlCommandTest extends TestCase
$output = $this->commandTester->getDisplay();
$this->assertEquals(ExitCodes::EXIT_SUCCESS, $this->commandTester->getStatusCode());
$this->assertStringContainsString('http://foo.com/abc123', $output);
$this->assertStringContainsString($shortUrl->toString(self::DOMAIN_CONFIG), $output);
$urlToShortCode->shouldHaveBeenCalledOnce();
}
}

View file

@ -0,0 +1,18 @@
<?php
declare(strict_types=1);
namespace Shlinkio\Shlink\Core;
use PUGX\Shortid\Factory as ShortIdFactory;
function generateRandomShortCode(int $length = 5): string
{
static $shortIdFactory;
if ($shortIdFactory === null) {
$shortIdFactory = new ShortIdFactory();
}
$alphabet = '0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ';
return $shortIdFactory->generate($length, $alphabet)->serialize();
}

View file

@ -10,6 +10,7 @@ use Doctrine\Common\Collections\Collection;
use Shlinkio\Shlink\Common\Entity\AbstractEntity;
use Shlinkio\Shlink\Core\Domain\Resolver\DomainResolverInterface;
use Shlinkio\Shlink\Core\Domain\Resolver\SimpleDomainResolver;
use Shlinkio\Shlink\Core\Exception\ShortCodeCannotBeRegeneratedException;
use Shlinkio\Shlink\Core\Model\ShortUrlMeta;
use Zend\Diactoros\Uri;
@ -17,6 +18,7 @@ use function array_reduce;
use function count;
use function Functional\contains;
use function Functional\invoke;
use function Shlinkio\Shlink\Core\generateRandomShortCode;
class ShortUrl extends AbstractEntity
{
@ -38,6 +40,8 @@ class ShortUrl extends AbstractEntity
private $maxVisits;
/** @var Domain|null */
private $domain;
/** @var bool */
private $customSlugWasProvided;
public function __construct(
string $longUrl,
@ -53,7 +57,8 @@ class ShortUrl extends AbstractEntity
$this->validSince = $meta->getValidSince();
$this->validUntil = $meta->getValidUntil();
$this->maxVisits = $meta->getMaxVisits();
$this->shortCode = $meta->getCustomSlug() ?? ''; // TODO logic to calculate short code should be passed somehow
$this->customSlugWasProvided = $meta->hasCustomSlug();
$this->shortCode = $meta->getCustomSlug() ?? generateRandomShortCode();
$this->domain = ($domainResolver ?? new SimpleDomainResolver())->resolveDomain($meta->getDomain());
}
@ -67,13 +72,6 @@ class ShortUrl extends AbstractEntity
return $this->shortCode;
}
// TODO Short code is currently calculated based on the ID, so a setter is needed
public function setShortCode(string $shortCode): self
{
$this->shortCode = $shortCode;
return $this;
}
public function getDateCreated(): Chronos
{
return $this->dateCreated;
@ -109,6 +107,25 @@ class ShortUrl extends AbstractEntity
}
}
/**
* @throws ShortCodeCannotBeRegeneratedException
*/
public function regenerateShortCode(): self
{
// In ShortUrls where a custom slug was provided, do nothing
if ($this->customSlugWasProvided) {
throw ShortCodeCannotBeRegeneratedException::forShortUrlWithCustomSlug();
}
// The short code can be regenerated only on ShortUrl which have not been persisted yet
if ($this->id !== null) {
throw ShortCodeCannotBeRegeneratedException::forShortUrlAlreadyPersisted();
}
$this->shortCode = generateRandomShortCode();
return $this;
}
public function getValidSince(): ?Chronos
{
return $this->validSince;

View file

@ -0,0 +1,18 @@
<?php
declare(strict_types=1);
namespace Shlinkio\Shlink\Core\Exception;
class ShortCodeCannotBeRegeneratedException extends RuntimeException
{
public static function forShortUrlWithCustomSlug(): self
{
return new self('The short code cannot be regenerated on ShortUrls where a custom slug was provided.');
}
public static function forShortUrlAlreadyPersisted(): self
{
return new self('The short code can be regenerated only on new ShortUrls which have not been persisted yet.');
}
}

View file

@ -8,26 +8,12 @@ use Zend\Stdlib\AbstractOptions;
class UrlShortenerOptions extends AbstractOptions
{
public const DEFAULT_CHARS = '0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ';
// phpcs:disable
protected $__strictMode__ = false;
// phpcs:enable
private $shortcodeChars = self::DEFAULT_CHARS;
private $validateUrl = true;
public function getChars(): string
{
return $this->shortcodeChars;
}
protected function setShortcodeChars(string $shortcodeChars): self
{
$this->shortcodeChars = empty($shortcodeChars) ? self::DEFAULT_CHARS : $shortcodeChars;
return $this;
}
public function isUrlValidationEnabled(): bool
{
return $this->validateUrl;

View file

@ -155,7 +155,7 @@ DQL;
return $shortUrl !== null && ! $shortUrl->maxVisitsReached() ? $shortUrl : null;
}
public function slugIsInUse(string $slug, ?string $domain = null): bool
public function shortCodeIsInUse(string $slug, ?string $domain = null): bool
{
$qb = $this->getEntityManager()->createQueryBuilder();
$qb->select('COUNT(DISTINCT s.id)')

View file

@ -29,5 +29,5 @@ interface ShortUrlRepositoryInterface extends ObjectRepository
public function findOneByShortCode(string $shortCode, ?string $domain = null): ?ShortUrl;
public function slugIsInUse(string $slug, ?string $domain): bool;
public function shortCodeIsInUse(string $slug, ?string $domain): bool;
}

View file

@ -16,7 +16,6 @@ use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException;
use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException;
use Shlinkio\Shlink\Core\Exception\InvalidUrlException;
use Shlinkio\Shlink\Core\Exception\NonUniqueSlugException;
use Shlinkio\Shlink\Core\Exception\RuntimeException;
use Shlinkio\Shlink\Core\Model\ShortUrlMeta;
use Shlinkio\Shlink\Core\Options\UrlShortenerOptions;
use Shlinkio\Shlink\Core\Repository\ShortUrlRepository;
@ -24,17 +23,11 @@ use Shlinkio\Shlink\Core\Util\TagManagerTrait;
use Throwable;
use function array_reduce;
use function floor;
use function fmod;
use function preg_match;
use function strlen;
class UrlShortener implements UrlShortenerInterface
{
use TagManagerTrait;
private const ID_INCREMENT = 200000;
/** @var ClientInterface */
private $httpClient;
/** @var EntityManagerInterface */
@ -53,7 +46,7 @@ class UrlShortener implements UrlShortenerInterface
* @param string[] $tags
* @throws NonUniqueSlugException
* @throws InvalidUrlException
* @throws RuntimeException
* @throws Throwable
*/
public function urlToShortCode(UriInterface $url, array $tags, ShortUrlMeta $meta): ShortUrl
{
@ -69,36 +62,26 @@ class UrlShortener implements UrlShortenerInterface
if ($this->options->isUrlValidationEnabled()) {
$this->checkUrlExists($url);
}
$this->verifyCustomSlug($meta);
// Transactionally insert the short url, then generate the short code and finally update the short code
$this->em->beginTransaction();
$shortUrl = new ShortUrl($url, $meta, new PersistenceDomainResolver($this->em));
$shortUrl->setTags($this->tagNamesToEntities($this->em, $tags));
try {
$this->em->beginTransaction();
// First, create the short URL with an empty short code
$shortUrl = new ShortUrl($url, $meta, new PersistenceDomainResolver($this->em));
$this->verifyShortCodeUniqueness($meta, $shortUrl);
$this->em->persist($shortUrl);
$this->em->flush();
// Generate the short code and persist it if no custom slug was provided
if (! $meta->hasCustomSlug()) {
// TODO Somehow provide the logic to calculate the shortCode to avoid the need of a setter
$shortCode = $this->convertAutoincrementIdToShortCode((float) $shortUrl->getId());
$shortUrl->setShortCode($shortCode);
}
$shortUrl->setTags($this->tagNamesToEntities($this->em, $tags));
$this->em->flush();
$this->em->commit();
return $shortUrl;
} catch (Throwable $e) {
if ($this->em->getConnection()->isTransactionActive()) {
$this->em->rollback();
$this->em->close();
}
throw new RuntimeException('An error occurred while persisting the short URL', -1, $e);
throw $e;
}
return $shortUrl;
}
private function findExistingShortUrlIfExists(string $url, array $tags, ShortUrlMeta $meta): ?ShortUrl
@ -138,38 +121,23 @@ class UrlShortener implements UrlShortenerInterface
}
}
private function verifyCustomSlug(ShortUrlMeta $meta): void
private function verifyShortCodeUniqueness(ShortUrlMeta $meta, ShortUrl $shortUrlToBeCreated): void
{
if (! $meta->hasCustomSlug()) {
return;
}
$customSlug = $meta->getCustomSlug();
$shortCode = $shortUrlToBeCreated->getShortCode();
$domain = $meta->getDomain();
/** @var ShortUrlRepository $repo */
$repo = $this->em->getRepository(ShortUrl::class);
$shortUrlsCount = $repo->slugIsInUse($customSlug, $domain);
if ($shortUrlsCount > 0) {
throw NonUniqueSlugException::fromSlug($customSlug, $domain);
}
}
$otherShortUrlsExist = $repo->shortCodeIsInUse($shortCode, $domain);
private function convertAutoincrementIdToShortCode(float $id): string
{
$id += self::ID_INCREMENT; // Increment the Id so that the generated shortcode is not too short
$chars = $this->options->getChars();
$length = strlen($chars);
$code = '';
while ($id > 0) {
// Determine the value of the next higher character in the short code and prepend it
$code = $chars[(int) fmod($id, $length)] . $code;
$id = floor($id / $length);
if ($otherShortUrlsExist && $meta->hasCustomSlug()) {
throw NonUniqueSlugException::fromSlug($shortCode, $domain);
}
return $chars[(int) $id] . $code;
if ($otherShortUrlsExist) {
$shortUrlToBeCreated->regenerateShortCode();
$this->verifyShortCodeUniqueness($meta, $shortUrlToBeCreated);
}
}
/**
@ -178,13 +146,6 @@ class UrlShortener implements UrlShortenerInterface
*/
public function shortCodeToUrl(string $shortCode, ?string $domain = null): ShortUrl
{
$chars = $this->options->getChars();
// Validate short code format
if (! preg_match('|[' . $chars . ']+|', $shortCode)) {
throw InvalidShortCodeException::fromCharset($shortCode, $chars);
}
/** @var ShortUrlRepository $shortUrlRepo */
$shortUrlRepo = $this->em->getRepository(ShortUrl::class);
$shortUrl = $shortUrlRepo->findOneByShortCode($shortCode, $domain);

View file

@ -37,37 +37,41 @@ class ShortUrlRepositoryTest extends DatabaseTestCase
/** @test */
public function findOneByShortCodeReturnsProperData(): void
{
$regularOne = new ShortUrl('foo');
$regularOne->setShortCode('foo');
$regularOne = new ShortUrl('foo', ShortUrlMeta::createFromParams(null, null, 'foo'));
$this->getEntityManager()->persist($regularOne);
$notYetValid = new ShortUrl('bar', ShortUrlMeta::createFromParams(Chronos::now()->addMonth()));
$notYetValid->setShortCode('bar_very_long_text');
$notYetValid = new ShortUrl(
'bar',
ShortUrlMeta::createFromParams(Chronos::now()->addMonth(), null, 'bar_very_long_text')
);
$this->getEntityManager()->persist($notYetValid);
$expired = new ShortUrl('expired', ShortUrlMeta::createFromParams(null, Chronos::now()->subMonth()));
$expired->setShortCode('expired');
$expired = new ShortUrl('expired', ShortUrlMeta::createFromParams(null, Chronos::now()->subMonth(), 'expired'));
$this->getEntityManager()->persist($expired);
$allVisitsComplete = new ShortUrl('baz', ShortUrlMeta::createFromRawData(['maxVisits' => 3]));
$allVisitsComplete = new ShortUrl('baz', ShortUrlMeta::createFromRawData([
'maxVisits' => 3,
'customSlug' => 'baz',
]));
$visits = [];
for ($i = 0; $i < 3; $i++) {
$visit = new Visit($allVisitsComplete, Visitor::emptyInstance());
$this->getEntityManager()->persist($visit);
$visits[] = $visit;
}
$allVisitsComplete->setShortCode('baz')
->setVisits(new ArrayCollection($visits));
$allVisitsComplete->setVisits(new ArrayCollection($visits));
$this->getEntityManager()->persist($allVisitsComplete);
$withDomain = new ShortUrl('foo', ShortUrlMeta::createFromRawData(['domain' => 'example.com']));
$withDomain->setShortCode('domain-short-code');
$withDomain = new ShortUrl('foo', ShortUrlMeta::createFromRawData([
'domain' => 'example.com',
'customSlug' => 'domain-short-code',
]));
$this->getEntityManager()->persist($withDomain);
$withDomainDuplicatingRegular = new ShortUrl('foo_with_domain', ShortUrlMeta::createFromRawData([
'domain' => 'doma.in',
'customSlug' => 'foo',
]));
$withDomainDuplicatingRegular->setShortCode('foo');
$this->getEntityManager()->persist($withDomainDuplicatingRegular);
$this->getEntityManager()->flush();
@ -96,9 +100,7 @@ class ShortUrlRepositoryTest extends DatabaseTestCase
{
$count = 5;
for ($i = 0; $i < $count; $i++) {
$this->getEntityManager()->persist(
(new ShortUrl((string) $i))->setShortCode((string) $i)
);
$this->getEntityManager()->persist(new ShortUrl((string) $i));
}
$this->getEntityManager()->flush();
@ -112,19 +114,16 @@ class ShortUrlRepositoryTest extends DatabaseTestCase
$this->getEntityManager()->persist($tag);
$foo = new ShortUrl('foo');
$foo->setShortCode('foo')
->setTags(new ArrayCollection([$tag]));
$foo->setTags(new ArrayCollection([$tag]));
$this->getEntityManager()->persist($foo);
$bar = new ShortUrl('bar');
$visit = new Visit($bar, Visitor::emptyInstance());
$this->getEntityManager()->persist($visit);
$bar->setShortCode('bar_very_long_text')
->setVisits(new ArrayCollection([$visit]));
$bar->setVisits(new ArrayCollection([$visit]));
$this->getEntityManager()->persist($bar);
$foo2 = new ShortUrl('foo_2');
$foo2->setShortCode('foo_2');
$this->getEntityManager()->persist($foo2);
$this->getEntityManager()->flush();
@ -155,9 +154,7 @@ class ShortUrlRepositoryTest extends DatabaseTestCase
{
$urls = ['a', 'z', 'c', 'b'];
foreach ($urls as $url) {
$this->getEntityManager()->persist(
(new ShortUrl($url))->setShortCode($url)
);
$this->getEntityManager()->persist(new ShortUrl($url));
}
$this->getEntityManager()->flush();
@ -172,24 +169,24 @@ class ShortUrlRepositoryTest extends DatabaseTestCase
}
/** @test */
public function slugIsInUseLooksForShortUrlInProperSetOfTables(): void
public function shortCodeIsInUseLooksForShortUrlInProperSetOfTables(): void
{
$shortUrlWithoutDomain = (new ShortUrl('foo'))->setShortCode('my-cool-slug');
$shortUrlWithoutDomain = new ShortUrl('foo', ShortUrlMeta::createFromRawData(['customSlug' => 'my-cool-slug']));
$this->getEntityManager()->persist($shortUrlWithoutDomain);
$shortUrlWithDomain = (new ShortUrl(
$shortUrlWithDomain = new ShortUrl(
'foo',
ShortUrlMeta::createFromRawData(['domain' => 'doma.in'])
))->setShortCode('another-slug');
ShortUrlMeta::createFromRawData(['domain' => 'doma.in', 'customSlug' => 'another-slug'])
);
$this->getEntityManager()->persist($shortUrlWithDomain);
$this->getEntityManager()->flush();
$this->assertTrue($this->repo->slugIsInUse('my-cool-slug'));
$this->assertFalse($this->repo->slugIsInUse('my-cool-slug', 'doma.in'));
$this->assertFalse($this->repo->slugIsInUse('slug-not-in-use'));
$this->assertFalse($this->repo->slugIsInUse('another-slug'));
$this->assertFalse($this->repo->slugIsInUse('another-slug', 'example.com'));
$this->assertTrue($this->repo->slugIsInUse('another-slug', 'doma.in'));
$this->assertTrue($this->repo->shortCodeIsInUse('my-cool-slug'));
$this->assertFalse($this->repo->shortCodeIsInUse('my-cool-slug', 'doma.in'));
$this->assertFalse($this->repo->shortCodeIsInUse('slug-not-in-use'));
$this->assertFalse($this->repo->shortCodeIsInUse('another-slug'));
$this->assertFalse($this->repo->shortCodeIsInUse('another-slug', 'example.com'));
$this->assertTrue($this->repo->shortCodeIsInUse('another-slug', 'doma.in'));
}
}

View file

@ -0,0 +1,51 @@
<?php
declare(strict_types=1);
namespace ShlinkioTest\Shlink\Core\Entity;
use PHPUnit\Framework\TestCase;
use Shlinkio\Shlink\Core\Entity\ShortUrl;
use Shlinkio\Shlink\Core\Exception\ShortCodeCannotBeRegeneratedException;
use Shlinkio\Shlink\Core\Model\ShortUrlMeta;
class ShortUrlTest extends TestCase
{
/**
* @test
* @dataProvider provideInvalidShortUrls
*/
public function regenerateShortCodeThrowsExceptionIfStateIsInvalid(
ShortUrl $shortUrl,
string $expectedMessage
): void {
$this->expectException(ShortCodeCannotBeRegeneratedException::class);
$this->expectExceptionMessage($expectedMessage);
$shortUrl->regenerateShortCode();
}
public function provideInvalidShortUrls(): iterable
{
yield 'with custom slug' => [
new ShortUrl('', ShortUrlMeta::createFromRawData(['customSlug' => 'custom-slug'])),
'The short code cannot be regenerated on ShortUrls where a custom slug was provided.',
];
yield 'already persisted' => [
(new ShortUrl(''))->setId('1'),
'The short code can be regenerated only on new ShortUrls which have not been persisted yet.',
];
}
/** @test */
public function regenerateShortCodeProperlyChangesTheValueOnValidShortUrls(): void
{
$shortUrl = new ShortUrl('');
$firstShortCode = $shortUrl->getShortCode();
$shortUrl->regenerateShortCode();
$secondShortCode = $shortUrl->getShortCode();
$this->assertNotEquals($firstShortCode, $secondShortCode);
}
}

View file

@ -19,20 +19,21 @@ use Shlinkio\Shlink\Core\Service\ShortUrl\DeleteShortUrlService;
use function Functional\map;
use function range;
use function sprintf;
class DeleteShortUrlServiceTest extends TestCase
{
/** @var DeleteShortUrlService */
private $service;
/** @var ObjectProphecy */
private $em;
/** @var string */
private $shortCode;
public function setUp(): void
{
$shortUrl = (new ShortUrl(''))->setShortCode('abc123')
->setVisits(new ArrayCollection(map(range(0, 10), function () {
return new Visit(new ShortUrl(''), Visitor::emptyInstance());
})));
$shortUrl = (new ShortUrl(''))->setVisits(new ArrayCollection(map(range(0, 10), function () {
return new Visit(new ShortUrl(''), Visitor::emptyInstance());
})));
$this->shortCode = $shortUrl->getShortCode();
$this->em = $this->prophesize(EntityManagerInterface::class);
@ -42,55 +43,56 @@ class DeleteShortUrlServiceTest extends TestCase
}
/** @test */
public function deleteByShortCodeThrowsExceptionWhenThresholdIsReached()
public function deleteByShortCodeThrowsExceptionWhenThresholdIsReached(): void
{
$service = $this->createService();
$this->expectException(DeleteShortUrlException::class);
$this->expectExceptionMessage(
'Impossible to delete short URL with short code "abc123" since it has more than "5" visits.'
);
$this->expectExceptionMessage(sprintf(
'Impossible to delete short URL with short code "%s" since it has more than "5" visits.',
$this->shortCode
));
$service->deleteByShortCode('abc123');
$service->deleteByShortCode($this->shortCode);
}
/** @test */
public function deleteByShortCodeDeletesUrlWhenThresholdIsReachedButExplicitlyIgnored()
public function deleteByShortCodeDeletesUrlWhenThresholdIsReachedButExplicitlyIgnored(): void
{
$service = $this->createService();
$remove = $this->em->remove(Argument::type(ShortUrl::class))->willReturn(null);
$flush = $this->em->flush()->willReturn(null);
$service->deleteByShortCode('abc123', true);
$service->deleteByShortCode($this->shortCode, true);
$remove->shouldHaveBeenCalledOnce();
$flush->shouldHaveBeenCalledOnce();
}
/** @test */
public function deleteByShortCodeDeletesUrlWhenThresholdIsReachedButCheckIsDisabled()
public function deleteByShortCodeDeletesUrlWhenThresholdIsReachedButCheckIsDisabled(): void
{
$service = $this->createService(false);
$remove = $this->em->remove(Argument::type(ShortUrl::class))->willReturn(null);
$flush = $this->em->flush()->willReturn(null);
$service->deleteByShortCode('abc123');
$service->deleteByShortCode($this->shortCode);
$remove->shouldHaveBeenCalledOnce();
$flush->shouldHaveBeenCalledOnce();
}
/** @test */
public function deleteByShortCodeDeletesUrlWhenThresholdIsNotReached()
public function deleteByShortCodeDeletesUrlWhenThresholdIsNotReached(): void
{
$service = $this->createService(true, 100);
$remove = $this->em->remove(Argument::type(ShortUrl::class))->willReturn(null);
$flush = $this->em->flush()->willReturn(null);
$service->deleteByShortCode('abc123');
$service->deleteByShortCode($this->shortCode);
$remove->shouldHaveBeenCalledOnce();
$flush->shouldHaveBeenCalledOnce();

View file

@ -17,10 +17,8 @@ use Prophecy\Argument;
use Prophecy\Prophecy\ObjectProphecy;
use Shlinkio\Shlink\Core\Entity\ShortUrl;
use Shlinkio\Shlink\Core\Entity\Tag;
use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException;
use Shlinkio\Shlink\Core\Exception\InvalidUrlException;
use Shlinkio\Shlink\Core\Exception\NonUniqueSlugException;
use Shlinkio\Shlink\Core\Exception\RuntimeException;
use Shlinkio\Shlink\Core\Model\ShortUrlMeta;
use Shlinkio\Shlink\Core\Options\UrlShortenerOptions;
use Shlinkio\Shlink\Core\Repository\ShortUrlRepository;
@ -56,7 +54,7 @@ class UrlShortenerTest extends TestCase
$shortUrl->setId('10');
});
$repo = $this->prophesize(ShortUrlRepository::class);
$repo->slugIsInUse(Argument::cetera())->willReturn(false);
$repo->shortCodeIsInUse(Argument::cetera())->willReturn(false);
$this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal());
$this->setUrlShortener(false);
@ -74,17 +72,43 @@ class UrlShortenerTest extends TestCase
/** @test */
public function urlIsProperlyShortened(): void
{
// 10 -> 0Q1Y
$shortUrl = $this->urlShortener->urlToShortCode(
new Uri('http://foobar.com/12345/hello?foo=bar'),
[],
ShortUrlMeta::createEmpty()
);
$this->assertEquals('0Q1Y', $shortUrl->getShortCode());
$this->assertEquals('http://foobar.com/12345/hello?foo=bar', $shortUrl->getLongUrl());
}
/** @test */
public function exceptionIsThrownWhenOrmThrowsException(): void
public function shortCodeIsRegeneratedIfAlreadyInUse(): void
{
$callIndex = 0;
$expectedCalls = 3;
$repo = $this->prophesize(ShortUrlRepository::class);
$shortCodeIsInUse = $repo->shortCodeIsInUse(Argument::cetera())->will(
function () use (&$callIndex, $expectedCalls) {
$callIndex++;
return $callIndex < $expectedCalls;
}
);
$repo->findBy(Argument::cetera())->willReturn([]);
$getRepo = $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal());
$shortUrl = $this->urlShortener->urlToShortCode(
new Uri('http://foobar.com/12345/hello?foo=bar'),
[],
ShortUrlMeta::createEmpty()
);
$this->assertEquals('http://foobar.com/12345/hello?foo=bar', $shortUrl->getLongUrl());
$getRepo->shouldBeCalledTimes($expectedCalls);
$shortCodeIsInUse->shouldBeCalledTimes($expectedCalls);
}
/** @test */
public function transactionIsRolledBackAndExceptionRethrownWhenExceptionIsThrown(): void
{
$conn = $this->prophesize(Connection::class);
$conn->isTransactionActive()->willReturn(true);
@ -94,7 +118,7 @@ class UrlShortenerTest extends TestCase
$this->em->flush()->willThrow(new ORMException());
$this->expectException(RuntimeException::class);
$this->expectException(ORMException::class);
$this->urlShortener->urlToShortCode(
new Uri('http://foobar.com/12345/hello?foo=bar'),
[],
@ -123,11 +147,11 @@ class UrlShortenerTest extends TestCase
public function exceptionIsThrownWhenNonUniqueSlugIsProvided(): void
{
$repo = $this->prophesize(ShortUrlRepository::class);
$slugIsInUse = $repo->slugIsInUse('custom-slug', null)->willReturn(true);
$shortCodeIsInUse = $repo->shortCodeIsInUse('custom-slug', null)->willReturn(true);
$repo->findBy(Argument::cetera())->willReturn([]);
$getRepo = $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal());
$slugIsInUse->shouldBeCalledOnce();
$shortCodeIsInUse->shouldBeCalledOnce();
$getRepo->shouldBeCalled();
$this->expectException(NonUniqueSlugException::class);
@ -146,26 +170,24 @@ class UrlShortenerTest extends TestCase
string $url,
array $tags,
ShortUrlMeta $meta,
?ShortUrl $expected
ShortUrl $expected
): void {
$repo = $this->prophesize(ShortUrlRepository::class);
$findExisting = $repo->findBy(Argument::any())->willReturn($expected !== null ? [$expected] : []);
$findExisting = $repo->findBy(Argument::any())->willReturn([$expected]);
$getRepo = $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal());
$result = $this->urlShortener->urlToShortCode(new Uri($url), $tags, $meta);
$findExisting->shouldHaveBeenCalledOnce();
$getRepo->shouldHaveBeenCalledOnce();
if ($expected) {
$this->assertSame($expected, $result);
}
$this->em->persist(Argument::cetera())->shouldNotHaveBeenCalled();
$this->assertSame($expected, $result);
}
public function provideExistingShortUrls(): iterable
{
$url = 'http://foo.com';
yield [$url, [], ShortUrlMeta::createFromRawData(['findIfExists' => true]), null];
yield [$url, [], ShortUrlMeta::createFromRawData(['findIfExists' => true]), new ShortUrl($url)];
yield [$url, [], ShortUrlMeta::createFromRawData(
['findIfExists' => true, 'customSlug' => 'foo']
@ -194,6 +216,12 @@ class UrlShortenerTest extends TestCase
ShortUrlMeta::createFromRawData(['findIfExists' => true, 'validUntil' => Chronos::parse('2017-01-01')]),
new ShortUrl($url, ShortUrlMeta::createFromRawData(['validUntil' => Chronos::parse('2017-01-01')])),
];
yield [
$url,
[],
ShortUrlMeta::createFromRawData(['findIfExists' => true, 'domain' => 'example.com']),
new ShortUrl($url, ShortUrlMeta::createFromRawData(['domain' => 'example.com'])),
];
yield [
$url,
['baz', 'foo', 'bar'],
@ -243,9 +271,8 @@ class UrlShortenerTest extends TestCase
/** @test */
public function shortCodeIsProperlyParsed(): void
{
$shortCode = '12C1c';
$shortUrl = new ShortUrl('expected_url');
$shortUrl->setShortCode($shortCode);
$shortCode = $shortUrl->getShortCode();
$repo = $this->prophesize(ShortUrlRepositoryInterface::class);
$repo->findOneByShortCode($shortCode, null)->willReturn($shortUrl);
@ -254,11 +281,4 @@ class UrlShortenerTest extends TestCase
$url = $this->urlShortener->shortCodeToUrl($shortCode);
$this->assertSame($shortUrl, $url);
}
/** @test */
public function invalidCharSetThrowsException(): void
{
$this->expectException(InvalidShortCodeException::class);
$this->urlShortener->shortCodeToUrl('&/(');
}
}

View file

@ -90,7 +90,7 @@ class CreateShortUrlActionTest extends ApiTestCase
$this->assertEquals(self::STATUS_OK, $statusCode);
// Request to the short URL will return a 404 since ist' not valid yet
// Request to the short URL will return a 404 since it's not valid yet
$lastResp = $this->callShortUrl($shortCode);
$this->assertEquals(self::STATUS_NOT_FOUND, $lastResp->getStatusCode());
}

View file

@ -20,13 +20,15 @@ class ShortUrlsFixture extends AbstractFixture
*/
public function load(ObjectManager $manager): void
{
$abcShortUrl = $this->setShortUrlDate(new ShortUrl('https://shlink.io'))->setShortCode('abc123');
$abcShortUrl = $this->setShortUrlDate(
new ShortUrl('https://shlink.io', ShortUrlMeta::createFromRawData(['customSlug' => 'abc123']))
);
$manager->persist($abcShortUrl);
$defShortUrl = $this->setShortUrlDate(new ShortUrl(
'https://blog.alejandrocelaya.com/2017/12/09/acmailer-7-0-the-most-important-release-in-a-long-time/',
ShortUrlMeta::createFromParams(Chronos::parse('2020-05-01'))
))->setShortCode('def456');
ShortUrlMeta::createFromParams(Chronos::parse('2020-05-01'), null, 'def456')
));
$manager->persist($defShortUrl);
$customShortUrl = $this->setShortUrlDate(new ShortUrl(
@ -37,14 +39,14 @@ class ShortUrlsFixture extends AbstractFixture
$withDomainShortUrl = $this->setShortUrlDate(new ShortUrl(
'https://blog.alejandrocelaya.com/2019/04/27/considerations-to-properly-use-open-source-software-projects/',
ShortUrlMeta::createFromRawData(['domain' => 'example.com'])
))->setShortCode('ghi789');
ShortUrlMeta::createFromRawData(['domain' => 'example.com', 'customSlug' => 'ghi789'])
));
$manager->persist($withDomainShortUrl);
$withDomainAndSlugShortUrl = $this->setShortUrlDate(new ShortUrl(
'https://google.com',
ShortUrlMeta::createFromRawData(['domain' => 'some-domain.com'])
))->setShortCode('custom-with-domain');
ShortUrlMeta::createFromRawData(['domain' => 'some-domain.com', 'customSlug' => 'custom-with-domain'])
));
$manager->persist($withDomainAndSlugShortUrl);
$manager->flush();

View file

@ -22,6 +22,11 @@ use function strpos;
class CreateShortUrlActionTest extends TestCase
{
private const DOMAIN_CONFIG = [
'schema' => 'http',
'hostname' => 'foo.com',
];
/** @var CreateShortUrlAction */
private $action;
/** @var ObjectProphecy */
@ -30,10 +35,7 @@ class CreateShortUrlActionTest extends TestCase
public function setUp(): void
{
$this->urlShortener = $this->prophesize(UrlShortener::class);
$this->action = new CreateShortUrlAction($this->urlShortener->reveal(), [
'schema' => 'http',
'hostname' => 'foo.com',
]);
$this->action = new CreateShortUrlAction($this->urlShortener->reveal(), self::DOMAIN_CONFIG);
}
/** @test */
@ -46,10 +48,9 @@ class CreateShortUrlActionTest extends TestCase
/** @test */
public function properShortcodeConversionReturnsData(): void
{
$shortUrl = new ShortUrl('');
$this->urlShortener->urlToShortCode(Argument::type(Uri::class), Argument::type('array'), Argument::cetera())
->willReturn(
(new ShortUrl(''))->setShortCode('abc123')
)
->willReturn($shortUrl)
->shouldBeCalledOnce();
$request = (new ServerRequest())->withParsedBody([
@ -57,7 +58,7 @@ class CreateShortUrlActionTest extends TestCase
]);
$response = $this->action->handle($request);
$this->assertEquals(200, $response->getStatusCode());
$this->assertTrue(strpos($response->getBody()->getContents(), 'http://foo.com/abc123') > 0);
$this->assertTrue(strpos($response->getBody()->getContents(), $shortUrl->toString(self::DOMAIN_CONFIG)) > 0);
}
/** @test */