diff --git a/db/src/db.rs b/db/src/db.rs index e26395f0..7df269ab 100644 --- a/db/src/db.rs +++ b/db/src/db.rs @@ -2085,7 +2085,7 @@ mod tests { // Verify that we can explode map notation with nested maps, even if the inner map would be // dangling, if we give a :db/id explicitly. - assert_transact!(conn, "[{:test/dangling {:db/id \"t\" :test/many 11}}]"); + assert_transact!(conn, "[{:test/dangling {:db/id \"t\" :test/many 12}}]"); } #[test] @@ -2191,21 +2191,35 @@ mod tests { assert_matches!(tempids(&report), "{}"); - // Verify that we can't explode direct reverse notation with nested value maps. - assert_transact!(conn, - "[[:db/add \"t\" :test/_dangling {:test/many 11}]]", - Err("not yet implemented: Cannot explode map notation value in :attr/_reversed notation for attribute 444")); + } + + #[test] + fn test_explode_reversed_notation_errors() { + let mut conn = TestConn::default(); + + // Start by installing a few attributes. + assert_transact!(conn, "[[:db/add 111 :db/ident :test/many] + [:db/add 111 :db/valueType :db.type/long] + [:db/add 111 :db/cardinality :db.cardinality/many] + [:db/add 222 :db/ident :test/component] + [:db/add 222 :db/isComponent true] + [:db/add 222 :db/valueType :db.type/ref] + [:db/add 333 :db/ident :test/unique] + [:db/add 333 :db/unique :db.unique/identity] + [:db/add 333 :db/index true] + [:db/add 333 :db/valueType :db.type/long] + [:db/add 444 :db/ident :test/dangling] + [:db/add 444 :db/valueType :db.type/ref]]"); + + // `tx-parser` should fail to parse direct reverse notation with nested value maps and + // nested value vectors, so we only test things that "get through" to the map notation + // dynamic processor here. // Verify that we can't explode reverse notation in map notation with nested value maps. assert_transact!(conn, - "[{:test/_dangling {:test/many 11}}]", + "[{:test/_dangling {:test/many 14}}]", Err("not yet implemented: Cannot explode map notation value in :attr/_reversed notation for attribute 444")); - // Verify that we can't explode direct reverse notation with nested value vectors. - assert_transact!(conn, - "[[:db/add \"t\" :test/_dangling [:test/many]]]", - Err("not yet implemented: Cannot explode vector value in :attr/_reversed notation for attribute 444")); - // Verify that we can't explode reverse notation in map notation with nested value vectors. assert_transact!(conn, "[{:test/_dangling [:test/many]}]", diff --git a/parser-utils/src/value_and_span.rs b/parser-utils/src/value_and_span.rs index 20b9de3c..ba073ad7 100644 --- a/parser-utils/src/value_and_span.rs +++ b/parser-utils/src/value_and_span.rs @@ -452,6 +452,26 @@ pub fn namespaced_keyword<'a>() -> Expected, fn(Stream<'a>) parser(namespaced_keyword_ as fn(Stream<'a>) -> ParseResult<&'a edn::NamespacedKeyword, Stream<'a>>).expected("namespaced_keyword") } +pub fn forward_keyword_<'a>(input: Stream<'a>) -> ParseResult<&'a edn::NamespacedKeyword, Stream<'a>> { + satisfy_map(|v: &'a edn::ValueAndSpan| v.inner.as_namespaced_keyword().and_then(|k| if k.is_forward() { Some(k) } else { None })) + .parse_lazy(input) + .into() +} + +pub fn forward_keyword<'a>() -> Expected, fn(Stream<'a>) -> ParseResult<&'a edn::NamespacedKeyword, Stream<'a>>>> { + parser(forward_keyword_ as fn(Stream<'a>) -> ParseResult<&'a edn::NamespacedKeyword, Stream<'a>>).expected("forward_keyword") +} + +pub fn backward_keyword_<'a>(input: Stream<'a>) -> ParseResult<&'a edn::NamespacedKeyword, Stream<'a>> { + satisfy_map(|v: &'a edn::ValueAndSpan| v.inner.as_namespaced_keyword().and_then(|k| if k.is_backward() { Some(k) } else { None })) + .parse_lazy(input) + .into() +} + +pub fn backward_keyword<'a>() -> Expected, fn(Stream<'a>) -> ParseResult<&'a edn::NamespacedKeyword, Stream<'a>>>> { + parser(backward_keyword_ as fn(Stream<'a>) -> ParseResult<&'a edn::NamespacedKeyword, Stream<'a>>).expected("backward_keyword") +} + /// Generate a `satisfy` expression that matches a `PlainSymbol` value with the given name. /// /// We do this rather than using `combine::token` so that we don't need to allocate a new `String` diff --git a/tx-parser/src/lib.rs b/tx-parser/src/lib.rs index e92ccf35..875303e3 100644 --- a/tx-parser/src/lib.rs +++ b/tx-parser/src/lib.rs @@ -45,6 +45,8 @@ use mentat_parser_utils::{ResultParser}; use mentat_parser_utils::value_and_span::{ Item, OfExactlyParsing, + backward_keyword, + forward_keyword, integer, list, map, @@ -57,12 +59,25 @@ pub use errors::*; pub struct Tx<'a>(std::marker::PhantomData<&'a ()>); +// Accepts entid, :attribute/forward, and :attribute/_backward. def_parser!(Tx, entid, Entid, { integer() .map(|x| Entid::Entid(x)) .or(namespaced_keyword().map(|x| Entid::Ident(x.clone()))) }); +// Accepts entid and :attribute/forward. +def_parser!(Tx, forward_entid, Entid, { + integer() + .map(|x| Entid::Entid(x)) + .or(forward_keyword().map(|x| Entid::Ident(x.clone()))) +}); + +// Accepts only :attribute/_backward. +def_parser!(Tx, backward_entid, Entid, { + backward_keyword().map(|x| Entid::Ident(x.to_reversed())) +}); + def_matches_plain_symbol!(Tx, literal_lookup_ref, "lookup-ref"); def_parser!(Tx, lookup_ref, LookupRef, { @@ -106,18 +121,23 @@ def_matches_namespaced_keyword!(Tx, literal_db_retract, "db", "retract"); def_parser!(Tx, add_or_retract, Entity, { vector().of_exactly( + // TODO: This commits as soon as it sees :db/{add,retract}, but could use an improved error message. (Tx::literal_db_add().map(|_| OpType::Add).or(Tx::literal_db_retract().map(|_| OpType::Retract)), - Tx::entid_or_lookup_ref_or_temp_id(), - Tx::entid(), - Tx::atom_or_lookup_ref_or_vector()) - .map(|(op, e, a, v)| { - Entity::AddOrRetract { - op: op, - e: e, - a: a, - v: v, - } - })) + try((Tx::entid_or_lookup_ref_or_temp_id(), + Tx::forward_entid(), + Tx::atom_or_lookup_ref_or_vector())) + .or(try((Tx::atom_or_lookup_ref_or_vector(), + Tx::backward_entid(), + Tx::entid_or_lookup_ref_or_temp_id())) + .map(|(v, a, e)| (e, a, v)))) + .map(|(op, (e, a, v))| { + Entity::AddOrRetract { + op: op, + e: e, + a: a, + v: v, + } + })) }); def_parser!(Tx, map_notation, MapNotation, { diff --git a/tx-parser/tests/parser.rs b/tx-parser/tests/parser.rs index 67d60631..8602487d 100644 --- a/tx-parser/tests/parser.rs +++ b/tx-parser/tests/parser.rs @@ -85,4 +85,22 @@ fn test_entities() { ]); } +#[test] +fn test_reverse_notation_illegal_nested_values() { + // Verify that we refuse to parse direct reverse notation with nested value maps or vectors. + + let input = "[[:db/add 100 :test/_dangling {:test/many 13}]]"; + let edn = parse::value(input).expect("to parse test input"); + let result = Tx::parse(&edn); + // TODO: it would be much better to assert details about the error (here and below), but right + // now the error message isn't clear that the given value isn't valid for the backward attribute + // :test/_dangling. + assert!(result.is_err()); + + let input = "[[:db/add 100 :test/_dangling [:test/many 13]]]"; + let edn = parse::value(input).expect("to parse test input"); + let result = Tx::parse(&edn); + assert!(result.is_err()); +} + // TODO: test error handling in select cases.