From 1b8e1c067d7ed88cad015c3faaf6b86101d60231 Mon Sep 17 00:00:00 2001 From: "Elf M. Sternberg" Date: Thu, 8 Oct 2020 12:18:08 -0700 Subject: [PATCH] derive_builder pattern is applied successfully. This is mostly an exercise to understand the derive_builder pattern. It required a few tips to get it working, but in the end, it's actually what I want. I also learned a lot about how the Executor pattern, the Results<> object, error mapping, and futures interact in this code. This is going to be incredibly useful long-term, as long as I still keep this project "live" in my head. --- server/nm-store/Cargo.toml | 3 + server/nm-store/src/errors.rs | 2 +- server/nm-store/src/lib.rs | 8 +- server/nm-store/src/row_structs.rs | 42 ++-- .../sql/select_note_collection_from_root.sql | 119 +++++++---- server/nm-store/src/store.rs | 197 ++++++++++-------- 6 files changed, 226 insertions(+), 145 deletions(-) diff --git a/server/nm-store/Cargo.toml b/server/nm-store/Cargo.toml index dacc3a6..d0d9960 100644 --- a/server/nm-store/Cargo.toml +++ b/server/nm-store/Cargo.toml @@ -14,6 +14,9 @@ readme = "./README.org" friendly_id = "0.3.0" thiserror = "1.0.20" derive_builder = "0.9.0" +lazy_static = "1.4.0" +regex = "1.3.9" +slug = "0.1.4" tokio = { version = "0.2.22", features = ["rt-threaded", "blocking"] } serde = { version = "1.0.116", features = ["derive"] } serde_json = "1.0.56" diff --git a/server/nm-store/src/errors.rs b/server/nm-store/src/errors.rs index 76c7247..26031f2 100644 --- a/server/nm-store/src/errors.rs +++ b/server/nm-store/src/errors.rs @@ -13,6 +13,6 @@ pub enum NoteStoreError { NotFound, /// All other errors from the database. - #[error(transparent)] + #[error("Sqlx")] DBError(#[from] sqlx::Error), } diff --git a/server/nm-store/src/lib.rs b/server/nm-store/src/lib.rs index 40a17c4..8f0cb10 100644 --- a/server/nm-store/src/lib.rs +++ b/server/nm-store/src/lib.rs @@ -1,4 +1,3 @@ -use chrono::{DateTime, Utc}; mod errors; mod row_structs; mod store; @@ -9,6 +8,7 @@ pub use crate::store::NoteStore; #[cfg(test)] mod tests { + use chrono; use super::*; use tokio; @@ -38,12 +38,12 @@ mod tests { #[tokio::test(threaded_scheduler)] async fn fetching_unfound_page_by_title_works() { let title = "Nonexistent Page"; - let now = chrono::Utc::now(); + let _now = chrono::Utc::now(); let storagepool = fresh_inmemory_database().await; let newpageresult = storagepool.get_page_by_title(&title).await; - assert!(newpageresult.is_ok(), "{:?}", newpage); - let (newpage, newnotes) = newpageresult.unwrap(); + assert!(newpageresult.is_ok(), "{:?}", newpageresult); + let (newpage, _newnotes) = newpageresult.unwrap(); assert_eq!(newpage.title, title, "{:?}", newpage.title); assert_eq!(newpage.slug, "nonexistent-page"); diff --git a/server/nm-store/src/row_structs.rs b/server/nm-store/src/row_structs.rs index 1ca344d..cab6ca9 100644 --- a/server/nm-store/src/row_structs.rs +++ b/server/nm-store/src/row_structs.rs @@ -1,5 +1,5 @@ use chrono::{DateTime, Utc}; -use derive_builder; +use derive_builder::Builder; use serde::{Deserialize, Serialize}; use sqlx::{self, FromRow}; @@ -32,12 +32,13 @@ pub struct NewPage { pub slug: String, pub title: String, pub note_id: i64, - #[builder(default = "chrono::Utc::now()")] + #[builder(default = r#"chrono::Utc::now()"#)] pub creation_date: DateTime, - #[builder(default = "chrono::Utc::now()")] + #[builder(default = r#"chrono::Utc::now()"#)] pub updated_date: DateTime, - #[builder(default = "chrono::Utc::now()")] + #[builder(default = r#"chrono::Utc::now()"#)] pub lastview_date: DateTime, + #[builder(default = r#"None"#)] pub deleted_date: Option>, } @@ -45,28 +46,43 @@ pub struct NewPage { pub struct NewNote { pub uuid: String, pub content: String, - #[builder(default = "note")] + #[builder(default = r#""note".to_string()"#)] pub notetype: String, - #[builder(default = "chrono::Utc::now()")] + #[builder(default = r#"chrono::Utc::now()"#)] pub creation_date: DateTime, - #[builder(default = "chrono::Utc::now()")] + #[builder(default = r#"chrono::Utc::now()"#)] pub updated_date: DateTime, - #[builder(default = "chrono::Utc::now()")] + #[builder(default = r#"chrono::Utc::now()"#)] pub lastview_date: DateTime, + #[builder(default = r#"None"#)] pub deleted_date: Option>, } +#[derive(Clone, Serialize, Deserialize, Debug, FromRow)] +pub(crate) struct JustSlugs { + pub slug: String, +} + +#[derive(Clone, Serialize, Deserialize, Debug, FromRow)] +pub struct JustTitles { + title: String, +} + #[cfg(test)] mod tests { use super::*; #[test] - fn can_build_new_page() { + fn can_build_new_note() { let now = chrono::Utc::now(); - let newnote = NewNoteBuilder::default(); - assert!((newnote.creation_date - now).num_minutes() < 1.0); - assert!((newnote.updated_date - now).num_minutes() < 1.0); - assert!((newnote.lastview_date - now).num_minutes() < 1.0); + let newnote = NewNoteBuilder::default() + .uuid("foo".to_string()) + .content("bar".to_string()) + .build() + .unwrap(); + assert!((newnote.creation_date - now).num_minutes() < 1); + assert!((newnote.updated_date - now).num_minutes() < 1); + assert!((newnote.lastview_date - now).num_minutes() < 1); assert!(newnote.deleted_date.is_none()); } } diff --git a/server/nm-store/src/sql/select_note_collection_from_root.sql b/server/nm-store/src/sql/select_note_collection_from_root.sql index 96a3c09..40ef0ba 100644 --- a/server/nm-store/src/sql/select_note_collection_from_root.sql +++ b/server/nm-store/src/sql/select_note_collection_from_root.sql @@ -1,38 +1,87 @@ -SELECT parent_uuid, uuid, content, notetype, nature, position FROM ( +-- This is undoubtedly one of the more complex bits of code I've +-- written recently, and I do wish there had been macros because +-- there's a lot of hand-written, copy-pasted code here around the +-- basic content of a note; it would have been nice to be able to DRY +-- that out. - WITH RECURSIVE children( - parent_id, - parent_uuid, id, - uuid, - content, - notetype, - creation_date, - updated_date, - lastview_date, - deleted_date, - cycle - ) AS ( +-- This expression creates a table, 'notetree', that contains all of +-- the notes nested under a page. Each entry in the table includes +-- the note's parent's internal and external ids so that applications +-- can build an actual tree out of a vec of these things. - SELECT - notes.id, - notes.uuid, - notes.id, - notes.uuid, - notes.content, - notes.notetype, 'page', 0, ','||notes.id||',' - FROM notes INNER JOIN pages - ON pages.note_id = notes.id - WHERE pages.id = ? - AND notes.notetype="page" +-- TODO: Extensive testing to validate that the nodes are delivered +-- *in nesting order* to the client. + +SELECT + parent_id, + parent_uuid, + note_id, + note_uuid, + content, + position, + notetype, + creation_date, + updated_date, + lastview_date, + deleted_date + +FROM ( + + WITH RECURSIVE notetree( + parent_id, + parent_uuid, + id, + uuid, + content, + position, + notetype, + creation_date, + updated_date, + lastview_date, + deleted_date, + cycle) AS + +-- ROOT expression + SELECT + notes.id, + notes.uuid, + notes.id, + notes.uuid, + notes.content, + 0, -- Root notes are always in position 0 + notes.notetype, + notes.creation_date, + notes.updated_date, + notes.lastview_date, + notes.deleted_date, + ','||notes.id||',' -- Cycle monitor + FROM notes + INNER JOIN pages ON pages.note_id = notes.id + WHERE + pages.slug = ? AND notes.notetype = "root" + +-- RECURSIVE expression + UNION SELECT + notetree.id, + notetree.uuid, + notes.id, + notes.uuid, + notes.content, + note_relationships.position, + notes.notetype, + notes.creation_date, + notes.updated_date, + notes.lastview_date, + notes.deleted_date, + notetree.cycle||notes.id||',' + FROM notes + INNER JOIN note_relationships ON notes.id = note_relationships.note_id + -- For a given ID in the level of notetree in *this* recursion, + -- we want each note's branches one level down. + INNER JOIN notetree ON note_relationships.parent_id = notetree.id + -- And we want to make sure there are no cycles. There shouldn't + -- be; we're supposed to prevent those. But you never know. + WHERE + notetree.cycle NOT LIKE '%,'||notes.id||',%' + ORDER BY note_relationships.position); - UNION - SELECT note_relationships.parent_id, notes.id, - notes.content, notes.notetype, note_relationships.nature, - note_relationships.position, - children.cycle||notes.id||',' - FROM notes - INNER JOIN note_relationships ON notes.id = note_relationships.note_id - INNER JOIN children ON note_relationships.parent_id = children.id - WHERE children.cycle NOT LIKE '%,'||notes.id||',%' - ORDER BY note_relationships.position) - SELECT * from children); diff --git a/server/nm-store/src/store.rs b/server/nm-store/src/store.rs index 1c6026b..ed399ff 100644 --- a/server/nm-store/src/store.rs +++ b/server/nm-store/src/store.rs @@ -1,11 +1,12 @@ use crate::errors::NoteStoreError; -use crate::row_structs::{RawNote, RawPage}; -use chrono; -use friendly_id; +use crate::row_structs::{JustSlugs, NewNote, NewPage, RawNote, RawPage}; +use lazy_static::lazy_static; +use regex::Regex; +use slug::slugify; use sqlx; use sqlx::{ sqlite::{Sqlite, SqlitePool}, - Done, Executor, + Executor, }; use std::sync::Arc; @@ -26,7 +27,7 @@ impl NoteStore { // to its original empty form. Do not use unless you // really, really want that to happen. pub async fn reset_database(&self) -> NoteResult<()> { - reset_databate(&*self.0).await + reset_database(&*self.0).await.map_err(NoteStoreError::DBError) } /// Fetch page by slug @@ -39,6 +40,7 @@ impl NoteStore { pub async fn get_page_by_slug(&self, slug: &str) -> NoteResult<(RawPage, Vec)> { // let select_note_collection_for_root = include_str!("sql/select_note_collection_for_root.sql"); let mut tx = self.0.begin().await?; + let page = select_page_by_slug(&mut tx, slug).await?; // let notes = sqlx::query_as(select_note_collection_for_root) // .bind(page.note_id) // .fetch(&tx) @@ -49,19 +51,17 @@ impl NoteStore { pub async fn get_page_by_title(&self, title: &str) -> NoteResult<(RawPage, Vec)> { let mut tx = self.0.begin().await?; - let page = match select_page_by_title(&mut tx, title) { - Ok(page) => page, - Err(sqlx::Error::NotFound) => { - match create_page_for_title(&mut tx, title) { - Ok(page) => page, - Err(e) => return Err(e) - } - }, - Err(e) => return Err(e), - }; - // Todo: Replace vec with the results of the CTE - return Ok((page, vec![])) - } + let page = match select_page_by_title(&mut tx, title).await { + Ok(page) => page, + Err(sqlx::Error::RowNotFound) => match create_page_for_title(&mut tx, title).await { + Ok(page) => page, + Err(e) => return Err(NoteStoreError::DBError(e)) + }, + Err(e) => return Err(NoteStoreError::DBError(e)), + }; + // Todo: Replace vec with the results of the CTE + return Ok((page, vec![])); + } } // ___ _ _ @@ -79,14 +79,14 @@ async fn select_page_by_slug<'e, E>(executor: E, slug: &str) -> SqlResult, { - let select_one_page_by_title_sql = concat!( + let select_one_page_by_slug_sql = concat!( "SELECT id, title, slug, note_id, creation_date, updated_date, ", "lastview_date, deleted_date FROM pages WHERE slug=?;" ); - sqlx::query_as(select_one_page_by_slug_sql) + Ok(sqlx::query_as(&select_one_page_by_slug_sql) .bind(&slug) - .fetch_one(&mut executor) - .await? + .fetch_one(executor) + .await?) } async fn select_page_by_title<'e, E>(executor: E, title: &str) -> SqlResult @@ -97,10 +97,10 @@ where "SELECT id, title, slug, note_id, creation_date, updated_date, ", "lastview_date, deleted_date FROM pages WHERE title=?;" ); - sqlx::query_as(select_one_page_by_title_sql) + Ok(sqlx::query_as(&select_one_page_by_title_sql) .bind(&title) - .fetch_one(&mut executor) - .await? + .fetch_one(executor) + .await?) } async fn reset_database<'e, E>(executor: E) -> SqlResult<()> @@ -108,42 +108,63 @@ where E: 'e + Executor<'e, Database = Sqlite>, { let initialize_sql = include_str!("sql/initialize_database.sql"); - sqlx::query(initialize_sql).execute(&*self.0).await? + sqlx::query(initialize_sql).execute(executor).await.map(|_| ()) } -async fn get_note_collection_for_root<'e, E>(executor: E, root: i64) -> SqlResult> +async fn get_note_collection_for_root<'e, E>(executor: E, root: i64) -> SqlResult> where E: 'e + Executor<'e, Database = Sqlite>, { - let select_note_collection_for_root = include_str!("sql/select_note_collection_for_root.sql"); - sqlx::query_as(select_note_collection_for_root) - .fetch(&*self.0) - .await? + let select_note_collection_for_root = include_str!("sql/select_note_collection_from_root.sql"); + Ok(sqlx::query_as(&select_note_collection_for_root) + .bind(&root) + .fetch_all(executor) + .await?) } -async fn insert_one_new_note<'e, E>(executor: E, note: &NewNote) -> SqlResult where +async fn insert_one_new_note<'e, E>(executor: E, note: &NewNote) -> SqlResult +where E: 'e + Executor<'e, Database = Sqlite>, { - let insert_one_note_sql = concat!( - "INSERT INTO notes ( ", - " uuid, ", - " content, ", - " notetype, ", - " creation_date, ", - " updated_date, ", - " lastview_date) ", - "VALUES (?, ?, ?, ?, ?, ?);"); - + let insert_one_note_sql = concat!( + "INSERT INTO notes ( ", + " uuid, ", + " content, ", + " notetype, ", + " creation_date, ", + " updated_date, ", + " lastview_date) ", + "VALUES (?, ?, ?, ?, ?, ?);" + ); + Ok(sqlx::query(insert_one_note_sql) - .bind(¬e.uuid) - .bind(¬e.content) - .bind(¬e.note_type) - .bind(¬e.creation_date) - .bind(¬e.updated_date) - .bind(¬e.lastview_date) - .execute(&mut tx) - .await? - .last_insert_rowid()) + .bind(¬e.uuid) + .bind(¬e.content) + .bind(¬e.notetype) + .bind(¬e.creation_date) + .bind(¬e.updated_date) + .bind(¬e.lastview_date) + .execute(executor) + .await? + .last_insert_rowid()) +} + +fn find_maximal_slug(slugs: &Vec) -> Option { + lazy_static! { + static ref RE_CAP_NUM: Regex = Regex::new(r"-(\d+)$").unwrap(); + } + + if slugs.len() == 0 { + return None; + } + + let mut slug_counters: Vec = slugs + .iter() + .filter_map(|slug| RE_CAP_NUM.captures(&slug.slug)) + .map(|cap| cap.get(1).unwrap().as_str().parse::().unwrap()) + .collect(); + slug_counters.sort_unstable(); + slug_counters.pop() } // Given an initial string and an existing collection of slugs, @@ -156,28 +177,19 @@ where lazy_static! { static ref RE_STRIP_NUM: Regex = Regex::new(r"-\d+$").unwrap(); } - lazy_static! { - static ref RE_CAP_NUM: Regex = Regex::new(r"-(\d+)$").unwrap(); - } - let initial_slug = slugify::slugify(title); - let sample_slug = RE_STRIP_NUM.replace_all(slug, ""); - let similar_slugs: Vec = sqlx::query("SELECT slug FROM pages WHERE slug LIKE '?%';") - .bind(&sample_slug) - .execute(executor) - .await?; - let slug_counters = similar_slugs - .iter() - .map(|slug| RE_CAPNUM.captures(slug.slug)) - .filter_map(|cap| cap.get(1).unwrap().parse::().unwrap()) - .collect(); - match slug_counters.len() { - 0 => Ok(initial_slug), - _ => { - slug_counters.sort_unstable(); - return Ok(format!("{}-{}", initial_slug, slug_counters.pop() + 1)) - } - } + let initial_slug = slugify(title); + let sample_slug = RE_STRIP_NUM.replace_all(&initial_slug, ""); + let slug_finder_sql = "SELECT slug FROM pages WHERE slug LIKE '?%';"; + let similar_slugs: Vec = sqlx::query_as(&slug_finder_sql) + .bind(&*sample_slug) + .fetch_all(executor) + .await?; + let maximal_slug = find_maximal_slug(&similar_slugs); + match maximal_slug { + None => Ok(initial_slug), + Some(max_slug) => Ok(format!("{}-{}", initial_slug, max_slug + 1)), + } } async fn insert_one_new_page<'e, E>(executor: E, page: &NewPage) -> SqlResult @@ -185,30 +197,31 @@ where E: 'e + Executor<'e, Database = Sqlite>, { let insert_one_page_sql = concat!( - "INSERT INTO pages ( ", - " slug, ", - " title, ", - " note_id, ", - " creation_date, ", - " updated_date, ", - " lastview_date) ", - "VALUES (?, ?, ?, ?, ?, ?);"); + "INSERT INTO pages ( ", + " slug, ", + " title, ", + " note_id, ", + " creation_date, ", + " updated_date, ", + " lastview_date) ", + "VALUES (?, ?, ?, ?, ?, ?);" + ); Ok(sqlx::query(insert_one_page_sql) - .bind(&page.slug) - .bind(&page.title) - .bind(&page.note_id) - .bind(&page.creation_date) - .bind(&page.updated_date) - .bind(&page.lastview_date) - .execute(&mut tx) - .await? - .last_insert_rowid()) + .bind(&page.slug) + .bind(&page.title) + .bind(&page.note_id) + .bind(&page.creation_date) + .bind(&page.updated_date) + .bind(&page.lastview_date) + .execute(executor) + .await? + .last_insert_rowid()) } - -async fn create_page_for_title<'e, E>(executor: E, title: &str) -> SqlResult where +async fn create_page_for_title<'e, E>(_executor: E, _title: &str) -> SqlResult +where E: 'e + Executor<'e, Database = Sqlite>, { - todo!() + todo!() }