From eb9cda4c803d2cf0bb1d8067a995900b2c0b2ad9 Mon Sep 17 00:00:00 2001 From: Sunli Date: Thu, 18 Nov 2021 19:37:10 +0800 Subject: [PATCH] Remove skipped fields from the document before executing the query. --- CHANGELOG.md | 4 ++ src/context.rs | 49 +------------------- src/look_ahead.rs | 9 ---- src/resolver_utils/container.rs | 15 ------ src/schema.rs | 71 ++++++++++++++++++++++++----- src/subscription.rs | 3 -- tests/directive.rs | 81 +++++++++------------------------ 7 files changed, 85 insertions(+), 147 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b15800be..1373db11 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## Unreleased + +- Remove skipped fields from the document before executing the query. + ## [3.0.4] 2021-11-18 - Remove `OutputJson` because `Json` can replace it. diff --git a/src/context.rs b/src/context.rs index 0c65bf9a..a236b9f8 100644 --- a/src/context.rs +++ b/src/context.rs @@ -14,7 +14,7 @@ use serde::Serialize; use crate::extensions::Extensions; use crate::parser::types::{ - Directive, Field, FragmentDefinition, OperationDefinition, Selection, SelectionSet, + Field, FragmentDefinition, OperationDefinition, Selection, SelectionSet, }; use crate::schema::SchemaEnv; use crate::{ @@ -497,42 +497,6 @@ impl<'a, T> ContextBase<'a, T> { .node .into_const_with(|name| self.var_value(&name, pos)) } - - #[doc(hidden)] - pub fn is_ifdef(&self, directives: &[Positioned]) -> bool { - directives - .iter() - .any(|directive| directive.node.name.node == "ifdef") - } - - #[doc(hidden)] - pub fn is_skip(&self, directives: &[Positioned]) -> ServerResult { - for directive in directives { - let include = match &*directive.node.name.node { - "skip" => false, - "include" => true, - _ => continue, - }; - - let condition_input = directive - .node - .get_argument("if") - .ok_or_else(|| ServerError::new(format!(r#"Directive @{} requires argument `if` of type `Boolean!` but it was not provided."#, if include { "include" } else { "skip" }),Some(directive.pos)))? - .clone(); - - let pos = condition_input.pos; - let condition_input = self.resolve_input_value(condition_input)?; - - if include - != ::parse(Some(condition_input)) - .map_err(|e| e.into_server_error(pos))? - { - return Ok(true); - } - } - - Ok(false) - } } impl<'a> ContextBase<'a, &'a Positioned> { @@ -734,17 +698,6 @@ impl<'a> Iterator for SelectionFieldsIter<'a> { loop { let it = self.iter.last_mut()?; let item = it.next(); - if let Some(item) = item { - // ignore any items that are skipped (i.e. @skip/@include) - if self - .context - .is_skip(&item.node.directives()) - .unwrap_or(false) - { - // TODO: should we throw errors here? they will be caught later in execution and it'd cause major backwards compatibility issues - continue; - } - } match item { Some(selection) => match &selection.node { diff --git a/src/look_ahead.rs b/src/look_ahead.rs index a142221c..440ed9e2 100644 --- a/src/look_ahead.rs +++ b/src/look_ahead.rs @@ -110,15 +110,6 @@ fn filter<'a>( context: &'a Context<'a>, ) { for item in &selection_set.items { - // doing this imperatively is a bit nasty, but using iterators would - // require a boxed return type (I believe) as its recusive - - // ignore any items that are skipped (i.e. @skip/@include) - if context.is_skip(&item.node.directives()).unwrap_or(false) { - // TODO: should we throw errors here? they will be caught later in execution and it'd cause major backwards compatibility issues - continue; - } - match &item.node { Selection::Field(field) => { if field.node.name.node == name { diff --git a/src/resolver_utils/container.rs b/src/resolver_utils/container.rs index d0dc3048..2364e25b 100644 --- a/src/resolver_utils/container.rs +++ b/src/resolver_utils/container.rs @@ -5,7 +5,6 @@ use indexmap::IndexMap; use crate::extensions::ResolveInfo; use crate::parser::types::Selection; -use crate::registry::MetaType; use crate::{Context, ContextSelectionSet, Name, OutputType, ServerError, ServerResult, Value}; /// Represents a GraphQL container object. @@ -139,10 +138,6 @@ impl<'a> Fields<'a> { root: &'a T, ) -> ServerResult<()> { for selection in &ctx.item.node.items { - if ctx.is_skip(&selection.node.directives())? { - continue; - } - match &selection.node { Selection::Field(field) => { if field.node.name.node == "__typename" { @@ -157,16 +152,6 @@ impl<'a> Fields<'a> { continue; } - if ctx.is_ifdef(&field.node.directives) { - if let Some(MetaType::Object { fields, .. }) = - ctx.schema_env.registry.types.get(T::type_name().as_ref()) - { - if !fields.contains_key(field.node.name.node.as_str()) { - continue; - } - } - } - self.0.push(Box::pin({ let ctx = ctx.clone(); async move { diff --git a/src/schema.rs b/src/schema.rs index 4544e300..536bb30c 100644 --- a/src/schema.rs +++ b/src/schema.rs @@ -8,8 +8,8 @@ use indexmap::map::IndexMap; use crate::context::{Data, QueryEnvInner}; use crate::extensions::{ExtensionFactory, Extensions}; use crate::model::__DirectiveLocation; -use crate::parser::parse_query; -use crate::parser::types::{DocumentOperations, OperationType}; +use crate::parser::types::{Directive, DocumentOperations, OperationType, Selection, SelectionSet}; +use crate::parser::{parse_query, Positioned}; use crate::registry::{MetaDirective, MetaInputValue, Registry}; use crate::resolver_utils::{resolve_container, resolve_container_serial}; use crate::subscription::collect_subscription_streams; @@ -17,7 +17,7 @@ use crate::types::QueryRoot; use crate::validation::{check_rules, ValidationMode}; use crate::{ BatchRequest, BatchResponse, CacheControl, ContextBase, InputType, ObjectType, OutputType, - QueryEnv, Request, Response, ServerError, SubscriptionType, ID, + QueryEnv, Request, Response, ServerError, SubscriptionType, Variables, ID, }; /// Schema builder @@ -312,13 +312,6 @@ where } }); - registry.add_directive(MetaDirective { - name: "ifdef", - description: Some("Directs the executor to query only when the field exists."), - locations: vec![__DirectiveLocation::FIELD], - args: Default::default(), - }); - // register scalars ::create_type_info(&mut registry); ::create_type_info(&mut registry); @@ -391,7 +384,7 @@ where extensions.attach_query_data(query_data.clone()); let request = extensions.prepare_request(request).await?; - let document = { + let mut document = { let query = &request.query; let fut_parse = async { parse_query(&query).map_err(Into::::into) }; futures_util::pin_mut!(fut_parse); @@ -454,7 +447,13 @@ where } }; - let (operation_name, operation) = operation.map_err(|err| vec![err])?; + let (operation_name, mut operation) = operation.map_err(|err| vec![err])?; + + // remove skipped fields + for fragment in document.fragments.values_mut() { + remove_skipped_selection(&mut fragment.node.selection_set.node, &request.variables); + } + remove_skipped_selection(&mut operation.node.selection_set.node, &request.variables); let env = QueryEnvInner { extensions, @@ -599,3 +598,51 @@ where self.execute_stream_with_session_data(request.into(), Default::default()) } } + +fn remove_skipped_selection(selection_set: &mut SelectionSet, variables: &Variables) { + fn is_skipped(directives: &[Positioned], variables: &Variables) -> bool { + for directive in directives { + let include = match &*directive.node.name.node { + "skip" => false, + "include" => true, + _ => continue, + }; + + if let Some(condition_input) = directive.node.get_argument("if") { + let value = condition_input + .node + .clone() + .into_const_with(|name| variables.get(&name).cloned().ok_or(())) + .unwrap_or_default(); + let value: bool = InputType::parse(Some(value)).unwrap_or_default(); + if include != value { + return true; + } + } + } + + false + } + + selection_set + .items + .retain(|selection| !is_skipped(selection.node.directives(), variables)); + + for selection in &mut selection_set.items { + selection.node.directives_mut().retain(|directive| { + directive.node.name.node != "skip" && directive.node.name.node != "include" + }); + } + + for selection in &mut selection_set.items { + match &mut selection.node { + Selection::Field(field) => { + remove_skipped_selection(&mut field.node.selection_set.node, variables); + } + Selection::FragmentSpread(_) => {} + Selection::InlineFragment(inline_fragment) => { + remove_skipped_selection(&mut inline_fragment.node.selection_set.node, variables); + } + } + } +} diff --git a/src/subscription.rs b/src/subscription.rs index 12f3a560..7257a7e9 100644 --- a/src/subscription.rs +++ b/src/subscription.rs @@ -43,9 +43,6 @@ pub(crate) fn collect_subscription_streams<'a, T: SubscriptionType + 'static>( streams: &mut Vec>, ) -> ServerResult<()> { for selection in &ctx.item.node.items { - if ctx.is_skip(selection.node.directives())? { - continue; - } match &selection.node { Selection::Field(field) => streams.push(Box::pin({ let ctx = ctx.clone(); diff --git a/tests/directive.rs b/tests/directive.rs index 3a922345..8ed815b7 100644 --- a/tests/directive.rs +++ b/tests/directive.rs @@ -12,20 +12,37 @@ pub async fn test_directive_skip() { } let schema = Schema::new(QueryRoot, EmptyMutation, EmptySubscription); - let resp = schema + let data = schema .execute( r#" - { + fragment A on QueryRoot { + value5: value @skip(if: true) + value6: value @skip(if: false) + } + + query { value1: value @skip(if: true) value2: value @skip(if: false) + ... @skip(if: true) { + value3: value + } + ... @skip(if: false) { + value4: value + } + ... A } "#, ) - .await; + .await + .into_result() + .unwrap() + .data; assert_eq!( - resp.data, + data, value!({ "value2": 10, + "value4": 10, + "value6": 10, }) ); } @@ -59,59 +76,3 @@ pub async fn test_directive_include() { }) ); } - -#[tokio::test] -pub async fn test_directive_ifdef() { - struct QueryRoot; - - #[Object] - impl QueryRoot { - pub async fn value1(&self) -> i32 { - 10 - } - } - - struct MutationRoot; - - #[Object] - impl MutationRoot { - pub async fn action1(&self) -> i32 { - 10 - } - } - - let schema = Schema::new(QueryRoot, MutationRoot, EmptySubscription); - let resp = schema - .execute( - r#" - { - value1 @ifdef - value2 @ifdef - } - "#, - ) - .await; - assert_eq!( - resp.data, - value!({ - "value1": 10, - }) - ); - - let resp = schema - .execute( - r#" - mutation { - action1 @ifdef - action2 @ifdef - } - "#, - ) - .await; - assert_eq!( - resp.data, - value!({ - "action1": 10, - }) - ); -}