From 2d9819bb3338a139004c69a9079fdcb4ad14ab9e Mon Sep 17 00:00:00 2001 From: aleics Date: Sun, 2 Feb 2025 12:04:00 +0100 Subject: [PATCH] Improve error handling Remove internal panics with results. --- .github/workflows/ci.yml | 4 + Cargo.toml | 4 +- src/index.rs | 375 ++++++++++++++++++++++++++++----------- src/lib.rs | 4 +- src/main.rs | 15 +- src/query.rs | 104 +++++++---- src/storage.rs | 52 ++++-- 7 files changed, 400 insertions(+), 158 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7c13e38..13afa00 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -66,6 +66,10 @@ jobs: - name: start container run: docker compose up -d + - name: wait for init + run: sleep 30s + shell: bash + - name: run integration tests uses: actions-rs/cargo@v1 with: diff --git a/Cargo.toml b/Cargo.toml index 1d1d218..64a0ca3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -24,9 +24,9 @@ serde = { version = "1.0.217", features = ["derive"] } [dev-dependencies] lazy_static = "1.5.0" -rand = "0.8.5" +rand = "0.9.0" reqwest = { version = "0.12.12", features = ["json"] } -serde_json = "1.0.137" +serde_json = "1.0.138" tokio-test = "0.4.4" [features] diff --git a/src/index.rs b/src/index.rs index 9fd48fb..8225963 100644 --- a/src/index.rs +++ b/src/index.rs @@ -1,14 +1,16 @@ use std::collections::{BTreeMap, HashSet}; +use std::fmt::Display; use std::iter::FromIterator; use std::ops::Bound; use std::panic; use crate::data::{date_to_timestamp, parse_date, timestamp_to_date, FieldValue}; -use crate::query::{FilterOperation, FilterResult, SortDirection}; +use crate::query::{FilterName, FilterOperation, SortDirection}; use indexmap::IndexSet; use ordered_float::OrderedFloat; use roaring::{MultiOps, RoaringBitmap}; use serde::{Deserialize, Serialize}; +use thiserror::Error; use time::format_description::well_known::Iso8601; #[derive(Clone, Debug)] @@ -21,8 +23,8 @@ pub enum TypeDescriptor { } trait FilterableIndex { - fn filter(&self, op: &FilterOperation) -> FilterResult { - let hits = match op { + fn filter(&self, op: &FilterOperation) -> Result { + match op { FilterOperation::Eq(value) => self.equal(value), FilterOperation::Between(first, second) => { self.between(Bound::Included(first), Bound::Included(second)) @@ -39,19 +41,16 @@ trait FilterableIndex { FilterOperation::LessThanOrEqual(value) => { self.between(Bound::Unbounded, Bound::Included(value)) } - }; - - hits.map(FilterResult::new) - .unwrap_or_else(FilterResult::empty) + } } - fn equal(&self, value: &FieldValue) -> Option; + fn equal(&self, value: &FieldValue) -> Result; fn between( &self, first: Bound<&FieldValue>, second: Bound<&FieldValue>, - ) -> Option; + ) -> Result; } #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] @@ -89,7 +88,7 @@ impl Index { } } - pub(crate) fn filter(&self, op: &FilterOperation) -> FilterResult { + pub(crate) fn filter(&self, op: &FilterOperation) -> Result { match self { Index::String(index) => index.filter(op), Index::Numeric(index) => index.filter(op), @@ -119,7 +118,7 @@ impl Index { } } - pub(crate) fn put(&mut self, value: FieldValue, position: u32) { + pub(crate) fn put(&mut self, value: FieldValue, position: u32) -> Result<(), IndexError> { match self { Index::String(index) => index.put(value, position), Index::Numeric(index) => index.put(value, position), @@ -129,29 +128,41 @@ impl Index { } } - pub(crate) fn plus(&mut self, index: &Index) { + pub(crate) fn plus(&mut self, index: &Index) -> Result<(), IndexError> { match (self, index) { (Index::String(left), Index::String(right)) => left.plus(right), (Index::Numeric(left), Index::Numeric(right)) => left.plus(right), (Index::Date(left), Index::Date(right)) => left.plus(right), (Index::Enum(left), Index::Enum(right)) => left.plus(right), (Index::Bool(left), Index::Bool(right)) => left.plus(right), - _ => panic!("Could not apply a plus operation for indices of different types"), - } + _ => { + return Err(IndexError::UnsupportedOperation { + operation: "plus".to_string(), + }); + } + }; + + Ok(()) } - pub(crate) fn minus(&mut self, index: &Index) { + pub(crate) fn minus(&mut self, index: &Index) -> Result<(), IndexError> { match (self, index) { (Index::String(left), Index::String(right)) => left.minus(right), (Index::Numeric(left), Index::Numeric(right)) => left.minus(right), (Index::Date(left), Index::Date(right)) => left.minus(right), (Index::Enum(left), Index::Enum(right)) => left.minus(right), (Index::Bool(left), Index::Bool(right)) => left.minus(right), - _ => panic!("Could not apply a minus operation for indices of different types"), - } + _ => { + return Err(IndexError::UnsupportedOperation { + operation: "minus".to_string(), + }); + } + }; + + Ok(()) } - pub(crate) fn remove(&mut self, value: &FieldValue, position: u32) { + pub(crate) fn remove(&mut self, value: &FieldValue, position: u32) -> Result<(), IndexError> { match self { Index::String(index) => index.remove(value, position), Index::Numeric(index) => index.remove(value, position), @@ -204,20 +215,28 @@ impl StringIndex { .map(|value| FieldValue::str(value.as_str())) } - fn put(&mut self, value: FieldValue, position: u32) { - let value = value - .get_string() - .expect("String index only allows to insert string values."); + fn put(&mut self, value: FieldValue, position: u32) -> Result<(), IndexError> { + let Some(value) = value.get_string() else { + return Err(IndexError::UnexpectedValue { + expected_type: TypeName::String, + }); + }; self.inner.put(value, position); + + Ok(()) } - fn remove(&mut self, value: &FieldValue, position: u32) { - let value = value - .as_string() - .expect("String index only allows to remove string values."); + fn remove(&mut self, value: &FieldValue, position: u32) -> Result<(), IndexError> { + let Some(value) = value.as_string() else { + return Err(IndexError::UnexpectedValue { + expected_type: TypeName::String, + }); + }; self.inner.remove(value, position); + + Ok(()) } fn plus(&mut self, other: &StringIndex) { @@ -238,16 +257,32 @@ impl StringIndex { } impl FilterableIndex for StringIndex { - fn equal(&self, value: &FieldValue) -> Option { - let string_value = value - .as_string() - .expect("Invalid value for \"equal\" filter. Expected string value."); + fn equal(&self, value: &FieldValue) -> Result { + let Some(string_value) = value.as_string() else { + return Err(FilterError::InvalidInput { + filter: FilterName::Eq, + type_name: TypeName::String, + }); + }; - self.inner.get(string_value).cloned() + let hits = self + .inner + .get(string_value) + .cloned() + .unwrap_or_else(RoaringBitmap::new); + + Ok(hits) } - fn between(&self, _: Bound<&FieldValue>, _: Bound<&FieldValue>) -> Option { - panic!("Unsupported filter operation \"between\" for string index") + fn between( + &self, + _: Bound<&FieldValue>, + _: Bound<&FieldValue>, + ) -> Result { + Err(FilterError::UnsupportedOperation { + filter: FilterName::Between, + type_name: TypeName::String, + }) } } @@ -273,22 +308,32 @@ impl NumericIndex { .map(|value| FieldValue::Decimal(*value)) } - fn put(&mut self, value: FieldValue, position: u32) { + fn put(&mut self, value: FieldValue, position: u32) -> Result<(), IndexError> { let value = match value { FieldValue::Integer(value) => OrderedFloat(value as f64), FieldValue::Decimal(value) => value, - _ => panic!("Numeric index only allows to insert numeric values."), + _ => { + return Err(IndexError::UnexpectedValue { + expected_type: TypeName::Numeric, + }) + } }; self.inner.put(value, position); + + Ok(()) } - fn remove(&mut self, value: &FieldValue, position: u32) { - let value = value - .as_decimal() - .expect("Numeric index only allows to remove numeric values."); + fn remove(&mut self, value: &FieldValue, position: u32) -> Result<(), IndexError> { + let Some(value) = value.as_decimal() else { + return Err(IndexError::UnexpectedValue { + expected_type: TypeName::Numeric, + }); + }; self.inner.remove(value, position); + + Ok(()) } fn plus(&mut self, other: &NumericIndex) { @@ -309,19 +354,28 @@ impl NumericIndex { } impl FilterableIndex for NumericIndex { - fn equal(&self, value: &FieldValue) -> Option { - let numeric_value = value - .as_decimal() - .expect("Invalid value for \"equal\" filter. Expected numeric value."); + fn equal(&self, value: &FieldValue) -> Result { + let Some(numeric_value) = value.as_decimal() else { + return Err(FilterError::InvalidInput { + filter: FilterName::Eq, + type_name: TypeName::Numeric, + }); + }; + + let hits = self + .inner + .get(numeric_value) + .cloned() + .unwrap_or_else(RoaringBitmap::new); - self.inner.get(numeric_value).cloned() + Ok(hits) } fn between( &self, first: Bound<&FieldValue>, second: Bound<&FieldValue>, - ) -> Option { + ) -> Result { let first_bound = first.map(|value| { value .as_decimal() @@ -339,11 +393,7 @@ impl FilterableIndex for NumericIndex { matches |= bitmap; } - if matches.is_empty() { - None - } else { - Some(matches) - } + Ok(matches) } } @@ -384,18 +434,28 @@ impl DateIndex { Some(FieldValue::String(date)) } - fn put(&mut self, value: FieldValue, position: u32) { - let value = - DateIndex::parse_value(&value).expect("Date index only allows to insert date values."); + fn put(&mut self, value: FieldValue, position: u32) -> Result<(), IndexError> { + let Some(value) = DateIndex::parse_value(&value) else { + return Err(IndexError::UnexpectedValue { + expected_type: TypeName::Date, + }); + }; self.inner.put(value, position); + + Ok(()) } - fn remove(&mut self, value: &FieldValue, position: u32) { - let value = - DateIndex::parse_value(value).expect("Date index only allows to remove date values."); + fn remove(&mut self, value: &FieldValue, position: u32) -> Result<(), IndexError> { + let Some(value) = DateIndex::parse_value(value) else { + return Err(IndexError::UnexpectedValue { + expected_type: TypeName::Date, + }); + }; self.inner.remove(&value, position); + + Ok(()) } fn plus(&mut self, other: &DateIndex) { @@ -408,18 +468,28 @@ impl DateIndex { } impl FilterableIndex for DateIndex { - fn equal(&self, value: &FieldValue) -> Option { - let date_value = DateIndex::parse_value(value) - .expect("Invalid value for \"equal\" filter. Expected date value."); + fn equal(&self, value: &FieldValue) -> Result { + let Some(date_value) = DateIndex::parse_value(value) else { + return Err(FilterError::InvalidInput { + filter: FilterName::Eq, + type_name: TypeName::Date, + }); + }; - self.inner.get(&date_value).cloned() + let hits = self + .inner + .get(&date_value) + .cloned() + .unwrap_or_else(RoaringBitmap::new); + + Ok(hits) } fn between( &self, first: Bound<&FieldValue>, second: Bound<&FieldValue>, - ) -> Option { + ) -> Result { let first_bound = first.map(|value| { DateIndex::parse_value(value) .expect("Invalid \"between\" filter value. Expected date value.") @@ -435,11 +505,7 @@ impl FilterableIndex for DateIndex { matches |= bitmap; } - if matches.is_empty() { - None - } else { - Some(matches) - } + Ok(matches) } } @@ -474,30 +540,40 @@ impl EnumIndex { .map(|value| FieldValue::str(value.as_str())) } - fn put(&mut self, value: FieldValue, position: u32) { - let value = value - .as_string() - .expect("Enum index only allows to insert string values."); + fn put(&mut self, value: FieldValue, position: u32) -> Result<(), IndexError> { + let Some(value) = value.as_string() else { + return Err(IndexError::UnexpectedValue { + expected_type: TypeName::String, + }); + }; - let index = self - .values - .get_index_of(value) - .expect("Enum index does not know value to be inserted."); + let Some(index) = self.values.get_index_of(value) else { + return Err(IndexError::UnknownEnumValue { + value: value.clone(), + }); + }; self.inner.put(index, position); + + Ok(()) } - fn remove(&mut self, value: &FieldValue, position: u32) { - let value = value - .as_string() - .expect("Enum index only allows to remove string values."); + fn remove(&mut self, value: &FieldValue, position: u32) -> Result<(), IndexError> { + let Some(value) = value.as_string() else { + return Err(IndexError::UnexpectedValue { + expected_type: TypeName::String, + }); + }; - let index = self - .values - .get_index_of(value) - .expect("Enum index does not know value to be removed."); + let Some(index) = self.values.get_index_of(value) else { + return Err(IndexError::UnknownEnumValue { + value: value.clone(), + }); + }; self.inner.remove(&index, position); + + Ok(()) } fn plus(&mut self, other: &EnumIndex) { @@ -522,16 +598,35 @@ impl EnumIndex { } impl FilterableIndex for EnumIndex { - fn equal(&self, value: &FieldValue) -> Option { - let string_value = value - .as_string() - .expect("Enum index only supports string values for \"equal\" filter."); + fn equal(&self, value: &FieldValue) -> Result { + let Some(string_value) = value.as_string() else { + return Err(FilterError::InvalidInput { + filter: FilterName::Eq, + type_name: TypeName::Enum, + }); + }; - let index = self.values.get_index_of(string_value)?; - self.inner.get(&index).cloned() + let Some(index) = self.values.get_index_of(string_value) else { + return Err(FilterError::UnknownEnumValue { + filter: FilterName::Eq, + value: string_value.to_string(), + }); + }; + + let hits = self + .inner + .get(&index) + .cloned() + .unwrap_or_else(RoaringBitmap::new); + + Ok(hits) } - fn between(&self, _: Bound<&FieldValue>, _: Bound<&FieldValue>) -> Option { + fn between( + &self, + _: Bound<&FieldValue>, + _: Bound<&FieldValue>, + ) -> Result { panic!("Unsupported filter operation \"between\" for enum index") } } @@ -560,20 +655,28 @@ impl BoolIndex { .map(|value| FieldValue::Bool(*value)) } - fn put(&mut self, value: FieldValue, position: u32) { - let value = value - .get_bool() - .expect("Bool index only allows to insert bool values."); + fn put(&mut self, value: FieldValue, position: u32) -> Result<(), IndexError> { + let Some(value) = value.get_bool() else { + return Err(IndexError::UnexpectedValue { + expected_type: TypeName::Bool, + }); + }; self.inner.put(value, position); + + Ok(()) } - fn remove(&mut self, value: &FieldValue, position: u32) { - let value = value - .as_bool() - .expect("Bool index only allows to remove bool values."); + fn remove(&mut self, value: &FieldValue, position: u32) -> Result<(), IndexError> { + let Some(value) = value.as_bool() else { + return Err(IndexError::UnexpectedValue { + expected_type: TypeName::Bool, + }); + }; self.inner.remove(value, position); + + Ok(()) } fn plus(&mut self, other: &BoolIndex) { @@ -594,15 +697,28 @@ impl BoolIndex { } impl FilterableIndex for BoolIndex { - fn equal(&self, value: &FieldValue) -> Option { - let bool_value = value - .as_bool() - .expect("Invalid value for \"equal\" filter. Expected bool value."); + fn equal(&self, value: &FieldValue) -> Result { + let Some(bool_value) = value.as_bool() else { + return Err(FilterError::InvalidInput { + filter: FilterName::Eq, + type_name: TypeName::Bool, + }); + }; - self.inner.get(bool_value).cloned() + let hits = self + .inner + .get(bool_value) + .cloned() + .unwrap_or_else(RoaringBitmap::new); + + Ok(hits) } - fn between(&self, _: Bound<&FieldValue>, _: Bound<&FieldValue>) -> Option { + fn between( + &self, + _: Bound<&FieldValue>, + _: Bound<&FieldValue>, + ) -> Result { panic!("Unsupported filter operation \"between\" for bool index") } } @@ -709,6 +825,55 @@ impl SortableIndex { } } +#[derive(Clone, Debug)] +pub enum TypeName { + String, + Numeric, + Date, + Bool, + Enum, +} + +impl Display for TypeName { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + TypeName::String => write!(f, "string"), + TypeName::Numeric => write!(f, "numeric"), + TypeName::Date => write!(f, "date"), + TypeName::Bool => write!(f, "bool"), + TypeName::Enum => write!(f, "enum"), + } + } +} + +#[derive(Error, Debug)] +#[non_exhaustive] +pub enum IndexError { + #[error("index operation \"{operation}\" is not supported")] + UnsupportedOperation { operation: String }, + #[error("unexpected value for type {expected_type}")] + UnexpectedValue { expected_type: TypeName }, + #[error("Value \"{value}\" is unknown for enum")] + UnknownEnumValue { value: String }, +} + +#[derive(Error, Debug)] +#[non_exhaustive] +pub enum FilterError { + #[error("Invalid filter value for filter \"{filter}\". Expected {type_name} value")] + InvalidInput { + filter: FilterName, + type_name: TypeName, + }, + #[error("Unsupported operation for filter \"{filter}\". Expected {type_name} value")] + UnsupportedOperation { + filter: FilterName, + type_name: TypeName, + }, + #[error("Value \"{value}\" is unknown for enum in filter \"{filter}\"")] + UnknownEnumValue { value: String, filter: FilterName }, +} + #[cfg(test)] mod tests { use roaring::RoaringBitmap; @@ -729,7 +894,7 @@ mod tests { ])); // when - left.plus(&right); + left.plus(&right).unwrap(); // then assert_eq!( @@ -756,7 +921,7 @@ mod tests { )])); // when - left.minus(&right); + left.minus(&right).unwrap(); // then assert_eq!( diff --git a/src/lib.rs b/src/lib.rs index bec663e..d094051 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -43,7 +43,7 @@ impl Engine { pub fn create_entity(&self, name: String) -> Result<(), EngineError> { if self.entities.pin().contains_key(&name) { - panic!("Entity with name \"{}\" already exists", name); + return Err(EngineError::EntityAlreadyExists { name }); } let entity = StorageBuilder::new(&name).build()?; @@ -136,6 +136,8 @@ pub enum EngineError { Query(#[from] QueryError), #[error("entity not found")] EntityNotFound, + #[error("entity already exists")] + EntityAlreadyExists { name: String }, } #[cfg(test)] diff --git a/src/main.rs b/src/main.rs index bd0b31d..23f9be9 100644 --- a/src/main.rs +++ b/src/main.rs @@ -21,7 +21,7 @@ use delta_search::query::{ QueryExecution, Sort, SortDirection, }; use delta_search::storage::CreateFieldIndex; -use delta_search::Engine; +use delta_search::{Engine, EngineError}; use tracing::{error, info}; const DEFAULT_START_PAGE: usize = 0; @@ -47,7 +47,12 @@ impl App { self.inner .create_entity(name.to_string()) .inspect_err(|err| error!("Could not create entity: {}", err)) - .map_err(|_| anyhow!("Could not create entity `{}`", name))?; + .map_err(|err| match err { + EngineError::EntityAlreadyExists { name } => AppError::EntityAlreadyExists { + message: format!("Entity with name \"{}\" already exists", name), + }, + _ => AppError::ServerError(anyhow!("Could not create entity `{}`", name)), + })?; Ok(()) } @@ -163,6 +168,8 @@ impl App { enum AppError { #[error("filter query is not valid")] InvalidFilterQuery, + #[error("entity already exists")] + EntityAlreadyExists { message: String }, #[error("request is not valid")] InvalidRequest { message: String }, #[error(transparent)] @@ -178,6 +185,10 @@ impl IntoResponse for AppError { "Filter query is invalid or could not be parsed.".to_string(), )) .unwrap(), + AppError::EntityAlreadyExists { message } => Response::builder() + .status(StatusCode::CONFLICT) + .body(Body::new(format!("Conflict: \"{}\"", message))) + .unwrap(), AppError::InvalidRequest { message } => Response::builder() .status(StatusCode::BAD_REQUEST) .body(Body::new(format!("Invalid request: \"{}\"", message))) diff --git a/src/query.rs b/src/query.rs index c2becbe..bb76c2d 100644 --- a/src/query.rs +++ b/src/query.rs @@ -1,4 +1,5 @@ use std::collections::BTreeMap; +use std::fmt::Display; use pest::iterators::Pair; use pest::Parser; @@ -8,7 +9,7 @@ use thiserror::Error; use time::Date; use crate::data::{DataItem, DataItemId, FieldValue}; -use crate::index::Index; +use crate::index::{FilterError, Index}; use crate::storage::{position_to_id, EntityIndices, EntityStorage, StorageError}; #[derive(Debug, PartialEq, Serialize, Deserialize)] @@ -63,40 +64,56 @@ impl QueryIndices { self.indices.field_indices.get(name) } - fn execute_filter(&self, filter: &CompositeFilter) -> FilterResult { - match filter { + fn execute_filter(&self, filter: &CompositeFilter) -> Result { + let result = match filter { CompositeFilter::And(filters) => { - let result: Option = filters.iter().fold(None, |acc, filter| { - let inner = acc - .map(|current| current.and(self.execute_filter(filter))) - .unwrap_or_else(|| self.execute_filter(filter)); - Some(inner) - }); + let mut result: Option = None; + + for filter in filters { + let inner = self.execute_filter(filter)?; + let next = if let Some(current) = result { + current.and(inner) + } else { + inner + }; + + result = Some(next); + } result.unwrap_or_else(FilterResult::empty) } CompositeFilter::Or(filters) => { - let result: Option = filters.iter().fold(None, |acc, filter| { - let inner = acc - .map(|current| current.or(self.execute_filter(filter))) - .unwrap_or_else(|| self.execute_filter(filter)); - Some(inner) - }); + let mut result: Option = None; + + for filter in filters { + let inner = self.execute_filter(filter)?; + let next = if let Some(current) = result { + current.or(inner) + } else { + inner + }; + + result = Some(next); + } result.unwrap_or_else(FilterResult::empty) } CompositeFilter::Not(filter) => { - let result = self.execute_filter(filter); + let result = self.execute_filter(filter)?; FilterResult::new(&self.indices.all - result.hits) } CompositeFilter::Single(filter) => { - let index = self.get(&filter.name).unwrap_or_else(|| { - panic!("Filter with name {} has no index assigned", &filter.name) - }); + let Some(index) = self.get(&filter.name) else { + return Err(QueryError::MissingIndex(filter.name.to_string())); + }; + + let hits = index.filter(&filter.operation)?; - index.filter(&filter.operation) + FilterResult::new(hits) } - } + }; + + Ok(result) } fn execute_sort(&self, items: &RoaringBitmap, sort: &Sort) -> Result, QueryError> { @@ -156,11 +173,11 @@ impl OptionsQueryExecution { let indices = QueryIndices::new(indices); - let filter_result = self - .filter - .as_ref() - .map(|filter| indices.execute_filter(filter)) - .unwrap_or_else(|| FilterResult::new(indices.indices.all.clone())); + let filter_result = if let Some(filter) = self.filter.as_ref() { + indices.execute_filter(filter)? + } else { + FilterResult::new(indices.indices.all.clone()) + }; Ok(indices.compute_filter_options(filter_result.hits)) } @@ -212,11 +229,11 @@ impl QueryExecution { let indices = QueryIndices::new(indices); // Apply filter given the indices - let filter_result = self - .filter - .as_ref() - .map(|filter| indices.execute_filter(filter)) - .unwrap_or_else(|| FilterResult::new(indices.indices.all.clone())); + let filter_result = if let Some(filter) = self.filter.as_ref() { + indices.execute_filter(filter)? + } else { + FilterResult::new(indices.indices.all.clone()) + }; // Sort filter results into a vector of IDs let sorted_ids = self.sort(filter_result, &indices)?; @@ -417,6 +434,29 @@ pub enum FilterOperation { LessThanOrEqual(FieldValue), } +#[derive(Clone, Debug)] +pub enum FilterName { + Eq, + Between, + GreaterThan, + GreaterOrEqual, + LessThan, + LessThanOrEqual, +} + +impl Display for FilterName { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + FilterName::Eq => write!(f, "equal"), + FilterName::Between => write!(f, "between"), + FilterName::GreaterThan => write!(f, "greater than"), + FilterName::GreaterOrEqual => write!(f, "greater or equal than"), + FilterName::LessThan => write!(f, "less than"), + FilterName::LessThanOrEqual => write!(f, "less than or equal"), + } + } +} + #[derive(Debug, Clone)] pub struct DeltaChange { pub id: DataItemId, @@ -593,6 +633,8 @@ pub enum QueryError { #[error("index is not present for field \"{0}\"")] MissingIndex(String), #[error(transparent)] + Filter(#[from] FilterError), + #[error(transparent)] Storage(#[from] StorageError), } diff --git a/src/storage.rs b/src/storage.rs index 0baa641..1c633be 100644 --- a/src/storage.rs +++ b/src/storage.rs @@ -11,7 +11,7 @@ use serde::{Deserialize, Serialize}; use thiserror::Error; use crate::data::{date_to_timestamp, DataItem}; -use crate::index::{Index, TypeDescriptor}; +use crate::index::{Index, IndexError, TypeDescriptor}; use crate::query::{DeltaChange, DeltaScope}; use crate::DataItemId; @@ -172,10 +172,26 @@ impl<'a> BytesDecode<'a> for DeltaKeyContextCodec { pub struct EntityStorage { pub(crate) id: String, env: Env, + + /// Database storing the entity's data items, where the key is the data + /// item id and the value the data item itself. data: Database>, + + /// Database storing the entity's indices, where the key is the field name + /// and the value is the index. indices: Database>, + + /// Database storing custom bitmaps needed to index the data items' + /// and their positions in the indices. documents: Database>, + + /// Database to store deltas for each scope and affected field. The key + /// is an identifier for each delta scope, and the value is a map + /// of deltas for each field. deltas: Database>>, + + /// An in-memory key-value map to store type descriptors for each index. + /// This is propagated during initialization. index_descriptors: papaya::HashMap, } @@ -310,14 +326,14 @@ impl EntityStorage { }; if let Some(index) = indices_to_store.get_mut(index_name) { - index.put(value, position); + index.put(value, position)?; } else { let mut index = self .indices .get(&txn, index_name)? .unwrap_or_else(|| Index::from_type(index_descriptor)); - index.put(value, position); + index.put(value, position)?; indices_to_store.insert(index_name.clone(), index); } } @@ -362,7 +378,7 @@ impl EntityStorage { let position = id_to_position(id); if let Some(index) = indices_to_store.get_mut(&command.name) { - index.put(value, position); + index.put(value, position)?; } else { // Create the new index and appended in memory, after it's populated // with the item's data it will be stored. @@ -371,7 +387,7 @@ impl EntityStorage { .get(&txn, &command.name)? .unwrap_or_else(|| Index::from_type(&command.descriptor)); - index.put(value, position); + index.put(value, position)?; indices_to_store.insert(&command.name, index); self.index_descriptors .pin() @@ -382,10 +398,10 @@ impl EntityStorage { // Update the stored indices with the new entries for (name, index) in indices_to_store { - self.indices.put(&mut txn, name, &index).unwrap(); + self.indices.put(&mut txn, name, &index)?; } - txn.commit().unwrap(); + txn.commit()?; Ok(()) } @@ -499,8 +515,8 @@ impl EntityStorage { if stored_delta_key.timestamp <= scope_timestamp { for (field, stored_delta) in stored_deltas { if let Some(aggregated_delta) = aggregated_deltas.get_mut(&field) { - aggregated_delta.before.plus(&stored_delta.before); - aggregated_delta.after.plus(&stored_delta.after); + aggregated_delta.before.plus(&stored_delta.before)?; + aggregated_delta.after.plus(&stored_delta.after)?; aggregated_delta.affected |= &stored_delta.affected; } else { aggregated_deltas.insert(field, stored_delta); @@ -520,20 +536,20 @@ impl EntityStorage { fn apply_deltas( deltas: HashMap, existing: &mut EntityIndices, - ) -> AffectedData { + ) -> Result { let mut affected = AffectedData::default(); for (field_name, stored_delta) in &deltas { if let Some(index) = existing.field_indices.get_mut(field_name) { - index.minus(&stored_delta.before); - index.plus(&stored_delta.after); + index.minus(&stored_delta.before)?; + index.plus(&stored_delta.after)?; affected.items |= &stored_delta.affected; affected.fields.push(field_name.clone()); } } - affected + Ok(affected) } pub fn read_indices_in( @@ -553,7 +569,7 @@ impl EntityStorage { self.read_indices(&txn, &fields)? }; - let affected = EntityStorage::apply_deltas(deltas, &mut indices); + let affected = EntityStorage::apply_deltas(deltas, &mut indices)?; Ok(indices.with_affected(affected)) } @@ -564,7 +580,7 @@ impl EntityStorage { let deltas = self.read_deltas(&txn, scope)?; let mut indices = self.read_all_indices(&txn)?; - let affected = EntityStorage::apply_deltas(deltas, &mut indices); + let affected = EntityStorage::apply_deltas(deltas, &mut indices)?; Ok(indices.with_affected(affected)) } @@ -663,10 +679,10 @@ impl EntityStorage { let position = id_to_position(delta.id); if let Some(before) = index.as_ref().and_then(|index| index.get_value(position)) { - stored_delta.before.put(before, position); + stored_delta.before.put(before, position)?; } - stored_delta.after.put(delta.after.clone(), position); + stored_delta.after.put(delta.after.clone(), position)?; stored_delta.affected.insert(position); } } @@ -690,6 +706,8 @@ pub enum StorageError { CreateStoragePath(#[from] std::io::Error), #[error(transparent)] DbOperation(#[from] heed::Error), + #[error(transparent)] + Index(#[from] IndexError), } #[derive(Default, Debug)]