From cbf615dda68367600c47c6867d27737c0113ca39 Mon Sep 17 00:00:00 2001 From: Bryan Newbold Date: Wed, 9 Jan 2019 12:42:02 -0800 Subject: refactor to have consistent db conn argument order 'conn' parameter always comes first. --- rust/src/editing.rs | 6 ++--- rust/src/endpoint_handlers.rs | 52 +++++++++++++++++++------------------- rust/src/endpoints.rs | 24 +++++++++--------- rust/src/entity_crud.rs | 2 +- rust/tests/test_api_server_http.rs | 16 ++++++------ 5 files changed, 50 insertions(+), 50 deletions(-) diff --git a/rust/src/editing.rs b/rust/src/editing.rs index b501a6cf..b488e489 100644 --- a/rust/src/editing.rs +++ b/rust/src/editing.rs @@ -45,7 +45,7 @@ pub fn make_edit_context( .get_result(conn)?; FatCatId::from_uuid(&eg_row.id) } - (None, false) => FatCatId::from_uuid(&get_or_create_editgroup(editor_id.to_uuid(), conn)?), + (None, false) => FatCatId::from_uuid(&get_or_create_editgroup(conn, editor_id.to_uuid())?), }; Ok(EditContext { editor_id, @@ -86,7 +86,7 @@ pub fn update_editor_username( } /// This function should always be run within a transaction -pub fn get_or_create_editgroup(editor_id: Uuid, conn: &DbConn) -> Result { +pub fn get_or_create_editgroup(conn: &DbConn, editor_id: Uuid) -> Result { // check for current active let ed_row: EditorRow = editor::table.find(editor_id).first(conn)?; if let Some(current) = ed_row.active_editgroup_id { @@ -104,7 +104,7 @@ pub fn get_or_create_editgroup(editor_id: Uuid, conn: &DbConn) -> Result { } /// This function should always be run within a transaction -pub fn accept_editgroup(editgroup_id: FatCatId, conn: &DbConn) -> Result { +pub fn accept_editgroup(conn: &DbConn, editgroup_id: FatCatId) -> Result { // check that we haven't accepted already (in changelog) // NB: could leave this to a UNIQUE constraint // TODO: redundant with check_edit_context diff --git a/rust/src/endpoint_handlers.rs b/rust/src/endpoint_handlers.rs index d2576d53..ff49f3d1 100644 --- a/rust/src/endpoint_handlers.rs +++ b/rust/src/endpoint_handlers.rs @@ -20,11 +20,11 @@ macro_rules! entity_batch_handler { ($post_batch_handler:ident, $model:ident) => { pub fn $post_batch_handler( &self, + conn: &DbConn, entity_list: &[models::$model], autoaccept: bool, editor_id: FatCatId, editgroup_id: Option, - conn: &DbConn, ) -> Result> { let edit_context = make_edit_context(conn, editor_id, editgroup_id, autoaccept)?; @@ -43,9 +43,9 @@ macro_rules! entity_batch_handler { } pub fn get_release_files( + conn: &DbConn, ident: FatCatId, hide_flags: HideFlags, - conn: &DbConn, ) -> Result> { let rows: Vec<(FileRevRow, FileIdentRow, FileRevReleaseRow)> = file_rev::table .inner_join(file_ident::table) @@ -61,9 +61,9 @@ pub fn get_release_files( } pub fn get_release_filesets( + conn: &DbConn, ident: FatCatId, hide_flags: HideFlags, - conn: &DbConn, ) -> Result> { let rows: Vec<(FilesetRevRow, FilesetIdentRow, FilesetRevReleaseRow)> = fileset_rev::table .inner_join(fileset_ident::table) @@ -79,9 +79,9 @@ pub fn get_release_filesets( } pub fn get_release_webcaptures( + conn: &DbConn, ident: FatCatId, hide_flags: HideFlags, - conn: &DbConn, ) -> Result> { let rows: Vec<( WebcaptureRevRow, @@ -103,11 +103,11 @@ pub fn get_release_webcaptures( impl Server { pub fn lookup_container_handler( &self, + conn: &DbConn, issnl: &Option, wikidata_qid: &Option, expand_flags: ExpandFlags, hide_flags: HideFlags, - conn: &DbConn, ) -> Result { let (ident, rev): (ContainerIdentRow, ContainerRevRow) = match (issnl, wikidata_qid) { (Some(issnl), None) => { @@ -140,11 +140,11 @@ impl Server { pub fn lookup_creator_handler( &self, + conn: &DbConn, orcid: &Option, wikidata_qid: &Option, expand_flags: ExpandFlags, hide_flags: HideFlags, - conn: &DbConn, ) -> Result { let (ident, rev): (CreatorIdentRow, CreatorRevRow) = match (orcid, wikidata_qid) { (Some(orcid), None) => { @@ -177,9 +177,9 @@ impl Server { pub fn get_creator_releases_handler( &self, + conn: &DbConn, ident: FatCatId, hide_flags: HideFlags, - conn: &DbConn, ) -> Result> { // TODO: some kind of unique or group-by? let rows: Vec<(ReleaseRevRow, ReleaseIdentRow, ReleaseContribRow)> = release_rev::table @@ -198,12 +198,12 @@ impl Server { pub fn lookup_file_handler( &self, + conn: &DbConn, md5: &Option, sha1: &Option, sha256: &Option, expand_flags: ExpandFlags, hide_flags: HideFlags, - conn: &DbConn, ) -> Result { let (ident, rev): (FileIdentRow, FileRevRow) = match (md5, sha1, sha256) { (Some(md5), None, None) => { @@ -245,6 +245,7 @@ impl Server { pub fn lookup_release_handler( &self, + conn: &DbConn, doi: &Option, wikidata_qid: &Option, isbn13: &Option, @@ -253,7 +254,6 @@ impl Server { core_id: &Option, expand_flags: ExpandFlags, hide_flags: HideFlags, - conn: &DbConn, ) -> Result { let (ident, rev): (ReleaseIdentRow, ReleaseRevRow) = match (doi, wikidata_qid, isbn13, pmid, pmcid, core_id) { @@ -325,36 +325,36 @@ impl Server { pub fn get_release_files_handler( &self, + conn: &DbConn, ident: FatCatId, hide_flags: HideFlags, - conn: &DbConn, ) -> Result> { - get_release_files(ident, hide_flags, conn) + get_release_files(conn, ident, hide_flags) } pub fn get_release_filesets_handler( &self, + conn: &DbConn, ident: FatCatId, hide_flags: HideFlags, - conn: &DbConn, ) -> Result> { - get_release_filesets(ident, hide_flags, conn) + get_release_filesets(conn, ident, hide_flags) } pub fn get_release_webcaptures_handler( &self, + conn: &DbConn, ident: FatCatId, hide_flags: HideFlags, - conn: &DbConn, ) -> Result> { - get_release_webcaptures(ident, hide_flags, conn) + get_release_webcaptures(conn, ident, hide_flags) } pub fn get_work_releases_handler( &self, + conn: &DbConn, ident: FatCatId, hide_flags: HideFlags, - conn: &DbConn, ) -> Result> { let rows: Vec<(ReleaseRevRow, ReleaseIdentRow)> = release_rev::table .inner_join(release_ident::table) @@ -368,15 +368,15 @@ impl Server { .collect() } - pub fn accept_editgroup_handler(&self, editgroup_id: FatCatId, conn: &DbConn) -> Result<()> { - accept_editgroup(editgroup_id, conn)?; + pub fn accept_editgroup_handler(&self, conn: &DbConn, editgroup_id: FatCatId) -> Result<()> { + accept_editgroup(conn, editgroup_id)?; Ok(()) } pub fn create_editgroup_handler( &self, - entity: models::Editgroup, conn: &DbConn, + entity: models::Editgroup, ) -> Result { let row: EditgroupRow = insert_into(editgroup::table) .values(( @@ -397,8 +397,8 @@ impl Server { pub fn get_editgroup_handler( &self, - editgroup_id: FatCatId, conn: &DbConn, + editgroup_id: FatCatId, ) -> Result { let row: EditgroupRow = editgroup::table.find(editgroup_id.to_uuid()).first(conn)?; @@ -471,15 +471,15 @@ impl Server { Ok(eg) } - pub fn get_editor_handler(&self, editor_id: FatCatId, conn: &DbConn) -> Result { + pub fn get_editor_handler(&self, conn: &DbConn, editor_id: FatCatId) -> Result { let row: EditorRow = editor::table.find(editor_id.to_uuid()).first(conn)?; Ok(row.into_model()) } pub fn get_editor_changelog_handler( &self, - editor_id: FatCatId, conn: &DbConn, + editor_id: FatCatId, ) -> Result> { // TODO: single query let editor: EditorRow = editor::table.find(editor_id.to_uuid()).first(conn)?; @@ -502,8 +502,8 @@ impl Server { pub fn get_changelog_handler( &self, - limit: Option, conn: &DbConn, + limit: Option, ) -> Result> { let limit = limit.unwrap_or(50); @@ -525,10 +525,10 @@ impl Server { Ok(entries) } - pub fn get_changelog_entry_handler(&self, index: i64, conn: &DbConn) -> Result { + pub fn get_changelog_entry_handler(&self, conn: &DbConn, index: i64) -> Result { let cl_row: ChangelogRow = changelog::table.find(index).first(conn)?; let editgroup = - self.get_editgroup_handler(FatCatId::from_uuid(&cl_row.editgroup_id), conn)?; + self.get_editgroup_handler(conn, FatCatId::from_uuid(&cl_row.editgroup_id))?; let mut entry = cl_row.into_model(); entry.editgroup = Some(editgroup); @@ -544,7 +544,7 @@ impl Server { /// "{preferred_username}-{provider}"; the intent is for this to be temporary but unique. Might /// look like "bnewbold-github", or might look like "895139824-github". This is a hack to make /// check/creation idempotent. - pub fn auth_oidc_handler(&self, params: AuthOidc, conn: &DbConn) -> Result<(Editor, bool)> { + pub fn auth_oidc_handler(&self, conn: &DbConn, params: AuthOidc) -> Result<(Editor, bool)> { let existing: Vec<(EditorRow, AuthOidcRow)> = editor::table .inner_join(auth_oidc::table) .filter(auth_oidc::oidc_sub.eq(params.sub.clone())) diff --git a/rust/src/endpoints.rs b/rust/src/endpoints.rs index c1033bc7..6e67eadc 100644 --- a/rust/src/endpoints.rs +++ b/rust/src/endpoints.rs @@ -152,7 +152,7 @@ macro_rules! wrap_entity_handlers { auth_context.require_editgroup(&conn, eg_id)?; Some(eg_id) } else { None }; - self.$post_batch_handler(entity_list, autoaccept.unwrap_or(false), auth_context.editor_id, editgroup_id, &conn) + self.$post_batch_handler(&conn, entity_list, autoaccept.unwrap_or(false), auth_context.editor_id, editgroup_id) }) { Ok(edit) => $post_batch_resp::CreatedEntities(edit), @@ -487,7 +487,7 @@ macro_rules! wrap_lookup_handler { Some(param) => HideFlags::from_str(¶m).unwrap(), }; // No transaction for GET - let ret = match self.$get_handler(&$idname, &wikidata_qid, expand_flags, hide_flags, &conn) { + let ret = match self.$get_handler(&conn, &$idname, &wikidata_qid, expand_flags, hide_flags) { Ok(entity) => $get_resp::FoundEntity(entity), Err(Error(ErrorKind::Diesel(::diesel::result::Error::NotFound), _)) => @@ -521,7 +521,7 @@ macro_rules! wrap_fcid_handler { // No transaction for GET let ret = match (|| { let fcid = FatCatId::from_str(&id)?; - self.$get_handler(fcid, &conn) + self.$get_handler(&conn, fcid) })() { Ok(entity) => $get_resp::Found(entity), @@ -559,7 +559,7 @@ macro_rules! wrap_fcid_hide_handler { None => HideFlags::none(), Some(param) => HideFlags::from_str(¶m)?, }; - self.$get_handler(fcid, hide_flags, &conn) + self.$get_handler(&conn, fcid, hide_flags) })() { Ok(entity) => $get_resp::Found(entity), @@ -817,7 +817,7 @@ impl Api for Server { }; // No transaction for GET let ret = - match self.lookup_file_handler(&md5, &sha1, &sha256, expand_flags, hide_flags, &conn) { + match self.lookup_file_handler(&conn, &md5, &sha1, &sha256, expand_flags, hide_flags) { Ok(entity) => LookupFileResponse::FoundEntity(entity), Err(Error(ErrorKind::Diesel(::diesel::result::Error::NotFound), _)) => { LookupFileResponse::NotFound(ErrorResponse { @@ -872,6 +872,7 @@ impl Api for Server { }; // No transaction for GET let ret = match self.lookup_release_handler( + &conn, &doi, &wikidata_qid, &isbn13, @@ -880,7 +881,6 @@ impl Api for Server { &core_id, expand_flags, hide_flags, - &conn, ) { Ok(entity) => LookupReleaseResponse::FoundEntity(entity), Err(Error(ErrorKind::Diesel(::diesel::result::Error::NotFound), _)) => { @@ -1003,7 +1003,7 @@ impl Api for Server { auth_context.require_role(FatcatRole::Admin)?; // NOTE: this is currently redundant, but zero-cost auth_context.require_editgroup(&conn, editgroup_id)?; - self.accept_editgroup_handler(editgroup_id, &conn) + self.accept_editgroup_handler(&conn, editgroup_id) }) { Ok(()) => AcceptEditgroupResponse::MergedSuccessfully(Success { message: "horray!".to_string(), @@ -1043,7 +1043,7 @@ impl Api for Server { let conn = self.db_pool.get().expect("db_pool error"); let ret = match conn.transaction(|| { let editgroup_id = FatCatId::from_str(&editgroup_id)?; - self.get_editgroup_handler(editgroup_id, &conn) + self.get_editgroup_handler(&conn, editgroup_id) }) { Ok(entity) => GetEditgroupResponse::Found(entity), Err(Error(ErrorKind::Diesel(::diesel::result::Error::NotFound), _)) => { @@ -1088,7 +1088,7 @@ impl Api for Server { entity.editor_id = Some(auth_context.editor_id.to_string()); } }; - self.create_editgroup_handler(entity, &conn) + self.create_editgroup_handler(&conn, entity) }) { Ok(eg) => CreateEditgroupResponse::SuccessfullyCreated(eg), Err(Error(ErrorKind::InvalidCredentials(e), _)) => { @@ -1119,7 +1119,7 @@ impl Api for Server { ) -> Box + Send> { let conn = self.db_pool.get().expect("db_pool error"); // No transaction for GET - let ret = match self.get_changelog_handler(limit, &conn) { + let ret = match self.get_changelog_handler(&conn, limit) { Ok(changelog) => GetChangelogResponse::Success(changelog), Err(e) => { error!("{}", e); @@ -1138,7 +1138,7 @@ impl Api for Server { ) -> Box + Send> { let conn = self.db_pool.get().expect("db_pool error"); // No transaction for GET - let ret = match self.get_changelog_entry_handler(id, &conn) { + let ret = match self.get_changelog_entry_handler(&conn, id) { Ok(entry) => GetChangelogEntryResponse::FoundChangelogEntry(entry), Err(Error(ErrorKind::Diesel(::diesel::result::Error::NotFound), _)) => { GetChangelogEntryResponse::NotFound(ErrorResponse { @@ -1168,7 +1168,7 @@ impl Api for Server { Some("auth_oidc"), )?; auth_context.require_role(FatcatRole::Superuser)?; - let (editor, created) = self.auth_oidc_handler(params, &conn)?; + let (editor, created) = self.auth_oidc_handler(&conn, params)?; // create an auth token with 31 day duration let token = self.auth_confectionary.create_token( FatCatId::from_str(&editor.editor_id.clone().unwrap())?, diff --git a/rust/src/entity_crud.rs b/rust/src/entity_crud.rs index 186ee20c..81b359da 100644 --- a/rust/src/entity_crud.rs +++ b/rust/src/entity_crud.rs @@ -1649,7 +1649,7 @@ impl EntityCrud for ReleaseEntity { Some(redir) => FatCatId::from_str(&redir)?, }, }; - self.files = Some(get_release_files(ident, HideFlags::none(), conn)?); + self.files = Some(get_release_files(conn, ident, HideFlags::none())?); } if expand.container { if let Some(ref cid) = self.container_id { diff --git a/rust/tests/test_api_server_http.rs b/rust/tests/test_api_server_http.rs index 2ea01658..b1fc5e87 100644 --- a/rust/tests/test_api_server_http.rs +++ b/rust/tests/test_api_server_http.rs @@ -571,7 +571,7 @@ fn test_post_file() { ); let editor_id = Uuid::parse_str("00000000-0000-0000-AAAA-000000000001").unwrap(); - let editgroup_id = get_or_create_editgroup(editor_id, &conn).unwrap(); + let editgroup_id = get_or_create_editgroup(&conn, editor_id).unwrap(); helpers::check_http_response( request::post( &format!( @@ -637,7 +637,7 @@ fn test_post_fileset() { ); let editor_id = Uuid::parse_str("00000000-0000-0000-AAAA-000000000001").unwrap(); - let editgroup_id = get_or_create_editgroup(editor_id, &conn).unwrap(); + let editgroup_id = get_or_create_editgroup(&conn, editor_id).unwrap(); helpers::check_http_response( request::post( &format!( @@ -701,7 +701,7 @@ fn test_post_webcapture() { ); let editor_id = Uuid::parse_str("00000000-0000-0000-AAAA-000000000001").unwrap(); - let editgroup_id = get_or_create_editgroup(editor_id, &conn).unwrap(); + let editgroup_id = get_or_create_editgroup(&conn, editor_id).unwrap(); helpers::check_http_response( request::post( &format!( @@ -847,7 +847,7 @@ fn test_update_work() { ); let editor_id = Uuid::parse_str("00000000-0000-0000-AAAA-000000000001").unwrap(); - let editgroup_id = get_or_create_editgroup(editor_id, &conn).unwrap(); + let editgroup_id = get_or_create_editgroup(&conn, editor_id).unwrap(); helpers::check_http_response( request::post( &format!( @@ -878,7 +878,7 @@ fn test_delete_work() { ); let editor_id = Uuid::parse_str("00000000-0000-0000-AAAA-000000000001").unwrap(); - let editgroup_id = get_or_create_editgroup(editor_id, &conn).unwrap(); + let editgroup_id = get_or_create_editgroup(&conn, editor_id).unwrap(); helpers::check_http_response( request::post( &format!( @@ -899,7 +899,7 @@ fn test_accept_editgroup() { let (headers, router, conn) = helpers::setup_http(); let editor_id = Uuid::parse_str("00000000-0000-0000-AAAA-000000000001").unwrap(); - let editgroup_id = get_or_create_editgroup(editor_id, &conn).unwrap(); + let editgroup_id = get_or_create_editgroup(&conn, editor_id).unwrap(); let c: i64 = container_ident::table .filter(container_ident::is_live.eq(false)) @@ -1237,7 +1237,7 @@ fn test_abstracts() { ); let editor_id = Uuid::parse_str("00000000-0000-0000-AAAA-000000000001").unwrap(); - let editgroup_id = get_or_create_editgroup(editor_id, &conn).unwrap(); + let editgroup_id = get_or_create_editgroup(&conn, editor_id).unwrap(); helpers::check_http_response( request::post( &format!( @@ -1309,7 +1309,7 @@ fn test_contribs() { ); let editor_id = Uuid::parse_str("00000000-0000-0000-AAAA-000000000001").unwrap(); - let editgroup_id = get_or_create_editgroup(editor_id, &conn).unwrap(); + let editgroup_id = get_or_create_editgroup(&conn, editor_id).unwrap(); helpers::check_http_response( request::post( &format!( -- cgit v1.2.3