Skip to content
This repository has been archived by the owner on Jun 3, 2024. It is now read-only.

add endpoint search #224

Merged
merged 8 commits into from
May 3, 2021
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 66 additions & 0 deletions idunn/api/search.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
import logging
from fastapi import Depends

from idunn import settings
from idunn.geocoder.bragi_client import bragi_client
from idunn.geocoder.nlu_client import nlu_client, NluClientException
from idunn.utils import result_filter
from idunn.instant_answer import normalize
from ..geocoder.models import QueryParams, IdunnAutocomplete

logger = logging.getLogger(__name__)

nlu_allowed_languages = settings["NLU_ALLOWED_LANGUAGES"].split(",")


def no_search_result(query=None, lang=None):
if query is not None:
logger.info(
"search: no result",
extra={
"request": {
"query": query,
"lang": lang,
}
},
)
return IdunnAutocomplete()


async def search(query: QueryParams = Depends(QueryParams)) -> IdunnAutocomplete:
amatissart marked this conversation as resolved.
Show resolved Hide resolved
"""
Perform a query which is intended to display a relevant result directly (as
opposed to `autocomplete` which gives a list of plausible results).

Similarly to `instant_answer`, the result will need some quality checks.
"""
query.q = normalize(query.q)

if query.lang in nlu_allowed_languages:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if query.lang in nlu_allowed_languages:
if query.nlu and query.lang in nlu_allowed_languages:

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We also want to keep this option overridden by AUTOCOMPLETE_NLU_SHADOW_ENABLED, right?

Copy link
Contributor

@amatissart amatissart May 3, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, this option has never been actually used, and is probably not very useful now that the nlu=true is used by the application in most cases.
So I think this flag AUTOCOMPLETE_NLU_SHADOW_ENABLED can be ignored for /search.

try:
intentions = await nlu_client.get_intentions(text=query.q, lang=query.lang)
if intentions:
return IdunnAutocomplete(intentions=intentions)
except NluClientException:
# No intention could be interpreted from query
pass

# Direct geocoding query
bragi_response = await bragi_client.raw_autocomplete(
{"q": query.q, "lang": query.lang, "limit": 5}
)

features = sorted(
(
(result_filter.rank_bragi_response(query.q, geocoding), feature)
for feature in bragi_response["features"]
for geocoding in [feature["properties"]["geocoding"]]
if result_filter.check_bragi_response(query.q, geocoding)
),
key=lambda item: -item[0], # sort by descending rank
)

if not features:
return no_search_result(query=query.q, lang=query.lang)

return IdunnAutocomplete(features=[features[0][1]])
6 changes: 6 additions & 0 deletions idunn/api/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
from .urlsolver import follow_redirection
from .instant_answer import get_instant_answer, InstantAnswerResponse
from ..places.place import Address, Place
from .search import search
from ..utils.prometheus import (
expose_metrics,
expose_metrics_multiprocess,
Expand Down Expand Up @@ -74,6 +75,11 @@ def get_api_urls(settings):
response_model=IdunnAutocomplete,
response_model_exclude_unset=True,
),
APIRoute(
"/search",
search,
response_model=IdunnAutocomplete,
),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For consistency with "/autocomplete", methods and response_model_exclude_unset should be defined too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... and we should allow to provide extra parameters through POST queries 👍

# Solve URLs
APIRoute(
"/redirect",
Expand Down
2 changes: 1 addition & 1 deletion idunn/geocoder/models/geocodejson.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ class GeocodingPlace(BaseModel):

# The following fields are part of the GeocodeJson specification but are
# currently disabled in Bragi:
#  https://github.com/CanalTP/mimirsbrunn/blob/master/libs/bragi/src/model.rs#L194-L199
# https://github.com/CanalTP/mimirsbrunn/blob/master/libs/bragi/src/model.rs#L194-L199
#
# accuracy: Optional[PositiveInt]
# district: Optional[str]
Expand Down
29 changes: 29 additions & 0 deletions tests/test_search.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import pytest
from fastapi.testclient import TestClient
from app import app

from .fixtures.autocomplete import (
mock_autocomplete_get,
mock_NLU_with_city,
mock_NLU_with_brand_and_city,
mock_bragi_carrefour_in_bbox,
)


def test_search_paris(mock_autocomplete_get, mock_NLU_with_city):
client = TestClient(app)
response = client.get("/v1/search", params={"q": "paris", "lang": "fr"})
assert response.status_code == 200
response_json = response.json()
place = response_json["features"][0]["properties"]["geocoding"]
assert place["name"] == "Paris"


def test_search_intention_full_text(mock_NLU_with_brand_and_city, mock_autocomplete_get):
client = TestClient(app)
response = client.get("/v1/search", params={"q": "auchan à paris", "lang": "fr"})
assert response.status_code == 200
response_json = response.json()
intention = response_json["intentions"][0]["filter"]
assert intention["q"] == "auchan"
assert intention["bbox"] is not None