Preserve the order of requested fields

Fixes https://github.com/graphql-rust/juniper/issues/82

CHANGELOG.md

@@ -7,11 +7,17 @@ The repository was restructured to a multi crate workspace to enable several new
 
 ### New features
 
-* New juniper_codegen crate which provides custom derives: 
+* New juniper_codegen crate which provides custom derives:
   * `#[derive(GraphQLInputObject)]`
   * `#[derive(GraphQLEnum)]`
   * `#[derive(GraphQLObject)]`
 
+## Breaking changes
+
+* To better comply with the specification, order of requested fields is
+  now preserved.
+  ([#82](https://github.com/graphql-rust/juniper/issues/82)
+
 ## [0.8.1] – 2017-06-15
 
 Tiny release to fix broken crate metadata on crates.io.

juniper/Cargo.toml

@@ -25,6 +25,7 @@ expose-test-schema = []
 default = ["uuid"]
 
 [dependencies]
+ordermap = { version = "^0.2.11", features = ["serde-1"] }
 serde = { version = "^1.0.8" }
 serde_derive = {version="^1.0.8" }
 serde_json = { version="^1.0.2", optional = true }
@@ -32,4 +33,4 @@ uuid = { version = "0.5.1", optional = true }
 
 [dev-dependencies]
 bencher = "^0.1.2"
-serde_json = { version = "^1.0.2" }
+serde_json = { version = "^1.0.2" }

juniper/src/ast.rs

@@ -1,10 +1,11 @@
 use std::fmt;
 use std::borrow::Cow;
-use std::collections::HashMap;
 use std::hash::Hash;
 use std::vec;
 use std::slice;
 
+use ordermap::OrderMap;
+
 use executor::Variables;
 use parser::Spanning;
 
@@ -258,7 +259,7 @@ impl InputValue {
     ///
     /// Similar to `InputValue::list`, it makes each key and value in the given
     /// hash map not contain any location information.
-    pub fn object<K>(o: HashMap<K, InputValue>) -> InputValue
+    pub fn object<K>(o: OrderMap<K, InputValue>) -> InputValue
     where
         K: AsRef<str> + Eq + Hash,
     {
@@ -347,9 +348,9 @@ impl InputValue {
 
     /// Convert the input value to an unlocated object value.
     ///
-    /// This constructs a new hashmap that contain references to the keys
+    /// This constructs a new OrderMap that contain references to the keys
     /// and values in `self`.
-    pub fn to_object_value(&self) -> Option<HashMap<&str, &InputValue>> {
+    pub fn to_object_value(&self) -> Option<OrderMap<&str, &InputValue>> {
         match *self {
             InputValue::Object(ref o) => Some(
                 o.iter()

juniper/src/executor_tests/directives.rs

@@ -1,4 +1,4 @@
-use std::collections::HashMap;
+use ordermap::OrderMap;
 
 use value::Value;
 use executor::Variables;
@@ -19,7 +19,7 @@ graphql_object!(TestType: () |&self| {
 
 fn run_variable_query<F>(query: &str, vars: Variables, f: F)
 where
-    F: Fn(&HashMap<String, Value>) -> (),
+    F: Fn(&OrderMap<String, Value>) -> (),
 {
     let schema = RootNode::new(TestType, EmptyMutation::<()>::new());
 
@@ -36,7 +36,7 @@ where
 
 fn run_query<F>(query: &str, f: F)
 where
-    F: Fn(&HashMap<String, Value>) -> (),
+    F: Fn(&OrderMap<String, Value>) -> (),
 {
     run_variable_query(query, Variables::new(), f);
 }

juniper/src/executor_tests/enums.rs

@@ -1,4 +1,4 @@
-use std::collections::HashMap;
+use ordermap::OrderMap;
 
 use value::Value;
 use ast::InputValue;
@@ -35,7 +35,7 @@ graphql_object!(TestType: () |&self| {
 
 fn run_variable_query<F>(query: &str, vars: Variables, f: F)
 where
-    F: Fn(&HashMap<String, Value>) -> (),
+    F: Fn(&OrderMap<String, Value>) -> (),
 {
     let schema = RootNode::new(TestType, EmptyMutation::<()>::new());
 
@@ -52,7 +52,7 @@ where
 
 fn run_query<F>(query: &str, f: F)
 where
-    F: Fn(&HashMap<String, Value>) -> (),
+    F: Fn(&OrderMap<String, Value>) -> (),
 {
     run_variable_query(query, Variables::new(), f);
 }

juniper/src/executor_tests/executor.rs

@@ -148,15 +148,15 @@ mod merge_parallel_fragments {
             Value::object(vec![
                 ("a", Value::string("Apple")),
                 ("b", Value::string("Banana")),
-                ("c", Value::string("Cherry")),
                 ("deep", Value::object(vec![
                     ("b", Value::string("Banana")),
-                    ("c", Value::string("Cherry")),
                     ("deeper", Value::object(vec![
                         ("b", Value::string("Banana")),
                         ("c", Value::string("Cherry")),
                     ].into_iter().collect())),
+                    ("c", Value::string("Cherry")),
                 ].into_iter().collect())),
+                ("c", Value::string("Cherry")),
             ].into_iter().collect()));
     }
 }
@@ -209,7 +209,7 @@ mod threads_context_correctly {
 }
 
 mod dynamic_context_switching {
-    use std::collections::HashMap;
+    use ordermap::OrderMap;
 
     use value::Value;
     use types::scalars::EmptyMutation;
@@ -224,7 +224,7 @@ mod dynamic_context_switching {
     }
 
     struct OuterContext {
-        items: HashMap<i32, InnerContext>,
+        items: OrderMap<i32, InnerContext>,
     }
 
     impl Context for OuterContext {}

juniper/src/executor_tests/variables.rs

@@ -1,4 +1,4 @@
-use std::collections::HashMap;
+use ordermap::OrderMap;
 
 use value::Value;
 use ast::InputValue;
@@ -120,7 +120,7 @@ graphql_object!(TestType: () |&self| {
 
 fn run_variable_query<F>(query: &str, vars: Variables, f: F)
 where
-    F: Fn(&HashMap<String, Value>) -> (),
+    F: Fn(&OrderMap<String, Value>) -> (),
 {
     let schema = RootNode::new(TestType, EmptyMutation::<()>::new());
 
@@ -137,7 +137,7 @@ where
 
 fn run_query<F>(query: &str, f: F)
 where
-    F: Fn(&HashMap<String, Value>) -> (),
+    F: Fn(&OrderMap<String, Value>) -> (),
 {
     run_variable_query(query, Variables::new(), f);
 }

juniper/src/integrations/serde.rs

@@ -1,7 +1,8 @@
+use ordermap::OrderMap;
 use serde::{de, ser};
 use serde::ser::SerializeMap;
+
 use std::fmt;
-use std::collections::HashMap;
 
 use {GraphQLError, Value};
 use ast::InputValue;
@@ -9,7 +10,6 @@ use executor::ExecutionError;
 use parser::{ParseError, SourcePosition, Spanning};
 use validation::RuleError;
 
-
 impl ser::Serialize for ExecutionError {
     fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
     where
@@ -130,7 +130,7 @@ impl<'de> de::Deserialize<'de> for InputValue {
             where
                 V: de::MapAccess<'de>,
             {
-                let mut values: HashMap<String, InputValue> = HashMap::new();
+                let mut values: OrderMap<String, InputValue> = OrderMap::new();
 
                 while let Some((key, value)) = try!(visitor.next_entry()) {
                     values.insert(key, value);
@@ -161,7 +161,7 @@ impl ser::Serialize for InputValue {
                 .serialize(serializer),
             InputValue::Object(ref v) => v.iter()
                 .map(|&(ref k, ref v)| (k.item.clone(), v.item.clone()))
-                .collect::<HashMap<_, _>>()
+                .collect::<OrderMap<_, _>>()
                 .serialize(serializer),
         }
     }
@@ -214,7 +214,7 @@ impl<'a> ser::Serialize for Spanning<ParseError<'a>> {
         try!(map.serialize_key("message"));
         try!(map.serialize_value(&message));
 
-        let mut location = HashMap::new();
+        let mut location = OrderMap::new();
         location.insert("line".to_owned(), self.start.line() + 1);
         location.insert("column".to_owned(), self.start.column() + 1);
 

juniper/src/lib.rs

@@ -121,6 +121,7 @@ extern crate serde_derive;
 #[cfg(any(test, feature = "expose-test-schema"))]
 extern crate serde_json;
 
+extern crate ordermap;
 
 #[cfg(any(test, feature = "uuid"))]
 extern crate uuid;

juniper/src/macros/tests/enums.rs

@@ -1,4 +1,4 @@
-use std::collections::HashMap;
+use ordermap::OrderMap;
 
 use executor::Variables;
 use value::Value;
@@ -88,7 +88,7 @@ graphql_object!(Root: () |&self| {
 
 fn run_type_info_query<F>(doc: &str, f: F)
 where
-    F: Fn((&HashMap<String, Value>, &Vec<Value>)) -> (),
+    F: Fn((&OrderMap<String, Value>, &Vec<Value>)) -> (),
 {
     let schema = RootNode::new(Root {}, EmptyMutation::<()>::new());
 

juniper/src/macros/tests/field.rs

@@ -1,4 +1,4 @@
-use std::collections::HashMap;
+use ordermap::OrderMap;
 
 use value::Value;
 use ast::InputValue;
@@ -59,7 +59,7 @@ graphql_interface!(Interface: () |&self| {
 
 fn run_field_info_query<F>(type_name: &str, field_name: &str, f: F)
 where
-    F: Fn(&HashMap<String, Value>) -> (),
+    F: Fn(&OrderMap<String, Value>) -> (),
 {
     let doc = r#"
     query ($typeName: String!) {

juniper/src/macros/tests/input_object.rs

@@ -1,4 +1,4 @@
-use std::collections::HashMap;
+use ordermap::OrderMap;
 
 use ast::{FromInputValue, InputValue};
 use executor::Variables;
@@ -105,7 +105,7 @@ graphql_object!(Root: () |&self| {
 
 fn run_type_info_query<F>(doc: &str, f: F)
 where
-    F: Fn(&HashMap<String, Value>, &Vec<Value>) -> (),
+    F: Fn(&OrderMap<String, Value>, &Vec<Value>) -> (),
 {
     let schema = RootNode::new(Root {}, EmptyMutation::<()>::new());
 

juniper/src/macros/tests/interface.rs

@@ -1,4 +1,4 @@
-use std::collections::HashMap;
+use ordermap::OrderMap;
 use std::marker::PhantomData;
 
 use ast::InputValue;
@@ -135,7 +135,7 @@ graphql_object!(<'a> Root: () as "Root" |&self| {
 
 fn run_type_info_query<F>(type_name: &str, f: F)
 where
-    F: Fn(&HashMap<String, Value>, &Vec<Value>) -> (),
+    F: Fn(&OrderMap<String, Value>, &Vec<Value>) -> (),
 {
     let doc = r#"
     query ($typeName: String!) {

juniper/src/macros/tests/object.rs

@@ -1,4 +1,4 @@
-use std::collections::HashMap;
+use ordermap::OrderMap;
 use std::marker::PhantomData;
 
 use ast::InputValue;
@@ -119,7 +119,7 @@ graphql_object!(<'a> Root: () as "Root" |&self| {
 
 fn run_type_info_query<F>(type_name: &str, f: F)
 where
-    F: Fn(&HashMap<String, Value>, &Vec<Value>) -> (),
+    F: Fn(&OrderMap<String, Value>, &Vec<Value>) -> (),
 {
     let doc = r#"
     query ($typeName: String!) {

juniper/src/macros/tests/scalar.rs

@@ -1,4 +1,4 @@
-use std::collections::HashMap;
+use ordermap::OrderMap;
 
 use executor::Variables;
 use value::Value;
@@ -72,7 +72,7 @@ graphql_object!(Root: () |&self| {
 
 fn run_type_info_query<F>(doc: &str, f: F)
 where
-    F: Fn(&HashMap<String, Value>) -> (),
+    F: Fn(&OrderMap<String, Value>) -> (),
 {
     let schema = RootNode::new(Root {}, EmptyMutation::<()>::new());
 

juniper/src/macros/tests/union.rs

@@ -1,4 +1,4 @@
-use std::collections::HashMap;
+use ordermap::OrderMap;
 use std::marker::PhantomData;
 
 use ast::InputValue;
@@ -112,7 +112,7 @@ graphql_object!(<'a> Root: () as "Root" |&self| {
 
 fn run_type_info_query<F>(type_name: &str, f: F)
 where
-    F: Fn(&HashMap<String, Value>, &Vec<Value>) -> (),
+    F: Fn(&OrderMap<String, Value>, &Vec<Value>) -> (),
 {
     let doc = r#"
     query ($typeName: String!) {

juniper/src/parser/tests/value.rs

@@ -1,9 +1,10 @@
-use std::collections::HashMap;
+use ordermap::OrderMap;
 
 use ast::InputValue;
 use parser::{Lexer, Parser, SourcePosition, Spanning};
 use parser::value::parse_value_literal;
 
+
 fn parse_value(s: &str) -> Spanning<InputValue> {
     let mut lexer = Lexer::new(s);
     let mut parser = Parser::new(&mut lexer).expect(&format!("Lexer error on input {:#?}", s));
@@ -112,7 +113,7 @@ fn input_value_literals() {
         Spanning::start_end(
             &SourcePosition::new(0, 0, 0),
             &SourcePosition::new(2, 0, 2),
-            InputValue::object(HashMap::<String, InputValue>::new())
+            InputValue::object(OrderMap::<String, InputValue>::new())
         )
     );
     assert_eq!(

juniper/src/tests/query_tests.rs

@@ -33,6 +33,70 @@ fn test_hero_name() {
     );
 }
 
+#[test]
+fn test_hero_field_order() {
+    let database = Database::new();
+    let schema = RootNode::new(&database, EmptyMutation::<Database>::new());
+
+    let doc = r#"
+        {
+            hero {
+                id
+                name
+            }
+        }"#;
+    assert_eq!(
+        ::execute(doc, None, &schema, &Variables::new(), &database),
+        Ok((
+            Value::object(
+                vec![
+                    (
+                        "hero",
+                        Value::object(
+                            vec![
+                                ("id", Value::string("2001")),
+                                ("name", Value::string("R2-D2")),
+                            ].into_iter()
+                                .collect(),
+                        ),
+                    ),
+                ].into_iter()
+                    .collect()
+            ),
+            vec![]
+        ))
+    );
+
+    let doc_reversed = r#"
+        {
+            hero {
+                name
+                id
+            }
+        }"#;
+    assert_eq!(
+        ::execute(doc_reversed, None, &schema, &Variables::new(), &database),
+        Ok((
+            Value::object(
+                vec![
+                    (
+                        "hero",
+                        Value::object(
+                            vec![
+                                ("name", Value::string("R2-D2")),
+                                ("id", Value::string("2001")),
+                            ].into_iter()
+                                .collect(),
+                        ),
+                    ),
+                ].into_iter()
+                    .collect()
+            ),
+            vec![]
+        ))
+    );
+}
+
 #[test]
 fn test_hero_name_and_friends() {
     let doc = r#"
@@ -537,8 +601,8 @@ fn test_query_inline_fragments_droid() {
                         "hero",
                         Value::object(
                             vec![
-                                ("__typename", Value::string("Droid")),
                                 ("name", Value::string("R2-D2")),
+                                ("__typename", Value::string("Droid")),
                                 ("primaryFunction", Value::string("Astromech")),
                             ].into_iter()
                                 .collect(),
@@ -574,8 +638,8 @@ fn test_query_inline_fragments_human() {
                         "hero",
                         Value::object(
                             vec![
-                                ("__typename", Value::string("Human")),
                                 ("name", Value::string("Luke Skywalker")),
+                                ("__typename", Value::string("Human")),
                             ].into_iter()
                                 .collect(),
                         ),

juniper/src/tests/type_info_tests.rs

@@ -1,4 +1,4 @@
-use std::collections::HashMap;
+use ordermap::OrderMap;
 
 use executor::{ExecutionResult, Executor, Registry, Variables};
 use value::Value;
@@ -13,7 +13,7 @@ pub struct NodeTypeInfo {
 }
 
 pub struct Node {
-    attributes: HashMap<String, String>,
+    attributes: OrderMap<String, String>,
 }
 
 impl GraphQLType for Node {
@@ -59,7 +59,7 @@ fn test_node() {
         attribute_names: vec!["foo".to_string(), "bar".to_string(), "baz".to_string()],
     };
     let mut node = Node {
-        attributes: HashMap::new(),
+        attributes: OrderMap::new(),
     };
     node.attributes.insert("foo".to_string(), "1".to_string());
     node.attributes.insert("bar".to_string(), "2".to_string());

juniper/src/types/base.rs

@@ -1,5 +1,5 @@
-use std::collections::HashMap;
-use std::collections::hash_map::Entry;
+use ordermap::OrderMap;
+use ordermap::Entry;
 
 use ast::{Directive, FromInputValue, InputValue, Selection};
 use executor::Variables;
@@ -66,17 +66,17 @@ pub enum TypeKind {
 
 /// Field argument container
 pub struct Arguments<'a> {
-    args: Option<HashMap<&'a str, InputValue>>,
+    args: Option<OrderMap<&'a str, InputValue>>,
 }
 
 impl<'a> Arguments<'a> {
     #[doc(hidden)]
     pub fn new(
-        mut args: Option<HashMap<&'a str, InputValue>>,
+        mut args: Option<OrderMap<&'a str, InputValue>>,
         meta_args: &'a Option<Vec<Argument>>,
     ) -> Arguments<'a> {
         if meta_args.is_some() && args.is_none() {
-            args = Some(HashMap::new());
+            args = Some(OrderMap::new());
         }
 
         if let (&mut Some(ref mut args), &Some(ref meta_args)) = (&mut args, meta_args) {
@@ -306,7 +306,7 @@ pub trait GraphQLType: Sized {
         executor: &Executor<Self::Context>,
     ) -> Value {
         if let Some(selection_set) = selection_set {
-            let mut result = HashMap::new();
+            let mut result = OrderMap::new();
             resolve_selection_set_into(self, info, selection_set, executor, &mut result);
             Value::object(result)
         } else {
@@ -320,7 +320,7 @@ fn resolve_selection_set_into<T, CtxT>(
     info: &T::TypeInfo,
     selection_set: &[Selection],
     executor: &Executor<CtxT>,
-    result: &mut HashMap<String, Value>,
+    result: &mut OrderMap<String, Value>,
 ) where
     T: GraphQLType<Context = CtxT>,
 {
@@ -435,7 +435,7 @@ fn resolve_selection_set_into<T, CtxT>(
                     );
 
                     if let Ok(Value::Object(mut hash_map)) = sub_result {
-                        for (k, v) in hash_map.drain() {
+                        for (k, v) in hash_map.drain(..) {
                             result.insert(k, v);
                         }
                     } else if let Err(e) = sub_result {
@@ -480,7 +480,7 @@ fn is_excluded(directives: &Option<Vec<Spanning<Directive>>>, vars: &Variables)
     false
 }
 
-fn merge_key_into(result: &mut HashMap<String, Value>, response_name: &str, value: Value) {
+fn merge_key_into(result: &mut OrderMap<String, Value>, response_name: &str, value: Value) {
     match result.entry(response_name.to_owned()) {
         Entry::Occupied(mut e) => match (e.get_mut().as_mut_object_value(), value) {
             (Some(dest_obj), Value::Object(src_obj)) => {
@@ -494,7 +494,7 @@ fn merge_key_into(result: &mut HashMap<String, Value>, response_name: &str, valu
     }
 }
 
-fn merge_maps(dest: &mut HashMap<String, Value>, src: HashMap<String, Value>) {
+fn merge_maps(dest: &mut OrderMap<String, Value>, src: OrderMap<String, Value>) {
     for (key, value) in src {
         if dest.contains_key(&key) {
             merge_key_into(dest, &key, value);

juniper/src/value.rs

@@ -1,4 +1,4 @@
-use std::collections::HashMap;
+use ordermap::OrderMap;
 use std::hash::Hash;
 
 use parser::Spanning;
@@ -22,7 +22,7 @@ pub enum Value {
     String(String),
     Boolean(bool),
     List(Vec<Value>),
-    Object(HashMap<String, Value>),
+    Object(OrderMap<String, Value>),
 }
 
 impl Value {
@@ -59,7 +59,7 @@ impl Value {
     }
 
     /// Construct an object value.
-    pub fn object<K>(o: HashMap<K, Value>) -> Value
+    pub fn object<K>(o: OrderMap<K, Value>) -> Value
     where
         K: Into<String> + Eq + Hash,
     {
@@ -77,7 +77,7 @@ impl Value {
     }
 
     /// View the underlying object value, if present.
-    pub fn as_object_value(&self) -> Option<&HashMap<String, Value>> {
+    pub fn as_object_value(&self) -> Option<&OrderMap<String, Value>> {
         match *self {
             Value::Object(ref o) => Some(o),
             _ => None,
@@ -85,7 +85,7 @@ impl Value {
     }
 
     /// Mutable view into the underlying object value, if present.
-    pub fn as_mut_object_value(&mut self) -> Option<&mut HashMap<String, Value>> {
+    pub fn as_mut_object_value(&mut self) -> Option<&mut OrderMap<String, Value>> {
         match *self {
             Value::Object(ref mut o) => Some(o),
             _ => None,