RES-1858: geocode addresses with Mapbox, and only Mapbox

.circleci/config.yml

@@ -48,12 +48,12 @@ jobs:
             echo "MAPBOX_TOKEN=$MAPBOX_TOKEN" >> .env
 
             # CircleCI does not give project environment variables to jobs built
-            # from forked pull requests, so those runs have no geocoding key.
+            # from forked pull requests, so those runs have no Mapbox token.
             # Without this every group creation 422s on geocode_failed and the
             # whole Playwright suite fails at its first createGroup(), which
             # reads as the contributor having broken something.
-            if [ -z "$GOOGLE_API_CONSOLE_KEY" ]; then
-              echo "No geocoding key available - using the stub geocoder."
+            if [ -z "$MAPBOX_TOKEN" ]; then
+              echo "No Mapbox token available - using the stub geocoder."
               echo "GEOCODER_STUB=true" >> .env
             fi
 

app/Helpers/Geocoder.php

@@ -2,66 +2,50 @@
 
 namespace App\Helpers;
 
+use Geocoder\Provider\Mapbox\Mapbox;
+use Geocoder\Query\GeocodeQuery;
+use Illuminate\Support\Facades\Log;
+
 class Geocoder
 {
     public function __construct()
     {
     }
 
-    private function googleKey()
-    {
-        // We have this so that we can change the key in testing.
-        return config('GOOGLE_API_CONSOLE_KEY') ?? env('GOOGLE_API_CONSOLE_KEY');
-    }
-
-    /**
-     * Fetch a URL.  Separated out so that tests can exercise the parsing below
-     * without calling Google, which makes them depend on the network, the key
-     * and the quota all being healthy.
-     */
-    protected function fetch($url)
-    {
-        return file_get_contents($url);
-    }
-
     public function geocode($location)
     {
         if ($location != 'ForceGeocodeFailure') {
-            $json = $this->fetch('https://maps.googleapis.com/maps/api/geocode/json?address='.urlencode($location).'&key='.$this->googleKey());
-
-            if ($json) {
-                $res = json_decode($json);
-
-                if ($res && $res->results && count($res->results)) {
-                    $decoded = json_decode($json)->results[0];
-
-                    $latitude = $decoded->{'geometry'}->{'location'}->lat;
-                    $longitude = $decoded->{'geometry'}->{'location'}->lng;
-
-                    foreach ($decoded->{'address_components'} as $component) {
-                        if ($component->types && count($component->types) && $component->types[0] === 'country') {
-                            $country_code = $component->short_name;
-                        }
-                    }
+            try {
+                // Mapbox, configured in config/geocoder.php and resolved via the container so tests can bind a
+                // fake in its place.  There is no fallback provider.  This is the same geocoder that
+                // App\Console\Commands\SetPlaceNetworkData uses for reverse geocoding.
+                $geocodeResponse = app('geocoder')->geocodeQuery(
+                    GeocodeQuery::create($location)->withData('location_type', [Mapbox::TYPE_PLACE, Mapbox::TYPE_ADDRESS])
+                );
+                $addressCollection = $geocodeResponse->get();
+                // get(0) throws OutOfBounds on an empty collection, so an address the
+                // provider can't place would crash rather than returning false.
+                $address = $addressCollection->isEmpty() ? null : $addressCollection->first();
+
+                if ($address && $address->getCoordinates()) {
+                    // The provider gives us both the country name and its ISO code - use the code directly rather
+                    // than trying to match the name against our own country list.
+                    $country = $address->getCountry();
 
                     return [
-                        'latitude' => $latitude,
-                        'longitude' => $longitude,
-                        'country_code' => $country_code,
+                        'latitude' => $address->getCoordinates()->getLatitude(),
+                        'longitude' => $address->getCoordinates()->getLongitude(),
+                        'country_code' => $country ? $country->getCode() : null,
                     ];
                 }
+            } catch (\Throwable $e) {
+                // A bad token, an exhausted quota or Mapbox being unreachable is a failed geocode as far as the
+                // caller is concerned: it becomes a validation message, not a 500.  Log it so a misconfiguration
+                // is still visible.
+                Log::error("Geocoding '{$location}' failed: " . $e->getMessage());
             }
         }
 
         return false;
     }
-
-    public function reverseGeocode($lat, $lng)
-    {
-        $json = $this->fetch("https://maps.googleapis.com/maps/api/geocode/json?latlng=$lat,$lng&key=".$this->googleKey());
-
-        $decoded = json_decode($json)->results[0];
-
-        return $decoded;
-    }
 }

app/Helpers/StubGeocoder.php

@@ -3,17 +3,17 @@
 namespace App\Helpers;
 
 /**
- * A Geocoder that answers from a fixed table instead of calling Google.
+ * A Geocoder that answers from a fixed table instead of calling Mapbox.
  *
  * CircleCI does not pass project environment variables to jobs built from
- * forked pull requests, so GOOGLE_API_CONSOLE_KEY is empty on those runs.
+ * forked pull requests, so MAPBOX_TOKEN is empty on those runs.
  * Every geocode then returns false, group creation 422s on
  * groups.geocode_failed, and the whole Playwright suite fails at its first
  * createGroup() - which looks exactly like the contributor having broken
  * something. This stub keeps those runs meaningful, and keeps the suite off
- * the live Google API generally.
+ * the live Mapbox API generally.
  *
- * Only used when GEOCODER_STUB is set, which CI does when there is no key.
+ * Only used when GEOCODER_STUB is set, which CI does when there is no token.
  */
 class StubGeocoder extends Geocoder
 {

config/geocoder.php

@@ -1,7 +1,5 @@
 <?php
 
-use Geocoder\Provider\Chain\Chain;
-use Geocoder\Provider\GeoPlugin\GeoPlugin;
 use Geocoder\Provider\Mapbox\Mapbox;
 use Http\Client\Curl\Client;
 
@@ -43,24 +41,20 @@
     | Providers
     |---------------------------------------------------------------------------
     |
-    | Here you may specify any number of providers that should be used to
-    | perform geocaching operations. The `chain` provider is special,
-    | in that it can contain multiple providers that will be run in
-    | the sequence listed, should the previous provider fail. By
-    | default the first provider listed will be used, but you
-    | can explicitly call subsequently listed providers by
-    | alias: `app('geocoder')->using('google_maps')`.
+    | Mapbox is the only geocoder. There is deliberately no fallback provider:
+    | the address geocoder in App\Helpers\Geocoder and the reverse geocoder in
+    | App\Console\Commands\SetPlaceNetworkData both need a provider that
+    | understands street addresses, and a chain that fell through to an
+    | IP-address geocoder could never place one. If Mapbox can't answer, the
+    | geocode fails and the caller says so.
     |
     | Please consult the official Geocoder documentation for more info.
     | https://github.com/geocoder-php/Geocoder#providers
     |
     */
     'providers' => [
-        Chain::class => [
-            Mapbox::class => [
-                env('MAPBOX_TOKEN'),
-            ],
-            GeoPlugin::class  => [],
+        Mapbox::class => [
+            env('MAPBOX_TOKEN'),
         ],
     ],
 

docs/local-development.md

@@ -167,19 +167,18 @@ task docker:test:playwright-autocomplete
 
 ### Prerequisites for Testing
 
-Before running tests, you need to configure the Google API key for geocoding functionality:
+Geocoding (turning an address into coordinates when a group, event or profile is saved) uses Mapbox. The PHPUnit suite binds a deterministic fake geocoder, so it needs no token; the Playwright suite runs against the real application, so it does:
 
-1. **Set Google API Key**: Add a valid Google Maps API key to your `.env` file:
+1. **Set the Mapbox token**: Add a Mapbox access token to your `.env` file:
    ```bash
-   GOOGLE_API_CONSOLE_KEY=your_actual_google_api_key_here
+   MAPBOX_TOKEN=your_actual_mapbox_token_here
    ```
+   Or set `GEOCODER_STUB=true` to answer from a fixed table of places instead, as CI does on forked pull requests, which get no secrets.
 
-2. **API Key Requirements**: The key must have the following APIs enabled:
-   - Geocoding API
-   - Maps JavaScript API (for location validation)
+2. **Google API key**: the maps and the address autocomplete in the browser still use Google, so `GOOGLE_API_CONSOLE_KEY` is needed for those pages to look right, but not for geocoding.
 
 > [!WARNING]
-> Without a valid `GOOGLE_API_CONSOLE_KEY`, tests that create groups or events will fail with location validation errors.
+> Without a valid `MAPBOX_TOKEN` (or `GEOCODER_STUB=true`), Playwright tests that create groups or events will fail with location validation errors.
 
 > [!NOTE]
 > The PHPUnit task will automatically upload coverage to Coveralls if the `COVERALLS_REPO_TOKEN` environment variable is set.

tests/Feature/Users/EditProfileTest.php

@@ -11,7 +11,6 @@
 use Carbon\Carbon;
 use DB;
 use Illuminate\Foundation\Testing\RefreshDatabase;
-use Illuminate\Support\Facades\Config;
 use Illuminate\Support\Facades\Event;
 use Illuminate\Support\Facades\Storage;
 use Tests\TestCase;
@@ -111,10 +110,7 @@ public function test_location_update(): void
         $this->assertEquals(51.507, round($user->latitude, 3));
         $this->assertEquals(-0.128, round($user->longitude, 3));
 
-        $good = Config::get('GOOGLE_API_CONSOLE_KEY');
-        Config::set('GOOGLE_API_CONSOLE_KEY', 'zzz');
-
-        // Supply the id.
+        // Supply the id, and a town the test geocoder doesn't know so geocoding fails.
         $this->post('/profile/edit-info', [
             'id' => $user->id,
             'name' => $user->name,
@@ -124,8 +120,6 @@ public function test_location_update(): void
             'townCity' => 'ZZZZ',
         ]);
 
-        Config::set('GOOGLE_API_CONSOLE_KEY', $good);
-
         $user = $user->fresh();
         $this->assertNull($user->latitude);
         $this->assertNull($user->longitude);

tests/Feature/Users/Registration/AccountCreationTest.php

@@ -6,7 +6,6 @@
 use App\User;
 use DB;
 use Hash;
-use Illuminate\Support\Facades\Config;
 use Mockery;
 use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
 use Tests\TestCase;
@@ -54,15 +53,10 @@ public function testRegisterInvalidAddress(): void
     {
         $userAttributes = $this->userAttributes();
 
-        // Specify an invalid city and force geocoding to fail by invalidating the Google key.
-        $good = Config::get('GOOGLE_API_CONSOLE_KEY');
-        Config::set('GOOGLE_API_CONSOLE_KEY', 'zzz');
-
+        // A city the test geocoder doesn't know, so geocoding fails.
         $userAttributes['city'] = 'zzzzzzz';
         $response = $this->post('/user/register/', $userAttributes);
 
-        Config::set('GOOGLE_API_CONSOLE_KEY', $good);
-
         $response->assertStatus(302);
         $response->assertRedirect('dashboard');
         $this->assertDatabaseHas('users', [

tests/Support/GeocoderMock.php

@@ -7,10 +7,10 @@
 /**
  * Deterministic, location-aware Geocoder for the test suite.
  *
- * The real Geocoder makes a live HTTP call to the Google Maps Geocoding API, which is
+ * The real Geocoder makes a live HTTP call to the Mapbox Geocoding API, which is
  * rate-limited and non-deterministic. Tests that geocode a location were at the mercy
- * of Google's responses — e.g. APIv2GroupTest::testLocales flipped between Belgium and
- * the United Kingdom, and the "bad location" tests only passed while Google happened to
+ * of the provider's responses — e.g. APIv2GroupTest::testLocales flipped between Belgium and
+ * the United Kingdom, and the "bad location" tests only passed while the provider happened to
  * reject the gibberish string.
  *
  * This fake is bound globally in Tests\TestCase::setUp() so no test hits the live API.

tests/TestCase.php

@@ -101,9 +101,9 @@ protected function setUp(): void
         $this->withoutExceptionHandling();
         app('honeypot')->disable();
 
-        // Bind a deterministic geocoder so the suite never makes live calls to the
-        // Google Maps Geocoding API (rate-limited / intermittently failing, which
-        // made any geocoding test flaky). Tests that need the geocode-failure path
+        // Bind a deterministic geocoder so the suite never makes live calls to
+        // Mapbox (rate-limited / intermittently failing, which made any geocoding
+        // test flaky). Tests that need the geocode-failure path
         // bind their own failing mock in setUp(), overriding this.
         $this->app->bind(Geocoder::class, function () {
             return new GeocoderMock();