Fix JSON Patch array add and array insertion (#4784)

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"
 	}

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)
 	}

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.
+		// Create a new object for testing and patching
-		// If test operations fails, nothing in value will be changed.
+		let mut new = self.clone();
-		let mut tmp_val = 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),
+					// Split the last path part from the path
-					_ => tmp_val.put(&path, value),
+					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::<String>();
-							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);
+					let val = new.pick(&from);
-					tmp_val.put(&path, found_val);
+					new.put(&path, val);
 				}
+				// Move a value from one field to another
 				Operation::Move {
 					path,
 					from,
 				} => {
-					let found_val = tmp_val.pick(&from);
+					let val = new.pick(&from);
-					tmp_val.put(&path, found_val);
+					new.put(&path, val);
-					tmp_val.cut(&from);
+					new.cut(&from);
 				}
+				// Test whether a value matches another value
 				Operation::Test {
 					path,
 					value,
 				} => {
-					let found_val = tmp_val.pick(&path);
+					let val = new.pick(&path);
-
+					if value != val {
-					if value != found_val {
 						return Err(Error::PatchTest {
 							expected: value.to_string(),
-							got: found_val.to_string(),
+							got: val.to_string(),
 						});
 					}
 				}
 			}
 		}
-
+		// Set the document to the updated document
-		*self = tmp_val;
+		*self = new;
+		// Everything ok
 		Ok(())
 	}
 }

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
 //