mirror of
https://github.com/rojo-rbx/rojo.git
synced 2026-10-24 15:01:38 +00:00
Add support for filtering hidden services in syncback
- Introduced a new configuration option `ignore_hidden_services` in `SyncbackRules` to allow filtering of internal services during syncback. - Added a constant `VISIBLE_SERVICES` to define which services are considered visible and should be included when the filtering option is enabled. - Implemented the `strip_hidden_services` function to remove non-visible services from the syncback process. - Updated the syncback loop to apply the filtering logic based on the new configuration option. - Enhanced path uniqueness checks by pre-computing duplicate sibling information for improved performance in the syncback process.
This commit is contained in:
parent
5a85c5145c
commit
9b412d28ae
3 changed files with 220 additions and 63 deletions
|
|
@ -44,6 +44,26 @@ pub use snapshot::{inst_path, SyncbackData, SyncbackSnapshot};
|
||||||
/// new files.
|
/// new files.
|
||||||
const DEBUG_MODEL_FORMAT_VAR: &str = "ROJO_SYNCBACK_DEBUG";
|
const DEBUG_MODEL_FORMAT_VAR: &str = "ROJO_SYNCBACK_DEBUG";
|
||||||
|
|
||||||
|
/// Services that are considered "visible" and will be included when
|
||||||
|
/// `ignoreHiddenServices` is enabled. All other services will be ignored.
|
||||||
|
const VISIBLE_SERVICES: &[&str] = &[
|
||||||
|
"Lighting",
|
||||||
|
"MaterialService",
|
||||||
|
"NetworkClient",
|
||||||
|
"ReplicatedFirst",
|
||||||
|
"ReplicatedStorage",
|
||||||
|
"ServerScriptService",
|
||||||
|
"ServerStorage",
|
||||||
|
"SoundService",
|
||||||
|
"StarterGui",
|
||||||
|
"StarterPack",
|
||||||
|
"StarterPlayer",
|
||||||
|
"Teams",
|
||||||
|
"TextChatService",
|
||||||
|
"VoiceChatService",
|
||||||
|
"Workspace",
|
||||||
|
];
|
||||||
|
|
||||||
/// A glob that can be used to tell if a path contains a `.git` folder.
|
/// A glob that can be used to tell if a path contains a `.git` folder.
|
||||||
static GIT_IGNORE_GLOB: OnceLock<Glob> = OnceLock::new();
|
static GIT_IGNORE_GLOB: OnceLock<Glob> = OnceLock::new();
|
||||||
|
|
||||||
|
|
@ -91,6 +111,17 @@ pub fn syncback_loop(
|
||||||
strip_unknown_root_children(&mut new_tree, old_tree);
|
strip_unknown_root_children(&mut new_tree, old_tree);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// If ignoreHiddenServices is enabled, filter to only visible services
|
||||||
|
if project
|
||||||
|
.syncback_rules
|
||||||
|
.as_ref()
|
||||||
|
.map(|rules| rules.ignore_hidden_services())
|
||||||
|
.unwrap_or(false)
|
||||||
|
{
|
||||||
|
log::debug!("Filtering hidden services");
|
||||||
|
strip_hidden_services(&mut new_tree);
|
||||||
|
}
|
||||||
|
|
||||||
log::debug!("Collecting referents for new DOM...");
|
log::debug!("Collecting referents for new DOM...");
|
||||||
let deferred_referents = collect_referents(&new_tree, &pre_prune_paths);
|
let deferred_referents = collect_referents(&new_tree, &pre_prune_paths);
|
||||||
|
|
||||||
|
|
@ -408,6 +439,12 @@ pub struct SyncbackRules {
|
||||||
/// Defaults to `false`.
|
/// Defaults to `false`.
|
||||||
#[serde(skip_serializing_if = "Option::is_none")]
|
#[serde(skip_serializing_if = "Option::is_none")]
|
||||||
encode_windows_invalid_chars: Option<bool>,
|
encode_windows_invalid_chars: Option<bool>,
|
||||||
|
/// When enabled, only "visible" services will be synced back. This includes
|
||||||
|
/// commonly used services like Workspace, ReplicatedStorage, ServerScriptService,
|
||||||
|
/// etc., while ignoring internal/hidden services like Chat, HttpService, etc.
|
||||||
|
/// Defaults to `false`.
|
||||||
|
#[serde(skip_serializing_if = "Option::is_none")]
|
||||||
|
ignore_hidden_services: Option<bool>,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl SyncbackRules {
|
impl SyncbackRules {
|
||||||
|
|
@ -453,6 +490,14 @@ impl SyncbackRules {
|
||||||
pub fn encode_windows_invalid_chars(&self) -> bool {
|
pub fn encode_windows_invalid_chars(&self) -> bool {
|
||||||
self.encode_windows_invalid_chars.unwrap_or(true)
|
self.encode_windows_invalid_chars.unwrap_or(true)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Returns whether hidden/internal services should be ignored during
|
||||||
|
/// syncback. When `true`, only visible services like Workspace,
|
||||||
|
/// ReplicatedStorage, etc. will be synced. Defaults to `false`.
|
||||||
|
#[inline]
|
||||||
|
pub fn ignore_hidden_services(&self) -> bool {
|
||||||
|
self.ignore_hidden_services.unwrap_or(false)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
fn is_valid_path(globs: &Option<Vec<Glob>>, base_path: &Path, path: &Path) -> bool {
|
fn is_valid_path(globs: &Option<Vec<Glob>>, base_path: &Path, path: &Path) -> bool {
|
||||||
|
|
@ -524,6 +569,29 @@ fn descendants(dom: &WeakDom, root_ref: Ref) -> Vec<Ref> {
|
||||||
ordered
|
ordered
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Removes root children (services) that are not in the `VISIBLE_SERVICES` list.
|
||||||
|
/// This is used when `ignoreHiddenServices` is enabled to filter out internal
|
||||||
|
/// services like Chat, HttpService, etc.
|
||||||
|
fn strip_hidden_services(dom: &mut WeakDom) {
|
||||||
|
let root_children = dom.root().children().to_vec();
|
||||||
|
|
||||||
|
for child_ref in root_children {
|
||||||
|
let child = dom
|
||||||
|
.get_by_ref(child_ref)
|
||||||
|
.expect("all children of the root should exist in the DOM");
|
||||||
|
|
||||||
|
// Check if this service is in the visible services list
|
||||||
|
if !VISIBLE_SERVICES.contains(&child.name.as_str()) {
|
||||||
|
log::trace!(
|
||||||
|
"Pruning hidden service {} of class {}",
|
||||||
|
child.name,
|
||||||
|
child.class
|
||||||
|
);
|
||||||
|
dom.destroy(child_ref);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/// Removes the children of `new`'s root that are not also children of `old`'s
|
/// Removes the children of `new`'s root that are not also children of `old`'s
|
||||||
/// root.
|
/// root.
|
||||||
///
|
///
|
||||||
|
|
|
||||||
|
|
@ -51,13 +51,60 @@ pub fn collect_all_paths(dom: &WeakDom) -> HashMap<Ref, String> {
|
||||||
paths
|
paths
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Checks if a path is unique in the DOM by verifying no duplicates exist
|
/// Pre-computes which instances have duplicate-named siblings.
|
||||||
/// at ANY level of the path (target, parent, grandparent, etc.).
|
/// Returns a HashSet of Refs that have at least one sibling with the same name.
|
||||||
fn is_path_unique(dom: &WeakDom, target_ref: Ref) -> bool {
|
///
|
||||||
|
/// This is O(N) where N is the number of instances, and allows subsequent
|
||||||
|
/// path uniqueness checks to be O(d) instead of O(d × s) where d=depth, s=siblings.
|
||||||
|
fn compute_refs_with_duplicate_siblings(dom: &WeakDom) -> HashSet<Ref> {
|
||||||
|
let mut has_duplicate_siblings = HashSet::new();
|
||||||
|
let mut queue = VecDeque::new();
|
||||||
|
queue.push_back(dom.root_ref());
|
||||||
|
|
||||||
|
while let Some(inst_ref) = queue.pop_front() {
|
||||||
|
let inst = match dom.get_by_ref(inst_ref) {
|
||||||
|
Some(i) => i,
|
||||||
|
None => continue,
|
||||||
|
};
|
||||||
|
|
||||||
|
// Count children by name and collect their refs
|
||||||
|
let mut name_to_refs: HashMap<&str, Vec<Ref>> = HashMap::new();
|
||||||
|
for child_ref in inst.children() {
|
||||||
|
if let Some(child) = dom.get_by_ref(*child_ref) {
|
||||||
|
name_to_refs.entry(&child.name).or_default().push(*child_ref);
|
||||||
|
}
|
||||||
|
queue.push_back(*child_ref);
|
||||||
|
}
|
||||||
|
|
||||||
|
// Mark refs that share a name with siblings
|
||||||
|
for (_name, refs) in name_to_refs {
|
||||||
|
if refs.len() > 1 {
|
||||||
|
for r in refs {
|
||||||
|
has_duplicate_siblings.insert(r);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
has_duplicate_siblings
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Checks if a path is unique using pre-computed duplicate sibling info.
|
||||||
|
/// This is O(d) where d is the depth of the instance.
|
||||||
|
fn is_path_unique_with_cache(
|
||||||
|
dom: &WeakDom,
|
||||||
|
target_ref: Ref,
|
||||||
|
has_duplicate_siblings: &HashSet<Ref>,
|
||||||
|
) -> bool {
|
||||||
let mut current_ref = target_ref;
|
let mut current_ref = target_ref;
|
||||||
|
|
||||||
// Walk up the tree checking each level for duplicate names among siblings
|
// Walk up the tree checking each level using the pre-computed cache
|
||||||
loop {
|
loop {
|
||||||
|
// O(1) lookup instead of O(siblings) counting
|
||||||
|
if has_duplicate_siblings.contains(¤t_ref) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
let current = match dom.get_by_ref(current_ref) {
|
let current = match dom.get_by_ref(current_ref) {
|
||||||
Some(inst) => inst,
|
Some(inst) => inst,
|
||||||
None => return false,
|
None => return false,
|
||||||
|
|
@ -69,26 +116,6 @@ fn is_path_unique(dom: &WeakDom, target_ref: Ref) -> bool {
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
|
|
||||||
let parent = match dom.get_by_ref(parent_ref) {
|
|
||||||
Some(inst) => inst,
|
|
||||||
None => return true,
|
|
||||||
};
|
|
||||||
|
|
||||||
// Check if any sibling has the same name as current
|
|
||||||
let current_name = ¤t.name;
|
|
||||||
let mut count = 0;
|
|
||||||
for child_ref in parent.children() {
|
|
||||||
if let Some(child) = dom.get_by_ref(*child_ref) {
|
|
||||||
if &child.name == current_name {
|
|
||||||
count += 1;
|
|
||||||
if count > 1 {
|
|
||||||
// Duplicate found at this level - path is not unique
|
|
||||||
return false;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
// Move up to parent and check the next level
|
// Move up to parent and check the next level
|
||||||
current_ref = parent_ref;
|
current_ref = parent_ref;
|
||||||
}
|
}
|
||||||
|
|
@ -105,6 +132,13 @@ pub fn collect_referents(dom: &WeakDom, pre_prune_paths: &HashMap<Ref, String>)
|
||||||
let mut id_links: HashMap<Ref, Vec<IdRefLink>> = HashMap::new();
|
let mut id_links: HashMap<Ref, Vec<IdRefLink>> = HashMap::new();
|
||||||
let mut targets_needing_id: HashSet<Ref> = HashSet::new();
|
let mut targets_needing_id: HashSet<Ref> = HashSet::new();
|
||||||
|
|
||||||
|
// Pre-compute duplicate sibling info in O(N) - this makes all subsequent
|
||||||
|
// path uniqueness checks O(d) instead of O(d × s) where d=depth, s=siblings
|
||||||
|
let has_duplicate_siblings = compute_refs_with_duplicate_siblings(dom);
|
||||||
|
|
||||||
|
// Cache path uniqueness results to avoid re-checking the same target
|
||||||
|
let mut path_unique_cache: HashMap<Ref, bool> = HashMap::new();
|
||||||
|
|
||||||
let mut queue = VecDeque::new();
|
let mut queue = VecDeque::new();
|
||||||
queue.push_back(dom.root_ref());
|
queue.push_back(dom.root_ref());
|
||||||
|
|
||||||
|
|
@ -122,8 +156,12 @@ pub fn collect_referents(dom: &WeakDom, pre_prune_paths: &HashMap<Ref, String>)
|
||||||
|
|
||||||
// Check if target exists in current DOM
|
// Check if target exists in current DOM
|
||||||
if let Some(_target) = dom.get_by_ref(*target_ref) {
|
if let Some(_target) = dom.get_by_ref(*target_ref) {
|
||||||
// Target exists - check if path is unique
|
// Target exists - check if path is unique (with caching)
|
||||||
if is_path_unique(dom, *target_ref) {
|
let is_unique = *path_unique_cache
|
||||||
|
.entry(*target_ref)
|
||||||
|
.or_insert_with(|| is_path_unique_with_cache(dom, *target_ref, &has_duplicate_siblings));
|
||||||
|
|
||||||
|
if is_unique {
|
||||||
// Path is unique - use path-based system
|
// Path is unique - use path-based system
|
||||||
let target_path = inst_path(dom, *target_ref);
|
let target_path = inst_path(dom, *target_ref);
|
||||||
path_links.entry(inst_ref).or_default().push(PathRefLink {
|
path_links.entry(inst_ref).or_default().push(PathRefLink {
|
||||||
|
|
|
||||||
125
src/web/api.rs
125
src/web/api.rs
|
|
@ -221,13 +221,24 @@ impl ApiService {
|
||||||
}
|
}
|
||||||
|
|
||||||
// Process added instances (syncback: create files from Studio instances)
|
// Process added instances (syncback: create files from Studio instances)
|
||||||
for added in request.added.values() {
|
// We hold the tree lock during the entire processing to ensure the cache
|
||||||
if let Err(err) = self.syncback_added_instance(added) {
|
// remains consistent with the tree state being checked.
|
||||||
log::warn!(
|
if !request.added.is_empty() {
|
||||||
"Failed to syncback added instance '{}': {}",
|
let tree = self.serve_session.tree();
|
||||||
added.name,
|
// Pre-compute duplicate sibling info in O(N) for efficient path uniqueness checks
|
||||||
err
|
// This makes all subsequent path checks O(d) instead of O(d × s)
|
||||||
);
|
let duplicate_siblings_cache = Self::compute_tree_refs_with_duplicate_siblings(&tree);
|
||||||
|
|
||||||
|
for added in request.added.values() {
|
||||||
|
if let Err(err) =
|
||||||
|
self.syncback_added_instance(added, &tree, &duplicate_siblings_cache)
|
||||||
|
{
|
||||||
|
log::warn!(
|
||||||
|
"Failed to syncback added instance '{}': {}",
|
||||||
|
added.name,
|
||||||
|
err
|
||||||
|
);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -257,13 +268,73 @@ impl ApiService {
|
||||||
/// Syncback an added instance by creating a file in the filesystem.
|
/// Syncback an added instance by creating a file in the filesystem.
|
||||||
/// The file watcher will pick up the change and update Rojo's tree.
|
/// The file watcher will pick up the change and update Rojo's tree.
|
||||||
///
|
///
|
||||||
/// Checks if a path in the Rojo tree is unique by verifying no duplicates exist
|
/// Pre-computes which instances have duplicate-named siblings in a RojoTree.
|
||||||
/// at ANY level of the path (similar to ref_properties::is_path_unique).
|
/// Returns a HashSet of Refs that have at least one sibling with the same name.
|
||||||
|
///
|
||||||
|
/// This is O(N) where N is the number of instances, and allows subsequent
|
||||||
|
/// path uniqueness checks to be O(d) instead of O(d × s) where d=depth, s=siblings.
|
||||||
|
fn compute_tree_refs_with_duplicate_siblings(
|
||||||
|
tree: &crate::snapshot::RojoTree,
|
||||||
|
) -> HashSet<Ref> {
|
||||||
|
use std::collections::VecDeque;
|
||||||
|
|
||||||
|
let mut has_duplicate_siblings = HashSet::new();
|
||||||
|
let mut queue = VecDeque::new();
|
||||||
|
queue.push_back(tree.root().id());
|
||||||
|
|
||||||
|
while let Some(inst_ref) = queue.pop_front() {
|
||||||
|
let inst = match tree.get_instance(inst_ref) {
|
||||||
|
Some(i) => i,
|
||||||
|
None => continue,
|
||||||
|
};
|
||||||
|
|
||||||
|
// Count children by name and collect their refs
|
||||||
|
let mut name_to_refs: HashMap<&str, Vec<Ref>> = HashMap::new();
|
||||||
|
for child_ref in inst.children() {
|
||||||
|
if let Some(child) = tree.get_instance(*child_ref) {
|
||||||
|
name_to_refs.entry(child.name()).or_default().push(*child_ref);
|
||||||
|
}
|
||||||
|
queue.push_back(*child_ref);
|
||||||
|
}
|
||||||
|
|
||||||
|
// Mark refs that share a name with siblings
|
||||||
|
for (_name, refs) in name_to_refs {
|
||||||
|
if refs.len() > 1 {
|
||||||
|
for r in refs {
|
||||||
|
has_duplicate_siblings.insert(r);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
has_duplicate_siblings
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Checks if a path in the Rojo tree is unique using pre-computed duplicate sibling info.
|
||||||
/// Returns true if the path is unique, false if duplicates exist at any level.
|
/// Returns true if the path is unique, false if duplicates exist at any level.
|
||||||
fn is_tree_path_unique(&self, tree: &crate::snapshot::RojoTree, target_ref: Ref) -> bool {
|
/// This is O(d) where d is the depth of the instance.
|
||||||
|
fn is_tree_path_unique_with_cache(
|
||||||
|
tree: &crate::snapshot::RojoTree,
|
||||||
|
target_ref: Ref,
|
||||||
|
has_duplicate_siblings: &HashSet<Ref>,
|
||||||
|
) -> bool {
|
||||||
let mut current_ref = target_ref;
|
let mut current_ref = target_ref;
|
||||||
|
|
||||||
loop {
|
loop {
|
||||||
|
// O(1) lookup instead of O(siblings) counting
|
||||||
|
if has_duplicate_siblings.contains(¤t_ref) {
|
||||||
|
if let Some(current) = tree.get_instance(current_ref) {
|
||||||
|
if let Some(parent) = tree.get_instance(current.parent()) {
|
||||||
|
log::warn!(
|
||||||
|
"Syncback: Path to '{}' is not unique - duplicate-named siblings at '{}'",
|
||||||
|
current.name(),
|
||||||
|
parent.name()
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
let current = match tree.get_instance(current_ref) {
|
let current = match tree.get_instance(current_ref) {
|
||||||
Some(inst) => inst,
|
Some(inst) => inst,
|
||||||
None => return false,
|
None => return false,
|
||||||
|
|
@ -275,31 +346,6 @@ impl ApiService {
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
|
|
||||||
let parent = match tree.get_instance(parent_ref) {
|
|
||||||
Some(inst) => inst,
|
|
||||||
None => return true,
|
|
||||||
};
|
|
||||||
|
|
||||||
// Check if any sibling has the same name as current
|
|
||||||
let current_name = current.name();
|
|
||||||
let mut count = 0;
|
|
||||||
for child_ref in parent.children() {
|
|
||||||
if let Some(child) = tree.get_instance(*child_ref) {
|
|
||||||
if child.name() == current_name {
|
|
||||||
count += 1;
|
|
||||||
if count > 1 {
|
|
||||||
// Duplicate found at this level - path is not unique
|
|
||||||
log::warn!(
|
|
||||||
"Syncback: Path to '{}' is not unique - duplicate-named siblings at '{}'",
|
|
||||||
current_name,
|
|
||||||
parent.name()
|
|
||||||
);
|
|
||||||
return false;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
// Move up to parent and check the next level
|
// Move up to parent and check the next level
|
||||||
current_ref = parent_ref;
|
current_ref = parent_ref;
|
||||||
}
|
}
|
||||||
|
|
@ -307,9 +353,14 @@ impl ApiService {
|
||||||
|
|
||||||
/// This follows the same middleware selection logic as the dedicated syncback
|
/// This follows the same middleware selection logic as the dedicated syncback
|
||||||
/// system in `src/syncback/mod.rs` to ensure consistent behavior.
|
/// system in `src/syncback/mod.rs` to ensure consistent behavior.
|
||||||
|
///
|
||||||
|
/// Takes the tree reference and duplicate siblings cache as parameters to ensure
|
||||||
|
/// consistency - the cache was computed from this exact tree state.
|
||||||
fn syncback_added_instance(
|
fn syncback_added_instance(
|
||||||
&self,
|
&self,
|
||||||
added: &crate::web::interface::AddedInstance,
|
added: &crate::web::interface::AddedInstance,
|
||||||
|
tree: &crate::snapshot::RojoTree,
|
||||||
|
duplicate_siblings_cache: &HashSet<Ref>,
|
||||||
) -> anyhow::Result<()> {
|
) -> anyhow::Result<()> {
|
||||||
use crate::syncback::validate_file_name;
|
use crate::syncback::validate_file_name;
|
||||||
use anyhow::Context;
|
use anyhow::Context;
|
||||||
|
|
@ -323,13 +374,13 @@ impl ApiService {
|
||||||
})?;
|
})?;
|
||||||
|
|
||||||
// Find the parent instance to get its filesystem path
|
// Find the parent instance to get its filesystem path
|
||||||
let tree = self.serve_session.tree();
|
|
||||||
let parent_instance = tree
|
let parent_instance = tree
|
||||||
.get_instance(added.parent)
|
.get_instance(added.parent)
|
||||||
.context("Parent instance not found in Rojo tree")?;
|
.context("Parent instance not found in Rojo tree")?;
|
||||||
|
|
||||||
// Check if the parent path is unique (no duplicate-named siblings at any level)
|
// Check if the parent path is unique (no duplicate-named siblings at any level)
|
||||||
if !self.is_tree_path_unique(&tree, added.parent) {
|
// Uses pre-computed cache for O(d) instead of O(d × s) complexity
|
||||||
|
if !Self::is_tree_path_unique_with_cache(tree, added.parent, duplicate_siblings_cache) {
|
||||||
anyhow::bail!(
|
anyhow::bail!(
|
||||||
"Cannot sync instance '{}' - parent path contains duplicate-named siblings",
|
"Cannot sync instance '{}' - parent path contains duplicate-named siblings",
|
||||||
added.name
|
added.name
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue