From 0bd317145a933d6ced7c6eb7788c578c0cc69c11 Mon Sep 17 00:00:00 2001 From: giuseppe-g-gelardi Date: Wed, 27 Nov 2024 15:28:09 -0500 Subject: [PATCH 1/6] update database errors, start impl tracing info --- Cargo.toml | 1 + src/cargobase/database.rs | 91 ++++++++++++++++------------------ src/cargobase/errors/errors.rs | 12 ++--- src/cargobase/table.rs | 5 -- src/cargobase/util.rs | 2 +- src/lib.rs | 2 +- 6 files changed, 52 insertions(+), 61 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 1077d4a..c2e1dae 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -15,4 +15,5 @@ tempfile = "3.14.0" serde-reflection = "0.4.0" tracing = "0.1" tracing-subscriber = "0.3" +tracing-test = "0.2.5" diff --git a/src/cargobase/database.rs b/src/cargobase/database.rs index a460a9c..61112bb 100644 --- a/src/cargobase/database.rs +++ b/src/cargobase/database.rs @@ -1,4 +1,5 @@ use serde::{Deserialize, Serialize}; +use tracing::{error, info, warn}; use super::view::View; use super::DatabaseError; @@ -20,18 +21,19 @@ impl Database { // -- they might not have tracing enabled if std::path::Path::new(&file_name).exists() { - println!("Database already exists: {name}, loading database"); + info!("Database already exists: {name}, loading database"); + // Load the database from the file if let Ok(db) = Database::load_from_file(&file_name) { return db; } else { - eprintln!("Failed to load database from file: {file_name}"); + error!("Failed to load database from file: {file_name}"); } } else { - println!("Creating new database: {file_name}"); + info!("Creating new database: {file_name}"); // Create an empty JSON file for the new database if let Err(e) = std::fs::write(&file_name, "{}") { - eprintln!("Failed to create database file: {e}"); + error!("Failed to create database file: {e}"); } } @@ -44,28 +46,28 @@ impl Database { pub fn drop_database(&self) -> Result<(), DatabaseError> { if std::fs::remove_file(&self.file_name).is_err() { - return Err(DatabaseError::DeleteError( - "Failed to delete database file".to_string(), - )); + error!( + "{}", + DatabaseError::DeleteError("Failed to delete database file".to_string()) + ); + + // should we crash the program? + // return Err(DatabaseError::DeleteError( + // "Failed to delete database file".to_string(), + // )); } - println!("Database `{}` dropped successfully", self.name); + info!("Database `{}` dropped successfully", self.name); Ok(()) } - // TODO: update this: - /* - * if the table does not exist, add it to the Database - * - * if the table exists: - * -- do NOT add a duplicate to the db - * -- let the user know that the table already exists - * -- do NOT crash the program, just return and move on - */ pub fn add_table(&mut self, table: &mut Table) -> Result<(), DatabaseError> { - // table.set_file_name(self.file_name.clone()); if self.tables.iter().any(|t| t.name == table.name) { - return Err(DatabaseError::TableAlreadyExists(table.name.clone())); + warn!( + "{}", + DatabaseError::TableAlreadyExists(table.name.to_string()) + ); + return Ok(()); } self.tables.push(table.clone()); @@ -74,45 +76,34 @@ impl Database { Ok(()) } - // TODO: update this: - /* - * IF the table does not exist: - * -- let the user know that the table does not exist - * -- do NOT crash the program, just return and move on - * - * IF the table exists: - * -- remove the table from the db - * -- save the db to file - * -- let the user know that the table was removed successfully - */ pub fn drop_table(&mut self, table_name: &str) -> Result<(), DatabaseError> { let mut db = Database::load_from_file(&self.file_name).map_err(|e| DatabaseError::LoadError(e))?; if let Some(index) = db.tables.iter().position(|t| t.name == table_name) { let removed_table = db.tables.remove(index); - println!("Table `{}` dropped successfully", removed_table.name); + info!("Table `{}` dropped successfully", removed_table.name); db.save_to_file().map_err(|e| DatabaseError::SaveError(e))?; self.tables = db.tables; Ok(()) } else { - // eprintln!("{}", DatabaseError::TableNotFound(table_name.to_string())); - Err(DatabaseError::TableNotFound(table_name.to_string())) - // Ok(()) + error!("{}", DatabaseError::TableNotFound(table_name.to_string())); + Ok(()) } } pub(crate) fn save_to_file(&self) -> Result<(), std::io::Error> { let json_data = serde_json::to_string_pretty(&self)?; std::fs::write(&self.file_name, json_data)?; - println!("Database saved to file: {}", self.file_name); + info!("Database saved to file: {}", self.file_name); Ok(()) } pub(crate) fn load_from_file(file_name: &str) -> Result { let json_data = std::fs::read_to_string(file_name)?; let db: Database = serde_json::from_str(&json_data)?; + info!("Database loaded from file: {}", file_name); // needed? Ok(db) } @@ -183,10 +174,11 @@ impl Database { #[cfg(test)] mod tests { - use crate::cargobase::setup_temp_db; - use crate::{Columns, Table}; + use tracing_test::traced_test; use super::*; + use crate::cargobase::setup_temp_db; + use crate::{Columns, Table}; #[derive(Serialize, Deserialize, Debug, PartialEq, Clone, Default)] struct TestData { @@ -235,23 +227,25 @@ mod tests { std::fs::remove_file("test_db.json").ok(); } + #[traced_test] #[test] fn test_add_table_already_exists() { let mut db = setup_temp_db(); + // Create a duplicate table let columns = Columns::from_struct::(true); let mut duplicate_table = Table::new("TestTable".to_string(), columns); let result = db.add_table(&mut duplicate_table); - // Assert that an error is returned - assert!(matches!(result, Err(DatabaseError::TableAlreadyExists(_)))); - - if let Err(DatabaseError::TableAlreadyExists(name)) = result { - assert_eq!(name, "TestTable"); - } + // Assert that the result is Ok(()) even when the table already exists + assert!(result.is_ok()); // Ensure no duplicate tables exist assert_eq!(db.tables.len(), 1); + + let db_error = DatabaseError::TableAlreadyExists("TestTable".to_string()); + let logs = logs_contain(&format!("{}", db_error)); + assert!(logs, "Expected warning log for existing table not found."); } #[test] @@ -263,17 +257,18 @@ mod tests { assert_eq!(db.tables.len(), 0); } + #[traced_test] #[test] fn test_drop_table_not_found() { let mut db = setup_temp_db(); let result = db.drop_table("NonExistentTable"); - // Assert that an error is returned - assert!(matches!(result, Err(DatabaseError::TableNotFound(_)))); + assert!(result.is_ok()); - if let Err(DatabaseError::TableNotFound(name)) = result { - assert_eq!(name, "NonExistentTable"); - } + // Assert that an error is returned + let db_error = DatabaseError::TableNotFound("NonExistentTable".to_string()); + let logs = logs_contain(&format!("{}", db_error)); + assert!(logs, "Expected error log for non-existent table not found."); // Ensure no tables were removed assert_eq!(db.tables.len(), 1); diff --git a/src/cargobase/errors/errors.rs b/src/cargobase/errors/errors.rs index 0ff832e..fca0eb8 100644 --- a/src/cargobase/errors/errors.rs +++ b/src/cargobase/errors/errors.rs @@ -2,23 +2,23 @@ use thiserror::Error; #[derive(Error, Debug)] pub enum DatabaseError { - #[error("Failed to load the batabase: {0}")] + #[error("Failed to load the batabase: `{0}`")] LoadError(std::io::Error), - #[error("Failed to save the database: {0}")] + #[error("Failed to save the database: `{0}`")] SaveError(std::io::Error), - #[error("Failed to drop database: {0}")] + #[error("Failed to drop database: `{0}`")] // return Err(DatabaseError::DeleteError); DeleteError(String), - #[error("Table `{0}` already exists")] // skipping creation + #[error("Table `{0}` already exists. skipping creation.")] // skipping creation TableAlreadyExists(String), - #[error("Table {0} not found")] + #[error("Table `{0}` not found")] TableNotFound(String), - #[error("Invalid data: {0}")] + #[error("Invalid data: `{0}`")] InvalidData(String), #[error("Row not found with {0} = {1}")] diff --git a/src/cargobase/table.rs b/src/cargobase/table.rs index b309b87..a7ce1e0 100644 --- a/src/cargobase/table.rs +++ b/src/cargobase/table.rs @@ -19,11 +19,6 @@ impl Table { } } - // consider removing this. need to check what it is doing after removing the file name field - // pub(crate) fn set_file_name(&mut self, file_name: String) { - // println!("File name set to: {}", file_name); - // } - pub fn add_row(&mut self, db: &mut Database, data: Value) { if let Some(table) = db.get_table_mut(&self.name) { if data.is_array() { diff --git a/src/cargobase/util.rs b/src/cargobase/util.rs index 79d95e7..b4435ca 100644 --- a/src/cargobase/util.rs +++ b/src/cargobase/util.rs @@ -27,7 +27,7 @@ pub fn setup_temp_db() -> Database { pub fn init_tracing() { let subscriber = fmt::Subscriber::builder() - .with_max_level(tracing::Level::DEBUG) + .with_max_level(tracing::Level::WARN) .finish(); /* example implementation: diff --git a/src/lib.rs b/src/lib.rs index 4b89cd3..f42444a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,3 +1,3 @@ pub mod cargobase; -pub use cargobase::{Columns, Column, Row, Database, Table}; +pub use cargobase::{Columns, Column, Row, Database, Table, util::init_tracing}; From 8be23b637fba6aebb44cfb2bf43796cdbceca23e Mon Sep 17 00:00:00 2001 From: giuseppe-g-gelardi Date: Wed, 27 Nov 2024 15:55:58 -0500 Subject: [PATCH 2/6] add column error, update columns.validate to return new databaseerror, impl tracing, update relavant unit test --- src/cargobase/columns.rs | 54 +++++++++++++++++++++++++++++----- src/cargobase/errors/errors.rs | 3 ++ src/cargobase/query.rs | 21 ++++++++----- 3 files changed, 62 insertions(+), 16 deletions(-) diff --git a/src/cargobase/columns.rs b/src/cargobase/columns.rs index 819a477..1b40f24 100644 --- a/src/cargobase/columns.rs +++ b/src/cargobase/columns.rs @@ -2,6 +2,9 @@ use serde::de::DeserializeOwned; use serde::{Deserialize, Serialize}; use serde_json::Value; use serde_reflection::{ContainerFormat, Named, Tracer, TracerConfig}; +use tracing::{error, info, warn}; + +use super::DatabaseError; #[derive(Debug, PartialEq, Serialize, Deserialize, Clone)] pub struct Column { @@ -56,30 +59,41 @@ impl Columns { } // validate the columns - pub fn validate(&self, row_data: Value) -> Result<(), String> { + pub fn validate(&self, row_data: Value) -> Result<(), DatabaseError> { if let Value::Object(data) = row_data { for column in &self.0 { if column.required && !data.contains_key(&column.name) { - return Err(format!("Column '{}' is required.", column.name)); + error!("Column '{}' is required.", column.name); + return Err(DatabaseError::ColumnRequiredError(format!( + "Column '{}' is required.", + column.name + ))); } } for key in data.keys() { if !self.0.iter().any(|col| col.name == *key) { - return Err(format!("Column '{}' is not valid.", key)); + error!("Column '{}' is not valid.", key); + return Err(DatabaseError::InvalidData(format!( + "Column '{}' is not valid.", + key + ))); } } Ok(()) } else { - Err("Invalid row data.".to_string()) + error!("Invalid row data."); + Err(DatabaseError::InvalidData("Invalid row data.".to_string())) } } } #[cfg(test)] mod tests { - use super::*; use serde_json::json; + use tracing_test::traced_test; + + use super::*; #[test] fn test_column_new() { @@ -116,6 +130,7 @@ mod tests { assert!(result.is_ok()); } + #[traced_test] #[test] fn test_validate_missing_required_column() { let columns = Columns(vec![ @@ -130,10 +145,18 @@ mod tests { }); let result = columns.validate(row_data); + + // Assert that an error is returned assert!(result.is_err()); - assert_eq!(result.unwrap_err(), "Column 'name' is required."); + + // Verify the specific log message + assert!( + logs_contain("Column 'name' is required."), + "Expected log message for missing required column not found." + ); } + #[traced_test] #[test] fn test_validate_invalid_column() { let columns = Columns(vec![ @@ -149,10 +172,18 @@ mod tests { }); let result = columns.validate(row_data); + + // Assert that an error is returned assert!(result.is_err()); - assert_eq!(result.unwrap_err(), "Column 'phone' is not valid."); + + // Verify the specific log message + assert!( + logs_contain("Column 'phone' is not valid."), + "Expected log message for invalid column not found." + ); } + #[traced_test] #[test] fn test_validate_invalid_row_type() { let columns = Columns(vec![ @@ -170,8 +201,15 @@ mod tests { ]); // Invalid type (array instead of object) let result = columns.validate(row_data); + + // Assert that an error is returned assert!(result.is_err()); - assert_eq!(result.unwrap_err(), "Invalid row data."); + + // Verify the specific log message + assert!( + logs_contain("Invalid row data."), + "Expected log message for invalid row data not found." + ); } #[test] diff --git a/src/cargobase/errors/errors.rs b/src/cargobase/errors/errors.rs index fca0eb8..37e2396 100644 --- a/src/cargobase/errors/errors.rs +++ b/src/cargobase/errors/errors.rs @@ -26,4 +26,7 @@ pub enum DatabaseError { #[error("Column `{0}` is missing from the row data")] MissingColumn(String), + + #[error("Column `{0}` is required")] + ColumnRequiredError(String), } diff --git a/src/cargobase/query.rs b/src/cargobase/query.rs index 78d07d5..765ead8 100644 --- a/src/cargobase/query.rs +++ b/src/cargobase/query.rs @@ -2,7 +2,7 @@ use serde::de::DeserializeOwned; use serde::{Deserialize, Serialize}; use serde_json::Value; -use super::{Database, Row, Table}; +use super::{Database, DatabaseError, Row, Table}; #[derive(Debug, PartialEq, Serialize, Deserialize, Clone, Copy)] pub enum Operation { @@ -81,33 +81,38 @@ impl Query { } } - pub fn execute_add(self) -> Result<(), String> { + // pub fn execute_add(self) -> Result<(), String> { + pub fn execute_add(self) -> Result<(), DatabaseError> { let mut db = Database::load_from_file(&self.db_file_name) - .map_err(|e| format!("Failed to load database: {}", e))?; + .map_err(DatabaseError::LoadError)?; + // .map_err(|e| format!("Failed to load database: {}", e))?; let table_name = self .table_name .clone() - .ok_or_else(|| "Table name not specified.".to_string())?; + .ok_or_else(|| DatabaseError::InvalidData("Table name not specified.".to_string()))?; + // .ok_or_else(|| "Table name not specified.".to_string())?; // Find the table let table = db .tables .iter_mut() .find(|t| t.name == table_name) - .ok_or_else(|| format!("Table '{}' not found.", table_name))?; + .ok_or_else(|| DatabaseError::TableNotFound(table_name.clone()))?; + // .ok_or_else(|| format!("Table '{}' not found.", table_name))?; // Validate and add the row if let Some(row_data) = self.row_data { table.columns.validate(row_data.clone())?; // optional schema validation table.rows.push(Row::new(row_data)); - db.save_to_file() - .map_err(|e| format!("Failed to save database: {}", e))?; + db.save_to_file().map_err(DatabaseError::SaveError)?; + // .map_err(|e| format!("Failed to save database: {}", e))?; println!("Row added successfully to '{}'.", table_name); Ok(()) } else { - Err("No data provided for the new row.".to_string()) + // Err("No data provided for the new row.".to_string()) + Err(DatabaseError::InvalidData("No data provided for the new row.".to_string())) } } From a4b01245d55e20d1fec119140ac2d468e92a135c Mon Sep 17 00:00:00 2001 From: giuseppe-g-gelardi Date: Wed, 27 Nov 2024 16:10:43 -0500 Subject: [PATCH 3/6] clean up validate fn --- src/cargobase/columns.rs | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/src/cargobase/columns.rs b/src/cargobase/columns.rs index 1b40f24..6692299 100644 --- a/src/cargobase/columns.rs +++ b/src/cargobase/columns.rs @@ -2,7 +2,7 @@ use serde::de::DeserializeOwned; use serde::{Deserialize, Serialize}; use serde_json::Value; use serde_reflection::{ContainerFormat, Named, Tracer, TracerConfig}; -use tracing::{error, info, warn}; +use tracing::error; use super::DatabaseError; @@ -63,21 +63,17 @@ impl Columns { if let Value::Object(data) = row_data { for column in &self.0 { if column.required && !data.contains_key(&column.name) { - error!("Column '{}' is required.", column.name); - return Err(DatabaseError::ColumnRequiredError(format!( - "Column '{}' is required.", - column.name - ))); + let error_message = format!("Column '{}' is required.", column.name); + error!("{}", error_message); + return Err(DatabaseError::ColumnRequiredError(error_message)); } } for key in data.keys() { if !self.0.iter().any(|col| col.name == *key) { - error!("Column '{}' is not valid.", key); - return Err(DatabaseError::InvalidData(format!( - "Column '{}' is not valid.", - key - ))); + let error_message = format!("Column '{}' is not valid.", key); + error!("{}", error_message); + return Err(DatabaseError::InvalidData(error_message)); } } Ok(()) From 829a8e29a9d33d28e28759ef081201ea6f52a156 Mon Sep 17 00:00:00 2001 From: giuseppe-g-gelardi Date: Wed, 27 Nov 2024 16:14:55 -0500 Subject: [PATCH 4/6] update tracing calls for readability --- src/cargobase/columns.rs | 8 ++++---- src/cargobase/database.rs | 28 +++++++++++++--------------- 2 files changed, 17 insertions(+), 19 deletions(-) diff --git a/src/cargobase/columns.rs b/src/cargobase/columns.rs index 6692299..38a1e9f 100644 --- a/src/cargobase/columns.rs +++ b/src/cargobase/columns.rs @@ -2,7 +2,7 @@ use serde::de::DeserializeOwned; use serde::{Deserialize, Serialize}; use serde_json::Value; use serde_reflection::{ContainerFormat, Named, Tracer, TracerConfig}; -use tracing::error; +use tracing; use super::DatabaseError; @@ -64,7 +64,7 @@ impl Columns { for column in &self.0 { if column.required && !data.contains_key(&column.name) { let error_message = format!("Column '{}' is required.", column.name); - error!("{}", error_message); + tracing::error!("{}", error_message); return Err(DatabaseError::ColumnRequiredError(error_message)); } } @@ -72,13 +72,13 @@ impl Columns { for key in data.keys() { if !self.0.iter().any(|col| col.name == *key) { let error_message = format!("Column '{}' is not valid.", key); - error!("{}", error_message); + tracing::error!("{}", error_message); return Err(DatabaseError::InvalidData(error_message)); } } Ok(()) } else { - error!("Invalid row data."); + tracing::error!("Invalid row data."); Err(DatabaseError::InvalidData("Invalid row data.".to_string())) } } diff --git a/src/cargobase/database.rs b/src/cargobase/database.rs index 61112bb..98c0c2d 100644 --- a/src/cargobase/database.rs +++ b/src/cargobase/database.rs @@ -1,5 +1,5 @@ use serde::{Deserialize, Serialize}; -use tracing::{error, info, warn}; +use tracing; use super::view::View; use super::DatabaseError; @@ -21,19 +21,19 @@ impl Database { // -- they might not have tracing enabled if std::path::Path::new(&file_name).exists() { - info!("Database already exists: {name}, loading database"); + tracing::info!("Database already exists: {name}, loading database"); // Load the database from the file if let Ok(db) = Database::load_from_file(&file_name) { return db; } else { - error!("Failed to load database from file: {file_name}"); + tracing::error!("Failed to load database from file: {file_name}"); } } else { - info!("Creating new database: {file_name}"); + tracing::info!("Creating new database: {file_name}"); // Create an empty JSON file for the new database if let Err(e) = std::fs::write(&file_name, "{}") { - error!("Failed to create database file: {e}"); + tracing::error!("Failed to create database file: {e}"); } } @@ -46,24 +46,22 @@ impl Database { pub fn drop_database(&self) -> Result<(), DatabaseError> { if std::fs::remove_file(&self.file_name).is_err() { - error!( + tracing::error!( "{}", DatabaseError::DeleteError("Failed to delete database file".to_string()) ); // should we crash the program? - // return Err(DatabaseError::DeleteError( - // "Failed to delete database file".to_string(), - // )); + // return Err(DatabaseError::DeleteError("Failed to delete database file".to_string(),)); } - info!("Database `{}` dropped successfully", self.name); + tracing::info!("Database `{}` dropped successfully", self.name); Ok(()) } pub fn add_table(&mut self, table: &mut Table) -> Result<(), DatabaseError> { if self.tables.iter().any(|t| t.name == table.name) { - warn!( + tracing::warn!( "{}", DatabaseError::TableAlreadyExists(table.name.to_string()) ); @@ -82,13 +80,13 @@ impl Database { if let Some(index) = db.tables.iter().position(|t| t.name == table_name) { let removed_table = db.tables.remove(index); - info!("Table `{}` dropped successfully", removed_table.name); + tracing::info!("Table `{}` dropped successfully", removed_table.name); db.save_to_file().map_err(|e| DatabaseError::SaveError(e))?; self.tables = db.tables; Ok(()) } else { - error!("{}", DatabaseError::TableNotFound(table_name.to_string())); + tracing::error!("{}", DatabaseError::TableNotFound(table_name.to_string())); Ok(()) } } @@ -96,14 +94,14 @@ impl Database { pub(crate) fn save_to_file(&self) -> Result<(), std::io::Error> { let json_data = serde_json::to_string_pretty(&self)?; std::fs::write(&self.file_name, json_data)?; - info!("Database saved to file: {}", self.file_name); + tracing::info!("Database saved to file: {}", self.file_name); Ok(()) } pub(crate) fn load_from_file(file_name: &str) -> Result { let json_data = std::fs::read_to_string(file_name)?; let db: Database = serde_json::from_str(&json_data)?; - info!("Database loaded from file: {}", file_name); // needed? + tracing::info!("Database loaded from file: {}", file_name); // needed? Ok(db) } From 372a879bc8ad6a93fa0f665b1c790355f6f9108f Mon Sep 17 00:00:00 2001 From: giuseppe-g-gelardi Date: Wed, 27 Nov 2024 16:17:01 -0500 Subject: [PATCH 5/6] add tracing logs to table.add_row --- src/cargobase/table.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/cargobase/table.rs b/src/cargobase/table.rs index a7ce1e0..d26bff9 100644 --- a/src/cargobase/table.rs +++ b/src/cargobase/table.rs @@ -1,5 +1,6 @@ use serde::{Deserialize, Serialize}; use serde_json::Value; +use tracing; use super::{Columns, Database, Row}; @@ -31,10 +32,10 @@ impl Table { table.rows.push(Row::new(data)) } let _ = db.save_to_file().map_err(|e| { - println!("Failed to save to file: {}", e); + tracing::error!("Failed to save to file: {}", e); }); } else { - println!("Table {} not found", self.name); + tracing::error!("Table {} not found", self.name); } } } @@ -59,4 +60,6 @@ mod tests { // table.set_file_name("db.json".to_string()); assert_eq!(table.name, "users"); } + + // test add row... } From 3956e1c3907386ffa3971dcde42db012be3b9860 Mon Sep 17 00:00:00 2001 From: giuseppe-g-gelardi Date: Wed, 27 Nov 2024 16:38:54 -0500 Subject: [PATCH 6/6] update where eq error handling when calling crud methods --- src/cargobase/errors/errors.rs | 3 ++ src/cargobase/query.rs | 83 ++++++++++++++++++++-------------- 2 files changed, 53 insertions(+), 33 deletions(-) diff --git a/src/cargobase/errors/errors.rs b/src/cargobase/errors/errors.rs index 37e2396..b42c0e9 100644 --- a/src/cargobase/errors/errors.rs +++ b/src/cargobase/errors/errors.rs @@ -29,4 +29,7 @@ pub enum DatabaseError { #[error("Column `{0}` is required")] ColumnRequiredError(String), + + #[error("")] // could expand to specify serialization/deserialization error + JSONError(#[from] serde_json::Error), } diff --git a/src/cargobase/query.rs b/src/cargobase/query.rs index 765ead8..6f2177f 100644 --- a/src/cargobase/query.rs +++ b/src/cargobase/query.rs @@ -41,23 +41,25 @@ impl Query { self, key: &str, value: &str, - ) -> Result, String> { + ) -> Result, DatabaseError> { // Load the database let mut db = Database::load_from_file(&self.db_file_name) - .map_err(|e| format!("Failed to load database: {}", e))?; + .map_err(|e| DatabaseError::LoadError(e))?; // Clone table_name to avoid moving self let table_name = self .table_name .clone() - .ok_or_else(|| "Table name not specified.".to_string())?; + .ok_or_else(|| DatabaseError::TableNotFound("Table name not specified.".to_string()))?; // Find the index of the table let table_index = db .tables .iter() .position(|t| t.name == table_name) - .ok_or_else(|| format!("Table '{}' not found.", table_name))?; + .ok_or_else(|| { + DatabaseError::TableNotFound(format!("Table '{}' not found.", table_name)) + })?; // Borrow the table by index let table = &mut db.tables[table_index]; @@ -67,14 +69,18 @@ impl Query { Operation::Read => self.execute_select(table, key, value), Operation::Update => { let result = self.execute_update(table, key, value); - db.save_to_file() - .map_err(|e| format!("Failed to save database: {}", e))?; + if let Err(e) = db.save_to_file().map_err(DatabaseError::SaveError) { + tracing::error!("Failed to save database: {}", e); + return Err(e); + } result } Operation::Delete => { let result = self.execute_delete(table, key, value); - db.save_to_file() - .map_err(|e| format!("Failed to save database: {}", e))?; + if let Err(e) = db.save_to_file().map_err(DatabaseError::SaveError) { + tracing::error!("Failed to save database: {}", e); + return Err(e); + } result } Operation::Create => unreachable!(), @@ -83,15 +89,13 @@ impl Query { // pub fn execute_add(self) -> Result<(), String> { pub fn execute_add(self) -> Result<(), DatabaseError> { - let mut db = Database::load_from_file(&self.db_file_name) - .map_err(DatabaseError::LoadError)?; - // .map_err(|e| format!("Failed to load database: {}", e))?; + let mut db = + Database::load_from_file(&self.db_file_name).map_err(DatabaseError::LoadError)?; let table_name = self .table_name .clone() .ok_or_else(|| DatabaseError::InvalidData("Table name not specified.".to_string()))?; - // .ok_or_else(|| "Table name not specified.".to_string())?; // Find the table let table = db @@ -99,7 +103,6 @@ impl Query { .iter_mut() .find(|t| t.name == table_name) .ok_or_else(|| DatabaseError::TableNotFound(table_name.clone()))?; - // .ok_or_else(|| format!("Table '{}' not found.", table_name))?; // Validate and add the row if let Some(row_data) = self.row_data { @@ -107,12 +110,13 @@ impl Query { table.rows.push(Row::new(row_data)); db.save_to_file().map_err(DatabaseError::SaveError)?; - // .map_err(|e| format!("Failed to save database: {}", e))?; - println!("Row added successfully to '{}'.", table_name); + tracing::info!("Row added successfully to '{}'.", table_name); Ok(()) } else { - // Err("No data provided for the new row.".to_string()) - Err(DatabaseError::InvalidData("No data provided for the new row.".to_string())) + tracing::error!("No data provided for the new row."); + Err(DatabaseError::InvalidData( + "No data provided for the new row.".to_string(), + )) } } @@ -121,13 +125,15 @@ impl Query { table: &Table, key: &str, value: &str, - ) -> Result, String> { + ) -> Result, DatabaseError> { for row in &table.rows { if let Some(field_value) = row.data.get(key) { if field_value.as_str() == Some(value) { return serde_json::from_value(row.data.clone()) .map(Some) - .map_err(|e| format!("Deserialization error: {}", e)); + .map_err(|e| { + DatabaseError::InvalidData(format!("Deserialization error: {}", e)) + }); } } } @@ -139,7 +145,7 @@ impl Query { table: &mut Table, key: &str, value: &str, - ) -> Result, String> { + ) -> Result, DatabaseError> { for row in &mut table.rows { if let Some(field_value) = row.data.get(key) { if field_value.as_str() == Some(value) { @@ -150,18 +156,29 @@ impl Query { row_map.insert(k.clone(), v.clone()); } } else { - return Err("Row data is not a JSON object.".to_string()); + return Err(DatabaseError::InvalidData( + "Row data is not a JSON object.".to_string(), + )); } - println!("Record updated successfully."); - return serde_json::from_value(row.data.clone()) - .map(Some) - .map_err(|e| format!("Deserialization error: {}", e)); + tracing::info!("Record updated successfully."); + return serde_json::from_value(row.data.clone()).map(Some).map_err( + |e| { + DatabaseError::InvalidData(format!( + "Deserialization error: {}", + e + )) + }, + ); } else { - return Err("Invalid update data format.".to_string()); + return Err(DatabaseError::InvalidData( + "Invalid update data format.".to_string(), + )); } } else { - return Err("No update data provided.".to_string()); + return Err(DatabaseError::InvalidData( + "No update data provided.".to_string(), + )); } } } @@ -174,15 +191,15 @@ impl Query { table: &mut Table, key: &str, value: &str, - ) -> Result, String> { + ) -> Result, DatabaseError> { for (i, row) in table.rows.iter().enumerate() { if let Some(field_value) = row.data.get(key) { if field_value.as_str() == Some(value) { let record = serde_json::from_value(row.data.clone()) - .map_err(|e| format!("Deserialization error: {}", e))?; + .map_err(|e| DatabaseError::JSONError(e))?; table.rows.remove(i); - println!("Record deleted successfully."); + tracing::info!("Record deleted successfully."); return Ok(Some(record)); } } @@ -192,7 +209,7 @@ impl Query { } pub fn all(&self) -> Vec { let db = Database::load_from_file(&self.db_file_name).unwrap_or_else(|e| { - eprintln!("Failed to load database from file: {}", e); + tracing::error!("Failed to load database from file: {}", e); Database { name: String::new(), file_name: self.db_file_name.clone(), @@ -208,11 +225,11 @@ impl Query { .filter_map(|row| serde_json::from_value(row.data.clone()).ok()) .collect() } else { - eprintln!("Table {} not found", table_name); + tracing::error!("Table {} not found", table_name); Vec::new() } } else { - eprintln!("Table name not provided"); + tracing::error!("Table name not provided"); Vec::new() } }