Merge pull request #112 from QwantResearch/directions-place-id

New directions endpoint with place id parameters

idunn/api/directions.py

@@ -1,37 +1,67 @@
-from fastapi import HTTPException
-
-from pydantic import constr
-
+from fastapi import HTTPException, Query, Depends
 from starlette.requests import Request
 from starlette.responses import Response
 
 from idunn import settings
-from idunn.utils.geometry import city_surrounds_polygons
+from idunn.places import Latlon, place_from_id, InvalidPlaceId
 from idunn.utils.rate_limiter import IdunnRateLimiter
 from ..directions.client import directions_client
 
+
 rate_limiter = IdunnRateLimiter(
     resource="idunn.api.directions",
     max_requests=int(settings["DIRECTIONS_RL_MAX_REQUESTS"]),
     expire=int(settings["DIRECTIONS_RL_EXPIRE"]),
 )
 
 
-def get_directions(
-    response: Response,
+def directions_request(request: Request, response: Response):
+    """
+    FastAPI Dependency
+    Responsible for rate limit and cache headers for directions requests
+    """
+    rate_limiter.check_limit_per_client(request)
+    response.headers["cache-control"] = "max-age={}".format(settings["DIRECTIONS_CLIENT_CACHE"])
+    return request
+
+
+def get_directions_with_coordinates(
+    # URL values
     f_lon: float,
     f_lat: float,
     t_lon: float,
-    t_lat: float,  # URL values
-    request: Request,
-    type: constr(min_length=1),
-    language: str = "en",  # query parameters
+    t_lat: float,
+    # Query parameters
+    type: str,
+    language: str = "en",
+    # Request
+    request: Request = Depends(directions_request),
+):
+    from_place = Latlon(f_lat, f_lon)
+    to_place = Latlon(t_lat, t_lon)
+    if not type:
+        raise HTTPException(status_code=400, detail='"type" query param is required')
+    return directions_client.get_directions(
+        from_place, to_place, type, language, params=request.query_params
+    )
+
+
+def get_directions(
+    # Query parameters
+    origin: str = Query(..., description="Origin place id"),
+    destination: str = Query(..., description="Destination place id"),
+    type: str = Query(..., description="Transport mode"),
+    language: str = Query("en", description="User language"),
+    # Request
+    request: Request = Depends(directions_request),
 ):
     rate_limiter.check_limit_per_client(request)
-    from_position = (f_lon, f_lat)
-    to_position = (t_lon, t_lat)
+    try:
+        from_place = place_from_id(origin)
+        to_place = place_from_id(destination)
+    except InvalidPlaceId as exc:
+        raise HTTPException(status_code=404, detail=exc.message)
 
-    response.headers["cache-control"] = "max-age={}".format(settings["DIRECTIONS_CLIENT_CACHE"])
     return directions_client.get_directions(
-        from_position, to_position, type, language, params=request.query_params
+        from_place, to_place, type, language, params=request.query_params
     )

idunn/api/pages_jaunes.py

@@ -10,7 +10,7 @@
 
 
 class PjSource:
-    PLACE_ID_PREFIX = "pj:"
+    PLACE_ID_NAMESPACE = "pj"
 
     es_index = settings.get("PJ_ES_INDEX")
     es_query_template = settings.get("PJ_ES_QUERY_TEMPLATE")
@@ -54,7 +54,7 @@ def get_places_bbox(self, raw_categories, bbox, size=10, query=""):
         return [PjPOI(p["_source"]) for p in raw_places]
 
     def get_place(self, id):
-        internal_id = id.replace(self.PLACE_ID_PREFIX, "", 1)
+        internal_id = id.replace(f"{self.PLACE_ID_NAMESPACE}:", "", 1)
 
         es_places = self.es.search(
             index=self.es_index, body={"filter": {"term": {"_id": internal_id}}}

idunn/api/places.py

@@ -1,4 +1,3 @@
-import json
 import logging
 import urllib.parse
 
@@ -7,13 +6,11 @@
 from starlette.requests import Request
 
 from idunn import settings
-from idunn.utils import prometheus
 from idunn.utils.es_wrapper import get_elasticsearch
-from idunn.utils.index_names import INDICES
-from idunn.places import Place, Admin, Street, Address, POI, Latlon
+
+from idunn.places import Place, Latlon, place_from_id
 from idunn.places.base import BasePlace
-from idunn.api.utils import fetch_es_place, DEFAULT_VERBOSITY, ALL_VERBOSITY_LEVELS
-from idunn.api.pages_jaunes import pj_source
+from idunn.api.utils import DEFAULT_VERBOSITY, ALL_VERBOSITY_LEVELS
 from .closest import get_closest_place
 
 
@@ -80,34 +77,9 @@ def get_place(
     id: str, request: Request, lang: str = None, type=None, verbosity=DEFAULT_VERBOSITY
 ) -> Place:
     """Main handler that returns the requested place"""
-    es = get_elasticsearch()
     verbosity = validate_verbosity(verbosity)
     lang = validate_lang(lang)
-
-    # Handle place from "pages jaunes"
-    if id.startswith(pj_source.PLACE_ID_PREFIX):
-        pj_place = pj_source.get_place(id)
-        log_place_request(pj_place, request.headers)
-        return pj_place.load_place(lang, verbosity)
-
-    #  Otherwise handle places from the ES db
-    es_place = fetch_es_place(id, es, INDICES, type)
-
-    places = {
-        "admin": Admin,
-        "street": Street,
-        "addr": Address,
-        "poi": POI,
-    }
-    loader = places.get(es_place.get("_type"))
-
-    if loader is None:
-        prometheus.exception("FoundPlaceWithWrongType")
-        raise Exception(
-            "Place with id '{}' has a wrong type: '{}'".format(id, es_place[0].get("_type"))
-        )
-
-    place = loader(es_place["_source"])
+    place = place_from_id(id, type)
     log_place_request(place, request.headers)
     return place.load_place(lang, verbosity)
 

idunn/api/pois.py

@@ -1,9 +1,6 @@
-from elasticsearch import Elasticsearch
-
 from idunn import settings
 from idunn.places import POI
 from idunn.utils.es_wrapper import get_elasticsearch
-from idunn.utils.settings import Settings
 from idunn.api.utils import fetch_es_poi, DEFAULT_VERBOSITY
 
 

idunn/api/urls.py

@@ -4,10 +4,10 @@
 from .places_list import get_places_bbox, get_events_bbox
 from .categories import get_all_categories
 from .closest import closest_address
-from .directions import get_directions
 from ..directions.models import DirectionsResponse
 from .geocoder import get_autocomplete
 from ..geocoder.models import GeocodeJson
+from .directions import get_directions_with_coordinates, get_directions
 from ..utils.prometheus import (
     expose_metrics,
     expose_metrics_multiprocess,
@@ -33,19 +33,28 @@ def get_api_urls(settings):
     return [
         APIRoute("/metrics", metric_handler),
         APIRoute("/status", get_status),
-        # Deprecated
-        APIRoute("/pois/{id}", get_poi),
+        # Deprecated POI route
+        APIRoute("/pois/{id}", get_poi, deprecated=True),
+        # Places
         APIRoute("/places", get_places_bbox),
         APIRoute("/places/latlon:{lat}:{lon}", get_place_latlon),
         APIRoute("/places/{id}", handle_option, methods=["OPTIONS"]),
         APIRoute("/places/{id}", get_place),
+        # Categories
         APIRoute("/categories", get_all_categories),
+        # Reverse
         APIRoute("/reverse/{lat}:{lon}", closest_address),
         # Kuzzle events
         APIRoute("/events", get_events_bbox),
         # Directions
         APIRoute(
             "/directions/{f_lon},{f_lat};{t_lon},{t_lat}",
+            get_directions_with_coordinates,
+            response_model=DirectionsResponse,
+            responses={422: {"description": "Requested Path Not Allowed."}},
+        ),
+        APIRoute(
+            "/directions",
             get_directions,
             response_model=DirectionsResponse,
             responses={422: {"description": "Requested Path Not Allowed."}},

idunn/api/utils.py

@@ -23,6 +23,7 @@
     WikiUndefinedException,
 )
 from idunn.utils import prometheus
+from idunn.utils.index_names import INDICES
 import phonenumbers
 
 logger = logging.getLogger(__name__)
@@ -138,7 +139,9 @@ def fetch_es_poi(id, es) -> dict:
     This function gets from Elasticsearch the
     entry corresponding to the given id.
     """
-    es_pois = es.search(index=PLACE_POI_INDEX, body={"filter": {"term": {"_id": id}}})
+    es_pois = es.search(
+        index=PLACE_POI_INDEX, body={"filter": {"term": {"_id": id}}}, ignore_unavailable=True
+    )
 
     es_poi = es_pois.get("hits", {}).get("hits", [])
     if len(es_poi) == 0:
@@ -192,34 +195,41 @@ def fetch_bbox_places(es, indices, raw_filters, bbox, max_size) -> list:
         },
         size=max_size,
         timeout="3s",
+        ignore_unavailable=True,
     )
 
     bbox_places = bbox_places.get("hits", {}).get("hits", [])
     return bbox_places
 
 
-def fetch_es_place(id, es, indices, type) -> dict:
+def fetch_es_place(id, es, type) -> dict:
     """Returns the raw Place data
 
     This function gets from Elasticsearch the
     entry corresponding to the given id.
     """
     if type is None:
         index_name = PLACE_DEFAULT_INDEX
-    elif type not in indices:
+    elif type not in INDICES:
         raise HTTPException(status_code=400, detail=f"Wrong type parameter: type={type}")
     else:
-        index_name = indices.get(type)
+        index_name = INDICES.get(type)
 
     try:
-        es_places = es.search(index=index_name, body={"filter": {"term": {"_id": id}}})
+        es_places = es.search(
+            index=index_name, body={"filter": {"term": {"_id": id}}}, ignore_unavailable=True,
+        )
     except ElasticsearchException as error:
         logger.warning(f"error with database: {error}")
         raise HTTPException(detail="database issue", status_code=503)
 
     es_place = es_places.get("hits", {}).get("hits", [])
     if len(es_place) == 0:
-        raise HTTPException(status_code=404, detail=f"place {id} not found with type={type}")
+        if type is None:
+            message = f"place '{id}' not found"
+        else:
+            message = f"place '{id}' not found with type={type}"
+        raise HTTPException(status_code=404, detail=message)
     if len(es_place) > 1:
         logger.warning("Got multiple places with id %s", id)