Skip to content

Commit 40fd7a1

Browse files
nikhilsinhaparseablenikhilsinhaparseable
committed
refactor
1 parent 9d1419c commit 40fd7a1

File tree

2 files changed

+65
-44
lines changed

2 files changed

+65
-44
lines changed

‎src/handlers/http/users/dashboards.rs

Lines changed: 46 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
use crate::{
2020
handlers::http::rbac::RBACError,
2121
storage::ObjectStorageError,
22-
users::dashboards::{Dashboard, Tile, DASHBOARDS},
22+
users::dashboards::{validate_dashboard_id, Dashboard, Tile, DASHBOARDS},
2323
utils::{get_hash, get_user_from_request},
2424
};
2525
use actix_web::{
@@ -29,7 +29,6 @@ use actix_web::{
2929
};
3030
use http::StatusCode;
3131
use serde_json::{Error as SerdeError, Map};
32-
use ulid::Ulid;
3332

3433
pub async fn list() -> Result<impl Responder, DashboardError> {
3534
let dashboards = DASHBOARDS.list_dashboards().await;
@@ -42,30 +41,32 @@ pub async fn list() -> Result<impl Responder, DashboardError> {
4241
"title".to_string(),
4342
serde_json::Value::String(dashboard.title.clone()),
4443
);
45-
map.insert(
46-
"author".to_string(),
47-
serde_json::Value::String(dashboard.author.as_ref().unwrap().clone()),
48-
);
49-
map.insert(
50-
"modified".to_string(),
51-
serde_json::Value::String(dashboard.modified.unwrap().to_string()),
52-
);
53-
map.insert(
54-
"dashboard_id".to_string(),
55-
serde_json::Value::String(dashboard.dashboard_id.unwrap().to_string()),
56-
);
44+
if let Some(author) = &dashboard.author {
45+
map.insert(
46+
"author".to_string(),
47+
serde_json::Value::String(author.to_string()),
48+
);
49+
}
50+
if let Some(modified) = &dashboard.modified {
51+
map.insert(
52+
"modified".to_string(),
53+
serde_json::Value::String(modified.to_string()),
54+
);
55+
}
56+
if let Some(dashboard_id) = &dashboard.dashboard_id {
57+
map.insert(
58+
"dashboard_id".to_string(),
59+
serde_json::Value::String(dashboard_id.to_string()),
60+
);
61+
}
5762
map
5863
})
5964
.collect();
6065
Ok((web::Json(dashboards), StatusCode::OK))
6166
}
6267

6368
pub async fn get(dashboard_id: Path<String>) -> Result<impl Responder, DashboardError> {
64-
let dashboard_id = if let Ok(dashboard_id) = Ulid::from_string(&dashboard_id.into_inner()) {
65-
dashboard_id
66-
} else {
67-
return Err(DashboardError::Metadata("Invalid dashboard ID"));
68-
};
69+
let dashboard_id = validate_dashboard_id(dashboard_id.into_inner())?;
6970

7071
if let Some(dashboard) = DASHBOARDS.get_dashboard(dashboard_id).await {
7172
return Ok((web::Json(dashboard), StatusCode::OK));
@@ -78,6 +79,9 @@ pub async fn post(
7879
req: HttpRequest,
7980
Json(mut dashboard): Json<Dashboard>,
8081
) -> Result<impl Responder, DashboardError> {
82+
if dashboard.title.is_empty() {
83+
return Err(DashboardError::Metadata("Title must be provided"));
84+
}
8185
let mut user_id = get_user_from_request(&req)?;
8286
user_id = get_hash(&user_id);
8387
dashboard.author = Some(user_id.clone());
@@ -93,11 +97,15 @@ pub async fn update(
9397
) -> Result<impl Responder, DashboardError> {
9498
let mut user_id = get_user_from_request(&req)?;
9599
user_id = get_hash(&user_id);
96-
let dashboard_id = if let Ok(dashboard_id) = Ulid::from_string(&dashboard_id.into_inner()) {
97-
dashboard_id
98-
} else {
99-
return Err(DashboardError::Metadata("Invalid dashboard ID"));
100-
};
100+
let dashboard_id = validate_dashboard_id(dashboard_id.into_inner())?;
101+
102+
for tile in dashboard.tiles.as_ref().unwrap_or(&Vec::new()) {
103+
if tile.tile_id.is_nil() {
104+
return Err(DashboardError::Metadata(
105+
"Tile ID must be provided by the client",
106+
));
107+
}
108+
}
101109
dashboard.author = Some(user_id.clone());
102110

103111
DASHBOARDS
@@ -112,11 +120,7 @@ pub async fn delete(
112120
) -> Result<HttpResponse, DashboardError> {
113121
let mut user_id = get_user_from_request(&req)?;
114122
user_id = get_hash(&user_id);
115-
let dashboard_id = if let Ok(dashboard_id) = Ulid::from_string(&dashboard_id.into_inner()) {
116-
dashboard_id
117-
} else {
118-
return Err(DashboardError::Metadata("Invalid dashboard ID"));
119-
};
123+
let dashboard_id = validate_dashboard_id(dashboard_id.into_inner())?;
120124
DASHBOARDS.delete_dashboard(&user_id, dashboard_id).await?;
121125

122126
Ok(HttpResponse::Ok().finish())
@@ -129,17 +133,20 @@ pub async fn add_tile(
129133
) -> Result<impl Responder, DashboardError> {
130134
let mut user_id = get_user_from_request(&req)?;
131135
user_id = get_hash(&user_id);
132-
let dashboard_id = if let Ok(dashboard_id) = Ulid::from_string(&dashboard_id.into_inner()) {
133-
dashboard_id
134-
} else {
135-
return Err(DashboardError::Metadata("Invalid dashboard ID"));
136-
};
136+
let dashboard_id = validate_dashboard_id(dashboard_id.into_inner())?;
137+
138+
if tile.tile_id.is_nil() {
139+
return Err(DashboardError::Metadata(
140+
"Tile ID must be provided by the client",
141+
));
142+
}
137143

138144
let mut dashboard = DASHBOARDS
139145
.get_dashboard_by_user(dashboard_id, &user_id)
140146
.await
141-
.ok_or(DashboardError::Metadata("Dashboard does not exist"))?;
142-
dashboard.tiles.as_mut().unwrap().push(tile.clone());
147+
.ok_or(DashboardError::Unauthorized)?;
148+
let tiles = dashboard.tiles.get_or_insert_with(Vec::new);
149+
tiles.push(tile.clone());
143150
DASHBOARDS
144151
.update(&user_id, dashboard_id, &mut dashboard)
145152
.await?;
@@ -159,6 +166,8 @@ pub enum DashboardError {
159166
UserDoesNotExist(#[from] RBACError),
160167
#[error("Error: {0}")]
161168
Custom(String),
169+
#[error("Unauthorized to access resource")]
170+
Unauthorized,
162171
}
163172

164173
impl actix_web::ResponseError for DashboardError {
@@ -169,6 +178,7 @@ impl actix_web::ResponseError for DashboardError {
169178
Self::Metadata(_) => StatusCode::BAD_REQUEST,
170179
Self::UserDoesNotExist(_) => StatusCode::NOT_FOUND,
171180
Self::Custom(_) => StatusCode::INTERNAL_SERVER_ERROR,
181+
Self::Unauthorized => StatusCode::UNAUTHORIZED,
172182
}
173183
}
174184

‎src/users/dashboards.rs

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ impl Dashboards {
118118
.await
119119
.is_none()
120120
{
121-
return Err(DashboardError::Metadata("Dashboard does not exist"));
121+
return Err(DashboardError::Unauthorized);
122122
}
123123
dashboard.author = Some(user_id.to_string());
124124
dashboard.dashboard_id = Some(dashboard_id);
@@ -153,16 +153,17 @@ impl Dashboards {
153153
.await
154154
.is_none()
155155
{
156-
return Err(DashboardError::Metadata("Dashboard does not exist"));
156+
return Err(DashboardError::Unauthorized);
157157
}
158158

159159
let path = dashboard_path(user_id, &format!("{}.json", dashboard_id));
160160
let store = PARSEABLE.storage.get_object_store();
161161
store.delete_object(&path).await?;
162-
self.0
163-
.write()
164-
.await
165-
.retain(|d| *d.dashboard_id.as_ref().unwrap() != dashboard_id);
162+
self.0.write().await.retain(|d| {
163+
d.dashboard_id
164+
.as_ref()
165+
.map_or(false, |id| *id == dashboard_id)
166+
});
166167

167168
Ok(())
168169
}
@@ -172,7 +173,11 @@ impl Dashboards {
172173
.read()
173174
.await
174175
.iter()
175-
.find(|d| *d.dashboard_id.as_ref().unwrap() == dashboard_id)
176+
.find(|d| {
177+
d.dashboard_id
178+
.as_ref()
179+
.map_or(false, |id| *id == dashboard_id)
180+
})
176181
.cloned()
177182
}
178183

@@ -186,7 +191,9 @@ impl Dashboards {
186191
.await
187192
.iter()
188193
.find(|d| {
189-
*d.dashboard_id.as_ref().unwrap() == dashboard_id
194+
d.dashboard_id
195+
.as_ref()
196+
.map_or(false, |id| *id == dashboard_id)
190197
&& d.author == Some(user_id.to_string())
191198
})
192199
.cloned()
@@ -203,3 +210,7 @@ impl Dashboards {
203210
dashboards
204211
}
205212
}
213+
214+
pub fn validate_dashboard_id(dashboard_id: String) -> Result<Ulid, DashboardError> {
215+
Ulid::from_string(&dashboard_id).map_err(|_| DashboardError::Metadata("Invalid dashboard ID"))
216+
}

0 commit comments

Comments
 (0)