tests, report unknown cfg, rm catalog vector flds (#551)

* clean up reporting of the un-used config params - instead of printing,
collect them and print in one place if needed (allows testing too)
* remove `vector_layer` in catalog - too verbose, not needed - can be
received via tilejson for individual source
* clean up tests so that they all use the same config yaml
This commit is contained in:
Yuri Astrakhan 2023-01-12 11:48:15 -05:00 committed by GitHub
parent 8945438069
commit f23da97691
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 517 additions and 498 deletions

View File

@ -6,7 +6,6 @@ use std::path::Path;
use std::pin::Pin; use std::pin::Pin;
use futures::future::try_join_all; use futures::future::try_join_all;
use log::warn;
use serde::{Deserialize, Serialize}; use serde::{Deserialize, Serialize};
use serde_yaml::Value; use serde_yaml::Value;
use subst::VariableMap; use subst::VariableMap;
@ -40,12 +39,13 @@ pub struct Config {
impl Config { impl Config {
/// Apply defaults to the config, and validate if there is a connection string /// Apply defaults to the config, and validate if there is a connection string
pub fn finalize(&mut self) -> Result<()> { pub fn finalize(&mut self) -> Result<Unrecognized> {
report_unrecognized_config("", &self.unrecognized); let mut res = Unrecognized::new();
copy_unrecognized_config(&mut res, "", &self.unrecognized);
let mut any = if let Some(pg) = &mut self.postgres { let mut any = if let Some(pg) = &mut self.postgres {
for pg in pg.iter_mut() { for pg in pg.iter_mut() {
pg.finalize()?; res.extend(pg.finalize()?);
} }
!pg.is_empty() !pg.is_empty()
} else { } else {
@ -53,21 +53,21 @@ impl Config {
}; };
any |= if let Some(cfg) = &mut self.pmtiles { any |= if let Some(cfg) = &mut self.pmtiles {
cfg.finalize("pmtiles.")?; res.extend(cfg.finalize("pmtiles.")?);
!cfg.is_empty() !cfg.is_empty()
} else { } else {
false false
}; };
any |= if let Some(cfg) = &mut self.mbtiles { any |= if let Some(cfg) = &mut self.mbtiles {
cfg.finalize("mbtiles.")?; res.extend(cfg.finalize("mbtiles.")?);
!cfg.is_empty() !cfg.is_empty()
} else { } else {
false false
}; };
if any { if any {
Ok(()) Ok(res)
} else { } else {
Err(NoSources) Err(NoSources)
} }
@ -103,10 +103,18 @@ impl Config {
} }
} }
pub fn report_unrecognized_config(prefix: &str, unrecognized: &HashMap<String, Value>) { pub type Unrecognized = HashMap<String, Value>;
for key in unrecognized.keys() {
warn!("Unrecognized config key: {prefix}{key}"); pub fn copy_unrecognized_config(
} result: &mut Unrecognized,
prefix: &str,
unrecognized: &Unrecognized,
) {
result.extend(
unrecognized
.iter()
.map(|(k, v)| (format!("{prefix}{k}"), v.clone())),
);
} }
/// Read config from a file /// Read config from a file
@ -142,7 +150,8 @@ pub mod tests {
pub fn assert_config(yaml: &str, expected: &Config) { pub fn assert_config(yaml: &str, expected: &Config) {
let mut config = parse_cfg(yaml); let mut config = parse_cfg(yaml);
config.finalize().unwrap(); let res = config.finalize().unwrap();
assert!(res.is_empty(), "unrecognized config: {res:?}");
assert_eq!(&config, expected); assert_eq!(&config, expected);
} }
} }

View File

@ -8,7 +8,7 @@ use log::{info, warn};
use serde::{Deserialize, Serialize}; use serde::{Deserialize, Serialize};
use serde_yaml::Value; use serde_yaml::Value;
use crate::config::report_unrecognized_config; use crate::config::{copy_unrecognized_config, Unrecognized};
use crate::file_config::FileError::{InvalidFilePath, InvalidSourceFilePath}; use crate::file_config::FileError::{InvalidFilePath, InvalidSourceFilePath};
use crate::utils::sorted_opt_map; use crate::utils::sorted_opt_map;
use crate::OneOrMany::{Many, One}; use crate::OneOrMany::{Many, One};
@ -84,11 +84,12 @@ pub struct FileConfigSource {
} }
impl FileConfigEnum { impl FileConfigEnum {
pub fn finalize(&self, prefix: &str) -> Result<&Self, Error> { pub fn finalize(&self, prefix: &str) -> Result<Unrecognized, Error> {
let mut res = Unrecognized::new();
if let Self::Config(cfg) = self { if let Self::Config(cfg) = self {
report_unrecognized_config(prefix, &cfg.unrecognized); copy_unrecognized_config(&mut res, prefix, &cfg.unrecognized);
} }
Ok(self) Ok(res)
} }
#[must_use] #[must_use]
@ -239,7 +240,8 @@ mod tests {
path: /tmp/file.ext path: /tmp/file.ext
"}) "})
.unwrap(); .unwrap();
cfg.finalize("").unwrap(); let res = cfg.finalize("").unwrap();
assert!(res.is_empty(), "unrecognized config: {res:?}");
let FileConfigEnum::Config(cfg) = cfg else { let FileConfigEnum::Config(cfg) = cfg else {
panic!(); panic!();
}; };

View File

@ -2,7 +2,7 @@ use futures::future::try_join;
use serde::{Deserialize, Serialize}; use serde::{Deserialize, Serialize};
use tilejson::TileJSON; use tilejson::TileJSON;
use crate::config::report_unrecognized_config; use crate::config::{copy_unrecognized_config, Unrecognized};
use crate::pg::config_function::FuncInfoSources; use crate::pg::config_function::FuncInfoSources;
use crate::pg::config_table::TableInfoSources; use crate::pg::config_table::TableInfoSources;
use crate::pg::configurator::PgBuilder; use crate::pg::configurator::PgBuilder;
@ -55,22 +55,23 @@ pub struct PgCfgPublishType {
impl PgConfig { impl PgConfig {
/// Apply defaults to the config, and validate if there is a connection string /// Apply defaults to the config, and validate if there is a connection string
pub fn finalize(&mut self) -> Result<&Self> { pub fn finalize(&mut self) -> Result<Unrecognized> {
let mut res = Unrecognized::new();
if let Some(ref ts) = self.tables { if let Some(ref ts) = self.tables {
for (k, v) in ts { for (k, v) in ts {
report_unrecognized_config(&format!("tables.{k}."), &v.unrecognized); copy_unrecognized_config(&mut res, &format!("tables.{k}."), &v.unrecognized);
} }
} }
if let Some(ref fs) = self.functions { if let Some(ref fs) = self.functions {
for (k, v) in fs { for (k, v) in fs {
report_unrecognized_config(&format!("functions.{k}."), &v.unrecognized); copy_unrecognized_config(&mut res, &format!("functions.{k}."), &v.unrecognized);
} }
} }
if self.tables.is_none() && self.functions.is_none() && self.auto_publish.is_none() { if self.tables.is_none() && self.functions.is_none() && self.auto_publish.is_none() {
self.auto_publish = Some(BoolOrObject::Bool(true)); self.auto_publish = Some(BoolOrObject::Bool(true));
} }
Ok(self) Ok(res)
} }
pub async fn resolve(&mut self, id_resolver: IdResolver) -> crate::Result<Sources> { pub async fn resolve(&mut self, id_resolver: IdResolver) -> crate::Result<Sources> {

View File

@ -18,7 +18,7 @@ use itertools::Itertools;
use log::{debug, error}; use log::{debug, error};
use martin_tile_utils::DataFormat; use martin_tile_utils::DataFormat;
use serde::{Deserialize, Serialize}; use serde::{Deserialize, Serialize};
use tilejson::{TileJSON, VectorLayer}; use tilejson::TileJSON;
use crate::source::{Source, Sources, UrlQuery, Xyz}; use crate::source::{Source, Sources, UrlQuery, Xyz};
use crate::srv::config::{SrvConfig, KEEP_ALIVE_DEFAULT, LISTEN_ADDRESSES_DEFAULT}; use crate::srv::config::{SrvConfig, KEEP_ALIVE_DEFAULT, LISTEN_ADDRESSES_DEFAULT};
@ -106,8 +106,6 @@ pub struct IndexEntry {
pub description: Option<String>, pub description: Option<String>,
#[serde(skip_serializing_if = "Option::is_none")] #[serde(skip_serializing_if = "Option::is_none")]
pub attribution: Option<String>, pub attribution: Option<String>,
#[serde(skip_serializing_if = "Option::is_none")]
pub vector_layer: Option<Vec<VectorLayer>>,
} }
impl PartialOrd<Self> for IndexEntry { impl PartialOrd<Self> for IndexEntry {
@ -159,7 +157,6 @@ async fn get_catalog(state: Data<AppState>) -> impl Responder {
name: tilejson.name, name: tilejson.name,
description: tilejson.description, description: tilejson.description,
attribution: tilejson.attribution, attribution: tilejson.attribution,
vector_layer: tilejson.vector_layers,
} }
}) })
.sorted() .sorted()

View File

@ -126,17 +126,6 @@
"content_type": "application/x-protobuf", "content_type": "application/x-protobuf",
"description": "Major cities from Natural Earth data", "description": "Major cities from Natural Earth data",
"id": "world_cities", "id": "world_cities",
"name": "Major cities from Natural Earth data", "name": "Major cities from Natural Earth data"
"vector_layer": [
{
"description": "",
"fields": {
"name": "String"
},
"id": "cities",
"maxzoom": 6,
"minzoom": 0
}
]
} }
] ]

View File

@ -1,7 +1,6 @@
use ctor::ctor; use ctor::ctor;
use indoc::indoc; use indoc::indoc;
use itertools::Itertools; use itertools::Itertools;
use martin::pg::get_function_sources;
use martin::Xyz; use martin::Xyz;
pub mod utils; pub mod utils;
@ -12,26 +11,6 @@ fn init() {
let _ = env_logger::builder().is_test(true).try_init(); let _ = env_logger::builder().is_test(true).try_init();
} }
#[actix_rt::test]
async fn get_function_sources_ok() {
let pool = mock_pool().await;
let sources = get_function_sources(&pool).await.unwrap();
assert!(!sources.is_empty());
let funcs = sources.get("public").unwrap();
let source = funcs.get("function_zxy_query").unwrap();
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);
let source = funcs.get("function_zxy_query_jsonb").unwrap();
assert_eq!(source.1.schema, "public");
assert_eq!(source.1.function, "function_zxy_query_jsonb");
}
#[actix_rt::test] #[actix_rt::test]
async fn function_source_tilejson() { async fn function_source_tilejson() {
let mock = mock_sources(mock_pgcfg("connection_string: $DATABASE_URL")).await; let mock = mock_sources(mock_pgcfg("connection_string: $DATABASE_URL")).await;

View File

@ -16,8 +16,9 @@ fn init() {
} }
macro_rules! create_app { macro_rules! create_app {
($sources:literal) => {{ ($sources:expr) => {{
let sources = mock_sources(mock_pgcfg($sources)).await.0; let cfg = mock_cfg(indoc::indoc!($sources));
let sources = mock_sources(cfg).await.0;
let state = crate::utils::mock_app_data(sources).await; let state = crate::utils::mock_app_data(sources).await;
::actix_web::test::init_service( ::actix_web::test::init_service(
::actix_web::App::new() ::actix_web::App::new()
@ -34,7 +35,10 @@ fn test_get(path: &str) -> Request {
#[actix_rt::test] #[actix_rt::test]
async fn pg_get_catalog_ok() { async fn pg_get_catalog_ok() {
let app = create_app! { "connection_string: $DATABASE_URL" }; let app = create_app! { "
postgres:
connection_string: $DATABASE_URL
"};
let req = test_get("/catalog"); let req = test_get("/catalog");
let response = call_service(&app, req).await; let response = call_service(&app, req).await;
@ -55,8 +59,9 @@ async fn pg_get_catalog_ok() {
#[actix_rt::test] #[actix_rt::test]
async fn pg_get_table_source_ok() { async fn pg_get_table_source_ok() {
let app = create_app! { " let app = create_app! { "
connection_string: $DATABASE_URL postgres:
tables: connection_string: $DATABASE_URL
tables:
bad_srid: bad_srid:
schema: public schema: public
table: table_source table: table_source
@ -84,6 +89,24 @@ tables:
let req = test_get("/bad_srid"); let req = test_get("/bad_srid");
let response = call_service(&app, req).await; let response = call_service(&app, req).await;
assert_eq!(response.status(), StatusCode::NOT_FOUND); assert_eq!(response.status(), StatusCode::NOT_FOUND);
}
#[actix_rt::test]
async fn pg_get_table_source_ok_rewrite() {
let app = create_app! { "
postgres:
connection_string: $DATABASE_URL
tables:
table_source:
schema: public
table: table_source
srid: 4326
geometry_column: geom
bounds: [-180.0, -90.0, 180.0, 90.0]
geometry_type: GEOMETRY
properties:
gid: int4
" };
let req = TestRequest::get() let req = TestRequest::get()
.uri("/table_source?token=martin") .uri("/table_source?token=martin")
@ -101,8 +124,9 @@ tables:
#[actix_rt::test] #[actix_rt::test]
async fn pg_get_table_source_tile_ok() { async fn pg_get_table_source_tile_ok() {
let app = create_app! { " let app = create_app! { "
connection_string: $DATABASE_URL postgres:
tables: connection_string: $DATABASE_URL
tables:
points2: points2:
schema: public schema: public
table: points2 table: points2
@ -192,8 +216,9 @@ tables:
#[actix_rt::test] #[actix_rt::test]
async fn pg_get_table_source_multiple_geom_tile_ok() { async fn pg_get_table_source_multiple_geom_tile_ok() {
let app = create_app! { " let app = create_app! { "
connection_string: $DATABASE_URL postgres:
tables: connection_string: $DATABASE_URL
tables:
points2: points2:
schema: public schema: public
table: points2 table: points2
@ -283,8 +308,9 @@ tables:
#[actix_rt::test] #[actix_rt::test]
async fn pg_get_table_source_tile_minmax_zoom_ok() { async fn pg_get_table_source_tile_minmax_zoom_ok() {
let app = create_app! { " let app = create_app! { "
connection_string: $DATABASE_URL postgres:
tables: connection_string: $DATABASE_URL
tables:
points3857: points3857:
schema: public schema: public
table: points3857 table: points3857
@ -389,7 +415,10 @@ tables:
#[actix_rt::test] #[actix_rt::test]
async fn pg_get_function_tiles() { async fn pg_get_function_tiles() {
let app = create_app! { "connection_string: $DATABASE_URL" }; let app = create_app! { "
postgres:
connection_string: $DATABASE_URL
"};
let req = test_get("/function_zoom_xy/6/38/20"); let req = test_get("/function_zoom_xy/6/38/20");
assert!(call_service(&app, req).await.status().is_success()); assert!(call_service(&app, req).await.status().is_success());
@ -419,8 +448,9 @@ async fn pg_get_function_tiles() {
#[actix_rt::test] #[actix_rt::test]
async fn pg_get_composite_source_ok() { async fn pg_get_composite_source_ok() {
let app = create_app! { " let app = create_app! { "
connection_string: $DATABASE_URL postgres:
tables: connection_string: $DATABASE_URL
tables:
table_source_multiple_geom.geom2: table_source_multiple_geom.geom2:
schema: public schema: public
table: table_source_multiple_geom table: table_source_multiple_geom
@ -509,8 +539,9 @@ tables:
#[actix_rt::test] #[actix_rt::test]
async fn pg_get_composite_source_tile_ok() { async fn pg_get_composite_source_tile_ok() {
let app = create_app! { " let app = create_app! { "
connection_string: $DATABASE_URL postgres:
tables: connection_string: $DATABASE_URL
tables:
points_empty_srid: points_empty_srid:
schema: public schema: public
table: points_empty_srid table: points_empty_srid
@ -600,8 +631,9 @@ tables:
#[actix_rt::test] #[actix_rt::test]
async fn pg_get_composite_source_tile_minmax_zoom_ok() { async fn pg_get_composite_source_tile_minmax_zoom_ok() {
let app = create_app! { " let app = create_app! { "
connection_string: $DATABASE_URL postgres:
tables: connection_string: $DATABASE_URL
tables:
points1: points1:
schema: public schema: public
table: points1 table: points1
@ -664,7 +696,10 @@ tables:
#[actix_rt::test] #[actix_rt::test]
async fn pg_null_functions() { async fn pg_null_functions() {
let app = create_app! { "connection_string: $DATABASE_URL" }; let app = create_app! { "
postgres:
connection_string: $DATABASE_URL
"};
let req = test_get("/function_null/0/0/0"); let req = test_get("/function_null/0/0/0");
let response = call_service(&app, req).await; let response = call_service(&app, req).await;
@ -681,7 +716,10 @@ async fn pg_null_functions() {
#[actix_rt::test] #[actix_rt::test]
async fn pg_get_function_source_ok() { async fn pg_get_function_source_ok() {
let app = create_app! { "connection_string: $DATABASE_URL" }; let app = create_app! { "
postgres:
connection_string: $DATABASE_URL
"};
let req = test_get("/non_existent"); let req = test_get("/non_existent");
let response = call_service(&app, req).await; let response = call_service(&app, req).await;
@ -718,6 +756,14 @@ async fn pg_get_function_source_ok() {
let req = test_get("/function_zxy_row_key"); let req = test_get("/function_zxy_row_key");
let response = call_service(&app, req).await; let response = call_service(&app, req).await;
assert!(response.status().is_success()); assert!(response.status().is_success());
}
#[actix_rt::test]
async fn pg_get_function_source_ok_rewrite() {
let app = create_app! { "
postgres:
connection_string: $DATABASE_URL
"};
let req = TestRequest::get() let req = TestRequest::get()
.uri("/function_zxy_query?token=martin") .uri("/function_zxy_query?token=martin")
@ -745,7 +791,10 @@ async fn pg_get_function_source_ok() {
#[actix_rt::test] #[actix_rt::test]
async fn pg_get_function_source_tile_ok() { async fn pg_get_function_source_tile_ok() {
let app = create_app! { "connection_string: $DATABASE_URL" }; let app = create_app! { "
postgres:
connection_string: $DATABASE_URL
"};
let req = test_get("/function_zxy_query/0/0/0"); let req = test_get("/function_zxy_query/0/0/0");
let response = call_service(&app, req).await; let response = call_service(&app, req).await;
@ -755,8 +804,9 @@ async fn pg_get_function_source_tile_ok() {
#[actix_rt::test] #[actix_rt::test]
async fn pg_get_function_source_tile_minmax_zoom_ok() { async fn pg_get_function_source_tile_minmax_zoom_ok() {
let app = create_app! {" let app = create_app! {"
connection_string: $DATABASE_URL postgres:
functions: connection_string: $DATABASE_URL
functions:
function_source1: function_source1:
schema: public schema: public
function: function_zxy_query function: function_zxy_query
@ -811,7 +861,10 @@ functions:
#[actix_rt::test] #[actix_rt::test]
async fn pg_get_function_source_query_params_ok() { async fn pg_get_function_source_query_params_ok() {
let app = create_app! { "connection_string: $DATABASE_URL" }; let app = create_app! { "
postgres:
connection_string: $DATABASE_URL
"};
let req = test_get("/function_zxy_query_test/0/0/0"); let req = test_get("/function_zxy_query_test/0/0/0");
let response = call_service(&app, req).await; let response = call_service(&app, req).await;
@ -825,7 +878,10 @@ async fn pg_get_function_source_query_params_ok() {
#[actix_rt::test] #[actix_rt::test]
async fn pg_get_health_returns_ok() { async fn pg_get_health_returns_ok() {
let app = create_app! { "connection_string: $DATABASE_URL" }; let app = create_app! { "
postgres:
connection_string: $DATABASE_URL
"};
let req = test_get("/health"); let req = test_get("/health");
let response = call_service(&app, req).await; let response = call_service(&app, req).await;
@ -903,7 +959,7 @@ tables:
// -------------------------------------------- // --------------------------------------------
let state = crate::utils::mock_app_data(mock.0).await; let state = mock_app_data(mock.0).await;
let app = ::actix_web::test::init_service( let app = ::actix_web::test::init_service(
::actix_web::App::new() ::actix_web::App::new()
.app_data(state) .app_data(state)

View File

@ -25,6 +25,7 @@ pub fn mock_cfg(yaml: &str) -> Config {
}; };
let env = FauxEnv(vec![("DATABASE_URL", db_url.into())].into_iter().collect()); let env = FauxEnv(vec![("DATABASE_URL", db_url.into())].into_iter().collect());
let mut cfg: Config = subst::yaml::from_str(yaml, &env).unwrap(); let mut cfg: Config = subst::yaml::from_str(yaml, &env).unwrap();
cfg.finalize().unwrap(); let res = cfg.finalize().unwrap();
assert!(res.is_empty(), "unrecognized config: {res:?}");
cfg cfg
} }

View File

@ -1,9 +1,9 @@
use indoc::formatdoc;
pub use martin::args::Env; pub use martin::args::Env;
use martin::pg::{PgConfig, Pool, TableInfo}; use martin::pg::TableInfo;
use martin::OneOrMany::One; use martin::{Config, IdResolver, Source, Sources};
use martin::{Config, IdResolver, OneOrMany, Source, Sources};
use crate::FauxEnv; use crate::mock_cfg;
// //
// This file is used by many tests and benchmarks. // This file is used by many tests and benchmarks.
@ -15,25 +15,10 @@ pub type MockSource = (Sources, Config);
#[allow(dead_code)] #[allow(dead_code)]
#[must_use] #[must_use]
pub fn mock_pgcfg(yaml: &str) -> Config { pub fn mock_pgcfg(yaml: &str) -> Config {
let Ok(db_url) = std::env::var("DATABASE_URL") else { mock_cfg(&formatdoc! {"
panic!("DATABASE_URL env var is not set. Unable to do integration tests"); postgres:
}; {}
let env = FauxEnv(vec![("DATABASE_URL", db_url.into())].into_iter().collect()); ", yaml.replace('\n', "\n ")})
let cfg: PgConfig = subst::yaml::from_str(yaml, &env).unwrap();
let mut config = Config {
postgres: Some(One(cfg)),
..Default::default()
};
config.finalize().unwrap();
config
}
#[allow(dead_code)]
pub async fn mock_pool() -> Pool {
let cfg = mock_pgcfg("connection_string: $DATABASE_URL");
let OneOrMany::One(cfg) = cfg.postgres.unwrap() else { panic!() };
let res = Pool::new(&cfg).await;
res.expect("Failed to create pool")
} }
#[allow(dead_code)] #[allow(dead_code)]