all: Change GraphQL output to follow the order of fields in the query

That is demanded by the GraphQL spec. We need to have a predictable order
to ensure attestations remain stable, though this change breaks
attestations since it changes the order in which fields appear in the
output from alphabetical (however a `BTreeMap` orders string keys) to the
order in which fields appear in a query.

It also allows us to replace `BTreeMaps`, which are fairly memory
intensive, with cheaper `Vec`.

The test changes all reflect the changed output behavior; they only reorder
fields in the expected output but do not otherwise alter the tests.

Fixes #2943

Author: lutter

core/tests/interfaces.rs

@@ -553,7 +553,7 @@ async fn interface_inline_fragment_with_subquery() {
     .unwrap();
     let data = extract_data!(res).unwrap();
     let exp = object! {
-        leggeds: vec![ object!{ airspeed: 5, legs: 2, parent: object! { id: "mama_bird" } },
+        leggeds: vec![ object!{ legs: 2, airspeed: 5, parent: object! { id: "mama_bird" } },
                        object!{ legs: 4 }]
     };
     assert_eq!(data, exp);

graph/src/data/graphql/object_macro.rs

@@ -83,17 +83,17 @@ macro_rules! impl_into_values {
 
 impl_into_values![(String, String), (f64, Float), (bool, Boolean)];
 
-/// Creates a `graphql_parser::query::Value::Object` from key/value pairs.
+/// Creates a `data::value::Value::Object` from key/value pairs.
 #[macro_export]
 macro_rules! object {
     ($($name:ident: $value:expr,)*) => {
         {
-            let mut result = ::std::collections::BTreeMap::new();
+            let mut result = $crate::data::value::Object::new();
             $(
                 let value = $crate::data::graphql::object_macro::IntoValue::into_value($value);
                 result.insert(stringify!($name).to_string(), value);
             )*
-            $crate::prelude::r::Value::object(result)
+            $crate::prelude::r::Value::Object(result)
         }
     };
     ($($name:ident: $value:expr),*) => {

graph/src/data/value.rs

@@ -5,24 +5,41 @@ use std::collections::BTreeMap;
 use std::convert::TryFrom;
 use std::iter::FromIterator;
 
+const TOMBSTONE_KEY: &str = "*dead*";
+
+#[derive(Clone, Debug, PartialEq)]
+struct Entry {
+    key: String,
+    value: Value,
+}
+
 #[derive(Clone, PartialEq)]
-pub struct Object(BTreeMap<String, Value>);
+pub struct Object(Vec<Entry>);
 
 impl Object {
     pub fn new() -> Self {
-        Self(BTreeMap::default())
+        Self(Vec::new())
     }
 
     pub fn get(&self, key: &str) -> Option<&Value> {
-        self.0.get(key)
+        self.0
+            .iter()
+            .find(|entry| entry.key == key)
+            .map(|entry| &entry.value)
     }
 
     pub fn remove(&mut self, key: &str) -> Option<Value> {
-        self.0.remove(key)
+        self.0
+            .iter_mut()
+            .find(|entry| entry.key == key)
+            .map(|entry| {
+                entry.key = TOMBSTONE_KEY.to_string();
+                std::mem::replace(&mut entry.value, Value::Null)
+            })
     }
 
-    pub fn iter(&self) -> std::collections::btree_map::Iter<String, Value> {
-        self.into_iter()
+    pub fn iter(&self) -> impl Iterator<Item = (&String, &Value)> {
+        ObjectIter::new(self)
     }
 
     fn len(&self) -> usize {
@@ -34,33 +51,92 @@ impl Object {
     }
 
     pub fn insert(&mut self, key: String, value: Value) -> Option<Value> {
-        self.0.insert(key, value)
+        match self.0.iter_mut().find(|entry| &entry.key == &key) {
+            Some(entry) => Some(std::mem::replace(&mut entry.value, value)),
+            None => {
+                self.0.push(Entry { key, value });
+                None
+            }
+        }
     }
 }
 
 impl FromIterator<(String, Value)> for Object {
     fn from_iter<T: IntoIterator<Item = (String, Value)>>(iter: T) -> Self {
-        Object(BTreeMap::from_iter(iter))
+        let mut items: Vec<_> = Vec::new();
+        for (key, value) in iter {
+            items.push(Entry { key, value })
+        }
+        Object(items)
+    }
+}
+
+pub struct ObjectOwningIter {
+    iter: std::vec::IntoIter<Entry>,
+}
+
+impl Iterator for ObjectOwningIter {
+    type Item = (String, Value);
+
+    fn next(&mut self) -> Option<Self::Item> {
+        while let Some(entry) = self.iter.next() {
+            if &entry.key != TOMBSTONE_KEY {
+                return Some((entry.key, entry.value));
+            }
+        }
+        None
     }
 }
 
 impl IntoIterator for Object {
     type Item = (String, Value);
 
-    type IntoIter = std::collections::btree_map::IntoIter<String, Value>;
+    type IntoIter = ObjectOwningIter;
 
     fn into_iter(self) -> Self::IntoIter {
-        self.0.into_iter()
+        ObjectOwningIter {
+            iter: self.0.into_iter(),
+        }
     }
 }
 
-impl<'a> IntoIterator for &'a Object {
+pub struct ObjectIter<'a> {
+    iter: std::slice::Iter<'a, Entry>,
+}
+
+impl<'a> ObjectIter<'a> {
+    fn new(object: &'a Object) -> Self {
+        Self {
+            iter: object.0.as_slice().iter(),
+        }
+    }
+}
+impl<'a> Iterator for ObjectIter<'a> {
     type Item = (&'a String, &'a Value);
 
-    type IntoIter = std::collections::btree_map::Iter<'a, String, Value>;
+    fn next(&mut self) -> Option<Self::Item> {
+        while let Some(entry) = self.iter.next() {
+            if &entry.key != TOMBSTONE_KEY {
+                return Some((&entry.key, &entry.value));
+            }
+        }
+        None
+    }
+}
+
+impl<'a> IntoIterator for &'a Object {
+    type Item = <ObjectIter<'a> as Iterator>::Item;
+
+    type IntoIter = ObjectIter<'a>;
 
     fn into_iter(self) -> Self::IntoIter {
-        self.0.iter()
+        ObjectIter::new(self)
+    }
+}
+
+impl CacheWeight for Entry {
+    fn indirect_weight(&self) -> usize {
+        self.key.indirect_weight() + self.value.indirect_weight()
     }
 }
 
@@ -72,7 +148,7 @@ impl CacheWeight for Object {
 
 impl Default for Object {
     fn default() -> Self {
-        Self(BTreeMap::default())
+        Self(Vec::default())
     }
 }
 
@@ -96,7 +172,11 @@ pub enum Value {
 
 impl Value {
     pub fn object(map: BTreeMap<String, Value>) -> Self {
-        Value::Object(Object(map))
+        let items = map
+            .into_iter()
+            .map(|(key, value)| Entry { key, value })
+            .collect();
+        Value::Object(Object(items))
     }
 
     pub fn is_null(&self) -> bool {