From ebb6598c9e44209b586f8490748e14ab39390b21 Mon Sep 17 00:00:00 2001 From: Tobie Morgan Hitchcock Date: Tue, 17 Sep 2024 02:36:39 +0100 Subject: [PATCH] Fix JSON Patch array add and array insertion (#4784) --- core/src/sql/ident.rs | 4 ++ core/src/sql/idiom.rs | 2 +- core/src/sql/value/patch.rs | 85 +++++++++++++++++++------- sdk/tests/update.rs | 117 ++++++++++++++++++++++++++++++++++++ 4 files changed, 184 insertions(+), 24 deletions(-) diff --git a/core/src/sql/ident.rs b/core/src/sql/ident.rs index 01ba4429..82c45f1f 100644 --- a/core/src/sql/ident.rs +++ b/core/src/sql/ident.rs @@ -37,6 +37,10 @@ impl Ident { self.0.to_string() } /// Checks if this field is the `id` field + pub(crate) fn is_dash(&self) -> bool { + self.0.as_str() == "-" + } + /// Checks if this field is the `id` field pub(crate) fn is_id(&self) -> bool { self.0.as_str() == "id" } diff --git a/core/src/sql/idiom.rs b/core/src/sql/idiom.rs index 0a6489e8..b1e43b21 100644 --- a/core/src/sql/idiom.rs +++ b/core/src/sql/idiom.rs @@ -151,7 +151,7 @@ impl Idiom { self.0.truncate(self.len() - 1); } } - + /// Check if this Idiom starts with a specific path part pub(crate) fn starts_with(&self, other: &[Part]) -> bool { self.0.starts_with(other) } diff --git a/core/src/sql/value/patch.rs b/core/src/sql/value/patch.rs index 507736e4..1ff92a8d 100644 --- a/core/src/sql/value/patch.rs +++ b/core/src/sql/value/patch.rs @@ -1,35 +1,71 @@ use crate::err::Error; use crate::sql::operation::Operation; +use crate::sql::part::Part; use crate::sql::value::Value; impl Value { pub(crate) fn patch(&mut self, ops: Value) -> Result<(), Error> { - // This value is for test operation, value itself shouldn't change until all operations done. - // If test operations fails, nothing in value will be changed. - let mut tmp_val = self.clone(); - + // Create a new object for testing and patching + let mut new = self.clone(); + // Loop over the patch operations and apply them for operation in ops.to_operations()?.into_iter() { match operation { + // Add a value Operation::Add { path, value, - } => match tmp_val.pick(&path) { - Value::Array(_) => tmp_val.inc(&path, value), - _ => tmp_val.put(&path, value), - }, + } => { + // Split the last path part from the path + match path.split_last() { + // Check what the last path part is + Some((last, left)) => match last { + Part::Index(i) => match new.pick(left) { + Value::Array(mut v) => match v.len() > i.clone().as_usize() { + true => { + v.insert(i.clone().as_usize(), value); + new.put(left, Value::Array(v)); + } + false => { + v.push(value); + new.put(left, Value::Array(v)); + } + }, + _ => new.put(left, value), + }, + Part::Field(v) if v.is_dash() => match new.pick(left) { + Value::Array(mut v) => { + v.push(value); + new.put(left, Value::Array(v)); + } + _ => new.put(left, value), + }, + _ => match new.pick(&path) { + Value::Array(_) => new.inc(&path, value), + _ => new.put(&path, value), + }, + }, + None => match new.pick(&path) { + Value::Array(_) => new.inc(&path, value), + _ => new.put(&path, value), + }, + } + } + // Remove a value at the specified path Operation::Remove { path, - } => tmp_val.cut(&path), + } => new.cut(&path), + // Replace a value at the specified path Operation::Replace { path, value, - } => tmp_val.put(&path, value), + } => new.put(&path, value), + // Modify a string at the specified path Operation::Change { path, value, } => { if let Value::Strand(p) = value { - if let Value::Strand(v) = tmp_val.pick(&path) { + if let Value::Strand(v) = new.pick(&path) { let dmp = dmp::new(); let pch = dmp.patch_from_text(p.as_string()).map_err(|e| { Error::InvalidPatch { @@ -42,42 +78,45 @@ impl Value { } })?; let txt = txt.into_iter().collect::(); - tmp_val.put(&path, Value::from(txt)); + new.put(&path, Value::from(txt)); } } } + // Copy a value from one field to another Operation::Copy { path, from, } => { - let found_val = tmp_val.pick(&from); - tmp_val.put(&path, found_val); + let val = new.pick(&from); + new.put(&path, val); } + // Move a value from one field to another Operation::Move { path, from, } => { - let found_val = tmp_val.pick(&from); - tmp_val.put(&path, found_val); - tmp_val.cut(&from); + let val = new.pick(&from); + new.put(&path, val); + new.cut(&from); } + // Test whether a value matches another value Operation::Test { path, value, } => { - let found_val = tmp_val.pick(&path); - - if value != found_val { + let val = new.pick(&path); + if value != val { return Err(Error::PatchTest { expected: value.to_string(), - got: found_val.to_string(), + got: val.to_string(), }); } } } } - - *self = tmp_val; + // Set the document to the updated document + *self = new; + // Everything ok Ok(()) } } diff --git a/sdk/tests/update.rs b/sdk/tests/update.rs index e396e218..f5888662 100644 --- a/sdk/tests/update.rs +++ b/sdk/tests/update.rs @@ -322,6 +322,123 @@ async fn update_with_object_array_string_field_names() -> Result<(), Error> { Ok(()) } +#[tokio::test] +async fn update_records_and_arrays_with_json_patch() -> Result<(), Error> { + let sql = " + UPSERT person:test CONTENT { + username: 'parsley', + bugs: [], + biscuits: [ + { name: 'Digestive' }, + { name: 'Choco Leibniz' } + ] + }; + UPDATE person:test PATCH [ + { + op: 'add', + path: '/bugs', + value: 'rfc6902' + }, + { + op: 'add', + path: '/biscuits/0', + value: { name: 'Ginger Nut' } + }, + { + op: 'add', + path: '/test', + value: true, + } + ]; + UPSERT person:test PATCH [ + { + op: 'add', + path: '/bugs/-', + value: 'rfc6903' + } + ]; + "; + let dbs = new_ds().await?; + let ses = Session::owner().with_ns("test").with_db("test"); + let res = &mut dbs.execute(sql, &ses, None).await?; + assert_eq!(res.len(), 3); + // + let tmp = res.remove(0).result?; + let val = Value::parse( + "[ + { + biscuits: [ + { + name: 'Digestive' + }, + { + name: 'Choco Leibniz' + } + ], + bugs: [], + id: person:test, + username: 'parsley' + } + ]", + ); + assert_eq!(tmp, val); + // + let tmp = res.remove(0).result?; + let val = Value::parse( + "[ + { + biscuits: [ + { + name: 'Ginger Nut' + }, + { + name: 'Digestive' + }, + { + name: 'Choco Leibniz' + } + ], + bugs: [ + 'rfc6902' + ], + id: person:test, + test: true, + username: 'parsley' + } + ]", + ); + assert_eq!(tmp, val); + // + let tmp = res.remove(0).result?; + let val = Value::parse( + "[ + { + biscuits: [ + { + name: 'Ginger Nut' + }, + { + name: 'Digestive' + }, + { + name: 'Choco Leibniz' + } + ], + bugs: [ + 'rfc6902', + 'rfc6903' + ], + id: person:test, + test: true, + username: 'parsley' + } + ]", + ); + assert_eq!(tmp, val); + // + Ok(()) +} + // // Permissions //