Merge pull request #787 from shlinkio/develop

Release v2.2.2

.travis.yml

@@ -1,11 +1,25 @@
+dist: bionic
+
 language: php
 
 branches:
   only:
     - /.*/
 
-php:
+jobs:
-  - '7.4'
+  fast_finish: true
+  include:
+    - name: "Docker publish"
+      php: '7.4'
+      if: NOT type = pull_request
+      env:
+        - DOCKER_PUBLISH="true"
+    - name: "CI"
+      php: '7.4'
+      env:
+        - DOCKER_PUBLISH="false"
+  allow_failures:
+    - name: "Docker publish"
 
 services:
   - docker
@@ -15,36 +29,38 @@ cache:
     - $HOME/.composer/cache/files
 
 before_install:
-  - sudo ./data/infra/ci/install-ms-odbc.sh
-  - docker-compose -f docker-compose.yml -f docker-compose.ci.yml up -d shlink_db_ms shlink_db shlink_db_postgres shlink_db_maria
-  - yes | pecl install pdo_sqlsrv swoole-4.4.18
   - echo 'extension = apcu.so' >> ~/.phpenv/versions/$(phpenv version-name)/etc/php.ini
   - phpenv config-rm xdebug.ini || return 0
+  - if [[ "${DOCKER_PUBLISH}" == 'false' ]]; then sudo ./data/infra/ci/install-ms-odbc.sh ; fi
+  - if [[ "${DOCKER_PUBLISH}" == 'false' ]]; then docker-compose -f docker-compose.yml -f docker-compose.ci.yml up -d shlink_db_ms shlink_db shlink_db_postgres shlink_db_maria ; fi
+  - if [[ "${DOCKER_PUBLISH}" == 'false' ]]; then yes | pecl install pdo_sqlsrv swoole-4.4.18 ; fi
 
 install:
-  - composer self-update
+  - if [[ "${DOCKER_PUBLISH}" == 'true' ]]; then sudo ./data/infra/ci/install-docker.sh ; fi
-  - composer install --no-interaction --prefer-dist
+  - if [[ "${DOCKER_PUBLISH}" == 'false' ]]; then composer self-update ; fi
+  - if [[ "${DOCKER_PUBLISH}" == 'false' ]]; then composer install --no-interaction --prefer-dist ; fi
 
 before_script:
-  - docker-compose exec shlink_db_ms /opt/mssql-tools/bin/sqlcmd -S localhost -U sa -P 'Passw0rd!' -Q "CREATE DATABASE shlink_test;"
+  - if [[ "${DOCKER_PUBLISH}" == 'false' ]]; then docker-compose exec shlink_db_ms /opt/mssql-tools/bin/sqlcmd -S localhost -U sa -P 'Passw0rd!' -Q "CREATE DATABASE shlink_test;" ; fi
   - mkdir build
   - export DOCKERFILE_CHANGED=$(git diff ${TRAVIS_COMMIT_RANGE:-origin/master} --name-only | grep Dockerfile)
 
 script:
-  - composer ci
+  - if [[ "${DOCKER_PUBLISH}" == 'false' ]]; then bin/test/run-api-tests.sh --coverage-php build/coverage-api.cov && composer ci ; fi
-  - if [[ ! -z "$DOCKERFILE_CHANGED" && "${TRAVIS_PHP_VERSION}" == "7.4" ]]; then docker build -t shlink-docker-image:temp . ; fi
+  - if [[ ! -z "${DOCKERFILE_CHANGED}" && "${TRAVIS_PHP_VERSION}" == "7.4" && "${DOCKER_PUBLISH}" == "false" ]]; then docker build -t shlink-docker-image:temp . ; fi
+  - if [[ "${DOCKER_PUBLISH}" == 'true' ]]; then bash ./docker/build ; fi
 
 after_success:
   - rm -f build/clover.xml
-  - wget https://phar.phpunit.de/phpcov-7.0.2.phar
+  - if [[ "${DOCKER_PUBLISH}" == 'false' ]]; then wget https://phar.phpunit.de/phpcov-7.0.2.phar ; fi
-  - phpdbg -qrr phpcov-7.0.2.phar merge build --clover build/clover.xml
+  - if [[ "${DOCKER_PUBLISH}" == 'false' ]]; then phpdbg -qrr phpcov-7.0.2.phar merge build --clover build/clover.xml ; fi
-  - wget https://scrutinizer-ci.com/ocular.phar
+  - if [[ "${DOCKER_PUBLISH}" == 'false' ]]; then wget https://scrutinizer-ci.com/ocular.phar ; fi
-  - php ocular.phar code-coverage:upload --format=php-clover build/clover.xml
+  - if [[ "${DOCKER_PUBLISH}" == 'false' ]]; then php ocular.phar code-coverage:upload --format=php-clover build/clover.xml ; fi
 
 # Before deploying, build dist file for current travis tag
 before_deploy:
   - rm -f ocular.phar
-  - if [[ ! -z $TRAVIS_TAG && "${TRAVIS_PHP_VERSION}" == "7.4" ]]; then ./build.sh ${TRAVIS_TAG#?} ; fi
+  - if [[ ! -z ${TRAVIS_TAG} && "${TRAVIS_PHP_VERSION}" == "7.4" ]]; then ./build.sh ${TRAVIS_TAG#?} ; fi
 
 deploy:
   - provider: releases
@@ -53,11 +69,7 @@ deploy:
     file: "./build/shlink_${TRAVIS_TAG#?}_dist.zip"
     skip_cleanup: true
     on:
+      all_branches: true
+      condition: ${DOCKER_PUBLISH} == 'false'
       tags: true
       php: '7.4'
-  - provider: script
-    script: bash ./docker/build
-    on:
-      all_branches: true
-      condition: $TRAVIS_PULL_REQUEST == 'false'
-      php: '7.4'

CHANGELOG.md

@@ -4,6 +4,53 @@ 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).
 
+## 2.2.2 - 2020-06-08
+
+#### Added
+
+* [#709](https://github.com/shlinkio/shlink/issues/709) Added multi-architecture builds for the docker image.
+
+#### Changed
+
+* *Nothing*
+
+#### Deprecated
+
+* *Nothing*
+
+#### Removed
+
+* *Nothing*
+
+#### Fixed
+
+* [#769](https://github.com/shlinkio/shlink/issues/769) Fixed custom slugs not allowing valid URL characters, like `.`, `_` or `~`.
+* [#781](https://github.com/shlinkio/shlink/issues/781) Fixed memory leak when loading visits for a tag which is used for big amounts of short URLs.
+
+
+## 2.2.1 - 2020-05-11
+
+#### Added
+
+* *Nothing*
+
+#### Changed
+
+* *Nothing*
+
+#### Deprecated
+
+* *Nothing*
+
+#### Removed
+
+* *Nothing*
+
+#### Fixed
+
+* [#764](https://github.com/shlinkio/shlink/issues/764) Fixed error when trying to match an existing short URL which does not have `validSince` and/or `validUntil`, but you are providing either one of them for the new one.
+
+
 ## 2.2.0 - 2020-05-09
 
 #### Added

Dockerfile

@@ -23,14 +23,22 @@ RUN \
     apk add --no-cache libzip-dev zlib-dev libpng-dev && \
     docker-php-ext-install -j"$(nproc)" zip gd
 
-# Install swoole and sqlsrv driver
+# 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 && \
+RUN if [ $(uname -m) == "x86_64" ]; then \
-    apk add --allow-untrusted msodbcsql17_17.5.1.1-1_amd64.apk && \
+      wget https://download.microsoft.com/download/e/4/e/e4e67866-dffd-428c-aac7-8d28ddafb39b/msodbcsql17_17.5.1.1-1_amd64.apk && \
-    apk add --no-cache --virtual .phpize-deps $PHPIZE_DEPS unixodbc-dev && \
+      apk add --allow-untrusted msodbcsql17_17.5.1.1-1_amd64.apk && \
-    pecl install swoole-${SWOOLE_VERSION} pdo_sqlsrv && \
+      apk add --no-cache --virtual .phpize-deps $PHPIZE_DEPS unixodbc-dev && \
-    docker-php-ext-enable swoole pdo_sqlsrv && \
+      pecl install pdo_sqlsrv && \
-    apk del .phpize-deps && \
+      docker-php-ext-enable pdo_sqlsrv && \
-    rm msodbcsql17_17.5.1.1-1_amd64.apk
+      apk del .phpize-deps && \
+      rm msodbcsql17_17.5.1.1-1_amd64.apk ; \
+    fi
+
+# Install swoole
+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 shlink

composer.json

@@ -17,6 +17,7 @@
         "ext-pdo": "*",
         "akrabat/ip-address-middleware": "^1.0",
         "cakephp/chronos": "^1.2",
+        "cocur/slugify": "^4.0",
         "doctrine/cache": "^1.9",
         "doctrine/dbal": "^2.10",
         "doctrine/migrations": "^2.2",
@@ -53,11 +54,13 @@
         "shlinkio/shlink-event-dispatcher": "^1.4",
         "shlinkio/shlink-installer": "^5.0.0",
         "shlinkio/shlink-ip-geolocation": "^1.4",
-        "symfony/console": "^5.0",
+        "symfony/console": "^5.1",
-        "symfony/filesystem": "^5.0",
+        "symfony/filesystem": "^5.1",
-        "symfony/lock": "^5.0",
+        "symfony/lock": "^5.1",
         "symfony/mercure": "^0.3.0",
-        "symfony/process": "^5.0"
+        "symfony/process": "^5.1",
+        "symfony/string": "^5.1",
+        "symfony/translation-contracts": "^2.1"
     },
     "require-dev": {
         "devster/ubench": "^2.0",
@@ -109,8 +112,7 @@
         ],
         "test:ci": [
             "@test:unit:ci",
-            "@test:db",
+            "@test:db"
-            "@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",

data/infra/ci/install-docker.sh

@@ -0,0 +1,12 @@
+#!/usr/bin/env bash
+
+set -ex
+
+# install latest docker version
+curl -fsSL https://download.docker.com/linux/ubuntu/gpg | sudo apt-key add -
+add-apt-repository "deb [arch=amd64] https://download.docker.com/linux/ubuntu $(lsb_release -cs) stable"
+apt-get update
+apt-get -y -o Dpkg::Options::="--force-confnew" install docker-ce
+
+# enable multiarch execution
+docker run --rm --privileged multiarch/qemu-user-static --reset -p yes

docker/README.md

@@ -263,7 +263,13 @@ Once created just run shlink with the volume:
 docker run --name shlink -p 8080:8080 -v ${PWD}/my/config/dir:/etc/shlink/config/params shlinkio/shlink:stable
 ```
 
-## Multi instance considerations
+## Multi-architecture
+
+Starting on v2.3.0, Shlink's docker image is built for multiple architectures.
+
+The only limitation is that images for architectures other than `amd64` will not have support for Microsoft SQL databases, since there are no official binaries.
+
+## Multi-instance considerations
 
 These are some considerations to take into account when running multiple instances of shlink.
 

docker/build

@@ -1,17 +1,35 @@
 #!/bin/bash
+
 set -e
 
+# PLATFORMS="linux/arm/v7,linux/arm64/v8,linux/amd64"
+PLATFORMS="linux/amd64"
+DOCKER_IMAGE="shlinkio/shlink"
+BUILDX_VER=v0.4.1
+export DOCKER_CLI_EXPERIMENTAL=enabled
+
+mkdir -vp ~/.docker/cli-plugins/ ~/dockercache
+curl --silent -L "https://github.com/docker/buildx/releases/download/${BUILDX_VER}/buildx-${BUILDX_VER}.linux-amd64" > ~/.docker/cli-plugins/docker-buildx
+chmod a+x ~/.docker/cli-plugins/docker-buildx
+
+docker buildx create --use
+
 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 SHLINK_VERSION=${TRAVIS_TAG#?} -t shlinkio/shlink:${TRAVIS_TAG#?} -t shlinkio/shlink:stable .
+  TAGS="-t ${DOCKER_IMAGE}:${TRAVIS_TAG#?}"
-  docker push shlinkio/shlink:${TRAVIS_TAG#?}
-
   # Push stable tag only if this is not an alpha or beta tag
-  [[ $TRAVIS_TAG != *"alpha"* && $TRAVIS_TAG != *"beta"* ]] && docker push shlinkio/shlink:stable
+  [[ $TRAVIS_TAG != *"alpha"* && $TRAVIS_TAG != *"beta"* ]] && TAGS="${TAGS} -t ${DOCKER_IMAGE}:stable"
+
+  docker buildx build --push \
+    --build-arg SHLINK_VERSION=${TRAVIS_TAG#?} \
+    --platform ${PLATFORMS} \
+    ${TAGS} .
+
 # 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 buildx build --push \
-  docker push shlinkio/shlink:latest
+    --platform ${PLATFORMS} \
+    -t ${DOCKER_IMAGE}:latest .
 fi

module/Core/src/Entity/ShortUrl.php

@@ -204,10 +204,10 @@ class ShortUrl extends AbstractEntity
         if ($meta->hasDomain() && $meta->getDomain() !== $this->resolveDomain()) {
             return false;
         }
-        if ($meta->hasValidSince() && ! $meta->getValidSince()->eq($this->validSince)) {
+        if ($meta->hasValidSince() && ($this->validSince === null || ! $meta->getValidSince()->eq($this->validSince))) {
             return false;
         }
-        if ($meta->hasValidUntil() && ! $meta->getValidUntil()->eq($this->validUntil)) {
+        if ($meta->hasValidUntil() && ($this->validUntil === null || ! $meta->getValidUntil()->eq($this->validUntil))) {
             return false;
         }
 

module/Core/src/Repository/VisitRepository.php

@@ -12,8 +12,6 @@ use Shlinkio\Shlink\Core\Entity\ShortUrl;
 use Shlinkio\Shlink\Core\Entity\Visit;
 use Shlinkio\Shlink\Core\Entity\VisitLocation;
 
-use function array_column;
-
 use const PHP_INT_MAX;
 
 class VisitRepository extends EntityRepository implements VisitRepositoryInterface
@@ -142,26 +140,18 @@ class VisitRepository extends EntityRepository implements VisitRepositoryInterfa
 
     private function createVisitsByTagQueryBuilder(string $tag, ?DateRange $dateRange = null): QueryBuilder
     {
-        $qb = $this->getEntityManager()->createQueryBuilder();
-        $qb->select('s.id')
-           ->from(ShortUrl::class, 's')
-           ->join('s.tags', 't')
-           ->where($qb->expr()->eq('t.name', ':tag'))
-           ->setParameter('tag', $tag);
-
-        $shortUrlIds = array_column($qb->getQuery()->getArrayResult(), 'id');
-        $shortUrlIds[] = '-1'; // Add an invalid ID, in case the list is empty
-
         // Parameters in this query need to be part of the query itself, as we need to use it a sub-query later
         // Since they are not strictly provided by the caller, it's reasonably safe
-        $qb2 = $this->getEntityManager()->createQueryBuilder();
+        $qb = $this->getEntityManager()->createQueryBuilder();
-        $qb2->from(Visit::class, 'v')
+        $qb->from(Visit::class, 'v')
-            ->where($qb2->expr()->in('v.shortUrl', $shortUrlIds));
+           ->join('v.shortUrl', 's')
+           ->join('s.tags', 't')
+           ->where($qb->expr()->eq('t.name', '\'' . $tag . '\''));
 
         // Apply date range filtering
-        $this->applyDatesInline($qb2, $dateRange);
+        $this->applyDatesInline($qb, $dateRange);
 
-        return $qb2;
+        return $qb;
     }
 
     private function applyDatesInline(QueryBuilder $qb, ?DateRange $dateRange): void

module/Core/src/Util/CocurSymfonySluggerBridge.php

@@ -0,0 +1,26 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Shlinkio\Shlink\Core\Util;
+
+use Cocur\Slugify\SlugifyInterface;
+use Symfony\Component\String\AbstractUnicodeString;
+use Symfony\Component\String\Slugger\SluggerInterface;
+
+use function Symfony\Component\String\s;
+
+class CocurSymfonySluggerBridge implements SluggerInterface
+{
+    private SlugifyInterface $slugger;
+
+    public function __construct(SlugifyInterface $slugger)
+    {
+        $this->slugger = $slugger;
+    }
+
+    public function slug(string $string, string $separator = '-', ?string $locale = null): AbstractUnicodeString
+    {
+        return s($this->slugger->slugify($string, $separator));
+    }
+}

module/Core/src/Validation/ShortUrlMetaInputFilter.php

@@ -4,11 +4,13 @@ declare(strict_types=1);
 
 namespace Shlinkio\Shlink\Core\Validation;
 
+use Cocur\Slugify\Slugify;
 use DateTime;
 use Laminas\InputFilter\Input;
 use Laminas\InputFilter\InputFilter;
 use Laminas\Validator;
 use Shlinkio\Shlink\Common\Validation;
+use Shlinkio\Shlink\Core\Util\CocurSymfonySluggerBridge;
 
 use const Shlinkio\Shlink\Core\MIN_SHORT_CODES_LENGTH;
 
@@ -46,7 +48,10 @@ class ShortUrlMetaInputFilter extends InputFilter
         // 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->getFilterChain()->attach(new Validation\SluggerFilter(new CocurSymfonySluggerBridge(new Slugify([
+            'regexp' => '/[^A-Za-z0-9._~]+/',
+            'lowercase' => false,
+        ]))));
         $customSlug->getValidatorChain()->attach(new Validator\NotEmpty([
             Validator\NotEmpty::STRING,
             Validator\NotEmpty::SPACE,

module/Core/test/Entity/ShortUrlTest.php

@@ -4,6 +4,7 @@ declare(strict_types=1);
 
 namespace ShlinkioTest\Shlink\Core\Entity;
 
+use Cake\Chronos\Chronos;
 use PHPUnit\Framework\TestCase;
 use Shlinkio\Shlink\Core\Entity\ShortUrl;
 use Shlinkio\Shlink\Core\Exception\ShortCodeCannotBeRegeneratedException;
@@ -74,4 +75,38 @@ class ShortUrlTest extends TestCase
         yield [null, DEFAULT_SHORT_CODES_LENGTH];
         yield from map(range(4, 10), fn (int $value) => [$value, $value]);
     }
+
+    /**
+     * @test
+     * @dataProvider provideCriteriaToMatch
+     */
+    public function criteriaIsMatchedWhenDatesMatch(ShortUrl $shortUrl, ShortUrlMeta $meta, bool $expected): void
+    {
+        $this->assertEquals($expected, $shortUrl->matchesCriteria($meta, []));
+    }
+
+    public function provideCriteriaToMatch(): iterable
+    {
+        $start = Chronos::parse('2020-03-05 20:18:30');
+        $end = Chronos::parse('2021-03-05 20:18:30');
+
+        yield [new ShortUrl('foo'), ShortUrlMeta::fromRawData(['validSince' => $start]), false];
+        yield [new ShortUrl('foo'), ShortUrlMeta::fromRawData(['validUntil' => $end]), false];
+        yield [new ShortUrl('foo'), ShortUrlMeta::fromRawData(['validSince' => $start, 'validUntil' => $end]), false];
+        yield [
+            new ShortUrl('foo', ShortUrlMeta::fromRawData(['validSince' => $start])),
+            ShortUrlMeta::fromRawData(['validSince' => $start]),
+            true,
+        ];
+        yield [
+            new ShortUrl('foo', ShortUrlMeta::fromRawData(['validUntil' => $end])),
+            ShortUrlMeta::fromRawData(['validUntil' => $end]),
+            true,
+        ];
+        yield [
+            new ShortUrl('foo', ShortUrlMeta::fromRawData(['validUntil' => $end, 'validSince' => $start])),
+            ShortUrlMeta::fromRawData(['validUntil' => $end, 'validSince' => $start]),
+            true,
+        ];
+    }
 }

module/Core/test/Model/ShortUrlMetaTest.php

@@ -58,11 +58,14 @@ class ShortUrlMetaTest extends TestCase
         ]];
     }
 
-    /** @test */
+    /**
-    public function properlyCreatedInstanceReturnsValues(): void
+     * @test
+     * @dataProvider provideCustomSlugs
+     */
+    public function properlyCreatedInstanceReturnsValues(string $customSlug, string $expectedSlug): void
     {
         $meta = ShortUrlMeta::fromRawData(
-            ['validSince' => Chronos::parse('2015-01-01')->toAtomString(), 'customSlug' => 'foobar'],
+            ['validSince' => Chronos::parse('2015-01-01')->toAtomString(), 'customSlug' => $customSlug],
         );
 
         $this->assertTrue($meta->hasValidSince());
@@ -72,9 +75,18 @@ class ShortUrlMetaTest extends TestCase
         $this->assertNull($meta->getValidUntil());
 
         $this->assertTrue($meta->hasCustomSlug());
-        $this->assertEquals('foobar', $meta->getCustomSlug());
+        $this->assertEquals($expectedSlug, $meta->getCustomSlug());
 
         $this->assertFalse($meta->hasMaxVisits());
         $this->assertNull($meta->getMaxVisits());
     }
+
+    public function provideCustomSlugs(): iterable
+    {
+        yield ['foobar', 'foobar'];
+        yield ['foo bar', 'foo-bar'];
+        yield ['wp-admin.php', 'wp-admin.php'];
+        yield ['UPPER_lower', 'UPPER_lower'];
+        yield ['more~url_special.chars', 'more~url_special.chars'];
+    }
 }