Enhance patch computation to support glob-ignored children

- Introduced `glob_ignore_rules` to manage instance removal based on glob patterns, allowing for more granular control over which instances are retained.
- Updated the logic in `compute_patch_set_internal` and `compute_children_patches` to check against these rules before removing unmatched instances.
- Refactored related tests to ensure correct behavior when instances are matched or ignored based on their paths, improving the robustness of instance synchronization.
This commit is contained in:
Joe 2026-03-07 13:25:49 -06:00
parent 73ac94c67c
commit dad5320aaf

View file

@ -1,7 +1,7 @@
//! Defines the algorithm for computing a roughly-minimal patch set given an //! Defines the algorithm for computing a roughly-minimal patch set given an
//! existing instance tree and an instance snapshot. //! existing instance tree and an instance snapshot.
use std::{collections::HashMap, mem::take}; use std::{collections::HashMap, mem::take, sync::Arc};
use rbx_dom_weak::{ use rbx_dom_weak::{
types::{Ref, Variant}, types::{Ref, Variant},
@ -15,7 +15,7 @@ use crate::{
use super::{ use super::{
patch::{PatchAdd, PatchSet, PatchUpdate}, patch::{PatchAdd, PatchSet, PatchUpdate},
InstanceSnapshot, InstanceWithMeta, RojoTree, InstanceSnapshot, InstanceWithMeta, InstigatingSource, PathIgnoreRule, RojoTree,
}; };
#[profiling::function] #[profiling::function]
@ -110,7 +110,11 @@ fn compute_patch_set_internal(
.get_instance(id) .get_instance(id)
.expect("Instance did not exist in tree"); .expect("Instance did not exist in tree");
let suppress_removals = snapshot.metadata.glob_ignored_children; let glob_ignore_rules = if snapshot.metadata.glob_ignored_children {
Some(Arc::clone(&snapshot.metadata.context.path_ignore_rules))
} else {
None
};
compute_property_patches( compute_property_patches(
&context.snapshot_specified_ids, &context.snapshot_specified_ids,
@ -126,7 +130,7 @@ fn compute_patch_set_internal(
id, id,
patch_set, patch_set,
session, session,
suppress_removals, glob_ignore_rules.as_ref().map(|v| v.as_slice()),
); );
} }
@ -277,7 +281,7 @@ fn compute_children_patches(
id: Ref, id: Ref,
patch_set: &mut PatchSet, patch_set: &mut PatchSet,
session: &super::matching::MatchingSession, session: &super::matching::MatchingSession,
suppress_removals: bool, glob_ignore_rules: Option<&[PathIgnoreRule]>,
) { ) {
use super::matching::match_forward; use super::matching::match_forward;
@ -321,14 +325,27 @@ fn compute_children_patches(
}); });
} }
// Unmatched tree children: instances to be removed, unless the parent // Unmatched tree children: instances to be removed. When the parent has
// directory has glob-ignored children. Standard $ignoreUnknownInstances // glob-ignored children, each child is checked individually -- only
// (scripts-only mode) still allows server-side removals -- the plugin's // children whose InstigatingSource path matches a glob rule are kept.
// shouldDeleteChild handles the selective keep logic for non-scripts. // Standard $ignoreUnknownInstances (scripts-only mode) still allows
if !suppress_removals { // server-side removals; the plugin's shouldDeleteChild handles the
for tree_child_id in match_result.unmatched_tree { // selective keep logic for non-scripts.
patch_set.removed_instances.push(tree_child_id); for tree_child_id in match_result.unmatched_tree {
if let Some(rules) = glob_ignore_rules {
let is_glob_ignored = tree
.get_instance(tree_child_id)
.and_then(|inst| inst.metadata().instigating_source.as_ref())
.map(|source| match source {
InstigatingSource::Path(path) => !rules.iter().all(|rule| rule.passes(path)),
_ => false,
})
.unwrap_or(false);
if is_glob_ignored {
continue;
}
} }
patch_set.removed_instances.push(tree_child_id);
} }
} }
@ -443,8 +460,10 @@ mod test {
} }
#[test] #[test]
fn glob_ignored_children_suppresses_removals() { fn glob_ignored_children_suppresses_glob_matched_removals() {
use super::super::patch_apply::apply_patch_set; use super::super::patch_apply::apply_patch_set;
use crate::glob::Glob;
use std::path::PathBuf;
let mut tree = RojoTree::new(InstanceSnapshot::new().name("root").class_name("Folder")); let mut tree = RojoTree::new(InstanceSnapshot::new().name("root").class_name("Folder"));
let root_id = tree.get_root_id(); let root_id = tree.get_root_id();
@ -453,11 +472,23 @@ mod test {
added_instances: vec![ added_instances: vec![
PatchAdd { PatchAdd {
parent_id: root_id, parent_id: root_id,
instance: InstanceSnapshot::new().name("A").class_name("Folder"), instance: InstanceSnapshot::new()
.name("A")
.class_name("Folder")
.metadata(
super::super::InstanceMetadata::new()
.instigating_source(PathBuf::from("/project/src/A")),
),
}, },
PatchAdd { PatchAdd {
parent_id: root_id, parent_id: root_id,
instance: InstanceSnapshot::new().name("B").class_name("Folder"), instance: InstanceSnapshot::new()
.name("B")
.class_name("Folder")
.metadata(
super::super::InstanceMetadata::new()
.instigating_source(PathBuf::from("/project/src/generated/B")),
),
}, },
], ],
updated_instances: Vec::new(), updated_instances: Vec::new(),
@ -467,13 +498,20 @@ mod test {
}; };
apply_patch_set(&mut tree, add_patch); apply_patch_set(&mut tree, add_patch);
// Snapshot only has child A, with glob_ignored_children = true. let mut context = super::super::InstanceContext::new();
// Child B is "unknown" and should NOT be removed. context.add_path_ignore_rules(vec![PathIgnoreRule {
base_path: PathBuf::from("/project"),
glob: Glob::new("src/generated/**").unwrap(),
}]);
// Snapshot has only A. B is unmatched but its path matches the
// glob rule, so it should NOT be removed.
let snapshot = InstanceSnapshot { let snapshot = InstanceSnapshot {
snapshot_id: Ref::none(), snapshot_id: Ref::none(),
metadata: super::super::InstanceMetadata::new() metadata: super::super::InstanceMetadata::new()
.ignore_unknown_instances(true) .ignore_unknown_instances(true)
.glob_ignored_children(true), .glob_ignored_children(true)
.context(&context),
properties: UstrMap::new(), properties: UstrMap::new(),
name: Cow::Borrowed("root"), name: Cow::Borrowed("root"),
class_name: ustr("Folder"), class_name: ustr("Folder"),
@ -484,7 +522,95 @@ mod test {
assert!( assert!(
patch_set.removed_instances.is_empty(), patch_set.removed_instances.is_empty(),
"Unmatched tree children should NOT be removed when glob_ignored_children is true" "Glob-matched child should NOT be removed"
);
}
#[test]
fn glob_ignored_children_per_child_matching() {
use super::super::patch_apply::apply_patch_set;
use crate::glob::Glob;
use std::path::PathBuf;
let mut tree = RojoTree::new(InstanceSnapshot::new().name("root").class_name("Folder"));
let root_id = tree.get_root_id();
// A: will be in the snapshot (matched).
// B: deleted from filesystem, path does NOT match glob → removed.
// C: glob-ignored, path matches glob → NOT removed.
let add_patch = PatchSet {
added_instances: vec![
PatchAdd {
parent_id: root_id,
instance: InstanceSnapshot::new()
.name("A")
.class_name("Folder")
.metadata(
super::super::InstanceMetadata::new()
.instigating_source(PathBuf::from("/project/src/A")),
),
},
PatchAdd {
parent_id: root_id,
instance: InstanceSnapshot::new()
.name("B")
.class_name("Folder")
.metadata(
super::super::InstanceMetadata::new()
.instigating_source(PathBuf::from("/project/src/B")),
),
},
PatchAdd {
parent_id: root_id,
instance: InstanceSnapshot::new()
.name("C")
.class_name("Folder")
.metadata(
super::super::InstanceMetadata::new()
.instigating_source(PathBuf::from("/project/src/generated/C")),
),
},
],
updated_instances: Vec::new(),
removed_instances: Vec::new(),
stage_ids: std::collections::HashSet::new(),
stage_paths: Vec::new(),
};
apply_patch_set(&mut tree, add_patch);
let mut context = super::super::InstanceContext::new();
context.add_path_ignore_rules(vec![PathIgnoreRule {
base_path: PathBuf::from("/project"),
glob: Glob::new("src/generated/**").unwrap(),
}]);
let snapshot = InstanceSnapshot {
snapshot_id: Ref::none(),
metadata: super::super::InstanceMetadata::new()
.ignore_unknown_instances(true)
.glob_ignored_children(true)
.context(&context),
properties: UstrMap::new(),
name: Cow::Borrowed("root"),
class_name: ustr("Folder"),
children: vec![InstanceSnapshot::new().name("A").class_name("Folder")],
};
let patch_set = compute_patch_set(Some(snapshot), &tree, root_id);
assert_eq!(
patch_set.removed_instances.len(),
1,
"Only genuinely deleted child B should be removed, not glob-ignored child C"
);
let removed_inst = tree
.get_instance(patch_set.removed_instances[0])
.expect("Removed instance should exist in tree");
assert_eq!(
removed_inst.name(),
"B",
"The removed instance should be B (genuine deletion)"
); );
} }