Merge pull request #387 from acelaya/feature/fix-check-exists

Feature/fix check exists

.travis.yml

@@ -9,6 +9,10 @@ php:
   - 7.2
   - 7.3
 
+services:
+  - mysql
+  - postgresql
+
 before_install:
   - echo 'extension = memcached.so' >> ~/.phpenv/versions/$(phpenv version-name)/etc/php.ini
   - echo 'extension = apcu.so' >> ~/.phpenv/versions/$(phpenv version-name)/etc/php.ini
@@ -19,8 +23,12 @@ install:
   - composer self-update
   - composer install --no-interaction
 
-script:
+before_script:
+  - mysql -e 'CREATE DATABASE shlink_test;'
+  - psql -c 'create database shlink_test;' -U postgres
   - mkdir build
+
+script:
   - composer ci
 
 after_success:

CHANGELOG.md

@@ -4,6 +4,31 @@ All notable changes to this project will be documented in this file.
 
 The format is based on [Keep a Changelog](https://keepachangelog.com), and this project adheres to [Semantic Versioning](https://semver.org).
 
+## 1.16.3 - 2019-03-30
+
+#### Added
+
+* *Nothing*
+
+#### Changed
+
+* [#153](https://github.com/shlinkio/shlink/issues/153) Updated to [doctrine/migrations](https://github.com/doctrine/migrations) version 2.0.0
+* [#376](https://github.com/shlinkio/shlink/issues/376) Allowed `visit:update-db` command to not return an error exit code even if download fails, by passing the `-i` flag.
+* [#341](https://github.com/shlinkio/shlink/issues/341) Improved database tests so that they are executed against all supported database engines.
+
+#### Deprecated
+
+* *Nothing*
+
+#### Removed
+
+* *Nothing*
+
+#### Fixed
+
+* [#382](https://github.com/shlinkio/shlink/issues/382) Fixed existing short URLs not properly checked when providing the `findIfExists` flag.
+
+
 ## 1.16.2 - 2019-03-05
 
 #### Added

composer.json

@@ -20,7 +20,7 @@
         "cakephp/chronos": "^1.2",
         "cocur/slugify": "^3.0",
         "doctrine/cache": "^1.6",
-        "doctrine/migrations": "^1.4",
+        "doctrine/migrations": "^2.0",
         "doctrine/orm": "^2.5",
         "endroid/qr-code": "^1.7",
         "firebase/php-jwt": "^4.0",
@@ -42,7 +42,7 @@
         "zendframework/zend-expressive-fastroute": "^3.0",
         "zendframework/zend-expressive-helpers": "^5.0",
         "zendframework/zend-expressive-platesrenderer": "^2.0",
-        "zendframework/zend-expressive-swoole": "^2.2",
+        "zendframework/zend-expressive-swoole": "^2.4",
         "zendframework/zend-i18n": "^2.7",
         "zendframework/zend-inputfilter": "^2.8",
         "zendframework/zend-paginator": "^2.6",
@@ -55,7 +55,7 @@
         "filp/whoops": "^2.0",
         "infection/infection": "^0.12.2",
         "phpstan/phpstan": "^0.11.2",
-        "phpunit/phpcov": "^6.0@dev || ^5.0",
+        "phpunit/phpcov": "^6.0 || ^5.0",
         "phpunit/phpunit": "^8.0 || ^7.5",
         "roave/security-advisories": "dev-master",
         "shlinkio/php-coding-standard": "~1.1.0",
@@ -110,11 +110,15 @@
         "test:ci": [
             "@test:unit:ci",
             "@test:db",
+            "@test:db:mysql",
+            "@test:db:postgres",
             "@test:api"
         ],
         "test:unit": "phpdbg -qrr vendor/bin/phpunit --order-by=random --coverage-php build/coverage-unit.cov --testdox",
         "test:unit:ci": "phpdbg -qrr vendor/bin/phpunit --order-by=random --coverage-php build/coverage-unit.cov --coverage-clover=build/clover.xml --coverage-xml=build/coverage-xml --log-junit=build/phpunit.junit.xml --testdox",
         "test:db": "APP_ENV=test phpdbg -qrr vendor/bin/phpunit --order-by=random -c phpunit-db.xml --coverage-php build/coverage-db.cov --testdox",
+        "test:db:mysql": "DB_DRIVER=mysql composer test:db",
+        "test:db:postgres": "DB_DRIVER=postgres composer test:db",
         "test:api": "bin/test/run-api-tests.sh",
 
         "test:pretty": [
@@ -141,7 +145,9 @@
         "test:ci": "<fg=blue;options=bold>Runs all test suites, generating all needed reports and logs for CI envs</>",
         "test:unit": "<fg=blue;options=bold>Runs unit test suites</>",
         "test:unit:ci": "<fg=blue;options=bold>Runs unit test suites, generating all needed reports and logs for CI envs</>",
-        "test:db": "<fg=blue;options=bold>Runs database test suites (covering entity repositories)</>",
+        "test:db": "<fg=blue;options=bold>Runs database test suites on a SQLite database</>",
+        "test:db:mysql": "<fg=blue;options=bold>Runs database test suites on a MySQL database</>",
+        "test:db:postgres": "<fg=blue;options=bold>Runs database test suites on a PostgreSQL database</>",
         "test:api": "<fg=blue;options=bold>Runs API test suites</>",
         "test:pretty": "<fg=blue;options=bold>Runs all test suites and generates an HTML code coverage report</>",
         "test:unit:pretty": "<fg=blue;options=bold>Runs unit test suites and generates an HTML code coverage report</>",

config/autoload/swoole.local.php.dist

@@ -0,0 +1,21 @@
+<?php
+declare(strict_types=1);
+
+use Zend\Expressive\Swoole\HotCodeReload\FileWatcher\InotifyFileWatcher;
+use Zend\ServiceManager\Factory\InvokableFactory;
+
+return [
+
+    'zend-expressive-swoole' => [
+        'hot-code-reload' => [
+            'enable' => true,
+        ],
+    ],
+
+    'dependencies' => [
+        'factories' => [
+            InotifyFileWatcher::class => InvokableFactory::class,
+        ],
+    ],
+
+];

config/test/bootstrap_api_tests.php

@@ -20,7 +20,7 @@ $testHelper = $container->get(TestHelper::class);
 $config = $container->get('config');
 $em = $container->get(EntityManager::class);
 
-$testHelper->createTestDb($config['entity_manager']['connection']['path']);
+$testHelper->createTestDb();
 ApiTest\ApiTestCase::setApiClient($container->get('shlink_test_api_client'));
 ApiTest\ApiTestCase::setSeedFixturesCallback(function () use ($testHelper, $em, $config) {
     $testHelper->seedFixtures($em, $config['data_fixtures'] ?? []);

config/test/bootstrap_db_tests.php

@@ -15,7 +15,5 @@ if (! file_exists('.env')) {
 
 /** @var ContainerInterface $container */
 $container = require __DIR__ . '/../container.php';
-$config = $container->get('config');
+$container->get(TestHelper::class)->createTestDb();
-
-$container->get(TestHelper::class)->createTestDb($config['entity_manager']['connection']['path']);
 DbTest\DatabaseTestCase::setEntityManager($container->get('em'));

config/test/test_config.global.php

@@ -4,15 +4,53 @@ declare(strict_types=1);
 namespace ShlinkioTest\Shlink;
 
 use GuzzleHttp\Client;
+use PDO;
 use Zend\ConfigAggregator\ConfigAggregator;
 use Zend\ServiceManager\Factory\InvokableFactory;
 
+use function Shlinkio\Shlink\Common\env;
 use function sprintf;
 use function sys_get_temp_dir;
 
 $swooleTestingHost = '127.0.0.1';
 $swooleTestingPort = 9999;
 
+$buildDbConnection = function () {
+    $driver = env('DB_DRIVER', 'sqlite');
+    $isCi = env('TRAVIS', false);
+
+    switch ($driver) {
+        case 'sqlite':
+            return [
+                'driver' => 'pdo_sqlite',
+                'path' => sys_get_temp_dir() . '/shlink-tests.db',
+            ];
+        case 'mysql':
+            return [
+                'driver' => 'pdo_mysql',
+                'host' => $isCi ? '127.0.0.1' : 'shlink_db',
+                'user' => 'root',
+                'password' => $isCi ? '' : 'root',
+                'dbname' => 'shlink_test',
+                'charset' => 'utf8',
+                'driverOptions' => [
+                    PDO::MYSQL_ATTR_INIT_COMMAND => 'SET NAMES utf8',
+                ],
+            ];
+        case 'postgres':
+            return [
+                'driver' => 'pdo_pgsql',
+                'host' => $isCi ? '127.0.0.1' : 'shlink_db_postgres',
+                'user' => 'postgres',
+                'password' => $isCi ? '' : 'root',
+                'dbname' => 'shlink_test',
+                'charset' => 'utf8',
+            ];
+        default:
+            return [];
+    }
+};
+
 return [
 
     'debug' => true,
@@ -49,11 +87,7 @@ return [
     ],
 
     'entity_manager' => [
-        'connection' => [
+        'connection' => $buildDbConnection(),
-            'driver' => 'pdo_sqlite',
-             'path' => sys_get_temp_dir() . '/shlink-tests.db',
-//            'path' => __DIR__ . '/../../data/shlink-tests.db',
-        ],
     ],
 
     'data_fixtures' => [

data/infra/swoole.Dockerfile

@@ -5,6 +5,7 @@ ENV PREDIS_VERSION 4.2.0
 ENV MEMCACHED_VERSION 3.1.3
 ENV APCU_VERSION 5.1.16
 ENV APCU_BC_VERSION 1.0.4
+ENV INOTIFY_VERSION 2.0.0
 
 RUN apk update
 
@@ -76,6 +77,16 @@ RUN rm /tmp/apcu_bc.tar.gz
 RUN rm /usr/local/etc/php/conf.d/docker-php-ext-apcu.ini
 RUN echo extension=apcu.so > /usr/local/etc/php/conf.d/20-php-ext-apcu.ini
 
+# Install inotify extension
+ADD https://pecl.php.net/get/inotify-$INOTIFY_VERSION.tgz /tmp/inotify.tar.gz
+RUN mkdir -p /usr/src/php/ext/inotify\
+  && tar xf /tmp/inotify.tar.gz -C /usr/src/php/ext/inotify --strip-components=1
+# configure and install
+RUN docker-php-ext-configure inotify\
+  && docker-php-ext-install inotify
+# cleanup
+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 && \

data/migrations/Version20160819142757.php

@@ -3,21 +3,24 @@ declare(strict_types=1);
 
 namespace ShlinkMigrations;
 
-use Doctrine\DBAL\Migrations\AbstractMigration;
+use Doctrine\DBAL\DBALException;
 use Doctrine\DBAL\Schema\Schema;
+use Doctrine\DBAL\Schema\SchemaException;
+use Doctrine\Migrations\AbstractMigration;
 
 /**
  * Auto-generated Migration: Please modify to your needs!
  */
 class Version20160819142757 extends AbstractMigration
 {
-    const MYSQL = 'mysql';
+    private const MYSQL = 'mysql';
-    const SQLITE = 'sqlite';
+    private const SQLITE = 'sqlite';
 
     /**
-     * @param Schema $schema
+     * @throws DBALException
+     * @throws SchemaException
      */
-    public function up(Schema $schema)
+    public function up(Schema $schema): void
     {
         $db = $this->connection->getDatabasePlatform()->getName();
         $table = $schema->getTable('short_urls');
@@ -31,9 +34,9 @@ class Version20160819142757 extends AbstractMigration
     }
 
     /**
-     * @param Schema $schema
+     * @throws DBALException
      */
-    public function down(Schema $schema)
+    public function down(Schema $schema): void
     {
         $db = $this->connection->getDatabasePlatform()->getName();
     }

data/migrations/Version20160820191203.php

@@ -3,19 +3,16 @@ declare(strict_types=1);
 
 namespace ShlinkMigrations;
 
-use Doctrine\DBAL\Migrations\AbstractMigration;
 use Doctrine\DBAL\Schema\Schema;
 use Doctrine\DBAL\Types\Type;
+use Doctrine\Migrations\AbstractMigration;
 
 /**
  * Auto-generated Migration: Please modify to your needs!
  */
 class Version20160820191203 extends AbstractMigration
 {
-    /**
+    public function up(Schema $schema): void
-     * @param Schema $schema
-     */
-    public function up(Schema $schema)
     {
         // Check if the tables already exist
         $tables = $schema->getTables();
@@ -29,7 +26,7 @@ class Version20160820191203 extends AbstractMigration
         $this->createShortUrlsInTagsTable($schema);
     }
 
-    protected function createTagsTable(Schema $schema)
+    private function createTagsTable(Schema $schema): void
     {
         $table = $schema->createTable('tags');
         $table->addColumn('id', Type::BIGINT, [
@@ -46,7 +43,7 @@ class Version20160820191203 extends AbstractMigration
         $table->setPrimaryKey(['id']);
     }
 
-    protected function createShortUrlsInTagsTable(Schema $schema)
+    private function createShortUrlsInTagsTable(Schema $schema): void
     {
         $table = $schema->createTable('short_urls_in_tags');
         $table->addColumn('short_url_id', Type::BIGINT, [
@@ -70,10 +67,7 @@ class Version20160820191203 extends AbstractMigration
         $table->setPrimaryKey(['short_url_id', 'tag_id']);
     }
 
-    /**
+    public function down(Schema $schema): void
-     * @param Schema $schema
-     */
-    public function down(Schema $schema)
     {
         $schema->dropTable('short_urls_in_tags');
         $schema->dropTable('tags');

data/migrations/Version20171021093246.php

@@ -3,10 +3,10 @@ declare(strict_types=1);
 
 namespace ShlinkMigrations;
 
-use Doctrine\DBAL\Migrations\AbstractMigration;
 use Doctrine\DBAL\Schema\Schema;
 use Doctrine\DBAL\Schema\SchemaException;
 use Doctrine\DBAL\Types\Type;
+use Doctrine\Migrations\AbstractMigration;
 
 /**
  * Auto-generated Migration: Please modify to your needs!
@@ -14,10 +14,9 @@ use Doctrine\DBAL\Types\Type;
 class Version20171021093246 extends AbstractMigration
 {
     /**
-     * @param Schema $schema
      * @throws SchemaException
      */
-    public function up(Schema $schema)
+    public function up(Schema $schema): void
     {
         $shortUrls = $schema->getTable('short_urls');
         if ($shortUrls->hasColumn('valid_since')) {
@@ -33,10 +32,9 @@ class Version20171021093246 extends AbstractMigration
     }
 
     /**
-     * @param Schema $schema
      * @throws SchemaException
      */
-    public function down(Schema $schema)
+    public function down(Schema $schema): void
     {
         $shortUrls = $schema->getTable('short_urls');
         if (! $shortUrls->hasColumn('valid_since')) {

data/migrations/Version20171022064541.php

@@ -3,10 +3,10 @@ declare(strict_types=1);
 
 namespace ShlinkMigrations;
 
-use Doctrine\DBAL\Migrations\AbstractMigration;
 use Doctrine\DBAL\Schema\Schema;
 use Doctrine\DBAL\Schema\SchemaException;
 use Doctrine\DBAL\Types\Type;
+use Doctrine\Migrations\AbstractMigration;
 
 /**
  * Auto-generated Migration: Please modify to your needs!
@@ -14,10 +14,9 @@ use Doctrine\DBAL\Types\Type;
 class Version20171022064541 extends AbstractMigration
 {
     /**
-     * @param Schema $schema
      * @throws SchemaException
      */
-    public function up(Schema $schema)
+    public function up(Schema $schema): void
     {
         $shortUrls = $schema->getTable('short_urls');
         if ($shortUrls->hasColumn('max_visits')) {
@@ -31,10 +30,9 @@ class Version20171022064541 extends AbstractMigration
     }
 
     /**
-     * @param Schema $schema
      * @throws SchemaException
      */
-    public function down(Schema $schema)
+    public function down(Schema $schema): void
     {
         $shortUrls = $schema->getTable('short_urls');
         if (! $shortUrls->hasColumn('max_visits')) {

module/CLI/src/Command/Visit/UpdateDbCommand.php

@@ -9,9 +9,12 @@ use Shlinkio\Shlink\Common\IpGeolocation\GeoLite2\DbUpdaterInterface;
 use Symfony\Component\Console\Command\Command;
 use Symfony\Component\Console\Helper\ProgressBar;
 use Symfony\Component\Console\Input\InputInterface;
+use Symfony\Component\Console\Input\InputOption;
 use Symfony\Component\Console\Output\OutputInterface;
 use Symfony\Component\Console\Style\SymfonyStyle;
 
+use function sprintf;
+
 class UpdateDbCommand extends Command
 {
     public const NAME = 'visit:update-db';
@@ -33,6 +36,12 @@ class UpdateDbCommand extends Command
             ->setHelp(
                 'The GeoLite2 database is updated first Tuesday every month, so this command should be ideally run '
                 . 'every first Wednesday'
+            )
+            ->addOption(
+                'ignoreErrors',
+                'i',
+                InputOption::VALUE_NONE,
+                'Makes the command success even iof the update fails.'
             );
     }
 
@@ -49,19 +58,32 @@ class UpdateDbCommand extends Command
             });
 
             $progressBar->finish();
-            $io->writeln('');
+            $io->newLine();
 
             $io->success('GeoLite2 database properly updated');
             return ExitCodes::EXIT_SUCCESS;
         } catch (RuntimeException $e) {
             $progressBar->finish();
-            $io->writeln('');
+            $io->newLine();
 
-            $io->error('An error occurred while updating GeoLite2 database');
+            return $this->handleError($e, $io, $input);
-            if ($io->isVerbose()) {
-                $this->getApplication()->renderException($e, $output);
-            }
-            return ExitCodes::EXIT_FAILURE;
         }
     }
+
+    private function handleError(RuntimeException $e, SymfonyStyle $io, InputInterface $input): int
+    {
+        $ignoreErrors = $input->getOption('ignoreErrors');
+        $baseErrorMsg = 'An error occurred while updating GeoLite2 database';
+
+        if ($ignoreErrors) {
+            $io->warning(sprintf('%s, but it was ignored', $baseErrorMsg));
+            return ExitCodes::EXIT_SUCCESS;
+        }
+
+        $io->error($baseErrorMsg);
+        if ($io->isVerbose()) {
+            $this->getApplication()->renderException($e, $io);
+        }
+        return ExitCodes::EXIT_FAILURE;
+    }
 }

module/CLI/test/Command/Visit/UpdateDbCommandTest.php

@@ -7,6 +7,7 @@ use PHPUnit\Framework\TestCase;
 use Prophecy\Argument;
 use Prophecy\Prophecy\ObjectProphecy;
 use Shlinkio\Shlink\CLI\Command\Visit\UpdateDbCommand;
+use Shlinkio\Shlink\CLI\Util\ExitCodes;
 use Shlinkio\Shlink\Common\Exception\RuntimeException;
 use Shlinkio\Shlink\Common\IpGeolocation\GeoLite2\DbUpdaterInterface;
 use Symfony\Component\Console\Application;
@@ -31,27 +32,45 @@ class UpdateDbCommandTest extends TestCase
     }
 
     /** @test */
-    public function successMessageIsPrintedIfEverythingWorks()
+    public function successMessageIsPrintedIfEverythingWorks(): void
     {
         $download = $this->dbUpdater->downloadFreshCopy(Argument::type('callable'))->will(function () {
         });
 
         $this->commandTester->execute([]);
         $output = $this->commandTester->getDisplay();
+        $exitCode = $this->commandTester->getStatusCode();
 
         $this->assertStringContainsString('GeoLite2 database properly updated', $output);
+        $this->assertEquals(ExitCodes::EXIT_SUCCESS, $exitCode);
         $download->shouldHaveBeenCalledOnce();
     }
 
     /** @test */
-    public function errorMessageIsPrintedIfAnExceptionIsThrown()
+    public function errorMessageIsPrintedIfAnExceptionIsThrown(): void
     {
         $download = $this->dbUpdater->downloadFreshCopy(Argument::type('callable'))->willThrow(RuntimeException::class);
 
         $this->commandTester->execute([]);
         $output = $this->commandTester->getDisplay();
+        $exitCode = $this->commandTester->getStatusCode();
 
         $this->assertStringContainsString('An error occurred while updating GeoLite2 database', $output);
+        $this->assertEquals(ExitCodes::EXIT_FAILURE, $exitCode);
+        $download->shouldHaveBeenCalledOnce();
+    }
+
+    /** @test */
+    public function warningMessageIsPrintedIfAnExceptionIsThrownAndErrorsAreIgnored(): void
+    {
+        $download = $this->dbUpdater->downloadFreshCopy(Argument::type('callable'))->willThrow(RuntimeException::class);
+
+        $this->commandTester->execute(['--ignoreErrors' => true]);
+        $output = $this->commandTester->getDisplay();
+        $exitCode = $this->commandTester->getStatusCode();
+
+        $this->assertStringContainsString('ignored', $output);
+        $this->assertEquals(ExitCodes::EXIT_SUCCESS, $exitCode);
         $download->shouldHaveBeenCalledOnce();
     }
 }

module/Common/test-db/TestHelper.php

@@ -9,16 +9,13 @@ use Doctrine\Common\DataFixtures\Purger\ORMPurger;
 use Doctrine\ORM\EntityManagerInterface;
 use Symfony\Component\Process\Process;
 
-use function file_exists;
-use function unlink;
-
 class TestHelper
 {
-    public function createTestDb(string $shlinkDbPath): void
+    public function createTestDb(): void
     {
-        if (file_exists($shlinkDbPath)) {
+        $process = new Process(['vendor/bin/doctrine', 'orm:schema-tool:drop', '--force', '--no-interaction', '-q']);
-            unlink($shlinkDbPath);
+        $process->inheritEnvironmentVariables()
-        }
+                ->mustRun();
 
         $process = new Process(['vendor/bin/doctrine', 'orm:schema-tool:create', '--no-interaction', '-q']);
         $process->inheritEnvironmentVariables()

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

@@ -58,6 +58,6 @@ $builder->createOneToMany('visits', Entity\Visit::class)
 
 $builder->createManyToMany('tags', Entity\Tag::class)
         ->setJoinTable('short_urls_in_tags')
-        ->addInverseJoinColumn('tag_id', 'id')
+        ->addInverseJoinColumn('tag_id', 'id', true, false, 'CASCADE')
-        ->addJoinColumn('short_url_id', 'id')
+        ->addJoinColumn('short_url_id', 'id', true, false, 'CASCADE')
         ->build();

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

@@ -43,10 +43,10 @@ $builder->createField('userAgent', Type::STRING)
         ->build();
 
 $builder->createManyToOne('shortUrl', Entity\ShortUrl::class)
-        ->addJoinColumn('short_url_id', 'id', false)
+        ->addJoinColumn('short_url_id', 'id', false, false, 'CASCADE')
         ->build();
 
 $builder->createManyToOne('visitLocation', Entity\VisitLocation::class)
-        ->addJoinColumn('visit_location_id', 'id')
+        ->addJoinColumn('visit_location_id', 'id', true, false, 'Set NULL')
         ->cascadePersist()
         ->build();

module/Core/src/Service/UrlShortener.php

@@ -112,32 +112,39 @@ class UrlShortener implements UrlShortenerInterface
         if ($meta->hasCustomSlug()) {
             $criteria['shortCode'] = $meta->getCustomSlug();
         }
-        /** @var ShortUrl|null $shortUrl */
+        /** @var ShortUrl[] $shortUrls */
-        $shortUrl = $this->em->getRepository(ShortUrl::class)->findOneBy($criteria);
+        $shortUrls = $this->em->getRepository(ShortUrl::class)->findBy($criteria);
-        if ($shortUrl === null) {
+        if (empty($shortUrls)) {
             return null;
         }
 
-        if ($meta->hasMaxVisits() && $meta->getMaxVisits() !== $shortUrl->getMaxVisits()) {
+        // Iterate short URLs until one that matches is found, or return null otherwise
-            return null;
+        return array_reduce($shortUrls, function (?ShortUrl $found, ShortUrl $shortUrl) use ($tags, $meta) {
-        }
+            if ($found) {
-        if ($meta->hasValidSince() && ! $meta->getValidSince()->eq($shortUrl->getValidSince())) {
+                return $found;
-            return null;
+            }
-        }
-        if ($meta->hasValidUntil() && ! $meta->getValidUntil()->eq($shortUrl->getValidUntil())) {
-            return null;
-        }
 
-        $shortUrlTags = invoke($shortUrl->getTags(), '__toString');
+            if ($meta->hasMaxVisits() && $meta->getMaxVisits() !== $shortUrl->getMaxVisits()) {
-        $hasAllTags = count($shortUrlTags) === count($tags) && array_reduce(
+                return null;
-            $tags,
+            }
-            function (bool $hasAllTags, string $tag) use ($shortUrlTags) {
+            if ($meta->hasValidSince() && ! $meta->getValidSince()->eq($shortUrl->getValidSince())) {
-                return $hasAllTags && contains($shortUrlTags, $tag);
+                return null;
-            },
+            }
-            true
+            if ($meta->hasValidUntil() && ! $meta->getValidUntil()->eq($shortUrl->getValidUntil())) {
-        );
+                return null;
+            }
 
-        return $hasAllTags ? $shortUrl : null;
+            $shortUrlTags = invoke($shortUrl->getTags(), '__toString');
+            $hasAllTags = count($shortUrlTags) === count($tags) && array_reduce(
+                $tags,
+                function (bool $hasAllTags, string $tag) use ($shortUrlTags) {
+                    return $hasAllTags && contains($shortUrlTags, $tag);
+                },
+                true
+            );
+
+            return $hasAllTags ? $shortUrl : null;
+        });
     }
 
     private function checkUrlExists(string $url): void

module/Core/test-db/Repository/ShortUrlRepositoryTest.php

@@ -18,9 +18,9 @@ use function count;
 class ShortUrlRepositoryTest extends DatabaseTestCase
 {
     protected const ENTITIES_TO_EMPTY = [
-        ShortUrl::class,
-        Visit::class,
         Tag::class,
+        Visit::class,
+        ShortUrl::class,
     ];
 
     /** @var ShortUrlRepository */

module/Core/test/Service/UrlShortenerTest.php

@@ -27,6 +27,8 @@ use Shlinkio\Shlink\Core\Repository\ShortUrlRepositoryInterface;
 use Shlinkio\Shlink\Core\Service\UrlShortener;
 use Zend\Diactoros\Uri;
 
+use function array_map;
+
 class UrlShortenerTest extends TestCase
 {
     /** @var UrlShortener */
@@ -121,7 +123,7 @@ class UrlShortenerTest extends TestCase
     {
         $repo = $this->prophesize(ShortUrlRepository::class);
         $countBySlug = $repo->count(['shortCode' => 'custom-slug'])->willReturn(1);
-        $repo->findOneBy(Argument::cetera())->willReturn(null);
+        $repo->findBy(Argument::cetera())->willReturn([]);
         $getRepo = $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal());
 
         $countBySlug->shouldBeCalledOnce();
@@ -146,20 +148,23 @@ class UrlShortenerTest extends TestCase
         ?ShortUrl $expected
     ): void {
         $repo = $this->prophesize(ShortUrlRepository::class);
-        $findExisting = $repo->findOneBy(Argument::any())->willReturn($expected);
+        $findExisting = $repo->findBy(Argument::any())->willReturn($expected !== null ? [$expected] : []);
         $getRepo = $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal());
 
         $result = $this->urlShortener->urlToShortCode(new Uri($url), $tags, $meta);
 
-        $this->assertSame($expected, $result);
         $findExisting->shouldHaveBeenCalledOnce();
         $getRepo->shouldHaveBeenCalledOnce();
+        if ($expected) {
+            $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']
@@ -203,6 +208,37 @@ class UrlShortenerTest extends TestCase
         ];
     }
 
+    /** @test */
+    public function properExistingShortUrlIsReturnedWhenMultipleMatch(): void
+    {
+        $url = 'http://foo.com';
+        $tags = ['baz', 'foo', 'bar'];
+        $meta = ShortUrlMeta::createFromRawData([
+            'findIfExists' => true,
+            'validUntil' => Chronos::parse('2017-01-01'),
+            'maxVisits' => 4,
+        ]);
+        $tagsCollection = new ArrayCollection(array_map(function (string $tag) {
+            return new Tag($tag);
+        }, $tags));
+        $expected = (new ShortUrl($url, $meta))->setTags($tagsCollection);
+
+        $repo = $this->prophesize(ShortUrlRepository::class);
+        $findExisting = $repo->findBy(Argument::any())->willReturn([
+            new ShortUrl($url),
+            new ShortUrl($url, $meta),
+            $expected,
+            (new ShortUrl($url))->setTags($tagsCollection),
+        ]);
+        $getRepo = $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal());
+
+        $result = $this->urlShortener->urlToShortCode(new Uri($url), $tags, $meta);
+
+        $this->assertSame($expected, $result);
+        $findExisting->shouldHaveBeenCalledOnce();
+        $getRepo->shouldHaveBeenCalledOnce();
+    }
+
     /** @test */
     public function shortCodeIsProperlyParsed(): void
     {