Add validation compilation phase to ensure federated schema validity

kdawgwilk commented 3 years ago

It would be great if we had a schema phase that validated the federated aspects of the schema to give end users more insight into why the schema may be an invalid federated schema

kdawgwilk commented 2 years ago

I would refer mostly to the federation spec to determine which kinds of validations we want to enforce are in place https://www.apollographql.com/docs/federation/federation-spec an example would be that @key uses fields that actually exist on the GQL type so you can't have something like this

object :product do
  key_fields("upc")
  field :id, non_null(:id)
end

Since the field upc doesnt exist on the type. Another validation we could do is if a type is marked @extends the @key(fields: "upc") must also be @external

An example compliation validation phase you can reference would be this one from absinthe https://github.com/absinthe-graphql/absinthe/blob/master/lib/absinthe/phase/schema/validation/directives_must_be_valid.ex#L1

scottming commented 2 years ago

I'll take this.

kzlsakal commented 2 years ago

I didn't want to open a new issue since this one covers it. If we add duplicate directives for a field like @deprecate or @tag, it will break the composition. It will help to have a validation for making sure there are no duplicate directives for a field.

scottming commented 2 years ago

Do we need to verify anything else now?

maartenvanvliet commented 2 years ago

I didn't want to open a new issue since this one covers it. If we add duplicate directives for a field like @deprecate or @tag, it will break the composition. It will help to have a validation for making sure there are no duplicate directives for a field.

There's a validation in Absinthe to ensure that directives not marked as repeatable will return an error when applied multiple times. How does this break for you?

kdawgwilk commented 2 years ago

@maartenvanvliet maybe we run our phases after that validation phase? I wouldn't think we do but we are also doing some weird stuff like hand crafting the directive blueprint structs ourselves because of the lack of support in absinthe which may play into this

kzlsakal commented 2 years ago

I will try to replicate this in an example repo to demonstrate it when I have a chance.

I know that it doesn’t get caught by Apollo’s managed schema checks either, but it breaks IntrospectAndCompose of Federation Gateway since 2.0.

kzlsakal commented 2 years ago

I pushed the branch where I replicated the issue here: https://github.com/kzlsakal/federation_poc/tree/break-composition

If you start the products service it will compile and start successfully, but then starting the gateway will produce the following error:

IntrospectAndCompose failed to update supergraph with the following error: A valid schema couldn't be composed. The following composition errors were found:
        [products] The directive "@deprecated" can only be used once at this location.
Error: A valid schema couldn't be composed. The following composition errors were found:
        [products] The directive "@deprecated" can only be used once at this location.

Here's how it looks like in the schema (mix absinthe.federation.schema.sdl --schema ProductsWeb.Schema):

type Product @key(fields: "upc") {
  upc: String!
  name: String!
  price: Int @deprecated(reason: "This field is truly deprecated") @deprecated(reason: "This field is deprecated")
}

Here's another example branch with duplicate directives causing the same error on the gateway: https://github.com/kzlsakal/federation_poc/tree/break-composition-2

IntrospectAndCompose failed to update supergraph with the following error: A valid schema couldn't be composed. The following composition errors were found:
        [products] The directive "@requires" can only be used once at this location.
Error: A valid schema couldn't be composed. The following composition errors were found:
        [products] The directive "@requires" can only be used once at this location.

And here's how it looks like in the schema definition:

type Product @key(fields: "upc") {
  upc: String!
  name: String!
  price: Int @requires(fields: ["bar"]) @requires(fields: ["foo"])
}

kdawgwilk commented 2 years ago

Absinthe issue to add validation for this https://github.com/absinthe-graphql/absinthe/issues/1177

maartenvanvliet commented 1 year ago

Absinthe 1.7.1 is released with the fix for this. When I run the break-composition branch link above I get the following message:

== Compilation error in file lib/products_web/schema.ex ==
** (Absinthe.Schema.Error) Compilation failed:
---------------------------------------
## Locations
Column 0, Line 20

Directive `deprecated' cannot be applied repeatedly.
---------------------------------------
## Locations
Column 0, Line 19

Directive `deprecated' cannot be applied repeatedly.

So Absinthe now handles this correctly

DivvyPayHQ / absinthe_federation

Add validation compilation phase to ensure federated schema validity #19