Merge pull request #1649 from acelaya-forks/feature/detailed-visits-stats

Feature/detailed visits stats

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.

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",

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"
         }
     }
 }

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": {

docs/swagger/paths/v2_visits.json

@@ -31,8 +31,16 @@
                         },
                         "example": {
                             "visits": {
-                                "visitsCount": 1569874,
+                                "nonOrphanVisits": {
-                                "orphanVisitsCount": 71345
+                                    "total": 64994,
+                                    "nonBots": 64986,
+                                    "bots": 8
+                                },
+                                "orphanVisits": {
+                                    "total": 37,
+                                    "nonBots": 34,
+                                    "bots": 3
+                                }
                             }
                         }
                     }

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,
-        ];
-    }
 }

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,
+            'nonOrphanVisits' => $this->nonOrphanVisitsSummary,
-            'orphanVisitsCount' => $this->orphanVisitsCount,
+            'orphanVisits' => $this->orphanVisitsSummary,
+
+            // Deprecated
+            'visitsCount' => $this->nonOrphanVisitsSummary->total,
+            'orphanVisitsCount' => $this->orphanVisitsSummary->total,
         ];
     }
 }

module/Core/src/Visit/Model/VisitsSummary.php

@@ -0,0 +1,28 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Shlinkio\Shlink\Core\Visit\Model;
+
+use JsonSerializable;
+
+final class VisitsSummary implements JsonSerializable
+{
+    private function __construct(public readonly int $total, public readonly int $nonBots)
+    {
+    }
+
+    public static function fromTotalAndNonBots(int $total, int $nonBots): self
+    {
+        return new self($total, $nonBots);
+    }
+
+    public function jsonSerialize(): array
+    {
+        return [
+            'total' => $this->total,
+            'nonBots' => $this->nonBots,
+            'bots' => $this->total - $this->nonBots,
+        ];
+    }
+}

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,
+            dateRange: $this->params->dateRange,
-            $this->params->excludeBots,
+            excludeBots: $this->params->excludeBots,
         ));
     }
 
     public function getSlice(int $offset, int $length): iterable
     {
         return $this->repo->findOrphanVisits(new VisitsListFiltering(
-            $this->params->dateRange,
+            dateRange: $this->params->dateRange,
-            $this->params->excludeBots,
+            excludeBots: $this->params->excludeBots,
-            null,
+            limit: $length,
-            $length,
+            offset: $offset,
-            $offset,
         ));
     }
 }

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);
     }
 }

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)),
+            nonOrphanVisitsTotal: $visitsRepo->countNonOrphanVisits(VisitsCountFiltering::withApiKey($apiKey)),
-            $visitsRepo->countOrphanVisits(new VisitsCountFiltering()),
+            orphanVisitsTotal: $visitsRepo->countOrphanVisits(new VisitsCountFiltering()),
+            nonOrphanVisitsNonBots: $visitsRepo->countNonOrphanVisits(
+                new VisitsCountFiltering(excludeBots: true, apiKey: $apiKey),
+            ),
+            orphanVisitsNonBots: $visitsRepo->countOrphanVisits(new VisitsCountFiltering(excludeBots: true)),
         );
     }
 

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' => [
+                'visitsSummary' => VisitsSummary::fromTotalAndNonBots(0, 0),
-                    'total' => 0,
-                    'nonBots' => 0,
-                    'bots' => 0,
-                ],
             ],
             'visit' => [
                 'referer' => '',
@@ -144,11 +141,7 @@ class PublishingUpdatesGeneratorTest extends TestCase
             'title' => $shortUrl->title(),
             'crawlable' => false,
             'forwardQuery' => true,
-            'visitsSummary' => [
+            'visitsSummary' => VisitsSummary::fromTotalAndNonBots(0, 0),
-                'total' => 0,
-                'nonBots' => 0,
-                'bots' => 0,
-            ],
         ]], $update->payload);
     }
 }

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(
+        $repo->expects($this->exactly(2))->method('countNonOrphanVisits')->withConsecutive(
-            $expectedCount * 3,
+            [new VisitsCountFiltering()],
-        );
+            [new VisitsCountFiltering(excludeBots: true)],
-        $repo->expects($this->once())->method('countOrphanVisits')->with(
+        )->willReturn($expectedCount * 3);
-            $this->isInstanceOf(VisitsCountFiltering::class),
+        $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);
 

module/Rest/test-api/Action/VisitStatsTest.php

@@ -0,0 +1,68 @@
+<?php
+
+declare(strict_types=1);
+
+namespace ShlinkioApiTest\Shlink\Rest\Action;
+
+use Shlinkio\Shlink\TestUtils\ApiTest\ApiTestCase;
+
+class VisitStatsTest extends ApiTestCase
+{
+    /**
+     * @test
+     * @dataProvider provideApiKeysAndResults
+     */
+    public function expectedStatsAreReturned(string $apiKey, array $expectedPayload): void
+    {
+        $resp = $this->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,
+        ]];
+    }
+}