Move instigating source out of contributing paths (#253)

* Refactor contributing_paths into contributing_sources, deleting project node sources

* Instead of changing contributing_paths, add instigating_source

* Remove InstanceMetadata::project_node

* Stop pushing project path to front of contributing_paths since it doesn't matter now

* Remove accidental UI change for path display

src/change_processor.rs

@@ -11,7 +11,7 @@ use jod_thread::JoinHandle;
 use crate::{
     imfs::{Imfs, ImfsEvent, ImfsFetcher},
     message_queue::MessageQueue,
-    snapshot::{apply_patch_set, compute_patch_set, AppliedPatchSet, RojoTree},
+    snapshot::{apply_patch_set, compute_patch_set, AppliedPatchSet, InstigatingSource, RojoTree},
     snapshot_middleware::snapshot_from_imfs,
 };
 
@@ -84,22 +84,32 @@ impl ChangeProcessor {
                                     let metadata = tree.get_metadata(id)
                                         .expect("metadata missing for instance present in tree");
 
-                                    let instigating_path = match metadata.contributing_paths.get(0) {
+                                    let instigating_source = match &metadata.instigating_source {
                                         Some(path) => path,
                                         None => {
-                                            log::warn!("Instance {} did not have an instigating path, but was considered for an update.", id);
+                                            log::warn!("Instance {} did not have an instigating source, but was considered for an update.", id);
                                             log::warn!("This is a Rojo bug. Please file an issue!");
                                             continue;
                                         }
                                     };
 
-                                    let entry = imfs
-                                        .get(instigating_path)
-                                        .expect("could not get instigating path from filesystem");
+                                    let snapshot = match instigating_source {
+                                        InstigatingSource::Path(instigating_path) => {
+                                            let entry = imfs
+                                                .get(instigating_path)
+                                                .expect("could not get instigating path from filesystem");
 
-                                    let snapshot = snapshot_from_imfs(&mut imfs, &entry)
-                                        .expect("snapshot failed")
-                                        .expect("snapshot did not return an instance");
+                                            let snapshot = snapshot_from_imfs(&mut imfs, &entry)
+                                                .expect("snapshot failed")
+                                                .expect("snapshot did not return an instance");
+
+                                            snapshot
+                                        }
+                                        InstigatingSource::ProjectNode(_, _) => {
+                                            log::warn!("Instance {} had an instigating source that was a project node, which is not yet supported.", id);
+                                            continue;
+                                        }
+                                    };
 
                                     log::trace!("Computed snapshot: {:#?}", snapshot);
 

src/snapshot/metadata.rs

@@ -1,4 +1,4 @@
-use std::path::PathBuf;
+use std::{fmt, path::PathBuf};
 
 use serde::{Deserialize, Serialize};
 
@@ -13,48 +13,64 @@ pub struct InstanceMetadata {
     /// manage.
     pub ignore_unknown_instances: bool,
 
+    /// If a change occurs to this instance, the instigating source is what
+    /// should be run through the snapshot functions to regenerate it.
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub instigating_source: Option<InstigatingSource>,
+
     /// The paths that, when changed, could cause the function that generated
     /// this snapshot to generate a different snapshot. Paths should be included
     /// even if they don't exist, since the presence of a file can change the
     /// outcome of a snapshot function.
     ///
-    /// The first path in this  list is considered the "instigating path", and
-    /// will be the snapshot target if any of the contributing paths change.
-    ///
     /// For example, a file named foo.lua might have these contributing paths:
-    /// - foo.lua (instigating path)
+    /// - foo.lua
     /// - foo.meta.json (even if this file doesn't exist!)
     ///
-    /// A directory named bar/ included in the project file might have these:
-    /// - bar/ (instigating path)
+    /// A directory named bar/ might have these:
+    /// - bar/
     /// - bar/init.meta.json
     /// - bar/init.lua
     /// - bar/init.server.lua
     /// - bar/init.client.lua
-    /// - default.project.json
+    /// - bar/default.project.json
     ///
     /// This path is used to make sure that file changes update all instances
     /// that may need updates.
     // TODO: Change this to be a SmallVec for performance in common cases?
     #[serde(serialize_with = "path_serializer::serialize_vec_absolute")]
     pub contributing_paths: Vec<PathBuf>,
-
-    /// If this instance was defined in a project file, this is the name from
-    /// the project file and the node under it.
-    ///
-    /// This information is used to make sure the instance has the correct name,
-    /// project-added children, and metadata when it's updated in response to a
-    /// file change.
-    #[serde(skip_serializing_if = "Option::is_none")]
-    pub project_node: Option<(String, ProjectNode)>,
 }
 
 impl Default for InstanceMetadata {
     fn default() -> Self {
         InstanceMetadata {
             ignore_unknown_instances: false,
+            instigating_source: None,
             contributing_paths: Vec::new(),
-            project_node: None,
         }
     }
 }
+
+#[derive(Clone, PartialEq, Serialize, Deserialize)]
+pub enum InstigatingSource {
+    Path(#[serde(serialize_with = "path_serializer::serialize_absolute")] PathBuf),
+    ProjectNode(String, ProjectNode),
+}
+
+impl fmt::Debug for InstigatingSource {
+    fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self {
+            InstigatingSource::Path(path) => write!(formatter, "Path({})", path.display()),
+            InstigatingSource::ProjectNode(name, node) => {
+                write!(formatter, "ProjectNode({}: {:?}", name, node)
+            }
+        }
+    }
+}
+
+impl From<PathBuf> for InstigatingSource {
+    fn from(path: PathBuf) -> Self {
+        InstigatingSource::Path(path)
+    }
+}

src/snapshot/mod.rs

@@ -56,7 +56,7 @@ mod patch_compute;
 mod tree;
 
 pub use instance_snapshot::InstanceSnapshot;
-pub use metadata::InstanceMetadata;
+pub use metadata::*;
 pub use patch::*;
 pub use patch_apply::apply_patch_set;
 pub use patch_compute::compute_patch_set;

src/snapshot_middleware/project.rs

@@ -63,7 +63,7 @@ impl SnapshotMiddleware for SnapshotProject {
         snapshot
             .metadata
             .contributing_paths
-            .insert(0, entry.path().to_path_buf());
+            .push(entry.path().to_path_buf());
 
         Ok(Some(snapshot))
     }

src/snapshot_middleware/snapshots/test__project_path_property_overrides.snap

@@ -6,8 +6,8 @@ snapshot_id: ~
 metadata:
   ignore_unknown_instances: true
   contributing_paths:
-    - /foo/default.project.json
     - /foo/other.project.json
+    - /foo/default.project.json
 name: path-property-override
 class_name: StringValue
 properties:

src/snapshot_middleware/snapshots/test__project_with_path_to_project.snap

@@ -6,8 +6,8 @@ snapshot_id: ~
 metadata:
   ignore_unknown_instances: true
   contributing_paths:
-    - /foo/default.project.json
     - /foo/other.project.json
+    - /foo/default.project.json
 name: path-project
 class_name: Model
 properties: {}

src/snapshot_middleware/snapshots/test__project_with_path_to_project_with_children.snap

@@ -6,8 +6,8 @@ snapshot_id: ~
 metadata:
   ignore_unknown_instances: true
   contributing_paths:
-    - /foo/default.project.json
     - /foo/other.project.json
+    - /foo/default.project.json
 name: path-child-project
 class_name: Folder
 properties: {}

src/snapshot_middleware/snapshots/test__project_with_path_to_txt.snap

@@ -6,8 +6,8 @@ snapshot_id: ~
 metadata:
   ignore_unknown_instances: false
   contributing_paths:
-    - /foo/default.project.json
     - /foo/other.txt
+    - /foo/default.project.json
 name: path-project
 class_name: StringValue
 properties:

src/web/ui.rs

@@ -264,18 +264,11 @@ impl<F: ImfsFetcher> UiService<F> {
                 }
             };
 
-            let project_node = match &metadata.project_node {
-                None => HtmlContent::None,
-                Some(node) => html! {
-                    <div>"project node: " { format!("{:?}", node) }</div>
-                },
-            };
-
             let content = html! {
                 <>
                     <div>"ignore_unknown_instances: " { metadata.ignore_unknown_instances.to_string() }</div>
+                    <div>"instigating source: " { format!("{:?}", metadata.instigating_source) }</div>
                     { contributing_paths }
-                    { project_node }
                 </>
             };