-
Notifications
You must be signed in to change notification settings - Fork 6
feat: locations endpoint #1767
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
cka-y
wants to merge
7
commits into
main
Choose a base branch
from
feat/loc-hierarchy
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
feat: locations endpoint #1767
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
f239ab7
wip: locations endpoint
cka-y 5563371
fix: materialized view checks
cka-y 6312296
fix: materialized view on subdivision that are countries + rank + tests
cka-y 079e681
fix: tests
cka-y 4a6a635
fix: restoring userservice files
cka-y 7d33747
improved documentation and fixed coding inconsistency
cka-y e4741ed
improved documentation and fixed coding inconsistency
cka-y File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,118 @@ | ||
| import re | ||
|
|
||
| from sqlalchemy import and_, func, select | ||
| from sqlalchemy.orm import Session | ||
|
|
||
| from feeds_gen.apis.locations_api_base import BaseLocationsApi | ||
| from feeds_gen.models.location_search_response import LocationSearchResponse | ||
| from shared.database.database import with_db_session | ||
| from shared.database.sql_functions.unaccent import unaccent | ||
| from shared.database_gen.sqlacodegen_models import t_geopolygonlocationsearch as location_search | ||
| from shared.db_models.location_search_result_impl import LocationSearchResultImpl | ||
|
|
||
|
|
||
| def _to_prefix_tsquery(search_query: str) -> str | None: | ||
| # Full-text search matches whole lexemes, so turn each word into a prefix | ||
| # term ("mon" -> "mon:*") to support typeahead-style matching. | ||
| tokens = re.findall(r"\w+", search_query, flags=re.UNICODE) | ||
| if not tokens: | ||
| return None | ||
| return " & ".join(f"{token}:*" for token in tokens) | ||
|
|
||
|
|
||
| def _ts_query(search_query: str): | ||
| return func.to_tsquery("english", unaccent(_to_prefix_tsquery(search_query))) | ||
|
|
||
|
|
||
| def _normalize_search_query(search_query: str | None) -> str | None: | ||
| if search_query is None or len(search_query.strip()) == 0: | ||
| return None | ||
| if _to_prefix_tsquery(search_query) is None: | ||
| return None | ||
| return search_query.strip() | ||
|
|
||
|
|
||
| def _normalize_location_code(location_code: str | None) -> str | None: | ||
| if location_code is None or len(location_code.strip()) == 0: | ||
| return None | ||
| return location_code.strip().upper() | ||
|
|
||
|
|
||
| def _build_locations_conditions( | ||
| search_query: str | None, | ||
| country_code: str | None, | ||
| subdivision_code: str | None, | ||
| location_type: str | None, | ||
| ): | ||
| normalized_query = _normalize_search_query(search_query) | ||
| normalized_country = _normalize_location_code(country_code) | ||
| normalized_subdivision = _normalize_location_code(subdivision_code) | ||
| conditions = [] | ||
| if normalized_query is not None: | ||
| conditions.append(location_search.c.document.op("@@")(_ts_query(normalized_query))) | ||
| if normalized_country is not None: | ||
| conditions.append(location_search.c.country_code == normalized_country) | ||
| if normalized_subdivision is not None: | ||
| conditions.append(location_search.c.subdivision_code == normalized_subdivision) | ||
| if location_type is not None: | ||
| conditions.append(location_search.c.location_type == location_type) | ||
| return conditions, normalized_query | ||
|
|
||
|
|
||
| class LocationsApiImpl(BaseLocationsApi): | ||
| """This class represents the implementation of the `/locations` endpoints.""" | ||
|
|
||
| @with_db_session | ||
| def get_locations( | ||
| self, | ||
| limit: int, | ||
| offset: int, | ||
| search_query: str, | ||
| country_code: str, | ||
| subdivision_code: str, | ||
| location_type: str, | ||
| db_session: Session, | ||
| ) -> LocationSearchResponse: | ||
| """Search locations from the geopolygon materialized read model.""" | ||
| conditions, normalized_query = _build_locations_conditions( | ||
| search_query, country_code, subdivision_code, location_type | ||
| ) | ||
|
|
||
| count_query = select(func.count(location_search.c.osm_id)) | ||
| if conditions: | ||
| count_query = count_query.where(and_(*conditions)) | ||
| total = db_session.execute(count_query).scalar_one() | ||
|
|
||
| locations_query = select( | ||
| location_search.c.osm_id, | ||
| location_search.c.parent_osm_id, | ||
| location_search.c.name, | ||
| location_search.c.alt_name, | ||
| location_search.c.location_type, | ||
| location_search.c.country_name, | ||
| location_search.c.country_code, | ||
| location_search.c.subdivision_name, | ||
| location_search.c.subdivision_code, | ||
| location_search.c.path_names, | ||
| location_search.c.display_name, | ||
| ) | ||
| if conditions: | ||
| locations_query = locations_query.where(and_(*conditions)) | ||
| if normalized_query is not None: | ||
| locations_query = locations_query.order_by( | ||
| location_search.c.admin_level.asc(), | ||
| func.ts_rank(location_search.c.document, _ts_query(normalized_query)).desc(), | ||
| location_search.c.display_name.asc(), | ||
| ) | ||
| else: | ||
| locations_query = locations_query.order_by( | ||
| location_search.c.admin_level.asc(), | ||
| location_search.c.display_name.asc(), | ||
| ) | ||
| locations_query = locations_query.limit(limit).offset(offset) | ||
|
|
||
| rows = db_session.execute(locations_query).all() | ||
| return LocationSearchResponse( | ||
| total=total, | ||
| results=[LocationSearchResultImpl.from_orm(row) for row in rows], | ||
| ) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| from feeds_gen.models.location_search_result import LocationSearchResult | ||
|
|
||
| from shared.database_gen.sqlacodegen_models import t_geopolygonlocationsearch | ||
|
|
||
|
|
||
| class LocationSearchResultImpl(LocationSearchResult): | ||
| """Implementation of the `LocationSearchResult` model. | ||
| This class converts a SQLAlchemy row from the geopolygon location search read model | ||
| to a Pydantic model instance.""" | ||
|
|
||
| class Config: | ||
| """Pydantic configuration. | ||
| Enabling `from_orm` method to create a model instance from a SQLAlchemy row object.""" | ||
|
|
||
| from_attributes = True | ||
|
|
||
| @classmethod | ||
| def from_orm(cls, location_row: t_geopolygonlocationsearch): | ||
| """Create a model instance from a SQLAlchemy row object.""" | ||
| if location_row is None: | ||
| return None | ||
| return cls( | ||
| location_id=location_row.osm_id, | ||
| parent_location_id=location_row.parent_osm_id, | ||
| name=location_row.name, | ||
| alt_name=location_row.alt_name, | ||
| location_type=location_row.location_type, | ||
| country_name=location_row.country_name, | ||
| country_code=location_row.country_code, | ||
| subdivision_name=location_row.subdivision_name, | ||
| subdivision_code=location_row.subdivision_code, | ||
| path_names=location_row.path_names or [], | ||
| display_name=location_row.display_name, | ||
| ) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,159 @@ | ||
| from types import SimpleNamespace | ||
| from unittest.mock import MagicMock | ||
|
|
||
| from feeds.impl.locations_api_impl import ( | ||
| LocationsApiImpl, | ||
| _build_locations_conditions, | ||
| _normalize_location_code, | ||
| _normalize_search_query, | ||
| _to_prefix_tsquery, | ||
| ) | ||
| from feeds_gen.models.location_search_response import LocationSearchResponse | ||
| from feeds_gen.models.location_search_result import LocationSearchResult | ||
| from shared.db_models.location_search_result_impl import LocationSearchResultImpl | ||
|
|
||
|
|
||
| def test_to_prefix_tsquery_single_word(): | ||
| assert _to_prefix_tsquery("mon") == "mon:*" | ||
|
|
||
|
|
||
| def test_to_prefix_tsquery_multiple_words_are_anded(): | ||
| assert _to_prefix_tsquery("los angeles") == "los:* & angeles:*" | ||
|
|
||
|
|
||
| def test_to_prefix_tsquery_strips_operator_characters(): | ||
| # tsquery operators embedded in user input must not leak into the query. | ||
| assert _to_prefix_tsquery("a & b | c") == "a:* & b:* & c:*" | ||
|
|
||
|
|
||
| def test_to_prefix_tsquery_returns_none_without_word_characters(): | ||
| assert _to_prefix_tsquery("!!!") is None | ||
| assert _to_prefix_tsquery("") is None | ||
|
|
||
|
|
||
| def test_normalize_search_query(): | ||
| assert _normalize_search_query(None) is None | ||
| assert _normalize_search_query(" ") is None | ||
| assert _normalize_search_query("!!!") is None | ||
| assert _normalize_search_query(" montreal ") == "montreal" | ||
|
|
||
|
|
||
| def test_normalize_location_code_trims_and_uppercases(): | ||
| assert _normalize_location_code(None) is None | ||
| assert _normalize_location_code(" ") is None | ||
| assert _normalize_location_code(" ca ") == "CA" | ||
| assert _normalize_location_code("ca-qc") == "CA-QC" | ||
|
|
||
|
|
||
| def test_build_conditions_no_filters(): | ||
| conditions, normalized_query = _build_locations_conditions(None, None, None, None) | ||
| assert conditions == [] | ||
| assert normalized_query is None | ||
|
|
||
|
|
||
| def test_build_conditions_all_filters(): | ||
| conditions, normalized_query = _build_locations_conditions("montreal", "ca", "ca-qc", "municipality") | ||
| # search_query + country_code + subdivision_code + location_type | ||
| assert len(conditions) == 4 | ||
| assert normalized_query == "montreal" | ||
|
|
||
|
|
||
| def test_build_conditions_ignores_blank_search_query(): | ||
| conditions, normalized_query = _build_locations_conditions(" ", "CA", None, None) | ||
| # Only the country filter remains; blank search query is dropped. | ||
| assert len(conditions) == 1 | ||
| assert normalized_query is None | ||
|
|
||
|
|
||
| def test_location_from_row_maps_fields(): | ||
| row = SimpleNamespace( | ||
| osm_id=1634158, | ||
| parent_osm_id=8508277, | ||
| name="Montreal", | ||
| alt_name=None, | ||
| location_type="municipality", | ||
| country_name="Canada", | ||
| country_code="CA", | ||
| subdivision_name="Quebec", | ||
| subdivision_code="CA-QC", | ||
| path_names=["Canada", "Quebec", "Montreal"], | ||
| display_name="Canada, Quebec, Montreal", | ||
| ) | ||
| result = LocationSearchResultImpl.from_orm(row) | ||
| assert isinstance(result, LocationSearchResult) | ||
| assert result.location_id == 1634158 | ||
| assert result.parent_location_id == 8508277 | ||
| assert result.country_code == "CA" | ||
| assert result.subdivision_code == "CA-QC" | ||
| assert result.path_names == ["Canada", "Quebec", "Montreal"] | ||
|
|
||
|
|
||
| def test_location_from_row_defaults_missing_path_names_to_empty_list(): | ||
| row = SimpleNamespace( | ||
| osm_id=1, | ||
| parent_osm_id=None, | ||
| name="Canada", | ||
| alt_name=None, | ||
| location_type="country", | ||
| country_name="Canada", | ||
| country_code="CA", | ||
| subdivision_name=None, | ||
| subdivision_code=None, | ||
| path_names=None, | ||
| display_name="Canada", | ||
| ) | ||
| result = LocationSearchResultImpl.from_orm(row) | ||
| assert result.path_names == [] | ||
|
|
||
|
|
||
| def test_location_from_row_returns_none_for_none_row(): | ||
| assert LocationSearchResultImpl.from_orm(None) is None | ||
|
|
||
|
|
||
| def _mock_session(total, rows): | ||
| """Build a session whose two execute() calls return the count then the rows.""" | ||
| count_result = MagicMock() | ||
| count_result.scalar_one.return_value = total | ||
|
|
||
| rows_result = MagicMock() | ||
| rows_result.all.return_value = rows | ||
|
|
||
| session = MagicMock() | ||
| session.execute.side_effect = [count_result, rows_result] | ||
| return session | ||
|
|
||
|
|
||
| def test_get_locations_returns_total_and_results(): | ||
| rows = [ | ||
| SimpleNamespace( | ||
| osm_id=8508277, | ||
| parent_osm_id=61549, | ||
| name="Urban agglomeration of Montreal", | ||
| alt_name=None, | ||
| location_type="municipality", | ||
| country_name="Canada", | ||
| country_code="CA", | ||
| subdivision_name="Quebec", | ||
| subdivision_code="CA-QC", | ||
| path_names=["Canada", "Quebec", "Urban agglomeration of Montreal"], | ||
| display_name="Canada, Quebec, Urban agglomeration of Montreal", | ||
| ) | ||
| ] | ||
| session = _mock_session(total=17, rows=rows) | ||
|
|
||
| response = LocationsApiImpl().get_locations(5, 0, "montreal", None, "CA-QC", None, db_session=session) | ||
|
|
||
| assert isinstance(response, LocationSearchResponse) | ||
| assert response.total == 17 | ||
| assert len(response.results) == 1 | ||
| assert response.results[0].location_id == 8508277 | ||
| # total is serialized before results. | ||
| assert list(response.model_dump().keys())[0] == "total" | ||
| assert session.execute.call_count == 2 | ||
|
|
||
|
|
||
| def test_get_locations_empty_result(): | ||
| session = _mock_session(total=0, rows=[]) | ||
| response = LocationsApiImpl().get_locations(10, 0, None, None, None, None, db_session=session) | ||
| assert response.total == 0 | ||
| assert response.results == [] |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixes unaccent warning in logs