Merge pull request #3 from frostu8/lock-page-on-update

impl page lock on update

src/error.rs

@@ -19,6 +19,8 @@ pub const BAD_AUTHORIZATION: u32 = 1004;
 pub const NOT_FOUND: u32 = 1005;
 /// Edits to this page have been protected.
 pub const EDITS_FORBIDDEN: u32 = 1006;
+/// The page was edited while editing.
+pub const PAGE_ALREADY_CHANGED: u32 = 1007;
 
 /// The main API error type.
 #[derive(Clone, Debug, Deserialize, Serialize)]
@@ -45,6 +47,7 @@ impl Error {
             BAD_AUTHORIZATION => "bad authorization, suggest: clear cache",
             NOT_FOUND => "not found",
             EDITS_FORBIDDEN => "this page is protected",
+            PAGE_ALREADY_CHANGED => "other changes were made to this page; reload",
             _ => "unknown error",
         };
 

src/page/edit.rs

@@ -12,11 +12,19 @@ use serde::{Deserialize, Serialize};
 pub struct PageSource {
     /// The source content of the page.
     pub source: String,
+    /// The latest change of the hash.
+    ///
+    /// Used to protect against concurrent accesses. In the future, a better
+    /// system might be implemented.
+    pub latest_change_hash: Option<String>,
 }
 
 impl Default for PageSource {
     fn default() -> Self {
-        PageSource { source: "".into() }
+        PageSource {
+            source: "".into(),
+            latest_change_hash: None,
+        }
     }
 }
 
@@ -30,72 +38,93 @@ pub async fn get_page_source(path: String) -> Result<PageSource, ServerFnError<A
     // edits MUST be attributed
     let _token = extract_token().await?;
 
-    let content = get_page_content(&path, &state.pool)
+    let page = get_page_content(&path, &state.pool)
         .await
         .map_err(|e| ServerFnError::ServerError(e.to_string()))?;
 
-    if let Some(content) = content {
-        Ok(PageSource { source: content })
+    if let Some(page) = page {
+        Ok(PageSource {
+            source: page.content,
+            latest_change_hash: Some(page.latest_change_hash),
+        })
     } else {
         // user is trying to create page
         // send default page source as if it did exist
         Ok(PageSource::default())
     }
 }
 
+/// Result for [`push_page_changes`]
+#[derive(Clone, Debug, Deserialize, Serialize)]
+pub struct ChangeResult {
+    /// The hash of the change.
+    ///
+    /// May be `None` if the page did not change.
+    pub hash: Option<String>,
+}
+
 /// Creates or updates a new page.
 #[server(endpoint = "/page/source")]
 pub async fn push_page_changes(
     path: String,
+    latest_change_hash: String,
     source: String,
-) -> Result<(), ServerFnError<ApiError>> {
+) -> Result<ChangeResult, ServerFnError<ApiError>> {
     use crate::{
         account::extract_token,
-        schema::page::{get_page_content, save_change, update_page_content},
+        error,
+        schema::page::{get_page_content_for_update, save_change, update_page_content},
         ServerState,
     };
     use diff_match_patch_rs::{Compat, DiffMatchPatch, PatchInput};
 
-    let state = expect_context::<ServerState>();
-
     // attribute edits on the given token
     let token = extract_token().await?;
 
-    let old_source = get_page_content(&path, &state.pool)
+    // Begin transaction for reading things from the db.
+    let mut tx = {
+        // keep state in this scope to prevent database access
+        let state = expect_context::<ServerState>();
+        state
+            .pool
+            .begin()
+            .await
+            .map_err(|e| ServerFnError::ServerError(format!("{:?}", e)))?
+    };
+
+    let old_page = get_page_content_for_update(&path, &mut *tx)
         .await
-        .map_err(|e| ServerFnError::ServerError(e.to_string()))?
-        .unwrap_or_else(String::new);
+        .map_err(|e| ServerFnError::ServerError(e.to_string()))?;
 
-    if old_source == source {
+    if old_page.as_ref().map(|c| &c.latest_change_hash) != Some(&latest_change_hash) {
+        return Err(ApiError::from_code(error::PAGE_ALREADY_CHANGED).into());
+    }
+
+    if old_page.as_ref().map(|c| &c.content) == Some(&source) {
         // bail early if the two texts are the exact same
-        return Ok(());
+        return Ok(ChangeResult { hash: None });
     }
 
+    let old_source = old_page.as_ref().map(|c| c.content.as_str()).unwrap_or("");
+
     // do page diffing
     let dmp = DiffMatchPatch::new();
 
     let diffs = dmp
-        .diff_main::<Compat>(&old_source, &source)
+        .diff_main::<Compat>(old_source, &source)
         .map_err(|e| ServerFnError::ServerError(format!("{:?}", e)))?;
     let patches = dmp
         .patch_make(PatchInput::new_diffs(&diffs))
         .map_err(|e| ServerFnError::ServerError(format!("{:?}", e)))?;
     let changes = dmp.patch_to_text(&patches);
 
-    // save changes
-    let mut tx = state
-        .pool
-        .begin()
-        .await
-        .map_err(|e| ServerFnError::ServerError(format!("{:?}", e)))?;
-
     // make update to page content
     update_page_content(&path, &source, &mut *tx)
         .await
         .map_err(|e| ServerFnError::ServerError(format!("{:?}", e)))?;
 
     // add change to db
-    save_change(&path, &token.sub, &changes, &mut *tx)
+    let hash = save_change(&path, &token.sub, &changes, &mut *tx)
         .await
         .map_err(|e| ServerFnError::ServerError(format!("{:?}", e)))?;
 
@@ -104,5 +133,5 @@ pub async fn push_page_changes(
         .await
         .map_err(|e| ServerFnError::ServerError(format!("{:?}", e)))?;
 
-    Ok(())
+    Ok(ChangeResult { hash: Some(hash) })
 }

src/page/editor.rs

@@ -6,7 +6,7 @@ use wasm_bindgen::prelude::*;
 
 use web_sys::HtmlTextAreaElement;
 
-use crate::page::edit::PushPageChanges;
+use crate::page::edit::{PageSource, PushPageChanges};
 
 /// The page editor.
 ///
@@ -15,25 +15,49 @@ use crate::page::edit::PushPageChanges;
 /// `initial_content`, which means the content of the page may change, but the
 /// initial content will not.
 #[component]
-pub fn PageEditor(path: Signal<String>, initial_content: String) -> impl IntoView {
+pub fn PageEditor(path: Signal<String>, page: PageSource) -> impl IntoView {
     let textarea_ref = NodeRef::<Textarea>::new();
 
+    let (last_hash, set_last_hash) = signal(page.latest_change_hash);
+
     let push_page_changes = ServerAction::<PushPageChanges>::new();
 
+    let err_msg = move || {
+        let result = push_page_changes.value().get();
+
+        match result {
+            Some(Err(err)) => err.to_string(),
+            _ => unreachable!(),
+        }
+    };
+
+    // CodeMirror support, calls into the Inferno ext JS.
     Effect::new(move || {
         if let Some(node) = textarea_ref.get() {
             upgrade_editor(node);
         }
     });
 
+    // For making updates.
+    Effect::new(move || {
+        if let Some(Ok(change)) = push_page_changes.value().get() {
+            set_last_hash(change.hash);
+        }
+    });
+
     view! {
         <ActionForm attr:class="editor" action=push_page_changes>
+            // TODO: error modals
+            <Show when=move || push_page_changes.value().with(|c| matches!(c, Some(Err(_))))>
+                <p>{err_msg}</p>
+            </Show>
             <div class="page-admin">
                 <input type="submit" value="Save Changes" />
             </div>
             <textarea node_ref=textarea_ref id="page-source" name="source" rows="40">
-                {initial_content}
+                {page.source}
             </textarea>
+            <input type="hidden" name="latest_change_hash" value=last_hash />
             <input type="hidden" name="path" value=path />
         </ActionForm>
     }

src/page/mod.rs

@@ -143,7 +143,7 @@ pub fn EditPage() -> impl IntoView {
     let page_suspense = move || {
         Suspend::new(async move {
             match page.await {
-                Ok(page) => view! { <PageEditor path initial_content=page.source /> }.into_any(),
+                Ok(page) => view! { <PageEditor path page /> }.into_any(),
                 // TODO better 500 pages
                 Err(_) => view! { "error" }.into_any(),
             }

src/page/render.rs

@@ -38,19 +38,19 @@ pub async fn render_page(path: String) -> Result<RenderedPage, ServerFnError<Api
     let token = extract_token().await;
 
     // get page
-    let content = get_page_content(&path, &state.pool)
+    let page = get_page_content(&path, &state.pool)
         .await
         .map_err(|e| ServerFnError::ServerError(e.to_string()))?;
 
-    if let Some(content) = content {
+    if let Some(page) = page {
         let title = match path.rfind('/') {
             Some(idx) => path[idx + 1..].trim(),
             None => path.trim(),
         };
 
-        let parser = Parser::new(&content);
+        let parser = Parser::new(&page.content);
 
-        let mut html_output = String::with_capacity(content.len() * 3 / 2);
+        let mut html_output = String::with_capacity(page.content.len() * 3 / 2);
         html::push_html(&mut html_output, parser);
 
         // sanitize html

src/schema/page.rs

@@ -2,23 +2,62 @@
 
 use base16::encode_lower;
 use chrono::Utc;
-use sqlx::{Executor, PgPool, Postgres};
+use sqlx::{Executor, Postgres};
 
 use sha2::{Digest, Sha256};
 
-/// Gets the content of a page, returning it as a [`String`].
-pub async fn get_page_content(path: &str, db: &PgPool) -> Result<Option<String>, sqlx::Error> {
-    #[derive(sqlx::FromRow)]
-    struct Page {
-        pub content: String,
-    }
+/// Result of [`get_page_content`] and [`get_page_for_update`].
+#[derive(sqlx::FromRow)]
+pub struct Page {
+    pub content: String,
+    pub latest_change_hash: String,
+}
 
-    // get page
-    sqlx::query_as::<_, Page>("SELECT content FROM pages WHERE path = $1")
-        .bind(path)
-        .fetch_optional(db)
-        .await
-        .map(|result| result.map(|Page { content }| content))
+/// Gets the content of a page.
+pub async fn get_page_content<'c, E>(path: &str, db: E) -> Result<Option<Page>, sqlx::Error>
+where
+    E: Executor<'c, Database = Postgres>,
+{
+    sqlx::query_as(
+        r#"
+        SELECT p.content, c.hash AS latest_change_hash
+        FROM pages p
+        RIGHT JOIN changes c ON c.page_id = p.id
+        WHERE path = $1
+        ORDER BY c.inserted_at DESC
+        LIMIT 1
+        "#,
+    )
+    .bind(path)
+    .fetch_optional(db)
+    .await
+}
+
+/// Gets the content of a page for an update.
+///
+/// This function sets up a lock for an update, as opposed to
+/// [`get_page_content`]. If you just want the page, use [`get_page_content`].
+pub async fn get_page_content_for_update<'c, E>(
+    path: &str,
+    db: E,
+) -> Result<Option<Page>, sqlx::Error>
+where
+    E: Executor<'c, Database = Postgres>,
+{
+    sqlx::query_as(
+        r#"
+        SELECT p.content, c.hash AS latest_change_hash
+        FROM pages p
+        RIGHT JOIN changes c ON c.page_id = p.id
+        WHERE path = $1
+        ORDER BY c.inserted_at DESC
+        LIMIT 1
+        FOR UPDATE
+        "#,
+    )
+    .bind(path)
+    .fetch_optional(db)
+    .await
 }
 
 /// Updates the page content. Inserts a new page if it did not exist.
@@ -49,7 +88,7 @@ where
     .map(|_| ())
 }
 
-/// Saves a new change to the database.
+/// Saves a new change to the database. Returns the change hash.
 ///
 /// This does not actually modify the page; this function should typically be
 /// called in conjunction with [`update_page_content`].
@@ -58,7 +97,7 @@ pub async fn save_change<'c, E>(
     author: &str,
     changes: &str,
     db: E,
-) -> Result<(), sqlx::Error>
+) -> Result<String, sqlx::Error>
 where
     E: Executor<'c, Database = Postgres>,
 {
@@ -95,5 +134,5 @@ where
     .bind(inserted_at)
     .execute(db)
     .await
-    .map(|_| ())
+    .map(|_| hash)
 }