From 54e34ddd56b705cba239f88a9239c603d3ecd20e Mon Sep 17 00:00:00 2001 From: Bryan Newbold Date: Fri, 11 Jan 2019 14:52:44 -0800 Subject: yet more edit lifecycle progress --- rust/src/auth.rs | 8 +++---- rust/src/bin/fatcat-auth.rs | 20 +++++++++------- rust/src/database_models.rs | 2 ++ rust/src/editing.rs | 54 ++++++++++++++++++++----------------------- rust/src/editing_crud.rs | 31 ++++++++++++++++++------- rust/src/endpoint_handlers.rs | 17 ++++++++++---- rust/src/endpoints.rs | 43 ++++++++++++++++++++++++---------- rust/src/entity_crud.rs | 8 +++---- 8 files changed, 113 insertions(+), 70 deletions(-) (limited to 'rust') diff --git a/rust/src/auth.rs b/rust/src/auth.rs index 8e7160bb..4627a535 100644 --- a/rust/src/auth.rs +++ b/rust/src/auth.rs @@ -11,12 +11,14 @@ use swagger::auth::{AuthData, Authorization, Scopes}; use crate::database_models::*; use crate::database_schema::*; +use crate::editing_crud::{EditgroupCrud, EditorCrud}; use crate::errors::*; use crate::identifiers::*; use crate::server::*; use chrono::prelude::*; use diesel; use diesel::prelude::*; +use fatcat_api_spec::models::{Editgroup, Editor}; use std::collections::HashMap; use std::env; use std::str::FromStr; @@ -73,9 +75,7 @@ impl AuthContext { if self.has_role(FatcatRole::Admin) { return Ok(()); } - let editgroup: EditgroupRow = editgroup::table - .find(editgroup_id.to_uuid()) - .get_result(conn)?; + let editgroup: EditgroupRow = Editgroup::db_get(conn, editgroup_id)?; match editgroup.editor_id == self.editor_id.to_uuid() { true => Ok(()), false => Err(FatcatError::InsufficientPrivileges(format!( @@ -312,7 +312,7 @@ impl AuthConfectionary { "time > {}", created.to_rfc3339_opts(SecondsFormat::Secs, true) )); - let editor: EditorRow = editor::table.find(&editor_id.to_uuid()).get_result(conn)?; + let editor: EditorRow = Editor::db_get(conn, editor_id)?; let auth_epoch = DateTime::::from_utc(editor.auth_epoch, Utc); // allow a second of wiggle room for precision and, eg, tests if created < (auth_epoch - chrono::Duration::seconds(1)) { diff --git a/rust/src/bin/fatcat-auth.rs b/rust/src/bin/fatcat-auth.rs index 0ba543b4..652c0db5 100644 --- a/rust/src/bin/fatcat-auth.rs +++ b/rust/src/bin/fatcat-auth.rs @@ -2,9 +2,11 @@ use clap::{App, SubCommand}; +use fatcat::editing_crud::EditorCrud; use fatcat::errors::Result; use fatcat::identifiers::FatcatId; -use fatcat::{auth, editing, server}; +use fatcat::{auth, server}; +use fatcat_api_spec::models::Editor; use std::process; use std::str::FromStr; @@ -75,14 +77,16 @@ fn main() -> Result<()> { fatcat::auth::print_editors(&db_conn)?; } ("create-editor", Some(subm)) => { - let editor = editing::create_editor( - &db_conn, - subm.value_of("username").unwrap().to_string(), - subm.is_present("admin"), - subm.is_present("bot"), - )?; + let editor = Editor { + editor_id: None, + username: subm.value_of("username").unwrap().to_string(), + is_admin: Some(subm.is_present("admin")), + is_bot: Some(subm.is_present("bot")), + is_active: Some(true), + }; + let editor_row = editor.db_create(&db_conn)?; //println!("{:?}", editor); - println!("{}", FatcatId::from_uuid(&editor.id).to_string()); + println!("{}", FatcatId::from_uuid(&editor_row.id).to_string()); } ("create-token", Some(subm)) => { let editor_id = FatcatId::from_str(subm.value_of("editor-id").unwrap())?; diff --git a/rust/src/database_models.rs b/rust/src/database_models.rs index ef6eb65d..c033c6e5 100644 --- a/rust/src/database_models.rs +++ b/rust/src/database_models.rs @@ -567,6 +567,7 @@ impl EditgroupRow { Editgroup { editgroup_id: Some(uuid2fcid(&self.id)), editor_id: Some(uuid2fcid(&self.editor_id)), + editor: None, submitted: self .submitted .map(|v| chrono::DateTime::from_utc(v, chrono::Utc)), @@ -595,6 +596,7 @@ impl EditgroupAnnotationRow { annotation_id: Some(self.id.to_string()), editgroup_id: Some(uuid2fcid(&self.editgroup_id)), editor_id: Some(uuid2fcid(&self.editor_id)), + editor: None, created: Some(chrono::DateTime::from_utc(self.created, chrono::Utc)), comment_markdown: self.comment_markdown, extra: self.extra_json, diff --git a/rust/src/editing.rs b/rust/src/editing.rs index c2061ae9..0d7a6b13 100644 --- a/rust/src/editing.rs +++ b/rust/src/editing.rs @@ -5,6 +5,7 @@ use crate::database_models::*; use crate::database_schema::*; +use crate::editing_crud::EditgroupCrud; use crate::entity_crud::EntityCrud; use crate::errors::{FatcatError, Result}; use crate::identifiers::FatcatId; @@ -12,7 +13,6 @@ use crate::server::DbConn; use diesel; use diesel::prelude::*; use fatcat_api_spec::models::*; -use uuid::Uuid; pub struct EditContext { pub editor_id: FatcatId, @@ -41,16 +41,28 @@ pub fn make_edit_context( editgroup_id: Option, autoaccept: bool, ) -> Result { + // *either* autoaccept is false and editgroup_id is Some, *or* autoaccept is true and + // editgroup_id is None let editgroup_id: FatcatId = match (editgroup_id, autoaccept) { - (Some(eg), _) => eg, - // If autoaccept and no editgroup_id passed, always create a new one for this transaction + (Some(eg), false) => eg, (None, true) => { - let eg_row: EditgroupRow = diesel::insert_into(editgroup::table) - .values((editgroup::editor_id.eq(editor_id.to_uuid()),)) - .get_result(conn)?; - FatcatId::from_uuid(&eg_row.id) + let eg = Editgroup { + editgroup_id: None, + editor_id: Some(editor_id.to_string()), + editor: None, + submitted: None, + description: None, + extra: None, + annotations: None, + edits: None, + }; + let row = eg.db_create(conn, autoaccept)?; + FatcatId::from_uuid(&row.id) + } + _ => { + // TODO: better error response + bail!("unsupported editgroup context"); } - (None, false) => FatcatId::from_uuid(&create_editgroup(conn, editor_id.to_uuid())?), }; Ok(EditContext { editor_id, @@ -87,26 +99,10 @@ pub fn accept_editgroup(conn: &DbConn, editgroup_id: FatcatId) -> Result Result { - unimplemented!() -} + // update editgroup row with is_accepted + diesel::update(editgroup::table.find(editgroup_id.to_uuid())) + .set(editgroup::is_accepted.eq(true)) + .execute(conn)?; -pub fn create_editor( - conn: &DbConn, - username: String, - is_admin: bool, - is_bot: bool, -) -> Result { - unimplemented!() -} - -pub fn update_editor_username( - conn: &DbConn, - editor_id: FatcatId, - username: String, -) -> Result { - unimplemented!() + Ok(entry) } diff --git a/rust/src/editing_crud.rs b/rust/src/editing_crud.rs index 86cc2fb3..d3782f7b 100644 --- a/rust/src/editing_crud.rs +++ b/rust/src/editing_crud.rs @@ -19,12 +19,11 @@ use uuid::Uuid; * Generic verbs/actions look like: * * - db_get (single) - * - db_get_range (group; by timestamp) + * - db_get_range (group; by timestamp, with limits) * - db_create (single) * - db_update (single) * - db_expand (single) * - * Annotations can be fetch with a join on either editgroup or editor, with same range parameters. */ pub trait EditorCrud { @@ -42,14 +41,14 @@ impl EditorCrud for Editor { identifiers::check_username(&self.username)?; let is_admin = self.is_admin.unwrap_or(false); let is_bot = self.is_bot.unwrap_or(false); - let ed: EditorRow = diesel::insert_into(editor::table) + let row: EditorRow = diesel::insert_into(editor::table) .values(( editor::username.eq(&self.username), editor::is_admin.eq(is_admin), editor::is_bot.eq(is_bot), )) .get_result(conn)?; - Ok(ed) + Ok(row) } fn db_update_username(&self, conn: &DbConn, editor_id: FatcatId) -> Result { @@ -88,8 +87,8 @@ pub trait EditgroupCrud { } impl EditgroupCrud for Editgroup { - // XXX: this should probably *alwas* retun changelog status as well. If we do that, can we get rid - // of the is_accepted thing? no, still want it as denormalized speed-up in some queries/filters. + // XXX: this could *alwas* return changelog status as well. If we do that, can we get rid of + // the is_accepted thing? no, still want it as denormalized speed-up in some queries/filters. /// This method does *not* expand the 'edits'; currently that's still done in the endpoint /// handler, but it probably should be done in this trait with a db_expand() @@ -111,8 +110,16 @@ impl EditgroupCrud for Editgroup { Ok(row) } + /// Note: this *still* doesn't epand the 'edits', at least yet. fn db_expand(&mut self, conn: &DbConn, expand: ExpandFlags) -> Result<()> { - // XXX: impl + if expand.editors { + let editor_id = FatcatId::from_str( + self.editor_id + .as_ref() + .expect("tried to expand bare Editor model"), + )?; + self.editor = Some(Editor::db_get(conn, editor_id)?.into_model()); + } Ok(()) } @@ -244,7 +251,15 @@ impl EditgroupAnnotationCrud for EditgroupAnnotation { } fn db_expand(&mut self, conn: &DbConn, expand: ExpandFlags) -> Result<()> { - unimplemented!() + if expand.editors { + let editor_id = FatcatId::from_str( + self.editor_id + .as_ref() + .expect("tried to expand bare Editor model"), + )?; + self.editor = Some(Editor::db_get(conn, editor_id)?.into_model()); + } + Ok(()) } fn db_get_range_for_editor( diff --git a/rust/src/endpoint_handlers.rs b/rust/src/endpoint_handlers.rs index 4b3108af..dcbb3d90 100644 --- a/rust/src/endpoint_handlers.rs +++ b/rust/src/endpoint_handlers.rs @@ -8,8 +8,8 @@ use crate::database_models::*; use crate::database_schema::*; use crate::editing::*; +use crate::editing_crud::{EditgroupCrud, EditorCrud}; use crate::entity_crud::{EntityCrud, ExpandFlags, HideFlags}; -use crate::editing_crud::{EditorCrud, EditgroupCrud}; use crate::errors::*; use crate::identifiers::*; use crate::server::*; @@ -472,7 +472,7 @@ impl Server { ) -> Result> { // XXX: delete me? // TODO: single query - let editor: EditorRow = editor::table.find(editor_id.to_uuid()).first(conn)?; + let editor: EditorRow = Editor::db_get(&conn, editor_id)?; let changes: Vec<(ChangelogRow, EditgroupRow)> = changelog::table .inner_join(editgroup::table) .filter(editgroup::editor_id.eq(editor.id)) @@ -545,17 +545,24 @@ impl Server { Some((editor, _)) => (editor.clone(), false), None => { let username = format!("{}-{}", params.preferred_username, params.provider); - let editor = create_editor(conn, username, false, false)?; + let editor = Editor { + editor_id: None, + username: username, + is_admin: Some(false), + is_bot: Some(false), + is_active: Some(true), + }; + let row = editor.db_create(conn)?; // create an auth login row so the user can log back in diesel::insert_into(auth_oidc::table) .values(( - auth_oidc::editor_id.eq(editor.id), + auth_oidc::editor_id.eq(row.id), auth_oidc::provider.eq(params.provider), auth_oidc::oidc_iss.eq(params.iss), auth_oidc::oidc_sub.eq(params.sub), )) .execute(conn)?; - (editor, true) + (row, true) } }; diff --git a/rust/src/endpoints.rs b/rust/src/endpoints.rs index 5e7ff234..8ed428a7 100644 --- a/rust/src/endpoints.rs +++ b/rust/src/endpoints.rs @@ -10,8 +10,8 @@ use crate::auth::FatcatRole; use crate::database_models::EntityEditRow; use crate::editing::*; +use crate::editing_crud::{EditgroupAnnotationCrud, EditgroupCrud, EditorCrud}; use crate::entity_crud::{EntityCrud, ExpandFlags, HideFlags}; -use crate::editing_crud::{EditorCrud, EditgroupCrud, EditgroupAnnotationCrud}; use crate::errors::*; use crate::identifiers::FatcatId; use crate::server::*; @@ -22,9 +22,9 @@ use fatcat_api_spec::models::*; use fatcat_api_spec::*; use futures::{self, Future}; use sentry::integrations::failure::capture_fail; +use std::cmp; use std::str::FromStr; use uuid::{self, Uuid}; -use std::cmp; // This makes response matching below *much* more terse use crate::errors::FatcatError::*; @@ -724,7 +724,12 @@ impl Api for Server { // admin can update any username auth_context.require_role(FatcatRole::Admin)?; }; - update_editor_username(&conn, editor_id, editor.username).map(|e| e.into_model()) + // only update fixed set of fields (username) + let mut existing = Editor::db_get(&conn, editor_id)?.into_model(); + existing.username = editor.username; + existing + .db_update_username(&conn, editor_id) + .map(|e| e.into_model()) }) .map_err(|e| FatcatError::from(e)) { @@ -747,7 +752,8 @@ impl Api for Server { .transaction(|| { let editor_id = FatcatId::from_str(&editor_id)?; let limit = cmp::min(100, limit.unwrap_or(20)) as u64; - let row = Editgroup::db_get_range_for_editor(&conn, editor_id, limit, since, before)?; + let row = + Editgroup::db_get_range_for_editor(&conn, editor_id, limit, since, before)?; Ok(row.into_iter().map(|eg| eg.into_model_partial()).collect()) }) .map_err(|e: Error| FatcatError::from(e)) @@ -771,7 +777,9 @@ impl Api for Server { .transaction(|| { let editor_id = FatcatId::from_str(&editor_id)?; let limit = cmp::min(100, limit.unwrap_or(20)) as u64; - let annotations = EditgroupAnnotation::db_get_range_for_editor(&conn, editor_id, limit, since, before)?; + let annotations = EditgroupAnnotation::db_get_range_for_editor( + &conn, editor_id, limit, since, before, + )?; Ok(annotations.into_iter().map(|a| a.into_model()).collect()) }) .map_err(|e: Error| FatcatError::from(e)) @@ -846,13 +854,20 @@ impl Api for Server { let editgroup_id = FatcatId::from_str(&editgroup_id)?; let limit: u64 = 1000; // TODO: controllable expansion... for now always expands editors - let annotations = EditgroupAnnotation::db_get_range_for_editgroup(&conn, editgroup_id, limit, None, None)?; - let mut annotations: Vec = annotations.into_iter().map(|a| a.into_model()).collect(); + let annotations = EditgroupAnnotation::db_get_range_for_editgroup( + &conn, + editgroup_id, + limit, + None, + None, + )?; + let mut annotations: Vec = + annotations.into_iter().map(|a| a.into_model()).collect(); if let Some(expand) = expand { let expand = ExpandFlags::from_str(&expand)?; for a in annotations.iter_mut() { a.db_expand(&conn, expand)?; - }; + } }; Ok(annotations) }) @@ -877,15 +892,17 @@ impl Api for Server { .transaction(|| { let limit = cmp::min(100, limit.unwrap_or(20)) as u64; let row = Editgroup::db_get_range_reviewable(&conn, limit, since, before)?; - let mut editgroups: Vec = row.into_iter().map(|eg| eg.into_model_partial()).collect(); + let mut editgroups: Vec = + row.into_iter().map(|eg| eg.into_model_partial()).collect(); if let Some(expand) = expand { let expand = ExpandFlags::from_str(&expand)?; for eg in editgroups.iter_mut() { eg.db_expand(&conn, expand)?; - }; + } }; Ok(editgroups) - }).map_err(|e: Error| FatcatError::from(e)) + }) + .map_err(|e: Error| FatcatError::from(e)) { Ok(editgroups) => GetEditgroupsReviewableResponse::Found(editgroups), Err(fe) => generic_err_responses!(fe, GetEditgroupsReviewableResponse), @@ -973,7 +990,9 @@ impl Api for Server { // admin can update any editgroup auth_context.require_role(FatcatRole::Admin)?; }; - editgroup.db_update(&conn, editgroup_id, submit).map(|eg| eg.into_model_partial()) + editgroup + .db_update(&conn, editgroup_id, submit) + .map(|eg| eg.into_model_partial()) }) .map_err(|e: Error| FatcatError::from(e)) { diff --git a/rust/src/entity_crud.rs b/rust/src/entity_crud.rs index 2c54e715..b2ef6323 100644 --- a/rust/src/entity_crud.rs +++ b/rust/src/entity_crud.rs @@ -162,8 +162,8 @@ fn test_expand_flags() { webcaptures: true, container: true, releases: true, - creators: true - editors: true + creators: true, + editors: true, } ); assert!(ExpandFlags::from_str("").unwrap().files == false); @@ -181,8 +181,8 @@ fn test_expand_flags() { webcaptures: true, container: true, releases: true, - creators: true - editors: true + creators: true, + editors: true, } ); } -- cgit v1.2.3