Use custom serialize function for exports to handle nested tables in

exported config

Abstract out existing special casing for serialization of config and
re-use it when creating the exported config
Signed-off-by: James Casey <james@chef.io>

components/sup/src/manager/service/config.rs

@@ -15,6 +15,7 @@
 /// Collect all the configuration data that is exposed to users, and render it.
 use std;
 use std::borrow::Cow;
+use std::collections::HashMap;
 use std::env;
 use std::fs::File;
 use std::io::prelude::*;
@@ -29,6 +30,7 @@ use serde::{Serialize, Serializer};
 use serde_json;
 use serde_transcode;
 use toml;
+use toml::ser;
 
 use super::Pkg;
 use census::CensusGroup;
@@ -226,11 +228,11 @@ impl Cfg {
         }
     }
 
-    /// Returns a subset of the overall configuration which intersects with the given package's exports.
-    pub fn to_exported(&self, pkg: &Pkg) -> Result<Vec<u8>> {
+    /// Returns a subset of the overall configuration which intersects with the given package exports.
+    pub fn to_exported(&self, exports: &HashMap<String, String>) -> Result<Vec<u8>> {
         let mut map = toml::value::Table::default();
         let cfg = toml::Value::try_from(&self).unwrap();
-        for (key, path) in pkg.exports.iter() {
+        for (key, path) in exports.iter() {
             let fields: Vec<&str> = path.split('.').collect();
             let mut curr = &cfg;
             let mut found = false;
@@ -253,7 +255,12 @@ impl Cfg {
                 map.insert(key.clone(), curr.clone());
             }
         }
-        toml::ser::to_vec(&map).map_err(|e| sup_error!(Error::TomlEncode(e)))
+
+        let mut dst = String::with_capacity(128);
+        match serialize_table(&map, &mut ser::Serializer::new(&mut dst)) {
+            Ok(_) => Ok(dst.into_bytes()),
+            Err(e) => Err(sup_error!(Error::TomlEncode(e))),
+        }
     }
 
     fn load_toml_file<T1, T2>(dir: T1, file: T2) -> Result<Option<toml::value::Table>>
@@ -435,29 +442,36 @@ impl Serialize for Cfg {
             }
         }
 
-        // Be sure to visit non-tables first (and also non
-        // array-of-tables) as all keys must be emitted first.
-        let mut map = serializer.serialize_map(Some(table.len()))?;
-        for (k, v) in &table {
-            if !v.is_array() && !v.is_table() {
-                map.serialize_key(&k)?;
-                map.serialize_value(&v)?;
-            }
+        serialize_table(&table, serializer)
+    }
+}
+
+fn serialize_table<S>(table: &toml::value::Table, serializer: S) -> result::Result<S::Ok, S::Error>
+where
+    S: Serializer,
+{
+    // Be sure to visit non-tables first (and also non
+    // array-of-tables) as all keys must be emitted first.
+    let mut map = serializer.serialize_map(Some(table.len()))?;
+    for (k, v) in table {
+        if !v.is_array() && !v.is_table() {
+            map.serialize_key(&k)?;
+            map.serialize_value(&v)?;
         }
-        for (k, v) in &table {
-            if v.is_array() {
-                map.serialize_key(&k)?;
-                map.serialize_value(&v)?;
-            }
+    }
+    for (k, v) in table {
+        if v.is_array() {
+            map.serialize_key(&k)?;
+            map.serialize_value(&v)?;
         }
-        for (k, v) in &table {
-            if v.is_table() {
-                map.serialize_key(&k)?;
-                map.serialize_value(&v)?;
-            }
+    }
+    for (k, v) in table {
+        if v.is_table() {
+            map.serialize_key(&k)?;
+            map.serialize_value(&v)?;
         }
-        map.end()
     }
+    map.end()
 }
 
 #[derive(Debug)]
@@ -642,10 +656,14 @@ mod test {
     use std::env;
     use std::fs;
     use std::fs::OpenOptions;
+    use std::str;
 
     use tempfile::TempDir;
     use toml;
 
+    use hcore::package::{PackageIdent, PackageInstall};
+    use hcore::service::ServiceGroup;
+
     use super::*;
     use error::Error;
 
@@ -1076,4 +1094,72 @@ mod test {
         }
     }
 
+    fn pkg_with_exports(exports: HashMap<String, String>) -> Pkg {
+        let service_group = ServiceGroup::new(None, "dummy", "service", None)
+            .expect("couldn't create ServiceGroup");
+        let pg_id = PackageIdent::new(
+            "testing",
+            &service_group.service(),
+            Some("1.0.0"),
+            Some("20170712000000"),
+        );
+
+        let pkg_install = PackageInstall::new_from_parts(
+            pg_id.clone(),
+            PathBuf::from("/tmp"),
+            PathBuf::from("/tmp"),
+            PathBuf::from("/tmp"),
+        );
+
+        let mut pkg = Pkg::from_install(pkg_install).expect("Could not create package!");
+        pkg.exports = exports;
+        pkg
+    }
+
+    #[test]
+    fn to_exported_test() {
+        let export_map = [("a".to_string(), "foo".to_string())]
+            .iter()
+            .cloned()
+            .collect();
+        let pkg = pkg_with_exports(export_map);
+
+        let cfg_data = CfgTestData::new();
+        let toml = "foo = 13";
+        write_toml(&cfg_data.ducp, toml);
+        let cfg = Cfg::new(&cfg_data.pkg, None).expect("create config");
+
+        let result = cfg.to_exported(&pkg.exports).expect("export");
+        assert_eq!("a = 13\n", str::from_utf8(&result).unwrap());
+    }
+
+    /// TOML encoding by default doesn't handle tables being in the middle of the output.
+    /// Let's check if we correctly put tables at the end
+    #[test]
+    fn nested_table_to_exported_test() {
+        let export_map = [
+            ("a".to_string(), "server".to_string()),
+            ("b".to_string(), "frubnub".to_string()),
+        ]
+        .iter()
+        .cloned()
+        .collect();
+        let pkg = pkg_with_exports(export_map);
+
+        let cfg_data = CfgTestData::new();
+        let toml = r#"
+            frubnub = "foobar"
+
+            [server]
+            port = 5000
+            "#;
+        write_toml(&cfg_data.ducp, toml);
+        let cfg = Cfg::new(&cfg_data.pkg, None).expect("create config");
+
+        let result = cfg.to_exported(&pkg.exports).expect("export");
+        assert_eq!(
+            "b = \"foobar\"\n\n[a]\nport = 5000\n",
+            str::from_utf8(&result).unwrap()
+        );
+    }
 }

components/sup/src/manager/service/mod.rs

@@ -667,7 +667,7 @@ impl Service {
     }
 
     pub fn to_rumor(&self, incarnation: u64) -> ServiceRumor {
-        let exported = match self.cfg.to_exported(&self.pkg) {
+        let exported = match self.cfg.to_exported(&self.pkg.exports) {
             Ok(exported) => Some(exported),
             Err(err) => {
                 outputln!(preamble self.service_group,