Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
8b8c8b9
split queries between primary and deferred parts
Nov 15, 2022
994dac2
handle named fragments
Nov 15, 2022
84e9d83
cleanup
Nov 16, 2022
5f993b9
do nnot add the defer directive to the generated query
Nov 16, 2022
597fbc8
logs
Nov 16, 2022
62a3fb7
Merge branch 'dev' into geal/split-queries
Dec 5, 2022
73c24fb
update router-bridge and apollo-encoder
Dec 5, 2022
405803c
handle queryPath in deferred nodes
Dec 5, 2022
02bdb78
split arrays
Dec 6, 2022
0364c60
Merge branch 'dev' into geal/split-queries
Dec 6, 2022
0563c78
missing snapshots
Dec 6, 2022
f903ee5
missing snapshot
Dec 6, 2022
ecb2dcb
lint
Dec 6, 2022
93a7ec2
handle arrays
Dec 6, 2022
9b222bf
remove unused code
Dec 6, 2022
606dcbd
add comments
Dec 6, 2022
cdf1b03
Merge branch 'dev' into geal/split-queries
Dec 6, 2022
3a1bc19
update router-bridge to fix CI builds
Dec 6, 2022
e794955
update router-bridge
Dec 12, 2022
223bc97
Merge branch 'dev' into geal/split-queries
Dec 12, 2022
20eeefe
Merge branch 'dev' into geal/split-queries
Dec 12, 2022
b545775
fix: Return root when parts of a query with deferred fragment
bnjjj Dec 1, 2022
643b547
move code in the right method
bnjjj Dec 1, 2022
7160516
tests: fix test for root typename with defer
bnjjj Dec 2, 2022
ad1d075
tests: only keep the test
bnjjj Dec 2, 2022
0d0d385
fix: root typename with defer
bnjjj Dec 12, 2022
2cd855c
Merge branch 'dev' into bnjjj/fix_1677
Dec 12, 2022
1b679ee
tests: add test with typename and defer
bnjjj Dec 12, 2022
ba258f9
tests: add test with typename and defer
bnjjj Dec 12, 2022
6f928aa
Merge branch 'dev' of github.com:apollographql/router into bnjjj/fix_…
bnjjj Dec 13, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions NEXT_CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,31 @@ in a previous payload.

By [@Geal](https://github.com/geal) in https://github.com/apollographql/router/pull/2184

### Return root `__typename` when parts of a query with deferred fragment ([Issue #1677](https://github.com/apollographql/router/issues/1677))

With this query:

```graphql
{
__typename
fast
...deferedFragment @defer
}

fragment deferedFragment on Query {
slow
}
```

You will received first response chunk:

```json
{"data":{"__typename": "Query", "fast":0},"hasNext":true}
```

By [@bnjjj](https://github.com/bnjjj) in https://github.com/apollographql/router/pull/2188


### wait for opentelemetry tracer provider to shutdown ([PR #2191](https://github.com/apollographql/router/pull/2191))

When we drop Telemetry we spawn a thread to perform the global opentelemetry trace provider shutdown. The documentation of this function indicates that "This will invoke the shutdown method on all span processors. span processors should export remaining spans before return". We should give that process some time to complete (5 seconds currently) before returning from the `drop`. This will provide more opportunity for spans to be exported.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
---
source: apollo-router/src/services/supergraph_service.rs
expression: stream.next_response().await.unwrap()
---
{
"hasNext": false,
"incremental": [
{
"data": {
"name": null
},
"path": [
"currentUser",
"activeOrganization",
"suborga",
0
]
},
{
"data": {
"name": "A"
},
"path": [
"currentUser",
"activeOrganization",
"suborga",
1
]
},
{
"data": {
"name": null
},
"path": [
"currentUser",
"activeOrganization",
"suborga",
2
]
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
---
source: apollo-router/src/services/supergraph_service.rs
expression: res
---
{
"data": {
"__typename": "Query",
"currentUser": {
"activeOrganization": {
"id": "0",
"suborga": [
{
"id": "1"
},
{
"id": "2"
},
{
"id": "3"
}
]
}
}
},
"hasNext": true
}
134 changes: 134 additions & 0 deletions apollo-router/src/services/supergraph_service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -885,6 +885,140 @@ mod tests {
insta::assert_json_snapshot!(stream.next_response().await.unwrap());
}

#[tokio::test]
async fn root_typename_with_defer() {
let subgraphs = MockedSubgraphs([
("user", MockSubgraph::builder().with_json(
serde_json::json!{{"query":"{currentUser{activeOrganization{__typename id}}}"}},
serde_json::json!{{"data": {"currentUser": { "activeOrganization": { "__typename": "Organization", "id": "0" } }}}}
).build()),
("orga", MockSubgraph::builder().with_json(
serde_json::json!{{
"query":"query($representations:[_Any!]!){_entities(representations:$representations){...on Organization{suborga{__typename id}}}}",
"variables": {
"representations":[{"__typename": "Organization", "id":"0"}]
}
}},
serde_json::json!{{
"data": {
"_entities": [{ "suborga": [
{ "__typename": "Organization", "id": "1"},
{ "__typename": "Organization", "id": "2"},
{ "__typename": "Organization", "id": "3"},
] }]
},
}}
)
.with_json(
serde_json::json!{{
"query":"query($representations:[_Any!]!){_entities(representations:$representations){...on Organization{name}}}",
"variables": {
"representations":[
{"__typename": "Organization", "id":"1"},
{"__typename": "Organization", "id":"2"},
{"__typename": "Organization", "id":"3"}

]
}
}},
serde_json::json!{{
"data": {
"_entities": [
{ "__typename": "Organization", "id": "1"},
{ "__typename": "Organization", "id": "2", "name": "A"},
{ "__typename": "Organization", "id": "3"},
]
}
}}
).build())
].into_iter().collect());

let service = TestHarness::builder()
.configuration_json(serde_json::json!({"include_subgraph_errors": { "all": true } }))
.unwrap()
.schema(SCHEMA)
.extra_plugin(subgraphs)
.build()
.await
.unwrap();

let request = supergraph::Request::fake_builder()
.header("Accept", "multipart/mixed; deferSpec=20220824")
.query(
"query { __typename currentUser { activeOrganization { id suborga { id ...@defer { name } } } } }",
)
.build()
.unwrap();

let mut stream = service.oneshot(request).await.unwrap();
let res = stream.next_response().await.unwrap();
assert_eq!(
res.data.as_ref().unwrap().get("__typename"),
Some(&serde_json_bytes::Value::String("Query".into()))
);
insta::assert_json_snapshot!(res);

insta::assert_json_snapshot!(stream.next_response().await.unwrap());
}

#[tokio::test]
async fn root_typename_with_defer_in_defer() {
let subgraphs = MockedSubgraphs([
("user", MockSubgraph::builder().with_json(
serde_json::json!{{"query":"{currentUser{activeOrganization{__typename id}}}"}},
serde_json::json!{{"data": {"currentUser": { "activeOrganization": { "__typename": "Organization", "id": "0" } }}}}
).build()),
("orga", MockSubgraph::builder().with_json(
serde_json::json!{{
"query":"query($representations:[_Any!]!){_entities(representations:$representations){...on Organization{suborga{__typename id name}}}}",
"variables": {
"representations":[{"__typename": "Organization", "id":"0"}]
}
}},
serde_json::json!{{
"data": {
"_entities": [{ "suborga": [
{ "__typename": "Organization", "id": "1"},
{ "__typename": "Organization", "id": "2", "name": "A"},
{ "__typename": "Organization", "id": "3"},
] }]
},
}}
).build())
].into_iter().collect());

let service = TestHarness::builder()
.configuration_json(serde_json::json!({"include_subgraph_errors": { "all": true } }))
.unwrap()
.schema(SCHEMA)
.extra_plugin(subgraphs)
.build()
.await
.unwrap();

let request = supergraph::Request::fake_builder()
.header("Accept", "multipart/mixed; deferSpec=20220824")
.query(
"query { ...@defer { __typename currentUser { activeOrganization { id suborga { id name } } } } }",
)
.build()
.unwrap();

let mut stream = service.oneshot(request).await.unwrap();
let _res = stream.next_response().await.unwrap();
let res = stream.next_response().await.unwrap();
assert_eq!(
res.incremental
.get(0)
.unwrap()
.data
.as_ref()
.unwrap()
.get("__typename"),
Some(&serde_json_bytes::Value::String("Query".into()))
);
}

#[tokio::test]
async fn query_reconstruction() {
let schema = r#"schema
Expand Down
26 changes: 19 additions & 7 deletions apollo-router/src/spec/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ impl Query {
) -> Vec<Path> {
let data = std::mem::take(&mut response.data);
if let Some(Value::Object(mut input)) = data {
let operation = self.operation(operation_name);
let original_operation = self.operation(operation_name);
if is_deferred {
if let Some(subselection) = &response.subselection {
// Get subselection from hashmap
Expand All @@ -123,6 +123,19 @@ impl Query {
errors: Vec::new(),
nullified: Vec::new(),
};
// Detect if root __typename is asked in the original query (the qp doesn't put root __typename in subselections)
// cf https://github.com/apollographql/router/issues/1677
let operation_kind_if_root_typename =
original_operation.and_then(|op| {
op.selection_set
.iter()
.any(|f| f.is_typename_field())
.then(|| *op.kind())
});
if let Some(operation_kind) = operation_kind_if_root_typename {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should check that the defer fragment is at the root, right? if you have query { something ...@defer { __typename id} } would "__typename": "Query" be added to the root of output?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes Check the tests. I added a similar test

output.insert(TYPENAME, operation_kind.as_str().into());
}

response.data = Some(
match self.apply_root_selection_set(
operation,
Expand Down Expand Up @@ -151,7 +164,7 @@ impl Query {
response.data = Some(Value::Object(Object::default()));
return vec![];
}
} else if let Some(operation) = operation {
} else if let Some(operation) = original_operation {
let mut output = Object::default();

let all_variables = if operation.variables.is_empty() {
Expand Down Expand Up @@ -211,9 +224,8 @@ impl Query {
schema: &Schema,
configuration: &Configuration,
) -> Result<Self, SpecError> {
let string = query.into();

let parser = apollo_parser::Parser::new(string.as_str())
let query = query.into();
let parser = apollo_parser::Parser::new(query.as_str())
.recursion_limit(configuration.server.experimental_parser_recursion_limit);
let tree = parser.parse();

Expand Down Expand Up @@ -248,7 +260,7 @@ impl Query {
.collect::<Result<Vec<_>, SpecError>>()?;

Ok(Query {
string,
string: query,
fragments,
operations,
subselections: HashMap::new(),
Expand Down Expand Up @@ -1099,7 +1111,7 @@ impl Operation {
&& self
.selection_set
.get(0)
.map(|s| matches!(s, Selection::Field {name, ..} if name.as_str() == TYPENAME))
.map(|s| s.is_typename_field())
.unwrap_or_default()
}

Expand Down
4 changes: 4 additions & 0 deletions apollo-router/src/spec/selection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,10 @@ impl Selection {
Ok(selection)
}

pub(crate) fn is_typename_field(&self) -> bool {
matches!(self, Selection::Field {name, ..} if name.as_str() == TYPENAME)
}

pub(crate) fn contains_error_path(&self, path: &[PathElement], fragments: &Fragments) -> bool {
match (path.get(0), self) {
(None, _) => true,
Expand Down