From 42a5296f935957305ddde93f6a1836bf9d0d4260 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 9 Dec 2022 17:42:14 +0100 Subject: [PATCH 01/16] Added new params to short URLs list to filter out 'disabled' short ones --- composer.json | 2 +- module/Core/src/ShortUrl/Model/ShortUrlsParams.php | 4 ++++ module/Core/src/ShortUrl/Model/TagsMode.php | 7 +++++++ .../Model/Validation/ShortUrlsParamsInputFilter.php | 7 ++++++- 4 files changed, 18 insertions(+), 2 deletions(-) diff --git a/composer.json b/composer.json index 762ead49..6a5bb317 100644 --- a/composer.json +++ b/composer.json @@ -45,7 +45,7 @@ "php-middleware/request-id": "^4.1", "pugx/shortid-php": "^1.1", "ramsey/uuid": "^4.5", - "shlinkio/shlink-common": "dev-main#7515008 as 5.2", + "shlinkio/shlink-common": "dev-main#f4101bc as 5.2", "shlinkio/shlink-config": "dev-main#96c81fb as 2.3", "shlinkio/shlink-event-dispatcher": "^2.6", "shlinkio/shlink-importer": "dev-main#c97662b as 5.0", diff --git a/module/Core/src/ShortUrl/Model/ShortUrlsParams.php b/module/Core/src/ShortUrl/Model/ShortUrlsParams.php index 14e88132..e053a283 100644 --- a/module/Core/src/ShortUrl/Model/ShortUrlsParams.php +++ b/module/Core/src/ShortUrl/Model/ShortUrlsParams.php @@ -24,6 +24,8 @@ final class ShortUrlsParams public readonly array $tags, public readonly Ordering $orderBy, public readonly ?DateRange $dateRange, + public readonly bool $excludeMaxVisitsReached, + public readonly bool $excludePastValidUntil, public readonly TagsMode $tagsMode = TagsMode::ANY, ) { } @@ -55,6 +57,8 @@ final class ShortUrlsParams normalizeOptionalDate($inputFilter->getValue(ShortUrlsParamsInputFilter::START_DATE)), normalizeOptionalDate($inputFilter->getValue(ShortUrlsParamsInputFilter::END_DATE)), ), + excludeMaxVisitsReached: $inputFilter->getValue(ShortUrlsParamsInputFilter::EXCLUDE_MAX_VISITS_REACHED), + excludePastValidUntil: $inputFilter->getValue(ShortUrlsParamsInputFilter::EXCLUDE_PAST_VALID_UNTIL), tagsMode: self::resolveTagsMode($inputFilter->getValue(ShortUrlsParamsInputFilter::TAGS_MODE)), ); } diff --git a/module/Core/src/ShortUrl/Model/TagsMode.php b/module/Core/src/ShortUrl/Model/TagsMode.php index 593d6d83..01cdcc3b 100644 --- a/module/Core/src/ShortUrl/Model/TagsMode.php +++ b/module/Core/src/ShortUrl/Model/TagsMode.php @@ -4,8 +4,15 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\ShortUrl\Model; +use function Functional\map; + enum TagsMode: string { case ANY = 'any'; case ALL = 'all'; + + public static function values(): array + { + return map(self::cases(), static fn (TagsMode $mode) => $mode->value); + } } diff --git a/module/Core/src/ShortUrl/Model/Validation/ShortUrlsParamsInputFilter.php b/module/Core/src/ShortUrl/Model/Validation/ShortUrlsParamsInputFilter.php index a5301f21..3bdea8e2 100644 --- a/module/Core/src/ShortUrl/Model/Validation/ShortUrlsParamsInputFilter.php +++ b/module/Core/src/ShortUrl/Model/Validation/ShortUrlsParamsInputFilter.php @@ -23,6 +23,8 @@ class ShortUrlsParamsInputFilter extends InputFilter public const ITEMS_PER_PAGE = 'itemsPerPage'; public const TAGS_MODE = 'tagsMode'; public const ORDER_BY = 'orderBy'; + public const EXCLUDE_MAX_VISITS_REACHED = 'excludeMaxVisitsReached'; + public const EXCLUDE_PAST_VALID_UNTIL = 'excludePastValidUntil'; public function __construct(array $data) { @@ -44,11 +46,14 @@ class ShortUrlsParamsInputFilter extends InputFilter $tagsMode = $this->createInput(self::TAGS_MODE, false); $tagsMode->getValidatorChain()->attach(new InArray([ - 'haystack' => [TagsMode::ALL->value, TagsMode::ANY->value], + 'haystack' => TagsMode::values(), 'strict' => InArray::COMPARE_STRICT, ])); $this->add($tagsMode); $this->add($this->createOrderByInput(self::ORDER_BY, ShortUrlsParams::ORDERABLE_FIELDS)); + + $this->add($this->createBooleanInput(self::EXCLUDE_MAX_VISITS_REACHED, false)); + $this->add($this->createBooleanInput(self::EXCLUDE_PAST_VALID_UNTIL, false)); } } From c3ab87136614f04b2a95f103ab1cb6956e11ed31 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 9 Dec 2022 17:59:59 +0100 Subject: [PATCH 02/16] Exposed new short URLs list filtering params --- .../Persistence/ShortUrlsCountFiltering.php | 4 ++++ .../Persistence/ShortUrlsListFiltering.php | 15 ++++++++++++++- .../Adapter/ShortUrlRepositoryAdapterTest.php | 2 +- 3 files changed, 19 insertions(+), 2 deletions(-) diff --git a/module/Core/src/ShortUrl/Persistence/ShortUrlsCountFiltering.php b/module/Core/src/ShortUrl/Persistence/ShortUrlsCountFiltering.php index 2d1b1f21..906adc63 100644 --- a/module/Core/src/ShortUrl/Persistence/ShortUrlsCountFiltering.php +++ b/module/Core/src/ShortUrl/Persistence/ShortUrlsCountFiltering.php @@ -21,6 +21,8 @@ class ShortUrlsCountFiltering public readonly array $tags = [], public readonly ?TagsMode $tagsMode = null, public readonly ?DateRange $dateRange = null, + public readonly bool $excludeMaxVisitsReached = false, + public readonly bool $excludePastValidUntil = false, public readonly ?ApiKey $apiKey = null, ?string $defaultDomain = null, ) { @@ -37,6 +39,8 @@ class ShortUrlsCountFiltering $params->tags, $params->tagsMode, $params->dateRange, + $params->excludeMaxVisitsReached, + $params->excludePastValidUntil, $apiKey, $defaultDomain, ); diff --git a/module/Core/src/ShortUrl/Persistence/ShortUrlsListFiltering.php b/module/Core/src/ShortUrl/Persistence/ShortUrlsListFiltering.php index dd7eb0aa..ed8793be 100644 --- a/module/Core/src/ShortUrl/Persistence/ShortUrlsListFiltering.php +++ b/module/Core/src/ShortUrl/Persistence/ShortUrlsListFiltering.php @@ -20,10 +20,21 @@ class ShortUrlsListFiltering extends ShortUrlsCountFiltering array $tags = [], ?TagsMode $tagsMode = null, ?DateRange $dateRange = null, + bool $excludeMaxVisitsReached = false, + bool $excludePastValidUntil = false, ?ApiKey $apiKey = null, ?string $defaultDomain = null, ) { - parent::__construct($searchTerm, $tags, $tagsMode, $dateRange, $apiKey, $defaultDomain); + parent::__construct( + $searchTerm, + $tags, + $tagsMode, + $dateRange, + $excludeMaxVisitsReached, + $excludePastValidUntil, + $apiKey, + $defaultDomain, + ); } public static function fromLimitsAndParams( @@ -41,6 +52,8 @@ class ShortUrlsListFiltering extends ShortUrlsCountFiltering $params->tags, $params->tagsMode, $params->dateRange, + $params->excludePastValidUntil, + $params->excludePastValidUntil, $apiKey, $defaultDomain, ); diff --git a/module/Core/test/ShortUrl/Paginator/Adapter/ShortUrlRepositoryAdapterTest.php b/module/Core/test/ShortUrl/Paginator/Adapter/ShortUrlRepositoryAdapterTest.php index aa8efbd5..e271448c 100644 --- a/module/Core/test/ShortUrl/Paginator/Adapter/ShortUrlRepositoryAdapterTest.php +++ b/module/Core/test/ShortUrl/Paginator/Adapter/ShortUrlRepositoryAdapterTest.php @@ -74,7 +74,7 @@ class ShortUrlRepositoryAdapterTest extends TestCase $dateRange = $params->dateRange; $this->repo->expects($this->once())->method('countList')->with( - new ShortUrlsCountFiltering($searchTerm, $tags, TagsMode::ANY, $dateRange, $apiKey), + new ShortUrlsCountFiltering($searchTerm, $tags, TagsMode::ANY, $dateRange, apiKey: $apiKey), ); $adapter->getNbResults(); } From 40794c476fe2066e04af421f16102016797450d2 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 9 Dec 2022 18:03:20 +0100 Subject: [PATCH 03/16] Updated API docs with new short URLs list filters --- docs/swagger/paths/v1_short-urls.json | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/docs/swagger/paths/v1_short-urls.json b/docs/swagger/paths/v1_short-urls.json index 2675ab61..c5041ecc 100644 --- a/docs/swagger/paths/v1_short-urls.json +++ b/docs/swagger/paths/v1_short-urls.json @@ -97,6 +97,32 @@ "schema": { "type": "string" } + }, + { + "name": "excludeMaxVisitsReached", + "in": "query", + "description": "If true, short URLs which already reached their maximum amount of visits will be excluded.", + "required": false, + "schema": { + "type": "string", + "enum": [ + "false", + "true" + ] + } + }, + { + "name": "excludePastValidUntil", + "in": "query", + "description": "If true, short URLs which validUntil date is on the past will be excluded.", + "required": false, + "schema": { + "type": "string", + "enum": [ + "false", + "true" + ] + } } ], "security": [ From 7ba2cfc0105a786c863b9c48001073eb3fdb2517 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 10 Dec 2022 09:56:34 +0100 Subject: [PATCH 04/16] Moved true before false in swagger docs --- docs/swagger/paths/v1_short-urls.json | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/swagger/paths/v1_short-urls.json b/docs/swagger/paths/v1_short-urls.json index c5041ecc..05c5973a 100644 --- a/docs/swagger/paths/v1_short-urls.json +++ b/docs/swagger/paths/v1_short-urls.json @@ -106,8 +106,8 @@ "schema": { "type": "string", "enum": [ - "false", - "true" + "true", + "false" ] } }, @@ -119,8 +119,8 @@ "schema": { "type": "string", "enum": [ - "false", - "true" + "true", + "false" ] } } From 805c8c87ba0513dadaed132ca7e5f9dab3c67e2a Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 10 Dec 2022 09:56:56 +0100 Subject: [PATCH 05/16] Fixed nasty typo --- module/Core/src/ShortUrl/Persistence/ShortUrlsListFiltering.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/module/Core/src/ShortUrl/Persistence/ShortUrlsListFiltering.php b/module/Core/src/ShortUrl/Persistence/ShortUrlsListFiltering.php index ed8793be..db8b9a70 100644 --- a/module/Core/src/ShortUrl/Persistence/ShortUrlsListFiltering.php +++ b/module/Core/src/ShortUrl/Persistence/ShortUrlsListFiltering.php @@ -52,7 +52,7 @@ class ShortUrlsListFiltering extends ShortUrlsCountFiltering $params->tags, $params->tagsMode, $params->dateRange, - $params->excludePastValidUntil, + $params->excludeMaxVisitsReached, $params->excludePastValidUntil, $apiKey, $defaultDomain, From 463dfe972941c172855ace595aa1149ff081ad32 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 11 Dec 2022 10:17:21 +0100 Subject: [PATCH 06/16] Added support to filter out expired short URLs from list --- .../Repository/ShortUrlRepository.php | 69 ++++++++++++++----- 1 file changed, 50 insertions(+), 19 deletions(-) diff --git a/module/Core/src/ShortUrl/Repository/ShortUrlRepository.php b/module/Core/src/ShortUrl/Repository/ShortUrlRepository.php index ec2f7a94..2e18bc8c 100644 --- a/module/Core/src/ShortUrl/Repository/ShortUrlRepository.php +++ b/module/Core/src/ShortUrl/Repository/ShortUrlRepository.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\ShortUrl\Repository; +use Cake\Chronos\Chronos; use Doctrine\DBAL\LockMode; use Doctrine\DBAL\Platforms\PostgreSQLPlatform; use Doctrine\ORM\Query\Expr\Join; @@ -11,7 +12,6 @@ use Doctrine\ORM\QueryBuilder; use Happyr\DoctrineSpecification\Repository\EntitySpecificationRepository; use Happyr\DoctrineSpecification\Specification\Specification; use Shlinkio\Shlink\Common\Doctrine\Type\ChronosDateTimeType; -use Shlinkio\Shlink\Core\Model\Ordering; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlCreation; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlIdentifier; @@ -38,43 +38,58 @@ class ShortUrlRepository extends EntitySpecificationRepository implements ShortU // In case the ordering has been specified, the query could be more complex. Process it if ($filtering->orderBy->hasOrderField()) { - return $this->processOrderByForList($qb, $filtering->orderBy); + $this->processOrderByForList($qb, $filtering); } - // With no explicit order by, fallback to dateCreated-DESC - return $qb->orderBy('s.dateCreated', 'DESC')->getQuery()->getResult(); + $result = $qb->getQuery()->getResult(); + if ($filtering->excludeMaxVisitsReached || $filtering->orderBy->field === 'visits') { + return array_column($result, 0); + } + + return $result; } - private function processOrderByForList(QueryBuilder $qb, Ordering $orderBy): array + private function processOrderByForList(QueryBuilder $qb, ShortUrlsListFiltering $filtering): void { - $fieldName = $orderBy->field; - $order = $orderBy->direction; + $fieldName = $filtering->orderBy->field; + $order = $filtering->orderBy->direction; if ($fieldName === 'visits') { // FIXME This query is inefficient. // Diagnostic: It might need to use a sub-query, as done with the tags list query. - $qb->addSelect('COUNT(DISTINCT v) AS totalVisits') - ->leftJoin('s.visits', 'v') - ->groupBy('s') - ->orderBy('totalVisits', $order); + if (! $filtering->excludeMaxVisitsReached) { + // Left join only if this was not true, otherwise this left join already happened + $this->leftJoinShortUrlsWithVisitsCount($qb); + } - return array_column($qb->getQuery()->getResult(), 0); - } - - $orderableFields = ['longUrl', 'shortCode', 'dateCreated', 'title']; - if (contains($orderableFields, $fieldName)) { + $qb->orderBy('totalVisits', $order); + } elseif (contains(['longUrl', 'shortCode', 'dateCreated', 'title'], $fieldName)) { $qb->orderBy('s.' . $fieldName, $order); + } else { + // With no explicit order by, fallback to dateCreated-DESC + $qb->orderBy('s.dateCreated', 'DESC'); } - - return $qb->getQuery()->getResult(); } public function countList(ShortUrlsCountFiltering $filtering): int { $qb = $this->createListQueryBuilder($filtering); $qb->select('COUNT(DISTINCT s)'); + $query = $qb->getQuery(); - return (int) $qb->getQuery()->getSingleScalarResult(); +// dump($query->getSQL()); + + // TODO This is crap... + return $filtering->excludeMaxVisitsReached + ? count($query->getSingleColumnResult()) + : (int) $query->getSingleScalarResult(); + } + + private function leftJoinShortUrlsWithVisitsCount(QueryBuilder $qb): void + { + $qb->addSelect('COUNT(DISTINCT v) AS totalVisits') + ->leftJoin('s.visits', 'v') + ->groupBy('s'); } private function createListQueryBuilder(ShortUrlsCountFiltering $filtering): QueryBuilder @@ -134,6 +149,22 @@ class ShortUrlRepository extends EntitySpecificationRepository implements ShortU : $this->joinAllTags($qb, $tags); } + if ($filtering->excludeMaxVisitsReached) { + $this->leftJoinShortUrlsWithVisitsCount($qb); + $qb->having($qb->expr()->orX( + $qb->expr()->isNull('s.maxVisits'), + $qb->expr()->gt('s.maxVisits', 'COUNT(DISTINCT v)'), + )); + } + + if ($filtering->excludePastValidUntil) { + $qb->andWhere($qb->expr()->orX( + $qb->expr()->isNull('s.validUntil'), + $qb->expr()->gte('s.validUntil', ':minValidSince'), + )) + ->setParameter('minValidSince', Chronos::now()->toDateTimeString()); + } + $this->applySpecification($qb, $filtering->apiKey?->spec(), 's'); return $qb; From cdde59b5435a57d59a73ca52adf53a939441cf77 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 11 Dec 2022 11:41:37 +0100 Subject: [PATCH 07/16] Added db test for filtering of disabled short URLs --- module/Core/src/ShortUrl/Entity/ShortUrl.php | 32 +++++++------- .../Repository/ShortUrlRepository.php | 21 ++++++--- .../Repository/ShortUrlRepositoryTest.php | 44 +++++++++++++++++++ 3 files changed, 74 insertions(+), 23 deletions(-) diff --git a/module/Core/src/ShortUrl/Entity/ShortUrl.php b/module/Core/src/ShortUrl/Entity/ShortUrl.php index 7316c50b..28647bdc 100644 --- a/module/Core/src/ShortUrl/Entity/ShortUrl.php +++ b/module/Core/src/ShortUrl/Entity/ShortUrl.php @@ -65,29 +65,29 @@ class ShortUrl extends AbstractEntity return self::fromMeta(ShortUrlCreation::fromRawData([ShortUrlInputFilter::LONG_URL => $longUrl])); } - public static function fromMeta( - ShortUrlCreation $meta, + public static function fromMeta( // TODO Rename to create(...) + ShortUrlCreation $creation, ?ShortUrlRelationResolverInterface $relationResolver = null, ): self { $instance = new self(); $relationResolver = $relationResolver ?? new SimpleShortUrlRelationResolver(); - $instance->longUrl = $meta->getLongUrl(); + $instance->longUrl = $creation->getLongUrl(); $instance->dateCreated = Chronos::now(); $instance->visits = new ArrayCollection(); - $instance->tags = $relationResolver->resolveTags($meta->getTags()); - $instance->validSince = $meta->getValidSince(); - $instance->validUntil = $meta->getValidUntil(); - $instance->maxVisits = $meta->getMaxVisits(); - $instance->customSlugWasProvided = $meta->hasCustomSlug(); - $instance->shortCodeLength = $meta->getShortCodeLength(); - $instance->shortCode = $meta->getCustomSlug() ?? generateRandomShortCode($instance->shortCodeLength); - $instance->domain = $relationResolver->resolveDomain($meta->getDomain()); - $instance->authorApiKey = $meta->getApiKey(); - $instance->title = $meta->getTitle(); - $instance->titleWasAutoResolved = $meta->titleWasAutoResolved(); - $instance->crawlable = $meta->isCrawlable(); - $instance->forwardQuery = $meta->forwardQuery(); + $instance->tags = $relationResolver->resolveTags($creation->getTags()); + $instance->validSince = $creation->getValidSince(); + $instance->validUntil = $creation->getValidUntil(); + $instance->maxVisits = $creation->getMaxVisits(); + $instance->customSlugWasProvided = $creation->hasCustomSlug(); + $instance->shortCodeLength = $creation->getShortCodeLength(); + $instance->shortCode = $creation->getCustomSlug() ?? generateRandomShortCode($instance->shortCodeLength); + $instance->domain = $relationResolver->resolveDomain($creation->getDomain()); + $instance->authorApiKey = $creation->getApiKey(); + $instance->title = $creation->getTitle(); + $instance->titleWasAutoResolved = $creation->titleWasAutoResolved(); + $instance->crawlable = $creation->isCrawlable(); + $instance->forwardQuery = $creation->forwardQuery(); return $instance; } diff --git a/module/Core/src/ShortUrl/Repository/ShortUrlRepository.php b/module/Core/src/ShortUrl/Repository/ShortUrlRepository.php index 2e18bc8c..37121bdb 100644 --- a/module/Core/src/ShortUrl/Repository/ShortUrlRepository.php +++ b/module/Core/src/ShortUrl/Repository/ShortUrlRepository.php @@ -36,6 +36,11 @@ class ShortUrlRepository extends EntitySpecificationRepository implements ShortU ->setMaxResults($filtering->limit) ->setFirstResult($filtering->offset); + // Some DB engines (postgres and ms) require aggregates in the select for ordering and filtering + if ($filtering->excludeMaxVisitsReached || $filtering->orderBy->field === 'visits') { + $qb->addSelect('COUNT(DISTINCT v)'); + } + // In case the ordering has been specified, the query could be more complex. Process it if ($filtering->orderBy->hasOrderField()) { $this->processOrderByForList($qb, $filtering); @@ -59,10 +64,10 @@ class ShortUrlRepository extends EntitySpecificationRepository implements ShortU // Diagnostic: It might need to use a sub-query, as done with the tags list query. if (! $filtering->excludeMaxVisitsReached) { // Left join only if this was not true, otherwise this left join already happened - $this->leftJoinShortUrlsWithVisitsCount($qb); + $this->leftJoinShortUrlsWithVisits($qb); } - $qb->orderBy('totalVisits', $order); + $qb->orderBy('COUNT(DISTINCT v)', $order); } elseif (contains(['longUrl', 'shortCode', 'dateCreated', 'title'], $fieldName)) { $qb->orderBy('s.' . $fieldName, $order); } else { @@ -77,7 +82,10 @@ class ShortUrlRepository extends EntitySpecificationRepository implements ShortU $qb->select('COUNT(DISTINCT s)'); $query = $qb->getQuery(); -// dump($query->getSQL()); + // TODO Check if this is needed +// if ($filtering->excludeMaxVisitsReached) { +// $qb->addSelect('COUNT(DISTINCT v)'); +// } // TODO This is crap... return $filtering->excludeMaxVisitsReached @@ -85,10 +93,9 @@ class ShortUrlRepository extends EntitySpecificationRepository implements ShortU : (int) $query->getSingleScalarResult(); } - private function leftJoinShortUrlsWithVisitsCount(QueryBuilder $qb): void + private function leftJoinShortUrlsWithVisits(QueryBuilder $qb): void { - $qb->addSelect('COUNT(DISTINCT v) AS totalVisits') - ->leftJoin('s.visits', 'v') + $qb->leftJoin('s.visits', 'v') ->groupBy('s'); } @@ -150,7 +157,7 @@ class ShortUrlRepository extends EntitySpecificationRepository implements ShortU } if ($filtering->excludeMaxVisitsReached) { - $this->leftJoinShortUrlsWithVisitsCount($qb); + $this->leftJoinShortUrlsWithVisits($qb); $qb->having($qb->expr()->orX( $qb->expr()->isNull('s.maxVisits'), $qb->expr()->gt('s.maxVisits', 'COUNT(DISTINCT v)'), diff --git a/module/Core/test-db/ShortUrl/Repository/ShortUrlRepositoryTest.php b/module/Core/test-db/ShortUrl/Repository/ShortUrlRepositoryTest.php index e41eee15..186a4ecf 100644 --- a/module/Core/test-db/ShortUrl/Repository/ShortUrlRepositoryTest.php +++ b/module/Core/test-db/ShortUrl/Repository/ShortUrlRepositoryTest.php @@ -339,6 +339,50 @@ class ShortUrlRepositoryTest extends DatabaseTestCase self::assertCount(0, $this->repo->findList($buildFiltering('no results'))); } + /** @test */ + public function findListReturnsOnlyThoseWithoutExcludedUrls(): void + { + $shortUrl1 = ShortUrl::fromMeta(ShortUrlCreation::fromRawData([ + 'longUrl' => 'foo1', + 'validUntil' => Chronos::now()->addDays(1)->toAtomString(), + 'maxVisits' => 100, + ]), $this->relationResolver); + $this->getEntityManager()->persist($shortUrl1); + $shortUrl2 = ShortUrl::fromMeta(ShortUrlCreation::fromRawData([ + 'longUrl' => 'foo2', + 'validUntil' => Chronos::now()->subDays(1)->toAtomString(), + ]), $this->relationResolver); + $this->getEntityManager()->persist($shortUrl2); + $shortUrl3 = ShortUrl::fromMeta(ShortUrlCreation::fromRawData([ + 'longUrl' => 'foo3', + ]), $this->relationResolver); + $this->getEntityManager()->persist($shortUrl3); + $shortUrl4 = ShortUrl::fromMeta(ShortUrlCreation::fromRawData([ + 'longUrl' => 'foo4', + 'maxVisits' => 3, + ]), $this->relationResolver); + $this->getEntityManager()->persist($shortUrl4); + $this->getEntityManager()->persist(Visit::forValidShortUrl($shortUrl4, Visitor::emptyInstance())); + $this->getEntityManager()->persist(Visit::forValidShortUrl($shortUrl4, Visitor::emptyInstance())); + $this->getEntityManager()->persist(Visit::forValidShortUrl($shortUrl4, Visitor::emptyInstance())); + + $this->getEntityManager()->flush(); + + $filtering = static fn (bool $excludeMaxVisitsReached, bool $excludePastValidUntil) => + new ShortUrlsListFiltering( + null, + null, + Ordering::emptyInstance(), + excludeMaxVisitsReached: $excludeMaxVisitsReached, + excludePastValidUntil: $excludePastValidUntil, + ); + + self::assertCount(4, $this->repo->findList($filtering(false, false))); + self::assertCount(3, $this->repo->findList($filtering(true, false))); + self::assertCount(3, $this->repo->findList($filtering(false, true))); + self::assertCount(2, $this->repo->findList($filtering(true, true))); + } + /** @test */ public function shortCodeIsInUseLooksForShortUrlInProperSetOfTables(): void { From d83213341038f69584f2e9a11f2f7d7263946fe3 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 11 Dec 2022 12:33:17 +0100 Subject: [PATCH 08/16] Enhanced db tests for expired short urls filtering --- module/Core/src/ShortUrl/Repository/ShortUrlRepository.php | 5 ----- .../test-db/ShortUrl/Repository/ShortUrlRepositoryTest.php | 4 ++++ 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/module/Core/src/ShortUrl/Repository/ShortUrlRepository.php b/module/Core/src/ShortUrl/Repository/ShortUrlRepository.php index 37121bdb..f2a66fda 100644 --- a/module/Core/src/ShortUrl/Repository/ShortUrlRepository.php +++ b/module/Core/src/ShortUrl/Repository/ShortUrlRepository.php @@ -82,11 +82,6 @@ class ShortUrlRepository extends EntitySpecificationRepository implements ShortU $qb->select('COUNT(DISTINCT s)'); $query = $qb->getQuery(); - // TODO Check if this is needed -// if ($filtering->excludeMaxVisitsReached) { -// $qb->addSelect('COUNT(DISTINCT v)'); -// } - // TODO This is crap... return $filtering->excludeMaxVisitsReached ? count($query->getSingleColumnResult()) diff --git a/module/Core/test-db/ShortUrl/Repository/ShortUrlRepositoryTest.php b/module/Core/test-db/ShortUrl/Repository/ShortUrlRepositoryTest.php index 186a4ecf..7f2a5a89 100644 --- a/module/Core/test-db/ShortUrl/Repository/ShortUrlRepositoryTest.php +++ b/module/Core/test-db/ShortUrl/Repository/ShortUrlRepositoryTest.php @@ -378,9 +378,13 @@ class ShortUrlRepositoryTest extends DatabaseTestCase ); self::assertCount(4, $this->repo->findList($filtering(false, false))); + self::assertEquals(4, $this->repo->countList($filtering(false, false))); self::assertCount(3, $this->repo->findList($filtering(true, false))); + self::assertEquals(3, $this->repo->countList($filtering(true, false))); self::assertCount(3, $this->repo->findList($filtering(false, true))); + self::assertEquals(3, $this->repo->countList($filtering(false, true))); self::assertCount(2, $this->repo->findList($filtering(true, true))); + self::assertEquals(2, $this->repo->countList($filtering(true, true))); } /** @test */ From 8807a7846389c9e3df180876bf3250e987c7e775 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 11 Dec 2022 13:00:06 +0100 Subject: [PATCH 09/16] Improved performance when filtering out shortUrls which reached their limit by using a sub-query --- .../Repository/ShortUrlRepository.php | 36 ++++++------------- 1 file changed, 10 insertions(+), 26 deletions(-) diff --git a/module/Core/src/ShortUrl/Repository/ShortUrlRepository.php b/module/Core/src/ShortUrl/Repository/ShortUrlRepository.php index f2a66fda..34c3b34c 100644 --- a/module/Core/src/ShortUrl/Repository/ShortUrlRepository.php +++ b/module/Core/src/ShortUrl/Repository/ShortUrlRepository.php @@ -18,6 +18,7 @@ use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\ShortUrl\Model\TagsMode; use Shlinkio\Shlink\Core\ShortUrl\Persistence\ShortUrlsCountFiltering; use Shlinkio\Shlink\Core\ShortUrl\Persistence\ShortUrlsListFiltering; +use Shlinkio\Shlink\Core\Visit\Entity\Visit; use Shlinkio\Shlink\Importer\Model\ImportedShlinkUrl; use function array_column; @@ -36,18 +37,13 @@ class ShortUrlRepository extends EntitySpecificationRepository implements ShortU ->setMaxResults($filtering->limit) ->setFirstResult($filtering->offset); - // Some DB engines (postgres and ms) require aggregates in the select for ordering and filtering - if ($filtering->excludeMaxVisitsReached || $filtering->orderBy->field === 'visits') { - $qb->addSelect('COUNT(DISTINCT v)'); - } - // In case the ordering has been specified, the query could be more complex. Process it if ($filtering->orderBy->hasOrderField()) { $this->processOrderByForList($qb, $filtering); } $result = $qb->getQuery()->getResult(); - if ($filtering->excludeMaxVisitsReached || $filtering->orderBy->field === 'visits') { + if ($filtering->orderBy->field === 'visits') { return array_column($result, 0); } @@ -62,12 +58,10 @@ class ShortUrlRepository extends EntitySpecificationRepository implements ShortU if ($fieldName === 'visits') { // FIXME This query is inefficient. // Diagnostic: It might need to use a sub-query, as done with the tags list query. - if (! $filtering->excludeMaxVisitsReached) { - // Left join only if this was not true, otherwise this left join already happened - $this->leftJoinShortUrlsWithVisits($qb); - } - - $qb->orderBy('COUNT(DISTINCT v)', $order); + $qb->addSelect('COUNT(DISTINCT v)') + ->leftJoin('s.visits', 'v') + ->groupBy('s') + ->orderBy('COUNT(DISTINCT v)', $order); } elseif (contains(['longUrl', 'shortCode', 'dateCreated', 'title'], $fieldName)) { $qb->orderBy('s.' . $fieldName, $order); } else { @@ -80,18 +74,8 @@ class ShortUrlRepository extends EntitySpecificationRepository implements ShortU { $qb = $this->createListQueryBuilder($filtering); $qb->select('COUNT(DISTINCT s)'); - $query = $qb->getQuery(); - // TODO This is crap... - return $filtering->excludeMaxVisitsReached - ? count($query->getSingleColumnResult()) - : (int) $query->getSingleScalarResult(); - } - - private function leftJoinShortUrlsWithVisits(QueryBuilder $qb): void - { - $qb->leftJoin('s.visits', 'v') - ->groupBy('s'); + return (int) $qb->getQuery()->getSingleScalarResult(); } private function createListQueryBuilder(ShortUrlsCountFiltering $filtering): QueryBuilder @@ -152,10 +136,10 @@ class ShortUrlRepository extends EntitySpecificationRepository implements ShortU } if ($filtering->excludeMaxVisitsReached) { - $this->leftJoinShortUrlsWithVisits($qb); - $qb->having($qb->expr()->orX( + $visitEntity = Visit::class; + $qb->andWhere($qb->expr()->orX( $qb->expr()->isNull('s.maxVisits'), - $qb->expr()->gt('s.maxVisits', 'COUNT(DISTINCT v)'), + $qb->expr()->gt('s.maxVisits', "(SELECT COUNT(innerV.id) FROM $visitEntity as innerV WHERE innerV.shortUrl=s)"), )); } From 931bdb0cd76e2f4cc434e5197aae6478de7c2eda Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 11 Dec 2022 13:03:19 +0100 Subject: [PATCH 10/16] Fixed coding styles --- .../src/ShortUrl/Repository/ShortUrlRepository.php | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/module/Core/src/ShortUrl/Repository/ShortUrlRepository.php b/module/Core/src/ShortUrl/Repository/ShortUrlRepository.php index 34c3b34c..b1c82973 100644 --- a/module/Core/src/ShortUrl/Repository/ShortUrlRepository.php +++ b/module/Core/src/ShortUrl/Repository/ShortUrlRepository.php @@ -144,11 +144,12 @@ class ShortUrlRepository extends EntitySpecificationRepository implements ShortU } if ($filtering->excludePastValidUntil) { - $qb->andWhere($qb->expr()->orX( - $qb->expr()->isNull('s.validUntil'), - $qb->expr()->gte('s.validUntil', ':minValidSince'), - )) - ->setParameter('minValidSince', Chronos::now()->toDateTimeString()); + $qb + ->andWhere($qb->expr()->orX( + $qb->expr()->isNull('s.validUntil'), + $qb->expr()->gte('s.validUntil', ':minValidSince'), + )) + ->setParameter('minValidSince', Chronos::now()->toDateTimeString()); } $this->applySpecification($qb, $filtering->apiKey?->spec(), 's'); From 0d7a0ee9ea3d3a0c0bd00dfaf355e49f92aa4c99 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 11 Dec 2022 13:11:43 +0100 Subject: [PATCH 11/16] Fixed more coding styles --- module/Core/src/ShortUrl/Repository/ShortUrlRepository.php | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/module/Core/src/ShortUrl/Repository/ShortUrlRepository.php b/module/Core/src/ShortUrl/Repository/ShortUrlRepository.php index b1c82973..f51d2fa4 100644 --- a/module/Core/src/ShortUrl/Repository/ShortUrlRepository.php +++ b/module/Core/src/ShortUrl/Repository/ShortUrlRepository.php @@ -24,6 +24,7 @@ use Shlinkio\Shlink\Importer\Model\ImportedShlinkUrl; use function array_column; use function count; use function Functional\contains; +use function sprintf; class ShortUrlRepository extends EntitySpecificationRepository implements ShortUrlRepositoryInterface { @@ -136,10 +137,12 @@ class ShortUrlRepository extends EntitySpecificationRepository implements ShortU } if ($filtering->excludeMaxVisitsReached) { - $visitEntity = Visit::class; $qb->andWhere($qb->expr()->orX( $qb->expr()->isNull('s.maxVisits'), - $qb->expr()->gt('s.maxVisits', "(SELECT COUNT(innerV.id) FROM $visitEntity as innerV WHERE innerV.shortUrl=s)"), + $qb->expr()->gt( + 's.maxVisits', + sprintf('(SELECT COUNT(innerV.id) FROM %s as innerV WHERE innerV.shortUrl=s)', Visit::class), + ), )); } From 0c3523c34a5fa30a2118a84e48d56f0f41458d2a Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 11 Dec 2022 13:22:16 +0100 Subject: [PATCH 12/16] Fixed E2E test suites --- .../src/ShortUrl/Repository/ShortUrlRepository.php | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/module/Core/src/ShortUrl/Repository/ShortUrlRepository.php b/module/Core/src/ShortUrl/Repository/ShortUrlRepository.php index f51d2fa4..d6559973 100644 --- a/module/Core/src/ShortUrl/Repository/ShortUrlRepository.php +++ b/module/Core/src/ShortUrl/Repository/ShortUrlRepository.php @@ -39,9 +39,7 @@ class ShortUrlRepository extends EntitySpecificationRepository implements ShortU ->setFirstResult($filtering->offset); // In case the ordering has been specified, the query could be more complex. Process it - if ($filtering->orderBy->hasOrderField()) { - $this->processOrderByForList($qb, $filtering); - } + $this->processOrderByForList($qb, $filtering); $result = $qb->getQuery()->getResult(); if ($filtering->orderBy->field === 'visits') { @@ -53,6 +51,12 @@ class ShortUrlRepository extends EntitySpecificationRepository implements ShortU private function processOrderByForList(QueryBuilder $qb, ShortUrlsListFiltering $filtering): void { + // With no explicit order by, fallback to dateCreated-DESC + if (! $filtering->orderBy->hasOrderField()) { + $qb->orderBy('s.dateCreated', 'DESC'); + return; + } + $fieldName = $filtering->orderBy->field; $order = $filtering->orderBy->direction; @@ -65,9 +69,6 @@ class ShortUrlRepository extends EntitySpecificationRepository implements ShortU ->orderBy('COUNT(DISTINCT v)', $order); } elseif (contains(['longUrl', 'shortCode', 'dateCreated', 'title'], $fieldName)) { $qb->orderBy('s.' . $fieldName, $order); - } else { - // With no explicit order by, fallback to dateCreated-DESC - $qb->orderBy('s.dateCreated', 'DESC'); } } From 201f25e0ad698d3f760787c82e46e80a1b785f3d Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 11 Dec 2022 13:38:11 +0100 Subject: [PATCH 13/16] Improved API tests to cover exlucding disabled URLs from lists --- CHANGELOG.md | 7 +++++++ .../Rest/test-api/Action/ListShortUrlsTest.php | 18 ++++++++++++++++-- .../test-api/Fixtures/ShortUrlsFixture.php | 9 ++++++--- 3 files changed, 29 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1cbab421..31cd05d0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,13 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this ## [Unreleased] ### Added +* [#1612](https://github.com/shlinkio/shlink/issues/1612) Allowed to filter short URLs out of lists, when `validUntil` date is in the past or have reached their maximum amount of visits. + + This can be done by: + + * Providing `excludeMaxVisitsReached=true` and/or `excludePastValidUntil=true` to the `GET /short-urls` endpoint. + * Providing `--exclude-max-visits-reached` and/or `--exclude-past-valid-until` to the `short-urls:list` command. + * [#1616](https://github.com/shlinkio/shlink/issues/1616) Added support to import orphan visits when importing short URLs from another Shlink instance. * [#1519](https://github.com/shlinkio/shlink/issues/1519) Allowing to search short URLs by default domain. * [#1555](https://github.com/shlinkio/shlink/issues/1555) Added full support for PHP 8.2, pdating the dockr image to this version. diff --git a/module/Rest/test-api/Action/ListShortUrlsTest.php b/module/Rest/test-api/Action/ListShortUrlsTest.php index b28a0b5d..63664a6d 100644 --- a/module/Rest/test-api/Action/ListShortUrlsTest.php +++ b/module/Rest/test-api/Action/ListShortUrlsTest.php @@ -22,7 +22,7 @@ class ListShortUrlsTest extends ApiTestCase 'meta' => [ 'validSince' => null, 'validUntil' => null, - 'maxVisits' => null, + 'maxVisits' => 2, ], 'domain' => null, 'title' => 'My cool title', @@ -38,7 +38,7 @@ class ListShortUrlsTest extends ApiTestCase 'tags' => [], 'meta' => [ 'validSince' => null, - 'validUntil' => null, + 'validUntil' => '2020-05-01T00:00:00+00:00', 'maxVisits' => null, ], 'domain' => null, @@ -147,6 +147,20 @@ class ListShortUrlsTest extends ApiTestCase self::SHORT_URL_SHLINK_WITH_TITLE, self::SHORT_URL_DOCS, ], 'valid_api_key']; + yield [['excludePastValidUntil' => 'true'], [ + self::SHORT_URL_CUSTOM_DOMAIN, + self::SHORT_URL_CUSTOM_SLUG, + self::SHORT_URL_META, + self::SHORT_URL_CUSTOM_SLUG_AND_DOMAIN, + self::SHORT_URL_SHLINK_WITH_TITLE, + ], 'valid_api_key']; + yield [['excludeMaxVisitsReached' => 'true'], [ + self::SHORT_URL_CUSTOM_DOMAIN, + self::SHORT_URL_CUSTOM_SLUG, + self::SHORT_URL_META, + self::SHORT_URL_CUSTOM_SLUG_AND_DOMAIN, + self::SHORT_URL_DOCS, + ], 'valid_api_key']; yield [['orderBy' => 'shortCode'], [ self::SHORT_URL_SHLINK_WITH_TITLE, self::SHORT_URL_CUSTOM_SLUG, diff --git a/module/Rest/test-api/Fixtures/ShortUrlsFixture.php b/module/Rest/test-api/Fixtures/ShortUrlsFixture.php index eadf60ee..432e5cad 100644 --- a/module/Rest/test-api/Fixtures/ShortUrlsFixture.php +++ b/module/Rest/test-api/Fixtures/ShortUrlsFixture.php @@ -36,6 +36,7 @@ class ShortUrlsFixture extends AbstractFixture implements DependentFixtureInterf 'tags' => ['foo'], 'title' => 'My cool title', 'crawlable' => true, + 'maxVisits' => 2, ]), $relationResolver), '2018-05-01', ); @@ -61,9 +62,11 @@ class ShortUrlsFixture extends AbstractFixture implements DependentFixtureInterf $manager->persist($customShortUrl); $ghiShortUrl = $this->setShortUrlDate( - ShortUrl::fromMeta(ShortUrlCreation::fromRawData( - ['customSlug' => 'ghi789', 'longUrl' => 'https://shlink.io/documentation/'], - )), + ShortUrl::fromMeta(ShortUrlCreation::fromRawData([ + 'customSlug' => 'ghi789', + 'longUrl' => 'https://shlink.io/documentation/', + 'validUntil' => Chronos::parse('2020-05-01'), // In the past + ])), '2018-05-01', ); $manager->persist($ghiShortUrl); From c4f28b3a3215c059e880808166fa1cf0d1fd67a1 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 11 Dec 2022 18:24:47 +0100 Subject: [PATCH 14/16] Renamed ShortUrl::fromMeta to ShortUrl::create --- .../Command/ShortUrl/ListShortUrlsCommand.php | 12 ++++ .../ShortUrl/ListShortUrlsCommandTest.php | 2 +- module/Core/src/ShortUrl/Entity/ShortUrl.php | 8 +-- module/Core/src/ShortUrl/UrlShortener.php | 2 +- .../Repository/DomainRepositoryTest.php | 2 +- .../Repository/ShortUrlRepositoryTest.php | 64 +++++++++---------- .../Tag/Repository/TagRepositoryTest.php | 12 ++-- .../Visit/Repository/VisitRepositoryTest.php | 20 +++--- .../PublishingUpdatesGeneratorTest.php | 4 +- .../RabbitMq/NotifyVisitToRabbitMqTest.php | 2 +- .../test/ShortUrl/Entity/ShortUrlTest.php | 4 +- .../Helper/ShortUrlRedirectionBuilderTest.php | 2 +- .../Helper/ShortUrlStringifierTest.php | 2 +- .../test/ShortUrl/ShortUrlResolverTest.php | 8 +-- .../ShortUrlDataTransformerTest.php | 8 +-- .../Core/test/ShortUrl/UrlShortenerTest.php | 12 ++-- .../test-api/Fixtures/ShortUrlsFixture.php | 12 ++-- 17 files changed, 94 insertions(+), 82 deletions(-) diff --git a/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php b/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php index 11443abc..86a9dd57 100644 --- a/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php +++ b/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php @@ -77,6 +77,18 @@ class ListShortUrlsCommand extends Command InputOption::VALUE_NONE, 'If tags is provided, returns only short URLs having ALL tags.', ) + ->addOption( + 'exclude-max-visits-reached', + null, // TODO + InputOption::VALUE_NONE, + 'Excludes short URLs which reached their max amount of visits.', + ) + ->addOption( + 'exclude-past-valid-until', + null, // TODO + InputOption::VALUE_NONE, + 'Excludes short URLs which have a "validUntil" date in the past.', + ) ->addOption( 'order-by', 'o', diff --git a/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php b/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php index 3b186bd1..84b138d4 100644 --- a/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php @@ -114,7 +114,7 @@ class ListShortUrlsCommandTest extends TestCase $this->shortUrlService->expects($this->once())->method('listShortUrls')->with( ShortUrlsParams::emptyInstance(), )->willReturn(new Paginator(new ArrayAdapter([ - ShortUrl::fromMeta(ShortUrlCreation::fromRawData([ + ShortUrl::create(ShortUrlCreation::fromRawData([ 'longUrl' => 'foo.com', 'tags' => ['foo', 'bar', 'baz'], 'apiKey' => $apiKey, diff --git a/module/Core/src/ShortUrl/Entity/ShortUrl.php b/module/Core/src/ShortUrl/Entity/ShortUrl.php index 28647bdc..c7f10b75 100644 --- a/module/Core/src/ShortUrl/Entity/ShortUrl.php +++ b/module/Core/src/ShortUrl/Entity/ShortUrl.php @@ -57,15 +57,15 @@ class ShortUrl extends AbstractEntity public static function createEmpty(): self { - return self::fromMeta(ShortUrlCreation::createEmpty()); + return self::create(ShortUrlCreation::createEmpty()); } public static function withLongUrl(string $longUrl): self { - return self::fromMeta(ShortUrlCreation::fromRawData([ShortUrlInputFilter::LONG_URL => $longUrl])); + return self::create(ShortUrlCreation::fromRawData([ShortUrlInputFilter::LONG_URL => $longUrl])); } - public static function fromMeta( // TODO Rename to create(...) + public static function create( ShortUrlCreation $creation, ?ShortUrlRelationResolverInterface $relationResolver = null, ): self { @@ -109,7 +109,7 @@ class ShortUrl extends AbstractEntity $meta[ShortUrlInputFilter::CUSTOM_SLUG] = $url->shortCode; } - $instance = self::fromMeta(ShortUrlCreation::fromRawData($meta), $relationResolver); + $instance = self::create(ShortUrlCreation::fromRawData($meta), $relationResolver); $instance->importSource = $url->source->value; $instance->importOriginalShortCode = $url->shortCode; diff --git a/module/Core/src/ShortUrl/UrlShortener.php b/module/Core/src/ShortUrl/UrlShortener.php index 4f979921..d3f54650 100644 --- a/module/Core/src/ShortUrl/UrlShortener.php +++ b/module/Core/src/ShortUrl/UrlShortener.php @@ -44,7 +44,7 @@ class UrlShortener implements UrlShortenerInterface /** @var ShortUrl $newShortUrl */ $newShortUrl = $this->em->wrapInTransaction(function () use ($meta) { - $shortUrl = ShortUrl::fromMeta($meta, $this->relationResolver); + $shortUrl = ShortUrl::create($meta, $this->relationResolver); $this->verifyShortCodeUniqueness($meta, $shortUrl); $this->em->persist($shortUrl); diff --git a/module/Core/test-db/Domain/Repository/DomainRepositoryTest.php b/module/Core/test-db/Domain/Repository/DomainRepositoryTest.php index 17f65abc..c96d70ff 100644 --- a/module/Core/test-db/Domain/Repository/DomainRepositoryTest.php +++ b/module/Core/test-db/Domain/Repository/DomainRepositoryTest.php @@ -129,7 +129,7 @@ class DomainRepositoryTest extends DatabaseTestCase private function createShortUrl(Domain $domain, ?ApiKey $apiKey = null): ShortUrl { - return ShortUrl::fromMeta( + return ShortUrl::create( ShortUrlCreation::fromRawData( ['domain' => $domain->getAuthority(), 'apiKey' => $apiKey, 'longUrl' => 'foo'], ), diff --git a/module/Core/test-db/ShortUrl/Repository/ShortUrlRepositoryTest.php b/module/Core/test-db/ShortUrl/Repository/ShortUrlRepositoryTest.php index 7f2a5a89..dc1d6420 100644 --- a/module/Core/test-db/ShortUrl/Repository/ShortUrlRepositoryTest.php +++ b/module/Core/test-db/ShortUrl/Repository/ShortUrlRepositoryTest.php @@ -43,15 +43,15 @@ class ShortUrlRepositoryTest extends DatabaseTestCase /** @test */ public function findOneWithDomainFallbackReturnsProperData(): void { - $regularOne = ShortUrl::fromMeta(ShortUrlCreation::fromRawData(['customSlug' => 'foo', 'longUrl' => 'foo'])); + $regularOne = ShortUrl::create(ShortUrlCreation::fromRawData(['customSlug' => 'foo', 'longUrl' => 'foo'])); $this->getEntityManager()->persist($regularOne); - $withDomain = ShortUrl::fromMeta(ShortUrlCreation::fromRawData( + $withDomain = ShortUrl::create(ShortUrlCreation::fromRawData( ['domain' => 'example.com', 'customSlug' => 'domain-short-code', 'longUrl' => 'foo'], )); $this->getEntityManager()->persist($withDomain); - $withDomainDuplicatingRegular = ShortUrl::fromMeta(ShortUrlCreation::fromRawData( + $withDomainDuplicatingRegular = ShortUrl::create(ShortUrlCreation::fromRawData( ['domain' => 'doma.in', 'customSlug' => 'foo', 'longUrl' => 'foo_with_domain'], )); $this->getEntityManager()->persist($withDomainDuplicatingRegular); @@ -101,7 +101,7 @@ class ShortUrlRepositoryTest extends DatabaseTestCase /** @test */ public function findListProperlyFiltersResult(): void { - $foo = ShortUrl::fromMeta( + $foo = ShortUrl::create( ShortUrlCreation::fromRawData(['longUrl' => 'foo', 'tags' => ['bar']]), $this->relationResolver, ); @@ -197,27 +197,27 @@ class ShortUrlRepositoryTest extends DatabaseTestCase /** @test */ public function findListReturnsOnlyThoseWithMatchingTags(): void { - $shortUrl1 = ShortUrl::fromMeta(ShortUrlCreation::fromRawData([ + $shortUrl1 = ShortUrl::create(ShortUrlCreation::fromRawData([ 'longUrl' => 'foo1', 'tags' => ['foo', 'bar'], ]), $this->relationResolver); $this->getEntityManager()->persist($shortUrl1); - $shortUrl2 = ShortUrl::fromMeta(ShortUrlCreation::fromRawData([ + $shortUrl2 = ShortUrl::create(ShortUrlCreation::fromRawData([ 'longUrl' => 'foo2', 'tags' => ['foo', 'baz'], ]), $this->relationResolver); $this->getEntityManager()->persist($shortUrl2); - $shortUrl3 = ShortUrl::fromMeta(ShortUrlCreation::fromRawData([ + $shortUrl3 = ShortUrl::create(ShortUrlCreation::fromRawData([ 'longUrl' => 'foo3', 'tags' => ['foo'], ]), $this->relationResolver); $this->getEntityManager()->persist($shortUrl3); - $shortUrl4 = ShortUrl::fromMeta(ShortUrlCreation::fromRawData([ + $shortUrl4 = ShortUrl::create(ShortUrlCreation::fromRawData([ 'longUrl' => 'foo4', 'tags' => ['bar', 'baz'], ]), $this->relationResolver); $this->getEntityManager()->persist($shortUrl4); - $shortUrl5 = ShortUrl::fromMeta(ShortUrlCreation::fromRawData([ + $shortUrl5 = ShortUrl::create(ShortUrlCreation::fromRawData([ 'longUrl' => 'foo5', 'tags' => ['bar', 'baz'], ]), $this->relationResolver); @@ -306,17 +306,17 @@ class ShortUrlRepositoryTest extends DatabaseTestCase /** @test */ public function findListReturnsOnlyThoseWithMatchingDomains(): void { - $shortUrl1 = ShortUrl::fromMeta(ShortUrlCreation::fromRawData([ + $shortUrl1 = ShortUrl::create(ShortUrlCreation::fromRawData([ 'longUrl' => 'foo1', 'domain' => null, ]), $this->relationResolver); $this->getEntityManager()->persist($shortUrl1); - $shortUrl2 = ShortUrl::fromMeta(ShortUrlCreation::fromRawData([ + $shortUrl2 = ShortUrl::create(ShortUrlCreation::fromRawData([ 'longUrl' => 'foo2', 'domain' => null, ]), $this->relationResolver); $this->getEntityManager()->persist($shortUrl2); - $shortUrl3 = ShortUrl::fromMeta(ShortUrlCreation::fromRawData([ + $shortUrl3 = ShortUrl::create(ShortUrlCreation::fromRawData([ 'longUrl' => 'foo3', 'domain' => 'another.com', ]), $this->relationResolver); @@ -342,22 +342,22 @@ class ShortUrlRepositoryTest extends DatabaseTestCase /** @test */ public function findListReturnsOnlyThoseWithoutExcludedUrls(): void { - $shortUrl1 = ShortUrl::fromMeta(ShortUrlCreation::fromRawData([ + $shortUrl1 = ShortUrl::create(ShortUrlCreation::fromRawData([ 'longUrl' => 'foo1', 'validUntil' => Chronos::now()->addDays(1)->toAtomString(), 'maxVisits' => 100, ]), $this->relationResolver); $this->getEntityManager()->persist($shortUrl1); - $shortUrl2 = ShortUrl::fromMeta(ShortUrlCreation::fromRawData([ + $shortUrl2 = ShortUrl::create(ShortUrlCreation::fromRawData([ 'longUrl' => 'foo2', 'validUntil' => Chronos::now()->subDays(1)->toAtomString(), ]), $this->relationResolver); $this->getEntityManager()->persist($shortUrl2); - $shortUrl3 = ShortUrl::fromMeta(ShortUrlCreation::fromRawData([ + $shortUrl3 = ShortUrl::create(ShortUrlCreation::fromRawData([ 'longUrl' => 'foo3', ]), $this->relationResolver); $this->getEntityManager()->persist($shortUrl3); - $shortUrl4 = ShortUrl::fromMeta(ShortUrlCreation::fromRawData([ + $shortUrl4 = ShortUrl::create(ShortUrlCreation::fromRawData([ 'longUrl' => 'foo4', 'maxVisits' => 3, ]), $this->relationResolver); @@ -390,12 +390,12 @@ class ShortUrlRepositoryTest extends DatabaseTestCase /** @test */ public function shortCodeIsInUseLooksForShortUrlInProperSetOfTables(): void { - $shortUrlWithoutDomain = ShortUrl::fromMeta( + $shortUrlWithoutDomain = ShortUrl::create( ShortUrlCreation::fromRawData(['customSlug' => 'my-cool-slug', 'longUrl' => 'foo']), ); $this->getEntityManager()->persist($shortUrlWithoutDomain); - $shortUrlWithDomain = ShortUrl::fromMeta( + $shortUrlWithDomain = ShortUrl::create( ShortUrlCreation::fromRawData(['domain' => 'doma.in', 'customSlug' => 'another-slug', 'longUrl' => 'foo']), ); $this->getEntityManager()->persist($shortUrlWithDomain); @@ -419,12 +419,12 @@ class ShortUrlRepositoryTest extends DatabaseTestCase /** @test */ public function findOneLooksForShortUrlInProperSetOfTables(): void { - $shortUrlWithoutDomain = ShortUrl::fromMeta( + $shortUrlWithoutDomain = ShortUrl::create( ShortUrlCreation::fromRawData(['customSlug' => 'my-cool-slug', 'longUrl' => 'foo']), ); $this->getEntityManager()->persist($shortUrlWithoutDomain); - $shortUrlWithDomain = ShortUrl::fromMeta( + $shortUrlWithDomain = ShortUrl::create( ShortUrlCreation::fromRawData(['domain' => 'doma.in', 'customSlug' => 'another-slug', 'longUrl' => 'foo']), ); $this->getEntityManager()->persist($shortUrlWithDomain); @@ -465,29 +465,29 @@ class ShortUrlRepositoryTest extends DatabaseTestCase $start = Chronos::parse('2020-03-05 20:18:30'); $end = Chronos::parse('2021-03-05 20:18:30'); - $shortUrl = ShortUrl::fromMeta( + $shortUrl = ShortUrl::create( ShortUrlCreation::fromRawData(['validSince' => $start, 'longUrl' => 'foo', 'tags' => ['foo', 'bar']]), $this->relationResolver, ); $this->getEntityManager()->persist($shortUrl); - $shortUrl2 = ShortUrl::fromMeta(ShortUrlCreation::fromRawData(['validUntil' => $end, 'longUrl' => 'bar'])); + $shortUrl2 = ShortUrl::create(ShortUrlCreation::fromRawData(['validUntil' => $end, 'longUrl' => 'bar'])); $this->getEntityManager()->persist($shortUrl2); - $shortUrl3 = ShortUrl::fromMeta( + $shortUrl3 = ShortUrl::create( ShortUrlCreation::fromRawData(['validSince' => $start, 'validUntil' => $end, 'longUrl' => 'baz']), ); $this->getEntityManager()->persist($shortUrl3); - $shortUrl4 = ShortUrl::fromMeta( + $shortUrl4 = ShortUrl::create( ShortUrlCreation::fromRawData(['customSlug' => 'custom', 'validUntil' => $end, 'longUrl' => 'foo']), ); $this->getEntityManager()->persist($shortUrl4); - $shortUrl5 = ShortUrl::fromMeta(ShortUrlCreation::fromRawData(['maxVisits' => 3, 'longUrl' => 'foo'])); + $shortUrl5 = ShortUrl::create(ShortUrlCreation::fromRawData(['maxVisits' => 3, 'longUrl' => 'foo'])); $this->getEntityManager()->persist($shortUrl5); - $shortUrl6 = ShortUrl::fromMeta(ShortUrlCreation::fromRawData(['domain' => 'doma.in', 'longUrl' => 'foo'])); + $shortUrl6 = ShortUrl::create(ShortUrlCreation::fromRawData(['domain' => 'doma.in', 'longUrl' => 'foo'])); $this->getEntityManager()->persist($shortUrl6); $this->getEntityManager()->flush(); @@ -537,15 +537,15 @@ class ShortUrlRepositoryTest extends DatabaseTestCase ['validSince' => $start, 'maxVisits' => 50, 'longUrl' => 'foo', 'tags' => $tags], ); - $shortUrl1 = ShortUrl::fromMeta($meta, $this->relationResolver); + $shortUrl1 = ShortUrl::create($meta, $this->relationResolver); $this->getEntityManager()->persist($shortUrl1); $this->getEntityManager()->flush(); - $shortUrl2 = ShortUrl::fromMeta($meta, $this->relationResolver); + $shortUrl2 = ShortUrl::create($meta, $this->relationResolver); $this->getEntityManager()->persist($shortUrl2); $this->getEntityManager()->flush(); - $shortUrl3 = ShortUrl::fromMeta($meta, $this->relationResolver); + $shortUrl3 = ShortUrl::create($meta, $this->relationResolver); $this->getEntityManager()->persist($shortUrl3); $this->getEntityManager()->flush(); @@ -579,7 +579,7 @@ class ShortUrlRepositoryTest extends DatabaseTestCase $adminApiKey = ApiKey::create(); $this->getEntityManager()->persist($adminApiKey); - $shortUrl = ShortUrl::fromMeta(ShortUrlCreation::fromRawData([ + $shortUrl = ShortUrl::create(ShortUrlCreation::fromRawData([ 'validSince' => $start, 'apiKey' => $apiKey, 'domain' => $rightDomain->getAuthority(), @@ -588,7 +588,7 @@ class ShortUrlRepositoryTest extends DatabaseTestCase ]), $this->relationResolver); $this->getEntityManager()->persist($shortUrl); - $nonDomainShortUrl = ShortUrl::fromMeta(ShortUrlCreation::fromRawData([ + $nonDomainShortUrl = ShortUrl::create(ShortUrlCreation::fromRawData([ 'apiKey' => $apiKey, 'longUrl' => 'non-domain', ]), $this->relationResolver); @@ -707,7 +707,7 @@ class ShortUrlRepositoryTest extends DatabaseTestCase /** @test */ public function findCrawlableShortCodesReturnsExpectedResult(): void { - $createShortUrl = fn (bool $crawlable) => ShortUrl::fromMeta( + $createShortUrl = fn (bool $crawlable) => ShortUrl::create( ShortUrlCreation::fromRawData(['crawlable' => $crawlable, 'longUrl' => 'foo.com']), ); diff --git a/module/Core/test-db/Tag/Repository/TagRepositoryTest.php b/module/Core/test-db/Tag/Repository/TagRepositoryTest.php index b71577b9..ce0efff9 100644 --- a/module/Core/test-db/Tag/Repository/TagRepositoryTest.php +++ b/module/Core/test-db/Tag/Repository/TagRepositoryTest.php @@ -77,22 +77,22 @@ class TagRepositoryTest extends DatabaseTestCase ['longUrl' => '', 'tags' => $tags, 'apiKey' => $apiKey], ); - $shortUrl = ShortUrl::fromMeta($metaWithTags($firstUrlTags, $apiKey), $this->relationResolver); + $shortUrl = ShortUrl::create($metaWithTags($firstUrlTags, $apiKey), $this->relationResolver); $this->getEntityManager()->persist($shortUrl); $this->getEntityManager()->persist(Visit::forValidShortUrl($shortUrl, Visitor::emptyInstance())); $this->getEntityManager()->persist(Visit::forValidShortUrl($shortUrl, Visitor::emptyInstance())); $this->getEntityManager()->persist(Visit::forValidShortUrl($shortUrl, Visitor::emptyInstance())); - $shortUrl2 = ShortUrl::fromMeta($metaWithTags($secondUrlTags, null), $this->relationResolver); + $shortUrl2 = ShortUrl::create($metaWithTags($secondUrlTags, null), $this->relationResolver); $this->getEntityManager()->persist($shortUrl2); $this->getEntityManager()->persist(Visit::forValidShortUrl($shortUrl2, Visitor::emptyInstance())); // One of the tags has two extra short URLs, but with no visits $this->getEntityManager()->persist( - ShortUrl::fromMeta($metaWithTags(['bar'], null), $this->relationResolver), + ShortUrl::create($metaWithTags(['bar'], null), $this->relationResolver), ); $this->getEntityManager()->persist( - ShortUrl::fromMeta($metaWithTags(['bar'], $apiKey), $this->relationResolver), + ShortUrl::create($metaWithTags(['bar'], $apiKey), $this->relationResolver), ); $this->getEntityManager()->flush(); @@ -222,13 +222,13 @@ class TagRepositoryTest extends DatabaseTestCase [$firstUrlTags, $secondUrlTags] = array_chunk($names, 3); - $shortUrl = ShortUrl::fromMeta( + $shortUrl = ShortUrl::create( ShortUrlCreation::fromRawData(['apiKey' => $authorApiKey, 'longUrl' => '', 'tags' => $firstUrlTags]), $this->relationResolver, ); $this->getEntityManager()->persist($shortUrl); - $shortUrl2 = ShortUrl::fromMeta( + $shortUrl2 = ShortUrl::create( ShortUrlCreation::fromRawData( ['domain' => $domain->getAuthority(), 'longUrl' => '', 'tags' => $secondUrlTags], ), diff --git a/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php b/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php index b69190e5..da475832 100644 --- a/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php +++ b/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php @@ -313,7 +313,7 @@ class VisitRepositoryTest extends DatabaseTestCase $apiKey1 = ApiKey::fromMeta(ApiKeyMeta::withRoles(RoleDefinition::forAuthoredShortUrls())); $this->getEntityManager()->persist($apiKey1); - $shortUrl = ShortUrl::fromMeta( + $shortUrl = ShortUrl::create( ShortUrlCreation::fromRawData(['apiKey' => $apiKey1, 'domain' => $domain->getAuthority(), 'longUrl' => '']), $this->relationResolver, ); @@ -322,11 +322,11 @@ class VisitRepositoryTest extends DatabaseTestCase $apiKey2 = ApiKey::fromMeta(ApiKeyMeta::withRoles(RoleDefinition::forAuthoredShortUrls())); $this->getEntityManager()->persist($apiKey2); - $shortUrl2 = ShortUrl::fromMeta(ShortUrlCreation::fromRawData(['apiKey' => $apiKey2, 'longUrl' => ''])); + $shortUrl2 = ShortUrl::create(ShortUrlCreation::fromRawData(['apiKey' => $apiKey2, 'longUrl' => ''])); $this->getEntityManager()->persist($shortUrl2); $this->createVisitsForShortUrl($shortUrl2, 5); - $shortUrl3 = ShortUrl::fromMeta( + $shortUrl3 = ShortUrl::create( ShortUrlCreation::fromRawData(['apiKey' => $apiKey2, 'domain' => $domain->getAuthority(), 'longUrl' => '']), $this->relationResolver, ); @@ -365,7 +365,7 @@ class VisitRepositoryTest extends DatabaseTestCase /** @test */ public function findOrphanVisitsReturnsExpectedResult(): void { - $shortUrl = ShortUrl::fromMeta(ShortUrlCreation::fromRawData(['longUrl' => ''])); + $shortUrl = ShortUrl::create(ShortUrlCreation::fromRawData(['longUrl' => ''])); $this->getEntityManager()->persist($shortUrl); $this->createVisitsForShortUrl($shortUrl, 7); @@ -414,7 +414,7 @@ class VisitRepositoryTest extends DatabaseTestCase /** @test */ public function countOrphanVisitsReturnsExpectedResult(): void { - $shortUrl = ShortUrl::fromMeta(ShortUrlCreation::fromRawData(['longUrl' => ''])); + $shortUrl = ShortUrl::create(ShortUrlCreation::fromRawData(['longUrl' => ''])); $this->getEntityManager()->persist($shortUrl); $this->createVisitsForShortUrl($shortUrl, 7); @@ -451,15 +451,15 @@ class VisitRepositoryTest extends DatabaseTestCase /** @test */ public function findNonOrphanVisitsReturnsExpectedResult(): void { - $shortUrl = ShortUrl::fromMeta(ShortUrlCreation::fromRawData(['longUrl' => '1'])); + $shortUrl = ShortUrl::create(ShortUrlCreation::fromRawData(['longUrl' => '1'])); $this->getEntityManager()->persist($shortUrl); $this->createVisitsForShortUrl($shortUrl, 7); - $shortUrl2 = ShortUrl::fromMeta(ShortUrlCreation::fromRawData(['longUrl' => '2'])); + $shortUrl2 = ShortUrl::create(ShortUrlCreation::fromRawData(['longUrl' => '2'])); $this->getEntityManager()->persist($shortUrl2); $this->createVisitsForShortUrl($shortUrl2, 4); - $shortUrl3 = ShortUrl::fromMeta(ShortUrlCreation::fromRawData(['longUrl' => '3'])); + $shortUrl3 = ShortUrl::create(ShortUrlCreation::fromRawData(['longUrl' => '3'])); $this->getEntityManager()->persist($shortUrl3); $this->createVisitsForShortUrl($shortUrl3, 10); @@ -517,7 +517,7 @@ class VisitRepositoryTest extends DatabaseTestCase array $tags = [], ?ApiKey $apiKey = null, ): array { - $shortUrl = ShortUrl::fromMeta(ShortUrlCreation::fromRawData([ + $shortUrl = ShortUrl::create(ShortUrlCreation::fromRawData([ ShortUrlInputFilter::LONG_URL => '', ShortUrlInputFilter::TAGS => $tags, ShortUrlInputFilter::API_KEY => $apiKey, @@ -529,7 +529,7 @@ class VisitRepositoryTest extends DatabaseTestCase $this->createVisitsForShortUrl($shortUrl); if ($withDomain !== false) { - $shortUrlWithDomain = ShortUrl::fromMeta(ShortUrlCreation::fromRawData([ + $shortUrlWithDomain = ShortUrl::create(ShortUrlCreation::fromRawData([ 'customSlug' => $shortCode, 'domain' => $domain, 'longUrl' => '', diff --git a/module/Core/test/EventDispatcher/PublishingUpdatesGeneratorTest.php b/module/Core/test/EventDispatcher/PublishingUpdatesGeneratorTest.php index 99ebb820..3ac690d0 100644 --- a/module/Core/test/EventDispatcher/PublishingUpdatesGeneratorTest.php +++ b/module/Core/test/EventDispatcher/PublishingUpdatesGeneratorTest.php @@ -35,7 +35,7 @@ class PublishingUpdatesGeneratorTest extends TestCase */ public function visitIsProperlySerializedIntoUpdate(string $method, string $expectedTopic, ?string $title): void { - $shortUrl = ShortUrl::fromMeta(ShortUrlCreation::fromRawData([ + $shortUrl = ShortUrl::create(ShortUrlCreation::fromRawData([ 'customSlug' => 'foo', 'longUrl' => '', 'title' => $title, @@ -114,7 +114,7 @@ class PublishingUpdatesGeneratorTest extends TestCase /** @test */ public function shortUrlIsProperlySerializedIntoUpdate(): void { - $shortUrl = ShortUrl::fromMeta(ShortUrlCreation::fromRawData([ + $shortUrl = ShortUrl::create(ShortUrlCreation::fromRawData([ 'customSlug' => 'foo', 'longUrl' => '', 'title' => 'The title', diff --git a/module/Core/test/EventDispatcher/RabbitMq/NotifyVisitToRabbitMqTest.php b/module/Core/test/EventDispatcher/RabbitMq/NotifyVisitToRabbitMqTest.php index 0def544e..8b7b392c 100644 --- a/module/Core/test/EventDispatcher/RabbitMq/NotifyVisitToRabbitMqTest.php +++ b/module/Core/test/EventDispatcher/RabbitMq/NotifyVisitToRabbitMqTest.php @@ -98,7 +98,7 @@ class NotifyVisitToRabbitMqTest extends TestCase yield 'orphan visit' => [Visit::forBasePath($visitor), ['newOrphanVisitUpdate']]; yield 'non-orphan visit' => [ Visit::forValidShortUrl( - ShortUrl::fromMeta(ShortUrlCreation::fromRawData([ + ShortUrl::create(ShortUrlCreation::fromRawData([ 'longUrl' => 'foo', 'customSlug' => 'bar', ])), diff --git a/module/Core/test/ShortUrl/Entity/ShortUrlTest.php b/module/Core/test/ShortUrl/Entity/ShortUrlTest.php index d874dbf5..026778ae 100644 --- a/module/Core/test/ShortUrl/Entity/ShortUrlTest.php +++ b/module/Core/test/ShortUrl/Entity/ShortUrlTest.php @@ -38,7 +38,7 @@ class ShortUrlTest extends TestCase public function provideInvalidShortUrls(): iterable { yield 'with custom slug' => [ - ShortUrl::fromMeta(ShortUrlCreation::fromRawData(['customSlug' => 'custom-slug', 'longUrl' => ''])), + ShortUrl::create(ShortUrlCreation::fromRawData(['customSlug' => 'custom-slug', 'longUrl' => ''])), 'The short code cannot be regenerated on ShortUrls where a custom slug was provided.', ]; yield 'already persisted' => [ @@ -77,7 +77,7 @@ class ShortUrlTest extends TestCase */ public function shortCodesHaveExpectedLength(?int $length, int $expectedLength): void { - $shortUrl = ShortUrl::fromMeta(ShortUrlCreation::fromRawData( + $shortUrl = ShortUrl::create(ShortUrlCreation::fromRawData( [ShortUrlInputFilter::SHORT_CODE_LENGTH => $length, 'longUrl' => ''], )); diff --git a/module/Core/test/ShortUrl/Helper/ShortUrlRedirectionBuilderTest.php b/module/Core/test/ShortUrl/Helper/ShortUrlRedirectionBuilderTest.php index 1a077c19..cb94a9f1 100644 --- a/module/Core/test/ShortUrl/Helper/ShortUrlRedirectionBuilderTest.php +++ b/module/Core/test/ShortUrl/Helper/ShortUrlRedirectionBuilderTest.php @@ -30,7 +30,7 @@ class ShortUrlRedirectionBuilderTest extends TestCase ?string $extraPath, ?bool $forwardQuery, ): void { - $shortUrl = ShortUrl::fromMeta(ShortUrlCreation::fromRawData([ + $shortUrl = ShortUrl::create(ShortUrlCreation::fromRawData([ 'longUrl' => 'https://domain.com/foo/bar?some=thing', 'forwardQuery' => $forwardQuery, ])); diff --git a/module/Core/test/ShortUrl/Helper/ShortUrlStringifierTest.php b/module/Core/test/ShortUrl/Helper/ShortUrlStringifierTest.php index d46fbf92..b6d5a123 100644 --- a/module/Core/test/ShortUrl/Helper/ShortUrlStringifierTest.php +++ b/module/Core/test/ShortUrl/Helper/ShortUrlStringifierTest.php @@ -28,7 +28,7 @@ class ShortUrlStringifierTest extends TestCase public function provideConfigAndShortUrls(): iterable { - $shortUrlWithShortCode = fn (string $shortCode, ?string $domain = null) => ShortUrl::fromMeta( + $shortUrlWithShortCode = fn (string $shortCode, ?string $domain = null) => ShortUrl::create( ShortUrlCreation::fromRawData([ 'longUrl' => '', 'customSlug' => $shortCode, diff --git a/module/Core/test/ShortUrl/ShortUrlResolverTest.php b/module/Core/test/ShortUrl/ShortUrlResolverTest.php index 1e86ec1c..9c2bcab3 100644 --- a/module/Core/test/ShortUrl/ShortUrlResolverTest.php +++ b/module/Core/test/ShortUrl/ShortUrlResolverTest.php @@ -114,7 +114,7 @@ class ShortUrlResolverTest extends TestCase $now = Chronos::now(); yield 'maxVisits reached' => [(function () { - $shortUrl = ShortUrl::fromMeta(ShortUrlCreation::fromRawData(['maxVisits' => 3, 'longUrl' => ''])); + $shortUrl = ShortUrl::create(ShortUrlCreation::fromRawData(['maxVisits' => 3, 'longUrl' => ''])); $shortUrl->setVisits(new ArrayCollection(map( range(0, 4), fn () => Visit::forValidShortUrl($shortUrl, Visitor::emptyInstance()), @@ -122,14 +122,14 @@ class ShortUrlResolverTest extends TestCase return $shortUrl; })()]; - yield 'future validSince' => [ShortUrl::fromMeta(ShortUrlCreation::fromRawData( + yield 'future validSince' => [ShortUrl::create(ShortUrlCreation::fromRawData( ['validSince' => $now->addMonth()->toAtomString(), 'longUrl' => ''], ))]; - yield 'past validUntil' => [ShortUrl::fromMeta(ShortUrlCreation::fromRawData( + yield 'past validUntil' => [ShortUrl::create(ShortUrlCreation::fromRawData( ['validUntil' => $now->subMonth()->toAtomString(), 'longUrl' => ''], ))]; yield 'mixed' => [(function () use ($now) { - $shortUrl = ShortUrl::fromMeta(ShortUrlCreation::fromRawData([ + $shortUrl = ShortUrl::create(ShortUrlCreation::fromRawData([ 'maxVisits' => 3, 'validUntil' => $now->subMonth()->toAtomString(), 'longUrl' => '', diff --git a/module/Core/test/ShortUrl/Transformer/ShortUrlDataTransformerTest.php b/module/Core/test/ShortUrl/Transformer/ShortUrlDataTransformerTest.php index 376e3d03..c9df4e38 100644 --- a/module/Core/test/ShortUrl/Transformer/ShortUrlDataTransformerTest.php +++ b/module/Core/test/ShortUrl/Transformer/ShortUrlDataTransformerTest.php @@ -43,7 +43,7 @@ class ShortUrlDataTransformerTest extends TestCase 'validUntil' => null, 'maxVisits' => null, ]]; - yield 'max visits only' => [ShortUrl::fromMeta(ShortUrlCreation::fromRawData([ + yield 'max visits only' => [ShortUrl::create(ShortUrlCreation::fromRawData([ 'maxVisits' => $maxVisits, 'longUrl' => '', ])), [ @@ -52,7 +52,7 @@ class ShortUrlDataTransformerTest extends TestCase 'maxVisits' => $maxVisits, ]]; yield 'max visits and valid since' => [ - ShortUrl::fromMeta(ShortUrlCreation::fromRawData( + ShortUrl::create(ShortUrlCreation::fromRawData( ['validSince' => $now, 'maxVisits' => $maxVisits, 'longUrl' => ''], )), [ @@ -62,7 +62,7 @@ class ShortUrlDataTransformerTest extends TestCase ], ]; yield 'both dates' => [ - ShortUrl::fromMeta(ShortUrlCreation::fromRawData( + ShortUrl::create(ShortUrlCreation::fromRawData( ['validSince' => $now, 'validUntil' => $now->subDays(10), 'longUrl' => ''], )), [ @@ -72,7 +72,7 @@ class ShortUrlDataTransformerTest extends TestCase ], ]; yield 'everything' => [ - ShortUrl::fromMeta(ShortUrlCreation::fromRawData( + ShortUrl::create(ShortUrlCreation::fromRawData( ['validSince' => $now, 'validUntil' => $now->subDays(5), 'maxVisits' => $maxVisits, 'longUrl' => ''], )), [ diff --git a/module/Core/test/ShortUrl/UrlShortenerTest.php b/module/Core/test/ShortUrl/UrlShortenerTest.php index 4c9e8646..d59de634 100644 --- a/module/Core/test/ShortUrl/UrlShortenerTest.php +++ b/module/Core/test/ShortUrl/UrlShortenerTest.php @@ -107,17 +107,17 @@ class UrlShortenerTest extends TestCase ), ShortUrl::withLongUrl($url)]; yield [ ShortUrlCreation::fromRawData(['findIfExists' => true, 'longUrl' => $url, 'tags' => ['foo', 'bar']]), - ShortUrl::fromMeta(ShortUrlCreation::fromRawData(['longUrl' => $url, 'tags' => ['foo', 'bar']])), + ShortUrl::create(ShortUrlCreation::fromRawData(['longUrl' => $url, 'tags' => ['foo', 'bar']])), ]; yield [ ShortUrlCreation::fromRawData(['findIfExists' => true, 'maxVisits' => 3, 'longUrl' => $url]), - ShortUrl::fromMeta(ShortUrlCreation::fromRawData(['maxVisits' => 3, 'longUrl' => $url])), + ShortUrl::create(ShortUrlCreation::fromRawData(['maxVisits' => 3, 'longUrl' => $url])), ]; yield [ ShortUrlCreation::fromRawData( ['findIfExists' => true, 'validSince' => Chronos::parse('2017-01-01'), 'longUrl' => $url], ), - ShortUrl::fromMeta( + ShortUrl::create( ShortUrlCreation::fromRawData(['validSince' => Chronos::parse('2017-01-01'), 'longUrl' => $url]), ), ]; @@ -125,13 +125,13 @@ class UrlShortenerTest extends TestCase ShortUrlCreation::fromRawData( ['findIfExists' => true, 'validUntil' => Chronos::parse('2017-01-01'), 'longUrl' => $url], ), - ShortUrl::fromMeta( + ShortUrl::create( ShortUrlCreation::fromRawData(['validUntil' => Chronos::parse('2017-01-01'), 'longUrl' => $url]), ), ]; yield [ ShortUrlCreation::fromRawData(['findIfExists' => true, 'domain' => 'example.com', 'longUrl' => $url]), - ShortUrl::fromMeta(ShortUrlCreation::fromRawData(['domain' => 'example.com', 'longUrl' => $url])), + ShortUrl::create(ShortUrlCreation::fromRawData(['domain' => 'example.com', 'longUrl' => $url])), ]; yield [ ShortUrlCreation::fromRawData([ @@ -141,7 +141,7 @@ class UrlShortenerTest extends TestCase 'longUrl' => $url, 'tags' => ['baz', 'foo', 'bar'], ]), - ShortUrl::fromMeta(ShortUrlCreation::fromRawData([ + ShortUrl::create(ShortUrlCreation::fromRawData([ 'validUntil' => Chronos::parse('2017-01-01'), 'maxVisits' => 4, 'longUrl' => $url, diff --git a/module/Rest/test-api/Fixtures/ShortUrlsFixture.php b/module/Rest/test-api/Fixtures/ShortUrlsFixture.php index 432e5cad..9a876463 100644 --- a/module/Rest/test-api/Fixtures/ShortUrlsFixture.php +++ b/module/Rest/test-api/Fixtures/ShortUrlsFixture.php @@ -29,7 +29,7 @@ class ShortUrlsFixture extends AbstractFixture implements DependentFixtureInterf $authorApiKey = $this->getReference('author_api_key'); $abcShortUrl = $this->setShortUrlDate( - ShortUrl::fromMeta(ShortUrlCreation::fromRawData([ + ShortUrl::create(ShortUrlCreation::fromRawData([ 'customSlug' => 'abc123', 'apiKey' => $authorApiKey, 'longUrl' => 'https://shlink.io', @@ -42,7 +42,7 @@ class ShortUrlsFixture extends AbstractFixture implements DependentFixtureInterf ); $manager->persist($abcShortUrl); - $defShortUrl = $this->setShortUrlDate(ShortUrl::fromMeta(ShortUrlCreation::fromRawData([ + $defShortUrl = $this->setShortUrlDate(ShortUrl::create(ShortUrlCreation::fromRawData([ 'validSince' => Chronos::parse('2020-05-01'), 'customSlug' => 'def456', 'apiKey' => $authorApiKey, @@ -52,7 +52,7 @@ class ShortUrlsFixture extends AbstractFixture implements DependentFixtureInterf ]), $relationResolver), '2019-01-01 00:00:10'); $manager->persist($defShortUrl); - $customShortUrl = $this->setShortUrlDate(ShortUrl::fromMeta(ShortUrlCreation::fromRawData([ + $customShortUrl = $this->setShortUrlDate(ShortUrl::create(ShortUrlCreation::fromRawData([ 'customSlug' => 'custom', 'maxVisits' => 2, 'apiKey' => $authorApiKey, @@ -62,7 +62,7 @@ class ShortUrlsFixture extends AbstractFixture implements DependentFixtureInterf $manager->persist($customShortUrl); $ghiShortUrl = $this->setShortUrlDate( - ShortUrl::fromMeta(ShortUrlCreation::fromRawData([ + ShortUrl::create(ShortUrlCreation::fromRawData([ 'customSlug' => 'ghi789', 'longUrl' => 'https://shlink.io/documentation/', 'validUntil' => Chronos::parse('2020-05-01'), // In the past @@ -71,7 +71,7 @@ class ShortUrlsFixture extends AbstractFixture implements DependentFixtureInterf ); $manager->persist($ghiShortUrl); - $withDomainDuplicatingShortCode = $this->setShortUrlDate(ShortUrl::fromMeta(ShortUrlCreation::fromRawData([ + $withDomainDuplicatingShortCode = $this->setShortUrlDate(ShortUrl::create(ShortUrlCreation::fromRawData([ 'domain' => 'example.com', 'customSlug' => 'ghi789', 'longUrl' => 'https://blog.alejandrocelaya.com/2019/04/27/considerations-to-properly-use-open-' @@ -80,7 +80,7 @@ class ShortUrlsFixture extends AbstractFixture implements DependentFixtureInterf ]), $relationResolver), '2019-01-01 00:00:30'); $manager->persist($withDomainDuplicatingShortCode); - $withDomainAndSlugShortUrl = $this->setShortUrlDate(ShortUrl::fromMeta(ShortUrlCreation::fromRawData( + $withDomainAndSlugShortUrl = $this->setShortUrlDate(ShortUrl::create(ShortUrlCreation::fromRawData( ['domain' => 'some-domain.com', 'customSlug' => 'custom-with-domain', 'longUrl' => 'https://google.com'], )), '2018-10-20'); $manager->persist($withDomainAndSlugShortUrl); From 0952c488be5689551961bebff1b93d6ec5bcae46 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 11 Dec 2022 18:33:40 +0100 Subject: [PATCH 15/16] Added exclusion flags to ListShortUrlsCommand --- .../CLI/src/Command/ShortUrl/ListShortUrlsCommand.php | 6 ++++-- module/CLI/test-cli/Command/ListShortUrlsTest.php | 10 ++++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php b/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php index 86a9dd57..6e735221 100644 --- a/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php +++ b/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php @@ -79,13 +79,13 @@ class ListShortUrlsCommand extends Command ) ->addOption( 'exclude-max-visits-reached', - null, // TODO + null, InputOption::VALUE_NONE, 'Excludes short URLs which reached their max amount of visits.', ) ->addOption( 'exclude-past-valid-until', - null, // TODO + null, InputOption::VALUE_NONE, 'Excludes short URLs which have a "validUntil" date in the past.', ) @@ -145,6 +145,8 @@ class ListShortUrlsCommand extends Command ShortUrlsParamsInputFilter::ORDER_BY => $orderBy, ShortUrlsParamsInputFilter::START_DATE => $startDate?->toAtomString(), ShortUrlsParamsInputFilter::END_DATE => $endDate?->toAtomString(), + ShortUrlsParamsInputFilter::EXCLUDE_MAX_VISITS_REACHED => $input->getOption('exclude-max-visits-reached'), + ShortUrlsParamsInputFilter::EXCLUDE_PAST_VALID_UNTIL => $input->getOption('exclude-past-valid-until'), ]; if ($all) { diff --git a/module/CLI/test-cli/Command/ListShortUrlsTest.php b/module/CLI/test-cli/Command/ListShortUrlsTest.php index faa47a2f..c98573a5 100644 --- a/module/CLI/test-cli/Command/ListShortUrlsTest.php +++ b/module/CLI/test-cli/Command/ListShortUrlsTest.php @@ -61,6 +61,16 @@ class ListShortUrlsTest extends CliTestCase | custom-with-domain | | http://some-domain.com/custom-with-domain | https://google.com | 2018-10-20T00:00:00+00:00 | 0 | +--------------------+-------+-------------------------------------------+----------------------------- Page 1 of 1 -----------------------------------------------------------+---------------------------+--------------+ OUTPUT]; + yield 'expired excluded' => [['--exclude-max-visits-reached', '--exclude-past-valid-until'], << Date: Sun, 11 Dec 2022 18:36:46 +0100 Subject: [PATCH 16/16] Fixed typo --- module/Core/src/ShortUrl/Repository/ShortUrlRepository.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/module/Core/src/ShortUrl/Repository/ShortUrlRepository.php b/module/Core/src/ShortUrl/Repository/ShortUrlRepository.php index d6559973..6acf1c2c 100644 --- a/module/Core/src/ShortUrl/Repository/ShortUrlRepository.php +++ b/module/Core/src/ShortUrl/Repository/ShortUrlRepository.php @@ -151,9 +151,9 @@ class ShortUrlRepository extends EntitySpecificationRepository implements ShortU $qb ->andWhere($qb->expr()->orX( $qb->expr()->isNull('s.validUntil'), - $qb->expr()->gte('s.validUntil', ':minValidSince'), + $qb->expr()->gte('s.validUntil', ':minValidUntil'), )) - ->setParameter('minValidSince', Chronos::now()->toDateTimeString()); + ->setParameter('minValidUntil', Chronos::now()->toDateTimeString()); } $this->applySpecification($qb, $filtering->apiKey?->spec(), 's');