diff options
| -rw-r--r-- | rust/src/api_entity_crud.rs | 245 | ||||
| -rw-r--r-- | rust/src/api_server.rs | 6 | ||||
| -rw-r--r-- | rust/src/api_wrappers.rs | 49 | ||||
| -rw-r--r-- | rust/src/database_models.rs | 4 | ||||
| -rw-r--r-- | rust/src/lib.rs | 6 |
5 files changed, 272 insertions, 38 deletions
diff --git a/rust/src/api_entity_crud.rs b/rust/src/api_entity_crud.rs index ccbae26d..605c27ed 100644 --- a/rust/src/api_entity_crud.rs +++ b/rust/src/api_entity_crud.rs @@ -44,6 +44,7 @@ where type RevRow; // Generic Methods + fn from_deleted_row(ident_row: Self::IdentRow) -> Result<Self>; fn db_get(conn: &DbConn, ident: FatCatId, hide: HideFlags) -> Result<Self>; fn db_get_rev(conn: &DbConn, rev_id: Uuid, hide: HideFlags) -> Result<Self>; fn db_expand(&mut self, conn: &DbConn, expand: ExpandFlags) -> Result<()>; @@ -88,12 +89,26 @@ where macro_rules! generic_db_get { ($ident_table:ident, $rev_table:ident) => { fn db_get(conn: &DbConn, ident: FatCatId, hide: HideFlags) -> Result<Self> { - let (ident, rev): (Self::IdentRow, Self::RevRow) = $ident_table::table + let res: Option<(Self::IdentRow, Self::RevRow)> = $ident_table::table .find(ident.to_uuid()) .inner_join($rev_table::table) - .first(conn)?; - - Self::db_from_row(conn, rev, Some(ident), hide) + .first(conn) + .optional()?; + + match res { + Some((ident, rev)) => { + Self::db_from_row(conn, rev, Some(ident), hide) + }, + None => { + // return a stub (deleted) entity if it's just deleted state + let ident_row: Self::IdentRow = $ident_table::table.find(ident.to_uuid()).first(conn)?; + if ident_row.rev_id.is_none() { + Self::from_deleted_row(ident_row) + } else { + bail!("unexpected condition: entity ident/rev join failed, yet row isn't in deleted state") + } + }, + } } }; } @@ -183,23 +198,77 @@ macro_rules! generic_db_update { ($ident_table: ident, $edit_table: ident) => { fn db_update(&self, conn: &DbConn, edit_context: &EditContext, ident: FatCatId) -> Result<Self::EditRow> { let current: Self::IdentRow = $ident_table::table.find(ident.to_uuid()).first(conn)?; + let no_redirect: Option<Uuid> = None; // TODO: is this actually true? or should we allow updates in the same editgroup? if current.is_live != true { return Err(ErrorKind::InvalidEntityStateTransform( "can't update an entity that doesn't exist yet".to_string()).into()); } + if self.state.is_none() { + // special case: redirect to another entity + if let Some(ref redirect_ident) = self.redirect { + let redirect_ident = FatCatId::from_str(&redirect_ident)?.to_uuid(); + if Some(redirect_ident) == current.redirect_id { + return Err(ErrorKind::OtherBadRequest( + "redundantly redirecting entity to it's current target currently isn't supported".to_string()).into()); + } + // 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 { + // there is no race condition on this check because WIP -> is_live=true is + // a one-way operation + return Err(ErrorKind::OtherBadRequest( + "attempted to redirect to a WIP entity".to_string()).into()); + } + // Note: there is a condition where the target is already a redirect, but we + // don't handle that here because the state of the redirect could change before + // we accept this editgroup + let edit: Self::EditRow = insert_into($edit_table::table) + .values(( + $edit_table::editgroup_id.eq(edit_context.editgroup_id.to_uuid()), + $edit_table::ident_id.eq(&ident.to_uuid()), + $edit_table::rev_id.eq(target.rev_id), + $edit_table::redirect_id.eq(redirect_ident), + $edit_table::prev_rev.eq(current.rev_id), + $edit_table::extra_json.eq(&self.edit_extra), + )) + .get_result(conn)?; + return Ok(edit) + } + // special case: revert to point to an existing revision + if let Some(ref rev_id) = self.revision { + let rev_id = Uuid::from_str(&rev_id)?; + if Some(rev_id) == current.rev_id { + return Err(ErrorKind::OtherBadRequest( + "reverted entity to it's current state; this isn't currently supported".to_string()).into()); + } + let edit: Self::EditRow = insert_into($edit_table::table) + .values(( + $edit_table::editgroup_id.eq(edit_context.editgroup_id.to_uuid()), + $edit_table::ident_id.eq(&ident.to_uuid()), + $edit_table::rev_id.eq(&rev_id), + $edit_table::redirect_id.eq(no_redirect), + $edit_table::prev_rev.eq(current.rev_id), + $edit_table::extra_json.eq(&self.edit_extra), + )) + .get_result(conn)?; + return Ok(edit) + } + } + + // regular insert/update let rev_id = self.db_insert_rev(conn)?; let edit: Self::EditRow = insert_into($edit_table::table) .values(( $edit_table::editgroup_id.eq(edit_context.editgroup_id.to_uuid()), $edit_table::ident_id.eq(&ident.to_uuid()), $edit_table::rev_id.eq(&rev_id), - $edit_table::prev_rev.eq(current.rev_id.unwrap()), - $edit_table::extra_json.eq(&self.extra), + $edit_table::redirect_id.eq(no_redirect), + $edit_table::prev_rev.eq(current.rev_id), + $edit_table::extra_json.eq(&self.edit_extra), )) .get_result(conn)?; - Ok(edit) } } @@ -215,11 +284,16 @@ macro_rules! generic_db_delete { let current: Self::IdentRow = $ident_table::table.find(ident.to_uuid()).first(conn)?; if current.is_live != true { return Err(ErrorKind::InvalidEntityStateTransform( - "can't update an entity that doesn't exist yet; delete edit object instead".to_string()).into()); + "can't update an entity that doesn't exist yet; delete edit object instead" + .to_string(), + ) + .into()); } if current.rev_id.is_none() { return Err(ErrorKind::InvalidEntityStateTransform( - "entity was already deleted".to_string()).into()); + "entity was already deleted".to_string(), + ) + .into()); } let edit: Self::EditRow = insert_into($edit_table::table) .values(( @@ -290,7 +364,9 @@ macro_rules! generic_db_delete_edit { .get_results(conn)?; if accepted_rows.len() != 0 { return Err(ErrorKind::EditgroupAlreadyAccepted( - "attempted to delete an already accepted edit".to_string()).into()); + "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(()) @@ -366,6 +442,29 @@ macro_rules! generic_db_accept_edits_batch { )) .bind::<diesel::sql_types::Uuid, _>(editgroup_id.to_uuid()) .execute(conn)?; + // NOTE: all the following can be skipped for accepts that are all inserts (which I + // guess we only know for batch inserts with auto-accept?) + + // update any/all redirects for updated entities + let _redir_count = diesel::sql_query(format!( + " + UPDATE {entity}_ident + SET + rev_id = {entity}_edit.rev_id + FROM {entity}_edit + WHERE + {entity}_ident.redirect_id = {entity}_edit.ident_id + AND {entity}_edit.editgroup_id = $1", + entity = $entity_name_str + )) + .bind::<diesel::sql_types::Uuid, _>(editgroup_id.to_uuid()) + .execute(conn)?; + // TODO: backwards multiple redirect: if we redirected any entities to targets in this + // edit, then we need to update any redirects to *current* entities to *target* + // entities + // TODO: forward multiple redirect: if we redirected any entities to targets in this + // edit, and those targets are themselves redirects, we should point to the "final" + // targets Ok(count as u64) } }; @@ -439,6 +538,29 @@ impl EntityCrud for ContainerEntity { generic_db_accept_edits_batch!("container"); generic_db_insert_rev!(); + fn from_deleted_row(ident_row: Self::IdentRow) -> Result<Self> { + if ident_row.rev_id.is_some() { + bail!("called from_deleted_row with a non-deleted-state row") + } + + Ok(ContainerEntity { + issnl: None, + wikidata_qid: None, + publisher: None, + name: None, + abbrev: None, + coden: None, + state: Some(ident_row.state().unwrap().shortname()), + ident: Some(FatCatId::from_uuid(&ident_row.id).to_string()), + revision: ident_row.rev_id.map(|u| u.to_string()), + redirect: ident_row + .redirect_id + .map(|u| FatCatId::from_uuid(&u).to_string()), + extra: None, + edit_extra: None, + }) + } + fn db_from_row( _conn: &DbConn, rev_row: Self::RevRow, @@ -523,6 +645,28 @@ impl EntityCrud for CreatorEntity { generic_db_accept_edits_batch!("creator"); generic_db_insert_rev!(); + fn from_deleted_row(ident_row: Self::IdentRow) -> Result<Self> { + if ident_row.rev_id.is_some() { + bail!("called from_deleted_row with a non-deleted-state row") + } + + Ok(CreatorEntity { + extra: None, + edit_extra: None, + display_name: None, + given_name: None, + surname: None, + orcid: None, + wikidata_qid: None, + state: Some(ident_row.state().unwrap().shortname()), + ident: Some(FatCatId::from_uuid(&ident_row.id).to_string()), + revision: ident_row.rev_id.map(|u| u.to_string()), + redirect: ident_row + .redirect_id + .map(|u| FatCatId::from_uuid(&u).to_string()), + }) + } + fn db_from_row( _conn: &DbConn, rev_row: Self::RevRow, @@ -604,6 +748,30 @@ impl EntityCrud for FileEntity { generic_db_accept_edits_batch!("file"); generic_db_insert_rev!(); + fn from_deleted_row(ident_row: Self::IdentRow) -> Result<Self> { + if ident_row.rev_id.is_some() { + bail!("called from_deleted_row with a non-deleted-state row") + } + + Ok(FileEntity { + sha1: None, + sha256: None, + md5: None, + size: None, + urls: None, + mimetype: None, + releases: None, + state: Some(ident_row.state().unwrap().shortname()), + ident: Some(FatCatId::from_uuid(&ident_row.id).to_string()), + revision: ident_row.rev_id.map(|u| u.to_string()), + redirect: ident_row + .redirect_id + .map(|u| FatCatId::from_uuid(&u).to_string()), + extra: None, + edit_extra: None, + }) + } + fn db_from_row( conn: &DbConn, rev_row: Self::RevRow, @@ -742,6 +910,46 @@ impl EntityCrud for ReleaseEntity { generic_db_accept_edits_batch!("release"); generic_db_insert_rev!(); + fn from_deleted_row(ident_row: Self::IdentRow) -> Result<Self> { + if ident_row.rev_id.is_some() { + bail!("called from_deleted_row with a non-deleted-state row") + } + + Ok(ReleaseEntity { + title: None, + release_type: None, + release_status: None, + release_date: None, + doi: None, + pmid: None, + pmcid: None, + isbn13: None, + core_id: None, + wikidata_qid: None, + volume: None, + issue: None, + pages: None, + files: None, + container: None, + container_id: None, + publisher: None, + language: None, + work_id: None, + refs: None, + contribs: None, + abstracts: None, + + state: Some(ident_row.state().unwrap().shortname()), + ident: Some(FatCatId::from_uuid(&ident_row.id).to_string()), + revision: ident_row.rev_id.map(|u| u.to_string()), + redirect: ident_row + .redirect_id + .map(|u| FatCatId::from_uuid(&u).to_string()), + extra: None, + edit_extra: None, + }) + } + fn db_expand(&mut self, conn: &DbConn, expand: ExpandFlags) -> Result<()> { if expand.files { let ident = match &self.ident { @@ -1166,6 +1374,23 @@ impl EntityCrud for WorkEntity { generic_db_accept_edits_batch!("work"); generic_db_insert_rev!(); + fn from_deleted_row(ident_row: Self::IdentRow) -> Result<Self> { + if ident_row.rev_id.is_some() { + bail!("called from_deleted_row with a non-deleted-state row") + } + + Ok(WorkEntity { + state: Some(ident_row.state().unwrap().shortname()), + ident: Some(FatCatId::from_uuid(&ident_row.id).to_string()), + revision: ident_row.rev_id.map(|u| u.to_string()), + redirect: ident_row + .redirect_id + .map(|u| FatCatId::from_uuid(&u).to_string()), + extra: None, + edit_extra: None, + }) + } + fn db_from_row( _conn: &DbConn, rev_row: Self::RevRow, diff --git a/rust/src/api_server.rs b/rust/src/api_server.rs index 2278f64f..0961194b 100644 --- a/rust/src/api_server.rs +++ b/rust/src/api_server.rs @@ -53,7 +53,11 @@ pub struct Server { pub db_pool: ConnectionPool, } -pub fn get_release_files(id: FatCatId, hide_flags: HideFlags, conn: &DbConn) -> Result<Vec<FileEntity>> { +pub fn get_release_files( + id: FatCatId, + hide_flags: HideFlags, + conn: &DbConn, +) -> Result<Vec<FileEntity>> { let rows: Vec<(FileRevRow, FileIdentRow, FileReleaseRow)> = file_rev::table .inner_join(file_ident::table) .inner_join(file_release::table) diff --git a/rust/src/api_wrappers.rs b/rust/src/api_wrappers.rs index a2357fe1..c8f6b390 100644 --- a/rust/src/api_wrappers.rs +++ b/rust/src/api_wrappers.rs @@ -664,30 +664,31 @@ impl Api for Server { Some(param) => HideFlags::from_str(¶m).unwrap(), }; // No transaction for GET - let ret = match self.lookup_file_handler(&md5, &sha1, &sha256, expand_flags, hide_flags, &conn) { - Ok(entity) => LookupFileResponse::FoundEntity(entity), - Err(Error(ErrorKind::Diesel(::diesel::result::Error::NotFound), _)) => { - LookupFileResponse::NotFound(ErrorResponse { - message: format!("Not found: {:?} / {:?} / {:?}", md5, sha1, sha256), - }) - } - Err(Error(ErrorKind::MalformedExternalId(e), _)) => { - LookupFileResponse::BadRequest(ErrorResponse { - message: e.to_string(), - }) - } - Err(Error(ErrorKind::MissingOrMultipleExternalId(e), _)) => { - LookupFileResponse::BadRequest(ErrorResponse { - message: e.to_string(), - }) - } - Err(e) => { - error!("{}", e); - LookupFileResponse::BadRequest(ErrorResponse { - message: e.to_string(), - }) - } - }; + let ret = + match self.lookup_file_handler(&md5, &sha1, &sha256, expand_flags, hide_flags, &conn) { + Ok(entity) => LookupFileResponse::FoundEntity(entity), + Err(Error(ErrorKind::Diesel(::diesel::result::Error::NotFound), _)) => { + LookupFileResponse::NotFound(ErrorResponse { + message: format!("Not found: {:?} / {:?} / {:?}", md5, sha1, sha256), + }) + } + Err(Error(ErrorKind::MalformedExternalId(e), _)) => { + LookupFileResponse::BadRequest(ErrorResponse { + message: e.to_string(), + }) + } + Err(Error(ErrorKind::MissingOrMultipleExternalId(e), _)) => { + LookupFileResponse::BadRequest(ErrorResponse { + message: e.to_string(), + }) + } + Err(e) => { + error!("{}", e); + LookupFileResponse::BadRequest(ErrorResponse { + message: e.to_string(), + }) + } + }; Box::new(futures::done(Ok(ret))) } diff --git a/rust/src/database_models.rs b/rust/src/database_models.rs index a7076e09..82088f28 100644 --- a/rust/src/database_models.rs +++ b/rust/src/database_models.rs @@ -14,7 +14,7 @@ use uuid::Uuid; pub enum EntityState { WorkInProgress, Active(Uuid), - Redirect(Uuid, Uuid), + Redirect(Uuid, Option<Uuid>), Deleted, } @@ -111,7 +111,7 @@ macro_rules! entity_structs { } match (self.redirect_id, self.rev_id) { (None, None) => Ok(EntityState::Deleted), - (Some(redir), Some(rev)) => Ok(EntityState::Redirect(redir, rev)), + (Some(redir), rev) => Ok(EntityState::Redirect(redir, rev)), (None, Some(rev)) => Ok(EntityState::Active(rev)), _ => bail!("Invalid EntityIdentRow state"), } diff --git a/rust/src/lib.rs b/rust/src/lib.rs index ab293fa9..b7f1817a 100644 --- a/rust/src/lib.rs +++ b/rust/src/lib.rs @@ -1,5 +1,5 @@ #![allow(proc_macro_derive_resolution_fallback)] -#![recursion_limit="128"] +#![recursion_limit = "128"] extern crate chrono; extern crate fatcat_api_spec; @@ -67,6 +67,10 @@ pub mod errors { description("Invalid Entity State Transform") display("tried to mutate an entity which was not in an appropriate state: {}", message) } + OtherBadRequest(message: String) { + description("catch-all error for bad or unallowed requests") + display("broke a constraint or made an otherwise invalid request: {}", message) + } } } } |