From 011856cbfa3a0f709d2fe1e4139ae53ce6663bfe Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 23 Apr 2022 09:15:01 +0200 Subject: [PATCH 1/9] Removed redundant var --- module/CLI/src/Util/GeolocationDbUpdater.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/module/CLI/src/Util/GeolocationDbUpdater.php b/module/CLI/src/Util/GeolocationDbUpdater.php index 67e9d485..22a3bac5 100644 --- a/module/CLI/src/Util/GeolocationDbUpdater.php +++ b/module/CLI/src/Util/GeolocationDbUpdater.php @@ -66,9 +66,8 @@ class GeolocationDbUpdater implements GeolocationDbUpdaterInterface { $buildTimestamp = $this->resolveBuildTimestamp($meta); $buildDate = Chronos::createFromTimestamp($buildTimestamp); - $now = Chronos::now(); - return $now->gt($buildDate->addDays(35)); + return Chronos::now()->gt($buildDate->addDays(35)); } private function resolveBuildTimestamp(Metadata $meta): int From e029d915449214d48dc81a87ef4880e0679bee4d Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 23 Apr 2022 09:27:52 +0200 Subject: [PATCH 2/9] Documented new domain visits endpoint --- .../swagger/examples/short-url-not-found.json | 4 +- .../paths/v2_domains_{domain}_visits.json | 172 ++++++++++++++++++ docs/swagger/swagger.json | 3 + 3 files changed, 177 insertions(+), 2 deletions(-) create mode 100644 docs/swagger/paths/v2_domains_{domain}_visits.json diff --git a/docs/swagger/examples/short-url-not-found.json b/docs/swagger/examples/short-url-not-found.json index 74a5661c..4a58c847 100644 --- a/docs/swagger/examples/short-url-not-found.json +++ b/docs/swagger/examples/short-url-not-found.json @@ -1,7 +1,7 @@ { "value": { - "detail":"No URL found with short code \"abc123\"", - "title":"Short URL not found", + "detail": "No URL found with short code \"abc123\"", + "title": "Short URL not found", "type": "INVALID_SHORTCODE", "status": 404, "shortCode": "abc123" diff --git a/docs/swagger/paths/v2_domains_{domain}_visits.json b/docs/swagger/paths/v2_domains_{domain}_visits.json new file mode 100644 index 00000000..33389f32 --- /dev/null +++ b/docs/swagger/paths/v2_domains_{domain}_visits.json @@ -0,0 +1,172 @@ +{ + "get": { + "operationId": "getDomainVisits", + "tags": [ + "Visits" + ], + "summary": "List visits for domain", + "description": "Get the list of visits on any short URL which belongs to provided domain.", + "parameters": [ + { + "$ref": "../parameters/version.json" + }, + { + "name": "domain", + "in": "path", + "description": "The domain from which we want to get the visits, or **DEFAULT** keyword for default domain.", + "required": true, + "schema": { + "type": "string" + } + }, + { + "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" + } + }, + { + "name": "excludeBots", + "in": "query", + "description": "Tells if visits from potential bots should be excluded from the result set", + "required": false, + "schema": { + "type": "string", + "enum": ["true"] + } + } + ], + "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/Visit.json" + } + }, + "pagination": { + "$ref": "../definitions/Pagination.json" + } + } + } + } + }, + "example": { + "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, + "potentialBot": false + }, + { + "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" + }, + "potentialBot": false + }, + { + "referer": null, + "date": "2015-08-20T05:05:03+04:00", + "userAgent": "some_web_crawler/1.4", + "visitLocation": null, + "potentialBot": true + } + ], + "pagination": { + "currentPage": 5, + "pagesCount": 12, + "itemsPerPage": 10, + "itemsInCurrentPage": 10, + "totalItems": 115 + } + } + } + } + } + }, + "404": { + "description": "The domain does not exist.", + "content": { + "application/problem+json": { + "schema": { + "$ref": "../definitions/Error.json" + }, + "example": { + "detail": "Domain with authority \"example.com\" could not be found", + "title": "Domain not found", + "type": "DOMAIN_NOT_FOUND", + "status": 404, + "authority": "example.com" + } + } + } + }, + "default": { + "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 3730b527..ed29233b 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}/domain/{domain}/visits": { + "$ref": "paths/v2_domains_{domain}_visits.json" + }, "/rest/v{version}/visits/orphan": { "$ref": "paths/v2_visits_orphan.json" }, From e11bf6ac67e092f97aa921054ca2976f4938ae2f Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 23 Apr 2022 10:32:07 +0200 Subject: [PATCH 3/9] Created endpoint to get visits for one specific domain --- config/autoload/swoole.global.php | 2 +- config/autoload/url-shortener.global.php | 2 +- config/cli-app.php | 2 +- config/container.php | 2 +- config/entity-manager.php | 3 +- docs/swagger/swagger.json | 2 +- .../Core/src/Repository/VisitRepository.php | 41 ++++++++++ .../Repository/VisitRepositoryInterface.php | 7 ++ .../Adapter/DomainVisitsPaginatorAdapter.php | 49 ++++++++++++ module/Core/src/Visit/VisitsStatsHelper.php | 22 ++++++ .../src/Visit/VisitsStatsHelperInterface.php | 7 ++ module/Rest/config/dependencies.config.php | 5 ++ module/Rest/config/routes.config.php | 79 ++++++++++--------- .../src/Action/Visit/DomainVisitsAction.php | 48 +++++++++++ 14 files changed, 227 insertions(+), 44 deletions(-) create mode 100644 module/Core/src/Visit/Paginator/Adapter/DomainVisitsPaginatorAdapter.php create mode 100644 module/Rest/src/Action/Visit/DomainVisitsAction.php diff --git a/config/autoload/swoole.global.php b/config/autoload/swoole.global.php index 9d2c423f..987c967e 100644 --- a/config/autoload/swoole.global.php +++ b/config/autoload/swoole.global.php @@ -6,7 +6,7 @@ use Shlinkio\Shlink\Core\Config\EnvVars; use const Shlinkio\Shlink\MIN_TASK_WORKERS; -return (static function () { +return (static function (): array { $taskWorkers = (int) EnvVars::TASK_WORKER_NUM()->loadFromEnv(16); return [ diff --git a/config/autoload/url-shortener.global.php b/config/autoload/url-shortener.global.php index 25de914a..58c12f05 100644 --- a/config/autoload/url-shortener.global.php +++ b/config/autoload/url-shortener.global.php @@ -16,7 +16,7 @@ return (static function (): array { return [ 'url_shortener' => [ - 'domain' => [ + 'domain' => [ // TODO Refactor this structure to url_shortener.schema and url_shortener.default_domain 'schema' => ((bool) EnvVars::IS_HTTPS_ENABLED()->loadFromEnv(true)) ? 'https' : 'http', 'hostname' => EnvVars::DEFAULT_DOMAIN()->loadFromEnv(''), ], diff --git a/config/cli-app.php b/config/cli-app.php index a2272852..9287cbaf 100644 --- a/config/cli-app.php +++ b/config/cli-app.php @@ -5,7 +5,7 @@ declare(strict_types=1); use Psr\Container\ContainerInterface; use Symfony\Component\Console\Application as CliApp; -return (static function () { +return (static function (): CliApp { /** @var ContainerInterface $container */ $container = include __DIR__ . '/container.php'; return $container->get(CliApp::class); diff --git a/config/container.php b/config/container.php index 568eb1ee..074502cd 100644 --- a/config/container.php +++ b/config/container.php @@ -22,7 +22,7 @@ if (! class_exists(LOCAL_LOCK_FACTORY)) { } // Build container -return (static function () { +return (static function (): ServiceManager { $config = require __DIR__ . '/config.php'; $container = new ServiceManager($config['dependencies']); $container->setService('config', $config); diff --git a/config/entity-manager.php b/config/entity-manager.php index 2b4794f7..6721fec3 100644 --- a/config/entity-manager.php +++ b/config/entity-manager.php @@ -3,9 +3,10 @@ declare(strict_types=1); use Doctrine\ORM\EntityManager; +use Doctrine\ORM\EntityManagerInterface; use Psr\Container\ContainerInterface; -return (static function () { +return (static function (): EntityManagerInterface { /** @var ContainerInterface $container */ $container = include __DIR__ . '/container.php'; return $container->get(EntityManager::class); diff --git a/docs/swagger/swagger.json b/docs/swagger/swagger.json index ed29233b..06f57c41 100644 --- a/docs/swagger/swagger.json +++ b/docs/swagger/swagger.json @@ -95,7 +95,7 @@ "/rest/v{version}/tags/{tag}/visits": { "$ref": "paths/v2_tags_{tag}_visits.json" }, - "/rest/v{version}/domain/{domain}/visits": { + "/rest/v{version}/domains/{domain}/visits": { "$ref": "paths/v2_domains_{domain}_visits.json" }, "/rest/v{version}/visits/orphan": { diff --git a/module/Core/src/Repository/VisitRepository.php b/module/Core/src/Repository/VisitRepository.php index b43d676d..51a0c333 100644 --- a/module/Core/src/Repository/VisitRepository.php +++ b/module/Core/src/Repository/VisitRepository.php @@ -154,6 +154,47 @@ class VisitRepository extends EntitySpecificationRepository implements VisitRepo return $qb; } + /** + * @return Visit[] + */ + public function findVisitsByDomain(string $domain, VisitsListFiltering $filtering): array + { + $qb = $this->createVisitsByDomainQueryBuilder($domain, $filtering); + return $this->resolveVisitsWithNativeQuery($qb, $filtering->limit(), $filtering->offset()); + } + + public function countVisitsByDomain(string $domain, VisitsCountFiltering $filtering): int + { + $qb = $this->createVisitsByDomainQueryBuilder($domain, $filtering); + $qb->select('COUNT(v.id)'); + + return (int) $qb->getQuery()->getSingleScalarResult(); + } + + private function createVisitsByDomainQueryBuilder(string $domain, VisitsCountFiltering $filtering): QueryBuilder + { + // Parameters in this query need to be inlined, not bound, as we need to use it as sub-query later. + $qb = $this->getEntityManager()->createQueryBuilder(); + $qb->from(Visit::class, 'v') + ->join('v.shortUrl', 's'); + + if ($domain === 'DEFAULT') { + $qb->where($qb->expr()->isNull('s.domain')); + } else { + $qb->join('s.domain', 'd') + ->where($qb->expr()->eq('d.authority', $this->getEntityManager()->getConnection()->quote($domain))); + } + + if ($filtering->excludeBots()) { + $qb->andWhere($qb->expr()->eq('v.potentialBot', 'false')); + } + + $this->applyDatesInline($qb, $filtering->dateRange()); + $this->applySpecification($qb, $filtering->apiKey()?->inlinedSpec(), 'v'); + + return $qb; + } + public function findOrphanVisits(VisitsListFiltering $filtering): array { $qb = $this->createAllVisitsQueryBuilder($filtering); diff --git a/module/Core/src/Repository/VisitRepositoryInterface.php b/module/Core/src/Repository/VisitRepositoryInterface.php index 3d480c01..837dea1b 100644 --- a/module/Core/src/Repository/VisitRepositoryInterface.php +++ b/module/Core/src/Repository/VisitRepositoryInterface.php @@ -45,6 +45,13 @@ interface VisitRepositoryInterface extends ObjectRepository, EntitySpecification public function countVisitsByTag(string $tag, VisitsCountFiltering $filtering): int; + /** + * @return Visit[] + */ + public function findVisitsByDomain(string $domain, VisitsListFiltering $filtering): array; + + public function countVisitsByDomain(string $domain, VisitsCountFiltering $filtering): int; + /** * @return Visit[] */ diff --git a/module/Core/src/Visit/Paginator/Adapter/DomainVisitsPaginatorAdapter.php b/module/Core/src/Visit/Paginator/Adapter/DomainVisitsPaginatorAdapter.php new file mode 100644 index 00000000..508a7b36 --- /dev/null +++ b/module/Core/src/Visit/Paginator/Adapter/DomainVisitsPaginatorAdapter.php @@ -0,0 +1,49 @@ +visitRepository->countVisitsByDomain( + $this->domain, + new VisitsCountFiltering( + $this->params->getDateRange(), + $this->params->excludeBots(), + $this->apiKey, + ), + ); + } + + public function getSlice(int $offset, int $length): iterable + { + return $this->visitRepository->findVisitsByDomain( + $this->domain, + new VisitsListFiltering( + $this->params->getDateRange(), + $this->params->excludeBots(), + $this->apiKey, + $length, + $offset, + ), + ); + } +} diff --git a/module/Core/src/Visit/VisitsStatsHelper.php b/module/Core/src/Visit/VisitsStatsHelper.php index 914a9c5b..3acac90d 100644 --- a/module/Core/src/Visit/VisitsStatsHelper.php +++ b/module/Core/src/Visit/VisitsStatsHelper.php @@ -7,9 +7,12 @@ namespace Shlinkio\Shlink\Core\Visit; use Doctrine\ORM\EntityManagerInterface; use Pagerfanta\Adapter\AdapterInterface; use Shlinkio\Shlink\Common\Paginator\Paginator; +use Shlinkio\Shlink\Core\Domain\Repository\DomainRepository; +use Shlinkio\Shlink\Core\Entity\Domain; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Entity\Tag; use Shlinkio\Shlink\Core\Entity\Visit; +use Shlinkio\Shlink\Core\Exception\DomainNotFoundException; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; use Shlinkio\Shlink\Core\Exception\TagNotFoundException; use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; @@ -19,6 +22,7 @@ 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\Core\Visit\Paginator\Adapter\DomainVisitsPaginatorAdapter; use Shlinkio\Shlink\Core\Visit\Paginator\Adapter\NonOrphanVisitsPaginatorAdapter; use Shlinkio\Shlink\Core\Visit\Paginator\Adapter\OrphanVisitsPaginatorAdapter; use Shlinkio\Shlink\Core\Visit\Paginator\Adapter\ShortUrlVisitsPaginatorAdapter; @@ -85,6 +89,24 @@ class VisitsStatsHelper implements VisitsStatsHelperInterface return $this->createPaginator(new TagVisitsPaginatorAdapter($repo, $tag, $params, $apiKey), $params); } + /** + * @return Visit[]|Paginator + * @throws DomainNotFoundException + */ + public function visitsForDomain(string $domain, VisitsParams $params, ?ApiKey $apiKey = null): Paginator + { + /** @var DomainRepository $domainRepo */ + $domainRepo = $this->em->getRepository(Domain::class); + if ($domain !== 'DEFAULT' && $domainRepo->count(['authority' => $domain]) === 0) { + throw DomainNotFoundException::fromAuthority($domain); + } + + /** @var VisitRepositoryInterface $repo */ + $repo = $this->em->getRepository(Visit::class); + + return $this->createPaginator(new DomainVisitsPaginatorAdapter($repo, $domain, $params, $apiKey), $params); + } + /** * @return Visit[]|Paginator */ diff --git a/module/Core/src/Visit/VisitsStatsHelperInterface.php b/module/Core/src/Visit/VisitsStatsHelperInterface.php index 3616b531..b32fc99d 100644 --- a/module/Core/src/Visit/VisitsStatsHelperInterface.php +++ b/module/Core/src/Visit/VisitsStatsHelperInterface.php @@ -6,6 +6,7 @@ namespace Shlinkio\Shlink\Core\Visit; use Shlinkio\Shlink\Common\Paginator\Paginator; use Shlinkio\Shlink\Core\Entity\Visit; +use Shlinkio\Shlink\Core\Exception\DomainNotFoundException; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; use Shlinkio\Shlink\Core\Exception\TagNotFoundException; use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; @@ -33,6 +34,12 @@ interface VisitsStatsHelperInterface */ public function visitsForTag(string $tag, VisitsParams $params, ?ApiKey $apiKey = null): Paginator; + /** + * @return Visit[]|Paginator + * @throws DomainNotFoundException + */ + public function visitsForDomain(string $domain, VisitsParams $params, ?ApiKey $apiKey = null): Paginator; + /** * @return Visit[]|Paginator */ diff --git a/module/Rest/config/dependencies.config.php b/module/Rest/config/dependencies.config.php index a7a4b2ca..34be71f4 100644 --- a/module/Rest/config/dependencies.config.php +++ b/module/Rest/config/dependencies.config.php @@ -34,6 +34,7 @@ return [ Action\ShortUrl\ListShortUrlsAction::class => ConfigAbstractFactory::class, Action\Visit\ShortUrlVisitsAction::class => ConfigAbstractFactory::class, Action\Visit\TagVisitsAction::class => ConfigAbstractFactory::class, + Action\Visit\DomainVisitsAction::class => ConfigAbstractFactory::class, Action\Visit\GlobalVisitsAction::class => ConfigAbstractFactory::class, Action\Visit\OrphanVisitsAction::class => ConfigAbstractFactory::class, Action\Visit\NonOrphanVisitsAction::class => ConfigAbstractFactory::class, @@ -73,6 +74,10 @@ return [ ], Action\Visit\ShortUrlVisitsAction::class => [Visit\VisitsStatsHelper::class], Action\Visit\TagVisitsAction::class => [Visit\VisitsStatsHelper::class], + Action\Visit\DomainVisitsAction::class => [ + Visit\VisitsStatsHelper::class, + 'config.url_shortener.domain.hostname', + ], Action\Visit\GlobalVisitsAction::class => [Visit\VisitsStatsHelper::class], Action\Visit\OrphanVisitsAction::class => [ Visit\VisitsStatsHelper::class, diff --git a/module/Rest/config/routes.config.php b/module/Rest/config/routes.config.php index c9b42579..f318664f 100644 --- a/module/Rest/config/routes.config.php +++ b/module/Rest/config/routes.config.php @@ -6,49 +6,52 @@ namespace Shlinkio\Shlink\Rest; use Shlinkio\Shlink\Rest\Middleware\Mercure\NotConfiguredMercureErrorHandler; -$contentNegotiationMiddleware = Middleware\ShortUrl\CreateShortUrlContentNegotiationMiddleware::class; -$dropDomainMiddleware = Middleware\ShortUrl\DropDefaultDomainFromRequestMiddleware::class; -$overrideDomainMiddleware = Middleware\ShortUrl\OverrideDomainMiddleware::class; +return (static function (): array { + $contentNegotiationMiddleware = Middleware\ShortUrl\CreateShortUrlContentNegotiationMiddleware::class; + $dropDomainMiddleware = Middleware\ShortUrl\DropDefaultDomainFromRequestMiddleware::class; + $overrideDomainMiddleware = Middleware\ShortUrl\OverrideDomainMiddleware::class; -return [ + return [ - 'routes' => [ - Action\HealthAction::getRouteDef(), + 'routes' => [ + Action\HealthAction::getRouteDef(), - // Short URLs - Action\ShortUrl\CreateShortUrlAction::getRouteDef([ - $contentNegotiationMiddleware, - $dropDomainMiddleware, - $overrideDomainMiddleware, - Middleware\ShortUrl\DefaultShortCodesLengthMiddleware::class, - ]), - Action\ShortUrl\SingleStepCreateShortUrlAction::getRouteDef([ - $contentNegotiationMiddleware, - $overrideDomainMiddleware, - ]), - Action\ShortUrl\EditShortUrlAction::getRouteDef([$dropDomainMiddleware]), - Action\ShortUrl\DeleteShortUrlAction::getRouteDef([$dropDomainMiddleware]), - Action\ShortUrl\ResolveShortUrlAction::getRouteDef([$dropDomainMiddleware]), - Action\ShortUrl\ListShortUrlsAction::getRouteDef(), + // Short URLs + Action\ShortUrl\CreateShortUrlAction::getRouteDef([ + $contentNegotiationMiddleware, + $dropDomainMiddleware, + $overrideDomainMiddleware, + Middleware\ShortUrl\DefaultShortCodesLengthMiddleware::class, + ]), + Action\ShortUrl\SingleStepCreateShortUrlAction::getRouteDef([ + $contentNegotiationMiddleware, + $overrideDomainMiddleware, + ]), + Action\ShortUrl\EditShortUrlAction::getRouteDef([$dropDomainMiddleware]), + Action\ShortUrl\DeleteShortUrlAction::getRouteDef([$dropDomainMiddleware]), + Action\ShortUrl\ResolveShortUrlAction::getRouteDef([$dropDomainMiddleware]), + Action\ShortUrl\ListShortUrlsAction::getRouteDef(), - // Visits - Action\Visit\ShortUrlVisitsAction::getRouteDef([$dropDomainMiddleware]), - Action\Visit\TagVisitsAction::getRouteDef(), - Action\Visit\GlobalVisitsAction::getRouteDef(), - Action\Visit\OrphanVisitsAction::getRouteDef(), - Action\Visit\NonOrphanVisitsAction::getRouteDef(), + // Visits + Action\Visit\ShortUrlVisitsAction::getRouteDef([$dropDomainMiddleware]), + Action\Visit\TagVisitsAction::getRouteDef(), + Action\Visit\DomainVisitsAction::getRouteDef(), + Action\Visit\GlobalVisitsAction::getRouteDef(), + Action\Visit\OrphanVisitsAction::getRouteDef(), + Action\Visit\NonOrphanVisitsAction::getRouteDef(), - // Tags - Action\Tag\ListTagsAction::getRouteDef(), - Action\Tag\TagsStatsAction::getRouteDef(), - Action\Tag\DeleteTagsAction::getRouteDef(), - Action\Tag\UpdateTagAction::getRouteDef(), + // Tags + Action\Tag\ListTagsAction::getRouteDef(), + Action\Tag\TagsStatsAction::getRouteDef(), + Action\Tag\DeleteTagsAction::getRouteDef(), + Action\Tag\UpdateTagAction::getRouteDef(), - // Domains - Action\Domain\ListDomainsAction::getRouteDef(), - Action\Domain\DomainRedirectsAction::getRouteDef(), + // Domains + Action\Domain\ListDomainsAction::getRouteDef(), + Action\Domain\DomainRedirectsAction::getRouteDef(), - Action\MercureInfoAction::getRouteDef([NotConfiguredMercureErrorHandler::class]), - ], + Action\MercureInfoAction::getRouteDef([NotConfiguredMercureErrorHandler::class]), + ], -]; + ]; +})(); diff --git a/module/Rest/src/Action/Visit/DomainVisitsAction.php b/module/Rest/src/Action/Visit/DomainVisitsAction.php new file mode 100644 index 00000000..b68d971f --- /dev/null +++ b/module/Rest/src/Action/Visit/DomainVisitsAction.php @@ -0,0 +1,48 @@ +resolveDomainParam($request); + $params = VisitsParams::fromRawData($request->getQueryParams()); + $apiKey = AuthenticationMiddleware::apiKeyFromRequest($request); + $visits = $this->visitsHelper->visitsForDomain($domain, $params, $apiKey); + + return new JsonResponse([ + 'visits' => $this->serializePaginator($visits), + ]); + } + + private function resolveDomainParam(Request $request): string + { + $domainParam = $request->getAttribute('domain', ''); + if ($domainParam === $this->defaultDomain) { + return 'DEFAULT'; + } + + return $domainParam; + } +} From 984205e02c2e8f224d799b6d2635a5c123650b62 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 23 Apr 2022 10:45:42 +0200 Subject: [PATCH 4/9] Extended VisitRepositoryTest with domain visits functions --- .../Repository/VisitRepositoryTest.php | 50 ++++++++++++++++++- 1 file changed, 49 insertions(+), 1 deletion(-) diff --git a/module/Core/test-db/Repository/VisitRepositoryTest.php b/module/Core/test-db/Repository/VisitRepositoryTest.php index c23bd8aa..b16c3382 100644 --- a/module/Core/test-db/Repository/VisitRepositoryTest.php +++ b/module/Core/test-db/Repository/VisitRepositoryTest.php @@ -52,7 +52,7 @@ class VisitRepositoryTest extends DatabaseTestCase { $shortUrl = ShortUrl::createEmpty(); $this->getEntityManager()->persist($shortUrl); - $countIterable = function (iterable $results): int { + $countIterable = static function (iterable $results): int { $resultsCount = 0; foreach ($results as $value) { $resultsCount++; @@ -256,6 +256,54 @@ class VisitRepositoryTest extends DatabaseTestCase ))); } + /** @test */ + public function findVisitsByDomainReturnsProperData(): void + { + $this->createShortUrlsAndVisits('doma.in'); + $this->getEntityManager()->flush(); + + self::assertCount(0, $this->repo->findVisitsByDomain('invalid', new VisitsListFiltering())); + self::assertCount(6, $this->repo->findVisitsByDomain('DEFAULT', new VisitsListFiltering())); + self::assertCount(3, $this->repo->findVisitsByDomain('doma.in', new VisitsListFiltering())); + self::assertCount(1, $this->repo->findVisitsByDomain('doma.in', new VisitsListFiltering(null, true))); + self::assertCount(2, $this->repo->findVisitsByDomain('doma.in', new VisitsListFiltering( + DateRange::withStartAndEndDate(Chronos::parse('2016-01-02'), Chronos::parse('2016-01-03')), + ))); + self::assertCount(1, $this->repo->findVisitsByDomain('doma.in', new VisitsListFiltering( + DateRange::withStartDate(Chronos::parse('2016-01-03')), + ))); + self::assertCount(2, $this->repo->findVisitsByDomain('DEFAULT', new VisitsListFiltering( + DateRange::withStartAndEndDate(Chronos::parse('2016-01-02'), Chronos::parse('2016-01-03')), + ))); + self::assertCount(4, $this->repo->findVisitsByDomain('DEFAULT', new VisitsListFiltering( + DateRange::withStartDate(Chronos::parse('2016-01-03')), + ))); + } + + /** @test */ + public function countVisitsByDomainReturnsProperData(): void + { + $this->createShortUrlsAndVisits('doma.in'); + $this->getEntityManager()->flush(); + + self::assertEquals(0, $this->repo->countVisitsByDomain('invalid', new VisitsListFiltering())); + self::assertEquals(6, $this->repo->countVisitsByDomain('DEFAULT', new VisitsListFiltering())); + self::assertEquals(3, $this->repo->countVisitsByDomain('doma.in', new VisitsListFiltering())); + self::assertEquals(1, $this->repo->countVisitsByDomain('doma.in', new VisitsListFiltering(null, true))); + self::assertEquals(2, $this->repo->countVisitsByDomain('doma.in', new VisitsListFiltering( + DateRange::withStartAndEndDate(Chronos::parse('2016-01-02'), Chronos::parse('2016-01-03')), + ))); + self::assertEquals(1, $this->repo->countVisitsByDomain('doma.in', new VisitsListFiltering( + DateRange::withStartDate(Chronos::parse('2016-01-03')), + ))); + self::assertEquals(2, $this->repo->countVisitsByDomain('DEFAULT', new VisitsListFiltering( + DateRange::withStartAndEndDate(Chronos::parse('2016-01-02'), Chronos::parse('2016-01-03')), + ))); + self::assertEquals(4, $this->repo->countVisitsByDomain('DEFAULT', new VisitsListFiltering( + DateRange::withStartDate(Chronos::parse('2016-01-03')), + ))); + } + /** @test */ public function countVisitsReturnsExpectedResultBasedOnApiKey(): void { From 9a0e5ea626ccf52e0abea7183de0036656fe09e0 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 23 Apr 2022 10:58:33 +0200 Subject: [PATCH 5/9] Created method to check if domain exists based on authority and API key --- .../Domain/Repository/DomainRepository.php | 24 ++++++++++++++++--- .../Repository/DomainRepositoryInterface.php | 2 ++ .../Repository/DomainRepositoryTest.php | 10 ++++++++ 3 files changed, 33 insertions(+), 3 deletions(-) diff --git a/module/Core/src/Domain/Repository/DomainRepository.php b/module/Core/src/Domain/Repository/DomainRepository.php index 4de3ea36..0a99b3c6 100644 --- a/module/Core/src/Domain/Repository/DomainRepository.php +++ b/module/Core/src/Domain/Repository/DomainRepository.php @@ -5,6 +5,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Domain\Repository; use Doctrine\ORM\Query\Expr\Join; +use Doctrine\ORM\QueryBuilder; use Happyr\DoctrineSpecification\Repository\EntitySpecificationRepository; use Happyr\DoctrineSpecification\Spec; use Shlinkio\Shlink\Core\Domain\Spec\IsDomain; @@ -40,8 +41,25 @@ class DomainRepository extends EntitySpecificationRepository implements DomainRe public function findOneByAuthority(string $authority, ?ApiKey $apiKey = null): ?Domain { - $qb = $this->createQueryBuilder('d'); - $qb->leftJoin(ShortUrl::class, 's', Join::WITH, 's.domain = d') + $qb = $this->createDomainQueryBuilder($authority, $apiKey); + $qb->select('d'); + + return $qb->getQuery()->getOneOrNullResult(); + } + + public function domainExists(string $authority, ?ApiKey $apiKey = null): bool + { + $qb = $this->createDomainQueryBuilder($authority, $apiKey); + $qb->select('COUNT(d.id)'); + + return ((int) $qb->getQuery()->getSingleScalarResult()) > 0; + } + + private function createDomainQueryBuilder(string $authority, ?ApiKey $apiKey): QueryBuilder + { + $qb = $this->getEntityManager()->createQueryBuilder(); + $qb->from(Domain::class, 'd') + ->leftJoin(ShortUrl::class, 's', Join::WITH, 's.domain = d') ->where($qb->expr()->eq('d.authority', ':authority')) ->setParameter('authority', $authority) ->setMaxResults(1); @@ -51,7 +69,7 @@ class DomainRepository extends EntitySpecificationRepository implements DomainRe $this->applySpecification($qb, $spec, $alias); } - return $qb->getQuery()->getOneOrNullResult(); + return $qb; } private function determineExtraSpecs(?ApiKey $apiKey): iterable diff --git a/module/Core/src/Domain/Repository/DomainRepositoryInterface.php b/module/Core/src/Domain/Repository/DomainRepositoryInterface.php index 69e74e5b..d5f880bd 100644 --- a/module/Core/src/Domain/Repository/DomainRepositoryInterface.php +++ b/module/Core/src/Domain/Repository/DomainRepositoryInterface.php @@ -17,4 +17,6 @@ interface DomainRepositoryInterface extends ObjectRepository, EntitySpecificatio public function findDomains(?ApiKey $apiKey = null): array; public function findOneByAuthority(string $authority, ?ApiKey $apiKey = null): ?Domain; + + public function domainExists(string $authority, ?ApiKey $apiKey = null): bool; } diff --git a/module/Core/test-db/Domain/Repository/DomainRepositoryTest.php b/module/Core/test-db/Domain/Repository/DomainRepositoryTest.php index 3f69e7d9..d1b3bbeb 100644 --- a/module/Core/test-db/Domain/Repository/DomainRepositoryTest.php +++ b/module/Core/test-db/Domain/Repository/DomainRepositoryTest.php @@ -55,6 +55,10 @@ class DomainRepositoryTest extends DatabaseTestCase self::assertEquals($detachedWithRedirects, $this->repo->findOneByAuthority('detached-with-redirects.com')); self::assertNull($this->repo->findOneByAuthority('does-not-exist.com')); self::assertEquals($detachedDomain, $this->repo->findOneByAuthority('detached.com')); + self::assertTrue($this->repo->domainExists('bar.com')); + self::assertTrue($this->repo->domainExists('detached-with-redirects.com')); + self::assertFalse($this->repo->domainExists('does-not-exist.com')); + self::assertTrue($this->repo->domainExists('detached.com')); } /** @test */ @@ -115,6 +119,12 @@ class DomainRepositoryTest extends DatabaseTestCase $this->repo->findOneByAuthority('detached-with-redirects.com', $detachedWithRedirectsApiKey), ); self::assertNull($this->repo->findOneByAuthority('foo.com', $detachedWithRedirectsApiKey)); + + self::assertTrue($this->repo->domainExists('foo.com', $authorApiKey)); + self::assertFalse($this->repo->domainExists('bar.com', $authorApiKey)); + self::assertTrue($this->repo->domainExists('bar.com', $barDomainApiKey)); + self::assertTrue($this->repo->domainExists('detached-with-redirects.com', $detachedWithRedirectsApiKey)); + self::assertFalse($this->repo->domainExists('foo.com', $detachedWithRedirectsApiKey)); } private function createShortUrl(Domain $domain, ?ApiKey $apiKey = null): ShortUrl From 99b4f9f4ddb7f38a608062326fa7e73f56d4875a Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 23 Apr 2022 11:02:51 +0200 Subject: [PATCH 6/9] Improved VisitsStatsHelperTest covering visitsForDomain method --- module/Core/src/Visit/VisitsStatsHelper.php | 2 +- .../Core/test/Visit/VisitsStatsHelperTest.php | 66 +++++++++++++++++++ 2 files changed, 67 insertions(+), 1 deletion(-) diff --git a/module/Core/src/Visit/VisitsStatsHelper.php b/module/Core/src/Visit/VisitsStatsHelper.php index 3acac90d..007ed334 100644 --- a/module/Core/src/Visit/VisitsStatsHelper.php +++ b/module/Core/src/Visit/VisitsStatsHelper.php @@ -97,7 +97,7 @@ class VisitsStatsHelper implements VisitsStatsHelperInterface { /** @var DomainRepository $domainRepo */ $domainRepo = $this->em->getRepository(Domain::class); - if ($domain !== 'DEFAULT' && $domainRepo->count(['authority' => $domain]) === 0) { + if ($domain !== 'DEFAULT' && ! $domainRepo->domainExists($domain, $apiKey)) { throw DomainNotFoundException::fromAuthority($domain); } diff --git a/module/Core/test/Visit/VisitsStatsHelperTest.php b/module/Core/test/Visit/VisitsStatsHelperTest.php index 731697e6..42c821bb 100644 --- a/module/Core/test/Visit/VisitsStatsHelperTest.php +++ b/module/Core/test/Visit/VisitsStatsHelperTest.php @@ -10,9 +10,12 @@ use PHPUnit\Framework\TestCase; use Prophecy\Argument; use Prophecy\PhpUnit\ProphecyTrait; use Prophecy\Prophecy\ObjectProphecy; +use Shlinkio\Shlink\Core\Domain\Repository\DomainRepository; +use Shlinkio\Shlink\Core\Entity\Domain; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Entity\Tag; use Shlinkio\Shlink\Core\Entity\Visit; +use Shlinkio\Shlink\Core\Exception\DomainNotFoundException; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; use Shlinkio\Shlink\Core\Exception\TagNotFoundException; use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; @@ -158,6 +161,69 @@ class VisitsStatsHelperTest extends TestCase $getRepo->shouldHaveBeenCalledOnce(); } + /** @test */ + public function throwsExceptionWhenRequestingVisitsForInvalidDomain(): void + { + $domain = 'foo.com'; + $apiKey = ApiKey::create(); + $repo = $this->prophesize(DomainRepository::class); + $domainExists = $repo->domainExists($domain, $apiKey)->willReturn(false); + $getRepo = $this->em->getRepository(Domain::class)->willReturn($repo->reveal()); + + $this->expectException(DomainNotFoundException::class); + $domainExists->shouldBeCalledOnce(); + $getRepo->shouldBeCalledOnce(); + + $this->helper->visitsForDomain($domain, new VisitsParams(), $apiKey); + } + + /** + * @test + * @dataProvider provideAdminApiKeys + */ + public function visitsForNonDefaultDomainAreReturnedAsExpected(?ApiKey $apiKey): void + { + $domain = 'foo.com'; + $repo = $this->prophesize(DomainRepository::class); + $domainExists = $repo->domainExists($domain, $apiKey)->willReturn(true); + $getRepo = $this->em->getRepository(Domain::class)->willReturn($repo->reveal()); + + $list = map(range(0, 1), fn () => Visit::forValidShortUrl(ShortUrl::createEmpty(), Visitor::emptyInstance())); + $repo2 = $this->prophesize(VisitRepository::class); + $repo2->findVisitsByDomain($domain, Argument::type(VisitsListFiltering::class))->willReturn($list); + $repo2->countVisitsByDomain($domain, Argument::type(VisitsCountFiltering::class))->willReturn(1); + $this->em->getRepository(Visit::class)->willReturn($repo2->reveal())->shouldBeCalledOnce(); + + $paginator = $this->helper->visitsForDomain($domain, new VisitsParams(), $apiKey); + + self::assertEquals($list, ArrayUtils::iteratorToArray($paginator->getCurrentPageResults())); + $domainExists->shouldHaveBeenCalledOnce(); + $getRepo->shouldHaveBeenCalledOnce(); + } + + /** + * @test + * @dataProvider provideAdminApiKeys + */ + public function visitsForDefaultDomainAreReturnedAsExpected(?ApiKey $apiKey): void + { + $repo = $this->prophesize(DomainRepository::class); + $domainExists = $repo->domainExists(Argument::cetera()); + $getRepo = $this->em->getRepository(Domain::class)->willReturn($repo->reveal()); + + $list = map(range(0, 1), fn () => Visit::forValidShortUrl(ShortUrl::createEmpty(), Visitor::emptyInstance())); + $repo2 = $this->prophesize(VisitRepository::class); + $repo2->findVisitsByDomain('DEFAULT', Argument::type(VisitsListFiltering::class))->willReturn($list); + $repo2->countVisitsByDomain('DEFAULT', Argument::type(VisitsCountFiltering::class))->willReturn(1); + $this->em->getRepository(Visit::class)->willReturn($repo2->reveal())->shouldBeCalledOnce(); + + $paginator = $this->helper->visitsForDomain('DEFAULT', new VisitsParams(), $apiKey); + + self::assertEquals($list, ArrayUtils::iteratorToArray($paginator->getCurrentPageResults())); + $domainExists->shouldNotHaveBeenCalled(); + $getRepo->shouldHaveBeenCalledOnce(); + } + /** @test */ public function orphanVisitsAreReturnedAsExpected(): void { From af15e31b426a0f49928d870fbd64d40e1ea24b60 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 23 Apr 2022 11:07:10 +0200 Subject: [PATCH 7/9] Created DomainVisitsActionTest --- .../Action/Visit/DomainVisitsActionTest.php | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+) create mode 100644 module/Rest/test/Action/Visit/DomainVisitsActionTest.php diff --git a/module/Rest/test/Action/Visit/DomainVisitsActionTest.php b/module/Rest/test/Action/Visit/DomainVisitsActionTest.php new file mode 100644 index 00000000..84acb1f1 --- /dev/null +++ b/module/Rest/test/Action/Visit/DomainVisitsActionTest.php @@ -0,0 +1,60 @@ +visitsHelper = $this->prophesize(VisitsStatsHelperInterface::class); + $this->action = new DomainVisitsAction($this->visitsHelper->reveal(), 'the_default.com'); + } + + /** + * @test + * @dataProvider provideDomainAuthorities + */ + public function providingCorrectDomainReturnsVisits(string $providedDomain, string $expectedDomain): void + { + $apiKey = ApiKey::create(); + $getVisits = $this->visitsHelper->visitsForDomain( + $expectedDomain, + Argument::type(VisitsParams::class), + $apiKey, + )->willReturn(new Paginator(new ArrayAdapter([]))); + + $response = $this->action->handle( + ServerRequestFactory::fromGlobals()->withAttribute('domain', $providedDomain) + ->withAttribute(ApiKey::class, $apiKey), + ); + + self::assertEquals(200, $response->getStatusCode()); + $getVisits->shouldHaveBeenCalledOnce(); + } + + public function provideDomainAuthorities(): iterable + { + yield 'no default domain' => ['foo.com', 'foo.com']; + yield 'default domain' => ['the_default.com', 'DEFAULT']; + yield 'DEFAULT keyword' => ['DEFAULT', 'DEFAULT']; + } +} From 54c1c7ad847b68619d197f99bd198a1579b27564 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 23 Apr 2022 11:17:32 +0200 Subject: [PATCH 8/9] Created DomainVisits API test --- .../Rest/test-api/Action/DomainVisitsTest.php | 68 +++++++++++++++++++ 1 file changed, 68 insertions(+) create mode 100644 module/Rest/test-api/Action/DomainVisitsTest.php diff --git a/module/Rest/test-api/Action/DomainVisitsTest.php b/module/Rest/test-api/Action/DomainVisitsTest.php new file mode 100644 index 00000000..b6e29a12 --- /dev/null +++ b/module/Rest/test-api/Action/DomainVisitsTest.php @@ -0,0 +1,68 @@ +callApiWithKey(self::METHOD_GET, sprintf('/domains/%s/visits', $domain), [ + RequestOptions::QUERY => $excludeBots ? ['excludeBots' => true] : [], + ], $apiKey); + $payload = $this->getJsonResponsePayload($resp); + + self::assertEquals(self::STATUS_OK, $resp->getStatusCode()); + self::assertArrayHasKey('visits', $payload); + self::assertArrayHasKey('data', $payload['visits']); + self::assertCount($expectedVisitsAmount, $payload['visits']['data']); + } + + public function provideDomains(): iterable + { + yield 'example.com with admin API key' => ['valid_api_key', 'example.com', false, 0]; + yield 'DEFAULT with admin API key' => ['valid_api_key', 'DEFAULT', false, 7]; + yield 'DEFAULT with admin API key and no bots' => ['valid_api_key', 'DEFAULT', true, 6]; + yield 'DEFAULT with domain API key' => ['domain_api_key', 'DEFAULT', false, 0]; + yield 'DEFAULT with author API key' => ['author_api_key', 'DEFAULT', false, 5]; + yield 'DEFAULT with author API key and no bots' => ['author_api_key', 'DEFAULT', true, 4]; + } + + /** + * @test + * @dataProvider provideApiKeysAndTags + */ + public function notFoundErrorIsReturnedForInvalidTags(string $apiKey, string $domain): void + { + $resp = $this->callApiWithKey(self::METHOD_GET, sprintf('/domains/%s/visits', $domain), [], $apiKey); + $payload = $this->getJsonResponsePayload($resp); + + self::assertEquals(self::STATUS_NOT_FOUND, $resp->getStatusCode()); + self::assertEquals(self::STATUS_NOT_FOUND, $payload['status']); + self::assertEquals('DOMAIN_NOT_FOUND', $payload['type']); + self::assertEquals(sprintf('Domain with authority "%s" could not be found', $domain), $payload['detail']); + self::assertEquals('Domain not found', $payload['title']); + self::assertEquals($domain, $payload['authority']); + } + + public function provideApiKeysAndTags(): iterable + { + yield 'admin API key with invalid domain' => ['valid_api_key', 'invalid_domain.com']; + yield 'domain API key with not-owned valid domain' => ['domain_api_key', 'this_domain_is_detached.com']; + yield 'author API key with valid domain not used in URLs' => ['author_api_key', 'this_domain_is_detached.com']; + } +} From 85c79abd30d222d28b2da532c32b8ecb555936e9 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 23 Apr 2022 11:19:14 +0200 Subject: [PATCH 9/9] Updated changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f674691c..5c1fc006 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this * [#1416](https://github.com/shlinkio/shlink/issues/1416) Added support to import URLs from Kutt.it. * [#1418](https://github.com/shlinkio/shlink/issues/1418) Added support to customize the timezone used by Shlink, falling back to the default one set in PHP config. * [#1309](https://github.com/shlinkio/shlink/issues/1309) Improved URL importing, ensuring individual errors do not make the whole process fail, and instead, failing URLs are skipped. +* [#1162](https://github.com/shlinkio/shlink/issues/1162) Added new endpoint to get visits by domain. + + The endpoint is `GET /domains/{domain}/visits`, and it has the same capabilities as any other visits endpoint, allowing pagination and filtering. ### Changed * [#1359](https://github.com/shlinkio/shlink/issues/1359) Hidden database commands.