diff --git a/src/core/jit/synth/synth.rs b/src/core/jit/synth/synth.rs index e8b430d66b..446fc8f00a 100644 --- a/src/core/jit/synth/synth.rs +++ b/src/core/jit/synth/synth.rs @@ -36,6 +36,7 @@ where #[inline(always)] pub fn synthesize(&'a self) -> Result> { let mut data = Value::JsonObject::new(); + let mut path = Vec::new(); for child in self.plan.as_nested().iter() { if !self.include(child) { @@ -43,22 +44,25 @@ where } // TODO: in case of error set `child.output_name` to null // and append error to response error array - let val = self.iter(child, None, &DataPath::new())?; - + let val = self.iter(child, None, &DataPath::new(), &mut path)?; data.insert_key(&child.output_name, val); } Ok(Value::object(data)) } + #[allow(clippy::too_many_arguments)] #[inline(always)] fn iter( &'a self, node: &'a Field, Value>, value: Option<&'a Value>, data_path: &DataPath, + path: &mut Vec, ) -> Result> { - match self.store.get(&node.id) { + path.push(PathSegment::Field(node.output_name.clone())); + + let result = match self.store.get(&node.id) { Some(val) => { let mut data = val; @@ -76,9 +80,10 @@ where let value = result.as_ref().map_err(Clone::clone)?; if node.type_of.is_list() != value.as_array().is_some() { - return self.node_nullable_guard(node); + self.node_nullable_guard(node, path) + } else { + self.iter_inner(node, value, data_path, path) } - self.iter_inner(node, value, data_path) } _ => { // TODO: should bailout instead of returning Null @@ -87,10 +92,13 @@ where } } None => match value { - Some(result) => self.iter_inner(node, result, data_path), - None => self.node_nullable_guard(node), + Some(result) => self.iter_inner(node, result, data_path, path), + None => self.node_nullable_guard(node, path), }, - } + }; + + path.pop(); + result } /// This guard ensures to return Null value only if node type permits it, in @@ -98,21 +106,25 @@ where fn node_nullable_guard( &'a self, node: &'a Field, Value>, + path: &[PathSegment], ) -> Result> { // according to GraphQL spec https://spec.graphql.org/October2021/#sec-Handling-Field-Errors if node.type_of.is_nullable() { Ok(Value::null()) } else { - Err(ValidationError::ValueRequired.into()).map_err(|e| self.to_location_error(e, node)) + Err(ValidationError::ValueRequired.into()) + .map_err(|e| self.to_location_error(e, node, path)) } } + #[allow(clippy::too_many_arguments)] #[inline(always)] fn iter_inner( &'a self, node: &'a Field, Value>, value: &'a Value, data_path: &DataPath, + path: &mut Vec, ) -> Result> { // skip the field if field is not included in schema if !self.include(node) { @@ -166,13 +178,12 @@ where for child in self.plan.field_iter_only(node, value) { // all checks for skip must occur in `iter_inner` // and include be checked before calling `iter` or recursing. - let include = self.include(child); - if include { + if self.include(child) { let value = if child.name == "__typename" { Value::string(node.value_type(value).into()) } else { let val = obj.get_key(child.name.as_str()); - self.iter(child, val, data_path)? + self.iter(child, val, data_path, path)? }; ans.insert_key(&child.output_name, value); } @@ -183,8 +194,11 @@ where (Some(arr), _) => { let mut ans = vec![]; for (i, val) in arr.iter().enumerate() { - let val = self.iter_inner(node, val, &data_path.clone().with_index(i))?; - ans.push(val) + path.push(PathSegment::Index(i)); + let val = + self.iter_inner(node, val, &data_path.clone().with_index(i), path)?; + path.pop(); + ans.push(val); } Ok(Value::array(ans)) } @@ -192,32 +206,16 @@ where } }; - eval_result.map_err(|e| self.to_location_error(e, node)) + eval_result.map_err(|e| self.to_location_error(e, node, path)) } fn to_location_error( &'a self, error: Error, node: &'a Field, Value>, + path: &[PathSegment], ) -> Positioned { - // create path from the root to the current node in the fields tree - let path = { - let mut path = Vec::new(); - - let mut parent = self.plan.find_field(node.id.clone()); - - while let Some(field) = parent { - path.push(PathSegment::Field(field.output_name.to_string())); - parent = field - .parent() - .and_then(|id| self.plan.find_field(id.clone())); - } - - path.reverse(); - path - }; - - Positioned::new(error, node.pos).with_path(path) + Positioned::new(error, node.pos).with_path(path.to_vec()) } } diff --git a/tests/core/snapshots/test-required-fields.md_13.snap b/tests/core/snapshots/test-required-fields.md_13.snap index 65e3a4af79..5a2e8949c7 100644 --- a/tests/core/snapshots/test-required-fields.md_13.snap +++ b/tests/core/snapshots/test-required-fields.md_13.snap @@ -15,13 +15,12 @@ expression: response "locations": [ { "line": 1, - "column": 29 + "column": 9 } ], "path": [ "innerEntryMissing", - 1, - "id" + 1 ] } ] diff --git a/tests/core/snapshots/test-required-fields.md_17.snap b/tests/core/snapshots/test-required-fields.md_17.snap index b6f7294e44..cabdc83aea 100644 --- a/tests/core/snapshots/test-required-fields.md_17.snap +++ b/tests/core/snapshots/test-required-fields.md_17.snap @@ -8,22 +8,14 @@ expression: response "content-type": "application/json" }, "body": { - "data": null, - "errors": [ - { - "message": "internal: non-null types require a return value", - "locations": [ - { - "line": 1, - "column": 29 - } - ], - "path": [ - "outerEntryMissing", - 1, - "id" - ] - } - ] + "data": { + "outerEntryMissing": [ + { + "id": 1, + "bar": "bar_1" + }, + null + ] + } } } diff --git a/tests/core/snapshots/test-required-fields.md_21.snap b/tests/core/snapshots/test-required-fields.md_21.snap index 0bee8203d9..0a7dffb745 100644 --- a/tests/core/snapshots/test-required-fields.md_21.snap +++ b/tests/core/snapshots/test-required-fields.md_21.snap @@ -8,22 +8,14 @@ expression: response "content-type": "application/json" }, "body": { - "data": null, - "errors": [ - { - "message": "internal: non-null types require a return value", - "locations": [ - { - "line": 1, - "column": 28 - } - ], - "path": [ - "noneEntryMissing", - 1, - "id" - ] - } - ] + "data": { + "noneEntryMissing": [ + { + "id": 1, + "bar": "bar_1" + }, + null + ] + } } } diff --git a/tests/core/snapshots/test-required-fields.md_9.snap b/tests/core/snapshots/test-required-fields.md_9.snap index 392bb18994..046718d6d8 100644 --- a/tests/core/snapshots/test-required-fields.md_9.snap +++ b/tests/core/snapshots/test-required-fields.md_9.snap @@ -15,13 +15,12 @@ expression: response "locations": [ { "line": 1, - "column": 28 + "column": 9 } ], "path": [ "fullEntryMissing", - 1, - "id" + 1 ] } ] diff --git a/tests/core/snapshots/test-required-fields.md_merged.snap b/tests/core/snapshots/test-required-fields.md_merged.snap index 3a4bb400ff..d4c5a67bb7 100644 --- a/tests/core/snapshots/test-required-fields.md_merged.snap +++ b/tests/core/snapshots/test-required-fields.md_merged.snap @@ -2,7 +2,7 @@ source: tests/core/spec.rs expression: formatter --- -schema @server @upstream(baseURL: "http://jsonplaceholder.typicode.com") { +schema @server(enableJIT: true) @upstream(baseURL: "http://jsonplaceholder.typicode.com") { query: Query } diff --git a/tests/execution/test-required-fields.md b/tests/execution/test-required-fields.md index 115ca86075..925051fd58 100644 --- a/tests/execution/test-required-fields.md +++ b/tests/execution/test-required-fields.md @@ -1,7 +1,7 @@ # Test API ```graphql @config -schema @server @upstream(baseURL: "http://jsonplaceholder.typicode.com") { +schema @server(enableJIT: true) @upstream(baseURL: "http://jsonplaceholder.typicode.com") { query: Query }