diff --git a/src/parse/pull.rs b/src/parse/pull.rs index 386d9e39..5a935c5c 100644 --- a/src/parse/pull.rs +++ b/src/parse/pull.rs @@ -8,7 +8,6 @@ use crate::data::attr::AttributeCardinality; use crate::data::json::JsonValue; use crate::data::keyword::Keyword; use crate::data::value::DataValue; -use crate::parse::triple::TxError; use crate::query::pull::{AttrPullSpec, PullSpec, PullSpecs}; use crate::runtime::transact::SessionTx; @@ -42,7 +41,9 @@ impl SessionTx { } else { input_kw.clone() }; - let attr = self.attr_by_kw(&kw)?.ok_or(TxError::AttrNotFound(kw))?; + let attr = self + .attr_by_kw(&kw)? + .ok_or_else(|| anyhow!("attribute {} not found", kw))?; let cardinality = attr.cardinality; Ok(PullSpec::Attr(AttrPullSpec { attr, @@ -160,7 +161,9 @@ impl SessionTx { } else { input_kw.clone() }; - let attr = self.attr_by_kw(&kw)?.ok_or(TxError::AttrNotFound(kw))?; + let attr = self + .attr_by_kw(&kw)? + .ok_or_else(|| anyhow!("attribute not found: {}", kw))?; let cardinality = cardinality_override.unwrap_or(attr.cardinality); let nested = self.parse_pull(&JsonValue::Array(sub_target), depth + 1)?; diff --git a/src/parse/query.rs b/src/parse/query.rs index 162c0769..3fdc7740 100644 --- a/src/parse/query.rs +++ b/src/parse/query.rs @@ -10,7 +10,6 @@ use crate::data::expr::{get_op, Expr}; use crate::data::json::JsonValue; use crate::data::keyword::{Keyword, PROG_ENTRY}; use crate::data::value::DataValue; -use crate::parse::triple::TxError; use crate::query::compile::{ Atom, AttrTripleAtom, BindingHeadTerm, DatalogProgram, QueryCompilationError, Rule, RuleApplyAtom, RuleSet, Term, @@ -418,7 +417,9 @@ impl SessionTx { ); let (k, v) = m.iter().next().unwrap(); let kw = Keyword::from(k as &str); - let attr = self.attr_by_kw(&kw)?.ok_or(TxError::AttrNotFound(kw))?; + let attr = self + .attr_by_kw(&kw)? + .ok_or_else(|| anyhow!("attribute {} not found", kw))?; ensure!( attr.indexing.is_unique_index(), "pull inside query must use unique index, of which {} is not", @@ -495,7 +496,9 @@ impl SessionTx { match attr_rep { JsonValue::String(s) => { let kw = Keyword::from(s as &str); - let attr = self.attr_by_kw(&kw)?.ok_or(TxError::AttrNotFound(kw))?; + let attr = self + .attr_by_kw(&kw)? + .ok_or_else(|| anyhow!("attribute {} not found", kw))?; Ok(attr) } v => bail!("expect attribute keyword for triple atom, got {}", v), diff --git a/src/parse/schema.rs b/src/parse/schema.rs index 430cba27..adee3ef4 100644 --- a/src/parse/schema.rs +++ b/src/parse/schema.rs @@ -1,4 +1,4 @@ -use anyhow::Result; +use anyhow::{anyhow, bail, ensure, Result}; use itertools::Itertools; use crate::data::attr::Attribute; @@ -15,58 +15,43 @@ impl AttrTxItem { pub fn parse_request(req: &JsonValue) -> Result<(Vec, String)> { let map = req .as_object() - .ok_or_else(|| AttrTxItemError::Decoding(req.clone(), "expected object".to_string()))?; + .ok_or_else(|| anyhow!("expect object, got {}", req))?; let comment = match map.get("comment") { None => "".to_string(), Some(c) => c.to_string(), }; - let items = map.get("attrs").ok_or_else(|| { - AttrTxItemError::Decoding(req.clone(), "expected key 'attrs'".to_string()) - })?; - let items = items.as_array().ok_or_else(|| { - AttrTxItemError::Decoding(items.clone(), "expected array".to_string()) - })?; - if items.is_empty() { - return Err(AttrTxItemError::Decoding( - req.clone(), - "'attrs' cannot be empty".to_string(), - ) - .into()); - } + let items = map + .get("attrs") + .ok_or_else(|| anyhow!("expect key 'attrs' in {:?}", map))?; + let items = items + .as_array() + .ok_or_else(|| anyhow!("expect array for value of key 'attrs', got {:?}", items))?; + ensure!( + !items.is_empty(), + "array for value of key 'attrs' must be non-empty" + ); let res = items.iter().map(AttrTxItem::try_from).try_collect()?; Ok((res, comment)) } } -#[derive(Debug, thiserror::Error)] -pub enum AttrTxItemError { - #[error("Error decoding {0}: {1}")] - Decoding(JsonValue, String), -} - impl TryFrom<&'_ JsonValue> for AttrTxItem { type Error = anyhow::Error; fn try_from(value: &'_ JsonValue) -> Result { - let map = value.as_object().ok_or_else(|| { - AttrTxItemError::Decoding(value.clone(), "expected object".to_string()) - })?; - if map.len() != 1 { - return Err(AttrTxItemError::Decoding( - value.clone(), - "object must have exactly one field".to_string(), - ) - .into()); - } + let map = value + .as_object() + .ok_or_else(|| anyhow!("expect object for attribute tx, got {}", value))?; + ensure!( + map.len() == 1, + "attr definition must have exactly one pair, got {}", + value + ); let (k, v) = map.into_iter().next().unwrap(); let op = match k as &str { "put" => StoreOp::Assert, "retract" => StoreOp::Retract, - _ => { - return Err( - AttrTxItemError::Decoding(value.clone(), format!("unknown op {}", k)).into(), - ); - } + _ => bail!("unknown op {} for attribute tx", k), }; let attr = Attribute::try_from(v)?; diff --git a/src/parse/triple.rs b/src/parse/triple.rs index 80db53b4..c77fb24f 100644 --- a/src/parse/triple.rs +++ b/src/parse/triple.rs @@ -2,7 +2,7 @@ use std::collections::btree_map::Entry; use std::collections::BTreeMap; use std::fmt::{Display, Formatter}; -use anyhow::Result; +use anyhow::{anyhow, bail, ensure, Result}; use serde_json::Map; use crate::data::attr::{Attribute, AttributeIndex, AttributeTyping}; @@ -42,22 +42,6 @@ impl Display for TxAction { } } -#[derive(Debug, thiserror::Error)] -pub enum TxError { - #[error("Error decoding {0}: {1}")] - Decoding(JsonValue, String), - #[error("triple length error")] - TripleLength, - #[error("attribute not found: {0}")] - AttrNotFound(Keyword), - #[error("wrong specification of entity id {0}: {1}")] - EntityId(u64, String), - #[error("invalid action {0:?}: {1}")] - InvalidAction(TxAction, String), - #[error("temp id does not occur in head position: {0}")] - TempIdNoHead(String), -} - #[derive(Default)] pub(crate) struct TempIdCtx { store: BTreeMap, @@ -67,9 +51,11 @@ pub(crate) struct TempIdCtx { impl TempIdCtx { fn validate_usage(&self) -> Result<()> { for (k, (_, b)) in self.store.iter() { - if !*b { - return Err(TxError::TempIdNoHead(k.to_string()).into()); - } + ensure!( + *b, + "defining temp id {} in non-head position is not allowed", + k + ); } Ok(()) } @@ -124,23 +110,15 @@ impl SessionTx { /// } /// ``` /// nesting is allowed for values of type `ref` and `component` - pub fn parse_tx_requests( - &mut self, - req: &JsonValue, - ) -> Result<(Vec, String)> { + pub fn parse_tx_requests(&mut self, req: &JsonValue) -> Result<(Vec, String)> { let map = req .as_object() - .ok_or_else(|| TxError::Decoding(req.clone(), "expected object".to_string()))?; + .ok_or_else(|| anyhow!("expect tx request to be an object, got {}", req))?; let items = map .get("tx") - .ok_or_else(|| TxError::Decoding(req.clone(), "expected field 'tx'".to_string()))? + .ok_or_else(|| anyhow!("expect field 'tx' in tx request object {}", req))? .as_array() - .ok_or_else(|| { - TxError::Decoding( - req.clone(), - "expected field 'tx' to be an array".to_string(), - ) - })?; + .ok_or_else(|| anyhow!("expect field 'tx' to be an array in {}", req))?; let default_since = match map.get("since") { None => Validity::current(), Some(v) => v.try_into()?, @@ -166,7 +144,7 @@ impl SessionTx { ) -> Result<()> { let item = item .as_object() - .ok_or_else(|| TxError::Decoding(item.clone(), "expected object".to_string()))?; + .ok_or_else(|| anyhow!("expect tx request item to be an object, got {}", item))?; let (inner, action) = { if let Some(inner) = item.get("put") { (inner, TxAction::Put) @@ -175,11 +153,10 @@ impl SessionTx { } else if let Some(inner) = item.get("ensure") { (inner, TxAction::Ensure) } else { - return Err(TxError::Decoding( - JsonValue::Object(item.clone()), - "expect any of the keys 'put', 'retract', 'erase', 'ensure'".to_string(), - ) - .into()); + bail!( + "expect key 'put', 'retract', 'erase' or 'ensure' in tx request object, got {:?}", + item + ); } }; let since = match item.get("since") { @@ -196,7 +173,7 @@ impl SessionTx { .map(|_| ()); } - Err(TxError::Decoding(inner.clone(), "expected object or array".to_string()).into()) + bail!("expect object or array for tx object item, got {}", inner); } fn parse_tx_request_inner<'a>( &mut self, @@ -224,13 +201,11 @@ impl SessionTx { return Ok(()); } - if !eid.is_perm() && action != TxAction::Put { - return Err(TxError::InvalidAction( - action, - "using temp id instead of perm id".to_string(), - ) - .into()); - } + ensure!( + action == TxAction::Put || eid.is_perm(), + "using temp id instead of perm id for op {} is not allow", + action + ); let v = if let JsonValue::Object(inner) = value { self.parse_tx_component(attr, inner, action, since, temp_id_ctx, collected)? @@ -259,19 +234,15 @@ impl SessionTx { temp_id_ctx: &mut TempIdCtx, collected: &mut Vec, ) -> Result { - if action != TxAction::Put { - return Err(TxError::InvalidAction( - action, - "component shorthand cannot be used".to_string(), - ) - .into()); - } + ensure!( + action == TxAction::Put, + "component shorthand can only be use for 'put', got {}", + action + ); let (eid, has_unique_attr) = self.parse_tx_request_obj(comp, true, action, since, temp_id_ctx, collected)?; - if !has_unique_attr && parent_attr.val_type != AttributeTyping::Component { - return Err(TxError::InvalidAction(action, - "component shorthand must contain at least one unique/identity field for non-component refs".to_string()).into()); - } + ensure!(has_unique_attr || parent_attr.val_type == AttributeTyping::Component, + "component shorthand must contain at least one unique/identity field for non-component refs"); Ok(DataValue::EnId(eid)) } fn parse_tx_request_arr<'a>( @@ -284,22 +255,16 @@ impl SessionTx { ) -> Result<()> { match item { [eid] => { - if action != TxAction::Retract { - return Err(TxError::InvalidAction( - action, - "singlet only allowed for 'retract'".to_string(), - ) - .into()); - } - let eid = eid.as_u64().ok_or_else(|| { - TxError::Decoding(eid.clone(), "cannot parse as entity id".to_string()) - })?; + ensure!( + action == TxAction::Retract, + "singlet action only allowed for 'retract', got {}", + action + ); + let eid = eid + .as_u64() + .ok_or_else(|| anyhow!("cannot parse {} as entity id", eid))?; let eid = EntityId(eid); - if !eid.is_perm() { - return Err( - TxError::EntityId(eid.0, "expected perm entity id".to_string()).into(), - ); - } + ensure!(eid.is_perm(), "expected perm entity id, got {:?}", eid); collected.push(Quintuple { triple: Triple { id: eid, @@ -312,25 +277,21 @@ impl SessionTx { Ok(()) } [eid, attr] => { - if action != TxAction::Retract { - return Err(TxError::InvalidAction( - action, - "doublet only allowed for 'retract'".to_string(), - ) - .into()); - } + ensure!( + action == TxAction::Retract, + "double only allowed for 'retract', got {}", + action + ); let kw: Keyword = attr.try_into()?; - let attr = self.attr_by_kw(&kw)?.ok_or(TxError::AttrNotFound(kw))?; + let attr = self + .attr_by_kw(&kw)? + .ok_or_else(|| anyhow!("attribute not found {}", kw))?; - let eid = eid.as_u64().ok_or_else(|| { - TxError::Decoding(eid.clone(), "cannot parse as entity id".to_string()) - })?; + let eid = eid + .as_u64() + .ok_or_else(|| anyhow!("cannot parse {} as entity id", eid))?; let eid = EntityId(eid); - if !eid.is_perm() { - return Err( - TxError::EntityId(eid.0, "expected perm entity id".to_string()).into(), - ); - } + ensure!(eid.is_perm(), "expect perm entity id, got {:?}", eid); collected.push(Quintuple { triple: Triple { id: eid, @@ -345,7 +306,7 @@ impl SessionTx { [eid, attr_kw, val] => { self.parse_tx_triple(eid, attr_kw, val, action, since, temp_id_ctx, collected) } - _ => Err(TxError::TripleLength.into()), + arr => bail!("bad triple in tx: {:?}", arr), } } fn parse_tx_triple<'a>( @@ -359,7 +320,9 @@ impl SessionTx { collected: &mut Vec, ) -> Result<()> { let kw: Keyword = attr_kw.try_into()?; - let attr = self.attr_by_kw(&kw)?.ok_or(TxError::AttrNotFound(kw))?; + let attr = self + .attr_by_kw(&kw)? + .ok_or_else(|| anyhow!("attribute not found: {}", kw))?; if attr.cardinality.is_many() && attr.val_type != AttributeTyping::List && val.is_array() { for cur_val in val.as_array().unwrap() { self.parse_tx_triple(eid, attr_kw, cur_val, action, since, temp_id_ctx, collected)?; @@ -378,9 +341,7 @@ impl SessionTx { None => { if let Some(i) = eid.as_u64() { let id = EntityId(i); - if !id.is_perm() { - return Err(TxError::EntityId(id.0, "temp id specified".into()).into()); - } + ensure!(id.is_perm(), "temp id not allowed here, found {:?}", id); id } else if let Some(s) = eid.as_str() { temp_id_ctx.str2tempid(s, true) @@ -391,23 +352,19 @@ impl SessionTx { Some(existing_id) => { if let Some(i) = eid.as_u64() { let id = EntityId(i); - if !id.is_perm() { - return Err(TxError::EntityId(id.0, "temp id specified".into()).into()); - } - if existing_id != id { - return Err(TxError::EntityId( - id.0, - "conflicting id for identity value".into(), - ) - .into()); - } + ensure!(id.is_perm(), "temp id not allowed here, found {:?}", id); + ensure!( + existing_id == id, + "conflicting id for identity value: {:?} vs {:?}", + existing_id, + id + ); id } else if eid.is_string() { - return Err(TxError::EntityId( - existing_id.0, - "specifying temp_id string together with unique constraint".into(), - ) - .into()); + bail!( + "specifying temp_id string {} together with unique constraint", + eid + ); } else { existing_id } @@ -415,9 +372,7 @@ impl SessionTx { } } else if let Some(i) = eid.as_u64() { let id = EntityId(i); - if !id.is_perm() { - return Err(TxError::EntityId(id.0, "temp id specified".into()).into()); - } + ensure!(id.is_perm(), "temp id not allowed here, found {:?}", id); id } else if let Some(s) = eid.as_str() { temp_id_ctx.str2tempid(s, true) @@ -453,7 +408,7 @@ impl SessionTx { let kw = (k as &str).into(); let attr = self .attr_by_kw(&kw)? - .ok_or_else(|| TxError::AttrNotFound(kw.clone()))?; + .ok_or_else(|| anyhow!("attribute {} not found", kw))?; has_unique_attr = has_unique_attr || attr.indexing.is_unique_index(); has_identity_attr = has_identity_attr || attr.indexing == AttributeIndex::Identity; if attr.indexing == AttributeIndex::Identity { @@ -474,13 +429,12 @@ impl SessionTx { None => {} Some(existing_eid) => { if let Some(prev_eid) = eid { - if existing_eid != prev_eid { - return Err(TxError::EntityId( - existing_eid.0, - "conflicting entity id given".to_string(), - ) - .into()); - } + ensure!( + existing_eid == prev_eid, + "conflicting entity id: {:?} vs {:?}", + existing_eid, + prev_eid + ); } eid = Some(existing_eid) } @@ -490,62 +444,52 @@ impl SessionTx { } } if let Some(given_id) = item.get(PERM_ID_FIELD) { - let given_id = given_id.as_u64().ok_or_else(|| { - TxError::Decoding( - given_id.clone(), - "unable to interpret as entity id".to_string(), - ) - })?; + let given_id = given_id + .as_u64() + .ok_or_else(|| anyhow!("unable to interpret {} as entity id", given_id))?; let given_id = EntityId(given_id); - if !given_id.is_perm() { - return Err(TxError::EntityId( - given_id.0, - "temp id given where perm id is required".to_string(), - ) - .into()); - } + ensure!( + given_id.is_perm(), + "temp id not allowed here, found {:?}", + given_id + ); if let Some(prev_id) = eid { - if prev_id != given_id { - return Err(TxError::EntityId( - given_id.0, - "conflicting entity id given".to_string(), - ) - .into()); - } + ensure!( + prev_id == given_id, + "conflicting entity id give: {:?} vs {:?}", + prev_id, + given_id + ); } eid = Some(given_id); } if let Some(temp_id) = item.get(TEMP_ID_FIELD) { - if let Some(eid_inner) = eid { - return Err(TxError::EntityId( - eid_inner.0, - "conflicting entity id given".to_string(), - ) - .into()); - } - let temp_id_str = temp_id.as_str().ok_or_else(|| { - TxError::Decoding( - temp_id.clone(), - "unable to interpret as temp id".to_string(), - ) - })?; + ensure!( + eid.is_none(), + "conflicting entity id given: {:?} vs {}", + eid.unwrap(), + temp_id + ); + let temp_id_str = temp_id + .as_str() + .ok_or_else(|| anyhow!("unable to interpret {} as temp id", temp_id))?; eid = Some(temp_id_ctx.str2tempid(temp_id_str, true)); } let eid = match eid { Some(eid) => eid, None => temp_id_ctx.unnamed_tempid(), }; - if action != TxAction::Put && !eid.is_perm() { - return Err(TxError::InvalidAction(action, "temp id not allowed".to_string()).into()); - } + ensure!( + action == TxAction::Put || eid.is_perm(), + "temp id {:?} not allowed for {}", + eid, + action + ); if !is_sub_component { - if action == TxAction::Put && eid.is_perm() && !has_identity_attr { - return Err(TxError::InvalidAction( - action, - "upsert requires identity attribute present".to_string(), - ) - .into()); - } + ensure!( + action != TxAction::Put || !eid.is_perm() || has_identity_attr, + "upsert requires identity attribute present" + ); for (attr, v) in pairs { self.parse_tx_request_inner(eid, &attr, v, action, since, temp_id_ctx, collected)?; } @@ -555,15 +499,13 @@ impl SessionTx { } } else { for (attr, _v) in pairs { - if !attr.indexing.is_unique_index() { - return Err(TxError::InvalidAction( - action, - "cannot use non-unique fields to specify entity".to_string(), - ) - .into()); - } + ensure!( + attr.indexing.is_unique_index(), + "cannot use non-unique attribute {} to specify entity", + attr.keyword + ); } } Ok((eid, has_unique_attr)) } -} \ No newline at end of file +}