Place: a file read apart from the session goes only into its document

Session::place_read and Session::place_queue_read placed a file, or
loaded the place cursor, in whichever document was active when the read
finished. A job prepared in one document could land in another one that
was activated meanwhile, or that stayed open after the first was closed.
Replace did not stop it when the other document held an equal object
under the same id.

PlaceJob, QueueJob and QueueRead now keep the uid of the document they
were prepared in. Both calls refuse a read whose document is not the
active one with "the document it was prepared for is no longer active",
before any selection, edit, tool change or journal entry. Switching back
to the original document places the file as before.

crates/engine/src/cmd/place.rs

@@ -58,6 +58,9 @@ use crate::{EngineError, MAX_COORD};
 
 const PLACE: &str = "file.place";
 const QUEUE: &str = "file.place.queue";
+/// The error of a file read apart from the session while another document is active, or after the
+/// document it was read for closed ([`Session::place_read`], [`Session::place_queue_read`]).
+const NOT_ACTIVE: &str = "the document it was prepared for is no longer active";
 /// The most files one `file.place.queue` loads.
 const MAX_QUEUE: usize = 100;
 /// The longest thumbnail side `thumbnail` may ask for (px).
@@ -503,11 +506,13 @@ fn fit(natural: Rect, r: Rect) -> Affine {
     Affine::translate(r.center().to_vec2()) * Affine::scale(k) * Affine::translate(-natural.center().to_vec2())
 }
 
-/// What `file.place` takes from the session before it reads the file: the params, the object
-/// Replace swaps and the artboard under `at`, which a DXF drawing is laid out on
-/// ([`Session::prepare_place`]). [`PlaceJob::read`] reads and decodes the file without the session,
-/// on any thread.
+/// What `file.place` takes from the session before it reads the file: the document it places into,
+/// the params, the object Replace swaps and the artboard under `at`, which a DXF drawing is laid
+/// out on ([`Session::prepare_place`]). [`PlaceJob::read`] reads and decodes the file without the
+/// session, on any thread.
 pub struct PlaceJob {
+    /// The document it was prepared in (`DocState::uid`).
+    doc: u64,
     params: Value,
     old: Option<NodeId>,
     /// The object Replace swaps as it was when Place started.
@@ -560,10 +565,14 @@ impl Session {
 
     /// Run `file.place` with the params of `read` as a command (checked and journaled as any
     /// other) that places the file `read` holds instead of reading it again → `file.place`'s
-    /// reply. The object Replace swaps must still be in the document as it was when Place started;
-    /// when the selection changed meanwhile, `select.set` selects that object again first, so a
-    /// replay of the journal replaces the same object.
+    /// reply. The document Place started in must be the active one, and the object Replace swaps
+    /// must still be in it as it was when Place started; when the selection changed meanwhile,
+    /// `select.set` selects that object again first, so a replay of the journal replaces the same
+    /// object.
     pub fn place_read(&mut self, read: PlaceRead) -> Result<Value> {
+        if self.doc()?.uid != read.job.doc {
+            return Err(bad(PLACE, NOT_ACTIVE));
+        }
         let params = read.job.params.clone();
         if let Some(old) = read.job.old {
             let st = self.doc()?;
@@ -615,7 +624,7 @@ fn prepare(s: &Session, p: &Value) -> Result<PlaceJob> {
         (true, _) => return Err(bad(PLACE, "replace: select the one object to replace")),
     };
     let replaced = old.and_then(|id| st.doc.node(id)).cloned();
-    Ok(PlaceJob { params: p.clone(), old, replaced, target: board_at(&st.doc, at) })
+    Ok(PlaceJob { doc: st.uid, params: p.clone(), old, replaced, target: board_at(&st.doc, at) })
 }
 
 /// Place the file `read` holds in the active document.
@@ -755,6 +764,8 @@ fn queued_files(p: &Value) -> Result<Vec<Value>> {
 /// ([`Session::prepare_place_queue`]). [`QueueJob::read`] reads them without the session, on any
 /// thread.
 pub struct QueueJob {
+    /// The document it was prepared in (`DocState::uid`).
+    doc: u64,
     params: Value,
     files: Vec<Value>,
 }
@@ -762,6 +773,8 @@ pub struct QueueJob {
 /// The files [`QueueJob::read`] read, which [`Session::place_queue_read`] loads the place cursor
 /// with.
 pub struct QueueRead {
+    /// The document the files were read for (`DocState::uid`).
+    doc: u64,
     params: Value,
     entries: Vec<Value>,
     files: Vec<Value>,
@@ -776,13 +789,13 @@ impl QueueJob {
 
     /// Read every file for its size and thumbnail, as `file.place.queue` does, without the session.
     pub fn read(self) -> Result<QueueRead> {
-        read_queue(self.params, self.files)
+        read_queue(self.doc, self.params, self.files)
     }
 }
 
-/// Read each of `queued`, the files of `file.place.queue` with params `p`, for its size and
-/// thumbnail.
-fn read_queue(p: Value, queued: Vec<Value>) -> Result<QueueRead> {
+/// Read each of `queued`, the files of `file.place.queue` with params `p` for the document `doc`,
+/// for its size and thumbnail.
+fn read_queue(doc: u64, p: Value, queued: Vec<Value>) -> Result<QueueRead> {
     let thumbnail = p.get("thumbnail").and_then(Value::as_f64);
     let (mut entries, mut files, mut skipped) = (vec![], vec![], vec![]);
     for mut q in queued {
@@ -823,7 +836,7 @@ fn read_queue(p: Value, queued: Vec<Value>) -> Result<QueueRead> {
             }
         }
     }
-    Ok(QueueRead { params: p, entries, files, skipped })
+    Ok(QueueRead { doc, params: p, entries, files, skipped })
 }
 
 impl Session {
@@ -834,12 +847,17 @@ impl Session {
         has_doc(self).map_err(|why| EngineError::Disabled(QUEUE.into(), why))?;
         check_switches(params, &["link", "template"], QUEUE)?;
         check_numbers(params, &["thumbnail"], QUEUE)?;
-        Ok(QueueJob { params: params.clone(), files: queued_files(params)? })
+        let files = queued_files(params)?;
+        Ok(QueueJob { doc: self.doc()?.uid, params: params.clone(), files })
     }
 
     /// Run `file.place.queue` with the params of `read` as a command that loads the place cursor
-    /// with the files `read` holds instead of reading them again → `file.place.queue`'s reply.
+    /// with the files `read` holds instead of reading them again → `file.place.queue`'s reply. The
+    /// document the files were read for must be the active one.
     pub fn place_queue_read(&mut self, read: QueueRead) -> Result<Value> {
+        if self.doc()?.uid != read.doc {
+            return Err(bad(QUEUE, NOT_ACTIVE));
+        }
         let params = read.params.clone();
         self.queue_hand_off = Some(read);
         let r = self.execute(QUEUE, &params);

crates/engine/src/tests/place.rs

@@ -365,6 +365,95 @@ fn a_file_read_apart_from_the_session_replaces_what_was_selected_when_place_star
     }
 }
 
+/// Every open document with its art, selection and undo steps, and the journal, tool and tool
+/// options: what a refused hand-off leaves as it was.
+type Snapshot = (Vec<(u64, Arc<vectorcraft_doc::Document>, Vec<NodeId>, usize)>, usize, &'static str, Value);
+
+fn snapshot(s: &Session) -> Snapshot {
+    let docs = s.documents().iter().map(|d| (d.uid, d.doc.clone(), d.selection.objects.clone(), d.history.undo.len())).collect();
+    (docs, s.journal.len(), s.tool_id(), s.tool_options())
+}
+
+/// A file read apart from the session is placed only in the document Place started in: with
+/// another document active, Replace included when that document holds the same object under the
+/// same id, or after the document closed, nothing changes in any document; with the document
+/// active again, the file is placed in it.
+#[test]
+fn a_file_read_apart_from_the_session_is_placed_only_in_the_document_it_was_read_for() {
+    let blue = file("blue.png", &png(8, 8, [0, 0, 255, 255], None));
+    let not_active = "the document it was prepared for is no longer active";
+    for replace in [false, true] {
+        let p = with(blue.clone(), json!({"replace": replace}));
+        let mut s = session();
+        let square = json!({"x": 10, "y": 10, "width": 20, "height": 20});
+        let a = s.execute("shape.rectangle", &square).unwrap()["id"].clone();
+        let read = s.prepare_place(&p).unwrap().read().unwrap();
+        s.execute("file.new", &json!({"width": 400, "height": 300})).unwrap();
+        let b = s.execute("shape.rectangle", &square).unwrap()["id"].clone();
+        assert_eq!(a, b, "the other document holds an object with the same id");
+        assert!(node(&s, &b) == s.documents()[0].doc.node(NodeId(a.as_u64().unwrap())).unwrap().clone(), "and the same content");
+        let before = snapshot(&s);
+        let err = s.place_read(read).unwrap_err();
+        assert!(err.to_string().contains(not_active), "replace {replace}: {err}");
+        assert!(snapshot(&s) == before, "replace {replace}: nothing placed, selected or journaled in either document");
+        // Active again, the document Place started in takes the file.
+        let read = {
+            s.execute("document.activate", &json!({"index": 0})).unwrap();
+            s.prepare_place(&p).unwrap().read().unwrap()
+        };
+        s.execute("document.activate", &json!({"index": 1})).unwrap();
+        s.execute("document.activate", &json!({"index": 0})).unwrap();
+        let r = s.place_read(read).unwrap();
+        assert_eq!(s.active_index(), Some(0));
+        assert!(s.doc().unwrap().doc.node(NodeId(r["ids"][0].as_u64().unwrap())).is_some(), "replace {replace}: placed in the first document");
+        let others = s.documents()[1].doc.layers.last().unwrap().children().unwrap().len();
+        assert_eq!(others, 1, "replace {replace}: the other document keeps its one object");
+    }
+    // The document Place started in closed: refused in the document left open, and with none left.
+    let mut s = session();
+    let read = s.prepare_place(&blue).unwrap().read().unwrap();
+    s.execute("file.new", &json!({"width": 400, "height": 300})).unwrap();
+    s.execute("file.close", &json!({"index": 0})).unwrap();
+    let before = snapshot(&s);
+    assert!(s.place_read(read).unwrap_err().to_string().contains(not_active));
+    assert!(snapshot(&s) == before, "nothing placed in the document left open");
+    let read = s.prepare_place(&blue).unwrap().read().unwrap();
+    s.execute("file.close", &json!({})).unwrap();
+    let journal = s.journal.len();
+    assert!(s.place_read(read).is_err() && s.journal.len() == journal, "no document left: refused");
+}
+
+/// Files read for the place cursor apart from the session load it only in the document they were
+/// read for: with another document active, or after that document closed, its tool and queue stay
+/// as they were; with the document active again, the cursor is loaded.
+#[test]
+fn files_read_for_the_place_cursor_apart_from_the_session_load_it_only_in_their_document() {
+    let p = json!({"files": [file("a.png", &png(40, 20, [9, 9, 9, 255], None))], "link": false});
+    let not_active = "the document it was prepared for is no longer active";
+    let mut s = session();
+    let read = s.prepare_place_queue(&p).unwrap().read().unwrap();
+    s.execute("file.new", &json!({"width": 400, "height": 300})).unwrap();
+    let before = snapshot(&s);
+    assert!(s.place_queue_read(read).unwrap_err().to_string().contains(not_active));
+    assert!(snapshot(&s) == before && s.tool_id() == "selection", "the other document's tool and queue stay");
+    let read = {
+        s.execute("document.activate", &json!({"index": 0})).unwrap();
+        s.prepare_place_queue(&p).unwrap().read().unwrap()
+    };
+    s.execute("document.activate", &json!({"index": 1})).unwrap();
+    s.execute("document.activate", &json!({"index": 0})).unwrap();
+    assert_eq!(s.place_queue_read(read).unwrap()["count"], 1);
+    assert_eq!(s.tool_id(), "place");
+    // The document they were read for closed.
+    let mut s = session();
+    let read = s.prepare_place_queue(&p).unwrap().read().unwrap();
+    s.execute("file.new", &json!({"width": 400, "height": 300})).unwrap();
+    s.execute("file.close", &json!({"index": 0})).unwrap();
+    let before = snapshot(&s);
+    assert!(s.place_queue_read(read).unwrap_err().to_string().contains(not_active));
+    assert!(snapshot(&s) == before, "the document left open keeps its tool");
+}
+
 /// A linked image keeps the preview its file was read with ([`cmd::place::PlaceJob::read`] makes
 /// it), so placing the read file does not decode the image again; an embedded image has none.
 #[test]