fix(jit): input arguments required error (#2756)

Co-authored-by: Tushar Mathur <tusharmath@gmail.com>
Co-authored-by: Kiryl Mialeshka <8974488+meskill@users.noreply.github.com>

Author: 3 people

src/core/blueprint/index.rs

@@ -1,5 +1,6 @@
 use indexmap::IndexMap;
 
+use super::InputObjectTypeDefinition;
 use crate::core::blueprint::{
     Blueprint, Definition, FieldDefinition, InputFieldDefinition, SchemaDefinition,
 };
@@ -78,6 +79,13 @@ impl Index {
             false
         }
     }
+
+    pub fn get_input_type_definition(&self, type_name: &str) -> Option<&InputObjectTypeDefinition> {
+        match self.map.get(type_name) {
+            Some((Definition::InputObject(input), _)) => Some(input),
+            _ => None,
+        }
+    }
 }
 
 impl From<&Blueprint> for Index {

src/core/jit/context.rs

@@ -80,16 +80,9 @@ impl<'a, Input: Clone, Output> Context<'a, Input, Output> {
 
         for arg in field.args.iter() {
             let name = arg.name.as_str();
-            let value = arg
-                .value
-                .clone()
-                // TODO: default value resolution should happen in the InputResolver
-                .or_else(|| arg.default_value.clone());
+            let value = arg.value.clone();
             if let Some(value) = value {
                 arg_map.insert(Name::new(name), value);
-            } else if !arg.type_of.is_nullable() {
-                // TODO: throw error here
-                todo!()
             }
         }
         Some(arg_map)

src/core/jit/error.rs

@@ -20,6 +20,11 @@ pub enum BuildError {
 pub enum ResolveInputError {
     #[error("Variable `{0}` is not defined")]
     VariableIsNotFound(String),
+    #[error("Argument `{arg_name}` for field `{field_name}` is required")]
+    ArgumentIsRequired {
+        arg_name: String,
+        field_name: String,
+    },
 }
 
 #[derive(Error, Debug, Clone)]

src/core/jit/input_resolver.rs

@@ -1,6 +1,8 @@
 use async_graphql_value::{ConstValue, Value};
 
-use super::{OperationPlan, ResolveInputError, Variables};
+use super::{Arg, Field, OperationPlan, ResolveInputError, Variables};
+use crate::core::json::{JsonLikeOwned, JsonObjectLike};
+use crate::core::Type;
 
 /// Trait to represent conversion from some dynamic type (with variables)
 /// to the resolved variant based on the additional provided info.
@@ -18,8 +20,6 @@ pub trait InputResolvable {
 impl InputResolvable for Value {
     type Output = ConstValue;
 
-    // TODO:
-    // - provide default values
     fn resolve(self, variables: &Variables<ConstValue>) -> Result<Self::Output, ResolveInputError> {
         self.into_const_with(|name| {
             variables
@@ -45,8 +45,9 @@ impl<Input> InputResolver<Input> {
 impl<Input, Output> InputResolver<Input>
 where
     Input: Clone,
-    Output: Clone,
+    Output: Clone + JsonLikeOwned + TryFrom<serde_json::Value>,
     Input: InputResolvable<Output = Output>,
+    <Output as TryFrom<serde_json::Value>>::Error: std::fmt::Debug,
 {
     pub fn resolve_input(
         &self,
@@ -57,7 +58,28 @@ where
             .as_parent()
             .iter()
             .map(|field| field.clone().try_map(|value| value.resolve(variables)))
-            .collect::<Result<_, _>>()?;
+            .map(|field| match field {
+                Ok(field) => {
+                    let args = field
+                        .args
+                        .into_iter()
+                        .map(|arg| {
+                            let value = self.recursive_parse_arg(
+                                &field.name,
+                                &arg.name,
+                                &arg.type_of,
+                                &arg.default_value,
+                                arg.value,
+                            )?;
+                            Ok(Arg { value, ..arg })
+                        })
+                        .collect::<Result<_, _>>()?;
+
+                    Ok(Field { args, ..field })
+                }
+                Err(err) => Err(err),
+            })
+            .collect::<Result<Vec<_>, _>>()?;
 
         Ok(OperationPlan::new(
             new_fields,
@@ -66,4 +88,78 @@ where
             self.plan.is_introspection_query,
         ))
     }
+
+    #[allow(clippy::too_many_arguments)]
+    fn recursive_parse_arg(
+        &self,
+        parent_name: &str,
+        arg_name: &str,
+        type_of: &Type,
+        default_value: &Option<Output>,
+        value: Option<Output>,
+    ) -> Result<Option<Output>, ResolveInputError> {
+        let is_value_null = value.as_ref().map(|val| val.is_null()).unwrap_or(true);
+        let value = if !type_of.is_nullable() && value.is_none() {
+            let default_value = default_value.clone();
+
+            Some(default_value.ok_or(ResolveInputError::ArgumentIsRequired {
+                arg_name: arg_name.to_string(),
+                field_name: parent_name.to_string(),
+            })?)
+        } else if !type_of.is_nullable() && is_value_null {
+            return Err(ResolveInputError::ArgumentIsRequired {
+                arg_name: arg_name.to_string(),
+                field_name: parent_name.to_string(),
+            });
+        } else if value.is_none() {
+            default_value.clone()
+        } else {
+            value
+        };
+
+        let Some(mut value) = value else {
+            return Ok(None);
+        };
+
+        let Some(def) = self.plan.index.get_input_type_definition(type_of.name()) else {
+            return Ok(Some(value));
+        };
+
+        if let Some(obj) = value.as_object_mut() {
+            for arg_field in &def.fields {
+                let parent_name = format!("{}.{}", parent_name, arg_name);
+                let field_value = obj.get_key(&arg_field.name).cloned();
+                let field_default = arg_field
+                    .default_value
+                    .clone()
+                    .map(|value| Output::try_from(value).expect("The conversion cannot fail"));
+                let value = self.recursive_parse_arg(
+                    &parent_name,
+                    &arg_field.name,
+                    &arg_field.of_type,
+                    &field_default,
+                    field_value,
+                )?;
+                if let Some(value) = value {
+                    obj.insert_key(&arg_field.name, value);
+                }
+            }
+        } else if let Some(arr) = value.as_array_mut() {
+            for (index, item) in arr.iter_mut().enumerate() {
+                let parent_name = format!("{}.{}.{}", parent_name, arg_name, index);
+
+                *item = self
+                    .recursive_parse_arg(
+                        &parent_name,
+                        &index.to_string(),
+                        type_of,
+                        &None,
+                        Some(item.clone()),
+                    )?
+                    .expect("Because we start with `Some`, we will end with `Some`");
+            }
+        }
+
+        Ok(Some(value))
+    }
 }

src/core/jit/snapshots/tailcall__core__jit__builder__tests__default_value.snap

@@ -35,6 +35,11 @@ expression: plan.into_nested()
                             ): String(
                                 "tailcall test",
                             ),
+                            Name(
+                                "id",
+                            ): Number(
+                                Number(101),
+                            ),
                         },
                     ),
                 ),

src/core/jit/snapshots/tailcall__core__jit__builder__tests__resolving_operation-2.snap

@@ -35,6 +35,11 @@ expression: plan.into_nested()
                             ): String(
                                 "test-12",
                             ),
+                            Name(
+                                "id",
+                            ): Number(
+                                Number(101),
+                            ),
                         },
                     ),
                 ),

src/core/json/borrow.rs

@@ -47,6 +47,13 @@ impl<'ctx> JsonLike<'ctx> for Value<'ctx> {
         }
     }
 
+    fn as_array_mut(&mut self) -> Option<&mut Vec<Self>> {
+        match self {
+            Value::Array(arr) => Some(arr),
+            _ => None,
+        }
+    }
+
     fn into_array(self) -> Option<Vec<Self>> {
         match self {
             Value::Array(array) => Some(array),

src/core/json/graphql.rs

@@ -33,6 +33,13 @@ impl<'json> JsonLike<'json> for ConstValue {
         }
     }
 
+    fn as_array_mut(&mut self) -> Option<&mut Vec<Self>> {
+        match self {
+            ConstValue::List(seq) => Some(seq),
+            _ => None,
+        }
+    }
+
     fn into_array(self) -> Option<Vec<Self>> {
         match self {
             ConstValue::List(seq) => Some(seq),

src/core/json/json_like.rs

@@ -16,6 +16,7 @@ pub trait JsonLike<'json>: Sized {
 
     // Operators
     fn as_array(&self) -> Option<&Vec<Self>>;
+    fn as_array_mut(&mut self) -> Option<&mut Vec<Self>>;
     fn into_array(self) -> Option<Vec<Self>>;
     fn as_object(&self) -> Option<&Self::JsonObject>;
     fn as_object_mut(&mut self) -> Option<&mut Self::JsonObject>;

src/core/json/serde.rs

@@ -26,6 +26,10 @@ impl<'json> JsonLike<'json> for serde_json::Value {
         self.as_array()
     }
 
+    fn as_array_mut(&mut self) -> Option<&mut Vec<Self>> {
+        self.as_array_mut()
+    }
+
     fn into_array(self) -> Option<Vec<Self>> {
         if let Self::Array(vec) = self {
             Some(vec)

tests/core/snapshots/graphql-conformance-001.md_5.snap

@@ -0,0 +1,18 @@
+---
+source: tests/core/spec.rs
+expression: response
+---
+{
+  "status": 200,
+  "headers": {
+    "content-type": "application/json"
+  },
+  "body": {
+    "data": null,
+    "errors": [
+      {
+        "message": "Build error: ResolveInputError: Argument `id` for field `user` is required"
+      }
+    ]
+  }
+}

tests/core/snapshots/graphql-conformance-015.md_10.snap

@@ -0,0 +1,18 @@
+---
+source: tests/core/spec.rs
+expression: response
+---
+{
+  "status": 200,
+  "headers": {
+    "content-type": "application/json"
+  },
+  "body": {
+    "data": null,
+    "errors": [
+      {
+        "message": "Build error: ResolveInputError: Argument `size` for field `profilePic` is required"
+      }
+    ]
+  }
+}

tests/core/snapshots/graphql-conformance-015.md_11.snap

@@ -0,0 +1,19 @@
+---
+source: tests/core/spec.rs
+expression: response
+---
+{
+  "status": 200,
+  "headers": {
+    "content-type": "application/json"
+  },
+  "body": {
+    "data": {
+      "user": {
+        "id": 4,
+        "name": "User 4",
+        "spam": "FIZZ: [{\"bar\":\"BUZZ\"},{\"bar\":\"test\"}]"
+      }
+    }
+  }
+}

tests/core/snapshots/graphql-conformance-015.md_5.snap

@@ -12,7 +12,7 @@ expression: response
       "user": {
         "id": 4,
         "name": "User 4",
-        "featuredVideo": "video_4_1600_900_"
+        "featuredVideo": "video_4_1600_900_true"
       }
     }
   }

tests/core/snapshots/graphql-conformance-015.md_9.snap

@@ -0,0 +1,18 @@
+---
+source: tests/core/spec.rs
+expression: response
+---
+{
+  "status": 200,
+  "headers": {
+    "content-type": "application/json"
+  },
+  "body": {
+    "data": null,
+    "errors": [
+      {
+        "message": "Build error: ResolveInputError: Argument `height` for field `featuredVideoPreview.video` is required"
+      }
+    ]
+  }
+}

tests/core/snapshots/graphql-conformance-015.md_client.snap

@@ -12,6 +12,10 @@ scalar Email
 
 scalar Empty
 
+input Foo {
+  bar: String! = "BUZZ"
+}
+
 scalar Int128
 
 scalar Int16
@@ -49,6 +53,7 @@ type User {
   name: String!
   profilePic(size: Int! = 100, width: Int, height: Int = 100): String!
   searchComments(query: [[String!]!]! = [["today"]]): String!
+  spam(foo: [Foo!]!): String!
 }
 
 input VideoSize {

tests/core/snapshots/graphql-conformance-015.md_merged.snap

@@ -8,6 +8,10 @@ schema
   query: Query
 }
 
+input Foo {
+  bar: String! = "BUZZ"
+}
+
 input VideoSize {
   hdr: Boolean = true
   height: Int!
@@ -28,4 +32,5 @@ type User {
   profilePic(size: Int! = 100, width: Int, height: Int = 100): String!
     @expr(body: "{{.value.id}}_{{.args.size}}_{{.args.width}}_{{.args.height}}")
   searchComments(query: [[String!]!]! = [["today"]]): String! @expr(body: "video_{{.value.id}}_{{.args.query}}")
+  spam(foo: [Foo!]!): String! @expr(body: "FIZZ: {{.args.foo}}")
 }