commit c413779739673f88925e8dbf1d45cf4cd8c7fa0b parent 3f96391734e948f576b99ef1a5d6c92cdb9844dc Author: triesap <tyson@radroots.org> Date: Wed, 15 Jul 2026 03:26:35 +0000 sqlite: remove rusqlite runtime ownership - port geocoder storage and asset validation to bundled SQLx SQLite - align native SQL executor consumers with SqlxSqliteExecutor - harden protocol coverage and workspace coverage policy for release preflight - verify workspace check, tests, clippy, contracts, release preflight, and rusqlite absence Diffstat:
27 files changed, 789 insertions(+), 534 deletions(-)
diff --git a/Cargo.lock b/Cargo.lock @@ -1805,18 +1805,6 @@ dependencies = [ ] [[package]] -name = "fallible-iterator" -version = "0.3.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2acce4a10f12dc2fb14a218589d4f1f62ef011b2d0cc4b3cb1bba8e94da14649" - -[[package]] -name = "fallible-streaming-iterator" -version = "0.1.9" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7360491ce676a36bf9bb3c56c1aa791658183a54d2744120f27285738d90465a" - -[[package]] name = "fastrand" version = "2.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" @@ -4353,12 +4341,13 @@ dependencies = [ name = "radroots_geocoder" version = "0.1.0-alpha.2" dependencies = [ + "futures-executor", "hex", "radroots_runtime_paths", "reqwest", - "rusqlite", "serde", "sha2", + "sqlx", "tempfile", "thiserror 1.0.69", "url", @@ -5238,30 +5227,6 @@ dependencies = [ ] [[package]] -name = "rsqlite-vfs" -version = "0.1.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c51c9ae4df8a7fba42103df5c621fa3c37eccf3a3c650879e90fc48b11cc192c" -dependencies = [ - "hashbrown 0.16.1", - "thiserror 2.0.18", -] - -[[package]] -name = "rusqlite" -version = "0.39.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a0d2b0146dd9661bf67bb107c0bb2a55064d556eeb3fc314151b957f313bcd4e" -dependencies = [ - "bitflags 2.11.0", - "fallible-iterator", - "fallible-streaming-iterator", - "libsqlite3-sys", - "smallvec", - "sqlite-wasm-rs", -] - -[[package]] name = "rust-ini" version = "0.20.0" source = "registry+https://github.com/rust-lang/crates.io-index" @@ -6932,18 +6897,6 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3a0219bd7d979d58245a4f41f695e1ac9f8befdffadd7f61f1bae9e39abc6620" [[package]] -name = "sqlite-wasm-rs" -version = "0.5.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "dc3efc0da82635d7e1ced0053bbbfa8c7ab9645d0bf36ceb4f7127bb85315d75" -dependencies = [ - "cc", - "js-sys", - "rsqlite-vfs", - "wasm-bindgen", -] - -[[package]] name = "sqlx" version = "0.9.0" source = "registry+https://github.com/rust-lang/crates.io-index" diff --git a/Cargo.toml b/Cargo.toml @@ -198,7 +198,5 @@ uuid = { version = "1.22.0", features = ["v4", "v7"] } x509-parser = { version = "0.17", default-features = false } zstd = { version = "0.13", default-features = false } zeroize = { version = "1" } -rusqlite = { version = "0.39.0", default-features = false } - [patch.crates-io] libsqlite3-sys = { path = "crates/libsqlite3_sys_3_53_3" } diff --git a/crates/event_store/src/store.rs b/crates/event_store/src/store.rs @@ -1197,8 +1197,7 @@ mod tests { let missing = sqlx::query("SELECT COUNT(*) FROM event_envelopes") .fetch_one(store.pool()) .await - .err() - .expect("table should be removed"); + .expect_err("table should be removed"); assert!(missing.to_string().contains("event_envelopes")); } diff --git a/crates/geocoder/Cargo.toml b/crates/geocoder/Cargo.toml @@ -17,11 +17,12 @@ test-fixture-geonames-asset = [] [dependencies] hex = { workspace = true } +futures-executor = { workspace = true } radroots_runtime_paths = { workspace = true } reqwest = { workspace = true, features = ["blocking", "rustls-tls"] } -rusqlite = { workspace = true, features = ["bundled"] } serde = { workspace = true, features = ["derive"] } sha2 = { workspace = true } +sqlx = { workspace = true, features = ["derive", "sqlite-bundled"] } tempfile = { workspace = true } thiserror = { workspace = true } url = { workspace = true } diff --git a/crates/geocoder/src/asset.rs b/crates/geocoder/src/asset.rs @@ -3,8 +3,9 @@ use std::io::{Read, Write}; use std::path::{Path, PathBuf}; use radroots_runtime_paths::default_shared_geonames_database_path_from_cache_root; -use rusqlite::{Connection, OpenFlags}; use sha2::{Digest, Sha256}; +use sqlx::Connection; +use sqlx::sqlite::{SqliteConnectOptions, SqliteConnection}; use url::Url; use crate::GeocoderError; @@ -273,14 +274,14 @@ fn install_geonames_asset_bytes( #[cfg_attr(coverage_nightly, coverage(off))] fn validate_sqlite_integrity_and_schema(path: &Path) -> Result<(), GeocoderError> { - let conn = - Connection::open_with_flags(path, OpenFlags::SQLITE_OPEN_READ_ONLY).map_err(|error| { - GeocoderError::InvalidAssetSqlite { - path: path.to_path_buf(), - detail: error.to_string(), - } - })?; - validate_sqlite_integrity(path, &conn)?; + let mut conn = futures_executor::block_on(SqliteConnection::connect_with( + &SqliteConnectOptions::new().filename(path).read_only(true), + )) + .map_err(|error| GeocoderError::InvalidAssetSqlite { + path: path.to_path_buf(), + detail: error.to_string(), + })?; + validate_sqlite_integrity(path, &mut conn)?; for query in [ "SELECT id, name FROM countries LIMIT 1", "SELECT country_id, id, name FROM admin1 LIMIT 1", @@ -288,7 +289,7 @@ fn validate_sqlite_integrity_and_schema(path: &Path) -> Result<(), GeocoderError "SELECT feature_id, latitude, longitude FROM coordinates LIMIT 1", "SELECT id, name, admin1_id, admin1_name, country_id, country_name, latitude, longitude FROM geonames LIMIT 1", ] { - conn.prepare(query) + futures_executor::block_on(sqlx::query(query).fetch_optional(&mut conn)) .map(|_| ()) .map_err(|error| GeocoderError::InvalidAssetSchema { path: path.to_path_buf(), @@ -299,25 +300,17 @@ fn validate_sqlite_integrity_and_schema(path: &Path) -> Result<(), GeocoderError } #[cfg_attr(coverage_nightly, coverage(off))] -fn validate_sqlite_integrity(path: &Path, conn: &Connection) -> Result<(), GeocoderError> { - let mut stmt = conn.prepare("PRAGMA integrity_check").map_err(|error| { - GeocoderError::InvalidAssetSqlite { - path: path.to_path_buf(), - detail: error.to_string(), - } +fn validate_sqlite_integrity( + path: &Path, + conn: &mut SqliteConnection, +) -> Result<(), GeocoderError> { + let results = futures_executor::block_on( + sqlx::query_scalar::<_, String>("PRAGMA integrity_check").fetch_all(conn), + ) + .map_err(|error| GeocoderError::InvalidAssetSqlite { + path: path.to_path_buf(), + detail: error.to_string(), })?; - let rows = stmt - .query_map([], |row| row.get::<_, String>(0)) - .map_err(|error| GeocoderError::InvalidAssetSqlite { - path: path.to_path_buf(), - detail: error.to_string(), - })?; - let results = - rows.collect::<Result<Vec<_>, _>>() - .map_err(|error| GeocoderError::InvalidAssetSqlite { - path: path.to_path_buf(), - detail: error.to_string(), - })?; if results.as_slice() == ["ok"] { return Ok(()); } @@ -388,8 +381,9 @@ mod tests { use std::fs; use std::path::{Path, PathBuf}; - use rusqlite::Connection; use sha2::Digest; + use sqlx::Connection; + use sqlx::sqlite::{SqliteConnectOptions, SqliteConnection}; use super::{ GEONAMES_ASSET_HOST, GeoNamesAssetFetcher, GeoNamesAssetSpec, GeoNamesAssetState, @@ -573,38 +567,68 @@ mod tests { } fn build_fixture_database(path: &Path) { - let conn = Connection::open(path).expect("open fixture db"); - conn.execute_batch(FIXTURE_SCHEMA).expect("fixture schema"); - conn.execute( - "INSERT INTO countries (id, name) VALUES (?1, ?2)", - ("FX", "Fixtureland"), + let mut conn = open_test_path_connection(path); + execute_batch(&mut conn, FIXTURE_SCHEMA); + futures_executor::block_on( + sqlx::query("INSERT INTO countries (id, name) VALUES (?, ?)") + .bind("FX") + .bind("Fixtureland") + .execute(&mut conn), ) .expect("insert country"); - conn.execute( - "INSERT INTO admin1 (country_id, id, name) VALUES (?1, ?2, ?3)", - ("FX", 1_i64, "Fixture Region"), + futures_executor::block_on( + sqlx::query("INSERT INTO admin1 (country_id, id, name) VALUES (?, ?, ?)") + .bind("FX") + .bind(1_i64) + .bind("Fixture Region") + .execute(&mut conn), ) .expect("insert admin1"); - conn.execute( - "INSERT INTO features (id, name, country_id, admin1_id) VALUES (?1, ?2, ?3, ?4)", - (1_i64, "Fixture Town", "FX", 1_i64), + futures_executor::block_on( + sqlx::query( + "INSERT INTO features (id, name, country_id, admin1_id) VALUES (?, ?, ?, ?)", + ) + .bind(1_i64) + .bind("Fixture Town") + .bind("FX") + .bind(1_i64) + .execute(&mut conn), ) .expect("insert feature"); - conn.execute( - "INSERT INTO coordinates (feature_id, latitude, longitude) VALUES (?1, ?2, ?3)", - (1_i64, 12.25_f64, -34.5_f64), + futures_executor::block_on( + sqlx::query( + "INSERT INTO coordinates (feature_id, latitude, longitude) VALUES (?, ?, ?)", + ) + .bind(1_i64) + .bind(12.25_f64) + .bind(-34.5_f64) + .execute(&mut conn), ) .expect("insert coordinates"); } - fn build_bad_schema_database(path: &PathBuf) { - let conn = Connection::open(path).expect("open bad schema db"); - conn.execute_batch( + fn build_bad_schema_database(path: &Path) { + let mut conn = open_test_path_connection(path); + execute_batch( + &mut conn, r#" CREATE TABLE countries(id TEXT, name TEXT); "#, - ) - .expect("bad schema"); + ); + } + + fn open_test_path_connection(path: &Path) -> SqliteConnection { + futures_executor::block_on(SqliteConnection::connect_with( + &SqliteConnectOptions::new() + .filename(path) + .create_if_missing(true), + )) + .expect("open fixture database") + } + + fn execute_batch(conn: &mut SqliteConnection, sql: &str) { + futures_executor::block_on(sqlx::raw_sql(sqlx::AssertSqlSafe(sql)).execute(conn)) + .expect("execute fixture sql batch"); } const FIXTURE_SCHEMA: &str = r#" diff --git a/crates/geocoder/src/error.rs b/crates/geocoder/src/error.rs @@ -3,7 +3,9 @@ use thiserror::Error; #[derive(Debug, Error)] pub enum GeocoderError { #[error("sqlite error: {0}")] - Sqlite(#[from] rusqlite::Error), + Sqlite(#[from] sqlx::Error), + #[error("sqlite connection lock is unavailable")] + SqliteConnectionLockUnavailable, #[error("io error: {0}")] Io(#[from] std::io::Error), #[error("invalid GeoNames asset URL {url}")] diff --git a/crates/geocoder/src/geocoder.rs b/crates/geocoder/src/geocoder.rs @@ -5,20 +5,23 @@ use crate::model::{ GeocoderLocalityLookup, GeocoderLocalityQuery, GeocoderPoint, GeocoderReverseOptions, GeocoderReverseResult, }; -use rusqlite::{Connection, OpenFlags, named_params}; use std::io::Write; use std::path::Path; +use std::sync::Mutex; + +use sqlx::sqlite::{SqliteConnectOptions, SqliteConnection, SqliteRow}; +use sqlx::{Connection as _, Row}; pub struct Geocoder { - conn: Connection, + conn: Mutex<SqliteConnection>, _temp_path: Option<tempfile::TempPath>, } impl Geocoder { pub fn open_path<P: AsRef<Path>>(path: P) -> Result<Self, GeocoderError> { - let conn = Connection::open_with_flags(path, OpenFlags::SQLITE_OPEN_READ_ONLY)?; + let conn = open_read_only_connection(path)?; Ok(Self { - conn, + conn: Mutex::new(conn), _temp_path: None, }) } @@ -28,9 +31,9 @@ impl Geocoder { temp.as_file_mut().write_all(bytes)?; let temp_path = temp.into_temp_path(); let path: &Path = temp_path.as_ref(); - let conn = Connection::open_with_flags(path, OpenFlags::SQLITE_OPEN_READ_ONLY)?; + let conn = open_read_only_connection(path)?; Ok(Self { - conn, + conn: Mutex::new(conn), _temp_path: Some(temp_path), }) } @@ -50,8 +53,9 @@ impl Geocoder { ) -> Result<Vec<GeocoderReverseResult>, GeocoderError> { let options = options.unwrap_or_default(); let lng_weight = point.lat.to_radians().cos().powi(2); - let mut stmt = self.conn.prepare( - r#" + let rows = self.with_connection(|conn| { + let query = sqlx::query( + r#" SELECT g.id, g.name, @@ -64,27 +68,38 @@ impl Geocoder { FROM geonames AS g JOIN coordinates AS c ON g.id = c.feature_id - WHERE c.latitude BETWEEN :lat - :degree_offset AND :lat + :degree_offset - AND c.longitude BETWEEN :lng - :degree_offset AND :lng + :degree_offset + WHERE c.latitude BETWEEN ? - ? AND ? + ? + AND c.longitude BETWEEN ? - ? AND ? + ? ORDER BY - ((:lat - c.latitude) * (:lat - c.latitude)) - + ((:lng - c.longitude) * (:lng - c.longitude) * :lng_weight) ASC - LIMIT :limit + ((? - c.latitude) * (? - c.latitude)) + + ((? - c.longitude) * (? - c.longitude) * ?) ASC + LIMIT ? "#, - )?; - let params = named_params! { - ":lat": point.lat, - ":lng": point.lng, - ":degree_offset": options.degree_offset, - ":lng_weight": lng_weight, - ":limit": options.limit as i64, - }; - collect_mapped_rows(&mut stmt, params, map_reverse_row) + ) + .bind(point.lat) + .bind(options.degree_offset) + .bind(point.lat) + .bind(options.degree_offset) + .bind(point.lng) + .bind(options.degree_offset) + .bind(point.lng) + .bind(options.degree_offset) + .bind(point.lat) + .bind(point.lat) + .bind(point.lng) + .bind(point.lng) + .bind(lng_weight) + .bind(options.limit as i64); + futures_executor::block_on(query.fetch_all(conn)).map_err(GeocoderError::from) + })?; + collect_mapped_rows(rows, map_reverse_row) } pub fn country(&self, country_id: &str) -> Result<Vec<GeocoderReverseResult>, GeocoderError> { - let mut stmt = self.conn.prepare( - r#" + let rows = self.with_connection(|conn| { + futures_executor::block_on( + sqlx::query( + r#" SELECT id, name, @@ -95,20 +110,23 @@ impl Geocoder { latitude, longitude FROM geonames - WHERE country_id = :country_id + WHERE country_id = ? ORDER BY id ASC "#, - )?; - collect_mapped_rows( - &mut stmt, - named_params! { ":country_id": country_id }, - map_reverse_row, - ) + ) + .bind(country_id) + .fetch_all(conn), + ) + .map_err(GeocoderError::from) + })?; + collect_mapped_rows(rows, map_reverse_row) } pub fn country_list(&self) -> Result<Vec<GeocoderCountryListResult>, GeocoderError> { - let mut stmt = self.conn.prepare( - r#" + let rows = self.with_connection(|conn| { + futures_executor::block_on( + sqlx::query( + r#" SELECT country_id, country_name, @@ -118,19 +136,23 @@ impl Geocoder { GROUP BY country_id, country_name ORDER BY country_id ASC "#, - )?; - collect_mapped_rows(&mut stmt, [], |row| { + ) + .fetch_all(conn), + ) + .map_err(GeocoderError::from) + })?; + collect_mapped_rows(rows, |row| { Ok(GeocoderCountryListResult { - country_id: row.get("country_id")?, - country: row.get("country_name")?, - lat: row.get("latitude_c")?, - lng: row.get("longitude_c")?, + country_id: required_string(row, "country_id")?, + country: row.try_get("country_name")?, + lat: required_f64(row, "latitude_c")?, + lng: required_f64(row, "longitude_c")?, }) }) } pub fn country_center(&self, country_id: &str) -> Result<GeocoderPoint, GeocoderError> { - finalize_country_center(country_center_impl(&self.conn, country_id), country_id) + finalize_country_center(country_center_impl(self, country_id), country_id) } pub fn locality( @@ -168,8 +190,9 @@ impl Geocoder { return Ok(GeocoderLocalityLookup::NoMatch); }; let country = normalize_optional_name(country); - let mut stmt = self.conn.prepare( - r#" + let rows = self.with_connection(|conn| { + let query = sqlx::query( + r#" SELECT id, name, @@ -180,11 +203,11 @@ impl Geocoder { latitude, longitude FROM geonames - WHERE lower(name) = :locality + WHERE lower(name) = ? AND ( - :country IS NULL - OR lower(country_id) = :country - OR lower(country_name) = :country + ? IS NULL + OR lower(country_id) = ? + OR lower(country_name) = ? ) ORDER BY lower(name) ASC, @@ -194,15 +217,14 @@ impl Geocoder { coalesce(admin1_id, -1) ASC, id ASC "#, - )?; - let candidates = collect_mapped_rows( - &mut stmt, - named_params! { - ":locality": locality, - ":country": country, - }, - map_locality_candidate_row, - )?; + ) + .bind(locality) + .bind(country.as_deref()) + .bind(country.as_deref()) + .bind(country.as_deref()); + futures_executor::block_on(query.fetch_all(conn)).map_err(GeocoderError::from) + })?; + let candidates = collect_mapped_rows(rows, map_locality_candidate_row)?; let region = normalize_optional_name(region); let candidates = candidates .into_iter() @@ -212,8 +234,10 @@ impl Geocoder { } fn locality_by_feature_id(&self, id: i64) -> Result<GeocoderLocalityLookup, GeocoderError> { - let mut stmt = self.conn.prepare( - r#" + let rows = self.with_connection(|conn| { + futures_executor::block_on( + sqlx::query( + r#" SELECT id, name, @@ -224,19 +248,29 @@ impl Geocoder { latitude, longitude FROM geonames - WHERE id = :id + WHERE id = ? LIMIT 1 "#, - )?; - let candidates = collect_mapped_rows( - &mut stmt, - named_params! { - ":id": id, - }, - map_locality_candidate_row, - )?; + ) + .bind(id) + .fetch_all(conn), + ) + .map_err(GeocoderError::from) + })?; + let candidates = collect_mapped_rows(rows, map_locality_candidate_row)?; Ok(finalize_locality_lookup(candidates, 1)) } + + fn with_connection<T>( + &self, + f: impl FnOnce(&mut SqliteConnection) -> Result<T, GeocoderError>, + ) -> Result<T, GeocoderError> { + let mut conn = self + .conn + .lock() + .map_err(|_| GeocoderError::SqliteConnectionLockUnavailable)?; + f(&mut conn) + } } struct ParsedLocalityQuery { @@ -245,18 +279,15 @@ struct ParsedLocalityQuery { country: Option<String>, } -fn query_country_center_row( - stmt: &mut rusqlite::Statement<'_>, - country_id: &str, -) -> rusqlite::Result<(Option<f64>, Option<f64>)> { - stmt.query_row( - named_params! { ":country_id": country_id }, - map_country_center_row, - ) +fn open_read_only_connection(path: impl AsRef<Path>) -> Result<SqliteConnection, GeocoderError> { + let options = SqliteConnectOptions::new().filename(path).read_only(true); + Ok(futures_executor::block_on(SqliteConnection::connect_with( + &options, + ))?) } -fn map_country_center_row(row: &rusqlite::Row<'_>) -> rusqlite::Result<(Option<f64>, Option<f64>)> { - Ok((row.get("latitude_c")?, row.get("longitude_c")?)) +fn map_country_center_row(row: &SqliteRow) -> Result<(Option<f64>, Option<f64>), sqlx::Error> { + Ok((row.try_get("latitude_c")?, row.try_get("longitude_c")?)) } #[inline(never)] @@ -274,68 +305,69 @@ fn finalize_country_center( } fn country_center_impl( - conn: &Connection, + geocoder: &Geocoder, country_id: &str, ) -> Result<Option<GeocoderPoint>, GeocoderError> { - let mut stmt = conn.prepare( - r#" + let row = geocoder.with_connection(|conn| { + futures_executor::block_on( + sqlx::query( + r#" SELECT AVG(latitude) AS latitude_c, AVG(longitude) AS longitude_c FROM geonames - WHERE country_id = :country_id + WHERE country_id = ? "#, - )?; - let (lat, lng) = query_country_center_row(&mut stmt, country_id)?; + ) + .bind(country_id) + .fetch_one(conn), + ) + .map_err(GeocoderError::from) + })?; + let (lat, lng) = map_country_center_row(&row)?; if let (Some(lat), Some(lng)) = (lat, lng) { return Ok(Some(GeocoderPoint { lat, lng })); } Ok(None) } -fn collect_mapped_rows<T, P, F>( - stmt: &mut rusqlite::Statement<'_>, - params: P, - map: F, -) -> Result<Vec<T>, GeocoderError> +fn collect_mapped_rows<T, F>(rows: Vec<SqliteRow>, mut map: F) -> Result<Vec<T>, GeocoderError> where - P: rusqlite::Params, - F: FnMut(&rusqlite::Row<'_>) -> rusqlite::Result<T>, + F: FnMut(&SqliteRow) -> Result<T, sqlx::Error>, { - let rows = stmt.query_map(params, map)?; - rows.collect::<Result<Vec<_>, _>>() + rows.iter() + .map(&mut map) + .collect::<Result<Vec<_>, _>>() .map_err(GeocoderError::from) } -fn map_reverse_row(row: &rusqlite::Row<'_>) -> rusqlite::Result<GeocoderReverseResult> { +fn map_reverse_row(row: &SqliteRow) -> Result<GeocoderReverseResult, sqlx::Error> { Ok(GeocoderReverseResult { - id: row.get("id")?, - name: row.get("name")?, - admin1_id: row.get("admin1_id")?, - admin1_name: row.get("admin1_name")?, - country_id: row.get("country_id")?, - country_name: row.get("country_name")?, - latitude: row.get("latitude")?, - longitude: row.get("longitude")?, + id: required_i64(row, "id")?, + name: required_string(row, "name")?, + admin1_id: row.try_get("admin1_id")?, + admin1_name: row.try_get("admin1_name")?, + country_id: required_string(row, "country_id")?, + country_name: row.try_get("country_name")?, + latitude: required_f64(row, "latitude")?, + longitude: required_f64(row, "longitude")?, }) } -fn map_locality_candidate_row( - row: &rusqlite::Row<'_>, -) -> rusqlite::Result<GeocoderLocalityCandidate> { - let name = row.get("name")?; - let admin1_name = row.get("admin1_name")?; - let country_name = row.get("country_name")?; +fn map_locality_candidate_row(row: &SqliteRow) -> Result<GeocoderLocalityCandidate, sqlx::Error> { + let name = required_string(row, "name")?; + let admin1_name = row.try_get("admin1_name")?; + let country_name = row.try_get("country_name")?; let candidate = GeocoderLocalityCandidate { - id: row.get("id")?, + id: required_i64(row, "id")?, name, - admin1_id: row.get("admin1_id")?, + admin1_id: row.try_get("admin1_id")?, admin1_name, - country_id: row.get("country_id")?, + country_id: required_string(row, "country_id")?, country_name, point: GeocoderPoint { - lat: row.get("latitude")?, - lng: row.get("longitude")?, + lat: required_f64(row, "latitude")?, + lng: required_f64(row, "longitude")?, }, display_name: String::new(), }; @@ -345,6 +377,33 @@ fn map_locality_candidate_row( }) } +fn required_i64(row: &SqliteRow, column: &str) -> Result<i64, sqlx::Error> { + required_value(row, column) +} + +fn required_f64(row: &SqliteRow, column: &str) -> Result<f64, sqlx::Error> { + required_value(row, column) +} + +fn required_string(row: &SqliteRow, column: &str) -> Result<String, sqlx::Error> { + required_value(row, column) +} + +fn required_value<T>(row: &SqliteRow, column: &str) -> Result<T, sqlx::Error> +where + for<'r> T: sqlx::Decode<'r, sqlx::Sqlite> + sqlx::Type<sqlx::Sqlite>, +{ + row.try_get::<Option<T>, _>(column)? + .ok_or_else(|| unexpected_null_column(column)) +} + +fn unexpected_null_column(column: &str) -> sqlx::Error { + sqlx::Error::ColumnDecode { + index: column.to_owned(), + source: Box::new(sqlx::error::UnexpectedNullError), + } +} + fn parse_locality_query(query: &str) -> ParsedLocalityQuery { let parts = query .split(',') @@ -528,8 +587,8 @@ fn region_aliases(country_id: &str) -> &'static [(&'static str, &'static str)] { #[cfg(test)] mod tests { use super::*; - use rusqlite::Connection; use std::fs; + use std::sync::Mutex; use tempfile::NamedTempFile; #[test] @@ -771,18 +830,18 @@ mod tests { }), ) .expect_err("reverse should fail on invalid row mapping"); - assert_sqlite_error_contains(reverse_err, "Invalid column type"); + assert_sqlite_error_contains(reverse_err, "unexpected null"); let country_err = reverse_country .country("US") .expect_err("country should fail on invalid row mapping"); - assert_sqlite_error_contains(country_err, "Invalid column type"); + assert_sqlite_error_contains(country_err, "unexpected null"); let country_list = open_country_list_row_error_geocoder(); let country_list_err = country_list .country_list() .expect_err("country_list should fail on null aggregate row"); - assert_sqlite_error_contains(country_list_err, "Invalid column type"); + assert_sqlite_error_contains(country_list_err, "unexpected null"); } #[test] @@ -821,27 +880,27 @@ mod tests { geocoder_with_country_list_sql_row("1", "'United States'", "37.0", "1.0") .country_list() .expect_err("country_id type mismatch should fail"); - assert_sqlite_error_contains(country_id_err, "Invalid column type"); + assert_sqlite_error_contains(country_id_err, "mismatched types"); let country_name_err = geocoder_with_country_list_sql_row("'US'", "1", "37.0", "1.0") .country_list() .expect_err("country_name type mismatch should fail"); - assert_sqlite_error_contains(country_name_err, "Invalid column type"); + assert_sqlite_error_contains(country_name_err, "mismatched types"); let longitude_err = geocoder_with_country_list_sql_row("'US'", "'United States'", "37.0", "NULL") .country_list() .expect_err("longitude type mismatch should fail"); - assert_sqlite_error_contains(longitude_err, "Invalid column type"); + assert_sqlite_error_contains(longitude_err, "unexpected null"); } #[test] fn unit_harness_covers_country_center_row_error_paths() { let latitude_err = map_country_center_row_error("'bad'", "1.0"); - assert_sqlite_error_contains(GeocoderError::from(latitude_err), "Invalid column type"); + assert_sqlite_error_contains(latitude_err, "mismatched types"); let longitude_err = map_country_center_row_error("1.0", "'bad'"); - assert_sqlite_error_contains(GeocoderError::from(longitude_err), "Invalid column type"); + assert_sqlite_error_contains(longitude_err, "mismatched types"); } #[test] @@ -909,7 +968,7 @@ mod tests { "'bad'", ), ] { - assert_sqlite_error_contains(GeocoderError::from(err), "Invalid column type"); + assert_sqlite_error_contains(err, "mismatched types"); } } @@ -935,7 +994,7 @@ mod tests { assert!(sqlite_panic.is_err()); let country_center_panic = std::panic::catch_unwind(|| { - let mismatch_err = GeocoderError::Sqlite(rusqlite::Error::InvalidQuery); + let mismatch_err = GeocoderError::Sqlite(sqlx::Error::RowNotFound); assert_country_center_not_found(mismatch_err, "US"); }); assert!(country_center_panic.is_err()); @@ -993,8 +1052,9 @@ mod tests { } fn geocoder_with_reverse_country_query_execution_error() -> Geocoder { - let conn = Connection::open_in_memory().expect("open in-memory query error db"); - conn.execute_batch( + let mut conn = open_test_memory_connection(); + execute_batch( + &mut conn, r#" CREATE VIEW geonames AS SELECT @@ -1013,18 +1073,17 @@ mod tests { ); INSERT INTO coordinates (feature_id, latitude, longitude) VALUES (1, 1.0, 2.0); "#, - ) - .expect("create reverse/country execution error schema"); + ); Geocoder { - conn, + conn: Mutex::new(conn), _temp_path: None, } } fn geocoder_with_country_list_query_execution_error() -> Geocoder { - let conn = - Connection::open_in_memory().expect("open in-memory country_list query error db"); - conn.execute_batch( + let mut conn = open_test_memory_connection(); + execute_batch( + &mut conn, r#" CREATE VIEW geonames AS SELECT @@ -1033,18 +1092,17 @@ mod tests { 1.0 AS latitude, 2.0 AS longitude; "#, - ) - .expect("create country_list execution error schema"); + ); Geocoder { - conn, + conn: Mutex::new(conn), _temp_path: None, } } fn geocoder_with_country_center_query_execution_error() -> Geocoder { - let conn = - Connection::open_in_memory().expect("open in-memory country_center query error db"); - conn.execute_batch( + let mut conn = open_test_memory_connection(); + execute_batch( + &mut conn, r#" CREATE VIEW geonames AS SELECT @@ -1052,10 +1110,9 @@ mod tests { missing_latitude() AS latitude, 2.0 AS longitude; "#, - ) - .expect("create country_center execution error schema"); + ); Geocoder { - conn, + conn: Mutex::new(conn), _temp_path: None, } } @@ -1066,10 +1123,11 @@ mod tests { latitude_sql: &str, longitude_sql: &str, ) -> Geocoder { - let conn = - Connection::open_in_memory().expect("open in-memory country_list field error db"); - conn.execute_batch(&format!( - r#" + let mut conn = open_test_memory_connection(); + execute_batch( + &mut conn, + &format!( + r#" CREATE TABLE geonames( country_id, country_name, @@ -1079,23 +1137,26 @@ mod tests { INSERT INTO geonames (country_id, country_name, latitude, longitude) VALUES ({country_id_sql}, {country_name_sql}, {latitude_sql}, {longitude_sql}); "#, - )) - .expect("create country_list field error schema"); + ), + ); Geocoder { - conn, + conn: Mutex::new(conn), _temp_path: None, } } - fn map_country_center_row_error(latitude_sql: &str, longitude_sql: &str) -> rusqlite::Error { - let conn = - Connection::open_in_memory().expect("open in-memory country center row error db"); - conn.query_row( - &format!("SELECT {latitude_sql} AS latitude_c, {longitude_sql} AS longitude_c"), - [], - map_country_center_row, + fn map_country_center_row_error(latitude_sql: &str, longitude_sql: &str) -> GeocoderError { + let mut conn = open_test_memory_connection(); + let row = futures_executor::block_on( + sqlx::query(sqlx::AssertSqlSafe(format!( + "SELECT {latitude_sql} AS latitude_c, {longitude_sql} AS longitude_c" + ))) + .fetch_one(&mut conn), ) - .expect_err("country center row decode should fail") + .expect("country center row should fetch"); + map_country_center_row(&row) + .map_err(GeocoderError::from) + .expect_err("country center row decode should fail") } #[allow(clippy::too_many_arguments)] @@ -1108,10 +1169,10 @@ mod tests { country_name_sql: &str, latitude_sql: &str, longitude_sql: &str, - ) -> rusqlite::Error { - let conn = Connection::open_in_memory().expect("open in-memory reverse row error db"); - conn.query_row( - &format!( + ) -> GeocoderError { + let mut conn = open_test_memory_connection(); + let row = futures_executor::block_on( + sqlx::query(sqlx::AssertSqlSafe(format!( r#" SELECT {id_sql} AS id, @@ -1123,54 +1184,56 @@ mod tests { {latitude_sql} AS latitude, {longitude_sql} AS longitude "#, - ), - [], - map_reverse_row, + ))) + .fetch_one(&mut conn), ) - .expect_err("reverse row decode should fail") + .expect("reverse row should fetch"); + map_reverse_row(&row) + .map_err(GeocoderError::from) + .expect_err("reverse row decode should fail") } fn seed_fixture_database(path: &str) { - let conn = Connection::open(path).expect("open fixture database"); - seed_schema(&conn); + let mut conn = open_test_path_connection(path); + seed_schema(&mut conn); - insert_country(&conn, "US", "United States"); - insert_country(&conn, "BR", "Brazil"); + insert_country(&mut conn, "US", "United States"); + insert_country(&mut conn, "BR", "Brazil"); - insert_admin1(&conn, "US", 6, "California"); - insert_admin1(&conn, "US", 36, "New York"); - insert_admin1(&conn, "BR", 27, "Sao Paulo"); + insert_admin1(&mut conn, "US", 6, "California"); + insert_admin1(&mut conn, "US", 36, "New York"); + insert_admin1(&mut conn, "BR", 27, "Sao Paulo"); - insert_feature(&conn, 1, "San Francisco", "US", 6, 37.7749, -122.4194); - insert_feature(&conn, 2, "Los Angeles", "US", 6, 34.0522, -118.2437); - insert_feature(&conn, 3, "New York City", "US", 36, 40.7128, -74.0060); - insert_feature(&conn, 4, "Sao Paulo", "BR", 27, -23.5505, -46.6333); + insert_feature(&mut conn, 1, "San Francisco", "US", 6, 37.7749, -122.4194); + insert_feature(&mut conn, 2, "Los Angeles", "US", 6, 34.0522, -118.2437); + insert_feature(&mut conn, 3, "New York City", "US", 36, 40.7128, -74.0060); + insert_feature(&mut conn, 4, "Sao Paulo", "BR", 27, -23.5505, -46.6333); } fn seed_high_latitude_database(path: &str) { - let conn = Connection::open(path).expect("open fixture database"); - seed_schema(&conn); + let mut conn = open_test_path_connection(path); + seed_schema(&mut conn); - insert_country(&conn, "NO", "Norway"); - insert_admin1(&conn, "NO", 1, "Nord"); + insert_country(&mut conn, "NO", "Norway"); + insert_admin1(&mut conn, "NO", 1, "Nord"); - insert_feature(&conn, 1, "Polar East", "NO", 1, 75.02, 0.10); - insert_feature(&conn, 2, "Polar North", "NO", 1, 75.05, 0.05); + insert_feature(&mut conn, 1, "Polar East", "NO", 1, 75.02, 0.10); + insert_feature(&mut conn, 2, "Polar North", "NO", 1, 75.05, 0.05); } fn seed_forward_fixture_database(path: &str) { - let conn = Connection::open(path).expect("open fixture database"); - seed_schema(&conn); + let mut conn = open_test_path_connection(path); + seed_schema(&mut conn); - insert_country(&conn, "CA", "Canada"); - insert_country(&conn, "US", "United States"); + insert_country(&mut conn, "CA", "Canada"); + insert_country(&mut conn, "US", "United States"); - insert_admin1(&conn, "CA", 2, "British Columbia"); - insert_admin1(&conn, "CA", 3, "Prairie Region"); - insert_admin1(&conn, "US", 4, "River Region"); + insert_admin1(&mut conn, "CA", 2, "British Columbia"); + insert_admin1(&mut conn, "CA", 3, "Prairie Region"); + insert_admin1(&mut conn, "US", 4, "River Region"); insert_feature( - &conn, + &mut conn, 3001, "Fixture Victoria", "CA", @@ -1178,15 +1241,16 @@ mod tests { 48.4359, -123.35155, ); - insert_feature(&conn, 3002, "Shared Market", "CA", 2, 48.7, -123.2); - insert_feature(&conn, 3003, "Shared Market", "CA", 3, 50.2, -110.4); - insert_feature(&conn, 3004, "Identifier Grove", "CA", 2, 48.9, -123.4); - insert_feature(&conn, 3005, "Query Hamlet", "US", 4, 39.25, -77.5); + insert_feature(&mut conn, 3002, "Shared Market", "CA", 2, 48.7, -123.2); + insert_feature(&mut conn, 3003, "Shared Market", "CA", 3, 50.2, -110.4); + insert_feature(&mut conn, 3004, "Identifier Grove", "CA", 2, 48.9, -123.4); + insert_feature(&mut conn, 3005, "Query Hamlet", "US", 4, 39.25, -77.5); } fn seed_reverse_country_row_error_database(path: &str) { - let conn = Connection::open(path).expect("open invalid row fixture database"); - conn.execute_batch( + let mut conn = open_test_path_connection(path); + execute_batch( + &mut conn, r#" CREATE TABLE geonames( id INTEGER, @@ -1204,23 +1268,36 @@ mod tests { longitude REAL ); "#, - ) - .expect("create invalid row schema"); - conn.execute( - "INSERT INTO geonames (id, name, admin1_id, admin1_name, country_id, country_name, latitude, longitude) VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8)", - rusqlite::params![1_i64, Option::<String>::None, Option::<i64>::None, Option::<String>::None, "US", "United States", 37.7749_f64, -122.4194_f64], + ); + futures_executor::block_on( + sqlx::query("INSERT INTO geonames (id, name, admin1_id, admin1_name, country_id, country_name, latitude, longitude) VALUES (?, ?, ?, ?, ?, ?, ?, ?)") + .bind(1_i64) + .bind(Option::<String>::None) + .bind(Option::<i64>::None) + .bind(Option::<String>::None) + .bind("US") + .bind("United States") + .bind(37.7749_f64) + .bind(-122.4194_f64) + .execute(&mut conn), ) .expect("insert invalid reverse/country row"); - conn.execute( - "INSERT INTO coordinates (feature_id, latitude, longitude) VALUES (?1, ?2, ?3)", - (1_i64, 37.7749_f64, -122.4194_f64), + futures_executor::block_on( + sqlx::query( + "INSERT INTO coordinates (feature_id, latitude, longitude) VALUES (?, ?, ?)", + ) + .bind(1_i64) + .bind(37.7749_f64) + .bind(-122.4194_f64) + .execute(&mut conn), ) .expect("insert invalid reverse/country coordinate"); } fn seed_country_list_row_error_database(path: &str) { - let conn = Connection::open(path).expect("open aggregate error fixture database"); - conn.execute_batch( + let mut conn = open_test_path_connection(path); + execute_batch( + &mut conn, r#" CREATE TABLE geonames( country_id TEXT, @@ -1229,17 +1306,44 @@ mod tests { longitude REAL ); "#, - ) - .expect("create aggregate error schema"); - conn.execute( - "INSERT INTO geonames (country_id, country_name, latitude, longitude) VALUES (?1, ?2, ?3, ?4)", - rusqlite::params!["US", "United States", Option::<f64>::None, Option::<f64>::None], + ); + futures_executor::block_on( + sqlx::query( + "INSERT INTO geonames (country_id, country_name, latitude, longitude) VALUES (?, ?, ?, ?)", + ) + .bind("US") + .bind("United States") + .bind(Option::<f64>::None) + .bind(Option::<f64>::None) + .execute(&mut conn), ) .expect("insert aggregate error row"); } - fn seed_schema(conn: &Connection) { - conn.execute_batch( + fn open_test_path_connection(path: &str) -> SqliteConnection { + futures_executor::block_on(SqliteConnection::connect_with( + &SqliteConnectOptions::new() + .filename(path) + .create_if_missing(true), + )) + .expect("open fixture database") + } + + fn open_test_memory_connection() -> SqliteConnection { + futures_executor::block_on(SqliteConnection::connect_with( + &SqliteConnectOptions::new().in_memory(true), + )) + .expect("open in-memory fixture database") + } + + fn execute_batch(conn: &mut SqliteConnection, sql: &str) { + futures_executor::block_on(sqlx::raw_sql(sqlx::AssertSqlSafe(sql)).execute(conn)) + .expect("execute fixture sql batch"); + } + + fn seed_schema(conn: &mut SqliteConnection) { + execute_batch( + conn, r#" CREATE TABLE countries( id TEXT, @@ -1281,28 +1385,32 @@ mod tests { LEFT JOIN admin1 ON features.country_id = admin1.country_id AND features.admin1_id = admin1.id JOIN coordinates ON features.id = coordinates.feature_id; "#, - ) - .expect("create fixture schema"); + ); } - fn insert_country(conn: &Connection, id: &str, name: &str) { - conn.execute( - "INSERT INTO countries (id, name) VALUES (?1, ?2)", - (id, name), + fn insert_country(conn: &mut SqliteConnection, id: &str, name: &str) { + futures_executor::block_on( + sqlx::query("INSERT INTO countries (id, name) VALUES (?, ?)") + .bind(id) + .bind(name) + .execute(conn), ) .expect("insert country"); } - fn insert_admin1(conn: &Connection, country_id: &str, id: i64, name: &str) { - conn.execute( - "INSERT INTO admin1 (country_id, id, name) VALUES (?1, ?2, ?3)", - (country_id, id, name), + fn insert_admin1(conn: &mut SqliteConnection, country_id: &str, id: i64, name: &str) { + futures_executor::block_on( + sqlx::query("INSERT INTO admin1 (country_id, id, name) VALUES (?, ?, ?)") + .bind(country_id) + .bind(id) + .bind(name) + .execute(conn), ) .expect("insert admin1"); } fn insert_feature( - conn: &Connection, + conn: &mut SqliteConnection, id: i64, name: &str, country_id: &str, @@ -1310,14 +1418,25 @@ mod tests { latitude: f64, longitude: f64, ) { - conn.execute( - "INSERT INTO features (id, name, country_id, admin1_id) VALUES (?1, ?2, ?3, ?4)", - (id, name, country_id, admin1_id), + futures_executor::block_on( + sqlx::query( + "INSERT INTO features (id, name, country_id, admin1_id) VALUES (?, ?, ?, ?)", + ) + .bind(id) + .bind(name) + .bind(country_id) + .bind(admin1_id) + .execute(&mut *conn), ) .expect("insert feature"); - conn.execute( - "INSERT INTO coordinates (feature_id, latitude, longitude) VALUES (?1, ?2, ?3)", - (id, latitude, longitude), + futures_executor::block_on( + sqlx::query( + "INSERT INTO coordinates (feature_id, latitude, longitude) VALUES (?, ?, ?)", + ) + .bind(id) + .bind(latitude) + .bind(longitude) + .execute(conn), ) .expect("insert coordinate"); } diff --git a/crates/geocoder/tests/geocoder.rs b/crates/geocoder/tests/geocoder.rs @@ -6,8 +6,9 @@ use radroots_geocoder::{ inspect_default_geonames_asset_in_cache_root, inspect_geonames_asset_path, validate_geonames_asset_file, validate_geonames_asset_spec_source, }; -use rusqlite::Connection; use sha2::Digest; +use sqlx::Connection; +use sqlx::sqlite::{SqliteConnectOptions, SqliteConnection}; use std::cell::Cell; use std::fs; use std::path::Path; @@ -459,12 +460,12 @@ fn reverse_and_country_propagate_row_mapping_errors() { }), ) .expect_err("reverse should fail on invalid row mapping"); - assert_sqlite_error_contains(reverse_err, "Invalid column type"); + assert_sqlite_error_contains(reverse_err, "unexpected null"); let country_err = geocoder .country("US") .expect_err("country should fail on invalid row mapping"); - assert_sqlite_error_contains(country_err, "Invalid column type"); + assert_sqlite_error_contains(country_err, "unexpected null"); } #[test] @@ -474,7 +475,7 @@ fn country_list_propagates_aggregate_row_mapping_errors() { let err = geocoder .country_list() .expect_err("country_list should fail on null aggregate row"); - assert_sqlite_error_contains(err, "Invalid column type"); + assert_sqlite_error_contains(err, "unexpected null"); } fn open_fixture_geocoder() -> Geocoder { @@ -534,53 +535,55 @@ fn build_forward_fixture_database() -> tempfile::TempPath { } fn seed_fixture_database(path: &str) { - let conn = Connection::open(path).expect("open fixture database"); - seed_schema(&conn); + let mut conn = open_test_path_connection(path); + seed_schema(&mut conn); - insert_country(&conn, "US", "United States"); - insert_country(&conn, "BR", "Brazil"); + insert_country(&mut conn, "US", "United States"); + insert_country(&mut conn, "BR", "Brazil"); - insert_admin1(&conn, "US", 6, "California"); - insert_admin1(&conn, "US", 36, "New York"); - insert_admin1(&conn, "BR", 27, "Sao Paulo"); + insert_admin1(&mut conn, "US", 6, "California"); + insert_admin1(&mut conn, "US", 36, "New York"); + insert_admin1(&mut conn, "BR", 27, "Sao Paulo"); - insert_feature(&conn, 1, "San Francisco", "US", 6, 37.7749, -122.4194); - insert_feature(&conn, 2, "Los Angeles", "US", 6, 34.0522, -118.2437); - insert_feature(&conn, 3, "New York City", "US", 36, 40.7128, -74.0060); - insert_feature(&conn, 4, "Sao Paulo", "BR", 27, -23.5505, -46.6333); + insert_feature(&mut conn, 1, "San Francisco", "US", 6, 37.7749, -122.4194); + insert_feature(&mut conn, 2, "Los Angeles", "US", 6, 34.0522, -118.2437); + insert_feature(&mut conn, 3, "New York City", "US", 36, 40.7128, -74.0060); + insert_feature(&mut conn, 4, "Sao Paulo", "BR", 27, -23.5505, -46.6333); } fn seed_high_latitude_database(path: &str) { - let conn = Connection::open(path).expect("open fixture database"); - seed_schema(&conn); + let mut conn = open_test_path_connection(path); + seed_schema(&mut conn); - insert_country(&conn, "NO", "Norway"); - insert_admin1(&conn, "NO", 1, "Nord"); + insert_country(&mut conn, "NO", "Norway"); + insert_admin1(&mut conn, "NO", 1, "Nord"); - insert_feature(&conn, 1, "Polar East", "NO", 1, 75.02, 0.10); - insert_feature(&conn, 2, "Polar North", "NO", 1, 75.05, 0.05); + insert_feature(&mut conn, 1, "Polar East", "NO", 1, 75.02, 0.10); + insert_feature(&mut conn, 2, "Polar North", "NO", 1, 75.05, 0.05); } fn seed_forward_fixture_database(path: &str) { - let conn = Connection::open(path).expect("open fixture database"); - seed_schema(&conn); - - insert_country(&conn, "CA", "Canada"); - insert_country(&conn, "US", "United States"); - conn.execute( - "INSERT INTO countries (id, name) VALUES (?1, ?2)", - rusqlite::params!["ZZ", Option::<String>::None], + let mut conn = open_test_path_connection(path); + seed_schema(&mut conn); + + insert_country(&mut conn, "CA", "Canada"); + insert_country(&mut conn, "US", "United States"); + futures_executor::block_on( + sqlx::query("INSERT INTO countries (id, name) VALUES (?, ?)") + .bind("ZZ") + .bind(Option::<String>::None) + .execute(&mut conn), ) .expect("insert unnamed country"); - insert_admin1(&conn, "CA", 2, "British Columbia"); - insert_admin1(&conn, "CA", 3, "Prairie Region"); - insert_admin1(&conn, "US", 4, "River Region"); - insert_admin1(&conn, "US", 6, "California"); - insert_admin1(&conn, "ZZ", 100, "No Alias Region"); + insert_admin1(&mut conn, "CA", 2, "British Columbia"); + insert_admin1(&mut conn, "CA", 3, "Prairie Region"); + insert_admin1(&mut conn, "US", 4, "River Region"); + insert_admin1(&mut conn, "US", 6, "California"); + insert_admin1(&mut conn, "ZZ", 100, "No Alias Region"); insert_feature( - &conn, + &mut conn, 3001, "Fixture Victoria", "CA", @@ -588,18 +591,19 @@ fn seed_forward_fixture_database(path: &str) { 48.4359, -123.35155, ); - insert_feature(&conn, 3002, "Shared Market", "CA", 2, 48.7, -123.2); - insert_feature(&conn, 3003, "Shared Market", "CA", 3, 50.2, -110.4); - insert_feature(&conn, 3004, "Identifier Grove", "CA", 2, 48.9, -123.4); - insert_feature(&conn, 3005, "Query Hamlet", "US", 4, 39.25, -77.5); - insert_feature(&conn, 3006, "Alias Market", "US", 6, 38.5, -121.5); - insert_feature(&conn, 3007, "No Country Place", "ZZ", 99, 10.0, 11.0); - insert_feature(&conn, 3008, "No Alias Place", "ZZ", 100, 10.5, 11.5); + insert_feature(&mut conn, 3002, "Shared Market", "CA", 2, 48.7, -123.2); + insert_feature(&mut conn, 3003, "Shared Market", "CA", 3, 50.2, -110.4); + insert_feature(&mut conn, 3004, "Identifier Grove", "CA", 2, 48.9, -123.4); + insert_feature(&mut conn, 3005, "Query Hamlet", "US", 4, 39.25, -77.5); + insert_feature(&mut conn, 3006, "Alias Market", "US", 6, 38.5, -121.5); + insert_feature(&mut conn, 3007, "No Country Place", "ZZ", 99, 10.0, 11.0); + insert_feature(&mut conn, 3008, "No Alias Place", "ZZ", 100, 10.5, 11.5); } fn seed_reverse_country_row_error_database(path: &str) { - let conn = Connection::open(path).expect("open invalid row fixture database"); - conn.execute_batch( + let mut conn = open_test_path_connection(path); + execute_batch( + &mut conn, r#" CREATE TABLE geonames( id INTEGER, @@ -617,23 +621,34 @@ fn seed_reverse_country_row_error_database(path: &str) { longitude REAL ); "#, - ) - .expect("create invalid row schema"); - conn.execute( - "INSERT INTO geonames (id, name, admin1_id, admin1_name, country_id, country_name, latitude, longitude) VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8)", - rusqlite::params![1_i64, Option::<String>::None, Option::<i64>::None, Option::<String>::None, "US", "United States", 37.7749_f64, -122.4194_f64], + ); + futures_executor::block_on( + sqlx::query("INSERT INTO geonames (id, name, admin1_id, admin1_name, country_id, country_name, latitude, longitude) VALUES (?, ?, ?, ?, ?, ?, ?, ?)") + .bind(1_i64) + .bind(Option::<String>::None) + .bind(Option::<i64>::None) + .bind(Option::<String>::None) + .bind("US") + .bind("United States") + .bind(37.7749_f64) + .bind(-122.4194_f64) + .execute(&mut conn), ) .expect("insert invalid reverse/country row"); - conn.execute( - "INSERT INTO coordinates (feature_id, latitude, longitude) VALUES (?1, ?2, ?3)", - (1_i64, 37.7749_f64, -122.4194_f64), + futures_executor::block_on( + sqlx::query("INSERT INTO coordinates (feature_id, latitude, longitude) VALUES (?, ?, ?)") + .bind(1_i64) + .bind(37.7749_f64) + .bind(-122.4194_f64) + .execute(&mut conn), ) .expect("insert invalid reverse/country coordinate"); } fn seed_country_list_row_error_database(path: &str) { - let conn = Connection::open(path).expect("open aggregate error fixture database"); - conn.execute_batch( + let mut conn = open_test_path_connection(path); + execute_batch( + &mut conn, r#" CREATE TABLE geonames( country_id TEXT, @@ -642,17 +657,37 @@ fn seed_country_list_row_error_database(path: &str) { longitude REAL ); "#, - ) - .expect("create aggregate error schema"); - conn.execute( - "INSERT INTO geonames (country_id, country_name, latitude, longitude) VALUES (?1, ?2, ?3, ?4)", - rusqlite::params!["US", "United States", Option::<f64>::None, Option::<f64>::None], + ); + futures_executor::block_on( + sqlx::query( + "INSERT INTO geonames (country_id, country_name, latitude, longitude) VALUES (?, ?, ?, ?)", + ) + .bind("US") + .bind("United States") + .bind(Option::<f64>::None) + .bind(Option::<f64>::None) + .execute(&mut conn), ) .expect("insert aggregate error row"); } -fn seed_schema(conn: &Connection) { - conn.execute_batch( +fn open_test_path_connection(path: &str) -> SqliteConnection { + futures_executor::block_on(SqliteConnection::connect_with( + &SqliteConnectOptions::new() + .filename(path) + .create_if_missing(true), + )) + .expect("open fixture database") +} + +fn execute_batch(conn: &mut SqliteConnection, sql: &str) { + futures_executor::block_on(sqlx::raw_sql(sqlx::AssertSqlSafe(sql)).execute(conn)) + .expect("execute fixture sql batch"); +} + +fn seed_schema(conn: &mut SqliteConnection) { + execute_batch( + conn, r#" CREATE TABLE countries( id TEXT, @@ -694,28 +729,32 @@ fn seed_schema(conn: &Connection) { LEFT JOIN admin1 ON features.country_id = admin1.country_id AND features.admin1_id = admin1.id JOIN coordinates ON features.id = coordinates.feature_id; "#, - ) - .expect("create fixture schema"); + ); } -fn insert_country(conn: &Connection, id: &str, name: &str) { - conn.execute( - "INSERT INTO countries (id, name) VALUES (?1, ?2)", - (id, name), +fn insert_country(conn: &mut SqliteConnection, id: &str, name: &str) { + futures_executor::block_on( + sqlx::query("INSERT INTO countries (id, name) VALUES (?, ?)") + .bind(id) + .bind(name) + .execute(conn), ) .expect("insert country"); } -fn insert_admin1(conn: &Connection, country_id: &str, id: i64, name: &str) { - conn.execute( - "INSERT INTO admin1 (country_id, id, name) VALUES (?1, ?2, ?3)", - (country_id, id, name), +fn insert_admin1(conn: &mut SqliteConnection, country_id: &str, id: i64, name: &str) { + futures_executor::block_on( + sqlx::query("INSERT INTO admin1 (country_id, id, name) VALUES (?, ?, ?)") + .bind(country_id) + .bind(id) + .bind(name) + .execute(conn), ) .expect("insert admin1"); } fn insert_feature( - conn: &Connection, + conn: &mut SqliteConnection, id: i64, name: &str, country_id: &str, @@ -723,14 +762,21 @@ fn insert_feature( latitude: f64, longitude: f64, ) { - conn.execute( - "INSERT INTO features (id, name, country_id, admin1_id) VALUES (?1, ?2, ?3, ?4)", - (id, name, country_id, admin1_id), + futures_executor::block_on( + sqlx::query("INSERT INTO features (id, name, country_id, admin1_id) VALUES (?, ?, ?, ?)") + .bind(id) + .bind(name) + .bind(country_id) + .bind(admin1_id) + .execute(&mut *conn), ) .expect("insert feature"); - conn.execute( - "INSERT INTO coordinates (feature_id, latitude, longitude) VALUES (?1, ?2, ?3)", - (id, latitude, longitude), + futures_executor::block_on( + sqlx::query("INSERT INTO coordinates (feature_id, latitude, longitude) VALUES (?, ?, ?)") + .bind(id) + .bind(latitude) + .bind(longitude) + .execute(conn), ) .expect("insert coordinate"); } diff --git a/crates/mesh/tests/mesh.rs b/crates/mesh/tests/mesh.rs @@ -66,7 +66,7 @@ fn all_frame_types_round_trip_with_stable_codes_and_labels() { frame_type, RadrootsMeshScope::Community, format!("{label}-message"), - u64::from(code) + 1, + code + 1, 1_000, ); let encoded = encode_mesh_frame_cbor(&frame).expect("encode frame"); diff --git a/crates/nostr_connect/src/message.rs b/crates/nostr_connect/src/message.rs @@ -122,10 +122,7 @@ impl RadrootsNostrConnectRequest { }); } let remote_signer_public_key = parse_public_key(¶ms[0])?; - let secret = params - .get(1) - .cloned() - .and_then(|value| if value.is_empty() { None } else { Some(value) }); + let secret = params.get(1).cloned().filter(|value| !value.is_empty()); let requested_permissions = match params.get(2) { Some(value) => RadrootsNostrConnectPermissions::from_str(value)?, None => RadrootsNostrConnectPermissions::default(), diff --git a/crates/nostr_runtime/src/runtime.rs b/crates/nostr_runtime/src/runtime.rs @@ -306,7 +306,7 @@ impl RadrootsNostrRuntime { let mut connected = 0usize; let mut connecting = 0usize; - for (_, status) in statuses.iter() { + for status in statuses.values() { match status { RadrootsNostrRelayStatus::Connected => connected += 1, RadrootsNostrRelayStatus::Connecting => connecting += 1, diff --git a/crates/nostr_signer/src/sqlite.rs b/crates/nostr_signer/src/sqlite.rs @@ -1,10 +1,10 @@ use crate::error::RadrootsNostrSignerError; use crate::migrations; -use radroots_sql_core::{SqlExecutor, SqliteExecutor}; +use radroots_sql_core::{SqlExecutor, SqlxSqliteExecutor}; use std::path::Path; pub struct RadrootsNostrSignerSqliteDb { - executor: SqliteExecutor, + executor: SqlxSqliteExecutor, file_backed: bool, } @@ -17,7 +17,7 @@ impl RadrootsNostrSignerSqliteDb { std::fs::create_dir_all(parent) .map_err(|error| RadrootsNostrSignerError::Store(error.to_string()))?; } - let executor = SqliteExecutor::open(path)?; + let executor = SqlxSqliteExecutor::open(path)?; let db = Self { executor, file_backed: true, @@ -28,7 +28,7 @@ impl RadrootsNostrSignerSqliteDb { } pub fn open_memory() -> Result<Self, RadrootsNostrSignerError> { - let executor = SqliteExecutor::open_memory()?; + let executor = SqlxSqliteExecutor::open_memory()?; let db = Self { executor, file_backed: false, @@ -38,7 +38,7 @@ impl RadrootsNostrSignerSqliteDb { Ok(db) } - pub fn executor(&self) -> &SqliteExecutor { + pub fn executor(&self) -> &SqlxSqliteExecutor { &self.executor } diff --git a/crates/outbox/src/model.rs b/crates/outbox/src/model.rs @@ -250,8 +250,9 @@ impl RadrootsOutboxDeliveryTargetStatus { } } -#[derive(Clone, Copy, Debug, PartialEq, Eq)] +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] pub enum RadrootsOutboxReticulumPreviewBehavior { + #[default] RejectDeliveryAttempts, DeferDeliveryPlans, } @@ -265,12 +266,6 @@ impl RadrootsOutboxReticulumPreviewBehavior { } } -impl Default for RadrootsOutboxReticulumPreviewBehavior { - fn default() -> Self { - Self::RejectDeliveryAttempts - } -} - #[derive(Clone, Debug, PartialEq, Eq)] pub struct RadrootsOutboxDeliveryPlanInput { pub transport_profile_id: String, diff --git a/crates/outbox/src/store.rs b/crates/outbox/src/store.rs @@ -497,14 +497,16 @@ impl RadrootsOutbox { } else { claim_event( &mut tx, - outbox_event_id, - claimed_state, - None, - claim_owner.as_ref(), - claim_token.as_ref(), - claim_expires_at_ms, - now_ms, - "AND (claim_token IS NULL OR claim_expires_at_ms <= ?)", + ClaimEventUpdate { + outbox_event_id, + claimed_state, + active_delivery_plan_id: None, + claim_owner: claim_owner.as_ref(), + claim_token: claim_token.as_ref(), + claim_expires_at_ms, + now_ms, + suffix: "AND (claim_token IS NULL OR claim_expires_at_ms <= ?)", + }, ) .await? }; @@ -1810,17 +1812,31 @@ async fn ensure_event_signed( Ok(()) } -async fn claim_event( - tx: &mut sqlx::Transaction<'_, sqlx::Sqlite>, +struct ClaimEventUpdate<'a> { outbox_event_id: i64, claimed_state: RadrootsOutboxEventState, active_delivery_plan_id: Option<i64>, - claim_owner: &str, - claim_token: &str, + claim_owner: &'a str, + claim_token: &'a str, claim_expires_at_ms: i64, now_ms: i64, - suffix: &str, + suffix: &'a str, +} + +async fn claim_event( + tx: &mut sqlx::Transaction<'_, sqlx::Sqlite>, + update: ClaimEventUpdate<'_>, ) -> Result<SqliteQueryResult, RadrootsOutboxError> { + let ClaimEventUpdate { + outbox_event_id, + claimed_state, + active_delivery_plan_id, + claim_owner, + claim_token, + claim_expires_at_ms, + now_ms, + suffix, + } = update; let sql = format!( "UPDATE outbox_event SET state = ?, claim_token = ?, claim_owner = ?, claim_expires_at_ms = ?, active_delivery_plan_id = ?, attempt_count = attempt_count + 1, updated_at_ms = ? WHERE outbox_event_id = ? {suffix}" ); @@ -2167,11 +2183,7 @@ fn delivery_plan_status_targets<'a>( .. } => targets .iter() - .filter(|target| { - required_targets - .iter() - .any(|required| target.endpoint_fingerprint == *required) - }) + .filter(|target| required_targets.contains(&target.endpoint_fingerprint)) .collect(), RadrootsTransportSatisfactionPolicy::NoWait | RadrootsTransportSatisfactionPolicy::Any { .. } @@ -2631,15 +2643,13 @@ fn parse_satisfaction_policy( if let Some((class_label, threshold)) = value .strip_prefix("quorum_") .and_then(|stored| stored.split_once(':')) + && threshold == required_success_count.to_string() + && let Some(class) = parse_satisfaction_class_storage_value(class_label) { - if threshold == required_success_count.to_string() - && let Some(class) = parse_satisfaction_class_storage_value(class_label) - { - return Ok(RadrootsTransportSatisfactionPolicy::Quorum { - class, - threshold: required_count_u16(required_success_count)?, - }); - } + return Ok(RadrootsTransportSatisfactionPolicy::Quorum { + class, + threshold: required_count_u16(required_success_count)?, + }); } if let Some((class_label, fingerprints)) = value .strip_prefix("required_") @@ -2767,7 +2777,7 @@ mod tests { .expect("required targets policy"); let stored = satisfaction_policy_storage_value(&policy); - let mut fingerprints = vec![first.fingerprint.as_str(), second.fingerprint.as_str()]; + let mut fingerprints = [first.fingerprint.as_str(), second.fingerprint.as_str()]; fingerprints.sort(); assert_eq!( stored, diff --git a/crates/protocol_contract_v1/src/lib.rs b/crates/protocol_contract_v1/src/lib.rs @@ -460,6 +460,26 @@ mod tests { } #[test] + fn transport_kind_v1_parses_current_and_unknown_identities() { + for (raw, expected) in [ + ("local", TransportKindV1::Local), + ("nostr", TransportKindV1::Nostr), + ("reticulum", TransportKindV1::Reticulum), + ] { + let parsed = TransportKindV1::parse(raw).expect("current transport parses"); + assert_eq!(parsed, expected); + assert_eq!(parsed.as_str(), raw); + } + + assert_eq!( + TransportKindV1::parse("unknown_transport") + .expect_err("unknown transport") + .to_string(), + "unknown transport kind unknown_transport" + ); + } + + #[test] fn transport_kind_v1_rejects_retired_identities() { for identity in [ "reticulum_preview", @@ -472,6 +492,12 @@ mod tests { TransportKindV1::parse(identity), Err(ProtocolContractErrorV1::RetiredTransportIdentity { .. }) )); + assert_eq!( + TransportKindV1::parse(identity) + .expect_err("retired transport") + .to_string(), + alloc::format!("retired transport identity {identity}") + ); } } @@ -505,4 +531,92 @@ mod tests { "local_preview" ); } + + #[test] + fn reticulum_target_newtypes_reject_invalid_values() { + for value in ["", " local", "local ", "local/scope"] { + assert_eq!( + MeshScopeIdV1::parse(value) + .expect_err("invalid mesh scope") + .to_string(), + "invalid mesh scope id" + ); + } + + for value in [ + "", + " reticulum:local", + "reticulum:local ", + "reticulum:\nlocal", + ] { + assert_eq!( + ReticulumDestinationV1::parse(value) + .expect_err("invalid destination") + .to_string(), + "invalid Reticulum destination" + ); + } + } + + #[test] + fn validation_reports_transport_catalog_errors() { + let local = TRANSPORT_CAPABILITY_CATALOG_V1[0]; + let nostr = TRANSPORT_CAPABILITY_CATALOG_V1[1]; + let reticulum = TRANSPORT_CAPABILITY_CATALOG_V1[2]; + + assert_eq!( + validate_transport_capability_catalog(&[local, local]) + .expect_err("duplicate kind") + .to_string(), + "duplicate transport kind local" + ); + assert_eq!( + validate_transport_capability_catalog(&[nostr, reticulum]) + .expect_err("missing required kind") + .to_string(), + "missing required transport local" + ); + } + + #[test] + fn validation_reports_event_and_schema_catalog_errors() { + let event = PROTOCOL_EVENT_CATALOG_V1[0]; + let duplicate_name = ProtocolEventDescriptorV1 { + name: event.name, + kind: u32::MAX, + event_class: ProtocolEventClassV1::Regular, + purpose: "duplicate name", + }; + let duplicate_kind = ProtocolEventDescriptorV1 { + name: "duplicate_kind", + kind: event.kind, + event_class: ProtocolEventClassV1::Regular, + purpose: "duplicate kind", + }; + let schema = PROTOCOL_SCHEMA_METADATA_V1[0]; + let duplicate_schema = ProtocolSchemaMetadataV1 { + type_name: "Duplicate", + schema_id: schema.schema_id, + schema_version: schema.schema_version, + }; + + assert_eq!( + validate_event_catalog(&[event, duplicate_name]) + .expect_err("duplicate event name") + .to_string(), + alloc::format!("duplicate event name {}", event.name) + ); + assert_eq!( + validate_event_catalog(&[event, duplicate_kind]) + .expect_err("duplicate event kind") + .to_string(), + alloc::format!("duplicate event kind {}", event.kind) + ); + assert_eq!( + validate_schema_metadata(&[schema, duplicate_schema]) + .expect_err("duplicate schema id") + .to_string(), + alloc::format!("duplicate schema id {}", schema.schema_id) + ); + } } diff --git a/crates/runtime_store/src/migrations.rs b/crates/runtime_store/src/migrations.rs @@ -38,13 +38,13 @@ where #[cfg(test)] mod tests { - use radroots_sql_core::SqliteExecutor; + use radroots_sql_core::SqlxSqliteExecutor; use super::*; #[test] fn migration_entrypoints_apply_and_reverse_schema() { - let executor = SqliteExecutor::open_memory().expect("open memory sqlite"); + let executor = SqlxSqliteExecutor::open_memory().expect("open memory sqlite"); run_all_up(&executor).expect("migrate up"); executor diff --git a/crates/runtime_store/src/store.rs b/crates/runtime_store/src/store.rs @@ -390,13 +390,13 @@ mod tests { use std::sync::Mutex; use std::sync::atomic::{AtomicUsize, Ordering}; - use radroots_sql_core::{ExecOutcome, SqlExecutor, SqliteExecutor}; + use radroots_sql_core::{ExecOutcome, SqlExecutor, SqlxSqliteExecutor}; use serde_json::json; use super::*; - fn store() -> RuntimeStore<SqliteExecutor> { - let executor = SqliteExecutor::open_memory().expect("open memory sqlite"); + fn store() -> RuntimeStore<SqlxSqliteExecutor> { + let executor = SqlxSqliteExecutor::open_memory().expect("open memory sqlite"); let store = RuntimeStore::new(executor); store.migrate_up().expect("migrate up"); store diff --git a/crates/runtime_store/tests/store.rs b/crates/runtime_store/tests/store.rs @@ -3,11 +3,11 @@ use radroots_runtime_store::{ RuntimeStoreRecordInput, RuntimeStoreRecordStatus, RuntimeStoreRecordUpdate, SourceRuntime, }; use radroots_sql_core::migrations::migrations_run_all_up; -use radroots_sql_core::{SqlExecutor, SqliteExecutor}; +use radroots_sql_core::{SqlExecutor, SqlxSqliteExecutor}; use serde_json::json; -fn store() -> RuntimeStore<SqliteExecutor> { - let executor = SqliteExecutor::open_memory().expect("open memory sqlite"); +fn store() -> RuntimeStore<SqlxSqliteExecutor> { + let executor = SqlxSqliteExecutor::open_memory().expect("open memory sqlite"); let store = RuntimeStore::new(executor); store.migrate_up().expect("migrate runtime store"); store @@ -296,7 +296,7 @@ fn changed_latest_is_not_blocked_by_older_record_volume() { #[test] fn migration_assigns_existing_records_change_seq_from_insert_order() { - let executor = SqliteExecutor::open_memory().expect("open memory sqlite"); + let executor = SqlxSqliteExecutor::open_memory().expect("open memory sqlite"); migrations_run_all_up(&executor, &MIGRATIONS[..1]).expect("apply initial migration"); let first = insert_pre_change_tracking_record(&executor, "local-a"); let second = insert_pre_change_tracking_record(&executor, "local-b"); @@ -316,7 +316,7 @@ fn migration_assigns_existing_records_change_seq_from_insert_order() { #[test] fn migration_repairs_pre_network_source_runtime_constraint() { - let executor = SqliteExecutor::open_memory().expect("open memory sqlite"); + let executor = SqlxSqliteExecutor::open_memory().expect("open memory sqlite"); create_pre_network_change_tracking_schema(&executor); let legacy_seq = insert_pre_network_change_tracking_record(&executor, "legacy-cli", 1); let store = RuntimeStore::new(executor); @@ -345,7 +345,7 @@ fn migration_repairs_pre_network_source_runtime_constraint() { assert_eq!(rows[1].source_runtime, SourceRuntime::Network); } -fn insert_pre_change_tracking_record(executor: &SqliteExecutor, record_id: &str) -> i64 { +fn insert_pre_change_tracking_record(executor: &SqlxSqliteExecutor, record_id: &str) -> i64 { let input = local_work(record_id); let params = json!([ input.record_id, @@ -412,7 +412,7 @@ fn insert_pre_change_tracking_record(executor: &SqliteExecutor, record_id: &str) outcome.last_insert_id } -fn create_pre_network_change_tracking_schema(executor: &SqliteExecutor) { +fn create_pre_network_change_tracking_schema(executor: &SqlxSqliteExecutor) { let schema = [ "create table __migrations(id integer primary key, name text not null unique, applied_at text not null default (datetime('now')))", "create table runtime_store_record ( @@ -472,7 +472,7 @@ fn create_pre_network_change_tracking_schema(executor: &SqliteExecutor) { } fn insert_pre_network_change_tracking_record( - executor: &SqliteExecutor, + executor: &SqlxSqliteExecutor, record_id: &str, change_seq: i64, ) -> i64 { diff --git a/crates/trade_sp1_host/build.rs b/crates/trade_sp1_host/build.rs @@ -9,18 +9,22 @@ fn main() { if std::env::var(run_real_proof_tests_env).as_deref() == Ok("1") { println!("cargo:rustc-cfg=radroots_sp1_real_proof_tests"); } - if std::env::var(build_guest_elf_env).as_deref() != Ok("1") { - return; - } - #[cfg(feature = "sp1_verify")] - { - let args = sp1_build::BuildArgs { - binaries: vec!["radroots_sp1_trade_order_acceptance_guest".to_string()], - features: vec!["sp1_guest".to_string()], - locked: true, - ..sp1_build::BuildArgs::default() - }; - sp1_build::build_program_with_args("../trade_sp1_guest", args); - println!("cargo:rustc-cfg=radroots_sp1_guest_elf"); + if std::env::var(build_guest_elf_env).as_deref() == Ok("1") { + #[cfg(feature = "sp1_verify")] + { + build_guest_elf(); + } } } + +#[cfg(feature = "sp1_verify")] +fn build_guest_elf() { + let args = sp1_build::BuildArgs { + binaries: vec!["radroots_sp1_trade_order_acceptance_guest".to_string()], + features: vec!["sp1_guest".to_string()], + locked: true, + ..sp1_build::BuildArgs::default() + }; + sp1_build::build_program_with_args("../trade_sp1_guest", args); + println!("cargo:rustc-cfg=radroots_sp1_guest_elf"); +} diff --git a/crates/transport/src/target.rs b/crates/transport/src/target.rs @@ -389,7 +389,7 @@ fn canonicalize_nostr_relay_authority( let host = parts.next().unwrap_or_default(); let port = parts .next() - .map(|port| parse_nostr_relay_port_with_prefix(port)) + .map(parse_nostr_relay_port_with_prefix) .transpose()?; if host.is_empty() || !is_valid_nostr_relay_host(host) { return Err(RadrootsTransportError::InvalidTargetUri); diff --git a/crates/transport/tests/source_boundary.rs b/crates/transport/tests/source_boundary.rs @@ -672,9 +672,8 @@ fn required_target_semantics_stay_fingerprint_exact() { ); for required in [ "RadrootsTransportSatisfactionPolicy::RequiredTargets { targets, .. } =>", - "required_targets.as_ref().is_none_or(|required|", - "target.endpoint_fingerprint == *fingerprint", - "let required_for_satisfaction = required_targets.as_ref().is_some_and(|required|", + ".is_none_or(|required| required.contains(&target.endpoint_fingerprint))", + ".is_some_and(|required| required.contains(&target.endpoint_fingerprint))", "required_targets.is_none() || required_for_satisfaction", "required_targets.is_some()", ] { diff --git a/crates/transport/tests/transport.rs b/crates/transport/tests/transport.rs @@ -1030,7 +1030,7 @@ fn required_target_satisfaction_uses_fingerprints_not_target_counts() { policy .required_target_fingerprints() .expect("required targets"), - &[required.fingerprint.clone()] + std::slice::from_ref(&required.fingerprint) ); let unordered_policy = RadrootsTransportSatisfactionPolicy::required_targets( RadrootsTransportSatisfactionClass::Accepted, diff --git a/crates/transport_nostr/src/outbox.rs b/crates/transport_nostr/src/outbox.rs @@ -426,11 +426,10 @@ impl PublishableRelays { receipt .transport_status .counts_as_satisfied(self.satisfaction_class) - && self.required_targets.as_ref().is_none_or(|required| { - required - .iter() - .any(|fingerprint| receipt.endpoint_fingerprint == *fingerprint) - }) + && self + .required_targets + .as_ref() + .is_none_or(|required| required.contains(&receipt.endpoint_fingerprint)) }) .count() } @@ -730,13 +729,13 @@ fn publishable_transport_targets( relay .target_scope .as_deref() - .map(|scope| radroots_transport::RadrootsTransportMeshScopeId::parse(scope)) + .map(radroots_transport::RadrootsTransportMeshScopeId::parse) .transpose() .map_err(transport_error_to_relay_error)?, relay .target_label .as_deref() - .map(|label| radroots_transport::RadrootsTransportTargetLabel::parse(label)) + .map(radroots_transport::RadrootsTransportTargetLabel::parse) .transpose() .map_err(transport_error_to_relay_error)?, ) @@ -844,13 +843,12 @@ async fn publishable_relays( let satisfied_count = active_targets .iter() .filter(|target| { - required_targets.as_ref().is_none_or(|required| { - required - .iter() - .any(|fingerprint| target.endpoint_fingerprint == *fingerprint) - }) && target - .status - .counts_as_transport_satisfaction(satisfaction_class) + required_targets + .as_ref() + .is_none_or(|required| required.contains(&target.endpoint_fingerprint)) + && target + .status + .counts_as_transport_satisfaction(satisfaction_class) }) .count(); let remaining_satisfaction_count = @@ -875,11 +873,9 @@ async fn publishable_relays( if !is_nostr_target(target) { continue; } - let required_for_satisfaction = required_targets.as_ref().is_some_and(|required| { - required - .iter() - .any(|fingerprint| target.endpoint_fingerprint == *fingerprint) - }); + let required_for_satisfaction = required_targets + .as_ref() + .is_some_and(|required| required.contains(&target.endpoint_fingerprint)); if target .status .counts_as_transport_satisfaction(RadrootsTransportSatisfactionClass::Accepted) diff --git a/crates/transport_nostr/tests/transport.rs b/crates/transport_nostr/tests/transport.rs @@ -730,11 +730,8 @@ async fn nostr_transport_facade_delivers_signed_event_payloads() { let target = nostr_target(RELAY_PRIMARY_WSS); let request = RadrootsTransportDeliveryRequest::new( "facade-request-1", - RadrootsTransportPayload::unchecked_signed_event_json( - signed.id_str().to_owned(), - signed.raw_json().to_owned(), - ) - .expect("payload"), + RadrootsTransportPayload::unchecked_signed_event_json(signed.id_str(), signed.raw_json()) + .expect("payload"), RadrootsTransportTargetSet::new(vec![target.clone()]).expect("targets"), RadrootsTransportSatisfactionPolicy::all_accepted(), ); @@ -818,8 +815,8 @@ async fn nostr_transport_facade_rejects_unsupported_payloads_and_targets() { .deliver(RadrootsTransportDeliveryRequest::new( "facade-request-target", RadrootsTransportPayload::unchecked_signed_event_json( - signed.id_str().to_owned(), - signed.raw_json().to_owned(), + signed.id_str(), + signed.raw_json(), ) .expect("payload"), RadrootsTransportTargetSet::new(vec![non_nostr_target]).expect("targets"), @@ -844,8 +841,8 @@ async fn nostr_transport_facade_matches_canonical_equivalent_relay_receipts() { .deliver(RadrootsTransportDeliveryRequest::new( "facade-canonical-receipt", RadrootsTransportPayload::unchecked_signed_event_json( - signed.id_str().to_owned(), - signed.raw_json().to_owned(), + signed.id_str(), + signed.raw_json(), ) .expect("payload"), RadrootsTransportTargetSet::new(vec![target.clone()]).expect("target set"), @@ -891,11 +888,8 @@ async fn nostr_transport_facade_preserves_scoped_duplicate_target_metadata() { .expect("required targets"); let request = RadrootsTransportDeliveryRequest::new( "facade-request-scoped", - RadrootsTransportPayload::unchecked_signed_event_json( - signed.id_str().to_owned(), - signed.raw_json().to_owned(), - ) - .expect("payload"), + RadrootsTransportPayload::unchecked_signed_event_json(signed.id_str(), signed.raw_json()) + .expect("payload"), RadrootsTransportTargetSet::new(vec![first.clone(), second.clone()]).expect("targets"), policy.clone(), ); diff --git a/crates/transport_publish_protocol/src/lib.rs b/crates/transport_publish_protocol/src/lib.rs @@ -254,18 +254,13 @@ impl std::error::Error for TransportPublishProtocolError {} #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] #[cfg_attr(feature = "serde", serde(rename_all = "snake_case"))] -#[derive(Clone, Copy, Debug, PartialEq, Eq)] +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] pub enum TransportPublishPreviewBehavior { + #[default] RejectDeliveryAttempts, DeferDeliveryPlans, } -impl Default for TransportPublishPreviewBehavior { - fn default() -> Self { - Self::RejectDeliveryAttempts - } -} - #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] #[cfg_attr(feature = "serde", serde(deny_unknown_fields))] #[derive(Clone, Debug, PartialEq, Eq)] diff --git a/crates/transport_reticulum/src/lib.rs b/crates/transport_reticulum/src/lib.rs @@ -25,18 +25,13 @@ const DEFERRED_CODE: &str = "deferred_until_implemented"; const DEFERRED_MESSAGE: &str = "Reticulum preview delivery is deferred until implementation"; #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] -#[derive(Clone, Copy, Debug, PartialEq, Eq)] +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] pub enum RadrootsReticulumPreviewBehavior { + #[default] RejectDeliveryAttempts, DeferDeliveryPlans, } -impl Default for RadrootsReticulumPreviewBehavior { - fn default() -> Self { - Self::RejectDeliveryAttempts - } -} - #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] #[derive(Clone, Debug, PartialEq, Eq)] pub struct RadrootsReticulumPreviewEndpoint { diff --git a/tools/xtask/src/contract.rs b/tools/xtask/src/contract.rs @@ -2363,7 +2363,9 @@ fn coverage_required_workspace_crates(workspace_root: &Path) -> Result<BTreeSet< } fn coverage_policy_excludes_workspace_crate(crate_name: &str) -> bool { - crate_name.contains("_simplex_") || crate_name.starts_with("simplex_") + crate_name == "libsqlite3-sys" + || crate_name.contains("_simplex_") + || crate_name.starts_with("simplex_") } #[cfg_attr(not(test), allow(dead_code))] @@ -4686,12 +4688,12 @@ pub enum RadrootsCoreUnitDimension { } #[test] - fn coverage_required_workspace_crates_excludes_simplex_packages() { + fn coverage_required_workspace_crates_excludes_non_policy_packages() { let root = temp_root("coverage_required_workspace_simplex"); write_file( &root.join("Cargo.toml"), r#"[workspace] -members = ["crates/a", "crates/radroots_simplex_probe", "crates/simplex_probe"] +members = ["crates/a", "crates/libsqlite3_sys_3_53_3", "crates/radroots_simplex_probe", "crates/simplex_probe"] resolver = "2" "#, ); @@ -4706,6 +4708,17 @@ edition = "2024" write_file( &root .join("crates") + .join("libsqlite3_sys_3_53_3") + .join("Cargo.toml"), + r#"[package] +name = "libsqlite3-sys" +version = "0.1.0" +edition = "2024" +"#, + ); + write_file( + &root + .join("crates") .join("radroots_simplex_probe") .join("Cargo.toml"), r#"[package] @@ -4731,6 +4744,7 @@ edition = "2024" .into_iter() .collect::<BTreeSet<_>>() ); + assert!(coverage_policy_excludes_workspace_crate("libsqlite3-sys")); assert!(coverage_policy_excludes_workspace_crate( "radroots_simplex_probe" ));