Merge pull request #2532 from acelaya-forks/improve-coverage

Remove dead code that is affecting code coverage

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

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(

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(

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(

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])),

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) => (

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())

module/Core/src/Spec/InDateRange.php

@@ -1,33 +0,0 @@
-<?php
-
-declare(strict_types=1);
-
-namespace Shlinkio\Shlink\Core\Spec;
-
-use Happyr\DoctrineSpecification\Spec;
-use Happyr\DoctrineSpecification\Specification\BaseSpecification;
-use Happyr\DoctrineSpecification\Specification\Specification;
-use Shlinkio\Shlink\Common\Util\DateRange;
-
-class InDateRange extends BaseSpecification
-{
-    public function __construct(private DateRange|null $dateRange, private string $field = 'date')
-    {
-        parent::__construct();
-    }
-
-    protected function getSpec(): Specification
-    {
-        $criteria = [];
-
-        if ($this->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);
-    }
-}

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

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

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) {

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

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

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

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(),