feat: add a rate limiter for locationiq queries (#5185)

monicahq · May 14, 2021 · f8442ba · f8442ba
1 parent 0a77dfa
commit f8442ba
Show file tree

Hide file tree

Showing 20 changed files with 216 additions and 123 deletions.
diff --git a/app/Console/Commands/ImportVCards.php b/app/Console/Commands/ImportVCards.php
@@ -92,7 +92,7 @@ private function import(string $path, User $user): ImportJob
             'filename' => $pathName,
         ]);
 
-        dispatch_now(new AddContactFromVCard($importJob));
+        AddContactFromVCard::dispatchSync($importJob);
 
         return $importJob;
     }

diff --git a/app/Http/Controllers/Auth/InvitationController.php b/app/Http/Controllers/Auth/InvitationController.php
@@ -129,7 +129,7 @@ protected function create(array $data, $invitation)
         $user->save();
 
         // send me an alert
-        dispatch(new SendNewUserAlert($user));
+        SendNewUserAlert::dispatch($user);
 
         return $user;
     }

diff --git a/app/Http/Controllers/Auth/RegisterController.php b/app/Http/Controllers/Auth/RegisterController.php
@@ -107,7 +107,7 @@ protected function create(array $data): ?User
 
             if (! $first) {
                 // send me an alert
-                dispatch(new SendNewUserAlert($user));
+                SendNewUserAlert::dispatch($user);
             }
 
             return $user;

diff --git a/app/Http/Controllers/SettingsController.php b/app/Http/Controllers/SettingsController.php
@@ -199,7 +199,7 @@ public function export()
      */
     public function exportToSql()
     {
-        $path = dispatch_now(new ExportAccountAsSQL());
+        $path = ExportAccountAsSQL::dispatchSync();
 
         $adapter = disk_adapter(ExportAccountAsSQL::STORAGE);
 
@@ -250,7 +250,7 @@ public function storeImport(ImportsRequest $request)
             'filename' => $filename,
         ]);
 
-        dispatch(new AddContactFromVCard($importJob, $request->input('behaviour')));
+        AddContactFromVCard::dispatch($importJob, $request->input('behaviour'));
 
         return redirect()->route('settings.import');
     }

diff --git a/app/Jobs/GetGPSCoordinate.php b/app/Jobs/GetGPSCoordinate.php
@@ -0,0 +1,72 @@
+<?php
+
+namespace App\Jobs;
+
+use App\Models\Account\Place;
+use Illuminate\Bus\Queueable;
+use Illuminate\Queue\SerializesModels;
+use Illuminate\Queue\InteractsWithQueue;
+use Illuminate\Contracts\Queue\ShouldQueue;
+use Illuminate\Foundation\Bus\Dispatchable;
+use Illuminate\Queue\Middleware\RateLimited;
+use App\Services\Instance\Geolocalization\GetGPSCoordinate as GetGPSCoordinateService;
+
+class GetGPSCoordinate implements ShouldQueue
+{
+    use Dispatchable, InteractsWithQueue, Queueable, SerializesModels;
+
+    /**
+     * @var Place
+     */
+    protected $place;
+
+    /**
+     * The number of times the job may be attempted.
+     *
+     * @var int
+     */
+    public $tries = 10;
+
+    /**
+     * The maximum number of unhandled exceptions to allow before failing.
+     *
+     * @var int
+     */
+    public $maxExceptions = 1;
+
+    /**
+     * Create a new job instance.
+     *
+     * @return void
+     */
+    public function __construct(Place $place)
+    {
+        $this->place = $place->withoutRelations();
+    }
+
+    /**
+     * Get the middleware the job should pass through.
+     *
+     * @return array
+     */
+    public function middleware()
+    {
+        return [
+            new RateLimited('GPSCoordinatePerMinute'),
+            new RateLimited('GPSCoordinatePerDay'),
+        ];
+    }
+
+    /**
+     * Execute the job.
+     *
+     * @return void
+     */
+    public function handle()
+    {
+        app(GetGPSCoordinateService::class)->execute([
+            'account_id' => $this->place->account_id,
+            'place_id' => $this->place->id,
+        ]);
+    }
+}
diff --git a/app/Jobs/SendNewUserAlert.php b/app/Jobs/SendNewUserAlert.php
@@ -8,11 +8,12 @@
 use Illuminate\Queue\SerializesModels;
 use Illuminate\Queue\InteractsWithQueue;
 use Illuminate\Contracts\Queue\ShouldQueue;
+use Illuminate\Foundation\Bus\Dispatchable;
 use Illuminate\Support\Facades\Notification;
 
 class SendNewUserAlert implements ShouldQueue
 {
-    use InteractsWithQueue, Queueable, SerializesModels;
+    use Dispatchable, InteractsWithQueue, Queueable, SerializesModels;
 
     protected $user;
 

diff --git a/app/Providers/AppServiceProvider.php b/app/Providers/AppServiceProvider.php
@@ -12,6 +12,8 @@
 use App\Notifications\EmailMessaging;
 use Illuminate\Support\Facades\Schema;
 use Illuminate\Support\ServiceProvider;
+use Illuminate\Cache\RateLimiting\Limit;
+use Illuminate\Support\Facades\RateLimiter;
 use Illuminate\Auth\Notifications\VerifyEmail;
 use Illuminate\Auth\Notifications\ResetPassword;
 
@@ -65,6 +67,13 @@ public function boot()
         });
 
         Paginator::defaultView('vendor.pagination.default');
+
+        RateLimiter::for('GPSCoordinatePerMinute', function () {
+            return Limit::perMinute(60);
+        });
+        RateLimiter::for('GPSCoordinatePerDay', function () {
+            return Limit::perDay(5000);
+        });
     }
 
     /**

diff --git a/app/Services/Account/Place/CreatePlace.php b/app/Services/Account/Place/CreatePlace.php
@@ -4,8 +4,7 @@
 
 use App\Models\Account\Place;
 use App\Services\BaseService;
-use GuzzleHttp\Client as GuzzleClient;
-use App\Services\Instance\Geolocalization\GetGPSCoordinate;
+use App\Jobs\GetGPSCoordinate;
 
 class CreatePlace extends BaseService
 {
@@ -32,10 +31,9 @@ public function rules()
      * Create a place.
      *
      * @param array $data
-     * @param GuzzleClient $client the Guzzle client, only needed when unit testing
      * @return Place
      */
-    public function execute(array $data, GuzzleClient $client = null): Place
+    public function execute(array $data): Place
     {
         $this->validate($data);
 
@@ -50,8 +48,8 @@ public function execute(array $data, GuzzleClient $client = null): Place
             'longitude' => $this->nullOrValue($data, 'longitude'),
         ]);
 
-        if (is_null($place->latitude)) {
-            $this->getGeocodingInfo($place, $client);
+        if (is_null($place->latitude) || is_null($place->longitude)) {
+            $this->getGeocodingInfo($place);
         }
 
         return $place;
@@ -61,14 +59,12 @@ public function execute(array $data, GuzzleClient $client = null): Place
      * Get geocoding information about the place (lat/longitude).
      *
      * @param Place $place
-     * @param GuzzleClient $client the Guzzle client, only needed when unit testing
      * @return void
      */
-    private function getGeocodingInfo(Place $place, GuzzleClient $client = null)
+    private function getGeocodingInfo(Place $place)
     {
-        app(GetGPSCoordinate::class)->execute([
-            'account_id' => $place->account_id,
-            'place_id' => $place->id,
-        ], $client);
+        if (config('monica.enable_geolocation') && ! is_null(config('monica.location_iq_api_key'))) {
+            GetGPSCoordinate::dispatch($place);
+        }
     }
 }
diff --git a/app/Services/Account/Place/UpdatePlace.php b/app/Services/Account/Place/UpdatePlace.php
@@ -4,8 +4,7 @@
 
 use App\Models\Account\Place;
 use App\Services\BaseService;
-use GuzzleHttp\Client as GuzzleClient;
-use App\Services\Instance\Geolocalization\GetGPSCoordinate;
+use App\Jobs\GetGPSCoordinate;
 
 class UpdatePlace extends BaseService
 {
@@ -33,10 +32,9 @@ public function rules()
      * Update a place.
      *
      * @param array $data
-     * @param GuzzleClient $client the Guzzle client, only needed when unit testing
      * @return Place
      */
-    public function execute(array $data, GuzzleClient $client = null): Place
+    public function execute(array $data): Place
     {
         $this->validate($data);
 
@@ -54,8 +52,8 @@ public function execute(array $data, GuzzleClient $client = null): Place
             'longitude' => $this->nullOrValue($data, 'longitude'),
         ]);
 
-        if (is_null($place->latitude)) {
-            $this->getGeocodingInfo($place, $client);
+        if (is_null($place->latitude) || is_null($place->longitude)) {
+            $this->getGeocodingInfo($place);
         }
 
         return $place;
@@ -65,14 +63,12 @@ public function execute(array $data, GuzzleClient $client = null): Place
      * Get geocoding information about the place (lat/longitude).
      *
      * @param Place $place
-     * @param GuzzleClient $client the Guzzle client, only needed when unit testing
      * @return void
      */
-    private function getGeocodingInfo(Place $place, GuzzleClient $client = null)
+    private function getGeocodingInfo(Place $place)
     {
-        app(GetGPSCoordinate::class)->execute([
-            'account_id' => $place->account_id,
-            'place_id' => $place->id,
-        ], $client);
+        if (config('monica.enable_geolocation') && ! is_null(config('monica.location_iq_api_key'))) {
+            GetGPSCoordinate::dispatch($place);
+        }
     }
 }
diff --git a/app/Services/Instance/Geolocalization/GetGPSCoordinate.php b/app/Services/Instance/Geolocalization/GetGPSCoordinate.php
@@ -5,16 +5,12 @@
 use Illuminate\Support\Str;
 use App\Models\Account\Place;
 use App\Services\BaseService;
-use function Safe\json_decode;
 use Illuminate\Support\Facades\Log;
-use GuzzleHttp\Client as GuzzleClient;
-use GuzzleHttp\Exception\ClientException;
+use Illuminate\Support\Facades\Http;
+use Illuminate\Http\Client\HttpClientException;
 
 class GetGPSCoordinate extends BaseService
 {
-    /** @var GuzzleClient */
-    protected $client;
-
     /**
      * Get the validation rules that apply to the service.
      *
@@ -33,19 +29,12 @@ public function rules()
      * This method uses LocationIQ to process the geocoding.
      *
      * @param array $data
-     * @param GuzzleClient $client the Guzzle client, only needed when unit testing
      * @return Place|null
      */
-    public function execute(array $data, GuzzleClient $client = null)
+    public function execute(array $data)
     {
         $this->validate($data);
 
-        if (! is_null($client)) {
-            $this->client = $client;
-        } else {
-            $this->client = new GuzzleClient();
-        }
-
         $place = Place::where('account_id', $data['account_id'])
             ->findOrFail($data['place_id']);
 
@@ -88,19 +77,18 @@ private function query(Place $place): ?Place
         }
 
         try {
-            $response = $this->client->request('GET', $query);
-        } catch (ClientException $e) {
-            Log::error('Error making the call: '.$e);
+            $response = Http::get($query);
+            $response->throw();
 
-            return null;
-        }
-
-        $response = json_decode($response->getBody());
+            $place->latitude = $response->json('0.lat');
+            $place->longitude = $response->json('0.lon');
+            $place->save();
 
-        $place->latitude = $response[0]->lat;
-        $place->longitude = $response[0]->lon;
-        $place->save();
+            return $place;
+        } catch (HttpClientException $e) {
+            Log::error('Error making the call: '.$e);
+        }
 
-        return $place;
+        return null;
     }
 }