fix: ipapi-pro single lookup (#45)

* fix: ipapi-pro single lookup * Apply fixes from StyleCI --------- Co-authored-by: StyleCI Bot <[email protected]>
FriendsOfFlarum · Sep 14, 2024 · b2b6cad · b2b6cad
1 parent de4535c
commit b2b6cad
Show file tree

Hide file tree

Showing 8 changed files with 105 additions and 27 deletions.
diff --git a/extend.php b/extend.php
@@ -18,6 +18,7 @@
 use Flarum\Frontend\Document;
 use Flarum\Post\Post;
 use Flarum\Settings\Event\Saving;
+use Flarum\User\User;
 use FoF\GeoIP\Api\GeoIP;
 
 return [
@@ -37,7 +38,8 @@
     new Extend\Locales(__DIR__.'/resources/locale'),
 
     (new Extend\Event())
-        ->listen(Saving::class, Listeners\RemoveErrorsOnSettingsUpdate::class),
+        ->listen(Saving::class, Listeners\RemoveErrorsOnSettingsUpdate::class)
+        ->subscribe(Listeners\RetrieveIP::class),
 
     (new Extend\ApiSerializer(PostSerializer::class))
         ->relationship('ip_info', Api\AttachRelation::class),
@@ -72,7 +74,7 @@
         ->registerPreference('showIPCountry', 'boolval', false),
 
     (new Extend\ApiSerializer(BasicUserSerializer::class))
-        ->attribute('showIPCountry', function (BasicUserSerializer $serializer, $user) {
+        ->attribute('showIPCountry', function (BasicUserSerializer $serializer, User $user) {
             return (bool) $user->getPreference('showIPCountry');
         }),
 

diff --git a/src/Api/Services/BaseGeoService.php b/src/Api/Services/BaseGeoService.php
@@ -48,6 +48,11 @@ public function get(string $ip): ?ServiceResponse
     {
         $apiKey = $this->settings->get("{$this->settingPrefix}.access_key");
 
+        if (!empty($apiKey)) {
+            // ensure that we don't have any whitespace, etc in the key
+            $apiKey = trim($apiKey);
+        }
+
         if ($this->requiresApiKey() && !$apiKey) {
             $this->logger->error("No API key found for {$this->host}");
 
@@ -60,7 +65,13 @@ public function get(string $ip): ?ServiceResponse
 
         /** @phpstan-ignore-next-line */
         if (!$this->isRateLimited() || ($this->isRateLimited() && $this->singleLookupsRemaining > 0)) {
-            $response = $this->client->get($this->buildUrl($ip, $apiKey), $this->getRequestOptions($apiKey));
+            $url = $this->buildUrl($ip, $apiKey);
+            $options = $this->getRequestOptions($apiKey);
+            $response = $this->client->get($url, $options);
+
+            if ($response->getStatusCode() !== 200) {
+                $this->logger->error("Error detected in response from {$this->host}", ['status' => $response->getStatusCode(), 'body' => $response->getBody()->getContents(), 'requestUrl' => $url, 'requestOptions' => $options]);
+            }
 
             if ($this->isRateLimited()) {
                 $this->updateRateLimitsFromResponse($response);
@@ -108,10 +119,12 @@ public function getBatch(array $ips)
 
         /** @phpstan-ignore-next-line */
         if (!$this->isRateLimited() || ($this->isRateLimited() && $this->batchLookupsRemaining > 0)) {
-            $response = $this->client->post($this->buildBatchUrl($ips, $apiKey), $this->getRequestOptions($apiKey, $ips));
+            $url = $this->buildBatchUrl($ips, $apiKey);
+            $options = $this->getRequestOptions($apiKey, $ips);
+            $response = $this->client->post($url, $this->getRequestOptions($apiKey, $ips));
 
             if ($response->getStatusCode() !== 200) {
-                $this->logger->error("Error detected in response from {$this->host}", ['status' => $response->getStatusCode(), 'body' => $response->getBody()->getContents()]);
+                $this->logger->error("Error detected in response from {$this->host}", ['status' => $response->getStatusCode(), 'body' => $response->getBody()->getContents(), 'requestUrl' => $url, 'requestOptions' => $options]);
             }
 
             if ($this->isRateLimited()) {

diff --git a/src/Api/Services/IPApi.php b/src/Api/Services/IPApi.php
@@ -68,7 +68,7 @@ protected function buildUrl(string $ip, ?string $apiKey): string
 
     protected function buildBatchUrl(array $ips, ?string $apiKey): string
     {
-        return '/batch?'.http_build_query(['fields' => $this->r2]);
+        return '/batch';
     }
 
     protected function requiresApiKey(): bool
@@ -78,22 +78,21 @@ protected function requiresApiKey(): bool
 
     protected function getRequestOptions(?string $apiKey, array $ips = null): array
     {
+        $options = [];
+
+        $options['http_errors'] = false;
+        $options['query'] = [
+            'fields' => $this->requestFields,
+        ];
+
         if ($ips && is_array($ips)) {
             // array is key => value, we only want values, then encode to json
             $ips = array_values($ips);
 
-            return [
-                'http_errors' => false,
-                'json'        => $ips,
-            ];
+            $options['json'] = $ips;
         }
 
-        return [
-            'http_errors' => false,
-            'query'       => [
-                'fields' => $this->requestFields,
-            ],
-        ];
+        return $options;
     }
 
     protected function hasError(ResponseInterface $response, mixed $body): bool

diff --git a/src/Api/Services/IPApiPro.php b/src/Api/Services/IPApiPro.php
@@ -11,6 +11,10 @@
 
 namespace FoF\GeoIP\Api\Services;
 
+use FoF\GeoIP\Api\GeoIP;
+use FoF\GeoIP\Api\ServiceResponse;
+use Psr\Http\Message\ResponseInterface;
+
 class IPApiPro extends IPApi
 {
     protected $host = 'https://pro.ip-api.com';
@@ -26,15 +30,16 @@ public function isRateLimited(): bool
         return false;
     }
 
-    protected function buildUrl(string $ip, ?string $apiKey): string
+    protected function handleError(ResponseInterface $response, object $body): ?ServiceResponse
     {
-        return "/json/{$ip}?".http_build_query(['key' => $apiKey]);
+        return GeoIP::setError('ipapi-pro', $body->message ?? json_encode($body));
     }
 
-    protected function buildBatchUrl(array $ips, ?string $apiKey): string
+    protected function getRequestOptions(?string $apiKey, array $ips = null): array
     {
-        $url = '/batch?'.http_build_query(['key' => $apiKey, 'fields' => $this->r2]);
+        $options = parent::getRequestOptions($apiKey, $ips);
+        $options['query']['key'] = $apiKey;
 
-        return $url;
+        return $options;
     }
 }
diff --git a/src/Command/FetchIPInfoHandler.php b/src/Command/FetchIPInfoHandler.php
@@ -14,21 +14,21 @@
 use FoF\GeoIP\Api\GeoIP;
 use FoF\GeoIP\Model\IPInfo;
 use FoF\GeoIP\Repositories\GeoIPRepository;
-use Illuminate\Database\Eloquent\ModelNotFoundException;
-use RuntimeException;
+use Psr\Log\LoggerInterface;
 
 class FetchIPInfoHandler
 {
     public function __construct(
         protected GeoIP $geoip,
-        protected GeoIPRepository $repository
+        protected GeoIPRepository $repository,
+        protected LoggerInterface $log
     ) {
     }
 
     public function handle(FetchIPInfo $command): IPInfo
     {
         if (!$this->repository->isValidIP($command->ip)) {
-            throw new RuntimeException("Invalid IP address: {$command->ip}");
+            $this->log->info('Invalid IP address: '.$command->ip);
         }
 
         $ipInfo = IPInfo::query()->firstOrNew(['address' => $command->ip]);
@@ -37,7 +37,7 @@ public function handle(FetchIPInfo $command): IPInfo
             $response = $this->geoip->get($command->ip);
 
             if (!$response || $response->fake) {
-                throw new ModelNotFoundException("Unable to fetch IP information for IP: {$command->ip}");
+                $this->log->error("Unable to fetch IP information for IP: {$command->ip}", $response->toJSON());
             }
 
             if ($response) {

diff --git a/src/Jobs/RetrieveIP.php b/src/Jobs/RetrieveIP.php
@@ -49,7 +49,7 @@ public function handle(Repository $cache, Dispatcher $bus): void
         // Add to retrieving list and cache (don't attempt again for 1 hour if job fails)
         // Errors from the API do not count as job failures.
         static::$retrieving[] = $ip;
-        $cache->add($cacheKey, true, 60 * 60);
+        $cache->add($cacheKey, true, 60);
 
         /** @var IPInfo $ipInfo */
         $ipInfo = $bus->dispatch(new FetchIPInfo($ip));

diff --git a/src/Listeners/RetrieveIP.php b/src/Listeners/RetrieveIP.php
@@ -0,0 +1,54 @@
+<?php
+
+/*
+ * This file is part of fof/geoip.
+ *
+ * Copyright (c) FriendsOfFlarum.
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace FoF\GeoIP\Listeners;
+
+use Flarum\Post\Event\Saving as PostSaving;
+use FoF\GeoIP\Jobs;
+use FoF\GeoIP\Repositories\GeoIPRepository;
+use Illuminate\Contracts\Events\Dispatcher;
+use Illuminate\Contracts\Queue\Queue;
+
+class RetrieveIP
+{
+    /**
+     * @var Queue
+     */
+    protected $queue;
+
+    /**
+     * @var GeoIPRepository
+     */
+    protected $geo;
+
+    public function __construct(Queue $queue, GeoIPRepository $geo)
+    {
+        $this->queue = $queue;
+        $this->geo = $geo;
+    }
+
+    public function subscribe(Dispatcher $events)
+    {
+        $events->listen(PostSaving::class, [$this, 'handlePost']);
+    }
+
+    public function retrieveIP(string $ip): void
+    {
+        if ($this->geo->isValidIP($ip) && !$this->geo->recordExistsForIP($ip)) {
+            $this->queue->push(new Jobs\RetrieveIP($ip));
+        }
+    }
+
+    public function handlePost(PostSaving $event)
+    {
+        $this->retrieveIP($event->post->ip_address);
+    }
+}
diff --git a/src/Repositories/GeoIPRepository.php b/src/Repositories/GeoIPRepository.php
@@ -72,4 +72,9 @@ public function isValidIP(?string $ip): bool
     {
         return filter_var($ip, FILTER_VALIDATE_IP) !== false;
     }
+
+    public function recordExistsForIP(string $ip): bool
+    {
+        return IPInfo::where('address', $ip)->exists();
+    }
 }