Merge pull request #1691 from shlinkio/develop

Release 3.5.1

.github/ISSUE_TEMPLATE.md

@@ -1,7 +1,7 @@
 <!--
 Before opening an issue, just take into account that this is a completely free of charge and open source project.
 I'm always happy to help and provide support, but some understanding will be expected.
-I do this in my own free time, so expect some delays when implementing new features and fixing bugs, and don't take it personal if an issue gets eventually closed.
+I do this in my own free time, so expect some delays when implementing new features and fixing bugs, and don't take it personally if an issue gets eventually closed.
 You may also be asked to provide tests or ways to reproduce reported bugs.
 Try to be polite, and understand it is impossible for an OSS project to cover all use cases.
 -->

.github/ISSUE_TEMPLATE/Bug.md

@@ -7,18 +7,18 @@ labels: bug
 <!--
 Before opening an issue, just take into account that this is a completely free of charge and open source project.
 I'm always happy to help and provide support, but some understanding will be expected.
-I do this in my own free time, so expect some delays when implementing new features and fixing bugs, and don't take it personal if an issue gets eventually closed.
+I do this in my own free time, so expect some delays when implementing new features and fixing bugs, and don't take it personally if an issue gets eventually closed.
 You may also be asked to provide tests or ways to reproduce reported bugs.
 Try to be polite, and understand it is impossible for an OSS project to cover all use cases.
 
 With that said, please fill in the information requested next. More information might be requested next (like logs or system configs).
 -->
 
-#### How Shlink is set-up
+#### How Shlink is set up
 
 * Shlink Version: x.y.z
 * PHP Version: x.y.z
-* How do you serve Shlink: Self-hosted Apache|Self-hosted nginx|Self-hosted openswoole|Docker image
+* How do you serve Shlink: Self-hosted Apache|Self-hosted nginx|Self-hosted openswoole|Self-hosted RoadRunner|Openswoole Docker image|RoadRunner Docker image
 * Database engine used: MySQL|MariaDB|PostgreSQL|MicrosoftSQL|SQLite (x.y.z)
 
 #### Summary
@@ -31,7 +31,7 @@ With that said, please fill in the information requested next. More information
 
 #### Expected behavior
 
-<!-- How did you expected to behave? -->
+<!-- How did you expect it to behave? -->
 
 #### How to reproduce
 

.github/ISSUE_TEMPLATE/Feature_Request.md

@@ -7,7 +7,7 @@ labels: feature
 <!--
 Before opening an issue, just take into account that this is a completely free of charge and open source project.
 I'm always happy to help and provide support, but some understanding will be expected.
-I do this in my own free time, so expect some delays when implementing new features and fixing bugs, and don't take it personal if an issue gets eventually closed.
+I do this in my own free time, so expect some delays when implementing new features and fixing bugs, and don't take it personally if an issue gets eventually closed.
 You may also be asked to provide tests or ways to reproduce reported bugs.
 Try to be polite, and understand it is impossible for an OSS project to cover all use cases.
 

.github/ISSUE_TEMPLATE/Question_Support.md

@@ -7,18 +7,18 @@ labels: question
 <!--
 Before opening an issue, just take into account that this is a completely free of charge and open source project.
 I'm always happy to help and provide support, but some understanding will be expected.
-I do this in my own free time, so expect some delays when implementing new features and fixing bugs, and don't take it personal if an issue gets eventually closed.
+I do this in my own free time, so expect some delays when implementing new features and fixing bugs, and don't take it personally if an issue gets eventually closed.
 You may also be asked to provide tests or ways to reproduce reported bugs.
 Try to be polite, and understand it is impossible for an OSS project to cover all use cases.
 
 With that said, please fill in the information requested next. More information might be requested next (like logs or system configs).
 -->
 
-#### How Shlink is set-up
+#### How Shlink is set up
 
 * Shlink Version: x.y.z
 * PHP Version: x.y.z
-* How do you serve Shlink: Self-hosted Apache|Self-hosted nginx|Self-hosted openswoole|Docker image
+* How do you serve Shlink: Self-hosted Apache|Self-hosted nginx|Self-hosted openswoole|Self-hosted RoadRunner|Openswoole Docker image|RoadRunner Docker image
 * Database engine used: MySQL|MariaDB|PostgreSQL|MicrosoftSQL|SQLite (x.y.z)
 
 #### Summary

CHANGELOG.md

@@ -4,6 +4,24 @@ 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).
 
+## [3.5.1] - 2023-02-04
+### Added
+* *Nothing*
+
+### Changed
+* [#1685](https://github.com/shlinkio/shlink/issues/1685) Changed `loosely` mode to `loose`, as it was a typo. The old one keeps working and maps to the new one, but it's considered deprecated.
+
+### Deprecated
+* *Nothing*
+
+### Removed
+* *Nothing*
+
+### Fixed
+* [#1682](https://github.com/shlinkio/shlink/issues/1682) Fixed incorrect case-insensitive checks in short URLs when using Microsoft SQL server.
+* [#1684](https://github.com/shlinkio/shlink/issues/1684) Fixed entities metadata cache not being cleared at docker container start-up when using redis with replication.
+
+
 ## [3.5.0] - 2023-01-28
 ### Added
 * [#1557](https://github.com/shlinkio/shlink/issues/1557) Added support to dynamically redirect to different long URLs based on the visitor's device type.
@@ -25,9 +43,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this
 * [#1662](https://github.com/shlinkio/shlink/issues/1662) Added support to provide openswoole-specific config options via env vars prefixed with `OPENSWOOLE_`.
 * [#1389](https://github.com/shlinkio/shlink/issues/1389) and [#706](https://github.com/shlinkio/shlink/issues/706) Added support for case-insensitive short URLs.
 
-  In order to achieve this, a new env var/config option has been implemented (`SHORT_URL_MODE`), which allows either `strict` or `loosely`.
+  In order to achieve this, a new env var/config option has been implemented (`SHORT_URL_MODE`), which allows either `strict` or ~~`loosely`~~ `loose`.
 
-  Default value is `strict`, but if `loosely` is provided, then short URLs will be matched in a case-insensitive way, and new short URLs will be generated with short-codes in lowercase only.
+  Default value is `strict`, but if `loose` is provided, then short URLs will be matched in a case-insensitive way, and new short URLs will be generated with short-codes in lowercase only.
 
 ### Changed
 * *Nothing*

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?logo=docker&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/main/LICENSE)
-[![Twitter](https://img.shields.io/twitter/follow/shlinkio?color=blue&label=follow&logo=twitter&style=flat-square)](https://twitter.com/shlinkio)
+[![Twitter](https://img.shields.io/badge/follow-shlinkio-blue.svg?style=flat-square&logo=twitter&color=blue)](https://twitter.com/shlinkio)
 [![Mastodon](https://img.shields.io/mastodon/follow/109329425426175098?color=%236364ff&domain=https%3A%2F%2Ffosstodon.org&label=follow&logo=mastodon&logoColor=white&style=flat-square)](https://fosstodon.org/@shlinkio)
 [![Paypal donate](https://img.shields.io/badge/Donate-paypal-blue.svg?style=flat-square&logo=paypal&colorA=aaaaaa)](https://slnk.to/donate)
 

composer.json

@@ -46,7 +46,7 @@
         "php-middleware/request-id": "^4.1",
         "pugx/shortid-php": "^1.1",
         "ramsey/uuid": "^4.5",
-        "shlinkio/shlink-common": "^5.3",
+        "shlinkio/shlink-common": "^5.3.1",
         "shlinkio/shlink-config": "^2.4",
         "shlinkio/shlink-event-dispatcher": "^2.6",
         "shlinkio/shlink-importer": "^5.0",

config/autoload/url-shortener.global.php

@@ -14,7 +14,7 @@ return (static function (): array {
         MIN_SHORT_CODES_LENGTH,
     );
     $modeFromEnv = EnvVars::SHORT_URL_MODE->loadFromEnv(ShortUrlMode::STRICT->value);
-    $mode = ShortUrlMode::tryFrom($modeFromEnv) ?? ShortUrlMode::STRICT;
+    $mode = ShortUrlMode::tryDeprecated($modeFromEnv) ?? ShortUrlMode::STRICT;
 
     return [
 

data/migrations/Version20230130090946.php

@@ -0,0 +1,50 @@
+<?php
+
+declare(strict_types=1);
+
+namespace ShlinkMigrations;
+
+use Doctrine\DBAL\Platforms\MySQLPlatform;
+use Doctrine\DBAL\Platforms\SQLServerPlatform;
+use Doctrine\DBAL\Schema\Schema;
+use Doctrine\Migrations\AbstractMigration;
+
+final class Version20230130090946 extends AbstractMigration
+{
+    public function up(Schema $schema): void
+    {
+        $this->skipIf(! $this->isMsSql(), 'This only sets MsSQL-specific database options');
+
+        $shortUrls = $schema->getTable('short_urls');
+        $shortCode = $shortUrls->getColumn('short_code');
+        // Drop the unique index before changing the collation, as the field is part of this index
+        $shortUrls->dropIndex('unique_short_code_plus_domain');
+        $shortCode->setPlatformOption('collation', 'Latin1_General_CS_AS');
+    }
+
+    public function postUp(Schema $schema): void
+    {
+        if ($this->isMsSql()) {
+            // The index needs to be re-created in postUp, but here, we can only use statements run against the
+            // connection directly
+            $this->connection->executeStatement(
+                'CREATE INDEX unique_short_code_plus_domain ON short_urls (domain_id, short_code);',
+            );
+        }
+    }
+
+    public function down(Schema $schema): void
+    {
+        // No down
+    }
+
+    public function isTransactional(): bool
+    {
+        return ! ($this->connection->getDatabasePlatform() instanceof MySQLPlatform);
+    }
+
+    private function isMsSql(): bool
+    {
+        return $this->connection->getDatabasePlatform() instanceof SQLServerPlatform;
+    }
+}

module/Core/src/Options/UrlShortenerOptions.php

@@ -22,8 +22,8 @@ final class UrlShortenerOptions
     ) {
     }
 
-    public function isLooselyMode(): bool
+    public function isLooseMode(): bool
     {
-        return $this->mode === ShortUrlMode::LOOSELY;
+        return $this->mode === ShortUrlMode::LOOSE;
     }
 }

module/Core/src/ShortUrl/Entity/ShortUrl.php

@@ -39,8 +39,8 @@ class ShortUrl extends AbstractEntity
     private string $longUrl;
     private string $shortCode;
     private Chronos $dateCreated;
-    /** @var Collection<int, Visit> */
-    private Collection $visits;
+    /** @var Collection<int, Visit> & Selectable */
+    private Collection & Selectable $visits;
     /** @var Collection<string, DeviceLongUrl> */
     private Collection $deviceLongUrls;
     /** @var Collection<int, Tag> */
@@ -255,23 +255,19 @@ class ShortUrl extends AbstractEntity
 
     public function mostRecentImportedVisitDate(): ?Chronos
     {
-        /** @var Selectable $visits */
-        $visits = $this->visits;
         $criteria = Criteria::create()->where(Criteria::expr()->eq('type', VisitType::IMPORTED))
                                       ->orderBy(['id' => 'DESC'])
                                       ->setMaxResults(1);
+        $visit = $this->visits->matching($criteria)->last();
 
-        /** @var Visit|false $visit */
-        $visit = $visits->matching($criteria)->last();
-
-        return $visit === false ? null : $visit->getDate();
+        return $visit instanceof Visit ? $visit->getDate() : null;
     }
 
     /**
-     * @param Collection<int, Visit> $visits
+     * @param Collection<int, Visit> & Selectable $visits
      * @internal
      */
-    public function setVisits(Collection $visits): self
+    public function setVisits(Collection & Selectable $visits): self
     {
         $this->visits = $visits;
         return $this;

module/Core/src/ShortUrl/Model/ShortUrlMode.php

@@ -5,5 +5,11 @@ namespace Shlinkio\Shlink\Core\ShortUrl\Model;
 enum ShortUrlMode: string
 {
     case STRICT = 'strict';
-    case LOOSELY = 'loosely';
+    case LOOSE = 'loose';
+
+    /** @deprecated */
+    public static function tryDeprecated(string $mode): ?self
+    {
+        return $mode === 'loosely' ? self::LOOSE : self::tryFrom($mode);
+    }
 }

module/Core/src/ShortUrl/Model/Validation/CustomSlugFilter.php

@@ -24,7 +24,7 @@ class CustomSlugFilter implements FilterInterface
             return $value;
         }
 
-        $value = $this->options->isLooselyMode() ? strtolower($value) : $value;
+        $value = $this->options->isLooseMode() ? strtolower($value) : $value;
         return (match ($this->options->multiSegmentSlugsEnabled) {
             true => trim(str_replace(' ', '-', $value), '/'),
             false => str_replace([' ', '/'], '-', $value),

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

@@ -5,7 +5,6 @@ declare(strict_types=1);
 namespace ShlinkioDbTest\Shlink\Core\ShortUrl\Repository;
 
 use Cake\Chronos\Chronos;
-use Doctrine\DBAL\Platforms\SQLServerPlatform;
 use Shlinkio\Shlink\Core\Domain\Entity\Domain;
 use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl;
 use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlCreation;
@@ -55,19 +54,16 @@ class ShortUrlRepositoryTest extends DatabaseTestCase
         ));
         self::assertSame($regularOne, $this->repo->findOneWithDomainFallback(
             ShortUrlIdentifier::fromShortCodeAndDomain('foo'),
-            ShortUrlMode::LOOSELY,
+            ShortUrlMode::LOOSE,
         ));
         self::assertSame($regularOne, $this->repo->findOneWithDomainFallback(
             ShortUrlIdentifier::fromShortCodeAndDomain('fOo'),
-            ShortUrlMode::LOOSELY,
+            ShortUrlMode::LOOSE,
+        ));
+        self::assertNull($this->repo->findOneWithDomainFallback(
+            ShortUrlIdentifier::fromShortCodeAndDomain('foo'),
+            ShortUrlMode::STRICT,
         ));
-        // TODO MS is doing loosely checks always, making this fail.
-        if (! $this->getEntityManager()->getConnection()->getDatabasePlatform() instanceof SQLServerPlatform) {
-            self::assertNull($this->repo->findOneWithDomainFallback(
-                ShortUrlIdentifier::fromShortCodeAndDomain('foo'),
-                ShortUrlMode::STRICT,
-            ));
-        }
         self::assertSame($regularOne, $this->repo->findOneWithDomainFallback(
             ShortUrlIdentifier::fromShortCodeAndDomain($withDomainDuplicatingRegular->getShortCode()),
             ShortUrlMode::STRICT,

module/Core/test/ShortUrl/Entity/ShortUrlTest.php

@@ -139,7 +139,7 @@ class ShortUrlTest extends TestCase
     }
 
     /** @test */
-    public function generatesLowercaseOnlyShortCodesInLooselyMode(): void
+    public function generatesLowercaseOnlyShortCodesInLooseMode(): void
     {
         $range = range(1, 1000); // Use a "big" number to reduce false negatives
         $allFor = static fn (ShortUrlMode $mode): bool => every($range, static function () use ($mode): bool {
@@ -152,7 +152,7 @@ class ShortUrlTest extends TestCase
             return $shortCode === strtolower($shortCode);
         });
 
-        self::assertTrue($allFor(ShortUrlMode::LOOSELY));
+        self::assertTrue($allFor(ShortUrlMode::LOOSE));
         self::assertFalse($allFor(ShortUrlMode::STRICT));
     }
 }

module/Core/test/ShortUrl/Model/ShortUrlCreationTest.php

@@ -140,20 +140,20 @@ class ShortUrlCreationTest extends TestCase
     {
         yield ['🔥', '🔥'];
         yield ['🦣 🍅', '🦣-🍅'];
-        yield ['🦣 🍅', '🦣-🍅', false, ShortUrlMode::LOOSELY];
+        yield ['🦣 🍅', '🦣-🍅', false, ShortUrlMode::LOOSE];
         yield ['foobar', 'foobar'];
         yield ['foo bar', 'foo-bar'];
         yield ['foo bar baz', 'foo-bar-baz'];
         yield ['foo bar-baz', 'foo-bar-baz'];
-        yield ['foo BAR-baz', 'foo-bar-baz', false, ShortUrlMode::LOOSELY];
+        yield ['foo BAR-baz', 'foo-bar-baz', false, ShortUrlMode::LOOSE];
         yield ['foo/bar/baz', 'foo/bar/baz', true];
         yield ['/foo/bar/baz', 'foo/bar/baz', true];
-        yield ['/foo/baR/baZ', 'foo/bar/baz', true, ShortUrlMode::LOOSELY];
+        yield ['/foo/baR/baZ', 'foo/bar/baz', true, ShortUrlMode::LOOSE];
         yield ['foo/bar/baz', 'foo-bar-baz'];
         yield ['/foo/bar/baz', '-foo-bar-baz'];
         yield ['wp-admin.php', 'wp-admin.php'];
         yield ['UPPER_lower', 'UPPER_lower'];
-        yield ['UPPER_lower', 'upper_lower', false, ShortUrlMode::LOOSELY];
+        yield ['UPPER_lower', 'upper_lower', false, ShortUrlMode::LOOSE];
         yield ['more~url_special.chars', 'more~url_special.chars'];
         yield ['구글', '구글'];
         yield ['グーグル', 'グーグル'];

module/Core/test/ShortUrl/Model/ShortUrlModeTest.php

@@ -0,0 +1,29 @@
+<?php
+
+declare(strict_types=1);
+
+namespace ShlinkioTest\Shlink\Core\ShortUrl\Model;
+
+use PHPUnit\Framework\TestCase;
+use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlMode;
+
+class ShortUrlModeTest extends TestCase
+{
+    /**
+     * @test
+     * @dataProvider provideModes
+     */
+    public function deprecatedValuesAreProperlyParsed(string $mode, ?ShortUrlMode $expected): void
+    {
+        self::assertSame($expected, ShortUrlMode::tryDeprecated($mode));
+    }
+
+    public function provideModes(): iterable
+    {
+        yield 'invalid' => ['invalid', null];
+        yield 'foo' => ['foo', null];
+        yield 'loose' => ['loose', ShortUrlMode::LOOSE];
+        yield 'loosely' => ['loosely', ShortUrlMode::LOOSE];
+        yield 'strict' => ['strict', ShortUrlMode::STRICT];
+    }
+}