From 20a552f527d6ab007123cf8594c8d8625798ca88 Mon Sep 17 00:00:00 2001
From: Matthias Beyer <mail@beyermatthias.de>
Date: Fri, 22 Dec 2017 11:49:32 +0100
Subject: [PATCH] Refactor header checking to use toml-query

---
 lib/core/libimagstore/src/store.rs | 67 +++++++++++++-----------------
 1 file changed, 30 insertions(+), 37 deletions(-)

diff --git a/lib/core/libimagstore/src/store.rs b/lib/core/libimagstore/src/store.rs
index 86bd08f4..b2e9beba 100644
--- a/lib/core/libimagstore/src/store.rs
+++ b/lib/core/libimagstore/src/store.rs
@@ -32,10 +32,10 @@ use std::fmt::Debug;
 use std::fmt::Error as FMTError;
 
 use toml::Value;
-use toml::value::Table;
 use glob::glob;
 use walkdir::WalkDir;
 use walkdir::Iter as WalkDirIter;
+use toml_query::read::TomlValueReadExt;
 
 use error::{StoreError as SE, StoreErrorKind as SEK};
 use error::ResultExt;
@@ -1067,10 +1067,7 @@ pub trait Header {
 impl Header for Value {
 
     fn verify(&self) -> Result<()> {
-        match *self {
-            Value::Table(ref t) => verify_header(&t),
-            _ => Err(SE::from_kind(SEK::HeaderTypeFailure)),
-        }
+        verify_header(self)
     }
 
     fn parse(s: &str) -> Result<Value> {
@@ -1079,7 +1076,6 @@ impl Header for Value {
         from_str(s)
             .map_err(From::from)
             .and_then(verify_header_consistency)
-            .map(Value::Table)
     }
 
     fn default_header() -> Value {
@@ -1098,16 +1094,16 @@ impl Header for Value {
 
 }
 
-fn verify_header_consistency(t: Table) -> Result<Table> {
+fn verify_header_consistency(t: Value) -> Result<Value> {
     verify_header(&t).chain_err(|| SEK::HeaderInconsistency).map(|_| t)
 }
 
-fn verify_header(t: &Table) -> Result<()> {
-    if !has_main_section(t) {
+fn verify_header(t: &Value) -> Result<()> {
+    if !has_main_section(t)? {
         Err(SE::from_kind(SEK::MissingMainSection))
-    } else if !has_imag_version_in_main_section(t) {
+    } else if !has_imag_version_in_main_section(t)? {
         Err(SE::from_kind(SEK::MissingVersionInfo))
-    } else if !has_only_tables(t) {
+    } else if !has_only_tables(t)? {
         debug!("Could not verify that it only has tables in its base table");
         Err(SE::from_kind(SEK::NonTableInBaseTable))
     } else {
@@ -1115,33 +1111,30 @@ fn verify_header(t: &Table) -> Result<()> {
     }
 }
 
-fn has_only_tables(t: &Table) -> bool {
+fn has_only_tables(t: &Value) -> Result<bool> {
     debug!("Verifying that table has only tables");
-    t.iter().all(|(_, x)| is_match!(*x, Value::Table(_)))
-}
-
-fn has_main_section(t: &Table) -> bool {
-    t.contains_key("imag") && is_match!(t.get("imag"), Some(&Value::Table(_)))
-}
-
-fn has_imag_version_in_main_section(t: &Table) -> bool {
-    use semver::Version;
-
-    match *t.get("imag").unwrap() {
-        Value::Table(ref sec) => {
-            sec.get("version")
-                .and_then(|v| {
-                    match *v {
-                        Value::String(ref s) => Some(Version::parse(&s[..]).is_ok()),
-                        _                    => Some(false),
-                    }
-                })
-            .unwrap_or(false)
-        }
-        _ => false,
+    match *t {
+        Value::Table(ref tab) => Ok(tab.iter().all(|(_, x)| is_match!(*x, Value::Table(_)))),
+        _ => Err(SE::from_kind(SEK::HeaderTypeFailure)),
     }
 }
 
+fn has_main_section(t: &Value) -> Result<bool> {
+    t.read("imag")?
+        .ok_or(SE::from_kind(SEK::ConfigKeyMissingError("imag")))
+        .map(Value::is_table)
+}
+
+fn has_imag_version_in_main_section(t: &Value) -> Result<bool> {
+    use toml_query::read::TomlValueReadExt;
+
+    t.read("imag.version")?
+        .ok_or(SE::from_kind(SEK::ConfigKeyMissingError("imag.version")))?
+        .as_str()
+        .map(|s| ::semver::Version::parse(s).is_ok())
+        .ok_or(SE::from_kind(SEK::ConfigTypeError("imag.version", "String")))
+}
+
 
 #[cfg(test)]
 mod test {
@@ -1184,7 +1177,7 @@ mod test {
         let mut map = BTreeMap::new();
         map.insert("imag".into(), Value::Table(BTreeMap::new()));
 
-        assert!(!has_imag_version_in_main_section(&map));
+        assert!(has_imag_version_in_main_section(&map).is_err());
     }
 
     #[test]
@@ -1194,7 +1187,7 @@ mod test {
         sub.insert("version".into(), Value::String("0.0.0".into()));
         map.insert("imag".into(), Value::Table(sub));
 
-        assert!(has_imag_version_in_main_section(&map));
+        assert!(has_imag_version_in_main_section(&map)?);
     }
 
     #[test]
@@ -1204,7 +1197,7 @@ mod test {
         sub.insert("version".into(), Value::Boolean(false));
         map.insert("imag".into(), Value::Table(sub));
 
-        assert!(!has_imag_version_in_main_section(&map));
+        assert!(has_imag_version_in_main_section(&map).is_err());
     }
 
     #[test]