move object deletion into its own class

src/db-copy.cpp

@@ -4,6 +4,7 @@
 #include <thread>
 
 #include "db-copy.hpp"
+#include "format.hpp"
 #include "pgsql.hpp"
 
 db_copy_thread_t::db_copy_thread_t(std::string const &conninfo)
@@ -103,13 +104,14 @@ void db_copy_thread_t::disconnect() { m_conn.reset(); }
 
 void db_copy_thread_t::write_to_db(db_cmd_copy_t *buffer)
 {
-    if (!buffer->deletables.empty() ||
+    if (buffer->deleter.has_data() ||
         (m_inflight && !buffer->target->same_copy_target(*m_inflight.get()))) {
         finish_copy();
     }
 
-    if (!buffer->deletables.empty()) {
-        delete_rows(buffer);
+    if (buffer->deleter.has_data()) {
+        buffer->deleter.delete_rows(buffer->target->name, buffer->target->id,
+                                    m_conn.get());
     }
 
     if (!m_inflight) {
@@ -119,24 +121,19 @@ void db_copy_thread_t::write_to_db(db_cmd_copy_t *buffer)
     m_conn->copy_data(buffer->buffer, buffer->target->name);
 }
 
-void db_copy_thread_t::delete_rows(db_cmd_copy_t *buffer)
+void db_deleter_by_id_t::delete_rows(std::string const &table,
+                                     std::string const &column, pg_conn_t *conn)
 {
-    assert(!m_inflight);
+    std::string sql = "DELETE FROM {} WHERE {} IN ("_format(table, column);
+    sql.reserve(m_deletables.size() * 15 + sql.size());
 
-    std::string sql = "DELETE FROM ";
-    sql.reserve(buffer->target->name.size() + buffer->deletables.size() * 15 +
-                30);
-    sql += buffer->target->name;
-    sql += " WHERE ";
-    sql += buffer->target->id;
-    sql += " IN (";
-    for (auto id : buffer->deletables) {
-        sql += std::to_string(id);
+    for (auto id : m_deletables) {
+        sql += fmt::to_string(id);
         sql += ',';
     }
     sql[sql.size() - 1] = ')';
 
-    m_conn->exec(sql);
+    conn->exec(sql);
 }
 
 void db_copy_thread_t::start_copy(
@@ -184,7 +181,7 @@ void db_copy_mgr_t::new_line(std::shared_ptr<db_target_descr_t> const &table)
 void db_copy_mgr_t::delete_id(osmid_t osm_id)
 {
     assert(m_current);
-    m_current->deletables.push_back(osm_id);
+    m_current->deleter.add(osm_id);
 }
 
 void db_copy_mgr_t::sync()

src/db-copy.hpp

@@ -40,6 +40,24 @@ struct db_target_descr_t
     {}
 };
 
+/**
+ * Deleter which removes objects by id from the database.
+ */
+class db_deleter_by_id_t
+{
+public:
+    bool has_data() const noexcept { return !m_deletables.empty(); }
+
+    void add(osmid_t osm_id) { m_deletables.push_back(osm_id); }
+
+    void delete_rows(std::string const &table, std::string const &column,
+                     pg_conn_t *conn);
+
+private:
+    /// Vector with object to delete before copying
+    std::vector<osmid_t> m_deletables;
+};
+
 /**
  * A command for the copy thread to execute.
  */
@@ -82,10 +100,10 @@ struct db_cmd_copy_t : public db_cmd_t
     };
     /// Name of the target table for the copy operation
     std::shared_ptr<db_target_descr_t> target;
-    /// Vector with object to delete before copying
-    std::vector<osmid_t> deletables;
     /// actual copy buffer
     std::string buffer;
+    /// Deleter class for old items
+    db_deleter_by_id_t deleter;
 
     explicit db_cmd_copy_t(std::shared_ptr<db_target_descr_t> const &t)
     : db_cmd_t(db_cmd_t::Cmd_copy), target(t)

tests/test-db-copy-thread.cpp

@@ -70,8 +70,8 @@ TEST_CASE("db_copy_thread_t")
 
         SECTION("simple delete of existing rows")
         {
-            cmd->deletables.push_back(223);
-            cmd->deletables.push_back(42);
+            cmd->deleter.add(223);
+            cmd->deleter.add(42);
 
             t.add_buffer(std::unique_ptr<db_cmd_t>(cmd.release()));
             t.sync_and_wait();
@@ -82,7 +82,7 @@ TEST_CASE("db_copy_thread_t")
 
         SECTION("delete one and add another")
         {
-            cmd->deletables.push_back(133);
+            cmd->deleter.add(133);
             cmd->buffer += "134\n";
 
             t.add_buffer(std::unique_ptr<db_cmd_t>(cmd.release()));
@@ -94,7 +94,7 @@ TEST_CASE("db_copy_thread_t")
 
         SECTION("delete one and add the same")
         {
-            cmd->deletables.push_back(133);
+            cmd->deleter.add(133);
             cmd->buffer += "133\n";
 
             t.add_buffer(std::unique_ptr<db_cmd_t>(cmd.release()));
@@ -126,7 +126,7 @@ TEST_CASE("db_copy_thread_t")
         t.add_buffer(std::unique_ptr<db_cmd_t>(cmd.release()));
 
         cmd = std::unique_ptr<db_cmd_copy_t>(new db_cmd_copy_t(table));
-        cmd->deletables.push_back(542);
+        cmd->deleter.add(542);
         cmd->buffer += "12\n";
         t.add_buffer(std::unique_ptr<db_cmd_t>(cmd.release()));