diff --git a/CHANGELOG.md b/CHANGELOG.md index c3b9b2aa..58de5219 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,23 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com), and this project adheres to [Semantic Versioning](https://semver.org). +## [Unreleased] +### Added +* [#1632](https://github.com/shlinkio/shlink/issues/1632) Added amount of bots, non-bots and total visits to the visits summary endpoint. + +### Changed +* *Nothing* + +### Deprecated +* *Nothing* + +### Removed +* *Nothing* + +### Fixed +* *Nothing* + + ## [3.4.0] - 2022-12-16 ### Added * [#1612](https://github.com/shlinkio/shlink/issues/1612) Allowed to filter short URLs out of lists, when `validUntil` date is in the past or have reached their maximum amount of visits. diff --git a/docs/swagger/definitions/ShortUrl.json b/docs/swagger/definitions/ShortUrl.json index ab66f506..4d5d9f2d 100644 --- a/docs/swagger/definitions/ShortUrl.json +++ b/docs/swagger/definitions/ShortUrl.json @@ -38,7 +38,7 @@ "description": "**[DEPRECATED]** Use `visitsSummary.total` instead." }, "visitsSummary": { - "$ref": "./ShortUrlVisitsSummary.json" + "$ref": "./VisitsSummary.json" }, "tags": { "type": "array", diff --git a/docs/swagger/definitions/VisitStats.json b/docs/swagger/definitions/VisitStats.json index 2a97f597..2ed24375 100644 --- a/docs/swagger/definitions/VisitStats.json +++ b/docs/swagger/definitions/VisitStats.json @@ -1,14 +1,22 @@ { "type": "object", - "required": ["visitsCount", "orphanVisitsCount"], + "required": ["nonOrphanVisits", "orphanVisits", "visitsCount", "orphanVisitsCount"], "properties": { + "nonOrphanVisits": { + "$ref": "./VisitsSummary.json" + }, + "orphanVisits": { + "$ref": "./VisitsSummary.json" + }, "visitsCount": { + "deprecated": true, "type": "number", - "description": "The total amount of visits received on any short URL." + "description": "**[DEPRECATED]** Use nonOrphanVisits.total instead" }, "orphanVisitsCount": { + "deprecated": true, "type": "number", - "description": "The total amount of visits that could not be matched to a short URL (visits to the base URL, an invalid short URL or any other kind of 404)." + "description": "**[DEPRECATED]** Use orphanVisits.total instead" } } } diff --git a/docs/swagger/definitions/ShortUrlVisitsSummary.json b/docs/swagger/definitions/VisitsSummary.json similarity index 83% rename from docs/swagger/definitions/ShortUrlVisitsSummary.json rename to docs/swagger/definitions/VisitsSummary.json index 404b7a75..c59b2ccd 100644 --- a/docs/swagger/definitions/ShortUrlVisitsSummary.json +++ b/docs/swagger/definitions/VisitsSummary.json @@ -3,7 +3,7 @@ "required": ["total", "nonBots", "bots"], "properties": { "total": { - "description": "The total amount of visits that this short URL has received.", + "description": "The total amount of visits.", "type": "integer" }, "nonBots": { diff --git a/docs/swagger/paths/v2_visits.json b/docs/swagger/paths/v2_visits.json index ded6ac6b..3db0ef67 100644 --- a/docs/swagger/paths/v2_visits.json +++ b/docs/swagger/paths/v2_visits.json @@ -31,8 +31,16 @@ }, "example": { "visits": { - "visitsCount": 1569874, - "orphanVisitsCount": 71345 + "nonOrphanVisits": { + "total": 64994, + "nonBots": 64986, + "bots": 8 + }, + "orphanVisits": { + "total": 37, + "nonBots": 34, + "bots": 3 + } } } } diff --git a/module/Core/src/ShortUrl/Transformer/ShortUrlDataTransformer.php b/module/Core/src/ShortUrl/Transformer/ShortUrlDataTransformer.php index bd82cd9d..08327a98 100644 --- a/module/Core/src/ShortUrl/Transformer/ShortUrlDataTransformer.php +++ b/module/Core/src/ShortUrl/Transformer/ShortUrlDataTransformer.php @@ -7,6 +7,7 @@ namespace Shlinkio\Shlink\Core\ShortUrl\Transformer; use Shlinkio\Shlink\Common\Rest\DataTransformerInterface; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlStringifierInterface; +use Shlinkio\Shlink\Core\Visit\Model\VisitsSummary; use function Functional\invoke; use function Functional\invoke_if; @@ -33,7 +34,10 @@ class ShortUrlDataTransformer implements DataTransformerInterface 'title' => $shortUrl->title(), 'crawlable' => $shortUrl->crawlable(), 'forwardQuery' => $shortUrl->forwardQuery(), - 'visitsSummary' => $this->buildVisitsSummary($shortUrl), + 'visitsSummary' => VisitsSummary::fromTotalAndNonBots( + $shortUrl->getVisitsCount(), + $shortUrl->nonBotVisitsCount(), + ), // Deprecated 'visitsCount' => $shortUrl->getVisitsCount(), @@ -52,16 +56,4 @@ class ShortUrlDataTransformer implements DataTransformerInterface 'maxVisits' => $maxVisits, ]; } - - private function buildVisitsSummary(ShortUrl $shortUrl): array - { - $totalVisits = $shortUrl->getVisitsCount(); - $nonBotVisits = $shortUrl->nonBotVisitsCount(); - - return [ - 'total' => $totalVisits, - 'nonBots' => $nonBotVisits, - 'bots' => $totalVisits - $nonBotVisits, - ]; - } } diff --git a/module/Core/src/Visit/Model/VisitsStats.php b/module/Core/src/Visit/Model/VisitsStats.php index 475a25b5..adac34eb 100644 --- a/module/Core/src/Visit/Model/VisitsStats.php +++ b/module/Core/src/Visit/Model/VisitsStats.php @@ -8,15 +8,34 @@ use JsonSerializable; final class VisitsStats implements JsonSerializable { - public function __construct(private int $visitsCount, private int $orphanVisitsCount) - { + private readonly VisitsSummary $nonOrphanVisitsSummary; + private readonly VisitsSummary $orphanVisitsSummary; + + public function __construct( + int $nonOrphanVisitsTotal, + int $orphanVisitsTotal, + ?int $nonOrphanVisitsNonBots = null, + ?int $orphanVisitsNonBots = null, + ) { + $this->nonOrphanVisitsSummary = VisitsSummary::fromTotalAndNonBots( + $nonOrphanVisitsTotal, + $nonOrphanVisitsNonBots ?? $nonOrphanVisitsTotal, + ); + $this->orphanVisitsSummary = VisitsSummary::fromTotalAndNonBots( + $orphanVisitsTotal, + $orphanVisitsNonBots ?? $orphanVisitsTotal, + ); } public function jsonSerialize(): array { return [ - 'visitsCount' => $this->visitsCount, - 'orphanVisitsCount' => $this->orphanVisitsCount, + 'nonOrphanVisits' => $this->nonOrphanVisitsSummary, + 'orphanVisits' => $this->orphanVisitsSummary, + + // Deprecated + 'visitsCount' => $this->nonOrphanVisitsSummary->total, + 'orphanVisitsCount' => $this->orphanVisitsSummary->total, ]; } } diff --git a/module/Core/src/Visit/Model/VisitsSummary.php b/module/Core/src/Visit/Model/VisitsSummary.php new file mode 100644 index 00000000..654170cb --- /dev/null +++ b/module/Core/src/Visit/Model/VisitsSummary.php @@ -0,0 +1,28 @@ + $this->total, + 'nonBots' => $this->nonBots, + 'bots' => $this->total - $this->nonBots, + ]; + } +} diff --git a/module/Core/src/Visit/Paginator/Adapter/OrphanVisitsPaginatorAdapter.php b/module/Core/src/Visit/Paginator/Adapter/OrphanVisitsPaginatorAdapter.php index c181665e..4e6e4daf 100644 --- a/module/Core/src/Visit/Paginator/Adapter/OrphanVisitsPaginatorAdapter.php +++ b/module/Core/src/Visit/Paginator/Adapter/OrphanVisitsPaginatorAdapter.php @@ -12,26 +12,25 @@ use Shlinkio\Shlink\Core\Visit\Repository\VisitRepositoryInterface; class OrphanVisitsPaginatorAdapter extends AbstractCacheableCountPaginatorAdapter { - public function __construct(private VisitRepositoryInterface $repo, private VisitsParams $params) + public function __construct(private readonly VisitRepositoryInterface $repo, private readonly VisitsParams $params) { } protected function doCount(): int { return $this->repo->countOrphanVisits(new VisitsCountFiltering( - $this->params->dateRange, - $this->params->excludeBots, + dateRange: $this->params->dateRange, + excludeBots: $this->params->excludeBots, )); } public function getSlice(int $offset, int $length): iterable { return $this->repo->findOrphanVisits(new VisitsListFiltering( - $this->params->dateRange, - $this->params->excludeBots, - null, - $length, - $offset, + dateRange: $this->params->dateRange, + excludeBots: $this->params->excludeBots, + limit: $length, + offset: $offset, )); } } diff --git a/module/Core/src/Visit/Persistence/VisitsCountFiltering.php b/module/Core/src/Visit/Persistence/VisitsCountFiltering.php index f839a945..c445200e 100644 --- a/module/Core/src/Visit/Persistence/VisitsCountFiltering.php +++ b/module/Core/src/Visit/Persistence/VisitsCountFiltering.php @@ -18,6 +18,6 @@ class VisitsCountFiltering public static function withApiKey(?ApiKey $apiKey): self { - return new self(null, false, $apiKey); + return new self(apiKey: $apiKey); } } diff --git a/module/Core/src/Visit/VisitsStatsHelper.php b/module/Core/src/Visit/VisitsStatsHelper.php index dcba7030..25f44921 100644 --- a/module/Core/src/Visit/VisitsStatsHelper.php +++ b/module/Core/src/Visit/VisitsStatsHelper.php @@ -32,7 +32,7 @@ use Shlinkio\Shlink\Rest\Entity\ApiKey; class VisitsStatsHelper implements VisitsStatsHelperInterface { - public function __construct(private EntityManagerInterface $em) + public function __construct(private readonly EntityManagerInterface $em) { } @@ -42,8 +42,12 @@ class VisitsStatsHelper implements VisitsStatsHelperInterface $visitsRepo = $this->em->getRepository(Visit::class); return new VisitsStats( - $visitsRepo->countNonOrphanVisits(VisitsCountFiltering::withApiKey($apiKey)), - $visitsRepo->countOrphanVisits(new VisitsCountFiltering()), + nonOrphanVisitsTotal: $visitsRepo->countNonOrphanVisits(VisitsCountFiltering::withApiKey($apiKey)), + orphanVisitsTotal: $visitsRepo->countOrphanVisits(new VisitsCountFiltering()), + nonOrphanVisitsNonBots: $visitsRepo->countNonOrphanVisits( + new VisitsCountFiltering(excludeBots: true, apiKey: $apiKey), + ), + orphanVisitsNonBots: $visitsRepo->countOrphanVisits(new VisitsCountFiltering(excludeBots: true)), ); } diff --git a/module/Core/test/EventDispatcher/PublishingUpdatesGeneratorTest.php b/module/Core/test/EventDispatcher/PublishingUpdatesGeneratorTest.php index c7a4ecd0..cda8fe98 100644 --- a/module/Core/test/EventDispatcher/PublishingUpdatesGeneratorTest.php +++ b/module/Core/test/EventDispatcher/PublishingUpdatesGeneratorTest.php @@ -14,6 +14,7 @@ use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlCreation; use Shlinkio\Shlink\Core\ShortUrl\Transformer\ShortUrlDataTransformer; use Shlinkio\Shlink\Core\Visit\Entity\Visit; use Shlinkio\Shlink\Core\Visit\Model\Visitor; +use Shlinkio\Shlink\Core\Visit\Model\VisitsSummary; use Shlinkio\Shlink\Core\Visit\Model\VisitType; use Shlinkio\Shlink\Core\Visit\Transformer\OrphanVisitDataTransformer; @@ -63,11 +64,7 @@ class PublishingUpdatesGeneratorTest extends TestCase 'title' => $title, 'crawlable' => false, 'forwardQuery' => true, - 'visitsSummary' => [ - 'total' => 0, - 'nonBots' => 0, - 'bots' => 0, - ], + 'visitsSummary' => VisitsSummary::fromTotalAndNonBots(0, 0), ], 'visit' => [ 'referer' => '', @@ -144,11 +141,7 @@ class PublishingUpdatesGeneratorTest extends TestCase 'title' => $shortUrl->title(), 'crawlable' => false, 'forwardQuery' => true, - 'visitsSummary' => [ - 'total' => 0, - 'nonBots' => 0, - 'bots' => 0, - ], + 'visitsSummary' => VisitsSummary::fromTotalAndNonBots(0, 0), ]], $update->payload); } } diff --git a/module/Core/test/Visit/VisitsStatsHelperTest.php b/module/Core/test/Visit/VisitsStatsHelperTest.php index 8afd56db..1774ba6a 100644 --- a/module/Core/test/Visit/VisitsStatsHelperTest.php +++ b/module/Core/test/Visit/VisitsStatsHelperTest.php @@ -53,11 +53,13 @@ class VisitsStatsHelperTest extends TestCase public function returnsExpectedVisitsStats(int $expectedCount): void { $repo = $this->createMock(VisitRepository::class); - $repo->expects($this->once())->method('countNonOrphanVisits')->with(new VisitsCountFiltering())->willReturn( - $expectedCount * 3, - ); - $repo->expects($this->once())->method('countOrphanVisits')->with( - $this->isInstanceOf(VisitsCountFiltering::class), + $repo->expects($this->exactly(2))->method('countNonOrphanVisits')->withConsecutive( + [new VisitsCountFiltering()], + [new VisitsCountFiltering(excludeBots: true)], + )->willReturn($expectedCount * 3); + $repo->expects($this->exactly(2))->method('countOrphanVisits')->withConsecutive( + [$this->isInstanceOf(VisitsCountFiltering::class)], + [$this->isInstanceOf(VisitsCountFiltering::class)], )->willReturn($expectedCount); $this->em->expects($this->once())->method('getRepository')->with(Visit::class)->willReturn($repo); diff --git a/module/Rest/test-api/Action/VisitStatsTest.php b/module/Rest/test-api/Action/VisitStatsTest.php new file mode 100644 index 00000000..424eba73 --- /dev/null +++ b/module/Rest/test-api/Action/VisitStatsTest.php @@ -0,0 +1,68 @@ +callApiWithKey(self::METHOD_GET, '/visits', apiKey: $apiKey); + $payload = $this->getJsonResponsePayload($resp); + + self::assertEquals(['visits' => $expectedPayload], $payload); + } + + public function provideApiKeysAndResults(): iterable + { + yield 'valid API key' => ['valid_api_key', [ + 'nonOrphanVisits' => [ + 'total' => 7, + 'nonBots' => 6, + 'bots' => 1, + ], + 'orphanVisits' => [ + 'total' => 3, + 'nonBots' => 2, + 'bots' => 1, + ], + 'visitsCount' => 7, + 'orphanVisitsCount' => 3, + ]]; + yield 'domain-only API key' => ['domain_api_key', [ + 'nonOrphanVisits' => [ + 'total' => 0, + 'nonBots' => 0, + 'bots' => 0, + ], + 'orphanVisits' => [ + 'total' => 3, + 'nonBots' => 2, + 'bots' => 1, + ], + 'visitsCount' => 0, + 'orphanVisitsCount' => 3, + ]]; + yield 'author API key' => ['author_api_key', [ + 'nonOrphanVisits' => [ + 'total' => 5, + 'nonBots' => 4, + 'bots' => 1, + ], + 'orphanVisits' => [ + 'total' => 3, + 'nonBots' => 2, + 'bots' => 1, + ], + 'visitsCount' => 5, + 'orphanVisitsCount' => 3, + ]]; + } +}