Skip to content

Commit f85e153

Browse files
josecelanojosecelano
committed
feat!: [#144] don't allow to update settings
Without restarting the application. This feature was using the `config.toml` file. That approach is not good for theses reasons: - If you use env vars to inject the settings, there is no `config.toml` file. - In dockerized (clouds) envs it's harder to mount a file than injecting env vars. Sometimes it's only allowed to mount a single file.
1 parent f0e28a6 commit f85e153

File tree

8 files changed

+6
-120
lines changed

8 files changed

+6
-120
lines changed

‎src/config.rs‎

Lines changed: 0 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -372,34 +372,6 @@ impl Configuration {
372372
fs::write(config_path, toml_string).expect("Could not write to file!");
373373
}
374374

375-
/// Update the settings file based upon a supplied `new_settings`.
376-
///
377-
/// # Errors
378-
///
379-
/// Todo: Make an error if the save fails.
380-
///
381-
/// # Panics
382-
///
383-
/// Will panic if the configuration file path is not defined. That happens
384-
/// when the configuration was loaded from the environment variable.
385-
pub async fn update_settings(&self, new_settings: TorrustBackend) -> Result<(), ()> {
386-
match &self.config_path {
387-
Some(config_path) => {
388-
let mut settings = self.settings.write().await;
389-
*settings = new_settings;
390-
391-
drop(settings);
392-
393-
let _ = self.save_to_file(config_path).await;
394-
395-
Ok(())
396-
}
397-
None => panic!(
398-
"Cannot update settings when the config file path is not defined. For example: when it's loaded from env var."
399-
),
400-
}
401-
}
402-
403375
pub async fn get_all(&self) -> TorrustBackend {
404376
let settings_lock = self.settings.read().await;
405377

‎src/services/settings.rs‎

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -37,25 +37,6 @@ impl Service {
3737
Ok(self.configuration.get_all().await)
3838
}
3939

40-
/// It updates all the settings.
41-
///
42-
/// # Errors
43-
///
44-
/// It returns an error if the user does not have the required permissions.
45-
pub async fn update_all(&self, torrust_backend: TorrustBackend, user_id: &UserId) -> Result<TorrustBackend, ServiceError> {
46-
let user = self.user_repository.get_compact(user_id).await?;
47-
48-
// Check if user is administrator
49-
// todo: extract authorization service
50-
if !user.administrator {
51-
return Err(ServiceError::Unauthorized);
52-
}
53-
54-
let _ = self.configuration.update_settings(torrust_backend).await;
55-
56-
Ok(self.configuration.get_all().await)
57-
}
58-
5940
/// It gets only the public settings.
6041
///
6142
/// # Errors

‎src/web/api/v1/contexts/settings/handlers.rs‎

Lines changed: 2 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,12 @@
22
//! context.
33
use std::sync::Arc;
44

5-
use axum::extract::{self, State};
5+
use axum::extract::State;
66
use axum::response::{IntoResponse, Json, Response};
77

88
use crate::common::AppData;
9-
use crate::config::TorrustBackend;
109
use crate::web::api::v1::extractors::bearer_token::Extract;
11-
use crate::web::api::v1::responses::{self};
10+
use crate::web::api::v1::responses;
1211

1312
/// Get all settings.
1413
///
@@ -46,31 +45,3 @@ pub async fn get_site_name_handler(State(app_data): State<Arc<AppData>>) -> Resp
4645

4746
Json(responses::OkResponseData { data: site_name }).into_response()
4847
}
49-
50-
/// Update all the settings.
51-
///
52-
/// # Errors
53-
///
54-
/// This function will return an error if:
55-
///
56-
/// - The user does not have permission to update the settings.
57-
/// - The settings could not be updated because they were loaded from env vars.
58-
/// See <https://github.com/torrust/torrust-index-backend/issues/144.>
59-
#[allow(clippy::unused_async)]
60-
pub async fn update_handler(
61-
State(app_data): State<Arc<AppData>>,
62-
Extract(maybe_bearer_token): Extract,
63-
extract::Json(torrust_backend): extract::Json<TorrustBackend>,
64-
) -> Response {
65-
let user_id = match app_data.auth.get_user_id_from_bearer_token(&maybe_bearer_token).await {
66-
Ok(user_id) => user_id,
67-
Err(error) => return error.into_response(),
68-
};
69-
70-
let new_settings = match app_data.settings_service.update_all(torrust_backend, &user_id).await {
71-
Ok(new_settings) => new_settings,
72-
Err(error) => return error.into_response(),
73-
};
74-
75-
Json(responses::OkResponseData { data: new_settings }).into_response()
76-
}

‎src/web/api/v1/contexts/settings/routes.rs‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,17 +3,16 @@
33
//! Refer to the [API endpoint documentation](crate::web::api::v1::contexts::settings).
44
use std::sync::Arc;
55

6-
use axum::routing::{get, post};
6+
use axum::routing::get;
77
use axum::Router;
88

9-
use super::handlers::{get_all_handler, get_public_handler, get_site_name_handler, update_handler};
9+
use super::handlers::{get_all_handler, get_public_handler, get_site_name_handler};
1010
use crate::common::AppData;
1111

1212
/// Routes for the [`category`](crate::web::api::v1::contexts::category) API context.
1313
pub fn router(app_data: Arc<AppData>) -> Router {
1414
Router::new()
1515
.route("/", get(get_all_handler).with_state(app_data.clone()))
1616
.route("/name", get(get_site_name_handler).with_state(app_data.clone()))
17-
.route("/public", get(get_public_handler).with_state(app_data.clone()))
18-
.route("/", post(update_handler).with_state(app_data))
17+
.route("/public", get(get_public_handler).with_state(app_data))
1918
}

‎tests/common/client.rs‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ use serde::Serialize;
33

44
use super::connection_info::ConnectionInfo;
55
use super::contexts::category::forms::{AddCategoryForm, DeleteCategoryForm};
6-
use super::contexts::settings::form::UpdateSettings;
76
use super::contexts::tag::forms::{AddTagForm, DeleteTagForm};
87
use super::contexts::torrent::forms::UpdateTorrentFrom;
98
use super::contexts::torrent::requests::InfoHash;
@@ -17,8 +16,7 @@ pub struct Client {
1716
}
1817

1918
impl Client {
20-
// todo: forms in POST requests can be passed by reference. It's already
21-
// changed for the `update_settings` method.
19+
// todo: forms in POST requests can be passed by reference.
2220

2321
fn base_path() -> String {
2422
"/v1".to_string()
@@ -104,10 +102,6 @@ impl Client {
104102
self.http_client.get("/settings", Query::empty()).await
105103
}
106104

107-
pub async fn update_settings(&self, update_settings_form: &UpdateSettings) -> TextResponse {
108-
self.http_client.post("/settings", &update_settings_form).await
109-
}
110-
111105
// Context: torrent
112106

113107
pub async fn get_torrents(&self, params: Query) -> TextResponse {

‎tests/common/contexts/settings/form.rs‎

Lines changed: 0 additions & 3 deletions
This file was deleted.

‎tests/common/contexts/settings/mod.rs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
pub mod form;
21
pub mod responses;
32

43
use serde::{Deserialize, Serialize};

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

Lines changed: 0 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -65,30 +65,3 @@ async fn it_should_allow_admins_to_get_all_the_settings() {
6565

6666
assert_json_ok_response(&response);
6767
}
68-
69-
#[tokio::test]
70-
async fn it_should_allow_admins_to_update_all_the_settings() {
71-
let mut env = TestEnv::new();
72-
env.start(api::Version::V1).await;
73-
74-
if !env.is_isolated() {
75-
// This test can't be executed in a non-isolated environment because
76-
// it will change the settings for all the other tests.
77-
return;
78-
}
79-
80-
let logged_in_admin = new_logged_in_admin(&env).await;
81-
let client = Client::authenticated(&env.server_socket_addr().unwrap(), &logged_in_admin.token);
82-
83-
let mut new_settings = env.server_settings().unwrap();
84-
85-
new_settings.website.name = "UPDATED NAME".to_string();
86-
87-
let response = client.update_settings(&new_settings).await;
88-
89-
let res: AllSettingsResponse = serde_json::from_str(&response.body).unwrap();
90-
91-
assert_eq!(res.data, new_settings);
92-
93-
assert_json_ok_response(&response);
94-
}

0 commit comments

Comments
 (0)