Address review comments

martin-mbtiles/src/bin/main.rs

@@ -69,7 +69,7 @@ enum Commands {
         integrity_check: IntegrityCheck,
         /// Generate a hash of the tile data hashes and store under the 'global_hash' key in metadata
         #[arg(long)]
-        generate_global_hash: bool,
+        regenerate_global_hash: bool,
     },
 }
 
@@ -96,9 +96,9 @@ async fn main() -> Result<()> {
         Commands::Validate {
             file,
             integrity_check,
-            generate_global_hash,
+            regenerate_global_hash,
         } => {
-            validate_mbtiles(file.as_path(), integrity_check, generate_global_hash).await?;
+            validate_mbtiles(file.as_path(), integrity_check, regenerate_global_hash).await?;
         }
     }
 
@@ -126,13 +126,13 @@ async fn meta_set_value(file: &Path, key: &str, value: Option<String>) -> Result
 async fn validate_mbtiles(
     file: &Path,
     integrity_check: IntegrityCheck,
-    generate_global_hash: bool,
+    regenerate_global_hash: bool,
 ) -> Result<()> {
     let mbt = Mbtiles::new(file)?;
     let opt = SqliteConnectOptions::new().filename(file).read_only(true);
     let mut conn = SqliteConnection::connect_with(&opt).await?;
     mbt.validate_mbtiles(integrity_check, &mut conn).await?;
-    if generate_global_hash {
+    if regenerate_global_hash {
         mbt.generate_global_hash(&mut conn).await?;
     }
     Ok(())
@@ -427,7 +427,7 @@ mod tests {
                 command: Validate {
                     file: PathBuf::from("src_file"),
                     integrity_check: IntegrityCheck::Quick,
-                    generate_global_hash: false
+                    regenerate_global_hash: false
                 }
             }
         );

martin-mbtiles/src/errors.rs

@@ -20,12 +20,15 @@ pub enum MbtError {
     #[error("Invalid data format for MBTile file {0}")]
     InvalidDataFormat(String),
 
-    #[error("Integrity check failed for MBTile file {0}")]
-    FailedIntegrityCheck(String),
+    #[error("Integrity check failed for MBTile file {0} for the following reasons: \n {1:?}")]
+    FailedIntegrityCheck(String, Vec<String>),
 
     #[error("Invalid tile data for MBTile file {0}")]
     InvalidTileData(String),
 
+    #[error("Expected global_hash value in metadata table for MBTiles file {0}")]
+    GlobalHashValueNotFound(String),
+
     #[error(r#"Filename "{0}" passed to SQLite must be valid UTF-8"#)]
     InvalidFilenameType(PathBuf),
 

martin-mbtiles/src/mbtiles.rs

@@ -16,15 +16,15 @@ use martin_tile_utils::{Format, TileInfo};
 use serde_json::{Value as JSONValue, Value};
 use sqlite_hashes::register_md5_function;
 use sqlite_hashes::rusqlite::Connection as RusqliteConnection;
+use sqlx::sqlite::SqliteRow;
 use sqlx::{query, Row, SqliteExecutor};
 use tilejson::{tilejson, Bounds, Center, TileJSON};
 
 use crate::errors::{MbtError, MbtResult};
-use crate::mbtiles::MbtType::Flat;
 use crate::mbtiles_queries::{
     is_flat_tables_type, is_flat_with_hash_tables_type, is_normalized_tables_type,
 };
-use crate::MbtError::{FailedIntegrityCheck, InvalidTileData};
+use crate::MbtError::{FailedIntegrityCheck, GlobalHashValueNotFound, InvalidTileData};
 
 #[derive(Clone, Debug, PartialEq)]
 pub struct Metadata {
@@ -388,29 +388,38 @@ impl Mbtiles {
         Err(MbtError::NoUniquenessConstraint(self.filepath.clone()))
     }
 
-    pub async fn generate_global_hash<T>(&self, conn: &mut T) -> MbtResult<()>
+    async fn get_global_hash<T>(&self, conn: &mut T) -> MbtResult<Option<String>>
     where
         for<'e> &'e mut T: SqliteExecutor<'e>,
     {
+        let rusqlite_conn = RusqliteConnection::open(Path::new(&self.filepath()))?;
+        register_md5_function(&rusqlite_conn)?;
         let mbttype = self.detect_type(&mut *conn).await?;
 
-        let select_from = match mbttype {
-            Flat => {
+        let sql = match mbttype {
+            MbtType::Flat => {
                 println!("Cannot generate global hash, no hash column in flat table format. Skipping global_hash generation...");
-                return Ok(());
+                return Ok(None);
             }
-            MbtType::FlatWithHash => "SELECT 'global_hash', hex(md5_concat(tile_hash)) FROM tiles_with_hash ORDER BY zoom_level, tile_column, tile_row;",
-            MbtType::Normalized => "SELECT 'global_hash', hex(md5_concat(images.tile_id)) FROM images JOIN map ON images.tile_id=map.tile_id ORDER BY zoom_level, tile_column, tile_row;"
+            MbtType::FlatWithHash => "SELECT hex(md5_concat(cast(zoom_level AS text), cast(tile_column AS text), cast(tile_row AS text), tile_hash)) FROM tiles_with_hash ORDER BY zoom_level, tile_column, tile_row;",
+            MbtType::Normalized => "SELECT hex(md5_concat(cast(zoom_level AS text), cast(tile_column AS text), cast(tile_row AS text), tile_id)) FROM map ORDER BY zoom_level, tile_column, tile_row;"
+        };
 
-        }.to_string();
+        Ok(Some(rusqlite_conn.query_row_and_then(sql, [], |row| {
+            row.get::<_, String>(0)
+        })?))
+    }
 
-        let rusqlite_conn = RusqliteConnection::open(Path::new(&self.filepath()))?;
-        register_md5_function(&rusqlite_conn)?;
-        rusqlite_conn.execute(
-            format!("INSERT OR REPLACE INTO metadata(name, value) {select_from}").as_str(),
-            [],
-        )?;
-        Ok(())
+    pub async fn generate_global_hash<T>(&self, conn: &mut T) -> MbtResult<()>
+    where
+        for<'e> &'e mut T: SqliteExecutor<'e>,
+    {
+        if let Some(global_hash) = self.get_global_hash(&mut *conn).await? {
+            self.set_metadata_value(conn, "global_hash", Some(global_hash))
+                .await
+        } else {
+            Ok(())
+        }
     }
 
     pub async fn validate_mbtiles<T>(
@@ -422,20 +431,26 @@ impl Mbtiles {
         for<'e> &'e mut T: SqliteExecutor<'e>,
     {
         // SQLite Integrity check
-        if "ok"
-            != match integrity_check {
-                IntegrityCheck::Full => query("PRAGMA integrity_check;")
-                    .fetch_one(&mut *conn)
-                    .await?
-                    .get::<String, _>(0),
-                IntegrityCheck::Quick => query("PRAGMA integrity_check;")
-                    .fetch_one(&mut *conn)
-                    .await?
-                    .get::<String, _>(0),
-                IntegrityCheck::Off => "ok".to_string(),
+        if integrity_check != IntegrityCheck::Off {
+            let sql = if integrity_check == IntegrityCheck::Full {
+                "PRAGMA integrity_check;"
+            } else {
+                "PRAGMA quick_check;"
+            };
+
+            let result = query(sql)
+                .map(|row: SqliteRow| row.get::<String, _>(0))
+                .fetch_all(&mut *conn)
+                .await?;
+
+            if result.len() > 1
+                || result.get(0).ok_or(FailedIntegrityCheck(
+                    self.filepath().to_string(),
+                    vec!["SQLite could not perform integrity check".to_string()],
+                ))? != "ok"
+            {
+                return Err(FailedIntegrityCheck(self.filepath().to_string(), result));
             }
-        {
-            return Err(FailedIntegrityCheck(self.filepath().to_string()));
         }
 
         let mbttype = self.detect_type(&mut *conn).await?;
@@ -451,40 +466,24 @@ impl Mbtiles {
         register_md5_function(&rusqlite_conn)?;
 
         // Global hash check
-        if self
-            .get_metadata_value(&mut *conn, "global_hash")
-            .await?
-            .is_none()
-        {
-            println!(
-                "No value for 'global_hash' key found in metadata, skipping global hash validation step..."
-            );
-        } else {
-            let sql = if mbttype == MbtType::FlatWithHash {
-                "SELECT * FROM metadata 
-                          WHERE name='global_hash' 
-                                AND value!=(SELECT hex(md5_concat(tile_hash)) FROM tiles_with_hash ORDER BY zoom_level, tile_column, tile_row);"
-            } else {
-                "SELECT * FROM metadata 
-                          WHERE name='global_hash' 
-                                AND value!=(hex(md5_concat(images.tile_id)) FROM images JOIN map ON images.tile_id=map.tile_id ORDER BY zoom_level, tile_column, tile_row);"
-            }
-                .to_string();
-
-            if rusqlite_conn.prepare(&sql)?.exists(())? {
-                return Err(InvalidTileData(self.filepath().to_string()));
+        if let Some(global_hash) = self.get_metadata_value(&mut *conn, "global_hash").await? {
+            if let Some(new_global_hash) = self.get_global_hash(&mut *conn).await? {
+                if global_hash != new_global_hash {
+                    return Err(InvalidTileData(self.filepath().to_string()));
+                }
             }
+        } else {
+            return Err(GlobalHashValueNotFound(self.filepath().to_string()));
         }
 
         // Per-tile hash check
         let sql = if mbttype == MbtType::FlatWithHash {
-            "SELECT * FROM tiles_with_hash WHERE tile_hash!=hex(md5(tile_data)) LIMIT 1;"
+            "SELECT 1 FROM tiles_with_hash WHERE tile_hash != hex(md5(tile_data)) LIMIT 1;"
         } else {
-            "SELECT * FROM images WHERE tile_id!=hex(md5(tile_data)) LIMIT 1;"
-        }
-        .to_string();
+            "SELECT 1 FROM images WHERE tile_id != hex(md5(tile_data)) LIMIT 1;"
+        };
 
-        if rusqlite_conn.prepare(&sql)?.exists(())? {
+        if rusqlite_conn.prepare(sql)?.exists(())? {
             return Err(InvalidTileData(self.filepath().to_string()));
         }
 
@@ -659,20 +658,15 @@ mod tests {
         let (mut conn, mbt) =
             open("../tests/fixtures/files/invalid_zoomed_world_cities.mbtiles").await;
 
+        print!(
+            "VLAUE {:?}",
+            mbt.validate_mbtiles(IntegrityCheck::Quick, &mut conn).await
+        );
         assert!(matches!(
             mbt.validate_mbtiles(IntegrityCheck::Quick, &mut conn)
                 .await
                 .unwrap_err(),
             MbtError::InvalidTileData(..)
         ));
     }
-
-    #[actix_rt::test]
-    async fn validate_file_with_global_hash() {
-        let (mut conn, mbt) = open("../tests/fixtures/files/zoomed_world_cities.mbtiles").await;
-
-        mbt.validate_mbtiles(IntegrityCheck::Quick, &mut conn)
-            .await
-            .unwrap();
-    }
 }

martin-mbtiles/src/tile_copier.rs

@@ -31,7 +31,7 @@ pub struct TileCopierOptions {
     src_file: PathBuf,
     /// MBTiles file to write to
     dst_file: PathBuf,
-    /// Output format of the destination file, ignored if the file exists. if not specified, defaults to the type of source
+    /// Output format of the destination file, ignored if the file exists. If not specified, defaults to the type of source
     #[cfg_attr(feature = "cli", arg(long, value_enum))]
     dst_mbttype: Option<MbtType>,
     /// Specify copying behaviour when tiles with duplicate (zoom_level, tile_column, tile_row) values are found
@@ -49,6 +49,9 @@ pub struct TileCopierOptions {
     /// Compare source file with this file, and only copy non-identical tiles to destination
     #[cfg_attr(feature = "cli", arg(long))]
     diff_with_file: Option<PathBuf>,
+    /// Skip generating a global hash for mbtiles validation. By default, if dst_mbttype is flat-with-hash or normalized, generate a global hash and store in the metadata table
+    #[cfg_attr(feature = "cli", arg(long))]
+    skip_global_hash: bool,
 }
 
 #[cfg(feature = "cli")]
@@ -101,6 +104,7 @@ impl TileCopierOptions {
             min_zoom: None,
             max_zoom: None,
             diff_with_file: None,
+            skip_global_hash: false,
         }
     }
 
@@ -133,6 +137,11 @@ impl TileCopierOptions {
         self.diff_with_file = Some(diff_with_file);
         self
     }
+
+    pub fn skip_global_hash(mut self, skip_global_hash: bool) -> Self {
+        self.skip_global_hash = skip_global_hash;
+        self
+    }
 }
 
 impl TileCopier {
@@ -226,6 +235,12 @@ impl TileCopier {
             }
         };
 
+        if !self.options.skip_global_hash
+            && (dst_mbttype == FlatWithHash || dst_mbttype == Normalized)
+        {
+            self.dst_mbtiles.generate_global_hash(&mut conn).await?;
+        }
+
         Ok(conn)
     }