Avoid sending spurious changedMetadata updates during sync

Closes #743
This commit is contained in:
Companion 2026-08-31 13:56:45 +01:00
parent a30a8ecd0f
commit 427e5fa370
16 changed files with 347 additions and 15 deletions

View file

@ -35,9 +35,11 @@ Making a new release? Simply add the new header with the version and date undern
* Fixed `rojo serve` silently stopping syncing file changes on Windows when the served project path was a verbatim (`\\?\`) path, because tree paths and file-watcher event paths were canonicalized to different forms. ([#1290])
* Fixed `rojo sourcemap --absolute` emitting verbatim (`\\?\`) paths on Windows, which broke require types in luau-lsp. ([#1290])
* The plugin now disables the `Check for Updates` setting if you block access to `api.github.com`. ([#1297])
* Fixed sync updates including `changedMetadata` when only Rojo's internal metadata changed. ([#1320])
[#1290]: https://github.com/rojo-rbx/rojo/pull/1290
[#1297]: https://github.com/rojo-rbx/rojo/pull/1297
[#1320]: https://github.com/rojo-rbx/rojo/pull/1320
## [7.7.0] (July 1st, 2026)

View file

@ -0,0 +1,18 @@
---
source: tests/tests/serve.rs
expression: "packet.intern_and_redact(&mut redactions, ())"
---
body:
messageCursor: 2
messages:
- added: {}
removed: []
updated:
- changedClassName: ~
changedMetadata:
ignoreUnknownInstances: false
changedName: ~
changedProperties: {}
id: id-3
packetType: messages
sessionId: id-1

View file

@ -0,0 +1,20 @@
---
source: tests/tests/serve.rs
expression: "packet.intern_and_redact(&mut redactions, ())"
---
body:
messageCursor: 3
messages:
- added: {}
removed: []
updated:
- changedClassName: ~
changedMetadata:
ignoreUnknownInstances: true
changedName: ~
changedProperties:
Anchored:
Bool: true
id: id-3
packetType: messages
sessionId: id-1

View file

@ -0,0 +1,19 @@
---
source: tests/tests/serve.rs
expression: "packet.intern_and_redact(&mut redactions, ())"
---
body:
messageCursor: 1
messages:
- added: {}
removed: []
updated:
- changedClassName: ~
changedMetadata: ~
changedName: ~
changedProperties:
Anchored:
Bool: false
id: id-3
packetType: messages
sessionId: id-1

View file

@ -0,0 +1,12 @@
---
source: tests/tests/serve.rs
expression: "packet.intern_and_redact(&mut redactions, ())"
---
body:
messageCursor: 4
messages:
- added: {}
removed: []
updated: []
packetType: messages
sessionId: id-1

View file

@ -18,12 +18,6 @@ body:
NeedsPivotMigration:
Bool: false
removed: []
updated:
- changedClassName: ~
changedMetadata:
ignoreUnknownInstances: true
changedName: ~
changedProperties: {}
id: id-3
updated: []
packetType: messages
sessionId: id-1

View file

@ -26,8 +26,7 @@ body:
removed: []
updated:
- changedClassName: ~
changedMetadata:
ignoreUnknownInstances: false
changedMetadata: ~
changedName: ~
changedProperties:
Attributes:

View file

@ -0,0 +1 @@
-- shared contents

View file

@ -0,0 +1 @@
-- shared contents

View file

@ -0,0 +1,18 @@
{
"name": "changed_metadata",
"tree": {
"$className": "DataModel",
"Workspace": {
"$className": "Workspace",
"Baseplate": {
"$className": "Part",
"$properties": {
"Anchored": true
}
},
"Thing": {
"$path": "a"
}
}
}
}

View file

@ -269,6 +269,39 @@ mod test {
use insta::internals::Content;
use std::path::Path;
#[test]
fn internal_metadata_change_still_invalidates_sourcemap() {
use crate::cli::sourcemap::{filter_nothing, patch_set_affects_sourcemap};
use crate::serve_session::ServeSession;
use crate::snapshot::{AppliedPatchSet, AppliedPatchUpdate, InstanceMetadata};
use memofs::Vfs;
let project_path = fs_err::canonicalize(
Path::new(env!("CARGO_MANIFEST_DIR"))
.join("test-projects")
.join("relative_paths")
.join("project"),
)
.unwrap();
let session = ServeSession::new(Vfs::new_default().unwrap(), project_path).unwrap();
let mut update = AppliedPatchUpdate::new(session.tree().get_root_id());
update.changed_metadata = Some(InstanceMetadata::new());
assert_eq!(update.changed_ignore_unknown_instances, None);
let patch_set = vec![AppliedPatchSet {
removed: Vec::new(),
added: Vec::new(),
updated: vec![update],
}];
assert!(patch_set_affects_sourcemap(
&session,
&patch_set,
filter_nothing
));
}
#[test]
fn maps_relative_paths() {
let sourcemap_dir = tempfile::tempdir().unwrap();

View file

@ -92,6 +92,9 @@ pub struct AppliedPatchUpdate {
pub changed_class_name: Option<Ustr>,
pub changed_properties: UstrMap<Option<Variant>>,
pub changed_metadata: Option<InstanceMetadata>,
#[serde(default, skip_serializing_if = "Option::is_none")]
pub changed_ignore_unknown_instances: Option<bool>,
}
impl AppliedPatchUpdate {
@ -102,6 +105,7 @@ impl AppliedPatchUpdate {
changed_class_name: None,
changed_properties: UstrMap::new(),
changed_metadata: None,
changed_ignore_unknown_instances: None,
}
}
}

View file

@ -175,6 +175,12 @@ fn apply_update_child(context: &mut PatchApplyContext, tree: &mut RojoTree, patc
let mut applied_patch = AppliedPatchUpdate::new(patch.id);
if let Some(metadata) = patch.changed_metadata {
applied_patch.changed_ignore_unknown_instances = tree
.get_instance(patch.id)
.map(|instance| instance.metadata().ignore_unknown_instances)
.filter(|previous| *previous != metadata.ignore_unknown_instances)
.map(|_| metadata.ignore_unknown_instances);
tree.update_metadata(patch.id, metadata.clone());
applied_patch.changed_metadata = Some(metadata);
}

View file

@ -3,8 +3,12 @@ use insta::assert_yaml_snapshot;
use rbx_dom_weak::{ustr, UstrMap};
use rojo_insta_ext::RedactionMap;
use std::path::PathBuf;
use crate::{
snapshot::{apply_patch_set, InstanceSnapshot, PatchSet, PatchUpdate, RojoTree},
snapshot::{
apply_patch_set, InstanceMetadata, InstanceSnapshot, PatchSet, PatchUpdate, RojoTree,
},
tree_view::{intern_tree, view_tree},
};
@ -101,6 +105,54 @@ fn remove_property() {
assert_yaml_snapshot!("remove_property_appied_patch", applied_patch_value);
}
#[test]
fn changed_ignore_unknown_instances_is_set_when_it_changes() {
let mut tree = empty_tree();
let id = tree.get_root_id();
let patch_set = PatchSet {
updated_instances: vec![PatchUpdate {
id,
changed_name: None,
changed_class_name: None,
changed_properties: Default::default(),
changed_metadata: Some(InstanceMetadata::new().ignore_unknown_instances(true)),
}],
..Default::default()
};
let applied_patch_set = apply_patch_set(&mut tree, patch_set);
let update = &applied_patch_set.updated[0];
assert_eq!(update.changed_ignore_unknown_instances, Some(true));
assert!(update.changed_metadata.is_some());
}
#[test]
fn changed_ignore_unknown_instances_is_unset_when_only_internal_metadata_changes() {
let mut tree = empty_tree();
let id = tree.get_root_id();
let patch_set = PatchSet {
updated_instances: vec![PatchUpdate {
id,
changed_name: None,
changed_class_name: None,
changed_properties: Default::default(),
changed_metadata: Some(
InstanceMetadata::new().relevant_paths(vec![PathBuf::from("some/new/path.lua")]),
),
}],
..Default::default()
};
let applied_patch_set = apply_patch_set(&mut tree, patch_set);
let update = &applied_patch_set.updated[0];
assert_eq!(update.changed_ignore_unknown_instances, None);
assert!(update.changed_metadata.is_some());
}
fn empty_tree() -> RojoTree {
RojoTree::new(InstanceSnapshot::new().name("ROOT").class_name("ROOT"))
}

View file

@ -54,12 +54,14 @@ impl<'a> SubscribeMessage<'a> {
.updated
.into_iter()
.map(|update| {
let changed_metadata = update
.changed_metadata
.as_ref()
.map(InstanceMetadata::from_rojo_metadata);
let changed_metadata =
update
.changed_ignore_unknown_instances
.map(|ignore_unknown_instances| InstanceMetadata {
ignore_unknown_instances,
});
let changed_properties = update
let changed_properties: UstrMap<Option<Variant>> = update
.changed_properties
.into_iter()
.filter(|(_key, value)| property_filter(value.as_ref()))
@ -73,6 +75,12 @@ impl<'a> SubscribeMessage<'a> {
changed_metadata,
}
})
.filter(|update| {
update.changed_name.is_some()
|| update.changed_class_name.is_some()
|| !update.changed_properties.is_empty()
|| update.changed_metadata.is_some()
})
.collect();
Self {
@ -312,3 +320,71 @@ pub enum ErrorResponseKind {
Forbidden,
InternalError,
}
#[cfg(test)]
mod tests {
use super::*;
use rbx_dom_weak::ustr;
use crate::snapshot::{AppliedPatchUpdate, InstanceSnapshot};
fn empty_tree() -> RojoTree {
RojoTree::new(InstanceSnapshot::new().name("ROOT").class_name("ROOT"))
}
fn update_for(update: AppliedPatchUpdate) -> Vec<InstanceUpdate> {
let patch = AppliedPatchSet {
removed: Vec::new(),
added: Vec::new(),
updated: vec![update],
};
SubscribeMessage::from_patch_update(&empty_tree(), patch).updated
}
#[test]
fn name_only_update_is_kept() {
let mut update = AppliedPatchUpdate::new(Ref::none());
update.changed_name = Some("Renamed".to_owned());
let updated = update_for(update);
assert_eq!(updated.len(), 1);
assert_eq!(updated[0].changed_name.as_deref(), Some("Renamed"));
}
#[test]
fn class_name_only_update_is_kept() {
let mut update = AppliedPatchUpdate::new(Ref::none());
update.changed_class_name = Some(ustr("Folder"));
let updated = update_for(update);
assert_eq!(updated.len(), 1);
assert_eq!(updated[0].changed_class_name, Some(ustr("Folder")));
}
#[test]
fn ignore_unknown_instances_change_is_reported() {
let mut update = AppliedPatchUpdate::new(Ref::none());
update.changed_metadata = Some(RojoInstanceMetadata::new().ignore_unknown_instances(true));
update.changed_ignore_unknown_instances = Some(true);
let updated = update_for(update);
assert_eq!(updated.len(), 1);
assert_eq!(
updated[0]
.changed_metadata
.as_ref()
.map(|metadata| metadata.ignore_unknown_instances),
Some(true)
);
}
#[test]
fn internal_metadata_only_update_is_dropped() {
let mut update = AppliedPatchUpdate::new(Ref::none());
update.changed_metadata = Some(RojoInstanceMetadata::new());
assert!(update_for(update).is_empty());
}
}

View file

@ -775,3 +775,80 @@ fn forced_parent() {
assert_snapshot!("forced_parent_serialize_model", model);
});
}
#[test]
fn changed_metadata() {
run_serve_test("changed_metadata", |session, mut redactions| {
let _ = session.get_api_rojo().unwrap();
let project = |anchored: bool, ignore_unknown: Option<bool>, path: &str| {
let ignore_unknown = match ignore_unknown {
Some(value) => format!("\"$ignoreUnknownInstances\": {value},"),
None => String::new(),
};
format!(
r#"{{
"name": "changed_metadata",
"tree": {{
"$className": "DataModel",
"Workspace": {{
"$className": "Workspace",
"Baseplate": {{
"$className": "Part",
{ignore_unknown}
"$properties": {{
"Anchored": {anchored}
}}
}},
"Thing": {{
"$path": "{path}"
}}
}}
}}
}}
"#
)
};
let write_project = |contents: String| {
fs::write(session.path().join("default.project.json"), contents).unwrap();
};
write_project(project(false, None, "a"));
let packet = session
.get_api_socket_packet(SocketPacketType::Messages, 0)
.unwrap();
assert_yaml_snapshot!(
"changed_metadata_property_only",
packet.intern_and_redact(&mut redactions, ())
);
write_project(project(false, Some(false), "a"));
let packet = session
.get_api_socket_packet(SocketPacketType::Messages, 1)
.unwrap();
assert_yaml_snapshot!(
"changed_metadata_metadata_only",
packet.intern_and_redact(&mut redactions, ())
);
write_project(project(true, Some(true), "a"));
let packet = session
.get_api_socket_packet(SocketPacketType::Messages, 2)
.unwrap();
assert_yaml_snapshot!(
"changed_metadata_property_and_metadata",
packet.intern_and_redact(&mut redactions, ())
);
write_project(project(true, Some(true), "b"));
let packet = session
.get_api_socket_packet(SocketPacketType::Messages, 3)
.unwrap();
assert_yaml_snapshot!(
"changed_metadata_relevant_paths_only",
packet.intern_and_redact(&mut redactions, ())
);
});
}