Merge branch 'feature/es-improvements' into develop

Author: ewlarson

app/api/v1/endpoint_modules/gazetteer.py

@@ -1,3 +1,4 @@
+import json
 import logging
 import os
 from typing import Optional
@@ -330,10 +331,24 @@ async def search_wof(
     q: str = Query(..., description="Search query"),
     limit: int = Query(10, description="Maximum number of results", ge=1, le=100),
     offset: int = Query(0, description="Number of results to skip", ge=0),
+    exclude_placetypes: Optional[str] = Query(
+        None,
+        description="Comma-separated list of placetypes to exclude (default: microhood,neighbourhood,venue)",
+    ),
     request: Request = None,
 ):
     """Search Who's on First gazetteer."""
     try:
+        # Default placetypes to exclude for autosuggestion
+        if exclude_placetypes is None:
+            exclude_placetypes = "localadmin,microhood,neighbourhood,venue"
+        # Handle case where exclude_placetypes might be a Query object (when called directly in tests)
+        elif not isinstance(exclude_placetypes, str):
+            # If it's not a string (e.g., Query object), use default
+            exclude_placetypes = "localadmin,microhood,neighbourhood,venue"
+
+        excluded_types = [pt.strip() for pt in exclude_placetypes.split(",") if pt.strip()]
+
         # Build search query
         search_terms = q.split()
         conditions = []
@@ -346,6 +361,16 @@ async def search_wof(
                 )
             )
 
+        # Exclude confusing placetypes for autosuggestion
+        # Keep records where placetype is NULL or not in excluded list
+        if excluded_types:
+            conditions.append(
+                or_(
+                    gazetteer_wof_spr.c.placetype.is_(None),
+                    ~gazetteer_wof_spr.c.placetype.in_(excluded_types),
+                )
+            )
+
         query = (
             select(gazetteer_wof_spr)
             .where(and_(*conditions))
@@ -356,13 +381,121 @@ async def search_wof(
 
         results = await database.fetch_all(query)
 
+        # Collect wok_ids for batch fetching ancestors and geojson
+        wok_ids = [result["wok_id"] for result in results]
+
+        # Fetch ancestors for all results in batch
+        ancestors_map = {}
+        if wok_ids:
+            ancestors_query = select(gazetteer_wof_ancestors).where(
+                gazetteer_wof_ancestors.c.wok_id.in_(wok_ids)
+            )
+            ancestors = await database.fetch_all(ancestors_query)
+
+            # Group ancestors by wok_id
+            for ancestor in ancestors:
+                wok_id = ancestor["wok_id"]
+                if wok_id not in ancestors_map:
+                    ancestors_map[wok_id] = []
+                ancestors_map[wok_id].append(dict(ancestor))
+
+            # Fetch ancestor names from spr table
+            ancestor_ids = list(
+                set(
+                    [
+                        a["ancestor_id"]
+                        for ancestors_list in ancestors_map.values()
+                        for a in ancestors_list
+                    ]
+                )
+            )
+            if ancestor_ids:
+                # Compare ancestor_id (Integer) with wok_id (BigInteger) - PostgreSQL handles type coercion
+                ancestor_spr_query = select(gazetteer_wof_spr).where(
+                    gazetteer_wof_spr.c.wok_id.in_(ancestor_ids)
+                )
+                ancestor_sprs = await database.fetch_all(ancestor_spr_query)
+                ancestor_names_map = {spr["wok_id"]: spr["name"] for spr in ancestor_sprs}
+
+                # Add names to ancestors
+                for wok_id, ancestors_list in ancestors_map.items():
+                    for ancestor in ancestors_list:
+                        ancestor["name"] = ancestor_names_map.get(ancestor["ancestor_id"])
+
+        # Fetch GeoJSON for all results in batch
+        geojson_map = {}
+        if wok_ids:
+            geojson_query = (
+                select(gazetteer_wof_geojson)
+                .where(gazetteer_wof_geojson.c.wok_id.in_(wok_ids))
+                .order_by(
+                    # Prefer non-alt geometries, then by source preference
+                    gazetteer_wof_geojson.c.is_alt.asc(),
+                    gazetteer_wof_geojson.c.source.asc(),
+                )
+            )
+            geojson_records = await database.fetch_all(geojson_query)
+
+            # Group by wok_id, keeping only the first (best) one
+            for geojson_record in geojson_records:
+                wok_id = geojson_record["wok_id"]
+                if wok_id not in geojson_map:
+                    geojson_map[wok_id] = dict(geojson_record)
+
         # Convert results to JSON:API format
         data = []
         for row in results:
             row_dict = dict(row)
+            wok_id = row_dict.get("wok_id")
+
+            # Get ancestors for this place
+            ancestors = ancestors_map.get(wok_id, [])
+
+            # Build hierarchy: prefer region, county, locality for display
+            hierarchy_parts = []
+            hierarchy_placetypes = ["region", "county", "locality"]
+
+            # Sort ancestors by placetype priority
+            sorted_ancestors = sorted(
+                ancestors,
+                key=lambda a: hierarchy_placetypes.index(a.get("ancestor_placetype", ""))
+                if a.get("ancestor_placetype") in hierarchy_placetypes
+                else 999,
+            )
+
+            for ancestor in sorted_ancestors:
+                if ancestor.get("ancestor_placetype") in hierarchy_placetypes and ancestor.get(
+                    "name"
+                ):
+                    hierarchy_parts.append(ancestor["name"])
+
+            # Build display name: "Name, Parent1, Parent2, Country"
+            display_parts = [row_dict.get("name", "")]
+            display_parts.extend(hierarchy_parts)
+            if row_dict.get("country"):
+                display_parts.append(row_dict["country"])
+            display_name = ", ".join(display_parts)
+
+            # Get GeoJSON for this place
+            geojson_record = geojson_map.get(wok_id)
+            geojson_data = None
+            if geojson_record:
+                try:
+                    geojson_data = json.loads(geojson_record["body"])
+                except (json.JSONDecodeError, TypeError):
+                    geojson_data = None
+
             # Sanitize the data for JSON serialization
             row_dict = sanitize_for_json(row_dict)
 
+            # Sanitize ancestors/hierarchy data (may contain date fields)
+            sanitized_ancestors = sanitize_for_json(ancestors)
+
+            # Add enhanced fields
+            row_dict["display_name"] = display_name
+            row_dict["hierarchy"] = sanitized_ancestors
+            row_dict["geojson"] = geojson_data
+
             # Format as JSON:API resource
             formatted_row = {
                 "id": str(row_dict.get("wok_id", row_dict.get("id", ""))),
@@ -395,5 +528,6 @@ async def search_wof(
         return JSONResponse(content=reordered_response)
 
     except Exception as e:
-        logger.error(f"Error searching WOF: {str(e)}", exc_info=True)
-        raise HTTPException(status_code=500, detail="Failed to search WOF") from e
+        error_msg = str(e)
+        logger.error(f"Error searching WOF: {error_msg}", exc_info=True)
+        raise HTTPException(status_code=500, detail=f"Failed to search WOF: {error_msg}") from e

app/api/v1/endpoint_modules/resources.py

@@ -258,6 +258,15 @@ async def get_resource_distributions(
         raise HTTPException(status_code=500, detail=str(e)) from e
 
 
+@router.get("/resources/{id}/links")
+async def get_resource_links(
+    id: str,
+    callback: Optional[str] = Query(None, description="JSONP callback name"),
+):
+    """Get all links for a resource."""
+    return await LinkService.get_resource_links(id)
+
+
 @router.get("/resources/{id}/ogm")
 async def get_resource_ogm(
     id: str,
@@ -317,15 +326,6 @@ async def get_resource_ogm(
         return JSONResponse(content={"error": str(e)}, status_code=500)
 
 
-@router.get("/resources/{id}/links")
-async def get_resource_links(
-    id: str,
-    callback: Optional[str] = Query(None, description="JSONP callback name"),
-):
-    """Get all links for a resource."""
-    return await LinkService.get_resource_links(id)
-
-
 @router.get("/resources/{id}/relationships")
 async def get_resource_relationships(
     id: str,
@@ -389,8 +389,7 @@ async def get_resource_static_map(
     id: str,
     request: Request = None,
 ):
-    """Get a static map image for a resource based on its locn_geometry (or dcat_bbox as fallback).
-    """
+    """Get a static map image for a resource based on its locn_geometry (or dcat_bbox as fallback)."""
     try:
         # First check if the resource exists and has geometry
         async with async_session() as session:
@@ -506,16 +505,13 @@ async def get_resource_thumbnail(
         image_service = ImageService(resource_dict, distribution_context=distribution_context)
         thumbnail_url = image_service.get_thumbnail_url()
 
-        # Ensure placeholder when none available
-        if not thumbnail_url:
-            application_url = os.getenv("APPLICATION_URL", "http://localhost:8000").rstrip("/")
-            thumbnail_url = f"{application_url}/api/v1/thumbnails/placeholder"
-
-        is_placeholder = "/api/v1/thumbnails/placeholder" in thumbnail_url
+        # Determine if it's a placeholder (only true if URL is actually a placeholder)
+        # If thumbnail_url is None, placeholder should be false (frontend uses resource class icon)
+        is_placeholder = bool(thumbnail_url and "/thumbnails/placeholder" in str(thumbnail_url))
 
         response_payload = {
             "id": id,
-            "thumbnail_url": thumbnail_url,
+            "thumbnail_url": thumbnail_url,  # Can be None (use resource class), placeholder URL, or actual thumbnail URL
             "placeholder": is_placeholder,
         }
 
@@ -529,17 +525,23 @@ async def get_resource_thumbnail(
                 image_service._standardize_iiif_url(source_url) if source_url else None
             )
 
-            # If manifest, try to resolve to an image (network call; 2s timeout inside service)
+            # If manifest, try to resolve from cache only (no blocking HTTP calls)
             resolved_url = None
             if (
                 source_url
                 and isinstance(source_url, str)
-                and source_url.endswith(("/manifest", "manifest.json"))
+                and image_service._is_manifest_url(source_url)
             ):
                 try:
-                    resolved_url = image_service.get_iiif_manifest_thumbnail(source_url)
-                    if resolved_url:
-                        resolved_url = image_service._standardize_iiif_url(resolved_url)
+                    # Only check cache - don't fetch manifests synchronously
+                    manifest_cache_key = f"manifest:{source_url}"
+                    cached_manifest_data = image_service.cache.get(manifest_cache_key)
+                    if cached_manifest_data:
+                        import json
+                        manifest_json = json.loads(cached_manifest_data)
+                        resolved_url = image_service._extract_thumbnail_from_manifest_json(manifest_json, source_url)
+                        if resolved_url:
+                            resolved_url = image_service._standardize_iiif_url(resolved_url)
                 except Exception:
                     resolved_url = None
 

app/api/v1/endpoint_modules/search.py

@@ -2,6 +2,8 @@
 import logging
 from typing import Annotated, Optional
 
+logger = logging.getLogger(__name__)
+
 from fastapi import APIRouter, Body, HTTPException, Query, Request
 from fastapi.responses import JSONResponse
 from sqlalchemy import select
@@ -13,6 +15,7 @@
 from app.api.v1.utils import (
     create_jsonapi_response,
     create_pagination_links,
+    filter_empty_values,
     process_resource_optimized,
     sanitize_for_json,
 )
@@ -92,15 +95,23 @@ async def _handle_search(request: Request, params: dict) -> JSONResponse:
     for item in sanitized_results.get("data", []):
         rid = None
         score = None
+        overlap = None
         if isinstance(item, dict):
             rid = (
                 item.get("id")
                 or item.get("attributes", {}).get("id")
                 or item.get("attributes", {}).get("attributes", {}).get("id")
             )
-            score = item.get("score") or item.get("attributes", {}).get("score")
+            # Be careful not to treat 0.0 as falsy; only fall back on None
+            score = item.get("score")
+            if score is None:
+                score = item.get("attributes", {}).get("score")
+
+            overlap = item.get("bbox_overlap_ratio")
+            if overlap is None:
+                overlap = item.get("attributes", {}).get("bbox_overlap_ratio")
         if rid:
-            resource_data.append({"id": rid, "score": score})
+            resource_data.append({"id": rid, "score": score, "bbox_overlap_ratio": overlap})
 
     # Step 3: Batch fetch resource data
     async with async_session() as session:
@@ -117,6 +128,14 @@ async def _handle_search(request: Request, params: dict) -> JSONResponse:
             d = lookup.get(rd["id"]) or {}
             obj = await process_resource_optimized(d, {}, apply_field_mapping=False)
             mapped_attrs = OGMFieldMapper.map_resource_fields(obj.get("attributes", {}))
+            # Attach ES scoring and overlap ratio info into per-resource meta for debugging
+            if rd.get("score") is not None:
+                obj.setdefault("meta", {})
+                obj["meta"]["score"] = rd["score"]
+            if rd.get("bbox_overlap_ratio") is not None:
+                obj.setdefault("meta", {})
+                obj["meta"]["bbox_overlap_ratio"] = rd["bbox_overlap_ratio"]
+
             if isinstance(fields, str) and fields.strip():
                 requested = [f.strip() for f in fields.split(",") if f.strip()]
                 if "id" not in requested:
@@ -126,6 +145,9 @@ async def _handle_search(request: Request, params: dict) -> JSONResponse:
                 obj["attributes"] = filtered_attrs
             else:
                 obj["attributes"] = mapped_attrs
+            
+            # Filter out empty arrays and empty strings from attributes
+            obj["attributes"] = filter_empty_values(obj["attributes"])
             if not meta and "meta" in obj:
                 obj.pop("meta", None)
             processed_resources.append(obj)
@@ -216,6 +238,21 @@ async def search(
                     status_code=400,
                 )
 
+        # Get the raw query string from the request scope before FastAPI parses it
+        # This preserves bracket notation that FastAPI might filter from request.url.query
+        raw_query_string = (
+            request.scope.get("query_string", b"").decode("utf-8")
+            if isinstance(request.scope.get("query_string"), bytes)
+            else request.scope.get("query_string", "")
+        )
+        query_string = (
+            raw_query_string
+            if raw_query_string
+            else (request.url.query if request.url.query else "")
+        )
+        logger.info(
+            f"Search GET: raw_query_string length={len(raw_query_string)}, query_string length={len(query_string)}, sample={query_string[:300]}"
+        )
         return await _handle_search(
             request,
             {
@@ -228,7 +265,7 @@ async def search(
                 "facets": facets,
                 "meta": meta,
                 "callback": callback,
-                "request_query_params": str(request.query_params),
+                "request_query_params": query_string,
                 "adv_q": parsed_adv_q,
             },
         )