From e916d27b7c395b9bb2aed6e6569db7bc2b8652a5 Mon Sep 17 00:00:00 2001 From: Emily Love Watson Date: Wed, 20 Aug 2025 14:59:25 -0500 Subject: [PATCH] Update entrances when entrance nodes are updated --- lib-sql/functions/place_triggers.sql | 32 ++++++++ lib-sql/functions/placex_triggers.sql | 11 +-- lib-sql/functions/utils.sql | 43 ++++++++++ lib-sql/table-triggers.sql | 4 + src/nominatim_api/v1/format.py | 4 +- src/nominatim_api/v1/format_json.py | 33 ++++++-- src/nominatim_api/v1/format_xml.py | 18 +++- src/nominatim_api/v1/server_glue.py | 4 +- test/bdd/features/db/import/entrances.feature | 22 +++++ test/bdd/features/db/update/entrances.feature | 82 +++++++++++++++++++ 10 files changed, 232 insertions(+), 21 deletions(-) create mode 100644 test/bdd/features/db/import/entrances.feature create mode 100644 test/bdd/features/db/update/entrances.feature diff --git a/lib-sql/functions/place_triggers.sql b/lib-sql/functions/place_triggers.sql index 5b4a8441..f9e17805 100644 --- a/lib-sql/functions/place_triggers.sql +++ b/lib-sql/functions/place_triggers.sql @@ -365,3 +365,35 @@ BEGIN RETURN NULL; END; $$ LANGUAGE plpgsql; + +CREATE OR REPLACE FUNCTION place_after_insert() + RETURNS TRIGGER + AS $$ +BEGIN + {% if debug %} + RAISE WARNING 'place_after_insert: % % % % %',NEW.osm_type,NEW.osm_id,NEW.class,NEW.type,st_area(NEW.geometry); + {% endif %} + + IF NEW.class IN ('routing:entrance', 'entrance') THEN + PERFORM place_update_entrances_for_node(NEW.osm_id); + END IF; + + RETURN NEW; +END; +$$ LANGUAGE plpgsql; + +CREATE OR REPLACE FUNCTION place_after_delete() + RETURNS TRIGGER + AS $$ +BEGIN + {% if debug %} + RAISE WARNING 'place_after_delete: % % % % %',OLD.osm_type,OLD.osm_id,OLD.class,OLD.type,st_area(OLD.geometry); + {% endif %} + + IF OLD.class IN ('routing:entrance', 'entrance') THEN + PERFORM place_update_entrances_for_node(OLD.osm_id); + END IF; + + RETURN NULL; +END; +$$ LANGUAGE plpgsql; diff --git a/lib-sql/functions/placex_triggers.sql b/lib-sql/functions/placex_triggers.sql index fe9983ba..14a1228d 100644 --- a/lib-sql/functions/placex_triggers.sql +++ b/lib-sql/functions/placex_triggers.sql @@ -883,16 +883,7 @@ BEGIN -- Record the entrance node locations IF NEW.osm_type = 'W' and (NEW.rank_search > 27 or NEW.class IN ('landuse', 'leisure')) THEN - SELECT jsonb_agg(jsonb_build_object('osm_id', osm_id, 'type', type, 'lat', ST_Y(geometry), 'lon', ST_X(geometry), 'extratags', extratags)) - FROM place - WHERE osm_id IN (SELECT unnest(nodes) FROM planet_osm_ways WHERE id=NEW.osm_id) AND class IN ('routing:entrance', 'entrance') - INTO entrances; - IF entrances IS NOT NULL THEN - INSERT INTO place_entrance (place_id, entrances) - SELECT NEW.place_id, entrances - ON CONFLICT (place_id) DO UPDATE - SET entrances = excluded.entrances; - END IF; + PERFORM place_update_entrances(NEW.place_id, NEW.osm_id); END IF; -- recalculate country and partition diff --git a/lib-sql/functions/utils.sql b/lib-sql/functions/utils.sql index 250d38c1..ee5af047 100644 --- a/lib-sql/functions/utils.sql +++ b/lib-sql/functions/utils.sql @@ -623,3 +623,46 @@ BEGIN RETURN NULL; END; $$ LANGUAGE plpgsql; + + +CREATE OR REPLACE FUNCTION place_update_entrances_for_node(osmid BIGINT) + RETURNS INTEGER + AS $$ +DECLARE + entrance_way RECORD; +BEGIN + FOR entrance_way IN + SELECT osm_id, place_id FROM planet_osm_ways JOIN placex ON placex.osm_id = planet_osm_ways.id WHERE osmid=ANY(nodes) + LOOP + PERFORM place_update_entrances(entrance_way.place_id, entrance_way.osm_id); + END LOOP; + + RETURN NULL; +END; +$$ +LANGUAGE plpgsql; + + +CREATE OR REPLACE FUNCTION place_update_entrances(placeid BIGINT, osmid BIGINT) + RETURNS INTEGER + AS $$ +DECLARE + entrances JSONB; +BEGIN + SELECT jsonb_agg(jsonb_build_object('osm_id', osm_id, 'type', type, 'lat', ST_Y(geometry), 'lon', ST_X(geometry), 'extratags', extratags)) + FROM place + WHERE osm_id IN (SELECT unnest(nodes) FROM planet_osm_ways WHERE id=osmid) AND class IN ('routing:entrance', 'entrance') + INTO entrances; + IF entrances IS NOT NULL THEN + INSERT INTO place_entrance (place_id, entrances) + SELECT placeid, entrances + ON CONFLICT (place_id) DO UPDATE + SET entrances = excluded.entrances; + ELSE + DELETE FROM place_entrance WHERE place_id=placeid; + END IF; + + RETURN NULL; +END; +$$ +LANGUAGE plpgsql; diff --git a/lib-sql/table-triggers.sql b/lib-sql/table-triggers.sql index d1a37316..078a9d98 100644 --- a/lib-sql/table-triggers.sql +++ b/lib-sql/table-triggers.sql @@ -24,6 +24,10 @@ CREATE TRIGGER place_before_delete BEFORE DELETE ON place FOR EACH ROW EXECUTE PROCEDURE place_delete(); CREATE TRIGGER place_before_insert BEFORE INSERT ON place FOR EACH ROW EXECUTE PROCEDURE place_insert(); +CREATE TRIGGER place_after_insert AFTER INSERT ON place + FOR EACH ROW EXECUTE PROCEDURE place_after_insert(); +CREATE TRIGGER place_after_delete AFTER DELETE ON place + FOR EACH ROW EXECUTE PROCEDURE place_after_delete(); CREATE TRIGGER location_postcode_before_update BEFORE UPDATE ON location_postcode FOR EACH ROW EXECUTE PROCEDURE postcode_update(); diff --git a/src/nominatim_api/v1/format.py b/src/nominatim_api/v1/format.py index 2e9aae06..5a9a9c02 100644 --- a/src/nominatim_api/v1/format.py +++ b/src/nominatim_api/v1/format.py @@ -196,8 +196,8 @@ def _format_details_json(result: DetailedResult, options: Mapping[str, Any]) -> else: _add_address_rows(out, 'hierarchy', result.parented_rows, locales) - if result.entrances is not None: - out.keyval('entrances', result.entrances) + if options.get('entrances', False): + format_json.write_entrances(out, result.entrances) out.end_object() diff --git a/src/nominatim_api/v1/format_json.py b/src/nominatim_api/v1/format_json.py index 18e93dd0..de6a648a 100644 --- a/src/nominatim_api/v1/format_json.py +++ b/src/nominatim_api/v1/format_json.py @@ -7,11 +7,12 @@ """ Helper functions for output of results in json formats. """ -from typing import Mapping, Any, Optional, Tuple, Union +from typing import Mapping, Any, Optional, Tuple, Union, List from ..utils.json_writer import JsonWriter from ..results import AddressLines, ReverseResults, SearchResults from . import classtypes as cl +from ..types import EntranceDetails def _write_osm_id(out: JsonWriter, osm_object: Optional[Tuple[str, int]]) -> None: @@ -64,6 +65,28 @@ def _write_geocodejson_address(out: JsonWriter, out.keyval('country_code', country_code) +def write_entrances(out: JsonWriter, entrances: Optional[List[EntranceDetails]]) -> None: + if entrances is None: + out.keyval('entrances', None) + return + + out.key('entrances')\ + .start_array() + + for entrance in entrances: + out.start_object()\ + .keyval('osm_id', entrance.osm_id)\ + .keyval('type', entrance.type)\ + .keyval('lat', f"{entrance.lat:0.7f}")\ + .keyval('lon', f"{entrance.lon:0.7f}") + + if entrance.extratags: + out.keyval('extratags', entrance.extratags) + out.end_object().next() + + out.end_array().next() + + def format_base_json(results: Union[ReverseResults, SearchResults], options: Mapping[str, Any], simple: bool, class_label: str) -> str: @@ -107,8 +130,8 @@ def format_base_json(results: Union[ReverseResults, SearchResults], _write_typed_address(out, result.address_rows, result.country_code) out.end_object().next() - if options.get('entrances', False) and result.entrances: - out.keyval('entrances', result.entrances) + if options.get('entrances', False): + write_entrances(out, result.entrances) if options.get('extratags', False): out.keyval('extratags', result.extratags) @@ -184,7 +207,7 @@ def format_base_geojson(results: Union[ReverseResults, SearchResults], out.end_object().next() if options.get('entrances', False): - out.keyval('entrances', result.entrances) + write_entrances(out, result.entrances) if options.get('extratags', False): out.keyval('extratags', result.extratags) @@ -258,7 +281,7 @@ def format_base_geocodejson(results: Union[ReverseResults, SearchResults], out.end_object().next() if options.get('entrances', False): - out.keyval('entrances', result.entrances) + write_entrances(out, result.entrances) if options.get('extratags', False): out.keyval('extra', result.extratags) diff --git a/src/nominatim_api/v1/format_xml.py b/src/nominatim_api/v1/format_xml.py index 17a1131c..ec5b7255 100644 --- a/src/nominatim_api/v1/format_xml.py +++ b/src/nominatim_api/v1/format_xml.py @@ -8,13 +8,13 @@ Helper functions for output of results in XML format. """ from typing import Mapping, Any, Optional, Union -import dataclasses import datetime as dt import xml.etree.ElementTree as ET from ..results import AddressLines, ReverseResult, ReverseResults, \ SearchResult, SearchResults from . import classtypes as cl +from ..types import EntranceDetails def _write_xml_address(root: ET.Element, address: AddressLines, @@ -83,6 +83,18 @@ def _create_base_entry(result: Union[ReverseResult, SearchResult], return place +def _create_entrance(root: ET.Element, entrance: EntranceDetails) -> None: + entrance_node = ET.SubElement(root, 'entrance', attrib={ + "osm_id": str(entrance.osm_id), + "type": entrance.type, + "lat": f"{entrance.lat:0.7f}", + "lon": f"{entrance.lon:0.7f}", + }) + if entrance.extratags: + for k, v in entrance.extratags.items(): + ET.SubElement(entrance_node, 'tag', attrib={'key': k, 'value': v}) + + def format_base_xml(results: Union[ReverseResults, SearchResults], options: Mapping[str, Any], simple: bool, xml_root_tag: str, @@ -126,7 +138,7 @@ def format_base_xml(results: Union[ReverseResults, SearchResults], if options.get('entrances', False): eroot = ET.SubElement(root if simple else place, 'entrances') if result.entrances: - for entrance_detail in result.entrances: - ET.SubElement(eroot, 'entrance', attrib=dataclasses.asdict(entrance_detail)) + for entrance in result.entrances: + _create_entrance(eroot, entrance) return '\n' + ET.tostring(root, encoding='unicode') diff --git a/src/nominatim_api/v1/server_glue.py b/src/nominatim_api/v1/server_glue.py index 760afc69..a3a29199 100644 --- a/src/nominatim_api/v1/server_glue.py +++ b/src/nominatim_api/v1/server_glue.py @@ -180,7 +180,9 @@ async def details_endpoint(api: NominatimAPIAsync, params: ASGIAdaptor) -> Any: result, fmt, {'locales': locales, 'group_hierarchy': params.get_bool('group_hierarchy', False), - 'icon_base_url': params.config().MAPICON_URL}) + 'icon_base_url': params.config().MAPICON_URL, + 'entrances': params.get_bool('entrances', False), + }) return build_response(params, output, num_results=1) diff --git a/test/bdd/features/db/import/entrances.feature b/test/bdd/features/db/import/entrances.feature new file mode 100644 index 00000000..c3ea30b2 --- /dev/null +++ b/test/bdd/features/db/import/entrances.feature @@ -0,0 +1,22 @@ +Feature: Entrance nodes are recorded + Test that imported entrance nodes are saved + + Scenario: A building with two entrances + Given the grid + | 1 | 2 | + | 4 | 3 | + Given the places + | osm | class | type | geometry | extratags | + | W1 | building | yes | (1,2,3,4,1) | | + | N10 | entrance | main | 1 | 'wheelchair': 'yes' | + | N20 | entrance | yes | 3 | | + And the ways + | id | nodes | + | 1 | 10,20,30,40 | + When importing + Then placex contains exactly + | object | place_id | + | W1 | 1 | + Then place_entrance contains exactly + | place_id | entrances | + | 1 | [{'lat': 0, 'lon': 0, 'type': 'main', 'osm_id': 10, 'extratags': {'wheelchair': 'yes'}}, {'lat': 1e-05, 'lon': 1e-05, 'type': 'yes', 'osm_id': 20, 'extratags': {}}] | diff --git a/test/bdd/features/db/update/entrances.feature b/test/bdd/features/db/update/entrances.feature new file mode 100644 index 00000000..4cabe2a5 --- /dev/null +++ b/test/bdd/features/db/update/entrances.feature @@ -0,0 +1,82 @@ +Feature: Entrance nodes are recorded + Test that updated entrance nodes are saved + + Scenario: A building with a newly tagged entrance + Given the grid + | 1 | 2 | + | 4 | 3 | + Given the places + | osm | class | type | geometry | + | W1 | building | yes | (1,2,3,4,1) | + And the ways + | id | nodes | + | 1 | 10,20,30,40 | + When importing + Then placex contains exactly + | object | place_id | + | W1 | 1 | + Then place_entrance contains exactly + | place_id | entrances | + When updating places + | osm | class | type | geometry | + | N10 | entrance | main | 1 | + Then placex contains exactly + | object | place_id | + | W1 | 1 | + And place_entrance contains exactly + | place_id | entrances | + | 1 | [{'lat': 0, 'lon': 0, 'type': 'main', 'osm_id': 10, 'extratags': None}] | + + Scenario: A building with a updated entrance node + Given the grid + | 1 | 2 | + | 4 | 3 | + Given the places + | osm | class | type | geometry | + | W1 | building | yes | (1,2,3,4,1) | + | N10 | barrier | gate | 1 | + And the ways + | id | nodes | + | 1 | 10,20,30,40 | + When importing + Then placex contains exactly + | object | place_id | + | W1 | 1 | + | N10 | 2 | + Then place_entrance contains exactly + | place_id | entrances | + When updating places + | osm | class | type | geometry | + | N10 | entrance | main | 1 | + Then placex contains exactly + | object | place_id | + | W1 | 1 | + | N10 | 2 | + And place_entrance contains exactly + | place_id | entrances | + | 1 | [{'lat': 0, 'lon': 0, 'type': 'main', 'osm_id': 10, 'extratags': None}] | + + Scenario: A building with a removed entrance + Given the grid + | 1 | 2 | + | 4 | 3 | + Given the places + | osm | class | type | geometry | + | W1 | building | yes | (1,2,3,4,1) | + | N10 | entrance | main | 1 | + And the ways + | id | nodes | + | 1 | 10,20,30,40 | + When importing + Then placex contains exactly + | object | place_id | + | W1 | 1 | + And place_entrance contains exactly + | place_id | entrances | + | 1 | [{'lat': 0, 'lon': 0, 'type': 'main', 'osm_id': 10, 'extratags': None}] | + When marking for delete N10 + Then placex contains exactly + | object | place_id | + | W1 | 1 | + And place_entrance contains exactly + | place_id | entrances | -- 2.39.5