diff --git a/.gitignore b/.gitignore index 8cfea409..03b2790e 100644 --- a/.gitignore +++ b/.gitignore @@ -9,5 +9,6 @@ data/shlink-tests.db data/GeoLite2-City.mmdb data/GeoLite2-City.mmdb.* docs/swagger-ui* +docs/mercure.html docker-compose.override.yml .phpunit.result.cache diff --git a/CHANGELOG.md b/CHANGELOG.md index e11c74cc..9f00b680 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,14 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this The file requires the `Long URL` and `Short code` columns, and it also accepts the optional `title`, `domain` and `tags` columns. * [#1000](https://github.com/shlinkio/shlink/issues/1000) Added support to provide a `margin` query param when generating some URL's QR code. +* [#675](https://github.com/shlinkio/shlink/issues/1000) Added ability to track visits to the base URL, invalid short URLs or any other "not found" URL, as known as orphan visits. + + This behavior is enabled by default, but you can opt out via env vars or config options. + + This new orphan visits can be consumed in these ways: + + * The `https://shlink.io/new-orphan-visit` mercure topic, which gets notified when an orphan visit occurs. + * The `GET /visits/orphan` REST endpoint, which behaves like the short URL visits and tags visits endpoints, but returns only orphan visits. ### Changed * [#977](https://github.com/shlinkio/shlink/issues/977) Migrated from `laminas/laminas-paginator` to `pagerfanta/core` to handle pagination. diff --git a/composer.json b/composer.json index 29068f06..f4eedb4a 100644 --- a/composer.json +++ b/composer.json @@ -47,11 +47,11 @@ "predis/predis": "^1.1", "pugx/shortid-php": "^0.7", "ramsey/uuid": "^3.9", - "shlinkio/shlink-common": "dev-main#b889f5d as 3.5", + "shlinkio/shlink-common": "dev-main#62d4b84 as 3.5", "shlinkio/shlink-config": "^1.0", "shlinkio/shlink-event-dispatcher": "^2.0", "shlinkio/shlink-importer": "^2.2", - "shlinkio/shlink-installer": "dev-develop#1ed5ac8 as 5.4", + "shlinkio/shlink-installer": "dev-develop#c489d3f as 5.4", "shlinkio/shlink-ip-geolocation": "^1.5", "symfony/console": "^5.1", "symfony/filesystem": "^5.1", diff --git a/config/autoload/installer.global.php b/config/autoload/installer.global.php index 7a355dbe..d18f31f4 100644 --- a/config/autoload/installer.global.php +++ b/config/autoload/installer.global.php @@ -41,6 +41,7 @@ return [ Option\UrlShortener\RedirectStatusCodeConfigOption::class, Option\UrlShortener\RedirectCacheLifeTimeConfigOption::class, Option\UrlShortener\AutoResolveTitlesConfigOption::class, + Option\UrlShortener\OrphanVisitsTrackingConfigOption::class, ], 'installation_commands' => [ diff --git a/config/autoload/middleware-pipeline.global.php b/config/autoload/middleware-pipeline.global.php index 2eb009cb..c879205f 100644 --- a/config/autoload/middleware-pipeline.global.php +++ b/config/autoload/middleware-pipeline.global.php @@ -9,6 +9,7 @@ use Mezzio\Helper; use Mezzio\ProblemDetails; use Mezzio\Router; use PhpMiddleware\RequestId\RequestIdMiddleware; +use RKA\Middleware\IpAddress; use function extension_loaded; @@ -68,6 +69,10 @@ return [ ], 'not-found' => [ 'middleware' => [ + // This middleware is in front of tracking actions explicitly. Putting here for orphan visits tracking + IpAddress::class, + Core\ErrorHandler\NotFoundTypeResolverMiddleware::class, + Core\ErrorHandler\NotFoundTrackerMiddleware::class, Core\ErrorHandler\NotFoundRedirectHandler::class, Core\ErrorHandler\NotFoundTemplateHandler::class, ], diff --git a/config/autoload/url-shortener.global.php b/config/autoload/url-shortener.global.php index 015d459e..3751b1e9 100644 --- a/config/autoload/url-shortener.global.php +++ b/config/autoload/url-shortener.global.php @@ -13,13 +13,14 @@ return [ 'schema' => 'https', 'hostname' => '', ], - 'validate_url' => false, + 'validate_url' => false, // Deprecated 'anonymize_remote_addr' => true, 'visits_webhooks' => [], 'default_short_codes_length' => DEFAULT_SHORT_CODES_LENGTH, 'redirect_status_code' => DEFAULT_REDIRECT_STATUS_CODE, 'redirect_cache_lifetime' => DEFAULT_REDIRECT_CACHE_LIFETIME, 'auto_resolve_titles' => false, + 'track_orphan_visits' => true, ], ]; diff --git a/data/migrations/Version20210207100807.php b/data/migrations/Version20210207100807.php new file mode 100644 index 00000000..a74c0b08 --- /dev/null +++ b/data/migrations/Version20210207100807.php @@ -0,0 +1,53 @@ +getTable('visits'); + $shortUrlId = $visits->getColumn('short_url_id'); + + $this->skipIf(! $shortUrlId->getNotnull()); + + $shortUrlId->setNotnull(false); + + $visits->addColumn('visited_url', Types::STRING, [ + 'length' => Visitor::VISITED_URL_MAX_LENGTH, + 'notnull' => false, + ]); + $visits->addColumn('type', Types::STRING, [ + 'length' => 255, + 'default' => Visit::TYPE_VALID_SHORT_URL, + ]); + } + + public function down(Schema $schema): void + { + $visits = $schema->getTable('visits'); + $shortUrlId = $visits->getColumn('short_url_id'); + + $this->skipIf($shortUrlId->getNotnull()); + + $shortUrlId->setNotnull(true); + $visits->dropColumn('visited_url'); + $visits->dropColumn('type'); + } + + /** + * @fixme Workaround for https://github.com/doctrine/migrations/issues/1104 + */ + public function isTransactional(): bool + { + return false; + } +} diff --git a/docker/config/shlink_in_docker.local.php b/docker/config/shlink_in_docker.local.php index 40173d69..4ddd52e5 100644 --- a/docker/config/shlink_in_docker.local.php +++ b/docker/config/shlink_in_docker.local.php @@ -126,6 +126,7 @@ return [ 'redirect_status_code' => (int) env('REDIRECT_STATUS_CODE', DEFAULT_REDIRECT_STATUS_CODE), 'redirect_cache_lifetime' => (int) env('REDIRECT_CACHE_LIFETIME', DEFAULT_REDIRECT_CACHE_LIFETIME), 'auto_resolve_titles' => (bool) env('AUTO_RESOLVE_TITLES', false), + 'track_orphan_visits' => (bool) env('TRACK_ORPHAN_VISITS', true), ], 'not_found_redirects' => $helper->getNotFoundRedirectsConfig(), diff --git a/docs/adr/2021-02-07-track-visits-to-base-url-invalid-short-url-and-regular-404.md b/docs/adr/2021-02-07-track-visits-to-base-url-invalid-short-url-and-regular-404.md new file mode 100644 index 00000000..983410d1 --- /dev/null +++ b/docs/adr/2021-02-07-track-visits-to-base-url-invalid-short-url-and-regular-404.md @@ -0,0 +1,35 @@ +# Track visits to 'base_url', 'invalid_short_url' and 'regular_404' + +* Status: Accepted +* Date: 2021-02-07 + +## Context and problem statement + +Shlink has the mechanism to return either custom errors or custom redirects when visiting the instance's base URL, an invalid short URL, or any other kind of URL that would result in a "Not found" error. + +However, it does not track visits to any of those, just to valid short URLs. + +The intention is to change that, and allow users to track the cases mentioned above. + +## Considered option + +* Create a new table to track visits o this kind. +* Reuse the existing `visits` table, by making `short_url_id` nullable and adding a couple of other fields. + +## Decision outcome + +The decision is to use the existing table, as making the short URL nullable can be handled seamlessly by using named constructors, and it has a lot of benefits on regards of reusing existing components. + +Also, the domain name this kind of visits will receive is "Orphan Visits", as they are detached from any existing short URL. + +## Pros and Cons of the Options + +### New table + +* Good because we don't touch existing models and tables, reducing the risk to introduce a backwards compatibility break. +* Bad because we will have to repeat data modeling and logic, or refactor some components to support both contexts. This in turn increases the options to introduce a BC break. + +### Reuse existing table + +* Good because all the mechanisms in place to handle visits will work out of the box, including locating visits and such. +* Bad because we will have more optional properties, which means more double checks in many places. diff --git a/docs/adr/README.md b/docs/adr/README.md index 56df328f..93d82cff 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -2,4 +2,5 @@ Here listed you will find the different architectural decisions taken in the project, including all the reasoning behind it, options considered, and final outcome. +* [2021-02-07 Track visits to 'base_url', 'invalid_short_url' and 'regular_404'](2021-02-07-track-visits-to-base-url-invalid-short-url-and-regular-404.md) * [2021-01-17 Support restrictions and permissions in API keys](2021-01-17-support-restrictions-and-permissions-in-api-keys.md) diff --git a/docs/async-api/async-api.json b/docs/async-api/async-api.json index 5279ce91..df9bc6d6 100644 --- a/docs/async-api/async-api.json +++ b/docs/async-api/async-api.json @@ -58,6 +58,23 @@ } } } + }, + "http://shlink.io/new-orphan-visit": { + "subscribe": { + "summary": "Receive information about any new orphan visit.", + "operationId": "newOrphanVisit", + "message": { + "payload": { + "type": "object", + "additionalProperties": false, + "properties": { + "visit": { + "$ref": "#/components/schemas/OrphanVisit" + } + } + } + } + } } }, "components": { @@ -179,6 +196,46 @@ } } }, + "OrphanVisit": { + "allOf": [ + {"$ref": "#/components/schemas/Visit"}, + { + "type": "object", + "properties": { + "visitedUrl": { + "type": "string", + "nullable": true, + "description": "The originally visited URL that triggered the tracking of this visit" + }, + "type": { + "type": "string", + "enum": [ + "invalid_short_url", + "base_url", + "regular_404" + ], + "description": "Tells the type of orphan visit" + } + } + } + ], + "example": { + "referer": "https://t.co", + "date": "2015-08-20T05:05:03+04:00", + "userAgent": "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/51.0.2704.103 Safari/537.36", + "visitLocation": { + "cityName": "Cupertino", + "countryCode": "US", + "countryName": "United States", + "latitude": 37.3042, + "longitude": -122.0946, + "regionName": "California", + "timezone": "America/Los_Angeles" + }, + "visitedUrl": "https://doma.in", + "type": "base_url" + } + }, "VisitLocation": { "type": "object", "properties": { diff --git a/docs/swagger/definitions/OrphanVisit.json b/docs/swagger/definitions/OrphanVisit.json new file mode 100644 index 00000000..04d8386d --- /dev/null +++ b/docs/swagger/definitions/OrphanVisit.json @@ -0,0 +1,23 @@ +{ + "type": "object", + "required": ["visitedUrl", "type"], + "allOf": [{ + "$ref": "./Visit.json" + }], + "properties": { + "visitedUrl": { + "type": "string", + "nullable": true, + "description": "The originally visited URL that triggered the tracking of this visit" + }, + "type": { + "type": "string", + "enum": [ + "invalid_short_url", + "base_url", + "regular_404" + ], + "description": "Tells the type of orphan visit" + } + } +} diff --git a/docs/swagger/definitions/Visit.json b/docs/swagger/definitions/Visit.json index 9e1eb5b5..e004e4fe 100644 --- a/docs/swagger/definitions/Visit.json +++ b/docs/swagger/definitions/Visit.json @@ -1,5 +1,6 @@ { "type": "object", + "required": ["referer", "date", "userAgent", "visitLocation"], "properties": { "referer": { "type": "string", diff --git a/docs/swagger/definitions/VisitStats.json b/docs/swagger/definitions/VisitStats.json index 5f439c9b..2a97f597 100644 --- a/docs/swagger/definitions/VisitStats.json +++ b/docs/swagger/definitions/VisitStats.json @@ -1,10 +1,14 @@ { "type": "object", - "required": ["visitsCount"], + "required": ["visitsCount", "orphanVisitsCount"], "properties": { "visitsCount": { "type": "number", - "description": "The total amount of visits received." + "description": "The total amount of visits received on any short URL." + }, + "orphanVisitsCount": { + "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)." } } } diff --git a/docs/swagger/paths/v2_visits.json b/docs/swagger/paths/v2_visits.json index 089223b3..3c712b1f 100644 --- a/docs/swagger/paths/v2_visits.json +++ b/docs/swagger/paths/v2_visits.json @@ -34,7 +34,8 @@ "examples": { "application/json": { "visits": { - "visitsCount": 1569874 + "visitsCount": 1569874, + "orphanVisitsCount": 71345 } } } diff --git a/docs/swagger/paths/v2_visits_orphan.json b/docs/swagger/paths/v2_visits_orphan.json new file mode 100644 index 00000000..683f40ec --- /dev/null +++ b/docs/swagger/paths/v2_visits_orphan.json @@ -0,0 +1,141 @@ +{ + "get": { + "operationId": "getOrphanVisits", + "tags": [ + "Visits" + ], + "summary": "List orphan visits", + "description": "Get the list of visits to invalid short URLs, the base URL or any other 404.", + "parameters": [ + { + "$ref": "../parameters/version.json" + }, + { + "name": "startDate", + "in": "query", + "description": "The date (in ISO-8601 format) from which we want to get visits.", + "required": false, + "schema": { + "type": "string" + } + }, + { + "name": "endDate", + "in": "query", + "description": "The date (in ISO-8601 format) until which we want to get visits.", + "required": false, + "schema": { + "type": "string" + } + }, + { + "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" + } + } + ], + "security": [ + { + "ApiKey": [] + } + ], + "responses": { + "200": { + "description": "List of visits.", + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": { + "visits": { + "type": "object", + "properties": { + "data": { + "type": "array", + "items": { + "$ref": "../definitions/OrphanVisit.json" + } + }, + "pagination": { + "$ref": "../definitions/Pagination.json" + } + } + } + } + } + } + }, + "examples": { + "application/json": { + "visits": { + "data": [ + { + "referer": "https://twitter.com", + "date": "2015-08-20T05:05:03+04:00", + "userAgent": "Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:47.0) Gecko/20100101 Firefox/47.0 Mozilla/5.0 (Macintosh; Intel Mac OS X x.y; rv:42.0) Gecko/20100101 Firefox/42.0", + "visitLocation": null, + "visitedUrl": "https://doma.in", + "type": "base_url" + }, + { + "referer": "https://t.co", + "date": "2015-08-20T05:05:03+04:00", + "userAgent": "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/51.0.2704.103 Safari/537.36", + "visitLocation": { + "cityName": "Cupertino", + "countryCode": "US", + "countryName": "United States", + "latitude": 37.3042, + "longitude": -122.0946, + "regionName": "California", + "timezone": "America/Los_Angeles" + }, + "visitedUrl": "https://doma.in/foo", + "type": "invalid_short_url" + }, + { + "referer": null, + "date": "2015-08-20T05:05:03+04:00", + "userAgent": "some_web_crawler/1.4", + "visitLocation": null, + "visitedUrl": "https://doma.in/foo/bar/baz", + "type": "regular_404" + } + ], + "pagination": { + "currentPage": 5, + "pagesCount": 12, + "itemsPerPage": 10, + "itemsInCurrentPage": 10, + "totalItems": 115 + } + } + } + } + }, + "500": { + "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 dc834905..21547f90 100644 --- a/docs/swagger/swagger.json +++ b/docs/swagger/swagger.json @@ -95,6 +95,9 @@ "/rest/v{version}/tags/{tag}/visits": { "$ref": "paths/v2_tags_{tag}_visits.json" }, + "/rest/v{version}/visits/orphan": { + "$ref": "paths/v2_visits_orphan.json" + }, "/rest/v{version}/domains": { "$ref": "paths/v2_domains.json" diff --git a/module/CLI/config/dependencies.config.php b/module/CLI/config/dependencies.config.php index 685dc9fd..7e224c33 100644 --- a/module/CLI/config/dependencies.config.php +++ b/module/CLI/config/dependencies.config.php @@ -74,7 +74,7 @@ return [ Service\ShortUrlService::class, ShortUrlDataTransformer::class, ], - Command\ShortUrl\GetVisitsCommand::class => [Service\VisitsTracker::class], + Command\ShortUrl\GetVisitsCommand::class => [Visit\VisitsStatsHelper::class], Command\ShortUrl\DeleteShortUrlCommand::class => [Service\ShortUrl\DeleteShortUrlService::class], Command\Visit\LocateVisitsCommand::class => [ diff --git a/module/CLI/src/Command/ShortUrl/GetVisitsCommand.php b/module/CLI/src/Command/ShortUrl/GetVisitsCommand.php index 0b7de663..7b020356 100644 --- a/module/CLI/src/Command/ShortUrl/GetVisitsCommand.php +++ b/module/CLI/src/Command/ShortUrl/GetVisitsCommand.php @@ -11,8 +11,8 @@ use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Entity\Visit; use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Model\VisitsParams; -use Shlinkio\Shlink\Core\Service\VisitsTrackerInterface; use Shlinkio\Shlink\Core\Visit\Model\UnknownVisitLocation; +use Shlinkio\Shlink\Core\Visit\VisitsStatsHelperInterface; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\InputOption; @@ -27,11 +27,11 @@ class GetVisitsCommand extends AbstractWithDateRangeCommand { public const NAME = 'short-url:visits'; - private VisitsTrackerInterface $visitsTracker; + private VisitsStatsHelperInterface $visitsHelper; - public function __construct(VisitsTrackerInterface $visitsTracker) + public function __construct(VisitsStatsHelperInterface $visitsHelper) { - $this->visitsTracker = $visitsTracker; + $this->visitsHelper = $visitsHelper; parent::__construct(); } @@ -74,7 +74,10 @@ class GetVisitsCommand extends AbstractWithDateRangeCommand $startDate = $this->getStartDateOption($input, $output); $endDate = $this->getEndDateOption($input, $output); - $paginator = $this->visitsTracker->info($identifier, new VisitsParams(new DateRange($startDate, $endDate))); + $paginator = $this->visitsHelper->visitsForShortUrl( + $identifier, + new VisitsParams(new DateRange($startDate, $endDate)), + ); $rows = map($paginator->getCurrentPageResults(), function (Visit $visit) { $rowData = $visit->jsonSerialize(); diff --git a/module/CLI/test/Command/ShortUrl/GetVisitsCommandTest.php b/module/CLI/test/Command/ShortUrl/GetVisitsCommandTest.php index 3da492e3..d25d5763 100644 --- a/module/CLI/test/Command/ShortUrl/GetVisitsCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/GetVisitsCommandTest.php @@ -19,7 +19,7 @@ use Shlinkio\Shlink\Core\Entity\VisitLocation; use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Model\Visitor; use Shlinkio\Shlink\Core\Model\VisitsParams; -use Shlinkio\Shlink\Core\Service\VisitsTrackerInterface; +use Shlinkio\Shlink\Core\Visit\VisitsStatsHelperInterface; use Shlinkio\Shlink\IpGeolocation\Model\Location; use Symfony\Component\Console\Application; use Symfony\Component\Console\Tester\CommandTester; @@ -31,12 +31,12 @@ class GetVisitsCommandTest extends TestCase use ProphecyTrait; private CommandTester $commandTester; - private ObjectProphecy $visitsTracker; + private ObjectProphecy $visitsHelper; public function setUp(): void { - $this->visitsTracker = $this->prophesize(VisitsTrackerInterface::class); - $command = new GetVisitsCommand($this->visitsTracker->reveal()); + $this->visitsHelper = $this->prophesize(VisitsStatsHelperInterface::class); + $command = new GetVisitsCommand($this->visitsHelper->reveal()); $app = new Application(); $app->add($command); $this->commandTester = new CommandTester($command); @@ -46,7 +46,7 @@ class GetVisitsCommandTest extends TestCase public function noDateFlagsTriesToListWithoutDateRange(): void { $shortCode = 'abc123'; - $this->visitsTracker->info( + $this->visitsHelper->visitsForShortUrl( new ShortUrlIdentifier($shortCode), new VisitsParams(new DateRange(null, null)), ) @@ -62,7 +62,7 @@ class GetVisitsCommandTest extends TestCase $shortCode = 'abc123'; $startDate = '2016-01-01'; $endDate = '2016-02-01'; - $this->visitsTracker->info( + $this->visitsHelper->visitsForShortUrl( new ShortUrlIdentifier($shortCode), new VisitsParams(new DateRange(Chronos::parse($startDate), Chronos::parse($endDate))), ) @@ -81,8 +81,10 @@ class GetVisitsCommandTest extends TestCase { $shortCode = 'abc123'; $startDate = 'foo'; - $info = $this->visitsTracker->info(new ShortUrlIdentifier($shortCode), new VisitsParams(new DateRange())) - ->willReturn(new Paginator(new ArrayAdapter([]))); + $info = $this->visitsHelper->visitsForShortUrl( + new ShortUrlIdentifier($shortCode), + new VisitsParams(new DateRange()), + )->willReturn(new Paginator(new ArrayAdapter([]))); $this->commandTester->execute([ 'shortCode' => $shortCode, @@ -101,9 +103,9 @@ class GetVisitsCommandTest extends TestCase public function outputIsProperlyGenerated(): void { $shortCode = 'abc123'; - $this->visitsTracker->info(new ShortUrlIdentifier($shortCode), Argument::any())->willReturn( + $this->visitsHelper->visitsForShortUrl(new ShortUrlIdentifier($shortCode), Argument::any())->willReturn( new Paginator(new ArrayAdapter([ - (new Visit(ShortUrl::createEmpty(), new Visitor('bar', 'foo', '')))->locate( + Visit::forValidShortUrl(ShortUrl::createEmpty(), new Visitor('bar', 'foo', '', ''))->locate( new VisitLocation(new Location('', 'Spain', '', '', 0, 0, '')), ), ])), diff --git a/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php b/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php index 5ba0778a..236fac50 100644 --- a/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php +++ b/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php @@ -77,7 +77,7 @@ class LocateVisitsCommandTest extends TestCase bool $expectWarningPrint, array $args ): void { - $visit = new Visit(ShortUrl::createEmpty(), new Visitor('', '', '1.2.3.4')); + $visit = Visit::forValidShortUrl(ShortUrl::createEmpty(), new Visitor('', '', '1.2.3.4', '')); $location = new VisitLocation(Location::emptyInstance()); $mockMethodBehavior = $this->invokeHelperMethods($visit, $location); @@ -121,7 +121,7 @@ class LocateVisitsCommandTest extends TestCase */ public function localhostAndEmptyAddressesAreIgnored(?string $address, string $message): void { - $visit = new Visit(ShortUrl::createEmpty(), new Visitor('', '', $address)); + $visit = Visit::forValidShortUrl(ShortUrl::createEmpty(), new Visitor('', '', $address, '')); $location = new VisitLocation(Location::emptyInstance()); $locateVisits = $this->visitService->locateUnlocatedVisits(Argument::cetera())->will( @@ -154,7 +154,7 @@ class LocateVisitsCommandTest extends TestCase /** @test */ public function errorWhileLocatingIpIsDisplayed(): void { - $visit = new Visit(ShortUrl::createEmpty(), new Visitor('', '', '1.2.3.4')); + $visit = Visit::forValidShortUrl(ShortUrl::createEmpty(), new Visitor('', '', '1.2.3.4', '')); $location = new VisitLocation(Location::emptyInstance()); $locateVisits = $this->visitService->locateUnlocatedVisits(Argument::cetera())->will( diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index e742ad43..479b497a 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -15,6 +15,8 @@ return [ 'dependencies' => [ 'factories' => [ + ErrorHandler\NotFoundTypeResolverMiddleware::class => ConfigAbstractFactory::class, + ErrorHandler\NotFoundTrackerMiddleware::class => ConfigAbstractFactory::class, ErrorHandler\NotFoundRedirectHandler::class => ConfigAbstractFactory::class, ErrorHandler\NotFoundTemplateHandler::class => InvokableFactory::class, @@ -24,16 +26,20 @@ return [ Options\UrlShortenerOptions::class => ConfigAbstractFactory::class, Service\UrlShortener::class => ConfigAbstractFactory::class, - Service\VisitsTracker::class => ConfigAbstractFactory::class, Service\ShortUrlService::class => ConfigAbstractFactory::class, - Visit\VisitLocator::class => ConfigAbstractFactory::class, - Visit\VisitsStatsHelper::class => ConfigAbstractFactory::class, - Tag\TagService::class => ConfigAbstractFactory::class, Service\ShortUrl\DeleteShortUrlService::class => ConfigAbstractFactory::class, Service\ShortUrl\ShortUrlResolver::class => ConfigAbstractFactory::class, Service\ShortUrl\ShortCodeHelper::class => ConfigAbstractFactory::class, + + Tag\TagService::class => ConfigAbstractFactory::class, + Domain\DomainService::class => ConfigAbstractFactory::class, + Visit\VisitsTracker::class => ConfigAbstractFactory::class, + Visit\VisitLocator::class => ConfigAbstractFactory::class, + Visit\VisitsStatsHelper::class => ConfigAbstractFactory::class, + Visit\Transformer\OrphanVisitDataTransformer::class => InvokableFactory::class, + Util\UrlValidator::class => ConfigAbstractFactory::class, Util\DoctrineBatchHelper::class => ConfigAbstractFactory::class, Util\RedirectResponseHelper::class => ConfigAbstractFactory::class, @@ -58,10 +64,11 @@ return [ ], ConfigAbstractFactory::class => [ + ErrorHandler\NotFoundTypeResolverMiddleware::class => ['config.router.base_path'], + ErrorHandler\NotFoundTrackerMiddleware::class => [Visit\VisitsTracker::class], ErrorHandler\NotFoundRedirectHandler::class => [ NotFoundRedirectOptions::class, Util\RedirectResponseHelper::class, - 'config.router.base_path', ], Options\AppOptions::class => ['config.app_options'], @@ -75,10 +82,10 @@ return [ ShortUrl\Resolver\PersistenceShortUrlRelationResolver::class, Service\ShortUrl\ShortCodeHelper::class, ], - Service\VisitsTracker::class => [ + Visit\VisitsTracker::class => [ 'em', EventDispatcherInterface::class, - 'config.url_shortener.anonymize_remote_addr', + Options\UrlShortenerOptions::class, ], Service\ShortUrlService::class => [ 'em', @@ -104,14 +111,14 @@ return [ Action\RedirectAction::class => [ Service\ShortUrl\ShortUrlResolver::class, - Service\VisitsTracker::class, + Visit\VisitsTracker::class, Options\AppOptions::class, Util\RedirectResponseHelper::class, 'Logger_Shlink', ], Action\PixelAction::class => [ Service\ShortUrl\ShortUrlResolver::class, - Service\VisitsTracker::class, + Visit\VisitsTracker::class, Options\AppOptions::class, 'Logger_Shlink', ], @@ -126,7 +133,10 @@ return [ ShortUrl\Helper\ShortUrlTitleResolutionHelper::class => [Util\UrlValidator::class], ShortUrl\Transformer\ShortUrlDataTransformer::class => [ShortUrl\Helper\ShortUrlStringifier::class], - Mercure\MercureUpdatesGenerator::class => [ShortUrl\Transformer\ShortUrlDataTransformer::class], + Mercure\MercureUpdatesGenerator::class => [ + ShortUrl\Transformer\ShortUrlDataTransformer::class, + Visit\Transformer\OrphanVisitDataTransformer::class, + ], Importer\ImportedLinksProcessor::class => [ 'em', diff --git a/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Entity.Visit.php b/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Entity.Visit.php index 5143389b..efcccb65 100644 --- a/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Entity.Visit.php +++ b/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Entity.Visit.php @@ -47,11 +47,22 @@ return static function (ClassMetadata $metadata, array $emConfig): void { ->build(); $builder->createManyToOne('shortUrl', Entity\ShortUrl::class) - ->addJoinColumn('short_url_id', 'id', false, false, 'CASCADE') + ->addJoinColumn('short_url_id', 'id', true, false, 'CASCADE') ->build(); $builder->createManyToOne('visitLocation', Entity\VisitLocation::class) ->addJoinColumn('visit_location_id', 'id', true, false, 'Set NULL') ->cascadePersist() ->build(); + + $builder->createField('visitedUrl', Types::STRING) + ->columnName('visited_url') + ->length(Visitor::VISITED_URL_MAX_LENGTH) + ->nullable() + ->build(); + + $builder->createField('type', Types::STRING) + ->columnName('type') + ->length(255) + ->build(); }; diff --git a/module/Core/config/event_dispatcher.config.php b/module/Core/config/event_dispatcher.config.php index 66a23637..5c2c88e0 100644 --- a/module/Core/config/event_dispatcher.config.php +++ b/module/Core/config/event_dispatcher.config.php @@ -20,28 +20,28 @@ return [ ], ], 'async' => [ - EventDispatcher\Event\ShortUrlVisited::class => [ - EventDispatcher\LocateShortUrlVisit::class, + EventDispatcher\Event\UrlVisited::class => [ + EventDispatcher\LocateVisit::class, ], ], ], 'dependencies' => [ 'factories' => [ - EventDispatcher\LocateShortUrlVisit::class => ConfigAbstractFactory::class, + EventDispatcher\LocateVisit::class => ConfigAbstractFactory::class, EventDispatcher\NotifyVisitToWebHooks::class => ConfigAbstractFactory::class, EventDispatcher\NotifyVisitToMercure::class => ConfigAbstractFactory::class, ], 'delegators' => [ - EventDispatcher\LocateShortUrlVisit::class => [ + EventDispatcher\LocateVisit::class => [ EventDispatcher\CloseDbConnectionEventListenerDelegator::class, ], ], ], ConfigAbstractFactory::class => [ - EventDispatcher\LocateShortUrlVisit::class => [ + EventDispatcher\LocateVisit::class => [ IpLocationResolverInterface::class, 'em', 'Logger_Shlink', diff --git a/module/Core/functions/functions.php b/module/Core/functions/functions.php index f9a67e3d..00954049 100644 --- a/module/Core/functions/functions.php +++ b/module/Core/functions/functions.php @@ -9,6 +9,7 @@ use DateTimeInterface; use Fig\Http\Message\StatusCodeInterface; use Laminas\InputFilter\InputFilter; use PUGX\Shortid\Factory as ShortIdFactory; +use Shlinkio\Shlink\Common\Util\DateRange; use function Functional\reduce_left; use function is_array; @@ -44,6 +45,26 @@ function parseDateFromQuery(array $query, string $dateName): ?Chronos return ! isset($query[$dateName]) || empty($query[$dateName]) ? null : Chronos::parse($query[$dateName]); } +function parseDateRangeFromQuery(array $query, string $startDateName, string $endDateName): DateRange +{ + $startDate = parseDateFromQuery($query, $startDateName); + $endDate = parseDateFromQuery($query, $endDateName); + + if ($startDate === null && $endDate === null) { + return DateRange::emptyInstance(); + } + + if ($startDate !== null && $endDate !== null) { + return DateRange::withStartAndEndDate($startDate, $endDate); + } + + if ($startDate !== null) { + return DateRange::withStartDate($startDate); + } + + return DateRange::withEndDate($endDate); +} + /** * @param string|DateTimeInterface|Chronos|null $date */ diff --git a/module/Core/src/Action/AbstractTrackingAction.php b/module/Core/src/Action/AbstractTrackingAction.php index 86eb197b..b6a119b2 100644 --- a/module/Core/src/Action/AbstractTrackingAction.php +++ b/module/Core/src/Action/AbstractTrackingAction.php @@ -20,7 +20,7 @@ use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Model\Visitor; use Shlinkio\Shlink\Core\Options\AppOptions; use Shlinkio\Shlink\Core\Service\ShortUrl\ShortUrlResolverInterface; -use Shlinkio\Shlink\Core\Service\VisitsTrackerInterface; +use Shlinkio\Shlink\Core\Visit\VisitsTrackerInterface; use function array_key_exists; use function array_merge; diff --git a/module/Core/src/Action/RedirectAction.php b/module/Core/src/Action/RedirectAction.php index 0fc6232d..d346456b 100644 --- a/module/Core/src/Action/RedirectAction.php +++ b/module/Core/src/Action/RedirectAction.php @@ -11,8 +11,8 @@ use Psr\Http\Server\RequestHandlerInterface; use Psr\Log\LoggerInterface; use Shlinkio\Shlink\Core\Options; use Shlinkio\Shlink\Core\Service\ShortUrl\ShortUrlResolverInterface; -use Shlinkio\Shlink\Core\Service\VisitsTrackerInterface; use Shlinkio\Shlink\Core\Util\RedirectResponseHelperInterface; +use Shlinkio\Shlink\Core\Visit\VisitsTrackerInterface; class RedirectAction extends AbstractTrackingAction implements StatusCodeInterface { diff --git a/module/Core/src/Entity/Visit.php b/module/Core/src/Entity/Visit.php index 7e6ed060..61739dec 100644 --- a/module/Core/src/Entity/Visit.php +++ b/module/Core/src/Entity/Visit.php @@ -14,20 +14,29 @@ use Shlinkio\Shlink\Core\Visit\Model\VisitLocationInterface; class Visit extends AbstractEntity implements JsonSerializable { + public const TYPE_VALID_SHORT_URL = 'valid_short_url'; + public const TYPE_INVALID_SHORT_URL = 'invalid_short_url'; + public const TYPE_BASE_URL = 'base_url'; + public const TYPE_REGULAR_404 = 'regular_404'; + private string $referer; private Chronos $date; - private ?string $remoteAddr = null; + private ?string $remoteAddr; + private ?string $visitedUrl; private string $userAgent; - private ShortUrl $shortUrl; + private string $type; + private ?ShortUrl $shortUrl; private ?VisitLocation $visitLocation = null; - public function __construct(ShortUrl $shortUrl, Visitor $visitor, bool $anonymize = true, ?Chronos $date = null) + private function __construct(?ShortUrl $shortUrl, Visitor $visitor, string $type, bool $anonymize = true) { $this->shortUrl = $shortUrl; - $this->date = $date ?? Chronos::now(); + $this->date = Chronos::now(); $this->userAgent = $visitor->getUserAgent(); $this->referer = $visitor->getReferer(); $this->remoteAddr = $this->processAddress($anonymize, $visitor->getRemoteAddress()); + $this->visitedUrl = $visitor->getVisitedUrl(); + $this->type = $type; } private function processAddress(bool $anonymize, ?string $address): ?string @@ -44,6 +53,26 @@ class Visit extends AbstractEntity implements JsonSerializable } } + public static function forValidShortUrl(ShortUrl $shortUrl, Visitor $visitor, bool $anonymize = true): self + { + return new self($shortUrl, $visitor, self::TYPE_VALID_SHORT_URL, $anonymize); + } + + public static function forBasePath(Visitor $visitor, bool $anonymize = true): self + { + return new self(null, $visitor, self::TYPE_BASE_URL, $anonymize); + } + + public static function forInvalidShortUrl(Visitor $visitor, bool $anonymize = true): self + { + return new self(null, $visitor, self::TYPE_INVALID_SHORT_URL, $anonymize); + } + + public static function forRegularNotFound(Visitor $visitor, bool $anonymize = true): self + { + return new self(null, $visitor, self::TYPE_REGULAR_404, $anonymize); + } + public function getRemoteAddr(): ?string { return $this->remoteAddr; @@ -54,7 +83,7 @@ class Visit extends AbstractEntity implements JsonSerializable return ! empty($this->remoteAddr); } - public function getShortUrl(): ShortUrl + public function getShortUrl(): ?ShortUrl { return $this->shortUrl; } @@ -75,13 +104,21 @@ class Visit extends AbstractEntity implements JsonSerializable return $this; } - /** - * Specify data which should be serialized to JSON - * @link http://php.net/manual/en/jsonserializable.jsonserialize.php - * @return array data which can be serialized by json_encode, - * which is a value of any type other than a resource. - * @since 5.4.0 - */ + public function isOrphan(): bool + { + return $this->shortUrl === null; + } + + public function visitedUrl(): ?string + { + return $this->visitedUrl; + } + + public function type(): string + { + return $this->type; + } + public function jsonSerialize(): array { return [ diff --git a/module/Core/src/ErrorHandler/Model/NotFoundType.php b/module/Core/src/ErrorHandler/Model/NotFoundType.php new file mode 100644 index 00000000..57176e84 --- /dev/null +++ b/module/Core/src/ErrorHandler/Model/NotFoundType.php @@ -0,0 +1,57 @@ +type = $type; + } + + public static function fromRequest(ServerRequestInterface $request, string $basePath): self + { + $isBaseUrl = rtrim($request->getUri()->getPath(), '/') === $basePath; + if ($isBaseUrl) { + return new self(Visit::TYPE_BASE_URL); + } + + /** @var RouteResult $routeResult */ + $routeResult = $request->getAttribute(RouteResult::class, RouteResult::fromRouteFailure(null)); + if ($routeResult->isFailure()) { + return new self(Visit::TYPE_REGULAR_404); + } + + if ($routeResult->getMatchedRouteName() === RedirectAction::class) { + return new self(Visit::TYPE_INVALID_SHORT_URL); + } + + return new self(self::class); + } + + public function isBaseUrl(): bool + { + return $this->type === Visit::TYPE_BASE_URL; + } + + public function isRegularNotFound(): bool + { + return $this->type === Visit::TYPE_REGULAR_404; + } + + public function isInvalidShortUrl(): bool + { + return $this->type === Visit::TYPE_INVALID_SHORT_URL; + } +} diff --git a/module/Core/src/ErrorHandler/NotFoundRedirectHandler.php b/module/Core/src/ErrorHandler/NotFoundRedirectHandler.php index a49db5bb..1f3b4fed 100644 --- a/module/Core/src/ErrorHandler/NotFoundRedirectHandler.php +++ b/module/Core/src/ErrorHandler/NotFoundRedirectHandler.php @@ -4,67 +4,48 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\ErrorHandler; -use Mezzio\Router\RouteResult; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; -use Psr\Http\Message\UriInterface; use Psr\Http\Server\MiddlewareInterface; use Psr\Http\Server\RequestHandlerInterface; -use Shlinkio\Shlink\Core\Action\RedirectAction; +use Shlinkio\Shlink\Core\ErrorHandler\Model\NotFoundType; use Shlinkio\Shlink\Core\Options; use Shlinkio\Shlink\Core\Util\RedirectResponseHelperInterface; -use function rtrim; - class NotFoundRedirectHandler implements MiddlewareInterface { private Options\NotFoundRedirectOptions $redirectOptions; private RedirectResponseHelperInterface $redirectResponseHelper; - private string $shlinkBasePath; public function __construct( Options\NotFoundRedirectOptions $redirectOptions, - RedirectResponseHelperInterface $redirectResponseHelper, - string $shlinkBasePath + RedirectResponseHelperInterface $redirectResponseHelper ) { $this->redirectOptions = $redirectOptions; - $this->shlinkBasePath = $shlinkBasePath; $this->redirectResponseHelper = $redirectResponseHelper; } public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface { - /** @var RouteResult $routeResult */ - $routeResult = $request->getAttribute(RouteResult::class, RouteResult::fromRouteFailure(null)); - $redirectResponse = $this->createRedirectResponse($routeResult, $request->getUri()); + /** @var NotFoundType $notFoundType */ + $notFoundType = $request->getAttribute(NotFoundType::class); - return $redirectResponse ?? $handler->handle($request); - } - - private function createRedirectResponse(RouteResult $routeResult, UriInterface $uri): ?ResponseInterface - { - $isBaseUrl = rtrim($uri->getPath(), '/') === $this->shlinkBasePath; - - if ($isBaseUrl && $this->redirectOptions->hasBaseUrlRedirect()) { + if ($notFoundType->isBaseUrl() && $this->redirectOptions->hasBaseUrlRedirect()) { return $this->redirectResponseHelper->buildRedirectResponse($this->redirectOptions->getBaseUrlRedirect()); } - if (!$isBaseUrl && $routeResult->isFailure() && $this->redirectOptions->hasRegular404Redirect()) { + if ($notFoundType->isRegularNotFound() && $this->redirectOptions->hasRegular404Redirect()) { return $this->redirectResponseHelper->buildRedirectResponse( $this->redirectOptions->getRegular404Redirect(), ); } - if ( - $routeResult->isSuccess() && - $routeResult->getMatchedRouteName() === RedirectAction::class && - $this->redirectOptions->hasInvalidShortUrlRedirect() - ) { + if ($notFoundType->isInvalidShortUrl() && $this->redirectOptions->hasInvalidShortUrlRedirect()) { return $this->redirectResponseHelper->buildRedirectResponse( $this->redirectOptions->getInvalidShortUrlRedirect(), ); } - return null; + return $handler->handle($request); } } diff --git a/module/Core/src/ErrorHandler/NotFoundTemplateHandler.php b/module/Core/src/ErrorHandler/NotFoundTemplateHandler.php index 62b78973..61d67403 100644 --- a/module/Core/src/ErrorHandler/NotFoundTemplateHandler.php +++ b/module/Core/src/ErrorHandler/NotFoundTemplateHandler.php @@ -7,10 +7,10 @@ namespace Shlinkio\Shlink\Core\ErrorHandler; use Closure; use Fig\Http\Message\StatusCodeInterface; use Laminas\Diactoros\Response; -use Mezzio\Router\RouteResult; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Server\RequestHandlerInterface; +use Shlinkio\Shlink\Core\ErrorHandler\Model\NotFoundType; use function file_get_contents; use function sprintf; @@ -29,11 +29,11 @@ class NotFoundTemplateHandler implements RequestHandlerInterface public function handle(ServerRequestInterface $request): ResponseInterface { - /** @var RouteResult $routeResult */ - $routeResult = $request->getAttribute(RouteResult::class) ?? RouteResult::fromRouteFailure(null); + /** @var NotFoundType $notFoundType */ + $notFoundType = $request->getAttribute(NotFoundType::class); $status = StatusCodeInterface::STATUS_NOT_FOUND; - $template = $routeResult->isFailure() ? self::NOT_FOUND_TEMPLATE : self::INVALID_SHORT_CODE_TEMPLATE; + $template = $notFoundType->isInvalidShortUrl() ? self::INVALID_SHORT_CODE_TEMPLATE : self::NOT_FOUND_TEMPLATE; $templateContent = ($this->readFile)(sprintf('%s/%s', self::TEMPLATES_BASE_DIR, $template)); return new Response\HtmlResponse($templateContent, $status); } diff --git a/module/Core/src/ErrorHandler/NotFoundTrackerMiddleware.php b/module/Core/src/ErrorHandler/NotFoundTrackerMiddleware.php new file mode 100644 index 00000000..b81e55de --- /dev/null +++ b/module/Core/src/ErrorHandler/NotFoundTrackerMiddleware.php @@ -0,0 +1,40 @@ +visitsTracker = $visitsTracker; + } + + public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface + { + /** @var NotFoundType $notFoundType */ + $notFoundType = $request->getAttribute(NotFoundType::class); + $visitor = Visitor::fromRequest($request); + + if ($notFoundType->isBaseUrl()) { + $this->visitsTracker->trackBaseUrlVisit($visitor); + } elseif ($notFoundType->isRegularNotFound()) { + $this->visitsTracker->trackRegularNotFoundVisit($visitor); + } elseif ($notFoundType->isInvalidShortUrl()) { + $this->visitsTracker->trackInvalidShortUrlVisit($visitor); + } + + return $handler->handle($request); + } +} diff --git a/module/Core/src/ErrorHandler/NotFoundTypeResolverMiddleware.php b/module/Core/src/ErrorHandler/NotFoundTypeResolverMiddleware.php new file mode 100644 index 00000000..6f13db73 --- /dev/null +++ b/module/Core/src/ErrorHandler/NotFoundTypeResolverMiddleware.php @@ -0,0 +1,27 @@ +shlinkBasePath = $shlinkBasePath; + } + + public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface + { + $notFoundType = NotFoundType::fromRequest($request, $this->shlinkBasePath); + return $handler->handle($request->withAttribute(NotFoundType::class, $notFoundType)); + } +} diff --git a/module/Core/src/EventDispatcher/Event/ShortUrlVisited.php b/module/Core/src/EventDispatcher/Event/UrlVisited.php similarity index 88% rename from module/Core/src/EventDispatcher/Event/ShortUrlVisited.php rename to module/Core/src/EventDispatcher/Event/UrlVisited.php index f177721f..87b9e4cb 100644 --- a/module/Core/src/EventDispatcher/Event/ShortUrlVisited.php +++ b/module/Core/src/EventDispatcher/Event/UrlVisited.php @@ -4,7 +4,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\EventDispatcher\Event; -final class ShortUrlVisited extends AbstractVisitEvent +final class UrlVisited extends AbstractVisitEvent { private ?string $originalIpAddress; diff --git a/module/Core/src/EventDispatcher/LocateShortUrlVisit.php b/module/Core/src/EventDispatcher/LocateVisit.php similarity index 95% rename from module/Core/src/EventDispatcher/LocateShortUrlVisit.php rename to module/Core/src/EventDispatcher/LocateVisit.php index 8b193578..32da6060 100644 --- a/module/Core/src/EventDispatcher/LocateShortUrlVisit.php +++ b/module/Core/src/EventDispatcher/LocateVisit.php @@ -11,7 +11,7 @@ use Shlinkio\Shlink\CLI\Exception\GeolocationDbUpdateFailedException; use Shlinkio\Shlink\CLI\Util\GeolocationDbUpdaterInterface; use Shlinkio\Shlink\Core\Entity\Visit; use Shlinkio\Shlink\Core\Entity\VisitLocation; -use Shlinkio\Shlink\Core\EventDispatcher\Event\ShortUrlVisited; +use Shlinkio\Shlink\Core\EventDispatcher\Event\UrlVisited; use Shlinkio\Shlink\Core\EventDispatcher\Event\VisitLocated; use Shlinkio\Shlink\IpGeolocation\Exception\WrongIpException; use Shlinkio\Shlink\IpGeolocation\Model\Location; @@ -19,7 +19,7 @@ use Shlinkio\Shlink\IpGeolocation\Resolver\IpLocationResolverInterface; use function sprintf; -class LocateShortUrlVisit +class LocateVisit { private IpLocationResolverInterface $ipLocationResolver; private EntityManagerInterface $em; @@ -41,7 +41,7 @@ class LocateShortUrlVisit $this->eventDispatcher = $eventDispatcher; } - public function __invoke(ShortUrlVisited $shortUrlVisited): void + public function __invoke(UrlVisited $shortUrlVisited): void { $visitId = $shortUrlVisited->visitId(); diff --git a/module/Core/src/EventDispatcher/NotifyVisitToMercure.php b/module/Core/src/EventDispatcher/NotifyVisitToMercure.php index 33aab7af..0cf438ed 100644 --- a/module/Core/src/EventDispatcher/NotifyVisitToMercure.php +++ b/module/Core/src/EventDispatcher/NotifyVisitToMercure.php @@ -10,8 +10,11 @@ use Shlinkio\Shlink\Core\Entity\Visit; use Shlinkio\Shlink\Core\EventDispatcher\Event\VisitLocated; use Shlinkio\Shlink\Core\Mercure\MercureUpdatesGeneratorInterface; use Symfony\Component\Mercure\PublisherInterface; +use Symfony\Component\Mercure\Update; use Throwable; +use function Functional\each; + class NotifyVisitToMercure { private PublisherInterface $publisher; @@ -45,12 +48,26 @@ class NotifyVisitToMercure } try { - ($this->publisher)($this->updatesGenerator->newShortUrlVisitUpdate($visit)); - ($this->publisher)($this->updatesGenerator->newVisitUpdate($visit)); + each($this->determineUpdatesForVisit($visit), fn (Update $update) => ($this->publisher)($update)); } catch (Throwable $e) { $this->logger->debug('Error while trying to notify mercure hub with new visit. {e}', [ 'e' => $e, ]); } } + + /** + * @return Update[] + */ + private function determineUpdatesForVisit(Visit $visit): array + { + if ($visit->isOrphan()) { + return [$this->updatesGenerator->newOrphanVisitUpdate($visit)]; + } + + return [ + $this->updatesGenerator->newShortUrlVisitUpdate($visit), + $this->updatesGenerator->newVisitUpdate($visit), + ]; + } } diff --git a/module/Core/src/EventDispatcher/NotifyVisitToWebHooks.php b/module/Core/src/EventDispatcher/NotifyVisitToWebHooks.php index d3b27602..b236a1c1 100644 --- a/module/Core/src/EventDispatcher/NotifyVisitToWebHooks.php +++ b/module/Core/src/EventDispatcher/NotifyVisitToWebHooks.php @@ -10,6 +10,7 @@ use Fig\Http\Message\RequestMethodInterface; use GuzzleHttp\ClientInterface; use GuzzleHttp\Promise\Promise; use GuzzleHttp\Promise\PromiseInterface; +use GuzzleHttp\Promise\Utils; use GuzzleHttp\RequestOptions; use Psr\Log\LoggerInterface; use Shlinkio\Shlink\Common\Rest\DataTransformerInterface; @@ -20,7 +21,6 @@ use Throwable; use function Functional\map; use function Functional\partial_left; -use function GuzzleHttp\Promise\settle; class NotifyVisitToWebHooks { @@ -69,7 +69,7 @@ class NotifyVisitToWebHooks $requestPromises = $this->performRequests($requestOptions, $visitId); // Wait for all the promises to finish, ignoring rejections, as in those cases we only want to log the error. - settle($requestPromises)->wait(); + Utils::settle($requestPromises)->wait(); } private function buildRequestOptions(Visit $visit): array diff --git a/module/Core/src/Mercure/MercureUpdatesGenerator.php b/module/Core/src/Mercure/MercureUpdatesGenerator.php index bd00e836..23b3796c 100644 --- a/module/Core/src/Mercure/MercureUpdatesGenerator.php +++ b/module/Core/src/Mercure/MercureUpdatesGenerator.php @@ -16,29 +16,41 @@ use const JSON_THROW_ON_ERROR; final class MercureUpdatesGenerator implements MercureUpdatesGeneratorInterface { private const NEW_VISIT_TOPIC = 'https://shlink.io/new-visit'; + private const NEW_ORPHAN_VISIT_TOPIC = 'https://shlink.io/new-orphan-visit'; - private DataTransformerInterface $transformer; + private DataTransformerInterface $shortUrlTransformer; + private DataTransformerInterface $orphanVisitTransformer; - public function __construct(DataTransformerInterface $transformer) - { - $this->transformer = $transformer; + public function __construct( + DataTransformerInterface $shortUrlTransformer, + DataTransformerInterface $orphanVisitTransformer + ) { + $this->shortUrlTransformer = $shortUrlTransformer; + $this->orphanVisitTransformer = $orphanVisitTransformer; } public function newVisitUpdate(Visit $visit): Update { return new Update(self::NEW_VISIT_TOPIC, $this->serialize([ - 'shortUrl' => $this->transformer->transform($visit->getShortUrl()), + 'shortUrl' => $this->shortUrlTransformer->transform($visit->getShortUrl()), 'visit' => $visit, ])); } + public function newOrphanVisitUpdate(Visit $visit): Update + { + return new Update(self::NEW_ORPHAN_VISIT_TOPIC, $this->serialize([ + 'visit' => $this->orphanVisitTransformer->transform($visit), + ])); + } + public function newShortUrlVisitUpdate(Visit $visit): Update { $shortUrl = $visit->getShortUrl(); $topic = sprintf('%s/%s', self::NEW_VISIT_TOPIC, $shortUrl->getShortCode()); return new Update($topic, $this->serialize([ - 'shortUrl' => $this->transformer->transform($visit->getShortUrl()), + 'shortUrl' => $this->shortUrlTransformer->transform($shortUrl), 'visit' => $visit, ])); } diff --git a/module/Core/src/Mercure/MercureUpdatesGeneratorInterface.php b/module/Core/src/Mercure/MercureUpdatesGeneratorInterface.php index d433d9ad..951e805c 100644 --- a/module/Core/src/Mercure/MercureUpdatesGeneratorInterface.php +++ b/module/Core/src/Mercure/MercureUpdatesGeneratorInterface.php @@ -11,5 +11,7 @@ interface MercureUpdatesGeneratorInterface { public function newVisitUpdate(Visit $visit): Update; + public function newOrphanVisitUpdate(Visit $visit): Update; + public function newShortUrlVisitUpdate(Visit $visit): Update; } diff --git a/module/Core/src/Model/Visitor.php b/module/Core/src/Model/Visitor.php index 8c24ab26..7438bdce 100644 --- a/module/Core/src/Model/Visitor.php +++ b/module/Core/src/Model/Visitor.php @@ -14,15 +14,18 @@ final class Visitor public const USER_AGENT_MAX_LENGTH = 512; public const REFERER_MAX_LENGTH = 1024; public const REMOTE_ADDRESS_MAX_LENGTH = 256; + public const VISITED_URL_MAX_LENGTH = 2048; private string $userAgent; private string $referer; + private string $visitedUrl; private ?string $remoteAddress; - public function __construct(string $userAgent, string $referer, ?string $remoteAddress) + public function __construct(string $userAgent, string $referer, ?string $remoteAddress, string $visitedUrl) { $this->userAgent = $this->cropToLength($userAgent, self::USER_AGENT_MAX_LENGTH); $this->referer = $this->cropToLength($referer, self::REFERER_MAX_LENGTH); + $this->visitedUrl = $this->cropToLength($visitedUrl, self::VISITED_URL_MAX_LENGTH); $this->remoteAddress = $this->cropToLength($remoteAddress, self::REMOTE_ADDRESS_MAX_LENGTH); } @@ -37,12 +40,13 @@ final class Visitor $request->getHeaderLine('User-Agent'), $request->getHeaderLine('Referer'), $request->getAttribute(IpAddressMiddlewareFactory::REQUEST_ATTR), + $request->getUri()->__toString(), ); } public static function emptyInstance(): self { - return new self('', '', null); + return new self('', '', null, ''); } public function getUserAgent(): string @@ -59,4 +63,9 @@ final class Visitor { return $this->remoteAddress; } + + public function getVisitedUrl(): string + { + return $this->visitedUrl; + } } diff --git a/module/Core/src/Model/VisitsParams.php b/module/Core/src/Model/VisitsParams.php index 041aed9f..b579239b 100644 --- a/module/Core/src/Model/VisitsParams.php +++ b/module/Core/src/Model/VisitsParams.php @@ -6,7 +6,7 @@ namespace Shlinkio\Shlink\Core\Model; use Shlinkio\Shlink\Common\Util\DateRange; -use function Shlinkio\Shlink\Core\parseDateFromQuery; +use function Shlinkio\Shlink\Core\parseDateRangeFromQuery; final class VisitsParams { @@ -36,7 +36,7 @@ final class VisitsParams public static function fromRawData(array $query): self { return new self( - new DateRange(parseDateFromQuery($query, 'startDate'), parseDateFromQuery($query, 'endDate')), + parseDateRangeFromQuery($query, 'startDate', 'endDate'), (int) ($query['page'] ?? 1), isset($query['itemsPerPage']) ? (int) $query['itemsPerPage'] : null, ); diff --git a/module/Core/src/Options/UrlShortenerOptions.php b/module/Core/src/Options/UrlShortenerOptions.php index ebedbf97..e1956203 100644 --- a/module/Core/src/Options/UrlShortenerOptions.php +++ b/module/Core/src/Options/UrlShortenerOptions.php @@ -19,6 +19,8 @@ class UrlShortenerOptions extends AbstractOptions private int $redirectStatusCode = DEFAULT_REDIRECT_STATUS_CODE; private int $redirectCacheLifetime = DEFAULT_REDIRECT_CACHE_LIFETIME; private bool $autoResolveTitles = false; + private bool $anonymizeRemoteAddr = true; + private bool $trackOrphanVisits = true; public function isUrlValidationEnabled(): bool { @@ -62,9 +64,28 @@ class UrlShortenerOptions extends AbstractOptions return $this->autoResolveTitles; } - protected function setAutoResolveTitles(bool $autoResolveTitles): self + protected function setAutoResolveTitles(bool $autoResolveTitles): void { $this->autoResolveTitles = $autoResolveTitles; - return $this; + } + + public function anonymizeRemoteAddr(): bool + { + return $this->anonymizeRemoteAddr; + } + + protected function setAnonymizeRemoteAddr(bool $anonymizeRemoteAddr): void + { + $this->anonymizeRemoteAddr = $anonymizeRemoteAddr; + } + + public function trackOrphanVisits(): bool + { + return $this->trackOrphanVisits; + } + + protected function setTrackOrphanVisits(bool $trackOrphanVisits): void + { + $this->trackOrphanVisits = $trackOrphanVisits; } } diff --git a/module/Core/src/Paginator/Adapter/OrphanVisitsPaginatorAdapter.php b/module/Core/src/Paginator/Adapter/OrphanVisitsPaginatorAdapter.php new file mode 100644 index 00000000..7167b9e7 --- /dev/null +++ b/module/Core/src/Paginator/Adapter/OrphanVisitsPaginatorAdapter.php @@ -0,0 +1,30 @@ +repo = $repo; + $this->params = $params; + } + + protected function doCount(): int + { + return $this->repo->countOrphanVisits($this->params->getDateRange()); + } + + public function getSlice($offset, $length): iterable // phpcs:ignore + { + return $this->repo->findOrphanVisits($this->params->getDateRange(), $length, $offset); + } +} diff --git a/module/Core/src/Repository/VisitRepository.php b/module/Core/src/Repository/VisitRepository.php index a1df73a5..b869093e 100644 --- a/module/Core/src/Repository/VisitRepository.php +++ b/module/Core/src/Repository/VisitRepository.php @@ -7,13 +7,13 @@ namespace Shlinkio\Shlink\Core\Repository; use Doctrine\ORM\Query\ResultSetMappingBuilder; use Doctrine\ORM\QueryBuilder; use Happyr\DoctrineSpecification\EntitySpecificationRepository; -use Happyr\DoctrineSpecification\Spec; use Happyr\DoctrineSpecification\Specification\Specification; use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Entity\Visit; use Shlinkio\Shlink\Core\Entity\VisitLocation; -use Shlinkio\Shlink\Rest\ApiKey\Spec\WithApiKeySpecsEnsuringJoin; +use Shlinkio\Shlink\Core\Visit\Spec\CountOfOrphanVisits; +use Shlinkio\Shlink\Core\Visit\Spec\CountOfShortUrlVisits; use Shlinkio\Shlink\Rest\Entity\ApiKey; use const PHP_INT_MAX; @@ -168,6 +168,29 @@ class VisitRepository extends EntitySpecificationRepository implements VisitRepo return $qb; } + public function findOrphanVisits(?DateRange $dateRange = null, ?int $limit = null, ?int $offset = null): array + { + // Parameters in this query need to be inlined, not bound, as we need to use it as sub-query later + // Since they are not strictly provided by the caller, it's reasonably safe + $qb = $this->getEntityManager()->createQueryBuilder(); + $qb->from(Visit::class, 'v') + ->where($qb->expr()->isNull('v.shortUrl')); + + $this->applyDatesInline($qb, $dateRange); + + return $this->resolveVisitsWithNativeQuery($qb, $limit, $offset); + } + + public function countOrphanVisits(?DateRange $dateRange = null): int + { + return (int) $this->matchSingleScalarResult(new CountOfOrphanVisits($dateRange)); + } + + public function countVisits(?ApiKey $apiKey = null): int + { + return (int) $this->matchSingleScalarResult(new CountOfShortUrlVisits($apiKey)); + } + private function applyDatesInline(QueryBuilder $qb, ?DateRange $dateRange): void { if ($dateRange !== null && $dateRange->getStartDate() !== null) { @@ -208,11 +231,4 @@ class VisitRepository extends EntitySpecificationRepository implements VisitRepo return $query->getResult(); } - - public function countVisits(?ApiKey $apiKey = null): int - { - return (int) $this->matchSingleScalarResult( - Spec::countOf(new WithApiKeySpecsEnsuringJoin($apiKey, 'shortUrl')), - ); - } } diff --git a/module/Core/src/Repository/VisitRepositoryInterface.php b/module/Core/src/Repository/VisitRepositoryInterface.php index 526645df..3ecf0bca 100644 --- a/module/Core/src/Repository/VisitRepositoryInterface.php +++ b/module/Core/src/Repository/VisitRepositoryInterface.php @@ -62,5 +62,12 @@ interface VisitRepositoryInterface extends ObjectRepository, EntitySpecification public function countVisitsByTag(string $tag, ?DateRange $dateRange = null, ?Specification $spec = null): int; + /** + * @return Visit[] + */ + public function findOrphanVisits(?DateRange $dateRange = null, ?int $limit = null, ?int $offset = null): array; + + public function countOrphanVisits(?DateRange $dateRange = null): int; + public function countVisits(?ApiKey $apiKey = null): int; } diff --git a/module/Core/src/Service/VisitsTracker.php b/module/Core/src/Service/VisitsTracker.php deleted file mode 100644 index a8362f7c..00000000 --- a/module/Core/src/Service/VisitsTracker.php +++ /dev/null @@ -1,95 +0,0 @@ -em = $em; - $this->eventDispatcher = $eventDispatcher; - $this->anonymizeRemoteAddr = $anonymizeRemoteAddr; - } - - public function track(ShortUrl $shortUrl, Visitor $visitor): void - { - $visit = new Visit($shortUrl, $visitor, $this->anonymizeRemoteAddr); - - $this->em->persist($visit); - $this->em->flush(); - - $this->eventDispatcher->dispatch(new ShortUrlVisited($visit->getId(), $visitor->getRemoteAddress())); - } - - /** - * @return Visit[]|Paginator - * @throws ShortUrlNotFoundException - */ - public function info(ShortUrlIdentifier $identifier, VisitsParams $params, ?ApiKey $apiKey = null): Paginator - { - $spec = $apiKey !== null ? $apiKey->spec() : null; - - /** @var ShortUrlRepositoryInterface $repo */ - $repo = $this->em->getRepository(ShortUrl::class); - if (! $repo->shortCodeIsInUse($identifier->shortCode(), $identifier->domain(), $spec)) { - throw ShortUrlNotFoundException::fromNotFound($identifier); - } - - /** @var VisitRepositoryInterface $repo */ - $repo = $this->em->getRepository(Visit::class); - $paginator = new Paginator(new VisitsPaginatorAdapter($repo, $identifier, $params, $spec)); - $paginator->setMaxPerPage($params->getItemsPerPage()) - ->setCurrentPage($params->getPage()); - - return $paginator; - } - - /** - * @return Visit[]|Paginator - * @throws TagNotFoundException - */ - public function visitsForTag(string $tag, VisitsParams $params, ?ApiKey $apiKey = null): Paginator - { - /** @var TagRepository $tagRepo */ - $tagRepo = $this->em->getRepository(Tag::class); - if (! $tagRepo->tagExists($tag, $apiKey)) { - throw TagNotFoundException::fromTag($tag); - } - - /** @var VisitRepositoryInterface $repo */ - $repo = $this->em->getRepository(Visit::class); - $paginator = new Paginator(new VisitsForTagPaginatorAdapter($repo, $tag, $params, $apiKey)); - $paginator->setMaxPerPage($params->getItemsPerPage()) - ->setCurrentPage($params->getPage()); - - return $paginator; - } -} diff --git a/module/Core/src/Service/VisitsTrackerInterface.php b/module/Core/src/Service/VisitsTrackerInterface.php deleted file mode 100644 index 0814d986..00000000 --- a/module/Core/src/Service/VisitsTrackerInterface.php +++ /dev/null @@ -1,32 +0,0 @@ -dateRange = $dateRange; + $this->field = $field; + } + + protected function getSpec(): Specification + { + $criteria = []; + + if ($this->dateRange !== null && $this->dateRange->getStartDate() !== null) { + $criteria[] = Spec::gte($this->field, $this->dateRange->getStartDate()->toDateTimeString()); + } + + if ($this->dateRange !== null && $this->dateRange->getEndDate() !== null) { + $criteria[] = Spec::lte($this->field, $this->dateRange->getEndDate()->toDateTimeString()); + } + + return Spec::andX(...$criteria); + } +} diff --git a/module/Core/src/Visit/Model/VisitsStats.php b/module/Core/src/Visit/Model/VisitsStats.php index ac5083c7..982f03c4 100644 --- a/module/Core/src/Visit/Model/VisitsStats.php +++ b/module/Core/src/Visit/Model/VisitsStats.php @@ -9,16 +9,19 @@ use JsonSerializable; final class VisitsStats implements JsonSerializable { private int $visitsCount; + private int $orphanVisitsCount; - public function __construct(int $visitsCount) + public function __construct(int $visitsCount, int $orphanVisitsCount) { $this->visitsCount = $visitsCount; + $this->orphanVisitsCount = $orphanVisitsCount; } public function jsonSerialize(): array { return [ 'visitsCount' => $this->visitsCount, + 'orphanVisitsCount' => $this->orphanVisitsCount, ]; } } diff --git a/module/Core/src/Visit/Spec/CountOfOrphanVisits.php b/module/Core/src/Visit/Spec/CountOfOrphanVisits.php new file mode 100644 index 00000000..fb8ee3bd --- /dev/null +++ b/module/Core/src/Visit/Spec/CountOfOrphanVisits.php @@ -0,0 +1,30 @@ +dateRange = $dateRange; + } + + protected function getSpec(): Specification + { + return Spec::countOf(Spec::andX( + Spec::isNull('shortUrl'), + new InDateRange($this->dateRange), + )); + } +} diff --git a/module/Core/src/Visit/Spec/CountOfShortUrlVisits.php b/module/Core/src/Visit/Spec/CountOfShortUrlVisits.php new file mode 100644 index 00000000..6a125ee9 --- /dev/null +++ b/module/Core/src/Visit/Spec/CountOfShortUrlVisits.php @@ -0,0 +1,30 @@ +apiKey = $apiKey; + } + + protected function getSpec(): Specification + { + return Spec::countOf(Spec::andX( + Spec::isNotNull('shortUrl'), + new WithApiKeySpecsEnsuringJoin($this->apiKey, 'shortUrl'), + )); + } +} diff --git a/module/Core/src/Visit/Transformer/OrphanVisitDataTransformer.php b/module/Core/src/Visit/Transformer/OrphanVisitDataTransformer.php new file mode 100644 index 00000000..9f4842f5 --- /dev/null +++ b/module/Core/src/Visit/Transformer/OrphanVisitDataTransformer.php @@ -0,0 +1,24 @@ +jsonSerialize(); + $serializedVisit['visitedUrl'] = $visit->visitedUrl(); + $serializedVisit['type'] = $visit->type(); + + return $serializedVisit; + } +} diff --git a/module/Core/src/Visit/VisitsStatsHelper.php b/module/Core/src/Visit/VisitsStatsHelper.php index ab06079a..61d879fd 100644 --- a/module/Core/src/Visit/VisitsStatsHelper.php +++ b/module/Core/src/Visit/VisitsStatsHelper.php @@ -5,8 +5,22 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Visit; use Doctrine\ORM\EntityManagerInterface; +use Pagerfanta\Adapter\AdapterInterface; +use Shlinkio\Shlink\Common\Paginator\Paginator; +use Shlinkio\Shlink\Core\Entity\ShortUrl; +use Shlinkio\Shlink\Core\Entity\Tag; use Shlinkio\Shlink\Core\Entity\Visit; +use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; +use Shlinkio\Shlink\Core\Exception\TagNotFoundException; +use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; +use Shlinkio\Shlink\Core\Model\VisitsParams; +use Shlinkio\Shlink\Core\Paginator\Adapter\OrphanVisitsPaginatorAdapter; +use Shlinkio\Shlink\Core\Paginator\Adapter\VisitsForTagPaginatorAdapter; +use Shlinkio\Shlink\Core\Paginator\Adapter\VisitsPaginatorAdapter; +use Shlinkio\Shlink\Core\Repository\ShortUrlRepositoryInterface; +use Shlinkio\Shlink\Core\Repository\TagRepository; use Shlinkio\Shlink\Core\Repository\VisitRepository; +use Shlinkio\Shlink\Core\Repository\VisitRepositoryInterface; use Shlinkio\Shlink\Core\Visit\Model\VisitsStats; use Shlinkio\Shlink\Rest\Entity\ApiKey; @@ -20,14 +34,71 @@ class VisitsStatsHelper implements VisitsStatsHelperInterface } public function getVisitsStats(?ApiKey $apiKey = null): VisitsStats - { - return new VisitsStats($this->getVisitsCount($apiKey)); - } - - private function getVisitsCount(?ApiKey $apiKey): int { /** @var VisitRepository $visitsRepo */ $visitsRepo = $this->em->getRepository(Visit::class); - return $visitsRepo->countVisits($apiKey); + + return new VisitsStats($visitsRepo->countVisits($apiKey), $visitsRepo->countOrphanVisits()); + } + + /** + * @return Visit[]|Paginator + * @throws ShortUrlNotFoundException + */ + public function visitsForShortUrl( + ShortUrlIdentifier $identifier, + VisitsParams $params, + ?ApiKey $apiKey = null + ): Paginator { + $spec = $apiKey !== null ? $apiKey->spec() : null; + + /** @var ShortUrlRepositoryInterface $repo */ + $repo = $this->em->getRepository(ShortUrl::class); + if (! $repo->shortCodeIsInUse($identifier->shortCode(), $identifier->domain(), $spec)) { + throw ShortUrlNotFoundException::fromNotFound($identifier); + } + + /** @var VisitRepositoryInterface $repo */ + $repo = $this->em->getRepository(Visit::class); + + return $this->createPaginator(new VisitsPaginatorAdapter($repo, $identifier, $params, $spec), $params); + } + + /** + * @return Visit[]|Paginator + * @throws TagNotFoundException + */ + public function visitsForTag(string $tag, VisitsParams $params, ?ApiKey $apiKey = null): Paginator + { + /** @var TagRepository $tagRepo */ + $tagRepo = $this->em->getRepository(Tag::class); + if (! $tagRepo->tagExists($tag, $apiKey)) { + throw TagNotFoundException::fromTag($tag); + } + + /** @var VisitRepositoryInterface $repo */ + $repo = $this->em->getRepository(Visit::class); + + return $this->createPaginator(new VisitsForTagPaginatorAdapter($repo, $tag, $params, $apiKey), $params); + } + + /** + * @return Visit[]|Paginator + */ + public function orphanVisits(VisitsParams $params): Paginator + { + /** @var VisitRepositoryInterface $repo */ + $repo = $this->em->getRepository(Visit::class); + + return $this->createPaginator(new OrphanVisitsPaginatorAdapter($repo, $params), $params); + } + + private function createPaginator(AdapterInterface $adapter, VisitsParams $params): Paginator + { + $paginator = new Paginator($adapter); + $paginator->setMaxPerPage($params->getItemsPerPage()) + ->setCurrentPage($params->getPage()); + + return $paginator; } } diff --git a/module/Core/src/Visit/VisitsStatsHelperInterface.php b/module/Core/src/Visit/VisitsStatsHelperInterface.php index ca044d4b..d2bf6032 100644 --- a/module/Core/src/Visit/VisitsStatsHelperInterface.php +++ b/module/Core/src/Visit/VisitsStatsHelperInterface.php @@ -4,10 +4,37 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Visit; +use Shlinkio\Shlink\Common\Paginator\Paginator; +use Shlinkio\Shlink\Core\Entity\Visit; +use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; +use Shlinkio\Shlink\Core\Exception\TagNotFoundException; +use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; +use Shlinkio\Shlink\Core\Model\VisitsParams; use Shlinkio\Shlink\Core\Visit\Model\VisitsStats; use Shlinkio\Shlink\Rest\Entity\ApiKey; interface VisitsStatsHelperInterface { public function getVisitsStats(?ApiKey $apiKey = null): VisitsStats; + + /** + * @return Visit[]|Paginator + * @throws ShortUrlNotFoundException + */ + public function visitsForShortUrl( + ShortUrlIdentifier $identifier, + VisitsParams $params, + ?ApiKey $apiKey = null + ): Paginator; + + /** + * @return Visit[]|Paginator + * @throws TagNotFoundException + */ + public function visitsForTag(string $tag, VisitsParams $params, ?ApiKey $apiKey = null): Paginator; + + /** + * @return Visit[]|Paginator + */ + public function orphanVisits(VisitsParams $params): Paginator; } diff --git a/module/Core/src/Visit/VisitsTracker.php b/module/Core/src/Visit/VisitsTracker.php new file mode 100644 index 00000000..306da7a9 --- /dev/null +++ b/module/Core/src/Visit/VisitsTracker.php @@ -0,0 +1,73 @@ +em = $em; + $this->eventDispatcher = $eventDispatcher; + $this->options = $options; + } + + public function track(ShortUrl $shortUrl, Visitor $visitor): void + { + $this->trackVisit( + Visit::forValidShortUrl($shortUrl, $visitor, $this->options->anonymizeRemoteAddr()), + $visitor, + ); + } + + public function trackInvalidShortUrlVisit(Visitor $visitor): void + { + if (! $this->options->trackOrphanVisits()) { + return; + } + + $this->trackVisit(Visit::forInvalidShortUrl($visitor, $this->options->anonymizeRemoteAddr()), $visitor); + } + + public function trackBaseUrlVisit(Visitor $visitor): void + { + if (! $this->options->trackOrphanVisits()) { + return; + } + + $this->trackVisit(Visit::forBasePath($visitor, $this->options->anonymizeRemoteAddr()), $visitor); + } + + public function trackRegularNotFoundVisit(Visitor $visitor): void + { + if (! $this->options->trackOrphanVisits()) { + return; + } + + $this->trackVisit(Visit::forRegularNotFound($visitor, $this->options->anonymizeRemoteAddr()), $visitor); + } + + private function trackVisit(Visit $visit, Visitor $visitor): void + { + $this->em->persist($visit); + $this->em->flush(); + + $this->eventDispatcher->dispatch(new UrlVisited($visit->getId(), $visitor->getRemoteAddress())); + } +} diff --git a/module/Core/src/Visit/VisitsTrackerInterface.php b/module/Core/src/Visit/VisitsTrackerInterface.php new file mode 100644 index 00000000..ae70d550 --- /dev/null +++ b/module/Core/src/Visit/VisitsTrackerInterface.php @@ -0,0 +1,19 @@ +getEntityManager()->persist($foo); $bar = ShortUrl::withLongUrl('bar'); - $visit = new Visit($bar, Visitor::emptyInstance()); + $visit = Visit::forValidShortUrl($bar, Visitor::emptyInstance()); $this->getEntityManager()->persist($visit); $bar->setVisits(new ArrayCollection([$visit])); $this->getEntityManager()->persist($bar); diff --git a/module/Core/test-db/Repository/TagRepositoryTest.php b/module/Core/test-db/Repository/TagRepositoryTest.php index 0a91775b..34a06a40 100644 --- a/module/Core/test-db/Repository/TagRepositoryTest.php +++ b/module/Core/test-db/Repository/TagRepositoryTest.php @@ -64,13 +64,13 @@ class TagRepositoryTest extends DatabaseTestCase $shortUrl = ShortUrl::fromMeta($metaWithTags($firstUrlTags), $this->relationResolver); $this->getEntityManager()->persist($shortUrl); - $this->getEntityManager()->persist(new Visit($shortUrl, Visitor::emptyInstance())); - $this->getEntityManager()->persist(new Visit($shortUrl, Visitor::emptyInstance())); - $this->getEntityManager()->persist(new Visit($shortUrl, Visitor::emptyInstance())); + $this->getEntityManager()->persist(Visit::forValidShortUrl($shortUrl, Visitor::emptyInstance())); + $this->getEntityManager()->persist(Visit::forValidShortUrl($shortUrl, Visitor::emptyInstance())); + $this->getEntityManager()->persist(Visit::forValidShortUrl($shortUrl, Visitor::emptyInstance())); $shortUrl2 = ShortUrl::fromMeta($metaWithTags($secondUrlTags), $this->relationResolver); $this->getEntityManager()->persist($shortUrl2); - $this->getEntityManager()->persist(new Visit($shortUrl2, Visitor::emptyInstance())); + $this->getEntityManager()->persist(Visit::forValidShortUrl($shortUrl2, Visitor::emptyInstance())); $this->getEntityManager()->flush(); $result = $this->repo->findTagsWithInfo(); diff --git a/module/Core/test-db/Repository/VisitRepositoryTest.php b/module/Core/test-db/Repository/VisitRepositoryTest.php index 740edff5..b6c23699 100644 --- a/module/Core/test-db/Repository/VisitRepositoryTest.php +++ b/module/Core/test-db/Repository/VisitRepositoryTest.php @@ -5,6 +5,7 @@ declare(strict_types=1); namespace ShlinkioTest\Shlink\Core\Repository; use Cake\Chronos\Chronos; +use ReflectionObject; use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Entity\Domain; use Shlinkio\Shlink\Core\Entity\ShortUrl; @@ -52,7 +53,7 @@ class VisitRepositoryTest extends DatabaseTestCase }; for ($i = 0; $i < 6; $i++) { - $visit = new Visit($shortUrl, Visitor::emptyInstance()); + $visit = Visit::forValidShortUrl($shortUrl, Visitor::emptyInstance()); if ($i >= 2) { $location = new VisitLocation(Location::emptyInstance()); @@ -168,7 +169,7 @@ class VisitRepositoryTest extends DatabaseTestCase } /** @test */ - public function countReturnsExpectedResultBasedOnApiKey(): void + public function countVisitsReturnsExpectedResultBasedOnApiKey(): void { $domain = new Domain('foo.com'); $this->getEntityManager()->persist($domain); @@ -200,12 +201,87 @@ class VisitRepositoryTest extends DatabaseTestCase $domainApiKey = ApiKey::withRoles(RoleDefinition::forDomain($domain)); $this->getEntityManager()->persist($domainApiKey); + // Visits not linked to any short URL + $this->getEntityManager()->persist(Visit::forBasePath(Visitor::emptyInstance())); + $this->getEntityManager()->persist(Visit::forInvalidShortUrl(Visitor::emptyInstance())); + $this->getEntityManager()->persist(Visit::forRegularNotFound(Visitor::emptyInstance())); + $this->getEntityManager()->flush(); self::assertEquals(4 + 5 + 7, $this->repo->countVisits()); self::assertEquals(4, $this->repo->countVisits($apiKey1)); self::assertEquals(5 + 7, $this->repo->countVisits($apiKey2)); self::assertEquals(4 + 7, $this->repo->countVisits($domainApiKey)); + self::assertEquals(3, $this->repo->countOrphanVisits()); + } + + /** @test */ + public function findOrphanVisitsReturnsExpectedResult(): void + { + $shortUrl = ShortUrl::fromMeta(ShortUrlMeta::fromRawData(['longUrl' => ''])); + $this->getEntityManager()->persist($shortUrl); + $this->createVisitsForShortUrl($shortUrl, 7); + + for ($i = 0; $i < 6; $i++) { + $this->getEntityManager()->persist($this->setDateOnVisit( + Visit::forBasePath(Visitor::emptyInstance()), + Chronos::parse(sprintf('2020-01-0%s', $i + 1)), + )); + $this->getEntityManager()->persist($this->setDateOnVisit( + Visit::forInvalidShortUrl(Visitor::emptyInstance()), + Chronos::parse(sprintf('2020-01-0%s', $i + 1)), + )); + $this->getEntityManager()->persist($this->setDateOnVisit( + Visit::forRegularNotFound(Visitor::emptyInstance()), + Chronos::parse(sprintf('2020-01-0%s', $i + 1)), + )); + } + + $this->getEntityManager()->flush(); + + self::assertCount(18, $this->repo->findOrphanVisits()); + self::assertCount(5, $this->repo->findOrphanVisits(null, 5)); + self::assertCount(10, $this->repo->findOrphanVisits(null, 15, 8)); + self::assertCount(9, $this->repo->findOrphanVisits(DateRange::withStartDate(Chronos::parse('2020-01-04')), 15)); + self::assertCount(2, $this->repo->findOrphanVisits( + DateRange::withStartAndEndDate(Chronos::parse('2020-01-02'), Chronos::parse('2020-01-03')), + 6, + 4, + )); + self::assertCount(3, $this->repo->findOrphanVisits(DateRange::withEndDate(Chronos::parse('2020-01-01')))); + } + + /** @test */ + public function countOrphanVisitsReturnsExpectedResult(): void + { + $shortUrl = ShortUrl::fromMeta(ShortUrlMeta::fromRawData(['longUrl' => ''])); + $this->getEntityManager()->persist($shortUrl); + $this->createVisitsForShortUrl($shortUrl, 7); + + for ($i = 0; $i < 6; $i++) { + $this->getEntityManager()->persist($this->setDateOnVisit( + Visit::forBasePath(Visitor::emptyInstance()), + Chronos::parse(sprintf('2020-01-0%s', $i + 1)), + )); + $this->getEntityManager()->persist($this->setDateOnVisit( + Visit::forInvalidShortUrl(Visitor::emptyInstance()), + Chronos::parse(sprintf('2020-01-0%s', $i + 1)), + )); + $this->getEntityManager()->persist($this->setDateOnVisit( + Visit::forRegularNotFound(Visitor::emptyInstance()), + Chronos::parse(sprintf('2020-01-0%s', $i + 1)), + )); + } + + $this->getEntityManager()->flush(); + + self::assertEquals(18, $this->repo->countOrphanVisits()); + self::assertEquals(18, $this->repo->countOrphanVisits(DateRange::emptyInstance())); + self::assertEquals(9, $this->repo->countOrphanVisits(DateRange::withStartDate(Chronos::parse('2020-01-04')))); + self::assertEquals(6, $this->repo->countOrphanVisits( + DateRange::withStartAndEndDate(Chronos::parse('2020-01-02'), Chronos::parse('2020-01-03')), + )); + self::assertEquals(3, $this->repo->countOrphanVisits(DateRange::withEndDate(Chronos::parse('2020-01-01')))); } private function createShortUrlsAndVisits(bool $withDomain = true, array $tags = []): array @@ -237,13 +313,22 @@ class VisitRepositoryTest extends DatabaseTestCase private function createVisitsForShortUrl(ShortUrl $shortUrl, int $amount = 6): void { for ($i = 0; $i < $amount; $i++) { - $visit = new Visit( - $shortUrl, - Visitor::emptyInstance(), - true, + $visit = $this->setDateOnVisit( + Visit::forValidShortUrl($shortUrl, Visitor::emptyInstance()), Chronos::parse(sprintf('2016-01-0%s', $i + 1)), ); + $this->getEntityManager()->persist($visit); } } + + private function setDateOnVisit(Visit $visit, Chronos $date): Visit + { + $ref = new ReflectionObject($visit); + $dateProp = $ref->getProperty('date'); + $dateProp->setAccessible(true); + $dateProp->setValue($visit, $date); + + return $visit; + } } diff --git a/module/Core/test/Action/PixelActionTest.php b/module/Core/test/Action/PixelActionTest.php index cae74926..065cc2c4 100644 --- a/module/Core/test/Action/PixelActionTest.php +++ b/module/Core/test/Action/PixelActionTest.php @@ -16,7 +16,7 @@ use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Options\AppOptions; use Shlinkio\Shlink\Core\Service\ShortUrl\ShortUrlResolverInterface; -use Shlinkio\Shlink\Core\Service\VisitsTracker; +use Shlinkio\Shlink\Core\Visit\VisitsTracker; class PixelActionTest extends TestCase { diff --git a/module/Core/test/Action/RedirectActionTest.php b/module/Core/test/Action/RedirectActionTest.php index 411d9a50..f869e2c4 100644 --- a/module/Core/test/Action/RedirectActionTest.php +++ b/module/Core/test/Action/RedirectActionTest.php @@ -19,8 +19,8 @@ use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Options; use Shlinkio\Shlink\Core\Service\ShortUrl\ShortUrlResolverInterface; -use Shlinkio\Shlink\Core\Service\VisitsTrackerInterface; use Shlinkio\Shlink\Core\Util\RedirectResponseHelperInterface; +use Shlinkio\Shlink\Core\Visit\VisitsTrackerInterface; use function array_key_exists; diff --git a/module/Core/test/Entity/VisitTest.php b/module/Core/test/Entity/VisitTest.php index d583c799..7be3c3fc 100644 --- a/module/Core/test/Entity/VisitTest.php +++ b/module/Core/test/Entity/VisitTest.php @@ -4,7 +4,6 @@ declare(strict_types=1); namespace ShlinkioTest\Shlink\Core\Entity; -use Cake\Chronos\Chronos; use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\Common\Util\IpAddress; use Shlinkio\Shlink\Core\Entity\ShortUrl; @@ -13,35 +12,30 @@ use Shlinkio\Shlink\Core\Model\Visitor; class VisitTest extends TestCase { - /** - * @test - * @dataProvider provideDates - */ - public function isProperlyJsonSerialized(?Chronos $date): void + /** @test */ + public function isProperlyJsonSerialized(): void { - $visit = new Visit(ShortUrl::createEmpty(), new Visitor('Chrome', 'some site', '1.2.3.4'), true, $date); + $visit = Visit::forValidShortUrl(ShortUrl::createEmpty(), new Visitor('Chrome', 'some site', '1.2.3.4', '')); self::assertEquals([ 'referer' => 'some site', - 'date' => ($date ?? $visit->getDate())->toAtomString(), + 'date' => $visit->getDate()->toAtomString(), 'userAgent' => 'Chrome', 'visitLocation' => null, ], $visit->jsonSerialize()); } - public function provideDates(): iterable - { - yield 'null date' => [null]; - yield 'not null date' => [Chronos::now()->subDays(10)]; - } - /** * @test * @dataProvider provideAddresses */ public function addressIsAnonymizedWhenRequested(bool $anonymize, ?string $address, ?string $expectedAddress): void { - $visit = new Visit(ShortUrl::createEmpty(), new Visitor('Chrome', 'some site', $address), $anonymize); + $visit = Visit::forValidShortUrl( + ShortUrl::createEmpty(), + new Visitor('Chrome', 'some site', $address, ''), + $anonymize, + ); self::assertEquals($expectedAddress, $visit->getRemoteAddr()); } diff --git a/module/Core/test/ErrorHandler/NotFoundRedirectHandlerTest.php b/module/Core/test/ErrorHandler/NotFoundRedirectHandlerTest.php index 83810d22..9df49879 100644 --- a/module/Core/test/ErrorHandler/NotFoundRedirectHandlerTest.php +++ b/module/Core/test/ErrorHandler/NotFoundRedirectHandlerTest.php @@ -17,6 +17,7 @@ use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Server\MiddlewareInterface; use Psr\Http\Server\RequestHandlerInterface; use Shlinkio\Shlink\Core\Action\RedirectAction; +use Shlinkio\Shlink\Core\ErrorHandler\Model\NotFoundType; use Shlinkio\Shlink\Core\ErrorHandler\NotFoundRedirectHandler; use Shlinkio\Shlink\Core\Options\NotFoundRedirectOptions; use Shlinkio\Shlink\Core\Util\RedirectResponseHelperInterface; @@ -33,7 +34,7 @@ class NotFoundRedirectHandlerTest extends TestCase { $this->redirectOptions = new NotFoundRedirectOptions(); $this->helper = $this->prophesize(RedirectResponseHelperInterface::class); - $this->middleware = new NotFoundRedirectHandler($this->redirectOptions, $this->helper->reveal(), ''); + $this->middleware = new NotFoundRedirectHandler($this->redirectOptions, $this->helper->reveal()); } /** @@ -64,19 +65,19 @@ class NotFoundRedirectHandlerTest extends TestCase public function provideRedirects(): iterable { yield 'base URL with trailing slash' => [ - ServerRequestFactory::fromGlobals()->withUri(new Uri('/')), + $this->withNotFoundType(ServerRequestFactory::fromGlobals()->withUri(new Uri('/'))), 'baseUrl', ]; yield 'base URL without trailing slash' => [ - ServerRequestFactory::fromGlobals()->withUri(new Uri('')), + $this->withNotFoundType(ServerRequestFactory::fromGlobals()->withUri(new Uri(''))), 'baseUrl', ]; yield 'regular 404' => [ - ServerRequestFactory::fromGlobals()->withUri(new Uri('/foo/bar')), + $this->withNotFoundType(ServerRequestFactory::fromGlobals()->withUri(new Uri('/foo/bar'))), 'regular404', ]; yield 'invalid short URL' => [ - ServerRequestFactory::fromGlobals() + $this->withNotFoundType(ServerRequestFactory::fromGlobals() ->withAttribute( RouteResult::class, RouteResult::fromRoute( @@ -88,7 +89,7 @@ class NotFoundRedirectHandlerTest extends TestCase ), ), ) - ->withUri(new Uri('/abc123')), + ->withUri(new Uri('/abc123'))), 'invalidShortUrl', ]; } @@ -96,7 +97,7 @@ class NotFoundRedirectHandlerTest extends TestCase /** @test */ public function nextMiddlewareIsInvokedWhenNotRedirectNeedsToOccur(): void { - $req = ServerRequestFactory::fromGlobals(); + $req = $this->withNotFoundType(ServerRequestFactory::fromGlobals()); $resp = new Response(); $buildResp = $this->helper->buildRedirectResponse(Argument::cetera()); @@ -110,4 +111,10 @@ class NotFoundRedirectHandlerTest extends TestCase $buildResp->shouldNotHaveBeenCalled(); $handle->shouldHaveBeenCalledOnce(); } + + private function withNotFoundType(ServerRequestInterface $req): ServerRequestInterface + { + $type = NotFoundType::fromRequest($req, ''); + return $req->withAttribute(NotFoundType::class, $type); + } } diff --git a/module/Core/test/ErrorHandler/NotFoundTemplateHandlerTest.php b/module/Core/test/ErrorHandler/NotFoundTemplateHandlerTest.php index 6b9f9989..dcf42b54 100644 --- a/module/Core/test/ErrorHandler/NotFoundTemplateHandlerTest.php +++ b/module/Core/test/ErrorHandler/NotFoundTemplateHandlerTest.php @@ -4,30 +4,31 @@ declare(strict_types=1); namespace ShlinkioTest\Shlink\Core\ErrorHandler; -use Closure; use Laminas\Diactoros\Response; use Laminas\Diactoros\ServerRequestFactory; +use Laminas\Diactoros\Uri; use Mezzio\Router\Route; use Mezzio\Router\RouteResult; use PHPUnit\Framework\TestCase; use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Server\MiddlewareInterface; +use Shlinkio\Shlink\Core\Action\RedirectAction; +use Shlinkio\Shlink\Core\ErrorHandler\Model\NotFoundType; use Shlinkio\Shlink\Core\ErrorHandler\NotFoundTemplateHandler; class NotFoundTemplateHandlerTest extends TestCase { private NotFoundTemplateHandler $handler; - private Closure $readFile; private bool $readFileCalled; public function setUp(): void { $this->readFileCalled = false; - $this->readFile = function (string $fileName): string { + $readFile = function (string $fileName): string { $this->readFileCalled = true; return $fileName; }; - $this->handler = new NotFoundTemplateHandler($this->readFile); + $this->handler = new NotFoundTemplateHandler($readFile); } /** @@ -45,15 +46,29 @@ class NotFoundTemplateHandlerTest extends TestCase public function provideTemplates(): iterable { - $request = ServerRequestFactory::fromGlobals(); + $request = ServerRequestFactory::fromGlobals()->withUri(new Uri('/foo')); - yield [$request, NotFoundTemplateHandler::NOT_FOUND_TEMPLATE]; - yield [ - $request->withAttribute( + yield 'base url' => [$this->withNotFoundType($request, '/foo'), NotFoundTemplateHandler::NOT_FOUND_TEMPLATE]; + yield 'regular not found' => [$this->withNotFoundType($request), NotFoundTemplateHandler::NOT_FOUND_TEMPLATE]; + yield 'invalid short code' => [ + $this->withNotFoundType($request->withAttribute( RouteResult::class, - RouteResult::fromRoute(new Route('', $this->prophesize(MiddlewareInterface::class)->reveal())), - ), + RouteResult::fromRoute( + new Route( + '', + $this->prophesize(MiddlewareInterface::class)->reveal(), + ['GET'], + RedirectAction::class, + ), + ), + )), NotFoundTemplateHandler::INVALID_SHORT_CODE_TEMPLATE, ]; } + + private function withNotFoundType(ServerRequestInterface $req, string $baseUrl = ''): ServerRequestInterface + { + $type = NotFoundType::fromRequest($req, $baseUrl); + return $req->withAttribute(NotFoundType::class, $type); + } } diff --git a/module/Core/test/ErrorHandler/NotFoundTrackerMiddlewareTest.php b/module/Core/test/ErrorHandler/NotFoundTrackerMiddlewareTest.php new file mode 100644 index 00000000..560a2468 --- /dev/null +++ b/module/Core/test/ErrorHandler/NotFoundTrackerMiddlewareTest.php @@ -0,0 +1,95 @@ +notFoundType = $this->prophesize(NotFoundType::class); + $this->handler = $this->prophesize(RequestHandlerInterface::class); + $this->handler->handle(Argument::cetera())->willReturn(new Response()); + + $this->visitsTracker = $this->prophesize(VisitsTrackerInterface::class); + $this->middleware = new NotFoundTrackerMiddleware($this->visitsTracker->reveal()); + + $this->request = ServerRequestFactory::fromGlobals()->withAttribute( + NotFoundType::class, + $this->notFoundType->reveal(), + ); + } + + /** @test */ + public function baseUrlErrorIsTracked(): void + { + $isBaseUrl = $this->notFoundType->isBaseUrl()->willReturn(true); + $isRegularNotFound = $this->notFoundType->isRegularNotFound()->willReturn(false); + $isInvalidShortUrl = $this->notFoundType->isInvalidShortUrl()->willReturn(false); + + $this->middleware->process($this->request, $this->handler->reveal()); + + $isBaseUrl->shouldHaveBeenCalledOnce(); + $isRegularNotFound->shouldNotHaveBeenCalled(); + $isInvalidShortUrl->shouldNotHaveBeenCalled(); + $this->visitsTracker->trackBaseUrlVisit(Argument::type(Visitor::class))->shouldHaveBeenCalledOnce(); + $this->visitsTracker->trackRegularNotFoundVisit(Argument::type(Visitor::class))->shouldNotHaveBeenCalled(); + $this->visitsTracker->trackInvalidShortUrlVisit(Argument::type(Visitor::class))->shouldNotHaveBeenCalled(); + } + + /** @test */ + public function regularNotFoundErrorIsTracked(): void + { + $isBaseUrl = $this->notFoundType->isBaseUrl()->willReturn(false); + $isRegularNotFound = $this->notFoundType->isRegularNotFound()->willReturn(true); + $isInvalidShortUrl = $this->notFoundType->isInvalidShortUrl()->willReturn(false); + + $this->middleware->process($this->request, $this->handler->reveal()); + + $isBaseUrl->shouldHaveBeenCalledOnce(); + $isRegularNotFound->shouldHaveBeenCalledOnce(); + $isInvalidShortUrl->shouldNotHaveBeenCalled(); + $this->visitsTracker->trackBaseUrlVisit(Argument::type(Visitor::class))->shouldNotHaveBeenCalled(); + $this->visitsTracker->trackRegularNotFoundVisit(Argument::type(Visitor::class))->shouldHaveBeenCalledOnce(); + $this->visitsTracker->trackInvalidShortUrlVisit(Argument::type(Visitor::class))->shouldNotHaveBeenCalled(); + } + + /** @test */ + public function invalidShortUrlErrorIsTracked(): void + { + $isBaseUrl = $this->notFoundType->isBaseUrl()->willReturn(false); + $isRegularNotFound = $this->notFoundType->isRegularNotFound()->willReturn(false); + $isInvalidShortUrl = $this->notFoundType->isInvalidShortUrl()->willReturn(true); + + $this->middleware->process($this->request, $this->handler->reveal()); + + $isBaseUrl->shouldHaveBeenCalledOnce(); + $isRegularNotFound->shouldHaveBeenCalledOnce(); + $isInvalidShortUrl->shouldHaveBeenCalledOnce(); + $this->visitsTracker->trackBaseUrlVisit(Argument::type(Visitor::class))->shouldNotHaveBeenCalled(); + $this->visitsTracker->trackRegularNotFoundVisit(Argument::type(Visitor::class))->shouldNotHaveBeenCalled(); + $this->visitsTracker->trackInvalidShortUrlVisit(Argument::type(Visitor::class))->shouldHaveBeenCalledOnce(); + } +} diff --git a/module/Core/test/ErrorHandler/NotFoundTypeResolverMiddlewareTest.php b/module/Core/test/ErrorHandler/NotFoundTypeResolverMiddlewareTest.php new file mode 100644 index 00000000..c5d9be79 --- /dev/null +++ b/module/Core/test/ErrorHandler/NotFoundTypeResolverMiddlewareTest.php @@ -0,0 +1,47 @@ +middleware = new NotFoundTypeResolverMiddleware(''); + $this->handler = $this->prophesize(RequestHandlerInterface::class); + } + + /** @test */ + public function notFoundTypeIsAddedToRequest(): void + { + $request = ServerRequestFactory::fromGlobals(); + $handle = $this->handler->handle(Argument::that(function (ServerRequestInterface $req) { + Assert::assertArrayHasKey(NotFoundType::class, $req->getAttributes()); + + return true; + }))->willReturn(new Response()); + + $this->middleware->process($request, $this->handler->reveal()); + + self::assertArrayNotHasKey(NotFoundType::class, $request->getAttributes()); + $handle->shouldHaveBeenCalledOnce(); + } +} diff --git a/module/Core/test/EventDispatcher/LocateShortUrlVisitTest.php b/module/Core/test/EventDispatcher/LocateVisitTest.php similarity index 80% rename from module/Core/test/EventDispatcher/LocateShortUrlVisitTest.php rename to module/Core/test/EventDispatcher/LocateVisitTest.php index 4d348528..081f0f86 100644 --- a/module/Core/test/EventDispatcher/LocateShortUrlVisitTest.php +++ b/module/Core/test/EventDispatcher/LocateVisitTest.php @@ -17,19 +17,19 @@ use Shlinkio\Shlink\Common\Util\IpAddress; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Entity\Visit; use Shlinkio\Shlink\Core\Entity\VisitLocation; -use Shlinkio\Shlink\Core\EventDispatcher\Event\ShortUrlVisited; +use Shlinkio\Shlink\Core\EventDispatcher\Event\UrlVisited; use Shlinkio\Shlink\Core\EventDispatcher\Event\VisitLocated; -use Shlinkio\Shlink\Core\EventDispatcher\LocateShortUrlVisit; +use Shlinkio\Shlink\Core\EventDispatcher\LocateVisit; use Shlinkio\Shlink\Core\Model\Visitor; use Shlinkio\Shlink\IpGeolocation\Exception\WrongIpException; use Shlinkio\Shlink\IpGeolocation\Model\Location; use Shlinkio\Shlink\IpGeolocation\Resolver\IpLocationResolverInterface; -class LocateShortUrlVisitTest extends TestCase +class LocateVisitTest extends TestCase { use ProphecyTrait; - private LocateShortUrlVisit $locateVisit; + private LocateVisit $locateVisit; private ObjectProphecy $ipLocationResolver; private ObjectProphecy $em; private ObjectProphecy $logger; @@ -44,7 +44,7 @@ class LocateShortUrlVisitTest extends TestCase $this->dbUpdater = $this->prophesize(GeolocationDbUpdaterInterface::class); $this->eventDispatcher = $this->prophesize(EventDispatcherInterface::class); - $this->locateVisit = new LocateShortUrlVisit( + $this->locateVisit = new LocateVisit( $this->ipLocationResolver->reveal(), $this->em->reveal(), $this->logger->reveal(), @@ -56,7 +56,7 @@ class LocateShortUrlVisitTest extends TestCase /** @test */ public function invalidVisitLogsWarning(): void { - $event = new ShortUrlVisited('123'); + $event = new UrlVisited('123'); $findVisit = $this->em->find(Visit::class, '123')->willReturn(null); $logWarning = $this->logger->warning('Tried to locate visit with id "{visitId}", but it does not exist.', [ 'visitId' => 123, @@ -76,9 +76,9 @@ class LocateShortUrlVisitTest extends TestCase /** @test */ public function invalidAddressLogsWarning(): void { - $event = new ShortUrlVisited('123'); + $event = new UrlVisited('123'); $findVisit = $this->em->find(Visit::class, '123')->willReturn( - new Visit(ShortUrl::createEmpty(), new Visitor('', '', '1.2.3.4')), + Visit::forValidShortUrl(ShortUrl::createEmpty(), new Visitor('', '', '1.2.3.4', '')), ); $resolveLocation = $this->ipLocationResolver->resolveIpLocation(Argument::cetera())->willThrow( WrongIpException::class, @@ -105,7 +105,7 @@ class LocateShortUrlVisitTest extends TestCase */ public function nonLocatableVisitsResolveToEmptyLocations(Visit $visit): void { - $event = new ShortUrlVisited('123'); + $event = new UrlVisited('123'); $findVisit = $this->em->find(Visit::class, '123')->willReturn($visit); $flush = $this->em->flush()->will(function (): void { }); @@ -127,21 +127,20 @@ class LocateShortUrlVisitTest extends TestCase { $shortUrl = ShortUrl::createEmpty(); - yield 'null IP' => [new Visit($shortUrl, new Visitor('', '', null))]; - yield 'empty IP' => [new Visit($shortUrl, new Visitor('', '', ''))]; - yield 'localhost' => [new Visit($shortUrl, new Visitor('', '', IpAddress::LOCALHOST))]; + yield 'null IP' => [Visit::forValidShortUrl($shortUrl, new Visitor('', '', null, ''))]; + yield 'empty IP' => [Visit::forValidShortUrl($shortUrl, new Visitor('', '', '', ''))]; + yield 'localhost' => [Visit::forValidShortUrl($shortUrl, new Visitor('', '', IpAddress::LOCALHOST, ''))]; } /** * @test * @dataProvider provideIpAddresses */ - public function locatableVisitsResolveToLocation(string $anonymizedIpAddress, ?string $originalIpAddress): void + public function locatableVisitsResolveToLocation(Visit $visit, ?string $originalIpAddress): void { - $ipAddr = $originalIpAddress ?? $anonymizedIpAddress; - $visit = new Visit(ShortUrl::createEmpty(), new Visitor('', '', $ipAddr)); + $ipAddr = $originalIpAddress ?? $visit->getRemoteAddr(); $location = new Location('', '', '', '', 0.0, 0.0, ''); - $event = new ShortUrlVisited('123', $originalIpAddress); + $event = new UrlVisited('123', $originalIpAddress); $findVisit = $this->em->find(Visit::class, '123')->willReturn($visit); $flush = $this->em->flush()->will(function (): void { @@ -162,8 +161,17 @@ class LocateShortUrlVisitTest extends TestCase public function provideIpAddresses(): iterable { - yield 'no original IP address' => ['1.2.3.0', null]; - yield 'original IP address' => ['1.2.3.0', '1.2.3.4']; + yield 'no original IP address' => [ + Visit::forValidShortUrl(ShortUrl::createEmpty(), new Visitor('', '', '1.2.3.4', '')), + null, + ]; + yield 'original IP address' => [ + Visit::forValidShortUrl(ShortUrl::createEmpty(), new Visitor('', '', '1.2.3.4', '')), + '1.2.3.4', + ]; + yield 'base url' => [Visit::forBasePath(new Visitor('', '', '1.2.3.4', '')), '1.2.3.4']; + yield 'invalid short url' => [Visit::forInvalidShortUrl(new Visitor('', '', '1.2.3.4', '')), '1.2.3.4']; + yield 'regular not found' => [Visit::forRegularNotFound(new Visitor('', '', '1.2.3.4', '')), '1.2.3.4']; } /** @test */ @@ -171,9 +179,9 @@ class LocateShortUrlVisitTest extends TestCase { $e = GeolocationDbUpdateFailedException::withOlderDb(); $ipAddr = '1.2.3.0'; - $visit = new Visit(ShortUrl::createEmpty(), new Visitor('', '', $ipAddr)); + $visit = Visit::forValidShortUrl(ShortUrl::createEmpty(), new Visitor('', '', $ipAddr, '')); $location = new Location('', '', '', '', 0.0, 0.0, ''); - $event = new ShortUrlVisited('123'); + $event = new UrlVisited('123'); $findVisit = $this->em->find(Visit::class, '123')->willReturn($visit); $flush = $this->em->flush()->will(function (): void { @@ -202,9 +210,9 @@ class LocateShortUrlVisitTest extends TestCase { $e = GeolocationDbUpdateFailedException::withoutOlderDb(); $ipAddr = '1.2.3.0'; - $visit = new Visit(ShortUrl::createEmpty(), new Visitor('', '', $ipAddr)); + $visit = Visit::forValidShortUrl(ShortUrl::createEmpty(), new Visitor('', '', $ipAddr, '')); $location = new Location('', '', '', '', 0.0, 0.0, ''); - $event = new ShortUrlVisited('123'); + $event = new UrlVisited('123'); $findVisit = $this->em->find(Visit::class, '123')->willReturn($visit); $flush = $this->em->flush()->will(function (): void { diff --git a/module/Core/test/EventDispatcher/NotifyVisitToMercureTest.php b/module/Core/test/EventDispatcher/NotifyVisitToMercureTest.php index f5b525ea..f323a155 100644 --- a/module/Core/test/EventDispatcher/NotifyVisitToMercureTest.php +++ b/module/Core/test/EventDispatcher/NotifyVisitToMercureTest.php @@ -57,10 +57,9 @@ class NotifyVisitToMercureTest extends TestCase $logDebug = $this->logger->debug(Argument::cetera()); $buildNewShortUrlVisitUpdate = $this->updatesGenerator->newShortUrlVisitUpdate( Argument::type(Visit::class), - )->willReturn(new Update('', '')); - $buildNewVisitUpdate = $this->updatesGenerator->newVisitUpdate(Argument::type(Visit::class))->willReturn( - new Update('', ''), ); + $buildNewOrphanVisitUpdate = $this->updatesGenerator->newOrphanVisitUpdate(Argument::type(Visit::class)); + $buildNewVisitUpdate = $this->updatesGenerator->newVisitUpdate(Argument::type(Visit::class)); $publish = $this->publisher->__invoke(Argument::type(Update::class)); ($this->listener)(new VisitLocated($visitId)); @@ -70,6 +69,7 @@ class NotifyVisitToMercureTest extends TestCase $logDebug->shouldNotHaveBeenCalled(); $buildNewShortUrlVisitUpdate->shouldNotHaveBeenCalled(); $buildNewVisitUpdate->shouldNotHaveBeenCalled(); + $buildNewOrphanVisitUpdate->shouldNotHaveBeenCalled(); $publish->shouldNotHaveBeenCalled(); } @@ -77,13 +77,14 @@ class NotifyVisitToMercureTest extends TestCase public function notificationsAreSentWhenVisitIsFound(): void { $visitId = '123'; - $visit = new Visit(ShortUrl::createEmpty(), Visitor::emptyInstance()); + $visit = Visit::forValidShortUrl(ShortUrl::createEmpty(), Visitor::emptyInstance()); $update = new Update('', ''); $findVisit = $this->em->find(Visit::class, $visitId)->willReturn($visit); $logWarning = $this->logger->warning(Argument::cetera()); $logDebug = $this->logger->debug(Argument::cetera()); $buildNewShortUrlVisitUpdate = $this->updatesGenerator->newShortUrlVisitUpdate($visit)->willReturn($update); + $buildNewOrphanVisitUpdate = $this->updatesGenerator->newOrphanVisitUpdate($visit)->willReturn($update); $buildNewVisitUpdate = $this->updatesGenerator->newVisitUpdate($visit)->willReturn($update); $publish = $this->publisher->__invoke($update); @@ -94,6 +95,7 @@ class NotifyVisitToMercureTest extends TestCase $logDebug->shouldNotHaveBeenCalled(); $buildNewShortUrlVisitUpdate->shouldHaveBeenCalledOnce(); $buildNewVisitUpdate->shouldHaveBeenCalledOnce(); + $buildNewOrphanVisitUpdate->shouldNotHaveBeenCalled(); $publish->shouldHaveBeenCalledTimes(2); } @@ -101,7 +103,7 @@ class NotifyVisitToMercureTest extends TestCase public function debugIsLoggedWhenExceptionIsThrown(): void { $visitId = '123'; - $visit = new Visit(ShortUrl::createEmpty(), Visitor::emptyInstance()); + $visit = Visit::forValidShortUrl(ShortUrl::createEmpty(), Visitor::emptyInstance()); $update = new Update('', ''); $e = new RuntimeException('Error'); @@ -111,6 +113,7 @@ class NotifyVisitToMercureTest extends TestCase 'e' => $e, ]); $buildNewShortUrlVisitUpdate = $this->updatesGenerator->newShortUrlVisitUpdate($visit)->willReturn($update); + $buildNewOrphanVisitUpdate = $this->updatesGenerator->newOrphanVisitUpdate($visit)->willReturn($update); $buildNewVisitUpdate = $this->updatesGenerator->newVisitUpdate($visit)->willReturn($update); $publish = $this->publisher->__invoke($update)->willThrow($e); @@ -120,7 +123,45 @@ class NotifyVisitToMercureTest extends TestCase $logWarning->shouldNotHaveBeenCalled(); $logDebug->shouldHaveBeenCalledOnce(); $buildNewShortUrlVisitUpdate->shouldHaveBeenCalledOnce(); - $buildNewVisitUpdate->shouldNotHaveBeenCalled(); + $buildNewVisitUpdate->shouldHaveBeenCalledOnce(); + $buildNewOrphanVisitUpdate->shouldNotHaveBeenCalled(); $publish->shouldHaveBeenCalledOnce(); } + + /** + * @test + * @dataProvider provideOrphanVisits + */ + public function notificationsAreSentForOrphanVisits(Visit $visit): void + { + $visitId = '123'; + $update = new Update('', ''); + + $findVisit = $this->em->find(Visit::class, $visitId)->willReturn($visit); + $logWarning = $this->logger->warning(Argument::cetera()); + $logDebug = $this->logger->debug(Argument::cetera()); + $buildNewShortUrlVisitUpdate = $this->updatesGenerator->newShortUrlVisitUpdate($visit)->willReturn($update); + $buildNewOrphanVisitUpdate = $this->updatesGenerator->newOrphanVisitUpdate($visit)->willReturn($update); + $buildNewVisitUpdate = $this->updatesGenerator->newVisitUpdate($visit)->willReturn($update); + $publish = $this->publisher->__invoke($update); + + ($this->listener)(new VisitLocated($visitId)); + + $findVisit->shouldHaveBeenCalledOnce(); + $logWarning->shouldNotHaveBeenCalled(); + $logDebug->shouldNotHaveBeenCalled(); + $buildNewShortUrlVisitUpdate->shouldNotHaveBeenCalled(); + $buildNewVisitUpdate->shouldNotHaveBeenCalled(); + $buildNewOrphanVisitUpdate->shouldHaveBeenCalledOnce(); + $publish->shouldHaveBeenCalledOnce(); + } + + public function provideOrphanVisits(): iterable + { + $visitor = Visitor::emptyInstance(); + + yield Visit::TYPE_REGULAR_404 => [Visit::forRegularNotFound($visitor)]; + yield Visit::TYPE_INVALID_SHORT_URL => [Visit::forInvalidShortUrl($visitor)]; + yield Visit::TYPE_BASE_URL => [Visit::forBasePath($visitor)]; + } } diff --git a/module/Core/test/EventDispatcher/NotifyVisitToWebHooksTest.php b/module/Core/test/EventDispatcher/NotifyVisitToWebHooksTest.php index 9599c2c8..fcd97d2d 100644 --- a/module/Core/test/EventDispatcher/NotifyVisitToWebHooksTest.php +++ b/module/Core/test/EventDispatcher/NotifyVisitToWebHooksTest.php @@ -82,7 +82,7 @@ class NotifyVisitToWebHooksTest extends TestCase $invalidWebhooks = ['invalid', 'baz']; $find = $this->em->find(Visit::class, '1')->willReturn( - new Visit(ShortUrl::createEmpty(), Visitor::emptyInstance()), + Visit::forValidShortUrl(ShortUrl::createEmpty(), Visitor::emptyInstance()), ); $requestAsync = $this->httpClient->requestAsync( RequestMethodInterface::METHOD_POST, diff --git a/module/Core/test/Mercure/MercureUpdatesGeneratorTest.php b/module/Core/test/Mercure/MercureUpdatesGeneratorTest.php index 435fb4d8..b4361ca5 100644 --- a/module/Core/test/Mercure/MercureUpdatesGeneratorTest.php +++ b/module/Core/test/Mercure/MercureUpdatesGeneratorTest.php @@ -12,6 +12,7 @@ use Shlinkio\Shlink\Core\Model\ShortUrlMeta; use Shlinkio\Shlink\Core\Model\Visitor; use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlStringifier; use Shlinkio\Shlink\Core\ShortUrl\Transformer\ShortUrlDataTransformer; +use Shlinkio\Shlink\Core\Visit\Transformer\OrphanVisitDataTransformer; use function Shlinkio\Shlink\Common\json_decode; @@ -21,7 +22,10 @@ class MercureUpdatesGeneratorTest extends TestCase public function setUp(): void { - $this->generator = new MercureUpdatesGenerator(new ShortUrlDataTransformer(new ShortUrlStringifier([]))); + $this->generator = new MercureUpdatesGenerator( + new ShortUrlDataTransformer(new ShortUrlStringifier([])), + new OrphanVisitDataTransformer(), + ); } /** @@ -35,7 +39,7 @@ class MercureUpdatesGeneratorTest extends TestCase 'longUrl' => '', 'title' => $title, ])); - $visit = new Visit($shortUrl, Visitor::emptyInstance()); + $visit = Visit::forValidShortUrl($shortUrl, Visitor::emptyInstance()); $update = $this->generator->{$method}($visit); @@ -70,4 +74,34 @@ class MercureUpdatesGeneratorTest extends TestCase yield 'newVisitUpdate' => ['newVisitUpdate', 'https://shlink.io/new-visit', 'the cool title']; yield 'newShortUrlVisitUpdate' => ['newShortUrlVisitUpdate', 'https://shlink.io/new-visit/foo', null]; } + + /** + * @test + * @dataProvider provideOrphanVisits + */ + public function orphanVisitIsProperlySerializedIntoUpdate(Visit $orphanVisit): void + { + $update = $this->generator->newOrphanVisitUpdate($orphanVisit); + + self::assertEquals(['https://shlink.io/new-orphan-visit'], $update->getTopics()); + self::assertEquals([ + 'visit' => [ + 'referer' => '', + 'userAgent' => '', + 'visitLocation' => null, + 'date' => $orphanVisit->getDate()->toAtomString(), + 'visitedUrl' => $orphanVisit->visitedUrl(), + 'type' => $orphanVisit->type(), + ], + ], json_decode($update->getData())); + } + + public function provideOrphanVisits(): iterable + { + $visitor = Visitor::emptyInstance(); + + yield Visit::TYPE_REGULAR_404 => [Visit::forRegularNotFound($visitor)]; + yield Visit::TYPE_INVALID_SHORT_URL => [Visit::forInvalidShortUrl($visitor)]; + yield Visit::TYPE_BASE_URL => [Visit::forBasePath($visitor)]; + } } diff --git a/module/Core/test/Model/VisitorTest.php b/module/Core/test/Model/VisitorTest.php index d52a6389..e1003056 100644 --- a/module/Core/test/Model/VisitorTest.php +++ b/module/Core/test/Model/VisitorTest.php @@ -31,7 +31,7 @@ class VisitorTest extends TestCase public function provideParams(): iterable { yield 'all values are bigger' => [ - [str_repeat('a', 1000), str_repeat('b', 2000), str_repeat('c', 500)], + [str_repeat('a', 1000), str_repeat('b', 2000), str_repeat('c', 500), ''], [ 'userAgent' => str_repeat('a', Visitor::USER_AGENT_MAX_LENGTH), 'referer' => str_repeat('b', Visitor::REFERER_MAX_LENGTH), @@ -39,7 +39,7 @@ class VisitorTest extends TestCase ], ]; yield 'some values are smaller' => [ - [str_repeat('a', 10), str_repeat('b', 2000), null], + [str_repeat('a', 10), str_repeat('b', 2000), null, ''], [ 'userAgent' => str_repeat('a', 10), 'referer' => str_repeat('b', Visitor::REFERER_MAX_LENGTH), @@ -51,6 +51,7 @@ class VisitorTest extends TestCase $userAgent = $this->generateRandomString(2000), $referer = $this->generateRandomString(50), null, + '', ], [ 'userAgent' => substr($userAgent, 0, Visitor::USER_AGENT_MAX_LENGTH), diff --git a/module/Core/test/Paginator/Adapter/OrphanVisitsPaginatorAdapterTest.php b/module/Core/test/Paginator/Adapter/OrphanVisitsPaginatorAdapterTest.php new file mode 100644 index 00000000..6b28aa68 --- /dev/null +++ b/module/Core/test/Paginator/Adapter/OrphanVisitsPaginatorAdapterTest.php @@ -0,0 +1,65 @@ +repo = $this->prophesize(VisitRepositoryInterface::class); + $this->params = VisitsParams::fromRawData([]); + $this->adapter = new OrphanVisitsPaginatorAdapter($this->repo->reveal(), $this->params); + } + + /** @test */ + public function countDelegatesToRepository(): void + { + $expectedCount = 5; + $repoCount = $this->repo->countOrphanVisits($this->params->getDateRange())->willReturn($expectedCount); + + $result = $this->adapter->getNbResults(); + + self::assertEquals($expectedCount, $result); + $repoCount->shouldHaveBeenCalledOnce(); + } + + /** + * @test + * @dataProvider provideLimitAndOffset + */ + public function getSliceDelegatesToRepository(int $limit, int $offset): void + { + $visitor = Visitor::emptyInstance(); + $list = [Visit::forRegularNotFound($visitor), Visit::forInvalidShortUrl($visitor)]; + $repoFind = $this->repo->findOrphanVisits($this->params->getDateRange(), $limit, $offset)->willReturn($list); + + $result = $this->adapter->getSlice($offset, $limit); + + self::assertEquals($list, $result); + $repoFind->shouldHaveBeenCalledOnce(); + } + + public function provideLimitAndOffset(): iterable + { + yield [1, 5]; + yield [10, 4]; + yield [30, 18]; + } +} diff --git a/module/Core/test/Service/ShortUrl/DeleteShortUrlServiceTest.php b/module/Core/test/Service/ShortUrl/DeleteShortUrlServiceTest.php index 9e2ca4ec..4c066848 100644 --- a/module/Core/test/Service/ShortUrl/DeleteShortUrlServiceTest.php +++ b/module/Core/test/Service/ShortUrl/DeleteShortUrlServiceTest.php @@ -34,7 +34,7 @@ class DeleteShortUrlServiceTest extends TestCase public function setUp(): void { $shortUrl = ShortUrl::createEmpty()->setVisits(new ArrayCollection( - map(range(0, 10), fn () => new Visit(ShortUrl::createEmpty(), Visitor::emptyInstance())), + map(range(0, 10), fn () => Visit::forValidShortUrl(ShortUrl::createEmpty(), Visitor::emptyInstance())), )); $this->shortCode = $shortUrl->getShortCode(); diff --git a/module/Core/test/Service/ShortUrl/ShortUrlResolverTest.php b/module/Core/test/Service/ShortUrl/ShortUrlResolverTest.php index 038fe457..cf2330b3 100644 --- a/module/Core/test/Service/ShortUrl/ShortUrlResolverTest.php +++ b/module/Core/test/Service/ShortUrl/ShortUrlResolverTest.php @@ -121,7 +121,7 @@ class ShortUrlResolverTest extends TestCase $shortUrl = ShortUrl::fromMeta(ShortUrlMeta::fromRawData(['maxVisits' => 3, 'longUrl' => ''])); $shortUrl->setVisits(new ArrayCollection(map( range(0, 4), - fn () => new Visit($shortUrl, Visitor::emptyInstance()), + fn () => Visit::forValidShortUrl($shortUrl, Visitor::emptyInstance()), ))); return $shortUrl; @@ -140,7 +140,7 @@ class ShortUrlResolverTest extends TestCase ])); $shortUrl->setVisits(new ArrayCollection(map( range(0, 4), - fn () => new Visit($shortUrl, Visitor::emptyInstance()), + fn () => Visit::forValidShortUrl($shortUrl, Visitor::emptyInstance()), ))); return $shortUrl; diff --git a/module/Core/test/Service/VisitsTrackerTest.php b/module/Core/test/Service/VisitsTrackerTest.php deleted file mode 100644 index 9b627a17..00000000 --- a/module/Core/test/Service/VisitsTrackerTest.php +++ /dev/null @@ -1,144 +0,0 @@ -em = $this->prophesize(EntityManager::class); - $this->eventDispatcher = $this->prophesize(EventDispatcherInterface::class); - - $this->visitsTracker = new VisitsTracker($this->em->reveal(), $this->eventDispatcher->reveal(), true); - } - - /** @test */ - public function trackPersistsVisit(): void - { - $shortCode = '123ABC'; - - $this->em->persist(Argument::that(fn (Visit $visit) => $visit->setId('1')))->shouldBeCalledOnce(); - $this->em->flush()->shouldBeCalledOnce(); - - $this->visitsTracker->track(ShortUrl::withLongUrl($shortCode), Visitor::emptyInstance()); - - $this->eventDispatcher->dispatch(Argument::type(ShortUrlVisited::class))->shouldHaveBeenCalled(); - } - - /** - * @test - * @dataProvider provideAdminApiKeys - */ - public function infoReturnsVisitsForCertainShortCode(?ApiKey $apiKey): void - { - $shortCode = '123ABC'; - $spec = $apiKey === null ? null : $apiKey->spec(); - $repo = $this->prophesize(ShortUrlRepositoryInterface::class); - $count = $repo->shortCodeIsInUse($shortCode, null, $spec)->willReturn(true); - $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal())->shouldBeCalledOnce(); - - $list = map(range(0, 1), fn () => new Visit(ShortUrl::createEmpty(), Visitor::emptyInstance())); - $repo2 = $this->prophesize(VisitRepository::class); - $repo2->findVisitsByShortCode($shortCode, null, Argument::type(DateRange::class), 1, 0, $spec)->willReturn( - $list, - ); - $repo2->countVisitsByShortCode($shortCode, null, Argument::type(DateRange::class), $spec)->willReturn(1); - $this->em->getRepository(Visit::class)->willReturn($repo2->reveal())->shouldBeCalledOnce(); - - $paginator = $this->visitsTracker->info(new ShortUrlIdentifier($shortCode), new VisitsParams(), $apiKey); - - self::assertEquals($list, ArrayUtils::iteratorToArray($paginator->getCurrentPageResults())); - $count->shouldHaveBeenCalledOnce(); - } - - /** @test */ - public function throwsExceptionWhenRequestingVisitsForInvalidShortCode(): void - { - $shortCode = '123ABC'; - $repo = $this->prophesize(ShortUrlRepositoryInterface::class); - $count = $repo->shortCodeIsInUse($shortCode, null, null)->willReturn(false); - $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal())->shouldBeCalledOnce(); - - $this->expectException(ShortUrlNotFoundException::class); - $count->shouldBeCalledOnce(); - - $this->visitsTracker->info(new ShortUrlIdentifier($shortCode), new VisitsParams()); - } - - /** @test */ - public function throwsExceptionWhenRequestingVisitsForInvalidTag(): void - { - $tag = 'foo'; - $apiKey = new ApiKey(); - $repo = $this->prophesize(TagRepository::class); - $tagExists = $repo->tagExists($tag, $apiKey)->willReturn(false); - $getRepo = $this->em->getRepository(Tag::class)->willReturn($repo->reveal()); - - $this->expectException(TagNotFoundException::class); - $tagExists->shouldBeCalledOnce(); - $getRepo->shouldBeCalledOnce(); - - $this->visitsTracker->visitsForTag($tag, new VisitsParams(), $apiKey); - } - - /** - * @test - * @dataProvider provideAdminApiKeys - */ - public function visitsForTagAreReturnedAsExpected(?ApiKey $apiKey): void - { - $tag = 'foo'; - $repo = $this->prophesize(TagRepository::class); - $tagExists = $repo->tagExists($tag, $apiKey)->willReturn(true); - $getRepo = $this->em->getRepository(Tag::class)->willReturn($repo->reveal()); - - $spec = $apiKey === null ? null : $apiKey->spec(); - $list = map(range(0, 1), fn () => new Visit(ShortUrl::createEmpty(), Visitor::emptyInstance())); - $repo2 = $this->prophesize(VisitRepository::class); - $repo2->findVisitsByTag($tag, Argument::type(DateRange::class), 1, 0, $spec)->willReturn($list); - $repo2->countVisitsByTag($tag, Argument::type(DateRange::class), $spec)->willReturn(1); - $this->em->getRepository(Visit::class)->willReturn($repo2->reveal())->shouldBeCalledOnce(); - - $paginator = $this->visitsTracker->visitsForTag($tag, new VisitsParams(), $apiKey); - - self::assertEquals($list, ArrayUtils::iteratorToArray($paginator->getCurrentPageResults())); - $tagExists->shouldHaveBeenCalledOnce(); - $getRepo->shouldHaveBeenCalledOnce(); - } -} diff --git a/module/Core/test/Visit/Transformer/OrphanVisitDataTransformerTest.php b/module/Core/test/Visit/Transformer/OrphanVisitDataTransformerTest.php new file mode 100644 index 00000000..cf36c052 --- /dev/null +++ b/module/Core/test/Visit/Transformer/OrphanVisitDataTransformerTest.php @@ -0,0 +1,82 @@ +transformer = new OrphanVisitDataTransformer(); + } + + /** + * @test + * @dataProvider provideVisits + */ + public function visitsAreParsedAsExpected(Visit $visit, array $expectedResult): void + { + $result = $this->transformer->transform($visit); + + self::assertEquals($expectedResult, $result); + } + + public function provideVisits(): iterable + { + yield 'base path visit' => [ + $visit = Visit::forBasePath(Visitor::emptyInstance()), + [ + 'referer' => '', + 'date' => $visit->getDate()->toAtomString(), + 'userAgent' => '', + 'visitLocation' => null, + 'visitedUrl' => '', + 'type' => Visit::TYPE_BASE_URL, + ], + ]; + yield 'invalid short url visit' => [ + $visit = Visit::forInvalidShortUrl(Visitor::fromRequest( + ServerRequestFactory::fromGlobals()->withHeader('User-Agent', 'foo') + ->withHeader('Referer', 'bar') + ->withUri(new Uri('https://example.com/foo')), + )), + [ + 'referer' => 'bar', + 'date' => $visit->getDate()->toAtomString(), + 'userAgent' => 'foo', + 'visitLocation' => null, + 'visitedUrl' => 'https://example.com/foo', + 'type' => Visit::TYPE_INVALID_SHORT_URL, + ], + ]; + yield 'regular 404 visit' => [ + $visit = Visit::forRegularNotFound( + Visitor::fromRequest( + ServerRequestFactory::fromGlobals()->withHeader('User-Agent', 'user-agent') + ->withHeader('Referer', 'referer') + ->withUri(new Uri('https://doma.in/foo/bar')), + ), + )->locate($location = new VisitLocation(Location::emptyInstance())), + [ + 'referer' => 'referer', + 'date' => $visit->getDate()->toAtomString(), + 'userAgent' => 'user-agent', + 'visitLocation' => $location, + 'visitedUrl' => 'https://doma.in/foo/bar', + 'type' => Visit::TYPE_REGULAR_404, + ], + ]; + } +} diff --git a/module/Core/test/Visit/VisitLocatorTest.php b/module/Core/test/Visit/VisitLocatorTest.php index 96caf968..c99d051b 100644 --- a/module/Core/test/Visit/VisitLocatorTest.php +++ b/module/Core/test/Visit/VisitLocatorTest.php @@ -57,7 +57,8 @@ class VisitLocatorTest extends TestCase ): void { $unlocatedVisits = map( range(1, 200), - fn (int $i) => new Visit(ShortUrl::withLongUrl(sprintf('short_code_%s', $i)), Visitor::emptyInstance()), + fn (int $i) => + Visit::forValidShortUrl(ShortUrl::withLongUrl(sprintf('short_code_%s', $i)), Visitor::emptyInstance()), ); $findVisits = $this->mockRepoMethod($expectedRepoMethodName)->willReturn($unlocatedVisits); @@ -107,7 +108,7 @@ class VisitLocatorTest extends TestCase bool $isNonLocatableAddress ): void { $unlocatedVisits = [ - new Visit(ShortUrl::withLongUrl('foo'), Visitor::emptyInstance()), + Visit::forValidShortUrl(ShortUrl::withLongUrl('foo'), Visitor::emptyInstance()), ]; $findVisits = $this->mockRepoMethod($expectedRepoMethodName)->willReturn($unlocatedVisits); diff --git a/module/Core/test/Visit/VisitsStatsHelperTest.php b/module/Core/test/Visit/VisitsStatsHelperTest.php index cdc76bd4..de2a3534 100644 --- a/module/Core/test/Visit/VisitsStatsHelperTest.php +++ b/module/Core/test/Visit/VisitsStatsHelperTest.php @@ -5,19 +5,35 @@ declare(strict_types=1); namespace ShlinkioTest\Shlink\Core\Visit; use Doctrine\ORM\EntityManagerInterface; +use Laminas\Stdlib\ArrayUtils; use PHPUnit\Framework\TestCase; +use Prophecy\Argument; use Prophecy\PhpUnit\ProphecyTrait; use Prophecy\Prophecy\ObjectProphecy; +use Shlinkio\Shlink\Common\Util\DateRange; +use Shlinkio\Shlink\Core\Entity\ShortUrl; +use Shlinkio\Shlink\Core\Entity\Tag; use Shlinkio\Shlink\Core\Entity\Visit; +use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; +use Shlinkio\Shlink\Core\Exception\TagNotFoundException; +use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; +use Shlinkio\Shlink\Core\Model\Visitor; +use Shlinkio\Shlink\Core\Model\VisitsParams; +use Shlinkio\Shlink\Core\Repository\ShortUrlRepositoryInterface; +use Shlinkio\Shlink\Core\Repository\TagRepository; use Shlinkio\Shlink\Core\Repository\VisitRepository; use Shlinkio\Shlink\Core\Visit\Model\VisitsStats; use Shlinkio\Shlink\Core\Visit\VisitsStatsHelper; +use Shlinkio\Shlink\Rest\Entity\ApiKey; +use ShlinkioTest\Shlink\Core\Util\ApiKeyHelpersTrait; +use function count; use function Functional\map; use function range; class VisitsStatsHelperTest extends TestCase { + use ApiKeyHelpersTrait; use ProphecyTrait; private VisitsStatsHelper $helper; @@ -36,13 +52,15 @@ class VisitsStatsHelperTest extends TestCase public function returnsExpectedVisitsStats(int $expectedCount): void { $repo = $this->prophesize(VisitRepository::class); - $count = $repo->countVisits(null)->willReturn($expectedCount); + $count = $repo->countVisits(null)->willReturn($expectedCount * 3); + $countOrphan = $repo->countOrphanVisits()->willReturn($expectedCount); $getRepo = $this->em->getRepository(Visit::class)->willReturn($repo->reveal()); $stats = $this->helper->getVisitsStats(); - self::assertEquals(new VisitsStats($expectedCount), $stats); + self::assertEquals(new VisitsStats($expectedCount * 3, $expectedCount), $stats); $count->shouldHaveBeenCalledOnce(); + $countOrphan->shouldHaveBeenCalledOnce(); $getRepo->shouldHaveBeenCalledOnce(); } @@ -50,4 +68,102 @@ class VisitsStatsHelperTest extends TestCase { return map(range(0, 50, 5), fn (int $value) => [$value]); } + + /** + * @test + * @dataProvider provideAdminApiKeys + */ + public function infoReturnsVisitsForCertainShortCode(?ApiKey $apiKey): void + { + $shortCode = '123ABC'; + $spec = $apiKey === null ? null : $apiKey->spec(); + $repo = $this->prophesize(ShortUrlRepositoryInterface::class); + $count = $repo->shortCodeIsInUse($shortCode, null, $spec)->willReturn(true); + $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal())->shouldBeCalledOnce(); + + $list = map(range(0, 1), fn () => Visit::forValidShortUrl(ShortUrl::createEmpty(), Visitor::emptyInstance())); + $repo2 = $this->prophesize(VisitRepository::class); + $repo2->findVisitsByShortCode($shortCode, null, Argument::type(DateRange::class), 1, 0, $spec)->willReturn( + $list, + ); + $repo2->countVisitsByShortCode($shortCode, null, Argument::type(DateRange::class), $spec)->willReturn(1); + $this->em->getRepository(Visit::class)->willReturn($repo2->reveal())->shouldBeCalledOnce(); + + $paginator = $this->helper->visitsForShortUrl(new ShortUrlIdentifier($shortCode), new VisitsParams(), $apiKey); + + self::assertEquals($list, ArrayUtils::iteratorToArray($paginator->getCurrentPageResults())); + $count->shouldHaveBeenCalledOnce(); + } + + /** @test */ + public function throwsExceptionWhenRequestingVisitsForInvalidShortCode(): void + { + $shortCode = '123ABC'; + $repo = $this->prophesize(ShortUrlRepositoryInterface::class); + $count = $repo->shortCodeIsInUse($shortCode, null, null)->willReturn(false); + $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal())->shouldBeCalledOnce(); + + $this->expectException(ShortUrlNotFoundException::class); + $count->shouldBeCalledOnce(); + + $this->helper->visitsForShortUrl(new ShortUrlIdentifier($shortCode), new VisitsParams()); + } + + /** @test */ + public function throwsExceptionWhenRequestingVisitsForInvalidTag(): void + { + $tag = 'foo'; + $apiKey = new ApiKey(); + $repo = $this->prophesize(TagRepository::class); + $tagExists = $repo->tagExists($tag, $apiKey)->willReturn(false); + $getRepo = $this->em->getRepository(Tag::class)->willReturn($repo->reveal()); + + $this->expectException(TagNotFoundException::class); + $tagExists->shouldBeCalledOnce(); + $getRepo->shouldBeCalledOnce(); + + $this->helper->visitsForTag($tag, new VisitsParams(), $apiKey); + } + + /** + * @test + * @dataProvider provideAdminApiKeys + */ + public function visitsForTagAreReturnedAsExpected(?ApiKey $apiKey): void + { + $tag = 'foo'; + $repo = $this->prophesize(TagRepository::class); + $tagExists = $repo->tagExists($tag, $apiKey)->willReturn(true); + $getRepo = $this->em->getRepository(Tag::class)->willReturn($repo->reveal()); + + $spec = $apiKey === null ? null : $apiKey->spec(); + $list = map(range(0, 1), fn () => Visit::forValidShortUrl(ShortUrl::createEmpty(), Visitor::emptyInstance())); + $repo2 = $this->prophesize(VisitRepository::class); + $repo2->findVisitsByTag($tag, Argument::type(DateRange::class), 1, 0, $spec)->willReturn($list); + $repo2->countVisitsByTag($tag, Argument::type(DateRange::class), $spec)->willReturn(1); + $this->em->getRepository(Visit::class)->willReturn($repo2->reveal())->shouldBeCalledOnce(); + + $paginator = $this->helper->visitsForTag($tag, new VisitsParams(), $apiKey); + + self::assertEquals($list, ArrayUtils::iteratorToArray($paginator->getCurrentPageResults())); + $tagExists->shouldHaveBeenCalledOnce(); + $getRepo->shouldHaveBeenCalledOnce(); + } + + /** @test */ + public function orphanVisitsAreReturnedAsExpected(): void + { + $list = map(range(0, 3), fn () => Visit::forBasePath(Visitor::emptyInstance())); + $repo = $this->prophesize(VisitRepository::class); + $countVisits = $repo->countOrphanVisits(Argument::type(DateRange::class))->willReturn(count($list)); + $listVisits = $repo->findOrphanVisits(Argument::type(DateRange::class), Argument::cetera())->willReturn($list); + $getRepo = $this->em->getRepository(Visit::class)->willReturn($repo->reveal()); + + $paginator = $this->helper->orphanVisits(new VisitsParams()); + + self::assertEquals($list, ArrayUtils::iteratorToArray($paginator->getCurrentPageResults())); + $listVisits->shouldHaveBeenCalledOnce(); + $countVisits->shouldHaveBeenCalledOnce(); + $getRepo->shouldHaveBeenCalledOnce(); + } } diff --git a/module/Core/test/Visit/VisitsTrackerTest.php b/module/Core/test/Visit/VisitsTrackerTest.php new file mode 100644 index 00000000..118ebc06 --- /dev/null +++ b/module/Core/test/Visit/VisitsTrackerTest.php @@ -0,0 +1,81 @@ +em = $this->prophesize(EntityManager::class); + $this->eventDispatcher = $this->prophesize(EventDispatcherInterface::class); + $this->options = new UrlShortenerOptions(); + + $this->visitsTracker = new VisitsTracker($this->em->reveal(), $this->eventDispatcher->reveal(), $this->options); + } + + /** + * @test + * @dataProvider provideTrackingMethodNames + */ + public function trackPersistsVisitAndDispatchesEvent(string $method, array $args): void + { + $this->em->persist(Argument::that(fn (Visit $visit) => $visit->setId('1')))->shouldBeCalledOnce(); + $this->em->flush()->shouldBeCalledOnce(); + + $this->visitsTracker->{$method}(...$args); + + $this->eventDispatcher->dispatch(Argument::type(UrlVisited::class))->shouldHaveBeenCalled(); + } + + public function provideTrackingMethodNames(): iterable + { + yield 'track' => ['track', [ShortUrl::createEmpty(), Visitor::emptyInstance()]]; + yield 'trackInvalidShortUrlVisit' => ['trackInvalidShortUrlVisit', [Visitor::emptyInstance()]]; + yield 'trackBaseUrlVisit' => ['trackBaseUrlVisit', [Visitor::emptyInstance()]]; + yield 'trackRegularNotFoundVisit' => ['trackRegularNotFoundVisit', [Visitor::emptyInstance()]]; + } + + /** + * @test + * @dataProvider provideOrphanTrackingMethodNames + */ + public function orphanVisitsAreNotTrackedWhenDisabled(string $method): void + { + $this->options->trackOrphanVisits = false; + + $this->visitsTracker->{$method}(Visitor::emptyInstance()); + + $this->eventDispatcher->dispatch(Argument::cetera())->shouldNotHaveBeenCalled(); + $this->em->persist(Argument::cetera())->shouldNotHaveBeenCalled(); + $this->em->flush()->shouldNotHaveBeenCalled(); + } + + public function provideOrphanTrackingMethodNames(): iterable + { + yield 'trackInvalidShortUrlVisit' => ['trackInvalidShortUrlVisit']; + yield 'trackBaseUrlVisit' => ['trackBaseUrlVisit']; + yield 'trackRegularNotFoundVisit' => ['trackRegularNotFoundVisit']; + } +} diff --git a/module/Rest/config/dependencies.config.php b/module/Rest/config/dependencies.config.php index cfb97320..e1a869df 100644 --- a/module/Rest/config/dependencies.config.php +++ b/module/Rest/config/dependencies.config.php @@ -34,6 +34,7 @@ return [ Action\Visit\ShortUrlVisitsAction::class => ConfigAbstractFactory::class, Action\Visit\TagVisitsAction::class => ConfigAbstractFactory::class, Action\Visit\GlobalVisitsAction::class => ConfigAbstractFactory::class, + Action\Visit\OrphanVisitsAction::class => ConfigAbstractFactory::class, Action\Tag\ListTagsAction::class => ConfigAbstractFactory::class, Action\Tag\DeleteTagsAction::class => ConfigAbstractFactory::class, Action\Tag\CreateTagsAction::class => ConfigAbstractFactory::class, @@ -66,9 +67,13 @@ return [ Service\ShortUrl\ShortUrlResolver::class, ShortUrlDataTransformer::class, ], - Action\Visit\ShortUrlVisitsAction::class => [Service\VisitsTracker::class], - Action\Visit\TagVisitsAction::class => [Service\VisitsTracker::class], + Action\Visit\ShortUrlVisitsAction::class => [Visit\VisitsStatsHelper::class], + Action\Visit\TagVisitsAction::class => [Visit\VisitsStatsHelper::class], Action\Visit\GlobalVisitsAction::class => [Visit\VisitsStatsHelper::class], + Action\Visit\OrphanVisitsAction::class => [ + Visit\VisitsStatsHelper::class, + Visit\Transformer\OrphanVisitDataTransformer::class, + ], Action\ShortUrl\ListShortUrlsAction::class => [Service\ShortUrlService::class, ShortUrlDataTransformer::class], Action\ShortUrl\EditShortUrlTagsAction::class => [Service\ShortUrlService::class], Action\Tag\ListTagsAction::class => [TagService::class], diff --git a/module/Rest/config/routes.config.php b/module/Rest/config/routes.config.php index a5382c38..9b09a266 100644 --- a/module/Rest/config/routes.config.php +++ b/module/Rest/config/routes.config.php @@ -34,6 +34,7 @@ return [ Action\Visit\ShortUrlVisitsAction::getRouteDef([$dropDomainMiddleware]), Action\Visit\TagVisitsAction::getRouteDef(), Action\Visit\GlobalVisitsAction::getRouteDef(), + Action\Visit\OrphanVisitsAction::getRouteDef(), // Tags Action\Tag\ListTagsAction::getRouteDef(), diff --git a/module/Rest/src/Action/Visit/OrphanVisitsAction.php b/module/Rest/src/Action/Visit/OrphanVisitsAction.php new file mode 100644 index 00000000..7a65b920 --- /dev/null +++ b/module/Rest/src/Action/Visit/OrphanVisitsAction.php @@ -0,0 +1,43 @@ +visitsHelper = $visitsHelper; + $this->orphanVisitTransformer = $orphanVisitTransformer; + } + + public function handle(ServerRequestInterface $request): ResponseInterface + { + $params = VisitsParams::fromRawData($request->getQueryParams()); + $visits = $this->visitsHelper->orphanVisits($params); + + return new JsonResponse([ + 'visits' => $this->serializePaginator($visits, $this->orphanVisitTransformer), + ]); + } +} diff --git a/module/Rest/src/Action/Visit/ShortUrlVisitsAction.php b/module/Rest/src/Action/Visit/ShortUrlVisitsAction.php index 7b7c1055..8175d1c7 100644 --- a/module/Rest/src/Action/Visit/ShortUrlVisitsAction.php +++ b/module/Rest/src/Action/Visit/ShortUrlVisitsAction.php @@ -10,7 +10,7 @@ use Psr\Http\Message\ServerRequestInterface as Request; use Shlinkio\Shlink\Common\Paginator\Util\PagerfantaUtilsTrait; use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Model\VisitsParams; -use Shlinkio\Shlink\Core\Service\VisitsTrackerInterface; +use Shlinkio\Shlink\Core\Visit\VisitsStatsHelperInterface; use Shlinkio\Shlink\Rest\Action\AbstractRestAction; use Shlinkio\Shlink\Rest\Middleware\AuthenticationMiddleware; @@ -21,11 +21,11 @@ class ShortUrlVisitsAction extends AbstractRestAction protected const ROUTE_PATH = '/short-urls/{shortCode}/visits'; protected const ROUTE_ALLOWED_METHODS = [self::METHOD_GET]; - private VisitsTrackerInterface $visitsTracker; + private VisitsStatsHelperInterface $visitsHelper; - public function __construct(VisitsTrackerInterface $visitsTracker) + public function __construct(VisitsStatsHelperInterface $visitsHelper) { - $this->visitsTracker = $visitsTracker; + $this->visitsHelper = $visitsHelper; } public function handle(Request $request): Response @@ -33,7 +33,7 @@ class ShortUrlVisitsAction extends AbstractRestAction $identifier = ShortUrlIdentifier::fromApiRequest($request); $params = VisitsParams::fromRawData($request->getQueryParams()); $apiKey = AuthenticationMiddleware::apiKeyFromRequest($request); - $visits = $this->visitsTracker->info($identifier, $params, $apiKey); + $visits = $this->visitsHelper->visitsForShortUrl($identifier, $params, $apiKey); return new JsonResponse([ 'visits' => $this->serializePaginator($visits), diff --git a/module/Rest/src/Action/Visit/TagVisitsAction.php b/module/Rest/src/Action/Visit/TagVisitsAction.php index aec42ebb..8d981c82 100644 --- a/module/Rest/src/Action/Visit/TagVisitsAction.php +++ b/module/Rest/src/Action/Visit/TagVisitsAction.php @@ -9,7 +9,7 @@ use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface as Request; use Shlinkio\Shlink\Common\Paginator\Util\PagerfantaUtilsTrait; use Shlinkio\Shlink\Core\Model\VisitsParams; -use Shlinkio\Shlink\Core\Service\VisitsTrackerInterface; +use Shlinkio\Shlink\Core\Visit\VisitsStatsHelperInterface; use Shlinkio\Shlink\Rest\Action\AbstractRestAction; use Shlinkio\Shlink\Rest\Middleware\AuthenticationMiddleware; @@ -20,11 +20,11 @@ class TagVisitsAction extends AbstractRestAction protected const ROUTE_PATH = '/tags/{tag}/visits'; protected const ROUTE_ALLOWED_METHODS = [self::METHOD_GET]; - private VisitsTrackerInterface $visitsTracker; + private VisitsStatsHelperInterface $visitsHelper; - public function __construct(VisitsTrackerInterface $visitsTracker) + public function __construct(VisitsStatsHelperInterface $visitsHelper) { - $this->visitsTracker = $visitsTracker; + $this->visitsHelper = $visitsHelper; } public function handle(Request $request): Response @@ -32,7 +32,7 @@ class TagVisitsAction extends AbstractRestAction $tag = $request->getAttribute('tag', ''); $params = VisitsParams::fromRawData($request->getQueryParams()); $apiKey = AuthenticationMiddleware::apiKeyFromRequest($request); - $visits = $this->visitsTracker->visitsForTag($tag, $params, $apiKey); + $visits = $this->visitsHelper->visitsForTag($tag, $params, $apiKey); return new JsonResponse([ 'visits' => $this->serializePaginator($visits), diff --git a/module/Rest/test-api/Action/GlobalVisitsTest.php b/module/Rest/test-api/Action/GlobalVisitsTest.php index 99e05918..1b71f976 100644 --- a/module/Rest/test-api/Action/GlobalVisitsTest.php +++ b/module/Rest/test-api/Action/GlobalVisitsTest.php @@ -19,7 +19,9 @@ class GlobalVisitsTest extends ApiTestCase self::assertArrayHasKey('visits', $payload); self::assertArrayHasKey('visitsCount', $payload['visits']); + self::assertArrayHasKey('orphanVisitsCount', $payload['visits']); self::assertEquals($expectedVisits, $payload['visits']['visitsCount']); + self::assertEquals(3, $payload['visits']['orphanVisitsCount']); } public function provideApiKeys(): iterable diff --git a/module/Rest/test-api/Action/OrphanVisitsTest.php b/module/Rest/test-api/Action/OrphanVisitsTest.php new file mode 100644 index 00000000..ea890f9f --- /dev/null +++ b/module/Rest/test-api/Action/OrphanVisitsTest.php @@ -0,0 +1,59 @@ + 'https://doma.in/foo', + 'date' => '2020-03-01T00:00:00+00:00', + 'userAgent' => 'shlink-tests-agent', + 'visitLocation' => null, + 'visitedUrl' => 'foo.com', + 'type' => 'invalid_short_url', + + ]; + private const REGULAR_NOT_FOUND = [ + 'referer' => 'https://doma.in/foo/bar', + 'date' => '2020-02-01T00:00:00+00:00', + 'userAgent' => 'shlink-tests-agent', + 'visitLocation' => null, + 'visitedUrl' => '', + 'type' => 'regular_404', + ]; + private const BASE_URL = [ + 'referer' => 'https://doma.in', + 'date' => '2020-01-01T00:00:00+00:00', + 'userAgent' => 'shlink-tests-agent', + 'visitLocation' => null, + 'visitedUrl' => '', + 'type' => 'base_url', + ]; + + /** + * @test + * @dataProvider provideQueries + */ + public function properVisitsAreReturnedBasedInQuery(array $query, 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::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]]; + } +} diff --git a/module/Rest/test-api/Fixtures/VisitsFixture.php b/module/Rest/test-api/Fixtures/VisitsFixture.php index 73601748..412c79d5 100644 --- a/module/Rest/test-api/Fixtures/VisitsFixture.php +++ b/module/Rest/test-api/Fixtures/VisitsFixture.php @@ -4,9 +4,11 @@ declare(strict_types=1); namespace ShlinkioApiTest\Shlink\Rest\Fixtures; +use Cake\Chronos\Chronos; use Doctrine\Common\DataFixtures\AbstractFixture; use Doctrine\Common\DataFixtures\DependentFixtureInterface; use Doctrine\Persistence\ObjectManager; +use ReflectionObject; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Entity\Visit; use Shlinkio\Shlink\Core\Model\Visitor; @@ -22,20 +24,54 @@ class VisitsFixture extends AbstractFixture implements DependentFixtureInterface { /** @var ShortUrl $abcShortUrl */ $abcShortUrl = $this->getReference('abc123_short_url'); - $manager->persist(new Visit($abcShortUrl, new Visitor('shlink-tests-agent', '', '44.55.66.77'))); - $manager->persist(new Visit($abcShortUrl, new Visitor('shlink-tests-agent', 'https://google.com', '4.5.6.7'))); - $manager->persist(new Visit($abcShortUrl, new Visitor('shlink-tests-agent', '', '1.2.3.4'))); + $manager->persist( + Visit::forValidShortUrl($abcShortUrl, new Visitor('shlink-tests-agent', '', '44.55.66.77', '')), + ); + $manager->persist(Visit::forValidShortUrl( + $abcShortUrl, + new Visitor('shlink-tests-agent', 'https://google.com', '4.5.6.7', ''), + )); + $manager->persist(Visit::forValidShortUrl($abcShortUrl, new Visitor('shlink-tests-agent', '', '1.2.3.4', ''))); /** @var ShortUrl $defShortUrl */ $defShortUrl = $this->getReference('def456_short_url'); - $manager->persist(new Visit($defShortUrl, new Visitor('shlink-tests-agent', '', '127.0.0.1'))); - $manager->persist(new Visit($defShortUrl, new Visitor('shlink-tests-agent', 'https://app.shlink.io', ''))); + $manager->persist( + Visit::forValidShortUrl($defShortUrl, new Visitor('shlink-tests-agent', '', '127.0.0.1', '')), + ); + $manager->persist( + Visit::forValidShortUrl($defShortUrl, new Visitor('shlink-tests-agent', 'https://app.shlink.io', '', '')), + ); /** @var ShortUrl $ghiShortUrl */ $ghiShortUrl = $this->getReference('ghi789_short_url'); - $manager->persist(new Visit($ghiShortUrl, new Visitor('shlink-tests-agent', '', '1.2.3.4'))); - $manager->persist(new Visit($ghiShortUrl, new Visitor('shlink-tests-agent', 'https://app.shlink.io', ''))); + $manager->persist(Visit::forValidShortUrl($ghiShortUrl, new Visitor('shlink-tests-agent', '', '1.2.3.4', ''))); + $manager->persist( + Visit::forValidShortUrl($ghiShortUrl, new Visitor('shlink-tests-agent', 'https://app.shlink.io', '', '')), + ); + + $manager->persist($this->setVisitDate( + Visit::forBasePath(new Visitor('shlink-tests-agent', 'https://doma.in', '1.2.3.4', '')), + '2020-01-01', + )); + $manager->persist($this->setVisitDate( + Visit::forRegularNotFound(new Visitor('shlink-tests-agent', 'https://doma.in/foo/bar', '1.2.3.4', '')), + '2020-02-01', + )); + $manager->persist($this->setVisitDate( + Visit::forInvalidShortUrl(new Visitor('shlink-tests-agent', 'https://doma.in/foo', '1.2.3.4', 'foo.com')), + '2020-03-01', + )); $manager->flush(); } + + private function setVisitDate(Visit $visit, string $date): Visit + { + $ref = new ReflectionObject($visit); + $dateProp = $ref->getProperty('date'); + $dateProp->setAccessible(true); + $dateProp->setValue($visit, Chronos::parse($date)); + + return $visit; + } } diff --git a/module/Rest/test/Action/Visit/GlobalVisitsActionTest.php b/module/Rest/test/Action/Visit/GlobalVisitsActionTest.php index 6e3ab1e4..d53cb20d 100644 --- a/module/Rest/test/Action/Visit/GlobalVisitsActionTest.php +++ b/module/Rest/test/Action/Visit/GlobalVisitsActionTest.php @@ -31,7 +31,7 @@ class GlobalVisitsActionTest extends TestCase public function statsAreReturnedFromHelper(): void { $apiKey = new ApiKey(); - $stats = new VisitsStats(5); + $stats = new VisitsStats(5, 3); $getStats = $this->helper->getVisitsStats($apiKey)->willReturn($stats); /** @var JsonResponse $resp */ diff --git a/module/Rest/test/Action/Visit/OrphanVisitsActionTest.php b/module/Rest/test/Action/Visit/OrphanVisitsActionTest.php new file mode 100644 index 00000000..9fec7e1f --- /dev/null +++ b/module/Rest/test/Action/Visit/OrphanVisitsActionTest.php @@ -0,0 +1,57 @@ +visitsHelper = $this->prophesize(VisitsStatsHelperInterface::class); + $this->orphanVisitTransformer = $this->prophesize(DataTransformerInterface::class); + + $this->action = new OrphanVisitsAction($this->visitsHelper->reveal(), $this->orphanVisitTransformer->reveal()); + } + + /** @test */ + public function requestIsHandled(): void + { + $visitor = Visitor::emptyInstance(); + $visits = [Visit::forInvalidShortUrl($visitor), Visit::forRegularNotFound($visitor)]; + $orphanVisits = $this->visitsHelper->orphanVisits(Argument::type(VisitsParams::class))->willReturn( + new Paginator(new ArrayAdapter($visits)), + ); + $transform = $this->orphanVisitTransformer->transform(Argument::type(Visit::class))->willReturn([]); + + $response = $this->action->handle(ServerRequestFactory::fromGlobals()); + + self::assertInstanceOf(JsonResponse::class, $response); + self::assertEquals(200, $response->getStatusCode()); + $orphanVisits->shouldHaveBeenCalledOnce(); + $transform->shouldHaveBeenCalledTimes(count($visits)); + } +} diff --git a/module/Rest/test/Action/Visit/ShortUrlVisitsActionTest.php b/module/Rest/test/Action/Visit/ShortUrlVisitsActionTest.php index 9c751214..6b149877 100644 --- a/module/Rest/test/Action/Visit/ShortUrlVisitsActionTest.php +++ b/module/Rest/test/Action/Visit/ShortUrlVisitsActionTest.php @@ -16,7 +16,7 @@ use Shlinkio\Shlink\Common\Paginator\Paginator; use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Model\VisitsParams; -use Shlinkio\Shlink\Core\Service\VisitsTracker; +use Shlinkio\Shlink\Core\Visit\VisitsStatsHelperInterface; use Shlinkio\Shlink\Rest\Action\Visit\ShortUrlVisitsAction; use Shlinkio\Shlink\Rest\Entity\ApiKey; @@ -25,19 +25,19 @@ class ShortUrlVisitsActionTest extends TestCase use ProphecyTrait; private ShortUrlVisitsAction $action; - private ObjectProphecy $visitsTracker; + private ObjectProphecy $visitsHelper; public function setUp(): void { - $this->visitsTracker = $this->prophesize(VisitsTracker::class); - $this->action = new ShortUrlVisitsAction($this->visitsTracker->reveal()); + $this->visitsHelper = $this->prophesize(VisitsStatsHelperInterface::class); + $this->action = new ShortUrlVisitsAction($this->visitsHelper->reveal()); } /** @test */ public function providingCorrectShortCodeReturnsVisits(): void { $shortCode = 'abc123'; - $this->visitsTracker->info( + $this->visitsHelper->visitsForShortUrl( new ShortUrlIdentifier($shortCode), Argument::type(VisitsParams::class), Argument::type(ApiKey::class), @@ -52,7 +52,7 @@ class ShortUrlVisitsActionTest extends TestCase public function paramsAreReadFromQuery(): void { $shortCode = 'abc123'; - $this->visitsTracker->info(new ShortUrlIdentifier($shortCode), new VisitsParams( + $this->visitsHelper->visitsForShortUrl(new ShortUrlIdentifier($shortCode), new VisitsParams( new DateRange(null, Chronos::parse('2016-01-01 00:00:00')), 3, 10, diff --git a/module/Rest/test/Action/Visit/TagVisitsActionTest.php b/module/Rest/test/Action/Visit/TagVisitsActionTest.php index c9097d07..da046f26 100644 --- a/module/Rest/test/Action/Visit/TagVisitsActionTest.php +++ b/module/Rest/test/Action/Visit/TagVisitsActionTest.php @@ -12,7 +12,7 @@ use Prophecy\PhpUnit\ProphecyTrait; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\Common\Paginator\Paginator; use Shlinkio\Shlink\Core\Model\VisitsParams; -use Shlinkio\Shlink\Core\Service\VisitsTracker; +use Shlinkio\Shlink\Core\Visit\VisitsStatsHelperInterface; use Shlinkio\Shlink\Rest\Action\Visit\TagVisitsAction; use Shlinkio\Shlink\Rest\Entity\ApiKey; @@ -21,12 +21,12 @@ class TagVisitsActionTest extends TestCase use ProphecyTrait; private TagVisitsAction $action; - private ObjectProphecy $visitsTracker; + private ObjectProphecy $visitsHelper; protected function setUp(): void { - $this->visitsTracker = $this->prophesize(VisitsTracker::class); - $this->action = new TagVisitsAction($this->visitsTracker->reveal()); + $this->visitsHelper = $this->prophesize(VisitsStatsHelperInterface::class); + $this->action = new TagVisitsAction($this->visitsHelper->reveal()); } /** @test */ @@ -34,7 +34,7 @@ class TagVisitsActionTest extends TestCase { $tag = 'foo'; $apiKey = new ApiKey(); - $getVisits = $this->visitsTracker->visitsForTag($tag, Argument::type(VisitsParams::class), $apiKey)->willReturn( + $getVisits = $this->visitsHelper->visitsForTag($tag, Argument::type(VisitsParams::class), $apiKey)->willReturn( new Paginator(new ArrayAdapter([])), );