diff --git a/module/CLI/src/Command/Visit/ProcessVisitsCommand.php b/module/CLI/src/Command/Visit/ProcessVisitsCommand.php index 848a540c..f5b2efde 100644 --- a/module/CLI/src/Command/Visit/ProcessVisitsCommand.php +++ b/module/CLI/src/Command/Visit/ProcessVisitsCommand.php @@ -6,6 +6,7 @@ namespace Shlinkio\Shlink\CLI\Command\Visit; use Shlinkio\Shlink\CLI\Util\ExitCodes; use Shlinkio\Shlink\Common\Exception\WrongIpException; use Shlinkio\Shlink\Common\IpGeolocation\IpLocationResolverInterface; +use Shlinkio\Shlink\Common\IpGeolocation\Model\Location; use Shlinkio\Shlink\Common\Util\IpAddress; use Shlinkio\Shlink\Core\Entity\Visit; use Shlinkio\Shlink\Core\Entity\VisitLocation; @@ -75,7 +76,7 @@ class ProcessVisitsCommand extends Command } } - public function getGeolocationDataForVisit(Visit $visit): array + public function getGeolocationDataForVisit(Visit $visit): Location { if (! $visit->hasRemoteAddr()) { $this->output->writeln( diff --git a/module/CLI/test/Command/ShortUrl/GetVisitsCommandTest.php b/module/CLI/test/Command/ShortUrl/GetVisitsCommandTest.php index 6e0f830d..1d320bea 100644 --- a/module/CLI/test/Command/ShortUrl/GetVisitsCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/GetVisitsCommandTest.php @@ -8,6 +8,7 @@ use PHPUnit\Framework\TestCase; use Prophecy\Argument; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\CLI\Command\ShortUrl\GetVisitsCommand; +use Shlinkio\Shlink\Common\IpGeolocation\Model\Location; use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Entity\Visit; @@ -75,16 +76,14 @@ class GetVisitsCommandTest extends TestCase ]); } - /** - * @test - */ - public function outputIsProperlyGenerated() + /** @test */ + public function outputIsProperlyGenerated(): void { $shortCode = 'abc123'; $this->visitsTracker->info($shortCode, Argument::any())->willReturn( new Paginator(new ArrayAdapter([ (new Visit(new ShortUrl(''), new Visitor('bar', 'foo', '')))->locate( - new VisitLocation(['country_name' => 'Spain']) + new VisitLocation(new Location('', 'Spain', '', '', 0, 0, '')) ), ])) )->shouldBeCalledOnce(); diff --git a/module/CLI/test/Command/Visit/ProcessVisitsCommandTest.php b/module/CLI/test/Command/Visit/ProcessVisitsCommandTest.php index cd438637..7d4e015b 100644 --- a/module/CLI/test/Command/Visit/ProcessVisitsCommandTest.php +++ b/module/CLI/test/Command/Visit/ProcessVisitsCommandTest.php @@ -9,18 +9,17 @@ use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\CLI\Command\Visit\ProcessVisitsCommand; use Shlinkio\Shlink\Common\Exception\WrongIpException; use Shlinkio\Shlink\Common\IpGeolocation\IpApiLocationResolver; +use Shlinkio\Shlink\Common\IpGeolocation\Model\Location; 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\Exception\IpCannotBeLocatedException; use Shlinkio\Shlink\Core\Model\Visitor; use Shlinkio\Shlink\Core\Service\VisitService; use Symfony\Component\Console\Application; use Symfony\Component\Console\Output\OutputInterface; use Symfony\Component\Console\Tester\CommandTester; use Symfony\Component\Lock; -use Throwable; use function array_shift; use function sprintf; @@ -60,13 +59,11 @@ class ProcessVisitsCommandTest extends TestCase $this->commandTester = new CommandTester($command); } - /** - * @test - */ - public function allPendingVisitsAreProcessed() + /** @test */ + public function allPendingVisitsAreProcessed(): void { $visit = new Visit(new ShortUrl(''), new Visitor('', '', '1.2.3.4')); - $location = new VisitLocation([]); + $location = new VisitLocation(Location::emptyInstance()); $locateVisits = $this->visitService->locateVisits(Argument::cetera())->will( function (array $args) use ($visit, $location) { @@ -77,7 +74,9 @@ class ProcessVisitsCommandTest extends TestCase $secondCallback($location, $visit); } ); - $resolveIpLocation = $this->ipResolver->resolveIpLocation(Argument::any())->willReturn([]); + $resolveIpLocation = $this->ipResolver->resolveIpLocation(Argument::any())->willReturn( + Location::emptyInstance() + ); $this->commandTester->execute([ 'command' => 'visit:process', @@ -93,10 +92,10 @@ class ProcessVisitsCommandTest extends TestCase * @test * @dataProvider provideIgnoredAddresses */ - public function localhostAndEmptyAddressesAreIgnored(?string $address, string $message) + public function localhostAndEmptyAddressesAreIgnored(?string $address, string $message): void { $visit = new Visit(new ShortUrl(''), new Visitor('', '', $address)); - $location = new VisitLocation([]); + $location = new VisitLocation(Location::emptyInstance()); $locateVisits = $this->visitService->locateVisits(Argument::cetera())->will( function (array $args) use ($visit, $location) { @@ -107,39 +106,32 @@ class ProcessVisitsCommandTest extends TestCase $secondCallback($location, $visit); } ); - $resolveIpLocation = $this->ipResolver->resolveIpLocation(Argument::any())->willReturn([]); + $resolveIpLocation = $this->ipResolver->resolveIpLocation(Argument::any())->willReturn( + Location::emptyInstance() + ); - try { - $this->commandTester->execute([ - 'command' => 'visit:process', - ], ['verbosity' => OutputInterface::VERBOSITY_VERBOSE]); - } catch (Throwable $e) { - $output = $this->commandTester->getDisplay(); + $this->commandTester->execute([ + 'command' => 'visit:process', + ], ['verbosity' => OutputInterface::VERBOSITY_VERBOSE]); - $this->assertInstanceOf(IpCannotBeLocatedException::class, $e); - - $this->assertStringContainsString($message, $output); - $locateVisits->shouldHaveBeenCalledOnce(); - $resolveIpLocation->shouldNotHaveBeenCalled(); - } + $output = $this->commandTester->getDisplay(); + $this->assertStringContainsString($message, $output); + $locateVisits->shouldHaveBeenCalledOnce(); + $resolveIpLocation->shouldNotHaveBeenCalled(); } - public function provideIgnoredAddresses(): array + public function provideIgnoredAddresses(): iterable { - return [ - ['', 'Ignored visit with no IP address'], - [null, 'Ignored visit with no IP address'], - [IpAddress::LOCALHOST, 'Ignored localhost address'], - ]; + yield 'with empty address' => ['', 'Ignored visit with no IP address']; + yield 'with null address' => [null, 'Ignored visit with no IP address']; + yield 'with localhost address' => [IpAddress::LOCALHOST, 'Ignored localhost address']; } - /** - * @test - */ - public function errorWhileLocatingIpIsDisplayed() + /** @test */ + public function errorWhileLocatingIpIsDisplayed(): void { $visit = new Visit(new ShortUrl(''), new Visitor('', '', '1.2.3.4')); - $location = new VisitLocation([]); + $location = new VisitLocation(Location::emptyInstance()); $locateVisits = $this->visitService->locateVisits(Argument::cetera())->will( function (array $args) use ($visit, $location) { @@ -152,19 +144,15 @@ class ProcessVisitsCommandTest extends TestCase ); $resolveIpLocation = $this->ipResolver->resolveIpLocation(Argument::any())->willThrow(WrongIpException::class); - try { - $this->commandTester->execute([ - 'command' => 'visit:process', - ], ['verbosity' => OutputInterface::VERBOSITY_VERBOSE]); - } catch (Throwable $e) { - $output = $this->commandTester->getDisplay(); + $this->commandTester->execute([ + 'command' => 'visit:process', + ], ['verbosity' => OutputInterface::VERBOSITY_VERBOSE]); - $this->assertInstanceOf(IpCannotBeLocatedException::class, $e); + $output = $this->commandTester->getDisplay(); - $this->assertStringContainsString('An error occurred while locating IP. Skipped', $output); - $locateVisits->shouldHaveBeenCalledOnce(); - $resolveIpLocation->shouldHaveBeenCalledOnce(); - } + $this->assertStringContainsString('An error occurred while locating IP. Skipped', $output); + $locateVisits->shouldHaveBeenCalledOnce(); + $resolveIpLocation->shouldHaveBeenCalledOnce(); } /** diff --git a/module/Common/src/IpGeolocation/ChainIpLocationResolver.php b/module/Common/src/IpGeolocation/ChainIpLocationResolver.php index 6484b577..6d8659fa 100644 --- a/module/Common/src/IpGeolocation/ChainIpLocationResolver.php +++ b/module/Common/src/IpGeolocation/ChainIpLocationResolver.php @@ -18,7 +18,7 @@ class ChainIpLocationResolver implements IpLocationResolverInterface /** * @throws WrongIpException */ - public function resolveIpLocation(string $ipAddress): array + public function resolveIpLocation(string $ipAddress): Model\Location { $error = null; diff --git a/module/Common/src/IpGeolocation/EmptyIpLocationResolver.php b/module/Common/src/IpGeolocation/EmptyIpLocationResolver.php index c9e1f4a4..11aa9995 100644 --- a/module/Common/src/IpGeolocation/EmptyIpLocationResolver.php +++ b/module/Common/src/IpGeolocation/EmptyIpLocationResolver.php @@ -10,16 +10,8 @@ class EmptyIpLocationResolver implements IpLocationResolverInterface /** * @throws WrongIpException */ - public function resolveIpLocation(string $ipAddress): array + public function resolveIpLocation(string $ipAddress): Model\Location { - return [ - 'country_code' => '', - 'country_name' => '', - 'region_name' => '', - 'city' => '', - 'latitude' => '', - 'longitude' => '', - 'time_zone' => '', - ]; + return Model\Location::emptyInstance(); } } diff --git a/module/Common/src/IpGeolocation/GeoLite2LocationResolver.php b/module/Common/src/IpGeolocation/GeoLite2LocationResolver.php index c824f903..4aa4e868 100644 --- a/module/Common/src/IpGeolocation/GeoLite2LocationResolver.php +++ b/module/Common/src/IpGeolocation/GeoLite2LocationResolver.php @@ -24,7 +24,7 @@ class GeoLite2LocationResolver implements IpLocationResolverInterface /** * @throws WrongIpException */ - public function resolveIpLocation(string $ipAddress): array + public function resolveIpLocation(string $ipAddress): Model\Location { try { $city = $this->geoLiteDbReader->city($ipAddress); @@ -36,19 +36,19 @@ class GeoLite2LocationResolver implements IpLocationResolverInterface } } - private function mapFields(City $city): array + private function mapFields(City $city): Model\Location { /** @var Subdivision $region */ $region = first($city->subdivisions); - return [ - 'country_code' => $city->country->isoCode ?? '', - 'country_name' => $city->country->name ?? '', - 'region_name' => $region->name ?? '', - 'city' => $city->city->name ?? '', - 'latitude' => $city->location->latitude ?? '', - 'longitude' => $city->location->longitude ?? '', - 'time_zone' => $city->location->timeZone ?? '', - ]; + return new Model\Location( + $city->country->isoCode ?? '', + $city->country->name ?? '', + $region->name ?? '', + $city->city->name ?? '', + (float) ($city->location->latitude ?? ''), + (float) ($city->location->longitude ?? ''), + $city->location->timeZone ?? '' + ); } } diff --git a/module/Common/src/IpGeolocation/IpApiLocationResolver.php b/module/Common/src/IpGeolocation/IpApiLocationResolver.php index 25b6f774..0f9fc9a3 100644 --- a/module/Common/src/IpGeolocation/IpApiLocationResolver.php +++ b/module/Common/src/IpGeolocation/IpApiLocationResolver.php @@ -25,7 +25,7 @@ class IpApiLocationResolver implements IpLocationResolverInterface /** * @throws WrongIpException */ - public function resolveIpLocation(string $ipAddress): array + public function resolveIpLocation(string $ipAddress): Model\Location { try { $response = $this->httpClient->get(sprintf(self::SERVICE_PATTERN, $ipAddress)); @@ -37,16 +37,16 @@ class IpApiLocationResolver implements IpLocationResolverInterface } } - private function mapFields(array $entry): array + private function mapFields(array $entry): Model\Location { - return [ - 'country_code' => $entry['countryCode'] ?? '', - 'country_name' => $entry['country'] ?? '', - 'region_name' => $entry['regionName'] ?? '', - 'city' => $entry['city'] ?? '', - 'latitude' => $entry['lat'] ?? '', - 'longitude' => $entry['lon'] ?? '', - 'time_zone' => $entry['timezone'] ?? '', - ]; + return new Model\Location( + (string) ($entry['countryCode'] ?? ''), + (string) ($entry['country'] ?? ''), + (string) ($entry['regionName'] ?? ''), + (string) ($entry['city'] ?? ''), + (float) ($entry['lat'] ?? 0.0), + (float) ($entry['lon'] ?? 0.0), + (string) ($entry['timezone'] ?? '') + ); } } diff --git a/module/Common/src/IpGeolocation/IpLocationResolverInterface.php b/module/Common/src/IpGeolocation/IpLocationResolverInterface.php index 6017db61..ed73c0db 100644 --- a/module/Common/src/IpGeolocation/IpLocationResolverInterface.php +++ b/module/Common/src/IpGeolocation/IpLocationResolverInterface.php @@ -10,5 +10,5 @@ interface IpLocationResolverInterface /** * @throws WrongIpException */ - public function resolveIpLocation(string $ipAddress): array; + public function resolveIpLocation(string $ipAddress): Model\Location; } diff --git a/module/Common/src/IpGeolocation/Model/Location.php b/module/Common/src/IpGeolocation/Model/Location.php new file mode 100644 index 00000000..4ddb706e --- /dev/null +++ b/module/Common/src/IpGeolocation/Model/Location.php @@ -0,0 +1,80 @@ +countryCode = $countryCode; + $this->countryName = $countryName; + $this->regionName = $regionName; + $this->city = $city; + $this->latitude = $latitude; + $this->longitude = $longitude; + $this->timeZone = $timeZone; + } + + public static function emptyInstance(): self + { + return new self('', '', '', '', 0.0, 0.0, ''); + } + + public function countryCode(): string + { + return $this->countryCode; + } + + public function countryName(): string + { + return $this->countryName; + } + + public function regionName(): string + { + return $this->regionName; + } + + public function city(): string + { + return $this->city; + } + + public function latitude(): float + { + return $this->latitude; + } + + public function longitude(): float + { + return $this->longitude; + } + + public function timeZone(): string + { + return $this->timeZone; + } +} diff --git a/module/Common/test/IpGeolocation/ChainIpLocationResolverTest.php b/module/Common/test/IpGeolocation/ChainIpLocationResolverTest.php index 49bb0873..cbbe94df 100644 --- a/module/Common/test/IpGeolocation/ChainIpLocationResolverTest.php +++ b/module/Common/test/IpGeolocation/ChainIpLocationResolverTest.php @@ -8,6 +8,7 @@ use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\Common\Exception\WrongIpException; use Shlinkio\Shlink\Common\IpGeolocation\ChainIpLocationResolver; use Shlinkio\Shlink\Common\IpGeolocation\IpLocationResolverInterface; +use Shlinkio\Shlink\Common\IpGeolocation\Model\Location; class ChainIpLocationResolverTest extends TestCase { @@ -46,14 +47,12 @@ class ChainIpLocationResolverTest extends TestCase $this->resolver->resolveIpLocation($ipAddress); } - /** - * @test - */ - public function returnsResultOfFirstInnerResolver() + /** @test */ + public function returnsResultOfFirstInnerResolver(): void { $ipAddress = '1.2.3.4'; - $firstResolve = $this->firstInnerResolver->resolveIpLocation($ipAddress)->willReturn([]); + $firstResolve = $this->firstInnerResolver->resolveIpLocation($ipAddress)->willReturn(Location::emptyInstance()); $secondResolve = $this->secondInnerResolver->resolveIpLocation($ipAddress)->willThrow(WrongIpException::class); $this->resolver->resolveIpLocation($ipAddress); @@ -62,15 +61,15 @@ class ChainIpLocationResolverTest extends TestCase $secondResolve->shouldNotHaveBeenCalled(); } - /** - * @test - */ - public function returnsResultOfSecondInnerResolver() + /** @test */ + public function returnsResultOfSecondInnerResolver(): void { $ipAddress = '1.2.3.4'; $firstResolve = $this->firstInnerResolver->resolveIpLocation($ipAddress)->willThrow(WrongIpException::class); - $secondResolve = $this->secondInnerResolver->resolveIpLocation($ipAddress)->willReturn([]); + $secondResolve = $this->secondInnerResolver->resolveIpLocation($ipAddress)->willReturn( + Location::emptyInstance() + ); $this->resolver->resolveIpLocation($ipAddress); diff --git a/module/Common/test/IpGeolocation/EmptyIpLocationResolverTest.php b/module/Common/test/IpGeolocation/EmptyIpLocationResolverTest.php index 2c2962bb..283f332a 100644 --- a/module/Common/test/IpGeolocation/EmptyIpLocationResolverTest.php +++ b/module/Common/test/IpGeolocation/EmptyIpLocationResolverTest.php @@ -5,6 +5,7 @@ namespace ShlinkioTest\Shlink\Common\IpGeolocation; use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\Common\IpGeolocation\EmptyIpLocationResolver; +use Shlinkio\Shlink\Common\IpGeolocation\Model\Location; use Shlinkio\Shlink\Common\Util\StringUtilsTrait; use function Functional\map; use function range; @@ -13,16 +14,6 @@ class EmptyIpLocationResolverTest extends TestCase { use StringUtilsTrait; - private const EMPTY_RESP = [ - 'country_code' => '', - 'country_name' => '', - 'region_name' => '', - 'city' => '', - 'latitude' => '', - 'longitude' => '', - 'time_zone' => '', - ]; - /** @var EmptyIpLocationResolver */ private $resolver; @@ -35,15 +26,15 @@ class EmptyIpLocationResolverTest extends TestCase * @test * @dataProvider provideEmptyResponses */ - public function alwaysReturnsAnEmptyResponse(array $expected, string $ipAddress) + public function alwaysReturnsAnEmptyResponse(string $ipAddress): void { - $this->assertEquals($expected, $this->resolver->resolveIpLocation($ipAddress)); + $this->assertEquals(Location::emptyInstance(), $this->resolver->resolveIpLocation($ipAddress)); } public function provideEmptyResponses(): array { return map(range(0, 5), function () { - return [self::EMPTY_RESP, $this->generateRandomString(10)]; + return [$this->generateRandomString(15)]; }); } } diff --git a/module/Common/test/IpGeolocation/GeoLite2LocationResolverTest.php b/module/Common/test/IpGeolocation/GeoLite2LocationResolverTest.php index 2c5209f0..603d88ec 100644 --- a/module/Common/test/IpGeolocation/GeoLite2LocationResolverTest.php +++ b/module/Common/test/IpGeolocation/GeoLite2LocationResolverTest.php @@ -11,6 +11,7 @@ use PHPUnit\Framework\TestCase; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\Common\Exception\WrongIpException; use Shlinkio\Shlink\Common\IpGeolocation\GeoLite2LocationResolver; +use Shlinkio\Shlink\Common\IpGeolocation\Model\Location; class GeoLite2LocationResolverTest extends TestCase { @@ -51,10 +52,8 @@ class GeoLite2LocationResolverTest extends TestCase ]; } - /** - * @test - */ - public function resolvedCityIsProperlyMapped() + /** @test */ + public function resolvedCityIsProperlyMapped(): void { $ipAddress = '1.2.3.4'; $city = new City([]); @@ -63,15 +62,7 @@ class GeoLite2LocationResolverTest extends TestCase $result = $this->resolver->resolveIpLocation($ipAddress); - $this->assertEquals([ - 'country_code' => '', - 'country_name' => '', - 'region_name' => '', - 'city' => '', - 'latitude' => '', - 'longitude' => '', - 'time_zone' => '', - ], $result); + $this->assertEquals(Location::emptyInstance(), $result); $cityMethod->shouldHaveBeenCalledOnce(); } } diff --git a/module/Common/test/IpGeolocation/IpApiLocationResolverTest.php b/module/Common/test/IpGeolocation/IpApiLocationResolverTest.php index c830fc19..cc0c29b3 100644 --- a/module/Common/test/IpGeolocation/IpApiLocationResolverTest.php +++ b/module/Common/test/IpGeolocation/IpApiLocationResolverTest.php @@ -10,6 +10,7 @@ use PHPUnit\Framework\TestCase; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\Common\Exception\WrongIpException; use Shlinkio\Shlink\Common\IpGeolocation\IpApiLocationResolver; +use Shlinkio\Shlink\Common\IpGeolocation\Model\Location; use function json_encode; class IpApiLocationResolverTest extends TestCase @@ -25,25 +26,15 @@ class IpApiLocationResolverTest extends TestCase $this->ipResolver = new IpApiLocationResolver($this->client->reveal()); } - /** - * @test - */ - public function correctIpReturnsDecodedInfo() + /** @test */ + public function correctIpReturnsDecodedInfo(): void { $actual = [ 'countryCode' => 'bar', 'lat' => 5, 'lon' => 10, ]; - $expected = [ - 'country_code' => 'bar', - 'country_name' => '', - 'region_name' => '', - 'city' => '', - 'latitude' => 5, - 'longitude' => 10, - 'time_zone' => '', - ]; + $expected = new Location('bar', '', '', '', 5, 10, ''); $response = new Response(); $response->getBody()->write(json_encode($actual)); $response->getBody()->rewind(); @@ -54,7 +45,7 @@ class IpApiLocationResolverTest extends TestCase } /** @test */ - public function guzzleExceptionThrowsShlinkException() + public function guzzleExceptionThrowsShlinkException(): void { $this->client->get('http://ip-api.com/json/1.2.3.4')->willThrow(new TransferException()) ->shouldBeCalledOnce(); diff --git a/module/Core/src/Entity/VisitLocation.php b/module/Core/src/Entity/VisitLocation.php index ad68117b..b8376c01 100644 --- a/module/Core/src/Entity/VisitLocation.php +++ b/module/Core/src/Entity/VisitLocation.php @@ -4,8 +4,8 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Entity; use Shlinkio\Shlink\Common\Entity\AbstractEntity; +use Shlinkio\Shlink\Common\IpGeolocation\Model\Location; use Shlinkio\Shlink\Core\Visit\Model\VisitLocationInterface; -use function array_key_exists; class VisitLocation extends AbstractEntity implements VisitLocationInterface { @@ -24,9 +24,9 @@ class VisitLocation extends AbstractEntity implements VisitLocationInterface /** @var string */ private $timezone; - public function __construct(array $locationInfo) + public function __construct(Location $location) { - $this->exchangeArray($locationInfo); + $this->exchangeLocationInfo($location); } public function getCountryName(): string @@ -49,32 +49,15 @@ class VisitLocation extends AbstractEntity implements VisitLocationInterface return $this->cityName ?? ''; } - /** - * Exchange internal values from provided array - */ - private function exchangeArray(array $array): void + private function exchangeLocationInfo(Location $info): void { - if (array_key_exists('country_code', $array)) { - $this->countryCode = (string) $array['country_code']; - } - if (array_key_exists('country_name', $array)) { - $this->countryName = (string) $array['country_name']; - } - if (array_key_exists('region_name', $array)) { - $this->regionName = (string) $array['region_name']; - } - if (array_key_exists('city', $array)) { - $this->cityName = (string) $array['city']; - } - if (array_key_exists('latitude', $array)) { - $this->latitude = (string) $array['latitude']; - } - if (array_key_exists('longitude', $array)) { - $this->longitude = (string) $array['longitude']; - } - if (array_key_exists('time_zone', $array)) { - $this->timezone = (string) $array['time_zone']; - } + $this->countryCode = $info->countryCode(); + $this->countryName = $info->countryName(); + $this->regionName = $info->regionName(); + $this->cityName = $info->city(); + $this->latitude = (string) $info->latitude(); + $this->longitude = (string) $info->longitude(); + $this->timezone = $info->timeZone(); } public function jsonSerialize(): array diff --git a/module/Core/src/Service/VisitService.php b/module/Core/src/Service/VisitService.php index a9952e02..a56a4b7c 100644 --- a/module/Core/src/Service/VisitService.php +++ b/module/Core/src/Service/VisitService.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Service; use Doctrine\ORM\EntityManagerInterface; +use Shlinkio\Shlink\Common\IpGeolocation\Model\Location; use Shlinkio\Shlink\Core\Entity\Visit; use Shlinkio\Shlink\Core\Entity\VisitLocation; use Shlinkio\Shlink\Core\Exception\IpCannotBeLocatedException; @@ -19,7 +20,7 @@ class VisitService implements VisitServiceInterface $this->em = $em; } - public function locateVisits(callable $getGeolocationData, ?callable $locatedVisit = null): void + public function locateVisits(callable $geolocateVisit, ?callable $notifyVisitWithLocation = null): void { /** @var VisitRepository $repo */ $repo = $this->em->getRepository(Visit::class); @@ -27,26 +28,27 @@ class VisitService implements VisitServiceInterface foreach ($results as [$visit]) { try { - $locationData = $getGeolocationData($visit); + /** @var Location $location */ + $location = $geolocateVisit($visit); } catch (IpCannotBeLocatedException $e) { // Skip if the visit's IP could not be located continue; } - $location = new VisitLocation($locationData); - $this->locateVisit($visit, $location, $locatedVisit); + $location = new VisitLocation($location); + $this->locateVisit($visit, $location, $notifyVisitWithLocation); } } - private function locateVisit(Visit $visit, VisitLocation $location, ?callable $locatedVisit): void + private function locateVisit(Visit $visit, VisitLocation $location, ?callable $notifyVisitWithLocation): void { $visit->locate($location); $this->em->persist($visit); $this->em->flush(); - if ($locatedVisit !== null) { - $locatedVisit($location, $visit); + if ($notifyVisitWithLocation !== null) { + $notifyVisitWithLocation($location, $visit); } $this->em->clear(); diff --git a/module/Core/src/Service/VisitServiceInterface.php b/module/Core/src/Service/VisitServiceInterface.php index 907729fa..23dd273d 100644 --- a/module/Core/src/Service/VisitServiceInterface.php +++ b/module/Core/src/Service/VisitServiceInterface.php @@ -5,5 +5,5 @@ namespace Shlinkio\Shlink\Core\Service; interface VisitServiceInterface { - public function locateVisits(callable $getGeolocationData, ?callable $locatedVisit = null): void; + public function locateVisits(callable $geolocateVisit, ?callable $notifyVisitWithLocation = null): void; } diff --git a/module/Core/test-db/Repository/VisitRepositoryTest.php b/module/Core/test-db/Repository/VisitRepositoryTest.php index a9ee6f35..db797920 100644 --- a/module/Core/test-db/Repository/VisitRepositoryTest.php +++ b/module/Core/test-db/Repository/VisitRepositoryTest.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace ShlinkioTest\Shlink\Core\Repository; use Cake\Chronos\Chronos; +use Shlinkio\Shlink\Common\IpGeolocation\Model\Location; use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Entity\Visit; @@ -29,10 +30,8 @@ class VisitRepositoryTest extends DatabaseTestCase $this->repo = $this->getEntityManager()->getRepository(Visit::class); } - /** - * @test - */ - public function findUnlocatedVisitsReturnsProperVisits() + /** @test */ + public function findUnlocatedVisitsReturnsProperVisits(): void { $shortUrl = new ShortUrl(''); $this->getEntityManager()->persist($shortUrl); @@ -41,7 +40,7 @@ class VisitRepositoryTest extends DatabaseTestCase $visit = new Visit($shortUrl, Visitor::emptyInstance()); if ($i % 2 === 0) { - $location = new VisitLocation([]); + $location = new VisitLocation(Location::emptyInstance()); $this->getEntityManager()->persist($location); $visit->locate($location); } @@ -59,10 +58,8 @@ class VisitRepositoryTest extends DatabaseTestCase $this->assertEquals(3, $resultsCount); } - /** - * @test - */ - public function findVisitsByShortCodeReturnsProperData() + /** @test */ + public function findVisitsByShortCodeReturnsProperData(): void { $shortUrl = new ShortUrl(''); $this->getEntityManager()->persist($shortUrl); @@ -86,10 +83,8 @@ class VisitRepositoryTest extends DatabaseTestCase $this->assertCount(2, $this->repo->findVisitsByShortCode($shortUrl->getShortCode(), null, 5, 4)); } - /** - * @test - */ - public function countVisitsByShortCodeReturnsProperData() + /** @test */ + public function countVisitsByShortCodeReturnsProperData(): void { $shortUrl = new ShortUrl(''); $this->getEntityManager()->persist($shortUrl); diff --git a/module/Core/test/Entity/VisitLocationTest.php b/module/Core/test/Entity/VisitLocationTest.php index 17d4045e..6e008e15 100644 --- a/module/Core/test/Entity/VisitLocationTest.php +++ b/module/Core/test/Entity/VisitLocationTest.php @@ -4,20 +4,15 @@ declare(strict_types=1); namespace ShlinkioTest\Shlink\Core\Entity; use PHPUnit\Framework\TestCase; +use Shlinkio\Shlink\Common\IpGeolocation\Model\Location; use Shlinkio\Shlink\Core\Entity\VisitLocation; class VisitLocationTest extends TestCase { - /** - * @test - */ - public function valuesFoundWhenExchangingArrayAreCastToString() + /** @test */ + public function valuesFoundWhenExchangingArrayAreCastToString(): void { - $payload = [ - 'latitude' => 1000.7, - 'longitude' => -2000.4, - ]; - + $payload = new Location('', '', '', '', 1000.7, -2000.4, ''); $location = new VisitLocation($payload); $this->assertSame('1000.7', $location->getLatitude()); diff --git a/module/Core/test/Service/VisitServiceTest.php b/module/Core/test/Service/VisitServiceTest.php index 8d8753f0..0fd1da79 100644 --- a/module/Core/test/Service/VisitServiceTest.php +++ b/module/Core/test/Service/VisitServiceTest.php @@ -7,6 +7,7 @@ use Doctrine\ORM\EntityManager; use PHPUnit\Framework\TestCase; use Prophecy\Argument; use Prophecy\Prophecy\ObjectProphecy; +use Shlinkio\Shlink\Common\IpGeolocation\Model\Location; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Entity\Visit; use Shlinkio\Shlink\Core\Entity\VisitLocation; @@ -31,10 +32,8 @@ class VisitServiceTest extends TestCase $this->visitService = new VisitService($this->em->reveal()); } - /** - * @test - */ - public function locateVisitsIteratesAndLocatesUnlocatedVisits() + /** @test */ + public function locateVisitsIteratesAndLocatesUnlocatedVisits(): void { $unlocatedVisits = [ [new Visit(new ShortUrl('foo'), Visitor::emptyInstance())], @@ -53,7 +52,7 @@ class VisitServiceTest extends TestCase }); $this->visitService->locateVisits(function () { - return []; + return Location::emptyInstance(); }, function () { $args = func_get_args();