Fix mbtiles validation, CI, and logging

justfile

@@ -110,6 +110,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
     tests/test.sh
     rm -rf tests/expected

martin-mbtiles/Cargo.toml

@@ -12,7 +12,7 @@ rust-version.workspace = true
 [features]
 # TODO: Disable "cli" feature in default builds
 default = ["cli", "native-tls"]
-cli = ["dep:anyhow", "dep:clap", "dep:tokio"]
+cli = ["dep:anyhow", "dep:clap", "dep:env_logger", "dep:tokio"]
 # One of the following two must be used
 native-tls = ["sqlx/runtime-tokio-native-tls"]
 rustls = ["sqlx/runtime-tokio-rustls"]
@@ -30,6 +30,7 @@ tilejson.workspace = true
 # Bin dependencies
 anyhow = { workspace = true, optional = true }
 clap = { workspace = true, optional = true }
+env_logger = { workspace = true, optional = true }
 serde_yaml.workspace = true
 sqlite-hashes.workspace = true
 tokio = { workspace = true, features = ["rt-multi-thread"], optional = true }

martin-mbtiles/src/bin/main.rs

@@ -1,11 +1,10 @@
 use std::path::{Path, PathBuf};
 
 use clap::{Parser, Subcommand};
+use log::{error, LevelFilter};
 use martin_mbtiles::{
     apply_mbtiles_diff, IntegrityCheckType, MbtResult, Mbtiles, TileCopierOptions,
 };
-use sqlx::sqlite::SqliteConnectOptions;
-use sqlx::{Connection, SqliteConnection};
 
 #[derive(Parser, PartialEq, Eq, Debug)]
 #[command(
@@ -73,9 +72,23 @@ enum Commands {
 }
 
 #[tokio::main]
-async fn main() -> anyhow::Result<()> {
-    let args = Args::parse();
+async fn main() {
+    env_logger::builder()
+        .filter_level(LevelFilter::Info)
+        .format_indent(None)
+        .format_module_path(false)
+        .format_target(false)
+        .format_timestamp(None)
+        .init();
 
+    if let Err(err) = main_int().await {
+        error!("{err}");
+        std::process::exit(1);
+    }
+}
+
+async fn main_int() -> anyhow::Result<()> {
+    let args = Args::parse();
     match args.command {
         Commands::MetaAll { file } => {
             meta_print_all(file.as_path()).await?;
@@ -109,17 +122,15 @@ async fn main() -> anyhow::Result<()> {
 
 async fn meta_print_all(file: &Path) -> anyhow::Result<()> {
     let mbt = Mbtiles::new(file)?;
-    let opt = SqliteConnectOptions::new().filename(file).read_only(true);
-    let mut conn = SqliteConnection::connect_with(&opt).await?;
+    let mut conn = mbt.open_with_hashes(true).await?;
     let metadata = mbt.get_metadata(&mut conn).await?;
     println!("{}", serde_yaml::to_string(&metadata)?);
     Ok(())
 }
 
 async fn meta_get_value(file: &Path, key: &str) -> MbtResult<()> {
     let mbt = Mbtiles::new(file)?;
-    let opt = SqliteConnectOptions::new().filename(file).read_only(true);
-    let mut conn = SqliteConnection::connect_with(&opt).await?;
+    let mut conn = mbt.open_with_hashes(true).await?;
     if let Some(s) = mbt.get_metadata_value(&mut conn, key).await? {
         println!("{s}");
     }
@@ -128,8 +139,7 @@ async fn meta_get_value(file: &Path, key: &str) -> MbtResult<()> {
 
 async fn meta_set_value(file: &Path, key: &str, value: Option<String>) -> MbtResult<()> {
     let mbt = Mbtiles::new(file)?;
-    let opt = SqliteConnectOptions::new().filename(file);
-    let mut conn = SqliteConnection::connect_with(&opt).await?;
+    let mut conn = mbt.open_with_hashes(false).await?;
     mbt.set_metadata_value(&mut conn, key, value).await
 }
 
@@ -139,8 +149,7 @@ async fn validate_mbtiles(
     update_agg_tiles_hash: bool,
 ) -> MbtResult<()> {
     let mbt = Mbtiles::new(file)?;
-    let opt = SqliteConnectOptions::new().filename(file).read_only(true);
-    let mut conn = SqliteConnection::connect_with(&opt).await?;
+    let mut conn = mbt.open_with_hashes(!update_agg_tiles_hash).await?;
     mbt.check_integrity(&mut conn, check_type).await?;
     mbt.check_each_tile_hash(&mut conn).await?;
     if update_agg_tiles_hash {

martin-mbtiles/src/mbtiles.rs

@@ -435,7 +435,9 @@ impl Mbtiles {
     where
         for<'e> &'e mut T: SqliteExecutor<'e>,
     {
+        let filepath = self.filepath();
         if integrity_check == IntegrityCheckType::Off {
+            info!("Skipping integrity check for {filepath}");
             return Ok(());
         }
 
@@ -452,30 +454,32 @@ impl Mbtiles {
 
         if result.len() > 1
             || result.get(0).ok_or(FailedIntegrityCheck(
-                self.filepath().to_string(),
+                filepath.to_string(),
                 vec!["SQLite could not perform integrity check".to_string()],
             ))? != "ok"
         {
             return Err(FailedIntegrityCheck(self.filepath().to_string(), result));
         }
 
+        info!("{integrity_check:?} integrity check passed for {filepath}");
         Ok(())
     }
 
     pub async fn check_agg_tiles_hashes<T>(&self, conn: &mut T) -> MbtResult<()>
     where
         for<'e> &'e mut T: SqliteExecutor<'e>,
     {
+        let filepath = self.filepath();
         let Some(stored) = self.get_agg_tiles_hash(&mut *conn).await? else {
-            return Err(AggHashValueNotFound(self.filepath().to_string()));
+            return Err(AggHashValueNotFound(filepath.to_string()));
         };
-
         let computed = calc_agg_tiles_hash(&mut *conn).await?;
         if stored != computed {
-            let file = self.filepath().to_string();
+            let file = filepath.to_string();
             return Err(AggHashMismatch(computed, stored, file));
         }
 
+        info!("The agg_tiles_hashes={computed} has been verified for {filepath}");
         Ok(())
     }
 
@@ -486,23 +490,15 @@ impl Mbtiles {
     {
         let old_hash = self.get_agg_tiles_hash(&mut *conn).await?;
         let hash = calc_agg_tiles_hash(&mut *conn).await?;
+        let path = self.filepath();
         if old_hash.as_ref() == Some(&hash) {
-            info!(
-                "agg_tiles_hash is already set to the correct value `{hash}` in {}",
-                self.filepath()
-            );
+            info!("agg_tiles_hash is already set to the correct value `{hash}` in {path}");
             Ok(())
         } else {
             if let Some(old_hash) = old_hash {
-                info!(
-                    "Updating agg_tiles_hash from {old_hash} to {hash} in {}",
-                    self.filepath()
-                );
+                info!("Updating agg_tiles_hash from {old_hash} to {hash} in {path}");
             } else {
-                info!(
-                    "Initializing agg_tiles_hash to {hash} in {}",
-                    self.filepath()
-                );
+                info!("Creating new metadata value agg_tiles_hash = {hash} in {path}");
             }
             self.set_metadata_value(&mut *conn, "agg_tiles_hash", Some(hash))
                 .await
@@ -550,7 +546,10 @@ impl Mbtiles {
                     v.get(0),
                     v.get(1),
                 ))
-            })
+            })?;
+
+        info!("All tile hashes are valid for {}", self.filepath());
+        Ok(())
     }
 }
 

tests/expected/auto/catalog_auto.json

@@ -8,6 +8,12 @@
       "content_type": "application/x-protobuf",
       "description": "autodetect.auto_table.geom"
     },
+    "bad_hash": {
+      "content_type": "application/x-protobuf",
+      "content_encoding": "gzip",
+      "name": "Major cities from Natural Earth data",
+      "description": "Major cities from Natural Earth data"
+    },
     "bigint_table": {
       "content_type": "application/x-protobuf",
       "description": "autodetect.bigint_table.geom"

tests/expected/generated_config.yaml

@@ -141,6 +141,7 @@ pmtiles:
 mbtiles:
   paths: tests/fixtures/files
   sources:
+    bad_hash: tests/fixtures/files/bad_hash.mbtiles
     geography-class-jpg: tests/fixtures/files/geography-class-jpg.mbtiles
     geography-class-jpg-diff: tests/fixtures/files/geography-class-jpg-diff.mbtiles
     geography-class-jpg-modified: tests/fixtures/files/geography-class-jpg-modified.mbtiles

tests/expected/mbtiles/copy_diff.txt

@@ -0,0 +1 @@
+[INFO ] Creating new metadata value agg_tiles_hash = C7E2E5A9BA04693994DB1F57D1DF5646 in tests/temp/world_cities_diff.mbtiles

tests/expected/mbtiles/validate-bad.txt

@@ -0,0 +1,3 @@
+[INFO ] Quick integrity check passed for ./tests/fixtures/files/bad_hash.mbtiles
+[INFO ] All tile hashes are valid for ./tests/fixtures/files/bad_hash.mbtiles
+[ERROR] Computed aggregate tiles hash D4E1030D57751A0B45A28A71267E46B8 does not match tile data in metadata CAFEC0DEDEADBEEFDEADBEEFDEADBEEF for MBTile file ./tests/fixtures/files/bad_hash.mbtiles

tests/expected/mbtiles/validate-fix.txt

@@ -0,0 +1,3 @@
+[INFO ] Quick integrity check passed for tests/temp/fix_bad_hash.mbtiles
+[INFO ] All tile hashes are valid for tests/temp/fix_bad_hash.mbtiles
+[INFO ] Updating agg_tiles_hash from CAFEC0DEDEADBEEFDEADBEEFDEADBEEF to D4E1030D57751A0B45A28A71267E46B8 in tests/temp/fix_bad_hash.mbtiles

tests/expected/mbtiles/validate-fix2.txt

@@ -0,0 +1,3 @@
+[INFO ] Quick integrity check passed for tests/temp/fix_bad_hash.mbtiles
+[INFO ] All tile hashes are valid for tests/temp/fix_bad_hash.mbtiles
+[INFO ] The agg_tiles_hashes=D4E1030D57751A0B45A28A71267E46B8 has been verified for tests/temp/fix_bad_hash.mbtiles

tests/expected/mbtiles/validate-ok.txt

@@ -0,0 +1,3 @@
+[INFO ] Quick integrity check passed for ./tests/fixtures/files/zoomed_world_cities.mbtiles
+[INFO ] All tile hashes are valid for ./tests/fixtures/files/zoomed_world_cities.mbtiles
+[INFO ] The agg_tiles_hashes=D4E1030D57751A0B45A28A71267E46B8 has been verified for ./tests/fixtures/files/zoomed_world_cities.mbtiles

tests/pg_table_source_test.rs

@@ -113,5 +113,5 @@ async fn table_source_schemas() {
           functions: false
     "});
     let sources = mock_sources(cfg).await.0;
-    assert_eq!(sources.keys().collect::<Vec<_>>(), vec!["MixPoints"],);
+    assert_eq!(sources.keys().collect::<Vec<_>>(), vec!["MixPoints"]);
 }

tests/test.sh

@@ -293,6 +293,19 @@ if [[ "$MBTILES_BIN" != "-" ]]; then
   $MBTILES_BIN meta-get --help 2>&1 | tee "$TEST_OUT_DIR/meta-get_help.txt"
   $MBTILES_BIN meta-get ./tests/fixtures/files/world_cities.mbtiles name 2>&1 | tee "$TEST_OUT_DIR/meta-get_name.txt"
   $MBTILES_BIN meta-get ./tests/fixtures/files/world_cities.mbtiles missing_value 2>&1 | tee "$TEST_OUT_DIR/meta-get_missing_value.txt"
+  $MBTILES_BIN validate ./tests/fixtures/files/zoomed_world_cities.mbtiles 2>&1 | tee "$TEST_OUT_DIR/validate-ok.txt"
+
+  set +e
+  $MBTILES_BIN validate ./tests/fixtures/files/bad_hash.mbtiles 2>&1 | tee "$TEST_OUT_DIR/validate-bad.txt"
+  if [[ $? -eq 0 ]]; then
+    echo "ERROR: validate with bad_hash should have failed"
+    exit 1
+  fi
+  set -e
+
+  cp ./tests/fixtures/files/bad_hash.mbtiles "$TEST_TEMP_DIR/fix_bad_hash.mbtiles"
+  $MBTILES_BIN validate --update-agg-tiles-hash "$TEST_TEMP_DIR/fix_bad_hash.mbtiles" 2>&1 | tee "$TEST_OUT_DIR/validate-fix.txt"
+  $MBTILES_BIN validate "$TEST_TEMP_DIR/fix_bad_hash.mbtiles" 2>&1 | tee "$TEST_OUT_DIR/validate-fix2.txt"
 
   # Create diff file
   $MBTILES_BIN copy \