mirror of
https://github.com/rojo-rbx/rojo.git
synced 2026-10-23 05:51:39 +00:00
Complete integration tests for two-way sync operations
- Marked all planned integration tests for the two-way sync functionality as completed, ensuring coverage for critical operations such as source property writes, property persistence, instance renames, and lifecycle management. - Added a new test module specifically for two-way sync, enhancing overall test coverage and reliability of the sync system. - Updated the change processor to handle file path overrides during instance updates, improving the accuracy of source writes and file management.
This commit is contained in:
Joe
parent
06d296ffba
commit
6ce3717663
5 changed files with 739 additions and 101 deletions
|
|
@ -488,6 +488,11 @@ impl JobThreadContext {
|
|||
let id = update.id;
|
||||
|
||||
if let Some(instance) = tree.get_instance(id) {
|
||||
// Track the current source file path — rename and ClassName
|
||||
// handlers may move the file, so the Source write must target
|
||||
// the new location instead of the stale instigating_source.
|
||||
let mut overridden_source_path: Option<PathBuf> = None;
|
||||
|
||||
// Handle instance rename on disk
|
||||
if let Some(ref new_name) = update.changed_name {
|
||||
if let Some(instigating_source) = &instance.metadata().instigating_source {
|
||||
|
|
@ -495,10 +500,8 @@ impl JobThreadContext {
|
|||
InstigatingSource::Path(path) => {
|
||||
if path.exists() {
|
||||
let old_name = instance.name();
|
||||
let file_name = path
|
||||
.file_name()
|
||||
.and_then(|f| f.to_str())
|
||||
.unwrap_or("");
|
||||
let file_name =
|
||||
path.file_name().and_then(|f| f.to_str()).unwrap_or("");
|
||||
|
||||
// For directory-format scripts, the instigating_source
|
||||
// is the init file (e.g., src/MyModule/init.luau).
|
||||
|
|
@ -515,8 +518,7 @@ impl JobThreadContext {
|
|||
.unwrap_or("");
|
||||
let new_dir_name =
|
||||
dir_name.replacen(old_name, new_name, 1);
|
||||
let new_dir_path =
|
||||
grandparent.join(&new_dir_name);
|
||||
let new_dir_path = grandparent.join(&new_dir_name);
|
||||
|
||||
if new_dir_path != dir_path {
|
||||
log::info!(
|
||||
|
|
@ -534,16 +536,17 @@ impl JobThreadContext {
|
|||
err
|
||||
);
|
||||
} else {
|
||||
// The init file moved with the directory.
|
||||
overridden_source_path =
|
||||
Some(new_dir_path.join(file_name));
|
||||
let old_meta = grandparent.join(format!(
|
||||
"{}.meta.json5",
|
||||
old_name
|
||||
));
|
||||
if old_meta.exists() {
|
||||
let new_meta =
|
||||
grandparent.join(format!(
|
||||
"{}.meta.json5",
|
||||
new_name
|
||||
));
|
||||
let new_meta = grandparent.join(
|
||||
format!("{}.meta.json5", new_name),
|
||||
);
|
||||
let _ =
|
||||
fs::rename(&old_meta, &new_meta);
|
||||
}
|
||||
|
|
@ -569,6 +572,7 @@ impl JobThreadContext {
|
|||
err
|
||||
);
|
||||
} else {
|
||||
overridden_source_path = Some(new_path.clone());
|
||||
let old_meta = parent
|
||||
.join(format!("{}.meta.json5", old_name));
|
||||
if old_meta.exists() {
|
||||
|
|
@ -674,6 +678,8 @@ impl JobThreadContext {
|
|||
new_path,
|
||||
err
|
||||
);
|
||||
} else {
|
||||
overridden_source_path = Some(new_path.clone());
|
||||
}
|
||||
}
|
||||
} else {
|
||||
|
|
@ -709,33 +715,22 @@ impl JobThreadContext {
|
|||
|
||||
for (key, changed_value) in &update.changed_properties {
|
||||
if key == "Source" {
|
||||
if let Some(instigating_source) =
|
||||
// If a rename or ClassName change moved the file
|
||||
// earlier in this update, write to the new location
|
||||
// instead of the stale instigating_source path.
|
||||
let source_path = if let Some(ref overridden) = overridden_source_path {
|
||||
Some(overridden.clone())
|
||||
} else if let Some(instigating_source) =
|
||||
&instance.metadata().instigating_source
|
||||
{
|
||||
match instigating_source {
|
||||
InstigatingSource::Path(path) => {
|
||||
if let Some(Variant::String(value)) = changed_value {
|
||||
log::info!(
|
||||
"Two-way sync: Writing Source to {}",
|
||||
path.display()
|
||||
);
|
||||
if let Err(err) = fs::write(path, value) {
|
||||
log::error!(
|
||||
"Failed to write Source to {:?} for instance {:?}: {}",
|
||||
path,
|
||||
id,
|
||||
err
|
||||
);
|
||||
}
|
||||
} else {
|
||||
log::warn!("Cannot change Source to non-string value.");
|
||||
}
|
||||
}
|
||||
InstigatingSource::Path(path) => Some(path.clone()),
|
||||
InstigatingSource::ProjectNode { .. } => {
|
||||
log::warn!(
|
||||
"Cannot update instance {:?}, it's from a project file",
|
||||
id
|
||||
);
|
||||
None
|
||||
}
|
||||
}
|
||||
} else {
|
||||
|
|
@ -743,6 +738,26 @@ impl JobThreadContext {
|
|||
"Cannot update instance {:?}, it is not an instigating source.",
|
||||
id
|
||||
);
|
||||
None
|
||||
};
|
||||
|
||||
if let Some(ref write_path) = source_path {
|
||||
if let Some(Variant::String(value)) = changed_value {
|
||||
log::info!(
|
||||
"Two-way sync: Writing Source to {}",
|
||||
write_path.display()
|
||||
);
|
||||
if let Err(err) = fs::write(write_path, value) {
|
||||
log::error!(
|
||||
"Failed to write Source to {:?} for instance {:?}: {}",
|
||||
write_path,
|
||||
id,
|
||||
err
|
||||
);
|
||||
}
|
||||
} else {
|
||||
log::warn!("Cannot change Source to non-string value.");
|
||||
}
|
||||
}
|
||||
} else {
|
||||
log::trace!("Skipping non-Source property change: {}", key);
|
||||
|
|
|
|||
108
src/web/api.rs
108
src/web/api.rs
|
|
@ -967,6 +967,8 @@ impl ApiService {
|
|||
let meta_path = containing_dir.join(format!("{}.meta.json5", script_name));
|
||||
if meta_path.exists() {
|
||||
let init_meta_path = new_dir.join("init.meta.json5");
|
||||
self.suppress_path(&meta_path);
|
||||
self.suppress_path(&init_meta_path);
|
||||
fs::rename(&meta_path, &init_meta_path).with_context(|| {
|
||||
format!(
|
||||
"Failed to move meta file {} to {}",
|
||||
|
|
@ -1039,71 +1041,58 @@ impl ApiService {
|
|||
// Use json5::from_str (not serde_json) because .model.json5 files
|
||||
// can contain JSON5 features (comments, trailing commas, unquoted keys).
|
||||
let raw_str = String::from_utf8_lossy(&raw);
|
||||
let meta_content =
|
||||
if let Ok(model) = json5::from_str::<serde_json::Value>(&raw_str) {
|
||||
// Warn about inline children that will be lost
|
||||
if let Some(children) = model.get("children").or(model.get("Children")) {
|
||||
if children.is_array()
|
||||
&& children.as_array().map_or(false, |a| !a.is_empty())
|
||||
{
|
||||
log::warn!(
|
||||
"Syncback: .model.json5 at {} contains inline children \
|
||||
let meta_content = if let Ok(model) = json5::from_str::<serde_json::Value>(&raw_str)
|
||||
{
|
||||
// Warn about inline children that will be lost
|
||||
if let Some(children) = model.get("children").or(model.get("Children")) {
|
||||
if children.is_array() && children.as_array().is_some_and(|a| !a.is_empty())
|
||||
{
|
||||
log::warn!(
|
||||
"Syncback: .model.json5 at {} contains inline children \
|
||||
that cannot be represented in init.meta.json5 — \
|
||||
these children will be lost during directory conversion",
|
||||
standalone_path.display()
|
||||
);
|
||||
}
|
||||
standalone_path.display()
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
// Build init.meta.json5 with only the supported fields
|
||||
let mut meta = serde_json::Map::new();
|
||||
if let Some(cn) = model
|
||||
.get("className")
|
||||
.or(model.get("ClassName"))
|
||||
.cloned()
|
||||
{
|
||||
meta.insert("className".to_string(), cn);
|
||||
// Build init.meta.json5 with only the supported fields
|
||||
let mut meta = serde_json::Map::new();
|
||||
if let Some(cn) = model.get("className").or(model.get("ClassName")).cloned() {
|
||||
meta.insert("className".to_string(), cn);
|
||||
}
|
||||
if let Some(props) =
|
||||
model.get("properties").or(model.get("Properties")).cloned()
|
||||
{
|
||||
if props.is_object() && props.as_object().is_some_and(|o| !o.is_empty()) {
|
||||
meta.insert("properties".to_string(), props);
|
||||
}
|
||||
if let Some(props) = model
|
||||
.get("properties")
|
||||
.or(model.get("Properties"))
|
||||
.cloned()
|
||||
{
|
||||
if props.is_object()
|
||||
&& props.as_object().map_or(false, |o| !o.is_empty())
|
||||
{
|
||||
meta.insert("properties".to_string(), props);
|
||||
}
|
||||
}
|
||||
if let Some(attrs) = model
|
||||
.get("attributes")
|
||||
.or(model.get("Attributes"))
|
||||
.cloned()
|
||||
{
|
||||
if attrs.is_object()
|
||||
&& attrs.as_object().map_or(false, |o| !o.is_empty())
|
||||
{
|
||||
meta.insert("attributes".to_string(), attrs);
|
||||
}
|
||||
}
|
||||
if let Some(ignore) = model.get("ignoreUnknownInstances").cloned() {
|
||||
meta.insert("ignoreUnknownInstances".to_string(), ignore);
|
||||
}
|
||||
if let Some(id) = model.get("id").cloned() {
|
||||
meta.insert("id".to_string(), id);
|
||||
}
|
||||
if let Some(attrs) =
|
||||
model.get("attributes").or(model.get("Attributes")).cloned()
|
||||
{
|
||||
if attrs.is_object() && attrs.as_object().is_some_and(|o| !o.is_empty()) {
|
||||
meta.insert("attributes".to_string(), attrs);
|
||||
}
|
||||
}
|
||||
if let Some(ignore) = model.get("ignoreUnknownInstances").cloned() {
|
||||
meta.insert("ignoreUnknownInstances".to_string(), ignore);
|
||||
}
|
||||
if let Some(id) = model.get("id").cloned() {
|
||||
meta.insert("id".to_string(), id);
|
||||
}
|
||||
|
||||
crate::json::to_vec_pretty_sorted(&serde_json::Value::Object(meta))
|
||||
.unwrap_or(raw.clone())
|
||||
} else {
|
||||
// Parse failed — copy raw as fallback (best-effort)
|
||||
log::warn!(
|
||||
"Syncback: Could not parse {} as JSON — \
|
||||
crate::json::to_vec_pretty_sorted(&serde_json::Value::Object(meta))
|
||||
.unwrap_or(raw.clone())
|
||||
} else {
|
||||
// Parse failed — copy raw as fallback (best-effort)
|
||||
log::warn!(
|
||||
"Syncback: Could not parse {} as JSON — \
|
||||
copying raw content to init.meta.json5",
|
||||
standalone_path.display()
|
||||
);
|
||||
raw
|
||||
};
|
||||
standalone_path.display()
|
||||
);
|
||||
raw
|
||||
};
|
||||
|
||||
self.suppress_path(&init_meta_path);
|
||||
fs::write(&init_meta_path, &meta_content)
|
||||
|
|
@ -2201,10 +2190,7 @@ impl ApiService {
|
|||
// to preserve Windows-invalid character encoding (e.g., %3F for ?).
|
||||
// This matches AdjacentMetadata::read_and_apply_all which uses file_stem().
|
||||
let parent_dir = inst_path.parent().context("No parent directory")?;
|
||||
let file_stem = inst_path
|
||||
.file_stem()
|
||||
.and_then(|s| s.to_str())
|
||||
.unwrap_or("");
|
||||
let file_stem = inst_path.file_stem().and_then(|s| s.to_str()).unwrap_or("");
|
||||
let base_name = file_stem
|
||||
.strip_suffix(".server")
|
||||
.or_else(|| file_stem.strip_suffix(".client"))
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue