From 0604237b94e46178adbfb26605b377332cfcd14a Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 17 Nov 2025 12:12:06 +0100 Subject: [PATCH] Remove dead code that is affecting code coverage --- composer.json | 4 +-- .../Domain/GetDomainVisitsCommandTest.php | 2 +- .../ShortUrl/GetShortUrlVisitsCommandTest.php | 2 +- .../Command/Tag/GetTagVisitsCommandTest.php | 2 +- .../Visit/GetNonOrphanVisitsCommandTest.php | 2 +- .../Visit/GetOrphanVisitsCommandTest.php | 2 +- .../Command/Visit/LocateVisitsCommandTest.php | 6 ++-- module/Core/src/Spec/InDateRange.php | 33 ------------------- module/Core/src/Visit/Entity/Visit.php | 4 +-- .../Core/src/Visit/Entity/VisitLocation.php | 19 ++--------- .../src/Visit/Geolocation/VisitLocator.php | 2 +- .../VisitIterationRepositoryTest.php | 2 +- .../test/Matomo/MatomoVisitSenderTest.php | 2 +- .../test/Visit/Entity/VisitLocationTest.php | 28 +++++++++++++++- module/Core/test/Visit/Entity/VisitTest.php | 2 +- 15 files changed, 46 insertions(+), 66 deletions(-) delete mode 100644 module/Core/src/Spec/InDateRange.php diff --git a/composer.json b/composer.json index a4669587..78200845 100644 --- a/composer.json +++ b/composer.json @@ -145,12 +145,12 @@ "test:cli:ci": [ "@putenv GENERATE_COVERAGE=yes", "@test:cli", - "vendor/bin/phpcov merge build/coverage-cli --php build/coverage-cli.cov && rm build/coverage-cli/*.cov" + "@php -d memory_limit=-1 vendor/bin/phpcov merge build/coverage-cli --php build/coverage-cli.cov && rm build/coverage-cli/*.cov" ], "test:cli:pretty": [ "@putenv GENERATE_COVERAGE=yes", "@test:cli", - "phpcov merge build/coverage-cli --html build/coverage-cli/coverage-html && rm build/coverage-cli/*.cov" + "@php -d memory_limit=-1 phpcov merge build/coverage-cli --html build/coverage-cli/coverage-html && rm build/coverage-cli/*.cov" ], "openapi:validate": "php-openapi validate docs/swagger/swagger.json", "openapi:inline": "php-openapi inline docs/swagger/swagger.json docs/swagger/openapi-inlined.json", diff --git a/module/CLI/test/Command/Domain/GetDomainVisitsCommandTest.php b/module/CLI/test/Command/Domain/GetDomainVisitsCommandTest.php index e174a3b0..ddf55283 100644 --- a/module/CLI/test/Command/Domain/GetDomainVisitsCommandTest.php +++ b/module/CLI/test/Command/Domain/GetDomainVisitsCommandTest.php @@ -41,7 +41,7 @@ class GetDomainVisitsCommandTest extends TestCase { $shortUrl = ShortUrl::createFake(); $visit = Visit::forValidShortUrl($shortUrl, Visitor::fromParams('bar', 'foo', ''))->locate( - VisitLocation::fromGeolocation(new Location('', 'Spain', '', 'Madrid', 0, 0, '')), + VisitLocation::fromLocation(new Location('', 'Spain', '', 'Madrid', 0, 0, '')), ); $domain = 's.test'; $this->visitsHelper->expects($this->once())->method('visitsForDomain')->with( diff --git a/module/CLI/test/Command/ShortUrl/GetShortUrlVisitsCommandTest.php b/module/CLI/test/Command/ShortUrl/GetShortUrlVisitsCommandTest.php index a1905e38..709974ec 100644 --- a/module/CLI/test/Command/ShortUrl/GetShortUrlVisitsCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/GetShortUrlVisitsCommandTest.php @@ -94,7 +94,7 @@ class GetShortUrlVisitsCommandTest extends TestCase public function outputIsProperlyGenerated(): void { $visit = Visit::forValidShortUrl(ShortUrl::createFake(), Visitor::fromParams('bar', 'foo', ''))->locate( - VisitLocation::fromGeolocation(new Location('', 'Spain', '', 'Madrid', 0, 0, '')), + VisitLocation::fromLocation(new Location('', 'Spain', '', 'Madrid', 0, 0, '')), ); $shortCode = 'abc123'; $this->visitsHelper->expects($this->once())->method('visitsForShortUrl')->with( diff --git a/module/CLI/test/Command/Tag/GetTagVisitsCommandTest.php b/module/CLI/test/Command/Tag/GetTagVisitsCommandTest.php index 08ca2cd3..30580951 100644 --- a/module/CLI/test/Command/Tag/GetTagVisitsCommandTest.php +++ b/module/CLI/test/Command/Tag/GetTagVisitsCommandTest.php @@ -41,7 +41,7 @@ class GetTagVisitsCommandTest extends TestCase { $shortUrl = ShortUrl::createFake(); $visit = Visit::forValidShortUrl($shortUrl, Visitor::fromParams('bar', 'foo', ''))->locate( - VisitLocation::fromGeolocation(new Location('', 'Spain', '', 'Madrid', 0, 0, '')), + VisitLocation::fromLocation(new Location('', 'Spain', '', 'Madrid', 0, 0, '')), ); $tag = 'abc123'; $this->visitsHelper->expects($this->once())->method('visitsForTag')->with($tag, $this->anything())->willReturn( diff --git a/module/CLI/test/Command/Visit/GetNonOrphanVisitsCommandTest.php b/module/CLI/test/Command/Visit/GetNonOrphanVisitsCommandTest.php index 4ebe780f..f583d063 100644 --- a/module/CLI/test/Command/Visit/GetNonOrphanVisitsCommandTest.php +++ b/module/CLI/test/Command/Visit/GetNonOrphanVisitsCommandTest.php @@ -41,7 +41,7 @@ class GetNonOrphanVisitsCommandTest extends TestCase { $shortUrl = ShortUrl::createFake(); $visit = Visit::forValidShortUrl($shortUrl, Visitor::fromParams('bar', 'foo', ''))->locate( - VisitLocation::fromGeolocation(new Location('', 'Spain', '', 'Madrid', 0, 0, '')), + VisitLocation::fromLocation(new Location('', 'Spain', '', 'Madrid', 0, 0, '')), ); $this->visitsHelper->expects($this->once())->method('nonOrphanVisits')->withAnyParameters()->willReturn( new Paginator(new ArrayAdapter([$visit])), diff --git a/module/CLI/test/Command/Visit/GetOrphanVisitsCommandTest.php b/module/CLI/test/Command/Visit/GetOrphanVisitsCommandTest.php index 33a98448..bf453c65 100644 --- a/module/CLI/test/Command/Visit/GetOrphanVisitsCommandTest.php +++ b/module/CLI/test/Command/Visit/GetOrphanVisitsCommandTest.php @@ -38,7 +38,7 @@ class GetOrphanVisitsCommandTest extends TestCase public function outputIsProperlyGenerated(array $args, bool $includesType): void { $visit = Visit::forBasePath(Visitor::fromParams('bar', 'foo', ''))->locate( - VisitLocation::fromGeolocation(new Location('', 'Spain', '', 'Madrid', 0, 0, '')), + VisitLocation::fromLocation(new Location('', 'Spain', '', 'Madrid', 0, 0, '')), ); $this->visitsHelper->expects($this->once())->method('orphanVisits')->with($this->callback( fn (OrphanVisitsParams $param) => ( diff --git a/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php b/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php index 1f34baef..ca7db909 100644 --- a/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php +++ b/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php @@ -63,7 +63,7 @@ class LocateVisitsCommandTest extends TestCase array $args, ): void { $visit = Visit::forValidShortUrl(ShortUrl::createFake(), Visitor::fromParams('', '', '1.2.3.4')); - $location = VisitLocation::fromGeolocation(Location::empty()); + $location = VisitLocation::fromLocation(Location::empty()); $mockMethodBehavior = $this->invokeHelperMethods($visit, $location); $this->lock->method('acquire')->willReturn(true); @@ -107,7 +107,7 @@ class LocateVisitsCommandTest extends TestCase public function localhostAndEmptyAddressesAreIgnored(IpCannotBeLocatedException $e, string $message): void { $visit = Visit::forValidShortUrl(ShortUrl::createFake(), Visitor::empty()); - $location = VisitLocation::fromGeolocation(Location::empty()); + $location = VisitLocation::fromLocation(Location::empty()); $this->lock->method('acquire')->willReturn(true); $this->visitService->expects($this->once()) @@ -134,7 +134,7 @@ class LocateVisitsCommandTest extends TestCase public function errorWhileLocatingIpIsDisplayed(): void { $visit = Visit::forValidShortUrl(ShortUrl::createFake(), Visitor::fromParams(remoteAddress: '1.2.3.4')); - $location = VisitLocation::fromGeolocation(Location::emptyInstance()); + $location = VisitLocation::fromLocation(Location::empty()); $this->lock->method('acquire')->willReturn(true); $this->visitService->expects($this->once()) diff --git a/module/Core/src/Spec/InDateRange.php b/module/Core/src/Spec/InDateRange.php deleted file mode 100644 index d373a59d..00000000 --- a/module/Core/src/Spec/InDateRange.php +++ /dev/null @@ -1,33 +0,0 @@ -dateRange?->startDate !== null) { - $criteria[] = Spec::gte($this->field, $this->dateRange->startDate->toDateTimeString()); - } - - if ($this->dateRange?->endDate !== null) { - $criteria[] = Spec::lte($this->field, $this->dateRange->endDate->toDateTimeString()); - } - - return Spec::andX(...$criteria); - } -} diff --git a/module/Core/src/Visit/Entity/Visit.php b/module/Core/src/Visit/Entity/Visit.php index 70733593..b396bc07 100644 --- a/module/Core/src/Visit/Entity/Visit.php +++ b/module/Core/src/Visit/Entity/Visit.php @@ -70,7 +70,7 @@ class Visit extends AbstractEntity implements JsonSerializable remoteAddr: self::processAddress($visitor->remoteAddress, $anonymize), visitedUrl: $visitor->visitedUrl, redirectUrl: $visitor->redirectUrl, - visitLocation: $geolocation !== null ? VisitLocation::fromGeolocation($geolocation) : null, + visitLocation: $geolocation !== null ? VisitLocation::fromLocation($geolocation) : null, ); } @@ -114,7 +114,7 @@ class Visit extends AbstractEntity implements JsonSerializable referer: $importedVisit->referer, potentialBot: isCrawler($importedVisit->userAgent), visitedUrl: $importedVisit instanceof ImportedShlinkOrphanVisit ? $importedVisit->visitedUrl : null, - visitLocation: $importedLocation !== null ? VisitLocation::fromImport($importedLocation) : null, + visitLocation: $importedLocation !== null ? VisitLocation::fromLocation($importedLocation) : null, date: normalizeDate($importedVisit->date), ); } diff --git a/module/Core/src/Visit/Entity/VisitLocation.php b/module/Core/src/Visit/Entity/VisitLocation.php index 1f1db686..dcb56698 100644 --- a/module/Core/src/Visit/Entity/VisitLocation.php +++ b/module/Core/src/Visit/Entity/VisitLocation.php @@ -33,29 +33,16 @@ class VisitLocation extends AbstractEntity implements JsonSerializable ); } - public static function fromGeolocation(Location $location): self + public static function fromLocation(Location|ImportedShlinkVisitLocation $location): self { return new self( countryCode: $location->countryCode, countryName: $location->countryName, regionName: $location->regionName, - cityName: $location->city, + cityName: $location instanceof Location ? $location->city : $location->cityName, latitude: $location->latitude, longitude: $location->longitude, - timezone: $location->timeZone, - ); - } - - public static function fromImport(ImportedShlinkVisitLocation $location): self - { - return new self( - countryCode: $location->countryCode, - countryName: $location->countryName, - regionName: $location->regionName, - cityName: $location->cityName, - latitude: $location->latitude, - longitude: $location->longitude, - timezone: $location->timezone, + timezone: $location instanceof Location ? $location->timeZone : $location->timezone, ); } diff --git a/module/Core/src/Visit/Geolocation/VisitLocator.php b/module/Core/src/Visit/Geolocation/VisitLocator.php index 8f69ba2c..48245a29 100644 --- a/module/Core/src/Visit/Geolocation/VisitLocator.php +++ b/module/Core/src/Visit/Geolocation/VisitLocator.php @@ -57,7 +57,7 @@ readonly class VisitLocator implements VisitLocatorInterface $location = Location::empty(); } - $this->locateVisit($visit, VisitLocation::fromGeolocation($location), $helper); + $this->locateVisit($visit, VisitLocation::fromLocation($location), $helper); // Flush and clear after X iterations if ($count % $persistBlock === 0) { diff --git a/module/Core/test-db/Visit/Repository/VisitIterationRepositoryTest.php b/module/Core/test-db/Visit/Repository/VisitIterationRepositoryTest.php index 60c2fbea..88f273c2 100644 --- a/module/Core/test-db/Visit/Repository/VisitIterationRepositoryTest.php +++ b/module/Core/test-db/Visit/Repository/VisitIterationRepositoryTest.php @@ -40,7 +40,7 @@ class VisitIterationRepositoryTest extends DatabaseTestCase $visit = Visit::forValidShortUrl($shortUrl, Visitor::empty()); if ($i >= 2) { - $location = VisitLocation::fromGeolocation(Location::emptyInstance()); + $location = VisitLocation::fromLocation(Location::empty()); $this->getEntityManager()->persist($location); $visit->locate($location); } diff --git a/module/Core/test/Matomo/MatomoVisitSenderTest.php b/module/Core/test/Matomo/MatomoVisitSenderTest.php index 1b1b303c..0acc535b 100644 --- a/module/Core/test/Matomo/MatomoVisitSenderTest.php +++ b/module/Core/test/Matomo/MatomoVisitSenderTest.php @@ -87,7 +87,7 @@ class MatomoVisitSenderTest extends TestCase yield 'unlocated orphan visit' => [Visit::forBasePath(Visitor::empty()), null, []]; yield 'located regular visit' => [ Visit::forValidShortUrl(ShortUrl::withLongUrl('https://shlink.io'), Visitor::empty()) - ->locate(VisitLocation::fromGeolocation(new Location( + ->locate(VisitLocation::fromLocation(new Location( countryCode: 'US', countryName: 'countryName', regionName: 'regionName', diff --git a/module/Core/test/Visit/Entity/VisitLocationTest.php b/module/Core/test/Visit/Entity/VisitLocationTest.php index 5f5c458d..791eabe9 100644 --- a/module/Core/test/Visit/Entity/VisitLocationTest.php +++ b/module/Core/test/Visit/Entity/VisitLocationTest.php @@ -8,6 +8,7 @@ use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\Core\Visit\Entity\VisitLocation; +use Shlinkio\Shlink\Importer\Model\ImportedShlinkVisitLocation; use Shlinkio\Shlink\IpGeolocation\Model\Location; class VisitLocationTest extends TestCase @@ -16,7 +17,7 @@ class VisitLocationTest extends TestCase public function isEmptyReturnsTrueWhenAllValuesAreEmpty(array $args, bool $isEmpty): void { $payload = new Location(...$args); - $location = VisitLocation::fromGeolocation($payload); + $location = VisitLocation::fromLocation($payload); self::assertEquals($isEmpty, $location->isEmpty); } @@ -32,4 +33,29 @@ class VisitLocationTest extends TestCase yield [['', '', '', '', 1.0, 0.0, ''], false]; yield [['', '', '', '', 0.0, 1.0, ''], false]; } + + #[Test] + public function jsonSerialization(): void + { + $location = VisitLocation::fromLocation(new ImportedShlinkVisitLocation( + countryCode: 'countryCode', + countryName: 'countryName', + regionName: 'regionName', + cityName: 'cityName', + timezone: 'timezone', + latitude: 1, + longitude: 2, + )); + + self::assertEquals([ + 'countryCode' => $location->countryCode, + 'countryName' => $location->countryName, + 'regionName' => $location->regionName, + 'cityName' => $location->cityName, + 'latitude' => $location->latitude, + 'longitude' => $location->longitude, + 'timezone' => $location->timezone, + 'isEmpty' => $location->isEmpty, + ], $location->jsonSerialize()); + } } diff --git a/module/Core/test/Visit/Entity/VisitTest.php b/module/Core/test/Visit/Entity/VisitTest.php index 438ca55f..14f36551 100644 --- a/module/Core/test/Visit/Entity/VisitTest.php +++ b/module/Core/test/Visit/Entity/VisitTest.php @@ -95,7 +95,7 @@ class VisitTest extends TestCase ->withHeader('Referer', 'referer') ->withUri(new Uri('https://s.test/foo/bar')), ), - )->locate($location = VisitLocation::fromGeolocation(Location::emptyInstance())), + )->locate($location = VisitLocation::fromLocation(Location::empty())), [ 'referer' => 'referer', 'date' => $visit->date->toAtomString(),