Merge pull request #1782 from acelaya-forks/feature/clear-orphan-visits

Feature/clear orphan visits
This commit is contained in:
Alejandro Celaya 2023-05-18 09:49:31 +02:00 committed by GitHub
commit fb31e2a5e4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
23 changed files with 429 additions and 21 deletions

View File

@ -8,7 +8,13 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this
### Added
* [#1148](https://github.com/shlinkio/shlink/issues/1148) Add support to delete short URL visits.
This can be done via `DELETE /short-urls/{shortCode}/visits` REST endpoint or via `short-url:delete-visits` console command.
This can be done via `DELETE /short-urls/{shortCode}/visits` REST endpoint or via `short-url:visits-delete` console command.
The CLI command includes a warning and requires the user to confirm before proceeding.
* [#1681](https://github.com/shlinkio/shlink/issues/1681) Add support to delete orphan visits.
This can be done via `DELETE /visits/orphan` REST endpoint or via `visit:orphan-delete` console command.
The CLI command includes a warning and requires the user to confirm before proceeding.

View File

@ -65,7 +65,7 @@
"require-dev": {
"cebe/php-openapi": "^1.7",
"devster/ubench": "^2.1",
"infection/infection": "^0.26.19",
"infection/infection": "^0.27",
"openswoole/ide-helper": "~22.0.0",
"phpstan/phpstan": "^1.9",
"phpstan/phpstan-doctrine": "^1.3",

View File

@ -38,6 +38,7 @@ return (static function (): array {
Action\Visit\DomainVisitsAction::getRouteDef(),
Action\Visit\GlobalVisitsAction::getRouteDef(),
Action\Visit\OrphanVisitsAction::getRouteDef(),
Action\Visit\DeleteOrphanVisitsAction::getRouteDef(),
Action\Visit\NonOrphanVisitsAction::getRouteDef(),
// Short URLs

View File

@ -148,5 +148,55 @@
}
}
}
},
"delete": {
"operationId": "deleteOrphanVisits",
"tags": [
"Visits"
],
"summary": "Delete orphan visits",
"description": "Delete all visits to invalid short URLs, the base URL or any other 404.",
"parameters": [
{
"$ref": "../parameters/version.json"
}
],
"security": [
{
"ApiKey": []
}
],
"responses": {
"200": {
"description": "Deleted visits",
"content": {
"application/json": {
"schema": {
"type": "object",
"properties": {
"deletedVisits": {
"description": "Amount of affected visits",
"type": "number"
}
}
},
"example": {
"deletedVisits": 536
}
}
}
},
"default": {
"description": "Unexpected error.",
"content": {
"application/problem+json": {
"schema": {
"$ref": "../definitions/Error.json"
}
}
}
}
}
}
}

View File

@ -18,6 +18,7 @@ return [
Command\Visit\LocateVisitsCommand::NAME => Command\Visit\LocateVisitsCommand::class,
Command\Visit\DownloadGeoLiteDbCommand::NAME => Command\Visit\DownloadGeoLiteDbCommand::class,
Command\Visit\GetOrphanVisitsCommand::NAME => Command\Visit\GetOrphanVisitsCommand::class,
Command\Visit\DeleteOrphanVisitsCommand::NAME => Command\Visit\DeleteOrphanVisitsCommand::class,
Command\Visit\GetNonOrphanVisitsCommand::NAME => Command\Visit\GetNonOrphanVisitsCommand::class,
Command\Api\GenerateKeyCommand::NAME => Command\Api\GenerateKeyCommand::class,

View File

@ -47,6 +47,7 @@ return [
Command\Visit\DownloadGeoLiteDbCommand::class => ConfigAbstractFactory::class,
Command\Visit\LocateVisitsCommand::class => ConfigAbstractFactory::class,
Command\Visit\GetOrphanVisitsCommand::class => ConfigAbstractFactory::class,
Command\Visit\DeleteOrphanVisitsCommand::class => ConfigAbstractFactory::class,
Command\Visit\GetNonOrphanVisitsCommand::class => ConfigAbstractFactory::class,
Command\Api\GenerateKeyCommand::class => ConfigAbstractFactory::class,
@ -98,6 +99,7 @@ return [
LockFactory::class,
],
Command\Visit\GetOrphanVisitsCommand::class => [Visit\VisitsStatsHelper::class],
Command\Visit\DeleteOrphanVisitsCommand::class => [Visit\VisitsDeleter::class],
Command\Visit\GetNonOrphanVisitsCommand::class => [Visit\VisitsStatsHelper::class, ShortUrlStringifier::class],
Command\Api\GenerateKeyCommand::class => [ApiKeyService::class, ApiKey\RoleResolver::class],

View File

@ -4,22 +4,21 @@ declare(strict_types=1);
namespace Shlinkio\Shlink\CLI\Command\ShortUrl;
use Shlinkio\Shlink\CLI\Command\Visit\AbstractDeleteVisitsCommand;
use Shlinkio\Shlink\CLI\Util\ExitCode;
use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException;
use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlIdentifier;
use Shlinkio\Shlink\Core\ShortUrl\ShortUrlVisitsDeleterInterface;
use Symfony\Component\Console\Command\Command;
use Symfony\Component\Console\Input\InputArgument;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Input\InputOption;
use Symfony\Component\Console\Output\OutputInterface;
use Symfony\Component\Console\Style\SymfonyStyle;
use function sprintf;
class DeleteShortUrlVisitsCommand extends Command
class DeleteShortUrlVisitsCommand extends AbstractDeleteVisitsCommand
{
public const NAME = 'short-url:delete-visits';
public const NAME = 'short-url:visits-delete';
public function __construct(private readonly ShortUrlVisitsDeleterInterface $deleter)
{
@ -44,15 +43,9 @@ class DeleteShortUrlVisitsCommand extends Command
);
}
protected function execute(InputInterface $input, OutputInterface $output): ?int
protected function doExecute(InputInterface $input, SymfonyStyle $io): ?int
{
$identifier = ShortUrlIdentifier::fromCli($input);
$io = new SymfonyStyle($input, $output);
if (! $this->confirm($io)) {
$io->info('Operation aborted');
return ExitCode::EXIT_SUCCESS;
}
try {
$result = $this->deleter->deleteShortUrlVisits($identifier);
$io->success(sprintf('Successfully deleted %s visits', $result->affectedItems));
@ -64,9 +57,8 @@ class DeleteShortUrlVisitsCommand extends Command
}
}
private function confirm(SymfonyStyle $io): bool
protected function getWarningMessage(): string
{
$io->warning('You are about to delete all visits for a short URL. This operation cannot be undone.');
return $io->confirm('<comment>Continue deleting visits?</comment>', false);
return 'You are about to delete all visits for a short URL. This operation cannot be undone.';
}
}

View File

@ -0,0 +1,35 @@
<?php
declare(strict_types=1);
namespace Shlinkio\Shlink\CLI\Command\Visit;
use Shlinkio\Shlink\CLI\Util\ExitCode;
use Symfony\Component\Console\Command\Command;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Output\OutputInterface;
use Symfony\Component\Console\Style\SymfonyStyle;
abstract class AbstractDeleteVisitsCommand extends Command
{
final protected function execute(InputInterface $input, OutputInterface $output): ?int
{
$io = new SymfonyStyle($input, $output);
if (! $this->confirm($io)) {
$io->info('Operation aborted');
return ExitCode::EXIT_SUCCESS;
}
return $this->doExecute($input, $io);
}
private function confirm(SymfonyStyle $io): bool
{
$io->warning($this->getWarningMessage());
return $io->confirm('<comment>Continue deleting visits?</comment>', false);
}
abstract protected function doExecute(InputInterface $input, SymfonyStyle $io): ?int;
abstract protected function getWarningMessage(): string;
}

View File

@ -0,0 +1,42 @@
<?php
declare(strict_types=1);
namespace Shlinkio\Shlink\CLI\Command\Visit;
use Shlinkio\Shlink\CLI\Util\ExitCode;
use Shlinkio\Shlink\Core\Visit\VisitsDeleterInterface;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Style\SymfonyStyle;
use function sprintf;
class DeleteOrphanVisitsCommand extends AbstractDeleteVisitsCommand
{
public const NAME = 'visit:orphan-delete';
public function __construct(private readonly VisitsDeleterInterface $deleter)
{
parent::__construct();
}
protected function configure(): void
{
$this
->setName(self::NAME)
->setDescription('Deletes all orphan visits');
}
protected function doExecute(InputInterface $input, SymfonyStyle $io): ?int
{
$result = $this->deleter->deleteOrphanVisits();
$io->success(sprintf('Successfully deleted %s visits', $result->affectedItems));
return ExitCode::EXIT_SUCCESS;
}
protected function getWarningMessage(): string
{
return 'You are about to delete all orphan visits. This operation cannot be undone.';
}
}

View File

@ -0,0 +1,43 @@
<?php
declare(strict_types=1);
namespace ShlinkioTest\Shlink\CLI\Command\Visit;
use PHPUnit\Framework\Attributes\Test;
use PHPUnit\Framework\MockObject\MockObject;
use PHPUnit\Framework\TestCase;
use Shlinkio\Shlink\CLI\Command\Visit\DeleteOrphanVisitsCommand;
use Shlinkio\Shlink\CLI\Util\ExitCode;
use Shlinkio\Shlink\Core\Model\BulkDeleteResult;
use Shlinkio\Shlink\Core\Visit\VisitsDeleterInterface;
use ShlinkioTest\Shlink\CLI\CliTestUtilsTrait;
use Symfony\Component\Console\Tester\CommandTester;
class DeleteOrphanVisitsCommandTest extends TestCase
{
use CliTestUtilsTrait;
private CommandTester $commandTester;
private MockObject & VisitsDeleterInterface $deleter;
protected function setUp(): void
{
$this->deleter = $this->createMock(VisitsDeleterInterface::class);
$this->commandTester = $this->testerForCommand(new DeleteOrphanVisitsCommand($this->deleter));
}
#[Test]
public function successMessageIsPrintedAfterDeletion(): void
{
$this->deleter->expects($this->once())->method('deleteOrphanVisits')->willReturn(new BulkDeleteResult(5));
$this->commandTester->setInputs(['yes']);
$exitCode = $this->commandTester->execute([]);
$output = $this->commandTester->getDisplay();
self::assertEquals(ExitCode::EXIT_SUCCESS, $exitCode);
self::assertStringContainsString('You are about to delete all orphan visits.', $output);
self::assertStringContainsString('Successfully deleted 5 visits', $output);
}
}

View File

@ -62,6 +62,7 @@ return [
Visit\VisitsTracker::class => ConfigAbstractFactory::class,
Visit\RequestTracker::class => ConfigAbstractFactory::class,
Visit\VisitsDeleter::class => ConfigAbstractFactory::class,
Visit\Geolocation\VisitLocator::class => ConfigAbstractFactory::class,
Visit\Geolocation\VisitToLocationHelper::class => ConfigAbstractFactory::class,
Visit\VisitsStatsHelper::class => ConfigAbstractFactory::class,
@ -122,6 +123,7 @@ return [
Options\TrackingOptions::class,
],
Visit\RequestTracker::class => [Visit\VisitsTracker::class, Options\TrackingOptions::class],
Visit\VisitsDeleter::class => [Visit\Repository\VisitDeleterRepository::class],
ShortUrl\ShortUrlService::class => [
'em',
ShortUrl\ShortUrlResolver::class,

View File

@ -19,4 +19,13 @@ class VisitDeleterRepository extends EntitySpecificationRepository implements Vi
return $qb->getQuery()->execute();
}
public function deleteOrphanVisits(): int
{
$qb = $this->getEntityManager()->createQueryBuilder();
$qb->delete(Visit::class, 'v')
->where($qb->expr()->isNull('v.shortUrl'));
return $qb->getQuery()->execute();
}
}

View File

@ -9,4 +9,6 @@ use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl;
interface VisitDeleterRepositoryInterface
{
public function deleteShortUrlVisits(ShortUrl $shortUrl): int;
public function deleteOrphanVisits(): int;
}

View File

@ -0,0 +1,22 @@
<?php
declare(strict_types=1);
namespace Shlinkio\Shlink\Core\Visit;
use Shlinkio\Shlink\Core\Model\BulkDeleteResult;
use Shlinkio\Shlink\Core\Visit\Repository\VisitDeleterRepositoryInterface;
use Shlinkio\Shlink\Rest\Entity\ApiKey;
class VisitsDeleter implements VisitsDeleterInterface
{
public function __construct(private readonly VisitDeleterRepositoryInterface $repository)
{
}
public function deleteOrphanVisits(?ApiKey $apiKey = null): BulkDeleteResult
{
// TODO Check API key has permissions for orphan visits
return new BulkDeleteResult($this->repository->deleteOrphanVisits());
}
}

View File

@ -0,0 +1,13 @@
<?php
declare(strict_types=1);
namespace Shlinkio\Shlink\Core\Visit;
use Shlinkio\Shlink\Core\Model\BulkDeleteResult;
use Shlinkio\Shlink\Rest\Entity\ApiKey;
interface VisitsDeleterInterface
{
public function deleteOrphanVisits(?ApiKey $apiKey = null): BulkDeleteResult;
}

View File

@ -25,7 +25,7 @@ class VisitDeleterRepositoryTest extends DatabaseTestCase
}
#[Test]
public function deletesExpectedVisits(): void
public function deletesExpectedShortUrlVisits(): void
{
$shortUrl1 = ShortUrl::withLongUrl('https://foo.com');
$this->getEntityManager()->persist($shortUrl1);
@ -59,4 +59,21 @@ class VisitDeleterRepositoryTest extends DatabaseTestCase
self::assertEquals(1, $this->repo->deleteShortUrlVisits($shortUrl3));
self::assertEquals(0, $this->repo->deleteShortUrlVisits($shortUrl3));
}
#[Test]
public function deletesExpectedOrphanVisits(): void
{
$visitor = Visitor::emptyInstance();
$this->getEntityManager()->persist(Visit::forBasePath($visitor));
$this->getEntityManager()->persist(Visit::forInvalidShortUrl($visitor));
$this->getEntityManager()->persist(Visit::forRegularNotFound($visitor));
$this->getEntityManager()->persist(Visit::forBasePath($visitor));
$this->getEntityManager()->persist(Visit::forInvalidShortUrl($visitor));
$this->getEntityManager()->persist(Visit::forRegularNotFound($visitor));
$this->getEntityManager()->flush();
self::assertEquals(6, $this->repo->deleteOrphanVisits());
self::assertEquals(0, $this->repo->deleteOrphanVisits());
}
}

View File

@ -0,0 +1,41 @@
<?php
declare(strict_types=1);
namespace ShlinkioTest\Shlink\Core\Visit;
use PHPUnit\Framework\Attributes\DataProvider;
use PHPUnit\Framework\Attributes\Test;
use PHPUnit\Framework\MockObject\MockObject;
use PHPUnit\Framework\TestCase;
use Shlinkio\Shlink\Core\Visit\Repository\VisitDeleterRepositoryInterface;
use Shlinkio\Shlink\Core\Visit\VisitsDeleter;
class VisitsDeleterTest extends TestCase
{
private VisitsDeleter $visitsDeleter;
private MockObject & VisitDeleterRepositoryInterface $repo;
protected function setUp(): void
{
$this->repo = $this->createMock(VisitDeleterRepositoryInterface::class);
$this->visitsDeleter = new VisitsDeleter($this->repo);
}
#[Test, DataProvider('provideVisitsCounts')]
public function returnsDeletedVisitsFromRepo(int $visitsCount): void
{
$this->repo->expects($this->once())->method('deleteOrphanVisits')->willReturn($visitsCount);
$result = $this->visitsDeleter->deleteOrphanVisits();
self::assertEquals($visitsCount, $result->affectedItems);
}
public static function provideVisitsCounts(): iterable
{
yield '45' => [45];
yield '5000' => [5000];
yield '0' => [0];
}
}

View File

@ -38,6 +38,7 @@ return [
Action\Visit\DomainVisitsAction::class => ConfigAbstractFactory::class,
Action\Visit\GlobalVisitsAction::class => ConfigAbstractFactory::class,
Action\Visit\OrphanVisitsAction::class => ConfigAbstractFactory::class,
Action\Visit\DeleteOrphanVisitsAction::class => ConfigAbstractFactory::class,
Action\Visit\NonOrphanVisitsAction::class => ConfigAbstractFactory::class,
Action\Tag\ListTagsAction::class => ConfigAbstractFactory::class,
Action\Tag\TagsStatsAction::class => ConfigAbstractFactory::class,
@ -90,6 +91,7 @@ return [
Visit\VisitsStatsHelper::class,
Visit\Transformer\OrphanVisitDataTransformer::class,
],
Action\Visit\DeleteOrphanVisitsAction::class => [Visit\VisitsDeleter::class],
Action\Visit\NonOrphanVisitsAction::class => [Visit\VisitsStatsHelper::class],
Action\ShortUrl\ListShortUrlsAction::class => [
ShortUrl\ShortUrlListService::class,

View File

@ -0,0 +1,33 @@
<?php
declare(strict_types=1);
namespace Shlinkio\Shlink\Rest\Action\Visit;
use Laminas\Diactoros\Response\JsonResponse;
use Psr\Http\Message\ResponseInterface;
use Psr\Http\Message\ServerRequestInterface;
use Shlinkio\Shlink\Common\Paginator\Util\PagerfantaUtilsTrait;
use Shlinkio\Shlink\Core\Visit\VisitsDeleterInterface;
use Shlinkio\Shlink\Rest\Action\AbstractRestAction;
use Shlinkio\Shlink\Rest\Middleware\AuthenticationMiddleware;
class DeleteOrphanVisitsAction extends AbstractRestAction
{
use PagerfantaUtilsTrait;
protected const ROUTE_PATH = '/visits/orphan';
protected const ROUTE_ALLOWED_METHODS = [self::METHOD_DELETE];
public function __construct(private readonly VisitsDeleterInterface $visitsDeleter)
{
}
public function handle(ServerRequestInterface $request): ResponseInterface
{
$apiKey = AuthenticationMiddleware::apiKeyFromRequest($request);
$result = $this->visitsDeleter->deleteOrphanVisits($apiKey);
return new JsonResponse($result->toArray('deletedVisits'));
}
}

View File

@ -21,8 +21,8 @@ class OrphanVisitsAction extends AbstractRestAction
protected const ROUTE_ALLOWED_METHODS = [self::METHOD_GET];
public function __construct(
private VisitsStatsHelperInterface $visitsHelper,
private DataTransformerInterface $orphanVisitTransformer,
private readonly VisitsStatsHelperInterface $visitsHelper,
private readonly DataTransformerInterface $orphanVisitTransformer,
) {
}

View File

@ -0,0 +1,42 @@
<?php
declare(strict_types=1);
namespace ShlinkioApiTest\Shlink\Rest\Action;
use PHPUnit\Framework\Attributes\Test;
use Shlinkio\Shlink\TestUtils\ApiTest\ApiTestCase;
class DeleteOrphanVisitsTest extends ApiTestCase
{
#[Test]
public function deletesVisitsForShortUrlWithoutAffectingTheRest(): void
{
self::assertEquals(7, $this->getTotalVisits());
self::assertEquals(3, $this->getOrphanVisits());
$resp = $this->callApiWithKey(self::METHOD_DELETE, '/visits/orphan');
$payload = $this->getJsonResponsePayload($resp);
self::assertEquals(200, $resp->getStatusCode());
self::assertEquals(3, $payload['deletedVisits']);
self::assertEquals(7, $this->getTotalVisits()); // This verifies that regular visits have not been affected
self::assertEquals(0, $this->getOrphanVisits());
}
private function getTotalVisits(): int
{
$resp = $this->callApiWithKey(self::METHOD_GET, '/visits/non-orphan');
$payload = $this->getJsonResponsePayload($resp);
return $payload['visits']['pagination']['totalItems'];
}
private function getOrphanVisits(): int
{
$resp = $this->callApiWithKey(self::METHOD_GET, '/visits/orphan');
$payload = $this->getJsonResponsePayload($resp);
return $payload['visits']['pagination']['totalItems'];
}
}

View File

@ -22,8 +22,8 @@ class DeleteShortUrlVisitsTest extends ApiTestCase
self::assertEquals(200, $resp->getStatusCode());
self::assertEquals(3, $payload['deletedVisits']);
self::assertEquals(4, $this->getTotalVisits());
self::assertEquals(3, $this->getOrphanVisits());
self::assertEquals(4, $this->getTotalVisits()); // This verifies that other visits have not been affected
self::assertEquals(3, $this->getOrphanVisits()); // This verifies that orphan visits have not been affected
}
private function getTotalVisits(): int

View File

@ -0,0 +1,53 @@
<?php
declare(strict_types=1);
namespace ShlinkioTest\Shlink\Rest\Action\Visit;
use Laminas\Diactoros\Response\JsonResponse;
use Laminas\Diactoros\ServerRequestFactory;
use PHPUnit\Framework\Attributes\DataProvider;
use PHPUnit\Framework\Attributes\Test;
use PHPUnit\Framework\MockObject\MockObject;
use PHPUnit\Framework\TestCase;
use Shlinkio\Shlink\Core\Model\BulkDeleteResult;
use Shlinkio\Shlink\Core\Visit\VisitsDeleterInterface;
use Shlinkio\Shlink\Rest\Action\Visit\DeleteOrphanVisitsAction;
use Shlinkio\Shlink\Rest\Entity\ApiKey;
class DeleteOrphanVisitsActionTest extends TestCase
{
private DeleteOrphanVisitsAction $action;
private MockObject & VisitsDeleterInterface $deleter;
protected function setUp(): void
{
$this->deleter = $this->createMock(VisitsDeleterInterface::class);
$this->action = new DeleteOrphanVisitsAction($this->deleter);
}
#[Test, DataProvider('provideVisitsCounts')]
public function orphanVisitsAreDeleted(int $visitsCount): void
{
$apiKey = ApiKey::create();
$request = ServerRequestFactory::fromGlobals()->withAttribute(ApiKey::class, $apiKey);
$this->deleter->expects($this->once())->method('deleteOrphanVisits')->with($apiKey)->willReturn(
new BulkDeleteResult($visitsCount),
);
/** @var JsonResponse $resp */
$resp = $this->action->handle($request);
$payload = $resp->getPayload();
self::assertEquals(['deletedVisits' => $visitsCount], $payload);
}
public static function provideVisitsCounts(): iterable
{
yield '1' => [1];
yield '0' => [0];
yield '300' => [300];
yield '1234' => [1234];
}
}