Clean up some type coercion and make Variables a real struct which ho…

…lds Values, rather than Strings.
fastly · Dec 4, 2024 · 94c25ea · 94c25ea
1 parent 2bfac00
commit 94c25ea
Show file tree

Hide file tree

Showing 3 changed files with 94 additions and 59 deletions.
diff --git a/esi/src/expression.rs b/esi/src/expression.rs
@@ -1,51 +1,41 @@
-use std::collections::HashMap;
 use std::iter::Peekable;
 use std::str::Chars;
 
-use crate::{ExecutionError, Result};
+use crate::{ExecutionError, Result, Value, Variables};
 
-pub fn evaluate_expression(raw_expr: String, ctx: EvalContext) -> Result<EvalResult> {
+pub fn evaluate_expression(raw_expr: String, ctx: EvalContext) -> Result<Value> {
     // TODO: this got real ugly, figure out some better way to do this
     let tokens = match lex_expr(raw_expr) {
         Ok(r) => r,
-        Err(ExecutionError::ExpressionParseError(s)) => return Ok(EvalResult::Error(s)),
+        Err(ExecutionError::ExpressionParseError(s)) => return Ok(Value::Error(s)),
         Err(e) => return Err(e),
     };
     let expr = match parse_expr(tokens) {
         Ok(r) => r,
-        Err(ExecutionError::ExpressionParseError(s)) => return Ok(EvalResult::Error(s)),
+        Err(ExecutionError::ExpressionParseError(s)) => return Ok(Value::Error(s)),
         Err(e) => return Err(e),
     };
     match eval_expr(expr, ctx) {
         Ok(r) => Ok(r),
-        Err(ExecutionError::ExpressionParseError(s)) => return Ok(EvalResult::Error(s)),
+        Err(ExecutionError::ExpressionParseError(s)) => return Ok(Value::Error(s)),
         Err(e) => return Err(e),
     }
 }
 
-#[derive(Debug, Clone, PartialEq)]
-pub enum EvalResult {
-    String(String),
-    Error(String),
-}
-
 pub struct EvalContext<'a> {
-    variables: &'a HashMap<String, String>,
+    variables: &'a Variables,
     // request context
 }
 impl EvalContext<'_> {
-    pub fn new(variables: &HashMap<String, String>) -> EvalContext {
+    pub fn new(variables: &Variables) -> EvalContext {
         EvalContext { variables }
     }
 }
 
-fn eval_expr(expr: Expr, ctx: EvalContext) -> Result<EvalResult> {
+fn eval_expr(expr: Expr, ctx: EvalContext) -> Result<Value> {
     let result = match expr {
-        Expr::String(s) => EvalResult::String(s),
-        Expr::Variable(s) => match ctx.variables.get(&s) {
-            Some(value) => EvalResult::String(value.to_owned()),
-            None => EvalResult::String("".to_string()),
-        },
+        Expr::String(s) => Value::String(s),
+        Expr::Variable(s) => ctx.variables.get(&s).clone(),
     };
     Ok(result)
 }
@@ -169,13 +159,8 @@ mod tests {
     #[test]
     fn test_eval_string() -> Result<()> {
         let expr = Expr::String("hello".to_string());
-        let result = eval_expr(
-            expr,
-            EvalContext {
-                variables: &HashMap::new(),
-            },
-        )?;
-        assert_eq!(result, EvalResult::String("hello".to_string()));
+        let result = eval_expr(expr, EvalContext::new(&Variables::new()))?;
+        assert_eq!(result, Value::String("hello".to_string()));
         Ok(())
     }
 
@@ -184,11 +169,12 @@ mod tests {
         let expr = Expr::Variable("hello".to_string());
         let result = eval_expr(
             expr,
-            EvalContext {
-                variables: &HashMap::from([("hello".to_string(), "goodbye".to_string())]),
-            },
+            EvalContext::new(&Variables::from([(
+                "hello".to_string(),
+                Value::String("goodbye".to_string()),
+            )])),
         )?;
-        assert_eq!(result, EvalResult::String("goodbye".to_string()));
+        assert_eq!(result, Value::String("goodbye".to_string()));
         Ok(())
     }
 
@@ -197,12 +183,12 @@ mod tests {
     fn test_evaluation_error() -> Result<()> {
         let result = evaluate_expression(
             "$hello".to_string(),
-            EvalContext::new(&HashMap::from([(
+            EvalContext::new(&Variables::from([(
                 "hello".to_string(),
-                "goodbye".to_string(),
+                Value::String("goodbye".to_string()),
             )])),
         )?;
-        assert_eq!(result, EvalResult::Error("$hello".to_string()));
+        assert_eq!(result, Value::Error("$hello".to_string()));
         Ok(())
     }
 }
diff --git a/esi/src/lib.rs b/esi/src/lib.rs
@@ -5,19 +5,21 @@ mod document;
 mod error;
 mod expression;
 mod parse;
+mod variables;
 
 use document::{FetchState, Task};
-use expression::{evaluate_expression, EvalContext, EvalResult};
+use expression::{evaluate_expression, EvalContext};
 use fastly::http::request::PendingRequest;
 use fastly::http::{header, Method, StatusCode, Url};
 use fastly::{mime, Body, Request, Response};
 use log::{debug, error, trace};
-use std::collections::{HashMap, VecDeque};
+use std::collections::VecDeque;
 use std::io::{BufRead, Write};
 
 pub use crate::document::{Element, Fragment};
 pub use crate::error::Result;
 pub use crate::parse::{parse_tags, Event, Include, Tag, Tag::Try};
+pub use crate::variables::{Value, Variables};
 
 pub use crate::config::Configuration;
 pub use crate::error::ExecutionError;
@@ -145,7 +147,7 @@ impl Processor {
         let root_task = &mut Task::new();
 
         // variables
-        let mut variables = HashMap::new();
+        let mut variables = Variables::new();
 
         let is_escaped = self.configuration.is_escaped_content;
         // Call the library to parse fn `parse_tags` which will call the callback function
@@ -399,7 +401,7 @@ fn event_receiver(
     is_escaped: bool,
     original_request_metadata: &Request,
     dispatch_fragment_request: &FragmentRequestDispatcher,
-    variables: &mut HashMap<String, String>,
+    variables: &mut Variables,
 ) -> Result<()> {
     debug!("got {:?}", event);
 
@@ -460,24 +462,14 @@ fn event_receiver(
         }
         Event::ESI(Tag::Assign { name, value }) => {
             let result = evaluate_expression(value, EvalContext::new(&variables))?;
-            match result {
-                EvalResult::String(s) => {
-                    variables.insert(name, s);
-                }
-                EvalResult::Error(e) => {
-                    println!("Error {} while parsing expression, ignoring.", e);
-                    // do nothing
-                }
-            }
+            variables.insert(name, result);
         }
         Event::ESI(Tag::Vars { name }) => {
             if let Some(name) = name {
-                if let Some(value) = variables.get(&name) {
-                    let value = value.to_owned();
-                    queue.push_back(Element::Raw(value.into_bytes()));
+                match variables.get(&name) {
+                    &Value::Null => {}
+                    v @ _ => queue.push_back(Element::Raw(v.to_string().into_bytes())),
                 }
-            } else {
-                // TODO: long form
             }
         }
         Event::VarsContent(event) => {
@@ -498,12 +490,10 @@ fn event_receiver(
                         }
 
                         match String::from_utf8(varbuf) {
-                            Ok(name) => {
-                                if let Some(value) = variables.get(&name) {
-                                    let value = value.to_owned();
-                                    queue.push_back(Element::Raw(value.into_bytes()));
-                                }
-                            }
+                            Ok(name) => match variables.get(&name) {
+                                &Value::Null => {}
+                                v @ _ => queue.push_back(Element::Raw(v.to_string().into_bytes())),
+                            },
                             Err(e) => println!("Failed to parse variable: {}", e),
                         }
                     }
@@ -531,7 +521,7 @@ fn task_handler(
     is_escaped: bool,
     original_request_metadata: &Request,
     dispatch_fragment_request: &FragmentRequestDispatcher,
-    variables: &mut HashMap<String, String>,
+    variables: &mut Variables,
 ) -> Result<Task> {
     let mut task = Task::new();
     for event in events {

diff --git a/esi/src/variables.rs b/esi/src/variables.rs
@@ -0,0 +1,59 @@
+use std::collections::HashMap;
+
+#[derive(Debug, Clone, PartialEq)]
+pub enum Value {
+    String(String),
+    Error(String),
+    Null,
+}
+
+impl Value {
+    pub fn to_bool(&self) -> bool {
+        match self {
+            Value::String(_) => true,
+            Value::Error(_) => false,
+            Value::Null => false,
+        }
+    }
+
+    pub fn to_string(&self) -> String {
+        match self {
+            Value::String(s) => s.clone(),
+            Value::Error(_) => "".to_string(),
+            Value::Null => "".to_string(),
+        }
+    }
+}
+
+pub struct Variables {
+    map: HashMap<String, Value>,
+}
+
+impl Variables {
+    pub fn new() -> Variables {
+        Variables {
+            map: HashMap::new(),
+        }
+    }
+
+    pub fn insert(&mut self, name: String, value: Value) {
+        match value {
+            Value::Null => {}
+            _ => {
+                self.map.insert(name, value);
+            }
+        };
+    }
+
+    pub fn get(&self, name: &str) -> &Value {
+        self.map.get(name).unwrap_or(&Value::Null)
+    }
+}
+
+impl<const N: usize> From<[(String, Value); N]> for Variables {
+    fn from(data: [(String, Value); N]) -> Variables {
+        Variables {
+            map: HashMap::from(data),
+        }
+    }
+}