Skip to content

Commit dbf882d

Browse files
nikhilsinhaparseablenikhilsinhaparseable
committed
restriction on other fields, refactor
1 parent 9a9f929 commit dbf882d

File tree

2 files changed

+187
-78
lines changed

2 files changed

+187
-78
lines changed

‎src/alerts/alert_structs.rs‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,27 @@ use serde_json::Value;
2424
use tokio::sync::{RwLock, mpsc};
2525
use ulid::Ulid;
2626

27+
const RESERVED_FIELDS: &[&str] = &[
28+
"id",
29+
"version",
30+
"severity",
31+
"title",
32+
"query",
33+
"datasets",
34+
"alertType",
35+
"anomalyConfig",
36+
"forecastConfig",
37+
"thresholdConfig",
38+
"notificationConfig",
39+
"evalConfig",
40+
"targets",
41+
"tags",
42+
"state",
43+
"notificationState",
44+
"created",
45+
"lastTriggeredAt",
46+
];
47+
2748
use crate::{
2849
alerts::{
2950
AlertError, CURRENT_ALERTS_VERSION,
@@ -306,6 +327,26 @@ pub struct AlertRequest {
306327

307328
impl AlertRequest {
308329
pub async fn into(self) -> Result<AlertConfig, AlertError> {
330+
// Validate that other_fields doesn't contain reserved field names
331+
if let Some(ref other_fields) = self.other_fields {
332+
// Limit other_fields to maximum 10 fields
333+
if other_fields.len() > 10 {
334+
return Err(AlertError::ValidationFailure(format!(
335+
"other_fields can contain at most 10 fields, found {}",
336+
other_fields.len()
337+
)));
338+
}
339+
340+
for key in other_fields.keys() {
341+
if RESERVED_FIELDS.contains(&key.as_str()) {
342+
return Err(AlertError::ValidationFailure(format!(
343+
"Field '{}' cannot be in other_fields as it's a reserved field name",
344+
key
345+
)));
346+
}
347+
}
348+
}
349+
309350
// Validate that all target IDs exist
310351
for id in &self.targets {
311352
TARGETS.get_target_by_id(id).await?;

‎src/handlers/http/alerts.rs‎

Lines changed: 146 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -38,97 +38,121 @@ use actix_web::{
3838
use chrono::{DateTime, Utc};
3939
use ulid::Ulid;
4040

41-
// GET /alerts
42-
/// User needs at least a read access to the stream(s) that is being referenced in an alert
43-
/// Read all alerts then return alerts which satisfy the condition
44-
pub async fn list(req: HttpRequest) -> Result<impl Responder, AlertError> {
45-
let session_key = extract_session_key_from_req(&req)?;
46-
let query_map = web::Query::<HashMap<String, String>>::from_query(req.query_string())
47-
.map_err(|_| AlertError::InvalidQueryParameter("malformed query parameters".to_string()))?;
41+
// Reserved query parameter names that are not treated as other_fields filters
42+
const RESERVED_PARAMS: [&str; 3] = ["tags", "offset", "limit"];
43+
const MAX_LIMIT: usize = 1000;
44+
const DEFAULT_LIMIT: usize = 100;
45+
46+
/// Query parameters for listing alerts
47+
struct ListQueryParams {
48+
tags_list: Vec<String>,
49+
offset: usize,
50+
limit: usize,
51+
other_fields_filters: HashMap<String, String>,
52+
}
4853

54+
/// Parse and validate query parameters for listing alerts
55+
fn parse_list_query_params(
56+
query_map: &HashMap<String, String>,
57+
) -> Result<ListQueryParams, AlertError> {
4958
let mut tags_list = Vec::new();
5059
let mut offset = 0usize;
51-
let mut limit = 100usize; // Default limit
52-
const MAX_LIMIT: usize = 1000; // Maximum allowed limit
53-
54-
// Reserved query parameter names that are not treated as other_fields filters
55-
const RESERVED_PARAMS: [&str; 3] = ["tags", "offset", "limit"];
60+
let mut limit = DEFAULT_LIMIT;
5661
let mut other_fields_filters: HashMap<String, String> = HashMap::new();
5762

58-
// Parse query parameters
59-
if !query_map.is_empty() {
60-
// Parse tags parameter
61-
if let Some(tags) = query_map.get("tags") {
62-
tags_list = tags
63-
.split(',')
64-
.map(|s| s.trim().to_string())
65-
.filter(|s| !s.is_empty())
66-
.collect();
67-
if tags_list.is_empty() {
68-
return Err(AlertError::InvalidQueryParameter(
69-
"empty tags not allowed with query param tags".to_string(),
70-
));
71-
}
72-
}
63+
if query_map.is_empty() {
64+
return Ok(ListQueryParams {
65+
tags_list,
66+
offset,
67+
limit,
68+
other_fields_filters,
69+
});
70+
}
7371

74-
// Parse offset parameter
75-
if let Some(offset_str) = query_map.get("offset") {
76-
offset = offset_str.parse().map_err(|_| {
77-
AlertError::InvalidQueryParameter("offset is not a valid number".to_string())
78-
})?;
72+
// Parse tags parameter
73+
if let Some(tags) = query_map.get("tags") {
74+
tags_list = tags
75+
.split(',')
76+
.map(|s| s.trim().to_string())
77+
.filter(|s| !s.is_empty())
78+
.collect();
79+
if tags_list.is_empty() {
80+
return Err(AlertError::InvalidQueryParameter(
81+
"empty tags not allowed with query param tags".to_string(),
82+
));
7983
}
84+
}
85+
86+
// Parse offset parameter
87+
if let Some(offset_str) = query_map.get("offset") {
88+
offset = offset_str.parse().map_err(|_| {
89+
AlertError::InvalidQueryParameter("offset is not a valid number".to_string())
90+
})?;
91+
}
8092

81-
// Parse limit parameter
82-
if let Some(limit_str) = query_map.get("limit") {
83-
limit = limit_str.parse().map_err(|_| {
84-
AlertError::InvalidQueryParameter("limit is not a valid number".to_string())
85-
})?;
86-
87-
// Validate limit bounds
88-
if limit == 0 || limit > MAX_LIMIT {
89-
return Err(AlertError::InvalidQueryParameter(
90-
"limit should be between 1 and 1000".to_string(),
91-
));
92-
}
93+
// Parse limit parameter
94+
if let Some(limit_str) = query_map.get("limit") {
95+
limit = limit_str.parse().map_err(|_| {
96+
AlertError::InvalidQueryParameter("limit is not a valid number".to_string())
97+
})?;
98+
99+
// Validate limit bounds
100+
if limit == 0 || limit > MAX_LIMIT {
101+
return Err(AlertError::InvalidQueryParameter(
102+
"limit should be between 1 and 1000".to_string(),
103+
));
93104
}
105+
}
94106

95-
// Collect all other query parameters as potential other_fields filters
96-
for (key, value) in query_map.iter() {
97-
if !RESERVED_PARAMS.contains(&key.as_str()) {
98-
other_fields_filters.insert(key.clone(), value.clone());
99-
}
107+
// Collect all other query parameters as potential other_fields filters
108+
for (key, value) in query_map.iter() {
109+
if !RESERVED_PARAMS.contains(&key.as_str()) {
110+
other_fields_filters.insert(key.clone(), value.clone());
100111
}
101112
}
102-
let guard = ALERTS.read().await;
103-
let alerts = if let Some(alerts) = guard.as_ref() {
104-
alerts
105-
} else {
106-
return Err(AlertError::CustomError("No AlertManager set".into()));
107-
};
108113

109-
let alerts = alerts.list_alerts_for_user(session_key, tags_list).await?;
110-
let mut alerts_summary = alerts
111-
.iter()
112-
.map(|alert| alert.to_summary())
113-
.collect::<Vec<_>>();
114+
Ok(ListQueryParams {
115+
tags_list,
116+
offset,
117+
limit,
118+
other_fields_filters,
119+
})
120+
}
114121

115-
// Filter by other_fields if any filters are specified
116-
if !other_fields_filters.is_empty() {
117-
alerts_summary.retain(|alert_summary| {
118-
// Check if all specified other_fields filters match
119-
other_fields_filters
120-
.iter()
121-
.all(|(filter_key, filter_value)| {
122-
alert_summary
123-
.get(filter_key)
124-
.and_then(|v| v.as_str())
125-
.map(|alert_value| alert_value == filter_value)
126-
.unwrap_or(false)
127-
})
128-
});
122+
/// Filter alerts by other_fields
123+
fn filter_by_other_fields(
124+
mut alerts_summary: Vec<serde_json::Map<String, serde_json::Value>>,
125+
filters: &HashMap<String, String>,
126+
) -> Vec<serde_json::Map<String, serde_json::Value>> {
127+
if filters.is_empty() {
128+
return alerts_summary;
129129
}
130130

131-
// Sort by state priority (Triggered > NotTriggered) then by severity (Critical > High > Medium > Low)
131+
alerts_summary.retain(|alert_summary| {
132+
// Check if all specified other_fields filters match
133+
filters.iter().all(|(filter_key, filter_value)| {
134+
alert_summary
135+
.get(filter_key)
136+
.map(|v| {
137+
// Convert JSON value to string for comparison
138+
let value_as_string = if v.is_string() {
139+
// For strings, use the raw string value without quotes
140+
v.as_str().unwrap_or("").to_string()
141+
} else {
142+
// For numbers, booleans, arrays, objects, use JSON representation
143+
v.to_string()
144+
};
145+
value_as_string == *filter_value
146+
})
147+
.unwrap_or(false)
148+
})
149+
});
150+
151+
alerts_summary
152+
}
153+
154+
/// Sort alerts by state, severity, and title
155+
fn sort_alerts(alerts_summary: &mut [serde_json::Map<String, serde_json::Value>]) {
132156
alerts_summary.sort_by(|a, b| {
133157
// Parse state and severity from JSON values back to enums
134158
let state_a = a
@@ -156,22 +180,66 @@ pub async fn list(req: HttpRequest) -> Result<impl Responder, AlertError> {
156180
.unwrap_or(Severity::Low);
157181

158182
let title_a = a.get("title").and_then(|v| v.as_str()).unwrap_or("");
159-
160183
let title_b = b.get("title").and_then(|v| v.as_str()).unwrap_or("");
161184

162-
// First sort by state, then by severity
185+
// First sort by state, then by severity, then by title
163186
state_a
164187
.cmp(&state_b)
165188
.then_with(|| severity_a.cmp(&severity_b))
166189
.then_with(|| title_a.cmp(title_b))
167190
});
191+
}
168192

169-
let paginated_alerts = alerts_summary
193+
/// Paginate alerts
194+
fn paginate_alerts(
195+
alerts_summary: Vec<serde_json::Map<String, serde_json::Value>>,
196+
offset: usize,
197+
limit: usize,
198+
) -> Vec<serde_json::Map<String, serde_json::Value>> {
199+
alerts_summary
170200
.into_iter()
171201
.skip(offset)
172202
.take(limit)
203+
.collect()
204+
}
205+
206+
// GET /alerts
207+
/// User needs at least a read access to the stream(s) that is being referenced in an alert
208+
/// Read all alerts then return alerts which satisfy the condition
209+
pub async fn list(req: HttpRequest) -> Result<impl Responder, AlertError> {
210+
let session_key = extract_session_key_from_req(&req)?;
211+
let query_map = web::Query::<HashMap<String, String>>::from_query(req.query_string())
212+
.map_err(|_| AlertError::InvalidQueryParameter("malformed query parameters".to_string()))?;
213+
214+
// Parse and validate query parameters
215+
let params = parse_list_query_params(&query_map)?;
216+
217+
// Get alerts from the manager
218+
let guard = ALERTS.read().await;
219+
let alerts = if let Some(alerts) = guard.as_ref() {
220+
alerts
221+
} else {
222+
return Err(AlertError::CustomError("No AlertManager set".into()));
223+
};
224+
225+
// Fetch alerts for the user
226+
let alerts = alerts
227+
.list_alerts_for_user(session_key, params.tags_list)
228+
.await?;
229+
let mut alerts_summary = alerts
230+
.iter()
231+
.map(|alert| alert.to_summary())
173232
.collect::<Vec<_>>();
174233

234+
// Filter by other_fields
235+
alerts_summary = filter_by_other_fields(alerts_summary, &params.other_fields_filters);
236+
237+
// Sort alerts
238+
sort_alerts(&mut alerts_summary);
239+
240+
// Paginate results
241+
let paginated_alerts = paginate_alerts(alerts_summary, params.offset, params.limit);
242+
175243
Ok(web::Json(paginated_alerts))
176244
}
177245

0 commit comments

Comments
 (0)