From cce3e260afe33bc02b4d8edef1e3e156f277b99c Mon Sep 17 00:00:00 2001 From: Lennart <18233294+lennart-k@users.noreply.github.com> Date: Mon, 28 Oct 2024 17:10:03 +0100 Subject: [PATCH] store, store_sqlite: Refactor error typing --- Cargo.lock | 1 - crates/store/src/addressbook_store.rs | 2 +- crates/store/src/error.rs | 4 +- crates/store/src/timestamp.rs | 8 +- crates/store_sqlite/Cargo.toml | 1 - crates/store_sqlite/src/addressbook_store.rs | 53 +++++++------ crates/store_sqlite/src/calendar_store.rs | 81 +++++++++----------- crates/store_sqlite/src/error.rs | 27 +------ crates/store_sqlite/src/lib.rs | 4 +- 9 files changed, 75 insertions(+), 106 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2a2e96d..04d44e9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2646,7 +2646,6 @@ dependencies = [ name = "rustical_store_sqlite" version = "0.1.0" dependencies = [ - "anyhow", "async-trait", "rustical_store", "serde", diff --git a/crates/store/src/addressbook_store.rs b/crates/store/src/addressbook_store.rs index 53a8eec..bb2184f 100644 --- a/crates/store/src/addressbook_store.rs +++ b/crates/store/src/addressbook_store.rs @@ -1,6 +1,6 @@ use crate::{ - error::Error, model::{AddressObject, Addressbook}, + Error, }; use async_trait::async_trait; diff --git a/crates/store/src/error.rs b/crates/store/src/error.rs index 7217717..640ccae 100644 --- a/crates/store/src/error.rs +++ b/crates/store/src/error.rs @@ -11,8 +11,8 @@ pub enum Error { InvalidData(String), #[error(transparent)] - Other(#[from] anyhow::Error), + ParserError(#[from] ical::parser::ParserError), #[error(transparent)] - ParserError(#[from] ical::parser::ParserError), + Other(#[from] anyhow::Error), } diff --git a/crates/store/src/timestamp.rs b/crates/store/src/timestamp.rs index 93e66c2..f568ffa 100644 --- a/crates/store/src/timestamp.rs +++ b/crates/store/src/timestamp.rs @@ -130,9 +130,9 @@ impl CalDateTime { return Ok(CalDateTime::OlsonTZ(datetime)); } else { // This time does not exist because there's a gap in local time - return Err(Error::Other(anyhow!( - "Timestamp doesn't exist because of gap in local time" - ))); + return Err(Error::InvalidData( + "Timestamp doesn't exist because of gap in local time".to_owned(), + )); } } return Ok(CalDateTime::Local(datetime)); @@ -145,7 +145,7 @@ impl CalDateTime { return Ok(CalDateTime::Date(date)); } - Err(Error::Other(anyhow!("Invalid datetime format"))) + Err(Error::InvalidData("Invalid datetime format".to_owned())) } pub fn utc(&self) -> DateTime { diff --git a/crates/store_sqlite/Cargo.toml b/crates/store_sqlite/Cargo.toml index 6ebb140..27c4513 100644 --- a/crates/store_sqlite/Cargo.toml +++ b/crates/store_sqlite/Cargo.toml @@ -8,7 +8,6 @@ publish = false [dependencies] rustical_store = { workspace = true } -anyhow = { workspace = true } async-trait = { workspace = true } serde = { workspace = true } sqlx = { workspace = true } diff --git a/crates/store_sqlite/src/addressbook_store.rs b/crates/store_sqlite/src/addressbook_store.rs index 80b2afb..0ca74df 100644 --- a/crates/store_sqlite/src/addressbook_store.rs +++ b/crates/store_sqlite/src/addressbook_store.rs @@ -1,5 +1,4 @@ use super::{ChangeOperation, SqliteStore}; -use crate::Error; use async_trait::async_trait; use rustical_store::{ model::{AddressObject, Addressbook}, @@ -15,9 +14,9 @@ struct AddressObjectRow { } impl TryFrom for AddressObject { - type Error = Error; + type Error = crate::Error; - fn try_from(value: AddressObjectRow) -> Result { + fn try_from(value: AddressObjectRow) -> Result { Ok(Self::from_vcf(value.id, value.vcf)?) } } @@ -29,7 +28,7 @@ async fn log_object_operation( addressbook_id: &str, object_id: &str, operation: ChangeOperation, -) -> Result<(), Error> { +) -> Result<(), sqlx::Error> { sqlx::query!( r#" UPDATE addressbooks @@ -75,7 +74,7 @@ impl AddressbookStore for SqliteStore { ) .fetch_one(&self.db) .await - .map_err(Error::SqlxError)?; + .map_err(crate::Error::from)?; Ok(addressbook) } @@ -93,7 +92,7 @@ impl AddressbookStore for SqliteStore { ) .fetch_all(&self.db) .await - .map_err(Error::SqlxError)?; + .map_err(crate::Error::from)?; Ok(addressbooks) } @@ -116,7 +115,7 @@ impl AddressbookStore for SqliteStore { ) .execute(&self.db) .await - .map_err(Error::SqlxError)?; + .map_err(crate::Error::from)?; if result.rows_affected() == 0 { return Err(rustical_store::Error::NotFound); } @@ -138,7 +137,7 @@ impl AddressbookStore for SqliteStore { ) .execute(&self.db) .await - .map_err(Error::SqlxError)?; + .map_err(crate::Error::from)?; Ok(()) } @@ -156,7 +155,7 @@ impl AddressbookStore for SqliteStore { principal, addressbook_id ) .execute(&self.db) - .await.map_err(Error::SqlxError)?; + .await.map_err(crate::Error::from)?; } false => { sqlx::query!( @@ -166,7 +165,7 @@ impl AddressbookStore for SqliteStore { ) .execute(&self.db) .await - .map_err(Error::SqlxError)?; + .map_err(crate::Error::from)?; } }; Ok(()) @@ -185,7 +184,7 @@ impl AddressbookStore for SqliteStore { ) .execute(&self.db) .await - .map_err(Error::SqlxError)?; + .map_err(crate::Error::from)?; Ok(()) } @@ -210,7 +209,7 @@ impl AddressbookStore for SqliteStore { synctoken ) .fetch_all(&self.db) - .await.map_err(Error::SqlxError)?; + .await.map_err(crate::Error::from)?; let mut objects = vec![]; let mut deleted_objects = vec![]; @@ -244,7 +243,7 @@ impl AddressbookStore for SqliteStore { addressbook_id ) .fetch_all(&self.db) - .await.map_err(Error::SqlxError)? + .await.map_err(crate::Error::from)? .into_iter() .map(|row| row.try_into().map_err(rustical_store::Error::from)) .collect() @@ -266,7 +265,7 @@ impl AddressbookStore for SqliteStore { ) .fetch_one(&self.db) .await - .map_err(Error::SqlxError)? + .map_err(crate::Error::from)? .try_into()?) } @@ -279,7 +278,7 @@ impl AddressbookStore for SqliteStore { // TODO: implement overwrite: bool, ) -> Result<(), rustical_store::Error> { - let mut tx = self.db.begin().await.map_err(Error::SqlxError)?; + let mut tx = self.db.begin().await.map_err(crate::Error::from)?; let (object_id, vcf) = (object.get_id(), object.get_vcf()); @@ -292,7 +291,7 @@ impl AddressbookStore for SqliteStore { ) .execute(&mut *tx) .await - .map_err(Error::SqlxError)?; + .map_err(crate::Error::from)?; log_object_operation( &mut tx, @@ -302,9 +301,9 @@ impl AddressbookStore for SqliteStore { ChangeOperation::Add, ) .await - .map_err(rustical_store::Error::from)?; + .map_err(crate::Error::from)?; - tx.commit().await.map_err(Error::SqlxError)?; + tx.commit().await.map_err(crate::Error::from)?; Ok(()) } @@ -316,7 +315,7 @@ impl AddressbookStore for SqliteStore { object_id: &str, use_trashbin: bool, ) -> Result<(), rustical_store::Error> { - let mut tx = self.db.begin().await.map_err(Error::SqlxError)?; + let mut tx = self.db.begin().await.map_err(crate::Error::from)?; match use_trashbin { true => { @@ -327,7 +326,7 @@ impl AddressbookStore for SqliteStore { object_id ) .execute(&mut *tx) - .await.map_err(Error::SqlxError)?; + .await.map_err(crate::Error::from)?; } false => { sqlx::query!( @@ -337,7 +336,7 @@ impl AddressbookStore for SqliteStore { ) .execute(&mut *tx) .await - .map_err(Error::SqlxError)?; + .map_err(crate::Error::from)?; } }; log_object_operation( @@ -348,8 +347,8 @@ impl AddressbookStore for SqliteStore { ChangeOperation::Delete, ) .await - .map_err(rustical_store::Error::from)?; - tx.commit().await.map_err(Error::SqlxError)?; + .map_err(crate::Error::from)?; + tx.commit().await.map_err(crate::Error::from)?; Ok(()) } @@ -360,7 +359,7 @@ impl AddressbookStore for SqliteStore { addressbook_id: &str, object_id: &str, ) -> Result<(), rustical_store::Error> { - let mut tx = self.db.begin().await.map_err(Error::SqlxError)?; + let mut tx = self.db.begin().await.map_err(crate::Error::from)?; sqlx::query!( r#"UPDATE addressobjects SET deleted_at = NULL, updated_at = datetime() WHERE (principal, addressbook_id, id) = (?, ?, ?)"#, @@ -369,7 +368,7 @@ impl AddressbookStore for SqliteStore { object_id ) .execute(&mut *tx) - .await.map_err(Error::SqlxError)?; + .await.map_err(crate::Error::from)?; log_object_operation( &mut tx, @@ -379,8 +378,8 @@ impl AddressbookStore for SqliteStore { ChangeOperation::Add, ) .await - .map_err(rustical_store::Error::from)?; - tx.commit().await.map_err(Error::SqlxError)?; + .map_err(crate::Error::from)?; + tx.commit().await.map_err(crate::Error::from)?; Ok(()) } } diff --git a/crates/store_sqlite/src/calendar_store.rs b/crates/store_sqlite/src/calendar_store.rs index 2935703..a2305cf 100644 --- a/crates/store_sqlite/src/calendar_store.rs +++ b/crates/store_sqlite/src/calendar_store.rs @@ -1,10 +1,9 @@ use super::{ChangeOperation, SqliteStore}; -use crate::Error; -use anyhow::Result; use async_trait::async_trait; use rustical_store::model::object::CalendarObject; use rustical_store::model::Calendar; use rustical_store::CalendarStore; +use rustical_store::Error; use sqlx::Sqlite; use sqlx::Transaction; use tracing::instrument; @@ -30,7 +29,7 @@ async fn log_object_operation( cal_id: &str, object_id: &str, operation: ChangeOperation, -) -> Result<(), rustical_store::Error> { +) -> Result<(), Error> { sqlx::query!( r#" UPDATE calendars @@ -41,7 +40,7 @@ async fn log_object_operation( ) .execute(&mut **tx) .await - .map_err(Error::SqlxError)?; + .map_err(crate::Error::from)?; sqlx::query!( r#" @@ -56,18 +55,14 @@ async fn log_object_operation( ) .execute(&mut **tx) .await - .map_err(Error::SqlxError)?; + .map_err(crate::Error::from)?; Ok(()) } #[async_trait] impl CalendarStore for SqliteStore { #[instrument] - async fn get_calendar( - &self, - principal: &str, - id: &str, - ) -> Result { + async fn get_calendar(&self, principal: &str, id: &str) -> Result { let cal = sqlx::query_as!( Calendar, r#"SELECT principal, id, synctoken, "order", displayname, description, color, timezone, deleted_at @@ -77,12 +72,12 @@ impl CalendarStore for SqliteStore { id ) .fetch_one(&self.db) - .await.map_err(Error::SqlxError)?; + .await.map_err(crate::Error::from)?; Ok(cal) } #[instrument] - async fn get_calendars(&self, principal: &str) -> Result, rustical_store::Error> { + async fn get_calendars(&self, principal: &str) -> Result, Error> { let cals = sqlx::query_as!( Calendar, r#"SELECT principal, id, synctoken, displayname, "order", description, color, timezone, deleted_at @@ -91,12 +86,12 @@ impl CalendarStore for SqliteStore { principal ) .fetch_all(&self.db) - .await.map_err(Error::SqlxError)?; + .await.map_err(crate::Error::from)?; Ok(cals) } #[instrument] - async fn insert_calendar(&self, calendar: Calendar) -> Result<(), rustical_store::Error> { + async fn insert_calendar(&self, calendar: Calendar) -> Result<(), Error> { sqlx::query!( r#"INSERT INTO calendars (principal, id, displayname, description, "order", color, timezone) VALUES (?, ?, ?, ?, ?, ?, ?)"#, @@ -109,7 +104,7 @@ impl CalendarStore for SqliteStore { calendar.timezone ) .execute(&self.db) - .await.map_err(Error::SqlxError)?; + .await.map_err(crate::Error::from)?; Ok(()) } @@ -119,7 +114,7 @@ impl CalendarStore for SqliteStore { principal: String, id: String, calendar: Calendar, - ) -> Result<(), rustical_store::Error> { + ) -> Result<(), Error> { let result = sqlx::query!( r#"UPDATE calendars SET principal = ?, id = ?, displayname = ?, description = ?, "order" = ?, color = ?, timezone = ? WHERE (principal, id) = (?, ?)"#, @@ -132,7 +127,7 @@ impl CalendarStore for SqliteStore { calendar.timezone, principal, id - ).execute(&self.db).await.map_err(Error::SqlxError)?; + ).execute(&self.db).await.map_err(crate::Error::from)?; if result.rows_affected() == 0 { return Err(rustical_store::Error::NotFound); } @@ -146,7 +141,7 @@ impl CalendarStore for SqliteStore { principal: &str, id: &str, use_trashbin: bool, - ) -> Result<(), rustical_store::Error> { + ) -> Result<(), Error> { match use_trashbin { true => { sqlx::query!( @@ -154,7 +149,7 @@ impl CalendarStore for SqliteStore { principal, id ) .execute(&self.db) - .await.map_err(Error::SqlxError)?; + .await.map_err(crate::Error::from)?; } false => { sqlx::query!( @@ -164,18 +159,14 @@ impl CalendarStore for SqliteStore { ) .execute(&self.db) .await - .map_err(Error::SqlxError)?; + .map_err(crate::Error::from)?; } }; Ok(()) } #[instrument] - async fn restore_calendar( - &self, - principal: &str, - id: &str, - ) -> Result<(), rustical_store::Error> { + async fn restore_calendar(&self, principal: &str, id: &str) -> Result<(), Error> { sqlx::query!( r"UPDATE calendars SET deleted_at = NULL WHERE (principal, id) = (?, ?)", principal, @@ -183,7 +174,7 @@ impl CalendarStore for SqliteStore { ) .execute(&self.db) .await - .map_err(Error::SqlxError)?; + .map_err(crate::Error::from)?; Ok(()) } @@ -192,7 +183,7 @@ impl CalendarStore for SqliteStore { &self, principal: &str, cal_id: &str, - ) -> Result, rustical_store::Error> { + ) -> Result, Error> { sqlx::query_as!( CalendarObjectRow, "SELECT id, ics FROM calendarobjects WHERE principal = ? AND cal_id = ? AND deleted_at IS NULL", @@ -200,7 +191,7 @@ impl CalendarStore for SqliteStore { cal_id ) .fetch_all(&self.db) - .await.map_err(Error::SqlxError)? + .await.map_err(crate::Error::from)? .into_iter() .map(|row| row.try_into().map_err(rustical_store::Error::from)) .collect() @@ -212,7 +203,7 @@ impl CalendarStore for SqliteStore { principal: &str, cal_id: &str, object_id: &str, - ) -> Result { + ) -> Result { Ok(sqlx::query_as!( CalendarObjectRow, "SELECT id, ics FROM calendarobjects WHERE (principal, cal_id, id) = (?, ?, ?)", @@ -222,7 +213,7 @@ impl CalendarStore for SqliteStore { ) .fetch_one(&self.db) .await - .map_err(Error::SqlxError)? + .map_err(crate::Error::from)? .try_into()?) } @@ -234,8 +225,8 @@ impl CalendarStore for SqliteStore { object: CalendarObject, // TODO: implement overwrite: bool, - ) -> Result<(), rustical_store::Error> { - let mut tx = self.db.begin().await.map_err(Error::SqlxError)?; + ) -> Result<(), Error> { + let mut tx = self.db.begin().await.map_err(crate::Error::from)?; let (object_id, ics) = (object.get_id(), object.get_ics()); @@ -248,7 +239,7 @@ impl CalendarStore for SqliteStore { ) .execute(&mut *tx) .await - .map_err(Error::SqlxError)?; + .map_err(crate::Error::from)?; log_object_operation( &mut tx, @@ -259,7 +250,7 @@ impl CalendarStore for SqliteStore { ) .await?; - tx.commit().await.map_err(Error::SqlxError)?; + tx.commit().await.map_err(crate::Error::from)?; Ok(()) } @@ -270,8 +261,8 @@ impl CalendarStore for SqliteStore { cal_id: &str, id: &str, use_trashbin: bool, - ) -> Result<(), rustical_store::Error> { - let mut tx = self.db.begin().await.map_err(Error::SqlxError)?; + ) -> Result<(), Error> { + let mut tx = self.db.begin().await.map_err(crate::Error::from)?; match use_trashbin { true => { @@ -282,7 +273,7 @@ impl CalendarStore for SqliteStore { id ) .execute(&mut *tx) - .await.map_err(Error::SqlxError)?; + .await.map_err(crate::Error::from)?; } false => { sqlx::query!( @@ -292,11 +283,11 @@ impl CalendarStore for SqliteStore { ) .execute(&mut *tx) .await - .map_err(Error::SqlxError)?; + .map_err(crate::Error::from)?; } }; log_object_operation(&mut tx, principal, cal_id, id, ChangeOperation::Delete).await?; - tx.commit().await.map_err(Error::SqlxError)?; + tx.commit().await.map_err(crate::Error::from)?; Ok(()) } @@ -306,8 +297,8 @@ impl CalendarStore for SqliteStore { principal: &str, cal_id: &str, object_id: &str, - ) -> Result<(), rustical_store::Error> { - let mut tx = self.db.begin().await.map_err(Error::SqlxError)?; + ) -> Result<(), Error> { + let mut tx = self.db.begin().await.map_err(crate::Error::from)?; sqlx::query!( r#"UPDATE calendarobjects SET deleted_at = NULL, updated_at = datetime() WHERE (principal, cal_id, id) = (?, ?, ?)"#, @@ -316,10 +307,10 @@ impl CalendarStore for SqliteStore { object_id ) .execute(&mut *tx) - .await.map_err(Error::SqlxError)?; + .await.map_err(crate::Error::from)?; log_object_operation(&mut tx, principal, cal_id, object_id, ChangeOperation::Add).await?; - tx.commit().await.map_err(Error::SqlxError)?; + tx.commit().await.map_err(crate::Error::from)?; Ok(()) } @@ -329,7 +320,7 @@ impl CalendarStore for SqliteStore { principal: &str, cal_id: &str, synctoken: i64, - ) -> Result<(Vec, Vec, i64), rustical_store::Error> { + ) -> Result<(Vec, Vec, i64), Error> { struct Row { object_id: String, synctoken: i64, @@ -344,7 +335,7 @@ impl CalendarStore for SqliteStore { synctoken ) .fetch_all(&self.db) - .await.map_err(Error::SqlxError)?; + .await.map_err(crate::Error::from)?; let mut objects = vec![]; let mut deleted_objects = vec![]; diff --git a/crates/store_sqlite/src/error.rs b/crates/store_sqlite/src/error.rs index 26f4889..b93ad4f 100644 --- a/crates/store_sqlite/src/error.rs +++ b/crates/store_sqlite/src/error.rs @@ -1,51 +1,32 @@ #[derive(Debug, thiserror::Error)] pub enum Error { - #[error("Not found")] - NotFound, - - #[error("Resource already exists and overwrite=false")] - AlreadyExists, - - #[error("Invalid ics/vcf input: {0}")] - InvalidData(String), - #[error(transparent)] SqlxError(sqlx::Error), #[error(transparent)] - Other(#[from] anyhow::Error), + StoreError(rustical_store::Error), } impl From for Error { fn from(value: sqlx::Error) -> Self { match value { - sqlx::Error::RowNotFound => Error::NotFound, + sqlx::Error::RowNotFound => Error::StoreError(rustical_store::Error::NotFound), err => Error::SqlxError(err), } } } -// TODO: clean up error types impl From for rustical_store::Error { fn from(value: Error) -> Self { match value { - Error::NotFound => Self::NotFound, - Error::AlreadyExists => Self::AlreadyExists, - Error::InvalidData(b) => Self::InvalidData(b), Error::SqlxError(err) => Self::Other(err.into()), - Error::Other(err) => Self::Other(err), + Error::StoreError(err) => err, } } } impl From for Error { fn from(value: rustical_store::Error) -> Self { - match value { - rustical_store::Error::NotFound => Self::NotFound, - rustical_store::Error::AlreadyExists => Self::AlreadyExists, - rustical_store::Error::InvalidData(b) => Self::InvalidData(b), - rustical_store::Error::Other(err) => Self::Other(err), - rustical_store::Error::ParserError(err) => Self::Other(err.into()), - } + Self::StoreError(value) } } diff --git a/crates/store_sqlite/src/lib.rs b/crates/store_sqlite/src/lib.rs index 7410dd6..827a051 100644 --- a/crates/store_sqlite/src/lib.rs +++ b/crates/store_sqlite/src/lib.rs @@ -26,7 +26,7 @@ impl SqliteStore { } } -pub async fn create_db_pool(db_url: &str, migrate: bool) -> anyhow::Result> { +pub async fn create_db_pool(db_url: &str, migrate: bool) -> Result, sqlx::Error> { let db = SqlitePool::connect_with( SqliteConnectOptions::new() .filename(db_url) @@ -40,7 +40,7 @@ pub async fn create_db_pool(db_url: &str, migrate: bool) -> anyhow::Result anyhow::Result { +pub async fn create_test_store() -> Result { let db = SqlitePool::connect("sqlite::memory:").await?; sqlx::migrate!("./migrations").run(&db).await?; Ok(SqliteStore::new(db))