From f5ce9f2d790e102a42554b6acebaae7943cbea23 Mon Sep 17 00:00:00 2001 From: abrackx Date: Sun, 28 Feb 2021 23:10:53 -0500 Subject: [PATCH] Switch to HashMap for the internal representation of object fields (#872) * Resolves #818. Updates Object key_value_list to use HashMap>. Updates test cases. * Updates to use IndexMap. Reverts changes to test cases. Co-authored-by: Christian Legnitto --- integration_tests/async_await/src/main.rs | 3 +- juniper/src/executor_tests/async_await/mod.rs | 3 +- juniper/src/integrations/serde.rs | 2 +- juniper/src/types/base.rs | 39 -------- juniper/src/value/mod.rs | 2 +- juniper/src/value/object.rs | 91 +++++-------------- 6 files changed, 28 insertions(+), 112 deletions(-) diff --git a/integration_tests/async_await/src/main.rs b/integration_tests/async_await/src/main.rs index 6d9d3d41..42d914f7 100644 --- a/integration_tests/async_await/src/main.rs +++ b/integration_tests/async_await/src/main.rs @@ -96,8 +96,7 @@ async fn async_simple() { assert!(errs.is_empty()); - let mut obj = res.into_object().unwrap(); - obj.sort_by_field(); + let obj = res.into_object().unwrap(); let value = Value::Object(obj); assert_eq!( diff --git a/juniper/src/executor_tests/async_await/mod.rs b/juniper/src/executor_tests/async_await/mod.rs index 064b4067..769de94e 100644 --- a/juniper/src/executor_tests/async_await/mod.rs +++ b/juniper/src/executor_tests/async_await/mod.rs @@ -91,8 +91,7 @@ async fn async_simple() { assert!(errs.is_empty()); - let mut obj = res.into_object().unwrap(); - obj.sort_by_field(); + let obj = res.into_object().unwrap(); let value = Value::Object(obj); assert_eq!( diff --git a/juniper/src/integrations/serde.rs b/juniper/src/integrations/serde.rs index 985fa5d6..d29638c4 100644 --- a/juniper/src/integrations/serde.rs +++ b/juniper/src/integrations/serde.rs @@ -381,7 +381,7 @@ where { let mut map = serializer.serialize_map(Some(self.field_count()))?; - for &(ref f, ref v) in self.iter() { + for (ref f, ref v) in self.iter() { map.serialize_key(f)?; map.serialize_value(v)?; } diff --git a/juniper/src/types/base.rs b/juniper/src/types/base.rs index 4966f303..086722cd 100644 --- a/juniper/src/types/base.rs +++ b/juniper/src/types/base.rs @@ -599,44 +599,5 @@ where /// Merges `response_name`/`value` pair into `result` pub(crate) fn merge_key_into(result: &mut Object, response_name: &str, value: Value) { - if let Some(&mut (_, ref mut e)) = result - .iter_mut() - .find(|&&mut (ref key, _)| key == response_name) - { - match *e { - Value::Object(ref mut dest_obj) => { - if let Value::Object(src_obj) = value { - merge_maps(dest_obj, src_obj); - } - } - Value::List(ref mut dest_list) => { - if let Value::List(src_list) = value { - dest_list - .iter_mut() - .zip(src_list.into_iter()) - .for_each(|(d, s)| { - if let Value::Object(ref mut d_obj) = *d { - if let Value::Object(s_obj) = s { - merge_maps(d_obj, s_obj); - } - } - }); - } - } - _ => {} - } - return; - } result.add_field(response_name, value); } - -/// Merges `src` object's fields into `dest` -fn merge_maps(dest: &mut Object, src: Object) { - for (key, value) in src { - if dest.contains_field(&key) { - merge_key_into(dest, &key, value); - } else { - dest.add_field(key, value); - } - } -} diff --git a/juniper/src/value/mod.rs b/juniper/src/value/mod.rs index fab55d7b..ab8a762b 100644 --- a/juniper/src/value/mod.rs +++ b/juniper/src/value/mod.rs @@ -195,7 +195,7 @@ impl ToInputValue for Value { ), Value::Object(ref o) => InputValue::Object( o.iter() - .map(|&(ref k, ref v)| { + .map(|(k, v)| { ( Spanning::unlocated(k.clone()), Spanning::unlocated(v.to_input_value()), diff --git a/juniper/src/value/object.rs b/juniper/src/value/object.rs index cd19e0fd..64299b8a 100644 --- a/juniper/src/value/object.rs +++ b/juniper/src/value/object.rs @@ -1,11 +1,20 @@ -use std::{iter::FromIterator, vec::IntoIter}; +use std::iter::FromIterator; use super::Value; +use indexmap::map::{IndexMap, IntoIter}; /// A Object value -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, Clone)] pub struct Object { - key_value_list: Vec<(String, Value)>, + key_value_list: IndexMap>, +} + +impl PartialEq for Object { + fn eq(&self, _: &Object) -> bool { + match self { + Object { key_value_list } => self.key_value_list == *key_value_list, + } + } } impl Object { @@ -13,7 +22,7 @@ impl Object { /// preallocated slots for field-value pairs pub fn with_capacity(size: usize) -> Self { Object { - key_value_list: Vec::with_capacity(size), + key_value_list: IndexMap::with_capacity(size), } } @@ -26,39 +35,26 @@ impl Object { K: Into, for<'a> &'a str: PartialEq, { - if let Some(item) = self - .key_value_list - .iter_mut() - .find(|&&mut (ref key, _)| (key as &str) == k) - { - return Some(::std::mem::replace(&mut item.1, value)); - } - self.key_value_list.push((k.into(), value)); - None + self.key_value_list.insert(k.into(), value) } /// Check if the object already contains a field with the given name pub fn contains_field(&self, f: K) -> bool where + K: Into, for<'a> &'a str: PartialEq, { - self.key_value_list - .iter() - .any(|&(ref key, _)| (key as &str) == f) + self.key_value_list.contains_key(&f.into()) } /// Get a iterator over all field value pairs - pub fn iter(&self) -> impl Iterator)> { - FieldIter { - inner: self.key_value_list.iter(), - } + pub fn iter(&self) -> impl Iterator)> { + self.key_value_list.iter() } /// Get a iterator over all mutable field value pairs - pub fn iter_mut(&mut self) -> impl Iterator)> { - FieldIterMut { - inner: self.key_value_list.iter_mut(), - } + pub fn iter_mut(&mut self) -> impl Iterator)> { + self.key_value_list.iter_mut() } /// Get the current number of fields @@ -69,29 +65,16 @@ impl Object { /// Get the value for a given field pub fn get_field_value(&self, key: K) -> Option<&Value> where + K: Into, for<'a> &'a str: PartialEq, { - self.key_value_list - .iter() - .find(|&&(ref k, _)| (k as &str) == key) - .map(|&(_, ref value)| value) - } - - /// Recursively sort all keys by field. - pub fn sort_by_field(&mut self) { - self.key_value_list - .sort_by(|(key1, _), (key2, _)| key1.cmp(key2)); - for (_, ref mut value) in &mut self.key_value_list { - if let Value::Object(ref mut o) = value { - o.sort_by_field(); - } - } + self.key_value_list.get(&key.into()) } } impl IntoIterator for Object { type Item = (String, Value); - type IntoIter = IntoIter; + type IntoIter = IntoIter>; fn into_iter(self) -> Self::IntoIter { self.key_value_list.into_iter() @@ -115,7 +98,7 @@ where { let iter = iter.into_iter(); let mut ret = Self { - key_value_list: Vec::with_capacity(iter.size_hint().0), + key_value_list: IndexMap::with_capacity(iter.size_hint().0), }; for (k, v) in iter { ret.add_field(k, v); @@ -123,29 +106,3 @@ where ret } } - -#[doc(hidden)] -pub struct FieldIter<'a, S: 'a> { - inner: ::std::slice::Iter<'a, (String, Value)>, -} - -impl<'a, S> Iterator for FieldIter<'a, S> { - type Item = &'a (String, Value); - - fn next(&mut self) -> Option { - self.inner.next() - } -} - -#[doc(hidden)] -pub struct FieldIterMut<'a, S: 'a> { - inner: ::std::slice::IterMut<'a, (String, Value)>, -} - -impl<'a, S> Iterator for FieldIterMut<'a, S> { - type Item = &'a mut (String, Value); - - fn next(&mut self) -> Option { - self.inner.next() - } -}