From bb9e57fa8b93028c6c758a9fc30b24937dbacb22 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Mon, 3 Feb 2020 21:20:40 +0100
Subject: [PATCH 01/67] Added support for mssql on dev env

---
 composer.json                      |  4 +++-
 config/test/test_config.global.php |  7 +++++++
 data/infra/php.Dockerfile          | 12 ++++++++++++
 data/infra/swoole.Dockerfile       | 17 +++++++++++------
 docker-compose.yml                 | 15 +++++++++++++++
 5 files changed, 48 insertions(+), 7 deletions(-)

diff --git a/composer.json b/composer.json
index 908b9af3..3f897602 100644
--- a/composer.json
+++ b/composer.json
@@ -115,7 +115,8 @@
             "@test:db:sqlite",
             "@test:db:mysql",
             "@test:db:maria",
-            "@test:db:postgres"
+            "@test:db:postgres",
+            "@test:db:ms"
         ],
         "test:db:ci": [
             "@test:db:sqlite",
@@ -126,6 +127,7 @@
         "test:db:mysql": "DB_DRIVER=mysql composer test:db:sqlite",
         "test:db:maria": "DB_DRIVER=maria composer test:db:sqlite",
         "test:db:postgres": "DB_DRIVER=postgres composer test:db:sqlite",
+        "test:db:ms": "DB_DRIVER=mssql composer test:db:sqlite",
         "test:api": "bin/test/run-api-tests.sh",
         "test:unit:pretty": "phpdbg -qrr vendor/bin/phpunit --order-by=random --colors=always --coverage-html build/coverage",
         "infect": "infection --threads=4 --min-msi=80 --log-verbosity=default --only-covered",
diff --git a/config/test/test_config.global.php b/config/test/test_config.global.php
index c4556b8b..fa51c240 100644
--- a/config/test/test_config.global.php
+++ b/config/test/test_config.global.php
@@ -45,6 +45,13 @@ $buildDbConnection = function (): array {
             'dbname' => 'shlink_test',
             'charset' => 'utf8',
         ],
+        'mssql' => [
+            'driver' => 'pdo_sqlsrv',
+            'host' => $isCi ? '127.0.0.1' : 'shlink_db_ms',
+            'user' => 'sa',
+            'password' => $isCi ? '' : 'Passw0rd!',
+            'dbname' => 'shlink_test',
+        ],
     ];
     $driverConfigMap['maria'] = $driverConfigMap['mysql'];
 
diff --git a/data/infra/php.Dockerfile b/data/infra/php.Dockerfile
index e92cc815..d31ad6f0 100644
--- a/data/infra/php.Dockerfile
+++ b/data/infra/php.Dockerfile
@@ -65,6 +65,18 @@ RUN docker-php-ext-configure xdebug\
 # cleanup
 RUN rm /tmp/xdebug.tar.gz
 
+# Install sqlsrv driver
+RUN wget https://download.microsoft.com/download/e/4/e/e4e67866-dffd-428c-aac7-8d28ddafb39b/msodbcsql17_17.5.1.1-1_amd64.apk && \
+    wget https://download.microsoft.com/download/e/4/e/e4e67866-dffd-428c-aac7-8d28ddafb39b/mssql-tools_17.5.1.1-1_amd64.apk && \
+    apk add --allow-untrusted msodbcsql17_17.5.1.1-1_amd64.apk && \
+    apk add --allow-untrusted mssql-tools_17.5.1.1-1_amd64.apk && \
+    apk add --no-cache --virtual .phpize-deps $PHPIZE_DEPS unixodbc-dev && \
+    pecl install pdo_sqlsrv && \
+    docker-php-ext-enable pdo_sqlsrv && \
+    apk del .phpize-deps && \
+    rm msodbcsql17_17.5.1.1-1_amd64.apk && \
+    rm mssql-tools_17.5.1.1-1_amd64.apk
+
 # Install composer
 RUN php -r "readfile('https://getcomposer.org/installer');" | php
 RUN chmod +x composer.phar
diff --git a/data/infra/swoole.Dockerfile b/data/infra/swoole.Dockerfile
index 8bc821d9..bf591ee1 100644
--- a/data/infra/swoole.Dockerfile
+++ b/data/infra/swoole.Dockerfile
@@ -66,12 +66,17 @@ RUN docker-php-ext-configure 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 && \
-    pecl install swoole-${SWOOLE_VERSION} && \
-    docker-php-ext-enable swoole && \
-    apk del .phpize-deps
+# Install swoole and mssql driver
+RUN wget https://download.microsoft.com/download/e/4/e/e4e67866-dffd-428c-aac7-8d28ddafb39b/msodbcsql17_17.5.1.1-1_amd64.apk && \
+    wget https://download.microsoft.com/download/e/4/e/e4e67866-dffd-428c-aac7-8d28ddafb39b/mssql-tools_17.5.1.1-1_amd64.apk && \
+    apk add --allow-untrusted msodbcsql17_17.5.1.1-1_amd64.apk && \
+    apk add --allow-untrusted mssql-tools_17.5.1.1-1_amd64.apk && \
+    apk add --no-cache --virtual .phpize-deps $PHPIZE_DEPS unixodbc-dev && \
+    pecl install swoole-${SWOOLE_VERSION} pdo_sqlsrv && \
+    docker-php-ext-enable swoole pdo_sqlsrv && \
+    apk del .phpize-deps && \
+    rm msodbcsql17_17.5.1.1-1_amd64.apk && \
+    rm mssql-tools_17.5.1.1-1_amd64.apk
 
 # Install composer
 RUN php -r "readfile('https://getcomposer.org/installer');" | php
diff --git a/docker-compose.yml b/docker-compose.yml
index 99cc93fb..b672a05a 100644
--- a/docker-compose.yml
+++ b/docker-compose.yml
@@ -25,6 +25,7 @@ services:
             - shlink_db
             - shlink_db_postgres
             - shlink_db_maria
+            - shlink_db_ms
             - shlink_redis
 
     shlink_swoole:
@@ -42,7 +43,12 @@ services:
             - shlink_db
             - shlink_db_postgres
             - shlink_db_maria
+            - shlink_db_ms
             - shlink_redis
+        environment:
+            LANG: en_US.UTF-8
+            LANGUAGE: en_US.UTF-8
+            LC_ALL: C
 
     shlink_db:
         container_name: shlink_db
@@ -82,6 +88,15 @@ services:
             MYSQL_DATABASE: shlink
             MYSQL_INITDB_SKIP_TZINFO: 1
 
+    shlink_db_ms:
+        container_name: shlink_db_ms
+        image: mcr.microsoft.com/mssql/server:2019-latest
+        ports:
+            - "1433:1433"
+        environment:
+            ACCEPT_EULA: Y
+            SA_PASSWORD: "Passw0rd!"
+
     shlink_redis:
         container_name: shlink_redis
         image: redis:5.0-alpine

From 542673fcb070e96af3d6cc244fa6752e2f1913b8 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Fri, 14 Feb 2020 19:11:29 +0100
Subject: [PATCH 02/67] Updated development docker images

---
 data/infra/php.Dockerfile    | 2 +-
 data/infra/swoole.Dockerfile | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/data/infra/php.Dockerfile b/data/infra/php.Dockerfile
index d31ad6f0..c5401651 100644
--- a/data/infra/php.Dockerfile
+++ b/data/infra/php.Dockerfile
@@ -1,4 +1,4 @@
-FROM php:7.4.1-fpm-alpine3.10
+FROM php:7.4.2-fpm-alpine3.11
 MAINTAINER Alejandro Celaya <alejandro@alejandrocelaya.com>
 
 ENV APCU_VERSION 5.1.18
diff --git a/data/infra/swoole.Dockerfile b/data/infra/swoole.Dockerfile
index bf591ee1..3f7a1513 100644
--- a/data/infra/swoole.Dockerfile
+++ b/data/infra/swoole.Dockerfile
@@ -1,10 +1,10 @@
-FROM php:7.4.1-alpine3.10
+FROM php:7.4.2-alpine3.11
 MAINTAINER Alejandro Celaya <alejandro@alejandrocelaya.com>
 
 ENV APCU_VERSION 5.1.18
 ENV APCU_BC_VERSION 1.0.5
 ENV INOTIFY_VERSION 2.0.0
-ENV SWOOLE_VERSION 4.4.12
+ENV SWOOLE_VERSION 4.4.15
 
 RUN apk update
 

From 27fd9c598864983a1f7b22c74e8b7cf1d24be1f2 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Fri, 14 Feb 2020 19:20:54 +0100
Subject: [PATCH 03/67] Added MSSQL driver to prod docker image

---
 Dockerfile | 26 +++++++++++++++-----------
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/Dockerfile b/Dockerfile
index 01a93c26..a5e14a75 100644
--- a/Dockerfile
+++ b/Dockerfile
@@ -1,10 +1,9 @@
-FROM php:7.4.1-alpine3.10
+FROM php:7.4.2-alpine3.11
 LABEL maintainer="Alejandro Celaya <alejandro@alejandrocelaya.com>"
 
-ARG SHLINK_VERSION=2.0.0
+ARG SHLINK_VERSION=2.0.5
 ENV SHLINK_VERSION ${SHLINK_VERSION}
-ENV SWOOLE_VERSION 4.4.12
-ENV COMPOSER_VERSION 1.9.1
+ENV SWOOLE_VERSION 4.4.15
 
 WORKDIR /etc/shlink
 
@@ -24,17 +23,22 @@ RUN \
     apk add --no-cache libzip-dev zlib-dev libpng-dev && \
     docker-php-ext-install -j"$(nproc)" zip gd
 
-# 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-${SWOOLE_VERSION} && \
-    docker-php-ext-enable swoole && \
-    apk del .phpize-deps
+# Install swoole and sqlsrv driver
+RUN wget https://download.microsoft.com/download/e/4/e/e4e67866-dffd-428c-aac7-8d28ddafb39b/msodbcsql17_17.5.1.1-1_amd64.apk && \
+    wget https://download.microsoft.com/download/e/4/e/e4e67866-dffd-428c-aac7-8d28ddafb39b/mssql-tools_17.5.1.1-1_amd64.apk && \
+    apk add --allow-untrusted msodbcsql17_17.5.1.1-1_amd64.apk && \
+    apk add --allow-untrusted mssql-tools_17.5.1.1-1_amd64.apk && \
+    apk add --no-cache --virtual .phpize-deps $PHPIZE_DEPS unixodbc-dev && \
+    pecl install swoole-${SWOOLE_VERSION} pdo_sqlsrv && \
+    docker-php-ext-enable swoole pdo_sqlsrv && \
+    apk del .phpize-deps && \
+    rm msodbcsql17_17.5.1.1-1_amd64.apk && \
+    rm mssql-tools_17.5.1.1-1_amd64.apk
 
 # Install shlink
 COPY . .
+COPY --from=composer:1.9.3 /usr/bin/composer ./composer.phar
 RUN rm -rf ./docker && \
-    wget https://getcomposer.org/download/${COMPOSER_VERSION}/composer.phar && \
     php composer.phar install --no-dev --optimize-autoloader --prefer-dist --no-progress --no-interaction && \
     php composer.phar clear-cache && \
     rm composer.*

From 2bb2c2cde3ee01038fdff45aecf6521bb1d5591e Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Fri, 14 Feb 2020 19:27:21 +0100
Subject: [PATCH 04/67] Documented how to use mssql with the docker image

---
 docker/README.md                         | 10 ++++++----
 docker/config/shlink_in_docker.local.php |  2 ++
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/docker/README.md b/docker/README.md
index 0af66442..76c47a09 100644
--- a/docker/README.md
+++ b/docker/README.md
@@ -56,9 +56,9 @@ docker exec -it shlink_container shlink
 
 The image comes with a working sqlite database, but in production you will probably want to usa a distributed database.
 
-It is possible to use a set of env vars to make this shlink instance interact with an external MySQL, MariaDB or PostgreSQL database.
+It is possible to use a set of env vars to make this shlink instance interact with an external MySQL, MariaDB, PostgreSQL or Microsoft SQL Server database.
 
-* `DB_DRIVER`: **[Mandatory]**. Use the value **mysql**, **maria** or **postgres** to prevent the sqlite database to be used.
+* `DB_DRIVER`: **[Mandatory]**. Use the value **mysql**, **maria**, **postgres** or **mssql** to prevent the sqlite database to be used.
 * `DB_NAME`: [Optional]. The database name to be used. Defaults to **shlink**.
 * `DB_USER`: **[Mandatory]**. The username credential for the database server.
 * `DB_PASSWORD`: **[Mandatory]**. The password credential for the database server.
@@ -67,8 +67,9 @@ It is possible to use a set of env vars to make this shlink instance interact wi
     * Default value is based on the value provided for `DB_DRIVER`:
         * **mysql** or **maria** -> `3306`
         * **postgres** -> `5432`
+        * **mssql** -> `1433`
 
-> PostgreSQL is supported since v1.16.1 of this image. Do not try to use it with previous versions.
+> PostgreSQL is supported since v1.16.1 and Microsoft SQL server since v2.1.0. Do not try to use them with previous versions.
 
 Taking this into account, you could run shlink on a local docker service like this:
 
@@ -92,7 +93,7 @@ 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**.
-* `DB_DRIVER`: **sqlite** (which is the default value), **mysql**, **maria** or **postgres**.
+* `DB_DRIVER`: **sqlite** (which is the default value), **mysql**, **maria**, **postgres** or **mssql**.
 * `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.
 * `DB_PASSWORD`: The password credential to be used when using an external database driver.
@@ -101,6 +102,7 @@ This is the complete list of supported env vars:
     * Default value is based on the value provided for `DB_DRIVER`:
         * **mysql** or **maria** -> `3306`
         * **postgres** -> `5432`
+        * **mssql** -> `1433`
 * `DISABLE_TRACK_PARAM`: The name of a query param that can be used to visit short URLs avoiding the visit to be tracked. This feature won't be available if not value is provided.
 * `DELETE_SHORT_URL_THRESHOLD`: The amount of visits on short URLs which will not allow them to be deleted. Defaults to `15`.
 * `VALIDATE_URLS`: Boolean which tells if shlink should validate a status 20x is returned (after following redirects) when trying to shorten a URL. Defaults to `false`.
diff --git a/docker/config/shlink_in_docker.local.php b/docker/config/shlink_in_docker.local.php
index 7eba5560..9e10d419 100644
--- a/docker/config/shlink_in_docker.local.php
+++ b/docker/config/shlink_in_docker.local.php
@@ -16,11 +16,13 @@ $helper = new class {
         'mysql' => 'pdo_mysql',
         'maria' => 'pdo_mysql',
         'postgres' => 'pdo_pgsql',
+        'mssql' => 'pdo_sqlsrv',
     ];
     private const DB_PORTS_MAP = [
         'mysql' => '3306',
         'maria' => '3306',
         'postgres' => '5432',
+        'mssql' => '1433',
     ];
 
     public function getDbConfig(): array

From d8cbf0512b780fb1034f5098f1241c0651aa98d4 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Fri, 14 Feb 2020 19:44:11 +0100
Subject: [PATCH 05/67] Added missing env var which fixes issues with mssql
 driver on alpine

---
 Dockerfile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Dockerfile b/Dockerfile
index a5e14a75..23a142f8 100644
--- a/Dockerfile
+++ b/Dockerfile
@@ -4,6 +4,7 @@ LABEL maintainer="Alejandro Celaya <alejandro@alejandrocelaya.com>"
 ARG SHLINK_VERSION=2.0.5
 ENV SHLINK_VERSION ${SHLINK_VERSION}
 ENV SWOOLE_VERSION 4.4.15
+ENV LC_ALL "C"
 
 WORKDIR /etc/shlink
 

From 12adce9ac24cfe39750768ebf65931ffbd27a79b Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Fri, 14 Feb 2020 19:51:58 +0100
Subject: [PATCH 06/67] Updated changelog

---
 CHANGELOG.md | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 9e19191d..3ed83602 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,6 +4,29 @@ 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).
 
+## [Unreleased]
+
+#### Added
+
+* [#626](https://github.com/shlinkio/shlink/issues/626) Added support for Microsoft SQL Server.
+
+#### Changed
+
+* *Nothing*
+
+#### Deprecated
+
+* *Nothing*
+
+#### Removed
+
+* *Nothing*
+
+#### Fixed
+
+* *Nothing*
+
+
 ## 2.0.5 - 2020-02-09
 
 #### Added

From 5886d73093af9e4651a50b7a9fdce90de6a6128f Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Fri, 14 Feb 2020 19:55:24 +0100
Subject: [PATCH 07/67] Documented support for Microsoft SQL Server

---
 .github/ISSUE_TEMPLATE/Bug.md              | 2 +-
 .github/ISSUE_TEMPLATE/Question_Support.md | 2 +-
 README.md                                  | 6 +++---
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/.github/ISSUE_TEMPLATE/Bug.md b/.github/ISSUE_TEMPLATE/Bug.md
index 59f71b26..25a433c2 100644
--- a/.github/ISSUE_TEMPLATE/Bug.md
+++ b/.github/ISSUE_TEMPLATE/Bug.md
@@ -18,7 +18,7 @@ With that said, please fill in the information requested next. More information
 * Shlink Version: x.y.z
 * PHP Version: x.y.z
 * How do you serve Shlink: Self-hosted Apache|Self-hosted nginx|Self-hosted swoole|Docker image
-* Database engine used: MySQL|MariaDB|PostgreSQL|SQLite (x.y.z)
+* Database engine used: MySQL|MariaDB|PostgreSQL|MicrosoftSQL|SQLite (x.y.z)
 
 #### Summary
 
diff --git a/.github/ISSUE_TEMPLATE/Question_Support.md b/.github/ISSUE_TEMPLATE/Question_Support.md
index 885f866f..5d4f55c6 100644
--- a/.github/ISSUE_TEMPLATE/Question_Support.md
+++ b/.github/ISSUE_TEMPLATE/Question_Support.md
@@ -18,7 +18,7 @@ With that said, please fill in the information requested next. More information
 * Shlink Version: x.y.z
 * PHP Version: x.y.z
 * How do you serve Shlink: Self-hosted Apache|Self-hosted nginx|Self-hosted swoole|Docker image
-* Database engine used: MySQL|MariaDB|PostgreSQL|SQLite (x.y.z)
+* Database engine used: MySQL|MariaDB|PostgreSQL|MicrosoftSQL|SQLite (x.y.z)
 
 #### Summary
 
diff --git a/README.md b/README.md
index 3eb8a33d..414cb656 100644
--- a/README.md
+++ b/README.md
@@ -35,8 +35,8 @@ A PHP-based self-hosted URL shortener that can be used to serve shortened URLs u
 
 First, make sure the host where you are going to run shlink fulfills these requirements:
 
-* PHP 7.4 or greater with JSON, APCu, intl, curl, PDO and gd extensions enabled.
-* MySQL, MariaDB, PostgreSQL or SQLite.
+* PHP 7.4 or greater with JSON, curl, PDO and gd extensions enabled.
+* MySQL, MariaDB, PostgreSQL, Microsoft SQL Server or SQLite.
 * The web server of your choice with PHP integration (Apache or Nginx recommended).
 
 ### Download
@@ -67,7 +67,7 @@ In order to run Shlink, you will need a built version of the project. There are
 
 Despite how you built the project, you now need to configure it, by following these steps:
 
-* If you are going to use MySQL, MariaDB or PostgreSQL, create an empty database with the name of your choice.
+* If you are going to use MySQL, MariaDB, PostgreSQL or Microsoft SQL Server, create an empty database with the name of your choice.
 * Recursively grant write permissions to the `data` directory. Shlink uses it to cache some information.
 * Setup the application by running the `bin/install` script. It is a command line tool that will guide you through the installation process. **Take into account that this tool has to be run directly on the server where you plan to host Shlink. Do not run it before uploading/moving it there.**
 * Generate your first API key by running `bin/cli api-key:generate`. You will need the key in order to interact with shlink's API.

From a3fc1513e141154b0c688a8f7cef04722d3c82dd Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Sat, 15 Feb 2020 20:28:32 +0100
Subject: [PATCH 08/67] Updated Installer to include the one supporting MsSQL

---
 composer.json      | 6 +++---
 docker-compose.yml | 4 ++--
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/composer.json b/composer.json
index 3f897602..f26c9eec 100644
--- a/composer.json
+++ b/composer.json
@@ -40,16 +40,16 @@
         "mezzio/mezzio-helpers": "^5.3",
         "mezzio/mezzio-platesrenderer": "^2.1",
         "mezzio/mezzio-problem-details": "^1.1",
-        "mezzio/mezzio-swoole": "^2.4",
+        "mezzio/mezzio-swoole": "^2.6",
         "monolog/monolog": "^2.0",
         "nikolaposa/monolog-factory": "^3.0",
-        "ocramius/proxy-manager": "^2.6.0",
+        "ocramius/proxy-manager": "^2.7.0",
         "phly/phly-event-dispatcher": "^1.0",
         "predis/predis": "^1.1",
         "pugx/shortid-php": "^0.5",
         "shlinkio/shlink-common": "^2.7.0",
         "shlinkio/shlink-event-dispatcher": "^1.3",
-        "shlinkio/shlink-installer": "^4.0.1",
+        "shlinkio/shlink-installer": "^4.1.0",
         "shlinkio/shlink-ip-geolocation": "^1.3.1",
         "symfony/console": "^5.0",
         "symfony/filesystem": "^5.0",
diff --git a/docker-compose.yml b/docker-compose.yml
index b672a05a..c78cf85f 100644
--- a/docker-compose.yml
+++ b/docker-compose.yml
@@ -27,6 +27,8 @@ services:
             - shlink_db_maria
             - shlink_db_ms
             - shlink_redis
+        environment:
+            LC_ALL: C
 
     shlink_swoole:
         container_name: shlink_swoole
@@ -46,8 +48,6 @@ services:
             - shlink_db_ms
             - shlink_redis
         environment:
-            LANG: en_US.UTF-8
-            LANGUAGE: en_US.UTF-8
             LC_ALL: C
 
     shlink_db:

From a9269811dc102412bea959d446363a230d84b598 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Sat, 15 Feb 2020 20:55:04 +0100
Subject: [PATCH 09/67] Added command to run api tests with code coverage

---
 bin/test/run-api-tests.sh | 2 +-
 composer.json             | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/bin/test/run-api-tests.sh b/bin/test/run-api-tests.sh
index 2a14b218..fae0c628 100755
--- a/bin/test/run-api-tests.sh
+++ b/bin/test/run-api-tests.sh
@@ -9,7 +9,7 @@ echo 'Starting server...'
 vendor/bin/mezzio-swoole start -d
 sleep 2
 
-vendor/bin/phpunit --order-by=random -c phpunit-api.xml --testdox --colors=always $*
+phpdbg -qrr vendor/bin/phpunit --order-by=random -c phpunit-api.xml --testdox --colors=always $*
 testsExitCode=$?
 
 vendor/bin/mezzio-swoole stop
diff --git a/composer.json b/composer.json
index f26c9eec..c9199246 100644
--- a/composer.json
+++ b/composer.json
@@ -107,7 +107,7 @@
         "test:ci": [
             "@test:unit:ci",
             "@test:db:ci",
-            "@test:api"
+            "@test:api:ci"
         ],
         "test:unit": "phpdbg -qrr vendor/bin/phpunit --order-by=random --colors=always --coverage-php build/coverage-unit.cov --testdox",
         "test:unit:ci": "@test:unit --coverage-clover=build/clover.xml --coverage-xml=build/coverage-xml --log-junit=build/junit.xml",
@@ -129,6 +129,7 @@
         "test:db:postgres": "DB_DRIVER=postgres composer test:db:sqlite",
         "test:db:ms": "DB_DRIVER=mssql composer test:db:sqlite",
         "test:api": "bin/test/run-api-tests.sh",
+        "test:api:ci": "@test:api --coverage-php build/coverage-api.cov",
         "test:unit:pretty": "phpdbg -qrr vendor/bin/phpunit --order-by=random --colors=always --coverage-html build/coverage",
         "infect": "infection --threads=4 --min-msi=80 --log-verbosity=default --only-covered",
         "infect:ci": "@infect --coverage=build",

From 37c0a813db70e5efc480c0446a4abf7230b252b8 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Sat, 15 Feb 2020 21:14:14 +0100
Subject: [PATCH 10/67] Updated to PHPUnit 9

---
 .travis.yml   | 4 ++--
 composer.json | 6 +++---
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index 5d6176ed..4e4c70f5 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -37,8 +37,8 @@ script:
 
 after_success:
   - rm -f build/clover.xml
-  - wget https://phar.phpunit.de/phpcov-6.0.1.phar
-  - phpdbg -qrr phpcov-6.0.1.phar merge build --clover build/clover.xml
+  - wget https://phar.phpunit.de/phpcov-7.0.1.phar
+  - phpdbg -qrr phpcov-7.0.1.phar merge build --clover build/clover.xml
   - wget https://scrutinizer-ci.com/ocular.phar
   - php ocular.phar code-coverage:upload --format=php-clover build/clover.xml
 
diff --git a/composer.json b/composer.json
index c9199246..cdda9028 100644
--- a/composer.json
+++ b/composer.json
@@ -58,14 +58,14 @@
     },
     "require-dev": {
         "devster/ubench": "^2.0",
-        "dms/phpunit-arraysubset-asserts": "^0.1.0",
+        "dms/phpunit-arraysubset-asserts": "^0.2.0",
         "eaglewu/swoole-ide-helper": "dev-master",
         "infection/infection": "^0.15.0",
         "phpstan/phpstan": "^0.12.3",
-        "phpunit/phpunit": "^8.3",
+        "phpunit/phpunit": "^9.0.1",
         "roave/security-advisories": "dev-master",
         "shlinkio/php-coding-standard": "~2.1.0",
-        "shlinkio/shlink-test-utils": "^1.3",
+        "shlinkio/shlink-test-utils": "^1.4",
         "symfony/var-dumper": "^5.0"
     },
     "autoload": {

From 2cf9f64e8e9f9eb0733e0b4f176174757e943471 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Sat, 15 Feb 2020 21:14:55 +0100
Subject: [PATCH 11/67] Updated changelog

---
 CHANGELOG.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 3ed83602..edf73367 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -12,7 +12,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this
 
 #### Changed
 
-* *Nothing*
+* [#656](https://github.com/shlinkio/shlink/issues/656) Updated to PHPUnit 9.
 
 #### Deprecated
 

From 8162dafe164d985252cbb5bd6ebdfef94147cecd Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Sun, 16 Feb 2020 12:18:23 +0100
Subject: [PATCH 12/67] Added separator in readme before MaxMind reference

---
 README.md | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/README.md b/README.md
index 414cb656..eb9151db 100644
--- a/README.md
+++ b/README.md
@@ -344,4 +344,6 @@ Those are configured during Shlink's installation or via env vars when using the
 
 Currently those are all shared for all domains serving the same Shlink instance, but the plan is to update that and allow specific ones for every existing domain.
 
+---
+
 > This product includes GeoLite2 data created by MaxMind, available from [https://www.maxmind.com](https://www.maxmind.com)

From 13555366e3f16a06afee16921b4c4edaf9322598 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Tue, 18 Feb 2020 18:54:40 +0100
Subject: [PATCH 13/67] Short code lengths can now be customized

---
 config/autoload/url-shortener.global.php      |  3 +++
 module/Core/functions/functions.php           |  4 ++-
 module/Core/src/Entity/ShortUrl.php           |  6 +++--
 module/Core/src/Model/ShortUrlMeta.php        | 21 +++++++++++++--
 .../Validation/ShortUrlMetaInputFilter.php    | 17 +++++++++---
 module/Core/test/Entity/ShortUrlTest.php      | 26 +++++++++++++++++++
 6 files changed, 68 insertions(+), 9 deletions(-)

diff --git a/config/autoload/url-shortener.global.php b/config/autoload/url-shortener.global.php
index 5cf4f86f..165e0258 100644
--- a/config/autoload/url-shortener.global.php
+++ b/config/autoload/url-shortener.global.php
@@ -2,6 +2,8 @@
 
 declare(strict_types=1);
 
+use const Shlinkio\Shlink\Core\DEFAULT_SHORT_CODES_LENGTH;
+
 return [
 
     'url_shortener' => [
@@ -11,6 +13,7 @@ return [
         ],
         'validate_url' => false,
         'visits_webhooks' => [],
+        'default_short_codes_length' => DEFAULT_SHORT_CODES_LENGTH,
     ],
 
 ];
diff --git a/module/Core/functions/functions.php b/module/Core/functions/functions.php
index 7ab5ebbb..61d0be1e 100644
--- a/module/Core/functions/functions.php
+++ b/module/Core/functions/functions.php
@@ -10,7 +10,9 @@ use PUGX\Shortid\Factory as ShortIdFactory;
 
 use function sprintf;
 
-function generateRandomShortCode(int $length = 5): string
+const DEFAULT_SHORT_CODES_LENGTH = 5;
+
+function generateRandomShortCode(int $length): string
 {
     static $shortIdFactory;
     if ($shortIdFactory === null) {
diff --git a/module/Core/src/Entity/ShortUrl.php b/module/Core/src/Entity/ShortUrl.php
index 98d6a146..4af8844b 100644
--- a/module/Core/src/Entity/ShortUrl.php
+++ b/module/Core/src/Entity/ShortUrl.php
@@ -34,6 +34,7 @@ class ShortUrl extends AbstractEntity
     private ?int $maxVisits = null;
     private ?Domain $domain;
     private bool $customSlugWasProvided;
+    private int $shortCodeLength;
 
     public function __construct(
         string $longUrl,
@@ -50,7 +51,8 @@ class ShortUrl extends AbstractEntity
         $this->validUntil = $meta->getValidUntil();
         $this->maxVisits = $meta->getMaxVisits();
         $this->customSlugWasProvided = $meta->hasCustomSlug();
-        $this->shortCode = $meta->getCustomSlug() ?? generateRandomShortCode();
+        $this->shortCodeLength = $meta->getShortCodeLength();
+        $this->shortCode = $meta->getCustomSlug() ?? generateRandomShortCode($this->shortCodeLength);
         $this->domain = ($domainResolver ?? new SimpleDomainResolver())->resolveDomain($meta->getDomain());
     }
 
@@ -119,7 +121,7 @@ class ShortUrl extends AbstractEntity
             throw ShortCodeCannotBeRegeneratedException::forShortUrlAlreadyPersisted();
         }
 
-        $this->shortCode = generateRandomShortCode();
+        $this->shortCode = generateRandomShortCode($this->shortCodeLength);
         return $this;
     }
 
diff --git a/module/Core/src/Model/ShortUrlMeta.php b/module/Core/src/Model/ShortUrlMeta.php
index 27c8e624..3bba5c98 100644
--- a/module/Core/src/Model/ShortUrlMeta.php
+++ b/module/Core/src/Model/ShortUrlMeta.php
@@ -11,6 +11,8 @@ use Shlinkio\Shlink\Core\Validation\ShortUrlMetaInputFilter;
 use function array_key_exists;
 use function Shlinkio\Shlink\Core\parseDateField;
 
+use const Shlinkio\Shlink\Core\DEFAULT_SHORT_CODES_LENGTH;
+
 final class ShortUrlMeta
 {
     private bool $validSincePropWasProvided = false;
@@ -22,6 +24,7 @@ final class ShortUrlMeta
     private ?int $maxVisits = null;
     private ?bool $findIfExists = null;
     private ?string $domain = null;
+    private int $shortCodeLength = 5;
 
     // Force named constructors
     private function __construct()
@@ -58,11 +61,20 @@ final class ShortUrlMeta
         $this->validUntil = parseDateField($inputFilter->getValue(ShortUrlMetaInputFilter::VALID_UNTIL));
         $this->validUntilPropWasProvided = array_key_exists(ShortUrlMetaInputFilter::VALID_UNTIL, $data);
         $this->customSlug = $inputFilter->getValue(ShortUrlMetaInputFilter::CUSTOM_SLUG);
-        $maxVisits = $inputFilter->getValue(ShortUrlMetaInputFilter::MAX_VISITS);
-        $this->maxVisits = $maxVisits !== null ? (int) $maxVisits : null;
+        $this->maxVisits = $this->getOptionalIntFromInputFilter($inputFilter, ShortUrlMetaInputFilter::MAX_VISITS);
         $this->maxVisitsPropWasProvided = array_key_exists(ShortUrlMetaInputFilter::MAX_VISITS, $data);
         $this->findIfExists = $inputFilter->getValue(ShortUrlMetaInputFilter::FIND_IF_EXISTS);
         $this->domain = $inputFilter->getValue(ShortUrlMetaInputFilter::DOMAIN);
+        $this->shortCodeLength = $this->getOptionalIntFromInputFilter(
+            $inputFilter,
+            ShortUrlMetaInputFilter::SHORT_CODE_LENGTH,
+        ) ?? DEFAULT_SHORT_CODES_LENGTH;
+    }
+
+    private function getOptionalIntFromInputFilter(ShortUrlMetaInputFilter $inputFilter, string $fieldName): ?int
+    {
+        $value = $inputFilter->getValue($fieldName);
+        return $value !== null ? (int) $value : null;
     }
 
     public function getValidSince(): ?Chronos
@@ -119,4 +131,9 @@ final class ShortUrlMeta
     {
         return $this->domain;
     }
+
+    public function getShortCodeLength(): int
+    {
+        return $this->shortCodeLength;
+    }
 }
diff --git a/module/Core/src/Validation/ShortUrlMetaInputFilter.php b/module/Core/src/Validation/ShortUrlMetaInputFilter.php
index 187ec66f..0663a760 100644
--- a/module/Core/src/Validation/ShortUrlMetaInputFilter.php
+++ b/module/Core/src/Validation/ShortUrlMetaInputFilter.php
@@ -5,6 +5,7 @@ declare(strict_types=1);
 namespace Shlinkio\Shlink\Core\Validation;
 
 use DateTime;
+use Laminas\InputFilter\Input;
 use Laminas\InputFilter\InputFilter;
 use Laminas\Validator;
 use Shlinkio\Shlink\Common\Validation;
@@ -19,6 +20,7 @@ class ShortUrlMetaInputFilter extends InputFilter
     public const MAX_VISITS = 'maxVisits';
     public const FIND_IF_EXISTS = 'findIfExists';
     public const DOMAIN = 'domain';
+    public const SHORT_CODE_LENGTH = 'shortCodeLength';
 
     public function __construct(array $data)
     {
@@ -40,10 +42,8 @@ class ShortUrlMetaInputFilter extends InputFilter
         $customSlug->getFilterChain()->attach(new Validation\SluggerFilter());
         $this->add($customSlug);
 
-        $maxVisits = $this->createInput(self::MAX_VISITS, false);
-        $maxVisits->getValidatorChain()->attach(new Validator\Digits())
-                                       ->attach(new Validator\GreaterThan(['min' => 1, 'inclusive' => true]));
-        $this->add($maxVisits);
+        $this->add($this->createPositiveNumberInput(self::MAX_VISITS));
+        $this->add($this->createPositiveNumberInput(self::SHORT_CODE_LENGTH));
 
         $this->add($this->createBooleanInput(self::FIND_IF_EXISTS, false));
 
@@ -51,4 +51,13 @@ class ShortUrlMetaInputFilter extends InputFilter
         $domain->getValidatorChain()->attach(new Validation\HostAndPortValidator());
         $this->add($domain);
     }
+
+    private function createPositiveNumberInput(string $name): Input
+    {
+        $input = $this->createInput($name, false);
+        $input->getValidatorChain()->attach(new Validator\Digits())
+              ->attach(new Validator\GreaterThan(['min' => 1, 'inclusive' => true]));
+
+        return $input;
+    }
 }
diff --git a/module/Core/test/Entity/ShortUrlTest.php b/module/Core/test/Entity/ShortUrlTest.php
index 9aba83fa..21c869aa 100644
--- a/module/Core/test/Entity/ShortUrlTest.php
+++ b/module/Core/test/Entity/ShortUrlTest.php
@@ -8,6 +8,13 @@ use PHPUnit\Framework\TestCase;
 use Shlinkio\Shlink\Core\Entity\ShortUrl;
 use Shlinkio\Shlink\Core\Exception\ShortCodeCannotBeRegeneratedException;
 use Shlinkio\Shlink\Core\Model\ShortUrlMeta;
+use Shlinkio\Shlink\Core\Validation\ShortUrlMetaInputFilter;
+
+use function Functional\map;
+use function range;
+use function strlen;
+
+use const Shlinkio\Shlink\Core\DEFAULT_SHORT_CODES_LENGTH;
 
 class ShortUrlTest extends TestCase
 {
@@ -48,4 +55,23 @@ class ShortUrlTest extends TestCase
 
         $this->assertNotEquals($firstShortCode, $secondShortCode);
     }
+
+    /**
+     * @test
+     * @dataProvider provideLengths
+     */
+    public function shortCodesHaveExpectedLength(?int $length, int $expectedLength): void
+    {
+        $shortUrl = new ShortUrl('', ShortUrlMeta::fromRawData(
+            [ShortUrlMetaInputFilter::SHORT_CODE_LENGTH => $length],
+        ));
+
+        $this->assertEquals($expectedLength, strlen($shortUrl->getShortCode()));
+    }
+
+    public function provideLengths(): iterable
+    {
+        yield [null, DEFAULT_SHORT_CODES_LENGTH];
+        yield from map(range(1, 10), fn (int $value) => [$value, $value]);
+    }
 }

From 9372d1739aacf801698ae88cc8056a8e6176aee1 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Tue, 18 Feb 2020 18:57:24 +0100
Subject: [PATCH 14/67] Enforced short URLs length to be 4 at least

---
 module/Core/src/Validation/ShortUrlMetaInputFilter.php | 6 +++---
 module/Core/test/Entity/ShortUrlTest.php               | 2 +-
 module/Core/test/Model/ShortUrlMetaTest.php            | 3 +++
 3 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/module/Core/src/Validation/ShortUrlMetaInputFilter.php b/module/Core/src/Validation/ShortUrlMetaInputFilter.php
index 0663a760..9e8b1c7a 100644
--- a/module/Core/src/Validation/ShortUrlMetaInputFilter.php
+++ b/module/Core/src/Validation/ShortUrlMetaInputFilter.php
@@ -43,7 +43,7 @@ class ShortUrlMetaInputFilter extends InputFilter
         $this->add($customSlug);
 
         $this->add($this->createPositiveNumberInput(self::MAX_VISITS));
-        $this->add($this->createPositiveNumberInput(self::SHORT_CODE_LENGTH));
+        $this->add($this->createPositiveNumberInput(self::SHORT_CODE_LENGTH, 4));
 
         $this->add($this->createBooleanInput(self::FIND_IF_EXISTS, false));
 
@@ -52,11 +52,11 @@ class ShortUrlMetaInputFilter extends InputFilter
         $this->add($domain);
     }
 
-    private function createPositiveNumberInput(string $name): Input
+    private function createPositiveNumberInput(string $name, int $min = 1): Input
     {
         $input = $this->createInput($name, false);
         $input->getValidatorChain()->attach(new Validator\Digits())
-              ->attach(new Validator\GreaterThan(['min' => 1, 'inclusive' => true]));
+              ->attach(new Validator\GreaterThan(['min' => $min, 'inclusive' => true]));
 
         return $input;
     }
diff --git a/module/Core/test/Entity/ShortUrlTest.php b/module/Core/test/Entity/ShortUrlTest.php
index 21c869aa..e410dedb 100644
--- a/module/Core/test/Entity/ShortUrlTest.php
+++ b/module/Core/test/Entity/ShortUrlTest.php
@@ -72,6 +72,6 @@ class ShortUrlTest extends TestCase
     public function provideLengths(): iterable
     {
         yield [null, DEFAULT_SHORT_CODES_LENGTH];
-        yield from map(range(1, 10), fn (int $value) => [$value, $value]);
+        yield from map(range(4, 10), fn (int $value) => [$value, $value]);
     }
 }
diff --git a/module/Core/test/Model/ShortUrlMetaTest.php b/module/Core/test/Model/ShortUrlMetaTest.php
index 13c5ae14..fed2c662 100644
--- a/module/Core/test/Model/ShortUrlMetaTest.php
+++ b/module/Core/test/Model/ShortUrlMetaTest.php
@@ -44,6 +44,9 @@ class ShortUrlMetaTest extends TestCase
             ShortUrlMetaInputFilter::VALID_UNTIL => 500,
             ShortUrlMetaInputFilter::DOMAIN => 4,
         ]];
+        yield [[
+            ShortUrlMetaInputFilter::SHORT_CODE_LENGTH => 3,
+        ]];
     }
 
     /** @test */

From 343ee04acbb17ffc38c94953ef0799a9eb360186 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Tue, 18 Feb 2020 19:21:34 +0100
Subject: [PATCH 15/67] Created middleware which injects default short code
 length from config when a value was not explicitly provided

---
 docs/swagger/paths/v1_short-urls.json         |  4 ++
 module/Rest/config/dependencies.config.php    |  4 ++
 module/Rest/config/routes.config.php          |  6 ++-
 .../DefaultShortCodesLengthMiddleware.php     | 31 +++++++++++
 .../DefaultShortCodesLengthMiddlewareTest.php | 54 +++++++++++++++++++
 5 files changed, 98 insertions(+), 1 deletion(-)
 create mode 100644 module/Rest/src/Middleware/ShortUrl/DefaultShortCodesLengthMiddleware.php
 create mode 100644 module/Rest/test/Middleware/ShortUrl/DefaultShortCodesLengthMiddlewareTest.php

diff --git a/docs/swagger/paths/v1_short-urls.json b/docs/swagger/paths/v1_short-urls.json
index be274ab6..ee8a6060 100644
--- a/docs/swagger/paths/v1_short-urls.json
+++ b/docs/swagger/paths/v1_short-urls.json
@@ -243,6 +243,10 @@
                             "domain": {
                                 "description": "The domain to which the short URL will be attached",
                                 "type": "string"
+                            },
+                            "shortCodeLength": {
+                                "description": "The length for generated short code. It has to be at least 4 and defaults to 5. It will be ignored when customSlug is provided",
+                                "type": "number"
                             }
                         }
                     }
diff --git a/module/Rest/config/dependencies.config.php b/module/Rest/config/dependencies.config.php
index dc4c0e3b..b24ec1ee 100644
--- a/module/Rest/config/dependencies.config.php
+++ b/module/Rest/config/dependencies.config.php
@@ -38,6 +38,7 @@ return [
             Middleware\CrossDomainMiddleware::class => InvokableFactory::class,
             Middleware\ShortUrl\CreateShortUrlContentNegotiationMiddleware::class => InvokableFactory::class,
             Middleware\ShortUrl\DropDefaultDomainFromRequestMiddleware::class => ConfigAbstractFactory::class,
+            Middleware\ShortUrl\DefaultShortCodesLengthMiddleware::class => ConfigAbstractFactory::class,
         ],
     ],
 
@@ -75,6 +76,9 @@ return [
         Action\Tag\UpdateTagAction::class => [Service\Tag\TagService::class, LoggerInterface::class],
 
         Middleware\ShortUrl\DropDefaultDomainFromRequestMiddleware::class => ['config.url_shortener.domain.hostname'],
+        Middleware\ShortUrl\DefaultShortCodesLengthMiddleware::class => [
+            'config.url_shortener.default_short_codes_length',
+        ],
     ],
 
 ];
diff --git a/module/Rest/config/routes.config.php b/module/Rest/config/routes.config.php
index 61abb1b7..b104d81b 100644
--- a/module/Rest/config/routes.config.php
+++ b/module/Rest/config/routes.config.php
@@ -13,7 +13,11 @@ return [
         Action\HealthAction::getRouteDef(),
 
         // Short codes
-        Action\ShortUrl\CreateShortUrlAction::getRouteDef([$contentNegotiationMiddleware, $dropDomainMiddleware]),
+        Action\ShortUrl\CreateShortUrlAction::getRouteDef([
+            $contentNegotiationMiddleware,
+            $dropDomainMiddleware,
+            Middleware\ShortUrl\DefaultShortCodesLengthMiddleware::class,
+        ]),
         Action\ShortUrl\SingleStepCreateShortUrlAction::getRouteDef([$contentNegotiationMiddleware]),
         Action\ShortUrl\EditShortUrlAction::getRouteDef([$dropDomainMiddleware]),
         Action\ShortUrl\DeleteShortUrlAction::getRouteDef([$dropDomainMiddleware]),
diff --git a/module/Rest/src/Middleware/ShortUrl/DefaultShortCodesLengthMiddleware.php b/module/Rest/src/Middleware/ShortUrl/DefaultShortCodesLengthMiddleware.php
new file mode 100644
index 00000000..bcad748e
--- /dev/null
+++ b/module/Rest/src/Middleware/ShortUrl/DefaultShortCodesLengthMiddleware.php
@@ -0,0 +1,31 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Shlinkio\Shlink\Rest\Middleware\ShortUrl;
+
+use Psr\Http\Message\ResponseInterface;
+use Psr\Http\Message\ServerRequestInterface;
+use Psr\Http\Server\MiddlewareInterface;
+use Psr\Http\Server\RequestHandlerInterface;
+use Shlinkio\Shlink\Core\Validation\ShortUrlMetaInputFilter;
+
+class DefaultShortCodesLengthMiddleware implements MiddlewareInterface
+{
+    private int $defaultShortCodesLength;
+
+    public function __construct(int $defaultShortCodesLength)
+    {
+        $this->defaultShortCodesLength = $defaultShortCodesLength;
+    }
+
+    public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface
+    {
+        $body = $request->getParsedBody();
+        if (! isset($body[ShortUrlMetaInputFilter::SHORT_CODE_LENGTH])) {
+            $body[ShortUrlMetaInputFilter::SHORT_CODE_LENGTH] = $this->defaultShortCodesLength;
+        }
+
+        return $handler->handle($request->withParsedBody($body));
+    }
+}
diff --git a/module/Rest/test/Middleware/ShortUrl/DefaultShortCodesLengthMiddlewareTest.php b/module/Rest/test/Middleware/ShortUrl/DefaultShortCodesLengthMiddlewareTest.php
new file mode 100644
index 00000000..38d875d9
--- /dev/null
+++ b/module/Rest/test/Middleware/ShortUrl/DefaultShortCodesLengthMiddlewareTest.php
@@ -0,0 +1,54 @@
+<?php
+
+declare(strict_types=1);
+
+namespace ShlinkioApiTest\Shlink\Rest\Middleware\ShortUrl;
+
+use Laminas\Diactoros\Response;
+use Laminas\Diactoros\ServerRequestFactory;
+use PHPUnit\Framework\Assert;
+use PHPUnit\Framework\TestCase;
+use Prophecy\Argument;
+use Prophecy\Prophecy\ObjectProphecy;
+use Psr\Http\Message\ServerRequestInterface;
+use Psr\Http\Server\RequestHandlerInterface;
+use Shlinkio\Shlink\Core\Validation\ShortUrlMetaInputFilter;
+use Shlinkio\Shlink\Rest\Middleware\ShortUrl\DefaultShortCodesLengthMiddleware;
+
+class DefaultShortCodesLengthMiddlewareTest extends TestCase
+{
+    private DefaultShortCodesLengthMiddleware $middleware;
+    private ObjectProphecy $handler;
+
+    public function setUp(): void
+    {
+        $this->handler = $this->prophesize(RequestHandlerInterface::class);
+        $this->middleware = new DefaultShortCodesLengthMiddleware(8);
+    }
+
+    /**
+     * @test
+     * @dataProvider provideBodies
+     */
+    public function defaultValueIsInjectedInBodyWhenNotProvided(array $body, int $expectedLength): void
+    {
+        $request = ServerRequestFactory::fromGlobals()->withParsedBody($body);
+        $handle = $this->handler->handle(Argument::that(function (ServerRequestInterface $req) use ($expectedLength) {
+            $parsedBody = $req->getParsedBody();
+            Assert::assertArrayHasKey(ShortUrlMetaInputFilter::SHORT_CODE_LENGTH, $parsedBody);
+            Assert::assertEquals($expectedLength, $parsedBody[ShortUrlMetaInputFilter::SHORT_CODE_LENGTH]);
+
+            return $req;
+        }))->willReturn(new Response());
+
+        $this->middleware->process($request, $this->handler->reveal());
+
+        $handle->shouldHaveBeenCalledOnce();
+    }
+
+    public function provideBodies(): iterable
+    {
+        yield 'value provided' => [[ShortUrlMetaInputFilter::SHORT_CODE_LENGTH => 6], 6];
+        yield 'value not provided' => [[], 8];
+    }
+}

From 51e130c7a0d7330c783a0ea4911b9b46156bf3d3 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Tue, 18 Feb 2020 19:34:01 +0100
Subject: [PATCH 16/67] Added env var that can be used to define default short
 codes length on docker image

---
 docker/README.md                                       |  3 +++
 docker/config/shlink_in_docker.local.php               | 10 ++++++++++
 module/Core/functions/functions.php                    |  1 +
 module/Core/src/Config/SimplifiedConfigParser.php      |  1 +
 module/Core/src/Validation/ShortUrlMetaInputFilter.php |  4 +++-
 module/Core/test/Config/SimplifiedConfigParserTest.php |  2 ++
 6 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/docker/README.md b/docker/README.md
index 76c47a09..699c1c75 100644
--- a/docker/README.md
+++ b/docker/README.md
@@ -113,6 +113,7 @@ This is the complete list of supported env vars:
 * `WEB_WORKER_NUM`: The amount of concurrent http requests this shlink instance will be able to server. Defaults to 16.
 * `TASK_WORKER_NUM`: The amount of concurrent background tasks this shlink instance will be able to execute. Defaults to 16.
 * `VISITS_WEBHOOKS`: A comma-separated list of URLs that will receive a `POST` request when a short URL receives a visit.
+* `DEFAULT_SHORT_CODES_LENGTH`: The length you want generated short codes to have. It defaults to 5 and has to be at least 4, so any value smaller than that will fall back to 4.
 * `REDIS_SERVERS`: A comma-separated list of redis servers where Shlink locks are stored (locks are used to prevent some operations to be run more than once in parallel).
 
     This is important when running more than one Shlink instance ([Multi instance considerations](#multi-instance-considerations)). If not provided, Shlink stores locks on every instance separately.
@@ -146,6 +147,7 @@ docker run \
     -e WEB_WORKER_NUM=64 \
     -e TASK_WORKER_NUM=32 \
     -e "VISITS_WEBHOOKS=http://my-api.com/api/v2.3/notify,https://third-party.io/foo" \
+    -e DEFAULT_SHORT_CODES_LENGTH=6 \
     shlinkio/shlink:stable
 ```
 
@@ -170,6 +172,7 @@ The whole configuration should have this format, but it can be split into multip
     "base_path": "/my-campaign",
     "web_worker_num": 64,
     "task_worker_num": 32,
+    "default_short_codes_length": 6,
     "redis_servers": [
         "tcp://172.20.0.1:6379",
         "tcp://172.20.0.2:6379"
diff --git a/docker/config/shlink_in_docker.local.php b/docker/config/shlink_in_docker.local.php
index 9e10d419..6cf86434 100644
--- a/docker/config/shlink_in_docker.local.php
+++ b/docker/config/shlink_in_docker.local.php
@@ -11,6 +11,9 @@ use function explode;
 use function Functional\contains;
 use function Shlinkio\Shlink\Common\env;
 
+use const Shlinkio\Shlink\Core\DEFAULT_SHORT_CODES_LENGTH;
+use const Shlinkio\Shlink\Core\MIN_SHORT_CODES_LENGTH;
+
 $helper = new class {
     private const DB_DRIVERS_MAP = [
         'mysql' => 'pdo_mysql',
@@ -70,6 +73,12 @@ $helper = new class {
         $redisServers = env('REDIS_SERVERS');
         return $redisServers === null ? null : ['servers' => $redisServers];
     }
+
+    public function getDefaultShortCodesLength(): int
+    {
+        $value = (int) env('DEFAULT_SHORT_CODES_LENGTH', DEFAULT_SHORT_CODES_LENGTH);
+        return $value < MIN_SHORT_CODES_LENGTH ? MIN_SHORT_CODES_LENGTH : $value;
+    }
 };
 
 return [
@@ -96,6 +105,7 @@ return [
         ],
         'validate_url' => (bool) env('VALIDATE_URLS', false),
         'visits_webhooks' => $helper->getVisitsWebhooks(),
+        'default_short_codes_length' => $helper->getDefaultShortCodesLength(),
     ],
 
     'not_found_redirects' => $helper->getNotFoundRedirectsConfig(),
diff --git a/module/Core/functions/functions.php b/module/Core/functions/functions.php
index 61d0be1e..87399208 100644
--- a/module/Core/functions/functions.php
+++ b/module/Core/functions/functions.php
@@ -11,6 +11,7 @@ use PUGX\Shortid\Factory as ShortIdFactory;
 use function sprintf;
 
 const DEFAULT_SHORT_CODES_LENGTH = 5;
+const MIN_SHORT_CODES_LENGTH = 4;
 
 function generateRandomShortCode(int $length): string
 {
diff --git a/module/Core/src/Config/SimplifiedConfigParser.php b/module/Core/src/Config/SimplifiedConfigParser.php
index fa7a4acb..a03ccc3e 100644
--- a/module/Core/src/Config/SimplifiedConfigParser.php
+++ b/module/Core/src/Config/SimplifiedConfigParser.php
@@ -32,6 +32,7 @@ class SimplifiedConfigParser
         'web_worker_num' => ['mezzio-swoole', 'swoole-http-server', 'options', 'worker_num'],
         'task_worker_num' => ['mezzio-swoole', 'swoole-http-server', 'options', 'task_worker_num'],
         'visits_webhooks' => ['url_shortener', 'visits_webhooks'],
+        'default_short_codes_length' => ['url_shortener', 'default_short_codes_length'],
     ];
     private const SIMPLIFIED_CONFIG_SIDE_EFFECTS = [
         'delete_short_url_threshold' => [
diff --git a/module/Core/src/Validation/ShortUrlMetaInputFilter.php b/module/Core/src/Validation/ShortUrlMetaInputFilter.php
index 9e8b1c7a..a71b4cc2 100644
--- a/module/Core/src/Validation/ShortUrlMetaInputFilter.php
+++ b/module/Core/src/Validation/ShortUrlMetaInputFilter.php
@@ -10,6 +10,8 @@ use Laminas\InputFilter\InputFilter;
 use Laminas\Validator;
 use Shlinkio\Shlink\Common\Validation;
 
+use const Shlinkio\Shlink\Core\MIN_SHORT_CODES_LENGTH;
+
 class ShortUrlMetaInputFilter extends InputFilter
 {
     use Validation\InputFactoryTrait;
@@ -43,7 +45,7 @@ class ShortUrlMetaInputFilter extends InputFilter
         $this->add($customSlug);
 
         $this->add($this->createPositiveNumberInput(self::MAX_VISITS));
-        $this->add($this->createPositiveNumberInput(self::SHORT_CODE_LENGTH, 4));
+        $this->add($this->createPositiveNumberInput(self::SHORT_CODE_LENGTH, MIN_SHORT_CODES_LENGTH));
 
         $this->add($this->createBooleanInput(self::FIND_IF_EXISTS, false));
 
diff --git a/module/Core/test/Config/SimplifiedConfigParserTest.php b/module/Core/test/Config/SimplifiedConfigParserTest.php
index 1d4f3b8d..7a304ad5 100644
--- a/module/Core/test/Config/SimplifiedConfigParserTest.php
+++ b/module/Core/test/Config/SimplifiedConfigParserTest.php
@@ -57,6 +57,7 @@ class SimplifiedConfigParserTest extends TestCase
                 'http://my-api.com/api/v2.3/notify',
                 'https://third-party.io/foo',
             ],
+            'default_short_codes_length' => 8,
         ];
         $expected = [
             'app_options' => [
@@ -84,6 +85,7 @@ class SimplifiedConfigParserTest extends TestCase
                     'http://my-api.com/api/v2.3/notify',
                     'https://third-party.io/foo',
                 ],
+                'default_short_codes_length' => 8,
             ],
 
             'delete_short_urls' => [

From 33a404f051835998a50cfcfbba7393ccd70efe1e Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Tue, 18 Feb 2020 20:34:48 +0100
Subject: [PATCH 17/67] Updated CLI command to create short URLs so that it
 respects configs for short code length

---
 composer.json                                        |  2 +-
 config/autoload/installer.global.php                 |  1 +
 module/CLI/config/dependencies.config.php            |  6 +++++-
 .../src/Command/ShortUrl/GenerateShortUrlCommand.php | 12 +++++++++++-
 .../Command/ShortUrl/GenerateShortUrlCommandTest.php |  2 +-
 5 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/composer.json b/composer.json
index cdda9028..52681a0e 100644
--- a/composer.json
+++ b/composer.json
@@ -49,7 +49,7 @@
         "pugx/shortid-php": "^0.5",
         "shlinkio/shlink-common": "^2.7.0",
         "shlinkio/shlink-event-dispatcher": "^1.3",
-        "shlinkio/shlink-installer": "^4.1.0",
+        "shlinkio/shlink-installer": "^4.2.0",
         "shlinkio/shlink-ip-geolocation": "^1.3.1",
         "symfony/console": "^5.0",
         "symfony/filesystem": "^5.0",
diff --git a/config/autoload/installer.global.php b/config/autoload/installer.global.php
index 296c0635..c40d75d1 100644
--- a/config/autoload/installer.global.php
+++ b/config/autoload/installer.global.php
@@ -30,6 +30,7 @@ return [
             Option\TaskWorkerNumConfigOption::class,
             Option\WebWorkerNumConfigOption::class,
             Option\RedisServersConfigOption::class,
+            Option\ShortCodeLengthOption::class,
         ],
 
         'installation_commands' => [
diff --git a/module/CLI/config/dependencies.config.php b/module/CLI/config/dependencies.config.php
index 1f94f5a6..1cc67fd9 100644
--- a/module/CLI/config/dependencies.config.php
+++ b/module/CLI/config/dependencies.config.php
@@ -54,7 +54,11 @@ return [
     ConfigAbstractFactory::class => [
         GeolocationDbUpdater::class => [DbUpdater::class, Reader::class, 'Shlinkio\Shlink\LocalLockFactory'],
 
-        Command\ShortUrl\GenerateShortUrlCommand::class => [Service\UrlShortener::class, 'config.url_shortener.domain'],
+        Command\ShortUrl\GenerateShortUrlCommand::class => [
+            Service\UrlShortener::class,
+            'config.url_shortener.domain',
+            'config.url_shortener.default_short_codes_length',
+        ],
         Command\ShortUrl\ResolveUrlCommand::class => [Service\ShortUrl\ShortUrlResolver::class],
         Command\ShortUrl\ListShortUrlsCommand::class => [Service\ShortUrlService::class, 'config.url_shortener.domain'],
         Command\ShortUrl\GetVisitsCommand::class => [Service\VisitsTracker::class],
diff --git a/module/CLI/src/Command/ShortUrl/GenerateShortUrlCommand.php b/module/CLI/src/Command/ShortUrl/GenerateShortUrlCommand.php
index 28d192b1..7369f1f6 100644
--- a/module/CLI/src/Command/ShortUrl/GenerateShortUrlCommand.php
+++ b/module/CLI/src/Command/ShortUrl/GenerateShortUrlCommand.php
@@ -30,12 +30,14 @@ class GenerateShortUrlCommand extends Command
 
     private UrlShortenerInterface $urlShortener;
     private array $domainConfig;
+    private int $defaultShortCodeLength;
 
-    public function __construct(UrlShortenerInterface $urlShortener, array $domainConfig)
+    public function __construct(UrlShortenerInterface $urlShortener, array $domainConfig, int $defaultShortCodeLength)
     {
         parent::__construct();
         $this->urlShortener = $urlShortener;
         $this->domainConfig = $domainConfig;
+        $this->defaultShortCodeLength = $defaultShortCodeLength;
     }
 
     protected function configure(): void
@@ -87,6 +89,12 @@ class GenerateShortUrlCommand extends Command
                 'd',
                 InputOption::VALUE_REQUIRED,
                 'The domain to which this short URL will be attached.',
+            )
+            ->addOption(
+                'shortCodeLength',
+                'l',
+                InputOption::VALUE_REQUIRED,
+                'The length for generated short code (it will be ignored if --customSlug was provided).',
             );
     }
 
@@ -117,6 +125,7 @@ class GenerateShortUrlCommand extends Command
         $tags = unique(flatten(array_map($explodeWithComma, $input->getOption('tags'))));
         $customSlug = $input->getOption('customSlug');
         $maxVisits = $input->getOption('maxVisits');
+        $shortCodeLength = $input->getOption('shortCodeLength') ?? $this->defaultShortCodeLength;
 
         try {
             $shortUrl = $this->urlShortener->urlToShortCode(
@@ -129,6 +138,7 @@ class GenerateShortUrlCommand extends Command
                     ShortUrlMetaInputFilter::MAX_VISITS => $maxVisits !== null ? (int) $maxVisits : null,
                     ShortUrlMetaInputFilter::FIND_IF_EXISTS => $input->getOption('findIfExists'),
                     ShortUrlMetaInputFilter::DOMAIN => $input->getOption('domain'),
+                    ShortUrlMetaInputFilter::SHORT_CODE_LENGTH => $shortCodeLength,
                 ]),
             );
 
diff --git a/module/CLI/test/Command/ShortUrl/GenerateShortUrlCommandTest.php b/module/CLI/test/Command/ShortUrl/GenerateShortUrlCommandTest.php
index df1019b1..bcf00acb 100644
--- a/module/CLI/test/Command/ShortUrl/GenerateShortUrlCommandTest.php
+++ b/module/CLI/test/Command/ShortUrl/GenerateShortUrlCommandTest.php
@@ -31,7 +31,7 @@ class GenerateShortUrlCommandTest extends TestCase
     public function setUp(): void
     {
         $this->urlShortener = $this->prophesize(UrlShortener::class);
-        $command = new GenerateShortUrlCommand($this->urlShortener->reveal(), self::DOMAIN_CONFIG);
+        $command = new GenerateShortUrlCommand($this->urlShortener->reveal(), self::DOMAIN_CONFIG, 5);
         $app = new Application();
         $app->add($command);
         $this->commandTester = new CommandTester($command);

From 1f3e0d1f735b464cb4fd7f4e5390bf0cca6d5446 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Tue, 18 Feb 2020 20:35:41 +0100
Subject: [PATCH 18/67] Updated changelog

---
 CHANGELOG.md | 1 +
 1 file changed, 1 insertion(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index edf73367..139a5877 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -9,6 +9,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this
 #### Added
 
 * [#626](https://github.com/shlinkio/shlink/issues/626) Added support for Microsoft SQL Server.
+* [#556](https://github.com/shlinkio/shlink/issues/556) Short code lengths can now be customized, both globally and on a per-short URL basis.
 
 #### Changed
 

From bb231e668b53ffa98ce620c613fee502243c900f Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Wed, 19 Feb 2020 18:58:22 +0100
Subject: [PATCH 19/67] Registered middleware generating request ID

---
 composer.json                                 |  2 ++
 .../autoload/middleware-pipeline.global.php   |  2 ++
 config/autoload/request_id.global.php         | 33 +++++++++++++++++++
 module/Core/src/Options/AppOptions.php        |  3 --
 module/Rest/src/Entity/ApiKey.php             | 12 ++-----
 5 files changed, 40 insertions(+), 12 deletions(-)
 create mode 100644 config/autoload/request_id.global.php

diff --git a/composer.json b/composer.json
index 52681a0e..7b8bf39e 100644
--- a/composer.json
+++ b/composer.json
@@ -45,8 +45,10 @@
         "nikolaposa/monolog-factory": "^3.0",
         "ocramius/proxy-manager": "^2.7.0",
         "phly/phly-event-dispatcher": "^1.0",
+        "php-middleware/request-id": "^4.0",
         "predis/predis": "^1.1",
         "pugx/shortid-php": "^0.5",
+        "ramsey/uuid": "^3.9",
         "shlinkio/shlink-common": "^2.7.0",
         "shlinkio/shlink-event-dispatcher": "^1.3",
         "shlinkio/shlink-installer": "^4.2.0",
diff --git a/config/autoload/middleware-pipeline.global.php b/config/autoload/middleware-pipeline.global.php
index 2ace0700..45abf30e 100644
--- a/config/autoload/middleware-pipeline.global.php
+++ b/config/autoload/middleware-pipeline.global.php
@@ -7,6 +7,7 @@ namespace Shlinkio\Shlink;
 use Laminas\Stratigility\Middleware\ErrorHandler;
 use Mezzio;
 use Mezzio\ProblemDetails;
+use PhpMiddleware\RequestId\RequestIdMiddleware;
 
 return [
 
@@ -21,6 +22,7 @@ return [
             'path' => '/rest',
             'middleware' => [
                 Rest\Middleware\CrossDomainMiddleware::class,
+                RequestIdMiddleware::class,
                 ProblemDetails\ProblemDetailsMiddleware::class,
             ],
         ],
diff --git a/config/autoload/request_id.global.php b/config/autoload/request_id.global.php
new file mode 100644
index 00000000..87d1c72f
--- /dev/null
+++ b/config/autoload/request_id.global.php
@@ -0,0 +1,33 @@
+<?php
+
+declare(strict_types=1);
+
+use Laminas\ServiceManager\AbstractFactory\ConfigAbstractFactory;
+use Laminas\ServiceManager\Factory\InvokableFactory;
+use PhpMiddleware\RequestId;
+
+return [
+
+    'request_id' => [
+        'allow_override' => true,
+        'header_name' => 'X-Request-Id',
+    ],
+
+    'dependencies' => [
+        'factories' => [
+            RequestId\Generator\RamseyUuid4StaticGenerator::class => InvokableFactory::class,
+            RequestId\RequestIdProviderFactory::class => ConfigAbstractFactory::class,
+            RequestId\RequestIdMiddleware::class => ConfigAbstractFactory::class,
+        ],
+    ],
+
+    ConfigAbstractFactory::class => [
+        RequestId\RequestIdProviderFactory::class => [
+            RequestId\Generator\RamseyUuid4StaticGenerator::class,
+            'config.request_id.allow_override',
+            'config.request_id.header_name',
+        ],
+        RequestId\RequestIdMiddleware::class => [RequestId\RequestIdProviderFactory::class],
+    ],
+
+];
diff --git a/module/Core/src/Options/AppOptions.php b/module/Core/src/Options/AppOptions.php
index aa51b871..66d76126 100644
--- a/module/Core/src/Options/AppOptions.php
+++ b/module/Core/src/Options/AppOptions.php
@@ -5,14 +5,11 @@ declare(strict_types=1);
 namespace Shlinkio\Shlink\Core\Options;
 
 use Laminas\Stdlib\AbstractOptions;
-use Shlinkio\Shlink\Common\Util\StringUtilsTrait;
 
 use function sprintf;
 
 class AppOptions extends AbstractOptions
 {
-    use StringUtilsTrait;
-
     private string $name = '';
     private string $version = '1.0';
     private ?string $disableTrackParam = null;
diff --git a/module/Rest/src/Entity/ApiKey.php b/module/Rest/src/Entity/ApiKey.php
index 8c6d3aeb..f23d22e1 100644
--- a/module/Rest/src/Entity/ApiKey.php
+++ b/module/Rest/src/Entity/ApiKey.php
@@ -5,20 +5,18 @@ declare(strict_types=1);
 namespace Shlinkio\Shlink\Rest\Entity;
 
 use Cake\Chronos\Chronos;
+use Ramsey\Uuid\Uuid;
 use Shlinkio\Shlink\Common\Entity\AbstractEntity;
-use Shlinkio\Shlink\Common\Util\StringUtilsTrait;
 
 class ApiKey extends AbstractEntity
 {
-    use StringUtilsTrait;
-
     private string $key;
     private ?Chronos $expirationDate;
     private bool $enabled;
 
     public function __construct(?Chronos $expirationDate = null)
     {
-        $this->key = $this->generateV4Uuid();
+        $this->key = Uuid::uuid4()->toString();
         $this->expirationDate = $expirationDate;
         $this->enabled = true;
     }
@@ -30,11 +28,7 @@ class ApiKey extends AbstractEntity
 
     public function isExpired(): bool
     {
-        if ($this->expirationDate === null) {
-            return false;
-        }
-
-        return $this->expirationDate->lt(Chronos::now());
+        return $this->expirationDate !== null && $this->expirationDate->lt(Chronos::now());
     }
 
     public function isEnabled(): bool

From d0a986dd5ae39f2ec907f43320e0f2641974469e Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Wed, 19 Feb 2020 19:37:47 +0100
Subject: [PATCH 20/67] Added request ID to logs with monolog

---
 config/autoload/logger.global.php     | 4 +++-
 config/autoload/request_id.global.php | 2 ++
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/config/autoload/logger.global.php b/config/autoload/logger.global.php
index 879f700a..16515c9e 100644
--- a/config/autoload/logger.global.php
+++ b/config/autoload/logger.global.php
@@ -9,6 +9,7 @@ use Monolog\Handler;
 use Monolog\Logger;
 use Monolog\Processor;
 use MonologFactory\DiContainerLoggerFactory;
+use PhpMiddleware\RequestId;
 use Psr\Log\LoggerInterface;
 
 use const PHP_EOL;
@@ -20,11 +21,12 @@ $processors = [
     'psr3' => [
         'name' => Processor\PsrLogMessageProcessor::class,
     ],
+    'request_id' => RequestId\MonologProcessor::class,
 ];
 $formatter = [
     'name' => Formatter\LineFormatter::class,
     'params' => [
-        'format' => '[%datetime%] %channel%.%level_name% - %message%' . PHP_EOL,
+        'format' => '[%datetime%] [%extra.request_id%] %channel%.%level_name% - %message%' . PHP_EOL,
         'allow_inline_line_breaks' => true,
     ],
 ];
diff --git a/config/autoload/request_id.global.php b/config/autoload/request_id.global.php
index 87d1c72f..0a9ed6ce 100644
--- a/config/autoload/request_id.global.php
+++ b/config/autoload/request_id.global.php
@@ -18,6 +18,7 @@ return [
             RequestId\Generator\RamseyUuid4StaticGenerator::class => InvokableFactory::class,
             RequestId\RequestIdProviderFactory::class => ConfigAbstractFactory::class,
             RequestId\RequestIdMiddleware::class => ConfigAbstractFactory::class,
+            RequestId\MonologProcessor::class => ConfigAbstractFactory::class,
         ],
     ],
 
@@ -28,6 +29,7 @@ return [
             'config.request_id.header_name',
         ],
         RequestId\RequestIdMiddleware::class => [RequestId\RequestIdProviderFactory::class],
+        RequestId\MonologProcessor::class => [RequestId\RequestIdMiddleware::class],
     ],
 
 ];

From fb89cb80ac1352323d0ce0261ec8a8f052bbb164 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Wed, 19 Feb 2020 19:48:48 +0100
Subject: [PATCH 21/67] Added formatting to swoole logs, to avoid duplicating
 the date

---
 config/autoload/logger.global.php | 1 +
 1 file changed, 1 insertion(+)

diff --git a/config/autoload/logger.global.php b/config/autoload/logger.global.php
index 16515c9e..7c0e4f00 100644
--- a/config/autoload/logger.global.php
+++ b/config/autoload/logger.global.php
@@ -82,6 +82,7 @@ return [
         'swoole-http-server' => [
             'logger' => [
                 'logger-name' => 'Logger_Access',
+                'format' => '%h %l %u "%r" %>s %b',
             ],
         ],
     ],

From b728a7867393ca56ccc938dc813c8072e8c5a2ec Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Fri, 21 Feb 2020 19:47:30 +0100
Subject: [PATCH 22/67] Updated changelog

---
 CHANGELOG.md | 1 +
 1 file changed, 1 insertion(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 139a5877..30c989a2 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -10,6 +10,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this
 
 * [#626](https://github.com/shlinkio/shlink/issues/626) Added support for Microsoft SQL Server.
 * [#556](https://github.com/shlinkio/shlink/issues/556) Short code lengths can now be customized, both globally and on a per-short URL basis.
+* [#541](https://github.com/shlinkio/shlink/issues/541) Added a request ID that is returned on `X-Request-Id` header, can be provided from outside and is set in log entries.
 
 #### Changed
 

From 6b1dadc35c790afb46aec8799e81f108edb1857e Mon Sep 17 00:00:00 2001
From: Jordan Patterson <jordan@jpatterson.me>
Date: Mon, 24 Feb 2020 22:53:07 -0800
Subject: [PATCH 23/67] fixed incorrect configuration option for
 base_url_redirect_to

---
 module/Core/src/Config/SimplifiedConfigParser.php | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/module/Core/src/Config/SimplifiedConfigParser.php b/module/Core/src/Config/SimplifiedConfigParser.php
index a03ccc3e..6181e27a 100644
--- a/module/Core/src/Config/SimplifiedConfigParser.php
+++ b/module/Core/src/Config/SimplifiedConfigParser.php
@@ -24,7 +24,7 @@ class SimplifiedConfigParser
         'validate_url' => ['url_shortener', 'validate_url'],
         'invalid_short_url_redirect_to' => ['not_found_redirects', 'invalid_short_url'],
         'regular_404_redirect_to' => ['not_found_redirects', 'regular_404'],
-        'base_url_redirect_to' => ['not_found_redirects', 'base_path'],
+        'base_url_redirect_to' => ['not_found_redirects', 'base_url'],
         'db_config' => ['entity_manager', 'connection'],
         'delete_short_url_threshold' => ['delete_short_urls', 'visits_threshold'],
         'redis_servers' => ['cache', 'redis', 'servers'],

From 590fc3fc92037fc9e5a6276d9fdd37c3034483d0 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Tue, 25 Feb 2020 18:01:06 +0100
Subject: [PATCH 24/67] Added tests covering redirect simplified config parsing

---
 module/Core/test/Config/SimplifiedConfigParserTest.php | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/module/Core/test/Config/SimplifiedConfigParserTest.php b/module/Core/test/Config/SimplifiedConfigParserTest.php
index 7a304ad5..02f96423 100644
--- a/module/Core/test/Config/SimplifiedConfigParserTest.php
+++ b/module/Core/test/Config/SimplifiedConfigParserTest.php
@@ -41,6 +41,8 @@ class SimplifiedConfigParserTest extends TestCase
             'validate_url' => true,
             'delete_short_url_threshold' => 50,
             'invalid_short_url_redirect_to' => 'foobar.com',
+            'regular_404_redirect_to' => 'bar.com',
+            'base_url_redirect_to' => 'foo.com',
             'redis_servers' => [
                 'tcp://1.1.1.1:1111',
                 'tcp://1.2.2.2:2222',
@@ -114,6 +116,8 @@ class SimplifiedConfigParserTest extends TestCase
 
             'not_found_redirects' => [
                 'invalid_short_url' => 'foobar.com',
+                'regular_404' => 'bar.com',
+                'base_url' => 'foo.com',
             ],
 
             'mezzio-swoole' => [

From 8a0e902bdd337158b9215fb20ecbcf8bd7ad44ac Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Tue, 25 Feb 2020 18:02:38 +0100
Subject: [PATCH 25/67] Updated changelog

---
 CHANGELOG.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 30c989a2..a75e39ed 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -26,7 +26,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this
 
 #### Fixed
 
-* *Nothing*
+* [#665](https://github.com/shlinkio/shlink/issues/665) Fixed `base_url_redirect_to` simplified config option not being properly parsed.
 
 
 ## 2.0.5 - 2020-02-09

From 67e93a68749311ab99696adda81fa6067021551a Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Fri, 6 Mar 2020 19:20:33 +0100
Subject: [PATCH 26/67] Ensured empty values cannot be provided as the custom
 slug

---
 module/Core/src/Validation/ShortUrlMetaInputFilter.php | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/module/Core/src/Validation/ShortUrlMetaInputFilter.php b/module/Core/src/Validation/ShortUrlMetaInputFilter.php
index a71b4cc2..604f2648 100644
--- a/module/Core/src/Validation/ShortUrlMetaInputFilter.php
+++ b/module/Core/src/Validation/ShortUrlMetaInputFilter.php
@@ -40,8 +40,11 @@ class ShortUrlMetaInputFilter extends InputFilter
         $validUntil->getValidatorChain()->attach(new Validator\Date(['format' => DateTime::ATOM]));
         $this->add($validUntil);
 
-        $customSlug = $this->createInput(self::CUSTOM_SLUG, false);
+        // FIXME The only way to enforce the NotEmpty validator to be evaluated when the value is provided but it's
+        //       empty, is by using the deprecated setContinueIfEmpty
+        $customSlug = $this->createInput(self::CUSTOM_SLUG, false)->setContinueIfEmpty(true);
         $customSlug->getFilterChain()->attach(new Validation\SluggerFilter());
+        $customSlug->getValidatorChain()->attach(new Validator\NotEmpty());
         $this->add($customSlug);
 
         $this->add($this->createPositiveNumberInput(self::MAX_VISITS));
@@ -58,7 +61,7 @@ class ShortUrlMetaInputFilter extends InputFilter
     {
         $input = $this->createInput($name, false);
         $input->getValidatorChain()->attach(new Validator\Digits())
-              ->attach(new Validator\GreaterThan(['min' => $min, 'inclusive' => true]));
+                                   ->attach(new Validator\GreaterThan(['min' => $min, 'inclusive' => true]));
 
         return $input;
     }

From 18ceafeb60eda8b9b552694781d3d445487aad14 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Fri, 6 Mar 2020 19:25:05 +0100
Subject: [PATCH 27/67] Ensured only empty strings are checked while verifying
 empty value on custom slug

---
 module/Core/src/Validation/ShortUrlMetaInputFilter.php | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/module/Core/src/Validation/ShortUrlMetaInputFilter.php b/module/Core/src/Validation/ShortUrlMetaInputFilter.php
index 604f2648..6237daf7 100644
--- a/module/Core/src/Validation/ShortUrlMetaInputFilter.php
+++ b/module/Core/src/Validation/ShortUrlMetaInputFilter.php
@@ -44,7 +44,7 @@ class ShortUrlMetaInputFilter extends InputFilter
         //       empty, is by using the deprecated setContinueIfEmpty
         $customSlug = $this->createInput(self::CUSTOM_SLUG, false)->setContinueIfEmpty(true);
         $customSlug->getFilterChain()->attach(new Validation\SluggerFilter());
-        $customSlug->getValidatorChain()->attach(new Validator\NotEmpty());
+        $customSlug->getValidatorChain()->attach(new Validator\NotEmpty(Validator\NotEmpty::STRING));
         $this->add($customSlug);
 
         $this->add($this->createPositiveNumberInput(self::MAX_VISITS));

From f5c1e12db46bed5e1ff3e67f72964de70d56c20d Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Fri, 6 Mar 2020 20:01:41 +0100
Subject: [PATCH 28/67] Added more tests covering invalid custom slugs

---
 CHANGELOG.md                                           | 1 +
 composer.json                                          | 2 +-
 module/Core/src/Validation/ShortUrlMetaInputFilter.php | 5 ++++-
 module/Core/test/Model/ShortUrlMetaTest.php            | 9 +++++++++
 4 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index a75e39ed..5eecc684 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -27,6 +27,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this
 #### Fixed
 
 * [#665](https://github.com/shlinkio/shlink/issues/665) Fixed `base_url_redirect_to` simplified config option not being properly parsed.
+* [#663](https://github.com/shlinkio/shlink/issues/663) Fixed Shlink allowing short URLs to be created with an empty custom slug.
 
 
 ## 2.0.5 - 2020-02-09
diff --git a/composer.json b/composer.json
index 7b8bf39e..84707645 100644
--- a/composer.json
+++ b/composer.json
@@ -49,7 +49,7 @@
         "predis/predis": "^1.1",
         "pugx/shortid-php": "^0.5",
         "ramsey/uuid": "^3.9",
-        "shlinkio/shlink-common": "^2.7.0",
+        "shlinkio/shlink-common": "^2.8.0",
         "shlinkio/shlink-event-dispatcher": "^1.3",
         "shlinkio/shlink-installer": "^4.2.0",
         "shlinkio/shlink-ip-geolocation": "^1.3.1",
diff --git a/module/Core/src/Validation/ShortUrlMetaInputFilter.php b/module/Core/src/Validation/ShortUrlMetaInputFilter.php
index 6237daf7..013edd8f 100644
--- a/module/Core/src/Validation/ShortUrlMetaInputFilter.php
+++ b/module/Core/src/Validation/ShortUrlMetaInputFilter.php
@@ -44,7 +44,10 @@ class ShortUrlMetaInputFilter extends InputFilter
         //       empty, is by using the deprecated setContinueIfEmpty
         $customSlug = $this->createInput(self::CUSTOM_SLUG, false)->setContinueIfEmpty(true);
         $customSlug->getFilterChain()->attach(new Validation\SluggerFilter());
-        $customSlug->getValidatorChain()->attach(new Validator\NotEmpty(Validator\NotEmpty::STRING));
+        $customSlug->getValidatorChain()->attach(new Validator\NotEmpty([
+            Validator\NotEmpty::STRING,
+            Validator\NotEmpty::SPACE,
+        ]));
         $this->add($customSlug);
 
         $this->add($this->createPositiveNumberInput(self::MAX_VISITS));
diff --git a/module/Core/test/Model/ShortUrlMetaTest.php b/module/Core/test/Model/ShortUrlMetaTest.php
index fed2c662..7d0dd9b6 100644
--- a/module/Core/test/Model/ShortUrlMetaTest.php
+++ b/module/Core/test/Model/ShortUrlMetaTest.php
@@ -47,6 +47,15 @@ class ShortUrlMetaTest extends TestCase
         yield [[
             ShortUrlMetaInputFilter::SHORT_CODE_LENGTH => 3,
         ]];
+        yield [[
+            ShortUrlMetaInputFilter::CUSTOM_SLUG => '/',
+        ]];
+        yield [[
+            ShortUrlMetaInputFilter::CUSTOM_SLUG => '',
+        ]];
+        yield [[
+            ShortUrlMetaInputFilter::CUSTOM_SLUG => '   ',
+        ]];
     }
 
     /** @test */

From d9fee5582ab328d2d2d1d813c3d235ab734e2d53 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Sat, 7 Mar 2020 08:44:12 +0100
Subject: [PATCH 29/67] Added docker pulls badge to main readme file

---
 README.md | 1 +
 1 file changed, 1 insertion(+)

diff --git a/README.md b/README.md
index eb9151db..5ae374a6 100644
--- a/README.md
+++ b/README.md
@@ -4,6 +4,7 @@
 [![Code Coverage](https://img.shields.io/scrutinizer/coverage/g/shlinkio/shlink.svg?style=flat-square)](https://scrutinizer-ci.com/g/shlinkio/shlink/?branch=master)
 [![Scrutinizer Code Quality](https://img.shields.io/scrutinizer/g/shlinkio/shlink.svg?style=flat-square)](https://scrutinizer-ci.com/g/shlinkio/shlink/?branch=master)
 [![Latest Stable Version](https://img.shields.io/github/release/shlinkio/shlink.svg?style=flat-square)](https://packagist.org/packages/shlinkio/shlink)
+[![Docker pulls](https://img.shields.io/docker/pulls/shlinkio/shlink.svg?style=flat-square)](https://hub.docker.com/r/shlinkio/shlink/)
 [![License](https://img.shields.io/github/license/shlinkio/shlink.svg?style=flat-square)](https://github.com/shlinkio/shlink/blob/master/LICENSE)
 [![Paypal donate](https://img.shields.io/badge/Donate-paypal-blue.svg?style=flat-square&logo=paypal&colorA=aaaaaa)](https://acel.me/donate)
 

From ba8b041698eec9aed95e2378464a1fac7af9b5b1 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Tue, 10 Mar 2020 21:45:20 +0100
Subject: [PATCH 30/67] Updated API docs links

---
 README.md                 | 2 +-
 docs/swagger/swagger.json | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/README.md b/README.md
index eb9151db..b9260735 100644
--- a/README.md
+++ b/README.md
@@ -238,7 +238,7 @@ Once shlink is installed, there are two main ways to interact with it:
 
     It is probably a good idea to symlink the CLI entry point (`bin/cli`) to somewhere in your path, so that you can run shlink from any directory.
 
-* **The REST API**. The complete docs on how to use the API can be found [here](https://shlink.io/api-docs), and a sandbox which also documents every endpoint can be found in the [API Spec](https://api-spec.shlink.io/) portal.
+* **The REST API**. The complete docs on how to use the API can be found [here](https://shlink.io/documentation/api-docs), and a sandbox which also documents every endpoint can be found in the [API Spec](https://api-spec.shlink.io/) portal.
 
     However, you probably don't want to consume the raw API yourself. That's why a nice [web client](https://github.com/shlinkio/shlink-web-client) is provided that can be directly used from [https://app.shlink.io](https://app.shlink.io), or you can host it yourself too.
 
diff --git a/docs/swagger/swagger.json b/docs/swagger/swagger.json
index ddf2b3ad..32e0caf3 100644
--- a/docs/swagger/swagger.json
+++ b/docs/swagger/swagger.json
@@ -7,7 +7,7 @@
     },
 
     "externalDocs": {
-        "url": "https://shlink.io/api-docs",
+        "url": "https://shlink.io/documentation/api-docs",
         "description": "Find more info on how to start using this API here"
     },
 

From d32112fe7edc6e845bed86769cb4484563edde48 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Sat, 14 Mar 2020 19:24:21 +0100
Subject: [PATCH 31/67] Updated shlink packages and installed shlink-config

---
 README.md                                        |  2 +-
 composer.json                                    |  9 +++++----
 config/config.php                                |  3 ++-
 module/CLI/src/ConfigProvider.php                |  2 +-
 .../Core/src/Config/SimplifiedConfigParser.php   |  2 +-
 module/Core/src/ConfigProvider.php               |  2 +-
 module/Core/test/Model/VisitorTest.php           | 16 +++++++++++++---
 module/Rest/src/ConfigProvider.php               |  2 +-
 8 files changed, 25 insertions(+), 13 deletions(-)

diff --git a/README.md b/README.md
index 7804af37..973a234b 100644
--- a/README.md
+++ b/README.md
@@ -6,7 +6,7 @@
 [![Latest Stable Version](https://img.shields.io/github/release/shlinkio/shlink.svg?style=flat-square)](https://packagist.org/packages/shlinkio/shlink)
 [![Docker pulls](https://img.shields.io/docker/pulls/shlinkio/shlink.svg?style=flat-square)](https://hub.docker.com/r/shlinkio/shlink/)
 [![License](https://img.shields.io/github/license/shlinkio/shlink.svg?style=flat-square)](https://github.com/shlinkio/shlink/blob/master/LICENSE)
-[![Paypal donate](https://img.shields.io/badge/Donate-paypal-blue.svg?style=flat-square&logo=paypal&colorA=aaaaaa)](https://acel.me/donate)
+[![Paypal donate](https://img.shields.io/badge/Donate-paypal-blue.svg?style=flat-square&logo=paypal&colorA=aaaaaa)](https://slnk.to/donate)
 
 A PHP-based self-hosted URL shortener that can be used to serve shortened URLs under your own custom domain.
 
diff --git a/composer.json b/composer.json
index 84707645..5ee67a4d 100644
--- a/composer.json
+++ b/composer.json
@@ -49,10 +49,11 @@
         "predis/predis": "^1.1",
         "pugx/shortid-php": "^0.5",
         "ramsey/uuid": "^3.9",
-        "shlinkio/shlink-common": "^2.8.0",
-        "shlinkio/shlink-event-dispatcher": "^1.3",
-        "shlinkio/shlink-installer": "^4.2.0",
-        "shlinkio/shlink-ip-geolocation": "^1.3.1",
+        "shlinkio/shlink-common": "^3.0",
+        "shlinkio/shlink-config": "^1.0",
+        "shlinkio/shlink-event-dispatcher": "^1.4",
+        "shlinkio/shlink-installer": "^4.3",
+        "shlinkio/shlink-ip-geolocation": "^1.4",
         "symfony/console": "^5.0",
         "symfony/filesystem": "^5.0",
         "symfony/lock": "^5.0",
diff --git a/config/config.php b/config/config.php
index 3fb1a3e4..98b4552b 100644
--- a/config/config.php
+++ b/config/config.php
@@ -19,11 +19,12 @@ return (new ConfigAggregator\ConfigAggregator([
     Mezzio\Swoole\ConfigProvider::class,
     ProblemDetails\ConfigProvider::class,
     Common\ConfigProvider::class,
+    Config\ConfigProvider::class,
     IpGeolocation\ConfigProvider::class,
+    EventDispatcher\ConfigProvider::class,
     Core\ConfigProvider::class,
     CLI\ConfigProvider::class,
     Rest\ConfigProvider::class,
-    EventDispatcher\ConfigProvider::class,
     new ConfigAggregator\PhpFileProvider('config/autoload/{{,*.}global,{,*.}local}.php'),
     env('APP_ENV') === 'test'
         ? new ConfigAggregator\PhpFileProvider('config/test/*.global.php')
diff --git a/module/CLI/src/ConfigProvider.php b/module/CLI/src/ConfigProvider.php
index 40dcf775..8155b68b 100644
--- a/module/CLI/src/ConfigProvider.php
+++ b/module/CLI/src/ConfigProvider.php
@@ -4,7 +4,7 @@ declare(strict_types=1);
 
 namespace Shlinkio\Shlink\CLI;
 
-use function Shlinkio\Shlink\Common\loadConfigFromGlob;
+use function Shlinkio\Shlink\Config\loadConfigFromGlob;
 
 class ConfigProvider
 {
diff --git a/module/Core/src/Config/SimplifiedConfigParser.php b/module/Core/src/Config/SimplifiedConfigParser.php
index 6181e27a..ee29d195 100644
--- a/module/Core/src/Config/SimplifiedConfigParser.php
+++ b/module/Core/src/Config/SimplifiedConfigParser.php
@@ -5,7 +5,7 @@ declare(strict_types=1);
 namespace Shlinkio\Shlink\Core\Config;
 
 use Laminas\Stdlib\ArrayUtils;
-use Shlinkio\Shlink\Installer\Util\PathCollection;
+use Shlinkio\Shlink\Config\Collection\PathCollection;
 
 use function array_flip;
 use function array_intersect_key;
diff --git a/module/Core/src/ConfigProvider.php b/module/Core/src/ConfigProvider.php
index 086d093d..2c130ea9 100644
--- a/module/Core/src/ConfigProvider.php
+++ b/module/Core/src/ConfigProvider.php
@@ -4,7 +4,7 @@ declare(strict_types=1);
 
 namespace Shlinkio\Shlink\Core;
 
-use function Shlinkio\Shlink\Common\loadConfigFromGlob;
+use function Shlinkio\Shlink\Config\loadConfigFromGlob;
 
 class ConfigProvider
 {
diff --git a/module/Core/test/Model/VisitorTest.php b/module/Core/test/Model/VisitorTest.php
index 9aa928a8..0a0c1828 100644
--- a/module/Core/test/Model/VisitorTest.php
+++ b/module/Core/test/Model/VisitorTest.php
@@ -5,16 +5,15 @@ declare(strict_types=1);
 namespace ShlinkioTest\Shlink\Core\Model;
 
 use PHPUnit\Framework\TestCase;
-use Shlinkio\Shlink\Common\Util\StringUtilsTrait;
 use Shlinkio\Shlink\Core\Model\Visitor;
 
+use function random_int;
 use function str_repeat;
+use function strlen;
 use function substr;
 
 class VisitorTest extends TestCase
 {
-    use StringUtilsTrait;
-
     /**
      * @test
      * @dataProvider provideParams
@@ -60,4 +59,15 @@ class VisitorTest extends TestCase
             ],
         ];
     }
+
+    private function generateRandomString(int $length): string
+    {
+        $characters = '0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ';
+        $charactersLength = strlen($characters);
+        $randomString = '';
+        for ($i = 0; $i < $length; $i++) {
+            $randomString .= $characters[random_int(0, $charactersLength - 1)];
+        }
+        return $randomString;
+    }
 }
diff --git a/module/Rest/src/ConfigProvider.php b/module/Rest/src/ConfigProvider.php
index 570eab85..130617d6 100644
--- a/module/Rest/src/ConfigProvider.php
+++ b/module/Rest/src/ConfigProvider.php
@@ -8,7 +8,7 @@ use Closure;
 
 use function Functional\first;
 use function Functional\map;
-use function Shlinkio\Shlink\Common\loadConfigFromGlob;
+use function Shlinkio\Shlink\Config\loadConfigFromGlob;
 use function sprintf;
 
 class ConfigProvider

From 6ddd70d21d6ab5f63e669de0a6f5297f2d40b460 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Sun, 15 Mar 2020 17:25:39 +0100
Subject: [PATCH 32/67] Added --no-interaction to commands run internally from
 shlink DB commands

---
 module/CLI/src/Command/Db/AbstractDatabaseCommand.php     | 4 +---
 module/CLI/test/Command/Db/CreateDatabaseCommandTest.php  | 4 +++-
 module/CLI/test/Command/Db/MigrateDatabaseCommandTest.php | 4 +++-
 3 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/module/CLI/src/Command/Db/AbstractDatabaseCommand.php b/module/CLI/src/Command/Db/AbstractDatabaseCommand.php
index 4b45fa56..5e9374cf 100644
--- a/module/CLI/src/Command/Db/AbstractDatabaseCommand.php
+++ b/module/CLI/src/Command/Db/AbstractDatabaseCommand.php
@@ -11,8 +11,6 @@ use Symfony\Component\Console\Output\OutputInterface;
 use Symfony\Component\Lock\LockFactory;
 use Symfony\Component\Process\PhpExecutableFinder;
 
-use function array_unshift;
-
 abstract class AbstractDatabaseCommand extends AbstractLockedCommand
 {
     private ProcessHelper $processHelper;
@@ -27,7 +25,7 @@ abstract class AbstractDatabaseCommand extends AbstractLockedCommand
 
     protected function runPhpCommand(OutputInterface $output, array $command): void
     {
-        array_unshift($command, $this->phpBinary);
+        $command = [$this->phpBinary, ...$command, '--no-interaction'];
         $this->processHelper->mustRun($output, $command);
     }
 
diff --git a/module/CLI/test/Command/Db/CreateDatabaseCommandTest.php b/module/CLI/test/Command/Db/CreateDatabaseCommandTest.php
index c88e28fa..d890f264 100644
--- a/module/CLI/test/Command/Db/CreateDatabaseCommandTest.php
+++ b/module/CLI/test/Command/Db/CreateDatabaseCommandTest.php
@@ -18,6 +18,7 @@ use Symfony\Component\Console\Tester\CommandTester;
 use Symfony\Component\Lock\LockFactory;
 use Symfony\Component\Lock\LockInterface;
 use Symfony\Component\Process\PhpExecutableFinder;
+use Symfony\Component\Process\Process;
 
 class CreateDatabaseCommandTest extends TestCase
 {
@@ -114,7 +115,8 @@ class CreateDatabaseCommandTest extends TestCase
             '/usr/local/bin/php',
             CreateDatabaseCommand::DOCTRINE_SCRIPT,
             CreateDatabaseCommand::DOCTRINE_CREATE_SCHEMA_COMMAND,
-        ], Argument::cetera());
+            '--no-interaction',
+        ], Argument::cetera())->willReturn(new Process([]));
 
         $this->commandTester->execute([]);
         $output = $this->commandTester->getDisplay();
diff --git a/module/CLI/test/Command/Db/MigrateDatabaseCommandTest.php b/module/CLI/test/Command/Db/MigrateDatabaseCommandTest.php
index 15f756a7..71587eea 100644
--- a/module/CLI/test/Command/Db/MigrateDatabaseCommandTest.php
+++ b/module/CLI/test/Command/Db/MigrateDatabaseCommandTest.php
@@ -15,6 +15,7 @@ use Symfony\Component\Console\Tester\CommandTester;
 use Symfony\Component\Lock\LockFactory;
 use Symfony\Component\Lock\LockInterface;
 use Symfony\Component\Process\PhpExecutableFinder;
+use Symfony\Component\Process\Process;
 
 class MigrateDatabaseCommandTest extends TestCase
 {
@@ -53,7 +54,8 @@ class MigrateDatabaseCommandTest extends TestCase
             '/usr/local/bin/php',
             MigrateDatabaseCommand::DOCTRINE_MIGRATIONS_SCRIPT,
             MigrateDatabaseCommand::DOCTRINE_MIGRATE_COMMAND,
-        ], Argument::cetera());
+            '--no-interaction',
+        ], Argument::cetera())->willReturn(new Process([]));
 
         $this->commandTester->execute([]);
         $output = $this->commandTester->getDisplay();

From 8597966187bd1c934fe9f767c6a4137c123e36a1 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Sun, 15 Mar 2020 17:26:34 +0100
Subject: [PATCH 33/67] Updated changelog

---
 CHANGELOG.md | 1 +
 1 file changed, 1 insertion(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 5eecc684..e5915375 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -28,6 +28,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this
 
 * [#665](https://github.com/shlinkio/shlink/issues/665) Fixed `base_url_redirect_to` simplified config option not being properly parsed.
 * [#663](https://github.com/shlinkio/shlink/issues/663) Fixed Shlink allowing short URLs to be created with an empty custom slug.
+* [#678](https://github.com/shlinkio/shlink/issues/678) Fixed `db` commands not running in a non-interactive way.
 
 
 ## 2.0.5 - 2020-02-09

From 644f5be6fe983710b42f163246be724dbc75035f Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Sat, 21 Mar 2020 08:42:13 +0100
Subject: [PATCH 34/67] Added scripts and configs to build docker image on
 travis

---
 .gitignore   |  2 +-
 .travis.yml  |  6 +++++-
 Dockerfile   |  2 +-
 docker/build | 15 +++++++++++++++
 hooks/build  | 10 ----------
 5 files changed, 22 insertions(+), 13 deletions(-)
 create mode 100755 docker/build
 delete mode 100755 hooks/build

diff --git a/.gitignore b/.gitignore
index ab121a93..8cfea409 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,6 +1,6 @@
 .idea
 build
-!hooks/build
+!docker/build
 composer.lock
 composer.phar
 vendor/
diff --git a/.travis.yml b/.travis.yml
index 4e4c70f5..2387d956 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -45,9 +45,13 @@ after_success:
 # Before deploying, build dist file for current travis tag
 before_deploy:
   - rm -f ocular.phar
-  - ./build.sh ${TRAVIS_TAG#?}
+  - if [[ ! -z $TRAVIS_TAG ]]; then ./build.sh ${TRAVIS_TAG#?} ; fi
 
 deploy:
+  - provider: script
+    script: bash ./docker/build
+    on:
+      condition: $TRAVIS_PULL_REQUEST == 'false'
   - provider: releases
     api_key:
       secure: a9dbZchocqeuOViwUeNH54bQR5Sz7rEYXx5b9WPFtnFn9LGKKUaLbA2U91UQ9QKPrcTpsALubUYbw2CnNmvCwzaY+R8lCD3gkU4ohsEnbpnw3deOeixI74sqBHJAuCH9FSaRDGILoBMtUKx2xlzIymFxkIsgIukkGbdkWHDlRWY3oTUUuw1SQ2Xk9KDsbJQtjIc1+G/O6gHaV4qv/R9W8NPmJExKTNDrAZbC1vIUnxqp4UpVo1hst8qPd1at94CndDYM5rG+7imGbdtxTxzamt819qdTO1OfvtctKawNAm7YXZrrWft6c7gI6j6SI4hxd+ZrrPBqbaRFHkZHjnNssO/yn4SaOHFFzccmu0MzvpPCf0qWZwd3sGHVYer1MnR2mHYqU84QPlW3nrHwJjkrpq3+q0JcBY6GsJs+RskHNtkMTKV05Iz6QUI5YZGwTpuXaRm036SmavjGc4IDlMaYCk/NmbB9BKpthJxLdUpczOHpnjXXHziotWD6cfEnbjU3byfD8HY5WrxSjsNT7SKmXN3hRof7bk985ewQVjGT42O3NbnfnqjQQWr/B7/zFTpLR4f526Bkq12CdCyf5lvrbq+POkLVdJ+uFfR7ds248Ue/jBQy6kM1tWmKF9QiwisFlA84eQ4CW3I93Rp97URv+AQa9zmbD0Ve3Udp+g6nF5I=
diff --git a/Dockerfile b/Dockerfile
index 23a142f8..3a65fd8e 100644
--- a/Dockerfile
+++ b/Dockerfile
@@ -9,7 +9,7 @@ ENV LC_ALL "C"
 WORKDIR /etc/shlink
 
 RUN \
-    # Install mysl and calendar
+    # Install mysql and calendar
     docker-php-ext-install -j"$(nproc)" pdo_mysql calendar && \
     # Install sqlite
     apk add --no-cache sqlite-libs sqlite-dev && \
diff --git a/docker/build b/docker/build
new file mode 100755
index 00000000..a02bd4e1
--- /dev/null
+++ b/docker/build
@@ -0,0 +1,15 @@
+#!/bin/bash
+set -e
+
+echo "$DOCKER_PASSWORD" | docker login -u "$DOCKER_USERNAME" --password-stdin
+
+# If there is a tag, regardless the branch, build that docker tag and also "stable"
+if [[ ! -z $TRAVIS_TAG ]]; then
+  docker build --build-arg VERSION=${TRAVIS_TAG#?} -t shlinkio/shlink:${TRAVIS_TAG#?} -t shlinkio/shlink:stable .
+  docker push shlinkio/shlink:${TRAVIS_TAG#?}
+  docker push shlinkio/shlink:stable
+# If build branch is develop, build latest (on master, when there's no tag, do not build anything)
+elif [[ "$TRAVIS_BRANCH" == 'develop' ]]; then
+  docker build -t shlinkio/shlink:latest .
+  docker push shlinkio/shlink:latest
+fi
diff --git a/hooks/build b/hooks/build
deleted file mode 100755
index 6b381d74..00000000
--- a/hooks/build
+++ /dev/null
@@ -1,10 +0,0 @@
-#!/bin/bash
-set -ex
-
-if [[ ${SOURCE_BRANCH} == 'develop' ]]; then
-    SHLINK_RELEASE='latest'
-else
-    SHLINK_RELEASE=${SOURCE_BRANCH#?}
-fi
-
-docker build --build-arg SHLINK_VERSION=${SHLINK_RELEASE} -t ${IMAGE_NAME} .

From ca1b17863c28ed46fecdf9156a67f2938ca25279 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Sat, 21 Mar 2020 08:56:48 +0100
Subject: [PATCH 35/67] Split Dockerfile into multiple stages, making the build
 be independent and then the released image just copy files from it

---
 Dockerfile       | 25 ++++++++++++++-----------
 docker/README.md |  1 -
 2 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/Dockerfile b/Dockerfile
index 3a65fd8e..96733933 100644
--- a/Dockerfile
+++ b/Dockerfile
@@ -1,5 +1,4 @@
-FROM php:7.4.2-alpine3.11
-LABEL maintainer="Alejandro Celaya <alejandro@alejandrocelaya.com>"
+FROM php:7.4.2-alpine3.11 as base
 
 ARG SHLINK_VERSION=2.0.5
 ENV SHLINK_VERSION ${SHLINK_VERSION}
@@ -36,17 +35,21 @@ RUN wget https://download.microsoft.com/download/e/4/e/e4e67866-dffd-428c-aac7-8
     rm msodbcsql17_17.5.1.1-1_amd64.apk && \
     rm mssql-tools_17.5.1.1-1_amd64.apk
 
-# Install shlink
-COPY . .
-COPY --from=composer:1.9.3 /usr/bin/composer ./composer.phar
-RUN rm -rf ./docker && \
-    php composer.phar install --no-dev --optimize-autoloader --prefer-dist --no-progress --no-interaction && \
-    php composer.phar clear-cache && \
-    rm composer.*
 
-# Add shlink to the path to ease running it after container is created
+# Install shlink
+FROM base as builder
+COPY . .
+COPY --from=composer:1.10.1 /usr/bin/composer ./composer.phar
+RUN php composer.phar install --no-dev --optimize-autoloader --prefer-dist --no-progress --no-interaction && \
+    sed -i "s/%SHLINK_VERSION%/${SHLINK_VERSION}/g" config/autoload/app_options.global.php
+
+
+# Prepare final image
+FROM base
+LABEL maintainer="Alejandro Celaya <alejandro@alejandrocelaya.com>"
+
+COPY --from=builder /etc/shlink .
 RUN ln -s /etc/shlink/bin/cli /usr/local/bin/shlink
-RUN sed -i "s/%SHLINK_VERSION%/${SHLINK_VERSION}/g" config/autoload/app_options.global.php
 
 # Expose swoole port
 EXPOSE 8080
diff --git a/docker/README.md b/docker/README.md
index 699c1c75..f8e596ce 100644
--- a/docker/README.md
+++ b/docker/README.md
@@ -1,6 +1,5 @@
 # Shlink Docker image
 
-[![Docker build status](https://img.shields.io/docker/build/shlinkio/shlink.svg?style=flat-square)](https://hub.docker.com/r/shlinkio/shlink/)
 [![Docker pulls](https://img.shields.io/docker/pulls/shlinkio/shlink.svg?style=flat-square)](https://hub.docker.com/r/shlinkio/shlink/)
 
 This image provides an easy way to set up [shlink](https://shlink.io) on a container-based runtime.

From f811002c2b4f643361508344ca00923febfe843f Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Sat, 21 Mar 2020 09:37:31 +0100
Subject: [PATCH 36/67] Small adjustments on docker build

---
 .travis.yml | 3 ++-
 build.sh    | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index 2387d956..32333b41 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -45,13 +45,14 @@ after_success:
 # Before deploying, build dist file for current travis tag
 before_deploy:
   - rm -f ocular.phar
-  - if [[ ! -z $TRAVIS_TAG ]]; then ./build.sh ${TRAVIS_TAG#?} ; fi
+  - if [[ ! -z $TRAVIS_TAG && "${TRAVIS_PHP_VERSION}" == "7.4" ]]; then ./build.sh ${TRAVIS_TAG#?} ; fi
 
 deploy:
   - provider: script
     script: bash ./docker/build
     on:
       condition: $TRAVIS_PULL_REQUEST == 'false'
+      php: '7.4'
   - provider: releases
     api_key:
       secure: a9dbZchocqeuOViwUeNH54bQR5Sz7rEYXx5b9WPFtnFn9LGKKUaLbA2U91UQ9QKPrcTpsALubUYbw2CnNmvCwzaY+R8lCD3gkU4ohsEnbpnw3deOeixI74sqBHJAuCH9FSaRDGILoBMtUKx2xlzIymFxkIsgIukkGbdkWHDlRWY3oTUUuw1SQ2Xk9KDsbJQtjIc1+G/O6gHaV4qv/R9W8NPmJExKTNDrAZbC1vIUnxqp4UpVo1hst8qPd1at94CndDYM5rG+7imGbdtxTxzamt819qdTO1OfvtctKawNAm7YXZrrWft6c7gI6j6SI4hxd+ZrrPBqbaRFHkZHjnNssO/yn4SaOHFFzccmu0MzvpPCf0qWZwd3sGHVYer1MnR2mHYqU84QPlW3nrHwJjkrpq3+q0JcBY6GsJs+RskHNtkMTKV05Iz6QUI5YZGwTpuXaRm036SmavjGc4IDlMaYCk/NmbB9BKpthJxLdUpczOHpnjXXHziotWD6cfEnbjU3byfD8HY5WrxSjsNT7SKmXN3hRof7bk985ewQVjGT42O3NbnfnqjQQWr/B7/zFTpLR4f526Bkq12CdCyf5lvrbq+POkLVdJ+uFfR7ds248Ue/jBQy6kM1tWmKF9QiwisFlA84eQ4CW3I93Rp97URv+AQa9zmbD0Ve3Udp+g6nF5I=
diff --git a/build.sh b/build.sh
index cf42695b..b3d28b9d 100755
--- a/build.sh
+++ b/build.sh
@@ -25,7 +25,7 @@ cd "${builtcontent}"
 # Install dependencies
 echo "Installing dependencies with $composerBin..."
 ${composerBin} self-update
-${composerBin} install --no-dev --optimize-autoloader --no-progress --no-interaction
+${composerBin} install --no-dev --optimize-autoloader --prefer-dist --no-progress --no-interaction
 
 # Delete development files
 echo 'Deleting dev files...'

From 75b8ed813f6a58c8ba8ddb5060326a00cc4d4d12 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Sat, 21 Mar 2020 14:25:19 +0100
Subject: [PATCH 37/67] Enforced Swoole 4.4.15 to be installed during travis
 build, to match production one

---
 .travis.yml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.travis.yml b/.travis.yml
index 32333b41..40e6f0ea 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -18,7 +18,7 @@ cache:
 
 before_install:
   - echo 'extension = apcu.so' >> ~/.phpenv/versions/$(phpenv version-name)/etc/php.ini
-  - yes | pecl install swoole
+  - yes | pecl install swoole-4.4.15
   - phpenv config-rm xdebug.ini || return 0
 
 install:

From d2c06dd0ab3e7e39bd1292c72e9203f83eb0e8f1 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Sat, 21 Mar 2020 14:38:24 +0100
Subject: [PATCH 38/67] Initialized typed nullable props as null in all
 entities

---
 module/Core/src/Entity/ShortUrl.php | 2 +-
 module/Rest/src/Entity/ApiKey.php   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/module/Core/src/Entity/ShortUrl.php b/module/Core/src/Entity/ShortUrl.php
index 4af8844b..10df9352 100644
--- a/module/Core/src/Entity/ShortUrl.php
+++ b/module/Core/src/Entity/ShortUrl.php
@@ -32,7 +32,7 @@ class ShortUrl extends AbstractEntity
     private ?Chronos $validSince = null;
     private ?Chronos $validUntil = null;
     private ?int $maxVisits = null;
-    private ?Domain $domain;
+    private ?Domain $domain = null;
     private bool $customSlugWasProvided;
     private int $shortCodeLength;
 
diff --git a/module/Rest/src/Entity/ApiKey.php b/module/Rest/src/Entity/ApiKey.php
index f23d22e1..1d372c9c 100644
--- a/module/Rest/src/Entity/ApiKey.php
+++ b/module/Rest/src/Entity/ApiKey.php
@@ -11,7 +11,7 @@ use Shlinkio\Shlink\Common\Entity\AbstractEntity;
 class ApiKey extends AbstractEntity
 {
     private string $key;
-    private ?Chronos $expirationDate;
+    private ?Chronos $expirationDate = null;
     private bool $enabled;
 
     public function __construct(?Chronos $expirationDate = null)

From 881da3db3bcbe3053aaa64004bcd3f226b344e9f Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Sat, 21 Mar 2020 15:31:14 +0100
Subject: [PATCH 39/67] Ensured docker build happens in all branches

---
 .travis.yml | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index 40e6f0ea..9c41ec31 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -37,8 +37,8 @@ script:
 
 after_success:
   - rm -f build/clover.xml
-  - wget https://phar.phpunit.de/phpcov-7.0.1.phar
-  - phpdbg -qrr phpcov-7.0.1.phar merge build --clover build/clover.xml
+  - wget https://phar.phpunit.de/phpcov-7.0.2.phar
+  - phpdbg -qrr phpcov-7.0.2.phar merge build --clover build/clover.xml
   - wget https://scrutinizer-ci.com/ocular.phar
   - php ocular.phar code-coverage:upload --format=php-clover build/clover.xml
 
@@ -51,6 +51,7 @@ deploy:
   - provider: script
     script: bash ./docker/build
     on:
+      all_branches: true
       condition: $TRAVIS_PULL_REQUEST == 'false'
       php: '7.4'
   - provider: releases

From c556d8123bf6b628f45427fbfa326de1735a3d0f Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Sat, 21 Mar 2020 15:43:52 +0100
Subject: [PATCH 40/67] Fixed name of the build arg passed when building docker
 for a specific tag

---
 docker/build | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docker/build b/docker/build
index a02bd4e1..5eea7888 100755
--- a/docker/build
+++ b/docker/build
@@ -5,7 +5,7 @@ echo "$DOCKER_PASSWORD" | docker login -u "$DOCKER_USERNAME" --password-stdin
 
 # If there is a tag, regardless the branch, build that docker tag and also "stable"
 if [[ ! -z $TRAVIS_TAG ]]; then
-  docker build --build-arg VERSION=${TRAVIS_TAG#?} -t shlinkio/shlink:${TRAVIS_TAG#?} -t shlinkio/shlink:stable .
+  docker build --build-arg SHLINK_VERSION=${TRAVIS_TAG#?} -t shlinkio/shlink:${TRAVIS_TAG#?} -t shlinkio/shlink:stable .
   docker push shlinkio/shlink:${TRAVIS_TAG#?}
   docker push shlinkio/shlink:stable
 # If build branch is develop, build latest (on master, when there's no tag, do not build anything)

From d22f020eb53eaa21d4c78aad73a9d7f09373157b Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Sat, 21 Mar 2020 16:11:56 +0100
Subject: [PATCH 41/67] Ensured more non-prod files are ignored/deleted from
 the final docker image

---
 .dockerignore | 9 +++++----
 Dockerfile    | 2 ++
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/.dockerignore b/.dockerignore
index 7c730c69..4559d3e2 100644
--- a/.dockerignore
+++ b/.dockerignore
@@ -8,17 +8,18 @@ data/migrations_template.txt
 data/GeoLite2-City.*
 data/database.sqlite
 data/shlink-tests.db
-**/.gitignore
 CHANGELOG.md
+UPGRADE.md
 composer.lock
 vendor
 docs
 indocker
 docker-*
 php*
+.php*
 infection.json
-phpstan.neon
 **/test*
 build*
-.github
-hooks
+.git*
+.scrutinizer.yml
+.travis.yml
diff --git a/Dockerfile b/Dockerfile
index 96733933..1eb2715b 100644
--- a/Dockerfile
+++ b/Dockerfile
@@ -41,6 +41,8 @@ FROM base as builder
 COPY . .
 COPY --from=composer:1.10.1 /usr/bin/composer ./composer.phar
 RUN php composer.phar install --no-dev --optimize-autoloader --prefer-dist --no-progress --no-interaction && \
+    php composer.phar clear-cache && \
+    rm composer.* && \
     sed -i "s/%SHLINK_VERSION%/${SHLINK_VERSION}/g" config/autoload/app_options.global.php
 
 

From cea50a860efae5576ca80164a84b56f18662a0ab Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Sun, 22 Mar 2020 10:01:34 +0100
Subject: [PATCH 42/67] Improved docker image generation

---
 .dockerignore | 7 ++-----
 Dockerfile    | 5 +++--
 2 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/.dockerignore b/.dockerignore
index 4559d3e2..46b96529 100644
--- a/.dockerignore
+++ b/.dockerignore
@@ -15,11 +15,8 @@ vendor
 docs
 indocker
 docker-*
-php*
-.php*
+phpstan.neon
 infection.json
 **/test*
 build*
-.git*
-.scrutinizer.yml
-.travis.yml
+**/.*
diff --git a/Dockerfile b/Dockerfile
index 1eb2715b..64cd7ebe 100644
--- a/Dockerfile
+++ b/Dockerfile
@@ -40,9 +40,10 @@ RUN wget https://download.microsoft.com/download/e/4/e/e4e67866-dffd-428c-aac7-8
 FROM base as builder
 COPY . .
 COPY --from=composer:1.10.1 /usr/bin/composer ./composer.phar
-RUN php composer.phar install --no-dev --optimize-autoloader --prefer-dist --no-progress --no-interaction && \
+RUN apk add --no-cache git && \
+    php composer.phar install --no-dev --optimize-autoloader --prefer-dist --no-progress --no-interaction && \
     php composer.phar clear-cache && \
-    rm composer.* && \
+    rm -r docker composer.* && \
     sed -i "s/%SHLINK_VERSION%/${SHLINK_VERSION}/g" config/autoload/app_options.global.php
 
 

From 3fef4b4a2826eb2084eaa9f7ada007645bc0c337 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Sun, 22 Mar 2020 10:48:27 +0100
Subject: [PATCH 43/67] Ensured non-obfuscated IP address is passed to event
 listener which geolocates it

---
 .../Core/src/EventDispatcher/LocateShortUrlVisit.php  | 11 ++++++-----
 module/Core/src/EventDispatcher/ShortUrlVisited.php   | 11 +++++++++--
 module/Core/src/Service/VisitsTracker.php             |  2 +-
 3 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/module/Core/src/EventDispatcher/LocateShortUrlVisit.php b/module/Core/src/EventDispatcher/LocateShortUrlVisit.php
index a0f8d033..6abbe02b 100644
--- a/module/Core/src/EventDispatcher/LocateShortUrlVisit.php
+++ b/module/Core/src/EventDispatcher/LocateShortUrlVisit.php
@@ -53,7 +53,7 @@ class LocateShortUrlVisit
         }
 
         if ($this->downloadOrUpdateGeoLiteDb($visitId)) {
-            $this->locateVisit($visitId, $visit);
+            $this->locateVisit($visitId, $shortUrlVisited->originalIpAddress(), $visit);
         }
 
         $this->eventDispatcher->dispatch(new VisitLocated($visitId));
@@ -80,12 +80,13 @@ class LocateShortUrlVisit
         return true;
     }
 
-    private function locateVisit(string $visitId, Visit $visit): void
+    private function locateVisit(string $visitId, ?string $originalIpAddress, Visit $visit): void
     {
+        $isLocatable = $originalIpAddress !== null || $visit->isLocatable();
+        $addr = $originalIpAddress ?? $visit->getRemoteAddr();
+
         try {
-            $location = $visit->isLocatable()
-                ? $this->ipLocationResolver->resolveIpLocation($visit->getRemoteAddr())
-                : Location::emptyInstance();
+            $location = $isLocatable ? $this->ipLocationResolver->resolveIpLocation($addr) : Location::emptyInstance();
 
             $visit->locate(new VisitLocation($location));
             $this->em->flush();
diff --git a/module/Core/src/EventDispatcher/ShortUrlVisited.php b/module/Core/src/EventDispatcher/ShortUrlVisited.php
index 1f0b5b5c..c33f805a 100644
--- a/module/Core/src/EventDispatcher/ShortUrlVisited.php
+++ b/module/Core/src/EventDispatcher/ShortUrlVisited.php
@@ -9,10 +9,12 @@ use JsonSerializable;
 final class ShortUrlVisited implements JsonSerializable
 {
     private string $visitId;
+    private ?string $originalIpAddress;
 
-    public function __construct(string $visitId)
+    public function __construct(string $visitId, ?string $originalIpAddress = null)
     {
         $this->visitId = $visitId;
+        $this->originalIpAddress = $originalIpAddress;
     }
 
     public function visitId(): string
@@ -20,8 +22,13 @@ final class ShortUrlVisited implements JsonSerializable
         return $this->visitId;
     }
 
+    public function originalIpAddress(): ?string
+    {
+        return $this->originalIpAddress;
+    }
+
     public function jsonSerialize(): array
     {
-        return ['visitId' => $this->visitId];
+        return ['visitId' => $this->visitId, 'originalIpAddress' => $this->originalIpAddress];
     }
 }
diff --git a/module/Core/src/Service/VisitsTracker.php b/module/Core/src/Service/VisitsTracker.php
index 54abe319..f477681a 100644
--- a/module/Core/src/Service/VisitsTracker.php
+++ b/module/Core/src/Service/VisitsTracker.php
@@ -39,7 +39,7 @@ class VisitsTracker implements VisitsTrackerInterface
         $this->em->persist($visit);
         $this->em->flush();
 
-        $this->eventDispatcher->dispatch(new ShortUrlVisited($visit->getId()));
+        $this->eventDispatcher->dispatch(new ShortUrlVisited($visit->getId(), $visitor->getRemoteAddress()));
     }
 
     /**

From fdd8efc12d4a6ff8aae3bfb39faa5aeb0ae7f6ec Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Sun, 22 Mar 2020 11:12:30 +0100
Subject: [PATCH 44/67] Added test covering case in which the original address
 is provided when locating visits

---
 .../EventDispatcher/LocateShortUrlVisitTest.php | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/module/Core/test/EventDispatcher/LocateShortUrlVisitTest.php b/module/Core/test/EventDispatcher/LocateShortUrlVisitTest.php
index 5f40bb7b..c8f6f388 100644
--- a/module/Core/test/EventDispatcher/LocateShortUrlVisitTest.php
+++ b/module/Core/test/EventDispatcher/LocateShortUrlVisitTest.php
@@ -130,13 +130,16 @@ class LocateShortUrlVisitTest extends TestCase
         yield 'localhost' => [new Visit($shortUrl, new Visitor('', '', IpAddress::LOCALHOST))];
     }
 
-    /** @test */
-    public function locatableVisitsResolveToLocation(): void
+    /**
+     * @test
+     * @dataProvider provideIpAddresses
+     */
+    public function locatableVisitsResolveToLocation(string $anonymizedIpAddress, ?string $originalIpAddress): void
     {
-        $ipAddr = '1.2.3.0';
+        $ipAddr = $originalIpAddress ?? $anonymizedIpAddress;
         $visit = new Visit(new ShortUrl(''), new Visitor('', '', $ipAddr));
         $location = new Location('', '', '', '', 0.0, 0.0, '');
-        $event = new ShortUrlVisited('123');
+        $event = new ShortUrlVisited('123', $originalIpAddress);
 
         $findVisit = $this->em->find(Visit::class, '123')->willReturn($visit);
         $flush = $this->em->flush()->will(function (): void {
@@ -155,6 +158,12 @@ class LocateShortUrlVisitTest extends TestCase
         $dispatch->shouldHaveBeenCalledOnce();
     }
 
+    public function provideIpAddresses(): iterable
+    {
+        yield 'no original IP address' => ['1.2.3.0', null];
+        yield 'original IP address' => ['1.2.3.0', '1.2.3.4'];
+    }
+
     /** @test */
     public function errorWhenUpdatingGeoLiteWithExistingCopyLogsWarning(): void
     {

From 3a14483568e115e4dfa449f9bc097e99483640df Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Sun, 22 Mar 2020 11:13:33 +0100
Subject: [PATCH 45/67] Updated changelog

---
 CHANGELOG.md | 1 +
 1 file changed, 1 insertion(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index e5915375..8304e678 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -11,6 +11,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this
 * [#626](https://github.com/shlinkio/shlink/issues/626) Added support for Microsoft SQL Server.
 * [#556](https://github.com/shlinkio/shlink/issues/556) Short code lengths can now be customized, both globally and on a per-short URL basis.
 * [#541](https://github.com/shlinkio/shlink/issues/541) Added a request ID that is returned on `X-Request-Id` header, can be provided from outside and is set in log entries.
+* [#642](https://github.com/shlinkio/shlink/issues/642) IP geolocation is now performed over the non-anonymized IP address when using swoole.
 
 #### Changed
 

From 8fb54e815ea2c019494fe0208678ed6261bad061 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Sun, 22 Mar 2020 11:27:03 +0100
Subject: [PATCH 46/67] Ensured scrutinizer build ignores platform requirements
 when installing dependencies

---
 .scrutinizer.yml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/.scrutinizer.yml b/.scrutinizer.yml
index 3fa0e966..ed831706 100644
--- a/.scrutinizer.yml
+++ b/.scrutinizer.yml
@@ -6,6 +6,9 @@ checks:
     code_rating: true
     duplication: true
 build:
+  dependencies:
+    override:
+      - composer install --no-interaction --no-scripts --ignore-platform-reqs
   nodes:
     analysis:
       tests:

From e10b2884c08f186dc33551c6f2aa22d142c62d88 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Sun, 22 Mar 2020 11:33:00 +0100
Subject: [PATCH 47/67] Added more exclussions to dockerignore

---
 .dockerignore | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.dockerignore b/.dockerignore
index 46b96529..9a48c84c 100644
--- a/.dockerignore
+++ b/.dockerignore
@@ -16,6 +16,7 @@ docs
 indocker
 docker-*
 phpstan.neon
+php*xml*
 infection.json
 **/test*
 build*

From 4e6836c6051377a06097783d8bcdd2bce1640df7 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Sun, 22 Mar 2020 14:04:01 +0100
Subject: [PATCH 48/67] Long URLs can now be edited on existing short URLs

---
 module/Core/src/Entity/ShortUrl.php           |  18 +--
 module/Core/src/Model/ShortUrlEdit.php        | 106 ++++++++++++++++++
 module/Core/src/Model/ShortUrlMeta.php        |  15 +--
 module/Core/src/Service/ShortUrlService.php   |   6 +-
 .../src/Service/ShortUrlServiceInterface.php  |   4 +-
 .../Validation/ShortUrlMetaInputFilter.php    |   3 +
 .../Core/test/Service/ShortUrlServiceTest.php |   4 +-
 .../Action/ShortUrl/EditShortUrlAction.php    |   6 +-
 8 files changed, 134 insertions(+), 28 deletions(-)
 create mode 100644 module/Core/src/Model/ShortUrlEdit.php

diff --git a/module/Core/src/Entity/ShortUrl.php b/module/Core/src/Entity/ShortUrl.php
index 10df9352..5453d791 100644
--- a/module/Core/src/Entity/ShortUrl.php
+++ b/module/Core/src/Entity/ShortUrl.php
@@ -12,6 +12,7 @@ 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\ShortUrlEdit;
 use Shlinkio\Shlink\Core\Model\ShortUrlMeta;
 
 use function array_reduce;
@@ -93,16 +94,19 @@ class ShortUrl extends AbstractEntity
         return $this;
     }
 
-    public function updateMeta(ShortUrlMeta $shortCodeMeta): void
+    public function update(ShortUrlEdit $shortUrlEdit): void
     {
-        if ($shortCodeMeta->hasValidSince()) {
-            $this->validSince = $shortCodeMeta->getValidSince();
+        if ($shortUrlEdit->hasValidSince()) {
+            $this->validSince = $shortUrlEdit->validSince();
         }
-        if ($shortCodeMeta->hasValidUntil()) {
-            $this->validUntil = $shortCodeMeta->getValidUntil();
+        if ($shortUrlEdit->hasValidUntil()) {
+            $this->validUntil = $shortUrlEdit->validUntil();
         }
-        if ($shortCodeMeta->hasMaxVisits()) {
-            $this->maxVisits = $shortCodeMeta->getMaxVisits();
+        if ($shortUrlEdit->hasMaxVisits()) {
+            $this->maxVisits = $shortUrlEdit->maxVisits();
+        }
+        if ($shortUrlEdit->hasLongUrl()) {
+            $this->longUrl = $shortUrlEdit->longUrl();
         }
     }
 
diff --git a/module/Core/src/Model/ShortUrlEdit.php b/module/Core/src/Model/ShortUrlEdit.php
new file mode 100644
index 00000000..2f3f6919
--- /dev/null
+++ b/module/Core/src/Model/ShortUrlEdit.php
@@ -0,0 +1,106 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Shlinkio\Shlink\Core\Model;
+
+use Cake\Chronos\Chronos;
+use Shlinkio\Shlink\Core\Exception\ValidationException;
+use Shlinkio\Shlink\Core\Validation\ShortUrlMetaInputFilter;
+
+use function array_key_exists;
+use function Shlinkio\Shlink\Core\parseDateField;
+
+final class ShortUrlEdit
+{
+    private bool $longUrlPropWasProvided = false;
+    private ?string $longUrl = null;
+    private bool $validSincePropWasProvided = false;
+    private ?Chronos $validSince = null;
+    private bool $validUntilPropWasProvided = false;
+    private ?Chronos $validUntil = null;
+    private bool $maxVisitsPropWasProvided = false;
+    private ?int $maxVisits = null;
+
+    // Enforce named constructors
+    private function __construct()
+    {
+    }
+
+    /**
+     * @throws ValidationException
+     */
+    public static function fromRawData(array $data): self
+    {
+        $instance = new self();
+        $instance->validateAndInit($data);
+        return $instance;
+    }
+
+    /**
+     * @throws ValidationException
+     */
+    private function validateAndInit(array $data): void
+    {
+        $inputFilter = new ShortUrlMetaInputFilter($data);
+        if (! $inputFilter->isValid()) {
+            throw ValidationException::fromInputFilter($inputFilter);
+        }
+
+        $this->longUrlPropWasProvided = array_key_exists(ShortUrlMetaInputFilter::LONG_URL, $data);
+        $this->validSincePropWasProvided = array_key_exists(ShortUrlMetaInputFilter::VALID_SINCE, $data);
+        $this->validUntilPropWasProvided = array_key_exists(ShortUrlMetaInputFilter::VALID_UNTIL, $data);
+        $this->maxVisitsPropWasProvided = array_key_exists(ShortUrlMetaInputFilter::MAX_VISITS, $data);
+
+        $this->longUrl = $inputFilter->getValue(ShortUrlMetaInputFilter::LONG_URL);
+        $this->validSince = parseDateField($inputFilter->getValue(ShortUrlMetaInputFilter::VALID_SINCE));
+        $this->validUntil = parseDateField($inputFilter->getValue(ShortUrlMetaInputFilter::VALID_UNTIL));
+        $this->maxVisits = $this->getOptionalIntFromInputFilter($inputFilter, ShortUrlMetaInputFilter::MAX_VISITS);
+    }
+
+    private function getOptionalIntFromInputFilter(ShortUrlMetaInputFilter $inputFilter, string $fieldName): ?int
+    {
+        $value = $inputFilter->getValue($fieldName);
+        return $value !== null ? (int) $value : null;
+    }
+
+    public function longUrl(): ?string
+    {
+        return $this->longUrl;
+    }
+
+    public function hasLongUrl(): bool
+    {
+        return $this->longUrlPropWasProvided && $this->longUrl !== null;
+    }
+
+    public function validSince(): ?Chronos
+    {
+        return $this->validSince;
+    }
+
+    public function hasValidSince(): bool
+    {
+        return $this->validSincePropWasProvided;
+    }
+
+    public function validUntil(): ?Chronos
+    {
+        return $this->validUntil;
+    }
+
+    public function hasValidUntil(): bool
+    {
+        return $this->validUntilPropWasProvided;
+    }
+
+    public function maxVisits(): ?int
+    {
+        return $this->maxVisits;
+    }
+
+    public function hasMaxVisits(): bool
+    {
+        return $this->maxVisitsPropWasProvided;
+    }
+}
diff --git a/module/Core/src/Model/ShortUrlMeta.php b/module/Core/src/Model/ShortUrlMeta.php
index 3bba5c98..76f6d80b 100644
--- a/module/Core/src/Model/ShortUrlMeta.php
+++ b/module/Core/src/Model/ShortUrlMeta.php
@@ -8,25 +8,21 @@ use Cake\Chronos\Chronos;
 use Shlinkio\Shlink\Core\Exception\ValidationException;
 use Shlinkio\Shlink\Core\Validation\ShortUrlMetaInputFilter;
 
-use function array_key_exists;
 use function Shlinkio\Shlink\Core\parseDateField;
 
 use const Shlinkio\Shlink\Core\DEFAULT_SHORT_CODES_LENGTH;
 
 final class ShortUrlMeta
 {
-    private bool $validSincePropWasProvided = false;
     private ?Chronos $validSince = null;
-    private bool $validUntilPropWasProvided = false;
     private ?Chronos $validUntil = null;
     private ?string $customSlug = null;
-    private bool $maxVisitsPropWasProvided = false;
     private ?int $maxVisits = null;
     private ?bool $findIfExists = null;
     private ?string $domain = null;
     private int $shortCodeLength = 5;
 
-    // Force named constructors
+    // Enforce named constructors
     private function __construct()
     {
     }
@@ -57,12 +53,9 @@ final class ShortUrlMeta
         }
 
         $this->validSince = parseDateField($inputFilter->getValue(ShortUrlMetaInputFilter::VALID_SINCE));
-        $this->validSincePropWasProvided = array_key_exists(ShortUrlMetaInputFilter::VALID_SINCE, $data);
         $this->validUntil = parseDateField($inputFilter->getValue(ShortUrlMetaInputFilter::VALID_UNTIL));
-        $this->validUntilPropWasProvided = array_key_exists(ShortUrlMetaInputFilter::VALID_UNTIL, $data);
         $this->customSlug = $inputFilter->getValue(ShortUrlMetaInputFilter::CUSTOM_SLUG);
         $this->maxVisits = $this->getOptionalIntFromInputFilter($inputFilter, ShortUrlMetaInputFilter::MAX_VISITS);
-        $this->maxVisitsPropWasProvided = array_key_exists(ShortUrlMetaInputFilter::MAX_VISITS, $data);
         $this->findIfExists = $inputFilter->getValue(ShortUrlMetaInputFilter::FIND_IF_EXISTS);
         $this->domain = $inputFilter->getValue(ShortUrlMetaInputFilter::DOMAIN);
         $this->shortCodeLength = $this->getOptionalIntFromInputFilter(
@@ -84,7 +77,7 @@ final class ShortUrlMeta
 
     public function hasValidSince(): bool
     {
-        return $this->validSincePropWasProvided;
+        return $this->validSince !== null;
     }
 
     public function getValidUntil(): ?Chronos
@@ -94,7 +87,7 @@ final class ShortUrlMeta
 
     public function hasValidUntil(): bool
     {
-        return $this->validUntilPropWasProvided;
+        return $this->validUntil !== null;
     }
 
     public function getCustomSlug(): ?string
@@ -114,7 +107,7 @@ final class ShortUrlMeta
 
     public function hasMaxVisits(): bool
     {
-        return $this->maxVisitsPropWasProvided;
+        return $this->maxVisits !== null;
     }
 
     public function findIfExists(): bool
diff --git a/module/Core/src/Service/ShortUrlService.php b/module/Core/src/Service/ShortUrlService.php
index e9aaf637..ae0cc7a3 100644
--- a/module/Core/src/Service/ShortUrlService.php
+++ b/module/Core/src/Service/ShortUrlService.php
@@ -8,8 +8,8 @@ use Doctrine\ORM;
 use Laminas\Paginator\Paginator;
 use Shlinkio\Shlink\Core\Entity\ShortUrl;
 use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException;
+use Shlinkio\Shlink\Core\Model\ShortUrlEdit;
 use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier;
-use Shlinkio\Shlink\Core\Model\ShortUrlMeta;
 use Shlinkio\Shlink\Core\Model\ShortUrlsParams;
 use Shlinkio\Shlink\Core\Paginator\Adapter\ShortUrlRepositoryAdapter;
 use Shlinkio\Shlink\Core\Repository\ShortUrlRepository;
@@ -60,10 +60,10 @@ class ShortUrlService implements ShortUrlServiceInterface
     /**
      * @throws ShortUrlNotFoundException
      */
-    public function updateMetadataByShortCode(ShortUrlIdentifier $identifier, ShortUrlMeta $shortUrlMeta): ShortUrl
+    public function updateMetadataByShortCode(ShortUrlIdentifier $identifier, ShortUrlEdit $shortUrlEdit): ShortUrl
     {
         $shortUrl = $this->urlResolver->resolveShortUrl($identifier);
-        $shortUrl->updateMeta($shortUrlMeta);
+        $shortUrl->update($shortUrlEdit);
 
         $this->em->flush();
 
diff --git a/module/Core/src/Service/ShortUrlServiceInterface.php b/module/Core/src/Service/ShortUrlServiceInterface.php
index 379abc55..f17a7bea 100644
--- a/module/Core/src/Service/ShortUrlServiceInterface.php
+++ b/module/Core/src/Service/ShortUrlServiceInterface.php
@@ -7,8 +7,8 @@ namespace Shlinkio\Shlink\Core\Service;
 use Laminas\Paginator\Paginator;
 use Shlinkio\Shlink\Core\Entity\ShortUrl;
 use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException;
+use Shlinkio\Shlink\Core\Model\ShortUrlEdit;
 use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier;
-use Shlinkio\Shlink\Core\Model\ShortUrlMeta;
 use Shlinkio\Shlink\Core\Model\ShortUrlsParams;
 
 interface ShortUrlServiceInterface
@@ -27,5 +27,5 @@ interface ShortUrlServiceInterface
     /**
      * @throws ShortUrlNotFoundException
      */
-    public function updateMetadataByShortCode(ShortUrlIdentifier $identifier, ShortUrlMeta $shortUrlMeta): ShortUrl;
+    public function updateMetadataByShortCode(ShortUrlIdentifier $identifier, ShortUrlEdit $shortUrlEdit): ShortUrl;
 }
diff --git a/module/Core/src/Validation/ShortUrlMetaInputFilter.php b/module/Core/src/Validation/ShortUrlMetaInputFilter.php
index 013edd8f..8fde5e98 100644
--- a/module/Core/src/Validation/ShortUrlMetaInputFilter.php
+++ b/module/Core/src/Validation/ShortUrlMetaInputFilter.php
@@ -23,6 +23,7 @@ class ShortUrlMetaInputFilter extends InputFilter
     public const FIND_IF_EXISTS = 'findIfExists';
     public const DOMAIN = 'domain';
     public const SHORT_CODE_LENGTH = 'shortCodeLength';
+    public const LONG_URL = 'longUrl';
 
     public function __construct(array $data)
     {
@@ -32,6 +33,8 @@ class ShortUrlMetaInputFilter extends InputFilter
 
     private function initialize(): void
     {
+        $this->add($this->createInput(self::LONG_URL, false));
+
         $validSince = $this->createInput(self::VALID_SINCE, false);
         $validSince->getValidatorChain()->attach(new Validator\Date(['format' => DateTime::ATOM]));
         $this->add($validSince);
diff --git a/module/Core/test/Service/ShortUrlServiceTest.php b/module/Core/test/Service/ShortUrlServiceTest.php
index 842eac60..4f40683e 100644
--- a/module/Core/test/Service/ShortUrlServiceTest.php
+++ b/module/Core/test/Service/ShortUrlServiceTest.php
@@ -12,8 +12,8 @@ use Prophecy\Argument;
 use Prophecy\Prophecy\ObjectProphecy;
 use Shlinkio\Shlink\Core\Entity\ShortUrl;
 use Shlinkio\Shlink\Core\Entity\Tag;
+use Shlinkio\Shlink\Core\Model\ShortUrlEdit;
 use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier;
-use Shlinkio\Shlink\Core\Model\ShortUrlMeta;
 use Shlinkio\Shlink\Core\Model\ShortUrlsParams;
 use Shlinkio\Shlink\Core\Repository\ShortUrlRepository;
 use Shlinkio\Shlink\Core\Service\ShortUrl\ShortUrlResolverInterface;
@@ -82,7 +82,7 @@ class ShortUrlServiceTest extends TestCase
         $findShortUrl = $this->urlResolver->resolveShortUrl(new ShortUrlIdentifier('abc123'))->willReturn($shortUrl);
         $flush = $this->em->flush()->willReturn(null);
 
-        $result = $this->service->updateMetadataByShortCode(new ShortUrlIdentifier('abc123'), ShortUrlMeta::fromRawData(
+        $result = $this->service->updateMetadataByShortCode(new ShortUrlIdentifier('abc123'), ShortUrlEdit::fromRawData(
             [
                 'validSince' => Chronos::parse('2017-01-01 00:00:00')->toAtomString(),
                 'validUntil' => Chronos::parse('2017-01-05 00:00:00')->toAtomString(),
diff --git a/module/Rest/src/Action/ShortUrl/EditShortUrlAction.php b/module/Rest/src/Action/ShortUrl/EditShortUrlAction.php
index 8b3e65ab..da7012b6 100644
--- a/module/Rest/src/Action/ShortUrl/EditShortUrlAction.php
+++ b/module/Rest/src/Action/ShortUrl/EditShortUrlAction.php
@@ -8,8 +8,8 @@ use Laminas\Diactoros\Response\EmptyResponse;
 use Psr\Http\Message\ResponseInterface;
 use Psr\Http\Message\ServerRequestInterface;
 use Psr\Log\LoggerInterface;
+use Shlinkio\Shlink\Core\Model\ShortUrlEdit;
 use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier;
-use Shlinkio\Shlink\Core\Model\ShortUrlMeta;
 use Shlinkio\Shlink\Core\Service\ShortUrlServiceInterface;
 use Shlinkio\Shlink\Rest\Action\AbstractRestAction;
 
@@ -28,10 +28,10 @@ class EditShortUrlAction extends AbstractRestAction
 
     public function handle(ServerRequestInterface $request): ResponseInterface
     {
-        $postData = (array) $request->getParsedBody();
+        $shortUrlEdit = ShortUrlEdit::fromRawData((array) $request->getParsedBody());
         $identifier = ShortUrlIdentifier::fromApiRequest($request);
 
-        $this->shortUrlService->updateMetadataByShortCode($identifier, ShortUrlMeta::fromRawData($postData));
+        $this->shortUrlService->updateMetadataByShortCode($identifier, $shortUrlEdit);
         return new EmptyResponse();
     }
 }

From d29ebb706e66151dba7e1f4084ffe930c70af85f Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Sun, 22 Mar 2020 14:06:13 +0100
Subject: [PATCH 49/67] Documented longUrl param on PATCH short URL endpoint

---
 docs/swagger/paths/v1_short-urls_{shortCode}.json | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/docs/swagger/paths/v1_short-urls_{shortCode}.json b/docs/swagger/paths/v1_short-urls_{shortCode}.json
index b9baad92..71a6a427 100644
--- a/docs/swagger/paths/v1_short-urls_{shortCode}.json
+++ b/docs/swagger/paths/v1_short-urls_{shortCode}.json
@@ -112,6 +112,10 @@
                     "schema": {
                         "type": "object",
                         "properties": {
+                            "longUrl": {
+                                "description": "The long URL this short URL will redirect to",
+                                "type": "string"
+                            },
                             "validSince": {
                                 "description": "The date (in ISO-8601 format) from which this short code will be valid",
                                 "type": "string"
@@ -157,6 +161,7 @@
                                             "items": {
                                                 "type": "string",
                                                 "enum": [
+                                                    "longUrl",
                                                     "validSince",
                                                     "validUntil",
                                                     "maxVisits"

From 682a0768b78d4f4b450785c2a87352fc9ac75b90 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Sun, 22 Mar 2020 16:58:28 +0100
Subject: [PATCH 50/67] Moved check for URL validation config option to the
 UrlValidator itself

---
 module/Core/config/dependencies.config.php    |  4 +--
 module/Core/src/Service/UrlShortener.php      | 12 ++-----
 module/Core/src/Util/UrlValidator.php         | 10 +++++-
 module/Core/test/Service/UrlShortenerTest.php | 35 ++++---------------
 module/Core/test/Util/UrlValidatorTest.php    | 16 ++++++++-
 5 files changed, 34 insertions(+), 43 deletions(-)

diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php
index 9809c5dd..82bec10e 100644
--- a/module/Core/config/dependencies.config.php
+++ b/module/Core/config/dependencies.config.php
@@ -51,7 +51,7 @@ return [
         Options\NotFoundRedirectOptions::class => ['config.not_found_redirects'],
         Options\UrlShortenerOptions::class => ['config.url_shortener'],
 
-        Service\UrlShortener::class => [Util\UrlValidator::class, 'em', Options\UrlShortenerOptions::class],
+        Service\UrlShortener::class => [Util\UrlValidator::class, 'em'],
         Service\VisitsTracker::class => ['em', EventDispatcherInterface::class],
         Service\ShortUrlService::class => ['em', Service\ShortUrl\ShortUrlResolver::class],
         Service\VisitService::class => ['em'],
@@ -63,7 +63,7 @@ return [
         ],
         Service\ShortUrl\ShortUrlResolver::class => ['em'],
 
-        Util\UrlValidator::class => ['httpClient'],
+        Util\UrlValidator::class => ['httpClient', Options\UrlShortenerOptions::class],
 
         Action\RedirectAction::class => [
             Service\ShortUrl\ShortUrlResolver::class,
diff --git a/module/Core/src/Service/UrlShortener.php b/module/Core/src/Service/UrlShortener.php
index ab45143a..821c8e90 100644
--- a/module/Core/src/Service/UrlShortener.php
+++ b/module/Core/src/Service/UrlShortener.php
@@ -11,7 +11,6 @@ use Shlinkio\Shlink\Core\Entity\ShortUrl;
 use Shlinkio\Shlink\Core\Exception\InvalidUrlException;
 use Shlinkio\Shlink\Core\Exception\NonUniqueSlugException;
 use Shlinkio\Shlink\Core\Model\ShortUrlMeta;
-use Shlinkio\Shlink\Core\Options\UrlShortenerOptions;
 use Shlinkio\Shlink\Core\Repository\ShortUrlRepository;
 use Shlinkio\Shlink\Core\Util\TagManagerTrait;
 use Shlinkio\Shlink\Core\Util\UrlValidatorInterface;
@@ -24,17 +23,14 @@ class UrlShortener implements UrlShortenerInterface
     use TagManagerTrait;
 
     private EntityManagerInterface $em;
-    private UrlShortenerOptions $options;
     private UrlValidatorInterface $urlValidator;
 
     public function __construct(
         UrlValidatorInterface $urlValidator,
-        EntityManagerInterface $em,
-        UrlShortenerOptions $options
+        EntityManagerInterface $em
     ) {
         $this->urlValidator = $urlValidator;
         $this->em = $em;
-        $this->options = $options;
     }
 
     /**
@@ -53,11 +49,7 @@ class UrlShortener implements UrlShortenerInterface
             return $existingShortUrl;
         }
 
-        // If the URL validation is enabled, check that the URL actually exists
-        if ($this->options->isUrlValidationEnabled()) {
-            $this->urlValidator->validateUrl($url);
-        }
-
+        $this->urlValidator->validateUrl($url);
         $this->em->beginTransaction();
         $shortUrl = new ShortUrl($url, $meta, new PersistenceDomainResolver($this->em));
         $shortUrl->setTags($this->tagNamesToEntities($this->em, $tags));
diff --git a/module/Core/src/Util/UrlValidator.php b/module/Core/src/Util/UrlValidator.php
index dca037cd..8d8cd072 100644
--- a/module/Core/src/Util/UrlValidator.php
+++ b/module/Core/src/Util/UrlValidator.php
@@ -9,16 +9,19 @@ use GuzzleHttp\ClientInterface;
 use GuzzleHttp\Exception\GuzzleException;
 use GuzzleHttp\RequestOptions;
 use Shlinkio\Shlink\Core\Exception\InvalidUrlException;
+use Shlinkio\Shlink\Core\Options\UrlShortenerOptions;
 
 class UrlValidator implements UrlValidatorInterface, RequestMethodInterface
 {
     private const MAX_REDIRECTS = 15;
 
     private ClientInterface $httpClient;
+    private UrlShortenerOptions $options;
 
-    public function __construct(ClientInterface $httpClient)
+    public function __construct(ClientInterface $httpClient, UrlShortenerOptions $options)
     {
         $this->httpClient = $httpClient;
+        $this->options = $options;
     }
 
     /**
@@ -26,6 +29,11 @@ class UrlValidator implements UrlValidatorInterface, RequestMethodInterface
      */
     public function validateUrl(string $url): void
     {
+        // If the URL validation is not enabled, skip check
+        if (! $this->options->isUrlValidationEnabled()) {
+            return;
+        }
+
         try {
             $this->httpClient->request(self::METHOD_GET, $url, [
                 RequestOptions::ALLOW_REDIRECTS => ['max' => self::MAX_REDIRECTS],
diff --git a/module/Core/test/Service/UrlShortenerTest.php b/module/Core/test/Service/UrlShortenerTest.php
index a0392489..0e855d90 100644
--- a/module/Core/test/Service/UrlShortenerTest.php
+++ b/module/Core/test/Service/UrlShortenerTest.php
@@ -17,7 +17,6 @@ use Shlinkio\Shlink\Core\Entity\ShortUrl;
 use Shlinkio\Shlink\Core\Entity\Tag;
 use Shlinkio\Shlink\Core\Exception\NonUniqueSlugException;
 use Shlinkio\Shlink\Core\Model\ShortUrlMeta;
-use Shlinkio\Shlink\Core\Options\UrlShortenerOptions;
 use Shlinkio\Shlink\Core\Repository\ShortUrlRepository;
 use Shlinkio\Shlink\Core\Service\UrlShortener;
 use Shlinkio\Shlink\Core\Util\UrlValidatorInterface;
@@ -33,6 +32,10 @@ class UrlShortenerTest extends TestCase
     public function setUp(): void
     {
         $this->urlValidator = $this->prophesize(UrlValidatorInterface::class);
+        $this->urlValidator->validateUrl('http://foobar.com/12345/hello?foo=bar')->will(
+            function (): void {
+            },
+        );
 
         $this->em = $this->prophesize(EntityManagerInterface::class);
         $conn = $this->prophesize(Connection::class);
@@ -50,16 +53,7 @@ class UrlShortenerTest extends TestCase
         $repo->shortCodeIsInUse(Argument::cetera())->willReturn(false);
         $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal());
 
-        $this->setUrlShortener(false);
-    }
-
-    private function setUrlShortener(bool $urlValidationEnabled): void
-    {
-        $this->urlShortener = new UrlShortener(
-            $this->urlValidator->reveal(),
-            $this->em->reveal(),
-            new UrlShortenerOptions(['validate_url' => $urlValidationEnabled]),
-        );
+        $this->urlShortener = new UrlShortener($this->urlValidator->reveal(), $this->em->reveal());
     }
 
     /** @test */
@@ -119,24 +113,6 @@ class UrlShortenerTest extends TestCase
         );
     }
 
-    /** @test */
-    public function validatorIsCalledWhenUrlValidationIsEnabled(): void
-    {
-        $this->setUrlShortener(true);
-        $validateUrl = $this->urlValidator->validateUrl('http://foobar.com/12345/hello?foo=bar')->will(
-            function (): void {
-            },
-        );
-
-        $this->urlShortener->urlToShortCode(
-            new Uri('http://foobar.com/12345/hello?foo=bar'),
-            [],
-            ShortUrlMeta::createEmpty(),
-        );
-
-        $validateUrl->shouldHaveBeenCalledOnce();
-    }
-
     /** @test */
     public function exceptionIsThrownWhenNonUniqueSlugIsProvided(): void
     {
@@ -175,6 +151,7 @@ class UrlShortenerTest extends TestCase
         $findExisting->shouldHaveBeenCalledOnce();
         $getRepo->shouldHaveBeenCalledOnce();
         $this->em->persist(Argument::cetera())->shouldNotHaveBeenCalled();
+        $this->urlValidator->validateUrl(Argument::cetera())->shouldNotHaveBeenCalled();
         $this->assertSame($expected, $result);
     }
 
diff --git a/module/Core/test/Util/UrlValidatorTest.php b/module/Core/test/Util/UrlValidatorTest.php
index 5f018a05..50b70961 100644
--- a/module/Core/test/Util/UrlValidatorTest.php
+++ b/module/Core/test/Util/UrlValidatorTest.php
@@ -13,17 +13,20 @@ use PHPUnit\Framework\TestCase;
 use Prophecy\Argument;
 use Prophecy\Prophecy\ObjectProphecy;
 use Shlinkio\Shlink\Core\Exception\InvalidUrlException;
+use Shlinkio\Shlink\Core\Options\UrlShortenerOptions;
 use Shlinkio\Shlink\Core\Util\UrlValidator;
 
 class UrlValidatorTest extends TestCase
 {
     private UrlValidator $urlValidator;
     private ObjectProphecy $httpClient;
+    private UrlShortenerOptions $options;
 
     public function setUp(): void
     {
         $this->httpClient = $this->prophesize(ClientInterface::class);
-        $this->urlValidator = new UrlValidator($this->httpClient->reveal());
+        $this->options = new UrlShortenerOptions(['validate_url' => true]);
+        $this->urlValidator = new UrlValidator($this->httpClient->reveal(), $this->options);
     }
 
     /** @test */
@@ -52,4 +55,15 @@ class UrlValidatorTest extends TestCase
 
         $request->shouldHaveBeenCalledOnce();
     }
+
+    /** @test */
+    public function noCheckIsPerformedWhenUrlValidationIsDisabled(): void
+    {
+        $request = $this->httpClient->request(Argument::cetera())->willReturn(new Response());
+        $this->options->validateUrl = false;
+
+        $this->urlValidator->validateUrl('');
+
+        $request->shouldNotHaveBeenCalled();
+    }
 }

From 181ff164091d42e6aef8ebf7ce616d4f83fd1ce3 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Sun, 22 Mar 2020 17:05:59 +0100
Subject: [PATCH 51/67] Registered PersistenceDomainResolver as a service to
 avoid instantiating a new one on every ShortUrl creation

---
 module/Core/config/dependencies.config.php    | 7 ++++++-
 module/Core/src/Service/UrlShortener.php      | 9 ++++++---
 module/Core/test/Service/UrlShortenerTest.php | 7 ++++++-
 3 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php
index 82bec10e..6f33587a 100644
--- a/module/Core/config/dependencies.config.php
+++ b/module/Core/config/dependencies.config.php
@@ -9,6 +9,7 @@ use Laminas\ServiceManager\AbstractFactory\ConfigAbstractFactory;
 use Mezzio\Router\RouterInterface;
 use Mezzio\Template\TemplateRendererInterface;
 use Psr\EventDispatcher\EventDispatcherInterface;
+use Shlinkio\Shlink\Core\Domain\Resolver;
 use Shlinkio\Shlink\Core\ErrorHandler;
 use Shlinkio\Shlink\Core\Options\NotFoundRedirectOptions;
 
@@ -39,6 +40,8 @@ return [
             Action\QrCodeAction::class => ConfigAbstractFactory::class,
 
             Middleware\QrCodeCacheMiddleware::class => ConfigAbstractFactory::class,
+
+            Resolver\PersistenceDomainResolver::class => ConfigAbstractFactory::class,
         ],
     ],
 
@@ -51,7 +54,7 @@ return [
         Options\NotFoundRedirectOptions::class => ['config.not_found_redirects'],
         Options\UrlShortenerOptions::class => ['config.url_shortener'],
 
-        Service\UrlShortener::class => [Util\UrlValidator::class, 'em'],
+        Service\UrlShortener::class => [Util\UrlValidator::class, 'em', Resolver\PersistenceDomainResolver::class],
         Service\VisitsTracker::class => ['em', EventDispatcherInterface::class],
         Service\ShortUrlService::class => ['em', Service\ShortUrl\ShortUrlResolver::class],
         Service\VisitService::class => ['em'],
@@ -84,6 +87,8 @@ return [
         ],
 
         Middleware\QrCodeCacheMiddleware::class => [Cache::class],
+
+        Resolver\PersistenceDomainResolver::class => ['em'],
     ],
 
 ];
diff --git a/module/Core/src/Service/UrlShortener.php b/module/Core/src/Service/UrlShortener.php
index 821c8e90..4544bfc0 100644
--- a/module/Core/src/Service/UrlShortener.php
+++ b/module/Core/src/Service/UrlShortener.php
@@ -6,7 +6,7 @@ namespace Shlinkio\Shlink\Core\Service;
 
 use Doctrine\ORM\EntityManagerInterface;
 use Psr\Http\Message\UriInterface;
-use Shlinkio\Shlink\Core\Domain\Resolver\PersistenceDomainResolver;
+use Shlinkio\Shlink\Core\Domain\Resolver\DomainResolverInterface;
 use Shlinkio\Shlink\Core\Entity\ShortUrl;
 use Shlinkio\Shlink\Core\Exception\InvalidUrlException;
 use Shlinkio\Shlink\Core\Exception\NonUniqueSlugException;
@@ -24,13 +24,16 @@ class UrlShortener implements UrlShortenerInterface
 
     private EntityManagerInterface $em;
     private UrlValidatorInterface $urlValidator;
+    private DomainResolverInterface $domainResolver;
 
     public function __construct(
         UrlValidatorInterface $urlValidator,
-        EntityManagerInterface $em
+        EntityManagerInterface $em,
+        DomainResolverInterface $domainResolver
     ) {
         $this->urlValidator = $urlValidator;
         $this->em = $em;
+        $this->domainResolver = $domainResolver;
     }
 
     /**
@@ -51,7 +54,7 @@ class UrlShortener implements UrlShortenerInterface
 
         $this->urlValidator->validateUrl($url);
         $this->em->beginTransaction();
-        $shortUrl = new ShortUrl($url, $meta, new PersistenceDomainResolver($this->em));
+        $shortUrl = new ShortUrl($url, $meta, $this->domainResolver);
         $shortUrl->setTags($this->tagNamesToEntities($this->em, $tags));
 
         try {
diff --git a/module/Core/test/Service/UrlShortenerTest.php b/module/Core/test/Service/UrlShortenerTest.php
index 0e855d90..2c67bf27 100644
--- a/module/Core/test/Service/UrlShortenerTest.php
+++ b/module/Core/test/Service/UrlShortenerTest.php
@@ -13,6 +13,7 @@ use Laminas\Diactoros\Uri;
 use PHPUnit\Framework\TestCase;
 use Prophecy\Argument;
 use Prophecy\Prophecy\ObjectProphecy;
+use Shlinkio\Shlink\Core\Domain\Resolver\SimpleDomainResolver;
 use Shlinkio\Shlink\Core\Entity\ShortUrl;
 use Shlinkio\Shlink\Core\Entity\Tag;
 use Shlinkio\Shlink\Core\Exception\NonUniqueSlugException;
@@ -53,7 +54,11 @@ class UrlShortenerTest extends TestCase
         $repo->shortCodeIsInUse(Argument::cetera())->willReturn(false);
         $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal());
 
-        $this->urlShortener = new UrlShortener($this->urlValidator->reveal(), $this->em->reveal());
+        $this->urlShortener = new UrlShortener(
+            $this->urlValidator->reveal(),
+            $this->em->reveal(),
+            new SimpleDomainResolver(),
+        );
     }
 
     /** @test */

From 5432eb7b776df51645f330a350b89e3507884bf2 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Sun, 22 Mar 2020 17:22:52 +0100
Subject: [PATCH 52/67] Added URL validation to ShortUrl edition, as long URL
 can now be edited

---
 module/Core/config/dependencies.config.php    |  2 +-
 module/Core/src/Service/ShortUrlService.php   | 16 +++++-
 .../src/Service/ShortUrlServiceInterface.php  |  2 +
 .../Core/test/Service/ShortUrlServiceTest.php | 55 ++++++++++++++-----
 4 files changed, 58 insertions(+), 17 deletions(-)

diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php
index 6f33587a..2800cdd6 100644
--- a/module/Core/config/dependencies.config.php
+++ b/module/Core/config/dependencies.config.php
@@ -56,7 +56,7 @@ return [
 
         Service\UrlShortener::class => [Util\UrlValidator::class, 'em', Resolver\PersistenceDomainResolver::class],
         Service\VisitsTracker::class => ['em', EventDispatcherInterface::class],
-        Service\ShortUrlService::class => ['em', Service\ShortUrl\ShortUrlResolver::class],
+        Service\ShortUrlService::class => ['em', Service\ShortUrl\ShortUrlResolver::class, Util\UrlValidator::class],
         Service\VisitService::class => ['em'],
         Service\Tag\TagService::class => ['em'],
         Service\ShortUrl\DeleteShortUrlService::class => [
diff --git a/module/Core/src/Service/ShortUrlService.php b/module/Core/src/Service/ShortUrlService.php
index ae0cc7a3..5cdab93d 100644
--- a/module/Core/src/Service/ShortUrlService.php
+++ b/module/Core/src/Service/ShortUrlService.php
@@ -7,6 +7,7 @@ namespace Shlinkio\Shlink\Core\Service;
 use Doctrine\ORM;
 use Laminas\Paginator\Paginator;
 use Shlinkio\Shlink\Core\Entity\ShortUrl;
+use Shlinkio\Shlink\Core\Exception\InvalidUrlException;
 use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException;
 use Shlinkio\Shlink\Core\Model\ShortUrlEdit;
 use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier;
@@ -15,6 +16,7 @@ use Shlinkio\Shlink\Core\Paginator\Adapter\ShortUrlRepositoryAdapter;
 use Shlinkio\Shlink\Core\Repository\ShortUrlRepository;
 use Shlinkio\Shlink\Core\Service\ShortUrl\ShortUrlResolverInterface;
 use Shlinkio\Shlink\Core\Util\TagManagerTrait;
+use Shlinkio\Shlink\Core\Util\UrlValidatorInterface;
 
 class ShortUrlService implements ShortUrlServiceInterface
 {
@@ -22,11 +24,16 @@ class ShortUrlService implements ShortUrlServiceInterface
 
     private ORM\EntityManagerInterface $em;
     private ShortUrlResolverInterface $urlResolver;
+    private UrlValidatorInterface $urlValidator;
 
-    public function __construct(ORM\EntityManagerInterface $em, ShortUrlResolverInterface $urlResolver)
-    {
+    public function __construct(
+        ORM\EntityManagerInterface $em,
+        ShortUrlResolverInterface $urlResolver,
+        UrlValidatorInterface $urlValidator
+    ) {
         $this->em = $em;
         $this->urlResolver = $urlResolver;
+        $this->urlValidator = $urlValidator;
     }
 
     /**
@@ -59,9 +66,14 @@ class ShortUrlService implements ShortUrlServiceInterface
 
     /**
      * @throws ShortUrlNotFoundException
+     * @throws InvalidUrlException
      */
     public function updateMetadataByShortCode(ShortUrlIdentifier $identifier, ShortUrlEdit $shortUrlEdit): ShortUrl
     {
+        if ($shortUrlEdit->hasLongUrl()) {
+            $this->urlValidator->validateUrl($shortUrlEdit->longUrl());
+        }
+
         $shortUrl = $this->urlResolver->resolveShortUrl($identifier);
         $shortUrl->update($shortUrlEdit);
 
diff --git a/module/Core/src/Service/ShortUrlServiceInterface.php b/module/Core/src/Service/ShortUrlServiceInterface.php
index f17a7bea..3c09e7e9 100644
--- a/module/Core/src/Service/ShortUrlServiceInterface.php
+++ b/module/Core/src/Service/ShortUrlServiceInterface.php
@@ -6,6 +6,7 @@ namespace Shlinkio\Shlink\Core\Service;
 
 use Laminas\Paginator\Paginator;
 use Shlinkio\Shlink\Core\Entity\ShortUrl;
+use Shlinkio\Shlink\Core\Exception\InvalidUrlException;
 use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException;
 use Shlinkio\Shlink\Core\Model\ShortUrlEdit;
 use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier;
@@ -26,6 +27,7 @@ interface ShortUrlServiceInterface
 
     /**
      * @throws ShortUrlNotFoundException
+     * @throws InvalidUrlException
      */
     public function updateMetadataByShortCode(ShortUrlIdentifier $identifier, ShortUrlEdit $shortUrlEdit): ShortUrl;
 }
diff --git a/module/Core/test/Service/ShortUrlServiceTest.php b/module/Core/test/Service/ShortUrlServiceTest.php
index 4f40683e..9becdf8b 100644
--- a/module/Core/test/Service/ShortUrlServiceTest.php
+++ b/module/Core/test/Service/ShortUrlServiceTest.php
@@ -18,6 +18,7 @@ use Shlinkio\Shlink\Core\Model\ShortUrlsParams;
 use Shlinkio\Shlink\Core\Repository\ShortUrlRepository;
 use Shlinkio\Shlink\Core\Service\ShortUrl\ShortUrlResolverInterface;
 use Shlinkio\Shlink\Core\Service\ShortUrlService;
+use Shlinkio\Shlink\Core\Util\UrlValidatorInterface;
 
 use function count;
 
@@ -26,6 +27,7 @@ class ShortUrlServiceTest extends TestCase
     private ShortUrlService $service;
     private ObjectProphecy $em;
     private ObjectProphecy $urlResolver;
+    private ObjectProphecy $urlValidator;
 
     public function setUp(): void
     {
@@ -34,8 +36,13 @@ class ShortUrlServiceTest extends TestCase
         $this->em->flush()->willReturn(null);
 
         $this->urlResolver = $this->prophesize(ShortUrlResolverInterface::class);
+        $this->urlValidator = $this->prophesize(UrlValidatorInterface::class);
 
-        $this->service = new ShortUrlService($this->em->reveal(), $this->urlResolver->reveal());
+        $this->service = new ShortUrlService(
+            $this->em->reveal(),
+            $this->urlResolver->reveal(),
+            $this->urlValidator->reveal(),
+        );
     }
 
     /** @test */
@@ -74,27 +81,47 @@ class ShortUrlServiceTest extends TestCase
         $this->service->setTagsByShortCode(new ShortUrlIdentifier($shortCode), ['foo', 'bar']);
     }
 
-    /** @test */
-    public function updateMetadataByShortCodeUpdatesProvidedData(): void
-    {
-        $shortUrl = new ShortUrl('');
+    /**
+     * @test
+     * @dataProvider provideShortUrlEdits
+     */
+    public function updateMetadataByShortCodeUpdatesProvidedData(
+        int $expectedValidateCalls,
+        ShortUrlEdit $shortUrlEdit
+    ): void {
+        $originalLongUrl = 'originalLongUrl';
+        $shortUrl = new ShortUrl($originalLongUrl);
 
         $findShortUrl = $this->urlResolver->resolveShortUrl(new ShortUrlIdentifier('abc123'))->willReturn($shortUrl);
         $flush = $this->em->flush()->willReturn(null);
 
-        $result = $this->service->updateMetadataByShortCode(new ShortUrlIdentifier('abc123'), ShortUrlEdit::fromRawData(
+        $result = $this->service->updateMetadataByShortCode(new ShortUrlIdentifier('abc123'), $shortUrlEdit);
+
+        $this->assertSame($shortUrl, $result);
+        $this->assertEquals($shortUrlEdit->validSince(), $shortUrl->getValidSince());
+        $this->assertEquals($shortUrlEdit->validUntil(), $shortUrl->getValidUntil());
+        $this->assertEquals($shortUrlEdit->maxVisits(), $shortUrl->getMaxVisits());
+        $this->assertEquals($shortUrlEdit->longUrl() ?? $originalLongUrl, $shortUrl->getLongUrl());
+        $findShortUrl->shouldHaveBeenCalled();
+        $flush->shouldHaveBeenCalled();
+        $this->urlValidator->validateUrl($shortUrlEdit->longUrl())->shouldHaveBeenCalledTimes($expectedValidateCalls);
+    }
+
+    public function provideShortUrlEdits(): iterable
+    {
+        yield 'no long URL' => [0, ShortUrlEdit::fromRawData(
             [
                 'validSince' => Chronos::parse('2017-01-01 00:00:00')->toAtomString(),
                 'validUntil' => Chronos::parse('2017-01-05 00:00:00')->toAtomString(),
                 'maxVisits' => 5,
             ],
-        ));
-
-        $this->assertSame($shortUrl, $result);
-        $this->assertEquals(Chronos::parse('2017-01-01 00:00:00'), $shortUrl->getValidSince());
-        $this->assertEquals(Chronos::parse('2017-01-05 00:00:00'), $shortUrl->getValidUntil());
-        $this->assertEquals(5, $shortUrl->getMaxVisits());
-        $findShortUrl->shouldHaveBeenCalled();
-        $flush->shouldHaveBeenCalled();
+        )];
+        yield 'long URL' => [1, ShortUrlEdit::fromRawData(
+            [
+                'validSince' => Chronos::parse('2017-01-01 00:00:00')->toAtomString(),
+                'maxVisits' => 10,
+                'longUrl' => 'modifiedLongUrl',
+            ],
+        )];
     }
 }

From 3beb27acc27b47061aaabb33f15d87ea5a3e9db6 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Sun, 22 Mar 2020 17:30:01 +0100
Subject: [PATCH 53/67] Added API tests for the edition of the longURL

---
 .../Action/EditShortUrlActionTest.php         | 26 +++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/module/Rest/test-api/Action/EditShortUrlActionTest.php b/module/Rest/test-api/Action/EditShortUrlActionTest.php
index 171a40cc..b5cd4fd4 100644
--- a/module/Rest/test-api/Action/EditShortUrlActionTest.php
+++ b/module/Rest/test-api/Action/EditShortUrlActionTest.php
@@ -71,6 +71,32 @@ class EditShortUrlActionTest extends ApiTestCase
         return $matchingShortUrl['meta'] ?? null;
     }
 
+    /**
+     * @test
+     * @dataProvider provideLongUrls
+     */
+    public function longUrlCanBeEditedIfItIsValid(string $longUrl, int $expectedStatus, ?string $expectedError): void
+    {
+        $shortCode = 'abc123';
+        $url = sprintf('/short-urls/%s', $shortCode);
+
+        $resp = $this->callApiWithKey(self::METHOD_PATCH, $url, [RequestOptions::JSON => [
+            'longUrl' => $longUrl,
+        ]]);
+
+        $this->assertEquals($expectedStatus, $resp->getStatusCode());
+        if ($expectedError !== null) {
+            $payload = $this->getJsonResponsePayload($resp);
+            $this->assertEquals($expectedError, $payload['type']);
+        }
+    }
+
+    public function provideLongUrls(): iterable
+    {
+        yield 'valid URL' => ['https://shlink.io', self::STATUS_NO_CONTENT, null];
+        yield 'invalid URL' => ['htt:foo', self::STATUS_BAD_REQUEST, 'INVALID_URL'];
+    }
+
     /**
      * @test
      * @dataProvider provideInvalidUrls

From 774052a983fd4a3daac6fb0166cdead8dc6cd90f Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Sun, 22 Mar 2020 17:31:16 +0100
Subject: [PATCH 54/67] Updated changelog

---
 CHANGELOG.md | 1 +
 1 file changed, 1 insertion(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 8304e678..be5c993d 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -12,6 +12,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this
 * [#556](https://github.com/shlinkio/shlink/issues/556) Short code lengths can now be customized, both globally and on a per-short URL basis.
 * [#541](https://github.com/shlinkio/shlink/issues/541) Added a request ID that is returned on `X-Request-Id` header, can be provided from outside and is set in log entries.
 * [#642](https://github.com/shlinkio/shlink/issues/642) IP geolocation is now performed over the non-anonymized IP address when using swoole.
+* [#521](https://github.com/shlinkio/shlink/issues/521) The long URL for any existing short URL can now be edited using the `PATCH /short-urls/{shortCode}` endpoint.
 
 #### Changed
 

From 4539ab2dcff470917e4c1faec27c0f60f325e5ad Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Sun, 22 Mar 2020 17:42:56 +0100
Subject: [PATCH 55/67] Moved hardcoded class alias to a namespaced constant

---
 config/autoload/locks.global.php          | 6 +++---
 config/container.php                      | 7 +++++--
 module/CLI/config/dependencies.config.php | 4 +++-
 module/Core/functions/functions.php       | 1 +
 4 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/config/autoload/locks.global.php b/config/autoload/locks.global.php
index 22a51e38..25c00f22 100644
--- a/config/autoload/locks.global.php
+++ b/config/autoload/locks.global.php
@@ -8,7 +8,7 @@ use Shlinkio\Shlink\Common\Lock\RetryLockStoreDelegatorFactory;
 use Shlinkio\Shlink\Common\Logger\LoggerAwareDelegatorFactory;
 use Symfony\Component\Lock;
 
-$localLockFactory = 'Shlinkio\Shlink\LocalLockFactory';
+use const Shlinkio\Shlink\Core\LOCAL_LOCK_FACTORY;
 
 return [
 
@@ -21,7 +21,7 @@ return [
             Lock\Store\FlockStore::class => ConfigAbstractFactory::class,
             Lock\Store\RedisStore::class => ConfigAbstractFactory::class,
             Lock\LockFactory::class => ConfigAbstractFactory::class,
-            $localLockFactory => ConfigAbstractFactory::class,
+            LOCAL_LOCK_FACTORY => ConfigAbstractFactory::class,
         ],
         'aliases' => [
             // With this config, a user could alias 'lock_store' => 'redis_lock_store' to override the default
@@ -44,7 +44,7 @@ return [
         Lock\Store\FlockStore::class => ['config.locks.locks_dir'],
         Lock\Store\RedisStore::class => [RedisFactory::SERVICE_NAME],
         Lock\LockFactory::class => ['lock_store'],
-        $localLockFactory => ['local_lock_store'],
+        LOCAL_LOCK_FACTORY => ['local_lock_store'],
     ],
 
 ];
diff --git a/config/container.php b/config/container.php
index 3735e14e..7b6f0b08 100644
--- a/config/container.php
+++ b/config/container.php
@@ -5,13 +5,16 @@ declare(strict_types=1);
 use Laminas\ServiceManager\ServiceManager;
 use Symfony\Component\Lock;
 
+use const Shlinkio\Shlink\Core\LOCAL_LOCK_FACTORY;
+
 chdir(dirname(__DIR__));
 
 require 'vendor/autoload.php';
 
 // This class alias tricks the ConfigAbstractFactory to return Lock\Factory instances even with a different service name
-if (! class_exists('Shlinkio\Shlink\LocalLockFactory')) {
-    class_alias(Lock\LockFactory::class, 'Shlinkio\Shlink\LocalLockFactory');
+// It needs to be placed here as individual config files will not be loaded once config is cached
+if (! class_exists(LOCAL_LOCK_FACTORY)) {
+    class_alias(Lock\LockFactory::class, LOCAL_LOCK_FACTORY);
 }
 
 // Build container
diff --git a/module/CLI/config/dependencies.config.php b/module/CLI/config/dependencies.config.php
index 1cc67fd9..5edb6499 100644
--- a/module/CLI/config/dependencies.config.php
+++ b/module/CLI/config/dependencies.config.php
@@ -19,6 +19,8 @@ use Symfony\Component\Console as SymfonyCli;
 use Symfony\Component\Lock\LockFactory;
 use Symfony\Component\Process\PhpExecutableFinder;
 
+use const Shlinkio\Shlink\Core\LOCAL_LOCK_FACTORY;
+
 return [
 
     'dependencies' => [
@@ -52,7 +54,7 @@ return [
     ],
 
     ConfigAbstractFactory::class => [
-        GeolocationDbUpdater::class => [DbUpdater::class, Reader::class, 'Shlinkio\Shlink\LocalLockFactory'],
+        GeolocationDbUpdater::class => [DbUpdater::class, Reader::class, LOCAL_LOCK_FACTORY],
 
         Command\ShortUrl\GenerateShortUrlCommand::class => [
             Service\UrlShortener::class,
diff --git a/module/Core/functions/functions.php b/module/Core/functions/functions.php
index 87399208..3016b18c 100644
--- a/module/Core/functions/functions.php
+++ b/module/Core/functions/functions.php
@@ -12,6 +12,7 @@ use function sprintf;
 
 const DEFAULT_SHORT_CODES_LENGTH = 5;
 const MIN_SHORT_CODES_LENGTH = 4;
+const LOCAL_LOCK_FACTORY = 'Shlinkio\Shlink\LocalLockFactory';
 
 function generateRandomShortCode(int $length): string
 {

From c88401ef29a22ab29fcafbecd18d635f838f1109 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Mon, 23 Mar 2020 20:37:45 +0100
Subject: [PATCH 56/67] Added isEmpty column to VisitLocation

---
 data/migrations/Version20200323190014.php     | 44 +++++++++++++++++++
 ...inkio.Shlink.Core.Entity.VisitLocation.php |  6 +++
 module/Core/src/Entity/VisitLocation.php      | 28 +++++++-----
 3 files changed, 66 insertions(+), 12 deletions(-)
 create mode 100644 data/migrations/Version20200323190014.php

diff --git a/data/migrations/Version20200323190014.php b/data/migrations/Version20200323190014.php
new file mode 100644
index 00000000..fe3c340d
--- /dev/null
+++ b/data/migrations/Version20200323190014.php
@@ -0,0 +1,44 @@
+<?php
+
+declare(strict_types=1);
+
+namespace ShlinkMigrations;
+
+use Doctrine\DBAL\Schema\Schema;
+use Doctrine\DBAL\Types\Types;
+use Doctrine\Migrations\AbstractMigration;
+
+final class Version20200323190014 extends AbstractMigration
+{
+    public function up(Schema $schema): void
+    {
+        $visitLocations = $schema->getTable('visit_locations');
+        $this->skipIf($visitLocations->hasColumn('is_empty'));
+
+        $visitLocations->addColumn('is_empty', Types::BOOLEAN, ['default' => false]);
+    }
+
+    public function postUp(Schema $schema): void
+    {
+        $qb = $this->connection->createQueryBuilder();
+        $qb->update('visit_locations')
+           ->set('is_empty', true)
+           ->where($qb->expr()->eq('country_code', ':empty'))
+           ->andWhere($qb->expr()->eq('country_name', ':empty'))
+           ->andWhere($qb->expr()->eq('region_name', ':empty'))
+           ->andWhere($qb->expr()->eq('city_name', ':empty'))
+           ->andWhere($qb->expr()->eq('timezone', ':empty'))
+           ->andWhere($qb->expr()->eq('lat', 0))
+           ->andWhere($qb->expr()->eq('lon', 0))
+           ->setParameter('empty', '')
+           ->execute();
+    }
+
+    public function down(Schema $schema): void
+    {
+        $visitLocations = $schema->getTable('visit_locations');
+        $this->skipIf(!$visitLocations->hasColumn('is_empty'));
+
+        $visitLocations->dropColumn('is_empty');
+    }
+}
diff --git a/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Entity.VisitLocation.php b/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Entity.VisitLocation.php
index fde00abc..955fa1fa 100644
--- a/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Entity.VisitLocation.php
+++ b/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Entity.VisitLocation.php
@@ -44,4 +44,10 @@ return static function (ClassMetadata $metadata, array $emConfig): void {
             ->columnName('lon')
             ->nullable(false)
             ->build();
+
+    $builder->createField('isEmpty', Types::BOOLEAN)
+            ->columnName('is_empty')
+            ->option('default', false)
+            ->nullable(false)
+            ->build();
 };
diff --git a/module/Core/src/Entity/VisitLocation.php b/module/Core/src/Entity/VisitLocation.php
index 641b078e..ef545bba 100644
--- a/module/Core/src/Entity/VisitLocation.php
+++ b/module/Core/src/Entity/VisitLocation.php
@@ -17,6 +17,7 @@ class VisitLocation extends AbstractEntity implements VisitLocationInterface
     private float $latitude;
     private float $longitude;
     private string $timezone;
+    private bool $isEmpty;
 
     public function __construct(Location $location)
     {
@@ -43,6 +44,11 @@ class VisitLocation extends AbstractEntity implements VisitLocationInterface
         return $this->cityName;
     }
 
+    public function isEmpty(): bool
+    {
+        return $this->isEmpty;
+    }
+
     private function exchangeLocationInfo(Location $info): void
     {
         $this->countryCode = $info->countryCode();
@@ -52,6 +58,15 @@ class VisitLocation extends AbstractEntity implements VisitLocationInterface
         $this->latitude = $info->latitude();
         $this->longitude = $info->longitude();
         $this->timezone = $info->timeZone();
+        $this->isEmpty = (
+            $this->countryCode === '' &&
+            $this->countryName === '' &&
+            $this->regionName === '' &&
+            $this->cityName === '' &&
+            $this->latitude === 0.0 &&
+            $this->longitude === 0.0 &&
+            $this->timezone === ''
+        );
     }
 
     public function jsonSerialize(): array
@@ -64,18 +79,7 @@ class VisitLocation extends AbstractEntity implements VisitLocationInterface
             'latitude' => $this->latitude,
             'longitude' => $this->longitude,
             'timezone' => $this->timezone,
+            'isEmpty' => $this->isEmpty,
         ];
     }
-
-    public function isEmpty(): bool
-    {
-        return
-            $this->countryCode === '' &&
-            $this->countryName === '' &&
-            $this->regionName === '' &&
-            $this->cityName === '' &&
-            $this->latitude === 0.0 &&
-            $this->longitude === 0.0 &&
-            $this->timezone === '';
-    }
 }

From 5554675d03751a5df13c130e0cf82777ccdcac66 Mon Sep 17 00:00:00 2001
From: Lynne <lynne@bune.city>
Date: Wed, 25 Mar 2020 16:48:44 +1000
Subject: [PATCH 57/67] Update sample Nginx config to point to PHP 7.4

---
 README.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/README.md b/README.md
index 973a234b..4a7e25f6 100644
--- a/README.md
+++ b/README.md
@@ -97,7 +97,7 @@ Once Shlink is configured, you need to expose it to the web, either by using a t
 
         location ~ \.php$ {
             fastcgi_split_path_info ^(.+\.php)(/.+)$;
-            fastcgi_pass unix:/var/run/php/php7.2-fpm.sock;
+            fastcgi_pass unix:/var/run/php/php7.4-fpm.sock;
             fastcgi_index index.php;
             include fastcgi.conf;
         }

From b8522b8c176ec35ed08bac9d73610bf57586af1e Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Thu, 26 Mar 2020 22:17:13 +0100
Subject: [PATCH 58/67] Created new method to locate empty visits

---
 composer.json                                 |  2 +-
 config/autoload/request_id.global.php         |  5 +-
 module/CLI/config/dependencies.config.php     |  3 +-
 .../src/Command/Visit/LocateVisitsCommand.php | 48 ++++++++++++-------
 .../Command/Visit/LocateVisitsCommandTest.php |  4 +-
 module/Core/config/dependencies.config.php    |  4 +-
 .../Core/src/Repository/VisitRepository.php   |  7 +--
 .../src/Service/VisitServiceInterface.php     | 10 ----
 .../VisitLocator.php}                         | 26 ++++++----
 .../Core/src/Visit/VisitLocatorInterface.php  | 12 +++++
 .../VisitLocatorTest.php}                     | 11 +++--
 11 files changed, 82 insertions(+), 50 deletions(-)
 delete mode 100644 module/Core/src/Service/VisitServiceInterface.php
 rename module/Core/src/{Service/VisitService.php => Visit/VisitLocator.php} (71%)
 create mode 100644 module/Core/src/Visit/VisitLocatorInterface.php
 rename module/Core/test/{Service/VisitServiceTest.php => Visit/VisitLocatorTest.php} (93%)

diff --git a/composer.json b/composer.json
index 5ee67a4d..9cadd738 100644
--- a/composer.json
+++ b/composer.json
@@ -52,7 +52,7 @@
         "shlinkio/shlink-common": "^3.0",
         "shlinkio/shlink-config": "^1.0",
         "shlinkio/shlink-event-dispatcher": "^1.4",
-        "shlinkio/shlink-installer": "^4.3",
+        "shlinkio/shlink-installer": "^4.3.1",
         "shlinkio/shlink-ip-geolocation": "^1.4",
         "symfony/console": "^5.0",
         "symfony/filesystem": "^5.0",
diff --git a/config/autoload/request_id.global.php b/config/autoload/request_id.global.php
index 0a9ed6ce..f057bb09 100644
--- a/config/autoload/request_id.global.php
+++ b/config/autoload/request_id.global.php
@@ -28,7 +28,10 @@ return [
             'config.request_id.allow_override',
             'config.request_id.header_name',
         ],
-        RequestId\RequestIdMiddleware::class => [RequestId\RequestIdProviderFactory::class],
+        RequestId\RequestIdMiddleware::class => [
+            RequestId\RequestIdProviderFactory::class,
+            'config.request_id.header_name',
+        ],
         RequestId\MonologProcessor::class => [RequestId\RequestIdMiddleware::class],
     ],
 
diff --git a/module/CLI/config/dependencies.config.php b/module/CLI/config/dependencies.config.php
index 5edb6499..0f2e70a5 100644
--- a/module/CLI/config/dependencies.config.php
+++ b/module/CLI/config/dependencies.config.php
@@ -11,6 +11,7 @@ use Laminas\ServiceManager\Factory\InvokableFactory;
 use Shlinkio\Shlink\CLI\Util\GeolocationDbUpdater;
 use Shlinkio\Shlink\Common\Doctrine\NoDbNameConnectionFactory;
 use Shlinkio\Shlink\Core\Service;
+use Shlinkio\Shlink\Core\Visit;
 use Shlinkio\Shlink\Installer\Factory\ProcessHelperFactory;
 use Shlinkio\Shlink\IpGeolocation\GeoLite2\DbUpdater;
 use Shlinkio\Shlink\IpGeolocation\Resolver\IpLocationResolverInterface;
@@ -67,7 +68,7 @@ return [
         Command\ShortUrl\DeleteShortUrlCommand::class => [Service\ShortUrl\DeleteShortUrlService::class],
 
         Command\Visit\LocateVisitsCommand::class => [
-            Service\VisitService::class,
+            Visit\VisitLocator::class,
             IpLocationResolverInterface::class,
             LockFactory::class,
             GeolocationDbUpdater::class,
diff --git a/module/CLI/src/Command/Visit/LocateVisitsCommand.php b/module/CLI/src/Command/Visit/LocateVisitsCommand.php
index b19e8b19..935b9eee 100644
--- a/module/CLI/src/Command/Visit/LocateVisitsCommand.php
+++ b/module/CLI/src/Command/Visit/LocateVisitsCommand.php
@@ -4,7 +4,6 @@ declare(strict_types=1);
 
 namespace Shlinkio\Shlink\CLI\Command\Visit;
 
-use Exception;
 use Shlinkio\Shlink\CLI\Command\Util\AbstractLockedCommand;
 use Shlinkio\Shlink\CLI\Command\Util\LockedCommandConfig;
 use Shlinkio\Shlink\CLI\Exception\GeolocationDbUpdateFailedException;
@@ -14,12 +13,13 @@ use Shlinkio\Shlink\Common\Util\IpAddress;
 use Shlinkio\Shlink\Core\Entity\Visit;
 use Shlinkio\Shlink\Core\Entity\VisitLocation;
 use Shlinkio\Shlink\Core\Exception\IpCannotBeLocatedException;
-use Shlinkio\Shlink\Core\Service\VisitServiceInterface;
+use Shlinkio\Shlink\Core\Visit\VisitLocatorInterface;
 use Shlinkio\Shlink\IpGeolocation\Exception\WrongIpException;
 use Shlinkio\Shlink\IpGeolocation\Model\Location;
 use Shlinkio\Shlink\IpGeolocation\Resolver\IpLocationResolverInterface;
 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 Symfony\Component\Lock\LockFactory;
@@ -31,7 +31,7 @@ class LocateVisitsCommand extends AbstractLockedCommand
 {
     public const NAME = 'visit:locate';
 
-    private VisitServiceInterface $visitService;
+    private VisitLocatorInterface $visitLocator;
     private IpLocationResolverInterface $ipLocationResolver;
     private GeolocationDbUpdaterInterface $dbUpdater;
 
@@ -39,13 +39,13 @@ class LocateVisitsCommand extends AbstractLockedCommand
     private ?ProgressBar $progressBar = null;
 
     public function __construct(
-        VisitServiceInterface $visitService,
+        VisitLocatorInterface $visitLocator,
         IpLocationResolverInterface $ipLocationResolver,
         LockFactory $locker,
         GeolocationDbUpdaterInterface $dbUpdater
     ) {
         parent::__construct($locker);
-        $this->visitService = $visitService;
+        $this->visitLocator = $visitLocator;
         $this->ipLocationResolver = $ipLocationResolver;
         $this->dbUpdater = $dbUpdater;
     }
@@ -54,32 +54,46 @@ class LocateVisitsCommand extends AbstractLockedCommand
     {
         $this
             ->setName(self::NAME)
-            ->setDescription('Resolves visits origin locations.');
+            ->setDescription('Resolves visits origin locations.')
+            ->addOption(
+                'retry',
+                'r',
+                InputOption::VALUE_NONE,
+                'Will retry visits that were located with an empty location, in case it was a temporal issue.',
+            )
+            ->addOption(
+                'all',
+                'a',
+                InputOption::VALUE_NONE,
+                'Will locate all visits, ignoring if they have already been located.',
+            );
     }
 
     protected function lockedExecute(InputInterface $input, OutputInterface $output): int
     {
         $this->io = new SymfonyStyle($input, $output);
+        $retry = $input->getOption('retry');
+        $geolocateVisit = [$this, 'getGeolocationDataForVisit'];
+        $notifyVisitWithLocation = static function (VisitLocation $location) use ($output): void {
+            $message = ! $location->isEmpty()
+                ? sprintf(' [<info>Address located in "%s"</info>]', $location->getCountryName())
+                : ' [<comment>Address not found</comment>]';
+            $output->writeln($message);
+        };
 
         try {
             $this->checkDbUpdate();
 
-            $this->visitService->locateUnlocatedVisits(
-                [$this, 'getGeolocationDataForVisit'],
-                static function (VisitLocation $location) use ($output): void {
-                    if (!$location->isEmpty()) {
-                        $output->writeln(
-                            sprintf(' [<info>Address located at "%s"</info>]', $location->getCountryName()),
-                        );
-                    }
-                },
-            );
+            $this->visitLocator->locateUnlocatedVisits($geolocateVisit, $notifyVisitWithLocation);
+            if ($retry) {
+                $this->visitLocator->locateVisitsWithEmptyLocation($geolocateVisit, $notifyVisitWithLocation);
+            }
 
             $this->io->success('Finished processing all IPs');
             return ExitCodes::EXIT_SUCCESS;
         } catch (Throwable $e) {
             $this->io->error($e->getMessage());
-            if ($e instanceof Exception && $this->io->isVerbose()) {
+            if ($e instanceof Throwable && $this->io->isVerbose()) {
                 $this->getApplication()->renderThrowable($e, $this->io);
             }
 
diff --git a/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php b/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php
index 90073f10..94cd0bd1 100644
--- a/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php
+++ b/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php
@@ -15,7 +15,7 @@ use Shlinkio\Shlink\Core\Entity\ShortUrl;
 use Shlinkio\Shlink\Core\Entity\Visit;
 use Shlinkio\Shlink\Core\Entity\VisitLocation;
 use Shlinkio\Shlink\Core\Model\Visitor;
-use Shlinkio\Shlink\Core\Service\VisitService;
+use Shlinkio\Shlink\Core\Visit\VisitLocator;
 use Shlinkio\Shlink\IpGeolocation\Exception\WrongIpException;
 use Shlinkio\Shlink\IpGeolocation\Model\Location;
 use Shlinkio\Shlink\IpGeolocation\Resolver\IpLocationResolverInterface;
@@ -38,7 +38,7 @@ class LocateVisitsCommandTest extends TestCase
 
     public function setUp(): void
     {
-        $this->visitService = $this->prophesize(VisitService::class);
+        $this->visitService = $this->prophesize(VisitLocator::class);
         $this->ipResolver = $this->prophesize(IpLocationResolverInterface::class);
         $this->dbUpdater = $this->prophesize(GeolocationDbUpdaterInterface::class);
 
diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php
index 2800cdd6..13a74c36 100644
--- a/module/Core/config/dependencies.config.php
+++ b/module/Core/config/dependencies.config.php
@@ -28,7 +28,7 @@ return [
             Service\UrlShortener::class => ConfigAbstractFactory::class,
             Service\VisitsTracker::class => ConfigAbstractFactory::class,
             Service\ShortUrlService::class => ConfigAbstractFactory::class,
-            Service\VisitService::class => ConfigAbstractFactory::class,
+            Visit\VisitLocator::class => ConfigAbstractFactory::class,
             Service\Tag\TagService::class => ConfigAbstractFactory::class,
             Service\ShortUrl\DeleteShortUrlService::class => ConfigAbstractFactory::class,
             Service\ShortUrl\ShortUrlResolver::class => ConfigAbstractFactory::class,
@@ -57,7 +57,7 @@ return [
         Service\UrlShortener::class => [Util\UrlValidator::class, 'em', Resolver\PersistenceDomainResolver::class],
         Service\VisitsTracker::class => ['em', EventDispatcherInterface::class],
         Service\ShortUrlService::class => ['em', Service\ShortUrl\ShortUrlResolver::class, Util\UrlValidator::class],
-        Service\VisitService::class => ['em'],
+        Visit\VisitLocator::class => ['em'],
         Service\Tag\TagService::class => ['em'],
         Service\ShortUrl\DeleteShortUrlService::class => [
             'em',
diff --git a/module/Core/src/Repository/VisitRepository.php b/module/Core/src/Repository/VisitRepository.php
index 454323ef..28eb2466 100644
--- a/module/Core/src/Repository/VisitRepository.php
+++ b/module/Core/src/Repository/VisitRepository.php
@@ -14,7 +14,8 @@ class VisitRepository extends EntityRepository implements VisitRepositoryInterfa
     /**
      * This method will allow you to iterate the whole list of unlocated visits, but loading them into memory in
      * smaller blocks of a specific size.
-     * This will have side effects if you update those rows while you iterate them.
+     * This will have side effects if you update those rows while you iterate them, in a way that they are no longer
+     * unlocated.
      * If you plan to do so, pass the first argument as false in order to disable applying offsets while slicing the
      * dataset
      *
@@ -23,8 +24,8 @@ class VisitRepository extends EntityRepository implements VisitRepositoryInterfa
     public function findUnlocatedVisits(bool $applyOffset = true, int $blockSize = self::DEFAULT_BLOCK_SIZE): iterable
     {
         $dql = <<<DQL
-SELECT v FROM Shlinkio\Shlink\Core\Entity\Visit AS v WHERE v.visitLocation IS NULL
-DQL;
+            SELECT v FROM Shlinkio\Shlink\Core\Entity\Visit AS v WHERE v.visitLocation IS NULL
+        DQL;
         $query = $this->getEntityManager()->createQuery($dql)
                                           ->setMaxResults($blockSize);
         $remainingVisitsToProcess = $this->count(['visitLocation' => null]);
diff --git a/module/Core/src/Service/VisitServiceInterface.php b/module/Core/src/Service/VisitServiceInterface.php
deleted file mode 100644
index 78543549..00000000
--- a/module/Core/src/Service/VisitServiceInterface.php
+++ /dev/null
@@ -1,10 +0,0 @@
-<?php
-
-declare(strict_types=1);
-
-namespace Shlinkio\Shlink\Core\Service;
-
-interface VisitServiceInterface
-{
-    public function locateUnlocatedVisits(callable $geolocateVisit, ?callable $notifyVisitWithLocation = null): void;
-}
diff --git a/module/Core/src/Service/VisitService.php b/module/Core/src/Visit/VisitLocator.php
similarity index 71%
rename from module/Core/src/Service/VisitService.php
rename to module/Core/src/Visit/VisitLocator.php
index 20a30f4c..bcfc9e33 100644
--- a/module/Core/src/Service/VisitService.php
+++ b/module/Core/src/Visit/VisitLocator.php
@@ -2,7 +2,7 @@
 
 declare(strict_types=1);
 
-namespace Shlinkio\Shlink\Core\Service;
+namespace Shlinkio\Shlink\Core\Visit;
 
 use Doctrine\ORM\EntityManagerInterface;
 use Shlinkio\Shlink\Core\Entity\Visit;
@@ -11,7 +11,7 @@ use Shlinkio\Shlink\Core\Exception\IpCannotBeLocatedException;
 use Shlinkio\Shlink\Core\Repository\VisitRepository;
 use Shlinkio\Shlink\IpGeolocation\Model\Location;
 
-class VisitService implements VisitServiceInterface
+class VisitLocator implements VisitLocatorInterface
 {
     private EntityManagerInterface $em;
 
@@ -20,11 +20,23 @@ class VisitService implements VisitServiceInterface
         $this->em = $em;
     }
 
-    public function locateUnlocatedVisits(callable $geolocateVisit, ?callable $notifyVisitWithLocation = null): void
+    public function locateUnlocatedVisits(callable $geolocateVisit, callable $notifyVisitWithLocation): void
     {
         /** @var VisitRepository $repo */
         $repo = $this->em->getRepository(Visit::class);
-        $results = $repo->findUnlocatedVisits(false);
+        $this->locateVisits($repo->findUnlocatedVisits(false), $geolocateVisit, $notifyVisitWithLocation);
+    }
+
+    public function locateVisitsWithEmptyLocation(callable $geolocateVisit, callable $notifyVisitWithLocation): void
+    {
+        $this->locateVisits([], $geolocateVisit, $notifyVisitWithLocation);
+    }
+
+    /**
+     * @param iterable|Visit[] $results
+     */
+    private function locateVisits(iterable $results, callable $geolocateVisit, callable $notifyVisitWithLocation): void
+    {
         $count = 0;
         $persistBlock = 200;
 
@@ -58,13 +70,11 @@ class VisitService implements VisitServiceInterface
         $this->em->clear();
     }
 
-    private function locateVisit(Visit $visit, VisitLocation $location, ?callable $notifyVisitWithLocation): void
+    private function locateVisit(Visit $visit, VisitLocation $location, callable $notifyVisitWithLocation): void
     {
         $visit->locate($location);
         $this->em->persist($visit);
 
-        if ($notifyVisitWithLocation !== null) {
-            $notifyVisitWithLocation($location, $visit);
-        }
+        $notifyVisitWithLocation($location, $visit);
     }
 }
diff --git a/module/Core/src/Visit/VisitLocatorInterface.php b/module/Core/src/Visit/VisitLocatorInterface.php
new file mode 100644
index 00000000..ea06c1b1
--- /dev/null
+++ b/module/Core/src/Visit/VisitLocatorInterface.php
@@ -0,0 +1,12 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Shlinkio\Shlink\Core\Visit;
+
+interface VisitLocatorInterface
+{
+    public function locateUnlocatedVisits(callable $geolocateVisit, callable $notifyVisitWithLocation): void;
+
+    public function locateVisitsWithEmptyLocation(callable $geolocateVisit, callable $notifyVisitWithLocation): void;
+}
diff --git a/module/Core/test/Service/VisitServiceTest.php b/module/Core/test/Visit/VisitLocatorTest.php
similarity index 93%
rename from module/Core/test/Service/VisitServiceTest.php
rename to module/Core/test/Visit/VisitLocatorTest.php
index 26e95557..400a27c7 100644
--- a/module/Core/test/Service/VisitServiceTest.php
+++ b/module/Core/test/Visit/VisitLocatorTest.php
@@ -2,7 +2,7 @@
 
 declare(strict_types=1);
 
-namespace ShlinkioTest\Shlink\Core\Service;
+namespace ShlinkioTest\Shlink\Core\Visit;
 
 use Doctrine\ORM\EntityManager;
 use Exception;
@@ -15,7 +15,7 @@ use Shlinkio\Shlink\Core\Entity\VisitLocation;
 use Shlinkio\Shlink\Core\Exception\IpCannotBeLocatedException;
 use Shlinkio\Shlink\Core\Model\Visitor;
 use Shlinkio\Shlink\Core\Repository\VisitRepository;
-use Shlinkio\Shlink\Core\Service\VisitService;
+use Shlinkio\Shlink\Core\Visit\VisitLocator;
 use Shlinkio\Shlink\IpGeolocation\Model\Location;
 
 use function array_shift;
@@ -26,15 +26,15 @@ use function Functional\map;
 use function range;
 use function sprintf;
 
-class VisitServiceTest extends TestCase
+class VisitLocatorTest extends TestCase
 {
-    private VisitService $visitService;
+    private VisitLocator $visitService;
     private ObjectProphecy $em;
 
     public function setUp(): void
     {
         $this->em = $this->prophesize(EntityManager::class);
-        $this->visitService = new VisitService($this->em->reveal());
+        $this->visitService = new VisitLocator($this->em->reveal());
     }
 
     /** @test */
@@ -95,6 +95,7 @@ class VisitServiceTest extends TestCase
             throw $isNonLocatableAddress
                 ? new IpCannotBeLocatedException('Cannot be located')
                 : IpCannotBeLocatedException::forError(new Exception(''));
+        }, static function (): void {
         });
 
         $findUnlocatedVisits->shouldHaveBeenCalledOnce();

From f730c24ecb9f6a365209eab429b1e409df4195d6 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Thu, 26 Mar 2020 22:56:53 +0100
Subject: [PATCH 59/67] Created method to return visits with empty location

---
 .../Core/src/Repository/VisitRepository.php   | 46 +++++++++++++++++--
 .../Repository/VisitRepositoryInterface.php   | 19 ++++++--
 module/Core/src/Visit/VisitLocator.php        |  4 +-
 .../Repository/VisitRepositoryTest.php        | 34 +++++++-------
 4 files changed, 77 insertions(+), 26 deletions(-)

diff --git a/module/Core/src/Repository/VisitRepository.php b/module/Core/src/Repository/VisitRepository.php
index 28eb2466..aa3aa57d 100644
--- a/module/Core/src/Repository/VisitRepository.php
+++ b/module/Core/src/Repository/VisitRepository.php
@@ -5,32 +5,70 @@ declare(strict_types=1);
 namespace Shlinkio\Shlink\Core\Repository;
 
 use Doctrine\ORM\EntityRepository;
+use Doctrine\ORM\Query;
 use Doctrine\ORM\QueryBuilder;
 use Shlinkio\Shlink\Common\Util\DateRange;
 use Shlinkio\Shlink\Core\Entity\Visit;
 
 class VisitRepository extends EntityRepository implements VisitRepositoryInterface
 {
+    private const DEFAULT_BLOCK_SIZE = 10000;
+
     /**
      * This method will allow you to iterate the whole list of unlocated visits, but loading them into memory in
      * smaller blocks of a specific size.
      * This will have side effects if you update those rows while you iterate them, in a way that they are no longer
      * unlocated.
      * If you plan to do so, pass the first argument as false in order to disable applying offsets while slicing the
-     * dataset
+     * dataset.
      *
      * @return iterable|Visit[]
      */
-    public function findUnlocatedVisits(bool $applyOffset = true, int $blockSize = self::DEFAULT_BLOCK_SIZE): iterable
+    public function findUnlocatedVisits(bool $applyOffset = true): iterable
     {
         $dql = <<<DQL
             SELECT v FROM Shlinkio\Shlink\Core\Entity\Visit AS v WHERE v.visitLocation IS NULL
         DQL;
-        $query = $this->getEntityManager()->createQuery($dql)
-                                          ->setMaxResults($blockSize);
+        $query = $this->getEntityManager()->createQuery($dql);
         $remainingVisitsToProcess = $this->count(['visitLocation' => null]);
+
+        return $this->findVisitsForQuery($query, $remainingVisitsToProcess, $applyOffset);
+    }
+
+    /**
+     * This method will allow you to iterate the whole list of unlocated visits, but loading them into memory in
+     * smaller blocks of a specific size.
+     * This will have side effects if you update those rows while you iterate them, in a way that they are no longer
+     * unlocated.
+     * If you plan to do so, pass the first argument as false in order to disable applying offsets while slicing the
+     * dataset.
+     *
+     * @return iterable|Visit[]
+     */
+    public function findVisitsWithEmptyLocation(bool $applyOffset = true): iterable
+    {
+        $qb = $this->getEntityManager()->createQueryBuilder();
+        $qb->from(Visit::class, 'v')
+           ->join('v.visitLocation', 'vl')
+           ->where($qb->expr()->isNotNull('v.visitLocation'))
+           ->andWhere($qb->expr()->eq('vl.isEmpty', ':isEmpty'))
+           ->setParameter('isEmpty', true);
+        $countQb = clone $qb;
+
+        $query = $qb->select('v')->getQuery();
+        $remainingVisitsToProcess = (int) $countQb->select('COUNT(DISTINCT v.id)')->getQuery()->getSingleScalarResult();
+
+        return $this->findVisitsForQuery($query, $remainingVisitsToProcess, $applyOffset);
+    }
+
+    private function findVisitsForQuery(Query $query, int $remainingVisitsToProcess, bool $applyOffset = true): iterable
+    {
+        $blockSize = self::DEFAULT_BLOCK_SIZE;
+        $query = $query->setMaxResults($blockSize);
         $offset = 0;
 
+        // FIXME Do not use the $applyOffset workaround. Instead, always start with first result, but skip already
+        //       processed results. That should work both if any entry is edited or not
         while ($remainingVisitsToProcess > 0) {
             $iterator = $query->setFirstResult($applyOffset ? $offset : null)->iterate();
             foreach ($iterator as $key => [$value]) {
diff --git a/module/Core/src/Repository/VisitRepositoryInterface.php b/module/Core/src/Repository/VisitRepositoryInterface.php
index 0d0b66d0..ceb07564 100644
--- a/module/Core/src/Repository/VisitRepositoryInterface.php
+++ b/module/Core/src/Repository/VisitRepositoryInterface.php
@@ -10,18 +10,29 @@ use Shlinkio\Shlink\Core\Entity\Visit;
 
 interface VisitRepositoryInterface extends ObjectRepository
 {
-    public const DEFAULT_BLOCK_SIZE = 10000;
+    /**
+     * This method will allow you to iterate the whole list of unlocated visits, but loading them into memory in
+     * smaller blocks of a specific size.
+     * This will have side effects if you update those rows while you iterate them, in a way that they are no longer
+     * unlocated.
+     * If you plan to do so, pass the first argument as false in order to disable applying offsets while slicing the
+     * dataset.
+     *
+     * @return iterable|Visit[]
+     */
+    public function findUnlocatedVisits(bool $applyOffset = true): iterable;
 
     /**
      * This method will allow you to iterate the whole list of unlocated visits, but loading them into memory in
      * smaller blocks of a specific size.
-     * This will have side effects if you update those rows while you iterate them.
+     * This will have side effects if you update those rows while you iterate them, in a way that they are no longer
+     * unlocated.
      * If you plan to do so, pass the first argument as false in order to disable applying offsets while slicing the
-     * dataset
+     * dataset.
      *
      * @return iterable|Visit[]
      */
-    public function findUnlocatedVisits(bool $applyOffset = true, int $blockSize = self::DEFAULT_BLOCK_SIZE): iterable;
+    public function findVisitsWithEmptyLocation(bool $applyOffset = true): iterable;
 
     /**
      * @return Visit[]
diff --git a/module/Core/src/Visit/VisitLocator.php b/module/Core/src/Visit/VisitLocator.php
index bcfc9e33..9c6ed098 100644
--- a/module/Core/src/Visit/VisitLocator.php
+++ b/module/Core/src/Visit/VisitLocator.php
@@ -29,7 +29,9 @@ class VisitLocator implements VisitLocatorInterface
 
     public function locateVisitsWithEmptyLocation(callable $geolocateVisit, callable $notifyVisitWithLocation): void
     {
-        $this->locateVisits([], $geolocateVisit, $notifyVisitWithLocation);
+        /** @var VisitRepository $repo */
+        $repo = $this->em->getRepository(Visit::class);
+        $this->locateVisits($repo->findVisitsWithEmptyLocation(false), $geolocateVisit, $notifyVisitWithLocation);
     }
 
     /**
diff --git a/module/Core/test-db/Repository/VisitRepositoryTest.php b/module/Core/test-db/Repository/VisitRepositoryTest.php
index bd1a0f2d..60420e70 100644
--- a/module/Core/test-db/Repository/VisitRepositoryTest.php
+++ b/module/Core/test-db/Repository/VisitRepositoryTest.php
@@ -36,19 +36,24 @@ class VisitRepositoryTest extends DatabaseTestCase
         $this->repo = $this->getEntityManager()->getRepository(Visit::class);
     }
 
-    /**
-     * @test
-     * @dataProvider provideBlockSize
-     */
-    public function findUnlocatedVisitsReturnsProperVisits(int $blockSize): void
+    /** @test */
+    public function findVisitsReturnsProperVisits(): void
     {
         $shortUrl = new ShortUrl('');
         $this->getEntityManager()->persist($shortUrl);
+        $countIterable = function (iterable $results): int {
+            $resultsCount = 0;
+            foreach ($results as $value) {
+                $resultsCount++;
+            }
+
+            return $resultsCount;
+        };
 
         for ($i = 0; $i < 6; $i++) {
             $visit = new Visit($shortUrl, Visitor::emptyInstance());
 
-            if ($i % 2 === 0) {
+            if ($i >= 2) {
                 $location = new VisitLocation(Location::emptyInstance());
                 $this->getEntityManager()->persist($location);
                 $visit->locate($location);
@@ -58,18 +63,13 @@ class VisitRepositoryTest extends DatabaseTestCase
         }
         $this->getEntityManager()->flush();
 
-        $resultsCount = 0;
-        $results = $this->repo->findUnlocatedVisits(true, $blockSize);
-        foreach ($results as $value) {
-            $resultsCount++;
-        }
+        $withEmptyLocation = $this->repo->findVisitsWithEmptyLocation();
+        $unlocated = $this->repo->findUnlocatedVisits();
 
-        $this->assertEquals(3, $resultsCount);
-    }
-
-    public function provideBlockSize(): iterable
-    {
-        return map(range(1, 5), fn (int $value) => [$value]);
+        // Important! assertCount will not work here, as this iterable object loads data dynamically and counts to
+        // 0 if not iterated
+        $this->assertEquals(2, $countIterable($unlocated));
+        $this->assertEquals(4, $countIterable($withEmptyLocation));
     }
 
     /** @test */

From 43a3d469e7f74b244cb75778d38838cb7f24589f Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Fri, 27 Mar 2020 22:01:26 +0100
Subject: [PATCH 60/67] Improved how visits with some conditions are fetched
 from the database, so all internal logic is 100% transparent regardless the
 purpose

---
 .../Core/src/Repository/VisitRepository.php   | 70 +++++++------------
 .../Repository/VisitRepositoryInterface.php   | 26 ++-----
 module/Core/src/Visit/VisitLocator.php        |  4 +-
 .../Repository/VisitRepositoryTest.php        | 16 +++--
 module/Core/test/Visit/VisitLocatorTest.php   |  4 +-
 5 files changed, 49 insertions(+), 71 deletions(-)

diff --git a/module/Core/src/Repository/VisitRepository.php b/module/Core/src/Repository/VisitRepository.php
index aa3aa57d..ed18df10 100644
--- a/module/Core/src/Repository/VisitRepository.php
+++ b/module/Core/src/Repository/VisitRepository.php
@@ -5,79 +5,61 @@ declare(strict_types=1);
 namespace Shlinkio\Shlink\Core\Repository;
 
 use Doctrine\ORM\EntityRepository;
-use Doctrine\ORM\Query;
 use Doctrine\ORM\QueryBuilder;
 use Shlinkio\Shlink\Common\Util\DateRange;
 use Shlinkio\Shlink\Core\Entity\Visit;
 
 class VisitRepository extends EntityRepository implements VisitRepositoryInterface
 {
-    private const DEFAULT_BLOCK_SIZE = 10000;
-
     /**
-     * This method will allow you to iterate the whole list of unlocated visits, but loading them into memory in
-     * smaller blocks of a specific size.
-     * This will have side effects if you update those rows while you iterate them, in a way that they are no longer
-     * unlocated.
-     * If you plan to do so, pass the first argument as false in order to disable applying offsets while slicing the
-     * dataset.
-     *
      * @return iterable|Visit[]
      */
-    public function findUnlocatedVisits(bool $applyOffset = true): iterable
+    public function findUnlocatedVisits(int $blockSize = self::DEFAULT_BLOCK_SIZE): iterable
     {
-        $dql = <<<DQL
-            SELECT v FROM Shlinkio\Shlink\Core\Entity\Visit AS v WHERE v.visitLocation IS NULL
-        DQL;
-        $query = $this->getEntityManager()->createQuery($dql);
-        $remainingVisitsToProcess = $this->count(['visitLocation' => null]);
+        $qb = $this->getEntityManager()->createQueryBuilder();
+        $qb->select('v')
+           ->from(Visit::class, 'v')
+           ->where($qb->expr()->isNull('v.visitLocation'));
 
-        return $this->findVisitsForQuery($query, $remainingVisitsToProcess, $applyOffset);
+        return $this->findVisitsForQuery($qb, $blockSize);
     }
 
     /**
-     * This method will allow you to iterate the whole list of unlocated visits, but loading them into memory in
-     * smaller blocks of a specific size.
-     * This will have side effects if you update those rows while you iterate them, in a way that they are no longer
-     * unlocated.
-     * If you plan to do so, pass the first argument as false in order to disable applying offsets while slicing the
-     * dataset.
-     *
      * @return iterable|Visit[]
      */
-    public function findVisitsWithEmptyLocation(bool $applyOffset = true): iterable
+    public function findVisitsWithEmptyLocation(int $blockSize = self::DEFAULT_BLOCK_SIZE): iterable
     {
         $qb = $this->getEntityManager()->createQueryBuilder();
-        $qb->from(Visit::class, 'v')
+        $qb->select('v')
+           ->from(Visit::class, 'v')
            ->join('v.visitLocation', 'vl')
            ->where($qb->expr()->isNotNull('v.visitLocation'))
            ->andWhere($qb->expr()->eq('vl.isEmpty', ':isEmpty'))
            ->setParameter('isEmpty', true);
-        $countQb = clone $qb;
 
-        $query = $qb->select('v')->getQuery();
-        $remainingVisitsToProcess = (int) $countQb->select('COUNT(DISTINCT v.id)')->getQuery()->getSingleScalarResult();
-
-        return $this->findVisitsForQuery($query, $remainingVisitsToProcess, $applyOffset);
+        return $this->findVisitsForQuery($qb, $blockSize);
     }
 
-    private function findVisitsForQuery(Query $query, int $remainingVisitsToProcess, bool $applyOffset = true): iterable
+    private function findVisitsForQuery(QueryBuilder $qb, int $blockSize): iterable
     {
-        $blockSize = self::DEFAULT_BLOCK_SIZE;
-        $query = $query->setMaxResults($blockSize);
-        $offset = 0;
+        $originalQueryBuilder = $qb->setMaxResults($blockSize)
+                                   ->orderBy('v.id', 'ASC');
+        $lastId = '0';
 
-        // FIXME Do not use the $applyOffset workaround. Instead, always start with first result, but skip already
-        //       processed results. That should work both if any entry is edited or not
-        while ($remainingVisitsToProcess > 0) {
-            $iterator = $query->setFirstResult($applyOffset ? $offset : null)->iterate();
-            foreach ($iterator as $key => [$value]) {
-                yield $key => $value;
+        do {
+            $qb = (clone $originalQueryBuilder)->andWhere($qb->expr()->gt('v.id', $lastId));
+            $iterator = $qb->getQuery()->iterate();
+            $resultsFound = false;
+
+            /** @var Visit $visit */
+            foreach ($iterator as $key => [$visit]) {
+                $resultsFound = true;
+                yield $key => $visit;
             }
 
-            $remainingVisitsToProcess -= $blockSize;
-            $offset += $blockSize;
-        }
+            // As the query is ordered by ID, we can take the last one every time in order to exclude the whole list
+            $lastId = isset($visit) ? $visit->getId() : $lastId;
+        } while ($resultsFound);
     }
 
     /**
diff --git a/module/Core/src/Repository/VisitRepositoryInterface.php b/module/Core/src/Repository/VisitRepositoryInterface.php
index ceb07564..b076cff3 100644
--- a/module/Core/src/Repository/VisitRepositoryInterface.php
+++ b/module/Core/src/Repository/VisitRepositoryInterface.php
@@ -10,29 +10,17 @@ use Shlinkio\Shlink\Core\Entity\Visit;
 
 interface VisitRepositoryInterface extends ObjectRepository
 {
-    /**
-     * This method will allow you to iterate the whole list of unlocated visits, but loading them into memory in
-     * smaller blocks of a specific size.
-     * This will have side effects if you update those rows while you iterate them, in a way that they are no longer
-     * unlocated.
-     * If you plan to do so, pass the first argument as false in order to disable applying offsets while slicing the
-     * dataset.
-     *
-     * @return iterable|Visit[]
-     */
-    public function findUnlocatedVisits(bool $applyOffset = true): iterable;
+    public const DEFAULT_BLOCK_SIZE = 10000;
 
     /**
-     * This method will allow you to iterate the whole list of unlocated visits, but loading them into memory in
-     * smaller blocks of a specific size.
-     * This will have side effects if you update those rows while you iterate them, in a way that they are no longer
-     * unlocated.
-     * If you plan to do so, pass the first argument as false in order to disable applying offsets while slicing the
-     * dataset.
-     *
      * @return iterable|Visit[]
      */
-    public function findVisitsWithEmptyLocation(bool $applyOffset = true): iterable;
+    public function findUnlocatedVisits(int $defaultBlockSize = self::DEFAULT_BLOCK_SIZE): iterable;
+
+    /**
+     * @return iterable|Visit[]
+     */
+    public function findVisitsWithEmptyLocation(int $defaultBlockSize = self::DEFAULT_BLOCK_SIZE): iterable;
 
     /**
      * @return Visit[]
diff --git a/module/Core/src/Visit/VisitLocator.php b/module/Core/src/Visit/VisitLocator.php
index 9c6ed098..ee5acdf3 100644
--- a/module/Core/src/Visit/VisitLocator.php
+++ b/module/Core/src/Visit/VisitLocator.php
@@ -24,14 +24,14 @@ class VisitLocator implements VisitLocatorInterface
     {
         /** @var VisitRepository $repo */
         $repo = $this->em->getRepository(Visit::class);
-        $this->locateVisits($repo->findUnlocatedVisits(false), $geolocateVisit, $notifyVisitWithLocation);
+        $this->locateVisits($repo->findUnlocatedVisits(), $geolocateVisit, $notifyVisitWithLocation);
     }
 
     public function locateVisitsWithEmptyLocation(callable $geolocateVisit, callable $notifyVisitWithLocation): void
     {
         /** @var VisitRepository $repo */
         $repo = $this->em->getRepository(Visit::class);
-        $this->locateVisits($repo->findVisitsWithEmptyLocation(false), $geolocateVisit, $notifyVisitWithLocation);
+        $this->locateVisits($repo->findVisitsWithEmptyLocation(), $geolocateVisit, $notifyVisitWithLocation);
     }
 
     /**
diff --git a/module/Core/test-db/Repository/VisitRepositoryTest.php b/module/Core/test-db/Repository/VisitRepositoryTest.php
index 60420e70..b72453d6 100644
--- a/module/Core/test-db/Repository/VisitRepositoryTest.php
+++ b/module/Core/test-db/Repository/VisitRepositoryTest.php
@@ -36,8 +36,11 @@ class VisitRepositoryTest extends DatabaseTestCase
         $this->repo = $this->getEntityManager()->getRepository(Visit::class);
     }
 
-    /** @test */
-    public function findVisitsReturnsProperVisits(): void
+    /**
+     * @test
+     * @dataProvider provideBlockSize
+     */
+    public function findVisitsReturnsProperVisits(int $blockSize): void
     {
         $shortUrl = new ShortUrl('');
         $this->getEntityManager()->persist($shortUrl);
@@ -63,8 +66,8 @@ class VisitRepositoryTest extends DatabaseTestCase
         }
         $this->getEntityManager()->flush();
 
-        $withEmptyLocation = $this->repo->findVisitsWithEmptyLocation();
-        $unlocated = $this->repo->findUnlocatedVisits();
+        $withEmptyLocation = $this->repo->findVisitsWithEmptyLocation($blockSize);
+        $unlocated = $this->repo->findUnlocatedVisits($blockSize);
 
         // Important! assertCount will not work here, as this iterable object loads data dynamically and counts to
         // 0 if not iterated
@@ -72,6 +75,11 @@ class VisitRepositoryTest extends DatabaseTestCase
         $this->assertEquals(4, $countIterable($withEmptyLocation));
     }
 
+    public function provideBlockSize(): iterable
+    {
+        return map(range(1, 10), fn (int $value) => [$value]);
+    }
+
     /** @test */
     public function findVisitsByShortCodeReturnsProperData(): void
     {
diff --git a/module/Core/test/Visit/VisitLocatorTest.php b/module/Core/test/Visit/VisitLocatorTest.php
index 400a27c7..9e223e15 100644
--- a/module/Core/test/Visit/VisitLocatorTest.php
+++ b/module/Core/test/Visit/VisitLocatorTest.php
@@ -46,7 +46,7 @@ class VisitLocatorTest extends TestCase
         );
 
         $repo = $this->prophesize(VisitRepository::class);
-        $findUnlocatedVisits = $repo->findUnlocatedVisits(false)->willReturn($unlocatedVisits);
+        $findUnlocatedVisits = $repo->findUnlocatedVisits()->willReturn($unlocatedVisits);
         $getRepo = $this->em->getRepository(Visit::class)->willReturn($repo->reveal());
 
         $persist = $this->em->persist(Argument::type(Visit::class))->will(function (): void {
@@ -81,7 +81,7 @@ class VisitLocatorTest extends TestCase
         ];
 
         $repo = $this->prophesize(VisitRepository::class);
-        $findUnlocatedVisits = $repo->findUnlocatedVisits(false)->willReturn($unlocatedVisits);
+        $findUnlocatedVisits = $repo->findUnlocatedVisits()->willReturn($unlocatedVisits);
         $getRepo = $this->em->getRepository(Visit::class)->willReturn($repo->reveal());
 
         $persist = $this->em->persist(Argument::type(Visit::class))->will(function (): void {

From fcce18b0599b4eaa7279408b5ecc0ad4773b2561 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Sat, 28 Mar 2020 08:05:15 +0100
Subject: [PATCH 61/67] Changed VisitLocator signature so that it expects an
 object implementing an interface instead of two arbitrary callbacks

---
 .../src/Command/Visit/LocateVisitsCommand.php | 27 ++++----
 .../Command/Visit/LocateVisitsCommandTest.php | 37 +++++------
 .../Visit/VisitGeolocationHelperInterface.php | 20 ++++++
 module/Core/src/Visit/VisitLocator.php        | 30 ++++-----
 .../Core/src/Visit/VisitLocatorInterface.php  |  4 +-
 module/Core/test/Visit/VisitLocatorTest.php   | 61 +++++++++++++------
 6 files changed, 110 insertions(+), 69 deletions(-)
 create mode 100644 module/Core/src/Visit/VisitGeolocationHelperInterface.php

diff --git a/module/CLI/src/Command/Visit/LocateVisitsCommand.php b/module/CLI/src/Command/Visit/LocateVisitsCommand.php
index 935b9eee..89a8aa71 100644
--- a/module/CLI/src/Command/Visit/LocateVisitsCommand.php
+++ b/module/CLI/src/Command/Visit/LocateVisitsCommand.php
@@ -13,6 +13,7 @@ use Shlinkio\Shlink\Common\Util\IpAddress;
 use Shlinkio\Shlink\Core\Entity\Visit;
 use Shlinkio\Shlink\Core\Entity\VisitLocation;
 use Shlinkio\Shlink\Core\Exception\IpCannotBeLocatedException;
+use Shlinkio\Shlink\Core\Visit\VisitGeolocationHelperInterface;
 use Shlinkio\Shlink\Core\Visit\VisitLocatorInterface;
 use Shlinkio\Shlink\IpGeolocation\Exception\WrongIpException;
 use Shlinkio\Shlink\IpGeolocation\Model\Location;
@@ -27,7 +28,7 @@ use Throwable;
 
 use function sprintf;
 
-class LocateVisitsCommand extends AbstractLockedCommand
+class LocateVisitsCommand extends AbstractLockedCommand implements VisitGeolocationHelperInterface
 {
     public const NAME = 'visit:locate';
 
@@ -73,20 +74,13 @@ class LocateVisitsCommand extends AbstractLockedCommand
     {
         $this->io = new SymfonyStyle($input, $output);
         $retry = $input->getOption('retry');
-        $geolocateVisit = [$this, 'getGeolocationDataForVisit'];
-        $notifyVisitWithLocation = static function (VisitLocation $location) use ($output): void {
-            $message = ! $location->isEmpty()
-                ? sprintf(' [<info>Address located in "%s"</info>]', $location->getCountryName())
-                : ' [<comment>Address not found</comment>]';
-            $output->writeln($message);
-        };
 
         try {
             $this->checkDbUpdate();
 
-            $this->visitLocator->locateUnlocatedVisits($geolocateVisit, $notifyVisitWithLocation);
+            $this->visitLocator->locateUnlocatedVisits($this);
             if ($retry) {
-                $this->visitLocator->locateVisitsWithEmptyLocation($geolocateVisit, $notifyVisitWithLocation);
+                $this->visitLocator->locateVisitsWithEmptyLocation($this);
             }
 
             $this->io->success('Finished processing all IPs');
@@ -101,7 +95,10 @@ class LocateVisitsCommand extends AbstractLockedCommand
         }
     }
 
-    public function getGeolocationDataForVisit(Visit $visit): Location
+    /**
+     * @throws IpCannotBeLocatedException
+     */
+    public function geolocateVisit(Visit $visit): Location
     {
         if (! $visit->hasRemoteAddr()) {
             $this->io->writeln(
@@ -130,6 +127,14 @@ class LocateVisitsCommand extends AbstractLockedCommand
         }
     }
 
+    public function onVisitLocated(VisitLocation $visitLocation, Visit $visit): void
+    {
+        $message = ! $visitLocation->isEmpty()
+            ? sprintf(' [<info>Address located in "%s"</info>]', $visitLocation->getCountryName())
+            : ' [<comment>Address not found</comment>]';
+        $this->io->writeln($message);
+    }
+
     private function checkDbUpdate(): void
     {
         try {
diff --git a/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php b/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php
index 94cd0bd1..30be9846 100644
--- a/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php
+++ b/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php
@@ -15,6 +15,7 @@ use Shlinkio\Shlink\Core\Entity\ShortUrl;
 use Shlinkio\Shlink\Core\Entity\Visit;
 use Shlinkio\Shlink\Core\Entity\VisitLocation;
 use Shlinkio\Shlink\Core\Model\Visitor;
+use Shlinkio\Shlink\Core\Visit\VisitGeolocationHelperInterface;
 use Shlinkio\Shlink\Core\Visit\VisitLocator;
 use Shlinkio\Shlink\IpGeolocation\Exception\WrongIpException;
 use Shlinkio\Shlink\IpGeolocation\Model\Location;
@@ -24,7 +25,6 @@ use Symfony\Component\Console\Output\OutputInterface;
 use Symfony\Component\Console\Tester\CommandTester;
 use Symfony\Component\Lock;
 
-use function array_shift;
 use function sprintf;
 
 class LocateVisitsCommandTest extends TestCase
@@ -68,13 +68,7 @@ class LocateVisitsCommandTest extends TestCase
         $location = new VisitLocation(Location::emptyInstance());
 
         $locateVisits = $this->visitService->locateUnlocatedVisits(Argument::cetera())->will(
-            function (array $args) use ($visit, $location): void {
-                $firstCallback = array_shift($args);
-                $firstCallback($visit);
-
-                $secondCallback = array_shift($args);
-                $secondCallback($location, $visit);
-            },
+            $this->invokeHelperMethods($visit, $location),
         );
         $resolveIpLocation = $this->ipResolver->resolveIpLocation(Argument::any())->willReturn(
             Location::emptyInstance(),
@@ -98,13 +92,7 @@ class LocateVisitsCommandTest extends TestCase
         $location = new VisitLocation(Location::emptyInstance());
 
         $locateVisits = $this->visitService->locateUnlocatedVisits(Argument::cetera())->will(
-            function (array $args) use ($visit, $location): void {
-                $firstCallback = array_shift($args);
-                $firstCallback($visit);
-
-                $secondCallback = array_shift($args);
-                $secondCallback($location, $visit);
-            },
+            $this->invokeHelperMethods($visit, $location),
         );
         $resolveIpLocation = $this->ipResolver->resolveIpLocation(Argument::any())->willReturn(
             Location::emptyInstance(),
@@ -137,13 +125,7 @@ class LocateVisitsCommandTest extends TestCase
         $location = new VisitLocation(Location::emptyInstance());
 
         $locateVisits = $this->visitService->locateUnlocatedVisits(Argument::cetera())->will(
-            function (array $args) use ($visit, $location): void {
-                $firstCallback = array_shift($args);
-                $firstCallback($visit);
-
-                $secondCallback = array_shift($args);
-                $secondCallback($location, $visit);
-            },
+            $this->invokeHelperMethods($visit, $location),
         );
         $resolveIpLocation = $this->ipResolver->resolveIpLocation(Argument::any())->willThrow(WrongIpException::class);
 
@@ -156,6 +138,17 @@ class LocateVisitsCommandTest extends TestCase
         $resolveIpLocation->shouldHaveBeenCalledOnce();
     }
 
+    private function invokeHelperMethods(Visit $visit, VisitLocation $location): callable
+    {
+        return function (array $args) use ($visit, $location): void {
+            /** @var VisitGeolocationHelperInterface $helper */
+            [$helper] = $args;
+
+            $helper->geolocateVisit($visit);
+            $helper->onVisitLocated($location, $visit);
+        };
+    }
+
     /** @test */
     public function noActionIsPerformedIfLockIsAcquired(): void
     {
diff --git a/module/Core/src/Visit/VisitGeolocationHelperInterface.php b/module/Core/src/Visit/VisitGeolocationHelperInterface.php
new file mode 100644
index 00000000..95cca4a7
--- /dev/null
+++ b/module/Core/src/Visit/VisitGeolocationHelperInterface.php
@@ -0,0 +1,20 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Shlinkio\Shlink\Core\Visit;
+
+use Shlinkio\Shlink\Core\Entity\Visit;
+use Shlinkio\Shlink\Core\Entity\VisitLocation;
+use Shlinkio\Shlink\Core\Exception\IpCannotBeLocatedException;
+use Shlinkio\Shlink\IpGeolocation\Model\Location;
+
+interface VisitGeolocationHelperInterface
+{
+    /**
+     * @throws IpCannotBeLocatedException
+     */
+    public function geolocateVisit(Visit $visit): Location;
+
+    public function onVisitLocated(VisitLocation $visitLocation, Visit $visit): void;
+}
diff --git a/module/Core/src/Visit/VisitLocator.php b/module/Core/src/Visit/VisitLocator.php
index ee5acdf3..5a66208b 100644
--- a/module/Core/src/Visit/VisitLocator.php
+++ b/module/Core/src/Visit/VisitLocator.php
@@ -8,36 +8,37 @@ use Doctrine\ORM\EntityManagerInterface;
 use Shlinkio\Shlink\Core\Entity\Visit;
 use Shlinkio\Shlink\Core\Entity\VisitLocation;
 use Shlinkio\Shlink\Core\Exception\IpCannotBeLocatedException;
-use Shlinkio\Shlink\Core\Repository\VisitRepository;
+use Shlinkio\Shlink\Core\Repository\VisitRepositoryInterface;
 use Shlinkio\Shlink\IpGeolocation\Model\Location;
 
 class VisitLocator implements VisitLocatorInterface
 {
     private EntityManagerInterface $em;
+    private VisitRepositoryInterface $repo;
 
     public function __construct(EntityManagerInterface $em)
     {
         $this->em = $em;
+
+        /** @var VisitRepositoryInterface $repo */
+        $repo = $em->getRepository(Visit::class);
+        $this->repo = $repo;
     }
 
-    public function locateUnlocatedVisits(callable $geolocateVisit, callable $notifyVisitWithLocation): void
+    public function locateUnlocatedVisits(VisitGeolocationHelperInterface $helper): void
     {
-        /** @var VisitRepository $repo */
-        $repo = $this->em->getRepository(Visit::class);
-        $this->locateVisits($repo->findUnlocatedVisits(), $geolocateVisit, $notifyVisitWithLocation);
+        $this->locateVisits($this->repo->findUnlocatedVisits(), $helper);
     }
 
-    public function locateVisitsWithEmptyLocation(callable $geolocateVisit, callable $notifyVisitWithLocation): void
+    public function locateVisitsWithEmptyLocation(VisitGeolocationHelperInterface $helper): void
     {
-        /** @var VisitRepository $repo */
-        $repo = $this->em->getRepository(Visit::class);
-        $this->locateVisits($repo->findVisitsWithEmptyLocation(), $geolocateVisit, $notifyVisitWithLocation);
+        $this->locateVisits($this->repo->findVisitsWithEmptyLocation(), $helper);
     }
 
     /**
      * @param iterable|Visit[] $results
      */
-    private function locateVisits(iterable $results, callable $geolocateVisit, callable $notifyVisitWithLocation): void
+    private function locateVisits(iterable $results, VisitGeolocationHelperInterface $helper): void
     {
         $count = 0;
         $persistBlock = 200;
@@ -46,8 +47,7 @@ class VisitLocator implements VisitLocatorInterface
             $count++;
 
             try {
-                /** @var Location $location */
-                $location = $geolocateVisit($visit);
+                $location = $helper->geolocateVisit($visit);
             } catch (IpCannotBeLocatedException $e) {
                 if (! $e->isNonLocatableAddress()) {
                     // Skip if the visit's IP could not be located because of an error
@@ -59,7 +59,7 @@ class VisitLocator implements VisitLocatorInterface
             }
 
             $location = new VisitLocation($location);
-            $this->locateVisit($visit, $location, $notifyVisitWithLocation);
+            $this->locateVisit($visit, $location, $helper);
 
             // Flush and clear after X iterations
             if ($count % $persistBlock === 0) {
@@ -72,11 +72,11 @@ class VisitLocator implements VisitLocatorInterface
         $this->em->clear();
     }
 
-    private function locateVisit(Visit $visit, VisitLocation $location, callable $notifyVisitWithLocation): void
+    private function locateVisit(Visit $visit, VisitLocation $location, VisitGeolocationHelperInterface $helper): void
     {
         $visit->locate($location);
         $this->em->persist($visit);
 
-        $notifyVisitWithLocation($location, $visit);
+        $helper->onVisitLocated($location, $visit);
     }
 }
diff --git a/module/Core/src/Visit/VisitLocatorInterface.php b/module/Core/src/Visit/VisitLocatorInterface.php
index ea06c1b1..6e06d296 100644
--- a/module/Core/src/Visit/VisitLocatorInterface.php
+++ b/module/Core/src/Visit/VisitLocatorInterface.php
@@ -6,7 +6,7 @@ namespace Shlinkio\Shlink\Core\Visit;
 
 interface VisitLocatorInterface
 {
-    public function locateUnlocatedVisits(callable $geolocateVisit, callable $notifyVisitWithLocation): void;
+    public function locateUnlocatedVisits(VisitGeolocationHelperInterface $helper): void;
 
-    public function locateVisitsWithEmptyLocation(callable $geolocateVisit, callable $notifyVisitWithLocation): void;
+    public function locateVisitsWithEmptyLocation(VisitGeolocationHelperInterface $helper): void;
 }
diff --git a/module/Core/test/Visit/VisitLocatorTest.php b/module/Core/test/Visit/VisitLocatorTest.php
index 9e223e15..1972860b 100644
--- a/module/Core/test/Visit/VisitLocatorTest.php
+++ b/module/Core/test/Visit/VisitLocatorTest.php
@@ -6,6 +6,7 @@ namespace ShlinkioTest\Shlink\Core\Visit;
 
 use Doctrine\ORM\EntityManager;
 use Exception;
+use PHPUnit\Framework\Assert;
 use PHPUnit\Framework\TestCase;
 use Prophecy\Argument;
 use Prophecy\Prophecy\ObjectProphecy;
@@ -14,7 +15,8 @@ use Shlinkio\Shlink\Core\Entity\Visit;
 use Shlinkio\Shlink\Core\Entity\VisitLocation;
 use Shlinkio\Shlink\Core\Exception\IpCannotBeLocatedException;
 use Shlinkio\Shlink\Core\Model\Visitor;
-use Shlinkio\Shlink\Core\Repository\VisitRepository;
+use Shlinkio\Shlink\Core\Repository\VisitRepositoryInterface;
+use Shlinkio\Shlink\Core\Visit\VisitGeolocationHelperInterface;
 use Shlinkio\Shlink\Core\Visit\VisitLocator;
 use Shlinkio\Shlink\IpGeolocation\Model\Location;
 
@@ -30,10 +32,14 @@ class VisitLocatorTest extends TestCase
 {
     private VisitLocator $visitService;
     private ObjectProphecy $em;
+    private ObjectProphecy $repo;
 
     public function setUp(): void
     {
         $this->em = $this->prophesize(EntityManager::class);
+        $this->repo = $this->prophesize(VisitRepositoryInterface::class);
+        $this->em->getRepository(Visit::class)->willReturn($this->repo->reveal());
+
         $this->visitService = new VisitLocator($this->em->reveal());
     }
 
@@ -45,9 +51,7 @@ class VisitLocatorTest extends TestCase
             fn (int $i) => new Visit(new ShortUrl(sprintf('short_code_%s', $i)), Visitor::emptyInstance()),
         );
 
-        $repo = $this->prophesize(VisitRepository::class);
-        $findUnlocatedVisits = $repo->findUnlocatedVisits()->willReturn($unlocatedVisits);
-        $getRepo = $this->em->getRepository(Visit::class)->willReturn($repo->reveal());
+        $findUnlocatedVisits = $this->repo->findUnlocatedVisits()->willReturn($unlocatedVisits);
 
         $persist = $this->em->persist(Argument::type(Visit::class))->will(function (): void {
         });
@@ -56,15 +60,22 @@ class VisitLocatorTest extends TestCase
         $clear = $this->em->clear()->will(function (): void {
         });
 
-        $this->visitService->locateUnlocatedVisits(fn () => Location::emptyInstance(), function (): void {
-            $args = func_get_args();
+        $this->visitService->locateUnlocatedVisits(new class implements VisitGeolocationHelperInterface {
+            public function geolocateVisit(Visit $visit): Location
+            {
+                return Location::emptyInstance();
+            }
 
-            $this->assertInstanceOf(VisitLocation::class, array_shift($args));
-            $this->assertInstanceOf(Visit::class, array_shift($args));
+            public function onVisitLocated(VisitLocation $visitLocation, Visit $visit): void
+            {
+                $args = func_get_args();
+
+                Assert::assertInstanceOf(VisitLocation::class, array_shift($args));
+                Assert::assertInstanceOf(Visit::class, array_shift($args));
+            }
         });
 
         $findUnlocatedVisits->shouldHaveBeenCalledOnce();
-        $getRepo->shouldHaveBeenCalledOnce();
         $persist->shouldHaveBeenCalledTimes(count($unlocatedVisits));
         $flush->shouldHaveBeenCalledTimes(floor(count($unlocatedVisits) / 200) + 1);
         $clear->shouldHaveBeenCalledTimes(floor(count($unlocatedVisits) / 200) + 1);
@@ -80,9 +91,7 @@ class VisitLocatorTest extends TestCase
             new Visit(new ShortUrl('foo'), Visitor::emptyInstance()),
         ];
 
-        $repo = $this->prophesize(VisitRepository::class);
-        $findUnlocatedVisits = $repo->findUnlocatedVisits()->willReturn($unlocatedVisits);
-        $getRepo = $this->em->getRepository(Visit::class)->willReturn($repo->reveal());
+        $findUnlocatedVisits = $this->repo->findUnlocatedVisits()->willReturn($unlocatedVisits);
 
         $persist = $this->em->persist(Argument::type(Visit::class))->will(function (): void {
         });
@@ -91,15 +100,29 @@ class VisitLocatorTest extends TestCase
         $clear = $this->em->clear()->will(function (): void {
         });
 
-        $this->visitService->locateUnlocatedVisits(function () use ($isNonLocatableAddress): void {
-            throw $isNonLocatableAddress
-                ? new IpCannotBeLocatedException('Cannot be located')
-                : IpCannotBeLocatedException::forError(new Exception(''));
-        }, static function (): void {
-        });
+        $this->visitService->locateUnlocatedVisits(
+            new class ($isNonLocatableAddress) implements VisitGeolocationHelperInterface {
+                private bool $isNonLocatableAddress;
+
+                public function __construct(bool $isNonLocatableAddress)
+                {
+                    $this->isNonLocatableAddress = $isNonLocatableAddress;
+                }
+
+                public function geolocateVisit(Visit $visit): Location
+                {
+                    throw $this->isNonLocatableAddress
+                        ? new IpCannotBeLocatedException('Cannot be located')
+                        : IpCannotBeLocatedException::forError(new Exception(''));
+                }
+
+                public function onVisitLocated(VisitLocation $visitLocation, Visit $visit): void
+                {
+                }
+            },
+        );
 
         $findUnlocatedVisits->shouldHaveBeenCalledOnce();
-        $getRepo->shouldHaveBeenCalledOnce();
         $persist->shouldHaveBeenCalledTimes($isNonLocatableAddress ? 1 : 0);
         $flush->shouldHaveBeenCalledOnce();
         $clear->shouldHaveBeenCalledOnce();

From fb8ab0b5fe1b7f730b46ff576abcd7665d43bde7 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Sat, 28 Mar 2020 09:12:15 +0100
Subject: [PATCH 62/67] Implemented how to reprocess the locations of all
 existing visits

---
 .../src/Command/Visit/LocateVisitsCommand.php | 51 ++++++++++++++++---
 .../Core/src/Repository/VisitRepository.php   |  9 ++++
 .../Repository/VisitRepositoryInterface.php   |  9 +++-
 module/Core/src/Visit/VisitLocator.php        |  5 ++
 .../Core/src/Visit/VisitLocatorInterface.php  |  2 +
 .../Repository/VisitRepositoryTest.php        |  6 ++-
 6 files changed, 71 insertions(+), 11 deletions(-)

diff --git a/module/CLI/src/Command/Visit/LocateVisitsCommand.php b/module/CLI/src/Command/Visit/LocateVisitsCommand.php
index 89a8aa71..1a7d0bd0 100644
--- a/module/CLI/src/Command/Visit/LocateVisitsCommand.php
+++ b/module/CLI/src/Command/Visit/LocateVisitsCommand.php
@@ -18,6 +18,7 @@ use Shlinkio\Shlink\Core\Visit\VisitLocatorInterface;
 use Shlinkio\Shlink\IpGeolocation\Exception\WrongIpException;
 use Shlinkio\Shlink\IpGeolocation\Model\Location;
 use Shlinkio\Shlink\IpGeolocation\Resolver\IpLocationResolverInterface;
+use Symfony\Component\Console\Exception\RuntimeException;
 use Symfony\Component\Console\Helper\ProgressBar;
 use Symfony\Component\Console\Input\InputInterface;
 use Symfony\Component\Console\Input\InputOption;
@@ -60,30 +61,66 @@ class LocateVisitsCommand extends AbstractLockedCommand implements VisitGeolocat
                 'retry',
                 'r',
                 InputOption::VALUE_NONE,
-                'Will retry visits that were located with an empty location, in case it was a temporal issue.',
+                'Will retry the location of visits that were located with a not-found location, in case it was due to '
+                . 'a temporal issue.',
             )
             ->addOption(
                 'all',
                 'a',
                 InputOption::VALUE_NONE,
-                'Will locate all visits, ignoring if they have already been located.',
+                'When provided together with --retry, will locate all existing visits, regardless the fact that they '
+                . 'have already been located.',
             );
     }
 
+    protected function interact(InputInterface $input, OutputInterface $output): void
+    {
+        $this->io = new SymfonyStyle($input, $output);
+        $retry = $input->getOption('retry');
+        $all = $input->getOption('all');
+
+        if ($all && !$retry) {
+            $this->io->writeln(
+                '<comment>The <fg=yellow;options=bold>--all</> flag has no effect on its own. You have to provide it '
+                . 'together with <fg=yellow;options=bold>--retry</>.</comment>',
+            );
+        }
+
+        if ($all && $retry && ! $this->warnAndVerifyContinue()) {
+            throw new RuntimeException('Execution aborted');
+        }
+    }
+
+    private function warnAndVerifyContinue(): bool
+    {
+        $this->io->warning([
+            'You are about to process the location of all existing visits your short URLs received.',
+            'Since shlink saves visitors IP addresses anonymized, you could end up losing precision on some of '
+            . 'your visits.',
+            'Also, if you have a large amount of visits, this can be a very time consuming process. '
+            . 'Continue at your own risk.',
+        ]);
+        return $this->io->confirm('Do you want to proceed?', false);
+    }
+
     protected function lockedExecute(InputInterface $input, OutputInterface $output): int
     {
-        $this->io = new SymfonyStyle($input, $output);
         $retry = $input->getOption('retry');
+        $all = $retry && $input->getOption('all');
 
         try {
             $this->checkDbUpdate();
 
-            $this->visitLocator->locateUnlocatedVisits($this);
-            if ($retry) {
-                $this->visitLocator->locateVisitsWithEmptyLocation($this);
+            if ($all) {
+                $this->visitLocator->locateAllVisits($this);
+            } else {
+                $this->visitLocator->locateUnlocatedVisits($this);
+                if ($retry) {
+                    $this->visitLocator->locateVisitsWithEmptyLocation($this);
+                }
             }
 
-            $this->io->success('Finished processing all IPs');
+            $this->io->success('Finished locating visits');
             return ExitCodes::EXIT_SUCCESS;
         } catch (Throwable $e) {
             $this->io->error($e->getMessage());
diff --git a/module/Core/src/Repository/VisitRepository.php b/module/Core/src/Repository/VisitRepository.php
index ed18df10..61b2afb8 100644
--- a/module/Core/src/Repository/VisitRepository.php
+++ b/module/Core/src/Repository/VisitRepository.php
@@ -40,6 +40,15 @@ class VisitRepository extends EntityRepository implements VisitRepositoryInterfa
         return $this->findVisitsForQuery($qb, $blockSize);
     }
 
+    public function findAllVisits(int $blockSize = self::DEFAULT_BLOCK_SIZE): iterable
+    {
+        $qb = $this->getEntityManager()->createQueryBuilder();
+        $qb->select('v')
+           ->from(Visit::class, 'v');
+
+        return $this->findVisitsForQuery($qb, $blockSize);
+    }
+
     private function findVisitsForQuery(QueryBuilder $qb, int $blockSize): iterable
     {
         $originalQueryBuilder = $qb->setMaxResults($blockSize)
diff --git a/module/Core/src/Repository/VisitRepositoryInterface.php b/module/Core/src/Repository/VisitRepositoryInterface.php
index b076cff3..f9cbc8d9 100644
--- a/module/Core/src/Repository/VisitRepositoryInterface.php
+++ b/module/Core/src/Repository/VisitRepositoryInterface.php
@@ -15,12 +15,17 @@ interface VisitRepositoryInterface extends ObjectRepository
     /**
      * @return iterable|Visit[]
      */
-    public function findUnlocatedVisits(int $defaultBlockSize = self::DEFAULT_BLOCK_SIZE): iterable;
+    public function findUnlocatedVisits(int $blockSize = self::DEFAULT_BLOCK_SIZE): iterable;
 
     /**
      * @return iterable|Visit[]
      */
-    public function findVisitsWithEmptyLocation(int $defaultBlockSize = self::DEFAULT_BLOCK_SIZE): iterable;
+    public function findVisitsWithEmptyLocation(int $blockSize = self::DEFAULT_BLOCK_SIZE): iterable;
+
+    /**
+     * @return iterable|Visit[]
+     */
+    public function findAllVisits(int $blockSize = self::DEFAULT_BLOCK_SIZE): iterable;
 
     /**
      * @return Visit[]
diff --git a/module/Core/src/Visit/VisitLocator.php b/module/Core/src/Visit/VisitLocator.php
index 5a66208b..f955ef29 100644
--- a/module/Core/src/Visit/VisitLocator.php
+++ b/module/Core/src/Visit/VisitLocator.php
@@ -35,6 +35,11 @@ class VisitLocator implements VisitLocatorInterface
         $this->locateVisits($this->repo->findVisitsWithEmptyLocation(), $helper);
     }
 
+    public function locateAllVisits(VisitGeolocationHelperInterface $helper): void
+    {
+        $this->locateVisits($this->repo->findAllVisits(), $helper);
+    }
+
     /**
      * @param iterable|Visit[] $results
      */
diff --git a/module/Core/src/Visit/VisitLocatorInterface.php b/module/Core/src/Visit/VisitLocatorInterface.php
index 6e06d296..1c99de36 100644
--- a/module/Core/src/Visit/VisitLocatorInterface.php
+++ b/module/Core/src/Visit/VisitLocatorInterface.php
@@ -9,4 +9,6 @@ interface VisitLocatorInterface
     public function locateUnlocatedVisits(VisitGeolocationHelperInterface $helper): void;
 
     public function locateVisitsWithEmptyLocation(VisitGeolocationHelperInterface $helper): void;
+
+    public function locateAllVisits(VisitGeolocationHelperInterface $helper): void;
 }
diff --git a/module/Core/test-db/Repository/VisitRepositoryTest.php b/module/Core/test-db/Repository/VisitRepositoryTest.php
index b72453d6..034b15f9 100644
--- a/module/Core/test-db/Repository/VisitRepositoryTest.php
+++ b/module/Core/test-db/Repository/VisitRepositoryTest.php
@@ -68,11 +68,13 @@ class VisitRepositoryTest extends DatabaseTestCase
 
         $withEmptyLocation = $this->repo->findVisitsWithEmptyLocation($blockSize);
         $unlocated = $this->repo->findUnlocatedVisits($blockSize);
+        $all = $this->repo->findAllVisits($blockSize);
 
-        // Important! assertCount will not work here, as this iterable object loads data dynamically and counts to
-        // 0 if not iterated
+        // Important! assertCount will not work here, as this iterable object loads data dynamically and the count
+        // is 0 if not iterated
         $this->assertEquals(2, $countIterable($unlocated));
         $this->assertEquals(4, $countIterable($withEmptyLocation));
+        $this->assertEquals(6, $countIterable($all));
     }
 
     public function provideBlockSize(): iterable

From 55778eb810f8185bf4ada57b7dcd92e4f2d6bd80 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Sat, 28 Mar 2020 09:27:45 +0100
Subject: [PATCH 63/67] Ensured old visit locations are deleted when relocating
 a visit that has already been located

---
 module/CLI/src/Command/ShortUrl/GetVisitsCommand.php       | 3 ++-
 module/Core/src/Entity/Visit.php                           | 5 ++---
 module/Core/src/Visit/VisitLocator.php                     | 7 +++++++
 .../Core/test/EventDispatcher/LocateShortUrlVisitTest.php  | 3 +--
 4 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/module/CLI/src/Command/ShortUrl/GetVisitsCommand.php b/module/CLI/src/Command/ShortUrl/GetVisitsCommand.php
index 43949993..a0c2c91a 100644
--- a/module/CLI/src/Command/ShortUrl/GetVisitsCommand.php
+++ b/module/CLI/src/Command/ShortUrl/GetVisitsCommand.php
@@ -12,6 +12,7 @@ use Shlinkio\Shlink\Core\Entity\Visit;
 use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier;
 use Shlinkio\Shlink\Core\Model\VisitsParams;
 use Shlinkio\Shlink\Core\Service\VisitsTrackerInterface;
+use Shlinkio\Shlink\Core\Visit\Model\UnknownVisitLocation;
 use Symfony\Component\Console\Input\InputArgument;
 use Symfony\Component\Console\Input\InputInterface;
 use Symfony\Component\Console\Input\InputOption;
@@ -76,7 +77,7 @@ class GetVisitsCommand extends AbstractWithDateRangeCommand
 
         $rows = map($paginator->getCurrentItems(), function (Visit $visit) {
             $rowData = $visit->jsonSerialize();
-            $rowData['country'] = $visit->getVisitLocation()->getCountryName();
+            $rowData['country'] = ($visit->getVisitLocation() ?? new UnknownVisitLocation())->getCountryName();
             return select_keys($rowData, ['referer', 'date', 'userAgent', 'country']);
         });
         ShlinkTable::fromOutput($output)->render(['Referer', 'Date', 'User agent', 'Country'], $rows);
diff --git a/module/Core/src/Entity/Visit.php b/module/Core/src/Entity/Visit.php
index d278ed6a..e8cbb119 100644
--- a/module/Core/src/Entity/Visit.php
+++ b/module/Core/src/Entity/Visit.php
@@ -10,7 +10,6 @@ use Shlinkio\Shlink\Common\Entity\AbstractEntity;
 use Shlinkio\Shlink\Common\Exception\InvalidArgumentException;
 use Shlinkio\Shlink\Common\Util\IpAddress;
 use Shlinkio\Shlink\Core\Model\Visitor;
-use Shlinkio\Shlink\Core\Visit\Model\UnknownVisitLocation;
 use Shlinkio\Shlink\Core\Visit\Model\VisitLocationInterface;
 
 class Visit extends AbstractEntity implements JsonSerializable
@@ -60,9 +59,9 @@ class Visit extends AbstractEntity implements JsonSerializable
         return $this->shortUrl;
     }
 
-    public function getVisitLocation(): VisitLocationInterface
+    public function getVisitLocation(): ?VisitLocationInterface
     {
-        return $this->visitLocation ?? new UnknownVisitLocation();
+        return $this->visitLocation;
     }
 
     public function isLocatable(): bool
diff --git a/module/Core/src/Visit/VisitLocator.php b/module/Core/src/Visit/VisitLocator.php
index f955ef29..46a30559 100644
--- a/module/Core/src/Visit/VisitLocator.php
+++ b/module/Core/src/Visit/VisitLocator.php
@@ -79,9 +79,16 @@ class VisitLocator implements VisitLocatorInterface
 
     private function locateVisit(Visit $visit, VisitLocation $location, VisitGeolocationHelperInterface $helper): void
     {
+        $prevLocation = $visit->getVisitLocation();
+
         $visit->locate($location);
         $this->em->persist($visit);
 
+        // In order to avoid leaving orphan locations, remove the previous one
+        if ($prevLocation !== null) {
+            $this->em->remove($prevLocation);
+        }
+
         $helper->onVisitLocated($location, $visit);
     }
 }
diff --git a/module/Core/test/EventDispatcher/LocateShortUrlVisitTest.php b/module/Core/test/EventDispatcher/LocateShortUrlVisitTest.php
index c8f6f388..087c0e0b 100644
--- a/module/Core/test/EventDispatcher/LocateShortUrlVisitTest.php
+++ b/module/Core/test/EventDispatcher/LocateShortUrlVisitTest.php
@@ -20,7 +20,6 @@ use Shlinkio\Shlink\Core\EventDispatcher\LocateShortUrlVisit;
 use Shlinkio\Shlink\Core\EventDispatcher\ShortUrlVisited;
 use Shlinkio\Shlink\Core\EventDispatcher\VisitLocated;
 use Shlinkio\Shlink\Core\Model\Visitor;
-use Shlinkio\Shlink\Core\Visit\Model\UnknownVisitLocation;
 use Shlinkio\Shlink\IpGeolocation\Exception\WrongIpException;
 use Shlinkio\Shlink\IpGeolocation\Model\Location;
 use Shlinkio\Shlink\IpGeolocation\Resolver\IpLocationResolverInterface;
@@ -218,7 +217,7 @@ class LocateShortUrlVisitTest extends TestCase
 
         ($this->locateVisit)($event);
 
-        $this->assertEquals($visit->getVisitLocation(), new UnknownVisitLocation());
+        $this->assertNull($visit->getVisitLocation());
         $findVisit->shouldHaveBeenCalledOnce();
         $flush->shouldNotHaveBeenCalled();
         $resolveIp->shouldNotHaveBeenCalled();

From c012b4740db3a7feb7f39954a3c2942d6802e725 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Sat, 28 Mar 2020 10:00:46 +0100
Subject: [PATCH 64/67] Updated VisitLocator test so that it covers all public
 methods

---
 module/Core/test/Visit/VisitLocatorTest.php | 59 ++++++++++++++++-----
 1 file changed, 46 insertions(+), 13 deletions(-)

diff --git a/module/Core/test/Visit/VisitLocatorTest.php b/module/Core/test/Visit/VisitLocatorTest.php
index 1972860b..d856262c 100644
--- a/module/Core/test/Visit/VisitLocatorTest.php
+++ b/module/Core/test/Visit/VisitLocatorTest.php
@@ -9,6 +9,7 @@ use Exception;
 use PHPUnit\Framework\Assert;
 use PHPUnit\Framework\TestCase;
 use Prophecy\Argument;
+use Prophecy\Prophecy\MethodProphecy;
 use Prophecy\Prophecy\ObjectProphecy;
 use Shlinkio\Shlink\Core\Entity\ShortUrl;
 use Shlinkio\Shlink\Core\Entity\Visit;
@@ -43,15 +44,20 @@ class VisitLocatorTest extends TestCase
         $this->visitService = new VisitLocator($this->em->reveal());
     }
 
-    /** @test */
-    public function locateVisitsIteratesAndLocatesUnlocatedVisits(): void
-    {
+    /**
+     * @test
+     * @dataProvider provideMethodNames
+     */
+    public function locateVisitsIteratesAndLocatesExpectedVisits(
+        string $serviceMethodName,
+        string $expectedRepoMethodName
+    ): void {
         $unlocatedVisits = map(
             range(1, 200),
             fn (int $i) => new Visit(new ShortUrl(sprintf('short_code_%s', $i)), Visitor::emptyInstance()),
         );
 
-        $findUnlocatedVisits = $this->repo->findUnlocatedVisits()->willReturn($unlocatedVisits);
+        $findVisits = $this->mockRepoMethod($expectedRepoMethodName)->willReturn($unlocatedVisits);
 
         $persist = $this->em->persist(Argument::type(Visit::class))->will(function (): void {
         });
@@ -60,7 +66,7 @@ class VisitLocatorTest extends TestCase
         $clear = $this->em->clear()->will(function (): void {
         });
 
-        $this->visitService->locateUnlocatedVisits(new class implements VisitGeolocationHelperInterface {
+        $this->visitService->{$serviceMethodName}(new class implements VisitGeolocationHelperInterface {
             public function geolocateVisit(Visit $visit): Location
             {
                 return Location::emptyInstance();
@@ -75,23 +81,33 @@ class VisitLocatorTest extends TestCase
             }
         });
 
-        $findUnlocatedVisits->shouldHaveBeenCalledOnce();
+        $findVisits->shouldHaveBeenCalledOnce();
         $persist->shouldHaveBeenCalledTimes(count($unlocatedVisits));
         $flush->shouldHaveBeenCalledTimes(floor(count($unlocatedVisits) / 200) + 1);
         $clear->shouldHaveBeenCalledTimes(floor(count($unlocatedVisits) / 200) + 1);
     }
 
+    public function provideMethodNames(): iterable
+    {
+        yield 'locateUnlocatedVisits' => ['locateUnlocatedVisits', 'findUnlocatedVisits'];
+        yield 'locateVisitsWithEmptyLocation' => ['locateVisitsWithEmptyLocation', 'findVisitsWithEmptyLocation'];
+        yield 'locateAllVisits' => ['locateAllVisits', 'findAllVisits'];
+    }
+
     /**
      * @test
      * @dataProvider provideIsNonLocatableAddress
      */
-    public function visitsWhichCannotBeLocatedAreIgnoredOrLocatedAsEmpty(bool $isNonLocatableAddress): void
-    {
+    public function visitsWhichCannotBeLocatedAreIgnoredOrLocatedAsEmpty(
+        string $serviceMethodName,
+        string $expectedRepoMethodName,
+        bool $isNonLocatableAddress
+    ): void {
         $unlocatedVisits = [
             new Visit(new ShortUrl('foo'), Visitor::emptyInstance()),
         ];
 
-        $findUnlocatedVisits = $this->repo->findUnlocatedVisits()->willReturn($unlocatedVisits);
+        $findVisits = $this->mockRepoMethod($expectedRepoMethodName)->willReturn($unlocatedVisits);
 
         $persist = $this->em->persist(Argument::type(Visit::class))->will(function (): void {
         });
@@ -100,7 +116,7 @@ class VisitLocatorTest extends TestCase
         $clear = $this->em->clear()->will(function (): void {
         });
 
-        $this->visitService->locateUnlocatedVisits(
+        $this->visitService->{$serviceMethodName}(
             new class ($isNonLocatableAddress) implements VisitGeolocationHelperInterface {
                 private bool $isNonLocatableAddress;
 
@@ -122,7 +138,7 @@ class VisitLocatorTest extends TestCase
             },
         );
 
-        $findUnlocatedVisits->shouldHaveBeenCalledOnce();
+        $findVisits->shouldHaveBeenCalledOnce();
         $persist->shouldHaveBeenCalledTimes($isNonLocatableAddress ? 1 : 0);
         $flush->shouldHaveBeenCalledOnce();
         $clear->shouldHaveBeenCalledOnce();
@@ -130,7 +146,24 @@ class VisitLocatorTest extends TestCase
 
     public function provideIsNonLocatableAddress(): iterable
     {
-        yield 'The address is locatable' => [false];
-        yield 'The address is non-locatable' => [true];
+        yield 'locateUnlocatedVisits - locatable address' => ['locateUnlocatedVisits', 'findUnlocatedVisits', false];
+        yield 'locateUnlocatedVisits - non-locatable address' => ['locateUnlocatedVisits', 'findUnlocatedVisits', true];
+        yield 'locateVisitsWithEmptyLocation - locatable address' => [
+            'locateVisitsWithEmptyLocation',
+            'findVisitsWithEmptyLocation',
+            false,
+        ];
+        yield 'locateVisitsWithEmptyLocation - non-locatable address' => [
+            'locateVisitsWithEmptyLocation',
+            'findVisitsWithEmptyLocation',
+            true,
+        ];
+        yield 'locateAllVisits - locatable address' => ['locateAllVisits', 'findAllVisits', false];
+        yield 'locateAllVisits - non-locatable address' => ['locateAllVisits', 'findAllVisits', true];
+    }
+
+    private function mockRepoMethod(string $methodName): MethodProphecy
+    {
+        return (new MethodProphecy($this->repo, $methodName, new Argument\ArgumentsWildcard([])));
     }
 }

From 4d39c7041b1167d190d187bb3e620968f433568d Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Sat, 28 Mar 2020 10:23:34 +0100
Subject: [PATCH 65/67] Improved LocateVisitsCommandtest so that it covers all
 possible workflows

---
 .../Command/Visit/LocateVisitsCommandTest.php | 76 +++++++++++++++++--
 1 file changed, 68 insertions(+), 8 deletions(-)

diff --git a/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php b/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php
index 30be9846..803ae472 100644
--- a/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php
+++ b/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php
@@ -21,12 +21,15 @@ use Shlinkio\Shlink\IpGeolocation\Exception\WrongIpException;
 use Shlinkio\Shlink\IpGeolocation\Model\Location;
 use Shlinkio\Shlink\IpGeolocation\Resolver\IpLocationResolverInterface;
 use Symfony\Component\Console\Application;
+use Symfony\Component\Console\Exception\RuntimeException;
 use Symfony\Component\Console\Output\OutputInterface;
 use Symfony\Component\Console\Tester\CommandTester;
 use Symfony\Component\Lock;
 
 use function sprintf;
 
+use const PHP_EOL;
+
 class LocateVisitsCommandTest extends TestCase
 {
     private CommandTester $commandTester;
@@ -61,25 +64,53 @@ class LocateVisitsCommandTest extends TestCase
         $this->commandTester = new CommandTester($command);
     }
 
-    /** @test */
-    public function allPendingVisitsAreProcessed(): void
-    {
+    /**
+     * @test
+     * @dataProvider provideArgs
+     */
+    public function expectedSetOfVisitsIsProcessedBasedOnArgs(
+        int $expectedUnlocatedCalls,
+        int $expectedEmptyCalls,
+        int $expectedAllCalls,
+        bool $expectWarningPrint,
+        array $args
+    ): void {
         $visit = new Visit(new ShortUrl(''), new Visitor('', '', '1.2.3.4'));
         $location = new VisitLocation(Location::emptyInstance());
+        $mockMethodBehavior = $this->invokeHelperMethods($visit, $location);
 
-        $locateVisits = $this->visitService->locateUnlocatedVisits(Argument::cetera())->will(
-            $this->invokeHelperMethods($visit, $location),
+        $locateVisits = $this->visitService->locateUnlocatedVisits(Argument::cetera())->will($mockMethodBehavior);
+        $locateEmptyVisits = $this->visitService->locateVisitsWithEmptyLocation(Argument::cetera())->will(
+            $mockMethodBehavior,
         );
+        $locateAllVisits = $this->visitService->locateAllVisits(Argument::cetera())->will($mockMethodBehavior);
         $resolveIpLocation = $this->ipResolver->resolveIpLocation(Argument::any())->willReturn(
             Location::emptyInstance(),
         );
 
-        $this->commandTester->execute([]);
+        $this->commandTester->setInputs(['y']);
+        $this->commandTester->execute($args);
         $output = $this->commandTester->getDisplay();
 
         $this->assertStringContainsString('Processing IP 1.2.3.0', $output);
-        $locateVisits->shouldHaveBeenCalledOnce();
-        $resolveIpLocation->shouldHaveBeenCalledOnce();
+        if ($expectWarningPrint) {
+            $this->assertStringContainsString('Continue at your own risk', $output);
+        } else {
+            $this->assertStringNotContainsString('Continue at your own risk', $output);
+        }
+        $locateVisits->shouldHaveBeenCalledTimes($expectedUnlocatedCalls);
+        $locateEmptyVisits->shouldHaveBeenCalledTimes($expectedEmptyCalls);
+        $locateAllVisits->shouldHaveBeenCalledTimes($expectedAllCalls);
+        $resolveIpLocation->shouldHaveBeenCalledTimes(
+            $expectedUnlocatedCalls + $expectedEmptyCalls + $expectedAllCalls,
+        );
+    }
+
+    public function provideArgs(): iterable
+    {
+        yield 'no args' => [1, 0, 0, false, []];
+        yield 'retry' => [1, 1, 0, false, ['--retry' => true]];
+        yield 'all' => [0, 0, 1, true, ['--retry' => true, '--all' => true]];
     }
 
     /**
@@ -205,4 +236,33 @@ class LocateVisitsCommandTest extends TestCase
         yield [true, '[Warning] GeoLite2 database update failed. Proceeding with old version.'];
         yield [false, 'GeoLite2 database download failed. It is not possible to locate visits.'];
     }
+
+    /** @test */
+    public function providingAllFlagOnItsOwnDisplaysNotice(): void
+    {
+        $this->commandTester->execute(['--all' => true]);
+        $output = $this->commandTester->getDisplay();
+
+        $this->assertStringContainsString('The --all flag has no effect on its own', $output);
+    }
+
+    /**
+     * @test
+     * @dataProvider provideAbortInputs
+     */
+    public function processingAllCancelsCommandIfUserDoesNotActivelyAgreeToConfirmation(array $inputs): void
+    {
+        $this->expectException(RuntimeException::class);
+        $this->expectExceptionMessage('Execution aborted');
+
+        $this->commandTester->setInputs($inputs);
+        $this->commandTester->execute(['--all' => true, '--retry' => true]);
+    }
+
+    public function provideAbortInputs(): iterable
+    {
+        yield 'n' => [['n']];
+        yield 'no' => [['no']];
+        yield 'default' => [[PHP_EOL]];
+    }
 }

From 2a30afbe7da43e7a086dced0e1954194f7823e56 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Sat, 28 Mar 2020 10:29:12 +0100
Subject: [PATCH 66/67] Updated changelog

---
 CHANGELOG.md     | 6 +++++-
 docker/README.md | 4 ++--
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index be5c993d..718a094e 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,7 +4,7 @@ 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).
 
-## [Unreleased]
+## 2.1.0 - 2020-03-28
 
 #### Added
 
@@ -17,6 +17,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this
 #### Changed
 
 * [#656](https://github.com/shlinkio/shlink/issues/656) Updated to PHPUnit 9.
+* [#641](https://github.com/shlinkio/shlink/issues/641) Added two new flags to the `visit:locate` command, `--retry` and `--all`.
+
+    * When `--retry` is provided, it will try to re-locate visits which IP address was originally considered not found, in case it was a temporal issue.
+    * When `--all` is provided together with `--retry`, it will try to re-locate all existing visits. A warning and confirmation are displayed, as this can have side effects.
 
 #### Deprecated
 
diff --git a/docker/README.md b/docker/README.md
index f8e596ce..3977fa37 100644
--- a/docker/README.md
+++ b/docker/README.md
@@ -37,10 +37,10 @@ Or you can list all tags with:
 docker exec -it shlink_container shlink tag:list
 ```
 
-Or process remaining visits with:
+Or locate remaining visits with:
 
 ```bash
-docker exec -it shlink_container shlink visit:process
+docker exec -it shlink_container shlink visit:locate
 ```
 
 All shlink commands will work the same way.

From 53ba58d7e9478366f2c21e1e2efa17f9084d6f8d Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Sat, 28 Mar 2020 10:37:41 +0100
Subject: [PATCH 67/67] Moved initialization of the io object in
 LocateVisitsCommand to the initialize method

---
 module/CLI/src/Command/Visit/LocateVisitsCommand.php | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/module/CLI/src/Command/Visit/LocateVisitsCommand.php b/module/CLI/src/Command/Visit/LocateVisitsCommand.php
index 1a7d0bd0..bf1ac14b 100644
--- a/module/CLI/src/Command/Visit/LocateVisitsCommand.php
+++ b/module/CLI/src/Command/Visit/LocateVisitsCommand.php
@@ -73,9 +73,13 @@ class LocateVisitsCommand extends AbstractLockedCommand implements VisitGeolocat
             );
     }
 
-    protected function interact(InputInterface $input, OutputInterface $output): void
+    protected function initialize(InputInterface $input, OutputInterface $output): void
     {
         $this->io = new SymfonyStyle($input, $output);
+    }
+
+    protected function interact(InputInterface $input, OutputInterface $output): void
+    {
         $retry = $input->getOption('retry');
         $all = $input->getOption('all');