From d2b56abcd87f93884d6b5d3137d4ef6763890696 Mon Sep 17 00:00:00 2001 From: IanM Date: Sat, 14 Sep 2024 10:52:42 +0100 Subject: [PATCH] fix: ipapi-pro single lookup --- extend.php | 6 ++-- src/Api/Services/BaseGeoService.php | 19 ++++++++++-- src/Api/Services/IPApi.php | 21 +++++++------ src/Api/Services/IPApiPro.php | 17 +++++++---- src/Command/FetchIPInfoHandler.php | 8 +++-- src/Jobs/RetrieveIP.php | 2 +- src/Listeners/RetrieveIP.php | 45 ++++++++++++++++++++++++++++ src/Repositories/GeoIPRepository.php | 5 ++++ 8 files changed, 97 insertions(+), 26 deletions(-) create mode 100644 src/Listeners/RetrieveIP.php diff --git a/extend.php b/extend.php index 9e2cb2f..61e60b9 100644 --- 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 index 552c6db..fba586e 100644 --- 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 index f1aa2cb..a78f2c3 100644 --- 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 index e738022..7e503cb 100644 --- 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]); - - return $url; + $options = parent::getRequestOptions($apiKey, $ips); + $options['query']['key'] = $apiKey; + + return $options; } } diff --git a/src/Command/FetchIPInfoHandler.php b/src/Command/FetchIPInfoHandler.php index 74905d8..b4ff7e6 100644 --- a/src/Command/FetchIPInfoHandler.php +++ b/src/Command/FetchIPInfoHandler.php @@ -15,20 +15,22 @@ use FoF\GeoIP\Model\IPInfo; use FoF\GeoIP\Repositories\GeoIPRepository; use Illuminate\Database\Eloquent\ModelNotFoundException; +use Psr\Log\LoggerInterface; use RuntimeException; 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 +39,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 index 51991c9..49dc069 100644 --- 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 new file mode 100644 index 0000000..96a4242 --- /dev/null +++ b/src/Listeners/RetrieveIP.php @@ -0,0 +1,45 @@ +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 index 9f3d531..3c46ab4 100644 --- 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(); + } }