added subcommand to clean deleted relations for issue # 2444

diff --git a/nominatim/clicmd/admin.py b/nominatim/clicmd/admin.py
index 0f498ad220775a3f7f1890b4d217f8e74fa29259..debbd8d3f44efd9d18aa4360b42c8aebe8eaf4de 100644 (file)
--- a/nominatim/clicmd/admin.py
+++ b/nominatim/clicmd/admin.py
@@ -41,6 +41,8 @@ class AdminFuncs:
                           help='Print performance analysis of the indexing process')
         objs.add_argument('--collect-os-info', action="store_true",
                           help="Generate a report about the host system information")
                           help='Print performance analysis of the indexing process')
         objs.add_argument('--collect-os-info', action="store_true",
                           help="Generate a report about the host system information")

+        objs.add_argument('--clean-deleted', action='store_true',
+                          help='Clean up deleted relations')

         group = parser.add_argument_group('Arguments for cache warming')
         group.add_argument('--search-only', action='store_const', dest='target',
                            const='search',
         group = parser.add_argument_group('Arguments for cache warming')
         group.add_argument('--search-only', action='store_const', dest='target',
                            const='search',


@@ -54,8 +56,13 @@ class AdminFuncs:
                             help='Analyse indexing of the given OSM object')
         mgroup.add_argument('--place-id', type=int,
                             help='Analyse indexing of the given Nominatim object')
                             help='Analyse indexing of the given OSM object')
         mgroup.add_argument('--place-id', type=int,
                             help='Analyse indexing of the given Nominatim object')

+        group = parser.add_argument_group('Arguments for cleaning deleted')
+        group.add_argument('--age', type=str,
+                           help='Delete relations older than the given PostgreSQL time interval')
+

 
     def run(self, args: NominatimArgs) -> int:
 
     def run(self, args: NominatimArgs) -> int:

+        # pylint: disable=too-many-return-statements

         if args.warm:
             return self._warm(args)
 
         if args.warm:
             return self._warm(args)
 


@@ -81,6 +88,12 @@ class AdminFuncs:
             collect_os_info.report_system_information(args.config)
             return 0
 
             collect_os_info.report_system_information(args.config)
             return 0
 

+        if args.clean_deleted:
+            LOG.warning('Cleaning up deleted relations')
+            from ..tools import admin
+            admin.clean_deleted_relations(args.config, age=args.age)
+            return 0
+

         return 1
 
 
         return 1
 
 

diff --git a/nominatim/clicmd/args.py b/nominatim/clicmd/args.py
index 8b805496fffd750bec95f9e25303e17c5eca6305..5b4beb40ec290e202bda760798072cad07cb7211 100644 (file)
--- a/nominatim/clicmd/args.py
+++ b/nominatim/clicmd/args.py
@@ -72,10 +72,12 @@ class NominatimArgs:
     check_database: bool
     migrate: bool
     collect_os_info: bool
     check_database: bool
     migrate: bool
     collect_os_info: bool

+    clean_deleted: bool

     analyse_indexing: bool
     target: Optional[str]
     osm_id: Optional[str]
     place_id: Optional[int]
     analyse_indexing: bool
     target: Optional[str]
     osm_id: Optional[str]
     place_id: Optional[int]

+    age: str

 
     # Arguments to 'import'
     osm_file: List[str]
 
     # Arguments to 'import'
     osm_file: List[str]

diff --git a/nominatim/tools/admin.py b/nominatim/tools/admin.py
index da7845ebc949696145588b393664d4be2077eb37..f27d9bbccca8504c39b75de2e0c92656fad439d0 100644 (file)
--- a/nominatim/tools/admin.py
+++ b/nominatim/tools/admin.py
@@ -11,6 +11,7 @@ from typing import Optional, Tuple, Any, cast
 import logging
 
 from psycopg2.extras import Json, register_hstore
 import logging
 
 from psycopg2.extras import Json, register_hstore

+from psycopg2 import DataError

 
 from nominatim.config import Configuration
 from nominatim.db.connection import connect, Cursor
 
 from nominatim.config import Configuration
 from nominatim.db.connection import connect, Cursor


@@ -87,3 +88,23 @@ def analyse_indexing(config: Configuration, osm_id: Optional[str] = None,
 
         for msg in conn.notices:
             print(msg)
 
         for msg in conn.notices:
             print(msg)

+
+
+def clean_deleted_relations(config: Configuration, age: Optional[str] = None) -> None:
+    """ Clean deleted relations older than a given age
+    """
+    if not age:
+        LOG.fatal('No age given to delete relations')
+        raise UsageError('Age parameter not found')
+    with connect(config.get_libpq_dsn()) as conn:
+        with conn.cursor() as cur:
+            try:
+                cur.execute("""SELECT place_force_delete(p.place_id)
+                            FROM import_polygon_delete d, placex p
+                            WHERE p.osm_type = d.osm_type AND p.osm_id = d.osm_id
+                            AND age(p.indexed_date) > %s::interval""",
+                            (age, ))
+            except DataError as exc:
+                raise UsageError('Invalid PostgreSQL time interval format') from exc
+        conn.commit()
+            
\ No newline at end of file

diff --git a/test/python/cli/test_cmd_admin.py b/test/python/cli/test_cmd_admin.py
index 75ae3cd2a3bc9921b56b3ee0f950382874c6ec61..20e5be00f9257feb72fb4998850d9f0710d8a038 100644 (file)
--- a/test/python/cli/test_cmd_admin.py
+++ b/test/python/cli/test_cmd_admin.py
@@ -33,6 +33,12 @@ def test_admin_migrate(cli_call, mock_func_factory):
     assert mock.called == 1
 
 
     assert mock.called == 1
 
 

+def test_admin_clean_deleted_relations(cli_call, mock_func_factory):
+    mock = mock_func_factory(nominatim.tools.admin, 'clean_deleted_relations')
+
+    assert cli_call('admin', '--clean-deleted') == 0
+    assert mock.called == 1
+

 class TestCliAdminWithDb:
 
     @pytest.fixture(autouse=True)
 class TestCliAdminWithDb:
 
     @pytest.fixture(autouse=True)

diff --git a/test/python/tools/test_admin.py b/test/python/tools/test_admin.py
index 9c010b9d4b77e00bc784b305eb2c1cfceab3b2be..dd96c943082d5f808d38ed66eb88757ee10a3031 100644 (file)
--- a/test/python/tools/test_admin.py
+++ b/test/python/tools/test_admin.py
@@ -70,3 +70,94 @@ def test_analyse_indexing_with_osm_id(project_env, temp_db_cursor):
                               VALUES(9988, 'N', 10000)""")
 
     admin.analyse_indexing(project_env, osm_id='N10000')
                               VALUES(9988, 'N', 10000)""")
 
     admin.analyse_indexing(project_env, osm_id='N10000')

+
+
+class TestAdminCleanDeleted:
+
+    @pytest.fixture(autouse=True)
+    def setup_polygon_delete(self, project_env, table_factory, temp_db_cursor):
+        """ Set up import_polygon_delete table and simplified place_force_delete function
+        """
+        self.project_env = project_env
+        self.temp_db_cursor = temp_db_cursor
+        table_factory('import_polygon_delete',
+                      """osm_id BIGINT,
+                      osm_type CHAR(1),
+                      class TEXT NOT NULL,
+                      type TEXT NOT NULL""",
+                      ((100, 'N', 'boundary', 'administrative'),
+                      (145, 'N', 'boundary', 'administrative'),
+                      (175, 'R', 'landcover', 'grass')))
+        table_factory('place_to_be_deleted',
+                      """osm_id BIGINT,
+                      osm_type CHAR(1),
+                      class TEXT NOT NULL,
+                      type TEXT NOT NULL,
+                      deferred BOOLEAN""")
+        temp_db_cursor.execute("""INSERT INTO placex (place_id, osm_id, osm_type, class, type, indexed_date, indexed_status)
+                              VALUES(1, 100, 'N', 'boundary', 'administrative', current_date - INTERVAL '1 month', 1),
+                               (2, 145, 'N', 'boundary', 'administrative', current_date - INTERVAL '1 month', 1),
+                               (3, 175, 'R', 'landcover', 'grass', current_date - INTERVAL '1 month', 1)""")
+        temp_db_cursor.execute("""CREATE OR REPLACE FUNCTION flush_deleted_places()
+                               RETURNS INTEGER
+                               AS $$
+                               BEGIN
+                                UPDATE placex p SET indexed_status = 100 FROM place_to_be_deleted d
+                                WHERE p.osm_type = d.osm_type
+                                AND p.osm_id = d.osm_id
+                                AND p.class = d.class
+                                AND p.type = d.type
+                                AND NOT deferred;
+                               TRUNCATE TABLE place_to_be_deleted;
+                                RETURN NULL;
+                               END;
+                               $$
+                               LANGUAGE plpgsql;
+                               """)
+        temp_db_cursor.execute("""CREATE OR REPLACE FUNCTION place_force_delete(placeid BIGINT)
+                               RETURNS BOOLEAN
+                               AS $$
+                               DECLARE 
+                                osmid BIGINT;
+                                osmtype character(1);
+                                pclass text;
+                                ptype text;
+                               BEGIN
+                                SELECT osm_type, osm_id, class, type FROM placex WHERE place_id = placeid INTO osmtype, osmid, pclass, ptype;
+                                DELETE FROM import_polygon_delete WHERE osm_type = osmtype AND osm_id = osmid AND class = pclass AND type = ptype;
+                                INSERT INTO place_to_be_deleted (osm_type, osm_id, class, type, deferred)
+                                        VALUES(osmtype, osmid, pclass, ptype, false);
+                                PERFORM flush_deleted_places();
+                                RETURN TRUE;
+                               END;
+                               $$
+                               LANGUAGE plpgsql;
+                               """)
+        
+
+    def test_admin_clean_deleted_no_records(self):
+        admin.clean_deleted_relations(self.project_env, age='1 year')
+        assert self.temp_db_cursor.row_set('SELECT osm_id, osm_type, class, type, indexed_status FROM placex') == {(100, 'N', 'boundary', 'administrative', 1),
+                                                                                                                   (145, 'N', 'boundary', 'administrative', 1),
+                                                                                                                   (175, 'R', 'landcover', 'grass', 1)}
+        assert self.temp_db_cursor.table_rows('import_polygon_delete') == 3
+
+
+    def test_admin_clean_deleted_no_age(self):
+        with pytest.raises(UsageError):
+            admin.clean_deleted_relations(self.project_env)
+
+
+    @pytest.mark.parametrize('test_age', ['T week', '1 welk', 'P1E'])
+    def test_admin_clean_deleted_bad_age(self, test_age):
+        with pytest.raises(UsageError):
+            admin.clean_deleted_relations(self.project_env, age = test_age)
+
+
+    @pytest.mark.parametrize('test_age', ['1 week', 'P3D', '5 hours'])
+    def test_admin_clean_deleted(self, test_age):
+        admin.clean_deleted_relations(self.project_env, age = test_age)
+        assert self.temp_db_cursor.row_set('SELECT osm_id, osm_type, class, type, indexed_status FROM placex') == {(100, 'N', 'boundary', 'administrative', 100),
+                                                                                                                   (145, 'N', 'boundary', 'administrative', 100),
+                                                                                                                   (175, 'R', 'landcover', 'grass', 100)}
+        assert self.temp_db_cursor.table_rows('import_polygon_delete') == 0