Merge pull request #26 from yatharthmathur/ym_string_conversion_error…

…_handling

refactor(errors): error handling with string conversion

src/data_store/store.rs

@@ -1,5 +1,6 @@
 use super::value_entry::ValueEntry;
 use std::collections::HashMap;
+use std::string::FromUtf8Error;
 use std::time::{Duration, Instant};
 
 /// The main struct of the Key-Value store
@@ -26,14 +27,21 @@ impl KeyValueStore {
         self._data.insert(key.to_owned(), value_entry.to_owned());
     }
 
-    fn _remove(&mut self, key: &String) -> Option<ValueEntry> {
-        self._data.remove(key)
+    fn _remove_and_none_if_expired(&mut self, key: &String) -> Option<ValueEntry> {
+        if let Some(value_entry) = self._data.remove(key) {
+            if value_entry.is_expired_entry(None) {
+                None
+            } else {
+                Some(value_entry)
+            }
+        } else {
+            None
+        }
     }
 
     fn _get_or_none_if_expired(&mut self, key: &String) -> Option<&ValueEntry> {
-        let now = Instant::now();
         if let Some(value_entry) = self._data.get(key) {
-            if now >= value_entry.expiration {
+            if value_entry.is_expired_entry(None) {
                 return None;
             }
         };
@@ -45,7 +53,7 @@ impl KeyValueStore {
         let expired_keys: Vec<String> = self
             ._data
             .iter()
-            .filter(|(_, value_entry)| now >= value_entry.expiration)
+            .filter(|(_, value_entry)| value_entry.is_expired_entry(Some(now)))
             .map(|(key, _)| key.to_owned())
             .collect();
 
@@ -54,10 +62,18 @@ impl KeyValueStore {
         }
     }
 
-    pub fn contains(&mut self, key: String) -> bool {
-        match self._get_or_none_if_expired(&key) {
-            Some(_) => true,
-            _ => false,
+    /// Check whether the key exists in the store.
+    /// NOTE: this returns true, even if the key is expired.
+    pub fn contains_key(&mut self, key: String) -> bool {
+        self._data.contains_key(&key)
+    }
+
+    /// Check whether a key is expired or not.
+    pub fn is_expired(&mut self, key: String) -> Option<bool> {
+        if let Some(value_entry) = self._data.get(&key) {
+            Some(value_entry.is_expired_entry(None))
+        } else {
+            None
         }
     }
 
@@ -85,33 +101,40 @@ impl KeyValueStore {
     }
 
     /// Gets a Value (converted to String type) associated to the Key in the KeyValueStore
-    pub fn get_as_string(&mut self, key: String) -> Option<String> {
+    pub fn get_as_string(&mut self, key: String) -> Option<Result<String, FromUtf8Error>> {
         if let Some(value_entry) = self._get_or_none_if_expired(&key) {
-            ValueEntry::extract_string_value_from_value_entry(value_entry)
+            match value_entry.extract_string_value_from_value_entry() {
+                Ok(string_value) => Some(Ok(string_value)),
+                Err(e) => Some(Err(e)),
+            }
         } else {
             None
         }
     }
 
     /// Removes the Key-Value pair for the given Key in the KeyValueStore
     pub fn remove(&mut self, key: String) {
-        self._remove(&key);
+        self._remove_and_none_if_expired(&key);
     }
 
     /// Removes the Key-Value pair for the given Key in the KeyValueStore
     /// and returns the Value (in Vec<u8> type)
     pub fn pop(&mut self, key: String) -> Option<Vec<u8>> {
-        match self._remove(&key) {
-            Some(value_entry) => Some(value_entry.value),
-            _ => None,
+        if let Some(value_entry) = self._remove_and_none_if_expired(&key) {
+            Some(value_entry.value)
+        } else {
+            None
         }
     }
 
     /// Removes the Key-Value pair for the given Key in the KeyValueStore
     /// and returns the Value (converted to String type)
-    pub fn pop_as_string(&mut self, key: String) -> Option<String> {
-        if let Some(value_entry) = self._remove(&key) {
-            ValueEntry::extract_string_value_from_value_entry(&value_entry)
+    pub fn pop_as_string(&mut self, key: String) -> Option<Result<String, FromUtf8Error>> {
+        if let Some(value_entry) = self._remove_and_none_if_expired(&key) {
+            match value_entry.extract_string_value_from_value_entry() {
+                Ok(string_value) => Some(Ok(string_value)),
+                Err(e) => Some(Err(e)),
+            }
         } else {
             None
         }
@@ -122,30 +145,3 @@ impl KeyValueStore {
         self._data.clear();
     }
 }
-
-mod private_tests {
-    use super::KeyValueStore;
-    use std::time::Duration;
-
-    #[test]
-    fn test_clear_all_expired_keys() {
-        let mut store = KeyValueStore::new(5000);
-        store.set_with_string_value("ABC".to_string(), "HELLO".to_string(), Some(250));
-        store.set_with_string_value("XYZ".to_string(), "HELLO".to_string(), Some(250));
-        store.set_with_string_value("DEF".to_string(), "HELLO".to_string(), Some(250));
-
-        store.clear_all_expired_keys();
-
-        assert!(store._data.contains_key("ABC"));
-
-        std::thread::sleep(Duration::from_millis(250));
-        assert!(store._data.contains_key("ABC"));
-        assert!(store._data.contains_key("DEF"));
-        assert!(store._data.contains_key("XYZ"));
-
-        store.clear_all_expired_keys();
-        assert_eq!(store._data.contains_key("ABC"), false);
-        assert_eq!(store._data.contains_key("DEF"), false);
-        assert_eq!(store._data.contains_key("XYZ"), false);
-    }
-}

src/data_store/value_entry.rs

@@ -1,4 +1,4 @@
-use std::time::Instant;
+use std::{string::FromUtf8Error, time::Instant};
 
 /// Each entry of the Key-Value pair in the Data store is this struct.
 #[derive(Clone)]
@@ -22,12 +22,16 @@ impl ValueEntry {
     }
 
     /// Extract value and convert it to String from KeyValueEntry
-    pub fn extract_string_value_from_value_entry(value_entry: &ValueEntry) -> Option<String> {
-        match String::from_utf8(value_entry.value.to_owned()) {
-            Ok(string) => Some(string),
-            // This case will not happen as all values that
-            // were initially stored in the DB were valid.
-            _ => None,
+    pub fn extract_string_value_from_value_entry(&self) -> Result<String, FromUtf8Error> {
+        match String::from_utf8(self.value.to_owned()) {
+            Ok(string) => Ok(string),
+            Err(e) => Err(e),
         }
     }
+
+    /// Check if this entry is expired.
+    pub fn is_expired_entry(&self, option_now: Option<Instant>) -> bool {
+        let now = option_now.unwrap_or(Instant::now());
+        now >= self.expiration
+    }
 }

src/public_tests.rs

@@ -6,16 +6,42 @@ use std::time::Duration;
 fn test_contains() {
     let mut store = KeyValueStore::new(0);
     store.set_with_string_value("ABC".to_string(), "HELLO".to_string(), Some(5000));
-    assert!(store.contains("ABC".to_string()));
-    assert_ne!(store.contains("ABC".to_string()), false);
+    assert!(store.contains_key("ABC".to_string()));
+    assert_ne!(store.contains_key("ABC".to_string()), false);
+}
+
+#[test]
+fn test_clear_all_expired_keys() {
+    let mut store = KeyValueStore::new(5000);
+    store.set_with_string_value("ABC".to_string(), "HELLO".to_string(), Some(250));
+    store.set_with_string_value("XYZ".to_string(), "HELLO".to_string(), Some(250));
+    store.set_with_string_value("DEF".to_string(), "HELLO".to_string(), Some(250));
+
+    store.clear_all_expired_keys();
+
+    assert!(store.contains_key("ABC".to_string()));
+
+    std::thread::sleep(Duration::from_millis(250));
+    assert!(store.contains_key("ABC".to_string()));
+    assert!(store.contains_key("DEF".to_string()));
+    assert!(store.contains_key("XYZ".to_string()));
+
+    assert!(store.is_expired("ABC".to_string()).unwrap());
+
+    store.clear_all_expired_keys();
+    assert_eq!(store.contains_key("ABC".to_string()), false);
+    assert_eq!(store.contains_key("DEF".to_string()), false);
+    assert_eq!(store.contains_key("XYZ".to_string()), false);
+
+    assert_eq!(store.is_expired("ABC".to_string()), None);
 }
 
 #[test]
 fn test_set_get() {
     let mut store = KeyValueStore::new(0);
     store.set_with_string_value("ABC".to_string(), "HELLO".to_string(), Some(5000));
     if let Some(val) = store.get_as_string("ABC".to_string()) {
-        assert_eq!(val, "HELLO".to_string());
+        assert_eq!(val, Ok("HELLO".to_string()));
     }
 }
 
@@ -24,7 +50,7 @@ fn test_set_get_vec_u8() {
     let mut store = KeyValueStore::new(0);
     store.set("ABC".to_string(), "HELLO".as_bytes().to_vec(), Some(5000));
     if let Some(val) = store.get_as_string("ABC".to_string()) {
-        assert_eq!(val, "HELLO".to_string());
+        assert_eq!(val, Ok("HELLO".to_string()));
     }
 
     if let Some(vec_val) = store.get("ABC".to_string()) {
@@ -39,7 +65,7 @@ fn test_pop_vec_u8() {
     let mut store = KeyValueStore::new(0);
     store.set("ABC".to_string(), "HELLO".as_bytes().to_vec(), Some(5000));
     if let Some(val) = store.get_as_string("ABC".to_string()) {
-        assert_eq!(val, "HELLO".to_string());
+        assert_eq!(val, Ok("HELLO".to_string()));
     }
 
     if let Some(vec_val) = store.pop("ABC".to_string()) {
@@ -76,16 +102,38 @@ fn test_get_set_binary_data() {
     }
 }
 
+#[test]
+fn test_get_set_binary_data_conversion_to_string_fails() {
+    #[derive(Serialize, Deserialize)]
+    struct LocalStruct {
+        test1: f64,
+        test2: String,
+    }
+
+    let local_struct_instance: LocalStruct = LocalStruct {
+        test1: 3.1415,
+        test2: "Hey there".to_string(),
+    };
+    let mut store = KeyValueStore::new(0);
+    let bin_code = bincode::serialize(&local_struct_instance).unwrap();
+    store.set("ABC".to_string(), bin_code, Some(5000));
+
+    assert_eq!(
+        store.get_as_string("ABC".to_string()).unwrap().is_ok(),
+        false
+    );
+}
+
 #[test]
 fn test_pop_key() {
     let mut store = KeyValueStore::new(0);
     store.set_with_string_value("ABC".to_string(), "HELLO".to_string(), Some(5000));
     if let Some(val) = store.get_as_string("ABC".to_string()) {
-        assert_eq!(val, "HELLO".to_string());
+        assert_eq!(val, Ok("HELLO".to_string()));
     };
 
     if let Some(val) = store.pop_as_string("ABC".to_string()) {
-        assert_eq!(val, "HELLO".to_string());
+        assert_eq!(val, Ok("HELLO".to_string()));
     };
 
     match store.get_as_string("ABC".to_string()) {
@@ -100,7 +148,7 @@ fn test_remove_key() {
     store.set_with_string_value("ABC".to_string(), "HELLO".to_string(), Some(5000));
     store.remove("XYZ".to_string());
     if let Some(val) = store.get_as_string("ABC".to_string()) {
-        assert_eq!(val, "HELLO".to_string());
+        assert_eq!(val, Ok("HELLO".to_string()));
     };
 
     store.remove("ABC".to_string());