Apply some SQLite lessons

src/lz-cli/src/main.rs

@@ -76,9 +76,7 @@ async fn main() {
 
 async fn _main() -> Result<()> {
     let cli = Cli::parse();
-    let pool =
-        sqlx::sqlite::SqlitePool::connect(&format!("sqlite:{}", cli.db.to_string_lossy())).await?;
-    let conn = Connection::from_pool(pool);
+    let conn = Connection::from_path(&cli.db).await?;
     let txn = conn.begin_for_user(&cli.user).await?;
 
     match &cli.command {

src/lz-db/Cargo.toml

@@ -9,6 +9,7 @@ anyhow = { workspace = true }
 serde = { workspace = true, features = ["derive"] }
 tokio = { workspace = true, features = ["tracing", "macros", "rt-multi-thread"] }
 tracing = { workspace = true }
+thiserror = { workspace = true }
 once_cell = { workspace = true }
 indoc = { workspace = true }
 url = { workspace = true, features = ["serde"] }

src/lz-db/migrations/20240225141910_initial_schema.sql

@@ -3,39 +3,39 @@
 CREATE TABLE "users" (
   "user_id" INTEGER NOT NULL PRIMARY KEY,
   "name" TEXT NOT NULL UNIQUE,
-  "created_at" datetime NOT NULL
-);
+  "created_at" TEXT NOT NULL
+) STRICT;
 
 CREATE UNIQUE INDEX "users_by_name" ON "users" ("name");
 
 CREATE TABLE "bookmarks" (
   "bookmark_id" INTEGER NOT NULL PRIMARY KEY,
   "user_id" INTEGER NOT NULL,
-  "created_at" datetime NOT NULL,
-  "modified_at" datetime,
-  "accessed_at" datetime,
+  "created_at" TEXT NOT NULL,
+  "modified_at" TEXT,
+  "accessed_at" TEXT,
   "url" TEXT NOT NULL,
   "title" TEXT NOT NULL,
   "description" TEXT,
   "website_title" TEXT,
   "website_description" TEXT,
   "notes" TEXT,
-  "unread" BOOLEAN,
-  "shared" BOOLEAN,
-  "import_properties" JSON,
+  "unread" INTEGER,
+  "shared" INTEGER,
+  "import_properties" TEXT,
 
   FOREIGN KEY ("user_id") REFERENCES "users"("user_id")
-);
+) STRICT;
 
 CREATE UNIQUE INDEX "bookmarks_by_user_and_url" ON "bookmarks" ("user_id", "url");
 
 CREATE TABLE "tags" (
   "tag_id" INTEGER NOT NULL PRIMARY KEY,
-  "created_at" datetime NOT NULL,
+  "created_at" TEXT NOT NULL,
   "name" TEXT NOT NULL UNIQUE,
 
   CHECK("name" NOT LIKE '% %' AND length("name") >= 1)
-);
+) STRICT;
 
 CREATE UNIQUE INDEX "tags_by_name" ON "tags" ("name");
 
@@ -46,4 +46,4 @@ CREATE TABLE "bookmark_tags" (
   PRIMARY KEY ("bookmark_id","tag_id"),
   FOREIGN KEY("bookmark_id") REFERENCES "bookmarks"("bookmark_id"),
   FOREIGN KEY("tag_id") REFERENCES "tags"("tag_id")
-);
+) STRICT;

src/lz-db/src/connection.rs

@@ -1,11 +1,61 @@
+use std::path::{Path, PathBuf};
+use std::time::Duration;
+
+use sqlx::sqlite::{SqliteConnectOptions, SqliteJournalMode, SqlitePoolOptions, SqliteSynchronous};
+use thiserror::Error;
+
 /// A connection to an sqlite DB holding our bookmark data.
 pub struct Connection {
-    pub(crate) db: sqlx::sqlite::SqlitePool,
+    pub(crate) rw: sqlx::sqlite::SqlitePool,
+    pub(crate) ro: Option<sqlx::sqlite::SqlitePool>,
+}
+
+/// Error establishing sqlite connection pools to a database at a given path.
+#[derive(Error, Debug)]
+#[error("could not open database file {path}")]
+pub struct ConnectionFromPathFailed {
+    path: PathBuf,
+    source: sqlx::Error,
 }
 
 impl Connection {
+    /// Create a database connection to a file on disk.
+    pub async fn from_path(path: &Path) -> Result<Self, ConnectionFromPathFailed> {
+        let options = SqliteConnectOptions::new()
+            .filename(&path)
+            // Options from https://kerkour.com/sqlite-for-servers:
+            .journal_mode(SqliteJournalMode::Wal)
+            .busy_timeout(Duration::from_secs(5))
+            .synchronous(SqliteSynchronous::Normal)
+            .pragma("cache_size", "1000000000")
+            .foreign_keys(true)
+            .pragma("temp_store", "memory")
+            // Some settings that just seem like a good idea:
+            .shared_cache(true)
+            .optimize_on_close(true, None);
+
+        let rw = SqlitePoolOptions::new()
+            .max_connections(1)
+            .connect_with(options.clone())
+            .await
+            .map_err(|source| ConnectionFromPathFailed {
+                path: path.to_owned(),
+                source,
+            })?;
+        let ro = Some(
+            SqlitePoolOptions::new()
+                .connect_with(options.read_only(true))
+                .await
+                .map_err(|source| ConnectionFromPathFailed {
+                    path: path.to_owned(),
+                    source,
+                })?,
+        );
+        Ok(Connection { rw, ro })
+    }
+
     /// Create a database connection from an open SqlitePool.
-    pub fn from_pool(db: sqlx::sqlite::SqlitePool) -> Self {
-        Self { db }
+    pub fn from_pool(rw: sqlx::sqlite::SqlitePool) -> Self {
+        Self { rw, ro: None }
     }
 }

src/lz-db/src/lib.rs

@@ -16,10 +16,11 @@ pub use testing::{Context, NonMigratingContext};
 
 #[cfg(test)]
 mod tests {
-    use crate::{NonMigratingContext, MIGRATOR};
     use test_context::test_context;
     use testresult::TestResult;
 
+    use crate::{NonMigratingContext, MIGRATOR};
+
     #[test_context(NonMigratingContext)]
     #[tokio::test]
     async fn apply_migrations(ctx: &mut NonMigratingContext) -> TestResult {

src/lz-db/src/testing.rs

@@ -35,7 +35,12 @@ impl<M: MigrationBehavior> Context<M> {
 
     /// Returns the SQLite DB pool used in this context
     pub fn db_pool(&mut self) -> &sqlx::SqlitePool {
-        &self.connection.db
+        // TODO: Maybe find a way to test the rw/ro duality.
+        //
+        // The type system thankfully helps us keep the two straight,
+        // but it'd be nice if we could make a test that ensures the
+        // ro pool can see writes made by the rw pool.
+        &self.connection.rw
     }
 }
 

src/lz-db/src/transaction.rs

@@ -1,5 +1,23 @@
+use std::marker::PhantomData;
+
 use crate::Connection;
 
+/// The mode that the transaction is in: Either read-write or read-only.
+///
+/// This is an optimization for reducing timeouts / lock contention on
+/// readonly/read-write txns in a sqlite database, but should also
+/// help with correctness and safety: A readonly transaction does not
+/// have read-write methods defined.
+pub trait TransactionMode {}
+
+/// Read-only transaction mode. See [Transaction].
+pub struct ReadOnly {}
+impl TransactionMode for ReadOnly {}
+
+/// Read-write transaction mode. See [Transaction].
+pub struct ReadWrite {}
+impl TransactionMode for ReadWrite {}
+
 /// A database transaction, operating on the behalf of an `lz` user.
 ///
 /// Transactions are the main way that `lz` code uses the database:
@@ -10,9 +28,23 @@ use crate::Connection;
 /// an HTTP request), the transaction needs to be
 /// [`commit`][Transaction::commit]ed.
 #[derive(Debug)]
-pub struct Transaction {
+pub struct Transaction<M: TransactionMode = ReadWrite> {
     txn: sqlx::Transaction<'static, sqlx::sqlite::Sqlite>,
     user: User<UserId>,
+    marker: PhantomData<M>,
+}
+
+/// An error that can occur when beginning a readonly transaction for a user.
+#[derive(thiserror::Error, Debug)]
+pub enum RoTransactionError {
+    /// Any error raised by sqlx.
+    #[error("sql datastore error")]
+    Sqlx(#[from] sqlx::Error),
+
+    /// User doesn't exist and can not be created due to the read-only
+    /// nature of the connection.
+    #[error("username {user} does not yet exist")]
+    UserNotFound { user: String },
 }
 
 /// # Transactions
@@ -28,19 +60,58 @@ impl Connection {
     ///
     /// In order to commit the changes that happened in the
     /// transaction, call [`Transaction::commit`].
-    pub async fn begin_for_user(&self, username: &str) -> Result<Transaction, sqlx::Error> {
-        let mut txn = self.db.begin().await?;
+    pub async fn begin_for_user(
+        &self,
+        username: &str,
+    ) -> Result<Transaction<ReadWrite>, sqlx::Error> {
+        let mut txn = self.rw.begin().await?;
         let user = Connection::ensure_user(&mut txn, username).await?;
-        Ok(Transaction { txn, user })
+        Ok(Transaction {
+            txn,
+            user,
+            marker: PhantomData,
+        })
+    }
+
+    /// Begin a new read-only transaction as a given user.
+    ///
+    /// If the user with the given name doesn't exist yet, this raises
+    /// an error that the database is opened in read-only mode.
+    ///
+    /// In order to commit the changes that happened in the
+    /// transaction, call [`Transaction::commit`].
+    pub async fn begin_ro_for_user(
+        &self,
+        username: &str,
+    ) -> Result<Transaction<ReadOnly>, RoTransactionError> {
+        let mut txn = if let Some(ro) = &self.ro {
+            ro.begin()
+        } else {
+            self.rw.begin()
+        }
+        .await?;
+        let user = Connection::get_user(&mut txn, username)
+            .await?
+            .ok_or_else(|| RoTransactionError::UserNotFound {
+                user: username.to_string(),
+            })?;
+        Ok(Transaction {
+            txn,
+            user,
+            marker: PhantomData,
+        })
     }
 }
 
-impl Transaction {
+impl Transaction<ReadWrite> {
     /// Commits the transaction.
     pub async fn commit(self) -> Result<(), sqlx::Error> {
         self.txn.commit().await
     }
+}
 
+impl<M: TransactionMode> Transaction<M> {
+    /// Return the user whom the transaction is concerning.
     pub fn user(&self) -> &User<UserId> {
         &self.user
     }