From 30fdb192f83ef42ddf78f00b2afe3291ff2120d9 Mon Sep 17 00:00:00 2001 From: Alice Date: Wed, 29 Oct 2025 12:01:10 -0700 Subject: [PATCH 1/2] refine endpoint --- com/github/ott/pelias/adapter/core/config.py | 4 +- .../adapter/models/solr/solr_response.py | 8 +- .../ott/pelias/adapter/routers/pelias.py | 11 +- com/github/ott/pelias/adapter/routers/solr.py | 2 +- .../ott/pelias/adapter/schema/solr_schema.py | 4 +- .../ott/pelias/adapter/service/config.py | 10 +- .../pelias/adapter/service/pelias_service.py | 191 +++++++++++++++++- .../pelias/adapter/service/pelias_to_solr.py | 6 +- .../pelias/adapter/service/pelias_wrapper.py | 10 +- com/github/ott/pelias/adapter/service/util.py | 97 +++++++++ dev.env | 2 - docker/buildDocker.sh | 3 +- docker/compose.yml | 2 +- main.py | 10 +- pyproject.toml | 4 + tests/README.md | 2 +- tests/com/__init__.py | 0 tests/com/github/__init__.py | 0 tests/com/github/ott/__init__.py | 0 tests/com/github/ott/pelias/__init__.py | 0 .../com/github/ott/pelias/adapter/__init__.py | 0 .../adapter/service/test_pelias_service.py | 37 ++++ tests/comparison_testing/util.py | 4 +- 23 files changed, 371 insertions(+), 36 deletions(-) create mode 100644 com/github/ott/pelias/adapter/service/util.py create mode 100644 tests/com/__init__.py create mode 100644 tests/com/github/__init__.py create mode 100644 tests/com/github/ott/__init__.py create mode 100644 tests/com/github/ott/pelias/__init__.py create mode 100644 tests/com/github/ott/pelias/adapter/__init__.py create mode 100644 tests/com/github/ott/pelias/adapter/service/test_pelias_service.py diff --git a/com/github/ott/pelias/adapter/core/config.py b/com/github/ott/pelias/adapter/core/config.py index 2143671..61ab831 100644 --- a/com/github/ott/pelias/adapter/core/config.py +++ b/com/github/ott/pelias/adapter/core/config.py @@ -12,7 +12,7 @@ CACHE_LONG_STR = f"public, max-age={CACHE_LONG}" -logger.info(f"Environment: {ENVIRONMENT}") +logger.debug(f"Environment: {ENVIRONMENT}") ENV_FILE = f"{ENVIRONMENT}.env" @@ -40,4 +40,4 @@ class Settings(BaseSettings): # Global settings instance settings = Settings() -logger.info(f"Loaded settings for environment: {ENVIRONMENT}, {settings.debug}") +logger.debug(f"Loaded settings for environment: {ENVIRONMENT}, {settings.debug}") diff --git a/com/github/ott/pelias/adapter/models/solr/solr_response.py b/com/github/ott/pelias/adapter/models/solr/solr_response.py index 8eb6832..adf5f1d 100644 --- a/com/github/ott/pelias/adapter/models/solr/solr_response.py +++ b/com/github/ott/pelias/adapter/models/solr/solr_response.py @@ -23,14 +23,14 @@ def url(cls): return cls._url @classmethod - def find_record(cls, id): + def find_record(cls, _id): """see requests_cache: https://requests-cache.readthedocs.io/en/latest/user_guide.html""" ret_val = None try: - ret_val = cls.cache.get(id) + ret_val = cls.cache.get(_id) if ret_val is None: # step 1: query route stop service - rs = requests.get("{}/{}/routes/str".format(cls.url(), id)) + rs = requests.get("{}/{}/routes/str".format(cls.url(), _id)) # step 2: cache record if rs and len(rs.text): @@ -64,7 +64,7 @@ def parse_pelias(self, json, add_routes=False): # step 1: get pelias records features = json.get("features", []) - # step 2: loop thru pelias records + # step 2: loop through pelias records for f in features: # step 3: handle parsing of different layer types layer = f.get("properties", {}).get("layer") diff --git a/com/github/ott/pelias/adapter/routers/pelias.py b/com/github/ott/pelias/adapter/routers/pelias.py index 160439c..bf2363c 100644 --- a/com/github/ott/pelias/adapter/routers/pelias.py +++ b/com/github/ott/pelias/adapter/routers/pelias.py @@ -16,9 +16,10 @@ def get_json_response( request: Request, service: SearchApiType = SearchApiType.autocomplete, is_rtp: bool = False, + refine: bool = False, ) -> JSONResponse: ret_val = pelias_service.get_pelias_response( - service=service, request=request, is_rtp=is_rtp + request, service, is_rtp, refine=refine ) return JSONResponse(content=ret_val, headers={"Cache-Control": CACHE_LONG_STR}) @@ -32,6 +33,14 @@ def pelias( return get_json_response(request=request, service=api) +@router.get("/refine/{api}") +def refine( + request: Request, + api: SearchApiType = SearchApiType.autocomplete, +) -> JSONResponse: + return get_json_response(request=request, service=api, refine=True) + + @router.get("/rtp/{api}") def pelias_from_rtp( request: Request, diff --git a/com/github/ott/pelias/adapter/routers/solr.py b/com/github/ott/pelias/adapter/routers/solr.py index c608834..6a4516e 100644 --- a/com/github/ott/pelias/adapter/routers/solr.py +++ b/com/github/ott/pelias/adapter/routers/solr.py @@ -1,6 +1,6 @@ import logging -from fastapi import APIRouter, Request, Depends +from fastapi import APIRouter, Request from com.github.ott.pelias.adapter.models.solr.solr_response import SolrResponse from com.github.ott.pelias.adapter.schema.solr_schema import SolrResponseSchema diff --git a/com/github/ott/pelias/adapter/schema/solr_schema.py b/com/github/ott/pelias/adapter/schema/solr_schema.py index e7c65d6..43ece0e 100644 --- a/com/github/ott/pelias/adapter/schema/solr_schema.py +++ b/com/github/ott/pelias/adapter/schema/solr_schema.py @@ -57,7 +57,7 @@ class SolrDoc(BaseModel): model_config = {"extra": "allow", "exclude_none": False, "populate_by_name": True} @staticmethod - def toSchema(record: SolrStopRecord): + def to_schema(record: SolrStopRecord): record_dict = record.__dict__ return SolrDoc(**record_dict) @@ -69,7 +69,7 @@ class SolrResponseBody(BaseModel): @staticmethod def to_schema(response: SResponse) -> "SolrResponseBody": - docs = [SolrDoc.toSchema(doc) for doc in response.docs] + docs = [SolrDoc.to_schema(doc) for doc in response.docs] return SolrResponseBody( numFound=response.numFound, start=response.start, docs=docs ) diff --git a/com/github/ott/pelias/adapter/service/config.py b/com/github/ott/pelias/adapter/service/config.py index c7d9148..eda19b1 100644 --- a/com/github/ott/pelias/adapter/service/config.py +++ b/com/github/ott/pelias/adapter/service/config.py @@ -27,8 +27,8 @@ class SolrApiType(str, Enum): autocomplete = "autocomplete" -logger.info(f"Pelias URL: {PELIAS}") -logger.info(f"Pelias Search URL: {pelias_search_url}") -logger.info(f"Pelias Autocomplete URL: {pelias_autocomplete_url}") -logger.info(f"Pelias Reverse URL: {pelias_reverse_url}") -logger.info(f"Pelias Place URL: {pelias_place_url}") +logger.debug(f"Pelias URL: {PELIAS}") +logger.debug(f"Pelias Search URL: {pelias_search_url}") +logger.debug(f"Pelias Autocomplete URL: {pelias_autocomplete_url}") +logger.debug(f"Pelias Reverse URL: {pelias_reverse_url}") +logger.debug(f"Pelias Place URL: {pelias_place_url}") diff --git a/com/github/ott/pelias/adapter/service/pelias_service.py b/com/github/ott/pelias/adapter/service/pelias_service.py index 7ee376a..b5fd3ec 100644 --- a/com/github/ott/pelias/adapter/service/pelias_service.py +++ b/com/github/ott/pelias/adapter/service/pelias_service.py @@ -1,5 +1,8 @@ +from urllib.parse import urlencode + from fastapi import Request from ott.utils import json_utils +from starlette.datastructures import QueryParams from com.github.ott.pelias.adapter.core.errors import PeliasAdapterError from com.github.ott.pelias.adapter.service.config import ( @@ -10,9 +13,19 @@ from com.github.ott.pelias.adapter.service.config import pelias_reverse_url from com.github.ott.pelias.adapter.service.config import pelias_search_url from com.github.ott.pelias.adapter.service.pelias_wrapper import PeliasWrapper +from com.github.ott.pelias.adapter.service.util import ( + get_query_type, + QueryType, + normalize_address, + remove_non_digits, +) + +TRANSIT_LAYERS = ( + "trimet:stops,ctran:stops,sam:stops,smart:stops,mult:stops,wapark:stops" +) -def get_pelias_response( +def _get_pelias_response( request: Request, service: SearchApiType = SearchApiType.autocomplete, is_rtp=False ) -> dict: """ @@ -24,6 +37,7 @@ def get_pelias_response( # step 1: find the service based on pelias/{service}? ... default to autocomplete query = request.url.query + ret_val = {} match service: case SearchApiType.autocomplete: @@ -54,6 +68,12 @@ def get_pelias_response( # step 3: append the hostname to the response json_utils.append_hostname_to_json(ret_val) + # step 4: remove duplicate features if any + features = ret_val.get("features", []) + if features and len(features) > 0: + revised_features = remove_duplicate_features(ret_val.get("features", [])) + ret_val["features"] = revised_features + """ TODO: WIP find and add stops / in/out / etc... x = object_utils.find_elements('properties', ret_val) @@ -63,3 +83,172 @@ def get_pelias_response( """ return ret_val + + +def remove_duplicate_features(features): + """ + Best shot at normalizing and removing duplicate features from pelias response + """ + refined = {f["properties"]["name"].strip().lower(): f for f in features} + normalized_addresses = {normalize_address(key): v for key, v in refined.items()} + + return list(normalized_addresses.values()) + + +def get_best_match(ret_val, text: str, is_stop_request=False): + """ + from a pelias response, find the best match (first in list) and return it + :param ret_val: pelias response json + :param text: original query text + :param is_stop_request: whether this is a stop request + :return: first match from pelias response + """ + features = ret_val.get("features", []) + if not features or len(features) == 1: + return ret_val + else: + print(f"initial features length: {len(features)}") + features = remove_duplicate_features(features) + print(f"duplicates removed features length: {len(features)}") + if is_stop_request: + # for stop requests, we expect exactly one match + stop_id = remove_non_digits(text) + if stop_id: + matches = [ + feat + for feat in features + if isinstance(feat.get("id"), str) + and feat["id"].endswith(f":{stop_id}") + ] + ret_val["features"] = matches + else: + matches = [ + f + for f in features + if text == f.get("properties", {}).get("name", "").lower().strip() + ] + if not matches: + matches = [ + f + for f in features + if text in f.get("properties", {}).get("name", "").lower().strip() + ] + if matches: + ret_val["features"] = matches + return ret_val + + +def adjust_layers_for_query(query_type, query_params) -> tuple[bool, dict]: + if query_type in (QueryType.STOP_REQUEST, QueryType.JUST_A_NUMBER): + query_params["layers"] = TRANSIT_LAYERS + return True, query_params + elif query_type is QueryType.STREET_ADDRESS: + query_params["layers"] = "address" + return False, query_params + + +def refactor_pelias_request(request: Request, new_params: dict) -> Request: + """ " update both request._query_params and the ASGI scope query_string, + and clear cached URL so request.url reflects the new params + """ + request._query_params = QueryParams(**new_params) + request.scope["query_string"] = urlencode(new_params, doseq=True).encode() + if hasattr(request, "_url"): + # noinspection PyProtectedMember + del request._url + return request + + +def get_pelias_response( + request: Request, + service: SearchApiType = SearchApiType.autocomplete, + is_rtp: bool = False, + refine: bool = False, +): + """ + Get a response from Pelias, with optional refinement + + if refine is true, the method will try to get a better match (maybe a single match) + for stop requests or single result requests + + However, if the request is truly ambiguous (for example text="big"), the method will probably return a number of results... + Unless there is an item whose exact name matches the ambigous value + + """ + + if not refine: + return _get_pelias_response(request=request, service=service, is_rtp=is_rtp) + else: + """ + Do some data normalization here to eliminate duplicates and normalize addresses. + Try to get to a single result. + If query was ambiguous and we still end up with more than one result, return the first IF the query's size = 1 + + """ + + new_params = dict(request.query_params) + original_params = new_params.copy() + + size, text = ( + int(new_params.get("size", 10)), + (new_params.get("text") or "").lower().strip(), + ) + query_type, stop_id = get_query_type(text) + + # pelias will just return first, so we'll get 10 and try to get a better first + if size == 1: + new_params["size"] = "10" + + # determine of this is a stop request and apply reset layers to STOPS_LAYERS + # or if determined an address "address", for request + is_stop_request, new_params = adjust_layers_for_query(query_type, new_params) + + # required to ensure new params and layers stick + request = refactor_pelias_request(request=request, new_params=new_params) + + ret_val = _get_pelias_response(service=service, request=request, is_rtp=is_rtp) + + _features = ret_val.get("features", []) + + # after hte first request, we'll use feature.property.name to normalize + # and remove dupes + features = ( + remove_duplicate_features(_features) + if _features and len(_features) > 1 + else _features + ) + + # if the custom layers query resulted in none, try again without the layers filter + if ( + query_type + in ( + QueryType.STOP_REQUEST, + QueryType.JUST_A_NUMBER, + QueryType.STREET_ADDRESS, + ) + and not features + ): + # restore original params in scope and cached attrs + request = refactor_pelias_request(request, original_params) + + ret_val = _get_pelias_response( + service=service, request=request, is_rtp=is_rtp + ) + features = ret_val.get("features", {}) + + if len(features) > 1: + ret_val = get_best_match( + ret_val, text=text, is_stop_request=is_stop_request + ) + if size == 1: + # if size=1, give them 1 + features = ret_val.get("features", []) + if len(features) > 1: + ret_val["features"] = features[0:1] + else: + ret_val["features"] = features + + # restore original params before returning + refactor_pelias_request(request, original_params) + + return ret_val diff --git a/com/github/ott/pelias/adapter/service/pelias_to_solr.py b/com/github/ott/pelias/adapter/service/pelias_to_solr.py index 790d81d..3ca90b7 100644 --- a/com/github/ott/pelias/adapter/service/pelias_to_solr.py +++ b/com/github/ott/pelias/adapter/service/pelias_to_solr.py @@ -35,8 +35,8 @@ def solr_to_pelias_param(cls, solr_params, is_rtp=False): if size: ret_val["size"] = size - format = html_utils.get_first_param(solr_params, "wt") - if format and format == "xml": + fmt = html_utils.get_first_param(solr_params, "wt") + if fmt and fmt == "xml": ret_val["format"] = "xml" _ = html_utils.get_first_param(solr_params, "fq") @@ -75,7 +75,7 @@ def parse_json(cls, json, solr_params=None): @classmethod def fix_venues_in_pelias_response(cls, pelias_json): """ - will loop thru results, and append street names to venues + will loop through results, and append street names to venues NOTE: 2-24-2020: this routine is only used in the SOLR wrapper the Pelias wrapper has a different rendering (see above) """ diff --git a/com/github/ott/pelias/adapter/service/pelias_wrapper.py b/com/github/ott/pelias/adapter/service/pelias_wrapper.py index c966c83..d28460e 100644 --- a/com/github/ott/pelias/adapter/service/pelias_wrapper.py +++ b/com/github/ott/pelias/adapter/service/pelias_wrapper.py @@ -180,7 +180,7 @@ def dedup_addresses(cls, pelias_json): """ features = pelias_json.get("features", []) - # step 0: make sure there are 2+ records (eg something to dedupe) + # step 0: make sure there are 2+ records (e.g something to dedupe) if len(features) < 2: return @@ -191,12 +191,12 @@ def dedup_addresses(cls, pelias_json): p = f.get("properties") if p is None: continue - if p.get("layer") in ("address"): + if p.get("layer") in "address": adds.append(f) else: new_other.append(f) - # step 2: make sure we have at least 2 addresses (eg addresses to dedupe) + # step 2: make sure we have at least 2 addresses (e.g addresses to dedupe) if len(adds) < 2: return @@ -208,7 +208,7 @@ def dedup_addresses(cls, pelias_json): if ( num_addresses >= 2 - # make sure we have multiple addresses (eg duplicates to potentially + # make sure we have multiple addresses (e.g duplicates to potentially # dedup) ): # import pdb; pdb.set_trace() @@ -252,7 +252,7 @@ def dedup_addresses(cls, pelias_json): def fixup_response( cls, pelias_json, size=10, ele="label", is_calltaker=False, is_rtp=False ): - """will loop thru results, cleaning up / renaming / relabeling the specified element""" + """will loop through results, cleaning up / renaming / relabeling the specified element""" # step 1: loop thru the records in the Pelias response if cls.has_features(pelias_json): diff --git a/com/github/ott/pelias/adapter/service/util.py b/com/github/ott/pelias/adapter/service/util.py new file mode 100644 index 0000000..332cbd9 --- /dev/null +++ b/com/github/ott/pelias/adapter/service/util.py @@ -0,0 +1,97 @@ +import re +import enum +from typing import Optional + + +class QueryType(enum.Enum): + STREET_ADDRESS = "street_address" + JUST_A_NUMBER = "just_a_number" + STOP_REQUEST = "stop_request" + UNKNOWN = "unknown" + + +def is_street_address(text: str) -> bool: + return bool( + re.search( + r"\b\d{1,5}\s+\w+(?:\s+\w+)*\s+(?:st|street|ave|avenue|rd|road|blvd|boulevard|dr|drive|ln|lane|ct|court)\b", + text.lower(), + ) + ) + + +def get_query_type(query: str) -> tuple[QueryType, int]: + query = query.lower().strip() + stop_id = -1 + try: + stop_id = int(query) + except ValueError: + pass + if is_street_address(query): + query_type = QueryType.STREET_ADDRESS + elif ( + query.startswith("stop") + or query.startswith("stopid") + or query.startswith("stop_id") + ): + query_type = QueryType.STOP_REQUEST + elif stop_id > 0: + query_type = QueryType.JUST_A_NUMBER + else: + query_type = QueryType.UNKNOWN + + return query_type, stop_id + + +def normalize_address(addr: str) -> str: + """Normalize common street abbreviations and directions in a U.S. address.""" + if not addr: + return "" + + addr = addr.lower().strip() + + # Directional replacements + directions = { + r"\bn\b": "north", + r"\bs\b": "south", + r"\be\b": "east", + r"\bw\b": "west", + r"\bnw\b": "northwest", + r"\bne\b": "northeast", + r"\bsw\b": "southwest", + r"\bse\b": "southeast", + } + + # Street type replacements + streets = { + r"\bst\b": "street", + r"\bstreet\b": "street", + r"\bave\b": "avenue", + r"\bav\b": "avenue", + r"\baven\b": "avenue", + r"\bavenue\b": "avenue", + r"\bblvd\b": "boulevard", + r"\brd\b": "road", + r"\bdr\b": "drive", + r"\bln\b": "lane", + r"\bct\b": "court", + r"\bcir\b": "circle", + r"\bhwy\b": "highway", + r"\bpl\b": "place", + r"\bter\b": "terrace", + r"\bpkwy\b": "parkway", + } + + # Apply direction and street normalization + for pattern, repl in {**directions, **streets}.items(): + addr = re.sub(pattern + r"\.?", repl, addr) + + # Remove double spaces and capitalize properly + addr = re.sub(r"\s+", " ", addr).strip().title() + return addr + + +def remove_non_digits(s: Optional[str]) -> str: + """Return only the digits from `s`. None -> ''.""" + if not s: + return "" + return re.sub(r"\D+", "", s) diff --git a/dev.env b/dev.env index 338af27..2b535f4 100644 --- a/dev.env +++ b/dev.env @@ -1,7 +1,5 @@ APP_NAME=FastAPI Oracle Template DEBUG=True ENVIRONMENT=dev - - PELIAS_URL=https://ws.trimet.org MAPS_URL=https://maps.trimet.org \ No newline at end of file diff --git a/docker/buildDocker.sh b/docker/buildDocker.sh index 564245d..e6c1130 100755 --- a/docker/buildDocker.sh +++ b/docker/buildDocker.sh @@ -1,2 +1,3 @@ -docker build --debug --rm --progress=plain -f docker/Dockerfile -t pelias_adapter:latest . +export GIT_BRANCH=$(git rev-parse --abbrev-ref HEAD) +docker build --debug --rm --progress=plain -f docker/Dockerfile -t pelias_adapter:$(git rev-parse --abbrev-ref HEAD)" . diff --git a/docker/compose.yml b/docker/compose.yml index afa87da..90238e6 100644 --- a/docker/compose.yml +++ b/docker/compose.yml @@ -4,7 +4,7 @@ services: context: ../ dockerfile: docker/Dockerfile container_name: pelias_adapter - image: pelias_adapter:latest + image: ghcr.io/opentransittools/pelias-adapter:latest # todo make this tag dynamic ports: - "8000:8000" environment: diff --git a/main.py b/main.py index 9ab5c0f..c7985de 100644 --- a/main.py +++ b/main.py @@ -1,20 +1,17 @@ -import logging import logging.config from fastapi import FastAPI, Request from fastapi.middleware.cors import CORSMiddleware from fastapi.responses import JSONResponse -from com.github.ott.pelias.adapter.core.errors import PeliasAdapterError from com.github.ott.pelias.adapter.core.config import settings - +from com.github.ott.pelias.adapter.core.errors import PeliasAdapterError from com.github.ott.pelias.adapter.routers import core, pelias, solr # Configure logging logging.config.fileConfig("logging.ini", disable_existing_loggers=False) logger = logging.getLogger(__name__) - # Create FastAPI application app = FastAPI( title=settings.app_name, @@ -38,9 +35,10 @@ app.include_router(core.router, prefix="/core/v1", tags=["core"]) -def add_exception_handlers(app): - @app.exception_handler(PeliasAdapterError) +def add_exception_handlers(application: FastAPI): + @application.exception_handler(PeliasAdapterError) async def pelias_wrapper_error_handler(request: Request, exc: PeliasAdapterError): + logger.error(f"PeliasAdapterError: {request.url}") return JSONResponse(status_code=400, content={"error": exc.message}) diff --git a/pyproject.toml b/pyproject.toml index 65edf52..61d6cbd 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -56,6 +56,10 @@ exclude = ''' )/ ''' +[tool.pytest.ini_options] +addopts = "--cov -rA --log-cli-level=DEBUG" + + [build-system] requires = ["poetry-core>=1.7.0"] build-backend = "poetry.core.masonry.api" diff --git a/tests/README.md b/tests/README.md index 451facc..f1d9c7c 100644 --- a/tests/README.md +++ b/tests/README.md @@ -1,7 +1,7 @@ Tests todo: 1. check that errors (down Pelias) work (throws exception now) ... should return a json structure -1. remove first type:'street' results if there are are other non street / addresses close by = http://localhost:45554/pelias?text=45.5,-122.5 == a street polyline is first response +1. remove first type:'street' results if there are other non-street / addresses close by = http://localhost:45554/pelias?text=45.5,-122.5 == a street polyline is first response 1. test interpolation 1. test near stops / routes / etc... 1. test within / out boundaries diff --git a/tests/com/__init__.py b/tests/com/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/com/github/__init__.py b/tests/com/github/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/com/github/ott/__init__.py b/tests/com/github/ott/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/com/github/ott/pelias/__init__.py b/tests/com/github/ott/pelias/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/com/github/ott/pelias/adapter/__init__.py b/tests/com/github/ott/pelias/adapter/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/com/github/ott/pelias/adapter/service/test_pelias_service.py b/tests/com/github/ott/pelias/adapter/service/test_pelias_service.py new file mode 100644 index 0000000..d6409fa --- /dev/null +++ b/tests/com/github/ott/pelias/adapter/service/test_pelias_service.py @@ -0,0 +1,37 @@ +import pytest +from fastapi.testclient import TestClient + +from com.github.ott.pelias.adapter.service.util import QueryType +from main import app + +queries = { + "one": [ + ("949 NW ", "949 NW Overton Street", QueryType.STREET_ADDRESS), + ( + "5960", + "NE M L King & Weidler (TriMet Stop ID 5960)", + QueryType.JUST_A_NUMBER, + ), + ( + "stop 5955", + "NE M L King & Tillamook (TriMet Stop ID 5955)", + QueryType.STOP_REQUEST, + ), + ("moo", "tbd", QueryType.UNKNOWN), + ] +} + + +@pytest.mark.parametrize("query, expected_name, query_type", queries.get("one")) +def test_get_pelias_response_success_one(query, expected_name, query_type): + with TestClient(app) as client: + result = client.get(f"/pelias/v1/refine/autocomplete?text={query}") + assert result.status_code == 200 + data = result.json() + assert "features" in data + features = data.get("features", []) + if query != "moo": + name = features[0].get("properties", {}).get("name", "") + assert name == expected_name + else: + assert len(features) > 1 diff --git a/tests/comparison_testing/util.py b/tests/comparison_testing/util.py index ba14067..d77ea75 100644 --- a/tests/comparison_testing/util.py +++ b/tests/comparison_testing/util.py @@ -1,4 +1,6 @@ -def assert_builtins(local_data, stage_data, skip: list[str] = []): +def assert_builtins(local_data, stage_data, skip=None): + if skip is None: + skip = [] if skip: for s in skip: if s in local_data: From d7051d9d550128d47e82a9a4ce7d7c26e95b9e8a Mon Sep 17 00:00:00 2001 From: Alice Date: Wed, 29 Oct 2025 12:07:20 -0700 Subject: [PATCH 2/2] CoPilot Code Review Suggestions --- .../pelias/adapter/service/pelias_service.py | 22 ++++++++++++------- com/github/ott/pelias/adapter/service/util.py | 1 + docker/buildDocker.sh | 2 +- 3 files changed, 16 insertions(+), 9 deletions(-) diff --git a/com/github/ott/pelias/adapter/service/pelias_service.py b/com/github/ott/pelias/adapter/service/pelias_service.py index b5fd3ec..bcbfd96 100644 --- a/com/github/ott/pelias/adapter/service/pelias_service.py +++ b/com/github/ott/pelias/adapter/service/pelias_service.py @@ -89,7 +89,15 @@ def remove_duplicate_features(features): """ Best shot at normalizing and removing duplicate features from pelias response """ - refined = {f["properties"]["name"].strip().lower(): f for f in features} + refined = { + f["properties"]["name"].strip().lower(): f + for f in features + if isinstance(f, dict) + and "properties" in f + and isinstance(f["properties"], dict) + and "name" in f["properties"] + and isinstance(f["properties"]["name"], str) + } normalized_addresses = {normalize_address(key): v for key, v in refined.items()} return list(normalized_addresses.values()) @@ -107,9 +115,7 @@ def get_best_match(ret_val, text: str, is_stop_request=False): if not features or len(features) == 1: return ret_val else: - print(f"initial features length: {len(features)}") features = remove_duplicate_features(features) - print(f"duplicates removed features length: {len(features)}") if is_stop_request: # for stop requests, we expect exactly one match stop_id = remove_non_digits(text) @@ -148,7 +154,7 @@ def adjust_layers_for_query(query_type, query_params) -> tuple[bool, dict]: def refactor_pelias_request(request: Request, new_params: dict) -> Request: - """ " update both request._query_params and the ASGI scope query_string, + """update both request._query_params and the ASGI scope query_string, and clear cached URL so request.url reflects the new params """ request._query_params = QueryParams(**new_params) @@ -172,7 +178,7 @@ def get_pelias_response( for stop requests or single result requests However, if the request is truly ambiguous (for example text="big"), the method will probably return a number of results... - Unless there is an item whose exact name matches the ambigous value + Unless there is an item whose exact name matches the ambiguous value """ @@ -199,7 +205,7 @@ def get_pelias_response( if size == 1: new_params["size"] = "10" - # determine of this is a stop request and apply reset layers to STOPS_LAYERS + # determine if this is a stop request and apply reset layers to STOPS_LAYERS # or if determined an address "address", for request is_stop_request, new_params = adjust_layers_for_query(query_type, new_params) @@ -210,7 +216,7 @@ def get_pelias_response( _features = ret_val.get("features", []) - # after hte first request, we'll use feature.property.name to normalize + # after the first request, we'll use feature.property.name to normalize # and remove dupes features = ( remove_duplicate_features(_features) @@ -234,7 +240,7 @@ def get_pelias_response( ret_val = _get_pelias_response( service=service, request=request, is_rtp=is_rtp ) - features = ret_val.get("features", {}) + features = ret_val.get("features", []) if len(features) > 1: ret_val = get_best_match( diff --git a/com/github/ott/pelias/adapter/service/util.py b/com/github/ott/pelias/adapter/service/util.py index 332cbd9..e18ae2d 100644 --- a/com/github/ott/pelias/adapter/service/util.py +++ b/com/github/ott/pelias/adapter/service/util.py @@ -25,6 +25,7 @@ def get_query_type(query: str) -> tuple[QueryType, int]: try: stop_id = int(query) except ValueError: + # If parsing fails, stop_id remains -1, indicating the query is not a number. pass if is_street_address(query): query_type = QueryType.STREET_ADDRESS diff --git a/docker/buildDocker.sh b/docker/buildDocker.sh index e6c1130..da0d863 100755 --- a/docker/buildDocker.sh +++ b/docker/buildDocker.sh @@ -1,3 +1,3 @@ export GIT_BRANCH=$(git rev-parse --abbrev-ref HEAD) -docker build --debug --rm --progress=plain -f docker/Dockerfile -t pelias_adapter:$(git rev-parse --abbrev-ref HEAD)" . +docker build --debug --rm --progress=plain -f docker/Dockerfile -t pelias_adapter:$(git rev-parse --abbrev-ref HEAD) .