Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/common/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,10 @@ pub struct Cli {
#[clap(long)]
pub db_name: Option<String>,

/// Custom database connection URL (overrides individual connection parameters if provided)
#[clap(long)]
pub db_url: Option<String>,

/// PostgreSQL schema search path (default is "$user,public") https://www.postgresql.org/docs/current/ddl-schemas.html#DDL-SCHEMAS-PATH
#[clap(long)]
pub pg_search_path: Option<String>,
Expand Down
121 changes: 82 additions & 39 deletions src/common/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,16 +40,18 @@ pub struct GenerateTypesConfig {
pub struct DbConnectionConfig {
#[serde(rename = "DB_TYPE")]
pub db_type: DatabaseType,
#[serde(rename = "DB_HOST")]
#[serde(rename = "DB_HOST", default)]
pub db_host: String,
#[serde(rename = "DB_PORT")]
#[serde(rename = "DB_PORT", default)]
pub db_port: u16,
#[serde(rename = "DB_USER")]
#[serde(rename = "DB_USER", default)]
pub db_user: String,
Comment on lines +43 to 48
Copy link

Copilot AI Nov 30, 2025

Choose a reason for hiding this comment

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

Using default attribute on non-Optional fields with primitive types may cause deserialization issues. When DB_URL is provided, these fields should be Option<String> and Option<u16> respectively, or the struct should validate that either DB_URL or all individual parameters are present.

Copilot uses AI. Check for mistakes.
#[serde(rename = "DB_PASS")]
pub db_pass: Option<String>,
#[serde(rename = "DB_NAME")]
pub db_name: Option<String>,
#[serde(rename = "DB_URL")]
pub db_url: Option<String>,
#[serde(rename = "PG_SEARCH_PATH")]
pub pg_search_path: Option<String>,
#[serde(rename = "POOL_SIZE", default = "default_pool_size")]
Expand Down Expand Up @@ -218,43 +220,73 @@ impl Config {
panic!("")
});

let db_host = &CLI_ARGS
.db_host
let db_url = &CLI_ARGS
.db_url
.clone()
.or_else(|| dotenv.db_host.clone())
.or_else(|| default_config.map(|x| x.db_host.clone()))
.expect(
.or_else(|| dotenv.db_url.clone())
.or_else(|| default_config.map(|x| x.db_url.clone()).flatten());

let db_host_chain = || {
CLI_ARGS
.db_host
.clone()
.or_else(|| dotenv.db_host.clone())
.or_else(|| default_config.map(|x| x.db_host.clone()))
};

let db_host = match (db_url.is_some(), db_host_chain()) {
(true, Some(v)) => v,
(true, None) => String::new(),
(false, Some(v)) => v,
(false, None) => panic!(
Comment on lines +237 to +241
Copy link

Copilot AI Nov 30, 2025

Choose a reason for hiding this comment

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

The pattern (db_url.is_some(), db_host_chain()) is repeated three times for db_host, db_port, and db_user. Consider extracting this logic into a helper function to reduce code duplication and improve maintainability.

Copilot uses AI. Check for mistakes.
r"
Failed to fetch DB host.
Please provide it at least through a CLI arg or an environment variable or through
file based configuration
",
);

let db_port = &CLI_ARGS
.db_port
.or(dotenv.db_port)
.or_else(|| default_config.map(|x| x.db_port))
.expect(
Failed to fetch DB host.
Please provide it at least through a CLI arg or an environment variable or through
file based configuration, or provide a custom DB_URL
"
),
};

let db_port_chain = || {
CLI_ARGS
.db_port
.or(dotenv.db_port)
.or_else(|| default_config.map(|x| x.db_port))
};

let db_port = match (db_url.is_some(), db_port_chain()) {
(true, Some(v)) => v,
(true, None) => 0,
(false, Some(v)) => v,
Comment on lines +258 to +260
Copy link

Copilot AI Nov 30, 2025

Choose a reason for hiding this comment

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

Using 0 as a default port when db_url is provided but db_port is not may cause issues if the port value is used elsewhere in the code for validation or logging. Consider using a more explicit sentinel value or documenting this behavior, as port 0 is typically reserved and may not be a valid database port.

Suggested change
(true, Some(v)) => v,
(true, None) => 0,
(false, Some(v)) => v,
(true, Some(v)) => Some(v),
(true, None) => None,
(false, Some(v)) => Some(v),

Copilot uses AI. Check for mistakes.
(false, None) => panic!(
r"
Failed to fetch DB port.
Please provide it at least through a CLI arg or an environment variable or through
file based configuration
",
);

let db_user = &CLI_ARGS
.db_user
.clone()
.or_else(|| dotenv.db_user.clone())
.or_else(|| default_config.map(|x| x.db_user.clone()))
.expect(
Failed to fetch DB port.
Please provide it at least through a CLI arg or an environment variable or through
file based configuration, or provide a custom DB_URL
"
),
};

let db_user_chain = || {
CLI_ARGS
.db_user
.clone()
.or_else(|| dotenv.db_user.clone())
.or_else(|| default_config.map(|x| x.db_user.clone()))
};

let db_user = match (db_url.is_some(), db_user_chain()) {
(true, Some(v)) => v,
(true, None) => String::new(),
(false, Some(v)) => v,
(false, None) => panic!(
r"
Failed to fetch DB user.
Please provide it at least through a CLI arg or an environment variable or through
file based configuration
",
);
Failed to fetch DB user.
Please provide it at least through a CLI arg or an environment variable or through
file based configuration, or provide a custom DB_URL
"
),
};

let db_pass = &CLI_ARGS
.db_pass
Expand Down Expand Up @@ -286,11 +318,12 @@ impl Config {

DbConnectionConfig {
db_type: db_type.to_owned(),
db_host: db_host.to_owned(),
db_port: db_port.to_owned(),
db_user: db_user.to_owned(),
db_host,
db_port,
db_user,
Comment on lines +321 to +323
Copy link

Copilot AI Nov 30, 2025

Choose a reason for hiding this comment

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

[nitpick] Removing .to_owned() calls here changes the ownership semantics. Since these are now owned values instead of references, this is correct, but the surrounding code at lines 328-329 still uses .to_owned() creating inconsistency. Consider applying the same pattern to db_pass and db_name for consistency.

Copilot uses AI. Check for mistakes.
db_pass: db_pass.to_owned(),
db_name: db_name.to_owned(),
db_url: db_url.to_owned(),
pg_search_path: pg_search_path.to_owned(),
pool_size,
connection_timeout,
Expand Down Expand Up @@ -332,6 +365,11 @@ impl Config {
/// This is to follow the spec of connection string for MySQL
/// https://dev.mysql.com/doc/connector-j/8.1/en/connector-j-reference-jdbc-url-format.html
pub fn get_mysql_cred_str(&self, conn: &DbConnectionConfig) -> String {
// If custom DB_URL is provided, use it directly
if let Some(db_url) = &conn.db_url {
return db_url.to_owned();
}

format!(
"mysql://{user}:{pass}@{host}:{port}/{db_name}",
user = &conn.db_user,
Expand All @@ -344,6 +382,11 @@ impl Config {
}

pub fn get_postgres_cred(&self, conn: &DbConnectionConfig) -> String {
// If custom DB_URL is provided, use it directly
if let Some(db_url) = &conn.db_url {
return db_url.to_owned();
}

format!(
"postgresql://{user}:{pass}@{host}:{port}/{db_name}",
user = &conn.db_user,
Expand Down
2 changes: 2 additions & 0 deletions src/common/dotenv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ pub struct Dotenv {
pub db_port: Option<u16>,
pub db_pass: Option<String>,
pub db_name: Option<String>,
pub db_url: Option<String>,
pub pg_search_path: Option<String>,
}

Expand Down Expand Up @@ -44,6 +45,7 @@ impl Dotenv {
db_port: Self::get_var("DB_PORT").map(|val| val.parse::<u16>().expect("DB_PORT is not a valid integer")),
db_pass: Self::get_var("DB_PASS"),
db_name: Self::get_var("DB_NAME"),
db_url: Self::get_var("DB_URL"),
pg_search_path: Self::get_var("PG_SEARCH_PATH"),
}
}
Expand Down
Loading