From 3f5ef432481f4506feb5dc818281fce6c0f1e400 Mon Sep 17 00:00:00 2001
From: Micha de Vries <micha@devrie.sh>
Date: Thu, 8 Aug 2024 17:11:39 +0100
Subject: [PATCH] Skip `ASSERT` for type `option<T>` and value `NONE` (#4483)

Co-authored-by: Tobie Morgan Hitchcock <tobie@surrealdb.com>
---
 core/src/doc/field.rs | 39 ++++++++++++++++++++-----------
 lib/tests/field.rs    | 54 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 79 insertions(+), 14 deletions(-)

diff --git a/core/src/doc/field.rs b/core/src/doc/field.rs
index 5c005a0a..41ef7836 100644
--- a/core/src/doc/field.rs
+++ b/core/src/doc/field.rs
@@ -6,6 +6,7 @@ use crate::err::Error;
 use crate::iam::Action;
 use crate::sql::permission::Permission;
 use crate::sql::value::Value;
+use crate::sql::Kind;
 use reblessive::tree::Stk;
 
 impl<'a> Document<'a> {
@@ -110,20 +111,30 @@ impl<'a> Document<'a> {
 				}
 				// Check for a ASSERT clause
 				if let Some(expr) = &fd.assert {
-					// Configure the context
-					let mut ctx = Context::new(ctx);
-					ctx.add_value("input", &inp);
-					ctx.add_value("value", &val);
-					ctx.add_value("after", &val);
-					ctx.add_value("before", &old);
-					// Process the ASSERT clause
-					if !expr.compute(stk, &ctx, opt, Some(&self.current)).await?.is_truthy() {
-						return Err(Error::FieldValue {
-							thing: rid.to_string(),
-							field: fd.name.clone(),
-							value: val.to_string(),
-							check: expr.to_string(),
-						});
+					match (&val, &fd.kind) {
+						// The field TYPE is optional, and the field
+						// value was not set or a NONE value was
+						// specified, so let's ignore the ASSERT clause
+						(Value::None, Some(Kind::Option(_))) => (),
+						// Otherwise let's process the ASSERT clause
+						_ => {
+							// Configure the context
+							let mut ctx = Context::new(ctx);
+							ctx.add_value("input", &inp);
+							ctx.add_value("value", &val);
+							ctx.add_value("after", &val);
+							ctx.add_value("before", &old);
+							// Process the ASSERT clause
+							if !expr.compute(stk, &ctx, opt, Some(&self.current)).await?.is_truthy()
+							{
+								return Err(Error::FieldValue {
+									thing: rid.to_string(),
+									field: fd.name.clone(),
+									value: val.to_string(),
+									check: expr.to_string(),
+								});
+							}
+						}
 					}
 				}
 				// Check for a PERMISSIONS clause
diff --git a/lib/tests/field.rs b/lib/tests/field.rs
index a9e9907d..a6d9a11f 100644
--- a/lib/tests/field.rs
+++ b/lib/tests/field.rs
@@ -138,6 +138,60 @@ async fn field_definition_value_assert_success() -> Result<(), Error> {
 	Ok(())
 }
 
+#[tokio::test]
+async fn field_definition_option_kind_assert() -> Result<(), Error> {
+	let sql = "
+		DEFINE TABLE person SCHEMAFULL;
+		DEFINE FIELD name ON TABLE person TYPE option<string> ASSERT string::len($value) > 3;
+		CREATE person:test;
+		CREATE person:mark SET name = 'mark';
+		CREATE person:bob SET name = 'bob';
+	";
+	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(), 5);
+	//
+	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(
+		"[
+			{
+				id: person:test
+			}
+		]",
+	);
+	assert_eq!(tmp, val);
+	//
+	let tmp = res.remove(0).result?;
+	let val = Value::parse(
+		"[
+			{
+				id: person:mark,
+				name: 'mark'
+			}
+		]",
+	);
+	assert_eq!(tmp, val);
+	//
+	let tmp = res.remove(0).result;
+	assert!(
+		matches!(
+			&tmp,
+			Err(e) if e.to_string() == "Found 'bob' for field `name`, with record `person:bob`, but field must conform to: string::len($value) > 3"
+		),
+		"{}",
+		tmp.unwrap_err().to_string()
+	);
+
+	Ok(())
+}
+
 #[tokio::test]
 async fn field_definition_empty_nested_objects() -> Result<(), Error> {
 	let sql = "