fix: batch lookup with ipapi fails

README.md

@@ -35,6 +35,14 @@ Although IP addresses will be looked up when they are requested, this command wi
 php flarum fof:geoip:lookup
 ```
 
+#### `lookup --force`
+
+You may also force a refresh of IP data using the currently selected provider.
+
+```sh
+php flarum fof:geoip:lookup --force
+```
+
 ### Installation
 
 Install manually with composer:

src/Api/Services/BaseGeoService.php

@@ -38,6 +38,9 @@ public function __construct(protected SettingsRepositoryInterface $settings, pro
         $this->client = new Client([
             'base_uri' => $this->host,
             'verify'   => false,
+            'headers'  => [
+                'Accept'     => 'application/json',
+            ],
         ]);
     }
 
@@ -105,7 +108,11 @@ public function getBatch(array $ips)
 
         /** @phpstan-ignore-next-line */
         if (!$this->isRateLimited() || ($this->isRateLimited() && $this->batchLookupsRemaining > 0)) {
-            $response = $this->client->request('POST', $this->buildBatchUrl($ips, $apiKey), $this->getRequestOptions($apiKey, $ips));
+            $response = $this->client->post($this->buildBatchUrl($ips, $apiKey), $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()]);
+            }
 
             if ($this->isRateLimited()) {
                 $this->updateRateLimitsFromResponse($response, 'batch');

src/Api/Services/IPApi.php

@@ -20,7 +20,8 @@ class IPApi extends BaseGeoService
 {
     protected $host = 'http://ip-api.com';
     protected $settingPrefix = 'fof-geoip.services.ipapi';
-    protected $requestFields = 'status,message,query,countryCode,city,zip,lat,lon,isp,org,as,mobile';
+    protected $requestFields = 'status,message,countryCode,region,regionName,city,zip,lat,lon,isp,org,as,mobile,query';
+    protected $r2 = '126718';
 
     /**
      * 45 requests per minute.
@@ -67,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->requestFields]);
+        return '/batch?'.http_build_query(['fields' => $this->r2]);
     }
 
     protected function requiresApiKey(): bool
@@ -77,7 +78,10 @@ protected function requiresApiKey(): bool
 
     protected function getRequestOptions(?string $apiKey, array $ips = null): array
     {
-        if ($ips) {
+        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,

src/Command/FetchIPInfoBatchHandler.php

@@ -13,17 +13,21 @@
 
 use FoF\GeoIP\Api\GeoIP;
 use FoF\GeoIP\Model\IPInfo;
+use FoF\GeoIP\Repositories\GeoIPRepository;
 use Illuminate\Support\Arr;
 
 class FetchIPInfoBatchHandler
 {
     public function __construct(
-        protected GeoIP $geoip
-    ) {
-    }
+        protected GeoIP $geoip,
+        protected GeoIPRepository $repository
+    ) { }
 
     public function handle(FetchIPInfoBatch $command)
     {
+        // remove any duplicates or invalid addresses from the ips array
+        $command->ips = array_unique(array_filter($command->ips, fn ($ip) => $this->repository->isValidIP($ip)));
+
         // query the IPInfo model for all the ips in the command
         // if the ip doesn't exist, create a new IPInfo model
 
@@ -37,8 +41,9 @@ public function handle(FetchIPInfoBatch $command)
             $responses = $this->geoip->getBatch($ipsToQuery);
 
             foreach ($responses as $response) {
-                $ipInfo = new IPInfo();
-                $ipInfo->address = $response->getIP();
+                $ip = $response->getIP();
+                $ipInfo = IPInfo::query()->firstOrNew(['address' => $ip]);
+                $ipInfo->address = $ip;
                 $ipInfo->fill($response->toJSON());
                 $ipInfo->save();
 

src/Command/FetchIPInfoHandler.php

@@ -13,17 +13,23 @@
 
 use FoF\GeoIP\Api\GeoIP;
 use FoF\GeoIP\Model\IPInfo;
+use FoF\GeoIP\Repositories\GeoIPRepository;
 use Illuminate\Database\Eloquent\ModelNotFoundException;
+use RuntimeException;
 
 class FetchIPInfoHandler
 {
     public function __construct(
-        protected GeoIP $geoip
-    ) {
-    }
+        protected GeoIP $geoip,
+        protected GeoIPRepository $repository
+    ) { }
 
     public function handle(FetchIPInfo $command): IPInfo
     {
+        if (!$this->repository->isValidIP($command->ip)) {
+            throw new RuntimeException("Invalid IP address: {$command->ip}");
+        }
+
         $ipInfo = IPInfo::query()->firstOrNew(['address' => $command->ip]);
 
         if (!$ipInfo->exists || $command->refresh) {

src/Console/LookupUnknownIPsCommand.php

@@ -32,7 +32,7 @@ class LookupUnknownIPsCommand extends Command
         Draft::class       => 'ip_address',
     ];
 
-    protected $signature = 'fof:geoip:lookup';
+    protected $signature = 'fof:geoip:lookup {--force}';
 
     protected $description = 'Look up IP addresses which have not been looked up before.';
 
@@ -47,33 +47,53 @@ public function handle()
             if (!class_exists($model)) {
                 continue;
             }
-            $this->info("Looking up IP data for {$model}");
 
             /** @var AbstractModel $model */
-            $query = $model::query()
-                ->select('id', $column)
+            $query = $model::query();
+
+            $force = (bool) $this->option('force');
+
+            if ($force) {
+                $this->info("Forcing lookup for {$model}");
+                $query->select('id', $column);
+            } else {
+                $query->select('id', $column)
                 ->whereNotNull($column)
                 ->whereNotIn($column, function ($query) use ($column) {
                     $query->select('address')
                         ->from('ip_info')
                         ->whereColumn('address', $column);
-                })
-                ->groupBy($column);
-
-            $query
-                ->chunkById(100, function ($models) use ($column) {
-                    if ($this->geoIP->batchSupported()) {
-                        $ips = $models->pluck($column)->toArray();
-                        $count = count($ips);
-                        $this->info("Looking up IP data for {$count} IPs");
-                        $this->bus->dispatch(new FetchIPInfoBatch(ips: $ips, refresh: false));
-                    } else {
-                        $models->each(function ($model) use ($column) {
-                            $this->info("Looking up IP data for {$model->$column}");
-                            $this->bus->dispatch(new FetchIPInfo(ip: $model->$column, refresh: false));
-                        });
-                    }
                 });
+            }
+
+            $query->groupBy($column);
+
+            if ($query->count() > 0) {
+                $this->info("Looking up IP data for {$model}");
+
+                $this->output->progressStart($query->count());
+                $chunkSize = 100;
+
+                $query
+                    ->chunkById($chunkSize, function ($models) use ($column, $chunkSize, $force) {
+                        if ($this->geoIP->batchSupported()) {
+                            $ips = $models->pluck($column)->toArray();
+                            $count = count($ips);
+                            $this->bus->dispatch(new FetchIPInfoBatch(ips: $ips, refresh: $force));
+                            $this->output->progressAdvance($chunkSize === $count ? $chunkSize : $count);
+                        } else {
+                            $models->each(function ($model) use ($column, $force) {
+                                $this->info("Looking up IP data for {$model->$column}");
+                                $this->bus->dispatch(new FetchIPInfo(ip: $model->$column, refresh: $force));
+                                $this->output->progressAdvance();
+                            });
+                        }
+                    });
+
+                $this->output->progressFinish();
+            } else {
+                $this->info("Nothing to look up for {$model}");
+            }
         }
     }
 }

src/Model/IPInfo.php

@@ -35,6 +35,8 @@ class IPInfo extends AbstractModel
 {
     protected $table = 'ip_info';
 
+    protected $primaryKey = 'address';
+
     protected $fillable = [
         'country_code', 'zip_code', 'latitude', 'longitude',
         'isp', 'organization', 'as', 'mobile',
@@ -49,4 +51,5 @@ class IPInfo extends AbstractModel
         'created_at' => 'datetime',
         'updated_at' => 'datetime',
     ];
+
 }

src/Repositories/GeoIPRepository.php

@@ -20,9 +20,10 @@
 
 class GeoIPRepository
 {
-    public function __construct(protected GeoIP $geoIP, protected Queue $queue)
-    {
-    }
+    public function __construct(
+        protected GeoIP $geoIP,
+        protected Queue $queue
+    ) { }
 
     /**
      * @param string|null $ip
@@ -58,4 +59,17 @@ public function retrieveForPost(Post $post)
         // If using the sync queue driver (default), the job will be executed immediately
         return Arr::get(RetrieveIP::$retrieved, $ip);
     }
+
+    /**
+     * Determine if the given value is a valid IP address.
+     * 
+     * @param string $ip
+     *
+     * @return bool
+     * 
+     */
+    public function isValidIP(?string $ip): bool
+    {
+        return filter_var($ip, FILTER_VALIDATE_IP) !== false;
+    }
 }