From b82b1db8c1f92b15f04872fea14fec4c8e0070ce Mon Sep 17 00:00:00 2001 From: Oliver Tonnhofer Date: Tue, 9 May 2017 11:34:54 +0200 Subject: [PATCH] remove support for old-style multipolyon relation handling First simplification as a result of #142 and https://github.com/osmlab/fixing-polygons-in-osm/issues/15 --- geom/multipolygon.go | 26 ------------------ geom/multipolygon_test.go | 19 ++++++------- test/complete_db.osc | 10 +------ test/complete_db.osm | 56 ++++----------------------------------- test/completedb_test.go | 41 +++++++++++----------------- test/single_table_test.go | 4 +-- 6 files changed, 33 insertions(+), 123 deletions(-) diff --git a/geom/multipolygon.go b/geom/multipolygon.go index e5e631a..fd00605 100644 --- a/geom/multipolygon.go +++ b/geom/multipolygon.go @@ -16,15 +16,12 @@ type PreparedRelation struct { // PrepareRelation is the first step in building a (multi-)polygon of a Relation. // It builds rings from all ways and returns an error if there are unclosed rings. -// It also merges the Relation.Tags with the Tags of the outer way. func PrepareRelation(rel *element.Relation, srid int, maxRingGap float64) (PreparedRelation, error) { rings, err := buildRings(rel, maxRingGap) if err != nil { return PreparedRelation{}, err } - rel.Tags = relationTags(rel.Tags, rings[0].ways[0].Tags) - return PreparedRelation{rings, rel, srid}, nil } @@ -224,29 +221,6 @@ func buildRelGeometry(g *geos.Geos, rel *element.Relation, rings []*ring) (*geos return result, nil } -func relationTags(relTags, wayTags element.Tags) element.Tags { - result := make(element.Tags) - for k, v := range relTags { - if k == "name" || k == "type" { - continue - } - result[k] = v - } - - if len(result) == 0 { - // relation does not have tags? use way tags - for k, v := range wayTags { - result[k] = v - } - } else { - // add back name (if present) - if name, ok := relTags["name"]; ok { - result["name"] = name - } - } - return result -} - // ringIsHole returns true if rings[idx] is a hole, False if it is a // shell (also if hole in a hole, etc) func ringIsHole(rings []*ring, idx int) bool { diff --git a/geom/multipolygon_test.go b/geom/multipolygon_test.go index dc31b1c..ffa1926 100644 --- a/geom/multipolygon_test.go +++ b/geom/multipolygon_test.go @@ -94,7 +94,7 @@ func TestMultiPolygonWithHoleAndRelName(t *testing.T) { }) rel := element.Relation{ - OSMElem: element.OSMElem{Id: 1, Tags: element.Tags{"name": "rel"}}} + OSMElem: element.OSMElem{Id: 1, Tags: element.Tags{"name": "Relation", "natural": "forest", "type": "multipolygon"}}} rel.Members = []element.Member{ {Id: 1, Type: element.WAY, Role: "outer", Way: &w1}, {Id: 2, Type: element.WAY, Role: "inner", Way: &w2}, @@ -107,10 +107,11 @@ func TestMultiPolygonWithHoleAndRelName(t *testing.T) { g := geos.NewGeos() defer g.Finish() - if len(rel.Tags) != 2 { + if len(rel.Tags) != 3 { t.Fatal("wrong rel tags", rel.Tags) } - if rel.Tags["natural"] != "forest" || rel.Tags["name"] != "Blackwood" { + // name from way is ignored + if rel.Tags["natural"] != "forest" || rel.Tags["name"] != "Relation" { t.Fatal("wrong rel tags", rel.Tags) } @@ -147,7 +148,7 @@ func TestMultiPolygonWithMultipleHoles(t *testing.T) { }) rel := element.Relation{ - OSMElem: element.OSMElem{Id: 1, Tags: element.Tags{"landusage": "forest"}}} + OSMElem: element.OSMElem{Id: 1, Tags: element.Tags{"landusage": "forest", "type": "multipolygon"}}} rel.Members = []element.Member{ {Id: 1, Type: element.WAY, Role: "outer", Way: &w1}, {Id: 2, Type: element.WAY, Role: "inner", Way: &w2}, @@ -161,7 +162,7 @@ func TestMultiPolygonWithMultipleHoles(t *testing.T) { g := geos.NewGeos() defer g.Finish() - if len(rel.Tags) != 1 { + if len(rel.Tags) != 2 { t.Fatal("wrong rel tags", rel.Tags) } if rel.Tags["landusage"] != "forest" { @@ -214,7 +215,7 @@ func TestMultiPolygonWithNeastedHoles(t *testing.T) { {1, 4, 4}, }) - rel := element.Relation{OSMElem: element.OSMElem{Id: 1}} + rel := element.Relation{OSMElem: element.OSMElem{Id: 1, Tags: element.Tags{"landusage": "forest", "type": "multipolygon"}}} rel.Members = []element.Member{ {Id: 1, Type: element.WAY, Role: "outer", Way: &w1}, {Id: 2, Type: element.WAY, Role: "inner", Way: &w2}, @@ -230,7 +231,7 @@ func TestMultiPolygonWithNeastedHoles(t *testing.T) { g := geos.NewGeos() defer g.Finish() - if len(rel.Tags) != 1 { + if len(rel.Tags) != 2 { t.Fatal("wrong rel tags", rel.Tags) } if rel.Tags["landusage"] != "forest" { @@ -261,7 +262,7 @@ func TestPolygonFromThreeWays(t *testing.T) { {1, 0, 0}, }) - rel := element.Relation{OSMElem: element.OSMElem{Id: 1}} + rel := element.Relation{OSMElem: element.OSMElem{Id: 1, Tags: element.Tags{"landusage": "forest", "type": "multipolygon"}}} rel.Members = []element.Member{ {Id: 1, Type: element.WAY, Role: "outer", Way: &w1}, {Id: 2, Type: element.WAY, Role: "inner", Way: &w2}, @@ -275,7 +276,7 @@ func TestPolygonFromThreeWays(t *testing.T) { g := geos.NewGeos() defer g.Finish() - if len(rel.Tags) != 1 { + if len(rel.Tags) != 2 { t.Fatal("wrong rel tags", rel.Tags) } if rel.Tags["landusage"] != "forest" { diff --git a/test/complete_db.osc b/test/complete_db.osc index 3999726..9be7b66 100644 --- a/test/complete_db.osc +++ b/test/complete_db.osc @@ -35,15 +35,6 @@ - - - - - - - - - @@ -71,6 +62,7 @@ + diff --git a/test/complete_db.osm b/test/complete_db.osm index ed5ceb4..e417149 100644 --- a/test/complete_db.osm +++ b/test/complete_db.osm @@ -94,7 +94,6 @@ - @@ -267,6 +266,7 @@ + @@ -524,7 +524,6 @@ - @@ -540,37 +539,10 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - @@ -606,25 +578,6 @@ - - - - - - - - - - - - - - - - - - - @@ -672,6 +625,7 @@ + @@ -694,7 +648,6 @@ - @@ -708,6 +661,7 @@ + @@ -947,7 +901,7 @@ - + @@ -1031,10 +985,10 @@ - + diff --git a/test/completedb_test.go b/test/completedb_test.go index ba9b4b6..22720e7 100644 --- a/test/completedb_test.go +++ b/test/completedb_test.go @@ -66,32 +66,25 @@ func TestComplete_LandusageToWaterarea1(t *testing.T) { cache := ts.cache(t) defer cache.Close() assertCachedWay(t, cache, 11001) - assertCachedWay(t, cache, 12001) assertCachedWay(t, cache, 13001) assertRecords(t, []checkElem{ {"osm_waterareas", 11001, Missing, nil}, - {"osm_waterareas", -12001, Missing, nil}, {"osm_waterareas", -13001, Missing, nil}, {"osm_waterareas_gen0", 11001, Missing, nil}, - {"osm_waterareas_gen0", -12001, Missing, nil}, {"osm_waterareas_gen0", -13001, Missing, nil}, {"osm_waterareas_gen1", 11001, Missing, nil}, - {"osm_waterareas_gen1", -12001, Missing, nil}, {"osm_waterareas_gen1", -13001, Missing, nil}, {"osm_landusages", 11001, "park", nil}, - {"osm_landusages", -12001, "park", nil}, {"osm_landusages", -13001, "park", nil}, {"osm_landusages_gen0", 11001, "park", nil}, - {"osm_landusages_gen0", -12001, "park", nil}, {"osm_landusages_gen0", -13001, "park", nil}, {"osm_landusages_gen1", 11001, "park", nil}, - {"osm_landusages_gen1", -12001, "park", nil}, {"osm_landusages_gen1", -13001, "park", nil}, }) } @@ -106,7 +99,8 @@ func TestComplete_ChangedHoleTags1(t *testing.T) { assertRecords(t, []checkElem{ {"osm_waterareas", 14011, Missing, nil}, {"osm_waterareas", -14011, Missing, nil}, - {"osm_landusages", -14001, "park", nil}, + {"osm_landusages", 14001, "park", nil}, + {"osm_landusages", -14001, Missing, nil}, }) } @@ -218,19 +212,15 @@ func TestComplete_RelationWayNotInserted(t *testing.T) { func TestComplete_RelationWaysInserted(t *testing.T) { // Outer ways of multipolygon are inserted. assertRecords(t, []checkElem{ - {"osm_landusages", -9201, "park", map[string]string{"name": "9209"}}, + // no name on relation + {"osm_landusages", -9201, "park", map[string]string{"name": ""}}, {"osm_landusages", 9201, Missing, nil}, + {"osm_landusages", 9209, Missing, nil}, + {"osm_landusages", 9210, Missing, nil}, // outer ways of multipolygon stand for their own {"osm_roads", 9209, "secondary", map[string]string{"name": "9209"}}, {"osm_roads", 9210, "residential", map[string]string{"name": "9210"}}, - - // no name on relation - {"osm_landusages", -9301, "park", map[string]string{"name": ""}}, - // outer ways of multipolygon stand for their own - {"osm_roads", 9309, "secondary", map[string]string{"name": "9309"}}, - {"osm_roads", 9310, "residential", map[string]string{"name": "9310"}}, }) - } func TestComplete_RelationWayInserted(t *testing.T) { @@ -283,12 +273,12 @@ func TestComplete_RelationBeforeRemove(t *testing.T) { }) } -func TestComplete_RelationWithoutTags(t *testing.T) { - // Relation without tags is inserted. +func TestComplete_OldStyleRelationIsIgnored(t *testing.T) { + // Relation without tags is not inserted. assertRecords(t, []checkElem{ - {"osm_buildings", 50111, Missing, nil}, - {"osm_buildings", -50121, "yes", nil}, + {"osm_buildings", 50111, "yes", nil}, + {"osm_buildings", -50121, Missing, nil}, }) } @@ -483,27 +473,21 @@ func TestComplete_LandusageToWaterarea2(t *testing.T) { assertRecords(t, []checkElem{ {"osm_waterareas", 11001, "water", nil}, - {"osm_waterareas", -12001, "water", nil}, {"osm_waterareas", -13001, "water", nil}, {"osm_waterareas_gen0", 11001, "water", nil}, - {"osm_waterareas_gen0", -12001, "water", nil}, {"osm_waterareas_gen0", -13001, "water", nil}, {"osm_waterareas_gen1", 11001, "water", nil}, - {"osm_waterareas_gen1", -12001, "water", nil}, {"osm_waterareas_gen1", -13001, "water", nil}, {"osm_landusages", 11001, Missing, nil}, - {"osm_landusages", -12001, Missing, nil}, {"osm_landusages", -13001, Missing, nil}, {"osm_landusages_gen0", 11001, Missing, nil}, - {"osm_landusages_gen0", -12001, Missing, nil}, {"osm_landusages_gen0", -13001, Missing, nil}, {"osm_landusages_gen1", 11001, Missing, nil}, - {"osm_landusages_gen1", -12001, Missing, nil}, {"osm_landusages_gen1", -13001, Missing, nil}, }) } @@ -518,6 +502,11 @@ func TestComplete_ChangedHoleTags2(t *testing.T) { assertGeomArea(t, checkElem{"osm_waterareas", 14011, "water", nil}, 26672019779) assertGeomArea(t, checkElem{"osm_landusages", -14001, "park", nil}, 10373697182) + + assertRecords(t, []checkElem{ + {"osm_waterareas", -14011, Missing, nil}, + {"osm_landusages", -14001, "park", nil}, + }) } func TestComplete_SplitOuterMultipolygonWay2(t *testing.T) { diff --git a/test/single_table_test.go b/test/single_table_test.go index ed4b2a4..3bf2b05 100644 --- a/test/single_table_test.go +++ b/test/single_table_test.go @@ -159,7 +159,7 @@ func TestSingleTable_DuplicateIds1(t *testing.T) { } assertHstore(t, []checkElem{ - {"osm_all", RelOffset - 31101, "*", map[string]string{"building": "yes"}}, + {"osm_all", RelOffset - 31101, "*", map[string]string{"building": "yes", "type": "multipolygon"}}, }) assertGeomType(t, checkElem{"osm_all", RelOffset - 31101, "*", nil}, "Polygon") } @@ -185,7 +185,7 @@ func TestSingleTable_DuplicateIds2(t *testing.T) { } assertHstore(t, []checkElem{ - {"osm_all", RelOffset - 31101, "*", map[string]string{"building": "yes"}}, + {"osm_all", RelOffset - 31101, "*", map[string]string{"building": "yes", "type": "multipolygon"}}, }) assertGeomType(t, checkElem{"osm_all", RelOffset - 31101, "*", nil}, "Polygon") }