Skip to content

improve error msgs #108

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

Merged
merged 4 commits into from
Oct 6, 2023
Merged
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
39 changes: 39 additions & 0 deletions cli/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ anyhow = "1.0.69"
clap = { version = "4.1.6", features = ["derive"] }
dirs = "5.0.1"
postgrest = "1.5.0"
regex = "1.9"
reqwest = { version = "0.11.14", features = ["json", "native-tls-vendored"] }
rpassword = "7.2.0"
serde = { version = "1.0.156", features = ["derive"] }
Expand Down
4 changes: 4 additions & 0 deletions cli/src/commands/publish.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ pub async fn publish(
let request = create_publish_package_request(&payload);
client.publish_package(&jwt, &request).await?;

if payload.install_files.is_empty() {
return Err(anyhow::anyhow!("No valid script file (.sql) found."));
}

let mut num_published = 0;

for install_file in &payload.install_files {
Expand Down
61 changes: 39 additions & 22 deletions cli/src/models.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ impl Payload {

if !util::is_valid_extension_name(&extension_name) {
return Err(anyhow::anyhow!(
"invalid extension name detected {}",
"Invalid extension name detected: {}. It must begin with an alphabet, contain only alphanumeric characters or `_` and should be between 2 and 32 characters long.",
extension_name
));
}
Expand All @@ -166,31 +166,46 @@ impl Payload {
match &parts[..] {
[file_ext_name, ver] => {
// Make sure the file's extension name matches the control file
if file_ext_name == &extension_name && util::is_valid_version(ver) {
let ifile = InstallFile {
filename: file_name.to_string(),
version: ver.to_string(),
body: fs::read_to_string(&path)
.context(format!("Failed to read file {}", &file_name))?,
};
install_files.push(ifile);
if file_ext_name != &extension_name {
println!("Warning: file `{file_name}` will be skipped because its extension name(`{file_ext_name}`) doesn't match `{extension_name}`");
continue;
}
if !util::is_valid_version(ver) {
println!("Warning: file `{file_name}` will be skipped because its version (`{ver}`) is invalid. It should be have the format `major.minor.patch`.");
continue;
}

let ifile = InstallFile {
filename: file_name.to_string(),
version: ver.to_string(),
body: fs::read_to_string(&path)
.context(format!("Failed to read file {}", &file_name))?,
};
install_files.push(ifile);
}
[file_ext_name, from_ver, to_ver] => {
// Make sure the file's extension name matches the control file
if file_ext_name == &extension_name
&& util::is_valid_version(from_ver)
&& util::is_valid_version(to_ver)
{
let ufile = UpgradeFile {
filename: file_name.to_string(),
from_version: from_ver.to_string(),
to_version: to_ver.to_string(),
body: fs::read_to_string(&path)
.context(format!("Failed to read file {}", &file_name))?,
};
upgrade_files.push(ufile);
if file_ext_name != &extension_name {
println!("Warning: file `{file_name}` will be skipped because its extension name(`{file_ext_name}`) doesn't match `{extension_name}`");
continue;
}
if !util::is_valid_version(from_ver) {
println!("Warning: file `{file_name}` will be skipped because its from version(`{from_ver}`) is invalid. It should be have the format `major.minor.patch`.");
continue;
}
if !util::is_valid_version(to_ver) {
println!("Warning: file `{file_name}` will be skipped because its from version(`{to_ver}`) is invalid. It should be have the format `major.minor.patch`.");
continue;
}

let ufile = UpgradeFile {
filename: file_name.to_string(),
from_version: from_ver.to_string(),
to_version: to_ver.to_string(),
body: fs::read_to_string(&path)
.context(format!("Failed to read file {}", &file_name))?,
};
upgrade_files.push(ufile);
}
_ => (),
}
Expand Down Expand Up @@ -264,7 +279,9 @@ impl ControlFileRef {
return self.read_control_line_value(line);
}
}
Err(anyhow::anyhow!("default version is required"))
Err(anyhow::anyhow!(
"`default_version` in control file is required"
))
}

fn read_control_line_value(&self, line: &str) -> anyhow::Result<String> {
Expand Down
24 changes: 21 additions & 3 deletions cli/src/util.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use anyhow::Context;
use regex::Regex;
use sqlx::postgres::PgConnection;
use sqlx::Connection;
use std::fs::{File, OpenOptions};
Expand All @@ -10,11 +11,28 @@ pub async fn get_connection(connection_str: &str) -> anyhow::Result<PgConnection
.context("Failed to establish PostgreSQL connection")
}

pub fn is_valid_extension_name(_name: &str) -> bool {
true
pub fn is_valid_extension_name(name: &str) -> bool {
let name_regex = Regex::new(r"^[A-z][A-z0-9\_]{2,32}$").expect("regex is valid");
name_regex.is_match(name)
}

pub fn is_valid_version(_version: &str) -> bool {
pub fn is_valid_version(version: &str) -> bool {
let nums: Vec<&str> = version.split('.').collect();
if nums.len() != 3 {
println!("1");
return false;
}

for num_str in nums {
let num: i16 = match num_str.parse() {
Ok(n) => n,
_ => return false,
};
if num < 0 {
return false;
}
}

true
}

Expand Down