From 9f06d96bace017976ced626f156a5837bf0f2fb9 Mon Sep 17 00:00:00 2001 From: stefan-gorules <127550877+stefan-gorules@users.noreply.github.com> Date: Tue, 25 Apr 2023 21:13:18 +0200 Subject: [PATCH] refactor: remove exec result; (#25) --- core/engine/src/handler/table/zen.rs | 7 +- core/expression/src/isolate.rs | 16 +-- core/expression/src/opcodes.rs | 133 ++----------------- core/expression/tests/isolate.rs | 186 +++++++++++++-------------- 4 files changed, 110 insertions(+), 232 deletions(-) diff --git a/core/engine/src/handler/table/zen.rs b/core/engine/src/handler/table/zen.rs index eee068bd..d3e32351 100644 --- a/core/engine/src/handler/table/zen.rs +++ b/core/engine/src/handler/table/zen.rs @@ -109,7 +109,7 @@ impl<'a> DecisionTableHandler<'a> { return None; } - let is_ok = result.unwrap().bool().unwrap_or_else(|_| false); + let is_ok = result.unwrap().as_bool().unwrap_or(false); if !is_ok { return None; } @@ -127,10 +127,7 @@ impl<'a> DecisionTableHandler<'a> { return None; } - outputs.insert( - output.field.clone(), - RowOutputKind::Value(res.unwrap().to_value().ok()?), - ); + outputs.insert(output.field.clone(), RowOutputKind::Value(res.unwrap())); } if !self.trace { diff --git a/core/expression/src/isolate.rs b/core/expression/src/isolate.rs index e7a6a960..9a117374 100644 --- a/core/expression/src/isolate.rs +++ b/core/expression/src/isolate.rs @@ -16,7 +16,7 @@ use crate::parser::error::ParserError; use crate::parser::{StandardParser, UnaryParser}; use crate::compiler::{Compiler, CompilerError}; -use crate::opcodes::{ExecResult, Opcode, Variable}; +use crate::opcodes::{Opcode, Variable}; use crate::vm::{Scope, VMError, VM}; type ADefHasher = BuildHasherDefault; @@ -76,7 +76,7 @@ impl<'a> Isolate<'a> { *env = ManuallyDrop::new(new_env); } - pub fn run(&self, source: &'a str) -> Result { + pub fn run(&self, source: &'a str) -> Result { if self.contains_ref(source) { self.run_standard(source) } else { @@ -98,9 +98,7 @@ impl<'a> Isolate<'a> { if !references.contains_key(reference) { let result = self.run_standard(reference)?; - let value = result - .to_variable(bump) - .map_err(|_| IsolateError::ValueCastError)?; + let value = bump.alloc(Variable::from_serde(&result, bump)); references.insert(reference, value); } @@ -127,7 +125,7 @@ impl<'a> Isolate<'a> { Ok(()) } - pub fn run_standard(&self, source: &'a str) -> Result { + pub fn run_standard(&self, source: &'a str) -> Result { self.clear(); let tokens = self @@ -163,10 +161,10 @@ impl<'a> Isolate<'a> { .run(unsafe { &*self.environment.get() }) .map_err(|source| IsolateError::VMError { source })?; - ExecResult::try_from(res).map_err(|_| IsolateError::ValueCastError) + res.try_into().map_err(|_| IsolateError::ValueCastError) } - pub fn run_unary(&self, source: &'a str) -> Result { + pub fn run_unary(&self, source: &'a str) -> Result { self.clear(); let tokens = self @@ -202,7 +200,7 @@ impl<'a> Isolate<'a> { .run(unsafe { &*self.environment.get() }) .map_err(|source| IsolateError::VMError { source })?; - ExecResult::try_from(res).map_err(|_| IsolateError::ValueCastError) + res.try_into().map_err(|_| IsolateError::ValueCastError) } fn clear(&self) { diff --git a/core/expression/src/opcodes.rs b/core/expression/src/opcodes.rs index 882e3b91..d80ef7cd 100644 --- a/core/expression/src/opcodes.rs +++ b/core/expression/src/opcodes.rs @@ -1,5 +1,3 @@ -use std::collections::HashMap; -use std::fmt::{Display, Formatter}; use std::str::FromStr; use bumpalo::Bump; @@ -26,12 +24,6 @@ pub enum Variable<'a> { }, } -impl<'a> Display for Variable<'a> { - fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - write!(f, "{self:?}") - } -} - impl<'a> Variable<'a> { pub fn empty_object_in(bump: &'a Bump) -> Self { Variable::Object(hashbrown::HashMap::new_in(BumpWrapper(bump))) @@ -130,136 +122,35 @@ pub enum Opcode<'a> { End, } -impl<'a> Display for Opcode<'a> { - fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - write!(f, "{:?}", self) - } -} - -#[derive(Debug, PartialEq, Clone)] -pub enum ExecResult { - Null, - Bool(bool), - Number(Decimal), - String(String), - Array(Vec), - Object(HashMap), -} - -impl From<&Value> for ExecResult { - fn from(value: &Value) -> Self { - match value { - Value::Null => ExecResult::Null, - Value::Number(num) => { - ExecResult::Number(Decimal::from_str(num.to_string().as_str()).unwrap()) - } - Value::String(str) => ExecResult::String(str.clone()), - Value::Bool(b) => ExecResult::Bool(*b), - Value::Array(arr) => ExecResult::Array(arr.iter().map(ExecResult::from).collect()), - Value::Object(map) => { - let remapped = map - .iter() - .map(|(key, val)| (key.clone(), ExecResult::from(val))) - .collect::>(); - - ExecResult::Object(remapped) - } - } - } -} - -impl TryFrom<&Variable<'_>> for ExecResult { +impl TryFrom<&Variable<'_>> for Value { type Error = (); - fn try_from(value: &Variable) -> Result { + fn try_from(value: &Variable<'_>) -> Result { match value { - Variable::Null => Ok(ExecResult::Null), - Variable::Bool(b) => Ok(ExecResult::Bool(*b)), - Variable::Number(n) => Ok(ExecResult::Number(*n)), - Variable::String(s) => Ok(ExecResult::String(s.to_string())), - Variable::Array(arr) => { - let mut v = Vec::::with_capacity(arr.len()); - for i in *arr { - v.push(ExecResult::try_from(*i)?) - } - - Ok(ExecResult::Array(v)) - } - Variable::Object(obj) => { - let mut t = HashMap::new(); - - for k in obj.keys() { - let v = *obj.get(k).ok_or(())?; - t.insert(k.to_string(), ExecResult::try_from(v)?); - } - - Ok(ExecResult::Object(t)) - } - _ => Err(()), - } - } -} - -impl ExecResult { - pub fn to_variable<'a>(&self, bump: &'a Bump) -> Result<&'a Variable<'a>, ()> { - match self { - ExecResult::Null => Ok(bump.alloc(Variable::Null)), - ExecResult::Bool(b) => Ok(bump.alloc(Variable::Bool(*b))), - ExecResult::Number(n) => Ok(bump.alloc(Variable::Number(*n))), - ExecResult::String(str) => Ok(bump.alloc(Variable::String(bump.alloc_str(str)))), - ExecResult::Array(arr) => { - let mut v = Vec::<&'a Variable<'a>>::with_capacity(arr.len()); - for i in arr { - v.push(i.to_variable(bump)?) - } - - Ok(bump.alloc(Variable::Array(bump.alloc_slice_copy(v.as_slice())))) - } - ExecResult::Object(obj) => { - let mut t = hashbrown::HashMap::<&'a str, _, _, _>::new_in(BumpWrapper(bump)); - - for k in obj.keys() { - let v = obj.get(k).ok_or(())?; - t.insert(bump.alloc_str(k), v.to_variable(bump)?); - } - - Ok(bump.alloc(Variable::Object(t))) - } - } - } - - pub fn to_value(&self) -> Result { - match self { - ExecResult::Null => Ok(Value::Null), - ExecResult::Bool(b) => Ok(Value::Bool(*b)), - ExecResult::Number(n) => Ok(Value::Number( + Variable::Null => Ok(Value::Null), + Variable::Bool(b) => Ok(Value::Bool(*b)), + Variable::Number(n) => Ok(Value::Number( Number::from_str(n.to_string().as_str()).map_err(|_| ())?, )), - ExecResult::String(s) => Ok(Value::String(s.clone())), - ExecResult::Array(arr) => { + Variable::String(s) => Ok(Value::String(s.to_string())), + Variable::Array(arr) => { let mut v = Vec::::with_capacity(arr.len()); - for i in arr { - v.push(i.to_value()?) + for i in *arr { + v.push(Value::try_from(*i)?) } Ok(Value::Array(v)) } - ExecResult::Object(obj) => { + Variable::Object(obj) => { let mut t = Map::new(); for k in obj.keys() { - let v = obj.get(k).ok_or(())?; - t.insert(k.to_string(), v.to_value()?); + let v = *obj.get(k).ok_or(())?; + t.insert(k.to_string(), Value::try_from(v)?); } Ok(Value::Object(t)) } - } - } - - pub fn bool(&self) -> Result { - match self { - ExecResult::Bool(b) => Ok(*b), _ => Err(()), } } diff --git a/core/expression/tests/isolate.rs b/core/expression/tests/isolate.rs index 64e3e31e..2cdb448f 100644 --- a/core/expression/tests/isolate.rs +++ b/core/expression/tests/isolate.rs @@ -1,9 +1,9 @@ use bumpalo::Bump; -use rust_decimal_macros::dec; + use serde_json::{json, Value}; use zen_expression::isolate::Isolate; -use zen_expression::opcodes::{ExecResult, Variable}; +use zen_expression::opcodes::Variable; struct TestEnv { env: Value, @@ -12,7 +12,7 @@ struct TestEnv { struct TestCase { expr: &'static str, - result: ExecResult, + result: Value, } #[test] @@ -25,7 +25,7 @@ fn isolate_standard_test() { }), cases: Vec::from([TestCase { expr: "hello + world", - result: ExecResult::String("Hello, world!".to_string()), + result: json!("Hello, world!"), }]), }, TestEnv { @@ -37,19 +37,19 @@ fn isolate_standard_test() { cases: Vec::from([ TestCase { expr: "a + b - c", - result: ExecResult::Number(dec!(8.0)), + result: json!(8), }, TestCase { expr: "b^a", - result: ExecResult::Number(dec!(216.0)), + result: json!(216), }, TestCase { expr: "c * b / a", - result: ExecResult::Number(dec!(2.0)), + result: json!(2), }, TestCase { expr: "abs(a - b - c)", - result: ExecResult::Number(dec!(4.0)), + result: json!(4), }, ]), }, @@ -64,23 +64,23 @@ fn isolate_standard_test() { cases: Vec::from([ TestCase { expr: "a == a and a != b", - result: ExecResult::Bool(true), + result: json!(true), }, TestCase { expr: "b - a > c", - result: ExecResult::Bool(true), + result: json!(true), }, TestCase { expr: "b < a or a > b", - result: ExecResult::Bool(false), + result: json!(false), }, TestCase { expr: "t or f", - result: ExecResult::Bool(true), + result: json!(true), }, TestCase { expr: "t and f", - result: ExecResult::Bool(false), + result: json!(false), }, ]), }, @@ -89,15 +89,15 @@ fn isolate_standard_test() { cases: Vec::from([ TestCase { expr: "1 in [1..5] and 5 in [1..5]", - result: ExecResult::Bool(true), + result: json!(true), }, TestCase { expr: "1 not in (1..5] and 5 not in [1..5)", - result: ExecResult::Bool(true), + result: json!(true), }, TestCase { expr: "1 not in [1.01..5] and 5 not in [1..4.99]", - result: ExecResult::Bool(true), + result: json!(true), }, ]), }, @@ -106,15 +106,15 @@ fn isolate_standard_test() { cases: Vec::from([ TestCase { expr: r#"date("2022-04-04") > date("2022-03-04")"#, - result: ExecResult::Bool(true), + result: json!(true), }, TestCase { expr: r#"duration("60m") == duration("1h")"#, - result: ExecResult::Bool(true), + result: json!(true), }, TestCase { expr: r#"duration("24h") >= duration("1d")"#, - result: ExecResult::Bool(true), + result: json!(true), }, ]), }, @@ -123,27 +123,27 @@ fn isolate_standard_test() { cases: Vec::from([ TestCase { expr: r#"customer.firstName + " " + customer.lastName"#, - result: ExecResult::String("John Doe".to_string()), + result: json!("John Doe"), }, TestCase { expr: r#"startsWith(customer.firstName, "Jo")"#, - result: ExecResult::Bool(true), + result: json!(true), }, TestCase { expr: r#"endsWith(customer.firstName + customer.lastName, "oe")"#, - result: ExecResult::Bool(true), + result: json!(true), }, TestCase { expr: r#"contains(customer.lastName, "Do")"#, - result: ExecResult::Bool(true), + result: json!(true), }, TestCase { expr: "upper(customer.firstName) == 'JOHN'", - result: ExecResult::Bool(true), + result: json!(true), }, TestCase { expr: "lower(customer.firstName) == 'john'", - result: ExecResult::Bool(true), + result: json!(true), }, ]), }, @@ -157,58 +157,55 @@ fn isolate_standard_test() { cases: Vec::from([ TestCase { expr: r#"some(customer.groups, # == "admin")"#, - result: ExecResult::Bool(true), + result: json!(true), }, TestCase { expr: r#"all(customer.purchaseAmounts, # in [100..800])"#, - result: ExecResult::Bool(true), + result: json!(true), }, TestCase { expr: r#"not all(customer.purchaseAmounts, # in (100..800))"#, - result: ExecResult::Bool(true), + result: json!(true), }, TestCase { expr: r#"none(customer.purchaseAmounts, # == 99)"#, - result: ExecResult::Bool(true), + result: json!(true), }, TestCase { expr: r#"all(customer.groups, # == "admin" or # == "user")"#, - result: ExecResult::Bool(true), + result: json!(true), }, TestCase { expr: "count(customer.groups, true)", - result: ExecResult::Number(dec!(2.0)), + result: json!(2), }, TestCase { expr: "count(customer.purchaseAmounts, # > 150)", - result: ExecResult::Number(dec!(3.0)), + result: json!(3), }, TestCase { expr: "map(customer.purchaseAmounts, # + 50)[0] == 150", - result: ExecResult::Bool(true), + result: json!(true), }, TestCase { expr: "filter(customer.purchaseAmounts, # >= 200)[0] == 200", - result: ExecResult::Bool(true), + result: json!(true), }, TestCase { expr: "sum(customer.purchaseAmounts[0:1]) == 300", - result: ExecResult::Bool(true), + result: json!(true), }, TestCase { expr: "one(customer.groups, # == 'admin')", - result: ExecResult::Bool(true), + result: json!(true), }, TestCase { expr: "one(['admin', 'admin'], # == 'admin')", - result: ExecResult::Bool(false), + result: json!(false), }, TestCase { expr: r#"map(["admin", "user"], "hello " + #)"#, - result: ExecResult::Array(Vec::from([ - ExecResult::String("hello admin".to_string()), - ExecResult::String("hello user".to_string()), - ])), + result: json!(["hello admin", "hello user"]), }, ]), }, @@ -221,31 +218,31 @@ fn isolate_standard_test() { cases: Vec::from([ TestCase { expr: r#"contains(name, 'ello')"#, - result: ExecResult::Bool(true), + result: json!(true), }, TestCase { expr: r#"contains(name, '123')"#, - result: ExecResult::Bool(false), + result: json!(false), }, TestCase { expr: r#"contains(groups, 'admin')"#, - result: ExecResult::Bool(true), + result: json!(true), }, TestCase { expr: r#"contains(groups, 'hello')"#, - result: ExecResult::Bool(false), + result: json!(false), }, TestCase { expr: r#"contains(purchaseAmounts, 100)"#, - result: ExecResult::Bool(true), + result: json!(true), }, TestCase { expr: "contains(purchaseAmounts, 150)", - result: ExecResult::Bool(false), + result: json!(false), }, TestCase { expr: "len(purchaseAmounts)", - result: ExecResult::Number(dec!(4.0)), + result: json!(4), }, ]), }, @@ -254,31 +251,31 @@ fn isolate_standard_test() { cases: Vec::from([ TestCase { expr: r#"dayOfWeek(date("2022-11-08"))"#, - result: ExecResult::Number(dec!(2.0)), + result: json!(2), }, TestCase { expr: r#"dayOfMonth(date("2022-11-09"))"#, - result: ExecResult::Number(dec!(9.0)), + result: json!(9), }, TestCase { expr: r#"dayOfYear(date("2022-11-10"))"#, - result: ExecResult::Number(dec!(314.0)), + result: json!(314), }, TestCase { expr: r#"weekOfYear(date("2022-11-12"))"#, - result: ExecResult::Number(dec!(45.0)), + result: json!(45), }, TestCase { expr: r#"monthString(date("2022-11-14"))"#, - result: ExecResult::String("Nov".to_string()), + result: json!("Nov"), }, TestCase { expr: r#"monthString("2022-11-14")"#, - result: ExecResult::String("Nov".to_string()), + result: json!("Nov"), }, TestCase { expr: r#"weekdayString(date("2022-11-14"))"#, - result: ExecResult::String("Mon".to_string()), + result: json!("Mon"), }, ]), }, @@ -287,43 +284,43 @@ fn isolate_standard_test() { cases: Vec::from([ TestCase { expr: r#"sum([1, 2, 3])"#, - result: ExecResult::Number(dec!(6.0)), + result: json!(6), }, TestCase { expr: r#"avg([1, 2, 3])"#, - result: ExecResult::Number(dec!(2.0)), + result: json!(2), }, TestCase { expr: r#"min([1, 2, 3])"#, - result: ExecResult::Number(dec!(1.0)), + result: json!(1), }, TestCase { expr: r#"max([1, 2, 3])"#, - result: ExecResult::Number(dec!(3.0)), + result: json!(3), }, TestCase { expr: r#"floor(3.5)"#, - result: ExecResult::Number(dec!(3.0)), + result: json!(3), }, TestCase { expr: r#"ceil(3.5)"#, - result: ExecResult::Number(dec!(4.0)), + result: json!(4), }, TestCase { expr: r#"round(4.7)"#, - result: ExecResult::Number(dec!(5.0)), + result: json!(5), }, TestCase { expr: r#"rand(10) <= 10"#, - result: ExecResult::Bool(true), + result: json!(true), }, TestCase { expr: r#"10 % 4"#, - result: ExecResult::Number(dec!(2.0)), + result: json!(2), }, TestCase { expr: r#"true ? 10.0 == 10 : 1.0"#, - result: ExecResult::Bool(true), + result: json!(true), }, ]), }, @@ -332,15 +329,15 @@ fn isolate_standard_test() { cases: Vec::from([ TestCase { expr: r#"223_000.48 - 120_000_00 / 100"#, - result: ExecResult::Number(dec!(103_000.48)), + result: json!(103_000.48), }, TestCase { expr: r#"9223372036854775807"#, - result: ExecResult::Number(dec!(9223372036854775807)), + result: json!(9223372036854775807i64), }, TestCase { expr: r#"-9223372036854775807"#, - result: ExecResult::Number(dec!(-9223372036854775807)), + result: json!(-9223372036854775807i64), }, ]), }, @@ -351,19 +348,19 @@ fn isolate_standard_test() { cases: Vec::from([ TestCase { expr: r#"numbers[0]"#, - result: ExecResult::from(&json!([1, 2, 3])), + result: json!([1, 2, 3]), }, TestCase { expr: r#"map(numbers, sum(#))"#, - result: ExecResult::from(&json!([6, 15, 24])), + result: json!([6, 15, 24]), }, TestCase { expr: r#"map(numbers, map(#, # - 1))"#, - result: ExecResult::from(&json!([[0, 1, 2], [3, 4, 5], [6, 7, 8]])), + result: json!([[0, 1, 2], [3, 4, 5], [6, 7, 8]]), }, TestCase { expr: r#"filter(numbers, some(#, # < 5))"#, - result: ExecResult::from(&json!([[1, 2, 3], [4, 5, 6]])), + result: json!([[1, 2, 3], [4, 5, 6]]), }, ]), }, @@ -374,19 +371,19 @@ fn isolate_standard_test() { cases: Vec::from([ TestCase { expr: r#"numbers[0]"#, - result: ExecResult::from(&json!([1, 2, 3])), + result: json!([1, 2, 3]), }, TestCase { expr: r#"map(numbers, sum(#))"#, - result: ExecResult::from(&json!([6, 15, 24])), + result: json!([6, 15, 24]), }, TestCase { expr: r#"map(numbers, map(#, # - 1))"#, - result: ExecResult::from(&json!([[0, 1, 2], [3, 4, 5], [6, 7, 8]])), + result: json!([[0, 1, 2], [3, 4, 5], [6, 7, 8]]), }, TestCase { expr: r#"filter(numbers, some(#, # < 5))"#, - result: ExecResult::from(&json!([[1, 2, 3], [4, 5, 6]])), + result: json!([[1, 2, 3], [4, 5, 6]]), }, ]), }, @@ -401,18 +398,14 @@ fn isolate_standard_test() { cases: Vec::from([ TestCase { expr: r#"filter(cart, some(#.categories, #.categoryId == 'cat1'))"#, - result: ExecResult::from(&json!([ + result: json!([ { "id": "1", "categories": [{"categoryId": "cat1"}, {"categoryId": "cat2"}] }, { "id": "3", "categories": [{"categoryId": "cat1"}, {"categoryId": "cat5"}] } - ])), + ]), }, TestCase { expr: r#"map(cart, map(#.categories, #.categoryId))"#, - result: ExecResult::from(&json!([ - ["cat1", "cat2"], - ["cat3", "cat4"], - ["cat1", "cat5"], - ])), + result: json!([["cat1", "cat2"], ["cat3", "cat4"], ["cat1", "cat5"],]), }, ]), }, @@ -448,7 +441,7 @@ fn isolate_unary_tests() { reference: "customer.groups", cases: Vec::from([TestCase { expr: r#"some($, # == "admin")"#, - result: ExecResult::Bool(true), + result: json!(true), }]), }, UnaryTestEnv { @@ -462,47 +455,47 @@ fn isolate_unary_tests() { cases: Vec::from([ TestCase { expr: "300", - result: ExecResult::Bool(true), + result: json!(true), }, TestCase { expr: ")100..200(", - result: ExecResult::Bool(true), + result: json!(true), }, TestCase { expr: "in [100..300]", - result: ExecResult::Bool(true), + result: json!(true), }, TestCase { expr: "[100, 200, 300]", - result: ExecResult::Bool(true), + result: json!(true), }, TestCase { expr: "100, 200, 300", - result: ExecResult::Bool(true), + result: json!(true), }, TestCase { expr: "not in [250, 350]", - result: ExecResult::Bool(true), + result: json!(true), }, TestCase { expr: "> 250", - result: ExecResult::Bool(true), + result: json!(true), }, TestCase { expr: "< 350", - result: ExecResult::Bool(true), + result: json!(true), }, TestCase { expr: "== 300", - result: ExecResult::Bool(true), + result: json!(true), }, TestCase { expr: "!= 301", - result: ExecResult::Bool(true), + result: json!(true), }, TestCase { expr: ">= 300 and <= 300", - result: ExecResult::Bool(true), + result: json!(true), }, ]), }, @@ -541,7 +534,6 @@ fn variable_serde_test() { fn isolate_test_decimals() { let isolate = Isolate::default(); let result = isolate.run_standard("9223372036854775807").unwrap(); - let value = result.to_value().unwrap(); - assert_eq!(value, Value::from(9223372036854775807i64)); + assert_eq!(result, Value::from(9223372036854775807i64)); }