From 013ca18c6223bb052f03f7d300c2b0562a9fdede Mon Sep 17 00:00:00 2001 From: "Elf M. Sternberg" Date: Thu, 12 Nov 2020 13:33:27 -0800 Subject: [PATCH] As cool as the ParentId/NoteId thing was, it didn't feel zero-abstraction, and it was starting to prove to be clutter. Maybe it's a mistake to downtype them to a common type, but I don't think there was that much risk here. --- server/nm-store/src/store/private.rs | 107 ++++++++++++++------------- server/nm-store/src/store/store.rs | 36 +++------ 2 files changed, 66 insertions(+), 77 deletions(-) diff --git a/server/nm-store/src/store/private.rs b/server/nm-store/src/store/private.rs index 18ae54d..559eb38 100644 --- a/server/nm-store/src/store/private.rs +++ b/server/nm-store/src/store/private.rs @@ -35,8 +35,9 @@ lazy_static! { ); } + lazy_static! { - static ref SELECT_NOTES_BACKREFENCING_PAGE_SQL: &'static str = + static ref SELECT_NOTES_BACKREFERENCING_PAGE_SQL: &'static str = include_str!("sql/select_notes_backreferencing_page.sql"); } @@ -63,6 +64,20 @@ where // |_|\___|\__\__|_||_| |_|\_\__,_/__/\__\___|_||_| // +// The next three functions are essentially the same, although the internal +// SQL operations are quite different between the first two and the last. +async fn select_object_by_query<'a, E>(executor: E, query: &str, field: &str) -> SqlResult> +where + E: Executor<'a, Database = Sqlite>, +{ + let r: Vec = sqlx::query_as(query) + .bind(field) + .fetch_all(executor) + .await?; + Ok(r.into_iter().map(|z| Note::from(z)).collect()) +} + + // Select the requested page via its id. This is fairly rare; // pages should usually be picked up via their title, but if you're // navigating to an instance, this is how you specify the page in a @@ -72,15 +87,11 @@ where // // Recommended: Clients should update the URL whenever changing // page. -pub(crate) async fn select_page_by_slug<'a, E>(executor: E, slug: &NoteId) -> SqlResult> +pub(crate) async fn select_page_by_slug<'a, E>(executor: E, slug: &str) -> SqlResult> where E: Executor<'a, Database = Sqlite>, { - let r: Vec = sqlx::query_as(&SELECT_PAGE_BY_ID_SQL) - .bind(&**slug) - .fetch_all(executor) - .await?; - Ok(r.into_iter().map(|z| Note::from(z)).collect()) + select_object_by_query(executor, &SELECT_PAGE_BY_ID_SQL, &slug).await } // Fetch the page by title. The return value is an array of Note @@ -90,11 +101,7 @@ pub(crate) async fn select_page_by_title<'a, E>(executor: E, title: &str) -> Sql where E: Executor<'a, Database = Sqlite>, { - let r: Vec = sqlx::query_as(&SELECT_PAGE_BY_TITLE_SQL) - .bind(&title) - .fetch_all(executor) - .await?; - Ok(r.into_iter().map(|z| Note::from(z)).collect()) + select_object_by_query(executor, &SELECT_PAGE_BY_TITLE_SQL, &title).await } // Fetch all backreferences to a page. The return value is an array @@ -103,16 +110,12 @@ where // they want to display that collection. pub(crate) async fn select_backreferences_for_page<'a, E>( executor: E, - page_id: &NoteId, + page_id: &str, ) -> SqlResult> where E: Executor<'a, Database = Sqlite>, { - let r: Vec = sqlx::query_as(&SELECT_NOTES_BACKREFENCING_PAGE_SQL) - .bind(&**page_id) - .fetch_all(executor) - .await?; - Ok(r.into_iter().map(|z| Note::from(z)).collect()) + select_object_by_query(executor, &SELECT_NOTES_BACKREFERENCING_PAGE_SQL, &page_id).await } // ___ _ ___ _ _ _ @@ -275,8 +278,8 @@ where pub(crate) async fn select_note_to_note_relationship<'a, E>( executor: E, - parent_id: &ParentId, - note_id: &NoteId, + parent_id: &str, + note_id: &str, ) -> SqlResult where E: Executor<'a, Database = Sqlite>, @@ -288,8 +291,8 @@ where "LIMIT 1" ); let s: NoteRelationshipRow = sqlx::query_as(get_note_to_note_relationship_sql) - .bind(&**parent_id) - .bind(&**note_id) + .bind(parent_id) + .bind(note_id) .fetch_one(executor) .await?; Ok(NoteRelationship::from(s)) @@ -303,8 +306,8 @@ where pub(crate) async fn insert_note_to_note_relationship<'a, E>( executor: E, - parent_id: &ParentId, - note_id: &NoteId, + parent_id: &str, + note_id: &str, location: i64, kind: &RelationshipKind, ) -> SqlResult<()> @@ -317,10 +320,10 @@ where ); let _ = sqlx::query(insert_note_to_note_relationship_sql) - .bind(&**parent_id) - .bind(&**note_id) + .bind(parent_id) + .bind(note_id) .bind(&location) - .bind(&kind.to_string()) + .bind(kind.to_string()) .execute(executor) .await?; Ok(()) @@ -328,7 +331,7 @@ where pub(crate) async fn make_room_for_new_note<'a, E>( executor: E, - parent_id: &ParentId, + parent_id: &str, location: i64, ) -> SqlResult<()> where @@ -342,7 +345,7 @@ where let _ = sqlx::query(make_room_for_new_note_sql) .bind(&location) - .bind(&**parent_id) + .bind(parent_id) .execute(executor) .await?; Ok(()) @@ -350,7 +353,7 @@ where pub(crate) async fn determine_max_child_location_for_note<'a, E>( executor: E, - note_id: &ParentId, + note_id: &str, comp_loc: Option, ) -> SqlResult where @@ -365,7 +368,7 @@ where pub(crate) async fn assert_max_child_location_for_note<'a, E>( executor: E, - note_id: &ParentId, + note_id: &str, ) -> SqlResult where E: Executor<'a, Database = Sqlite>, @@ -374,7 +377,7 @@ where "SELECT MAX(location) AS count FROM note_relationships WHERE parent_id = ?;"; let count: RowCount = sqlx::query_as(assert_max_child_location_for_note_sql) - .bind(&**note_id) + .bind(note_id) .fetch_one(executor) .await?; @@ -389,8 +392,8 @@ where pub(crate) async fn insert_bulk_note_to_page_relationships<'a, E>( executor: E, - note_id: &NoteId, - references: &[NoteId], + note_id: &str, + references: &[String], ) -> SqlResult<()> where E: Executor<'a, Database = Sqlite>, @@ -408,7 +411,7 @@ where let mut request = sqlx::query(&insert_note_page_references_sql); for reference in references { - request = request.bind(&**note_id).bind(&**reference); + request = request.bind(note_id).bind(reference); } request.execute(executor).await.map(|_| ()) @@ -416,7 +419,7 @@ where pub(crate) async fn delete_bulk_note_to_page_relationships<'a, E>( executor: E, - note_id: &NoteId, + note_id: &str, ) -> SqlResult<()> where E: Executor<'a, Database = Sqlite>, @@ -424,7 +427,7 @@ where let delete_note_to_page_relationship_sql = "DELETE FROM note_page_relationships WHERE and note_id = ?;"; let _ = sqlx::query(delete_note_to_page_relationship_sql) - .bind(&**note_id) + .bind(note_id) .execute(executor) .await?; Ok(()) @@ -487,8 +490,8 @@ where pub(crate) async fn delete_note_to_note_relationship<'a, E>( executor: E, - parent_id: &ParentId, - note_id: &NoteId, + parent_id: &str, + note_id: &str, ) -> SqlResult<()> where E: Executor<'a, Database = Sqlite>, @@ -499,8 +502,8 @@ where ); let count = sqlx::query(delete_note_to_note_relationship_sql) - .bind(&**parent_id) - .bind(&**note_id) + .bind(parent_id) + .bind(note_id) .execute(executor) .await? .rows_affected(); @@ -513,7 +516,7 @@ where pub(crate) async fn delete_note_to_page_relationships<'a, E>( executor: E, - note_id: &NoteId, + note_id: &str, ) -> SqlResult<()> where E: Executor<'a, Database = Sqlite>, @@ -527,20 +530,20 @@ where } let _ = sqlx::query(&DELETE_NOTE_TO_PAGE_RELATIONSHIPS_SQL) - .bind(&**note_id) + .bind(note_id) .execute(executor) .await?; Ok(()) } -pub(crate) async fn delete_note<'a, E>(executor: E, note_id: &NoteId) -> SqlResult<()> +pub(crate) async fn delete_note<'a, E>(executor: E, note_id: &str) -> SqlResult<()> where E: Executor<'a, Database = Sqlite>, { let delete_note_sql = "DELETE FROM notes WHERE note_id = ?"; let count = sqlx::query(delete_note_sql) - .bind(&**note_id) + .bind(note_id) .execute(executor) .await? .rows_affected(); @@ -556,7 +559,7 @@ where // sequential. pub(crate) async fn close_hole_for_deleted_note<'a, E>( executor: E, - parent_id: &ParentId, + parent_id: &str, location: i64, ) -> SqlResult<()> where @@ -570,7 +573,7 @@ where let _ = sqlx::query(close_hole_for_deleted_note_sql) .bind(&location) - .bind(&**parent_id) + .bind(parent_id) .execute(executor) .await?; Ok(()) @@ -584,7 +587,7 @@ where pub(crate) async fn validate_or_generate_all_found_references( txi: &mut Transaction<'_, Sqlite>, references: &[String] -) -> SqlResult> { +) -> SqlResult> { let mut tx = txi.begin().await?; let found_references = @@ -597,8 +600,8 @@ pub(crate) async fn validate_or_generate_all_found_references( } let _ = bulk_insert_notes(&mut tx, &new_page).await?; - let mut all_reference_ids: Vec = found_references.iter().map(|r| NoteId(r.id.clone())).collect(); - all_reference_ids.append(&mut new_page.iter().map(|r| NoteId(r.id.clone())).collect()); + let mut all_reference_ids: Vec = found_references.iter().map(|r| r.id.clone()).collect(); + all_reference_ids.append(&mut new_page.iter().map(|r| r.id.clone()).collect()); tx.commit().await?; Ok(all_reference_ids) } @@ -611,14 +614,14 @@ pub(crate) async fn validate_or_generate_all_found_references( // The dreaded miscellaneous! -pub(crate) async fn count_existing_note_relationships<'a, E>(executor: E, note_id: &NoteId) -> SqlResult +pub(crate) async fn count_existing_note_relationships<'a, E>(executor: E, note_id: &str) -> SqlResult where E: Executor<'a, Database = Sqlite>, { let count_existing_note_relationships_sql = "SELECT COUNT(*) as count FROM note_relationships WHERE note_id = ?;"; let count: RowCount = sqlx::query_as(&count_existing_note_relationships_sql) - .bind(&**note_id) + .bind(note_id) .fetch_one(executor) .await?; Ok(count.count) diff --git a/server/nm-store/src/store/store.rs b/server/nm-store/src/store/store.rs index 3232096..d936f00 100644 --- a/server/nm-store/src/store/store.rs +++ b/server/nm-store/src/store/store.rs @@ -93,20 +93,18 @@ impl NoteStore { /// this use case says that in the event of a failure to find the /// requested page, return a basic NotFound. pub async fn get_page_by_slug(&self, slug: &str) -> NoteResult<(Vec, Vec)> { - let page = select_page_by_slug(&*self.0, &NoteId(slug.to_string())).await?; + let page = select_page_by_slug(&*self.0, slug).await?; if page.is_empty() { return Err(NoteStoreError::NotFound); } - let note_id = NoteId(page[0].id.clone()); - Ok(( - page, - select_backreferences_for_page(&*self.0, ¬e_id).await?, - )) + let note_id = &page[0].id; + let backreferences = select_backreferences_for_page(&*self.0, ¬e_id).await?; + Ok((page, backreferences)) } /// Fetch page by title - + /// /// The most common use case: the user is navigating by requesting /// a page. The page either exists or it doesn't. If it /// doesn't, we go out and make it. Since we know it doesn't exist, @@ -119,11 +117,9 @@ impl NoteStore { let page = select_page_by_title(&*self.0, title).await?; if page.len() > 0 { - let note_id = NoteId(page[0].id.clone()); - return Ok(( - page, - select_backreferences_for_page(&*self.0, ¬e_id).await?, - )); + let note_id = &page[0].id; + let backreferences = select_backreferences_for_page(&*self.0, ¬e_id).await?; + return Ok((page, backreferences)); } // Sanity check! @@ -149,14 +145,8 @@ impl NoteStore { parent_id: &str, location: Option, ) -> NoteResult { - let new_id = self - .insert_note( - note, - &ParentId(parent_id.to_string()), - location, - RelationshipKind::Direct, - ) - .await?; + let kind = RelationshipKind::Direct; + let new_id = self.insert_note(note, parent_id, location, kind).await?; Ok(new_id) } @@ -170,10 +160,6 @@ impl NoteStore { ) -> NoteResult<()> { let mut tx = self.0.begin().await?; - let old_parent_id = ParentId(old_parent_id.to_string()); - let new_parent_id = ParentId(new_parent_id.to_string()); - let note_id = NoteId(note_id.to_string()); - let old_note = select_note_to_note_relationship(&mut tx, &old_parent_id, ¬e_id).await?; let old_note_location = old_note.location; let old_note_kind = old_note.kind; @@ -239,7 +225,7 @@ impl NoteStore { async fn insert_note( &self, note: &NewNote, - parent_id: &ParentId, + parent_id: &str, location: Option, kind: RelationshipKind, ) -> NoteResult {