From a12c9f54c4c91af2bcbf7b7c2435e6fd24ad2c7d Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 22 May 2021 21:05:54 +0200 Subject: [PATCH] Added API tests covering the excludion of bot visits --- .../Adapter/OrphanVisitsPaginatorAdapter.php | 5 +++- .../Rest/test-api/Action/OrphanVisitsTest.php | 23 +++++++++++---- .../test-api/Action/ShortUrlVisitsTest.php | 26 +++++++++++++++++ module/Rest/test-api/Action/TagVisitsTest.php | 29 +++++++++++++------ .../Rest/test-api/Fixtures/VisitsFixture.php | 2 +- 5 files changed, 68 insertions(+), 17 deletions(-) diff --git a/module/Core/src/Paginator/Adapter/OrphanVisitsPaginatorAdapter.php b/module/Core/src/Paginator/Adapter/OrphanVisitsPaginatorAdapter.php index 0cc9ae7d..d7361fb3 100644 --- a/module/Core/src/Paginator/Adapter/OrphanVisitsPaginatorAdapter.php +++ b/module/Core/src/Paginator/Adapter/OrphanVisitsPaginatorAdapter.php @@ -22,7 +22,10 @@ class OrphanVisitsPaginatorAdapter extends AbstractCacheableCountPaginatorAdapte protected function doCount(): int { - return $this->repo->countOrphanVisits(new VisitsCountFiltering($this->params->getDateRange())); + return $this->repo->countOrphanVisits(new VisitsCountFiltering( + $this->params->getDateRange(), + $this->params->excludeBots(), + )); } public function getSlice($offset, $length): iterable // phpcs:ignore diff --git a/module/Rest/test-api/Action/OrphanVisitsTest.php b/module/Rest/test-api/Action/OrphanVisitsTest.php index 06857653..067cf9a4 100644 --- a/module/Rest/test-api/Action/OrphanVisitsTest.php +++ b/module/Rest/test-api/Action/OrphanVisitsTest.php @@ -41,21 +41,32 @@ class OrphanVisitsTest extends ApiTestCase * @test * @dataProvider provideQueries */ - public function properVisitsAreReturnedBasedInQuery(array $query, int $expectedAmount, array $expectedVisits): void - { + public function properVisitsAreReturnedBasedInQuery( + array $query, + int $totalItems, + int $expectedAmount, + array $expectedVisits + ): void { $resp = $this->callApiWithKey(self::METHOD_GET, '/visits/orphan', [RequestOptions::QUERY => $query]); $payload = $this->getJsonResponsePayload($resp); $visits = $payload['visits']['data'] ?? []; - self::assertEquals(3, $payload['visits']['pagination']['totalItems'] ?? -1); + self::assertEquals($totalItems, $payload['visits']['pagination']['totalItems'] ?? -1); self::assertCount($expectedAmount, $visits); self::assertEquals($expectedVisits, $visits); } public function provideQueries(): iterable { - yield 'all data' => [[], 3, [self::INVALID_SHORT_URL, self::REGULAR_NOT_FOUND, self::BASE_URL]]; - yield 'limit items' => [['itemsPerPage' => 2], 2, [self::INVALID_SHORT_URL, self::REGULAR_NOT_FOUND]]; - yield 'limit items and page' => [['itemsPerPage' => 2, 'page' => 2], 1, [self::BASE_URL]]; + yield 'all data' => [[], 3, 3, [self::INVALID_SHORT_URL, self::REGULAR_NOT_FOUND, self::BASE_URL]]; + yield 'limit items' => [['itemsPerPage' => 2], 3, 2, [self::INVALID_SHORT_URL, self::REGULAR_NOT_FOUND]]; + yield 'limit items and page' => [['itemsPerPage' => 2, 'page' => 2], 3, 1, [self::BASE_URL]]; + yield 'exclude bots' => [['excludeBots' => true], 2, 2, [self::REGULAR_NOT_FOUND, self::BASE_URL]]; + yield 'exclude bots and limit items' => [ + ['excludeBots' => true, 'itemsPerPage' => 1], + 2, + 1, + [self::REGULAR_NOT_FOUND], + ]; } } diff --git a/module/Rest/test-api/Action/ShortUrlVisitsTest.php b/module/Rest/test-api/Action/ShortUrlVisitsTest.php index c578d48d..1d572004 100644 --- a/module/Rest/test-api/Action/ShortUrlVisitsTest.php +++ b/module/Rest/test-api/Action/ShortUrlVisitsTest.php @@ -67,4 +67,30 @@ class ShortUrlVisitsTest extends ApiTestCase yield 'domain' => ['example.com', 0]; yield 'no domain' => [null, 2]; } + + /** + * @test + * @dataProvider provideVisitsForBots + */ + public function properVisitsAreReturnedWhenExcludingBots(bool $excludeBots, int $expectedAmountOfVisits): void + { + $shortCode = 'def456'; + $url = new Uri(sprintf('/short-urls/%s/visits', $shortCode)); + + if ($excludeBots) { + $url = $url->withQuery(Query::build(['excludeBots' => true])); + } + + $resp = $this->callApiWithKey(self::METHOD_GET, (string) $url); + $payload = $this->getJsonResponsePayload($resp); + + self::assertEquals($expectedAmountOfVisits, $payload['visits']['pagination']['totalItems'] ?? -1); + self::assertCount($expectedAmountOfVisits, $payload['visits']['data'] ?? []); + } + + public function provideVisitsForBots(): iterable + { + yield 'bots excluded' => [true, 1]; + yield 'bots not excluded' => [false, 2]; + } } diff --git a/module/Rest/test-api/Action/TagVisitsTest.php b/module/Rest/test-api/Action/TagVisitsTest.php index b30b787f..07b0576d 100644 --- a/module/Rest/test-api/Action/TagVisitsTest.php +++ b/module/Rest/test-api/Action/TagVisitsTest.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace ShlinkioApiTest\Shlink\Rest\Action; +use GuzzleHttp\RequestOptions; use Shlinkio\Shlink\TestUtils\ApiTest\ApiTestCase; use function sprintf; @@ -14,9 +15,15 @@ class TagVisitsTest extends ApiTestCase * @test * @dataProvider provideTags */ - public function expectedVisitsAreReturned(string $apiKey, string $tag, int $expectedVisitsAmount): void - { - $resp = $this->callApiWithKey(self::METHOD_GET, sprintf('/tags/%s/visits', $tag), [], $apiKey); + public function expectedVisitsAreReturned( + string $apiKey, + string $tag, + bool $excludeBots, + int $expectedVisitsAmount + ): void { + $resp = $this->callApiWithKey(self::METHOD_GET, sprintf('/tags/%s/visits', $tag), [ + RequestOptions::QUERY => $excludeBots ? ['excludeBots' => true] : [], + ], $apiKey); $payload = $this->getJsonResponsePayload($resp); self::assertEquals(self::STATUS_OK, $resp->getStatusCode()); @@ -27,12 +34,16 @@ class TagVisitsTest extends ApiTestCase public function provideTags(): iterable { - yield 'foo with admin API key' => ['valid_api_key', 'foo', 5]; - yield 'bar with admin API key' => ['valid_api_key', 'bar', 2]; - yield 'baz with admin API key' => ['valid_api_key', 'baz', 0]; - yield 'foo with author API key' => ['author_api_key', 'foo', 5]; - yield 'bar with author API key' => ['author_api_key', 'bar', 2]; - yield 'foo with domain API key' => ['domain_api_key', 'foo', 0]; + yield 'foo with admin API key' => ['valid_api_key', 'foo', false, 5]; + yield 'foo with admin API key and no bots' => ['valid_api_key', 'foo', true, 4]; + yield 'bar with admin API key' => ['valid_api_key', 'bar', false, 2]; + yield 'bar with admin API key and no bots' => ['valid_api_key', 'bar', true, 1]; + yield 'baz with admin API key' => ['valid_api_key', 'baz', false, 0]; + yield 'foo with author API key' => ['author_api_key', 'foo', false, 5]; + yield 'foo with author API key and no bots' => ['author_api_key', 'foo', true, 4]; + yield 'bar with author API key' => ['author_api_key', 'bar', false, 2]; + yield 'bar with author API key and no bots' => ['author_api_key', 'bar', true, 1]; + yield 'foo with domain API key' => ['domain_api_key', 'foo', false, 0]; } /** diff --git a/module/Rest/test-api/Fixtures/VisitsFixture.php b/module/Rest/test-api/Fixtures/VisitsFixture.php index 62e1527d..4432df92 100644 --- a/module/Rest/test-api/Fixtures/VisitsFixture.php +++ b/module/Rest/test-api/Fixtures/VisitsFixture.php @@ -36,7 +36,7 @@ class VisitsFixture extends AbstractFixture implements DependentFixtureInterface /** @var ShortUrl $defShortUrl */ $defShortUrl = $this->getReference('def456_short_url'); $manager->persist( - Visit::forValidShortUrl($defShortUrl, new Visitor('shlink-tests-agent', '', '127.0.0.1', '')), + Visit::forValidShortUrl($defShortUrl, new Visitor('cf-facebook', '', '127.0.0.1', '')), ); $manager->persist( Visit::forValidShortUrl($defShortUrl, new Visitor('shlink-tests-agent', 'https://app.shlink.io', '', '')),