From b3b388b11b0e4090f7a185b30ca752bdc6a9642c Mon Sep 17 00:00:00 2001
From: Oliver Tonnhofer <olt@bogosoft.com>
Date: Tue, 20 Oct 2015 13:20:48 +0200
Subject: [PATCH] import multipolygons if at least on ring was build

---
 geom/multipolygon.go      | 14 ++++-----
 geom/multipolygon_test.go | 61 ++++++++++++++++++++++++++++++++++++++-
 test/complete_db.osm      | 25 ++++++++++++++++
 test/complete_db_test.py  |  5 ++++
 4 files changed, 97 insertions(+), 8 deletions(-)

diff --git a/geom/multipolygon.go b/geom/multipolygon.go
index 2d2b887..f5f4ca8 100644
--- a/geom/multipolygon.go
+++ b/geom/multipolygon.go
@@ -93,23 +93,23 @@ func buildRings(rel *element.Relation, maxRingGap float64) ([]*ring, error) {
 	}
 	// merge incomplete rings
 	mergedRings = mergeRings(incompleteRings)
-	if len(completeRings)+len(mergedRings) == 0 {
-		err = ErrorNoRing // for defer
-		return nil, err
-	}
+
 	// create geometries for merged rings
 	for _, ring := range mergedRings {
 		if !ring.isClosed() && !ring.tryClose(maxRingGap) {
-			err = ErrorNoRing // for defer
-			return nil, err
+			continue
 		}
 		ring.geom, err = Polygon(g, ring.nodes)
 		if err != nil {
 			return nil, err
 		}
+		completeRings = append(completeRings, ring)
 	}
 
-	completeRings = append(completeRings, mergedRings...)
+	if len(completeRings) == 0 {
+		err = ErrorNoRing // for defer
+		return nil, err
+	}
 
 	// sort by area (large to small)
 	for _, r := range completeRings {
diff --git a/geom/multipolygon_test.go b/geom/multipolygon_test.go
index fae9b14..bdc6cad 100644
--- a/geom/multipolygon_test.go
+++ b/geom/multipolygon_test.go
@@ -31,7 +31,6 @@ func buildRelation(rel *element.Relation, srid int) (Geometry, error) {
 	if err != nil {
 		return Geometry{}, err
 	}
-
 	return prep.Build()
 }
 
@@ -599,3 +598,63 @@ func TestBrokenPolygonSelfIntersectTriangle(t *testing.T) {
 
 	}
 }
+
+func TestOpenRing(t *testing.T) {
+	w1 := makeWay(1, element.Tags{}, []coord{
+		{1, 0, 0},
+		{2, 10, 0},
+		{3, 10, 10},
+		{4, 0, 10},
+	})
+
+	rel := element.Relation{
+		OSMElem: element.OSMElem{Id: 1, Tags: element.Tags{}}}
+	rel.Members = []element.Member{
+		{1, element.WAY, "outer", &w1},
+	}
+
+	_, err := buildRelation(&rel, 3857)
+	if err == nil {
+		t.Fatal("no error from open ring")
+	}
+}
+
+func TestClosedAndOpenRing(t *testing.T) {
+	w1 := makeWay(1, element.Tags{}, []coord{
+		{1, 0, 0},
+		{2, 10, 0},
+		{3, 10, 10},
+		{4, 0, 10},
+		{1, 0, 0},
+	})
+	w2 := makeWay(2, element.Tags{}, []coord{
+		{5, 0, 0},
+		{6, -5, -2},
+	})
+	rel := element.Relation{
+		OSMElem: element.OSMElem{Id: 1, Tags: element.Tags{}}}
+	rel.Members = []element.Member{
+		{1, element.WAY, "outer", &w1},
+		{2, element.WAY, "outer", &w2},
+	}
+
+	prep, err := PrepareRelation(&rel, 3857, 0.1)
+	if err != nil {
+		t.Fatal(err)
+	}
+	// open ring is excluded
+	if len(prep.rings) != 1 {
+		t.Fatal("expected single ring")
+	}
+	geom, err := prep.Build()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	g := geos.NewGeos()
+	defer g.Finish()
+
+	if !g.IsValid(geom.Geom) {
+		t.Fatal("geometry not valid", g.AsWkt(geom.Geom))
+	}
+}
diff --git a/test/complete_db.osm b/test/complete_db.osm
index 1c84f7c..a6b7b8c 100644
--- a/test/complete_db.osm
+++ b/test/complete_db.osm
@@ -386,6 +386,31 @@
     <tag k="landuse" v="park"/>
   </way>
 
+  <!-- relation/way with "gap" (ways overlap, but are only sharing one endpoint) -->
+  <node id="7401" version="1" timestamp="2011-11-11T00:11:11Z" lat="60" lon="60"/>
+  <node id="7402" version="1" timestamp="2011-11-11T00:11:11Z" lat="60" lon="62"/>
+  <node id="7403" version="1" timestamp="2011-11-11T00:11:11Z" lat="62" lon="62"/>
+  <node id="7404" version="1" timestamp="2011-11-11T00:11:11Z" lat="62" lon="60"/>
+  <way id="7401" version="1" timestamp="2011-11-11T00:11:11Z">
+    <nd ref="7401"/>
+    <nd ref="7402"/>
+    <nd ref="7403"/>
+    <nd ref="7404"/>
+    <nd ref="7401"/>
+  </way>
+  <way id="7402" version="1" timestamp="2011-11-11T00:11:11Z">
+    <!-- connected to other way, but not at start/end node and neither a ring on its own -->
+    <nd ref="7402"/>
+    <nd ref="7404"/>
+  </way>
+  <relation id="7401" version="1" timestamp="2011-11-11T00:11:11Z">
+    <member type="way" ref="7401" role="outer"/>
+    <member type="way" ref="7402" role="outer"/>
+    <tag k="name" v="rel 7401"/>
+    <tag k="landuse" v="park"/>
+    <tag k="type" v="multipolygon"/>
+  </relation>
+
   <!-- test that single node ways or incomplete polygons are _not_ inserted -->
   <node id="30001" version="1" timestamp="2011-11-11T00:11:11Z" lat="47" lon="80"/>
   <node id="30002" version="1" timestamp="2011-11-11T00:11:11Z" lat="47" lon="80"/>
diff --git a/test/complete_db_test.py b/test/complete_db_test.py
index 5d51b02..aa5cf48 100644
--- a/test/complete_db_test.py
+++ b/test/complete_db_test.py
@@ -284,6 +284,11 @@ def test_ring_with_gap():
     park = t.query_row(t.db_conf, 'osm_landusages', 7311)
     assert park['geometry'].is_valid, park
 
+def test_multipolygon_with_open_ring():
+    """Multipolygon is inserted even if there is an open ring/member"""
+    park = t.query_row(t.db_conf, 'osm_landusages', -7401)
+    assert park['geometry'].is_valid, park
+
 def test_updated_nodes1():
     """Zig-Zag line is inserted."""
     road =  t.query_row(t.db_conf, 'osm_roads', 60000)