From 7f39754ec21a70a04e123d3972c298896f7b5cb4 Mon Sep 17 00:00:00 2001 From: Tobie Morgan Hitchcock Date: Mon, 22 Jan 2024 20:48:35 +0000 Subject: [PATCH] Fix: Ensure path idioms are correct when looping over (#3363) --- lib/src/sql/idiom.rs | 6 +++ lib/src/sql/value/every.rs | 89 +++++++++++++++++++++++++------------- lib/tests/field.rs | 70 ++++++++++++++++++++++++++++++ 3 files changed, 135 insertions(+), 30 deletions(-) diff --git a/lib/src/sql/idiom.rs b/lib/src/sql/idiom.rs index 40921600..aa40210c 100644 --- a/lib/src/sql/idiom.rs +++ b/lib/src/sql/idiom.rs @@ -130,6 +130,12 @@ impl Idiom { pub(crate) fn split_multi_yield(v: &Part) -> bool { matches!(v, Part::Graph(g) if g.alias.is_some()) } + /// Check if the path part is a yield in a multi-yield expression + pub(crate) fn remove_trailing_all(&mut self) { + if self.ends_with(&[Part::All]) { + self.0.truncate(self.len() - 1); + } + } } impl Idiom { diff --git a/lib/src/sql/value/every.rs b/lib/src/sql/value/every.rs index e939ddfb..b66c162b 100644 --- a/lib/src/sql/value/every.rs +++ b/lib/src/sql/value/every.rs @@ -9,39 +9,49 @@ impl Value { None => self._every(steps, arrays, Idiom::default()), } } - fn _every(&self, steps: bool, arrays: bool, prev: Idiom) -> Vec { + fn _every(&self, steps: bool, arrays: bool, mut prev: Idiom) -> Vec { match self { // Current path part is an object and is not empty - Value::Object(v) if !v.is_empty() => match steps { - // Let's log all intermediary nodes - true if !prev.is_empty() => Some(prev.clone()) - .into_iter() - .chain(v.iter().flat_map(|(k, v)| { - let p = Part::from(k.to_owned()); - v._every(steps, arrays, prev.clone().push(p)) - })) - .collect::>(), - // Let's not log intermediary nodes - _ => v - .iter() - .flat_map(|(k, v)| { - let p = Part::from(k.to_owned()); - v._every(steps, arrays, prev.clone().push(p)) - }) - .collect::>(), - }, + Value::Object(v) if !v.is_empty() => { + // Remove any trailing * path parts + prev.remove_trailing_all(); + // Check if we should log intermediary nodes + match steps { + // Let's log all intermediary nodes + true if !prev.is_empty() => Some(prev.clone()) + .into_iter() + .chain(v.iter().flat_map(|(k, v)| { + let p = Part::from(k.to_owned()); + v._every(steps, arrays, prev.clone().push(p)) + })) + .collect::>(), + // Let's not log intermediary nodes + _ => v + .iter() + .flat_map(|(k, v)| { + let p = Part::from(k.to_owned()); + v._every(steps, arrays, prev.clone().push(p)) + }) + .collect::>(), + } + } // Current path part is an array and is not empty - Value::Array(v) if !v.is_empty() => match arrays { - // Let's log all individual array items - true => std::iter::once(prev.clone()) - .chain(v.iter().enumerate().rev().flat_map(|(i, v)| { - let p = Part::from(i.to_owned()); - v._every(steps, arrays, prev.clone().push(p)) - })) - .collect::>(), - // Let's not log individual array items - false => vec![prev], - }, + Value::Array(v) if !v.is_empty() => { + // Remove any trailing * path parts + prev.remove_trailing_all(); + // Check if we should log individual array items + match arrays { + // Let's log all individual array items + true => std::iter::once(prev.clone()) + .chain(v.iter().enumerate().rev().flat_map(|(i, v)| { + let p = Part::from(i.to_owned()); + v._every(steps, arrays, prev.clone().push(p)) + })) + .collect::>(), + // Let's not log individual array items + false => vec![prev], + } + } // Process everything else _ => vec![prev], } @@ -117,4 +127,23 @@ mod tests { ]; assert_eq!(res, val.every(None, true, true)); } + + #[test] + fn every_including_intermediary_nodes_including_array_indexes_ending_all() { + let val = Value::parse("{ test: { something: [{ age: 34, tags: ['code', 'databases'] }, { age: 36, tags: ['design', 'operations'] }] } }"); + let res = vec![ + Idiom::parse("test.something"), + Idiom::parse("test.something[1]"), + Idiom::parse("test.something[1].age"), + Idiom::parse("test.something[1].tags"), + Idiom::parse("test.something[1].tags[1]"), + Idiom::parse("test.something[1].tags[0]"), + Idiom::parse("test.something[0]"), + Idiom::parse("test.something[0].age"), + Idiom::parse("test.something[0].tags"), + Idiom::parse("test.something[0].tags[1]"), + Idiom::parse("test.something[0].tags[0]"), + ]; + assert_eq!(res, val.every(Some(&Idiom::parse("test.something.*")), true, true)); + } } diff --git a/lib/tests/field.rs b/lib/tests/field.rs index 25631964..1c45b000 100644 --- a/lib/tests/field.rs +++ b/lib/tests/field.rs @@ -920,3 +920,73 @@ async fn field_definition_readonly() -> Result<(), Error> { // Ok(()) } + +#[tokio::test] +async fn field_definition_flexible_array_any() -> Result<(), Error> { + let sql = " + DEFINE TABLE user SCHEMAFULL; + DEFINE FIELD custom ON user TYPE option; + DEFINE FIELD custom.* ON user FLEXIBLE TYPE any; + CREATE user:one CONTENT { custom: ['sometext'] }; + CREATE user:two CONTENT { custom: [ ['sometext'] ] }; + CREATE user:three CONTENT { custom: [ { key: 'sometext' } ] }; + "; + let dbs = new_ds().await?.with_auth_enabled(true); + let ses = Session::owner().with_ns("test").with_db("test"); + let res = &mut dbs.execute(sql, &ses, None).await?; + assert_eq!(res.len(), 6); + // + let tmp = res.remove(0).result; + assert!(tmp.is_ok()); + // + let tmp = res.remove(0).result; + assert!(tmp.is_ok()); + // + let tmp = res.remove(0).result; + assert!(tmp.is_ok()); + // + let tmp = res.remove(0).result?; + let val = Value::parse( + "[ + { + custom: [ + 'sometext' + ], + id: user:one + }, + ]", + ); + assert_eq!(tmp, val); + // + let tmp = res.remove(0).result?; + let val = Value::parse( + "[ + { + custom: [ + [ + 'sometext' + ] + ], + id: user:two + }, + ]", + ); + assert_eq!(tmp, val); + // + let tmp = res.remove(0).result?; + let val = Value::parse( + "[ + { + custom: [ + { + key: 'sometext' + } + ], + id: user:three + } + ]", + ); + assert_eq!(tmp, val); + // + Ok(()) +}