masi/solvecraft
1
0
Fork 0
mirror of https://github.com/bherbruck/solidcraft.git synced 2026-11-03 18:45:55 +00:00
Code Issues Packages Projects Releases 1 Wiki Activity

Prevent deleted item IDs from being reused (#1016) (#3641)

Browse Source 
* claim: #1016

* Prevent deleted design item IDs from being reused
This commit is contained in:
Colin Faulkingham
2026-10-22 18:31:39 +00:00
committed by GitHub
parent aaa110218b
commit 3b982ca371
19 changed files with 276 additions and 72 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/doc/src/document.rs
+27
View File
@@ -2626,6 +2626,33 @@ impl Default for Document {
}
impl Document {
/// The next shared identity, including auxiliary lists written by older versions or
/// populated programmatically. Reading it does not reserve an id (used by joint previews).
pub fn next_item_id(&self) -> Result<u64> {
let a = &self.assembly;
let largest = self
.browser_groups
.iter()
.map(|g| g.id)
.chain(self.timeline_groups.iter().map(|g| g.id))
.chain(self.canvases.iter().map(|c| c.id))
.chain(a.joints.iter().map(|j| j.id))
.chain(a.contact_sets.iter().map(|c| c.id))
.chain(a.studies.iter().map(|s| s.id))
.chain(a.exploded.iter().map(|v| v.id))
.max()
.unwrap_or(0);
let floor = largest.checked_add(1).ok_or_else(|| DocError::Invalid("too many items in the design".into()))?;
Ok(self.next_id.max(floor).max(1))
}
/// Reserve an identity in the saved counter. Removing an item never rewinds this counter.
pub fn allocate_id(&mut self) -> Result<u64> {
let id = self.next_item_id()?;
self.next_id = id.checked_add(1).ok_or_else(|| DocError::Invalid("too many items in the design".into()))?;
Ok(id)
}
pub fn new(name: &str) -> Self {
Document {
render: Default::default(),
crates/doc/src/format.rs
+43
View File
@@ -674,6 +674,9 @@ pub fn read(s: &str) -> Result<Document> {
d.features.iter().map(|f| f.id).chain(d.components.iter().map(|c| c.id)).chain(d.occurrences.iter().map(|o| o.id)).max().unwrap_or(0);
let floor = largest.checked_add(1).ok_or_else(|| DocError::Invalid("damaged design: the id space is exhausted".into()))?;
d.next_id = d.next_id.max(floor).max(1);
// These lists used to allocate independently. Keep their existing identities, but never
// let a later deletion make them available again after loading an older design.
d.next_id = d.next_item_id()?;
d.format = format_string(FORMAT);
d.format_pending = pending_names();
Ok(d)
@@ -2038,6 +2041,46 @@ mod tests {
}
}
#[test]
fn older_auxiliary_ids_advance_the_saved_counter_before_deletion() {
use serde_json::json;
let origin = json!({"occurrence": 0, "snap": {"type": "point", "point": [0, 0, 0]}});
let cases = [
("browser_groups", json!({"id": 100, "name": "Group", "component": 0, "folder": "bodies", "items": []})),
("timeline_groups", json!({"id": 100, "name": "Folder", "features": []})),
(
"canvases",
json!({"id": 100, "name": "Canvas", "format": "png", "data": "", "pixels": [1, 1],
"plane": {"type": "origin", "name": "XY"}, "center": [0, 0], "width": 10,
"angle": 0, "opacity": 0.5, "flip": false, "visible": true}),
),
("joints", json!({"id": 100, "name": "Joint", "kind": "rigid", "a": origin, "b": origin})),
("contact_sets", json!({"id": 100, "name": "Contact", "occurrences": []})),
("studies", json!({"id": 100, "name": "Study", "steps": 10, "keys": []})),
("exploded", json!({"id": 100, "name": "View", "moves": []})),
];
for (field, item) in cases {
let json = edited(&Document::new("Legacy IDs"), |v| {
v.as_object_mut().unwrap().remove("next_id");
if matches!(field, "browser_groups" | "timeline_groups" | "canvases") {
v[field] = json!([item]);
} else {
v["assembly"] = json!({field: [item]});
}
});
let mut back = read(&json).unwrap_or_else(|e| panic!("{field}: {e}"));
assert_eq!(back.next_id, 101, "{field}");
back.browser_groups.clear();
back.timeline_groups.clear();
back.canvases.clear();
back.assembly = Default::default();
let mut reopened = read(&back.to_json()).unwrap();
assert_eq!(reopened.allocate_id().unwrap(), 101, "{field}: deleted legacy id was forgotten");
assert_eq!(reopened.allocate_id().unwrap(), 102, "{field}");
}
}
#[test]
fn duplicate_component_or_occurrence_ids_are_a_damaged_design() {
let d = two_components_one_occurrence();
crates/engine/src/cmd/browser.rs
+21 -7
View File
@@ -89,7 +89,7 @@ fn group(s: &mut Session, p: &Value) -> Result<Value> {
let component = component_of(s, p, cmd)?;
let items = keys(p, "items", cmd)?;
let d = s.doc_mut();
let id = super::next_free_id(cmd, d.browser_groups.iter().map(|g| g.id))?;
let id = d.allocate_id()?;
let name = match str_(p, "name").map(str::trim).filter(|n| !n.is_empty()) {
Some(n) if n.len() <= 128 => n.to_string(),
Some(_) => return Err(bad(cmd, "`name` is too long")),
@@ -1097,12 +1097,26 @@ mod tests {
}
#[test]
fn the_next_free_id_is_one_past_the_largest_and_never_wraps() {
use super::super::next_free_id;
assert_eq!(next_free_id("t", std::iter::empty()).unwrap(), 1);
assert_eq!(next_free_id("t", [3, 9, 4].into_iter()).unwrap(), 10);
assert_eq!(next_free_id("t", [u64::MAX - 1].into_iter()).unwrap(), u64::MAX);
assert!(next_free_id("t", [1, u64::MAX].into_iter()).is_err());
fn exhausted_shared_ids_refuse_a_group_without_changing_the_design() {
let mut s = Session::default();
s.doc_mut().next_id = u64::MAX;
let before = s.doc.to_json();
let result = s.execute("browser.group", &json!({"folder": "bodies"}));
assert!(result.is_err() && !matches!(result, Err(crate::EngineError::Internal(..))), "{result:?}");
assert_eq!(s.doc.to_json(), before);
}
#[test]
fn deleted_group_ids_are_not_reused_after_reopening() {
let mut s = Session::default();
let params = json!({"folder": "bodies"});
let first = run(&mut s, "browser.group", params.clone())["group"].as_u64().unwrap();
run(&mut s, "browser.ungroup", json!({"group": first}));
let doc = solidcraft_doc::Document::from_json(&s.doc.to_json()).unwrap();
*s.doc_mut() = doc;
let second = run(&mut s, "browser.group", params)["group"].as_u64().unwrap();
assert!(second > first, "deleted group {first} was reused as {second}");
assert!(s.execute("browser.rename_group", &json!({"group": first, "name": "Stale reference"})).is_err());
}
#[test]
crates/engine/src/cmd/component_clone_assembly.rs
+9 -6
View File
@@ -269,12 +269,14 @@ pub(super) fn clone_assembly(
return Err(invalid("copy occurrence map points to a missing occurrence"));
}
unique_ids(source.assembly.joints.iter().map(|j| j.id))?;
let mut joint_id = unique_ids(doc.assembly.joints.iter().map(|j| j.id))?;
unique_ids(doc.assembly.joints.iter().map(|j| j.id))?;
let aux = |a: &Assembly| {
a.contact_sets.iter().map(|x| x.id).chain(a.studies.iter().map(|x| x.id)).chain(a.exploded.iter().map(|x| x.id)).collect::<Vec<_>>()
};
unique_ids(aux(&source.assembly).into_iter())?;
let mut auxiliary_id = unique_ids(aux(&doc.assembly).into_iter())?;
unique_ids(aux(&doc.assembly).into_iter())?;
// Reserve from one persistent counter, committing it only when the entire copy succeeds.
let mut item_id = doc.next_item_id()?.saturating_sub(1);
let mut a = doc.assembly.clone();
let mut messages = Vec::new();
let mut origin_names = a.origins.iter().map(|o| o.name.clone()).collect();
@@ -302,7 +304,7 @@ pub(super) fn clone_assembly(
match (origin(&old.a, occurrences, features, bodies), origin(&old.b, occurrences, features, bodies)) {
(Some(aa), Some(bb)) => {
let mut item = old.clone();
item.id = next_id(&mut joint_id)?;
item.id = next_id(&mut item_id)?;
item.name = unique_name(&old.name, &mut joint_names)?;
item.a = aa;
item.b = bb;
@@ -354,7 +356,7 @@ pub(super) fn clone_assembly(
}
if let Some(ids) = old.occurrences.iter().map(|o| occurrences.get(o).copied()).collect::<Option<Vec<_>>>() {
let mut item = old.clone();
item.id = next_id(&mut auxiliary_id)?;
item.id = next_id(&mut item_id)?;
item.name = unique_name(&old.name, &mut contact_names)?;
item.occurrences = ids;
a.contact_sets.push(item);
@@ -369,7 +371,7 @@ pub(super) fn clone_assembly(
}
if old.keys.iter().all(|k| joints.contains_key(&k.joint)) {
let mut item = old.clone();
item.id = next_id(&mut auxiliary_id)?;
item.id = next_id(&mut item_id)?;
item.name = unique_name(&old.name, &mut study_names)?;
for key in &mut item.keys {
key.joint = *joints.get(&key.joint).ok_or_else(|| invalid("missing copied study joint"))?;
@@ -386,7 +388,7 @@ pub(super) fn clone_assembly(
}
if old.moves.iter().all(|(o, _)| occurrences.contains_key(o)) {
let mut item = old.clone();
item.id = next_id(&mut auxiliary_id)?;
item.id = next_id(&mut item_id)?;
item.name = unique_name(&old.name, &mut view_names)?;
for (o, _) in &mut item.moves {
*o = *occurrences.get(o).ok_or_else(|| invalid("missing copied exploded occurrence"))?;
@@ -397,6 +399,7 @@ pub(super) fn clone_assembly(
}
}
budget(&a)?;
doc.next_id = item_id.checked_add(1).ok_or_else(|| invalid("assembly identifier space is exhausted"))?;
doc.assembly = a;
Ok(messages)
}
crates/engine/src/cmd/component_clone_assembly_tests.rs
+10 -8
View File
@@ -76,12 +76,13 @@ fn documents() -> (Document, Document) {
#[test]
fn internal_relationships_receive_independent_ids_and_remapped_typed_references() {
let (source, mut destination) = documents();
destination.next_id = 1000;
let report =
clone_assembly(&mut destination, &source, &BTreeMap::from([(1, 101), (2, 102)]), &BTreeMap::from([(1, 11), (10, 110)]), &BTreeMap::new())
.unwrap();
let a = &destination.assembly;
assert_eq!(a.joints.len(), 6);
assert_eq!((a.joints[4].id, a.joints[5].id), (5, 6));
assert_eq!((a.joints[4].id, a.joints[5].id), (1000, 1001));
assert_eq!((a.joints[4].a.occurrence, a.joints[4].b.occurrence), (101, 102));
assert_eq!(a.joints[4].values, source.assembly.joints[0].values);
assert_eq!(a.joints[4].limits, source.assembly.joints[0].limits);
@@ -90,13 +91,14 @@ fn internal_relationships_receive_independent_ids_and_remapped_typed_references(
// Its x reference's face is the copy's too.
assert!(matches!(&a.origins[3].origin.snap, Snap::FaceCenter { face: Some(face), x: Some(XRef::Face { face: x, .. }), .. }
if face == "F11:start@F110.2" && x == "F110:side"));
assert_eq!((a.links[2].a, a.links[2].b, a.links[2].ratio), (5, 6, -2.0));
assert_eq!(a.contact_sets[2].id, 7);
assert_eq!((a.links[2].a, a.links[2].b, a.links[2].ratio), (1000, 1001, -2.0));
assert_eq!(a.contact_sets[2].id, 1002);
assert_eq!(a.contact_sets[2].occurrences, vec![101, 102]);
assert!(a.contact_sets[2].suppressed);
assert_eq!(a.studies[2].id, 8);
assert_eq!(a.studies[2].keys.iter().map(|k| k.joint).collect::<Vec<_>>(), vec![5, 6]);
assert_eq!(a.exploded[2].id, 9);
assert_eq!(a.studies[2].id, 1003);
assert_eq!(a.studies[2].keys.iter().map(|k| k.joint).collect::<Vec<_>>(), vec![1000, 1001]);
assert_eq!(a.exploded[2].id, 1004);
assert_eq!(destination.next_id, 1005);
assert_eq!(a.exploded[2].moves, vec![(101, Vec3::X * 10.0), (102, Vec3::Y * 20.0)]);
assert_eq!(report.len(), 6);
for kind in ["joint origin", "joint `Joint3`", "motion link", "contact set", "motion study", "exploded view"] {
@@ -171,9 +173,9 @@ fn malformed_maps_identifier_exhaustion_and_aggregate_budgets_are_atomic() {
invalid.assembly.links[0].ratio = f64::NAN;
cases.push((invalid, destination, BTreeMap::from([(1, 101), (2, 102)])));
for (source, mut destination, occurrences) in cases {
let before = destination.assembly.clone();
let before = destination.clone();
assert!(clone_assembly(&mut destination, &source, &occurrences, &BTreeMap::from([(1, 11), (10, 110)]), &BTreeMap::new()).is_err());
assert_eq!(destination.assembly, before);
assert_eq!(destination, before);
}
}
crates/engine/src/cmd/edit.rs
+1 -9
View File
@@ -330,15 +330,7 @@ fn group(s: &mut Session, p: &Value) -> Result<Value> {
Some(_) => return Err(bad(cmd, "`name` must be 1…128 characters")),
None => format!("Folder{}", s.doc.timeline_groups.len() + 1),
};
let id = s
.doc
.timeline_groups
.iter()
.map(|group| group.id)
.max()
.unwrap_or(0)
.checked_add(1)
.ok_or_else(|| bad(cmd, "no more timeline folder ids are available"))?;
let id = s.doc_mut().allocate_id()?;
let parent = match p.get("parent") {
Some(Value::Null) | None => None,
Some(_) => Some(timeline_group_id_field(s, p, "parent", cmd)?),
crates/engine/src/cmd/edit_tests.rs
+14
View File
@@ -503,6 +503,20 @@ fn timeline_folders_refuse_unknown_non_contiguous_and_split_contents() {
assert!(error.contains("not contiguous"), "{error}");
}
#[test]
fn deleted_timeline_group_ids_are_not_reused_after_reopening() {
let mut s = Session::default();
run(&mut s, "solid.box", json!({"length": 10, "width": 10, "height": 10, "name": "Block"}));
let params = json!({"features": ["Block"]});
let first = run(&mut s, "timeline.group", params.clone())["group"].as_u64().unwrap();
run(&mut s, "timeline.ungroup", json!({"group": first}));
let doc = solidcraft_doc::Document::from_json(&s.doc.to_json()).unwrap();
*s.doc_mut() = doc;
let second = run(&mut s, "timeline.group", params)["group"].as_u64().unwrap();
assert!(second > first, "deleted folder {first} was reused as {second}");
assert!(s.execute("timeline.rename_group", &json!({"group": first, "name": "Stale reference"})).is_err());
}
#[test]
fn timeline_move_many_is_atomic_and_one_undo_step() {
let mut s = Session::default();
crates/engine/src/cmd/file_import.rs
+4 -5
View File
@@ -344,17 +344,16 @@ fn add_rigid_joints(s: &mut Session, pairs: &[(u64, u64, Mat)]) -> Result<Vec<u6
// Too many to hold together: the parts are placed, not locked.
return Ok(Vec::new());
}
let mut id = s.doc.assembly.joints.iter().map(|j| j.id).max().unwrap_or(0);
let mut ids = Vec::with_capacity(pairs.len());
let point = |m: &Mat, p: Vec3| solidcraft_doc::apply_point(m, p);
let vector = |m: &Mat, v: Vec3| solidcraft_doc::apply_vector(m, v);
let made: Vec<Joint> = pairs
.iter()
.map(|(a, b, m)| {
id += 1;
let id = s.doc_mut().allocate_id()?;
ids.push(id);
// As built: B's origin, seen from A (B's placement in A) and from B itself.
Joint {
Ok(Joint {
id,
name: format!("Rigid Group {a}:{b}"),
kind: JointKind::Rigid,
@@ -370,9 +369,9 @@ fn add_rigid_joints(s: &mut Session, pairs: &[(u64, u64, Mat)]) -> Result<Vec<u6
angle: 0.0,
suppressed: false,
exprs: Default::default(),
}
})
})
.collect();
.collect::<Result<_>>()?;
s.doc_mut().assembly.joints.extend(made);
// Dozens of coinciding glyphs say nothing: the group shows in the browser instead.
s.visibility.hidden_joints.retain(|h| !ids.contains(h));
crates/engine/src/cmd/joints.rs
+13 -9
View File
@@ -576,10 +576,6 @@ fn limits_param(s: &Session, p: &Value, kind: JointKind, cmd: &str) -> Result<Ve
Ok(out)
}
fn next_joint_id(s: &Session, cmd: &str) -> Result<u64> {
super::next_free_id(cmd, s.doc.assembly.joints.iter().map(|j| j.id))
}
fn add_joint(s: &mut Session, mut j: Joint, p: &Value) -> Result<Value> {
if s.doc.assembly.joints.len() >= 10_000 {
return Err(EngineError::Other("too many joints".into()));
@@ -619,7 +615,7 @@ fn add_joint(s: &mut Session, mut j: Joint, p: &Value) -> Result<Value> {
j.values = q;
let id = j.id;
let name = j.name.clone();
// A new joint can take a deleted one's id; it starts shown.
// A new joint starts shown, even if a stale visibility entry came from an older design.
s.visibility.hidden_joints.retain(|h| *h != id);
s.doc_mut().assembly.joints.push(j);
let sol = resolve(s);
@@ -652,13 +648,21 @@ fn kind_param(p: &Value, cmd: &str) -> Result<JointKind> {
.ok_or_else(|| bad(cmd, "`type` must be rigid, revolute, slider, cylindrical, pin_slot, planar, ball, parallel, perpendicular, angle, point_on_plane, point_on_line or line_on_plane"))
}
fn new_joint_name(s: &Session, p: &Value) -> String {
str_(p, "name").map(str::to_string).unwrap_or_else(|| {
let names: std::collections::BTreeSet<_> = s.doc.assembly.joints.iter().map(|j| j.name.as_str()).collect();
let number = (1..=s.doc.assembly.joints.len().saturating_add(1)).find(|n| !names.contains(format!("Joint{n}").as_str())).unwrap_or(1);
format!("Joint{number}")
})
}
fn joint(s: &mut Session, p: &Value) -> Result<Value> {
let cmd = "joint.create";
let kind = kind_param(p, cmd)?;
let a = origin_param(s, p.get("a"), cmd)?;
let b = origin_param(s, p.get("b"), cmd)?;
let id = next_joint_id(s, cmd)?;
let name = str_(p, "name").map(str::to_string).unwrap_or_else(|| format!("Joint{id}"));
let id = s.doc_mut().allocate_id()?;
let name = new_joint_name(s, p);
let offset = match p.get("offset") {
Some(v) => value(s, v, false, cmd)?,
None => 0.0,
@@ -716,8 +720,8 @@ fn as_built(s: &mut Session, p: &Value) -> Result<Value> {
};
let fa = side(oa)?;
let fb = side(ob)?;
let id = next_joint_id(s, cmd)?;
let name = str_(p, "name").map(str::to_string).unwrap_or_else(|| format!("Joint{id}"));
let id = s.doc_mut().allocate_id()?;
let name = new_joint_name(s, p);
let n = kind.dofs().len();
let j = Joint {
id,
crates/engine/src/cmd/joints_tests.rs
+20
View File
@@ -25,6 +25,26 @@ fn near(a: Vec3, b: Vec3) -> bool {
a.dist(b) < 1e-6
}
#[test]
fn deleted_joint_ids_are_not_reused_after_reopening() {
for command in ["joint.create", "joint.as_built"] {
let mut s = Session::default();
run(&mut s, "component.create", json!({"name": "Part"}));
let params = if command == "joint.create" {
json!({"type": "revolute", "a": {"occurrence": 0, "point": [0, 0, 0]}, "b": {"occurrence": "Part", "point": [0, 0, 0]}})
} else {
json!({"type": "revolute", "a": 0, "b": "Part"})
};
let first = run(&mut s, command, params.clone())["joint"].as_u64().unwrap();
run(&mut s, "joint.delete", json!({"joint": first}));
let doc = solidcraft_doc::Document::from_json(&s.doc.to_json()).unwrap();
*s.doc_mut() = doc;
let second = run(&mut s, command, params)["joint"].as_u64().unwrap();
assert!(second > first, "{command}: deleted joint {first} was reused as {second}");
assert!(s.execute("joint.drive", &json!({"joint": first, "value": 45})).is_err());
}
}
/// A hinge leaf (oracle 63's knuckle geometry): a 40 x 30 x 3 plate and an r4 knuckle along X
/// at y = 33, z = 4.
fn leaf(s: &mut Session, name: &str) {
crates/engine/src/cmd/mod.rs
-7
View File
@@ -313,13 +313,6 @@ pub fn find_command(id: &str) -> Option<&'static CommandSpec> {
all.iter().find(|c| c.id == id).or_else(|| all.iter().find(|c| c.id.eq_ignore_ascii_case(id))).copied()
}
/// The id for a new item of a list: one past the largest id in it. Ids come from the design file,
/// so the largest may be the largest a `u64` holds, and one past that is an error, not an overflow
/// (a panic where overflow is checked, an id that is already taken where it is not).
pub(crate) fn next_free_id(cmd: &str, ids: impl Iterator<Item = u64>) -> Result<u64> {
ids.max().unwrap_or(0).checked_add(1).ok_or_else(|| crate::params::bad(cmd, "this design has no ids left to give"))
}
/// The end of an unknown command's error: up to three ids like `id`, nearest first. Ids a few
/// letters away (`solid.extrud` → `solid.extrude`) come before ids that end in the same word
/// (`file.open` → `doc.open`). Empty when nothing is close.
crates/engine/src/cmd/motion.rs
+9 -11
View File
@@ -98,12 +98,6 @@ pub static COMMANDS: &[CommandSpec] = &[
const MAX_ITEMS: usize = 10_000;
fn next_id(s: &Session, cmd: &str) -> Result<u64> {
let a = &s.doc.assembly;
let ids = a.contact_sets.iter().map(|c| c.id).chain(a.studies.iter().map(|m| m.id)).chain(a.exploded.iter().map(|e| e.id));
super::next_free_id(cmd, ids)
}
fn occurrences(s: &Session, p: &Value, cmd: &str) -> Result<Vec<u64>> {
let list = p.get("occurrences").and_then(Value::as_array).ok_or_else(|| bad(cmd, "`occurrences` must list occurrences"))?;
if list.len() > MAX_ITEMS {
@@ -477,7 +471,7 @@ fn new_set(s: &mut Session, p: &Value) -> Result<Value> {
if s.doc.assembly.contact_sets.len() >= MAX_ITEMS {
return Err(bad(cmd, "too many contact sets"));
}
let id = next_id(s, cmd)?;
let id = s.doc_mut().allocate_id()?;
let name = str_(p, "name").map(str::to_string).unwrap_or_else(|| format!("Contact Set{}", s.doc.assembly.contact_sets.len() + 1));
let a = &mut s.doc_mut().assembly;
a.contact_sets.push(ContactSet { id, name: name.clone(), occurrences: occ, suppressed: false });
@@ -585,9 +579,13 @@ fn study(s: &mut Session, p: &Value) -> Result<Value> {
keys.push(StudyKey { joint, index, points });
}
}
let id = next_id(s, cmd)?;
let count = s.doc.assembly.studies.len();
let a = &mut s.doc_mut().assembly;
let doc = s.doc_mut();
let id = match existing {
Some(i) => doc.assembly.studies.get(i).ok_or_else(|| bad(cmd, "no such motion study"))?.id,
None => doc.allocate_id()?,
};
let count = doc.assembly.studies.len();
let a = &mut doc.assembly;
let out = match existing.and_then(|i| a.studies.get_mut(i)) {
Some(m) => {
if let Some(n) = steps {
@@ -1003,7 +1001,7 @@ fn explode_create(s: &mut Session, p: &Value) -> Result<Value> {
if s.doc.assembly.exploded.len() >= MAX_ITEMS {
return Err(bad(cmd, "too many exploded views"));
}
let id = next_id(s, cmd)?;
let id = s.doc_mut().allocate_id()?;
let name = str_(p, "name").map(str::to_string).unwrap_or_else(|| format!("Exploded View{}", s.doc.assembly.exploded.len() + 1));
s.doc_mut().assembly.exploded.push(ExplodedView { id, name: name.clone(), moves: moves.clone() });
Ok(json!({"view": id, "name": name, "moves": moves.iter().map(|(o, t)| json!({"occurrence": o, "translate": t})).collect::<Vec<_>>()}))
crates/engine/src/cmd/motion_tests.rs
+23
View File
@@ -15,6 +15,29 @@ fn occ(s: &Session, component: &str) -> u64 {
s.doc.occurrence_of(c).map(|o| o.id).unwrap_or(0)
}
#[test]
fn deleted_motion_item_ids_are_not_reused_after_reopening() {
for (create, delete, key, params) in [
("contact.create", "contact.edit", "set", json!({"occurrences": ["A", "B"]})),
("motion.study", "motion.delete", "study", json!({"name": "Travel", "steps": 10})),
("explode.create", "explode.delete", "view", json!({"moves": [{"occurrence": "B", "translate": [20, 0, 0]}]})),
] {
let mut s = Session::default();
for name in ["A", "B"] {
run(&mut s, "component.activate", json!({"component": "root"}));
run(&mut s, "component.create", json!({"name": name}));
}
let first = run(&mut s, create, params.clone())[key].as_u64().unwrap();
let deletion = json!({key: first, "delete": true});
run(&mut s, delete, deletion.clone());
let doc = solidcraft_doc::Document::from_json(&s.doc.to_json()).unwrap();
*s.doc_mut() = doc;
let second = run(&mut s, create, params)[key].as_u64().unwrap();
assert!(second > first, "{create}: deleted item {first} was reused as {second}");
assert!(s.execute(delete, &deletion).is_err());
}
}
/// Where B's corner (20, 0, 0) is now.
fn b_x(s: &Session) -> f64 {
super::world_point(s, occ(s, "B"), Vec3::new(20.0, 0.0, 0.0)).x
crates/engine/src/cmd/sketch_canvas.rs
+19 -2
View File
@@ -56,9 +56,13 @@ fn make(s: &mut Session, p: &Value, cmd: &str, decal: bool) -> Result<Value> {
// It belongs to the active component (its plane is in that frame) and moves with it.
let component = s.active_component;
let doc = s.doc_mut();
let id = super::next_free_id(cmd, doc.canvases.iter().map(|c| c.id))?;
let id = doc.allocate_id()?;
let base = if decal { "Decal" } else { "Canvas" };
let name = str_(p, "name").map(str::to_string).unwrap_or_else(|| format!("{base}{id}"));
let name = str_(p, "name").map(str::to_string).unwrap_or_else(|| {
let names: std::collections::BTreeSet<_> = doc.canvases.iter().map(|c| c.name.as_str()).collect();
let number = (1..=doc.canvases.len().saturating_add(1)).find(|n| !names.contains(format!("{base}{n}").as_str())).unwrap_or(1);
format!("{base}{number}")
});
doc.canvases.push(Canvas {
id,
name: name.clone(),
@@ -170,6 +174,19 @@ mod tests {
base64_encode(&b)
}
#[test]
fn deleted_canvas_ids_are_not_reused_after_reopening() {
let mut s = Session::default();
let params = json!({"data": png(10, 10)});
let first = s.execute("canvas.insert", &params).unwrap()["canvas"].as_u64().unwrap();
s.execute("canvas.delete", &json!({"canvas": first})).unwrap();
let doc = solidcraft_doc::Document::from_json(&s.doc.to_json()).unwrap();
*s.doc_mut() = doc;
let second = s.execute("canvas.insert", &params).unwrap()["canvas"].as_u64().unwrap();
assert!(second > first, "deleted canvas {first} was reused as {second}");
assert!(s.execute("canvas.edit", &json!({"canvas": first, "width": 500})).is_err());
}
#[test]
fn canvas_insert_edit_calibrate_save() {
let mut s = Session::default();
crates/engine/src/tests/step_import_options.rs
+19
View File
@@ -64,6 +64,25 @@ fn total_volume(s: &mut Session) -> f64 {
run(s, "inspect.measure", json!({}))["total"]["volume_mm3"].as_f64().unwrap()
}
#[test]
fn imported_joints_do_not_reuse_deleted_ids() {
let path = frame_file("joint-ids");
let mut s = Session::default();
run(&mut s, "file.insert_step", json!({"path": path}));
let first: Vec<_> = s.doc.assembly.joints.iter().map(|j| j.id).collect();
assert!(!first.is_empty());
for id in &first {
run(&mut s, "joint.delete", json!({"joint": id}));
}
run(&mut s, "component.activate", json!({"component": "root"}));
run(&mut s, "file.insert_step", json!({"path": path}));
assert!(!s.doc.assembly.joints.is_empty());
for joint in &s.doc.assembly.joints {
assert!(!first.contains(&joint.id), "import reused deleted joint {}", joint.id);
assert!(joint.id < s.doc.next_id, "import did not reserve joint {}", joint.id);
}
}
/// `up: "y"` turns the file's bodies a quarter turn about X, Y becoming Z: the frame's height
/// lies along Z, and what was its depth now runs towards -Y.
#[test]
crates/io/src/design.rs
+2 -1
View File
@@ -995,8 +995,9 @@ mod tests {
other.name = "Same triangles, separate appearance".into();
other.color = None;
doc.add_feature(FeatureKind::MeshImport { file: "example.obj".into(), meshes: vec![mesh(), other] }, None).unwrap();
let id = doc.allocate_id().unwrap();
doc.canvases.push(solidcraft_doc::canvas::Canvas {
id: 99,
id,
name: "Reference".into(),
format: "png".into(),
data: solidcraft_doc::canvas::base64_encode(b"original image bytes"),
crates/ui-egui/src/dialogs_assembly.rs
+1 -5
View File
@@ -1007,7 +1007,7 @@ pub fn animate(app: &SolveApp, k: &Asm, cmds: &mut Vec<(String, Value)>) {
None
}
// As-built joints start at zero: drive the new one after it is made.
None => Some(next_joint_id(&app.session)),
None => app.session.doc.next_item_id().ok(),
},
};
if let Some(id) = drive {
@@ -1015,10 +1015,6 @@ pub fn animate(app: &SolveApp, k: &Asm, cmds: &mut Vec<(String, Value)>) {
}
}
fn next_joint_id(s: &Session) -> u64 {
s.doc.assembly.joints.iter().map(|j| j.id).max().unwrap_or(0) + 1
}
/// Joint and drive previews: the commands run on a scratch copy and the placed model before and
/// after is compared, so the components that move show where they go (`None`: not such
/// commands).
crates/ui-egui/src/dialogs_assembly_tests.rs
+39
View File
@@ -155,6 +155,45 @@ fn edit_joint_keeps_an_offset_typed_as_an_expression() {
assert_eq!((j.exprs.offset.as_deref(), j.exprs.angle.as_deref()), (Some("gap"), None));
}
#[test]
fn as_built_animation_drives_the_new_shared_id_and_handles_exhaustion() {
let mut app = two_boxes();
app.session.doc_mut().next_id = 500;
app.session.selection = vec![Sel::Body { name: "Body1".into() }, Sel::Body { name: "Body2".into() }];
app.start("joint.as_built");
let mut d = app.dialog.take().unwrap();
let Kind::Assembly(Asm::Joint(f)) = &mut d.kind else { panic!("expected joint dialog") };
f.kind = 1;
f.animate = true;
f.phase = 0.25;
let cmds = preview_commands(&app, &d).unwrap();
assert_eq!(cmds[1].0, "joint.drive");
assert_eq!(cmds[1].1["joint"], 500);
let mut preview = app.session.scratch();
for (command, params) in &cmds {
preview.execute(command, params).unwrap();
}
let joint = &preview.doc.assembly.joints[0];
assert_eq!(joint.id, 500);
assert!((joint.values[0] - std::f64::consts::FRAC_PI_2).abs() < 1e-9);
assert_eq!(app.session.doc.next_id, 500, "a preview must not reserve live ids");
app.session.doc_mut().next_id = u64::MAX;
let cmds = preview_commands(&app, &d).unwrap();
let result = app.session.scratch().execute(&cmds[0].0, &cmds[0].1);
assert!(result.is_err() && !matches!(result, Err(solidcraft_engine::EngineError::Internal(..))), "{result:?}");
// A programmatic document can also contain the largest possible joint id. The old
// preview added one to that id without checking, before the engine's panic guard.
let mut largest = joint.clone();
largest.id = u64::MAX;
app.session.doc_mut().assembly.joints.push(largest);
let cmds = preview_commands(&app, &d).unwrap();
assert_eq!(cmds.len(), 1, "an exhausted preview must not invent a drive target");
let result = app.session.scratch().execute(&cmds[0].0, &cmds[0].1);
assert!(result.is_err() && !matches!(result, Err(solidcraft_engine::EngineError::Internal(..))), "{result:?}");
}
/// As-built joints and rigid groups take components from body picks; New Component makes one
/// from bodies when bodies are picked.
#[test]
crates/ui-egui/tests/scenarios/s24_motion_study.json
+2 -2
View File
@@ -203,7 +203,7 @@
"params": {
"command": "motion.play",
"params": {
"study": 1,
"study": "Motion Study1",
"step": 100
}
}
@@ -226,4 +226,4 @@
{
"shot": "s24_motion_end"
}
]
]