From 99373d8662b63cc50d30a0a9b277a558f8c3ccc9 Mon Sep 17 00:00:00 2001 From: Bryan Newbold Date: Fri, 11 Jan 2019 16:18:20 -0800 Subject: implement since/before ordering for range requests --- rust/src/database_models.rs | 3 +- rust/src/editing.rs | 1 + rust/src/editing_crud.rs | 159 +++++++++++++++++++++++++++++++----------- rust/src/endpoint_handlers.rs | 33 ++------- rust/src/endpoints.rs | 8 +-- rust/src/entity_crud.rs | 2 +- 6 files changed, 131 insertions(+), 75 deletions(-) (limited to 'rust/src') diff --git a/rust/src/database_models.rs b/rust/src/database_models.rs index c033c6e5..63fbcb29 100644 --- a/rust/src/database_models.rs +++ b/rust/src/database_models.rs @@ -563,11 +563,12 @@ pub struct EditgroupRow { impl EditgroupRow { /// Returns an Editgroup API model *without* the entity edits actually populated. Useful for, /// eg, entity history queries (where we already have the entity edit we want) - pub fn into_model_partial(self) -> Editgroup { + pub fn into_model_partial(self, changelog_index: Option) -> Editgroup { Editgroup { editgroup_id: Some(uuid2fcid(&self.id)), editor_id: Some(uuid2fcid(&self.editor_id)), editor: None, + changelog_index: changelog_index, submitted: self .submitted .map(|v| chrono::DateTime::from_utc(v, chrono::Utc)), diff --git a/rust/src/editing.rs b/rust/src/editing.rs index 0d7a6b13..33caf6a9 100644 --- a/rust/src/editing.rs +++ b/rust/src/editing.rs @@ -50,6 +50,7 @@ pub fn make_edit_context( editgroup_id: None, editor_id: Some(editor_id.to_string()), editor: None, + changelog_index: None, submitted: None, description: None, extra: None, diff --git a/rust/src/editing_crud.rs b/rust/src/editing_crud.rs index d3782f7b..65403eee 100644 --- a/rust/src/editing_crud.rs +++ b/rust/src/editing_crud.rs @@ -63,6 +63,10 @@ impl EditorCrud for Editor { pub trait EditgroupCrud { fn db_get(conn: &DbConn, editgroup_id: FatcatId) -> Result; + fn db_get_with_changelog( + conn: &DbConn, + editgroup_id: FatcatId, + ) -> Result<(EditgroupRow, Option)>; fn db_expand(&mut self, conn: &DbConn, expand: ExpandFlags) -> Result<()>; fn db_get_range_for_editor( conn: &DbConn, @@ -70,7 +74,7 @@ pub trait EditgroupCrud { limit: u64, since: Option>, before: Option>, - ) -> Result>; + ) -> Result)>>; fn db_get_range_reviewable( conn: &DbConn, limit: u64, @@ -87,30 +91,35 @@ pub trait EditgroupCrud { } impl EditgroupCrud for Editgroup { - // 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() fn db_get(conn: &DbConn, editgroup_id: FatcatId) -> Result { - let row: EditgroupRow = editgroup::table - .find(editgroup_id.to_uuid()) - .get_result(conn)?; + // Note: at least for now, continue to fetch along with changelog to ensure is_accepted is + // consistent. + let (row, _): (EditgroupRow, Option) = + Self::db_get_with_changelog(conn, editgroup_id)?; + Ok(row) + } + + fn db_get_with_changelog( + conn: &DbConn, + editgroup_id: FatcatId, + ) -> Result<(EditgroupRow, Option)> { + let (eg_row, cl_row): (EditgroupRow, Option) = editgroup::table + .left_outer_join(changelog::table) + .filter(editgroup::id.eq(editgroup_id.to_uuid())) + .first(conn)?; - // Note: for now this is really conservative and verifies the is_accepted flag every time - let count: i64 = changelog::table - .filter(changelog::editgroup_id.eq(editgroup_id.to_uuid())) - .count() - .get_result(conn)?; ensure!( - (count > 0) == row.is_accepted, + cl_row.is_some() == eg_row.is_accepted, "internal database consistency error on editgroup: {}", editgroup_id ); - Ok(row) + Ok((eg_row, cl_row)) } - /// Note: this *still* doesn't epand the 'edits', at least yet. + /// Note: this *still* doesn't epand the 'edits', at least yet. *Only* the direct editgrop + /// 'GET' handler does that. fn db_expand(&mut self, conn: &DbConn, expand: ExpandFlags) -> Result<()> { if expand.editors { let editor_id = FatcatId::from_str( @@ -129,17 +138,34 @@ impl EditgroupCrud for Editgroup { limit: u64, since: Option>, before: Option>, - ) -> Result> { - // TODO: since/before - let rows: Vec = match (since, before) { - _ => { + ) -> Result)>> { + let rows: Vec<(EditgroupRow, Option)> = match (since, before) { + (Some(since), None) => { + editgroup::table + .left_outer_join(changelog::table) + .filter(editgroup::editor_id.eq(editor_id.to_uuid())) + .filter(editgroup::created.gt(since)) + .order_by(editgroup::created.asc()) + .limit(limit as i64) + .get_results(conn)? + }, + (_, Some(before)) => { editgroup::table + .left_outer_join(changelog::table) .filter(editgroup::editor_id.eq(editor_id.to_uuid())) - // XXX: .filter(editgroup::created + .filter(editgroup::created.lt(before)) .order_by(editgroup::created.desc()) .limit(limit as i64) .get_results(conn)? - } + }, + (None, None) => { + editgroup::table + .left_outer_join(changelog::table) + .filter(editgroup::editor_id.eq(editor_id.to_uuid())) + .order_by(editgroup::created.desc()) + .limit(limit as i64) + .get_results(conn)? + }, }; Ok(rows) } @@ -150,14 +176,33 @@ impl EditgroupCrud for Editgroup { since: Option>, before: Option>, ) -> Result> { - // TODO: since/before let rows: Vec = match (since, before) { - _ => editgroup::table - .filter(editgroup::is_accepted.eq(false)) - .filter(editgroup::submitted.is_not_null()) - .order_by(editgroup::created.desc()) - .limit(limit as i64) - .get_results(conn)?, + (Some(since), None) => { + editgroup::table + .filter(editgroup::is_accepted.eq(false)) + .filter(editgroup::submitted.is_not_null()) + .filter(editgroup::submitted.gt(since)) + .order_by(editgroup::submitted.asc()) + .limit(limit as i64) + .get_results(conn)? + }, + (_, Some(before)) => { + editgroup::table + .filter(editgroup::is_accepted.eq(false)) + .filter(editgroup::submitted.is_not_null()) + .filter(editgroup::submitted.lt(before)) + .order_by(editgroup::submitted.desc()) + .limit(limit as i64) + .get_results(conn)? + }, + (None, None) => { + editgroup::table + .filter(editgroup::is_accepted.eq(false)) + .filter(editgroup::submitted.is_not_null()) + .order_by(editgroup::created.desc()) + .limit(limit as i64) + .get_results(conn)? + }, }; Ok(rows) } @@ -269,13 +314,30 @@ impl EditgroupAnnotationCrud for EditgroupAnnotation { since: Option>, before: Option>, ) -> Result> { - // TODO: since/before let rows: Vec = match (since, before) { - _ => editgroup_annotation::table - .filter(editgroup_annotation::editor_id.eq(editor_id.to_uuid())) - .order_by(editgroup_annotation::created.desc()) - .limit(limit as i64) - .get_results(conn)?, + (Some(since), None) => { + editgroup_annotation::table + .filter(editgroup_annotation::editor_id.eq(editor_id.to_uuid())) + .filter(editgroup_annotation::created.gt(since)) + .order_by(editgroup_annotation::created.asc()) + .limit(limit as i64) + .get_results(conn)? + }, + (_, Some(before)) => { + editgroup_annotation::table + .filter(editgroup_annotation::editor_id.eq(editor_id.to_uuid())) + .filter(editgroup_annotation::created.lt(before)) + .order_by(editgroup_annotation::created.desc()) + .limit(limit as i64) + .get_results(conn)? + }, + (None, None) => { + editgroup_annotation::table + .filter(editgroup_annotation::editor_id.eq(editor_id.to_uuid())) + .order_by(editgroup_annotation::created.desc()) + .limit(limit as i64) + .get_results(conn)? + }, }; Ok(rows) } @@ -287,13 +349,30 @@ impl EditgroupAnnotationCrud for EditgroupAnnotation { since: Option>, before: Option>, ) -> Result> { - // TODO: since/before let rows: Vec = match (since, before) { - _ => editgroup_annotation::table - .filter(editgroup_annotation::editgroup_id.eq(editgroup_id.to_uuid())) - .order_by(editgroup_annotation::created.desc()) - .limit(limit as i64) - .get_results(conn)?, + (Some(since), None) => { + editgroup_annotation::table + .filter(editgroup_annotation::editgroup_id.eq(editgroup_id.to_uuid())) + .filter(editgroup_annotation::created.gt(since)) + .order_by(editgroup_annotation::created.asc()) + .limit(limit as i64) + .get_results(conn)? + }, + (_, Some(before)) => { + editgroup_annotation::table + .filter(editgroup_annotation::editgroup_id.eq(editgroup_id.to_uuid())) + .filter(editgroup_annotation::created.lt(before)) + .order_by(editgroup_annotation::created.desc()) + .limit(limit as i64) + .get_results(conn)? + }, + (None, None) => { + editgroup_annotation::table + .filter(editgroup_annotation::editgroup_id.eq(editgroup_id.to_uuid())) + .order_by(editgroup_annotation::created.desc()) + .limit(limit as i64) + .get_results(conn)? + }, }; Ok(rows) } diff --git a/rust/src/endpoint_handlers.rs b/rust/src/endpoint_handlers.rs index dcbb3d90..bc606af9 100644 --- a/rust/src/endpoint_handlers.rs +++ b/rust/src/endpoint_handlers.rs @@ -386,7 +386,7 @@ impl Server { editgroup: models::Editgroup, ) -> Result { let row = editgroup.db_create(conn, false)?; - Ok(row.into_model_partial()) + Ok(row.into_model_partial(None)) } pub fn get_editgroup_handler( @@ -394,8 +394,8 @@ impl Server { conn: &DbConn, editgroup_id: FatcatId, ) -> Result { - let row: EditgroupRow = Editgroup::db_get(conn, editgroup_id)?; - let mut editgroup = row.into_model_partial(); + let (eg_row, cl_row) = Editgroup::db_get_with_changelog(conn, editgroup_id)?; + let mut editgroup = eg_row.into_model_partial(cl_row.map(|cl| cl.id)); let edits = EditgroupEdits { containers: Some( @@ -465,31 +465,6 @@ impl Server { Ok(row.into_model()) } - pub fn get_editor_changelog_handler( - &self, - conn: &DbConn, - editor_id: FatcatId, - ) -> Result> { - // XXX: delete me? - // TODO: single query - 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)) - .load(conn)?; - - let entries = changes - .into_iter() - .map(|(cl_row, eg_row)| ChangelogEntry { - index: cl_row.id, - editgroup: Some(eg_row.into_model_partial()), - editgroup_id: uuid2fcid(&cl_row.editgroup_id), - timestamp: chrono::DateTime::from_utc(cl_row.timestamp, chrono::Utc), - }) - .collect(); - Ok(entries) - } - pub fn get_changelog_handler( &self, conn: &DbConn, @@ -507,7 +482,7 @@ impl Server { .into_iter() .map(|(cl_row, eg_row)| ChangelogEntry { index: cl_row.id, - editgroup: Some(eg_row.into_model_partial()), + editgroup: Some(eg_row.into_model_partial(None)), editgroup_id: uuid2fcid(&cl_row.editgroup_id), timestamp: chrono::DateTime::from_utc(cl_row.timestamp, chrono::Utc), }) diff --git a/rust/src/endpoints.rs b/rust/src/endpoints.rs index 77f70ce0..33c4bc3e 100644 --- a/rust/src/endpoints.rs +++ b/rust/src/endpoints.rs @@ -752,9 +752,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 row = + let rows = Editgroup::db_get_range_for_editor(&conn, editor_id, limit, since, before)?; - Ok(row.into_iter().map(|eg| eg.into_model_partial()).collect()) + Ok(rows.into_iter().map(|(eg, cl)| eg.into_model_partial(cl.map(|v| v.id))).collect()) }) .map_err(|e: Error| FatcatError::from(e)) { @@ -893,7 +893,7 @@ impl Api for Server { 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(); + row.into_iter().map(|eg| eg.into_model_partial(None)).collect(); if let Some(expand) = expand { let expand = ExpandFlags::from_str(&expand)?; for eg in editgroups.iter_mut() { @@ -991,7 +991,7 @@ impl Api for Server { }; editgroup .db_update(&conn, editgroup_id, submit) - .map(|eg| eg.into_model_partial()) + .map(|eg| eg.into_model_partial(None)) // can't update an accepted editgroup }) .map_err(|e: Error| FatcatError::from(e)) { diff --git a/rust/src/entity_crud.rs b/rust/src/entity_crud.rs index 0b70d417..ce1c1ed7 100644 --- a/rust/src/entity_crud.rs +++ b/rust/src/entity_crud.rs @@ -550,7 +550,7 @@ macro_rules! generic_db_get_history { .map(|(eg_row, cl_row, e_row)| { Ok(EntityHistoryEntry { edit: e_row.into_model()?, - editgroup: eg_row.into_model_partial(), + editgroup: eg_row.into_model_partial(None), changelog_entry: cl_row.into_model(), }) }) -- cgit v1.2.3