From 1937f3ea2268b83f031b4fea4deb936061b6cabf Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 26 Oct 2019 09:01:51 +0200 Subject: [PATCH 1/5] Trying to automatically persist tags --- .../Shlinkio.Shlink.Core.Entity.ShortUrl.php | 1 + module/Core/src/Util/TagManagerTrait.php | 17 ++++++++--------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Entity.ShortUrl.php b/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Entity.ShortUrl.php index 76e5ae7b..18fc6a0d 100644 --- a/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Entity.ShortUrl.php +++ b/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Entity.ShortUrl.php @@ -60,6 +60,7 @@ $builder->createManyToMany('tags', Entity\Tag::class) ->setJoinTable('short_urls_in_tags') ->addInverseJoinColumn('tag_id', 'id', true, false, 'CASCADE') ->addJoinColumn('short_url_id', 'id', true, false, 'CASCADE') + ->cascadePersist() ->build(); $builder->createManyToOne('domain', Entity\Domain::class) diff --git a/module/Core/src/Util/TagManagerTrait.php b/module/Core/src/Util/TagManagerTrait.php index 99c77c67..7d439229 100644 --- a/module/Core/src/Util/TagManagerTrait.php +++ b/module/Core/src/Util/TagManagerTrait.php @@ -24,21 +24,20 @@ trait TagManagerTrait $entities = []; foreach ($tags as $tagName) { $tagName = $this->normalizeTagName($tagName); - $tag = $em->getRepository(Tag::class)->findOneBy(['name' => $tagName]) ?: new Tag($tagName); - $em->persist($tag); + $tag = $em->getRepository(Tag::class)->findOneBy(['name' => $tagName]) ?? new Tag($tagName); + +// if (! $tag) { +// $tag = ; +// $em->persist($tag); +// } + $entities[] = $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))); } From 5361f33cc15df95dd7370ac4f71755fee5133b79 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 1 Nov 2019 09:52:56 +0100 Subject: [PATCH 2/5] Some more refactorings --- Dockerfile | 2 +- bin/test/run-api-tests.sh | 2 +- composer.json | 2 +- data/infra/swoole.Dockerfile | 3 ++- docker-compose.yml | 1 + .../Shlinkio.Shlink.Core.Entity.ShortUrl.php | 1 - module/Core/src/Util/TagManagerTrait.php | 14 +++++--------- .../test-api/Action/CreateShortUrlActionTest.php | 4 ++-- 8 files changed, 13 insertions(+), 16 deletions(-) diff --git a/Dockerfile b/Dockerfile index 4303b3dd..b1dbcd77 100644 --- a/Dockerfile +++ b/Dockerfile @@ -3,7 +3,7 @@ LABEL maintainer="Alejandro Celaya " 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 diff --git a/bin/test/run-api-tests.sh b/bin/test/run-api-tests.sh index 32339c19..eabc62d6 100755 --- a/bin/test/run-api-tests.sh +++ b/bin/test/run-api-tests.sh @@ -10,5 +10,5 @@ echo 'Starting server...' vendor/bin/zend-expressive-swoole start -d sleep 2 -vendor/bin/phpunit --order-by=random -c phpunit-api.xml --testdox --colors=always +APP_ENV=test DB_DRIVER=mysql vendor/bin/phpunit --order-by=random -c phpunit-api.xml --testdox --colors=always vendor/bin/zend-expressive-swoole stop diff --git a/composer.json b/composer.json index 8e2b1968..c3e071bc 100644 --- a/composer.json +++ b/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", "shlinkio/shlink-event-dispatcher": "^1.0", "shlinkio/shlink-installer": "^2.1", "shlinkio/shlink-ip-geolocation": "^1.1", diff --git a/data/infra/swoole.Dockerfile b/data/infra/swoole.Dockerfile index 87c16064..865ec5ba 100644 --- a/data/infra/swoole.Dockerfile +++ b/data/infra/swoole.Dockerfile @@ -4,6 +4,7 @@ MAINTAINER Alejandro Celaya 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 diff --git a/docker-compose.yml b/docker-compose.yml index 630ab955..811eec69 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -34,6 +34,7 @@ services: dockerfile: ./data/infra/swoole.Dockerfile ports: - "8080:8080" + - "9001:9001" volumes: - ./:/home/shlink links: diff --git a/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Entity.ShortUrl.php b/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Entity.ShortUrl.php index 18fc6a0d..76e5ae7b 100644 --- a/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Entity.ShortUrl.php +++ b/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Entity.ShortUrl.php @@ -60,7 +60,6 @@ $builder->createManyToMany('tags', Entity\Tag::class) ->setJoinTable('short_urls_in_tags') ->addInverseJoinColumn('tag_id', 'id', true, false, 'CASCADE') ->addJoinColumn('short_url_id', 'id', true, false, 'CASCADE') - ->cascadePersist() ->build(); $builder->createManyToOne('domain', Entity\Domain::class) diff --git a/module/Core/src/Util/TagManagerTrait.php b/module/Core/src/Util/TagManagerTrait.php index 7d439229..4fbfd288 100644 --- a/module/Core/src/Util/TagManagerTrait.php +++ b/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,18 +22,13 @@ trait TagManagerTrait */ private function tagNamesToEntities(EntityManagerInterface $em, array $tags): Collections\Collection { - $entities = []; - foreach ($tags as $tagName) { + $entities = map($tags, function (string $tagName) use ($em): Tag { $tagName = $this->normalizeTagName($tagName); $tag = $em->getRepository(Tag::class)->findOneBy(['name' => $tagName]) ?? new Tag($tagName); + $em->persist($tag); -// if (! $tag) { -// $tag = ; -// $em->persist($tag); -// } - - $entities[] = $tag; - } + return $tag; + }); return new Collections\ArrayCollection($entities); } diff --git a/module/Rest/test-api/Action/CreateShortUrlActionTest.php b/module/Rest/test-api/Action/CreateShortUrlActionTest.php index c3f7a4e4..9e691f7f 100644 --- a/module/Rest/test-api/Action/CreateShortUrlActionTest.php +++ b/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); } /** From ad906000c77eb5e4032492087e0eac1bd00caa8f Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 1 Nov 2019 10:04:25 +0100 Subject: [PATCH 3/5] Removed typehint making phpstan throw false positive --- module/Core/src/Util/TagManagerTrait.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/module/Core/src/Util/TagManagerTrait.php b/module/Core/src/Util/TagManagerTrait.php index 4fbfd288..f227afef 100644 --- a/module/Core/src/Util/TagManagerTrait.php +++ b/module/Core/src/Util/TagManagerTrait.php @@ -22,7 +22,7 @@ trait TagManagerTrait */ private function tagNamesToEntities(EntityManagerInterface $em, array $tags): Collections\Collection { - $entities = map($tags, function (string $tagName) use ($em): Tag { + $entities = map($tags, function (string $tagName) use ($em) { $tagName = $this->normalizeTagName($tagName); $tag = $em->getRepository(Tag::class)->findOneBy(['name' => $tagName]) ?? new Tag($tagName); $em->persist($tag); From 1f449e8ce1af64b45977d256368f9ff76f9cc719 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 1 Nov 2019 10:10:43 +0100 Subject: [PATCH 4/5] Disabled coroutines on swoole during API tests --- bin/test/run-api-tests.sh | 2 +- config/test/test_config.global.php | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/bin/test/run-api-tests.sh b/bin/test/run-api-tests.sh index eabc62d6..32339c19 100755 --- a/bin/test/run-api-tests.sh +++ b/bin/test/run-api-tests.sh @@ -10,5 +10,5 @@ echo 'Starting server...' vendor/bin/zend-expressive-swoole start -d sleep 2 -APP_ENV=test DB_DRIVER=mysql vendor/bin/phpunit --order-by=random -c phpunit-api.xml --testdox --colors=always +vendor/bin/phpunit --order-by=random -c phpunit-api.xml --testdox --colors=always vendor/bin/zend-expressive-swoole stop diff --git a/config/test/test_config.global.php b/config/test/test_config.global.php index 4e8ebdaf..a0a9aeb7 100644 --- a/config/test/test_config.global.php +++ b/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, ], ], ], From 39ac2efe2625b78c5b6ed77df4fb55842009b664 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 1 Nov 2019 17:16:56 +0100 Subject: [PATCH 5/5] Updated to latest shlink-common with bug fixes --- CHANGELOG.md | 1 + bin/test/run-api-tests.sh | 3 +-- composer.json | 2 +- module/Rest/test-api/Action/CreateShortUrlActionTest.php | 2 +- module/Rest/test-api/Action/ListShortUrlsTest.php | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f6e2556b..9800cfb0 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/bin/test/run-api-tests.sh b/bin/test/run-api-tests.sh index 32339c19..5a497716 100755 --- a/bin/test/run-api-tests.sh +++ b/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 diff --git a/composer.json b/composer.json index c3e071bc..54a3ea76 100644 --- a/composer.json +++ b/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.2", + "shlinkio/shlink-common": "^2.2.1", "shlinkio/shlink-event-dispatcher": "^1.0", "shlinkio/shlink-installer": "^2.1", "shlinkio/shlink-ip-geolocation": "^1.1", diff --git a/module/Rest/test-api/Action/CreateShortUrlActionTest.php b/module/Rest/test-api/Action/CreateShortUrlActionTest.php index 9e691f7f..c733be25 100644 --- a/module/Rest/test-api/Action/CreateShortUrlActionTest.php +++ b/module/Rest/test-api/Action/CreateShortUrlActionTest.php @@ -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]; }); } diff --git a/module/Rest/test-api/Action/ListShortUrlsTest.php b/module/Rest/test-api/Action/ListShortUrlsTest.php index 726f9f4d..d2171b3a 100644 --- a/module/Rest/test-api/Action/ListShortUrlsTest.php +++ b/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,