Add support for skipping fields in GraphQL objects (#224)

Fields can now be skipped with the `#[graphql(skip)]` annotation. Note this doesn't really make sense for GraphQLInputObjects so this isn't supported there. Fixes https://github.com/graphql-rust/juniper/issues/220.
2018-08-27 15:09:42 -07:00 · 2018-08-27 15:09:42 -07:00 · 22c955599a
commit 22c955599a
parent 62d015cf86
6 changed files with 112 additions and 33 deletions
--- a/changelog/master.md
+++ b/changelog/master.md
@ -45,6 +45,10 @@

  [#219](https://github.com/graphql-rust/juniper/pull/219)

+* When using the `GraphQLObject` custom derive, fields now be omitted by annotating the field with `#[graphql(skip)]`.
+
+  [#220](https://github.com/graphql-rust/juniper/issues/220)
+
 * Due to newer dependencies, the oldest Rust version supported is now 1.22.0

-  [#231](https://github.com/graphql-rust/juniper/pull/231)
+  [#231](https://github.com/graphql-rust/juniper/pull/231)
--- a/juniper_codegen/src/derive_enum.rs
+++ b/juniper_codegen/src/derive_enum.rs
@ -25,9 +25,9 @@ impl EnumAttrs {
        res.description = get_doc_comment(&input.attrs);

        // Check attributes for name and description.
-        if let Some(items) = get_graphl_attr(&input.attrs) {
+        if let Some(items) = get_graphql_attr(&input.attrs) {
            for item in items {
-                if let Some(val) = keyed_item_value(&item, "name", true) {
+                if let Some(AttributeValue::String(val)) = keyed_item_value(&item, "name", AttributeValidation::String)  {
                    if is_valid_name(&*val) {
                        res.name = Some(val);
                        continue;
@ -38,7 +38,7 @@ impl EnumAttrs {
                        );
                    }
                }
-                if let Some(val) = keyed_item_value(&item, "description", true) {
+                if let Some(AttributeValue::String(val)) = keyed_item_value(&item, "description", AttributeValidation::String)  {
                    res.description = Some(val);
                    continue;
                }
@ -76,9 +76,9 @@ impl EnumVariantAttrs {
        res.description = get_doc_comment(&variant.attrs);

        // Check attributes for name and description.
-        if let Some(items) = get_graphl_attr(&variant.attrs) {
+        if let Some(items) = get_graphql_attr(&variant.attrs) {
            for item in items {
-                if let Some(val) = keyed_item_value(&item, "name", true) {
+                if let Some(AttributeValue::String(val)) = keyed_item_value(&item, "name", AttributeValidation::String)  {
                    if is_valid_name(&*val) {
                        res.name = Some(val);
                        continue;
@ -89,11 +89,11 @@ impl EnumVariantAttrs {
                        );
                    }
                }
-                if let Some(val) = keyed_item_value(&item, "description", true) {
+                if let Some(AttributeValue::String(val)) = keyed_item_value(&item, "description", AttributeValidation::String)  {
                    res.description = Some(val);
                    continue;
                }
-                if let Some(val) = keyed_item_value(&item, "deprecated", true) {
+                if let Some(AttributeValue::String(val)) = keyed_item_value(&item, "deprecated", AttributeValidation::String)  {
                    res.deprecation = Some(val);
                    continue;
                }
--- a/juniper_codegen/src/derive_input_object.rs
+++ b/juniper_codegen/src/derive_input_object.rs
@ -21,9 +21,9 @@ impl ObjAttrs {
        res.description = get_doc_comment(&input.attrs);

        // Check attributes for name and description.
-        if let Some(items) = get_graphl_attr(&input.attrs) {
+        if let Some(items) = get_graphql_attr(&input.attrs) {
            for item in items {
-                if let Some(val) = keyed_item_value(&item, "name", true) {
+                if let Some(AttributeValue::String(val)) = keyed_item_value(&item, "name", AttributeValidation::String)  {
                    if is_valid_name(&*val) {
                        res.name = Some(val);
                        continue;
@ -34,7 +34,7 @@ impl ObjAttrs {
                        );
                    }
                }
-                if let Some(val) = keyed_item_value(&item, "description", true) {
+                if let Some(AttributeValue::String(val)) = keyed_item_value(&item, "description", AttributeValidation::String)  {
                    res.description = Some(val);
                    continue;
                }
@ -73,9 +73,9 @@ impl ObjFieldAttrs {
        res.description = get_doc_comment(&variant.attrs);

        // Check attributes for name and description.
-        if let Some(items) = get_graphl_attr(&variant.attrs) {
+        if let Some(items) = get_graphql_attr(&variant.attrs) {
            for item in items {
-                if let Some(val) = keyed_item_value(&item, "name", true) {
+                if let Some(AttributeValue::String(val)) = keyed_item_value(&item, "name", AttributeValidation::String)  {
                    if is_valid_name(&*val) {
                        res.name = Some(val);
                        continue;
@ -86,11 +86,11 @@ impl ObjFieldAttrs {
                        );
                    }
                }
-                if let Some(val) = keyed_item_value(&item, "description", true) {
+                if let Some(AttributeValue::String(val)) = keyed_item_value(&item, "description", AttributeValidation::String)  {
                    res.description = Some(val);
                    continue;
                }
-                if let Some(val) = keyed_item_value(&item, "default", true) {
+                if let Some(AttributeValue::String(val)) = keyed_item_value(&item, "default", AttributeValidation::Any) {
                    res.default_expr = Some(val);
                    continue;
                }
--- a/juniper_codegen/src/derive_object.rs
+++ b/juniper_codegen/src/derive_object.rs
@ -18,9 +18,9 @@ impl ObjAttrs {
        res.description = get_doc_comment(&input.attrs);

        // Check attributes for name and description.
-        if let Some(items) = get_graphl_attr(&input.attrs) {
+        if let Some(items) = get_graphql_attr(&input.attrs) {
            for item in items {
-                if let Some(val) = keyed_item_value(&item, "name", true) {
+                if let Some(AttributeValue::String(val)) = keyed_item_value(&item, "name", AttributeValidation::String)  {
                    if is_valid_name(&*val) {
                        res.name = Some(val);
                        continue;
@ -31,12 +31,12 @@ impl ObjAttrs {
                        );
                    }
                }
-                if let Some(val) = keyed_item_value(&item, "description", true) {
+                if let Some(AttributeValue::String(val)) = keyed_item_value(&item, "description", AttributeValidation::String)  {
                    res.description = Some(val);
                    continue;
                }
                panic!(format!(
-                    "Unknown attribute for #[derive(GraphQLObject)]: {:?}",
+                    "Unknown object attribute for #[derive(GraphQLObject)]: {:?}",
                    item
                ));
            }
@ -50,6 +50,7 @@ struct ObjFieldAttrs {
    name: Option<String>,
    description: Option<String>,
    deprecation: Option<String>,
+    skip: bool,
 }

 impl ObjFieldAttrs {
@ -59,10 +60,10 @@ impl ObjFieldAttrs {
        // Check doc comments for description.
        res.description = get_doc_comment(&variant.attrs);

-        // Check attributes for name and description.
-        if let Some(items) = get_graphl_attr(&variant.attrs) {
+        // Check attributes.
+        if let Some(items) = get_graphql_attr(&variant.attrs) {
            for item in items {
-                if let Some(val) = keyed_item_value(&item, "name", true) {
+                if let Some(AttributeValue::String(val)) = keyed_item_value(&item, "name", AttributeValidation::String)  {
                    if is_valid_name(&*val) {
                        res.name = Some(val);
                        continue;
@ -73,16 +74,20 @@ impl ObjFieldAttrs {
                        );
                    }
                }
-                if let Some(val) = keyed_item_value(&item, "description", true) {
+                if let Some(AttributeValue::String(val)) = keyed_item_value(&item, "description", AttributeValidation::String)  {
                    res.description = Some(val);
                    continue;
                }
-                if let Some(val) = keyed_item_value(&item, "deprecation", true) {
+                if let Some(AttributeValue::String(val)) = keyed_item_value(&item, "deprecation", AttributeValidation::String) {
                    res.deprecation = Some(val);
                    continue;
                }
+                if let Some(_) = keyed_item_value(&item, "skip", AttributeValidation::Bare) {
+                    res.skip = true;
+                    continue;
+                }
                panic!(format!(
-                    "Unknown attribute for #[derive(GraphQLObject)]: {:?}",
+                    "Unknown field attribute for #[derive(GraphQLObject)]: {:?}",
                    item
                ));
            }
@ -122,6 +127,11 @@ pub fn impl_object(ast: &syn::DeriveInput) -> TokenStream {
        let field_attrs = ObjFieldAttrs::from_input(field);
        let field_ident = field.ident.as_ref().unwrap();

+        // Check if we should skip this field.
+        if field_attrs.skip {
+            continue;
+        }
+
        // Build value.
        let name = match field_attrs.name {
            Some(ref name) => {
--- a/juniper_codegen/src/util.rs
+++ b/juniper_codegen/src/util.rs
@ -1,6 +1,17 @@
 use regex::Regex;
 use syn::{Attribute, Lit, Meta, MetaNameValue, NestedMeta};

+pub enum AttributeValidation {
+    Any,
+    Bare,
+    String,
+}
+
+pub enum AttributeValue {
+    Bare,
+    String(String),
+}
+
 // Gets doc comment.
 pub fn get_doc_comment(attrs: &Vec<Attribute>) -> Option<String> {
    if let Some(items) = get_doc_attr(attrs) {
@ -59,7 +70,7 @@ fn get_doc_attr(attrs: &Vec<Attribute>) -> Option<Vec<MetaNameValue>> {
 }

 // Get the nested items of a a #[graphql(...)] attribute.
-pub fn get_graphl_attr(attrs: &Vec<Attribute>) -> Option<Vec<NestedMeta>> {
+pub fn get_graphql_attr(attrs: &Vec<Attribute>) -> Option<Vec<NestedMeta>> {
    for attr in attrs {
        match attr.interpret_meta() {
            Some(Meta::List(ref list)) if list.ident == "graphql" => {
@ -71,21 +82,36 @@ pub fn get_graphl_attr(attrs: &Vec<Attribute>) -> Option<Vec<NestedMeta>> {
    None
 }

-pub fn keyed_item_value(item: &NestedMeta, name: &str, must_be_string: bool) -> Option<String> {
+pub fn keyed_item_value(item: &NestedMeta, name: &str, validation: AttributeValidation) -> Option<AttributeValue> {
    match item {
+        // Attributes in the form of `#[graphql(name = "value")]`.
        &NestedMeta::Meta(Meta::NameValue(ref nameval)) if nameval.ident == name => {
            match &nameval.lit {
-                &Lit::Str(ref strlit) => Some(strlit.value()),
-                _ => if must_be_string {
+                // We have a string attribute value.
+                &Lit::Str(ref strlit) => match validation {
+                    AttributeValidation::Bare => {
+                        panic!(format!(
+                            "Invalid format for attribute \"{:?}\": expected a bare attribute without a value",
+                            item
+                        ));
+                    },
+                    _ => Some(AttributeValue::String(strlit.value())),
+                },
+                _  => None,
+            }
+        },
+        // Attributes in the form of `#[graphql(name)]`.
+        &NestedMeta::Meta(Meta::Word(ref ident)) if ident.to_string() == name => {
+            match validation {
+                AttributeValidation::String => {
                    panic!(format!(
-                        "Invalid format for attribute \"{:?}\": expected a string",
+                    "Invalid format for attribute \"{:?}\": expected a string value",
                        item
                    ));
-                } else {
-                    None
                },
+                _ => Some(AttributeValue::Bare),
            }
-        }
+        },
        _ => None,
    }
 }
--- a/juniper_tests/src/codegen/derive_object.rs
+++ b/juniper_tests/src/codegen/derive_object.rs
@ -48,6 +48,13 @@ struct OverrideDocComment {
    regular_field: bool,
 }

+#[derive(GraphQLObject, Debug, PartialEq)]
+struct SkippedFieldObj {
+    regular_field: bool,
+    #[graphql(skip)]
+    skipped: i32,
+}
+
 graphql_object!(Query: () |&self| {
    field obj() -> Obj {
      Obj{
@ -82,6 +89,13 @@ graphql_object!(Query: () |&self| {
        regular_field: true,
      }
    }
+
+    field skipped_field_obj() -> SkippedFieldObj {
+        SkippedFieldObj{
+            regular_field: false,
+            skipped: 42,
+        }
+    }
 });

 #[test]
@ -171,6 +185,31 @@ fn test_derived_object() {
    );
 }

+#[test]
+#[should_panic]
+fn test_cannot_query_skipped_field() {
+    let doc = r#"
+        {
+            skippedFieldObj {
+                skippedField
+            }
+        }"#;
+    let schema = RootNode::new(Query, EmptyMutation::<()>::new());
+    execute(doc, None, &schema, &Variables::new(), &()).unwrap();
+}
+
+#[test]
+fn test_skipped_field_siblings_unaffected() {
+    let doc = r#"
+        {
+            skippedFieldObj {
+                regularField
+            }
+        }"#;
+    let schema = RootNode::new(Query, EmptyMutation::<()>::new());
+    execute(doc, None, &schema, &Variables::new(), &()).unwrap();
+}
+
 #[test]
 fn test_derived_object_nested() {
    let doc = r#"