mbtiles improvements, minor changes (#1013)

# MBTiles
* New `--agg-hash (update|verify|off)` flag replaces
`--update-agg-tiles-hash` (still supported, but not shown in the help
screen). This allows bypassing aggregate hash validation entirely,
without either updating or validating it.
* Simplify MBTiles SQL generation
* MBTiles now uses faster `1 << zoom` everywhere, and a dedicated TMS
inversion fn
* split up metadata insert and delete into separate fn
* consolidated schema initialization
* ensure db settings (like pragma) are always reset on new files

# Other
* Always sort JSON-serialized keys for consistency
  * this affects `/catalog` key ordering, but content is the same
* Minor code cleanup

.github/workflows/ci.yml

@@ -177,10 +177,9 @@ jobs:
         run: |
           PLATFORM=linux/arm64
           TAG=${{ github.repository }}:linux-arm64
-          export MBTILES_BUILD=-
-          export MBTILES_BIN="docker run --rm --net host --platform $PLATFORM -e DATABASE_URL -v $PWD/tests:/tests --entrypoint /usr/local/bin/mbtiles $TAG"
-          export MARTIN_BUILD=-
+          export MARTIN_BUILD_ALL=-
           export MARTIN_BIN="docker run --rm --net host --platform $PLATFORM -e DATABASE_URL -v $PWD/tests:/tests $TAG"
+          export MBTILES_BIN="docker run --rm --net host --platform $PLATFORM -e DATABASE_URL -v $PWD/tests:/tests --entrypoint /usr/local/bin/mbtiles $TAG"
           tests/test.sh
         env:
           DATABASE_URL: postgres://${{ env.PGUSER }}:${{ env.PGUSER }}@${{ env.PGHOST }}:${{ job.services.postgres.ports[5432] }}/${{ env.PGDATABASE }}?sslmode=require
@@ -198,10 +197,9 @@ jobs:
         run: |
           PLATFORM=linux/amd64
           TAG=${{ github.repository }}:linux-amd64
-          export MBTILES_BUILD=-
-          export MBTILES_BIN="docker run --rm --net host --platform $PLATFORM -e DATABASE_URL -v $PWD/tests:/tests --entrypoint /usr/local/bin/mbtiles $TAG"
-          export MARTIN_BUILD=-
+          export MARTIN_BUILD_ALL=-
           export MARTIN_BIN="docker run --rm --net host --platform $PLATFORM -e DATABASE_URL -v $PWD/tests:/tests $TAG"
+          export MBTILES_BIN="docker run --rm --net host --platform $PLATFORM -e DATABASE_URL -v $PWD/tests:/tests --entrypoint /usr/local/bin/mbtiles $TAG"
           tests/test.sh
         env:
           DATABASE_URL: postgres://${{ env.PGUSER }}:${{ env.PGUSER }}@${{ env.PGHOST }}:${{ job.services.postgres.ports[5432] }}/${{ env.PGDATABASE }}?sslmode=require
@@ -328,9 +326,8 @@ jobs:
           path: target/
       - name: Integration Tests
         run: |
-          export MARTIN_BUILD=-
+          export MARTIN_BUILD_ALL=-
           export MARTIN_BIN=target/martin${{ matrix.ext }}
-          export MBTILES_BUILD=-
           export MBTILES_BIN=target/mbtiles${{ matrix.ext }}
           if [[ "${{ runner.os }}" != "Windows" ]]; then
             chmod +x "$MARTIN_BIN" "$MBTILES_BIN"
@@ -351,9 +348,8 @@ jobs:
         if: matrix.target == 'x86_64-unknown-linux-gnu'
         run: |
           sudo dpkg -i target/debian-x86_64.deb
-          export MARTIN_BUILD=-
+          export MARTIN_BUILD_ALL=-
           export MARTIN_BIN=/usr/bin/martin${{ matrix.ext }}
-          export MBTILES_BUILD=-
           export MBTILES_BIN=/usr/bin/mbtiles${{ matrix.ext }}
           tests/test.sh
         env:
@@ -446,9 +442,8 @@ jobs:
           if [[ "${{ matrix.sslmode }}" == "verify-ca" || "${{ matrix.sslmode }}" == "verify-full" ]]; then
             export PGSSLROOTCERT=target/certs/server.crt
           fi
-          export MARTIN_BUILD=-
+          export MARTIN_BUILD_ALL=-
           export MARTIN_BIN=target_releases/martin
-          export MBTILES_BUILD=-
           export MBTILES_BIN=target_releases/mbtiles
           chmod +x "$MARTIN_BIN" "$MBTILES_BIN"
           tests/test.sh
@@ -466,9 +461,8 @@ jobs:
           if [[ "${{ matrix.sslmode }}" == "verify-ca" || "${{ matrix.sslmode }}" == "verify-full" ]]; then
             export PGSSLROOTCERT=target/certs/server.crt
           fi
-          export MARTIN_BUILD=-
+          export MARTIN_BUILD_ALL=-
           export MARTIN_BIN=/usr/bin/martin
-          export MBTILES_BUILD=-
           export MBTILES_BIN=/usr/bin/mbtiles
           tests/test.sh
           sudo dpkg -P martin

Cargo.toml

@@ -11,6 +11,7 @@ readme = "README.md"
 homepage = "https://martin.maplibre.org/"
 
 [workspace.lints.rust]
+unused_qualifications = "warn"
 # Lints inheritance from workspace is not yet supported
 # Crate mbtiles uses unsafe code, so we can't forbid it here
 # unsafe_code = "forbid"
@@ -84,5 +85,6 @@ insta.opt-level = 3
 similar.opt-level = 3
 
 #[patch.crates-io]
-#sqlite-hashes = { path = "/home/nyurik/dev/rust/sqlite-hashes" }
 #pmtiles = { path = "../pmtiles-rs" }
+#sqlite-hashes = { path = "/home/nyurik/dev/rust/sqlite-hashes" }
+#tilejson = { path = "../tilejson" }

docs/src/21-run-with-cli.md

@@ -46,7 +46,7 @@ Options:
           If a spatial PG table has SRID 0, then this default SRID will be used as a fallback
 
   -p, --pool-size <POOL_SIZE>
-          Maximum connections pool size [DEFAULT: 20]
+          Maximum Postgres connections pool size [DEFAULT: 20]
 
   -m, --max-feature-count <MAX_FEATURE_COUNT>
           Limit the number of features in a tile from a PG table source

docs/src/30-config-file.md

@@ -41,7 +41,7 @@ postgres:
   #  If a spatial table has SRID 0, then this SRID will be used as a fallback
   default_srid: 4326
 
-  # Maximum connections pool size [default: 20]
+  # Maximum Postgres connections pool size [default: 20]
   pool_size: 20
 
   # Limit the number of table geo features included in a tile. Unlimited by default.

docs/src/53-mbtiles-validation.md

@@ -35,4 +35,4 @@ md5_concat_hex(
 
 In case there are no rows or all are NULL, the hash value of an empty string is used. Note that SQLite allows any value type to be stored as in any column, so if `tile_data` accidentally contains non-blob/text/null value, validation will fail.
 
-The `mbtiles` tool will compute `agg_tiles_hash` value when copying or validating mbtiles files. Use `--update-agg-tiles-hash` to force the value to be updated, even if it is incorrect or does not exist.
+The `mbtiles` tool will compute `agg_tiles_hash` value when copying or validating mbtiles files. Use `--agg-hash update` to force the value to be updated, even if it is incorrect or does not exist.

martin/Cargo.toml

@@ -83,7 +83,7 @@ rustls-pemfile.workspace = true
 rustls.workspace = true
 semver.workspace = true
 serde.workspace = true
-serde_json = { workspace = true, features = ["preserve_order"] }
+serde_json.workspace = true
 serde_with.workspace = true
 serde_yaml.workspace = true
 spreet.workspace = true

martin/src/args/environment.rs

@@ -40,6 +40,7 @@ pub struct OsEnv(RefCell<HashSet<String>>);
 
 impl<'a> Env<'a> for OsEnv {
     fn var_os(&self, key: &str) -> Option<OsString> {
+        #[allow(unused_qualifications)]
         std::env::var_os(key)
     }
 

martin/src/args/mod.rs

@@ -1,10 +1,14 @@
 mod connections;
-mod environment;
-mod pg;
-mod root;
-mod srv;
-
 pub use connections::{Arguments, State};
+
+mod environment;
 pub use environment::{Env, OsEnv};
+
+mod pg;
 pub use pg::{BoundsCalcType, PgArgs, DEFAULT_BOUNDS_TIMEOUT};
+
+mod root;
 pub use root::{Args, ExtraArgs, MetaArgs};
+
+mod srv;
+pub use srv::SrvArgs;

martin/src/args/pg.rs

@@ -37,7 +37,7 @@ pub struct PgArgs {
     /// If a spatial PG table has SRID 0, then this default SRID will be used as a fallback.
     #[arg(short, long)]
     pub default_srid: Option<i32>,
-    #[arg(help = format!("Maximum connections pool size [DEFAULT: {}]", POOL_SIZE_DEFAULT), short, long)]
+    #[arg(help = format!("Maximum Postgres connections pool size [DEFAULT: {}]", POOL_SIZE_DEFAULT), short, long)]
     pub pool_size: Option<usize>,
     /// Limit the number of features in a tile from a PG table source.
     #[arg(short, long)]

martin/src/fonts/mod.rs

@@ -47,7 +47,7 @@ pub enum FontError {
     #[error("Given font range {0}-{1} is invalid. It must be {CP_RANGE_SIZE} characters long (e.g. 0-255, 256-511, ...)")]
     InvalidFontRange(u32, u32),
 
-    #[error("FreeType font error: {0}")]
+    #[error(transparent)]
     FreeType(#[from] pbf_font_tools::freetype::Error),
 
     #[error("IO error accessing {}: {0}", .1.display())]
@@ -62,10 +62,10 @@ pub enum FontError {
     #[error("Font {0} is missing a family name")]
     MissingFamilyName(PathBuf),
 
-    #[error("PBF Font error: {0}")]
+    #[error(transparent)]
     PbfFontError(#[from] PbfFontError),
 
-    #[error("Error serializing protobuf: {0}")]
+    #[error(transparent)]
     ErrorSerializingProtobuf(#[from] pbf_font_tools::protobuf::Error),
 }
 

martin/src/lib.rs

@@ -1,18 +1,25 @@
 #![doc = include_str!("../README.md")]
 #![forbid(unsafe_code)]
 
-pub mod args;
 mod config;
+pub use config::{read_config, Config, ServerState};
+
+mod source;
+pub use source::Source;
+
+mod utils;
+pub use utils::{
+    decode_brotli, decode_gzip, Error, IdResolver, OptBoolObj, OptOneMany, Result, Xyz,
+};
+
+pub mod args;
 pub mod file_config;
 pub mod fonts;
 pub mod mbtiles;
 pub mod pg;
 pub mod pmtiles;
-mod source;
 pub mod sprites;
 pub mod srv;
-mod utils;
-pub use utils::Xyz;
 
 #[cfg(test)]
 #[path = "utils/test_utils.rs"]
@@ -22,11 +29,6 @@ mod test_utils;
 // Must make it accessible as carte::Env from both places when testing.
 #[cfg(test)]
 pub use crate::args::Env;
-pub use crate::config::{read_config, Config, ServerState};
-pub use crate::source::Source;
-pub use crate::utils::{
-    decode_brotli, decode_gzip, Error, IdResolver, OptBoolObj, OptOneMany, Result,
-};
 
 // Ensure README.md contains valid code
 #[cfg(doctest)]

martin/src/pg/errors.rs

@@ -28,7 +28,7 @@ pub enum PgError {
     #[error("Unable to use client certificate pair {} / {}: {0}", .1.display(), .2.display())]
     CannotUseClientKey(#[source] rustls::Error, PathBuf, PathBuf),
 
-    #[error("Rustls Error: {0:?}")]
+    #[error(transparent)]
     RustlsError(#[from] rustls::Error),
 
     #[error("Unknown SSL mode: {0:?}")]

martin/src/srv/mod.rs

@@ -2,4 +2,6 @@ mod config;
 pub use config::{SrvConfig, KEEP_ALIVE_DEFAULT, LISTEN_ADDRESSES_DEFAULT};
 
 mod server;
-pub use server::{get_composite_tile, new_server, router, Catalog, RESERVED_KEYWORDS};
+pub use server::{
+    get_composite_tile, merge_tilejson, new_server, router, Catalog, RESERVED_KEYWORDS,
+};

martin/src/srv/server.rs

@@ -202,7 +202,7 @@ async fn git_source_info(
     let tiles_path = get_request_path(&req);
     let tiles_url = get_tiles_url(info.scheme(), info.host(), req.query_string(), &tiles_path)?;
 
-    Ok(HttpResponse::Ok().json(merge_tilejson(sources, tiles_url)))
+    Ok(HttpResponse::Ok().json(merge_tilejson(&sources, tiles_url)))
 }
 
 fn get_request_path(req: &HttpRequest) -> String {
@@ -228,7 +228,8 @@ fn get_tiles_url(scheme: &str, host: &str, query_string: &str, tiles_path: &str)
         .map_err(|e| ErrorBadRequest(format!("Can't build tiles URL: {e}")))
 }
 
-fn merge_tilejson(sources: Vec<&dyn Source>, tiles_url: String) -> TileJSON {
+#[must_use]
+pub fn merge_tilejson(sources: &[&dyn Source], tiles_url: String) -> TileJSON {
     if sources.len() == 1 {
         let mut tj = sources[0].get_tilejson().clone();
         tj.tiles = vec![tiles_url];
@@ -547,7 +548,6 @@ mod tests {
 
     use super::*;
     use crate::source::{Source, Tile};
-    use crate::utils;
 
     #[derive(Debug, Clone)]
     struct TestSource {
@@ -572,11 +572,7 @@ mod tests {
             unimplemented!()
         }
 
-        async fn get_tile(
-            &self,
-            _xyz: &Xyz,
-            _url_query: &Option<UrlQuery>,
-        ) -> Result<Tile, utils::Error> {
+        async fn get_tile(&self, _xyz: &Xyz, _url_query: &Option<UrlQuery>) -> Result<Tile, Error> {
             unimplemented!()
         }
     }
@@ -599,7 +595,7 @@ mod tests {
                 ],
             },
         };
-        let tj = merge_tilejson(vec![&src1], url.clone());
+        let tj = merge_tilejson(&[&src1], url.clone());
         assert_eq!(
             TileJSON {
                 tiles: vec![url.clone()],
@@ -624,7 +620,7 @@ mod tests {
             },
         };
 
-        let tj = merge_tilejson(vec![&src1, &src2], url.clone());
+        let tj = merge_tilejson(&[&src1, &src2], url.clone());
         assert_eq!(tj.tiles, vec![url]);
         assert_eq!(tj.name, Some("layer1,layer2".to_string()));
         assert_eq!(tj.minzoom, Some(5));

martin/tests/mb_server_test.rs

@@ -52,24 +52,24 @@ async fn mbt_get_catalog() {
     let body: serde_json::Value = read_body_json(response).await;
     assert_yaml_snapshot!(body, @r###"
     ---
+    fonts: {}
+    sprites: {}
     tiles:
       m_json:
         content_type: application/json
         name: Dummy json data
       m_mvt:
-        content_type: application/x-protobuf
         content_encoding: gzip
-        name: Major cities from Natural Earth data
+        content_type: application/x-protobuf
         description: Major cities from Natural Earth data
+        name: Major cities from Natural Earth data
       m_raw_mvt:
         content_type: application/x-protobuf
-        name: Major cities from Natural Earth data
         description: Major cities from Natural Earth data
+        name: Major cities from Natural Earth data
       m_webp:
         content_type: image/webp
         name: ne2sr
-    sprites: {}
-    fonts: {}
     "###);
 }
 
@@ -84,24 +84,24 @@ async fn mbt_get_catalog_gzip() {
     let body: serde_json::Value = serde_json::from_slice(&body).unwrap();
     assert_yaml_snapshot!(body, @r###"
     ---
+    fonts: {}
+    sprites: {}
     tiles:
       m_json:
         content_type: application/json
         name: Dummy json data
       m_mvt:
-        content_type: application/x-protobuf
         content_encoding: gzip
-        name: Major cities from Natural Earth data
+        content_type: application/x-protobuf
         description: Major cities from Natural Earth data
+        name: Major cities from Natural Earth data
       m_raw_mvt:
         content_type: application/x-protobuf
-        name: Major cities from Natural Earth data
         description: Major cities from Natural Earth data
+        name: Major cities from Natural Earth data
       m_webp:
         content_type: image/webp
         name: ne2sr
-    sprites: {}
-    fonts: {}
     "###);
 }
 
@@ -245,7 +245,7 @@ async fn mbt_get_raw_mvt_gzip() {
     assert_eq!(response.headers().get(CONTENT_ENCODING).unwrap(), "gzip");
     let body = read_body(response).await;
     assert_eq!(body.len(), 1107); // this number could change if compression gets more optimized
-    let body = martin::decode_gzip(&body).unwrap();
+    let body = decode_gzip(&body).unwrap();
     assert_eq!(body.len(), 1828);
 }
 
@@ -302,6 +302,6 @@ async fn mbt_get_json_gzip() {
     assert_eq!(response.headers().get(CONTENT_ENCODING).unwrap(), "gzip");
     let body = read_body(response).await;
     assert_eq!(body.len(), 33); // this number could change if compression gets more optimized
-    let body = martin::decode_gzip(&body).unwrap();
+    let body = decode_gzip(&body).unwrap();
     assert_eq!(body.len(), 13);
 }

martin/tests/pg_server_test.rs

@@ -49,6 +49,8 @@ postgres:
     let body: serde_json::Value = serde_json::from_slice(&body).unwrap();
     assert_yaml_snapshot!(body, @r###"
     ---
+    fonts: {}
+    sprites: {}
     tiles:
       MixPoints:
         content_type: application/x-protobuf
@@ -114,8 +116,6 @@ postgres:
       table_source_multiple_geom.1:
         content_type: application/x-protobuf
         description: public.table_source_multiple_geom.geom2
-    sprites: {}
-    fonts: {}
     "###);
 }