Skip to content

Commit

Permalink
fix(2911): add validation in federation resolvers (#3102)
Browse files Browse the repository at this point in the history
  • Loading branch information
dekkku authored Nov 16, 2024
1 parent fb0974d commit d9baf52
Show file tree
Hide file tree
Showing 3 changed files with 152 additions and 10 deletions.
89 changes: 79 additions & 10 deletions src/core/config/transformer/subgraph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,22 +39,24 @@ impl Transform for Subgraph {
// if federation is disabled don't process the config
return Valid::succeed(config);
}

let config_types = config.types.clone();
let mut resolver_by_type = BTreeMap::new();

let valid = Valid::from_iter(config.types.iter_mut(), |(type_name, ty)| {
if let Some(resolver) = &ty.resolver {
resolver_by_type.insert(type_name.clone(), resolver.clone());

KeysExtractor::extract_keys(resolver).and_then(|fields| match fields {
Some(fields) => {
let key = Key { fields };

to_directive(key.to_directive()).map(|directive| {
ty.directives.push(directive);
})
}
None => Valid::succeed(()),
KeysExtractor::validate(&config_types, resolver, type_name).and_then(|_| {
KeysExtractor::extract_keys(resolver).and_then(|fields| match fields {
Some(fields) => {
let key = Key { fields };

to_directive(key.to_directive()).map(|directive| {
ty.directives.push(directive);
})
}
None => Valid::succeed(()),
})
})
} else {
Valid::succeed(())
Expand Down Expand Up @@ -192,6 +194,72 @@ fn combine_keys(v: Vec<Keys>) -> Keys {
struct KeysExtractor;

impl KeysExtractor {
fn validate_expressions<'a>(
type_name: &str,
type_map: &BTreeMap<String, config::Type>,
expr_iter: impl Iterator<Item = &'a Segment>,
) -> Valid<(), String> {
Valid::from_iter(expr_iter, |segment| {
if let Segment::Expression(expr) = segment {
if expr.len() > 1 && expr[0].as_str() == "value" {
Self::validate_iter(type_map, type_name, expr.iter().skip(1))
} else {
Valid::succeed(())
}
} else {
Valid::succeed(())
}
})
.unit()
}

fn validate_iter<'a>(
type_map: &BTreeMap<String, config::Type>,
current_type: &str,
fields_iter: impl Iterator<Item = &'a String>,
) -> Valid<(), String> {
let mut current_type = current_type;
Valid::from_iter(fields_iter.enumerate(), |(index, key)| {
if let Some(type_def) = type_map.get(current_type) {
if !type_def.fields.contains_key(key) {
return Valid::fail(format!(
"Invalid key at index {}: '{}' is not a field of '{}'",
index, key, current_type
));
}
current_type = type_def.fields[key].type_of.name();
} else {
return Valid::fail(format!("Type '{}' not found in config", current_type));
}
Valid::succeed(())
})
.unit()
}

fn validate(
type_map: &BTreeMap<String, config::Type>,
resolver: &Resolver,
type_name: &str,
) -> Valid<(), String> {
if let Resolver::Http(http) = resolver {
Valid::from_iter(http.query.iter(), |q| {
Self::validate_expressions(
type_name,
type_map,
Mustache::parse(&q.value).segments().iter(),
)
})
.and(Self::validate_expressions(
type_name,
type_map,
Mustache::parse(&http.url).segments().iter(),
))
.unit()
} else {
Valid::succeed(())
}
}

fn extract_keys(resolver: &Resolver) -> Valid<Option<String>, String> {
// TODO: add validation for available fields from the type
match resolver {
Expand Down Expand Up @@ -376,6 +444,7 @@ mod tests {
}
}

#[cfg(test)]
mod extractor {
use insta::assert_debug_snapshot;
use serde_json::json;
Expand Down
27 changes: 27 additions & 0 deletions tests/core/snapshots/apollo-federation-validation.md_error.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
---
source: tests/core/spec.rs
expression: errors
---
[
{
"message": "Type 'AccountType' not found in config",
"trace": [
"Account"
],
"description": null
},
{
"message": "Invalid key at index 0: 'blogId' is not a field of 'Blog'",
"trace": [
"Blog"
],
"description": null
},
{
"message": "Invalid key at index 1: 'userId' is not a field of 'Blog'",
"trace": [
"User"
],
"description": null
}
]
46 changes: 46 additions & 0 deletions tests/execution/apollo-federation-validation.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
---
error: true
---

# Apollo federation validation

```graphql @config
schema @server(port: 8000, enableFederation: true) @upstream(httpCache: 42, batch: {delay: 100}) {
query: Query
}

type Query {
post(id: Int!): Post @http(url: "http://jsonplaceholder.typicode.com/posts/{{.args.id}}")
}

type User @http(url: "http://jsonplaceholder.typicode.com/users/{{.value.blog.userId}}") {
id: Int!
username: String!
blog: Blog!
}

type Post
@http(
url: "http://jsonplaceholder.typicode.com/posts"
query: [{key: "id", value: "{{.value.id}}"}]
batchKey: ["blog", "blogId"]
) {
id: Int!
blog: Blog!
}

type Account
@http(
url: "http://jsonplaceholder.typicode.com/posts"
query: [{key: "id", value: "{{.value.type.id}}"}]
batchKey: ["blog", "blogId"]
) {
id: Int!
balance: Blog!
type: AccountType
}

type Blog @http(url: "http://jsonplaceholder.typicode.com/posts", query: [{key: "id", value: "{{.value.blogId}}"}]) {
id: Int!
}
```

1 comment on commit d9baf52

@github-actions
Copy link

Choose a reason for hiding this comment

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

Running 30s test @ http://localhost:8000/graphql

4 threads and 100 connections

Thread Stats Avg Stdev Max +/- Stdev
Latency 4.04ms 2.02ms 26.91ms 81.33%
Req/Sec 6.40k 0.90k 7.32k 93.83%

763950 requests in 30.01s, 3.83GB read

Requests/sec: 25456.82

Transfer/sec: 130.66MB

Please sign in to comment.