From 899771cc2e10de36c3ffe301111a35c2fe9b0f4e Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 31 Jul 2018 20:24:13 +0200 Subject: [PATCH] Fixed geolocation by switching to different API --- module/CLI/config/dependencies.config.php | 4 +- .../Visit/ProcessVisitsCommandTest.php | 4 +- module/Common/config/dependencies.config.php | 4 +- .../src/Service/IpApiLocationResolver.php | 51 +++++++++++++++++++ .../Common/src/Service/IpLocationResolver.php | 38 -------------- ...Test.php => IpApiLocationResolverTest.php} | 32 ++++++++---- 6 files changed, 78 insertions(+), 55 deletions(-) create mode 100644 module/Common/src/Service/IpApiLocationResolver.php delete mode 100644 module/Common/src/Service/IpLocationResolver.php rename module/Common/test/Service/{IpLocationResolverTest.php => IpApiLocationResolverTest.php} (50%) diff --git a/module/CLI/config/dependencies.config.php b/module/CLI/config/dependencies.config.php index 6f692af7..4dd461be 100644 --- a/module/CLI/config/dependencies.config.php +++ b/module/CLI/config/dependencies.config.php @@ -3,7 +3,7 @@ declare(strict_types=1); use Shlinkio\Shlink\CLI\Command; use Shlinkio\Shlink\CLI\Factory\ApplicationFactory; -use Shlinkio\Shlink\Common\Service\IpLocationResolver; +use Shlinkio\Shlink\Common\Service\IpApiLocationResolver; use Shlinkio\Shlink\Common\Service\PreviewGenerator; use Shlinkio\Shlink\Core\Service; use Shlinkio\Shlink\Rest\Service\ApiKeyService; @@ -51,7 +51,7 @@ return [ ], Command\Visit\ProcessVisitsCommand::class => [ Service\VisitService::class, - IpLocationResolver::class, + IpApiLocationResolver::class, 'translator', ], Command\Config\GenerateCharsetCommand::class => ['translator'], diff --git a/module/CLI/test/Command/Visit/ProcessVisitsCommandTest.php b/module/CLI/test/Command/Visit/ProcessVisitsCommandTest.php index c80f5223..bc62124f 100644 --- a/module/CLI/test/Command/Visit/ProcessVisitsCommandTest.php +++ b/module/CLI/test/Command/Visit/ProcessVisitsCommandTest.php @@ -7,7 +7,7 @@ use PHPUnit\Framework\TestCase; use Prophecy\Argument; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\CLI\Command\Visit\ProcessVisitsCommand; -use Shlinkio\Shlink\Common\Service\IpLocationResolver; +use Shlinkio\Shlink\Common\Service\IpApiLocationResolver; use Shlinkio\Shlink\Core\Entity\Visit; use Shlinkio\Shlink\Core\Service\VisitService; use Symfony\Component\Console\Application; @@ -32,7 +32,7 @@ class ProcessVisitsCommandTest extends TestCase public function setUp() { $this->visitService = $this->prophesize(VisitService::class); - $this->ipResolver = $this->prophesize(IpLocationResolver::class); + $this->ipResolver = $this->prophesize(IpApiLocationResolver::class); $command = new ProcessVisitsCommand( $this->visitService->reveal(), $this->ipResolver->reveal(), diff --git a/module/Common/config/dependencies.config.php b/module/Common/config/dependencies.config.php index 19093549..e9422d38 100644 --- a/module/Common/config/dependencies.config.php +++ b/module/Common/config/dependencies.config.php @@ -32,7 +32,7 @@ return [ Image\ImageBuilder::class => Image\ImageBuilderFactory::class, - Service\IpLocationResolver::class => ConfigAbstractFactory::class, + Service\IpApiLocationResolver::class => ConfigAbstractFactory::class, Service\PreviewGenerator::class => ConfigAbstractFactory::class, ], 'aliases' => [ @@ -51,7 +51,7 @@ return [ ConfigAbstractFactory::class => [ TranslatorExtension::class => ['translator'], LocaleMiddleware::class => ['translator'], - Service\IpLocationResolver::class => ['httpClient'], + Service\IpApiLocationResolver::class => ['httpClient'], Service\PreviewGenerator::class => [ ImageBuilder::class, Filesystem::class, diff --git a/module/Common/src/Service/IpApiLocationResolver.php b/module/Common/src/Service/IpApiLocationResolver.php new file mode 100644 index 00000000..9787770c --- /dev/null +++ b/module/Common/src/Service/IpApiLocationResolver.php @@ -0,0 +1,51 @@ +httpClient = $httpClient; + } + + /** + * @param string $ipAddress + * @return array + * @throws WrongIpException + */ + public function resolveIpLocation(string $ipAddress): array + { + try { + $response = $this->httpClient->get(\sprintf(self::SERVICE_PATTERN, $ipAddress)); + return $this->mapFields(\json_decode((string) $response->getBody(), true)); + } catch (GuzzleException $e) { + throw WrongIpException::fromIpAddress($ipAddress, $e); + } + } + + private function mapFields(array $entry): array + { + 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'] ?? '', + ]; + } +} diff --git a/module/Common/src/Service/IpLocationResolver.php b/module/Common/src/Service/IpLocationResolver.php deleted file mode 100644 index 017bf89c..00000000 --- a/module/Common/src/Service/IpLocationResolver.php +++ /dev/null @@ -1,38 +0,0 @@ -httpClient = $httpClient; - } - - /** - * @param string $ipAddress - * @return array - * @throws WrongIpException - */ - public function resolveIpLocation(string $ipAddress): array - { - try { - $response = $this->httpClient->get(sprintf(self::SERVICE_PATTERN, $ipAddress)); - return json_decode((string) $response->getBody(), true); - } catch (GuzzleException $e) { - throw WrongIpException::fromIpAddress($ipAddress, $e); - } - } -} diff --git a/module/Common/test/Service/IpLocationResolverTest.php b/module/Common/test/Service/IpApiLocationResolverTest.php similarity index 50% rename from module/Common/test/Service/IpLocationResolverTest.php rename to module/Common/test/Service/IpApiLocationResolverTest.php index f5fb162c..1d36ce9a 100644 --- a/module/Common/test/Service/IpLocationResolverTest.php +++ b/module/Common/test/Service/IpApiLocationResolverTest.php @@ -8,12 +8,12 @@ use GuzzleHttp\Exception\TransferException; use GuzzleHttp\Psr7\Response; use PHPUnit\Framework\TestCase; use Prophecy\Prophecy\ObjectProphecy; -use Shlinkio\Shlink\Common\Service\IpLocationResolver; +use Shlinkio\Shlink\Common\Service\IpApiLocationResolver; -class IpLocationResolverTest extends TestCase +class IpApiLocationResolverTest extends TestCase { /** - * @var IpLocationResolver + * @var IpApiLocationResolver */ protected $ipResolver; /** @@ -24,7 +24,7 @@ class IpLocationResolverTest extends TestCase public function setUp() { $this->client = $this->prophesize(Client::class); - $this->ipResolver = new IpLocationResolver($this->client->reveal()); + $this->ipResolver = new IpApiLocationResolver($this->client->reveal()); } /** @@ -32,16 +32,26 @@ class IpLocationResolverTest extends TestCase */ public function correctIpReturnsDecodedInfo() { + $actual = [ + 'countryCode' => 'bar', + 'lat' => 5, + 'lon' => 10, + ]; $expected = [ - 'foo' => 'bar', - 'baz' => 'foo', + 'country_code' => 'bar', + 'country_name' => '', + 'region_name' => '', + 'city' => '', + 'latitude' => 5, + 'longitude' => 10, + 'time_zone' => '', ]; $response = new Response(); - $response->getBody()->write(json_encode($expected)); + $response->getBody()->write(\json_encode($actual)); $response->getBody()->rewind(); - $this->client->get('http://freegeoip.net/json/1.2.3.4')->willReturn($response) - ->shouldBeCalledTimes(1); + $this->client->get('http://ip-api.com/json/1.2.3.4')->willReturn($response) + ->shouldBeCalledTimes(1); $this->assertEquals($expected, $this->ipResolver->resolveIpLocation('1.2.3.4')); } @@ -51,8 +61,8 @@ class IpLocationResolverTest extends TestCase */ public function guzzleExceptionThrowsShlinkException() { - $this->client->get('http://freegeoip.net/json/1.2.3.4')->willThrow(new TransferException()) - ->shouldBeCalledTimes(1); + $this->client->get('http://ip-api.com/json/1.2.3.4')->willThrow(new TransferException()) + ->shouldBeCalledTimes(1); $this->ipResolver->resolveIpLocation('1.2.3.4'); } }