Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added validate function to PgConfig and invoked it in finalize and pool initialization #1546

Merged
merged 10 commits into from
Oct 23, 2024
20 changes: 20 additions & 0 deletions martin/src/pg/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use crate::pg::builder::PgBuilder;
use crate::pg::config_function::FuncInfoSources;
use crate::pg::config_table::TableInfoSources;
use crate::pg::utils::on_slow;
use crate::pg::PgError;
use crate::pg::PgResult;
use crate::source::TileInfoSources;
use crate::utils::{IdResolver, OptBoolObj, OptOneMany};
Expand Down Expand Up @@ -94,6 +95,23 @@ pub struct PgCfgPublishFuncs {

impl PgConfig {
/// Apply defaults to the config, and validate if there is a connection string
pub fn validate(&self) -> PgResult<()> {
if let Some(pool_size) = self.pool_size {
if pool_size < 1 {
return Err(PgError::ValidationError(
"pool_size must be greater than or equal to 1.".to_string(),
));
}
}
if self.connection_string.is_none() {
return Err(PgError::ValidationError(
"A connection string must be provided.".to_string(),
));
}

Ok(())
}

pub fn finalize(&mut self) -> PgResult<UnrecognizedValues> {
let mut res = UnrecognizedValues::new();
if let Some(ref ts) = self.tables {
Expand All @@ -110,10 +128,12 @@ impl PgConfig {
self.auto_publish = OptBoolObj::Bool(true);
}

self.validate()?;
Ok(res)
}

pub async fn resolve(&mut self, id_resolver: IdResolver) -> MartinResult<TileInfoSources> {
self.validate()?;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need to validate it in so many places - here, in the pool, and in finalize? I don't remember for sure which codepath always executes, but we should probably limit it to just one. Probably finalize? Please check. Thx!

let pg = PgBuilder::new(self, id_resolver).await?;
let inst_tables = on_slow(
pg.instantiate_tables(),
Expand Down
3 changes: 3 additions & 0 deletions martin/src/pg/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,4 +69,7 @@ pub enum PgError {

#[error(r#"Unable to get tile {2:#} with {:?} params from {1}: {0}"#, query_to_json(.3.as_ref()))]
GetTileWithQueryError(#[source] TokioPgError, String, TileCoord, Option<UrlQuery>),

#[error("Validation error: {0}")]
ValidationError(String),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets call this ConfigError. Also, at least for now, all errors are &'static str - so we don't need to call .into() on them to convert str into an allocated String.

}
2 changes: 2 additions & 0 deletions martin/src/pg/pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ pub struct PgPool {

impl PgPool {
pub async fn new(config: &PgConfig) -> PgResult<Self> {
config.validate()?;

let (id, mgr) = Self::parse_config(config)?;

let pool = Pool::builder(mgr)
Expand Down
Loading