diff --git a/docs/swagger/paths/v1_short-urls.json b/docs/swagger/paths/v1_short-urls.json index a89dd187..a81853d8 100644 --- a/docs/swagger/paths/v1_short-urls.json +++ b/docs/swagger/paths/v1_short-urls.json @@ -191,7 +191,7 @@ "Short URLs" ], "summary": "Create short URL", - "description": "Creates a new short URL.

**Param findIfExists:**: Starting with v1.16, this new param allows to force shlink to return existing short URLs when found based on provided params, instead of creating a new one. However, it might add complexity and have unexpected outputs.\n\nThese are the use cases:\n* Only the long URL is provided: It will return the newest match or create a new short URL if none is found.\n* Long url and custom slug are provided: It will return the short URL when both params match, return an error when the slug is in use for another long URL, or create a new short URL otherwise.\n* Any of the above but including other params (tags, validSince, validUntil, maxVisits): It will behave the same as the previous two cases, but it will try to exactly match existing results using all the params. If any of them does not match, it will try to create a new short URL.", + "description": "Creates a new short URL.

**Param findIfExists**: This new param allows to force shlink to return existing short URLs when found based on provided params, instead of creating a new one. However, it might add complexity and have unexpected outputs.\n\nThese are the use cases:\n* Only the long URL is provided: It will return the newest match or create a new short URL if none is found.\n* Long url and custom slug are provided: It will return the short URL when both params match, return an error when the slug is in use for another long URL, or create a new short URL otherwise.\n* Any of the above but including other params (tags, validSince, validUntil, maxVisits): It will behave the same as the previous two cases, but it will try to exactly match existing results using all the params. If any of them does not match, it will try to create a new short URL.", "security": [ { "ApiKey": [] diff --git a/module/Core/src/Repository/TagRepository.php b/module/Core/src/Repository/TagRepository.php index 7cb66a6a..dd15c292 100644 --- a/module/Core/src/Repository/TagRepository.php +++ b/module/Core/src/Repository/TagRepository.php @@ -56,7 +56,7 @@ class TagRepository extends EntitySpecificationRepository implements TagReposito { $result = (int) $this->matchSingleScalarResult(Spec::andX( new CountTagsWithName($tag), - new WithApiKeySpecsEnsuringJoin($apiKey, 'shortUrls'), + new WithApiKeySpecsEnsuringJoin($apiKey), )); return $result > 0; diff --git a/module/Core/src/Tag/TagService.php b/module/Core/src/Tag/TagService.php index 786e102d..342c5d31 100644 --- a/module/Core/src/Tag/TagService.php +++ b/module/Core/src/Tag/TagService.php @@ -6,6 +6,7 @@ namespace Shlinkio\Shlink\Core\Tag; use Doctrine\Common\Collections\Collection; use Doctrine\ORM; +use Happyr\DoctrineSpecification\Spec; use Shlinkio\Shlink\Core\Entity\Tag; use Shlinkio\Shlink\Core\Exception\TagConflictException; use Shlinkio\Shlink\Core\Exception\TagNotFoundException; @@ -13,6 +14,7 @@ use Shlinkio\Shlink\Core\Repository\TagRepository; use Shlinkio\Shlink\Core\Repository\TagRepositoryInterface; use Shlinkio\Shlink\Core\Tag\Model\TagInfo; use Shlinkio\Shlink\Core\Util\TagManagerTrait; +use Shlinkio\Shlink\Rest\ApiKey\Spec\WithApiKeySpecsEnsuringJoin; use Shlinkio\Shlink\Rest\Entity\ApiKey; class TagService implements TagServiceInterface @@ -29,10 +31,15 @@ class TagService implements TagServiceInterface /** * @return Tag[] */ - public function listTags(): array + public function listTags(?ApiKey $apiKey = null): array { + /** @var TagRepository $repo */ + $repo = $this->em->getRepository(Tag::class); /** @var Tag[] $tags */ - $tags = $this->em->getRepository(Tag::class)->findBy([], ['name' => 'ASC']); + $tags = $repo->match(Spec::andX( + Spec::orderBy('name'), + new WithApiKeySpecsEnsuringJoin($apiKey), + )); return $tags; } diff --git a/module/Core/src/Tag/TagServiceInterface.php b/module/Core/src/Tag/TagServiceInterface.php index bd96a225..698736a6 100644 --- a/module/Core/src/Tag/TagServiceInterface.php +++ b/module/Core/src/Tag/TagServiceInterface.php @@ -16,7 +16,7 @@ interface TagServiceInterface /** * @return Tag[] */ - public function listTags(): array; + public function listTags(?ApiKey $apiKey = null): array; /** * @return TagInfo[] diff --git a/module/Core/test/Service/Tag/TagServiceTest.php b/module/Core/test/Service/Tag/TagServiceTest.php index c4203c85..a66fdc6f 100644 --- a/module/Core/test/Service/Tag/TagServiceTest.php +++ b/module/Core/test/Service/Tag/TagServiceTest.php @@ -38,12 +38,12 @@ class TagServiceTest extends TestCase { $expected = [new Tag('foo'), new Tag('bar')]; - $find = $this->repo->findBy(Argument::cetera())->willReturn($expected); + $match = $this->repo->match(Argument::cetera())->willReturn($expected); $result = $this->service->listTags(); self::assertEquals($expected, $result); - $find->shouldHaveBeenCalled(); + $match->shouldHaveBeenCalled(); } /** @test */ diff --git a/module/Rest/src/Action/Tag/ListTagsAction.php b/module/Rest/src/Action/Tag/ListTagsAction.php index 64ddad33..48cf923b 100644 --- a/module/Rest/src/Action/Tag/ListTagsAction.php +++ b/module/Rest/src/Action/Tag/ListTagsAction.php @@ -30,16 +30,16 @@ class ListTagsAction extends AbstractRestAction { $query = $request->getQueryParams(); $withStats = ($query['withStats'] ?? null) === 'true'; + $apiKey = AuthenticationMiddleware::apiKeyFromRequest($request); if (! $withStats) { return new JsonResponse([ 'tags' => [ - 'data' => $this->tagService->listTags(), + 'data' => $this->tagService->listTags($apiKey), ], ]); } - $apiKey = AuthenticationMiddleware::apiKeyFromRequest($request); $tagsInfo = $this->tagService->tagsInfo($apiKey); $data = map($tagsInfo, fn (TagInfo $info) => (string) $info->tag()); diff --git a/module/Rest/src/ApiKey/Spec/WithApiKeySpecsEnsuringJoin.php b/module/Rest/src/ApiKey/Spec/WithApiKeySpecsEnsuringJoin.php index 04ed9565..64359d15 100644 --- a/module/Rest/src/ApiKey/Spec/WithApiKeySpecsEnsuringJoin.php +++ b/module/Rest/src/ApiKey/Spec/WithApiKeySpecsEnsuringJoin.php @@ -14,7 +14,7 @@ class WithApiKeySpecsEnsuringJoin extends BaseSpecification private ?ApiKey $apiKey; private string $fieldToJoin; - public function __construct(?ApiKey $apiKey, string $fieldToJoin) + public function __construct(?ApiKey $apiKey, string $fieldToJoin = 'shortUrls') { parent::__construct(); $this->apiKey = $apiKey; diff --git a/module/Rest/test/Action/Tag/ListTagsActionTest.php b/module/Rest/test/Action/Tag/ListTagsActionTest.php index c8b65a48..9bdad15b 100644 --- a/module/Rest/test/Action/Tag/ListTagsActionTest.php +++ b/module/Rest/test/Action/Tag/ListTagsActionTest.php @@ -7,8 +7,10 @@ namespace ShlinkioTest\Shlink\Rest\Action\Tag; use Laminas\Diactoros\Response\JsonResponse; use Laminas\Diactoros\ServerRequestFactory; use PHPUnit\Framework\TestCase; +use Prophecy\Argument; use Prophecy\PhpUnit\ProphecyTrait; use Prophecy\Prophecy\ObjectProphecy; +use Psr\Http\Message\ServerRequestInterface; use Shlinkio\Shlink\Core\Entity\Tag; use Shlinkio\Shlink\Core\Tag\Model\TagInfo; use Shlinkio\Shlink\Core\Tag\TagServiceInterface; @@ -35,10 +37,10 @@ class ListTagsActionTest extends TestCase public function returnsBaseDataWhenStatsAreNotRequested(array $query): void { $tags = [new Tag('foo'), new Tag('bar')]; - $listTags = $this->tagService->listTags()->willReturn($tags); + $listTags = $this->tagService->listTags(Argument::type(ApiKey::class))->willReturn($tags); /** @var JsonResponse $resp */ - $resp = $this->action->handle(ServerRequestFactory::fromGlobals()->withQueryParams($query)); + $resp = $this->action->handle($this->requestWithApiKey()->withQueryParams($query)); $payload = $resp->getPayload(); self::assertEquals([ @@ -63,10 +65,8 @@ class ListTagsActionTest extends TestCase new TagInfo(new Tag('foo'), 1, 1), new TagInfo(new Tag('bar'), 3, 10), ]; - $apiKey = new ApiKey(); - $tagsInfo = $this->tagService->tagsInfo($apiKey)->willReturn($stats); - $req = ServerRequestFactory::fromGlobals()->withQueryParams(['withStats' => 'true']) - ->withAttribute(ApiKey::class, $apiKey); + $tagsInfo = $this->tagService->tagsInfo(Argument::type(ApiKey::class))->willReturn($stats); + $req = $this->requestWithApiKey()->withQueryParams(['withStats' => 'true']); /** @var JsonResponse $resp */ $resp = $this->action->handle($req); @@ -80,4 +80,9 @@ class ListTagsActionTest extends TestCase ], $payload); $tagsInfo->shouldHaveBeenCalled(); } + + private function requestWithApiKey(): ServerRequestInterface + { + return ServerRequestFactory::fromGlobals()->withAttribute(ApiKey::class, new ApiKey()); + } }