Skip to content

Support seq and maps (rebase of #142, refactor of #129) #154

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
134 changes: 118 additions & 16 deletions src/ser/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::io::Write;

use serde::ser::{self, Impossible, Serialize};

use self::var::{Map, Struct};
use self::var::{Map, Seq, Struct};
use error::{Error, Result};

mod var;
Expand Down Expand Up @@ -85,7 +85,7 @@ where
W: Write,
{
pub fn new(writer: W) -> Self {
Self { writer: writer }
Self { writer }
}

fn write_primitive<P: Display>(&mut self, primitive: P) -> Result<()> {
Expand All @@ -109,7 +109,7 @@ where
type Ok = ();
type Error = Error;

type SerializeSeq = Impossible<Self::Ok, Self::Error>;
type SerializeSeq = Seq<'w, W>;
type SerializeTuple = Impossible<Self::Ok, Self::Error>;
type SerializeTupleStruct = Impossible<Self::Ok, Self::Error>;
type SerializeTupleVariant = Impossible<Self::Ok, Self::Error>;
Expand Down Expand Up @@ -205,9 +205,7 @@ where
variant_index: u32,
variant: &'static str,
) -> Result<Self::Ok> {
Err(Error::UnsupportedOperation {
operation: "serialize_unit_variant".to_string(),
})
self.serialize_none()
}

fn serialize_newtype_struct<T: ?Sized + Serialize>(
Expand All @@ -231,10 +229,7 @@ where
}

fn serialize_seq(self, len: Option<usize>) -> Result<Self::SerializeSeq> {
// TODO: Figure out how to constrain the things written to only be composites
Err(Error::UnsupportedOperation {
operation: "serialize_seq".to_string(),
})
Ok(Self::SerializeSeq::new(self))
}

fn serialize_tuple(self, len: usize) -> Result<Self::SerializeTuple> {
Expand Down Expand Up @@ -291,6 +286,7 @@ where
mod tests {
use super::*;
use serde::ser::{SerializeMap, SerializeStruct};
use serde::Serialize;
use serde::Serializer as SerSerializer;

#[test]
Expand Down Expand Up @@ -361,6 +357,61 @@ mod tests {
assert_eq!(got, should_be);
}

#[test]
fn test_serialize_simple_map() {
let mut hashmap = std::collections::HashMap::new();
hashmap.insert("key1", "val1");
let mut buffer = Vec::new();
{
let mut ser = Serializer::new(&mut buffer);
hashmap
.serialize(&mut ser)
.expect("unable to serialize a hashmap instance");
}
let got = String::from_utf8(buffer).unwrap();
assert_eq!("<key1>val1</key1>", got)
}

#[test]
fn test_serialize_map_within_struct() {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test actually fails intermittently.

I'm wondering if it is from the implementation of serializing Map's or the test relying on a fixed order of printed keys.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly, this test is relying on the Map's items being ordered, which is totally wrong. Sorry for that.

I'd suggest removing the second insert or relaxing the assert using .contains() to assert that at least the content is ok, rather than the exact output content+order.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the hints, I pushed an update to fix the test.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly, this test is relying on the Map's items being ordered, which is totally wrong. Sorry for that.

ah, makes sense @adrianbenavides. Thank you for explaining! No need to apologize too. Part of learning!

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

stellar, thanks @joemat!

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh, cannot find the resolve button. I ran cargo test ser repeatedly locally and things look great!

#[derive(Serialize)]
struct Thing {
things: std::collections::HashMap<String, String>,
}

let mut hashmap = std::collections::HashMap::new();
hashmap.insert("key1".to_string(), "val1".to_string());
hashmap.insert("key2".to_string(), "val2".to_string());
let sut = Thing { things: hashmap };

let mut buffer = Vec::new();
{
let mut ser = Serializer::new(&mut buffer);
sut.serialize(&mut ser)
.expect("unable to serialize a struct with a hashmap");
}
let got = String::from_utf8(buffer).unwrap();

/*
* Since there is no defined order when serializing a map,
* this test has to verify the Thing tags at beginning and end
* of the result string and also the existence of the two
* key/value map entries.
*/
let expected_start = "<Thing><things>";
let key_val1 = "<key1>val1</key1>";
let key_val2 = "<key2>val2</key2>";
let expected_end = "</things></Thing>";
assert!(got.starts_with(expected_start));
assert!(got.contains(key_val1));
assert!(got.contains(key_val2));
assert!(got.ends_with(expected_end));

// ensure that the result does only contain the expected strings
assert_eq!(expected_start.len() + key_val1.len() + key_val2.len() + expected_end.len(),
got.len());
}

#[test]
fn test_serialize_map_entries() {
let should_be = "<name>Bob</name><age>5</age>";
Expand Down Expand Up @@ -401,19 +452,70 @@ mod tests {
}

#[test]
#[ignore]
fn serialize_a_list() {
let inputs = vec![1, 2, 3, 4];
fn test_serialize_vector() {
let vector = vec![1, 2, 3];
let mut buffer = Vec::new();
{
let mut ser = Serializer::new(&mut buffer);
vector.serialize(&mut ser).unwrap();
}
let got = String::from_utf8(buffer).unwrap();
assert_eq!("123", got);
}

#[test]
fn test_serialize_vector_of_primitives_within_struct() {
#[derive(Serialize)]
struct Team {
projects: Vec<String>,
}

let sut = Team {
projects: vec!["thing_1".to_string(), "thing two".to_string()],
};

let mut buffer = Vec::new();
{
let mut ser = Serializer::new(&mut buffer);
sut.serialize(&mut ser).unwrap();
}

let got = String::from_utf8(buffer).unwrap();
assert_eq!("<Team><projects>thing_1thing two</projects></Team>", got);
}

#[test]
fn test_serialize_vector_of_structs_within_struct() {
#[derive(Serialize)]
#[serde(rename = "team")]
struct Team {
people: Vec<Person>,
}

#[derive(Serialize)]
#[serde(rename = "person")]
struct Person {
name: String,
}

let sut = Team {
people: vec![
Person {
name: "Joe".to_string(),
},
Person {
name: "Jane".to_string(),
},
],
};

let mut buffer = Vec::new();
{
let mut ser = Serializer::new(&mut buffer);
inputs.serialize(&mut ser).unwrap();
sut.serialize(&mut ser).unwrap();
}

let got = String::from_utf8(buffer).unwrap();
println!("{}", got);
panic!();
assert_eq!("<team><people><person><name>Joe</name></person><person><name>Jane</name></person></people></team>", got);
}
}
54 changes: 48 additions & 6 deletions src/ser/var.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,16 @@ where
type Ok = ();
type Error = Error;

fn serialize_key<T: ?Sized + Serialize>(&mut self, _: &T) -> Result<()> {
panic!("impossible to serialize the key on its own, please use serialize_entry()")
fn serialize_key<T: ?Sized + Serialize>(&mut self, key: &T) -> Result<()> {
write!(self.parent.writer, "<field fieldName=\"")?;
key.serialize(&mut *self.parent)?;
write!(self.parent.writer, "\">")?;
Ok(())
}

fn serialize_value<T: ?Sized + Serialize>(&mut self, value: &T) -> Result<()> {
value.serialize(&mut *self.parent)
}

fn end(self) -> Result<Self::Ok> {
value.serialize(&mut *self.parent)?;
write!(self.parent.writer, "</field>")?;
Ok(())
}

Expand All @@ -59,6 +60,10 @@ where
write!(self.parent.writer, ">")?;
Ok(())
}

fn end(self) -> Result<Self::Ok> {
Ok(())
}
}

/// An implementation of `SerializeStruct` for serializing to XML.
Expand Down Expand Up @@ -101,3 +106,40 @@ where
write!(self.parent.writer, "</{}>", self.name).map_err(|e| e.into())
}
}

/// An implementation of `SerializeSequence` for serializing to XML.
pub struct Seq<'w, W>
where
W: 'w + Write,
{
parent: &'w mut Serializer<W>,
}

impl<'w, W> ser::SerializeSeq for Seq<'w, W>
where
W: 'w + Write,
{
type Ok = ();
type Error = Error;

fn serialize_element<T: ?Sized>(&mut self, value: &T) -> Result<()>
where
T: Serialize,
{
value.serialize(&mut *self.parent)?;
Ok(())
}

fn end(self) -> Result<Self::Ok> {
Ok(())
}
}

impl<'w, W> Seq<'w, W>
where
W: 'w + Write,
{
pub fn new(parent: &'w mut Serializer<W>) -> Seq<'w, W> {
Self { parent }
}
}
4 changes: 2 additions & 2 deletions tests/migrated.rs
Original file line number Diff line number Diff line change
Expand Up @@ -984,12 +984,12 @@ fn futile2() {
#[derive(Eq, PartialEq, Debug, Serialize, Deserialize)]
struct Object {
field: Option<Null>,
};
}

#[derive(Eq, PartialEq, Debug, Serialize, Deserialize)]
struct Stuff {
stuff_field: Option<Object>,
};
}

test_parse_ok(&[
(
Expand Down
14 changes: 6 additions & 8 deletions tests/round_trip.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@
extern crate serde_derive;
extern crate serde;
extern crate serde_xml_rs;
extern crate xml;

use serde::Deserialize;
use serde_xml_rs::{from_str, to_string, EventReader, ParserConfig};
use serde_xml_rs::{from_str, to_string};
use serde_xml_rs::{EventReader, ParserConfig};

#[derive(Debug, Serialize, Deserialize, PartialEq)]
struct Item {
Expand Down Expand Up @@ -90,18 +92,14 @@ fn whitespace_preserving_config() {
name: " space banana ".to_string(),
source: " fantasy costco ".to_string(),
};
let config = ParserConfig::new()
.trim_whitespace(false)
.whitespace_to_characters(false);
let mut deserializer =
serde_xml_rs::Deserializer::new(EventReader::new_with_config(src.as_bytes(), config));
let config = ParserConfig::new().trim_whitespace(false).whitespace_to_characters(false);
let mut deserializer = serde_xml_rs::Deserializer::new(EventReader::new_with_config(src.as_bytes(), config));

let item = Item::deserialize(&mut deserializer).unwrap();
assert_eq!(item, item_should_be);

// Space outside values is not preserved.
let serialized_should_be =
"<Item><name> space banana </name><source> fantasy costco </source></Item>";
let serialized_should_be = "<Item><name> space banana </name><source> fantasy costco </source></Item>";
let reserialized_item = to_string(&item).unwrap();
assert_eq!(reserialized_item, serialized_should_be);
}
2 changes: 1 addition & 1 deletion tests/test.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
extern crate serde;
#[macro_use]
extern crate serde_derive;
extern crate serde_xml_rs;

extern crate log;
extern crate simple_logger;
extern crate serde;

use serde::Deserialize;
use serde_xml_rs::{from_str, Deserializer};
Expand Down