NetworkInterfaceBody: reworked de/serialization

The de/serialization of this structure previously relied on two
local helper functions to handle the guest_mac: Option<MacAddr>
field. However, this led to buggy behaviour, as the guest_mac = None
case was not handled properly. We eliminate the local functions, and
rely on the implementation of Serialize and Deserialize for MacAddr
instead. Also added one more unit test function.

Signed-off-by: Alexandru Agache <[email protected]>

Author: alexandruag

api_server/src/request/sync/net.rs

@@ -2,40 +2,9 @@ use std::result;
 
 use futures::sync::oneshot;
 
-use request::ParsedRequest;
-use serde::de::{self, Deserialize, Deserializer};
-use serde::ser::{Serialize, Serializer};
-
 use super::{DeviceState, SyncRequest};
 use net_util::MacAddr;
-
-// used to serialize an Option<MacAddr>
-fn mac_serialize_with<S>(what: &Option<MacAddr>, serializer: S) -> result::Result<S::Ok, S::Error>
-where
-    S: Serializer,
-{
-    if let Some(ref mac) = *what {
-        mac.to_string().serialize(serializer)
-    } else {
-        "".serialize(serializer)
-    }
-}
-
-// used to deserialize an Option<MacAddr>
-fn mac_deserialize_with<'de, D>(deserializer: D) -> result::Result<Option<MacAddr>, D::Error>
-where
-    D: Deserializer<'de>,
-{
-    // TODO: is there a more efficient way around this? (i.e. deserialize to a slice
-    // instead of a String?!)
-    if let Some(ref s) = Option::<String>::deserialize(deserializer)? {
-        MacAddr::parse_str(s)
-            .map(|mac_addr| Some(mac_addr))
-            .map_err(|_| de::Error::custom("invalid MAC address."))
-    } else {
-        Ok(None)
-    }
-}
+use request::ParsedRequest;
 
 // This struct represents the strongly typed equivalent of the json body from net iface
 // related requests.
@@ -44,8 +13,7 @@ pub struct NetworkInterfaceBody {
     pub iface_id: String,
     pub state: DeviceState,
     pub host_dev_name: String,
-    #[serde(serialize_with = "self::mac_serialize_with",
-            deserialize_with = "self::mac_deserialize_with")]
+    #[serde(skip_serializing_if = "Option::is_none")]
     pub guest_mac: Option<MacAddr>,
 }
 
@@ -68,6 +36,7 @@ impl NetworkInterfaceBody {
 #[cfg(test)]
 mod tests {
     use super::*;
+    use serde_json;
 
     #[test]
     fn test_netif_into_parsed_request() {
@@ -90,4 +59,38 @@ mod tests {
                 )))
         );
     }
+
+    #[test]
+    fn test_network_interface_body_serialization_and_deserialization() {
+        let netif = NetworkInterfaceBody {
+            iface_id: String::from("foo"),
+            state: DeviceState::Attached,
+            host_dev_name: String::from("bar"),
+            guest_mac: Some(MacAddr::parse_str("12:34:56:78:9A:BC").unwrap()),
+        };
+
+        // This is the json encoding of the netif variable.
+        let jstr = r#"{
+            "iface_id": "foo",
+            "host_dev_name": "bar",
+            "state": "Attached",
+            "guest_mac": "12:34:56:78:9A:bc"
+        }"#;
+
+        let x = serde_json::from_str(jstr).expect("deserialization failed.");
+        assert_eq!(netif, x);
+
+        let y = serde_json::to_string(&netif).expect("serialization failed.");
+        let z = serde_json::from_str(y.as_ref()).expect("deserialization (2) failed.");
+        assert_eq!(x, z);
+
+        // Check that guest_mac is truly optional.
+        let jstr_no_mac = r#"{
+            "iface_id": "foo",
+            "host_dev_name": "bar",
+            "state": "Attached"
+        }"#;
+
+        assert!(serde_json::from_str::<NetworkInterfaceBody>(jstr_no_mac).is_ok())
+    }
 }