From 42265350c9f0b7a5731103c191a807a691f8f2ef Mon Sep 17 00:00:00 2001 From: Bryan Newbold Date: Fri, 21 Dec 2018 13:07:28 -0800 Subject: more state/edit edge case tests --- rust/src/api_entity_crud.rs | 42 +++++++++++++++++++++++++++++++++++++----- 1 file changed, 37 insertions(+), 5 deletions(-) (limited to 'rust/src/api_entity_crud.rs') diff --git a/rust/src/api_entity_crud.rs b/rust/src/api_entity_crud.rs index 3b12a446..0315f0a7 100644 --- a/rust/src/api_entity_crud.rs +++ b/rust/src/api_entity_crud.rs @@ -136,6 +136,10 @@ macro_rules! generic_db_create { // TODO: this path should call generic_db_create_batch ($ident_table: ident, $edit_table: ident) => { fn db_create(&self, conn: &DbConn, edit_context: &EditContext) -> Result { + if self.redirect.is_some() { + return Err(ErrorKind::OtherBadRequest( + "can't create an entity that redirects from the start".to_string()).into()); + } let rev_id = self.db_insert_rev(conn)?; let ident: Uuid = insert_into($ident_table::table) .values($ident_table::rev_id.eq(&rev_id)) @@ -160,6 +164,10 @@ macro_rules! generic_db_create_batch { edit_context: &EditContext, models: &[&Self], ) -> Result> { + if models.iter().any(|m| m.redirect.is_some()) { + return Err(ErrorKind::OtherBadRequest( + "can't create an entity that redirects from the start".to_string()).into()); + } let rev_ids: Vec = Self::db_insert_revs(conn, models)?; let ident_ids: Vec = insert_into($ident_table::table) .values( @@ -221,9 +229,10 @@ macro_rules! generic_db_update { } // TODO: if we get a diesel not-found here, should be a special error response? let target: Self::IdentRow = $ident_table::table.find(redirect_ident).first(conn)?; - if target.is_live == false { + if target.is_live != true { // there is no race condition on this check because WIP -> is_live=true is // a one-way operation + // XXX: return Err(ErrorKind::OtherBadRequest( "attempted to redirect to a WIP entity".to_string()).into()); } @@ -642,12 +651,17 @@ impl EntityCrud for ContainerEntity { } } + if models.iter().any(|m| m.name.is_none()) { + return Err(ErrorKind::OtherBadRequest( + "name is required for all Container entities".to_string()).into()); + } + let rev_ids: Vec = insert_into(container_rev::table) .values( models .iter() .map(|model| ContainerRevNewRow { - name: model.name.clone().unwrap(), // XXX: unwrap + name: model.name.clone().unwrap(), // unwrap checked above publisher: model.publisher.clone(), issnl: model.issnl.clone(), wikidata_qid: model.wikidata_qid.clone(), @@ -746,12 +760,17 @@ impl EntityCrud for CreatorEntity { } } + if models.iter().any(|m| m.display_name.is_none()) { + return Err(ErrorKind::OtherBadRequest( + "display_name is required for all Creator entities".to_string()).into()); + } + let rev_ids: Vec = insert_into(creator_rev::table) .values( models .iter() - .map(|model| CreatorRevNewRow { - display_name: model.display_name.clone().unwrap(), // XXX: unwrap + .map(|model|CreatorRevNewRow { + display_name: model.display_name.clone().unwrap(), // unwrapped checked above given_name: model.given_name.clone(), surname: model.surname.clone(), orcid: model.orcid.clone(), @@ -1010,6 +1029,10 @@ impl EntityCrud for ReleaseEntity { } fn db_create(&self, conn: &DbConn, edit_context: &EditContext) -> Result { + if self.redirect.is_some() { + return Err(ErrorKind::OtherBadRequest( + "can't create an entity that redirects from the start".to_string()).into()); + } let mut edits = Self::db_create_batch(conn, edit_context, &[self])?; // probably a more elegant way to destroy the vec and take first element Ok(edits.pop().unwrap()) @@ -1022,6 +1045,10 @@ impl EntityCrud for ReleaseEntity { ) -> Result> { // This isn't the generic implementation because we need to create Work entities for each // of the release entities passed (at least in the common case) + if models.iter().any(|m| m.redirect.is_some()) { + return Err(ErrorKind::OtherBadRequest( + "can't create an entity that redirects from the start".to_string()).into()); + } // Generate the set of new work entities to insert (usually one for each release, but some // releases might be pointed to a work already) @@ -1239,13 +1266,18 @@ impl EntityCrud for ReleaseEntity { } } + if models.iter().any(|m| m.title.is_none()) { + return Err(ErrorKind::OtherBadRequest( + "title is required for all Release entities".to_string()).into()); + } + let rev_ids: Vec = insert_into(release_rev::table) .values( models .iter() .map(|model| { Ok(ReleaseRevNewRow { - title: model.title.clone().unwrap(), // XXX: unwrap() + title: model.title.clone().unwrap(), // titles checked above release_type: model.release_type.clone(), release_status: model.release_status.clone(), release_date: model.release_date, -- cgit v1.2.3