From ea797428209fb77fa2b5e45c68955d18149667aa Mon Sep 17 00:00:00 2001
From: Lucas Pickering <lucas@lucaspickering.me>
Date: Wed, 25 Oct 2023 07:23:12 -0400
Subject: [PATCH] Pre-parse JSONPath in collection

---
 docs/src/user_guide/templates.md |  2 +-
 src/config/mod.rs                |  4 ++--
 src/template.rs                  | 37 ++++++--------------------------
 3 files changed, 9 insertions(+), 34 deletions(-)

diff --git a/docs/src/user_guide/templates.md b/docs/src/user_guide/templates.md
index 2472fa57..0727428f 100644
--- a/docs/src/user_guide/templates.md
+++ b/docs/src/user_guide/templates.md
@@ -2,7 +2,7 @@
 
 Templates enable dynamic string construction. Slumber's template language is relatively simple, compared to complex HTML templating languages like Handlebars or Jinja. The goal is to be intuitive and unsurprising. It doesn't support complex features like for loops, conditionals, etc.
 
-All string _values_ (i.e. _not_ keys) in a request collection are template strings, meaning they support templating. The syntax for templating a value into a string is double curly braces (`{{...}}`). The contents inside the braces tell Slumber how to retrieve the dynamic value.
+All string _values_ (i.e. _not_ keys) in a request collection are template strings, meaning they support templating. The syntax for templating a value into a string is double curly braces `{{...}}`. The contents inside the braces tell Slumber how to retrieve the dynamic value.
 
 TODO add some more content here
 
diff --git a/src/config/mod.rs b/src/config/mod.rs
index 167104ea..f234d8e1 100644
--- a/src/config/mod.rs
+++ b/src/config/mod.rs
@@ -5,6 +5,7 @@ use anyhow::{anyhow, Context};
 use derive_more::{Deref, Display, From};
 use indexmap::IndexMap;
 use serde::{Deserialize, Serialize};
+use serde_json_path::JsonPath;
 use std::{
     future::Future,
     path::{Path, PathBuf},
@@ -106,8 +107,7 @@ pub struct Chain {
     #[serde(default)]
     pub sensitive: bool,
     /// JSONpath to extract a value from the response. For JSON data only.
-    // TODO strong typing on this
-    pub selector: Option<String>,
+    pub selector: Option<JsonPath>,
 }
 
 /// The source of data for a chain
diff --git a/src/template.rs b/src/template.rs
index 76a631ce..c432b41d 100644
--- a/src/template.rs
+++ b/src/template.rs
@@ -356,15 +356,8 @@ impl<'a> ChainTemplateSource<'a> {
     fn apply_selector(
         &self,
         value: Cow<'_, str>,
-        selector: &'a str,
+        selector: &JsonPath,
     ) -> Result<Cow<'a, str>, ChainError> {
-        // Parse the JSON path
-        let path =
-            JsonPath::parse(selector).map_err(|err| ChainError::JsonPath {
-                selector: selector.to_owned(),
-                error: err,
-            })?;
-
         // Parse the response as JSON. Intentionally ignore the
         // content-type. If the user wants to treat it as JSON, we
         // should allow that even if the server is wrong.
@@ -372,7 +365,7 @@ impl<'a> ChainTemplateSource<'a> {
             .map_err(|err| ChainError::ParseResponse { error: err })?;
 
         // Apply the path to the json
-        let found_value = path
+        let found_value = selector
             .query(&json_value)
             .exactly_one()
             .map_err(|err| ChainError::InvalidResult { error: err })?;
@@ -451,12 +444,6 @@ pub enum ChainError {
     /// response
     #[error("No response available")]
     NoResponse,
-    #[error("Error parsing JSON path {selector:?}")]
-    JsonPath {
-        selector: String,
-        #[source]
-        error: serde_json_path::ParseError,
-    },
     /// Failed to parse the response body before applying a selector
     #[error("Error parsing response")]
     ParseResponse {
@@ -561,11 +548,12 @@ mod tests {
             )
             .await
             .unwrap();
+        let selector = selector.map(|s| s.parse().unwrap());
         let chains = vec![create!(
             Chain,
             id: "chain1".into(),
             source: ChainSource::Request(recipe_id),
-            selector: selector.map(String::from),
+            selector: selector,
         )];
         let context = create!(
             TemplateContext, repository: repository, chains: chains,
@@ -591,20 +579,7 @@ mod tests {
             Chain,
             id: "chain1".into(),
             source: ChainSource::Request("recipe1".into()),
-            selector: Some("$.".into()),
-        ),
-        Some((
-            create!(Request, recipe_id: "recipe1".into()),
-            create!(Response, body: "{}".into()),
-        )),
-        "Error parsing JSON path \"$.\"",
-    )]
-    #[case(
-        create!(
-            Chain,
-            id: "chain1".into(),
-            source: ChainSource::Request("recipe1".into()),
-            selector: Some("$.message".into()),
+            selector: Some("$.message".parse().unwrap()),
         ),
         Some((
             create!(Request, recipe_id: "recipe1".into()),
@@ -617,7 +592,7 @@ mod tests {
             Chain,
             id: "chain1".into(),
             source: ChainSource::Request("recipe1".into()),
-            selector: Some("$.*".into()),
+            selector: Some("$.*".parse().unwrap()),
         ),
         Some((
             create!(Request, recipe_id: "recipe1".into()),