de: setup reading attributes part of non-NodeStart nodes

src/de/custom.rs

@@ -1,27 +1,27 @@
-use serde::de::{DeserializeSeed, EnumAccess, IntoDeserializer, VariantAccess, Visitor};
+use serde::de::{Deserializer, DeserializeSeed, EnumAccess, Error, IntoDeserializer, VariantAccess, Visitor};
 
-use de::{Deserializer, Result};
-use error::Error;
 use node_types::StandardType;
 
-pub struct Custom<'a, 'de: 'a> {
-  de: &'a mut Deserializer<'de>,
+pub struct Custom<D> {
+  de: D,
   node_type: StandardType,
 }
 
-impl<'de, 'a> Custom<'a, 'de> {
-  pub fn new(de: &'a mut Deserializer<'de>, node_type: StandardType) -> Self {
+impl<D> Custom<D> {
+  pub fn new(de: D, node_type: StandardType) -> Self {
     trace!("Custom::new(node_type: {:?})", node_type);
 
     Self { de, node_type }
   }
 }
 
-impl<'de, 'a> EnumAccess<'de> for Custom<'a, 'de> {
-  type Error = Error;
+impl<'de, D> EnumAccess<'de> for Custom<D>
+  where D: Deserializer<'de>
+{
+  type Error = D::Error;
   type Variant = Self;
 
-  fn variant_seed<V>(self, seed: V) -> Result<(V::Value, Self::Variant)>
+  fn variant_seed<V>(self, seed: V) -> Result<(V::Value, Self::Variant), D::Error>
     where V: DeserializeSeed<'de>
   {
     trace!("<Custom as EnumAccess>::variant_seed(node_type: {:?})", self.node_type);
@@ -30,30 +30,52 @@ impl<'de, 'a> EnumAccess<'de> for Custom<'a, 'de> {
   }
 }
 
-impl<'de, 'a> VariantAccess<'de> for Custom<'a, 'de> {
-  type Error = Error;
+impl<'de, D> VariantAccess<'de> for Custom<D>
+  where D: Deserializer<'de>
+{
+  type Error = D::Error;
 
-  fn unit_variant(self) -> Result<()> {
-    Err(Error::Message("unit variant not supported".into()))
+  fn unit_variant(self) -> Result<(), D::Error> {
+    Err(D::Error::custom("unit variant not supported"))
   }
 
   // Used to get the value the `Visitor` wants through the `DeserializeSeed`
-  fn newtype_variant_seed<T>(self, seed: T) -> Result<T::Value>
+  fn newtype_variant_seed<T>(self, seed: T) -> Result<T::Value, D::Error>
     where T: DeserializeSeed<'de>
   {
     trace!("<Custom as VariantAccess>::newtype_variant_seed()");
     seed.deserialize(self.de)
   }
 
-  fn tuple_variant<V>(self, _len: usize, _visitor: V) -> Result<V::Value>
+  fn tuple_variant<V>(self, _len: usize, _visitor: V) -> Result<V::Value, D::Error>
     where V: Visitor<'de>
   {
-    Err(Error::Message("tuple variant not supported".into()))
+    Err(D::Error::custom("tuple variant not supported"))
   }
 
-  fn struct_variant<V>(self, _fields: &'static [&'static str], _visitor: V) -> Result<V::Value>
+  fn struct_variant<V>(self, _fields: &'static [&'static str], _visitor: V) -> Result<V::Value, D::Error>
     where V: Visitor<'de>
   {
-    Err(Error::Message("struct variant not supported".into()))
+    Err(D::Error::custom("struct variant not supported"))
+  }
+}
+
+impl<'de, D> Deserializer<'de> for Custom<D>
+  where D: Deserializer<'de>
+{
+  type Error = D::Error;
+
+  #[inline]
+  fn deserialize_any<V>(self, visitor: V) -> Result<V::Value, D::Error>
+    where V: Visitor<'de>
+  {
+    trace!("<Custom as Deserializer>::deserialize_any(node_type: {:?})", self.node_type);
+    visitor.visit_enum(self)
+  }
+
+  forward_to_deserialize_any! {
+    bool i8 i16 i32 i64 i128 u8 u16 u32 u64 u128 f32 f64 char str
+    string bytes byte_buf option unit unit_struct newtype_struct seq
+    tuple tuple_struct map struct enum identifier ignored_any
   }
 }

src/de/mod.rs

@@ -10,11 +10,13 @@ use node_types::StandardType;
 use reader::Reader;
 
 mod custom;
+mod node_contents;
 mod seq;
 mod structure;
 mod tuple;
 
 use self::custom::Custom;
+use self::node_contents::NodeContents;
 use self::seq::Seq;
 use self::structure::Struct;
 use self::tuple::TupleBytesDeserializer;
@@ -32,6 +34,7 @@ pub struct Deserializer<'de> {
   read_mode: ReadMode,
   node_stack: Vec<(StandardType, bool)>,
   first_struct: bool,
+  ignore_attributes: bool,
 
   reader: Reader<'de>,
 }
@@ -52,6 +55,7 @@ impl<'de> Deserializer<'de> {
       read_mode: ReadMode::Single,
       node_stack: Vec::new(),
       first_struct: true,
+      ignore_attributes: true,
       reader,
     })
   }
@@ -377,16 +381,30 @@ impl<'de, 'a> de::Deserializer<'de> for &'a mut Deserializer<'de> {
   {
     trace!("Deserializer::deserialize_tuple_struct(name: {:?}, len: {})", name, len);
 
-    if name == "__key" {
-      let (node_type, _is_array) = self.reader.last_node_type().ok_or(KbinErrorKind::InvalidState)?;
-      visitor.visit_enum(Custom::new(self, node_type))
-    } else {
-      let old_read_mode = self.set_read_mode(ReadMode::Array);
-      let value = visitor.visit_seq(Seq::new(self, Some(len))?)?;
-      self.read_mode = old_read_mode;
-      self.reader.data_buf.realign_reads(None)?;
+    match name {
+      "__key" => {
+        self.ignore_attributes = false;
 
-      Ok(value)
+        let (node_type, _is_array) = self.reader.last_node_type().ok_or(KbinErrorKind::InvalidState)?;
+        visitor.visit_enum(Custom::new(self, node_type))
+      },
+      "__value" => {
+        let (node_type, _is_array) = self.reader.last_node_type().ok_or(KbinErrorKind::InvalidState)?;
+        debug!("Deserializer::deserialize_tuple_struct(name: {:?}) => node_type: {:?}", name, node_type);
+
+        let value = visitor.visit_map(NodeContents::new(self, node_type))?;
+        self.ignore_attributes = true;
+
+        Ok(value)
+      },
+      _ => {
+        let old_read_mode = self.set_read_mode(ReadMode::Array);
+        let value = visitor.visit_seq(Seq::new(self, Some(len))?)?;
+        self.read_mode = old_read_mode;
+        self.reader.data_buf.realign_reads(None)?;
+
+        Ok(value)
+      },
     }
   }
 
@@ -460,5 +478,3 @@ impl<'de, 'a> de::Deserializer<'de> for &'a mut Deserializer<'de> {
     self.deserialize_any(visitor)
   }
 }
-
-// TODO: Add test with array of two Ip4

src/de/node_contents.rs

@@ -0,0 +1,87 @@
+use serde::de::{DeserializeSeed, IntoDeserializer, MapAccess};
+
+use de::{Custom, Deserializer, ReadMode, Result};
+use error::{Error, KbinErrorKind};
+use node_types::StandardType;
+
+#[derive(Debug)]
+enum ReadState {
+  Value,
+  Attributes,
+}
+
+pub struct NodeContents<'a, 'de: 'a> {
+  de: &'a mut Deserializer<'de>,
+  node_type: StandardType,
+  state: ReadState,
+}
+
+impl<'de, 'a> NodeContents<'a, 'de> {
+  pub fn new(de: &'a mut Deserializer<'de>, node_type: StandardType) -> Self {
+    trace!("--> NodeContents::new()");
+
+    Self {
+      de,
+      node_type,
+      state: ReadState::Value,
+    }
+  }
+}
+
+impl<'de, 'a> MapAccess<'de> for NodeContents<'a, 'de> {
+  type Error = Error;
+
+  fn next_key_seed<K>(&mut self, seed: K) -> Result<Option<K::Value>>
+    where K: DeserializeSeed<'de>
+  {
+    trace!("--> <NodeContents as MapAccess>::next_key_seed(state: {:?})", self.state);
+
+    match self.state {
+      ReadState::Value => {
+        let de = "__value".into_deserializer();
+        seed.deserialize(Custom::new(de, self.node_type)).map(Some)
+      },
+      ReadState::Attributes => {
+        let (node_type, _is_array) = self.de.reader.read_node_type()?;
+        debug!("NodeContents::next_key_seed() => node_type: {:?}", node_type);
+
+        match node_type {
+          StandardType::Attribute => {},
+          StandardType::NodeEnd |
+          StandardType::FileEnd => {
+            debug!("<-- <NodeContents as MapAccess>::next_key_seed() => end of map, node stack: {:?}", self.de.node_stack);
+
+            return Ok(None);
+          },
+          _ => return Err(KbinErrorKind::InvalidState.into()),
+        };
+
+        let old_read_mode = self.de.set_read_mode(ReadMode::Key);
+        let key = seed.deserialize(&mut *self.de).map(Some)?;
+        self.de.read_mode = old_read_mode;
+
+        self.node_type = node_type;
+
+        Ok(key)
+      },
+    }
+  }
+
+  fn next_value_seed<V>(&mut self, seed: V) -> Result<V::Value>
+    where V: DeserializeSeed<'de>
+  {
+    trace!("--> <NodeContents as MapAccess>::next_value_seed(state: {:?})", self.state);
+
+    match self.state {
+      ReadState::Value => {
+        let value = seed.deserialize(&mut *self.de)?;
+        self.state = ReadState::Attributes;
+
+        Ok(value)
+      },
+      ReadState::Attributes => {
+        seed.deserialize(Custom::new(&mut *self.de, self.node_type))
+      },
+    }
+  }
+}

src/de/structure.rs

@@ -1,6 +1,6 @@
 use std::marker::PhantomData;
 
-use serde::de::{DeserializeSeed, MapAccess};
+use serde::de::{Deserialize, DeserializeSeed, MapAccess};
 
 use de::{Deserializer, ReadMode, Result};
 use error::{Error, KbinErrorKind};
@@ -53,14 +53,18 @@ impl<'de, 'a> MapAccess<'de> for Struct<'a, 'de> {
       StandardType::Attribute => {
         debug!("Struct::next_key_seed() => got an attribute!");
       },
-      _ => {
+      _ if self.de.ignore_attributes => {
         // TODO(mbilker): Fix processing of `Attribute` nodes for non-NodeStart
         // elements
         loop {
           let (node_type, _is_array) = self.de.reader.peek_node_type()?;
           if node_type == StandardType::Attribute {
+            let _ = self.de.reader.read_node_type()?;
             warn!("Struct::next_key_seed() => ignoring Attribute node");
-            let key: Option<String> = self.next_key_seed(PhantomData)?;
+
+            let old_read_mode = self.de.set_read_mode(ReadMode::Key);
+            let key = String::deserialize(&mut *self.de).map(Some)?;
+            self.de.read_mode = old_read_mode;
             warn!("Struct::next_key_seed() => ignored Attribute key: {:?}", key);
 
             self.values_to_consume += 1;
@@ -75,6 +79,7 @@ impl<'de, 'a> MapAccess<'de> for Struct<'a, 'de> {
           return Err(KbinErrorKind::TypeMismatch(*StandardType::NodeEnd, *node_type).into());
         }
       },
+      _ => {},
     }
 
     // Store the current node type on the stack for stateful handling based on
@@ -90,17 +95,19 @@ impl<'de, 'a> MapAccess<'de> for Struct<'a, 'de> {
     debug!("--> <Struct as MapAccess>::next_value_seed()");
     let value = seed.deserialize(&mut *self.de)?;
 
-    // Cannot use `next_value_seed` recursively here as it would restart this for loop
-    for _ in 0..self.values_to_consume {
-      warn!("Struct::next_value_seed() => ignoring Attribute node value");
-      let seed = PhantomData;
-      let value: String = seed.deserialize(&mut *self.de)?;
-      warn!("Struct::next_value_seed() => ignored Attribute value: {:?}", value);
+    if self.de.ignore_attributes {
+      // Cannot use `next_value_seed` recursively here as it would restart this for loop
+      for _ in 0..self.values_to_consume {
+        warn!("Struct::next_value_seed() => ignoring Attribute node value");
+        let seed = PhantomData;
+        let value: String = seed.deserialize(&mut *self.de)?;
+        warn!("Struct::next_value_seed() => ignored Attribute value: {:?}", value);
 
-      let popped = self.de.node_stack.pop();
-      debug!("<Struct as MapAccess>::next_value_seed() => popped: {:?}, node_stack: {:?}", popped, self.de.node_stack);
+        let popped = self.de.node_stack.pop();
+        debug!("<Struct as MapAccess>::next_value_seed() => popped: {:?}, node_stack: {:?}", popped, self.de.node_stack);
+      }
+      self.values_to_consume = 0;
     }
-    self.values_to_consume = 0;
 
     let popped = self.de.node_stack.pop();
     debug!("<Struct as MapAccess>::next_value_seed() => popped: {:?}, node_stack: {:?}", popped, self.de.node_stack);

src/node/de.rs

@@ -14,7 +14,7 @@ pub(crate) struct NodeVisitor {
 }
 
 impl<'de> NodeVisitor {
-  pub(crate) fn map_to_node<A>(node_type: StandardType, key: &str, map: &mut A) -> Result<Node, A::Error>
+  fn map_to_node<A>(node_type: StandardType, key: &str, map: &mut A) -> Result<Node, A::Error>
     where A: MapAccess<'de>
   {
     trace!("NodeVisitor::map_to_node(node_type: {:?})", node_type);
@@ -29,10 +29,11 @@ impl<'de> NodeVisitor {
       },
       // TODO: roll up `NodeStart` and everything else into a single map handler
       node_type => {
-        let value = try!(map.next_value());
-        debug!("NodeVisitor::map_to_node(node_type: {:?}) => value: {:?}", node_type, value);
+        //let value = try!(map.next_value());
+        //debug!("NodeVisitor::map_to_node(node_type: {:?}) => value: {:?}", node_type, value);
 
-        let node = Node::with_value(key.to_owned(), value);
+        //let node = Node::with_value(key.to_owned(), value);
+        let node = try!(map.next_value_seed(NodeWithValueSeed(key.to_owned())));
         debug!("NodeVisitor::map_to_node(node_type: {:?}) => node: {:?}", node_type, node);
 
         Ok(node)
@@ -56,50 +57,65 @@ impl<'de> Visitor<'de> for NodeVisitor {
 
     let mut attributes = None;
     let mut nodes = IndexMap::new();
+    let mut value = None;
 
     while let Some(NodeStart { key, node_type }) = try!(map.next_key()) {
       debug!("NodeVisitor::visit_map() => node_type: {:?}, key: {:?}", node_type, key);
 
-      match node_type {
-        StandardType::Attribute => {
-          let value = map.next_value();
-          debug!("NodeVisitor::visit_map() => value: {:?}", value);
+      if key == "__value" {
+        trace!("NodeVisitor::visit_map() => got __value, getting node value");
 
-          if let Value::Attribute(s) = try!(value) {
-            let key = String::from(&key["attr_".len()..]);
-            let attributes = attributes.get_or_insert_with(IndexMap::new);
-            attributes.insert(key, s);
-          } else {
-            return Err(A::Error::custom("`Attribute` node must have `Value::Attribute` value"));
-          }
-        },
-        _ => {
-          let node = NodeVisitor::map_to_node(node_type, &key, &mut map)?;
-          debug!("NodeVisitor::visit_map() => node: {:?}", node);
+        let node_value = try!(map.next_value());
+        debug!("NodeVisitor::visit_map() => node value: {:?}", node_value);
 
-          if !nodes.contains_key(&key) {
-            nodes.insert(key, node);
-          } else {
-            let mut new_key = format!("{}1", key);
-            let mut i = 2;
-            while nodes.contains_key(&new_key) {
-              new_key.truncate(key.len());
-              write!(new_key, "{}", i);
-              i += 1;
+        value = Some(node_value);
+      } else {
+        match node_type {
+          StandardType::Attribute => {
+            let value = map.next_value();
+            debug!("NodeVisitor::visit_map() => value: {:?}", value);
+
+            if let Value::Attribute(s) = try!(value) {
+              let key = String::from(&key["attr_".len()..]);
+              let attributes = attributes.get_or_insert_with(IndexMap::new);
+              attributes.insert(key, s);
+            } else {
+              return Err(A::Error::custom("`Attribute` node must have `Value::Attribute` value"));
             }
-            debug!("Node::visit_map() => next open key: {:?}", new_key);
-            nodes.insert(new_key, node);
-          }
-        },
-      };
+          },
+          _ => {
+            let node = NodeVisitor::map_to_node(node_type, &key, &mut map)?;
+            debug!("NodeVisitor::visit_map() => node: {:?}", node);
+
+            if !nodes.contains_key(&key) {
+              nodes.insert(key, node);
+            } else {
+              let mut new_key = format!("{}1", key);
+              let mut i = 2;
+              while nodes.contains_key(&new_key) {
+                new_key.truncate(key.len());
+                write!(new_key, "{}", i);
+                i += 1;
+              }
+              debug!("Node::visit_map() => next open key: {:?}", new_key);
+              nodes.insert(new_key, node);
+            }
+          },
+        };
+      }
     }
 
-    debug!("NodeVisitor::visit_map() => nodes: {:#?}", nodes);
+    let children = match nodes.len() {
+      0 => None,
+      _ => Some(nodes),
+    };
+
+    //debug!("NodeVisitor::visit_map() => nodes: {:#?}", nodes);
     Ok(Node {
       key: self.key.unwrap_or_else(|| "".to_owned()),
       attributes,
-      children: Some(nodes),
-      value: None,
+      children,
+      value,
     })
   }
 
@@ -108,10 +124,10 @@ impl<'de> Visitor<'de> for NodeVisitor {
     where A: SeqAccess<'de>
   {
     trace!("NodeVisitor::visit_seq()");
-    let key = seq.next_element()?.unwrap();
-    let attributes = seq.next_element()?.unwrap();
-    let children = seq.next_element()?.unwrap();
-    let value = seq.next_element()?.unwrap();
+    let key = seq.next_element()?.ok_or_else(|| A::Error::custom("first element must be `key`"))?;
+    let attributes = seq.next_element()?.ok_or_else(|| A::Error::custom("second element must be `attributes`"))?;
+    let children = seq.next_element()?.ok_or_else(|| A::Error::custom("third element must be `children`"))?;
+    let value = seq.next_element()?.ok_or_else(|| A::Error::custom("fourth element must be `value`"))?;
     Ok(Node {
       key,
       attributes,
@@ -201,28 +217,28 @@ impl<'de, E: Error> de::Deserializer<'de> for NodeDeserializer<E> {
 impl<'de, E: Error> SeqAccess<'de> for NodeDeserializer<E> {
   type Error = E;
 
-  /// "Deserializes" the key, attributes as (key, string), and children as
-  /// (key, node)
+  /// "Deserializes" the key, attributes as (key, string), children as
+  /// (key, node), and value as itself
   fn next_element_seed<T>(&mut self, seed: T) -> Result<Option<T::Value>, Self::Error>
     where T: DeserializeSeed<'de>
   {
+    macro_rules! map_deserializer {
+      ($value:expr) => {
+        match $value.take() {
+          Some(value) => {
+            let deserializer = MapDeserializer::new(value.into_iter());
+            seed.deserialize(deserializer).map(Some)
+          },
+          None => seed.deserialize(().into_deserializer()).map(Some),
+        }
+      };
+    }
+
     trace!("--> <NodeDeserializer as SeqAccess>::next_element_seed(index: {})", self.index);
     let value = match self.index {
       0 => seed.deserialize(self.node.key.as_str().into_deserializer()).map(Some),
-      1 => match self.node.attributes.take() {
-        Some(attributes) => {
-          let deserializer = MapDeserializer::new(attributes.into_iter());
-          seed.deserialize(deserializer).map(Some)
-        },
-        None => seed.deserialize(().into_deserializer()).map(Some),
-      },
-      2 => match self.node.children.take() {
-        Some(children) => {
-          let deserializer = MapDeserializer::new(children.into_iter());
-          seed.deserialize(deserializer).map(Some)
-        },
-        None => seed.deserialize(().into_deserializer()).map(Some),
-      },
+      1 => map_deserializer!(self.node.attributes),
+      2 => map_deserializer!(self.node.children),
       3 => match self.node.value.take() {
         Some(value) => seed.deserialize(value.into_deserializer()).map(Some),
         None => seed.deserialize(().into_deserializer()).map(Some),