diff --git a/CHANGELOG.md b/CHANGELOG.md index baf1f824..12517bc6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this All the existing camelCase flags will continue working for now, but will be removed in Shlink 3.0.0 +* [#862](https://github.com/shlinkio/shlink/issues/862) Deprecated endpoint to edit tags for a short URL (`PUT /short-urls/{shortCode}/tags`). + + The short URL edition endpoint (`PATCH /short-urls/{shortCode}`) now supports setting the tags too. Use it instead. + ### Removed * *Nothing* diff --git a/docs/swagger/paths/v1_short-urls_{shortCode}.json b/docs/swagger/paths/v1_short-urls_{shortCode}.json index c7e7dc8a..6cfa3c97 100644 --- a/docs/swagger/paths/v1_short-urls_{shortCode}.json +++ b/docs/swagger/paths/v1_short-urls_{shortCode}.json @@ -131,6 +131,13 @@ "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." } } } @@ -143,8 +150,33 @@ } ], "responses": { - "204": { - "description": "The short code has been properly updated." + "200": { + "description": "The short URL has been properly updated.", + "content": { + "application/json": { + "schema": { + "$ref": "../definitions/ShortUrl.json" + } + } + }, + "examples": { + "application/json": { + "shortCode": "12Kb3", + "shortUrl": "https://doma.in/12Kb3", + "longUrl": "https://shlink.io", + "dateCreated": "2016-05-01T20:34:16+02:00", + "visitsCount": 1029, + "tags": [ + "shlink" + ], + "meta": { + "validSince": "2017-01-21T00:00:00+02:00", + "validUntil": null, + "maxVisits": 100 + }, + "domain": null + } + } }, "400": { "description": "Provided meta arguments are invalid.", diff --git a/docs/swagger/paths/v1_short-urls_{shortCode}_tags.json b/docs/swagger/paths/v1_short-urls_{shortCode}_tags.json index fd497380..6ea642b0 100644 --- a/docs/swagger/paths/v1_short-urls_{shortCode}_tags.json +++ b/docs/swagger/paths/v1_short-urls_{shortCode}_tags.json @@ -1,11 +1,12 @@ { "put": { + "deprecated": true, "operationId": "editShortUrlTags", "tags": [ "Short URLs" ], "summary": "Edit tags on short URL", - "description": "Edit the tags on URL identified by provided short code.", + "description": "Edit the tags on URL identified by provided short code.
This endpoint is deprecated. Use the [Edit short URL](#/Short%20URLs/editShortUrl) endpoint to edit tags.", "parameters": [ { "$ref": "../parameters/version.json" diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index a843a0a2..0eaa7a8e 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -77,7 +77,12 @@ return [ EventDispatcherInterface::class, 'config.url_shortener.anonymize_remote_addr', ], - Service\ShortUrlService::class => ['em', Service\ShortUrl\ShortUrlResolver::class, Util\UrlValidator::class], + Service\ShortUrlService::class => [ + 'em', + Service\ShortUrl\ShortUrlResolver::class, + Util\UrlValidator::class, + ShortUrl\Resolver\PersistenceShortUrlRelationResolver::class, + ], Visit\VisitLocator::class => ['em'], Visit\VisitsStatsHelper::class => ['em'], Tag\TagService::class => ['em'], diff --git a/module/Core/src/Entity/ShortUrl.php b/module/Core/src/Entity/ShortUrl.php index 2c86f9ec..2919be17 100644 --- a/module/Core/src/Entity/ShortUrl.php +++ b/module/Core/src/Entity/ShortUrl.php @@ -64,7 +64,7 @@ class ShortUrl extends AbstractEntity $instance->longUrl = $meta->getLongUrl(); $instance->dateCreated = Chronos::now(); $instance->visits = new ArrayCollection(); - $instance->tags = new ArrayCollection(); + $instance->tags = $relationResolver->resolveTags($meta->getTags()); $instance->validSince = $meta->getValidSince(); $instance->validUntil = $meta->getValidUntil(); $instance->maxVisits = $meta->getMaxVisits(); @@ -85,6 +85,7 @@ class ShortUrl extends AbstractEntity $meta = [ ShortUrlInputFilter::LONG_URL => $url->longUrl(), ShortUrlInputFilter::DOMAIN => $url->domain(), + ShortUrlInputFilter::TAGS => $url->tags(), ShortUrlInputFilter::VALIDATE_URL => false, ]; if ($importShortCode) { @@ -127,17 +128,10 @@ class ShortUrl extends AbstractEntity return $this->tags; } - /** - * @param Collection|Tag[] $tags - */ - public function setTags(Collection $tags): self - { - $this->tags = $tags; - return $this; - } - - public function update(ShortUrlEdit $shortUrlEdit): void - { + public function update( + ShortUrlEdit $shortUrlEdit, + ?ShortUrlRelationResolverInterface $relationResolver = null + ): void { if ($shortUrlEdit->hasValidSince()) { $this->validSince = $shortUrlEdit->validSince(); } @@ -150,6 +144,10 @@ class ShortUrl extends AbstractEntity if ($shortUrlEdit->hasLongUrl()) { $this->longUrl = $shortUrlEdit->longUrl(); } + if ($shortUrlEdit->hasTags()) { + $relationResolver = $relationResolver ?? new SimpleShortUrlRelationResolver(); + $this->tags = $relationResolver->resolveTags($shortUrlEdit->tags()); + } } /** diff --git a/module/Core/src/Importer/ImportedLinksProcessor.php b/module/Core/src/Importer/ImportedLinksProcessor.php index 8bac7395..2b5cde17 100644 --- a/module/Core/src/Importer/ImportedLinksProcessor.php +++ b/module/Core/src/Importer/ImportedLinksProcessor.php @@ -10,7 +10,6 @@ use Shlinkio\Shlink\Core\Repository\ShortUrlRepositoryInterface; use Shlinkio\Shlink\Core\Service\ShortUrl\ShortCodeHelperInterface; use Shlinkio\Shlink\Core\ShortUrl\Resolver\ShortUrlRelationResolverInterface; use Shlinkio\Shlink\Core\Util\DoctrineBatchHelperInterface; -use Shlinkio\Shlink\Core\Util\TagManagerTrait; use Shlinkio\Shlink\Importer\ImportedLinksProcessorInterface; use Shlinkio\Shlink\Importer\Model\ImportedShlinkUrl; use Symfony\Component\Console\Style\StyleInterface; @@ -19,8 +18,6 @@ use function sprintf; class ImportedLinksProcessor implements ImportedLinksProcessorInterface { - use TagManagerTrait; - private EntityManagerInterface $em; private ShortUrlRelationResolverInterface $relationResolver; private ShortCodeHelperInterface $shortCodeHelper; @@ -59,8 +56,6 @@ class ImportedLinksProcessor implements ImportedLinksProcessorInterface } $shortUrl = ShortUrl::fromImport($url, $importShortCodes, $this->relationResolver); - $shortUrl->setTags($this->tagNamesToEntities($this->em, $url->tags())); - if (! $this->handleShortCodeUniqueness($url, $shortUrl, $io, $importShortCodes)) { continue; } diff --git a/module/Core/src/Model/ShortUrlEdit.php b/module/Core/src/Model/ShortUrlEdit.php index 824a5f60..b8cb0e0c 100644 --- a/module/Core/src/Model/ShortUrlEdit.php +++ b/module/Core/src/Model/ShortUrlEdit.php @@ -23,6 +23,8 @@ final class ShortUrlEdit private ?Chronos $validUntil = null; private bool $maxVisitsPropWasProvided = false; private ?int $maxVisits = null; + private bool $tagsPropWasProvided = false; + private array $tags = []; private ?bool $validateUrl = null; private function __construct() @@ -53,12 +55,14 @@ final class ShortUrlEdit $this->validSincePropWasProvided = array_key_exists(ShortUrlInputFilter::VALID_SINCE, $data); $this->validUntilPropWasProvided = array_key_exists(ShortUrlInputFilter::VALID_UNTIL, $data); $this->maxVisitsPropWasProvided = array_key_exists(ShortUrlInputFilter::MAX_VISITS, $data); + $this->tagsPropWasProvided = array_key_exists(ShortUrlInputFilter::TAGS, $data); $this->longUrl = $inputFilter->getValue(ShortUrlInputFilter::LONG_URL); $this->validSince = parseDateField($inputFilter->getValue(ShortUrlInputFilter::VALID_SINCE)); $this->validUntil = parseDateField($inputFilter->getValue(ShortUrlInputFilter::VALID_UNTIL)); $this->maxVisits = getOptionalIntFromInputFilter($inputFilter, ShortUrlInputFilter::MAX_VISITS); $this->validateUrl = getOptionalBoolFromInputFilter($inputFilter, ShortUrlInputFilter::VALIDATE_URL); + $this->tags = $inputFilter->getValue(ShortUrlInputFilter::TAGS); } public function longUrl(): ?string @@ -101,6 +105,19 @@ final class ShortUrlEdit return $this->maxVisitsPropWasProvided; } + /** + * @return string[] + */ + public function tags(): array + { + return $this->tags; + } + + public function hasTags(): bool + { + return $this->tagsPropWasProvided; + } + public function doValidateUrl(): ?bool { return $this->validateUrl; diff --git a/module/Core/src/Service/ShortUrlService.php b/module/Core/src/Service/ShortUrlService.php index aeb5233b..70606219 100644 --- a/module/Core/src/Service/ShortUrlService.php +++ b/module/Core/src/Service/ShortUrlService.php @@ -15,26 +15,27 @@ use Shlinkio\Shlink\Core\Model\ShortUrlsParams; use Shlinkio\Shlink\Core\Paginator\Adapter\ShortUrlRepositoryAdapter; use Shlinkio\Shlink\Core\Repository\ShortUrlRepository; use Shlinkio\Shlink\Core\Service\ShortUrl\ShortUrlResolverInterface; -use Shlinkio\Shlink\Core\Util\TagManagerTrait; +use Shlinkio\Shlink\Core\ShortUrl\Resolver\ShortUrlRelationResolverInterface; use Shlinkio\Shlink\Core\Util\UrlValidatorInterface; use Shlinkio\Shlink\Rest\Entity\ApiKey; class ShortUrlService implements ShortUrlServiceInterface { - use TagManagerTrait; - private ORM\EntityManagerInterface $em; private ShortUrlResolverInterface $urlResolver; private UrlValidatorInterface $urlValidator; + private ShortUrlRelationResolverInterface $relationResolver; public function __construct( ORM\EntityManagerInterface $em, ShortUrlResolverInterface $urlResolver, - UrlValidatorInterface $urlValidator + UrlValidatorInterface $urlValidator, + ShortUrlRelationResolverInterface $relationResolver ) { $this->em = $em; $this->urlResolver = $urlResolver; $this->urlValidator = $urlValidator; + $this->relationResolver = $relationResolver; } /** @@ -51,25 +52,11 @@ class ShortUrlService implements ShortUrlServiceInterface return $paginator; } - /** - * @param string[] $tags - * @throws ShortUrlNotFoundException - */ - public function setTagsByShortCode(ShortUrlIdentifier $identifier, array $tags, ?ApiKey $apiKey = null): ShortUrl - { - $shortUrl = $this->urlResolver->resolveShortUrl($identifier, $apiKey); - $shortUrl->setTags($this->tagNamesToEntities($this->em, $tags)); - - $this->em->flush(); - - return $shortUrl; - } - /** * @throws ShortUrlNotFoundException * @throws InvalidUrlException */ - public function updateMetadataByShortCode( + public function updateShortUrl( ShortUrlIdentifier $identifier, ShortUrlEdit $shortUrlEdit, ?ApiKey $apiKey = null @@ -79,7 +66,7 @@ class ShortUrlService implements ShortUrlServiceInterface } $shortUrl = $this->urlResolver->resolveShortUrl($identifier, $apiKey); - $shortUrl->update($shortUrlEdit); + $shortUrl->update($shortUrlEdit, $this->relationResolver); $this->em->flush(); diff --git a/module/Core/src/Service/ShortUrlServiceInterface.php b/module/Core/src/Service/ShortUrlServiceInterface.php index b1cfbc2d..3884b55e 100644 --- a/module/Core/src/Service/ShortUrlServiceInterface.php +++ b/module/Core/src/Service/ShortUrlServiceInterface.php @@ -20,17 +20,11 @@ interface ShortUrlServiceInterface */ public function listShortUrls(ShortUrlsParams $params, ?ApiKey $apiKey = null): Paginator; - /** - * @param string[] $tags - * @throws ShortUrlNotFoundException - */ - public function setTagsByShortCode(ShortUrlIdentifier $identifier, array $tags, ?ApiKey $apiKey = null): ShortUrl; - /** * @throws ShortUrlNotFoundException * @throws InvalidUrlException */ - public function updateMetadataByShortCode( + public function updateShortUrl( ShortUrlIdentifier $identifier, ShortUrlEdit $shortUrlEdit, ?ApiKey $apiKey = null diff --git a/module/Core/src/Service/UrlShortener.php b/module/Core/src/Service/UrlShortener.php index 08e64e60..f8125524 100644 --- a/module/Core/src/Service/UrlShortener.php +++ b/module/Core/src/Service/UrlShortener.php @@ -12,14 +12,11 @@ use Shlinkio\Shlink\Core\Model\ShortUrlMeta; use Shlinkio\Shlink\Core\Repository\ShortUrlRepositoryInterface; use Shlinkio\Shlink\Core\Service\ShortUrl\ShortCodeHelperInterface; use Shlinkio\Shlink\Core\ShortUrl\Resolver\ShortUrlRelationResolverInterface; -use Shlinkio\Shlink\Core\Util\TagManagerTrait; use Shlinkio\Shlink\Core\Util\UrlValidatorInterface; use Throwable; class UrlShortener implements UrlShortenerInterface { - use TagManagerTrait; - private EntityManagerInterface $em; private UrlValidatorInterface $urlValidator; private ShortUrlRelationResolverInterface $relationResolver; @@ -54,7 +51,6 @@ class UrlShortener implements UrlShortenerInterface return $this->em->transactional(function () use ($meta) { $shortUrl = ShortUrl::fromMeta($meta, $this->relationResolver); - $shortUrl->setTags($this->tagNamesToEntities($this->em, $meta->getTags())); $this->verifyShortCodeUniqueness($meta, $shortUrl); $this->em->persist($shortUrl); diff --git a/module/Core/src/ShortUrl/Resolver/PersistenceShortUrlRelationResolver.php b/module/Core/src/ShortUrl/Resolver/PersistenceShortUrlRelationResolver.php index 0e3afa23..fd0428bf 100644 --- a/module/Core/src/ShortUrl/Resolver/PersistenceShortUrlRelationResolver.php +++ b/module/Core/src/ShortUrl/Resolver/PersistenceShortUrlRelationResolver.php @@ -4,8 +4,13 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\ShortUrl\Resolver; +use Doctrine\Common\Collections; +use Doctrine\Common\Collections\Collection; use Doctrine\ORM\EntityManagerInterface; use Shlinkio\Shlink\Core\Entity\Domain; +use Shlinkio\Shlink\Core\Entity\Tag; + +use function Functional\map; class PersistenceShortUrlRelationResolver implements ShortUrlRelationResolverInterface { @@ -26,4 +31,23 @@ class PersistenceShortUrlRelationResolver implements ShortUrlRelationResolverInt $existingDomain = $this->em->getRepository(Domain::class)->findOneBy(['authority' => $domain]); return $existingDomain ?? new Domain($domain); } + + /** + * @param string[] $tags + * @return Collection|Tag[] + */ + public function resolveTags(array $tags): Collections\Collection + { + if (empty($tags)) { + return new Collections\ArrayCollection(); + } + + $repo = $this->em->getRepository(Tag::class); + return new Collections\ArrayCollection(map($tags, function (string $tagName) use ($repo): Tag { + $tag = $repo->findOneBy(['name' => $tagName]) ?? new Tag($tagName); + $this->em->persist($tag); + + return $tag; + })); + } } diff --git a/module/Core/src/ShortUrl/Resolver/ShortUrlRelationResolverInterface.php b/module/Core/src/ShortUrl/Resolver/ShortUrlRelationResolverInterface.php index bc576dbd..2d46a17b 100644 --- a/module/Core/src/ShortUrl/Resolver/ShortUrlRelationResolverInterface.php +++ b/module/Core/src/ShortUrl/Resolver/ShortUrlRelationResolverInterface.php @@ -4,9 +4,17 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\ShortUrl\Resolver; +use Doctrine\Common\Collections\Collection; use Shlinkio\Shlink\Core\Entity\Domain; +use Shlinkio\Shlink\Core\Entity\Tag; interface ShortUrlRelationResolverInterface { public function resolveDomain(?string $domain): ?Domain; + + /** + * @param string[] $tags + * @return Collection|Tag[] + */ + public function resolveTags(array $tags): Collection; } diff --git a/module/Core/src/ShortUrl/Resolver/SimpleShortUrlRelationResolver.php b/module/Core/src/ShortUrl/Resolver/SimpleShortUrlRelationResolver.php index 4e4620f5..2cda44df 100644 --- a/module/Core/src/ShortUrl/Resolver/SimpleShortUrlRelationResolver.php +++ b/module/Core/src/ShortUrl/Resolver/SimpleShortUrlRelationResolver.php @@ -4,7 +4,12 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\ShortUrl\Resolver; +use Doctrine\Common\Collections; +use Doctrine\Common\Collections\Collection; use Shlinkio\Shlink\Core\Entity\Domain; +use Shlinkio\Shlink\Core\Entity\Tag; + +use function Functional\map; class SimpleShortUrlRelationResolver implements ShortUrlRelationResolverInterface { @@ -12,4 +17,13 @@ class SimpleShortUrlRelationResolver implements ShortUrlRelationResolverInterfac { return $domain !== null ? new Domain($domain) : null; } + + /** + * @param string[] $tags + * @return Collection|Tag[] + */ + public function resolveTags(array $tags): Collections\Collection + { + return new Collections\ArrayCollection(map($tags, fn (string $tag) => new Tag($tag))); + } } diff --git a/module/Core/src/Util/TagManagerTrait.php b/module/Core/src/Util/TagManagerTrait.php index 27fb22b5..9fac8700 100644 --- a/module/Core/src/Util/TagManagerTrait.php +++ b/module/Core/src/Util/TagManagerTrait.php @@ -7,22 +7,25 @@ namespace Shlinkio\Shlink\Core\Util; use Doctrine\Common\Collections; use Doctrine\ORM\EntityManagerInterface; use Shlinkio\Shlink\Core\Entity\Tag; +use Shlinkio\Shlink\Core\Validation\ShortUrlInputFilter; use function Functional\map; -use function str_replace; -use function strtolower; -use function trim; +/** @deprecated */ trait TagManagerTrait { /** * @param string[] $tags + * @deprecated * @return Collections\Collection|Tag[] */ private function tagNamesToEntities(EntityManagerInterface $em, array $tags): Collections\Collection { - $entities = map($tags, function (string $tagName) use ($em) { - $tagName = $this->normalizeTagName($tagName); + $normalizedTags = ShortUrlInputFilter::withNonRequiredLongUrl([ + ShortUrlInputFilter::TAGS => $tags, + ])->getValue(ShortUrlInputFilter::TAGS); + + $entities = map($normalizedTags, function (string $tagName) use ($em) { $tag = $em->getRepository(Tag::class)->findOneBy(['name' => $tagName]) ?? new Tag($tagName); $em->persist($tag); @@ -31,9 +34,4 @@ trait TagManagerTrait return new Collections\ArrayCollection($entities); } - - private function normalizeTagName(string $tagName): string - { - return str_replace(' ', '-', strtolower(trim($tagName))); - } } diff --git a/module/Core/test-db/Domain/Repository/DomainRepositoryTest.php b/module/Core/test-db/Domain/Repository/DomainRepositoryTest.php index 3231eec4..49265eb0 100644 --- a/module/Core/test-db/Domain/Repository/DomainRepositoryTest.php +++ b/module/Core/test-db/Domain/Repository/DomainRepositoryTest.php @@ -4,6 +4,8 @@ declare(strict_types=1); namespace ShlinkioTest\Shlink\Core\Domain\Repository; +use Doctrine\Common\Collections\ArrayCollection; +use Doctrine\Common\Collections\Collection; use Shlinkio\Shlink\Core\Domain\Repository\DomainRepository; use Shlinkio\Shlink\Core\Entity\Domain; use Shlinkio\Shlink\Core\Entity\ShortUrl; @@ -102,6 +104,11 @@ class DomainRepositoryTest extends DatabaseTestCase { return $this->domain; } + + public function resolveTags(array $tags): Collection + { + return new ArrayCollection(); + } }, ); } diff --git a/module/Core/test-db/Repository/ShortUrlRepositoryTest.php b/module/Core/test-db/Repository/ShortUrlRepositoryTest.php index 6802d093..cf38d5a6 100644 --- a/module/Core/test-db/Repository/ShortUrlRepositoryTest.php +++ b/module/Core/test-db/Repository/ShortUrlRepositoryTest.php @@ -10,14 +10,12 @@ use ReflectionObject; use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Entity\Domain; use Shlinkio\Shlink\Core\Entity\ShortUrl; -use Shlinkio\Shlink\Core\Entity\Tag; use Shlinkio\Shlink\Core\Entity\Visit; use Shlinkio\Shlink\Core\Model\ShortUrlMeta; use Shlinkio\Shlink\Core\Model\ShortUrlsOrdering; use Shlinkio\Shlink\Core\Model\Visitor; use Shlinkio\Shlink\Core\Repository\ShortUrlRepository; use Shlinkio\Shlink\Core\ShortUrl\Resolver\PersistenceShortUrlRelationResolver; -use Shlinkio\Shlink\Core\Util\TagManagerTrait; use Shlinkio\Shlink\Importer\Model\ImportedShlinkUrl; use Shlinkio\Shlink\Rest\ApiKey\Model\RoleDefinition; use Shlinkio\Shlink\Rest\Entity\ApiKey; @@ -27,13 +25,13 @@ use function count; class ShortUrlRepositoryTest extends DatabaseTestCase { - use TagManagerTrait; - private ShortUrlRepository $repo; + private PersistenceShortUrlRelationResolver $relationResolver; public function beforeEach(): void { $this->repo = $this->getEntityManager()->getRepository(ShortUrl::class); + $this->relationResolver = new PersistenceShortUrlRelationResolver($this->getEntityManager()); } /** @test */ @@ -90,11 +88,10 @@ class ShortUrlRepositoryTest extends DatabaseTestCase /** @test */ public function findListProperlyFiltersResult(): void { - $tag = new Tag('bar'); - $this->getEntityManager()->persist($tag); - - $foo = ShortUrl::withLongUrl('foo'); - $foo->setTags(new ArrayCollection([$tag])); + $foo = ShortUrl::fromMeta( + ShortUrlMeta::fromRawData(['longUrl' => 'foo', 'tags' => ['bar']]), + $this->relationResolver, + ); $this->getEntityManager()->persist($foo); $bar = ShortUrl::withLongUrl('bar'); @@ -235,8 +232,10 @@ 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(ShortUrlMeta::fromRawData(['validSince' => $start, 'longUrl' => 'foo'])); - $shortUrl->setTags($this->tagNamesToEntities($this->getEntityManager(), ['foo', 'bar'])); + $shortUrl = ShortUrl::fromMeta( + ShortUrlMeta::fromRawData(['validSince' => $start, 'longUrl' => 'foo', 'tags' => ['foo', 'bar']]), + $this->relationResolver, + ); $this->getEntityManager()->persist($shortUrl); $shortUrl2 = ShortUrl::fromMeta(ShortUrlMeta::fromRawData(['validUntil' => $end, 'longUrl' => 'bar'])); @@ -300,28 +299,24 @@ class ShortUrlRepositoryTest extends DatabaseTestCase public function findOneMatchingReturnsOldestOneWhenThereAreMultipleMatches(): void { $start = Chronos::parse('2020-03-05 20:18:30'); - $meta = ShortUrlMeta::fromRawData(['validSince' => $start, 'maxVisits' => 50, 'longUrl' => 'foo']); $tags = ['foo', 'bar']; - $tagEntities = $this->tagNamesToEntities($this->getEntityManager(), $tags); - $metaWithTags = ShortUrlMeta::fromRawData( + $meta = ShortUrlMeta::fromRawData( ['validSince' => $start, 'maxVisits' => 50, 'longUrl' => 'foo', 'tags' => $tags], ); - $shortUrl1 = ShortUrl::fromMeta($meta); - $shortUrl1->setTags($tagEntities); + $shortUrl1 = ShortUrl::fromMeta($meta, $this->relationResolver); $this->getEntityManager()->persist($shortUrl1); - - $shortUrl2 = ShortUrl::fromMeta($meta); - $shortUrl2->setTags($tagEntities); - $this->getEntityManager()->persist($shortUrl2); - - $shortUrl3 = ShortUrl::fromMeta($meta); - $shortUrl3->setTags($tagEntities); - $this->getEntityManager()->persist($shortUrl3); - $this->getEntityManager()->flush(); - $result = $this->repo->findOneMatching($metaWithTags); + $shortUrl2 = ShortUrl::fromMeta($meta, $this->relationResolver); + $this->getEntityManager()->persist($shortUrl2); + $this->getEntityManager()->flush(); + + $shortUrl3 = ShortUrl::fromMeta($meta, $this->relationResolver); + $this->getEntityManager()->persist($shortUrl3); + $this->getEntityManager()->flush(); + + $result = $this->repo->findOneMatching($meta); self::assertSame($shortUrl1, $result); self::assertNotSame($shortUrl2, $result); @@ -349,10 +344,13 @@ class ShortUrlRepositoryTest extends DatabaseTestCase $rightDomainApiKey = ApiKey::withRoles(RoleDefinition::forDomain($rightDomain)); $this->getEntityManager()->persist($rightDomainApiKey); - $shortUrl = ShortUrl::fromMeta(ShortUrlMeta::fromRawData( - ['validSince' => $start, 'apiKey' => $apiKey, 'domain' => $rightDomain->getAuthority(), 'longUrl' => 'foo'], - ), new PersistenceShortUrlRelationResolver($this->getEntityManager())); - $shortUrl->setTags($this->tagNamesToEntities($this->getEntityManager(), ['foo', 'bar'])); + $shortUrl = ShortUrl::fromMeta(ShortUrlMeta::fromRawData([ + 'validSince' => $start, + 'apiKey' => $apiKey, + 'domain' => $rightDomain->getAuthority(), + 'longUrl' => 'foo', + 'tags' => ['foo', 'bar'], + ]), $this->relationResolver); $this->getEntityManager()->persist($shortUrl); $this->getEntityManager()->flush(); diff --git a/module/Core/test-db/Repository/TagRepositoryTest.php b/module/Core/test-db/Repository/TagRepositoryTest.php index 58f146f3..0a91775b 100644 --- a/module/Core/test-db/Repository/TagRepositoryTest.php +++ b/module/Core/test-db/Repository/TagRepositoryTest.php @@ -4,7 +4,6 @@ declare(strict_types=1); namespace ShlinkioTest\Shlink\Core\Repository; -use Doctrine\Common\Collections\ArrayCollection; use Shlinkio\Shlink\Core\Entity\Domain; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Entity\Tag; @@ -22,10 +21,12 @@ use function array_chunk; class TagRepositoryTest extends DatabaseTestCase { private TagRepository $repo; + private PersistenceShortUrlRelationResolver $relationResolver; protected function beforeEach(): void { $this->repo = $this->getEntityManager()->getRepository(Tag::class); + $this->relationResolver = new PersistenceShortUrlRelationResolver($this->getEntityManager()); } /** @test */ @@ -52,49 +53,44 @@ class TagRepositoryTest extends DatabaseTestCase public function properTagsInfoIsReturned(): void { $names = ['foo', 'bar', 'baz', 'another']; - $tags = []; foreach ($names as $name) { - $tag = new Tag($name); - $tags[] = $tag; - $this->getEntityManager()->persist($tag); + $this->getEntityManager()->persist(new Tag($name)); } + $this->getEntityManager()->flush(); - [$firstUrlTags] = array_chunk($tags, 3); - $secondUrlTags = [$tags[0]]; + [$firstUrlTags] = array_chunk($names, 3); + $secondUrlTags = [$names[0]]; + $metaWithTags = fn (array $tags) => ShortUrlMeta::fromRawData(['longUrl' => '', 'tags' => $tags]); - $shortUrl = ShortUrl::createEmpty(); - $shortUrl->setTags(new ArrayCollection($firstUrlTags)); + $shortUrl = ShortUrl::fromMeta($metaWithTags($firstUrlTags), $this->relationResolver); $this->getEntityManager()->persist($shortUrl); $this->getEntityManager()->persist(new Visit($shortUrl, Visitor::emptyInstance())); $this->getEntityManager()->persist(new Visit($shortUrl, Visitor::emptyInstance())); $this->getEntityManager()->persist(new Visit($shortUrl, Visitor::emptyInstance())); - $shortUrl2 = ShortUrl::createEmpty(); - $shortUrl2->setTags(new ArrayCollection($secondUrlTags)); + $shortUrl2 = ShortUrl::fromMeta($metaWithTags($secondUrlTags), $this->relationResolver); $this->getEntityManager()->persist($shortUrl2); $this->getEntityManager()->persist(new Visit($shortUrl2, Visitor::emptyInstance())); - $this->getEntityManager()->flush(); $result = $this->repo->findTagsWithInfo(); self::assertCount(4, $result); - self::assertEquals( - ['tag' => $tags[3], 'shortUrlsCount' => 0, 'visitsCount' => 0], - $result[0]->jsonSerialize(), - ); - self::assertEquals( - ['tag' => $tags[1], 'shortUrlsCount' => 1, 'visitsCount' => 3], - $result[1]->jsonSerialize(), - ); - self::assertEquals( - ['tag' => $tags[2], 'shortUrlsCount' => 1, 'visitsCount' => 3], - $result[2]->jsonSerialize(), - ); - self::assertEquals( - ['tag' => $tags[0], 'shortUrlsCount' => 2, 'visitsCount' => 4], - $result[3]->jsonSerialize(), - ); + self::assertEquals(0, $result[0]->shortUrlsCount()); + self::assertEquals(0, $result[0]->visitsCount()); + self::assertEquals($names[3], $result[0]->tag()->__toString()); + + self::assertEquals(1, $result[1]->shortUrlsCount()); + self::assertEquals(3, $result[1]->visitsCount()); + self::assertEquals($names[1], $result[1]->tag()->__toString()); + + self::assertEquals(1, $result[2]->shortUrlsCount()); + self::assertEquals(3, $result[2]->visitsCount()); + self::assertEquals($names[2], $result[2]->tag()->__toString()); + + self::assertEquals(2, $result[3]->shortUrlsCount()); + self::assertEquals(4, $result[3]->visitsCount()); + self::assertEquals($names[0], $result[3]->tag()->__toString()); } /** @test */ @@ -110,24 +106,23 @@ class TagRepositoryTest extends DatabaseTestCase $this->getEntityManager()->persist($domainApiKey); $names = ['foo', 'bar', 'baz', 'another']; - $tags = []; foreach ($names as $name) { - $tag = new Tag($name); - $tags[] = $tag; - $this->getEntityManager()->persist($tag); + $this->getEntityManager()->persist(new Tag($name)); } + $this->getEntityManager()->flush(); - [$firstUrlTags, $secondUrlTags] = array_chunk($tags, 3); + [$firstUrlTags, $secondUrlTags] = array_chunk($names, 3); - $shortUrl = ShortUrl::fromMeta(ShortUrlMeta::fromRawData(['apiKey' => $authorApiKey, 'longUrl' => ''])); - $shortUrl->setTags(new ArrayCollection($firstUrlTags)); + $shortUrl = ShortUrl::fromMeta( + ShortUrlMeta::fromRawData(['apiKey' => $authorApiKey, 'longUrl' => '', 'tags' => $firstUrlTags]), + $this->relationResolver, + ); $this->getEntityManager()->persist($shortUrl); $shortUrl2 = ShortUrl::fromMeta( - ShortUrlMeta::fromRawData(['domain' => $domain->getAuthority(), 'longUrl' => '']), - new PersistenceShortUrlRelationResolver($this->getEntityManager()), + ShortUrlMeta::fromRawData(['domain' => $domain->getAuthority(), 'longUrl' => '', 'tags' => $secondUrlTags]), + $this->relationResolver, ); - $shortUrl2->setTags(new ArrayCollection($secondUrlTags)); $this->getEntityManager()->persist($shortUrl2); $this->getEntityManager()->flush(); diff --git a/module/Core/test-db/Repository/VisitRepositoryTest.php b/module/Core/test-db/Repository/VisitRepositoryTest.php index ebdc2116..740edff5 100644 --- a/module/Core/test-db/Repository/VisitRepositoryTest.php +++ b/module/Core/test-db/Repository/VisitRepositoryTest.php @@ -5,11 +5,9 @@ declare(strict_types=1); namespace ShlinkioTest\Shlink\Core\Repository; use Cake\Chronos\Chronos; -use Doctrine\Common\Collections\ArrayCollection; use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Entity\Domain; use Shlinkio\Shlink\Core\Entity\ShortUrl; -use Shlinkio\Shlink\Core\Entity\Tag; use Shlinkio\Shlink\Core\Entity\Visit; use Shlinkio\Shlink\Core\Entity\VisitLocation; use Shlinkio\Shlink\Core\Model\ShortUrlMeta; @@ -28,10 +26,12 @@ use function sprintf; class VisitRepositoryTest extends DatabaseTestCase { private VisitRepository $repo; + private PersistenceShortUrlRelationResolver $relationResolver; protected function beforeEach(): void { $this->repo = $this->getEntityManager()->getRepository(Visit::class); + $this->relationResolver = new PersistenceShortUrlRelationResolver($this->getEntityManager()); } /** @@ -126,58 +126,45 @@ class VisitRepositoryTest extends DatabaseTestCase /** @test */ public function findVisitsByTagReturnsProperData(): void { - $foo = new Tag('foo'); - $this->getEntityManager()->persist($foo); + $foo = 'foo'; /** @var ShortUrl $shortUrl */ - [,, $shortUrl] = $this->createShortUrlsAndVisits(false); - /** @var ShortUrl $shortUrl2 */ - [,, $shortUrl2] = $this->createShortUrlsAndVisits(false); - /** @var ShortUrl $shortUrl3 */ - [,, $shortUrl3] = $this->createShortUrlsAndVisits(false); + $this->createShortUrlsAndVisits(false, [$foo]); + $this->getEntityManager()->flush(); - $shortUrl->setTags(new ArrayCollection([$foo])); - $shortUrl2->setTags(new ArrayCollection([$foo])); - $shortUrl3->setTags(new ArrayCollection([$foo])); + $this->createShortUrlsAndVisits(false, [$foo]); + $this->getEntityManager()->flush(); + $this->createShortUrlsAndVisits(false, [$foo]); $this->getEntityManager()->flush(); self::assertCount(0, $this->repo->findVisitsByTag('invalid')); - self::assertCount(18, $this->repo->findVisitsByTag((string) $foo)); - self::assertCount(6, $this->repo->findVisitsByTag((string) $foo, new DateRange( + self::assertCount(18, $this->repo->findVisitsByTag($foo)); + self::assertCount(6, $this->repo->findVisitsByTag($foo, new DateRange( Chronos::parse('2016-01-02'), Chronos::parse('2016-01-03'), ))); - self::assertCount(12, $this->repo->findVisitsByTag((string) $foo, new DateRange( - Chronos::parse('2016-01-03'), - ))); + self::assertCount(12, $this->repo->findVisitsByTag($foo, new DateRange(Chronos::parse('2016-01-03')))); } /** @test */ public function countVisitsByTagReturnsProperData(): void { - $foo = new Tag('foo'); - $this->getEntityManager()->persist($foo); + $foo = 'foo'; - /** @var ShortUrl $shortUrl */ - [,, $shortUrl] = $this->createShortUrlsAndVisits(false); - /** @var ShortUrl $shortUrl2 */ - [,, $shortUrl2] = $this->createShortUrlsAndVisits(false); - - $shortUrl->setTags(new ArrayCollection([$foo])); - $shortUrl2->setTags(new ArrayCollection([$foo])); + $this->createShortUrlsAndVisits(false, [$foo]); + $this->getEntityManager()->flush(); + $this->createShortUrlsAndVisits(false, [$foo]); $this->getEntityManager()->flush(); self::assertEquals(0, $this->repo->countVisitsByTag('invalid')); - self::assertEquals(12, $this->repo->countVisitsByTag((string) $foo)); - self::assertEquals(4, $this->repo->countVisitsByTag((string) $foo, new DateRange( + self::assertEquals(12, $this->repo->countVisitsByTag($foo)); + self::assertEquals(4, $this->repo->countVisitsByTag($foo, new DateRange( Chronos::parse('2016-01-02'), Chronos::parse('2016-01-03'), ))); - self::assertEquals(8, $this->repo->countVisitsByTag((string) $foo, new DateRange( - Chronos::parse('2016-01-03'), - ))); + self::assertEquals(8, $this->repo->countVisitsByTag($foo, new DateRange(Chronos::parse('2016-01-03')))); } /** @test */ @@ -192,7 +179,7 @@ class VisitRepositoryTest extends DatabaseTestCase $this->getEntityManager()->persist($apiKey1); $shortUrl = ShortUrl::fromMeta( ShortUrlMeta::fromRawData(['apiKey' => $apiKey1, 'domain' => $domain->getAuthority(), 'longUrl' => '']), - new PersistenceShortUrlRelationResolver($this->getEntityManager()), + $this->relationResolver, ); $this->getEntityManager()->persist($shortUrl); $this->createVisitsForShortUrl($shortUrl, 4); @@ -205,7 +192,7 @@ class VisitRepositoryTest extends DatabaseTestCase $shortUrl3 = ShortUrl::fromMeta( ShortUrlMeta::fromRawData(['apiKey' => $apiKey2, 'domain' => $domain->getAuthority(), 'longUrl' => '']), - new PersistenceShortUrlRelationResolver($this->getEntityManager()), + $this->relationResolver, ); $this->getEntityManager()->persist($shortUrl3); $this->createVisitsForShortUrl($shortUrl3, 7); @@ -221,9 +208,12 @@ class VisitRepositoryTest extends DatabaseTestCase self::assertEquals(4 + 7, $this->repo->countVisits($domainApiKey)); } - private function createShortUrlsAndVisits(bool $withDomain = true): array + private function createShortUrlsAndVisits(bool $withDomain = true, array $tags = []): array { - $shortUrl = ShortUrl::createEmpty(); + $shortUrl = ShortUrl::fromMeta(ShortUrlMeta::fromRawData([ + 'longUrl' => '', + 'tags' => $tags, + ]), $this->relationResolver); $domain = 'example.com'; $shortCode = $shortUrl->getShortCode(); $this->getEntityManager()->persist($shortUrl); diff --git a/module/Core/test/Service/ShortUrlServiceTest.php b/module/Core/test/Service/ShortUrlServiceTest.php index d056b91b..178561f0 100644 --- a/module/Core/test/Service/ShortUrlServiceTest.php +++ b/module/Core/test/Service/ShortUrlServiceTest.php @@ -6,19 +6,18 @@ namespace ShlinkioTest\Shlink\Core\Service; use Cake\Chronos\Chronos; use Doctrine\ORM\EntityManagerInterface; -use Doctrine\ORM\EntityRepository; use PHPUnit\Framework\TestCase; use Prophecy\Argument; use Prophecy\PhpUnit\ProphecyTrait; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\Core\Entity\ShortUrl; -use Shlinkio\Shlink\Core\Entity\Tag; use Shlinkio\Shlink\Core\Model\ShortUrlEdit; use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Model\ShortUrlsParams; use Shlinkio\Shlink\Core\Repository\ShortUrlRepository; use Shlinkio\Shlink\Core\Service\ShortUrl\ShortUrlResolverInterface; use Shlinkio\Shlink\Core\Service\ShortUrlService; +use Shlinkio\Shlink\Core\ShortUrl\Resolver\SimpleShortUrlRelationResolver; use Shlinkio\Shlink\Core\Util\UrlValidatorInterface; use Shlinkio\Shlink\Rest\Entity\ApiKey; use ShlinkioTest\Shlink\Core\Util\ApiKeyHelpersTrait; @@ -48,6 +47,7 @@ class ShortUrlServiceTest extends TestCase $this->em->reveal(), $this->urlResolver->reveal(), $this->urlValidator->reveal(), + new SimpleShortUrlRelationResolver(), ); } @@ -75,32 +75,11 @@ class ShortUrlServiceTest extends TestCase self::assertCount(4, $paginator->getCurrentPageResults()); } - /** - * @test - * @dataProvider provideAdminApiKeys - */ - public function providedTagsAreGetFromRepoAndSetToTheShortUrl(?ApiKey $apiKey): void - { - $shortUrl = $this->prophesize(ShortUrl::class); - $shortUrl->setTags(Argument::any())->shouldBeCalledOnce(); - $shortCode = 'abc123'; - $this->urlResolver->resolveShortUrl(new ShortUrlIdentifier($shortCode), $apiKey) - ->willReturn($shortUrl->reveal()) - ->shouldBeCalledOnce(); - - $tagRepo = $this->prophesize(EntityRepository::class); - $tagRepo->findOneBy(['name' => 'foo'])->willReturn(new Tag('foo'))->shouldBeCalledOnce(); - $tagRepo->findOneBy(['name' => 'bar'])->willReturn(null)->shouldBeCalledOnce(); - $this->em->getRepository(Tag::class)->willReturn($tagRepo->reveal()); - - $this->service->setTagsByShortCode(new ShortUrlIdentifier($shortCode), ['foo', 'bar'], $apiKey); - } - /** * @test * @dataProvider provideShortUrlEdits */ - public function updateMetadataByShortCodeUpdatesProvidedData( + public function updateShortUrlUpdatesProvidedData( int $expectedValidateCalls, ShortUrlEdit $shortUrlEdit, ?ApiKey $apiKey @@ -114,7 +93,7 @@ class ShortUrlServiceTest extends TestCase )->willReturn($shortUrl); $flush = $this->em->flush()->willReturn(null); - $result = $this->service->updateMetadataByShortCode(new ShortUrlIdentifier('abc123'), $shortUrlEdit, $apiKey); + $result = $this->service->updateShortUrl(new ShortUrlIdentifier('abc123'), $shortUrlEdit, $apiKey); self::assertSame($shortUrl, $result); self::assertEquals($shortUrlEdit->validSince(), $shortUrl->getValidSince()); diff --git a/module/Core/test/Service/UrlShortenerTest.php b/module/Core/test/Service/UrlShortenerTest.php index caa224b6..a9ba783f 100644 --- a/module/Core/test/Service/UrlShortenerTest.php +++ b/module/Core/test/Service/UrlShortenerTest.php @@ -5,14 +5,12 @@ declare(strict_types=1); namespace ShlinkioTest\Shlink\Core\Service; use Cake\Chronos\Chronos; -use Doctrine\Common\Collections\ArrayCollection; use Doctrine\ORM\EntityManagerInterface; use PHPUnit\Framework\TestCase; use Prophecy\Argument; use Prophecy\PhpUnit\ProphecyTrait; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\Core\Entity\ShortUrl; -use Shlinkio\Shlink\Core\Entity\Tag; use Shlinkio\Shlink\Core\Exception\NonUniqueSlugException; use Shlinkio\Shlink\Core\Model\ShortUrlMeta; use Shlinkio\Shlink\Core\Repository\ShortUrlRepository; @@ -119,7 +117,7 @@ class UrlShortenerTest extends TestCase ), ShortUrl::withLongUrl($url)]; yield [ ShortUrlMeta::fromRawData(['findIfExists' => true, 'longUrl' => $url, 'tags' => ['foo', 'bar']]), - ShortUrl::withLongUrl($url)->setTags(new ArrayCollection([new Tag('bar'), new Tag('foo')])), + ShortUrl::fromMeta(ShortUrlMeta::fromRawData(['longUrl' => $url, 'tags' => ['foo', 'bar']])), ]; yield [ ShortUrlMeta::fromRawData(['findIfExists' => true, 'maxVisits' => 3, 'longUrl' => $url]), @@ -157,7 +155,8 @@ class UrlShortenerTest extends TestCase 'validUntil' => Chronos::parse('2017-01-01'), 'maxVisits' => 4, 'longUrl' => $url, - ]))->setTags(new ArrayCollection([new Tag('foo'), new Tag('bar'), new Tag('baz')])), + 'tags' => ['foo', 'bar', 'baz'], + ])), ]; } } diff --git a/module/Core/test/ShortUrl/Resolver/PersistenceShortUrlRelationResolverTest.php b/module/Core/test/ShortUrl/Resolver/PersistenceShortUrlRelationResolverTest.php index 9cea7883..463ee1ef 100644 --- a/module/Core/test/ShortUrl/Resolver/PersistenceShortUrlRelationResolverTest.php +++ b/module/Core/test/ShortUrl/Resolver/PersistenceShortUrlRelationResolverTest.php @@ -7,9 +7,12 @@ namespace ShlinkioTest\Shlink\Core\ShortUrl\Resolver; use Doctrine\ORM\EntityManagerInterface; use Doctrine\Persistence\ObjectRepository; use PHPUnit\Framework\TestCase; +use Prophecy\Argument; use Prophecy\PhpUnit\ProphecyTrait; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\Core\Entity\Domain; +use Shlinkio\Shlink\Core\Entity\Tag; +use Shlinkio\Shlink\Core\Repository\TagRepositoryInterface; use Shlinkio\Shlink\Core\ShortUrl\Resolver\PersistenceShortUrlRelationResolver; class PersistenceShortUrlRelationResolverTest extends TestCase @@ -62,4 +65,42 @@ class PersistenceShortUrlRelationResolverTest extends TestCase yield 'not found domain' => [null, $authority]; yield 'found domain' => [new Domain($authority), $authority]; } + + /** @test */ + public function findsAndPersistsTagsWrappedIntoCollection(): void + { + $tags = ['foo', 'bar', 'baz']; + + $tagRepo = $this->prophesize(TagRepositoryInterface::class); + $findTag = $tagRepo->findOneBy(Argument::type('array'))->will(function (array $args): ?Tag { + ['name' => $name] = $args[0]; + return $name === 'foo' ? new Tag($name) : null; + }); + $getRepo = $this->em->getRepository(Tag::class)->willReturn($tagRepo->reveal()); + $persist = $this->em->persist(Argument::type(Tag::class)); + + $result = $this->resolver->resolveTags($tags); + + self::assertCount(3, $result); + self::assertEquals([new Tag('foo'), new Tag('bar'), new Tag('baz')], $result->toArray()); + $findTag->shouldHaveBeenCalledTimes(3); + $getRepo->shouldHaveBeenCalledOnce(); + $persist->shouldHaveBeenCalledTimes(3); + } + + /** @test */ + public function returnsEmptyCollectionWhenProvidingEmptyListOfTags(): void + { + $tagRepo = $this->prophesize(TagRepositoryInterface::class); + $findTag = $tagRepo->findOneBy(Argument::type('array'))->willReturn(null); + $getRepo = $this->em->getRepository(Tag::class)->willReturn($tagRepo->reveal()); + $persist = $this->em->persist(Argument::type(Tag::class)); + + $result = $this->resolver->resolveTags([]); + + self::assertEmpty($result); + $findTag->shouldNotHaveBeenCalled(); + $getRepo->shouldNotHaveBeenCalled(); + $persist->shouldNotHaveBeenCalled(); + } } diff --git a/module/Core/test/ShortUrl/Resolver/SimpleShortUrlRelationResolverTest.php b/module/Core/test/ShortUrl/Resolver/SimpleShortUrlRelationResolverTest.php index 84d838b9..483cb67a 100644 --- a/module/Core/test/ShortUrl/Resolver/SimpleShortUrlRelationResolverTest.php +++ b/module/Core/test/ShortUrl/Resolver/SimpleShortUrlRelationResolverTest.php @@ -6,6 +6,7 @@ namespace ShlinkioTest\Shlink\Core\ShortUrl\Resolver; use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\Core\Entity\Domain; +use Shlinkio\Shlink\Core\Entity\Tag; use Shlinkio\Shlink\Core\ShortUrl\Resolver\SimpleShortUrlRelationResolver; class SimpleShortUrlRelationResolverTest extends TestCase @@ -38,4 +39,15 @@ class SimpleShortUrlRelationResolverTest extends TestCase yield 'empty domain' => [null]; yield 'non-empty domain' => ['domain.com']; } + + /** @test */ + public function tagsAreWrappedInEntityCollection(): void + { + $tags = ['foo', 'bar', 'baz']; + + $result = $this->resolver->resolveTags($tags); + + self::assertCount(3, $result); + self::assertEquals([new Tag('foo'), new Tag('bar'), new Tag('baz')], $result->toArray()); + } } diff --git a/module/Rest/config/dependencies.config.php b/module/Rest/config/dependencies.config.php index dc960fb4..7891b2a0 100644 --- a/module/Rest/config/dependencies.config.php +++ b/module/Rest/config/dependencies.config.php @@ -59,7 +59,7 @@ return [ Service\UrlShortener::class, 'config.url_shortener.domain', ], - Action\ShortUrl\EditShortUrlAction::class => [Service\ShortUrlService::class], + Action\ShortUrl\EditShortUrlAction::class => [Service\ShortUrlService::class, 'config.url_shortener.domain'], Action\ShortUrl\DeleteShortUrlAction::class => [Service\ShortUrl\DeleteShortUrlService::class], Action\ShortUrl\ResolveShortUrlAction::class => [ Service\ShortUrl\ShortUrlResolver::class, diff --git a/module/Rest/src/Action/ShortUrl/AbstractCreateShortUrlAction.php b/module/Rest/src/Action/ShortUrl/AbstractCreateShortUrlAction.php index cfec8cac..a7278457 100644 --- a/module/Rest/src/Action/ShortUrl/AbstractCreateShortUrlAction.php +++ b/module/Rest/src/Action/ShortUrl/AbstractCreateShortUrlAction.php @@ -16,22 +16,20 @@ use Shlinkio\Shlink\Rest\Action\AbstractRestAction; abstract class AbstractCreateShortUrlAction extends AbstractRestAction { private UrlShortenerInterface $urlShortener; - private array $domainConfig; + private ShortUrlDataTransformer $transformer; public function __construct(UrlShortenerInterface $urlShortener, array $domainConfig) { $this->urlShortener = $urlShortener; - $this->domainConfig = $domainConfig; + $this->transformer = new ShortUrlDataTransformer($domainConfig); } public function handle(Request $request): Response { $shortUrlMeta = $this->buildShortUrlData($request); - $shortUrl = $this->urlShortener->shorten($shortUrlMeta); - $transformer = new ShortUrlDataTransformer($this->domainConfig); - return new JsonResponse($transformer->transform($shortUrl)); + return new JsonResponse($this->transformer->transform($shortUrl)); } /** diff --git a/module/Rest/src/Action/ShortUrl/EditShortUrlAction.php b/module/Rest/src/Action/ShortUrl/EditShortUrlAction.php index 32d95b2d..672d3963 100644 --- a/module/Rest/src/Action/ShortUrl/EditShortUrlAction.php +++ b/module/Rest/src/Action/ShortUrl/EditShortUrlAction.php @@ -4,12 +4,13 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Rest\Action\ShortUrl; -use Laminas\Diactoros\Response\EmptyResponse; +use Laminas\Diactoros\Response\JsonResponse; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; use Shlinkio\Shlink\Core\Model\ShortUrlEdit; use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Service\ShortUrlServiceInterface; +use Shlinkio\Shlink\Core\Transformer\ShortUrlDataTransformer; use Shlinkio\Shlink\Rest\Action\AbstractRestAction; use Shlinkio\Shlink\Rest\Middleware\AuthenticationMiddleware; @@ -19,10 +20,12 @@ class EditShortUrlAction extends AbstractRestAction protected const ROUTE_ALLOWED_METHODS = [self::METHOD_PATCH, self::METHOD_PUT]; private ShortUrlServiceInterface $shortUrlService; + private ShortUrlDataTransformer $transformer; - public function __construct(ShortUrlServiceInterface $shortUrlService) + public function __construct(ShortUrlServiceInterface $shortUrlService, array $domainConfig) { $this->shortUrlService = $shortUrlService; + $this->transformer = new ShortUrlDataTransformer($domainConfig); } public function handle(ServerRequestInterface $request): ResponseInterface @@ -31,7 +34,8 @@ class EditShortUrlAction extends AbstractRestAction $identifier = ShortUrlIdentifier::fromApiRequest($request); $apiKey = AuthenticationMiddleware::apiKeyFromRequest($request); - $this->shortUrlService->updateMetadataByShortCode($identifier, $shortUrlEdit, $apiKey); - return new EmptyResponse(); + $shortUrl = $this->shortUrlService->updateShortUrl($identifier, $shortUrlEdit, $apiKey); + + return new JsonResponse($this->transformer->transform($shortUrl)); } } diff --git a/module/Rest/src/Action/ShortUrl/EditShortUrlTagsAction.php b/module/Rest/src/Action/ShortUrl/EditShortUrlTagsAction.php index 7d115765..d114049c 100644 --- a/module/Rest/src/Action/ShortUrl/EditShortUrlTagsAction.php +++ b/module/Rest/src/Action/ShortUrl/EditShortUrlTagsAction.php @@ -8,11 +8,14 @@ use Laminas\Diactoros\Response\JsonResponse; use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface as Request; use Shlinkio\Shlink\Core\Exception\ValidationException; +use Shlinkio\Shlink\Core\Model\ShortUrlEdit; use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Service\ShortUrlServiceInterface; +use Shlinkio\Shlink\Core\Validation\ShortUrlInputFilter; use Shlinkio\Shlink\Rest\Action\AbstractRestAction; use Shlinkio\Shlink\Rest\Middleware\AuthenticationMiddleware; +/** @deprecated */ class EditShortUrlTagsAction extends AbstractRestAction { protected const ROUTE_PATH = '/short-urls/{shortCode}/tags'; @@ -38,7 +41,9 @@ class EditShortUrlTagsAction extends AbstractRestAction $identifier = ShortUrlIdentifier::fromApiRequest($request); $apiKey = AuthenticationMiddleware::apiKeyFromRequest($request); - $shortUrl = $this->shortUrlService->setTagsByShortCode($identifier, $tags, $apiKey); + $shortUrl = $this->shortUrlService->updateShortUrl($identifier, ShortUrlEdit::fromRawData([ + ShortUrlInputFilter::TAGS => $tags, + ]), $apiKey); return new JsonResponse(['tags' => $shortUrl->getTags()->toArray()]); } } diff --git a/module/Rest/src/Action/ShortUrl/ListShortUrlsAction.php b/module/Rest/src/Action/ShortUrl/ListShortUrlsAction.php index 8da502cf..cd1bb4af 100644 --- a/module/Rest/src/Action/ShortUrl/ListShortUrlsAction.php +++ b/module/Rest/src/Action/ShortUrl/ListShortUrlsAction.php @@ -22,12 +22,12 @@ class ListShortUrlsAction extends AbstractRestAction protected const ROUTE_ALLOWED_METHODS = [self::METHOD_GET]; private ShortUrlServiceInterface $shortUrlService; - private array $domainConfig; + private ShortUrlDataTransformer $transformer; public function __construct(ShortUrlServiceInterface $shortUrlService, array $domainConfig) { $this->shortUrlService = $shortUrlService; - $this->domainConfig = $domainConfig; + $this->transformer = new ShortUrlDataTransformer($domainConfig); } public function handle(Request $request): Response @@ -36,8 +36,6 @@ class ListShortUrlsAction extends AbstractRestAction ShortUrlsParams::fromRawData($request->getQueryParams()), AuthenticationMiddleware::apiKeyFromRequest($request), ); - return new JsonResponse(['shortUrls' => $this->serializePaginator($shortUrls, new ShortUrlDataTransformer( - $this->domainConfig, - ))]); + return new JsonResponse(['shortUrls' => $this->serializePaginator($shortUrls, $this->transformer)]); } } diff --git a/module/Rest/src/Action/ShortUrl/ResolveShortUrlAction.php b/module/Rest/src/Action/ShortUrl/ResolveShortUrlAction.php index 99e58fee..fafd15df 100644 --- a/module/Rest/src/Action/ShortUrl/ResolveShortUrlAction.php +++ b/module/Rest/src/Action/ShortUrl/ResolveShortUrlAction.php @@ -19,22 +19,21 @@ class ResolveShortUrlAction extends AbstractRestAction protected const ROUTE_ALLOWED_METHODS = [self::METHOD_GET]; private ShortUrlResolverInterface $urlResolver; - private array $domainConfig; + private ShortUrlDataTransformer $transformer; public function __construct(ShortUrlResolverInterface $urlResolver, array $domainConfig) { $this->urlResolver = $urlResolver; - $this->domainConfig = $domainConfig; + $this->transformer = new ShortUrlDataTransformer($domainConfig); } public function handle(Request $request): Response { - $transformer = new ShortUrlDataTransformer($this->domainConfig); $url = $this->urlResolver->resolveShortUrl( ShortUrlIdentifier::fromApiRequest($request), AuthenticationMiddleware::apiKeyFromRequest($request), ); - return new JsonResponse($transformer->transform($url)); + return new JsonResponse($this->transformer->transform($url)); } } diff --git a/module/Rest/test-api/Action/CreateShortUrlTest.php b/module/Rest/test-api/Action/CreateShortUrlTest.php index 54ea0218..868ad142 100644 --- a/module/Rest/test-api/Action/CreateShortUrlTest.php +++ b/module/Rest/test-api/Action/CreateShortUrlTest.php @@ -60,13 +60,23 @@ class CreateShortUrlTest extends ApiTestCase } } - /** @test */ - public function createsNewShortUrlWithTags(): void + /** + * @test + * @dataProvider provideTags + */ + public function createsNewShortUrlWithTags(array $providedTags, array $expectedTags): void { - [$statusCode, ['tags' => $tags]] = $this->createShortUrl(['tags' => ['foo', 'bar', 'baz']]); + [$statusCode, ['tags' => $tags]] = $this->createShortUrl(['tags' => $providedTags]); self::assertEquals(self::STATUS_OK, $statusCode); - self::assertEquals(['foo', 'bar', 'baz'], $tags); + self::assertEquals($expectedTags, $tags); + } + + public function provideTags(): iterable + { + yield 'simple tags' => [$simpleTags = ['foo', 'bar', 'baz'], $simpleTags]; + yield 'tags with spaces' => [['fo o', ' bar', 'b az'], ['fo-o', 'bar', 'b-az']]; + yield 'tags with special chars' => [['UUU', 'Aäa'], ['uuu', 'aäa']]; } /** diff --git a/module/Rest/test-api/Action/EditShortUrlTagsTest.php b/module/Rest/test-api/Action/EditShortUrlTagsTest.php index f016882b..18f6f3b0 100644 --- a/module/Rest/test-api/Action/EditShortUrlTagsTest.php +++ b/module/Rest/test-api/Action/EditShortUrlTagsTest.php @@ -52,6 +52,25 @@ class EditShortUrlTagsTest extends ApiTestCase self::assertEquals($domain, $payload['domain'] ?? null); } + /** @test */ + public function allowsEditingTagsWithTwoEndpoints(): void + { + $getUrlTagsFromApi = fn () => $this->getJsonResponsePayload( + $this->callApiWithKey(self::METHOD_GET, '/short-urls/abc123'), + )['tags'] ?? null; + self::assertEquals(['foo'], $getUrlTagsFromApi()); + + $this->callApiWithKey(self::METHOD_PUT, '/short-urls/abc123/tags', [RequestOptions::JSON => [ + 'tags' => ['a', 'e'], + ]]); + self::assertEquals(['a', 'e'], $getUrlTagsFromApi()); + + $this->callApiWithKey(self::METHOD_PATCH, '/short-urls/abc123', [RequestOptions::JSON => [ + 'tags' => ['i', 'o', 'u'], + ]]); + self::assertEquals(['i', 'o', 'u'], $getUrlTagsFromApi()); + } + /** @test */ public function tagsAreSetOnProperShortUrlBasedOnProvidedDomain(): void { diff --git a/module/Rest/test-api/Action/EditShortUrlTest.php b/module/Rest/test-api/Action/EditShortUrlTest.php index 84612b0f..6652c1a4 100644 --- a/module/Rest/test-api/Action/EditShortUrlTest.php +++ b/module/Rest/test-api/Action/EditShortUrlTest.php @@ -41,8 +41,8 @@ class EditShortUrlTest extends ApiTestCase ]); $metaAfterResetting = $this->findShortUrlMetaByShortCode($shortCode); - self::assertEquals(self::STATUS_NO_CONTENT, $editWithProvidedMeta->getStatusCode()); - self::assertEquals(self::STATUS_NO_CONTENT, $editWithResetMeta->getStatusCode()); + self::assertEquals(self::STATUS_OK, $editWithProvidedMeta->getStatusCode()); + self::assertEquals(self::STATUS_OK, $editWithResetMeta->getStatusCode()); self::assertEquals($resetMeta, $metaAfterResetting); self::assertArraySubset($meta, $metaAfterEditing); } @@ -93,7 +93,7 @@ class EditShortUrlTest extends ApiTestCase public function provideLongUrls(): iterable { - yield 'valid URL' => ['https://shlink.io', self::STATUS_NO_CONTENT, null]; + yield 'valid URL' => ['https://shlink.io', self::STATUS_OK, null]; yield 'invalid URL' => ['htt:foo', self::STATUS_BAD_REQUEST, 'INVALID_URL']; } @@ -155,7 +155,7 @@ class EditShortUrlTest extends ApiTestCase ]]); $editedShortUrl = $this->getJsonResponsePayload($this->callApiWithKey(self::METHOD_GET, (string) $url)); - self::assertEquals(self::STATUS_NO_CONTENT, $editResp->getStatusCode()); + self::assertEquals(self::STATUS_OK, $editResp->getStatusCode()); self::assertEquals($domain, $editedShortUrl['domain']); self::assertEquals($expectedUrl, $editedShortUrl['longUrl']); self::assertEquals(100, $editedShortUrl['meta']['maxVisits'] ?? null); diff --git a/module/Rest/test-api/Action/ResolveShortUrlTest.php b/module/Rest/test-api/Action/ResolveShortUrlTest.php index d256a2ad..ca99f058 100644 --- a/module/Rest/test-api/Action/ResolveShortUrlTest.php +++ b/module/Rest/test-api/Action/ResolveShortUrlTest.php @@ -29,7 +29,7 @@ class ResolveShortUrlTest extends ApiTestCase $visitResp = $this->callShortUrl($shortCode); $fetchResp = $this->callApiWithKey(self::METHOD_GET, $url); - self::assertEquals(self::STATUS_NO_CONTENT, $editResp->getStatusCode()); + self::assertEquals(self::STATUS_OK, $editResp->getStatusCode()); self::assertEquals(self::STATUS_NOT_FOUND, $visitResp->getStatusCode()); self::assertEquals(self::STATUS_OK, $fetchResp->getStatusCode()); } diff --git a/module/Rest/test-api/Fixtures/ShortUrlsFixture.php b/module/Rest/test-api/Fixtures/ShortUrlsFixture.php index 390a2144..93defe90 100644 --- a/module/Rest/test-api/Fixtures/ShortUrlsFixture.php +++ b/module/Rest/test-api/Fixtures/ShortUrlsFixture.php @@ -18,18 +18,23 @@ class ShortUrlsFixture extends AbstractFixture implements DependentFixtureInterf { public function getDependencies(): array { - return [ApiKeyFixture::class]; + return [ApiKeyFixture::class, TagsFixture::class]; } public function load(ObjectManager $manager): void { + $relationResolver = new PersistenceShortUrlRelationResolver($manager); + /** @var ApiKey $authorApiKey */ $authorApiKey = $this->getReference('author_api_key'); $abcShortUrl = $this->setShortUrlDate( - ShortUrl::fromMeta(ShortUrlMeta::fromRawData( - ['customSlug' => 'abc123', 'apiKey' => $authorApiKey, 'longUrl' => 'https://shlink.io'], - )), + ShortUrl::fromMeta(ShortUrlMeta::fromRawData([ + 'customSlug' => 'abc123', + 'apiKey' => $authorApiKey, + 'longUrl' => 'https://shlink.io', + 'tags' => ['foo'], + ]), $relationResolver), '2018-05-01', ); $manager->persist($abcShortUrl); @@ -40,7 +45,8 @@ class ShortUrlsFixture extends AbstractFixture implements DependentFixtureInterf 'apiKey' => $authorApiKey, 'longUrl' => 'https://blog.alejandrocelaya.com/2017/12/09/acmailer-7-0-the-most-important-release-in-a-long-time/', - ])), '2019-01-01 00:00:10'); + 'tags' => ['foo', 'bar'], + ]), $relationResolver), '2019-01-01 00:00:10'); $manager->persist($defShortUrl); $customShortUrl = $this->setShortUrlDate(ShortUrl::fromMeta(ShortUrlMeta::fromRawData( @@ -61,7 +67,8 @@ class ShortUrlsFixture extends AbstractFixture implements DependentFixtureInterf 'customSlug' => 'ghi789', 'longUrl' => 'https://blog.alejandrocelaya.com/2019/04/27/considerations-to-properly-use-open-' . 'source-software-projects/', - ]), new PersistenceShortUrlRelationResolver($manager)), '2019-01-01 00:00:30'); + 'tags' => ['foo'], + ]), $relationResolver), '2019-01-01 00:00:30'); $manager->persist($withDomainDuplicatingShortCode); $withDomainAndSlugShortUrl = $this->setShortUrlDate(ShortUrl::fromMeta(ShortUrlMeta::fromRawData( diff --git a/module/Rest/test-api/Fixtures/TagsFixture.php b/module/Rest/test-api/Fixtures/TagsFixture.php index bf16104e..a28357a1 100644 --- a/module/Rest/test-api/Fixtures/TagsFixture.php +++ b/module/Rest/test-api/Fixtures/TagsFixture.php @@ -4,40 +4,18 @@ declare(strict_types=1); namespace ShlinkioApiTest\Shlink\Rest\Fixtures; -use Doctrine\Common\Collections\ArrayCollection; use Doctrine\Common\DataFixtures\AbstractFixture; -use Doctrine\Common\DataFixtures\DependentFixtureInterface; use Doctrine\Persistence\ObjectManager; -use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Entity\Tag; -class TagsFixture extends AbstractFixture implements DependentFixtureInterface +class TagsFixture extends AbstractFixture { - public function getDependencies(): array - { - return [ShortUrlsFixture::class]; - } - public function load(ObjectManager $manager): void { - $fooTag = new Tag('foo'); - $manager->persist($fooTag); - $barTag = new Tag('bar'); - $manager->persist($barTag); + $manager->persist(new Tag('foo')); + $manager->persist(new Tag('bar')); $manager->persist(new Tag('baz')); - /** @var ShortUrl $abcShortUrl */ - $abcShortUrl = $this->getReference('abc123_short_url'); - $abcShortUrl->setTags(new ArrayCollection([$fooTag])); - - /** @var ShortUrl $defShortUrl */ - $defShortUrl = $this->getReference('def456_short_url'); - $defShortUrl->setTags(new ArrayCollection([$fooTag, $barTag])); - - /** @var ShortUrl $exampleShortUrl */ - $exampleShortUrl = $this->getReference('example_short_url'); - $exampleShortUrl->setTags(new ArrayCollection([$fooTag])); - $manager->flush(); } } diff --git a/module/Rest/test/Action/ShortUrl/EditShortUrlActionTest.php b/module/Rest/test/Action/ShortUrl/EditShortUrlActionTest.php index ad482098..be70eec8 100644 --- a/module/Rest/test/Action/ShortUrl/EditShortUrlActionTest.php +++ b/module/Rest/test/Action/ShortUrl/EditShortUrlActionTest.php @@ -25,7 +25,7 @@ class EditShortUrlActionTest extends TestCase public function setUp(): void { $this->shortUrlService = $this->prophesize(ShortUrlServiceInterface::class); - $this->action = new EditShortUrlAction($this->shortUrlService->reveal()); + $this->action = new EditShortUrlAction($this->shortUrlService->reveal(), []); } /** @test */ @@ -48,13 +48,13 @@ class EditShortUrlActionTest extends TestCase ->withParsedBody([ 'maxVisits' => 5, ]); - $updateMeta = $this->shortUrlService->updateMetadataByShortCode(Argument::cetera())->willReturn( + $updateMeta = $this->shortUrlService->updateShortUrl(Argument::cetera())->willReturn( ShortUrl::createEmpty(), ); $resp = $this->action->handle($request); - self::assertEquals(204, $resp->getStatusCode()); + self::assertEquals(200, $resp->getStatusCode()); $updateMeta->shouldHaveBeenCalled(); } } diff --git a/module/Rest/test/Action/ShortUrl/EditShortUrlTagsActionTest.php b/module/Rest/test/Action/ShortUrl/EditShortUrlTagsActionTest.php index 60d1d093..a345046a 100644 --- a/module/Rest/test/Action/ShortUrl/EditShortUrlTagsActionTest.php +++ b/module/Rest/test/Action/ShortUrl/EditShortUrlTagsActionTest.php @@ -12,6 +12,7 @@ use Prophecy\Prophecy\ObjectProphecy; use Psr\Http\Message\ServerRequestInterface; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Exception\ValidationException; +use Shlinkio\Shlink\Core\Model\ShortUrlEdit; use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Service\ShortUrlService; use Shlinkio\Shlink\Rest\Action\ShortUrl\EditShortUrlTagsAction; @@ -41,9 +42,9 @@ class EditShortUrlTagsActionTest extends TestCase public function tagsListIsReturnedIfCorrectShortCodeIsProvided(): void { $shortCode = 'abc123'; - $this->shortUrlService->setTagsByShortCode( + $this->shortUrlService->updateShortUrl( new ShortUrlIdentifier($shortCode), - [], + Argument::type(ShortUrlEdit::class), Argument::type(ApiKey::class), )->willReturn(ShortUrl::createEmpty()) ->shouldBeCalledOnce();