Skip to content

Commit

Permalink
API: Add schlag DTO validation
Browse files Browse the repository at this point in the history
  • Loading branch information
Arthi-chaud committed Mar 17, 2024
1 parent 9fa7046 commit 5c16b3f
Show file tree
Hide file tree
Showing 8 changed files with 132 additions and 15 deletions.
2 changes: 1 addition & 1 deletion api/Dockerfile.dev
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,4 @@ RUN cargo install cargo-watch
ENV ROCKET_ADDRESS=0.0.0.0
EXPOSE 8000
# We reduce the pool_size because of this: https://github.com/rwf2/Rocket/issues/1931#issuecomment-955945953
CMD cargo build && DB_URL="postgresql://${POSTGRES_USER}:${POSTGRES_PASSWORD}@${POSTGRES_HOST}:${POSTGRES_PORT}/${POSTGRES_DB}" ROCKET_DATABASES="{sea_orm={url=$DB_URL, pool_size=2}}" cargo watch -x run
CMD cargo build && cd api && DB_URL="postgresql://${POSTGRES_USER}:${POSTGRES_PASSWORD}@${POSTGRES_HOST}:${POSTGRES_PORT}/${POSTGRES_DB}" ROCKET_DATABASES="{sea_orm={url=$DB_URL, pool_size=2}}" cargo watch -x run
6 changes: 6 additions & 0 deletions api/api/src/controllers/extras.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,12 @@ async fn new_extra(
db: Database<'_>,
data: Json<NewExtra>,
) -> ApiRawResult<status::Created<Json<ExtraCreationResponse>>> {
// TODO: This should be validated before the controller is called
if data.types.is_empty() {
return Err(ApiError::ValidationError(
"There should be at least one type.".to_owned(),
));
}
db.into_inner()
.transaction::<_, ExtraCreationResponse, DbErr>(|txn| {
Box::pin(async move {
Expand Down
9 changes: 9 additions & 0 deletions api/api/src/controllers/movies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,15 @@ async fn new_movie(
db: Database<'_>,
data: Json<NewMovie>,
) -> ApiRawResult<status::Created<Json<MovieCreationResponse>>> {
// TODO: This should be validated before the controller is called
for chapter in data.0.chapters.iter() {
if chapter.types.is_empty() {
return Err(ApiError::ValidationError(format!(
"Chapter '{}' should have at least one type",
chapter.name
)));
}
}
db.into_inner()
.transaction::<_, MovieCreationResponse, DbErr>(|txn| {
Box::pin(async move {
Expand Down
1 change: 1 addition & 0 deletions api/api/src/dto/chapter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,5 +88,6 @@ pub struct NewChapter {
pub start_timestamp: u64,
/// the end timespamp of the chapter, in seconds
pub end_timestamp: u64,
/// Must not be empty
pub types: Vec<ChapterType>,
}
2 changes: 1 addition & 1 deletion api/api/src/dto/extra.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ use rocket::serde::uuid::Uuid;
use rocket_okapi::okapi::schemars;
use rocket_okapi::okapi::schemars::JsonSchema;
use serde::{Deserialize, Serialize};

#[derive(Serialize, JsonSchema)]
pub struct ExtraResponseWithRelations {
#[serde(flatten)]
Expand Down Expand Up @@ -143,6 +142,7 @@ pub struct NewExtra {
pub disc_index: Option<i32>,
#[schemars(example = "example_index")]
pub track_index: Option<i32>,
/// Must Not Be Empty
pub types: Vec<ExtraType>,
pub file: NewFile,
}
36 changes: 27 additions & 9 deletions api/api/src/error_handling.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use rocket_okapi::okapi::Map;
use rocket_okapi::response::OpenApiResponderInner;
use rocket_okapi::OpenApiError;
use sea_orm::error::DbErr;
use sea_orm::RuntimeErr;
use sea_orm::SqlErr;
use sea_orm::TransactionError;
use serde::Serialize;
Expand All @@ -19,14 +20,15 @@ use serde::Serialize;
pub enum ApiError {
DatabaseError(DbErr),
SqlError(SqlErr),
ValidationError(String),
ImageProcessingError,
ImageServingError,
}

#[derive(Serialize)]
pub struct ErrorResponse<'s> {
pub struct ErrorResponse {
pub status_code: Status,
pub message: &'s str,
pub message: String,
}

pub type ApiResult<T> = Result<Json<T>, ApiError>;
Expand All @@ -36,27 +38,43 @@ pub type ApiRawResult<T> = Result<T, ApiError>;
impl<'r> Responder<'r, 'static> for ApiError {
fn respond_to(self, _: &'r Request<'_>) -> response::Result<'static> {
let response_body: ErrorResponse = match self {
ApiError::ValidationError(s) => ErrorResponse {
status_code: Status::BadRequest,
message: s,
},
ApiError::ImageProcessingError => ErrorResponse {
status_code: Status::BadRequest,
message: "Could not process received image.",
message: "Could not process received image.".to_owned(),
},
ApiError::ImageServingError => ErrorResponse {
status_code: Status::NotFound,
message: "Could not serve requested image.",
message: "Could not serve requested image.".to_owned(),
},
ApiError::DatabaseError(DbErr::RecordNotFound(_)) => ErrorResponse {
status_code: Status::NotFound,
message: "Resource not found.",
message: "Resource not found.".to_owned(),
},
ApiError::DatabaseError(DbErr::Query(RuntimeErr::SqlxError(
sqlx::error::Error::Database(pg_error),
))) => match pg_error.code().unwrap() {
std::borrow::Cow::Borrowed("23505") => ErrorResponse {
status_code: Status::Conflict,
message: "Resource already exists.".to_owned(),
},
_ => ErrorResponse {
status_code: Status::InternalServerError,
message: "Unhandled Database Error".to_owned(),
},
},
ApiError::SqlError(SqlErr::UniqueConstraintViolation(_)) => ErrorResponse {
status_code: Status::Conflict,
message: "Resource already exists.",
message: "Resource already exists.".to_owned(),
},
_ => {
// TODO: Log
ErrorResponse {
status_code: Status::InternalServerError,
message: "An unknown error occured",
message: "An unknown error occured".to_owned(),
}
}
};
Expand Down Expand Up @@ -123,9 +141,9 @@ impl OpenApiResponderInner for ApiError {
}

#[catch(404)]
pub fn not_found() -> Json<ErrorResponse<'static>> {
pub fn not_found() -> Json<ErrorResponse> {
Json(ErrorResponse {
status_code: Status::NotFound,
message: "Route not found.",
message: "Route not found.".to_owned(),
})
}
55 changes: 54 additions & 1 deletion api/api/tests/extras.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ mod test_extra {

#[test]
/// Test POST `/extras`
fn test_extras() {
fn test_new_extra() {
let client = test_client().lock().unwrap();
let dto = NewExtra {
artist_name: "Madonna".to_owned(),
Expand Down Expand Up @@ -93,4 +93,57 @@ mod test_extra {
let quality = file_value.get("quality").unwrap().as_str().unwrap();
assert_eq!(quality, "480p");
}

#[test]
fn test_new_extra_failure_already_exists() {
let client = test_client().lock().unwrap();
let dto = NewExtra {
artist_name: "Madonna".to_owned(),
extra_name: "Secret (Music Video)".to_owned(),
package_artist_name: Some("Madonna".to_owned()),
package_name: "The Video Collection 93:99".to_owned(),
disc_index: Some(1),
track_index: Some(8),
package_release_date: NaiveDate::from_ymd_opt(1999, 01, 02),
types: vec![ExtraType::MusicVideo],
file: NewFile {
path: "/data/Madonna/The Video Collection 93_99/1-08 Secret (Music Video).mp4"
.to_owned(),
size: 160000,
quality: VideoQuality::P480,
},
};
let response = client
.post("/extras")
.header(ContentType::JSON)
.body(serde_json::to_value(dto).unwrap().to_string())
.dispatch();
assert_eq!(response.status(), Status::Conflict);
}

#[test]
fn test_new_extra_failure_empty_type_list() {
let client = test_client().lock().unwrap();
let dto = NewExtra {
artist_name: "Madonna".to_owned(),
extra_name: "Secret (Music Video)".to_owned(),
package_artist_name: Some("Madonna".to_owned()),
package_name: "The Video Collection 93:99".to_owned(),
disc_index: Some(1),
track_index: Some(8),
package_release_date: NaiveDate::from_ymd_opt(1999, 01, 02),
types: vec![],
file: NewFile {
path: "Secret (Music Video).mp4".to_owned(),
size: 160000,
quality: VideoQuality::P480,
},
};
let response = client
.post("/extras")
.header(ContentType::JSON)
.body(serde_json::to_value(dto).unwrap().to_string())
.dispatch();
assert_eq!(response.status(), Status::BadRequest);
}
}
36 changes: 33 additions & 3 deletions api/api/tests/movies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ mod test_movie {

use crate::common::*;
use api::dto::{
chapter::NewChapter,
chapter::{ChapterType, NewChapter},
file::{NewFile, VideoQuality},
movie::{MovieType, NewMovie},
};
Expand Down Expand Up @@ -35,13 +35,13 @@ mod test_movie {
name: "Part 1".to_owned(),
start_timestamp: 60,
end_timestamp: 120,
types: vec![],
types: vec![ChapterType::Other],
},
NewChapter {
name: "Part 2".to_owned(),
start_timestamp: 120,
end_timestamp: 180,
types: vec![],
types: vec![ChapterType::Other],
},
],
};
Expand Down Expand Up @@ -119,4 +119,34 @@ mod test_movie {
let quality = file_value.get("quality").unwrap().as_str().unwrap();
assert_eq!(quality, "1080p");
}

#[test]
fn test_movies_failure_empty_chapter_type() {
let client = test_client().lock().unwrap();
let dto = NewMovie {
artist_name: "Taylor Swift".to_owned(),
movie_name: "Miss Americana".to_owned(),
package_artist_name: Some("Taylor Swift".to_owned()),
package_name: "Miss Americana".to_owned(),
package_release_date: NaiveDate::from_ymd_opt(2019, 02, 01),
movie_type: MovieType::Documentary,
file: NewFile {
path: "/data/Taylor Swift/Miss Americana.mp4".to_owned(),
size: 160000,
quality: VideoQuality::P1080,
},
chapters: vec![NewChapter {
name: "Part 1".to_owned(),
start_timestamp: 60,
end_timestamp: 120,
types: vec![],
}],
};
let response = client
.post("/movies")
.header(ContentType::JSON)
.body(serde_json::to_value(dto).unwrap().to_string())
.dispatch();
assert_eq!(response.status(), Status::BadRequest);
}
}

0 comments on commit 5c16b3f

Please sign in to comment.