refactor(rules): replace unwrap() with explicit error handling and propagate errors safely
- Replace Option::unwrap() with pattern matching in add_rule label check - Handle home directory resolution failures with proper Error types - Add directory creation in save() to ensure parent directories exist - Replace chrono unwraps with ? operator for date calculations - Use unwrap_or with sensible defaults for parsing edge cases - Keep unwrap in test code where immediate failure is desired - All existing functionality preserved; no API changes
This commit is contained in:
committed by
Jeremiah Russell
parent
74512bdea3
commit
a43eb9e4a2
26
src/rules.rs
26
src/rules.rs
@@ -202,9 +202,11 @@ impl Rules {
|
|||||||
current_labels.append(&mut ls);
|
current_labels.append(&mut ls);
|
||||||
}
|
}
|
||||||
|
|
||||||
if label.is_some() && current_labels.contains(label.unwrap()) {
|
if let Some(label_ref) = label {
|
||||||
log::warn!("a rule already applies to label {}", label.unwrap());
|
if current_labels.contains(label_ref) {
|
||||||
return self;
|
log::warn!("a rule already applies to label {}", label_ref);
|
||||||
|
return self;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
let id = if let Some((_, max)) = self.rules.iter().max_by_key(|(_, r)| r.id()) {
|
let id = if let Some((_, max)) = self.rules.iter().max_by_key(|(_, r)| r.id()) {
|
||||||
@@ -497,9 +499,16 @@ impl Rules {
|
|||||||
/// * IO errors when writing to the file system
|
/// * IO errors when writing to the file system
|
||||||
/// * File system permission errors
|
/// * File system permission errors
|
||||||
pub fn save(&self) -> Result<()> {
|
pub fn save(&self) -> Result<()> {
|
||||||
let home_dir = env::home_dir().unwrap();
|
let home_dir = env::home_dir().ok_or_else(|| {
|
||||||
|
Error::HomeExpansionFailed("~/.cull-gmail/rules.toml".to_string())
|
||||||
|
})?;
|
||||||
let path = PathBuf::new().join(home_dir).join(".cull-gmail/rules.toml");
|
let path = PathBuf::new().join(home_dir).join(".cull-gmail/rules.toml");
|
||||||
|
|
||||||
|
// Ensure directory exists
|
||||||
|
if let Some(parent) = path.parent() {
|
||||||
|
fs::create_dir_all(parent)?;
|
||||||
|
}
|
||||||
|
|
||||||
let res = toml::to_string(self);
|
let res = toml::to_string(self);
|
||||||
log::trace!("toml conversion result: {res:#?}");
|
log::trace!("toml conversion result: {res:#?}");
|
||||||
|
|
||||||
@@ -536,7 +545,9 @@ impl Rules {
|
|||||||
/// * TOML parsing errors if the file is malformed
|
/// * TOML parsing errors if the file is malformed
|
||||||
/// * File not found errors if the configuration doesn't exist
|
/// * File not found errors if the configuration doesn't exist
|
||||||
pub fn load() -> Result<Rules> {
|
pub fn load() -> Result<Rules> {
|
||||||
let home_dir = env::home_dir().unwrap();
|
let home_dir = env::home_dir().ok_or_else(|| {
|
||||||
|
Error::HomeExpansionFailed("~/.cull-gmail/rules.toml".to_string())
|
||||||
|
})?;
|
||||||
let path = PathBuf::new().join(home_dir).join(".cull-gmail/rules.toml");
|
let path = PathBuf::new().join(home_dir).join(".cull-gmail/rules.toml");
|
||||||
log::trace!("Loading config from {}", path.display());
|
log::trace!("Loading config from {}", path.display());
|
||||||
|
|
||||||
@@ -584,7 +595,10 @@ mod tests {
|
|||||||
fn setup_test_environment() {
|
fn setup_test_environment() {
|
||||||
get_test_logger();
|
get_test_logger();
|
||||||
// Clean up any existing test files
|
// Clean up any existing test files
|
||||||
let home_dir = env::home_dir().unwrap();
|
let Some(home_dir) = env::home_dir() else {
|
||||||
|
// Skip cleanup if home directory cannot be determined
|
||||||
|
return;
|
||||||
|
};
|
||||||
let test_config_dir = home_dir.join(".cull-gmail");
|
let test_config_dir = home_dir.join(".cull-gmail");
|
||||||
let test_rules_file = test_config_dir.join("rules.toml");
|
let test_rules_file = test_config_dir.join("rules.toml");
|
||||||
if test_rules_file.exists() {
|
if test_rules_file.exists() {
|
||||||
|
|||||||
@@ -320,7 +320,7 @@ impl EolRule {
|
|||||||
/// Describe the action that will be performed by the rule and its conditions
|
/// Describe the action that will be performed by the rule and its conditions
|
||||||
fn get_action_period_count_strings(&self) -> (String, usize, String) {
|
fn get_action_period_count_strings(&self) -> (String, usize, String) {
|
||||||
let count = &self.retention[2..];
|
let count = &self.retention[2..];
|
||||||
let count = count.parse::<usize>().unwrap();
|
let count = count.parse::<usize>().unwrap_or(0); // Default to 0 if parsing fails
|
||||||
let mut period = match self.retention.chars().nth(0) {
|
let mut period = match self.retention.chars().nth(0) {
|
||||||
Some('d') => "day",
|
Some('d') => "day",
|
||||||
Some('w') => "week",
|
Some('w') => "week",
|
||||||
@@ -375,11 +375,11 @@ impl EolRule {
|
|||||||
let deadline = match message_age {
|
let deadline = match message_age {
|
||||||
MessageAge::Days(c) => {
|
MessageAge::Days(c) => {
|
||||||
let delta = TimeDelta::days(c);
|
let delta = TimeDelta::days(c);
|
||||||
today.checked_sub_signed(delta).unwrap()
|
today.checked_sub_signed(delta)?
|
||||||
}
|
}
|
||||||
MessageAge::Weeks(c) => {
|
MessageAge::Weeks(c) => {
|
||||||
let delta = TimeDelta::weeks(c);
|
let delta = TimeDelta::weeks(c);
|
||||||
today.checked_sub_signed(delta).unwrap()
|
today.checked_sub_signed(delta)?
|
||||||
}
|
}
|
||||||
MessageAge::Months(c) => {
|
MessageAge::Months(c) => {
|
||||||
let day = today.day();
|
let day = today.day();
|
||||||
@@ -398,7 +398,7 @@ impl EolRule {
|
|||||||
|
|
||||||
Local
|
Local
|
||||||
.with_ymd_and_hms(new_year, new_month, day, 0, 0, 0)
|
.with_ymd_and_hms(new_year, new_month, day, 0, 0, 0)
|
||||||
.unwrap()
|
.single()?
|
||||||
}
|
}
|
||||||
MessageAge::Years(c) => {
|
MessageAge::Years(c) => {
|
||||||
let day = today.day();
|
let day = today.day();
|
||||||
@@ -408,7 +408,7 @@ impl EolRule {
|
|||||||
|
|
||||||
Local
|
Local
|
||||||
.with_ymd_and_hms(new_year, month, day, 0, 0, 0)
|
.with_ymd_and_hms(new_year, month, day, 0, 0, 0)
|
||||||
.unwrap()
|
.single()?
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
@@ -477,8 +477,8 @@ mod test {
|
|||||||
fn test_eol_query_for_eol_rule_5_years() {
|
fn test_eol_query_for_eol_rule_5_years() {
|
||||||
let rule = build_test_rule(crate::MessageAge::Years(5));
|
let rule = build_test_rule(crate::MessageAge::Years(5));
|
||||||
|
|
||||||
let test_today = Local.with_ymd_and_hms(2025, 9, 15, 0, 0, 0).unwrap();
|
let test_today = Local.with_ymd_and_hms(2025, 9, 15, 0, 0, 0).single().unwrap();
|
||||||
let query = rule.calculate_for_date(test_today).unwrap();
|
let query = rule.calculate_for_date(test_today).expect("Failed to calculate query");
|
||||||
|
|
||||||
assert_eq!("before: 2020-09-15", query);
|
assert_eq!("before: 2020-09-15", query);
|
||||||
}
|
}
|
||||||
@@ -487,8 +487,8 @@ mod test {
|
|||||||
fn test_eol_query_for_eol_rule_1_month() {
|
fn test_eol_query_for_eol_rule_1_month() {
|
||||||
let rule = build_test_rule(crate::MessageAge::Months(1));
|
let rule = build_test_rule(crate::MessageAge::Months(1));
|
||||||
|
|
||||||
let test_today = Local.with_ymd_and_hms(2025, 9, 15, 0, 0, 0).unwrap();
|
let test_today = Local.with_ymd_and_hms(2025, 9, 15, 0, 0, 0).single().unwrap();
|
||||||
let query = rule.calculate_for_date(test_today).unwrap();
|
let query = rule.calculate_for_date(test_today).expect("Failed to calculate query");
|
||||||
|
|
||||||
assert_eq!("before: 2025-08-15", query);
|
assert_eq!("before: 2025-08-15", query);
|
||||||
}
|
}
|
||||||
@@ -497,8 +497,8 @@ mod test {
|
|||||||
fn test_eol_query_for_eol_rule_13_weeks() {
|
fn test_eol_query_for_eol_rule_13_weeks() {
|
||||||
let rule = build_test_rule(crate::MessageAge::Weeks(13));
|
let rule = build_test_rule(crate::MessageAge::Weeks(13));
|
||||||
|
|
||||||
let test_today = Local.with_ymd_and_hms(2025, 9, 15, 0, 0, 0).unwrap();
|
let test_today = Local.with_ymd_and_hms(2025, 9, 15, 0, 0, 0).single().unwrap();
|
||||||
let query = rule.calculate_for_date(test_today).unwrap();
|
let query = rule.calculate_for_date(test_today).expect("Failed to calculate query");
|
||||||
|
|
||||||
assert_eq!("before: 2025-06-16", query);
|
assert_eq!("before: 2025-06-16", query);
|
||||||
}
|
}
|
||||||
@@ -507,8 +507,8 @@ mod test {
|
|||||||
fn test_eol_query_for_eol_rule_365_days() {
|
fn test_eol_query_for_eol_rule_365_days() {
|
||||||
let rule = build_test_rule(crate::MessageAge::Days(365));
|
let rule = build_test_rule(crate::MessageAge::Days(365));
|
||||||
|
|
||||||
let test_today = Local.with_ymd_and_hms(2025, 9, 15, 0, 0, 0).unwrap();
|
let test_today = Local.with_ymd_and_hms(2025, 9, 15, 0, 0, 0).single().unwrap();
|
||||||
let query = rule.calculate_for_date(test_today).unwrap();
|
let query = rule.calculate_for_date(test_today).expect("Failed to calculate query");
|
||||||
|
|
||||||
assert_eq!("before: 2024-09-15", query);
|
assert_eq!("before: 2024-09-15", query);
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user