bdd: switch to auto commit mode

Put the connection to the test database into auto-commit mode
and get rid of the explicit commits. Also use cursors always in
context managers and unify the two implementations that copy
data from the place table.

test/bdd/steps/nominatim_environment.py

@@ -179,6 +179,7 @@ class NominatimEnvironment:
         cur.execute('CREATE DATABASE {} TEMPLATE = {}'.format(self.test_db, self.template_db))
         conn.close()
         context.db = self.connect_database(self.test_db)
+        context.db.autocommit = True
         psycopg2.extras.register_hstore(context.db, globally=False)
 
     def teardown_db(self, context):
@@ -215,3 +216,25 @@ class NominatimEnvironment:
             cwd = self.build_dir
 
         run_script(cmd, cwd=cwd, env=self.test_env)
+
+    def copy_from_place(self, db):
+        """ Copy data from place to the placex and location_property_osmline
+            tables invoking the appropriate triggers.
+        """
+        self.run_setup_script('create-functions', 'create-partition-functions')
+
+        with db.cursor() as cur:
+            cur.execute("""INSERT INTO placex (osm_type, osm_id, class, type,
+                                               name, admin_level, address,
+                                               extratags, geometry)
+                             SELECT osm_type, osm_id, class, type,
+                                    name, admin_level, address,
+                                    extratags, geometry
+                               FROM place
+                               WHERE not (class='place' and type='houses' and osm_type='W')""")
+            cur.execute("""INSERT INTO location_property_osmline (osm_id, address, linegeo)
+                             SELECT osm_id, address, geometry
+                               FROM place
+                              WHERE class='place' and type='houses'
+                                    and osm_type='W'
+                                    and ST_GeometryType(geometry) = 'ST_LineString'""")

test/bdd/steps/steps_db_ops.py

@@ -218,7 +218,7 @@ def assert_db_column(row, column, value, context):
             "Row '%s': expected: %s, got: %s" % (column, value, str(row[column]))
 
 
-################################ STEPS ##################################
+################################ GIVEN ##################################
 
 @given(u'the scene (?P<scene>.+)')
 def set_default_scene(context, scene):
@@ -226,7 +226,7 @@ def set_default_scene(context, scene):
 
 @given("the (?P<named>named )?places")
 def add_data_to_place_table(context, named):
-    cur = context.db.cursor()
+    with context.db.cursor() as cur:
         cur.execute('ALTER TABLE place DISABLE TRIGGER place_before_insert')
         for r in context.table:
             col = PlaceColumn(context, named is not None)
@@ -236,12 +236,10 @@ def add_data_to_place_table(context, named):
 
             col.db_insert(cur)
         cur.execute('ALTER TABLE place ENABLE TRIGGER place_before_insert')
-    cur.close()
-    context.db.commit()
 
 @given("the relations")
 def add_data_to_planet_relations(context):
-    cur = context.db.cursor()
+    with context.db.cursor() as cur:
         for r in context.table:
             last_node = 0
             last_way = 0
@@ -272,11 +270,10 @@ def add_data_to_planet_relations(context):
             cur.execute("""INSERT INTO planet_osm_rels (id, way_off, rel_off, parts, members, tags)
                            VALUES (%s, %s, %s, %s, %s, %s)""",
                         (r['id'], last_node, last_way, parts, members, tags))
-    context.db.commit()
 
 @given("the ways")
 def add_data_to_planet_ways(context):
-    cur = context.db.cursor()
+    with context.db.cursor() as cur:
         for r in context.table:
             tags = []
             for h in r.headings:
@@ -287,22 +284,14 @@ def add_data_to_planet_ways(context):
 
             cur.execute("INSERT INTO planet_osm_ways (id, nodes, tags) VALUES (%s, %s, %s)",
                         (r['id'], nodes, tags))
-    context.db.commit()
+
+################################ WHEN ##################################
 
 @when("importing")
 def import_and_index_data_from_place_table(context):
-    context.nominatim.run_setup_script('create-functions', 'create-partition-functions')
+    """ Import data previously set up in the place table.
-    cur = context.db.cursor()
+    """
-    cur.execute(
+    context.nominatim.copy_from_place(context.db)
-        """insert into placex (osm_type, osm_id, class, type, name, admin_level, address, extratags, geometry)
-           select              osm_type, osm_id, class, type, name, admin_level, address, extratags, geometry
-           from place where not (class='place' and type='houses' and osm_type='W')""")
-    cur.execute(
-            """insert into location_property_osmline (osm_id, address, linegeo)
-             SELECT osm_id, address, geometry from place
-              WHERE class='place' and type='houses' and osm_type='W'
-                    and ST_GeometryType(geometry) = 'ST_LineString'""")
-    context.db.commit()
     context.nominatim.run_setup_script('calculate-postcodes', 'index', 'index-noanalyse')
     check_database_integrity(context)
 
@@ -310,7 +299,7 @@ def import_and_index_data_from_place_table(context):
 def update_place_table(context):
     context.nominatim.run_setup_script(
         'create-functions', 'create-partition-functions', 'enable-diff-updates')
-    cur = context.db.cursor()
+    with context.db.cursor() as cur:
         for r in context.table:
             col = PlaceColumn(context, False)
 
@@ -319,12 +308,9 @@ def update_place_table(context):
 
             col.db_insert(cur)
 
-    context.db.commit()
-
         while True:
             context.nominatim.run_update_script('index')
 
-        cur = context.db.cursor()
             cur.execute("SELECT 'a' FROM placex WHERE indexed_status != 0 LIMIT 1")
             if cur.rowcount == 0:
                 break
@@ -339,24 +325,24 @@ def update_postcodes(context):
 def delete_places(context, oids):
     context.nominatim.run_setup_script(
         'create-functions', 'create-partition-functions', 'enable-diff-updates')
-    cur = context.db.cursor()
+    with context.db.cursor() as cur:
         for oid in oids.split(','):
             where, params = NominatimID(oid).table_select()
             cur.execute("DELETE FROM place WHERE " + where, params)
-    context.db.commit()
 
     while True:
         context.nominatim.run_update_script('index')
 
-        cur = context.db.cursor()
+        with context.db.cursor() as cur:
             cur.execute("SELECT 'a' FROM placex WHERE indexed_status != 0 LIMIT 1")
             if cur.rowcount == 0:
                 break
 
+################################ THEN ##################################
+
 @then("placex contains(?P<exact> exactly)?")
 def check_placex_contents(context, exact):
-    cur = context.db.cursor(cursor_factory=psycopg2.extras.DictCursor)
+    with context.db.cursor(cursor_factory=psycopg2.extras.DictCursor) as cur:
-
         expected_content = set()
         for row in context.table:
             nid = NominatimID(row['object'])
@@ -399,12 +385,9 @@ def check_placex_contents(context, exact):
             cur.execute('SELECT osm_type, osm_id, class from placex')
             assert expected_content == set([(r[0], r[1], r[2]) for r in cur])
 
-    context.db.commit()
-
 @then("place contains(?P<exact> exactly)?")
 def check_placex_contents(context, exact):
-    cur = context.db.cursor(cursor_factory=psycopg2.extras.DictCursor)
+    with context.db.cursor(cursor_factory=psycopg2.extras.DictCursor) as cur:
-
         expected_content = set()
         for row in context.table:
             nid = NominatimID(row['object'])
@@ -445,12 +428,9 @@ def check_placex_contents(context, exact):
             cur.execute('SELECT osm_type, osm_id, class from place')
             assert expected_content, set([(r[0], r[1], r[2]) for r in cur])
 
-    context.db.commit()
-
 @then("search_name contains(?P<exclude> not)?")
 def check_search_name_contents(context, exclude):
-    cur = context.db.cursor(cursor_factory=psycopg2.extras.DictCursor)
+    with context.db.cursor(cursor_factory=psycopg2.extras.DictCursor) as cur:
-
         for row in context.table:
             pid = NominatimID(row['object']).get_place_id(cur)
             cur.execute("""SELECT *, ST_X(centroid) as cx, ST_Y(centroid) as cy
@@ -462,7 +442,7 @@ def check_search_name_contents(context, exclude):
                     if h in ('name_vector', 'nameaddress_vector'):
                         terms = [x.strip() for x in row[h].split(',') if not x.strip().startswith('#')]
                         words = [x.strip()[1:] for x in row[h].split(',') if x.strip().startswith('#')]
-                    subcur = context.db.cursor()
+                        with context.db.cursor() as subcur:
                             subcur.execute(""" SELECT word_id, word_token
                                                FROM word, (SELECT unnest(%s::TEXT[]) as term) t
                                                WHERE word_token = make_standard_name(t.term)
@@ -487,13 +467,9 @@ def check_search_name_contents(context, exclude):
                     else:
                         assert_db_column(res, h, row[h], context)
 
-
-    context.db.commit()
-
 @then("location_postcode contains exactly")
 def check_location_postcode(context):
-    cur = context.db.cursor(cursor_factory=psycopg2.extras.DictCursor)
+    with context.db.cursor(cursor_factory=psycopg2.extras.DictCursor) as cur:
-
         cur.execute("SELECT *, ST_AsText(geometry) as geomtxt FROM location_postcode")
         assert cur.rowcount == len(list(context.table)), \
             "Postcode table has %d rows, expected %d rows." % (cur.rowcount, len(list(context.table)))
@@ -510,8 +486,7 @@ def check_location_postcode(context):
 
 @then("word contains(?P<exclude> not)?")
 def check_word_table(context, exclude):
-    cur = context.db.cursor(cursor_factory=psycopg2.extras.DictCursor)
+    with context.db.cursor(cursor_factory=psycopg2.extras.DictCursor) as cur:
-
         for row in context.table:
             wheres = []
             values = []
@@ -526,8 +501,7 @@ def check_word_table(context, exclude):
 
 @then("place_addressline contains")
 def check_place_addressline(context):
-    cur = context.db.cursor(cursor_factory=psycopg2.extras.DictCursor)
+    with context.db.cursor(cursor_factory=psycopg2.extras.DictCursor) as cur:
-
         for row in context.table:
             pid = NominatimID(row['object']).get_place_id(cur)
             apid = NominatimID(row['address']).get_place_id(cur)
@@ -542,12 +516,9 @@ def check_place_addressline(context):
                     if h not in ('address', 'object'):
                         assert_db_column(res, h, row[h], context)
 
-    context.db.commit()
-
 @then("place_addressline doesn't contain")
 def check_place_addressline_exclude(context):
-    cur = context.db.cursor(cursor_factory=psycopg2.extras.DictCursor)
+    with context.db.cursor(cursor_factory=psycopg2.extras.DictCursor) as cur:
-
         for row in context.table:
             pid = NominatimID(row['object']).get_place_id(cur)
             apid = NominatimID(row['address']).get_place_id(cur)
@@ -557,15 +528,13 @@ def check_place_addressline_exclude(context):
             assert cur.rowcount == 0, \
                 "Row found for place %s and address %s" % (row['object'], row['address'])
 
-    context.db.commit()
-
 @then("(?P<oid>\w+) expands to(?P<neg> no)? interpolation")
 def check_location_property_osmline(context, oid, neg):
-    cur = context.db.cursor(cursor_factory=psycopg2.extras.DictCursor)
     nid = NominatimID(oid)
 
     assert 'W' == nid.typ, "interpolation must be a way"
 
+    with context.db.cursor(cursor_factory=psycopg2.extras.DictCursor) as cur:
         cur.execute("""SELECT *, ST_AsText(linegeo) as geomtxt
                        FROM location_property_osmline
                        WHERE osm_id = %s AND startnumber IS NOT NULL""",
@@ -599,17 +568,15 @@ def check_location_property_osmline(context, oid, neg):
 
 @then("(?P<table>placex|place) has no entry for (?P<oid>.*)")
 def check_placex_has_entry(context, table, oid):
-    cur = context.db.cursor(cursor_factory=psycopg2.extras.DictCursor)
+    with context.db.cursor(cursor_factory=psycopg2.extras.DictCursor) as cur:
         nid = NominatimID(oid)
         where, params = nid.table_select()
         cur.execute("SELECT * FROM %s where %s" % (table, where), params)
         assert cur.rowcount == 0
-    context.db.commit()
 
 @then("search_name has no entry for (?P<oid>.*)")
 def check_search_name_has_entry(context, oid):
-    cur = context.db.cursor(cursor_factory=psycopg2.extras.DictCursor)
+    with context.db.cursor(cursor_factory=psycopg2.extras.DictCursor) as cur:
         pid = NominatimID(oid).get_place_id(cur)
         cur.execute("SELECT * FROM search_name WHERE place_id = %s", (pid, ))
         assert cur.rowcount == 0
-    context.db.commit()

test/bdd/steps/steps_osm_data.py

@@ -70,17 +70,7 @@ def update_from_osm_file(context):
 
     The data is expected as attached text in OPL format.
     """
-    context.nominatim.run_setup_script('create-functions', 'create-partition-functions')
+    context.nominatim.copy_from_place(context.db)
-
-    cur = context.db.cursor()
-    cur.execute("""insert into placex (osm_type, osm_id, class, type, name, admin_level, address, extratags, geometry)
-                     select            osm_type, osm_id, class, type, name, admin_level, address, extratags, geometry from place""")
-    cur.execute(
-        """insert into location_property_osmline (osm_id, address, linegeo)
-             SELECT osm_id, address, geometry from place
-              WHERE class='place' and type='houses' and osm_type='W'
-                    and ST_GeometryType(geometry) = 'ST_LineString'""")
-    context.db.commit()
     context.nominatim.run_setup_script('index', 'index-noanalyse')
     context.nominatim.run_setup_script('create-functions', 'create-partition-functions',
                                        'enable-diff-updates')