From b4cf7b91f3e52e81785c9abc43ec5d6d95319b0a Mon Sep 17 00:00:00 2001 From: Oliver Tonnhofer Date: Tue, 9 May 2017 15:55:21 +0200 Subject: [PATCH] update tag filtering --- mapping/config.go | 18 ++- mapping/filter.go | 86 ++--------- mapping/filter_test.go | 318 +++++++++++------------------------------ mapping/matcher.go | 21 ++- 4 files changed, 121 insertions(+), 322 deletions(-) diff --git a/mapping/config.go b/mapping/config.go index 0366328..932a11f 100644 --- a/mapping/config.go +++ b/mapping/config.go @@ -276,9 +276,9 @@ func (m *Mapping) extraTags(tableType TableType, tags map[Key]bool) { tags["area"] = true } -func (m *Mapping) ElementFilters() map[string][]ElementFilter { - result := make(map[string][]ElementFilter) +type tableFilters map[string][]ElementFilter +func (m *Mapping) addTypedFilters(tableType TableType, filters tableFilters) { var areaTags map[Key]struct{} var linearTags map[Key]struct{} if m.Areas.AreaTags != nil { @@ -295,6 +295,9 @@ func (m *Mapping) ElementFilters() map[string][]ElementFilter { } for name, t := range m.Tables { + if t.Type != GeometryTable && t.Type != tableType { + continue + } if t.Type == LineStringTable && areaTags != nil { f := func(tags element.Tags, key Key, closed bool) bool { if closed { @@ -309,7 +312,7 @@ func (m *Mapping) ElementFilters() map[string][]ElementFilter { } return true } - result[name] = append(result[name], f) + filters[name] = append(filters[name], f) } if t.Type == PolygonTable && linearTags != nil { f := func(tags element.Tags, key Key, closed bool) bool { @@ -323,9 +326,13 @@ func (m *Mapping) ElementFilters() map[string][]ElementFilter { } return true } - result[name] = append(result[name], f) + filters[name] = append(filters[name], f) } + } +} +func (m *Mapping) addFilters(filters tableFilters) { + for name, t := range m.Tables { if t.Filters == nil { continue } @@ -339,9 +346,8 @@ func (m *Mapping) ElementFilters() map[string][]ElementFilter { } return true } - result[name] = append(result[name], f) + filters[name] = append(filters[name], f) } } } - return result } diff --git a/mapping/filter.go b/mapping/filter.go index d5507d6..0142869 100644 --- a/mapping/filter.go +++ b/mapping/filter.go @@ -36,18 +36,18 @@ func (m *Mapping) RelationTagFilter() TagFilterer { return newExcludeFilter(m.Tags.Exclude) } mappings := make(map[Key]map[Value][]OrderedDestTable) - m.mappings("linestring", mappings) - m.mappings("polygon", mappings) - tags := make(map[Key]bool) - m.extraTags("linestring", tags) - m.extraTags("polygon", tags) - // do not filter out type tag + // do not filter out type tag for common relations mappings["type"] = map[Value][]OrderedDestTable{ "multipolygon": []OrderedDestTable{}, "boundary": []OrderedDestTable{}, "land_area": []OrderedDestTable{}, } - return &RelationTagFilter{TagFilter{mappings, tags}} + m.mappings("linestring", mappings) + m.mappings("polygon", mappings) + tags := make(map[Key]bool) + m.extraTags("linestring", tags) + m.extraTags("polygon", tags) + return &TagFilter{mappings, tags} } type TagFilter struct { @@ -55,10 +55,6 @@ type TagFilter struct { extraTags map[Key]bool } -type RelationTagFilter struct { - TagFilter -} - type ExcludeFilter struct { keys map[Key]struct{} matches []string @@ -79,8 +75,8 @@ func newExcludeFilter(tags []Key) *ExcludeFilter { return &f } -func (f *ExcludeFilter) Filter(tags *element.Tags) bool { - for k, _ := range *tags { +func (f *ExcludeFilter) Filter(tags *element.Tags) { + for k := range *tags { if _, ok := f.keys[Key(k)]; ok { delete(*tags, k) } else if f.matches != nil { @@ -92,26 +88,22 @@ func (f *ExcludeFilter) Filter(tags *element.Tags) bool { } } } - return true } type TagFilterer interface { - Filter(tags *element.Tags) bool + Filter(tags *element.Tags) } -func (f *TagFilter) Filter(tags *element.Tags) bool { +func (f *TagFilter) Filter(tags *element.Tags) { if tags == nil { - return false + return } - foundMapping := false for k, v := range *tags { values, ok := f.mappings[Key(k)] if ok { if _, ok := values["__any__"]; ok { - foundMapping = true continue } else if _, ok := values[Value(v)]; ok { - foundMapping = true continue } else if _, ok := f.extraTags[Key(k)]; !ok { delete(*tags, k) @@ -120,58 +112,4 @@ func (f *TagFilter) Filter(tags *element.Tags) bool { delete(*tags, k) } } - if foundMapping { - return true - } else { - *tags = nil - return false - } -} - -func (f *RelationTagFilter) Filter(tags *element.Tags) bool { - if tags == nil { - return false - } - - // TODO improve filtering for relation/relation_member mappings - // right now this only works with tags.load_all:true - if t, ok := (*tags)["type"]; ok { - if t != "multipolygon" && t != "boundary" && t != "land_area" { - *tags = nil - return false - } - if t == "boundary" { - if _, ok := (*tags)["boundary"]; !ok { - // a lot of the boundary relations are not multipolygon - // only import with boundary tags (e.g. boundary=administrative) - *tags = nil - return false - } - } - } else { - *tags = nil - return false - } - tagCount := len(*tags) - f.TagFilter.Filter(tags) - - // we removed tags... - if len(*tags) < tagCount { - expectedTags := 0 - if _, ok := (*tags)["name"]; ok { - expectedTags += 1 - } - if _, ok := (*tags)["type"]; ok { - expectedTags += 1 - } - if len(*tags) == expectedTags { - // but no tags except name and type are left - // remove all, otherwise tags from longest - // way/ring would be used during MP building - *tags = nil - return false - } - } - // always return true here since we found a matching type - return true } diff --git a/mapping/filter_test.go b/mapping/filter_test.go index 209b053..6869d65 100644 --- a/mapping/filter_test.go +++ b/mapping/filter_test.go @@ -19,20 +19,37 @@ func init() { func stringMapEquals(t *testing.T, expected, actual map[string]string) { if len(expected) != len(actual) { - t.Fatalf("different length in %v and %v\n", expected, actual) + t.Errorf("different length in %v and %v\n", expected, actual) } for k, v := range expected { if actualV, ok := actual[k]; ok { if actualV != v { - t.Fatalf("%s != %s in %v and %v\n", v, actualV, expected, actual) + t.Errorf("%s != %s in %v and %v\n", v, actualV, expected, actual) } } else { - t.Fatalf("%s not in %v\n", k, actual) + t.Errorf("%s not in %v\n", k, actual) } } } +func stringMapEqual(expected, actual map[string]string) bool { + if len(expected) != len(actual) { + return false + } + + for k, v := range expected { + if actualV, ok := actual[k]; ok { + if actualV != v { + return false + } + } else { + return false + } + } + return true +} + func matchesEqual(t *testing.T, expected []Match, actual []Match) { expectedMatches := make(map[DestTable]Match) actualMatches := make(map[DestTable]Match) @@ -62,194 +79,78 @@ func matchesEqual(t *testing.T, expected []Match, actual []Match) { } func TestTagFilterNodes(t *testing.T) { - var tags element.Tags + tests := []struct { + tags element.Tags + expected element.Tags + }{ + {tags: element.Tags{}, expected: element.Tags{}}, + {tags: element.Tags{"name": "foo"}, expected: element.Tags{"name": "foo"}}, + {tags: element.Tags{"name": "foo", "unknown": "foo"}, expected: element.Tags{"name": "foo"}}, + {tags: element.Tags{"name": "foo", "place": "unknown"}, expected: element.Tags{"name": "foo"}}, + {tags: element.Tags{"name": "foo", "place": "unknown", "population": "1000"}, expected: element.Tags{"name": "foo", "population": "1000"}}, + {tags: element.Tags{"name": "foo", "place": "village"}, expected: element.Tags{"name": "foo", "place": "village"}}, + {tags: element.Tags{"name": "foo", "place": "village", "population": "1000"}, expected: element.Tags{"name": "foo", "place": "village", "population": "1000"}}, + {tags: element.Tags{"name": "foo", "place": "village", "unknown": "foo"}, expected: element.Tags{"name": "foo", "place": "village"}}, + {tags: element.Tags{"name": "foo", "place": "village", "highway": "bus_stop"}, expected: element.Tags{"name": "foo", "place": "village", "highway": "bus_stop"}}, + } + nodes := mapping.NodeTagFilter() - - tags = element.Tags{"name": "foo"} - if nodes.Filter(&tags) != false { - t.Fatal("unexpected filter response for", tags) + for i, test := range tests { + nodes.Filter(&test.tags) + if !stringMapEqual(test.tags, test.expected) { + t.Errorf("unexpected result for case %d: %v != %v", i+1, test.tags, test.expected) + } } - stringMapEquals(t, element.Tags{}, tags) - - tags = element.Tags{"name": "foo", "unknown": "baz"} - if nodes.Filter(&tags) != false { - t.Fatal("unexpected filter response for", tags) - } - stringMapEquals(t, element.Tags{}, tags) - - tags = element.Tags{"name": "foo", "place": "unknown"} - if nodes.Filter(&tags) != false { - t.Fatal("unexpected filter response for", tags) - } - stringMapEquals(t, element.Tags{}, tags) - - tags = element.Tags{"name": "foo", "place": "village"} - if nodes.Filter(&tags) != true { - t.Fatal("unexpected filter response for", tags) - } - stringMapEquals(t, element.Tags{"name": "foo", "place": "village"}, tags) - - tags = element.Tags{"name": "foo", "place": "village", "population": "1000"} - if nodes.Filter(&tags) != true { - t.Fatal("unexpected filter response for", tags) - } - stringMapEquals(t, element.Tags{"name": "foo", "place": "village", "population": "1000"}, tags) - - tags = element.Tags{"name": "foo", "place": "village", "highway": "unknown"} - if nodes.Filter(&tags) != true { - t.Fatal("unexpected filter response for", tags) - } - stringMapEquals(t, element.Tags{"name": "foo", "place": "village"}, tags) - - tags = element.Tags{"name": "foo", "place": "village", "highway": "bus_stop"} - if nodes.Filter(&tags) != true { - t.Fatal("unexpected filter response for", tags) - } - stringMapEquals(t, element.Tags{"name": "foo", "place": "village", "highway": "bus_stop"}, tags) } func TestTagFilterWays(t *testing.T) { - var tags element.Tags + tests := []struct { + tags element.Tags + expected element.Tags + }{ + {tags: element.Tags{}, expected: element.Tags{}}, + {tags: element.Tags{"name": "foo"}, expected: element.Tags{"name": "foo"}}, + {tags: element.Tags{"name": "foo", "unknown": "foo"}, expected: element.Tags{"name": "foo"}}, + {tags: element.Tags{"name": "foo", "highway": "unknown"}, expected: element.Tags{"name": "foo"}}, + {tags: element.Tags{"name": "foo", "highway": "track"}, expected: element.Tags{"name": "foo", "highway": "track"}}, + {tags: element.Tags{"name": "foo", "building": "whatever"}, expected: element.Tags{"name": "foo", "building": "whatever"}}, + {tags: element.Tags{"name": "foo", "highway": "track", "unknown": "foo"}, expected: element.Tags{"name": "foo", "highway": "track"}}, + {tags: element.Tags{"name": "foo", "place": "village", "highway": "track"}, expected: element.Tags{"name": "foo", "highway": "track"}}, + {tags: element.Tags{"name": "foo", "highway": "track", "oneway": "yes", "tunnel": "1"}, expected: element.Tags{"name": "foo", "highway": "track", "oneway": "yes", "tunnel": "1"}}, + } + ways := mapping.WayTagFilter() - - tags = element.Tags{"name": "foo"} - if ways.Filter(&tags) != false { - t.Fatal("unexpected filter response for", tags) + for i, test := range tests { + ways.Filter(&test.tags) + if !stringMapEqual(test.tags, test.expected) { + t.Errorf("unexpected result for case %d: %v != %v", i+1, test.tags, test.expected) + } } - stringMapEquals(t, element.Tags{}, tags) - - tags = element.Tags{"name": "foo", "unknown": "baz"} - if ways.Filter(&tags) != false { - t.Fatal("unexpected filter response for", tags) - } - stringMapEquals(t, element.Tags{}, tags) - - tags = element.Tags{"name": "foo", "highway": "unknown"} - if ways.Filter(&tags) != false { - t.Fatal("unexpected filter response for", tags) - } - stringMapEquals(t, element.Tags{}, tags) - - tags = element.Tags{"name": "foo", "highway": "track"} - if ways.Filter(&tags) != true { - t.Fatal("unexpected filter response for", tags) - } - stringMapEquals(t, element.Tags{"name": "foo", "highway": "track"}, tags) - - tags = element.Tags{"name": "foo", "highway": "track", "oneway": "yes", "tunnel": "1"} - if ways.Filter(&tags) != true { - t.Fatal("unexpected filter response for", tags) - } - stringMapEquals(t, element.Tags{"name": "foo", "highway": "track", "oneway": "yes", "tunnel": "1"}, tags) - - tags = element.Tags{"name": "foo", "place": "village", "highway": "track"} - if ways.Filter(&tags) != true { - t.Fatal("unexpected filter response for", tags) - } - stringMapEquals(t, element.Tags{"name": "foo", "highway": "track"}, tags) - - tags = element.Tags{"name": "foo", "railway": "tram", "highway": "secondary"} - if ways.Filter(&tags) != true { - t.Fatal("unexpected filter response for", tags) - } - stringMapEquals(t, element.Tags{"name": "foo", "railway": "tram", "highway": "secondary"}, tags) - - // with __any__ value - tags = element.Tags{"name": "foo", "building": "yes"} - if ways.Filter(&tags) != true { - t.Fatal("unexpected filter response for", tags) - } - stringMapEquals(t, element.Tags{"name": "foo", "building": "yes"}, tags) - - tags = element.Tags{"name": "foo", "building": "whatever"} - if ways.Filter(&tags) != true { - t.Fatal("unexpected filter response for", tags) - } - stringMapEquals(t, element.Tags{"name": "foo", "building": "whatever"}, tags) } func TestTagFilterRelations(t *testing.T) { - var tags element.Tags + tests := []struct { + tags element.Tags + expected element.Tags + }{ + {tags: element.Tags{}, expected: element.Tags{}}, + {tags: element.Tags{"name": "foo"}, expected: element.Tags{"name": "foo"}}, + {tags: element.Tags{"name": "foo", "unknown": "foo"}, expected: element.Tags{"name": "foo"}}, + {tags: element.Tags{"name": "foo", "landuse": "unknown"}, expected: element.Tags{"name": "foo"}}, + {tags: element.Tags{"name": "foo", "landuse": "farm"}, expected: element.Tags{"name": "foo", "landuse": "farm"}}, + {tags: element.Tags{"name": "foo", "landuse": "farm", "type": "multipolygon"}, expected: element.Tags{"name": "foo", "landuse": "farm", "type": "multipolygon"}}, + {tags: element.Tags{"name": "foo", "type": "multipolygon"}, expected: element.Tags{"name": "foo", "type": "multipolygon"}}, + {tags: element.Tags{"name": "foo", "type": "boundary"}, expected: element.Tags{"name": "foo", "type": "boundary"}}, + {tags: element.Tags{"name": "foo", "landuse": "farm", "type": "boundary"}, expected: element.Tags{"name": "foo", "landuse": "farm", "type": "boundary"}}, + } + relations := mapping.RelationTagFilter() - - tags = element.Tags{"name": "foo"} - if relations.Filter(&tags) != false { - t.Fatal("unexpected filter response for", tags) + for i, test := range tests { + relations.Filter(&test.tags) + if !stringMapEqual(test.tags, test.expected) { + t.Errorf("unexpected result for case %d: %v != %v", i+1, test.tags, test.expected) + } } - stringMapEquals(t, element.Tags{}, tags) - - tags = element.Tags{"name": "foo", "unknown": "baz"} - if relations.Filter(&tags) != false { - t.Fatal("unexpected filter response for", tags) - } - stringMapEquals(t, element.Tags{}, tags) - - tags = element.Tags{"name": "foo", "landuse": "unknown"} - if relations.Filter(&tags) != false { - t.Fatal("unexpected filter response for", tags) - } - stringMapEquals(t, element.Tags{}, tags) - - tags = element.Tags{"name": "foo", "landuse": "farm"} - if relations.Filter(&tags) != false { - t.Fatal("unexpected filter response for", tags) - } - stringMapEquals(t, element.Tags{}, tags) - - tags = element.Tags{"name": "foo", "landuse": "farm", "type": "multipolygon"} - if relations.Filter(&tags) != true { - t.Fatal("unexpected filter response for", tags) - } - stringMapEquals(t, element.Tags{"name": "foo", "landuse": "farm", "type": "multipolygon"}, tags) - - // skip multipolygon with filtered tags, otherwise tags from - // longest way would be used - tags = element.Tags{"name": "foo", "landuse": "unknown", "type": "multipolygon"} - if relations.Filter(&tags) != false { - t.Fatal("unexpected filter response for", tags) - } - stringMapEquals(t, element.Tags{}, tags) - - tags = element.Tags{"name": "foo", "landuse": "park", "type": "multipolygon"} - if relations.Filter(&tags) != true { - t.Fatal("unexpected filter response for", tags) - } - stringMapEquals(t, element.Tags{"name": "foo", "type": "multipolygon", "landuse": "park"}, tags) - - tags = element.Tags{"name": "foo", "landuse": "farm", "boundary": "administrative", "type": "multipolygon"} - if relations.Filter(&tags) != true { - t.Fatal("unexpected filter response for", tags) - } - stringMapEquals(t, element.Tags{"name": "foo", "landuse": "farm", "boundary": "administrative", "type": "multipolygon"}, tags) - - // boundary relation for boundary - tags = element.Tags{"name": "foo", "landuse": "farm", "boundary": "administrative", "type": "boundary"} - if relations.Filter(&tags) != true { - t.Fatal("unexpected filter response for", tags) - } - stringMapEquals(t, element.Tags{"name": "foo", "landuse": "farm", "boundary": "administrative", "type": "boundary"}, tags) - - // boundary relation for non boundary - tags = element.Tags{"name": "foo", "landuse": "farm", "type": "boundary"} - if relations.Filter(&tags) != false { - t.Fatal("unexpected filter response for", tags) - } - stringMapEquals(t, element.Tags{}, tags) - - /* skip boundary with filtered tags, otherwise tags from longest way would - be used */ - tags = element.Tags{"name": "foo", "boundary": "unknown", "type": "boundary"} - if relations.Filter(&tags) != false { - t.Fatal("unexpected filter response for", tags) - } - stringMapEquals(t, element.Tags{}, tags) - - tags = element.Tags{"name": "foo", "boundary": "administrative", "type": "boundary"} - if relations.Filter(&tags) != true { - t.Fatal("unexpected filter response for", tags) - } - stringMapEquals(t, element.Tags{"name": "foo", "boundary": "administrative", "type": "boundary"}, tags) - } func TestPointMatcher(t *testing.T) { @@ -400,61 +301,6 @@ func TestMatcherMappingOrder(t *testing.T) { matchesEqual(t, []Match{{"amenity", "university", DestTable{Name: "landusages"}, nil}}, polys.MatchRelation(&elem)) } -func TestFilterNodes(t *testing.T) { - var tags element.Tags - - // test name only - tags = make(element.Tags) - tags["name"] = "foo" - - points := mapping.NodeTagFilter() - if points.Filter(&tags) != false { - t.Fatal("Filter result not false") - } - if len(tags) != 0 { - t.Fatal("Filter result not empty") - } - - // test name + unmapped tags - tags = make(element.Tags) - tags["name"] = "foo" - tags["boring"] = "true" - - if points.Filter(&tags) != false { - t.Fatal("Filter result not false") - } - if len(tags) != 0 { - t.Fatal("Filter result not empty") - } - - // test fields only, but no mapping - tags = make(element.Tags) - tags["population"] = "0" - tags["name"] = "foo" - tags["boring"] = "true" - - if points.Filter(&tags) != false { - t.Fatal("Filter result true", tags) - } - if len(tags) != 0 { - t.Fatal("Filter result not empty", tags) - } - - // ... not with mapped tag (place) - tags = make(element.Tags) - tags["population"] = "0" - tags["name"] = "foo" - tags["boring"] = "true" - tags["place"] = "village" - - if points.Filter(&tags) != true { - t.Fatal("Filter result true", tags) - } - if len(tags) != 3 && tags["population"] == "0" && tags["name"] == "foo" && tags["place"] == "village" { - t.Fatal("Filter result not expected", tags) - } -} - func TestExcludeFilter(t *testing.T) { var f TagFilterer var tags element.Tags @@ -495,9 +341,7 @@ func BenchmarkFilterNodes(b *testing.B) { tags["boring"] = "true" points := mapping.NodeTagFilter() - if points.Filter(&tags) != true { - b.Fatal("Filter result true", tags) - } + points.Filter(&tags) if len(tags) != 2 && tags["population"] == "0" && tags["name"] == "foo" { b.Fatal("Filter result not expected", tags) } diff --git a/mapping/matcher.go b/mapping/matcher.go index 3796eae..da69520 100644 --- a/mapping/matcher.go +++ b/mapping/matcher.go @@ -8,7 +8,9 @@ import ( func (m *Mapping) PointMatcher() NodeMatcher { mappings := make(TagTables) m.mappings(PointTable, mappings) - filters := m.ElementFilters() + filters := make(tableFilters) + m.addFilters(filters) + m.addTypedFilters(PointTable, filters) return &tagMatcher{ mappings: mappings, tables: m.tables(PointTable), @@ -20,7 +22,9 @@ func (m *Mapping) PointMatcher() NodeMatcher { func (m *Mapping) LineStringMatcher() WayMatcher { mappings := make(TagTables) m.mappings(LineStringTable, mappings) - filters := m.ElementFilters() + filters := make(tableFilters) + m.addFilters(filters) + m.addTypedFilters(LineStringTable, filters) return &tagMatcher{ mappings: mappings, tables: m.tables(LineStringTable), @@ -32,7 +36,9 @@ func (m *Mapping) LineStringMatcher() WayMatcher { func (m *Mapping) PolygonMatcher() RelWayMatcher { mappings := make(TagTables) m.mappings(PolygonTable, mappings) - filters := m.ElementFilters() + filters := make(tableFilters) + m.addFilters(filters) + m.addTypedFilters(PolygonTable, filters) return &tagMatcher{ mappings: mappings, tables: m.tables(PolygonTable), @@ -44,7 +50,10 @@ func (m *Mapping) PolygonMatcher() RelWayMatcher { func (m *Mapping) RelationMatcher() RelationMatcher { mappings := make(TagTables) m.mappings(RelationTable, mappings) - filters := m.ElementFilters() + filters := make(tableFilters) + m.addFilters(filters) + m.addTypedFilters(PolygonTable, filters) + m.addTypedFilters(RelationTable, filters) return &tagMatcher{ mappings: mappings, tables: m.tables(RelationTable), @@ -56,7 +65,9 @@ func (m *Mapping) RelationMatcher() RelationMatcher { func (m *Mapping) RelationMemberMatcher() RelationMatcher { mappings := make(TagTables) m.mappings(RelationMemberTable, mappings) - filters := m.ElementFilters() + filters := make(tableFilters) + m.addFilters(filters) + m.addTypedFilters(RelationMemberTable, filters) return &tagMatcher{ mappings: mappings, tables: m.tables(RelationMemberTable),