From 4c4af46315511fff72620c64ecb68185aea1480d Mon Sep 17 00:00:00 2001 From: Nick Alexander Date: Mon, 7 May 2018 13:55:20 -0700 Subject: [PATCH] Add TransactableValue abstracting value places that can be transacted. This is a stepping stone to transacting entities that are not based on `edn::ValueAndSpan`. We need to turn some value places (general) into entity places (restricted), and those restrictions are captured in tx-parser right now. But for `TypedValue` value places, those restrictions are encoded in the type itself. This lays the track to accept other value types in value places, which is good for programmatic builder interfaces. --- db/src/internal_types.rs | 47 ++++++++++++++++++++++++++++- db/src/tx.rs | 64 ++++++++++++++++++++++++++++------------ tx-parser/src/lib.rs | 31 +------------------ 3 files changed, 92 insertions(+), 50 deletions(-) diff --git a/db/src/internal_types.rs b/db/src/internal_types.rs index 11104499..10b5bcdf 100644 --- a/db/src/internal_types.rs +++ b/db/src/internal_types.rs @@ -19,18 +19,63 @@ use mentat_core::KnownEntid; use mentat_core::util::Either; +use edn::{ + ValueAndSpan, +}; + use errors; -use errors::ErrorKind; +use errors::{ + ErrorKind, + Result, + ResultExt, +}; +use schema::{ + SchemaTypeChecking, +}; use types::{ AVMap, AVPair, Entid, + Schema, TypedValue, + ValueType, }; use mentat_tx::entities::{ + EntidOrLookupRefOrTempId, OpType, TempId, }; +use mentat_tx_parser; + +/// The transactor is tied to `edn::ValueAndSpan` right now, but in the future we'd like to support +/// `TypedValue` directly for programmatic use. `TransactableValue` encapsulates the interface +/// value types (i.e., values in the value place) need to support to be transacted. +pub trait TransactableValue { + /// Coerce this value place into the given type. This is where we perform schema-aware + /// coercion, for example coercing an integral value into a ref where appropriate. + fn into_typed_value(self, schema: &Schema, value_type: ValueType) -> Result; + + /// Make an entity place out of this value place. This is where we limit values in nested maps + /// to valid entity places. + fn into_entity_place(self) -> Result; + + fn as_tempid(&self) -> Option; +} + +impl TransactableValue for ValueAndSpan { + fn into_typed_value(self, schema: &Schema, value_type: ValueType) -> Result { + schema.to_typed_value(&self.without_spans(), value_type) + } + + fn into_entity_place(self) -> Result { + mentat_tx_parser::Tx::parse_entid_or_lookup_ref_or_temp_id(self) + .chain_err(|| ErrorKind::NotYetImplemented("db id error".into())) + } + + fn as_tempid(&self) -> Option { + self.inner.as_text().cloned().map(TempId::External) + } +} #[derive(Clone, Debug, Eq, Hash, Ord, PartialOrd, PartialEq)] pub enum Term { diff --git a/db/src/tx.rs b/db/src/tx.rs index 6e03c400..15a001f4 100644 --- a/db/src/tx.rs +++ b/db/src/tx.rs @@ -82,6 +82,7 @@ use internal_types::{ TermWithTempIds, TermWithTempIdsAndLookupRefs, TermWithoutTempIds, + TransactableValue, TypedValueOr, replace_lookup_ref, }; @@ -105,7 +106,6 @@ use mentat_tx::entities::{ OpType, TempId, }; -use mentat_tx_parser; use metadata; use rusqlite; use schema::{ @@ -161,6 +161,29 @@ pub struct Tx<'conn, 'a, W> where W: TransactWatcher { tx_instant: Option>, } +/// Remove any :db/id value from the given map notation, converting the returned value into +/// something suitable for the entity position rather than something suitable for a value position. +pub fn remove_db_id(map: &mut entmod::MapNotation) -> Result> { + // TODO: extract lazy defined constant. + let db_id_key = entmod::Entid::Ident(NamespacedKeyword::new("db", "id")); + + let db_id: Option = if let Some(id) = map.remove(&db_id_key) { + match id { + entmod::AtomOrLookupRefOrVectorOrMapNotation::Atom(v) => Some(v.into_entity_place()?), + entmod::AtomOrLookupRefOrVectorOrMapNotation::LookupRef(_) | + entmod::AtomOrLookupRefOrVectorOrMapNotation::TxFunction(_) | + entmod::AtomOrLookupRefOrVectorOrMapNotation::Vector(_) | + entmod::AtomOrLookupRefOrVectorOrMapNotation::MapNotation(_) => { + bail!(ErrorKind::NotYetImplemented("db id error".into())) + }, + } + } else { + None + }; + + Ok(db_id) +} + impl<'conn, 'a, W> Tx<'conn, 'a, W> where W: TransactWatcher { pub fn new( store: &'conn rusqlite::Connection, @@ -340,18 +363,19 @@ impl<'conn, 'a, W> Tx<'conn, 'a, W> where W: TransactWatcher { } match x { - entmod::AtomOrLookupRefOrVectorOrMapNotation::Atom(ref v) => { + entmod::AtomOrLookupRefOrVectorOrMapNotation::Atom(v) => { // Here is where we do schema-aware typechecking: we either assert // that the given value is in the attribute's value set, or (in // limited cases) coerce the value into the attribute's value set. - if let Some(text) = v.inner.as_text() { - Ok(Either::Right(LookupRefOrTempId::TempId(self.intern_temp_id(TempId::External(text.clone()))))) - } else { - if let TypedValue::Ref(entid) = self.schema.to_typed_value(&v.clone().without_spans(), ValueType::Ref)? { - Ok(Either::Left(KnownEntid(entid))) - } else { - // The given value is expected to be :db.type/ref, so this shouldn't happen. - bail!(ErrorKind::NotYetImplemented(format!("Cannot use :attr/_reversed notation for attribute {} with value that is not :db.valueType :db.type/ref", forward_a))) + match v.as_tempid() { + Some(tempid) => Ok(Either::Right(LookupRefOrTempId::TempId(self.intern_temp_id(tempid)))), + None => { + if let TypedValue::Ref(entid) = v.into_typed_value(&self.schema, ValueType::Ref)? { + Ok(Either::Left(KnownEntid(entid))) + } else { + // The given value is expected to be :db.type/ref, so this shouldn't happen. + bail!(ErrorKind::NotYetImplemented(format!("Cannot use :attr/_reversed notation for attribute {} with value that is not :db.valueType :db.type/ref", forward_a))) + } } } }, @@ -392,7 +416,7 @@ impl<'conn, 'a, W> Tx<'conn, 'a, W> where W: TransactWatcher { Entity::MapNotation(mut map_notation) => { // :db/id is optional; if it's not given, we generate a special internal tempid // to use for upserting. This tempid will not be reported in the TxReport. - let db_id: entmod::EntidOrLookupRefOrTempId = mentat_tx_parser::remove_db_id(&mut map_notation)?.unwrap_or_else(|| in_process.allocate_mentat_id()); + let db_id: entmod::EntidOrLookupRefOrTempId = remove_db_id(&mut map_notation)?.unwrap_or_else(|| in_process.allocate_mentat_id()); // We're not nested, so :db/isComponent is not relevant. We just explode the // map notation. @@ -418,14 +442,16 @@ impl<'conn, 'a, W> Tx<'conn, 'a, W> where W: TransactWatcher { let v = match v { entmod::AtomOrLookupRefOrVectorOrMapNotation::Atom(v) => { - if attribute.value_type == ValueType::Ref && v.inner.is_text() { - Either::Right(LookupRefOrTempId::TempId(in_process.intern_temp_id(v.inner.as_text().cloned().map(TempId::External).unwrap()))) + // Here is where we do schema-aware typechecking: we either assert + // that the given value is in the attribute's value set, or (in + // limited cases) coerce the value into the attribute's value set. + if attribute.value_type == ValueType::Ref { + match v.as_tempid() { + Some(tempid) => Either::Right(LookupRefOrTempId::TempId(in_process.intern_temp_id(tempid))), + None => v.into_typed_value(&self.schema, attribute.value_type).map(Either::Left)?, + } } else { - // Here is where we do schema-aware typechecking: we either assert that - // the given value is in the attribute's value set, or (in limited - // cases) coerce the value into the attribute's value set. - let typed_value: TypedValue = self.schema.to_typed_value(&v.without_spans(), attribute.value_type)?; - Either::Left(typed_value) + v.into_typed_value(&self.schema, attribute.value_type).map(Either::Left)? } }, @@ -488,7 +514,7 @@ impl<'conn, 'a, W> Tx<'conn, 'a, W> where W: TransactWatcher { // :db/id is optional; if it's not given, we generate a special internal tempid // to use for upserting. This tempid will not be reported in the TxReport. - let db_id: Option = mentat_tx_parser::remove_db_id(&mut map_notation)?; + let db_id: Option = remove_db_id(&mut map_notation)?; let mut dangling = db_id.is_none(); let db_id: entmod::EntidOrLookupRefOrTempId = db_id.unwrap_or_else(|| in_process.allocate_mentat_id()); diff --git a/tx-parser/src/lib.rs b/tx-parser/src/lib.rs index 37987bf0..e1af767c 100644 --- a/tx-parser/src/lib.rs +++ b/tx-parser/src/lib.rs @@ -177,7 +177,7 @@ impl<'a> Tx<'a> { .map_err(|e| Error::from_kind(ErrorKind::ParseError(e.into()))) } - fn parse_entid_or_lookup_ref_or_temp_id(input: edn::ValueAndSpan) -> std::result::Result { + pub fn parse_entid_or_lookup_ref_or_temp_id(input: edn::ValueAndSpan) -> std::result::Result { Tx::entid_or_lookup_ref_or_temp_id() .skip(eof()) .parse(input.atom_stream()) @@ -186,35 +186,6 @@ impl<'a> Tx<'a> { } } -/// Remove any :db/id value from the given map notation, converting the returned value into -/// something suitable for the entity position rather than something suitable for a value position. -/// -/// This is here simply to not expose some of the internal APIs of the tx-parser. -pub fn remove_db_id(map: &mut MapNotation) -> std::result::Result, errors::Error> { - // TODO: extract lazy defined constant. - let db_id_key = Entid::Ident(edn::NamespacedKeyword::new("db", "id")); - - let db_id: Option = if let Some(id) = map.remove(&db_id_key) { - match id { - AtomOrLookupRefOrVectorOrMapNotation::Atom(v) => { - let db_id = Tx::parse_entid_or_lookup_ref_or_temp_id(v) - .chain_err(|| Error::from(ErrorKind::DbIdError))?; - Some(db_id) - }, - AtomOrLookupRefOrVectorOrMapNotation::LookupRef(_) | - AtomOrLookupRefOrVectorOrMapNotation::TxFunction(_) | - AtomOrLookupRefOrVectorOrMapNotation::Vector(_) | - AtomOrLookupRefOrVectorOrMapNotation::MapNotation(_) => { - bail!(ErrorKind::DbIdError) - }, - } - } else { - None - }; - - Ok(db_id) -} - #[cfg(test)] mod tests { use super::*;