From b61ca14f5380f8cce9d63cf1e46fcd5a4d334c15 Mon Sep 17 00:00:00 2001 From: Magnus Hallin <mhallin@fastmail.com> Date: Sun, 11 Dec 2016 20:09:52 +0100 Subject: [PATCH] Pass the executor immutably through resolution The executor is now using internal mutability through RwLock to make it thread-safe. --- src/executor.rs | 24 ++++++++++++++---------- src/executor_tests/executor.rs | 2 +- src/lib.rs | 8 ++++---- src/macros/args.rs | 6 +++--- src/macros/enums.rs | 2 +- src/macros/interface.rs | 4 ++-- src/macros/object.rs | 6 +++--- src/macros/scalar.rs | 2 +- src/macros/tests/args.rs | 4 ++-- src/macros/union.rs | 2 +- src/schema/schema.rs | 10 +++++----- src/tests/schema.rs | 6 +++--- src/types/base.rs | 10 +++++----- src/types/containers.rs | 6 +++--- src/types/pointers.rs | 12 ++++++------ src/types/scalars.rs | 2 +- 16 files changed, 55 insertions(+), 51 deletions(-) diff --git a/src/executor.rs b/src/executor.rs index 764027ab..ae277917 100644 --- a/src/executor.rs +++ b/src/executor.rs @@ -1,5 +1,6 @@ use std::collections::HashMap; use std::marker::PhantomData; +use std::sync::RwLock; use ::GraphQLError; use ast::{InputValue, ToInputValue, Document, Selection, Fragment, Definition, Type, FromInputValue, OperationType}; @@ -41,7 +42,7 @@ pub struct Executor<'a, CtxT> where CtxT: 'a { current_selection_set: Option<Vec<Selection>>, schema: &'a SchemaType, context: &'a CtxT, - errors: &'a mut Vec<ExecutionError>, + errors: &'a RwLock<Vec<ExecutionError>>, field_path: FieldPath<'a>, } @@ -81,7 +82,7 @@ impl<T> IntoFieldResult<T> for FieldResult<T> { impl<'a, CtxT> Executor<'a, CtxT> { /// Resolve a single arbitrary value into an `ExecutionResult` - pub fn resolve<T: GraphQLType<CtxT>>(&mut self, value: &T) -> ExecutionResult { + pub fn resolve<T: GraphQLType<CtxT>>(&self, value: &T) -> ExecutionResult { Ok(value.resolve( match self.current_selection_set { Some(ref sel) => Some(sel.clone()), @@ -93,7 +94,7 @@ impl<'a, CtxT> Executor<'a, CtxT> { /// Resolve a single arbitrary value into a return value /// /// If the field fails to resolve, `null` will be returned. - pub fn resolve_into_value<T: GraphQLType<CtxT>>(&mut self, value: &T) -> Value { + pub fn resolve_into_value<T: GraphQLType<CtxT>>(&self, value: &T) -> Value { match self.resolve(value) { Ok(v) => v, Err(e) => { @@ -108,7 +109,7 @@ impl<'a, CtxT> Executor<'a, CtxT> { /// /// This can be used to connect different types, e.g. from different Rust /// libraries, that require different context types. - pub fn replaced_context<'b, NewCtxT>(&'b mut self, ctx: &'b NewCtxT) -> Executor<'b, NewCtxT> { + pub fn replaced_context<'b, NewCtxT>(&'b self, ctx: &'b NewCtxT) -> Executor<'b, NewCtxT> { Executor { fragments: self.fragments, variables: self.variables, @@ -122,7 +123,7 @@ impl<'a, CtxT> Executor<'a, CtxT> { #[doc(hidden)] pub fn sub_executor( - &mut self, + &self, field_name: Option<String>, location: SourcePosition, selection_set: Option<Vec<Selection>>, @@ -167,11 +168,13 @@ impl<'a, CtxT> Executor<'a, CtxT> { } /// Add an error to the execution engine - pub fn push_error(&mut self, error: String, location: SourcePosition) { + pub fn push_error(&self, error: String, location: SourcePosition) { let mut path = Vec::new(); self.field_path.construct_path(&mut path); - self.errors.push(ExecutionError { + let mut errors = self.errors.write().unwrap(); + + errors.push(ExecutionError { location: location, path: path, message: error, @@ -261,17 +264,17 @@ pub fn execute_validated_query<'a, QueryT, MutationT, CtxT>( None => return Err(GraphQLError::UnknownOperationName), }; - let mut errors = Vec::new(); + let errors = RwLock::new(Vec::new()); let value; { - let mut executor = Executor { + let executor = Executor { fragments: &fragments.into_iter().map(|f| (f.item.name.item.clone(), f.item)).collect(), variables: variables, current_selection_set: Some(op.item.selection_set), schema: &root_node.schema, context: context, - errors: &mut errors, + errors: &errors, field_path: FieldPath::Root(op.start), }; @@ -281,6 +284,7 @@ pub fn execute_validated_query<'a, QueryT, MutationT, CtxT>( }; } + let mut errors = errors.into_inner().unwrap(); errors.sort(); Ok((value, errors)) diff --git a/src/executor_tests/executor.rs b/src/executor_tests/executor.rs index f85995c4..bf725156 100644 --- a/src/executor_tests/executor.rs +++ b/src/executor_tests/executor.rs @@ -167,7 +167,7 @@ mod threads_context_correctly { struct Schema; graphql_object!(Schema: String |&self| { - field a(&mut executor) -> String { executor.context().clone() } + field a(&executor) -> String { executor.context().clone() } }); #[test] diff --git a/src/lib.rs b/src/lib.rs index 5a42fbd2..99fbcb6b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -67,7 +67,7 @@ graphql_object!(User: Database |&self| { // // In this example, the context is used to convert the friend_ids array // into actual User objects. - field friends(&mut executor) -> Vec<&User> { + field friends(&executor) -> Vec<&User> { self.friend_ids.iter() .filter_map(|id| executor.context().users.get(id)) .collect() @@ -79,7 +79,7 @@ graphql_object!(User: Database |&self| { graphql_object!(QueryRoot: Database |&self| { // Arguments work just like they do on functions. - field user(&mut executor, id: String) -> Option<&User> { + field user(&executor, id: String) -> Option<&User> { executor.context().users.get(&id) } }); @@ -121,7 +121,7 @@ use juniper::iron_handlers::GraphQLHandler; # Ok(&self.name) # } # -# field friends(&mut executor) -> FieldResult<Vec<&User>> { +# field friends(&executor) -> FieldResult<Vec<&User>> { # Ok(self.friend_ids.iter() # .filter_map(|id| executor.context().users.get(id)) # .collect()) @@ -129,7 +129,7 @@ use juniper::iron_handlers::GraphQLHandler; # }); # # graphql_object!(QueryRoot: Database |&self| { -# field user(&mut executor, id: String) -> FieldResult<Option<&User>> { +# field user(&executor, id: String) -> FieldResult<Option<&User>> { # Ok(executor.context().users.get(&id)) # } # }); diff --git a/src/macros/args.rs b/src/macros/args.rs index f6326fc1..951f1fe2 100644 --- a/src/macros/args.rs +++ b/src/macros/args.rs @@ -15,9 +15,9 @@ macro_rules! __graphql__args { ( @assign_arg_vars, - $args:ident, $executorvar:ident, &mut $exec:ident $($rest:tt)* + $args:ident, $executorvar:ident, &$exec:ident $($rest:tt)* ) => { - let __graphql__args!(@as_pattern, $exec) = &mut $executorvar; + let __graphql__args!(@as_pattern, $exec) = &$executorvar; __graphql__args!(@assign_arg_vars, $args, $executorvar, $($rest)*); }; @@ -59,7 +59,7 @@ macro_rules! __graphql__args { ( @apply_args, - $reg:expr, $base:expr, ( &mut executor $( $rest:tt )* ) + $reg:expr, $base:expr, ( &executor $( $rest:tt )* ) ) => { __graphql__args!( @apply_args, diff --git a/src/macros/enums.rs b/src/macros/enums.rs index e758bff8..3e61c964 100644 --- a/src/macros/enums.rs +++ b/src/macros/enums.rs @@ -81,7 +81,7 @@ macro_rules! graphql_enum { .into_meta() } - fn resolve(&self, _: Option<Vec<$crate::Selection>>, _: &mut $crate::Executor<CtxT>) -> $crate::Value { + fn resolve(&self, _: Option<Vec<$crate::Selection>>, _: &$crate::Executor<CtxT>) -> $crate::Value { match self { $( &graphql_enum!(@as_pattern, $eval) => diff --git a/src/macros/interface.rs b/src/macros/interface.rs index 2fddfbe6..01d27d42 100644 --- a/src/macros/interface.rs +++ b/src/macros/interface.rs @@ -249,7 +249,7 @@ macro_rules! graphql_interface { #[allow(unused_variables)] #[allow(unused_mut)] - fn resolve_field(&$mainself, field: &str, args: &$crate::Arguments, mut executor: &mut $crate::Executor<$ctxt>) -> $crate::ExecutionResult { + fn resolve_field(&$mainself, field: &str, args: &$crate::Arguments, mut executor: &$crate::Executor<$ctxt>) -> $crate::ExecutionResult { __graphql__build_field_matches!( ($outname, $mainself, field, args, executor), (), @@ -267,7 +267,7 @@ macro_rules! graphql_interface { &$mainself, type_name: &str, _: Option<Vec<$crate::Selection>>, - executor: &mut $crate::Executor<$ctxt>, + executor: &$crate::Executor<$ctxt>, ) -> $crate::ExecutionResult { diff --git a/src/macros/object.rs b/src/macros/object.rs index 6509a207..5330f0ed 100644 --- a/src/macros/object.rs +++ b/src/macros/object.rs @@ -194,7 +194,7 @@ string as documentation on the field. ### Field arguments ```text -&mut executor +&executor arg_name: ArgType arg_name = default_value: ArgType arg_name: ArgType as "Argument description" @@ -202,7 +202,7 @@ arg_name = default_value: ArgType as "Argument description" ``` Field arguments can take many forms. If the field needs access to the executor -or context, it can take an [Executor][1] instance by specifying `&mut executor` +or context, it can take an [Executor][1] instance by specifying `&executor` as the first argument. The other cases are similar to regular Rust arguments, with two additions: @@ -382,7 +382,7 @@ macro_rules! graphql_object { &$mainself, field: &str, args: &$crate::Arguments, - mut executor: &mut $crate::Executor<$ctxt> + executor: &$crate::Executor<$ctxt> ) -> $crate::ExecutionResult { diff --git a/src/macros/scalar.rs b/src/macros/scalar.rs index 3948b9fd..ad0d3253 100644 --- a/src/macros/scalar.rs +++ b/src/macros/scalar.rs @@ -76,7 +76,7 @@ macro_rules! graphql_scalar { fn resolve( &$resolve_selfvar, _: Option<Vec<$crate::Selection>>, - _: &mut $crate::Executor<CtxT>) -> $crate::Value { + _: &$crate::Executor<CtxT>) -> $crate::Value { $resolve_body } } diff --git a/src/macros/tests/args.rs b/src/macros/tests/args.rs index 9afee3f0..f677d43b 100644 --- a/src/macros/tests/args.rs +++ b/src/macros/tests/args.rs @@ -20,8 +20,8 @@ Syntax to validate: graphql_object!(Root: () |&self| { field simple() -> i64 { 0 } - field exec_arg(&mut executor) -> i64 { 0 } - field exec_arg_and_more(&mut executor, arg: i64) -> i64 { 0 } + field exec_arg(&executor) -> i64 { 0 } + field exec_arg_and_more(&executor, arg: i64) -> i64 { 0 } field single_arg(arg: i64) -> i64 { 0 } field multi_args( diff --git a/src/macros/union.rs b/src/macros/union.rs index aeaed936..5a4dd0c6 100644 --- a/src/macros/union.rs +++ b/src/macros/union.rs @@ -136,7 +136,7 @@ macro_rules! graphql_union { &$mainself, type_name: &str, _: Option<Vec<$crate::Selection>>, - executor: &mut $crate::Executor<$ctxt>, + executor: &$crate::Executor<$ctxt>, ) -> $crate::ExecutionResult { diff --git a/src/schema/schema.rs b/src/schema/schema.rs index 736d7c7c..9e3d580c 100644 --- a/src/schema/schema.rs +++ b/src/schema/schema.rs @@ -19,7 +19,7 @@ impl<CtxT, QueryT, MutationT> GraphQLType<CtxT> for RootNode<CtxT, QueryT, Mutat QueryT::meta(registry) } - fn resolve_field(&self, field: &str, args: &Arguments, executor: &mut Executor<CtxT>) -> ExecutionResult { + fn resolve_field(&self, field: &str, args: &Arguments, executor: &Executor<CtxT>) -> ExecutionResult { match field { "__schema" => executor.replaced_context(&self.schema).resolve(&self.schema), "__type" => { @@ -99,7 +99,7 @@ graphql_object!(<'a> TypeType<'a>: SchemaType as "__Type" |&self| { } } - field interfaces(&mut executor) -> Option<Vec<TypeType>> { + field interfaces(&executor) -> Option<Vec<TypeType>> { match *self { TypeType::Concrete(&MetaType::Object(ObjectMeta { ref interface_names, .. })) => { let schema = executor.context(); @@ -112,7 +112,7 @@ graphql_object!(<'a> TypeType<'a>: SchemaType as "__Type" |&self| { } } - field possible_types(&mut executor) -> Option<Vec<TypeType>> { + field possible_types(&executor) -> Option<Vec<TypeType>> { let schema = executor.context(); match *self { TypeType::Concrete(&MetaType::Union(UnionMeta { ref of_type_names, .. })) => { @@ -162,7 +162,7 @@ graphql_object!(Field: SchemaType as "__Field" |&self| { self.arguments.as_ref().map_or_else(|| Vec::new(), |v| v.iter().collect()) } - field type(&mut executor) -> TypeType { + field type(&executor) -> TypeType { executor.context().make_type(&self.field_type) } @@ -184,7 +184,7 @@ graphql_object!(Argument: SchemaType as "__InputValue" |&self| { &self.description } - field type(&mut executor) -> TypeType { + field type(&executor) -> TypeType { executor.context().make_type(&self.arg_type) } diff --git a/src/tests/schema.rs b/src/tests/schema.rs index ee129a00..e1182d68 100644 --- a/src/tests/schema.rs +++ b/src/tests/schema.rs @@ -17,7 +17,7 @@ graphql_interface!(<'a> &'a Character: Database as "Character" |&self| { Some(self.name()) } - field friends(&mut executor) -> Vec<&Character> + field friends(&executor) -> Vec<&Character> as "The friends of the character" { executor.context().get_friends(self.as_character()) } @@ -45,7 +45,7 @@ graphql_object!(<'a> &'a Human: Database as "Human" |&self| { Some(self.name()) } - field friends(&mut executor) -> Vec<&Character> + field friends(&executor) -> Vec<&Character> as "The friends of the human" { executor.context().get_friends(self.as_character()) } @@ -72,7 +72,7 @@ graphql_object!(<'a> &'a Droid: Database as "Droid" |&self| { Some(self.name()) } - field friends(&mut executor) -> Vec<&Character> + field friends(&executor) -> Vec<&Character> as "The friends of the droid" { executor.context().get_friends(self.as_character()) } diff --git a/src/types/base.rs b/src/types/base.rs index ab2576cf..3585b73e 100644 --- a/src/types/base.rs +++ b/src/types/base.rs @@ -165,7 +165,7 @@ impl GraphQLType<Database> for User { &self, field_name: &str, args: &Arguments, - executor: &mut Executor<Database> + executor: &Executor<Database> ) -> ExecutionResult { @@ -221,7 +221,7 @@ pub trait GraphQLType<CtxT>: Sized { /// /// The default implementation panics. #[allow(unused_variables)] - fn resolve_field(&self, field_name: &str, arguments: &Arguments, executor: &mut Executor<CtxT>) + fn resolve_field(&self, field_name: &str, arguments: &Arguments, executor: &Executor<CtxT>) -> ExecutionResult { panic!("resolve_field must be implemented by object types"); @@ -234,7 +234,7 @@ pub trait GraphQLType<CtxT>: Sized { /// /// The default implementation panics. #[allow(unused_variables)] - fn resolve_into_type(&self, type_name: &str, selection_set: Option<Vec<Selection>>, executor: &mut Executor<CtxT>) -> ExecutionResult { + fn resolve_into_type(&self, type_name: &str, selection_set: Option<Vec<Selection>>, executor: &Executor<CtxT>) -> ExecutionResult { if Self::name().unwrap() == type_name { Ok(self.resolve(selection_set, executor)) } else { @@ -260,7 +260,7 @@ pub trait GraphQLType<CtxT>: Sized { /// The default implementation uses `resolve_field` to resolve all fields, /// including those through fragment expansion, for object types. For /// non-object types, this method panics. - fn resolve(&self, selection_set: Option<Vec<Selection>>, executor: &mut Executor<CtxT>) -> Value { + fn resolve(&self, selection_set: Option<Vec<Selection>>, executor: &Executor<CtxT>) -> Value { if let Some(selection_set) = selection_set { let mut result = HashMap::new(); resolve_selection_set_into(self, selection_set, executor, &mut result); @@ -275,7 +275,7 @@ pub trait GraphQLType<CtxT>: Sized { fn resolve_selection_set_into<T, CtxT>( instance: &T, selection_set: Vec<Selection>, - executor: &mut Executor<CtxT>, + executor: &Executor<CtxT>, result: &mut HashMap<String, Value>) where T: GraphQLType<CtxT> { diff --git a/src/types/containers.rs b/src/types/containers.rs index 45c23efe..2fcb0d4e 100644 --- a/src/types/containers.rs +++ b/src/types/containers.rs @@ -14,7 +14,7 @@ impl<T, CtxT> GraphQLType<CtxT> for Option<T> where T: GraphQLType<CtxT> { registry.build_nullable_type::<T>().into_meta() } - fn resolve(&self, _: Option<Vec<Selection>>, executor: &mut Executor<CtxT>) -> Value { + fn resolve(&self, _: Option<Vec<Selection>>, executor: &Executor<CtxT>) -> Value { match *self { Some(ref obj) => executor.resolve_into_value(obj), None => Value::null(), @@ -59,7 +59,7 @@ impl<T, CtxT> GraphQLType<CtxT> for Vec<T> where T: GraphQLType<CtxT> { registry.build_list_type::<T>().into_meta() } - fn resolve(&self, _: Option<Vec<Selection>>, executor: &mut Executor<CtxT>) -> Value { + fn resolve(&self, _: Option<Vec<Selection>>, executor: &Executor<CtxT>) -> Value { Value::list( self.iter().map(|e| executor.resolve_into_value(e)).collect() ) @@ -111,7 +111,7 @@ impl<'a, T, CtxT> GraphQLType<CtxT> for &'a [T] where T: GraphQLType<CtxT> { registry.build_list_type::<T>().into_meta() } - fn resolve(&self, _: Option<Vec<Selection>>, executor: &mut Executor<CtxT>) -> Value { + fn resolve(&self, _: Option<Vec<Selection>>, executor: &Executor<CtxT>) -> Value { Value::list( self.iter().map(|e| executor.resolve_into_value(e)).collect() ) diff --git a/src/types/pointers.rs b/src/types/pointers.rs index 01a7e5dc..7124945a 100644 --- a/src/types/pointers.rs +++ b/src/types/pointers.rs @@ -14,16 +14,16 @@ impl<T, CtxT> GraphQLType<CtxT> for Box<T> where T: GraphQLType<CtxT> { T::meta(registry) } - fn resolve_into_type(&self, name: &str, selection_set: Option<Vec<Selection>>, executor: &mut Executor<CtxT>) -> ExecutionResult { + fn resolve_into_type(&self, name: &str, selection_set: Option<Vec<Selection>>, executor: &Executor<CtxT>) -> ExecutionResult { (**self).resolve_into_type(name, selection_set, executor) } - fn resolve_field(&self, field: &str, args: &Arguments, executor: &mut Executor<CtxT>) -> ExecutionResult + fn resolve_field(&self, field: &str, args: &Arguments, executor: &Executor<CtxT>) -> ExecutionResult { (**self).resolve_field(field, args, executor) } - fn resolve(&self, selection_set: Option<Vec<Selection>>, executor: &mut Executor<CtxT>) -> Value { + fn resolve(&self, selection_set: Option<Vec<Selection>>, executor: &Executor<CtxT>) -> Value { (**self).resolve(selection_set, executor) } } @@ -58,16 +58,16 @@ impl<'a, T, CtxT> GraphQLType<CtxT> for &'a T where T: GraphQLType<CtxT> { T::meta(registry) } - fn resolve_into_type(&self, name: &str, selection_set: Option<Vec<Selection>>, executor: &mut Executor<CtxT>) -> ExecutionResult { + fn resolve_into_type(&self, name: &str, selection_set: Option<Vec<Selection>>, executor: &Executor<CtxT>) -> ExecutionResult { (**self).resolve_into_type(name, selection_set, executor) } - fn resolve_field(&self, field: &str, args: &Arguments, executor: &mut Executor<CtxT>) -> ExecutionResult + fn resolve_field(&self, field: &str, args: &Arguments, executor: &Executor<CtxT>) -> ExecutionResult { (**self).resolve_field(field, args, executor) } - fn resolve(&self, selection_set: Option<Vec<Selection>>, executor: &mut Executor<CtxT>) -> Value { + fn resolve(&self, selection_set: Option<Vec<Selection>>, executor: &Executor<CtxT>) -> Value { (**self).resolve(selection_set, executor) } } diff --git a/src/types/scalars.rs b/src/types/scalars.rs index f4b7eb9d..c7eac471 100644 --- a/src/types/scalars.rs +++ b/src/types/scalars.rs @@ -49,7 +49,7 @@ impl<'a, CtxT> GraphQLType<CtxT> for &'a str { registry.build_scalar_type::<String>().into_meta() } - fn resolve(&self, _: Option<Vec<Selection>>, _: &mut Executor<CtxT>) -> Value { + fn resolve(&self, _: Option<Vec<Selection>>, _: &Executor<CtxT>) -> Value { Value::string(self) } }