diff --git a/CHANGELOG.md b/CHANGELOG.md index e81dc273..aad20edc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,10 +27,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this * [#1300](https://github.com/shlinkio/shlink/issues/1300) Changed default ordering for short URLs list, returning always from newest to oldest. ### Deprecated -* *Nothing* +* [#1315](https://github.com/shlinkio/shlink/issues/1315) Deprecated `GET /tags?withStats=true` endpoint. Use `GET /tags/stats` instead. ### Removed -* [#1275](https://github.com/shlinkio/shlink/issues/1275) Removed everything that was deprecated. +* [#1275](https://github.com/shlinkio/shlink/issues/1275) Removed everything that was deprecated in Shlink 2.x. See [UPGRADE](UPGRADE.md#from-v2x-to-v3x) doc in order to get details on how to migrate to this version. diff --git a/docs/swagger/paths/v1_tags.json b/docs/swagger/paths/v1_tags.json index 1e36e112..a8219bf1 100644 --- a/docs/swagger/paths/v1_tags.json +++ b/docs/swagger/paths/v1_tags.json @@ -5,7 +5,7 @@ "Tags" ], "summary": "List existing tags", - "description": "Returns the list of all tags used in any short URL, ordered by name", + "description": "Returns the list of all tags used in any short URL", "security": [ { "ApiKey": [] @@ -17,7 +17,8 @@ }, { "name": "withStats", - "description": "Whether you want to include also a list with general stats by tag or not. Defaults to false.", + "deprecated": true, + "description": "**[Deprecated]** Use [GET /tags/stats](#/Tags/tagsWithStats) endpoint to get tags with their stats.", "in": "query", "required": false, "schema": { @@ -101,53 +102,20 @@ } } }, - "examples": { - "Without stats": { - "value": { - "tags": { - "data": [ - "games", - "php", - "shlink", - "tech" - ], - "pagination": { - "currentPage": 5, - "pagesCount": 10, - "itemsPerPage": 4, - "itemsInCurrentPage": 4, - "totalItems": 38 - } - } - } - }, - "With stats": { - "value": { - "tags": { - "data": [ - "games", - "shlink" - ], - "stats": [ - { - "tag": "games", - "shortUrlsCount": 10, - "visitsCount": 521 - }, - { - "tag": "shlink", - "shortUrlsCount": 7, - "visitsCount": 1087 - } - ], - "pagination": { - "currentPage": 5, - "pagesCount": 5, - "itemsPerPage": 10, - "itemsInCurrentPage": 2, - "totalItems": 42 - } - } + "example": { + "tags": { + "data": [ + "games", + "php", + "shlink", + "tech" + ], + "pagination": { + "currentPage": 5, + "pagesCount": 10, + "itemsPerPage": 4, + "itemsInCurrentPage": 4, + "totalItems": 38 } } } diff --git a/docs/swagger/paths/v2_tags_stats.json b/docs/swagger/paths/v2_tags_stats.json new file mode 100644 index 00000000..bd745fd0 --- /dev/null +++ b/docs/swagger/paths/v2_tags_stats.json @@ -0,0 +1,123 @@ +{ + "get": { + "operationId": "tagsWithStats", + "tags": [ + "Tags" + ], + "summary": "Get tags with stats", + "description": "Returns the list of all tags used in any short URL, together with the amount of short URLs and visits for it", + "security": [ + { + "ApiKey": [] + } + ], + "parameters": [ + { + "$ref": "../parameters/version.json" + }, + { + "name": "page", + "in": "query", + "description": "The page to display. Defaults to 1", + "required": false, + "schema": { + "type": "number" + } + }, + { + "name": "itemsPerPage", + "in": "query", + "description": "The amount of items to return on every page. Defaults to all the items", + "required": false, + "schema": { + "type": "number" + } + }, + { + "name": "searchTerm", + "in": "query", + "description": "A query used to filter results by searching for it on the tag name.", + "required": false, + "schema": { + "type": "string" + } + }, + { + "name": "orderBy", + "in": "query", + "description": "To determine how to order the results.", + "required": false, + "schema": { + "type": "string", + "enum": [ + "tag-ASC", + "tag-DESC" + ] + } + } + ], + "responses": { + "200": { + "description": "The list of tags", + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": { + "tags": { + "type": "object", + "required": ["data"], + "properties": { + "data": { + "description": "The tag stats will be returned only if the withStats param was provided with value 'true'", + "type": "array", + "items": { + "$ref": "../definitions/TagInfo.json" + } + }, + "pagination": { + "$ref": "../definitions/Pagination.json" + } + } + } + } + }, + "example": { + "tags": { + "data": [ + { + "tag": "games", + "shortUrlsCount": 10, + "visitsCount": 521 + }, + { + "tag": "shlink", + "shortUrlsCount": 7, + "visitsCount": 1087 + } + ], + "pagination": { + "currentPage": 5, + "pagesCount": 5, + "itemsPerPage": 10, + "itemsInCurrentPage": 2, + "totalItems": 42 + } + } + } + } + } + }, + "default": { + "description": "Unexpected error.", + "content": { + "application/problem+json": { + "schema": { + "$ref": "../definitions/Error.json" + } + } + } + } + } + } +} diff --git a/docs/swagger/swagger.json b/docs/swagger/swagger.json index 8e71f362..f04510d0 100644 --- a/docs/swagger/swagger.json +++ b/docs/swagger/swagger.json @@ -82,6 +82,9 @@ "/rest/v{version}/tags": { "$ref": "paths/v1_tags.json" }, + "/rest/v{version}/tags/stats": { + "$ref": "paths/v2_tags_stats.json" + }, "/rest/v{version}/visits": { "$ref": "paths/v2_visits.json" diff --git a/module/Rest/config/dependencies.config.php b/module/Rest/config/dependencies.config.php index 7e48552e..e7d99a85 100644 --- a/module/Rest/config/dependencies.config.php +++ b/module/Rest/config/dependencies.config.php @@ -35,6 +35,7 @@ return [ Action\Visit\GlobalVisitsAction::class => ConfigAbstractFactory::class, Action\Visit\OrphanVisitsAction::class => ConfigAbstractFactory::class, Action\Tag\ListTagsAction::class => ConfigAbstractFactory::class, + Action\Tag\TagsStatsAction::class => ConfigAbstractFactory::class, Action\Tag\DeleteTagsAction::class => ConfigAbstractFactory::class, Action\Tag\UpdateTagAction::class => ConfigAbstractFactory::class, Action\Domain\ListDomainsAction::class => ConfigAbstractFactory::class, @@ -75,6 +76,7 @@ return [ ], Action\ShortUrl\ListShortUrlsAction::class => [Service\ShortUrlService::class, ShortUrlDataTransformer::class], Action\Tag\ListTagsAction::class => [TagService::class], + Action\Tag\TagsStatsAction::class => [TagService::class], Action\Tag\DeleteTagsAction::class => [TagService::class], Action\Tag\UpdateTagAction::class => [TagService::class], Action\Domain\ListDomainsAction::class => [DomainService::class, Options\NotFoundRedirectOptions::class], diff --git a/module/Rest/config/routes.config.php b/module/Rest/config/routes.config.php index 4af6304d..49d9f107 100644 --- a/module/Rest/config/routes.config.php +++ b/module/Rest/config/routes.config.php @@ -37,6 +37,7 @@ return [ // Tags Action\Tag\ListTagsAction::getRouteDef(), + Action\Tag\TagsStatsAction::getRouteDef(), Action\Tag\DeleteTagsAction::getRouteDef(), Action\Tag\UpdateTagAction::getRouteDef(), diff --git a/module/Rest/src/Action/Tag/ListTagsAction.php b/module/Rest/src/Action/Tag/ListTagsAction.php index ecf379cb..ba25ffe5 100644 --- a/module/Rest/src/Action/Tag/ListTagsAction.php +++ b/module/Rest/src/Action/Tag/ListTagsAction.php @@ -29,8 +29,7 @@ class ListTagsAction extends AbstractRestAction public function handle(ServerRequestInterface $request): ResponseInterface { - $query = $request->getQueryParams(); - $params = TagsParams::fromRawData($query); + $params = TagsParams::fromRawData($request->getQueryParams()); $apiKey = AuthenticationMiddleware::apiKeyFromRequest($request); if (! $params->withStats()) { @@ -39,6 +38,7 @@ class ListTagsAction extends AbstractRestAction ]); } + // This part is deprecated. To get tags with stats, the /tags/stats endpoint should be used instead $tagsInfo = $this->tagService->tagsInfo($params, $apiKey); $rawTags = $this->serializePaginator($tagsInfo, null, 'stats'); $rawTags['data'] = map($tagsInfo, static fn (TagInfo $info) => $info->tag()->__toString()); diff --git a/module/Rest/src/Action/Tag/TagsStatsAction.php b/module/Rest/src/Action/Tag/TagsStatsAction.php new file mode 100644 index 00000000..cec8edd6 --- /dev/null +++ b/module/Rest/src/Action/Tag/TagsStatsAction.php @@ -0,0 +1,35 @@ +getQueryParams()); + $apiKey = AuthenticationMiddleware::apiKeyFromRequest($request); + $tagsInfo = $this->tagService->tagsInfo($params, $apiKey); + + return new JsonResponse(['tags' => $this->serializePaginator($tagsInfo)]); + } +} diff --git a/module/Rest/test-api/Action/ListTagsTest.php b/module/Rest/test-api/Action/ListTagsTest.php index a1dade87..4c627e7c 100644 --- a/module/Rest/test-api/Action/ListTagsTest.php +++ b/module/Rest/test-api/Action/ListTagsTest.php @@ -23,7 +23,7 @@ class ListTagsTest extends ApiTestCase public function provideQueries(): iterable { - yield 'admin API key without stats' => ['valid_api_key', [], [ + yield 'admin API key' => ['valid_api_key', [], [ 'data' => ['bar', 'baz', 'foo'], 'pagination' => [ 'currentPage' => 1, @@ -43,61 +43,7 @@ class ListTagsTest extends ApiTestCase 'totalItems' => 3, ], ]]; - yield 'admin API key with stats' => ['valid_api_key', ['withStats' => 'true'], [ - 'data' => ['bar', 'baz', 'foo'], - 'stats' => [ - [ - 'tag' => 'bar', - 'shortUrlsCount' => 1, - 'visitsCount' => 2, - ], - [ - 'tag' => 'baz', - 'shortUrlsCount' => 0, - 'visitsCount' => 0, - ], - [ - 'tag' => 'foo', - 'shortUrlsCount' => 3, - 'visitsCount' => 5, - ], - ], - 'pagination' => [ - 'currentPage' => 1, - 'pagesCount' => 1, - 'itemsPerPage' => 3, - 'itemsInCurrentPage' => 3, - 'totalItems' => 3, - ], - ]]; - yield 'admin API key with pagination and stats' => ['valid_api_key', [ - 'withStats' => 'true', - 'page' => 1, - 'itemsPerPage' => 2, - ], [ - 'data' => ['bar', 'baz'], - 'stats' => [ - [ - 'tag' => 'bar', - 'shortUrlsCount' => 1, - 'visitsCount' => 2, - ], - [ - 'tag' => 'baz', - 'shortUrlsCount' => 0, - 'visitsCount' => 0, - ], - ], - 'pagination' => [ - 'currentPage' => 1, - 'pagesCount' => 2, - 'itemsPerPage' => 2, - 'itemsInCurrentPage' => 2, - 'totalItems' => 3, - ], - ]]; - - yield 'author API key without stats' => ['author_api_key', [], [ + yield 'author API key' => ['author_api_key', [], [ 'data' => ['bar', 'foo'], 'pagination' => [ 'currentPage' => 1, @@ -107,30 +53,7 @@ class ListTagsTest extends ApiTestCase 'totalItems' => 2, ], ]]; - yield 'author API key with stats' => ['author_api_key', ['withStats' => 'true'], [ - 'data' => ['bar', 'foo'], - 'stats' => [ - [ - 'tag' => 'bar', - 'shortUrlsCount' => 1, - 'visitsCount' => 2, - ], - [ - 'tag' => 'foo', - 'shortUrlsCount' => 2, - 'visitsCount' => 5, - ], - ], - 'pagination' => [ - 'currentPage' => 1, - 'pagesCount' => 1, - 'itemsPerPage' => 2, - 'itemsInCurrentPage' => 2, - 'totalItems' => 2, - ], - ]]; - - yield 'domain API key without stats' => ['domain_api_key', [], [ + yield 'domain API key' => ['domain_api_key', [], [ 'data' => ['foo'], 'pagination' => [ 'currentPage' => 1, @@ -140,22 +63,5 @@ class ListTagsTest extends ApiTestCase 'totalItems' => 1, ], ]]; - yield 'domain API key with stats' => ['domain_api_key', ['withStats' => 'true'], [ - 'data' => ['foo'], - 'stats' => [ - [ - 'tag' => 'foo', - 'shortUrlsCount' => 1, - 'visitsCount' => 0, - ], - ], - 'pagination' => [ - 'currentPage' => 1, - 'pagesCount' => 1, - 'itemsPerPage' => 1, - 'itemsInCurrentPage' => 1, - 'totalItems' => 1, - ], - ]]; } } diff --git a/module/Rest/test-api/Action/TagsStatsTest.php b/module/Rest/test-api/Action/TagsStatsTest.php new file mode 100644 index 00000000..3b91cbf0 --- /dev/null +++ b/module/Rest/test-api/Action/TagsStatsTest.php @@ -0,0 +1,136 @@ +callApiWithKey(self::METHOD_GET, '/tags/stats', [RequestOptions::QUERY => $query], $apiKey); + ['tags' => $tags] = $this->getJsonResponsePayload($resp); + + self::assertEquals($expectedStats, $tags['data']); + self::assertEquals($expectedPagination, $tags['pagination']); + } + + /** + * @test + * @dataProvider provideQueries + */ + public function expectedListOfTagsIsReturnedForDeprecatedApproach( + string $apiKey, + array $query, + array $expectedStats, + array $expectedPagination, + ): void { + $query['withStats'] = 'true'; + $resp = $this->callApiWithKey(self::METHOD_GET, '/tags', [RequestOptions::QUERY => $query], $apiKey); + ['tags' => $tags] = $this->getJsonResponsePayload($resp); + + self::assertEquals($expectedStats, $tags['stats']); + self::assertEquals($expectedPagination, $tags['pagination']); + self::assertArrayHasKey('data', $tags); + } + + public function provideQueries(): iterable + { + yield 'admin API key' => ['valid_api_key', [], [ + [ + 'tag' => 'bar', + 'shortUrlsCount' => 1, + 'visitsCount' => 2, + ], + [ + 'tag' => 'baz', + 'shortUrlsCount' => 0, + 'visitsCount' => 0, + ], + [ + 'tag' => 'foo', + 'shortUrlsCount' => 3, + 'visitsCount' => 5, + ], + ], [ + 'currentPage' => 1, + 'pagesCount' => 1, + 'itemsPerPage' => 3, + 'itemsInCurrentPage' => 3, + 'totalItems' => 3, + ]]; + yield 'admin API key with pagination' => ['valid_api_key', ['page' => 1, 'itemsPerPage' => 2], [ + [ + 'tag' => 'bar', + 'shortUrlsCount' => 1, + 'visitsCount' => 2, + ], + [ + 'tag' => 'baz', + 'shortUrlsCount' => 0, + 'visitsCount' => 0, + ], + ], [ + 'currentPage' => 1, + 'pagesCount' => 2, + 'itemsPerPage' => 2, + 'itemsInCurrentPage' => 2, + 'totalItems' => 3, + ]]; + yield 'author API key' => ['author_api_key', [], [ + [ + 'tag' => 'bar', + 'shortUrlsCount' => 1, + 'visitsCount' => 2, + ], + [ + 'tag' => 'foo', + 'shortUrlsCount' => 2, + 'visitsCount' => 5, + ], + ], [ + 'currentPage' => 1, + 'pagesCount' => 1, + 'itemsPerPage' => 2, + 'itemsInCurrentPage' => 2, + 'totalItems' => 2, + ]]; + yield 'author API key with pagination' => ['author_api_key', ['page' => 2, 'itemsPerPage' => 1], [ + [ + 'tag' => 'foo', + 'shortUrlsCount' => 2, + 'visitsCount' => 5, + ], + ], [ + 'currentPage' => 2, + 'pagesCount' => 2, + 'itemsPerPage' => 1, + 'itemsInCurrentPage' => 1, + 'totalItems' => 2, + ]]; + yield 'domain API key' => ['domain_api_key', [], [ + [ + 'tag' => 'foo', + 'shortUrlsCount' => 1, + 'visitsCount' => 0, + ], + ], [ + 'currentPage' => 1, + 'pagesCount' => 1, + 'itemsPerPage' => 1, + 'itemsInCurrentPage' => 1, + 'totalItems' => 1, + ]]; + } +} diff --git a/module/Rest/test/Action/Tag/TagsStatsActionTest.php b/module/Rest/test/Action/Tag/TagsStatsActionTest.php new file mode 100644 index 00000000..3f98b64e --- /dev/null +++ b/module/Rest/test/Action/Tag/TagsStatsActionTest.php @@ -0,0 +1,73 @@ +tagService = $this->prophesize(TagServiceInterface::class); + $this->action = new TagsStatsAction($this->tagService->reveal()); + } + + /** @test */ + public function returnsTagsStatsWhenRequested(): void + { + $stats = [ + new TagInfo(new Tag('foo'), 1, 1), + new TagInfo(new Tag('bar'), 3, 10), + ]; + $itemsCount = count($stats); + $tagsInfo = $this->tagService->tagsInfo(Argument::any(), Argument::type(ApiKey::class))->willReturn( + new Paginator(new ArrayAdapter($stats)), + ); + $req = $this->requestWithApiKey()->withQueryParams(['withStats' => 'true']); + + /** @var JsonResponse $resp */ + $resp = $this->action->handle($req); + $payload = $resp->getPayload(); + + self::assertEquals([ + 'tags' => [ + 'data' => $stats, + 'pagination' => [ + 'currentPage' => 1, + 'pagesCount' => 1, + 'itemsPerPage' => 10, + 'itemsInCurrentPage' => $itemsCount, + 'totalItems' => $itemsCount, + ], + ], + ], $payload); + $tagsInfo->shouldHaveBeenCalled(); + } + + private function requestWithApiKey(): ServerRequestInterface + { + return ServerRequestFactory::fromGlobals()->withAttribute(ApiKey::class, ApiKey::create()); + } +}