From 62ab4b79f977e063d3da2dc4b61ad55cfa68d425 Mon Sep 17 00:00:00 2001 From: Ryan Levick Date: Thu, 13 Apr 2023 16:11:55 +0200 Subject: [PATCH 01/16] Add sqlite host component scaffolding Signed-off-by: Ryan Levick --- Cargo.lock | 13 ++++++ crates/sqlite/Cargo.toml | 12 +++++ crates/sqlite/src/host_component.rs | 33 ++++++++++++++ crates/sqlite/src/lib.rs | 54 +++++++++++++++++++++++ crates/trigger/Cargo.toml | 1 + crates/trigger/src/key_value.rs | 65 ++++++++++++++++++++++++++++ crates/trigger/src/lib.rs | 5 +++ crates/trigger/src/runtime_config.rs | 15 +++++++ crates/trigger/src/sqlite.rs | 27 ++++++++++++ wit/preview2/reactor.wit | 1 + wit/preview2/sqlite.wit | 23 ++++++++++ 11 files changed, 249 insertions(+) create mode 100644 crates/sqlite/Cargo.toml create mode 100644 crates/sqlite/src/host_component.rs create mode 100644 crates/sqlite/src/lib.rs create mode 100644 crates/trigger/src/key_value.rs create mode 100644 crates/trigger/src/sqlite.rs create mode 100644 wit/preview2/sqlite.wit diff --git a/Cargo.lock b/Cargo.lock index abd7f00319..b725f9b385 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5421,6 +5421,18 @@ dependencies = [ "wit-bindgen-rust", ] +[[package]] +name = "spin-sqlite" +version = "0.1.0" +dependencies = [ + "anyhow", + "rand 0.8.5", + "rusqlite", + "spin-app", + "spin-core", + "wit-bindgen-wasmtime", +] + [[package]] name = "spin-templates" version = "1.3.0-pre0" @@ -5505,6 +5517,7 @@ dependencies = [ "spin-key-value-sqlite", "spin-loader", "spin-manifest", + "spin-sqlite", "tempfile", "tokio", "toml 0.5.11", diff --git a/crates/sqlite/Cargo.toml b/crates/sqlite/Cargo.toml new file mode 100644 index 0000000000..d61e2ec52d --- /dev/null +++ b/crates/sqlite/Cargo.toml @@ -0,0 +1,12 @@ +[package] +name = "spin-sqlite" +version = "0.1.0" +edition = "2021" + +[dependencies] +spin-core = { path = "../core" } +spin-app = { path = "../app" } +anyhow = "1.0" +wit-bindgen-wasmtime = { workspace = true } +rusqlite = { version = "0.27.0", features = [ "bundled" ] } +rand = "0.8" \ No newline at end of file diff --git a/crates/sqlite/src/host_component.rs b/crates/sqlite/src/host_component.rs new file mode 100644 index 0000000000..3294ffe432 --- /dev/null +++ b/crates/sqlite/src/host_component.rs @@ -0,0 +1,33 @@ +use spin_app::{AppComponent, DynamicHostComponent}; +use spin_core::{sqlite, HostComponent}; + +use crate::SqliteImpl; + +pub struct SqliteComponent; + +impl SqliteComponent { + pub fn new() -> Self { + Self + } +} + +impl HostComponent for SqliteComponent { + type Data = super::SqliteImpl; + + fn add_to_linker( + linker: &mut spin_core::Linker, + get: impl Fn(&mut spin_core::Data) -> &mut Self::Data + Send + Sync + Copy + 'static, + ) -> anyhow::Result<()> { + sqlite::add_to_linker(linker, get) + } + + fn build_data(&self) -> Self::Data { + SqliteImpl::new() + } +} + +impl DynamicHostComponent for SqliteComponent { + fn update_data(&self, _data: &mut Self::Data, _component: &AppComponent) -> anyhow::Result<()> { + Ok(()) + } +} diff --git a/crates/sqlite/src/lib.rs b/crates/sqlite/src/lib.rs new file mode 100644 index 0000000000..2540b27f03 --- /dev/null +++ b/crates/sqlite/src/lib.rs @@ -0,0 +1,54 @@ +mod host_component; + +use std::collections::HashMap; + +pub use host_component::SqliteComponent; +use rand::Rng; +use spin_core::{ + async_trait, + sqlite::{self, Host}, +}; + +pub struct SqliteImpl { + connections: HashMap, +} + +impl SqliteImpl { + pub fn new() -> Self { + Self { + connections: HashMap::default(), + } + } + + pub fn component_init(&mut self) {} +} + +#[async_trait] +impl Host for SqliteImpl { + async fn open( + &mut self, + name: String, + ) -> anyhow::Result> { + let conn = rusqlite::Connection::open_in_memory()?; + let mut rng = rand::thread_rng(); + let c: sqlite::Connection = rng.gen(); + self.connections.insert(c, conn); + Ok(Ok(c)) + } + + async fn execute( + &mut self, + connection: sqlite::Connection, + query: String, + ) -> anyhow::Result> { + if let Some(c) = self.connections.get(&connection) { + c.execute(&query, []); + } + Ok(Ok(())) + } + + async fn close(&mut self, connection: spin_core::sqlite::Connection) -> anyhow::Result<()> { + let _ = self.connections.remove(&connection); + Ok(()) + } +} diff --git a/crates/trigger/Cargo.toml b/crates/trigger/Cargo.toml index 8dc4713600..ee730df36d 100644 --- a/crates/trigger/Cargo.toml +++ b/crates/trigger/Cargo.toml @@ -21,6 +21,7 @@ spin-key-value = { path = "../key-value" } spin-key-value-azure = { path = "../key-value-azure" } spin-key-value-redis = { path = "../key-value-redis" } spin-key-value-sqlite = { path = "../key-value-sqlite" } +spin-sqlite = { path = "../sqlite" } sanitize-filename = "0.4" serde = "1.0" serde_json = "1.0" diff --git a/crates/trigger/src/key_value.rs b/crates/trigger/src/key_value.rs new file mode 100644 index 0000000000..70c4298f2a --- /dev/null +++ b/crates/trigger/src/key_value.rs @@ -0,0 +1,65 @@ +use std::{path::Path, sync::Arc}; + +use anyhow::{Context, Result}; +use spin_key_value::{ + CachingStoreManager, DelegatingStoreManager, KeyValueComponent, KEY_VALUE_STORES_KEY, +}; +use spin_key_value_sqlite::{DatabaseLocation, KeyValueSqlite}; + +use crate::{runtime_config::RuntimeConfig, TriggerHooks}; + +// TODO: Once we have runtime configuration for key-value stores, the user will be able +// to both change the default store configuration (e.g. use Redis, or an SQLite +// in-memory database, or use a different path) and add other named stores with their +// own configurations. + +pub(crate) fn build_key_value_component( + runtime_config: &RuntimeConfig, +) -> Result { + let location = match runtime_config.key_value_sqlite_db_path() { + Some(path) => { + // Create the store's parent directory if necessary + create_parent_dir(&path).context("Failed to create key value store")?; + DatabaseLocation::Path(path) + } + None => DatabaseLocation::InMemory, + }; + + let manager = Arc::new(CachingStoreManager::new(DelegatingStoreManager::new([( + "default".to_owned(), + Arc::new(KeyValueSqlite::new(location)) as _, + )]))); + + Ok(KeyValueComponent::new(spin_key_value::manager(move |_| { + manager.clone() + }))) +} + +fn create_parent_dir(path: &Path) -> Result<()> { + let dir = path + .parent() + .with_context(|| format!("{path:?} missing parent dir"))?; + std::fs::create_dir_all(dir).with_context(|| format!("Failed to create parent dir {dir:?}")) +} + +pub struct KeyValuePersistenceMessageHook; + +impl TriggerHooks for KeyValuePersistenceMessageHook { + fn app_loaded(&mut self, app: &spin_app::App, runtime_config: &RuntimeConfig) -> Result<()> { + // Don't print anything if the app doesn't use KV + if app.components().all(|c| { + c.get_metadata(KEY_VALUE_STORES_KEY) + .unwrap_or_default() + .unwrap_or_default() + .is_empty() + }) { + return Ok(()); + } + if let Some(path) = runtime_config.key_value_sqlite_db_path() { + println!("Storing key-value data to {path:?}"); + } else { + println!("Using in-memory key-value store; data will not be saved!"); + } + Ok(()) + } +} diff --git a/crates/trigger/src/lib.rs b/crates/trigger/src/lib.rs index d742a89555..507184bdd2 100644 --- a/crates/trigger/src/lib.rs +++ b/crates/trigger/src/lib.rs @@ -2,6 +2,7 @@ pub mod cli; pub mod loader; pub mod locked; mod runtime_config; +mod sqlite; mod stdio; use std::{collections::HashMap, marker::PhantomData, path::PathBuf}; @@ -123,6 +124,10 @@ impl TriggerExecutorBuilder { ) .await?, )?; + self.loader.add_dynamic_host_component( + &mut builder, + crate::sqlite::build_component(&runtime_config)?, + )?; self.loader.add_dynamic_host_component( &mut builder, outbound_http::OutboundHttpComponent, diff --git a/crates/trigger/src/runtime_config.rs b/crates/trigger/src/runtime_config.rs index 34ccc81785..7d1283ca19 100644 --- a/crates/trigger/src/runtime_config.rs +++ b/crates/trigger/src/runtime_config.rs @@ -18,6 +18,8 @@ use self::{ pub const DEFAULT_STATE_DIR: &str = ".spin"; const DEFAULT_LOGS_DIR: &str = "logs"; +const DEFAULT_SQLITE_DB_FILENAME: &str = "sqlite.db"; + /// RuntimeConfig allows multiple sources of runtime configuration to be /// queried uniformly. #[derive(Debug, Default)] @@ -131,6 +133,16 @@ impl RuntimeConfig { } } + /// Return a path to the sqlite DB used for key value storage if set. + pub fn sqlite_db_path(&self) -> Option { + if let Some(state_dir) = self.state_dir() { + // If the state dir is set, build the default path + Some(state_dir.join(DEFAULT_SQLITE_DB_FILENAME)) + } else { + None + } + } + /// Returns an iterator of RuntimeConfigOpts in order of decreasing precedence fn opts_layers(&self) -> impl Iterator { std::iter::once(&self.overrides).chain(self.files.iter().rev()) @@ -193,6 +205,7 @@ mod tests { assert_eq!(config.state_dir(), None); assert_eq!(config.log_dir(), None); assert_eq!(default_spin_store_path(&config), None); + assert_eq!(config.key_value_sqlite_db_path(), None); Ok(()) } @@ -210,6 +223,8 @@ mod tests { let default_db_path = default_spin_store_path(&config).unwrap(); assert!(default_db_path.starts_with(&state_dir)); + let sqlite_db_path = config.key_value_sqlite_db_path().unwrap(); + assert!(sqlite_db_path.starts_with(&state_dir)); Ok(()) } diff --git a/crates/trigger/src/sqlite.rs b/crates/trigger/src/sqlite.rs new file mode 100644 index 0000000000..24af419ca1 --- /dev/null +++ b/crates/trigger/src/sqlite.rs @@ -0,0 +1,27 @@ +use std::path::Path; + +use crate::runtime_config::RuntimeConfig; +use anyhow::Context; +use spin_key_value_sqlite::DatabaseLocation; +use spin_sqlite::SqliteComponent; + +// TODO: debup with the stuff in key_value +pub(crate) fn build_component(runtime_config: &RuntimeConfig) -> anyhow::Result { + let location = match runtime_config.sqlite_db_path() { + Some(path) => { + // Create the store's parent directory if necessary + create_parent_dir(&path).context("Failed to create sqlite db")?; + DatabaseLocation::Path(path) + } + None => DatabaseLocation::InMemory, + }; + + Ok(SqliteComponent::new()) +} + +fn create_parent_dir(path: &Path) -> anyhow::Result<()> { + let dir = path + .parent() + .with_context(|| format!("{path:?} missing parent dir"))?; + std::fs::create_dir_all(dir).with_context(|| format!("Failed to create parent dir {dir:?}")) +} diff --git a/wit/preview2/reactor.wit b/wit/preview2/reactor.wit index 954aa38a98..042c1da4b8 100644 --- a/wit/preview2/reactor.wit +++ b/wit/preview2/reactor.wit @@ -2,6 +2,7 @@ default world reactor { import config: pkg.config import postgres: pkg.postgres import mysql: pkg.mysql + import sqlite: pkg.sqlite import redis: pkg.redis import key-value: pkg.key-value import http: pkg.http diff --git a/wit/preview2/sqlite.wit b/wit/preview2/sqlite.wit new file mode 100644 index 0000000000..f801780353 --- /dev/null +++ b/wit/preview2/sqlite.wit @@ -0,0 +1,23 @@ + +default interface sqlite { + // A handle to an open sqlite instance + type connection = u32 + + // The set of errors which may be raised by functions in this interface + variant error { + // Some implementation-specific error has occurred (e.g. I/O) + io(string) + } + + // Open a connection to a named instance. + // + // If `name` is "default", the default instance is opened. + // + // `error::no-such-store` will be raised if the `name` is not recognized. + open: func(name: string) -> result + + execute: func(conn: connection, query: string) -> result<_, error> + + // Close the specified `connection`. + close: func(conn: connection) +} From 18771eaa0ea370d0dc9f802f02965c4c0024ff1e Mon Sep 17 00:00:00 2001 From: Ryan Levick Date: Thu, 13 Apr 2023 17:53:14 +0200 Subject: [PATCH 02/16] Hack support into the rust sdk Signed-off-by: Ryan Levick --- Cargo.toml | 2 +- crates/sqlite/src/lib.rs | 4 ++++ crates/trigger/src/lib.rs | 11 +++++++++-- sdk/rust/src/lib.rs | 3 +++ sdk/rust/src/sqlite.rs | 31 +++++++++++++++++++++++++++++++ wit/ephemeral/sqlite.wit | 20 ++++++++++++++++++++ 6 files changed, 68 insertions(+), 3 deletions(-) create mode 100644 sdk/rust/src/sqlite.rs create mode 100644 wit/ephemeral/sqlite.wit diff --git a/Cargo.toml b/Cargo.toml index 47782f23cc..fe24e4d58b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -115,7 +115,7 @@ tracing = { version = "0.1", features = ["log"] } wasmtime-wasi = { version = "8.0.1", features = ["tokio"] } wasi-common-preview1 = { package = "wasi-common", version = "8.0.1" } wasmtime = { version = "8.0.1", features = ["component-model"] } -spin-componentize = { git = "https://github.com/fermyon/spin-componentize", rev = "51c3fade751c4e364142719e42130943fd8b0a76" } +spin-componentize = { git = "https://github.com/fermyon/spin-componentize", rev = "acdd2d6d233eb71d1f64dbd237aeb5e17eb21ffb" } wasi-host = { package = "host", git = "https://github.com/fermyon/spin-componentize", rev = "51c3fade751c4e364142719e42130943fd8b0a76" } wasi-common = { git = "https://github.com/fermyon/spin-componentize", rev = "51c3fade751c4e364142719e42130943fd8b0a76" } wasi-cap-std-sync = { git = "https://github.com/fermyon/spin-componentize", rev = "51c3fade751c4e364142719e42130943fd8b0a76" } diff --git a/crates/sqlite/src/lib.rs b/crates/sqlite/src/lib.rs index 2540b27f03..0b28e6ebf7 100644 --- a/crates/sqlite/src/lib.rs +++ b/crates/sqlite/src/lib.rs @@ -29,6 +29,7 @@ impl Host for SqliteImpl { &mut self, name: String, ) -> anyhow::Result> { + println!("Opening.."); let conn = rusqlite::Connection::open_in_memory()?; let mut rng = rand::thread_rng(); let c: sqlite::Connection = rng.gen(); @@ -43,6 +44,9 @@ impl Host for SqliteImpl { ) -> anyhow::Result> { if let Some(c) = self.connections.get(&connection) { c.execute(&query, []); + let s: Result = + c.query_row("SELECT COUNT(name) FROM person;", [], |row| row.get(0)); + println!("{s:?}"); } Ok(Ok(())) } diff --git a/crates/trigger/src/lib.rs b/crates/trigger/src/lib.rs index 507184bdd2..89d7b34412 100644 --- a/crates/trigger/src/lib.rs +++ b/crates/trigger/src/lib.rs @@ -340,8 +340,15 @@ pub fn decode_preinstantiation_error(e: anyhow::Error) -> anyhow::Error { if err_text.contains("unknown import") && err_text.contains("has not been defined") { // TODO: how to maintain this list? - let sdk_imported_interfaces = - &["config", "http", "key-value", "mysql", "postgres", "redis"]; + let sdk_imported_interfaces = &[ + "config", + "http", + "key-value", + "mysql", + "postgres", + "redis", + "sqlite", + ]; if sdk_imported_interfaces .iter() diff --git a/sdk/rust/src/lib.rs b/sdk/rust/src/lib.rs index 248dbcba5f..02927d60bc 100644 --- a/sdk/rust/src/lib.rs +++ b/sdk/rust/src/lib.rs @@ -8,6 +8,9 @@ pub mod outbound_http; /// Key/Value storage. pub mod key_value; +/// Sqlite +pub mod sqlite; + /// Exports the procedural macros for writing handlers for Spin components. pub use spin_macro::*; diff --git a/sdk/rust/src/sqlite.rs b/sdk/rust/src/sqlite.rs new file mode 100644 index 0000000000..99dc60d85f --- /dev/null +++ b/sdk/rust/src/sqlite.rs @@ -0,0 +1,31 @@ +wit_bindgen_rust::import!("../../wit/ephemeral/sqlite.wit"); + +use sqlite::Connection as RawConnection; + +/// Errors which may be raised by the methods of `Store` +pub type Error = sqlite::Error; + +/// Represents a store in which key value tuples may be placed +#[derive(Debug)] +pub struct Connection(RawConnection); + +impl Connection { + /// Open a connection + pub fn open() -> Result { + Ok(Self(sqlite::open("foo")?)) + } + + /// + pub fn execute(&self, query: &str) -> Result<(), Error> { + sqlite::execute(self.0, query)?; + Ok(()) + } +} + +impl std::fmt::Display for Error { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{self:?}") + } +} + +impl std::error::Error for Error {} diff --git a/wit/ephemeral/sqlite.wit b/wit/ephemeral/sqlite.wit new file mode 100644 index 0000000000..80de7fa518 --- /dev/null +++ b/wit/ephemeral/sqlite.wit @@ -0,0 +1,20 @@ +// A handle to an open sqlite instance +type connection = u32 + +// The set of errors which may be raised by functions in this interface +variant error { + // Some implementation-specific error has occurred (e.g. I/O) + io(string) +} + +// Open a connection to a named instance. +// +// If `name` is "default", the default instance is opened. +// +// `error::no-such-store` will be raised if the `name` is not recognized. +open: func(name: string) -> expected + +execute: func(conn: connection, query: string) -> expected + +// Close the specified `connection`. +close: func(conn: connection) From 9ff3ed4d059b153ab80589477f056754c57d385b Mon Sep 17 00:00:00 2001 From: Ryan Levick Date: Fri, 14 Apr 2023 21:02:33 +0200 Subject: [PATCH 03/16] Make queries work Signed-off-by: Ryan Levick --- crates/sqlite/src/lib.rs | 76 ++++++++++++++++++++++++++++++++++++---- sdk/rust/src/sqlite.rs | 29 +++++++++++++-- wit/ephemeral/sqlite.wit | 33 ++++++++++++++++- wit/preview2/sqlite.wit | 34 ++++++++++++++++-- 4 files changed, 159 insertions(+), 13 deletions(-) diff --git a/crates/sqlite/src/lib.rs b/crates/sqlite/src/lib.rs index 0b28e6ebf7..4cc360390d 100644 --- a/crates/sqlite/src/lib.rs +++ b/crates/sqlite/src/lib.rs @@ -11,16 +11,33 @@ use spin_core::{ pub struct SqliteImpl { connections: HashMap, + statements: HashMap)>, } impl SqliteImpl { pub fn new() -> Self { Self { connections: HashMap::default(), + statements: HashMap::default(), } } pub fn component_init(&mut self) {} + + fn find_statement( + &mut self, + connection: sqlite::Connection, + statement: sqlite::Statement, + ) -> Result<(&mut rusqlite::Connection, (&str, &[String])), sqlite::Error> { + match ( + self.connections.get_mut(&connection), + self.statements.get(&statement), + ) { + (Some(c), Some((s, a))) => Ok((c, (s, a))), + (Some(_), None) => todo!(), + (None, _) => todo!(), + } + } } #[async_trait] @@ -40,19 +57,64 @@ impl Host for SqliteImpl { async fn execute( &mut self, connection: sqlite::Connection, - query: String, + statement: sqlite::Statement, ) -> anyhow::Result> { - if let Some(c) = self.connections.get(&connection) { - c.execute(&query, []); - let s: Result = - c.query_row("SELECT COUNT(name) FROM person;", [], |row| row.get(0)); - println!("{s:?}"); - } + let (c, (s, a)) = self.find_statement(connection, statement)?; + c.execute(s, rusqlite::params_from_iter(a.iter())) + .map_err(|e| sqlite::Error::Io(e.to_string()))?; Ok(Ok(())) } + async fn query( + &mut self, + connection: sqlite::Connection, + query: sqlite::Statement, + ) -> anyhow::Result, sqlite::Error>> { + let (c, (q, a)) = self.find_statement(connection, query)?; + let mut statement = c.prepare(q).map_err(|e| sqlite::Error::Io(e.to_string()))?; + let rows = statement + .query_map(rusqlite::params_from_iter(a.iter()), |row| { + let mut values = vec![]; + for column in 0.. { + let value = match row.get_ref(column) { + Ok(rusqlite::types::ValueRef::Null) => sqlite::DataType::Null, + Ok(rusqlite::types::ValueRef::Integer(i)) => sqlite::DataType::Int64(i), + Ok(rusqlite::types::ValueRef::Real(f)) => sqlite::DataType::Double(f), + Ok(rusqlite::types::ValueRef::Text(t)) => { + sqlite::DataType::Str(String::from_utf8(t.to_vec()).unwrap()) + } + Ok(rusqlite::types::ValueRef::Blob(b)) => { + sqlite::DataType::Binary(b.to_vec()) + } + Err(rusqlite::Error::InvalidColumnIndex(_)) => break, + _ => todo!(), + }; + values.push(value); + } + Ok(sqlite::Row { values }) + }) + .map_err(|e| sqlite::Error::Io(e.to_string()))?; + Ok(Ok(rows.collect::>()?)) + } + async fn close(&mut self, connection: spin_core::sqlite::Connection) -> anyhow::Result<()> { let _ = self.connections.remove(&connection); Ok(()) } + + async fn prepare_statement( + &mut self, + statement: String, + params: Vec, + ) -> anyhow::Result> { + let mut rng = rand::thread_rng(); + let s: sqlite::Statement = rng.gen(); + self.statements.insert(s, (statement, params)); + Ok(Ok(s)) + } + + async fn drop_statement(&mut self, statement: sqlite::Statement) -> anyhow::Result<()> { + self.statements.remove(&statement); + Ok(()) + } } diff --git a/sdk/rust/src/sqlite.rs b/sdk/rust/src/sqlite.rs index 99dc60d85f..6cfbcc8401 100644 --- a/sdk/rust/src/sqlite.rs +++ b/sdk/rust/src/sqlite.rs @@ -15,9 +15,15 @@ impl Connection { Ok(Self(sqlite::open("foo")?)) } - /// - pub fn execute(&self, query: &str) -> Result<(), Error> { - sqlite::execute(self.0, query)?; + /// Make a query against the database + pub fn query(&self, statement: &Statement) -> Result, Error> { + sqlite::query(self.0, statement.0) + } + + /// Execute a statement against the database + pub fn execute(&self, statement: &str) -> Result<(), Error> { + let statement = Statement::prepare(statement, &[])?; + sqlite::execute(self.0, statement.0)?; Ok(()) } } @@ -29,3 +35,20 @@ impl std::fmt::Display for Error { } impl std::error::Error for Error {} + +/// A prepared statement +pub struct Statement(sqlite::Statement); + +impl Statement { + /// Prepare a statement + pub fn prepare(query: &str, params: &[&str]) -> Result { + let statement = sqlite::prepare_statement(query, params)?; + Ok(Statement(statement)) + } +} + +impl Drop for Statement { + fn drop(&mut self) { + sqlite::drop_statement(self.0); + } +} diff --git a/wit/ephemeral/sqlite.wit b/wit/ephemeral/sqlite.wit index 80de7fa518..12eb458ec4 100644 --- a/wit/ephemeral/sqlite.wit +++ b/wit/ephemeral/sqlite.wit @@ -14,7 +14,38 @@ variant error { // `error::no-such-store` will be raised if the `name` is not recognized. open: func(name: string) -> expected -execute: func(conn: connection, query: string) -> expected +// allows parameterized queries +type statement = u32 +drop-statement: func(statement: statement) +prepare-statement: func(query: string, params: list) -> expected + +// Execute a statement +execute: func(conn: connection, statement: statement) -> expected + +// Query data +query: func(conn: connection, q: statement) -> expected, error> // Close the specified `connection`. close: func(conn: connection) + +// A database row +record row { + values: list, +} + +// Common data types +variant data-type { + int32(s32), + int64(s64), + uint32(u32), + uint64(u64), + float(float64), + double(float64), + str(string), + boolean(bool), + date(string), + time(string), + timestamp(string), + binary(list), + null +} diff --git a/wit/preview2/sqlite.wit b/wit/preview2/sqlite.wit index f801780353..322899ad49 100644 --- a/wit/preview2/sqlite.wit +++ b/wit/preview2/sqlite.wit @@ -1,4 +1,3 @@ - default interface sqlite { // A handle to an open sqlite instance type connection = u32 @@ -16,8 +15,39 @@ default interface sqlite { // `error::no-such-store` will be raised if the `name` is not recognized. open: func(name: string) -> result - execute: func(conn: connection, query: string) -> result<_, error> + // Execute a statement + execute: func(conn: connection, statement: statement) -> result<_, error> + + // Query data + query: func(conn: connection, query: statement) -> result, error> // Close the specified `connection`. close: func(conn: connection) + + // allows parameterized queries + type statement = u32 + prepare-statement: func(query: string, params: list) -> result + drop-statement: func(s: statement) + + // A database row + record row { + values: list, + } + + // Common data types + variant data-type { + int32(s32), + int64(s64), + uint32(u32), + uint64(u64), + float(float64), + double(float64), + str(string), + boolean(bool), + date(string), + time(string), + timestamp(string), + binary(list), + null + } } From bc2edfde62891cf3c00933787b06333209f77ecb Mon Sep 17 00:00:00 2001 From: Ryan Levick Date: Sun, 16 Apr 2023 11:18:58 +0200 Subject: [PATCH 04/16] Allow using database file Signed-off-by: Ryan Levick --- crates/sqlite/src/host_component.rs | 18 ++++++++++++++---- crates/sqlite/src/lib.rs | 15 ++++++++++++--- crates/trigger/src/sqlite.rs | 7 +++---- 3 files changed, 29 insertions(+), 11 deletions(-) diff --git a/crates/sqlite/src/host_component.rs b/crates/sqlite/src/host_component.rs index 3294ffe432..7cbecf00a7 100644 --- a/crates/sqlite/src/host_component.rs +++ b/crates/sqlite/src/host_component.rs @@ -1,13 +1,23 @@ +use std::path::PathBuf; + use spin_app::{AppComponent, DynamicHostComponent}; use spin_core::{sqlite, HostComponent}; use crate::SqliteImpl; -pub struct SqliteComponent; +#[derive(Debug, Clone)] +pub enum DatabaseLocation { + InMemory, + Path(PathBuf), +} + +pub struct SqliteComponent { + location: DatabaseLocation, +} impl SqliteComponent { - pub fn new() -> Self { - Self + pub fn new(location: DatabaseLocation) -> Self { + Self { location } } } @@ -22,7 +32,7 @@ impl HostComponent for SqliteComponent { } fn build_data(&self) -> Self::Data { - SqliteImpl::new() + SqliteImpl::new(self.location.clone()) } } diff --git a/crates/sqlite/src/lib.rs b/crates/sqlite/src/lib.rs index 4cc360390d..ee497d8fcf 100644 --- a/crates/sqlite/src/lib.rs +++ b/crates/sqlite/src/lib.rs @@ -2,21 +2,25 @@ mod host_component; use std::collections::HashMap; -pub use host_component::SqliteComponent; use rand::Rng; use spin_core::{ async_trait, sqlite::{self, Host}, }; +pub use host_component::DatabaseLocation; +pub use host_component::SqliteComponent; + pub struct SqliteImpl { + location: DatabaseLocation, connections: HashMap, statements: HashMap)>, } impl SqliteImpl { - pub fn new() -> Self { + pub fn new(location: DatabaseLocation) -> Self { Self { + location, connections: HashMap::default(), statements: HashMap::default(), } @@ -47,7 +51,12 @@ impl Host for SqliteImpl { name: String, ) -> anyhow::Result> { println!("Opening.."); - let conn = rusqlite::Connection::open_in_memory()?; + let conn = match &self.location { + DatabaseLocation::InMemory => rusqlite::Connection::open_in_memory()?, + DatabaseLocation::Path(p) => rusqlite::Connection::open(p)?, + }; + + // TODO: this is not the best way to do this... let mut rng = rand::thread_rng(); let c: sqlite::Connection = rng.gen(); self.connections.insert(c, conn); diff --git a/crates/trigger/src/sqlite.rs b/crates/trigger/src/sqlite.rs index 24af419ca1..dc85900aa2 100644 --- a/crates/trigger/src/sqlite.rs +++ b/crates/trigger/src/sqlite.rs @@ -2,10 +2,9 @@ use std::path::Path; use crate::runtime_config::RuntimeConfig; use anyhow::Context; -use spin_key_value_sqlite::DatabaseLocation; -use spin_sqlite::SqliteComponent; +use spin_sqlite::{DatabaseLocation, SqliteComponent}; -// TODO: debup with the stuff in key_value +// TODO: dedup with the stuff in key_value pub(crate) fn build_component(runtime_config: &RuntimeConfig) -> anyhow::Result { let location = match runtime_config.sqlite_db_path() { Some(path) => { @@ -16,7 +15,7 @@ pub(crate) fn build_component(runtime_config: &RuntimeConfig) -> anyhow::Result< None => DatabaseLocation::InMemory, }; - Ok(SqliteComponent::new()) + Ok(SqliteComponent::new(location)) } fn create_parent_dir(path: &Path) -> anyhow::Result<()> { From cfac9e77d13ceacfe3e334363c2388d2c1238b53 Mon Sep 17 00:00:00 2001 From: Ryan Levick Date: Sun, 16 Apr 2023 15:47:30 +0200 Subject: [PATCH 05/16] Params as data enum not strings Signed-off-by: Ryan Levick --- crates/sqlite/src/lib.rs | 33 ++++++++++++++++++++++----------- sdk/rust/src/sqlite.rs | 10 +++++++++- wit/ephemeral/sqlite.wit | 18 +++++------------- wit/preview2/sqlite.wit | 20 ++++++-------------- 4 files changed, 42 insertions(+), 39 deletions(-) diff --git a/crates/sqlite/src/lib.rs b/crates/sqlite/src/lib.rs index ee497d8fcf..0f24b1692a 100644 --- a/crates/sqlite/src/lib.rs +++ b/crates/sqlite/src/lib.rs @@ -14,7 +14,7 @@ pub use host_component::SqliteComponent; pub struct SqliteImpl { location: DatabaseLocation, connections: HashMap, - statements: HashMap)>, + statements: HashMap)>, } impl SqliteImpl { @@ -32,7 +32,7 @@ impl SqliteImpl { &mut self, connection: sqlite::Connection, statement: sqlite::Statement, - ) -> Result<(&mut rusqlite::Connection, (&str, &[String])), sqlite::Error> { + ) -> Result<(&mut rusqlite::Connection, (&str, &[sqlite::DataType])), sqlite::Error> { match ( self.connections.get_mut(&connection), self.statements.get(&statement), @@ -48,9 +48,8 @@ impl SqliteImpl { impl Host for SqliteImpl { async fn open( &mut self, - name: String, + _name: String, ) -> anyhow::Result> { - println!("Opening.."); let conn = match &self.location { DatabaseLocation::InMemory => rusqlite::Connection::open_in_memory()?, DatabaseLocation::Path(p) => rusqlite::Connection::open(p)?, @@ -69,7 +68,7 @@ impl Host for SqliteImpl { statement: sqlite::Statement, ) -> anyhow::Result> { let (c, (s, a)) = self.find_statement(connection, statement)?; - c.execute(s, rusqlite::params_from_iter(a.iter())) + c.execute(s, rusqlite::params_from_iter(convert_data(a.iter()))) .map_err(|e| sqlite::Error::Io(e.to_string()))?; Ok(Ok(())) } @@ -82,18 +81,18 @@ impl Host for SqliteImpl { let (c, (q, a)) = self.find_statement(connection, query)?; let mut statement = c.prepare(q).map_err(|e| sqlite::Error::Io(e.to_string()))?; let rows = statement - .query_map(rusqlite::params_from_iter(a.iter()), |row| { + .query_map(rusqlite::params_from_iter(convert_data(a.iter())), |row| { let mut values = vec![]; for column in 0.. { let value = match row.get_ref(column) { Ok(rusqlite::types::ValueRef::Null) => sqlite::DataType::Null, - Ok(rusqlite::types::ValueRef::Integer(i)) => sqlite::DataType::Int64(i), - Ok(rusqlite::types::ValueRef::Real(f)) => sqlite::DataType::Double(f), + Ok(rusqlite::types::ValueRef::Integer(i)) => sqlite::DataType::Integer(i), + Ok(rusqlite::types::ValueRef::Real(f)) => sqlite::DataType::Real(f), Ok(rusqlite::types::ValueRef::Text(t)) => { - sqlite::DataType::Str(String::from_utf8(t.to_vec()).unwrap()) + sqlite::DataType::Text(String::from_utf8(t.to_vec()).unwrap()) } Ok(rusqlite::types::ValueRef::Blob(b)) => { - sqlite::DataType::Binary(b.to_vec()) + sqlite::DataType::Blob(b.to_vec()) } Err(rusqlite::Error::InvalidColumnIndex(_)) => break, _ => todo!(), @@ -114,7 +113,7 @@ impl Host for SqliteImpl { async fn prepare_statement( &mut self, statement: String, - params: Vec, + params: Vec, ) -> anyhow::Result> { let mut rng = rand::thread_rng(); let s: sqlite::Statement = rng.gen(); @@ -127,3 +126,15 @@ impl Host for SqliteImpl { Ok(()) } } + +fn convert_data<'a>( + arguments: impl Iterator + 'a, +) -> impl Iterator + 'a { + arguments.map(|a| match a { + sqlite::DataType::Null => rusqlite::types::Value::Null, + sqlite::DataType::Integer(i) => rusqlite::types::Value::Integer(*i), + sqlite::DataType::Real(r) => rusqlite::types::Value::Real(*r), + sqlite::DataType::Text(t) => rusqlite::types::Value::Text(t.clone()), + sqlite::DataType::Blob(b) => rusqlite::types::Value::Blob(b.clone()), + }) +} diff --git a/sdk/rust/src/sqlite.rs b/sdk/rust/src/sqlite.rs index 6cfbcc8401..bf2967f83c 100644 --- a/sdk/rust/src/sqlite.rs +++ b/sdk/rust/src/sqlite.rs @@ -5,6 +5,14 @@ use sqlite::Connection as RawConnection; /// Errors which may be raised by the methods of `Store` pub type Error = sqlite::Error; +/// +pub type Row = sqlite::Row; + +/// +pub type DataTypeParam<'a> = sqlite::DataTypeParam<'a>; +/// +pub type DataTypeResult = sqlite::DataTypeResult; + /// Represents a store in which key value tuples may be placed #[derive(Debug)] pub struct Connection(RawConnection); @@ -41,7 +49,7 @@ pub struct Statement(sqlite::Statement); impl Statement { /// Prepare a statement - pub fn prepare(query: &str, params: &[&str]) -> Result { + pub fn prepare(query: &str, params: &[DataTypeParam]) -> Result { let statement = sqlite::prepare_statement(query, params)?; Ok(Statement(statement)) } diff --git a/wit/ephemeral/sqlite.wit b/wit/ephemeral/sqlite.wit index 12eb458ec4..9564b3514c 100644 --- a/wit/ephemeral/sqlite.wit +++ b/wit/ephemeral/sqlite.wit @@ -17,7 +17,7 @@ open: func(name: string) -> expected // allows parameterized queries type statement = u32 drop-statement: func(statement: statement) -prepare-statement: func(query: string, params: list) -> expected +prepare-statement: func(query: string, params: list) -> expected // Execute a statement execute: func(conn: connection, statement: statement) -> expected @@ -35,17 +35,9 @@ record row { // Common data types variant data-type { - int32(s32), - int64(s64), - uint32(u32), - uint64(u64), - float(float64), - double(float64), - str(string), - boolean(bool), - date(string), - time(string), - timestamp(string), - binary(list), + integer(s64), + real(float64), + text(string), + blob(list), null } diff --git a/wit/preview2/sqlite.wit b/wit/preview2/sqlite.wit index 322899ad49..59bdd9f1bd 100644 --- a/wit/preview2/sqlite.wit +++ b/wit/preview2/sqlite.wit @@ -26,28 +26,20 @@ default interface sqlite { // allows parameterized queries type statement = u32 - prepare-statement: func(query: string, params: list) -> result + prepare-statement: func(query: string, params: list) -> result drop-statement: func(s: statement) // A database row record row { values: list, } - + // Common data types variant data-type { - int32(s32), - int64(s64), - uint32(u32), - uint64(u64), - float(float64), - double(float64), - str(string), - boolean(bool), - date(string), - time(string), - timestamp(string), - binary(list), + integer(s64), + real(float64), + text(string), + blob(list), null } } From 7d12d4ae048e54ad2a777c3957a927c122a574fc Mon Sep 17 00:00:00 2001 From: Ryan Levick Date: Sun, 16 Apr 2023 18:26:33 +0200 Subject: [PATCH 06/16] sqlite sip draft Signed-off-by: Ryan Levick --- docs/content/sips/000-sqlite.md | 79 +++++++++++++++++++++++++++++++++ 1 file changed, 79 insertions(+) create mode 100644 docs/content/sips/000-sqlite.md diff --git a/docs/content/sips/000-sqlite.md b/docs/content/sips/000-sqlite.md new file mode 100644 index 0000000000..b4f062d9c4 --- /dev/null +++ b/docs/content/sips/000-sqlite.md @@ -0,0 +1,79 @@ +title = "SIP 000 - Sqlite" +template = "main" +date = "2023-04-17:00:00Z" +--- + +Summary: Provide a generic interface for access to a sqlite databases + +Owner(s): ryan.levick@fermyon.com + +Created: Apr 17, 2023 + +## Background + +Spin currently supports two database types: mysql and [postgres](https://developer.fermyon.com/cloud/data-postgres) which both require the user to provide their own database that is exposed to users through the SDK. + +In contrast to the these other interfaces, the sqlite implementation would easily allow local spin deployment to use a local sqlite database file, and it provides those hosting spin deployment envionments (e.g., Fermyon Cloud) to implement lightweight sqlite implementations. + +### What about `wasi-sql`? + +[`wasi-sql`](https://github.com/WebAssembly/wasi-sql) is a work-in-progress spec for a generic SQL interface that aims to support "the features commonly used by 80% of user application". It is likely that when `wasi-sql` is more mature users will be able to successfully use functionality based on the `wasi-sql` interface to interact with the sqlite databases. However, there are still reasons that a dedicated sqlite interface is still useful: + +* For the 20% of use cases where `wasi-sql` is too generic a dedicated `sqlite` interface can provide that functionality. +* The `wasi-sql` spec is under active investigation, and there are large questions about how to best support such a wide breadth of sql flavors. This implementation can help clarify those questions and push upstream work further along. + +## Proposal + +In order to support sqlite, the following need to be added to Spin: + +- A `WIT` file that defines the sqlite interface +- SDK implementations for various programming languages +- A default local sqlite store (note: spin already uses sqlite for the KV implementation so this should be trivial) +- Potentially runtime configuration for configuring how sqlite is provisioned. + +### Key-Value Interface (`.wit`) + +We will start with the `wasi-sql` interface but deliberately change that interface as to better match sqlite semantics. This will ensure that we're not simply implementing early versions of the `wasi-sql` interface while still having good answers for why the interface differs when it does. + +Like `wasi-sql` and the key-value store, we model resources such as database connections as pseudo-[resource handles](https://github.com/WebAssembly/component-model/blob/main/design/mvp/WIT.md#item-resource) which may be created using an `open` function and disposed using a `close` function. Each operation on a connection is a function which accepts a handle as its first parameter. + +Note that the syntax of the following `WIT` file matches the `wit-bindgen` version currently used by Spin, which is out-of-date with respect to the latest [`WIT` specification](https://github.com/WebAssembly/component-model/blob/main/design/mvp/WIT.md) and implementation. Once we're able to update `wit-bindgen`, we'll update the syntax of all the Spin `WIT` files, including this one. + +```fsharp +TODO: copy over the wit file +``` + +*Note: the pseudo-resource design was inspired by the interface of similar functions in [WASI preview 2](https://github.com/bytecodealliance/preview2-prototyping/blob/d56b8977a2b700432d1f7f84656d542f1d8854b0/wit/wasi.wit#L772-L794).* + +#### Implementation requirements + +TODO: Open questions: +* Assumed sqlite version? + +#### Built-in local database + +By default, each app will have its own default database which is independent of all other apps. For local apps, the database will be stored by default in a hidden `.spin` directory adjacent to the app's `spin.toml`. For remote apps: TODO + +#### Granting access to components + +By default, a given component of an app will _not_ have access to any database. Access must be granted specifically to each component via the following `spin.toml` syntax: + +```toml +sqlite_databases = ["", "", ...] +``` + +For example, a component could be given access to the default database using `sqlite_databases = ["default"]`. + +### Runtime Config + +Sqlite databases may be configured with `[sqlite_database.]` sections in the runtime config file: + +```toml +# The `default` config can be overridden +[sqlite_database.default] +path = ".spin/sqlite_key_value.db" +``` + +## Future work + +TODO From b56be4b879007c6d754a8279d922edd9fe365ad8 Mon Sep 17 00:00:00 2001 From: Ryan Levick Date: Mon, 17 Apr 2023 14:21:42 +0200 Subject: [PATCH 07/16] Clean up of small bits Signed-off-by: Ryan Levick --- crates/sqlite/src/host_component.rs | 10 +- crates/sqlite/src/lib.rs | 190 +++++++++++++++------------ crates/trigger/src/key_value.rs | 65 --------- crates/trigger/src/runtime_config.rs | 3 - docs/content/sips/000-sqlite.md | 91 +++++++++++-- sdk/rust/src/sqlite.rs | 85 ++++++++---- wit/ephemeral/sqlite.wit | 28 ++-- wit/preview2/sqlite.wit | 34 +++-- 8 files changed, 289 insertions(+), 217 deletions(-) delete mode 100644 crates/trigger/src/key_value.rs diff --git a/crates/sqlite/src/host_component.rs b/crates/sqlite/src/host_component.rs index 7cbecf00a7..955c00848b 100644 --- a/crates/sqlite/src/host_component.rs +++ b/crates/sqlite/src/host_component.rs @@ -1,10 +1,12 @@ -use std::path::PathBuf; +use std::{collections::HashSet, path::PathBuf}; -use spin_app::{AppComponent, DynamicHostComponent}; +use spin_app::{AppComponent, DynamicHostComponent, MetadataKey}; use spin_core::{sqlite, HostComponent}; use crate::SqliteImpl; +pub const DATABASES_KEY: MetadataKey> = MetadataKey::new("databases"); + #[derive(Debug, Clone)] pub enum DatabaseLocation { InMemory, @@ -37,7 +39,9 @@ impl HostComponent for SqliteComponent { } impl DynamicHostComponent for SqliteComponent { - fn update_data(&self, _data: &mut Self::Data, _component: &AppComponent) -> anyhow::Result<()> { + fn update_data(&self, data: &mut Self::Data, component: &AppComponent) -> anyhow::Result<()> { + let allowed_databases = component.get_metadata(DATABASES_KEY)?.unwrap_or_default(); + data.component_init(allowed_databases); Ok(()) } } diff --git a/crates/sqlite/src/lib.rs b/crates/sqlite/src/lib.rs index 0f24b1692a..22544712da 100644 --- a/crates/sqlite/src/lib.rs +++ b/crates/sqlite/src/lib.rs @@ -1,8 +1,7 @@ mod host_component; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; -use rand::Rng; use spin_core::{ async_trait, sqlite::{self, Host}, @@ -14,7 +13,8 @@ pub use host_component::SqliteComponent; pub struct SqliteImpl { location: DatabaseLocation, connections: HashMap, - statements: HashMap)>, + next_conn_key: u32, + allowed_databases: HashSet, } impl SqliteImpl { @@ -22,25 +22,13 @@ impl SqliteImpl { Self { location, connections: HashMap::default(), - statements: HashMap::default(), + next_conn_key: 0, + allowed_databases: HashSet::new(), } } - pub fn component_init(&mut self) {} - - fn find_statement( - &mut self, - connection: sqlite::Connection, - statement: sqlite::Statement, - ) -> Result<(&mut rusqlite::Connection, (&str, &[sqlite::DataType])), sqlite::Error> { - match ( - self.connections.get_mut(&connection), - self.statements.get(&statement), - ) { - (Some(c), Some((s, a))) => Ok((c, (s, a))), - (Some(_), None) => todo!(), - (None, _) => todo!(), - } + pub fn component_init(&mut self, allowed_databases: HashSet) { + self.allowed_databases = allowed_databases } } @@ -48,93 +36,131 @@ impl SqliteImpl { impl Host for SqliteImpl { async fn open( &mut self, - _name: String, + database: String, ) -> anyhow::Result> { - let conn = match &self.location { - DatabaseLocation::InMemory => rusqlite::Connection::open_in_memory()?, - DatabaseLocation::Path(p) => rusqlite::Connection::open(p)?, - }; + Ok(async { + if !self.allowed_databases.contains(&database) { + return Err(sqlite::Error::AccessDenied); + } + // TODO: handle more than one database + let conn = match &self.location { + DatabaseLocation::InMemory => rusqlite::Connection::open_in_memory() + .map_err(|e| sqlite::Error::Io(e.to_string()))?, + DatabaseLocation::Path(p) => { + rusqlite::Connection::open(p).map_err(|e| sqlite::Error::Io(e.to_string()))? + } + }; - // TODO: this is not the best way to do this... - let mut rng = rand::thread_rng(); - let c: sqlite::Connection = rng.gen(); - self.connections.insert(c, conn); - Ok(Ok(c)) + loop { + let key = self.next_conn_key; + self.next_conn_key = self.next_conn_key.wrapping_add(1); + if !self.connections.contains_key(&key) { + self.connections.insert(key, conn); + break Ok(key); + } + } + } + .await) } async fn execute( &mut self, connection: sqlite::Connection, - statement: sqlite::Statement, + statement: String, + parameters: Vec, ) -> anyhow::Result> { - let (c, (s, a)) = self.find_statement(connection, statement)?; - c.execute(s, rusqlite::params_from_iter(convert_data(a.iter()))) - .map_err(|e| sqlite::Error::Io(e.to_string()))?; - Ok(Ok(())) + Ok(async move { + let conn = self + .connections + .get(&connection) + .ok_or_else(|| sqlite::Error::InvalidConnection)?; + let mut statement = conn + .prepare_cached(&statement) + .map_err(|e| sqlite::Error::Io(e.to_string()))?; + statement + .execute(rusqlite::params_from_iter(convert_data( + parameters.into_iter(), + ))) + .map_err(|e| sqlite::Error::Io(e.to_string()))?; + Ok(()) + } + .await) } async fn query( &mut self, connection: sqlite::Connection, - query: sqlite::Statement, + query: String, + parameters: Vec, ) -> anyhow::Result, sqlite::Error>> { - let (c, (q, a)) = self.find_statement(connection, query)?; - let mut statement = c.prepare(q).map_err(|e| sqlite::Error::Io(e.to_string()))?; - let rows = statement - .query_map(rusqlite::params_from_iter(convert_data(a.iter())), |row| { - let mut values = vec![]; - for column in 0.. { - let value = match row.get_ref(column) { - Ok(rusqlite::types::ValueRef::Null) => sqlite::DataType::Null, - Ok(rusqlite::types::ValueRef::Integer(i)) => sqlite::DataType::Integer(i), - Ok(rusqlite::types::ValueRef::Real(f)) => sqlite::DataType::Real(f), - Ok(rusqlite::types::ValueRef::Text(t)) => { - sqlite::DataType::Text(String::from_utf8(t.to_vec()).unwrap()) + Ok(async move { + let conn = self.connections.get(&connection).expect("TODO"); + let mut statement = conn + .prepare_cached(&query) + .map_err(|e| sqlite::Error::Io(e.to_string()))?; + let rows = statement + .query_map( + rusqlite::params_from_iter(convert_data(parameters.into_iter())), + |row| { + let mut values = vec![]; + for column in 0.. { + let name = row.as_ref().column_name(column); + if let Err(rusqlite::Error::InvalidColumnIndex(_)) = name { + break; + } + let name = name?.to_string(); + let value = row.get::(column); + if let Err(rusqlite::Error::InvalidColumnIndex(_)) = value { + break; + } + let value = value?.0; + values.push(sqlite::ColumnValue { name, value }); } - Ok(rusqlite::types::ValueRef::Blob(b)) => { - sqlite::DataType::Blob(b.to_vec()) - } - Err(rusqlite::Error::InvalidColumnIndex(_)) => break, - _ => todo!(), - }; - values.push(value); - } - Ok(sqlite::Row { values }) - }) - .map_err(|e| sqlite::Error::Io(e.to_string()))?; - Ok(Ok(rows.collect::>()?)) + Ok(sqlite::Row { values }) + }, + ) + .map_err(|e| sqlite::Error::Io(e.to_string()))?; + Ok(rows + .map(|r| r.map_err(|e| sqlite::Error::Io(e.to_string()))) + .collect::>()?) + } + .await) } async fn close(&mut self, connection: spin_core::sqlite::Connection) -> anyhow::Result<()> { - let _ = self.connections.remove(&connection); - Ok(()) + Ok(async { + let _ = self.connections.remove(&connection); + } + .await) } +} - async fn prepare_statement( - &mut self, - statement: String, - params: Vec, - ) -> anyhow::Result> { - let mut rng = rand::thread_rng(); - let s: sqlite::Statement = rng.gen(); - self.statements.insert(s, (statement, params)); - Ok(Ok(s)) - } +// A wrapper around sqlite::Value so that we can convert from rusqlite ValueRef +struct ValueWrapper(sqlite::Value); - async fn drop_statement(&mut self, statement: sqlite::Statement) -> anyhow::Result<()> { - self.statements.remove(&statement); - Ok(()) +impl rusqlite::types::FromSql for ValueWrapper { + fn column_result(value: rusqlite::types::ValueRef<'_>) -> rusqlite::types::FromSqlResult { + let value = match value { + rusqlite::types::ValueRef::Null => sqlite::Value::Null, + rusqlite::types::ValueRef::Integer(i) => sqlite::Value::Integer(i), + rusqlite::types::ValueRef::Real(f) => sqlite::Value::Real(f), + rusqlite::types::ValueRef::Text(t) => { + sqlite::Value::Text(String::from_utf8(t.to_vec()).unwrap()) + } + rusqlite::types::ValueRef::Blob(b) => sqlite::Value::Blob(b.to_vec()), + }; + Ok(ValueWrapper(value)) } } -fn convert_data<'a>( - arguments: impl Iterator + 'a, -) -> impl Iterator + 'a { +fn convert_data( + arguments: impl Iterator, +) -> impl Iterator { arguments.map(|a| match a { - sqlite::DataType::Null => rusqlite::types::Value::Null, - sqlite::DataType::Integer(i) => rusqlite::types::Value::Integer(*i), - sqlite::DataType::Real(r) => rusqlite::types::Value::Real(*r), - sqlite::DataType::Text(t) => rusqlite::types::Value::Text(t.clone()), - sqlite::DataType::Blob(b) => rusqlite::types::Value::Blob(b.clone()), + sqlite::Value::Null => rusqlite::types::Value::Null, + sqlite::Value::Integer(i) => rusqlite::types::Value::Integer(i), + sqlite::Value::Real(r) => rusqlite::types::Value::Real(r), + sqlite::Value::Text(t) => rusqlite::types::Value::Text(t), + sqlite::Value::Blob(b) => rusqlite::types::Value::Blob(b), }) } diff --git a/crates/trigger/src/key_value.rs b/crates/trigger/src/key_value.rs deleted file mode 100644 index 70c4298f2a..0000000000 --- a/crates/trigger/src/key_value.rs +++ /dev/null @@ -1,65 +0,0 @@ -use std::{path::Path, sync::Arc}; - -use anyhow::{Context, Result}; -use spin_key_value::{ - CachingStoreManager, DelegatingStoreManager, KeyValueComponent, KEY_VALUE_STORES_KEY, -}; -use spin_key_value_sqlite::{DatabaseLocation, KeyValueSqlite}; - -use crate::{runtime_config::RuntimeConfig, TriggerHooks}; - -// TODO: Once we have runtime configuration for key-value stores, the user will be able -// to both change the default store configuration (e.g. use Redis, or an SQLite -// in-memory database, or use a different path) and add other named stores with their -// own configurations. - -pub(crate) fn build_key_value_component( - runtime_config: &RuntimeConfig, -) -> Result { - let location = match runtime_config.key_value_sqlite_db_path() { - Some(path) => { - // Create the store's parent directory if necessary - create_parent_dir(&path).context("Failed to create key value store")?; - DatabaseLocation::Path(path) - } - None => DatabaseLocation::InMemory, - }; - - let manager = Arc::new(CachingStoreManager::new(DelegatingStoreManager::new([( - "default".to_owned(), - Arc::new(KeyValueSqlite::new(location)) as _, - )]))); - - Ok(KeyValueComponent::new(spin_key_value::manager(move |_| { - manager.clone() - }))) -} - -fn create_parent_dir(path: &Path) -> Result<()> { - let dir = path - .parent() - .with_context(|| format!("{path:?} missing parent dir"))?; - std::fs::create_dir_all(dir).with_context(|| format!("Failed to create parent dir {dir:?}")) -} - -pub struct KeyValuePersistenceMessageHook; - -impl TriggerHooks for KeyValuePersistenceMessageHook { - fn app_loaded(&mut self, app: &spin_app::App, runtime_config: &RuntimeConfig) -> Result<()> { - // Don't print anything if the app doesn't use KV - if app.components().all(|c| { - c.get_metadata(KEY_VALUE_STORES_KEY) - .unwrap_or_default() - .unwrap_or_default() - .is_empty() - }) { - return Ok(()); - } - if let Some(path) = runtime_config.key_value_sqlite_db_path() { - println!("Storing key-value data to {path:?}"); - } else { - println!("Using in-memory key-value store; data will not be saved!"); - } - Ok(()) - } -} diff --git a/crates/trigger/src/runtime_config.rs b/crates/trigger/src/runtime_config.rs index 7d1283ca19..8be63b1536 100644 --- a/crates/trigger/src/runtime_config.rs +++ b/crates/trigger/src/runtime_config.rs @@ -205,7 +205,6 @@ mod tests { assert_eq!(config.state_dir(), None); assert_eq!(config.log_dir(), None); assert_eq!(default_spin_store_path(&config), None); - assert_eq!(config.key_value_sqlite_db_path(), None); Ok(()) } @@ -223,8 +222,6 @@ mod tests { let default_db_path = default_spin_store_path(&config).unwrap(); assert!(default_db_path.starts_with(&state_dir)); - let sqlite_db_path = config.key_value_sqlite_db_path().unwrap(); - assert!(sqlite_db_path.starts_with(&state_dir)); Ok(()) } diff --git a/docs/content/sips/000-sqlite.md b/docs/content/sips/000-sqlite.md index b4f062d9c4..7a01cf580a 100644 --- a/docs/content/sips/000-sqlite.md +++ b/docs/content/sips/000-sqlite.md @@ -1,4 +1,4 @@ -title = "SIP 000 - Sqlite" +title = "SIP 000 - sqlite" template = "main" date = "2023-04-17:00:00Z" --- @@ -11,15 +11,15 @@ Created: Apr 17, 2023 ## Background -Spin currently supports two database types: mysql and [postgres](https://developer.fermyon.com/cloud/data-postgres) which both require the user to provide their own database that is exposed to users through the SDK. +Spin currently supports two database types: mysql and [postgres](https://developer.fermyon.com/cloud/data-postgres) which both require the user to provide their own database that is exposed to users through the SDK. This is largely a stopgap until sockets are supported in wasi and there is no longer a need for bespoke clients for these databases (users can bring their favorite client libraries instead). -In contrast to the these other interfaces, the sqlite implementation would easily allow local spin deployment to use a local sqlite database file, and it provides those hosting spin deployment envionments (e.g., Fermyon Cloud) to implement lightweight sqlite implementations. +In contrast to the these other interfaces, the sqlite implementation would easily allow local Spin deployment to use a local sqlite database file, and it provides those hosting Spin deployment envionments (e.g., Fermyon Cloud) to implement lightweight sqlite implementations. In short, a sqlite interface in Spin would allow for a "zero config" experience when users want to work with a SQL database. ### What about `wasi-sql`? -[`wasi-sql`](https://github.com/WebAssembly/wasi-sql) is a work-in-progress spec for a generic SQL interface that aims to support "the features commonly used by 80% of user application". It is likely that when `wasi-sql` is more mature users will be able to successfully use functionality based on the `wasi-sql` interface to interact with the sqlite databases. However, there are still reasons that a dedicated sqlite interface is still useful: +[`wasi-sql`](https://github.com/WebAssembly/wasi-sql) is a work-in-progress spec for a generic SQL interface that aims to support "the features commonly used by 80% of user application". It is likely that when `wasi-sql` is more mature users will be able to successfully use functionality based on the `wasi-sql` interface to interact with a sqlite databases. However, there are still reasons that a dedicated sqlite interface would still be useful: -* For the 20% of use cases where `wasi-sql` is too generic a dedicated `sqlite` interface can provide that functionality. +* For the 20% of use cases where `wasi-sql` is too generic, a dedicated `sqlite` interface can provide that functionality. * The `wasi-sql` spec is under active investigation, and there are large questions about how to best support such a wide breadth of sql flavors. This implementation can help clarify those questions and push upstream work further along. ## Proposal @@ -28,38 +28,101 @@ In order to support sqlite, the following need to be added to Spin: - A `WIT` file that defines the sqlite interface - SDK implementations for various programming languages -- A default local sqlite store (note: spin already uses sqlite for the KV implementation so this should be trivial) +- A default local sqlite store (note: Spin already uses sqlite for the KV implementation so this should be trivial) - Potentially runtime configuration for configuring how sqlite is provisioned. +- Potentially a mechansim for handling database migrations -### Key-Value Interface (`.wit`) +### Interface (`.wit`) We will start with the `wasi-sql` interface but deliberately change that interface as to better match sqlite semantics. This will ensure that we're not simply implementing early versions of the `wasi-sql` interface while still having good answers for why the interface differs when it does. -Like `wasi-sql` and the key-value store, we model resources such as database connections as pseudo-[resource handles](https://github.com/WebAssembly/component-model/blob/main/design/mvp/WIT.md#item-resource) which may be created using an `open` function and disposed using a `close` function. Each operation on a connection is a function which accepts a handle as its first parameter. +Like `wasi-sql` and the key-value store, we model resources such as database connections as pseudo-[resource handles](https://github.com/WebAssembly/component-model/blob/main/design/mvp/WIT.md#item-resource) which may be created using an `open` function and disposed using a `close` function. Each operation on a connection is a function which accepts a handle as its first parameter. -Note that the syntax of the following `WIT` file matches the `wit-bindgen` version currently used by Spin, which is out-of-date with respect to the latest [`WIT` specification](https://github.com/WebAssembly/component-model/blob/main/design/mvp/WIT.md) and implementation. Once we're able to update `wit-bindgen`, we'll update the syntax of all the Spin `WIT` files, including this one. +Note that the syntax of the following `WIT` file matches the `wit-bindgen` version currently used by Spin, which is out-of-date with respect to the latest [`WIT` specification](https://github.com/WebAssembly/component-model/blob/main/design/mvp/WIT.md) and implementation. Once we're able to update `wit-bindgen`, we'll update the syntax of all the Spin `WIT` files, including this one. ```fsharp -TODO: copy over the wit file +// A handle to an open sqlite instance +type connection = u32 + +// The set of errors which may be raised by functions in this interface +variant error { + // The host does not recognize the database name requested. + no-such-database, + // Some implementation-specific error has occurred (e.g. I/O) + io(string) +} + +// Open a connection to a named database instance. +// +// If `database` is "default", the default instance is opened. +// +// `error::no-such-database` will be raised if the `name` is not recognized. +open: func(name: string) -> expected + +// Execute a statement +execute: func(conn: connection, statement: string, parameters: list) -> expected + +// Query data +query: func(conn: connection, query: string, parameters: list) -> expected, error> + +// Close the specified `connection`. +close: func(conn: connection) + +// A database row +record row { + values: list, +} + +// A single column's value +record column-value { + name: string, + value: value +} + +variant value { + integer(s64), + real(float64), + text(string), + blob(list), + null +} ``` *Note: the pseudo-resource design was inspired by the interface of similar functions in [WASI preview 2](https://github.com/bytecodealliance/preview2-prototyping/blob/d56b8977a2b700432d1f7f84656d542f1d8854b0/wit/wasi.wit#L772-L794).* +#### Database migrations + +Database tables typically require some sort of configuration in the form of database migrations to get table schemas into the correct state. While we could require the user to ensure that the database is in the correct state each time the trigger handler function is run, there are a few issues with this: +* Schema tracking schemes (e.g., a "migrations" table) themselves require some sort of bootstrap step. +* This goes against the design principle of keeping components handler functions simple and single purpose. + +There are several possible ways to address this issue such as: +* Some mechanism for running spin components before others where the component receives the current schema version and decides whether or not to perform migrations. +* The spin component could expose a current schema version as an exported value type so that an exported function would not need to called. If the exported schema version does not match the current schema version, an exported migrate function then gets called. +* A spin component that gets called just after pre-initialization finishes. Similarly, this component would expose a schema version and have an exported migration function called when the exported schema version does not match the current schema version. +* Configuration option in spin.toml manifest for running arbitrary SQL instructions on start up (e.g., `sqlite.migration = "CREATE TABLE users..."`) +* Command line supplied option for running SQL instructions on start up (e.g., `--sqlite-migration "CREATE TABLE users..."`) + +It should be noted that many of these options are not mutually exclusive and we could introduce more than one (perhaps starting with one option that will mostly be replaced later with a more generalized approach). + +TODO: decide which of these (or another mechanism) to use + #### Implementation requirements TODO: Open questions: -* Assumed sqlite version? +* Assumed sqlite version +* Capacity limits #### Built-in local database -By default, each app will have its own default database which is independent of all other apps. For local apps, the database will be stored by default in a hidden `.spin` directory adjacent to the app's `spin.toml`. For remote apps: TODO +By default, each app will have its own default database which is independent of all other apps. For local apps, the database will be stored by default in a hidden `.spin` directory adjacent to the app's `spin.toml`. For remote apps: TODO #### Granting access to components -By default, a given component of an app will _not_ have access to any database. Access must be granted specifically to each component via the following `spin.toml` syntax: +By default, a given component of an app will _not_ have access to any database. Access must be granted specifically to each component via the following `spin.toml` syntax: ```toml -sqlite_databases = ["", "", ...] +sqlite_databases = ["", ""] ``` For example, a component could be given access to the default database using `sqlite_databases = ["default"]`. diff --git a/sdk/rust/src/sqlite.rs b/sdk/rust/src/sqlite.rs index bf2967f83c..8d818c1978 100644 --- a/sdk/rust/src/sqlite.rs +++ b/sdk/rust/src/sqlite.rs @@ -9,9 +9,9 @@ pub type Error = sqlite::Error; pub type Row = sqlite::Row; /// -pub type DataTypeParam<'a> = sqlite::DataTypeParam<'a>; +pub type DataTypeParam<'a> = sqlite::ValueParam<'a>; /// -pub type DataTypeResult = sqlite::DataTypeResult; +pub type DataTypeResult = sqlite::ValueResult; /// Represents a store in which key value tuples may be placed #[derive(Debug)] @@ -23,40 +23,79 @@ impl Connection { Ok(Self(sqlite::open("foo")?)) } + /// Execute a statement against the database + pub fn execute<'a>( + &self, + statement: &str, + parameters: &[sqlite::ValueParam<'a>], + ) -> Result<(), Error> { + sqlite::execute(self.0, statement, parameters)?; + Ok(()) + } + /// Make a query against the database - pub fn query(&self, statement: &Statement) -> Result, Error> { - sqlite::query(self.0, statement.0) + pub fn query<'a>( + &self, + query: &str, + parameters: &[DataTypeParam<'a>], + ) -> Result, Error> { + sqlite::query(self.0, query, parameters) } +} - /// Execute a statement against the database - pub fn execute(&self, statement: &str) -> Result<(), Error> { - let statement = Statement::prepare(statement, &[])?; - sqlite::execute(self.0, statement.0)?; - Ok(()) +impl Row { + pub fn get<'a, T: TryFrom<&'a sqlite::ValueResult>>(&'a self, name: &str) -> Option { + self.values + .iter() + .find_map(|c| (c.name == name).then(|| (&c.value).try_into().ok())) + .flatten() + } + + pub fn geti<'a, T: TryFrom<&'a sqlite::ValueResult>>(&'a self, index: usize) -> Option { + self.values + .get(index) + .map(|c| (&c.value).try_into().ok()) + .flatten() } } -impl std::fmt::Display for Error { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "{self:?}") +impl<'a> TryFrom<&'a sqlite::ValueResult> for bool { + type Error = (); + + fn try_from(value: &'a sqlite::ValueResult) -> Result { + match value { + sqlite::ValueResult::Integer(i) => Ok(*i != 0), + _ => Err(()), + } } } -impl std::error::Error for Error {} +impl<'a> TryFrom<&'a sqlite::ValueResult> for u32 { + type Error = (); + + fn try_from(value: &'a sqlite::ValueResult) -> Result { + match value { + sqlite::ValueResult::Integer(i) => Ok(*i as u32), + _ => Err(()), + } + } +} -/// A prepared statement -pub struct Statement(sqlite::Statement); +impl<'a> TryFrom<&'a sqlite::ValueResult> for &'a str { + type Error = (); -impl Statement { - /// Prepare a statement - pub fn prepare(query: &str, params: &[DataTypeParam]) -> Result { - let statement = sqlite::prepare_statement(query, params)?; - Ok(Statement(statement)) + fn try_from(value: &'a sqlite::ValueResult) -> Result { + match value { + sqlite::ValueResult::Text(s) => Ok(s.as_str()), + _ => Err(()), + } } } -impl Drop for Statement { - fn drop(&mut self) { - sqlite::drop_statement(self.0); +impl std::fmt::Display for Error { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{self:?}") } } + +impl std::error::Error for Error {} diff --git a/wit/ephemeral/sqlite.wit b/wit/ephemeral/sqlite.wit index 9564b3514c..c42a4babef 100644 --- a/wit/ephemeral/sqlite.wit +++ b/wit/ephemeral/sqlite.wit @@ -3,38 +3,40 @@ type connection = u32 // The set of errors which may be raised by functions in this interface variant error { + // The host does not recognize the database name requested. + no-such-database, // Some implementation-specific error has occurred (e.g. I/O) io(string) } -// Open a connection to a named instance. +// Open a connection to a named database instance. // -// If `name` is "default", the default instance is opened. +// If `database` is "default", the default instance is opened. // -// `error::no-such-store` will be raised if the `name` is not recognized. +// `error::no-such-database` will be raised if the `name` is not recognized. open: func(name: string) -> expected -// allows parameterized queries -type statement = u32 -drop-statement: func(statement: statement) -prepare-statement: func(query: string, params: list) -> expected - // Execute a statement -execute: func(conn: connection, statement: statement) -> expected +execute: func(conn: connection, statement: string, parameters: list) -> expected // Query data -query: func(conn: connection, q: statement) -> expected, error> +query: func(conn: connection, query: string, parameters: list) -> expected, error> // Close the specified `connection`. close: func(conn: connection) // A database row record row { - values: list, + values: list, +} + +// A single column's value +record column-value { + name: string, + value: value } -// Common data types -variant data-type { +variant value { integer(s64), real(float64), text(string), diff --git a/wit/preview2/sqlite.wit b/wit/preview2/sqlite.wit index 59bdd9f1bd..b4f4258c16 100644 --- a/wit/preview2/sqlite.wit +++ b/wit/preview2/sqlite.wit @@ -4,38 +4,44 @@ default interface sqlite { // The set of errors which may be raised by functions in this interface variant error { + // The host does not recognize the database name requested. + no-such-database, + // The requesting component does not have access to the specified database (which may or may not exist). + access-denied, + // The provided connection is not valid + invalid-connection, // Some implementation-specific error has occurred (e.g. I/O) io(string) } - // Open a connection to a named instance. + // Open a connection to a named database instance. // - // If `name` is "default", the default instance is opened. + // If `database` is "default", the default instance is opened. // - // `error::no-such-store` will be raised if the `name` is not recognized. - open: func(name: string) -> result + // `error::no-such-database` will be raised if the `name` is not recognized. + open: func(database: string) -> result // Execute a statement - execute: func(conn: connection, statement: statement) -> result<_, error> + execute: func(conn: connection, statement: string, parameters: list) -> result<_, error> // Query data - query: func(conn: connection, query: statement) -> result, error> + query: func(conn: connection, query: string, parameters: list) -> result, error> // Close the specified `connection`. close: func(conn: connection) - // allows parameterized queries - type statement = u32 - prepare-statement: func(query: string, params: list) -> result - drop-statement: func(s: statement) - // A database row record row { - values: list, + values: list, + } + + // A single column's value + record column-value { + name: string, + value: value } - // Common data types - variant data-type { + variant value { integer(s64), real(float64), text(string), From 4a1e98f2cb0a80d93a234237245d5bdf54698b80 Mon Sep 17 00:00:00 2001 From: Ryan Levick Date: Thu, 20 Apr 2023 19:32:59 +0200 Subject: [PATCH 08/16] Allow runtime config Signed-off-by: Ryan Levick --- Cargo.lock | 2 + crates/bindle/src/expander.rs | 1 + crates/key-value/src/lib.rs | 2 +- crates/loader/src/bindle/config.rs | 2 + crates/loader/src/bindle/mod.rs | 2 + crates/loader/src/local/config.rs | 2 + crates/loader/src/local/mod.rs | 2 + crates/manifest/src/lib.rs | 2 + crates/sqlite/Cargo.toml | 4 +- crates/sqlite/src/host_component.rs | 63 ++++++++++++++--- crates/sqlite/src/lib.rs | 75 ++++++++++++--------- crates/trigger/src/lib.rs | 3 +- crates/trigger/src/locked.rs | 2 + crates/trigger/src/runtime_config.rs | 28 ++++++++ crates/trigger/src/runtime_config/sqlite.rs | 69 +++++++++++++++++++ crates/trigger/src/sqlite.rs | 26 ------- sdk/rust/src/sqlite.rs | 4 +- wit/ephemeral/sqlite.wit | 4 ++ 18 files changed, 219 insertions(+), 74 deletions(-) create mode 100644 crates/trigger/src/runtime_config/sqlite.rs delete mode 100644 crates/trigger/src/sqlite.rs diff --git a/Cargo.lock b/Cargo.lock index b725f9b385..91edd49e6a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5426,10 +5426,12 @@ name = "spin-sqlite" version = "0.1.0" dependencies = [ "anyhow", + "once_cell", "rand 0.8.5", "rusqlite", "spin-app", "spin-core", + "spin-key-value", "wit-bindgen-wasmtime", ] diff --git a/crates/bindle/src/expander.rs b/crates/bindle/src/expander.rs index 4ba24d394d..00c2c488f2 100644 --- a/crates/bindle/src/expander.rs +++ b/crates/bindle/src/expander.rs @@ -123,6 +123,7 @@ async fn bindle_component_manifest( files: asset_group, allowed_http_hosts: local.wasm.allowed_http_hosts.clone(), key_value_stores: local.wasm.key_value_stores.clone(), + sqlite_databases: local.wasm.sqlite_databases.clone(), }, trigger: local.trigger.clone(), config: local.config.clone(), diff --git a/crates/key-value/src/lib.rs b/crates/key-value/src/lib.rs index 880bb6cc7f..0d307ef3e5 100644 --- a/crates/key-value/src/lib.rs +++ b/crates/key-value/src/lib.rs @@ -6,7 +6,7 @@ use std::{collections::HashSet, sync::Arc}; use table::Table; mod host_component; -mod table; +pub mod table; mod util; pub use host_component::{manager, KeyValueComponent}; diff --git a/crates/loader/src/bindle/config.rs b/crates/loader/src/bindle/config.rs index a0a7cbaded..f22850a660 100644 --- a/crates/loader/src/bindle/config.rs +++ b/crates/loader/src/bindle/config.rs @@ -49,6 +49,8 @@ pub struct RawWasmConfig { pub allowed_http_hosts: Option>, /// Optional list of key-value stores the component is allowed to use. pub key_value_stores: Option>, + /// Optional list of sqlite databases the component is allowed to use. + pub sqlite_databases: Option>, /// Environment variables to be mapped inside the Wasm module at runtime. pub environment: Option>, } diff --git a/crates/loader/src/bindle/mod.rs b/crates/loader/src/bindle/mod.rs index 1eacc29410..74220567a2 100644 --- a/crates/loader/src/bindle/mod.rs +++ b/crates/loader/src/bindle/mod.rs @@ -131,11 +131,13 @@ async fn core( let environment = raw.wasm.environment.unwrap_or_default(); let allowed_http_hosts = raw.wasm.allowed_http_hosts.unwrap_or_default(); let key_value_stores = raw.wasm.key_value_stores.unwrap_or_default(); + let sqlite_databases = raw.wasm.sqlite_databases.unwrap_or_default(); let wasm = WasmConfig { environment, mounts, allowed_http_hosts, key_value_stores, + sqlite_databases, }; let config = raw.config.unwrap_or_default(); Ok(CoreComponent { diff --git a/crates/loader/src/local/config.rs b/crates/loader/src/local/config.rs index 02ffc8ef25..7d16064c29 100644 --- a/crates/loader/src/local/config.rs +++ b/crates/loader/src/local/config.rs @@ -142,6 +142,8 @@ pub struct RawWasmConfig { pub allowed_http_hosts: Option>, /// Optional list of key-value stores the component is allowed to use. pub key_value_stores: Option>, + /// Optional list of sqlite databases the component is allowed to use. + pub sqlite_databases: Option>, /// Environment variables to be mapped inside the Wasm module at runtime. pub environment: Option>, } diff --git a/crates/loader/src/local/mod.rs b/crates/loader/src/local/mod.rs index 63cbaeb912..a257357b6c 100644 --- a/crates/loader/src/local/mod.rs +++ b/crates/loader/src/local/mod.rs @@ -204,11 +204,13 @@ async fn core( let environment = raw.wasm.environment.unwrap_or_default(); let allowed_http_hosts = raw.wasm.allowed_http_hosts.unwrap_or_default(); let key_value_stores = raw.wasm.key_value_stores.unwrap_or_default(); + let sqlite_databases = raw.wasm.sqlite_databases.unwrap_or_default(); let wasm = WasmConfig { environment, mounts, allowed_http_hosts, key_value_stores, + sqlite_databases, }; let config = raw.config.unwrap_or_default(); Ok(CoreComponent { diff --git a/crates/manifest/src/lib.rs b/crates/manifest/src/lib.rs index a099051e71..ff2e7a657e 100644 --- a/crates/manifest/src/lib.rs +++ b/crates/manifest/src/lib.rs @@ -270,6 +270,8 @@ pub struct WasmConfig { pub allowed_http_hosts: Vec, /// Optional list of key-value stores the component is allowed to use. pub key_value_stores: Vec, + /// Optional list of sqlite databases the component is allowed to use. + pub sqlite_databases: Vec, } /// Directory mount for the assets of a component. diff --git a/crates/sqlite/Cargo.toml b/crates/sqlite/Cargo.toml index d61e2ec52d..ea484e7575 100644 --- a/crates/sqlite/Cargo.toml +++ b/crates/sqlite/Cargo.toml @@ -6,7 +6,9 @@ edition = "2021" [dependencies] spin-core = { path = "../core" } spin-app = { path = "../app" } +spin-key-value = { path = "../key-value" } anyhow = "1.0" wit-bindgen-wasmtime = { workspace = true } rusqlite = { version = "0.27.0", features = [ "bundled" ] } -rand = "0.8" \ No newline at end of file +rand = "0.8" +once_cell = "1" \ No newline at end of file diff --git a/crates/sqlite/src/host_component.rs b/crates/sqlite/src/host_component.rs index 955c00848b..f8fad9386d 100644 --- a/crates/sqlite/src/host_component.rs +++ b/crates/sqlite/src/host_component.rs @@ -1,25 +1,67 @@ -use std::{collections::HashSet, path::PathBuf}; +use std::{ + collections::HashMap, + path::PathBuf, + sync::{Arc, Mutex}, +}; -use spin_app::{AppComponent, DynamicHostComponent, MetadataKey}; +use once_cell::sync::OnceCell; +use rusqlite::Connection; +use spin_app::{AppComponent, DynamicHostComponent}; use spin_core::{sqlite, HostComponent}; use crate::SqliteImpl; -pub const DATABASES_KEY: MetadataKey> = MetadataKey::new("databases"); - #[derive(Debug, Clone)] pub enum DatabaseLocation { InMemory, Path(PathBuf), } -pub struct SqliteComponent { +/// A connection to a sqlite database +pub struct SqliteConnection { location: DatabaseLocation, + connection: OnceCell>>, } -impl SqliteComponent { +impl SqliteConnection { pub fn new(location: DatabaseLocation) -> Self { - Self { location } + Self { + location, + connection: OnceCell::new(), + } + } +} + +impl ConnectionManager for SqliteConnection { + fn get_connection(&self) -> Result>, sqlite::Error> { + let connection = self + .connection + .get_or_try_init(|| -> Result<_, sqlite::Error> { + let c = match &self.location { + DatabaseLocation::InMemory => Connection::open_in_memory(), + DatabaseLocation::Path(path) => Connection::open(path), + } + .map_err(|e| sqlite::Error::Io(e.to_string()))?; + Ok(Arc::new(Mutex::new(c))) + })? + .clone(); + Ok(connection) + } +} + +pub trait ConnectionManager: Send + Sync { + fn get_connection(&self) -> Result>, sqlite::Error>; +} + +pub struct SqliteComponent { + connection_managers: HashMap>, +} + +impl SqliteComponent { + pub fn new(connection_managers: HashMap>) -> Self { + Self { + connection_managers, + } } } @@ -34,14 +76,17 @@ impl HostComponent for SqliteComponent { } fn build_data(&self) -> Self::Data { - SqliteImpl::new(self.location.clone()) + SqliteImpl::new(self.connection_managers.clone()) } } impl DynamicHostComponent for SqliteComponent { fn update_data(&self, data: &mut Self::Data, component: &AppComponent) -> anyhow::Result<()> { - let allowed_databases = component.get_metadata(DATABASES_KEY)?.unwrap_or_default(); + let allowed_databases = component + .get_metadata(crate::DATABASES_KEY)? + .unwrap_or_default(); data.component_init(allowed_databases); + // TODO: allow dynamically updating connection manager Ok(()) } } diff --git a/crates/sqlite/src/lib.rs b/crates/sqlite/src/lib.rs index 22544712da..92832223ba 100644 --- a/crates/sqlite/src/lib.rs +++ b/crates/sqlite/src/lib.rs @@ -1,35 +1,52 @@ mod host_component; -use std::collections::{HashMap, HashSet}; +use std::{ + collections::{HashMap, HashSet}, + sync::{Arc, Mutex, MutexGuard}, +}; +use rusqlite::Connection; +use spin_app::MetadataKey; use spin_core::{ async_trait, sqlite::{self, Host}, }; -pub use host_component::DatabaseLocation; -pub use host_component::SqliteComponent; +pub use host_component::{ConnectionManager, DatabaseLocation, SqliteComponent, SqliteConnection}; +use spin_key_value::table; + +pub const DATABASES_KEY: MetadataKey> = MetadataKey::new("databases"); pub struct SqliteImpl { - location: DatabaseLocation, - connections: HashMap, - next_conn_key: u32, allowed_databases: HashSet, + connections: table::Table>>, + client_manager: HashMap>, } impl SqliteImpl { - pub fn new(location: DatabaseLocation) -> Self { + pub fn new(client_manager: HashMap>) -> Self { Self { - location, - connections: HashMap::default(), - next_conn_key: 0, + connections: table::Table::new(256), allowed_databases: HashSet::new(), + client_manager, } } pub fn component_init(&mut self, allowed_databases: HashSet) { self.allowed_databases = allowed_databases } + + fn get_connection<'a>( + &'a self, + connection: sqlite::Connection, + ) -> Result, sqlite::Error> { + Ok(self + .connections + .get(connection) + .ok_or_else(|| sqlite::Error::InvalidConnection)? + .lock() + .unwrap()) + } } #[async_trait] @@ -39,26 +56,20 @@ impl Host for SqliteImpl { database: String, ) -> anyhow::Result> { Ok(async { + println!("{database}"); if !self.allowed_databases.contains(&database) { return Err(sqlite::Error::AccessDenied); } - // TODO: handle more than one database - let conn = match &self.location { - DatabaseLocation::InMemory => rusqlite::Connection::open_in_memory() - .map_err(|e| sqlite::Error::Io(e.to_string()))?, - DatabaseLocation::Path(p) => { - rusqlite::Connection::open(p).map_err(|e| sqlite::Error::Io(e.to_string()))? - } - }; - - loop { - let key = self.next_conn_key; - self.next_conn_key = self.next_conn_key.wrapping_add(1); - if !self.connections.contains_key(&key) { - self.connections.insert(key, conn); - break Ok(key); - } - } + self.connections + .push( + self.client_manager + .get(&database) + .ok_or(sqlite::Error::NoSuchDatabase)? + .get_connection()?, + ) + .map_err(|()| { + todo!("Create an error for when we reach capacity on number of databases") + }) } .await) } @@ -70,10 +81,7 @@ impl Host for SqliteImpl { parameters: Vec, ) -> anyhow::Result> { Ok(async move { - let conn = self - .connections - .get(&connection) - .ok_or_else(|| sqlite::Error::InvalidConnection)?; + let conn = self.get_connection(connection)?; let mut statement = conn .prepare_cached(&statement) .map_err(|e| sqlite::Error::Io(e.to_string()))?; @@ -94,7 +102,7 @@ impl Host for SqliteImpl { parameters: Vec, ) -> anyhow::Result, sqlite::Error>> { Ok(async move { - let conn = self.connections.get(&connection).expect("TODO"); + let conn = self.get_connection(connection)?; let mut statement = conn .prepare_cached(&query) .map_err(|e| sqlite::Error::Io(e.to_string()))?; @@ -121,6 +129,7 @@ impl Host for SqliteImpl { ) .map_err(|e| sqlite::Error::Io(e.to_string()))?; Ok(rows + .into_iter() .map(|r| r.map_err(|e| sqlite::Error::Io(e.to_string()))) .collect::>()?) } @@ -129,7 +138,7 @@ impl Host for SqliteImpl { async fn close(&mut self, connection: spin_core::sqlite::Connection) -> anyhow::Result<()> { Ok(async { - let _ = self.connections.remove(&connection); + let _ = self.connections.remove(connection); } .await) } diff --git a/crates/trigger/src/lib.rs b/crates/trigger/src/lib.rs index 89d7b34412..f63441739f 100644 --- a/crates/trigger/src/lib.rs +++ b/crates/trigger/src/lib.rs @@ -2,7 +2,6 @@ pub mod cli; pub mod loader; pub mod locked; mod runtime_config; -mod sqlite; mod stdio; use std::{collections::HashMap, marker::PhantomData, path::PathBuf}; @@ -126,7 +125,7 @@ impl TriggerExecutorBuilder { )?; self.loader.add_dynamic_host_component( &mut builder, - crate::sqlite::build_component(&runtime_config)?, + runtime_config::sqlite::build_component(&runtime_config)?, )?; self.loader.add_dynamic_host_component( &mut builder, diff --git a/crates/trigger/src/locked.rs b/crates/trigger/src/locked.rs index c77d00d597..88f595bb1d 100644 --- a/crates/trigger/src/locked.rs +++ b/crates/trigger/src/locked.rs @@ -17,6 +17,7 @@ use spin_manifest::{ Application, ApplicationInformation, ApplicationOrigin, ApplicationTrigger, CoreComponent, HttpConfig, HttpTriggerConfiguration, RedisConfig, TriggerConfig, }; +use spin_sqlite::DATABASES_KEY; pub const NAME_KEY: MetadataKey = MetadataKey::new("name"); pub const VERSION_KEY: MetadataKey = MetadataKey::new("version"); @@ -146,6 +147,7 @@ impl LockedAppBuilder { .string_option(DESCRIPTION_KEY, component.description) .string_array(ALLOWED_HTTP_HOSTS_KEY, component.wasm.allowed_http_hosts) .string_array(KEY_VALUE_STORES_KEY, component.wasm.key_value_stores) + .string_array(DATABASES_KEY, component.wasm.sqlite_databases) .take(); let source = { diff --git a/crates/trigger/src/runtime_config.rs b/crates/trigger/src/runtime_config.rs index 8be63b1536..64d2a73134 100644 --- a/crates/trigger/src/runtime_config.rs +++ b/crates/trigger/src/runtime_config.rs @@ -1,5 +1,6 @@ pub mod config_provider; pub mod key_value; +pub mod sqlite; use std::{ collections::HashMap, @@ -13,6 +14,7 @@ use serde::Deserialize; use self::{ config_provider::{ConfigProvider, ConfigProviderOpts}, key_value::{KeyValueStore, KeyValueStoreOpts}, + sqlite::SqliteDatabaseOpts, }; pub const DEFAULT_STATE_DIR: &str = ".spin"; @@ -94,6 +96,29 @@ impl RuntimeConfig { .unwrap_or_else(|| KeyValueStoreOpts::default_store_opts(self)) } + /// Return an iterator of named configured [`SqliteDatabase`]s. + pub fn sqlite_databases( + &self, + ) -> Result> { + let mut databases = HashMap::new(); + // Insert explicitly-configured databases + for opts in self.opts_layers() { + for (name, database) in &opts.sqlite_databases { + if !databases.contains_key(name) { + let store = database.build(name, opts)?; + databases.insert(name.to_owned(), store); + } + } + } + // Upsert default store + if !databases.contains_key("default") { + let store = SqliteDatabaseOpts::default(self) + .build("default", &RuntimeConfigOpts::default())?; + databases.insert("default".into(), store); + } + Ok(databases.into_iter()) + } + /// Set the state dir, overriding any other runtime config source. pub fn set_state_dir(&mut self, state_dir: impl Into) { self.overrides.state_dir = Some(state_dir.into()); @@ -169,6 +194,9 @@ pub struct RuntimeConfigOpts { #[serde(rename = "key_value_store", default)] pub key_value_stores: HashMap, + #[serde(rename = "sqlite_database", default)] + pub sqlite_databases: HashMap, + #[serde(skip)] pub file_path: Option, } diff --git a/crates/trigger/src/runtime_config/sqlite.rs b/crates/trigger/src/runtime_config/sqlite.rs new file mode 100644 index 0000000000..4b56a0516e --- /dev/null +++ b/crates/trigger/src/runtime_config/sqlite.rs @@ -0,0 +1,69 @@ +use std::{path::PathBuf, sync::Arc}; + +use crate::runtime_config::RuntimeConfig; +use anyhow::Context; +use spin_sqlite::{DatabaseLocation, SqliteComponent, SqliteConnection}; + +use super::RuntimeConfigOpts; + +pub type SqliteDatabase = Arc; + +pub(crate) fn build_component(runtime_config: &RuntimeConfig) -> anyhow::Result { + let databases = runtime_config + .sqlite_databases() + .context("Failed to build sqlite component")? + .into_iter() + .collect(); + Ok(SqliteComponent::new(databases)) +} + +// Holds deserialized options from a `[sqlite_database.]` runtime config section. +#[derive(Clone, Debug, serde::Deserialize)] +#[serde(rename_all = "snake_case", tag = "type")] +pub enum SqliteDatabaseOpts { + Spin(SpinSqliteDatabaseOpts), +} + +impl SqliteDatabaseOpts { + pub fn default(runtime_config: &RuntimeConfig) -> Self { + Self::Spin(SpinSqliteDatabaseOpts::default(runtime_config)) + } + + pub fn build( + &self, + name: &str, + config_opts: &RuntimeConfigOpts, + ) -> anyhow::Result { + match self { + Self::Spin(opts) => opts.build(name, config_opts), + } + } +} + +#[derive(Clone, Debug, Default, serde::Deserialize)] +#[serde(deny_unknown_fields)] +pub struct SpinSqliteDatabaseOpts { + pub path: Option, +} + +impl SpinSqliteDatabaseOpts { + pub fn default(runtime_config: &RuntimeConfig) -> Self { + // If the state dir is set, build the default path + let path = runtime_config.state_dir(); + Self { path } + } + + fn build(&self, name: &str, config_opts: &RuntimeConfigOpts) -> anyhow::Result { + let location = match self.path.as_ref() { + Some(path) => { + let path = super::resolve_config_path(path, config_opts)?; + // Create the store's parent directory if necessary + std::fs::create_dir_all(path.parent().unwrap()) + .context("Failed to create sqlite database directory")?; + DatabaseLocation::Path(path.join(format!("{name}.db"))) + } + None => DatabaseLocation::InMemory, + }; + Ok(Arc::new(SqliteConnection::new(location))) + } +} diff --git a/crates/trigger/src/sqlite.rs b/crates/trigger/src/sqlite.rs deleted file mode 100644 index dc85900aa2..0000000000 --- a/crates/trigger/src/sqlite.rs +++ /dev/null @@ -1,26 +0,0 @@ -use std::path::Path; - -use crate::runtime_config::RuntimeConfig; -use anyhow::Context; -use spin_sqlite::{DatabaseLocation, SqliteComponent}; - -// TODO: dedup with the stuff in key_value -pub(crate) fn build_component(runtime_config: &RuntimeConfig) -> anyhow::Result { - let location = match runtime_config.sqlite_db_path() { - Some(path) => { - // Create the store's parent directory if necessary - create_parent_dir(&path).context("Failed to create sqlite db")?; - DatabaseLocation::Path(path) - } - None => DatabaseLocation::InMemory, - }; - - Ok(SqliteComponent::new(location)) -} - -fn create_parent_dir(path: &Path) -> anyhow::Result<()> { - let dir = path - .parent() - .with_context(|| format!("{path:?} missing parent dir"))?; - std::fs::create_dir_all(dir).with_context(|| format!("Failed to create parent dir {dir:?}")) -} diff --git a/sdk/rust/src/sqlite.rs b/sdk/rust/src/sqlite.rs index 8d818c1978..2695772a70 100644 --- a/sdk/rust/src/sqlite.rs +++ b/sdk/rust/src/sqlite.rs @@ -19,8 +19,8 @@ pub struct Connection(RawConnection); impl Connection { /// Open a connection - pub fn open() -> Result { - Ok(Self(sqlite::open("foo")?)) + pub fn open(database: &str) -> Result { + Ok(Self(sqlite::open(database)?)) } /// Execute a statement against the database diff --git a/wit/ephemeral/sqlite.wit b/wit/ephemeral/sqlite.wit index c42a4babef..1bad9d3bc4 100644 --- a/wit/ephemeral/sqlite.wit +++ b/wit/ephemeral/sqlite.wit @@ -5,6 +5,10 @@ type connection = u32 variant error { // The host does not recognize the database name requested. no-such-database, + // The requesting component does not have access to the specified database (which may or may not exist). + access-denied, + // The provided connection is not valid + invalid-connection, // Some implementation-specific error has occurred (e.g. I/O) io(string) } From 7449358a07d87782dfd8a1a76f29738fe43ef1e8 Mon Sep 17 00:00:00 2001 From: Ryan Levick Date: Thu, 20 Apr 2023 19:44:16 +0200 Subject: [PATCH 09/16] Changes based on feedback Signed-off-by: Ryan Levick --- Cargo.lock | 11 +++--- Cargo.toml | 8 ++--- crates/key-value/src/table.rs | 1 + crates/sqlite/Cargo.toml | 3 +- crates/sqlite/src/host_component.rs | 3 +- crates/sqlite/src/lib.rs | 49 +++++++++++++------------- docs/content/sips/000-sqlite.md | 52 ++++++++++++++++++++-------- sdk/rust/Cargo.toml | 1 + sdk/rust/src/lib.rs | 1 + sdk/rust/src/sqlite.rs | 53 +++++++++++++++++------------ wit/ephemeral/sqlite.wit | 20 ++++++----- wit/preview2/sqlite.wit | 20 ++++++----- 12 files changed, 133 insertions(+), 89 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 91edd49e6a..647c626d7a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2341,7 +2341,7 @@ dependencies = [ [[package]] name = "host" version = "0.0.0" -source = "git+https://github.com/fermyon/spin-componentize?rev=51c3fade751c4e364142719e42130943fd8b0a76#51c3fade751c4e364142719e42130943fd8b0a76" +source = "git+https://github.com/fermyon/spin-componentize?branch=sqlite#cad37ef9bd846c1c010de631e7c8d2657397acde" dependencies = [ "anyhow", "async-trait", @@ -5155,7 +5155,7 @@ dependencies = [ [[package]] name = "spin-componentize" version = "0.1.0" -source = "git+https://github.com/fermyon/spin-componentize?rev=51c3fade751c4e364142719e42130943fd8b0a76#51c3fade751c4e364142719e42130943fd8b0a76" +source = "git+https://github.com/fermyon/spin-componentize?branch=sqlite#cad37ef9bd846c1c010de631e7c8d2657397acde" dependencies = [ "anyhow", "wasm-encoder 0.26.0", @@ -5432,6 +5432,7 @@ dependencies = [ "spin-app", "spin-core", "spin-key-value", + "spin-world", "wit-bindgen-wasmtime", ] @@ -6174,7 +6175,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "97fee6b57c6a41524a810daee9286c02d7752c4253064d0b05472833a438f675" dependencies = [ "cfg-if", - "rand 0.8.5", + "rand 0.7.3", "static_assertions", ] @@ -6369,7 +6370,7 @@ checksum = "9c8d87e72b64a3b4db28d11ce29237c246188f4f51057d65a7eab63b7987e423" [[package]] name = "wasi-cap-std-sync" version = "0.0.0" -source = "git+https://github.com/fermyon/spin-componentize?rev=51c3fade751c4e364142719e42130943fd8b0a76#51c3fade751c4e364142719e42130943fd8b0a76" +source = "git+https://github.com/fermyon/spin-componentize?branch=sqlite#cad37ef9bd846c1c010de631e7c8d2657397acde" dependencies = [ "anyhow", "async-trait", @@ -6417,7 +6418,7 @@ dependencies = [ [[package]] name = "wasi-common" version = "0.0.0" -source = "git+https://github.com/fermyon/spin-componentize?rev=51c3fade751c4e364142719e42130943fd8b0a76#51c3fade751c4e364142719e42130943fd8b0a76" +source = "git+https://github.com/fermyon/spin-componentize?branch=sqlite#cad37ef9bd846c1c010de631e7c8d2657397acde" dependencies = [ "anyhow", "async-trait", diff --git a/Cargo.toml b/Cargo.toml index fe24e4d58b..3fe47cd539 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -115,10 +115,10 @@ tracing = { version = "0.1", features = ["log"] } wasmtime-wasi = { version = "8.0.1", features = ["tokio"] } wasi-common-preview1 = { package = "wasi-common", version = "8.0.1" } wasmtime = { version = "8.0.1", features = ["component-model"] } -spin-componentize = { git = "https://github.com/fermyon/spin-componentize", rev = "acdd2d6d233eb71d1f64dbd237aeb5e17eb21ffb" } -wasi-host = { package = "host", git = "https://github.com/fermyon/spin-componentize", rev = "51c3fade751c4e364142719e42130943fd8b0a76" } -wasi-common = { git = "https://github.com/fermyon/spin-componentize", rev = "51c3fade751c4e364142719e42130943fd8b0a76" } -wasi-cap-std-sync = { git = "https://github.com/fermyon/spin-componentize", rev = "51c3fade751c4e364142719e42130943fd8b0a76" } +spin-componentize = { git = "https://github.com/fermyon/spin-componentize", branch = "sqlite" } +wasi-host = { package = "host", git = "https://github.com/fermyon/spin-componentize", branch = "sqlite" } +wasi-common = { git = "https://github.com/fermyon/spin-componentize", branch = "sqlite" } +wasi-cap-std-sync = { git = "https://github.com/fermyon/spin-componentize", branch = "sqlite" } [workspace.dependencies.bindle] git = "https://github.com/fermyon/bindle" diff --git a/crates/key-value/src/table.rs b/crates/key-value/src/table.rs index 9023482dc9..3ff4b4b939 100644 --- a/crates/key-value/src/table.rs +++ b/crates/key-value/src/table.rs @@ -34,6 +34,7 @@ impl Table { /// /// This function will attempt to avoid reusing recently closed identifiers, but after 2^32 calls to this /// function they will start repeating. + #[allow(clippy::result_unit_err)] pub fn push(&mut self, value: V) -> Result { if self.tuples.len() == self.capacity as usize { Err(()) diff --git a/crates/sqlite/Cargo.toml b/crates/sqlite/Cargo.toml index ea484e7575..486e1a94eb 100644 --- a/crates/sqlite/Cargo.toml +++ b/crates/sqlite/Cargo.toml @@ -7,8 +7,9 @@ edition = "2021" spin-core = { path = "../core" } spin-app = { path = "../app" } spin-key-value = { path = "../key-value" } +spin-world = { path = "../world" } anyhow = "1.0" wit-bindgen-wasmtime = { workspace = true } -rusqlite = { version = "0.27.0", features = [ "bundled" ] } +rusqlite = { version = "0.29.0", features = [ "bundled" ] } rand = "0.8" once_cell = "1" \ No newline at end of file diff --git a/crates/sqlite/src/host_component.rs b/crates/sqlite/src/host_component.rs index f8fad9386d..2fde3f1230 100644 --- a/crates/sqlite/src/host_component.rs +++ b/crates/sqlite/src/host_component.rs @@ -7,7 +7,8 @@ use std::{ use once_cell::sync::OnceCell; use rusqlite::Connection; use spin_app::{AppComponent, DynamicHostComponent}; -use spin_core::{sqlite, HostComponent}; +use spin_core::HostComponent; +use spin_world::sqlite; use crate::SqliteImpl; diff --git a/crates/sqlite/src/lib.rs b/crates/sqlite/src/lib.rs index 92832223ba..a06d9d0b87 100644 --- a/crates/sqlite/src/lib.rs +++ b/crates/sqlite/src/lib.rs @@ -7,10 +7,8 @@ use std::{ use rusqlite::Connection; use spin_app::MetadataKey; -use spin_core::{ - async_trait, - sqlite::{self, Host}, -}; +use spin_core::async_trait; +use spin_world::sqlite::{self, Host}; pub use host_component::{ConnectionManager, DatabaseLocation, SqliteComponent, SqliteConnection}; use spin_key_value::table; @@ -36,14 +34,14 @@ impl SqliteImpl { self.allowed_databases = allowed_databases } - fn get_connection<'a>( - &'a self, + fn get_connection( + &self, connection: sqlite::Connection, - ) -> Result, sqlite::Error> { + ) -> Result, sqlite::Error> { Ok(self .connections .get(connection) - .ok_or_else(|| sqlite::Error::InvalidConnection)? + .ok_or(sqlite::Error::InvalidConnection)? .lock() .unwrap()) } @@ -54,9 +52,8 @@ impl Host for SqliteImpl { async fn open( &mut self, database: String, - ) -> anyhow::Result> { + ) -> anyhow::Result> { Ok(async { - println!("{database}"); if !self.allowed_databases.contains(&database) { return Err(sqlite::Error::AccessDenied); } @@ -67,9 +64,7 @@ impl Host for SqliteImpl { .ok_or(sqlite::Error::NoSuchDatabase)? .get_connection()?, ) - .map_err(|()| { - todo!("Create an error for when we reach capacity on number of databases") - }) + .map_err(|()| sqlite::Error::DatabaseFull) } .await) } @@ -100,47 +95,49 @@ impl Host for SqliteImpl { connection: sqlite::Connection, query: String, parameters: Vec, - ) -> anyhow::Result, sqlite::Error>> { + ) -> anyhow::Result> { Ok(async move { let conn = self.get_connection(connection)?; let mut statement = conn .prepare_cached(&query) .map_err(|e| sqlite::Error::Io(e.to_string()))?; + let columns = statement + .column_names() + .into_iter() + .map(ToOwned::to_owned) + .collect(); let rows = statement .query_map( rusqlite::params_from_iter(convert_data(parameters.into_iter())), |row| { let mut values = vec![]; for column in 0.. { - let name = row.as_ref().column_name(column); - if let Err(rusqlite::Error::InvalidColumnIndex(_)) = name { - break; - } - let name = name?.to_string(); let value = row.get::(column); if let Err(rusqlite::Error::InvalidColumnIndex(_)) = value { break; } let value = value?.0; - values.push(sqlite::ColumnValue { name, value }); + values.push(value); } - Ok(sqlite::Row { values }) + Ok(sqlite::RowResult { values }) }, ) .map_err(|e| sqlite::Error::Io(e.to_string()))?; - Ok(rows + let rows = rows .into_iter() .map(|r| r.map_err(|e| sqlite::Error::Io(e.to_string()))) - .collect::>()?) + .collect::>()?; + Ok(sqlite::QueryResult { columns, rows }) } .await) } - async fn close(&mut self, connection: spin_core::sqlite::Connection) -> anyhow::Result<()> { - Ok(async { + async fn close(&mut self, connection: sqlite::Connection) -> anyhow::Result<()> { + async { let _ = self.connections.remove(connection); } - .await) + .await; + Ok(()) } } diff --git a/docs/content/sips/000-sqlite.md b/docs/content/sips/000-sqlite.md index 7a01cf580a..91f4930c54 100644 --- a/docs/content/sips/000-sqlite.md +++ b/docs/content/sips/000-sqlite.md @@ -46,8 +46,14 @@ type connection = u32 // The set of errors which may be raised by functions in this interface variant error { - // The host does not recognize the database name requested. + // A database with the supplied name does not exist no-such-database, + // The requesting component does not have access to the specified database (which may or may not exist). + access-denied, + // The provided connection is not valid + invalid-connection, + // The database has reached its capacity + database-full, // Some implementation-specific error has occurred (e.g. I/O) io(string) } @@ -63,22 +69,25 @@ open: func(name: string) -> expected execute: func(conn: connection, statement: string, parameters: list) -> expected // Query data -query: func(conn: connection, query: string, parameters: list) -> expected, error> +query: func(conn: connection, query: string, parameters: list) -> expected // Close the specified `connection`. close: func(conn: connection) -// A database row -record row { - values: list, +// A result of a query +record query-result { + // The names of the columns retrieved in the query + columns: list, + // The row results each containing the values for all the columns for a given row + rows: list, } -// A single column's value -record column-value { - name: string, - value: value +// A set of values for each of the columns in a query-result +record row-result { + values: list } +// The values used in statements/queries and returned in query results variant value { integer(s64), real(float64), @@ -90,6 +99,12 @@ variant value { *Note: the pseudo-resource design was inspired by the interface of similar functions in [WASI preview 2](https://github.com/bytecodealliance/preview2-prototyping/blob/d56b8977a2b700432d1f7f84656d542f1d8854b0/wit/wasi.wit#L772-L794).* +#### Interface open questions + +* `row-result` can be very large. Should we provide some paging mechanism or a different API that allows for reading subsets of the returned data? + * Crossing the wit boundary could potentially be expensive if the results are large enough. Giving the user control of how they read that data could be helpful. +* Is there really a need for query *and* execute functions since at the end of the day, they are basically equivalent? + #### Database migrations Database tables typically require some sort of configuration in the form of database migrations to get table schemas into the correct state. While we could require the user to ensure that the database is in the correct state each time the trigger handler function is run, there are a few issues with this: @@ -105,13 +120,17 @@ There are several possible ways to address this issue such as: It should be noted that many of these options are not mutually exclusive and we could introduce more than one (perhaps starting with one option that will mostly be replaced later with a more generalized approach). -TODO: decide which of these (or another mechanism) to use +**TODO**: decide which of these (or another mechanism) to use #### Implementation requirements -TODO: Open questions: -* Assumed sqlite version -* Capacity limits +**TODO**: Open questions: +* Assumed sqlite version? + * Semantics may change slightly depending on the sqlite version. It's unlikely that we'll be able to match the exact versions between whatever sqlite implementation spin users, Fermyon Cloud, and the user (if they decide to create their own databases manually). Having some guidance on which versions are expected to work might make it easier to guide the user down the right path. +* Capacity limits? The following are different capacities we might want to control: + * The number of databases in total + * The number of rows in a database + * The size of certain row values (**question**: does sqlite or libsql impose any restrictions and do we just pass those on to the user?) #### Built-in local database @@ -134,9 +153,12 @@ Sqlite databases may be configured with `[sqlite_database.]` sect ```toml # The `default` config can be overridden [sqlite_database.default] -path = ".spin/sqlite_key_value.db" +path = ".spin/some-other-database.db" + +[sqlite_database.other] +path = ".spin/yet-another-database.db" ``` ## Future work -TODO +**TODO** diff --git a/sdk/rust/Cargo.toml b/sdk/rust/Cargo.toml index eca4d99c3d..a43f9b4e44 100644 --- a/sdk/rust/Cargo.toml +++ b/sdk/rust/Cargo.toml @@ -24,3 +24,4 @@ serde = {version = "1.0.163", optional = true} default = ["export-sdk-language"] export-sdk-language = [] json = ["dep:serde", "dep:serde_json"] +experimental-sqlite = [] diff --git a/sdk/rust/src/lib.rs b/sdk/rust/src/lib.rs index 02927d60bc..78125faa42 100644 --- a/sdk/rust/src/lib.rs +++ b/sdk/rust/src/lib.rs @@ -9,6 +9,7 @@ pub mod outbound_http; pub mod key_value; /// Sqlite +#[cfg(feature = "experimental-sqlite")] pub mod sqlite; /// Exports the procedural macros for writing handlers for Spin components. diff --git a/sdk/rust/src/sqlite.rs b/sdk/rust/src/sqlite.rs index 2695772a70..5bcba99fa0 100644 --- a/sdk/rust/src/sqlite.rs +++ b/sdk/rust/src/sqlite.rs @@ -5,12 +5,9 @@ use sqlite::Connection as RawConnection; /// Errors which may be raised by the methods of `Store` pub type Error = sqlite::Error; -/// -pub type Row = sqlite::Row; - -/// +/// A parameter used when executing a sqlite statement pub type DataTypeParam<'a> = sqlite::ValueParam<'a>; -/// +/// A single column's result from a database query pub type DataTypeResult = sqlite::ValueResult; /// Represents a store in which key value tuples may be placed @@ -24,38 +21,52 @@ impl Connection { } /// Execute a statement against the database - pub fn execute<'a>( + pub fn execute( &self, statement: &str, - parameters: &[sqlite::ValueParam<'a>], + parameters: &[sqlite::ValueParam<'_>], ) -> Result<(), Error> { sqlite::execute(self.0, statement, parameters)?; Ok(()) } /// Make a query against the database - pub fn query<'a>( + pub fn query( &self, query: &str, - parameters: &[DataTypeParam<'a>], - ) -> Result, Error> { + parameters: &[DataTypeParam<'_>], + ) -> Result { sqlite::query(self.0, query, parameters) } } -impl Row { - pub fn get<'a, T: TryFrom<&'a sqlite::ValueResult>>(&'a self, name: &str) -> Option { - self.values - .iter() - .find_map(|c| (c.name == name).then(|| (&c.value).try_into().ok())) - .flatten() +impl sqlite::QueryResult { + /// Get all the rows for this query result + pub fn rows<'a>(&'a self) -> impl Iterator> { + self.rows.iter().map(|r| Row { + columns: self.columns.as_slice(), + result: r, + }) } +} + +/// A database row result +pub struct Row<'a> { + columns: &'a [String], + result: &'a sqlite::RowResult, +} + +impl<'a> Row<'a> { + /// Get a value by its column name + pub fn get>(&self, column: &str) -> Option { + let i = self.columns.iter().position(|c| c == column)?; + self.result.get(i) + } +} - pub fn geti<'a, T: TryFrom<&'a sqlite::ValueResult>>(&'a self, index: usize) -> Option { - self.values - .get(index) - .map(|c| (&c.value).try_into().ok()) - .flatten() +impl sqlite::RowResult { + pub fn get<'a, T: TryFrom<&'a sqlite::ValueResult>>(&'a self, index: usize) -> Option { + self.values.get(index).and_then(|c| c.try_into().ok()) } } diff --git a/wit/ephemeral/sqlite.wit b/wit/ephemeral/sqlite.wit index 1bad9d3bc4..7708106b01 100644 --- a/wit/ephemeral/sqlite.wit +++ b/wit/ephemeral/sqlite.wit @@ -9,6 +9,8 @@ variant error { access-denied, // The provided connection is not valid invalid-connection, + // The database has reached its capacity + database-full, // Some implementation-specific error has occurred (e.g. I/O) io(string) } @@ -24,20 +26,22 @@ open: func(name: string) -> expected execute: func(conn: connection, statement: string, parameters: list) -> expected // Query data -query: func(conn: connection, query: string, parameters: list) -> expected, error> +query: func(conn: connection, query: string, parameters: list) -> expected // Close the specified `connection`. close: func(conn: connection) -// A database row -record row { - values: list, +// A result of a query +record query-result { + // The names of the columns retrieved in the query + columns: list, + // the row results each containing the values for all the columns for a given row + rows: list, } -// A single column's value -record column-value { - name: string, - value: value +// A set of values for each of the columns in a query-result +record row-result { + values: list } variant value { diff --git a/wit/preview2/sqlite.wit b/wit/preview2/sqlite.wit index b4f4258c16..750539c327 100644 --- a/wit/preview2/sqlite.wit +++ b/wit/preview2/sqlite.wit @@ -10,6 +10,8 @@ default interface sqlite { access-denied, // The provided connection is not valid invalid-connection, + // The database has reached its capacity + database-full, // Some implementation-specific error has occurred (e.g. I/O) io(string) } @@ -25,20 +27,22 @@ default interface sqlite { execute: func(conn: connection, statement: string, parameters: list) -> result<_, error> // Query data - query: func(conn: connection, query: string, parameters: list) -> result, error> + query: func(conn: connection, query: string, parameters: list) -> result // Close the specified `connection`. close: func(conn: connection) - // A database row - record row { - values: list, + // A result of a query + record query-result { + // The names of the columns retrieved in the query + columns: list, + // the row results each containing the values for all the columns for a given row + rows: list, } - // A single column's value - record column-value { - name: string, - value: value + // A set of values for each of the columns in a query-result + record row-result { + values: list } variant value { From 99f453041997447ce4f56b78ec6b8c54dbab06c9 Mon Sep 17 00:00:00 2001 From: Ryan Levick Date: Wed, 10 May 2023 11:51:58 +0200 Subject: [PATCH 10/16] Allow running migrations from spin up Signed-off-by: Ryan Levick --- crates/trigger/src/cli.rs | 5 ++++ crates/trigger/src/lib.rs | 6 ++++- crates/trigger/src/runtime_config/sqlite.rs | 29 ++++++++++++++++++--- sdk/rust/src/sqlite.rs | 2 +- 4 files changed, 37 insertions(+), 5 deletions(-) diff --git a/crates/trigger/src/cli.rs b/crates/trigger/src/cli.rs index 6b2241aff3..5ac8e99835 100644 --- a/crates/trigger/src/cli.rs +++ b/crates/trigger/src/cli.rs @@ -106,6 +106,10 @@ where #[clap(long = "key-value", parse(try_from_str = parse_kv))] key_values: Vec<(String, String)>, + /// Run a sqlite migration against the default database + #[clap(long = "sqlite-migration")] + sqlite_migrations: Vec, + #[clap(long = "help-args-only", hide = true)] pub help_args_only: bool, } @@ -136,6 +140,7 @@ where let init_data = crate::HostComponentInitData { kv: self.key_values.clone(), + migrations: self.sqlite_migrations.clone(), }; let loader = TriggerLoader::new(working_dir, self.allow_transient_write); diff --git a/crates/trigger/src/lib.rs b/crates/trigger/src/lib.rs index f63441739f..30a19a76db 100644 --- a/crates/trigger/src/lib.rs +++ b/crates/trigger/src/lib.rs @@ -125,7 +125,10 @@ impl TriggerExecutorBuilder { )?; self.loader.add_dynamic_host_component( &mut builder, - runtime_config::sqlite::build_component(&runtime_config)?, + runtime_config::sqlite::build_component( + &runtime_config, + &init_data.migrations, + )?, )?; self.loader.add_dynamic_host_component( &mut builder, @@ -158,6 +161,7 @@ impl TriggerExecutorBuilder { #[derive(Default)] // TODO: the implementation of Default is only for tests - would like to get rid of pub struct HostComponentInitData { kv: Vec<(String, String)>, + migrations: Vec, } /// Execution context for a TriggerExecutor executing a particular App. diff --git a/crates/trigger/src/runtime_config/sqlite.rs b/crates/trigger/src/runtime_config/sqlite.rs index 4b56a0516e..0827959cf6 100644 --- a/crates/trigger/src/runtime_config/sqlite.rs +++ b/crates/trigger/src/runtime_config/sqlite.rs @@ -1,4 +1,4 @@ -use std::{path::PathBuf, sync::Arc}; +use std::{collections::HashMap, path::PathBuf, sync::Arc}; use crate::runtime_config::RuntimeConfig; use anyhow::Context; @@ -8,15 +8,38 @@ use super::RuntimeConfigOpts; pub type SqliteDatabase = Arc; -pub(crate) fn build_component(runtime_config: &RuntimeConfig) -> anyhow::Result { - let databases = runtime_config +pub(crate) fn build_component( + runtime_config: &RuntimeConfig, + init_migrations: &[String], +) -> anyhow::Result { + let databases: HashMap<_, _> = runtime_config .sqlite_databases() .context("Failed to build sqlite component")? .into_iter() .collect(); + perform_migrations(init_migrations, &databases)?; Ok(SqliteComponent::new(databases)) } +fn perform_migrations( + init_migrations: &[String], + databases: &HashMap>, +) -> anyhow::Result<()> { + if !init_migrations.is_empty() { + if let Some(default) = databases.get("default") { + let c = default.get_connection().context( + "could not get connection to default database in order to perform migrations", + )?; + let c = c.lock().unwrap(); + for m in init_migrations { + c.execute(m, []) + .with_context(|| format!("failed to execute migration: '{m}'"))?; + } + } + } + Ok(()) +} + // Holds deserialized options from a `[sqlite_database.]` runtime config section. #[derive(Clone, Debug, serde::Deserialize)] #[serde(rename_all = "snake_case", tag = "type")] diff --git a/sdk/rust/src/sqlite.rs b/sdk/rust/src/sqlite.rs index 5bcba99fa0..7debcb7387 100644 --- a/sdk/rust/src/sqlite.rs +++ b/sdk/rust/src/sqlite.rs @@ -42,7 +42,7 @@ impl Connection { impl sqlite::QueryResult { /// Get all the rows for this query result - pub fn rows<'a>(&'a self) -> impl Iterator> { + pub fn rows(&self) -> impl Iterator> { self.rows.iter().map(|r| Row { columns: self.columns.as_slice(), result: r, From d6fee05f79a5b55e48a62e7e747d7851e289b24c Mon Sep 17 00:00:00 2001 From: Ryan Levick Date: Wed, 10 May 2023 12:20:06 +0200 Subject: [PATCH 11/16] Address small PR feedback Signed-off-by: Ryan Levick --- Cargo.lock | 1 + crates/loader/src/bindle/config.rs | 2 +- crates/sqlite/Cargo.toml | 3 +- crates/sqlite/src/lib.rs | 20 ++++------- crates/trigger/src/cli.rs | 6 ++-- crates/trigger/src/lib.rs | 7 ++-- crates/trigger/src/runtime_config/sqlite.rs | 16 ++++----- .../sips/{000-sqlite.md => 013-sqlite.md} | 26 +++++++++------ sdk/rust/src/sqlite.rs | 33 +++++++++++-------- 9 files changed, 59 insertions(+), 55 deletions(-) rename docs/content/sips/{000-sqlite.md => 013-sqlite.md} (82%) diff --git a/Cargo.lock b/Cargo.lock index 647c626d7a..15430c40ba 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5433,6 +5433,7 @@ dependencies = [ "spin-core", "spin-key-value", "spin-world", + "tokio", "wit-bindgen-wasmtime", ] diff --git a/crates/loader/src/bindle/config.rs b/crates/loader/src/bindle/config.rs index f22850a660..eb73d0d5cb 100644 --- a/crates/loader/src/bindle/config.rs +++ b/crates/loader/src/bindle/config.rs @@ -49,7 +49,7 @@ pub struct RawWasmConfig { pub allowed_http_hosts: Option>, /// Optional list of key-value stores the component is allowed to use. pub key_value_stores: Option>, - /// Optional list of sqlite databases the component is allowed to use. + /// Optional list of SQLite databases the component is allowed to use. pub sqlite_databases: Option>, /// Environment variables to be mapped inside the Wasm module at runtime. pub environment: Option>, diff --git a/crates/sqlite/Cargo.toml b/crates/sqlite/Cargo.toml index 486e1a94eb..4631e9f26c 100644 --- a/crates/sqlite/Cargo.toml +++ b/crates/sqlite/Cargo.toml @@ -12,4 +12,5 @@ anyhow = "1.0" wit-bindgen-wasmtime = { workspace = true } rusqlite = { version = "0.29.0", features = [ "bundled" ] } rand = "0.8" -once_cell = "1" \ No newline at end of file +once_cell = "1" +tokio = "1" \ No newline at end of file diff --git a/crates/sqlite/src/lib.rs b/crates/sqlite/src/lib.rs index a06d9d0b87..56421572ec 100644 --- a/crates/sqlite/src/lib.rs +++ b/crates/sqlite/src/lib.rs @@ -53,7 +53,7 @@ impl Host for SqliteImpl { &mut self, database: String, ) -> anyhow::Result> { - Ok(async { + Ok(tokio::task::block_in_place(|| { if !self.allowed_databases.contains(&database) { return Err(sqlite::Error::AccessDenied); } @@ -65,8 +65,7 @@ impl Host for SqliteImpl { .get_connection()?, ) .map_err(|()| sqlite::Error::DatabaseFull) - } - .await) + })) } async fn execute( @@ -75,7 +74,7 @@ impl Host for SqliteImpl { statement: String, parameters: Vec, ) -> anyhow::Result> { - Ok(async move { + Ok(tokio::task::block_in_place(|| { let conn = self.get_connection(connection)?; let mut statement = conn .prepare_cached(&statement) @@ -86,8 +85,7 @@ impl Host for SqliteImpl { ))) .map_err(|e| sqlite::Error::Io(e.to_string()))?; Ok(()) - } - .await) + })) } async fn query( @@ -96,7 +94,7 @@ impl Host for SqliteImpl { query: String, parameters: Vec, ) -> anyhow::Result> { - Ok(async move { + Ok(tokio::task::block_in_place(|| { let conn = self.get_connection(connection)?; let mut statement = conn .prepare_cached(&query) @@ -128,15 +126,11 @@ impl Host for SqliteImpl { .map(|r| r.map_err(|e| sqlite::Error::Io(e.to_string()))) .collect::>()?; Ok(sqlite::QueryResult { columns, rows }) - } - .await) + })) } async fn close(&mut self, connection: sqlite::Connection) -> anyhow::Result<()> { - async { - let _ = self.connections.remove(connection); - } - .await; + let _ = self.connections.remove(connection); Ok(()) } } diff --git a/crates/trigger/src/cli.rs b/crates/trigger/src/cli.rs index 5ac8e99835..f57f59907b 100644 --- a/crates/trigger/src/cli.rs +++ b/crates/trigger/src/cli.rs @@ -107,8 +107,8 @@ where key_values: Vec<(String, String)>, /// Run a sqlite migration against the default database - #[clap(long = "sqlite-migration")] - sqlite_migrations: Vec, + #[clap(long = "sqlite")] + sqlite_statements: Vec, #[clap(long = "help-args-only", hide = true)] pub help_args_only: bool, @@ -140,7 +140,7 @@ where let init_data = crate::HostComponentInitData { kv: self.key_values.clone(), - migrations: self.sqlite_migrations.clone(), + sqlite: self.sqlite_statements.clone(), }; let loader = TriggerLoader::new(working_dir, self.allow_transient_write); diff --git a/crates/trigger/src/lib.rs b/crates/trigger/src/lib.rs index 30a19a76db..0b76eeae24 100644 --- a/crates/trigger/src/lib.rs +++ b/crates/trigger/src/lib.rs @@ -125,10 +125,7 @@ impl TriggerExecutorBuilder { )?; self.loader.add_dynamic_host_component( &mut builder, - runtime_config::sqlite::build_component( - &runtime_config, - &init_data.migrations, - )?, + runtime_config::sqlite::build_component(&runtime_config, &init_data.sqlite)?, )?; self.loader.add_dynamic_host_component( &mut builder, @@ -161,7 +158,7 @@ impl TriggerExecutorBuilder { #[derive(Default)] // TODO: the implementation of Default is only for tests - would like to get rid of pub struct HostComponentInitData { kv: Vec<(String, String)>, - migrations: Vec, + sqlite: Vec, } /// Execution context for a TriggerExecutor executing a particular App. diff --git a/crates/trigger/src/runtime_config/sqlite.rs b/crates/trigger/src/runtime_config/sqlite.rs index 0827959cf6..9fc8b7b807 100644 --- a/crates/trigger/src/runtime_config/sqlite.rs +++ b/crates/trigger/src/runtime_config/sqlite.rs @@ -10,30 +10,30 @@ pub type SqliteDatabase = Arc; pub(crate) fn build_component( runtime_config: &RuntimeConfig, - init_migrations: &[String], + sqlite_statements: &[String], ) -> anyhow::Result { let databases: HashMap<_, _> = runtime_config .sqlite_databases() .context("Failed to build sqlite component")? .into_iter() .collect(); - perform_migrations(init_migrations, &databases)?; + execute_statements(sqlite_statements, &databases)?; Ok(SqliteComponent::new(databases)) } -fn perform_migrations( - init_migrations: &[String], +fn execute_statements( + statements: &[String], databases: &HashMap>, ) -> anyhow::Result<()> { - if !init_migrations.is_empty() { + if !statements.is_empty() { if let Some(default) = databases.get("default") { let c = default.get_connection().context( - "could not get connection to default database in order to perform migrations", + "could not get connection to default database in order to execute statements", )?; let c = c.lock().unwrap(); - for m in init_migrations { + for m in statements { c.execute(m, []) - .with_context(|| format!("failed to execute migration: '{m}'"))?; + .with_context(|| format!("failed to execute statement: '{m}'"))?; } } } diff --git a/docs/content/sips/000-sqlite.md b/docs/content/sips/013-sqlite.md similarity index 82% rename from docs/content/sips/000-sqlite.md rename to docs/content/sips/013-sqlite.md index 91f4930c54..42fe2624bc 100644 --- a/docs/content/sips/000-sqlite.md +++ b/docs/content/sips/013-sqlite.md @@ -1,4 +1,4 @@ -title = "SIP 000 - sqlite" +title = "SIP 013 - sqlite" template = "main" date = "2023-04-17:00:00Z" --- @@ -101,26 +101,32 @@ variant value { #### Interface open questions +**TODO**: answer these questions * `row-result` can be very large. Should we provide some paging mechanism or a different API that allows for reading subsets of the returned data? * Crossing the wit boundary could potentially be expensive if the results are large enough. Giving the user control of how they read that data could be helpful. * Is there really a need for query *and* execute functions since at the end of the day, they are basically equivalent? #### Database migrations -Database tables typically require some sort of configuration in the form of database migrations to get table schemas into the correct state. While we could require the user to ensure that the database is in the correct state each time the trigger handler function is run, there are a few issues with this: -* Schema tracking schemes (e.g., a "migrations" table) themselves require some sort of bootstrap step. -* This goes against the design principle of keeping components handler functions simple and single purpose. +Database tables typically require some sort of configuration in the form of database migrations to get table schemas into the correct state. To begin with a command line option supplied to `spin up` will be available for running any arbitrary SQL statements on start up and thus will be a place for users to run their migrations (i.e., `--sqlite "CREATE TABLE users..."`). It will be up to the user to provide idempotent statements such that running them multiple times does not produce unexpected results. -There are several possible ways to address this issue such as: +##### Future approaches + +This CLI approach (while useful) is likely to not be sufficient for more advanced use cases. There are several alternative ways to address the need for migrations: * Some mechanism for running spin components before others where the component receives the current schema version and decides whether or not to perform migrations. * The spin component could expose a current schema version as an exported value type so that an exported function would not need to called. If the exported schema version does not match the current schema version, an exported migrate function then gets called. * A spin component that gets called just after pre-initialization finishes. Similarly, this component would expose a schema version and have an exported migration function called when the exported schema version does not match the current schema version. -* Configuration option in spin.toml manifest for running arbitrary SQL instructions on start up (e.g., `sqlite.migration = "CREATE TABLE users..."`) -* Command line supplied option for running SQL instructions on start up (e.g., `--sqlite-migration "CREATE TABLE users..."`) +* Configuration option in spin.toml manifest for running arbitrary SQL instructions on start up (e.g., `sqlite.execute = "CREATE TABLE users..."`) It should be noted that many of these options are not mutually exclusive and we could introduce more than one (perhaps starting with one option that will mostly be replaced later with a more generalized approach). -**TODO**: decide which of these (or another mechanism) to use +For now, we punt on this question and only provide a mechanism for running SQL statements on start up through the CLI. + +##### Alternatives + +An alternative approach that was considered but ultimately reject was to require the user to ensure that the database is in the correct state each time their trigger handler function is run (i.e., provide no bespoke mechanism for migrations - the user only has access to the database when their component runs). There are a few issues with taking such an approach: +* Schema tracking schemes (e.g., a "migrations" table) themselves require some sort of bootstrap step. +* This goes against the design principle of keeping components handler functions simple and single purpose. #### Implementation requirements @@ -134,7 +140,7 @@ It should be noted that many of these options are not mutually exclusive and we #### Built-in local database -By default, each app will have its own default database which is independent of all other apps. For local apps, the database will be stored by default in a hidden `.spin` directory adjacent to the app's `spin.toml`. For remote apps: TODO +By default, each app will have its own default database which is independent of all other apps. For local apps, the database will be stored by default in a hidden `.spin` directory adjacent to the app's `spin.toml`. For remote apps, the user should be able to rely on a default database as well. It is up to the implementor how this remote database is exposed (i.e., by having a sqlite database on disk or by using a third party network enabled database like [Turso](https://turso.tech)). #### Granting access to components @@ -161,4 +167,4 @@ path = ".spin/yet-another-database.db" ## Future work -**TODO** +In the future we may want to try to unify the three SQL flavors we currently have support for (sqlite, mysql, and postgres). This may not be desirable if it becomes clear that unifying these three (fairly different) SQL flavors actually causes more confusion than is worthwhile. diff --git a/sdk/rust/src/sqlite.rs b/sdk/rust/src/sqlite.rs index 7debcb7387..17f9a02626 100644 --- a/sdk/rust/src/sqlite.rs +++ b/sdk/rust/src/sqlite.rs @@ -6,15 +6,20 @@ use sqlite::Connection as RawConnection; pub type Error = sqlite::Error; /// A parameter used when executing a sqlite statement -pub type DataTypeParam<'a> = sqlite::ValueParam<'a>; +pub type ValueParam<'a> = sqlite::ValueParam<'a>; /// A single column's result from a database query -pub type DataTypeResult = sqlite::ValueResult; +pub type ValueResult = sqlite::ValueResult; /// Represents a store in which key value tuples may be placed #[derive(Debug)] pub struct Connection(RawConnection); impl Connection { + /// Open a connection to the default database + pub fn open_default() -> Result { + Ok(Self(sqlite::open("default")?)) + } + /// Open a connection pub fn open(database: &str) -> Result { Ok(Self(sqlite::open(database)?)) @@ -34,7 +39,7 @@ impl Connection { pub fn query( &self, query: &str, - parameters: &[DataTypeParam<'_>], + parameters: &[ValueParam<'_>], ) -> Result { sqlite::query(self.0, query, parameters) } @@ -58,46 +63,46 @@ pub struct Row<'a> { impl<'a> Row<'a> { /// Get a value by its column name - pub fn get>(&self, column: &str) -> Option { + pub fn get>(&self, column: &str) -> Option { let i = self.columns.iter().position(|c| c == column)?; self.result.get(i) } } impl sqlite::RowResult { - pub fn get<'a, T: TryFrom<&'a sqlite::ValueResult>>(&'a self, index: usize) -> Option { + pub fn get<'a, T: TryFrom<&'a ValueResult>>(&'a self, index: usize) -> Option { self.values.get(index).and_then(|c| c.try_into().ok()) } } -impl<'a> TryFrom<&'a sqlite::ValueResult> for bool { +impl<'a> TryFrom<&'a ValueResult> for bool { type Error = (); - fn try_from(value: &'a sqlite::ValueResult) -> Result { + fn try_from(value: &'a ValueResult) -> Result { match value { - sqlite::ValueResult::Integer(i) => Ok(*i != 0), + ValueResult::Integer(i) => Ok(*i != 0), _ => Err(()), } } } -impl<'a> TryFrom<&'a sqlite::ValueResult> for u32 { +impl<'a> TryFrom<&'a ValueResult> for u32 { type Error = (); - fn try_from(value: &'a sqlite::ValueResult) -> Result { + fn try_from(value: &'a ValueResult) -> Result { match value { - sqlite::ValueResult::Integer(i) => Ok(*i as u32), + ValueResult::Integer(i) => Ok(*i as u32), _ => Err(()), } } } -impl<'a> TryFrom<&'a sqlite::ValueResult> for &'a str { +impl<'a> TryFrom<&'a ValueResult> for &'a str { type Error = (); - fn try_from(value: &'a sqlite::ValueResult) -> Result { + fn try_from(value: &'a ValueResult) -> Result { match value { - sqlite::ValueResult::Text(s) => Ok(s.as_str()), + ValueResult::Text(s) => Ok(s.as_str()), _ => Err(()), } } From 4db7f648161a1dd85c9a1b26f4d59558facd19cb Mon Sep 17 00:00:00 2001 From: Ryan Levick Date: Fri, 12 May 2023 12:02:17 +0200 Subject: [PATCH 12/16] Specify minimum capacity limits Signed-off-by: Ryan Levick --- docs/content/sips/013-sqlite.md | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/docs/content/sips/013-sqlite.md b/docs/content/sips/013-sqlite.md index 42fe2624bc..adeb2014e7 100644 --- a/docs/content/sips/013-sqlite.md +++ b/docs/content/sips/013-sqlite.md @@ -130,13 +130,16 @@ An alternative approach that was considered but ultimately reject was to require #### Implementation requirements +### Minimum capacity limits + +In addition to the above interface, we specify a few additional implementation requirements which guest components may rely on. At minimum, an conforming implementation must support: +* At least 10,000 rows +* Text and blob sizes of at least 1 megabyte +* Maximum number of columns of at least 32 + **TODO**: Open questions: * Assumed sqlite version? * Semantics may change slightly depending on the sqlite version. It's unlikely that we'll be able to match the exact versions between whatever sqlite implementation spin users, Fermyon Cloud, and the user (if they decide to create their own databases manually). Having some guidance on which versions are expected to work might make it easier to guide the user down the right path. -* Capacity limits? The following are different capacities we might want to control: - * The number of databases in total - * The number of rows in a database - * The size of certain row values (**question**: does sqlite or libsql impose any restrictions and do we just pass those on to the user?) #### Built-in local database From 326aced9b83917c61fcdf9ab58f1323394e69a6c Mon Sep 17 00:00:00 2001 From: Ryan Levick Date: Fri, 12 May 2023 12:21:11 +0200 Subject: [PATCH 13/16] Allow running sql from file Signed-off-by: Ryan Levick --- crates/trigger/src/runtime_config/sqlite.rs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/crates/trigger/src/runtime_config/sqlite.rs b/crates/trigger/src/runtime_config/sqlite.rs index 9fc8b7b807..bcd519c984 100644 --- a/crates/trigger/src/runtime_config/sqlite.rs +++ b/crates/trigger/src/runtime_config/sqlite.rs @@ -32,8 +32,16 @@ fn execute_statements( )?; let c = c.lock().unwrap(); for m in statements { - c.execute(m, []) - .with_context(|| format!("failed to execute statement: '{m}'"))?; + if let Some(file) = m.strip_prefix('@') { + let sql = std::fs::read_to_string(file).with_context(|| { + format!("could not read file '{file}' containing sql statements") + })?; + c.execute_batch(&sql) + .with_context(|| format!("failed to execute sql from file '{file}'"))?; + } else { + c.execute(m, []) + .with_context(|| format!("failed to execute statement: '{m}'"))?; + } } } } From b9db2cace51925db8a571493682bc355a7a25f80 Mon Sep 17 00:00:00 2001 From: Ryan Levick Date: Tue, 16 May 2023 14:30:54 +0200 Subject: [PATCH 14/16] remove experimental feature flag Signed-off-by: Ryan Levick --- sdk/rust/Cargo.toml | 1 - sdk/rust/src/lib.rs | 1 - 2 files changed, 2 deletions(-) diff --git a/sdk/rust/Cargo.toml b/sdk/rust/Cargo.toml index a43f9b4e44..eca4d99c3d 100644 --- a/sdk/rust/Cargo.toml +++ b/sdk/rust/Cargo.toml @@ -24,4 +24,3 @@ serde = {version = "1.0.163", optional = true} default = ["export-sdk-language"] export-sdk-language = [] json = ["dep:serde", "dep:serde_json"] -experimental-sqlite = [] diff --git a/sdk/rust/src/lib.rs b/sdk/rust/src/lib.rs index 78125faa42..02927d60bc 100644 --- a/sdk/rust/src/lib.rs +++ b/sdk/rust/src/lib.rs @@ -9,7 +9,6 @@ pub mod outbound_http; pub mod key_value; /// Sqlite -#[cfg(feature = "experimental-sqlite")] pub mod sqlite; /// Exports the procedural macros for writing handlers for Spin components. From 85b93f830d39d94c32896bc8d199546d332fb919 Mon Sep 17 00:00:00 2001 From: Ryan Levick Date: Wed, 17 May 2023 15:14:43 +0200 Subject: [PATCH 15/16] Remove query Signed-off-by: Ryan Levick --- Cargo.lock | 8 ++++---- crates/key-value/Cargo.toml | 2 +- crates/sqlite/src/lib.rs | 20 -------------------- sdk/rust/src/sqlite.rs | 12 +----------- wit/ephemeral/sqlite.wit | 5 +---- wit/preview2/sqlite.wit | 7 ++----- 6 files changed, 9 insertions(+), 45 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 15430c40ba..f29bf1db5f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2341,7 +2341,7 @@ dependencies = [ [[package]] name = "host" version = "0.0.0" -source = "git+https://github.com/fermyon/spin-componentize?branch=sqlite#cad37ef9bd846c1c010de631e7c8d2657397acde" +source = "git+https://github.com/fermyon/spin-componentize?branch=sqlite#75c34a53a429861e1e933933b844e55051356e56" dependencies = [ "anyhow", "async-trait", @@ -5155,7 +5155,7 @@ dependencies = [ [[package]] name = "spin-componentize" version = "0.1.0" -source = "git+https://github.com/fermyon/spin-componentize?branch=sqlite#cad37ef9bd846c1c010de631e7c8d2657397acde" +source = "git+https://github.com/fermyon/spin-componentize?branch=sqlite#75c34a53a429861e1e933933b844e55051356e56" dependencies = [ "anyhow", "wasm-encoder 0.26.0", @@ -6371,7 +6371,7 @@ checksum = "9c8d87e72b64a3b4db28d11ce29237c246188f4f51057d65a7eab63b7987e423" [[package]] name = "wasi-cap-std-sync" version = "0.0.0" -source = "git+https://github.com/fermyon/spin-componentize?branch=sqlite#cad37ef9bd846c1c010de631e7c8d2657397acde" +source = "git+https://github.com/fermyon/spin-componentize?branch=sqlite#75c34a53a429861e1e933933b844e55051356e56" dependencies = [ "anyhow", "async-trait", @@ -6419,7 +6419,7 @@ dependencies = [ [[package]] name = "wasi-common" version = "0.0.0" -source = "git+https://github.com/fermyon/spin-componentize?branch=sqlite#cad37ef9bd846c1c010de631e7c8d2657397acde" +source = "git+https://github.com/fermyon/spin-componentize?branch=sqlite#75c34a53a429861e1e933933b844e55051356e56" dependencies = [ "anyhow", "async-trait", diff --git a/crates/key-value/Cargo.toml b/crates/key-value/Cargo.toml index b04977664c..696851bfd2 100644 --- a/crates/key-value/Cargo.toml +++ b/crates/key-value/Cargo.toml @@ -9,7 +9,7 @@ doctest = false [dependencies] anyhow = "1.0" -tokio = { version = "1", features = [ "macros" ] } +tokio = { version = "1", features = [ "macros", "sync" ] } spin-app = { path = "../app" } spin-core = { path = "../core" } spin-world = { path = "../world" } diff --git a/crates/sqlite/src/lib.rs b/crates/sqlite/src/lib.rs index 56421572ec..2a67d8ecb8 100644 --- a/crates/sqlite/src/lib.rs +++ b/crates/sqlite/src/lib.rs @@ -69,26 +69,6 @@ impl Host for SqliteImpl { } async fn execute( - &mut self, - connection: sqlite::Connection, - statement: String, - parameters: Vec, - ) -> anyhow::Result> { - Ok(tokio::task::block_in_place(|| { - let conn = self.get_connection(connection)?; - let mut statement = conn - .prepare_cached(&statement) - .map_err(|e| sqlite::Error::Io(e.to_string()))?; - statement - .execute(rusqlite::params_from_iter(convert_data( - parameters.into_iter(), - ))) - .map_err(|e| sqlite::Error::Io(e.to_string()))?; - Ok(()) - })) - } - - async fn query( &mut self, connection: sqlite::Connection, query: String, diff --git a/sdk/rust/src/sqlite.rs b/sdk/rust/src/sqlite.rs index 17f9a02626..bb3090d614 100644 --- a/sdk/rust/src/sqlite.rs +++ b/sdk/rust/src/sqlite.rs @@ -27,21 +27,11 @@ impl Connection { /// Execute a statement against the database pub fn execute( - &self, - statement: &str, - parameters: &[sqlite::ValueParam<'_>], - ) -> Result<(), Error> { - sqlite::execute(self.0, statement, parameters)?; - Ok(()) - } - - /// Make a query against the database - pub fn query( &self, query: &str, parameters: &[ValueParam<'_>], ) -> Result { - sqlite::query(self.0, query, parameters) + sqlite::execute(self.0, query, parameters) } } diff --git a/wit/ephemeral/sqlite.wit b/wit/ephemeral/sqlite.wit index 7708106b01..8bf9384b55 100644 --- a/wit/ephemeral/sqlite.wit +++ b/wit/ephemeral/sqlite.wit @@ -23,10 +23,7 @@ variant error { open: func(name: string) -> expected // Execute a statement -execute: func(conn: connection, statement: string, parameters: list) -> expected - -// Query data -query: func(conn: connection, query: string, parameters: list) -> expected +execute: func(conn: connection, statement: string, parameters: list) -> expected // Close the specified `connection`. close: func(conn: connection) diff --git a/wit/preview2/sqlite.wit b/wit/preview2/sqlite.wit index 750539c327..a539b905e8 100644 --- a/wit/preview2/sqlite.wit +++ b/wit/preview2/sqlite.wit @@ -23,11 +23,8 @@ default interface sqlite { // `error::no-such-database` will be raised if the `name` is not recognized. open: func(database: string) -> result - // Execute a statement - execute: func(conn: connection, statement: string, parameters: list) -> result<_, error> - - // Query data - query: func(conn: connection, query: string, parameters: list) -> result + // Execute a statement returning back data if there is any + execute: func(conn: connection, statement: string, parameters: list) -> result // Close the specified `connection`. close: func(conn: connection) From c5155599b756f756a41cad323e5d4dd2323bb3fc Mon Sep 17 00:00:00 2001 From: Ryan Levick Date: Mon, 22 May 2023 13:23:06 +0200 Subject: [PATCH 16/16] Update to latest version of spin-componentize Signed-off-by: Ryan Levick --- Cargo.lock | 8 ++++---- Cargo.toml | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f29bf1db5f..f8480a4c34 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2341,7 +2341,7 @@ dependencies = [ [[package]] name = "host" version = "0.0.0" -source = "git+https://github.com/fermyon/spin-componentize?branch=sqlite#75c34a53a429861e1e933933b844e55051356e56" +source = "git+https://github.com/fermyon/spin-componentize?rev=b6d42fe41e5690844a661deb631d996a2b49debc#b6d42fe41e5690844a661deb631d996a2b49debc" dependencies = [ "anyhow", "async-trait", @@ -5155,7 +5155,7 @@ dependencies = [ [[package]] name = "spin-componentize" version = "0.1.0" -source = "git+https://github.com/fermyon/spin-componentize?branch=sqlite#75c34a53a429861e1e933933b844e55051356e56" +source = "git+https://github.com/fermyon/spin-componentize?rev=b6d42fe41e5690844a661deb631d996a2b49debc#b6d42fe41e5690844a661deb631d996a2b49debc" dependencies = [ "anyhow", "wasm-encoder 0.26.0", @@ -6371,7 +6371,7 @@ checksum = "9c8d87e72b64a3b4db28d11ce29237c246188f4f51057d65a7eab63b7987e423" [[package]] name = "wasi-cap-std-sync" version = "0.0.0" -source = "git+https://github.com/fermyon/spin-componentize?branch=sqlite#75c34a53a429861e1e933933b844e55051356e56" +source = "git+https://github.com/fermyon/spin-componentize?rev=b6d42fe41e5690844a661deb631d996a2b49debc#b6d42fe41e5690844a661deb631d996a2b49debc" dependencies = [ "anyhow", "async-trait", @@ -6419,7 +6419,7 @@ dependencies = [ [[package]] name = "wasi-common" version = "0.0.0" -source = "git+https://github.com/fermyon/spin-componentize?branch=sqlite#75c34a53a429861e1e933933b844e55051356e56" +source = "git+https://github.com/fermyon/spin-componentize?rev=b6d42fe41e5690844a661deb631d996a2b49debc#b6d42fe41e5690844a661deb631d996a2b49debc" dependencies = [ "anyhow", "async-trait", diff --git a/Cargo.toml b/Cargo.toml index 3fe47cd539..cf584d7f80 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -115,10 +115,10 @@ tracing = { version = "0.1", features = ["log"] } wasmtime-wasi = { version = "8.0.1", features = ["tokio"] } wasi-common-preview1 = { package = "wasi-common", version = "8.0.1" } wasmtime = { version = "8.0.1", features = ["component-model"] } -spin-componentize = { git = "https://github.com/fermyon/spin-componentize", branch = "sqlite" } -wasi-host = { package = "host", git = "https://github.com/fermyon/spin-componentize", branch = "sqlite" } -wasi-common = { git = "https://github.com/fermyon/spin-componentize", branch = "sqlite" } -wasi-cap-std-sync = { git = "https://github.com/fermyon/spin-componentize", branch = "sqlite" } +spin-componentize = { git = "https://github.com/fermyon/spin-componentize", rev = "b6d42fe41e5690844a661deb631d996a2b49debc" } +wasi-host = { package = "host", git = "https://github.com/fermyon/spin-componentize", rev = "b6d42fe41e5690844a661deb631d996a2b49debc" } +wasi-common = { git = "https://github.com/fermyon/spin-componentize", rev = "b6d42fe41e5690844a661deb631d996a2b49debc" } +wasi-cap-std-sync = { git = "https://github.com/fermyon/spin-componentize", rev = "b6d42fe41e5690844a661deb631d996a2b49debc" } [workspace.dependencies.bindle] git = "https://github.com/fermyon/bindle"