Refactor: Parsing and connection establishing should be member functions

This refactors the parsing of the `DbConnectionConfig` object and the
establishing of the connection to be member functions of the type, rather than
free functions. Way more idomatic.

Signed-off-by: Matthias Beyer <matthias.beyer@atos.net>

4 files changed, 54 insertions, 62 deletions

diff --git a/src/commands/db.rs b/src/commands/db.rs
index fa009db..09cd67d 100644
--- a/src/commands/db.rs
+++ b/src/commands/db.rs
@@ -144,7 +144,7 @@ fn artifacts(conn_cfg: DbConnectionConfig<'_>, matches: &ArgMatches) -> Result<(
 
     let csv = matches.is_present("csv");
     let hdrs = crate::commands::util::mk_header(vec!["id", "path", "released", "job id"]);
-    let conn = crate::db::establish_connection(conn_cfg)?;
+    let conn = conn_cfg.establish_connection()?;
     let data = matches
         .value_of("job_uuid")
         .map(uuid::Uuid::parse_str)
@@ -193,7 +193,7 @@ fn envvars(conn_cfg: DbConnectionConfig<'_>, matches: &ArgMatches) -> Result<()>
 
     let csv = matches.is_present("csv");
     let hdrs = crate::commands::util::mk_header(vec!["id", "name", "value"]);
-    let conn = crate::db::establish_connection(conn_cfg)?;
+    let conn = conn_cfg.establish_connection()?;
     let data = dsl::envvars
         .load::<models::EnvVar>(&conn)?
         .into_iter()
@@ -214,7 +214,7 @@ fn images(conn_cfg: DbConnectionConfig<'_>, matches: &ArgMatches) -> Result<()>
 
     let csv = matches.is_present("csv");
     let hdrs = crate::commands::util::mk_header(vec!["id", "name"]);
-    let conn = crate::db::establish_connection(conn_cfg)?;
+    let conn = conn_cfg.establish_connection()?;
     let data = dsl::images
         .load::<models::Image>(&conn)?
         .into_iter()
@@ -231,7 +231,7 @@ fn images(conn_cfg: DbConnectionConfig<'_>, matches: &ArgMatches) -> Result<()>
 }
 
 fn submit(conn_cfg: DbConnectionConfig<'_>, matches: &ArgMatches) -> Result<()> {
-    let conn = crate::db::establish_connection(conn_cfg)?;
+    let conn = conn_cfg.establish_connection()?;
     let submit_id = matches.value_of("submit")
         .map(uuid::Uuid::from_str)
         .transpose()
@@ -304,7 +304,7 @@ fn submits(conn_cfg: DbConnectionConfig<'_>, matches: &ArgMatches) -> Result<()>
     let csv = matches.is_present("csv");
     let limit = matches.value_of("limit").map(i64::from_str).transpose()?;
     let hdrs = crate::commands::util::mk_header(vec!["id", "time", "uuid"]);
-    let conn = crate::db::establish_connection(conn_cfg)?;
+    let conn = conn_cfg.establish_connection()?;
 
     let query = schema::submits::table
         .order_by(schema::submits::id.desc()); // required for the --limit implementation
@@ -378,7 +378,7 @@ fn jobs(conn_cfg: DbConnectionConfig<'_>, matches: &ArgMatches) -> Result<()> {
         "package",
         "version",
     ]);
-    let conn = crate::db::establish_connection(conn_cfg)?;
+    let conn = conn_cfg.establish_connection()?;
 
     let sel = schema::jobs::table
         .inner_join(schema::submits::table)
@@ -464,7 +464,7 @@ fn job(conn_cfg: DbConnectionConfig<'_>, config: &Configuration, matches: &ArgMa
     let show_log = matches.is_present("show_log");
     let show_script = matches.is_present("show_script");
     let csv = matches.is_present("csv");
-    let conn = crate::db::establish_connection(conn_cfg)?;
+    let conn = conn_cfg.establish_connection()?;
     let job_uuid = matches
         .value_of("job_uuid")
         .map(uuid::Uuid::parse_str)
@@ -629,7 +629,7 @@ fn job(conn_cfg: DbConnectionConfig<'_>, config: &Configuration, matches: &ArgMa
 
 /// Implementation of the subcommand "db log-of"
 fn log_of(conn_cfg: DbConnectionConfig<'_>, matches: &ArgMatches) -> Result<()> {
-    let conn   = crate::db::establish_connection(conn_cfg)?;
+    let conn   = conn_cfg.establish_connection()?;
     let job_uuid = matches
         .value_of("job_uuid")
         .map(uuid::Uuid::parse_str)
@@ -652,7 +652,7 @@ fn log_of(conn_cfg: DbConnectionConfig<'_>, matches: &ArgMatches) -> Result<()>
 
 fn releases(conn_cfg: DbConnectionConfig<'_>, config: &Configuration, matches: &ArgMatches) -> Result<()> {
     let csv    = matches.is_present("csv");
-    let conn   = crate::db::establish_connection(conn_cfg)?;
+    let conn   = conn_cfg.establish_connection()?;
     let header = crate::commands::util::mk_header(["Package", "Version", "Date", "Path"].to_vec());
     let data   = schema::jobs::table
         .inner_join(schema::packages::table)
diff --git a/src/commands/release.rs b/src/commands/release.rs
index ec12d30..837ebb8 100644
--- a/src/commands/release.rs
+++ b/src/commands/release.rs
@@ -58,7 +58,7 @@ async fn new_release(
 
     debug!("Release called for: {:?} {:?}", pname, pvers);
 
-    let conn = crate::db::establish_connection(db_connection_config)?;
+    let conn = db_connection_config.establish_connection()?;
     let submit_uuid = matches
         .value_of("submit_uuid")
         .map(uuid::Uuid::parse_str)
@@ -207,7 +207,7 @@ pub async fn rm_release(
     let pvers = matches.value_of("package_version").map(String::from).unwrap(); // safe by clap
     debug!("Remove Release called for: {:?} {:?}", pname, pvers);
 
-    let conn = crate::db::establish_connection(db_connection_config)?;
+    let conn = db_connection_config.establish_connection()?;
 
     let (release, artifact) = crate::schema::jobs::table
         .inner_join(crate::schema::packages::table)
diff --git a/src/db/connection.rs b/src/db/connection.rs
index 4b1e386..3e77cdb 100644
--- a/src/db/connection.rs
+++ b/src/db/connection.rs
@@ -41,52 +41,44 @@ pub struct DbConnectionConfig<'a> {
     database_connection_timeout: u16,
 }
 
-pub fn parse_db_connection_config<'a>(config: &'a Configuration, cli: &'a ArgMatches) -> Result<DbConnectionConfig<'a>> {
-    Ok(DbConnectionConfig {
-        database_host: {
-            cli.value_of("database_host")
-                .unwrap_or_else(|| config.database_host())
-        },
-        database_port: {
-            cli.value_of("database_port")
-                .map(u16::from_str)
-                .transpose()?
-                .unwrap_or_else(|| *config.database_port())
-        },
-        database_user: {
-            cli.value_of("database_user")
-                .unwrap_or_else(|| config.database_user())
-        },
-        database_password: {
-            cli.value_of("database_password")
-                .unwrap_or_else(|| config.database_password())
-        },
-        database_name: {
-            cli.value_of("database_name")
-                .unwrap_or_else(|| config.database_name())
-        },
-        database_connection_timeout: {
-            cli.value_of("database_connection_timeout")
-                .map(u16::from_str)
-                .transpose()?
-                .unwrap_or_else( || {
-                    // hardcoded default of 30 seconds database timeout
-                    config.database_connection_timeout().unwrap_or(30)
-                })
-        },
-    })
-}
+impl<'a> DbConnectionConfig<'a> {
+    pub fn parse(config: &'a Configuration, cli: &'a ArgMatches) -> Result<DbConnectionConfig<'a>> {
+        Ok(DbConnectionConfig {
+            database_host: cli.value_of("database_host").unwrap_or_else(|| config.database_host()),
+            database_port: {
+                cli.value_of("database_port")
+                    .map(u16::from_str)
+                    .transpose()?
+                    .unwrap_or_else(|| *config.database_port())
+            },
+            database_user: cli.value_of("database_user").unwrap_or_else(|| config.database_user()),
+            database_password: cli.value_of("database_password").unwrap_or_else(|| config.database_password()),
+            database_name: cli.value_of("database_name").unwrap_or_else(|| config.database_name()),
+            database_connection_timeout: {
+                cli.value_of("database_connection_timeout")
+                    .map(u16::from_str)
+                    .transpose()?
+                    .unwrap_or_else( || {
+                        // hardcoded default of 30 seconds database timeout
+                        config.database_connection_timeout().unwrap_or(30)
+                    })
+            },
+        })
+    }
+
+    pub fn establish_connection(self) -> Result<PgConnection> {
+        let database_uri: String = format!(
+            "postgres://{user}:{password}@{host}:{port}/{name}?connect_timeout={timeout}",
+            host = self.database_host,
+            port = self.database_port,
+            user = self.database_user,
+            password = self.database_password,
+            name = self.database_name,
+            timeout = self.database_connection_timeout,
+        );
+        debug!("Trying to connect to database: {}", database_uri);
+        PgConnection::establish(&database_uri).map_err(Error::from)
+    }
 
-pub fn establish_connection<'a>(conn_config: DbConnectionConfig<'a>) -> Result<PgConnection> {
-    let database_uri: String = format!(
-        "postgres://{user}:{password}@{host}:{port}/{name}?connect_timeout={timeout}",
-        host = conn_config.database_host,
-        port = conn_config.database_port,
-        user = conn_config.database_user,
-        password = conn_config.database_password,
-        name = conn_config.database_name,
-        timeout = conn_config.database_connection_timeout,
-    );
-    debug!("Trying to connect to database: {}", database_uri);
-    PgConnection::establish(&database_uri).map_err(Error::from)
 }
+
diff --git a/src/main.rs b/src/main.rs
index 69dd0fa..7feb99b 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -146,7 +146,7 @@ async fn main() -> Result<()> {
         Ok(repo)
     };
 
-    let db_connection_config = crate::db::parse_db_connection_config(&config, &cli)?;
+    let db_connection_config = crate::db::DbConnectionConfig::parse(&config, &cli)?;
     match cli.subcommand() {
         Some(("generate-completions", matches)) => generate_completions(matches),
         Some(("db", matches)) => {
@@ -154,7 +154,7 @@ async fn main() -> Result<()> {
             crate::commands::db(db_connection_config, &config, matches)?
         },
         Some(("build", matches)) => {
-            let conn = crate::db::establish_connection(db_connection_config)?;
+            let conn = db_connection_config.establish_connection()?;
 
             let repo = load_repo()?;
 
@@ -205,7 +205,7 @@ async fn main() -> Result<()> {
         Some(("find-artifact", matches)) => {
             let repo = load_repo()?;
             setup_pager();
-            let conn = crate::db::establish_connection(db_connection_config)?;
+            let conn = db_connection_config.establish_connection()?;
             crate::commands::find_artifact(matches, &config, progressbars, repo, conn)
                 .await
                 .context("find-artifact command failed")?
@@ -252,7 +252,7 @@ async fn main() -> Result<()> {
         Some(("metrics", _)) => {
             let repo = load_repo()?;
             setup_pager();
-            let conn = crate::db::establish_connection(db_connection_config)?;
+            let conn = db_connection_config.establish_connection()?;
             crate::commands::metrics(&repo_path, &config, repo, conn)
                 .await
                 .context("metrics command failed")?