Skip to content

Commit

Permalink
Merge #953: Config overhaul: lowercase for DatabaseDriver
Browse files Browse the repository at this point in the history
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
  • Loading branch information
josecelano committed Jul 5, 2024
2 parents c747321 + ca348a8 commit 035d630
Show file tree
Hide file tree
Showing 17 changed files with 85 additions and 72 deletions.
1 change: 0 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Containerfile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion compose.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions docs/containers.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`).
Expand Down Expand Up @@ -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"
```

Expand Down
1 change: 0 additions & 1 deletion packages/configuration/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
1 change: 1 addition & 0 deletions packages/configuration/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>;
Expand Down
31 changes: 19 additions & 12 deletions packages/configuration/src/v2/database.rs
Original file line number Diff line number Diff line change
@@ -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`.
Expand All @@ -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 {
Expand All @@ -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();
Expand All @@ -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(),
};

Expand Down
4 changes: 2 additions & 2 deletions packages/configuration/src/v2/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@
//! interval_min = 120
//!
//! [core.database]
//! driver = "Sqlite3"
//! driver = "sqlite3"
//! path = "./storage/tracker/lib/database/sqlite3.db"
//!
//! [core.net]
Expand Down Expand Up @@ -420,7 +420,7 @@ mod tests {
interval_min = 120
[core.database]
driver = "Sqlite3"
driver = "sqlite3"
path = "./storage/tracker/lib/database/sqlite3.db"
[core.net]
Expand Down
19 changes: 0 additions & 19 deletions packages/primitives/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>;
6 changes: 3 additions & 3 deletions share/container/entry_script_sh
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion share/default/config/tracker.container.mysql.toml
Original file line number Diff line number Diff line change
@@ -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
Expand Down
32 changes: 24 additions & 8 deletions src/core/databases/driver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
/// ```
Expand All @@ -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);
/// ```
Expand All @@ -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.");
Expand Down
27 changes: 14 additions & 13 deletions src/core/databases/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
}

Expand All @@ -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,
},
}
}
Expand All @@ -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,
}
}
}
Expand All @@ -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(),
Expand Down
5 changes: 3 additions & 2 deletions src/core/databases/mysql.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Expand Down
Loading

0 comments on commit 035d630

Please sign in to comment.