From: Sarah Hoffmann <lonvia@denofr.de>
Date: Fri, 15 Nov 2024 08:45:14 +0000 (+0100)
Subject: Merge remote-tracking branch 'upstream/master'
X-Git-Url: https://git.openstreetmap.org/nominatim.git/commitdiff_plain/9f417d5f64aecf72d4a3bd22d6e9d6c8fac76a51?hp=2c8be0704b764e38f15e4ff5a6625dc8e7cfbaa3

Merge remote-tracking branch 'upstream/master'
---

diff --git a/.github/ISSUE_TEMPLATE/report-issues-with-search-results.md b/.github/ISSUE_TEMPLATE/report-issues-with-search-results.md
index 7f6b76fe..cf783f70 100644
--- a/.github/ISSUE_TEMPLATE/report-issues-with-search-results.md
+++ b/.github/ISSUE_TEMPLATE/report-issues-with-search-results.md
@@ -17,11 +17,11 @@ assignees: ''
 
 ## What result did you expect?
 
-**When the result in the right place and just named wrongly:** 
+**When the result is in the right place and just named wrongly:** 
 
 <!-- Please tell us the display name you expected. -->
 
-**When the result missing completely:**
+**When the result is missing completely:**
 
 <!-- Make sure that the data you are looking for is in OpenStreetMap. Provide a link to the OpenStreetMap object or if you cannot get it, a link to the map on https://openstreetmap.org where you expect the result to be.
 
diff --git a/src/nominatim_api/lookup.py b/src/nominatim_api/lookup.py
index 618e0a09..3f5b61ae 100644
--- a/src/nominatim_api/lookup.py
+++ b/src/nominatim_api/lookup.py
@@ -43,7 +43,7 @@ class LookupTuple:
 class LookupCollector:
     """ Result collector for the simple lookup.
 
-        Allows for lookup of multiple places simultaniously.
+        Allows for lookup of multiple places simultaneously.
     """
 
     def __init__(self, places: Iterable[ntyp.PlaceRef],
@@ -201,22 +201,24 @@ async def find_in_placex(conn: SearchConnection, collector: Collector) -> bool:
                     t.c.geometry.ST_Expand(0).label('bbox'),
                     t.c.centroid)
 
-    osm_ids = [{'i': i, 'ot': p.osm_type, 'oi': p.osm_id, 'oc': p.osm_class or ''}
-               for i, p in collector.enumerate_free_osm_ids()]
-
-    if osm_ids:
-        oid_tab = sa.func.JsonArrayEach(sa.type_coerce(osm_ids, sa.JSON))\
-                    .table_valued(sa.column('value', type_=sa.JSON))
-        psql = sql.add_columns(oid_tab.c.value['i'].as_integer().label('_idx'))\
-                  .where(t.c.osm_type == oid_tab.c.value['ot'].as_string())\
-                  .where(t.c.osm_id == oid_tab.c.value['oi'].as_string().cast(sa.BigInteger))\
-                  .where(sa.or_(oid_tab.c.value['oc'].as_string() == '',
-                                oid_tab.c.value['oc'].as_string() == t.c.class_))\
-                  .order_by(t.c.class_)
-
-        if await collector.add_rows_from_sql(conn, psql, t.c.geometry,
-                                             nres.create_from_placex_row):
-            return True
+    for osm_type in ('N', 'W', 'R'):
+        osm_ids = [{'i': i, 'oi': p.osm_id, 'oc': p.osm_class or ''}
+                   for i, p in collector.enumerate_free_osm_ids()
+                   if p.osm_type == osm_type]
+
+        if osm_ids:
+            oid_tab = sa.func.JsonArrayEach(sa.type_coerce(osm_ids, sa.JSON))\
+                        .table_valued(sa.column('value', type_=sa.JSON))
+            psql = sql.add_columns(oid_tab.c.value['i'].as_integer().label('_idx'))\
+                      .where(t.c.osm_type == osm_type)\
+                      .where(t.c.osm_id == oid_tab.c.value['oi'].as_string().cast(sa.BigInteger))\
+                      .where(sa.or_(oid_tab.c.value['oc'].as_string() == '',
+                                    oid_tab.c.value['oc'].as_string() == t.c.class_))\
+                      .order_by(t.c.class_)
+
+            if await collector.add_rows_from_sql(conn, psql, t.c.geometry,
+                                                 nres.create_from_placex_row):
+                return True
 
     place_ids = [{'i': i, 'id': p.place_id}
                  for i, p in collector.enumerate_free_place_ids()]
diff --git a/src/nominatim_api/server/falcon/server.py b/src/nominatim_api/server/falcon/server.py
index b58c1cfa..13e79311 100644
--- a/src/nominatim_api/server/falcon/server.py
+++ b/src/nominatim_api/server/falcon/server.py
@@ -147,12 +147,36 @@ class FileLoggingMiddleware:
                       f'{resource.name} "{params}"\n')
 
 
-class APIShutdown:
-    """ Middleware that closes any open database connections.
+class APIMiddleware:
+    """ Middleware managing the Nominatim database connection.
     """
 
-    def __init__(self, api: NominatimAPIAsync) -> None:
-        self.api = api
+    def __init__(self, project_dir: Path, environ: Optional[Mapping[str, str]]) -> None:
+        self.api = NominatimAPIAsync(project_dir, environ)
+        self.app: Optional[App] = None
+
+    @property
+    def config(self) -> Configuration:
+        """ Get the configuration for Nominatim.
+        """
+        return self.api.config
+
+    def set_app(self, app: App) -> None:
+        """ Set the Falcon application this middleware is connected to.
+        """
+        self.app = app
+
+    async def process_startup(self, *_: Any) -> None:
+        """ Process the ASGI lifespan startup event.
+        """
+        assert self.app is not None
+        legacy_urls = self.api.config.get_bool('SERVE_LEGACY_URLS')
+        formatter = load_format_dispatcher('v1', self.api.config.project_dir)
+        for name, func in await api_impl.get_routes(self.api):
+            endpoint = EndpointWrapper(name, func, self.api, formatter)
+            self.app.add_route(f"/{name}", endpoint)
+            if legacy_urls:
+                self.app.add_route(f"/{name}.php", endpoint)
 
     async def process_shutdown(self, *_: Any) -> None:
         """Process the ASGI lifespan shutdown event.
@@ -164,28 +188,22 @@ def get_application(project_dir: Path,
                     environ: Optional[Mapping[str, str]] = None) -> App:
     """ Create a Nominatim Falcon ASGI application.
     """
-    api = NominatimAPIAsync(project_dir, environ)
+    apimw = APIMiddleware(project_dir, environ)
 
-    middleware: List[object] = [APIShutdown(api)]
-    log_file = api.config.LOG_FILE
+    middleware: List[object] = [apimw]
+    log_file = apimw.config.LOG_FILE
     if log_file:
         middleware.append(FileLoggingMiddleware(log_file))
 
-    app = App(cors_enable=api.config.get_bool('CORS_NOACCESSCONTROL'),
+    app = App(cors_enable=apimw.config.get_bool('CORS_NOACCESSCONTROL'),
               middleware=middleware)
+
+    apimw.set_app(app)
     app.add_error_handler(HTTPNominatimError, nominatim_error_handler)
     app.add_error_handler(TimeoutError, timeout_error_handler)
     # different from TimeoutError in Python <= 3.10
     app.add_error_handler(asyncio.TimeoutError, timeout_error_handler)  # type: ignore[arg-type]
 
-    legacy_urls = api.config.get_bool('SERVE_LEGACY_URLS')
-    formatter = load_format_dispatcher('v1', project_dir)
-    for name, func in api_impl.ROUTES:
-        endpoint = EndpointWrapper(name, func, api, formatter)
-        app.add_route(f"/{name}", endpoint)
-        if legacy_urls:
-            app.add_route(f"/{name}.php", endpoint)
-
     return app
 
 
diff --git a/src/nominatim_api/server/starlette/server.py b/src/nominatim_api/server/starlette/server.py
index 48f0207a..e6c97693 100644
--- a/src/nominatim_api/server/starlette/server.py
+++ b/src/nominatim_api/server/starlette/server.py
@@ -7,10 +7,12 @@
 """
 Server implementation using the starlette webserver framework.
 """
-from typing import Any, Optional, Mapping, Callable, cast, Coroutine, Dict, Awaitable
+from typing import Any, Optional, Mapping, Callable, cast, Coroutine, Dict, \
+                   Awaitable, AsyncIterator
 from pathlib import Path
 import datetime as dt
 import asyncio
+import contextlib
 
 from starlette.applications import Starlette
 from starlette.routing import Route
@@ -66,7 +68,7 @@ class ParamWrapper(ASGIAdaptor):
         return cast(Configuration, self.request.app.state.API.config)
 
     def formatting(self) -> FormatDispatcher:
-        return cast(FormatDispatcher, self.request.app.state.API.formatter)
+        return cast(FormatDispatcher, self.request.app.state.formatter)
 
 
 def _wrap_endpoint(func: EndpointFunc)\
@@ -132,14 +134,6 @@ def get_application(project_dir: Path,
     """
     config = Configuration(project_dir, environ)
 
-    routes = []
-    legacy_urls = config.get_bool('SERVE_LEGACY_URLS')
-    for name, func in api_impl.ROUTES:
-        endpoint = _wrap_endpoint(func)
-        routes.append(Route(f"/{name}", endpoint=endpoint))
-        if legacy_urls:
-            routes.append(Route(f"/{name}.php", endpoint=endpoint))
-
     middleware = []
     if config.get_bool('CORS_NOACCESSCONTROL'):
         middleware.append(Middleware(CORSMiddleware,
@@ -156,14 +150,26 @@ def get_application(project_dir: Path,
         asyncio.TimeoutError: timeout_error
     }
 
-    async def _shutdown() -> None:
+    @contextlib.asynccontextmanager
+    async def lifespan(app: Starlette) -> AsyncIterator[Any]:
+        app.state.API = NominatimAPIAsync(project_dir, environ)
+        config = app.state.API.config
+
+        legacy_urls = config.get_bool('SERVE_LEGACY_URLS')
+        for name, func in await api_impl.get_routes(app.state.API):
+            endpoint = _wrap_endpoint(func)
+            app.routes.append(Route(f"/{name}", endpoint=endpoint))
+            if legacy_urls:
+                app.routes.append(Route(f"/{name}.php", endpoint=endpoint))
+
+        yield
+
         await app.state.API.close()
 
-    app = Starlette(debug=debug, routes=routes, middleware=middleware,
+    app = Starlette(debug=debug, middleware=middleware,
                     exception_handlers=exceptions,
-                    on_shutdown=[_shutdown])
+                    lifespan=lifespan)
 
-    app.state.API = NominatimAPIAsync(project_dir, environ)
     app.state.formatter = load_format_dispatcher('v1', project_dir)
 
     return app
diff --git a/src/nominatim_api/v1/__init__.py b/src/nominatim_api/v1/__init__.py
index f49de2a2..2304994f 100644
--- a/src/nominatim_api/v1/__init__.py
+++ b/src/nominatim_api/v1/__init__.py
@@ -8,4 +8,4 @@
 Implementation of API version v1 (aka the legacy version).
 """
 
-from .server_glue import ROUTES as ROUTES
+from .server_glue import get_routes as get_routes
diff --git a/src/nominatim_api/v1/server_glue.py b/src/nominatim_api/v1/server_glue.py
index ee502d8b..a6450bf2 100644
--- a/src/nominatim_api/v1/server_glue.py
+++ b/src/nominatim_api/v1/server_glue.py
@@ -8,7 +8,7 @@
 Generic part of the server implementation of the v1 API.
 Combine with the scaffolding provided for the various Python ASGI frameworks.
 """
-from typing import Optional, Any, Type, Dict, cast
+from typing import Optional, Any, Type, Dict, cast, Sequence, Tuple
 from functools import reduce
 import dataclasses
 from urllib.parse import urlencode
@@ -25,7 +25,8 @@ from ..results import DetailedResult, ReverseResults, SearchResult, SearchResult
 from ..localization import Locales
 from . import helpers
 from ..server import content_types as ct
-from ..server.asgi_adaptor import ASGIAdaptor
+from ..server.asgi_adaptor import ASGIAdaptor, EndpointFunc
+from ..sql.async_core_library import PGCORE_ERROR
 
 
 def build_response(adaptor: ASGIAdaptor, output: str, status: int = 200,
@@ -417,12 +418,25 @@ async def polygons_endpoint(api: NominatimAPIAsync, params: ASGIAdaptor) -> Any:
     return build_response(params, params.formatting().format_result(results, fmt, {}))
 
 
-ROUTES = [
-    ('status', status_endpoint),
-    ('details', details_endpoint),
-    ('reverse', reverse_endpoint),
-    ('lookup', lookup_endpoint),
-    ('search', search_endpoint),
-    ('deletable', deletable_endpoint),
-    ('polygons', polygons_endpoint),
-]
+async def get_routes(api: NominatimAPIAsync) -> Sequence[Tuple[str, EndpointFunc]]:
+    routes = [
+        ('status', status_endpoint),
+        ('details', details_endpoint),
+        ('reverse', reverse_endpoint),
+        ('lookup', lookup_endpoint),
+        ('deletable', deletable_endpoint),
+        ('polygons', polygons_endpoint),
+    ]
+
+    def has_search_name(conn: sa.engine.Connection) -> bool:
+        insp = sa.inspect(conn)
+        return insp.has_table('search_name')
+
+    try:
+        async with api.begin() as conn:
+            if await conn.connection.run_sync(has_search_name):
+                routes.append(('search', search_endpoint))
+    except (PGCORE_ERROR, sa.exc.OperationalError):
+        pass  # ignored
+
+    return routes