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/editing_crud.rs | 159 +++++++++++++++++++++++++++++++++++------------ 1 file changed, 119 insertions(+), 40 deletions(-) (limited to 'rust/src/editing_crud.rs') 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) } -- cgit v1.2.3