Add unit testing to file manager; Replace uuid by stringified version in FileManagerThreadMsg

components/net/filemanager_thread.rs

@@ -8,7 +8,7 @@ use mime_classifier::MIMEClassifier;
 use mime_guess::guess_mime_type_opt;
 use net_traits::blob_url_store::{BlobURLStoreEntry, BlobURLStoreError};
 use net_traits::filemanager_thread::{FileManagerThreadMsg, FileManagerResult};
-use net_traits::filemanager_thread::{SelectedFile, FileManagerThreadError};
+use net_traits::filemanager_thread::{SelectedFile, FileManagerThreadError, SelectedFileId};
 use std::collections::HashMap;
 use std::fs::File;
 use std::io::Read;
@@ -18,13 +18,6 @@ use url::Origin;
 use util::thread::spawn_named;
 use uuid::Uuid;
 
-pub struct FileManager {
-    receiver: IpcReceiver<FileManagerThreadMsg>,
-    idmap: HashMap<Uuid, PathBuf>,
-    classifier: Arc<MIMEClassifier>,
-    blob_url_store: Arc<RwLock<BlobURLStore>>,
-}
-
 pub trait FileManagerThreadFactory {
     fn new() -> Self;
 }
@@ -42,6 +35,12 @@ impl FileManagerThreadFactory for IpcSender<FileManagerThreadMsg> {
     }
 }
 
+struct FileManager {
+    receiver: IpcReceiver<FileManagerThreadMsg>,
+    idmap: HashMap<Uuid, PathBuf>,
+    classifier: Arc<MIMEClassifier>,
+    blob_url_store: Arc<RwLock<BlobURLStore>>,
+}
 
 impl FileManager {
     fn new(recv: IpcReceiver<FileManagerThreadMsg>) -> FileManager {
@@ -59,7 +58,12 @@ impl FileManager {
             match self.receiver.recv().unwrap() {
                 FileManagerThreadMsg::SelectFile(sender) => self.select_file(sender),
                 FileManagerThreadMsg::SelectFiles(sender) => self.select_files(sender),
-                FileManagerThreadMsg::ReadFile(sender, id) => self.read_file(sender, id),
+                FileManagerThreadMsg::ReadFile(sender, id) => {
+                    match self.try_read_file(id) {
+                        Ok(buffer) => { let _ = sender.send(Ok(buffer)); }
+                        Err(_) => { let _ = sender.send(Err(FileManagerThreadError::ReadFileError)); }
+                    }
+                }
                 FileManagerThreadMsg::DeleteFileID(id) => self.delete_fileid(id),
                 FileManagerThreadMsg::LoadBlob(load_data, consumer) => {
                     blob_loader::load(load_data, consumer,
@@ -67,7 +71,7 @@ impl FileManager {
                                       self.classifier.clone());
                 },
                 FileManagerThreadMsg::Exit => break,
-            }
+            };
         }
     }
 }
@@ -75,31 +79,25 @@ impl FileManager {
 impl FileManager {
     fn select_file(&mut self, sender: IpcSender<FileManagerResult<SelectedFile>>) {
         // TODO: Pull the dialog UI in and get selected
-        let selected_path = Path::new("");
+        // XXX: "test.txt" is "tests/unit/net/test.txt", for temporary testing purpose
+        let selected_path = Path::new("test.txt");
 
         match self.create_entry(selected_path) {
-            Some(triple) => {
+            Some(triple) => { let _ = sender.send(Ok(triple)); }
-                let _ = sender.send(Ok(triple));
+            None => { let _ = sender.send(Err(FileManagerThreadError::InvalidSelection)); }
-            },
+        };
-            None => {
-                let _ = sender.send(Err(FileManagerThreadError::InvalidSelection));
-            }
-        }
     }
 
     fn select_files(&mut self, sender: IpcSender<FileManagerResult<Vec<SelectedFile>>>) {
-        let selected_paths = vec![Path::new("")];
+        let selected_paths = vec![Path::new("test.txt")];
 
         let mut replies = vec![];
 
         for path in selected_paths {
             match self.create_entry(path) {
                 Some(triple) => replies.push(triple),
-                None => {
+                None => { let _ = sender.send(Err(FileManagerThreadError::InvalidSelection)); }
-                    let _ = sender.send(Err(FileManagerThreadError::InvalidSelection));
+            };
-                    return;
-                }
-            }
         }
 
         let _ = sender.send(Ok(replies));
@@ -128,7 +126,7 @@ impl FileManager {
                         let filename_path = Path::new(filename);
                         let mime = guess_mime_type_opt(filename_path);
                         Some(SelectedFile {
-                            id: id,
+                            id: SelectedFileId(id.simple().to_string()),
                             filename: filename_path.to_path_buf(),
                             modified: epoch,
                             type_string: match mime {
@@ -144,30 +142,24 @@ impl FileManager {
         }
     }
 
-    fn read_file(&mut self, sender: IpcSender<FileManagerResult<Vec<u8>>>, id: Uuid) {
+    fn try_read_file(&mut self, id: SelectedFileId) -> Result<Vec<u8>, ()> {
-        match self.idmap.get(&id).and_then(|filepath| {
+        let id = try!(Uuid::parse_str(&id.0).map_err(|_| ()));
-            let mut buffer = vec![];
+
-            match File::open(&filepath) {
+        match self.idmap.get(&id) {
-                Ok(mut handler) => {
+            Some(filepath) => {
-                    match handler.read_to_end(&mut buffer) {
+                let mut buffer = vec![];
-                        Ok(_) => Some(buffer),
+                let mut handler = try!(File::open(&filepath).map_err(|_| ()));
-                        Err(_) => None,
+                try!(handler.read_to_end(&mut buffer).map_err(|_| ()));
-                    }
+                Ok(buffer)
-                },
-                Err(_) => None,
-            }
-        }) {
-            Some(buffer) => {
-                let _ = sender.send(Ok(buffer));
             },
-            None => {
+            None => Err(())
-                let _ = sender.send(Err(FileManagerThreadError::ReadFileError));
+        }
-            }
-        };
     }
 
-    fn delete_fileid(&mut self, id: Uuid) {
+    fn delete_fileid(&mut self, id: SelectedFileId) {
-        self.idmap.remove(&id);
+        if let Ok(id) = Uuid::parse_str(&id.0) {
+            self.idmap.remove(&id);
+        }
     }
 }
 

components/net_traits/blob_url_store.rs

@@ -2,7 +2,6 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-use ipc_channel::ipc::IpcSender;
 use std::str::FromStr;
 use url::Url;
 use uuid::Uuid;
@@ -29,13 +28,6 @@ pub struct BlobURLStoreEntry {
     pub bytes: Vec<u8>,
 }
 
-/// Message-passing style interface between store and loader
-#[derive(Serialize, Deserialize)]
-pub enum BlobURLStoreMsg {
-    /// Request for an blob entry identified by uuid
-    Request(Uuid, IpcSender<Result<BlobURLStoreEntry, BlobURLStoreError>>),
-}
-
 /// Parse URL as Blob URL scheme's definition
 /// https://w3c.github.io/FileAPI/#DefinitionOfScheme
 pub fn parse_blob_url(url: &Url) -> Option<(Uuid, Option<&str>)> {

components/net_traits/filemanager_thread.rs

@@ -5,11 +5,13 @@
 use ipc_channel::ipc::IpcSender;
 use std::path::PathBuf;
 use super::{LoadConsumer, LoadData};
-use uuid::Uuid;
+
+#[derive(Clone, Debug, Deserialize, Serialize)]
+pub struct SelectedFileId(pub String);
 
 #[derive(Debug, Deserialize, Serialize)]
 pub struct SelectedFile {
-    pub id: Uuid,
+    pub id: SelectedFileId,
     pub filename: PathBuf,
     pub modified: u64,
     // https://w3c.github.io/FileAPI/#dfn-type
@@ -25,10 +27,10 @@ pub enum FileManagerThreadMsg {
     SelectFiles(IpcSender<FileManagerResult<Vec<SelectedFile>>>),
 
     /// Read file, return the bytes
-    ReadFile(IpcSender<FileManagerResult<Vec<u8>>>, Uuid),
+    ReadFile(IpcSender<FileManagerResult<Vec<u8>>>, SelectedFileId),
 
     /// Delete the FileID entry
-    DeleteFileID(Uuid),
+    DeleteFileID(SelectedFileId),
 
     /// Load resource by Blob URL
     LoadBlob(LoadData, LoadConsumer),

components/script/dom/bindings/trace.rs

@@ -57,6 +57,7 @@ use js::rust::Runtime;
 use layout_interface::LayoutRPC;
 use libc;
 use msg::constellation_msg::{FrameType, PipelineId, SubpageId, WindowSizeData, WindowSizeType, ReferrerPolicy};
+use net_traits::filemanager_thread::SelectedFileId;
 use net_traits::image::base::{Image, ImageMetadata};
 use net_traits::image_cache_thread::{ImageCacheChan, ImageCacheThread};
 use net_traits::response::HttpsState;
@@ -324,6 +325,7 @@ no_jsmanaged_fields!(USVString);
 no_jsmanaged_fields!(ReferrerPolicy);
 no_jsmanaged_fields!(ResourceThreads);
 no_jsmanaged_fields!(SystemTime);
+no_jsmanaged_fields!(SelectedFileId);
 
 impl JSTraceable for Box<ScriptChan + Send> {
     #[inline]

components/script/dom/blob.rs

@@ -14,14 +14,13 @@ use dom::bindings::str::DOMString;
 use encoding::all::UTF_8;
 use encoding::types::{EncoderTrap, Encoding};
 use ipc_channel::ipc;
-use net_traits::filemanager_thread::FileManagerThreadMsg;
+use net_traits::filemanager_thread::{FileManagerThreadMsg, SelectedFileId};
 use num_traits::ToPrimitive;
 use std::ascii::AsciiExt;
 use std::borrow::ToOwned;
 use std::cell::Cell;
 use std::cmp::{max, min};
 use std::sync::Arc;
-use uuid::Uuid;
 
 #[derive(Clone, JSTraceable)]
 pub struct DataSlice {
@@ -95,7 +94,7 @@ impl DataSlice {
 #[derive(Clone, JSTraceable)]
 pub enum BlobImpl {
     /// File-based, cached backend
-    File(Uuid, DOMRefCell<Option<DataSlice>>),
+    File(SelectedFileId, DOMRefCell<Option<DataSlice>>),
     /// Memory-based backend
     Memory(DataSlice),
 }
@@ -107,7 +106,7 @@ impl BlobImpl {
     }
 
     /// Construct file-backed BlobImpl from File ID
-    pub fn new_from_file(file_id: Uuid) -> BlobImpl {
+    pub fn new_from_file(file_id: SelectedFileId) -> BlobImpl {
         BlobImpl::File(file_id, DOMRefCell::new(None))
     }
 
@@ -184,7 +183,7 @@ impl Blob {
     }
 }
 
-fn read_file(global: GlobalRef, id: Uuid) -> Result<DataSlice, ()> {
+fn read_file(global: GlobalRef, id: SelectedFileId) -> Result<DataSlice, ()> {
     let file_manager = global.filemanager_thread();
     let (chan, recv) = ipc::channel().map_err(|_|())?;
     let _ = file_manager.send(FileManagerThreadMsg::ReadFile(chan, id));

tests/unit/net/filemanager_thread.rs

@@ -5,18 +5,60 @@
 use ipc_channel::ipc::{self, IpcSender};
 use net::filemanager_thread::FileManagerThreadFactory;
 use net_traits::filemanager_thread::{FileManagerThreadMsg, FileManagerThreadError};
+use std::fs::File;
+use std::io::Read;
+use std::path::PathBuf;
 
 #[test]
 fn test_filemanager() {
     let chan: IpcSender<FileManagerThreadMsg> = FileManagerThreadFactory::new();
 
-    {
+    // Try to open a dummy file "tests/unit/net/test.txt" in tree
-        let (tx, rx) = ipc::channel().unwrap();
+    let mut handler = File::open("test.txt").expect("test.txt is stolen");
-        let _ = chan.send(FileManagerThreadMsg::SelectFile(tx));
+    let mut test_file_content = vec![];
 
-        match rx.recv().unwrap() {
+    handler.read_to_end(&mut test_file_content)
-            Err(FileManagerThreadError::InvalidSelection) => {},
+           .expect("Read tests/unit/net/test.txt error");
-            _ => assert!(false, "Should be an invalid selection before dialog is implemented"),
+
+
+    {
+        // Try to select a dummy file "tests/unit/net/test.txt"
+        let (tx, rx) = ipc::channel().unwrap();
+        chan.send(FileManagerThreadMsg::SelectFile(tx)).unwrap();
+        let selected = rx.recv().expect("File manager channel is broken")
+                                .expect("The file manager failed to find test.txt");
+
+        // Expecting attributes conforming the spec
+        assert!(selected.filename == PathBuf::from("test.txt"));
+        assert!(selected.type_string == "text/plain".to_string());
+
+        // Test by reading, expecting same content
+        {
+            let (tx2, rx2) = ipc::channel().unwrap();
+            chan.send(FileManagerThreadMsg::ReadFile(tx2, selected.id.clone())).unwrap();
+
+            let msg = rx2.recv().expect("File manager channel is broken");
+
+            let vec = msg.expect("File manager reading failure is unexpected");
+            assert!(test_file_content == vec, "Read content differs");
+        }
+
+        // Delete the id
+        chan.send(FileManagerThreadMsg::DeleteFileID(selected.id.clone())).unwrap();
+
+        // Test by reading again, expecting read error because we invalidated the id
+        {
+            let (tx2, rx2) = ipc::channel().unwrap();
+            chan.send(FileManagerThreadMsg::ReadFile(tx2, selected.id.clone())).unwrap();
+
+            let msg = rx2.recv().expect("File manager channel is broken");
+
+            match msg {
+                Err(FileManagerThreadError::ReadFileError) => {},
+                other => {
+                    assert!(false, "Get unexpected response after deleting the id: {:?}", other);
+                }
+            }
         }
     }
 
@@ -26,9 +68,6 @@ fn test_filemanager() {
         let (tx, rx) = ipc::channel().unwrap();
         let _ = chan.send(FileManagerThreadMsg::SelectFile(tx));
 
-        match rx.try_recv() {
+        assert!(rx.try_recv().is_err(), "The thread should not respond normally after exited");
-            Ok(_) => assert!(false, "The thread should not response fine after exited"),
-            Err(_) => {},
-        }
     }
 }

tests/unit/net/test.txt

@@ -0,0 +1 @@
+hello, servo