From f0868eda12a9222bd603c934ee4c008f1afdab03 Mon Sep 17 00:00:00 2001 From: David Disch Date: Sat, 15 Feb 2025 13:00:04 +1100 Subject: [PATCH 01/19] Start implementing basic API changes --- martin/src/file_config.rs | 4 ++ martin/src/mbtiles/mod.rs | 97 +++++++++++++++++++++++++++++++++------ mbtiles/src/pool.rs | 19 +++++++- mbtiles/src/validation.rs | 6 +-- 4 files changed, 109 insertions(+), 17 deletions(-) diff --git a/martin/src/file_config.rs b/martin/src/file_config.rs index a489d7a2d..7c12386bd 100644 --- a/martin/src/file_config.rs +++ b/martin/src/file_config.rs @@ -50,6 +50,10 @@ pub enum FileError { #[error(r"PMTiles error {0} processing {1}")] PmtError(pmtiles::PmtError, String), + #[cfg(feature = "mbtiles")] + #[error(r"MBTiles error {0} processing {1}")] + MbtError(mbtiles::MbtError, String), + #[cfg(feature = "cog")] #[error(transparent)] CogError(#[from] crate::cog::CogError), diff --git a/martin/src/mbtiles/mod.rs b/martin/src/mbtiles/mod.rs index affb342a0..1b52ef577 100644 --- a/martin/src/mbtiles/mod.rs +++ b/martin/src/mbtiles/mod.rs @@ -4,23 +4,55 @@ use std::path::PathBuf; use std::sync::Arc; use async_trait::async_trait; +use clap::ValueEnum; use log::trace; use martin_tile_utils::{TileCoord, TileInfo}; -use mbtiles::MbtilesPool; +use mbtiles::{IntegrityCheckType, MbtilesPool}; use serde::{Deserialize, Serialize}; use tilejson::TileJSON; use url::Url; use crate::config::UnrecognizedValues; -use crate::file_config::FileError::{AcquireConnError, InvalidMetadata, IoError}; +use crate::file_config::FileError::{self, AcquireConnError, InvalidMetadata, IoError}; use crate::file_config::{ConfigExtras, FileResult, SourceConfigExtras}; use crate::source::{TileData, TileInfoSource, UrlQuery}; use crate::{MartinResult, Source}; +// #[derive(PartialEq, Eq, Debug, Clone, Copy, Default, Serialize, Deserialize, ValueEnum)] +// #[serde(rename_all = "lowercase")] +// pub enum Validate { +// /// Do not validate +// Skip, + +// /// Quickly check the file +// #[default] +// Fast, + +// /// Do a slow check of everything +// Thorough, +// } + +#[derive(PartialEq, Eq, Debug, Clone, Copy, Default, Serialize, Deserialize, ValueEnum)] +#[serde(rename_all = "lowercase")] +pub enum OnInvalid { + /// Print warning message, and abort if the error is critical + #[default] + Warn, + + /// Skip this source + IgnoreSource, + + /// Do not start Martin on any warnings + Abort, +} + #[derive(Clone, Debug, Default, PartialEq, Serialize, Deserialize)] pub struct MbtConfig { #[serde(flatten)] pub unrecognized: UnrecognizedValues, + + pub validate: IntegrityCheckType, + pub on_invalid: OnInvalid, } impl ConfigExtras for MbtConfig { @@ -31,7 +63,18 @@ impl ConfigExtras for MbtConfig { impl SourceConfigExtras for MbtConfig { async fn new_sources(&self, id: String, path: PathBuf) -> FileResult { - Ok(Box::new(MbtSource::new(id, path).await?)) + let source = MbtSource::new(id, path, self.validate) + .await + .map_err(|e| { + match e { + FileError::MbtError => { + + }, + _ => e + } + })?; + + Ok(Box::new(source)) } // TODO: Remove #[allow] after switching to Rust/Clippy v1.78+ in CI @@ -62,7 +105,11 @@ impl Debug for MbtSource { } impl MbtSource { - async fn new(id: String, path: PathBuf) -> FileResult { + async fn new( + id: String, + path: PathBuf, + validate: IntegrityCheckType, + ) -> FileResult { let mbt = MbtilesPool::new(&path) .await .map_err(|e| io::Error::other(format!("{e:?}: Cannot open file {}", path.display()))) @@ -73,12 +120,30 @@ impl MbtSource { .await .map_err(|e| InvalidMetadata(e.to_string(), path))?; - Ok(Self { - id, - mbtiles: Arc::new(mbt), - tilejson: meta.tilejson, - tile_info: meta.tile_info, - }) + mbt + .validate(validate) + .await + .map_err(|e| FileError::MbtError(e, String::new())) + .map(|_| { + Self { + id, + mbtiles: Arc::new(mbt), + tilejson: meta.tilejson, + tile_info: meta.tile_info, + } + }) + // match validate { + // Validate::Thorough => mbt + // .validate(mbtiles::IntegrityCheckType::Full) + // .await + // .map_err(|e| FileError::MbtError(e, String::new())), + // Validate::Fast => mbt + // .validate(mbtiles::IntegrityCheckType::Quick) + // .await + + // Validate::Skip => Ok(()), + // } + // .map(|_| ) } } @@ -128,13 +193,15 @@ mod tests { use std::path::PathBuf; use indoc::indoc; + use mbtiles::IntegrityCheckType; use crate::file_config::{FileConfigEnum, FileConfigSource, FileConfigSrc}; - use crate::mbtiles::MbtConfig; + use crate::mbtiles::{MbtConfig, OnInvalid}; #[test] fn parse() { - let cfg = serde_yaml::from_str::>(indoc! {" + let cfg: FileConfigEnum = + serde_yaml::from_str::>(indoc! {" paths: - /dir-path - /path/to/file2.ext @@ -146,8 +213,10 @@ mod tests { pm-src3: https://example.org/file3.ext pm-src4: path: https://example.org/file4.ext + validate: quick + on_invalid: abort "}) - .unwrap(); + .unwrap(); let res = cfg.finalize(""); assert!(res.is_empty(), "unrecognized config: {res:?}"); let FileConfigEnum::Config(cfg) = cfg else { @@ -187,5 +256,7 @@ mod tests { ), ])) ); + assert_eq!(cfg.custom.validate, IntegrityCheckType::Quick); + assert_eq!(cfg.custom.on_invalid, OnInvalid::Abort); } } diff --git a/mbtiles/src/pool.rs b/mbtiles/src/pool.rs index 02fac693d..1ab236b14 100644 --- a/mbtiles/src/pool.rs +++ b/mbtiles/src/pool.rs @@ -3,7 +3,7 @@ use std::path::Path; use sqlx::{Pool, Sqlite, SqlitePool}; use crate::errors::MbtResult; -use crate::{Mbtiles, Metadata}; +use crate::{IntegrityCheckType, Mbtiles, Metadata}; #[derive(Clone, Debug)] pub struct MbtilesPool { @@ -27,4 +27,21 @@ impl MbtilesPool { let mut conn = self.pool.acquire().await?; self.mbtiles.get_tile(&mut *conn, z, x, y).await } + + pub async fn validate(&self, check_type: IntegrityCheckType) -> MbtResult<()> { + let mut conn = self.pool.acquire().await?; + match check_type { + IntegrityCheckType::Full => { + self.mbtiles.detect_type(&mut *conn).await?; + self.mbtiles.check_integrity(&mut *conn, check_type).await?; + self.mbtiles.check_tiles_type_validity(&mut *conn).await?; + self.mbtiles.check_each_tile_hash(&mut *conn).await?; + } + IntegrityCheckType::Quick => { + self.mbtiles.detect_type(&mut *conn).await?; + } + IntegrityCheckType::Off => {} + } + Ok(()) + } } diff --git a/mbtiles/src/validation.rs b/mbtiles/src/validation.rs index c17e87d79..b0104fbbf 100644 --- a/mbtiles/src/validation.rs +++ b/mbtiles/src/validation.rs @@ -3,8 +3,8 @@ use std::str::from_utf8; use enum_display::EnumDisplay; use log::{debug, info, warn}; -use martin_tile_utils::{Format, MAX_ZOOM, TileInfo}; -use serde::Serialize; +use martin_tile_utils::{Format, TileInfo, MAX_ZOOM}; +use serde::{Deserialize, Serialize}; use serde_json::Value; use sqlx::sqlite::SqliteRow; use sqlx::{Row, SqliteConnection, SqliteExecutor, query}; @@ -51,7 +51,7 @@ impl MbtType { } } -#[derive(Default, Debug, Clone, Copy, PartialEq, Eq, EnumDisplay)] +#[derive(Default, Debug, Clone, Copy, PartialEq, Eq, EnumDisplay, Serialize, Deserialize)] #[enum_display(case = "Kebab")] #[cfg_attr(feature = "cli", derive(clap::ValueEnum))] pub enum IntegrityCheckType { From 5106dfe500ec29e8c98654c3a8c6cd48bdee8d19 Mon Sep 17 00:00:00 2001 From: David Disch Date: Sat, 15 Feb 2025 14:29:53 +1100 Subject: [PATCH 02/19] Refactor API --- martin/src/file_config.rs | 17 +++++--- martin/src/mbtiles/mod.rs | 86 ++++++++++++++------------------------- mbtiles/src/lib.rs | 2 +- mbtiles/src/mbtiles.rs | 16 ++++++++ mbtiles/src/pool.rs | 13 +++--- 5 files changed, 66 insertions(+), 68 deletions(-) diff --git a/martin/src/file_config.rs b/martin/src/file_config.rs index 7c12386bd..09ac62b50 100644 --- a/martin/src/file_config.rs +++ b/martin/src/file_config.rs @@ -46,14 +46,16 @@ pub enum FileError { #[error(r"Unable to acquire connection to file: {0}")] AcquireConnError(String), + #[error("Source {0} caused an abort due to validation error {1}")] + AbortOnInvalid(PathBuf, String), + + #[error("Source {0} was ignored due to validation error {1}")] + IgnoreOnInvalid(PathBuf, String), + #[cfg(feature = "pmtiles")] #[error(r"PMTiles error {0} processing {1}")] PmtError(pmtiles::PmtError, String), - #[cfg(feature = "mbtiles")] - #[error(r"MBTiles error {0} processing {1}")] - MbtError(mbtiles::MbtError, String), - #[cfg(feature = "cog")] #[error(transparent)] CogError(#[from] crate::cog::CogError), @@ -286,7 +288,12 @@ async fn resolve_int( let id = idr.resolve(&id, can.to_string_lossy().to_string()); info!("Configured {dup}source {id} from {}", can.display()); configs.insert(id.clone(), source.clone()); - results.push(cfg.custom.new_sources(id, source.into_path()).await?); + let src_result = cfg.custom.new_sources(id, source.into_path()).await; + match src_result { + Err(FileError::IgnoreOnInvalid(_, _)) => {}, + Err(e) => return Err(e), + Ok(src) => results.push(src), + }; } } } diff --git a/martin/src/mbtiles/mod.rs b/martin/src/mbtiles/mod.rs index 1b52ef577..a41bda04a 100644 --- a/martin/src/mbtiles/mod.rs +++ b/martin/src/mbtiles/mod.rs @@ -5,9 +5,9 @@ use std::sync::Arc; use async_trait::async_trait; use clap::ValueEnum; -use log::trace; +use log::{trace, warn}; use martin_tile_utils::{TileCoord, TileInfo}; -use mbtiles::{IntegrityCheckType, MbtilesPool}; +use mbtiles::{MbtResult, MbtilesPool, ValidationLevel}; use serde::{Deserialize, Serialize}; use tilejson::TileJSON; use url::Url; @@ -18,20 +18,6 @@ use crate::file_config::{ConfigExtras, FileResult, SourceConfigExtras}; use crate::source::{TileData, TileInfoSource, UrlQuery}; use crate::{MartinResult, Source}; -// #[derive(PartialEq, Eq, Debug, Clone, Copy, Default, Serialize, Deserialize, ValueEnum)] -// #[serde(rename_all = "lowercase")] -// pub enum Validate { -// /// Do not validate -// Skip, - -// /// Quickly check the file -// #[default] -// Fast, - -// /// Do a slow check of everything -// Thorough, -// } - #[derive(PartialEq, Eq, Debug, Clone, Copy, Default, Serialize, Deserialize, ValueEnum)] #[serde(rename_all = "lowercase")] pub enum OnInvalid { @@ -51,7 +37,7 @@ pub struct MbtConfig { #[serde(flatten)] pub unrecognized: UnrecognizedValues, - pub validate: IntegrityCheckType, + pub validate: ValidationLevel, pub on_invalid: OnInvalid, } @@ -63,17 +49,20 @@ impl ConfigExtras for MbtConfig { impl SourceConfigExtras for MbtConfig { async fn new_sources(&self, id: String, path: PathBuf) -> FileResult { - let source = MbtSource::new(id, path, self.validate) - .await - .map_err(|e| { - match e { - FileError::MbtError => { - - }, - _ => e + let source = MbtSource::new(id, path.clone()).await?; + if let Err(validation_error) = source.validate(self.validate).await { + match self.on_invalid { + OnInvalid::Abort => { + return Err(FileError::AbortOnInvalid(path, validation_error.to_string())); } - })?; - + OnInvalid::IgnoreSource => { + return Err(FileError::IgnoreOnInvalid(path, validation_error.to_string())); + } + OnInvalid::Warn => { + warn!("MBTile file {} failed validity check {:?}", path.display(), validation_error); + } + } + } Ok(Box::new(source)) } @@ -107,8 +96,7 @@ impl Debug for MbtSource { impl MbtSource { async fn new( id: String, - path: PathBuf, - validate: IntegrityCheckType, + path: PathBuf ) -> FileResult { let mbt = MbtilesPool::new(&path) .await @@ -120,30 +108,16 @@ impl MbtSource { .await .map_err(|e| InvalidMetadata(e.to_string(), path))?; - mbt - .validate(validate) - .await - .map_err(|e| FileError::MbtError(e, String::new())) - .map(|_| { - Self { - id, - mbtiles: Arc::new(mbt), - tilejson: meta.tilejson, - tile_info: meta.tile_info, - } - }) - // match validate { - // Validate::Thorough => mbt - // .validate(mbtiles::IntegrityCheckType::Full) - // .await - // .map_err(|e| FileError::MbtError(e, String::new())), - // Validate::Fast => mbt - // .validate(mbtiles::IntegrityCheckType::Quick) - // .await - - // Validate::Skip => Ok(()), - // } - // .map(|_| ) + Ok(Self { + id, + mbtiles: Arc::new(mbt), + tilejson: meta.tilejson, + tile_info: meta.tile_info, + }) + } + + async fn validate(&self, validation_level: ValidationLevel) -> MbtResult<()> { + self.mbtiles.validate(validation_level).await } } @@ -193,7 +167,7 @@ mod tests { use std::path::PathBuf; use indoc::indoc; - use mbtiles::IntegrityCheckType; + use mbtiles::{IntegrityCheckType, ValidationLevel}; use crate::file_config::{FileConfigEnum, FileConfigSource, FileConfigSrc}; use crate::mbtiles::{MbtConfig, OnInvalid}; @@ -213,7 +187,7 @@ mod tests { pm-src3: https://example.org/file3.ext pm-src4: path: https://example.org/file4.ext - validate: quick + validate: thorough on_invalid: abort "}) .unwrap(); @@ -256,7 +230,7 @@ mod tests { ), ])) ); - assert_eq!(cfg.custom.validate, IntegrityCheckType::Quick); + assert_eq!(cfg.custom.validate, ValidationLevel::Thorough); assert_eq!(cfg.custom.on_invalid, OnInvalid::Abort); } } diff --git a/mbtiles/src/lib.rs b/mbtiles/src/lib.rs index 3a60ed625..dac8da7c7 100644 --- a/mbtiles/src/lib.rs +++ b/mbtiles/src/lib.rs @@ -11,7 +11,7 @@ mod errors; pub use errors::{MbtError, MbtResult}; mod mbtiles; -pub use mbtiles::{CopyType, MbtTypeCli, Mbtiles}; +pub use mbtiles::{CopyType, MbtTypeCli, Mbtiles, ValidationLevel}; mod metadata; pub use metadata::Metadata; diff --git a/mbtiles/src/mbtiles.rs b/mbtiles/src/mbtiles.rs index 2376ffe71..ac23ab339 100644 --- a/mbtiles/src/mbtiles.rs +++ b/mbtiles/src/mbtiles.rs @@ -23,6 +23,22 @@ pub enum MbtTypeCli { Normalized, } +#[derive(PartialEq, Eq, Debug, Clone, Copy, Default, Serialize, Deserialize)] +#[serde(rename_all = "lowercase")] +#[cfg_attr(feature = "cli", derive(clap::ValueEnum))] +pub enum ValidationLevel { + /// Do not validate + Skip, + + /// Quickly check the file + #[default] + Fast, + + /// Do a slow check of everything + Thorough, +} + + #[derive(Default, Debug, Clone, Copy, Hash, PartialEq, Eq, Serialize, Deserialize, EnumDisplay)] #[enum_display(case = "Kebab")] #[cfg_attr(feature = "cli", derive(clap::ValueEnum))] diff --git a/mbtiles/src/pool.rs b/mbtiles/src/pool.rs index 1ab236b14..c6bc3919f 100644 --- a/mbtiles/src/pool.rs +++ b/mbtiles/src/pool.rs @@ -3,6 +3,7 @@ use std::path::Path; use sqlx::{Pool, Sqlite, SqlitePool}; use crate::errors::MbtResult; +use crate::mbtiles::ValidationLevel; use crate::{IntegrityCheckType, Mbtiles, Metadata}; #[derive(Clone, Debug)] @@ -28,19 +29,19 @@ impl MbtilesPool { self.mbtiles.get_tile(&mut *conn, z, x, y).await } - pub async fn validate(&self, check_type: IntegrityCheckType) -> MbtResult<()> { + pub async fn validate(&self, validation_level: ValidationLevel) -> MbtResult<()> { let mut conn = self.pool.acquire().await?; - match check_type { - IntegrityCheckType::Full => { + match validation_level { + ValidationLevel::Thorough => { self.mbtiles.detect_type(&mut *conn).await?; - self.mbtiles.check_integrity(&mut *conn, check_type).await?; + self.mbtiles.check_integrity(&mut *conn, IntegrityCheckType::Full).await?; self.mbtiles.check_tiles_type_validity(&mut *conn).await?; self.mbtiles.check_each_tile_hash(&mut *conn).await?; } - IntegrityCheckType::Quick => { + ValidationLevel::Fast => { self.mbtiles.detect_type(&mut *conn).await?; } - IntegrityCheckType::Off => {} + ValidationLevel::Skip => {} } Ok(()) } From 6c10b573e20100d91bbdb275526152e1c2ad49a5 Mon Sep 17 00:00:00 2001 From: David Disch Date: Sat, 15 Feb 2025 14:32:18 +1100 Subject: [PATCH 03/19] Undo changes to mbtiles/src/validation.rs --- mbtiles/src/validation.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mbtiles/src/validation.rs b/mbtiles/src/validation.rs index b0104fbbf..7077ef84e 100644 --- a/mbtiles/src/validation.rs +++ b/mbtiles/src/validation.rs @@ -4,7 +4,7 @@ use std::str::from_utf8; use enum_display::EnumDisplay; use log::{debug, info, warn}; use martin_tile_utils::{Format, TileInfo, MAX_ZOOM}; -use serde::{Deserialize, Serialize}; +use serde::Serialize; use serde_json::Value; use sqlx::sqlite::SqliteRow; use sqlx::{Row, SqliteConnection, SqliteExecutor, query}; @@ -51,7 +51,7 @@ impl MbtType { } } -#[derive(Default, Debug, Clone, Copy, PartialEq, Eq, EnumDisplay, Serialize, Deserialize)] +#[derive(Default, Debug, Clone, Copy, PartialEq, Eq, EnumDisplay)] #[enum_display(case = "Kebab")] #[cfg_attr(feature = "cli", derive(clap::ValueEnum))] pub enum IntegrityCheckType { From 02c7c48f66972348c5b654745d3d6f429b757dff Mon Sep 17 00:00:00 2001 From: David Disch Date: Sat, 15 Feb 2025 14:33:54 +1100 Subject: [PATCH 04/19] fmt --- martin/src/file_config.rs | 2 +- martin/src/mbtiles/mod.rs | 23 +++++++++++++++-------- mbtiles/src/mbtiles.rs | 1 - mbtiles/src/pool.rs | 4 +++- 4 files changed, 19 insertions(+), 11 deletions(-) diff --git a/martin/src/file_config.rs b/martin/src/file_config.rs index 09ac62b50..3f08d1f9d 100644 --- a/martin/src/file_config.rs +++ b/martin/src/file_config.rs @@ -290,7 +290,7 @@ async fn resolve_int( configs.insert(id.clone(), source.clone()); let src_result = cfg.custom.new_sources(id, source.into_path()).await; match src_result { - Err(FileError::IgnoreOnInvalid(_, _)) => {}, + Err(FileError::IgnoreOnInvalid(_, _)) => {} Err(e) => return Err(e), Ok(src) => results.push(src), }; diff --git a/martin/src/mbtiles/mod.rs b/martin/src/mbtiles/mod.rs index a41bda04a..75b22f2aa 100644 --- a/martin/src/mbtiles/mod.rs +++ b/martin/src/mbtiles/mod.rs @@ -53,13 +53,23 @@ impl SourceConfigExtras for MbtConfig { if let Err(validation_error) = source.validate(self.validate).await { match self.on_invalid { OnInvalid::Abort => { - return Err(FileError::AbortOnInvalid(path, validation_error.to_string())); + return Err(FileError::AbortOnInvalid( + path, + validation_error.to_string(), + )); } OnInvalid::IgnoreSource => { - return Err(FileError::IgnoreOnInvalid(path, validation_error.to_string())); + return Err(FileError::IgnoreOnInvalid( + path, + validation_error.to_string(), + )); } OnInvalid::Warn => { - warn!("MBTile file {} failed validity check {:?}", path.display(), validation_error); + warn!( + "MBTile file {} failed validity check {:?}", + path.display(), + validation_error + ); } } } @@ -94,10 +104,7 @@ impl Debug for MbtSource { } impl MbtSource { - async fn new( - id: String, - path: PathBuf - ) -> FileResult { + async fn new(id: String, path: PathBuf) -> FileResult { let mbt = MbtilesPool::new(&path) .await .map_err(|e| io::Error::other(format!("{e:?}: Cannot open file {}", path.display()))) @@ -115,7 +122,7 @@ impl MbtSource { tile_info: meta.tile_info, }) } - + async fn validate(&self, validation_level: ValidationLevel) -> MbtResult<()> { self.mbtiles.validate(validation_level).await } diff --git a/mbtiles/src/mbtiles.rs b/mbtiles/src/mbtiles.rs index ac23ab339..ff871ca94 100644 --- a/mbtiles/src/mbtiles.rs +++ b/mbtiles/src/mbtiles.rs @@ -38,7 +38,6 @@ pub enum ValidationLevel { Thorough, } - #[derive(Default, Debug, Clone, Copy, Hash, PartialEq, Eq, Serialize, Deserialize, EnumDisplay)] #[enum_display(case = "Kebab")] #[cfg_attr(feature = "cli", derive(clap::ValueEnum))] diff --git a/mbtiles/src/pool.rs b/mbtiles/src/pool.rs index c6bc3919f..d1df7e4f7 100644 --- a/mbtiles/src/pool.rs +++ b/mbtiles/src/pool.rs @@ -34,7 +34,9 @@ impl MbtilesPool { match validation_level { ValidationLevel::Thorough => { self.mbtiles.detect_type(&mut *conn).await?; - self.mbtiles.check_integrity(&mut *conn, IntegrityCheckType::Full).await?; + self.mbtiles + .check_integrity(&mut *conn, IntegrityCheckType::Full) + .await?; self.mbtiles.check_tiles_type_validity(&mut *conn).await?; self.mbtiles.check_each_tile_hash(&mut *conn).await?; } From 0723494715a15c65f2652578a365d7651ec24f06 Mon Sep 17 00:00:00 2001 From: David Disch Date: Sat, 15 Feb 2025 14:42:36 +1100 Subject: [PATCH 05/19] clippy --- martin/src/mbtiles/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/martin/src/mbtiles/mod.rs b/martin/src/mbtiles/mod.rs index 75b22f2aa..8e27d66cf 100644 --- a/martin/src/mbtiles/mod.rs +++ b/martin/src/mbtiles/mod.rs @@ -174,7 +174,7 @@ mod tests { use std::path::PathBuf; use indoc::indoc; - use mbtiles::{IntegrityCheckType, ValidationLevel}; + use mbtiles::ValidationLevel; use crate::file_config::{FileConfigEnum, FileConfigSource, FileConfigSrc}; use crate::mbtiles::{MbtConfig, OnInvalid}; From 35ff080d88b8e677ca6825c52a06eb5950e030ca Mon Sep 17 00:00:00 2001 From: David Disch Date: Sat, 15 Feb 2025 14:53:47 +1100 Subject: [PATCH 06/19] Make errors better, add defaults to config --- martin/src/mbtiles/mod.rs | 7 +++++-- mbtiles/src/errors.rs | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/martin/src/mbtiles/mod.rs b/martin/src/mbtiles/mod.rs index 8e27d66cf..1b88b3632 100644 --- a/martin/src/mbtiles/mod.rs +++ b/martin/src/mbtiles/mod.rs @@ -37,7 +37,10 @@ pub struct MbtConfig { #[serde(flatten)] pub unrecognized: UnrecognizedValues, + #[serde(default)] pub validate: ValidationLevel, + + #[serde(default)] pub on_invalid: OnInvalid, } @@ -66,9 +69,9 @@ impl SourceConfigExtras for MbtConfig { } OnInvalid::Warn => { warn!( - "MBTile file {} failed validity check {:?}", + "Source {} failed validation, this may cause performance issues: {}", path.display(), - validation_error + validation_error.to_string() ); } } diff --git a/mbtiles/src/errors.rs b/mbtiles/src/errors.rs index f4ba76fad..c3651fe41 100644 --- a/mbtiles/src/errors.rs +++ b/mbtiles/src/errors.rs @@ -65,7 +65,7 @@ pub enum MbtError { )] NonEmptyTargetFile(PathBuf), - #[error("The file {0} does not have the required uniqueness constraint")] + #[error("The file {0} does not have the required uniqueness constraint. Try adding a unique index on the `map` or `tiles` table.")] NoUniquenessConstraint(String), #[error("Could not copy MBTiles file: {reason}")] From 5b815da261a933a1afa8055f0d329e7e0aa0256b Mon Sep 17 00:00:00 2001 From: David Disch Date: Sat, 15 Feb 2025 14:57:41 +1100 Subject: [PATCH 07/19] Reuse validate function from mbtiles for thorough check --- mbtiles/src/pool.rs | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/mbtiles/src/pool.rs b/mbtiles/src/pool.rs index d1df7e4f7..55d7c760b 100644 --- a/mbtiles/src/pool.rs +++ b/mbtiles/src/pool.rs @@ -4,7 +4,7 @@ use sqlx::{Pool, Sqlite, SqlitePool}; use crate::errors::MbtResult; use crate::mbtiles::ValidationLevel; -use crate::{IntegrityCheckType, Mbtiles, Metadata}; +use crate::{AggHashType, IntegrityCheckType, Mbtiles, Metadata}; #[derive(Clone, Debug)] pub struct MbtilesPool { @@ -33,12 +33,7 @@ impl MbtilesPool { let mut conn = self.pool.acquire().await?; match validation_level { ValidationLevel::Thorough => { - self.mbtiles.detect_type(&mut *conn).await?; - self.mbtiles - .check_integrity(&mut *conn, IntegrityCheckType::Full) - .await?; - self.mbtiles.check_tiles_type_validity(&mut *conn).await?; - self.mbtiles.check_each_tile_hash(&mut *conn).await?; + self.mbtiles.validate(&mut *conn, IntegrityCheckType::Full, AggHashType::Verify).await?; } ValidationLevel::Fast => { self.mbtiles.detect_type(&mut *conn).await?; From e8cb9c802c29ee622cc7731eb0e77ff0d0555eb2 Mon Sep 17 00:00:00 2001 From: David Disch Date: Sat, 15 Feb 2025 15:01:34 +1100 Subject: [PATCH 08/19] fmt and clippy --- mbtiles/src/pool.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/mbtiles/src/pool.rs b/mbtiles/src/pool.rs index 55d7c760b..2d74f6c4d 100644 --- a/mbtiles/src/pool.rs +++ b/mbtiles/src/pool.rs @@ -33,7 +33,9 @@ impl MbtilesPool { let mut conn = self.pool.acquire().await?; match validation_level { ValidationLevel::Thorough => { - self.mbtiles.validate(&mut *conn, IntegrityCheckType::Full, AggHashType::Verify).await?; + self.mbtiles + .validate(&mut *conn, IntegrityCheckType::Full, AggHashType::Verify) + .await?; } ValidationLevel::Fast => { self.mbtiles.detect_type(&mut *conn).await?; From 00e8dbd09cd2cc47535d9c3f125de4eda4b4e711 Mon Sep 17 00:00:00 2001 From: David Disch Date: Sat, 15 Feb 2025 16:25:48 +1100 Subject: [PATCH 09/19] Add CLI parameters --- martin/src/args/mbtiles.rs | 36 ++++++++++++++++++++++++++++++++++++ martin/src/args/mod.rs | 7 ++++++- martin/src/args/root.rs | 13 +++++++++++-- martin/src/bin/martin-cp.rs | 5 +++++ martin/src/file_config.rs | 7 ++++++- martin/src/mbtiles/mod.rs | 9 ++++++--- mbtiles/src/mbtiles.rs | 2 +- 7 files changed, 71 insertions(+), 8 deletions(-) create mode 100644 martin/src/args/mbtiles.rs diff --git a/martin/src/args/mbtiles.rs b/martin/src/args/mbtiles.rs new file mode 100644 index 000000000..8a564ccfe --- /dev/null +++ b/martin/src/args/mbtiles.rs @@ -0,0 +1,36 @@ +use log::info; +use mbtiles::ValidationLevel; + +use crate::mbtiles::{MbtConfig, OnInvalid}; + +#[derive(clap::Args, Debug, PartialEq, Default)] +#[command(about, version)] +pub struct MbtArgs { + /// Level of validation to apply to .mbtiles + #[arg(long)] + pub validate_mbtiles: Option, + /// How to handle invalid .mbtiles + #[arg(long)] + pub on_invalid_mbtiles: Option, +} + +impl MbtArgs { + /// Apply CLI parameters from `self` to the configuration loaded from the config file `mbtiles` + pub fn override_config(self, mbt_config: &mut MbtConfig) { + // This ensures that if a new parameter is added to the struct, it will not be forgotten here + let Self { + validate_mbtiles, + on_invalid_mbtiles, + } = self; + + if let Some(value) = validate_mbtiles { + info!("Overriding configured default mbtiles.validate to {value}"); + mbt_config.validate = validate_mbtiles.unwrap_or_default(); + } + + if let Some(value) = on_invalid_mbtiles { + info!("Overriding configured default mbtiles.on_invalid to {value}"); + mbt_config.on_invalid = on_invalid_mbtiles.unwrap_or_default(); + } + } +} diff --git a/martin/src/args/mod.rs b/martin/src/args/mod.rs index b0ff3fb4f..0c1bfe929 100644 --- a/martin/src/args/mod.rs +++ b/martin/src/args/mod.rs @@ -7,7 +7,12 @@ pub use environment::{Env, OsEnv}; #[cfg(feature = "postgres")] mod pg; #[cfg(feature = "postgres")] -pub use pg::{BoundsCalcType, DEFAULT_BOUNDS_TIMEOUT, PgArgs}; +pub use pg::{BoundsCalcType, PgArgs, DEFAULT_BOUNDS_TIMEOUT}; + +#[cfg(feature = "mbtiles")] +mod mbtiles; +#[cfg(feature = "mbtiles")] +pub use mbtiles::MbtArgs; mod root; pub use root::{Args, ExtraArgs, MetaArgs}; diff --git a/martin/src/args/root.rs b/martin/src/args/root.rs index 1cac7a862..bd1c31c5d 100644 --- a/martin/src/args/root.rs +++ b/martin/src/args/root.rs @@ -35,6 +35,9 @@ pub struct Args { #[cfg(feature = "postgres")] #[command(flatten)] pub pg: Option, + #[cfg(feature = "mbtiles")] + #[command(flatten)] + pub mbtiles: Option, } // None of these params will be transferred to the config @@ -116,8 +119,14 @@ impl Args { } #[cfg(feature = "mbtiles")] - if !cli_strings.is_empty() { - config.mbtiles = parse_file_args(&mut cli_strings, &["mbtiles"], false); + { + if !cli_strings.is_empty() { + config.mbtiles = parse_file_args(&mut cli_strings, &["mbtiles"], false); + } + let mbt_args = self.mbtiles.unwrap_or_default(); + if let FileConfigEnum::Config(c) = &mut config.mbtiles { + mbt_args.override_config(&mut c.custom); + } } #[cfg(feature = "cog")] diff --git a/martin/src/bin/martin-cp.rs b/martin/src/bin/martin-cp.rs index e0cd807cd..3be849546 100644 --- a/martin/src/bin/martin-cp.rs +++ b/martin/src/bin/martin-cp.rs @@ -49,6 +49,9 @@ pub struct CopierArgs { #[cfg(feature = "postgres")] #[command(flatten)] pub pg: Option, + #[cfg(feature = "mbtiles")] + #[command(flatten)] + pub mbtiles: Option, } #[serde_with::serde_as] @@ -141,6 +144,8 @@ async fn start(copy_args: CopierArgs) -> MartinCpResult<()> { srv: SrvArgs::default(), #[cfg(feature = "postgres")] pg: copy_args.pg, + #[cfg(feature = "mbtiles")] + mbtiles: copy_args.mbtiles, }; args.merge_into_config(&mut config, &env)?; diff --git a/martin/src/file_config.rs b/martin/src/file_config.rs index 3f08d1f9d..2d89d8aba 100644 --- a/martin/src/file_config.rs +++ b/martin/src/file_config.rs @@ -346,7 +346,12 @@ async fn resolve_int( info!("Configured source {id} from {}", can.display()); files.insert(can); configs.insert(id.clone(), FileConfigSrc::Path(path.clone())); - results.push(cfg.custom.new_sources(id, path).await?); + let src_result = cfg.custom.new_sources(id, path).await; + match src_result { + Err(FileError::IgnoreOnInvalid(_, _)) => {} + Err(e) => return Err(e), + Ok(src) => results.push(src), + }; } } } diff --git a/martin/src/mbtiles/mod.rs b/martin/src/mbtiles/mod.rs index 1b88b3632..047de4b3b 100644 --- a/martin/src/mbtiles/mod.rs +++ b/martin/src/mbtiles/mod.rs @@ -5,6 +5,7 @@ use std::sync::Arc; use async_trait::async_trait; use clap::ValueEnum; +use enum_display::EnumDisplay; use log::{trace, warn}; use martin_tile_utils::{TileCoord, TileInfo}; use mbtiles::{MbtResult, MbtilesPool, ValidationLevel}; @@ -18,7 +19,9 @@ use crate::file_config::{ConfigExtras, FileResult, SourceConfigExtras}; use crate::source::{TileData, TileInfoSource, UrlQuery}; use crate::{MartinResult, Source}; -#[derive(PartialEq, Eq, Debug, Clone, Copy, Default, Serialize, Deserialize, ValueEnum)] +#[derive( + PartialEq, Eq, Debug, Clone, Copy, Default, Serialize, Deserialize, ValueEnum, EnumDisplay, +)] #[serde(rename_all = "lowercase")] pub enum OnInvalid { /// Print warning message, and abort if the error is critical @@ -26,7 +29,7 @@ pub enum OnInvalid { Warn, /// Skip this source - IgnoreSource, + Ignore, /// Do not start Martin on any warnings Abort, @@ -61,7 +64,7 @@ impl SourceConfigExtras for MbtConfig { validation_error.to_string(), )); } - OnInvalid::IgnoreSource => { + OnInvalid::Ignore => { return Err(FileError::IgnoreOnInvalid( path, validation_error.to_string(), diff --git a/mbtiles/src/mbtiles.rs b/mbtiles/src/mbtiles.rs index ff871ca94..dffff8341 100644 --- a/mbtiles/src/mbtiles.rs +++ b/mbtiles/src/mbtiles.rs @@ -23,7 +23,7 @@ pub enum MbtTypeCli { Normalized, } -#[derive(PartialEq, Eq, Debug, Clone, Copy, Default, Serialize, Deserialize)] +#[derive(PartialEq, Eq, Debug, Clone, Copy, Default, Serialize, Deserialize, EnumDisplay)] #[serde(rename_all = "lowercase")] #[cfg_attr(feature = "cli", derive(clap::ValueEnum))] pub enum ValidationLevel { From 1002e794715fce50d154730c8d4ae6cce3473509 Mon Sep 17 00:00:00 2001 From: David Disch Date: Sun, 16 Feb 2025 17:15:08 +1100 Subject: [PATCH 10/19] Fix column types in webp.mbtiles, add ignore on invalid escape hatch to new_sources_url in case tile sources from URLs produce this error --- martin/src/file_config.rs | 14 ++++++++++++-- tests/fixtures/mbtiles/webp.mbtiles | Bin 28672 -> 40960 bytes 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/martin/src/file_config.rs b/martin/src/file_config.rs index 2d89d8aba..38a556dc1 100644 --- a/martin/src/file_config.rs +++ b/martin/src/file_config.rs @@ -274,7 +274,12 @@ async fn resolve_int( let dup = if dup { "duplicate " } else { "" }; let id = idr.resolve(&id, url.to_string()); configs.insert(id.clone(), source); - results.push(cfg.custom.new_sources_url(id.clone(), url.clone()).await?); + let src_result = cfg.custom.new_sources_url(id.clone(), url.clone()).await; + match src_result { + Err(FileError::IgnoreOnInvalid(_, _)) => {} + Err(e) => return Err(e), + Ok(src) => results.push(src), + }; info!("Configured {dup}source {id} from {}", sanitize_url(&url)); } else { let can = source.abs_path()?; @@ -317,7 +322,12 @@ async fn resolve_int( let id = idr.resolve(id, url.to_string()); configs.insert(id.clone(), FileConfigSrc::Path(path)); - results.push(cfg.custom.new_sources_url(id.clone(), url.clone()).await?); + let src_result = cfg.custom.new_sources_url(id.clone(), url.clone()).await; + match src_result { + Err(FileError::IgnoreOnInvalid(_, _)) => {} + Err(e) => return Err(e), + Ok(src) => results.push(src), + }; info!("Configured source {id} from URL {}", sanitize_url(&url)); } else { let is_dir = path.is_dir(); diff --git a/tests/fixtures/mbtiles/webp.mbtiles b/tests/fixtures/mbtiles/webp.mbtiles index 3b53baf2e4d7f75bdfda11815b8be831cc3d50cb..d041f0a340e16f2848cb3000b06d5ad5f285b1f4 100644 GIT binary patch delta 283 zcmZp8z}RqrX@az%GXnzy7Z9@nF*6XWPSi0LcV^Jb+r`WGkb#9ynSp;ke-PgsK4m`n zjg4!0>&sc$#l^)L8+}U>lX6l^GILUk!32kMkgH>et3rsQlaH%{5?D-0L8B@^KQ}%n zwJbG9!P76q)!j8nM*$=kpPZjlnwy6vSd?FmCX$j^lBnS1$O8ZjK1t*N delta 254 zcmZoTz|`=7ae}mpP5};T%57dwj?nrC$%IqC$$(%us8>~I)=C^gg83+xGE@t#gsI-6cnoR^K;{K zQp-|v6g>Sxbbultx%lM#oYLGp6rrO0auj|_Vo4&GrZ*FtxT-2+G}yl4%?bPq1-OB( zW8%NW!2g2((q=)0gZvW%SSMf77vte#WD#fJ2itAB`HMck0xx^aw69+Z7s}tsa@XYE JED-QP9sr;tNA~~# From c118f50079e425633457160c064116fe4514bfe8 Mon Sep 17 00:00:00 2001 From: David Disch Date: Sun, 16 Feb 2025 17:23:29 +1100 Subject: [PATCH 11/19] clippy --- martin/src/file_config.rs | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/martin/src/file_config.rs b/martin/src/file_config.rs index 38a556dc1..fe72a4618 100644 --- a/martin/src/file_config.rs +++ b/martin/src/file_config.rs @@ -274,8 +274,7 @@ async fn resolve_int( let dup = if dup { "duplicate " } else { "" }; let id = idr.resolve(&id, url.to_string()); configs.insert(id.clone(), source); - let src_result = cfg.custom.new_sources_url(id.clone(), url.clone()).await; - match src_result { + match cfg.custom.new_sources_url(id.clone(), url.clone()).await { Err(FileError::IgnoreOnInvalid(_, _)) => {} Err(e) => return Err(e), Ok(src) => results.push(src), @@ -293,8 +292,7 @@ async fn resolve_int( let id = idr.resolve(&id, can.to_string_lossy().to_string()); info!("Configured {dup}source {id} from {}", can.display()); configs.insert(id.clone(), source.clone()); - let src_result = cfg.custom.new_sources(id, source.into_path()).await; - match src_result { + match cfg.custom.new_sources(id, source.into_path()).await { Err(FileError::IgnoreOnInvalid(_, _)) => {} Err(e) => return Err(e), Ok(src) => results.push(src), @@ -322,8 +320,7 @@ async fn resolve_int( let id = idr.resolve(id, url.to_string()); configs.insert(id.clone(), FileConfigSrc::Path(path)); - let src_result = cfg.custom.new_sources_url(id.clone(), url.clone()).await; - match src_result { + match cfg.custom.new_sources_url(id.clone(), url.clone()).await { Err(FileError::IgnoreOnInvalid(_, _)) => {} Err(e) => return Err(e), Ok(src) => results.push(src), @@ -356,8 +353,7 @@ async fn resolve_int( info!("Configured source {id} from {}", can.display()); files.insert(can); configs.insert(id.clone(), FileConfigSrc::Path(path.clone())); - let src_result = cfg.custom.new_sources(id, path).await; - match src_result { + match cfg.custom.new_sources(id, path).await { Err(FileError::IgnoreOnInvalid(_, _)) => {} Err(e) => return Err(e), Ok(src) => results.push(src), From 94585520bdf3beb19431978a70c7315dfd1c531e Mon Sep 17 00:00:00 2001 From: David Disch Date: Sun, 16 Feb 2025 17:38:51 +1100 Subject: [PATCH 12/19] Add index --- tests/fixtures/mbtiles/webp.mbtiles | Bin 40960 -> 40960 bytes 1 file changed, 0 insertions(+), 0 deletions(-) diff --git a/tests/fixtures/mbtiles/webp.mbtiles b/tests/fixtures/mbtiles/webp.mbtiles index d041f0a340e16f2848cb3000b06d5ad5f285b1f4..8e0a2a5f6377e4f6d795ac1ee5e8572ff182548f 100644 GIT binary patch delta 233 zcmZoTz|?SnX@az%3j+fK7Z9@nF%uB0P1G?KcVW=W+r`WGkb#v?nSp;ke-PgsK4rew zn-vAt@=jLfJ6I3W0R*XR?Bep`jE#z!c`2zCC7C&?#qm`Ym0$*obC9cJh^s=VpJ!mG ztAeMWi)(}eT#16eAB3ZzQI(&c8=sR}mYSmj7K%^K&neB#gK~=U%Qd~3*u+&;8KX-Q zlYll9=45V8=AWy;1#$!<|2qc$cbf$j4)RZ&pv=a~%E7_Gx%rE|umB_DW{!X#{s7X# BL4p7P delta 90 zcmZoTz|?SnX@az%GXnzy7Z9@nF*6XWPSi0LcV^Jb+r`WGkb#9ynSp;ke-PgsK4m`n r&58nRc_%CL9b_z=*qFOHh<}~Jrh?D>jBJ~~*b56VGHvDv_~8!#cZ?S! From 9286647f032d3b74030f3bdc478a2abf575d7d22 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Sat, 22 Mar 2025 23:18:15 +0000 Subject: [PATCH 13/19] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- martin/src/args/mod.rs | 2 +- mbtiles/src/errors.rs | 4 +++- mbtiles/src/validation.rs | 2 +- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/martin/src/args/mod.rs b/martin/src/args/mod.rs index 0c1bfe929..89e2f9f04 100644 --- a/martin/src/args/mod.rs +++ b/martin/src/args/mod.rs @@ -7,7 +7,7 @@ pub use environment::{Env, OsEnv}; #[cfg(feature = "postgres")] mod pg; #[cfg(feature = "postgres")] -pub use pg::{BoundsCalcType, PgArgs, DEFAULT_BOUNDS_TIMEOUT}; +pub use pg::{BoundsCalcType, DEFAULT_BOUNDS_TIMEOUT, PgArgs}; #[cfg(feature = "mbtiles")] mod mbtiles; diff --git a/mbtiles/src/errors.rs b/mbtiles/src/errors.rs index c3651fe41..1faed0a22 100644 --- a/mbtiles/src/errors.rs +++ b/mbtiles/src/errors.rs @@ -65,7 +65,9 @@ pub enum MbtError { )] NonEmptyTargetFile(PathBuf), - #[error("The file {0} does not have the required uniqueness constraint. Try adding a unique index on the `map` or `tiles` table.")] + #[error( + "The file {0} does not have the required uniqueness constraint. Try adding a unique index on the `map` or `tiles` table." + )] NoUniquenessConstraint(String), #[error("Could not copy MBTiles file: {reason}")] diff --git a/mbtiles/src/validation.rs b/mbtiles/src/validation.rs index 7077ef84e..c17e87d79 100644 --- a/mbtiles/src/validation.rs +++ b/mbtiles/src/validation.rs @@ -3,7 +3,7 @@ use std::str::from_utf8; use enum_display::EnumDisplay; use log::{debug, info, warn}; -use martin_tile_utils::{Format, TileInfo, MAX_ZOOM}; +use martin_tile_utils::{Format, MAX_ZOOM, TileInfo}; use serde::Serialize; use serde_json::Value; use sqlx::sqlite::SqliteRow; From 3964b1e8f4098aef37d20842e3dd87eeee6ace42 Mon Sep 17 00:00:00 2001 From: David Disch Date: Sun, 23 Mar 2025 10:50:26 +1100 Subject: [PATCH 14/19] Add maybe_add_source fn which consumes IgnoreOnInvalid errors, adds the source to result if Ok and returns all other errors --- martin/src/file_config.rs | 38 +++++++++++++++++--------------------- martin/src/mbtiles/mod.rs | 4 ++-- mbtiles/src/errors.rs | 4 +--- 3 files changed, 20 insertions(+), 26 deletions(-) diff --git a/martin/src/file_config.rs b/martin/src/file_config.rs index fe72a4618..df14332ab 100644 --- a/martin/src/file_config.rs +++ b/martin/src/file_config.rs @@ -5,10 +5,11 @@ use std::path::{Path, PathBuf}; use futures::TryFutureExt; use log::{info, warn}; +use martin_tile_utils::TileInfo; use serde::{Deserialize, Serialize}; use url::Url; -use crate::MartinResult; +use crate::{MartinResult, Source}; use crate::OptOneMany::{Many, One}; use crate::config::{UnrecognizedValues, copy_unrecognized_config}; use crate::file_config::FileError::{ @@ -274,11 +275,7 @@ async fn resolve_int( let dup = if dup { "duplicate " } else { "" }; let id = idr.resolve(&id, url.to_string()); configs.insert(id.clone(), source); - match cfg.custom.new_sources_url(id.clone(), url.clone()).await { - Err(FileError::IgnoreOnInvalid(_, _)) => {} - Err(e) => return Err(e), - Ok(src) => results.push(src), - }; + maybe_add_source(&mut results, cfg.custom.new_sources_url(id.clone(), url.clone()).await )?; info!("Configured {dup}source {id} from {}", sanitize_url(&url)); } else { let can = source.abs_path()?; @@ -292,11 +289,7 @@ async fn resolve_int( let id = idr.resolve(&id, can.to_string_lossy().to_string()); info!("Configured {dup}source {id} from {}", can.display()); configs.insert(id.clone(), source.clone()); - match cfg.custom.new_sources(id, source.into_path()).await { - Err(FileError::IgnoreOnInvalid(_, _)) => {} - Err(e) => return Err(e), - Ok(src) => results.push(src), - }; + maybe_add_source(&mut results, cfg.custom.new_sources(id, source.into_path()).await)?; } } } @@ -320,11 +313,7 @@ async fn resolve_int( let id = idr.resolve(id, url.to_string()); configs.insert(id.clone(), FileConfigSrc::Path(path)); - match cfg.custom.new_sources_url(id.clone(), url.clone()).await { - Err(FileError::IgnoreOnInvalid(_, _)) => {} - Err(e) => return Err(e), - Ok(src) => results.push(src), - }; + maybe_add_source(&mut results, cfg.custom.new_sources_url(id.clone(), url.clone()).await)?; info!("Configured source {id} from URL {}", sanitize_url(&url)); } else { let is_dir = path.is_dir(); @@ -353,11 +342,7 @@ async fn resolve_int( info!("Configured source {id} from {}", can.display()); files.insert(can); configs.insert(id.clone(), FileConfigSrc::Path(path.clone())); - match cfg.custom.new_sources(id, path).await { - Err(FileError::IgnoreOnInvalid(_, _)) => {} - Err(e) => return Err(e), - Ok(src) => results.push(src), - }; + maybe_add_source(&mut results, cfg.custom.new_sources(id, path).await)?; } } } @@ -417,3 +402,14 @@ fn parse_url(is_enabled: bool, path: &Path) -> Result, FileError> { .map(|v| Url::parse(v).map_err(|e| InvalidSourceUrl(e, v.to_string()))) .transpose() } + +fn maybe_add_source(results: &mut Vec>, result: FileResult) -> Result<(), FileError> { + match result { + Err(FileError::IgnoreOnInvalid(_, _)) => Ok(()), + Err(e) => Err(e), + Ok(src) => { + results.push(src); + Ok(()) + }, + } +} diff --git a/martin/src/mbtiles/mod.rs b/martin/src/mbtiles/mod.rs index 047de4b3b..e7fdc50b4 100644 --- a/martin/src/mbtiles/mod.rs +++ b/martin/src/mbtiles/mod.rs @@ -189,6 +189,8 @@ mod tests { fn parse() { let cfg: FileConfigEnum = serde_yaml::from_str::>(indoc! {" + validate: thorough + on_invalid: abort paths: - /dir-path - /path/to/file2.ext @@ -200,8 +202,6 @@ mod tests { pm-src3: https://example.org/file3.ext pm-src4: path: https://example.org/file4.ext - validate: thorough - on_invalid: abort "}) .unwrap(); let res = cfg.finalize(""); diff --git a/mbtiles/src/errors.rs b/mbtiles/src/errors.rs index 1faed0a22..c332260b8 100644 --- a/mbtiles/src/errors.rs +++ b/mbtiles/src/errors.rs @@ -65,9 +65,7 @@ pub enum MbtError { )] NonEmptyTargetFile(PathBuf), - #[error( - "The file {0} does not have the required uniqueness constraint. Try adding a unique index on the `map` or `tiles` table." - )] + #[error("The file {0} does not have the required uniqueness constraint. Try adding a unique index on the `map` or `tiles` table. See: https://maplibre.org/martin/mbtiles-schema.html for more information.")] NoUniquenessConstraint(String), #[error("Could not copy MBTiles file: {reason}")] From fb80bf2b4dee3b1a88a49541ea7ef480083984ae Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Sat, 22 Mar 2025 23:51:21 +0000 Subject: [PATCH 15/19] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- martin/src/file_config.rs | 24 ++++++++++++++++++------ mbtiles/src/errors.rs | 4 +++- 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/martin/src/file_config.rs b/martin/src/file_config.rs index df14332ab..ccbb34f83 100644 --- a/martin/src/file_config.rs +++ b/martin/src/file_config.rs @@ -9,7 +9,6 @@ use martin_tile_utils::TileInfo; use serde::{Deserialize, Serialize}; use url::Url; -use crate::{MartinResult, Source}; use crate::OptOneMany::{Many, One}; use crate::config::{UnrecognizedValues, copy_unrecognized_config}; use crate::file_config::FileError::{ @@ -17,6 +16,7 @@ use crate::file_config::FileError::{ }; use crate::source::{TileInfoSource, TileInfoSources}; use crate::utils::{IdResolver, OptMainCache, OptOneMany}; +use crate::{MartinResult, Source}; pub type FileResult = Result; @@ -275,7 +275,10 @@ async fn resolve_int( let dup = if dup { "duplicate " } else { "" }; let id = idr.resolve(&id, url.to_string()); configs.insert(id.clone(), source); - maybe_add_source(&mut results, cfg.custom.new_sources_url(id.clone(), url.clone()).await )?; + maybe_add_source( + &mut results, + cfg.custom.new_sources_url(id.clone(), url.clone()).await, + )?; info!("Configured {dup}source {id} from {}", sanitize_url(&url)); } else { let can = source.abs_path()?; @@ -289,7 +292,10 @@ async fn resolve_int( let id = idr.resolve(&id, can.to_string_lossy().to_string()); info!("Configured {dup}source {id} from {}", can.display()); configs.insert(id.clone(), source.clone()); - maybe_add_source(&mut results, cfg.custom.new_sources(id, source.into_path()).await)?; + maybe_add_source( + &mut results, + cfg.custom.new_sources(id, source.into_path()).await, + )?; } } } @@ -313,7 +319,10 @@ async fn resolve_int( let id = idr.resolve(id, url.to_string()); configs.insert(id.clone(), FileConfigSrc::Path(path)); - maybe_add_source(&mut results, cfg.custom.new_sources_url(id.clone(), url.clone()).await)?; + maybe_add_source( + &mut results, + cfg.custom.new_sources_url(id.clone(), url.clone()).await, + )?; info!("Configured source {id} from URL {}", sanitize_url(&url)); } else { let is_dir = path.is_dir(); @@ -403,13 +412,16 @@ fn parse_url(is_enabled: bool, path: &Path) -> Result, FileError> { .transpose() } -fn maybe_add_source(results: &mut Vec>, result: FileResult) -> Result<(), FileError> { +fn maybe_add_source( + results: &mut Vec>, + result: FileResult, +) -> Result<(), FileError> { match result { Err(FileError::IgnoreOnInvalid(_, _)) => Ok(()), Err(e) => Err(e), Ok(src) => { results.push(src); Ok(()) - }, + } } } diff --git a/mbtiles/src/errors.rs b/mbtiles/src/errors.rs index c332260b8..47a11182a 100644 --- a/mbtiles/src/errors.rs +++ b/mbtiles/src/errors.rs @@ -65,7 +65,9 @@ pub enum MbtError { )] NonEmptyTargetFile(PathBuf), - #[error("The file {0} does not have the required uniqueness constraint. Try adding a unique index on the `map` or `tiles` table. See: https://maplibre.org/martin/mbtiles-schema.html for more information.")] + #[error( + "The file {0} does not have the required uniqueness constraint. Try adding a unique index on the `map` or `tiles` table. See: https://maplibre.org/martin/mbtiles-schema.html for more information." + )] NoUniquenessConstraint(String), #[error("Could not copy MBTiles file: {reason}")] From 723f2e4fb5fe2eb80572082930d8da353201baa5 Mon Sep 17 00:00:00 2001 From: David Disch Date: Sat, 12 Apr 2025 00:52:21 +1000 Subject: [PATCH 16/19] Move config to the top level args, start to allow override at source level --- martin/benches/bench.rs | 7 ++- martin/src/args/mbtiles.rs | 36 ------------- martin/src/args/mod.rs | 5 -- martin/src/args/root.rs | 14 ----- martin/src/args/srv.rs | 17 +++++- martin/src/bin/martin-cp.rs | 5 -- martin/src/cog/source.rs | 6 ++- martin/src/config.rs | 32 +++++++++++- martin/src/file_config.rs | 50 +++++++++++++----- martin/src/mbtiles/mod.rs | 100 +++++++++++++----------------------- martin/src/pg/pg_source.rs | 5 ++ martin/src/pmtiles/mod.rs | 6 ++- martin/src/source.rs | 11 ++-- martin/src/srv/config.rs | 29 ++++++++++- martin/src/srv/server.rs | 5 ++ martin/src/srv/tiles.rs | 8 +-- mbtiles/src/lib.rs | 2 +- mbtiles/src/mbtiles.rs | 15 ------ mbtiles/src/pool.rs | 23 ++++----- 19 files changed, 196 insertions(+), 180 deletions(-) delete mode 100644 martin/src/args/mbtiles.rs diff --git a/martin/benches/bench.rs b/martin/benches/bench.rs index bc26a171d..54d39abf6 100644 --- a/martin/benches/bench.rs +++ b/martin/benches/bench.rs @@ -1,6 +1,7 @@ use async_trait::async_trait; use criterion::async_executor::FuturesExecutor; use criterion::{Criterion, criterion_group, criterion_main}; +use martin::file_config::ValidationLevel; use martin::srv::DynTileSource; use martin::{CatalogSourceEntry, MartinResult, Source, TileData, TileSources, UrlQuery}; use martin_tile_utils::{Encoding, Format, TileCoord, TileInfo}; @@ -42,6 +43,10 @@ impl Source for NullSource { false } + async fn validate(&self, _validation_level: ValidationLevel) -> MartinResult<()> { + MartinResult::Ok(()) + } + async fn get_tile( &self, _xyz: TileCoord, @@ -63,7 +68,7 @@ async fn process_tile(sources: &TileSources) { } fn bench_null_source(c: &mut Criterion) { - let sources = TileSources::new(vec![vec![Box::new(NullSource::new())]]); + let sources = TileSources::new(vec![Box::new(NullSource::new())]); c.bench_function("get_table_source_tile", |b| { b.to_async(FuturesExecutor).iter(|| process_tile(&sources)); }); diff --git a/martin/src/args/mbtiles.rs b/martin/src/args/mbtiles.rs deleted file mode 100644 index 8a564ccfe..000000000 --- a/martin/src/args/mbtiles.rs +++ /dev/null @@ -1,36 +0,0 @@ -use log::info; -use mbtiles::ValidationLevel; - -use crate::mbtiles::{MbtConfig, OnInvalid}; - -#[derive(clap::Args, Debug, PartialEq, Default)] -#[command(about, version)] -pub struct MbtArgs { - /// Level of validation to apply to .mbtiles - #[arg(long)] - pub validate_mbtiles: Option, - /// How to handle invalid .mbtiles - #[arg(long)] - pub on_invalid_mbtiles: Option, -} - -impl MbtArgs { - /// Apply CLI parameters from `self` to the configuration loaded from the config file `mbtiles` - pub fn override_config(self, mbt_config: &mut MbtConfig) { - // This ensures that if a new parameter is added to the struct, it will not be forgotten here - let Self { - validate_mbtiles, - on_invalid_mbtiles, - } = self; - - if let Some(value) = validate_mbtiles { - info!("Overriding configured default mbtiles.validate to {value}"); - mbt_config.validate = validate_mbtiles.unwrap_or_default(); - } - - if let Some(value) = on_invalid_mbtiles { - info!("Overriding configured default mbtiles.on_invalid to {value}"); - mbt_config.on_invalid = on_invalid_mbtiles.unwrap_or_default(); - } - } -} diff --git a/martin/src/args/mod.rs b/martin/src/args/mod.rs index 89e2f9f04..b0ff3fb4f 100644 --- a/martin/src/args/mod.rs +++ b/martin/src/args/mod.rs @@ -9,11 +9,6 @@ mod pg; #[cfg(feature = "postgres")] pub use pg::{BoundsCalcType, DEFAULT_BOUNDS_TIMEOUT, PgArgs}; -#[cfg(feature = "mbtiles")] -mod mbtiles; -#[cfg(feature = "mbtiles")] -pub use mbtiles::MbtArgs; - mod root; pub use root::{Args, ExtraArgs, MetaArgs}; diff --git a/martin/src/args/root.rs b/martin/src/args/root.rs index bd1c31c5d..6c58d493c 100644 --- a/martin/src/args/root.rs +++ b/martin/src/args/root.rs @@ -35,9 +35,6 @@ pub struct Args { #[cfg(feature = "postgres")] #[command(flatten)] pub pg: Option, - #[cfg(feature = "mbtiles")] - #[command(flatten)] - pub mbtiles: Option, } // None of these params will be transferred to the config @@ -118,17 +115,6 @@ impl Args { config.pmtiles = parse_file_args(&mut cli_strings, &["pmtiles"], true); } - #[cfg(feature = "mbtiles")] - { - if !cli_strings.is_empty() { - config.mbtiles = parse_file_args(&mut cli_strings, &["mbtiles"], false); - } - let mbt_args = self.mbtiles.unwrap_or_default(); - if let FileConfigEnum::Config(c) = &mut config.mbtiles { - mbt_args.override_config(&mut c.custom); - } - } - #[cfg(feature = "cog")] if !cli_strings.is_empty() { config.cog = parse_file_args(&mut cli_strings, &["tif", "tiff"], false); diff --git a/martin/src/args/srv.rs b/martin/src/args/srv.rs index 6722cb2da..cf3db4b03 100644 --- a/martin/src/args/srv.rs +++ b/martin/src/args/srv.rs @@ -1,7 +1,10 @@ use clap::ValueEnum; use serde::{Deserialize, Serialize}; -use crate::srv::{KEEP_ALIVE_DEFAULT, LISTEN_ADDRESSES_DEFAULT, SrvConfig}; +use crate::{ + file_config::{OnInvalid, ValidationLevel}, + srv::{KEEP_ALIVE_DEFAULT, LISTEN_ADDRESSES_DEFAULT, SrvConfig}, +}; #[allow(clippy::doc_markdown)] #[derive(clap::Args, Debug, PartialEq, Default)] @@ -34,6 +37,12 @@ pub struct SrvArgs { #[arg(short = 'u', long = "webui")] #[cfg(feature = "webui")] pub web_ui: Option, + /// Level of validation to apply to sources + #[arg(long)] + pub validate: Option, + /// How to handle invalid source + #[arg(long)] + pub on_invalid: Option, } #[cfg(feature = "webui")] @@ -81,6 +90,12 @@ impl SrvArgs { if self.preferred_encoding.is_some() { srv_config.preferred_encoding = self.preferred_encoding; } + if let Some(v) = self.validate { + srv_config.validate = v + } + if let Some(v) = self.on_invalid { + srv_config.on_invalid = v; + } #[cfg(feature = "webui")] if self.web_ui.is_some() { srv_config.web_ui = self.web_ui; diff --git a/martin/src/bin/martin-cp.rs b/martin/src/bin/martin-cp.rs index 3be849546..e0cd807cd 100644 --- a/martin/src/bin/martin-cp.rs +++ b/martin/src/bin/martin-cp.rs @@ -49,9 +49,6 @@ pub struct CopierArgs { #[cfg(feature = "postgres")] #[command(flatten)] pub pg: Option, - #[cfg(feature = "mbtiles")] - #[command(flatten)] - pub mbtiles: Option, } #[serde_with::serde_as] @@ -144,8 +141,6 @@ async fn start(copy_args: CopierArgs) -> MartinCpResult<()> { srv: SrvArgs::default(), #[cfg(feature = "postgres")] pg: copy_args.pg, - #[cfg(feature = "mbtiles")] - mbtiles: copy_args.mbtiles, }; args.merge_into_config(&mut config, &env)?; diff --git a/martin/src/cog/source.rs b/martin/src/cog/source.rs index 68db36de1..38f948884 100644 --- a/martin/src/cog/source.rs +++ b/martin/src/cog/source.rs @@ -13,7 +13,7 @@ use tiff::tags::Tag::{self, GdalNodata}; use tilejson::{TileJSON, tilejson}; use super::CogError; -use crate::file_config::{FileError, FileResult}; +use crate::file_config::{FileError, FileResult, ValidationLevel}; use crate::{MartinResult, Source, TileData, UrlQuery}; #[derive(Clone, Debug)] @@ -151,6 +151,10 @@ impl Source for CogSource { Box::new(self.clone()) } + async fn validate(&self, _validation_level: ValidationLevel) -> MartinResult<()> { + MartinResult::Ok(()) + } + async fn get_tile( &self, xyz: TileCoord, diff --git a/martin/src/config.rs b/martin/src/config.rs index 227ec5b9e..c17290ff2 100644 --- a/martin/src/config.rs +++ b/martin/src/config.rs @@ -6,10 +6,11 @@ use std::path::{Path, PathBuf}; use std::pin::Pin; use futures::future::try_join_all; -use log::info; +use log::{info, warn}; use serde::{Deserialize, Serialize}; use subst::VariableMap; +use crate::file_config::OnInvalid; use crate::MartinError::{ConfigLoadError, ConfigParseError, ConfigWriteError, NoSources}; #[cfg(any(feature = "fonts", feature = "postgres"))] use crate::OptOneMany; @@ -209,7 +210,34 @@ impl Config { sources.push(Box::pin(val)); } - Ok(TileSources::new(try_join_all(sources).await?)) + let resolved_sources = try_join_all(sources).await? + .into_iter() + .flatten() + .collect::(); + + let mut sources_to_prune: Vec = vec![]; + for (idx, source) in resolved_sources.iter().enumerate() { + let validation_result = source.validate(self.srv.validate).await; + if let Err(e) = validation_result { + match self.srv.on_invalid { + OnInvalid::Abort => { + return MartinResult::Err(e) + }, + OnInvalid::Warn => { + warn!( + "Source {} failed validation, this may cause performance issues: {}", + source.get_id(), + e.to_string() + ); + }, + OnInvalid::Ignore => { + sources_to_prune.push(idx) + }, + } + } + } + + Ok(TileSources::new(resolved_sources)) } pub fn save_to_file(&self, file_name: PathBuf) -> MartinResult<()> { diff --git a/martin/src/file_config.rs b/martin/src/file_config.rs index ccbb34f83..883f07d81 100644 --- a/martin/src/file_config.rs +++ b/martin/src/file_config.rs @@ -3,9 +3,9 @@ use std::fmt::Debug; use std::mem; use std::path::{Path, PathBuf}; +use clap::ValueEnum; use futures::TryFutureExt; use log::{info, warn}; -use martin_tile_utils::TileInfo; use serde::{Deserialize, Serialize}; use url::Url; @@ -62,6 +62,31 @@ pub enum FileError { CogError(#[from] crate::cog::CogError), } +#[derive(PartialEq, Eq, Debug, Clone, Copy, Default, Serialize, Deserialize, ValueEnum)] +#[serde(rename_all = "lowercase")] +pub enum ValidationLevel { + /// Quickly check the source + #[default] + Fast, + + /// Do a slow check of everything + Thorough, +} + +#[derive(PartialEq, Eq, Debug, Clone, Copy, Default, Serialize, Deserialize, ValueEnum)] +#[serde(rename_all = "lowercase")] +pub enum OnInvalid { + /// Print warning message, and abort if the error is critical + #[default] + Warn, + + /// Skip this source + Ignore, + + /// Do not start Martin on any warnings + Abort, +} + pub trait ConfigExtras: Clone + Debug + Default + PartialEq + Send { fn init_parsing(&mut self, _cache: OptMainCache) -> FileResult<()> { Ok(()) @@ -130,6 +155,8 @@ impl FileConfigEnum { } else { Some(configs) }, + validate: None, + on_invalid: None, custom, }) } @@ -187,6 +214,10 @@ pub struct FileConfig { pub paths: OptOneMany, /// A map of source IDs to file paths or config objects pub sources: Option>, + #[serde(default)] + pub validate: Option, + #[serde(default)] + pub on_invalid: Option, /// Any customizations related to the specifics of the configuration section #[serde(flatten)] pub custom: T, @@ -275,10 +306,7 @@ async fn resolve_int( let dup = if dup { "duplicate " } else { "" }; let id = idr.resolve(&id, url.to_string()); configs.insert(id.clone(), source); - maybe_add_source( - &mut results, - cfg.custom.new_sources_url(id.clone(), url.clone()).await, - )?; + results.push(cfg.custom.new_sources_url(id.clone(), url.clone()).await?); info!("Configured {dup}source {id} from {}", sanitize_url(&url)); } else { let can = source.abs_path()?; @@ -292,10 +320,7 @@ async fn resolve_int( let id = idr.resolve(&id, can.to_string_lossy().to_string()); info!("Configured {dup}source {id} from {}", can.display()); configs.insert(id.clone(), source.clone()); - maybe_add_source( - &mut results, - cfg.custom.new_sources(id, source.into_path()).await, - )?; + results.push(cfg.custom.new_sources(id, source.into_path()).await?); } } } @@ -319,10 +344,7 @@ async fn resolve_int( let id = idr.resolve(id, url.to_string()); configs.insert(id.clone(), FileConfigSrc::Path(path)); - maybe_add_source( - &mut results, - cfg.custom.new_sources_url(id.clone(), url.clone()).await, - )?; + results.push(cfg.custom.new_sources_url(id.clone(), url.clone()).await?); info!("Configured source {id} from URL {}", sanitize_url(&url)); } else { let is_dir = path.is_dir(); @@ -351,7 +373,7 @@ async fn resolve_int( info!("Configured source {id} from {}", can.display()); files.insert(can); configs.insert(id.clone(), FileConfigSrc::Path(path.clone())); - maybe_add_source(&mut results, cfg.custom.new_sources(id, path).await)?; + results.push(cfg.custom.new_sources(id, path).await?); } } } diff --git a/martin/src/mbtiles/mod.rs b/martin/src/mbtiles/mod.rs index e7fdc50b4..2f63019f5 100644 --- a/martin/src/mbtiles/mod.rs +++ b/martin/src/mbtiles/mod.rs @@ -4,47 +4,23 @@ use std::path::PathBuf; use std::sync::Arc; use async_trait::async_trait; -use clap::ValueEnum; -use enum_display::EnumDisplay; -use log::{trace, warn}; +use log::trace; use martin_tile_utils::{TileCoord, TileInfo}; -use mbtiles::{MbtResult, MbtilesPool, ValidationLevel}; +use mbtiles::MbtilesPool; use serde::{Deserialize, Serialize}; use tilejson::TileJSON; use url::Url; use crate::config::UnrecognizedValues; -use crate::file_config::FileError::{self, AcquireConnError, InvalidMetadata, IoError}; -use crate::file_config::{ConfigExtras, FileResult, SourceConfigExtras}; +use crate::file_config::FileError::{AcquireConnError, InvalidMetadata, IoError}; +use crate::file_config::{ConfigExtras, FileResult, OnInvalid, SourceConfigExtras, ValidationLevel}; use crate::source::{TileData, TileInfoSource, UrlQuery}; -use crate::{MartinResult, Source}; - -#[derive( - PartialEq, Eq, Debug, Clone, Copy, Default, Serialize, Deserialize, ValueEnum, EnumDisplay, -)] -#[serde(rename_all = "lowercase")] -pub enum OnInvalid { - /// Print warning message, and abort if the error is critical - #[default] - Warn, - - /// Skip this source - Ignore, - - /// Do not start Martin on any warnings - Abort, -} +use crate::{MartinError, MartinResult, Source}; #[derive(Clone, Debug, Default, PartialEq, Serialize, Deserialize)] pub struct MbtConfig { #[serde(flatten)] pub unrecognized: UnrecognizedValues, - - #[serde(default)] - pub validate: ValidationLevel, - - #[serde(default)] - pub on_invalid: OnInvalid, } impl ConfigExtras for MbtConfig { @@ -55,31 +31,7 @@ impl ConfigExtras for MbtConfig { impl SourceConfigExtras for MbtConfig { async fn new_sources(&self, id: String, path: PathBuf) -> FileResult { - let source = MbtSource::new(id, path.clone()).await?; - if let Err(validation_error) = source.validate(self.validate).await { - match self.on_invalid { - OnInvalid::Abort => { - return Err(FileError::AbortOnInvalid( - path, - validation_error.to_string(), - )); - } - OnInvalid::Ignore => { - return Err(FileError::IgnoreOnInvalid( - path, - validation_error.to_string(), - )); - } - OnInvalid::Warn => { - warn!( - "Source {} failed validation, this may cause performance issues: {}", - path.display(), - validation_error.to_string() - ); - } - } - } - Ok(Box::new(source)) + Ok(Box::new(MbtSource::new(id, path.clone()).await?)) } // TODO: Remove #[allow] after switching to Rust/Clippy v1.78+ in CI @@ -96,6 +48,8 @@ pub struct MbtSource { mbtiles: Arc, tilejson: TileJSON, tile_info: TileInfo, + // validation_level: ValidationLevel, + // on_invalid: OnInvalid } impl Debug for MbtSource { @@ -128,10 +82,6 @@ impl MbtSource { tile_info: meta.tile_info, }) } - - async fn validate(&self, validation_level: ValidationLevel) -> MbtResult<()> { - self.mbtiles.validate(validation_level).await - } } #[async_trait] @@ -152,6 +102,30 @@ impl Source for MbtSource { Box::new(self.clone()) } + // fn get_validation_level(&self) -> ValidationLevel { + // self.validation_level + // } + + // fn get_on_invalid(&self) -> OnInvalid { + // self.on_invalid + // } + + async fn validate(&self, validation_level: ValidationLevel) -> MartinResult<()> { + match validation_level { + ValidationLevel::Thorough => self + .mbtiles + .validate_thorough() + .await + .map_err(MartinError::from), + ValidationLevel::Fast => self + .mbtiles + .validate_fast() + .await + .map_err(MartinError::from), + _ => Ok(()), + } + } + async fn get_tile( &self, xyz: TileCoord, @@ -179,11 +153,11 @@ mod tests { use std::collections::BTreeMap; use std::path::PathBuf; + use crate::mbtiles::ValidationLevel; use indoc::indoc; - use mbtiles::ValidationLevel; - use crate::file_config::{FileConfigEnum, FileConfigSource, FileConfigSrc}; - use crate::mbtiles::{MbtConfig, OnInvalid}; + use crate::file_config::{FileConfigEnum, FileConfigSource, FileConfigSrc, OnInvalid}; + use crate::mbtiles::MbtConfig; #[test] fn parse() { @@ -243,7 +217,7 @@ mod tests { ), ])) ); - assert_eq!(cfg.custom.validate, ValidationLevel::Thorough); - assert_eq!(cfg.custom.on_invalid, OnInvalid::Abort); + assert_eq!(cfg.validate, Some(ValidationLevel::Thorough)); + assert_eq!(cfg.on_invalid, Some(OnInvalid::Abort)); } } diff --git a/martin/src/pg/pg_source.rs b/martin/src/pg/pg_source.rs index dd5c5c087..c71bcb006 100644 --- a/martin/src/pg/pg_source.rs +++ b/martin/src/pg/pg_source.rs @@ -6,6 +6,7 @@ use martin_tile_utils::Format::Mvt; use martin_tile_utils::{TileCoord, TileInfo}; use tilejson::TileJSON; +use crate::file_config::ValidationLevel; use crate::MartinResult; use crate::pg::PgError::{GetTileError, GetTileWithQueryError, PrepareQueryError}; use crate::pg::pool::PgPool; @@ -54,6 +55,10 @@ impl Source for PgSource { self.info.use_url_query } + async fn validate(&self, _validation_level: ValidationLevel) -> MartinResult<()> { + MartinResult::Ok(()) + } + async fn get_tile( &self, xyz: TileCoord, diff --git a/martin/src/pmtiles/mod.rs b/martin/src/pmtiles/mod.rs index d4f66fc7e..cd946d6b5 100644 --- a/martin/src/pmtiles/mod.rs +++ b/martin/src/pmtiles/mod.rs @@ -19,7 +19,7 @@ use url::Url; use crate::config::UnrecognizedValues; use crate::file_config::FileError::{InvalidMetadata, InvalidUrlMetadata, IoError}; -use crate::file_config::{ConfigExtras, FileError, FileResult, SourceConfigExtras}; +use crate::file_config::{ConfigExtras, FileError, FileResult, SourceConfigExtras, ValidationLevel}; use crate::source::{TileInfoSource, UrlQuery}; use crate::utils::cache::get_cached_value; use crate::utils::{CacheKey, CacheValue, OptMainCache}; @@ -256,6 +256,10 @@ macro_rules! impl_pmtiles_source { Box::new(self.clone()) } + async fn validate(&self, _validation_level: ValidationLevel) -> MartinResult<()> { + MartinResult::Ok(()) + } + async fn get_tile( &self, xyz: TileCoord, diff --git a/martin/src/source.rs b/martin/src/source.rs index 4679ef244..a0fe0ce36 100644 --- a/martin/src/source.rs +++ b/martin/src/source.rs @@ -9,7 +9,7 @@ use martin_tile_utils::{TileCoord, TileInfo}; use serde::{Deserialize, Serialize}; use tilejson::TileJSON; -use crate::MartinResult; +use crate::{file_config::{OnInvalid, ValidationLevel}, MartinResult}; pub type TileData = Vec; pub type UrlQuery = HashMap; @@ -24,11 +24,10 @@ pub type TileCatalog = DashMap; impl TileSources { #[must_use] - pub fn new(sources: Vec) -> Self { + pub fn new(sources: TileInfoSources) -> Self { Self( sources .into_iter() - .flatten() .map(|src| (src.get_id().to_string(), src)) .collect(), ) @@ -115,6 +114,12 @@ pub trait Source: Send + Debug { false } + // fn get_validation_level(&self) -> ValidationLevel; + + // fn get_on_invalid(&self) -> OnInvalid; + + async fn validate(&self, validation_level: ValidationLevel) -> MartinResult<()>; + async fn get_tile( &self, xyz: TileCoord, diff --git a/martin/src/srv/config.rs b/martin/src/srv/config.rs index 1e1168093..86246e15a 100644 --- a/martin/src/srv/config.rs +++ b/martin/src/srv/config.rs @@ -1,6 +1,9 @@ use serde::{Deserialize, Serialize}; -use crate::args::PreferredEncoding; +use crate::{ + args::PreferredEncoding, + file_config::{OnInvalid, ValidationLevel}, +}; pub const KEEP_ALIVE_DEFAULT: u64 = 75; pub const LISTEN_ADDRESSES_DEFAULT: &str = "0.0.0.0:3000"; @@ -13,6 +16,10 @@ pub struct SrvConfig { pub base_path: Option, pub worker_processes: Option, pub preferred_encoding: Option, + #[serde(default)] + pub validate: ValidationLevel, + #[serde(default)] + pub on_invalid: OnInvalid, #[cfg(feature = "webui")] pub web_ui: Option, } @@ -72,5 +79,25 @@ mod tests { ..Default::default() } ); + assert_eq!( + serde_yaml::from_str::(indoc! {" + keep_alive: 75 + listen_addresses: '0.0.0.0:3000' + worker_processes: 8 + preferred_encoding: brotli + validate: thorough + on_invalid: abort + "}) + .unwrap(), + SrvConfig { + keep_alive: Some(75), + listen_addresses: some("0.0.0.0:3000"), + worker_processes: Some(8), + preferred_encoding: Some(PreferredEncoding::Brotli), + validate: ValidationLevel::Thorough, + on_invalid: OnInvalid::Abort, + ..Default::default() + } + ); } } diff --git a/martin/src/srv/server.rs b/martin/src/srv/server.rs index acb977fe2..d442faa19 100644 --- a/martin/src/srv/server.rs +++ b/martin/src/srv/server.rs @@ -212,6 +212,7 @@ pub mod tests { use tilejson::TileJSON; use super::*; + use crate::file_config::ValidationLevel; use crate::UrlQuery; use crate::source::{Source, TileData, TileInfoSource}; @@ -240,6 +241,10 @@ pub mod tests { Box::new(self.clone()) } + async fn validate(&self, _validation_level: ValidationLevel) -> MartinResult<()> { + MartinResult::Ok(()) + } + async fn get_tile( &self, _xyz: TileCoord, diff --git a/martin/src/srv/tiles.rs b/martin/src/srv/tiles.rs index 50e0ef534..d06f677fb 100644 --- a/martin/src/srv/tiles.rs +++ b/martin/src/srv/tiles.rs @@ -319,11 +319,11 @@ mod tests { #[case] preferred_enc: Option, #[case] expected_enc: Encoding, ) { - let sources = TileSources::new(vec![vec![Box::new(TestSource { + let sources = TileSources::new(vec![Box::new(TestSource { id: "test_source", tj: tilejson! { tiles: vec![] }, data: vec![1_u8, 2, 3], - })]]); + })]); let accept_enc = Some(AcceptEncoding( accept_enc.iter().map(|s| s.parse().unwrap()).collect(), @@ -357,10 +357,10 @@ mod tests { tj: tilejson! { tiles: vec![] }, data: Vec::default(), }; - let sources = TileSources::new(vec![vec![ + let sources = TileSources::new(vec![ Box::new(non_empty_source), Box::new(empty_source), - ]]); + ]); for (source_id, expected) in &[ ("non-empty", vec![1_u8, 2, 3]), diff --git a/mbtiles/src/lib.rs b/mbtiles/src/lib.rs index dac8da7c7..3a60ed625 100644 --- a/mbtiles/src/lib.rs +++ b/mbtiles/src/lib.rs @@ -11,7 +11,7 @@ mod errors; pub use errors::{MbtError, MbtResult}; mod mbtiles; -pub use mbtiles::{CopyType, MbtTypeCli, Mbtiles, ValidationLevel}; +pub use mbtiles::{CopyType, MbtTypeCli, Mbtiles}; mod metadata; pub use metadata::Metadata; diff --git a/mbtiles/src/mbtiles.rs b/mbtiles/src/mbtiles.rs index dffff8341..2376ffe71 100644 --- a/mbtiles/src/mbtiles.rs +++ b/mbtiles/src/mbtiles.rs @@ -23,21 +23,6 @@ pub enum MbtTypeCli { Normalized, } -#[derive(PartialEq, Eq, Debug, Clone, Copy, Default, Serialize, Deserialize, EnumDisplay)] -#[serde(rename_all = "lowercase")] -#[cfg_attr(feature = "cli", derive(clap::ValueEnum))] -pub enum ValidationLevel { - /// Do not validate - Skip, - - /// Quickly check the file - #[default] - Fast, - - /// Do a slow check of everything - Thorough, -} - #[derive(Default, Debug, Clone, Copy, Hash, PartialEq, Eq, Serialize, Deserialize, EnumDisplay)] #[enum_display(case = "Kebab")] #[cfg_attr(feature = "cli", derive(clap::ValueEnum))] diff --git a/mbtiles/src/pool.rs b/mbtiles/src/pool.rs index 2d74f6c4d..7ef3f64b9 100644 --- a/mbtiles/src/pool.rs +++ b/mbtiles/src/pool.rs @@ -3,7 +3,6 @@ use std::path::Path; use sqlx::{Pool, Sqlite, SqlitePool}; use crate::errors::MbtResult; -use crate::mbtiles::ValidationLevel; use crate::{AggHashType, IntegrityCheckType, Mbtiles, Metadata}; #[derive(Clone, Debug)] @@ -29,19 +28,17 @@ impl MbtilesPool { self.mbtiles.get_tile(&mut *conn, z, x, y).await } - pub async fn validate(&self, validation_level: ValidationLevel) -> MbtResult<()> { + pub async fn validate_thorough(&self) -> MbtResult<()> { let mut conn = self.pool.acquire().await?; - match validation_level { - ValidationLevel::Thorough => { - self.mbtiles - .validate(&mut *conn, IntegrityCheckType::Full, AggHashType::Verify) - .await?; - } - ValidationLevel::Fast => { - self.mbtiles.detect_type(&mut *conn).await?; - } - ValidationLevel::Skip => {} - } + self.mbtiles + .validate(&mut *conn, IntegrityCheckType::Full, AggHashType::Verify) + .await?; + Ok(()) + } + + pub async fn validate_fast(&self) -> MbtResult<()> { + let mut conn = self.pool.acquire().await?; + self.mbtiles.detect_type(&mut *conn).await?; Ok(()) } } From 6fffec6041244418d9d02270cde5045f94144785 Mon Sep 17 00:00:00 2001 From: David Disch Date: Sat, 12 Apr 2025 00:55:30 +1000 Subject: [PATCH 17/19] clippy and fmt --- martin/src/args/srv.rs | 2 +- martin/src/config.rs | 19 +++++++++---------- martin/src/file_config.rs | 16 +--------------- martin/src/mbtiles/mod.rs | 3 +-- martin/src/pg/pg_source.rs | 2 +- martin/src/pmtiles/mod.rs | 4 +++- martin/src/source.rs | 2 +- martin/src/srv/server.rs | 2 +- martin/src/srv/tiles.rs | 5 +---- 9 files changed, 19 insertions(+), 36 deletions(-) diff --git a/martin/src/args/srv.rs b/martin/src/args/srv.rs index cf3db4b03..2c28beb6d 100644 --- a/martin/src/args/srv.rs +++ b/martin/src/args/srv.rs @@ -91,7 +91,7 @@ impl SrvArgs { srv_config.preferred_encoding = self.preferred_encoding; } if let Some(v) = self.validate { - srv_config.validate = v + srv_config.validate = v; } if let Some(v) = self.on_invalid { srv_config.on_invalid = v; diff --git a/martin/src/config.rs b/martin/src/config.rs index c17290ff2..886763734 100644 --- a/martin/src/config.rs +++ b/martin/src/config.rs @@ -10,7 +10,6 @@ use log::{info, warn}; use serde::{Deserialize, Serialize}; use subst::VariableMap; -use crate::file_config::OnInvalid; use crate::MartinError::{ConfigLoadError, ConfigParseError, ConfigWriteError, NoSources}; #[cfg(any(feature = "fonts", feature = "postgres"))] use crate::OptOneMany; @@ -21,6 +20,7 @@ use crate::OptOneMany; feature = "cog" ))] use crate::file_config::FileConfigEnum; +use crate::file_config::OnInvalid; use crate::source::{TileInfoSources, TileSources}; use crate::srv::{RESERVED_KEYWORDS, SrvConfig}; use crate::utils::{CacheValue, MainCache, OptMainCache, init_aws_lc_tls, parse_base_path}; @@ -210,29 +210,28 @@ impl Config { sources.push(Box::pin(val)); } - let resolved_sources = try_join_all(sources).await? + let resolved_sources = try_join_all(sources) + .await? .into_iter() .flatten() .collect::(); - + let mut sources_to_prune: Vec = vec![]; for (idx, source) in resolved_sources.iter().enumerate() { let validation_result = source.validate(self.srv.validate).await; if let Err(e) = validation_result { match self.srv.on_invalid { - OnInvalid::Abort => { - return MartinResult::Err(e) - }, + OnInvalid::Abort => return MartinResult::Err(e), OnInvalid::Warn => { warn!( "Source {} failed validation, this may cause performance issues: {}", source.get_id(), - e.to_string() + e ); - }, + } OnInvalid::Ignore => { - sources_to_prune.push(idx) - }, + sources_to_prune.push(idx); + } } } } diff --git a/martin/src/file_config.rs b/martin/src/file_config.rs index 883f07d81..e54defab4 100644 --- a/martin/src/file_config.rs +++ b/martin/src/file_config.rs @@ -9,6 +9,7 @@ use log::{info, warn}; use serde::{Deserialize, Serialize}; use url::Url; +use crate::MartinResult; use crate::OptOneMany::{Many, One}; use crate::config::{UnrecognizedValues, copy_unrecognized_config}; use crate::file_config::FileError::{ @@ -16,7 +17,6 @@ use crate::file_config::FileError::{ }; use crate::source::{TileInfoSource, TileInfoSources}; use crate::utils::{IdResolver, OptMainCache, OptOneMany}; -use crate::{MartinResult, Source}; pub type FileResult = Result; @@ -433,17 +433,3 @@ fn parse_url(is_enabled: bool, path: &Path) -> Result, FileError> { .map(|v| Url::parse(v).map_err(|e| InvalidSourceUrl(e, v.to_string()))) .transpose() } - -fn maybe_add_source( - results: &mut Vec>, - result: FileResult, -) -> Result<(), FileError> { - match result { - Err(FileError::IgnoreOnInvalid(_, _)) => Ok(()), - Err(e) => Err(e), - Ok(src) => { - results.push(src); - Ok(()) - } - } -} diff --git a/martin/src/mbtiles/mod.rs b/martin/src/mbtiles/mod.rs index 2f63019f5..2cd86acfd 100644 --- a/martin/src/mbtiles/mod.rs +++ b/martin/src/mbtiles/mod.rs @@ -13,7 +13,7 @@ use url::Url; use crate::config::UnrecognizedValues; use crate::file_config::FileError::{AcquireConnError, InvalidMetadata, IoError}; -use crate::file_config::{ConfigExtras, FileResult, OnInvalid, SourceConfigExtras, ValidationLevel}; +use crate::file_config::{ConfigExtras, FileResult, SourceConfigExtras, ValidationLevel}; use crate::source::{TileData, TileInfoSource, UrlQuery}; use crate::{MartinError, MartinResult, Source}; @@ -122,7 +122,6 @@ impl Source for MbtSource { .validate_fast() .await .map_err(MartinError::from), - _ => Ok(()), } } diff --git a/martin/src/pg/pg_source.rs b/martin/src/pg/pg_source.rs index c71bcb006..cf77ffcbe 100644 --- a/martin/src/pg/pg_source.rs +++ b/martin/src/pg/pg_source.rs @@ -6,8 +6,8 @@ use martin_tile_utils::Format::Mvt; use martin_tile_utils::{TileCoord, TileInfo}; use tilejson::TileJSON; -use crate::file_config::ValidationLevel; use crate::MartinResult; +use crate::file_config::ValidationLevel; use crate::pg::PgError::{GetTileError, GetTileWithQueryError, PrepareQueryError}; use crate::pg::pool::PgPool; use crate::pg::utils::query_to_json; diff --git a/martin/src/pmtiles/mod.rs b/martin/src/pmtiles/mod.rs index cd946d6b5..7eb06823f 100644 --- a/martin/src/pmtiles/mod.rs +++ b/martin/src/pmtiles/mod.rs @@ -19,7 +19,9 @@ use url::Url; use crate::config::UnrecognizedValues; use crate::file_config::FileError::{InvalidMetadata, InvalidUrlMetadata, IoError}; -use crate::file_config::{ConfigExtras, FileError, FileResult, SourceConfigExtras, ValidationLevel}; +use crate::file_config::{ + ConfigExtras, FileError, FileResult, SourceConfigExtras, ValidationLevel, +}; use crate::source::{TileInfoSource, UrlQuery}; use crate::utils::cache::get_cached_value; use crate::utils::{CacheKey, CacheValue, OptMainCache}; diff --git a/martin/src/source.rs b/martin/src/source.rs index a0fe0ce36..5768a4212 100644 --- a/martin/src/source.rs +++ b/martin/src/source.rs @@ -9,7 +9,7 @@ use martin_tile_utils::{TileCoord, TileInfo}; use serde::{Deserialize, Serialize}; use tilejson::TileJSON; -use crate::{file_config::{OnInvalid, ValidationLevel}, MartinResult}; +use crate::{MartinResult, file_config::ValidationLevel}; pub type TileData = Vec; pub type UrlQuery = HashMap; diff --git a/martin/src/srv/server.rs b/martin/src/srv/server.rs index d442faa19..70eb90d06 100644 --- a/martin/src/srv/server.rs +++ b/martin/src/srv/server.rs @@ -212,8 +212,8 @@ pub mod tests { use tilejson::TileJSON; use super::*; - use crate::file_config::ValidationLevel; use crate::UrlQuery; + use crate::file_config::ValidationLevel; use crate::source::{Source, TileData, TileInfoSource}; #[derive(Debug, Clone)] diff --git a/martin/src/srv/tiles.rs b/martin/src/srv/tiles.rs index d06f677fb..6291c8480 100644 --- a/martin/src/srv/tiles.rs +++ b/martin/src/srv/tiles.rs @@ -357,10 +357,7 @@ mod tests { tj: tilejson! { tiles: vec![] }, data: Vec::default(), }; - let sources = TileSources::new(vec![ - Box::new(non_empty_source), - Box::new(empty_source), - ]); + let sources = TileSources::new(vec![Box::new(non_empty_source), Box::new(empty_source)]); for (source_id, expected) in &[ ("non-empty", vec![1_u8, 2, 3]), From bb33bf00fb3937f1ebc5c70bacb78933bbc71aa0 Mon Sep 17 00:00:00 2001 From: David Disch Date: Sat, 12 Apr 2025 01:27:18 +1000 Subject: [PATCH 18/19] Allow per source level config --- martin/benches/bench.rs | 10 +++++++- martin/src/cog/config.rs | 20 ++++++++++++--- martin/src/cog/source.rs | 14 ++++++++++- martin/src/config.rs | 6 +++-- martin/src/file_config.rs | 28 ++++++++++++++++++--- martin/src/mbtiles/mod.rs | 49 ++++++++++++++++++++++++++---------- martin/src/pg/pg_source.rs | 14 ++++++++++- martin/src/pmtiles/mod.rs | 30 +++++++++++++++++++--- martin/src/source.rs | 9 ++++--- martin/src/srv/server.rs | 12 ++++++++- martin/src/srv/tiles.rs | 6 +++++ martin/src/srv/tiles_info.rs | 4 +++ 12 files changed, 170 insertions(+), 32 deletions(-) diff --git a/martin/benches/bench.rs b/martin/benches/bench.rs index 54d39abf6..b7bd79779 100644 --- a/martin/benches/bench.rs +++ b/martin/benches/bench.rs @@ -1,7 +1,7 @@ use async_trait::async_trait; use criterion::async_executor::FuturesExecutor; use criterion::{Criterion, criterion_group, criterion_main}; -use martin::file_config::ValidationLevel; +use martin::file_config::{OnInvalid, ValidationLevel}; use martin::srv::DynTileSource; use martin::{CatalogSourceEntry, MartinResult, Source, TileData, TileSources, UrlQuery}; use martin_tile_utils::{Encoding, Format, TileCoord, TileInfo}; @@ -43,6 +43,14 @@ impl Source for NullSource { false } + fn get_validation_level(&self) -> Option { + None + } + + fn get_on_invalid(&self) -> Option { + None + } + async fn validate(&self, _validation_level: ValidationLevel) -> MartinResult<()> { MartinResult::Ok(()) } diff --git a/martin/src/cog/config.rs b/martin/src/cog/config.rs index f636c17d0..76b852c4c 100644 --- a/martin/src/cog/config.rs +++ b/martin/src/cog/config.rs @@ -7,7 +7,9 @@ use url::Url; use super::source::CogSource; use crate::Source; use crate::config::UnrecognizedValues; -use crate::file_config::{ConfigExtras, FileResult, SourceConfigExtras}; +use crate::file_config::{ + ConfigExtras, FileResult, OnInvalid, SourceConfigExtras, ValidationLevel, +}; #[derive(Clone, Debug, Default, PartialEq, Serialize, Deserialize)] pub struct CogConfig { @@ -22,13 +24,25 @@ impl ConfigExtras for CogConfig { } impl SourceConfigExtras for CogConfig { - async fn new_sources(&self, id: String, path: PathBuf) -> FileResult> { + async fn new_sources( + &self, + id: String, + path: PathBuf, + _validation_level: Option, + _on_invalid: Option, + ) -> FileResult> { let cog = CogSource::new(id, path)?; Ok(Box::new(cog)) } #[allow(clippy::no_effect_underscore_binding)] - async fn new_sources_url(&self, _id: String, _url: Url) -> FileResult> { + async fn new_sources_url( + &self, + _id: String, + _url: Url, + _validation_level: Option, + _on_invalid: Option, + ) -> FileResult> { unreachable!() } diff --git a/martin/src/cog/source.rs b/martin/src/cog/source.rs index 38f948884..c873d41a6 100644 --- a/martin/src/cog/source.rs +++ b/martin/src/cog/source.rs @@ -13,7 +13,7 @@ use tiff::tags::Tag::{self, GdalNodata}; use tilejson::{TileJSON, tilejson}; use super::CogError; -use crate::file_config::{FileError, FileResult, ValidationLevel}; +use crate::file_config::{FileError, FileResult, OnInvalid, ValidationLevel}; use crate::{MartinResult, Source, TileData, UrlQuery}; #[derive(Clone, Debug)] @@ -32,6 +32,8 @@ pub struct CogSource { meta: Meta, tilejson: TileJSON, tileinfo: TileInfo, + validation_level: Option, + on_invalid: Option, } impl CogSource { @@ -49,6 +51,8 @@ impl CogSource { meta, tilejson, tileinfo, + validation_level: None, + on_invalid: None, }) } #[allow(clippy::cast_sign_loss)] @@ -151,6 +155,14 @@ impl Source for CogSource { Box::new(self.clone()) } + fn get_validation_level(&self) -> Option { + self.validation_level + } + + fn get_on_invalid(&self) -> Option { + self.on_invalid + } + async fn validate(&self, _validation_level: ValidationLevel) -> MartinResult<()> { MartinResult::Ok(()) } diff --git a/martin/src/config.rs b/martin/src/config.rs index 886763734..8a0d6d395 100644 --- a/martin/src/config.rs +++ b/martin/src/config.rs @@ -218,9 +218,11 @@ impl Config { let mut sources_to_prune: Vec = vec![]; for (idx, source) in resolved_sources.iter().enumerate() { - let validation_result = source.validate(self.srv.validate).await; + let validation_result = source + .validate(source.get_validation_level().unwrap_or(self.srv.validate)) + .await; if let Err(e) = validation_result { - match self.srv.on_invalid { + match source.get_on_invalid().unwrap_or(self.srv.on_invalid) { OnInvalid::Abort => return MartinResult::Err(e), OnInvalid::Warn => { warn!( diff --git a/martin/src/file_config.rs b/martin/src/file_config.rs index e54defab4..ad651f4c1 100644 --- a/martin/src/file_config.rs +++ b/martin/src/file_config.rs @@ -110,12 +110,16 @@ pub trait SourceConfigExtras: ConfigExtras { &self, id: String, path: PathBuf, + validation_level: Option, + on_invalid: Option, ) -> impl Future> + Send; fn new_sources_url( &self, id: String, url: Url, + validation_level: Option, + on_invalid: Option, ) -> impl Future> + Send; } @@ -306,7 +310,11 @@ async fn resolve_int( let dup = if dup { "duplicate " } else { "" }; let id = idr.resolve(&id, url.to_string()); configs.insert(id.clone(), source); - results.push(cfg.custom.new_sources_url(id.clone(), url.clone()).await?); + results.push( + cfg.custom + .new_sources_url(id.clone(), url.clone(), cfg.validate, cfg.on_invalid) + .await?, + ); info!("Configured {dup}source {id} from {}", sanitize_url(&url)); } else { let can = source.abs_path()?; @@ -320,7 +328,11 @@ async fn resolve_int( let id = idr.resolve(&id, can.to_string_lossy().to_string()); info!("Configured {dup}source {id} from {}", can.display()); configs.insert(id.clone(), source.clone()); - results.push(cfg.custom.new_sources(id, source.into_path()).await?); + results.push( + cfg.custom + .new_sources(id, source.into_path(), cfg.validate, cfg.on_invalid) + .await?, + ); } } } @@ -344,7 +356,11 @@ async fn resolve_int( let id = idr.resolve(id, url.to_string()); configs.insert(id.clone(), FileConfigSrc::Path(path)); - results.push(cfg.custom.new_sources_url(id.clone(), url.clone()).await?); + results.push( + cfg.custom + .new_sources_url(id.clone(), url.clone(), cfg.validate, cfg.on_invalid) + .await?, + ); info!("Configured source {id} from URL {}", sanitize_url(&url)); } else { let is_dir = path.is_dir(); @@ -373,7 +389,11 @@ async fn resolve_int( info!("Configured source {id} from {}", can.display()); files.insert(can); configs.insert(id.clone(), FileConfigSrc::Path(path.clone())); - results.push(cfg.custom.new_sources(id, path).await?); + results.push( + cfg.custom + .new_sources(id, path, cfg.validate, cfg.on_invalid) + .await?, + ); } } } diff --git a/martin/src/mbtiles/mod.rs b/martin/src/mbtiles/mod.rs index 2cd86acfd..4e5c2525b 100644 --- a/martin/src/mbtiles/mod.rs +++ b/martin/src/mbtiles/mod.rs @@ -13,7 +13,9 @@ use url::Url; use crate::config::UnrecognizedValues; use crate::file_config::FileError::{AcquireConnError, InvalidMetadata, IoError}; -use crate::file_config::{ConfigExtras, FileResult, SourceConfigExtras, ValidationLevel}; +use crate::file_config::{ + ConfigExtras, FileResult, OnInvalid, SourceConfigExtras, ValidationLevel, +}; use crate::source::{TileData, TileInfoSource, UrlQuery}; use crate::{MartinError, MartinResult, Source}; @@ -30,14 +32,28 @@ impl ConfigExtras for MbtConfig { } impl SourceConfigExtras for MbtConfig { - async fn new_sources(&self, id: String, path: PathBuf) -> FileResult { - Ok(Box::new(MbtSource::new(id, path.clone()).await?)) + async fn new_sources( + &self, + id: String, + path: PathBuf, + validation_level: Option, + on_invalid: Option, + ) -> FileResult { + Ok(Box::new( + MbtSource::new(id, path.clone(), validation_level, on_invalid).await?, + )) } // TODO: Remove #[allow] after switching to Rust/Clippy v1.78+ in CI // See https://github.com/rust-lang/rust-clippy/pull/12323 #[allow(clippy::no_effect_underscore_binding)] - async fn new_sources_url(&self, _id: String, _url: Url) -> FileResult { + async fn new_sources_url( + &self, + _id: String, + _url: Url, + _validation_level: Option, + _on_invalidd: Option, + ) -> FileResult { unreachable!() } } @@ -48,8 +64,8 @@ pub struct MbtSource { mbtiles: Arc, tilejson: TileJSON, tile_info: TileInfo, - // validation_level: ValidationLevel, - // on_invalid: OnInvalid + validation_level: Option, + on_invalid: Option, } impl Debug for MbtSource { @@ -64,7 +80,12 @@ impl Debug for MbtSource { } impl MbtSource { - async fn new(id: String, path: PathBuf) -> FileResult { + async fn new( + id: String, + path: PathBuf, + validation_level: Option, + on_invalid: Option, + ) -> FileResult { let mbt = MbtilesPool::new(&path) .await .map_err(|e| io::Error::other(format!("{e:?}: Cannot open file {}", path.display()))) @@ -80,6 +101,8 @@ impl MbtSource { mbtiles: Arc::new(mbt), tilejson: meta.tilejson, tile_info: meta.tile_info, + validation_level, + on_invalid, }) } } @@ -102,13 +125,13 @@ impl Source for MbtSource { Box::new(self.clone()) } - // fn get_validation_level(&self) -> ValidationLevel { - // self.validation_level - // } + fn get_validation_level(&self) -> Option { + self.validation_level + } - // fn get_on_invalid(&self) -> OnInvalid { - // self.on_invalid - // } + fn get_on_invalid(&self) -> Option { + self.on_invalid + } async fn validate(&self, validation_level: ValidationLevel) -> MartinResult<()> { match validation_level { diff --git a/martin/src/pg/pg_source.rs b/martin/src/pg/pg_source.rs index cf77ffcbe..bb355a0fe 100644 --- a/martin/src/pg/pg_source.rs +++ b/martin/src/pg/pg_source.rs @@ -7,7 +7,7 @@ use martin_tile_utils::{TileCoord, TileInfo}; use tilejson::TileJSON; use crate::MartinResult; -use crate::file_config::ValidationLevel; +use crate::file_config::{OnInvalid, ValidationLevel}; use crate::pg::PgError::{GetTileError, GetTileWithQueryError, PrepareQueryError}; use crate::pg::pool::PgPool; use crate::pg::utils::query_to_json; @@ -19,6 +19,8 @@ pub struct PgSource { info: PgSqlInfo, pool: PgPool, tilejson: TileJSON, + validation_level: Option, + on_invalid: Option, } impl PgSource { @@ -29,6 +31,8 @@ impl PgSource { info, pool, tilejson, + validation_level: None, + on_invalid: None, } } } @@ -55,6 +59,14 @@ impl Source for PgSource { self.info.use_url_query } + fn get_validation_level(&self) -> Option { + self.validation_level + } + + fn get_on_invalid(&self) -> Option { + self.on_invalid + } + async fn validate(&self, _validation_level: ValidationLevel) -> MartinResult<()> { MartinResult::Ok(()) } diff --git a/martin/src/pmtiles/mod.rs b/martin/src/pmtiles/mod.rs index 7eb06823f..7bd03b64c 100644 --- a/martin/src/pmtiles/mod.rs +++ b/martin/src/pmtiles/mod.rs @@ -20,7 +20,7 @@ use url::Url; use crate::config::UnrecognizedValues; use crate::file_config::FileError::{InvalidMetadata, InvalidUrlMetadata, IoError}; use crate::file_config::{ - ConfigExtras, FileError, FileResult, SourceConfigExtras, ValidationLevel, + ConfigExtras, FileError, FileResult, OnInvalid, SourceConfigExtras, ValidationLevel, }; use crate::source::{TileInfoSource, UrlQuery}; use crate::utils::cache::get_cached_value; @@ -138,13 +138,25 @@ impl SourceConfigExtras for PmtConfig { true } - async fn new_sources(&self, id: String, path: PathBuf) -> FileResult { + async fn new_sources( + &self, + id: String, + path: PathBuf, + _validation_level: Option, + _on_invalidd: Option, + ) -> FileResult { Ok(Box::new( PmtFileSource::new(self.new_cached_source(), id, path).await?, )) } - async fn new_sources_url(&self, id: String, url: Url) -> FileResult { + async fn new_sources_url( + &self, + id: String, + url: Url, + _validation_level: Option, + _on_invalidd: Option, + ) -> FileResult { Ok(Box::new( PmtHttpSource::new( self.client.clone().unwrap(), @@ -166,6 +178,8 @@ macro_rules! impl_pmtiles_source { pmtiles: Arc>, tilejson: TileJSON, tile_info: TileInfo, + validation_level: Option, + on_invalid: Option, } impl Debug for $name { @@ -236,6 +250,8 @@ macro_rules! impl_pmtiles_source { pmtiles: Arc::new(reader), tilejson, tile_info: format, + validation_level: None, + on_invalid: None, }) } } @@ -258,6 +274,14 @@ macro_rules! impl_pmtiles_source { Box::new(self.clone()) } + fn get_validation_level(&self) -> Option { + self.validation_level + } + + fn get_on_invalid(&self) -> Option { + self.on_invalid + } + async fn validate(&self, _validation_level: ValidationLevel) -> MartinResult<()> { MartinResult::Ok(()) } diff --git a/martin/src/source.rs b/martin/src/source.rs index 5768a4212..10d4cb4cf 100644 --- a/martin/src/source.rs +++ b/martin/src/source.rs @@ -9,7 +9,10 @@ use martin_tile_utils::{TileCoord, TileInfo}; use serde::{Deserialize, Serialize}; use tilejson::TileJSON; -use crate::{MartinResult, file_config::ValidationLevel}; +use crate::{ + MartinResult, + file_config::{OnInvalid, ValidationLevel}, +}; pub type TileData = Vec; pub type UrlQuery = HashMap; @@ -114,9 +117,9 @@ pub trait Source: Send + Debug { false } - // fn get_validation_level(&self) -> ValidationLevel; + fn get_validation_level(&self) -> Option; - // fn get_on_invalid(&self) -> OnInvalid; + fn get_on_invalid(&self) -> Option; async fn validate(&self, validation_level: ValidationLevel) -> MartinResult<()>; diff --git a/martin/src/srv/server.rs b/martin/src/srv/server.rs index 70eb90d06..adaab7f98 100644 --- a/martin/src/srv/server.rs +++ b/martin/src/srv/server.rs @@ -213,7 +213,7 @@ pub mod tests { use super::*; use crate::UrlQuery; - use crate::file_config::ValidationLevel; + use crate::file_config::{OnInvalid, ValidationLevel}; use crate::source::{Source, TileData, TileInfoSource}; #[derive(Debug, Clone)] @@ -221,6 +221,8 @@ pub mod tests { pub id: &'static str, pub tj: TileJSON, pub data: TileData, + pub validation_level: Option, + pub on_invalid: Option, } #[async_trait] @@ -241,6 +243,14 @@ pub mod tests { Box::new(self.clone()) } + fn get_validation_level(&self) -> Option { + self.validation_level + } + + fn get_on_invalid(&self) -> Option { + self.on_invalid + } + async fn validate(&self, _validation_level: ValidationLevel) -> MartinResult<()> { MartinResult::Ok(()) } diff --git a/martin/src/srv/tiles.rs b/martin/src/srv/tiles.rs index 6291c8480..04a95cf41 100644 --- a/martin/src/srv/tiles.rs +++ b/martin/src/srv/tiles.rs @@ -323,6 +323,8 @@ mod tests { id: "test_source", tj: tilejson! { tiles: vec![] }, data: vec![1_u8, 2, 3], + validation_level: None, + on_invalid: None, })]); let accept_enc = Some(AcceptEncoding( @@ -351,11 +353,15 @@ mod tests { id: "non-empty", tj: tilejson! { tiles: vec![] }, data: vec![1_u8, 2, 3], + validation_level: None, + on_invalid: None, }; let empty_source = TestSource { id: "empty", tj: tilejson! { tiles: vec![] }, data: Vec::default(), + validation_level: None, + on_invalid: None, }; let sources = TileSources::new(vec![Box::new(non_empty_source), Box::new(empty_source)]); diff --git a/martin/src/srv/tiles_info.rs b/martin/src/srv/tiles_info.rs index 9d0d97ae9..acf55bdbd 100644 --- a/martin/src/srv/tiles_info.rs +++ b/martin/src/srv/tiles_info.rs @@ -181,6 +181,8 @@ pub mod tests { ])) ], }, + on_invalid: None, + validation_level: None, data: Vec::default(), }; let tj = merge_tilejson(&[Box::new(src1.clone())], url.clone()); @@ -208,6 +210,8 @@ pub mod tests { ], }, data: Vec::default(), + validation_level: None, + on_invalid: None, }; let tj = merge_tilejson(&[Box::new(src1.clone()), Box::new(src2)], url.clone()); From 97880c51f59cbb4a9f70fbbc34a9e6ac64ffc52a Mon Sep 17 00:00:00 2001 From: David Disch Date: Sat, 12 Apr 2025 15:56:36 +1000 Subject: [PATCH 19/19] Add back missing feature flagged args, ignore sources marked for pruning --- martin/src/args/root.rs | 5 +++++ martin/src/config.rs | 15 +++++++++++---- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/martin/src/args/root.rs b/martin/src/args/root.rs index 6c58d493c..1cac7a862 100644 --- a/martin/src/args/root.rs +++ b/martin/src/args/root.rs @@ -115,6 +115,11 @@ impl Args { config.pmtiles = parse_file_args(&mut cli_strings, &["pmtiles"], true); } + #[cfg(feature = "mbtiles")] + if !cli_strings.is_empty() { + config.mbtiles = parse_file_args(&mut cli_strings, &["mbtiles"], false); + } + #[cfg(feature = "cog")] if !cli_strings.is_empty() { config.cog = parse_file_args(&mut cli_strings, &["tif", "tiff"], false); diff --git a/martin/src/config.rs b/martin/src/config.rs index 8a0d6d395..70fe9499e 100644 --- a/martin/src/config.rs +++ b/martin/src/config.rs @@ -1,4 +1,4 @@ -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::ffi::OsStr; use std::fs::File; use std::io::prelude::*; @@ -216,7 +216,7 @@ impl Config { .flatten() .collect::(); - let mut sources_to_prune: Vec = vec![]; + let mut sources_to_prune: HashSet = HashSet::new(); for (idx, source) in resolved_sources.iter().enumerate() { let validation_result = source .validate(source.get_validation_level().unwrap_or(self.srv.validate)) @@ -232,13 +232,20 @@ impl Config { ); } OnInvalid::Ignore => { - sources_to_prune.push(idx); + sources_to_prune.insert(idx); } } } } - Ok(TileSources::new(resolved_sources)) + Ok(TileSources::new( + resolved_sources + .into_iter() + .enumerate() + .filter(|e| !sources_to_prune.contains(&e.0)) + .map(|(_, s)| s) + .collect(), + )) } pub fn save_to_file(&self, file_name: PathBuf) -> MartinResult<()> {