From b286e4d5cfeba9e388197b7afd1fc12b20631609 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Tue, 27 Apr 2021 09:20:39 +0200 Subject: 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 --- src/commands/db.rs | 18 +++++------ src/commands/release.rs | 4 +-- src/db/connection.rs | 86 ++++++++++++++++++++++--------------------------- src/main.rs | 8 ++--- 4 files changed, 54 insertions(+), 62 deletions(-) (limited to 'src') 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::(&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::(&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> { - 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> { + 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 { + 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 { - 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")? -- cgit v1.2.3