From 486b067567f3771910b6e08100e17dc830ba1005 Mon Sep 17 00:00:00 2001 From: Lucien Greathouse Date: Wed, 17 Jun 2020 13:47:09 -0700 Subject: [PATCH] Flatten snapshot middleware to be much simpler (#324) * First take at flattening middleware for simpler code and better perf * Undo debug prints * Fix using wrong path in snapshot_from_vfs * Disable some broken tests * Re-enable (mistakenly?) disabled CSV test * Fix some tests * Update project file tests * Fix benchmark --- benches/build.rs | 1 + src/snapshot_middleware/csv.rs | 97 ++++---- src/snapshot_middleware/dir.rs | 124 +++++----- src/snapshot_middleware/json.rs | 89 +++---- src/snapshot_middleware/json_model.rs | 94 +++----- src/snapshot_middleware/lua.rs | 97 +++----- src/snapshot_middleware/middleware.rs | 10 +- src/snapshot_middleware/mod.rs | 117 +++++---- src/snapshot_middleware/project.rs | 226 +++++++++--------- src/snapshot_middleware/rbxlx.rs | 46 ---- src/snapshot_middleware/rbxm.rs | 74 +++--- src/snapshot_middleware/rbxmx.rs | 66 +++-- ..._project__test__project_with_children.snap | 6 +- ...ct_with_path_to_project_with_children.snap | 2 +- ...est__project_with_resolved_properties.snap | 4 +- ...t__project_with_unresolved_properties.snap | 4 +- src/snapshot_middleware/txt.rs | 89 ++++--- src/snapshot_middleware/user_plugins.rs | 56 ----- 18 files changed, 489 insertions(+), 713 deletions(-) delete mode 100644 src/snapshot_middleware/rbxlx.rs delete mode 100644 src/snapshot_middleware/user_plugins.rs diff --git a/benches/build.rs b/benches/build.rs index 802f43a3..0b1e8934 100644 --- a/benches/build.rs +++ b/benches/build.rs @@ -35,6 +35,7 @@ fn place_setup>(input_path: P) -> (TempDir, BuildCommand) { let options = BuildCommand { project: input, + watch: false, output, }; diff --git a/src/snapshot_middleware/csv.rs b/src/snapshot_middleware/csv.rs index 72075a3e..fcd358f6 100644 --- a/src/snapshot_middleware/csv.rs +++ b/src/snapshot_middleware/csv.rs @@ -8,54 +8,41 @@ use serde::Serialize; use crate::snapshot::{InstanceContext, InstanceMetadata, InstanceSnapshot}; use super::{ - error::SnapshotError, - meta_file::AdjacentMetadata, - middleware::{SnapshotInstanceResult, SnapshotMiddleware}, - util::match_file_name, + error::SnapshotError, meta_file::AdjacentMetadata, middleware::SnapshotInstanceResult, }; -pub struct SnapshotCsv; +pub fn snapshot_csv( + _context: &InstanceContext, + vfs: &Vfs, + path: &Path, + instance_name: &str, +) -> SnapshotInstanceResult { + let meta_path = path.with_file_name(format!("{}.meta.json", instance_name)); + let contents = vfs.read(path)?; -impl SnapshotMiddleware for SnapshotCsv { - fn from_vfs(_context: &InstanceContext, vfs: &Vfs, path: &Path) -> SnapshotInstanceResult { - let meta = vfs.metadata(path)?; + let table_contents = convert_localization_csv(&contents) + .map_err(|source| SnapshotError::malformed_l10n_csv(source, path))?; - if meta.is_dir() { - return Ok(None); - } + let mut snapshot = InstanceSnapshot::new() + .name(instance_name) + .class_name("LocalizationTable") + .properties(hashmap! { + "Contents".to_owned() => RbxValue::String { + value: table_contents, + }, + }) + .metadata( + InstanceMetadata::new() + .instigating_source(path) + .relevant_paths(vec![path.to_path_buf(), meta_path.clone()]), + ); - let instance_name = match match_file_name(path, ".csv") { - Some(name) => name, - None => return Ok(None), - }; - - let meta_path = path.with_file_name(format!("{}.meta.json", instance_name)); - let contents = vfs.read(path)?; - - let table_contents = convert_localization_csv(&contents) - .map_err(|source| SnapshotError::malformed_l10n_csv(source, path))?; - - let mut snapshot = InstanceSnapshot::new() - .name(instance_name) - .class_name("LocalizationTable") - .properties(hashmap! { - "Contents".to_owned() => RbxValue::String { - value: table_contents, - }, - }) - .metadata( - InstanceMetadata::new() - .instigating_source(path) - .relevant_paths(vec![path.to_path_buf(), meta_path.clone()]), - ); - - if let Some(meta_contents) = vfs.read(&meta_path).with_not_found()? { - let mut metadata = AdjacentMetadata::from_slice(&meta_contents, &meta_path)?; - metadata.apply_all(&mut snapshot); - } - - Ok(Some(snapshot)) + if let Some(meta_contents) = vfs.read(&meta_path).with_not_found()? { + let mut metadata = AdjacentMetadata::from_slice(&meta_contents, &meta_path)?; + metadata.apply_all(&mut snapshot); } + + Ok(Some(snapshot)) } /// Struct that holds any valid row from a Roblox CSV translation table. @@ -156,10 +143,14 @@ Ack,Ack!,,An exclamation of despair,¡Ay!"#, let mut vfs = Vfs::new(imfs); - let instance_snapshot = - SnapshotCsv::from_vfs(&InstanceContext::default(), &mut vfs, Path::new("/foo.csv")) - .unwrap() - .unwrap(); + let instance_snapshot = snapshot_csv( + &InstanceContext::default(), + &mut vfs, + Path::new("/foo.csv"), + "foo", + ) + .unwrap() + .unwrap(); insta::assert_yaml_snapshot!(instance_snapshot); } @@ -184,9 +175,15 @@ Ack,Ack!,,An exclamation of despair,¡Ay!"#, let mut vfs = Vfs::new(imfs); - let instance_snapshot = - SnapshotCsv::from_vfs(&InstanceContext::default(), &mut vfs, Path::new("/foo.csv")) - .unwrap() - .unwrap(); + let instance_snapshot = snapshot_csv( + &InstanceContext::default(), + &mut vfs, + Path::new("/foo.csv"), + "foo", + ) + .unwrap() + .unwrap(); + + insta::assert_yaml_snapshot!(instance_snapshot); } } diff --git a/src/snapshot_middleware/dir.rs b/src/snapshot_middleware/dir.rs index 8e9bb932..359856d0 100644 --- a/src/snapshot_middleware/dir.rs +++ b/src/snapshot_middleware/dir.rs @@ -5,81 +5,69 @@ use memofs::{DirEntry, IoResultExt, Vfs}; use crate::snapshot::{InstanceContext, InstanceMetadata, InstanceSnapshot}; use super::{ - error::SnapshotError, - meta_file::DirectoryMetadata, - middleware::{SnapshotInstanceResult, SnapshotMiddleware}, + error::SnapshotError, meta_file::DirectoryMetadata, middleware::SnapshotInstanceResult, snapshot_from_vfs, }; -pub struct SnapshotDir; +pub fn snapshot_dir(context: &InstanceContext, vfs: &Vfs, path: &Path) -> SnapshotInstanceResult { + let passes_filter_rules = |child: &DirEntry| { + context + .path_ignore_rules + .iter() + .all(|rule| rule.passes(child.path())) + }; -impl SnapshotMiddleware for SnapshotDir { - fn from_vfs(context: &InstanceContext, vfs: &Vfs, path: &Path) -> SnapshotInstanceResult { - let meta = vfs.metadata(path)?; + let mut snapshot_children = Vec::new(); - if meta.is_file() { - return Ok(None); + for entry in vfs.read_dir(path)? { + let entry = entry?; + + if !passes_filter_rules(&entry) { + continue; } - let passes_filter_rules = |child: &DirEntry| { - context - .path_ignore_rules - .iter() - .all(|rule| rule.passes(child.path())) - }; - - let mut snapshot_children = Vec::new(); - - for entry in vfs.read_dir(path)? { - let entry = entry?; - - if !passes_filter_rules(&entry) { - continue; - } - - if let Some(child_snapshot) = snapshot_from_vfs(context, vfs, entry.path())? { - snapshot_children.push(child_snapshot); - } + if let Some(child_snapshot) = snapshot_from_vfs(context, vfs, entry.path())? { + snapshot_children.push(child_snapshot); } - - let instance_name = path - .file_name() - .expect("Could not extract file name") - .to_str() - .ok_or_else(|| SnapshotError::file_name_bad_unicode(path))? - .to_string(); - - let meta_path = path.join("init.meta.json"); - - let relevant_paths = vec![ - path.to_path_buf(), - meta_path.clone(), - // TODO: We shouldn't need to know about Lua existing in this - // middleware. Should we figure out a way for that function to add - // relevant paths to this middleware? - path.join("init.lua"), - path.join("init.server.lua"), - path.join("init.client.lua"), - ]; - - let mut snapshot = InstanceSnapshot::new() - .name(instance_name) - .class_name("Folder") - .children(snapshot_children) - .metadata( - InstanceMetadata::new() - .instigating_source(path) - .relevant_paths(relevant_paths) - .context(context), - ); - - if let Some(meta_contents) = vfs.read(&meta_path).with_not_found()? { - let mut metadata = DirectoryMetadata::from_slice(&meta_contents, &meta_path)?; - metadata.apply_all(&mut snapshot); - } - - Ok(Some(snapshot)) } + + let instance_name = path + .file_name() + .expect("Could not extract file name") + .to_str() + .ok_or_else(|| SnapshotError::file_name_bad_unicode(path))? + .to_string(); + + let meta_path = path.join("init.meta.json"); + + let relevant_paths = vec![ + path.to_path_buf(), + meta_path.clone(), + // TODO: We shouldn't need to know about Lua existing in this + // middleware. Should we figure out a way for that function to add + // relevant paths to this middleware? + path.join("init.lua"), + path.join("init.server.lua"), + path.join("init.client.lua"), + ]; + + let mut snapshot = InstanceSnapshot::new() + .name(instance_name) + .class_name("Folder") + .children(snapshot_children) + .metadata( + InstanceMetadata::new() + .instigating_source(path) + .relevant_paths(relevant_paths) + .context(context), + ); + + if let Some(meta_contents) = vfs.read(&meta_path).with_not_found()? { + let mut metadata = DirectoryMetadata::from_slice(&meta_contents, &meta_path)?; + metadata.apply_all(&mut snapshot); + } + + Ok(Some(snapshot)) } #[cfg(test)] @@ -98,7 +86,7 @@ mod test { let mut vfs = Vfs::new(imfs); let instance_snapshot = - SnapshotDir::from_vfs(&InstanceContext::default(), &mut vfs, Path::new("/foo")) + snapshot_dir(&InstanceContext::default(), &mut vfs, Path::new("/foo")) .unwrap() .unwrap(); @@ -119,7 +107,7 @@ mod test { let mut vfs = Vfs::new(imfs); let instance_snapshot = - SnapshotDir::from_vfs(&InstanceContext::default(), &mut vfs, Path::new("/foo")) + snapshot_dir(&InstanceContext::default(), &mut vfs, Path::new("/foo")) .unwrap() .unwrap(); diff --git a/src/snapshot_middleware/json.rs b/src/snapshot_middleware/json.rs index 4b3508e3..15efe21b 100644 --- a/src/snapshot_middleware/json.rs +++ b/src/snapshot_middleware/json.rs @@ -10,69 +10,47 @@ use crate::{ }; use super::{ - error::SnapshotError, - meta_file::AdjacentMetadata, - middleware::{SnapshotInstanceResult, SnapshotMiddleware}, - util::match_file_name, + error::SnapshotError, meta_file::AdjacentMetadata, middleware::SnapshotInstanceResult, }; -/// Catch-all middleware for snapshots on JSON files that aren't used for other -/// features, like Rojo projects, JSON models, or meta files. -pub struct SnapshotJson; +pub fn snapshot_json( + context: &InstanceContext, + vfs: &Vfs, + path: &Path, + instance_name: &str, +) -> SnapshotInstanceResult { + let contents = vfs.read(path)?; -impl SnapshotMiddleware for SnapshotJson { - fn from_vfs(context: &InstanceContext, vfs: &Vfs, path: &Path) -> SnapshotInstanceResult { - let meta = vfs.metadata(path)?; + let value: serde_json::Value = serde_json::from_slice(&contents) + .map_err(|err| SnapshotError::malformed_json(err, path))?; - if meta.is_dir() { - return Ok(None); - } + let as_lua = json_to_lua(value).to_string(); - // FIXME: This middleware should not need to know about the .meta.json - // middleware. Should there be a way to signal "I'm not returning an - // instance and no one should"? - if match_file_name(path, ".meta.json").is_some() { - return Ok(None); - } + let properties = hashmap! { + "Source".to_owned() => RbxValue::String { + value: as_lua, + }, + }; - let instance_name = match match_file_name(path, ".json") { - Some(name) => name, - None => return Ok(None), - }; + let meta_path = path.with_file_name(format!("{}.meta.json", instance_name)); - let contents = vfs.read(path)?; + let mut snapshot = InstanceSnapshot::new() + .name(instance_name) + .class_name("ModuleScript") + .properties(properties) + .metadata( + InstanceMetadata::new() + .instigating_source(path) + .relevant_paths(vec![path.to_path_buf(), meta_path.clone()]) + .context(context), + ); - let value: serde_json::Value = serde_json::from_slice(&contents) - .map_err(|err| SnapshotError::malformed_json(err, path))?; - - let as_lua = json_to_lua(value).to_string(); - - let properties = hashmap! { - "Source".to_owned() => RbxValue::String { - value: as_lua, - }, - }; - - let meta_path = path.with_file_name(format!("{}.meta.json", instance_name)); - - let mut snapshot = InstanceSnapshot::new() - .name(instance_name) - .class_name("ModuleScript") - .properties(properties) - .metadata( - InstanceMetadata::new() - .instigating_source(path) - .relevant_paths(vec![path.to_path_buf(), meta_path.clone()]) - .context(context), - ); - - if let Some(meta_contents) = vfs.read(&meta_path).with_not_found()? { - let mut metadata = AdjacentMetadata::from_slice(&meta_contents, &meta_path)?; - metadata.apply_all(&mut snapshot); - } - - Ok(Some(snapshot)) + if let Some(meta_contents) = vfs.read(&meta_path).with_not_found()? { + let mut metadata = AdjacentMetadata::from_slice(&meta_contents, &meta_path)?; + metadata.apply_all(&mut snapshot); } + + Ok(Some(snapshot)) } fn json_to_lua(value: serde_json::Value) -> Statement { @@ -129,10 +107,11 @@ mod test { let mut vfs = Vfs::new(imfs.clone()); - let instance_snapshot = SnapshotJson::from_vfs( + let instance_snapshot = snapshot_json( &InstanceContext::default(), &mut vfs, Path::new("/foo.json"), + "foo", ) .unwrap() .unwrap(); diff --git a/src/snapshot_middleware/json_model.rs b/src/snapshot_middleware/json_model.rs index f132fdf6..74ed97fd 100644 --- a/src/snapshot_middleware/json_model.rs +++ b/src/snapshot_middleware/json_model.rs @@ -7,68 +7,45 @@ use serde::Deserialize; use crate::snapshot::{InstanceContext, InstanceSnapshot}; -use super::{ - error::SnapshotError, - middleware::{SnapshotInstanceResult, SnapshotMiddleware}, - util::match_file_name, -}; +use super::{error::SnapshotError, middleware::SnapshotInstanceResult}; -pub struct SnapshotJsonModel; +pub fn snapshot_json_model( + context: &InstanceContext, + vfs: &Vfs, + path: &Path, + instance_name: &str, +) -> SnapshotInstanceResult { + let contents = vfs.read(path)?; + let instance: JsonModel = serde_json::from_slice(&contents) + .map_err(|source| SnapshotError::malformed_model_json(source, path))?; -impl SnapshotMiddleware for SnapshotJsonModel { - fn from_vfs(context: &InstanceContext, vfs: &Vfs, path: &Path) -> SnapshotInstanceResult { - let meta = vfs.metadata(path)?; - - if meta.is_dir() { - return Ok(None); + if let Some(json_name) = &instance.name { + if json_name != instance_name { + log::warn!( + "Name from JSON model did not match its file name: {}", + path.display() + ); + log::warn!( + "In Rojo < alpha 14, this model is named \"{}\" (from its 'Name' property)", + json_name + ); + log::warn!( + "In Rojo >= alpha 14, this model is named \"{}\" (from its file name)", + instance_name + ); + log::warn!("'Name' for the top-level instance in a JSON model is now optional and will be ignored."); } - - let instance_name = match match_file_name(path, ".model.json") { - Some(name) => name, - None => return Ok(None), - }; - - let contents = vfs.read(path)?; - let instance: JsonModel = serde_json::from_slice(&contents) - .map_err(|source| SnapshotError::malformed_model_json(source, path))?; - - if let Some(json_name) = &instance.name { - if json_name != instance_name { - log::warn!( - "Name from JSON model did not match its file name: {}", - path.display() - ); - log::warn!( - "In Rojo < alpha 14, this model is named \"{}\" (from its 'Name' property)", - json_name - ); - log::warn!( - "In Rojo >= alpha 14, this model is named \"{}\" (from its file name)", - instance_name - ); - log::warn!("'Name' for the top-level instance in a JSON model is now optional and will be ignored."); - } - } - - let mut snapshot = instance.core.into_snapshot(instance_name.to_owned()); - - snapshot.metadata = snapshot - .metadata - .instigating_source(path) - .relevant_paths(vec![path.to_path_buf()]) - .context(context); - - Ok(Some(snapshot)) } -} -fn match_trailing<'a>(input: &'a str, trailer: &str) -> Option<&'a str> { - if input.ends_with(trailer) { - let end = input.len().saturating_sub(trailer.len()); - Some(&input[..end]) - } else { - None - } + let mut snapshot = instance.core.into_snapshot(instance_name.to_owned()); + + snapshot.metadata = snapshot + .metadata + .instigating_source(path) + .relevant_paths(vec![path.to_path_buf()]) + .context(context); + + Ok(Some(snapshot)) } #[derive(Debug, Deserialize)] @@ -164,10 +141,11 @@ mod test { let mut vfs = Vfs::new(imfs); - let instance_snapshot = SnapshotJsonModel::from_vfs( + let instance_snapshot = snapshot_json_model( &InstanceContext::default(), &mut vfs, Path::new("/foo.model.json"), + "foo", ) .unwrap() .unwrap(); diff --git a/src/snapshot_middleware/lua.rs b/src/snapshot_middleware/lua.rs index abf98973..668f57a7 100644 --- a/src/snapshot_middleware/lua.rs +++ b/src/snapshot_middleware/lua.rs @@ -7,50 +7,12 @@ use rbx_dom_weak::RbxValue; use crate::snapshot::{InstanceContext, InstanceMetadata, InstanceSnapshot}; use super::{ - dir::SnapshotDir, - meta_file::AdjacentMetadata, - middleware::{SnapshotInstanceResult, SnapshotMiddleware}, + dir::snapshot_dir, meta_file::AdjacentMetadata, middleware::SnapshotInstanceResult, util::match_trailing, }; -pub struct SnapshotLua; - -impl SnapshotMiddleware for SnapshotLua { - fn from_vfs(context: &InstanceContext, vfs: &Vfs, path: &Path) -> SnapshotInstanceResult { - let file_name = path.file_name().unwrap().to_string_lossy(); - - // These paths alter their parent instance, so we don't need to turn - // them into a script instance here. - match &*file_name { - "init.lua" | "init.server.lua" | "init.client.lua" => return Ok(None), - _ => {} - } - - let meta = vfs.metadata(path)?; - - if meta.is_file() { - snapshot_lua_file(context, vfs, path) - } else { - // At this point, our entry is definitely a directory! - - if let Some(snapshot) = snapshot_init(context, vfs, path, "init.lua")? { - // An `init.lua` file turns its parent into a ModuleScript - Ok(Some(snapshot)) - } else if let Some(snapshot) = snapshot_init(context, vfs, path, "init.server.lua")? { - // An `init.server.lua` file turns its parent into a Script - Ok(Some(snapshot)) - } else if let Some(snapshot) = snapshot_init(context, vfs, path, "init.client.lua")? { - // An `init.client.lua` file turns its parent into a LocalScript - Ok(Some(snapshot)) - } else { - Ok(None) - } - } - } -} - /// Core routine for turning Lua files into snapshots. -fn snapshot_lua_file(context: &InstanceContext, vfs: &Vfs, path: &Path) -> SnapshotInstanceResult { +pub fn snapshot_lua(context: &InstanceContext, vfs: &Vfs, path: &Path) -> SnapshotInstanceResult { let file_name = path.file_name().unwrap().to_string_lossy(); let (class_name, instance_name) = if let Some(name) = match_trailing(&file_name, ".server.lua") @@ -100,35 +62,29 @@ fn snapshot_lua_file(context: &InstanceContext, vfs: &Vfs, path: &Path) -> Snaps /// /// Scripts named `init.lua`, `init.server.lua`, or `init.client.lua` usurp /// their parents, which acts similarly to `__init__.py` from the Python world. -fn snapshot_init( +pub fn snapshot_lua_init( context: &InstanceContext, vfs: &Vfs, - folder_path: &Path, - init_name: &str, + init_path: &Path, ) -> SnapshotInstanceResult { - let init_path = folder_path.join(init_name); + let folder_path = init_path.parent().unwrap(); + let dir_snapshot = snapshot_dir(context, vfs, folder_path)?.unwrap(); - if vfs.metadata(&init_path).with_not_found()?.is_some() { - if let Some(dir_snapshot) = SnapshotDir::from_vfs(context, vfs, folder_path)? { - if let Some(mut init_snapshot) = snapshot_lua_file(context, vfs, &init_path)? { - if dir_snapshot.class_name != "Folder" { - panic!( - "init.lua, init.server.lua, and init.client.lua can \ - only be used if the instance produced by the parent \ - directory would be a Folder." - ); - } - - init_snapshot.name = dir_snapshot.name; - init_snapshot.children = dir_snapshot.children; - init_snapshot.metadata = dir_snapshot.metadata; - - return Ok(Some(init_snapshot)); - } - } + if dir_snapshot.class_name != "Folder" { + panic!( + "init.lua, init.server.lua, and init.client.lua can \ + only be used if the instance produced by the parent \ + directory would be a Folder." + ); } - Ok(None) + let mut init_snapshot = snapshot_lua(context, vfs, init_path)?.unwrap(); + + init_snapshot.name = dir_snapshot.name; + init_snapshot.children = dir_snapshot.children; + init_snapshot.metadata = dir_snapshot.metadata; + + Ok(Some(init_snapshot)) } #[cfg(test)] @@ -146,7 +102,7 @@ mod test { let mut vfs = Vfs::new(imfs); let instance_snapshot = - SnapshotLua::from_vfs(&InstanceContext::default(), &mut vfs, Path::new("/foo.lua")) + snapshot_lua(&InstanceContext::default(), &mut vfs, Path::new("/foo.lua")) .unwrap() .unwrap(); @@ -161,7 +117,7 @@ mod test { let mut vfs = Vfs::new(imfs); - let instance_snapshot = SnapshotLua::from_vfs( + let instance_snapshot = snapshot_lua( &InstanceContext::default(), &mut vfs, Path::new("/foo.server.lua"), @@ -180,7 +136,7 @@ mod test { let mut vfs = Vfs::new(imfs); - let instance_snapshot = SnapshotLua::from_vfs( + let instance_snapshot = snapshot_lua( &InstanceContext::default(), &mut vfs, Path::new("/foo.client.lua"), @@ -191,6 +147,7 @@ mod test { insta::assert_yaml_snapshot!(instance_snapshot); } + #[ignore = "init.lua functionality has moved to the root snapshot function"] #[test] fn init_module_from_vfs() { let mut imfs = InMemoryFs::new(); @@ -205,7 +162,7 @@ mod test { let mut vfs = Vfs::new(imfs); let instance_snapshot = - SnapshotLua::from_vfs(&InstanceContext::default(), &mut vfs, Path::new("/root")) + snapshot_lua(&InstanceContext::default(), &mut vfs, Path::new("/root")) .unwrap() .unwrap(); @@ -232,7 +189,7 @@ mod test { let mut vfs = Vfs::new(imfs); let instance_snapshot = - SnapshotLua::from_vfs(&InstanceContext::default(), &mut vfs, Path::new("/foo.lua")) + snapshot_lua(&InstanceContext::default(), &mut vfs, Path::new("/foo.lua")) .unwrap() .unwrap(); @@ -258,7 +215,7 @@ mod test { let mut vfs = Vfs::new(imfs); - let instance_snapshot = SnapshotLua::from_vfs( + let instance_snapshot = snapshot_lua( &InstanceContext::default(), &mut vfs, Path::new("/foo.server.lua"), @@ -290,7 +247,7 @@ mod test { let mut vfs = Vfs::new(imfs); - let instance_snapshot = SnapshotLua::from_vfs( + let instance_snapshot = snapshot_lua( &InstanceContext::default(), &mut vfs, Path::new("/bar.server.lua"), diff --git a/src/snapshot_middleware/middleware.rs b/src/snapshot_middleware/middleware.rs index 18e79439..b05f0b9e 100644 --- a/src/snapshot_middleware/middleware.rs +++ b/src/snapshot_middleware/middleware.rs @@ -1,13 +1,5 @@ -use std::path::Path; - -use memofs::Vfs; - -use crate::snapshot::{InstanceContext, InstanceSnapshot}; +use crate::snapshot::InstanceSnapshot; use super::error::SnapshotError; pub type SnapshotInstanceResult = Result, SnapshotError>; - -pub trait SnapshotMiddleware { - fn from_vfs(context: &InstanceContext, vfs: &Vfs, path: &Path) -> SnapshotInstanceResult; -} diff --git a/src/snapshot_middleware/mod.rs b/src/snapshot_middleware/mod.rs index 561949c2..9563d8b6 100644 --- a/src/snapshot_middleware/mod.rs +++ b/src/snapshot_middleware/mod.rs @@ -12,7 +12,6 @@ mod lua; mod meta_file; mod middleware; mod project; -mod rbxlx; mod rbxm; mod rbxmx; mod txt; @@ -20,59 +19,87 @@ mod util; use std::path::Path; -use memofs::Vfs; +use memofs::{IoResultExt, Vfs}; use crate::snapshot::InstanceContext; use self::{ - csv::SnapshotCsv, - dir::SnapshotDir, - json::SnapshotJson, - json_model::SnapshotJsonModel, - lua::SnapshotLua, - middleware::{SnapshotInstanceResult, SnapshotMiddleware}, - project::SnapshotProject, - rbxlx::SnapshotRbxlx, - rbxm::SnapshotRbxm, - rbxmx::SnapshotRbxmx, - txt::SnapshotTxt, + csv::snapshot_csv, + dir::snapshot_dir, + json::snapshot_json, + json_model::snapshot_json_model, + lua::{snapshot_lua, snapshot_lua_init}, + middleware::SnapshotInstanceResult, + project::snapshot_project, + rbxm::snapshot_rbxm, + rbxmx::snapshot_rbxmx, + txt::snapshot_txt, + util::match_file_name, }; pub use self::error::*; pub use self::project::snapshot_project_node; -macro_rules! middlewares { - ( $($middleware: ident,)* ) => { - /// Generates a snapshot of instances from the given path. - pub fn snapshot_from_vfs( - context: &InstanceContext, - vfs: &Vfs, - path: &Path, - ) -> SnapshotInstanceResult { - $( - log::trace!("trying middleware {} on {}", stringify!($middleware), path.display()); - - if let Some(snapshot) = $middleware::from_vfs(context, vfs, path)? { - log::trace!("middleware {} success on {}", stringify!($middleware), path.display()); - return Ok(Some(snapshot)); - } - )* - - log::trace!("no middleware returned Ok(Some)"); - Ok(None) - } +pub fn snapshot_from_vfs( + context: &InstanceContext, + vfs: &Vfs, + path: &Path, +) -> SnapshotInstanceResult { + let meta = match vfs.metadata(path).with_not_found()? { + Some(meta) => meta, + None => return Ok(None), }; -} -middlewares! { - SnapshotProject, - SnapshotJsonModel, - SnapshotRbxlx, - SnapshotRbxmx, - SnapshotRbxm, - SnapshotLua, - SnapshotCsv, - SnapshotTxt, - SnapshotJson, - SnapshotDir, + if meta.is_dir() { + let project_path = path.join("default.project.json"); + if vfs.metadata(&project_path).with_not_found()?.is_some() { + return snapshot_project(context, vfs, &project_path); + } + + let init_path = path.join("init.lua"); + if vfs.metadata(&init_path).with_not_found()?.is_some() { + return snapshot_lua_init(context, vfs, &init_path); + } + + let init_path = path.join("init.server.lua"); + if vfs.metadata(&init_path).with_not_found()?.is_some() { + return snapshot_lua_init(context, vfs, &init_path); + } + + let init_path = path.join("init.client.lua"); + if vfs.metadata(&init_path).with_not_found()?.is_some() { + return snapshot_lua_init(context, vfs, &init_path); + } + + snapshot_dir(context, vfs, path) + } else { + if let Some(name) = match_file_name(path, ".lua") { + match name { + // init scripts are handled elsewhere and should not turn into + // their own children. + "init" | "init.client" | "init.server" => return Ok(None), + + _ => return snapshot_lua(context, vfs, path), + } + } else if let Some(_name) = match_file_name(path, ".project.json") { + return snapshot_project(context, vfs, path); + } else if let Some(name) = match_file_name(path, ".model.json") { + return snapshot_json_model(context, vfs, path, name); + } else if let Some(_name) = match_file_name(path, ".meta.json") { + // .meta.json files do not turn into their own instances. + return Ok(None); + } else if let Some(name) = match_file_name(path, ".json") { + return snapshot_json(context, vfs, path, name); + } else if let Some(name) = match_file_name(path, ".csv") { + return snapshot_csv(context, vfs, path, name); + } else if let Some(name) = match_file_name(path, ".txt") { + return snapshot_txt(context, vfs, path, name); + } else if let Some(name) = match_file_name(path, ".rbxmx") { + return snapshot_rbxmx(context, vfs, path, name); + } else if let Some(name) = match_file_name(path, ".rbxm") { + return snapshot_rbxm(context, vfs, path, name); + } + + Ok(None) + } } diff --git a/src/snapshot_middleware/project.rs b/src/snapshot_middleware/project.rs index cefed521..94414bd5 100644 --- a/src/snapshot_middleware/project.rs +++ b/src/snapshot_middleware/project.rs @@ -1,6 +1,6 @@ use std::{borrow::Cow, collections::HashMap, path::Path}; -use memofs::{IoResultExt, Vfs}; +use memofs::Vfs; use rbx_reflection::{get_class_descriptor, try_resolve_value}; use crate::{ @@ -10,92 +10,61 @@ use crate::{ }, }; -use super::{ - error::SnapshotError, - middleware::{SnapshotInstanceResult, SnapshotMiddleware}, - snapshot_from_vfs, -}; +use super::{error::SnapshotError, middleware::SnapshotInstanceResult, snapshot_from_vfs}; -/// Handles snapshots for: -/// * Files ending in `.project.json` -/// * Folders containing a file named `default.project.json` -pub struct SnapshotProject; +pub fn snapshot_project( + context: &InstanceContext, + vfs: &Vfs, + path: &Path, +) -> SnapshotInstanceResult { + let project = Project::load_from_slice(&vfs.read(path)?, path) + .map_err(|err| SnapshotError::malformed_project(err, path))?; -impl SnapshotMiddleware for SnapshotProject { - fn from_vfs(context: &InstanceContext, vfs: &Vfs, path: &Path) -> SnapshotInstanceResult { - let meta = vfs.metadata(path)?; + let mut context = context.clone(); - if meta.is_dir() { - let project_path = path.join("default.project.json"); + let rules = project.glob_ignore_paths.iter().map(|glob| PathIgnoreRule { + glob: glob.clone(), + base_path: project.folder_location().to_path_buf(), + }); - match vfs.metadata(&project_path).with_not_found()? { - // TODO: Do we need to muck with the relevant paths if we're a - // project file within a folder? Should the folder path be the - // relevant path instead of the project file path? - Some(_meta) => return SnapshotProject::from_vfs(context, vfs, &project_path), - None => return Ok(None), - } - } + context.add_path_ignore_rules(rules); - if !path.to_string_lossy().ends_with(".project.json") { - // This isn't a project file, so it's not our job. - return Ok(None); - } + // TODO: If this project node is a path to an instance that Rojo doesn't + // understand, this may panic! + let mut snapshot = + snapshot_project_node(&context, path, &project.name, &project.tree, vfs, None)?.unwrap(); - let project = Project::load_from_slice(&vfs.read(path)?, path) - .map_err(|err| SnapshotError::malformed_project(err, path))?; + // Setting the instigating source to the project file path is a little + // coarse. + // + // Ideally, we'd only snapshot the project file if the project file + // actually changed. Because Rojo only has the concept of one + // relevant path -> snapshot path mapping per instance, we pick the more + // conservative approach of snapshotting the project file if any + // relevant paths changed. + snapshot.metadata.instigating_source = Some(path.to_path_buf().into()); - let mut context = context.clone(); + // Mark this snapshot (the root node of the project file) as being + // related to the project file. + // + // We SHOULD NOT mark the project file as a relevant path for any + // nodes that aren't roots. They'll be updated as part of the project + // file being updated. + snapshot.metadata.relevant_paths.push(path.to_path_buf()); - let rules = project.glob_ignore_paths.iter().map(|glob| PathIgnoreRule { - glob: glob.clone(), - base_path: project.folder_location().to_path_buf(), - }); - - context.add_path_ignore_rules(rules); - - // Snapshotting a project should always return an instance, so this - // unwrap is safe. - let mut snapshot = snapshot_project_node( - &context, - project.folder_location(), - &project.name, - &project.tree, - vfs, - None, - )? - .unwrap(); - - // Setting the instigating source to the project file path is a little - // coarse. - // - // Ideally, we'd only snapshot the project file if the project file - // actually changed. Because Rojo only has the concept of one - // relevant path -> snapshot path mapping per instance, we pick the more - // conservative approach of snapshotting the project file if any - // relevant paths changed. - snapshot.metadata.instigating_source = Some(path.to_path_buf().into()); - - // Mark this snapshot (the root node of the project file) as being - // related to the project file. - // - // We SHOULD NOT mark the project file as a relevant path for any - // nodes that aren't roots. They'll be updated as part of the project - // file being updated. - snapshot.metadata.relevant_paths.push(path.to_path_buf()); - - Ok(Some(snapshot)) - } + Ok(Some(snapshot)) } pub fn snapshot_project_node( context: &InstanceContext, - project_folder: &Path, + project_path: &Path, instance_name: &str, node: &ProjectNode, vfs: &Vfs, parent_class: Option<&str>, ) -> SnapshotInstanceResult { + let project_folder = project_path.parent().unwrap(); + let name = Cow::Owned(instance_name.to_owned()); let mut class_name = node .class_name @@ -191,7 +160,7 @@ pub fn snapshot_project_node( for (child_name, child_project_node) in &node.children { if let Some(child) = snapshot_project_node( context, - project_folder, + project_path, child_name, child_project_node, vfs, @@ -223,7 +192,7 @@ pub fn snapshot_project_node( } metadata.instigating_source = Some(InstigatingSource::ProjectNode( - project_folder.to_path_buf(), + project_path.to_path_buf(), instance_name.to_string(), node.clone(), parent_class.map(|name| name.to_owned()), @@ -239,6 +208,7 @@ pub fn snapshot_project_node( })) } +// #[cfg(feature = "broken-tests")] #[cfg(test)] mod test { use super::*; @@ -246,6 +216,7 @@ mod test { use maplit::hashmap; use memofs::{InMemoryFs, VfsSnapshot}; + #[ignore = "Functionality moved to root snapshot middleware"] #[test] fn project_from_folder() { let _ = env_logger::try_init(); @@ -269,7 +240,7 @@ mod test { let mut vfs = Vfs::new(imfs); let instance_snapshot = - SnapshotProject::from_vfs(&InstanceContext::default(), &mut vfs, Path::new("/foo")) + snapshot_project(&InstanceContext::default(), &mut vfs, Path::new("/foo")) .expect("snapshot error") .expect("snapshot returned no instances"); @@ -298,7 +269,7 @@ mod test { let mut vfs = Vfs::new(imfs); - let instance_snapshot = SnapshotProject::from_vfs( + let instance_snapshot = snapshot_project( &InstanceContext::default(), &mut vfs, Path::new("/foo/hello.project.json"), @@ -315,9 +286,9 @@ mod test { let mut imfs = InMemoryFs::new(); imfs.load_snapshot( - "/foo", - VfsSnapshot::dir(hashmap! { - "default.project.json" => VfsSnapshot::file(r#" + "/foo.project.json", + VfsSnapshot::file( + r#" { "name": "resolved-properties", "tree": { @@ -330,17 +301,20 @@ mod test { } } } - "#), - }), + "#, + ), ) .unwrap(); let mut vfs = Vfs::new(imfs); - let instance_snapshot = - SnapshotProject::from_vfs(&InstanceContext::default(), &mut vfs, Path::new("/foo")) - .expect("snapshot error") - .expect("snapshot returned no instances"); + let instance_snapshot = snapshot_project( + &InstanceContext::default(), + &mut vfs, + Path::new("/foo.project.json"), + ) + .expect("snapshot error") + .expect("snapshot returned no instances"); insta::assert_yaml_snapshot!(instance_snapshot); } @@ -351,9 +325,9 @@ mod test { let mut imfs = InMemoryFs::new(); imfs.load_snapshot( - "/foo", - VfsSnapshot::dir(hashmap! { - "default.project.json" => VfsSnapshot::file(r#" + "/foo.project.json", + VfsSnapshot::file( + r#" { "name": "unresolved-properties", "tree": { @@ -363,17 +337,20 @@ mod test { } } } - "#), - }), + "#, + ), ) .unwrap(); let mut vfs = Vfs::new(imfs); - let instance_snapshot = - SnapshotProject::from_vfs(&InstanceContext::default(), &mut vfs, Path::new("/foo")) - .expect("snapshot error") - .expect("snapshot returned no instances"); + let instance_snapshot = snapshot_project( + &InstanceContext::default(), + &mut vfs, + Path::new("/foo.project.json"), + ) + .expect("snapshot error") + .expect("snapshot returned no instances"); insta::assert_yaml_snapshot!(instance_snapshot); } @@ -384,9 +361,9 @@ mod test { let mut imfs = InMemoryFs::new(); imfs.load_snapshot( - "/foo", - VfsSnapshot::dir(hashmap! { - "default.project.json" => VfsSnapshot::file(r#" + "/foo.project.json", + VfsSnapshot::file( + r#" { "name": "children", "tree": { @@ -397,17 +374,20 @@ mod test { } } } - "#), - }), + "#, + ), ) .unwrap(); let mut vfs = Vfs::new(imfs); - let instance_snapshot = - SnapshotProject::from_vfs(&InstanceContext::default(), &mut vfs, Path::new("/foo")) - .expect("snapshot error") - .expect("snapshot returned no instances"); + let instance_snapshot = snapshot_project( + &InstanceContext::default(), + &mut vfs, + Path::new("/foo.project.json"), + ) + .expect("snapshot error") + .expect("snapshot returned no instances"); insta::assert_yaml_snapshot!(instance_snapshot); } @@ -435,10 +415,13 @@ mod test { let mut vfs = Vfs::new(imfs); - let instance_snapshot = - SnapshotProject::from_vfs(&InstanceContext::default(), &mut vfs, Path::new("/foo")) - .expect("snapshot error") - .expect("snapshot returned no instances"); + let instance_snapshot = snapshot_project( + &InstanceContext::default(), + &mut vfs, + Path::new("/foo/default.project.json"), + ) + .expect("snapshot error") + .expect("snapshot returned no instances"); insta::assert_yaml_snapshot!(instance_snapshot); } @@ -473,10 +456,13 @@ mod test { let mut vfs = Vfs::new(imfs); - let instance_snapshot = - SnapshotProject::from_vfs(&InstanceContext::default(), &mut vfs, Path::new("/foo")) - .expect("snapshot error") - .expect("snapshot returned no instances"); + let instance_snapshot = snapshot_project( + &InstanceContext::default(), + &mut vfs, + Path::new("/foo/default.project.json"), + ) + .expect("snapshot error") + .expect("snapshot returned no instances"); insta::assert_yaml_snapshot!(instance_snapshot); } @@ -515,10 +501,13 @@ mod test { let mut vfs = Vfs::new(imfs); - let instance_snapshot = - SnapshotProject::from_vfs(&InstanceContext::default(), &mut vfs, Path::new("/foo")) - .expect("snapshot error") - .expect("snapshot returned no instances"); + let instance_snapshot = snapshot_project( + &InstanceContext::default(), + &mut vfs, + Path::new("/foo/default.project.json"), + ) + .expect("snapshot error") + .expect("snapshot returned no instances"); insta::assert_yaml_snapshot!(instance_snapshot); } @@ -562,10 +551,13 @@ mod test { let mut vfs = Vfs::new(imfs); - let instance_snapshot = - SnapshotProject::from_vfs(&InstanceContext::default(), &mut vfs, Path::new("/foo")) - .expect("snapshot error") - .expect("snapshot returned no instances"); + let instance_snapshot = snapshot_project( + &InstanceContext::default(), + &mut vfs, + Path::new("/foo/default.project.json"), + ) + .expect("snapshot error") + .expect("snapshot returned no instances"); insta::assert_yaml_snapshot!(instance_snapshot); } diff --git a/src/snapshot_middleware/rbxlx.rs b/src/snapshot_middleware/rbxlx.rs deleted file mode 100644 index 94cc942d..00000000 --- a/src/snapshot_middleware/rbxlx.rs +++ /dev/null @@ -1,46 +0,0 @@ -use std::path::Path; - -use memofs::Vfs; - -use crate::snapshot::{InstanceContext, InstanceMetadata, InstanceSnapshot}; - -use super::{ - middleware::{SnapshotInstanceResult, SnapshotMiddleware}, - util::match_file_name, -}; - -pub struct SnapshotRbxlx; - -impl SnapshotMiddleware for SnapshotRbxlx { - fn from_vfs(context: &InstanceContext, vfs: &Vfs, path: &Path) -> SnapshotInstanceResult { - let meta = vfs.metadata(path)?; - - if meta.is_dir() { - return Ok(None); - } - - let instance_name = match match_file_name(path, ".rbxlx") { - Some(name) => name, - None => return Ok(None), - }; - - let options = rbx_xml::DecodeOptions::new() - .property_behavior(rbx_xml::DecodePropertyBehavior::ReadUnknown); - - let temp_tree = rbx_xml::from_reader(vfs.read(path)?.as_slice(), options) - .expect("TODO: Handle rbx_xml errors"); - - let root_id = temp_tree.get_root_id(); - - let snapshot = InstanceSnapshot::from_tree(&temp_tree, root_id) - .name(instance_name) - .metadata( - InstanceMetadata::new() - .instigating_source(path) - .relevant_paths(vec![path.to_path_buf()]) - .context(context), - ); - - Ok(Some(snapshot)) - } -} diff --git a/src/snapshot_middleware/rbxm.rs b/src/snapshot_middleware/rbxm.rs index 319d316a..5c7624ba 100644 --- a/src/snapshot_middleware/rbxm.rs +++ b/src/snapshot_middleware/rbxm.rs @@ -5,53 +5,40 @@ use rbx_dom_weak::{RbxInstanceProperties, RbxTree}; use crate::snapshot::{InstanceContext, InstanceMetadata, InstanceSnapshot}; -use super::{ - middleware::{SnapshotInstanceResult, SnapshotMiddleware}, - util::match_file_name, -}; +use super::middleware::SnapshotInstanceResult; -pub struct SnapshotRbxm; +pub fn snapshot_rbxm( + context: &InstanceContext, + vfs: &Vfs, + path: &Path, + instance_name: &str, +) -> SnapshotInstanceResult { + let mut temp_tree = RbxTree::new(RbxInstanceProperties { + name: "DataModel".to_owned(), + class_name: "DataModel".to_owned(), + properties: HashMap::new(), + }); -impl SnapshotMiddleware for SnapshotRbxm { - fn from_vfs(context: &InstanceContext, vfs: &Vfs, path: &Path) -> SnapshotInstanceResult { - let meta = vfs.metadata(path)?; + let root_id = temp_tree.get_root_id(); + rbx_binary::decode(&mut temp_tree, root_id, vfs.read(path)?.as_slice()) + .expect("TODO: Handle rbx_binary errors"); - if meta.is_dir() { - return Ok(None); - } + let root_instance = temp_tree.get_instance(root_id).unwrap(); + let children = root_instance.get_children_ids(); - let instance_name = match match_file_name(path, ".rbxm") { - Some(name) => name, - None => return Ok(None), - }; + if children.len() == 1 { + let snapshot = InstanceSnapshot::from_tree(&temp_tree, children[0]) + .name(instance_name) + .metadata( + InstanceMetadata::new() + .instigating_source(path) + .relevant_paths(vec![path.to_path_buf()]) + .context(context), + ); - let mut temp_tree = RbxTree::new(RbxInstanceProperties { - name: "DataModel".to_owned(), - class_name: "DataModel".to_owned(), - properties: HashMap::new(), - }); - - let root_id = temp_tree.get_root_id(); - rbx_binary::decode(&mut temp_tree, root_id, vfs.read(path)?.as_slice()) - .expect("TODO: Handle rbx_binary errors"); - - let root_instance = temp_tree.get_instance(root_id).unwrap(); - let children = root_instance.get_children_ids(); - - if children.len() == 1 { - let snapshot = InstanceSnapshot::from_tree(&temp_tree, children[0]) - .name(instance_name) - .metadata( - InstanceMetadata::new() - .instigating_source(path) - .relevant_paths(vec![path.to_path_buf()]) - .context(context), - ); - - Ok(Some(snapshot)) - } else { - panic!("Rojo doesn't have support for model files with zero or more than one top-level instances yet."); - } + Ok(Some(snapshot)) + } else { + panic!("Rojo doesn't have support for model files with zero or more than one top-level instances yet."); } } @@ -72,10 +59,11 @@ mod test { let mut vfs = Vfs::new(imfs); - let instance_snapshot = SnapshotRbxm::from_vfs( + let instance_snapshot = snapshot_rbxm( &InstanceContext::default(), &mut vfs, Path::new("/foo.rbxm"), + "foo", ) .unwrap() .unwrap(); diff --git a/src/snapshot_middleware/rbxmx.rs b/src/snapshot_middleware/rbxmx.rs index 90193b62..b8012101 100644 --- a/src/snapshot_middleware/rbxmx.rs +++ b/src/snapshot_middleware/rbxmx.rs @@ -4,49 +4,36 @@ use memofs::Vfs; use crate::snapshot::{InstanceContext, InstanceMetadata, InstanceSnapshot}; -use super::{ - middleware::{SnapshotInstanceResult, SnapshotMiddleware}, - util::match_file_name, -}; +use super::middleware::SnapshotInstanceResult; -pub struct SnapshotRbxmx; +pub fn snapshot_rbxmx( + context: &InstanceContext, + vfs: &Vfs, + path: &Path, + instance_name: &str, +) -> SnapshotInstanceResult { + let options = rbx_xml::DecodeOptions::new() + .property_behavior(rbx_xml::DecodePropertyBehavior::ReadUnknown); -impl SnapshotMiddleware for SnapshotRbxmx { - fn from_vfs(context: &InstanceContext, vfs: &Vfs, path: &Path) -> SnapshotInstanceResult { - let meta = vfs.metadata(path)?; + let temp_tree = rbx_xml::from_reader(vfs.read(path)?.as_slice(), options) + .expect("TODO: Handle rbx_xml errors"); - if meta.is_dir() { - return Ok(None); - } + let root_instance = temp_tree.get_instance(temp_tree.get_root_id()).unwrap(); + let children = root_instance.get_children_ids(); - let instance_name = match match_file_name(path, ".rbxmx") { - Some(name) => name, - None => return Ok(None), - }; + if children.len() == 1 { + let snapshot = InstanceSnapshot::from_tree(&temp_tree, children[0]) + .name(instance_name) + .metadata( + InstanceMetadata::new() + .instigating_source(path) + .relevant_paths(vec![path.to_path_buf()]) + .context(context), + ); - let options = rbx_xml::DecodeOptions::new() - .property_behavior(rbx_xml::DecodePropertyBehavior::ReadUnknown); - - let temp_tree = rbx_xml::from_reader(vfs.read(path)?.as_slice(), options) - .expect("TODO: Handle rbx_xml errors"); - - let root_instance = temp_tree.get_instance(temp_tree.get_root_id()).unwrap(); - let children = root_instance.get_children_ids(); - - if children.len() == 1 { - let snapshot = InstanceSnapshot::from_tree(&temp_tree, children[0]) - .name(instance_name) - .metadata( - InstanceMetadata::new() - .instigating_source(path) - .relevant_paths(vec![path.to_path_buf()]) - .context(context), - ); - - Ok(Some(snapshot)) - } else { - panic!("Rojo doesn't have support for model files with zero or more than one top-level instances yet."); - } + Ok(Some(snapshot)) + } else { + panic!("Rojo doesn't have support for model files with zero or more than one top-level instances yet."); } } @@ -77,10 +64,11 @@ mod test { let mut vfs = Vfs::new(imfs); - let instance_snapshot = SnapshotRbxmx::from_vfs( + let instance_snapshot = snapshot_rbxmx( &InstanceContext::default(), &mut vfs, Path::new("/foo.rbxmx"), + "foo", ) .unwrap() .unwrap(); diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_children.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_children.snap index e5603c97..ccf6b535 100644 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_children.snap +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_children.snap @@ -6,9 +6,9 @@ snapshot_id: ~ metadata: ignore_unknown_instances: true instigating_source: - Path: /foo/default.project.json + Path: /foo.project.json relevant_paths: - - /foo/default.project.json + - /foo.project.json context: {} name: children class_name: Folder @@ -19,7 +19,7 @@ children: ignore_unknown_instances: true instigating_source: ProjectNode: - - /foo + - /foo.project.json - Child - $className: Model - Folder diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_path_to_project_with_children.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_path_to_project_with_children.snap index 389d24e2..c71b4d2d 100644 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_path_to_project_with_children.snap +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_path_to_project_with_children.snap @@ -20,7 +20,7 @@ children: ignore_unknown_instances: true instigating_source: ProjectNode: - - /foo + - /foo/other.project.json - SomeChild - $className: Model - Folder diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_resolved_properties.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_resolved_properties.snap index cb6025f7..21159485 100644 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_resolved_properties.snap +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_resolved_properties.snap @@ -6,9 +6,9 @@ snapshot_id: ~ metadata: ignore_unknown_instances: true instigating_source: - Path: /foo/default.project.json + Path: /foo.project.json relevant_paths: - - /foo/default.project.json + - /foo.project.json context: {} name: resolved-properties class_name: StringValue diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_unresolved_properties.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_unresolved_properties.snap index a8a0e6df..d3cbb593 100644 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_unresolved_properties.snap +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_unresolved_properties.snap @@ -6,9 +6,9 @@ snapshot_id: ~ metadata: ignore_unknown_instances: true instigating_source: - Path: /foo/default.project.json + Path: /foo.project.json relevant_paths: - - /foo/default.project.json + - /foo.project.json context: {} name: unresolved-properties class_name: StringValue diff --git a/src/snapshot_middleware/txt.rs b/src/snapshot_middleware/txt.rs index 6e4fcc0c..edf1fbf5 100644 --- a/src/snapshot_middleware/txt.rs +++ b/src/snapshot_middleware/txt.rs @@ -7,58 +7,45 @@ use rbx_dom_weak::RbxValue; use crate::snapshot::{InstanceContext, InstanceMetadata, InstanceSnapshot}; use super::{ - error::SnapshotError, - meta_file::AdjacentMetadata, - middleware::{SnapshotInstanceResult, SnapshotMiddleware}, - util::match_file_name, + error::SnapshotError, meta_file::AdjacentMetadata, middleware::SnapshotInstanceResult, }; -pub struct SnapshotTxt; +pub fn snapshot_txt( + context: &InstanceContext, + vfs: &Vfs, + path: &Path, + instance_name: &str, +) -> SnapshotInstanceResult { + let contents = vfs.read(path)?; + let contents_str = str::from_utf8(&contents) + .map_err(|err| SnapshotError::file_contents_bad_unicode(err, path))? + .to_string(); -impl SnapshotMiddleware for SnapshotTxt { - fn from_vfs(context: &InstanceContext, vfs: &Vfs, path: &Path) -> SnapshotInstanceResult { - let meta = vfs.metadata(path)?; + let properties = hashmap! { + "Value".to_owned() => RbxValue::String { + value: contents_str, + }, + }; - if meta.is_dir() { - return Ok(None); - } + let meta_path = path.with_file_name(format!("{}.meta.json", instance_name)); - let instance_name = match match_file_name(path, ".txt") { - Some(name) => name, - None => return Ok(None), - }; + let mut snapshot = InstanceSnapshot::new() + .name(instance_name) + .class_name("StringValue") + .properties(properties) + .metadata( + InstanceMetadata::new() + .instigating_source(path) + .relevant_paths(vec![path.to_path_buf(), meta_path.clone()]) + .context(context), + ); - let contents = vfs.read(path)?; - let contents_str = str::from_utf8(&contents) - .map_err(|err| SnapshotError::file_contents_bad_unicode(err, path))? - .to_string(); - - let properties = hashmap! { - "Value".to_owned() => RbxValue::String { - value: contents_str, - }, - }; - - let meta_path = path.with_file_name(format!("{}.meta.json", instance_name)); - - let mut snapshot = InstanceSnapshot::new() - .name(instance_name) - .class_name("StringValue") - .properties(properties) - .metadata( - InstanceMetadata::new() - .instigating_source(path) - .relevant_paths(vec![path.to_path_buf(), meta_path.clone()]) - .context(context), - ); - - if let Some(meta_contents) = vfs.read(&meta_path).with_not_found()? { - let mut metadata = AdjacentMetadata::from_slice(&meta_contents, &meta_path)?; - metadata.apply_all(&mut snapshot); - } - - Ok(Some(snapshot)) + if let Some(meta_contents) = vfs.read(&meta_path).with_not_found()? { + let mut metadata = AdjacentMetadata::from_slice(&meta_contents, &meta_path)?; + metadata.apply_all(&mut snapshot); } + + Ok(Some(snapshot)) } #[cfg(test)] @@ -75,10 +62,14 @@ mod test { let mut vfs = Vfs::new(imfs.clone()); - let instance_snapshot = - SnapshotTxt::from_vfs(&InstanceContext::default(), &mut vfs, Path::new("/foo.txt")) - .unwrap() - .unwrap(); + let instance_snapshot = snapshot_txt( + &InstanceContext::default(), + &mut vfs, + Path::new("/foo.txt"), + "foo", + ) + .unwrap() + .unwrap(); insta::assert_yaml_snapshot!(instance_snapshot); } diff --git a/src/snapshot_middleware/user_plugins.rs b/src/snapshot_middleware/user_plugins.rs deleted file mode 100644 index c28a7a76..00000000 --- a/src/snapshot_middleware/user_plugins.rs +++ /dev/null @@ -1,56 +0,0 @@ -use crate::{ - snapshot::InstanceContext, - vfs::{Vfs, VfsEntry, VfsFetcher}, -}; - -use super::middleware::{SnapshotInstanceResult, SnapshotMiddleware}; - -/// Handles snapshotting of any file that a user plugin wants to handle. -/// -/// User plugins are specified in the project file, but there are never user -/// plugins specified unless a Cargo feature is enabled, `user-plugins`. -/// Additionally, extra data needs to be set up inside the snapshot context -/// which is not currently wired up. -pub struct SnapshotUserPlugins; - -impl SnapshotMiddleware for SnapshotUserPlugins { - fn from_vfs( - _context: &InstanceContext, - _vfs: &Vfs, - _entry: &VfsEntry, - ) -> SnapshotInstanceResult { - // TODO: Invoke plugin here and get result out. - - // The current plan for plugins here is to make them work - // like Redux/Rodux middleware. A plugin will be a function - // that accepts the next middleware in the chain as a - // function and the snapshot subject (the VFS entry). - // - // Plugins can (but don't have to) invoke the next snapshot - // function and may or may not mutate the result. The hope - // is that this model enables the most flexibility possible - // for plugins to modify existing Rojo output, as well as - // generate new outputs. - // - // Open questions: - // * How will middleware be ordered? Does putting user - // middleware always at the beginning or always at the end - // of the chain reduce the scope of what that middleware - // can do? - // - // * Will plugins hurt Rojo's ability to parallelize - // snapshotting in the future? - // - // * Do the mutable handles to the Vfs and the snapshot - // context prevent plugins from invoking other plugins - // indirectly? - // - // * Will there be problems using a single Lua state because - // of re-entrancy? - // - // * Can the Lua <-> Rojo bindings used for middleware be - // reused for or from another project like Remodel? - - Ok(None) - } -}