fix(jit): handle scalar values (#2408)

tailcallhq · Jul 16, 2024 · eb1fdd1 · eb1fdd1 · github-actions · Jul 16, 2024
1 parent 806830f
commit eb1fdd1
Show file tree

Hide file tree

Showing 14 changed files with 97 additions and 7 deletions.
diff --git a/src/core/blueprint/index.rs b/src/core/blueprint/index.rs
@@ -3,9 +3,10 @@ use indexmap::IndexMap;
 use crate::core::blueprint::{
     Blueprint, Definition, FieldDefinition, InputFieldDefinition, SchemaDefinition,
 };
+use crate::core::scalar::is_predefined_scalar;
 
 ///
-/// A read optimized index of all the fields in the Blueprint. Provide O(1)
+/// A read optimized index of all the types in the Blueprint. Provide O(1)
 /// access to getting any field information.
 
 pub struct Index {
@@ -29,6 +30,12 @@ impl QueryField {
 }
 
 impl Index {
+    pub fn type_is_scalar(&self, type_name: &str) -> bool {
+        let def = self.map.get(type_name).map(|(def, _)| def);
+
+        matches!(def, Some(Definition::Scalar(_))) || is_predefined_scalar(type_name)
+    }
+
     pub fn get_field(&self, type_name: &str, field_name: &str) -> Option<&QueryField> {
         self.map
             .get(type_name)

diff --git a/src/core/jit/builder.rs b/src/core/jit/builder.rs
@@ -202,6 +202,7 @@ impl Builder {
                             id,
                             name,
                             ir,
+                            is_scalar: self.index.type_is_scalar(type_of.name()),
                             type_of,
                             skip,
                             include,

diff --git a/src/core/jit/error.rs b/src/core/jit/error.rs
@@ -1,5 +1,5 @@
 use async_graphql::parser::types::OperationType;
-use async_graphql::ErrorExtensions;
+use async_graphql::{ErrorExtensions, PathSegment};
 use thiserror::Error;
 
 #[derive(Error, Debug, Clone)]
@@ -18,6 +18,14 @@ pub enum ResolveInputError {
     VariableIsNotFound(String),
 }
 
+#[derive(Error, Debug, Clone)]
+pub enum ValidationError {
+    // TODO: replace with sane error message. Right now, it's defined as is only for compatibility
+    // with async_graphql error message for this case
+    #[error(r#"internal: invalid value for scalar "{type_of}", expected "FieldValue::Value""#)]
+    ScalarInvalid { type_of: String, path: String },
+}
+
 #[derive(Debug, Clone, Error)]
 pub enum Error {
     #[error("Build error: {0}")]
@@ -26,6 +34,8 @@ pub enum Error {
     ParseError(#[from] async_graphql::parser::Error),
     #[error(transparent)]
     IR(#[from] crate::core::ir::Error),
+    #[error(transparent)]
+    Validation(#[from] ValidationError),
 }
 
 impl ErrorExtensions for Error {
@@ -34,6 +44,20 @@ impl ErrorExtensions for Error {
             Error::BuildError(error) => error.extend(),
             Error::ParseError(error) => error.extend(),
             Error::IR(error) => error.extend(),
+            Error::Validation(error) => error.extend(),
+        }
+    }
+}
+
+impl Error {
+    pub fn path(&self) -> Vec<PathSegment> {
+        match self {
+            Error::Validation(error) => match error {
+                ValidationError::ScalarInvalid { type_of: _, path } => {
+                    vec![PathSegment::Field(path.clone())]
+                }
+            },
+            _ => Vec::new(),
         }
     }
 }

diff --git a/src/core/jit/model.rs b/src/core/jit/model.rs
@@ -102,6 +102,7 @@ pub struct Field<Extensions, Input> {
     pub args: Vec<Arg<Input>>,
     pub extensions: Option<Extensions>,
     pub pos: Pos,
+    pub is_scalar: bool,
 }
 
 #[derive(Clone, Debug, PartialEq)]
@@ -138,6 +139,7 @@ impl<Extensions, Input> Field<Extensions, Input> {
                 .into_iter()
                 .map(|arg| arg.try_map(&map))
                 .collect::<Result<_, _>>()?,
+            is_scalar: self.is_scalar,
         })
     }
 }
@@ -189,6 +191,7 @@ impl<Input> Field<Flat, Input> {
             args: self.args,
             pos: self.pos,
             extensions,
+            is_scalar: self.is_scalar,
         }
     }
 }
@@ -208,6 +211,8 @@ impl<Extensions: Debug, Input: Debug> Debug for Field<Extensions, Input> {
         if self.extensions.is_some() {
             debug_struct.field("extensions", &self.extensions);
         }
+        debug_struct.field("is_scalar", &self.is_scalar);
+
         debug_struct.finish()
     }
 }

diff --git a/src/core/jit/response.rs b/src/core/jit/response.rs
@@ -34,11 +34,13 @@ impl Response<async_graphql::Value, jit::Error> {
             resp = resp.extension(name, value);
         }
         for error in self.errors {
-            let extensions = error.node.extend().extensions;
-            let mut server_error =
-                async_graphql::ServerError::new(error.node.to_string(), Some(error.pos));
+            let pos = error.pos;
+            let error = error.node;
+            let extensions = error.extend().extensions;
+            let mut server_error = async_graphql::ServerError::new(error.to_string(), Some(pos));
 
             server_error.extensions = extensions;
+            server_error.path = error.path();
 
             resp.errors.push(server_error);
         }

diff --git a/src/core/jit/snapshots/tailcall__core__jit__builder__tests__default_value.snap b/src/core/jit/snapshots/tailcall__core__jit__builder__tests__default_value.snap
@@ -44,9 +44,11 @@ expression: plan.into_nested()
                         id: 1,
                         name: "id",
                         type_of: ID!,
+                        is_scalar: true,
                     },
                 ],
             ),
         ),
+        is_scalar: false,
     },
 ]
diff --git a/src/core/jit/snapshots/tailcall__core__jit__builder__tests__fragments.snap b/src/core/jit/snapshots/tailcall__core__jit__builder__tests__fragments.snap
@@ -28,19 +28,23 @@ expression: plan.into_nested()
                         id: 1,
                         name: "name",
                         type_of: String!,
+                        is_scalar: true,
                     },
                     Field {
                         id: 2,
                         name: "email",
                         type_of: String!,
+                        is_scalar: true,
                     },
                     Field {
                         id: 3,
                         name: "phone",
                         type_of: String,
+                        is_scalar: true,
                     },
                 ],
             ),
         ),
+        is_scalar: false,
     },
 ]
diff --git a/src/core/jit/snapshots/tailcall__core__jit__builder__tests__from_document.snap b/src/core/jit/snapshots/tailcall__core__jit__builder__tests__from_document.snap
@@ -23,18 +23,22 @@ expression: plan.into_nested()
                                         id: 2,
                                         name: "id",
                                         type_of: ID!,
+                                        is_scalar: true,
                                     },
                                     Field {
                                         id: 3,
                                         name: "name",
                                         type_of: String!,
+                                        is_scalar: true,
                                     },
                                 ],
                             ),
                         ),
+                        is_scalar: false,
                     },
                 ],
             ),
         ),
+        is_scalar: false,
     },
 ]
diff --git a/src/core/jit/snapshots/tailcall__core__jit__builder__tests__multiple_operations.snap b/src/core/jit/snapshots/tailcall__core__jit__builder__tests__multiple_operations.snap
@@ -28,15 +28,18 @@ expression: plan.into_nested()
                         id: 1,
                         name: "id",
                         type_of: ID!,
+                        is_scalar: true,
                     },
                     Field {
                         id: 2,
                         name: "username",
                         type_of: String!,
+                        is_scalar: true,
                     },
                 ],
             ),
         ),
+        is_scalar: false,
     },
     Field {
         id: 3,
@@ -50,14 +53,17 @@ expression: plan.into_nested()
                         id: 4,
                         name: "id",
                         type_of: ID!,
+                        is_scalar: true,
                     },
                     Field {
                         id: 5,
                         name: "title",
                         type_of: String!,
+                        is_scalar: true,
                     },
                 ],
             ),
         ),
+        is_scalar: false,
     },
 ]
diff --git a/src/core/jit/snapshots/tailcall__core__jit__builder__tests__simple_mutation.snap b/src/core/jit/snapshots/tailcall__core__jit__builder__tests__simple_mutation.snap
@@ -59,34 +59,41 @@ expression: plan.into_nested()
                         id: 1,
                         name: "id",
                         type_of: ID!,
+                        is_scalar: true,
                     },
                     Field {
                         id: 2,
                         name: "name",
                         type_of: String!,
+                        is_scalar: true,
                     },
                     Field {
                         id: 3,
                         name: "email",
                         type_of: String!,
+                        is_scalar: true,
                     },
                     Field {
                         id: 4,
                         name: "phone",
                         type_of: String,
+                        is_scalar: true,
                     },
                     Field {
                         id: 5,
                         name: "website",
                         type_of: String,
+                        is_scalar: true,
                     },
                     Field {
                         id: 6,
                         name: "username",
                         type_of: String!,
+                        is_scalar: true,
                     },
                 ],
             ),
         ),
+        is_scalar: false,
     },
 ]
diff --git a/src/core/jit/snapshots/tailcall__core__jit__builder__tests__simple_query.snap b/src/core/jit/snapshots/tailcall__core__jit__builder__tests__simple_query.snap
@@ -23,13 +23,16 @@ expression: plan.into_nested()
                                         id: 2,
                                         name: "id",
                                         type_of: ID!,
+                                        is_scalar: true,
                                     },
                                 ],
                             ),
                         ),
+                        is_scalar: false,
                     },
                 ],
             ),
         ),
+        is_scalar: false,
     },
 ]
diff --git a/src/core/jit/snapshots/tailcall__core__jit__builder__tests__unions.snap b/src/core/jit/snapshots/tailcall__core__jit__builder__tests__unions.snap
@@ -1,6 +1,6 @@
 ---
 source: src/core/jit/builder.rs
-expression: plan
+expression: plan.into_nested()
 ---
 [
     Field {
@@ -21,5 +21,6 @@ expression: plan
                 default_value: None,
             },
         ],
+        is_scalar: false,
     },
 ]
diff --git a/src/core/jit/snapshots/tailcall__core__jit__builder__tests__variables.snap b/src/core/jit/snapshots/tailcall__core__jit__builder__tests__variables.snap
@@ -28,14 +28,17 @@ expression: plan.into_nested()
                         id: 1,
                         name: "id",
                         type_of: ID!,
+                        is_scalar: true,
                     },
                     Field {
                         id: 2,
                         name: "name",
                         type_of: String!,
+                        is_scalar: true,
                     },
                 ],
             ),
         ),
+        is_scalar: false,
     },
 ]
diff --git a/src/core/jit/synth/synth_const.rs b/src/core/jit/synth/synth_const.rs
@@ -5,8 +5,9 @@ use indexmap::IndexMap;
 use super::Synthesizer;
 use crate::core::jit::model::{Field, Nested};
 use crate::core::jit::store::{Data, Store};
-use crate::core::jit::{DataPath, Error, OperationPlan, Variable, Variables};
+use crate::core::jit::{DataPath, Error, OperationPlan, ValidationError, Variable, Variables};
 use crate::core::json::JsonLike;
+use crate::core::scalar::get_scalar;
 
 pub struct Synth {
     selection: Vec<Field<Nested<ConstValue>, ConstValue>>,
@@ -127,6 +128,26 @@ impl Synth {
         let include = self.include(node);
 
         match parent {
+            // scalar values should be returned as is
+            val if node.is_scalar => {
+                let validation = get_scalar(node.type_of.name());
+
+                // TODO: add validation for input type as well. But input types are not checked
+                // by async_graphql anyway so it should be done after replacing
+                // default engine with JIT
+                if validation(val) {
+                    Ok(val.clone())
+                } else {
+                    Err(Positioned {
+                        pos: node.pos,
+                        node: ValidationError::ScalarInvalid {
+                            type_of: node.type_of.name().to_string(),
+                            path: node.name.clone(),
+                        }
+                        .into(),
+                    })
+                }
+            }
             ConstValue::Object(obj) => {
                 let mut ans = IndexMap::default();
                 if include {
Thread Stats	Avg	Stdev	Max	+/- Stdev
Latency	7.36ms	3.38ms	150.53ms	72.27%
Req/Sec	3.44k	128.71	3.70k	82.75%