Merge #141: Add environment variable to change the default config fil…

…e path `./config.toml`

655f631 feat: [#130] add env var to change the default config path (Jose Celano)

Pull request description:

  You can now overwrite the default config file path with:

  ```s
  TORRUST_IDX_BACK_CONFIG_PATH=./storage/config/config.toml cargo run
  ```

  The default path is `./config.toml`

ACKs for top commit:
  josecelano:
    ACK 655f631

Tree-SHA512: 5d81dead21ca87c75440baefdd85d57002afd83bf3bb857dbc627c4bf215e74686ec474642f15228d8f0cba4f6b638dad3b26f50b2e3b7c43734f6f69ceac63b

src/bootstrap/config.rs

@@ -6,8 +6,12 @@ use std::env;
 // Environment variables
 
 /// The whole `config.toml` file content. It has priority over the config file.
+/// Even if the file is not on the default path.
 pub const ENV_VAR_CONFIG: &str = "TORRUST_IDX_BACK_CONFIG";
 
+/// The `config.toml` file location.
+pub const ENV_VAR_CONFIG_PATH: &str = "TORRUST_IDX_BACK_CONFIG_PATH";
+
 // Default values
 
 pub const ENV_VAR_DEFAULT_CONFIG_PATH: &str = "./config.toml";
@@ -25,9 +29,11 @@ pub async fn init_configuration() -> Configuration {
 
         Configuration::load_from_env_var(ENV_VAR_CONFIG).unwrap()
     } else {
-        println!("Loading configuration from config file `{}`", ENV_VAR_DEFAULT_CONFIG_PATH);
+        let config_path = env::var(ENV_VAR_CONFIG_PATH).unwrap_or_else(|_| ENV_VAR_DEFAULT_CONFIG_PATH.to_string());
+
+        println!("Loading configuration from config file `{}`", config_path);
 
-        match Configuration::load_from_file(ENV_VAR_DEFAULT_CONFIG_PATH).await {
+        match Configuration::load_from_file(&config_path).await {
             Ok(config) => config,
             Err(error) => {
                 panic!("{}", error)

src/config.rs

@@ -151,12 +151,14 @@ impl Default for AppConfiguration {
 #[derive(Debug)]
 pub struct Configuration {
     pub settings: RwLock<AppConfiguration>,
+    pub config_path: Option<String>,
 }
 
 impl Default for Configuration {
     fn default() -> Self {
         Self {
             settings: RwLock::new(AppConfiguration::default()),
+            config_path: None,
         }
     }
 }
@@ -188,6 +190,7 @@ impl Configuration {
 
         Ok(Configuration {
             settings: RwLock::new(torrust_config),
+            config_path: Some(config_path.to_string()),
         })
     }
 
@@ -207,6 +210,7 @@ impl Configuration {
                 let torrust_config: AppConfiguration = config_builder.try_deserialize()?;
                 Ok(Configuration {
                     settings: RwLock::new(torrust_config),
+                    config_path: None,
                 })
             }
             Err(_) => Err(ConfigError::Message(
@@ -215,26 +219,36 @@ impl Configuration {
         }
     }
 
-    pub async fn save_to_file(&self, config_path: &str) -> Result<(), ()> {
+    pub async fn save_to_file(&self, config_path: &str) {
         let settings = self.settings.read().await;
 
         let toml_string = toml::to_string(&*settings).expect("Could not encode TOML value");
 
         drop(settings);
 
         fs::write(config_path, toml_string).expect("Could not write to file!");
-        Ok(())
     }
 
-    pub async fn update_settings(&self, new_settings: AppConfiguration, config_path: &str) -> Result<(), ()> {
-        let mut settings = self.settings.write().await;
-        *settings = new_settings;
-
-        drop(settings);
+    /// Updates the settings and saves them to the configuration file.
+    ///
+    /// # Panics
+    ///
+    /// Will panic if the configuration file path is not defined. That happens
+    /// when the configuration was loaded from the environment variable.
+    pub async fn update_settings(&self, new_settings: AppConfiguration) {
+        match &self.config_path {
+            Some(config_path) => {
+                let mut settings = self.settings.write().await;
+                *settings = new_settings;
 
-        let _ = self.save_to_file(config_path).await;
+                drop(settings);
 
-        Ok(())
+                let _ = self.save_to_file(config_path).await;
+            }
+            None => panic!(
+                "Cannot update settings when the config file path is not defined. For example: when it's loaded from env var."
+            ),
+        }
     }
 
     pub async fn get_public(&self) -> ConfigurationPublic {

src/routes/settings.rs

@@ -1,6 +1,5 @@
 use actix_web::{web, HttpRequest, HttpResponse, Responder};
 
-use crate::bootstrap::config::ENV_VAR_DEFAULT_CONFIG_PATH;
 use crate::common::WebAppData;
 use crate::config::AppConfiguration;
 use crate::errors::{ServiceError, ServiceResult};
@@ -12,7 +11,7 @@ pub fn init_routes(cfg: &mut web::ServiceConfig) {
             .service(
                 web::resource("")
                     .route(web::get().to(get_settings))
-                    .route(web::post().to(update_settings)),
+                    .route(web::post().to(update_settings_handler)),
             )
             .service(web::resource("/name").route(web::get().to(get_site_name)))
             .service(web::resource("/public").route(web::get().to(get_public_settings))),
@@ -47,7 +46,16 @@ pub async fn get_site_name(app_data: WebAppData) -> ServiceResult<impl Responder
     }))
 }
 
-pub async fn update_settings(
+/// Update the settings
+///
+/// # Errors
+///
+/// Will return an error if:
+///
+/// - There is no logged-in user.
+/// - The user is not an administrator.
+/// - The settings could not be updated because they were loaded from env vars.
+pub async fn update_settings_handler(
     req: HttpRequest,
     payload: web::Json<AppConfiguration>,
     app_data: WebAppData,
@@ -60,10 +68,7 @@ pub async fn update_settings(
         return Err(ServiceError::Unauthorized);
     }
 
-    let _ = app_data
-        .cfg
-        .update_settings(payload.into_inner(), ENV_VAR_DEFAULT_CONFIG_PATH)
-        .await;
+    let _ = app_data.cfg.update_settings(payload.into_inner()).await;
 
     let settings = app_data.cfg.settings.read().await;
 

tests/e2e/contexts/settings/contract.rs

@@ -67,15 +67,14 @@ async fn it_should_allow_admins_to_get_all_the_settings() {
 #[tokio::test]
 async fn it_should_allow_admins_to_update_all_the_settings() {
     let mut env = TestEnv::new();
+    env.start().await;
 
     if !env.is_isolated() {
         // This test can't be executed in a non-isolated environment because
         // it will change the settings for all the other tests.
         return;
     }
 
-    env.start().await;
-
     let logged_in_admin = new_logged_in_admin(&env).await;
     let client = Client::authenticated(&env.server_socket_addr().unwrap(), &logged_in_admin.token);
 

tests/environments/app_starter.rs

@@ -8,6 +8,7 @@ use torrust_index_backend::config::{AppConfiguration, Configuration};
 /// It launches the app and provides a way to stop it.
 pub struct AppStarter {
     configuration: AppConfiguration,
+    config_path: Option<String>,
     /// The application binary state (started or not):
     ///  - `None`: if the app is not started,
     ///  - `RunningState`: if the app was started.
@@ -16,9 +17,10 @@ pub struct AppStarter {
 
 impl AppStarter {
     #[must_use]
-    pub fn with_custom_configuration(configuration: AppConfiguration) -> Self {
+    pub fn with_custom_configuration(configuration: AppConfiguration, config_path: Option<String>) -> Self {
         Self {
             configuration,
+            config_path,
             running_state: None,
         }
     }
@@ -29,6 +31,7 @@ impl AppStarter {
     pub async fn start(&mut self) {
         let configuration = Configuration {
             settings: RwLock::new(self.configuration.clone()),
+            config_path: self.config_path.clone(),
         };
 
         // Open a channel to communicate back with this function

tests/environments/isolated.rs

@@ -27,8 +27,12 @@ impl TestEnv {
         let temp_dir = TempDir::new().expect("failed to create a temporary directory");
 
         let configuration = ephemeral(&temp_dir);
+        // Even if we load the configuration from the environment variable, we
+        // still need to provide a path to save the configuration when the
+        // configuration is updated via the `POST /settings` endpoints.
+        let config_path = format!("{}/config.toml", temp_dir.path().to_string_lossy());
 
-        let app_starter = AppStarter::with_custom_configuration(configuration);
+        let app_starter = AppStarter::with_custom_configuration(configuration, Some(config_path));
 
         Self { app_starter, temp_dir }
     }