From 11021ea305d6e7b961ad0f1662f34a19843a427d Mon Sep 17 00:00:00 2001 From: Georg Semmler Date: Fri, 28 Sep 2018 11:59:04 +0200 Subject: [PATCH] Fix a bug regarding to input value casting in custom scalars This fix introduces a new way to convert a scalar value into one of the 4 standardized scalar values (Int, Float, String, Bool). Using this abstraction it is possible to do certain needed conversations in custom scalar value code, for example that one that converts a integer value to a floating point value. --- juniper/src/types/scalars.rs | 16 ++- juniper/src/value/scalar.rs | 102 +++++++++++++++++- .../src/derive_juniper_scalar_value.rs | 19 ---- juniper_codegen/src/lib.rs | 2 +- juniper_codegen/src/util.rs | 10 +- juniper_tests/src/custom_scalar.rs | 42 +++++++- 6 files changed, 146 insertions(+), 45 deletions(-) diff --git a/juniper/src/types/scalars.rs b/juniper/src/types/scalars.rs index 55ce11556..7e7472611 100644 --- a/juniper/src/types/scalars.rs +++ b/juniper/src/types/scalars.rs @@ -39,9 +39,8 @@ graphql_scalar!(ID as "ID" where Scalar = { from_input_value(v: &InputValue) -> Option { match *v { InputValue::Scalar(ref s) => { - <_ as Into>>::into(s.clone()).or_else(||{ - <_ as Into>>::into(s.clone()).map(|i| i.to_string()) - }).map(ID) + s.as_string().or_else(|| s.as_int().map(|i| i.to_string())) + .map(ID) } _ => None } @@ -64,7 +63,7 @@ graphql_scalar!(String as "String" where Scalar = { from_input_value(v: &InputValue) -> Option { match *v { - InputValue::Scalar(ref s) => <_ as Into>>::into(s.clone()), + InputValue::Scalar(ref s) => s.as_string(), _ => None, } } @@ -202,7 +201,7 @@ graphql_scalar!(bool as "Boolean" where Scalar = { from_input_value(v: &InputValue) -> Option { match *v { - InputValue::Scalar(ref b) => <_ as Into>>::into(b).map(|b| *b), + InputValue::Scalar(ref b) => b.as_boolean(), _ => None, } } @@ -221,7 +220,7 @@ graphql_scalar!(i32 as "Int" where Scalar = { from_input_value(v: &InputValue) -> Option { match *v { - InputValue::Scalar(ref i) => <_ as Into>>::into(i).map(|i| *i), + InputValue::Scalar(ref i) => i.as_int(), _ => None, } } @@ -244,10 +243,7 @@ graphql_scalar!(f64 as "Float" where Scalar = { from_input_value(v: &InputValue) -> Option { match *v { - InputValue::Scalar(ref s) => { - <_ as Into>>::into(s).map(|i| *i) - .or_else(|| <_ as Into>>::into(s).map(|i| *i as f64)) - } + InputValue::Scalar(ref s) => s.as_float(), _ => None, } } diff --git a/juniper/src/value/scalar.rs b/juniper/src/value/scalar.rs index 88b25154a..5c110ead2 100644 --- a/juniper/src/value/scalar.rs +++ b/juniper/src/value/scalar.rs @@ -35,10 +35,10 @@ pub trait ParseScalarValue { /// # extern crate juniper; /// # extern crate serde; /// # use serde::{de, Deserialize, Deserializer}; +/// # use juniper::ScalarValue; /// # use std::fmt; /// # /// #[derive(Debug, Clone, PartialEq, ScalarValue)] -/// #[juniper(visitor = "MyScalarValueVisitor")] /// enum MyScalarValue { /// Int(i32), /// Long(i64), @@ -47,6 +47,39 @@ pub trait ParseScalarValue { /// Boolean(bool), /// } /// +/// impl ScalarValue for MyScalarValue { +/// type Visitor = MyScalarValueVisitor; +/// +/// fn as_int(&self) -> Option { +/// match *self { +/// MyScalarValue::Int(ref i) => Some(*i), +/// _ => None, +/// } +/// } +/// +/// fn as_string(&self) -> Option { +/// match *self { +/// MyScalarValue::String(ref s) => Some(s.clone()), +/// _ => None, +/// } +/// } +/// +/// fn as_float(&self) -> Option { +/// match *self { +/// MyScalarValue::Int(ref i) => Some(*i as f64), +/// MyScalarValue::Float(ref f) => Some(*f), +/// _ => None, +/// } +/// } +/// +/// fn as_boolean(&self) -> Option { +/// match *self { +/// MyScalarValue::Boolean(ref b) => Some(*b), +/// _ => None, +/// } +/// } +/// } +/// /// #[derive(Default)] /// struct MyScalarValueVisitor; /// @@ -72,7 +105,11 @@ pub trait ParseScalarValue { /// where /// E: de::Error, /// { -/// Ok(MyScalarValue::Long(value)) +/// if value <= i32::max_value() as i64 { +/// self.visit_i32(value as i32) +/// } else { +/// Ok(MyScalarValue::Long(value)) +/// } /// } /// /// fn visit_u32(self, value: u32) -> Result @@ -156,6 +193,33 @@ pub trait ScalarValue: { self.into().is_some() } + + /// Convert the given scalar value into an integer value + /// + /// This function is used for implementing `GraphQLType` for `i32` for all + /// scalar values. Implementations should convert all supported integer + /// types with 32 bit or less to an integer if requested. + fn as_int(&self) -> Option; + + /// Convert the given scalar value into a string value + /// + /// This function is used for implementing `GraphQLType` for `String` for all + /// scalar values + fn as_string(&self) -> Option; + + /// Convert the given scalar value into a float value + /// + /// This function is used for implementing `GraphQLType` for `f64` for all + /// scalar values. Implementations should convert all supported integer + /// types with 64 bit or less and all floating point values with 64 bit or + /// less to a float if requested. + fn as_float(&self) -> Option; + + /// Convert the given scalar value into a boolean value + /// + /// This function is used for implementing `GraphQLType` for `bool` for all + /// scalar values. + fn as_boolean(&self) -> Option; } /// A marker trait extending the [`ScalarValue`](../trait.ScalarValue.html) trait @@ -189,7 +253,6 @@ where /// This types closely follows the graphql specification. #[derive(Debug, PartialEq, Clone, ScalarValue)] #[allow(missing_docs)] -#[juniper(visitor = "DefaultScalarValueVisitor")] pub enum DefaultScalarValue { Int(i32), Float(f64), @@ -197,6 +260,39 @@ pub enum DefaultScalarValue { Boolean(bool), } +impl ScalarValue for DefaultScalarValue { + type Visitor = DefaultScalarValueVisitor; + + fn as_int(&self) -> Option { + match *self { + DefaultScalarValue::Int(ref i) => Some(*i), + _ => None, + } + } + + fn as_string(&self) -> Option { + match *self { + DefaultScalarValue::String(ref s) => Some(s.clone()), + _ => None, + } + } + + fn as_float(&self) -> Option { + match *self { + DefaultScalarValue::Int(ref i) => Some(*i as f64), + DefaultScalarValue::Float(ref f) => Some(*f), + _ => None, + } + } + + fn as_boolean(&self) -> Option { + match *self { + DefaultScalarValue::Boolean(ref b) => Some(*b), + _ => None, + } + } +} + impl<'a> From<&'a str> for DefaultScalarValue { fn from(s: &'a str) -> Self { DefaultScalarValue::String(s.into()) diff --git a/juniper_codegen/src/derive_juniper_scalar_value.rs b/juniper_codegen/src/derive_juniper_scalar_value.rs index 3a318b7fa..7f6fb1c0b 100644 --- a/juniper_codegen/src/derive_juniper_scalar_value.rs +++ b/juniper_codegen/src/derive_juniper_scalar_value.rs @@ -1,23 +1,9 @@ use proc_macro2::{Span, TokenStream}; use syn::{self, Data, Fields, Ident, Variant}; -use util::{get_juniper_attr, keyed_item_value, AttributeValidation, AttributeValue}; pub fn impl_scalar_value(ast: &syn::DeriveInput) -> TokenStream { let ident = &ast.ident; - let visitor = if let Some(items) = get_juniper_attr(&ast.attrs) { - items.into_iter() - .filter_map(|i| keyed_item_value(&i, "visitor", AttributeValidation::String)) - .next() - .and_then(|t| if let AttributeValue::String(s) = t { - Some(Ident::new(&s, Span::call_site())) - } else { - None - }) - .expect("`#[derive(ScalarValue)]` needs a annotation of the form `#[juniper(visitor = \"VisitorType\")]`") - } else { - panic!("`#[derive(ScalarValue)]` needs a annotation of the form `#[juniper(visitor = \"VisitorType\")]`"); - }; let variants = match ast.data { Data::Enum(ref enum_data) => &enum_data.variants, @@ -54,11 +40,6 @@ pub fn impl_scalar_value(ast: &syn::DeriveInput) -> TokenStream { #serialize #display - - impl juniper::ScalarValue for #ident { - type Visitor = #visitor; - } - }; } } diff --git a/juniper_codegen/src/lib.rs b/juniper_codegen/src/lib.rs index d15c97435..03be1b608 100644 --- a/juniper_codegen/src/lib.rs +++ b/juniper_codegen/src/lib.rs @@ -45,7 +45,7 @@ pub fn derive_object(input: TokenStream) -> TokenStream { gen.into() } -#[proc_macro_derive(ScalarValue, attributes(juniper))] +#[proc_macro_derive(ScalarValue)] pub fn derive_juniper_scalar_value(input: TokenStream) -> TokenStream { let ast = syn::parse::(input).unwrap(); let gen = derive_juniper_scalar_value::impl_scalar_value(&ast); diff --git a/juniper_codegen/src/util.rs b/juniper_codegen/src/util.rs index c20c7cf5e..676a98cb4 100644 --- a/juniper_codegen/src/util.rs +++ b/juniper_codegen/src/util.rs @@ -71,17 +71,9 @@ fn get_doc_attr(attrs: &Vec) -> Option> { // Get the nested items of a a #[graphql(...)] attribute. pub fn get_graphql_attr(attrs: &Vec) -> Option> { - get_named_attr(attrs, "graphql") -} - -pub fn get_juniper_attr(attrs: &Vec) -> Option> { - get_named_attr(attrs, "juniper") -} - -pub fn get_named_attr(attrs: &Vec, name: &str) -> Option> { for attr in attrs { match attr.interpret_meta() { - Some(Meta::List(ref list)) if list.ident == name => { + Some(Meta::List(ref list)) if list.ident == "graphql" => { return Some(list.nested.iter().map(|x| x.clone()).collect()); } _ => {} diff --git a/juniper_tests/src/custom_scalar.rs b/juniper_tests/src/custom_scalar.rs index a493b8757..d54e3fa7d 100644 --- a/juniper_tests/src/custom_scalar.rs +++ b/juniper_tests/src/custom_scalar.rs @@ -6,11 +6,10 @@ use juniper::parser::{ParseError, ScalarToken, Token}; use juniper::serde::de; #[cfg(test)] use juniper::{execute, EmptyMutation, Object, RootNode, Variables}; -use juniper::{InputValue, ParseScalarResult, Value}; +use juniper::{InputValue, ParseScalarResult, ScalarValue, Value}; use std::fmt; #[derive(Debug, Clone, PartialEq, ScalarValue)] -#[juniper(visitor = "MyScalarValueVisitor")] enum MyScalarValue { Int(i32), Long(i64), @@ -19,6 +18,39 @@ enum MyScalarValue { Boolean(bool), } +impl ScalarValue for MyScalarValue { + type Visitor = MyScalarValueVisitor; + + fn as_int(&self) -> Option { + match *self { + MyScalarValue::Int(ref i) => Some(*i), + _ => None, + } + } + + fn as_string(&self) -> Option { + match *self { + MyScalarValue::String(ref s) => Some(s.clone()), + _ => None, + } + } + + fn as_float(&self) -> Option { + match *self { + MyScalarValue::Int(ref i) => Some(*i as f64), + MyScalarValue::Float(ref f) => Some(*f), + _ => None, + } + } + + fn as_boolean(&self) -> Option { + match *self { + MyScalarValue::Boolean(ref b) => Some(*b), + _ => None, + } + } +} + #[derive(Default, Debug)] struct MyScalarValueVisitor; @@ -44,7 +76,11 @@ impl<'de> de::Visitor<'de> for MyScalarValueVisitor { where E: de::Error, { - Ok(MyScalarValue::Long(value)) + if value <= i32::max_value() as i64 { + self.visit_i32(value as i32) + } else { + Ok(MyScalarValue::Long(value)) + } } fn visit_u32(self, value: u32) -> Result