From 226014b1291c89956219a6c5142f7d2689d47f39 Mon Sep 17 00:00:00 2001 From: Yuri Astrakhan Date: Tue, 29 Nov 2022 19:42:14 -0500 Subject: [PATCH 01/16] Require PostGIS 3 and use ST_TileEnvelope * All tests and internal code now uses ST_TileEnvelope function * Remove `tile_bbox` * Rename test function sources for clarity - this will be needed in a subsequent PR to add other function tests --- tests/fixtures/function_zxy_query.sql | 17 +++++++++++++++++ tests/fixtures/function_zxy_query_test.sql | 21 +++++++++++++++++++++ 2 files changed, 38 insertions(+) create mode 100644 tests/fixtures/function_zxy_query.sql create mode 100644 tests/fixtures/function_zxy_query_test.sql diff --git a/tests/fixtures/function_zxy_query.sql b/tests/fixtures/function_zxy_query.sql new file mode 100644 index 000000000..70bab5e7b --- /dev/null +++ b/tests/fixtures/function_zxy_query.sql @@ -0,0 +1,17 @@ +DROP FUNCTION IF EXISTS public.function_zxy_query; +CREATE OR REPLACE FUNCTION public.function_zxy_query(z integer, x integer, y integer, query_params json) RETURNS bytea AS $$ +DECLARE + mvt bytea; +BEGIN + RAISE NOTICE 'query_params: %', query_params; + + SELECT INTO mvt ST_AsMVT(tile, 'public.function_zxy_query', 4096, 'geom') FROM ( + SELECT + ST_AsMVTGeom(ST_Transform(ST_CurveToLine(geom), 3857), ST_TileEnvelope(z, x, y), 4096, 64, true) AS geom + FROM public.table_source + WHERE geom && ST_Transform(ST_TileEnvelope(z, x, y), 4326) + ) as tile WHERE geom IS NOT NULL; + + RETURN mvt; +END +$$ LANGUAGE plpgsql IMMUTABLE STRICT PARALLEL SAFE; diff --git a/tests/fixtures/function_zxy_query_test.sql b/tests/fixtures/function_zxy_query_test.sql new file mode 100644 index 000000000..7abe379e3 --- /dev/null +++ b/tests/fixtures/function_zxy_query_test.sql @@ -0,0 +1,21 @@ +DROP FUNCTION IF EXISTS public.function_zxy_query_test; +CREATE OR REPLACE FUNCTION public.function_zxy_query_test(z integer, x integer, y integer, query_params json) RETURNS bytea AS $$ +DECLARE + mvt bytea; +BEGIN + RAISE DEBUG 'query_params: %', query_params; + + IF (query_params->>'token')::varchar IS NULL THEN + RAISE EXCEPTION 'the `token` json parameter does not exist in `query_params`'; + END IF; + + SELECT INTO mvt ST_AsMVT(tile, 'public.function_zxy_query_test', 4096, 'geom') FROM ( + SELECT + ST_AsMVTGeom(ST_Transform(ST_CurveToLine(geom), 3857), ST_TileEnvelope(z, x, y), 4096, 64, true) AS geom + FROM public.table_source + WHERE geom && ST_Transform(ST_TileEnvelope(z, x, y), 4326) + ) as tile WHERE geom IS NOT NULL; + + RETURN mvt; +END +$$ LANGUAGE plpgsql IMMUTABLE STRICT PARALLEL SAFE; From d3887e4e68795638be34c73126c7cb01f6762730 Mon Sep 17 00:00:00 2001 From: Yuri Astrakhan Date: Tue, 29 Nov 2022 20:32:28 -0500 Subject: [PATCH 02/16] move sql files to ensure the order --- tests/fixtures/function_zxy_query.sql | 17 ----------------- tests/fixtures/function_zxy_query_test.sql | 21 --------------------- 2 files changed, 38 deletions(-) delete mode 100644 tests/fixtures/function_zxy_query.sql delete mode 100644 tests/fixtures/function_zxy_query_test.sql diff --git a/tests/fixtures/function_zxy_query.sql b/tests/fixtures/function_zxy_query.sql deleted file mode 100644 index 70bab5e7b..000000000 --- a/tests/fixtures/function_zxy_query.sql +++ /dev/null @@ -1,17 +0,0 @@ -DROP FUNCTION IF EXISTS public.function_zxy_query; -CREATE OR REPLACE FUNCTION public.function_zxy_query(z integer, x integer, y integer, query_params json) RETURNS bytea AS $$ -DECLARE - mvt bytea; -BEGIN - RAISE NOTICE 'query_params: %', query_params; - - SELECT INTO mvt ST_AsMVT(tile, 'public.function_zxy_query', 4096, 'geom') FROM ( - SELECT - ST_AsMVTGeom(ST_Transform(ST_CurveToLine(geom), 3857), ST_TileEnvelope(z, x, y), 4096, 64, true) AS geom - FROM public.table_source - WHERE geom && ST_Transform(ST_TileEnvelope(z, x, y), 4326) - ) as tile WHERE geom IS NOT NULL; - - RETURN mvt; -END -$$ LANGUAGE plpgsql IMMUTABLE STRICT PARALLEL SAFE; diff --git a/tests/fixtures/function_zxy_query_test.sql b/tests/fixtures/function_zxy_query_test.sql deleted file mode 100644 index 7abe379e3..000000000 --- a/tests/fixtures/function_zxy_query_test.sql +++ /dev/null @@ -1,21 +0,0 @@ -DROP FUNCTION IF EXISTS public.function_zxy_query_test; -CREATE OR REPLACE FUNCTION public.function_zxy_query_test(z integer, x integer, y integer, query_params json) RETURNS bytea AS $$ -DECLARE - mvt bytea; -BEGIN - RAISE DEBUG 'query_params: %', query_params; - - IF (query_params->>'token')::varchar IS NULL THEN - RAISE EXCEPTION 'the `token` json parameter does not exist in `query_params`'; - END IF; - - SELECT INTO mvt ST_AsMVT(tile, 'public.function_zxy_query_test', 4096, 'geom') FROM ( - SELECT - ST_AsMVTGeom(ST_Transform(ST_CurveToLine(geom), 3857), ST_TileEnvelope(z, x, y), 4096, 64, true) AS geom - FROM public.table_source - WHERE geom && ST_Transform(ST_TileEnvelope(z, x, y), 4326) - ) as tile WHERE geom IS NOT NULL; - - RETURN mvt; -END -$$ LANGUAGE plpgsql IMMUTABLE STRICT PARALLEL SAFE; From aa84b155a0598df05aa55e9a5e3ddfa7ea11d2ba Mon Sep 17 00:00:00 2001 From: Yuri Astrakhan Date: Sat, 26 Nov 2022 16:00:50 -0500 Subject: [PATCH 03/16] Support z,x,y and record-returning func Can handle the `getmvt(z,x,y) -> [bytea,md5]` case and several others. --- README.md | 31 ++- justfile | 1 + src/bin/main.rs | 17 +- src/config.rs | 25 ++- src/pg/config.rs | 89 +++++++- src/pg/db.rs | 158 +++++++------ src/pg/function_source.rs | 208 +++++++++++------- src/pg/scripts/call_rpc.sql | 1 - src/pg/scripts/get_function_sources.sql | 68 +++++- src/srv/server.rs | 10 +- tests/fixtures/functions/function_zoom_xy.sql | 16 ++ tests/fixtures/functions/function_zxy.sql | 16 ++ tests/fixtures/functions/function_zxy2.sql | 10 + .../fixtures/functions/function_zxy_query.sql | 5 +- .../functions/function_zxy_query_test.sql | 1 + tests/fixtures/functions/function_zxy_row.sql | 11 + .../fixtures/functions/function_zxy_row2.sql | 15 ++ .../functions/function_zxy_row_key.sql | 12 + tests/function_source_test.rs | 23 +- tests/server_test.rs | 85 +++++-- tests/test.sh | 9 + tests/utils.rs | 97 +++----- 22 files changed, 600 insertions(+), 308 deletions(-) delete mode 100755 src/pg/scripts/call_rpc.sql create mode 100644 tests/fixtures/functions/function_zoom_xy.sql create mode 100644 tests/fixtures/functions/function_zxy.sql create mode 100644 tests/fixtures/functions/function_zxy2.sql create mode 100644 tests/fixtures/functions/function_zxy_row.sql create mode 100644 tests/fixtures/functions/function_zxy_row2.sql create mode 100644 tests/fixtures/functions/function_zxy_row_key.sql diff --git a/README.md b/README.md index 211c29546..680609948 100755 --- a/README.md +++ b/README.md @@ -311,17 +311,34 @@ curl localhost:3000/points,lines/0/0/0 ## Function Sources -Function Source is a database function which can be used to query [vector tiles](https://github.com/mapbox/vector-tile-spec). When started, martin will look for the functions with a suitable signature. A function that takes `z integer`, `x integer`, `y integer`, and `query_params json` and returns `bytea`, can be used as a Function Source. +Function Source is a database function which can be used to query [vector tiles](https://github.com/mapbox/vector-tile-spec). When started, martin will look for the functions with a suitable signature. A function that takes `z integer` (or `zoom integer`), `x integer`, `y integer`, and an optional `query json` and returns `bytea`, can be used as a Function Source. Alternatively the function could return a record with a single `bytea` field, or a record with two fields of types `bytea` and `text`, where the `text` field is a etag key (i.e. md5 hash). -| Argument | Type | Description | -|--------------|---------|-------------------------| -| z | integer | Tile zoom parameter | -| x | integer | Tile x parameter | -| y | integer | Tile y parameter | -| query_params | json | Query string parameters | +| Argument | Type | Description | +|----------------------------|---------|-------------------------| +| z (or zoom) | integer | Tile zoom parameter | +| x | integer | Tile x parameter | +| y | integer | Tile y parameter | +| query (optional, any name) | json | Query string parameters | For example, if you have a table `table_source` in WGS84 (`4326` SRID), then you can use this function as a Function Source: +```sql, ignore +CREATE OR REPLACE FUNCTION function_zxy_query(z integer, x integer, y integer) RETURNS bytea AS $$ +DECLARE + mvt bytea; +BEGIN + SELECT INTO mvt ST_AsMVT(tile, 'function_zxy_query', 4096, 'geom') FROM ( + SELECT + ST_AsMVTGeom(ST_Transform(ST_CurveToLine(geom), 3857), ST_TileEnvelope(z, x, y), 4096, 64, true) AS geom + FROM table_source + WHERE geom && ST_Transform(ST_TileEnvelope(z, x, y), 4326) + ) as tile WHERE geom IS NOT NULL; + + RETURN mvt; +END +$$ LANGUAGE plpgsql IMMUTABLE STRICT PARALLEL SAFE; +``` + ```sql, ignore CREATE OR REPLACE FUNCTION function_zxy_query(z integer, x integer, y integer, query_params json) RETURNS bytea AS $$ DECLARE diff --git a/justfile b/justfile index 3e67f5cbf..6b9cadc91 100644 --- a/justfile +++ b/justfile @@ -3,6 +3,7 @@ set shell := ["bash", "-c"] export DATABASE_URL := "postgres://postgres@localhost/db" export CARGO_TERM_COLOR := "always" +# export RUST_LOG := "debug" # export RUST_BACKTRACE := "1" @_default: diff --git a/src/bin/main.rs b/src/bin/main.rs index 646d02781..4229b51df 100644 --- a/src/bin/main.rs +++ b/src/bin/main.rs @@ -1,9 +1,9 @@ use actix_web::dev::Server; use clap::Parser; use log::{error, info, warn}; -use martin::config::{read_config, ConfigBuilder}; +use martin::config::{read_config, ConfigBuilder, ConfigDb}; use martin::pg::config::{PgArgs, PgConfigBuilder}; -use martin::pg::db::PgConfigurator; +use martin::pg::db::resolve_pg_data; use martin::source::IdResolver; use martin::srv::config::{SrvArgs, SrvConfigBuilder}; use martin::srv::server; @@ -70,13 +70,14 @@ async fn start(args: Args) -> io::Result { if let Some(file_cfg) = file_cfg { builder.merge(file_cfg); } - let mut config = builder.finalize()?; + let config = builder.finalize()?; let id_resolver = IdResolver::new(RESERVED_KEYWORDS); - let mut pg_config = PgConfigurator::new(&config.pg, id_resolver).await?; - let sources = pg_config.discover_db_sources().await?; - config.pg.table_sources = pg_config.table_sources; - config.pg.function_sources = pg_config.function_sources; + let (sources, pg_config, _) = resolve_pg_data(config.pg, id_resolver).await?; + let config = ConfigDb { + srv: config.srv, + pg: pg_config, + }; if save_config.is_some() { let yaml = serde_yaml::to_string(&config).expect("Unable to serialize config"); @@ -94,7 +95,7 @@ async fn start(args: Args) -> io::Result { } let listen_addresses = config.srv.listen_addresses.clone(); - let server = server::new(config, sources); + let server = server::new(config.srv, sources); info!("Martin has been started on {listen_addresses}."); info!("Use http://{listen_addresses}/catalog to get the list of available sources."); Ok(server) diff --git a/src/config.rs b/src/config.rs index ce36c1ff2..dfbb711b1 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1,5 +1,5 @@ use crate::io_error; -use crate::pg::config::{PgConfig, PgConfigBuilder}; +use crate::pg::config::{PgConfig, PgConfigBuilder, PgConfigDb}; use crate::srv::config::{SrvConfig, SrvConfigBuilder}; use log::warn; use serde::{Deserialize, Serialize}; @@ -17,6 +17,14 @@ pub struct Config { pub pg: PgConfig, } +#[derive(Debug, Clone, PartialEq, Serialize)] +pub struct ConfigDb { + #[serde(flatten)] + pub srv: SrvConfig, + #[serde(flatten)] + pub pg: PgConfigDb, +} + #[derive(Debug, PartialEq, Deserialize)] pub struct ConfigBuilder { #[serde(flatten)] @@ -154,14 +162,13 @@ mod tests { )]), function_sources: HashMap::from([( "function_zxy_query".to_string(), - FunctionInfo { - schema: "public".to_string(), - function: "function_zxy_query".to_string(), - minzoom: Some(0), - maxzoom: Some(30), - bounds: Some(Bounds::MAX), - unrecognized: HashMap::new(), - }, + FunctionInfo::new_extended( + "public".to_string(), + "function_zxy_query".to_string(), + 0, + 30, + Bounds::MAX, + ), )]), }, }; diff --git a/src/pg/config.rs b/src/pg/config.rs index 41fe745a1..67b4cde09 100644 --- a/src/pg/config.rs +++ b/src/pg/config.rs @@ -94,7 +94,28 @@ impl FormatId for TableInfo { } } -#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)] +#[derive(Clone, Serialize, Debug, PartialEq, Default)] +pub struct FunctionInfoDbInfo { + #[serde(skip_serializing)] + pub query: String, + #[serde(skip_serializing)] + pub has_query_params: bool, + #[serde(skip_serializing)] + pub signature: String, + #[serde(flatten)] + pub info: FunctionInfo, +} + +impl FunctionInfoDbInfo { + pub fn with_info(&self, info: &FunctionInfo) -> Self { + Self { + info: info.clone(), + ..self.clone() + } + } +} + +#[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Default)] pub struct FunctionInfo { /// Schema name pub schema: String, @@ -121,6 +142,33 @@ pub struct FunctionInfo { pub unrecognized: HashMap, } +impl FunctionInfo { + pub fn new(schema: String, function: String) -> Self { + Self { + schema, + function, + ..Default::default() + } + } + + pub fn new_extended( + schema: String, + function: String, + minzoom: u8, + maxzoom: u8, + bounds: Bounds, + ) -> Self { + Self { + schema, + function, + minzoom: Some(minzoom), + maxzoom: Some(maxzoom), + bounds: Some(bounds), + ..Default::default() + } + } +} + impl FormatId for FunctionInfo { fn format_id(&self, db_id: &str) -> String { format!("{}.{}.{}", db_id, self.schema, self.function) @@ -129,11 +177,16 @@ impl FormatId for FunctionInfo { pub type TableInfoSources = HashMap; pub type TableInfoVec = Vec; -pub type FunctionInfoSources = HashMap; -pub type FunctionInfoVec = Vec; +pub type FuncInfoSources = HashMap; +pub type FuncInfoDbSources = HashMap; +pub type FuncInfoDbMapMap = HashMap>; +pub type FuncInfoDbVec = Vec; + +pub type PgConfig = PgConfigRaw; +pub type PgConfigDb = PgConfigRaw; #[derive(Clone, Debug, Serialize, PartialEq)] -pub struct PgConfig { +pub struct PgConfigRaw { pub connection_string: String, #[cfg(feature = "ssl")] #[serde(skip_serializing_if = "Option::is_none")] @@ -145,8 +198,8 @@ pub struct PgConfig { pub pool_size: u32, pub discover_functions: bool, pub discover_tables: bool, - pub table_sources: TableInfoSources, - pub function_sources: FunctionInfoSources, + pub table_sources: T, + pub function_sources: F, } #[derive(Debug, Default, PartialEq, Deserialize)] @@ -159,7 +212,7 @@ pub struct PgConfigBuilder { pub default_srid: Option, pub pool_size: Option, pub table_sources: Option, - pub function_sources: Option, + pub function_sources: Option, } impl PgConfigBuilder { @@ -244,3 +297,25 @@ impl From<(PgArgs, Option)> for PgConfigBuilder { } } } + +impl PgConfig { + pub fn to_db( + self, + table_sources: TableInfoSources, + function_sources: FuncInfoDbSources, + ) -> PgConfigDb { + PgConfigDb { + connection_string: self.connection_string, + #[cfg(feature = "ssl")] + ca_root_file: self.ca_root_file, + #[cfg(feature = "ssl")] + danger_accept_invalid_certs: self.danger_accept_invalid_certs, + default_srid: self.default_srid, + pool_size: self.pool_size, + discover_functions: self.discover_functions, + discover_tables: self.discover_tables, + table_sources, + function_sources, + } + } +} diff --git a/src/pg/db.rs b/src/pg/db.rs index e5b756bab..fdd95eaed 100755 --- a/src/pg/db.rs +++ b/src/pg/db.rs @@ -1,5 +1,6 @@ use crate::pg::config::{ - FormatId, FunctionInfo, FunctionInfoSources, PgConfig, TableInfo, TableInfoSources, + FormatId, FuncInfoDbMapMap, FuncInfoDbSources, FuncInfoSources, FunctionInfoDbInfo, PgConfig, + PgConfigDb, TableInfo, TableInfoSources, }; use crate::pg::function_source::{get_function_sources, FunctionSource}; use crate::pg::table_source::{get_table_sources, TableSource}; @@ -9,7 +10,7 @@ use crate::srv::server::Sources; use bb8::PooledConnection; use bb8_postgres::{tokio_postgres as pg, PostgresConnectionManager}; use futures::future::try_join; -use log::info; +use log::{debug, info, warn}; #[cfg(feature = "ssl")] use openssl::ssl::{SslConnector, SslMethod, SslVerifyMode}; #[cfg(feature = "ssl")] @@ -30,19 +31,37 @@ pub type Connection<'a> = PooledConnection<'a, ConnectionManager>; // We require ST_TileEnvelope that was added in PostGIS 3.0.0 const REQUIRED_POSTGIS_VERSION: &str = ">= 3.0.0"; -pub struct PgConfigurator { - pool: Pool, +pub async fn resolve_pg_data( + config: PgConfig, + id_resolver: IdResolver, +) -> io::Result<(Sources, PgConfigDb, Pool)> { + let pg = PgConfigurator::new(&config, id_resolver).await?; + let ((mut tables, tbl_info), (funcs, func_info)) = + try_join(pg.instantiate_tables(), pg.instantiate_functions()).await?; + tables.extend(funcs); + Ok((tables, config.to_db(tbl_info, func_info), pg.pool)) +} + +/// A test helper to configure and instantiate a Postgres connection pool +pub async fn make_pool(config: PgConfig) -> io::Result { + Ok(PgConfigurator::new(&config, IdResolver::default()) + .await? + .pool) +} + +struct PgConfigurator { + pub pool: Pool, default_srid: Option, discover_functions: bool, discover_tables: bool, id_resolver: IdResolver, db_id: String, - pub table_sources: TableInfoSources, - pub function_sources: FunctionInfoSources, + table_sources: TableInfoSources, + function_sources: FuncInfoSources, } impl PgConfigurator { - pub async fn new(config: &PgConfig, id_resolver: IdResolver) -> io::Result { + async fn new(config: &PgConfig, id_resolver: IdResolver) -> io::Result { let conn_str = config.connection_string.as_str(); info!("Connecting to {conn_str}"); let pg_cfg = pg::config::Config::from_str(conn_str) @@ -101,23 +120,10 @@ impl PgConfigurator { } } - pub async fn discover_db_sources(&mut self) -> io::Result { - let ((mut tables, tbl_info), (funcs, func_info)) = - try_join(self.instantiate_tables(), self.instantiate_functions()).await?; - self.table_sources.extend(tbl_info); - self.function_sources.extend(func_info); - tables.extend(funcs); - Ok(tables) - } - - pub fn get_pool(&self) -> &Pool { - &self.pool - } - - pub async fn instantiate_tables(&self) -> Result<(Sources, TableInfoSources), io::Error> { + async fn instantiate_tables(&self) -> Result<(Sources, TableInfoSources), io::Error> { let mut sources: Sources = HashMap::new(); for (id, info) in &self.table_sources { - self.add_source(&mut sources, None, id.clone(), info.clone()); + self.add_table_src(&mut sources, None, id.clone(), info.clone()); } let mut tables = TableInfoSources::new(); if self.discover_tables { @@ -157,72 +163,64 @@ impl PgConfigurator { sources.insert(id, Box::new(source)); } - pub async fn instantiate_functions(&self) -> Result<(Sources, FunctionInfoSources), io::Error> { - let mut sources: Sources = HashMap::new(); + async fn instantiate_functions(&self) -> Result<(Sources, FuncInfoDbSources), io::Error> { + let mut res: Sources = HashMap::new(); + let mut func_info = FuncInfoDbSources::new(); + let mut sources = get_function_sources(&self.pool).await?; + let mut used_srcs = FuncInfoDbMapMap::new(); + for (id, info) in &self.function_sources { - self.add_func_src(&mut sources, None, id.clone(), info.clone()); + let schema = info.schema.as_str(); + let name = info.function.as_str(); + if let Some(db_inf) = sources.get_mut(schema).and_then(|v| v.remove(name)) { + let db_inf = db_inf.with_info(info); + let id = self.add_func_src(&mut res, id.clone(), db_inf.clone()); + let sig = &db_inf.signature; + info!("Configured function source {id} -> {sig}"); + debug!("{}", db_inf.query); + func_info.insert(id, db_inf.clone()); + // Store it just in case another source needs the same function + used_srcs + .entry(schema.to_string()) + .or_default() + .insert(name.to_string(), db_inf); + } else if let Some(db_inf) = used_srcs.get_mut(schema).and_then(|v| v.get(name)) { + // This function was already used in another source + let db_inf = db_inf.with_info(info); + let id = self.add_func_src(&mut res, id.clone(), db_inf.clone()); + let sig = &db_inf.signature; + info!("Configured duplicate function source {id} -> {sig}"); + debug!("{}", db_inf.query); + func_info.insert(id, db_inf); + } else { + warn!( + "Configured function source {id} from {schema}.{name} doesn't exist or \ + doesn't have an expected signature like (z,x,y) -> bytea. See README.md", + ); + } } - let mut funcs = FunctionInfoSources::new(); + if self.discover_functions { - info!("Automatically detecting function sources"); - for info in get_function_sources(&self.pool, &self.function_sources).await? { - self.add_func_src(&mut sources, Some(&mut funcs), info.function.clone(), info); + for funcs in sources.into_values() { + for (name, db_inf) in funcs { + let id = self.add_func_src(&mut res, name, db_inf.clone()); + let sig = &db_inf.signature; + info!("Discovered function source {id} -> {sig}"); + debug!("{}", db_inf.query); + func_info.insert(id, db_inf); + } } } - Ok((sources, funcs)) - } - fn add_func_src( - &self, - sources: &mut Sources, - funcs: Option<&mut FunctionInfoSources>, - id: String, - info: FunctionInfo, - ) { - let id = self.id_resolver.resolve(id, info.format_id(&self.db_id)); - let prefix = if let Some(funcs) = funcs { - funcs.insert(id.clone(), info.clone()); - "Discovered" - } else { - "Configured" - }; - info!( - r#"{prefix} function source "{id}" that calls {}.{}(...)"#, - info.schema, info.function - ); - sources.insert( - id.clone(), - Box::new(FunctionSource::new(id, info, self.pool.clone())), - ); + Ok((res, func_info)) } - fn add_source( - &self, - sources: &mut Sources, - tables: Option<&mut TableInfoSources>, - id: String, - info: TableInfo, - ) { - let unique_id = info.format_id(&self.db_id); - let id = self.id_resolver.resolve(id, unique_id); - let prefix = if let Some(tables) = tables { - tables.insert(id.clone(), info.clone()); - "Discovered" - } else { - "Configured" - }; - info!( - r#"{prefix} table source "{id}" from "{}.{}" with "{}" column ({}, SRID={})"#, - info.schema, - info.table, - info.geometry_column, - info.geometry_type.as_deref().unwrap_or("null"), - info.srid - ); - sources.insert( - id.clone(), - Box::new(TableSource::new(id, info, self.pool.clone())), - ); + fn add_func_src(&self, sources: &mut Sources, id: String, info: FunctionInfoDbInfo) -> String { + let resolver = &self.id_resolver; + let id = resolver.resolve(id, info.info.format_id(&self.db_id)); + let source = FunctionSource::new(id.clone(), info, self.pool.clone()); + sources.insert(id.clone(), Box::new(source)); + id } } diff --git a/src/pg/function_source.rs b/src/pg/function_source.rs index 2b88c8a8b..ec4200591 100644 --- a/src/pg/function_source.rs +++ b/src/pg/function_source.rs @@ -1,33 +1,38 @@ -use crate::pg::config::{FormatId, FunctionInfo, FunctionInfoSources, FunctionInfoVec}; +use crate::pg::config::{FuncInfoDbMapMap, FunctionInfo, FunctionInfoDbInfo}; use crate::pg::db::get_connection; use crate::pg::db::Pool; use crate::pg::utils::{creat_tilejson, io_error, is_valid_zoom, query_to_json}; use crate::source::{Source, Tile, UrlQuery, Xyz}; use async_trait::async_trait; -use log::info; +use bb8_postgres::tokio_postgres::types::ToSql; +use log::{debug, warn}; use martin_tile_utils::DataFormat; use postgres::types::Type; use postgres_protocol::escape::escape_identifier; -use std::collections::{HashMap, HashSet}; +use serde_json::Value; +use std::collections::HashMap; +use std::fmt::Write; use std::io; +use std::iter::zip; use tilejson::TileJSON; #[derive(Clone, Debug)] pub struct FunctionSource { - pub id: String, - pub info: FunctionInfo, + id: String, + info: FunctionInfoDbInfo, pool: Pool, tilejson: TileJSON, } impl FunctionSource { - pub fn new(id: String, info: FunctionInfo, pool: Pool) -> Self { + pub fn new(id: String, info: FunctionInfoDbInfo, pool: Pool) -> Self { + let func = &info.info; Self { tilejson: creat_tilejson( - format!("{}.{}", info.schema, info.function), - info.minzoom, - info.maxzoom, - info.bounds, + format!("{}.{}", func.schema, func.function), + func.minzoom, + func.maxzoom, + func.bounds, None, ), id, @@ -52,86 +57,137 @@ impl Source for FunctionSource { } fn is_valid_zoom(&self, zoom: i32) -> bool { - is_valid_zoom(zoom, self.info.minzoom, self.info.maxzoom) + is_valid_zoom(zoom, self.info.info.minzoom, self.info.info.maxzoom) } - async fn get_tile(&self, xyz: &Xyz, query: &Option) -> Result { + async fn get_tile(&self, xyz: &Xyz, url_query: &Option) -> Result { let empty_query = HashMap::new(); - let query = query.as_ref().unwrap_or(&empty_query); - let query_json = query_to_json(query); - - // Query preparation : the schema and function can't be part of a prepared query, so they - // need to be escaped by hand. - // However schema and function comes from database introspection so they shall be safe. - // The query expects the following arguments : - // $1 : x - // $2 : y - // $3 : z - // $4 : query_json - - let escaped_schema = escape_identifier(&self.info.schema); - let escaped_function = escape_identifier(&self.info.function); - let raw_query = format!( - include_str!("scripts/call_rpc.sql"), - schema = escaped_schema, - function = escaped_function - ); - + let url_query = url_query.as_ref().unwrap_or(&empty_query); let conn = get_connection(&self.pool).await?; - let query = conn - .prepare_typed( - &raw_query, - &[Type::INT4, Type::INT4, Type::INT4, Type::JSON], - ) + + let param_types: &[Type] = if self.info.has_query_params { + &[Type::INT4, Type::INT4, Type::INT4, Type::JSON] + } else { + &[Type::INT4, Type::INT4, Type::INT4] + }; + + let query = &self.info.query; + let prep_query = conn + .prepare_typed(query, param_types) .await .map_err(|e| io_error!(e, "Can't create prepared statement for the tile"))?; - let tile = conn - .query_one(&query, &[&xyz.x, &xyz.y, &xyz.z, &query_json]) - .await - .map(|row| row.get(self.info.function.as_str())) - .map_err(|e| { - io_error!( - e, - r#"Can't get "{}" tile at {xyz} with {query_json:?} params"#, - self.id, - ) - })?; + let tile = if self.info.has_query_params { + let json = query_to_json(url_query); + debug!("SQL: {query} [{}, {}, {}, {json:?}]", xyz.x, xyz.y, xyz.z); + let params: &[&(dyn ToSql + Sync)] = &[&xyz.z, &xyz.x, &xyz.y, &json]; + conn.query_one(&prep_query, params).await + } else { + debug!("SQL: {query} [{}, {}, {}]", xyz.x, xyz.y, xyz.z); + conn.query_one(&prep_query, &[&xyz.z, &xyz.x, &xyz.y]).await + }; + + let tile = tile.map(|row| row.get(0)).map_err(|e| { + if self.info.has_query_params { + let url_q = query_to_json(url_query); + io_error!(e, r#"Can't get {}/{xyz} with {url_q:?} params"#, self.id) + } else { + io_error!(e, r#"Can't get {}/{xyz}"#, self.id) + } + })?; Ok(tile) } } -pub async fn get_function_sources( - pool: &Pool, - explicit_funcs: &FunctionInfoSources, -) -> Result { - let skip_funcs: HashSet = explicit_funcs.values().map(|v| v.format_id("")).collect(); - let conn = get_connection(pool).await?; - let rows = conn +pub async fn get_function_sources(pool: &Pool) -> Result { + let mut res = FuncInfoDbMapMap::new(); + get_connection(pool) + .await? .query(include_str!("scripts/get_function_sources.sql"), &[]) .await - .map_err(|e| io_error!(e, "Can't get function sources"))?; - - let mut result = FunctionInfoVec::default(); - for row in &rows { - let info = FunctionInfo { - schema: row.get("specific_schema"), - function: row.get("routine_name"), - minzoom: None, - maxzoom: None, - bounds: None, - unrecognized: HashMap::new(), - }; - if !skip_funcs.contains(&info.format_id("")) { - result.push(info); - } - } - result.sort_by_key(|v| v.function.clone()); + .map_err(|e| io_error!(e, "Can't get function sources"))? + .into_iter() + .for_each(|row| { + let schema: String = row.get("schema"); + let function: String = row.get("name"); + let output_type: &str = row.get("output_type"); + let output_record_types = jsonb_to_vec(&row.get("output_record_types")); + let output_record_names = jsonb_to_vec(&row.get("output_record_names")); + let input_types = jsonb_to_vec(&row.get("input_types")).expect("Can't get input types"); + let input_names = jsonb_to_vec(&row.get("input_names")).expect("Can't get input names"); + + assert!(input_types.len() >= 3 && input_types.len() <= 4); + assert_eq!(input_types.len(), input_names.len()); + match (&output_record_names, &output_record_types) { + (Some(n), Some(t)) if n.len() == 1 && n.len() == t.len() => { + assert_eq!(t, &["bytea"]); + } + (Some(n), Some(t)) if n.len() == 2 && n.len() == t.len() => { + assert_eq!(t, &["bytea", "text"]); + } + (None, None) => {} + _ => panic!("Invalid output record names or types"), + } + assert!(output_type == "bytea" || output_type == "record"); + + // Query preparation: the schema and function can't be part of a prepared query, so they + // need to be escaped by hand. + // However schema and function comes from database introspection so they shall be safe. + let mut query = String::new(); + query.push_str(&escape_identifier(&schema)); + query.push('.'); + query.push_str(&escape_identifier(&function)); + query.push('('); + for (idx, (name, typ)) in zip(input_names.iter(), input_types.iter()).enumerate() { + if idx > 0 { + write!(query, ", ").unwrap(); + } + write!(query, "{name} => ${index}::{typ}", index = idx + 1).unwrap(); + } + write!(query, ")").unwrap(); + + // This is the same as if let-chain, but that's not yet available + match (output_record_names, output_type) { + (Some(names), "record") => { + // SELECT mvt FROM "public"."function_zxy_row2"(z => $1::integer, x => $2::integer, y => $3::integer); + query.insert_str(0, " FROM "); + query.insert_str(0, &escape_identifier(names[0].as_str())); + query.insert_str(0, "SELECT "); + } + (_, _) => { + query.insert_str(0, "SELECT "); + query.push_str(" AS tile"); + } + } + warn!("SQL: {query}"); + + if let Some(v) = res + .entry(schema.clone()) + .or_insert_with(HashMap::new) + .insert( + function.clone(), + FunctionInfoDbInfo { + query, + has_query_params: input_types.len() == 4, + signature: format!("{schema}.{function}({})", input_names.join(", ")), + info: FunctionInfo::new(schema, function), + }, + ) + { + warn!("Unexpected duplicate function {}", v.signature); + } + }); - if result.is_empty() { - info!("No function sources found"); - } + Ok(res) +} - Ok(result) +fn jsonb_to_vec(jsonb: &Option) -> Option> { + jsonb.as_ref().map(|json| { + json.as_array() + .unwrap() + .iter() + .map(|v| v.as_str().unwrap().to_string()) + .collect() + }) } diff --git a/src/pg/scripts/call_rpc.sql b/src/pg/scripts/call_rpc.sql deleted file mode 100755 index 53c0e1e95..000000000 --- a/src/pg/scripts/call_rpc.sql +++ /dev/null @@ -1 +0,0 @@ -SELECT {schema}.{function}(z => $3, x => $1, y => $2, query_params => $4::json); diff --git a/src/pg/scripts/get_function_sources.sql b/src/pg/scripts/get_function_sources.sql index 8d21cbf76..1c23a6331 100755 --- a/src/pg/scripts/get_function_sources.sql +++ b/src/pg/scripts/get_function_sources.sql @@ -1,12 +1,58 @@ -SELECT - routines.specific_schema, - routines.routine_name +-- Find SQL functions that match these criteria: +-- * The function must have 3 or 4 input parameters, +-- first 3 must be integers and named z (or zoom), x, y (in that order), +-- with the optional JSON parameter as the 4th parameter (any name). +-- * The function output must be either a single bytea value or a table, +-- with the table row being either [bytea] or [bytea, text] (in that order). +-- * If the output is a two-column row, the second column will be used as etag (usually the MD5 hash) +-- +-- Output fields: +-- schema: the schema the function is in +-- name: the function name +-- output_type: either "bytea" or "record" +-- output_record_types: an optional JSON array of parameter types ["bytea"] or ["bytea", "text"] +-- output_record_names: an optional JSON array of output column names, e.g. ["mvt", "key"] +-- input_names: a JSON array of input parameter names +-- input_types: a JSON array of input parameter types +WITH inputs AS ( + -- list of input parameters for each function, returned as a jsonb array [{name: type}, ...] + SELECT specific_name, + jsonb_agg(COALESCE(parameter_name::text, '_') ORDER BY ordinal_position) as input_names, + jsonb_agg(data_type::text ORDER BY ordinal_position) as input_types + FROM information_schema.parameters + WHERE parameter_mode = 'IN' + GROUP BY specific_name), + outputs AS ( + -- list of output parameters for each function, returned as a jsonb array [{name: type}, ...] + SELECT specific_name, + jsonb_agg(data_type::text ORDER BY ordinal_position) as out_params, + jsonb_agg(parameter_name::text ORDER BY ordinal_position) as out_names + FROM information_schema.parameters + WHERE parameter_mode = 'OUT' + GROUP BY specific_name) +SELECT routines.specific_schema AS schema, + routines.routine_name AS name, + routines.data_type AS output_type, + outputs.out_params AS output_record_types, + out_names AS output_record_names, + inputs.input_types AS input_types, + inputs.input_names AS input_names FROM information_schema.routines - LEFT JOIN information_schema.parameters ON routines.specific_name=parameters.specific_name -WHERE - routines.data_type = 'bytea' -GROUP BY - routines.specific_schema, routines.routine_name, routines.data_type -HAVING - array_agg(array[parameters.parameter_name::text, parameters.data_type::text]) @> - array[array['z', 'integer'], array['x', 'integer'], array['y', 'integer'], array['query_params', 'json']]; \ No newline at end of file + JOIN inputs ON routines.specific_name = inputs.specific_name + LEFT JOIN outputs ON routines.specific_name = outputs.specific_name +WHERE jsonb_array_length(input_names) IN (3, 4) + AND input_names ->> 0 IN ('z', 'zoom') + AND input_names ->> 1 = 'x' + AND input_names ->> 2 = 'y' + -- the 4th parameter is optional, and can be any name + AND input_types ->> 0 = 'integer' + AND input_types ->> 1 = 'integer' + AND input_types ->> 2 = 'integer' + AND (input_types ->> 3 = 'json' OR (input_types ->> 3) IS NULL) + AND ( + (data_type = 'bytea' AND out_params IS NULL) + OR (data_type = 'bytea' AND out_params = '["bytea"]'::jsonb) + OR (data_type = 'record' AND out_params = '["bytea"]'::jsonb) + OR (data_type = 'record' AND out_params = '["bytea", "text"]'::jsonb) + ) +ORDER BY routines.specific_schema, routines.routine_name; diff --git a/src/srv/server.rs b/src/srv/server.rs index 223259921..561629b60 100755 --- a/src/srv/server.rs +++ b/src/srv/server.rs @@ -1,6 +1,6 @@ -use crate::config::Config; use crate::pg::utils::parse_x_rewrite_url; use crate::source::{Source, Xyz}; +use crate::srv::config::SrvConfig; use actix_cors::Cors; use actix_web::dev::Server; use actix_web::http::header::CACHE_CONTROL; @@ -272,10 +272,10 @@ pub fn router(cfg: &mut web::ServiceConfig) { .service(get_tile); } -pub fn new(config: Config, sources: Sources) -> Server { - let keep_alive = config.srv.keep_alive; - let worker_processes = config.srv.worker_processes; - let listen_addresses = config.srv.listen_addresses; +pub fn new(config: SrvConfig, sources: Sources) -> Server { + let keep_alive = config.keep_alive; + let worker_processes = config.worker_processes; + let listen_addresses = config.listen_addresses; HttpServer::new(move || { let state = AppState { diff --git a/tests/fixtures/functions/function_zoom_xy.sql b/tests/fixtures/functions/function_zoom_xy.sql new file mode 100644 index 000000000..a637f7925 --- /dev/null +++ b/tests/fixtures/functions/function_zoom_xy.sql @@ -0,0 +1,16 @@ +DROP FUNCTION IF EXISTS public.function_zoom_xy; + +CREATE OR REPLACE FUNCTION public.function_zoom_xy(zoom integer, x integer, y integer) RETURNS bytea AS $$ +DECLARE + mvt bytea; +BEGIN + SELECT INTO mvt ST_AsMVT(tile, 'public.function_zoom_xy', 4096, 'geom') FROM ( + SELECT + ST_AsMVTGeom(ST_Transform(ST_CurveToLine(geom), 3857), ST_TileEnvelope(zoom, x, y), 4096, 64, true) AS geom + FROM public.table_source + WHERE geom && ST_Transform(ST_TileEnvelope(zoom, x, y), 4326) + ) as tile WHERE geom IS NOT NULL; + + RETURN mvt; +END +$$ LANGUAGE plpgsql IMMUTABLE STRICT PARALLEL SAFE; diff --git a/tests/fixtures/functions/function_zxy.sql b/tests/fixtures/functions/function_zxy.sql new file mode 100644 index 000000000..10caf69f0 --- /dev/null +++ b/tests/fixtures/functions/function_zxy.sql @@ -0,0 +1,16 @@ +DROP FUNCTION IF EXISTS public.function_zxy; + +CREATE OR REPLACE FUNCTION public.function_zxy(z integer, x integer, y integer) RETURNS bytea AS $$ +DECLARE + mvt bytea; +BEGIN + SELECT INTO mvt ST_AsMVT(tile, 'public.function_zxy', 4096, 'geom') FROM ( + SELECT + ST_AsMVTGeom(ST_Transform(ST_CurveToLine(geom), 3857), ST_TileEnvelope(z, x, y), 4096, 64, true) AS geom + FROM public.table_source + WHERE geom && ST_Transform(ST_TileEnvelope(z, x, y), 4326) + ) as tile WHERE geom IS NOT NULL; + + RETURN mvt; +END +$$ LANGUAGE plpgsql IMMUTABLE STRICT PARALLEL SAFE; diff --git a/tests/fixtures/functions/function_zxy2.sql b/tests/fixtures/functions/function_zxy2.sql new file mode 100644 index 000000000..2490f47be --- /dev/null +++ b/tests/fixtures/functions/function_zxy2.sql @@ -0,0 +1,10 @@ +DROP FUNCTION IF EXISTS public.function_zxy2; + +CREATE OR REPLACE FUNCTION public.function_zxy2(z integer, x integer, y integer) RETURNS bytea AS $$ + SELECT ST_AsMVT(tile, 'public.function_zxy2', 4096, 'geom') FROM ( + SELECT + ST_AsMVTGeom(ST_Transform(ST_CurveToLine(geom), 3857), ST_TileEnvelope(z, x, y), 4096, 64, true) AS geom + FROM public.table_source + WHERE geom && ST_Transform(ST_TileEnvelope(z, x, y), 4326) + ) as tile WHERE geom IS NOT NULL +$$ LANGUAGE SQL IMMUTABLE STRICT PARALLEL SAFE; diff --git a/tests/fixtures/functions/function_zxy_query.sql b/tests/fixtures/functions/function_zxy_query.sql index 70bab5e7b..dd860640d 100644 --- a/tests/fixtures/functions/function_zxy_query.sql +++ b/tests/fixtures/functions/function_zxy_query.sql @@ -1,9 +1,10 @@ DROP FUNCTION IF EXISTS public.function_zxy_query; -CREATE OR REPLACE FUNCTION public.function_zxy_query(z integer, x integer, y integer, query_params json) RETURNS bytea AS $$ + +CREATE OR REPLACE FUNCTION public.function_zxy_query(z integer, x integer, y integer, query json) RETURNS bytea AS $$ DECLARE mvt bytea; BEGIN - RAISE NOTICE 'query_params: %', query_params; + RAISE NOTICE 'query: %', query; SELECT INTO mvt ST_AsMVT(tile, 'public.function_zxy_query', 4096, 'geom') FROM ( SELECT diff --git a/tests/fixtures/functions/function_zxy_query_test.sql b/tests/fixtures/functions/function_zxy_query_test.sql index 7abe379e3..62d488b1c 100644 --- a/tests/fixtures/functions/function_zxy_query_test.sql +++ b/tests/fixtures/functions/function_zxy_query_test.sql @@ -1,4 +1,5 @@ DROP FUNCTION IF EXISTS public.function_zxy_query_test; + CREATE OR REPLACE FUNCTION public.function_zxy_query_test(z integer, x integer, y integer, query_params json) RETURNS bytea AS $$ DECLARE mvt bytea; diff --git a/tests/fixtures/functions/function_zxy_row.sql b/tests/fixtures/functions/function_zxy_row.sql new file mode 100644 index 000000000..62975bc4a --- /dev/null +++ b/tests/fixtures/functions/function_zxy_row.sql @@ -0,0 +1,11 @@ +DROP FUNCTION IF EXISTS public.function_zxy_row; + +CREATE OR REPLACE FUNCTION public.function_zxy_row(z integer, x integer, y integer) +RETURNS TABLE(mvt bytea) AS $$ + SELECT ST_AsMVT(tile, 'public.function_zxy_row', 4096, 'geom') as mvt FROM ( + SELECT + ST_AsMVTGeom(ST_Transform(ST_CurveToLine(geom), 3857), ST_TileEnvelope(z, x, y), 4096, 64, true) AS geom + FROM public.table_source + WHERE geom && ST_Transform(ST_TileEnvelope(z, x, y), 4326) + ) as tile WHERE geom IS NOT NULL +$$ LANGUAGE SQL IMMUTABLE STRICT PARALLEL SAFE; diff --git a/tests/fixtures/functions/function_zxy_row2.sql b/tests/fixtures/functions/function_zxy_row2.sql new file mode 100644 index 000000000..22053bd7b --- /dev/null +++ b/tests/fixtures/functions/function_zxy_row2.sql @@ -0,0 +1,15 @@ +DROP FUNCTION IF EXISTS public.function_zxy_row2; + +CREATE OR REPLACE FUNCTION public.function_zxy_row2(z integer, x integer, y integer) +RETURNS TABLE(mvt bytea, key text) AS $$ + SELECT mvt, md5(mvt) as key FROM ( + SELECT ST_AsMVT(tile, 'public.function_zxy_row2', 4096, 'geom') as mvt FROM ( + SELECT + ST_AsMVTGeom( + ST_Transform(ST_CurveToLine(geom), 3857), + ST_TileEnvelope(z, x, y), + 4096, 64, true) AS geom + FROM public.table_source + WHERE geom && ST_Transform(ST_TileEnvelope(z, x, y), 4326) + ) as tile WHERE geom IS NOT NULL) src +$$ LANGUAGE SQL IMMUTABLE STRICT PARALLEL SAFE; diff --git a/tests/fixtures/functions/function_zxy_row_key.sql b/tests/fixtures/functions/function_zxy_row_key.sql new file mode 100644 index 000000000..7a12e94ff --- /dev/null +++ b/tests/fixtures/functions/function_zxy_row_key.sql @@ -0,0 +1,12 @@ +DROP FUNCTION IF EXISTS public.function_zxy_row_key; + +CREATE OR REPLACE FUNCTION public.function_zxy_row_key(z integer, x integer, y integer) +RETURNS TABLE(mvt bytea, key text) AS $$ + SELECT mvt, md5(mvt) as key FROM ( + SELECT ST_AsMVT(tile, 'public.function_zxy_row_key', 4096, 'geom') as mvt FROM ( + SELECT + ST_AsMVTGeom(ST_Transform(ST_CurveToLine(geom), 3857), ST_TileEnvelope(z, x, y), 4096, 64, true) AS geom + FROM public.table_source + WHERE geom && ST_Transform(ST_TileEnvelope(z, x, y), 4326) + ) as tile WHERE geom IS NOT NULL) src +$$ LANGUAGE sql IMMUTABLE STRICT PARALLEL SAFE; diff --git a/tests/function_source_test.rs b/tests/function_source_test.rs index a44e40ab7..a0d6c6cee 100644 --- a/tests/function_source_test.rs +++ b/tests/function_source_test.rs @@ -1,6 +1,5 @@ use ctor::ctor; use log::info; -use martin::pg::config::FunctionInfoSources; use martin::pg::function_source::get_function_sources as get_sources; use martin::source::Xyz; @@ -16,20 +15,20 @@ fn init() { #[actix_rt::test] async fn get_function_sources() { let pool = mock_pool().await; - let sources = get_sources(&pool, &FunctionInfoSources::default()) - .await - .unwrap(); - - info!("sources = {sources:#?}"); + let sources = get_sources(&pool).await.unwrap(); + // dbg!(&sources); assert!(!sources.is_empty()); - let source = single(&sources, |v| v.function == "function_zxy_query") + + let funcs = sources.get("public").expect("public schema not found"); + let source = funcs + .get("function_zxy_query") .expect("function_zxy_query not found"); - assert_eq!(source.schema, "public"); - assert_eq!(source.function, "function_zxy_query"); - assert_eq!(source.minzoom, None); - assert_eq!(source.maxzoom, None); - assert_eq!(source.bounds, None); + assert_eq!(source.info.schema, "public"); + assert_eq!(source.info.function, "function_zxy_query"); + assert_eq!(source.info.minzoom, None); + assert_eq!(source.info.maxzoom, None); + assert_eq!(source.info.bounds, None); } #[actix_rt::test] diff --git a/tests/server_test.rs b/tests/server_test.rs index 9e90f3289..5700d4213 100644 --- a/tests/server_test.rs +++ b/tests/server_test.rs @@ -48,7 +48,7 @@ async fn get_table_catalog_ok() { #[actix_rt::test] async fn get_function_catalog_ok() { - let app = create_app!(mock_default_function_sources()); + let app = create_app!(mock_sources(None, None)); let req = test_get("/catalog"); let response = call_service(&app, req).await; @@ -253,6 +253,32 @@ async fn get_table_source_tile_minmax_zoom_ok() { assert_eq!(response.status(), StatusCode::NOT_FOUND); } +#[actix_rt::test] +async fn get_function_tiles() { + let app = create_app!(mock_sources(None, None)); + + let req = test_get("/function_zoom_xy/6/38/20"); + assert!(call_service(&app, req).await.status().is_success()); + + let req = test_get("/function_zxy/6/38/20"); + assert!(call_service(&app, req).await.status().is_success()); + + let req = test_get("/function_zxy2/6/38/20"); + assert!(call_service(&app, req).await.status().is_success()); + + let req = test_get("/function_zxy_query/6/38/20"); + assert!(call_service(&app, req).await.status().is_success()); + + let req = test_get("/function_zxy_row/6/38/20"); + assert!(call_service(&app, req).await.status().is_success()); + + let req = test_get("/function_zxy_row2/6/38/20"); + assert!(call_service(&app, req).await.status().is_success()); + + let req = test_get("/function_zxy_row_key/6/38/20"); + assert!(call_service(&app, req).await.status().is_success()); +} + #[actix_rt::test] async fn get_composite_source_ok() { let app = create_app!(mock_default_table_sources()); @@ -356,16 +382,40 @@ async fn get_composite_source_tile_minmax_zoom_ok() { #[actix_rt::test] async fn get_function_source_ok() { - let app = create_app!(mock_default_function_sources()); + let app = create_app!(mock_sources(None, None)); let req = test_get("/non_existent"); let response = call_service(&app, req).await; assert_eq!(response.status(), StatusCode::NOT_FOUND); + let req = test_get("/function_zoom_xy"); + let response = call_service(&app, req).await; + assert!(response.status().is_success()); + + let req = test_get("/function_zxy"); + let response = call_service(&app, req).await; + assert!(response.status().is_success()); + let req = test_get("/function_zxy_query"); let response = call_service(&app, req).await; assert!(response.status().is_success()); + let req = test_get("/function_zxy_query_test"); + let response = call_service(&app, req).await; + assert!(response.status().is_success()); + + let req = test_get("/function_zxy_row"); + let response = call_service(&app, req).await; + assert!(response.status().is_success()); + + let req = test_get("/function_zxy_row2"); + let response = call_service(&app, req).await; + assert!(response.status().is_success()); + + let req = test_get("/function_zxy_row_key"); + let response = call_service(&app, req).await; + assert!(response.status().is_success()); + let req = TestRequest::get() .uri("/function_zxy_query?token=martin") .insert_header(("x-rewrite-url", "/tiles/function_zxy_query?token=martin")) @@ -379,7 +429,7 @@ async fn get_function_source_ok() { #[actix_rt::test] async fn get_function_source_tile_ok() { - let app = create_app!(mock_default_function_sources()); + let app = create_app!(mock_sources(None, None)); let req = test_get("/function_zxy_query/0/0/0"); let response = call_service(&app, req).await; @@ -388,23 +438,14 @@ async fn get_function_source_tile_ok() { #[actix_rt::test] async fn get_function_source_tile_minmax_zoom_ok() { - let function_source1 = FunctionInfo { - schema: "public".to_owned(), - function: "function_zxy_query".to_owned(), - minzoom: None, - maxzoom: None, - bounds: Some(Bounds::MAX), - unrecognized: HashMap::new(), - }; - - let function_source2 = FunctionInfo { - schema: "public".to_owned(), - function: "function_zxy_query".to_owned(), - minzoom: Some(6), - maxzoom: Some(12), - bounds: Some(Bounds::MAX), - unrecognized: HashMap::new(), - }; + let function_source1 = FunctionInfo::new("public".to_owned(), "function_zxy_query".to_owned()); + let function_source2 = FunctionInfo::new_extended( + "public".to_owned(), + "function_zxy_query".to_owned(), + 6, + 12, + Bounds::MAX, + ); let funcs = &[ ("function_source1", function_source1), @@ -455,7 +496,7 @@ async fn get_function_source_tile_minmax_zoom_ok() { #[actix_rt::test] async fn get_function_source_query_params_ok() { - let app = create_app!(mock_default_function_sources()); + let app = create_app!(mock_sources(None, None)); let req = test_get("/function_zxy_query_test/0/0/0"); let response = call_service(&app, req).await; @@ -469,7 +510,7 @@ async fn get_function_source_query_params_ok() { #[actix_rt::test] async fn get_health_returns_ok() { - let app = create_app!(mock_default_function_sources()); + let app = create_app!(mock_sources(None, None)); let req = test_get("/health"); let response = call_service(&app, req).await; diff --git a/tests/test.sh b/tests/test.sh index 7b8985c8c..1d0ef97a2 100755 --- a/tests/test.sh +++ b/tests/test.sh @@ -118,6 +118,15 @@ test_pbf fnc_20_633856_327787 http://localhost:3000/function_zxy_query/20/6338 test_pbf fnc_21_1267712_655574 http://localhost:3000/function_zxy_query/21/1267712/655574 test_pbf fnc_0_0_0_token http://localhost:3000/function_zxy_query_test/0/0/0?token=martin +>&2 echo "Test server response for different function call types" +test_pbf fnc_zoom_xy_6_38_20 http://localhost:3000/function_zoom_xy/6/38/20 +test_pbf fnc_zxy_6_38_20 http://localhost:3000/function_zxy/6/38/20 +test_pbf fnc_zxy2_6_38_20 http://localhost:3000/function_zxy2/6/38/20 +test_pbf fnc_zxy_query_6_38_20 http://localhost:3000/function_zxy_query/6/38/20 +test_pbf fnc_zxy_row_6_38_20 http://localhost:3000/function_zxy_row/6/38/20 +test_pbf fnc_zxy_row2_6_38_20 http://localhost:3000/function_zxy_row2/6/38/20 +test_pbf fnc_zxy_row_key_6_38_20 http://localhost:3000/function_zxy_row_key/6/38/20 + >&2 echo "Test server response for table source with different SRID" test_pbf points3857_srid_0_0_0 http://localhost:3000/points3857/0/0/0 diff --git a/tests/utils.rs b/tests/utils.rs index 79ffb93be..e92020f0e 100644 --- a/tests/utils.rs +++ b/tests/utils.rs @@ -1,11 +1,9 @@ use actix_web::web::Data; use log::info; -use martin::config::ConfigBuilder; -use martin::pg::config::TableInfo; use martin::pg::config::{FunctionInfo, PgConfigBuilder}; -use martin::pg::db::{PgConfigurator, Pool}; +use martin::pg::config::{PgConfig, TableInfo}; +use martin::pg::db::{make_pool, resolve_pg_data, Pool}; use martin::source::IdResolver; -use martin::srv::config::SrvConfigBuilder; use martin::srv::server::{AppState, Sources}; use std::collections::HashMap; use std::env; @@ -18,8 +16,8 @@ use tilejson::Bounds; #[allow(dead_code)] pub async fn mock_pool() -> Pool { - let pg = mock_pg_config(None, None).await; - pg.get_pool().clone() + let res = make_pool(mock_config(None, None).await).await; + res.expect("Failed to create pool") } #[allow(dead_code)] @@ -27,48 +25,39 @@ pub async fn mock_sources( function_sources: Option<&[(&'static str, FunctionInfo)]>, table_sources: Option<&[(&'static str, TableInfo)]>, ) -> Sources { - let mut pg = mock_pg_config(function_sources, table_sources).await; - pg.discover_db_sources().await.unwrap() + let cfg = mock_config(function_sources, table_sources).await; + let res = resolve_pg_data(cfg, IdResolver::default()).await; + res.expect("Failed to resolve pg data").0 } #[allow(dead_code)] -pub async fn mock_pg_config( +pub async fn mock_config( function_sources: Option<&[(&'static str, FunctionInfo)]>, table_sources: Option<&[(&'static str, TableInfo)]>, -) -> PgConfigurator { +) -> PgConfig { let connection_string: String = env::var("DATABASE_URL").unwrap(); info!("Connecting to {connection_string}"); - let config = ConfigBuilder { - srv: SrvConfigBuilder { - keep_alive: None, - listen_addresses: None, - worker_processes: None, - }, - pg: PgConfigBuilder { - connection_string: Some(connection_string), - #[cfg(feature = "ssl")] - ca_root_file: None, - #[cfg(feature = "ssl")] - danger_accept_invalid_certs: None, - default_srid: None, - pool_size: None, - table_sources: table_sources.map(|s| { - s.iter() - .map(|v| (v.0.to_string(), v.1.clone())) - .collect::>() - }), - function_sources: function_sources.map(|s| { - s.iter() - .map(|v| (v.0.to_string(), v.1.clone())) - .collect::>() - }), - // unrecognized: Default::default(), - }, - unrecognized: Default::default(), + let config = PgConfigBuilder { + connection_string: Some(connection_string), + #[cfg(feature = "ssl")] + ca_root_file: None, + #[cfg(feature = "ssl")] + danger_accept_invalid_certs: None, + default_srid: None, + pool_size: None, + table_sources: table_sources.map(|s| { + s.iter() + .map(|v| (v.0.to_string(), v.1.clone())) + .collect::>() + }), + function_sources: function_sources.map(|s| { + s.iter() + .map(|v| (v.0.to_string(), v.1.clone())) + .collect::>() + }), + // unrecognized: Default::default(), }; - let config = config.finalize().expect("Unable to finalize config"); - let id_resolver = IdResolver::default(); - PgConfigurator::new(&config.pg, id_resolver).await.unwrap() + config.finalize().expect("Unable to finalize config") } #[allow(dead_code)] @@ -171,34 +160,6 @@ pub async fn mock_default_table_sources() -> Sources { .await } -#[allow(dead_code)] -pub async fn mock_default_function_sources() -> Sources { - let function_zxy_query = FunctionInfo { - schema: "public".to_owned(), - function: "function_zxy_query".to_owned(), - minzoom: Some(0), - maxzoom: Some(30), - bounds: Some(Bounds::MAX), - unrecognized: HashMap::new(), - }; - - let function_zxy_query_test = FunctionInfo { - schema: "public".to_owned(), - function: "function_zxy_query_test".to_owned(), - unrecognized: HashMap::new(), - ..function_zxy_query - }; - - mock_sources( - Some(&[ - ("function_zxy_query", function_zxy_query), - ("function_zxy_query_test", function_zxy_query_test), - ]), - None, - ) - .await -} - #[allow(dead_code)] pub fn single(vec: &[T], mut cb: P) -> Option<&T> where From 84108268ce40c6b93e3344aec7a34536300941cb Mon Sep 17 00:00:00 2001 From: Yuri Astrakhan Date: Sat, 3 Dec 2022 10:39:05 -0500 Subject: [PATCH 04/16] Massive update to tables * All table sources now use the same system as functions * Proper table name escaping * Prepared statements might speed things up, but this is not certain because we prepare on each get request. At least now we can optimize in one place. --- benches/sources.rs | 6 +- src/bin/main.rs | 8 +- src/config.rs | 10 +- src/pg/config.rs | 106 +++++------ src/pg/configurator.rs | 186 +++++++++++++++++++ src/pg/connection.rs | 102 ++++++++++ src/pg/db.rs | 240 ------------------------ src/pg/function_source.rs | 133 +++----------- src/pg/mod.rs | 4 +- src/pg/pg_source.rs | 89 +++++++++ src/pg/scripts/get_bounds.sql | 4 +- src/pg/scripts/get_bounds_cte.sql | 1 - src/pg/scripts/get_geom.sql | 4 - src/pg/scripts/get_srid_bounds.sql | 1 - src/pg/scripts/get_table_sources.sql | 4 +- src/pg/scripts/get_tile.sql | 2 - src/pg/table_source.rs | 266 ++++++++++----------------- src/pg/utils.rs | 30 +-- src/source.rs | 16 +- src/srv/server.rs | 6 +- tests/function_source_test.rs | 22 +-- tests/server_test.rs | 20 +- tests/table_source_test.rs | 100 +++++----- tests/utils.rs | 5 +- 24 files changed, 640 insertions(+), 725 deletions(-) create mode 100755 src/pg/configurator.rs create mode 100755 src/pg/connection.rs delete mode 100755 src/pg/db.rs create mode 100644 src/pg/pg_source.rs delete mode 100644 src/pg/scripts/get_bounds_cte.sql delete mode 100644 src/pg/scripts/get_geom.sql delete mode 100644 src/pg/scripts/get_srid_bounds.sql delete mode 100755 src/pg/scripts/get_tile.sql diff --git a/benches/sources.rs b/benches/sources.rs index 5c7791de6..bac20aa6c 100644 --- a/benches/sources.rs +++ b/benches/sources.rs @@ -52,7 +52,7 @@ fn main() {} // // async fn get_table_source_tile() { // let source = mock_table_source("public", "table_source").await; -// let xyz = Xyz { z: 0, x: 0, y: 0 }; +// let xyz = Xyz::new(0, 0, 0); // let _tile = source.get_tile(&xyz, &None).await.unwrap(); // } // @@ -77,7 +77,7 @@ fn main() {} // table_sources: vec![points1, points2], // }; // -// let xyz = Xyz { z: 0, x: 0, y: 0 }; +// let xyz = Xyz::new(0, 0, 0); // let _tile = source.get_tile(&xyz, &None).await.unwrap(); // } // @@ -88,7 +88,7 @@ fn main() {} // // async fn get_function_source_tile() { // let source = mock_function_source("public", "function_zxy_query"); -// let xyz = Xyz { z: 0, x: 0, y: 0 }; +// let xyz = Xyz::new(0, 0, 0); // // let _tile = source.get_tile(&xyz, &None).await.unwrap(); // } diff --git a/src/bin/main.rs b/src/bin/main.rs index 4229b51df..649865824 100644 --- a/src/bin/main.rs +++ b/src/bin/main.rs @@ -1,9 +1,9 @@ use actix_web::dev::Server; use clap::Parser; use log::{error, info, warn}; -use martin::config::{read_config, ConfigBuilder, ConfigDb}; +use martin::config::{read_config, Config, ConfigBuilder}; use martin::pg::config::{PgArgs, PgConfigBuilder}; -use martin::pg::db::resolve_pg_data; +use martin::pg::configurator::resolve_pg_data; use martin::source::IdResolver; use martin::srv::config::{SrvArgs, SrvConfigBuilder}; use martin::srv::server; @@ -74,9 +74,9 @@ async fn start(args: Args) -> io::Result { let id_resolver = IdResolver::new(RESERVED_KEYWORDS); let (sources, pg_config, _) = resolve_pg_data(config.pg, id_resolver).await?; - let config = ConfigDb { - srv: config.srv, + let config = Config { pg: pg_config, + ..config }; if save_config.is_some() { diff --git a/src/config.rs b/src/config.rs index dfbb711b1..6270e3859 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1,5 +1,5 @@ use crate::io_error; -use crate::pg::config::{PgConfig, PgConfigBuilder, PgConfigDb}; +use crate::pg::config::{PgConfig, PgConfigBuilder}; use crate::srv::config::{SrvConfig, SrvConfigBuilder}; use log::warn; use serde::{Deserialize, Serialize}; @@ -17,14 +17,6 @@ pub struct Config { pub pg: PgConfig, } -#[derive(Debug, Clone, PartialEq, Serialize)] -pub struct ConfigDb { - #[serde(flatten)] - pub srv: SrvConfig, - #[serde(flatten)] - pub pg: PgConfigDb, -} - #[derive(Debug, PartialEq, Deserialize)] pub struct ConfigBuilder { #[serde(flatten)] diff --git a/src/pg/config.rs b/src/pg/config.rs index 67b4cde09..544bebe9e 100644 --- a/src/pg/config.rs +++ b/src/pg/config.rs @@ -1,9 +1,10 @@ use crate::config::{report_unrecognized_config, set_option}; +use crate::pg::utils::create_tilejson; use serde::{Deserialize, Serialize}; use serde_yaml::Value; use std::collections::HashMap; use std::{env, io}; -use tilejson::Bounds; +use tilejson::{Bounds, TileJSON}; pub const POOL_SIZE_DEFAULT: u32 = 20; @@ -25,11 +26,7 @@ pub struct PgArgs { pub pool_size: Option, } -pub trait FormatId { - fn format_id(&self, db_id: &str) -> String; -} - -#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)] +#[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Default)] pub struct TableInfo { /// Table schema pub schema: String, @@ -85,36 +82,44 @@ pub struct TableInfo { pub unrecognized: HashMap, } -impl FormatId for TableInfo { - fn format_id(&self, db_id: &str) -> String { - format!( - "{}.{}.{}.{}", - db_id, self.schema, self.table, self.geometry_column +impl PgInfo for TableInfo { + fn format_id(&self) -> String { + format!("{}.{}.{}", self.schema, self.table, self.geometry_column) + } + + fn to_tilejson(&self) -> TileJSON { + create_tilejson( + self.format_id(), + self.minzoom, + self.maxzoom, + self.bounds, + None, ) } } -#[derive(Clone, Serialize, Debug, PartialEq, Default)] -pub struct FunctionInfoDbInfo { - #[serde(skip_serializing)] +#[derive(Clone, Debug)] +pub struct PgSqlInfo { pub query: String, - #[serde(skip_serializing)] pub has_query_params: bool, - #[serde(skip_serializing)] pub signature: String, - #[serde(flatten)] - pub info: FunctionInfo, } -impl FunctionInfoDbInfo { - pub fn with_info(&self, info: &FunctionInfo) -> Self { +impl PgSqlInfo { + pub fn new(query: String, has_query_params: bool, signature: String) -> Self { Self { - info: info.clone(), - ..self.clone() + query, + has_query_params, + signature, } } } +pub trait PgInfo { + fn format_id(&self) -> String; + fn to_tilejson(&self) -> TileJSON; +} + #[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Default)] pub struct FunctionInfo { /// Schema name @@ -169,24 +174,31 @@ impl FunctionInfo { } } -impl FormatId for FunctionInfo { - fn format_id(&self, db_id: &str) -> String { - format!("{}.{}.{}", db_id, self.schema, self.function) +impl PgInfo for FunctionInfo { + fn format_id(&self) -> String { + format!("{}.{}", self.schema, self.function) + } + + fn to_tilejson(&self) -> TileJSON { + create_tilejson( + self.format_id(), + self.minzoom, + self.maxzoom, + self.bounds, + None, + ) } } pub type TableInfoSources = HashMap; -pub type TableInfoVec = Vec; pub type FuncInfoSources = HashMap; -pub type FuncInfoDbSources = HashMap; -pub type FuncInfoDbMapMap = HashMap>; -pub type FuncInfoDbVec = Vec; - -pub type PgConfig = PgConfigRaw; -pub type PgConfigDb = PgConfigRaw; +pub type InfoMap = HashMap; +pub type SqlFuncInfoMapMap = HashMap>; +pub type SqlTableInfoMapMapMap = + HashMap>>; #[derive(Clone, Debug, Serialize, PartialEq)] -pub struct PgConfigRaw { +pub struct PgConfig { pub connection_string: String, #[cfg(feature = "ssl")] #[serde(skip_serializing_if = "Option::is_none")] @@ -196,10 +208,12 @@ pub struct PgConfigRaw { #[serde(skip_serializing_if = "Option::is_none")] pub default_srid: Option, pub pool_size: u32, + #[serde(skip_serializing)] pub discover_functions: bool, + #[serde(skip_serializing)] pub discover_tables: bool, - pub table_sources: T, - pub function_sources: F, + pub table_sources: TableInfoSources, + pub function_sources: FuncInfoSources, } #[derive(Debug, Default, PartialEq, Deserialize)] @@ -297,25 +311,3 @@ impl From<(PgArgs, Option)> for PgConfigBuilder { } } } - -impl PgConfig { - pub fn to_db( - self, - table_sources: TableInfoSources, - function_sources: FuncInfoDbSources, - ) -> PgConfigDb { - PgConfigDb { - connection_string: self.connection_string, - #[cfg(feature = "ssl")] - ca_root_file: self.ca_root_file, - #[cfg(feature = "ssl")] - danger_accept_invalid_certs: self.danger_accept_invalid_certs, - default_srid: self.default_srid, - pool_size: self.pool_size, - discover_functions: self.discover_functions, - discover_tables: self.discover_tables, - table_sources, - function_sources, - } - } -} diff --git a/src/pg/configurator.rs b/src/pg/configurator.rs new file mode 100755 index 000000000..c4d1bbec9 --- /dev/null +++ b/src/pg/configurator.rs @@ -0,0 +1,186 @@ +use crate::pg::config::FunctionInfo; +use crate::pg::config::TableInfo; +use crate::pg::config::{FuncInfoSources, InfoMap, PgConfig, PgInfo, PgSqlInfo, TableInfoSources}; +use crate::pg::connection::Pool; +use crate::pg::function_source::get_function_sources; +use crate::pg::pg_source::PgSource; +use crate::pg::table_source::get_table_sources; +use crate::source::IdResolver; +use crate::srv::server::Sources; +use futures::future::try_join; +use log::{debug, info, warn}; +use std::collections::HashMap; +use std::io; + +pub async fn resolve_pg_data( + config: PgConfig, + id_resolver: IdResolver, +) -> io::Result<(Sources, PgConfig, Pool)> { + let pg = PgConfigurator::new(&config, id_resolver).await?; + let ((mut tables, tbl_info), (funcs, func_info)) = + try_join(pg.instantiate_tables(), pg.instantiate_functions()).await?; + + tables.extend(funcs); + Ok(( + tables, + PgConfig { + table_sources: tbl_info, + function_sources: func_info, + ..config + }, + pg.pool, + )) +} + +struct PgConfigurator { + pool: Pool, + default_srid: Option, + discover_functions: bool, + discover_tables: bool, + id_resolver: IdResolver, + table_sources: TableInfoSources, + function_sources: FuncInfoSources, +} + +impl PgConfigurator { + async fn new(config: &PgConfig, id_resolver: IdResolver) -> io::Result { + let pool = Pool::new(config).await?; + Ok(Self { + pool, + default_srid: config.default_srid, + discover_functions: config.discover_functions, + discover_tables: config.discover_tables, + id_resolver, + table_sources: config.table_sources.clone(), + function_sources: config.function_sources.clone(), + }) + } + + async fn instantiate_tables(&self) -> Result<(Sources, InfoMap), io::Error> { + let mut res: Sources = HashMap::new(); + let mut info_map = InfoMap::new(); + let mut discovered_sources = get_table_sources(&self.pool, self.default_srid).await?; + + for (id, src_inf) in &self.table_sources { + if let Some((pg_sql, _)) = discovered_sources + .get_mut(&src_inf.schema) + .and_then(|v| v.get_mut(&src_inf.table)) + .and_then(|v| v.remove(&src_inf.geometry_column)) + { + let id2 = self.resolve_id(id.clone(), src_inf); + self.add_func_src(&mut res, id2.clone(), src_inf, pg_sql.clone()); + warn_on_rename(id, &id2, "table"); + info!("Configured source {id2} from table {}", pg_sql.signature); + debug!("{}", pg_sql.query); + info_map.insert(id2, src_inf.clone()); + } else { + warn!( + "Configured table source {id} as {}.{} with geo column {} does not exist", + src_inf.schema, src_inf.table, src_inf.geometry_column + ); + } + } + + if self.discover_tables { + for tables in discovered_sources.into_values() { + for geoms in tables.into_values() { + for (pg_sql, src_inf) in geoms.into_values() { + let table = &src_inf.table; + let id2 = self.resolve_id(table.clone(), &src_inf); + self.add_func_src(&mut res, id2.clone(), &src_inf, pg_sql.clone()); + info!( + "Discovered source {id2} from table {}.{} with {} column ({}, SRID={})", + src_inf.schema, + src_inf.table, + src_inf.geometry_column, + src_inf.geometry_type.as_deref().unwrap_or("UNKNOWN"), + src_inf.srid, + ); + debug!("{}", pg_sql.query); + info_map.insert(id2, src_inf); + } + } + } + } + + Ok((res, info_map)) + } + + async fn instantiate_functions(&self) -> Result<(Sources, InfoMap), io::Error> { + let mut discovered_sources = get_function_sources(&self.pool).await?; + let mut res: Sources = HashMap::new(); + let mut info_map = InfoMap::new(); + let mut used_funcs: HashMap> = HashMap::new(); + + for (id, src_inf) in &self.function_sources { + let schema = &src_inf.schema; + let name = &src_inf.function; + if let Some((pg_sql, _)) = discovered_sources + .get_mut(schema) + .and_then(|v| v.remove(name)) + { + let id2 = self.resolve_id(id.clone(), src_inf); + self.add_func_src(&mut res, id2.clone(), src_inf, pg_sql.clone()); + warn_on_rename(id, &id2, "function"); + info!("Configured source {id2} from function {}", pg_sql.signature); + debug!("{}", pg_sql.query); + info_map.insert(id2, src_inf.clone()); + // Store it just in case another source needs the same function + used_funcs + .entry(schema.to_string()) + .or_default() + .insert(name.to_string(), pg_sql); + } else if let Some(pg_sql) = used_funcs.get_mut(schema).and_then(|v| v.get(name)) { + // This function was already used by another source + let id2 = self.resolve_id(id.clone(), src_inf); + self.add_func_src(&mut res, id2.clone(), src_inf, pg_sql.clone()); + warn_on_rename(id, &id2, "function"); + let sig = &pg_sql.signature; + info!("Configured duplicate source {id2} from function {sig}"); + debug!("{}", pg_sql.query); + info_map.insert(id2, src_inf.clone()); + } else { + warn!( + "Configured function source {id} from {schema}.{name} does not exist or \ + does not have an expected signature like (z,x,y) -> bytea. See README.md", + ); + } + } + + if self.discover_functions { + for funcs in discovered_sources.into_values() { + for (name, (pg_sql, src_inf)) in funcs { + let id2 = self.resolve_id(name.clone(), &src_inf); + self.add_func_src(&mut res, id2.clone(), &src_inf, pg_sql.clone()); + info!("Discovered source {id2} from function {}", pg_sql.signature); + debug!("{}", pg_sql.query); + info_map.insert(id2, src_inf); + } + } + } + + Ok((res, info_map)) + } + + fn resolve_id(&self, id: String, src_inf: &T) -> String { + let signature = format!("{}.{}", self.pool.get_id(), src_inf.format_id()); + self.id_resolver.resolve(id, signature) + } + + fn add_func_src( + &self, + sources: &mut Sources, + id: String, + src_inf: &impl PgInfo, + pg_sql: PgSqlInfo, + ) { + let source = PgSource::new(id.clone(), pg_sql, src_inf.to_tilejson(), self.pool.clone()); + sources.insert(id, Box::new(source)); + } +} + +fn warn_on_rename(old_id: &String, new_id: &String, typ: &str) { + if old_id != new_id { + warn!("Configured {typ} source {old_id} was renamed to {new_id} due to ID conflict"); + } +} diff --git a/src/pg/connection.rs b/src/pg/connection.rs new file mode 100755 index 000000000..c662ed767 --- /dev/null +++ b/src/pg/connection.rs @@ -0,0 +1,102 @@ +use crate::pg::config::PgConfig; +use crate::pg::utils::io_error; +use bb8::PooledConnection; +use bb8_postgres::{tokio_postgres as pg, PostgresConnectionManager}; +use log::info; +#[cfg(feature = "ssl")] +use openssl::ssl::{SslConnector, SslMethod, SslVerifyMode}; +#[cfg(feature = "ssl")] +use postgres_openssl::MakeTlsConnector; +use semver::{Version, VersionReq}; +use std::io; +use std::str::FromStr; + +#[cfg(feature = "ssl")] +pub type ConnectionManager = PostgresConnectionManager; +#[cfg(not(feature = "ssl"))] +pub type ConnectionManager = PostgresConnectionManager; + +pub type InternalPool = bb8::Pool; +pub type Connection<'a> = PooledConnection<'a, ConnectionManager>; + +// We require ST_TileEnvelope that was added in PostGIS 3.0.0 +const REQUIRED_POSTGIS_VERSION: &str = ">= 3.0.0"; + +#[derive(Clone, Debug)] +pub struct Pool { + id: String, + pool: InternalPool, +} + +impl Pool { + pub async fn new(config: &PgConfig) -> io::Result { + let conn_str = config.connection_string.as_str(); + info!("Connecting to {conn_str}"); + let pg_cfg = pg::config::Config::from_str(conn_str) + .map_err(|e| io_error!(e, "Can't parse connection string {conn_str}"))?; + + let id = pg_cfg + .get_dbname() + .map_or_else(|| format!("{:?}", pg_cfg.get_hosts()[0]), |v| v.to_string()); + + #[cfg(not(feature = "ssl"))] + let manager = ConnectionManager::new(pg_cfg, postgres::NoTls); + + #[cfg(feature = "ssl")] + let manager = { + let mut builder = SslConnector::builder(SslMethod::tls()) + .map_err(|e| io_error!(e, "Can't build TLS connection"))?; + + if config.danger_accept_invalid_certs { + builder.set_verify(SslVerifyMode::NONE); + } + + if let Some(ca_root_file) = &config.ca_root_file { + info!("Using {ca_root_file} as trusted root certificate"); + builder.set_ca_file(ca_root_file).map_err(|e| { + io_error!(e, "Can't set trusted root certificate {ca_root_file}") + })?; + } + PostgresConnectionManager::new(pg_cfg, MakeTlsConnector::new(builder.build())) + }; + + let pool = InternalPool::builder() + .max_size(config.pool_size) + .build(manager) + .await + .map_err(|e| io_error!(e, "Can't build connection pool"))?; + let pool = Pool { id, pool }; + + let postgis_version = pool.get_postgis_version().await?; + let req = VersionReq::parse(REQUIRED_POSTGIS_VERSION) + .map_err(|e| io_error!(e, "Can't parse required PostGIS version"))?; + let version = Version::parse(postgis_version.as_str()) + .map_err(|e| io_error!(e, "Can't parse database PostGIS version"))?; + + if !req.matches(&version) { + Err(io::Error::new(io::ErrorKind::Other, format!("Martin requires PostGIS {REQUIRED_POSTGIS_VERSION}, current version is {postgis_version}"))) + } else { + Ok(pool) + } + } + + pub async fn get(&self) -> io::Result> { + self.pool + .get() + .await + .map_err(|e| io_error!(e, "Can't retrieve connection from the pool")) + } + + pub fn get_id(&self) -> &str { + self.id.as_str() + } + + async fn get_postgis_version(&self) -> io::Result { + self.get() + .await? + .query_one(include_str!("scripts/get_postgis_version.sql"), &[]) + .await + .map(|row| row.get::<_, String>("postgis_version")) + .map_err(|e| io_error!(e, "Can't get PostGIS version")) + } +} diff --git a/src/pg/db.rs b/src/pg/db.rs deleted file mode 100755 index fdd95eaed..000000000 --- a/src/pg/db.rs +++ /dev/null @@ -1,240 +0,0 @@ -use crate::pg::config::{ - FormatId, FuncInfoDbMapMap, FuncInfoDbSources, FuncInfoSources, FunctionInfoDbInfo, PgConfig, - PgConfigDb, TableInfo, TableInfoSources, -}; -use crate::pg::function_source::{get_function_sources, FunctionSource}; -use crate::pg::table_source::{get_table_sources, TableSource}; -use crate::pg::utils::io_error; -use crate::source::IdResolver; -use crate::srv::server::Sources; -use bb8::PooledConnection; -use bb8_postgres::{tokio_postgres as pg, PostgresConnectionManager}; -use futures::future::try_join; -use log::{debug, info, warn}; -#[cfg(feature = "ssl")] -use openssl::ssl::{SslConnector, SslMethod, SslVerifyMode}; -#[cfg(feature = "ssl")] -use postgres_openssl::MakeTlsConnector; -use semver::{Version, VersionReq}; -use std::collections::HashMap; -use std::io; -use std::str::FromStr; - -#[cfg(feature = "ssl")] -pub type ConnectionManager = PostgresConnectionManager; -#[cfg(not(feature = "ssl"))] -pub type ConnectionManager = PostgresConnectionManager; - -pub type Pool = bb8::Pool; -pub type Connection<'a> = PooledConnection<'a, ConnectionManager>; - -// We require ST_TileEnvelope that was added in PostGIS 3.0.0 -const REQUIRED_POSTGIS_VERSION: &str = ">= 3.0.0"; - -pub async fn resolve_pg_data( - config: PgConfig, - id_resolver: IdResolver, -) -> io::Result<(Sources, PgConfigDb, Pool)> { - let pg = PgConfigurator::new(&config, id_resolver).await?; - let ((mut tables, tbl_info), (funcs, func_info)) = - try_join(pg.instantiate_tables(), pg.instantiate_functions()).await?; - tables.extend(funcs); - Ok((tables, config.to_db(tbl_info, func_info), pg.pool)) -} - -/// A test helper to configure and instantiate a Postgres connection pool -pub async fn make_pool(config: PgConfig) -> io::Result { - Ok(PgConfigurator::new(&config, IdResolver::default()) - .await? - .pool) -} - -struct PgConfigurator { - pub pool: Pool, - default_srid: Option, - discover_functions: bool, - discover_tables: bool, - id_resolver: IdResolver, - db_id: String, - table_sources: TableInfoSources, - function_sources: FuncInfoSources, -} - -impl PgConfigurator { - async fn new(config: &PgConfig, id_resolver: IdResolver) -> io::Result { - let conn_str = config.connection_string.as_str(); - info!("Connecting to {conn_str}"); - let pg_cfg = pg::config::Config::from_str(conn_str) - .map_err(|e| io_error!(e, "Can't parse connection string {conn_str}"))?; - - let db_id = pg_cfg - .get_dbname() - .map_or_else(|| format!("{:?}", pg_cfg.get_hosts()[0]), |v| v.to_string()); - - #[cfg(not(feature = "ssl"))] - let manager = ConnectionManager::new(pg_cfg, postgres::NoTls); - - #[cfg(feature = "ssl")] - let manager = { - let mut builder = SslConnector::builder(SslMethod::tls()) - .map_err(|e| io_error!(e, "Can't build TLS connection"))?; - - if config.danger_accept_invalid_certs { - builder.set_verify(SslVerifyMode::NONE); - } - - if let Some(ca_root_file) = &config.ca_root_file { - info!("Using {ca_root_file} as trusted root certificate"); - builder.set_ca_file(ca_root_file).map_err(|e| { - io_error!(e, "Can't set trusted root certificate {ca_root_file}") - })?; - } - PostgresConnectionManager::new(pg_cfg, MakeTlsConnector::new(builder.build())) - }; - - let pool = Pool::builder() - .max_size(config.pool_size) - .build(manager) - .await - .map_err(|e| io_error!(e, "Can't build connection pool"))?; - - let postgis_version = select_postgis_version(&pool).await?; - let req = VersionReq::parse(REQUIRED_POSTGIS_VERSION) - .map_err(|e| io_error!(e, "Can't parse required PostGIS version"))?; - let version = Version::parse(postgis_version.as_str()) - .map_err(|e| io_error!(e, "Can't parse database PostGIS version"))?; - - if !req.matches(&version) { - Err(io::Error::new(io::ErrorKind::Other, format!("Martin requires PostGIS {REQUIRED_POSTGIS_VERSION}, current version is {postgis_version}"))) - } else { - Ok(Self { - pool, - default_srid: config.default_srid, - discover_functions: config.discover_functions, - discover_tables: config.discover_tables, - id_resolver, - db_id, - table_sources: config.table_sources.clone(), - function_sources: config.function_sources.clone(), - }) - } - } - - async fn instantiate_tables(&self) -> Result<(Sources, TableInfoSources), io::Error> { - let mut sources: Sources = HashMap::new(); - for (id, info) in &self.table_sources { - self.add_table_src(&mut sources, None, id.clone(), info.clone()); - } - let mut tables = TableInfoSources::new(); - if self.discover_tables { - info!("Automatically detecting table sources"); - let srcs = get_table_sources(&self.pool, &self.table_sources, self.default_srid).await; - for info in srcs? { - self.add_table_src(&mut sources, Some(&mut tables), info.table.clone(), info); - } - } - Ok((sources, tables)) - } - - fn add_table_src( - &self, - sources: &mut Sources, - tables: Option<&mut TableInfoSources>, - id: String, - info: TableInfo, - ) { - let unique_id = info.format_id(&self.db_id); - let id = self.id_resolver.resolve(id, unique_id); - let prefix = if let Some(tables) = tables { - tables.insert(id.clone(), info.clone()); - "Discovered" - } else { - "Configured" - }; - info!( - r#"{prefix} table source "{id}" from "{}.{}" with "{}" column ({}, SRID={})"#, - info.schema, - info.table, - info.geometry_column, - info.geometry_type.as_deref().unwrap_or("null"), - info.srid - ); - let source = TableSource::new(id.clone(), info, self.pool.clone()); - sources.insert(id, Box::new(source)); - } - - async fn instantiate_functions(&self) -> Result<(Sources, FuncInfoDbSources), io::Error> { - let mut res: Sources = HashMap::new(); - let mut func_info = FuncInfoDbSources::new(); - let mut sources = get_function_sources(&self.pool).await?; - let mut used_srcs = FuncInfoDbMapMap::new(); - - for (id, info) in &self.function_sources { - let schema = info.schema.as_str(); - let name = info.function.as_str(); - if let Some(db_inf) = sources.get_mut(schema).and_then(|v| v.remove(name)) { - let db_inf = db_inf.with_info(info); - let id = self.add_func_src(&mut res, id.clone(), db_inf.clone()); - let sig = &db_inf.signature; - info!("Configured function source {id} -> {sig}"); - debug!("{}", db_inf.query); - func_info.insert(id, db_inf.clone()); - // Store it just in case another source needs the same function - used_srcs - .entry(schema.to_string()) - .or_default() - .insert(name.to_string(), db_inf); - } else if let Some(db_inf) = used_srcs.get_mut(schema).and_then(|v| v.get(name)) { - // This function was already used in another source - let db_inf = db_inf.with_info(info); - let id = self.add_func_src(&mut res, id.clone(), db_inf.clone()); - let sig = &db_inf.signature; - info!("Configured duplicate function source {id} -> {sig}"); - debug!("{}", db_inf.query); - func_info.insert(id, db_inf); - } else { - warn!( - "Configured function source {id} from {schema}.{name} doesn't exist or \ - doesn't have an expected signature like (z,x,y) -> bytea. See README.md", - ); - } - } - - if self.discover_functions { - for funcs in sources.into_values() { - for (name, db_inf) in funcs { - let id = self.add_func_src(&mut res, name, db_inf.clone()); - let sig = &db_inf.signature; - info!("Discovered function source {id} -> {sig}"); - debug!("{}", db_inf.query); - func_info.insert(id, db_inf); - } - } - } - - Ok((res, func_info)) - } - - fn add_func_src(&self, sources: &mut Sources, id: String, info: FunctionInfoDbInfo) -> String { - let resolver = &self.id_resolver; - let id = resolver.resolve(id, info.info.format_id(&self.db_id)); - let source = FunctionSource::new(id.clone(), info, self.pool.clone()); - sources.insert(id.clone(), Box::new(source)); - id - } -} - -pub async fn get_connection(pool: &Pool) -> io::Result> { - pool.get() - .await - .map_err(|e| io_error!(e, "Can't retrieve connection from the pool")) -} - -async fn select_postgis_version(pool: &Pool) -> io::Result { - get_connection(pool) - .await? - .query_one(include_str!("scripts/get_postgis_version.sql"), &[]) - .await - .map(|row| row.get::<_, String>("postgis_version")) - .map_err(|e| io_error!(e, "Can't get PostGIS version")) -} diff --git a/src/pg/function_source.rs b/src/pg/function_source.rs index ec4200591..d6d4949a9 100644 --- a/src/pg/function_source.rs +++ b/src/pg/function_source.rs @@ -1,108 +1,17 @@ -use crate::pg::config::{FuncInfoDbMapMap, FunctionInfo, FunctionInfoDbInfo}; -use crate::pg::db::get_connection; -use crate::pg::db::Pool; -use crate::pg::utils::{creat_tilejson, io_error, is_valid_zoom, query_to_json}; -use crate::source::{Source, Tile, UrlQuery, Xyz}; -use async_trait::async_trait; -use bb8_postgres::tokio_postgres::types::ToSql; -use log::{debug, warn}; -use martin_tile_utils::DataFormat; -use postgres::types::Type; +use crate::pg::config::{FunctionInfo, PgSqlInfo, SqlFuncInfoMapMap}; +use crate::pg::connection::Pool; +use crate::pg::utils::io_error; +use log::warn; use postgres_protocol::escape::escape_identifier; use serde_json::Value; use std::collections::HashMap; use std::fmt::Write; use std::io; use std::iter::zip; -use tilejson::TileJSON; -#[derive(Clone, Debug)] -pub struct FunctionSource { - id: String, - info: FunctionInfoDbInfo, - pool: Pool, - tilejson: TileJSON, -} - -impl FunctionSource { - pub fn new(id: String, info: FunctionInfoDbInfo, pool: Pool) -> Self { - let func = &info.info; - Self { - tilejson: creat_tilejson( - format!("{}.{}", func.schema, func.function), - func.minzoom, - func.maxzoom, - func.bounds, - None, - ), - id, - info, - pool, - } - } -} - -#[async_trait] -impl Source for FunctionSource { - fn get_tilejson(&self) -> TileJSON { - self.tilejson.clone() - } - - fn get_format(&self) -> DataFormat { - DataFormat::Mvt - } - - fn clone_source(&self) -> Box { - Box::new(self.clone()) - } - - fn is_valid_zoom(&self, zoom: i32) -> bool { - is_valid_zoom(zoom, self.info.info.minzoom, self.info.info.maxzoom) - } - - async fn get_tile(&self, xyz: &Xyz, url_query: &Option) -> Result { - let empty_query = HashMap::new(); - let url_query = url_query.as_ref().unwrap_or(&empty_query); - let conn = get_connection(&self.pool).await?; - - let param_types: &[Type] = if self.info.has_query_params { - &[Type::INT4, Type::INT4, Type::INT4, Type::JSON] - } else { - &[Type::INT4, Type::INT4, Type::INT4] - }; - - let query = &self.info.query; - let prep_query = conn - .prepare_typed(query, param_types) - .await - .map_err(|e| io_error!(e, "Can't create prepared statement for the tile"))?; - - let tile = if self.info.has_query_params { - let json = query_to_json(url_query); - debug!("SQL: {query} [{}, {}, {}, {json:?}]", xyz.x, xyz.y, xyz.z); - let params: &[&(dyn ToSql + Sync)] = &[&xyz.z, &xyz.x, &xyz.y, &json]; - conn.query_one(&prep_query, params).await - } else { - debug!("SQL: {query} [{}, {}, {}]", xyz.x, xyz.y, xyz.z); - conn.query_one(&prep_query, &[&xyz.z, &xyz.x, &xyz.y]).await - }; - - let tile = tile.map(|row| row.get(0)).map_err(|e| { - if self.info.has_query_params { - let url_q = query_to_json(url_query); - io_error!(e, r#"Can't get {}/{xyz} with {url_q:?} params"#, self.id) - } else { - io_error!(e, r#"Can't get {}/{xyz}"#, self.id) - } - })?; - - Ok(tile) - } -} - -pub async fn get_function_sources(pool: &Pool) -> Result { - let mut res = FuncInfoDbMapMap::new(); - get_connection(pool) +pub async fn get_function_sources(pool: &Pool) -> Result { + let mut res = SqlFuncInfoMapMap::new(); + pool.get() .await? .query(include_str!("scripts/get_function_sources.sql"), &[]) .await @@ -111,7 +20,7 @@ pub async fn get_function_sources(pool: &Pool) -> Result Result { // SELECT mvt FROM "public"."function_zxy_row2"(z => $1::integer, x => $2::integer, y => $3::integer); query.insert_str(0, " FROM "); query.insert_str(0, &escape_identifier(names[0].as_str())); query.insert_str(0, "SELECT "); + format!("[{}]", names.join(", ")) } (_, _) => { query.insert_str(0, "SELECT "); query.push_str(" AS tile"); + output_type } - } - warn!("SQL: {query}"); + }; if let Some(v) = res .entry(schema.clone()) .or_insert_with(HashMap::new) .insert( function.clone(), - FunctionInfoDbInfo { - query, - has_query_params: input_types.len() == 4, - signature: format!("{schema}.{function}({})", input_names.join(", ")), - info: FunctionInfo::new(schema, function), - }, + ( + PgSqlInfo::new( + query, + input_types.len() == 4, + format!( + "{schema}.{function}({}) -> {ret_inf}", + input_names.join(", ") + ), + ), + FunctionInfo::new(schema, function), + ), ) { - warn!("Unexpected duplicate function {}", v.signature); + warn!("Unexpected duplicate function {}", v.0.signature); } }); diff --git a/src/pg/mod.rs b/src/pg/mod.rs index d40526fd9..e6430d3d9 100644 --- a/src/pg/mod.rs +++ b/src/pg/mod.rs @@ -1,5 +1,7 @@ pub mod config; -pub mod db; +pub mod configurator; +pub mod connection; pub mod function_source; +pub mod pg_source; pub mod table_source; pub mod utils; diff --git a/src/pg/pg_source.rs b/src/pg/pg_source.rs new file mode 100644 index 000000000..3f11b6fe6 --- /dev/null +++ b/src/pg/pg_source.rs @@ -0,0 +1,89 @@ +use crate::pg::config::PgSqlInfo; +use crate::pg::connection::Pool; +use crate::pg::utils::{io_error, is_valid_zoom, query_to_json}; +use crate::source::{Source, Tile, UrlQuery, Xyz}; +use async_trait::async_trait; +use bb8_postgres::tokio_postgres::types::ToSql; +use log::debug; +use martin_tile_utils::DataFormat; +use postgres::types::Type; +use std::collections::HashMap; +use std::io; +use tilejson::TileJSON; + +#[derive(Clone, Debug)] +pub struct PgSource { + id: String, + info: PgSqlInfo, + pool: Pool, + tilejson: TileJSON, +} + +impl PgSource { + pub fn new(id: String, info: PgSqlInfo, tilejson: TileJSON, pool: Pool) -> Self { + Self { + tilejson, + id, + info, + pool, + } + } +} + +#[async_trait] +impl Source for PgSource { + fn get_tilejson(&self) -> TileJSON { + self.tilejson.clone() + } + + fn get_format(&self) -> DataFormat { + DataFormat::Mvt + } + + fn clone_source(&self) -> Box { + Box::new(self.clone()) + } + + fn is_valid_zoom(&self, zoom: i32) -> bool { + is_valid_zoom(zoom, self.tilejson.minzoom, self.tilejson.maxzoom) + } + + async fn get_tile(&self, xyz: &Xyz, url_query: &Option) -> Result { + let empty_query = HashMap::new(); + let url_query = url_query.as_ref().unwrap_or(&empty_query); + let conn = self.pool.get().await?; + + let param_types: &[Type] = if self.info.has_query_params { + &[Type::INT4, Type::INT4, Type::INT4, Type::JSON] + } else { + &[Type::INT4, Type::INT4, Type::INT4] + }; + + let query = &self.info.query; + let prep_query = conn + .prepare_typed(query, param_types) + .await + .map_err(|e| io_error!(e, "Can't create prepared statement for the tile"))?; + + let tile = if self.info.has_query_params { + let json = query_to_json(url_query); + debug!("SQL: {query} [{xyz:,>}, {json:?}]"); + let params: &[&(dyn ToSql + Sync)] = &[&xyz.z, &xyz.x, &xyz.y, &json]; + conn.query_one(&prep_query, params).await + } else { + debug!("SQL: {query} [{xyz:,>}]"); + conn.query_one(&prep_query, &[&xyz.z, &xyz.x, &xyz.y]).await + }; + + let tile = tile.map(|row| row.get(0)).map_err(|e| { + if self.info.has_query_params { + let url_q = query_to_json(url_query); + io_error!(e, r#"Can't get {}/{xyz:/>} with {url_q:?} params"#, self.id) + } else { + io_error!(e, r#"Can't get {}/{xyz:/>}"#, self.id) + } + })?; + + Ok(tile) + } +} diff --git a/src/pg/scripts/get_bounds.sql b/src/pg/scripts/get_bounds.sql index 1c7ab4f99..b554db80d 100755 --- a/src/pg/scripts/get_bounds.sql +++ b/src/pg/scripts/get_bounds.sql @@ -1,4 +1,4 @@ -WITH real_bounds AS (SELECT ST_SetSRID(ST_Extent({geometry_column}), {srid}) AS rb FROM {id}) +WITH real_bounds AS (SELECT ST_SetSRID(ST_Extent({geometry_column}), {srid}) AS rb FROM {schema}.{table}) SELECT ST_Transform( CASE WHEN (SELECT ST_GeometryType(rb) FROM real_bounds LIMIT 1) = 'ST_Point' @@ -6,4 +6,4 @@ SELECT ST_Transform( ELSE (SELECT * FROM real_bounds) END , 4326) AS bounds -FROM {id}; +FROM {schema}.{table}; diff --git a/src/pg/scripts/get_bounds_cte.sql b/src/pg/scripts/get_bounds_cte.sql deleted file mode 100644 index f57a453d5..000000000 --- a/src/pg/scripts/get_bounds_cte.sql +++ /dev/null @@ -1 +0,0 @@ -WITH bounds AS (SELECT {srid_bounds}) \ No newline at end of file diff --git a/src/pg/scripts/get_geom.sql b/src/pg/scripts/get_geom.sql deleted file mode 100644 index e080f99d4..000000000 --- a/src/pg/scripts/get_geom.sql +++ /dev/null @@ -1,4 +0,0 @@ -SELECT - ST_AsMVTGeom (ST_Transform (ST_CurveToLine("{geometry_column}"), 3857), ST_TileEnvelope({z}, {x}, {y}), {extent}, {buffer}, {clip_geom}) AS geom {properties} FROM {schema}."{table}", bounds - WHERE - "{geometry_column}" && bounds.srid_{srid} diff --git a/src/pg/scripts/get_srid_bounds.sql b/src/pg/scripts/get_srid_bounds.sql deleted file mode 100644 index 0c69709a8..000000000 --- a/src/pg/scripts/get_srid_bounds.sql +++ /dev/null @@ -1 +0,0 @@ -ST_Transform(ST_TileEnvelope({z}, {x}, {y}), {srid}) AS srid_{srid} diff --git a/src/pg/scripts/get_table_sources.sql b/src/pg/scripts/get_table_sources.sql index 19bf51db4..5e0b81d2d 100755 --- a/src/pg/scripts/get_table_sources.sql +++ b/src/pg/scripts/get_table_sources.sql @@ -10,7 +10,7 @@ WITH columns AS ( JOIN pg_catalog.pg_type AS tp ON tp.oid = attr.atttypid WHERE NOT attr.attisdropped AND attr.attnum > 0) SELECT - f_table_schema, f_table_name, f_geometry_column, srid, type, + f_table_schema schema, f_table_name name, f_geometry_column geom, srid, type, COALESCE( jsonb_object_agg(columns.column_name, columns.type_name) FILTER (WHERE columns.column_name IS NOT NULL), '{}'::jsonb @@ -20,4 +20,4 @@ LEFT JOIN columns ON geometry_columns.f_table_schema = columns.table_schema AND geometry_columns.f_table_name = columns.table_name AND geometry_columns.f_geometry_column != columns.column_name -GROUP BY f_table_schema, f_table_name, f_geometry_column, srid, type; \ No newline at end of file +GROUP BY f_table_schema, f_table_name, f_geometry_column, srid, type; diff --git a/src/pg/scripts/get_tile.sql b/src/pg/scripts/get_tile.sql deleted file mode 100755 index d80dc27d6..000000000 --- a/src/pg/scripts/get_tile.sql +++ /dev/null @@ -1,2 +0,0 @@ -SELECT - ST_AsMVT (tile, '{id}', {extent}, 'geom' {id_column}) FROM ({geom_query}) AS tile diff --git a/src/pg/table_source.rs b/src/pg/table_source.rs index a3cc53dc8..e1ddcc889 100644 --- a/src/pg/table_source.rs +++ b/src/pg/table_source.rs @@ -1,135 +1,10 @@ -use crate::pg::config::{FormatId, TableInfo, TableInfoSources, TableInfoVec}; -use crate::pg::db::get_connection; -use crate::pg::db::Pool; -use crate::pg::utils::{ - creat_tilejson, get_bounds_cte, get_source_bounds, get_srid_bounds, io_error, is_valid_zoom, - json_to_hashmap, polygon_to_bbox, -}; -use crate::source::{Source, Tile, UrlQuery, Xyz}; -use async_trait::async_trait; -use log::{info, warn}; -use martin_tile_utils::DataFormat; -use std::collections::{HashMap, HashSet}; +use crate::pg::config::{PgInfo, PgSqlInfo, SqlTableInfoMapMapMap, TableInfo}; +use crate::pg::connection::Pool; +use crate::pg::utils::{io_error, json_to_hashmap, polygon_to_bbox}; +use log::warn; +use postgres_protocol::escape::{escape_identifier, escape_literal}; +use std::collections::HashMap; use std::io; -use tilejson::{TileJSON, VectorLayer}; - -#[derive(Clone, Debug)] -pub struct TableSource { - pub id: String, - pub info: TableInfo, - pool: Pool, - tilejson: TileJSON, -} - -pub type TableSources = HashMap>; - -impl TableSource { - pub fn new(id: String, info: TableInfo, pool: Pool) -> Self { - let mut layer = VectorLayer::new(id.clone(), info.properties.clone()); - layer.minzoom = info.minzoom; - layer.maxzoom = info.maxzoom; - Self { - tilejson: creat_tilejson( - format!("{}.{}.{}", info.schema, info.table, info.geometry_column), - info.minzoom, - info.maxzoom, - info.bounds, - Some(vec![layer]), - ), - id, - info, - pool, - } - } - - pub fn get_geom_query(&self, xyz: &Xyz) -> String { - let info = &self.info; - let properties = if info.properties.is_empty() { - String::new() - } else { - let properties = info - .properties - .keys() - .map(|column| format!(r#""{column}""#)) - .collect::>() - .join(","); - - format!(", {properties}") - }; - - format!( - include_str!("scripts/get_geom.sql"), - schema = info.schema, - table = info.table, - srid = info.srid, - geometry_column = info.geometry_column, - z = xyz.z, - x = xyz.x, - y = xyz.y, - extent = info.extent.unwrap_or(DEFAULT_EXTENT), - buffer = info.buffer.unwrap_or(DEFAULT_BUFFER), - clip_geom = info.clip_geom.unwrap_or(DEFAULT_CLIP_GEOM), - properties = properties - ) - } - - pub fn get_tile_query(&self, xyz: &Xyz) -> String { - let geom_query = self.get_geom_query(xyz); - - let id_column = self - .info - .id_column - .clone() - .map_or(String::new(), |id_column| format!(", '{id_column}'")); - - format!( - include_str!("scripts/get_tile.sql"), - id = self.id, - id_column = id_column, - geom_query = geom_query, - extent = self.info.extent.unwrap_or(DEFAULT_EXTENT), - ) - } - - pub fn build_tile_query(&self, xyz: &Xyz) -> String { - let srid_bounds = get_srid_bounds(self.info.srid, xyz); - let bounds_cte = get_bounds_cte(&srid_bounds); - let tile_query = self.get_tile_query(xyz); - - format!("{bounds_cte} {tile_query}") - } -} - -#[async_trait] -impl Source for TableSource { - fn get_tilejson(&self) -> TileJSON { - self.tilejson.clone() - } - - fn get_format(&self) -> DataFormat { - DataFormat::Mvt - } - - fn clone_source(&self) -> Box { - Box::new(self.clone()) - } - - fn is_valid_zoom(&self, zoom: i32) -> bool { - is_valid_zoom(zoom, self.info.minzoom, self.info.maxzoom) - } - - async fn get_tile(&self, xyz: &Xyz, _query: &Option) -> Result { - let tile_query = self.build_tile_query(xyz); - let conn = get_connection(&self.pool).await?; - let tile: Tile = conn - .query_one(tile_query.as_str(), &[]) - .await - .map(|row| row.get("st_asmvt")) - .map_err(|error| io_error!(error, r#"Can't get "{}" tile at {xyz}"#, self.id))?; - - Ok(tile) - } -} static DEFAULT_EXTENT: u32 = 4096; static DEFAULT_BUFFER: u32 = 64; @@ -137,60 +12,58 @@ static DEFAULT_CLIP_GEOM: bool = true; pub async fn get_table_sources( pool: &Pool, - explicit_tables: &TableInfoSources, default_srid: Option, -) -> Result { - let skip_tables: HashSet = explicit_tables.values().map(|v| v.format_id("")).collect(); - let conn = get_connection(pool).await?; +) -> Result { + let conn = pool.get().await?; let rows = conn .query(include_str!("scripts/get_table_sources.sql"), &[]) .await .map_err(|e| io_error!(e, "Can't get table sources"))?; - let mut result = TableInfoVec::default(); + let mut res = SqlTableInfoMapMapMap::new(); for row in &rows { - let schema: String = row.get("f_table_schema"); - let table: String = row.get("f_table_name"); - let geometry_column: String = row.get("f_geometry_column"); - let srid: i32 = row.get("srid"); - let mut info = TableInfo { - schema, - table, - id_column: None, - geometry_column, - bounds: None, - minzoom: None, - maxzoom: None, - srid: srid as u32, + schema: row.get("schema"), + table: row.get("name"), + geometry_column: row.get("geom"), + srid: 0, extent: Some(DEFAULT_EXTENT), buffer: Some(DEFAULT_BUFFER), clip_geom: Some(DEFAULT_CLIP_GEOM), geometry_type: row.get("type"), properties: json_to_hashmap(&row.get("properties")), unrecognized: HashMap::new(), + ..TableInfo::default() }; - if skip_tables.contains(&info.format_id("")) { - continue; - } - - // FIXME: in theory, schema or table can be arbitrary, and thus may cause SQL injection - let table_id = format!("{}.{}", info.schema, info.table); - - if srid == 0 { - if let Some(default_srid) = default_srid { + let table_id = info.format_id(); + let srid: i32 = row.get("srid"); + info.srid = match (srid, default_srid) { + (0, Some(default_srid)) => { warn!(r#""{table_id}" has SRID 0, using the provided default SRID {default_srid}"#); - info.srid = default_srid as u32; - } else { - warn!( - r#""{table_id}" has SRID 0, skipping. To use this table source, you must specify the SRID using the config file or provide the default SRID"# - ); + default_srid as u32 + } + (0, None) => { + let info = "To use this table source, you must specify the SRID using the config file or provide the default SRID"; + warn!(r#""{table_id}" has SRID 0, skipping. {info}"#); continue; } - } + (srid, _) if srid < 0 => { + // TODO: why do we even use signed SRIDs? + warn!("Skipping unexpected srid {srid} for {table_id}"); + continue; + } + (srid, _) => srid as u32, + }; + + let bounds_query = format!( + include_str!("scripts/get_bounds.sql"), + schema = info.schema, + table = info.table, + srid = info.srid, + geometry_column = info.geometry_column, + ); - let bounds_query = get_source_bounds(&table_id, srid as u32, &info.geometry_column); info.bounds = conn .query_one(bounds_query.as_str(), &[]) .await @@ -199,13 +72,64 @@ pub async fn get_table_sources( .flatten() .and_then(|v| polygon_to_bbox(&v)); - result.push(info); - } - result.sort_by_key(|v| v.table.clone()); + let properties = if info.properties.is_empty() { + String::new() + } else { + let properties = info + .properties + .keys() + .map(|column| format!(r#""{column}""#)) + .collect::>() + .join(","); + format!(", {properties}") + }; + + let id_column = info + .id_column + .clone() + .map_or(String::new(), |id_column| format!(", '{id_column}'")); - if result.is_empty() { - info!("No table sources found"); + let query = format!( + r#" +SELECT + ST_AsMVT(tile, {table_id}, {extent}, 'geom' {id_column}) +FROM ( + SELECT + ST_AsMVTGeom( + ST_Transform(ST_CurveToLine({geometry_column}), 3857), + ST_TileEnvelope($1::integer, $2::integer, $3::integer), + {extent}, {buffer}, {clip_geom} + ) AS geom + {properties} + FROM + {schema}.{table} + WHERE + {geometry_column} && ST_Transform(ST_TileEnvelope($1::integer, $2::integer, $3::integer), {srid}) +) AS tile +"#, + table_id = escape_literal(table_id.as_str()), + extent = info.extent.unwrap_or(DEFAULT_EXTENT), + geometry_column = escape_identifier(&info.geometry_column), + buffer = info.buffer.unwrap_or(DEFAULT_BUFFER), + clip_geom = info.clip_geom.unwrap_or(DEFAULT_CLIP_GEOM), + schema = escape_identifier(&info.schema), + table = escape_identifier(&info.table), + srid = info.srid, + ).trim().to_string(); + + if let Some(v) = res + .entry(info.schema.clone()) + .or_insert_with(HashMap::new) + .entry(info.table.clone()) + .or_insert_with(HashMap::new) + .insert( + info.geometry_column.clone(), + (PgSqlInfo::new(query, false, table_id), info), + ) + { + warn!("Unexpected duplicate function {}", v.0.signature); + } } - Ok(result) + Ok(res) } diff --git a/src/pg/utils.rs b/src/pg/utils.rs index fd9d1f448..40bf80584 100755 --- a/src/pg/utils.rs +++ b/src/pg/utils.rs @@ -1,4 +1,4 @@ -use crate::source::{UrlQuery, Xyz}; +use crate::source::UrlQuery; use actix_http::header::HeaderValue; use actix_web::http::Uri; use postgis::{ewkb, LineString, Point, Polygon}; @@ -42,32 +42,6 @@ pub fn query_to_json(query: &UrlQuery) -> Json> { Json(query_as_json) } -pub fn get_bounds_cte(srid_bounds: &str) -> String { - format!( - include_str!("scripts/get_bounds_cte.sql"), - srid_bounds = srid_bounds - ) -} - -pub fn get_srid_bounds(srid: u32, xyz: &Xyz) -> String { - format!( - include_str!("scripts/get_srid_bounds.sql"), - z = xyz.z, - x = xyz.x, - y = xyz.y, - srid = srid, - ) -} - -pub fn get_source_bounds(id: &str, srid: u32, geometry_column: &str) -> String { - format!( - include_str!("scripts/get_bounds.sql"), - id = id, - srid = srid, - geometry_column = geometry_column, - ) -} - pub fn polygon_to_bbox(polygon: &ewkb::Polygon) -> Option { polygon.rings().next().and_then(|linestring| { let mut points = linestring.points(); @@ -92,7 +66,7 @@ pub fn parse_x_rewrite_url(header: &HeaderValue) -> Option { .map(|uri| uri.path().to_owned()) } -pub fn creat_tilejson( +pub fn create_tilejson( name: String, minzoom: Option, maxzoom: Option, diff --git a/src/source.rs b/src/source.rs index e61503327..fe2c1d711 100644 --- a/src/source.rs +++ b/src/source.rs @@ -15,9 +15,21 @@ pub struct Xyz { pub y: i32, } +impl Xyz { + pub fn new(z: i32, x: i32, y: i32) -> Self { + Self { z, x, y } + } +} + impl Display for Xyz { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - write!(f, "{}/{}/{}", self.z, self.x, self.y) + match f.fill() { + ',' => write!(f, "{},{},{}", self.z, self.x, self.y), + '/' => write!(f, "{}/{}/{}", self.z, self.x, self.y), + v => { + panic!("Invalid fill character '{v}' -- must be either ',' or '/'. Use {{xyz:/>}} or {{xyz:,>}}") + } + } } } @@ -83,7 +95,7 @@ impl IdResolver { let mut new_name = String::new(); loop { new_name.clear(); - write!(&mut new_name, "{}.{}", name, index).unwrap(); + write!(&mut new_name, "{name}.{index}").unwrap(); index = index.checked_add(1).unwrap(); match names.entry(new_name.clone()) { Entry::Vacant(e) => { diff --git a/src/srv/server.rs b/src/srv/server.rs index 561629b60..a50d1744a 100755 --- a/src/srv/server.rs +++ b/src/srv/server.rs @@ -243,11 +243,7 @@ async fn get_tile( let (sources, format) = state.get_sources(&path.source_ids, Some(path.z))?; let query = Some(query.into_inner()); - let xyz = Xyz { - z: path.z, - x: path.x, - y: path.y, - }; + let xyz = Xyz::new(path.z, path.x, path.y); let tile = try_join_all(sources.into_iter().map(|s| s.get_tile(&xyz, &query))) .await diff --git a/tests/function_source_test.rs b/tests/function_source_test.rs index a0d6c6cee..5f176803b 100644 --- a/tests/function_source_test.rs +++ b/tests/function_source_test.rs @@ -1,6 +1,6 @@ use ctor::ctor; use log::info; -use martin::pg::function_source::get_function_sources as get_sources; +use martin::pg::function_source::get_function_sources; use martin::source::Xyz; #[path = "utils.rs"] @@ -13,22 +13,21 @@ fn init() { } #[actix_rt::test] -async fn get_function_sources() { +async fn get_function_sources_ok() { let pool = mock_pool().await; - let sources = get_sources(&pool).await.unwrap(); + let sources = get_function_sources(&pool).await.unwrap(); - // dbg!(&sources); assert!(!sources.is_empty()); let funcs = sources.get("public").expect("public schema not found"); let source = funcs .get("function_zxy_query") .expect("function_zxy_query not found"); - assert_eq!(source.info.schema, "public"); - assert_eq!(source.info.function, "function_zxy_query"); - assert_eq!(source.info.minzoom, None); - assert_eq!(source.info.maxzoom, None); - assert_eq!(source.info.bounds, None); + assert_eq!(source.1.schema, "public"); + assert_eq!(source.1.function, "function_zxy_query"); + assert_eq!(source.1.minzoom, None); + assert_eq!(source.1.maxzoom, None); + assert_eq!(source.1.bounds, None); } #[actix_rt::test] @@ -53,10 +52,7 @@ async fn function_source_tilejson() { async fn function_source_tile() { let sources = mock_sources(None, None).await; let source = sources.get("function_zxy_query").unwrap(); - let tile = source - .get_tile(&Xyz { x: 0, y: 0, z: 0 }, &None) - .await - .unwrap(); + let tile = source.get_tile(&Xyz::new(0, 0, 0), &None).await.unwrap(); assert!(!tile.is_empty()); } diff --git a/tests/server_test.rs b/tests/server_test.rs index 5700d4213..40c383600 100644 --- a/tests/server_test.rs +++ b/tests/server_test.rs @@ -33,8 +33,8 @@ fn test_get(path: &str) -> Request { } #[actix_rt::test] -async fn get_table_catalog_ok() { - let app = create_app!(mock_default_table_sources()); +async fn get_catalog_ok() { + let app = create_app!(mock_sources(None, None)); let req = test_get("/catalog"); let response = call_service(&app, req).await; @@ -42,22 +42,12 @@ async fn get_table_catalog_ok() { let body = read_body(response).await; let sources: Vec = serde_json::from_slice(&body).unwrap(); - let expected = "table_source"; - assert_eq!(sources.into_iter().filter(|v| v.id == expected).count(), 1); -} - -#[actix_rt::test] -async fn get_function_catalog_ok() { - let app = create_app!(mock_sources(None, None)); - let req = test_get("/catalog"); - let response = call_service(&app, req).await; - assert!(response.status().is_success()); + let expected = "table_source"; + assert_eq!(sources.iter().filter(|v| v.id == expected).count(), 1); - let body = read_body(response).await; - let sources: Vec = serde_json::from_slice(&body).unwrap(); let expected = "function_zxy_query"; - assert_eq!(sources.into_iter().filter(|v| v.id == expected).count(), 1); + assert_eq!(sources.iter().filter(|v| v.id == expected).count(), 1); } #[actix_rt::test] diff --git a/tests/table_source_test.rs b/tests/table_source_test.rs index f64f42fa8..add0bb341 100644 --- a/tests/table_source_test.rs +++ b/tests/table_source_test.rs @@ -1,6 +1,6 @@ use ctor::ctor; use log::info; -use martin::pg::config::{TableInfo, TableInfoSources, TableInfoVec}; +use martin::pg::config::{PgSqlInfo, SqlTableInfoMapMapMap, TableInfo}; use martin::pg::table_source::get_table_sources; use martin::source::Xyz; use std::collections::HashMap; @@ -17,30 +17,26 @@ fn init() { #[actix_rt::test] async fn get_table_sources_ok() { let pool = mock_pool().await; - let table_sources = get_table_sources(&pool, &TableInfoSources::default(), None) - .await - .unwrap(); - - info!("table_sources = {table_sources:#?}"); - - assert!(!table_sources.is_empty()); - let table_source = get_source(&table_sources, "table_source"); - assert_eq!(table_source.schema, "public"); - assert_eq!(table_source.table, "table_source"); - assert_eq!(table_source.srid, 4326); - assert_eq!(table_source.geometry_column, "geom"); - assert_eq!(table_source.id_column, None); - assert_eq!(table_source.minzoom, None); - assert_eq!(table_source.maxzoom, None); - assert!(table_source.bounds.is_some()); - assert_eq!(table_source.extent, Some(4096)); - assert_eq!(table_source.buffer, Some(64)); - assert_eq!(table_source.clip_geom, Some(true)); - assert_eq!(table_source.geometry_type, Some("GEOMETRY".to_owned())); + let sources = get_table_sources(&pool, Some(900913)).await.unwrap(); + assert!(!sources.is_empty()); + + let (_, source) = get_source(&sources, "table_source", "geom"); + assert_eq!(source.schema, "public"); + assert_eq!(source.table, "table_source"); + assert_eq!(source.srid, 4326); + assert_eq!(source.geometry_column, "geom"); + assert_eq!(source.id_column, None); + assert_eq!(source.minzoom, None); + assert_eq!(source.maxzoom, None); + assert!(source.bounds.is_some()); + assert_eq!(source.extent, Some(4096)); + assert_eq!(source.buffer, Some(64)); + assert_eq!(source.clip_geom, Some(true)); + assert_eq!(source.geometry_type, Some("GEOMETRY".to_owned())); let mut properties = HashMap::new(); properties.insert("gid".to_owned(), "int4".to_owned()); - assert_eq!(table_source.properties, properties); + assert_eq!(source.properties, properties); } #[actix_rt::test] @@ -65,10 +61,7 @@ async fn table_source_tilejson_ok() { async fn table_source_tile_ok() { let sources = mock_sources(None, None).await; let source = sources.get("table_source").unwrap(); - let tile = source - .get_tile(&Xyz { x: 0, y: 0, z: 0 }, &None) - .await - .unwrap(); + let tile = source.get_tile(&Xyz::new(0, 0, 0), &None).await.unwrap(); assert!(!tile.is_empty()); } @@ -76,43 +69,42 @@ async fn table_source_tile_ok() { #[actix_rt::test] async fn table_source_srid_ok() { let pool = mock_pool().await; - let table_sources = get_table_sources(&pool, &TableInfoSources::default(), Some(900913)) - .await - .unwrap(); + let table_sources = get_table_sources(&pool, Some(900913)).await.unwrap(); - let points1 = get_source(&table_sources, "points1"); - assert_eq!(points1.srid, 4326); + let (_, source) = get_source(&table_sources, "points1", "geom"); + assert_eq!(source.srid, 4326); - let points2 = get_source(&table_sources, "points2"); - assert_eq!(points2.srid, 4326); + let (_, source) = get_source(&table_sources, "points2", "geom"); + assert_eq!(source.srid, 4326); - let points3857 = get_source(&table_sources, "points3857"); - assert_eq!(points3857.srid, 3857); + let (_, source) = get_source(&table_sources, "points3857", "geom"); + assert_eq!(source.srid, 3857); - let points_empty_srid = get_source(&table_sources, "points_empty_srid"); - assert_eq!(points_empty_srid.srid, 900913); + let (_, source) = get_source(&table_sources, "points_empty_srid", "geom"); + assert_eq!(source.srid, 900913); } #[actix_rt::test] async fn table_source_multiple_geom_ok() { let pool = mock_pool().await; - let table_sources = get_table_sources(&pool, &TableInfoSources::default(), None) - .await - .unwrap(); - - let table_source_multiple_geom = single(&table_sources, |v| { - v.table == "table_source_multiple_geom" && v.geometry_column == "geom1" - }) - .expect("table_source_multiple_geom.geom1 not found"); - assert_eq!(table_source_multiple_geom.geometry_column, "geom1"); - - let table_source_multiple_geom = single(&table_sources, |v| { - v.table == "table_source_multiple_geom" && v.geometry_column == "geom2" - }) - .expect("table_source_multiple_geom.geom2 not found"); - assert_eq!(table_source_multiple_geom.geometry_column, "geom2"); + let sources = get_table_sources(&pool, Some(900913)).await.unwrap(); + + let (_, source) = get_source(&sources, "table_source_multiple_geom", "geom1"); + assert_eq!(source.geometry_column, "geom1"); + + let (_, source) = get_source(&sources, "table_source_multiple_geom", "geom2"); + assert_eq!(source.geometry_column, "geom2"); } -fn get_source<'a>(table_sources: &'a TableInfoVec, name: &'static str) -> &'a TableInfo { - single(table_sources, |v| v.table == *name).unwrap_or_else(|| panic!("{name} not found")) +fn get_source<'a>( + sources: &'a SqlTableInfoMapMapMap, + name: &'static str, + geom: &'static str, +) -> &'a (PgSqlInfo, TableInfo) { + let srcs = sources.get("public").expect("public schema not found"); + let cols = srcs + .get(name) + .unwrap_or_else(|| panic!("table {name} not found")); + cols.get(geom) + .unwrap_or_else(|| panic!("table {name}.{geom} not found")) } diff --git a/tests/utils.rs b/tests/utils.rs index e92020f0e..17bbade7c 100644 --- a/tests/utils.rs +++ b/tests/utils.rs @@ -2,7 +2,8 @@ use actix_web::web::Data; use log::info; use martin::pg::config::{FunctionInfo, PgConfigBuilder}; use martin::pg::config::{PgConfig, TableInfo}; -use martin::pg::db::{make_pool, resolve_pg_data, Pool}; +use martin::pg::configurator::resolve_pg_data; +use martin::pg::connection::Pool; use martin::source::IdResolver; use martin::srv::server::{AppState, Sources}; use std::collections::HashMap; @@ -16,7 +17,7 @@ use tilejson::Bounds; #[allow(dead_code)] pub async fn mock_pool() -> Pool { - let res = make_pool(mock_config(None, None).await).await; + let res = Pool::new(&mock_config(None, None).await).await; res.expect("Failed to create pool") } From aa9023a59e43498427f743d4cf9c77c6f598b01f Mon Sep 17 00:00:00 2001 From: Yuri Astrakhan Date: Sat, 3 Dec 2022 23:39:59 -0500 Subject: [PATCH 05/16] More fixes BREAKING: * srid is now the same type as PG -- i32 * renamed config vals `table_sources` and `function_sources` into `tables` and `functions` Fixes: * predictable order of instantiation * bounding boxes computed in parallel for all tables (when not configured) * split up discovery and query creation - this way user overrides happen before the final query is generated * more proper name escaping * lots of test improvements --- Cargo.toml | 1 + README.md | 4 +- justfile | 6 +- src/config.rs | 8 +- src/pg/config.rs | 58 ++---- src/pg/configurator.rs | 235 +++++++++++++++-------- src/pg/function_source.rs | 4 +- src/pg/pg_source.rs | 18 +- src/pg/table_source.rs | 160 ++++++++-------- src/srv/server.rs | 2 +- tests/config.yaml | 4 +- tests/function_source_test.rs | 11 +- tests/server_test.rs | 197 +++++++------------ tests/table_source_test.rs | 55 ++---- tests/utils.rs | 347 +++++++++++++++++++++++----------- 15 files changed, 620 insertions(+), 490 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 5c81797a9..7a0adb9ca 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -52,6 +52,7 @@ tilejson = "0.3" [dev-dependencies] ctor = "0.1" indoc = "1" +#test-log = "0.2" [dev-dependencies.criterion] version = "0.4.0" diff --git a/README.md b/README.md index 680609948..da50f3e72 100755 --- a/README.md +++ b/README.md @@ -483,7 +483,7 @@ pool_size: 20 worker_processes: 8 # Associative arrays of table sources -table_sources: +tables: table_source_id: # Table schema (required) schema: public @@ -529,7 +529,7 @@ table_sources: gid: int4 # Associative arrays of function sources -function_sources: +functions: function_source_id: # Schema name (required) schema: public diff --git a/justfile b/justfile index 6b9cadc91..a9347ded2 100644 --- a/justfile +++ b/justfile @@ -50,9 +50,9 @@ bench: start-db test: test-unit test-int # Run Rust unit and doc tests (cargo test) -test-unit: start-db - cargo test --all-targets - cargo test --all-targets --all-features +test-unit *ARGS: start-db + cargo test --all-targets {{ARGS}} + cargo test --all-targets --all-features {{ARGS}} cargo test --doc # Run integration tests diff --git a/src/config.rs b/src/config.rs index 6270e3859..caa962774 100644 --- a/src/config.rs +++ b/src/config.rs @@ -89,7 +89,7 @@ mod tests { pool_size: 20 worker_processes: 8 - table_sources: + tables: table_source: schema: public table: table_source @@ -106,7 +106,7 @@ mod tests { properties: gid: int4 - function_sources: + functions: function_zxy_query: schema: public function: function_zxy_query @@ -133,7 +133,7 @@ mod tests { pool_size: 20, discover_functions: false, discover_tables: false, - table_sources: HashMap::from([( + tables: HashMap::from([( "table_source".to_string(), TableInfo { schema: "public".to_string(), @@ -152,7 +152,7 @@ mod tests { unrecognized: HashMap::new(), }, )]), - function_sources: HashMap::from([( + functions: HashMap::from([( "function_zxy_query".to_string(), FunctionInfo::new_extended( "public".to_string(), diff --git a/src/pg/config.rs b/src/pg/config.rs index 544bebe9e..4e111800f 100644 --- a/src/pg/config.rs +++ b/src/pg/config.rs @@ -35,7 +35,7 @@ pub struct TableInfo { pub table: String, /// Geometry SRID - pub srid: u32, + pub srid: i32, /// Geometry column name pub geometry_column: String, @@ -98,23 +98,6 @@ impl PgInfo for TableInfo { } } -#[derive(Clone, Debug)] -pub struct PgSqlInfo { - pub query: String, - pub has_query_params: bool, - pub signature: String, -} - -impl PgSqlInfo { - pub fn new(query: String, has_query_params: bool, signature: String) -> Self { - Self { - query, - has_query_params, - signature, - } - } -} - pub trait PgInfo { fn format_id(&self) -> String; fn to_tilejson(&self) -> TileJSON; @@ -190,12 +173,9 @@ impl PgInfo for FunctionInfo { } } -pub type TableInfoSources = HashMap; -pub type FuncInfoSources = HashMap; pub type InfoMap = HashMap; -pub type SqlFuncInfoMapMap = HashMap>; -pub type SqlTableInfoMapMapMap = - HashMap>>; +pub type TableInfoSources = InfoMap; +pub type FuncInfoSources = InfoMap; #[derive(Clone, Debug, Serialize, PartialEq)] pub struct PgConfig { @@ -212,8 +192,8 @@ pub struct PgConfig { pub discover_functions: bool, #[serde(skip_serializing)] pub discover_tables: bool, - pub table_sources: TableInfoSources, - pub function_sources: FuncInfoSources, + pub tables: TableInfoSources, + pub functions: FuncInfoSources, } #[derive(Debug, Default, PartialEq, Deserialize)] @@ -225,8 +205,8 @@ pub struct PgConfigBuilder { pub danger_accept_invalid_certs: Option, pub default_srid: Option, pub pool_size: Option, - pub table_sources: Option, - pub function_sources: Option, + pub tables: Option, + pub functions: Option, } impl PgConfigBuilder { @@ -241,21 +221,21 @@ impl PgConfigBuilder { ); set_option(&mut self.default_srid, other.default_srid); set_option(&mut self.pool_size, other.pool_size); - set_option(&mut self.table_sources, other.table_sources); - set_option(&mut self.function_sources, other.function_sources); + set_option(&mut self.tables, other.tables); + set_option(&mut self.functions, other.functions); self } /// Apply defaults to the config, and validate if there is a connection string pub fn finalize(self) -> io::Result { - if let Some(ref ts) = self.table_sources { + if let Some(ref ts) = self.tables { for (k, v) in ts { - report_unrecognized_config(&format!("table_sources.{}.", k), &v.unrecognized); + report_unrecognized_config(&format!("tables.{}.", k), &v.unrecognized); } } - if let Some(ref fs) = self.function_sources { + if let Some(ref fs) = self.functions { for (k, v) in fs { - report_unrecognized_config(&format!("function_sources.{}.", k), &v.unrecognized); + report_unrecognized_config(&format!("functions.{}.", k), &v.unrecognized); } } let connection_string = self.connection_string.ok_or_else(|| { @@ -272,10 +252,10 @@ impl PgConfigBuilder { danger_accept_invalid_certs: self.danger_accept_invalid_certs.unwrap_or_default(), default_srid: self.default_srid, pool_size: self.pool_size.unwrap_or(POOL_SIZE_DEFAULT), - discover_functions: self.table_sources.is_none() && self.function_sources.is_none(), - discover_tables: self.table_sources.is_none() && self.function_sources.is_none(), - table_sources: self.table_sources.unwrap_or_default(), - function_sources: self.function_sources.unwrap_or_default(), + discover_functions: self.tables.is_none() && self.functions.is_none(), + discover_tables: self.tables.is_none() && self.functions.is_none(), + tables: self.tables.unwrap_or_default(), + functions: self.functions.unwrap_or_default(), }) } } @@ -306,8 +286,8 @@ impl From<(PgArgs, Option)> for PgConfigBuilder { }) }), pool_size: args.pool_size, - table_sources: None, - function_sources: None, + tables: None, + functions: None, } } } diff --git a/src/pg/configurator.rs b/src/pg/configurator.rs index c4d1bbec9..497319ec8 100755 --- a/src/pg/configurator.rs +++ b/src/pg/configurator.rs @@ -1,14 +1,16 @@ -use crate::pg::config::FunctionInfo; -use crate::pg::config::TableInfo; -use crate::pg::config::{FuncInfoSources, InfoMap, PgConfig, PgInfo, PgSqlInfo, TableInfoSources}; +use crate::pg::config::{ + FuncInfoSources, FunctionInfo, InfoMap, PgConfig, PgInfo, TableInfo, TableInfoSources, +}; use crate::pg::connection::Pool; use crate::pg::function_source::get_function_sources; -use crate::pg::pg_source::PgSource; -use crate::pg::table_source::get_table_sources; +use crate::pg::pg_source::{PgSource, PgSqlInfo}; +use crate::pg::table_source::{get_table_sources, table_to_query}; use crate::source::IdResolver; use crate::srv::server::Sources; -use futures::future::try_join; -use log::{debug, info, warn}; +use futures::future::{join_all, try_join}; +use itertools::Itertools; +use log::{debug, error, info, warn}; +use std::cmp::Ordering; use std::collections::HashMap; use std::io; @@ -16,7 +18,7 @@ pub async fn resolve_pg_data( config: PgConfig, id_resolver: IdResolver, ) -> io::Result<(Sources, PgConfig, Pool)> { - let pg = PgConfigurator::new(&config, id_resolver).await?; + let pg = PgBuilder::new(&config, id_resolver).await?; let ((mut tables, tbl_info), (funcs, func_info)) = try_join(pg.instantiate_tables(), pg.instantiate_functions()).await?; @@ -24,25 +26,25 @@ pub async fn resolve_pg_data( Ok(( tables, PgConfig { - table_sources: tbl_info, - function_sources: func_info, + tables: tbl_info, + functions: func_info, ..config }, pg.pool, )) } -struct PgConfigurator { +struct PgBuilder { pool: Pool, default_srid: Option, discover_functions: bool, discover_tables: bool, id_resolver: IdResolver, - table_sources: TableInfoSources, - function_sources: FuncInfoSources, + tables: TableInfoSources, + functions: FuncInfoSources, } -impl PgConfigurator { +impl PgBuilder { async fn new(config: &PgConfig, id_resolver: IdResolver) -> io::Result { let pool = Pool::new(config).await?; Ok(Self { @@ -51,94 +53,124 @@ impl PgConfigurator { discover_functions: config.discover_functions, discover_tables: config.discover_tables, id_resolver, - table_sources: config.table_sources.clone(), - function_sources: config.function_sources.clone(), + tables: config.tables.clone(), + functions: config.functions.clone(), }) } - async fn instantiate_tables(&self) -> Result<(Sources, InfoMap), io::Error> { - let mut res: Sources = HashMap::new(); - let mut info_map = InfoMap::new(); - let mut discovered_sources = get_table_sources(&self.pool, self.default_srid).await?; + pub async fn instantiate_tables(&self) -> Result<(Sources, TableInfoSources), io::Error> { + let mut info_map = TableInfoSources::new(); + let mut discovered_sources = get_table_sources(&self.pool).await?; + let mut used = SqlTableInfoMapMapMap::new(); + let mut pending = Vec::new(); - for (id, src_inf) in &self.table_sources { - if let Some((pg_sql, _)) = discovered_sources - .get_mut(&src_inf.schema) - .and_then(|v| v.get_mut(&src_inf.table)) - .and_then(|v| v.remove(&src_inf.geometry_column)) + // First match configured sources with the discovered ones and add them to the pending list. + // Note that multiple configured sources could map to a single discovered one. + // After that, add the remaining discovered sources to the pending list if auto-config is enabled. + for (id, cfg_inf) in &self.tables { + if let Some(src_inf) = discovered_sources + .get_mut(&cfg_inf.schema) + .and_then(|v| v.get_mut(&cfg_inf.table)) + .and_then(|v| v.remove(&cfg_inf.geometry_column)) { - let id2 = self.resolve_id(id.clone(), src_inf); - self.add_func_src(&mut res, id2.clone(), src_inf, pg_sql.clone()); + // Store it just in case another source needs the same table + used.entry(src_inf.schema.to_string()) + .or_default() + .entry(src_inf.table.to_string()) + .or_default() + .insert(src_inf.geometry_column.to_string(), src_inf.clone()); + + let id2 = self.resolve_id(id.clone(), cfg_inf); + let Some(cfg_inf) = self.merge_info(&id2, cfg_inf, &src_inf) else {continue}; warn_on_rename(id, &id2, "table"); - info!("Configured source {id2} from table {}", pg_sql.signature); - debug!("{}", pg_sql.query); - info_map.insert(id2, src_inf.clone()); + info!("Configured source {id2} from {}", summary(&cfg_inf)); + pending.push(table_to_query(id2, cfg_inf, self.pool.clone())); + } else if let Some(src_inf) = used + .get_mut(&cfg_inf.schema) + .and_then(|v| v.get(&cfg_inf.table)) + .and_then(|v| v.get(&cfg_inf.geometry_column)) + { + // This table was already used by another source + let id2 = self.resolve_id(id.clone(), cfg_inf); + let Some(cfg_inf) = self.merge_info(&id2, cfg_inf, src_inf) else {continue}; + warn_on_rename(id, &id2, "table"); + let info = summary(&cfg_inf); + info!("Configured duplicate source {id2} from {info}"); + pending.push(table_to_query(id2, cfg_inf, self.pool.clone())); } else { warn!( "Configured table source {id} as {}.{} with geo column {} does not exist", - src_inf.schema, src_inf.table, src_inf.geometry_column + cfg_inf.schema, cfg_inf.table, cfg_inf.geometry_column ); } } if self.discover_tables { - for tables in discovered_sources.into_values() { - for geoms in tables.into_values() { - for (pg_sql, src_inf) in geoms.into_values() { - let table = &src_inf.table; - let id2 = self.resolve_id(table.clone(), &src_inf); - self.add_func_src(&mut res, id2.clone(), &src_inf, pg_sql.clone()); - info!( - "Discovered source {id2} from table {}.{} with {} column ({}, SRID={})", - src_inf.schema, - src_inf.table, - src_inf.geometry_column, - src_inf.geometry_type.as_deref().unwrap_or("UNKNOWN"), - src_inf.srid, - ); - debug!("{}", pg_sql.query); - info_map.insert(id2, src_inf); + // Sort the discovered sources by schema, table and geometry column to ensure a consistent behavior + for (_, tables) in discovered_sources.into_iter().sorted_by(by_key) { + for (_, geoms) in tables.into_iter().sorted_by(by_key) { + for (_, src_inf) in geoms.into_iter().sorted_by(by_key) { + let id2 = self.resolve_id(src_inf.table.clone(), &src_inf); + let Some(cfg_inf) = self.merge_info(&id2, &src_inf, &src_inf) else {continue}; + info!("Discovered source {id2} from {}", summary(&cfg_inf)); + pending.push(table_to_query(id2, cfg_inf, self.pool.clone())); } } } } + let mut res: Sources = HashMap::new(); + let pending = join_all(pending).await; + for src in pending { + match src { + Err(v) => { + error!("Failed to create a source: {v}"); + continue; + } + Ok((id, pg_sql, src_inf)) => { + debug!("{id} query: {}", pg_sql.query); + self.add_func_src(&mut res, id.clone(), &src_inf, pg_sql.clone()); + info_map.insert(id, src_inf); + } + } + } + Ok((res, info_map)) } - async fn instantiate_functions(&self) -> Result<(Sources, InfoMap), io::Error> { + pub async fn instantiate_functions(&self) -> Result<(Sources, FuncInfoSources), io::Error> { let mut discovered_sources = get_function_sources(&self.pool).await?; let mut res: Sources = HashMap::new(); - let mut info_map = InfoMap::new(); - let mut used_funcs: HashMap> = HashMap::new(); + let mut info_map = FuncInfoSources::new(); + let mut used: HashMap> = HashMap::new(); - for (id, src_inf) in &self.function_sources { - let schema = &src_inf.schema; - let name = &src_inf.function; + for (id, cfg_inf) in &self.functions { + let schema = &cfg_inf.schema; + let name = &cfg_inf.function; if let Some((pg_sql, _)) = discovered_sources .get_mut(schema) .and_then(|v| v.remove(name)) { - let id2 = self.resolve_id(id.clone(), src_inf); - self.add_func_src(&mut res, id2.clone(), src_inf, pg_sql.clone()); + // Store it just in case another source needs the same function + used.entry(schema.to_string()) + .or_default() + .insert(name.to_string(), pg_sql.clone()); + + let id2 = self.resolve_id(id.clone(), cfg_inf); + self.add_func_src(&mut res, id2.clone(), cfg_inf, pg_sql.clone()); warn_on_rename(id, &id2, "function"); info!("Configured source {id2} from function {}", pg_sql.signature); debug!("{}", pg_sql.query); - info_map.insert(id2, src_inf.clone()); - // Store it just in case another source needs the same function - used_funcs - .entry(schema.to_string()) - .or_default() - .insert(name.to_string(), pg_sql); - } else if let Some(pg_sql) = used_funcs.get_mut(schema).and_then(|v| v.get(name)) { + info_map.insert(id2, cfg_inf.clone()); + } else if let Some(pg_sql) = used.get_mut(schema).and_then(|v| v.get(name)) { // This function was already used by another source - let id2 = self.resolve_id(id.clone(), src_inf); - self.add_func_src(&mut res, id2.clone(), src_inf, pg_sql.clone()); + let id2 = self.resolve_id(id.clone(), cfg_inf); + self.add_func_src(&mut res, id2.clone(), cfg_inf, pg_sql.clone()); warn_on_rename(id, &id2, "function"); let sig = &pg_sql.signature; info!("Configured duplicate source {id2} from function {sig}"); debug!("{}", pg_sql.query); - info_map.insert(id2, src_inf.clone()); + info_map.insert(id2, cfg_inf.clone()); } else { warn!( "Configured function source {id} from {schema}.{name} does not exist or \ @@ -148,8 +180,9 @@ impl PgConfigurator { } if self.discover_functions { - for funcs in discovered_sources.into_values() { - for (name, (pg_sql, src_inf)) in funcs { + // Sort the discovered sources by schema and function name to ensure a consistent behavior + for (_, funcs) in discovered_sources.into_iter().sorted_by(by_key) { + for (name, (pg_sql, src_inf)) in funcs.into_iter().sorted_by(by_key) { let id2 = self.resolve_id(name.clone(), &src_inf); self.add_func_src(&mut res, id2.clone(), &src_inf, pg_sql.clone()); info!("Discovered source {id2} from function {}", pg_sql.signature); @@ -167,16 +200,48 @@ impl PgConfigurator { self.id_resolver.resolve(id, signature) } - fn add_func_src( - &self, - sources: &mut Sources, - id: String, - src_inf: &impl PgInfo, - pg_sql: PgSqlInfo, - ) { - let source = PgSource::new(id.clone(), pg_sql, src_inf.to_tilejson(), self.pool.clone()); + fn add_func_src(&self, sources: &mut Sources, id: String, info: &impl PgInfo, sql: PgSqlInfo) { + let source = PgSource::new(id.clone(), sql, info.to_tilejson(), self.pool.clone()); sources.insert(id, Box::new(source)); } + + fn merge_info( + &self, + new_id: &String, + cfg_inf: &TableInfo, + src_inf: &TableInfo, + ) -> Option { + // Assume cfg_inf and src_inf have the same schema/table/geometry_column + let table_id = src_inf.format_id(); + let mut inf = cfg_inf.clone(); + inf.srid = match (src_inf.srid, cfg_inf.srid, self.default_srid) { + (0, 0, Some(default_srid)) => { + info!("Table {table_id} has SRID=0, using provided default SRID={default_srid}"); + default_srid + } + (0, 0, None) => { + let info = "To use this table source, set default or specify this table SRID in the config file, or set the default SRID with --default-srid=..."; + warn!("Table {table_id} has SRID=0, skipping. {info}"); + return None; + } + (0, cfg, _) => cfg, // Use the configured SRID + (src, 0, _) => src, // Use the source SRID + (src, cfg, _) if src != cfg => { + warn!("Table {table_id} has SRID={src}, but source {new_id} has SRID={cfg}"); + return None; + } + (_, cfg, _) => cfg, + }; + + match (&src_inf.geometry_type, &cfg_inf.geometry_type) { + (Some(src), Some(cfg)) if src != cfg => { + warn!(r#"Table {table_id} has geometry type={src}, but source {new_id} has {cfg}"#); + } + _ => {} + } + + Some(inf) + } } fn warn_on_rename(old_id: &String, new_id: &String, typ: &str) { @@ -184,3 +249,23 @@ fn warn_on_rename(old_id: &String, new_id: &String, typ: &str) { warn!("Configured {typ} source {old_id} was renamed to {new_id} due to ID conflict"); } } + +fn summary(info: &TableInfo) -> String { + format!( + "table {}.{} with {} column ({}, SRID={})", + info.schema, + info.table, + info.geometry_column, + info.geometry_type + .as_deref() + .unwrap_or("UNKNOWN GEOMETRY TYPE"), + info.srid, + ) +} + +fn by_key(a: &(String, T), b: &(String, T)) -> Ordering { + a.0.cmp(&b.0) +} + +pub type SqlFuncInfoMapMap = InfoMap>; +pub type SqlTableInfoMapMapMap = InfoMap>>; diff --git a/src/pg/function_source.rs b/src/pg/function_source.rs index d6d4949a9..631d8d636 100644 --- a/src/pg/function_source.rs +++ b/src/pg/function_source.rs @@ -1,5 +1,7 @@ -use crate::pg::config::{FunctionInfo, PgSqlInfo, SqlFuncInfoMapMap}; +use crate::pg::config::FunctionInfo; +use crate::pg::configurator::SqlFuncInfoMapMap; use crate::pg::connection::Pool; +use crate::pg::pg_source::PgSqlInfo; use crate::pg::utils::io_error; use log::warn; use postgres_protocol::escape::escape_identifier; diff --git a/src/pg/pg_source.rs b/src/pg/pg_source.rs index 3f11b6fe6..c8c11fc97 100644 --- a/src/pg/pg_source.rs +++ b/src/pg/pg_source.rs @@ -1,4 +1,3 @@ -use crate::pg::config::PgSqlInfo; use crate::pg::connection::Pool; use crate::pg::utils::{io_error, is_valid_zoom, query_to_json}; use crate::source::{Source, Tile, UrlQuery, Xyz}; @@ -87,3 +86,20 @@ impl Source for PgSource { Ok(tile) } } + +#[derive(Clone, Debug)] +pub struct PgSqlInfo { + pub query: String, + pub has_query_params: bool, + pub signature: String, +} + +impl PgSqlInfo { + pub fn new(query: String, has_query_params: bool, signature: String) -> Self { + Self { + query, + has_query_params, + signature, + } + } +} diff --git a/src/pg/table_source.rs b/src/pg/table_source.rs index e1ddcc889..a35a39260 100644 --- a/src/pg/table_source.rs +++ b/src/pg/table_source.rs @@ -1,5 +1,7 @@ -use crate::pg::config::{PgInfo, PgSqlInfo, SqlTableInfoMapMapMap, TableInfo}; +use crate::pg::config::{PgInfo, TableInfo}; +use crate::pg::configurator::SqlTableInfoMapMapMap; use crate::pg::connection::Pool; +use crate::pg::pg_source::PgSqlInfo; use crate::pg::utils::{io_error, json_to_hashmap, polygon_to_bbox}; use log::warn; use postgres_protocol::escape::{escape_identifier, escape_literal}; @@ -10,10 +12,12 @@ static DEFAULT_EXTENT: u32 = 4096; static DEFAULT_BUFFER: u32 = 64; static DEFAULT_CLIP_GEOM: bool = true; -pub async fn get_table_sources( - pool: &Pool, - default_srid: Option, -) -> Result { +#[derive(Clone, Debug)] +pub struct PgSqlTableInfo { + pub info: TableInfo, +} + +pub async fn get_table_sources(pool: &Pool) -> Result { let conn = pool.get().await?; let rows = conn .query(include_str!("scripts/get_table_sources.sql"), &[]) @@ -22,11 +26,11 @@ pub async fn get_table_sources( let mut res = SqlTableInfoMapMapMap::new(); for row in &rows { - let mut info = TableInfo { + let info = TableInfo { schema: row.get("schema"), table: row.get("name"), geometry_column: row.get("geom"), - srid: 0, + srid: row.get("srid"), // casting i32 to u32? extent: Some(DEFAULT_EXTENT), buffer: Some(DEFAULT_BUFFER), clip_geom: Some(DEFAULT_CLIP_GEOM), @@ -36,78 +40,84 @@ pub async fn get_table_sources( ..TableInfo::default() }; - let table_id = info.format_id(); - let srid: i32 = row.get("srid"); - info.srid = match (srid, default_srid) { - (0, Some(default_srid)) => { - warn!(r#""{table_id}" has SRID 0, using the provided default SRID {default_srid}"#); - default_srid as u32 - } - (0, None) => { - let info = "To use this table source, you must specify the SRID using the config file or provide the default SRID"; - warn!(r#""{table_id}" has SRID 0, skipping. {info}"#); - continue; - } - (srid, _) if srid < 0 => { - // TODO: why do we even use signed SRIDs? - warn!("Skipping unexpected srid {srid} for {table_id}"); - continue; - } - (srid, _) => srid as u32, - }; + if let Some(v) = res + .entry(info.schema.clone()) + .or_insert_with(HashMap::new) + .entry(info.table.clone()) + .or_insert_with(HashMap::new) + .insert(info.geometry_column.clone(), info) + { + warn!("Unexpected duplicate function {}", v.format_id()); + } + } - let bounds_query = format!( - include_str!("scripts/get_bounds.sql"), - schema = info.schema, - table = info.table, - srid = info.srid, - geometry_column = info.geometry_column, - ); + Ok(res) +} - info.bounds = conn +pub async fn table_to_query( + id: String, + mut info: TableInfo, + pool: Pool, +) -> Result<(String, PgSqlInfo, TableInfo), io::Error> { + let bounds_query = format!( + include_str!("scripts/get_bounds.sql"), + schema = info.schema, + table = info.table, + srid = info.srid, + geometry_column = info.geometry_column, + ); + + if info.bounds.is_none() { + info.bounds = pool + .get() + .await? .query_one(bounds_query.as_str(), &[]) .await .map(|row| row.get("bounds")) .ok() .flatten() .and_then(|v| polygon_to_bbox(&v)); + } - let properties = if info.properties.is_empty() { - String::new() - } else { - let properties = info - .properties - .keys() - .map(|column| format!(r#""{column}""#)) - .collect::>() - .join(","); - format!(", {properties}") - }; + let properties = if info.properties.is_empty() { + String::new() + } else { + let properties = info + .properties + .keys() + .map(|column| escape_identifier(column)) + .collect::>() + .join(","); + format!(", {properties}") + }; + + let id_column = info + .id_column + .clone() + .map_or(String::new(), |id_column| format!(", '{id_column}'")); - let id_column = info - .id_column - .clone() - .map_or(String::new(), |id_column| format!(", '{id_column}'")); + // TODO: extend ST_TileEnvelope by the buffer size. In PostGIS 3.3, this will be possible with + // the 5th margin parameter of the ST_TileEnvelope(z,x,y, srid, margin) - let query = format!( + let query = format!( r#" -SELECT - ST_AsMVT(tile, {table_id}, {extent}, 'geom' {id_column}) -FROM ( - SELECT - ST_AsMVTGeom( - ST_Transform(ST_CurveToLine({geometry_column}), 3857), - ST_TileEnvelope($1::integer, $2::integer, $3::integer), - {extent}, {buffer}, {clip_geom} - ) AS geom - {properties} - FROM - {schema}.{table} - WHERE - {geometry_column} && ST_Transform(ST_TileEnvelope($1::integer, $2::integer, $3::integer), {srid}) -) AS tile -"#, - table_id = escape_literal(table_id.as_str()), + SELECT + ST_AsMVT(tile, {table_id}, {extent}, 'geom' {id_column}) + FROM ( + SELECT + ST_AsMVTGeom( + ST_Transform(ST_CurveToLine({geometry_column}), 3857), + ST_TileEnvelope($1::integer, $2::integer, $3::integer), + {extent}, {buffer}, {clip_geom} + ) AS geom + {properties} + FROM + {schema}.{table} + WHERE + {geometry_column} && ST_Transform(ST_TileEnvelope($1::integer, $2::integer, $3::integer), {srid}) + ) AS tile + "#, + table_id = escape_literal(info.format_id().as_str()), extent = info.extent.unwrap_or(DEFAULT_EXTENT), geometry_column = escape_identifier(&info.geometry_column), buffer = info.buffer.unwrap_or(DEFAULT_BUFFER), @@ -117,19 +127,5 @@ FROM ( srid = info.srid, ).trim().to_string(); - if let Some(v) = res - .entry(info.schema.clone()) - .or_insert_with(HashMap::new) - .entry(info.table.clone()) - .or_insert_with(HashMap::new) - .insert( - info.geometry_column.clone(), - (PgSqlInfo::new(query, false, table_id), info), - ) - { - warn!("Unexpected duplicate function {}", v.0.signature); - } - } - - Ok(res) + Ok((id, PgSqlInfo::new(query, false, info.format_id()), info)) } diff --git a/src/srv/server.rs b/src/srv/server.rs index a50d1744a..2bc984075 100755 --- a/src/srv/server.rs +++ b/src/srv/server.rs @@ -183,7 +183,7 @@ fn get_tiles_url( let path_and_query = if query_string.is_empty() { format!("{tiles_path}/{{z}}/{{x}}/{{y}}") } else { - format!("{tiles_path}/{{z}}/{{x}}/{{y}}?{query_string}",) + format!("{tiles_path}/{{z}}/{{x}}/{{y}}?{query_string}") }; Uri::builder() diff --git a/tests/config.yaml b/tests/config.yaml index 4cca611fe..508ab4e78 100644 --- a/tests/config.yaml +++ b/tests/config.yaml @@ -21,7 +21,7 @@ pool_size: 20 worker_processes: 8 # Associative arrays of table sources -table_sources: +tables: table_source: # Table schema (required) schema: public @@ -115,7 +115,7 @@ table_sources: gid: int4 # Associative arrays of function sources -function_sources: +functions: function_zxy_query: # Schema name (required) schema: public diff --git a/tests/function_source_test.rs b/tests/function_source_test.rs index 5f176803b..32846985f 100644 --- a/tests/function_source_test.rs +++ b/tests/function_source_test.rs @@ -32,9 +32,8 @@ async fn get_function_sources_ok() { #[actix_rt::test] async fn function_source_tilejson() { - let sources = mock_sources(None, None).await; - let source = sources.get("function_zxy_query").unwrap(); - let tilejson = source.get_tilejson(); + let mock = mock_unconfigured().await; + let tilejson = source(&mock, "function_zxy_query").get_tilejson(); info!("tilejson = {tilejson:#?}"); @@ -50,9 +49,9 @@ async fn function_source_tilejson() { #[actix_rt::test] async fn function_source_tile() { - let sources = mock_sources(None, None).await; - let source = sources.get("function_zxy_query").unwrap(); - let tile = source.get_tile(&Xyz::new(0, 0, 0), &None).await.unwrap(); + let mock = mock_unconfigured().await; + let src = source(&mock, "function_zxy_query"); + let tile = src.get_tile(&Xyz::new(0, 0, 0), &None).await.unwrap(); assert!(!tile.is_empty()); } diff --git a/tests/server_test.rs b/tests/server_test.rs index 40c383600..3175d6c9b 100644 --- a/tests/server_test.rs +++ b/tests/server_test.rs @@ -4,7 +4,6 @@ use actix_web::test::{call_and_read_body_json, call_service, read_body, TestRequ use ctor::ctor; use martin::pg::config::{FunctionInfo, TableInfo}; use martin::srv::server::IndexEntry; -use std::collections::HashMap; use tilejson::{Bounds, TileJSON}; #[path = "utils.rs"] @@ -18,7 +17,8 @@ fn init() { macro_rules! create_app { ($sources:expr) => {{ - let state = crate::utils::mock_app_data($sources.await).await; + let sources = $sources.await.0; + let state = crate::utils::mock_app_data(sources).await; ::actix_web::test::init_service( ::actix_web::App::new() .app_data(state) @@ -34,12 +34,11 @@ fn test_get(path: &str) -> Request { #[actix_rt::test] async fn get_catalog_ok() { - let app = create_app!(mock_sources(None, None)); + let app = create_app!(mock_unconfigured()); let req = test_get("/catalog"); let response = call_service(&app, req).await; assert!(response.status().is_success()); - let body = read_body(response).await; let sources: Vec = serde_json::from_slice(&body).unwrap(); @@ -52,39 +51,39 @@ async fn get_catalog_ok() { #[actix_rt::test] async fn get_table_source_ok() { + let mut tables = mock_table_config_map(); + let table = tables.remove("table_source").unwrap(); let table_source = TableInfo { - schema: "public".to_owned(), - table: "table_source".to_owned(), - id_column: None, - geometry_column: "geom".to_owned(), - bounds: Some(Bounds::MAX), minzoom: Some(0), maxzoom: Some(30), - srid: 4326, - extent: Some(4096), - buffer: Some(64), - clip_geom: Some(true), - geometry_type: None, - properties: HashMap::new(), - unrecognized: HashMap::new(), + ..table.clone() }; - - let app = create_app!(mock_sources(None, Some(&[("table_source", table_source)]))); + let bad_srid = TableInfo { + srid: 3857, + ..table + }; + let app = create_app!(mock_sources( + None, + Some(vec![("table_source", table_source), ("bad_srid", bad_srid)]), + None + )); let req = test_get("/non_existent"); let response = call_service(&app, req).await; assert_eq!(response.status(), StatusCode::NOT_FOUND); + let req = test_get("/bad_srid"); + let response = call_service(&app, req).await; + assert_eq!(response.status(), StatusCode::NOT_FOUND); + let req = TestRequest::get() .uri("/table_source?token=martin") .insert_header(("x-rewrite-url", "/tiles/table_source?token=martin")) .to_request(); let result: TileJSON = call_and_read_body_json(&app, req).await; assert_eq!(result.name, Some(String::from("public.table_source.geom"))); - assert_eq!( - result.tiles, - &["http://localhost:8080/tiles/table_source/{z}/{x}/{y}?token=martin"] - ); + let expected_uri = "http://localhost:8080/tiles/table_source/{z}/{x}/{y}?token=martin"; + assert_eq!(result.tiles, &[expected_uri]); assert_eq!(result.minzoom, Some(0)); assert_eq!(result.maxzoom, Some(30)); assert_eq!(result.bounds, Some(Bounds::MAX)); @@ -92,7 +91,7 @@ async fn get_table_source_ok() { #[actix_rt::test] async fn get_table_source_tile_ok() { - let app = create_app!(mock_default_table_sources()); + let app = create_app!(mock_configured_tables(None)); let req = test_get("/non_existent/0/0/0"); let response = call_service(&app, req).await; @@ -105,7 +104,7 @@ async fn get_table_source_tile_ok() { #[actix_rt::test] async fn get_table_source_multiple_geom_tile_ok() { - let app = create_app!(mock_default_table_sources()); + let app = create_app!(mock_configured_tables(None)); let req = test_get("/table_source_multiple_geom.geom1/0/0/0"); let response = call_service(&app, req).await; @@ -118,69 +117,37 @@ async fn get_table_source_multiple_geom_tile_ok() { #[actix_rt::test] async fn get_table_source_tile_minmax_zoom_ok() { - let table_source = TableInfo { - schema: "public".to_owned(), - table: "table_source".to_owned(), - id_column: None, - geometry_column: "geom".to_owned(), - bounds: Some(Bounds::MAX), - minzoom: None, - maxzoom: Some(6), - srid: 4326, - extent: Some(4096), - buffer: Some(64), - clip_geom: Some(true), - geometry_type: None, - properties: HashMap::new(), - unrecognized: HashMap::new(), - }; - - let points1 = TableInfo { - schema: "public".to_owned(), - table: "points1".to_owned(), - id_column: None, - geometry_column: "geom".to_owned(), - minzoom: Some(6), - maxzoom: Some(12), - geometry_type: None, - properties: HashMap::new(), - unrecognized: HashMap::new(), - ..table_source - }; - - let points2 = TableInfo { - schema: "public".to_owned(), - table: "points2".to_owned(), - id_column: None, - geometry_column: "geom".to_owned(), - minzoom: None, - maxzoom: None, - geometry_type: None, - properties: HashMap::new(), - unrecognized: HashMap::new(), - ..table_source - }; - - let points3857 = TableInfo { - schema: "public".to_owned(), - table: "points3857".to_owned(), - id_column: None, - geometry_column: "geom".to_owned(), - minzoom: Some(6), - maxzoom: None, - geometry_type: None, - properties: HashMap::new(), - unrecognized: HashMap::new(), - ..table_source - }; - - let tables = &[ - ("points1", points1), - ("points2", points2), - ("points3857", points3857), - ("table_source", table_source), - ]; - let app = create_app!(mock_sources(None, Some(tables))); + let mut tables = mock_table_config_map(); + + let app = create_app!(mock_sources( + None, + Some(vec![ + ( + "points1", + TableInfo { + minzoom: Some(6), + maxzoom: Some(12), + ..tables.remove("points1").unwrap() + }, + ), + ("points2", tables.remove("points2").unwrap()), + ( + "points3857", + TableInfo { + minzoom: Some(6), + ..tables.remove("points3857").unwrap() + }, + ), + ( + "table_source", + TableInfo { + maxzoom: Some(6), + ..tables.remove("table_source").unwrap() + }, + ), + ]), + None + )); // zoom = 0 (nothing) let req = test_get("/points1/0/0/0"); @@ -245,7 +212,7 @@ async fn get_table_source_tile_minmax_zoom_ok() { #[actix_rt::test] async fn get_function_tiles() { - let app = create_app!(mock_sources(None, None)); + let app = create_app!(mock_unconfigured()); let req = test_get("/function_zoom_xy/6/38/20"); assert!(call_service(&app, req).await.status().is_success()); @@ -271,7 +238,7 @@ async fn get_function_tiles() { #[actix_rt::test] async fn get_composite_source_ok() { - let app = create_app!(mock_default_table_sources()); + let app = create_app!(mock_configured_tables(None)); let req = test_get("/non_existent1,non_existent2"); let response = call_service(&app, req).await; @@ -284,7 +251,7 @@ async fn get_composite_source_ok() { #[actix_rt::test] async fn get_composite_source_tile_ok() { - let app = create_app!(mock_default_table_sources()); + let app = create_app!(mock_configured_tables(None)); let req = test_get("/non_existent1,non_existent2/0/0/0"); let response = call_service(&app, req).await; @@ -297,42 +264,20 @@ async fn get_composite_source_tile_ok() { #[actix_rt::test] async fn get_composite_source_tile_minmax_zoom_ok() { - let public_points1 = TableInfo { - schema: "public".to_owned(), - table: "points1".to_owned(), - id_column: None, - geometry_column: "geom".to_owned(), - bounds: Some(Bounds::MAX), + let mut tables = mock_table_config_map(); + + let points1 = TableInfo { minzoom: Some(6), maxzoom: Some(13), - srid: 4326, - extent: Some(4096), - buffer: Some(64), - clip_geom: Some(true), - geometry_type: None, - properties: HashMap::new(), - unrecognized: HashMap::new(), + ..tables.remove("points1").unwrap() }; - - let public_points2 = TableInfo { - schema: "public".to_owned(), - table: "points2".to_owned(), - id_column: None, - geometry_column: "geom".to_owned(), - bounds: Some(Bounds::MAX), + let points2 = TableInfo { minzoom: Some(13), maxzoom: Some(20), - srid: 4326, - extent: Some(4096), - buffer: Some(64), - clip_geom: Some(true), - geometry_type: None, - properties: HashMap::new(), - unrecognized: HashMap::new(), + ..tables.remove("points2").unwrap() }; - - let tables = &[("points1", public_points1), ("points2", public_points2)]; - let app = create_app!(mock_sources(None, Some(tables))); + let tables = vec![("points1", points1), ("points2", points2)]; + let app = create_app!(mock_sources(None, Some(tables), None)); // zoom = 0 (nothing) let req = test_get("/points1,points2/0/0/0"); @@ -372,7 +317,7 @@ async fn get_composite_source_tile_minmax_zoom_ok() { #[actix_rt::test] async fn get_function_source_ok() { - let app = create_app!(mock_sources(None, None)); + let app = create_app!(mock_unconfigured()); let req = test_get("/non_existent"); let response = call_service(&app, req).await; @@ -419,7 +364,7 @@ async fn get_function_source_ok() { #[actix_rt::test] async fn get_function_source_tile_ok() { - let app = create_app!(mock_sources(None, None)); + let app = create_app!(mock_unconfigured()); let req = test_get("/function_zxy_query/0/0/0"); let response = call_service(&app, req).await; @@ -437,11 +382,11 @@ async fn get_function_source_tile_minmax_zoom_ok() { Bounds::MAX, ); - let funcs = &[ + let funcs = vec![ ("function_source1", function_source1), ("function_source2", function_source2), ]; - let app = create_app!(mock_sources(Some(funcs), None)); + let app = create_app!(mock_sources(Some(funcs), None, None)); // zoom = 0 (function_source1) let req = test_get("/function_source1/0/0/0"); @@ -486,7 +431,7 @@ async fn get_function_source_tile_minmax_zoom_ok() { #[actix_rt::test] async fn get_function_source_query_params_ok() { - let app = create_app!(mock_sources(None, None)); + let app = create_app!(mock_unconfigured()); let req = test_get("/function_zxy_query_test/0/0/0"); let response = call_service(&app, req).await; @@ -500,7 +445,7 @@ async fn get_function_source_query_params_ok() { #[actix_rt::test] async fn get_health_returns_ok() { - let app = create_app!(mock_sources(None, None)); + let app = create_app!(mock_unconfigured()); let req = test_get("/health"); let response = call_service(&app, req).await; diff --git a/tests/table_source_test.rs b/tests/table_source_test.rs index add0bb341..f027293aa 100644 --- a/tests/table_source_test.rs +++ b/tests/table_source_test.rs @@ -1,7 +1,5 @@ use ctor::ctor; use log::info; -use martin::pg::config::{PgSqlInfo, SqlTableInfoMapMapMap, TableInfo}; -use martin::pg::table_source::get_table_sources; use martin::source::Xyz; use std::collections::HashMap; @@ -15,12 +13,11 @@ fn init() { } #[actix_rt::test] -async fn get_table_sources_ok() { - let pool = mock_pool().await; - let sources = get_table_sources(&pool, Some(900913)).await.unwrap(); - assert!(!sources.is_empty()); +async fn table_sources() { + let mock = mock_unconfigured().await; + assert!(!mock.0.is_empty()); - let (_, source) = get_source(&sources, "table_source", "geom"); + let source = table(&mock, "table_source"); assert_eq!(source.schema, "public"); assert_eq!(source.table, "table_source"); assert_eq!(source.srid, 4326); @@ -41,9 +38,8 @@ async fn get_table_sources_ok() { #[actix_rt::test] async fn table_source_tilejson_ok() { - let sources = mock_sources(None, None).await; - let source = sources.get("table_source").unwrap(); - let tilejson = source.get_tilejson(); + let mock = mock_unconfigured().await; + let tilejson = source(&mock, "table_source").get_tilejson(); info!("tilejson = {tilejson:#?}"); @@ -59,52 +55,39 @@ async fn table_source_tilejson_ok() { #[actix_rt::test] async fn table_source_tile_ok() { - let sources = mock_sources(None, None).await; - let source = sources.get("table_source").unwrap(); - let tile = source.get_tile(&Xyz::new(0, 0, 0), &None).await.unwrap(); + let mock = mock_unconfigured().await; + let src = source(&mock, "table_source"); + let tile = src.get_tile(&Xyz::new(0, 0, 0), &None).await.unwrap(); assert!(!tile.is_empty()); } #[actix_rt::test] async fn table_source_srid_ok() { - let pool = mock_pool().await; - let table_sources = get_table_sources(&pool, Some(900913)).await.unwrap(); + let mock = mock_unconfigured_srid(Some(900913)).await; - let (_, source) = get_source(&table_sources, "points1", "geom"); + dbg!(&mock); + + let source = table(&mock, "points1"); assert_eq!(source.srid, 4326); - let (_, source) = get_source(&table_sources, "points2", "geom"); + let source = table(&mock, "points2"); assert_eq!(source.srid, 4326); - let (_, source) = get_source(&table_sources, "points3857", "geom"); + let source = table(&mock, "points3857"); assert_eq!(source.srid, 3857); - let (_, source) = get_source(&table_sources, "points_empty_srid", "geom"); + let source = table(&mock, "points_empty_srid"); assert_eq!(source.srid, 900913); } #[actix_rt::test] async fn table_source_multiple_geom_ok() { - let pool = mock_pool().await; - let sources = get_table_sources(&pool, Some(900913)).await.unwrap(); + let mock = mock_unconfigured().await; - let (_, source) = get_source(&sources, "table_source_multiple_geom", "geom1"); + let source = table(&mock, "table_source_multiple_geom"); assert_eq!(source.geometry_column, "geom1"); - let (_, source) = get_source(&sources, "table_source_multiple_geom", "geom2"); + let source = table(&mock, "table_source_multiple_geom.1"); assert_eq!(source.geometry_column, "geom2"); } - -fn get_source<'a>( - sources: &'a SqlTableInfoMapMapMap, - name: &'static str, - geom: &'static str, -) -> &'a (PgSqlInfo, TableInfo) { - let srcs = sources.get("public").expect("public schema not found"); - let cols = srcs - .get(name) - .unwrap_or_else(|| panic!("table {name} not found")); - cols.get(geom) - .unwrap_or_else(|| panic!("table {name}.{geom} not found")) -} diff --git a/tests/utils.rs b/tests/utils.rs index 17bbade7c..8c69f3ea8 100644 --- a/tests/utils.rs +++ b/tests/utils.rs @@ -1,10 +1,12 @@ +#![allow(clippy::redundant_clone)] + use actix_web::web::Data; use log::info; use martin::pg::config::{FunctionInfo, PgConfigBuilder}; use martin::pg::config::{PgConfig, TableInfo}; use martin::pg::configurator::resolve_pg_data; use martin::pg::connection::Pool; -use martin::source::IdResolver; +use martin::source::{IdResolver, Source}; use martin::srv::server::{AppState, Sources}; use std::collections::HashMap; use std::env; @@ -15,26 +17,13 @@ use tilejson::Bounds; // Each function should allow dead_code as they might not be used by a specific test file. // -#[allow(dead_code)] -pub async fn mock_pool() -> Pool { - let res = Pool::new(&mock_config(None, None).await).await; - res.expect("Failed to create pool") -} - -#[allow(dead_code)] -pub async fn mock_sources( - function_sources: Option<&[(&'static str, FunctionInfo)]>, - table_sources: Option<&[(&'static str, TableInfo)]>, -) -> Sources { - let cfg = mock_config(function_sources, table_sources).await; - let res = resolve_pg_data(cfg, IdResolver::default()).await; - res.expect("Failed to resolve pg data").0 -} +pub type MockSource = (Sources, PgConfig); #[allow(dead_code)] pub async fn mock_config( - function_sources: Option<&[(&'static str, FunctionInfo)]>, - table_sources: Option<&[(&'static str, TableInfo)]>, + functions: Option>, + tables: Option>, + default_srid: Option, ) -> PgConfig { let connection_string: String = env::var("DATABASE_URL").unwrap(); info!("Connecting to {connection_string}"); @@ -44,14 +33,14 @@ pub async fn mock_config( ca_root_file: None, #[cfg(feature = "ssl")] danger_accept_invalid_certs: None, - default_srid: None, + default_srid, pool_size: None, - table_sources: table_sources.map(|s| { + tables: tables.map(|s| { s.iter() .map(|v| (v.0.to_string(), v.1.clone())) .collect::>() }), - function_sources: function_sources.map(|s| { + functions: functions.map(|s| { s.iter() .map(|v| (v.0.to_string(), v.1.clone())) .collect::>() @@ -61,22 +50,144 @@ pub async fn mock_config( config.finalize().expect("Unable to finalize config") } +#[allow(dead_code)] +pub async fn mock_pool() -> Pool { + let res = Pool::new(&mock_config(None, None, None).await).await; + res.expect("Failed to create pool") +} + +#[allow(dead_code)] +pub async fn mock_sources( + functions: Option>, + tables: Option>, + default_srid: Option, +) -> MockSource { + let cfg = mock_config(functions, tables, default_srid).await; + let res = resolve_pg_data(cfg, IdResolver::default()).await; + let res = res.expect("Failed to resolve pg data"); + (res.0, res.1) +} + #[allow(dead_code)] pub async fn mock_app_data(sources: Sources) -> Data { Data::new(AppState { sources }) } #[allow(dead_code)] -pub async fn mock_default_table_sources() -> Sources { - let table_source = TableInfo { - schema: "public".to_owned(), - table: "table_source".to_owned(), +pub async fn mock_unconfigured() -> MockSource { + mock_sources(None, None, None).await +} + +#[allow(dead_code)] +pub async fn mock_unconfigured_srid(default_srid: Option) -> MockSource { + mock_sources(None, None, default_srid).await +} + +#[allow(dead_code)] +pub async fn mock_configured() -> MockSource { + mock_sources(mock_func_config(), mock_table_config(), None).await +} + +#[allow(dead_code)] +pub async fn mock_configured_funcs() -> MockSource { + mock_sources(mock_func_config(), None, None).await +} + +#[allow(dead_code)] +pub async fn mock_configured_tables(default_srid: Option) -> MockSource { + mock_sources(None, mock_table_config(), default_srid).await +} + +pub fn mock_func_config() -> Option> { + Some(mock_func_config_map().into_iter().collect()) +} + +pub fn mock_table_config() -> Option> { + Some(mock_table_config_map().into_iter().collect()) +} + +pub fn mock_func_config_map() -> HashMap<&'static str, FunctionInfo> { + let default = FunctionInfo::default(); + [ + ( + "function_zxy", + FunctionInfo { + schema: "public".to_string(), + function: "function_zxy".to_string(), + ..default.clone() + }, + ), + ( + "function_zxy_query_test", + FunctionInfo { + schema: "public".to_string(), + function: "function_zxy_query_test".to_string(), + ..default.clone() + }, + ), + ( + "function_zxy_row_key", + FunctionInfo { + schema: "public".to_string(), + function: "function_zxy_row_key".to_string(), + ..default.clone() + }, + ), + ( + "function_zxy_query", + FunctionInfo { + schema: "public".to_string(), + function: "function_zxy_query".to_string(), + ..default.clone() + }, + ), + ( + "function_zxy_row", + FunctionInfo { + schema: "public".to_string(), + function: "function_zxy_row".to_string(), + ..default.clone() + }, + ), + ( + "function_zxy_row2", + FunctionInfo { + schema: "public".to_string(), + function: "function_zxy_row2".to_string(), + ..default.clone() + }, + ), + ( + "function_zoom_xy", + FunctionInfo { + schema: "public".to_string(), + function: "function_zoom_xy".to_string(), + ..default.clone() + }, + ), + ( + "function_zxy2", + FunctionInfo { + schema: "public".to_string(), + function: "function_zxy2".to_string(), + ..default.clone() + }, + ), + ] + .into_iter() + .collect() +} + +pub fn mock_table_config_map() -> HashMap<&'static str, TableInfo> { + let default = TableInfo { + schema: String::new(), + table: String::new(), + srid: 4326, + geometry_column: String::new(), id_column: None, - geometry_column: "geom".to_owned(), minzoom: Some(0), maxzoom: Some(30), bounds: Some(Bounds::MAX), - srid: 4326, extent: Some(4096), buffer: Some(64), clip_geom: Some(true), @@ -85,94 +196,106 @@ pub async fn mock_default_table_sources() -> Sources { unrecognized: HashMap::new(), }; - let table_source_multiple_geom1 = TableInfo { - schema: "public".to_owned(), - table: "table_source_multiple_geom".to_owned(), - id_column: None, - geometry_column: "geom1".to_owned(), - geometry_type: None, - properties: HashMap::new(), - unrecognized: HashMap::new(), - ..table_source - }; - - let table_source_multiple_geom2 = TableInfo { - schema: "public".to_owned(), - table: "table_source_multiple_geom".to_owned(), - id_column: None, - geometry_column: "geom2".to_owned(), - geometry_type: None, - properties: HashMap::new(), - unrecognized: HashMap::new(), - ..table_source - }; - - let table_source1 = TableInfo { - schema: "public".to_owned(), - table: "points1".to_owned(), - id_column: None, - geometry_column: "geom".to_owned(), - geometry_type: None, - properties: HashMap::new(), - unrecognized: HashMap::new(), - ..table_source - }; - - let table_source2 = TableInfo { - schema: "public".to_owned(), - table: "points2".to_owned(), - id_column: None, - geometry_column: "geom".to_owned(), - geometry_type: None, - properties: HashMap::new(), - unrecognized: HashMap::new(), - ..table_source - }; + [ + ( + "points1", + TableInfo { + schema: "public".to_string(), + table: "points1".to_string(), + geometry_column: "geom".to_string(), + geometry_type: Some("POINT".to_string()), + properties: props(&[("gid", "int4")]), + ..default.clone() + }, + ), + ( + "points2", + TableInfo { + schema: "public".to_string(), + table: "points2".to_string(), + geometry_column: "geom".to_string(), + geometry_type: Some("POINT".to_string()), + properties: props(&[("gid", "int4")]), + ..default.clone() + }, + ), + ( + "points3857", + TableInfo { + schema: "public".to_string(), + table: "points3857".to_string(), + srid: 3857, + geometry_column: "geom".to_string(), + geometry_type: Some("POINT".to_string()), + properties: props(&[("gid", "int4")]), + ..default.clone() + }, + ), + ( + "points_empty_srid", + TableInfo { + schema: "public".to_string(), + table: "points_empty_srid".to_string(), + srid: 900973, + geometry_column: "geom".to_string(), + geometry_type: Some("GEOMETRY".to_string()), + properties: props(&[("gid", "int4")]), + ..default.clone() + }, + ), + ( + "table_source", + TableInfo { + schema: "public".to_string(), + table: "table_source".to_string(), + geometry_column: "geom".to_string(), + geometry_type: Some("GEOMETRY".to_string()), + properties: props(&[("gid", "int4")]), + ..default.clone() + }, + ), + ( + "table_source_multiple_geom.geom1", + TableInfo { + schema: "public".to_string(), + table: "table_source_multiple_geom".to_string(), + geometry_column: "geom1".to_string(), + geometry_type: Some("POINT".to_string()), + properties: props(&[("geom2", "geometry"), ("gid", "int4")]), + ..default.clone() + }, + ), + ( + "table_source_multiple_geom.geom2", + TableInfo { + schema: "public".to_string(), + table: "table_source_multiple_geom".to_string(), + geometry_column: "geom2".to_string(), + geometry_type: Some("POINT".to_string()), + properties: props(&[("gid", "int4"), ("geom1", "geometry")]), + ..default.clone() + }, + ), + ] + .into_iter() + .collect() +} - let table_source3857 = TableInfo { - schema: "public".to_owned(), - table: "points3857".to_owned(), - id_column: None, - geometry_column: "geom".to_owned(), - srid: 3857, - geometry_type: None, - properties: HashMap::new(), - unrecognized: HashMap::new(), - ..table_source - }; +fn props(props: &[(&'static str, &'static str)]) -> HashMap { + props + .iter() + .map(|(k, v)| (k.to_string(), v.to_string())) + .collect() +} - mock_sources( - None, - Some(&[ - ("table_source", table_source), - ( - "table_source_multiple_geom.geom1", - table_source_multiple_geom1, - ), - ( - "table_source_multiple_geom.geom2", - table_source_multiple_geom2, - ), - ("points1", table_source1), - ("points2", table_source2), - ("points3857", table_source3857), - ]), - ) - .await +#[allow(dead_code)] +pub fn table<'a>(mock: &'a MockSource, name: &str) -> &'a TableInfo { + let (_, PgConfig { tables, .. }) = mock; + tables.get(name).unwrap() } #[allow(dead_code)] -pub fn single(vec: &[T], mut cb: P) -> Option<&T> -where - T: Sized, - P: FnMut(&T) -> bool, -{ - let mut iter = vec.iter().filter(|v| cb(v)); - match iter.next() { - None => None, - Some(element) => match iter.next() { - None => Some(element), - Some(_) => None, - }, - } +pub fn source<'a>(mock: &'a MockSource, name: &str) -> &'a dyn Source { + let (sources, _) = mock; + sources.get(name).unwrap().as_ref() } From 8354ec1e9c04fc85903a832abd03992522109f31 Mon Sep 17 00:00:00 2001 From: Yuri Astrakhan Date: Sun, 4 Dec 2022 01:28:36 -0500 Subject: [PATCH 06/16] wip --- src/pg/configurator.rs | 12 ++++++- src/pg/function_source.rs | 2 +- src/pg/mod.rs | 2 +- src/pg/pg_source.rs | 2 +- src/pg/{connection.rs => pool.rs} | 55 ++++++++++++++++++------------- src/pg/table_source.rs | 42 +++++++++++++++-------- tests/utils.rs | 2 +- 7 files changed, 76 insertions(+), 41 deletions(-) rename src/pg/{connection.rs => pool.rs} (72%) diff --git a/src/pg/configurator.rs b/src/pg/configurator.rs index 497319ec8..b1a081794 100755 --- a/src/pg/configurator.rs +++ b/src/pg/configurator.rs @@ -1,9 +1,9 @@ use crate::pg::config::{ FuncInfoSources, FunctionInfo, InfoMap, PgConfig, PgInfo, TableInfo, TableInfoSources, }; -use crate::pg::connection::Pool; use crate::pg::function_source::get_function_sources; use crate::pg::pg_source::{PgSource, PgSqlInfo}; +use crate::pg::pool::Pool; use crate::pg::table_source::{get_table_sources, table_to_query}; use crate::source::IdResolver; use crate::srv::server::Sources; @@ -68,6 +68,16 @@ impl PgBuilder { // Note that multiple configured sources could map to a single discovered one. // After that, add the remaining discovered sources to the pending list if auto-config is enabled. for (id, cfg_inf) in &self.tables { + // TODO: move this validation to serde somehow? + if let Some(extent) = cfg_inf.extent { + if extent == 0 { + return Err(io::Error::new( + io::ErrorKind::InvalidInput, + format!("Configuration for source {id} has extent=0"), + )); + } + } + if let Some(src_inf) = discovered_sources .get_mut(&cfg_inf.schema) .and_then(|v| v.get_mut(&cfg_inf.table)) diff --git a/src/pg/function_source.rs b/src/pg/function_source.rs index 631d8d636..2bad56a81 100644 --- a/src/pg/function_source.rs +++ b/src/pg/function_source.rs @@ -1,7 +1,7 @@ use crate::pg::config::FunctionInfo; use crate::pg::configurator::SqlFuncInfoMapMap; -use crate::pg::connection::Pool; use crate::pg::pg_source::PgSqlInfo; +use crate::pg::pool::Pool; use crate::pg::utils::io_error; use log::warn; use postgres_protocol::escape::escape_identifier; diff --git a/src/pg/mod.rs b/src/pg/mod.rs index e6430d3d9..250d5479e 100644 --- a/src/pg/mod.rs +++ b/src/pg/mod.rs @@ -1,7 +1,7 @@ pub mod config; pub mod configurator; -pub mod connection; pub mod function_source; pub mod pg_source; +pub mod pool; pub mod table_source; pub mod utils; diff --git a/src/pg/pg_source.rs b/src/pg/pg_source.rs index c8c11fc97..12dd36844 100644 --- a/src/pg/pg_source.rs +++ b/src/pg/pg_source.rs @@ -1,4 +1,4 @@ -use crate::pg::connection::Pool; +use crate::pg::pool::Pool; use crate::pg::utils::{io_error, is_valid_zoom, query_to_json}; use crate::source::{Source, Tile, UrlQuery, Xyz}; use async_trait::async_trait; diff --git a/src/pg/connection.rs b/src/pg/pool.rs similarity index 72% rename from src/pg/connection.rs rename to src/pg/pool.rs index c662ed767..c69e5a2c6 100755 --- a/src/pg/connection.rs +++ b/src/pg/pool.rs @@ -7,7 +7,7 @@ use log::info; use openssl::ssl::{SslConnector, SslMethod, SslVerifyMode}; #[cfg(feature = "ssl")] use postgres_openssl::MakeTlsConnector; -use semver::{Version, VersionReq}; +use semver::Version; use std::io; use std::str::FromStr; @@ -20,12 +20,17 @@ pub type InternalPool = bb8::Pool; pub type Connection<'a> = PooledConnection<'a, ConnectionManager>; // We require ST_TileEnvelope that was added in PostGIS 3.0.0 -const REQUIRED_POSTGIS_VERSION: &str = ">= 3.0.0"; +// See https://postgis.net/docs/ST_TileEnvelope.html +const MINIMUM_POSTGIS_VER: Version = Version::new(3, 0, 0); +// After this version we can use margin parameter in ST_TileEnvelope +const MARGIN_PARAM_VER: Version = Version::new(3, 1, 0); #[derive(Clone, Debug)] pub struct Pool { id: String, pool: InternalPool, + // When true, we can use margin parameter in ST_TileEnvelope + margin: bool, } impl Pool { @@ -65,38 +70,44 @@ impl Pool { .build(manager) .await .map_err(|e| io_error!(e, "Can't build connection pool"))?; - let pool = Pool { id, pool }; - let postgis_version = pool.get_postgis_version().await?; - let req = VersionReq::parse(REQUIRED_POSTGIS_VERSION) - .map_err(|e| io_error!(e, "Can't parse required PostGIS version"))?; - let version = Version::parse(postgis_version.as_str()) + let version: Version = get_connection(&pool) + .await? + .query_one(include_str!("scripts/get_postgis_version.sql"), &[]) + .await + .map(|row| row.get::<_, String>("postgis_version")) + .map_err(|e| io_error!(e, "Can't get PostGIS version"))? + .parse() .map_err(|e| io_error!(e, "Can't parse database PostGIS version"))?; - if !req.matches(&version) { - Err(io::Error::new(io::ErrorKind::Other, format!("Martin requires PostGIS {REQUIRED_POSTGIS_VERSION}, current version is {postgis_version}"))) - } else { - Ok(pool) + if version < MINIMUM_POSTGIS_VER { + Err(io::Error::new( + io::ErrorKind::Other, + format!( + "Martin requires PostGIS {MINIMUM_POSTGIS_VER}, current version is {version}" + ), + ))?; } + + let margin = version >= MARGIN_PARAM_VER; + Ok(Pool { id, pool, margin }) } pub async fn get(&self) -> io::Result> { - self.pool - .get() - .await - .map_err(|e| io_error!(e, "Can't retrieve connection from the pool")) + get_connection(&self.pool).await } pub fn get_id(&self) -> &str { self.id.as_str() } - async fn get_postgis_version(&self) -> io::Result { - self.get() - .await? - .query_one(include_str!("scripts/get_postgis_version.sql"), &[]) - .await - .map(|row| row.get::<_, String>("postgis_version")) - .map_err(|e| io_error!(e, "Can't get PostGIS version")) + pub fn supports_tile_margin(&self) -> bool { + self.margin } } + +async fn get_connection(pool: &InternalPool) -> io::Result> { + pool.get() + .await + .map_err(|e| io_error!(e, "Can't retrieve connection from the pool")) +} diff --git a/src/pg/table_source.rs b/src/pg/table_source.rs index a35a39260..2a6800cb7 100644 --- a/src/pg/table_source.rs +++ b/src/pg/table_source.rs @@ -1,7 +1,7 @@ use crate::pg::config::{PgInfo, TableInfo}; use crate::pg::configurator::SqlTableInfoMapMapMap; -use crate::pg::connection::Pool; use crate::pg::pg_source::PgSqlInfo; +use crate::pg::pool::Pool; use crate::pg::utils::{io_error, json_to_hashmap, polygon_to_bbox}; use log::warn; use postgres_protocol::escape::{escape_identifier, escape_literal}; @@ -96,11 +96,25 @@ pub async fn table_to_query( .clone() .map_or(String::new(), |id_column| format!(", '{id_column}'")); - // TODO: extend ST_TileEnvelope by the buffer size. In PostGIS 3.3, this will be possible with - // the 5th margin parameter of the ST_TileEnvelope(z,x,y, srid, margin) + let extent = info.extent.unwrap_or(DEFAULT_EXTENT); + let buffer = info.buffer.unwrap_or(DEFAULT_BUFFER); + + let bbox_search = if buffer == 0 { + "ST_TileEnvelope($1::integer, $2::integer, $3::integer)".to_string() + } else if pool.supports_tile_margin() { + let margin = buffer as f64 / extent as f64; + format!("ST_TileEnvelope($1::integer, $2::integer, $3::integer, margin={margin})") + } else { + // TODO: we should use ST_Expand here, but it may require a bit more math work, + // TODO: so might not be worth it as it is only used for PostGIS < 3.1 + // let earth_circumference = 40075016.6855785; + // let val = earth_circumference * buffer as f64 / extent as f64; + // format!("ST_Expand(ST_TileEnvelope($1::integer, $2::integer, $3::integer), {val}/2^$1::integer)") + "ST_TileEnvelope($1::integer, $2::integer, $3::integer)".to_string() + }; let query = format!( - r#" + r#" SELECT ST_AsMVT(tile, {table_id}, {extent}, 'geom' {id_column}) FROM ( @@ -114,18 +128,18 @@ pub async fn table_to_query( FROM {schema}.{table} WHERE - {geometry_column} && ST_Transform(ST_TileEnvelope($1::integer, $2::integer, $3::integer), {srid}) + {geometry_column} && ST_Transform({bbox_search}, {srid}) ) AS tile "#, - table_id = escape_literal(info.format_id().as_str()), - extent = info.extent.unwrap_or(DEFAULT_EXTENT), - geometry_column = escape_identifier(&info.geometry_column), - buffer = info.buffer.unwrap_or(DEFAULT_BUFFER), - clip_geom = info.clip_geom.unwrap_or(DEFAULT_CLIP_GEOM), - schema = escape_identifier(&info.schema), - table = escape_identifier(&info.table), - srid = info.srid, - ).trim().to_string(); + table_id = escape_literal(info.format_id().as_str()), + geometry_column = escape_identifier(&info.geometry_column), + clip_geom = info.clip_geom.unwrap_or(DEFAULT_CLIP_GEOM), + schema = escape_identifier(&info.schema), + table = escape_identifier(&info.table), + srid = info.srid, + ) + .trim() + .to_string(); Ok((id, PgSqlInfo::new(query, false, info.format_id()), info)) } diff --git a/tests/utils.rs b/tests/utils.rs index 8c69f3ea8..a7e90c194 100644 --- a/tests/utils.rs +++ b/tests/utils.rs @@ -5,7 +5,7 @@ use log::info; use martin::pg::config::{FunctionInfo, PgConfigBuilder}; use martin::pg::config::{PgConfig, TableInfo}; use martin::pg::configurator::resolve_pg_data; -use martin::pg::connection::Pool; +use martin::pg::pool::Pool; use martin::source::{IdResolver, Source}; use martin::srv::server::{AppState, Sources}; use std::collections::HashMap; From 9ab7e9e517a5cbfc4facd469c061c384a412321e Mon Sep 17 00:00:00 2001 From: Yuri Astrakhan Date: Sun, 4 Dec 2022 02:53:10 -0500 Subject: [PATCH 07/16] Margin, fix id fields * Fix id column issue * Detect if postgis is newer than 3.1 and use st_tileenvolope margin param --- src/pg/pool.rs | 10 ++-- src/pg/table_source.rs | 49 +++++++++-------- tests/config.yaml | 6 +-- tests/fixtures/tables/points3_source.sql | 18 +++++++ tests/server_test.rs | 67 ++++++++++++++++++++++++ tests/table_source_test.rs | 12 ++--- tests/utils.rs | 14 ++++- 7 files changed, 140 insertions(+), 36 deletions(-) create mode 100644 tests/fixtures/tables/points3_source.sql diff --git a/src/pg/pool.rs b/src/pg/pool.rs index c69e5a2c6..7b12087a4 100755 --- a/src/pg/pool.rs +++ b/src/pg/pool.rs @@ -2,7 +2,7 @@ use crate::pg::config::PgConfig; use crate::pg::utils::io_error; use bb8::PooledConnection; use bb8_postgres::{tokio_postgres as pg, PostgresConnectionManager}; -use log::info; +use log::{info, warn}; #[cfg(feature = "ssl")] use openssl::ssl::{SslConnector, SslMethod, SslVerifyMode}; #[cfg(feature = "ssl")] @@ -23,7 +23,7 @@ pub type Connection<'a> = PooledConnection<'a, ConnectionManager>; // See https://postgis.net/docs/ST_TileEnvelope.html const MINIMUM_POSTGIS_VER: Version = Version::new(3, 0, 0); // After this version we can use margin parameter in ST_TileEnvelope -const MARGIN_PARAM_VER: Version = Version::new(3, 1, 0); +const RECOMMENDED_POSTGIS_VER: Version = Version::new(3, 1, 0); #[derive(Clone, Debug)] pub struct Pool { @@ -89,7 +89,11 @@ impl Pool { ))?; } - let margin = version >= MARGIN_PARAM_VER; + if version < RECOMMENDED_POSTGIS_VER { + warn!("PostGIS {version} is before the recommended {RECOMMENDED_POSTGIS_VER}. Margin parameter in ST_TileEnvelope is not supported, so tiles may be cut off at the edges."); + } + + let margin = version >= RECOMMENDED_POSTGIS_VER; Ok(Pool { id, pool, margin }) } diff --git a/src/pg/table_source.rs b/src/pg/table_source.rs index 2a6800cb7..441e6df88 100644 --- a/src/pg/table_source.rs +++ b/src/pg/table_source.rs @@ -91,10 +91,14 @@ pub async fn table_to_query( format!(", {properties}") }; - let id_column = info - .id_column - .clone() - .map_or(String::new(), |id_column| format!(", '{id_column}'")); + let (id_name, id_field) = if let Some(id_column) = &info.id_column { + ( + format!(", {}", escape_literal(id_column)), + format!(", {}", escape_identifier(id_column)), + ) + } else { + (String::new(), String::new()) + }; let extent = info.extent.unwrap_or(DEFAULT_EXTENT); let buffer = info.buffer.unwrap_or(DEFAULT_BUFFER); @@ -103,10 +107,11 @@ pub async fn table_to_query( "ST_TileEnvelope($1::integer, $2::integer, $3::integer)".to_string() } else if pool.supports_tile_margin() { let margin = buffer as f64 / extent as f64; - format!("ST_TileEnvelope($1::integer, $2::integer, $3::integer, margin={margin})") + format!("ST_TileEnvelope($1::integer, $2::integer, $3::integer, margin => {margin})") } else { // TODO: we should use ST_Expand here, but it may require a bit more math work, - // TODO: so might not be worth it as it is only used for PostGIS < 3.1 + // so might not be worth it as it is only used for PostGIS < v3.1. + // v3.1 has been out for 2+ years (december 2020) // let earth_circumference = 40075016.6855785; // let val = earth_circumference * buffer as f64 / extent as f64; // format!("ST_Expand(ST_TileEnvelope($1::integer, $2::integer, $3::integer), {val}/2^$1::integer)") @@ -115,22 +120,22 @@ pub async fn table_to_query( let query = format!( r#" - SELECT - ST_AsMVT(tile, {table_id}, {extent}, 'geom' {id_column}) - FROM ( - SELECT - ST_AsMVTGeom( - ST_Transform(ST_CurveToLine({geometry_column}), 3857), - ST_TileEnvelope($1::integer, $2::integer, $3::integer), - {extent}, {buffer}, {clip_geom} - ) AS geom - {properties} - FROM - {schema}.{table} - WHERE - {geometry_column} && ST_Transform({bbox_search}, {srid}) - ) AS tile - "#, +SELECT + ST_AsMVT(tile, {table_id}, {extent}, 'geom' {id_name}) +FROM ( + SELECT + ST_AsMVTGeom( + ST_Transform(ST_CurveToLine({geometry_column}), 3857), + ST_TileEnvelope($1::integer, $2::integer, $3::integer), + {extent}, {buffer}, {clip_geom} + ) AS geom + {id_field}{properties} + FROM + {schema}.{table} + WHERE + {geometry_column} && ST_Transform({bbox_search}, {srid}) +) AS tile +"#, table_id = escape_literal(info.format_id().as_str()), geometry_column = escape_identifier(&info.geometry_column), clip_geom = info.clip_geom.unwrap_or(DEFAULT_CLIP_GEOM), diff --git a/tests/config.yaml b/tests/config.yaml index 508ab4e78..a54936954 100644 --- a/tests/config.yaml +++ b/tests/config.yaml @@ -78,7 +78,7 @@ tables: extent: 4096 buffer: 64 clip_geom: true - geometry_type: GEOMETRY + geometry_type: POINT properties: gid: int4 @@ -94,7 +94,7 @@ tables: extent: 4096 buffer: 64 clip_geom: true - geometry_type: GEOMETRY + geometry_type: POINT properties: gid: int4 @@ -110,7 +110,7 @@ tables: extent: 4096 buffer: 64 clip_geom: true - geometry_type: GEOMETRY + geometry_type: POINT properties: gid: int4 diff --git a/tests/fixtures/tables/points3_source.sql b/tests/fixtures/tables/points3_source.sql new file mode 100644 index 000000000..a28485812 --- /dev/null +++ b/tests/fixtures/tables/points3_source.sql @@ -0,0 +1,18 @@ +CREATE TABLE points3(gid SERIAL PRIMARY KEY, fld1 TEXT, fld2 TEXT, geom GEOMETRY(POINT, 4326)); + +INSERT INTO points3 + SELECT + generate_series(1, 10000) as id, + md5(random()::text) as fld1, + md5(random()::text) as fld2, + ( + ST_DUMP( + ST_GENERATEPOINTS( + ST_GEOMFROMTEXT('POLYGON ((-180 90, 180 90, 180 -90, -180 -90, -180 90))', 4326), + 10000 + ) + ) + ).geom; + +CREATE INDEX ON points3 USING GIST(geom); +CLUSTER points3_geom_idx ON points3; diff --git a/tests/server_test.rs b/tests/server_test.rs index 3175d6c9b..7bc7f54df 100644 --- a/tests/server_test.rs +++ b/tests/server_test.rs @@ -451,3 +451,70 @@ async fn get_health_returns_ok() { let response = call_service(&app, req).await; assert!(response.status().is_success()); } + +#[actix_rt::test] +async fn tables_feature_id() { + let mut tables = mock_table_config_map(); + + let default = tables.remove("points3").unwrap(); + + let no_id = TableInfo { + id_column: None, + properties: props(&[("fld1", "text"), ("fld2", "text")]), + ..default.clone() + }; + let id_only = TableInfo { + id_column: Some("gid".to_string()), + properties: props(&[("fld1", "text"), ("fld2", "text")]), + ..default.clone() + }; + let id_and_prop = TableInfo { + id_column: Some("gid".to_string()), + properties: props(&[("gid", "int4"), ("fld1", "text"), ("fld2", "text")]), + ..default.clone() + }; + let prop_only = TableInfo { + id_column: None, + properties: props(&[("gid", "int4"), ("fld1", "text"), ("fld2", "text")]), + ..default.clone() + }; + + let tables = vec![ + ("no_id", no_id), + ("id_only", id_only), + ("id_and_prop", id_and_prop), + ("prop_only", prop_only), + ]; + let mock = mock_sources(None, Some(tables.clone()), None).await; + dbg!(&mock); + + let src = table(&mock, "no_id"); + assert_eq!(src.id_column, None); + assert_eq!(src.properties.len(), 2); + // let tj = source(&mock, "no_id").get_tilejson(); + // tj.vector_layers.unwrap().iter().for_each(|vl| { + // assert_eq!(vl.id, "no_id"); + // assert_eq!(vl.fields.len(), 2); + // }); + + let src = table(&mock, "id_only"); + assert_eq!(src.id_column, Some("gid".to_string())); + assert_eq!(src.properties.len(), 2); + + let src = table(&mock, "id_and_prop"); + assert_eq!(src.id_column, Some("gid".to_string())); + assert_eq!(src.properties.len(), 3); + + let src = table(&mock, "prop_only"); + assert_eq!(src.id_column, None); + assert_eq!(src.properties.len(), 3); + + // -------------------------------------------- + + let app = create_app!(mock_sources(None, Some(tables.clone()), None)); + for (name, _) in tables.iter() { + let req = test_get(format!("/{name}/0/0/0").as_str()); + let response = call_service(&app, req).await; + assert!(response.status().is_success()); + } +} diff --git a/tests/table_source_test.rs b/tests/table_source_test.rs index f027293aa..296f7e3c6 100644 --- a/tests/table_source_test.rs +++ b/tests/table_source_test.rs @@ -13,7 +13,7 @@ fn init() { } #[actix_rt::test] -async fn table_sources() { +async fn table_source() { let mock = mock_unconfigured().await; assert!(!mock.0.is_empty()); @@ -37,7 +37,7 @@ async fn table_sources() { } #[actix_rt::test] -async fn table_source_tilejson_ok() { +async fn tables_tilejson_ok() { let mock = mock_unconfigured().await; let tilejson = source(&mock, "table_source").get_tilejson(); @@ -54,7 +54,7 @@ async fn table_source_tilejson_ok() { } #[actix_rt::test] -async fn table_source_tile_ok() { +async fn tables_tile_ok() { let mock = mock_unconfigured().await; let src = source(&mock, "table_source"); let tile = src.get_tile(&Xyz::new(0, 0, 0), &None).await.unwrap(); @@ -63,11 +63,9 @@ async fn table_source_tile_ok() { } #[actix_rt::test] -async fn table_source_srid_ok() { +async fn tables_srid_ok() { let mock = mock_unconfigured_srid(Some(900913)).await; - dbg!(&mock); - let source = table(&mock, "points1"); assert_eq!(source.srid, 4326); @@ -82,7 +80,7 @@ async fn table_source_srid_ok() { } #[actix_rt::test] -async fn table_source_multiple_geom_ok() { +async fn tables_multiple_geom_ok() { let mock = mock_unconfigured().await; let source = table(&mock, "table_source_multiple_geom"); diff --git a/tests/utils.rs b/tests/utils.rs index a7e90c194..ea0cd49be 100644 --- a/tests/utils.rs +++ b/tests/utils.rs @@ -219,6 +219,18 @@ pub fn mock_table_config_map() -> HashMap<&'static str, TableInfo> { ..default.clone() }, ), + ( + "points3", + TableInfo { + schema: "public".to_string(), + table: "points3".to_string(), + geometry_column: "geom".to_string(), + geometry_type: Some("POINT".to_string()), + id_column: Some("gid".to_string()), + properties: props(&[("fld1", "text"), ("fld2", "text")]), + ..default.clone() + }, + ), ( "points3857", TableInfo { @@ -281,7 +293,7 @@ pub fn mock_table_config_map() -> HashMap<&'static str, TableInfo> { .collect() } -fn props(props: &[(&'static str, &'static str)]) -> HashMap { +pub fn props(props: &[(&'static str, &'static str)]) -> HashMap { props .iter() .map(|(k, v)| (k.to_string(), v.to_string())) From 812524230aa3c83ba8f6ec5c7bb599ee841d1103 Mon Sep 17 00:00:00 2001 From: Yuri Astrakhan Date: Tue, 6 Dec 2022 13:06:17 -0500 Subject: [PATCH 08/16] Use non-lowercase table name --- tests/config.yaml | 11 +++++++++++ tests/fixtures/functions/function_zxy_row2.sql | 10 +++++----- .../tables/{points1_source.sql => points1.sql} | 0 .../tables/{points2_source.sql => points2.sql} | 0 tests/fixtures/tables/points3.sql | 18 ++++++++++++++++++ .../{points3857_source.sql => points3857.sql} | 0 tests/fixtures/tables/points3_source.sql | 18 ------------------ ...y_srid_source.sql => points_empty_srid.sql} | 0 tests/server_test.rs | 18 +++++++++--------- tests/utils.rs | 14 ++++++++------ 10 files changed, 51 insertions(+), 38 deletions(-) rename tests/fixtures/tables/{points1_source.sql => points1.sql} (100%) rename tests/fixtures/tables/{points2_source.sql => points2.sql} (100%) create mode 100644 tests/fixtures/tables/points3.sql rename tests/fixtures/tables/{points3857_source.sql => points3857.sql} (100%) delete mode 100644 tests/fixtures/tables/points3_source.sql rename tests/fixtures/tables/{points_empty_srid_source.sql => points_empty_srid.sql} (100%) diff --git a/tests/config.yaml b/tests/config.yaml index a54936954..a0cba5233 100644 --- a/tests/config.yaml +++ b/tests/config.yaml @@ -98,6 +98,17 @@ tables: properties: gid: int4 + POINTS3: + schema: PUBLIC + table: pointS3 + id_column: giD + geometry_column: geoM + srid: 4326 + geometry_type: POINT + properties: + flD1: text + flD2: text + points3857: schema: public table: points3857 diff --git a/tests/fixtures/functions/function_zxy_row2.sql b/tests/fixtures/functions/function_zxy_row2.sql index 22053bd7b..ad3572675 100644 --- a/tests/fixtures/functions/function_zxy_row2.sql +++ b/tests/fixtures/functions/function_zxy_row2.sql @@ -1,15 +1,15 @@ -DROP FUNCTION IF EXISTS public.function_zxy_row2; +DROP FUNCTION IF EXISTS public.Function_Zxy_row2; -CREATE OR REPLACE FUNCTION public.function_zxy_row2(z integer, x integer, y integer) +CREATE OR REPLACE FUNCTION public.Function_Zxy_row2(Z integer, x integer, y integer) RETURNS TABLE(mvt bytea, key text) AS $$ SELECT mvt, md5(mvt) as key FROM ( - SELECT ST_AsMVT(tile, 'public.function_zxy_row2', 4096, 'geom') as mvt FROM ( + SELECT ST_AsMVT(tile, 'public.Function_Zxy_row2', 4096, 'geom') as mvt FROM ( SELECT ST_AsMVTGeom( ST_Transform(ST_CurveToLine(geom), 3857), - ST_TileEnvelope(z, x, y), + ST_TileEnvelope(Z, x, y), 4096, 64, true) AS geom FROM public.table_source - WHERE geom && ST_Transform(ST_TileEnvelope(z, x, y), 4326) + WHERE geom && ST_Transform(ST_TileEnvelope(Z, x, y), 4326) ) as tile WHERE geom IS NOT NULL) src $$ LANGUAGE SQL IMMUTABLE STRICT PARALLEL SAFE; diff --git a/tests/fixtures/tables/points1_source.sql b/tests/fixtures/tables/points1.sql similarity index 100% rename from tests/fixtures/tables/points1_source.sql rename to tests/fixtures/tables/points1.sql diff --git a/tests/fixtures/tables/points2_source.sql b/tests/fixtures/tables/points2.sql similarity index 100% rename from tests/fixtures/tables/points2_source.sql rename to tests/fixtures/tables/points2.sql diff --git a/tests/fixtures/tables/points3.sql b/tests/fixtures/tables/points3.sql new file mode 100644 index 000000000..b50b8fe0a --- /dev/null +++ b/tests/fixtures/tables/points3.sql @@ -0,0 +1,18 @@ +CREATE TABLE Points3(Gid SERIAL PRIMARY KEY, Fld1 TEXT, Fld2 TEXT, Geom GEOMETRY(POINT, 4326)); + +INSERT INTO Points3 + SELECT + generate_series(1, 10000) as id, + md5(random()::text) as Fld1, + md5(random()::text) as Fld2, + ( + ST_DUMP( + ST_GENERATEPOINTS( + ST_GEOMFROMTEXT('POLYGON ((-180 90, 180 90, 180 -90, -180 -90, -180 90))', 4326), + 10000 + ) + ) + ).Geom; + +CREATE INDEX ON Points3 USING GIST(Geom); +CLUSTER Points3_geom_idx ON Points3; diff --git a/tests/fixtures/tables/points3857_source.sql b/tests/fixtures/tables/points3857.sql similarity index 100% rename from tests/fixtures/tables/points3857_source.sql rename to tests/fixtures/tables/points3857.sql diff --git a/tests/fixtures/tables/points3_source.sql b/tests/fixtures/tables/points3_source.sql deleted file mode 100644 index a28485812..000000000 --- a/tests/fixtures/tables/points3_source.sql +++ /dev/null @@ -1,18 +0,0 @@ -CREATE TABLE points3(gid SERIAL PRIMARY KEY, fld1 TEXT, fld2 TEXT, geom GEOMETRY(POINT, 4326)); - -INSERT INTO points3 - SELECT - generate_series(1, 10000) as id, - md5(random()::text) as fld1, - md5(random()::text) as fld2, - ( - ST_DUMP( - ST_GENERATEPOINTS( - ST_GEOMFROMTEXT('POLYGON ((-180 90, 180 90, 180 -90, -180 -90, -180 90))', 4326), - 10000 - ) - ) - ).geom; - -CREATE INDEX ON points3 USING GIST(geom); -CLUSTER points3_geom_idx ON points3; diff --git a/tests/fixtures/tables/points_empty_srid_source.sql b/tests/fixtures/tables/points_empty_srid.sql similarity index 100% rename from tests/fixtures/tables/points_empty_srid_source.sql rename to tests/fixtures/tables/points_empty_srid.sql diff --git a/tests/server_test.rs b/tests/server_test.rs index 7bc7f54df..be7e71a2f 100644 --- a/tests/server_test.rs +++ b/tests/server_test.rs @@ -456,26 +456,26 @@ async fn get_health_returns_ok() { async fn tables_feature_id() { let mut tables = mock_table_config_map(); - let default = tables.remove("points3").unwrap(); + let default = tables.remove("POINTS3").unwrap(); let no_id = TableInfo { id_column: None, - properties: props(&[("fld1", "text"), ("fld2", "text")]), + properties: props(&[("flD1", "text"), ("flD2", "text")]), ..default.clone() }; let id_only = TableInfo { - id_column: Some("gid".to_string()), - properties: props(&[("fld1", "text"), ("fld2", "text")]), + id_column: Some("giD".to_string()), + properties: props(&[("flD1", "text"), ("flD2", "text")]), ..default.clone() }; let id_and_prop = TableInfo { - id_column: Some("gid".to_string()), - properties: props(&[("gid", "int4"), ("fld1", "text"), ("fld2", "text")]), + id_column: Some("giD".to_string()), + properties: props(&[("giD", "int4"), ("flD1", "text"), ("flD2", "text")]), ..default.clone() }; let prop_only = TableInfo { id_column: None, - properties: props(&[("gid", "int4"), ("fld1", "text"), ("fld2", "text")]), + properties: props(&[("giD", "int4"), ("flD1", "text"), ("flD2", "text")]), ..default.clone() }; @@ -498,11 +498,11 @@ async fn tables_feature_id() { // }); let src = table(&mock, "id_only"); - assert_eq!(src.id_column, Some("gid".to_string())); + assert_eq!(src.id_column, Some("giD".to_string())); assert_eq!(src.properties.len(), 2); let src = table(&mock, "id_and_prop"); - assert_eq!(src.id_column, Some("gid".to_string())); + assert_eq!(src.id_column, Some("giD".to_string())); assert_eq!(src.properties.len(), 3); let src = table(&mock, "prop_only"); diff --git a/tests/utils.rs b/tests/utils.rs index ea0cd49be..03edb0012 100644 --- a/tests/utils.rs +++ b/tests/utils.rs @@ -150,6 +150,7 @@ pub fn mock_func_config_map() -> HashMap<&'static str, FunctionInfo> { }, ), ( + // This function is created with non-lowercase name and field names "function_zxy_row2", FunctionInfo { schema: "public".to_string(), @@ -220,14 +221,15 @@ pub fn mock_table_config_map() -> HashMap<&'static str, TableInfo> { }, ), ( - "points3", + // This table is created with non-lowercase name and field names + "POINTS3", TableInfo { - schema: "public".to_string(), - table: "points3".to_string(), - geometry_column: "geom".to_string(), + schema: "PUBLIC".to_string(), + table: "pointS3".to_string(), + geometry_column: "geoM".to_string(), geometry_type: Some("POINT".to_string()), - id_column: Some("gid".to_string()), - properties: props(&[("fld1", "text"), ("fld2", "text")]), + id_column: Some("giD".to_string()), + properties: props(&[("flD1", "text"), ("flD2", "text")]), ..default.clone() }, ), From 380d044ea269d68dde0875f6c620dfe8b5b448af Mon Sep 17 00:00:00 2001 From: Yuri Astrakhan Date: Tue, 6 Dec 2022 13:36:40 -0500 Subject: [PATCH 09/16] Use non-lowercase table name --- src/pg/pg_source.rs | 13 ++++++++---- tests/config.yaml | 2 +- tests/fixtures/functions/function_zxy_row.sql | 14 +++++++------ .../fixtures/functions/function_zxy_row2.sql | 16 +++++++------- tests/fixtures/tables/points3.sql | 21 ++++++++----------- tests/server_test.rs | 4 ++-- tests/test.sh | 2 +- tests/utils.rs | 6 +++--- 8 files changed, 41 insertions(+), 37 deletions(-) diff --git a/src/pg/pg_source.rs b/src/pg/pg_source.rs index 12dd36844..bbf8eec9d 100644 --- a/src/pg/pg_source.rs +++ b/src/pg/pg_source.rs @@ -59,10 +59,15 @@ impl Source for PgSource { }; let query = &self.info.query; - let prep_query = conn - .prepare_typed(query, param_types) - .await - .map_err(|e| io_error!(e, "Can't create prepared statement for the tile"))?; + let prep_query = conn.prepare_typed(query, param_types).await.map_err(|e| { + io_error!( + e, + "Can't create prepared statement for the tile '{}' ({}): {}", + self.id, + self.info.signature, + self.info.query + ) + })?; let tile = if self.info.has_query_params { let json = query_to_json(url_query); diff --git a/tests/config.yaml b/tests/config.yaml index a0cba5233..cafb45bae 100644 --- a/tests/config.yaml +++ b/tests/config.yaml @@ -99,7 +99,7 @@ tables: gid: int4 POINTS3: - schema: PUBLIC + schema: MIXEDCASE table: pointS3 id_column: giD geometry_column: geoM diff --git a/tests/fixtures/functions/function_zxy_row.sql b/tests/fixtures/functions/function_zxy_row.sql index 62975bc4a..19d7eb531 100644 --- a/tests/fixtures/functions/function_zxy_row.sql +++ b/tests/fixtures/functions/function_zxy_row.sql @@ -1,11 +1,13 @@ -DROP FUNCTION IF EXISTS public.function_zxy_row; +-- Uses mixed case names but without double quotes -CREATE OR REPLACE FUNCTION public.function_zxy_row(z integer, x integer, y integer) -RETURNS TABLE(mvt bytea) AS $$ - SELECT ST_AsMVT(tile, 'public.function_zxy_row', 4096, 'geom') as mvt FROM ( +DROP FUNCTION IF EXISTS public.function_zxy_ROW; + +CREATE OR REPLACE FUNCTION public.function_zxy_ROW(Z integer, x integer, y integer) +RETURNS TABLE(MVT bytea) AS $$ + SELECT ST_AsMVT(tile, 'public.function_zxy_ROW', 4096, 'geom') as MVT FROM ( SELECT - ST_AsMVTGeom(ST_Transform(ST_CurveToLine(geom), 3857), ST_TileEnvelope(z, x, y), 4096, 64, true) AS geom + ST_AsMVTGeom(ST_Transform(ST_CurveToLine(geom), 3857), ST_TileEnvelope(Z, x, y), 4096, 64, true) AS geom FROM public.table_source - WHERE geom && ST_Transform(ST_TileEnvelope(z, x, y), 4326) + WHERE geom && ST_Transform(ST_TileEnvelope(Z, x, y), 4326) ) as tile WHERE geom IS NOT NULL $$ LANGUAGE SQL IMMUTABLE STRICT PARALLEL SAFE; diff --git a/tests/fixtures/functions/function_zxy_row2.sql b/tests/fixtures/functions/function_zxy_row2.sql index ad3572675..916508d73 100644 --- a/tests/fixtures/functions/function_zxy_row2.sql +++ b/tests/fixtures/functions/function_zxy_row2.sql @@ -1,15 +1,15 @@ -DROP FUNCTION IF EXISTS public.Function_Zxy_row2; +DROP FUNCTION IF EXISTS "MixedCase"."function_ZXY_row2"; -CREATE OR REPLACE FUNCTION public.Function_Zxy_row2(Z integer, x integer, y integer) -RETURNS TABLE(mvt bytea, key text) AS $$ +CREATE OR REPLACE FUNCTION "MixedCase"."function_ZXY_row2"("Z" integer, x integer, y integer) +RETURNS TABLE("mVt" bytea, key text) AS $$ SELECT mvt, md5(mvt) as key FROM ( - SELECT ST_AsMVT(tile, 'public.Function_Zxy_row2', 4096, 'geom') as mvt FROM ( + SELECT ST_AsMVT(tile, 'MixedCase.function_ZXY_row2', 4096, 'geom') as mvt FROM ( SELECT ST_AsMVTGeom( - ST_Transform(ST_CurveToLine(geom), 3857), - ST_TileEnvelope(Z, x, y), + ST_Transform(ST_CurveToLine("Geom"), 3857), + ST_TileEnvelope("Z", x, y), 4096, 64, true) AS geom - FROM public.table_source - WHERE geom && ST_Transform(ST_TileEnvelope(Z, x, y), 4326) + FROM "MixedCase"."Points3" + WHERE "Geom" && ST_Transform(ST_TileEnvelope("Z", x, y), 4326) ) as tile WHERE geom IS NOT NULL) src $$ LANGUAGE SQL IMMUTABLE STRICT PARALLEL SAFE; diff --git a/tests/fixtures/tables/points3.sql b/tests/fixtures/tables/points3.sql index b50b8fe0a..bb65b6f87 100644 --- a/tests/fixtures/tables/points3.sql +++ b/tests/fixtures/tables/points3.sql @@ -1,18 +1,15 @@ -CREATE TABLE Points3(Gid SERIAL PRIMARY KEY, Fld1 TEXT, Fld2 TEXT, Geom GEOMETRY(POINT, 4326)); +DROP SCHEMA IF EXISTS "MixedCase" CASCADE; +CREATE SCHEMA "MixedCase"; -INSERT INTO Points3 +CREATE TABLE "MixedCase"."PoiNTs3"("Gid" SERIAL PRIMARY KEY, "TABLE" TEXT, "table" INT, "Geom" GEOMETRY(POINT, 4326)); +CREATE TABLE "MixedCase"."Points3"("Gid" SERIAL PRIMARY KEY, "TABLE" TEXT, "Geom" GEOMETRY(POINT, 4326)); + +INSERT INTO "MixedCase"."Points3" SELECT generate_series(1, 10000) as id, - md5(random()::text) as Fld1, - md5(random()::text) as Fld2, + md5(random()::text) as "TABLE", ( - ST_DUMP( - ST_GENERATEPOINTS( - ST_GEOMFROMTEXT('POLYGON ((-180 90, 180 90, 180 -90, -180 -90, -180 90))', 4326), - 10000 - ) - ) + ST_DUMP(ST_GENERATEPOINTS(ST_GEOMFROMTEXT('POLYGON ((-180 90, 180 90, 180 -90, -180 -90, -180 90))', 4326), 10000)) ).Geom; -CREATE INDEX ON Points3 USING GIST(Geom); -CLUSTER Points3_geom_idx ON Points3; +CREATE INDEX ON "MixedCase"."Points3" USING GIST("Geom"); diff --git a/tests/server_test.rs b/tests/server_test.rs index be7e71a2f..8169d5e58 100644 --- a/tests/server_test.rs +++ b/tests/server_test.rs @@ -229,7 +229,7 @@ async fn get_function_tiles() { let req = test_get("/function_zxy_row/6/38/20"); assert!(call_service(&app, req).await.status().is_success()); - let req = test_get("/function_zxy_row2/6/38/20"); + let req = test_get("/function_ZXY_row2/6/38/20"); assert!(call_service(&app, req).await.status().is_success()); let req = test_get("/function_zxy_row_key/6/38/20"); @@ -343,7 +343,7 @@ async fn get_function_source_ok() { let response = call_service(&app, req).await; assert!(response.status().is_success()); - let req = test_get("/function_zxy_row2"); + let req = test_get("/function_ZXY_row2"); let response = call_service(&app, req).await; assert!(response.status().is_success()); diff --git a/tests/test.sh b/tests/test.sh index 1d0ef97a2..cc91e6aa1 100755 --- a/tests/test.sh +++ b/tests/test.sh @@ -124,7 +124,7 @@ test_pbf fnc_zxy_6_38_20 http://localhost:3000/function_zxy/6/38/20 test_pbf fnc_zxy2_6_38_20 http://localhost:3000/function_zxy2/6/38/20 test_pbf fnc_zxy_query_6_38_20 http://localhost:3000/function_zxy_query/6/38/20 test_pbf fnc_zxy_row_6_38_20 http://localhost:3000/function_zxy_row/6/38/20 -test_pbf fnc_zxy_row2_6_38_20 http://localhost:3000/function_zxy_row2/6/38/20 +test_pbf fnc_zxy_row2_6_38_20 http://localhost:3000/function_ZXY_row2/6/38/20 test_pbf fnc_zxy_row_key_6_38_20 http://localhost:3000/function_zxy_row_key/6/38/20 >&2 echo "Test server response for table source with different SRID" diff --git a/tests/utils.rs b/tests/utils.rs index 03edb0012..438c5843f 100644 --- a/tests/utils.rs +++ b/tests/utils.rs @@ -153,8 +153,8 @@ pub fn mock_func_config_map() -> HashMap<&'static str, FunctionInfo> { // This function is created with non-lowercase name and field names "function_zxy_row2", FunctionInfo { - schema: "public".to_string(), - function: "function_zxy_row2".to_string(), + schema: "MixedCase".to_string(), + function: "function_ZXY_row2".to_string(), ..default.clone() }, ), @@ -224,7 +224,7 @@ pub fn mock_table_config_map() -> HashMap<&'static str, TableInfo> { // This table is created with non-lowercase name and field names "POINTS3", TableInfo { - schema: "PUBLIC".to_string(), + schema: "MIXEDCASE".to_string(), table: "pointS3".to_string(), geometry_column: "geoM".to_string(), geometry_type: Some("POINT".to_string()), From 6fa777921b241806636b8a45eb819943a8448011 Mon Sep 17 00:00:00 2001 From: Yuri Astrakhan Date: Tue, 6 Dec 2022 19:39:17 -0500 Subject: [PATCH 10/16] fix casing --- src/config.rs | 3 +- src/lib.rs | 1 + src/pg/config.rs | 6 +- src/pg/configurator.rs | 122 +++++------------- src/pg/table_source.rs | 96 ++++++++++++-- src/pg/utils.rs | 6 +- src/utils.rs | 57 ++++++++ tests/config.yaml | 22 ++-- .../fixtures/functions/function_zxy_row2.sql | 2 +- .../tables/{points3.sql => MixPoints.sql} | 7 +- tests/server_test.rs | 18 +-- tests/utils.rs | 14 +- 12 files changed, 214 insertions(+), 140 deletions(-) create mode 100644 src/utils.rs rename tests/fixtures/tables/{points3.sql => MixPoints.sql} (50%) diff --git a/src/config.rs b/src/config.rs index caa962774..8b88a0981 100644 --- a/src/config.rs +++ b/src/config.rs @@ -140,7 +140,6 @@ mod tests { table: "table_source".to_string(), srid: 4326, geometry_column: "geom".to_string(), - id_column: None, minzoom: Some(0), maxzoom: Some(30), bounds: Some([-180, -90, 180, 90].into()), @@ -149,7 +148,7 @@ mod tests { clip_geom: Some(true), geometry_type: Some("GEOMETRY".to_string()), properties: HashMap::from([("gid".to_string(), "int4".to_string())]), - unrecognized: HashMap::new(), + ..Default::default() }, )]), functions: HashMap::from([( diff --git a/src/lib.rs b/src/lib.rs index 36cf431c6..59a96f83d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -6,6 +6,7 @@ pub mod config; pub mod pg; pub mod source; pub mod srv; +pub mod utils; // Ensure README.md contains valid code #[cfg(doctest)] diff --git a/src/pg/config.rs b/src/pg/config.rs index 4e111800f..18cf2f7cd 100644 --- a/src/pg/config.rs +++ b/src/pg/config.rs @@ -1,5 +1,6 @@ use crate::config::{report_unrecognized_config, set_option}; use crate::pg::utils::create_tilejson; +use crate::utils::InfoMap; use serde::{Deserialize, Serialize}; use serde_yaml::Value; use std::collections::HashMap; @@ -78,6 +79,10 @@ pub struct TableInfo { /// List of columns, that should be encoded as tile properties pub properties: HashMap, + /// Mapping of properties to the actual table columns + #[serde(skip_deserializing, skip_serializing)] + pub prop_mapping: HashMap, + #[serde(flatten, skip_serializing)] pub unrecognized: HashMap, } @@ -173,7 +178,6 @@ impl PgInfo for FunctionInfo { } } -pub type InfoMap = HashMap; pub type TableInfoSources = InfoMap; pub type FuncInfoSources = InfoMap; diff --git a/src/pg/configurator.rs b/src/pg/configurator.rs index b1a081794..62df7c909 100755 --- a/src/pg/configurator.rs +++ b/src/pg/configurator.rs @@ -1,17 +1,18 @@ use crate::pg::config::{ - FuncInfoSources, FunctionInfo, InfoMap, PgConfig, PgInfo, TableInfo, TableInfoSources, + FuncInfoSources, FunctionInfo, PgConfig, PgInfo, TableInfo, TableInfoSources, }; use crate::pg::function_source::get_function_sources; use crate::pg::pg_source::{PgSource, PgSqlInfo}; use crate::pg::pool::Pool; -use crate::pg::table_source::{get_table_sources, table_to_query}; +use crate::pg::table_source::{calc_srid, get_table_sources, merge_table_info, table_to_query}; use crate::source::IdResolver; use crate::srv::server::Sources; +use crate::utils::{find_info, InfoMap}; use futures::future::{join_all, try_join}; use itertools::Itertools; use log::{debug, error, info, warn}; use std::cmp::Ordering; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::io; pub async fn resolve_pg_data( @@ -59,14 +60,13 @@ impl PgBuilder { } pub async fn instantiate_tables(&self) -> Result<(Sources, TableInfoSources), io::Error> { - let mut info_map = TableInfoSources::new(); - let mut discovered_sources = get_table_sources(&self.pool).await?; - let mut used = SqlTableInfoMapMapMap::new(); - let mut pending = Vec::new(); + let all_tables = get_table_sources(&self.pool).await?; - // First match configured sources with the discovered ones and add them to the pending list. - // Note that multiple configured sources could map to a single discovered one. - // After that, add the remaining discovered sources to the pending list if auto-config is enabled. + dbg!(&all_tables); + + // Match configured sources with the discovered ones and add them to the pending list. + let mut used = HashSet::<(&str, &str, &str)>::new(); + let mut pending = Vec::new(); for (id, cfg_inf) in &self.tables { // TODO: move this validation to serde somehow? if let Some(extent) = cfg_inf.extent { @@ -78,58 +78,40 @@ impl PgBuilder { } } - if let Some(src_inf) = discovered_sources - .get_mut(&cfg_inf.schema) - .and_then(|v| v.get_mut(&cfg_inf.table)) - .and_then(|v| v.remove(&cfg_inf.geometry_column)) - { - // Store it just in case another source needs the same table - used.entry(src_inf.schema.to_string()) - .or_default() - .entry(src_inf.table.to_string()) - .or_default() - .insert(src_inf.geometry_column.to_string(), src_inf.clone()); + let Some(schemas) = find_info(&all_tables, &cfg_inf.schema, "schema", id) else { continue }; + let Some(tables) = find_info(schemas, &cfg_inf.table, "table", id) else { continue }; + let Some(src_inf) = find_info(tables, &cfg_inf.geometry_column, "geometry column", id) else { continue }; - let id2 = self.resolve_id(id.clone(), cfg_inf); - let Some(cfg_inf) = self.merge_info(&id2, cfg_inf, &src_inf) else {continue}; - warn_on_rename(id, &id2, "table"); - info!("Configured source {id2} from {}", summary(&cfg_inf)); - pending.push(table_to_query(id2, cfg_inf, self.pool.clone())); - } else if let Some(src_inf) = used - .get_mut(&cfg_inf.schema) - .and_then(|v| v.get(&cfg_inf.table)) - .and_then(|v| v.get(&cfg_inf.geometry_column)) - { - // This table was already used by another source - let id2 = self.resolve_id(id.clone(), cfg_inf); - let Some(cfg_inf) = self.merge_info(&id2, cfg_inf, src_inf) else {continue}; - warn_on_rename(id, &id2, "table"); - let info = summary(&cfg_inf); - info!("Configured duplicate source {id2} from {info}"); - pending.push(table_to_query(id2, cfg_inf, self.pool.clone())); - } else { - warn!( - "Configured table source {id} as {}.{} with geo column {} does not exist", - cfg_inf.schema, cfg_inf.table, cfg_inf.geometry_column - ); - } + let dup = used.insert((&cfg_inf.schema, &cfg_inf.table, &cfg_inf.geometry_column)); + let dup = if dup { "duplicate " } else { "" }; + + let id2 = self.resolve_id(id.clone(), cfg_inf); + let Some(cfg_inf) = merge_table_info(self.default_srid,&id2, cfg_inf, src_inf) else { continue }; + warn_on_rename(id, &id2, "table"); + info!("Configured {dup}source {id2} from {}", summary(&cfg_inf)); + pending.push(table_to_query(id2, cfg_inf, self.pool.clone())); } if self.discover_tables { // Sort the discovered sources by schema, table and geometry column to ensure a consistent behavior - for (_, tables) in discovered_sources.into_iter().sorted_by(by_key) { - for (_, geoms) in tables.into_iter().sorted_by(by_key) { - for (_, src_inf) in geoms.into_iter().sorted_by(by_key) { - let id2 = self.resolve_id(src_inf.table.clone(), &src_inf); - let Some(cfg_inf) = self.merge_info(&id2, &src_inf, &src_inf) else {continue}; - info!("Discovered source {id2} from {}", summary(&cfg_inf)); - pending.push(table_to_query(id2, cfg_inf, self.pool.clone())); + for (schema, tables) in all_tables.into_iter().sorted_by(by_key) { + for (table, geoms) in tables.into_iter().sorted_by(by_key) { + for (geom, mut src_inf) in geoms.into_iter().sorted_by(by_key) { + if used.contains(&(schema.as_str(), table.as_str(), geom.as_str())) { + continue; + } + let id2 = self.resolve_id(table.clone(), &src_inf); + let Some(srid) = calc_srid(&src_inf.format_id(), &id2, src_inf.srid,0, self.default_srid) else {continue}; + src_inf.srid = srid; + info!("Discovered source {id2} from {}", summary(&src_inf)); + pending.push(table_to_query(id2, src_inf, self.pool.clone())); } } } } let mut res: Sources = HashMap::new(); + let mut info_map = TableInfoSources::new(); let pending = join_all(pending).await; for src in pending { match src { @@ -214,44 +196,6 @@ impl PgBuilder { let source = PgSource::new(id.clone(), sql, info.to_tilejson(), self.pool.clone()); sources.insert(id, Box::new(source)); } - - fn merge_info( - &self, - new_id: &String, - cfg_inf: &TableInfo, - src_inf: &TableInfo, - ) -> Option { - // Assume cfg_inf and src_inf have the same schema/table/geometry_column - let table_id = src_inf.format_id(); - let mut inf = cfg_inf.clone(); - inf.srid = match (src_inf.srid, cfg_inf.srid, self.default_srid) { - (0, 0, Some(default_srid)) => { - info!("Table {table_id} has SRID=0, using provided default SRID={default_srid}"); - default_srid - } - (0, 0, None) => { - let info = "To use this table source, set default or specify this table SRID in the config file, or set the default SRID with --default-srid=..."; - warn!("Table {table_id} has SRID=0, skipping. {info}"); - return None; - } - (0, cfg, _) => cfg, // Use the configured SRID - (src, 0, _) => src, // Use the source SRID - (src, cfg, _) if src != cfg => { - warn!("Table {table_id} has SRID={src}, but source {new_id} has SRID={cfg}"); - return None; - } - (_, cfg, _) => cfg, - }; - - match (&src_inf.geometry_type, &cfg_inf.geometry_type) { - (Some(src), Some(cfg)) if src != cfg => { - warn!(r#"Table {table_id} has geometry type={src}, but source {new_id} has {cfg}"#); - } - _ => {} - } - - Some(inf) - } } fn warn_on_rename(old_id: &String, new_id: &String, typ: &str) { diff --git a/src/pg/table_source.rs b/src/pg/table_source.rs index 441e6df88..0089c91cd 100644 --- a/src/pg/table_source.rs +++ b/src/pg/table_source.rs @@ -3,7 +3,8 @@ use crate::pg::configurator::SqlTableInfoMapMapMap; use crate::pg::pg_source::PgSqlInfo; use crate::pg::pool::Pool; use crate::pg::utils::{io_error, json_to_hashmap, polygon_to_bbox}; -use log::warn; +use crate::utils::normalize_key; +use log::{info, warn}; use postgres_protocol::escape::{escape_identifier, escape_literal}; use std::collections::HashMap; use std::io; @@ -47,13 +48,26 @@ pub async fn get_table_sources(pool: &Pool) -> Result, field: &str) -> String { + let column = mapping.get(field).map_or(field, |v| v.as_str()); + if field != column { + format!( + ", {} AS {}", + escape_identifier(column), + escape_identifier(field), + ) + } else { + format!(", {}", escape_identifier(column)) + } +} + pub async fn table_to_query( id: String, mut info: TableInfo, @@ -82,19 +96,16 @@ pub async fn table_to_query( let properties = if info.properties.is_empty() { String::new() } else { - let properties = info - .properties + info.properties .keys() - .map(|column| escape_identifier(column)) - .collect::>() - .join(","); - format!(", {properties}") + .map(|column| escape_with_alias(&info.prop_mapping, column)) + .collect::() }; let (id_name, id_field) = if let Some(id_column) = &info.id_column { ( format!(", {}", escape_literal(id_column)), - format!(", {}", escape_identifier(id_column)), + escape_with_alias(&info.prop_mapping, id_column), ) } else { (String::new(), String::new()) @@ -121,7 +132,7 @@ pub async fn table_to_query( let query = format!( r#" SELECT - ST_AsMVT(tile, {table_id}, {extent}, 'geom' {id_name}) + ST_AsMVT(tile, {table_id}, {extent}, 'geom'{id_name}) FROM ( SELECT ST_AsMVTGeom( @@ -148,3 +159,68 @@ FROM ( Ok((id, PgSqlInfo::new(query, false, info.format_id()), info)) } + +pub fn merge_table_info( + default_srid: Option, + new_id: &String, + cfg_inf: &TableInfo, + src_inf: &TableInfo, +) -> Option { + // Assume cfg_inf and src_inf have the same schema/table/geometry_column + let table_id = src_inf.format_id(); + let mut inf = TableInfo { + // These values must match the database exactly + schema: src_inf.schema.clone(), + table: src_inf.table.clone(), + geometry_column: src_inf.geometry_column.clone(), + srid: calc_srid(&table_id, new_id, src_inf.srid, cfg_inf.srid, default_srid)?, + prop_mapping: HashMap::new(), + ..cfg_inf.clone() + }; + + match (&src_inf.geometry_type, &cfg_inf.geometry_type) { + (Some(src), Some(cfg)) if src != cfg => { + warn!(r#"Table {table_id} has geometry type={src}, but source {new_id} has {cfg}"#); + } + _ => {} + } + + if let Some(id_column) = &cfg_inf.id_column { + let prop = normalize_key(&src_inf.properties, id_column.as_str(), "id_column", new_id)?; + inf.prop_mapping.insert(id_column.clone(), prop); + } + + for key in cfg_inf.properties.keys() { + let prop = normalize_key(&src_inf.properties, key.as_str(), "property", new_id)?; + inf.prop_mapping.insert(key.clone(), prop); + } + + Some(inf) +} + +pub fn calc_srid( + table_id: &str, + new_id: &str, + src_srid: i32, + cfg_srid: i32, + default_srid: Option, +) -> Option { + match (src_srid, cfg_srid, default_srid) { + (0, 0, Some(default_srid)) => { + info!("Table {table_id} has SRID=0, using provided default SRID={default_srid}"); + Some(default_srid) + } + (0, 0, None) => { + let info = "To use this table source, set default or specify this table SRID in the config file, or set the default SRID with --default-srid=..."; + warn!("Table {table_id} has SRID=0, skipping. {info}"); + None + } + (0, cfg, _) => Some(cfg), // Use the configured SRID + (src, 0, _) => Some(src), // Use the source SRID + (src, cfg, _) if src != cfg => { + warn!("Table {table_id} has SRID={src}, but source {new_id} has SRID={cfg}"); + None + } + (_, cfg, _) => Some(cfg), + } +} diff --git a/src/pg/utils.rs b/src/pg/utils.rs index 40bf80584..a22d1194e 100755 --- a/src/pg/utils.rs +++ b/src/pg/utils.rs @@ -1,4 +1,5 @@ use crate::source::UrlQuery; +use crate::utils::InfoMap; use actix_http::header::HeaderValue; use actix_web::http::Uri; use postgis::{ewkb, LineString, Point, Polygon}; @@ -15,10 +16,9 @@ macro_rules! io_error { ::std::format!("{}: {}", ::std::format_args!($($arg,)+), $error)) }; } - pub(crate) use io_error; -pub fn json_to_hashmap(value: &serde_json::Value) -> HashMap { +pub fn json_to_hashmap(value: &serde_json::Value) -> InfoMap { let mut hashmap = HashMap::new(); let object = value.as_object().unwrap(); @@ -30,7 +30,7 @@ pub fn json_to_hashmap(value: &serde_json::Value) -> HashMap { hashmap } -pub fn query_to_json(query: &UrlQuery) -> Json> { +pub fn query_to_json(query: &UrlQuery) -> Json> { let mut query_as_json = HashMap::new(); for (k, v) in query.iter() { let json_value: serde_json::Value = diff --git a/src/utils.rs b/src/utils.rs new file mode 100644 index 000000000..da58319b6 --- /dev/null +++ b/src/utils.rs @@ -0,0 +1,57 @@ +use log::{error, info, warn}; +use std::collections::HashMap; + +pub type InfoMap = HashMap; + +pub fn normalize_key<'a, T>( + map: &'a InfoMap, + key: &str, + info: &str, + id: &str, +) -> Option { + find_info_kv(map, key, info, id).map(|(k, _)| k.to_string()) +} + +pub fn find_info<'a, T>(map: &'a InfoMap, key: &'a str, info: &str, id: &str) -> Option<&'a T> { + find_info_kv(map, key, info, id).map(|(_, v)| v) +} + +pub fn find_info_kv<'a, T>( + map: &'a InfoMap, + key: &'a str, + info: &str, + id: &str, +) -> Option<(&'a str, &'a T)> { + if let Some(v) = map.get(key) { + return Some((key, v)); + } + + let mut result = None; + let mut multiple = Vec::new(); + for k in map.keys() { + if k.to_lowercase() == key.to_lowercase() { + match result { + None => result = Some(k), + Some(result) => { + if multiple.is_empty() { + multiple.push(result.to_string()); + } + multiple.push(k.to_string()); + } + } + } + } + if multiple.is_empty() { + if let Some(result) = result { + info!("For source {id}, {info} '{key}' was not found, using '{result}' instead."); + Some((result.as_str(), map.get(result)?)) + } else { + warn!("Unable to configure source {id} because {info} '{key}' was not found. Possible values are: {}", + map.keys().map(|k| k.as_str()).collect::>().join(", ")); + None + } + } else { + error!("Unable to configure source {id} because {info} '{key}' has no exact match and more than one potential matches: {}", multiple.join(", ")); + None + } +} diff --git a/tests/config.yaml b/tests/config.yaml index cafb45bae..418ce06a9 100644 --- a/tests/config.yaml +++ b/tests/config.yaml @@ -66,6 +66,17 @@ tables: properties: gid: int4 + MixPoints: + schema: MIXEDCASE + table: MixPoints + id_column: giD + geometry_column: geoM + srid: 4326 + geometry_type: POINT + properties: + flD1: text + flD2: text + points1: schema: public table: points1 @@ -98,17 +109,6 @@ tables: properties: gid: int4 - POINTS3: - schema: MIXEDCASE - table: pointS3 - id_column: giD - geometry_column: geoM - srid: 4326 - geometry_type: POINT - properties: - flD1: text - flD2: text - points3857: schema: public table: points3857 diff --git a/tests/fixtures/functions/function_zxy_row2.sql b/tests/fixtures/functions/function_zxy_row2.sql index 916508d73..d8935376e 100644 --- a/tests/fixtures/functions/function_zxy_row2.sql +++ b/tests/fixtures/functions/function_zxy_row2.sql @@ -9,7 +9,7 @@ RETURNS TABLE("mVt" bytea, key text) AS $$ ST_Transform(ST_CurveToLine("Geom"), 3857), ST_TileEnvelope("Z", x, y), 4096, 64, true) AS geom - FROM "MixedCase"."Points3" + FROM "MixedCase"."MixPoints" WHERE "Geom" && ST_Transform(ST_TileEnvelope("Z", x, y), 4326) ) as tile WHERE geom IS NOT NULL) src $$ LANGUAGE SQL IMMUTABLE STRICT PARALLEL SAFE; diff --git a/tests/fixtures/tables/points3.sql b/tests/fixtures/tables/MixPoints.sql similarity index 50% rename from tests/fixtures/tables/points3.sql rename to tests/fixtures/tables/MixPoints.sql index bb65b6f87..76c83b09e 100644 --- a/tests/fixtures/tables/points3.sql +++ b/tests/fixtures/tables/MixPoints.sql @@ -1,10 +1,9 @@ DROP SCHEMA IF EXISTS "MixedCase" CASCADE; CREATE SCHEMA "MixedCase"; -CREATE TABLE "MixedCase"."PoiNTs3"("Gid" SERIAL PRIMARY KEY, "TABLE" TEXT, "table" INT, "Geom" GEOMETRY(POINT, 4326)); -CREATE TABLE "MixedCase"."Points3"("Gid" SERIAL PRIMARY KEY, "TABLE" TEXT, "Geom" GEOMETRY(POINT, 4326)); +CREATE TABLE "MixedCase"."MixPoints"("Gid" SERIAL PRIMARY KEY, "TABLE" TEXT, "Geom" GEOMETRY(POINT, 4326)); -INSERT INTO "MixedCase"."Points3" +INSERT INTO "MixedCase"."MixPoints" SELECT generate_series(1, 10000) as id, md5(random()::text) as "TABLE", @@ -12,4 +11,4 @@ INSERT INTO "MixedCase"."Points3" ST_DUMP(ST_GENERATEPOINTS(ST_GEOMFROMTEXT('POLYGON ((-180 90, 180 90, 180 -90, -180 -90, -180 90))', 4326), 10000)) ).Geom; -CREATE INDEX ON "MixedCase"."Points3" USING GIST("Geom"); +CREATE INDEX ON "MixedCase"."MixPoints" USING GIST("Geom"); diff --git a/tests/server_test.rs b/tests/server_test.rs index 8169d5e58..231dd8488 100644 --- a/tests/server_test.rs +++ b/tests/server_test.rs @@ -456,26 +456,26 @@ async fn get_health_returns_ok() { async fn tables_feature_id() { let mut tables = mock_table_config_map(); - let default = tables.remove("POINTS3").unwrap(); + let default = tables.remove("MIXPOINTS").unwrap(); let no_id = TableInfo { id_column: None, - properties: props(&[("flD1", "text"), ("flD2", "text")]), + properties: props(&[("TABLE", "text")]), ..default.clone() }; let id_only = TableInfo { id_column: Some("giD".to_string()), - properties: props(&[("flD1", "text"), ("flD2", "text")]), + properties: props(&[("TABLE", "text")]), ..default.clone() }; let id_and_prop = TableInfo { id_column: Some("giD".to_string()), - properties: props(&[("giD", "int4"), ("flD1", "text"), ("flD2", "text")]), + properties: props(&[("giD", "int4"), ("TABLE", "text")]), ..default.clone() }; let prop_only = TableInfo { id_column: None, - properties: props(&[("giD", "int4"), ("flD1", "text"), ("flD2", "text")]), + properties: props(&[("giD", "int4"), ("TABLE", "text")]), ..default.clone() }; @@ -490,7 +490,7 @@ async fn tables_feature_id() { let src = table(&mock, "no_id"); assert_eq!(src.id_column, None); - assert_eq!(src.properties.len(), 2); + assert_eq!(src.properties.len(), 1); // let tj = source(&mock, "no_id").get_tilejson(); // tj.vector_layers.unwrap().iter().for_each(|vl| { // assert_eq!(vl.id, "no_id"); @@ -499,15 +499,15 @@ async fn tables_feature_id() { let src = table(&mock, "id_only"); assert_eq!(src.id_column, Some("giD".to_string())); - assert_eq!(src.properties.len(), 2); + assert_eq!(src.properties.len(), 1); let src = table(&mock, "id_and_prop"); assert_eq!(src.id_column, Some("giD".to_string())); - assert_eq!(src.properties.len(), 3); + assert_eq!(src.properties.len(), 2); let src = table(&mock, "prop_only"); assert_eq!(src.id_column, None); - assert_eq!(src.properties.len(), 3); + assert_eq!(src.properties.len(), 2); // -------------------------------------------- diff --git a/tests/utils.rs b/tests/utils.rs index 438c5843f..15085de4d 100644 --- a/tests/utils.rs +++ b/tests/utils.rs @@ -181,20 +181,14 @@ pub fn mock_func_config_map() -> HashMap<&'static str, FunctionInfo> { pub fn mock_table_config_map() -> HashMap<&'static str, TableInfo> { let default = TableInfo { - schema: String::new(), - table: String::new(), srid: 4326, - geometry_column: String::new(), - id_column: None, minzoom: Some(0), maxzoom: Some(30), bounds: Some(Bounds::MAX), extent: Some(4096), buffer: Some(64), clip_geom: Some(true), - geometry_type: None, - properties: HashMap::new(), - unrecognized: HashMap::new(), + ..Default::default() }; [ @@ -222,14 +216,14 @@ pub fn mock_table_config_map() -> HashMap<&'static str, TableInfo> { ), ( // This table is created with non-lowercase name and field names - "POINTS3", + "MIXPOINTS", TableInfo { schema: "MIXEDCASE".to_string(), - table: "pointS3".to_string(), + table: "mixPoints".to_string(), geometry_column: "geoM".to_string(), geometry_type: Some("POINT".to_string()), id_column: Some("giD".to_string()), - properties: props(&[("flD1", "text"), ("flD2", "text")]), + properties: props(&[("tAble", "text")]), ..default.clone() }, ), From caef7bc0dd77a7f5205be756ddc78ec2a088e546 Mon Sep 17 00:00:00 2001 From: Yuri Astrakhan Date: Tue, 6 Dec 2022 20:05:52 -0500 Subject: [PATCH 11/16] minor cleanup --- src/pg/config.rs | 2 +- src/pg/pg_source.rs | 8 ++++---- src/pg/utils.rs | 5 +++++ src/source.rs | 17 +++++++++++------ src/srv/config.rs | 2 +- tests/utils.rs | 1 - 6 files changed, 22 insertions(+), 13 deletions(-) diff --git a/src/pg/config.rs b/src/pg/config.rs index 18cf2f7cd..1995d32a6 100644 --- a/src/pg/config.rs +++ b/src/pg/config.rs @@ -214,7 +214,7 @@ pub struct PgConfigBuilder { } impl PgConfigBuilder { - pub fn merge(&mut self, other: PgConfigBuilder) -> &mut Self { + pub fn merge(&mut self, other: Self) -> &mut Self { set_option(&mut self.connection_string, other.connection_string); #[cfg(feature = "ssl")] set_option(&mut self.ca_root_file, other.ca_root_file); diff --git a/src/pg/pg_source.rs b/src/pg/pg_source.rs index bbf8eec9d..097ca67c2 100644 --- a/src/pg/pg_source.rs +++ b/src/pg/pg_source.rs @@ -71,20 +71,20 @@ impl Source for PgSource { let tile = if self.info.has_query_params { let json = query_to_json(url_query); - debug!("SQL: {query} [{xyz:,>}, {json:?}]"); + debug!("SQL: {query} [{xyz}, {json:?}]"); let params: &[&(dyn ToSql + Sync)] = &[&xyz.z, &xyz.x, &xyz.y, &json]; conn.query_one(&prep_query, params).await } else { - debug!("SQL: {query} [{xyz:,>}]"); + debug!("SQL: {query} [{xyz}]"); conn.query_one(&prep_query, &[&xyz.z, &xyz.x, &xyz.y]).await }; let tile = tile.map(|row| row.get(0)).map_err(|e| { if self.info.has_query_params { let url_q = query_to_json(url_query); - io_error!(e, r#"Can't get {}/{xyz:/>} with {url_q:?} params"#, self.id) + io_error!(e, r#"Can't get {}/{xyz:#} with {url_q:?} params"#, self.id) } else { - io_error!(e, r#"Can't get {}/{xyz:/>}"#, self.id) + io_error!(e, r#"Can't get {}/{xyz:#}"#, self.id) } })?; diff --git a/src/pg/utils.rs b/src/pg/utils.rs index a22d1194e..706f8fd5e 100755 --- a/src/pg/utils.rs +++ b/src/pg/utils.rs @@ -10,6 +10,11 @@ use tilejson::{tilejson, Bounds, TileJSON, VectorLayer}; #[macro_export] macro_rules! io_error { + ($format:literal $(, $arg:expr)* $(,)?) => { + ::std::io::Error::new( + ::std::io::ErrorKind::Other, + ::std::format!($format, $($arg,)*)) + }; ($error:ident $(, $arg:expr)* $(,)?) => { ::std::io::Error::new( ::std::io::ErrorKind::Other, diff --git a/src/source.rs b/src/source.rs index fe2c1d711..9b72fcaac 100644 --- a/src/source.rs +++ b/src/source.rs @@ -23,12 +23,10 @@ impl Xyz { impl Display for Xyz { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - match f.fill() { - ',' => write!(f, "{},{},{}", self.z, self.x, self.y), - '/' => write!(f, "{}/{}/{}", self.z, self.x, self.y), - v => { - panic!("Invalid fill character '{v}' -- must be either ',' or '/'. Use {{xyz:/>}} or {{xyz:,>}}") - } + if f.alternate() { + write!(f, "{}/{}/{}", self.z, self.x, self.y) + } else { + write!(f, "{},{},{}", self.z, self.x, self.y) } } } @@ -128,4 +126,11 @@ mod tests { assert_eq!(r.resolve("a.1".to_string(), "a".to_string()), "a.1.1"); assert_eq!(r.resolve("a.1".to_string(), "b".to_string()), "a.1"); } + + #[test] + fn xyz_format() { + let xyz = Xyz::new(1, 2, 3); + assert_eq!(format!("{xyz}"), "1,2,3"); + assert_eq!(format!("{xyz:#}"), "1/2/3"); + } } diff --git a/src/srv/config.rs b/src/srv/config.rs index 593725140..974f17f84 100644 --- a/src/srv/config.rs +++ b/src/srv/config.rs @@ -35,7 +35,7 @@ pub struct SrvConfigBuilder { } impl SrvConfigBuilder { - pub fn merge(&mut self, other: SrvConfigBuilder) -> &mut Self { + pub fn merge(&mut self, other: Self) -> &mut Self { set_option(&mut self.keep_alive, other.keep_alive); set_option(&mut self.listen_addresses, other.listen_addresses); set_option(&mut self.worker_processes, other.worker_processes); diff --git a/tests/utils.rs b/tests/utils.rs index 15085de4d..fd6748d87 100644 --- a/tests/utils.rs +++ b/tests/utils.rs @@ -45,7 +45,6 @@ pub async fn mock_config( .map(|v| (v.0.to_string(), v.1.clone())) .collect::>() }), - // unrecognized: Default::default(), }; config.finalize().expect("Unable to finalize config") } From 12c42a2991b008f35461bbf7fa8fc6e0205ef1f3 Mon Sep 17 00:00:00 2001 From: Yuri Astrakhan Date: Tue, 6 Dec 2022 20:58:01 -0500 Subject: [PATCH 12/16] fix mixed case functions --- src/pg/configurator.rs | 59 ++++++++----------- src/pg/function_source.rs | 10 +++- src/pg/scripts/get_function_sources.sql | 6 +- src/utils.rs | 1 + tests/config.yaml | 3 +- ...n_zxy_row2.sql => function_Mixed_Name.sql} | 6 +- tests/server_test.rs | 5 +- tests/test.sh | 2 +- tests/utils.rs | 4 +- 9 files changed, 43 insertions(+), 53 deletions(-) rename tests/fixtures/functions/{function_zxy_row2.sql => function_Mixed_Name.sql} (64%) diff --git a/src/pg/configurator.rs b/src/pg/configurator.rs index 62df7c909..e731b5be4 100755 --- a/src/pg/configurator.rs +++ b/src/pg/configurator.rs @@ -62,8 +62,6 @@ impl PgBuilder { pub async fn instantiate_tables(&self) -> Result<(Sources, TableInfoSources), io::Error> { let all_tables = get_table_sources(&self.pool).await?; - dbg!(&all_tables); - // Match configured sources with the discovered ones and add them to the pending list. let mut used = HashSet::<(&str, &str, &str)>::new(); let mut pending = Vec::new(); @@ -131,50 +129,39 @@ impl PgBuilder { } pub async fn instantiate_functions(&self) -> Result<(Sources, FuncInfoSources), io::Error> { - let mut discovered_sources = get_function_sources(&self.pool).await?; + let all_functions = get_function_sources(&self.pool).await?; + let mut res: Sources = HashMap::new(); let mut info_map = FuncInfoSources::new(); - let mut used: HashMap> = HashMap::new(); + let mut used = HashSet::<(&str, &str)>::new(); for (id, cfg_inf) in &self.functions { - let schema = &cfg_inf.schema; - let name = &cfg_inf.function; - if let Some((pg_sql, _)) = discovered_sources - .get_mut(schema) - .and_then(|v| v.remove(name)) - { - // Store it just in case another source needs the same function - used.entry(schema.to_string()) - .or_default() - .insert(name.to_string(), pg_sql.clone()); - - let id2 = self.resolve_id(id.clone(), cfg_inf); - self.add_func_src(&mut res, id2.clone(), cfg_inf, pg_sql.clone()); - warn_on_rename(id, &id2, "function"); - info!("Configured source {id2} from function {}", pg_sql.signature); - debug!("{}", pg_sql.query); - info_map.insert(id2, cfg_inf.clone()); - } else if let Some(pg_sql) = used.get_mut(schema).and_then(|v| v.get(name)) { - // This function was already used by another source - let id2 = self.resolve_id(id.clone(), cfg_inf); - self.add_func_src(&mut res, id2.clone(), cfg_inf, pg_sql.clone()); - warn_on_rename(id, &id2, "function"); - let sig = &pg_sql.signature; - info!("Configured duplicate source {id2} from function {sig}"); - debug!("{}", pg_sql.query); - info_map.insert(id2, cfg_inf.clone()); - } else { - warn!( - "Configured function source {id} from {schema}.{name} does not exist or \ - does not have an expected signature like (z,x,y) -> bytea. See README.md", - ); + let Some(schemas) = find_info(&all_functions, &cfg_inf.schema, "schema", id) else { continue }; + if schemas.is_empty() { + warn!("No functions found in schema {}. Only functions like (z,x,y) -> bytea and similar are considered. See README.md", cfg_inf.schema); + continue; } + let Some((pg_sql, _)) = find_info(schemas, &cfg_inf.function, "function", id) else { continue }; + + let dup = used.insert((&cfg_inf.schema, &cfg_inf.function)); + let dup = if dup { "duplicate " } else { "" }; + + let id2 = self.resolve_id(id.clone(), cfg_inf); + self.add_func_src(&mut res, id2.clone(), cfg_inf, pg_sql.clone()); + warn_on_rename(id, &id2, "function"); + let signature = &pg_sql.signature; + info!("Configured {dup}source {id2} from function {signature}"); + debug!("{}", pg_sql.query); + info_map.insert(id2, cfg_inf.clone()); } if self.discover_functions { // Sort the discovered sources by schema and function name to ensure a consistent behavior - for (_, funcs) in discovered_sources.into_iter().sorted_by(by_key) { + for (schema, funcs) in all_functions.into_iter().sorted_by(by_key) { for (name, (pg_sql, src_inf)) in funcs.into_iter().sorted_by(by_key) { + if used.contains(&(schema.as_str(), name.as_str())) { + continue; + } let id2 = self.resolve_id(name.clone(), &src_inf); self.add_func_src(&mut res, id2.clone(), &src_inf, pg_sql.clone()); info!("Discovered source {id2} from function {}", pg_sql.signature); diff --git a/src/pg/function_source.rs b/src/pg/function_source.rs index 2bad56a81..461f27dd1 100644 --- a/src/pg/function_source.rs +++ b/src/pg/function_source.rs @@ -50,18 +50,22 @@ pub async fn get_function_sources(pool: &Pool) -> Result 0 { write!(query, ", ").unwrap(); } - write!(query, "{name} => ${index}::{typ}", index = idx + 1).unwrap(); + // This could also be done as "{name} => ${index}::{typ}" + // where the name must be passed through escape_identifier + write!(query, "${index}::{typ}", index = idx + 1).unwrap(); } write!(query, ")").unwrap(); // This is the same as if let-chain, but that's not yet available let ret_inf = match (output_record_names, output_type.as_str()) { (Some(names), "record") => { - // SELECT mvt FROM "public"."function_zxy_row2"(z => $1::integer, x => $2::integer, y => $3::integer); + // SELECT mvt FROM "public"."function_zxy_row2"( + // "z" => $1::integer, "x" => $2::integer, "y" => $3::integer + // ); query.insert_str(0, " FROM "); query.insert_str(0, &escape_identifier(names[0].as_str())); query.insert_str(0, "SELECT "); diff --git a/src/pg/scripts/get_function_sources.sql b/src/pg/scripts/get_function_sources.sql index 1c23a6331..46552f5c6 100755 --- a/src/pg/scripts/get_function_sources.sql +++ b/src/pg/scripts/get_function_sources.sql @@ -41,9 +41,9 @@ FROM information_schema.routines JOIN inputs ON routines.specific_name = inputs.specific_name LEFT JOIN outputs ON routines.specific_name = outputs.specific_name WHERE jsonb_array_length(input_names) IN (3, 4) - AND input_names ->> 0 IN ('z', 'zoom') - AND input_names ->> 1 = 'x' - AND input_names ->> 2 = 'y' + AND lower(input_names ->> 0) IN ('z', 'zoom') + AND lower(input_names ->> 1) = 'x' + AND lower(input_names ->> 2) = 'y' -- the 4th parameter is optional, and can be any name AND input_types ->> 0 = 'integer' AND input_types ->> 1 = 'integer' diff --git a/src/utils.rs b/src/utils.rs index da58319b6..750b8f4e7 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -41,6 +41,7 @@ pub fn find_info_kv<'a, T>( } } } + if multiple.is_empty() { if let Some(result) = result { info!("For source {id}, {info} '{key}' was not found, using '{result}' instead."); diff --git a/tests/config.yaml b/tests/config.yaml index 418ce06a9..4c27348c6 100644 --- a/tests/config.yaml +++ b/tests/config.yaml @@ -74,8 +74,7 @@ tables: srid: 4326 geometry_type: POINT properties: - flD1: text - flD2: text + taBLe: text points1: schema: public diff --git a/tests/fixtures/functions/function_zxy_row2.sql b/tests/fixtures/functions/function_Mixed_Name.sql similarity index 64% rename from tests/fixtures/functions/function_zxy_row2.sql rename to tests/fixtures/functions/function_Mixed_Name.sql index d8935376e..504006940 100644 --- a/tests/fixtures/functions/function_zxy_row2.sql +++ b/tests/fixtures/functions/function_Mixed_Name.sql @@ -1,9 +1,9 @@ -DROP FUNCTION IF EXISTS "MixedCase"."function_ZXY_row2"; +DROP FUNCTION IF EXISTS "MixedCase"."function_Mixed_Name"; -CREATE OR REPLACE FUNCTION "MixedCase"."function_ZXY_row2"("Z" integer, x integer, y integer) +CREATE OR REPLACE FUNCTION "MixedCase"."function_Mixed_Name"("Z" integer, x integer, y integer) RETURNS TABLE("mVt" bytea, key text) AS $$ SELECT mvt, md5(mvt) as key FROM ( - SELECT ST_AsMVT(tile, 'MixedCase.function_ZXY_row2', 4096, 'geom') as mvt FROM ( + SELECT ST_AsMVT(tile, 'MixedCase.function_Mixed_Name', 4096, 'geom') as mvt FROM ( SELECT ST_AsMVTGeom( ST_Transform(ST_CurveToLine("Geom"), 3857), diff --git a/tests/server_test.rs b/tests/server_test.rs index 231dd8488..f7a59ff3e 100644 --- a/tests/server_test.rs +++ b/tests/server_test.rs @@ -229,7 +229,7 @@ async fn get_function_tiles() { let req = test_get("/function_zxy_row/6/38/20"); assert!(call_service(&app, req).await.status().is_success()); - let req = test_get("/function_ZXY_row2/6/38/20"); + let req = test_get("/function_Mixed_Name/6/38/20"); assert!(call_service(&app, req).await.status().is_success()); let req = test_get("/function_zxy_row_key/6/38/20"); @@ -343,7 +343,7 @@ async fn get_function_source_ok() { let response = call_service(&app, req).await; assert!(response.status().is_success()); - let req = test_get("/function_ZXY_row2"); + let req = test_get("/function_Mixed_Name"); let response = call_service(&app, req).await; assert!(response.status().is_success()); @@ -486,7 +486,6 @@ async fn tables_feature_id() { ("prop_only", prop_only), ]; let mock = mock_sources(None, Some(tables.clone()), None).await; - dbg!(&mock); let src = table(&mock, "no_id"); assert_eq!(src.id_column, None); diff --git a/tests/test.sh b/tests/test.sh index cc91e6aa1..45bd99a27 100755 --- a/tests/test.sh +++ b/tests/test.sh @@ -124,7 +124,7 @@ test_pbf fnc_zxy_6_38_20 http://localhost:3000/function_zxy/6/38/20 test_pbf fnc_zxy2_6_38_20 http://localhost:3000/function_zxy2/6/38/20 test_pbf fnc_zxy_query_6_38_20 http://localhost:3000/function_zxy_query/6/38/20 test_pbf fnc_zxy_row_6_38_20 http://localhost:3000/function_zxy_row/6/38/20 -test_pbf fnc_zxy_row2_6_38_20 http://localhost:3000/function_ZXY_row2/6/38/20 +test_pbf fnc_zxy_row2_6_38_20 http://localhost:3000/function_Mixed_Name/6/38/20 test_pbf fnc_zxy_row_key_6_38_20 http://localhost:3000/function_zxy_row_key/6/38/20 >&2 echo "Test server response for table source with different SRID" diff --git a/tests/utils.rs b/tests/utils.rs index fd6748d87..73e46ce21 100644 --- a/tests/utils.rs +++ b/tests/utils.rs @@ -150,10 +150,10 @@ pub fn mock_func_config_map() -> HashMap<&'static str, FunctionInfo> { ), ( // This function is created with non-lowercase name and field names - "function_zxy_row2", + "function_mixed_name", FunctionInfo { schema: "MixedCase".to_string(), - function: "function_ZXY_row2".to_string(), + function: "function_Mixed_Name".to_string(), ..default.clone() }, ), From ac02930c18ffbfe7792645753f5c82fc686c4183 Mon Sep 17 00:00:00 2001 From: Yuri Astrakhan Date: Wed, 7 Dec 2022 13:32:52 -0500 Subject: [PATCH 13/16] remove Xyz::new(), makes it more complicated --- benches/sources.rs | 6 +++--- src/source.rs | 8 +------- src/srv/server.rs | 6 +++++- tests/function_source_test.rs | 5 ++++- tests/table_source_test.rs | 5 ++++- 5 files changed, 17 insertions(+), 13 deletions(-) diff --git a/benches/sources.rs b/benches/sources.rs index bac20aa6c..5c7791de6 100644 --- a/benches/sources.rs +++ b/benches/sources.rs @@ -52,7 +52,7 @@ fn main() {} // // async fn get_table_source_tile() { // let source = mock_table_source("public", "table_source").await; -// let xyz = Xyz::new(0, 0, 0); +// let xyz = Xyz { z: 0, x: 0, y: 0 }; // let _tile = source.get_tile(&xyz, &None).await.unwrap(); // } // @@ -77,7 +77,7 @@ fn main() {} // table_sources: vec![points1, points2], // }; // -// let xyz = Xyz::new(0, 0, 0); +// let xyz = Xyz { z: 0, x: 0, y: 0 }; // let _tile = source.get_tile(&xyz, &None).await.unwrap(); // } // @@ -88,7 +88,7 @@ fn main() {} // // async fn get_function_source_tile() { // let source = mock_function_source("public", "function_zxy_query"); -// let xyz = Xyz::new(0, 0, 0); +// let xyz = Xyz { z: 0, x: 0, y: 0 }; // // let _tile = source.get_tile(&xyz, &None).await.unwrap(); // } diff --git a/src/source.rs b/src/source.rs index 9b72fcaac..2a95a9076 100644 --- a/src/source.rs +++ b/src/source.rs @@ -15,12 +15,6 @@ pub struct Xyz { pub y: i32, } -impl Xyz { - pub fn new(z: i32, x: i32, y: i32) -> Self { - Self { z, x, y } - } -} - impl Display for Xyz { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { if f.alternate() { @@ -129,7 +123,7 @@ mod tests { #[test] fn xyz_format() { - let xyz = Xyz::new(1, 2, 3); + let xyz = Xyz { z: 1, x: 2, y: 3 }; assert_eq!(format!("{xyz}"), "1,2,3"); assert_eq!(format!("{xyz:#}"), "1/2/3"); } diff --git a/src/srv/server.rs b/src/srv/server.rs index 2bc984075..5c5d4efcd 100755 --- a/src/srv/server.rs +++ b/src/srv/server.rs @@ -243,7 +243,11 @@ async fn get_tile( let (sources, format) = state.get_sources(&path.source_ids, Some(path.z))?; let query = Some(query.into_inner()); - let xyz = Xyz::new(path.z, path.x, path.y); + let xyz = Xyz { + z: path.z, + x: path.x, + y: path.y, + }; let tile = try_join_all(sources.into_iter().map(|s| s.get_tile(&xyz, &query))) .await diff --git a/tests/function_source_test.rs b/tests/function_source_test.rs index 32846985f..dbd2798e9 100644 --- a/tests/function_source_test.rs +++ b/tests/function_source_test.rs @@ -51,7 +51,10 @@ async fn function_source_tilejson() { async fn function_source_tile() { let mock = mock_unconfigured().await; let src = source(&mock, "function_zxy_query"); - let tile = src.get_tile(&Xyz::new(0, 0, 0), &None).await.unwrap(); + let tile = src + .get_tile(&Xyz { z: 0, x: 0, y: 0 }, &None) + .await + .unwrap(); assert!(!tile.is_empty()); } diff --git a/tests/table_source_test.rs b/tests/table_source_test.rs index 296f7e3c6..d596eef23 100644 --- a/tests/table_source_test.rs +++ b/tests/table_source_test.rs @@ -57,7 +57,10 @@ async fn tables_tilejson_ok() { async fn tables_tile_ok() { let mock = mock_unconfigured().await; let src = source(&mock, "table_source"); - let tile = src.get_tile(&Xyz::new(0, 0, 0), &None).await.unwrap(); + let tile = src + .get_tile(&Xyz { z: 0, x: 0, y: 0 }, &None) + .await + .unwrap(); assert!(!tile.is_empty()); } From 3a873e254c80fb2f59e1d6ba793938b1b1ba468a Mon Sep 17 00:00:00 2001 From: Yuri Astrakhan Date: Thu, 8 Dec 2022 21:45:32 -0500 Subject: [PATCH 14/16] Allow auto discovery per schema --- src/bin/main.rs | 6 +-- src/config.rs | 23 ++++------- src/pg/config.rs | 77 +++++++++++++---------------------- src/pg/configurator.rs | 76 +++++++++++++++++----------------- src/pg/pool.rs | 6 +-- src/utils.rs | 32 +++++++++++++++ tests/function_source_test.rs | 14 +++++++ tests/server_test.rs | 31 +++++++------- tests/table_source_test.rs | 14 +++++++ tests/utils.rs | 44 ++++++++------------ 10 files changed, 174 insertions(+), 149 deletions(-) diff --git a/src/bin/main.rs b/src/bin/main.rs index 649865824..aa97e6512 100644 --- a/src/bin/main.rs +++ b/src/bin/main.rs @@ -2,7 +2,7 @@ use actix_web::dev::Server; use clap::Parser; use log::{error, info, warn}; use martin::config::{read_config, Config, ConfigBuilder}; -use martin::pg::config::{PgArgs, PgConfigBuilder}; +use martin::pg::config::{PgArgs, PgConfig}; use martin::pg::configurator::resolve_pg_data; use martin::source::IdResolver; use martin::srv::config::{SrvArgs, SrvConfigBuilder}; @@ -49,7 +49,7 @@ impl From for ConfigBuilder { ConfigBuilder { srv: SrvConfigBuilder::from(args.srv), - pg: PgConfigBuilder::from((args.pg, args.connection)), + pg: PgConfig::from((args.pg, args.connection)), unrecognized: HashMap::new(), } } @@ -89,7 +89,7 @@ async fn start(args: Args) -> io::Result { info!("Saving config to {file_name}, use --config to load it"); File::create(file_name)?.write_all(yaml.as_bytes())?; } - } else if config.pg.discover_functions || config.pg.discover_tables { + } else if config.pg.run_autodiscovery { info!("Martin has been configured with automatic settings."); info!("Use --save-config to save or print Martin configuration."); } diff --git a/src/config.rs b/src/config.rs index 8b88a0981..db4bfb114 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1,5 +1,5 @@ use crate::io_error; -use crate::pg::config::{PgConfig, PgConfigBuilder}; +use crate::pg::config::PgConfig; use crate::srv::config::{SrvConfig, SrvConfigBuilder}; use log::warn; use serde::{Deserialize, Serialize}; @@ -22,7 +22,7 @@ pub struct ConfigBuilder { #[serde(flatten)] pub srv: SrvConfigBuilder, #[serde(flatten)] - pub pg: PgConfigBuilder, + pub pg: PgConfig, #[serde(flatten)] pub unrecognized: HashMap, } @@ -124,16 +124,10 @@ mod tests { worker_processes: 8, }, pg: PgConfig { - connection_string: "postgres://postgres@localhost:5432/db".to_string(), - #[cfg(feature = "ssl")] - ca_root_file: None, - #[cfg(feature = "ssl")] - danger_accept_invalid_certs: false, + connection_string: Some("postgres://postgres@localhost:5432/db".to_string()), default_srid: Some(4326), - pool_size: 20, - discover_functions: false, - discover_tables: false, - tables: HashMap::from([( + pool_size: Some(20), + tables: Some(HashMap::from([( "table_source".to_string(), TableInfo { schema: "public".to_string(), @@ -150,8 +144,8 @@ mod tests { properties: HashMap::from([("gid".to_string(), "int4".to_string())]), ..Default::default() }, - )]), - functions: HashMap::from([( + )])), + functions: Some(HashMap::from([( "function_zxy_query".to_string(), FunctionInfo::new_extended( "public".to_string(), @@ -160,7 +154,8 @@ mod tests { 30, Bounds::MAX, ), - )]), + )])), + ..Default::default() }, }; assert_eq!(config, expected); diff --git a/src/pg/config.rs b/src/pg/config.rs index 1995d32a6..aa23b32d9 100644 --- a/src/pg/config.rs +++ b/src/pg/config.rs @@ -1,6 +1,6 @@ use crate::config::{report_unrecognized_config, set_option}; use crate::pg::utils::create_tilejson; -use crate::utils::InfoMap; +use crate::utils::{InfoMap, Schemas}; use serde::{Deserialize, Serialize}; use serde_yaml::Value; use std::collections::HashMap; @@ -181,50 +181,43 @@ impl PgInfo for FunctionInfo { pub type TableInfoSources = InfoMap; pub type FuncInfoSources = InfoMap; -#[derive(Clone, Debug, Serialize, PartialEq)] +#[derive(Clone, Debug, Default, PartialEq, Serialize, Deserialize)] pub struct PgConfig { - pub connection_string: String, + pub connection_string: Option, #[cfg(feature = "ssl")] #[serde(skip_serializing_if = "Option::is_none")] pub ca_root_file: Option, #[cfg(feature = "ssl")] + #[serde(skip_serializing_if = "Clone::clone")] pub danger_accept_invalid_certs: bool, #[serde(skip_serializing_if = "Option::is_none")] pub default_srid: Option, - pub pool_size: u32, - #[serde(skip_serializing)] - pub discover_functions: bool, - #[serde(skip_serializing)] - pub discover_tables: bool, - pub tables: TableInfoSources, - pub functions: FuncInfoSources, -} - -#[derive(Debug, Default, PartialEq, Deserialize)] -pub struct PgConfigBuilder { - pub connection_string: Option, - #[cfg(feature = "ssl")] - pub ca_root_file: Option, - #[cfg(feature = "ssl")] - pub danger_accept_invalid_certs: Option, - pub default_srid: Option, + #[serde(skip_serializing_if = "Option::is_none")] pub pool_size: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub auto_tables: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub auto_functions: Option, + #[serde(skip_serializing_if = "Option::is_none")] pub tables: Option, + #[serde(skip_serializing_if = "Option::is_none")] pub functions: Option, + #[serde(skip)] + pub run_autodiscovery: bool, } -impl PgConfigBuilder { +impl PgConfig { pub fn merge(&mut self, other: Self) -> &mut Self { set_option(&mut self.connection_string, other.connection_string); #[cfg(feature = "ssl")] - set_option(&mut self.ca_root_file, other.ca_root_file); - #[cfg(feature = "ssl")] - set_option( - &mut self.danger_accept_invalid_certs, - other.danger_accept_invalid_certs, - ); + { + set_option(&mut self.ca_root_file, other.ca_root_file); + self.danger_accept_invalid_certs |= other.danger_accept_invalid_certs; + } set_option(&mut self.default_srid, other.default_srid); set_option(&mut self.pool_size, other.pool_size); + set_option(&mut self.auto_tables, other.auto_tables); + set_option(&mut self.auto_functions, other.auto_functions); set_option(&mut self.tables, other.tables); set_option(&mut self.functions, other.functions); self @@ -249,24 +242,16 @@ impl PgConfigBuilder { ) })?; Ok(PgConfig { - connection_string, - #[cfg(feature = "ssl")] - ca_root_file: self.ca_root_file, - #[cfg(feature = "ssl")] - danger_accept_invalid_certs: self.danger_accept_invalid_certs.unwrap_or_default(), - default_srid: self.default_srid, - pool_size: self.pool_size.unwrap_or(POOL_SIZE_DEFAULT), - discover_functions: self.tables.is_none() && self.functions.is_none(), - discover_tables: self.tables.is_none() && self.functions.is_none(), - tables: self.tables.unwrap_or_default(), - functions: self.functions.unwrap_or_default(), + connection_string: Some(connection_string), + run_autodiscovery: self.tables.is_none() && self.functions.is_none(), + ..self }) } } -impl From<(PgArgs, Option)> for PgConfigBuilder { +impl From<(PgArgs, Option)> for PgConfig { fn from((args, connection): (PgArgs, Option)) -> Self { - PgConfigBuilder { + PgConfig { connection_string: connection.or_else(|| { env::var_os("DATABASE_URL").and_then(|connection| connection.into_string().ok()) }), @@ -275,13 +260,8 @@ impl From<(PgArgs, Option)> for PgConfigBuilder { env::var_os("CA_ROOT_FILE").and_then(|connection| connection.into_string().ok()) }), #[cfg(feature = "ssl")] - danger_accept_invalid_certs: if args.danger_accept_invalid_certs - || env::var_os("DANGER_ACCEPT_INVALID_CERTS").is_some() - { - Some(true) - } else { - None - }, + danger_accept_invalid_certs: args.danger_accept_invalid_certs + || env::var_os("DANGER_ACCEPT_INVALID_CERTS").is_some(), default_srid: args.default_srid.or_else(|| { env::var_os("DEFAULT_SRID").and_then(|srid| { srid.into_string() @@ -290,8 +270,7 @@ impl From<(PgArgs, Option)> for PgConfigBuilder { }) }), pool_size: args.pool_size, - tables: None, - functions: None, + ..Default::default() } } } diff --git a/src/pg/configurator.rs b/src/pg/configurator.rs index e731b5be4..ba732eadd 100755 --- a/src/pg/configurator.rs +++ b/src/pg/configurator.rs @@ -7,7 +7,7 @@ use crate::pg::pool::Pool; use crate::pg::table_source::{calc_srid, get_table_sources, merge_table_info, table_to_query}; use crate::source::IdResolver; use crate::srv::server::Sources; -use crate::utils::{find_info, InfoMap}; +use crate::utils::{find_info, normalize_key, InfoMap, Schemas}; use futures::future::{join_all, try_join}; use itertools::Itertools; use log::{debug, error, info, warn}; @@ -27,8 +27,8 @@ pub async fn resolve_pg_data( Ok(( tables, PgConfig { - tables: tbl_info, - functions: func_info, + tables: Some(tbl_info), + functions: Some(func_info), ..config }, pg.pool, @@ -38,8 +38,8 @@ pub async fn resolve_pg_data( struct PgBuilder { pool: Pool, default_srid: Option, - discover_functions: bool, - discover_tables: bool, + auto_functions: Schemas, + auto_tables: Schemas, id_resolver: IdResolver, tables: TableInfoSources, functions: FuncInfoSources, @@ -48,19 +48,20 @@ struct PgBuilder { impl PgBuilder { async fn new(config: &PgConfig, id_resolver: IdResolver) -> io::Result { let pool = Pool::new(config).await?; + let auto = config.run_autodiscovery; Ok(Self { pool, default_srid: config.default_srid, - discover_functions: config.discover_functions, - discover_tables: config.discover_tables, + auto_functions: config.auto_functions.clone().unwrap_or(Schemas::Bool(auto)), + auto_tables: config.auto_tables.clone().unwrap_or(Schemas::Bool(auto)), id_resolver, - tables: config.tables.clone(), - functions: config.functions.clone(), + tables: config.tables.clone().unwrap_or_default(), + functions: config.functions.clone().unwrap_or_default(), }) } pub async fn instantiate_tables(&self) -> Result<(Sources, TableInfoSources), io::Error> { - let all_tables = get_table_sources(&self.pool).await?; + let mut all_tables = get_table_sources(&self.pool).await?; // Match configured sources with the discovered ones and add them to the pending list. let mut used = HashSet::<(&str, &str, &str)>::new(); @@ -90,20 +91,20 @@ impl PgBuilder { pending.push(table_to_query(id2, cfg_inf, self.pool.clone())); } - if self.discover_tables { - // Sort the discovered sources by schema, table and geometry column to ensure a consistent behavior - for (schema, tables) in all_tables.into_iter().sorted_by(by_key) { - for (table, geoms) in tables.into_iter().sorted_by(by_key) { - for (geom, mut src_inf) in geoms.into_iter().sorted_by(by_key) { - if used.contains(&(schema.as_str(), table.as_str(), geom.as_str())) { - continue; - } - let id2 = self.resolve_id(table.clone(), &src_inf); - let Some(srid) = calc_srid(&src_inf.format_id(), &id2, src_inf.srid,0, self.default_srid) else {continue}; - src_inf.srid = srid; - info!("Discovered source {id2} from {}", summary(&src_inf)); - pending.push(table_to_query(id2, src_inf, self.pool.clone())); + // Sort the discovered sources by schema, table and geometry column to ensure a consistent behavior + for schema in self.auto_tables.get(|| all_tables.keys()) { + let Some(schema2) = normalize_key(&all_tables, &schema, "schema", "") else { continue }; + let tables = all_tables.remove(&schema2).unwrap(); + for (table, geoms) in tables.into_iter().sorted_by(by_key) { + for (geom, mut src_inf) in geoms.into_iter().sorted_by(by_key) { + if used.contains(&(schema.as_str(), table.as_str(), geom.as_str())) { + continue; } + let id2 = self.resolve_id(table.clone(), &src_inf); + let Some(srid) = calc_srid(&src_inf.format_id(), &id2, src_inf.srid,0, self.default_srid) else {continue}; + src_inf.srid = srid; + info!("Discovered source {id2} from {}", summary(&src_inf)); + pending.push(table_to_query(id2, src_inf, self.pool.clone())); } } } @@ -129,14 +130,13 @@ impl PgBuilder { } pub async fn instantiate_functions(&self) -> Result<(Sources, FuncInfoSources), io::Error> { - let all_functions = get_function_sources(&self.pool).await?; - + let mut all_funcs = get_function_sources(&self.pool).await?; let mut res: Sources = HashMap::new(); let mut info_map = FuncInfoSources::new(); let mut used = HashSet::<(&str, &str)>::new(); for (id, cfg_inf) in &self.functions { - let Some(schemas) = find_info(&all_functions, &cfg_inf.schema, "schema", id) else { continue }; + let Some(schemas) = find_info(&all_funcs, &cfg_inf.schema, "schema", id) else { continue }; if schemas.is_empty() { warn!("No functions found in schema {}. Only functions like (z,x,y) -> bytea and similar are considered. See README.md", cfg_inf.schema); continue; @@ -155,19 +155,19 @@ impl PgBuilder { info_map.insert(id2, cfg_inf.clone()); } - if self.discover_functions { - // Sort the discovered sources by schema and function name to ensure a consistent behavior - for (schema, funcs) in all_functions.into_iter().sorted_by(by_key) { - for (name, (pg_sql, src_inf)) in funcs.into_iter().sorted_by(by_key) { - if used.contains(&(schema.as_str(), name.as_str())) { - continue; - } - let id2 = self.resolve_id(name.clone(), &src_inf); - self.add_func_src(&mut res, id2.clone(), &src_inf, pg_sql.clone()); - info!("Discovered source {id2} from function {}", pg_sql.signature); - debug!("{}", pg_sql.query); - info_map.insert(id2, src_inf); + // Sort the discovered sources by schema and function name to ensure a consistent behavior + for schema in self.auto_functions.get(|| all_funcs.keys()) { + let Some(schema2) = normalize_key(&all_funcs, &schema, "schema", "") else { continue }; + let funcs = all_funcs.remove(&schema2).unwrap(); + for (name, (pg_sql, src_inf)) in funcs.into_iter().sorted_by(by_key) { + if used.contains(&(schema.as_str(), name.as_str())) { + continue; } + let id2 = self.resolve_id(name.clone(), &src_inf); + self.add_func_src(&mut res, id2.clone(), &src_inf, pg_sql.clone()); + info!("Discovered source {id2} from function {}", pg_sql.signature); + debug!("{}", pg_sql.query); + info_map.insert(id2, src_inf); } } diff --git a/src/pg/pool.rs b/src/pg/pool.rs index 7b12087a4..68e1e3812 100755 --- a/src/pg/pool.rs +++ b/src/pg/pool.rs @@ -1,4 +1,4 @@ -use crate::pg::config::PgConfig; +use crate::pg::config::{PgConfig, POOL_SIZE_DEFAULT}; use crate::pg::utils::io_error; use bb8::PooledConnection; use bb8_postgres::{tokio_postgres as pg, PostgresConnectionManager}; @@ -35,7 +35,7 @@ pub struct Pool { impl Pool { pub async fn new(config: &PgConfig) -> io::Result { - let conn_str = config.connection_string.as_str(); + let conn_str = config.connection_string.as_ref().unwrap().as_str(); info!("Connecting to {conn_str}"); let pg_cfg = pg::config::Config::from_str(conn_str) .map_err(|e| io_error!(e, "Can't parse connection string {conn_str}"))?; @@ -66,7 +66,7 @@ impl Pool { }; let pool = InternalPool::builder() - .max_size(config.pool_size) + .max_size(config.pool_size.unwrap_or(POOL_SIZE_DEFAULT)) .build(manager) .await .map_err(|e| io_error!(e, "Can't build connection pool"))?; diff --git a/src/utils.rs b/src/utils.rs index 750b8f4e7..fc6c5b676 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -1,4 +1,6 @@ +use itertools::Itertools; use log::{error, info, warn}; +use serde::{Deserialize, Serialize}; use std::collections::HashMap; pub type InfoMap = HashMap; @@ -56,3 +58,33 @@ pub fn find_info_kv<'a, T>( None } } + +/// A list of schemas to include in the discovery process, or a boolean to +/// indicate whether to run discovery at all. +#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] +#[serde(untagged)] +pub enum Schemas { + Bool(bool), + List(Vec), +} + +impl Schemas { + /// Returns a list of schemas to include in the discovery process. + /// If self is a true, returns a list of all schemas produced by the callback. + pub fn get<'a, I, F>(&self, keys: F) -> Vec + where + I: Iterator, + F: FnOnce() -> I, + { + match self { + Schemas::List(lst) => lst.clone(), + Schemas::Bool(all) => { + if *all { + keys().sorted().map(String::to_string).collect() + } else { + Vec::new() + } + } + } + } +} diff --git a/tests/function_source_test.rs b/tests/function_source_test.rs index dbd2798e9..ff8a2396d 100644 --- a/tests/function_source_test.rs +++ b/tests/function_source_test.rs @@ -1,7 +1,9 @@ use ctor::ctor; +use itertools::Itertools; use log::info; use martin::pg::function_source::get_function_sources; use martin::source::Xyz; +use martin::utils::Schemas; #[path = "utils.rs"] mod utils; @@ -58,3 +60,15 @@ async fn function_source_tile() { assert!(!tile.is_empty()); } + +#[actix_rt::test] +async fn function_source_schemas() { + let mut cfg = mock_empty_config().await; + cfg.auto_functions = Some(Schemas::List(vec!["MixedCase".to_owned()])); + cfg.auto_tables = Some(Schemas::Bool(false)); + let sources = mock_sources(cfg).await.0; + assert_eq!( + sources.keys().sorted().collect::>(), + vec!["function_Mixed_Name"], + ); +} diff --git a/tests/server_test.rs b/tests/server_test.rs index f7a59ff3e..77764332b 100644 --- a/tests/server_test.rs +++ b/tests/server_test.rs @@ -17,7 +17,7 @@ fn init() { macro_rules! create_app { ($sources:expr) => {{ - let sources = $sources.await.0; + let sources = mock_sources($sources.await).await.0; let state = crate::utils::mock_app_data(sources).await; ::actix_web::test::init_service( ::actix_web::App::new() @@ -34,7 +34,7 @@ fn test_get(path: &str) -> Request { #[actix_rt::test] async fn get_catalog_ok() { - let app = create_app!(mock_unconfigured()); + let app = create_app!(mock_empty_config()); let req = test_get("/catalog"); let response = call_service(&app, req).await; @@ -62,7 +62,7 @@ async fn get_table_source_ok() { srid: 3857, ..table }; - let app = create_app!(mock_sources( + let app = create_app!(mock_config( None, Some(vec![("table_source", table_source), ("bad_srid", bad_srid)]), None @@ -119,7 +119,7 @@ async fn get_table_source_multiple_geom_tile_ok() { async fn get_table_source_tile_minmax_zoom_ok() { let mut tables = mock_table_config_map(); - let app = create_app!(mock_sources( + let cfg = mock_config( None, Some(vec![ ( @@ -146,8 +146,9 @@ async fn get_table_source_tile_minmax_zoom_ok() { }, ), ]), - None - )); + None, + ); + let app = create_app!(cfg); // zoom = 0 (nothing) let req = test_get("/points1/0/0/0"); @@ -212,7 +213,7 @@ async fn get_table_source_tile_minmax_zoom_ok() { #[actix_rt::test] async fn get_function_tiles() { - let app = create_app!(mock_unconfigured()); + let app = create_app!(mock_empty_config()); let req = test_get("/function_zoom_xy/6/38/20"); assert!(call_service(&app, req).await.status().is_success()); @@ -277,7 +278,7 @@ async fn get_composite_source_tile_minmax_zoom_ok() { ..tables.remove("points2").unwrap() }; let tables = vec![("points1", points1), ("points2", points2)]; - let app = create_app!(mock_sources(None, Some(tables), None)); + let app = create_app!(mock_config(None, Some(tables), None)); // zoom = 0 (nothing) let req = test_get("/points1,points2/0/0/0"); @@ -317,7 +318,7 @@ async fn get_composite_source_tile_minmax_zoom_ok() { #[actix_rt::test] async fn get_function_source_ok() { - let app = create_app!(mock_unconfigured()); + let app = create_app!(mock_empty_config()); let req = test_get("/non_existent"); let response = call_service(&app, req).await; @@ -364,7 +365,7 @@ async fn get_function_source_ok() { #[actix_rt::test] async fn get_function_source_tile_ok() { - let app = create_app!(mock_unconfigured()); + let app = create_app!(mock_empty_config()); let req = test_get("/function_zxy_query/0/0/0"); let response = call_service(&app, req).await; @@ -386,7 +387,7 @@ async fn get_function_source_tile_minmax_zoom_ok() { ("function_source1", function_source1), ("function_source2", function_source2), ]; - let app = create_app!(mock_sources(Some(funcs), None, None)); + let app = create_app!(mock_config(Some(funcs), None, None)); // zoom = 0 (function_source1) let req = test_get("/function_source1/0/0/0"); @@ -431,7 +432,7 @@ async fn get_function_source_tile_minmax_zoom_ok() { #[actix_rt::test] async fn get_function_source_query_params_ok() { - let app = create_app!(mock_unconfigured()); + let app = create_app!(mock_empty_config()); let req = test_get("/function_zxy_query_test/0/0/0"); let response = call_service(&app, req).await; @@ -445,7 +446,7 @@ async fn get_function_source_query_params_ok() { #[actix_rt::test] async fn get_health_returns_ok() { - let app = create_app!(mock_unconfigured()); + let app = create_app!(mock_empty_config()); let req = test_get("/health"); let response = call_service(&app, req).await; @@ -485,7 +486,7 @@ async fn tables_feature_id() { ("id_and_prop", id_and_prop), ("prop_only", prop_only), ]; - let mock = mock_sources(None, Some(tables.clone()), None).await; + let mock = mock_sources(mock_config(None, Some(tables.clone()), None).await).await; let src = table(&mock, "no_id"); assert_eq!(src.id_column, None); @@ -510,7 +511,7 @@ async fn tables_feature_id() { // -------------------------------------------- - let app = create_app!(mock_sources(None, Some(tables.clone()), None)); + let app = create_app!(mock_config(None, Some(tables.clone()), None)); for (name, _) in tables.iter() { let req = test_get(format!("/{name}/0/0/0").as_str()); let response = call_service(&app, req).await; diff --git a/tests/table_source_test.rs b/tests/table_source_test.rs index d596eef23..ff3a80da3 100644 --- a/tests/table_source_test.rs +++ b/tests/table_source_test.rs @@ -1,6 +1,8 @@ use ctor::ctor; +use itertools::Itertools; use log::info; use martin::source::Xyz; +use martin::utils::Schemas; use std::collections::HashMap; #[path = "utils.rs"] @@ -92,3 +94,15 @@ async fn tables_multiple_geom_ok() { let source = table(&mock, "table_source_multiple_geom.1"); assert_eq!(source.geometry_column, "geom2"); } + +#[actix_rt::test] +async fn table_source_schemas() { + let mut cfg = mock_empty_config().await; + cfg.auto_functions = Some(Schemas::Bool(false)); + cfg.auto_tables = Some(Schemas::List(vec!["MixedCase".to_owned()])); + let sources = mock_sources(cfg).await.0; + assert_eq!( + sources.keys().sorted().collect::>(), + vec!["MixPoints"], + ); +} diff --git a/tests/utils.rs b/tests/utils.rs index 73e46ce21..674e7bec4 100644 --- a/tests/utils.rs +++ b/tests/utils.rs @@ -2,8 +2,7 @@ use actix_web::web::Data; use log::info; -use martin::pg::config::{FunctionInfo, PgConfigBuilder}; -use martin::pg::config::{PgConfig, TableInfo}; +use martin::pg::config::{FunctionInfo, PgConfig, TableInfo}; use martin::pg::configurator::resolve_pg_data; use martin::pg::pool::Pool; use martin::source::{IdResolver, Source}; @@ -27,14 +26,9 @@ pub async fn mock_config( ) -> PgConfig { let connection_string: String = env::var("DATABASE_URL").unwrap(); info!("Connecting to {connection_string}"); - let config = PgConfigBuilder { + let config = PgConfig { connection_string: Some(connection_string), - #[cfg(feature = "ssl")] - ca_root_file: None, - #[cfg(feature = "ssl")] - danger_accept_invalid_certs: None, default_srid, - pool_size: None, tables: tables.map(|s| { s.iter() .map(|v| (v.0.to_string(), v.1.clone())) @@ -45,24 +39,25 @@ pub async fn mock_config( .map(|v| (v.0.to_string(), v.1.clone())) .collect::>() }), + ..Default::default() }; config.finalize().expect("Unable to finalize config") } +#[allow(dead_code)] +pub async fn mock_empty_config() -> PgConfig { + mock_config(None, None, None).await +} + #[allow(dead_code)] pub async fn mock_pool() -> Pool { - let res = Pool::new(&mock_config(None, None, None).await).await; + let res = Pool::new(&mock_empty_config().await).await; res.expect("Failed to create pool") } #[allow(dead_code)] -pub async fn mock_sources( - functions: Option>, - tables: Option>, - default_srid: Option, -) -> MockSource { - let cfg = mock_config(functions, tables, default_srid).await; - let res = resolve_pg_data(cfg, IdResolver::default()).await; +pub async fn mock_sources(config: PgConfig) -> MockSource { + let res = resolve_pg_data(config, IdResolver::default()).await; let res = res.expect("Failed to resolve pg data"); (res.0, res.1) } @@ -74,27 +69,22 @@ pub async fn mock_app_data(sources: Sources) -> Data { #[allow(dead_code)] pub async fn mock_unconfigured() -> MockSource { - mock_sources(None, None, None).await + mock_sources(mock_empty_config().await).await } #[allow(dead_code)] pub async fn mock_unconfigured_srid(default_srid: Option) -> MockSource { - mock_sources(None, None, default_srid).await -} - -#[allow(dead_code)] -pub async fn mock_configured() -> MockSource { - mock_sources(mock_func_config(), mock_table_config(), None).await + mock_sources(mock_config(None, None, default_srid).await).await } #[allow(dead_code)] pub async fn mock_configured_funcs() -> MockSource { - mock_sources(mock_func_config(), None, None).await + mock_sources(mock_config(mock_func_config(), None, None).await).await } #[allow(dead_code)] -pub async fn mock_configured_tables(default_srid: Option) -> MockSource { - mock_sources(None, mock_table_config(), default_srid).await +pub async fn mock_configured_tables(default_srid: Option) -> PgConfig { + mock_config(None, mock_table_config(), default_srid).await } pub fn mock_func_config() -> Option> { @@ -298,7 +288,7 @@ pub fn props(props: &[(&'static str, &'static str)]) -> HashMap #[allow(dead_code)] pub fn table<'a>(mock: &'a MockSource, name: &str) -> &'a TableInfo { let (_, PgConfig { tables, .. }) = mock; - tables.get(name).unwrap() + tables.as_ref().map(|v| v.get(name).unwrap()).unwrap() } #[allow(dead_code)] From 918b97c832fb9c3c8d295f7322b622d0389c7731 Mon Sep 17 00:00:00 2001 From: Yuri Astrakhan Date: Sat, 10 Dec 2022 11:39:30 -0500 Subject: [PATCH 15/16] minor panic fix --- src/pg/function_source.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pg/function_source.rs b/src/pg/function_source.rs index 461f27dd1..9ce2bb5cd 100644 --- a/src/pg/function_source.rs +++ b/src/pg/function_source.rs @@ -38,7 +38,7 @@ pub async fn get_function_sources(pool: &Pool) -> Result {} - _ => panic!("Invalid output record names or types"), + _ => panic!("Invalid output record names or types: {output_record_names:?} {output_record_types:?}"), } assert!(output_type == "bytea" || output_type == "record"); From 580094ce22ad5755f1d971fa164e69a604ed6a26 Mon Sep 17 00:00:00 2001 From: Yuri Astrakhan Date: Sat, 10 Dec 2022 14:45:21 -0500 Subject: [PATCH 16/16] hide auto-schema for now --- src/pg/config.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/pg/config.rs b/src/pg/config.rs index aa23b32d9..2e23987e9 100644 --- a/src/pg/config.rs +++ b/src/pg/config.rs @@ -194,9 +194,9 @@ pub struct PgConfig { pub default_srid: Option, #[serde(skip_serializing_if = "Option::is_none")] pub pool_size: Option, - #[serde(skip_serializing_if = "Option::is_none")] + #[serde(skip)] pub auto_tables: Option, - #[serde(skip_serializing_if = "Option::is_none")] + #[serde(skip)] pub auto_functions: Option, #[serde(skip_serializing_if = "Option::is_none")] pub tables: Option,