From 222b65197a0aa387f9590f96a399fcb646bf0582 Mon Sep 17 00:00:00 2001 From: IanM Date: Mon, 1 Jul 2024 15:07:16 +0100 Subject: [PATCH] fix: batch lookup with ipapi fails --- README.md | 8 ++++ src/Api/Services/BaseGeoService.php | 9 +++- src/Api/Services/IPApi.php | 10 +++-- src/Command/FetchIPInfoBatchHandler.php | 15 ++++--- src/Command/FetchIPInfoHandler.php | 12 +++-- src/Console/LookupUnknownIPsCommand.php | 60 ++++++++++++++++--------- src/Model/IPInfo.php | 3 ++ src/Repositories/GeoIPRepository.php | 20 +++++++-- 8 files changed, 102 insertions(+), 35 deletions(-) diff --git a/README.md b/README.md index 8fafbb0..ffed682 100644 --- a/README.md +++ b/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: diff --git a/src/Api/Services/BaseGeoService.php b/src/Api/Services/BaseGeoService.php index ee01a08..552c6db 100644 --- a/src/Api/Services/BaseGeoService.php +++ b/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'); diff --git a/src/Api/Services/IPApi.php b/src/Api/Services/IPApi.php index a5bf85c..61f20d3 100644 --- a/src/Api/Services/IPApi.php +++ b/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, diff --git a/src/Command/FetchIPInfoBatchHandler.php b/src/Command/FetchIPInfoBatchHandler.php index 199c80a..c7ae1f3 100644 --- a/src/Command/FetchIPInfoBatchHandler.php +++ b/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(); diff --git a/src/Command/FetchIPInfoHandler.php b/src/Command/FetchIPInfoHandler.php index 0722ea3..73d2c5f 100644 --- a/src/Command/FetchIPInfoHandler.php +++ b/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) { diff --git a/src/Console/LookupUnknownIPsCommand.php b/src/Console/LookupUnknownIPsCommand.php index 80b2b96..83b8549 100644 --- a/src/Console/LookupUnknownIPsCommand.php +++ b/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}"); + } } } } diff --git a/src/Model/IPInfo.php b/src/Model/IPInfo.php index 3b026d5..b646c4f 100644 --- a/src/Model/IPInfo.php +++ b/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', ]; + } diff --git a/src/Repositories/GeoIPRepository.php b/src/Repositories/GeoIPRepository.php index feaad67..4b49ef5 100644 --- a/src/Repositories/GeoIPRepository.php +++ b/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; + } }