From 73f5a377771a8e36ab8d955b6c2af86146963696 Mon Sep 17 00:00:00 2001 From: David Pedersen Date: Sat, 5 Oct 2019 13:51:49 +0200 Subject: [PATCH 1/2] Make `QueryTrail` part of this crate's public interface Fixes #67 --- CHANGELOG.md | 2 +- .../code_gen_pass/gen_query_trails.rs | 131 +++++++++++------- juniper-from-schema/src/lib.rs | 91 ++++++++++-- .../tests/converting_query_trails_test.rs | 4 +- 4 files changed, 168 insertions(+), 60 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8eb92f1..87c5f3b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,7 +12,7 @@ for Rust libraries in [RFC #1105](https://github.com/rust-lang/rfcs/blob/master/ ### Changed -N/A +- The `QueryTrail` type is now part of this library rather than being emitted as part of the generated code. This was done so other libraries could make sure of the type. If you're getting errors about missing methods adding `use crate::graphql_schema::query_trails::*` to you module should fix it. ### Removed diff --git a/juniper-from-schema-code-gen/src/ast_pass/code_gen_pass/gen_query_trails.rs b/juniper-from-schema-code-gen/src/ast_pass/code_gen_pass/gen_query_trails.rs index e705bbc..f05aefe 100644 --- a/juniper-from-schema-code-gen/src/ast_pass/code_gen_pass/gen_query_trails.rs +++ b/juniper-from-schema-code-gen/src/ast_pass/code_gen_pass/gen_query_trails.rs @@ -12,6 +12,8 @@ use syn::Ident; impl<'doc> CodeGenPass<'doc> { pub fn gen_query_trails(&mut self, doc: &'doc Document) { + let original_tokens = std::mem::replace(&mut self.tokens, quote! {}); + self.gen_query_trail(); let fields_map = build_fields_map(doc); @@ -37,64 +39,72 @@ impl<'doc> CodeGenPass<'doc> { } } } - } - fn gen_query_trail(&mut self) { - self.extend(quote! { - pub use juniper_from_schema::{Walked, NotWalked}; + let query_trail_tokens = &self.tokens; + self.tokens = quote! { + pub use juniper_from_schema::{Walked, NotWalked, QueryTrail, MakeQueryTrail}; + pub use self::query_trails::*; - /// A wrapper around a `juniper::LookAheadSelection` with methods for each possible child. + #original_tokens + + /// `QueryTrail` exntesion traits specific to the GraphQL schema /// - /// Generated by `juniper-from-schema`. - pub struct QueryTrail<'a, T, K> { - look_ahead: Option<&'a juniper::LookAheadSelection<'a, juniper::DefaultScalarValue>>, - node_type: std::marker::PhantomData, - walked: K, - } + /// Generated by juniper-from-schema + pub mod query_trails { + #![allow(unused_imports, dead_code, missing_docs)] - impl<'a, T> QueryTrail<'a, T, juniper_from_schema::NotWalked> { - /// Check if the trail is present in the query being executed - /// - /// Generated by `juniper-from-schema`. - pub fn walk(self) -> Option> { - match self.look_ahead { - Some(inner) => Some(QueryTrail { - look_ahead: Some(inner), - node_type: self.node_type, - walked: juniper_from_schema::Walked, - }), - None => None, - } - } - } + use super::*; - trait MakeQueryTrail<'a> { - fn make_query_trail(&'a self) -> QueryTrail<'a, T, juniper_from_schema::Walked>; + #query_trail_tokens } + }; + } - impl<'a> MakeQueryTrail<'a> for juniper::LookAheadSelection<'a, juniper::DefaultScalarValue> { - fn make_query_trail(&'a self) -> QueryTrail<'a, T, juniper_from_schema::Walked> { - QueryTrail { - look_ahead: Some(self), - node_type: std::marker::PhantomData, - walked: juniper_from_schema::Walked, - } - } + fn gen_query_trail(&mut self) { + self.extend(quote! { + use juniper_from_schema::{Walked, NotWalked, QueryTrail, MakeQueryTrail}; + + /// Convert from one type of `QueryTrail` to another. Used for converting interface and + /// union trails into concrete subtypes. + /// + /// This trait cannot live in juniper-from-schema itself because then we wouldn't be + /// able to implement it for `QueryTrail` in the user's code. That would result in + /// orphan instances. + /// + /// Generated by juniper-from-schema. + pub trait DowncastQueryTrail<'a, T> { + /// Perform the downcast. + /// + /// Generated by juniper-from-schema. + fn downcast(self) -> QueryTrail<'a, T, Walked>; } }) } fn gen_field_walk_methods(&mut self, obj: InternalQueryTrailNode<'_>) { let name = ident(&obj.name()); + let trait_name = ident(&format!("QueryTrail{}Extensions", obj.name())); let fields = obj.fields(); - let methods = fields - .iter() - .map(|field| self.gen_field_walk_method(field)) - .collect::>(); + + let mut method_signatures = vec![]; + let mut method_implementations = vec![]; + + for field in fields { + let FieldWalkMethod { + method_signature, + method_implementation, + } = self.gen_field_walk_method(field); + method_signatures.push(method_signature); + method_implementations.push(method_implementation); + } self.extend(quote! { - impl<'a, K> QueryTrail<'a, #name, K> { - #(#methods)* + pub trait #trait_name<'a> { + #(#method_signatures)* + } + + impl<'a, K> #trait_name<'a> for QueryTrail<'a, #name, K> { + #(#method_implementations)* } }); @@ -128,8 +138,8 @@ impl<'doc> CodeGenPass<'doc> { for type_ in destination_types { self.extend(quote! { - impl<'a> Into> for &QueryTrail<'a, #original_type_name, Walked> { - fn into(self) -> QueryTrail<'a, #type_, Walked> { + impl<'a> DowncastQueryTrail<'a, #type_> for &QueryTrail<'a, #original_type_name, Walked> { + fn downcast(self) -> QueryTrail<'a, #type_, Walked> { QueryTrail { look_ahead: self.look_ahead, node_type: std::marker::PhantomData, @@ -175,7 +185,7 @@ impl<'doc> CodeGenPass<'doc> { } } - fn gen_field_walk_method(&mut self, field: &Field) -> TokenStream { + fn gen_field_walk_method(&mut self, field: &Field) -> FieldWalkMethod { let field_type = type_name(&field.field_type); let (_, ty) = self.graphql_scalar_type_to_rust_type(&field_type, field.position); let field_type = ident(field_type.clone().to_camel_case()); @@ -185,28 +195,41 @@ impl<'doc> CodeGenPass<'doc> { let name = ident(&field.name.to_snake_case()); let string_name = &field.name.to_mixed_case(); - quote! { + let method_signature = quote! { /// Check if a scalar leaf node is queried for /// /// Generated by `juniper-from-schema`. - pub fn #name(&self) -> bool { + fn #name(&self) -> bool; + }; + + let method_implementation = quote! { + fn #name(&self) -> bool { use juniper::LookAheadMethods; self.look_ahead .and_then(|la| la.select_child(#string_name)) .is_some() } + }; + + FieldWalkMethod { + method_signature, + method_implementation, } } TypeKind::Type => { let name = ident(&field.name.to_snake_case()); let string_name = &field.name.to_mixed_case(); - quote! { + let method_signature = quote! { /// Walk the trail into a field. /// /// Generated by `juniper-from-schema`. - pub fn #name(&self) -> QueryTrail<'a, #field_type, juniper_from_schema::NotWalked> { + fn #name(&self) -> QueryTrail<'a, #field_type, juniper_from_schema::NotWalked>; + }; + + let method_implementation = quote! { + fn #name(&self) -> QueryTrail<'a, #field_type, juniper_from_schema::NotWalked> { use juniper::LookAheadMethods; let child = self.look_ahead.and_then(|la| la.select_child(#string_name)); @@ -217,12 +240,22 @@ impl<'doc> CodeGenPass<'doc> { walked: juniper_from_schema::NotWalked, } } + }; + + FieldWalkMethod { + method_signature, + method_implementation, } } } } } +struct FieldWalkMethod { + method_signature: TokenStream, + method_implementation: TokenStream, +} + #[derive(Clone)] struct HashFieldByName<'a>(&'a Field); diff --git a/juniper-from-schema/src/lib.rs b/juniper-from-schema/src/lib.rs index 6244e08..b9e66b2 100644 --- a/juniper-from-schema/src/lib.rs +++ b/juniper-from-schema/src/lib.rs @@ -739,9 +739,33 @@ //! on all your types. This means the compiler will reject your code if you're checking for invalid //! fields. //! -//! Fields that return objects types (non scalar values) will also get a `QueryTrail` argument +//! Fields that return object types (non scalar values) will also get a `QueryTrail` argument //! besides the executor. //! +//! Since the `QueryTrail` type itself is defined in this crate (rather than being inserted into +//! your code) we cannot directly add methods for your GraphQL fields. Those methods have to be +//! added through ["extension traits"](http://xion.io/post/code/rust-extension-traits.html). So if +//! you see an error like +//! +//! ```text +//! | trail.foo(); +//! | ^^^ method not found in `&juniper_from_schema::QueryTrail<'a, User, juniper_from_schema::Walked>` +//! | +//! = help: items from traits can only be used if the trait is in scope +//! help: the following trait is implemented but not in scope, perhaps add a `use` for it: +//! | +//! 2 | use crate::graphql_schema::query_trails::QueryTrailUserExtensions; +//! | +//! ``` +//! +//! Then adding `use crate::graphql_schema::query_trails::*` to you module should fix it. This is +//! necessary because all the extention traits are generated inside a module called `query_trails`. +//! This is done so you can glob import the `QueryTrail` extension traits without glob importing +//! everything from your GraphQL schema. +//! +//! If you just want everything from the schema `use crate::graphql_schema::*` will also bring in +//! the extension traits. +//! //! [N+1 query bugs]: https://secure.phabricator.com/book/phabcontrib/article/n_plus_one/ //! [look ahead api]: https://docs.rs/juniper/0.11.1/juniper/struct.LookAheadSelection.html //! @@ -883,7 +907,8 @@ //! specific to that library._ //! //! If you have a `QueryTrail<'a, T, Walked>` where `T` is an interface or union type you can use -//! `.into()` to convert that `QueryTrail` into one of the implementors of the interface or union. +//! `.downcast()` to convert that `QueryTrail` into one of the implementors of the interface or +//! union. //! //! Example: //! @@ -941,8 +966,8 @@ //! trail: &QueryTrail<'_, SearchResult, juniper_from_schema::Walked>, //! query: String, //! ) -> FieldResult<&Vec> { -//! let article_trail: QueryTrail<'_, Article, Walked> = trail.into(); -//! let tweet_trail: QueryTrail<'_, Tweet, Walked> = trail.into(); +//! let article_trail: QueryTrail<'_, Article, Walked> = trail.downcast(); +//! let tweet_trail: QueryTrail<'_, Tweet, Walked> = trail.downcast(); //! //! // ... //! # unimplemented!() @@ -997,9 +1022,8 @@ //! That type doesn't work with `preload_users`. So we have to convert our `QueryTrail<'a, //! SearchResult, Walked>` into `QueryTrail<'a, User, Walked>`. //! -//! This can be done [`std::convert::Into`](https://doc.rust-lang.org/std/convert/trait.Into.html) -//! which automatically gets implemented for interface and union query trails. See above for an -//! example. +//! This can be done calling [`.downcast()`] which automatically gets implemented for interface and +//! union query trails. See above for an example. //! //! # Customizing the error type //! @@ -1130,9 +1154,18 @@ //! [feature]: https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#choosing-features //! [rustfmt]: https://github.com/rust-lang/rustfmt -#![deny(unused_imports, dead_code, unused_variables, unused_must_use)] +#![deny( + missing_docs, + unused_imports, + dead_code, + unused_variables, + unused_must_use +)] #![doc(html_root_url = "https://docs.rs/juniper-from-schema/0.3.2")] +use juniper::{DefaultScalarValue, LookAheadSelection}; +use std::marker::PhantomData; + pub use juniper_from_schema_code_gen::{graphql_schema, graphql_schema_from_file}; /// A type used to parameterize `QueryTrail` to know that `walk` has been called. @@ -1141,6 +1174,48 @@ pub struct Walked; /// A type used to parameterize `QueryTrail` to know that `walk` has *not* been called. pub struct NotWalked; +/// A wrapper around a `juniper::LookAheadSelection` with methods for each possible child. +pub struct QueryTrail<'a, T, K> { + #[doc(hidden)] + pub look_ahead: Option<&'a LookAheadSelection<'a, DefaultScalarValue>>, + #[doc(hidden)] + pub node_type: PhantomData, + #[doc(hidden)] + pub walked: K, +} + +impl<'a, T> QueryTrail<'a, T, NotWalked> { + /// Check if the trail is present in the query being executed + pub fn walk(self) -> Option> { + match self.look_ahead { + Some(inner) => Some(QueryTrail { + look_ahead: Some(inner), + node_type: self.node_type, + walked: Walked, + }), + None => None, + } + } +} + +/// Make a `QueryTrail` from something, normally a `juniper::LookAheadSelection`. +/// +/// You normally shouldn't need to worry about this trait. +pub trait MakeQueryTrail<'a> { + #[allow(missing_docs)] + fn make_query_trail(&'a self) -> QueryTrail<'a, T, Walked>; +} + +impl<'a> MakeQueryTrail<'a> for LookAheadSelection<'a, DefaultScalarValue> { + fn make_query_trail(&'a self) -> QueryTrail<'a, T, Walked> { + QueryTrail { + look_ahead: Some(self), + node_type: PhantomData, + walked: Walked, + } + } +} + #[cfg(test)] mod test { #[allow(unused_imports)] diff --git a/juniper-from-schema/tests/converting_query_trails_test.rs b/juniper-from-schema/tests/converting_query_trails_test.rs index aa13e11..0775a2c 100644 --- a/juniper-from-schema/tests/converting_query_trails_test.rs +++ b/juniper-from-schema/tests/converting_query_trails_test.rs @@ -37,7 +37,7 @@ impl QueryFields for Query { trail: &QueryTrail<'a, Entity, Walked>, ) -> FieldResult> { verify_entity_query_trail(trail); - verify_user_query_trail(&trail.into()); + verify_user_query_trail(&trail.downcast()); Ok(vec![]) } @@ -49,7 +49,7 @@ impl QueryFields for Query { _query: String, ) -> FieldResult> { verify_search_result_query_trail(trail); - verify_user_query_trail(&trail.into()); + verify_user_query_trail(&trail.downcast()); Ok(vec![]) } From 3c6bd0c9001d7f8102dbc8fa9fd4dd4fbf072304 Mon Sep 17 00:00:00 2001 From: David Pedersen Date: Sat, 5 Oct 2019 14:58:26 +0200 Subject: [PATCH 2/2] Fix typos in docs --- .../src/ast_pass/code_gen_pass/gen_query_trails.rs | 2 +- juniper-from-schema/src/lib.rs | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/juniper-from-schema-code-gen/src/ast_pass/code_gen_pass/gen_query_trails.rs b/juniper-from-schema-code-gen/src/ast_pass/code_gen_pass/gen_query_trails.rs index f05aefe..54ef465 100644 --- a/juniper-from-schema-code-gen/src/ast_pass/code_gen_pass/gen_query_trails.rs +++ b/juniper-from-schema-code-gen/src/ast_pass/code_gen_pass/gen_query_trails.rs @@ -47,7 +47,7 @@ impl<'doc> CodeGenPass<'doc> { #original_tokens - /// `QueryTrail` exntesion traits specific to the GraphQL schema + /// `QueryTrail` extension traits specific to the GraphQL schema /// /// Generated by juniper-from-schema pub mod query_trails { diff --git a/juniper-from-schema/src/lib.rs b/juniper-from-schema/src/lib.rs index b9e66b2..81168e6 100644 --- a/juniper-from-schema/src/lib.rs +++ b/juniper-from-schema/src/lib.rs @@ -978,7 +978,7 @@ //! ### Why is this useful? //! //! If you were do perform some kind of preloading of data you might have a function that inspects -//! a `QueryTrail` and load the necessary data from a database. Such a function could look like +//! a `QueryTrail` and loads the necessary data from a database. Such a function could look like //! this: //! //! ```ignore @@ -1018,11 +1018,11 @@ //! } //! ``` //! -//! The method `QueryFields::field_seach` will receive a `QueryTrail<'a, SearchResult, Walked>`. +//! The method `QueryFields::field_search` will receive a `QueryTrail<'a, SearchResult, Walked>`. //! That type doesn't work with `preload_users`. So we have to convert our `QueryTrail<'a, //! SearchResult, Walked>` into `QueryTrail<'a, User, Walked>`. //! -//! This can be done calling [`.downcast()`] which automatically gets implemented for interface and +//! This can be done by calling `.downcast()` which automatically gets implemented for interface and //! union query trails. See above for an example. //! //! # Customizing the error type