Merge pull request #7 from tophatsteve/refactor

Refactor

src/bucket.rs

@@ -88,9 +88,16 @@ mod tests {
     }
 
     impl storage::Storage for MockStorage {
-        fn upload(&self, blob_name: &str, data: Vec<u8>) {}
+        fn upload(&self, blob_name: &str, data: Vec<u8>) -> Result<(), storage::StorageError> {
+            Ok(())
+        }
         fn download(&self, p: &PathBuf) {}
-        fn delete(&self, blob_name: &str) {}
+        fn delete(&self, blob_name: &str) -> Result<(), storage::StorageError> {
+            Ok(())
+        }
+        fn list_folder_blobs(&self, blob_name: &str) -> Result<Vec<String>, storage::StorageError> {
+            Ok(Vec::new())
+        }
     }
 
     struct MockFileSystem {
@@ -116,6 +123,9 @@ mod tests {
             *self.get_file_contents_called.borrow_mut() = true;
             Vec::new()
         }
+        fn encode_file_name(&self, f: &str) -> String {
+            String::from("")
+        }
     }
 
     struct MockPathEventHandler {

src/event_handlers.rs

@@ -56,7 +56,9 @@ impl PathEventHandler for CreatedEvent {
         }
         let blob_name = file_system.get_blob_name(path);
         let file_content = file_system.get_file_contents(path);
-        storage.upload(&blob_name, file_content);
+        if let Err(e) = storage.upload(&blob_name, file_content) {
+            trace!("Error uploading - {}", e);
+        }
     }
 }
 
@@ -70,7 +72,19 @@ impl PathEventHandler for RemovedEvent {
         file_system: &file_system::FileSystem,
     ) {
         let blob_name = file_system.get_blob_name(path);
-        storage.delete(&blob_name);
+
+        match storage.delete(&blob_name) {
+            Err(storage::StorageError::PathNotFound) => {
+                let blobs_to_delete = storage.list_folder_blobs(&blob_name).unwrap();
+                for blob in blobs_to_delete {
+                    if let Err(e) = storage.delete(&file_system.encode_file_name(&blob_name)) {
+                        trace!("Error deleting folder content - {}", e);
+                    }
+                }
+            }
+            Err(e) => trace!("Error deleting - {}", e),
+            Ok(_) => (),
+        };
     }
 }
 
@@ -96,6 +110,8 @@ mod tests {
         upload_called: RefCell<bool>,
         download_called: RefCell<bool>,
         delete_called: RefCell<bool>,
+        list_folder_blobs_called: RefCell<bool>,
+        return_path_not_found_error: RefCell<bool>,
     }
 
     impl MockStorage {
@@ -104,19 +120,36 @@ mod tests {
                 upload_called: RefCell::new(false),
                 download_called: RefCell::new(false),
                 delete_called: RefCell::new(false),
+                list_folder_blobs_called: RefCell::new(false),
+                return_path_not_found_error: RefCell::new(false),
             }
         }
+
+        fn set_return_path_not_found_error(&self, f: bool) {
+            *self.return_path_not_found_error.borrow_mut() = f;
+        }
     }
 
     impl storage::Storage for MockStorage {
-        fn upload(&self, blob_name: &str, data: Vec<u8>) {
+        fn upload(&self, blob_name: &str, data: Vec<u8>) -> Result<(), storage::StorageError> {
             *self.upload_called.borrow_mut() = true;
+            Ok(())
         }
         fn download(&self, p: &PathBuf) {
             *self.download_called.borrow_mut() = true;
         }
-        fn delete(&self, blob_name: &str) {
+        fn delete(&self, blob_name: &str) -> Result<(), storage::StorageError> {
             *self.delete_called.borrow_mut() = true;
+
+            if *self.return_path_not_found_error.borrow() {
+                return Err(storage::StorageError::PathNotFound);
+            }
+
+            Ok(())
+        }
+        fn list_folder_blobs(&self, blob_name: &str) -> Result<Vec<String>, storage::StorageError> {
+            *self.list_folder_blobs_called.borrow_mut() = true;
+            Ok(Vec::new())
         }
     }
 
@@ -146,13 +179,15 @@ mod tests {
     struct MockFileSystem {
         get_blob_name_called: RefCell<bool>,
         get_file_contents_called: RefCell<bool>,
+        encode_file_name_called: RefCell<bool>,
     }
 
     impl MockFileSystem {
         fn new() -> MockFileSystem {
             MockFileSystem {
                 get_blob_name_called: RefCell::new(false),
                 get_file_contents_called: RefCell::new(false),
+                encode_file_name_called: RefCell::new(false),
             }
         }
     }
@@ -166,6 +201,10 @@ mod tests {
             *self.get_file_contents_called.borrow_mut() = true;
             Vec::new()
         }
+        fn encode_file_name(&self, f: &str) -> String {
+            *self.encode_file_name_called.borrow_mut() = true;
+            String::from("")
+        }
     }
 
     #[test]
@@ -228,4 +267,17 @@ mod tests {
 
         assert!(*mock_storage.delete_called.borrow());
     }
+
+    #[test]
+    fn test_remove_non_existing_file_calls_list_folder_blobs() {
+        let mock_file_system = MockFileSystem::new();
+        let mock_storage = MockStorage::new();
+        mock_storage.set_return_path_not_found_error(true);
+        let mut e = EventHandler::new(&mock_storage, &mock_file_system);
+
+        e.add("remove", &RemovedEvent {});
+        e.call("remove", &PathBuf::new());
+
+        assert!(*mock_storage.list_folder_blobs_called.borrow());
+    }
 }

src/file_system.rs

@@ -7,6 +7,7 @@ use url::percent_encoding::{utf8_percent_encode, DEFAULT_ENCODE_SET};
 pub trait FileSystem {
     fn get_blob_name(&self, p: &PathBuf) -> String;
     fn get_file_contents(&self, p: &PathBuf) -> Vec<u8>;
+    fn encode_file_name(&self, f: &str) -> String;
 }
 
 pub struct LocalFileSystem {
@@ -17,8 +18,12 @@ impl FileSystem for LocalFileSystem {
     fn get_blob_name(&self, p: &PathBuf) -> String {
         let root = Path::new(&self.root_folder);
         let stripped = p.strip_prefix(root).unwrap();
+        self.encode_file_name(stripped.to_str().unwrap())
+    }
+
+    fn encode_file_name(&self, f: &str) -> String {
         // convert Windows paths to standard format
-        let normalized = stripped.to_str().unwrap().replace("\\", "/");
+        let normalized = f.replace("\\", "/");
         utf8_percent_encode(&normalized, DEFAULT_ENCODE_SET).collect()
     }
 

src/main.rs

@@ -3,7 +3,6 @@
 
 extern crate azure_sdk_for_rust;
 extern crate env_logger;
-extern crate failure;
 extern crate futures;
 extern crate hyper;
 extern crate hyper_tls;
@@ -14,6 +13,8 @@ extern crate log;
 extern crate md5;
 extern crate tokio_core;
 extern crate url;
+#[macro_use]
+extern crate failure;
 
 mod bucket;
 mod event_handlers;

src/storage.rs

@@ -4,14 +4,37 @@ use azure_sdk_for_rust::core::DeleteSnapshotsMethod;
 use azure_sdk_for_rust::prelude::*;
 use futures::future::*;
 use hyper::StatusCode;
+use std::io;
 use std::path::PathBuf;
 use tokio_core::reactor::Core;
-use url::percent_encoding::{utf8_percent_encode, DEFAULT_ENCODE_SET};
+
+#[derive(Debug, Fail)]
+pub enum StorageError {
+    #[fail(display = "The specified path was not found")]
+    PathNotFound,
+    #[fail(display = "An io error has occurred - {:?}", _0)]
+    IOError(io::Error),
+    #[fail(display = "An unknown error has occurred - {:?}", _0)]
+    UnknownError(AzureError),
+}
+
+impl From<io::Error> for StorageError {
+    fn from(error: io::Error) -> Self {
+        StorageError::IOError(error)
+    }
+}
+
+impl From<AzureError> for StorageError {
+    fn from(error: AzureError) -> Self {
+        StorageError::UnknownError(error)
+    }
+}
 
 pub trait Storage {
-    fn upload(&self, &str, Vec<u8>);
+    fn upload(&self, &str, Vec<u8>) -> Result<(), StorageError>;
     fn download(&self, &PathBuf);
-    fn delete(&self, &str);
+    fn delete(&self, &str) -> Result<(), StorageError>;
+    fn list_folder_blobs(&self, &str) -> Result<Vec<String>, StorageError>;
 }
 
 pub struct AzureStorage {
@@ -21,11 +44,11 @@ pub struct AzureStorage {
 }
 
 impl Storage for AzureStorage {
-    fn upload(&self, blob_name: &str, data: Vec<u8>) {
+    fn upload(&self, blob_name: &str, data: Vec<u8>) -> Result<(), StorageError> {
         trace!("Uploading - {:?}", blob_name);
 
-        let mut core = Core::new().unwrap();
-        let client = Client::new(&self.storage_account, &self.account_key).unwrap();
+        let mut core = Core::new()?;
+        let client = Client::new(&self.storage_account, &self.account_key)?;
 
         let digest = md5::compute(&data[..]);
 
@@ -37,18 +60,20 @@ impl Storage for AzureStorage {
             .with_content_md5(&digest[..])
             .finalize();
 
-        core.run(future).unwrap();
+        core.run(future)?;
+
+        Ok(())
     }
 
     fn download(&self, p: &PathBuf) {
         trace!("Downloading - {:?}", p);
     }
 
-    fn delete(&self, blob_name: &str) {
+    fn delete(&self, blob_name: &str) -> Result<(), StorageError> {
         trace!("Deleting - {:?}", blob_name);
 
-        let mut core = Core::new().unwrap();
-        let client = Client::new(&self.storage_account, &self.account_key).unwrap();
+        let mut core = Core::new()?;
+        let client = Client::new(&self.storage_account, &self.account_key)?;
 
         let future = client
             .delete_blob()
@@ -63,38 +88,22 @@ impl Storage for AzureStorage {
             Err(AzureError::UnexpectedHTTPResult(ref h))
                 if h.status_code() == StatusCode::NOT_FOUND =>
             {
-                // if it is not found then we may be deleting a folder
-                self.delete_folder(blob_name);
+                Err(StorageError::PathNotFound)
             }
-            Err(e) => trace!("Error deleting {} - {:?}", blob_name, e),
-            Ok(_) => (),
-        };
-    }
-}
-
-impl AzureStorage {
-    pub fn new(config: &bucket::Config) -> AzureStorage {
-        AzureStorage {
-            storage_account: config.storage_account.clone(),
-            account_key: config.account_key.clone(),
-            root_container_name: config.root_container_name.clone(),
-        }
-    }
-
-    fn delete_folder(&self, blob_name: &str) {
-        let blobs_to_delete = self.list_blobs_by_folder(blob_name);
-        for blob in blobs_to_delete {
-            let encoded_blob_name: String =
-                utf8_percent_encode(&blob, DEFAULT_ENCODE_SET).collect();
-            self.delete(&encoded_blob_name);
+            Err(e) => {
+                trace!("Error deleting {} - {:?}", blob_name, e);
+                Err(StorageError::UnknownError(e))
+            }
+            Ok(_) => Ok(()),
         }
     }
 
-    fn list_blobs_by_folder(&self, blob_name: &str) -> Vec<String> {
+    fn list_folder_blobs(&self, blob_name: &str) -> Result<Vec<String>, StorageError> {
         let mut blobs = Vec::<String>::new();
         let folder_name = format!("{}/", blob_name);
-        let mut core = Core::new().unwrap();
-        let client = Client::new(&self.storage_account, &self.account_key).unwrap();
+        let mut core = Core::new()?;
+        let client = Client::new(&self.storage_account, &self.account_key)?;
+
         let future = client
             .list_blobs()
             .with_container_name(&self.root_container_name)
@@ -107,31 +116,17 @@ impl AzureStorage {
                 blobs
             });
 
-        let r = core.run(future);
-
-        match r {
-            Err(_) => Vec::<String>::new(),
-            Ok(o) => o,
-        }
+        let blobs = core.run(future)?;
+        Ok(blobs)
     }
+}
 
-    fn list_container_contents(&self) {
-        let mut core = Core::new().unwrap();
-        let client = Client::new(&self.storage_account, &self.account_key).unwrap();
-        let future = client
-            .list_blobs()
-            .with_container_name(&self.root_container_name)
-            .finalize()
-            .map(|iv| {
-                println!("List blob returned {} blobs.", iv.incomplete_vector.len());
-                for cont in iv.incomplete_vector.iter() {
-                    println!(
-                        "\t{}\t{} B\t{:?}\t{:?}",
-                        cont.name, cont.content_length, cont.last_modified, cont.content_type
-                    );
-                }
-            });
-
-        core.run(future).unwrap();
+impl AzureStorage {
+    pub fn new(config: &bucket::Config) -> AzureStorage {
+        AzureStorage {
+            storage_account: config.storage_account.clone(),
+            account_key: config.account_key.clone(),
+            root_container_name: config.root_container_name.clone(),
+        }
     }
 }