From 0459d3f748d8dca40c84c0ef7b97613f798adf92 Mon Sep 17 00:00:00 2001 From: Yuri Astrakhan Date: Tue, 3 Oct 2023 21:20:41 -0400 Subject: [PATCH] add encoding=UTF8 pragma, refactoring (#919) --- Cargo.toml | 4 +- justfile | 2 +- ...05f2a7999788167f41c685af3ca6f5a1359f4.json | 12 +++ martin-mbtiles/src/bin/main.rs | 20 ++-- martin-mbtiles/src/copier.rs | 102 ++++++++---------- martin-mbtiles/src/lib.rs | 2 +- martin-mbtiles/src/mbtiles.rs | 28 +++-- martin-mbtiles/src/queries.rs | 32 +++--- 8 files changed, 112 insertions(+), 90 deletions(-) create mode 100644 martin-mbtiles/.sqlx/query-428a035a55a07cbb9daac42c3ab05f2a7999788167f41c685af3ca6f5a1359f4.json diff --git a/Cargo.toml b/Cargo.toml index 173ab1b41..3f806e0b6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -55,6 +55,6 @@ tilejson = "0.3" tokio = { version = "1.32.0", features = ["macros"] } tokio-postgres-rustls = "0.10" -[profile.dev.package.sqlx-macros] +[profile.dev.package] # See https://github.com/launchbadge/sqlx#compile-time-verification -opt-level = 3 +sqlx-macros.opt-level = 3 diff --git a/justfile b/justfile index b5484a2e5..b60808eb2 100644 --- a/justfile +++ b/justfile @@ -134,7 +134,7 @@ test-int: clean-test install-sqlx # Run integration tests and save its output as the new expected output bless: start clean-test rm -rf tests/temp - cargo test --features bless-tests + cargo test -p martin --features bless-tests tests/test.sh rm -rf tests/expected mv tests/output tests/expected diff --git a/martin-mbtiles/.sqlx/query-428a035a55a07cbb9daac42c3ab05f2a7999788167f41c685af3ca6f5a1359f4.json b/martin-mbtiles/.sqlx/query-428a035a55a07cbb9daac42c3ab05f2a7999788167f41c685af3ca6f5a1359f4.json new file mode 100644 index 000000000..83f5d8a66 --- /dev/null +++ b/martin-mbtiles/.sqlx/query-428a035a55a07cbb9daac42c3ab05f2a7999788167f41c685af3ca6f5a1359f4.json @@ -0,0 +1,12 @@ +{ + "db_name": "SQLite", + "query": "PRAGMA encoding = 'UTF-8'", + "describe": { + "columns": [], + "parameters": { + "Right": 0 + }, + "nullable": [] + }, + "hash": "428a035a55a07cbb9daac42c3ab05f2a7999788167f41c685af3ca6f5a1359f4" +} diff --git a/martin-mbtiles/src/bin/main.rs b/martin-mbtiles/src/bin/main.rs index c587428d1..a555794d0 100644 --- a/martin-mbtiles/src/bin/main.rs +++ b/martin-mbtiles/src/bin/main.rs @@ -2,7 +2,7 @@ use std::path::{Path, PathBuf}; use clap::{Parser, Subcommand}; use log::{error, LevelFilter}; -use martin_mbtiles::{apply_mbtiles_diff, IntegrityCheckType, MbtResult, Mbtiles, MbtilesCopier}; +use martin_mbtiles::{apply_diff, IntegrityCheckType, MbtResult, Mbtiles, MbtilesCopier}; #[derive(Parser, PartialEq, Eq, Debug)] #[command( @@ -104,14 +104,14 @@ async fn main_int() -> anyhow::Result<()> { src_file, diff_file, } => { - apply_mbtiles_diff(src_file, diff_file).await?; + apply_diff(src_file, diff_file).await?; } Commands::Validate { file, integrity_check, update_agg_tiles_hash, } => { - validate_mbtiles(file.as_path(), integrity_check, update_agg_tiles_hash).await?; + validate(file.as_path(), integrity_check, update_agg_tiles_hash).await?; } } @@ -120,7 +120,7 @@ async fn main_int() -> anyhow::Result<()> { async fn meta_print_all(file: &Path) -> anyhow::Result<()> { let mbt = Mbtiles::new(file)?; - let mut conn = mbt.open_with_hashes(true).await?; + let mut conn = mbt.open_readonly().await?; let metadata = mbt.get_metadata(&mut conn).await?; println!("{}", serde_yaml::to_string(&metadata)?); Ok(()) @@ -128,7 +128,7 @@ async fn meta_print_all(file: &Path) -> anyhow::Result<()> { async fn meta_get_value(file: &Path, key: &str) -> MbtResult<()> { let mbt = Mbtiles::new(file)?; - let mut conn = mbt.open_with_hashes(true).await?; + let mut conn = mbt.open_readonly().await?; if let Some(s) = mbt.get_metadata_value(&mut conn, key).await? { println!("{s}"); } @@ -137,17 +137,21 @@ async fn meta_get_value(file: &Path, key: &str) -> MbtResult<()> { async fn meta_set_value(file: &Path, key: &str, value: Option) -> MbtResult<()> { let mbt = Mbtiles::new(file)?; - let mut conn = mbt.open_with_hashes(false).await?; + let mut conn = mbt.open().await?; mbt.set_metadata_value(&mut conn, key, value).await } -async fn validate_mbtiles( +async fn validate( file: &Path, check_type: IntegrityCheckType, update_agg_tiles_hash: bool, ) -> MbtResult<()> { let mbt = Mbtiles::new(file)?; - let mut conn = mbt.open_with_hashes(!update_agg_tiles_hash).await?; + let mut conn = if update_agg_tiles_hash { + mbt.open().await? + } else { + mbt.open_readonly().await? + }; mbt.check_integrity(&mut conn, check_type).await?; mbt.check_each_tile_hash(&mut conn).await?; if update_agg_tiles_hash { diff --git a/martin-mbtiles/src/copier.rs b/martin-mbtiles/src/copier.rs index 418f4c908..ba1a6a9bd 100644 --- a/martin-mbtiles/src/copier.rs +++ b/martin-mbtiles/src/copier.rs @@ -6,14 +6,14 @@ use clap::{builder::ValueParser, error::ErrorKind, Args, ValueEnum}; use sqlite_hashes::rusqlite; use sqlite_hashes::rusqlite::params_from_iter; use sqlx::sqlite::SqliteConnectOptions; -use sqlx::{query, Connection, Row, SqliteConnection}; +use sqlx::{query, Connection, Executor as _, Row, SqliteConnection}; use crate::errors::MbtResult; use crate::mbtiles::MbtType::{Flat, FlatWithHash, Normalized}; use crate::mbtiles::{attach_hash_fn, MbtType}; use crate::queries::{ - create_flat_tables, create_flat_with_hash_tables, create_metadata_table, - create_normalized_tables, create_tiles_with_hash_view, + create_flat_tables, create_flat_with_hash_tables, create_normalized_tables, + create_tiles_with_hash_view, }; use crate::{MbtError, Mbtiles}; @@ -187,8 +187,7 @@ impl MbtileCopierInt { let dst_type = if is_empty { let dst_type = self.options.dst_type.unwrap_or(src_type); - self.create_new_mbtiles(&mut conn, src_type, dst_type) - .await?; + self.copy_to_new(&mut conn, src_type, dst_type).await?; dst_type } else if self.options.diff_with_file.is_some() { return Err(MbtError::NonEmptyTargetFile(self.options.dst_file)); @@ -257,39 +256,41 @@ impl MbtileCopierInt { Ok(conn) } - async fn create_new_mbtiles( + async fn copy_to_new( &self, conn: &mut SqliteConnection, src: MbtType, dst: MbtType, ) -> MbtResult<()> { query!("PRAGMA page_size = 512").execute(&mut *conn).await?; + query!("PRAGMA encoding = 'UTF-8'") + .execute(&mut *conn) + .await?; query!("VACUUM").execute(&mut *conn).await?; self.src_mbtiles.attach_to(&mut *conn, "sourceDb").await?; if src == dst { // DB objects must be created in a specific order: tables, views, triggers, indexes. - let sql_objects = query( - "SELECT sql - FROM sourceDb.sqlite_schema - WHERE tbl_name IN ('metadata', 'tiles', 'map', 'images', 'tiles_with_hash') - AND type IN ('table', 'view', 'trigger', 'index') - ORDER BY CASE - WHEN type = 'table' THEN 1 - WHEN type = 'view' THEN 2 - WHEN type = 'trigger' THEN 3 - WHEN type = 'index' THEN 4 - ELSE 5 END", - ) - .fetch_all(&mut *conn) - .await?; + let sql_objects = conn + .fetch_all( + "SELECT sql + FROM sourceDb.sqlite_schema + WHERE tbl_name IN ('metadata', 'tiles', 'map', 'images', 'tiles_with_hash') + AND type IN ('table', 'view', 'trigger', 'index') + ORDER BY CASE + WHEN type = 'table' THEN 1 + WHEN type = 'view' THEN 2 + WHEN type = 'trigger' THEN 3 + WHEN type = 'index' THEN 4 + ELSE 5 END", + ) + .await?; for row in sql_objects { query(row.get(0)).execute(&mut *conn).await?; } } else { - create_metadata_table(&mut *conn).await?; match dst { Flat => create_flat_tables(&mut *conn).await?, FlatWithHash => create_flat_with_hash_tables(&mut *conn).await?, @@ -302,8 +303,7 @@ impl MbtileCopierInt { create_tiles_with_hash_view(&mut *conn).await?; } - query("INSERT INTO metadata SELECT * FROM sourceDb.metadata") - .execute(&mut *conn) + conn.execute("INSERT INTO metadata SELECT * FROM sourceDb.metadata") .await?; Ok(()) @@ -406,12 +406,12 @@ impl MbtileCopierInt { } } -pub async fn apply_mbtiles_diff(src_file: PathBuf, diff_file: PathBuf) -> MbtResult<()> { +pub async fn apply_diff(src_file: PathBuf, diff_file: PathBuf) -> MbtResult<()> { let src_mbtiles = Mbtiles::new(src_file)?; let diff_mbtiles = Mbtiles::new(diff_file)?; let diff_type = diff_mbtiles.open_and_detect_type().await?; - let mut conn = src_mbtiles.open_with_hashes(false).await?; + let mut conn = src_mbtiles.open().await?; diff_mbtiles.attach_to(&mut conn, "diffDb").await?; let src_type = src_mbtiles.detect_type(&mut conn).await?; @@ -491,12 +491,10 @@ mod tests { expected_dst_type ); - assert!( - query("SELECT * FROM srcDb.tiles EXCEPT SELECT * FROM tiles") - .fetch_optional(&mut dst_conn) - .await? - .is_none() - ); + assert!(dst_conn + .fetch_optional("SELECT * FROM srcDb.tiles EXCEPT SELECT * FROM tiles") + .await? + .is_none()); Ok(()) } @@ -626,8 +624,8 @@ mod tests { let mut dst_conn = copy_opts.run().await?; - assert!(query("SELECT 1 FROM sqlite_schema WHERE name = 'tiles';") - .fetch_optional(&mut dst_conn) + assert!(dst_conn + .fetch_optional("SELECT 1 FROM sqlite_schema WHERE name = 'tiles';") .await? .is_some()); @@ -712,12 +710,10 @@ mod tests { Mbtiles::new(src_file)? .attach_to(&mut dst_conn, "otherDb") .await?; - assert!( - query("SELECT * FROM otherDb.tiles EXCEPT SELECT * FROM tiles;") - .fetch_optional(&mut dst_conn) - .await? - .is_none() - ); + assert!(dst_conn + .fetch_optional("SELECT * FROM otherDb.tiles EXCEPT SELECT * FROM tiles;") + .await? + .is_none()); Ok(()) } @@ -750,7 +746,8 @@ mod tests { // Create a temporary table with all the tiles in the original database and // all the tiles in the source database except for those that conflict with tiles in the original database - query("CREATE TEMP TABLE expected_tiles AS + dst_conn.execute( + "CREATE TEMP TABLE expected_tiles AS SELECT COALESCE(t1.zoom_level, t2.zoom_level) as zoom_level, COALESCE(t1.tile_column, t2.zoom_level) as tile_column, COALESCE(t1.tile_row, t2.tile_row) as tile_row, @@ -758,7 +755,6 @@ mod tests { FROM originalDb.tiles as t1 FULL OUTER JOIN srcDb.tiles as t2 ON t1.zoom_level = t2.zoom_level AND t1.tile_column = t2.tile_column AND t1.tile_row = t2.tile_row") - .execute(&mut dst_conn) .await?; // Ensure all entries in expected_tiles are in tiles and vice versa @@ -786,19 +782,17 @@ mod tests { // Apply diff to the src data in in-memory DB let diff_file = PathBuf::from("../tests/fixtures/mbtiles/world_cities_diff.mbtiles"); - apply_mbtiles_diff(src, diff_file).await?; + apply_diff(src, diff_file).await?; // Verify the data is the same as the file the diff was generated from Mbtiles::new("../tests/fixtures/mbtiles/world_cities_modified.mbtiles")? .attach_to(&mut src_conn, "otherDb") .await?; - assert!( - query("SELECT * FROM tiles EXCEPT SELECT * FROM otherDb.tiles;") - .fetch_optional(&mut src_conn) - .await? - .is_none() - ); + assert!(src_conn + .fetch_optional("SELECT * FROM tiles EXCEPT SELECT * FROM otherDb.tiles;") + .await? + .is_none()); Ok(()) } @@ -815,19 +809,17 @@ mod tests { // Apply diff to the src data in in-memory DB let diff_file = PathBuf::from("../tests/fixtures/mbtiles/geography-class-jpg-diff.mbtiles"); - apply_mbtiles_diff(src, diff_file).await?; + apply_diff(src, diff_file).await?; // Verify the data is the same as the file the diff was generated from Mbtiles::new("../tests/fixtures/mbtiles/geography-class-jpg-modified.mbtiles")? .attach_to(&mut src_conn, "otherDb") .await?; - assert!( - query("SELECT * FROM tiles EXCEPT SELECT * FROM otherDb.tiles;") - .fetch_optional(&mut src_conn) - .await? - .is_none() - ); + assert!(src_conn + .fetch_optional("SELECT * FROM tiles EXCEPT SELECT * FROM otherDb.tiles;") + .await? + .is_none()); Ok(()) } diff --git a/martin-mbtiles/src/lib.rs b/martin-mbtiles/src/lib.rs index 66ce9d40f..dff5b1052 100644 --- a/martin-mbtiles/src/lib.rs +++ b/martin-mbtiles/src/lib.rs @@ -10,6 +10,6 @@ mod pool; pub use pool::MbtilesPool; mod copier; -pub use copier::{apply_mbtiles_diff, CopyDuplicateMode, MbtilesCopier}; +pub use copier::{apply_diff, CopyDuplicateMode, MbtilesCopier}; mod queries; diff --git a/martin-mbtiles/src/mbtiles.rs b/martin-mbtiles/src/mbtiles.rs index 6c28acfc3..6cedee852 100644 --- a/martin-mbtiles/src/mbtiles.rs +++ b/martin-mbtiles/src/mbtiles.rs @@ -93,11 +93,27 @@ impl Mbtiles { }) } - pub async fn open_with_hashes(&self, readonly: bool) -> MbtResult { + pub async fn open(&self) -> MbtResult { + let opt = SqliteConnectOptions::new().filename(self.filepath()); + Self::open_int(&opt).await + } + + pub async fn open_or_new(&self) -> MbtResult { + let opt = SqliteConnectOptions::new() + .filename(self.filepath()) + .create_if_missing(true); + Self::open_int(&opt).await + } + + pub async fn open_readonly(&self) -> MbtResult { let opt = SqliteConnectOptions::new() .filename(self.filepath()) - .read_only(readonly); - let mut conn = SqliteConnection::connect_with(&opt).await?; + .read_only(true); + Self::open_int(&opt).await + } + + async fn open_int(opt: &SqliteConnectOptions) -> Result { + let mut conn = SqliteConnection::connect_with(opt).await?; attach_hash_fn(&mut conn).await?; Ok(conn) } @@ -373,7 +389,7 @@ impl Mbtiles { } pub async fn open_and_detect_type(&self) -> MbtResult { - let mut conn = self.open_with_hashes(true).await?; + let mut conn = self.open_readonly().await?; self.detect_type(&mut conn).await } @@ -615,6 +631,7 @@ mod tests { use std::collections::HashMap; use martin_tile_utils::Encoding; + use sqlx::Executor as _; use tilejson::VectorLayer; use super::*; @@ -706,8 +723,7 @@ mod tests { async fn metadata_set_key() -> MbtResult<()> { let (mut conn, mbt) = open("file:metadata_set_key_mem_db?mode=memory&cache=shared").await?; - query("CREATE TABLE metadata (name text NOT NULL PRIMARY KEY, value text);") - .execute(&mut conn) + conn.execute("CREATE TABLE metadata (name text NOT NULL PRIMARY KEY, value text);") .await?; mbt.set_metadata_value(&mut conn, "bounds", Some("0.0, 0.0, 0.0, 0.0".to_string())) diff --git a/martin-mbtiles/src/queries.rs b/martin-mbtiles/src/queries.rs index 9df2731bf..2986dab69 100644 --- a/martin-mbtiles/src/queries.rs +++ b/martin-mbtiles/src/queries.rs @@ -1,4 +1,4 @@ -use sqlx::{query, SqliteExecutor}; +use sqlx::{query, Executor as _, SqliteExecutor}; use crate::errors::MbtResult; @@ -125,12 +125,11 @@ pub async fn create_metadata_table(conn: &mut T) -> MbtResult<()> where for<'e> &'e mut T: SqliteExecutor<'e>, { - query( + conn.execute( "CREATE TABLE IF NOT EXISTS metadata ( name text NOT NULL PRIMARY KEY, value text);", ) - .execute(&mut *conn) .await?; Ok(()) @@ -140,7 +139,9 @@ pub async fn create_flat_tables(conn: &mut T) -> MbtResult<()> where for<'e> &'e mut T: SqliteExecutor<'e>, { - query( + create_metadata_table(&mut *conn).await?; + + conn.execute( "CREATE TABLE IF NOT EXISTS tiles ( zoom_level integer NOT NULL, tile_column integer NOT NULL, @@ -148,7 +149,6 @@ where tile_data blob, PRIMARY KEY(zoom_level, tile_column, tile_row));", ) - .execute(&mut *conn) .await?; Ok(()) @@ -158,7 +158,9 @@ pub async fn create_flat_with_hash_tables(conn: &mut T) -> MbtResult<()> where for<'e> &'e mut T: SqliteExecutor<'e>, { - query( + create_metadata_table(&mut *conn).await?; + + conn.execute( "CREATE TABLE IF NOT EXISTS tiles_with_hash ( zoom_level integer NOT NULL, tile_column integer NOT NULL, @@ -167,14 +169,12 @@ where tile_hash text, PRIMARY KEY(zoom_level, tile_column, tile_row));", ) - .execute(&mut *conn) .await?; - query( + conn.execute( "CREATE VIEW IF NOT EXISTS tiles AS SELECT zoom_level, tile_column, tile_row, tile_data FROM tiles_with_hash;", ) - .execute(&mut *conn) .await?; Ok(()) @@ -184,7 +184,9 @@ pub async fn create_normalized_tables(conn: &mut T) -> MbtResult<()> where for<'e> &'e mut T: SqliteExecutor<'e>, { - query( + create_metadata_table(&mut *conn).await?; + + conn.execute( "CREATE TABLE IF NOT EXISTS map ( zoom_level integer NOT NULL, tile_column integer NOT NULL, @@ -192,18 +194,16 @@ where tile_id text, PRIMARY KEY(zoom_level, tile_column, tile_row));", ) - .execute(&mut *conn) .await?; - query( + conn.execute( "CREATE TABLE IF NOT EXISTS images ( tile_data blob, tile_id text NOT NULL PRIMARY KEY);", ) - .execute(&mut *conn) .await?; - query( + conn.execute( "CREATE VIEW IF NOT EXISTS tiles AS SELECT map.zoom_level AS zoom_level, map.tile_column AS tile_column, @@ -212,7 +212,6 @@ where FROM map JOIN images ON images.tile_id = map.tile_id;", ) - .execute(&mut *conn) .await?; Ok(()) @@ -222,7 +221,7 @@ pub async fn create_tiles_with_hash_view(conn: &mut T) -> MbtResult<()> where for<'e> &'e mut T: SqliteExecutor<'e>, { - query( + conn.execute( "CREATE VIEW IF NOT EXISTS tiles_with_hash AS SELECT map.zoom_level AS zoom_level, @@ -233,7 +232,6 @@ where FROM map JOIN images ON images.tile_id = map.tile_id", ) - .execute(&mut *conn) .await?; Ok(())