-
Notifications
You must be signed in to change notification settings - Fork 93
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: introspection SDL encoder #283
Conversation
Oh, and if you would like to read the docs on encoder in a little more UI friendly way: cd crates/sdl-encoder && cargo doc --open |
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.
I'd like to send this over for a first pass of reviews. I've left a few comments about certain items. If there is anything that feels like you'd like to talk over zoom, please let me know.
You can run this work with cargo run graph introspect /url/of/graph/to/introspect
. For example this one cargo run graph introspect https://api.react-finland.fi/graphql
gives you a nice (non panic
ed) SDL. As noted in one of the comments, this only so far does ofType encoding for up-to [[Type!]!]!
.
A couple of things here -- not all things have encodings quite yet. Things that I am aware so far that I need to encode: directives
, arguments (i.e. (includeDeprecated: Boolean = false)
in fields(includeDeprecated: Boolean = false): [__Field!]
), union
, interface
and being able to provide default arguments. Union
, Interface
, and (I think) directives
are incredibly straightforward. I have to think a tad more about arguments and defaults though. If this list is missing something else, please let me know.
impl TryFrom<Introspection> for Schema { | ||
type Error = &'static str; | ||
|
||
fn try_from(src: Introspection) -> Result<Self, Self::Error> { |
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.
A note on this, and the lack of a constructor method on Schema.
Because the Introspection response may or may not have a malformed result, we need to make sure construction of Schema
is allowed to fail. None of the usual new
, with_
, and from_
constructor methods are allowed to fail, so our by-the-books way of constructing a Schema is using a TryFrom
trait implementation.
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.
TIL new
is not supposed to be fallible!! I... might need to update a library or two 😆
#[cfg(test)] | ||
mod tests { | ||
#[test] | ||
fn it_build_simple_schema() {} |
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.
This is empty so far, unfortunately. I am thinking of a good way to be able to run 10 or so overall tests for overall schema encoding. The problem is that I need the actual Introspection
type that I think I can literally only get by using and making queries to the graphql_client, which is really quite bulky. If I can't come up with anything smarter, that's what I'll end up doing, I think.
Help: If you can think of another clever way, please let me know!
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.
Couldn't you manually execute introspection queries and store the results in files?
(With the old apollo
CLI, you could get those by running something like apollo schema:download --endpoint=http://localhost:8080/graphql schema.json
.) Or am I misunderstanding the issue?
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.
What you're suggesting is not possible as Schema
is currently created for an IntrospectionResult
. .json
file will not work. A .json
file would have worked if IntrospectionResult
had a Serialize
implementation, but this implementation doesn't exist and the following code does not work:
let file = File::open("src/introspection/introspection.json")
.expect("File should be able to be opened.");
let result: IntrospectionResult =
serde_json::from_reader(file).expect("File is not a proper JSON.");
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.
I don't understand. Since executing an introspection query against a server would return the same JSON contents, and we are able to load those into an IntrospectionResult
, why is this different?
@@ -0,0 +1,292 @@ | |||
//! SDL Encoder provides methods to serialise a GraphQL Schema. |
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.
This crate is what does the actual SDL creation. It's divided into smaller mod
s based on various types we can encounter in a Schema, plus two convenience impls: Field
and FieldType
. Overall, we are trying to do the simplest thing possible -- create a buffer, keep adding to it as we iterate over introspection result, and at the end call .finish
to be able to get the final string. All individual types and helpers have a Display implementation which make this whole thing really straightforward.
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.
I think this is still missing support for interfaces, unions, field arguments, and directives?
@martijnwalraven, yes, I explicitly pointed it out in the last paragraph of the comment here. Although it's no longer true for directives, as I wrote the implementation for that this morning. |
This introduces the concept of a SDL encoder/serialiser that will be used when doing a code generation pass on an Introspection Result.
Each type and schem defs can have a field. A field is a key value pair describing a field and its type, which can be nullable, or a vector, and can have a default. This commit also introduces a Display implementation for Field and applicable tests.
FieldType encompasses the idea that type name can be infinitely recurssed in a Nullable or a NonNullable List. To accommodate that we introduce a new Enum that references itself in a FieldType::List variant.
This commit adds an ability to add input objects to the overarching SDL schema.
This commit creates a new function on the introspetion schema's impl to parse the incoming introspection result and create an SDL with the sdl-encoder.
This commit creates a new function on the introspetion schema's impl to parse the incoming introspection result and create an SDL with the sdl-encoder.
This allows for encoding of enum types that will look like this: ```js enum VeryGoodCats { NORI CHASHU } ```
Creates encoding for scalar type: `scalar NameOfScalar`.
1. Iterate over the first level of NON_NULL Because the ResponseData creates its own types and does not create types we can recurse over in an introspection response, we have to manually unwrap Introspection's `ofType`. This commit unwraps the initial NON_NULL. 2. Refer to enum's variants as 'values' 3. clarify comment for object type/interface arguments
This commit adds Directive encodings. So far no arguments can be passed, but we are able to accept description and locations. The output is as follows: ```js directive @CacheControl on FIELD_DEFINITION | INPUT_FIELD_DEFINITION ```
Since all of the OfType types are unique in Introspection Response, **and** we want to be able to recurse over many nested versions of them, let's create an OfType Trait that can be implemented for all underlying OfType types. To make the trait implementations a bit easier, we are also creating a macro.
Unions are now encoded into appropriate SDL format: `union Cat = NORI | CHASHU`, for example.
This commit creates an Interface type, as well as allow to specify if a specific type implements an interface. This means we are able to create: ```js interface Meal { main: String """ Cat's post meal snack. """ snack: [String!]! """ Does cat get a pat after meal? """ pats: Boolean } ``` , as well as: ```js type Dinner implements Meal { } ```
This commit adds a `deprecated` and `deprecation_reason` to the Field struct. This lets each field to be deprecated and encoded with, for example: ```js cat: [SpaceProgram] @deprecated(reason: "Cats are no longer sent to space.") ```
adds NonNull as an enum variant for a FieldType. This makes it easier to recurse and create nested non-nullable field values.
Allows for various fields to have arguments. To make it easier to iterate over the Introspection Response, we do not differentiate between the various `__TypeKind`s that are _able_ to have an argument, so all `__TypeKind`s with fields will check for an `args` field.
FieldType was previously the only possible value a type could have. This commit introduces EnumValue that is applicable for EnumTypes, as well as renames FieldType to FieldValue to standardise Value naming.
This commit allows to set default values to Fields and InputValues. It also renames FieldArguments to InputValues to align with the graphql spec.
To align a bit closer on descriptions in Studio, we are now also multilining descriptions only when a new line symbol is present in the description.
This commit ensures that all structs and enums in sdl_encoder are documented for docs.rs. It also cleans up struct and parameter names to be as consistent as possible with GraphQL specification.
if self.is_deprecated { | ||
write!(f, " @deprecated")?; | ||
// Just in case deprecated directive is ever used without a reason, | ||
// let's properly unwrap this Option. | ||
if let Some(reason) = &self.deprecation_reason { | ||
write!(f, "(reason: \"{}\")", reason)?; | ||
} | ||
} |
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.
Maybe the logic for formatting @deprecated
could be extracted and reused in other places?
Make sure encoder types are aligned to the spec: - InputObjects now have InputFields instead of using Fields - FieldValue is renamed to Type_ to align with graphql spec - Type_'s named type is now referred to as `NamedType`
Add the ability to pass through headers, similarly to how curl works with `--header` or `-H`. ```console # single headers rover graph introspect http://localhost:4000 --header auth:hello rover graph introspect http://localhost:4000 -H auth:hello rover graph introspect http://localhost:4000 -H "auth:hello" rover graph introspect http://localhost:4000 --header auth:"hello" rover graph introspect http://localhost:4000 --header "auth:value with spaces" rover graph introspect http://localhost:4000 --header auth:"value with spaces" # I think reqwest is stripping leading whitespace from header values automatically (which is what we'd want), # even though with the current implementation, leading whitespaces are added to the header map rover graph introspect http://localhost:4000 --header "auth: value with leading spaces" # multiple headers rover graph introspect http://localhost:4000 --header auth:hello another:header rover graph introspect http://localhost:4000 --header auth:hello --header another:header rover graph introspect http://localhost:4000 --header auth:hello -H another:header rover graph introspect http://localhost:4000 --header auth:hello -H "another-header:with spaces" ```
Schema Definition has three possible fields: query, mutation and subscription. Create those fields directly with schema_def.query(), schema_def.mutation() etc. We encode Schema Definition depending on whether or not we have mutations/subscriptions. If either of those is present, record all possible values if there are any. If neither mutation or subscription is present, record query if its value is something other than `Query`.
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.
✔️
Description
As a first step of working with an Introspection result and creating an SDL, I am introducing a encoder that produces the correct output as part of a code generation step.
As part of this work, I am also aiming to establish a data flow of how an SDL will be produced, and so far it's as follows:
crates/rover-client/introspect.rs
)crates/rover-client/introspect.rs
)graphql_client
already returns a parsed result (i.e. we have some types we can work with), we can create an intermediary representation of schema that will be used to do SDL code generation. (a very smol amount of work for this is done in `crates/rover-client/schema.rs).crates/rover-client/schema.rs
traverses thetypes
anddirectives
received from Introspection Result. It then uses teh SDL encoder to create an SDL string.crates/sdl-encoder
which is introduced by this PR. This is responsible to serialising the output.Running this PR
Please run introspection on your favourite endpoints by checking out this branch and running:
cargo run graph introspect http/to/endpoint
You may also wish to run the docs on the SDL encoder:
cd crates/sdl-encoder && cargo doc --open
About
sdl-encoder
Design for this was influenced by
wasm-encoder
. This tool is doing a similar job of creating specific pieces of the final formatted output, and I find the library to be a well executed example to follow.The encoder is meant to be used in
crates/rover-client/schema.rs
as we build out the functionality of an intermediate schema, and work with the parsed input provided by IntrospectionResult.Most GraphQL Introspection response types are represented with this encoder. The exception is the
__InputObject
using the prescribed by the spec__InputValue
type. As we do not have an AST to visit to encode the schema, all types that have fields currently use__Field
type as their input.The encoder is written in a way for us to use and expand in the future to encode schemas that are not just part of Introspection response. It will require some tweaking, such as allowing Types to have other directives other than deprecated, but this can be added in a very straightforward manner when the time comes.
closes #180
closes #171