Skip to content
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

feat(2911): add validations on resolvers for federation #2916

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
74 changes: 66 additions & 8 deletions src/core/blueprint/operators/apollo_federation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,59 @@
ApolloFederation, Config, ConfigModule, EntityResolver, Field, GraphQLOperationType, Resolver,
};
use crate::core::ir::model::IR;
use crate::core::mustache::Segment;
use crate::core::try_fold::TryFold;
use crate::core::valid::{Valid, Validator};
use crate::core::{config, Type};
use crate::core::{config, Mustache, Type};

pub struct CompileEntityResolver<'a> {
config_module: &'a ConfigModule,
entity_resolver: &'a EntityResolver,
operation_type: &'a GraphQLOperationType,
}

fn validate_expressions<'a>(
type_name: &str,
config_module: &ConfigModule,
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" {
return validate_iter(config_module, type_name, expr.iter().skip(1));
} else {
Valid::succeed(())

Check warning on line 33 in src/core/blueprint/operators/apollo_federation.rs

View check run for this annotation

Codecov / codecov/patch

src/core/blueprint/operators/apollo_federation.rs#L33

Added line #L33 was not covered by tests
}
} else {
Valid::succeed(())
}
})
.map(|_| ())
}

fn validate_iter<'a>(
config_module: &ConfigModule,
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) = config_module.types.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));

Check warning on line 58 in src/core/blueprint/operators/apollo_federation.rs

View check run for this annotation

Codecov / codecov/patch

src/core/blueprint/operators/apollo_federation.rs#L58

Added line #L58 was not covered by tests
}
Valid::succeed(())
})
.map(|_| ())
}

pub fn compile_entity_resolver(inputs: CompileEntityResolver<'_>) -> Valid<IR, String> {
let CompileEntityResolver { config_module, entity_resolver, operation_type } = inputs;
let mut resolver_by_type = HashMap::new();
Expand All @@ -30,17 +73,32 @@
// TODO: should be a proper way to run the validation both
// on types and fields
let field = &Field { type_of: Type::from(type_name.clone()), ..Default::default() };

// TODO: make this code reusable in other operators like call
let ir = match resolver {
// TODO: there are `validate_field` for field, but not for types
// implement validation as shared entity and use it for types
Resolver::Http(http) => compile_http(
config_module,
http,
// inner resolver should resolve only single instance of type, not a list
false,
),
Resolver::Http(http) => {
Valid::from_iter(http.query.iter(), |query| {
let mustache = Mustache::parse(&query.value);
validate_expressions(type_name, config_module, mustache.segments().iter())
})
.and_then(|_| {
let mustache = Mustache::parse(&http.path);
validate_expressions(type_name, config_module, mustache.segments().iter())
})
.and(validate_iter(
config_module,
type_name,
http.batch_key.iter(),
))
.and(compile_http(
config_module,
http,
// inner resolver should resolve only single instance of type, not
// a list
false,
))
}
Comment on lines +80 to +101
Copy link
Contributor

Choose a reason for hiding this comment

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

There is no need to parse templates and extract keys again because it's already done in the src/core/config/transformer/subgraph.rs transformer. Also there is handler to http resolver only.

Consider making changes there the way that after Keys are calculated we add additional validation step that will check that entries are part of the currently processed type

Resolver::Grpc(grpc) => compile_grpc(super::CompileGrpc {
config_module,
operation_type,
Expand Down
33 changes: 33 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,33 @@
---
source: tests/core/spec.rs
expression: errors
---
[
{
"message": "Invalid key at index 0: 'blogId' is not a field of 'Blog'",
"trace": [
"Query",
"_entities",
"_entities"
],
"description": null
},
{
"message": "Invalid key at index 1: 'blogId' is not a field of 'Blog'",
"trace": [
"Query",
"_entities",
"_entities"
],
"description": null
},
{
"message": "Invalid key at index 1: 'userId' is not a field of 'Blog'",
"trace": [
"Query",
"_entities",
"_entities"
],
"description": null
}
]
32 changes: 32 additions & 0 deletions tests/execution/apollo-federation-validation.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
---
error: true
---

# Apollo federation validation

```graphql @config
schema
@server(port: 8000)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
@server(port: 8000)
@server(port: 8000, enableFederation: true)

It's required now to enable the subgraph functionality

@upstream(baseURL: "http://jsonplaceholder.typicode.com", httpCache: 42, batch: {delay: 100}) {
query: Query
}

type Query {
post(id: Int!): Post @http(path: "/posts/{{.args.id}}")
}

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

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

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