Skip to content

Commit

Permalink
refactor: [#599] use URL type for connect_url in database config
Browse files Browse the repository at this point in the history
We were using a cusomt type but sqlx defines it as a URL. WE can
validate this type in the configration and let the app domain validate
if the connection URL is a valid one. For example, if the app supports
the database driver. Otherwise we have to change this first level
valitation anytime we support a new DB.
  • Loading branch information
josecelano committed May 23, 2024
1 parent ab4717d commit b1d5267
Show file tree
Hide file tree
Showing 5 changed files with 19 additions and 76 deletions.
74 changes: 4 additions & 70 deletions src/config/v1/database.rs
Original file line number Diff line number Diff line change
@@ -1,86 +1,20 @@
use std::fmt;

use serde::{Deserialize, Serialize};
use url::Url;

/// Database configuration.
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)]
pub struct Database {
/// The connection string for the database. For example:
/// The connection URL for the database. For example:
///
/// Masked: `***`.
/// Sqlite: `sqlite://data.db?mode=rwc`.
/// Mysql: `mysql://root:root_secret_password@mysql:3306/torrust_index_e2e_testing`.
pub connect_url: ConnectOptions,
pub connect_url: Url,
}

impl Default for Database {
fn default() -> Self {
Self {
connect_url: ConnectOptions::new("sqlite://data.db?mode=rwc"),
connect_url: Url::parse("sqlite://data.db?mode=rwc").unwrap(),
}
}
}

/// This allows a particular case when we want to hide the connection options
/// because it contains secrets we don't want to show.
const DB_CONNECT_MASKED: &str = "***";

/// Prefix for connection to `SQLite` database.
const DB_CONNECT_SQLITE_PREFIX: &str = "sqlite://";

/// Prefix for connection to `MySQL` database.
const DB_CONNECT_MYSQL_PREFIX: &str = "mysql://";

#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)]
pub struct ConnectOptions(String);

impl ConnectOptions {
/// # Panics
///
/// Will panic if the connect options are empty.
#[must_use]
pub fn new(connect_options: &str) -> Self {
assert!(!connect_options.is_empty(), "database connect options cannot be empty");
assert!(
connect_options.starts_with(DB_CONNECT_SQLITE_PREFIX)
|| connect_options.starts_with(DB_CONNECT_MYSQL_PREFIX)
|| connect_options.starts_with(DB_CONNECT_MASKED),
"database driver not supported"
);

Self(connect_options.to_owned())
}

#[must_use]
pub fn as_bytes(&self) -> &[u8] {
self.0.as_bytes()
}
}

impl fmt::Display for ConnectOptions {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "{}", self.0)
}
}

#[cfg(test)]
mod tests {
use super::ConnectOptions;

#[test]
#[should_panic(expected = "database connect options cannot be empty")]
fn database_connect_options_can_not_be_empty() {
drop(ConnectOptions::new(""));
}

#[test]
#[should_panic(expected = "database driver not supported")]
fn database_connect_options_only_supports_sqlite_and_mysql() {
drop(ConnectOptions::new("not-supported://"));
}

#[test]
fn database_connect_options_can_be_masked() {
drop(ConnectOptions::new("***"));
}
}
6 changes: 4 additions & 2 deletions src/config/v1/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use serde::{Deserialize, Serialize};

use self::api::Api;
use self::auth::{Auth, SecretKey};
use self::database::{ConnectOptions, Database};
use self::database::Database;
use self::image_cache::ImageCache;
use self::mail::Mail;
use self::net::Network;
Expand Down Expand Up @@ -58,7 +58,9 @@ impl Settings {

pub fn remove_secrets(&mut self) {
"***".clone_into(&mut self.tracker.token);
self.database.connect_url = ConnectOptions::new("***");
if let Some(_password) = self.database.connect_url.password() {
let _ = self.database.connect_url.set_password(Some("***"));
}
"***".clone_into(&mut self.mail.password);
self.auth.secret_key = SecretKey::new("***");
}
Expand Down
2 changes: 1 addition & 1 deletion src/console/commands/tracker_statistics_importer/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ pub async fn import() {
eprintln!("Tracker url: {}", tracker_url.green());

let database = Arc::new(
database::connect(&settings.database.connect_url.to_string())
database::connect(settings.database.connect_url.as_ref())
.await
.expect("unable to connect to db"),
);
Expand Down
9 changes: 8 additions & 1 deletion tests/e2e/environment.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use std::env;

use torrust_index::web::api::Version;
use url::Url;

use super::config::{initialize_configuration, ENV_VAR_DB_CONNECT_URL, ENV_VAR_INDEX_SHARED};
use crate::common::contexts::settings::Settings;
Expand Down Expand Up @@ -90,8 +91,14 @@ impl TestEnv {
pub fn server_settings_masking_secrets(&self) -> Option<Settings> {
match self.starting_settings.clone() {
Some(mut settings) => {
// Mask password in DB connect URL if present
let mut connect_url = Url::parse(&settings.database.connect_url).expect("valid database connect URL");
if let Some(_password) = connect_url.password() {
let _ = connect_url.set_password(Some("***"));
settings.database.connect_url = connect_url.to_string();
}

"***".clone_into(&mut settings.tracker.token);
"***".clone_into(&mut settings.database.connect_url);
"***".clone_into(&mut settings.mail.password);
"***".clone_into(&mut settings.auth.secret_key);
Some(settings)
Expand Down
4 changes: 2 additions & 2 deletions tests/environments/isolated.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
use tempfile::TempDir;
use torrust_index::config;
use torrust_index::config::v1::database::ConnectOptions;
use torrust_index::config::FREE_PORT;
use torrust_index::web::api::Version;
use url::Url;

use super::app_starter::AppStarter;
use crate::common::random;
Expand Down Expand Up @@ -84,7 +84,7 @@ fn ephemeral(temp_dir: &TempDir) -> config::Settings {

// Ephemeral SQLite database
configuration.database.connect_url =
ConnectOptions::new(&format!("sqlite://{}?mode=rwc", random_database_file_path_in(temp_dir)));
Url::parse(&format!("sqlite://{}?mode=rwc", random_database_file_path_in(temp_dir))).unwrap();

configuration
}
Expand Down

0 comments on commit b1d5267

Please sign in to comment.