Simplified transactional URL shortening

config/autoload/redirects.global.php

@@ -5,7 +5,7 @@ declare(strict_types=1);
 return [
 
     'not_found_redirects' => [
-        'invalid_short_url' => null, // Formerly url_shortener.not_found_short_url.redirect_to
+        'invalid_short_url' => null,
         'regular_404' => null,
         'base_url' => null,
     ],

module/CLI/src/Command/ShortUrl/GenerateShortUrlCommand.php

@@ -142,7 +142,7 @@ class GenerateShortUrlCommand extends Command
         $doValidateUrl = $this->doValidateUrl($input);
 
         try {
-            $shortUrl = $this->urlShortener->urlToShortCode($longUrl, $tags, ShortUrlMeta::fromRawData([
+            $shortUrl = $this->urlShortener->shorten($longUrl, $tags, ShortUrlMeta::fromRawData([
                 ShortUrlMetaInputFilter::VALID_SINCE => $input->getOption('validSince'),
                 ShortUrlMetaInputFilter::VALID_UNTIL => $input->getOption('validUntil'),
                 ShortUrlMetaInputFilter::CUSTOM_SLUG => $customSlug,

module/CLI/test/Command/ShortUrl/GenerateShortUrlCommandTest.php

@@ -44,7 +44,7 @@ class GenerateShortUrlCommandTest extends TestCase
     public function properShortCodeIsCreatedIfLongUrlIsCorrect(): void
     {
         $shortUrl = new ShortUrl('');
-        $urlToShortCode = $this->urlShortener->urlToShortCode(Argument::cetera())->willReturn($shortUrl);
+        $urlToShortCode = $this->urlShortener->shorten(Argument::cetera())->willReturn($shortUrl);
 
         $this->commandTester->execute([
             'longUrl' => 'http://domain.com/foo/bar',
@@ -61,7 +61,7 @@ class GenerateShortUrlCommandTest extends TestCase
     public function exceptionWhileParsingLongUrlOutputsError(): void
     {
         $url = 'http://domain.com/invalid';
-        $this->urlShortener->urlToShortCode(Argument::cetera())->willThrow(InvalidUrlException::fromUrl($url))
+        $this->urlShortener->shorten(Argument::cetera())->willThrow(InvalidUrlException::fromUrl($url))
                                                                ->shouldBeCalledOnce();
 
         $this->commandTester->execute(['longUrl' => $url]);
@@ -74,7 +74,7 @@ class GenerateShortUrlCommandTest extends TestCase
     /** @test */
     public function providingNonUniqueSlugOutputsError(): void
     {
-        $urlToShortCode = $this->urlShortener->urlToShortCode(Argument::cetera())->willThrow(
+        $urlToShortCode = $this->urlShortener->shorten(Argument::cetera())->willThrow(
             NonUniqueSlugException::fromSlug('my-slug'),
         );
 
@@ -90,7 +90,7 @@ class GenerateShortUrlCommandTest extends TestCase
     public function properlyProcessesProvidedTags(): void
     {
         $shortUrl = new ShortUrl('');
-        $urlToShortCode = $this->urlShortener->urlToShortCode(
+        $urlToShortCode = $this->urlShortener->shorten(
             Argument::type('string'),
             Argument::that(function (array $tags) {
                 Assert::assertEquals(['foo', 'bar', 'baz', 'boo', 'zar'], $tags);
@@ -117,7 +117,7 @@ class GenerateShortUrlCommandTest extends TestCase
     public function urlValidationHasExpectedValueBasedOnProvidedTags(array $options, ?bool $expectedValidateUrl): void
     {
         $shortUrl = new ShortUrl('');
-        $urlToShortCode = $this->urlShortener->urlToShortCode(
+        $urlToShortCode = $this->urlShortener->shorten(
             Argument::type('string'),
             Argument::type('array'),
             Argument::that(function (ShortUrlMeta $meta) use ($expectedValidateUrl) {

module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Entity.ShortUrl.php

@@ -8,6 +8,7 @@ use Doctrine\DBAL\Types\Types;
 use Doctrine\ORM\Mapping\Builder\ClassMetadataBuilder;
 use Doctrine\ORM\Mapping\ClassMetadata;
 use Shlinkio\Shlink\Common\Doctrine\Type\ChronosDateTimeType;
+use Shlinkio\Shlink\Rest\Entity\ApiKey;
 
 return static function (ClassMetadata $metadata, array $emConfig): void {
     $builder = new ClassMetadataBuilder($metadata);
@@ -78,5 +79,9 @@ return static function (ClassMetadata $metadata, array $emConfig): void {
             ->cascadePersist()
             ->build();
 
+    $builder->createManyToOne('authorApiKey', ApiKey::class)
+            ->addJoinColumn('author_api_key_id', 'id', true, false, 'SET NULL')
+            ->build();
+
     $builder->addUniqueConstraint(['short_code', 'domain_id'], 'unique_short_code_plus_domain');
 };

module/Core/src/Entity/ShortUrl.php

@@ -16,6 +16,7 @@ use Shlinkio\Shlink\Core\Model\ShortUrlEdit;
 use Shlinkio\Shlink\Core\Model\ShortUrlMeta;
 use Shlinkio\Shlink\Core\Validation\ShortUrlMetaInputFilter;
 use Shlinkio\Shlink\Importer\Model\ImportedShlinkUrl;
+use Shlinkio\Shlink\Rest\Entity\ApiKey;
 
 use function count;
 use function Shlinkio\Shlink\Core\generateRandomShortCode;
@@ -37,6 +38,7 @@ class ShortUrl extends AbstractEntity
     private int $shortCodeLength;
     private ?string $importSource = null;
     private ?string $importOriginalShortCode = null;
+    private ?ApiKey $authorApiKey = null;
 
     public function __construct(
         string $longUrl,

module/Core/src/Service/UrlShortener.php

@@ -43,7 +43,7 @@ class UrlShortener implements UrlShortenerInterface
      * @throws InvalidUrlException
      * @throws Throwable
      */
-    public function urlToShortCode(string $url, array $tags, ShortUrlMeta $meta): ShortUrl
+    public function shorten(string $url, array $tags, ShortUrlMeta $meta): ShortUrl
     {
         // First, check if a short URL exists for all provided params
         $existingShortUrl = $this->findExistingShortUrlIfExists($url, $tags, $meta);
@@ -52,25 +52,16 @@ class UrlShortener implements UrlShortenerInterface
         }
 
         $this->urlValidator->validateUrl($url, $meta->doValidateUrl());
-        $this->em->beginTransaction();
-        $shortUrl = new ShortUrl($url, $meta, $this->domainResolver);
-        $shortUrl->setTags($this->tagNamesToEntities($this->em, $tags));
 
-        try {
+        return $this->em->transactional(function () use ($url, $tags, $meta) {
+            $shortUrl = new ShortUrl($url, $meta, $this->domainResolver);
+            $shortUrl->setTags($this->tagNamesToEntities($this->em, $tags));
+
             $this->verifyShortCodeUniqueness($meta, $shortUrl);
             $this->em->persist($shortUrl);
-            $this->em->flush();
-            $this->em->commit();
-        } catch (Throwable $e) {
-            if ($this->em->getConnection()->isTransactionActive()) {
-                $this->em->rollback();
-                $this->em->close();
-            }
 
-            throw $e;
+            return $shortUrl;
-        }
+        });
-
-        return $shortUrl;
     }
 
     private function findExistingShortUrlIfExists(string $url, array $tags, ShortUrlMeta $meta): ?ShortUrl

module/Core/src/Service/UrlShortenerInterface.php

@@ -16,5 +16,5 @@ interface UrlShortenerInterface
      * @throws NonUniqueSlugException
      * @throws InvalidUrlException
      */
-    public function urlToShortCode(string $url, array $tags, ShortUrlMeta $meta): ShortUrl;
+    public function shorten(string $url, array $tags, ShortUrlMeta $meta): ShortUrl;
 }

module/Core/test/Service/UrlShortenerTest.php

@@ -8,7 +8,6 @@ use Cake\Chronos\Chronos;
 use Doctrine\Common\Collections\ArrayCollection;
 use Doctrine\DBAL\Connection;
 use Doctrine\ORM\EntityManagerInterface;
-use Doctrine\ORM\ORMException;
 use PHPUnit\Framework\TestCase;
 use Prophecy\Argument;
 use Prophecy\PhpUnit\ProphecyTrait;
@@ -42,16 +41,18 @@ class UrlShortenerTest extends TestCase
 
         $this->em = $this->prophesize(EntityManagerInterface::class);
         $conn = $this->prophesize(Connection::class);
-        $conn->isTransactionActive()->willReturn(false);
         $this->em->getConnection()->willReturn($conn->reveal());
-        $this->em->flush()->willReturn(null);
-        $this->em->commit()->willReturn(null);
-        $this->em->beginTransaction()->willReturn(null);
         $this->em->persist(Argument::any())->will(function ($arguments): void {
             /** @var ShortUrl $shortUrl */
             [$shortUrl] = $arguments;
             $shortUrl->setId('10');
         });
+        $this->em->transactional(Argument::type('callable'))->will(function (array $args) {
+            /** @var callable $callback */
+            [$callback] = $args;
+
+            return $callback();
+        });
         $repo = $this->prophesize(ShortUrlRepository::class);
         $repo->shortCodeIsInUse(Argument::cetera())->willReturn(false);
         $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal());
@@ -70,7 +71,7 @@ class UrlShortenerTest extends TestCase
     /** @test */
     public function urlIsProperlyShortened(): void
     {
-        $shortUrl = $this->urlShortener->urlToShortCode(
+        $shortUrl = $this->urlShortener->shorten(
             'http://foobar.com/12345/hello?foo=bar',
             [],
             ShortUrlMeta::createEmpty(),
@@ -87,32 +88,13 @@ class UrlShortenerTest extends TestCase
         $ensureUniqueness->shouldBeCalledOnce();
         $this->expectException(NonUniqueSlugException::class);
 
-        $this->urlShortener->urlToShortCode(
+        $this->urlShortener->shorten(
             'http://foobar.com/12345/hello?foo=bar',
             [],
             ShortUrlMeta::fromRawData(['customSlug' => 'custom-slug']),
         );
     }
 
-    /** @test */
-    public function transactionIsRolledBackAndExceptionRethrownWhenExceptionIsThrown(): void
-    {
-        $conn = $this->prophesize(Connection::class);
-        $conn->isTransactionActive()->willReturn(true);
-        $this->em->getConnection()->willReturn($conn->reveal());
-        $this->em->rollback()->shouldBeCalledOnce();
-        $this->em->close()->shouldBeCalledOnce();
-
-        $this->em->flush()->willThrow(new ORMException());
-
-        $this->expectException(ORMException::class);
-        $this->urlShortener->urlToShortCode(
-            'http://foobar.com/12345/hello?foo=bar',
-            [],
-            ShortUrlMeta::createEmpty(),
-        );
-    }
-
     /**
      * @test
      * @dataProvider provideExistingShortUrls
@@ -127,7 +109,7 @@ class UrlShortenerTest extends TestCase
         $findExisting = $repo->findOneMatching(Argument::cetera())->willReturn($expected);
         $getRepo = $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal());
 
-        $result = $this->urlShortener->urlToShortCode($url, $tags, $meta);
+        $result = $this->urlShortener->shorten($url, $tags, $meta);
 
         $findExisting->shouldHaveBeenCalledOnce();
         $getRepo->shouldHaveBeenCalledOnce();

module/Rest/src/Action/ShortUrl/AbstractCreateShortUrlAction.php

@@ -31,7 +31,7 @@ abstract class AbstractCreateShortUrlAction extends AbstractRestAction
         $tags = $shortUrlData->getTags();
         $shortUrlMeta = $shortUrlData->getMeta();
 
-        $shortUrl = $this->urlShortener->urlToShortCode($longUrl, $tags, $shortUrlMeta);
+        $shortUrl = $this->urlShortener->shorten($longUrl, $tags, $shortUrlMeta);
         $transformer = new ShortUrlDataTransformer($this->domainConfig);
 
         return new JsonResponse($transformer->transform($shortUrl));

module/Rest/test/Action/ShortUrl/CreateShortUrlActionTest.php

@@ -51,7 +51,7 @@ class CreateShortUrlActionTest extends TestCase
     public function properShortcodeConversionReturnsData(array $body, ShortUrlMeta $expectedMeta): void
     {
         $shortUrl = new ShortUrl('');
-        $shorten = $this->urlShortener->urlToShortCode(
+        $shorten = $this->urlShortener->shorten(
             Argument::type('string'),
             Argument::type('array'),
             $expectedMeta,
@@ -88,7 +88,7 @@ class CreateShortUrlActionTest extends TestCase
     public function anInvalidDomainReturnsError(string $domain): void
     {
         $shortUrl = new ShortUrl('');
-        $urlToShortCode = $this->urlShortener->urlToShortCode(Argument::cetera())->willReturn($shortUrl);
+        $urlToShortCode = $this->urlShortener->shorten(Argument::cetera())->willReturn($shortUrl);
 
         $request = (new ServerRequest())->withParsedBody([
             'longUrl' => 'http://www.domain.com/foo/bar',

module/Rest/test/Action/ShortUrl/SingleStepCreateShortUrlActionTest.php

@@ -72,7 +72,7 @@ class SingleStepCreateShortUrlActionTest extends TestCase
             'longUrl' => 'http://foobar.com',
         ]);
         $findApiKey = $this->apiKeyService->check('abc123')->willReturn(true);
-        $generateShortCode = $this->urlShortener->urlToShortCode(
+        $generateShortCode = $this->urlShortener->shorten(
             Argument::that(function (string $argument): string {
                 Assert::assertEquals('http://foobar.com', $argument);
                 return $argument;