Merge pull request #120 from rustprooflabs/fix-bench-119

Rework and consolidate logic for amenity inclusion

Author: rustprooflabs

db/qc/osm_tag_suggestions.sql

@@ -1,10 +1,20 @@
 
 
 SELECT geom_type, osm_id,
-        'Update to: amenity=bicycle_parking per https://wiki.openstreetmap.org/wiki/Key:bicycle_parking'::TEXT AS suggestion,
+        'Update to: amenity=bicycle_parking per https://wiki.openstreetmap.org/wiki/Key:bicycle_parking'::TEXT
+            AS suggestion,
         osm_url, tags
     FROM osm.tags
     WHERE tags->>'bicycle_parking' IS NOT NULL
         AND tags->>'amenity' IS NULL
+UNION
+SELECT t.geom_type, t.osm_id,
+        'Invald bench value. Valid values for `bench` are "yes" and "no" per https://wiki.openstreetmap.org/wiki/Key:bench'::TEXT
+            AS suggestion,
+        t.osm_url, t.tags
+    FROM osm.tags t
+    WHERE t.tags->>'amenity' IS NULL
+        AND t.tags->>'bench' IS NOT NULL
+        AND t.tags->>'bench' NOT IN ('yes', 'no')
 ;
 

flex-config/style/amenity.lua

@@ -64,22 +64,31 @@ local amenity_first_level_keys = {
 
 local is_first_level_amenity = make_check_in_list_func(amenity_first_level_keys)
 
-function amenity_process_node(object)
-    if not is_first_level_amenity(object.tags) then
-        return
-    end
-
-    if object.tags.bench == 'no' then
-        return
-    end
 
+local function get_osm_type(object)
     local osm_type = object.tags.amenity
+
     if osm_type == nil and object.tags.bench == 'yes' then
         osm_type = 'bench'
     elseif osm_type == nil and object.tags.brewery then
         osm_type = 'brewery'
     end
 
+    return osm_type
+end
+
+
+function amenity_process_node(object)
+    if not is_first_level_amenity(object.tags) then
+        return
+    end
+
+    local osm_type = get_osm_type(object)
+
+    if osm_type == nil then
+        return
+    end
+
     local name = get_name(object.tags)
 
 
@@ -111,15 +120,10 @@ function amenity_process_way(object)
         return
     end
 
-    if object.tags.bench == 'no' then
-        return
-    end
+    local osm_type = get_osm_type(object)
 
-    local osm_type = object.tags.amenity
-    if osm_type == nil and object.tags.bench == 'yes' then
-        osm_type = 'bench'
-    elseif osm_type == nil and object.tags.brewery then
-        osm_type = 'brewery'
+    if osm_type == nil then
+        return
     end
 
     local name = get_name(object.tags)
@@ -166,18 +170,12 @@ function amenity_process_relation(object)
         return
     end
 
-    if object.tags.bench == 'no' then
-        return
-    end
+    local osm_type = get_osm_type(object)
 
-    local osm_type = object.tags.amenity
-    if osm_type == nil and object.tags.bench == 'yes' then
-        osm_type = 'bench'
-    elseif osm_type == nil and object.tags.brewery then
-        osm_type = 'brewery'
+    if osm_type == nil then
+        return
     end
 
-
     local name = get_name(object.tags)
 
     local address = get_address(object.tags)

tests/README.md

@@ -3,29 +3,48 @@
 
 Under development - See #112.
 
-## Data
 
-Load `data/district-of-columbia-2021-01-13.osm.pbf` for all tests.
+```bash
+cd tests/
+```
+
 
-PBF sourced [from Geofabrik's download service](https://download.geofabrik.de/)
-on January 13, 2021.
+## Test for import failures
 
+Test for specific regions that have had failures due to unusual
+tags and/or bugs in PgOSM-Flex.
 
-Run tests
+
+Run extra region load tests.
 
 ```bash
-cd tests/
-./run-tests.sh
+./run-extra-loads.sh
+```
+
+## Output Tests
+
+Load `data/district-of-columbia-2021-01-13.osm.pbf` with `run-all`
+before running these tests.
+
+
+Run output tests (need D.C. region loaded first).
+
+```bash
+./run-output-tests.sh
 ```
 
 
-## Creating Tests
+> PBF sourced [from Geofabrik's download service](https://download.geofabrik.de/) on January 13, 2021.
+
+
+
+### Creating Tests
 
 Write query. Ensure results are ordered using `COLLATE "C"` to ensure consistent ordering across
 systems.
 
 
-## Creating expected output
+### Creating expected output
 
 
 To create new tests, or to update existing tests use `psql --no-psqlrc -tA <details>`.
@@ -38,3 +57,23 @@ psql --no-psqlrc -tA  \
      > expected/amenity_osm_type_count.out
 ```
 
+
+
+## Create PBFs for areas w/ Failures
+
+Identify a feature related to the issue and load small region around
+into JOSM (as if making an edit).
+
+Use JOSM's "Save As..." to save the `<region-failure-name>.osm` file.
+Use `osmium-tool` (https://osmcode.org/osmium-tool/manual.html)
+to convert to `.pbf` format.
+
+```bash
+osmium cat  <region-failure-name>.osm -o <region-failure-name>.osm.pbf
+mv <region-failure-name>.osm.pbf ~/git/pgosm-flex/tests/data/extra-regions/
+``` 
+
+
+
+
+

tests/data/extra-regions/osm-test-bench-tags.osm.pbf

@@ -0,0 +1,43 @@
+#!/bin/bash
+echo "Running PgOSM-Flex extra region loads"
+
+set -e
+
+if [ -z $PGOSM_CONN ]; then
+  PGOSM_CONN=pgosm_tests
+else
+  PGOSM_CONN=$PGOSM_CONN
+fi
+
+if [ -z $PGOSM_CONN_PG ]; then
+  PGOSM_CONN_PG=postgres
+else
+  PGOSM_CONN_PG=$PGOSM_CONN_PG
+fi
+
+DATA_PATH=data/extra-regions
+
+for filename in ${DATA_PATH}/*.osm.pbf; do
+    #file_base=$(basename "${filename}" .osm.pbf)
+    #echo $file_base
+    echo $filename
+
+    echo 'Dropping test DB pgosm_tests'
+    psql -d $PGOSM_CONN_PG -c "DROP DATABASE pgosm_tests;" || true
+
+    echo 'Creating test DB pgosm_tests'
+    psql -d $PGOSM_CONN_PG -c "CREATE DATABASE pgosm_tests;"
+    psql -d $PGOSM_CONN -c "CREATE EXTENSION postgis; CREATE SCHEMA osm;"
+
+    original_dir=$PWD
+    echo $original_dir
+
+    cd ../flex-config
+    osm2pgsql --slim --drop -d ${PGOSM_CONN} \
+        --output=flex --style=run-all.lua \
+        ../tests/${filename}
+
+    cd $original_dir
+
+done
+