refactor: move ConfigurationPublic

josecelano · josecelano · commit a5e745e5ee3f · 2024-07-09T17:34:36.000+01:00
ConfigurationPublic is not a configuration type. It does not belong to
the configuration. We don't need to version this type when the
configuration changes.

It's a type containing a subset of the data contained in the
configuration that we want to expose via the API. It happens that those
are the fields we want to expose via the API becuase they are the fields
we are using in the webapp (Index GUI) but it's not part of the
configuration. It's a concrete view of the configration used for other
purposes rather than initialize the Index app.

In the future, it could be even moved to the API as a API resource.

Changing this struct changes the API contract. The contract with the API
consumers, not the contract with the Index administrators, the people
responsible for setting up the Index and it's configuration.

That the reason why it was moved from the config mod to the config
service.

It's not a problem now, but we should cerate an API resource for this
type becuase it should be versioned in the API. WE are using versioning
in the API but the type was excluded, meaning it cannot be versioned.
diff --git a/src/config/mod.rs b/src/config/mod.rs
@@ -2,9 +2,8 @@
 pub mod v2;
 pub mod validator;
 
-use std::str::FromStr;
+use std::env;
 use std::sync::Arc;
-use std::{env, fmt};
 
 use camino::Utf8PathBuf;
 use derive_more::Display;
@@ -15,7 +14,6 @@ use serde_with::{serde_as, NoneAsEmptyString};
 use thiserror::Error;
 use tokio::sync::RwLock;
 use torrust_index_located_error::LocatedError;
-use url::Url;
 
 use crate::web::api::server::DynError;
 
@@ -301,32 +299,6 @@ impl Configuration {
         settings_lock.clone()
     }
 
-    pub async fn get_public(&self) -> ConfigurationPublic {
-        let settings_lock = self.settings.read().await;
-
-        let email_on_signup = match &settings_lock.registration {
-            Some(registration) => match &registration.email {
-                Some(email) => {
-                    if email.required {
-                        EmailOnSignup::Required
-                    } else {
-                        EmailOnSignup::Optional
-                    }
-                }
-                None => EmailOnSignup::NotIncluded,
-            },
-            None => EmailOnSignup::NotIncluded,
-        };
-
-        ConfigurationPublic {
-            website_name: settings_lock.website.name.clone(),
-            tracker_url: settings_lock.tracker.url.clone(),
-            tracker_listed: settings_lock.tracker.listed,
-            tracker_private: settings_lock.tracker.private,
-            email_on_signup,
-        }
-    }
-
     pub async fn get_site_name(&self) -> String {
         let settings_lock = self.settings.read().await;
 
@@ -339,67 +311,12 @@ impl Configuration {
     }
 }
 
-/// The public index configuration.
-/// There is an endpoint to get this configuration.
-#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)]
-pub struct ConfigurationPublic {
-    website_name: String,
-    tracker_url: Url,
-    tracker_listed: bool,
-    tracker_private: bool,
-    email_on_signup: EmailOnSignup,
-}
-
-/// Whether the email is required on signup or not.
-#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)]
-#[serde(rename_all = "lowercase")]
-pub enum EmailOnSignup {
-    /// The email is required on signup.
-    Required,
-    /// The email is optional on signup.
-    Optional,
-    /// The email is not allowed on signup. It will only be ignored if provided.
-    NotIncluded,
-}
-
-impl Default for EmailOnSignup {
-    fn default() -> Self {
-        Self::Optional
-    }
-}
-
-impl fmt::Display for EmailOnSignup {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        let display_str = match self {
-            EmailOnSignup::Required => "required",
-            EmailOnSignup::Optional => "optional",
-            EmailOnSignup::NotIncluded => "ignored",
-        };
-        write!(f, "{display_str}")
-    }
-}
-
-impl FromStr for EmailOnSignup {
-    type Err = String;
-
-    fn from_str(s: &str) -> Result<Self, Self::Err> {
-        match s.to_lowercase().as_str() {
-            "required" => Ok(EmailOnSignup::Required),
-            "optional" => Ok(EmailOnSignup::Optional),
-            "none" => Ok(EmailOnSignup::NotIncluded),
-            _ => Err(format!(
-                "Unknown config 'email_on_signup' option (required, optional, none): {s}"
-            )),
-        }
-    }
-}
-
 #[cfg(test)]
 mod tests {
 
     use url::Url;
 
-    use crate::config::{ApiToken, Configuration, ConfigurationPublic, EmailOnSignup, Info, SecretKey, Settings};
+    use crate::config::{ApiToken, Configuration, Info, SecretKey, Settings};
 
     #[cfg(test)]
     fn default_config_toml() -> String {
@@ -484,37 +401,6 @@ mod tests {
         assert_eq!(toml, default_config_toml());
     }
 
-    #[tokio::test]
-    async fn configuration_should_return_only_public_settings() {
-        let configuration = Configuration::default();
-        let all_settings = configuration.get_all().await;
-
-        let email_on_signup = match &all_settings.registration {
-            Some(registration) => match &registration.email {
-                Some(email) => {
-                    if email.required {
-                        EmailOnSignup::Required
-                    } else {
-                        EmailOnSignup::Optional
-                    }
-                }
-                None => EmailOnSignup::NotIncluded,
-            },
-            None => EmailOnSignup::NotIncluded,
-        };
-
-        assert_eq!(
-            configuration.get_public().await,
-            ConfigurationPublic {
-                website_name: all_settings.website.name,
-                tracker_url: all_settings.tracker.url,
-                tracker_listed: all_settings.tracker.listed,
-                tracker_private: all_settings.tracker.private,
-                email_on_signup,
-            }
-        );
-    }
-
     #[tokio::test]
     async fn configuration_should_return_the_site_name() {
         let configuration = Configuration::default();
diff --git a/src/services/settings.rs b/src/services/settings.rs
@@ -1,8 +1,13 @@
 //! Settings service.
+use std::fmt;
+use std::str::FromStr;
 use std::sync::Arc;
 
+use serde::{Deserialize, Serialize};
+use url::Url;
+
 use super::authorization::{self, ACTION};
-use crate::config::{Configuration, ConfigurationPublic, Settings};
+use crate::config::{Configuration, Settings};
 use crate::errors::ServiceError;
 use crate::models::user::UserId;
 
@@ -58,7 +63,8 @@ impl Service {
     ///
     /// It returns an error if the user does not have the required permissions.
     pub async fn get_public(&self) -> ConfigurationPublic {
-        self.configuration.get_public().await
+        let settings_lock = self.configuration.get_all().await;
+        extract_public_settings(&settings_lock)
     }
 
     /// It gets the site name from the settings.
@@ -70,3 +76,119 @@ impl Service {
         self.configuration.get_site_name().await
     }
 }
+
+fn extract_public_settings(settings: &Settings) -> ConfigurationPublic {
+    let email_on_signup = match &settings.registration {
+        Some(registration) => match &registration.email {
+            Some(email) => {
+                if email.required {
+                    EmailOnSignup::Required
+                } else {
+                    EmailOnSignup::Optional
+                }
+            }
+            None => EmailOnSignup::NotIncluded,
+        },
+        None => EmailOnSignup::NotIncluded,
+    };
+
+    ConfigurationPublic {
+        website_name: settings.website.name.clone(),
+        tracker_url: settings.tracker.url.clone(),
+        tracker_listed: settings.tracker.listed,
+        tracker_private: settings.tracker.private,
+        email_on_signup,
+    }
+}
+
+/// The public index configuration.
+/// There is an endpoint to get this configuration.
+#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)]
+pub struct ConfigurationPublic {
+    website_name: String,
+    tracker_url: Url,
+    tracker_listed: bool,
+    tracker_private: bool,
+    email_on_signup: EmailOnSignup,
+}
+
+/// Whether the email is required on signup or not.
+#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)]
+#[serde(rename_all = "lowercase")]
+pub enum EmailOnSignup {
+    /// The email is required on signup.
+    Required,
+    /// The email is optional on signup.
+    Optional,
+    /// The email is not allowed on signup. It will only be ignored if provided.
+    NotIncluded,
+}
+
+impl Default for EmailOnSignup {
+    fn default() -> Self {
+        Self::Optional
+    }
+}
+
+impl fmt::Display for EmailOnSignup {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        let display_str = match self {
+            EmailOnSignup::Required => "required",
+            EmailOnSignup::Optional => "optional",
+            EmailOnSignup::NotIncluded => "ignored",
+        };
+        write!(f, "{display_str}")
+    }
+}
+
+impl FromStr for EmailOnSignup {
+    type Err = String;
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        match s.to_lowercase().as_str() {
+            "required" => Ok(EmailOnSignup::Required),
+            "optional" => Ok(EmailOnSignup::Optional),
+            "none" => Ok(EmailOnSignup::NotIncluded),
+            _ => Err(format!(
+                "Unknown config 'email_on_signup' option (required, optional, none): {s}"
+            )),
+        }
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use crate::config::Configuration;
+    use crate::services::settings::{extract_public_settings, ConfigurationPublic, EmailOnSignup};
+
+    #[tokio::test]
+    async fn configuration_should_return_only_public_settings() {
+        let configuration = Configuration::default();
+        let all_settings = configuration.get_all().await;
+
+        let email_on_signup = match &all_settings.registration {
+            Some(registration) => match &registration.email {
+                Some(email) => {
+                    if email.required {
+                        EmailOnSignup::Required
+                    } else {
+                        EmailOnSignup::Optional
+                    }
+                }
+                None => EmailOnSignup::NotIncluded,
+            },
+            None => EmailOnSignup::NotIncluded,
+        };
+
+        assert_eq!(
+            extract_public_settings(&all_settings),
+            ConfigurationPublic {
+                website_name: all_settings.website.name,
+                tracker_url: all_settings.tracker.url,
+                tracker_listed: all_settings.tracker.listed,
+                tracker_private: all_settings.tracker.private,
+                email_on_signup,
+            }
+        );
+    }
+}
diff --git a/src/web/api/server/v1/contexts/settings/mod.rs b/src/web/api/server/v1/contexts/settings/mod.rs
@@ -183,7 +183,7 @@
 //!
 //! **Resource**
 //!
-//! Refer to the [`ConfigurationPublic`](crate::config::ConfigurationPublic)
+//! Refer to the [`ConfigurationPublic`](crate::services::settings::ConfigurationPublic)
 //! struct for more information about the response attributes.
 pub mod handlers;
 pub mod routes;
diff --git a/tests/e2e/web/api/v1/contexts/settings/contract.rs b/tests/e2e/web/api/v1/contexts/settings/contract.rs
@@ -1,6 +1,6 @@
 //! API contract for `settings` context.
 
-use torrust_index::config::EmailOnSignup;
+use torrust_index::services::settings::EmailOnSignup;
 use torrust_index::web::api;
 
 use crate::common::asserts::assert_json_ok_response;
