Perform all reverse geocoding on the back-end via geonames api

README.md

@@ -129,8 +129,7 @@ POST /game/{game_id}/round/{round}/guess/{player_id}
     Accepts {
         "timeRemaining": number,
         "lat": number,
-        "lng": number,
-        "country": string || null (default: null),
+        "lng": number
     }
     Returns (404, 409) vs 201 and {
         "totalScore": number,

client/src/components/screens/GamePanel/GuessPane/GuessPane.jsx

@@ -1,6 +1,5 @@
 import { useCallback, useState } from "react";
 import { dispatch } from "../../../../domain/gameStore";
-import { reverseGeocode } from "../../../../domain/geocoding";
 import ClickMarkerMap from "./ClickMarkerMap";
 import styles from "./GuessPane.module.css";
 import RoundTimer from "./RoundTimer";
@@ -26,12 +25,7 @@ const GuessPane = () => {
   const handleSubmitGuess = async () => {
     setSubmitted(true);
     if (!submitted) {
-      await dispatch.submitGuess(
-        selectedPoint && {
-          country: await reverseGeocode(selectedPoint),
-          ...selectedPoint,
-        }
-      );
+      await dispatch.submitGuess(selectedPoint);
     }
   };
 

client/src/domain/geocoding.js

@@ -3,25 +3,6 @@ import iso from "iso-3166-1";
 
 export const GEOCODER = new google.maps.Geocoder();
 
-export const reverseGeocode = async location => {
-  try {
-    const { results } = await GEOCODER.geocode({ location });
-    // eslint-disable-next-line no-restricted-syntax
-    for (const { address_components: comps } of results) {
-      // eslint-disable-next-line no-restricted-syntax
-      for (const { short_name: name, types } of comps) {
-        if (types.indexOf("country") >= 0) {
-          return name;
-        }
-      }
-    }
-  } catch (e) {
-    // ignore errors - just use null
-    // TODO probably alert the user?
-  }
-  return null;
-};
-
 const boundsCache = {};
 
 export const getCountryBounds = async countryCode => {

client/src/tests/GuessPane.test.js

@@ -3,11 +3,9 @@ import { shallow } from "enzyme";
 import GuessPane from "../components/screens/GamePanel/GuessPane";
 
 jest.mock("../domain/gameStore");
-jest.mock("../domain/geocoding");
 jest.mock("../components/screens/GamePanel/GuessPane/hooks");
 
 import { dispatch } from "../domain/gameStore";
-import { reverseGeocode } from "../domain/geocoding";
 
 describe("GuessPane", () => {
   it("renders", () => {
@@ -72,7 +70,6 @@ describe("GuessPane", () => {
   });
 
   it("submits", async () => {
-    reverseGeocode.mockReturnValue("geocoded");
     const handle = shallow(<GuessPane />);
     handle.find("ClickMarkerMap").first().prop("onMarkerMoved")({
       lat: "lat",
@@ -81,12 +78,8 @@ describe("GuessPane", () => {
     // check submit enabled
     expect(handle).toMatchSnapshot();
     await handle.find("button").first().simulate("click");
-    expect(reverseGeocode).toHaveBeenCalledWith(
-      expect.objectContaining({ lat: "lat", lng: "lng" })
-    );
     expect(dispatch.submitGuess).toHaveBeenCalledWith(
       expect.objectContaining({
-        country: "geocoded",
         lat: "lat",
         lng: "lng",
       })
@@ -96,7 +89,6 @@ describe("GuessPane", () => {
   });
 
   it("cannot be tricked into resubmitting", async () => {
-    reverseGeocode.mockReturnValue("geocoded");
     const handle = shallow(<GuessPane />);
     handle.find("ClickMarkerMap").first().prop("onMarkerMoved")({
       lat: "lat",
@@ -106,7 +98,6 @@ describe("GuessPane", () => {
     expect(handle).toMatchSnapshot();
     await handle.find("button").first().simulate("click");
     await handle.find("button").prop("onClick")();
-    expect(reverseGeocode).toHaveBeenCalledTimes(1);
     expect(dispatch.submitGuess).toHaveBeenCalledTimes(1);
     // check submit disabled again
     expect(handle).toMatchSnapshot();

client/src/tests/geocoding.test.js

@@ -1,58 +1,10 @@
-import {
-  GEOCODER,
-  getCountryBounds,
-  reverseGeocode,
-} from "../domain/geocoding";
+import { GEOCODER, getCountryBounds } from "../domain/geocoding";
 
 jest.mock("iso-3166-1");
 
 import iso from "iso-3166-1";
 
 describe("geocoding", () => {
-  describe("reverseGeocode", () => {
-    it("unpacks reverse geocoding data and returns country", async () => {
-      GEOCODER.geocode.mockReturnValue({
-        results: [
-          {
-            address_components: [
-              {
-                short_name: "us",
-                types: ["country"],
-              },
-            ],
-          },
-        ],
-      });
-      expect(await reverseGeocode("test")).toBe("us");
-      expect(GEOCODER.geocode).toHaveBeenCalledWith(
-        expect.objectContaining({
-          location: "test",
-        })
-      );
-    });
-    it("handles missing data gracefully", async () => {
-      GEOCODER.geocode.mockReturnValue({
-        results: [
-          {
-            address_components: [
-              {
-                short_name: "md",
-                types: ["not-country"],
-              },
-            ],
-          },
-        ],
-      });
-      expect(await reverseGeocode()).toBe(null);
-    });
-    it("suppresses errors and returns null", async () => {
-      GEOCODER.geocode.mockImplementation(() => {
-        throw new Error();
-      });
-      expect(await reverseGeocode()).toBe(null);
-    });
-  });
-
   describe("getCountryBounds", () => {
     it("gets bounds for a given country", async () => {
       iso.whereAlpha2.mockReturnValue({ country: "test-country" });

server/app/api/game.py

@@ -9,7 +9,7 @@ from sqlalchemy.orm import Session
 from .. import scoring
 from ..schemas import GameConfig, Guess, RuleSetEnum
 from ..db import get_db, queries, models
-from ..point_gen import points, ExhaustedSourceError
+from ..point_gen import points, ExhaustedSourceError, reverse_geocode
 
 logger = logging.getLogger(__name__)
 
@@ -140,18 +140,19 @@ def get_first_submitter(game_id: str, round_number: conint(gt=0), db: Session =
 
 
 @router.post("/{game_id}/round/{round_number}/guess/{player_id}", response_model=GuessResult)
-def submit_guess(round_number: conint(gt=0), 
+async def submit_guess(round_number: conint(gt=0), 
                  guess: Guess, 
                  db: Session = Depends(get_db), 
                  game: models.Game = Depends(get_game), 
                  player: models.Player = Depends(get_player)):
     target = queries.get_coordinate(db, player.game_id, round_number)
+    country_code = await reverse_geocode(guess.lat, guess.lng)
     if game.rule_set == RuleSetEnum.country_race:
-        score = scoring.score_country_race(target.country_code, guess.country, guess.time_remaining, game.timer)
+        score = scoring.score_country_race(target.country_code, country_code, guess.time_remaining, game.timer)
         distance = None
     else:
         score, distance = scoring.score((target.latitude, target.longitude), (guess.lat, guess.lng))
-    added = queries.add_guess(db, guess, player, round_number, score)
+    added = queries.add_guess(db, guess, player, country_code, round_number, score)
     if not added:
         raise HTTPException(status_code=409, detail="Already submitted guess for this round")
     total_score = queries.get_total_score(player)

server/app/api/other.py

@@ -43,7 +43,7 @@ class RecentResponse(CamelModel):
 
 @router.get("/health")
 def health():
-    return { "status": "healthy", "version": "3.1" }
+    return { "status": "healthy", "version": "4.0" }
 
 
 @router.post("/score", response_model=Score)

server/app/db/queries.py

@@ -1,5 +1,5 @@
 import uuid
-from typing import List, Tuple
+from typing import List, Tuple, Union
 from datetime import datetime
 
 from sqlalchemy.orm import Session
@@ -40,7 +40,7 @@ def create_game(db: Session, conf: schemas.GameConfig, coords: List[Tuple[str, f
 
 
 def get_recent_games(db: Session, count: int) -> List[Game]:
-    return db.query(Game).order_by(Game.created_at).limit(count).all()
+    return db.query(Game).order_by(Game.created_at.desc()).limit(count).all()
 
 
 def get_game(db: Session, game_id: str) -> Game:
@@ -98,7 +98,7 @@ def get_next_round_time(player: Player) -> int:
     return player.game.timer
 
 
-def add_guess(db: Session, guess: schemas.Guess, player: Player, round_number: int, score: int) -> bool:
+def add_guess(db: Session, guess: schemas.Guess, player: Player, country_code: Union[str, None], round_number: int, score: int) -> bool:
     existing = db.query(Guess).filter(Guess.player_id == player.player_id, Guess.round_number == round_number).first()
     if existing is not None:
         return False
@@ -107,7 +107,7 @@ def add_guess(db: Session, guess: schemas.Guess, player: Player, round_number: i
         round_number=round_number,
         latitude=guess.lat,
         longitude=guess.lng,
-        country_code=guess.country,
+        country_code=country_code,
         round_score=score,
         time_remaining=guess.time_remaining,
     )
@@ -120,7 +120,7 @@ def add_guess(db: Session, guess: schemas.Guess, player: Player, round_number: i
 
 
 def add_timeout(db: Session, player: Player, round_number: int) -> bool:
-    return add_guess(db, schemas.Guess(lat=0, lng=0, time_remaining=0), player, round_number, None)
+    return add_guess(db, schemas.Guess(lat=0, lng=0, time_remaining=0), player, None, round_number, None)
 
 
 def get_first_submitter(db: Session, game_id: str, round_number: int) -> str:

server/app/point_gen/__init__.py

@@ -2,11 +2,12 @@ import asyncio
 import collections
 import logging
 import random
+from itertools import groupby
 from typing import List, Tuple, Dict, Union
 
 from .random_street_view import call_random_street_view, VALID_COUNTRIES as RSV_COUNTRIES
 from .urban_centers import urban_coord, VALID_COUNTRIES as URBAN_COUNTRIES
-from .shared import aiohttp_client
+from .shared import aiohttp_client, reverse_geocode
 
 from ..schemas import GameConfig, GenMethodEnum, CountryCode, CacheInfo, GeneratorInfo
 
@@ -51,19 +52,26 @@ class PointStore:
 
     async def _gen_rsv_point(self, country: CountryCode):
         # RSV point function returns a collection of points, which should be cached
-        points = await call_random_street_view(country)
-        if len(points) > 0:
-            point = points.pop()
-            self.store[(GenMethodEnum.rsv, country)].extend(points)
-            return point
+        for actual_country, points in groupby(await call_random_street_view(country), key=lambda p: p[0]):
+            # but these points need to be cached according to the actual reverse geocoded country they are in
+            self.store[(GenMethodEnum.rsv, actual_country)].extend(points)
+        stock = self.store[(GenMethodEnum.rsv, country)]
+        if len(stock) > 0:
+            return stock.popleft()
 
     async def _gen_urban_point(self, countries: List[CountryCode], city_retries: int):
         for country in countries:
             logger.info(f"Selecting urban centers from {country}")
             pt = await urban_coord(country, city_retries=city_retries)
             if pt is not None:
-                return pt
-
+                if pt[0] == country:
+                    return pt
+                else:
+                    # TODO technically this is slightly wasted effort in rare edge cases
+                    self.store[(GenMethodEnum.urban, pt[0])].append(pt)
+
+    # TODO I think all of this logic still gets stuck in the trap of generating points even when there's a stock in some edge cases
+    # this needs a rewrite but I'm not doing that now
     async def get_point(self, generator: GenMethodEnum, country: Union[CountryCode, None], force_generate: bool = False) -> Tuple[str, float, float]:
         if country is None:
             # generating points across the whole world
@@ -159,7 +167,7 @@ class PointStore:
             await asyncio.wait_for(gathered, timeout)
         except (asyncio.TimeoutError, ExhaustedSourceError):
             # if this task times out, it's fine, as it's just intended to be a best effort
-            logger.exception(f"Failed to fully restock point cache for {config}")
+            logger.exception("Failed to fully restock a point cache!")
 
 
 points = PointStore({

server/app/point_gen/random_street_view.py

@@ -2,7 +2,7 @@ import random
 import logging
 import asyncio
 
-from .shared import aiohttp_client, point_has_streetview
+from .shared import aiohttp_client, point_has_streetview, reverse_geocode
 
 RSV_URL = "https://randomstreetview.com/data"
 VALID_COUNTRIES = ("ad", "au", "ar", "bd", "be", "bt", "bw", 
@@ -39,7 +39,8 @@ async def call_random_street_view(country_lock):
     points = []
     async def add_point_if_valid(point):
         if await point_has_streetview(point["lat"], point["lng"]):
-            points.append((country_lock, point["lat"], point["lng"]))
+            country_code = await reverse_geocode(point["lat"], point["lng"])
+            points.append((country_code or country_lock, point["lat"], point["lng"]))
     
     await asyncio.gather(*[add_point_if_valid(p) for p in rsv_js["locations"]])
     return points

server/app/point_gen/shared.py

@@ -6,6 +6,9 @@ import aiohttp
 
 from ..schemas import GenMethodEnum, CountryCode
 
+geonames_user = "terrassumptions"
+geonames_url = "http://api.geonames.org/countryCodeJSON"
+
 # Google API key, with access to Street View Static API
 # this can be safely committed due to permission restriction
 google_api_key = "AIzaSyAqjCYR6Szph0X0H_iD6O1HenFhL9jySOo"
@@ -30,3 +33,14 @@ async def point_has_streetview(lat, lng):
     async with aiohttp_client.get(metadata_url, params=params) as response:
         body = await response.json()
         return body["status"] == "OK"
+
+
+async def reverse_geocode(lat, lng):
+    params = {
+        "lat": str(lat),
+        "lng": str(lng),
+        "username": geonames_user,
+    }
+    async with aiohttp_client.get(geonames_url, params=params) as response:
+        body = await response.json()
+        return body.get("countryCode", None)

server/app/point_gen/urban_centers.py

@@ -4,7 +4,7 @@ import csv
 import logging
 from collections import defaultdict
 
-from .shared import point_has_streetview
+from .shared import point_has_streetview, reverse_geocode
 from ..scoring import mean_earth_radius_km
 
 logger = logging.getLogger(__name__)
@@ -66,4 +66,5 @@ async def urban_coord(country_lock, city_retries=10, point_retries=10, max_dist_
             pt_lng = math.degrees(pt_lng_rad)
             if await point_has_streetview(pt_lat, pt_lng):
                 logger.info("Point found!")
-                return (country_lock, pt_lat, pt_lng)
+                country_code = await reverse_geocode(pt_lat, pt_lng)
+                return (country_code or country_lock, pt_lat, pt_lng)

server/app/schemas.py

@@ -36,7 +36,6 @@ class Guess(CamelModel):
     lat: confloat(ge=-90.0, le=90.0)
     lng: confloat(ge=-180.0, le=180.0)
     time_remaining: int
-    country: Union[CountryCode, None] = None
 
 
 class CacheInfo(CamelModel):