Merge pull request #2129 from joto/connection-params

Store db connection parameters in a their own class

src/command-line-app.cpp

@@ -32,29 +32,44 @@ bool command_line_app_t::want_version() const { return count("--version"); }
 
 void command_line_app_t::init_database_options()
 {
-    add_option("-d,--database", m_database_options.db)
+    add_option_function<std::string>("-d,--database",
+                                     [&](std::string const &value) {
+                                         m_connection_params.set("dbname",
+                                                                 value);
+                                     })
         ->description("Database name or PostgreSQL conninfo string.")
         ->type_name("DB")
         ->group("Database options");
 
-    add_option("-U,--user", m_database_options.username)
+    add_option_function<std::string>("-U,--user",
+                                     [&](std::string const &value) {
+                                         m_connection_params.set("user", value);
+                                     })
         ->description("Database user.")
         ->type_name("USERNAME")
         ->group("Database options");
 
-    add_flag_function(
-        "-W,--password",
-        [&](int64_t) { m_database_options.password = util::get_password(); })
+    add_flag_function("-W,--password",
+                      [&](int64_t) {
+                          m_connection_params.set("password",
+                                                  util::get_password());
+                      })
         ->description("Force password prompt.")
         ->group("Database options");
 
-    add_option("-H,--host", m_database_options.host)
+    add_option_function<std::string>("-H,--host",
+                                     [&](std::string const &value) {
+                                         m_connection_params.set("host", value);
+                                     })
         ->description(
             "Database server hostname or unix domain socket location.")
         ->type_name("HOST")
         ->group("Database options");
 
-    add_option("-P,--port", m_database_options.port)
+    add_option_function<std::string>("-P,--port",
+                                     [&](std::string const &value) {
+                                         m_connection_params.set("port", value);
+                                     })
         ->description("Database server port.")
         ->type_name("PORT")
         ->group("Database options");

src/command-line-app.hpp

@@ -26,13 +26,13 @@ class command_line_app_t : public CLI::App
 
     bool want_version() const;
 
-    database_options_t database_options() const noexcept
+    connection_params_t connection_params() const noexcept
     {
-        return m_database_options;
+        return m_connection_params;
     }
 
 private:
-    database_options_t m_database_options;
+    connection_params_t m_connection_params;
 
     void init_database_options();
     void init_logging_options();

src/command-line-parser.cpp

@@ -32,48 +32,6 @@
 #include <stdexcept>
 #include <thread> // for number of threads
 
-static bool compare_prefix(std::string const &str,
-                           std::string const &prefix) noexcept
-{
-    return std::strncmp(str.c_str(), prefix.c_str(), prefix.size()) == 0;
-}
-
-std::string build_conninfo(database_options_t const &opt)
-{
-    if (compare_prefix(opt.db, "postgresql://") ||
-        compare_prefix(opt.db, "postgres://")) {
-        return opt.db;
-    }
-
-    util::string_joiner_t joiner{' '};
-    joiner.add("fallback_application_name='osm2pgsql'");
-
-    if (std::strchr(opt.db.c_str(), '=') != nullptr) {
-        joiner.add(opt.db);
-        return joiner();
-    }
-
-    joiner.add("client_encoding='UTF8'");
-
-    if (!opt.db.empty()) {
-        joiner.add(fmt::format("dbname='{}'", opt.db));
-    }
-    if (!opt.username.empty()) {
-        joiner.add(fmt::format("user='{}'", opt.username));
-    }
-    if (!opt.password.empty()) {
-        joiner.add(fmt::format("password='{}'", opt.password));
-    }
-    if (!opt.host.empty()) {
-        joiner.add(fmt::format("host='{}'", opt.host));
-    }
-    if (!opt.port.empty()) {
-        joiner.add(fmt::format("port='{}'", opt.port));
-    }
-
-    return joiner();
-}
-
 static osmium::Box parse_bbox_param(std::string const &arg)
 {
     double minx = NAN;
@@ -730,7 +688,7 @@ options_t parse_command_line(int argc, char *argv[])
 
     check_options(&options);
 
-    options.conninfo = build_conninfo(app.database_options());
+    options.connection_params = app.connection_params();
 
     if (!options.slim) {
         options.middle_database_format = 0;

src/db-copy.cpp

@@ -81,11 +81,12 @@ void db_deleter_by_type_and_id_t::delete_rows(std::string const &table,
     conn->exec(sql.data());
 }
 
-db_copy_thread_t::db_copy_thread_t(std::string const &conninfo)
+db_copy_thread_t::db_copy_thread_t(connection_params_t const &connection_params)
 {
-    // conninfo is captured by copy here, because we don't know wether the
-    // reference will still be valid once we get around to running the thread
-    m_worker = std::thread{thread_t{conninfo, &m_shared}};
+    // Connection params are captured by copy here, because we don't know
+    // whether the reference will still be valid once we get around to running
+    // the thread.
+    m_worker = std::thread{thread_t{connection_params, &m_shared}};
 }
 
 db_copy_thread_t::~db_copy_thread_t() { finish(); }
@@ -119,14 +120,15 @@ void db_copy_thread_t::finish()
     }
 }
 
-db_copy_thread_t::thread_t::thread_t(std::string conninfo, shared *shared)
-: m_conninfo(std::move(conninfo)), m_shared(shared)
+db_copy_thread_t::thread_t::thread_t(connection_params_t connection_params,
+                                     shared *shared)
+: m_connection_params(std::move(connection_params)), m_shared(shared)
 {}
 
 void db_copy_thread_t::thread_t::operator()()
 {
     try {
-        m_conn = std::make_unique<pg_conn_t>(m_conninfo);
+        m_conn = std::make_unique<pg_conn_t>(m_connection_params);
 
         // Let commits happen faster by delaying when they actually occur.
         m_conn->exec("SET synchronous_commit = off");

src/db-copy.hpp

@@ -249,7 +249,7 @@ struct db_cmd_finish_t : public db_cmd_t
 class db_copy_thread_t
 {
 public:
-    explicit db_copy_thread_t(std::string const &conninfo);
+    explicit db_copy_thread_t(connection_params_t const &connection_params);
 
     db_copy_thread_t(db_copy_thread_t const &) = delete;
     db_copy_thread_t &operator=(db_copy_thread_t const &) = delete;
@@ -290,7 +290,7 @@ class db_copy_thread_t
     class thread_t
     {
     public:
-        thread_t(std::string conninfo, shared *shared);
+        thread_t(connection_params_t connection_params, shared *shared);
 
         void operator()();
 
@@ -300,7 +300,7 @@ class db_copy_thread_t
         void finish_copy();
         void delete_rows(db_cmd_copy_t *buffer);
 
-        std::string m_conninfo;
+        connection_params_t m_connection_params;
         std::unique_ptr<pg_conn_t> m_conn;
 
         // Target for copy operation currently ongoing.

src/expire-output.cpp

@@ -16,15 +16,16 @@
 
 #include <system_error>
 
-std::size_t expire_output_t::output(quadkey_list_t const &tile_list,
-                                    std::string const &conninfo) const
+std::size_t
+expire_output_t::output(quadkey_list_t const &tile_list,
+                        connection_params_t const &connection_params) const
 {
     std::size_t num = 0;
     if (!m_filename.empty()) {
         num = output_tiles_to_file(tile_list);
     }
     if (!m_table.empty()) {
-        num = output_tiles_to_table(tile_list, conninfo);
+        num = output_tiles_to_table(tile_list, connection_params);
     }
     return num;
 }
@@ -51,13 +52,13 @@ std::size_t expire_output_t::output_tiles_to_file(
     return count;
 }
 
-std::size_t
-expire_output_t::output_tiles_to_table(quadkey_list_t const &tiles_at_maxzoom,
-                                       std::string const &conninfo) const
+std::size_t expire_output_t::output_tiles_to_table(
+    quadkey_list_t const &tiles_at_maxzoom,
+    connection_params_t const &connection_params) const
 {
     auto const qn = qualified_name(m_schema, m_table);
 
-    pg_conn_t connection{conninfo};
+    pg_conn_t connection{connection_params};
 
     auto const result = connection.exec("SELECT * FROM {} LIMIT 1", qn);
 

src/expire-output.hpp

@@ -18,6 +18,7 @@
 #include <utility>
 
 class pg_conn_t;
+class connection_params_t;
 
 /**
  * Output for tile expiry.
@@ -52,7 +53,7 @@ class expire_output_t
     void set_maxzoom(uint32_t maxzoom) noexcept { m_maxzoom = maxzoom; }
 
     std::size_t output(quadkey_list_t const &tile_list,
-                       std::string const &conninfo) const;
+                       connection_params_t const &connection_params) const;
 
     /**
      * Write the list of tiles to a file.
@@ -66,10 +67,11 @@ class expire_output_t
      * Write the list of tiles to a database table.
      *
      * \param tiles_at_maxzoom The list of tiles at maximum zoom level
-     * \param conninfo database connection info
+     * \param connection_params Database connection parameters
      */
-    std::size_t output_tiles_to_table(quadkey_list_t const &tiles_at_maxzoom,
-                                      std::string const &conninfo) const;
+    std::size_t
+    output_tiles_to_table(quadkey_list_t const &tiles_at_maxzoom,
+                          connection_params_t const &connection_params) const;
 
     /**
      * Create table for tiles.

src/flex-table.cpp

@@ -241,11 +241,11 @@ bool flex_table_t::has_columns_with_expire() const noexcept
                        [](auto const &column) { return column.has_expire(); });
 }
 
-void table_connection_t::connect(std::string const &conninfo)
+void table_connection_t::connect(connection_params_t const &connection_params)
 {
     assert(!m_db_connection);
 
-    m_db_connection = std::make_unique<pg_conn_t>(conninfo);
+    m_db_connection = std::make_unique<pg_conn_t>(connection_params);
     m_db_connection->exec("SET synchronous_commit = off");
 }
 

src/flex-table.hpp

@@ -259,7 +259,7 @@ class table_connection_t
     {
     }
 
-    void connect(std::string const &conninfo);
+    void connect(connection_params_t const &connection_params);
 
     void start(bool append);