From efa3bc48c3aba5213f53dd57e9e12106d5180552 Mon Sep 17 00:00:00 2001 From: yggverse Date: Fri, 22 Nov 2024 15:51:03 +0200 Subject: [PATCH] update errors handle --- src/profile.rs | 32 +++++++----- src/profile/bookmark/memory.rs | 2 +- src/profile/bookmark/memory/error.rs | 2 +- src/profile/database.rs | 54 ++++++--------------- src/profile/identity.rs | 15 +++--- src/profile/identity/database.rs | 23 ++++----- src/profile/identity/error.rs | 3 +- src/profile/identity/gemini/memory.rs | 2 +- src/profile/identity/gemini/memory/error.rs | 2 +- 9 files changed, 61 insertions(+), 74 deletions(-) diff --git a/src/profile.rs b/src/profile.rs index f99b1e25..3cd6993f 100644 --- a/src/profile.rs +++ b/src/profile.rs @@ -59,25 +59,24 @@ impl Profile { // Init writable connection let mut connection = match connection.write() { Ok(connection) => connection, - Err(e) => todo!("{e}"), + Err(reason) => todo!("{reason}"), }; // Init new transaction let transaction = match connection.transaction() { Ok(transaction) => transaction, - Err(e) => todo!("{e}"), + Err(reason) => todo!("{reason}"), }; // Begin migration match migrate(&transaction) { Ok(_) => { // Confirm changes - match transaction.commit() { - Ok(_) => {} // @TODO - Err(e) => todo!("{e}"), + if let Err(reason) = transaction.commit() { + todo!("{reason}") } } - Err(e) => todo!("{e}"), + Err(reason) => todo!("{reason}"), } } // unlock database @@ -85,18 +84,27 @@ impl Profile { let database = Rc::new(Database::new(connection.clone())); // Get active profile or create new one - let profile_id = Rc::new(match database.active() { + let profile_id = Rc::new(match database.active().unwrap() { Some(profile) => profile.id, None => match database.add(true, DateTime::now_local().unwrap(), None) { Ok(id) => id, - Err(_) => todo!(), + Err(reason) => todo!("{:?}", reason), }, }); + // Init bookmark component @TODO handle errors + let bookmark = Rc::new(Bookmark::new(connection.clone(), profile_id.clone())); + + // Init identity component + let identity = Rc::new(match Identity::new(connection, profile_id) { + Ok(result) => result, + Err(reason) => todo!("{:?}", reason), + }); + // Result Self { - bookmark: Rc::new(Bookmark::new(connection.clone(), profile_id.clone())), - identity: Rc::new(Identity::new(connection, profile_id).unwrap()), // @TODO handle + bookmark, + identity, database, config_path, } @@ -105,8 +113,8 @@ impl Profile { pub fn migrate(tx: &Transaction) -> Result<(), String> { // Migrate self components - if let Err(e) = database::init(tx) { - return Err(e.to_string()); + if let Err(reason) = database::init(tx) { + return Err(reason.to_string()); } // Delegate migration to children components diff --git a/src/profile/bookmark/memory.rs b/src/profile/bookmark/memory.rs index efcb51a3..51878a5a 100644 --- a/src/profile/bookmark/memory.rs +++ b/src/profile/bookmark/memory.rs @@ -24,7 +24,7 @@ impl Memory { /// * validate record with same key does not exist yet pub fn add(&self, request: String, id: i64) -> Result<(), Error> { match self.index.borrow_mut().insert(request, id) { - Some(_) => Err(Error::Overwrite), // @TODO prevent? + Some(key) => Err(Error::Overwrite(key)), // @TODO prevent? None => Ok(()), } } diff --git a/src/profile/bookmark/memory/error.rs b/src/profile/bookmark/memory/error.rs index 09edab81..ed0b40ff 100644 --- a/src/profile/bookmark/memory/error.rs +++ b/src/profile/bookmark/memory/error.rs @@ -1,5 +1,5 @@ #[derive(Debug)] pub enum Error { NotFound, - Overwrite, + Overwrite(i64), } diff --git a/src/profile/database.rs b/src/profile/database.rs index d5df732c..b86dc078 100644 --- a/src/profile/database.rs +++ b/src/profile/database.rs @@ -24,71 +24,45 @@ impl Database { // Getters /// Get all records - pub fn records(&self) -> Vec { + pub fn records(&self) -> Result, Error> { let readable = self.connection.read().unwrap(); - let tx = readable.unchecked_transaction().unwrap(); - select(&tx).unwrap() + let tx = readable.unchecked_transaction()?; + select(&tx) } /// Get active profile record if exist - pub fn active(&self) -> Option
{ - self.records().into_iter().find(|record| record.is_active) + pub fn active(&self) -> Result, Error> { + let records = self.records()?; + Ok(records.into_iter().find(|record| record.is_active)) } // Setters /// Create new record in `Self` database connected - pub fn add(&self, is_active: bool, time: DateTime, name: Option) -> Result { + pub fn add(&self, is_active: bool, time: DateTime, name: Option) -> Result { // Begin new transaction let mut writable = self.connection.write().unwrap(); - let tx = writable.transaction().unwrap(); + let tx = writable.transaction()?; // New record has active status if is_active { // Deactivate other records as only one profile should be active - for record in select(&tx).unwrap() { - let _ = update(&tx, record.id, false, record.time, record.name); + for record in select(&tx)? { + update(&tx, record.id, false, record.time, record.name)?; } } // Create new record - insert(&tx, is_active, time, name).unwrap(); + insert(&tx, is_active, time, name)?; // Hold insert ID for result let id = last_insert_id(&tx); // Done - match tx.commit() { - Ok(_) => Ok(id), - Err(_) => Err(()), // @TODO - } + tx.commit()?; + + Ok(id) } - - /* @TODO not in use - /// Set `is_active` status `true` for the record with given profile ID - /// * reset other records to `false` - pub fn activate(&self, id: i64) -> Result<(), ()> { - // Begin new transaction - let mut writable = self.connection.write().unwrap(); - let tx = writable.transaction().unwrap(); - - // Deactivate other records as only one profile should be active - for record in select(&tx).unwrap() { - let _ = update( - &tx, - record.id, - if record.id == id { true } else { false }, - record.time, - record.name, - ); - } - - // Done - match tx.commit() { - Ok(_) => Ok(()), - Err(_) => Err(()), - } // @TODO make sure ID exist and was changed - } */ } // Low-level DB API diff --git a/src/profile/identity.rs b/src/profile/identity.rs index 57931d56..959a5d64 100644 --- a/src/profile/identity.rs +++ b/src/profile/identity.rs @@ -25,11 +25,14 @@ impl Identity { // Get active identity set for profile or create new one let profile_identity_id = Rc::new(match database.active() { - Some(identity) => identity.id, - None => match database.add(profile_id, true) { - Ok(id) => id, - Err(_) => return Err(Error::Database), + Ok(result) => match result { + Some(identity) => identity.id, + None => match database.add(profile_id, true) { + Ok(id) => id, + Err(reason) => return Err(Error::DatabaseAdd(reason)), + }, }, + Err(reason) => return Err(Error::DatabaseActive(reason)), }); // Init gemini component @@ -63,8 +66,8 @@ impl Identity { pub fn migrate(tx: &Transaction) -> Result<(), String> { // Migrate self components - if let Err(e) = database::init(tx) { - return Err(e.to_string()); + if let Err(reason) = database::init(tx) { + return Err(reason.to_string()); } // Delegate migration to childs diff --git a/src/profile/identity/database.rs b/src/profile/identity/database.rs index 11a4f7d4..a9b95220 100644 --- a/src/profile/identity/database.rs +++ b/src/profile/identity/database.rs @@ -22,35 +22,36 @@ impl Database { // Getters /// Get all records - pub fn records(&self) -> Vec
{ + pub fn records(&self) -> Result, Error> { let readable = self.connection.read().unwrap(); - let tx = readable.unchecked_transaction().unwrap(); - select(&tx).unwrap() + let tx = readable.unchecked_transaction()?; + select(&tx) } /// Get active identity record if exist - pub fn active(&self) -> Option
{ - self.records().into_iter().find(|record| record.is_active) + pub fn active(&self) -> Result, Error> { + let records = self.records()?; + Ok(records.into_iter().find(|record| record.is_active)) } // Setters /// Create new record in `Self` database connected - pub fn add(&self, profile_id: Rc, is_active: bool) -> Result { + pub fn add(&self, profile_id: Rc, is_active: bool) -> Result { // Begin new transaction let mut writable = self.connection.write().unwrap(); - let tx = writable.transaction().unwrap(); + let tx = writable.transaction()?; // New record has active status if is_active { // Deactivate other records as only one profile should be active - for record in select(&tx).unwrap() { - let _ = update(&tx, record.profile_id, record.id, false); + for record in select(&tx)? { + update(&tx, record.profile_id, record.id, false)?; } } // Create new record - insert(&tx, profile_id, is_active).unwrap(); + insert(&tx, profile_id, is_active)?; // Hold insert ID for result let id = last_insert_id(&tx); @@ -58,7 +59,7 @@ impl Database { // Done match tx.commit() { Ok(_) => Ok(id), - Err(_) => Err(()), // @TODO + Err(reason) => Err(reason), } } } diff --git a/src/profile/identity/error.rs b/src/profile/identity/error.rs index 4031e93f..ee50c871 100644 --- a/src/profile/identity/error.rs +++ b/src/profile/identity/error.rs @@ -1,5 +1,6 @@ #[derive(Debug)] pub enum Error { - Database, + DatabaseActive(sqlite::Error), + DatabaseAdd(sqlite::Error), GeminiInit(super::gemini::Error), } diff --git a/src/profile/identity/gemini/memory.rs b/src/profile/identity/gemini/memory.rs index 32ddb2f3..8ef10dc8 100644 --- a/src/profile/identity/gemini/memory.rs +++ b/src/profile/identity/gemini/memory.rs @@ -24,7 +24,7 @@ impl Memory { /// * validate record with same key does not exist yet pub fn add(&self, id: i64, pem: String) -> Result<(), Error> { match self.index.borrow_mut().insert(id, pem) { - Some(_) => Err(Error::Overwrite(id)), // @TODO prevent? + Some(key) => Err(Error::Overwrite(key)), // @TODO prevent? None => Ok(()), } } diff --git a/src/profile/identity/gemini/memory/error.rs b/src/profile/identity/gemini/memory/error.rs index 83f64a5d..5efe607f 100644 --- a/src/profile/identity/gemini/memory/error.rs +++ b/src/profile/identity/gemini/memory/error.rs @@ -1,5 +1,5 @@ #[derive(Debug)] pub enum Error { NotFound(i64), - Overwrite(i64), + Overwrite(String), }