-
Notifications
You must be signed in to change notification settings - Fork 259
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: bug with graphql nested queries #3089
Changes from all commits
26da27b
8ec733a
e766c22
859a179
a8bbb91
0750c3f
5ea8adb
2f64c1d
a8c89d8
c551486
92a9d2c
68db95f
2bd576e
351c1f7
b9f8cf0
2fa57cf
2a594d1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,48 +12,67 @@ | |
use crate::core::ir::RelatedFields; | ||
use crate::core::try_fold::TryFold; | ||
|
||
#[allow(clippy::too_many_arguments)] | ||
fn create_related_fields( | ||
config: &Config, | ||
type_name: &str, | ||
visited: &mut HashSet<String>, | ||
) -> RelatedFields { | ||
paths: &mut HashMap<String, Vec<String>>, | ||
path: Vec<String>, | ||
root: &str, | ||
) -> Option<RelatedFields> { | ||
let mut map = HashMap::new(); | ||
if visited.contains(type_name) { | ||
return RelatedFields(map); | ||
return Some(RelatedFields(map)); | ||
} | ||
visited.insert(type_name.to_string()); | ||
|
||
if let Some(type_) = config.find_type(type_name) { | ||
for (name, field) in &type_.fields { | ||
if !field.has_resolver() { | ||
if let Some(modify) = &field.modify { | ||
if let Some(modified_name) = &modify.name { | ||
map.insert( | ||
modified_name.clone(), | ||
( | ||
name.clone(), | ||
create_related_fields(config, field.type_of.name(), visited), | ||
), | ||
); | ||
} | ||
} else { | ||
map.insert( | ||
let used_name = match &field.modify { | ||
Some(modify) => match &modify.name { | ||
Some(modified_name) => Some(modified_name), | ||
_ => None, | ||
}, | ||
_ => Some(name), | ||
}; | ||
let mut next_path = path.clone(); | ||
next_path.push(used_name?.to_string()); | ||
if !(paths.contains_key(field.type_of.name())) { | ||
paths.insert(field.type_of.name().to_string(), next_path.clone()); | ||
}; | ||
map.insert( | ||
used_name?.to_string(), | ||
( | ||
name.clone(), | ||
( | ||
name.clone(), | ||
create_related_fields(config, field.type_of.name(), visited), | ||
), | ||
); | ||
} | ||
create_related_fields( | ||
config, | ||
field.type_of.name(), | ||
visited, | ||
paths, | ||
next_path.clone(), | ||
root, | ||
)?, | ||
paths.get(field.type_of.name())?.to_vec(), | ||
root == field.type_of.name(), | ||
), | ||
); | ||
} | ||
} | ||
} else if let Some(union_) = config.find_union(type_name) { | ||
for type_name in &union_.types { | ||
map.extend(create_related_fields(config, type_name, visited).0); | ||
let mut next_path = path.clone(); | ||
next_path.push(type_name.to_string()); | ||
if !(paths.contains_key(type_name)) { | ||
paths.insert(type_name.to_string(), next_path.clone()); | ||
}; | ||
map.extend( | ||
create_related_fields(config, type_name, visited, paths, next_path, root)?.0, | ||
); | ||
} | ||
}; | ||
|
||
RelatedFields(map) | ||
Some(RelatedFields(map)) | ||
} | ||
|
||
pub fn compile_graphql( | ||
|
@@ -66,17 +85,30 @@ | |
Valid::succeed(graphql.url.as_str()) | ||
.zip(helpers::headers::to_mustache_headers(&graphql.headers)) | ||
.and_then(|(base_url, headers)| { | ||
Valid::from( | ||
RequestTemplate::new( | ||
base_url.to_owned(), | ||
operation_type, | ||
&graphql.name, | ||
args, | ||
headers, | ||
create_related_fields(config, type_name, &mut HashSet::new()), | ||
) | ||
.map_err(|e| ValidationError::new(e.to_string())), | ||
Valid::from_option( | ||
create_related_fields( | ||
config, | ||
type_name, | ||
&mut HashSet::new(), | ||
&mut HashMap::new(), | ||
vec![], | ||
type_name, | ||
), | ||
"Logical error occurred while creating Related Fields".to_string(), | ||
) | ||
.and_then(|related_fields| { | ||
Valid::from( | ||
RequestTemplate::new( | ||
base_url.to_owned(), | ||
operation_type, | ||
&graphql.name, | ||
args, | ||
headers, | ||
related_fields, | ||
) | ||
.map_err(|e| ValidationError::new(e.to_string())), | ||
) | ||
}) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This implementation needs to be changed. The related field information can't be inferred while creating a blueprint. It should be implemented as a part of the JIT transformation step. The JIT Transformations should identify exactly what the RequestTemplate should look like for each query and then while evaluating we should simply use that operation and execute the GraphQL request. |
||
}) | ||
.map(|req_template| { | ||
let field_name = graphql.name.clone(); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
--- | ||
source: tests/core/spec.rs | ||
expression: response | ||
snapshot_kind: text | ||
--- | ||
{ | ||
"status": 200, | ||
"headers": { | ||
"content-type": "application/json" | ||
}, | ||
"body": { | ||
"data": { | ||
"queryNodeA": { | ||
"name": "nodeA", | ||
"nodeB": { | ||
"name": "nodeB" | ||
}, | ||
"nodeC": { | ||
"name": "nodeC" | ||
}, | ||
"child": { | ||
"name": "nodeA" | ||
} | ||
} | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
--- | ||
source: tests/core/spec.rs | ||
expression: formatted | ||
snapshot_kind: text | ||
--- | ||
type NodeA { | ||
child: NodeA | ||
name: String | ||
nodeB: NodeB | ||
nodeC: NodeC | ||
} | ||
|
||
type NodeB { | ||
name: String | ||
nodeA: NodeA | ||
nodeC: NodeC | ||
} | ||
|
||
type NodeC { | ||
name: String | ||
nodeA: NodeA | ||
nodeB: NodeB | ||
} | ||
|
||
type Query { | ||
queryNodeA: NodeA | ||
} | ||
|
||
schema { | ||
query: Query | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
--- | ||
source: tests/core/spec.rs | ||
expression: formatter | ||
snapshot_kind: text | ||
--- | ||
schema @server(hostname: "0.0.0.0", port: 8000) @upstream { | ||
query: Query | ||
} | ||
|
||
type NodeA { | ||
name: String | ||
nodeA: NodeA @modify(name: "child") | ||
nodeB: NodeB | ||
nodeC: NodeC | ||
} | ||
|
||
type NodeB { | ||
name: String | ||
nodeA: NodeA | ||
nodeC: NodeC | ||
} | ||
|
||
type NodeC { | ||
name: String | ||
nodeA: NodeA | ||
nodeB: NodeB | ||
} | ||
|
||
type Query { | ||
queryNodeA: NodeA @graphQL(url: "http://upstream/graphql", name: "nodeA") | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,71 @@ | ||
# Complicated queries | ||
|
||
```graphql @config | ||
schema @server(port: 8000, hostname: "0.0.0.0") { | ||
query: Query | ||
} | ||
|
||
type Query { | ||
queryNodeA: NodeA @graphQL(url: "http://upstream/graphql", name: "nodeA") | ||
} | ||
|
||
type NodeA { | ||
name: String | ||
nodeB: NodeB | ||
nodeC: NodeC | ||
nodeA: NodeA @modify(name: "child") | ||
} | ||
|
||
type NodeB { | ||
name: String | ||
nodeA: NodeA | ||
nodeC: NodeC | ||
} | ||
|
||
type NodeC { | ||
name: String | ||
nodeA: NodeA | ||
nodeB: NodeB | ||
} | ||
``` | ||
|
||
```yml @mock | ||
- request: | ||
method: POST | ||
url: http://upstream/graphql | ||
textBody: '{ "query": "query { nodeA { name nodeB { name } nodeC { name } nodeA { name } } }" }' | ||
expectedHits: 1 | ||
response: | ||
status: 200 | ||
body: | ||
data: | ||
nodeA: | ||
name: nodeA | ||
nodeB: | ||
name: nodeB | ||
nodeC: | ||
name: nodeC | ||
nodeA: | ||
name: nodeA | ||
``` | ||
|
||
```yml @test | ||
- method: POST | ||
url: http://localhost:8080/graphql | ||
body: | ||
query: | | ||
query queryNodeA { | ||
queryNodeA { | ||
name | ||
nodeB { | ||
name | ||
} | ||
nodeC { | ||
name | ||
} | ||
child { | ||
name | ||
} | ||
} | ||
} | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.