Skip to content

Commit a5e745e

Browse files
committed
refactor: move ConfigurationPublic
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.
1 parent 4fbeead commit a5e745e

File tree

4 files changed

+128
-120
lines changed

4 files changed

+128
-120
lines changed

src/config/mod.rs

+2-116
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,8 @@
22
pub mod v2;
33
pub mod validator;
44

5-
use std::str::FromStr;
5+
use std::env;
66
use std::sync::Arc;
7-
use std::{env, fmt};
87

98
use camino::Utf8PathBuf;
109
use derive_more::Display;
@@ -15,7 +14,6 @@ use serde_with::{serde_as, NoneAsEmptyString};
1514
use thiserror::Error;
1615
use tokio::sync::RwLock;
1716
use torrust_index_located_error::LocatedError;
18-
use url::Url;
1917

2018
use crate::web::api::server::DynError;
2119

@@ -301,32 +299,6 @@ impl Configuration {
301299
settings_lock.clone()
302300
}
303301

304-
pub async fn get_public(&self) -> ConfigurationPublic {
305-
let settings_lock = self.settings.read().await;
306-
307-
let email_on_signup = match &settings_lock.registration {
308-
Some(registration) => match &registration.email {
309-
Some(email) => {
310-
if email.required {
311-
EmailOnSignup::Required
312-
} else {
313-
EmailOnSignup::Optional
314-
}
315-
}
316-
None => EmailOnSignup::NotIncluded,
317-
},
318-
None => EmailOnSignup::NotIncluded,
319-
};
320-
321-
ConfigurationPublic {
322-
website_name: settings_lock.website.name.clone(),
323-
tracker_url: settings_lock.tracker.url.clone(),
324-
tracker_listed: settings_lock.tracker.listed,
325-
tracker_private: settings_lock.tracker.private,
326-
email_on_signup,
327-
}
328-
}
329-
330302
pub async fn get_site_name(&self) -> String {
331303
let settings_lock = self.settings.read().await;
332304

@@ -339,67 +311,12 @@ impl Configuration {
339311
}
340312
}
341313

342-
/// The public index configuration.
343-
/// There is an endpoint to get this configuration.
344-
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)]
345-
pub struct ConfigurationPublic {
346-
website_name: String,
347-
tracker_url: Url,
348-
tracker_listed: bool,
349-
tracker_private: bool,
350-
email_on_signup: EmailOnSignup,
351-
}
352-
353-
/// Whether the email is required on signup or not.
354-
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)]
355-
#[serde(rename_all = "lowercase")]
356-
pub enum EmailOnSignup {
357-
/// The email is required on signup.
358-
Required,
359-
/// The email is optional on signup.
360-
Optional,
361-
/// The email is not allowed on signup. It will only be ignored if provided.
362-
NotIncluded,
363-
}
364-
365-
impl Default for EmailOnSignup {
366-
fn default() -> Self {
367-
Self::Optional
368-
}
369-
}
370-
371-
impl fmt::Display for EmailOnSignup {
372-
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
373-
let display_str = match self {
374-
EmailOnSignup::Required => "required",
375-
EmailOnSignup::Optional => "optional",
376-
EmailOnSignup::NotIncluded => "ignored",
377-
};
378-
write!(f, "{display_str}")
379-
}
380-
}
381-
382-
impl FromStr for EmailOnSignup {
383-
type Err = String;
384-
385-
fn from_str(s: &str) -> Result<Self, Self::Err> {
386-
match s.to_lowercase().as_str() {
387-
"required" => Ok(EmailOnSignup::Required),
388-
"optional" => Ok(EmailOnSignup::Optional),
389-
"none" => Ok(EmailOnSignup::NotIncluded),
390-
_ => Err(format!(
391-
"Unknown config 'email_on_signup' option (required, optional, none): {s}"
392-
)),
393-
}
394-
}
395-
}
396-
397314
#[cfg(test)]
398315
mod tests {
399316

400317
use url::Url;
401318

402-
use crate::config::{ApiToken, Configuration, ConfigurationPublic, EmailOnSignup, Info, SecretKey, Settings};
319+
use crate::config::{ApiToken, Configuration, Info, SecretKey, Settings};
403320

404321
#[cfg(test)]
405322
fn default_config_toml() -> String {
@@ -484,37 +401,6 @@ mod tests {
484401
assert_eq!(toml, default_config_toml());
485402
}
486403

487-
#[tokio::test]
488-
async fn configuration_should_return_only_public_settings() {
489-
let configuration = Configuration::default();
490-
let all_settings = configuration.get_all().await;
491-
492-
let email_on_signup = match &all_settings.registration {
493-
Some(registration) => match &registration.email {
494-
Some(email) => {
495-
if email.required {
496-
EmailOnSignup::Required
497-
} else {
498-
EmailOnSignup::Optional
499-
}
500-
}
501-
None => EmailOnSignup::NotIncluded,
502-
},
503-
None => EmailOnSignup::NotIncluded,
504-
};
505-
506-
assert_eq!(
507-
configuration.get_public().await,
508-
ConfigurationPublic {
509-
website_name: all_settings.website.name,
510-
tracker_url: all_settings.tracker.url,
511-
tracker_listed: all_settings.tracker.listed,
512-
tracker_private: all_settings.tracker.private,
513-
email_on_signup,
514-
}
515-
);
516-
}
517-
518404
#[tokio::test]
519405
async fn configuration_should_return_the_site_name() {
520406
let configuration = Configuration::default();

src/services/settings.rs

+124-2
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,13 @@
11
//! Settings service.
2+
use std::fmt;
3+
use std::str::FromStr;
24
use std::sync::Arc;
35

6+
use serde::{Deserialize, Serialize};
7+
use url::Url;
8+
49
use super::authorization::{self, ACTION};
5-
use crate::config::{Configuration, ConfigurationPublic, Settings};
10+
use crate::config::{Configuration, Settings};
611
use crate::errors::ServiceError;
712
use crate::models::user::UserId;
813

@@ -58,7 +63,8 @@ impl Service {
5863
///
5964
/// It returns an error if the user does not have the required permissions.
6065
pub async fn get_public(&self) -> ConfigurationPublic {
61-
self.configuration.get_public().await
66+
let settings_lock = self.configuration.get_all().await;
67+
extract_public_settings(&settings_lock)
6268
}
6369

6470
/// It gets the site name from the settings.
@@ -70,3 +76,119 @@ impl Service {
7076
self.configuration.get_site_name().await
7177
}
7278
}
79+
80+
fn extract_public_settings(settings: &Settings) -> ConfigurationPublic {
81+
let email_on_signup = match &settings.registration {
82+
Some(registration) => match &registration.email {
83+
Some(email) => {
84+
if email.required {
85+
EmailOnSignup::Required
86+
} else {
87+
EmailOnSignup::Optional
88+
}
89+
}
90+
None => EmailOnSignup::NotIncluded,
91+
},
92+
None => EmailOnSignup::NotIncluded,
93+
};
94+
95+
ConfigurationPublic {
96+
website_name: settings.website.name.clone(),
97+
tracker_url: settings.tracker.url.clone(),
98+
tracker_listed: settings.tracker.listed,
99+
tracker_private: settings.tracker.private,
100+
email_on_signup,
101+
}
102+
}
103+
104+
/// The public index configuration.
105+
/// There is an endpoint to get this configuration.
106+
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)]
107+
pub struct ConfigurationPublic {
108+
website_name: String,
109+
tracker_url: Url,
110+
tracker_listed: bool,
111+
tracker_private: bool,
112+
email_on_signup: EmailOnSignup,
113+
}
114+
115+
/// Whether the email is required on signup or not.
116+
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)]
117+
#[serde(rename_all = "lowercase")]
118+
pub enum EmailOnSignup {
119+
/// The email is required on signup.
120+
Required,
121+
/// The email is optional on signup.
122+
Optional,
123+
/// The email is not allowed on signup. It will only be ignored if provided.
124+
NotIncluded,
125+
}
126+
127+
impl Default for EmailOnSignup {
128+
fn default() -> Self {
129+
Self::Optional
130+
}
131+
}
132+
133+
impl fmt::Display for EmailOnSignup {
134+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
135+
let display_str = match self {
136+
EmailOnSignup::Required => "required",
137+
EmailOnSignup::Optional => "optional",
138+
EmailOnSignup::NotIncluded => "ignored",
139+
};
140+
write!(f, "{display_str}")
141+
}
142+
}
143+
144+
impl FromStr for EmailOnSignup {
145+
type Err = String;
146+
147+
fn from_str(s: &str) -> Result<Self, Self::Err> {
148+
match s.to_lowercase().as_str() {
149+
"required" => Ok(EmailOnSignup::Required),
150+
"optional" => Ok(EmailOnSignup::Optional),
151+
"none" => Ok(EmailOnSignup::NotIncluded),
152+
_ => Err(format!(
153+
"Unknown config 'email_on_signup' option (required, optional, none): {s}"
154+
)),
155+
}
156+
}
157+
}
158+
159+
#[cfg(test)]
160+
mod tests {
161+
use crate::config::Configuration;
162+
use crate::services::settings::{extract_public_settings, ConfigurationPublic, EmailOnSignup};
163+
164+
#[tokio::test]
165+
async fn configuration_should_return_only_public_settings() {
166+
let configuration = Configuration::default();
167+
let all_settings = configuration.get_all().await;
168+
169+
let email_on_signup = match &all_settings.registration {
170+
Some(registration) => match &registration.email {
171+
Some(email) => {
172+
if email.required {
173+
EmailOnSignup::Required
174+
} else {
175+
EmailOnSignup::Optional
176+
}
177+
}
178+
None => EmailOnSignup::NotIncluded,
179+
},
180+
None => EmailOnSignup::NotIncluded,
181+
};
182+
183+
assert_eq!(
184+
extract_public_settings(&all_settings),
185+
ConfigurationPublic {
186+
website_name: all_settings.website.name,
187+
tracker_url: all_settings.tracker.url,
188+
tracker_listed: all_settings.tracker.listed,
189+
tracker_private: all_settings.tracker.private,
190+
email_on_signup,
191+
}
192+
);
193+
}
194+
}

src/web/api/server/v1/contexts/settings/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,7 @@
183183
//!
184184
//! **Resource**
185185
//!
186-
//! Refer to the [`ConfigurationPublic`](crate::config::ConfigurationPublic)
186+
//! Refer to the [`ConfigurationPublic`](crate::services::settings::ConfigurationPublic)
187187
//! struct for more information about the response attributes.
188188
pub mod handlers;
189189
pub mod routes;

tests/e2e/web/api/v1/contexts/settings/contract.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
//! API contract for `settings` context.
22
3-
use torrust_index::config::EmailOnSignup;
3+
use torrust_index::services::settings::EmailOnSignup;
44
use torrust_index::web::api;
55

66
use crate::common::asserts::assert_json_ok_response;

0 commit comments

Comments
 (0)