phase 5 qa and review pass

This commit is contained in:
2026-08-18 22:21:45 +01:00
parent bd7459e455
commit 42a697aa57
4 changed files with 295 additions and 168 deletions
+143 -143
View File
@@ -1,3 +1,4 @@
// src/api.rs
use crate::db;
use crate::types::{
Config, LandingDto, LookupDto, MonthlyAggregate, PriceTrend, SpeciesDto, SummaryDto, TopSpecies,
@@ -31,7 +32,7 @@ struct StaticAssets;
#[derive(Debug, Clone, Deserialize)]
pub struct LandingsQuery {
pub month: Option<String>, // Kept for backward compatibility or single-month quick lookups
pub month: Option<String>,
pub month_from: Option<String>,
pub month_to: Option<String>,
pub species: Option<String>,
@@ -164,7 +165,6 @@ async fn get_landings(
Query(params): Query<LandingsQuery>,
) -> ApiResult<Json<Vec<LandingDto>>> {
let conn = state.conn.clone();
// Apply hard cap to prevent abuse
let capped_limit = std::cmp::min(params.limit, MAX_LIMIT);
let rows = tokio::task::spawn_blocking(move || -> db::Result<Vec<LandingDto>> {
@@ -178,14 +178,12 @@ async fn get_landings(
let mut args: Vec<Box<dyn duckdb::ToSql>> = Vec::new();
let mut idx = 1;
// Single month filter (legacy)
if let Some(ref month) = params.month {
sql.push_str(&format!(" AND month = ${idx}"));
args.push(Box::new(month.clone()));
idx += 1;
}
// Range filter (new)
if let Some(ref month_from) = params.month_from {
sql.push_str(&format!(" AND month >= ${idx}"));
args.push(Box::new(month_from.clone()));
@@ -466,14 +464,12 @@ async fn export_parquet(State(state): State<AppState>) -> ApiResult<Response> {
async fn static_handler(uri: axum::http::Uri) -> Response {
let path = uri.path().trim_start_matches('/');
// Strip "static/" prefix if present
let asset_path = if let Some(stripped) = path.strip_prefix("static/") {
stripped
} else {
path
};
// If empty path (e.g., visiting "/"), serve index.html directly
let asset_path = if asset_path.is_empty() {
"index.html"
} else {
@@ -493,7 +489,12 @@ async fn static_handler(uri: axum::http::Uri) -> Response {
.status(StatusCode::OK)
.header(header::CONTENT_TYPE, mime)
.body(Body::from(file.data.into_owned()))
.unwrap()
.unwrap_or_else(|_| {
Response::builder()
.status(StatusCode::INTERNAL_SERVER_ERROR)
.body(Body::from("Internal server error"))
.unwrap()
})
}
None => Response::builder()
.status(StatusCode::NOT_FOUND)
@@ -655,37 +656,44 @@ mod tests {
}
}
#[tokio::test]
async fn test_healthz_returns_ok() {
let state = test_state();
fn empty_test_state() -> AppState {
let conn = Connection::open_in_memory().unwrap();
db::init_schema(&conn).unwrap();
AppState {
conn: Arc::new(Mutex::new(conn)),
config: Arc::new(Config {
allowed_origins: vec![],
..Default::default()
}),
}
}
async fn spawn_test_server(state: AppState) -> String {
let app = build_router(state);
let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap();
let addr = listener.local_addr().unwrap();
tokio::spawn(async move {
axum::serve(listener, app).await.unwrap();
});
format!("http://{addr}")
}
let resp = reqwest::get(format!("http://{addr}/healthz"))
.await
.unwrap();
#[tokio::test]
async fn test_healthz_returns_ok() {
let state = test_state();
let base = spawn_test_server(state).await;
let resp = reqwest::get(format!("{base}/healthz")).await.unwrap();
assert_eq!(resp.status(), StatusCode::OK);
}
#[tokio::test]
async fn test_get_species_returns_all() {
let state = test_state();
let app = build_router(state);
let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap();
let addr = listener.local_addr().unwrap();
let base = spawn_test_server(state).await;
tokio::spawn(async move {
axum::serve(listener, app).await.unwrap();
});
let resp = reqwest::get(format!("http://{addr}/api/species"))
.await
.unwrap();
let resp = reqwest::get(format!("{base}/api/species")).await.unwrap();
assert_eq!(resp.status(), StatusCode::OK);
let body: Vec<SpeciesDto> = resp.json().await.unwrap();
@@ -696,17 +704,9 @@ mod tests {
#[tokio::test]
async fn test_get_landings_no_filter() {
let state = test_state();
let app = build_router(state);
let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap();
let addr = listener.local_addr().unwrap();
let base = spawn_test_server(state).await;
tokio::spawn(async move {
axum::serve(listener, app).await.unwrap();
});
let resp = reqwest::get(format!("http://{addr}/api/landings"))
.await
.unwrap();
let resp = reqwest::get(format!("{base}/api/landings")).await.unwrap();
assert_eq!(resp.status(), StatusCode::OK);
let body: Vec<LandingDto> = resp.json().await.unwrap();
@@ -717,15 +717,9 @@ mod tests {
#[tokio::test]
async fn test_get_landings_filter_by_species() {
let state = test_state();
let app = build_router(state);
let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap();
let addr = listener.local_addr().unwrap();
let base = spawn_test_server(state).await;
tokio::spawn(async move {
axum::serve(listener, app).await.unwrap();
});
let resp = reqwest::get(format!("http://{addr}/api/landings?species=COD"))
let resp = reqwest::get(format!("{base}/api/landings?species=COD"))
.await
.unwrap();
assert_eq!(resp.status(), StatusCode::OK);
@@ -738,15 +732,9 @@ mod tests {
#[tokio::test]
async fn test_get_landings_filter_by_measure() {
let state = test_state();
let app = build_router(state);
let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap();
let addr = listener.local_addr().unwrap();
let base = spawn_test_server(state).await;
tokio::spawn(async move {
axum::serve(listener, app).await.unwrap();
});
let resp = reqwest::get(format!("http://{addr}/api/landings?measure=MASS"))
let resp = reqwest::get(format!("{base}/api/landings?measure=MASS"))
.await
.unwrap();
assert_eq!(resp.status(), StatusCode::OK);
@@ -759,17 +747,10 @@ mod tests {
#[tokio::test]
async fn test_get_landings_range_filter() {
let state = test_state();
let app = build_router(state);
let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap();
let addr = listener.local_addr().unwrap();
let base = spawn_test_server(state).await;
tokio::spawn(async move {
axum::serve(listener, app).await.unwrap();
});
// Test single month range (same from and to)
let resp = reqwest::get(format!(
"http://{addr}/api/landings?month_from=2024M01&month_to=2024M01"
"{base}/api/landings?month_from=2024M01&month_to=2024M01"
))
.await
.unwrap();
@@ -779,32 +760,23 @@ mod tests {
assert!(body.iter().all(|l| l.month == "2024M01"));
assert_eq!(body.len(), 4);
// Test multi-month range
let resp = reqwest::get(format!(
"http://{addr}/api/landings?month_from=2024M01&month_to=2024M02"
"{base}/api/landings?month_from=2024M01&month_to=2024M02"
))
.await
.unwrap();
assert_eq!(resp.status(), StatusCode::OK);
let body: Vec<LandingDto> = resp.json().await.unwrap();
assert!(body.len() > 0);
assert!(!body.is_empty());
}
#[tokio::test]
async fn test_get_summary_monthly_aggregates() {
let state = test_state();
let app = build_router(state);
let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap();
let addr = listener.local_addr().unwrap();
let base = spawn_test_server(state).await;
tokio::spawn(async move {
axum::serve(listener, app).await.unwrap();
});
let resp = reqwest::get(format!("http://{addr}/api/summary"))
.await
.unwrap();
let resp = reqwest::get(format!("{base}/api/summary")).await.unwrap();
assert_eq!(resp.status(), StatusCode::OK);
let body: SummaryDto = resp.json().await.unwrap();
@@ -823,17 +795,9 @@ mod tests {
#[tokio::test]
async fn test_get_summary_top_species() {
let state = test_state();
let app = build_router(state);
let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap();
let addr = listener.local_addr().unwrap();
let base = spawn_test_server(state).await;
tokio::spawn(async move {
axum::serve(listener, app).await.unwrap();
});
let resp = reqwest::get(format!("http://{addr}/api/summary"))
.await
.unwrap();
let resp = reqwest::get(format!("{base}/api/summary")).await.unwrap();
let body: SummaryDto = resp.json().await.unwrap();
@@ -845,17 +809,9 @@ mod tests {
#[tokio::test]
async fn test_get_summary_price_trend() {
let state = test_state();
let app = build_router(state);
let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap();
let addr = listener.local_addr().unwrap();
let base = spawn_test_server(state).await;
tokio::spawn(async move {
axum::serve(listener, app).await.unwrap();
});
let resp = reqwest::get(format!("http://{addr}/api/summary"))
.await
.unwrap();
let resp = reqwest::get(format!("{base}/api/summary")).await.unwrap();
let body: SummaryDto = resp.json().await.unwrap();
@@ -877,18 +833,41 @@ mod tests {
assert!((feb.price_per_kg.unwrap() - 5.0).abs() < f64::EPSILON);
}
#[tokio::test]
async fn test_get_summary_with_species_filter() {
let state = test_state();
let base = spawn_test_server(state).await;
let resp = reqwest::get(format!("{base}/api/summary?species=COD"))
.await
.unwrap();
assert_eq!(resp.status(), StatusCode::OK);
let body: SummaryDto = resp.json().await.unwrap();
assert_eq!(body.monthly.len(), 2);
let jan = body.monthly.iter().find(|m| m.month == "2024M01").unwrap();
assert!((jan.total_mass.unwrap() - 1000.0).abs() < f64::EPSILON);
assert!((jan.total_value.unwrap() - 5000.0).abs() < f64::EPSILON);
assert!(!body.top_species.is_empty());
for ts in &body.top_species {
assert_eq!(ts.species_code, "COD");
}
assert_eq!(body.price_trend.len(), 2);
for pt in &body.price_trend {
assert!(pt.price_per_kg.is_some());
}
}
#[tokio::test]
async fn test_get_landings_faroese_label_preserved() {
let state = test_state();
let app = build_router(state);
let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap();
let addr = listener.local_addr().unwrap();
let base = spawn_test_server(state).await;
tokio::spawn(async move {
axum::serve(listener, app).await.unwrap();
});
let resp = reqwest::get(format!("http://{addr}/api/landings?species=COD"))
let resp = reqwest::get(format!("{base}/api/landings?species=COD"))
.await
.unwrap();
@@ -901,15 +880,9 @@ mod tests {
#[tokio::test]
async fn test_get_landings_empty_result() {
let state = test_state();
let app = build_router(state);
let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap();
let addr = listener.local_addr().unwrap();
let base = spawn_test_server(state).await;
tokio::spawn(async move {
axum::serve(listener, app).await.unwrap();
});
let resp = reqwest::get(format!("http://{addr}/api/landings?species=NONEXISTENT"))
let resp = reqwest::get(format!("{base}/api/landings?species=NONEXISTENT"))
.await
.unwrap();
assert_eq!(resp.status(), StatusCode::OK);
@@ -921,15 +894,9 @@ mod tests {
#[tokio::test]
async fn test_get_landings_limit_applied() {
let state = test_state();
let app = build_router(state);
let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap();
let addr = listener.local_addr().unwrap();
let base = spawn_test_server(state).await;
tokio::spawn(async move {
axum::serve(listener, app).await.unwrap();
});
let resp = reqwest::get(format!("http://{addr}/api/landings?limit=2"))
let resp = reqwest::get(format!("{base}/api/landings?limit=2"))
.await
.unwrap();
assert_eq!(resp.status(), StatusCode::OK);
@@ -941,15 +908,9 @@ mod tests {
#[tokio::test]
async fn test_export_parquet() {
let state = test_state();
let app = build_router(state);
let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap();
let addr = listener.local_addr().unwrap();
let base = spawn_test_server(state).await;
tokio::spawn(async move {
axum::serve(listener, app).await.unwrap();
});
let resp = reqwest::get(format!("http://{addr}/api/export.parquet"))
let resp = reqwest::get(format!("{base}/api/export.parquet"))
.await
.unwrap();
assert_eq!(resp.status(), StatusCode::OK);
@@ -963,18 +924,27 @@ mod tests {
assert_eq!(&bytes[..4], b"PAR1");
}
#[tokio::test]
async fn test_export_parquet_empty_db() {
let state = empty_test_state();
let base = spawn_test_server(state).await;
let resp = reqwest::get(format!("{base}/api/export.parquet"))
.await
.unwrap();
assert_eq!(resp.status(), StatusCode::OK);
let bytes = resp.bytes().await.unwrap();
assert_eq!(&bytes[..4], b"PAR1");
assert!(bytes.len() > 4);
}
#[tokio::test]
async fn test_concurrent_export_requests() {
let state = test_state();
let app = build_router(state);
let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap();
let addr = listener.local_addr().unwrap();
let base = spawn_test_server(state).await;
tokio::spawn(async move {
axum::serve(listener, app).await.unwrap();
});
let url = format!("http://{addr}/api/export.parquet");
let url = format!("{base}/api/export.parquet");
let (resp1, resp2) = tokio::join!(reqwest::get(&url), reqwest::get(&url));
let resp1 = resp1.unwrap();
@@ -992,27 +962,57 @@ mod tests {
assert!(!bytes2.is_empty());
}
#[tokio::test]
async fn test_static_handler_falls_back_to_index_html() {
let state = test_state();
let base = spawn_test_server(state).await;
let resp = reqwest::get(format!("{base}/nonexistent-path-xyz"))
.await
.unwrap();
assert_eq!(resp.status(), StatusCode::OK);
let body = resp.text().await.unwrap();
assert!(body.contains("<html"));
assert!(body.contains("<title>Fisheries Dashboard"));
}
#[tokio::test]
async fn test_sql_injection_safe() {
let state = test_state();
let state_clone = state.clone();
let app = build_router(state.clone());
let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap();
let addr = listener.local_addr().unwrap();
tokio::spawn(async move {
axum::serve(listener, app).await.unwrap();
});
let base = spawn_test_server(state.clone()).await;
let malicious_params = "?species='; DROP TABLE landings; --";
let url = format!("http://{addr}/api/landings{malicious_params}");
let url = format!("{base}/api/landings{malicious_params}");
let resp = reqwest::get(&url).await.unwrap();
assert_eq!(resp.status(), StatusCode::OK);
let count: i64 = tokio::task::spawn_blocking(move || {
let conn = state_clone.conn.blocking_lock();
let conn = state.conn.blocking_lock();
conn.query_row("SELECT COUNT(*) FROM landings", [], |row| row.get(0))
.unwrap()
})
.await
.unwrap();
assert_eq!(count, 6);
}
#[tokio::test]
async fn test_sql_injection_all_params_safe() {
let state = test_state();
let base = spawn_test_server(state.clone()).await;
let malicious_params =
"?month_from=2024M01'; DROP TABLE landings;--&month_to=2024M12&species=COD";
let url = format!("{base}/api/landings{malicious_params}");
let resp = reqwest::get(&url).await.unwrap();
assert_eq!(resp.status(), StatusCode::OK);
let count: i64 = tokio::task::spawn_blocking(move || {
let conn = state.conn.blocking_lock();
conn.query_row("SELECT COUNT(*) FROM landings", [], |row| row.get(0))
.unwrap()
})