From f239ab7d747d08c22e65a585aecde7e28e086f69 Mon Sep 17 00:00:00 2001 From: cka-y Date: Mon, 20 Jul 2026 10:21:05 -0400 Subject: [PATCH 1/7] wip: locations endpoint --- api/.openapi-generator/FILES | 24 +-- api/src/feeds/impl/locations_api_impl.py | 145 ++++++++++++++ api/src/main.py | 2 + docs/DatabaseCatalogAPI.yaml | 111 ++++++++++ .../reverse_geolocation_populate/src/main.py | 189 ++++++++++++++++++ .../test_reverse_geolocation_populate.py | 19 +- liquibase/changelog.xml | 6 +- liquibase/changes/feat_pt_202.sql | 15 ++ .../geopolygon_location_search.sql | 71 +++++++ .../materialized_views/materialized_views.xml | 10 + 10 files changed, 570 insertions(+), 22 deletions(-) create mode 100644 api/src/feeds/impl/locations_api_impl.py create mode 100644 liquibase/changes/feat_pt_202.sql create mode 100644 liquibase/materialized_views/geopolygon_location_search.sql diff --git a/api/.openapi-generator/FILES b/api/.openapi-generator/FILES index 35fcc3da8..c36805165 100644 --- a/api/.openapi-generator/FILES +++ b/api/.openapi-generator/FILES @@ -6,6 +6,8 @@ src/feeds_gen/apis/feeds_api.py src/feeds_gen/apis/feeds_api_base.py src/feeds_gen/apis/licenses_api.py src/feeds_gen/apis/licenses_api_base.py +src/feeds_gen/apis/locations_api.py +src/feeds_gen/apis/locations_api_base.py src/feeds_gen/apis/metadata_api.py src/feeds_gen/apis/metadata_api_base.py src/feeds_gen/apis/search_api.py @@ -35,6 +37,9 @@ src/feeds_gen/models/license_base.py src/feeds_gen/models/license_rule.py src/feeds_gen/models/license_with_rules.py src/feeds_gen/models/location.py +src/feeds_gen/models/location_hierarchy_item.py +src/feeds_gen/models/location_search_response.py +src/feeds_gen/models/location_search_result.py src/feeds_gen/models/matching_license.py src/feeds_gen/models/metadata.py src/feeds_gen/models/redirect.py @@ -43,22 +48,3 @@ src/feeds_gen/models/search_feeds200_response.py src/feeds_gen/models/source_info.py src/feeds_gen/models/validation_report.py src/feeds_gen/security_api.py -src/user_service/impl/__init__.py -src/user_service_gen/apis/__init__.py -src/user_service_gen/apis/notifications_api.py -src/user_service_gen/apis/notifications_api_base.py -src/user_service_gen/apis/subscriptions_api.py -src/user_service_gen/apis/subscriptions_api_base.py -src/user_service_gen/apis/users_api.py -src/user_service_gen/apis/users_api_base.py -src/user_service_gen/main.py -src/user_service_gen/models/__init__.py -src/user_service_gen/models/create_notification_subscription_request.py -src/user_service_gen/models/extra_models.py -src/user_service_gen/models/feature_flag.py -src/user_service_gen/models/notification_subscription.py -src/user_service_gen/models/notification_type.py -src/user_service_gen/models/update_notification_subscription_request.py -src/user_service_gen/models/update_user_request.py -src/user_service_gen/models/user_profile.py -src/user_service_gen/security_api.py diff --git a/api/src/feeds/impl/locations_api_impl.py b/api/src/feeds/impl/locations_api_impl.py new file mode 100644 index 000000000..2f698a851 --- /dev/null +++ b/api/src/feeds/impl/locations_api_impl.py @@ -0,0 +1,145 @@ +from fastapi import APIRouter, Query, Security +from pydantic import BaseModel, Field +from sqlalchemy import and_, func, select +from sqlalchemy.orm import Session + +from feeds_gen.models.extra_models import TokenModel +from feeds_gen.security_api import get_token_Authentication +from shared.database.database import with_db_session +from shared.database_gen.sqlacodegen_models import t_geopolygonlocationsearch as location_search + +router = APIRouter() + + +class LocationSearchResult(BaseModel): + location_id: int + parent_location_id: int | None = None + name: str | None = None + alt_name: str | None = None + location_type: str + country_name: str | None = None + country_code: str | None = None + subdivision_name: str | None = None + subdivision_code: str | None = None + path_names: list[str] = Field(default_factory=list) + display_name: str | None = None + + +class LocationsSearchResponse(BaseModel): + results: list[LocationSearchResult] + total: int + + +def _normalize_search_query(search_query: str | None) -> str | None: + if search_query is None or len(search_query.strip()) == 0: + return None + return search_query.strip() + + +def _normalize_country_code(country_code: str | None) -> str | None: + if country_code is None or len(country_code.strip()) == 0: + return None + return country_code.strip().upper() + + +def _build_locations_conditions(search_query: str | None, country_code: str | None, location_type: str | None): + normalized_query = _normalize_search_query(search_query) + normalized_country = _normalize_country_code(country_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 location_type is not None: + conditions.append(location_search.c.location_type == location_type) + return conditions, normalized_query + + +def _ts_query(search_query: str): + return func.plainto_tsquery("english", func.unaccent(search_query)) + + +def _location_from_row(row) -> LocationSearchResult: + return LocationSearchResult( + location_id=row["osm_id"], + parent_location_id=row["parent_osm_id"], + name=row["name"], + alt_name=row["alt_name"], + location_type=row["location_type"], + country_name=row["country_name"], + country_code=row["country_code"], + subdivision_name=row["subdivision_name"], + subdivision_code=row["subdivision_code"], + path_names=row["path_names"] or [], + display_name=row["display_name"], + ) + + +@with_db_session +def search_locations( + limit: int, + offset: int, + search_query: str | None, + country_code: str | None, + location_type: str | None, + db_session: Session, +) -> LocationsSearchResponse: + conditions, normalized_query = _build_locations_conditions(search_query, country_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( + 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.display_name.asc()) + locations_query = locations_query.limit(limit).offset(offset) + + rows = db_session.execute(locations_query).mappings().all() + return LocationsSearchResponse( + results=[_location_from_row(row) for row in rows], + total=total, + ) + + +@router.get( + "/v1/locations", + tags=["locations"], + response_model=LocationsSearchResponse, + response_model_by_alias=True, +) +def get_locations( + limit: int = Query(100, description="The number of locations to return.", alias="limit", ge=0, le=3500), + offset: int = Query(0, description="Offset of the first location to return.", alias="offset", ge=0), + search_query: str = Query(None, description="Location search query.", alias="search_query"), + country_code: str = Query(None, description="ISO 3166-1 alpha-2 country code.", alias="country_code"), + location_type: str = Query( + None, + description="Location type to return: country, subdivision, or municipality.", + alias="location_type", + pattern="^(country|subdivision|municipality)$", + ), + token_Authentication: TokenModel = Security(get_token_Authentication), +) -> LocationsSearchResponse: + """Search locations from the geopolygon materialized read model.""" + return search_locations(limit, offset, search_query, country_code, location_type) diff --git a/api/src/main.py b/api/src/main.py index 097616b67..9ff7067ab 100644 --- a/api/src/main.py +++ b/api/src/main.py @@ -25,6 +25,7 @@ from feeds_gen.apis.metadata_api import router as MetadataApiRouter from feeds_gen.apis.search_api import router as SearchApiRouter from feeds_gen.apis.licenses_api import router as LicensesApiRouter +from feeds.impl.locations_api_impl import router as LocationsApiRouter from user_service_gen.apis.users_api import router as UsersApiRouter from user_service_gen.apis.notifications_api import router as NotificationsApiRouter from user_service_gen.apis.subscriptions_api import router as SubscriptionsApiRouter @@ -59,6 +60,7 @@ app.include_router(MetadataApiRouter) app.include_router(SearchApiRouter) app.include_router(LicensesApiRouter) +app.include_router(LocationsApiRouter) # The user-service routes are generated as ``async def`` but call blocking # synchronous impls (database + Brevo HTTP). Offload them to the threadpool so a # slow Brevo call cannot freeze the event loop for every other request. diff --git a/docs/DatabaseCatalogAPI.yaml b/docs/DatabaseCatalogAPI.yaml index 537e23b2e..d6cc1b308 100644 --- a/docs/DatabaseCatalogAPI.yaml +++ b/docs/DatabaseCatalogAPI.yaml @@ -37,6 +37,8 @@ tags: description: "Beta endpoints of the API." - name: "licenses" description: "Licenses of the Mobility Database" + - name: "locations" + description: "Locations in the Mobility Database" paths: /v1/feeds: @@ -390,6 +392,47 @@ paths: items: $ref: "#/components/schemas/SearchFeedItemResult" + /v1/locations: + get: + description: Search locations in the Mobility Database. + operationId: getLocations + tags: + - "locations" + parameters: + - name: limit + in: query + description: The number of locations to return. + schema: + type: integer + minimum: 0 + maximum: 3500 + default: 100 + - $ref: "#/components/parameters/offset" + - name: search_query + in: query + description: Location search query. + schema: + type: string + - $ref: "#/components/parameters/country_code" + - name: location_type + in: query + description: Filter by location type. + schema: + type: string + enum: + - country + - subdivision + - municipality + security: + - Authentication: [] + responses: + 200: + description: Successful search of locations. + content: + application/json: + schema: + $ref: "#/components/schemas/LocationSearchResponse" + /v1/licenses: get: description: Get the list of all licenses in the DB. @@ -1233,6 +1276,74 @@ components: type: string example: Los Angeles + LocationSearchResponse: + type: object + properties: + total: + type: integer + description: The total number of matching locations regardless of limit and offset. + results: + type: array + items: + $ref: "#/components/schemas/LocationSearchResult" + + LocationSearchResult: + type: object + properties: + location_id: + type: integer + description: Stable location identifier. + example: 175905 + parent_location_id: + type: integer + nullable: true + description: Stable identifier of the nearest containing location. + example: 161950 + name: + type: string + nullable: true + example: Boston + alt_name: + type: string + nullable: true + example: City of Boston + location_type: + type: string + enum: + - country + - subdivision + - municipality + example: municipality + country_name: + type: string + nullable: true + example: United States + country_code: + type: string + nullable: true + example: US + subdivision_name: + type: string + nullable: true + example: Massachusetts + subdivision_code: + type: string + nullable: true + example: US-MA + path_names: + type: array + items: + type: string + example: + - United States + - Massachusetts + - Suffolk County + - Boston + display_name: + type: string + nullable: true + example: United States, Massachusetts, Suffolk County, Boston + # Have to put the enum inline because of a bug in openapi-generator # FeedStatus: # description: > diff --git a/functions-python/reverse_geolocation_populate/src/main.py b/functions-python/reverse_geolocation_populate/src/main.py index db50b30a3..6dd407186 100644 --- a/functions-python/reverse_geolocation_populate/src/main.py +++ b/functions-python/reverse_geolocation_populate/src/main.py @@ -6,11 +6,16 @@ import pycountry from geoalchemy2 import WKTElement from google.cloud import bigquery +from sqlalchemy import func, literal, select, true +from sqlalchemy.dialects.postgresql import insert +from sqlalchemy.orm import aliased +from sqlalchemy.schema import DDL from shared.database_gen.sqlacodegen_models import Geopolygon from shared.database.database import with_db_session from shared.helpers.logger import init_logger from enum import Enum +from shared.database_gen.sqlacodegen_models import Geopolygonhierarchy # Initialize logging init_logger() @@ -209,6 +214,187 @@ def save_to_database(data, db_session=None): db_session.commit() +def get_saved_geopolygon_rows(data): + """Get rows that will be saved to the geopolygon table.""" + rows_by_osm_id = {} + for row in data: + if row.get("osm_id") is not None and row.get("name") and row.get("geometry"): + rows_by_osm_id[row["osm_id"]] = row + return list(rows_by_osm_id.values()) + + +def _upsert_hierarchy_rows(db_session, rows): + """Upsert precomputed hierarchy edges keyed by osm_id.""" + hierarchy_table = Geopolygonhierarchy.__table__ + statement = insert(hierarchy_table).values(rows) + db_session.execute( + statement.on_conflict_do_update( + index_elements=[hierarchy_table.c.osm_id], + set_={ + "parent_osm_id": statement.excluded.parent_osm_id, + "country_osm_id": statement.excluded.country_osm_id, + "subdivision_osm_id": statement.excluded.subdivision_osm_id, + "updated_at": func.current_timestamp(), + }, + ) + ) + + +def _upsert_locality_hierarchy( + db_session, locality_osm_ids, subdivision_osm_ids, country_osm_id +): + """Match localities to their parents within the current run and upsert the edges. + + ``parent_osm_id`` is the deepest lower-admin-level polygon from this run that + covers the locality (so nested localities keep their real parent), while + ``subdivision_osm_id`` records the covering subdivision for direct lookups. Each + locality's representative point is computed once, and containment is tested with + ``ST_Covers`` against the raw (spatially indexed) candidate geometry. Point-in- + polygon is robust to invalid polygons, so the large candidate geometries are + never repaired with ``ST_MakeValid`` here. + """ + hierarchy_table = Geopolygonhierarchy.__table__ + child = aliased(Geopolygon) + parent_candidate = aliased(Geopolygon) + subdivision_candidate = aliased(Geopolygon) + parent_candidate_ids = subdivision_osm_ids + locality_osm_ids + + locality_points = ( + select( + child.osm_id.label("osm_id"), + child.admin_level.label("admin_level"), + func.ST_PointOnSurface(func.ST_MakeValid(child.geometry)).label("point"), + ) + .where(child.osm_id.in_(locality_osm_ids)) + .where(child.geometry.isnot(None)) + .cte("locality_points") + ) + + parent = ( + select(parent_candidate.osm_id.label("parent_osm_id")) + .where(parent_candidate.osm_id.in_(parent_candidate_ids)) + .where(parent_candidate.admin_level < locality_points.c.admin_level) + .where(parent_candidate.geometry.op("&&")(locality_points.c.point)) + .where(func.ST_Covers(parent_candidate.geometry, locality_points.c.point)) + .order_by( + parent_candidate.admin_level.desc(), + func.ST_Area(parent_candidate.geometry).asc(), + ) + .limit(1) + .lateral("parent_match") + ) + + subdivision = ( + select(subdivision_candidate.osm_id.label("subdivision_osm_id")) + .where(subdivision_candidate.osm_id.in_(subdivision_osm_ids)) + .where(subdivision_candidate.geometry.op("&&")(locality_points.c.point)) + .where(func.ST_Covers(subdivision_candidate.geometry, locality_points.c.point)) + .order_by(func.ST_Area(subdivision_candidate.geometry).asc()) + .limit(1) + .lateral("subdivision_match") + ) + + matched = select( + locality_points.c.osm_id.label("osm_id"), + func.coalesce(parent.c.parent_osm_id, literal(country_osm_id)).label( + "parent_osm_id" + ), + literal(country_osm_id).label("country_osm_id"), + subdivision.c.subdivision_osm_id.label("subdivision_osm_id"), + func.current_timestamp().label("updated_at"), + ).select_from( + locality_points.outerjoin(parent, true()).outerjoin(subdivision, true()) + ) + + statement = insert(hierarchy_table).from_select( + [ + "osm_id", + "parent_osm_id", + "country_osm_id", + "subdivision_osm_id", + "updated_at", + ], + matched, + ) + db_session.execute( + statement.on_conflict_do_update( + index_elements=[hierarchy_table.c.osm_id], + set_={ + "parent_osm_id": statement.excluded.parent_osm_id, + "country_osm_id": statement.excluded.country_osm_id, + "subdivision_osm_id": statement.excluded.subdivision_osm_id, + "updated_at": statement.excluded.updated_at, + }, + ) + ) + + +@with_db_session +def update_geopolygon_hierarchy(saved_rows, db_session=None): + """Update hierarchy edges for the geopolygons saved in the current populate run. + + Country and subdivision edges are derived directly from the ISO codes collected + during the run, so they require no spatial computation. Localities are the only + rows matched by geometry, and only against this run's subdivisions. + """ + if not saved_rows: + logging.info("Skipping hierarchy update because no geopolygons were saved.") + return + + logging.info("Updating hierarchy for %s geopolygons.", len(saved_rows)) + country_rows = [row for row in saved_rows if row.get("iso3166_1")] + subdivision_rows = [ + row for row in saved_rows if not row.get("iso3166_1") and row.get("iso3166_2") + ] + locality_osm_ids = [ + row["osm_id"] + for row in saved_rows + if not row.get("iso3166_1") and not row.get("iso3166_2") + ] + country_osm_id = country_rows[0]["osm_id"] if country_rows else None + subdivision_osm_ids = [row["osm_id"] for row in subdivision_rows] + + direct_hierarchy_rows = [ + { + "osm_id": row["osm_id"], + "parent_osm_id": None, + "country_osm_id": None, + "subdivision_osm_id": None, + } + for row in country_rows + ] + direct_hierarchy_rows.extend( + { + "osm_id": row["osm_id"], + "parent_osm_id": country_osm_id, + "country_osm_id": country_osm_id, + "subdivision_osm_id": None, + } + for row in subdivision_rows + ) + if direct_hierarchy_rows: + _upsert_hierarchy_rows(db_session, direct_hierarchy_rows) + + if locality_osm_ids: + _upsert_locality_hierarchy( + db_session, locality_osm_ids, subdivision_osm_ids, country_osm_id + ) + + db_session.commit() + logging.info("Hierarchy update completed for %s geopolygons.", len(saved_rows)) + + +@with_db_session +def refresh_location_search_view(db_session=None): + """Refresh the location search read model after geopolygon updates.""" + logging.info("Refreshing GeopolygonLocationSearch materialized view.") + db_session.execute( + DDL("REFRESH MATERIALIZED VIEW CONCURRENTLY GeopolygonLocationSearch") + ) + db_session.commit() + logging.info("GeopolygonLocationSearch materialized view refreshed.") + + @functions_framework.http def reverse_geolocation_populate(request): """ @@ -269,7 +455,10 @@ def reverse_geolocation_populate(request): data.extend( fetch_data(level, country_code, LocationType.LOCALITY, country_name) ) + saved_rows = get_saved_geopolygon_rows(data) save_to_database(data) + update_geopolygon_hierarchy(saved_rows) + refresh_location_search_view() result = f"Database initialized for {country_code}." logging.info(result) return result, 200 diff --git a/functions-python/reverse_geolocation_populate/tests/test_reverse_geolocation_populate.py b/functions-python/reverse_geolocation_populate/tests/test_reverse_geolocation_populate.py index 3ce451529..0a2e04c5f 100644 --- a/functions-python/reverse_geolocation_populate/tests/test_reverse_geolocation_populate.py +++ b/functions-python/reverse_geolocation_populate/tests/test_reverse_geolocation_populate.py @@ -233,11 +233,21 @@ def test_get_admin_levels(self, _): self.assertIsNotNone(result) self.assertEqual(result, [6, 8]) + def test_update_geopolygon_hierarchy_skips_empty_rows(self): + from main import update_geopolygon_hierarchy + + db_session = MagicMock() + update_geopolygon_hierarchy([], db_session=db_session) + db_session.execute.assert_not_called() + db_session.commit.assert_not_called() + @patch("main.bigquery") @patch("main.parse_request_parameters") @patch("main.fetch_country_admin_levels") @patch("main.fetch_data") @patch("main.save_to_database") + @patch("main.update_geopolygon_hierarchy") + @patch("main.refresh_location_search_view") @patch("main.fetch_subdivision_admin_levels") @patch.dict( "os.environ", @@ -249,7 +259,9 @@ def test_get_admin_levels(self, _): def test_reverse_geolocation_populate( self, mock_fetch_subdivision_admin_lvl, - __, + mock_refresh_location_search_view, + mock_update_geopolygon_hierarchy, + mock_save_to_database, mock_fetch_data, mock_fetch_country_admin_lvl, mock_parse_req, @@ -276,3 +288,8 @@ def test_reverse_geolocation_populate( ] _, response_code = reverse_geolocation_populate(MagicMock()) self.assertEqual(200, response_code) + mock_save_to_database.assert_called_once() + mock_update_geopolygon_hierarchy.assert_called_once_with( + mock_fetch_data.return_value + ) + mock_refresh_location_search_view.assert_called_once() diff --git a/liquibase/changelog.xml b/liquibase/changelog.xml index 13822937d..f21a3454b 100644 --- a/liquibase/changelog.xml +++ b/liquibase/changelog.xml @@ -119,7 +119,9 @@ - + + + diff --git a/liquibase/changes/feat_pt_202.sql b/liquibase/changes/feat_pt_202.sql new file mode 100644 index 000000000..8e2a59a0e --- /dev/null +++ b/liquibase/changes/feat_pt_202.sql @@ -0,0 +1,15 @@ +CREATE TABLE IF NOT EXISTS GeopolygonHierarchy ( + id BIGINT GENERATED BY DEFAULT AS IDENTITY PRIMARY KEY, + osm_id INTEGER NOT NULL UNIQUE REFERENCES Geopolygon(osm_id) ON DELETE CASCADE, + parent_osm_id INTEGER REFERENCES Geopolygon(osm_id) ON DELETE SET NULL, + country_osm_id INTEGER REFERENCES Geopolygon(osm_id) ON DELETE SET NULL, + subdivision_osm_id INTEGER REFERENCES Geopolygon(osm_id) ON DELETE SET NULL, + updated_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, + CHECK (osm_id <> parent_osm_id), + CHECK (osm_id <> country_osm_id), + CHECK (osm_id <> subdivision_osm_id) +); + +CREATE INDEX IF NOT EXISTS idx_geopolygonhierarchy_parent_osm_id ON GeopolygonHierarchy(parent_osm_id); +CREATE INDEX IF NOT EXISTS idx_geopolygonhierarchy_country_osm_id ON GeopolygonHierarchy(country_osm_id); +CREATE INDEX IF NOT EXISTS idx_geopolygonhierarchy_subdivision_osm_id ON GeopolygonHierarchy(subdivision_osm_id); diff --git a/liquibase/materialized_views/geopolygon_location_search.sql b/liquibase/materialized_views/geopolygon_location_search.sql new file mode 100644 index 000000000..5aa6e1eca --- /dev/null +++ b/liquibase/materialized_views/geopolygon_location_search.sql @@ -0,0 +1,71 @@ +CREATE MATERIALIZED VIEW GeopolygonLocationSearch AS +WITH RECURSIVE hierarchy AS ( + SELECT + gp.osm_id, + gp.osm_id AS ancestor_osm_id, + 0 AS depth, + ARRAY[gp.osm_id] AS visited_osm_ids + FROM Geopolygon gp + + UNION ALL + + SELECT + hierarchy.osm_id, + GeopolygonHierarchy.parent_osm_id AS ancestor_osm_id, + hierarchy.depth + 1 AS depth, + hierarchy.visited_osm_ids || GeopolygonHierarchy.parent_osm_id AS visited_osm_ids + FROM hierarchy + JOIN GeopolygonHierarchy ON GeopolygonHierarchy.osm_id = hierarchy.ancestor_osm_id + WHERE GeopolygonHierarchy.parent_osm_id IS NOT NULL + AND NOT GeopolygonHierarchy.parent_osm_id = ANY(hierarchy.visited_osm_ids) +), +location_paths AS ( + SELECT + hierarchy.osm_id, + array_agg(ancestor.name ORDER BY hierarchy.depth DESC) AS path_names, + string_agg(ancestor.name, ', ' ORDER BY hierarchy.depth DESC) AS display_name, + (array_agg(ancestor.osm_id ORDER BY hierarchy.depth DESC) + FILTER (WHERE ancestor.iso_3166_1_code IS NOT NULL))[1] AS country_osm_id, + (array_agg(ancestor.osm_id ORDER BY hierarchy.depth DESC) + FILTER (WHERE ancestor.iso_3166_2_code IS NOT NULL))[1] AS subdivision_osm_id + FROM hierarchy + JOIN Geopolygon ancestor ON ancestor.osm_id = hierarchy.ancestor_osm_id + GROUP BY hierarchy.osm_id +) +SELECT + gp.osm_id, + GeopolygonHierarchy.parent_osm_id, + gp.name, + gp.alt_name, + CASE + WHEN gp.iso_3166_1_code IS NOT NULL THEN 'country' + WHEN gp.iso_3166_1_code IS NULL AND gp.iso_3166_2_code IS NOT NULL THEN 'subdivision' + ELSE 'municipality' + END AS location_type, + gp.admin_level, + country.name AS country_name, + COALESCE(gp.iso_3166_1_code, country.iso_3166_1_code) AS country_code, + subdivision.name AS subdivision_name, + subdivision.iso_3166_2_code AS subdivision_code, + location_paths.path_names, + location_paths.display_name, + setweight(to_tsvector('english', coalesce(unaccent(location_paths.display_name), '')), 'A') || + setweight(to_tsvector('english', coalesce(unaccent(gp.name), '')), 'A') || + setweight(to_tsvector('english', coalesce(unaccent(gp.alt_name), '')), 'B') || + setweight(to_tsvector('english', coalesce(unaccent(gp.iso_3166_1_code), '')), 'B') || + setweight(to_tsvector('english', coalesce(unaccent(gp.iso_3166_2_code), '')), 'B') + AS document +FROM Geopolygon gp +LEFT JOIN GeopolygonHierarchy ON GeopolygonHierarchy.osm_id = gp.osm_id +LEFT JOIN location_paths ON location_paths.osm_id = gp.osm_id +LEFT JOIN Geopolygon country ON country.osm_id = COALESCE(GeopolygonHierarchy.country_osm_id, location_paths.country_osm_id) +LEFT JOIN Geopolygon subdivision ON subdivision.osm_id = COALESCE( + GeopolygonHierarchy.subdivision_osm_id, + location_paths.subdivision_osm_id +); + +CREATE UNIQUE INDEX idx_unique_geopolygon_location_search_osm_id ON GeopolygonLocationSearch(osm_id); +CREATE INDEX geopolygon_location_search_document_idx ON GeopolygonLocationSearch USING GIN(document); +CREATE INDEX geopolygon_location_search_country_code_idx ON GeopolygonLocationSearch(country_code); +CREATE INDEX geopolygon_location_search_location_type_idx ON GeopolygonLocationSearch(location_type); +CREATE INDEX geopolygon_location_search_parent_osm_id_idx ON GeopolygonLocationSearch(parent_osm_id); diff --git a/liquibase/materialized_views/materialized_views.xml b/liquibase/materialized_views/materialized_views.xml index 67abd537e..10de2454a 100644 --- a/liquibase/materialized_views/materialized_views.xml +++ b/liquibase/materialized_views/materialized_views.xml @@ -15,4 +15,14 @@ endDelimiter=";"/> + + DROP MATERIALIZED VIEW IF EXISTS geopolygonlocationsearch; + + + + From 556337132bb53a1f9d41c422df60485755a8ae61 Mon Sep 17 00:00:00 2001 From: cka-y Date: Mon, 20 Jul 2026 11:59:27 -0400 Subject: [PATCH 2/7] fix: materialized view checks --- api/src/feeds/impl/locations_api_impl.py | 15 ++++++++++++++- .../geopolygon_location_search.sql | 18 +++++++++++++++++- 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/api/src/feeds/impl/locations_api_impl.py b/api/src/feeds/impl/locations_api_impl.py index 2f698a851..910073222 100644 --- a/api/src/feeds/impl/locations_api_impl.py +++ b/api/src/feeds/impl/locations_api_impl.py @@ -1,3 +1,5 @@ +import re + from fastapi import APIRouter, Query, Security from pydantic import BaseModel, Field from sqlalchemy import and_, func, select @@ -33,6 +35,8 @@ class LocationsSearchResponse(BaseModel): 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() @@ -55,8 +59,17 @@ def _build_locations_conditions(search_query: str | None, country_code: str | No return conditions, normalized_query +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.plainto_tsquery("english", func.unaccent(search_query)) + return func.to_tsquery("english", func.unaccent(_to_prefix_tsquery(search_query))) def _location_from_row(row) -> LocationSearchResult: diff --git a/liquibase/materialized_views/geopolygon_location_search.sql b/liquibase/materialized_views/geopolygon_location_search.sql index 5aa6e1eca..bbdbcff2e 100644 --- a/liquibase/materialized_views/geopolygon_location_search.sql +++ b/liquibase/materialized_views/geopolygon_location_search.sql @@ -62,7 +62,23 @@ LEFT JOIN Geopolygon country ON country.osm_id = COALESCE(GeopolygonHierarchy.co LEFT JOIN Geopolygon subdivision ON subdivision.osm_id = COALESCE( GeopolygonHierarchy.subdivision_osm_id, location_paths.subdivision_osm_id -); +) +WHERE + -- Countries are always kept. + gp.iso_3166_1_code IS NOT NULL + -- Subdivisions must resolve to a country. + OR ( + gp.iso_3166_1_code IS NULL + AND gp.iso_3166_2_code IS NOT NULL + AND country.osm_id IS NOT NULL + ) + -- Municipalities must resolve to both a country and a subdivision. + OR ( + gp.iso_3166_1_code IS NULL + AND gp.iso_3166_2_code IS NULL + AND country.osm_id IS NOT NULL + AND subdivision.osm_id IS NOT NULL + ); CREATE UNIQUE INDEX idx_unique_geopolygon_location_search_osm_id ON GeopolygonLocationSearch(osm_id); CREATE INDEX geopolygon_location_search_document_idx ON GeopolygonLocationSearch USING GIN(document); From 6312296d5284659d3a7e6adbd880c18773f64fc1 Mon Sep 17 00:00:00 2001 From: cka-y Date: Mon, 20 Jul 2026 14:52:08 -0400 Subject: [PATCH 3/7] fix: materialized view on subdivision that are countries + rank + tests --- api/.openapi-generator/FILES | 1 - api/src/feeds/impl/locations_api_impl.py | 186 ++++++++---------- api/src/main.py | 2 +- .../shared/database/sql_functions/unaccent.py | 2 +- api/tests/unittest/test_locations_api_impl.py | 154 +++++++++++++++ docs/DatabaseCatalogAPI.yaml | 56 ++++-- .../test_reverse_geolocation_populate.py | 83 ++++++++ .../geopolygon_location_search.sql | 3 +- 8 files changed, 358 insertions(+), 129 deletions(-) create mode 100644 api/tests/unittest/test_locations_api_impl.py diff --git a/api/.openapi-generator/FILES b/api/.openapi-generator/FILES index c36805165..dfa325a83 100644 --- a/api/.openapi-generator/FILES +++ b/api/.openapi-generator/FILES @@ -37,7 +37,6 @@ src/feeds_gen/models/license_base.py src/feeds_gen/models/license_rule.py src/feeds_gen/models/license_with_rules.py src/feeds_gen/models/location.py -src/feeds_gen/models/location_hierarchy_item.py src/feeds_gen/models/location_search_response.py src/feeds_gen/models/location_search_result.py src/feeds_gen/models/matching_license.py diff --git a/api/src/feeds/impl/locations_api_impl.py b/api/src/feeds/impl/locations_api_impl.py index 910073222..402834b0e 100644 --- a/api/src/feeds/impl/locations_api_impl.py +++ b/api/src/feeds/impl/locations_api_impl.py @@ -1,35 +1,27 @@ import re -from fastapi import APIRouter, Query, Security -from pydantic import BaseModel, Field from sqlalchemy import and_, func, select from sqlalchemy.orm import Session -from feeds_gen.models.extra_models import TokenModel -from feeds_gen.security_api import get_token_Authentication +from feeds_gen.apis.locations_api_base import BaseLocationsApi +from feeds_gen.models.location_search_response import LocationSearchResponse +from feeds_gen.models.location_search_result import LocationSearchResult 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 -router = APIRouter() - -class LocationSearchResult(BaseModel): - location_id: int - parent_location_id: int | None = None - name: str | None = None - alt_name: str | None = None - location_type: str - country_name: str | None = None - country_code: str | None = None - subdivision_name: str | None = None - subdivision_code: str | None = None - path_names: list[str] = Field(default_factory=list) - display_name: str | None = None +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) -class LocationsSearchResponse(BaseModel): - results: list[LocationSearchResult] - total: int +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: @@ -40,38 +32,33 @@ def _normalize_search_query(search_query: str | None) -> str | None: return search_query.strip() -def _normalize_country_code(country_code: str | None) -> str | None: - if country_code is None or len(country_code.strip()) == 0: +def _normalize_location_code(location_code: str | None) -> str | None: + if location_code is None or len(location_code.strip()) == 0: return None - return country_code.strip().upper() + return location_code.strip().upper() -def _build_locations_conditions(search_query: str | None, country_code: str | None, location_type: str | None): +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_country_code(country_code) + 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 -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", func.unaccent(_to_prefix_tsquery(search_query))) - - def _location_from_row(row) -> LocationSearchResult: return LocationSearchResult( location_id=row["osm_id"], @@ -88,71 +75,60 @@ def _location_from_row(row) -> LocationSearchResult: ) -@with_db_session -def search_locations( - limit: int, - offset: int, - search_query: str | None, - country_code: str | None, - location_type: str | None, - db_session: Session, -) -> LocationsSearchResponse: - conditions, normalized_query = _build_locations_conditions(search_query, country_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( - func.ts_rank(location_search.c.document, _ts_query(normalized_query)).desc(), - location_search.c.display_name.asc(), +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 ) - else: - locations_query = locations_query.order_by(location_search.c.display_name.asc()) - locations_query = locations_query.limit(limit).offset(offset) - - rows = db_session.execute(locations_query).mappings().all() - return LocationsSearchResponse( - results=[_location_from_row(row) for row in rows], - total=total, - ) - -@router.get( - "/v1/locations", - tags=["locations"], - response_model=LocationsSearchResponse, - response_model_by_alias=True, -) -def get_locations( - limit: int = Query(100, description="The number of locations to return.", alias="limit", ge=0, le=3500), - offset: int = Query(0, description="Offset of the first location to return.", alias="offset", ge=0), - search_query: str = Query(None, description="Location search query.", alias="search_query"), - country_code: str = Query(None, description="ISO 3166-1 alpha-2 country code.", alias="country_code"), - location_type: str = Query( - None, - description="Location type to return: country, subdivision, or municipality.", - alias="location_type", - pattern="^(country|subdivision|municipality)$", - ), - token_Authentication: TokenModel = Security(get_token_Authentication), -) -> LocationsSearchResponse: - """Search locations from the geopolygon materialized read model.""" - return search_locations(limit, offset, search_query, country_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).mappings().all() + return LocationSearchResponse( + total=total, + results=[_location_from_row(row) for row in rows], + ) diff --git a/api/src/main.py b/api/src/main.py index 9ff7067ab..5a3feee06 100644 --- a/api/src/main.py +++ b/api/src/main.py @@ -25,7 +25,7 @@ from feeds_gen.apis.metadata_api import router as MetadataApiRouter from feeds_gen.apis.search_api import router as SearchApiRouter from feeds_gen.apis.licenses_api import router as LicensesApiRouter -from feeds.impl.locations_api_impl import router as LocationsApiRouter +from feeds_gen.apis.locations_api import router as LocationsApiRouter from user_service_gen.apis.users_api import router as UsersApiRouter from user_service_gen.apis.notifications_api import router as NotificationsApiRouter from user_service_gen.apis.subscriptions_api import router as SubscriptionsApiRouter diff --git a/api/src/shared/database/sql_functions/unaccent.py b/api/src/shared/database/sql_functions/unaccent.py index 6eb67ea45..39d58fd49 100644 --- a/api/src/shared/database/sql_functions/unaccent.py +++ b/api/src/shared/database/sql_functions/unaccent.py @@ -9,4 +9,4 @@ class unaccent(ReturnTypeFromArgs): Be aware that this function is not available in all databases nor in all versions of PostgreSQL. """ - pass + inherit_cache = True diff --git a/api/tests/unittest/test_locations_api_impl.py b/api/tests/unittest/test_locations_api_impl.py new file mode 100644 index 000000000..0f4333eaa --- /dev/null +++ b/api/tests/unittest/test_locations_api_impl.py @@ -0,0 +1,154 @@ +from unittest.mock import MagicMock + +from feeds.impl.locations_api_impl import ( + LocationsApiImpl, + _build_locations_conditions, + _location_from_row, + _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 + + +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 = { + "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 = _location_from_row(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 = { + "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 = _location_from_row(row) + assert result.path_names == [] + + +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.mappings.return_value.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 = [ + { + "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 == [] diff --git a/docs/DatabaseCatalogAPI.yaml b/docs/DatabaseCatalogAPI.yaml index d6cc1b308..723a0789e 100644 --- a/docs/DatabaseCatalogAPI.yaml +++ b/docs/DatabaseCatalogAPI.yaml @@ -399,21 +399,25 @@ paths: tags: - "locations" parameters: - - name: limit - in: query - description: The number of locations to return. - schema: - type: integer - minimum: 0 - maximum: 3500 - default: 100 + - $ref: "#/components/parameters/limit_query_param_locations_endpoint" - $ref: "#/components/parameters/offset" - name: search_query in: query description: Location search query. schema: type: string - - $ref: "#/components/parameters/country_code" + - name: country_code + in: query + description: Limit results to locations within this ISO 3166-1 alpha-2 country code. + schema: + type: string + example: CA + - name: subdivision_code + in: query + description: Limit results to locations within this ISO 3166-2 subdivision code. + schema: + type: string + example: CA-QC - name: location_type in: query description: Filter by location type. @@ -1302,11 +1306,11 @@ components: name: type: string nullable: true - example: Boston + example: Montréal alt_name: type: string nullable: true - example: City of Boston + example: City of Montréal location_type: type: string enum: @@ -1317,32 +1321,32 @@ components: country_name: type: string nullable: true - example: United States + example: Canada country_code: type: string nullable: true - example: US + example: CA subdivision_name: type: string nullable: true - example: Massachusetts + example: Quebec subdivision_code: type: string nullable: true - example: US-MA + example: CA-QC path_names: type: array items: type: string example: - - United States - - Massachusetts - - Suffolk County - - Boston + - Canada + - Quebec + - Montréal (region) + - Montréal display_name: type: string nullable: true - example: United States, Massachusetts, Suffolk County, Boston + example: Canada, Quebec, Montréal (region), Montréal # Have to put the enum inline because of a bug in openapi-generator # FeedStatus: @@ -1872,6 +1876,18 @@ components: type: boolean default: null + limit_query_param_locations_endpoint: + name: limit + in: query + description: The number of items to be returned. + required: False + schema: + type: integer + minimum: 0 + maximum: 100 + default: 100 + example: 10 + limit_query_param_feeds_endpoint: name: limit in: query diff --git a/functions-python/reverse_geolocation_populate/tests/test_reverse_geolocation_populate.py b/functions-python/reverse_geolocation_populate/tests/test_reverse_geolocation_populate.py index 0a2e04c5f..eacf0469a 100644 --- a/functions-python/reverse_geolocation_populate/tests/test_reverse_geolocation_populate.py +++ b/functions-python/reverse_geolocation_populate/tests/test_reverse_geolocation_populate.py @@ -241,6 +241,89 @@ def test_update_geopolygon_hierarchy_skips_empty_rows(self): db_session.execute.assert_not_called() db_session.commit.assert_not_called() + @patch("main._upsert_locality_hierarchy") + @patch("main._upsert_hierarchy_rows") + def test_update_geopolygon_hierarchy_classifies_rows( + self, mock_upsert_rows, mock_upsert_locality + ): + from main import update_geopolygon_hierarchy + + saved_rows = [ + {"osm_id": 3, "iso3166_1": "CA", "iso3166_2": None}, # country + {"osm_id": 2, "iso3166_1": None, "iso3166_2": "CA-QC"}, # subdivision + {"osm_id": 1, "iso3166_1": None, "iso3166_2": None}, # locality + ] + db_session = MagicMock() + update_geopolygon_hierarchy(saved_rows, db_session=db_session) + + # Direct country + subdivision edges are upserted without geometry work. + direct_rows = mock_upsert_rows.call_args[0][1] + by_osm_id = {row["osm_id"]: row for row in direct_rows} + # Country: no parent, no country/subdivision references. + self.assertEqual( + by_osm_id[3], + { + "osm_id": 3, + "parent_osm_id": None, + "country_osm_id": None, + "subdivision_osm_id": None, + }, + ) + # Subdivision: parent and country point at the country in the run. + self.assertEqual( + by_osm_id[2], + { + "osm_id": 2, + "parent_osm_id": 3, + "country_osm_id": 3, + "subdivision_osm_id": None, + }, + ) + + # Localities are matched by geometry against this run's subdivisions. + mock_upsert_locality.assert_called_once_with(db_session, [1], [2], 3) + db_session.commit.assert_called_once() + + @patch("main._upsert_locality_hierarchy") + @patch("main._upsert_hierarchy_rows") + def test_update_geopolygon_hierarchy_country_with_both_codes_is_country( + self, mock_upsert_rows, mock_upsert_locality + ): + from main import update_geopolygon_hierarchy + + # A country that also carries an ISO 3166-2 code must stay a country, + # never be treated as a subdivision. + saved_rows = [{"osm_id": 100, "iso3166_1": "TL", "iso3166_2": "XX-TL"}] + db_session = MagicMock() + update_geopolygon_hierarchy(saved_rows, db_session=db_session) + + direct_rows = mock_upsert_rows.call_args[0][1] + self.assertEqual( + direct_rows, + [ + { + "osm_id": 100, + "parent_osm_id": None, + "country_osm_id": None, + "subdivision_osm_id": None, + } + ], + ) + # No localities -> geometry matching is skipped. + mock_upsert_locality.assert_not_called() + + def test_refresh_location_search_view_executes_refresh(self): + from main import refresh_location_search_view + + db_session = MagicMock() + refresh_location_search_view(db_session=db_session) + + db_session.execute.assert_called_once() + executed = str(db_session.execute.call_args[0][0]) + self.assertIn("REFRESH MATERIALIZED VIEW", executed.upper()) + self.assertIn("GEOPOLYGONLOCATIONSEARCH", executed.upper()) + db_session.commit.assert_called_once() + @patch("main.bigquery") @patch("main.parse_request_parameters") @patch("main.fetch_country_admin_levels") diff --git a/liquibase/materialized_views/geopolygon_location_search.sql b/liquibase/materialized_views/geopolygon_location_search.sql index bbdbcff2e..7efc05f0b 100644 --- a/liquibase/materialized_views/geopolygon_location_search.sql +++ b/liquibase/materialized_views/geopolygon_location_search.sql @@ -27,7 +27,8 @@ location_paths AS ( (array_agg(ancestor.osm_id ORDER BY hierarchy.depth DESC) FILTER (WHERE ancestor.iso_3166_1_code IS NOT NULL))[1] AS country_osm_id, (array_agg(ancestor.osm_id ORDER BY hierarchy.depth DESC) - FILTER (WHERE ancestor.iso_3166_2_code IS NOT NULL))[1] AS subdivision_osm_id + FILTER (WHERE ancestor.iso_3166_2_code IS NOT NULL + AND ancestor.iso_3166_1_code IS NULL))[1] AS subdivision_osm_id FROM hierarchy JOIN Geopolygon ancestor ON ancestor.osm_id = hierarchy.ancestor_osm_id GROUP BY hierarchy.osm_id From 079e68166ea9f579296b36fb52dc871f671d22ee Mon Sep 17 00:00:00 2001 From: cka-y Date: Mon, 20 Jul 2026 14:58:23 -0400 Subject: [PATCH 4/7] fix: tests --- api/tests/test_utils/db_utils.py | 1 + functions-python/test_utils/database_utils.py | 1 + 2 files changed, 2 insertions(+) diff --git a/api/tests/test_utils/db_utils.py b/api/tests/test_utils/db_utils.py index 736782b9c..c05d0a7e1 100644 --- a/api/tests/test_utils/db_utils.py +++ b/api/tests/test_utils/db_utils.py @@ -186,6 +186,7 @@ def is_test_db(url): "spatial_ref_sys", # Excluding the views "feedsearch", + "geopolygonlocationsearch", "location_with_translations_en", ] diff --git a/functions-python/test_utils/database_utils.py b/functions-python/test_utils/database_utils.py index dbe6f1242..124bc135e 100644 --- a/functions-python/test_utils/database_utils.py +++ b/functions-python/test_utils/database_utils.py @@ -43,6 +43,7 @@ "spatial_ref_sys", # Excluding the views "feedsearch", + "geopolygonlocationsearch", ] From 4a6a6353de2159859fe8cb4ea4bb5cac546ee5e8 Mon Sep 17 00:00:00 2001 From: cka-y Date: Mon, 20 Jul 2026 15:03:12 -0400 Subject: [PATCH 5/7] fix: restoring userservice files --- api/.openapi-generator/FILES | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/api/.openapi-generator/FILES b/api/.openapi-generator/FILES index dfa325a83..64662a984 100644 --- a/api/.openapi-generator/FILES +++ b/api/.openapi-generator/FILES @@ -47,3 +47,22 @@ src/feeds_gen/models/search_feeds200_response.py src/feeds_gen/models/source_info.py src/feeds_gen/models/validation_report.py src/feeds_gen/security_api.py +src/user_service/impl/__init__.py +src/user_service_gen/apis/__init__.py +src/user_service_gen/apis/notifications_api.py +src/user_service_gen/apis/notifications_api_base.py +src/user_service_gen/apis/subscriptions_api.py +src/user_service_gen/apis/subscriptions_api_base.py +src/user_service_gen/apis/users_api.py +src/user_service_gen/apis/users_api_base.py +src/user_service_gen/main.py +src/user_service_gen/models/__init__.py +src/user_service_gen/models/create_notification_subscription_request.py +src/user_service_gen/models/extra_models.py +src/user_service_gen/models/feature_flag.py +src/user_service_gen/models/notification_subscription.py +src/user_service_gen/models/notification_type.py +src/user_service_gen/models/update_notification_subscription_request.py +src/user_service_gen/models/update_user_request.py +src/user_service_gen/models/user_profile.py +src/user_service_gen/security_api.py From 7d337479594266c7ce0d8efe06c60ac1e8ac4911 Mon Sep 17 00:00:00 2001 From: cka-y Date: Mon, 20 Jul 2026 16:06:57 -0400 Subject: [PATCH 6/7] improved documentation and fixed coding inconsistency --- api/src/feeds/impl/locations_api_impl.py | 22 +---- api/tests/unittest/test_locations_api_impl.py | 91 ++++++++++--------- docs/DatabaseCatalogAPI.yaml | 40 +++++++- 3 files changed, 86 insertions(+), 67 deletions(-) diff --git a/api/src/feeds/impl/locations_api_impl.py b/api/src/feeds/impl/locations_api_impl.py index 402834b0e..055db00fc 100644 --- a/api/src/feeds/impl/locations_api_impl.py +++ b/api/src/feeds/impl/locations_api_impl.py @@ -5,10 +5,10 @@ from feeds_gen.apis.locations_api_base import BaseLocationsApi from feeds_gen.models.location_search_response import LocationSearchResponse -from feeds_gen.models.location_search_result import LocationSearchResult 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: @@ -59,22 +59,6 @@ def _build_locations_conditions( return conditions, normalized_query -def _location_from_row(row) -> LocationSearchResult: - return LocationSearchResult( - location_id=row["osm_id"], - parent_location_id=row["parent_osm_id"], - name=row["name"], - alt_name=row["alt_name"], - location_type=row["location_type"], - country_name=row["country_name"], - country_code=row["country_code"], - subdivision_name=row["subdivision_name"], - subdivision_code=row["subdivision_code"], - path_names=row["path_names"] or [], - display_name=row["display_name"], - ) - - class LocationsApiImpl(BaseLocationsApi): """This class represents the implementation of the `/locations` endpoints.""" @@ -127,8 +111,8 @@ def get_locations( ) locations_query = locations_query.limit(limit).offset(offset) - rows = db_session.execute(locations_query).mappings().all() + rows = db_session.execute(locations_query).all() return LocationSearchResponse( total=total, - results=[_location_from_row(row) for row in rows], + results=[LocationSearchResultImpl.from_orm(row) for row in rows], ) diff --git a/api/tests/unittest/test_locations_api_impl.py b/api/tests/unittest/test_locations_api_impl.py index 0f4333eaa..2875c0083 100644 --- a/api/tests/unittest/test_locations_api_impl.py +++ b/api/tests/unittest/test_locations_api_impl.py @@ -1,15 +1,16 @@ +from types import SimpleNamespace from unittest.mock import MagicMock from feeds.impl.locations_api_impl import ( LocationsApiImpl, _build_locations_conditions, - _location_from_row, _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(): @@ -65,20 +66,20 @@ def test_build_conditions_ignores_blank_search_query(): def test_location_from_row_maps_fields(): - row = { - "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 = _location_from_row(row) + 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 @@ -88,30 +89,34 @@ def test_location_from_row_maps_fields(): def test_location_from_row_defaults_missing_path_names_to_empty_list(): - row = { - "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 = _location_from_row(row) + 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.mappings.return_value.all.return_value = rows + rows_result.all.return_value = rows session = MagicMock() session.execute.side_effect = [count_result, rows_result] @@ -120,19 +125,19 @@ def _mock_session(total, rows): def test_get_locations_returns_total_and_results(): rows = [ - { - "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", - } + 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) diff --git a/docs/DatabaseCatalogAPI.yaml b/docs/DatabaseCatalogAPI.yaml index 723a0789e..360149342 100644 --- a/docs/DatabaseCatalogAPI.yaml +++ b/docs/DatabaseCatalogAPI.yaml @@ -394,7 +394,11 @@ paths: /v1/locations: get: - description: Search locations in the Mobility Database. + description: > + Search locations (countries, subdivisions and municipalities). + Results can be filtered by a free-text query and narrowed to a specific country, + subdivision or location type. Matches are ordered from the broadest area to the + most specific, and by relevance within each level. operationId: getLocations tags: - "locations" @@ -403,30 +407,44 @@ paths: - $ref: "#/components/parameters/offset" - name: search_query in: query - description: Location search query. + description: > + Free-text search matched against the location name, alternate name and its + full hierarchy (e.g. "Canada, Quebec, Montréal"). Matching is accent-insensitive + and supports typeahead-style prefix matching, so "mon" matches "Montréal". + When several words are provided, all of them must match. schema: type: string + example: montreal - name: country_code in: query - description: Limit results to locations within this ISO 3166-1 alpha-2 country code. + description: > + Limit results to locations contained within this country, given as its + ISO 3166-1 alpha-2 code. Case-insensitive. schema: type: string example: CA - name: subdivision_code in: query - description: Limit results to locations within this ISO 3166-2 subdivision code. + description: > + Limit results to locations contained within this subdivision, given as its + ISO 3166-2 code. Case-insensitive. schema: type: string example: CA-QC - name: location_type in: query - description: Filter by location type. + description: > + Filter by the type of location: + * `country` - a sovereign country, identified by an ISO 3166-1 code. + * `subdivision` - a first-level subdivision (e.g. state or province), identified by an ISO 3166-2 code. + * `municipality` - a locality below the subdivision level (e.g. a city or town). schema: type: string enum: - country - subdivision - municipality + example: municipality security: - Authentication: [] responses: @@ -1288,6 +1306,7 @@ components: description: The total number of matching locations regardless of limit and offset. results: type: array + description: The page of matching locations, ordered from the broadest area to the most specific and by relevance within each level. items: $ref: "#/components/schemas/LocationSearchResult" @@ -1306,13 +1325,18 @@ components: name: type: string nullable: true + description: The primary name of the location, in English when available. example: Montréal alt_name: type: string nullable: true + description: An alternate or local name for the location, when available. example: City of Montréal location_type: type: string + description: > + The type of location: `country` (has an ISO 3166-1 code), `subdivision` + (has an ISO 3166-2 code) or `municipality` (a locality below the subdivision level). enum: - country - subdivision @@ -1321,21 +1345,26 @@ components: country_name: type: string nullable: true + description: The name of the country that contains this location. example: Canada country_code: type: string nullable: true + description: The ISO 3166-1 alpha-2 code of the country that contains this location. example: CA subdivision_name: type: string nullable: true + description: The name of the subdivision (e.g. state or province) that contains this location, when applicable. example: Quebec subdivision_code: type: string nullable: true + description: The ISO 3166-2 code of the subdivision that contains this location, when applicable. example: CA-QC path_names: type: array + description: The ordered list of location names from the broadest containing area down to this location. items: type: string example: @@ -1346,6 +1375,7 @@ components: display_name: type: string nullable: true + description: A human-readable representation of the full location hierarchy, joined from the broadest area to this location. example: Canada, Quebec, Montréal (region), Montréal # Have to put the enum inline because of a bug in openapi-generator From e4741ed12458f4544e739c30d970a3b67c888a8b Mon Sep 17 00:00:00 2001 From: cka-y Date: Mon, 20 Jul 2026 16:07:06 -0400 Subject: [PATCH 7/7] improved documentation and fixed coding inconsistency --- .../db_models/location_search_result_impl.py | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) create mode 100644 api/src/shared/db_models/location_search_result_impl.py diff --git a/api/src/shared/db_models/location_search_result_impl.py b/api/src/shared/db_models/location_search_result_impl.py new file mode 100644 index 000000000..a00e19947 --- /dev/null +++ b/api/src/shared/db_models/location_search_result_impl.py @@ -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, + )