Refactor instance metadata handling to incorporate glob-ignored children

- Renamed `ignore_unknown_instances` to `suppress_removals` in patch computation logic for clarity.
- Updated tests to reflect the new naming and behavior, ensuring correct handling of instance removals based on `glob_ignored_children`.
- Enhanced API and interface definitions to include `glob_ignored_children`, improving metadata management for instance synchronization.
This commit is contained in:
Joe 2026-03-07 12:02:34 -06:00
parent 12bccf91c5
commit 1406c1a1b7
3 changed files with 66 additions and 15 deletions

View file

@ -110,7 +110,7 @@ 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 ignore_unknown = snapshot.metadata.ignore_unknown_instances; let suppress_removals = snapshot.metadata.glob_ignored_children;
compute_property_patches( compute_property_patches(
&context.snapshot_specified_ids, &context.snapshot_specified_ids,
@ -126,7 +126,7 @@ fn compute_patch_set_internal(
id, id,
patch_set, patch_set,
session, session,
ignore_unknown, suppress_removals,
); );
} }
@ -277,7 +277,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,
ignore_unknown_instances: bool, suppress_removals: bool,
) { ) {
use super::matching::match_forward; use super::matching::match_forward;
@ -322,9 +322,10 @@ fn compute_children_patches(
} }
// Unmatched tree children: instances to be removed, unless the parent // Unmatched tree children: instances to be removed, unless the parent
// snapshot says to ignore unknown instances (e.g. $ignoreUnknownInstances // directory has glob-ignored children. Standard $ignoreUnknownInstances
// or directories with glob-ignored children). // (scripts-only mode) still allows server-side removals -- the plugin's
if !ignore_unknown_instances { // shouldDeleteChild handles the selective keep logic for non-scripts.
if !suppress_removals {
for tree_child_id in match_result.unmatched_tree { for tree_child_id in match_result.unmatched_tree {
patch_set.removed_instances.push(tree_child_id); patch_set.removed_instances.push(tree_child_id);
} }
@ -442,13 +443,12 @@ mod test {
} }
#[test] #[test]
fn ignore_unknown_instances_suppresses_removals() { fn glob_ignored_children_suppresses_removals() {
use super::super::patch_apply::apply_patch_set; use super::super::patch_apply::apply_patch_set;
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();
// Add two children (A and B) to the tree via a patch.
let add_patch = PatchSet { let add_patch = PatchSet {
added_instances: vec![ added_instances: vec![
PatchAdd { PatchAdd {
@ -467,8 +467,55 @@ mod test {
}; };
apply_patch_set(&mut tree, add_patch); apply_patch_set(&mut tree, add_patch);
// Snapshot only has child A, with ignore_unknown_instances = true. // Snapshot only has child A, with glob_ignored_children = true.
// Child B is "unknown" and should NOT be removed. // Child B is "unknown" and should NOT be removed.
let snapshot = InstanceSnapshot {
snapshot_id: Ref::none(),
metadata: super::super::InstanceMetadata::new()
.ignore_unknown_instances(true)
.glob_ignored_children(true),
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!(
patch_set.removed_instances.is_empty(),
"Unmatched tree children should NOT be removed when glob_ignored_children is true"
);
}
#[test]
fn ignore_unknown_instances_alone_still_removes() {
use super::super::patch_apply::apply_patch_set;
let mut tree = RojoTree::new(InstanceSnapshot::new().name("root").class_name("Folder"));
let root_id = tree.get_root_id();
let add_patch = PatchSet {
added_instances: vec![
PatchAdd {
parent_id: root_id,
instance: InstanceSnapshot::new().name("A").class_name("Folder"),
},
PatchAdd {
parent_id: root_id,
instance: InstanceSnapshot::new().name("B").class_name("Folder"),
},
],
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);
// ignore_unknown_instances = true (scripts-only mode) but
// glob_ignored_children = false. Server still sends removals;
// the plugin's shouldDeleteChild handles the selective keep.
let snapshot = InstanceSnapshot { let snapshot = InstanceSnapshot {
snapshot_id: Ref::none(), snapshot_id: Ref::none(),
metadata: super::super::InstanceMetadata::new().ignore_unknown_instances(true), metadata: super::super::InstanceMetadata::new().ignore_unknown_instances(true),
@ -480,9 +527,10 @@ mod test {
let patch_set = compute_patch_set(Some(snapshot), &tree, root_id); let patch_set = compute_patch_set(Some(snapshot), &tree, root_id);
assert!( assert_eq!(
patch_set.removed_instances.is_empty(), patch_set.removed_instances.len(),
"Unmatched tree children should NOT be removed when ignore_unknown_instances is true" 1,
"ignore_unknown_instances alone should NOT suppress server-side removals"
); );
} }
@ -511,8 +559,6 @@ mod test {
}; };
apply_patch_set(&mut tree, add_patch); apply_patch_set(&mut tree, add_patch);
// Snapshot only has child A, with default metadata (ignore_unknown_instances = false).
// Child B should be removed.
let snapshot = InstanceSnapshot { let snapshot = InstanceSnapshot {
snapshot_id: Ref::none(), snapshot_id: Ref::none(),
metadata: Default::default(), metadata: Default::default(),
@ -527,7 +573,7 @@ mod test {
assert_eq!( assert_eq!(
patch_set.removed_instances.len(), patch_set.removed_instances.len(),
1, 1,
"Unmatched tree children should be removed when ignore_unknown_instances is false" "Unmatched tree children should be removed with default metadata"
); );
} }

View file

@ -4317,6 +4317,7 @@ fn filter_subscribe_message_for_scripts<'a>(
for inst in msg.added.values_mut() { for inst in msg.added.values_mut() {
inst.metadata = Some(InstanceMetadata { inst.metadata = Some(InstanceMetadata {
ignore_unknown_instances: true, ignore_unknown_instances: true,
glob_ignored_children: false,
}); });
// Non-scripts don't sync properties - they're just tree structure // Non-scripts don't sync properties - they're just tree structure
@ -4384,6 +4385,7 @@ fn instance_for_scripts_only<'a>(
// This ensures we only sync script content, never delete/modify other instances // This ensures we only sync script content, never delete/modify other instances
let metadata = Some(InstanceMetadata { let metadata = Some(InstanceMetadata {
ignore_unknown_instances: true, ignore_unknown_instances: true,
glob_ignored_children: false,
}); });
Instance { Instance {

View file

@ -155,12 +155,15 @@ pub struct InstanceUpdate {
#[serde(rename_all = "camelCase")] #[serde(rename_all = "camelCase")]
pub struct InstanceMetadata { pub struct InstanceMetadata {
pub ignore_unknown_instances: bool, pub ignore_unknown_instances: bool,
#[serde(default, skip_serializing_if = "std::ops::Not::not")]
pub glob_ignored_children: bool,
} }
impl InstanceMetadata { impl InstanceMetadata {
pub(crate) fn from_rojo_metadata(meta: &RojoInstanceMetadata) -> Self { pub(crate) fn from_rojo_metadata(meta: &RojoInstanceMetadata) -> Self {
Self { Self {
ignore_unknown_instances: meta.ignore_unknown_instances, ignore_unknown_instances: meta.ignore_unknown_instances,
glob_ignored_children: meta.glob_ignored_children,
} }
} }
} }