From 883ac1007af0c0bf467c4e5792bbf20dd512fc33 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Wed, 4 Aug 2021 18:46:19 +0200 Subject: [PATCH 01/55] Updated to provisional hero-common v4.0 --- composer.json | 2 +- module/CLI/src/Command/ShortUrl/GetVisitsCommand.php | 4 ++-- module/CLI/test/Command/ShortUrl/GetVisitsCommandTest.php | 6 +++--- module/Core/functions/functions.php | 8 ++------ module/Core/src/Model/ShortUrlsParams.php | 3 ++- module/Core/src/Model/VisitsParams.php | 2 +- module/Core/src/Repository/ShortUrlRepository.php | 8 ++++---- module/Core/src/Repository/VisitRepository.php | 8 ++++---- module/Core/src/Spec/InDateRange.php | 8 ++++---- module/Core/test-db/Repository/ShortUrlRepositoryTest.php | 8 ++++---- .../Adapter/VisitsForTagPaginatorAdapterTest.php | 2 +- .../test/Paginator/Adapter/VisitsPaginatorAdapterTest.php | 4 ++-- .../Rest/test/Action/Visit/ShortUrlVisitsActionTest.php | 2 +- 13 files changed, 31 insertions(+), 34 deletions(-) diff --git a/composer.json b/composer.json index 17a5e21f..284a34e5 100644 --- a/composer.json +++ b/composer.json @@ -47,7 +47,7 @@ "predis/predis": "^1.1", "pugx/shortid-php": "^0.7", "ramsey/uuid": "^3.9", - "shlinkio/shlink-common": "^3.7", + "shlinkio/shlink-common": "dev-main#6e96039 as 4.0", "shlinkio/shlink-config": "^1.2", "shlinkio/shlink-event-dispatcher": "^2.1", "shlinkio/shlink-importer": "^2.3.1", diff --git a/module/CLI/src/Command/ShortUrl/GetVisitsCommand.php b/module/CLI/src/Command/ShortUrl/GetVisitsCommand.php index 5113debc..bb2f0229 100644 --- a/module/CLI/src/Command/ShortUrl/GetVisitsCommand.php +++ b/module/CLI/src/Command/ShortUrl/GetVisitsCommand.php @@ -7,7 +7,6 @@ namespace Shlinkio\Shlink\CLI\Command\ShortUrl; use Shlinkio\Shlink\CLI\Command\Util\AbstractWithDateRangeCommand; use Shlinkio\Shlink\CLI\Util\ExitCodes; use Shlinkio\Shlink\CLI\Util\ShlinkTable; -use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Entity\Visit; use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Model\VisitsParams; @@ -21,6 +20,7 @@ use Symfony\Component\Console\Style\SymfonyStyle; use function Functional\map; use function Functional\select_keys; +use function Shlinkio\Shlink\Common\buildDateRange; use function sprintf; class GetVisitsCommand extends AbstractWithDateRangeCommand @@ -73,7 +73,7 @@ class GetVisitsCommand extends AbstractWithDateRangeCommand $paginator = $this->visitsHelper->visitsForShortUrl( $identifier, - new VisitsParams(new DateRange($startDate, $endDate)), + new VisitsParams(buildDateRange($startDate, $endDate)), ); $rows = map($paginator->getCurrentPageResults(), function (Visit $visit) { diff --git a/module/CLI/test/Command/ShortUrl/GetVisitsCommandTest.php b/module/CLI/test/Command/ShortUrl/GetVisitsCommandTest.php index b9262217..ca9e0981 100644 --- a/module/CLI/test/Command/ShortUrl/GetVisitsCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/GetVisitsCommandTest.php @@ -45,7 +45,7 @@ class GetVisitsCommandTest extends TestCase $shortCode = 'abc123'; $this->visitsHelper->visitsForShortUrl( new ShortUrlIdentifier($shortCode), - new VisitsParams(new DateRange(null, null)), + new VisitsParams(DateRange::emptyInstance()), ) ->willReturn(new Paginator(new ArrayAdapter([]))) ->shouldBeCalledOnce(); @@ -61,7 +61,7 @@ class GetVisitsCommandTest extends TestCase $endDate = '2016-02-01'; $this->visitsHelper->visitsForShortUrl( new ShortUrlIdentifier($shortCode), - new VisitsParams(new DateRange(Chronos::parse($startDate), Chronos::parse($endDate))), + new VisitsParams(DateRange::withStartAndEndDate(Chronos::parse($startDate), Chronos::parse($endDate))), ) ->willReturn(new Paginator(new ArrayAdapter([]))) ->shouldBeCalledOnce(); @@ -80,7 +80,7 @@ class GetVisitsCommandTest extends TestCase $startDate = 'foo'; $info = $this->visitsHelper->visitsForShortUrl( new ShortUrlIdentifier($shortCode), - new VisitsParams(new DateRange()), + new VisitsParams(DateRange::emptyInstance()), )->willReturn(new Paginator(new ArrayAdapter([]))); $this->commandTester->execute([ diff --git a/module/Core/functions/functions.php b/module/Core/functions/functions.php index 7910ad1a..888e1833 100644 --- a/module/Core/functions/functions.php +++ b/module/Core/functions/functions.php @@ -16,6 +16,7 @@ use function Functional\reduce_left; use function is_array; use function lcfirst; use function print_r; +use function Shlinkio\Shlink\Common\buildDateRange; use function sprintf; use function str_repeat; use function str_replace; @@ -51,12 +52,7 @@ function parseDateRangeFromQuery(array $query, string $startDateName, string $en $startDate = parseDateFromQuery($query, $startDateName); $endDate = parseDateFromQuery($query, $endDateName); - return match (true) { - $startDate === null && $endDate === null => DateRange::emptyInstance(), - $startDate !== null && $endDate !== null => DateRange::withStartAndEndDate($startDate, $endDate), - $startDate !== null => DateRange::withStartDate($startDate), - default => DateRange::withEndDate($endDate), - }; + return buildDateRange($startDate, $endDate); } /** diff --git a/module/Core/src/Model/ShortUrlsParams.php b/module/Core/src/Model/ShortUrlsParams.php index 2336b18a..b3761ea8 100644 --- a/module/Core/src/Model/ShortUrlsParams.php +++ b/module/Core/src/Model/ShortUrlsParams.php @@ -8,6 +8,7 @@ use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Exception\ValidationException; use Shlinkio\Shlink\Core\Validation\ShortUrlsParamsInputFilter; +use function Shlinkio\Shlink\Common\buildDateRange; use function Shlinkio\Shlink\Core\parseDateField; final class ShortUrlsParams @@ -54,7 +55,7 @@ final class ShortUrlsParams $this->page = (int) ($inputFilter->getValue(ShortUrlsParamsInputFilter::PAGE) ?? 1); $this->searchTerm = $inputFilter->getValue(ShortUrlsParamsInputFilter::SEARCH_TERM); $this->tags = (array) $inputFilter->getValue(ShortUrlsParamsInputFilter::TAGS); - $this->dateRange = new DateRange( + $this->dateRange = buildDateRange( parseDateField($inputFilter->getValue(ShortUrlsParamsInputFilter::START_DATE)), parseDateField($inputFilter->getValue(ShortUrlsParamsInputFilter::END_DATE)), ); diff --git a/module/Core/src/Model/VisitsParams.php b/module/Core/src/Model/VisitsParams.php index 5ace1d8d..efe082db 100644 --- a/module/Core/src/Model/VisitsParams.php +++ b/module/Core/src/Model/VisitsParams.php @@ -23,7 +23,7 @@ final class VisitsParams ?int $itemsPerPage = null, private bool $excludeBots = false ) { - $this->dateRange = $dateRange ?? new DateRange(); + $this->dateRange = $dateRange ?? DateRange::emptyInstance(); $this->page = $this->determinePage($page); $this->itemsPerPage = $this->determineItemsPerPage($itemsPerPage); } diff --git a/module/Core/src/Repository/ShortUrlRepository.php b/module/Core/src/Repository/ShortUrlRepository.php index 4c3a4e9c..fb853b96 100644 --- a/module/Core/src/Repository/ShortUrlRepository.php +++ b/module/Core/src/Repository/ShortUrlRepository.php @@ -105,13 +105,13 @@ class ShortUrlRepository extends EntitySpecificationRepository implements ShortU $qb->from(ShortUrl::class, 's') ->where('1=1'); - if ($dateRange?->getStartDate() !== null) { + if ($dateRange?->startDate() !== null) { $qb->andWhere($qb->expr()->gte('s.dateCreated', ':startDate')); - $qb->setParameter('startDate', $dateRange->getStartDate(), ChronosDateTimeType::CHRONOS_DATETIME); + $qb->setParameter('startDate', $dateRange->startDate(), ChronosDateTimeType::CHRONOS_DATETIME); } - if ($dateRange?->getEndDate() !== null) { + if ($dateRange?->endDate() !== null) { $qb->andWhere($qb->expr()->lte('s.dateCreated', ':endDate')); - $qb->setParameter('endDate', $dateRange->getEndDate(), ChronosDateTimeType::CHRONOS_DATETIME); + $qb->setParameter('endDate', $dateRange->endDate(), ChronosDateTimeType::CHRONOS_DATETIME); } // Apply search term to every searchable field if not empty diff --git a/module/Core/src/Repository/VisitRepository.php b/module/Core/src/Repository/VisitRepository.php index 6adba193..1e157757 100644 --- a/module/Core/src/Repository/VisitRepository.php +++ b/module/Core/src/Repository/VisitRepository.php @@ -187,11 +187,11 @@ class VisitRepository extends EntitySpecificationRepository implements VisitRepo private function applyDatesInline(QueryBuilder $qb, ?DateRange $dateRange): void { - if ($dateRange?->getStartDate() !== null) { - $qb->andWhere($qb->expr()->gte('v.date', '\'' . $dateRange->getStartDate()->toDateTimeString() . '\'')); + if ($dateRange?->startDate() !== null) { + $qb->andWhere($qb->expr()->gte('v.date', '\'' . $dateRange->startDate()->toDateTimeString() . '\'')); } - if ($dateRange?->getEndDate() !== null) { - $qb->andWhere($qb->expr()->lte('v.date', '\'' . $dateRange->getEndDate()->toDateTimeString() . '\'')); + if ($dateRange?->endDate() !== null) { + $qb->andWhere($qb->expr()->lte('v.date', '\'' . $dateRange->endDate()->toDateTimeString() . '\'')); } } diff --git a/module/Core/src/Spec/InDateRange.php b/module/Core/src/Spec/InDateRange.php index 81d11b9e..05cb6f0a 100644 --- a/module/Core/src/Spec/InDateRange.php +++ b/module/Core/src/Spec/InDateRange.php @@ -20,12 +20,12 @@ class InDateRange extends BaseSpecification { $criteria = []; - if ($this->dateRange?->getStartDate() !== null) { - $criteria[] = Spec::gte($this->field, $this->dateRange->getStartDate()->toDateTimeString()); + if ($this->dateRange?->startDate() !== null) { + $criteria[] = Spec::gte($this->field, $this->dateRange->startDate()->toDateTimeString()); } - if ($this->dateRange?->getEndDate() !== null) { - $criteria[] = Spec::lte($this->field, $this->dateRange->getEndDate()->toDateTimeString()); + if ($this->dateRange?->endDate() !== null) { + $criteria[] = Spec::lte($this->field, $this->dateRange->endDate()->toDateTimeString()); } return Spec::andX(...$criteria); diff --git a/module/Core/test-db/Repository/ShortUrlRepositoryTest.php b/module/Core/test-db/Repository/ShortUrlRepositoryTest.php index adc3d67f..a614844f 100644 --- a/module/Core/test-db/Repository/ShortUrlRepositoryTest.php +++ b/module/Core/test-db/Repository/ShortUrlRepositoryTest.php @@ -133,16 +133,16 @@ class ShortUrlRepositoryTest extends DatabaseTestCase self::assertCount(3, $result); self::assertSame($bar, $result[0]); - $result = $this->repo->findList(null, null, null, [], null, new DateRange(null, Chronos::now()->subDays(2))); + $result = $this->repo->findList(null, null, null, [], null, DateRange::withEndDate(Chronos::now()->subDays(2))); self::assertCount(1, $result); - self::assertEquals(1, $this->repo->countList(null, [], new DateRange(null, Chronos::now()->subDays(2)))); + self::assertEquals(1, $this->repo->countList(null, [], DateRange::withEndDate(Chronos::now()->subDays(2)))); self::assertSame($foo2, $result[0]); self::assertCount( 2, - $this->repo->findList(null, null, null, [], null, new DateRange(Chronos::now()->subDays(2))), + $this->repo->findList(null, null, null, [], null, DateRange::withStartDate(Chronos::now()->subDays(2))), ); - self::assertEquals(2, $this->repo->countList(null, [], new DateRange(Chronos::now()->subDays(2)))); + self::assertEquals(2, $this->repo->countList(null, [], DateRange::withStartDate(Chronos::now()->subDays(2)))); } /** @test */ diff --git a/module/Core/test/Paginator/Adapter/VisitsForTagPaginatorAdapterTest.php b/module/Core/test/Paginator/Adapter/VisitsForTagPaginatorAdapterTest.php index aa684b70..c92e21c6 100644 --- a/module/Core/test/Paginator/Adapter/VisitsForTagPaginatorAdapterTest.php +++ b/module/Core/test/Paginator/Adapter/VisitsForTagPaginatorAdapterTest.php @@ -35,7 +35,7 @@ class VisitsForTagPaginatorAdapterTest extends TestCase $adapter = $this->createAdapter(null); $findVisits = $this->repo->findVisitsByTag( 'foo', - new VisitsListFiltering(new DateRange(), false, null, $limit, $offset), + new VisitsListFiltering(DateRange::emptyInstance(), false, null, $limit, $offset), )->willReturn([]); for ($i = 0; $i < $count; $i++) { diff --git a/module/Core/test/Paginator/Adapter/VisitsPaginatorAdapterTest.php b/module/Core/test/Paginator/Adapter/VisitsPaginatorAdapterTest.php index 97a2c1f0..413ae1cd 100644 --- a/module/Core/test/Paginator/Adapter/VisitsPaginatorAdapterTest.php +++ b/module/Core/test/Paginator/Adapter/VisitsPaginatorAdapterTest.php @@ -36,7 +36,7 @@ class VisitsPaginatorAdapterTest extends TestCase $adapter = $this->createAdapter(null); $findVisits = $this->repo->findVisitsByShortCode( ShortUrlIdentifier::fromShortCodeAndDomain(''), - new VisitsListFiltering(new DateRange(), false, null, $limit, $offset), + new VisitsListFiltering(DateRange::emptyInstance(), false, null, $limit, $offset), )->willReturn([]); for ($i = 0; $i < $count; $i++) { @@ -54,7 +54,7 @@ class VisitsPaginatorAdapterTest extends TestCase $adapter = $this->createAdapter($apiKey); $countVisits = $this->repo->countVisitsByShortCode( ShortUrlIdentifier::fromShortCodeAndDomain(''), - new VisitsCountFiltering(new DateRange(), false, $apiKey->spec()), + new VisitsCountFiltering(DateRange::emptyInstance(), false, $apiKey->spec()), )->willReturn(3); for ($i = 0; $i < $count; $i++) { diff --git a/module/Rest/test/Action/Visit/ShortUrlVisitsActionTest.php b/module/Rest/test/Action/Visit/ShortUrlVisitsActionTest.php index d0c67e7c..6e982aec 100644 --- a/module/Rest/test/Action/Visit/ShortUrlVisitsActionTest.php +++ b/module/Rest/test/Action/Visit/ShortUrlVisitsActionTest.php @@ -53,7 +53,7 @@ class ShortUrlVisitsActionTest extends TestCase { $shortCode = 'abc123'; $this->visitsHelper->visitsForShortUrl(new ShortUrlIdentifier($shortCode), new VisitsParams( - new DateRange(null, Chronos::parse('2016-01-01 00:00:00')), + DateRange::withEndDate(Chronos::parse('2016-01-01 00:00:00')), 3, 10, ), Argument::type(ApiKey::class)) From 6ae0c7dcfc45ef612d575616ef9857d58741488c Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 5 Aug 2021 14:05:44 +0200 Subject: [PATCH 02/55] Updated to latest common with symfony/cache support --- composer.json | 3 +-- config/autoload/locks.global.php | 4 ++-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/composer.json b/composer.json index 284a34e5..e94caa16 100644 --- a/composer.json +++ b/composer.json @@ -18,7 +18,6 @@ "akrabat/ip-address-middleware": "^2.0", "cakephp/chronos": "^2.2", "cocur/slugify": "^4.0", - "doctrine/cache": "^1.12", "doctrine/migrations": "^3.2", "doctrine/orm": "^2.9", "endroid/qr-code": "^4.2", @@ -47,7 +46,7 @@ "predis/predis": "^1.1", "pugx/shortid-php": "^0.7", "ramsey/uuid": "^3.9", - "shlinkio/shlink-common": "dev-main#6e96039 as 4.0", + "shlinkio/shlink-common": "dev-main#2832a4a as 4.0", "shlinkio/shlink-config": "^1.2", "shlinkio/shlink-event-dispatcher": "^2.1", "shlinkio/shlink-importer": "^2.3.1", diff --git a/config/autoload/locks.global.php b/config/autoload/locks.global.php index 25c00f22..26b31e15 100644 --- a/config/autoload/locks.global.php +++ b/config/autoload/locks.global.php @@ -3,7 +3,7 @@ declare(strict_types=1); use Laminas\ServiceManager\AbstractFactory\ConfigAbstractFactory; -use Shlinkio\Shlink\Common\Cache\RedisFactory; +use Predis\ClientInterface as PredisClient; use Shlinkio\Shlink\Common\Lock\RetryLockStoreDelegatorFactory; use Shlinkio\Shlink\Common\Logger\LoggerAwareDelegatorFactory; use Symfony\Component\Lock; @@ -42,7 +42,7 @@ return [ ConfigAbstractFactory::class => [ Lock\Store\FlockStore::class => ['config.locks.locks_dir'], - Lock\Store\RedisStore::class => [RedisFactory::SERVICE_NAME], + Lock\Store\RedisStore::class => [PredisClient::class], Lock\LockFactory::class => ['lock_store'], LOCAL_LOCK_FACTORY => ['local_lock_store'], ], From 2bfe21aef4ad740f98bc7a9a3c8e683c6e8d2b21 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 5 Aug 2021 16:47:30 +0200 Subject: [PATCH 03/55] Documented architectural decission on what component to pick for caching --- ...-08-05-migrate-to-a-new-caching-library.md | 59 +++++++++++++++++++ docs/adr/README.md | 1 + 2 files changed, 60 insertions(+) create mode 100644 docs/adr/2021-08-05-migrate-to-a-new-caching-library.md diff --git a/docs/adr/2021-08-05-migrate-to-a-new-caching-library.md b/docs/adr/2021-08-05-migrate-to-a-new-caching-library.md new file mode 100644 index 00000000..aa19f160 --- /dev/null +++ b/docs/adr/2021-08-05-migrate-to-a-new-caching-library.md @@ -0,0 +1,59 @@ +# Migrate to a new caching library + +* Status: Accepted +* Date: 2021-08-05 + +## Context and problem statement + +Shlink has always used the `doctrine/cache` library to handle anything related with cache. + +It was convenient, as it provided several adapters, and it was the library used by other doctrine packages. + +However, after the creation of the caching PSRs ([PSR-6 - Cache](https://www.php-fig.org/psr/psr-6) and [PSR-16 - Simple cache](https://www.php-fig.org/psr/psr-16)), most library authors have moved to those interfaces, and the doctrine team has decided to recommend using any other existing package and decommission their own solution. + +Also, Shlink needs support for Redis clusters and Redis sentinels, which is not supported by `doctrine/cache` Redis adapters. + +## Considered option + +After some research, the only packages that seem to support the capabilities required by Shlink and also seem healthy, are these: + +* [Symfony cache](https://symfony.com/doc/current/components/cache.html) + * 🟢 PSR-6 compliant: **yes** + * 🟢 PSR-16 compliant: **yes** + * 🟢 APCu support: **yes** + * 🟢 Redis support: **yes** + * 🟢 Redis cluster support: **yes** + * 🟢 Redis sentinel support: **yes** + * 🟢 Can use redis through Predis: **yes** + * 🔴 Individual packages per adapter: **no** +* [Laminas cache](https://docs.laminas.dev/laminas-cache/) + * 🟢 PSR-6 compliant: **yes** + * 🟢 PSR-16 compliant: **yes** + * 🟢 APCu support: **yes** + * 🟢 Redis support: **yes** + * 🟢 Redis cluster support: **yes** + * 🔴 Redis sentinel support: **no** + * 🔴 Can use redis through Predis: **no** + * 🟢 Individual packages per adapter: **yes** + +## Decision outcome + +Even though Symfony packs all their adapters in a single component, which means we will install some code that will never be used, Laminas relies on the native redis extension for anything related with redis. + +That would make Shlink more complex to install, so it seems Symfony's package is the option where it's easier to migrate to. + +Also, it's important that the cache component can share the Redis integration (through `Predis`, in this case), as it's also used by other components (the lock component, to name one). + +## Pros and Cons of the Options + +### Symfony cache + +* Good because it supports Redis Sentinel. +* Good because it allows using a external `Predis` instance. +* Bad because it packs all the adapters in a single component. + +### Laminas cache + +* Good because allows installing only the adapters you are going to use, through separated packages. +* Bad because it requires the php-redis native extension in order to interact with Redis. +* Bad because it does ot seem to support Redis Sentinels. diff --git a/docs/adr/README.md b/docs/adr/README.md index 93d82cff..af03faac 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -2,5 +2,6 @@ Here listed you will find the different architectural decisions taken in the project, including all the reasoning behind it, options considered, and final outcome. +* [2021-08-05 Migrate to a new caching library](2021-08-05-migrate-to-a-new-caching-library.md) * [2021-02-07 Track visits to 'base_url', 'invalid_short_url' and 'regular_404'](2021-02-07-track-visits-to-base-url-invalid-short-url-and-regular-404.md) * [2021-01-17 Support restrictions and permissions in API keys](2021-01-17-support-restrictions-and-permissions-in-api-keys.md) From eff744580494e747387faadc721e75498ae6ea43 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 5 Aug 2021 16:50:50 +0200 Subject: [PATCH 04/55] Updated changelog --- CHANGELOG.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 46a3b884..ad370d99 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,23 @@ 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 +* *Nothing* + +### Changed +* [#1142](https://github.com/shlinkio/shlink/issues/1142) Replaced `doctrine/cache` package with `symfony/cache`. + +### Deprecated +* *Nothing* + +### Removed +* *Nothing* + +### Fixed +* [#1098](https://github.com/shlinkio/shlink/issues/1098) Fixed errors when using a Redis Cluster for caching, caused by `doctrine/cache` not fully supporting clusters. + + ## [2.8.0] - 2021-08-04 ### Added * [#1089](https://github.com/shlinkio/shlink/issues/1089) Added new `ENABLE_PERIODIC_VISIT_LOCATE` env var to docker image which schedules the `visit:locate` command every hour when provided with value `true`. From 9a31f53d4db739cba5d9810028ecdb89e6377a13 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 5 Aug 2021 19:47:17 +0200 Subject: [PATCH 05/55] Updated to coding standard v2.2.0 --- composer.json | 4 ++-- data/migrations/Version20201102113208.php | 5 +---- module/CLI/src/Command/Api/GenerateKeyCommand.php | 2 +- module/CLI/src/Command/Db/AbstractDatabaseCommand.php | 2 +- module/CLI/src/Command/Db/CreateDatabaseCommand.php | 2 +- .../CLI/src/Command/ShortUrl/GenerateShortUrlCommand.php | 2 +- module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php | 2 +- module/CLI/src/Command/Util/LockedCommandConfig.php | 2 +- module/CLI/src/Command/Visit/LocateVisitsCommand.php | 2 +- module/CLI/src/Util/GeolocationDbUpdater.php | 2 +- .../test/Command/ShortUrl/ListShortUrlsCommandTest.php | 3 +-- module/CLI/test/Util/GeolocationDbUpdaterTest.php | 8 ++------ module/Core/functions/functions.php | 5 +---- module/Core/src/Action/Model/QrCodeParams.php | 2 +- module/Core/src/Action/QrCodeAction.php | 2 +- module/Core/src/Config/NotFoundRedirectResolver.php | 2 +- .../Core/src/Config/NotFoundRedirectResolverInterface.php | 2 +- module/Core/src/Domain/DomainService.php | 2 +- module/Core/src/Domain/Model/DomainItem.php | 2 +- module/Core/src/EventDispatcher/LocateVisit.php | 2 +- module/Core/src/EventDispatcher/NotifyVisitToMercure.php | 2 +- module/Core/src/EventDispatcher/NotifyVisitToWebHooks.php | 2 +- module/Core/src/Importer/ImportedLinksProcessor.php | 2 +- module/Core/src/Mercure/MercureUpdatesGenerator.php | 2 +- module/Core/src/Model/VisitsParams.php | 2 +- .../src/Paginator/Adapter/ShortUrlRepositoryAdapter.php | 2 +- .../Paginator/Adapter/VisitsForTagPaginatorAdapter.php | 2 +- .../Core/src/Paginator/Adapter/VisitsPaginatorAdapter.php | 2 +- .../Core/src/Service/ShortUrl/DeleteShortUrlService.php | 2 +- module/Core/src/Service/ShortUrlService.php | 2 +- module/Core/src/Service/UrlShortener.php | 2 +- .../Core/src/Visit/Persistence/VisitsCountFiltering.php | 2 +- module/Core/src/Visit/Persistence/VisitsListFiltering.php | 2 +- module/Core/src/Visit/VisitsTracker.php | 2 +- .../Middleware/ExtraPathRedirectMiddlewareTest.php | 2 +- module/Rest/src/Action/ShortUrl/ListShortUrlsAction.php | 2 +- module/Rest/src/Action/Visit/OrphanVisitsAction.php | 2 +- module/Rest/src/Middleware/AuthenticationMiddleware.php | 2 +- 38 files changed, 40 insertions(+), 51 deletions(-) diff --git a/composer.json b/composer.json index e94caa16..47fa16d8 100644 --- a/composer.json +++ b/composer.json @@ -46,7 +46,7 @@ "predis/predis": "^1.1", "pugx/shortid-php": "^0.7", "ramsey/uuid": "^3.9", - "shlinkio/shlink-common": "dev-main#2832a4a as 4.0", + "shlinkio/shlink-common": "dev-main#baef0ca as 4.0", "shlinkio/shlink-config": "^1.2", "shlinkio/shlink-event-dispatcher": "^2.1", "shlinkio/shlink-importer": "^2.3.1", @@ -71,7 +71,7 @@ "phpunit/php-code-coverage": "^9.2", "phpunit/phpunit": "^9.5", "roave/security-advisories": "dev-master", - "shlinkio/php-coding-standard": "~2.1.1", + "shlinkio/php-coding-standard": "~2.2.0", "shlinkio/shlink-test-utils": "^2.2", "symfony/var-dumper": "^5.3", "veewee/composer-run-parallel": "^1.0" diff --git a/data/migrations/Version20201102113208.php b/data/migrations/Version20201102113208.php index 1e1237a4..405ca5c7 100644 --- a/data/migrations/Version20201102113208.php +++ b/data/migrations/Version20201102113208.php @@ -60,10 +60,7 @@ final class Version20201102113208 extends AbstractMigration ->execute(); } - /** - * @return string|int|null - */ - private function resolveOneApiKeyId(Result $result) + private function resolveOneApiKeyId(Result $result): string|int|null { $results = []; while ($row = $result->fetchAssociative()) { diff --git a/module/CLI/src/Command/Api/GenerateKeyCommand.php b/module/CLI/src/Command/Api/GenerateKeyCommand.php index d39c05fa..a43c9e65 100644 --- a/module/CLI/src/Command/Api/GenerateKeyCommand.php +++ b/module/CLI/src/Command/Api/GenerateKeyCommand.php @@ -25,7 +25,7 @@ class GenerateKeyCommand extends BaseCommand public function __construct( private ApiKeyServiceInterface $apiKeyService, - private RoleResolverInterface $roleResolver + private RoleResolverInterface $roleResolver, ) { parent::__construct(); } diff --git a/module/CLI/src/Command/Db/AbstractDatabaseCommand.php b/module/CLI/src/Command/Db/AbstractDatabaseCommand.php index 9cd6e9ea..f803c50c 100644 --- a/module/CLI/src/Command/Db/AbstractDatabaseCommand.php +++ b/module/CLI/src/Command/Db/AbstractDatabaseCommand.php @@ -18,7 +18,7 @@ abstract class AbstractDatabaseCommand extends AbstractLockedCommand public function __construct( LockFactory $locker, private ProcessRunnerInterface $processRunner, - PhpExecutableFinder $phpFinder + PhpExecutableFinder $phpFinder, ) { parent::__construct($locker); $this->phpBinary = $phpFinder->find(false) ?: 'php'; diff --git a/module/CLI/src/Command/Db/CreateDatabaseCommand.php b/module/CLI/src/Command/Db/CreateDatabaseCommand.php index ad3959ca..100dc49d 100644 --- a/module/CLI/src/Command/Db/CreateDatabaseCommand.php +++ b/module/CLI/src/Command/Db/CreateDatabaseCommand.php @@ -26,7 +26,7 @@ class CreateDatabaseCommand extends AbstractDatabaseCommand ProcessRunnerInterface $processRunner, PhpExecutableFinder $phpFinder, private Connection $regularConn, - private Connection $noDbNameConn + private Connection $noDbNameConn, ) { parent::__construct($locker, $processRunner, $phpFinder); } diff --git a/module/CLI/src/Command/ShortUrl/GenerateShortUrlCommand.php b/module/CLI/src/Command/ShortUrl/GenerateShortUrlCommand.php index e0d2babc..6ed3e37c 100644 --- a/module/CLI/src/Command/ShortUrl/GenerateShortUrlCommand.php +++ b/module/CLI/src/Command/ShortUrl/GenerateShortUrlCommand.php @@ -33,7 +33,7 @@ class GenerateShortUrlCommand extends BaseCommand public function __construct( private UrlShortenerInterface $urlShortener, private ShortUrlStringifierInterface $stringifier, - private int $defaultShortCodeLength + private int $defaultShortCodeLength, ) { parent::__construct(); } diff --git a/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php b/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php index ff01030a..53e47d3c 100644 --- a/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php +++ b/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php @@ -35,7 +35,7 @@ class ListShortUrlsCommand extends AbstractWithDateRangeCommand public function __construct( private ShortUrlServiceInterface $shortUrlService, - private DataTransformerInterface $transformer + private DataTransformerInterface $transformer, ) { parent::__construct(); } diff --git a/module/CLI/src/Command/Util/LockedCommandConfig.php b/module/CLI/src/Command/Util/LockedCommandConfig.php index af9d704d..f053d99a 100644 --- a/module/CLI/src/Command/Util/LockedCommandConfig.php +++ b/module/CLI/src/Command/Util/LockedCommandConfig.php @@ -11,7 +11,7 @@ final class LockedCommandConfig private function __construct( private string $lockName, private bool $isBlocking, - private float $ttl = self::DEFAULT_TTL + private float $ttl = self::DEFAULT_TTL, ) { } diff --git a/module/CLI/src/Command/Visit/LocateVisitsCommand.php b/module/CLI/src/Command/Visit/LocateVisitsCommand.php index 7352211e..de66e84e 100644 --- a/module/CLI/src/Command/Visit/LocateVisitsCommand.php +++ b/module/CLI/src/Command/Visit/LocateVisitsCommand.php @@ -35,7 +35,7 @@ class LocateVisitsCommand extends AbstractLockedCommand implements VisitGeolocat public function __construct( private VisitLocatorInterface $visitLocator, private IpLocationResolverInterface $ipLocationResolver, - LockFactory $locker + LockFactory $locker, ) { parent::__construct($locker); } diff --git a/module/CLI/src/Util/GeolocationDbUpdater.php b/module/CLI/src/Util/GeolocationDbUpdater.php index afa0a864..67e9d485 100644 --- a/module/CLI/src/Util/GeolocationDbUpdater.php +++ b/module/CLI/src/Util/GeolocationDbUpdater.php @@ -23,7 +23,7 @@ class GeolocationDbUpdater implements GeolocationDbUpdaterInterface private DbUpdaterInterface $dbUpdater, private Reader $geoLiteDbReader, private LockFactory $locker, - private TrackingOptions $trackingOptions + private TrackingOptions $trackingOptions, ) { } diff --git a/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php b/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php index f4ba2bb1..8150d0c8 100644 --- a/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php @@ -238,11 +238,10 @@ class ListShortUrlsCommandTest extends TestCase } /** - * @param string|array|null $expectedOrderBy * @test * @dataProvider provideOrderBy */ - public function orderByIsProperlyComputed(array $commandArgs, $expectedOrderBy): void + public function orderByIsProperlyComputed(array $commandArgs, string|array|null $expectedOrderBy): void { $listShortUrls = $this->shortUrlService->listShortUrls(ShortUrlsParams::fromRawData([ 'orderBy' => $expectedOrderBy, diff --git a/module/CLI/test/Util/GeolocationDbUpdaterTest.php b/module/CLI/test/Util/GeolocationDbUpdaterTest.php index 0a52660f..83340fc5 100644 --- a/module/CLI/test/Util/GeolocationDbUpdaterTest.php +++ b/module/CLI/test/Util/GeolocationDbUpdaterTest.php @@ -116,9 +116,8 @@ class GeolocationDbUpdaterTest extends TestCase /** * @test * @dataProvider provideSmallDays - * @param string|int $buildEpoch */ - public function databaseIsNotUpdatedIfItIsYoungerThanOneWeek($buildEpoch): void + public function databaseIsNotUpdatedIfItIsYoungerThanOneWeek(string|int $buildEpoch): void { $fileExists = $this->dbUpdater->databaseFileExists()->willReturn(true); $getMeta = $this->geoLiteDbReader->metadata()->willReturn($this->buildMetaWithBuildEpoch($buildEpoch)); @@ -161,10 +160,7 @@ class GeolocationDbUpdaterTest extends TestCase $this->geolocationDbUpdater->checkDbUpdate(); } - /** - * @param string|int $buildEpoch - */ - private function buildMetaWithBuildEpoch($buildEpoch): Metadata + private function buildMetaWithBuildEpoch(string|int $buildEpoch): Metadata { return new Metadata([ 'binary_format_major_version' => '', diff --git a/module/Core/functions/functions.php b/module/Core/functions/functions.php index 888e1833..9ba4bdcc 100644 --- a/module/Core/functions/functions.php +++ b/module/Core/functions/functions.php @@ -55,10 +55,7 @@ function parseDateRangeFromQuery(array $query, string $startDateName, string $en return buildDateRange($startDate, $endDate); } -/** - * @param string|DateTimeInterface|Chronos|null $date - */ -function parseDateField($date): ?Chronos +function parseDateField(string|DateTimeInterface|Chronos|null $date): ?Chronos { if ($date === null || $date instanceof Chronos) { return $date; diff --git a/module/Core/src/Action/Model/QrCodeParams.php b/module/Core/src/Action/Model/QrCodeParams.php index 742d3f07..39cd59a9 100644 --- a/module/Core/src/Action/Model/QrCodeParams.php +++ b/module/Core/src/Action/Model/QrCodeParams.php @@ -28,7 +28,7 @@ final class QrCodeParams private int $size, private int $margin, private WriterInterface $writer, - private ErrorCorrectionLevelInterface $errorCorrectionLevel + private ErrorCorrectionLevelInterface $errorCorrectionLevel, ) { } diff --git a/module/Core/src/Action/QrCodeAction.php b/module/Core/src/Action/QrCodeAction.php index 2f816c98..977e0864 100644 --- a/module/Core/src/Action/QrCodeAction.php +++ b/module/Core/src/Action/QrCodeAction.php @@ -22,7 +22,7 @@ class QrCodeAction implements MiddlewareInterface public function __construct( private ShortUrlResolverInterface $urlResolver, private ShortUrlStringifierInterface $stringifier, - private LoggerInterface $logger + private LoggerInterface $logger, ) { } diff --git a/module/Core/src/Config/NotFoundRedirectResolver.php b/module/Core/src/Config/NotFoundRedirectResolver.php index 14264034..a6a70c31 100644 --- a/module/Core/src/Config/NotFoundRedirectResolver.php +++ b/module/Core/src/Config/NotFoundRedirectResolver.php @@ -16,7 +16,7 @@ class NotFoundRedirectResolver implements NotFoundRedirectResolverInterface public function resolveRedirectResponse( NotFoundType $notFoundType, - NotFoundRedirectConfigInterface $config + NotFoundRedirectConfigInterface $config, ): ?ResponseInterface { return match (true) { $notFoundType->isBaseUrl() && $config->hasBaseUrlRedirect() => diff --git a/module/Core/src/Config/NotFoundRedirectResolverInterface.php b/module/Core/src/Config/NotFoundRedirectResolverInterface.php index a5c55f3d..ab010d2e 100644 --- a/module/Core/src/Config/NotFoundRedirectResolverInterface.php +++ b/module/Core/src/Config/NotFoundRedirectResolverInterface.php @@ -11,6 +11,6 @@ interface NotFoundRedirectResolverInterface { public function resolveRedirectResponse( NotFoundType $notFoundType, - NotFoundRedirectConfigInterface $config + NotFoundRedirectConfigInterface $config, ): ?ResponseInterface; } diff --git a/module/Core/src/Domain/DomainService.php b/module/Core/src/Domain/DomainService.php index 6051c254..b5141324 100644 --- a/module/Core/src/Domain/DomainService.php +++ b/module/Core/src/Domain/DomainService.php @@ -84,7 +84,7 @@ class DomainService implements DomainServiceInterface public function configureNotFoundRedirects( string $authority, NotFoundRedirects $notFoundRedirects, - ?ApiKey $apiKey = null + ?ApiKey $apiKey = null, ): Domain { if ($authority === $this->defaultDomain) { throw InvalidDomainException::forDefaultDomainRedirects(); diff --git a/module/Core/src/Domain/Model/DomainItem.php b/module/Core/src/Domain/Model/DomainItem.php index cfd09d90..909cca7d 100644 --- a/module/Core/src/Domain/Model/DomainItem.php +++ b/module/Core/src/Domain/Model/DomainItem.php @@ -14,7 +14,7 @@ final class DomainItem implements JsonSerializable private function __construct( private string $authority, private NotFoundRedirectConfigInterface $notFoundRedirectConfig, - private bool $isDefault + private bool $isDefault, ) { } diff --git a/module/Core/src/EventDispatcher/LocateVisit.php b/module/Core/src/EventDispatcher/LocateVisit.php index bb6ba1d0..fbd32962 100644 --- a/module/Core/src/EventDispatcher/LocateVisit.php +++ b/module/Core/src/EventDispatcher/LocateVisit.php @@ -24,7 +24,7 @@ class LocateVisit private EntityManagerInterface $em, private LoggerInterface $logger, private DbUpdaterInterface $dbUpdater, - private EventDispatcherInterface $eventDispatcher + private EventDispatcherInterface $eventDispatcher, ) { } diff --git a/module/Core/src/EventDispatcher/NotifyVisitToMercure.php b/module/Core/src/EventDispatcher/NotifyVisitToMercure.php index d1ad8201..ed205f40 100644 --- a/module/Core/src/EventDispatcher/NotifyVisitToMercure.php +++ b/module/Core/src/EventDispatcher/NotifyVisitToMercure.php @@ -21,7 +21,7 @@ class NotifyVisitToMercure private HubInterface $hub, private MercureUpdatesGeneratorInterface $updatesGenerator, private EntityManagerInterface $em, - private LoggerInterface $logger + private LoggerInterface $logger, ) { } diff --git a/module/Core/src/EventDispatcher/NotifyVisitToWebHooks.php b/module/Core/src/EventDispatcher/NotifyVisitToWebHooks.php index 5b4e2818..dcd69b21 100644 --- a/module/Core/src/EventDispatcher/NotifyVisitToWebHooks.php +++ b/module/Core/src/EventDispatcher/NotifyVisitToWebHooks.php @@ -31,7 +31,7 @@ class NotifyVisitToWebHooks /** @var string[] */ private array $webhooks, private DataTransformerInterface $transformer, - private AppOptions $appOptions + private AppOptions $appOptions, ) { } diff --git a/module/Core/src/Importer/ImportedLinksProcessor.php b/module/Core/src/Importer/ImportedLinksProcessor.php index b153430b..cddfbb88 100644 --- a/module/Core/src/Importer/ImportedLinksProcessor.php +++ b/module/Core/src/Importer/ImportedLinksProcessor.php @@ -26,7 +26,7 @@ class ImportedLinksProcessor implements ImportedLinksProcessorInterface private EntityManagerInterface $em, private ShortUrlRelationResolverInterface $relationResolver, private ShortCodeHelperInterface $shortCodeHelper, - private DoctrineBatchHelperInterface $batchHelper + private DoctrineBatchHelperInterface $batchHelper, ) { $this->shortUrlRepo = $this->em->getRepository(ShortUrl::class); } diff --git a/module/Core/src/Mercure/MercureUpdatesGenerator.php b/module/Core/src/Mercure/MercureUpdatesGenerator.php index f2489da3..cc0f785a 100644 --- a/module/Core/src/Mercure/MercureUpdatesGenerator.php +++ b/module/Core/src/Mercure/MercureUpdatesGenerator.php @@ -20,7 +20,7 @@ final class MercureUpdatesGenerator implements MercureUpdatesGeneratorInterface public function __construct( private DataTransformerInterface $shortUrlTransformer, - private DataTransformerInterface $orphanVisitTransformer + private DataTransformerInterface $orphanVisitTransformer, ) { } diff --git a/module/Core/src/Model/VisitsParams.php b/module/Core/src/Model/VisitsParams.php index efe082db..ed98d4d2 100644 --- a/module/Core/src/Model/VisitsParams.php +++ b/module/Core/src/Model/VisitsParams.php @@ -21,7 +21,7 @@ final class VisitsParams ?DateRange $dateRange = null, int $page = self::FIRST_PAGE, ?int $itemsPerPage = null, - private bool $excludeBots = false + private bool $excludeBots = false, ) { $this->dateRange = $dateRange ?? DateRange::emptyInstance(); $this->page = $this->determinePage($page); diff --git a/module/Core/src/Paginator/Adapter/ShortUrlRepositoryAdapter.php b/module/Core/src/Paginator/Adapter/ShortUrlRepositoryAdapter.php index e297b6c0..93b69d33 100644 --- a/module/Core/src/Paginator/Adapter/ShortUrlRepositoryAdapter.php +++ b/module/Core/src/Paginator/Adapter/ShortUrlRepositoryAdapter.php @@ -14,7 +14,7 @@ class ShortUrlRepositoryAdapter implements AdapterInterface public function __construct( private ShortUrlRepositoryInterface $repository, private ShortUrlsParams $params, - private ?ApiKey $apiKey + private ?ApiKey $apiKey, ) { } diff --git a/module/Core/src/Paginator/Adapter/VisitsForTagPaginatorAdapter.php b/module/Core/src/Paginator/Adapter/VisitsForTagPaginatorAdapter.php index dbbf8bb9..20af1598 100644 --- a/module/Core/src/Paginator/Adapter/VisitsForTagPaginatorAdapter.php +++ b/module/Core/src/Paginator/Adapter/VisitsForTagPaginatorAdapter.php @@ -16,7 +16,7 @@ class VisitsForTagPaginatorAdapter extends AbstractCacheableCountPaginatorAdapte private VisitRepositoryInterface $visitRepository, private string $tag, private VisitsParams $params, - private ?ApiKey $apiKey + private ?ApiKey $apiKey, ) { } diff --git a/module/Core/src/Paginator/Adapter/VisitsPaginatorAdapter.php b/module/Core/src/Paginator/Adapter/VisitsPaginatorAdapter.php index fa6833f8..9ff13e3c 100644 --- a/module/Core/src/Paginator/Adapter/VisitsPaginatorAdapter.php +++ b/module/Core/src/Paginator/Adapter/VisitsPaginatorAdapter.php @@ -17,7 +17,7 @@ class VisitsPaginatorAdapter extends AbstractCacheableCountPaginatorAdapter private VisitRepositoryInterface $visitRepository, private ShortUrlIdentifier $identifier, private VisitsParams $params, - private ?Specification $spec + private ?Specification $spec, ) { } diff --git a/module/Core/src/Service/ShortUrl/DeleteShortUrlService.php b/module/Core/src/Service/ShortUrl/DeleteShortUrlService.php index 1bcd5ccb..0732b737 100644 --- a/module/Core/src/Service/ShortUrl/DeleteShortUrlService.php +++ b/module/Core/src/Service/ShortUrl/DeleteShortUrlService.php @@ -16,7 +16,7 @@ class DeleteShortUrlService implements DeleteShortUrlServiceInterface public function __construct( private EntityManagerInterface $em, private DeleteShortUrlsOptions $deleteShortUrlsOptions, - private ShortUrlResolverInterface $urlResolver + private ShortUrlResolverInterface $urlResolver, ) { } diff --git a/module/Core/src/Service/ShortUrlService.php b/module/Core/src/Service/ShortUrlService.php index 2a576ce9..f1e3bf32 100644 --- a/module/Core/src/Service/ShortUrlService.php +++ b/module/Core/src/Service/ShortUrlService.php @@ -25,7 +25,7 @@ class ShortUrlService implements ShortUrlServiceInterface private ORM\EntityManagerInterface $em, private ShortUrlResolverInterface $urlResolver, private ShortUrlTitleResolutionHelperInterface $titleResolutionHelper, - private ShortUrlRelationResolverInterface $relationResolver + private ShortUrlRelationResolverInterface $relationResolver, ) { } diff --git a/module/Core/src/Service/UrlShortener.php b/module/Core/src/Service/UrlShortener.php index 24ac2c70..41779715 100644 --- a/module/Core/src/Service/UrlShortener.php +++ b/module/Core/src/Service/UrlShortener.php @@ -20,7 +20,7 @@ class UrlShortener implements UrlShortenerInterface private ShortUrlTitleResolutionHelperInterface $titleResolutionHelper, private EntityManagerInterface $em, private ShortUrlRelationResolverInterface $relationResolver, - private ShortCodeHelperInterface $shortCodeHelper + private ShortCodeHelperInterface $shortCodeHelper, ) { } diff --git a/module/Core/src/Visit/Persistence/VisitsCountFiltering.php b/module/Core/src/Visit/Persistence/VisitsCountFiltering.php index 9f48275f..bf459768 100644 --- a/module/Core/src/Visit/Persistence/VisitsCountFiltering.php +++ b/module/Core/src/Visit/Persistence/VisitsCountFiltering.php @@ -12,7 +12,7 @@ class VisitsCountFiltering public function __construct( private ?DateRange $dateRange = null, private bool $excludeBots = false, - private ?Specification $spec = null + private ?Specification $spec = null, ) { } diff --git a/module/Core/src/Visit/Persistence/VisitsListFiltering.php b/module/Core/src/Visit/Persistence/VisitsListFiltering.php index 173e308e..fb715182 100644 --- a/module/Core/src/Visit/Persistence/VisitsListFiltering.php +++ b/module/Core/src/Visit/Persistence/VisitsListFiltering.php @@ -14,7 +14,7 @@ final class VisitsListFiltering extends VisitsCountFiltering bool $excludeBots = false, ?Specification $spec = null, private ?int $limit = null, - private ?int $offset = null + private ?int $offset = null, ) { parent::__construct($dateRange, $excludeBots, $spec); } diff --git a/module/Core/src/Visit/VisitsTracker.php b/module/Core/src/Visit/VisitsTracker.php index 523454fc..d5d0dc8e 100644 --- a/module/Core/src/Visit/VisitsTracker.php +++ b/module/Core/src/Visit/VisitsTracker.php @@ -17,7 +17,7 @@ class VisitsTracker implements VisitsTrackerInterface public function __construct( private ORM\EntityManagerInterface $em, private EventDispatcherInterface $eventDispatcher, - private TrackingOptions $options + private TrackingOptions $options, ) { } diff --git a/module/Core/test/ShortUrl/Middleware/ExtraPathRedirectMiddlewareTest.php b/module/Core/test/ShortUrl/Middleware/ExtraPathRedirectMiddlewareTest.php index 24917366..d8997524 100644 --- a/module/Core/test/ShortUrl/Middleware/ExtraPathRedirectMiddlewareTest.php +++ b/module/Core/test/ShortUrl/Middleware/ExtraPathRedirectMiddlewareTest.php @@ -65,7 +65,7 @@ class ExtraPathRedirectMiddlewareTest extends TestCase */ public function handlerIsCalledWhenConfigPreventsRedirectWithExtraPath( bool $appendExtraPath, - ServerRequestInterface $request + ServerRequestInterface $request, ): void { $this->options->appendExtraPath = $appendExtraPath; diff --git a/module/Rest/src/Action/ShortUrl/ListShortUrlsAction.php b/module/Rest/src/Action/ShortUrl/ListShortUrlsAction.php index 075b56e1..054211d4 100644 --- a/module/Rest/src/Action/ShortUrl/ListShortUrlsAction.php +++ b/module/Rest/src/Action/ShortUrl/ListShortUrlsAction.php @@ -23,7 +23,7 @@ class ListShortUrlsAction extends AbstractRestAction public function __construct( private ShortUrlServiceInterface $shortUrlService, - private DataTransformerInterface $transformer + private DataTransformerInterface $transformer, ) { } diff --git a/module/Rest/src/Action/Visit/OrphanVisitsAction.php b/module/Rest/src/Action/Visit/OrphanVisitsAction.php index b05d7b31..2632d70a 100644 --- a/module/Rest/src/Action/Visit/OrphanVisitsAction.php +++ b/module/Rest/src/Action/Visit/OrphanVisitsAction.php @@ -22,7 +22,7 @@ class OrphanVisitsAction extends AbstractRestAction public function __construct( private VisitsStatsHelperInterface $visitsHelper, - private DataTransformerInterface $orphanVisitTransformer + private DataTransformerInterface $orphanVisitTransformer, ) { } diff --git a/module/Rest/src/Middleware/AuthenticationMiddleware.php b/module/Rest/src/Middleware/AuthenticationMiddleware.php index 705bc9c5..25f1fbe5 100644 --- a/module/Rest/src/Middleware/AuthenticationMiddleware.php +++ b/module/Rest/src/Middleware/AuthenticationMiddleware.php @@ -26,7 +26,7 @@ class AuthenticationMiddleware implements MiddlewareInterface, StatusCodeInterfa public function __construct( private ApiKeyServiceInterface $apiKeyService, private array $routesWithoutApiKey, - private array $routesWithQueryApiKey + private array $routesWithQueryApiKey, ) { } From 66a4a9bce6b671859427d12a6117e70b7b7be03d Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 6 Aug 2021 13:57:39 +0200 Subject: [PATCH 06/55] Moved bugfix from Unreleased to v2.8.0, as it's already fixed there --- CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ad370d99..e53d4f98 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,7 +18,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this * *Nothing* ### Fixed -* [#1098](https://github.com/shlinkio/shlink/issues/1098) Fixed errors when using a Redis Cluster for caching, caused by `doctrine/cache` not fully supporting clusters. +* *Nothing* ## [2.8.0] - 2021-08-04 @@ -50,7 +50,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this * [#1046](https://github.com/shlinkio/shlink/issues/1046) Dropped support for PHP 7.4. ### Fixed -* *Nothing* +* [#1098](https://github.com/shlinkio/shlink/issues/1098) Fixed errors when using a Redis Cluster for caching, caused by `doctrine/cache` not fully supporting clusters. ## [2.7.3] - 2021-08-02 From c5cf116f33a3e8b0ef3d2045f3fa3f1519271079 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 6 Aug 2021 19:59:35 +0200 Subject: [PATCH 07/55] Fixed changelog message --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e53d4f98..fa5f900b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,7 +50,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this * [#1046](https://github.com/shlinkio/shlink/issues/1046) Dropped support for PHP 7.4. ### Fixed -* [#1098](https://github.com/shlinkio/shlink/issues/1098) Fixed errors when using a Redis Cluster for caching, caused by `doctrine/cache` not fully supporting clusters. +* [#1098](https://github.com/shlinkio/shlink/issues/1098) Fixed errors when using Redis for caching, caused by some third party lib bug that was fixed on dependencies update. ## [2.7.3] - 2021-08-02 From 2c5d6d1651e41b4500e5a12021871b0410a49165 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 7 Aug 2021 11:05:20 +0200 Subject: [PATCH 08/55] Moved env vars to common global config files, so that theycan be used in non-docker contexts too --- composer.json | 2 +- config/autoload/delete_short_urls.global.php | 6 +- config/autoload/entity-manager.global.php | 56 +++++-- config/autoload/geolite2.global.php | 4 +- config/autoload/locks.global.php | 9 +- config/autoload/mercure.global.php | 48 +++--- config/autoload/redirects.global.php | 8 +- config/autoload/redis.global.php | 20 +++ config/autoload/router.global.php | 4 +- config/autoload/swoole.global.php | 7 +- config/autoload/tracking.global.php | 16 +- config/autoload/url-shortener.global.php | 41 +++-- config/config.php | 5 +- docker/config/shlink_in_docker.local.php | 150 ------------------- 14 files changed, 151 insertions(+), 225 deletions(-) create mode 100644 config/autoload/redis.global.php diff --git a/composer.json b/composer.json index 47fa16d8..4e42f2cb 100644 --- a/composer.json +++ b/composer.json @@ -46,7 +46,7 @@ "predis/predis": "^1.1", "pugx/shortid-php": "^0.7", "ramsey/uuid": "^3.9", - "shlinkio/shlink-common": "dev-main#baef0ca as 4.0", + "shlinkio/shlink-common": "dev-main#3eacc46 as 4.0", "shlinkio/shlink-config": "^1.2", "shlinkio/shlink-event-dispatcher": "^2.1", "shlinkio/shlink-importer": "^2.3.1", diff --git a/config/autoload/delete_short_urls.global.php b/config/autoload/delete_short_urls.global.php index de2514a4..986b33ac 100644 --- a/config/autoload/delete_short_urls.global.php +++ b/config/autoload/delete_short_urls.global.php @@ -4,11 +4,15 @@ declare(strict_types=1); namespace Shlinkio\Shlink; +use function Shlinkio\Shlink\Common\env; + +use const Shlinkio\Shlink\Core\DEFAULT_DELETE_SHORT_URL_THRESHOLD; + return [ 'delete_short_urls' => [ - 'visits_threshold' => 15, 'check_visits_threshold' => true, + 'visits_threshold' => (int) env('DELETE_SHORT_URL_THRESHOLD', DEFAULT_DELETE_SHORT_URL_THRESHOLD), ], ]; diff --git a/config/autoload/entity-manager.global.php b/config/autoload/entity-manager.global.php index c3d2ab83..7cab2a32 100644 --- a/config/autoload/entity-manager.global.php +++ b/config/autoload/entity-manager.global.php @@ -2,24 +2,52 @@ declare(strict_types=1); -namespace Shlinkio\Shlink\Common; - use Happyr\DoctrineSpecification\Repository\EntitySpecificationRepository; -return [ +use function Functional\contains; +use function Shlinkio\Shlink\Common\env; - 'entity_manager' => [ - 'orm' => [ - 'proxies_dir' => 'data/proxies', - 'load_mappings_using_functional_style' => true, - 'default_repository_classname' => EntitySpecificationRepository::class, +return (static function (): array { + $driver = env('DB_DRIVER'); + $isMysqlCompatible = contains(['maria', 'mysql'], $driver); + + $resolveDriver = static fn () => match ($driver) { + 'postgres' => 'pdo_pgsql', + 'mssql' => 'pdo_sqlsrv', + default => 'pdo_mysql', + }; + $resolveDefaultPort = static fn () => match ($driver) { + 'postgres' => '5432', + 'mssql' => '1433', + default => '3306', + }; + $resolveConnection = static fn () => match ($driver) { + 'sqlite' => [ + 'driver' => 'pdo_sqlite', + 'path' => 'data/database.sqlite', ], - 'connection' => [ - 'user' => '', - 'password' => '', - 'dbname' => 'shlink', + default => [ + 'driver' => $resolveDriver(), + 'dbname' => env('DB_NAME', 'shlink'), + 'user' => env('DB_USER'), + 'password' => env('DB_PASSWORD'), + 'host' => env('DB_HOST', $driver === 'postgres' ? env('DB_UNIX_SOCKET') : null), + 'port' => env('DB_PORT', $resolveDefaultPort()), + 'unix_socket' => $isMysqlCompatible ? env('DB_UNIX_SOCKET') : null, 'charset' => 'utf8', ], - ], + }; -]; + return [ + + 'entity_manager' => [ + 'orm' => [ + 'proxies_dir' => 'data/proxies', + 'load_mappings_using_functional_style' => true, + 'default_repository_classname' => EntitySpecificationRepository::class, + ], + 'connection' => $resolveConnection(), + ], + + ]; +})(); diff --git a/config/autoload/geolite2.global.php b/config/autoload/geolite2.global.php index 83702ca3..3d8f0848 100644 --- a/config/autoload/geolite2.global.php +++ b/config/autoload/geolite2.global.php @@ -2,12 +2,14 @@ declare(strict_types=1); +use function Shlinkio\Shlink\Common\env; + return [ 'geolite2' => [ 'db_location' => __DIR__ . '/../../data/GeoLite2-City.mmdb', 'temp_dir' => __DIR__ . '/../../data', - 'license_key' => 'G4Lm0C60yJsnkdPi', // Deprecated. Remove hardcoded license on v3 + 'license_key' => env('GEOLITE_LICENSE_KEY', 'G4Lm0C60yJsnkdPi'), // Deprecated. Remove hardcoded license on v3 ], ]; diff --git a/config/autoload/locks.global.php b/config/autoload/locks.global.php index 26b31e15..35f4e90c 100644 --- a/config/autoload/locks.global.php +++ b/config/autoload/locks.global.php @@ -4,10 +4,11 @@ declare(strict_types=1); use Laminas\ServiceManager\AbstractFactory\ConfigAbstractFactory; use Predis\ClientInterface as PredisClient; -use Shlinkio\Shlink\Common\Lock\RetryLockStoreDelegatorFactory; use Shlinkio\Shlink\Common\Logger\LoggerAwareDelegatorFactory; use Symfony\Component\Lock; +use function Shlinkio\Shlink\Common\env; + use const Shlinkio\Shlink\Core\LOCAL_LOCK_FACTORY; return [ @@ -24,16 +25,12 @@ return [ LOCAL_LOCK_FACTORY => ConfigAbstractFactory::class, ], 'aliases' => [ - // With this config, a user could alias 'lock_store' => 'redis_lock_store' to override the default - 'lock_store' => 'local_lock_store', + 'lock_store' => env('REDIS_SERVERS') === null ? 'local_lock_store' : 'redis_lock_store', 'redis_lock_store' => Lock\Store\RedisStore::class, 'local_lock_store' => Lock\Store\FlockStore::class, ], 'delegators' => [ - Lock\Store\RedisStore::class => [ - RetryLockStoreDelegatorFactory::class, - ], Lock\LockFactory::class => [ LoggerAwareDelegatorFactory::class, ], diff --git a/config/autoload/mercure.global.php b/config/autoload/mercure.global.php index 72fafe58..aff8c6ee 100644 --- a/config/autoload/mercure.global.php +++ b/config/autoload/mercure.global.php @@ -7,30 +7,36 @@ use Shlinkio\Shlink\Common\Mercure\LcobucciJwtProvider; use Symfony\Component\Mercure\Hub; use Symfony\Component\Mercure\HubInterface; -return [ +use function Shlinkio\Shlink\Common\env; - 'mercure' => [ - 'public_hub_url' => null, - 'internal_hub_url' => null, - 'jwt_secret' => null, - 'jwt_issuer' => 'Shlink', - ], +return (static function (): array { + $publicUrl = env('MERCURE_PUBLIC_HUB_URL'); - 'dependencies' => [ - 'delegators' => [ - LcobucciJwtProvider::class => [ - LazyServiceFactory::class, + return [ + + 'mercure' => [ + 'public_hub_url' => $publicUrl, + 'internal_hub_url' => env('MERCURE_INTERNAL_HUB_URL', $publicUrl), + 'jwt_secret' => env('MERCURE_JWT_SECRET'), + 'jwt_issuer' => 'Shlink', + ], + + 'dependencies' => [ + 'delegators' => [ + LcobucciJwtProvider::class => [ + LazyServiceFactory::class, + ], + Hub::class => [ + LazyServiceFactory::class, + ], ], - Hub::class => [ - LazyServiceFactory::class, + 'lazy_services' => [ + 'class_map' => [ + LcobucciJwtProvider::class => LcobucciJwtProvider::class, + Hub::class => HubInterface::class, + ], ], ], - 'lazy_services' => [ - 'class_map' => [ - LcobucciJwtProvider::class => LcobucciJwtProvider::class, - Hub::class => HubInterface::class, - ], - ], - ], -]; + ]; +})(); diff --git a/config/autoload/redirects.global.php b/config/autoload/redirects.global.php index 173c435c..0707f1e4 100644 --- a/config/autoload/redirects.global.php +++ b/config/autoload/redirects.global.php @@ -2,12 +2,14 @@ declare(strict_types=1); +use function Shlinkio\Shlink\Common\env; + return [ 'not_found_redirects' => [ - 'invalid_short_url' => null, - 'regular_404' => null, - 'base_url' => null, + 'invalid_short_url' => env('INVALID_SHORT_URL_REDIRECT_TO'), + 'regular_404' => env('REGULAR_404_REDIRECT_TO'), + 'base_url' => env('BASE_URL_REDIRECT_TO'), ], ]; diff --git a/config/autoload/redis.global.php b/config/autoload/redis.global.php new file mode 100644 index 00000000..a47d814c --- /dev/null +++ b/config/autoload/redis.global.php @@ -0,0 +1,20 @@ + [], + default => [ + 'cache' => [ + 'redis' => [ + 'servers' => $redisServers, + ], + ], + ], + }; +})(); diff --git a/config/autoload/router.global.php b/config/autoload/router.global.php index d45ee330..a6c6d5f0 100644 --- a/config/autoload/router.global.php +++ b/config/autoload/router.global.php @@ -4,10 +4,12 @@ declare(strict_types=1); use Mezzio\Router\FastRouteRouter; +use function Shlinkio\Shlink\Common\env; + return [ 'router' => [ - 'base_path' => '', + 'base_path' => env('BASE_PATH', ''), 'fastroute' => [ FastRouteRouter::CONFIG_CACHE_ENABLED => true, diff --git a/config/autoload/swoole.global.php b/config/autoload/swoole.global.php index 29c1ea37..3db4cf5c 100644 --- a/config/autoload/swoole.global.php +++ b/config/autoload/swoole.global.php @@ -2,6 +2,8 @@ declare(strict_types=1); +use function Shlinkio\Shlink\Common\env; + return [ 'mezzio-swoole' => [ @@ -10,11 +12,12 @@ return [ 'swoole-http-server' => [ 'host' => '0.0.0.0', + 'port' => (int) env('PORT', 8080), 'process-name' => 'shlink', 'options' => [ - 'worker_num' => 16, - 'task_worker_num' => 16, + 'worker_num' => (int) env('WEB_WORKER_NUM', 16), + 'task_worker_num' => (int) env('TASK_WORKER_NUM', 16), ], ], ], diff --git a/config/autoload/tracking.global.php b/config/autoload/tracking.global.php index 4fdf0ba6..5ef0eaca 100644 --- a/config/autoload/tracking.global.php +++ b/config/autoload/tracking.global.php @@ -2,30 +2,32 @@ declare(strict_types=1); +use function Shlinkio\Shlink\Common\env; + return [ 'tracking' => [ // Tells if IP addresses should be anonymized before persisting, to fulfil data protection regulations // This applies only if IP address tracking is enabled - 'anonymize_remote_addr' => true, + 'anonymize_remote_addr' => (bool) env('ANONYMIZE_REMOTE_ADDR', true), // Tells if visits to not-found URLs should be tracked. The disable_tracking option takes precedence - 'track_orphan_visits' => true, + 'track_orphan_visits' => (bool) env('TRACK_ORPHAN_VISITS', true), // A query param that, if provided, will disable tracking of one particular visit. Always takes precedence - 'disable_track_param' => null, + 'disable_track_param' => env('DISABLE_TRACK_PARAM'), // If true, visits will not be tracked at all - 'disable_tracking' => false, + 'disable_tracking' => (bool) env('DISABLE_TRACKING', false), // If true, visits will be tracked, but neither the IP address, nor the location will be resolved - 'disable_ip_tracking' => false, + 'disable_ip_tracking' => (bool) env('DISABLE_IP_TRACKING', false), // If true, the referrer will not be tracked - 'disable_referrer_tracking' => false, + 'disable_referrer_tracking' => (bool) env('DISABLE_REFERRER_TRACKING', false), // If true, the user agent will not be tracked - 'disable_ua_tracking' => false, + 'disable_ua_tracking' => (bool) env('DISABLE_UA_TRACKING', false), ], ]; diff --git a/config/autoload/url-shortener.global.php b/config/autoload/url-shortener.global.php index 4a6afbc2..35f95ec6 100644 --- a/config/autoload/url-shortener.global.php +++ b/config/autoload/url-shortener.global.php @@ -2,26 +2,35 @@ declare(strict_types=1); +use function Shlinkio\Shlink\Common\env; + use const Shlinkio\Shlink\Core\DEFAULT_REDIRECT_CACHE_LIFETIME; use const Shlinkio\Shlink\Core\DEFAULT_REDIRECT_STATUS_CODE; use const Shlinkio\Shlink\Core\DEFAULT_SHORT_CODES_LENGTH; +use const Shlinkio\Shlink\Core\MIN_SHORT_CODES_LENGTH; -return [ +return (static function (): array { + $webhooks = env('VISITS_WEBHOOKS'); + $shortCodesLength = (int) env('DEFAULT_SHORT_CODES_LENGTH', DEFAULT_SHORT_CODES_LENGTH); + $shortCodesLength = $shortCodesLength < MIN_SHORT_CODES_LENGTH ? MIN_SHORT_CODES_LENGTH : $shortCodesLength; - 'url_shortener' => [ - 'domain' => [ - 'schema' => 'https', - 'hostname' => '', + return [ + + 'url_shortener' => [ + 'domain' => [ + 'schema' => env('SHORT_DOMAIN_SCHEMA', 'http'), + 'hostname' => env('SHORT_DOMAIN_HOST', ''), + ], + 'validate_url' => (bool) env('VALIDATE_URLS', false), // Deprecated + 'visits_webhooks' => $webhooks === null ? [] : explode(',', $webhooks), + 'default_short_codes_length' => $shortCodesLength, + 'auto_resolve_titles' => (bool) env('AUTO_RESOLVE_TITLES', false), + 'append_extra_path' => (bool) env('REDIRECT_APPEND_EXTRA_PATH', false), + + // TODO Move these two options to their own config namespace. Maybe "redirects". + 'redirect_status_code' => (int) env('REDIRECT_STATUS_CODE', DEFAULT_REDIRECT_STATUS_CODE), + 'redirect_cache_lifetime' => (int) env('REDIRECT_CACHE_LIFETIME', DEFAULT_REDIRECT_CACHE_LIFETIME), ], - 'validate_url' => false, // Deprecated - 'visits_webhooks' => [], - 'default_short_codes_length' => DEFAULT_SHORT_CODES_LENGTH, - 'auto_resolve_titles' => false, - 'append_extra_path' => false, - // TODO Move these two options to their own config namespace. Maybe "redirects". - 'redirect_status_code' => DEFAULT_REDIRECT_STATUS_CODE, - 'redirect_cache_lifetime' => DEFAULT_REDIRECT_CACHE_LIFETIME, - ], - -]; + ]; +})(); diff --git a/config/config.php b/config/config.php index 2b562874..887aa365 100644 --- a/config/config.php +++ b/config/config.php @@ -8,7 +8,7 @@ use Laminas\ConfigAggregator; use Laminas\Diactoros; use Mezzio; use Mezzio\ProblemDetails; -use Mezzio\Swoole\ConfigProvider as SwooleConfigProvider; +use Mezzio\Swoole; use function class_exists; use function Shlinkio\Shlink\Common\env; @@ -17,7 +17,7 @@ return (new ConfigAggregator\ConfigAggregator([ Mezzio\ConfigProvider::class, Mezzio\Router\ConfigProvider::class, Mezzio\Router\FastRouteRouter\ConfigProvider::class, - class_exists(SwooleConfigProvider::class) ? SwooleConfigProvider::class : new ConfigAggregator\ArrayProvider([]), + class_exists(Swoole\ConfigProvider::class) ? Swoole\ConfigProvider::class : new ConfigAggregator\ArrayProvider([]), ProblemDetails\ConfigProvider::class, Diactoros\ConfigProvider::class, Common\ConfigProvider::class, @@ -31,6 +31,7 @@ return (new ConfigAggregator\ConfigAggregator([ new ConfigAggregator\PhpFileProvider('config/autoload/{{,*.}global,{,*.}local}.php'), env('APP_ENV') === 'test' ? new ConfigAggregator\PhpFileProvider('config/test/*.global.php') + // Deprecated. When the SimplifiedConfigParser is removed, load only generated_config.php here : new ConfigAggregator\LaminasConfigProvider('config/params/{generated_config.php,*.config.{php,json}}'), ], 'data/cache/app_config.php', [ Core\Config\SimplifiedConfigParser::class, diff --git a/docker/config/shlink_in_docker.local.php b/docker/config/shlink_in_docker.local.php index d4526f50..73cc3fdc 100644 --- a/docker/config/shlink_in_docker.local.php +++ b/docker/config/shlink_in_docker.local.php @@ -7,128 +7,8 @@ namespace Shlinkio\Shlink; use Monolog\Handler\StreamHandler; use Monolog\Logger; -use function explode; -use function Functional\contains; -use function Shlinkio\Shlink\Common\env; - -use const Shlinkio\Shlink\Core\DEFAULT_DELETE_SHORT_URL_THRESHOLD; -use const Shlinkio\Shlink\Core\DEFAULT_REDIRECT_CACHE_LIFETIME; -use const Shlinkio\Shlink\Core\DEFAULT_REDIRECT_STATUS_CODE; -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', - '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 - { - $driver = env('DB_DRIVER'); - $isMysql = contains(['maria', 'mysql'], $driver); - if ($driver === null || $driver === 'sqlite') { - return [ - 'driver' => 'pdo_sqlite', - 'path' => 'data/database.sqlite', - ]; - } - - return [ - 'driver' => self::DB_DRIVERS_MAP[$driver], - 'dbname' => env('DB_NAME', 'shlink'), - 'user' => env('DB_USER'), - 'password' => env('DB_PASSWORD'), - 'host' => env('DB_HOST', $driver === 'postgres' ? env('DB_UNIX_SOCKET') : null), - 'port' => env('DB_PORT', self::DB_PORTS_MAP[$driver]), - 'unix_socket' => $isMysql ? env('DB_UNIX_SOCKET') : null, - ]; - } - - public function getNotFoundRedirectsConfig(): array - { - return [ - 'invalid_short_url' => env('INVALID_SHORT_URL_REDIRECT_TO'), - 'regular_404' => env('REGULAR_404_REDIRECT_TO'), - 'base_url' => env('BASE_URL_REDIRECT_TO'), - ]; - } - - public function getVisitsWebhooks(): array - { - $webhooks = env('VISITS_WEBHOOKS'); - return $webhooks === null ? [] : explode(',', $webhooks); - } - - public function getRedisConfig(): ?array - { - $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; - } - - public function getMercureConfig(): array - { - $publicUrl = env('MERCURE_PUBLIC_HUB_URL'); - - return [ - 'public_hub_url' => $publicUrl, - 'internal_hub_url' => env('MERCURE_INTERNAL_HUB_URL', $publicUrl), - 'jwt_secret' => env('MERCURE_JWT_SECRET'), - ]; - } -}; - return [ - 'delete_short_urls' => [ - 'check_visits_threshold' => true, - 'visits_threshold' => (int) env('DELETE_SHORT_URL_THRESHOLD', DEFAULT_DELETE_SHORT_URL_THRESHOLD), - ], - - 'entity_manager' => [ - 'connection' => $helper->getDbConfig(), - ], - - 'url_shortener' => [ - 'domain' => [ - 'schema' => env('SHORT_DOMAIN_SCHEMA', 'http'), - 'hostname' => env('SHORT_DOMAIN_HOST', ''), - ], - 'validate_url' => (bool) env('VALIDATE_URLS', false), - 'visits_webhooks' => $helper->getVisitsWebhooks(), - 'default_short_codes_length' => $helper->getDefaultShortCodesLength(), - 'auto_resolve_titles' => (bool) env('AUTO_RESOLVE_TITLES', false), - 'redirect_status_code' => (int) env('REDIRECT_STATUS_CODE', DEFAULT_REDIRECT_STATUS_CODE), - 'redirect_cache_lifetime' => (int) env('REDIRECT_CACHE_LIFETIME', DEFAULT_REDIRECT_CACHE_LIFETIME), - 'append_extra_path' => (bool) env('REDIRECT_APPEND_EXTRA_PATH', false), - ], - - 'tracking' => [ - 'anonymize_remote_addr' => (bool) env('ANONYMIZE_REMOTE_ADDR', true), - 'track_orphan_visits' => (bool) env('TRACK_ORPHAN_VISITS', true), - 'disable_track_param' => env('DISABLE_TRACK_PARAM'), - 'disable_tracking' => (bool) env('DISABLE_TRACKING', false), - 'disable_ip_tracking' => (bool) env('DISABLE_IP_TRACKING', false), - 'disable_referrer_tracking' => (bool) env('DISABLE_REFERRER_TRACKING', false), - 'disable_ua_tracking' => (bool) env('DISABLE_UA_TRACKING', false), - ], - - 'not_found_redirects' => $helper->getNotFoundRedirectsConfig(), - 'logger' => [ 'Shlink' => [ 'handlers' => [ @@ -143,34 +23,4 @@ return [ ], ], - 'dependencies' => [ - 'aliases' => env('REDIS_SERVERS') === null ? [] : [ - 'lock_store' => 'redis_lock_store', - ], - ], - - 'cache' => [ - 'redis' => $helper->getRedisConfig(), - ], - - 'router' => [ - 'base_path' => env('BASE_PATH', ''), - ], - - 'mezzio-swoole' => [ - 'swoole-http-server' => [ - 'port' => (int) env('PORT', 8080), - 'options' => [ - 'worker_num' => (int) env('WEB_WORKER_NUM', 16), - 'task_worker_num' => (int) env('TASK_WORKER_NUM', 16), - ], - ], - ], - - 'geolite2' => [ - 'license_key' => env('GEOLITE_LICENSE_KEY', 'G4Lm0C60yJsnkdPi'), // Deprecated. Remove hardcoded license on v3 - ], - - 'mercure' => $helper->getMercureConfig(), - ]; From 9b75e076b5e38b9a097f9a053689d27c3d6ce516 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 7 Aug 2021 11:08:52 +0200 Subject: [PATCH 09/55] Updated changelog --- CHANGELOG.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fa5f900b..cdf36c75 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this ## [Unreleased] ### Added -* *Nothing* +* [#1015](https://github.com/shlinkio/shlink/issues/1015) Shlink now accepts configuration via env vars even when not using docker. + + The config generated with the installing tool still has precedence over the env vars, so it cannot be combined. Either you use the tool, or use env vars. ### Changed * [#1142](https://github.com/shlinkio/shlink/issues/1142) Replaced `doctrine/cache` package with `symfony/cache`. From 92e831175fe1be539f0f4d20ca76a9fafb52fd48 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 7 Aug 2021 13:32:59 +0200 Subject: [PATCH 10/55] Ensure no DB driver config falls back to SQLite --- config/autoload/entity-manager.global.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/config/autoload/entity-manager.global.php b/config/autoload/entity-manager.global.php index 7cab2a32..08427898 100644 --- a/config/autoload/entity-manager.global.php +++ b/config/autoload/entity-manager.global.php @@ -21,8 +21,8 @@ return (static function (): array { 'mssql' => '1433', default => '3306', }; - $resolveConnection = static fn () => match ($driver) { - 'sqlite' => [ + $resolveConnection = static fn () => match (true) { + $driver === null || $driver === 'sqlite' => [ 'driver' => 'pdo_sqlite', 'path' => 'data/database.sqlite', ], From a7dd441333d0c88151932ca76b37c95983d16463 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 9 Aug 2021 22:16:12 +0200 Subject: [PATCH 11/55] Added missing double quote. Closes #1151 --- docs/swagger/paths/v2_domains_redirects.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/swagger/paths/v2_domains_redirects.json b/docs/swagger/paths/v2_domains_redirects.json index 9bf16841..d9863dcd 100644 --- a/docs/swagger/paths/v2_domains_redirects.json +++ b/docs/swagger/paths/v2_domains_redirects.json @@ -5,7 +5,7 @@ "Domains" ], "summary": "Sets domain \"not found\" redirects", - "description": "Sets the URLs that you want a visitor to get redirected to for \not found\" URLs for a specific domain", + "description": "Sets the URLs that you want a visitor to get redirected to for \"not found\" URLs for a specific domain", "security": [ { "ApiKey": [] From 80e033c91dd04d139683c4e5f81b658c61d7846d Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 14 Aug 2021 19:23:08 +0200 Subject: [PATCH 12/55] Fixed local dev config for db --- config/autoload/entity-manager.local.php.dist | 2 ++ 1 file changed, 2 insertions(+) diff --git a/config/autoload/entity-manager.local.php.dist b/config/autoload/entity-manager.local.php.dist index f3cca338..c4d2b921 100644 --- a/config/autoload/entity-manager.local.php.dist +++ b/config/autoload/entity-manager.local.php.dist @@ -10,6 +10,8 @@ return [ 'password' => 'root', 'driver' => 'pdo_mysql', 'host' => 'shlink_db', + 'dbname' => 'shlink', + 'charset' => 'utf8', ], ], From 8a46b410f60640365552f25713c1c9c5657c437c Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 16 Aug 2021 12:49:15 +0200 Subject: [PATCH 13/55] Ensured Cors middleware applies for all routes, not only rest ones --- config/autoload/middleware-pipeline.global.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/autoload/middleware-pipeline.global.php b/config/autoload/middleware-pipeline.global.php index 0466ebc5..becd3ad3 100644 --- a/config/autoload/middleware-pipeline.global.php +++ b/config/autoload/middleware-pipeline.global.php @@ -18,12 +18,12 @@ return [ 'middleware' => [ ContentLengthMiddleware::class, ErrorHandler::class, + Rest\Middleware\CrossDomainMiddleware::class, ], ], 'error-handler-rest' => [ 'path' => '/rest', 'middleware' => [ - Rest\Middleware\CrossDomainMiddleware::class, RequestIdMiddleware::class, ProblemDetails\ProblemDetailsMiddleware::class, ], From 6ee248d6560a4b8a8af6f2c4aa5c3e39d300f29b Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 16 Aug 2021 12:50:18 +0200 Subject: [PATCH 14/55] Updated changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 72fa8244..87f9f68e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this ### Changed * [#1142](https://github.com/shlinkio/shlink/issues/1142) Replaced `doctrine/cache` package with `symfony/cache`. +* [#1157](https://github.com/shlinkio/shlink/issues/1157) All routes now support CORS, not only rest ones. ### Deprecated * *Nothing* From 0f51b5b1ce98438e50c9dabd9fa341920def0298 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 26 Aug 2021 09:52:11 +0200 Subject: [PATCH 15/55] Fixed warning displayed when trying to late visits and there are no pending --- module/Core/src/Repository/VisitRepository.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/module/Core/src/Repository/VisitRepository.php b/module/Core/src/Repository/VisitRepository.php index 1e157757..0fe539af 100644 --- a/module/Core/src/Repository/VisitRepository.php +++ b/module/Core/src/Repository/VisitRepository.php @@ -70,15 +70,17 @@ class VisitRepository extends EntitySpecificationRepository implements VisitRepo $qb = (clone $originalQueryBuilder)->andWhere($qb->expr()->gt('v.id', $lastId)); $iterator = $qb->getQuery()->toIterable(); $resultsFound = false; + /** @var Visit|null $lastProcessedVisit */ + $lastProcessedVisit = null; foreach ($iterator as $key => $visit) { $resultsFound = true; + $lastProcessedVisit = $visit; yield $key => $visit; } // As the query is ordered by ID, we can take the last one every time in order to exclude the whole list - /** @var Visit|null $visit */ - $lastId = $visit?->getId() ?? $lastId; + $lastId = $lastProcessedVisit?->getId() ?? $lastId; } while ($resultsFound); } From 066cc20ee602a3054bf8544c34f36d65221041bb Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 26 Aug 2021 09:53:10 +0200 Subject: [PATCH 16/55] Updated changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 87f9f68e..1901cb29 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,7 +21,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this * *Nothing* ### Fixed -* *Nothing* +* [#1165](https://github.com/shlinkio/shlink/issues/1165) Fixed warning displayed when trying to locate visits and there are none pending. ## [2.8.1] - 2021-08-15 From 14c6ead389df61cd5694b91d0abfe3a83047923a Mon Sep 17 00:00:00 2001 From: Nick Reilingh Date: Sat, 11 Sep 2021 13:46:52 -0400 Subject: [PATCH 17/55] Dockerfile - comment misused VOLUME instructions Issuing a VOLUME instruction in a production Dockerfile requires the Docker engine to create a volume whether or not it is mapped to the host or a named volume. Neither of these paths have data that needs to be persisted for production use, so their inclusion under a typical `docker run` example forces the engine to create extraneous volumes which quickly become orphaned whenever the container is recreated. --- Dockerfile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Dockerfile b/Dockerfile index dcfb030b..ea1a6ea3 100644 --- a/Dockerfile +++ b/Dockerfile @@ -69,9 +69,9 @@ RUN ln -s /etc/shlink/bin/cli /usr/local/bin/shlink EXPOSE 8080 # Expose params config dir, since the user is expected to provide custom config from there -VOLUME /etc/shlink/config/params +#VOLUME /etc/shlink/config/params # Expose data dir to allow persistent runtime data and SQLite db -VOLUME /etc/shlink/data +#VOLUME /etc/shlink/data # Copy config specific for the image COPY docker/docker-entrypoint.sh docker-entrypoint.sh From ef3c59152f6e8de03f4c3beb0da15b0690b4a79f Mon Sep 17 00:00:00 2001 From: Nick Reilingh Date: Sat, 11 Sep 2021 16:40:09 -0400 Subject: [PATCH 18/55] Dockerfile -- remove unneeded VOLUME instructions --- Dockerfile | 5 ----- 1 file changed, 5 deletions(-) diff --git a/Dockerfile b/Dockerfile index ea1a6ea3..d2684545 100644 --- a/Dockerfile +++ b/Dockerfile @@ -68,11 +68,6 @@ RUN ln -s /etc/shlink/bin/cli /usr/local/bin/shlink # Expose default swoole port EXPOSE 8080 -# Expose params config dir, since the user is expected to provide custom config from there -#VOLUME /etc/shlink/config/params -# Expose data dir to allow persistent runtime data and SQLite db -#VOLUME /etc/shlink/data - # Copy config specific for the image COPY docker/docker-entrypoint.sh docker-entrypoint.sh COPY docker/config/shlink_in_docker.local.php config/autoload/shlink_in_docker.local.php From dc466f238b3142ad05368632b2d6ee115c0b882e Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 12 Sep 2021 08:32:24 +0200 Subject: [PATCH 19/55] Updated changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1901cb29..7d816951 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this ### Fixed * [#1165](https://github.com/shlinkio/shlink/issues/1165) Fixed warning displayed when trying to locate visits and there are none pending. +* [#1172](https://github.com/shlinkio/shlink/pull/1172) Removed unneeded explicitly defined volumes in docker image. ## [2.8.1] - 2021-08-15 From e7ec8f04894978a37f2bc8eb5d48e50854239580 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 26 Sep 2021 09:10:54 +0200 Subject: [PATCH 20/55] Deprecated SHORT_DOMAIN_* env vars with replacements --- config/autoload/url-shortener.global.php | 6 ++++-- docker/README.md | 8 ++++---- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/config/autoload/url-shortener.global.php b/config/autoload/url-shortener.global.php index 35f95ec6..5193ee1b 100644 --- a/config/autoload/url-shortener.global.php +++ b/config/autoload/url-shortener.global.php @@ -13,13 +13,15 @@ return (static function (): array { $webhooks = env('VISITS_WEBHOOKS'); $shortCodesLength = (int) env('DEFAULT_SHORT_CODES_LENGTH', DEFAULT_SHORT_CODES_LENGTH); $shortCodesLength = $shortCodesLength < MIN_SHORT_CODES_LENGTH ? MIN_SHORT_CODES_LENGTH : $shortCodesLength; + $useHttps = env('USE_HTTPS'); // Deprecated. For v3, set this to true by default, instead of null return [ 'url_shortener' => [ 'domain' => [ - 'schema' => env('SHORT_DOMAIN_SCHEMA', 'http'), - 'hostname' => env('SHORT_DOMAIN_HOST', ''), + // Deprecated SHORT_DOMAIN_* env vars + 'schema' => $useHttps !== null ? (bool) $useHttps : env('SHORT_DOMAIN_SCHEMA', 'http'), + 'hostname' => env('DEFAULT_DOMAIN', env('SHORT_DOMAIN_HOST', '')), ], 'validate_url' => (bool) env('VALIDATE_URLS', false), // Deprecated 'visits_webhooks' => $webhooks === null ? [] : explode(',', $webhooks), diff --git a/docker/README.md b/docker/README.md index 5269ebb6..9f97642c 100644 --- a/docker/README.md +++ b/docker/README.md @@ -11,8 +11,8 @@ It exposes a shlink instance served with [swoole](https://www.swoole.co.uk/), wh The most basic way to run Shlink's docker image is by providing these mandatory 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**. +* `DEFAULT_DOMAIN`: The default short domain used for this shlink instance. For example **doma.in**. +* `USE_HTTPS`: Either **true** or **false**. * `GEOLITE_LICENSE_KEY`: Your GeoLite2 license key. [Learn more](https://shlink.io/documentation/geolite-license-key/) about this. To run shlink on top of a local docker service, and using an internal SQLite database, do the following: @@ -21,8 +21,8 @@ To run shlink on top of a local docker service, and using an internal SQLite dat docker run \ --name shlink \ -p 8080:8080 \ - -e SHORT_DOMAIN_HOST=doma.in \ - -e SHORT_DOMAIN_SCHEMA=https \ + -e DEFAULT_DOMAIN=doma.in \ + -e USE_HTTPS=true \ -e GEOLITE_LICENSE_KEY=kjh23ljkbndskj345 \ shlinkio/shlink:stable ``` From c6226547f7fe7ec69b2d6d038bad286deed83e56 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 26 Sep 2021 09:12:26 +0200 Subject: [PATCH 21/55] Updated changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7d816951..c3a12e21 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,7 +15,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this * [#1157](https://github.com/shlinkio/shlink/issues/1157) All routes now support CORS, not only rest ones. ### Deprecated -* *Nothing* +* [#1164](https://github.com/shlinkio/shlink/issues/1164) Deprecated `SHORT_DOMAIN_HOST` and `SHORT_DOMAIN_SCHEMA` env vars. Use `DEFAULT_DOMAIN` and `USE_HTTPS=true|false` instead. ### Removed * *Nothing* From 7db613643667b2cf07baa64a3847faa892263c3f Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 26 Sep 2021 09:40:24 +0200 Subject: [PATCH 22/55] Simplified how the not-found redirects are resolved --- .../src/ErrorHandler/NotFoundRedirectHandler.php | 13 +++---------- .../ErrorHandler/NotFoundRedirectHandlerTest.php | 11 ++++++----- 2 files changed, 9 insertions(+), 15 deletions(-) diff --git a/module/Core/src/ErrorHandler/NotFoundRedirectHandler.php b/module/Core/src/ErrorHandler/NotFoundRedirectHandler.php index 44cd2ddd..57bc4c41 100644 --- a/module/Core/src/ErrorHandler/NotFoundRedirectHandler.php +++ b/module/Core/src/ErrorHandler/NotFoundRedirectHandler.php @@ -27,16 +27,9 @@ class NotFoundRedirectHandler implements MiddlewareInterface /** @var NotFoundType $notFoundType */ $notFoundType = $request->getAttribute(NotFoundType::class); $authority = $request->getUri()->getAuthority(); - $domainSpecificRedirect = $this->resolveDomainSpecificRedirect($authority, $notFoundType); + $redirectConfig = $this->domainService->findByAuthority($authority) ?? $this->redirectOptions; + $redirectResponse = $this->redirectResolver->resolveRedirectResponse($notFoundType, $redirectConfig); - return $domainSpecificRedirect - ?? $this->redirectResolver->resolveRedirectResponse($notFoundType, $this->redirectOptions) - ?? $handler->handle($request); - } - - private function resolveDomainSpecificRedirect(string $authority, NotFoundType $notFoundType): ?ResponseInterface - { - $domain = $this->domainService->findByAuthority($authority); - return $domain === null ? null : $this->redirectResolver->resolveRedirectResponse($notFoundType, $domain); + return $redirectResponse ?? $handler->handle($request); } } diff --git a/module/Core/test/ErrorHandler/NotFoundRedirectHandlerTest.php b/module/Core/test/ErrorHandler/NotFoundRedirectHandlerTest.php index 0d257d8e..877e96b7 100644 --- a/module/Core/test/ErrorHandler/NotFoundRedirectHandlerTest.php +++ b/module/Core/test/ErrorHandler/NotFoundRedirectHandlerTest.php @@ -72,17 +72,18 @@ class NotFoundRedirectHandlerTest extends TestCase $domainService->findByAuthority(Argument::cetera()) ->willReturn(null) ->shouldBeCalledOnce(); - $resolver->resolveRedirectResponse(Argument::cetera()) - ->willReturn(null) - ->shouldBeCalledOnce(); + $resolver->resolveRedirectResponse( + Argument::type(NotFoundType::class), + Argument::type(NotFoundRedirectOptions::class), + )->willReturn(null)->shouldBeCalledOnce(); }]; yield 'non-redirecting domain' => [function (ObjectProphecy $domainService, ObjectProphecy $resolver): void { $domainService->findByAuthority(Argument::cetera()) ->willReturn(Domain::withAuthority('')) ->shouldBeCalledOnce(); - $resolver->resolveRedirectResponse(Argument::cetera()) + $resolver->resolveRedirectResponse(Argument::type(NotFoundType::class), Argument::type(Domain::class)) ->willReturn(null) - ->shouldBeCalledTimes(2); + ->shouldBeCalledOnce(); }]; } From f5aaf298e11f5c6737f05506b972cc7dfc34585a Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 26 Sep 2021 09:49:51 +0200 Subject: [PATCH 23/55] Added experimental builds under PHP 8.1 --- .github/workflows/ci.yml | 64 +++++++++++++++++++++++++++--------- Dockerfile | 2 +- data/infra/swoole.Dockerfile | 2 +- 3 files changed, 50 insertions(+), 18 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 741f80c0..80922334 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -48,7 +48,8 @@ jobs: runs-on: ubuntu-20.04 strategy: matrix: - php-version: ['8.0'] + php-version: ['8.0', '8.1'] + continue-on-error: ${{ matrix.php-version == '8.1' }} steps: - name: Checkout code uses: actions/checkout@v2 @@ -60,7 +61,10 @@ jobs: extensions: swoole-4.6.7 coverage: pcov ini-values: pcov.directory=module - - run: composer install --no-interaction --prefer-dist + - if: ${{ matrix.php-version == '8.1' }} + run: composer install --no-interaction --prefer-dist --ignore-platform-req=php + - if: ${{ matrix.php-version != '8.1' }} + run: composer install --no-interaction --prefer-dist - run: composer test:unit:ci - uses: actions/upload-artifact@v2 if: ${{ matrix.php-version == '8.0' }} @@ -74,7 +78,8 @@ jobs: runs-on: ubuntu-20.04 strategy: matrix: - php-version: ['8.0'] + php-version: ['8.0', '8.1'] + continue-on-error: ${{ matrix.php-version == '8.1' }} steps: - name: Checkout code uses: actions/checkout@v2 @@ -86,7 +91,10 @@ jobs: extensions: swoole-4.6.7 coverage: pcov ini-values: pcov.directory=module - - run: composer install --no-interaction --prefer-dist + - if: ${{ matrix.php-version == '8.1' }} + run: composer install --no-interaction --prefer-dist --ignore-platform-req=php + - if: ${{ matrix.php-version != '8.1' }} + run: composer install --no-interaction --prefer-dist - run: composer test:db:sqlite:ci - uses: actions/upload-artifact@v2 if: ${{ matrix.php-version == '8.0' }} @@ -100,7 +108,8 @@ jobs: runs-on: ubuntu-20.04 strategy: matrix: - php-version: ['8.0'] + php-version: ['8.0', '8.1'] + continue-on-error: ${{ matrix.php-version == '8.1' }} steps: - name: Checkout code uses: actions/checkout@v2 @@ -113,14 +122,18 @@ jobs: tools: composer extensions: swoole-4.6.7 coverage: none - - run: composer install --no-interaction --prefer-dist + - if: ${{ matrix.php-version == '8.1' }} + run: composer install --no-interaction --prefer-dist --ignore-platform-req=php + - if: ${{ matrix.php-version != '8.1' }} + run: composer install --no-interaction --prefer-dist - run: composer test:db:mysql db-tests-maria: runs-on: ubuntu-20.04 strategy: matrix: - php-version: ['8.0'] + php-version: ['8.0', '8.1'] + continue-on-error: ${{ matrix.php-version == '8.1' }} steps: - name: Checkout code uses: actions/checkout@v2 @@ -133,14 +146,18 @@ jobs: tools: composer extensions: swoole-4.6.7 coverage: none - - run: composer install --no-interaction --prefer-dist + - if: ${{ matrix.php-version == '8.1' }} + run: composer install --no-interaction --prefer-dist --ignore-platform-req=php + - if: ${{ matrix.php-version != '8.1' }} + run: composer install --no-interaction --prefer-dist - run: composer test:db:maria db-tests-postgres: runs-on: ubuntu-20.04 strategy: matrix: - php-version: ['8.0'] + php-version: ['8.0', '8.1'] + continue-on-error: ${{ matrix.php-version == '8.1' }} steps: - name: Checkout code uses: actions/checkout@v2 @@ -153,14 +170,18 @@ jobs: tools: composer extensions: swoole-4.6.7 coverage: none - - run: composer install --no-interaction --prefer-dist + - if: ${{ matrix.php-version == '8.1' }} + run: composer install --no-interaction --prefer-dist --ignore-platform-req=php + - if: ${{ matrix.php-version != '8.1' }} + run: composer install --no-interaction --prefer-dist - run: composer test:db:postgres db-tests-ms: runs-on: ubuntu-20.04 strategy: matrix: - php-version: ['8.0'] + php-version: ['8.0', '8.1'] + continue-on-error: ${{ matrix.php-version == '8.1' }} steps: - name: Checkout code uses: actions/checkout@v2 @@ -175,7 +196,10 @@ jobs: tools: composer extensions: swoole-4.6.7, pdo_sqlsrv-5.9.0 coverage: none - - run: composer install --no-interaction --prefer-dist + - if: ${{ matrix.php-version == '8.1' }} + run: composer install --no-interaction --prefer-dist --ignore-platform-req=php + - if: ${{ matrix.php-version != '8.1' }} + run: composer install --no-interaction --prefer-dist - name: Create test database run: docker-compose exec -T shlink_db_ms /opt/mssql-tools/bin/sqlcmd -S localhost -U sa -P 'Passw0rd!' -Q "CREATE DATABASE shlink_test;" - run: composer test:db:ms @@ -184,7 +208,8 @@ jobs: runs-on: ubuntu-20.04 strategy: matrix: - php-version: ['8.0'] + php-version: ['8.0', '8.1'] + continue-on-error: ${{ matrix.php-version == '8.1' }} steps: - name: Checkout code uses: actions/checkout@v2 @@ -198,7 +223,10 @@ jobs: extensions: swoole-4.6.7 coverage: pcov ini-values: pcov.directory=module - - run: composer install --no-interaction --prefer-dist + - if: ${{ matrix.php-version == '8.1' }} + run: composer install --no-interaction --prefer-dist --ignore-platform-req=php + - if: ${{ matrix.php-version != '8.1' }} + run: composer install --no-interaction --prefer-dist - run: bin/test/run-api-tests.sh - uses: actions/upload-artifact@v2 if: ${{ matrix.php-version == '8.0' }} @@ -216,8 +244,9 @@ jobs: runs-on: ubuntu-20.04 strategy: matrix: - php-version: ['8.0'] + php-version: ['8.0', '8.1'] test-group: ['unit', 'db'] + continue-on-error: ${{ matrix.php-version == '8.1' }} steps: - name: Checkout code uses: actions/checkout@v2 @@ -229,7 +258,10 @@ jobs: extensions: swoole-4.6.7 coverage: pcov ini-values: pcov.directory=module - - run: composer install --no-interaction --prefer-dist + - if: ${{ matrix.php-version == '8.1' }} + run: composer install --no-interaction --prefer-dist --ignore-platform-req=php + - if: ${{ matrix.php-version != '8.1' }} + run: composer install --no-interaction --prefer-dist - uses: actions/download-artifact@v2 with: path: build diff --git a/Dockerfile b/Dockerfile index d2684545..afb9c865 100644 --- a/Dockerfile +++ b/Dockerfile @@ -2,7 +2,7 @@ FROM php:8.0.9-alpine3.14 as base ARG SHLINK_VERSION=latest ENV SHLINK_VERSION ${SHLINK_VERSION} -ENV SWOOLE_VERSION 4.7.0 +ENV SWOOLE_VERSION 4.7.1 ENV PDO_SQLSRV_VERSION 5.9.0 ENV MS_ODBC_SQL_VERSION 17.5.2.1 ENV LC_ALL "C" diff --git a/data/infra/swoole.Dockerfile b/data/infra/swoole.Dockerfile index 3170729b..0e3fe17f 100644 --- a/data/infra/swoole.Dockerfile +++ b/data/infra/swoole.Dockerfile @@ -4,7 +4,7 @@ MAINTAINER Alejandro Celaya ENV APCU_VERSION 5.1.20 ENV PDO_SQLSRV_VERSION 5.9.0 ENV INOTIFY_VERSION 3.0.0 -ENV SWOOLE_VERSION 4.7.0 +ENV SWOOLE_VERSION 4.7.1 ENV MS_ODBC_SQL_VERSION 17.5.2.1 RUN apk update From 633e389275f65c9eaa0b25f8caeb270ccf839659 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 26 Sep 2021 09:50:35 +0200 Subject: [PATCH 24/55] Updated changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c3a12e21..46f386eb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this ### Changed * [#1142](https://github.com/shlinkio/shlink/issues/1142) Replaced `doctrine/cache` package with `symfony/cache`. * [#1157](https://github.com/shlinkio/shlink/issues/1157) All routes now support CORS, not only rest ones. +* [#1144](https://github.com/shlinkio/shlink/issues/1144) Added experimental builds under PHP 8.1. ### Deprecated * [#1164](https://github.com/shlinkio/shlink/issues/1164) Deprecated `SHORT_DOMAIN_HOST` and `SHORT_DOMAIN_SCHEMA` env vars. Use `DEFAULT_DOMAIN` and `USE_HTTPS=true|false` instead. From c2cd21c15ed4019068e3ed38514c00066e968115 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 26 Sep 2021 09:53:58 +0200 Subject: [PATCH 25/55] Updated swoole version used in CI workflow --- .github/workflows/ci.yml | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 80922334..e29a0a2c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -21,7 +21,7 @@ jobs: with: php-version: ${{ matrix.php-version }} tools: composer - extensions: swoole-4.6.7 + extensions: swoole-4.7.1 coverage: none - run: composer install --no-interaction --prefer-dist - run: composer cs @@ -39,7 +39,7 @@ jobs: with: php-version: ${{ matrix.php-version }} tools: composer - extensions: swoole-4.6.7 + extensions: swoole-4.7.1 coverage: none - run: composer install --no-interaction --prefer-dist - run: composer stan @@ -58,7 +58,7 @@ jobs: with: php-version: ${{ matrix.php-version }} tools: composer - extensions: swoole-4.6.7 + extensions: swoole-4.7.1 coverage: pcov ini-values: pcov.directory=module - if: ${{ matrix.php-version == '8.1' }} @@ -88,7 +88,7 @@ jobs: with: php-version: ${{ matrix.php-version }} tools: composer - extensions: swoole-4.6.7 + extensions: swoole-4.7.1 coverage: pcov ini-values: pcov.directory=module - if: ${{ matrix.php-version == '8.1' }} @@ -120,7 +120,7 @@ jobs: with: php-version: ${{ matrix.php-version }} tools: composer - extensions: swoole-4.6.7 + extensions: swoole-4.7.1 coverage: none - if: ${{ matrix.php-version == '8.1' }} run: composer install --no-interaction --prefer-dist --ignore-platform-req=php @@ -144,7 +144,7 @@ jobs: with: php-version: ${{ matrix.php-version }} tools: composer - extensions: swoole-4.6.7 + extensions: swoole-4.7.1 coverage: none - if: ${{ matrix.php-version == '8.1' }} run: composer install --no-interaction --prefer-dist --ignore-platform-req=php @@ -168,7 +168,7 @@ jobs: with: php-version: ${{ matrix.php-version }} tools: composer - extensions: swoole-4.6.7 + extensions: swoole-4.7.1 coverage: none - if: ${{ matrix.php-version == '8.1' }} run: composer install --no-interaction --prefer-dist --ignore-platform-req=php @@ -194,7 +194,7 @@ jobs: with: php-version: ${{ matrix.php-version }} tools: composer - extensions: swoole-4.6.7, pdo_sqlsrv-5.9.0 + extensions: swoole-4.7.1, pdo_sqlsrv-5.9.0 coverage: none - if: ${{ matrix.php-version == '8.1' }} run: composer install --no-interaction --prefer-dist --ignore-platform-req=php @@ -220,7 +220,7 @@ jobs: with: php-version: ${{ matrix.php-version }} tools: composer - extensions: swoole-4.6.7 + extensions: swoole-4.7.1 coverage: pcov ini-values: pcov.directory=module - if: ${{ matrix.php-version == '8.1' }} @@ -255,7 +255,7 @@ jobs: with: php-version: ${{ matrix.php-version }} tools: composer - extensions: swoole-4.6.7 + extensions: swoole-4.7.1 coverage: pcov ini-values: pcov.directory=module - if: ${{ matrix.php-version == '8.1' }} From f5beec70c807db4c777ead33418c7e98c5c645cc Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 26 Sep 2021 10:03:07 +0200 Subject: [PATCH 26/55] Updated MS native deps --- Dockerfile | 4 ++-- data/infra/php.Dockerfile | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Dockerfile b/Dockerfile index afb9c865..fa31a9af 100644 --- a/Dockerfile +++ b/Dockerfile @@ -3,8 +3,8 @@ FROM php:8.0.9-alpine3.14 as base ARG SHLINK_VERSION=latest ENV SHLINK_VERSION ${SHLINK_VERSION} ENV SWOOLE_VERSION 4.7.1 -ENV PDO_SQLSRV_VERSION 5.9.0 -ENV MS_ODBC_SQL_VERSION 17.5.2.1 +ENV PDO_SQLSRV_VERSION 5.10.0beta1 +ENV MS_ODBC_SQL_VERSION 17.5.2.2 ENV LC_ALL "C" WORKDIR /etc/shlink diff --git a/data/infra/php.Dockerfile b/data/infra/php.Dockerfile index 02e815b1..ef50db50 100644 --- a/data/infra/php.Dockerfile +++ b/data/infra/php.Dockerfile @@ -2,8 +2,8 @@ FROM php:8.0.9-fpm-alpine3.14 MAINTAINER Alejandro Celaya ENV APCU_VERSION 5.1.20 -ENV PDO_SQLSRV_VERSION 5.9.0 -ENV MS_ODBC_SQL_VERSION 17.5.2.1 +ENV PDO_SQLSRV_VERSION 5.10.0beta1 +ENV MS_ODBC_SQL_VERSION 17.5.2.2 RUN apk update From 3305f4c03ae95d52ce52286e3c48f68443affdd5 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 26 Sep 2021 10:04:50 +0200 Subject: [PATCH 27/55] Updated pdo_sqlsrv version used in CI workflow --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e29a0a2c..c4939367 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -194,7 +194,7 @@ jobs: with: php-version: ${{ matrix.php-version }} tools: composer - extensions: swoole-4.7.1, pdo_sqlsrv-5.9.0 + extensions: swoole-4.7.1, pdo_sqlsrv-5.10.0beta1 coverage: none - if: ${{ matrix.php-version == '8.1' }} run: composer install --no-interaction --prefer-dist --ignore-platform-req=php From 42dbeaa1a5fa7b5ec4f90daca467d816e49d2809 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 26 Sep 2021 10:06:35 +0200 Subject: [PATCH 28/55] Updated MS native deps in swoole dev container --- data/infra/swoole.Dockerfile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/data/infra/swoole.Dockerfile b/data/infra/swoole.Dockerfile index 0e3fe17f..54635153 100644 --- a/data/infra/swoole.Dockerfile +++ b/data/infra/swoole.Dockerfile @@ -2,10 +2,10 @@ FROM php:8.0.9-alpine3.14 MAINTAINER Alejandro Celaya ENV APCU_VERSION 5.1.20 -ENV PDO_SQLSRV_VERSION 5.9.0 ENV INOTIFY_VERSION 3.0.0 ENV SWOOLE_VERSION 4.7.1 -ENV MS_ODBC_SQL_VERSION 17.5.2.1 +ENV PDO_SQLSRV_VERSION 5.10.0beta1 +ENV MS_ODBC_SQL_VERSION 17.5.2.2 RUN apk update From fb26a8ae50f808dd2131e311b5e54c09fff9aaac Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 26 Sep 2021 10:19:26 +0200 Subject: [PATCH 29/55] Downgraded pdo_sqlsrv version for PHP 8.0 --- .github/workflows/ci.yml | 9 +++++++++ Dockerfile | 2 +- data/infra/php.Dockerfile | 2 +- data/infra/swoole.Dockerfile | 2 +- 4 files changed, 12 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c4939367..00ae8550 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -190,12 +190,21 @@ jobs: - name: Start database server run: docker-compose -f docker-compose.yml -f docker-compose.ci.yml up -d shlink_db_ms - name: Use PHP + if: ${{ matrix.php-version == '8.1' }} uses: shivammathur/setup-php@v2 with: php-version: ${{ matrix.php-version }} tools: composer extensions: swoole-4.7.1, pdo_sqlsrv-5.10.0beta1 coverage: none + - name: Use PHP + if: ${{ matrix.php-version == '8.1' }} + uses: shivammathur/setup-php@v2 + with: + php-version: ${{ matrix.php-version }} + tools: composer + extensions: swoole-4.7.1, pdo_sqlsrv-5.9.0 + coverage: none - if: ${{ matrix.php-version == '8.1' }} run: composer install --no-interaction --prefer-dist --ignore-platform-req=php - if: ${{ matrix.php-version != '8.1' }} diff --git a/Dockerfile b/Dockerfile index fa31a9af..aea95a86 100644 --- a/Dockerfile +++ b/Dockerfile @@ -3,7 +3,7 @@ FROM php:8.0.9-alpine3.14 as base ARG SHLINK_VERSION=latest ENV SHLINK_VERSION ${SHLINK_VERSION} ENV SWOOLE_VERSION 4.7.1 -ENV PDO_SQLSRV_VERSION 5.10.0beta1 +ENV PDO_SQLSRV_VERSION 5.9.0 ENV MS_ODBC_SQL_VERSION 17.5.2.2 ENV LC_ALL "C" diff --git a/data/infra/php.Dockerfile b/data/infra/php.Dockerfile index ef50db50..1503ddf2 100644 --- a/data/infra/php.Dockerfile +++ b/data/infra/php.Dockerfile @@ -2,7 +2,7 @@ FROM php:8.0.9-fpm-alpine3.14 MAINTAINER Alejandro Celaya ENV APCU_VERSION 5.1.20 -ENV PDO_SQLSRV_VERSION 5.10.0beta1 +ENV PDO_SQLSRV_VERSION 5.9.0 ENV MS_ODBC_SQL_VERSION 17.5.2.2 RUN apk update diff --git a/data/infra/swoole.Dockerfile b/data/infra/swoole.Dockerfile index 54635153..5b4fac1c 100644 --- a/data/infra/swoole.Dockerfile +++ b/data/infra/swoole.Dockerfile @@ -4,7 +4,7 @@ MAINTAINER Alejandro Celaya ENV APCU_VERSION 5.1.20 ENV INOTIFY_VERSION 3.0.0 ENV SWOOLE_VERSION 4.7.1 -ENV PDO_SQLSRV_VERSION 5.10.0beta1 +ENV PDO_SQLSRV_VERSION 5.9.0 ENV MS_ODBC_SQL_VERSION 17.5.2.2 RUN apk update From 1f8fcdb0f3c3e5a0b46dc242cd6bf9f5aa7fdb06 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 26 Sep 2021 10:20:09 +0200 Subject: [PATCH 30/55] Fixed typo in ci workflow --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 00ae8550..9d2067da 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -198,7 +198,7 @@ jobs: extensions: swoole-4.7.1, pdo_sqlsrv-5.10.0beta1 coverage: none - name: Use PHP - if: ${{ matrix.php-version == '8.1' }} + if: ${{ matrix.php-version != '8.1' }} uses: shivammathur/setup-php@v2 with: php-version: ${{ matrix.php-version }} From c39e1e649d33bd9a07e0cf78311b1bc33694e684 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 26 Sep 2021 11:10:00 +0200 Subject: [PATCH 31/55] Reolled-back logic that would have made domains with no specific redirects to not fall back to the default redirects --- .../src/ErrorHandler/NotFoundRedirectHandler.php | 15 ++++++++++++--- .../ErrorHandler/NotFoundRedirectHandlerTest.php | 4 ++++ 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/module/Core/src/ErrorHandler/NotFoundRedirectHandler.php b/module/Core/src/ErrorHandler/NotFoundRedirectHandler.php index 57bc4c41..84918876 100644 --- a/module/Core/src/ErrorHandler/NotFoundRedirectHandler.php +++ b/module/Core/src/ErrorHandler/NotFoundRedirectHandler.php @@ -27,9 +27,18 @@ class NotFoundRedirectHandler implements MiddlewareInterface /** @var NotFoundType $notFoundType */ $notFoundType = $request->getAttribute(NotFoundType::class); $authority = $request->getUri()->getAuthority(); - $redirectConfig = $this->domainService->findByAuthority($authority) ?? $this->redirectOptions; - $redirectResponse = $this->redirectResolver->resolveRedirectResponse($notFoundType, $redirectConfig); + $domainSpecificRedirect = $this->resolveDomainSpecificRedirect($authority, $notFoundType); - return $redirectResponse ?? $handler->handle($request); + return $domainSpecificRedirect + // If we did not find domain-specific redirects for current domain, we try to fall back to default redirects + ?? $this->redirectResolver->resolveRedirectResponse($notFoundType, $this->redirectOptions) + // Ultimately, we just call next handler if no domain-specific redirects or default redirects were found + ?? $handler->handle($request); + } + + private function resolveDomainSpecificRedirect(string $authority, NotFoundType $notFoundType): ?ResponseInterface + { + $domain = $this->domainService->findByAuthority($authority); + return $domain === null ? null : $this->redirectResolver->resolveRedirectResponse($notFoundType, $domain); } } diff --git a/module/Core/test/ErrorHandler/NotFoundRedirectHandlerTest.php b/module/Core/test/ErrorHandler/NotFoundRedirectHandlerTest.php index 877e96b7..e508a87b 100644 --- a/module/Core/test/ErrorHandler/NotFoundRedirectHandlerTest.php +++ b/module/Core/test/ErrorHandler/NotFoundRedirectHandlerTest.php @@ -81,6 +81,10 @@ class NotFoundRedirectHandlerTest extends TestCase $domainService->findByAuthority(Argument::cetera()) ->willReturn(Domain::withAuthority('')) ->shouldBeCalledOnce(); + $resolver->resolveRedirectResponse( + Argument::type(NotFoundType::class), + Argument::type(NotFoundRedirectOptions::class), + )->willReturn(null)->shouldBeCalledOnce(); $resolver->resolveRedirectResponse(Argument::type(NotFoundType::class), Argument::type(Domain::class)) ->willReturn(null) ->shouldBeCalledOnce(); From cbec4a4e816510901f17960c4cf4664282e0aa7d Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 26 Sep 2021 11:20:29 +0200 Subject: [PATCH 32/55] Moved constants to its own file inside config folder --- composer.json | 1 + config/autoload/delete_short_urls.global.php | 2 +- config/autoload/locks.global.php | 2 +- config/autoload/url-shortener.global.php | 8 ++++---- config/constants.php | 16 ++++++++++++++++ config/container.php | 2 +- module/CLI/config/dependencies.config.php | 2 +- module/Core/functions/functions.php | 10 ---------- module/Core/src/Model/ShortUrlMeta.php | 2 +- .../Core/src/Options/DeleteShortUrlsOptions.php | 2 +- module/Core/src/Options/UrlShortenerOptions.php | 4 ++-- module/Core/src/Util/UrlValidator.php | 2 +- .../Core/src/Validation/ShortUrlInputFilter.php | 4 ++-- module/Core/test/Entity/ShortUrlTest.php | 2 +- 14 files changed, 33 insertions(+), 26 deletions(-) create mode 100644 config/constants.php diff --git a/composer.json b/composer.json index 4e42f2cb..b69306cf 100644 --- a/composer.json +++ b/composer.json @@ -83,6 +83,7 @@ "Shlinkio\\Shlink\\Core\\": "module/Core/src" }, "files": [ + "config/constants.php", "module/Core/functions/functions.php" ] }, diff --git a/config/autoload/delete_short_urls.global.php b/config/autoload/delete_short_urls.global.php index 986b33ac..7f64abd7 100644 --- a/config/autoload/delete_short_urls.global.php +++ b/config/autoload/delete_short_urls.global.php @@ -6,7 +6,7 @@ namespace Shlinkio\Shlink; use function Shlinkio\Shlink\Common\env; -use const Shlinkio\Shlink\Core\DEFAULT_DELETE_SHORT_URL_THRESHOLD; +use const Shlinkio\Shlink\DEFAULT_DELETE_SHORT_URL_THRESHOLD; return [ diff --git a/config/autoload/locks.global.php b/config/autoload/locks.global.php index 35f4e90c..60054147 100644 --- a/config/autoload/locks.global.php +++ b/config/autoload/locks.global.php @@ -9,7 +9,7 @@ use Symfony\Component\Lock; use function Shlinkio\Shlink\Common\env; -use const Shlinkio\Shlink\Core\LOCAL_LOCK_FACTORY; +use const Shlinkio\Shlink\LOCAL_LOCK_FACTORY; return [ diff --git a/config/autoload/url-shortener.global.php b/config/autoload/url-shortener.global.php index 5193ee1b..432c63e1 100644 --- a/config/autoload/url-shortener.global.php +++ b/config/autoload/url-shortener.global.php @@ -4,10 +4,10 @@ declare(strict_types=1); use function Shlinkio\Shlink\Common\env; -use const Shlinkio\Shlink\Core\DEFAULT_REDIRECT_CACHE_LIFETIME; -use const Shlinkio\Shlink\Core\DEFAULT_REDIRECT_STATUS_CODE; -use const Shlinkio\Shlink\Core\DEFAULT_SHORT_CODES_LENGTH; -use const Shlinkio\Shlink\Core\MIN_SHORT_CODES_LENGTH; +use const Shlinkio\Shlink\DEFAULT_REDIRECT_CACHE_LIFETIME; +use const Shlinkio\Shlink\DEFAULT_REDIRECT_STATUS_CODE; +use const Shlinkio\Shlink\DEFAULT_SHORT_CODES_LENGTH; +use const Shlinkio\Shlink\MIN_SHORT_CODES_LENGTH; return (static function (): array { $webhooks = env('VISITS_WEBHOOKS'); diff --git a/config/constants.php b/config/constants.php new file mode 100644 index 00000000..5a270e3c --- /dev/null +++ b/config/constants.php @@ -0,0 +1,16 @@ +]*>(.*?)<\/title>/i'; // Matches the value inside an html title tag diff --git a/config/container.php b/config/container.php index 7b6f0b08..56fb345d 100644 --- a/config/container.php +++ b/config/container.php @@ -5,7 +5,7 @@ declare(strict_types=1); use Laminas\ServiceManager\ServiceManager; use Symfony\Component\Lock; -use const Shlinkio\Shlink\Core\LOCAL_LOCK_FACTORY; +use const Shlinkio\Shlink\LOCAL_LOCK_FACTORY; chdir(dirname(__DIR__)); diff --git a/module/CLI/config/dependencies.config.php b/module/CLI/config/dependencies.config.php index 95ea1bbc..d89a8af2 100644 --- a/module/CLI/config/dependencies.config.php +++ b/module/CLI/config/dependencies.config.php @@ -24,7 +24,7 @@ use Symfony\Component\Console as SymfonyCli; use Symfony\Component\Lock\LockFactory; use Symfony\Component\Process\PhpExecutableFinder; -use const Shlinkio\Shlink\Core\LOCAL_LOCK_FACTORY; +use const Shlinkio\Shlink\LOCAL_LOCK_FACTORY; return [ diff --git a/module/Core/functions/functions.php b/module/Core/functions/functions.php index 9ba4bdcc..bba0c17b 100644 --- a/module/Core/functions/functions.php +++ b/module/Core/functions/functions.php @@ -6,7 +6,6 @@ namespace Shlinkio\Shlink\Core; use Cake\Chronos\Chronos; use DateTimeInterface; -use Fig\Http\Message\StatusCodeInterface; use Jaybizzle\CrawlerDetect\CrawlerDetect; use Laminas\InputFilter\InputFilter; use PUGX\Shortid\Factory as ShortIdFactory; @@ -22,15 +21,6 @@ use function str_repeat; use function str_replace; use function ucwords; -const DEFAULT_DELETE_SHORT_URL_THRESHOLD = 15; -const DEFAULT_SHORT_CODES_LENGTH = 5; -const MIN_SHORT_CODES_LENGTH = 4; -const DEFAULT_REDIRECT_STATUS_CODE = StatusCodeInterface::STATUS_FOUND; -const DEFAULT_REDIRECT_CACHE_LIFETIME = 30; -const LOCAL_LOCK_FACTORY = 'Shlinkio\Shlink\LocalLockFactory'; -const CUSTOM_SLUGS_REGEXP = '/[^\pL\pN._~]/u'; // Any unicode letter or number, plus ".", "_" and "~" chars -const TITLE_TAG_VALUE = '/]*>(.*?)<\/title>/i'; // Matches the value inside an html title tag - function generateRandomShortCode(int $length): string { static $shortIdFactory; diff --git a/module/Core/src/Model/ShortUrlMeta.php b/module/Core/src/Model/ShortUrlMeta.php index 06e0eee7..0c982f17 100644 --- a/module/Core/src/Model/ShortUrlMeta.php +++ b/module/Core/src/Model/ShortUrlMeta.php @@ -14,7 +14,7 @@ use function Shlinkio\Shlink\Core\getOptionalBoolFromInputFilter; use function Shlinkio\Shlink\Core\getOptionalIntFromInputFilter; use function Shlinkio\Shlink\Core\parseDateField; -use const Shlinkio\Shlink\Core\DEFAULT_SHORT_CODES_LENGTH; +use const Shlinkio\Shlink\DEFAULT_SHORT_CODES_LENGTH; final class ShortUrlMeta implements TitleResolutionModelInterface { diff --git a/module/Core/src/Options/DeleteShortUrlsOptions.php b/module/Core/src/Options/DeleteShortUrlsOptions.php index 9fa7fcf0..ff1c356a 100644 --- a/module/Core/src/Options/DeleteShortUrlsOptions.php +++ b/module/Core/src/Options/DeleteShortUrlsOptions.php @@ -6,7 +6,7 @@ namespace Shlinkio\Shlink\Core\Options; use Laminas\Stdlib\AbstractOptions; -use const Shlinkio\Shlink\Core\DEFAULT_DELETE_SHORT_URL_THRESHOLD; +use const Shlinkio\Shlink\DEFAULT_DELETE_SHORT_URL_THRESHOLD; class DeleteShortUrlsOptions extends AbstractOptions { diff --git a/module/Core/src/Options/UrlShortenerOptions.php b/module/Core/src/Options/UrlShortenerOptions.php index 31ecc137..f760220e 100644 --- a/module/Core/src/Options/UrlShortenerOptions.php +++ b/module/Core/src/Options/UrlShortenerOptions.php @@ -8,8 +8,8 @@ use Laminas\Stdlib\AbstractOptions; use function Functional\contains; -use const Shlinkio\Shlink\Core\DEFAULT_REDIRECT_CACHE_LIFETIME; -use const Shlinkio\Shlink\Core\DEFAULT_REDIRECT_STATUS_CODE; +use const Shlinkio\Shlink\DEFAULT_REDIRECT_CACHE_LIFETIME; +use const Shlinkio\Shlink\DEFAULT_REDIRECT_STATUS_CODE; class UrlShortenerOptions extends AbstractOptions { diff --git a/module/Core/src/Util/UrlValidator.php b/module/Core/src/Util/UrlValidator.php index 0756f55e..5b84c5f8 100644 --- a/module/Core/src/Util/UrlValidator.php +++ b/module/Core/src/Util/UrlValidator.php @@ -15,7 +15,7 @@ use Shlinkio\Shlink\Core\Options\UrlShortenerOptions; use function preg_match; use function trim; -use const Shlinkio\Shlink\Core\TITLE_TAG_VALUE; +use const Shlinkio\Shlink\TITLE_TAG_VALUE; class UrlValidator implements UrlValidatorInterface, RequestMethodInterface { diff --git a/module/Core/src/Validation/ShortUrlInputFilter.php b/module/Core/src/Validation/ShortUrlInputFilter.php index b969d95e..fd5a92e7 100644 --- a/module/Core/src/Validation/ShortUrlInputFilter.php +++ b/module/Core/src/Validation/ShortUrlInputFilter.php @@ -13,8 +13,8 @@ use Shlinkio\Shlink\Common\Validation; use Shlinkio\Shlink\Core\Util\CocurSymfonySluggerBridge; use Shlinkio\Shlink\Rest\Entity\ApiKey; -use const Shlinkio\Shlink\Core\CUSTOM_SLUGS_REGEXP; -use const Shlinkio\Shlink\Core\MIN_SHORT_CODES_LENGTH; +use const Shlinkio\Shlink\CUSTOM_SLUGS_REGEXP; +use const Shlinkio\Shlink\MIN_SHORT_CODES_LENGTH; class ShortUrlInputFilter extends InputFilter { diff --git a/module/Core/test/Entity/ShortUrlTest.php b/module/Core/test/Entity/ShortUrlTest.php index 89ccc805..d41357cd 100644 --- a/module/Core/test/Entity/ShortUrlTest.php +++ b/module/Core/test/Entity/ShortUrlTest.php @@ -16,7 +16,7 @@ use function Functional\map; use function range; use function strlen; -use const Shlinkio\Shlink\Core\DEFAULT_SHORT_CODES_LENGTH; +use const Shlinkio\Shlink\DEFAULT_SHORT_CODES_LENGTH; class ShortUrlTest extends TestCase { From 6a1ee2b894a657eb49b50d3d14afe9c145d55e35 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 26 Sep 2021 13:25:02 +0200 Subject: [PATCH 33/55] Added new config to set custom defaults for QR codes --- config/autoload/qr-codes.global.php | 21 ++++++ config/constants.php | 4 ++ module/Core/config/dependencies.config.php | 3 + module/Core/src/Action/Model/QrCodeParams.php | 41 ++++++----- module/Core/src/Action/QrCodeAction.php | 4 +- module/Core/src/Options/QrCodeOptions.php | 60 ++++++++++++++++ module/Core/test/Action/QrCodeActionTest.php | 72 +++++++++++++++---- 7 files changed, 174 insertions(+), 31 deletions(-) create mode 100644 config/autoload/qr-codes.global.php create mode 100644 module/Core/src/Options/QrCodeOptions.php diff --git a/config/autoload/qr-codes.global.php b/config/autoload/qr-codes.global.php new file mode 100644 index 00000000..1cf6fecb --- /dev/null +++ b/config/autoload/qr-codes.global.php @@ -0,0 +1,21 @@ + [ + 'size' => (int) env('DEFAULT_QR_CODE_SIZE', DEFAULT_QR_CODE_SIZE), + 'margin' => (int) env('DEFAULT_QR_CODE_MARGIN', DEFAULT_QR_CODE_MARGIN), + 'format' => env('DEFAULT_QR_CODE_FORMAT', DEFAULT_QR_CODE_FORMAT), + 'error_correction' => env('DEFAULT_QR_CODE_ERROR_CORRECTION', DEFAULT_QR_CODE_ERROR_CORRECTION), + ], + +]; diff --git a/config/constants.php b/config/constants.php index 5a270e3c..43de270a 100644 --- a/config/constants.php +++ b/config/constants.php @@ -14,3 +14,7 @@ const DEFAULT_REDIRECT_CACHE_LIFETIME = 30; const LOCAL_LOCK_FACTORY = 'Shlinkio\Shlink\LocalLockFactory'; const CUSTOM_SLUGS_REGEXP = '/[^\pL\pN._~]/u'; // Any unicode letter or number, plus ".", "_" and "~" chars const TITLE_TAG_VALUE = '/]*>(.*?)<\/title>/i'; // Matches the value inside an html title tag +const DEFAULT_QR_CODE_SIZE = 300; +const DEFAULT_QR_CODE_MARGIN = 0; +const DEFAULT_QR_CODE_FORMAT = 'png'; +const DEFAULT_QR_CODE_ERROR_CORRECTION = 'l'; diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index 7f28b14d..66d854c3 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -25,6 +25,7 @@ return [ Options\NotFoundRedirectOptions::class => ConfigAbstractFactory::class, Options\UrlShortenerOptions::class => ConfigAbstractFactory::class, Options\TrackingOptions::class => ConfigAbstractFactory::class, + Options\QrCodeOptions::class => ConfigAbstractFactory::class, Service\UrlShortener::class => ConfigAbstractFactory::class, Service\ShortUrlService::class => ConfigAbstractFactory::class, @@ -86,6 +87,7 @@ return [ Options\NotFoundRedirectOptions::class => ['config.not_found_redirects'], Options\UrlShortenerOptions::class => ['config.url_shortener'], Options\TrackingOptions::class => ['config.tracking'], + Options\QrCodeOptions::class => ['config.qr_codes'], Service\UrlShortener::class => [ ShortUrl\Helper\ShortUrlTitleResolutionHelper::class, @@ -138,6 +140,7 @@ return [ Service\ShortUrl\ShortUrlResolver::class, ShortUrl\Helper\ShortUrlStringifier::class, 'Logger_Shlink', + Options\QrCodeOptions::class, ], Action\RobotsAction::class => [Crawling\CrawlingHelper::class], diff --git a/module/Core/src/Action/Model/QrCodeParams.php b/module/Core/src/Action/Model/QrCodeParams.php index 39cd59a9..0e889c32 100644 --- a/module/Core/src/Action/Model/QrCodeParams.php +++ b/module/Core/src/Action/Model/QrCodeParams.php @@ -14,15 +14,17 @@ use Endroid\QrCode\Writer\SvgWriter; use Endroid\QrCode\Writer\WriterInterface; use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Message\ServerRequestInterface as Request; +use Shlinkio\Shlink\Core\Options\QrCodeOptions; +use function Functional\contains; use function strtolower; use function trim; final class QrCodeParams { - private const DEFAULT_SIZE = 300; private const MIN_SIZE = 50; private const MAX_SIZE = 1000; + private const SUPPORTED_FORMATS = ['png', 'svg']; private function __construct( private int $size, @@ -32,22 +34,22 @@ final class QrCodeParams ) { } - public static function fromRequest(ServerRequestInterface $request): self + public static function fromRequest(ServerRequestInterface $request, QrCodeOptions $defaults): self { $query = $request->getQueryParams(); return new self( - self::resolveSize($request, $query), - self::resolveMargin($query), - self::resolveWriter($query), - self::resolveErrorCorrection($query), + self::resolveSize($request, $query, $defaults), + self::resolveMargin($query, $defaults), + self::resolveWriter($query, $defaults), + self::resolveErrorCorrection($query, $defaults), ); } - private static function resolveSize(Request $request, array $query): int + private static function resolveSize(Request $request, array $query, QrCodeOptions $defaults): int { // FIXME Size attribute is deprecated. After v3.0.0, always use the query param instead - $size = (int) $request->getAttribute('size', $query['size'] ?? self::DEFAULT_SIZE); + $size = (int) $request->getAttribute('size', $query['size'] ?? $defaults->size()); if ($size < self::MIN_SIZE) { return self::MIN_SIZE; } @@ -55,13 +57,9 @@ final class QrCodeParams return $size > self::MAX_SIZE ? self::MAX_SIZE : $size; } - private static function resolveMargin(array $query): int + private static function resolveMargin(array $query, QrCodeOptions $defaults): int { - $margin = $query['margin'] ?? null; - if ($margin === null) { - return 0; - } - + $margin = $query['margin'] ?? (string) $defaults->margin(); $intMargin = (int) $margin; if ($margin !== (string) $intMargin) { return 0; @@ -70,18 +68,20 @@ final class QrCodeParams return $intMargin < 0 ? 0 : $intMargin; } - private static function resolveWriter(array $query): WriterInterface + private static function resolveWriter(array $query, QrCodeOptions $defaults): WriterInterface { - $format = strtolower(trim($query['format'] ?? 'png')); + $qFormat = self::normalizeParam($query['format'] ?? ''); + $format = contains(self::SUPPORTED_FORMATS, $qFormat) ? $qFormat : self::normalizeParam($defaults->format()); + return match ($format) { 'svg' => new SvgWriter(), default => new PngWriter(), }; } - private static function resolveErrorCorrection(array $query): ErrorCorrectionLevelInterface + private static function resolveErrorCorrection(array $query, QrCodeOptions $defaults): ErrorCorrectionLevelInterface { - $errorCorrectionLevel = strtolower(trim($query['errorCorrection'] ?? 'l')); + $errorCorrectionLevel = self::normalizeParam($query['errorCorrection'] ?? $defaults->errorCorrection()); return match ($errorCorrectionLevel) { 'h' => new ErrorCorrectionLevelHigh(), 'q' => new ErrorCorrectionLevelQuartile(), @@ -90,6 +90,11 @@ final class QrCodeParams }; } + private static function normalizeParam(string $param): string + { + return strtolower(trim($param)); + } + public function size(): int { return $this->size; diff --git a/module/Core/src/Action/QrCodeAction.php b/module/Core/src/Action/QrCodeAction.php index 977e0864..f8d2e275 100644 --- a/module/Core/src/Action/QrCodeAction.php +++ b/module/Core/src/Action/QrCodeAction.php @@ -14,6 +14,7 @@ use Shlinkio\Shlink\Common\Response\QrCodeResponse; use Shlinkio\Shlink\Core\Action\Model\QrCodeParams; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; +use Shlinkio\Shlink\Core\Options\QrCodeOptions; use Shlinkio\Shlink\Core\Service\ShortUrl\ShortUrlResolverInterface; use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlStringifierInterface; @@ -23,6 +24,7 @@ class QrCodeAction implements MiddlewareInterface private ShortUrlResolverInterface $urlResolver, private ShortUrlStringifierInterface $stringifier, private LoggerInterface $logger, + private QrCodeOptions $defaultOptions, ) { } @@ -37,7 +39,7 @@ class QrCodeAction implements MiddlewareInterface return $handler->handle($request); } - $params = QrCodeParams::fromRequest($request); + $params = QrCodeParams::fromRequest($request, $this->defaultOptions); $qrCodeBuilder = Builder::create() ->data($this->stringifier->stringify($shortUrl)) ->size($params->size()) diff --git a/module/Core/src/Options/QrCodeOptions.php b/module/Core/src/Options/QrCodeOptions.php new file mode 100644 index 00000000..80d6e456 --- /dev/null +++ b/module/Core/src/Options/QrCodeOptions.php @@ -0,0 +1,60 @@ +size; + } + + protected function setSize(int $size): void + { + $this->size = $size; + } + + public function margin(): int + { + return $this->margin; + } + + protected function setMargin(int $margin): void + { + $this->margin = $margin; + } + + public function format(): string + { + return $this->format; + } + + protected function setFormat(string $format): void + { + $this->format = $format; + } + + public function errorCorrection(): string + { + return $this->errorCorrection; + } + + protected function setErrorCorrection(string $errorCorrection): void + { + $this->errorCorrection = $errorCorrection; + } +} diff --git a/module/Core/test/Action/QrCodeActionTest.php b/module/Core/test/Action/QrCodeActionTest.php index 0595734e..1fdc35ef 100644 --- a/module/Core/test/Action/QrCodeActionTest.php +++ b/module/Core/test/Action/QrCodeActionTest.php @@ -20,6 +20,7 @@ use Shlinkio\Shlink\Core\Action\QrCodeAction; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; +use Shlinkio\Shlink\Core\Options\QrCodeOptions; use Shlinkio\Shlink\Core\Service\ShortUrl\ShortUrlResolverInterface; use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlStringifier; @@ -31,6 +32,7 @@ class QrCodeActionTest extends TestCase private QrCodeAction $action; private ObjectProphecy $urlResolver; + private QrCodeOptions $options; public function setUp(): void { @@ -38,11 +40,13 @@ class QrCodeActionTest extends TestCase $router->generateUri(Argument::cetera())->willReturn('/foo/bar'); $this->urlResolver = $this->prophesize(ShortUrlResolverInterface::class); + $this->options = new QrCodeOptions(); $this->action = new QrCodeAction( $this->urlResolver->reveal(), new ShortUrlStringifier(['domain' => 'doma.in']), new NullLogger(), + $this->options, ); } @@ -85,9 +89,11 @@ class QrCodeActionTest extends TestCase * @dataProvider provideQueries */ public function imageIsReturnedWithExpectedContentTypeBasedOnProvidedFormat( + string $defaultFormat, array $query, string $expectedContentType, ): void { + $this->options->setFromArray(['format' => $defaultFormat]); $code = 'abc123'; $this->urlResolver->resolveEnabledShortUrl(new ShortUrlIdentifier($code, ''))->willReturn( ShortUrl::createEmpty(), @@ -102,18 +108,26 @@ class QrCodeActionTest extends TestCase public function provideQueries(): iterable { - yield 'no format' => [[], 'image/png']; - yield 'png format' => [['format' => 'png'], 'image/png']; - yield 'svg format' => [['format' => 'svg'], 'image/svg+xml']; - yield 'unsupported format' => [['format' => 'jpg'], 'image/png']; + yield 'no format, png default' => ['png', [], 'image/png']; + yield 'no format, svg default' => ['svg', [], 'image/svg+xml']; + yield 'png format, png default' => ['png', ['format' => 'png'], 'image/png']; + yield 'png format, svg default' => ['svg', ['format' => 'png'], 'image/png']; + yield 'svg format, png default' => ['png', ['format' => 'svg'], 'image/svg+xml']; + yield 'svg format, svg default' => ['svg', ['format' => 'svg'], 'image/svg+xml']; + yield 'unsupported format, png default' => ['png', ['format' => 'jpg'], 'image/png']; + yield 'unsupported format, svg default' => ['svg', ['format' => 'jpg'], 'image/svg+xml']; } /** * @test * @dataProvider provideRequestsWithSize */ - public function imageIsReturnedWithExpectedSize(ServerRequestInterface $req, int $expectedSize): void - { + public function imageIsReturnedWithExpectedSize( + array $defaults, + ServerRequestInterface $req, + int $expectedSize, + ): void { + $this->options->setFromArray($defaults); $code = 'abc123'; $this->urlResolver->resolveEnabledShortUrl(new ShortUrlIdentifier($code, ''))->willReturn( ShortUrl::createEmpty(), @@ -128,25 +142,59 @@ class QrCodeActionTest extends TestCase public function provideRequestsWithSize(): iterable { - yield 'no size' => [ServerRequestFactory::fromGlobals(), 300]; - yield 'size in attr' => [ServerRequestFactory::fromGlobals()->withAttribute('size', '400'), 400]; - yield 'size in query' => [ServerRequestFactory::fromGlobals()->withQueryParams(['size' => '123']), 123]; + yield 'different margin and size defaults' => [ + ['size' => 660, 'margin' => 40], + ServerRequestFactory::fromGlobals(), + 740, + ]; + yield 'no size' => [[], ServerRequestFactory::fromGlobals(), 300]; + yield 'no size, different default' => [['size' => 500], ServerRequestFactory::fromGlobals(), 500]; + yield 'size in attr' => [[], ServerRequestFactory::fromGlobals()->withAttribute('size', '400'), 400]; + yield 'size in query' => [[], ServerRequestFactory::fromGlobals()->withQueryParams(['size' => '123']), 123]; + yield 'size in query, default margin' => [ + ['margin' => 25], + ServerRequestFactory::fromGlobals()->withQueryParams(['size' => '123']), + 173, + ]; yield 'size in query and attr' => [ + [], ServerRequestFactory::fromGlobals()->withAttribute('size', '350')->withQueryParams(['size' => '123']), 350, ]; - yield 'margin' => [ServerRequestFactory::fromGlobals()->withQueryParams(['margin' => '35']), 370]; + yield 'margin' => [[], ServerRequestFactory::fromGlobals()->withQueryParams(['margin' => '35']), 370]; + yield 'margin and different default' => [ + ['size' => 400], + ServerRequestFactory::fromGlobals()->withQueryParams(['margin' => '35']), + 470, + ]; yield 'margin and size' => [ + [], ServerRequestFactory::fromGlobals()->withQueryParams(['margin' => '100', 'size' => '200']), 400, ]; - yield 'negative margin' => [ServerRequestFactory::fromGlobals()->withQueryParams(['margin' => '-50']), 300]; - yield 'non-numeric margin' => [ServerRequestFactory::fromGlobals()->withQueryParams(['margin' => 'foo']), 300]; + yield 'negative margin' => [[], ServerRequestFactory::fromGlobals()->withQueryParams(['margin' => '-50']), 300]; + yield 'negative margin, default margin' => [ + ['margin' => 10], + ServerRequestFactory::fromGlobals()->withQueryParams(['margin' => '-50']), + 300, + ]; + yield 'non-numeric margin' => [ + [], + ServerRequestFactory::fromGlobals()->withQueryParams(['margin' => 'foo']), + 300, + ]; yield 'negative margin and size' => [ + [], + ServerRequestFactory::fromGlobals()->withQueryParams(['margin' => '-1', 'size' => '150']), + 150, + ]; + yield 'negative margin and size, default margin' => [ + ['margin' => 5], ServerRequestFactory::fromGlobals()->withQueryParams(['margin' => '-1', 'size' => '150']), 150, ]; yield 'non-numeric margin and size' => [ + [], ServerRequestFactory::fromGlobals()->withQueryParams(['margin' => 'foo', 'size' => '538']), 538, ]; From cfd3c13751dd1c3ab04d83a06f6a6a2a791f2f69 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 26 Sep 2021 20:13:50 +0200 Subject: [PATCH 34/55] Updated to latest installer --- composer.json | 2 +- config/autoload/installer.global.php | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/composer.json b/composer.json index b69306cf..2e920760 100644 --- a/composer.json +++ b/composer.json @@ -50,7 +50,7 @@ "shlinkio/shlink-config": "^1.2", "shlinkio/shlink-event-dispatcher": "^2.1", "shlinkio/shlink-importer": "^2.3.1", - "shlinkio/shlink-installer": "^6.1", + "shlinkio/shlink-installer": "dev-develop#07f1ac8 as 6.2", "shlinkio/shlink-ip-geolocation": "^2.0", "symfony/console": "^5.3", "symfony/filesystem": "^5.3", diff --git a/config/autoload/installer.global.php b/config/autoload/installer.global.php index 0a3374e1..fe9f3f5d 100644 --- a/config/autoload/installer.global.php +++ b/config/autoload/installer.global.php @@ -50,6 +50,10 @@ return [ Option\Tracking\DisableIpTrackingConfigOption::class, Option\Tracking\DisableReferrerTrackingConfigOption::class, Option\Tracking\DisableUaTrackingConfigOption::class, + Option\QrCode\DefaultSizeConfigOption::class, + Option\QrCode\DefaultMarginConfigOption::class, + Option\QrCode\DefaultFormatConfigOption::class, + Option\QrCode\DefaultErrorCorrectionConfigOption::class, ], 'installation_commands' => [ From 4b7e1222542b16fdfaca8865f9d2e02fadcd1e04 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 26 Sep 2021 20:15:00 +0200 Subject: [PATCH 35/55] Updated changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 46f386eb..f15fbbdc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this The config generated with the installing tool still has precedence over the env vars, so it cannot be combined. Either you use the tool, or use env vars. +* [#1149](https://github.com/shlinkio/shlink/issues/1149) Allowed to set custom defaults for the QR codes. + ### Changed * [#1142](https://github.com/shlinkio/shlink/issues/1142) Replaced `doctrine/cache` package with `symfony/cache`. * [#1157](https://github.com/shlinkio/shlink/issues/1157) All routes now support CORS, not only rest ones. From abc954aa47780eb8b28c1768e173b4eee04ec690 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 30 Sep 2021 22:57:24 +0200 Subject: [PATCH 36/55] Added extra DB tests ensuring proper short URL visits are resolved from an API key --- .../Repository/VisitRepositoryTest.php | 53 ++++++++++++++++--- 1 file changed, 47 insertions(+), 6 deletions(-) diff --git a/module/Core/test-db/Repository/VisitRepositoryTest.php b/module/Core/test-db/Repository/VisitRepositoryTest.php index 9f7859ff..c78583af 100644 --- a/module/Core/test-db/Repository/VisitRepositoryTest.php +++ b/module/Core/test-db/Repository/VisitRepositoryTest.php @@ -16,6 +16,7 @@ use Shlinkio\Shlink\Core\Model\ShortUrlMeta; use Shlinkio\Shlink\Core\Model\Visitor; use Shlinkio\Shlink\Core\Repository\VisitRepository; use Shlinkio\Shlink\Core\ShortUrl\Resolver\PersistenceShortUrlRelationResolver; +use Shlinkio\Shlink\Core\Validation\ShortUrlInputFilter; use Shlinkio\Shlink\Core\Visit\Persistence\VisitsCountFiltering; use Shlinkio\Shlink\Core\Visit\Persistence\VisitsListFiltering; use Shlinkio\Shlink\IpGeolocation\Model\Location; @@ -25,6 +26,7 @@ use Shlinkio\Shlink\Rest\Entity\ApiKey; use Shlinkio\Shlink\TestUtils\DbTest\DatabaseTestCase; use function Functional\map; +use function is_string; use function range; use function sprintf; @@ -171,6 +173,38 @@ class VisitRepositoryTest extends DatabaseTestCase )); } + /** @test */ + public function findVisitsByShortCodeReturnsProperDataWhenUsingAPiKeys(): void + { + $adminApiKey = ApiKey::create(); + $this->getEntityManager()->persist($adminApiKey); + + $restrictedApiKey = ApiKey::fromMeta(ApiKeyMeta::withRoles(RoleDefinition::forAuthoredShortUrls())); + $this->getEntityManager()->persist($restrictedApiKey); + + $this->getEntityManager()->flush(); + + [$shortCode1] = $this->createShortUrlsAndVisits(true, [], $adminApiKey); + [$shortCode2] = $this->createShortUrlsAndVisits('bar.com', [], $restrictedApiKey); + + self::assertNotEmpty($this->repo->findVisitsByShortCode( + ShortUrlIdentifier::fromShortCodeAndDomain($shortCode1), + new VisitsListFiltering(null, false, $adminApiKey->spec()), + )); + self::assertNotEmpty($this->repo->findVisitsByShortCode( + ShortUrlIdentifier::fromShortCodeAndDomain($shortCode2), + new VisitsListFiltering(null, false, $adminApiKey->spec()), + )); + self::assertEmpty($this->repo->findVisitsByShortCode( + ShortUrlIdentifier::fromShortCodeAndDomain($shortCode1), + new VisitsListFiltering(null, false, $restrictedApiKey->spec()), + )); + self::assertNotEmpty($this->repo->findVisitsByShortCode( + ShortUrlIdentifier::fromShortCodeAndDomain($shortCode2), + new VisitsListFiltering(null, false, $restrictedApiKey->spec()), + )); + } + /** @test */ public function findVisitsByTagReturnsProperData(): void { @@ -354,19 +388,26 @@ class VisitRepositoryTest extends DatabaseTestCase )); } - private function createShortUrlsAndVisits(bool $withDomain = true, array $tags = []): array - { + /** + * @return array{string, string, ShortUrl} + */ + private function createShortUrlsAndVisits( + bool|string $withDomain = true, + array $tags = [], + ?ApiKey $apiKey = null, + ): array { $shortUrl = ShortUrl::fromMeta(ShortUrlMeta::fromRawData([ - 'longUrl' => '', - 'tags' => $tags, + ShortUrlInputFilter::LONG_URL => '', + ShortUrlInputFilter::TAGS => $tags, + ShortUrlInputFilter::API_KEY => $apiKey, ]), $this->relationResolver); - $domain = 'example.com'; + $domain = is_string($withDomain) ? $withDomain : 'example.com'; $shortCode = $shortUrl->getShortCode(); $this->getEntityManager()->persist($shortUrl); $this->createVisitsForShortUrl($shortUrl); - if ($withDomain) { + if ($withDomain !== false) { $shortUrlWithDomain = ShortUrl::fromMeta(ShortUrlMeta::fromRawData([ 'customSlug' => $shortCode, 'domain' => $domain, From 5e627641eac275a85294d754de006f9710645d61 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 1 Oct 2021 19:32:34 +0200 Subject: [PATCH 37/55] Added more tests ensuring any short URL can be fetched by using an admin API key --- .../Repository/ShortUrlRepositoryTest.php | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/module/Core/test-db/Repository/ShortUrlRepositoryTest.php b/module/Core/test-db/Repository/ShortUrlRepositoryTest.php index a614844f..d4ff42b8 100644 --- a/module/Core/test-db/Repository/ShortUrlRepositoryTest.php +++ b/module/Core/test-db/Repository/ShortUrlRepositoryTest.php @@ -355,6 +355,8 @@ class ShortUrlRepositoryTest extends DatabaseTestCase $this->getEntityManager()->persist($wrongDomainApiKey); $rightDomainApiKey = ApiKey::fromMeta(ApiKeyMeta::withRoles(RoleDefinition::forDomain($rightDomain))); $this->getEntityManager()->persist($rightDomainApiKey); + $adminApiKey = ApiKey::create(); + $this->getEntityManager()->persist($adminApiKey); $shortUrl = ShortUrl::fromMeta(ShortUrlMeta::fromRawData([ 'validSince' => $start, @@ -365,6 +367,12 @@ class ShortUrlRepositoryTest extends DatabaseTestCase ]), $this->relationResolver); $this->getEntityManager()->persist($shortUrl); + $nonDomainShortUrl = ShortUrl::fromMeta(ShortUrlMeta::fromRawData([ + 'apiKey' => $apiKey, + 'longUrl' => 'non-domain', + ]), $this->relationResolver); + $this->getEntityManager()->persist($nonDomainShortUrl); + $this->getEntityManager()->flush(); self::assertSame( @@ -379,6 +387,12 @@ class ShortUrlRepositoryTest extends DatabaseTestCase 'longUrl' => 'foo', 'tags' => ['foo', 'bar'], ]))); + self::assertSame($shortUrl, $this->repo->findOneMatching(ShortUrlMeta::fromRawData([ + 'validSince' => $start, + 'apiKey' => $adminApiKey, + 'longUrl' => 'foo', + 'tags' => ['foo', 'bar'], + ]))); self::assertNull($this->repo->findOneMatching(ShortUrlMeta::fromRawData([ 'validSince' => $start, 'apiKey' => $otherApiKey, @@ -424,6 +438,27 @@ class ShortUrlRepositoryTest extends DatabaseTestCase 'tags' => ['foo', 'bar'], ])), ); + + self::assertSame( + $nonDomainShortUrl, + $this->repo->findOneMatching(ShortUrlMeta::fromRawData([ + 'apiKey' => $apiKey, + 'longUrl' => 'non-domain', + ])), + ); + self::assertSame( + $nonDomainShortUrl, + $this->repo->findOneMatching(ShortUrlMeta::fromRawData([ + 'apiKey' => $adminApiKey, + 'longUrl' => 'non-domain', + ])), + ); + self::assertNull( + $this->repo->findOneMatching(ShortUrlMeta::fromRawData([ + 'apiKey' => $otherApiKey, + 'longUrl' => 'non-domain', + ])), + ); } /** @test */ From 1ed6458b39dad70b41a76fdc46f4bec195c0caea Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 2 Oct 2021 09:32:04 +0200 Subject: [PATCH 38/55] Added forwardQuery property in short URLs, that determines if the query should be forwarded to the long URL --- data/migrations/Version20211002072605.php | 26 +++++++++++++++++++ .../Shlinkio.Shlink.Core.Entity.ShortUrl.php | 5 ++++ module/Core/src/Entity/ShortUrl.php | 6 +++++ .../Helper/ShortUrlRedirectionBuilder.php | 3 ++- 4 files changed, 39 insertions(+), 1 deletion(-) create mode 100644 data/migrations/Version20211002072605.php diff --git a/data/migrations/Version20211002072605.php b/data/migrations/Version20211002072605.php new file mode 100644 index 00000000..5f8db987 --- /dev/null +++ b/data/migrations/Version20211002072605.php @@ -0,0 +1,26 @@ +getTable('short_urls'); + $this->skipIf($shortUrls->hasColumn('forward_query')); + $shortUrls->addColumn('forward_query', Types::BOOLEAN, ['default' => true]); + } + + public function down(Schema $schema): void + { + $shortUrls = $schema->getTable('short_urls'); + $this->skipIf(! $shortUrls->hasColumn('forward_query')); + $shortUrls->dropColumn('forward_query'); + } +} diff --git a/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Entity.ShortUrl.php b/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Entity.ShortUrl.php index a9269d36..83fd7e79 100644 --- a/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Entity.ShortUrl.php +++ b/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Entity.ShortUrl.php @@ -100,4 +100,9 @@ return static function (ClassMetadata $metadata, array $emConfig): void { ->columnName('crawlable') ->option('default', false) ->build(); + + $builder->createField('forwardQuery', Types::BOOLEAN) + ->columnName('forward_query') + ->option('default', true) + ->build(); }; diff --git a/module/Core/src/Entity/ShortUrl.php b/module/Core/src/Entity/ShortUrl.php index 78527115..210310f5 100644 --- a/module/Core/src/Entity/ShortUrl.php +++ b/module/Core/src/Entity/ShortUrl.php @@ -43,6 +43,7 @@ class ShortUrl extends AbstractEntity private ?string $title = null; private bool $titleWasAutoResolved = false; private bool $crawlable = false; + private bool $forwardQuery = true; private function __construct() { @@ -207,6 +208,11 @@ class ShortUrl extends AbstractEntity return $this->crawlable; } + public function forwardQuery(): bool + { + return $this->forwardQuery; + } + public function update( ShortUrlEdit $shortUrlEdit, ?ShortUrlRelationResolverInterface $relationResolver = null, diff --git a/module/Core/src/ShortUrl/Helper/ShortUrlRedirectionBuilder.php b/module/Core/src/ShortUrl/Helper/ShortUrlRedirectionBuilder.php index f83a7eb9..3251922d 100644 --- a/module/Core/src/ShortUrl/Helper/ShortUrlRedirectionBuilder.php +++ b/module/Core/src/ShortUrl/Helper/ShortUrlRedirectionBuilder.php @@ -21,9 +21,10 @@ class ShortUrlRedirectionBuilder implements ShortUrlRedirectionBuilderInterface public function buildShortUrlRedirect(ShortUrl $shortUrl, array $currentQuery, ?string $extraPath = null): string { $uri = Uri::createFromString($shortUrl->getLongUrl()); + $shouldForwardQuery = $shortUrl->forwardQuery(); return $uri - ->withQuery($this->resolveQuery($uri, $currentQuery)) + ->withQuery($shouldForwardQuery ? $this->resolveQuery($uri, $currentQuery) : $uri->getQuery()) ->withPath($this->resolvePath($uri, $extraPath)) ->__toString(); } From 8212d3c540879e2458a50a6ee13b228a0341c2a6 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 2 Oct 2021 10:02:43 +0200 Subject: [PATCH 39/55] Allowed to set and update the forwardQuery param on short URLs --- docs/swagger/definitions/ShortUrlEdition.json | 48 +++++++++++ docs/swagger/paths/v1_short-urls.json | 82 +++++++------------ .../paths/v1_short-urls_{shortCode}.json | 43 +--------- module/Core/src/Entity/ShortUrl.php | 4 + module/Core/src/Model/ShortUrlEdit.php | 14 ++++ module/Core/src/Model/ShortUrlMeta.php | 7 ++ .../src/Validation/ShortUrlInputFilter.php | 6 +- 7 files changed, 106 insertions(+), 98 deletions(-) create mode 100644 docs/swagger/definitions/ShortUrlEdition.json diff --git a/docs/swagger/definitions/ShortUrlEdition.json b/docs/swagger/definitions/ShortUrlEdition.json new file mode 100644 index 00000000..94ef6135 --- /dev/null +++ b/docs/swagger/definitions/ShortUrlEdition.json @@ -0,0 +1,48 @@ +{ + "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", + "nullable": true + }, + "validUntil": { + "description": "The date (in ISO-8601 format) until which this short code will be valid", + "type": "string", + "nullable": true + }, + "maxVisits": { + "description": "The maximum number of allowed visits for this short code", + "type": "number", + "nullable": true + }, + "validateUrl": { + "description": "Tells if the long URL (if provided) should or should not be validated as a reachable URL. If not provided, it will fall back to app-level config", + "type": "boolean" + }, + "tags": { + "type": "array", + "items": { + "type": "string" + }, + "description": "The list of tags to set to the short URL." + }, + "title": { + "type": "string", + "description": "A descriptive title of the short URL.", + "nullable": true + }, + "crawlable": { + "type": "boolean", + "description": "Tells if this URL will be included as 'Allow' in Shlink's robots.txt." + }, + "forwardQuery": { + "type": "boolean", + "description": "Tells if the query params should be forwarded from the short URL to the long one, as explained in [the docs](https://shlink.io/documentation/some-features/#query-params-forwarding)." + } + } +} diff --git a/docs/swagger/paths/v1_short-urls.json b/docs/swagger/paths/v1_short-urls.json index 8cf22045..a4643058 100644 --- a/docs/swagger/paths/v1_short-urls.json +++ b/docs/swagger/paths/v1_short-urls.json @@ -225,63 +225,37 @@ "content": { "application/json": { "schema": { - "type": "object", - "required": [ - "longUrl" - ], - "properties": { - "longUrl": { - "description": "The URL to parse", - "type": "string" + "allOf": [ + { + "$ref": "../definitions/ShortUrlEdition.json" }, - "tags": { - "description": "The URL to parse", - "type": "array", - "items": { - "type": "string" + { + "type": "object", + "required": ["longUrl"], + "properties": { + "customSlug": { + "description": "A unique custom slug to be used instead of the generated short code", + "type": "string" + }, + "findIfExists": { + "description": "Will force existing matching URL to be returned if found, instead of creating a new one", + "type": "boolean" + }, + "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" + }, + "validateUrl": { + "description": "Tells if the long URL should or should not be validated as a reachable URL. If not provided, it will fall back to app-level config", + "type": "boolean" + } } - }, - "validSince": { - "description": "The date (in ISO-8601 format) from which this short code will be valid", - "type": "string" - }, - "validUntil": { - "description": "The date (in ISO-8601 format) until which this short code will be valid", - "type": "string" - }, - "customSlug": { - "description": "A unique custom slug to be used instead of the generated short code", - "type": "string" - }, - "maxVisits": { - "description": "The maximum number of allowed visits for this short code", - "type": "number" - }, - "findIfExists": { - "description": "Will force existing matching URL to be returned if found, instead of creating a new one", - "type": "boolean" - }, - "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" - }, - "validateUrl": { - "description": "Tells if the long URL should or should not be validated as a reachable URL. If not provided, it will fall back to app-level config", - "type": "boolean" - }, - "title": { - "type": "string", - "description": "A descriptive title of the short URL." - }, - "crawlable": { - "type": "boolean", - "description": "Tells if this URL will be included as 'Allow' in Shlink's robots.txt." } - } + ] } } } diff --git a/docs/swagger/paths/v1_short-urls_{shortCode}.json b/docs/swagger/paths/v1_short-urls_{shortCode}.json index 8691c0b5..e37df965 100644 --- a/docs/swagger/paths/v1_short-urls_{shortCode}.json +++ b/docs/swagger/paths/v1_short-urls_{shortCode}.json @@ -112,48 +112,7 @@ "content": { "application/json": { "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", - "nullable": true - }, - "validUntil": { - "description": "The date (in ISO-8601 format) until which this short code will be valid", - "type": "string", - "nullable": true - }, - "maxVisits": { - "description": "The maximum number of allowed visits for this short code", - "type": "number", - "nullable": true - }, - "validateUrl": { - "description": "Tells if the long URL (if provided) should or should not be validated as a reachable URL. If not provided, it will fall back to app-level config", - "type": "boolean" - }, - "tags": { - "type": "array", - "items": { - "type": "string" - }, - "description": "The list of tags to set to the short URL." - }, - "title": { - "type": "string", - "description": "A descriptive title of the short URL.", - "nullable": true - }, - "crawlable": { - "type": "boolean", - "description": "Tells if this URL will be included as 'Allow' in Shlink's robots.txt." - } - } + "$ref": "../definitions/ShortUrlEdition.json" } } } diff --git a/module/Core/src/Entity/ShortUrl.php b/module/Core/src/Entity/ShortUrl.php index 210310f5..9fff1509 100644 --- a/module/Core/src/Entity/ShortUrl.php +++ b/module/Core/src/Entity/ShortUrl.php @@ -81,6 +81,7 @@ class ShortUrl extends AbstractEntity $instance->title = $meta->getTitle(); $instance->titleWasAutoResolved = $meta->titleWasAutoResolved(); $instance->crawlable = $meta->isCrawlable(); + $instance->forwardQuery = $meta->forwardQuery(); return $instance; } @@ -244,6 +245,9 @@ class ShortUrl extends AbstractEntity $this->title = $shortUrlEdit->title(); $this->titleWasAutoResolved = $shortUrlEdit->titleWasAutoResolved(); } + if ($shortUrlEdit->forwardQueryWasProvided()) { + $this->forwardQuery = $shortUrlEdit->forwardQuery(); + } } /** diff --git a/module/Core/src/Model/ShortUrlEdit.php b/module/Core/src/Model/ShortUrlEdit.php index 32c1ca1e..8e187cf6 100644 --- a/module/Core/src/Model/ShortUrlEdit.php +++ b/module/Core/src/Model/ShortUrlEdit.php @@ -32,6 +32,8 @@ final class ShortUrlEdit implements TitleResolutionModelInterface private ?bool $validateUrl = null; private bool $crawlablePropWasProvided = false; private bool $crawlable = false; + private bool $forwardQueryPropWasProvided = false; + private bool $forwardQuery = true; private function __construct() { @@ -64,6 +66,7 @@ final class ShortUrlEdit implements TitleResolutionModelInterface $this->tagsPropWasProvided = array_key_exists(ShortUrlInputFilter::TAGS, $data); $this->titlePropWasProvided = array_key_exists(ShortUrlInputFilter::TITLE, $data); $this->crawlablePropWasProvided = array_key_exists(ShortUrlInputFilter::CRAWLABLE, $data); + $this->forwardQueryPropWasProvided = array_key_exists(ShortUrlInputFilter::FORWARD_QUERY, $data); $this->longUrl = $inputFilter->getValue(ShortUrlInputFilter::LONG_URL); $this->validSince = parseDateField($inputFilter->getValue(ShortUrlInputFilter::VALID_SINCE)); @@ -73,6 +76,7 @@ final class ShortUrlEdit implements TitleResolutionModelInterface $this->tags = $inputFilter->getValue(ShortUrlInputFilter::TAGS); $this->title = $inputFilter->getValue(ShortUrlInputFilter::TITLE); $this->crawlable = $inputFilter->getValue(ShortUrlInputFilter::CRAWLABLE); + $this->forwardQuery = getOptionalBoolFromInputFilter($inputFilter, ShortUrlInputFilter::FORWARD_QUERY) ?? true; } public function longUrl(): ?string @@ -176,4 +180,14 @@ final class ShortUrlEdit implements TitleResolutionModelInterface { return $this->crawlablePropWasProvided; } + + public function forwardQuery(): bool + { + return $this->forwardQuery; + } + + public function forwardQueryWasProvided(): bool + { + return $this->forwardQueryPropWasProvided; + } } diff --git a/module/Core/src/Model/ShortUrlMeta.php b/module/Core/src/Model/ShortUrlMeta.php index 0c982f17..74390281 100644 --- a/module/Core/src/Model/ShortUrlMeta.php +++ b/module/Core/src/Model/ShortUrlMeta.php @@ -32,6 +32,7 @@ final class ShortUrlMeta implements TitleResolutionModelInterface private ?string $title = null; private bool $titleWasAutoResolved = false; private bool $crawlable = false; + private bool $forwardQuery = true; private function __construct() { @@ -82,6 +83,7 @@ final class ShortUrlMeta implements TitleResolutionModelInterface $this->tags = $inputFilter->getValue(ShortUrlInputFilter::TAGS); $this->title = $inputFilter->getValue(ShortUrlInputFilter::TITLE); $this->crawlable = $inputFilter->getValue(ShortUrlInputFilter::CRAWLABLE); + $this->forwardQuery = getOptionalBoolFromInputFilter($inputFilter, ShortUrlInputFilter::FORWARD_QUERY) ?? true; } public function getLongUrl(): string @@ -195,4 +197,9 @@ final class ShortUrlMeta implements TitleResolutionModelInterface { return $this->crawlable; } + + public function forwardQuery(): bool + { + return $this->forwardQuery; + } } diff --git a/module/Core/src/Validation/ShortUrlInputFilter.php b/module/Core/src/Validation/ShortUrlInputFilter.php index fd5a92e7..47f6f8ac 100644 --- a/module/Core/src/Validation/ShortUrlInputFilter.php +++ b/module/Core/src/Validation/ShortUrlInputFilter.php @@ -33,6 +33,7 @@ class ShortUrlInputFilter extends InputFilter public const TAGS = 'tags'; public const TITLE = 'title'; public const CRAWLABLE = 'crawlable'; + public const FORWARD_QUERY = 'forwardQuery'; private function __construct(array $data, bool $requireLongUrl) { @@ -89,9 +90,10 @@ class ShortUrlInputFilter extends InputFilter $this->add($this->createBooleanInput(self::FIND_IF_EXISTS, false)); - // This cannot be defined as a boolean input because it can actually have 3 values, true, false and null. - // Defining it as boolean will make null fall back to false, which is not the desired behavior. + // These cannot be defined as a boolean inputs, because they can actually have 3 values: true, false and null. + // Defining them as boolean will make null fall back to false, which is not the desired behavior. $this->add($this->createInput(self::VALIDATE_URL, false)); + $this->add($this->createInput(self::FORWARD_QUERY, false)); $domain = $this->createInput(self::DOMAIN, false); $domain->getValidatorChain()->attach(new Validation\HostAndPortValidator()); From 74a08b86cefaf31519a09c9169e340a9833a9ab2 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 2 Oct 2021 10:16:56 +0200 Subject: [PATCH 40/55] Estended ShortUrlRedirectionBuilderTest covering short URLS withput query forwarding --- .../Helper/ShortUrlRedirectionBuilderTest.php | 76 +++++++++++++++---- 1 file changed, 62 insertions(+), 14 deletions(-) diff --git a/module/Core/test/ShortUrl/Helper/ShortUrlRedirectionBuilderTest.php b/module/Core/test/ShortUrl/Helper/ShortUrlRedirectionBuilderTest.php index 985a3cea..829d77ea 100644 --- a/module/Core/test/ShortUrl/Helper/ShortUrlRedirectionBuilderTest.php +++ b/module/Core/test/ShortUrl/Helper/ShortUrlRedirectionBuilderTest.php @@ -6,27 +6,34 @@ namespace ShlinkioTest\Shlink\Core\ShortUrl\Helper; use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\Core\Entity\ShortUrl; +use Shlinkio\Shlink\Core\Model\ShortUrlMeta; use Shlinkio\Shlink\Core\Options\TrackingOptions; use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlRedirectionBuilder; class ShortUrlRedirectionBuilderTest extends TestCase { private ShortUrlRedirectionBuilder $redirectionBuilder; - private TrackingOptions $trackingOptions; protected function setUp(): void { - $this->trackingOptions = new TrackingOptions(['disable_track_param' => 'foobar']); - $this->redirectionBuilder = new ShortUrlRedirectionBuilder($this->trackingOptions); + $trackingOptions = new TrackingOptions(['disable_track_param' => 'foobar']); + $this->redirectionBuilder = new ShortUrlRedirectionBuilder($trackingOptions); } /** * @test * @dataProvider provideData */ - public function buildShortUrlRedirectBuildsExpectedUrl(string $expectedUrl, array $query, ?string $extraPath): void - { - $shortUrl = ShortUrl::withLongUrl('https://domain.com/foo/bar?some=thing'); + public function buildShortUrlRedirectBuildsExpectedUrl( + string $expectedUrl, + array $query, + ?string $extraPath, + ?bool $forwardQuery, + ): void { + $shortUrl = ShortUrl::fromMeta(ShortUrlMeta::fromRawData([ + 'longUrl' => 'https://domain.com/foo/bar?some=thing', + 'forwardQuery' => $forwardQuery, + ])); $result = $this->redirectionBuilder->buildShortUrlRedirect($shortUrl, $query, $extraPath); self::assertEquals($expectedUrl, $result); @@ -34,18 +41,59 @@ class ShortUrlRedirectionBuilderTest extends TestCase public function provideData(): iterable { - yield ['https://domain.com/foo/bar?some=thing', [], null]; - yield ['https://domain.com/foo/bar?some=thing&else', ['else' => null], null]; - yield ['https://domain.com/foo/bar?some=thing&foo=bar', ['foo' => 'bar'], null]; - yield ['https://domain.com/foo/bar?some=thing&123=foo', ['123' => 'foo'], null]; - yield ['https://domain.com/foo/bar?some=thing&456=foo', [456 => 'foo'], null]; - yield ['https://domain.com/foo/bar?some=overwritten&foo=bar', ['foo' => 'bar', 'some' => 'overwritten'], null]; - yield ['https://domain.com/foo/bar?some=overwritten', ['foobar' => 'notrack', 'some' => 'overwritten'], null]; - yield ['https://domain.com/foo/bar/something/else-baz?some=thing', [], '/something/else-baz']; + yield ['https://domain.com/foo/bar?some=thing', [], null, true]; + yield ['https://domain.com/foo/bar?some=thing', [], null, null]; + yield ['https://domain.com/foo/bar?some=thing', [], null, false]; + yield ['https://domain.com/foo/bar?some=thing&else', ['else' => null], null, true]; + yield ['https://domain.com/foo/bar?some=thing&foo=bar', ['foo' => 'bar'], null, true]; + yield ['https://domain.com/foo/bar?some=thing&foo=bar', ['foo' => 'bar'], null, null]; + yield ['https://domain.com/foo/bar?some=thing', ['foo' => 'bar'], null, false]; + yield ['https://domain.com/foo/bar?some=thing&123=foo', ['123' => 'foo'], null, true]; + yield ['https://domain.com/foo/bar?some=thing&456=foo', [456 => 'foo'], null, true]; + yield ['https://domain.com/foo/bar?some=thing&456=foo', [456 => 'foo'], null, null]; + yield ['https://domain.com/foo/bar?some=thing', [456 => 'foo'], null, false]; + yield [ + 'https://domain.com/foo/bar?some=overwritten&foo=bar', + ['foo' => 'bar', 'some' => 'overwritten'], + null, + true, + ]; + yield [ + 'https://domain.com/foo/bar?some=overwritten', + ['foobar' => 'notrack', 'some' => 'overwritten'], + null, + true, + ]; + yield [ + 'https://domain.com/foo/bar?some=overwritten', + ['foobar' => 'notrack', 'some' => 'overwritten'], + null, + null, + ]; + yield [ + 'https://domain.com/foo/bar?some=thing', + ['foobar' => 'notrack', 'some' => 'overwritten'], + null, + false, + ]; + yield ['https://domain.com/foo/bar/something/else-baz?some=thing', [], '/something/else-baz', true]; yield [ 'https://domain.com/foo/bar/something/else-baz?some=thing&hello=world', ['hello' => 'world'], '/something/else-baz', + true, + ]; + yield [ + 'https://domain.com/foo/bar/something/else-baz?some=thing&hello=world', + ['hello' => 'world'], + '/something/else-baz', + null, + ]; + yield [ + 'https://domain.com/foo/bar/something/else-baz?some=thing', + ['hello' => 'world'], + '/something/else-baz', + false, ]; } } From e21f9dd1fbb13ebabbc695f62239c7e0b37871f9 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 2 Oct 2021 10:31:23 +0200 Subject: [PATCH 41/55] Added forwardQuery prop to the SHortUrl serialization --- docs/swagger/definitions/ShortUrl.json | 17 +++++++++++++++++ .../Transformer/ShortUrlDataTransformer.php | 1 + .../Mercure/MercureUpdatesGeneratorTest.php | 1 + .../Rest/test-api/Action/ListShortUrlsTest.php | 6 ++++++ .../Rest/test-api/Fixtures/ShortUrlsFixture.php | 10 +++++++--- 5 files changed, 32 insertions(+), 3 deletions(-) diff --git a/docs/swagger/definitions/ShortUrl.json b/docs/swagger/definitions/ShortUrl.json index b2ffd3f6..a5dee481 100644 --- a/docs/swagger/definitions/ShortUrl.json +++ b/docs/swagger/definitions/ShortUrl.json @@ -1,5 +1,18 @@ { "type": "object", + "required": [ + "shortCode", + "shortUrl", + "longUrl", + "dateCreated", + "visitsCount", + "tags", + "meta", + "domain", + "title", + "crawlable", + "forwardQuery" + ], "properties": { "shortCode": { "type": "string", @@ -45,6 +58,10 @@ "crawlable": { "type": "boolean", "description": "Tells if this URL will be included as 'Allow' in Shlink's robots.txt." + }, + "forwardQuery": { + "type": "boolean", + "description": "Tells if this URL will forward the query params to the long URL when visited, as explained in [the docs](https://shlink.io/documentation/some-features/#query-params-forwarding)." } } } diff --git a/module/Core/src/ShortUrl/Transformer/ShortUrlDataTransformer.php b/module/Core/src/ShortUrl/Transformer/ShortUrlDataTransformer.php index 61049626..554f9894 100644 --- a/module/Core/src/ShortUrl/Transformer/ShortUrlDataTransformer.php +++ b/module/Core/src/ShortUrl/Transformer/ShortUrlDataTransformer.php @@ -33,6 +33,7 @@ class ShortUrlDataTransformer implements DataTransformerInterface 'domain' => $shortUrl->getDomain(), 'title' => $shortUrl->title(), 'crawlable' => $shortUrl->crawlable(), + 'forwardQuery' => $shortUrl->forwardQuery(), ]; } diff --git a/module/Core/test/Mercure/MercureUpdatesGeneratorTest.php b/module/Core/test/Mercure/MercureUpdatesGeneratorTest.php index 86d1b3d5..14378b4f 100644 --- a/module/Core/test/Mercure/MercureUpdatesGeneratorTest.php +++ b/module/Core/test/Mercure/MercureUpdatesGeneratorTest.php @@ -60,6 +60,7 @@ class MercureUpdatesGeneratorTest extends TestCase 'domain' => null, 'title' => $title, 'crawlable' => false, + 'forwardQuery' => true, ], 'visit' => [ 'referer' => '', diff --git a/module/Rest/test-api/Action/ListShortUrlsTest.php b/module/Rest/test-api/Action/ListShortUrlsTest.php index 95d77dc6..fcc07719 100644 --- a/module/Rest/test-api/Action/ListShortUrlsTest.php +++ b/module/Rest/test-api/Action/ListShortUrlsTest.php @@ -27,6 +27,7 @@ class ListShortUrlsTest extends ApiTestCase 'domain' => null, 'title' => 'My cool title', 'crawlable' => true, + 'forwardQuery' => true, ]; private const SHORT_URL_DOCS = [ 'shortCode' => 'ghi789', @@ -43,6 +44,7 @@ class ListShortUrlsTest extends ApiTestCase 'domain' => null, 'title' => null, 'crawlable' => false, + 'forwardQuery' => true, ]; private const SHORT_URL_CUSTOM_SLUG_AND_DOMAIN = [ 'shortCode' => 'custom-with-domain', @@ -59,6 +61,7 @@ class ListShortUrlsTest extends ApiTestCase 'domain' => 'some-domain.com', 'title' => null, 'crawlable' => false, + 'forwardQuery' => true, ]; private const SHORT_URL_META = [ 'shortCode' => 'def456', @@ -77,6 +80,7 @@ class ListShortUrlsTest extends ApiTestCase 'domain' => null, 'title' => null, 'crawlable' => false, + 'forwardQuery' => true, ]; private const SHORT_URL_CUSTOM_SLUG = [ 'shortCode' => 'custom', @@ -93,6 +97,7 @@ class ListShortUrlsTest extends ApiTestCase 'domain' => null, 'title' => null, 'crawlable' => false, + 'forwardQuery' => false, ]; private const SHORT_URL_CUSTOM_DOMAIN = [ 'shortCode' => 'ghi789', @@ -111,6 +116,7 @@ class ListShortUrlsTest extends ApiTestCase 'domain' => 'example.com', 'title' => null, 'crawlable' => false, + 'forwardQuery' => true, ]; /** diff --git a/module/Rest/test-api/Fixtures/ShortUrlsFixture.php b/module/Rest/test-api/Fixtures/ShortUrlsFixture.php index ccc83525..9510f8ed 100644 --- a/module/Rest/test-api/Fixtures/ShortUrlsFixture.php +++ b/module/Rest/test-api/Fixtures/ShortUrlsFixture.php @@ -51,9 +51,13 @@ class ShortUrlsFixture extends AbstractFixture implements DependentFixtureInterf ]), $relationResolver), '2019-01-01 00:00:10'); $manager->persist($defShortUrl); - $customShortUrl = $this->setShortUrlDate(ShortUrl::fromMeta(ShortUrlMeta::fromRawData( - ['customSlug' => 'custom', 'maxVisits' => 2, 'apiKey' => $authorApiKey, 'longUrl' => 'https://shlink.io'], - )), '2019-01-01 00:00:20'); + $customShortUrl = $this->setShortUrlDate(ShortUrl::fromMeta(ShortUrlMeta::fromRawData([ + 'customSlug' => 'custom', + 'maxVisits' => 2, + 'apiKey' => $authorApiKey, + 'longUrl' => 'https://shlink.io', + 'forwardQuery' => false, + ])), '2019-01-01 00:00:20'); $manager->persist($customShortUrl); $ghiShortUrl = $this->setShortUrlDate( From 0c95b978b4b3b61d4b5235f379e1f72faab0fe0c Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 2 Oct 2021 10:45:00 +0200 Subject: [PATCH 42/55] Added option in CLI to disable query forwarding when creating Short URLs --- CHANGELOG.md | 3 +++ .../Command/ShortUrl/GenerateShortUrlCommand.php | 16 +++++++++++++++- module/Core/src/Util/UrlValidator.php | 2 +- 3 files changed, 19 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f15fbbdc..9d253e85 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this The config generated with the installing tool still has precedence over the env vars, so it cannot be combined. Either you use the tool, or use env vars. * [#1149](https://github.com/shlinkio/shlink/issues/1149) Allowed to set custom defaults for the QR codes. +* [#1112](https://github.com/shlinkio/shlink/issues/1112) Added new option to define if the query string should be forwarded on a per-short URL basis. + + The new `forwardQuery=true|false` param can be provided during short URL creation or edition, via REST API or CLI command, allowing to override the default behavior which makes the query string to always be forwarded. ### Changed * [#1142](https://github.com/shlinkio/shlink/issues/1142) Replaced `doctrine/cache` package with `symfony/cache`. diff --git a/module/CLI/src/Command/ShortUrl/GenerateShortUrlCommand.php b/module/CLI/src/Command/ShortUrl/GenerateShortUrlCommand.php index 6ed3e37c..e43b4ec5 100644 --- a/module/CLI/src/Command/ShortUrl/GenerateShortUrlCommand.php +++ b/module/CLI/src/Command/ShortUrl/GenerateShortUrlCommand.php @@ -104,7 +104,19 @@ class GenerateShortUrlCommand extends BaseCommand 'no-validate-url', null, InputOption::VALUE_NONE, - 'Forces the long URL to not be validated, regardless what is globally configured.', + '[DEPRECATED] Forces the long URL to not be validated, regardless what is globally configured.', + ) + ->addOption( + 'crawlable', + 'r', + InputOption::VALUE_NONE, + 'Tells if this URL will be included as "Allow" in Shlink\'s robots.txt.', + ) + ->addOption( + 'no-forward-query', + 'w', + InputOption::VALUE_NONE, + 'Disables the forwarding of the query string to the long URL, when the new short URL is visited.', ); } @@ -156,6 +168,8 @@ class GenerateShortUrlCommand extends BaseCommand ShortUrlInputFilter::SHORT_CODE_LENGTH => $shortCodeLength, ShortUrlInputFilter::VALIDATE_URL => $doValidateUrl, ShortUrlInputFilter::TAGS => $tags, + ShortUrlInputFilter::CRAWLABLE => $input->getOption('crawlable'), + ShortUrlInputFilter::FORWARD_QUERY => !$input->getOption('no-forward-query'), ])); $io->writeln([ diff --git a/module/Core/src/Util/UrlValidator.php b/module/Core/src/Util/UrlValidator.php index 5b84c5f8..bd0a5cfb 100644 --- a/module/Core/src/Util/UrlValidator.php +++ b/module/Core/src/Util/UrlValidator.php @@ -32,7 +32,7 @@ class UrlValidator implements UrlValidatorInterface, RequestMethodInterface */ public function validateUrl(string $url, ?bool $doValidate): void { - // If the URL validation is not enabled or it was explicitly set to not validate, skip check + // If the URL validation is not enabled, or it was explicitly set to not validate, skip check $doValidate = $doValidate ?? $this->options->isUrlValidationEnabled(); if (! $doValidate) { return; From 36e740f4cc645e73773022fd7f296b8f5997e269 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 2 Oct 2021 17:30:25 +0200 Subject: [PATCH 43/55] Added logic to forward path and domain to not-found redirects when they contain placeholders --- .../src/Config/NotFoundRedirectResolver.php | 48 ++++++++++++++++--- .../NotFoundRedirectResolverInterface.php | 2 + .../ErrorHandler/NotFoundRedirectHandler.php | 21 +++++--- .../Config/NotFoundRedirectResolverTest.php | 4 +- .../NotFoundRedirectHandlerTest.php | 13 +++-- 5 files changed, 70 insertions(+), 18 deletions(-) diff --git a/module/Core/src/Config/NotFoundRedirectResolver.php b/module/Core/src/Config/NotFoundRedirectResolver.php index a6a70c31..64837a64 100644 --- a/module/Core/src/Config/NotFoundRedirectResolver.php +++ b/module/Core/src/Config/NotFoundRedirectResolver.php @@ -4,12 +4,20 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Config; +use League\Uri\Uri; use Psr\Http\Message\ResponseInterface; +use Psr\Http\Message\UriInterface; use Shlinkio\Shlink\Core\ErrorHandler\Model\NotFoundType; use Shlinkio\Shlink\Core\Util\RedirectResponseHelperInterface; +use function Functional\compose; +use function str_replace; + class NotFoundRedirectResolver implements NotFoundRedirectResolverInterface { + private const DOMAIN_PLACEHOLDER = '{DOMAIN}'; + private const ORIGINAL_PATH_PLACEHOLDER = '{ORIGINAL_PATH}'; + public function __construct(private RedirectResponseHelperInterface $redirectResponseHelper) { } @@ -17,18 +25,46 @@ class NotFoundRedirectResolver implements NotFoundRedirectResolverInterface public function resolveRedirectResponse( NotFoundType $notFoundType, NotFoundRedirectConfigInterface $config, + UriInterface $currentUri, ): ?ResponseInterface { return match (true) { $notFoundType->isBaseUrl() && $config->hasBaseUrlRedirect() => - // @phpstan-ignore-next-line Create custom PHPStan rule - $this->redirectResponseHelper->buildRedirectResponse($config->baseUrlRedirect()), + $this->redirectResponseHelper->buildRedirectResponse( + // @phpstan-ignore-next-line Create custom PHPStan rule + $this->resolvePlaceholders($currentUri, $config->baseUrlRedirect()), + ), $notFoundType->isRegularNotFound() && $config->hasRegular404Redirect() => - // @phpstan-ignore-next-line Create custom PHPStan rule - $this->redirectResponseHelper->buildRedirectResponse($config->regular404Redirect()), + $this->redirectResponseHelper->buildRedirectResponse( + // @phpstan-ignore-next-line Create custom PHPStan rule + $this->resolvePlaceholders($currentUri, $config->regular404Redirect()), + ), $notFoundType->isInvalidShortUrl() && $config->hasInvalidShortUrlRedirect() => - // @phpstan-ignore-next-line Create custom PHPStan rule - $this->redirectResponseHelper->buildRedirectResponse($config->invalidShortUrlRedirect()), + $this->redirectResponseHelper->buildRedirectResponse( + // @phpstan-ignore-next-line Create custom PHPStan rule + $this->resolvePlaceholders($currentUri, $config->invalidShortUrlRedirect()), + ), default => null, }; } + + private function resolvePlaceholders(UriInterface $currentUri, string $redirectUrl): string + { + $domain = $currentUri->getAuthority(); + $path = $currentUri->getPath(); + $redirectUri = Uri::createFromString($redirectUrl); + + $replacePlaceholders = static fn (callable $modifier) => compose( + static fn (?string $value) => + $value === null ? null : str_replace(self::DOMAIN_PLACEHOLDER, $modifier($domain), $value), + static fn (?string $value) => + $value === null ? null : str_replace(self::ORIGINAL_PATH_PLACEHOLDER, $modifier($path), $value), + ); + $replacePlaceholdersInPath = $replacePlaceholders('\Functional\id'); + $replacePlaceholdersInQuery = $replacePlaceholders('\urlencode'); + + return $redirectUri + ->withPath($replacePlaceholdersInPath($redirectUri->getPath())) + ->withQuery($replacePlaceholdersInQuery($redirectUri->getQuery())) + ->__toString(); + } } diff --git a/module/Core/src/Config/NotFoundRedirectResolverInterface.php b/module/Core/src/Config/NotFoundRedirectResolverInterface.php index ab010d2e..6cbdf702 100644 --- a/module/Core/src/Config/NotFoundRedirectResolverInterface.php +++ b/module/Core/src/Config/NotFoundRedirectResolverInterface.php @@ -5,6 +5,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Config; use Psr\Http\Message\ResponseInterface; +use Psr\Http\Message\UriInterface; use Shlinkio\Shlink\Core\ErrorHandler\Model\NotFoundType; interface NotFoundRedirectResolverInterface @@ -12,5 +13,6 @@ interface NotFoundRedirectResolverInterface public function resolveRedirectResponse( NotFoundType $notFoundType, NotFoundRedirectConfigInterface $config, + UriInterface $currentUri, ): ?ResponseInterface; } diff --git a/module/Core/src/ErrorHandler/NotFoundRedirectHandler.php b/module/Core/src/ErrorHandler/NotFoundRedirectHandler.php index 84918876..4138a72e 100644 --- a/module/Core/src/ErrorHandler/NotFoundRedirectHandler.php +++ b/module/Core/src/ErrorHandler/NotFoundRedirectHandler.php @@ -6,6 +6,7 @@ namespace Shlinkio\Shlink\Core\ErrorHandler; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; +use Psr\Http\Message\UriInterface; use Psr\Http\Server\MiddlewareInterface; use Psr\Http\Server\RequestHandlerInterface; use Shlinkio\Shlink\Core\Config\NotFoundRedirectResolverInterface; @@ -26,19 +27,25 @@ class NotFoundRedirectHandler implements MiddlewareInterface { /** @var NotFoundType $notFoundType */ $notFoundType = $request->getAttribute(NotFoundType::class); - $authority = $request->getUri()->getAuthority(); - $domainSpecificRedirect = $this->resolveDomainSpecificRedirect($authority, $notFoundType); + $currentUri = $request->getUri(); + $domainSpecificRedirect = $this->resolveDomainSpecificRedirect($currentUri, $notFoundType); return $domainSpecificRedirect // If we did not find domain-specific redirects for current domain, we try to fall back to default redirects - ?? $this->redirectResolver->resolveRedirectResponse($notFoundType, $this->redirectOptions) + ?? $this->redirectResolver->resolveRedirectResponse($notFoundType, $this->redirectOptions, $currentUri) // Ultimately, we just call next handler if no domain-specific redirects or default redirects were found ?? $handler->handle($request); } - private function resolveDomainSpecificRedirect(string $authority, NotFoundType $notFoundType): ?ResponseInterface - { - $domain = $this->domainService->findByAuthority($authority); - return $domain === null ? null : $this->redirectResolver->resolveRedirectResponse($notFoundType, $domain); + private function resolveDomainSpecificRedirect( + UriInterface $currentUri, + NotFoundType $notFoundType, + ): ?ResponseInterface { + $domain = $this->domainService->findByAuthority($currentUri->getAuthority()); + if ($domain === null) { + return null; + } + + return $this->redirectResolver->resolveRedirectResponse($notFoundType, $domain, $currentUri); } } diff --git a/module/Core/test/Config/NotFoundRedirectResolverTest.php b/module/Core/test/Config/NotFoundRedirectResolverTest.php index fe482a41..3a4c6476 100644 --- a/module/Core/test/Config/NotFoundRedirectResolverTest.php +++ b/module/Core/test/Config/NotFoundRedirectResolverTest.php @@ -53,7 +53,7 @@ class NotFoundRedirectResolverTest extends TestCase $expectedResp = new Response(); $buildResp = $this->helper->buildRedirectResponse($expectedRedirectTo)->willReturn($expectedResp); - $resp = $this->resolver->resolveRedirectResponse($notFoundType, $this->config); + $resp = $this->resolver->resolveRedirectResponse($notFoundType, $this->config, new Uri()); self::assertSame($expectedResp, $resp); $buildResp->shouldHaveBeenCalledOnce(); @@ -84,7 +84,7 @@ class NotFoundRedirectResolverTest extends TestCase { $notFoundType = $this->notFoundType($this->requestForRoute('foo')); - $result = $this->resolver->resolveRedirectResponse($notFoundType, $this->config); + $result = $this->resolver->resolveRedirectResponse($notFoundType, $this->config, new Uri()); self::assertNull($result); $this->helper->buildRedirectResponse(Argument::cetera())->shouldNotHaveBeenCalled(); diff --git a/module/Core/test/ErrorHandler/NotFoundRedirectHandlerTest.php b/module/Core/test/ErrorHandler/NotFoundRedirectHandlerTest.php index e508a87b..70063764 100644 --- a/module/Core/test/ErrorHandler/NotFoundRedirectHandlerTest.php +++ b/module/Core/test/ErrorHandler/NotFoundRedirectHandlerTest.php @@ -11,6 +11,7 @@ use Prophecy\Argument; use Prophecy\PhpUnit\ProphecyTrait; use Prophecy\Prophecy\ObjectProphecy; use Psr\Http\Message\ServerRequestInterface; +use Psr\Http\Message\UriInterface; use Psr\Http\Server\RequestHandlerInterface; use Shlinkio\Shlink\Core\Config\NotFoundRedirectResolverInterface; use Shlinkio\Shlink\Core\Domain\DomainServiceInterface; @@ -75,6 +76,7 @@ class NotFoundRedirectHandlerTest extends TestCase $resolver->resolveRedirectResponse( Argument::type(NotFoundType::class), Argument::type(NotFoundRedirectOptions::class), + Argument::type(UriInterface::class), )->willReturn(null)->shouldBeCalledOnce(); }]; yield 'non-redirecting domain' => [function (ObjectProphecy $domainService, ObjectProphecy $resolver): void { @@ -84,10 +86,13 @@ class NotFoundRedirectHandlerTest extends TestCase $resolver->resolveRedirectResponse( Argument::type(NotFoundType::class), Argument::type(NotFoundRedirectOptions::class), + Argument::type(UriInterface::class), + )->willReturn(null)->shouldBeCalledOnce(); + $resolver->resolveRedirectResponse( + Argument::type(NotFoundType::class), + Argument::type(Domain::class), + Argument::type(UriInterface::class), )->willReturn(null)->shouldBeCalledOnce(); - $resolver->resolveRedirectResponse(Argument::type(NotFoundType::class), Argument::type(Domain::class)) - ->willReturn(null) - ->shouldBeCalledOnce(); }]; } @@ -100,6 +105,7 @@ class NotFoundRedirectHandlerTest extends TestCase $resolveRedirect = $this->resolver->resolveRedirectResponse( Argument::type(NotFoundType::class), $this->redirectOptions, + Argument::type(UriInterface::class), )->willReturn($expectedResp); $result = $this->middleware->process($this->req, $this->next->reveal()); @@ -120,6 +126,7 @@ class NotFoundRedirectHandlerTest extends TestCase $resolveRedirect = $this->resolver->resolveRedirectResponse( Argument::type(NotFoundType::class), $domain, + Argument::type(UriInterface::class), )->willReturn($expectedResp); $result = $this->middleware->process($this->req, $this->next->reveal()); From b0a8a03f0ad7216152a6e28b4693de7391099534 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 3 Oct 2021 10:35:35 +0200 Subject: [PATCH 44/55] Refactored NotFoundRedirectResolver to remove duplicated lines and non-strict code --- .../src/Config/NotFoundRedirectResolver.php | 27 +++++++++---------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/module/Core/src/Config/NotFoundRedirectResolver.php b/module/Core/src/Config/NotFoundRedirectResolver.php index 64837a64..1d1d4519 100644 --- a/module/Core/src/Config/NotFoundRedirectResolver.php +++ b/module/Core/src/Config/NotFoundRedirectResolver.php @@ -27,24 +27,21 @@ class NotFoundRedirectResolver implements NotFoundRedirectResolverInterface NotFoundRedirectConfigInterface $config, UriInterface $currentUri, ): ?ResponseInterface { - return match (true) { - $notFoundType->isBaseUrl() && $config->hasBaseUrlRedirect() => - $this->redirectResponseHelper->buildRedirectResponse( - // @phpstan-ignore-next-line Create custom PHPStan rule - $this->resolvePlaceholders($currentUri, $config->baseUrlRedirect()), - ), - $notFoundType->isRegularNotFound() && $config->hasRegular404Redirect() => - $this->redirectResponseHelper->buildRedirectResponse( - // @phpstan-ignore-next-line Create custom PHPStan rule - $this->resolvePlaceholders($currentUri, $config->regular404Redirect()), - ), + $urlToRedirectTo = match (true) { + $notFoundType->isBaseUrl() && $config->hasBaseUrlRedirect() => $config->baseUrlRedirect(), + $notFoundType->isRegularNotFound() && $config->hasRegular404Redirect() => $config->regular404Redirect(), $notFoundType->isInvalidShortUrl() && $config->hasInvalidShortUrlRedirect() => - $this->redirectResponseHelper->buildRedirectResponse( - // @phpstan-ignore-next-line Create custom PHPStan rule - $this->resolvePlaceholders($currentUri, $config->invalidShortUrlRedirect()), - ), + $config->invalidShortUrlRedirect(), default => null, }; + + if ($urlToRedirectTo === null) { + return null; + } + + return $this->redirectResponseHelper->buildRedirectResponse( + $this->resolvePlaceholders($currentUri, $urlToRedirectTo), + ); } private function resolvePlaceholders(UriInterface $currentUri, string $redirectUrl): string From 994a28f31da54e6e718b5bd0433529ce2dce0f1b Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 3 Oct 2021 16:45:13 +0200 Subject: [PATCH 45/55] Ensured NotFoundRedirectResolver replaces placeholders from the URL --- module/Core/config/dependencies.config.php | 2 +- .../src/Config/NotFoundRedirectResolver.php | 28 ++++++-- .../Config/NotFoundRedirectResolverTest.php | 64 +++++++++++++++---- 3 files changed, 72 insertions(+), 22 deletions(-) diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index 66d854c3..7c3d7468 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -127,7 +127,7 @@ return [ Util\DoctrineBatchHelper::class => ['em'], Util\RedirectResponseHelper::class => [Options\UrlShortenerOptions::class], - Config\NotFoundRedirectResolver::class => [Util\RedirectResponseHelper::class], + Config\NotFoundRedirectResolver::class => [Util\RedirectResponseHelper::class, 'Logger_Shlink'], Action\RedirectAction::class => [ Service\ShortUrl\ShortUrlResolver::class, diff --git a/module/Core/src/Config/NotFoundRedirectResolver.php b/module/Core/src/Config/NotFoundRedirectResolver.php index 1d1d4519..531254f7 100644 --- a/module/Core/src/Config/NotFoundRedirectResolver.php +++ b/module/Core/src/Config/NotFoundRedirectResolver.php @@ -4,9 +4,11 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Config; +use League\Uri\Exceptions\SyntaxError; use League\Uri\Uri; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\UriInterface; +use Psr\Log\LoggerInterface; use Shlinkio\Shlink\Core\ErrorHandler\Model\NotFoundType; use Shlinkio\Shlink\Core\Util\RedirectResponseHelperInterface; @@ -18,8 +20,10 @@ class NotFoundRedirectResolver implements NotFoundRedirectResolverInterface private const DOMAIN_PLACEHOLDER = '{DOMAIN}'; private const ORIGINAL_PATH_PLACEHOLDER = '{ORIGINAL_PATH}'; - public function __construct(private RedirectResponseHelperInterface $redirectResponseHelper) - { + public function __construct( + private RedirectResponseHelperInterface $redirectResponseHelper, + private LoggerInterface $logger, + ) { } public function resolveRedirectResponse( @@ -48,13 +52,23 @@ class NotFoundRedirectResolver implements NotFoundRedirectResolverInterface { $domain = $currentUri->getAuthority(); $path = $currentUri->getPath(); - $redirectUri = Uri::createFromString($redirectUrl); + try { + $redirectUri = Uri::createFromString($redirectUrl); + } catch (SyntaxError $e) { + $this->logger->warning('It was not possible to parse "{url}" as a valid URL: {e}', [ + 'e' => $e, + 'url' => $redirectUrl, + ]); + return $redirectUrl; + } + + $replacePlaceholderForPattern = static fn (string $pattern, string $replace, callable $modifier) => + static fn (?string $value) => + $value === null ? null : str_replace($modifier($pattern), $modifier($replace), $value); $replacePlaceholders = static fn (callable $modifier) => compose( - static fn (?string $value) => - $value === null ? null : str_replace(self::DOMAIN_PLACEHOLDER, $modifier($domain), $value), - static fn (?string $value) => - $value === null ? null : str_replace(self::ORIGINAL_PATH_PLACEHOLDER, $modifier($path), $value), + $replacePlaceholderForPattern(self::DOMAIN_PLACEHOLDER, $domain, $modifier), + $replacePlaceholderForPattern(self::ORIGINAL_PATH_PLACEHOLDER, $path, $modifier), ); $replacePlaceholdersInPath = $replacePlaceholders('\Functional\id'); $replacePlaceholdersInQuery = $replacePlaceholders('\urlencode'); diff --git a/module/Core/test/Config/NotFoundRedirectResolverTest.php b/module/Core/test/Config/NotFoundRedirectResolverTest.php index 3a4c6476..0dc25768 100644 --- a/module/Core/test/Config/NotFoundRedirectResolverTest.php +++ b/module/Core/test/Config/NotFoundRedirectResolverTest.php @@ -14,9 +14,10 @@ use Prophecy\Argument; use Prophecy\PhpUnit\ProphecyTrait; use Prophecy\Prophecy\ObjectProphecy; use Psr\Http\Message\ServerRequestInterface; +use Psr\Http\Message\UriInterface; use Psr\Http\Server\MiddlewareInterface; +use Psr\Log\NullLogger; use Shlinkio\Shlink\Core\Action\RedirectAction; -use Shlinkio\Shlink\Core\Config\NotFoundRedirectConfigInterface; use Shlinkio\Shlink\Core\Config\NotFoundRedirectResolver; use Shlinkio\Shlink\Core\ErrorHandler\Model\NotFoundType; use Shlinkio\Shlink\Core\Options\NotFoundRedirectOptions; @@ -28,18 +29,11 @@ class NotFoundRedirectResolverTest extends TestCase private NotFoundRedirectResolver $resolver; private ObjectProphecy $helper; - private NotFoundRedirectConfigInterface $config; protected function setUp(): void { $this->helper = $this->prophesize(RedirectResponseHelperInterface::class); - $this->resolver = new NotFoundRedirectResolver($this->helper->reveal()); - - $this->config = new NotFoundRedirectOptions([ - 'invalidShortUrl' => 'invalidShortUrl', - 'regular404' => 'regular404', - 'baseUrl' => 'baseUrl', - ]); + $this->resolver = new NotFoundRedirectResolver($this->helper->reveal(), new NullLogger()); } /** @@ -47,13 +41,15 @@ class NotFoundRedirectResolverTest extends TestCase * @dataProvider provideRedirects */ public function expectedRedirectionIsReturnedDependingOnTheCase( + UriInterface $uri, NotFoundType $notFoundType, + NotFoundRedirectOptions $redirectConfig, string $expectedRedirectTo, ): void { $expectedResp = new Response(); $buildResp = $this->helper->buildRedirectResponse($expectedRedirectTo)->willReturn($expectedResp); - $resp = $this->resolver->resolveRedirectResponse($notFoundType, $this->config, new Uri()); + $resp = $this->resolver->resolveRedirectResponse($notFoundType, $redirectConfig, $uri); self::assertSame($expectedResp, $resp); $buildResp->shouldHaveBeenCalledOnce(); @@ -62,21 +58,61 @@ class NotFoundRedirectResolverTest extends TestCase public function provideRedirects(): iterable { yield 'base URL with trailing slash' => [ - $this->notFoundType(ServerRequestFactory::fromGlobals()->withUri(new Uri('/'))), + $uri = new Uri('/'), + $this->notFoundType(ServerRequestFactory::fromGlobals()->withUri($uri)), + new NotFoundRedirectOptions(['baseUrl' => 'baseUrl']), 'baseUrl', ]; + yield 'base URL with domain placeholder' => [ + $uri = new Uri('https://doma.in'), + $this->notFoundType(ServerRequestFactory::fromGlobals()->withUri($uri)), + new NotFoundRedirectOptions(['baseUrl' => 'https://redirect-here.com/{DOMAIN}']), + 'https://redirect-here.com/doma.in', + ]; + yield 'base URL with domain placeholder in query' => [ + $uri = new Uri('https://doma.in'), + $this->notFoundType(ServerRequestFactory::fromGlobals()->withUri($uri)), + new NotFoundRedirectOptions(['baseUrl' => 'https://redirect-here.com/?domain={DOMAIN}']), + 'https://redirect-here.com/?domain=doma.in', + ]; yield 'base URL without trailing slash' => [ - $this->notFoundType(ServerRequestFactory::fromGlobals()->withUri(new Uri(''))), + $uri = new Uri(''), + $this->notFoundType(ServerRequestFactory::fromGlobals()->withUri($uri)), + new NotFoundRedirectOptions(['baseUrl' => 'baseUrl']), 'baseUrl', ]; yield 'regular 404' => [ - $this->notFoundType(ServerRequestFactory::fromGlobals()->withUri(new Uri('/foo/bar'))), + $uri = new Uri('/foo/bar'), + $this->notFoundType(ServerRequestFactory::fromGlobals()->withUri($uri)), + new NotFoundRedirectOptions(['regular404' => 'regular404']), 'regular404', ]; + yield 'regular 404 with path placeholder in query' => [ + $uri = new Uri('/foo/bar'), + $this->notFoundType(ServerRequestFactory::fromGlobals()->withUri($uri)), + new NotFoundRedirectOptions(['regular404' => 'https://redirect-here.com/?path={ORIGINAL_PATH}']), + 'https://redirect-here.com/?path=%2Ffoo%2Fbar', + ]; + yield 'regular 404 with multiple placeholders' => [ + $uri = new Uri('https://doma.in/foo/bar'), + $this->notFoundType(ServerRequestFactory::fromGlobals()->withUri($uri)), + new NotFoundRedirectOptions([ + 'regular404' => 'https://redirect-here.com/{ORIGINAL_PATH}/{DOMAIN}/?d={DOMAIN}&p={ORIGINAL_PATH}', + ]), + 'https://redirect-here.com//foo/bar/doma.in/?d=doma.in&p=%2Ffoo%2Fbar', // TODO Fix duplicated slash + ]; yield 'invalid short URL' => [ + new Uri('/foo'), $this->notFoundType($this->requestForRoute(RedirectAction::class)), + new NotFoundRedirectOptions(['invalidShortUrl' => 'invalidShortUrl']), 'invalidShortUrl', ]; + yield 'invalid short URL with path placeholder' => [ + new Uri('/foo'), + $this->notFoundType($this->requestForRoute(RedirectAction::class)), + new NotFoundRedirectOptions(['invalidShortUrl' => 'https://redirect-here.com/{ORIGINAL_PATH}']), + 'https://redirect-here.com//foo', // TODO Fix duplicated slash + ]; } /** @test */ @@ -84,7 +120,7 @@ class NotFoundRedirectResolverTest extends TestCase { $notFoundType = $this->notFoundType($this->requestForRoute('foo')); - $result = $this->resolver->resolveRedirectResponse($notFoundType, $this->config, new Uri()); + $result = $this->resolver->resolveRedirectResponse($notFoundType, new NotFoundRedirectOptions(), new Uri()); self::assertNull($result); $this->helper->buildRedirectResponse(Argument::cetera())->shouldNotHaveBeenCalled(); From 69740493b78792a3b0bf828e3a264717f4ae08e7 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 3 Oct 2021 16:47:43 +0200 Subject: [PATCH 46/55] Updated changelog --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9d253e85..d8d5b37b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,12 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this The new `forwardQuery=true|false` param can be provided during short URL creation or edition, via REST API or CLI command, allowing to override the default behavior which makes the query string to always be forwarded. +* [#1105](https://github.com/shlinkio/shlink/issues/1105) Added support to define placeholders on not-found redirects, so that the redirected URL receives the originally visited path and/or domain. + + Currently, `{DOMAIN}` and `{ORIGINAL_PATH}` placeholders are supported, and they can be used both in the redirected URL's path or query. + + When they are used in the query, the values are URL encoded. + ### Changed * [#1142](https://github.com/shlinkio/shlink/issues/1142) Replaced `doctrine/cache` package with `symfony/cache`. * [#1157](https://github.com/shlinkio/shlink/issues/1157) All routes now support CORS, not only rest ones. From 952648185cbf113092d828095effe615870ed6c4 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 3 Oct 2021 16:48:39 +0200 Subject: [PATCH 47/55] Removed duplicated space --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d8d5b37b..7e1542ef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,7 +15,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this The new `forwardQuery=true|false` param can be provided during short URL creation or edition, via REST API or CLI command, allowing to override the default behavior which makes the query string to always be forwarded. -* [#1105](https://github.com/shlinkio/shlink/issues/1105) Added support to define placeholders on not-found redirects, so that the redirected URL receives the originally visited path and/or domain. +* [#1105](https://github.com/shlinkio/shlink/issues/1105) Added support to define placeholders on not-found redirects, so that the redirected URL receives the originally visited path and/or domain. Currently, `{DOMAIN}` and `{ORIGINAL_PATH}` placeholders are supported, and they can be used both in the redirected URL's path or query. From 95cf0d86bc8c59ed832ded501a52b187ee6a1947 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 8 Oct 2021 18:52:17 +0200 Subject: [PATCH 48/55] Added support to provide redis sentinel when using redis cache --- composer.json | 4 ++-- config/autoload/redis.global.php | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/composer.json b/composer.json index 2e920760..b00900e7 100644 --- a/composer.json +++ b/composer.json @@ -46,11 +46,11 @@ "predis/predis": "^1.1", "pugx/shortid-php": "^0.7", "ramsey/uuid": "^3.9", - "shlinkio/shlink-common": "dev-main#3eacc46 as 4.0", + "shlinkio/shlink-common": "^4.0", "shlinkio/shlink-config": "^1.2", "shlinkio/shlink-event-dispatcher": "^2.1", "shlinkio/shlink-importer": "^2.3.1", - "shlinkio/shlink-installer": "dev-develop#07f1ac8 as 6.2", + "shlinkio/shlink-installer": "dev-develop#2f87995 as 6.2", "shlinkio/shlink-ip-geolocation": "^2.0", "symfony/console": "^5.3", "symfony/filesystem": "^5.3", diff --git a/config/autoload/redis.global.php b/config/autoload/redis.global.php index a47d814c..22101b65 100644 --- a/config/autoload/redis.global.php +++ b/config/autoload/redis.global.php @@ -13,6 +13,7 @@ return (static function (): array { 'cache' => [ 'redis' => [ 'servers' => $redisServers, + 'sentinel_service' => env('REDIS_SENTINEL_SERVICE'), ], ], ], From 3ffe5304619f85ba99d59066326393d08879ef0a Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 8 Oct 2021 18:52:53 +0200 Subject: [PATCH 49/55] Updated changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7e1542ef..eef5ab39 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this When they are used in the query, the values are URL encoded. +* [#1119](https://github.com/shlinkio/shlink/issues/1119) Added support to provide redis sentinel when using redis cache. + ### Changed * [#1142](https://github.com/shlinkio/shlink/issues/1142) Replaced `doctrine/cache` package with `symfony/cache`. * [#1157](https://github.com/shlinkio/shlink/issues/1157) All routes now support CORS, not only rest ones. From c718b94937c0acc372e20ec9b7a5934236ec88f6 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 9 Oct 2021 10:35:10 +0200 Subject: [PATCH 50/55] Fixed crash when notifying orphan visits to a webhook --- .../EventDispatcher/NotifyVisitToWebHooks.php | 21 +++++------- .../NotifyVisitToWebHooksTest.php | 34 +++++++++++++------ 2 files changed, 33 insertions(+), 22 deletions(-) diff --git a/module/Core/src/EventDispatcher/NotifyVisitToWebHooks.php b/module/Core/src/EventDispatcher/NotifyVisitToWebHooks.php index dcd69b21..ad8381be 100644 --- a/module/Core/src/EventDispatcher/NotifyVisitToWebHooks.php +++ b/module/Core/src/EventDispatcher/NotifyVisitToWebHooks.php @@ -4,7 +4,6 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\EventDispatcher; -use Closure; use Doctrine\ORM\EntityManagerInterface; use Fig\Http\Message\RequestMethodInterface; use GuzzleHttp\ClientInterface; @@ -20,7 +19,6 @@ use Shlinkio\Shlink\Core\Options\AppOptions; use Throwable; use function Functional\map; -use function Functional\partial_left; class NotifyVisitToWebHooks { @@ -61,15 +59,16 @@ class NotifyVisitToWebHooks private function buildRequestOptions(Visit $visit): array { + $payload = ['visit' => $visit->jsonSerialize()]; + $shortUrl = $visit->getShortUrl(); + if ($shortUrl !== null) { + $payload['shortUrl'] = $this->transformer->transform($shortUrl); + } + return [ RequestOptions::TIMEOUT => 10, - RequestOptions::HEADERS => [ - 'User-Agent' => (string) $this->appOptions, - ], - RequestOptions::JSON => [ - 'shortUrl' => $this->transformer->transform($visit->getShortUrl()), - 'visit' => $visit->jsonSerialize(), - ], + RequestOptions::JSON => $payload, + RequestOptions::HEADERS => ['User-Agent' => $this->appOptions->__toString()], ]; } @@ -78,13 +77,11 @@ class NotifyVisitToWebHooks */ private function performRequests(array $requestOptions, string $visitId): array { - $logWebhookFailure = Closure::fromCallable([$this, 'logWebhookFailure']); - return map( $this->webhooks, fn (string $webhook): PromiseInterface => $this->httpClient ->requestAsync(RequestMethodInterface::METHOD_POST, $webhook, $requestOptions) - ->otherwise(partial_left($logWebhookFailure, $webhook, $visitId)), + ->otherwise(fn (Throwable $e) => $this->logWebhookFailure($webhook, $visitId, $e)), ); } diff --git a/module/Core/test/EventDispatcher/NotifyVisitToWebHooksTest.php b/module/Core/test/EventDispatcher/NotifyVisitToWebHooksTest.php index fcd97d2d..9e884753 100644 --- a/module/Core/test/EventDispatcher/NotifyVisitToWebHooksTest.php +++ b/module/Core/test/EventDispatcher/NotifyVisitToWebHooksTest.php @@ -75,34 +75,39 @@ class NotifyVisitToWebHooksTest extends TestCase $requestAsync->shouldNotHaveBeenCalled(); } - /** @test */ - public function expectedRequestsArePerformedToWebhooks(): void + /** + * @test + * @dataProvider provideVisits + */ + public function expectedRequestsArePerformedToWebhooks(Visit $visit, array $expectedResponseKeys): void { $webhooks = ['foo', 'invalid', 'bar', 'baz']; $invalidWebhooks = ['invalid', 'baz']; - $find = $this->em->find(Visit::class, '1')->willReturn( - Visit::forValidShortUrl(ShortUrl::createEmpty(), Visitor::emptyInstance()), - ); + $find = $this->em->find(Visit::class, '1')->willReturn($visit); $requestAsync = $this->httpClient->requestAsync( RequestMethodInterface::METHOD_POST, Argument::type('string'), - Argument::that(function (array $requestOptions) { + Argument::that(function (array $requestOptions) use ($expectedResponseKeys) { Assert::assertArrayHasKey(RequestOptions::HEADERS, $requestOptions); Assert::assertArrayHasKey(RequestOptions::JSON, $requestOptions); Assert::assertArrayHasKey(RequestOptions::TIMEOUT, $requestOptions); Assert::assertEquals($requestOptions[RequestOptions::TIMEOUT], 10); Assert::assertEquals($requestOptions[RequestOptions::HEADERS], ['User-Agent' => 'Shlink:v1.2.3']); - Assert::assertArrayHasKey('shortUrl', $requestOptions[RequestOptions::JSON]); - Assert::assertArrayHasKey('visit', $requestOptions[RequestOptions::JSON]); + + $json = $requestOptions[RequestOptions::JSON]; + Assert::assertCount(count($expectedResponseKeys), $json); + foreach ($expectedResponseKeys as $key) { + Assert::assertArrayHasKey($key, $json); + } return $requestOptions; }), )->will(function (array $args) use ($invalidWebhooks) { [, $webhook] = $args; - $e = new Exception(''); + $shouldReject = contains($invalidWebhooks, $webhook); - return contains($invalidWebhooks, $webhook) ? new RejectedPromise($e) : new FulfilledPromise(''); + return $shouldReject ? new RejectedPromise(new Exception('')) : new FulfilledPromise(''); }); $logWarning = $this->logger->warning( 'Failed to notify visit with id "{visitId}" to webhook "{webhook}". {e}', @@ -122,6 +127,15 @@ class NotifyVisitToWebHooksTest extends TestCase $logWarning->shouldHaveBeenCalledTimes(count($invalidWebhooks)); } + public function provideVisits(): iterable + { + yield 'regular visit' => [ + Visit::forValidShortUrl(ShortUrl::createEmpty(), Visitor::emptyInstance()), + ['shortUrl', 'visit'], + ]; + yield 'orphan visit' => [Visit::forBasePath(Visitor::emptyInstance()), ['visit'],]; + } + private function createListener(array $webhooks): NotifyVisitToWebHooks { return new NotifyVisitToWebHooks( From d16fda3f1698f952cad60b83fedaecc357c3eaeb Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 9 Oct 2021 10:53:21 +0200 Subject: [PATCH 51/55] Added option to send orphan visits to webhooks --- config/autoload/redirects.global.php | 9 +++++ config/autoload/url-shortener.global.php | 8 ---- config/autoload/webhooks.global.php | 19 +++++++++ module/Core/config/dependencies.config.php | 2 + .../Core/config/event_dispatcher.config.php | 2 +- .../EventDispatcher/NotifyVisitToWebHooks.php | 12 ++++-- module/Core/src/Options/WebhookOptions.php | 40 +++++++++++++++++++ .../NotifyVisitToWebHooksTest.php | 25 +++++++++++- 8 files changed, 102 insertions(+), 15 deletions(-) create mode 100644 config/autoload/webhooks.global.php create mode 100644 module/Core/src/Options/WebhookOptions.php diff --git a/config/autoload/redirects.global.php b/config/autoload/redirects.global.php index 0707f1e4..339ca27d 100644 --- a/config/autoload/redirects.global.php +++ b/config/autoload/redirects.global.php @@ -4,6 +4,9 @@ declare(strict_types=1); use function Shlinkio\Shlink\Common\env; +use const Shlinkio\Shlink\DEFAULT_REDIRECT_CACHE_LIFETIME; +use const Shlinkio\Shlink\DEFAULT_REDIRECT_STATUS_CODE; + return [ 'not_found_redirects' => [ @@ -12,4 +15,10 @@ return [ 'base_url' => env('BASE_URL_REDIRECT_TO'), ], + 'url_shortener' => [ + // TODO Move these options to their own config namespace. Maybe "redirects". + 'redirect_status_code' => (int) env('REDIRECT_STATUS_CODE', DEFAULT_REDIRECT_STATUS_CODE), + 'redirect_cache_lifetime' => (int) env('REDIRECT_CACHE_LIFETIME', DEFAULT_REDIRECT_CACHE_LIFETIME), + ], + ]; diff --git a/config/autoload/url-shortener.global.php b/config/autoload/url-shortener.global.php index 432c63e1..ae6bfe84 100644 --- a/config/autoload/url-shortener.global.php +++ b/config/autoload/url-shortener.global.php @@ -4,13 +4,10 @@ declare(strict_types=1); use function Shlinkio\Shlink\Common\env; -use const Shlinkio\Shlink\DEFAULT_REDIRECT_CACHE_LIFETIME; -use const Shlinkio\Shlink\DEFAULT_REDIRECT_STATUS_CODE; use const Shlinkio\Shlink\DEFAULT_SHORT_CODES_LENGTH; use const Shlinkio\Shlink\MIN_SHORT_CODES_LENGTH; return (static function (): array { - $webhooks = env('VISITS_WEBHOOKS'); $shortCodesLength = (int) env('DEFAULT_SHORT_CODES_LENGTH', DEFAULT_SHORT_CODES_LENGTH); $shortCodesLength = $shortCodesLength < MIN_SHORT_CODES_LENGTH ? MIN_SHORT_CODES_LENGTH : $shortCodesLength; $useHttps = env('USE_HTTPS'); // Deprecated. For v3, set this to true by default, instead of null @@ -24,14 +21,9 @@ return (static function (): array { 'hostname' => env('DEFAULT_DOMAIN', env('SHORT_DOMAIN_HOST', '')), ], 'validate_url' => (bool) env('VALIDATE_URLS', false), // Deprecated - 'visits_webhooks' => $webhooks === null ? [] : explode(',', $webhooks), 'default_short_codes_length' => $shortCodesLength, 'auto_resolve_titles' => (bool) env('AUTO_RESOLVE_TITLES', false), 'append_extra_path' => (bool) env('REDIRECT_APPEND_EXTRA_PATH', false), - - // TODO Move these two options to their own config namespace. Maybe "redirects". - 'redirect_status_code' => (int) env('REDIRECT_STATUS_CODE', DEFAULT_REDIRECT_STATUS_CODE), - 'redirect_cache_lifetime' => (int) env('REDIRECT_CACHE_LIFETIME', DEFAULT_REDIRECT_CACHE_LIFETIME), ], ]; diff --git a/config/autoload/webhooks.global.php b/config/autoload/webhooks.global.php new file mode 100644 index 00000000..585d3eb2 --- /dev/null +++ b/config/autoload/webhooks.global.php @@ -0,0 +1,19 @@ + [ + // TODO Move these options to their own config namespace + 'visits_webhooks' => $webhooks === null ? [] : explode(',', $webhooks), + 'notify_orphan_visits_to_webhooks' => (bool) env('NOTIFY_ORPHAN_VISITS_TO_WEBHOOKS', false), + ], + + ]; +})(); diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index 7c3d7468..16b84819 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -26,6 +26,7 @@ return [ Options\UrlShortenerOptions::class => ConfigAbstractFactory::class, Options\TrackingOptions::class => ConfigAbstractFactory::class, Options\QrCodeOptions::class => ConfigAbstractFactory::class, + Options\WebhookOptions::class => ConfigAbstractFactory::class, Service\UrlShortener::class => ConfigAbstractFactory::class, Service\ShortUrlService::class => ConfigAbstractFactory::class, @@ -88,6 +89,7 @@ return [ Options\UrlShortenerOptions::class => ['config.url_shortener'], Options\TrackingOptions::class => ['config.tracking'], Options\QrCodeOptions::class => ['config.qr_codes'], + Options\WebhookOptions::class => ['config.url_shortener'], // TODO This config is currently under url_shortener Service\UrlShortener::class => [ ShortUrl\Helper\ShortUrlTitleResolutionHelper::class, diff --git a/module/Core/config/event_dispatcher.config.php b/module/Core/config/event_dispatcher.config.php index bddd59f5..5256bc92 100644 --- a/module/Core/config/event_dispatcher.config.php +++ b/module/Core/config/event_dispatcher.config.php @@ -58,7 +58,7 @@ return [ 'httpClient', 'em', 'Logger_Shlink', - 'config.url_shortener.visits_webhooks', + Options\WebhookOptions::class, ShortUrl\Transformer\ShortUrlDataTransformer::class, Options\AppOptions::class, ], diff --git a/module/Core/src/EventDispatcher/NotifyVisitToWebHooks.php b/module/Core/src/EventDispatcher/NotifyVisitToWebHooks.php index ad8381be..b5c2e501 100644 --- a/module/Core/src/EventDispatcher/NotifyVisitToWebHooks.php +++ b/module/Core/src/EventDispatcher/NotifyVisitToWebHooks.php @@ -16,6 +16,7 @@ use Shlinkio\Shlink\Common\Rest\DataTransformerInterface; use Shlinkio\Shlink\Core\Entity\Visit; use Shlinkio\Shlink\Core\EventDispatcher\Event\VisitLocated; use Shlinkio\Shlink\Core\Options\AppOptions; +use Shlinkio\Shlink\Core\Options\WebhookOptions; use Throwable; use function Functional\map; @@ -26,8 +27,7 @@ class NotifyVisitToWebHooks private ClientInterface $httpClient, private EntityManagerInterface $em, private LoggerInterface $logger, - /** @var string[] */ - private array $webhooks, + private WebhookOptions $webhookOptions, private DataTransformerInterface $transformer, private AppOptions $appOptions, ) { @@ -35,7 +35,7 @@ class NotifyVisitToWebHooks public function __invoke(VisitLocated $shortUrlLocated): void { - if (empty($this->webhooks)) { + if (! $this->webhookOptions->hasWebhooks()) { return; } @@ -50,6 +50,10 @@ class NotifyVisitToWebHooks return; } + if ($visit->isOrphan() && ! $this->webhookOptions->notifyOrphanVisits()) { + return; + } + $requestOptions = $this->buildRequestOptions($visit); $requestPromises = $this->performRequests($requestOptions, $visitId); @@ -78,7 +82,7 @@ class NotifyVisitToWebHooks private function performRequests(array $requestOptions, string $visitId): array { return map( - $this->webhooks, + $this->webhookOptions->webhooks(), fn (string $webhook): PromiseInterface => $this->httpClient ->requestAsync(RequestMethodInterface::METHOD_POST, $webhook, $requestOptions) ->otherwise(fn (Throwable $e) => $this->logWebhookFailure($webhook, $visitId, $e)), diff --git a/module/Core/src/Options/WebhookOptions.php b/module/Core/src/Options/WebhookOptions.php new file mode 100644 index 00000000..c86789b2 --- /dev/null +++ b/module/Core/src/Options/WebhookOptions.php @@ -0,0 +1,40 @@ +visitsWebhooks; + } + + public function hasWebhooks(): bool + { + return ! empty($this->visitsWebhooks); + } + + protected function setVisitsWebhooks(array $visitsWebhooks): void + { + $this->visitsWebhooks = $visitsWebhooks; + } + + public function notifyOrphanVisits(): bool + { + return $this->notifyOrphanVisitsToWebhooks; + } + + protected function setNotifyOrphanVisitsToWebhooks(bool $notifyOrphanVisitsToWebhooks): void + { + $this->notifyOrphanVisitsToWebhooks = $notifyOrphanVisitsToWebhooks; + } +} diff --git a/module/Core/test/EventDispatcher/NotifyVisitToWebHooksTest.php b/module/Core/test/EventDispatcher/NotifyVisitToWebHooksTest.php index 9e884753..99609bb4 100644 --- a/module/Core/test/EventDispatcher/NotifyVisitToWebHooksTest.php +++ b/module/Core/test/EventDispatcher/NotifyVisitToWebHooksTest.php @@ -23,6 +23,7 @@ use Shlinkio\Shlink\Core\EventDispatcher\Event\VisitLocated; use Shlinkio\Shlink\Core\EventDispatcher\NotifyVisitToWebHooks; use Shlinkio\Shlink\Core\Model\Visitor; use Shlinkio\Shlink\Core\Options\AppOptions; +use Shlinkio\Shlink\Core\Options\WebhookOptions; use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlStringifier; use Shlinkio\Shlink\Core\ShortUrl\Transformer\ShortUrlDataTransformer; @@ -75,6 +76,24 @@ class NotifyVisitToWebHooksTest extends TestCase $requestAsync->shouldNotHaveBeenCalled(); } + /** @test */ + public function orphanVisitDoesNotPerformAnyRequestWhenDisabled(): void + { + $find = $this->em->find(Visit::class, '1')->willReturn(Visit::forBasePath(Visitor::emptyInstance())); + $requestAsync = $this->httpClient->requestAsync( + RequestMethodInterface::METHOD_POST, + Argument::type('string'), + Argument::type('array'), + )->willReturn(new FulfilledPromise('')); + $logWarning = $this->logger->warning(Argument::cetera()); + + $this->createListener(['foo', 'bar'], false)(new VisitLocated('1')); + + $find->shouldHaveBeenCalledOnce(); + $logWarning->shouldNotHaveBeenCalled(); + $requestAsync->shouldNotHaveBeenCalled(); + } + /** * @test * @dataProvider provideVisits @@ -136,13 +155,15 @@ class NotifyVisitToWebHooksTest extends TestCase yield 'orphan visit' => [Visit::forBasePath(Visitor::emptyInstance()), ['visit'],]; } - private function createListener(array $webhooks): NotifyVisitToWebHooks + private function createListener(array $webhooks, bool $notifyOrphanVisits = true): NotifyVisitToWebHooks { return new NotifyVisitToWebHooks( $this->httpClient->reveal(), $this->em->reveal(), $this->logger->reveal(), - $webhooks, + new WebhookOptions( + ['visits_webhooks' => $webhooks, 'notify_orphan_visits_to_webhooks' => $notifyOrphanVisits], + ), new ShortUrlDataTransformer(new ShortUrlStringifier([])), new AppOptions(['name' => 'Shlink', 'version' => '1.2.3']), ); From 483bdddb18583d5a0a93905817579f327ada3104 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 9 Oct 2021 12:35:45 +0200 Subject: [PATCH 52/55] Updated to installer version with support for orphan visits webhooks --- CHANGELOG.md | 3 +++ composer.json | 2 +- config/autoload/installer.global.php | 1 + 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index eef5ab39..d17a5b8f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this When they are used in the query, the values are URL encoded. * [#1119](https://github.com/shlinkio/shlink/issues/1119) Added support to provide redis sentinel when using redis cache. +* [#1016](https://github.com/shlinkio/shlink/issues/1016) Added new option to send orphan visits to webhooks, via env var or installer tool. + + The option is disabled by default, as the payload is backwards incompatible. You will need to adapt your webhooks to treat the `shortUrl` property as optional before enabling this option. ### Changed * [#1142](https://github.com/shlinkio/shlink/issues/1142) Replaced `doctrine/cache` package with `symfony/cache`. diff --git a/composer.json b/composer.json index b00900e7..694cb399 100644 --- a/composer.json +++ b/composer.json @@ -50,7 +50,7 @@ "shlinkio/shlink-config": "^1.2", "shlinkio/shlink-event-dispatcher": "^2.1", "shlinkio/shlink-importer": "^2.3.1", - "shlinkio/shlink-installer": "dev-develop#2f87995 as 6.2", + "shlinkio/shlink-installer": "dev-develop#b45a340 as 6.2", "shlinkio/shlink-ip-geolocation": "^2.0", "symfony/console": "^5.3", "symfony/filesystem": "^5.3", diff --git a/config/autoload/installer.global.php b/config/autoload/installer.global.php index fe9f3f5d..e89b382d 100644 --- a/config/autoload/installer.global.php +++ b/config/autoload/installer.global.php @@ -24,6 +24,7 @@ return [ Option\UrlShortener\ShortDomainSchemaConfigOption::class, Option\UrlShortener\ValidateUrlConfigOption::class, Option\Visit\VisitsWebhooksConfigOption::class, + Option\Visit\OrphanVisitsWebhooksConfigOption::class, Option\Redirect\BaseUrlRedirectConfigOption::class, Option\Redirect\InvalidShortUrlRedirectConfigOption::class, Option\Redirect\Regular404RedirectConfigOption::class, From 14ba11e1ab4e4d73e655ec980f511d8fda606a94 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 9 Oct 2021 12:36:37 +0200 Subject: [PATCH 53/55] Enhanced changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d17a5b8f..c30406e2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,7 +22,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this When they are used in the query, the values are URL encoded. * [#1119](https://github.com/shlinkio/shlink/issues/1119) Added support to provide redis sentinel when using redis cache. -* [#1016](https://github.com/shlinkio/shlink/issues/1016) Added new option to send orphan visits to webhooks, via env var or installer tool. +* [#1016](https://github.com/shlinkio/shlink/issues/1016) Added new option to send orphan visits to webhooks, via `NOTIFY_ORPHAN_VISITS_TO_WEBHOOKS` env var or installer tool. The option is disabled by default, as the payload is backwards incompatible. You will need to adapt your webhooks to treat the `shortUrl` property as optional before enabling this option. From ed1d886f01988efd220c50a3820008c59c865e21 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 10 Oct 2021 22:00:22 +0200 Subject: [PATCH 54/55] Added option to disable tracking based on IP address patterns --- composer.json | 1 + config/autoload/tracking.global.php | 3 + module/Core/src/Options/TrackingOptions.php | 29 ++++++++ module/Core/src/Visit/RequestTracker.php | 68 ++++++++++++++++--- module/Core/test/Visit/RequestTrackerTest.php | 18 ++++- 5 files changed, 107 insertions(+), 12 deletions(-) diff --git a/composer.json b/composer.json index 694cb399..f117f3d7 100644 --- a/composer.json +++ b/composer.json @@ -46,6 +46,7 @@ "predis/predis": "^1.1", "pugx/shortid-php": "^0.7", "ramsey/uuid": "^3.9", + "rlanvin/php-ip": "3.0.0-rc2", "shlinkio/shlink-common": "^4.0", "shlinkio/shlink-config": "^1.2", "shlinkio/shlink-event-dispatcher": "^2.1", diff --git a/config/autoload/tracking.global.php b/config/autoload/tracking.global.php index 5ef0eaca..26fe4639 100644 --- a/config/autoload/tracking.global.php +++ b/config/autoload/tracking.global.php @@ -28,6 +28,9 @@ return [ // If true, the user agent will not be tracked 'disable_ua_tracking' => (bool) env('DISABLE_UA_TRACKING', false), + + // A list of IP addresses, patterns or CIDR blocks from which tracking is disabled by default + 'disable_tracking_from' => env('DISABLE_TRACKING_FROM'), ], ]; diff --git a/module/Core/src/Options/TrackingOptions.php b/module/Core/src/Options/TrackingOptions.php index 98e09085..db74b61b 100644 --- a/module/Core/src/Options/TrackingOptions.php +++ b/module/Core/src/Options/TrackingOptions.php @@ -6,6 +6,10 @@ namespace Shlinkio\Shlink\Core\Options; use Laminas\Stdlib\AbstractOptions; +use function array_key_exists; +use function explode; +use function is_array; + class TrackingOptions extends AbstractOptions { private bool $anonymizeRemoteAddr = true; @@ -15,6 +19,7 @@ class TrackingOptions extends AbstractOptions private bool $disableIpTracking = false; private bool $disableReferrerTracking = false; private bool $disableUaTracking = false; + private array $disableTrackingFrom = []; public function anonymizeRemoteAddr(): bool { @@ -41,6 +46,11 @@ class TrackingOptions extends AbstractOptions return $this->disableTrackParam; } + public function queryHasDisableTrackParam(array $query): bool + { + return $this->disableTrackParam !== null && array_key_exists($this->disableTrackParam, $query); + } + protected function setDisableTrackParam(?string $disableTrackParam): void { $this->disableTrackParam = $disableTrackParam; @@ -85,4 +95,23 @@ class TrackingOptions extends AbstractOptions { $this->disableUaTracking = $disableUaTracking; } + + public function disableTrackingFrom(): array + { + return $this->disableTrackingFrom; + } + + public function hasDisableTrackingFrom(): bool + { + return ! empty($this->disableTrackingFrom); + } + + protected function setDisableTrackingFrom(string|array|null $disableTrackingFrom): void + { + if (is_array($disableTrackingFrom)) { + $this->disableTrackingFrom = $disableTrackingFrom; + } else { + $this->disableTrackingFrom = $disableTrackingFrom === null ? [] : explode(',', $disableTrackingFrom); + } + } } diff --git a/module/Core/src/Visit/RequestTracker.php b/module/Core/src/Visit/RequestTracker.php index 3e5bfb51..eee75ea4 100644 --- a/module/Core/src/Visit/RequestTracker.php +++ b/module/Core/src/Visit/RequestTracker.php @@ -5,14 +5,21 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Visit; use Fig\Http\Message\RequestMethodInterface; +use InvalidArgumentException; use Mezzio\Router\Middleware\ImplicitHeadMiddleware; +use PhpIP\IP; use Psr\Http\Message\ServerRequestInterface; +use Shlinkio\Shlink\Common\Middleware\IpAddressMiddlewareFactory; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\ErrorHandler\Model\NotFoundType; use Shlinkio\Shlink\Core\Model\Visitor; use Shlinkio\Shlink\Core\Options\TrackingOptions; -use function array_key_exists; +use function explode; +use function Functional\map; +use function Functional\some; +use function implode; +use function str_contains; class RequestTracker implements RequestTrackerInterface, RequestMethodInterface { @@ -37,24 +44,63 @@ class RequestTracker implements RequestTrackerInterface, RequestMethodInterface $notFoundType = $request->getAttribute(NotFoundType::class); $visitor = Visitor::fromRequest($request); - if ($notFoundType?->isBaseUrl()) { - $this->visitsTracker->trackBaseUrlVisit($visitor); - } elseif ($notFoundType?->isRegularNotFound()) { - $this->visitsTracker->trackRegularNotFoundVisit($visitor); - } elseif ($notFoundType?->isInvalidShortUrl()) { - $this->visitsTracker->trackInvalidShortUrlVisit($visitor); - } + match (true) { // @phpstan-ignore-line + $notFoundType?->isBaseUrl() => $this->visitsTracker->trackBaseUrlVisit($visitor), + $notFoundType?->isRegularNotFound() => $this->visitsTracker->trackRegularNotFoundVisit($visitor), + $notFoundType?->isInvalidShortUrl() => $this->visitsTracker->trackInvalidShortUrlVisit($visitor), + }; } private function shouldTrackRequest(ServerRequestInterface $request): bool { - $query = $request->getQueryParams(); - $disableTrackParam = $this->trackingOptions->getDisableTrackParam(); $forwardedMethod = $request->getAttribute(ImplicitHeadMiddleware::FORWARDED_HTTP_METHOD_ATTRIBUTE); if ($forwardedMethod === self::METHOD_HEAD) { return false; } - return $disableTrackParam === null || ! array_key_exists($disableTrackParam, $query); + $remoteAddr = $request->getAttribute(IpAddressMiddlewareFactory::REQUEST_ATTR); + if ($this->shouldDisableTrackingFromAddress($remoteAddr)) { + return false; + } + + $query = $request->getQueryParams(); + return ! $this->trackingOptions->queryHasDisableTrackParam($query); + } + + private function shouldDisableTrackingFromAddress(?string $remoteAddr): bool + { + if ($remoteAddr === null || ! $this->trackingOptions->hasDisableTrackingFrom()) { + return false; + } + + try { + $ip = IP::create($remoteAddr); + } catch (InvalidArgumentException) { + return false; + } + + $remoteAddrParts = explode('.', $remoteAddr); + $disableTrackingFrom = $this->trackingOptions->disableTrackingFrom(); + + return some($disableTrackingFrom, function (string $value) use ($ip, $remoteAddrParts): bool { + try { + return match (true) { + str_contains($value, '*') => $ip->matches($this->parseValueWithWildcards($value, $remoteAddrParts)), + str_contains($value, '/') => $ip->isIn($value), + default => $ip->matches($value), + }; + } catch (InvalidArgumentException) { + return false; + } + }); + } + + private function parseValueWithWildcards(string $value, array $remoteAddrParts): string + { + // Replace wildcard parts with the corresponding ones from the remote address + return implode('.', map( + explode('.', $value), + fn (string $part, int $index) => $part === '*' ? $remoteAddrParts[$index] : $part, + )); } } diff --git a/module/Core/test/Visit/RequestTrackerTest.php b/module/Core/test/Visit/RequestTrackerTest.php index 46faf9fd..144087ad 100644 --- a/module/Core/test/Visit/RequestTrackerTest.php +++ b/module/Core/test/Visit/RequestTrackerTest.php @@ -12,6 +12,7 @@ use Prophecy\Argument; use Prophecy\PhpUnit\ProphecyTrait; use Prophecy\Prophecy\ObjectProphecy; use Psr\Http\Message\ServerRequestInterface; +use Shlinkio\Shlink\Common\Middleware\IpAddressMiddlewareFactory; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\ErrorHandler\Model\NotFoundType; use Shlinkio\Shlink\Core\Model\Visitor; @@ -37,7 +38,10 @@ class RequestTrackerTest extends TestCase $this->requestTracker = new RequestTracker( $this->visitsTracker->reveal(), - new TrackingOptions(['disable_track_param' => 'foobar']), + new TrackingOptions([ + 'disable_track_param' => 'foobar', + 'disable_tracking_from' => ['80.90.100.110', '192.168.10.0/24', '1.2.*.*'], + ]), ); $this->request = ServerRequestFactory::fromGlobals()->withAttribute( @@ -69,6 +73,18 @@ class RequestTrackerTest extends TestCase yield 'disable track param as null' => [ ServerRequestFactory::fromGlobals()->withQueryParams(['foobar' => null]), ]; + yield 'exact remote address' => [ServerRequestFactory::fromGlobals()->withAttribute( + IpAddressMiddlewareFactory::REQUEST_ATTR, + '80.90.100.110', + )]; + yield 'matching wildcard remote address' => [ServerRequestFactory::fromGlobals()->withAttribute( + IpAddressMiddlewareFactory::REQUEST_ATTR, + '1.2.3.4', + )]; + yield 'matching CIDR block remote address' => [ServerRequestFactory::fromGlobals()->withAttribute( + IpAddressMiddlewareFactory::REQUEST_ATTR, + '192.168.10.100', + )]; } /** @test */ From ceb642b74555aaeaeb50fc80ef243715892933b4 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 10 Oct 2021 22:31:26 +0200 Subject: [PATCH 55/55] Updated to latest installer and changelog --- CHANGELOG.md | 6 +++++- composer.json | 4 ++-- config/autoload/installer.global.php | 1 + 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c30406e2..227b60e3 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.9.0] - 2021-10-10 ### Added * [#1015](https://github.com/shlinkio/shlink/issues/1015) Shlink now accepts configuration via env vars even when not using docker. @@ -26,6 +26,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this The option is disabled by default, as the payload is backwards incompatible. You will need to adapt your webhooks to treat the `shortUrl` property as optional before enabling this option. +* [#1104](https://github.com/shlinkio/shlink/issues/1104) Added ability to disable tracking based on IP addresses. + + IP addresses can be provided in the form of fixed addresses, CIDR blocks, or wildcard patterns (192.168.*.*). + ### Changed * [#1142](https://github.com/shlinkio/shlink/issues/1142) Replaced `doctrine/cache` package with `symfony/cache`. * [#1157](https://github.com/shlinkio/shlink/issues/1157) All routes now support CORS, not only rest ones. diff --git a/composer.json b/composer.json index f117f3d7..aea53f62 100644 --- a/composer.json +++ b/composer.json @@ -51,7 +51,7 @@ "shlinkio/shlink-config": "^1.2", "shlinkio/shlink-event-dispatcher": "^2.1", "shlinkio/shlink-importer": "^2.3.1", - "shlinkio/shlink-installer": "dev-develop#b45a340 as 6.2", + "shlinkio/shlink-installer": "^6.2", "shlinkio/shlink-ip-geolocation": "^2.0", "symfony/console": "^5.3", "symfony/filesystem": "^5.3", @@ -64,7 +64,7 @@ "devster/ubench": "^2.1", "dms/phpunit-arraysubset-asserts": "^0.3.0", "eaglewu/swoole-ide-helper": "dev-master", - "infection/infection": "^0.24.0", + "infection/infection": "^0.25.0", "phpspec/prophecy-phpunit": "^2.0", "phpstan/phpstan": "^0.12.94", "phpstan/phpstan-doctrine": "^0.12.42", diff --git a/config/autoload/installer.global.php b/config/autoload/installer.global.php index e89b382d..24461e70 100644 --- a/config/autoload/installer.global.php +++ b/config/autoload/installer.global.php @@ -47,6 +47,7 @@ return [ Option\Tracking\IpAnonymizationConfigOption::class, Option\Tracking\OrphanVisitsTrackingConfigOption::class, Option\Tracking\DisableTrackParamConfigOption::class, + Option\Tracking\DisableTrackingFromConfigOption::class, Option\Tracking\DisableTrackingConfigOption::class, Option\Tracking\DisableIpTrackingConfigOption::class, Option\Tracking\DisableReferrerTrackingConfigOption::class,