Merge pull request #523 from acelaya-forks/feature/refactorings

Feature/refactorings

CHANGELOG.md

@@ -31,6 +31,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this
 * [#507](https://github.com/shlinkio/shlink/issues/507) Fixed error with too long original URLs by increasing size to the maximum value (2048) based on [the standard](https://stackoverflow.com/a/417184).
 * [#502](https://github.com/shlinkio/shlink/issues/502) Fixed error when providing the port as part of the domain on short URLs.
 * [#509](https://github.com/shlinkio/shlink/issues/509) Fixed error when trying to generate a QR code for a short URL which uses a custom domain.
+* [#522](https://github.com/shlinkio/shlink/issues/522) Highly mitigated errors thrown when lots of short URLs are created concurrently including new and existing tags.
 
 
 ## 1.19.0 - 2019-10-05

Dockerfile

@@ -3,7 +3,7 @@ LABEL maintainer="Alejandro Celaya <alejandro@alejandrocelaya.com>"
 
 ARG SHLINK_VERSION=1.18.1
 ENV SHLINK_VERSION ${SHLINK_VERSION}
-ENV SWOOLE_VERSION 4.3.3
+ENV SWOOLE_VERSION 4.4.8
 ENV COMPOSER_VERSION 1.9.0
 
 WORKDIR /etc/shlink

bin/test/run-api-tests.sh

@@ -1,7 +1,6 @@
 #!/usr/bin/env sh
-set -e
-
 export APP_ENV=test
+export DB_DRIVER=mysql
 
 # Try to stop server just in case it hanged in last execution
 vendor/bin/zend-expressive-swoole stop

composer.json

@@ -34,7 +34,7 @@
         "phly/phly-event-dispatcher": "^1.0",
         "predis/predis": "^1.1",
         "pugx/shortid-php": "^0.5",
-        "shlinkio/shlink-common": "^2.1",
+        "shlinkio/shlink-common": "^2.2.1",
         "shlinkio/shlink-event-dispatcher": "^1.0",
         "shlinkio/shlink-installer": "^2.1",
         "shlinkio/shlink-ip-geolocation": "^1.1",

config/test/test_config.global.php

@@ -66,6 +66,7 @@ return [
     ],
 
     'zend-expressive-swoole' => [
+        'enable_coroutine' => false,
         'swoole-http-server' => [
             'host' => $swooleTestingHost,
             'port' => $swooleTestingPort,
@@ -74,6 +75,7 @@ return [
                 'pid_file' => sys_get_temp_dir() . '/shlink-test-swoole.pid',
                 'worker_num' => 1,
                 'task_worker_num' => 1,
+                'enable_coroutine' => false,
             ],
         ],
     ],

data/infra/swoole.Dockerfile

@@ -4,6 +4,7 @@ MAINTAINER Alejandro Celaya <alejandro@alejandrocelaya.com>
 ENV APCU_VERSION 5.1.16
 ENV APCU_BC_VERSION 1.0.4
 ENV INOTIFY_VERSION 2.0.0
+ENV SWOOLE_VERSION 4.4.8
 
 RUN apk update
 
@@ -66,7 +67,7 @@ RUN rm /tmp/inotify.tar.gz
 # Install swoole
 # First line fixes an error when installing pecl extensions. Found in https://github.com/docker-library/php/issues/233
 RUN apk add --no-cache --virtual .phpize-deps $PHPIZE_DEPS && \
-    pecl install swoole && \
+    pecl install swoole-${SWOOLE_VERSION} && \
     docker-php-ext-enable swoole && \
     apk del .phpize-deps
 

docker-compose.yml

@@ -34,6 +34,7 @@ services:
             dockerfile: ./data/infra/swoole.Dockerfile
         ports:
             - "8080:8080"
+            - "9001:9001"
         volumes:
             - ./:/home/shlink
         links:

module/Core/src/Util/TagManagerTrait.php

@@ -8,6 +8,7 @@ use Doctrine\Common\Collections;
 use Doctrine\ORM\EntityManagerInterface;
 use Shlinkio\Shlink\Core\Entity\Tag;
 
+use function Functional\map;
 use function str_replace;
 use function strtolower;
 use function trim;
@@ -21,24 +22,18 @@ trait TagManagerTrait
      */
     private function tagNamesToEntities(EntityManagerInterface $em, array $tags): Collections\Collection
     {
-        $entities = [];
-        foreach ($tags as $tagName) {
+        $entities = map($tags, function (string $tagName) use ($em) {
             $tagName = $this->normalizeTagName($tagName);
-            $tag = $em->getRepository(Tag::class)->findOneBy(['name' => $tagName]) ?: new Tag($tagName);
+            $tag = $em->getRepository(Tag::class)->findOneBy(['name' => $tagName]) ?? new Tag($tagName);
             $em->persist($tag);
-            $entities[] = $tag;
-        }
+
+            return $tag;
+        });
 
         return new Collections\ArrayCollection($entities);
     }
 
-    /**
-     * Tag names are trimmed, lower cased and spaces are replaced by dashes
-     *
-     * @param string $tagName
-     * @return string
-     */
-    private function normalizeTagName($tagName): string
+    private function normalizeTagName(string $tagName): string
     {
         return str_replace(' ', '-', strtolower(trim($tagName)));
     }

module/Rest/test-api/Action/CreateShortUrlActionTest.php

@@ -50,10 +50,10 @@ class CreateShortUrlActionTest extends ApiTestCase
     /** @test */
     public function createsNewShortUrlWithTags(): void
     {
-        [$statusCode, $payload] = $this->createShortUrl(['tags' => ['foo', 'bar', 'baz']]);
+        [$statusCode, ['tags' => $tags]] = $this->createShortUrl(['tags' => ['foo', 'bar', 'baz']]);
 
         $this->assertEquals(self::STATUS_OK, $statusCode);
-        $this->assertEquals(['foo', 'bar', 'baz'], $payload['tags']);
+        $this->assertEquals(['foo', 'bar', 'baz'], $tags);
     }
 
     /**
@@ -76,7 +76,7 @@ class CreateShortUrlActionTest extends ApiTestCase
 
     public function provideMaxVisits(): array
     {
-        return map(range(1, 20), function (int $i) {
+        return map(range(10, 15), function (int $i) {
             return [$i];
         });
     }

module/Rest/test-api/Action/ListShortUrlsTest.php

@@ -40,7 +40,7 @@ class ListShortUrlsTest extends ApiTestCase
                             . '/acmailer-7-0-the-most-important-release-in-a-long-time/',
                         'dateCreated' => '2019-01-01T00:00:00+00:00',
                         'visitsCount' => 2,
-                        'tags' => ['foo', 'bar'],
+                        'tags' => ['bar', 'foo'],
                         'meta' => [
                             'validSince' => '2020-05-01T00:00:00+00:00',
                             'validUntil' => null,