BREAK, BREAKATPOINT and JOIN: leave objects on locked layers alone (fixes #727) (#730)

BREAK and BREAKATPOINT (break_entity) and line/arc/polyline JOIN
(modify::join) removed their source objects and added replacements
without checking the source layer's lock, so a locked line could be cut
or merged into a polyline. ERASE, MOVE, TRIM, FILLET, EXPLODE and the
ellipse/spline JOIN already guard against this.

Both now refuse with the same errors the sibling commands use:
"The object is on a locked layer." (BREAK, BREAKATPOINT, like FILLET)
and "An object is on a locked layer." (JOIN, like the ellipse/spline
join). JOIN refuses the whole request when any selected object is
locked, so a mixed-layer pair keeps its locked original in either
order. The guards sit in the shared functions, so JSON calls with
explicit handles or the selection, the interactive prompts and the
command line all reach them. The interactive commands report the
refusal on the command line.

Tests (crates/engine/tests/break_join_locked_layers.rs, red before the
fix): BREAK and BREAKATPOINT on a locked line by handle and by
selection; BREAK on a locked arc and circle; JOIN of locked lines and
of a locked/unlocked pair in both orders; interactive BREAK and JOIN.
Each checks the drawing is unchanged while locked, and that the same
edit works unlocked (BREAK also undoes and redoes).

Co-authored-by: Brandon Thomas <[email protected]>
Co-authored-by: Claude Opus 5.5 <[email protected]>

crates/engine/src/cmd/modify.rs

@@ -1414,6 +1414,9 @@ fn run_explode(s: &mut Session, p: &Value) -> Result<Value> {
 
 /// Break `h` between two points (or at one point when p1 == p2).
 pub(crate) fn break_entity(s: &mut Session, h: Handle, p1: Vec2, p2: Vec2) -> Result<Vec<Handle>> {
+    if super::curves::is_locked(s, h) {
+        return Err(EngineError::Other("The object is on a locked layer.".into()));
+    }
     let e = s.doc()?.entity(h).map(|e| (**e).clone()).ok_or_else(|| EngineError::Other("no such object".into()))?;
     let pieces: Vec<EntityKind> = match &e.kind {
         EntityKind::Line(l) => {
@@ -1523,6 +1526,9 @@ pub(crate) fn join(s: &mut Session, hs: &[Handle]) -> Result<Option<Handle>> {
     if let Some(h) = super::modify2::join_curves(s, hs)? {
         return Ok(Some(h));
     }
+    if hs.iter().any(|h| super::curves::is_locked(s, *h)) {
+        return Err(EngineError::Other("An object is on a locked layer.".into()));
+    }
     let d = s.doc()?;
     let mut segs: Vec<Segment> = Vec::new();
     // Source entity of each segment in `segs`, so only entities that end up in the chain are removed.

crates/engine/tests/break_join_locked_layers.rs

@@ -0,0 +1,135 @@
+//! BREAK, BREAKATPOINT and JOIN leave objects on locked layers alone (#727).
+
+use cadcraft_engine::geom::Vec2;
+use cadcraft_engine::{Input, Session};
+use serde_json::{Value, json};
+
+fn h(r: &Value) -> String {
+    r["handle"].as_str().or_else(|| r["handles"][0].as_str()).unwrap().to_string()
+}
+
+/// Everything in the drawing a locked-layer edit could disturb.
+fn snapshot(s: &Session) -> String {
+    let d = s.doc().unwrap();
+    format!("{:?}", d.model.iter().map(|e| (e.handle, e.common.layer.clone(), e.kind.clone())).collect::<Vec<_>>())
+}
+
+fn set_lock(s: &mut Session, locked: bool) {
+    s.execute("layer.set", &json!({"name": "0", "locked": locked})).unwrap();
+}
+
+fn line(s: &mut Session, a: [f64; 2], b: [f64; 2]) -> String {
+    h(&s.execute("line", &json!({"points": [a, b]})).unwrap())
+}
+
+fn lines_ab(s: &mut Session) -> (String, String) {
+    (line(s, [0.0, 0.0], [10.0, 0.0]), line(s, [10.0, 0.0], [20.0, 0.0]))
+}
+
+#[test]
+fn break_and_breakatpoint_refuse_a_locked_line() {
+    let mut s = Session::new();
+    let a = line(&mut s, [0.0, 0.0], [10.0, 0.0]);
+    let control = line(&mut s, [0.0, 5.0], [10.0, 5.0]);
+    set_lock(&mut s, true);
+    let before = snapshot(&s);
+    assert!(s.execute("break", &json!({"handle": a, "p1": [3, 0], "p2": [7, 0]})).is_err());
+    assert!(s.execute("breakatpoint", &json!({"handle": a, "at": [5, 0]})).is_err());
+    // Selection fallback reaches the same guard.
+    s.execute("select", &json!({"handles": [a]})).unwrap();
+    assert!(s.execute("break", &json!({"p1": [3, 0], "p2": [7, 0]})).is_err());
+    assert!(s.execute("breakatpoint", &json!({"at": [5, 0]})).is_err());
+    assert_eq!(snapshot(&s), before);
+    // Unlocked, the same edits work, and undo and redo them.
+    set_lock(&mut s, false);
+    let unlocked = snapshot(&s);
+    let r = s.execute("break", &json!({"handle": a, "p1": [3, 0], "p2": [7, 0]})).unwrap();
+    assert_eq!(r["handles"].as_array().unwrap().len(), 2);
+    assert!(s.doc().unwrap().entity(cadcraft_engine::doc::Handle::parse_hex(&a).unwrap()).is_none());
+    assert!(s.doc().unwrap().entity(cadcraft_engine::doc::Handle::parse_hex(&control).unwrap()).is_some());
+    let broken = snapshot(&s);
+    s.execute("undo", &json!({})).unwrap();
+    assert_eq!(snapshot(&s), unlocked);
+    s.execute("redo", &json!({})).unwrap();
+    assert_eq!(snapshot(&s), broken);
+}
+
+#[test]
+fn break_refuses_a_locked_arc_and_circle() {
+    let mut s = Session::new();
+    let arc = h(&s.execute("arc", &json!({"center": [0, 0], "radius": 5, "start": 0, "end": 90})).unwrap());
+    let circle = h(&s.execute("circle", &json!({"center": [20, 0], "radius": 3})).unwrap());
+    set_lock(&mut s, true);
+    let before = snapshot(&s);
+    assert!(s.execute("break", &json!({"handle": arc, "p1": [5, 0], "p2": [0, 5]})).is_err());
+    assert!(s.execute("break", &json!({"handle": circle, "p1": [23, 0], "p2": [20, 3]})).is_err());
+    assert_eq!(snapshot(&s), before);
+}
+
+#[test]
+fn join_refuses_when_any_line_is_locked() {
+    let mut s = Session::new();
+    let (a, b) = lines_ab(&mut s);
+    set_lock(&mut s, true);
+    let before = snapshot(&s);
+    assert!(s.execute("join", &json!({"handles": [a, b]})).is_err());
+    s.execute("select", &json!({"handles": [a, b]})).unwrap();
+    assert!(s.execute("join", &json!({})).is_err());
+    assert_eq!(snapshot(&s), before);
+    set_lock(&mut s, false);
+    let r = s.execute("join", &json!({"handles": [a, b]})).unwrap();
+    assert!(r["handle"].is_string());
+    assert_eq!(s.doc().unwrap().model.len(), 1);
+}
+
+#[test]
+fn join_keeps_a_locked_original_in_a_mixed_layer_pair() {
+    for locked_first in [true, false] {
+        let mut s = Session::new();
+        s.execute("layer.new", &json!({"name": "L"})).unwrap();
+        // The first line goes on the soon-locked layer 0, the second on "L" (or the reverse).
+        let on_l = |s: &mut Session, a: [f64; 2], b: [f64; 2]| {
+            s.execute("layer.current", &json!({"name": "L"})).unwrap();
+            let r = line(s, a, b);
+            s.execute("layer.current", &json!({"name": "0"})).unwrap();
+            r
+        };
+        let (a, b) = if locked_first {
+            (line(&mut s, [0.0, 0.0], [10.0, 0.0]), on_l(&mut s, [10.0, 0.0], [20.0, 0.0]))
+        } else {
+            (on_l(&mut s, [0.0, 0.0], [10.0, 0.0]), line(&mut s, [10.0, 0.0], [20.0, 0.0]))
+        };
+        let locked = if locked_first { &a } else { &b };
+        set_lock(&mut s, true);
+        let before = snapshot(&s);
+        assert!(s.execute("join", &json!({"handles": [a, b]})).is_err());
+        assert_eq!(snapshot(&s), before, "locked_first = {locked_first}");
+        assert!(s.doc().unwrap().entity(cadcraft_engine::doc::Handle::parse_hex(locked).unwrap()).is_some());
+    }
+}
+
+#[test]
+fn interactive_break_and_join_refuse_locked_lines() {
+    let mut s = Session::new();
+    let (a, b) = lines_ab(&mut s);
+    set_lock(&mut s, true);
+    let before = snapshot(&s);
+    // BREAK: pick the line, then the second break point.
+    s.start("break").unwrap();
+    s.input(Input::Point(Vec2::new(3.0, 0.0))).unwrap();
+    let _ = s.input(Input::Point(Vec2::new(7.0, 0.0)));
+    assert!(s.log.last().is_some_and(|m| m.contains("locked layer")), "{:?}", s.log.last());
+    assert_eq!(snapshot(&s), before);
+    // JOIN on a preselected pair reports the refusal on the command line.
+    s.cancel();
+    s.execute("select", &json!({"handles": [a, b]})).unwrap();
+    s.start("join").unwrap();
+    assert!(s.log.last().is_some_and(|m| m.contains("locked layer")), "{:?}", s.log.last());
+    assert_eq!(snapshot(&s), before);
+    // Unlocked, the interactive commands still edit.
+    set_lock(&mut s, false);
+    s.start("break").unwrap();
+    s.input(Input::Point(Vec2::new(3.0, 0.0))).unwrap();
+    s.input(Input::Point(Vec2::new(7.0, 0.0))).unwrap();
+    assert_ne!(snapshot(&s), before);
+}