From a52ed366e42b3309463b807cd25ad08cce8f0bfe Mon Sep 17 00:00:00 2001 From: Sarah Hoffmann Date: Wed, 1 Dec 2021 20:27:40 +0100 Subject: [PATCH] add tests for migration --- nominatim/tools/migration.py | 5 +- test/python/cursor.py | 9 ++ test/python/mocks.py | 13 +- test/python/tools/test_migration.py | 237 ++++++++++++++++++++++++++++ 4 files changed, 259 insertions(+), 5 deletions(-) create mode 100644 test/python/tools/test_migration.py diff --git a/nominatim/tools/migration.py b/nominatim/tools/migration.py index 87febacc..bcf8f142 100644 --- a/nominatim/tools/migration.py +++ b/nominatim/tools/migration.py @@ -26,7 +26,7 @@ def migrate(config, paths): if db_version_str is not None: parts = db_version_str.split('.') - db_version = tuple([int(x) for x in parts[:2] + parts[2].split('-')]) + db_version = tuple(int(x) for x in parts[:2] + parts[2].split('-')) if db_version == NOMINATIM_VERSION: LOG.warning("Database already at latest version (%s)", db_version_str) @@ -96,6 +96,7 @@ def _migration(major, minor, patch=0, dbpatch=0): """ def decorator(func): _MIGRATION_FUNCTIONS.append(((major, minor, patch, dbpatch), func)) + return func return decorator @@ -195,7 +196,7 @@ def install_legacy_tokenizer(conn, config, **_): @_migration(4, 0, 99, 0) -def create_tiger_housenumber_index(conn, _, **_): +def create_tiger_housenumber_index(conn, **_): """ Create idx_location_property_tiger_parent_place_id with included house number. diff --git a/test/python/cursor.py b/test/python/cursor.py index 46069020..620cdd98 100644 --- a/test/python/cursor.py +++ b/test/python/cursor.py @@ -37,6 +37,15 @@ class CursorForTesting(psycopg2.extras.DictCursor): return num == 1 + def index_exists(self, table, index): + """ Check that an indexwith the given name exists on the given table. + """ + num = self.scalar("""SELECT count(*) FROM pg_indexes + WHERE tablename = %s and indexname = %s""", + (table, index)) + return num == 1 + + def table_rows(self, table, where=None): """ Return the number of rows in the given table. """ diff --git a/test/python/mocks.py b/test/python/mocks.py index 17b76052..d01d5fc2 100644 --- a/test/python/mocks.py +++ b/test/python/mocks.py @@ -47,15 +47,16 @@ class MockPlacexTable: def add(self, osm_type='N', osm_id=None, cls='amenity', typ='cafe', names=None, admin_level=None, address=None, extratags=None, geom='POINT(10 4)', - country=None): + country=None, housenumber=None): with self.conn.cursor() as cur: psycopg2.extras.register_hstore(cur) cur.execute("""INSERT INTO placex (place_id, osm_type, osm_id, class, type, name, admin_level, address, + housenumber, extratags, geometry, country_code) - VALUES(nextval('seq_place'), %s, %s, %s, %s, %s, %s, %s, %s, %s, %s)""", + VALUES(nextval('seq_place'), %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s)""", (osm_type, osm_id or next(self.idseq), cls, typ, names, - admin_level, address, extratags, 'SRID=4326;' + geom, + admin_level, address, housenumber, extratags, 'SRID=4326;' + geom, country)) self.conn.commit() @@ -71,3 +72,9 @@ class MockPropertyTable: """ Set a property in the table to the given value. """ properties.set_property(self.conn, name, value) + + + def get(self, name): + """ Set a property in the table to the given value. + """ + return properties.get_property(self.conn, name) diff --git a/test/python/tools/test_migration.py b/test/python/tools/test_migration.py new file mode 100644 index 00000000..79ec8a0e --- /dev/null +++ b/test/python/tools/test_migration.py @@ -0,0 +1,237 @@ +""" +Tests for migration functions +""" +import pytest +import psycopg2.extras + +from nominatim.tools import migration +from nominatim.errors import UsageError +import nominatim.version + +class DummyTokenizer: + + def update_sql_functions(self, config): + pass + + +@pytest.fixture +def postprocess_mock(monkeypatch): + monkeypatch.setattr(migration.refresh, 'create_functions', lambda *args: args) + monkeypatch.setattr(migration.tokenizer_factory, 'get_tokenizer_for_db', + lambda *args: DummyTokenizer()) + + +def test_no_migration_old_versions(temp_db_with_extensions, table_factory, def_config): + table_factory('country_name', 'name HSTORE, country_code TEXT') + + with pytest.raises(UsageError, match='Migration not possible'): + migration.migrate(def_config, {}) + + +def test_set_up_migration_for_36(temp_db_with_extensions, temp_db_cursor, + table_factory, def_config, monkeypatch, + postprocess_mock): + psycopg2.extras.register_hstore(temp_db_cursor) + # don't actually run any migration, except the property table creation + monkeypatch.setattr(migration, '_MIGRATION_FUNCTIONS', + [((3, 5, 0, 99), migration.add_nominatim_property_table)]) + # Use a r/o user name that always exists + monkeypatch.setenv('NOMINATIM_DATABASE_WEBUSER', 'postgres') + + table_factory('country_name', 'name HSTORE, country_code TEXT', + (({str(x): 'a' for x in range(200)}, 'gb'),)) + + assert not temp_db_cursor.table_exists('nominatim_properties') + + assert migration.migrate(def_config, {}) == 0 + + assert temp_db_cursor.table_exists('nominatim_properties') + + assert 1 == temp_db_cursor.scalar(""" SELECT count(*) FROM nominatim_properties + WHERE property = 'database_version'""") + + +def test_already_at_version(def_config, property_table): + + property_table.set('database_version', + '{0[0]}.{0[1]}.{0[2]}-{0[3]}'.format(nominatim.version.NOMINATIM_VERSION)) + + assert migration.migrate(def_config, {}) == 0 + + +def test_no_migrations_necessary(def_config, temp_db_cursor, property_table, + monkeypatch): + oldversion = [x for x in nominatim.version.NOMINATIM_VERSION] + oldversion[0] -= 1 + property_table.set('database_version', + '{0[0]}.{0[1]}.{0[2]}-{0[3]}'.format(oldversion)) + + oldversion[0] = 0 + monkeypatch.setattr(migration, '_MIGRATION_FUNCTIONS', + [(tuple(oldversion), lambda **attr: True)]) + + assert migration.migrate(def_config, {}) == 0 + + +def test_run_single_migration(def_config, temp_db_cursor, property_table, + monkeypatch, postprocess_mock): + oldversion = [x for x in nominatim.version.NOMINATIM_VERSION] + oldversion[0] -= 1 + property_table.set('database_version', + '{0[0]}.{0[1]}.{0[2]}-{0[3]}'.format(oldversion)) + + done = {'old': False, 'new': False} + def _migration(**_): + """ Dummy migration""" + done['new'] = True + + def _old_migration(**_): + """ Dummy migration""" + done['old'] = True + + oldversion[0] = 0 + monkeypatch.setattr(migration, '_MIGRATION_FUNCTIONS', + [(tuple(oldversion), _old_migration), + (nominatim.version.NOMINATIM_VERSION, _migration)]) + + assert migration.migrate(def_config, {}) == 0 + + assert done['new'] + assert not done['old'] + assert property_table.get('database_version') == \ + '{0[0]}.{0[1]}.{0[2]}-{0[3]}'.format(nominatim.version.NOMINATIM_VERSION) + + +###### Tests for specific migrations +# +# Each migration should come with two tests: +# 1. Test that migration from old to new state works as expected. +# 2. Test that the migration can be rerun on the new state without side effects. + + +@pytest.mark.parametrize('in_attr', ('', 'with time zone')) +def test_import_status_timestamp_change(temp_db_conn, temp_db_cursor, + table_factory, in_attr): + table_factory('import_status', + f"""lastimportdate timestamp {in_attr}, + sequence_id integer, + indexed boolean""") + + migration.import_status_timestamp_change(temp_db_conn) + temp_db_conn.commit() + + assert temp_db_cursor.scalar("""SELECT data_type FROM information_schema.columns + WHERE table_name = 'import_status' + and column_name = 'lastimportdate'""")\ + == 'timestamp with time zone' + + +def test_add_nominatim_property_table(temp_db_conn, temp_db_cursor, + def_config, monkeypatch): + # Use a r/o user name that always exists + monkeypatch.setenv('NOMINATIM_DATABASE_WEBUSER', 'postgres') + + assert not temp_db_cursor.table_exists('nominatim_properties') + + migration.add_nominatim_property_table(temp_db_conn, def_config) + temp_db_conn.commit() + + assert temp_db_cursor.table_exists('nominatim_properties') + + +def test_add_nominatim_property_table_repeat(temp_db_conn, temp_db_cursor, + def_config, property_table): + assert temp_db_cursor.table_exists('nominatim_properties') + + migration.add_nominatim_property_table(temp_db_conn, def_config) + temp_db_conn.commit() + + assert temp_db_cursor.table_exists('nominatim_properties') + + +def test_change_housenumber_transliteration(temp_db_conn, temp_db_cursor, + word_table, placex_table): + placex_table.add(housenumber='3A') + + temp_db_cursor.execute("""CREATE OR REPLACE FUNCTION make_standard_name(name TEXT) + RETURNS TEXT AS $$ SELECT lower(name) $$ LANGUAGE SQL """) + temp_db_cursor.execute("""CREATE OR REPLACE FUNCTION getorcreate_housenumber_id(lookup_word TEXT) + RETURNS INTEGER AS $$ SELECT 4325 $$ LANGUAGE SQL """) + + migration.change_housenumber_transliteration(temp_db_conn) + temp_db_conn.commit() + + assert temp_db_cursor.scalar('SELECT housenumber from placex') == '3a' + + migration.change_housenumber_transliteration(temp_db_conn) + temp_db_conn.commit() + + assert temp_db_cursor.scalar('SELECT housenumber from placex') == '3a' + + +def test_switch_placenode_geometry_index(temp_db_conn, temp_db_cursor, placex_table): + temp_db_cursor.execute("""CREATE INDEX idx_placex_adminname + ON placex (place_id)""") + + migration.switch_placenode_geometry_index(temp_db_conn) + temp_db_conn.commit() + + assert temp_db_cursor.index_exists('placex', 'idx_placex_geometry_placenode') + assert not temp_db_cursor.index_exists('placex', 'idx_placex_adminname') + + +def test_switch_placenode_geometry_index_repeat(temp_db_conn, temp_db_cursor, placex_table): + temp_db_cursor.execute("""CREATE INDEX idx_placex_geometry_placenode + ON placex (place_id)""") + + migration.switch_placenode_geometry_index(temp_db_conn) + temp_db_conn.commit() + + assert temp_db_cursor.index_exists('placex', 'idx_placex_geometry_placenode') + assert not temp_db_cursor.index_exists('placex', 'idx_placex_adminname') + assert temp_db_cursor.scalar("""SELECT indexdef from pg_indexes + WHERE tablename = 'placex' + and indexname = 'idx_placex_geometry_placenode' + """).endswith('(place_id)') + + +def test_install_legacy_tokenizer(temp_db_conn, temp_db_cursor, project_env, + property_table, table_factory, monkeypatch, + tmp_path): + table_factory('placex', 'place_id BIGINT') + table_factory('location_property_osmline', 'place_id BIGINT') + + # Setting up the tokenizer is problematic + class MiniTokenizer: + def migrate_database(self, config): + pass + + monkeypatch.setattr(migration.tokenizer_factory, 'create_tokenizer', + lambda cfg, **kwargs: MiniTokenizer()) + + migration.install_legacy_tokenizer(temp_db_conn, project_env) + temp_db_conn.commit() + + + +def test_install_legacy_tokenizer_repeat(temp_db_conn, temp_db_cursor, + def_config, property_table): + + property_table.set('tokenizer', 'dummy') + migration.install_legacy_tokenizer(temp_db_conn, def_config) + temp_db_conn.commit() + + +def test_create_tiger_housenumber_index(temp_db_conn, temp_db_cursor, table_factory): + table_factory('location_property_tiger', + 'parent_place_id BIGINT, startnumber INT, endnumber INT') + + migration.create_tiger_housenumber_index(temp_db_conn) + temp_db_conn.commit() + + if temp_db_conn.server_version_tuple() >= (11, 0, 0): + assert temp_db_cursor.index_exists('location_property_tiger', + 'idx_location_property_tiger_housenumber_migrated') + + migration.create_tiger_housenumber_index(temp_db_conn) + temp_db_conn.commit() -- 2.39.5