mapping: fix (way)zorder for non-int32 layer values

omniscale · Jan 18, 2021 · 53bb807 · 53bb807
1 parent 4758cf4
commit 53bb807
Show file tree

Hide file tree

Showing 6 changed files with 55 additions and 7 deletions.
diff --git a/mapping/columns.go b/mapping/columns.go
@@ -217,7 +217,8 @@ func MakeWayZOrder(columnName string, columnType ColumnType, column config.Colum
 
 	wayZOrder := func(val string, elem *osm.Element, geom *geom.Geometry, match Match) interface{} {
 		var z int
-		layer, _ := strconv.ParseInt(elem.Tags["layer"], 10, 64)
+		layer, _ := strconv.ParseInt(elem.Tags["layer"], 10, 32)
+
 		z += int(layer) * levelOffset
 
 		rank, ok := ranks[match.Value]
@@ -236,6 +237,9 @@ func MakeWayZOrder(columnName string, columnType ColumnType, column config.Colum
 			z += levelOffset
 		}
 
+		if z < math.MinInt32 || z > math.MaxInt32 {
+			return nil
+		}
 		return z
 	}
 	return wayZOrder, nil
@@ -285,6 +289,9 @@ func DefaultWayZOrder(val string, elem *osm.Element, geom *geom.Geometry, match
 		z += 10
 	}
 
+	if z < math.MinInt32 || z > math.MaxInt32 {
+		return nil
+	}
 	return z
 }
 

diff --git a/mapping/columns_test.go b/mapping/columns_test.go
@@ -208,6 +208,7 @@ func TestWayZOrder(t *testing.T) {
 		t.Fatal(err)
 	}
 
+	NIL := 999 // marker value
 	tests := []struct {
 		key      string
 		tags     osm.Tags
@@ -223,12 +224,18 @@ func TestWayZOrder(t *testing.T) {
 		{"path", osm.Tags{"tunnel": "yes"}, -10},
 		{"unknown", osm.Tags{"tunnel": "yes"}, -6},
 		{"unknown", osm.Tags{"tunnel": "yes", "layer": "1"}, 5},
+		{"unknown", osm.Tags{"tunnel": "yes", "layer": "123456789123456789"}, NIL},
 	}
 	for _, test := range tests {
 		elem := &osm.Element{Tags: test.tags}
 		match := Match{Value: test.key}
 
-		if v := zOrder("", elem, nil, match); v.(int) != test.expected {
+		if test.expected == NIL {
+			if v := zOrder("", elem, nil, match); v != nil {
+				t.Errorf("%v %v %#v != nil", test.key, test.tags, v)
+			}
+
+		} else if v := zOrder("", elem, nil, match); v.(int) != test.expected {
 			t.Errorf("%v %v %d != %d", test.key, test.tags, v, test.expected)
 		}
 	}

diff --git a/test/complete_db.osc b/test/complete_db.osc
@@ -96,6 +96,17 @@
     </relation>
   </modify>
 
+  <modify>
+    <way id="17003" version="2" timestamp="2011-11-11T00:11:11Z">
+      <nd ref="17001"/>
+      <nd ref="17002"/>
+      <nd ref="17003"/>
+      <tag k="name" v="way 17003"/>
+      <tag k="layer" v="2"/>
+      <tag k="highway" v="residential"/>
+    </way>
+  </modify>
+
   <!-- test merge relation way -->
   <delete>
     <way id="16002" version="2" timestamp="2011-11-11T00:11:11Z"/>

diff --git a/test/complete_db.osm b/test/complete_db.osm
@@ -743,6 +743,16 @@
     <tag k="landuse" v="park"/>
   </relation>
 
+  <!-- way with invalid layer -->
+  <way id="17003" version="1" timestamp="2011-11-11T00:11:11Z">
+    <nd ref="17001"/>
+    <nd ref="17002"/>
+    <nd ref="17003"/>
+    <tag k="name" v="way 17003"/>
+    <tag k="layer" v="11111111111111111"/>
+    <tag k="highway" v="residential"/>
+  </way>
+
   <way id="17101" version="1" timestamp="2011-11-11T00:11:11Z">
     <nd ref="17001"/>
     <nd ref="17002"/>

diff --git a/test/completedb_test.go b/test/completedb_test.go
@@ -5,16 +5,13 @@ import (
 	"fmt"
 	"io/ioutil"
 	"os"
+	"testing"
 
 	osm "github.com/omniscale/go-osm"
 	"github.com/omniscale/imposm3/cache"
-
 	"github.com/omniscale/imposm3/geom"
-	"github.com/omniscale/imposm3/proj"
-
-	"testing"
-
 	"github.com/omniscale/imposm3/geom/geos"
+	"github.com/omniscale/imposm3/proj"
 )
 
 func TestComplete(t *testing.T) {
@@ -171,6 +168,13 @@ func TestComplete(t *testing.T) {
 		})
 	})
 
+	t.Run("WayWithInvalidLayer", func(t *testing.T) {
+		// Layer value is not a valid int32.
+		ts.assertRecords(t, []checkElem{
+			{"osm_roads", 17003, "residential", map[string]string{"z_order": "NULL"}},
+		})
+	})
+
 	t.Run("NodeWayInsertedTwice", func(t *testing.T) {
 		// Way with multiple mappings is inserted twice in same table
 		rows := ts.queryRows(t, "osm_roads", 18001)
@@ -591,6 +595,13 @@ func TestComplete(t *testing.T) {
 		ts.assertGeomArea(t, checkElem{"osm_landusages", -16001, "park", nil}, 12779350582)
 	})
 
+	t.Run("WayWithInvalidLayerUpdate", func(t *testing.T) {
+		// Layer value is now a valid int32.
+		ts.assertRecords(t, []checkElem{
+			{"osm_roads", 17003, "residential", map[string]string{"z_order": "23"}},
+		})
+	})
+
 	t.Run("NodeWayRefAfterDelete2", func(t *testing.T) {
 		// Node does not referece deleted way
 

diff --git a/test/helper_test.go b/test/helper_test.go
@@ -245,6 +245,8 @@ func (ts *importTestSuite) query(t *testing.T, table string, id int64, keys []st
 	for k, v := range h.Map {
 		if v.Valid {
 			r.tags[k] = v.String
+		} else {
+			r.tags[k] = "NULL"
 		}
 	}