Skip to content

Commit 9a2d5f5

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

File tree

3 files changed

+131
-105
lines changed

3 files changed

+131
-105
lines changed

‎src/handlers/http/modal/server.rs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -247,12 +247,12 @@ impl Server {
247247
web::resource("")
248248
.route(
249249
web::post()
250-
.to(dashboards::post)
250+
.to(dashboards::create_dashboard)
251251
.authorize(Action::CreateDashboard),
252252
)
253253
.route(
254254
web::get()
255-
.to(dashboards::list)
255+
.to(dashboards::list_dashboards)
256256
.authorize(Action::ListDashboard),
257257
),
258258
)
@@ -262,17 +262,17 @@ impl Server {
262262
web::resource("")
263263
.route(
264264
web::get()
265-
.to(dashboards::get)
265+
.to(dashboards::get_dashboard)
266266
.authorize(Action::GetDashboard),
267267
)
268268
.route(
269269
web::delete()
270-
.to(dashboards::delete)
270+
.to(dashboards::delete_dashboard)
271271
.authorize(Action::DeleteDashboard),
272272
)
273273
.route(
274274
web::put()
275-
.to(dashboards::update)
275+
.to(dashboards::update_dashboard)
276276
.authorize(Action::CreateDashboard),
277277
),
278278
)

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

Lines changed: 53 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -28,99 +28,86 @@ use actix_web::{
2828
HttpRequest, HttpResponse, Responder,
2929
};
3030
use http::StatusCode;
31-
use serde_json::{Error as SerdeError, Map};
31+
use serde_json::Error as SerdeError;
3232

33-
pub async fn list() -> Result<impl Responder, DashboardError> {
33+
pub async fn list_dashboards() -> Result<impl Responder, DashboardError> {
3434
let dashboards = DASHBOARDS.list_dashboards().await;
35-
//dashboards list should contain the title, author and modified date
36-
let dashboards: Vec<Map<String, serde_json::Value>> = dashboards
35+
let dashboard_summaries = dashboards
3736
.iter()
38-
.map(|dashboard| {
39-
let mut map = Map::new();
40-
map.insert(
41-
"title".to_string(),
42-
serde_json::Value::String(dashboard.title.clone()),
43-
);
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-
}
62-
map
63-
})
64-
.collect();
65-
Ok((web::Json(dashboards), StatusCode::OK))
37+
.map(|dashboard| dashboard.to_summary())
38+
.collect::<Vec<_>>();
39+
40+
Ok((web::Json(dashboard_summaries), StatusCode::OK))
6641
}
6742

68-
pub async fn get(dashboard_id: Path<String>) -> Result<impl Responder, DashboardError> {
43+
pub async fn get_dashboard(dashboard_id: Path<String>) -> Result<impl Responder, DashboardError> {
6944
let dashboard_id = validate_dashboard_id(dashboard_id.into_inner())?;
7045

71-
if let Some(dashboard) = DASHBOARDS.get_dashboard(dashboard_id).await {
72-
return Ok((web::Json(dashboard), StatusCode::OK));
73-
}
46+
let dashboard = DASHBOARDS
47+
.get_dashboard(dashboard_id)
48+
.await
49+
.ok_or_else(|| DashboardError::Metadata("Dashboard does not exist"))?;
7450

75-
Err(DashboardError::Metadata("Dashboard does not exist"))
51+
Ok((web::Json(dashboard), StatusCode::OK))
7652
}
7753

78-
pub async fn post(
54+
pub async fn create_dashboard(
7955
req: HttpRequest,
8056
Json(mut dashboard): Json<Dashboard>,
8157
) -> Result<impl Responder, DashboardError> {
8258
if dashboard.title.is_empty() {
8359
return Err(DashboardError::Metadata("Title must be provided"));
8460
}
85-
let mut user_id = get_user_from_request(&req)?;
86-
user_id = get_hash(&user_id);
87-
dashboard.author = Some(user_id.clone());
61+
62+
let user_id = get_hash(&get_user_from_request(&req)?);
8863

8964
DASHBOARDS.create(&user_id, &mut dashboard).await?;
9065
Ok((web::Json(dashboard), StatusCode::OK))
9166
}
9267

93-
pub async fn update(
68+
pub async fn update_dashboard(
9469
req: HttpRequest,
9570
dashboard_id: Path<String>,
9671
Json(mut dashboard): Json<Dashboard>,
9772
) -> Result<impl Responder, DashboardError> {
98-
let mut user_id = get_user_from_request(&req)?;
99-
user_id = get_hash(&user_id);
73+
let user_id = get_hash(&get_user_from_request(&req)?);
10074
let dashboard_id = validate_dashboard_id(dashboard_id.into_inner())?;
10175

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-
));
76+
// Validate all tiles have valid IDs
77+
if let Some(tiles) = &dashboard.tiles {
78+
if tiles.iter().any(|tile| tile.tile_id.is_nil()) {
79+
return Err(DashboardError::Metadata("Tile ID must be provided"));
80+
}
81+
}
82+
83+
// Check if tile_id are unique
84+
if let Some(tiles) = &dashboard.tiles {
85+
let unique_tiles: Vec<_> = tiles
86+
.iter()
87+
.map(|tile| tile.tile_id)
88+
.collect::<std::collections::HashSet<_>>()
89+
.into_iter()
90+
.collect();
91+
92+
if unique_tiles.len() != tiles.len() {
93+
return Err(DashboardError::Metadata("Tile IDs must be unique"));
10794
}
10895
}
109-
dashboard.author = Some(user_id.clone());
11096

11197
DASHBOARDS
11298
.update(&user_id, dashboard_id, &mut dashboard)
11399
.await?;
100+
114101
Ok((web::Json(dashboard), StatusCode::OK))
115102
}
116103

117-
pub async fn delete(
104+
pub async fn delete_dashboard(
118105
req: HttpRequest,
119106
dashboard_id: Path<String>,
120107
) -> Result<HttpResponse, DashboardError> {
121-
let mut user_id = get_user_from_request(&req)?;
122-
user_id = get_hash(&user_id);
108+
let user_id = get_hash(&get_user_from_request(&req)?);
123109
let dashboard_id = validate_dashboard_id(dashboard_id.into_inner())?;
110+
124111
DASHBOARDS.delete_dashboard(&user_id, dashboard_id).await?;
125112

126113
Ok(HttpResponse::Ok().finish())
@@ -131,22 +118,26 @@ pub async fn add_tile(
131118
dashboard_id: Path<String>,
132119
Json(tile): Json<Tile>,
133120
) -> Result<impl Responder, DashboardError> {
134-
let mut user_id = get_user_from_request(&req)?;
135-
user_id = get_hash(&user_id);
136-
let dashboard_id = validate_dashboard_id(dashboard_id.into_inner())?;
137-
138121
if tile.tile_id.is_nil() {
139-
return Err(DashboardError::Metadata(
140-
"Tile ID must be provided by the client",
141-
));
122+
return Err(DashboardError::Metadata("Tile ID must be provided"));
142123
}
143124

125+
let user_id = get_hash(&get_user_from_request(&req)?);
126+
let dashboard_id = validate_dashboard_id(dashboard_id.into_inner())?;
127+
144128
let mut dashboard = DASHBOARDS
145129
.get_dashboard_by_user(dashboard_id, &user_id)
146130
.await
147131
.ok_or(DashboardError::Unauthorized)?;
132+
148133
let tiles = dashboard.tiles.get_or_insert_with(Vec::new);
149-
tiles.push(tile.clone());
134+
135+
// check if the tile already exists
136+
if tiles.iter().any(|t| t.tile_id == tile.tile_id) {
137+
return Err(DashboardError::Metadata("Tile already exists"));
138+
}
139+
tiles.push(tile);
140+
150141
DASHBOARDS
151142
.update(&user_id, dashboard_id, &mut dashboard)
152143
.await?;
@@ -166,7 +157,7 @@ pub enum DashboardError {
166157
UserDoesNotExist(#[from] RBACError),
167158
#[error("Error: {0}")]
168159
Custom(String),
169-
#[error("Unauthorized to access resource")]
160+
#[error("Dashboard does not exist or is not accessible")]
170161
Unauthorized,
171162
}
172163

‎src/users/dashboards.rs‎

Lines changed: 73 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,53 @@ pub struct Dashboard {
5656
pub tiles: Option<Vec<Tile>>,
5757
}
5858

59+
impl Dashboard {
60+
pub fn set_metadata(&mut self, user_id: &str, dashboard_id: Option<Ulid>) {
61+
self.author = Some(user_id.to_string());
62+
self.dashboard_id = dashboard_id.or_else(|| Some(Ulid::new()));
63+
self.version = Some(CURRENT_DASHBOARD_VERSION.to_string());
64+
self.modified = Some(Utc::now());
65+
self.dashboard_type = Some(DashboardType::Dashboard);
66+
}
67+
68+
pub fn to_summary(&self) -> serde_json::Map<String, serde_json::Value> {
69+
let mut map = serde_json::Map::new();
70+
71+
map.insert(
72+
"title".to_string(),
73+
serde_json::Value::String(self.title.clone()),
74+
);
75+
76+
if let Some(author) = &self.author {
77+
map.insert(
78+
"author".to_string(),
79+
serde_json::Value::String(author.to_string()),
80+
);
81+
}
82+
83+
if let Some(modified) = &self.modified {
84+
map.insert(
85+
"modified".to_string(),
86+
serde_json::Value::String(modified.to_string()),
87+
);
88+
}
89+
90+
if let Some(dashboard_id) = &self.dashboard_id {
91+
map.insert(
92+
"dashboard_id".to_string(),
93+
serde_json::Value::String(dashboard_id.to_string()),
94+
);
95+
}
96+
97+
map
98+
}
99+
}
100+
101+
pub fn validate_dashboard_id(dashboard_id: String) -> Result<Ulid, DashboardError> {
102+
Ulid::from_string(&dashboard_id)
103+
.map_err(|_| DashboardError::Metadata("Dashboard ID must be provided"))
104+
}
105+
59106
#[derive(Default, Debug)]
60107
pub struct Dashboards(RwLock<Vec<Dashboard>>);
61108

@@ -64,11 +111,13 @@ impl Dashboards {
64111
let mut this = vec![];
65112
let store = PARSEABLE.storage.get_object_store();
66113
let all_dashboards = store.get_all_dashboards().await.unwrap_or_default();
114+
67115
for (_, dashboards) in all_dashboards {
68116
for dashboard in dashboards {
69117
if dashboard.is_empty() {
70118
continue;
71119
}
120+
72121
let dashboard_value = serde_json::from_slice::<serde_json::Value>(&dashboard)?;
73122
if let Ok(dashboard) = serde_json::from_value::<Dashboard>(dashboard_value) {
74123
this.retain(|d: &Dashboard| d.dashboard_id != dashboard.dashboard_id);
@@ -83,17 +132,14 @@ impl Dashboards {
83132
Ok(())
84133
}
85134

86-
pub async fn create(
135+
async fn save_dashboard(
87136
&self,
88137
user_id: &str,
89-
dashboard: &mut Dashboard,
138+
dashboard: &Dashboard,
90139
) -> Result<(), DashboardError> {
91-
let dashboard_id = Ulid::new();
92-
dashboard.author = Some(user_id.to_string());
93-
dashboard.dashboard_id = Some(dashboard_id);
94-
dashboard.version = Some(CURRENT_DASHBOARD_VERSION.to_string());
95-
dashboard.modified = Some(Utc::now());
96-
dashboard.dashboard_type = Some(DashboardType::Dashboard);
140+
let dashboard_id = dashboard
141+
.dashboard_id
142+
.ok_or(DashboardError::Metadata("Dashboard ID must be provided"))?;
97143
let path = dashboard_path(user_id, &format!("{}.json", dashboard_id));
98144

99145
let store = PARSEABLE.storage.get_object_store();
@@ -102,6 +148,17 @@ impl Dashboards {
102148
.put_object(&path, Bytes::from(dashboard_bytes))
103149
.await?;
104150

151+
Ok(())
152+
}
153+
154+
pub async fn create(
155+
&self,
156+
user_id: &str,
157+
dashboard: &mut Dashboard,
158+
) -> Result<(), DashboardError> {
159+
dashboard.set_metadata(user_id, None);
160+
161+
self.save_dashboard(user_id, dashboard).await?;
105162
self.0.write().await.push(dashboard.clone());
106163

107164
Ok(())
@@ -120,25 +177,13 @@ impl Dashboards {
120177
{
121178
return Err(DashboardError::Unauthorized);
122179
}
123-
dashboard.author = Some(user_id.to_string());
124-
dashboard.dashboard_id = Some(dashboard_id);
125-
dashboard.version = Some(CURRENT_DASHBOARD_VERSION.to_string());
126-
dashboard.modified = Some(Utc::now());
127-
dashboard.dashboard_type = Some(DashboardType::Dashboard);
128180

129-
let path = dashboard_path(user_id, &format!("{}.json", dashboard_id));
181+
dashboard.set_metadata(user_id, Some(dashboard_id));
182+
self.save_dashboard(user_id, dashboard).await?;
130183

131-
let store = PARSEABLE.storage.get_object_store();
132-
let dashboard_bytes = serde_json::to_vec(&dashboard)?;
133-
store
134-
.put_object(&path, Bytes::from(dashboard_bytes))
135-
.await?;
136-
137-
self.0
138-
.write()
139-
.await
140-
.retain(|d| d.dashboard_id != dashboard.dashboard_id);
141-
self.0.write().await.push(dashboard.clone());
184+
let mut dashboards = self.0.write().await;
185+
dashboards.retain(|d| d.dashboard_id != dashboard.dashboard_id);
186+
dashboards.push(dashboard.clone());
142187

143188
Ok(())
144189
}
@@ -159,10 +204,11 @@ impl Dashboards {
159204
let path = dashboard_path(user_id, &format!("{}.json", dashboard_id));
160205
let store = PARSEABLE.storage.get_object_store();
161206
store.delete_object(&path).await?;
207+
162208
self.0.write().await.retain(|d| {
163209
d.dashboard_id
164210
.as_ref()
165-
.map_or(false, |id| *id == dashboard_id)
211+
.map_or(true, |id| *id != dashboard_id)
166212
});
167213

168214
Ok(())
@@ -200,17 +246,6 @@ impl Dashboards {
200246
}
201247

202248
pub async fn list_dashboards(&self) -> Vec<Dashboard> {
203-
let read = self.0.read().await;
204-
205-
let mut dashboards = Vec::new();
206-
207-
for d in read.iter() {
208-
dashboards.push(d.clone());
209-
}
210-
dashboards
249+
self.0.read().await.clone()
211250
}
212251
}
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)