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

@@ -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<TypedValue>;
+
+    /// 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<EntidOrLookupRefOrTempId>;
+
+    fn as_tempid(&self) -> Option<TempId>;
+}
+
+impl TransactableValue for ValueAndSpan {
+    fn into_typed_value(self, schema: &Schema, value_type: ValueType) -> Result<TypedValue> {
+        schema.to_typed_value(&self.without_spans(), value_type)
+    }
+
+    fn into_entity_place(self) -> Result<EntidOrLookupRefOrTempId> {
+        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<TempId> {
+        self.inner.as_text().cloned().map(TempId::External)
+    }
+}
 
 #[derive(Clone, Debug, Eq, Hash, Ord, PartialOrd, PartialEq)]
 pub enum Term<E, V> {

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<DateTime<Utc>>,
 }
 
+/// 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<Option<entmod::EntidOrLookupRefOrTempId>> {
+    // TODO: extract lazy defined constant.
+    let db_id_key = entmod::Entid::Ident(NamespacedKeyword::new("db", "id"));
+
+    let db_id: Option<entmod::EntidOrLookupRefOrTempId> = 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() {
+                                match v.as_tempid() {
-                                    Ok(Either::Right(LookupRefOrTempId::TempId(self.intern_temp_id(TempId::External(text.clone())))))
+                                    Some(tempid) => Ok(Either::Right(LookupRefOrTempId::TempId(self.intern_temp_id(tempid)))),
-                                } else {
+                                    None => {
-                                    if let TypedValue::Ref(entid) = self.schema.to_typed_value(&v.clone().without_spans(), ValueType::Ref)? {
+                                        if let TypedValue::Ref(entid) = v.into_typed_value(&self.schema, ValueType::Ref)? {
-                                        Ok(Either::Left(KnownEntid(entid)))
+                                            Ok(Either::Left(KnownEntid(entid)))
-                                    } else {
+                                        } else {
-                                        // The given value is expected to be :db.type/ref, so this shouldn't happen.
+                                            // 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)))
+                                            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() {
+                                // Here is where we do schema-aware typechecking: we either assert
-                                    Either::Right(LookupRefOrTempId::TempId(in_process.intern_temp_id(v.inner.as_text().cloned().map(TempId::External).unwrap())))
+                                // 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
+                                    v.into_typed_value(&self.schema, attribute.value_type).map(Either::Left)?
-                                    // 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)
                                 }
                             },
 
@@ -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<entmod::EntidOrLookupRefOrTempId> = mentat_tx_parser::remove_db_id(&mut map_notation)?;
+                                let db_id: Option<entmod::EntidOrLookupRefOrTempId> = 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());
 

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<EntidOrLookupRefOrTempId, errors::Error> {
+    pub fn parse_entid_or_lookup_ref_or_temp_id(input: edn::ValueAndSpan) -> std::result::Result<EntidOrLookupRefOrTempId, errors::Error> {
         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<Option<EntidOrLookupRefOrTempId>, errors::Error> {
-    // TODO: extract lazy defined constant.
-    let db_id_key = Entid::Ident(edn::NamespacedKeyword::new("db", "id"));
-
-    let db_id: Option<EntidOrLookupRefOrTempId> = 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::*;