diff --git a/src/change_processor.rs b/src/change_processor.rs index 55238ce8..5e0d3220 100644 --- a/src/change_processor.rs +++ b/src/change_processor.rs @@ -21,10 +21,10 @@ pub struct ChangeProcessor { } impl ChangeProcessor { - pub fn start( + pub fn start( tree: Arc>, message_queue: Arc>, - vfs: Arc>>, + vfs: Arc>, ) -> Self { let (shutdown_sender, shutdown_receiver) = crossbeam_channel::bounded(1); @@ -47,12 +47,9 @@ impl ChangeProcessor { shutdown_receiver: Receiver<()>, tree: Arc>, message_queue: Arc>, - vfs: Arc>>, + vfs: Arc>, ) { - let vfs_receiver = { - let vfs = vfs.lock().unwrap(); - vfs.change_receiver() - }; + let vfs_receiver = vfs.change_receiver(); // Crossbeam's select macro generates code that Clippy doesn't like, and // Clippy blames us for it. @@ -65,7 +62,6 @@ impl ChangeProcessor { log::trace!("Vfs event: {:?}", event); let applied_patches = { - let mut vfs = vfs.lock().unwrap(); vfs.commit_change(&event).expect("Error applying VFS change"); let mut tree = tree.lock().unwrap(); @@ -102,7 +98,7 @@ impl ChangeProcessor { // TODO: Use persisted snapshot // context struct instead of // recreating it every time. - let snapshot = snapshot_from_vfs(&mut InstanceSnapshotContext::default(), &mut vfs, &entry) + let snapshot = snapshot_from_vfs(&mut InstanceSnapshotContext::default(), &vfs, &entry) .expect("snapshot failed") .expect("snapshot did not return an instance"); diff --git a/src/common_setup.rs b/src/common_setup.rs index 50734117..51914181 100644 --- a/src/common_setup.rs +++ b/src/common_setup.rs @@ -14,7 +14,7 @@ use crate::{ pub fn start( fuzzy_project_path: &Path, - vfs: &mut Vfs, + vfs: &Vfs, ) -> (Option, RojoTree) { log::trace!("Loading project file from {}", fuzzy_project_path.display()); let maybe_project = match Project::load_fuzzy(fuzzy_project_path) { diff --git a/src/serve_session.rs b/src/serve_session.rs index 6a24bf65..cd3865ce 100644 --- a/src/serve_session.rs +++ b/src/serve_session.rs @@ -52,7 +52,7 @@ pub struct ServeSession { /// /// The main use for accessing it from the session is for debugging issues /// with Rojo's live-sync protocol. - vfs: Arc>>, + vfs: Arc>, /// A queue of changes that have been applied to `tree` that affect clients. /// @@ -71,7 +71,7 @@ pub struct ServeSession { /// Methods that need thread-safety bounds on VfsFetcher are limited to this /// block to prevent needing to spread Send + Sync + 'static into everything /// that handles ServeSession. -impl ServeSession { +impl ServeSession { /// Start a new serve session from the given in-memory filesystem and start /// path. /// @@ -92,7 +92,7 @@ impl ServeSession { let tree = Arc::new(Mutex::new(tree)); let message_queue = Arc::new(message_queue); - let vfs = Arc::new(Mutex::new(vfs)); + let vfs = Arc::new(vfs); log::trace!("Starting ChangeProcessor"); let change_processor = ChangeProcessor::start( @@ -122,8 +122,8 @@ impl ServeSession { self.tree.lock().unwrap() } - pub fn vfs(&self) -> MutexGuard<'_, Vfs> { - self.vfs.lock().unwrap() + pub fn vfs(&self) -> &Vfs { + &self.vfs } pub fn message_queue(&self) -> &MessageQueue { @@ -179,7 +179,7 @@ mod serve_session { #[test] fn just_folder() { - let mut vfs = Vfs::new(NoopFetcher); + let vfs = Vfs::new(NoopFetcher); vfs.debug_load_snapshot("/foo", VfsSnapshot::empty_dir()); @@ -191,7 +191,7 @@ mod serve_session { #[test] fn project_with_folder() { - let mut vfs = Vfs::new(NoopFetcher); + let vfs = Vfs::new(NoopFetcher); vfs.debug_load_snapshot( "/foo", @@ -218,7 +218,7 @@ mod serve_session { #[test] fn script_with_meta() { - let mut vfs = Vfs::new(NoopFetcher); + let vfs = Vfs::new(NoopFetcher); vfs.debug_load_snapshot( "/root", diff --git a/src/snapshot_middleware/csv.rs b/src/snapshot_middleware/csv.rs index 08c2b755..c9a09ef2 100644 --- a/src/snapshot_middleware/csv.rs +++ b/src/snapshot_middleware/csv.rs @@ -21,7 +21,7 @@ pub struct SnapshotCsv; impl SnapshotMiddleware for SnapshotCsv { fn from_vfs( _context: &mut InstanceSnapshotContext, - vfs: &mut Vfs, + vfs: &Vfs, entry: &VfsEntry, ) -> SnapshotInstanceResult<'static> { if entry.is_directory() { diff --git a/src/snapshot_middleware/dir.rs b/src/snapshot_middleware/dir.rs index 9b54ef59..7d1bed84 100644 --- a/src/snapshot_middleware/dir.rs +++ b/src/snapshot_middleware/dir.rs @@ -20,7 +20,7 @@ pub struct SnapshotDir; impl SnapshotMiddleware for SnapshotDir { fn from_vfs( context: &mut InstanceSnapshotContext, - vfs: &mut Vfs, + vfs: &Vfs, entry: &VfsEntry, ) -> SnapshotInstanceResult<'static> { if entry.is_file() { diff --git a/src/snapshot_middleware/json_model.rs b/src/snapshot_middleware/json_model.rs index 3ccde84c..18aaa53d 100644 --- a/src/snapshot_middleware/json_model.rs +++ b/src/snapshot_middleware/json_model.rs @@ -20,7 +20,7 @@ pub struct SnapshotJsonModel; impl SnapshotMiddleware for SnapshotJsonModel { fn from_vfs( _context: &mut InstanceSnapshotContext, - vfs: &mut Vfs, + vfs: &Vfs, entry: &VfsEntry, ) -> SnapshotInstanceResult<'static> { if entry.is_directory() { diff --git a/src/snapshot_middleware/lua.rs b/src/snapshot_middleware/lua.rs index bd6c54b2..2ebb7c2a 100644 --- a/src/snapshot_middleware/lua.rs +++ b/src/snapshot_middleware/lua.rs @@ -21,7 +21,7 @@ pub struct SnapshotLua; impl SnapshotMiddleware for SnapshotLua { fn from_vfs( context: &mut InstanceSnapshotContext, - vfs: &mut Vfs, + vfs: &Vfs, entry: &VfsEntry, ) -> SnapshotInstanceResult<'static> { let file_name = entry.path().file_name().unwrap().to_string_lossy(); @@ -54,7 +54,7 @@ impl SnapshotMiddleware for SnapshotLua { /// Core routine for turning Lua files into snapshots. fn snapshot_lua_file( - vfs: &mut Vfs, + vfs: &Vfs, entry: &VfsEntry, ) -> SnapshotInstanceResult<'static> { let file_name = entry.path().file_name().unwrap().to_string_lossy(); @@ -117,7 +117,7 @@ fn snapshot_lua_file( /// their parents, which acts similarly to `__init__.py` from the Python world. fn snapshot_init( context: &mut InstanceSnapshotContext, - vfs: &mut Vfs, + vfs: &Vfs, folder_entry: &VfsEntry, init_name: &str, ) -> SnapshotInstanceResult<'static> { diff --git a/src/snapshot_middleware/middleware.rs b/src/snapshot_middleware/middleware.rs index fe9a2ebb..6282e474 100644 --- a/src/snapshot_middleware/middleware.rs +++ b/src/snapshot_middleware/middleware.rs @@ -15,7 +15,7 @@ pub type SnapshotFileResult = Option<(String, VfsSnapshot)>; pub trait SnapshotMiddleware { fn from_vfs( context: &mut InstanceSnapshotContext, - vfs: &mut Vfs, + vfs: &Vfs, entry: &VfsEntry, ) -> SnapshotInstanceResult<'static>; diff --git a/src/snapshot_middleware/mod.rs b/src/snapshot_middleware/mod.rs index 4e466d7b..25f0bedb 100644 --- a/src/snapshot_middleware/mod.rs +++ b/src/snapshot_middleware/mod.rs @@ -44,7 +44,7 @@ macro_rules! middlewares { /// Generates a snapshot of instances from the given VfsEntry. pub fn snapshot_from_vfs( context: &mut InstanceSnapshotContext, - vfs: &mut Vfs, + vfs: &Vfs, entry: &VfsEntry, ) -> SnapshotInstanceResult<'static> { $( diff --git a/src/snapshot_middleware/project.rs b/src/snapshot_middleware/project.rs index a3050ff0..bb4a8ffe 100644 --- a/src/snapshot_middleware/project.rs +++ b/src/snapshot_middleware/project.rs @@ -23,7 +23,7 @@ pub struct SnapshotProject; impl SnapshotMiddleware for SnapshotProject { fn from_vfs( context: &mut InstanceSnapshotContext, - vfs: &mut Vfs, + vfs: &Vfs, entry: &VfsEntry, ) -> SnapshotInstanceResult<'static> { if entry.is_directory() { @@ -80,7 +80,7 @@ fn snapshot_project_node( context: &mut InstanceSnapshotContext, instance_name: &str, node: &ProjectNode, - vfs: &mut Vfs, + vfs: &Vfs, ) -> SnapshotInstanceResult<'static> { let name = Cow::Owned(instance_name.to_owned()); let mut class_name = node diff --git a/src/snapshot_middleware/rbxlx.rs b/src/snapshot_middleware/rbxlx.rs index 631afe22..2935fad4 100644 --- a/src/snapshot_middleware/rbxlx.rs +++ b/src/snapshot_middleware/rbxlx.rs @@ -16,7 +16,7 @@ pub struct SnapshotRbxlx; impl SnapshotMiddleware for SnapshotRbxlx { fn from_vfs( _context: &mut InstanceSnapshotContext, - vfs: &mut Vfs, + vfs: &Vfs, entry: &VfsEntry, ) -> SnapshotInstanceResult<'static> { if entry.is_directory() { diff --git a/src/snapshot_middleware/rbxm.rs b/src/snapshot_middleware/rbxm.rs index 8af33296..ec9b4ab1 100644 --- a/src/snapshot_middleware/rbxm.rs +++ b/src/snapshot_middleware/rbxm.rs @@ -18,7 +18,7 @@ pub struct SnapshotRbxm; impl SnapshotMiddleware for SnapshotRbxm { fn from_vfs( _context: &mut InstanceSnapshotContext, - vfs: &mut Vfs, + vfs: &Vfs, entry: &VfsEntry, ) -> SnapshotInstanceResult<'static> { if entry.is_directory() { diff --git a/src/snapshot_middleware/rbxmx.rs b/src/snapshot_middleware/rbxmx.rs index bd54160d..e7b05752 100644 --- a/src/snapshot_middleware/rbxmx.rs +++ b/src/snapshot_middleware/rbxmx.rs @@ -16,7 +16,7 @@ pub struct SnapshotRbxmx; impl SnapshotMiddleware for SnapshotRbxmx { fn from_vfs( _context: &mut InstanceSnapshotContext, - vfs: &mut Vfs, + vfs: &Vfs, entry: &VfsEntry, ) -> SnapshotInstanceResult<'static> { if entry.is_directory() { diff --git a/src/snapshot_middleware/txt.rs b/src/snapshot_middleware/txt.rs index 0b79eaac..54b08aa6 100644 --- a/src/snapshot_middleware/txt.rs +++ b/src/snapshot_middleware/txt.rs @@ -21,7 +21,7 @@ pub struct SnapshotTxt; impl SnapshotMiddleware for SnapshotTxt { fn from_vfs( _context: &mut InstanceSnapshotContext, - vfs: &mut Vfs, + vfs: &Vfs, entry: &VfsEntry, ) -> SnapshotInstanceResult<'static> { if entry.is_directory() { diff --git a/src/snapshot_middleware/user_plugins.rs b/src/snapshot_middleware/user_plugins.rs index f39d9336..10daecd4 100644 --- a/src/snapshot_middleware/user_plugins.rs +++ b/src/snapshot_middleware/user_plugins.rs @@ -16,7 +16,7 @@ pub struct SnapshotUserPlugins; impl SnapshotMiddleware for SnapshotUserPlugins { fn from_vfs( context: &mut InstanceSnapshotContext, - _vfs: &mut Vfs, + _vfs: &Vfs, _entry: &VfsEntry, ) -> SnapshotInstanceResult<'static> { // User plugins are only enabled if present on the snapshot context. diff --git a/src/vfs/fetcher.rs b/src/vfs/fetcher.rs index bb321093..7d38e370 100644 --- a/src/vfs/fetcher.rs +++ b/src/vfs/fetcher.rs @@ -17,20 +17,21 @@ pub enum FileType { /// In tests, it's stubbed out to do different versions of absolutely nothing /// depending on the test. pub trait VfsFetcher { - fn file_type(&mut self, path: &Path) -> io::Result; - fn read_children(&mut self, path: &Path) -> io::Result>; - fn read_contents(&mut self, path: &Path) -> io::Result>; + fn file_type(&self, path: &Path) -> io::Result; + fn read_children(&self, path: &Path) -> io::Result>; + fn read_contents(&self, path: &Path) -> io::Result>; - fn create_directory(&mut self, path: &Path) -> io::Result<()>; - fn write_file(&mut self, path: &Path, contents: &[u8]) -> io::Result<()>; - fn remove(&mut self, path: &Path) -> io::Result<()>; + fn create_directory(&self, path: &Path) -> io::Result<()>; + fn write_file(&self, path: &Path, contents: &[u8]) -> io::Result<()>; + fn remove(&self, path: &Path) -> io::Result<()>; - fn watch(&mut self, path: &Path); - fn unwatch(&mut self, path: &Path); fn receiver(&self) -> Receiver; + fn watch(&self, _path: &Path) {} + fn unwatch(&self, _path: &Path) {} + /// A method intended for debugging what paths the fetcher is watching. - fn watched_paths(&self) -> Vec<&Path> { + fn watched_paths(&self) -> Vec { Vec::new() } } diff --git a/src/vfs/noop_fetcher.rs b/src/vfs/noop_fetcher.rs index b0312185..58533a83 100644 --- a/src/vfs/noop_fetcher.rs +++ b/src/vfs/noop_fetcher.rs @@ -19,42 +19,42 @@ use super::{ pub struct NoopFetcher; impl VfsFetcher for NoopFetcher { - fn file_type(&mut self, _path: &Path) -> io::Result { + fn file_type(&self, _path: &Path) -> io::Result { Err(io::Error::new( io::ErrorKind::NotFound, "NoopFetcher always returns NotFound", )) } - fn read_children(&mut self, _path: &Path) -> io::Result> { + fn read_children(&self, _path: &Path) -> io::Result> { Err(io::Error::new( io::ErrorKind::NotFound, "NoopFetcher always returns NotFound", )) } - fn read_contents(&mut self, _path: &Path) -> io::Result> { + fn read_contents(&self, _path: &Path) -> io::Result> { Err(io::Error::new( io::ErrorKind::NotFound, "NoopFetcher always returns NotFound", )) } - fn create_directory(&mut self, _path: &Path) -> io::Result<()> { + fn create_directory(&self, _path: &Path) -> io::Result<()> { Ok(()) } - fn write_file(&mut self, _path: &Path, _contents: &[u8]) -> io::Result<()> { + fn write_file(&self, _path: &Path, _contents: &[u8]) -> io::Result<()> { Ok(()) } - fn remove(&mut self, _path: &Path) -> io::Result<()> { + fn remove(&self, _path: &Path) -> io::Result<()> { Ok(()) } - fn watch(&mut self, _path: &Path) {} + fn watch(&self, _path: &Path) {} - fn unwatch(&mut self, _path: &Path) {} + fn unwatch(&self, _path: &Path) {} fn receiver(&self) -> Receiver { crossbeam_channel::never() diff --git a/src/vfs/real_fetcher.rs b/src/vfs/real_fetcher.rs index f2f686d9..5dce585c 100644 --- a/src/vfs/real_fetcher.rs +++ b/src/vfs/real_fetcher.rs @@ -5,7 +5,7 @@ use std::{ collections::HashSet, fs, io, path::{Path, PathBuf}, - sync::mpsc, + sync::{mpsc, Mutex}, time::Duration, }; @@ -34,7 +34,7 @@ pub struct RealFetcher { // // `watcher` must be dropped before `_converter_thread` or else joining the // thread will cause a deadlock. - watcher: Option, + watcher: Option>, /// Thread handle to convert notify's mpsc channel messages into /// crossbeam_channel messages. @@ -45,7 +45,7 @@ pub struct RealFetcher { /// All of the paths that the fetcher is watching, tracked here because /// notify does not expose this information. - watched_paths: HashSet, + watched_paths: Mutex>, } impl RealFetcher { @@ -68,10 +68,12 @@ impl RealFetcher { // causing our program to deadlock. Once this is fixed, watcher no // longer needs to be optional, but is still maybe useful? let watcher = match watch_mode { - WatchMode::Enabled => Some( - notify::watcher(notify_sender, Duration::from_millis(300)) - .expect("Couldn't start 'notify' file watcher"), - ), + WatchMode::Enabled => { + let watcher = notify::watcher(notify_sender, Duration::from_millis(300)) + .expect("Couldn't start 'notify' file watcher"); + + Some(Mutex::new(watcher)) + } WatchMode::Disabled => None, }; @@ -79,7 +81,7 @@ impl RealFetcher { watcher, _converter_thread: handle, receiver, - watched_paths: HashSet::new(), + watched_paths: Mutex::new(HashSet::new()), } } } @@ -122,7 +124,7 @@ fn converter_thread(notify_receiver: mpsc::Receiver, sender: Sen } impl VfsFetcher for RealFetcher { - fn file_type(&mut self, path: &Path) -> io::Result { + fn file_type(&self, path: &Path) -> io::Result { let metadata = fs::metadata(path)?; if metadata.is_file() { @@ -132,7 +134,7 @@ impl VfsFetcher for RealFetcher { } } - fn read_children(&mut self, path: &Path) -> io::Result> { + fn read_children(&self, path: &Path) -> io::Result> { log::trace!("Reading directory {}", path.display()); let mut result = Vec::new(); @@ -146,25 +148,25 @@ impl VfsFetcher for RealFetcher { Ok(result) } - fn read_contents(&mut self, path: &Path) -> io::Result> { + fn read_contents(&self, path: &Path) -> io::Result> { log::trace!("Reading file {}", path.display()); fs::read(path) } - fn create_directory(&mut self, path: &Path) -> io::Result<()> { + fn create_directory(&self, path: &Path) -> io::Result<()> { log::trace!("Creating directory {}", path.display()); fs::create_dir(path) } - fn write_file(&mut self, path: &Path, contents: &[u8]) -> io::Result<()> { + fn write_file(&self, path: &Path, contents: &[u8]) -> io::Result<()> { log::trace!("Writing path {}", path.display()); fs::write(path, contents) } - fn remove(&mut self, path: &Path) -> io::Result<()> { + fn remove(&self, path: &Path) -> io::Result<()> { log::trace!("Removing path {}", path.display()); let metadata = fs::metadata(path)?; @@ -176,13 +178,16 @@ impl VfsFetcher for RealFetcher { } } - fn watch(&mut self, path: &Path) { + fn watch(&self, path: &Path) { log::trace!("Watching path {}", path.display()); - if let Some(watcher) = self.watcher.as_mut() { + if let Some(watcher_handle) = &self.watcher { + let mut watcher = watcher_handle.lock().unwrap(); + match watcher.watch(path, RecursiveMode::NonRecursive) { Ok(_) => { - self.watched_paths.insert(path.to_path_buf()); + let mut watched_paths = self.watched_paths.lock().unwrap(); + watched_paths.insert(path.to_path_buf()); } Err(err) => { log::warn!("Couldn't watch path {}: {:?}", path.display(), err); @@ -191,14 +196,17 @@ impl VfsFetcher for RealFetcher { } } - fn unwatch(&mut self, path: &Path) { + fn unwatch(&self, path: &Path) { log::trace!("Stopped watching path {}", path.display()); - if let Some(watcher) = self.watcher.as_mut() { + if let Some(watcher_handle) = &self.watcher { + let mut watcher = watcher_handle.lock().unwrap(); + // Remove the path from our watched paths regardless of the outcome // of notify's unwatch to ensure we drop old paths in the event of a // rename. - self.watched_paths.remove(path); + let mut watched_paths = self.watched_paths.lock().unwrap(); + watched_paths.remove(path); if let Err(err) = watcher.unwatch(path) { log::warn!("Couldn't unwatch path {}: {:?}", path.display(), err); @@ -210,7 +218,8 @@ impl VfsFetcher for RealFetcher { self.receiver.clone() } - fn watched_paths(&self) -> Vec<&Path> { - self.watched_paths.iter().map(|v| v.as_path()).collect() + fn watched_paths(&self) -> Vec { + let watched_paths = self.watched_paths.lock().unwrap(); + watched_paths.iter().map(|v| v.clone()).collect() } } diff --git a/src/vfs/test_fetcher.rs b/src/vfs/test_fetcher.rs index 9e37717c..e97f9334 100644 --- a/src/vfs/test_fetcher.rs +++ b/src/vfs/test_fetcher.rs @@ -107,7 +107,7 @@ impl TestFetcher { } impl VfsFetcher for TestFetcher { - fn file_type(&mut self, path: &Path) -> io::Result { + fn file_type(&self, path: &Path) -> io::Result { let inner = self.state.inner.lock().unwrap(); match inner.entries.get(path) { @@ -117,7 +117,7 @@ impl VfsFetcher for TestFetcher { } } - fn read_children(&mut self, path: &Path) -> io::Result> { + fn read_children(&self, path: &Path) -> io::Result> { let inner = self.state.inner.lock().unwrap(); Ok(inner @@ -129,7 +129,7 @@ impl VfsFetcher for TestFetcher { .collect()) } - fn read_contents(&mut self, path: &Path) -> io::Result> { + fn read_contents(&self, path: &Path) -> io::Result> { let inner = self.state.inner.lock().unwrap(); let node = inner.entries.get(path); @@ -144,31 +144,27 @@ impl VfsFetcher for TestFetcher { } } - fn create_directory(&mut self, _path: &Path) -> io::Result<()> { + fn create_directory(&self, _path: &Path) -> io::Result<()> { Err(io::Error::new( io::ErrorKind::Other, "TestFetcher is not mutable yet", )) } - fn write_file(&mut self, _path: &Path, _contents: &[u8]) -> io::Result<()> { + fn write_file(&self, _path: &Path, _contents: &[u8]) -> io::Result<()> { Err(io::Error::new( io::ErrorKind::Other, "TestFetcher is not mutable yet", )) } - fn remove(&mut self, _path: &Path) -> io::Result<()> { + fn remove(&self, _path: &Path) -> io::Result<()> { Err(io::Error::new( io::ErrorKind::Other, "TestFetcher is not mutable yet", )) } - fn watch(&mut self, _path: &Path) {} - - fn unwatch(&mut self, _path: &Path) {} - fn receiver(&self) -> Receiver { self.receiver.clone() } diff --git a/src/vfs/vfs.rs b/src/vfs/vfs.rs index 509e1e2e..ee3ee177 100644 --- a/src/vfs/vfs.rs +++ b/src/vfs/vfs.rs @@ -1,7 +1,7 @@ use std::{ io, path::{Path, PathBuf}, - sync::Arc, + sync::{Arc, Mutex}, }; use crossbeam_channel::Receiver; @@ -28,7 +28,7 @@ use super::{ pub struct Vfs { /// A hierarchical map from paths to items that have been read or partially /// read into memory by the Vfs. - data: PathMap, + data: Mutex>, /// This Vfs's fetcher, which is used for all actual interactions with the /// filesystem. It's referred to by the type parameter `F` all over, and is @@ -39,7 +39,7 @@ pub struct Vfs { impl Vfs { pub fn new(fetcher: F) -> Self { Self { - data: PathMap::new(), + data: Mutex::new(PathMap::new()), fetcher, } } @@ -48,108 +48,37 @@ impl Vfs { self.fetcher.receiver() } - pub fn commit_change(&mut self, event: &VfsEvent) -> FsResult<()> { + pub fn commit_change(&self, event: &VfsEvent) -> FsResult<()> { use VfsEvent::*; log::trace!("Committing Vfs change {:?}", event); + let mut data = self.data.lock().unwrap(); + match event { Created(path) | Modified(path) => { - self.raise_file_changed(path)?; + Self::raise_file_changed(&mut data, &self.fetcher, path)?; } Removed(path) => { - self.raise_file_removed(path)?; + Self::raise_file_removed(&mut data, &self.fetcher, path)?; } } Ok(()) } - fn raise_file_changed(&mut self, path: impl AsRef) -> FsResult<()> { - let path = path.as_ref(); - - if !self.would_be_resident(path) { - return Ok(()); - } - - let new_type = self - .fetcher - .file_type(path) - .map_err(|err| FsError::new(err, path.to_path_buf()))?; - - match self.data.get_mut(path) { - Some(existing_item) => { - match (existing_item, &new_type) { - (VfsItem::File(existing_file), FileType::File) => { - // Invalidate the existing file contents. - // We can probably be smarter about this by reading the changed file. - existing_file.contents = None; - } - (VfsItem::Directory(_), FileType::Directory) => { - // No changes required, a directory updating doesn't mean anything to us. - self.fetcher.watch(path); - } - (VfsItem::File(_), FileType::Directory) => { - self.data.remove(path); - self.data.insert( - path.to_path_buf(), - VfsItem::new_from_type(FileType::Directory, path), - ); - self.fetcher.watch(path); - } - (VfsItem::Directory(_), FileType::File) => { - self.data.remove(path); - self.data.insert( - path.to_path_buf(), - VfsItem::new_from_type(FileType::File, path), - ); - self.fetcher.unwatch(path); - } - } - } - None => { - self.data - .insert(path.to_path_buf(), VfsItem::new_from_type(new_type, path)); - } - } - - Ok(()) + pub fn get(&self, path: impl AsRef) -> FsResult { + let mut data = self.data.lock().unwrap(); + Self::get_internal(&mut data, &self.fetcher, path) } - fn raise_file_removed(&mut self, path: impl AsRef) -> FsResult<()> { + pub fn get_contents(&self, path: impl AsRef) -> FsResult>> { let path = path.as_ref(); - if !self.would_be_resident(path) { - return Ok(()); - } + let mut data = self.data.lock().unwrap(); + Self::read_if_not_exists(&mut data, &self.fetcher, path)?; - self.data.remove(path); - self.fetcher.unwatch(path); - Ok(()) - } - - pub fn get(&mut self, path: impl AsRef) -> FsResult { - self.read_if_not_exists(path.as_ref())?; - - let item = self.data.get(path.as_ref()).unwrap(); - - let is_file = match item { - VfsItem::File(_) => true, - VfsItem::Directory(_) => false, - }; - - Ok(VfsEntry { - path: item.path().to_path_buf(), - is_file, - }) - } - - pub fn get_contents(&mut self, path: impl AsRef) -> FsResult>> { - let path = path.as_ref(); - - self.read_if_not_exists(path)?; - - match self.data.get_mut(path).unwrap() { + match data.get_mut(path).unwrap() { VfsItem::File(file) => { if file.contents.is_none() { file.contents = Some( @@ -169,26 +98,26 @@ impl Vfs { } } - pub fn get_children(&mut self, path: impl AsRef) -> FsResult> { + pub fn get_children(&self, path: impl AsRef) -> FsResult> { let path = path.as_ref(); - self.read_if_not_exists(path)?; + let mut data = self.data.lock().unwrap(); + Self::read_if_not_exists(&mut data, &self.fetcher, path)?; - match self.data.get_mut(path).unwrap() { + match data.get_mut(path).unwrap() { VfsItem::Directory(dir) => { self.fetcher.watch(path); let enumerated = dir.children_enumerated; if enumerated { - self.data - .children(path) + data.children(path) .unwrap() // TODO: Handle None here, which means the PathMap entry did not exist. .into_iter() .map(PathBuf::from) // Convert paths from &Path to PathBuf .collect::>() // Collect all PathBufs, since self.get needs to borrow self mutably. .into_iter() - .map(|path| self.get(path)) + .map(|path| Self::get_internal(&mut data, &self.fetcher, path)) .collect::>>() } else { dir.children_enumerated = true; @@ -197,7 +126,7 @@ impl Vfs { .read_children(path) .map_err(|err| FsError::new(err, path.to_path_buf()))? .into_iter() - .map(|path| self.get(path)) + .map(|path| Self::get_internal(&mut data, &self.fetcher, path)) .collect::>>() } } @@ -208,6 +137,118 @@ impl Vfs { } } + fn get_internal( + data: &mut PathMap, + fetcher: &F, + path: impl AsRef, + ) -> FsResult { + let path = path.as_ref(); + + Self::read_if_not_exists(data, fetcher, path)?; + + let item = data.get(path).unwrap(); + + let is_file = match item { + VfsItem::File(_) => true, + VfsItem::Directory(_) => false, + }; + + Ok(VfsEntry { + path: item.path().to_path_buf(), + is_file, + }) + } + + fn raise_file_changed( + data: &mut PathMap, + fetcher: &F, + path: impl AsRef, + ) -> FsResult<()> { + let path = path.as_ref(); + + if !Self::would_be_resident(&data, path) { + return Ok(()); + } + + let new_type = fetcher + .file_type(path) + .map_err(|err| FsError::new(err, path.to_path_buf()))?; + + match data.get_mut(path) { + Some(existing_item) => { + match (existing_item, &new_type) { + (VfsItem::File(existing_file), FileType::File) => { + // Invalidate the existing file contents. + // We can probably be smarter about this by reading the changed file. + existing_file.contents = None; + } + (VfsItem::Directory(_), FileType::Directory) => { + // No changes required, a directory updating doesn't mean anything to us. + fetcher.watch(path); + } + (VfsItem::File(_), FileType::Directory) => { + data.remove(path); + data.insert( + path.to_path_buf(), + VfsItem::new_from_type(FileType::Directory, path), + ); + fetcher.watch(path); + } + (VfsItem::Directory(_), FileType::File) => { + data.remove(path); + data.insert( + path.to_path_buf(), + VfsItem::new_from_type(FileType::File, path), + ); + fetcher.unwatch(path); + } + } + } + None => { + data.insert(path.to_path_buf(), VfsItem::new_from_type(new_type, path)); + } + } + + Ok(()) + } + + fn raise_file_removed( + data: &mut PathMap, + fetcher: &F, + path: impl AsRef, + ) -> FsResult<()> { + let path = path.as_ref(); + + if !Self::would_be_resident(data, path) { + return Ok(()); + } + + data.remove(path); + fetcher.unwatch(path); + Ok(()) + } + + /// Attempts to read the path into the `Vfs` if it doesn't exist. + /// + /// This does not necessitate that file contents or directory children will + /// be read. Depending on the `VfsFetcher` implementation that the `Vfs` + /// is using, this call may read exactly only the given path and no more. + fn read_if_not_exists(data: &mut PathMap, fetcher: &F, path: &Path) -> FsResult<()> { + if !data.contains_key(path) { + let kind = fetcher + .file_type(path) + .map_err(|err| FsError::new(err, path.to_path_buf()))?; + + if kind == FileType::Directory { + fetcher.watch(path); + } + + data.insert(path.to_path_buf(), VfsItem::new_from_type(kind, path)); + } + + Ok(()) + } + /// Tells whether the given path, if it were loaded, would be loaded if it /// existed. /// @@ -218,113 +259,108 @@ impl Vfs { /// tangible changes to the in-memory filesystem. If a path would be /// resident, we need to read it, and if its contents were known before, we /// need to update them. - fn would_be_resident(&self, path: &Path) -> bool { - if self.data.contains_key(path) { + fn would_be_resident(data: &PathMap, path: &Path) -> bool { + if data.contains_key(path) { return true; } if let Some(parent) = path.parent() { - if let Some(VfsItem::Directory(dir)) = self.data.get(parent) { + if let Some(VfsItem::Directory(dir)) = data.get(parent) { return !dir.children_enumerated; } } false } - - /// Attempts to read the path into the `Vfs` if it doesn't exist. - /// - /// This does not necessitate that file contents or directory children will - /// be read. Depending on the `VfsFetcher` implementation that the `Vfs` - /// is using, this call may read exactly only the given path and no more. - fn read_if_not_exists(&mut self, path: &Path) -> FsResult<()> { - if !self.data.contains_key(path) { - let kind = self - .fetcher - .file_type(path) - .map_err(|err| FsError::new(err, path.to_path_buf()))?; - - if kind == FileType::Directory { - self.fetcher.watch(path); - } - - self.data - .insert(path.to_path_buf(), VfsItem::new_from_type(kind, path)); - } - - Ok(()) - } } /// Contains extra methods that should only be used for debugging. They're /// broken out into a separate trait to make it more explicit to depend on them. pub trait VfsDebug { - fn debug_load_snapshot>(&mut self, path: P, snapshot: VfsSnapshot); + fn debug_load_snapshot>(&self, path: P, snapshot: VfsSnapshot); fn debug_is_file(&self, path: &Path) -> bool; - fn debug_contents<'a>(&'a self, path: &Path) -> Option<&'a [u8]>; - fn debug_children<'a>(&'a self, path: &Path) -> Option<(bool, Vec<&'a Path>)>; - fn debug_orphans(&self) -> Vec<&Path>; - fn debug_watched_paths(&self) -> Vec<&Path>; + fn debug_contents(&self, path: &Path) -> Option>>; + fn debug_children(&self, path: &Path) -> Option<(bool, Vec)>; + fn debug_orphans(&self) -> Vec; + fn debug_watched_paths(&self) -> Vec; } impl VfsDebug for Vfs { - fn debug_load_snapshot>(&mut self, path: P, snapshot: VfsSnapshot) { - let path = path.as_ref(); + fn debug_load_snapshot>(&self, path: P, snapshot: VfsSnapshot) { + fn load_snapshot>( + data: &mut PathMap, + path: P, + snapshot: VfsSnapshot, + ) { + let path = path.as_ref(); - match snapshot { - VfsSnapshot::File(file) => { - self.data.insert( - path.to_path_buf(), - VfsItem::File(VfsFile { - path: path.to_path_buf(), - contents: Some(Arc::new(file.contents)), - }), - ); - } - VfsSnapshot::Directory(directory) => { - self.data.insert( - path.to_path_buf(), - VfsItem::Directory(VfsDirectory { - path: path.to_path_buf(), - children_enumerated: true, - }), - ); + match snapshot { + VfsSnapshot::File(file) => { + data.insert( + path.to_path_buf(), + VfsItem::File(VfsFile { + path: path.to_path_buf(), + contents: Some(Arc::new(file.contents)), + }), + ); + } + VfsSnapshot::Directory(directory) => { + data.insert( + path.to_path_buf(), + VfsItem::Directory(VfsDirectory { + path: path.to_path_buf(), + children_enumerated: true, + }), + ); - for (child_name, child) in directory.children.into_iter() { - self.debug_load_snapshot(path.join(child_name), child); + for (child_name, child) in directory.children.into_iter() { + load_snapshot(data, path.join(child_name), child); + } } } } + + let mut data = self.data.lock().unwrap(); + load_snapshot(&mut data, path, snapshot) } fn debug_is_file(&self, path: &Path) -> bool { - match self.data.get(path) { + let data = self.data.lock().unwrap(); + match data.get(path) { Some(VfsItem::File(_)) => true, _ => false, } } - fn debug_contents<'a>(&'a self, path: &Path) -> Option<&'a [u8]> { - match self.data.get(path) { - Some(VfsItem::File(file)) => file.contents.as_ref().map(|vec| vec.as_slice()), + fn debug_contents(&self, path: &Path) -> Option>> { + let data = self.data.lock().unwrap(); + match data.get(path) { + Some(VfsItem::File(file)) => file.contents.clone(), _ => None, } } - fn debug_children<'a>(&'a self, path: &Path) -> Option<(bool, Vec<&'a Path>)> { - match self.data.get(path) { - Some(VfsItem::Directory(dir)) => { - Some((dir.children_enumerated, self.data.children(path).unwrap())) - } + fn debug_children(&self, path: &Path) -> Option<(bool, Vec)> { + let data = self.data.lock().unwrap(); + match data.get(path) { + Some(VfsItem::Directory(dir)) => Some(( + dir.children_enumerated, + data.children(path) + .unwrap() + .iter() + .map(|path| path.to_path_buf()) + .collect(), + )), _ => None, } } - fn debug_orphans(&self) -> Vec<&Path> { - self.data.orphans().collect() + fn debug_orphans(&self) -> Vec { + let data = self.data.lock().unwrap(); + data.orphans().map(|path| path.to_path_buf()).collect() } - fn debug_watched_paths(&self) -> Vec<&Path> { + fn debug_watched_paths(&self) -> Vec { self.fetcher.watched_paths() } } @@ -345,11 +381,11 @@ impl VfsEntry { &self.path } - pub fn contents<'vfs>(&self, vfs: &'vfs mut Vfs) -> FsResult>> { + pub fn contents(&self, vfs: &Vfs) -> FsResult>> { vfs.get_contents(&self.path) } - pub fn children(&self, vfs: &mut Vfs) -> FsResult> { + pub fn children(&self, vfs: &Vfs) -> FsResult> { vfs.get_children(&self.path) } @@ -414,7 +450,7 @@ mod test { #[test] fn from_snapshot_file() { - let mut vfs = Vfs::new(NoopFetcher); + let vfs = Vfs::new(NoopFetcher); let file = VfsSnapshot::file("hello, world!"); vfs.debug_load_snapshot("/hello.txt", file); @@ -425,7 +461,7 @@ mod test { #[test] fn from_snapshot_dir() { - let mut vfs = Vfs::new(NoopFetcher); + let vfs = Vfs::new(NoopFetcher); let dir = VfsSnapshot::dir(hashmap! { "a.txt" => VfsSnapshot::file("contents of a.txt"), "b.lua" => VfsSnapshot::file("contents of b.lua"), @@ -470,7 +506,7 @@ mod test { } impl VfsFetcher for MockFetcher { - fn file_type(&mut self, path: &Path) -> io::Result { + fn file_type(&self, path: &Path) -> io::Result { if path == Path::new("/dir/a.txt") { return Ok(FileType::File); } @@ -478,7 +514,7 @@ mod test { unimplemented!(); } - fn read_contents(&mut self, path: &Path) -> io::Result> { + fn read_contents(&self, path: &Path) -> io::Result> { if path == Path::new("/dir/a.txt") { let inner = self.inner.borrow(); @@ -488,26 +524,22 @@ mod test { unimplemented!(); } - fn read_children(&mut self, _path: &Path) -> io::Result> { + fn read_children(&self, _path: &Path) -> io::Result> { unimplemented!(); } - fn create_directory(&mut self, _path: &Path) -> io::Result<()> { + fn create_directory(&self, _path: &Path) -> io::Result<()> { unimplemented!(); } - fn write_file(&mut self, _path: &Path, _contents: &[u8]) -> io::Result<()> { + fn write_file(&self, _path: &Path, _contents: &[u8]) -> io::Result<()> { unimplemented!(); } - fn remove(&mut self, _path: &Path) -> io::Result<()> { + fn remove(&self, _path: &Path) -> io::Result<()> { unimplemented!(); } - fn watch(&mut self, _path: &Path) {} - - fn unwatch(&mut self, _path: &Path) {} - fn receiver(&self) -> Receiver { crossbeam_channel::never() } @@ -532,7 +564,7 @@ mod test { mock_state.a_contents = "Changed contents"; } - vfs.raise_file_changed("/dir/a.txt") + vfs.commit_change(&VfsEvent::Modified(PathBuf::from("/dir/a.txt"))) .expect("error processing file change"); let contents = a.contents(&mut vfs).expect("mock file contents error"); @@ -555,7 +587,7 @@ mod test { assert_eq!(contents.as_slice(), b"hello, world!"); - vfs.raise_file_removed("/hello.txt") + vfs.commit_change(&VfsEvent::Removed(PathBuf::from("/hello.txt"))) .expect("error processing file removal"); match vfs.get("hello.txt") { diff --git a/src/web/ui.rs b/src/web/ui.rs index a4224948..8f7276be 100644 --- a/src/web/ui.rs +++ b/src/web/ui.rs @@ -102,7 +102,7 @@ impl UiService { let orphans: Vec<_> = vfs .debug_orphans() .into_iter() - .map(|path| Self::render_vfs_path(&vfs, path, true)) + .map(|path| Self::render_vfs_path(&vfs, &path, true)) .collect(); let watched_list: Vec<_> = vfs @@ -153,7 +153,7 @@ impl UiService { let children: Vec<_> = children .into_iter() - .map(|child| Self::render_vfs_path(vfs, child, false)) + .map(|child| Self::render_vfs_path(vfs, &child, false)) .collect(); let note = if is_exhaustive {