From 736f4b932576428452059843dd6da63426a5c6c1 Mon Sep 17 00:00:00 2001 From: Jiri Drbalek Date: Fri, 26 Oct 2018 13:43:47 +0000 Subject: [PATCH] Support for importing non-area relations as MultiLineString --- database/postgis/postgis.go | 4 +- geom/multilinestring.go | 43 + geom/multilinestring_test.go | 43 + import_/import.go | 1 + mapping/mapping.go | 7 +- mapping/matcher.go | 5 +- test/multilinestring.osc | 274 +++ test/multilinestring.osm | 3044 ++++++++++++++++++++++++++++++ test/multilinestring_mapping.yml | 42 + test/multilinestring_test.go | 111 ++ update/process.go | 1 + writer/relations.go | 54 +- 12 files changed, 3621 insertions(+), 8 deletions(-) create mode 100644 geom/multilinestring.go create mode 100644 geom/multilinestring_test.go create mode 100644 test/multilinestring.osc create mode 100644 test/multilinestring.osm create mode 100644 test/multilinestring_mapping.yml create mode 100644 test/multilinestring_test.go diff --git a/database/postgis/postgis.go b/database/postgis/postgis.go index c7da1ae6..dcdbb6a8 100644 --- a/database/postgis/postgis.go +++ b/database/postgis/postgis.go @@ -72,8 +72,8 @@ func addGeometryColumn(tx *sql.Tx, tableName string, spec TableSpec) error { } geomType := strings.ToUpper(spec.GeometryType) - if geomType == "POLYGON" { - geomType = "GEOMETRY" // for multipolygon support + if geomType == "POLYGON" || geomType == "LINESTRING" { + geomType = "GEOMETRY" // for multigeometry support } sql := fmt.Sprintf("SELECT AddGeometryColumn('%s', '%s', '%s', '%d', '%s', 2);", spec.Schema, tableName, colName, spec.Srid, geomType) diff --git a/geom/multilinestring.go b/geom/multilinestring.go new file mode 100644 index 00000000..6c24e76a --- /dev/null +++ b/geom/multilinestring.go @@ -0,0 +1,43 @@ +package geom + +import ( + "errors" + "github.com/omniscale/imposm3/element" + "github.com/omniscale/imposm3/geom/geos" + "runtime" +) + +func BuildMultiLinestring(rel *element.Relation, srid int) (*geos.Geom, error) { + g := geos.NewGeos() + g.SetHandleSrid(srid) + defer g.Finish() + + var lines []*geos.Geom + + for _, member := range rel.Members { + if member.Way == nil { + continue + } + + line, err := LineString(g, member.Way.Nodes) + + // Clear the finalizer created in LineString() + // as we want to make the object a part of MultiLineString. + runtime.SetFinalizer(line, nil) + + if err != nil { + return nil, err + } + + lines = append(lines, line) + } + + result := g.MultiLineString(lines) + if result == nil { + return nil, errors.New("Error while building multi-linestring.") + } + + g.DestroyLater(result) + + return result, nil +} diff --git a/geom/multilinestring_test.go b/geom/multilinestring_test.go new file mode 100644 index 00000000..53e6dfee --- /dev/null +++ b/geom/multilinestring_test.go @@ -0,0 +1,43 @@ +package geom + +import ( + "testing" + + "github.com/omniscale/imposm3/element" + "github.com/omniscale/imposm3/geom/geos" +) + +func TestSimpleMultiLineString(t *testing.T) { + w1 := makeWay(1, element.Tags{}, []coord{ + {1, 1, 0}, + {2, 2, 0}, + }) + w2 := makeWay(2, element.Tags{}, []coord{ + {3, 2, 0}, + {4, 3, 0}, + }) + + rel := element.Relation{ + OSMElem: element.OSMElem{Id: 1, Tags: element.Tags{}}} + rel.Members = []element.Member{ + {Id: 1, Type: element.WAY, Role: "", Way: &w1}, + {Id: 2, Type: element.WAY, Role: "", Way: &w2}, + } + + geom, err := BuildMultiLinestring(&rel, 3857) + + if err != nil { + t.Fatal(err) + } + + g := geos.NewGeos() + defer g.Finish() + + if !g.IsValid(geom) { + t.Fatal("geometry not valid", g.AsWkt(geom)) + } + + if length := geom.Length(); length != 2 { + t.Fatal("length invalid", length) + } +} diff --git a/import_/import.go b/import_/import.go index 1b31d7f4..d76d786a 100644 --- a/import_/import.go +++ b/import_/import.go @@ -180,6 +180,7 @@ func Import(importOpts config.Import) { tagmapping.Conf.SingleIdSpace, relations, db, progress, + tagmapping.LineStringMatcher, tagmapping.PolygonMatcher, tagmapping.RelationMatcher, tagmapping.RelationMemberMatcher, diff --git a/mapping/mapping.go b/mapping/mapping.go index e44c1d3e..e93be57a 100644 --- a/mapping/mapping.go +++ b/mapping/mapping.go @@ -84,7 +84,7 @@ const ( type Mapping struct { Conf config.Mapping PointMatcher NodeMatcher - LineStringMatcher WayMatcher + LineStringMatcher RelWayMatcher PolygonMatcher RelWayMatcher RelationMatcher RelationMatcher RelationMemberMatcher RelationMatcher @@ -356,6 +356,11 @@ func (m *Mapping) addRelationFilters(tableType TableType, filters tableElementFi return false } filters[name] = append(filters[name], f) + } else if TableType(t.Type) == LineStringTable { + f := func(tags element.Tags, key Key, closed bool) bool { + return false + } + filters[name] = append(filters[name], f) } } } diff --git a/mapping/matcher.go b/mapping/matcher.go index 2c584460..29cab88f 100644 --- a/mapping/matcher.go +++ b/mapping/matcher.go @@ -20,17 +20,20 @@ func (m *Mapping) pointMatcher() (NodeMatcher, error) { }, err } -func (m *Mapping) lineStringMatcher() (WayMatcher, error) { +func (m *Mapping) lineStringMatcher() (RelWayMatcher, error) { mappings := make(TagTableMapping) m.mappings(LineStringTable, mappings) filters := make(tableElementFilters) m.addFilters(filters) m.addTypedFilters(LineStringTable, filters) + relFilters := make(tableElementFilters) + m.addRelationFilters(LineStringTable, relFilters) tables, err := m.tables(LineStringTable) return &tagMatcher{ mappings: mappings, filters: filters, tables: tables, + relFilters: relFilters, matchAreas: false, }, err } diff --git a/test/multilinestring.osc b/test/multilinestring.osc new file mode 100644 index 00000000..15a18559 --- /dev/null +++ b/test/multilinestring.osc @@ -0,0 +1,274 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/test/multilinestring.osm b/test/multilinestring.osm new file mode 100644 index 00000000..c7585eda --- /dev/null +++ b/test/multilinestring.osm @@ -0,0 +1,3044 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/test/multilinestring_mapping.yml b/test/multilinestring_mapping.yml new file mode 100644 index 00000000..7d16f9cc --- /dev/null +++ b/test/multilinestring_mapping.yml @@ -0,0 +1,42 @@ +areas: + area_tags: + - leisure +tables: + multilinestring: + type: linestring + columns: + - name: osm_id + type: id + - name: geometry + type: geometry + - name: name + type: string + key: name + - name: type + type: mapping_value + relation_types: + - route + mapping: + type: + - route + highway: + - trunk + building: + - residential + leisure: + - park + multilinestring_no_relations: + type: linestring + columns: + - name: osm_id + type: id + - name: geometry + type: geometry + - name: name + type: string + key: name + - name: type + type: mapping_value + mapping: + type: + - route diff --git a/test/multilinestring_test.go b/test/multilinestring_test.go new file mode 100644 index 00000000..3ae47fdd --- /dev/null +++ b/test/multilinestring_test.go @@ -0,0 +1,111 @@ +package test + +import ( + "database/sql" + "io/ioutil" + + "testing" + + "github.com/omniscale/imposm3/geom/geos" +) + +func TestMultiLineString(t *testing.T) { + if testing.Short() { + t.Skip("system test skipped with -test.short") + } + t.Parallel() + + ts := importTestSuite{ + name: "multilinestring", + } + + t.Run("Prepare", func(t *testing.T) { + var err error + + ts.dir, err = ioutil.TempDir("", "imposm_test") + if err != nil { + t.Fatal(err) + } + ts.config = importConfig{ + connection: "postgis://", + cacheDir: ts.dir, + osmFileName: "build/multilinestring.pbf", + mappingFileName: "multilinestring_mapping.yml", + } + ts.g = geos.NewGeos() + + ts.db, err = sql.Open("postgres", "sslmode=disable") + if err != nil { + t.Fatal(err) + } + ts.dropSchemas() + }) + + const mlsTable = "osm_multilinestring" + + t.Run("Import", func(t *testing.T) { + if ts.tableExists(t, ts.dbschemaImport(), mlsTable) != false { + t.Fatalf("table %s exists in schema %s", mlsTable, ts.dbschemaImport()) + } + ts.importOsm(t) + if ts.tableExists(t, ts.dbschemaImport(), mlsTable) != true { + t.Fatalf("table %s does not exists in schema %s", mlsTable, ts.dbschemaImport()) + } + }) + + t.Run("Deploy", func(t *testing.T) { + ts.deployOsm(t) + if ts.tableExists(t, ts.dbschemaImport(), mlsTable) != false { + t.Fatalf("table %s exists in schema %s", mlsTable, ts.dbschemaImport()) + } + if ts.tableExists(t, ts.dbschemaProduction(), mlsTable) != true { + t.Fatalf("table %s does not exists in schema %s", mlsTable, ts.dbschemaProduction()) + } + }) + + t.Run("CheckMultiLineStringGeometry", func(t *testing.T) { + element := checkElem{mlsTable, -1077494, "*", nil} + ts.assertGeomType(t, element, "MultiLineString") + ts.assertGeomValid(t, element) + ts.assertGeomLength(t, element, 22005) + }) + + t.Run("CheckLineStringGeometry", func(t *testing.T) { + element := checkElem{mlsTable, 40881141, "*", nil} + ts.assertGeomType(t, element, "LineString") + ts.assertGeomValid(t, element) + ts.assertGeomLength(t, element, 207) + }) + + t.Run("CheckFilters", func(t *testing.T) { + if records := ts.queryRows(t, mlsTable, 25948796); len(records) > 0 { + t.Fatalf("The way 25948796 should be filtered out by typed filter") + } + if records := ts.queryRows(t, mlsTable, 27037886); len(records) > 0 { + t.Fatalf("The way 27037886 should be filtered out as it is closed path with area=yes") + } + }) + + t.Run("RelationTypesFilter", func(t *testing.T) { + if records := ts.queryRows(t, "osm_multilinestring_no_relations", -1077494); len(records) > 0 { + t.Fatalf("The relation -1077494 should not be imported due to empty relation_types") + } + }) + + t.Run("Update", func(t *testing.T) { + ts.updateOsm(t, "build/multilinestring.osc.gz") + }) + + t.Run("CheckFilters2", func(t *testing.T) { + if records := ts.queryRows(t, mlsTable, 27037886); len(records) == 0 { + t.Fatalf("The way 27037886 should now be there as we removed area=yes in the update") + } + }) + + t.Run("CheckNewRelation", func(t *testing.T) { + if records := ts.queryRows(t, mlsTable, -8747972); len(records) == 0 { + t.Fatalf("The relation -8747972 should be created") + } + }) +} + diff --git a/update/process.go b/update/process.go index 8baf2d05..aae3ed43 100644 --- a/update/process.go +++ b/update/process.go @@ -172,6 +172,7 @@ func Update( tagmapping.Conf.SingleIdSpace, relations, db, progress, + tagmapping.LineStringMatcher, tagmapping.PolygonMatcher, tagmapping.RelationMatcher, tagmapping.RelationMemberMatcher, diff --git a/writer/relations.go b/writer/relations.go index a9dab611..2dcc1778 100644 --- a/writer/relations.go +++ b/writer/relations.go @@ -1,6 +1,7 @@ package writer import ( + "errors" "sync" "time" @@ -19,6 +20,7 @@ type RelationWriter struct { OsmElemWriter singleIdSpace bool rel chan *element.Relation + lineMatcher mapping.RelWayMatcher polygonMatcher mapping.RelWayMatcher relationMatcher mapping.RelationMatcher relationMemberMatcher mapping.RelationMatcher @@ -32,7 +34,8 @@ func NewRelationWriter( rel chan *element.Relation, inserter database.Inserter, progress *stats.Statistics, - matcher mapping.RelWayMatcher, + lineMatcher mapping.RelWayMatcher, + polygonMatcher mapping.RelWayMatcher, relMatcher mapping.RelationMatcher, relMemberMatcher mapping.RelationMatcher, srid int, @@ -51,7 +54,8 @@ func NewRelationWriter( srid: srid, }, singleIdSpace: singleIdSpace, - polygonMatcher: matcher, + lineMatcher: lineMatcher, + polygonMatcher: polygonMatcher, relationMatcher: relMatcher, relationMemberMatcher: relMemberMatcher, rel: rel, @@ -113,6 +117,9 @@ NextRel: if handleMultiPolygon(rw, r, geos) { inserted = true } + if handleMultiLinestring(rw, r, geos) { + inserted = true + } if inserted && rw.diffCache != nil { rw.diffCache.Ways.AddFromMembers(r.Id, allMembers) @@ -161,6 +168,45 @@ func handleMultiPolygon(rw *RelationWriter, r *element.Relation, geos *geosp.Geo return false } + return clipAndInsertGeom(geom, rw, r, matches, geos, rw.inserter.InsertPolygon) +} + +func handleMultiLinestring(rw *RelationWriter, r *element.Relation, geos *geosp.Geos) bool { + matches := rw.lineMatcher.MatchRelation(r) + if matches == nil { + return false + } + + geosGeom, err := geomp.BuildMultiLinestring(r, rw.srid) + + if err != nil { + if errl, ok := err.(ErrorLevel); !ok || errl.Level() > 0 { + log.Println("[warn]: ", err) + } + return false + } + + wkb := geos.AsEwkbHex(geosGeom) + if wkb == nil { + log.Println("[warn]: ", errors.New("unable to create WKB for relation")) + return false + } + + geom := geomp.Geometry{Geom: geosGeom, Wkb: wkb} + + return clipAndInsertGeom(geom, rw, r, matches, geos, rw.inserter.InsertLineString) +} + +type insertGeom func(element.OSMElem, geomp.Geometry, []mapping.Match) error + +func clipAndInsertGeom( + geom geomp.Geometry, + rw *RelationWriter, + r *element.Relation, + matches []mapping.Match, + geos *geosp.Geos, + insertFunc insertGeom, +) bool { if rw.limiter != nil { start := time.Now() parts, err := rw.limiter.Clip(geom.Geom) @@ -178,7 +224,7 @@ func handleMultiPolygon(rw *RelationWriter, r *element.Relation, geos *geosp.Geo rel := element.Relation(*r) rel.Id = rw.relId(r.Id) geom = geomp.Geometry{Geom: g, Wkb: geos.AsEwkbHex(g)} - err := rw.inserter.InsertPolygon(rel.OSMElem, geom, matches) + err := insertFunc(rel.OSMElem, geom, matches) if err != nil { if errl, ok := err.(ErrorLevel); !ok || errl.Level() > 0 { log.Println("[warn]: ", err) @@ -189,7 +235,7 @@ func handleMultiPolygon(rw *RelationWriter, r *element.Relation, geos *geosp.Geo } else { rel := element.Relation(*r) rel.Id = rw.relId(r.Id) - err := rw.inserter.InsertPolygon(rel.OSMElem, geom, matches) + err := insertFunc(rel.OSMElem, geom, matches) if err != nil { if errl, ok := err.(ErrorLevel); !ok || errl.Level() > 0 { log.Println("[warn]: ", err)