From da113570daf4b2d17cd828e3b512774485870ff0 Mon Sep 17 00:00:00 2001 From: Bryan Newbold Date: Mon, 26 Nov 2018 23:43:56 -0800 Subject: implement hide flag --- rust/src/api_entity_crud.rs | 139 ++++++++++++++++++++---------------- rust/src/api_helpers.rs | 93 ++++++++++++++++++------ rust/src/api_server.rs | 40 +++++++---- rust/src/api_wrappers.rs | 58 +++++++++++++-- rust/src/bin/fatcat-export.rs | 2 +- rust/tests/test_api_server_http.rs | 16 ++++- rust/tests/test_old_python_tests.rs | 4 +- 7 files changed, 242 insertions(+), 110 deletions(-) (limited to 'rust') diff --git a/rust/src/api_entity_crud.rs b/rust/src/api_entity_crud.rs index 80cae471..3a2760c2 100644 --- a/rust/src/api_entity_crud.rs +++ b/rust/src/api_entity_crud.rs @@ -41,8 +41,8 @@ where type RevRow; // Generic Methods - fn db_get(conn: &DbConn, ident: FatCatId) -> Result; - fn db_get_rev(conn: &DbConn, rev_id: Uuid) -> Result; + fn db_get(conn: &DbConn, ident: FatCatId, hide: HideFlags) -> Result; + fn db_get_rev(conn: &DbConn, rev_id: Uuid, hide: HideFlags) -> Result; fn db_expand(&mut self, conn: &DbConn, expand: ExpandFlags) -> Result<()>; fn db_create(&self, conn: &DbConn, edit_context: &EditContext) -> Result; fn db_create_batch( @@ -73,6 +73,7 @@ where conn: &DbConn, rev_row: Self::RevRow, ident_row: Option, + hide: HideFlags, ) -> Result; fn db_insert_rev(&self, conn: &DbConn) -> Result; fn db_insert_revs(conn: &DbConn, models: &[&Self]) -> Result>; @@ -80,23 +81,23 @@ where macro_rules! generic_db_get { ($ident_table:ident, $rev_table:ident) => { - fn db_get(conn: &DbConn, ident: FatCatId) -> Result { + fn db_get(conn: &DbConn, ident: FatCatId, hide: HideFlags) -> Result { let (ident, rev): (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)) + Self::db_from_row(conn, rev, Some(ident), hide) } }; } macro_rules! generic_db_get_rev { ($rev_table:ident) => { - fn db_get_rev(conn: &DbConn, rev_id: Uuid) -> Result { + fn db_get_rev(conn: &DbConn, rev_id: Uuid, hide: HideFlags) -> Result { let rev = $rev_table::table.find(rev_id).first(conn)?; - Self::db_from_row(conn, rev, None) + Self::db_from_row(conn, rev, None, hide) } }; } @@ -388,6 +389,7 @@ impl EntityCrud for ContainerEntity { _conn: &DbConn, rev_row: Self::RevRow, ident_row: Option, + _hide: HideFlags, ) -> Result { let (state, ident_id, redirect_id) = match ident_row { Some(i) => ( @@ -466,6 +468,7 @@ impl EntityCrud for CreatorEntity { _conn: &DbConn, rev_row: Self::RevRow, ident_row: Option, + _hide: HideFlags, ) -> Result { let (state, ident_id, redirect_id) = match ident_row { Some(i) => ( @@ -541,6 +544,7 @@ impl EntityCrud for FileEntity { conn: &DbConn, rev_row: Self::RevRow, ident_row: Option, + _hide: HideFlags, ) -> Result { let (state, ident_id, redirect_id) = match ident_row { Some(i) => ( @@ -660,8 +664,6 @@ impl EntityCrud for ReleaseEntity { generic_db_get!(release_ident, release_rev); generic_db_get_rev!(release_rev); - //generic_db_create!(release_ident, release_edit); - //generic_db_create_batch!(release_ident, release_edit); generic_db_update!(release_ident, release_edit); generic_db_delete!(release_ident, release_edit); generic_db_get_history!(release_edit); @@ -674,11 +676,11 @@ impl EntityCrud for ReleaseEntity { None => bail!("Can't expand files on a non-concrete entity"), Some(s) => FatCatId::from_str(&s)?, }; - self.files = Some(get_release_files(ident, conn)?); + self.files = Some(get_release_files(ident, HideFlags::none(), conn)?); } if expand.container { if let Some(ref cid) = self.container_id { - self.container = Some(ContainerEntity::db_get(conn, FatCatId::from_str(&cid)?)?); + self.container = Some(ContainerEntity::db_get(conn, FatCatId::from_str(&cid)?, HideFlags::none())?); } } Ok(()) @@ -768,6 +770,7 @@ impl EntityCrud for ReleaseEntity { conn: &DbConn, rev_row: Self::RevRow, ident_row: Option, + hide: HideFlags, ) -> Result { let (state, ident_id, redirect_id) = match ident_row { Some(i) => ( @@ -778,55 +781,70 @@ impl EntityCrud for ReleaseEntity { None => (None, None, None), }; - let refs: Vec = release_ref::table - .filter(release_ref::release_rev.eq(rev_row.id)) - .order(release_ref::index_val.asc()) - .get_results(conn)? - .into_iter() - .map(|r: ReleaseRefRow| ReleaseRef { - index: r.index_val.map(|v| v as i64), - key: r.key, - extra: r.extra_json, - container_name: r.container_name, - year: r.year.map(|v| v as i64), - title: r.title, - locator: r.locator, - target_release_id: r - .target_release_ident_id - .map(|v| FatCatId::from_uuid(&v).to_string()), - }).collect(); + let refs: Option> = match hide.refs { + true => None, + false => Some( + release_ref::table + .filter(release_ref::release_rev.eq(rev_row.id)) + .order(release_ref::index_val.asc()) + .get_results(conn)? + .into_iter() + .map(|r: ReleaseRefRow| ReleaseRef { + index: r.index_val.map(|v| v as i64), + key: r.key, + extra: r.extra_json, + container_name: r.container_name, + year: r.year.map(|v| v as i64), + title: r.title, + locator: r.locator, + target_release_id: r + .target_release_ident_id + .map(|v| FatCatId::from_uuid(&v).to_string()), + }).collect(), + ), + }; - let contribs: Vec = release_contrib::table - .filter(release_contrib::release_rev.eq(rev_row.id)) - .order(( - release_contrib::role.asc(), - release_contrib::index_val.asc(), - )).get_results(conn)? - .into_iter() - .map(|c: ReleaseContribRow| ReleaseContrib { - index: c.index_val.map(|v| v as i64), - raw_name: c.raw_name, - role: c.role, - extra: c.extra_json, - creator_id: c - .creator_ident_id - .map(|v| FatCatId::from_uuid(&v).to_string()), - creator: None, - }).collect(); + let contribs: Option> = match hide.contribs { + true => None, + false => Some( + release_contrib::table + .filter(release_contrib::release_rev.eq(rev_row.id)) + .order(( + release_contrib::role.asc(), + release_contrib::index_val.asc(), + )).get_results(conn)? + .into_iter() + .map(|c: ReleaseContribRow| ReleaseContrib { + index: c.index_val.map(|v| v as i64), + raw_name: c.raw_name, + role: c.role, + extra: c.extra_json, + creator_id: c + .creator_ident_id + .map(|v| FatCatId::from_uuid(&v).to_string()), + creator: None, + }).collect(), + ), + }; - let abstracts: Vec = release_rev_abstract::table - .inner_join(abstracts::table) - .filter(release_rev_abstract::release_rev.eq(rev_row.id)) - .get_results(conn)? - .into_iter() - .map( - |r: (ReleaseRevAbstractRow, AbstractsRow)| ReleaseEntityAbstracts { - sha1: Some(r.0.abstract_sha1), - mimetype: r.0.mimetype, - lang: r.0.lang, - content: Some(r.1.content), - }, - ).collect(); + let abstracts: Option> = match hide.abstracts { + true => None, + false => Some( + release_rev_abstract::table + .inner_join(abstracts::table) + .filter(release_rev_abstract::release_rev.eq(rev_row.id)) + .get_results(conn)? + .into_iter() + .map( + |r: (ReleaseRevAbstractRow, AbstractsRow)| ReleaseEntityAbstracts { + sha1: Some(r.0.abstract_sha1), + mimetype: r.0.mimetype, + lang: r.0.lang, + content: Some(r.1.content), + }, + ).collect(), + ), + }; Ok(ReleaseEntity { title: rev_row.title, @@ -850,9 +868,9 @@ impl EntityCrud for ReleaseEntity { publisher: rev_row.publisher, language: rev_row.language, work_id: Some(FatCatId::from_uuid(&rev_row.work_ident_id).to_string()), - refs: Some(refs), - contribs: Some(contribs), - abstracts: Some(abstracts), + refs: refs, + contribs: contribs, + abstracts: abstracts, state: state, ident: ident_id, revision: Some(rev_row.id.to_string()), @@ -1060,6 +1078,7 @@ impl EntityCrud for WorkEntity { _conn: &DbConn, rev_row: Self::RevRow, ident_row: Option, + _hide: HideFlags, ) -> Result { let (state, ident_id, redirect_id) = match ident_row { Some(i) => ( diff --git a/rust/src/api_helpers.rs b/rust/src/api_helpers.rs index 4b3f573b..952cb9cd 100644 --- a/rust/src/api_helpers.rs +++ b/rust/src/api_helpers.rs @@ -39,25 +39,11 @@ impl FromStr for ExpandFlags { impl ExpandFlags { pub fn from_str_list(list: &[&str]) -> ExpandFlags { - if list.contains(&"none") { - ExpandFlags::none() - } else if list.contains(&"all") { - ExpandFlags::all() - } else { - ExpandFlags { - files: list.contains(&"files"), - container: list.contains(&"container"), - releases: list.contains(&"releases"), - creators: list.contains(&"creators"), - } - } - } - pub fn all() -> ExpandFlags { ExpandFlags { - files: true, - container: true, - releases: true, - creators: true, + files: list.contains(&"files"), + container: list.contains(&"container"), + releases: list.contains(&"releases"), + creators: list.contains(&"creators"), } } pub fn none() -> ExpandFlags { @@ -103,7 +89,70 @@ fn test_expand_flags() { creators: true } ); - assert!(all == ExpandFlags::all()); +} + +#[derive(Clone, Copy, PartialEq)] +pub struct HideFlags { + pub abstracts: bool, + pub refs: bool, + pub contribs: bool, +} + +impl FromStr for HideFlags { + type Err = Error; + fn from_str(param: &str) -> Result { + let list: Vec<&str> = param.split_terminator(",").collect(); + Ok(HideFlags::from_str_list(&list)) + } +} + +impl HideFlags { + pub fn from_str_list(list: &[&str]) -> HideFlags { + HideFlags { + abstracts: list.contains(&"abstracts"), + refs: list.contains(&"refs"), + contribs: list.contains(&"contribs"), + } + } + pub fn none() -> HideFlags { + HideFlags { + abstracts: false, + refs: false, + contribs: false, + } + } +} + +#[test] +fn test_hide_flags() { + assert!(HideFlags::from_str_list(&vec![]).abstracts == false); + assert!(HideFlags::from_str_list(&vec!["abstracts"]).abstracts == true); + assert!(HideFlags::from_str_list(&vec!["abstract"]).abstracts == false); + let all = HideFlags::from_str_list(&vec!["abstracts", "refs", "other_thing", "contribs"]); + assert!( + all == HideFlags { + abstracts: true, + refs: true, + contribs: true, + } + ); + assert!(HideFlags::from_str("").unwrap().abstracts == false); + assert!(HideFlags::from_str("abstracts").unwrap().abstracts == true); + assert!( + HideFlags::from_str("something,,abstracts") + .unwrap() + .abstracts + == true + ); + assert!(HideFlags::from_str("file").unwrap().abstracts == false); + let all = HideFlags::from_str("abstracts,refs,other_thing,contribs").unwrap(); + assert!( + all == HideFlags { + abstracts: true, + refs: true, + contribs: true, + } + ); } pub fn make_edit_context( @@ -320,7 +369,6 @@ fn test_check_orcid() { } pub fn check_release_type(raw: &str) -> Result<()> { - let valid_types = vec![ // Citation Style Language official types "article", @@ -365,7 +413,7 @@ pub fn check_release_type(raw: &str) -> Result<()> { ]; for good in valid_types { if raw == good { - return Ok(()) + return Ok(()); } } Err(ErrorKind::NotInControlledVocabulary(format!( @@ -385,7 +433,6 @@ fn test_check_release_type() { } pub fn check_contrib_role(raw: &str) -> Result<()> { - let valid_types = vec![ // Citation Style Language official role types "author", @@ -407,7 +454,7 @@ pub fn check_contrib_role(raw: &str) -> Result<()> { ]; for good in valid_types { if raw == good { - return Ok(()) + return Ok(()); } } Err(ErrorKind::NotInControlledVocabulary(format!( diff --git a/rust/src/api_server.rs b/rust/src/api_server.rs index 9562a0f2..2bcd4a7f 100644 --- a/rust/src/api_server.rs +++ b/rust/src/api_server.rs @@ -53,7 +53,7 @@ pub struct Server { pub db_pool: ConnectionPool, } -pub fn get_release_files(id: FatCatId, conn: &DbConn) -> Result> { +pub fn get_release_files(id: FatCatId, hide: HideFlags, conn: &DbConn) -> Result> { let rows: Vec<(FileRevRow, FileIdentRow, FileReleaseRow)> = file_rev::table .inner_join(file_ident::table) .inner_join(file_release::table) @@ -63,12 +63,13 @@ pub fn get_release_files(id: FatCatId, conn: &DbConn) -> Result> .load(conn)?; rows.into_iter() - .map(|(rev, ident, _)| FileEntity::db_from_row(conn, rev, Some(ident))) - .collect() + .map( + |(rev, ident, _)| FileEntity::db_from_row(conn, rev, Some(ident), hide), + ).collect() } impl Server { - pub fn lookup_container_handler(&self, issnl: &str, conn: &DbConn) -> Result { + pub fn lookup_container_handler(&self, issnl: &str, hide: HideFlags, conn: &DbConn) -> Result { check_issn(issnl)?; let (ident, rev): (ContainerIdentRow, ContainerRevRow) = container_ident::table .inner_join(container_rev::table) @@ -77,10 +78,10 @@ impl Server { .filter(container_ident::redirect_id.is_null()) .first(conn)?; - ContainerEntity::db_from_row(conn, rev, Some(ident)) + ContainerEntity::db_from_row(conn, rev, Some(ident), hide) } - pub fn lookup_creator_handler(&self, orcid: &str, conn: &DbConn) -> Result { + pub fn lookup_creator_handler(&self, orcid: &str, hide: HideFlags, conn: &DbConn) -> Result { check_orcid(orcid)?; let (ident, rev): (CreatorIdentRow, CreatorRevRow) = creator_ident::table .inner_join(creator_rev::table) @@ -89,12 +90,13 @@ impl Server { .filter(creator_ident::redirect_id.is_null()) .first(conn)?; - CreatorEntity::db_from_row(conn, rev, Some(ident)) + CreatorEntity::db_from_row(conn, rev, Some(ident), hide) } pub fn get_creator_releases_handler( &self, id: FatCatId, + hide: HideFlags, conn: &DbConn, ) -> Result> { // TODO: some kind of unique or group-by? @@ -108,11 +110,11 @@ impl Server { // TODO: from_rows, not from_row? rows.into_iter() - .map(|(rev, ident, _)| ReleaseEntity::db_from_row(conn, rev, Some(ident))) + .map(|(rev, ident, _)| ReleaseEntity::db_from_row(conn, rev, Some(ident), hide)) .collect() } - pub fn lookup_file_handler(&self, sha1: &str, conn: &DbConn) -> Result { + pub fn lookup_file_handler(&self, sha1: &str, hide: HideFlags, conn: &DbConn) -> Result { let (ident, rev): (FileIdentRow, FileRevRow) = file_ident::table .inner_join(file_rev::table) .filter(file_rev::sha1.eq(sha1)) @@ -120,10 +122,15 @@ impl Server { .filter(file_ident::redirect_id.is_null()) .first(conn)?; - FileEntity::db_from_row(conn, rev, Some(ident)) + FileEntity::db_from_row(conn, rev, Some(ident), hide) } - pub fn lookup_release_handler(&self, doi: &str, conn: &DbConn) -> Result { + pub fn lookup_release_handler( + &self, + doi: &str, + hide: HideFlags, + conn: &DbConn, + ) -> Result { check_doi(doi)?; let (ident, rev): (ReleaseIdentRow, ReleaseRevRow) = release_ident::table .inner_join(release_rev::table) @@ -132,20 +139,22 @@ impl Server { .filter(release_ident::redirect_id.is_null()) .first(conn)?; - ReleaseEntity::db_from_row(conn, rev, Some(ident)) + ReleaseEntity::db_from_row(conn, rev, Some(ident), hide) } pub fn get_release_files_handler( &self, id: FatCatId, + hide: HideFlags, conn: &DbConn, ) -> Result> { - get_release_files(id, conn) + get_release_files(id, hide, conn) } pub fn get_work_releases_handler( &self, id: FatCatId, + hide: HideFlags, conn: &DbConn, ) -> Result> { let rows: Vec<(ReleaseRevRow, ReleaseIdentRow)> = release_rev::table @@ -156,8 +165,9 @@ impl Server { .load(conn)?; rows.into_iter() - .map(|(rev, ident)| ReleaseEntity::db_from_row(conn, rev, Some(ident))) - .collect() + .map(|(rev, ident)| { + ReleaseEntity::db_from_row(conn, rev, Some(ident), hide) + }).collect() } pub fn accept_editgroup_handler(&self, id: FatCatId, conn: &DbConn) -> Result<()> { diff --git a/rust/src/api_wrappers.rs b/rust/src/api_wrappers.rs index 098ea0b9..e5176ac6 100644 --- a/rust/src/api_wrappers.rs +++ b/rust/src/api_wrappers.rs @@ -30,17 +30,22 @@ macro_rules! wrap_entity_handlers { &self, id: String, expand: Option, + hide: Option, _context: &Context, ) -> Box + Send> { let conn = self.db_pool.get().expect("db_pool error"); // No transaction for GET let ret = match conn.transaction(|| { let entity_id = FatCatId::from_str(&id)?; + let hide_flags = match hide { + None => HideFlags::none(), + Some(param) => HideFlags::from_str(¶m)?, + }; match expand { - None => $model::db_get(&conn, entity_id), + None => $model::db_get(&conn, entity_id, hide_flags), Some(param) => { let expand_flags = ExpandFlags::from_str(¶m)?; - let mut entity = $model::db_get(&conn, entity_id)?; + let mut entity = $model::db_get(&conn, entity_id, hide_flags)?; entity.db_expand(&conn, expand_flags)?; Ok(entity) }, @@ -247,11 +252,16 @@ macro_rules! wrap_lookup_handler { fn $get_fn( &self, $idname: $idtype, + hide: Option, _context: &Context, ) -> Box + Send> { let conn = self.db_pool.get().expect("db_pool error"); + let hide_flags = match hide { + None => HideFlags::none(), + Some(param) => HideFlags::from_str(¶m).unwrap(), + }; // No transaction for GET - let ret = match self.$get_handler(&$idname, &conn) { + let ret = match self.$get_handler(&$idname, hide_flags, &conn) { Ok(entity) => $get_resp::FoundEntity(entity), Err(Error(ErrorKind::Diesel(::diesel::result::Error::NotFound), _)) => @@ -299,6 +309,42 @@ macro_rules! wrap_fcid_handler { } } +macro_rules! wrap_fcid_hide_handler { + ($get_fn:ident, $get_handler:ident, $get_resp:ident) => { + fn $get_fn( + &self, + id: String, + hide: Option, + _context: &Context, + ) -> Box + Send> { + let conn = self.db_pool.get().expect("db_pool error"); + // No transaction for GET + let ret = match (|| { + let fcid = FatCatId::from_str(&id)?; + let hide_flags = match hide { + None => HideFlags::none(), + Some(param) => HideFlags::from_str(¶m)?, + }; + self.$get_handler(fcid, hide_flags, &conn) + })() { + Ok(entity) => + $get_resp::Found(entity), + Err(Error(ErrorKind::Diesel(::diesel::result::Error::NotFound), _)) => + $get_resp::NotFound(ErrorResponse { message: format!("Not found: {}", id) }), + Err(Error(ErrorKind::MalformedExternalId(e), _)) => + $get_resp::BadRequest(ErrorResponse { message: e.to_string() }), + Err(Error(ErrorKind::NotInControlledVocabulary(e), _)) => + $get_resp::BadRequest(ErrorResponse { message: e.to_string() }), + Err(e) => { + error!("{}", e); + $get_resp::BadRequest(ErrorResponse { message: e.to_string() }) + }, + }; + Box::new(futures::done(Ok(ret))) + } + } +} + impl Api for Server { wrap_entity_handlers!( get_container, @@ -411,17 +457,17 @@ impl Api for Server { String ); - wrap_fcid_handler!( + wrap_fcid_hide_handler!( get_release_files, get_release_files_handler, GetReleaseFilesResponse ); - wrap_fcid_handler!( + wrap_fcid_hide_handler!( get_work_releases, get_work_releases_handler, GetWorkReleasesResponse ); - wrap_fcid_handler!( + wrap_fcid_hide_handler!( get_creator_releases, get_creator_releases_handler, GetCreatorReleasesResponse diff --git a/rust/src/bin/fatcat-export.rs b/rust/src/bin/fatcat-export.rs index 0d6d69b1..9dd5138e 100644 --- a/rust/src/bin/fatcat-export.rs +++ b/rust/src/bin/fatcat-export.rs @@ -73,7 +73,7 @@ macro_rules! generic_loop_work { fn $fn_name(row_receiver: channel::Receiver, output_sender: channel::Sender, db_conn: &DbConn, expand: Option) { let result: Result<()> = (|| { for row in row_receiver { - let mut entity = $entity_model::db_get_rev(db_conn, row.rev_id.expect("valid, non-deleted row")) + let mut entity = $entity_model::db_get_rev(db_conn, row.rev_id.expect("valid, non-deleted row"), HideFlags::none()) .chain_err(|| "reading entity from database")?; //let mut entity = ReleaseEntity::db_get_rev(db_conn, row.rev_id.expect("valid, non-deleted row"))?; entity.state = Some("active".to_string()); // XXX diff --git a/rust/tests/test_api_server_http.rs b/rust/tests/test_api_server_http.rs index c179637c..614e6b65 100644 --- a/rust/tests/test_api_server_http.rs +++ b/rust/tests/test_api_server_http.rs @@ -81,7 +81,7 @@ fn test_entity_gets() { // expand keyword check_http_response( request::get( - "http://localhost:9411/v0/release/aaaaaaaaaaaaarceaaaaaaaaai?expand=all", + "http://localhost:9411/v0/release/aaaaaaaaaaaaarceaaaaaaaaai?expand=container", headers.clone(), &router, ), @@ -89,6 +89,17 @@ fn test_entity_gets() { Some("MySpace Blog"), ); + // hide keyword + check_http_response( + request::get( + "http://localhost:9411/v0/release/aaaaaaaaaaaaarceaaaaaaaaai?hide=refs,container", + headers.clone(), + &router, + ), + status::Ok, + Some("bigger example"), + ); + check_http_response( request::get( "http://localhost:9411/v0/work/aaaaaaaaaaaaavkvaaaaaaaaai", @@ -463,8 +474,7 @@ fn test_post_release() { status::BadRequest, None, ); - */ -} + */} #[test] fn test_post_work() { diff --git a/rust/tests/test_old_python_tests.rs b/rust/tests/test_old_python_tests.rs index 1b496328..5e33a7f6 100644 --- a/rust/tests/test_old_python_tests.rs +++ b/rust/tests/test_old_python_tests.rs @@ -148,7 +148,7 @@ fn test_api_rich_create() { }; // test that foreign key relations worked - let re = match client.get_release(release_id.clone(), None).wait().unwrap() { + let re = match client.get_release(release_id.clone(), None, None).wait().unwrap() { GetReleaseResponse::FoundEntity(e) => e, _ => unreachable!(), }; @@ -166,7 +166,7 @@ fn test_api_rich_create() { stub_release_id ); - let fe = match client.get_file(file_id, None).wait().unwrap() { + let fe = match client.get_file(file_id, None, None).wait().unwrap() { GetFileResponse::FoundEntity(e) => e, _ => unreachable!(), }; -- cgit v1.2.3