From 28880401a4d64f9cc1317a3f3bf515166a812c1b Mon Sep 17 00:00:00 2001 From: Bryan Newbold Date: Fri, 14 Dec 2018 19:17:55 +0800 Subject: better return status for some error conditions --- rust/src/api_entity_crud.rs | 21 +++++++++------------ rust/src/api_wrappers.rs | 16 ++++++++++++++++ rust/src/lib.rs | 7 ++++++- 3 files changed, 31 insertions(+), 13 deletions(-) diff --git a/rust/src/api_entity_crud.rs b/rust/src/api_entity_crud.rs index 8770644c..ccbae26d 100644 --- a/rust/src/api_entity_crud.rs +++ b/rust/src/api_entity_crud.rs @@ -183,13 +183,10 @@ macro_rules! generic_db_update { ($ident_table: ident, $edit_table: ident) => { fn db_update(&self, conn: &DbConn, edit_context: &EditContext, ident: FatCatId) -> Result { let current: Self::IdentRow = $ident_table::table.find(ident.to_uuid()).first(conn)?; + // TODO: is this actually true? or should we allow updates in the same editgroup? if current.is_live != true { - // TODO: what if isn't live? 4xx not 5xx - bail!("can't delete an entity that doesn't exist yet"); - } - if current.rev_id.is_none() { - // TODO: what if it's already deleted? 4xx not 5xx - bail!("entity was already deleted"); + return Err(ErrorKind::InvalidEntityStateTransform( + "can't update an entity that doesn't exist yet".to_string()).into()); } let rev_id = self.db_insert_rev(conn)?; @@ -217,12 +214,12 @@ macro_rules! generic_db_delete { ) -> Result { let current: Self::IdentRow = $ident_table::table.find(ident.to_uuid()).first(conn)?; if current.is_live != true { - // TODO: what if isn't live? 4xx not 5xx - bail!("can't delete an entity that doesn't exist yet"); + return Err(ErrorKind::InvalidEntityStateTransform( + "can't update an entity that doesn't exist yet; delete edit object instead".to_string()).into()); } if current.rev_id.is_none() { - // TODO: what if it's already deleted? 4xx not 5xx - bail!("entity was already deleted"); + return Err(ErrorKind::InvalidEntityStateTransform( + "entity was already deleted".to_string()).into()); } let edit: Self::EditRow = insert_into($edit_table::table) .values(( @@ -292,8 +289,8 @@ macro_rules! generic_db_delete_edit { .limit(1) .get_results(conn)?; if accepted_rows.len() != 0 { - // TODO: should be a 4xx, not a 5xx - bail!("attempted to delete an already accepted edit") + return Err(ErrorKind::EditgroupAlreadyAccepted( + "attempted to delete an already accepted edit".to_string()).into()); } diesel::delete($edit_table::table.filter($edit_table::id.eq(edit_id))).execute(conn)?; Ok(()) diff --git a/rust/src/api_wrappers.rs b/rust/src/api_wrappers.rs index cb93ec54..a2357fe1 100644 --- a/rust/src/api_wrappers.rs +++ b/rust/src/api_wrappers.rs @@ -64,6 +64,8 @@ macro_rules! wrap_entity_handlers { message: ErrorKind::InvalidFatcatId(e).to_string() }), Err(Error(ErrorKind::MalformedExternalId(e), _)) => $get_resp::BadRequest(ErrorResponse { message: e.to_string() }), + Err(Error(ErrorKind::EditgroupAlreadyAccepted(e), _)) => + $get_resp::BadRequest(ErrorResponse { message: e.to_string() }), Err(e) => { error!("{}", e); $get_resp::GenericError(ErrorResponse { message: e.to_string() }) @@ -99,6 +101,8 @@ macro_rules! wrap_entity_handlers { $post_resp::BadRequest(ErrorResponse { message: e.to_string() }), Err(Error(ErrorKind::NotInControlledVocabulary(e), _)) => $post_resp::BadRequest(ErrorResponse { message: e.to_string() }), + Err(Error(ErrorKind::EditgroupAlreadyAccepted(e), _)) => + $post_resp::BadRequest(ErrorResponse { message: e.to_string() }), Err(e) => { error!("{}", e); $post_resp::GenericError(ErrorResponse { message: e.to_string() }) @@ -134,6 +138,8 @@ macro_rules! wrap_entity_handlers { $post_batch_resp::BadRequest(ErrorResponse { message: e.to_string() }), Err(Error(ErrorKind::NotInControlledVocabulary(e), _)) => $post_batch_resp::BadRequest(ErrorResponse { message: e.to_string() }), + Err(Error(ErrorKind::EditgroupAlreadyAccepted(e), _)) => + $post_batch_resp::BadRequest(ErrorResponse { message: e.to_string() }), Err(e) => { error!("{}", e); $post_batch_resp::GenericError(ErrorResponse { message: e.to_string() }) @@ -173,6 +179,10 @@ macro_rules! wrap_entity_handlers { $update_resp::BadRequest(ErrorResponse { message: e.to_string() }), Err(Error(ErrorKind::NotInControlledVocabulary(e), _)) => $update_resp::BadRequest(ErrorResponse { message: e.to_string() }), + Err(Error(ErrorKind::EditgroupAlreadyAccepted(e), _)) => + $update_resp::BadRequest(ErrorResponse { message: e.to_string() }), + Err(Error(ErrorKind::InvalidEntityStateTransform(e), _)) => + $update_resp::BadRequest(ErrorResponse { message: e.to_string() }), Err(e) => { error!("{}", e); $update_resp::GenericError(ErrorResponse { message: e.to_string() }) @@ -210,6 +220,10 @@ macro_rules! wrap_entity_handlers { message: ErrorKind::InvalidFatcatId(e).to_string() }), Err(Error(ErrorKind::MalformedExternalId(e), _)) => $delete_resp::BadRequest(ErrorResponse { message: e.to_string() }), + Err(Error(ErrorKind::EditgroupAlreadyAccepted(e), _)) => + $delete_resp::BadRequest(ErrorResponse { message: e.to_string() }), + Err(Error(ErrorKind::InvalidEntityStateTransform(e), _)) => + $delete_resp::BadRequest(ErrorResponse { message: e.to_string() }), Err(e) => { error!("{}", e); $delete_resp::GenericError(ErrorResponse { message: e.to_string() }) @@ -325,6 +339,8 @@ macro_rules! wrap_entity_handlers { $delete_edit_resp::NotFound(ErrorResponse { message: format!("No such {} edit: {}", stringify!($model), edit_id) }), Err(Error(ErrorKind::Diesel(e), _)) => $delete_edit_resp::BadRequest(ErrorResponse { message: e.to_string() }), + Err(Error(ErrorKind::EditgroupAlreadyAccepted(e), _)) => + $delete_edit_resp::BadRequest(ErrorResponse { message: e.to_string() }), Err(e) => { error!("{}", e); $delete_edit_resp::GenericError(ErrorResponse { message: e.to_string() }) diff --git a/rust/src/lib.rs b/rust/src/lib.rs index 6dbfc468..ab293fa9 100644 --- a/rust/src/lib.rs +++ b/rust/src/lib.rs @@ -1,4 +1,5 @@ #![allow(proc_macro_derive_resolution_fallback)] +#![recursion_limit="128"] extern crate chrono; extern crate fatcat_api_spec; @@ -56,12 +57,16 @@ pub mod errors { } EditgroupAlreadyAccepted(id: String) { description("editgroup was already accepted") - display("attempted to accept an editgroup which was already accepted: {}", id) + display("attempted to accept or mutate an editgroup which was already accepted: {}", id) } MissingOrMultipleExternalId(message: String) { description("external identifiers missing or multiple specified") display("external identifiers missing or multiple specified; please supply exactly one") } + InvalidEntityStateTransform(message: String) { + description("Invalid Entity State Transform") + display("tried to mutate an entity which was not in an appropriate state: {}", message) + } } } } -- cgit v1.2.3