ari-party/rojo
1
0
Fork 0
forked from rojo-rbx/rojo
Code Pull Requests Activity

Support setting referent properties via attributes (#843)

Browse Source 
Co-authored-by: Kenneth Loeffler <kenloef@gmail.com>
This commit is contained in:
Micah
2024-06-20 15:48:52 -07:00
committed by GitHub
parent a7b45ee859
commit 7e2bab921a
64 changed files with 942 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

20
CHANGELOG.md
View File

@@ -1,6 +1,25 @@
# Rojo Changelog
## Unreleased Changes
* Projects may now manually link `Ref` properties together using `Attributes`. ([#843])
This has two parts: using `id` or `$id` in JSON files or a `Rojo_Target` attribute, an Instance
is given an ID. Then, that ID may be used elsewhere in the project to point to an Instance
using an attribute named `Rojo_Target_PROP_NAME`, where `PROP_NAME` is the name of a property.
As an example, here is a `model.json` for an ObjectValue that refers to itself:
```json
{
"id": "arbitrary string",
"attributes": {
"Rojo_Target_Value": "arbitrary string"
}
}
```
This is a very rough implementation and the usage will become more ergonomic
over time.
* Updated Undo/Redo history to be more robust ([#915])
* Fixed removing trailing newlines ([#903])
* Added Never option to Confirmation ([#893])
@@ -61,6 +80,7 @@
[#834]: https://github.com/rojo-rbx/rojo/pull/834
[#838]: https://github.com/rojo-rbx/rojo/pull/838
[#840]: https://github.com/rojo-rbx/rojo/pull/840
[#843]: https://github.com/rojo-rbx/rojo/pull/843
[#883]: https://github.com/rojo-rbx/rojo/pull/883
[#886]: https://github.com/rojo-rbx/rojo/pull/886
[#893]: https://github.com/rojo-rbx/rojo/pull/893

141
rojo-test/serve-test-snapshots/end_to_end__tests__serve__ref_properties_all-2.snap Normal file
View File

@@ -0,0 +1,141 @@
---
source: tests/tests/serve.rs
expression: "read_response.intern_and_redact(&mut redactions, root_id)"
---
instances:
id-10:
Children: []
ClassName: ObjectValue
Id: id-10
Metadata:
ignoreUnknownInstances: true
Name: ProjectPointer
Parent: id-9
Properties:
Attributes:
Attributes:
Rojo_Target_Value:
String: project target
Value:
Ref: id-9
id-11:
Children: []
ClassName: Model
Id: id-11
Metadata:
ignoreUnknownInstances: false
Name: ProjectPointer
Parent: id-7
Properties:
Attributes:
Attributes:
Rojo_Target_PrimaryPart:
String: project target
PrimaryPart:
Ref: id-9
id-2:
Children:
- id-3
ClassName: DataModel
Id: id-2
Metadata:
ignoreUnknownInstances: true
Name: ref_properties
Parent: "00000000000000000000000000000000"
Properties: {}
id-3:
Children:
- id-4
- id-5
- id-7
- id-9
ClassName: Workspace
Id: id-3
Metadata:
ignoreUnknownInstances: true
Name: Workspace
Parent: id-2
Properties: {}
id-4:
Children: []
ClassName: ObjectValue
Id: id-4
Metadata:
ignoreUnknownInstances: true
Name: CrossFormatPointer
Parent: id-3
Properties:
Attributes:
Attributes:
Rojo_Target_Value:
String: folder target
Value:
Ref: id-5
id-5:
Children:
- id-6
ClassName: Folder
Id: id-5
Metadata:
ignoreUnknownInstances: false
Name: FolderTarget
Parent: id-3
Properties: {}
id-6:
Children: []
ClassName: ObjectValue
Id: id-6
Metadata:
ignoreUnknownInstances: false
Name: FolderPointer
Parent: id-5
Properties:
Attributes:
Attributes:
Rojo_Target_Value:
String: folder target
Value:
Ref: id-5
id-7:
Children:
- id-8
- id-11
ClassName: Folder
Id: id-7
Metadata:
ignoreUnknownInstances: false
Name: ModelTarget
Parent: id-3
Properties:
Attributes:
Attributes:
Rojo_Id:
String: model target 2
id-8:
Children: []
ClassName: Model
Id: id-8
Metadata:
ignoreUnknownInstances: false
Name: ModelPointer
Parent: id-7
Properties:
Attributes:
Attributes:
Rojo_Target_PrimaryPart:
String: model target 2
PrimaryPart:
Ref: id-7
id-9:
Children:
- id-10
ClassName: Folder
Id: id-9
Metadata:
ignoreUnknownInstances: true
Name: ProjectTarget
Parent: id-3
Properties: {}
messageCursor: 1
sessionId: id-1

121
rojo-test/serve-test-snapshots/end_to_end__tests__serve__ref_properties_all.snap Normal file
View File

@@ -0,0 +1,121 @@
---
source: tests/tests/serve.rs
expression: "read_response.intern_and_redact(&mut redactions, root_id)"
---
instances:
id-10:
Children: []
ClassName: ObjectValue
Id: id-10
Metadata:
ignoreUnknownInstances: true
Name: ProjectPointer
Parent: id-9
Properties:
Attributes:
Attributes:
Rojo_Target_Value:
String: project target
Value:
Ref: id-9
id-2:
Children:
- id-3
ClassName: DataModel
Id: id-2
Metadata:
ignoreUnknownInstances: true
Name: ref_properties
Parent: "00000000000000000000000000000000"
Properties: {}
id-3:
Children:
- id-4
- id-5
- id-7
- id-9
ClassName: Workspace
Id: id-3
Metadata:
ignoreUnknownInstances: true
Name: Workspace
Parent: id-2
Properties: {}
id-4:
Children: []
ClassName: ObjectValue
Id: id-4
Metadata:
ignoreUnknownInstances: true
Name: CrossFormatPointer
Parent: id-3
Properties:
Attributes:
Attributes:
Rojo_Target_Value:
String: folder target
Value:
Ref: id-5
id-5:
Children:
- id-6
ClassName: Folder
Id: id-5
Metadata:
ignoreUnknownInstances: false
Name: FolderTarget
Parent: id-3
Properties: {}
id-6:
Children: []
ClassName: ObjectValue
Id: id-6
Metadata:
ignoreUnknownInstances: false
Name: FolderPointer
Parent: id-5
Properties:
Attributes:
Attributes:
Rojo_Target_Value:
String: folder target
Value:
Ref: id-5
id-7:
Children:
- id-8
ClassName: Folder
Id: id-7
Metadata:
ignoreUnknownInstances: false
Name: ModelTarget
Parent: id-3
Properties: {}
id-8:
Children: []
ClassName: Model
Id: id-8
Metadata:
ignoreUnknownInstances: false
Name: ModelPointer
Parent: id-7
Properties:
Attributes:
Attributes:
Rojo_Target_PrimaryPart:
String: model target
PrimaryPart:
Ref: id-7
id-9:
Children:
- id-10
ClassName: Folder
Id: id-9
Metadata:
ignoreUnknownInstances: true
Name: ProjectTarget
Parent: id-3
Properties: {}
messageCursor: 0
sessionId: id-1

13
rojo-test/serve-test-snapshots/end_to_end__tests__serve__ref_properties_info.snap Normal file
View File

@@ -0,0 +1,13 @@
---
source: tests/tests/serve.rs
expression: redactions.redacted_yaml(info)
---
expectedPlaceIds: ~
gameId: ~
placeId: ~
projectName: ref_properties
protocolVersion: 4
rootInstanceId: id-2
serverVersion: "[server-version]"
sessionId: id-1

33
rojo-test/serve-test-snapshots/end_to_end__tests__serve__ref_properties_remove_all-2.snap Normal file
View File

@@ -0,0 +1,33 @@
---
source: tests/tests/serve.rs
expression: "read_response.intern_and_redact(&mut redactions, root_id)"
---
instances:
id-2:
Children:
- id-3
ClassName: Folder
Id: id-2
Metadata:
ignoreUnknownInstances: false
Name: ref_properties_remove
Parent: "00000000000000000000000000000000"
Properties: {}
id-3:
Children: []
ClassName: ObjectValue
Id: id-3
Metadata:
ignoreUnknownInstances: false
Name: pointer
Parent: id-2
Properties:
Attributes:
Attributes:
Rojo_Target_Value:
String: test pointer
Value:
Ref: id-4
messageCursor: 1
sessionId: id-1

47
rojo-test/serve-test-snapshots/end_to_end__tests__serve__ref_properties_remove_all.snap Normal file
View File

@@ -0,0 +1,47 @@
---
source: tests/tests/serve.rs
expression: "read_response.intern_and_redact(&mut redactions, root_id)"
---
instances:
id-2:
Children:
- id-3
- id-4
ClassName: Folder
Id: id-2
Metadata:
ignoreUnknownInstances: false
Name: ref_properties_remove
Parent: "00000000000000000000000000000000"
Properties: {}
id-3:
Children: []
ClassName: ObjectValue
Id: id-3
Metadata:
ignoreUnknownInstances: false
Name: pointer
Parent: id-2
Properties:
Attributes:
Attributes:
Rojo_Target_Value:
String: test pointer
Value:
Ref: id-4
id-4:
Children: []
ClassName: ObjectValue
Id: id-4
Metadata:
ignoreUnknownInstances: false
Name: target
Parent: id-2
Properties:
Attributes:
Attributes:
Rojo_Id:
String: test pointer
messageCursor: 0
sessionId: id-1

13
rojo-test/serve-test-snapshots/end_to_end__tests__serve__ref_properties_remove_info.snap Normal file
View File

@@ -0,0 +1,13 @@
---
source: tests/tests/serve.rs
expression: redactions.redacted_yaml(info)
---
expectedPlaceIds: ~
gameId: ~
placeId: ~
projectName: ref_properties_remove
protocolVersion: 4
rootInstanceId: id-2
serverVersion: "[server-version]"
sessionId: id-1

12
rojo-test/serve-test-snapshots/end_to_end__tests__serve__ref_properties_remove_subscribe.snap Normal file
View File

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

46
rojo-test/serve-test-snapshots/end_to_end__tests__serve__ref_properties_subscribe.snap Normal file
View File

@@ -0,0 +1,46 @@
---
source: tests/tests/serve.rs
expression: "subscribe_response.intern_and_redact(&mut redactions, ())"
---
messageCursor: 1
messages:
- added:
id-11:
Children: []
ClassName: Model
Id: id-11
Metadata:
ignoreUnknownInstances: false
Name: ProjectPointer
Parent: id-7
Properties:
Attributes:
Attributes:
Rojo_Target_PrimaryPart:
String: project target
PrimaryPart:
Ref: id-9
removed: []
updated:
- changedClassName: ~
changedMetadata:
ignoreUnknownInstances: false
changedName: ~
changedProperties:
Attributes:
Attributes:
Rojo_Id:
String: model target 2
id: id-7
- changedClassName: ~
changedMetadata: ~
changedName: ~
changedProperties:
Attributes:
Attributes:
Rojo_Target_PrimaryPart:
String: model target 2
PrimaryPart: ~
id: id-8
sessionId: id-1

6
rojo-test/serve-tests/ref_properties/FolderTarget/FolderPointer.model.json Normal file
View File

@@ -0,0 +1,6 @@
{
"className": "ObjectValue",
"attributes": {
"Rojo_Target_Value": "folder target"
}
}

3
rojo-test/serve-tests/ref_properties/FolderTarget/init.meta.json Normal file
View File

@@ -0,0 +1,3 @@
{
"id": "folder target"
}

13
rojo-test/serve-tests/ref_properties/ModelTarget.model.json Normal file
View File

@@ -0,0 +1,13 @@
{
"id": "model target",
"className": "Folder",
"children": [
{
"name": "ModelPointer",
"className": "Model",
"attributes": {
"Rojo_Target_PrimaryPart": "model target"
}
}
]
}

32
rojo-test/serve-tests/ref_properties/default.project.json Normal file
View File

@@ -0,0 +1,32 @@
{
"name": "ref_properties",
"tree": {
"$className": "DataModel",
"Workspace": {
"ProjectTarget": {
"$className": "Folder",
"$id": "project target",
"ProjectPointer": {
"$className": "ObjectValue",
"$attributes": {
"Rojo_Target_Value": {
"String": "project target"
}
}
}
},
"ModelTarget": {
"$path": "ModelTarget.model.json"
},
"FolderTarget": {
"$path": "FolderTarget"
},
"CrossFormatPointer": {
"$className": "ObjectValue",
"$attributes": {
"Rojo_Target_Value": "folder target"
}
}
}
}
}

6
rojo-test/serve-tests/ref_properties_remove/default.project.json Normal file
View File

@@ -0,0 +1,6 @@
{
"name": "ref_properties_remove",
"tree": {
"$path": "src"
}
}

6
rojo-test/serve-tests/ref_properties_remove/src/pointer.model.json Normal file
View File

@@ -0,0 +1,6 @@
{
"className": "ObjectValue",
"attributes": {
"Rojo_Target_Value": "test pointer"
}
}

6
rojo-test/serve-tests/ref_properties_remove/src/target.model.json Normal file
View File

@@ -0,0 +1,6 @@
{
"className": "ObjectValue",
"attributes": {
"Rojo_Id": "test pointer"
}
}

2
src/lib.rs
View File

@@ -16,6 +16,7 @@ mod multimap;
mod path_serializer;
mod project;
mod resolution;
mod rojo_ref;
mod serve_session;
mod session_id;
mod snapshot;
@@ -23,5 +24,6 @@ mod snapshot_middleware;
mod web;
pub use project::*;
pub use rojo_ref::*;
pub use session_id::SessionId;
pub use web::interface as web_api;

32
src/multimap.rs
View File

@@ -1,6 +1,6 @@
use std::{
borrow::Borrow,
collections::HashMap,
collections::{hash_map, HashMap},
fmt::{self, Debug},
hash::Hash,
};
@@ -71,3 +71,33 @@ impl<K: Hash + Eq, V: Eq> PartialEq for MultiMap<K, V> {
self.inner == other.inner
}
}
impl<K, V> Default for MultiMap<K, V> {
fn default() -> Self {
Self {
inner: Default::default(),
}
}
}
impl<K: Hash + Eq, V: Eq> IntoIterator for MultiMap<K, V> {
type IntoIter = MultiMapIntoIter<K, V>;
type Item = (K, Vec<V>);
fn into_iter(self) -> Self::IntoIter {
Self::IntoIter {
inner: self.inner.into_iter(),
}
}
}
pub struct MultiMapIntoIter<K: Hash + Eq, V: Eq> {
inner: hash_map::IntoIter<K, Vec<V>>,
}
impl<K: Hash + Eq, V: Eq> Iterator for MultiMapIntoIter<K, V> {
type Item = (K, Vec<V>);
fn next(&mut self) -> Option<Self::Item> {
self.inner.next()
}
}

5
src/project.rs
View File

@@ -226,6 +226,11 @@ pub struct ProjectNode {
#[serde(rename = "$className", skip_serializing_if = "Option::is_none")]
pub class_name: Option<String>,
/// If set, defines an ID for the described Instance that can be used
/// to refer to it for the purpose of referent properties.
#[serde(rename = "$id", skip_serializing_if = "Option::is_none")]
pub id: Option<String>,
/// Contains all of the children of the described instance.
#[serde(flatten)]
pub children: BTreeMap<String, ProjectNode>,

6
src/resolution.rs
View File

@@ -8,6 +8,8 @@ use rbx_dom_weak::types::{
use rbx_reflection::{DataType, PropertyDescriptor};
use serde::{Deserialize, Serialize};
use crate::REF_POINTER_ATTRIBUTE_PREFIX;
/// A user-friendly version of `Variant` that supports specifying ambiguous
/// values. Ambiguous values need a reflection database to be resolved to a
/// usable value.
@@ -147,6 +149,10 @@ impl AmbiguousValue {
Ok(value.into())
}
(VariantType::Ref, AmbiguousValue::String(_)) => Err(format_err!(
"Cannot resolve Ref properties as a String.\
Use an attribute named `{REF_POINTER_ATTRIBUTE_PREFIX}{prop_name}"
)),
(_, unresolved) => Err(format_err!(
"Wrong type of value for property {}.{}. Expected {:?}, got {}",
class_name,

30
src/rojo_ref.rs Normal file
View File

@@ -0,0 +1,30 @@
use std::{fmt, sync::Arc};
use serde::{Deserialize, Serialize};
pub const REF_ID_ATTRIBUTE_NAME: &str = "Rojo_Id";
pub const REF_POINTER_ATTRIBUTE_PREFIX: &str = "Rojo_Target_";
// TODO add an internment strategy for RojoRefs
// Something like what rbx-dom does for SharedStrings probably works
#[derive(Debug, Default, PartialEq, Hash, Clone, Serialize, Deserialize, Eq)]
pub struct RojoRef(Arc<String>);
impl RojoRef {
#[inline]
pub fn new(id: String) -> Self {
Self(Arc::from(id))
}
#[inline]
pub fn as_str(&self) -> &str {
self.0.as_str()
}
}
impl fmt::Display for RojoRef {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "{}", self.as_str())
}
}

12
src/snapshot/metadata.rs
View File

@@ -12,6 +12,7 @@ use crate::{
path_serializer,
project::ProjectNode,
snapshot_middleware::{emit_legacy_scripts_default, Middleware},
RojoRef,
};
/// Rojo-specific metadata that can be associated with an instance or a snapshot
@@ -58,6 +59,9 @@ pub struct InstanceMetadata {
/// that instance's instigating source is snapshotted directly, the same
/// context will be passed into it.
pub context: InstanceContext,
/// Indicates the ID used for Ref properties pointing to this Instance.
pub specified_id: Option<RojoRef>,
}
impl InstanceMetadata {
@@ -67,6 +71,7 @@ impl InstanceMetadata {
instigating_source: None,
relevant_paths: Vec::new(),
context: InstanceContext::default(),
specified_id: None,
}
}
@@ -97,6 +102,13 @@ impl InstanceMetadata {
..self
}
}
pub fn specified_id(self, id: Option<RojoRef>) -> Self {
Self {
specified_id: id,
..self
}
}
}
impl Default for InstanceMetadata {

87
src/snapshot/patch_apply.rs
View File

@@ -11,6 +11,7 @@ use super::{
patch::{AppliedPatchSet, AppliedPatchUpdate, PatchSet, PatchUpdate},
InstanceSnapshot, RojoTree,
};
use crate::{multimap::MultiMap, RojoRef, REF_ID_ATTRIBUTE_NAME, REF_POINTER_ATTRIBUTE_PREFIX};
/// Consumes the input `PatchSet`, applying all of its prescribed changes to the
/// tree and returns an `AppliedPatchSet`, which can be used to keep another
@@ -72,6 +73,11 @@ struct PatchApplyContext {
/// to be rewritten.
has_refs_to_rewrite: HashSet<Ref>,
/// Tracks all ref properties that were specified using attributes. This has
/// to be handled after everything else is done just like normal referent
/// properties.
attribute_refs_to_rewrite: MultiMap<Ref, (String, String)>,
/// The current applied patch result, describing changes made to the tree.
applied_patch_set: AppliedPatchSet,
}
@@ -104,6 +110,22 @@ fn finalize_patch_application(context: PatchApplyContext, tree: &mut RojoTree) -
}
}
// This is to get around the fact that `RojoTre::get_specified_id` borrows
// the tree as immutable, but we need to hold a mutable reference to it.
// Not exactly elegant, but it does the job.
let mut real_rewrites = Vec::new();
for (id, map) in context.attribute_refs_to_rewrite {
for (prop_name, prop_value) in map {
if let Some(target) = tree.get_specified_id(&RojoRef::new(prop_value)) {
real_rewrites.push((prop_name, Variant::Ref(target)))
}
}
let mut instance = tree
.get_instance_mut(id)
.expect("Invalid instance ID in deferred attribute ref map");
instance.properties_mut().extend(real_rewrites.drain(..));
}
context.applied_patch_set
}
@@ -142,6 +164,8 @@ fn apply_add_child(
for child in children {
apply_add_child(context, tree, id, child);
}
defer_ref_properties(tree, id, context);
}
fn apply_update_child(context: &mut PatchApplyContext, tree: &mut RojoTree, patch: PatchUpdate) {
@@ -208,9 +232,72 @@ fn apply_update_child(context: &mut PatchApplyContext, tree: &mut RojoTree, patc
applied_patch.changed_properties.insert(key, property_entry);
}
defer_ref_properties(tree, patch.id, context);
context.applied_patch_set.updated.push(applied_patch)
}
/// Calculates manually-specified Ref properties and marks them in the provided
/// `PatchApplyContext` to be rewritten at the end of the patch application
/// process.
///
/// Currently, this only uses attributes but it can easily handle rewriting
/// referents in other ways too!
fn defer_ref_properties(tree: &mut RojoTree, id: Ref, context: &mut PatchApplyContext) {
let instance = tree
.get_instance(id)
.expect("Instances should exist when calculating deferred refs");
let attributes = match instance.properties().get("Attributes") {
Some(Variant::Attributes(attrs)) => attrs,
_ => return,
};
let mut attr_id = None;
for (attr_name, attr_value) in attributes.iter() {
if attr_name == REF_ID_ATTRIBUTE_NAME {
if let Variant::String(specified_id) = attr_value {
attr_id = Some(RojoRef::new(specified_id.clone()));
} else if let Variant::BinaryString(specified_id) = attr_value {
if let Ok(str) = std::str::from_utf8(specified_id.as_ref()) {
attr_id = Some(RojoRef::new(str.to_string()))
} else {
log::error!("Specified IDs must be valid UTF-8 strings.")
}
} else {
log::warn!(
"Attribute {attr_name} is of type {:?} when it was \
expected to be a String",
attr_value.ty()
)
}
}
if let Some(prop_name) = attr_name.strip_prefix(REF_POINTER_ATTRIBUTE_PREFIX) {
if let Variant::String(prop_value) = attr_value {
context
.attribute_refs_to_rewrite
.insert(id, (prop_name.to_owned(), prop_value.clone()));
} else if let Variant::BinaryString(prop_value) = attr_value {
if let Ok(str) = std::str::from_utf8(prop_value.as_ref()) {
context
.attribute_refs_to_rewrite
.insert(id, (prop_name.to_owned(), str.to_string()));
} else {
log::error!("IDs specified by referent property attributes must be valid UTF-8 strings.")
}
} else {
log::warn!(
"Attribute {attr_name} is of type {:?} when it was \
expected to be a String",
attr_value.ty()
)
}
}
}
if let Some(specified_id) = attr_id {
tree.set_specified_id(id, specified_id);
}
}
#[cfg(test)]
mod test {
use super::*;

1
src/snapshot/tests/snapshots/librojo__snapshot__tests__apply__add_property.snap
View File

@@ -13,5 +13,6 @@ metadata:
relevant_paths: []
context:
emit_legacy_scripts: true
specified_id: ~
children: []

1
src/snapshot/tests/snapshots/librojo__snapshot__tests__apply__remove_property_after_patch.snap
View File

@@ -11,5 +11,6 @@ metadata:
relevant_paths: []
context:
emit_legacy_scripts: true
specified_id: ~
children: []

1
src/snapshot/tests/snapshots/librojo__snapshot__tests__apply__remove_property_initial.snap
View File

@@ -13,5 +13,6 @@ metadata:
relevant_paths: []
context:
emit_legacy_scripts: true
specified_id: ~
children: []

1
src/snapshot/tests/snapshots/librojo__snapshot__tests__apply__set_name_and_class_name.snap
View File

@@ -11,5 +11,6 @@ metadata:
relevant_paths: []
context:
emit_legacy_scripts: true
specified_id: ~
children: []

1
src/snapshot/tests/snapshots/librojo__snapshot__tests__compute__add_child.snap
View File

@@ -12,6 +12,7 @@ added_instances:
relevant_paths: []
context:
emit_legacy_scripts: true
specified_id: ~
name: New
class_name: Folder
properties: {}

80
src/snapshot/tree.rs
View File

@@ -8,7 +8,7 @@ use rbx_dom_weak::{
Instance, InstanceBuilder, WeakDom,
};
use crate::multimap::MultiMap;
use crate::{multimap::MultiMap, RojoRef};
use super::{InstanceMetadata, InstanceSnapshot};
@@ -33,6 +33,12 @@ pub struct RojoTree {
/// appearing multiple times in the same Rojo project. This is sometimes
/// called "path aliasing" in various Rojo documentation.
path_to_ids: MultiMap<PathBuf, Ref>,
/// A map of specified RojoRefs to underlying Refs they represent.
/// This field is a MultiMap to allow for the possibility of the user specifying
/// the same RojoRef for multiple different instances. An entry containing
/// multiple elements is an error condition that should be raised to the user.
specified_id_to_refs: MultiMap<RojoRef, Ref>,
}
impl RojoTree {
@@ -45,6 +51,7 @@ impl RojoTree {
inner: WeakDom::new(root_builder),
metadata_map: HashMap::new(),
path_to_ids: MultiMap::new(),
specified_id_to_refs: MultiMap::new(),
};
let root_ref = tree.inner.root_ref();
@@ -137,6 +144,20 @@ impl RojoTree {
self.path_to_ids.insert(new_path.clone(), id);
}
}
if existing_metadata.specified_id != metadata.specified_id {
// We need to uphold the invariant that each ID can only map
// to one referent.
if let Some(new) = &metadata.specified_id {
if self.specified_id_to_refs.get(new).len() > 0 {
log::error!("Duplicate user-specified referent '{new}'");
}
self.specified_id_to_refs.insert(new.clone(), id);
}
if let Some(old) = &existing_metadata.specified_id {
self.specified_id_to_refs.remove(old, id);
}
}
entry.insert(metadata);
}
@@ -161,11 +182,37 @@ impl RojoTree {
self.metadata_map.get(&id)
}
/// Get the backing Ref of the given RojoRef. If the RojoRef maps to exactly
/// one Ref, this method returns Some. Otherwise, it returns None.
pub fn get_specified_id(&self, specified: &RojoRef) -> Option<Ref> {
match self.specified_id_to_refs.get(specified)[..] {
[referent] => Some(referent),
_ => None,
}
}
pub fn set_specified_id(&mut self, id: Ref, specified: RojoRef) {
if let Some(metadata) = self.metadata_map.get_mut(&id) {
if let Some(old) = metadata.specified_id.replace(specified.clone()) {
self.specified_id_to_refs.remove(&old, id);
}
}
self.specified_id_to_refs.insert(specified, id);
}
fn insert_metadata(&mut self, id: Ref, metadata: InstanceMetadata) {
for path in &metadata.relevant_paths {
self.path_to_ids.insert(path.clone(), id);
}
if let Some(specified_id) = &metadata.specified_id {
if self.specified_id_to_refs.get(specified_id).len() > 0 {
log::error!("Duplicate user-specified referent '{specified_id}'");
}
self.set_specified_id(id, specified_id.clone());
}
self.metadata_map.insert(id, metadata);
}
@@ -174,6 +221,10 @@ impl RojoTree {
fn remove_metadata(&mut self, id: Ref) {
let metadata = self.metadata_map.remove(&id).unwrap();
if let Some(specified) = metadata.specified_id {
self.specified_id_to_refs.remove(&specified, id);
}
for path in &metadata.relevant_paths {
self.path_to_ids.remove(path, id);
}
@@ -297,3 +348,30 @@ impl InstanceWithMetaMut<'_> {
self.metadata
}
}
#[cfg(test)]
mod test {
use crate::{
snapshot::{InstanceMetadata, InstanceSnapshot},
RojoRef,
};
use super::RojoTree;
#[test]
fn swap_duped_specified_ids() {
let custom_ref = RojoRef::new("MyCoolRef".into());
let snapshot = InstanceSnapshot::new()
.metadata(InstanceMetadata::new().specified_id(Some(custom_ref.clone())));
let mut tree = RojoTree::new(InstanceSnapshot::new());
let original = tree.insert_instance(tree.get_root_id(), snapshot.clone());
assert_eq!(tree.get_specified_id(&custom_ref.clone()), Some(original));
let duped = tree.insert_instance(tree.get_root_id(), snapshot.clone());
assert_eq!(tree.get_specified_id(&custom_ref.clone()), None);
tree.remove(original);
assert_eq!(tree.get_specified_id(&custom_ref.clone()), Some(duped));
}
}

9
src/snapshot_middleware/json_model.rs
View File

@@ -8,6 +8,7 @@ use serde::Deserialize;
use crate::{
resolution::UnresolvedValue,
snapshot::{InstanceContext, InstanceSnapshot},
RojoRef,
};
pub fn snapshot_json_model(
@@ -41,6 +42,8 @@ pub fn snapshot_json_model(
instance.name = Some(name.to_owned());
let id = instance.id.take().map(RojoRef::new);
let mut snapshot = instance
.into_snapshot()
.with_context(|| format!("Could not load JSON model: {}", path.display()))?;
@@ -49,7 +52,8 @@ pub fn snapshot_json_model(
.metadata
.instigating_source(path)
.relevant_paths(vec![path.to_path_buf()])
.context(context);
.context(context)
.specified_id(id);
Ok(Some(snapshot))
}
@@ -63,6 +67,9 @@ struct JsonModel {
#[serde(alias = "ClassName")]
class_name: String,
#[serde(skip_serializing_if = "Option::is_none")]
id: Option<String>,
#[serde(
alias = "Children",
default = "Vec::new",

32
src/snapshot_middleware/meta_file.rs
View File

@@ -4,7 +4,7 @@ use anyhow::{format_err, Context};
use rbx_dom_weak::types::Attributes;
use serde::{Deserialize, Serialize};
use crate::{resolution::UnresolvedValue, snapshot::InstanceSnapshot};
use crate::{resolution::UnresolvedValue, snapshot::InstanceSnapshot, RojoRef};
/// Represents metadata in a sibling file with the same basename.
///
@@ -13,6 +13,9 @@ use crate::{resolution::UnresolvedValue, snapshot::InstanceSnapshot};
#[derive(Debug, Serialize, Deserialize)]
#[serde(rename_all = "camelCase")]
pub struct AdjacentMetadata {
#[serde(skip_serializing_if = "Option::is_none")]
pub id: Option<String>,
#[serde(skip_serializing_if = "Option::is_none")]
pub ignore_unknown_instances: Option<bool>,
@@ -72,9 +75,21 @@ impl AdjacentMetadata {
Ok(())
}
fn apply_id(&mut self, snapshot: &mut InstanceSnapshot) -> anyhow::Result<()> {
if self.id.is_some() && snapshot.metadata.specified_id.is_some() {
anyhow::bail!(
"cannot specify an ID using {} (instance has an ID from somewhere else)",
self.path.display()
);
}
snapshot.metadata.specified_id = self.id.take().map(RojoRef::new);
Ok(())
}
pub fn apply_all(&mut self, snapshot: &mut InstanceSnapshot) -> anyhow::Result<()> {
self.apply_ignore_unknown_instances(snapshot);
self.apply_properties(snapshot)?;
self.apply_id(snapshot)?;
Ok(())
}
@@ -89,6 +104,9 @@ impl AdjacentMetadata {
#[derive(Debug, Serialize, Deserialize)]
#[serde(rename_all = "camelCase")]
pub struct DirectoryMetadata {
#[serde(skip_serializing_if = "Option::is_none")]
pub id: Option<String>,
#[serde(skip_serializing_if = "Option::is_none")]
pub ignore_unknown_instances: Option<bool>,
@@ -122,6 +140,7 @@ impl DirectoryMetadata {
self.apply_ignore_unknown_instances(snapshot);
self.apply_class_name(snapshot)?;
self.apply_properties(snapshot)?;
self.apply_id(snapshot)?;
Ok(())
}
@@ -174,4 +193,15 @@ impl DirectoryMetadata {
Ok(())
}
fn apply_id(&mut self, snapshot: &mut InstanceSnapshot) -> anyhow::Result<()> {
if self.id.is_some() && snapshot.metadata.specified_id.is_some() {
anyhow::bail!(
"cannot specify an ID using {} (instance has an ID from somewhere else)",
self.path.display()
);
}
snapshot.metadata.specified_id = self.id.take().map(RojoRef::new);
Ok(())
}
}

5
src/snapshot_middleware/project.rs
View File

@@ -11,6 +11,7 @@ use crate::{
InstanceContext, InstanceMetadata, InstanceSnapshot, InstigatingSource, PathIgnoreRule,
SyncRule,
},
RojoRef,
};
use super::{emit_legacy_scripts_default, snapshot_from_vfs};
@@ -279,6 +280,10 @@ pub fn snapshot_project_node(
metadata.ignore_unknown_instances = true;
}
if let Some(id) = &node.id {
metadata.specified_id = Some(RojoRef::new(id.clone()))
}
metadata.instigating_source = Some(InstigatingSource::ProjectNode(
project_path.to_path_buf(),
instance_name.to_string(),

1
src/snapshot_middleware/snapshots/librojo__snapshot_middleware__csv__test__csv_from_vfs.snap
View File

@@ -12,6 +12,7 @@ metadata:
- /foo.meta.json
context:
emit_legacy_scripts: true
specified_id: ~
name: foo
class_name: LocalizationTable
properties:

1
src/snapshot_middleware/snapshots/librojo__snapshot_middleware__csv__test__csv_with_meta.snap
View File

@@ -12,6 +12,7 @@ metadata:
- /foo.meta.json
context:
emit_legacy_scripts: true
specified_id: ~
name: foo
class_name: LocalizationTable
properties:

1
src/snapshot_middleware/snapshots/librojo__snapshot_middleware__dir__test__empty_folder.snap
View File

@@ -19,6 +19,7 @@ metadata:
- /foo/init.csv
context:
emit_legacy_scripts: true
specified_id: ~
name: foo
class_name: Folder
properties: {}

2
src/snapshot_middleware/snapshots/librojo__snapshot_middleware__dir__test__folder_in_folder.snap
View File

@@ -19,6 +19,7 @@ metadata:
- /foo/init.csv
context:
emit_legacy_scripts: true
specified_id: ~
name: foo
class_name: Folder
properties: {}
@@ -40,6 +41,7 @@ children:
- /foo/Child/init.csv
context:
emit_legacy_scripts: true
specified_id: ~
name: Child
class_name: Folder
properties: {}

1
src/snapshot_middleware/snapshots/librojo__snapshot_middleware__json__test__instance_from_vfs.snap
View File

@@ -12,6 +12,7 @@ metadata:
- /foo.meta.json
context:
emit_legacy_scripts: true
specified_id: ~
name: foo
class_name: ModuleScript
properties:

2
src/snapshot_middleware/snapshots/librojo__snapshot_middleware__json_model__test__model_from_vfs.snap
View File

@@ -11,6 +11,7 @@ metadata:
- /foo.model.json
context:
emit_legacy_scripts: true
specified_id: ~
name: foo
class_name: IntValue
properties:
@@ -23,6 +24,7 @@ children:
relevant_paths: []
context:
emit_legacy_scripts: true
specified_id: ~
name: The Child
class_name: StringValue
properties: {}

2
src/snapshot_middleware/snapshots/librojo__snapshot_middleware__json_model__test__model_from_vfs_legacy.snap
View File

@@ -11,6 +11,7 @@ metadata:
- /foo.model.json
context:
emit_legacy_scripts: true
specified_id: ~
name: foo
class_name: IntValue
properties:
@@ -23,6 +24,7 @@ children:
relevant_paths: []
context:
emit_legacy_scripts: true
specified_id: ~
name: The Child
class_name: StringValue
properties: {}

1
src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__class_client_from_vfs.snap
View File

@@ -12,6 +12,7 @@ metadata:
- /foo.meta.json
context:
emit_legacy_scripts: true
specified_id: ~
name: foo
class_name: LocalScript
properties:

1
src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__class_module_from_vfs.snap
View File

@@ -12,6 +12,7 @@ metadata:
- /foo.meta.json
context:
emit_legacy_scripts: true
specified_id: ~
name: foo
class_name: ModuleScript
properties:

1
src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__class_module_with_meta.snap
View File

@@ -12,6 +12,7 @@ metadata:
- /foo.meta.json
context:
emit_legacy_scripts: true
specified_id: ~
name: foo
class_name: ModuleScript
properties:

1
src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__class_script_disabled.snap
View File

@@ -12,6 +12,7 @@ metadata:
- /bar.meta.json
context:
emit_legacy_scripts: true
specified_id: ~
name: bar
class_name: Script
properties:

1
src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__class_script_with_meta.snap
View File

@@ -12,6 +12,7 @@ metadata:
- /foo.meta.json
context:
emit_legacy_scripts: true
specified_id: ~
name: foo
class_name: Script
properties:

1
src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__class_server_from_vfs.snap
View File

@@ -12,6 +12,7 @@ metadata:
- /foo.meta.json
context:
emit_legacy_scripts: true
specified_id: ~
name: foo
class_name: Script
properties:

1
src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__runcontext_client_from_vfs.snap
View File

@@ -12,6 +12,7 @@ metadata:
- /foo.meta.json
context:
emit_legacy_scripts: false
specified_id: ~
name: foo
class_name: Script
properties:

1
src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__runcontext_module_from_vfs.snap
View File

@@ -12,6 +12,7 @@ metadata:
- /foo.meta.json
context:
emit_legacy_scripts: false
specified_id: ~
name: foo
class_name: ModuleScript
properties:

1
src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__runcontext_module_with_meta.snap
View File

@@ -12,6 +12,7 @@ metadata:
- /foo.meta.json
context:
emit_legacy_scripts: false
specified_id: ~
name: foo
class_name: ModuleScript
properties:

1
src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__runcontext_script_disabled.snap
View File

@@ -12,6 +12,7 @@ metadata:
- /bar.meta.json
context:
emit_legacy_scripts: false
specified_id: ~
name: bar
class_name: Script
properties:

1
src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__runcontext_script_with_meta.snap
View File

@@ -12,6 +12,7 @@ metadata:
- /foo.meta.json
context:
emit_legacy_scripts: false
specified_id: ~
name: foo
class_name: Script
properties:

1
src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__runcontext_server_from_vfs.snap
View File

@@ -12,6 +12,7 @@ metadata:
- /foo.meta.json
context:
emit_legacy_scripts: false
specified_id: ~
name: foo
class_name: Script
properties:

4
src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__no_name_project.snap
View File

@@ -1,6 +1,6 @@
---
source: src/snapshot_middleware/project.rs
assertion_line: 725
assertion_line: 730
expression: instance_snapshot
---
snapshot_id: "00000000000000000000000000000000"
@@ -12,8 +12,8 @@ metadata:
- /foo/default.project.json
context:
emit_legacy_scripts: true
specified_id: ~
name: no_name_project
class_name: Model
properties: {}
children: []

1
src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_from_direct_file.snap
View File

@@ -11,6 +11,7 @@ metadata:
- /foo/hello.project.json
context:
emit_legacy_scripts: true
specified_id: ~
name: direct-project
class_name: Model
properties: {}

1
src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_path_property_overrides.snap
View File

@@ -12,6 +12,7 @@ metadata:
- /foo/default.project.json
context:
emit_legacy_scripts: true
specified_id: ~
name: path-property-override
class_name: StringValue
properties:

2
src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_children.snap
View File

@@ -11,6 +11,7 @@ metadata:
- /foo.project.json
context:
emit_legacy_scripts: true
specified_id: ~
name: children
class_name: Folder
properties: {}
@@ -27,6 +28,7 @@ children:
relevant_paths: []
context:
emit_legacy_scripts: true
specified_id: ~
name: Child
class_name: Model
properties: {}

1
src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_path_to_project.snap
View File

@@ -12,6 +12,7 @@ metadata:
- /foo/default.project.json
context:
emit_legacy_scripts: true
specified_id: ~
name: path-project
class_name: Model
properties: {}

2
src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_path_to_project_with_children.snap
View File

@@ -12,6 +12,7 @@ metadata:
- /foo/default.project.json
context:
emit_legacy_scripts: true
specified_id: ~
name: path-child-project
class_name: Folder
properties: {}
@@ -28,6 +29,7 @@ children:
relevant_paths: []
context:
emit_legacy_scripts: true
specified_id: ~
name: SomeChild
class_name: Model
properties: {}

1
src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_path_to_txt.snap
View File

@@ -13,6 +13,7 @@ metadata:
- /foo/default.project.json
context:
emit_legacy_scripts: true
specified_id: ~
name: path-project
class_name: StringValue
properties:

1
src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_resolved_properties.snap
View File

@@ -11,6 +11,7 @@ metadata:
- /foo.project.json
context:
emit_legacy_scripts: true
specified_id: ~
name: resolved-properties
class_name: StringValue
properties:

1
src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_unresolved_properties.snap
View File

@@ -11,6 +11,7 @@ metadata:
- /foo.project.json
context:
emit_legacy_scripts: true
specified_id: ~
name: unresolved-properties
class_name: StringValue
properties:

1
src/snapshot_middleware/snapshots/librojo__snapshot_middleware__toml__test__instance_from_vfs.snap
View File

@@ -12,6 +12,7 @@ metadata:
- /foo.meta.json
context:
emit_legacy_scripts: true
specified_id: ~
name: foo
class_name: ModuleScript
properties:

1
src/snapshot_middleware/snapshots/librojo__snapshot_middleware__txt__test__instance_from_vfs.snap
View File

@@ -12,6 +12,7 @@ metadata:
- /foo.meta.json
context:
emit_legacy_scripts: true
specified_id: ~
name: foo
class_name: StringValue
properties:

3
src/web/ui.rs
View File

@@ -163,6 +163,7 @@ impl UiService {
let content = html! {
<>
<div>"specified_id: " { format!("{:?}", metadata.specified_id) } </div>
<div>"ignore_unknown_instances: " { metadata.ignore_unknown_instances.to_string() }</div>
<div>"instigating source: " { format!("{:?}", metadata.instigating_source) }</div>
{ relevant_paths }
@@ -192,7 +193,7 @@ impl UiService {
html! {
<div class="instance">
<label class="instance-title" for={ format!("instance-{:?}", id) }>
<label class="instance-title" for={ format!("instance-{:?}", id) } title={ format!("ref: {:?}", instance.id())}>
{ instance.name().to_owned() }
{ class_name_specifier }
</label>

85
tests/tests/serve.rs
View File

@@ -360,3 +360,88 @@ fn no_name_top_level_project() {
);
});
}
#[test]
fn ref_properties() {
run_serve_test("ref_properties", |session, mut redactions| {
let info = session.get_api_rojo().unwrap();
let root_id = info.root_instance_id;
assert_yaml_snapshot!("ref_properties_info", redactions.redacted_yaml(info));
let read_response = session.get_api_read(root_id).unwrap();
assert_yaml_snapshot!(
"ref_properties_all",
read_response.intern_and_redact(&mut redactions, root_id)
);
fs::write(
session.path().join("ModelTarget.model.json"),
r#"{
"className": "Folder",
"attributes": {
"Rojo_Id": "model target 2"
},
"children": [
{
"name": "ModelPointer",
"className": "Model",
"attributes": {
"Rojo_Target_PrimaryPart": "model target 2"
}
},
{
"name": "ProjectPointer",
"className": "Model",
"attributes": {
"Rojo_Target_PrimaryPart": "project target"
}
}
]
}"#,
)
.unwrap();
let subscribe_response = session.get_api_subscribe(0).unwrap();
assert_yaml_snapshot!(
"ref_properties_subscribe",
subscribe_response.intern_and_redact(&mut redactions, ())
);
let read_response = session.get_api_read(root_id).unwrap();
assert_yaml_snapshot!(
"ref_properties_all-2",
read_response.intern_and_redact(&mut redactions, root_id)
);
});
}
#[test]
fn ref_properties_remove() {
run_serve_test("ref_properties_remove", |session, mut redactions| {
let info = session.get_api_rojo().unwrap();
let root_id = info.root_instance_id;
assert_yaml_snapshot!("ref_properties_remove_info", redactions.redacted_yaml(info));
let read_response = session.get_api_read(root_id).unwrap();
assert_yaml_snapshot!(
"ref_properties_remove_all",
read_response.intern_and_redact(&mut redactions, root_id)
);
fs::remove_file(session.path().join("src/target.model.json")).unwrap();
let subscribe_response = session.get_api_subscribe(0).unwrap();
assert_yaml_snapshot!(
"ref_properties_remove_subscribe",
subscribe_response.intern_and_redact(&mut redactions, ())
);
let read_response = session.get_api_read(root_id).unwrap();
assert_yaml_snapshot!(
"ref_properties_remove_all-2",
read_response.intern_and_redact(&mut redactions, root_id)
);
});
}