Fixed geolocation by switching to different API

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

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

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,

module/Common/src/Service/IpApiLocationResolver.php

@@ -0,0 +1,51 @@
+<?php
+declare(strict_types=1);
+
+namespace Shlinkio\Shlink\Common\Service;
+
+use GuzzleHttp\Client;
+use GuzzleHttp\Exception\GuzzleException;
+use Shlinkio\Shlink\Common\Exception\WrongIpException;
+
+class IpApiLocationResolver implements IpLocationResolverInterface
+{
+    private const SERVICE_PATTERN = 'http://ip-api.com/json/%s';
+
+    /**
+     * @var Client
+     */
+    private $httpClient;
+
+    public function __construct(Client $httpClient)
+    {
+        $this->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'] ?? '',
+        ];
+    }
+}

module/Common/src/Service/IpLocationResolver.php

@@ -1,38 +0,0 @@
-<?php
-declare(strict_types=1);
-
-namespace Shlinkio\Shlink\Common\Service;
-
-use GuzzleHttp\Client;
-use GuzzleHttp\Exception\GuzzleException;
-use Shlinkio\Shlink\Common\Exception\WrongIpException;
-
-class IpLocationResolver implements IpLocationResolverInterface
-{
-    const SERVICE_PATTERN = 'http://freegeoip.net/json/%s';
-
-    /**
-     * @var Client
-     */
-    private $httpClient;
-
-    public function __construct(Client $httpClient)
-    {
-        $this->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);
-        }
-    }
-}

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',
+            'country_code' => 'bar',
-            'baz' => 'foo',
+            '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)
+        $this->client->get('http://ip-api.com/json/1.2.3.4')->willReturn($response)
-                                                               ->shouldBeCalledTimes(1);
+                                                            ->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())
+        $this->client->get('http://ip-api.com/json/1.2.3.4')->willThrow(new TransferException())
-                                                               ->shouldBeCalledTimes(1);
+                                                            ->shouldBeCalledTimes(1);
         $this->ipResolver->resolveIpLocation('1.2.3.4');
     }
 }