Merge #953: Config overhaul: lowercase for DatabaseDriver

josecelano · josecelano · commit 035d630c239c · 2024-07-05T11:06:56.000+01:00
ca348a8 chore: remove unused dependency (Jose Celano) 9d72f51 feat: [#950] use lowercase for database driver values in configuration (Jose Celano) d970bb8 refactor: [#950] rename DatabaseDriver to Driver (Jose Celano) 954295a refactor: [#950] move DatabaseDriver to databases mod (Jose Celano) 9be9638 refactor: [#950] decouple database driver enum (Jose Celano) Pull request description: Use lowercase for database driver values in the configuration: ```toml [core.database] driver = "sqlite3" #driver = "MySQL" ``` Instead of: ```toml [core.database] driver = "Sqlite3" #driver = "MySql" ``` We are normalizing all enum variants in the configuration to lowercase. It also decouples the internal database driver enum from the enum used in the configuration. ACKs for top commit: josecelano: ACK ca348a8 Tree-SHA512: 499ed0b628e385f7927d2bea50334c68eece6fe2e6b0170bf372e3db7d88837fb908fdbf0e94d7f7144141a1d985732b21694515e5551155bd8d7cbe9e58bb15
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Containerfile b/Containerfile
@@ -96,7 +96,7 @@ RUN ["/busybox/cp", "-sp", "/busybox/sh","/busybox/cat","/busybox/ls","/busybox/
 COPY --from=gcc --chmod=0555 /usr/local/bin/su-exec /bin/su-exec
 
 ARG TORRUST_TRACKER_CONFIG_TOML_PATH="/etc/torrust/tracker/tracker.toml"
-ARG TORRUST_TRACKER_CONFIG_OVERRIDE_CORE__DATABASE__DRIVER="Sqlite3"
+ARG TORRUST_TRACKER_CONFIG_OVERRIDE_CORE__DATABASE__DRIVER="sqlite3"
 ARG USER_ID=1000
 ARG UDP_PORT=6969
 ARG HTTP_PORT=7070
diff --git a/compose.yaml b/compose.yaml
@@ -4,7 +4,7 @@ services:
     image: torrust-tracker:release
     tty: true
     environment:
-      - TORRUST_TRACKER_CONFIG_OVERRIDE_CORE__DATABASE__DRIVER=${TORRUST_TRACKER_CONFIG_OVERRIDE_CORE__DATABASE__DRIVER:-MySQL}
+      - TORRUST_TRACKER_CONFIG_OVERRIDE_CORE__DATABASE__DRIVER=${TORRUST_TRACKER_CONFIG_OVERRIDE_CORE__DATABASE__DRIVER:-mysql}
       - TORRUST_TRACKER_CONFIG_OVERRIDE_HTTP_API__ACCESS_TOKENS__ADMIN=${TORRUST_TRACKER_CONFIG_OVERRIDE_HTTP_API__ACCESS_TOKENS__ADMIN:-MyAccessToken}
     networks:
       - server_side
diff --git a/docs/containers.md b/docs/containers.md
@@ -149,7 +149,7 @@ The following environmental variables can be set:
 
 - `TORRUST_TRACKER_CONFIG_TOML_PATH` - The in-container path to the tracker configuration file, (default: `"/etc/torrust/tracker/tracker.toml"`).
 - `TORRUST_TRACKER_CONFIG_OVERRIDE_HTTP_API__ACCESS_TOKENS__ADMIN` - Override of the admin token. If set, this value overrides any value set in the config.
-- `TORRUST_TRACKER_CONFIG_OVERRIDE_CORE__DATABASE__DRIVER` - The database type used for the container, (options: `Sqlite3`, `MySQL`, default `Sqlite3`). Please Note: This dose not override the database configuration within the `.toml` config file.
+- `TORRUST_TRACKER_CONFIG_OVERRIDE_CORE__DATABASE__DRIVER` - The database type used for the container, (options: `sqlite3`, `mysql`, default `sqlite3`). Please Note: This dose not override the database configuration within the `.toml` config file.
 - `TORRUST_TRACKER_CONFIG_TOML` - Load config from this environmental variable instead from a file, (i.e: `TORRUST_TRACKER_CONFIG_TOML=$(cat tracker-tracker.toml)`).
 - `USER_ID` - The user id for the runtime crated `torrust` user. Please Note: This user id should match the ownership of the host-mapped volumes, (default `1000`).
 - `UDP_PORT` - The port for the UDP tracker. This should match the port used in the configuration, (default `6969`).
@@ -244,7 +244,7 @@ The docker-compose configuration includes the MySQL service configuration. If yo
 
 ```toml
 [core.database]
-driver = "MySQL"
+driver = "mysql"
 path = "mysql://db_user:db_user_secret_password@mysql:3306/torrust_tracker"
 ```
 
diff --git a/packages/configuration/Cargo.toml b/packages/configuration/Cargo.toml
@@ -24,7 +24,6 @@ serde_with = "3"
 thiserror = "1"
 toml = "0"
 torrust-tracker-located-error = { version = "3.0.0-alpha.12-develop", path = "../located-error" }
-torrust-tracker-primitives = { version = "3.0.0-alpha.12-develop", path = "../primitives" }
 url = "2.5.2"
 
 [dev-dependencies]
diff --git a/packages/configuration/src/lib.rs b/packages/configuration/src/lib.rs
@@ -41,6 +41,7 @@ pub type HttpApi = v2::tracker_api::HttpApi;
 pub type HttpTracker = v2::http_tracker::HttpTracker;
 pub type UdpTracker = v2::udp_tracker::UdpTracker;
 pub type Database = v2::database::Database;
+pub type Driver = v2::database::Driver;
 pub type Threshold = v2::logging::Threshold;
 
 pub type AccessTokens = HashMap<String, String>;
diff --git a/packages/configuration/src/v2/database.rs b/packages/configuration/src/v2/database.rs
@@ -1,17 +1,16 @@
 use serde::{Deserialize, Serialize};
-use torrust_tracker_primitives::DatabaseDriver;
 use url::Url;
 
 #[allow(clippy::struct_excessive_bools)]
 #[derive(Serialize, Deserialize, PartialEq, Eq, Debug, Clone)]
 pub struct Database {
     // Database configuration
-    /// Database driver. Possible values are: `Sqlite3`, and `MySQL`.
+    /// Database driver. Possible values are: `sqlite3`, and `mysql`.
     #[serde(default = "Database::default_driver")]
-    pub driver: DatabaseDriver,
+    pub driver: Driver,
 
     /// Database connection string. The format depends on the database driver.
-    /// For `Sqlite3`, the format is `path/to/database.db`, for example:
+    /// For `sqlite3`, the format is `path/to/database.db`, for example:
     /// `./storage/tracker/lib/database/sqlite3.db`.
     /// For `Mysql`, the format is `mysql://db_user:db_user_password:port/db_name`, for
     /// example: `mysql://root:password@localhost:3306/torrust`.
@@ -29,8 +28,8 @@ impl Default for Database {
 }
 
 impl Database {
-    fn default_driver() -> DatabaseDriver {
-        DatabaseDriver::Sqlite3
+    fn default_driver() -> Driver {
+        Driver::Sqlite3
     }
 
     fn default_path() -> String {
@@ -44,10 +43,10 @@ impl Database {
     /// Will panic if the database path for `MySQL` is not a valid URL.
     pub fn mask_secrets(&mut self) {
         match self.driver {
-            DatabaseDriver::Sqlite3 => {
+            Driver::Sqlite3 => {
                 // Nothing to mask
             }
-            DatabaseDriver::MySQL => {
+            Driver::MySQL => {
                 let mut url = Url::parse(&self.path).expect("path for MySQL driver should be a valid URL");
                 url.set_password(Some("***")).expect("url password should be changed");
                 self.path = url.to_string();
@@ -56,17 +55,25 @@ impl Database {
     }
 }
 
+/// The database management system used by the tracker.
+#[derive(Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord, Debug, Hash, Clone)]
+#[serde(rename_all = "lowercase")]
+pub enum Driver {
+    /// The `Sqlite3` database driver.
+    Sqlite3,
+    /// The `MySQL` database driver.
+    MySQL,
+}
+
 #[cfg(test)]
 mod tests {
 
-    use torrust_tracker_primitives::DatabaseDriver;
-
-    use super::Database;
+    use super::{Database, Driver};
 
     #[test]
     fn it_should_allow_masking_the_mysql_user_password() {
         let mut database = Database {
-            driver: DatabaseDriver::MySQL,
+            driver: Driver::MySQL,
             path: "mysql://root:password@localhost:3306/torrust".to_string(),
         };
 
diff --git a/packages/configuration/src/v2/mod.rs b/packages/configuration/src/v2/mod.rs
@@ -209,7 +209,7 @@
 //! interval_min = 120
 //!
 //! [core.database]
-//! driver = "Sqlite3"
+//! driver = "sqlite3"
 //! path = "./storage/tracker/lib/database/sqlite3.db"
 //!
 //! [core.net]
@@ -420,7 +420,7 @@ mod tests {
                                 interval_min = 120
 
                                 [core.database]
-                                driver = "Sqlite3"
+                                driver = "sqlite3"
                                 path = "./storage/tracker/lib/database/sqlite3.db"
 
                                 [core.net]
diff --git a/packages/primitives/src/lib.rs b/packages/primitives/src/lib.rs
@@ -42,23 +42,4 @@ pub enum IPVersion {
 #[derive(Hash, Clone, Copy, Debug, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord)]
 pub struct NumberOfBytes(pub i64);
 
-/// The database management system used by the tracker.
-///
-/// Refer to:
-///
-/// - [Torrust Tracker Configuration](https://docs.rs/torrust-tracker-configuration).
-/// - [Torrust Tracker](https://docs.rs/torrust-tracker).
-///
-/// For more information about persistence.
-#[derive(Serialize, Deserialize, PartialEq, Eq, Debug, derive_more::Display, Clone)]
-pub enum DatabaseDriver {
-    // TODO:
-    //   - Move to the database crate once that gets its own crate.
-    //   - Rename serialized values to lowercase: `sqlite3` and `mysql`.
-    /// The Sqlite3 database driver.
-    Sqlite3,
-    /// The `MySQL` database driver.
-    MySQL,
-}
-
 pub type PersistentTorrents = BTreeMap<InfoHash, u32>;
diff --git a/share/container/entry_script_sh b/share/container/entry_script_sh
@@ -27,15 +27,15 @@ chmod -R 2770 /var/lib/torrust /var/log/torrust /etc/torrust
 
 # Install the database and config:
 if [ -n "$TORRUST_TRACKER_CONFIG_OVERRIDE_CORE__DATABASE__DRIVER" ]; then
-    if cmp_lc "$TORRUST_TRACKER_CONFIG_OVERRIDE_CORE__DATABASE__DRIVER" "Sqlite3"; then
+    if cmp_lc "$TORRUST_TRACKER_CONFIG_OVERRIDE_CORE__DATABASE__DRIVER" "sqlite3"; then
 
         # Select Sqlite3 empty database
         default_database="/usr/share/torrust/default/database/tracker.sqlite3.db"
 
         # Select Sqlite3 default configuration
         default_config="/usr/share/torrust/default/config/tracker.container.sqlite3.toml"
 
-    elif cmp_lc "$TORRUST_TRACKER_CONFIG_OVERRIDE_CORE__DATABASE__DRIVER" "MySQL"; then
+    elif cmp_lc "$TORRUST_TRACKER_CONFIG_OVERRIDE_CORE__DATABASE__DRIVER" "mysql"; then
 
         # (no database file needed for MySQL)
 
@@ -44,7 +44,7 @@ if [ -n "$TORRUST_TRACKER_CONFIG_OVERRIDE_CORE__DATABASE__DRIVER" ]; then
 
     else 
         echo "Error: Unsupported Database Type: \"$TORRUST_TRACKER_CONFIG_OVERRIDE_CORE__DATABASE__DRIVER\"."
-        echo "Please Note: Supported Database Types: \"Sqlite3\", \"MySQL\"."
+        echo "Please Note: Supported Database Types: \"sqlite3\", \"mysql\"."
         exit 1
     fi
 else
diff --git a/share/default/config/tracker.container.mysql.toml b/share/default/config/tracker.container.mysql.toml
@@ -1,7 +1,7 @@
 version = "2"
 
 [core.database]
-driver = "MySQL"
+driver = "mysql"
 path = "mysql://db_user:db_user_secret_password@mysql:3306/torrust_tracker"
 
 # Uncomment to enable services
diff --git a/src/core/databases/driver.rs b/src/core/databases/driver.rs
@@ -2,22 +2,38 @@
 //!
 //! See [`databases::driver::build`](crate::core::databases::driver::build)
 //! function for more information.
-use torrust_tracker_primitives::DatabaseDriver;
+use serde::{Deserialize, Serialize};
 
 use super::error::Error;
 use super::mysql::Mysql;
 use super::sqlite::Sqlite;
 use super::{Builder, Database};
 
+/// The database management system used by the tracker.
+///
+/// Refer to:
+///
+/// - [Torrust Tracker Configuration](https://docs.rs/torrust-tracker-configuration).
+/// - [Torrust Tracker](https://docs.rs/torrust-tracker).
+///
+/// For more information about persistence.
+#[derive(Serialize, Deserialize, PartialEq, Eq, Debug, derive_more::Display, Clone)]
+pub enum Driver {
+    /// The Sqlite3 database driver.
+    Sqlite3,
+    /// The `MySQL` database driver.
+    MySQL,
+}
+
 /// It builds a new database driver.
 ///
 /// Example for `SQLite3`:
 ///
 /// ```rust,no_run
 /// use torrust_tracker::core::databases;
-/// use torrust_tracker_primitives::DatabaseDriver;
+/// use torrust_tracker::core::databases::driver::Driver;
 ///
-/// let db_driver = DatabaseDriver::Sqlite3;
+/// let db_driver = Driver::Sqlite3;
 /// let db_path = "./storage/tracker/lib/database/sqlite3.db".to_string();
 /// let database = databases::driver::build(&db_driver, &db_path);
 /// ```
@@ -26,9 +42,9 @@ use super::{Builder, Database};
 ///
 /// ```rust,no_run
 /// use torrust_tracker::core::databases;
-/// use torrust_tracker_primitives::DatabaseDriver;
+/// use torrust_tracker::core::databases::driver::Driver;
 ///
-/// let db_driver = DatabaseDriver::MySQL;
+/// let db_driver = Driver::MySQL;
 /// let db_path = "mysql://db_user:db_user_secret_password@mysql:3306/torrust_tracker".to_string();
 /// let database = databases::driver::build(&db_driver, &db_path);
 /// ```
@@ -45,10 +61,10 @@ use super::{Builder, Database};
 /// # Panics
 ///
 /// This function will panic if unable to create database tables.
-pub fn build(driver: &DatabaseDriver, db_path: &str) -> Result<Box<dyn Database>, Error> {
+pub fn build(driver: &Driver, db_path: &str) -> Result<Box<dyn Database>, Error> {
     let database = match driver {
-        DatabaseDriver::Sqlite3 => Builder::<Sqlite>::build(db_path),
-        DatabaseDriver::MySQL => Builder::<Mysql>::build(db_path),
+        Driver::Sqlite3 => Builder::<Sqlite>::build(db_path),
+        Driver::MySQL => Builder::<Mysql>::build(db_path),
     }?;
 
     database.create_database_tables().expect("Could not create database tables.");
diff --git a/src/core/databases/error.rs b/src/core/databases/error.rs
@@ -6,51 +6,52 @@ use std::sync::Arc;
 
 use r2d2_mysql::mysql::UrlError;
 use torrust_tracker_located_error::{DynError, Located, LocatedError};
-use torrust_tracker_primitives::DatabaseDriver;
+
+use super::driver::Driver;
 
 #[derive(thiserror::Error, Debug, Clone)]
 pub enum Error {
     /// The query unexpectedly returned nothing.
     #[error("The {driver} query unexpectedly returned nothing: {source}")]
     QueryReturnedNoRows {
         source: LocatedError<'static, dyn std::error::Error + Send + Sync>,
-        driver: DatabaseDriver,
+        driver: Driver,
     },
 
     /// The query was malformed.
     #[error("The {driver} query was malformed: {source}")]
     InvalidQuery {
         source: LocatedError<'static, dyn std::error::Error + Send + Sync>,
-        driver: DatabaseDriver,
+        driver: Driver,
     },
 
     /// Unable to insert a record into the database
     #[error("Unable to insert record into {driver} database, {location}")]
     InsertFailed {
         location: &'static Location<'static>,
-        driver: DatabaseDriver,
+        driver: Driver,
     },
 
     /// Unable to delete a record into the database
     #[error("Failed to remove record from {driver} database, error-code: {error_code}, {location}")]
     DeleteFailed {
         location: &'static Location<'static>,
         error_code: usize,
-        driver: DatabaseDriver,
+        driver: Driver,
     },
 
     /// Unable to connect to the database
     #[error("Failed to connect to {driver} database: {source}")]
     ConnectionError {
         source: LocatedError<'static, UrlError>,
-        driver: DatabaseDriver,
+        driver: Driver,
     },
 
     /// Unable to create a connection pool
     #[error("Failed to create r2d2 {driver} connection pool: {source}")]
     ConnectionPool {
         source: LocatedError<'static, r2d2::Error>,
-        driver: DatabaseDriver,
+        driver: Driver,
     },
 }
 
@@ -60,11 +61,11 @@ impl From<r2d2_sqlite::rusqlite::Error> for Error {
         match err {
             r2d2_sqlite::rusqlite::Error::QueryReturnedNoRows => Error::QueryReturnedNoRows {
                 source: (Arc::new(err) as DynError).into(),
-                driver: DatabaseDriver::Sqlite3,
+                driver: Driver::Sqlite3,
             },
             _ => Error::InvalidQuery {
                 source: (Arc::new(err) as DynError).into(),
-                driver: DatabaseDriver::Sqlite3,
+                driver: Driver::Sqlite3,
             },
         }
     }
@@ -76,7 +77,7 @@ impl From<r2d2_mysql::mysql::Error> for Error {
         let e: DynError = Arc::new(err);
         Error::InvalidQuery {
             source: e.into(),
-            driver: DatabaseDriver::MySQL,
+            driver: Driver::MySQL,
         }
     }
 }
@@ -86,14 +87,14 @@ impl From<UrlError> for Error {
     fn from(err: UrlError) -> Self {
         Self::ConnectionError {
             source: Located(err).into(),
-            driver: DatabaseDriver::MySQL,
+            driver: Driver::MySQL,
         }
     }
 }
 
-impl From<(r2d2::Error, DatabaseDriver)> for Error {
+impl From<(r2d2::Error, Driver)> for Error {
     #[track_caller]
-    fn from(e: (r2d2::Error, DatabaseDriver)) -> Self {
+    fn from(e: (r2d2::Error, Driver)) -> Self {
         let (err, driver) = e;
         Self::ConnectionPool {
             source: Located(err).into(),
diff --git a/src/core/databases/mysql.rs b/src/core/databases/mysql.rs
@@ -8,14 +8,15 @@ use r2d2_mysql::mysql::prelude::Queryable;
 use r2d2_mysql::mysql::{params, Opts, OptsBuilder};
 use r2d2_mysql::MySqlConnectionManager;
 use torrust_tracker_primitives::info_hash::InfoHash;
-use torrust_tracker_primitives::{DatabaseDriver, PersistentTorrents};
+use torrust_tracker_primitives::PersistentTorrents;
 use tracing::debug;
 
+use super::driver::Driver;
 use super::{Database, Error};
 use crate::core::auth::{self, Key};
 use crate::shared::bit_torrent::common::AUTH_KEY_LENGTH;
 
-const DRIVER: DatabaseDriver = DatabaseDriver::MySQL;
+const DRIVER: Driver = Driver::MySQL;
 
 pub struct Mysql {
     pool: Pool<MySqlConnectionManager>,
diff --git a/src/core/databases/sqlite.rs b/src/core/databases/sqlite.rs
diff --git a/src/core/mod.rs b/src/core/mod.rs
diff --git a/src/lib.rs b/src/lib.rs
