Refactor of middle: Remove slim middle class and change middle API

This commit changes the way the middle is called. The main interface for
feeding data to the middle are now the functions node/way/relation which
are called for every object, regardless of whether it is new, changed,
or deleted. The middle can then figure out itself how to handle these.
This will allow for possible optimizations when the middle can handle
an update better than a delete and create.

This also makes it natural to remove the slim_middle_t class, its
functionality is now rolled into the middle_t class. This makes the
generic middle API independent of whether we are using slim mode or not,
allowing middles which behave as slim or not depending on their own
criteria.

More refactoring based on this in later commits.

Author: joto

src/middle-pgsql.cpp

@@ -314,6 +314,46 @@ std::size_t middle_query_pgsql_t::get_way_node_locations_db(
     return count;
 }
 
+void middle_pgsql_t::node(osmium::Node const &node)
+{
+    if (node.deleted()) {
+        node_delete(node.id());
+    } else {
+        if (m_options->append) {
+            node_delete(node.id());
+            node_set(node);
+        } else {
+            node_set(node);
+        }
+    }
+}
+
+void middle_pgsql_t::way(osmium::Way const &way) {
+    if (way.deleted()) {
+        way_delete(way.id());
+    } else {
+        if (m_options->append) {
+            way_delete(way.id());
+            way_set(way);
+        } else {
+            way_set(way);
+        }
+    }
+}
+
+void middle_pgsql_t::relation(osmium::Relation const &relation) {
+    if (relation.deleted()) {
+        relation_delete(relation.id());
+    } else {
+        if (m_options->append) {
+            relation_delete(relation.id());
+            relation_set(relation);
+        } else {
+            relation_set(relation);
+        }
+    }
+}
+
 void middle_pgsql_t::node_set(osmium::Node const &node)
 {
     m_cache->set(node.id(), node.location());

src/middle-pgsql.hpp

@@ -54,7 +54,7 @@ struct table_sql {
     char const *create_fw_dep_indexes = "";
 };
 
-struct middle_pgsql_t : public slim_middle_t
+struct middle_pgsql_t : public middle_t
 {
     explicit middle_pgsql_t(options_t const *options);
 
@@ -63,14 +63,9 @@ struct middle_pgsql_t : public slim_middle_t
     void analyze() override;
     void commit() override;
 
-    void node_set(osmium::Node const &node) override;
-    void node_delete(osmid_t id) override;
-
-    void way_set(osmium::Way const &way) override;
-    void way_delete(osmid_t id) override;
-
-    void relation_set(osmium::Relation const &rel) override;
-    void relation_delete(osmid_t id) override;
+    void node(osmium::Node const &node) override;
+    void way(osmium::Way const &way) override;
+    void relation(osmium::Relation const &rel) override;
 
     void flush() override;
 
@@ -122,6 +117,15 @@ struct middle_pgsql_t : public slim_middle_t
         NUM_TABLES
     };
 
+    void node_set(osmium::Node const &node);
+    void node_delete(osmid_t id);
+
+    void way_set(osmium::Way const &way);
+    void way_delete(osmid_t id);
+
+    void relation_set(osmium::Relation const &rel);
+    void relation_delete(osmid_t id);
+
     void buffer_store_tags(osmium::OSMObject const &obj, bool attrs);
 
     idlist_t get_ids(const char* stmt, osmid_t osm_id);

src/middle-ram.cpp

@@ -30,18 +30,21 @@
  *
  */
 
-void middle_ram_t::node_set(osmium::Node const &node)
+void middle_ram_t::node(osmium::Node const &node)
 {
+    assert(node.visible());
     m_cache->set(node.id(), node.location());
 }
 
-void middle_ram_t::way_set(osmium::Way const &way)
+void middle_ram_t::way(osmium::Way const &way)
 {
+    assert(way.visible());
     m_ways.set(way.id(), new ramWay{way, m_extra_attributes});
 }
 
-void middle_ram_t::relation_set(osmium::Relation const &rel)
+void middle_ram_t::relation(osmium::Relation const &rel)
 {
+    assert(rel.visible());
     m_rels.set(rel.id(), new ramRel{rel, m_extra_attributes});
 }
 

src/middle-ram.hpp

@@ -99,17 +99,18 @@ struct middle_ram_t : public middle_t, public middle_query_t
     void analyze() override {}
     void commit() override {}
 
-    void node_set(osmium::Node const &node) override;
+    void node(osmium::Node const &node) override;
+    void way(osmium::Way const &way) override;
+    void relation(osmium::Relation const &rel) override;
+
     size_t nodes_get_list(osmium::WayNodeList *nodes) const override;
 
-    void way_set(osmium::Way const &way) override;
     bool way_get(osmid_t id, osmium::memory::Buffer &buffer) const override;
     size_t rel_way_members_get(osmium::Relation const &rel, rolelist_t *roles,
                                osmium::memory::Buffer &buffer) const override;
 
     bool relation_get(osmid_t id,
                       osmium::memory::Buffer &buffer) const override;
-    void relation_set(osmium::Relation const &rel) override;
 
     void flush() override {}
 

src/middle.hpp

@@ -1,15 +1,8 @@
 #ifndef OSM2PGSQL_MIDDLE_HPP
 #define OSM2PGSQL_MIDDLE_HPP
 
-/**
- * Common middle layer interface
- * Each middle layer data store must provide methods for
- * storing and retrieving node and way data.
- */
-
 #include <osmium/memory/buffer.hpp>
 
-#include <cstddef>
 #include <memory>
 
 #include "osmtypes.hpp"
@@ -70,10 +63,8 @@ struct middle_query_t : std::enable_shared_from_this<middle_query_t>
 inline middle_query_t::~middle_query_t() = default;
 
 /**
- * Interface for storing raw OSM data in an intermediate object store.
- *
- * This interface only allows for setting OSM data once, not changing it.
- * If you need updates and deletions, look at the derived class slim_middle_t.
+ * Interface for storing "raw" OSM data in an intermediate object store and
+ * getting it back.
  */
 struct middle_t
 {
@@ -84,23 +75,14 @@ struct middle_t
     virtual void analyze() = 0;
     virtual void commit() = 0;
 
-    /**
-     * Add a node to data storage. The node must not already be in the
-     * data storage.
-     */
-    virtual void node_set(osmium::Node const &node) = 0;
+    /// This is called for every added, changed or deleted node.
+    virtual void node(osmium::Node const &node) = 0;
 
-    /**
-     * Add a way to data storage. The way must not already be in the data
-     * storage.
-     */
-    virtual void way_set(osmium::Way const &way) = 0;
+    /// This is called for every added, changed or deleted way.
+    virtual void way(osmium::Way const &way) = 0;
 
-    /**
-     * Add a relation to data storage. The way must not already be in the
-     * data storage.
-     */
-    virtual void relation_set(osmium::Relation const &rel) = 0;
+    /// This is called for every added, changed or deleted relation.
+    virtual void relation(osmium::Relation const &relation) = 0;
 
     /**
      * Ensure all pending data is written to the storage.
@@ -122,32 +104,4 @@ struct middle_t
 
 inline middle_t::~middle_t() = default;
 
-/**
- * Extends the middle_t interface to allow updates and deletions of objects.
- */
-struct slim_middle_t : public middle_t
-{
-    ~slim_middle_t() override = 0;
-
-    /**
-     * Delete a node from data storage. Either because you want it removed
-     * entirely or before you can node_set() a new version of it.
-     */
-    virtual void node_delete(osmid_t id) = 0;
-
-    /**
-     * Delete a way from data storage. Either because you want it removed
-     * entirely or before you can way_set() a new version of it.
-     */
-    virtual void way_delete(osmid_t id) = 0;
-
-    /**
-     * Delete a relation from data storage. Either because you want it removed
-     * entirely or before you can relation_set() a new version of it.
-     */
-    virtual void relation_delete(osmid_t id) = 0;
-};
-
-inline slim_middle_t::~slim_middle_t() = default;
-
 #endif // OSM2PGSQL_MIDDLE_HPP

src/osmdata.cpp

@@ -34,21 +34,12 @@ osmdata_t::osmdata_t(std::unique_ptr<dependency_manager_t> dependency_manager,
     assert(!m_outs.empty());
 }
 
-/**
- * For modify and delete member functions a middle_t is not enough, an object
- * of the derived class slim_middle_t is needed. This function does the
- * conversion. It should always succeed, because the modify and delete
- * functions are never called for non-slim middles.
- */
-slim_middle_t &osmdata_t::slim_middle() const noexcept
-{
-    auto *slim = dynamic_cast<slim_middle_t *>(m_mid.get());
-    assert(slim);
-    return *slim;
-}
-
 void osmdata_t::node(osmium::Node const &node)
 {
+    if (!m_bbox.valid() || node.deleted() || m_bbox.contains(node.location())) {
+        m_mid->node(node);
+    }
+
     if (node.deleted()) {
         node_delete(node.id());
     } else {
@@ -75,6 +66,8 @@ void osmdata_t::after_nodes() { flush(); }
 
 void osmdata_t::way(osmium::Way &way)
 {
+    m_mid->way(way);
+
     if (way.deleted()) {
         way_delete(way.id());
     } else {
@@ -90,6 +83,14 @@ void osmdata_t::after_ways() { flush(); }
 
 void osmdata_t::relation(osmium::Relation const &rel)
 {
+    if (m_append && !rel.deleted()) {
+        for (auto const &out : m_outs) {
+            out->select_relation_members(rel.id());
+        }
+    }
+
+    m_mid->relation(rel);
+
     if (rel.deleted()) {
         relation_delete(rel.id());
     } else {
@@ -108,8 +109,6 @@ void osmdata_t::after_relations() { flush(); }
 
 void osmdata_t::node_add(osmium::Node const &node) const
 {
-    m_mid->node_set(node);
-
     if (m_with_extra_attrs || !node.tags().empty()) {
         for (auto const &out : m_outs) {
             out->node_add(node);
@@ -119,8 +118,6 @@ void osmdata_t::node_add(osmium::Node const &node) const
 
 void osmdata_t::way_add(osmium::Way *way) const
 {
-    m_mid->way_set(*way);
-
     if (m_with_extra_attrs || !way->tags().empty()) {
         for (auto const &out : m_outs) {
             out->way_add(way);
@@ -130,8 +127,6 @@ void osmdata_t::way_add(osmium::Way *way) const
 
 void osmdata_t::relation_add(osmium::Relation const &rel) const
 {
-    m_mid->relation_set(rel);
-
     if (m_with_extra_attrs || !rel.tags().empty()) {
         for (auto const &out : m_outs) {
             out->relation_add(rel);
@@ -141,11 +136,6 @@ void osmdata_t::relation_add(osmium::Relation const &rel) const
 
 void osmdata_t::node_modify(osmium::Node const &node) const
 {
-    auto &slim = slim_middle();
-
-    slim.node_delete(node.id());
-    slim.node_set(node);
-
     for (auto const &out : m_outs) {
         out->node_modify(node);
     }
@@ -155,11 +145,6 @@ void osmdata_t::node_modify(osmium::Node const &node) const
 
 void osmdata_t::way_modify(osmium::Way *way) const
 {
-    auto &slim = slim_middle();
-
-    slim.way_delete(way->id());
-    slim.way_set(*way);
-
     for (auto const &out : m_outs) {
         out->way_modify(way);
     }
@@ -169,15 +154,6 @@ void osmdata_t::way_modify(osmium::Way *way) const
 
 void osmdata_t::relation_modify(osmium::Relation const &rel) const
 {
-    auto &slim = slim_middle();
-
-    for (auto const &out : m_outs) {
-        out->select_relation_members(rel.id());
-    }
-
-    slim.relation_delete(rel.id());
-    slim.relation_set(rel);
-
     for (auto const &out : m_outs) {
         out->relation_modify(rel);
     }
@@ -188,26 +164,20 @@ void osmdata_t::node_delete(osmid_t id) const
     for (auto const &out : m_outs) {
         out->node_delete(id);
     }
-
-    slim_middle().node_delete(id);
 }
 
 void osmdata_t::way_delete(osmid_t id) const
 {
     for (auto const &out : m_outs) {
         out->way_delete(id);
     }
-
-    slim_middle().way_delete(id);
 }
 
 void osmdata_t::relation_delete(osmid_t id) const
 {
     for (auto const &out : m_outs) {
         out->relation_delete(id);
     }
-
-    slim_middle().relation_delete(id);
 }
 
 void osmdata_t::start() const

src/osmdata.hpp

@@ -22,7 +22,6 @@
 class options_t;
 class output_t;
 struct middle_t;
-struct slim_middle_t;
 
 /**
  * This class guides the processing of the OSM data through its multiple
@@ -84,8 +83,6 @@ class osmdata_t : public osmium::handler::Handler
      */
     void postprocess_database() const;
 
-    slim_middle_t &slim_middle() const noexcept;
-
     std::unique_ptr<dependency_manager_t> m_dependency_manager;
     std::shared_ptr<middle_t> m_mid;
     std::vector<std::shared_ptr<output_t>> m_outs;