gazetteer: move main tag filtering into processing function

lonvia · lonvia · commit 6846d98c392b · 2019-12-07T09:38:58.000+01:00
Filtering of main tags (i.e. handling of named-only tags or
fallback tags) must happen before the actual output because
the filtered list is already needed when computing the list
of classes to keep. Currently the function that creates the
class list does its own filtering and is inconsisten with
what the output function does.
diff --git a/src/gazetteer-style.cpp b/src/gazetteer-style.cpp
@@ -9,28 +9,33 @@
 #include "pgsql.hpp"
 #include "wkb.hpp"
 
+namespace {
+
 enum : int
 {
     MAX_ADMINLEVEL = 15
 };
 
-static std::vector<osmium::Tag const *>
-domain_names(char const *cls, osmium::TagList const &tags)
+class DomainMatcher
 {
-    std::vector<osmium::Tag const *> ret;
-
-    std::string const prefix = cls + std::string(":name");
-    auto plen = prefix.length();
+public:
+    DomainMatcher(char const *cls) : m_domain(cls), m_len(strlen(cls)) {}
 
-    for (auto const &item : tags) {
-        char const *k = item.key();
-        if (prefix.compare(0, plen, k) == 0 &&
-            (k[plen] == '\0' || k[plen] == ':')) {
-            ret.push_back(&item);
+    char const *operator()(osmium::Tag const &t) const noexcept
+    {
+        if (strncmp(t.key(), m_domain, m_len) == 0 &&
+            strncmp(t.key() + m_len, ":name", 5) == 0 &&
+            (t.key()[m_len + 5] == '\0' || t.key()[m_len + 5] == ':')) {
+            return t.key() + m_len + 6;
         }
+
+        return nullptr;
     }
 
-    return ret;
+private:
+    char const *m_domain;
+    size_t m_len;
+};
 }
 
 namespace pt = boost::property_tree;
@@ -59,18 +64,14 @@ void gazetteer_style_t::clear()
     m_address.clear();
     m_operator = nullptr;
     m_admin_level = MAX_ADMINLEVEL;
-    m_is_named = false;
 }
 
 std::string gazetteer_style_t::class_list() const
 {
     fmt::memory_buffer buf;
 
     for (auto const &m : m_main) {
-        // XXX should handle SF_MAIN_NAMED_KEY as well
-        if (!(std::get<2>(m) & SF_MAIN_NAMED) || !m_names.empty()) {
-            fmt::format_to(buf, FMT_STRING("'{}',"), std::get<0>(m));
-        }
+        fmt::format_to(buf, FMT_STRING("'{}',"), std::get<0>(m));
     }
 
     if (buf.size() > 0) {
@@ -284,14 +285,15 @@ void gazetteer_style_t::process_tags(osmium::OSMObject const &o)
 {
     clear();
 
-    char const *postcode = nullptr;
-    char const *country = nullptr;
+    bool has_postcode = false;
+    bool has_country = false;
     char const *place = nullptr;
     flag_t place_flag;
     bool address_point = false;
     bool interpolation = false;
     bool admin_boundary = false;
     bool postcode_fallback = false;
+    bool is_named = false;
 
     for (auto const &item : o.tags()) {
         char const *k = item.key();
@@ -330,7 +332,7 @@ void gazetteer_style_t::process_tags(osmium::OSMObject const &o)
         if (flag & (SF_NAME | SF_REF)) {
             m_names.emplace_back(k, v);
             if (flag & SF_NAME) {
-                m_is_named = true;
+                is_named = true;
             }
         }
 
@@ -344,39 +346,31 @@ void gazetteer_style_t::process_tags(osmium::OSMObject const &o)
                 addr_key = k;
             }
 
-            if (strcmp(addr_key, "postcode") == 0) {
-                if (!postcode) {
-                    postcode = v;
-                }
-            } else if (strcmp(addr_key, "country") == 0) {
-                if (!country && strlen(v) == 2) {
-                    country = v;
-                }
-            } else {
-                bool first = std::none_of(
-                    m_address.begin(), m_address.end(), [&](ptag_t const &t) {
-                        return strcmp(t.first, addr_key) == 0;
-                    });
-                if (first) {
-                    m_address.emplace_back(addr_key, v);
-                }
+            bool first = std::none_of(m_address.begin(), m_address.end(),
+                                      [&](ptag_t const &t) {
+                                          return strcmp(t.first, addr_key) == 0;
+                                      });
+            if (first) {
+                m_address.emplace_back(addr_key, v);
             }
         }
 
         if (flag & SF_ADDRESS_POINT) {
             address_point = true;
-            m_is_named = true;
+            is_named = true;
         }
 
-        if ((flag & SF_POSTCODE) && !postcode) {
-            postcode = v;
+        if ((flag & SF_POSTCODE) && !has_postcode) {
+            has_postcode = true;
+            m_address.emplace_back("postcode", v);
             if (flag & SF_MAIN_FALLBACK) {
                 postcode_fallback = true;
             }
         }
 
-        if ((flag & SF_COUNTRY) && !country && std::strlen(v) == 2) {
-            country = v;
+        if ((flag & SF_COUNTRY) && !has_country && std::strlen(v) == 2) {
+            has_country = true;
+            m_address.emplace_back("country", v);
         }
 
         if (flag & SF_EXTRA) {
@@ -389,12 +383,6 @@ void gazetteer_style_t::process_tags(osmium::OSMObject const &o)
         }
     }
 
-    if (postcode) {
-        m_address.emplace_back("postcode", postcode);
-    }
-    if (country) {
-        m_address.emplace_back("country", country);
-    }
     if (place) {
         if (interpolation || (admin_boundary && strncmp(place, "isl", 3) !=
                                                     0)) { // island or islet
@@ -403,56 +391,75 @@ void gazetteer_style_t::process_tags(osmium::OSMObject const &o)
             m_main.emplace_back("place", place, place_flag);
         }
     }
-    if (address_point) {
-        m_main.emplace_back("place", "house", SF_MAIN | SF_MAIN_FALLBACK);
-    } else if (postcode_fallback && postcode) {
-        m_main.emplace_back("place", "postcode", SF_MAIN | SF_MAIN_FALLBACK);
+
+    filter_main_tags(is_named, o.tags());
+
+    if (m_main.empty()) {
+        if (address_point) {
+            m_main.emplace_back("place", "house", SF_MAIN | SF_MAIN_FALLBACK);
+        } else if (postcode_fallback && has_postcode) {
+            m_main.emplace_back("place", "postcode",
+                                SF_MAIN | SF_MAIN_FALLBACK);
+        }
     }
 }
 
-bool gazetteer_style_t::copy_out(osmium::OSMObject const &o,
-                                 std::string const &geom, copy_mgr_t &buffer)
+void gazetteer_style_t::filter_main_tags(bool is_named,
+                                         osmium::TagList const &tags)
 {
-    bool any = false;
-    for (auto const &main : m_main) {
-        if (!(std::get<2>(main) & SF_MAIN_FALLBACK)) {
-            any |= copy_out_maintag(main, o, geom, buffer);
-        }
-    }
+    // first throw away unnamed mains
+    auto mend =
+        std::remove_if(m_main.begin(), m_main.end(), [&](pmaintag_t const &t) {
+            auto flags = std::get<2>(t);
+
+            if (flags & SF_MAIN_NAMED) {
+                return !is_named;
+            }
 
-    if (any) {
-        return true;
+            if (flags & SF_MAIN_NAMED_KEY) {
+                return !std::any_of(tags.begin(), tags.end(),
+                                    DomainMatcher(std::get<0>(t)));
+            }
+
+            return false;
+        });
+
+    // any non-fallback mains left?
+    bool has_primary =
+        std::any_of(m_main.begin(), mend, [](pmaintag_t const &t) {
+            return !(std::get<2>(t) & SF_MAIN_FALLBACK);
+        });
+
+    if (has_primary) {
+        // remove all fallbacks
+        mend = std::remove_if(m_main.begin(), mend, [&](pmaintag_t const &t) {
+            return (std::get<2>(t) & SF_MAIN_FALLBACK);
+        });
+        m_main.erase(mend, m_main.end());
+    } else if (mend == m_main.begin()) {
+        m_main.clear();
+    } else {
+        // remove everything except the first entry
+        m_main.resize(1);
     }
+}
 
+bool gazetteer_style_t::copy_out(osmium::OSMObject const &o,
+                                 std::string const &geom, copy_mgr_t &buffer)
+{
     for (auto const &main : m_main) {
-        if ((std::get<2>(main) & SF_MAIN_FALLBACK) &&
-            copy_out_maintag(main, o, geom, buffer)) {
-            return true;
-        }
+        copy_out_maintag(main, o, geom, buffer);
     }
 
-    return false;
+    return !m_main.empty();
 }
 
-bool gazetteer_style_t::copy_out_maintag(pmaintag_t const &tag,
+void gazetteer_style_t::copy_out_maintag(pmaintag_t const &tag,
                                          osmium::OSMObject const &o,
                                          std::string const &geom,
                                          copy_mgr_t &buffer)
 {
-    std::vector<osmium::Tag const *> domain_name;
-    if (std::get<2>(tag) & SF_MAIN_NAMED_KEY) {
-        domain_name = domain_names(std::get<0>(tag), o.tags());
-        if (domain_name.empty()) {
-            return false;
-        }
-    }
-
-    if (std::get<2>(tag) & SF_MAIN_NAMED) {
-        if (domain_name.empty() && !m_is_named) {
-            return false;
-        }
-    }
-
+    buffer.new_line();
     // osm_id
     buffer.add_column(o.id());
     // osm_type
@@ -464,11 +471,14 @@ bool gazetteer_style_t::copy_out_maintag(pmaintag_t const &tag,
     // type
     buffer.add_column(std::get<1>(tag));
     // names
-    if (!domain_name.empty()) {
-        auto prefix_len = strlen(std::get<0>(tag)) + 1; // class name and ':'
+    if (std::get<2>(tag) & SF_MAIN_NAMED_KEY) {
+        DomainMatcher m(std::get<0>(tag));
         buffer.new_hash();
-        for (auto *t : domain_name) {
-            buffer.add_hash_elem(t->key() + prefix_len, t->value());
+        for (auto const &t : o.tags()) {
+            char const *k = m(t);
+            if (k) {
+                buffer.add_hash_elem(k, t.value());
+            }
         }
         buffer.finish_hash();
     } else {
@@ -552,6 +562,4 @@ bool gazetteer_style_t::copy_out_maintag(pmaintag_t const &tag,
     buffer.add_hex_geom(geom);
 
     buffer.finish_line();
-
-    return true;
 }
diff --git a/src/gazetteer-style.hpp b/src/gazetteer-style.hpp
@@ -53,6 +53,22 @@ class db_deleter_place_t
     std::vector<item_t> m_deletables;
 };
 
+class gazetteer_copy_mgr_t : public db_copy_mgr_t<db_deleter_place_t>
+{
+public:
+    gazetteer_copy_mgr_t(std::shared_ptr<db_copy_thread_t> const &processor)
+    : db_copy_mgr_t<db_deleter_place_t>(processor),
+      m_table(std::make_shared<db_target_descr_t>("place", "place_id"))
+    {}
+
+    using db_copy_mgr_t<db_deleter_place_t>::new_line;
+
+    void new_line() { new_line(m_table); }
+
+private:
+    std::shared_ptr<db_target_descr_t> m_table;
+};
+
 class gazetteer_style_t
 {
     using flag_t = uint16_t;
@@ -100,7 +116,7 @@ class gazetteer_style_t
     using flag_list_t = std::vector<string_with_flag_t>;
 
 public:
-    using copy_mgr_t = db_copy_mgr_t<db_deleter_place_t>;
+    using copy_mgr_t = gazetteer_copy_mgr_t;
 
     void load_style(std::string const &filename);
     void process_tags(osmium::OSMObject const &o);
@@ -116,8 +132,9 @@ class gazetteer_style_t
                          flag_t flags);
     flag_t parse_flags(std::string const &str);
     flag_t find_flag(char const *k, char const *v) const;
+    void filter_main_tags(bool is_named, osmium::TagList const &tags);
 
-    bool copy_out_maintag(pmaintag_t const &tag, osmium::OSMObject const &o,
+    void copy_out_maintag(pmaintag_t const &tag, osmium::OSMObject const &o,
                           std::string const &geom, copy_mgr_t &buffer);
     void clear();
 
@@ -140,8 +157,6 @@ class gazetteer_style_t
     char const *m_operator;
     /// admin level
     int m_admin_level;
-    /// True if there is an actual name to the object (not a ref).
-    bool m_is_named;
 
     /// which metadata fields of the OSM objects should be written to the output
     osmium::metadata_options m_metadata_fields{"none"};
diff --git a/src/output-gazetteer.cpp b/src/output-gazetteer.cpp
@@ -13,9 +13,6 @@
 #include <iostream>
 #include <memory>
 
-static auto place_table =
-    std::make_shared<db_target_descr_t>("place", "place_id");
-
 void output_gazetteer_t::delete_unused_classes(char osm_type, osmid_t osm_id)
 {
     if (!m_options.append) {
@@ -76,7 +73,7 @@ void output_gazetteer_t::commit() { m_copy.sync(); }
 
 int output_gazetteer_t::process_node(osmium::Node const &node)
 {
-    m_copy.new_line(place_table);
+    m_copy.new_line();
     m_style.process_tags(node);
     delete_unused_classes('N', node.id());
 
@@ -93,7 +90,7 @@ int output_gazetteer_t::process_node(osmium::Node const &node)
 
 int output_gazetteer_t::process_way(osmium::Way *way)
 {
-    m_copy.new_line(place_table);
+    m_copy.new_line();
     m_style.process_tags(*way);
     delete_unused_classes('W', way->id());
 
@@ -127,7 +124,7 @@ int output_gazetteer_t::process_way(osmium::Way *way)
 
 int output_gazetteer_t::process_relation(osmium::Relation const &rel)
 {
-    m_copy.new_line(place_table);
+    m_copy.new_line();
 
     auto const &tags = rel.tags();
     char const *type = tags["type"];
diff --git a/src/output-gazetteer.hpp b/src/output-gazetteer.hpp
@@ -115,7 +115,7 @@ class output_gazetteer_t : public output_t
         }
     }
 
-    db_copy_mgr_t<db_deleter_place_t> m_copy;
+    gazetteer_copy_mgr_t m_copy;
     gazetteer_style_t m_style;
 
     geom::osmium_builder_t m_builder;
diff --git a/tests/test-output-gazetteer.cpp b/tests/test-output-gazetteer.cpp

Original file line number	Diff line number	Diff line change
`@@ -115,7 +115,7 @@ class output_gazetteer_t : public output_t`
`115`	`115`	`}`
`116`	`116`	`}`
`117`	`117`
`118`		`- db_copy_mgr_t<db_deleter_place_t> m_copy;`
	`118`	`+ gazetteer_copy_mgr_t m_copy;`
`119`	`119`	`gazetteer_style_t m_style;`
`120`	`120`
`121`	`121`	`geom::osmium_builder_t m_builder;`