M8: Opening a file contacts no team library server it names until you check it (#1621)

* M8: opening a file contacts no team library server it names until you check it (#1097)

* M8: translate the team library check strings (#1097)

* M8: the Assets panel library test checks what allows a library, without its own fake link (Sonar duplication and coverage)

crates/engine/src/assets/library_server.rs

@@ -26,6 +26,15 @@ pub const NOT_FETCHED: &str = "the library hasn't been fetched from its server y
 pub const NO_LIBRARY: &str = "that file has no library on the server yet: publish it there first (Libraries ▸ Publish to server)";
 /// Most components one usage report lists.
 pub const MAX_USAGE: usize = 10_000;
+/// Most team libraries remembered as yours ([`Prefs::team_libraries`](crate::Prefs)): adding or
+/// checking another forgets the one used longest ago.
+pub const MAX_TEAM_LIBRARIES: usize = 500;
+
+/// How [`Prefs::team_libraries`](crate::Prefs) keeps a link: its SHA-256, so the preferences hold
+/// no token.
+fn link_key(link: &str) -> String {
+    figcraft_format::image_hash(link.trim().as_bytes())
+}
 
 /// Whether a library's path is a share link of a file on a server (else it is a library file).
 pub fn is_server_link(path: &str) -> bool {
@@ -156,15 +165,35 @@ impl Session {
         self.take_library(path, reply).map_err(EngineError::Other)
     }
 
+    /// Whether team library `link` is one you added or checked here ([`Prefs::team_libraries`](crate::Prefs)):
+    /// only those are checked for updates by themselves.
+    pub fn library_allowed(&self, link: &str) -> bool {
+        self.prefs.team_libraries.contains(&link_key(link))
+    }
+
+    /// Remember team library `link` as one you use (the app saves the preferences at once).
+    pub(crate) fn allow_library(&mut self, link: &str) {
+        let key = link_key(link);
+        let list = &mut self.prefs.team_libraries;
+        if list.first() != Some(&key) {
+            list.retain(|k| *k != key);
+            list.insert(0, key);
+            list.truncate(MAX_TEAM_LIBRARIES);
+            self.live.unsaved = true;
+        }
+    }
+
     /// Ask the servers of the active document's team libraries for newer versions, in the
     /// background, reporting how the document uses each (the Assets panel calls it now and then).
+    /// Only libraries you added or checked here are asked: opening a file that names another
+    /// server contacts nothing until you check it (`library.check`, #1097).
     pub fn refresh_server_libraries(&mut self) {
         let Ok(doc) = self.doc() else { return };
         let jobs: Vec<(String, LibraryReq)> = doc
             .libraries
             .enabled
             .iter()
-            .filter(|l| is_server_link(&l.path))
+            .filter(|l| is_server_link(&l.path) && self.library_allowed(&l.path))
             .map(|l| {
                 let have = match self.libraries.cached(&l.path) {
                     Some(Ok(lib)) => Some(lib.manifest.version),

crates/engine/src/cmd/library.rs

@@ -85,6 +85,13 @@ pub fn specs() -> Vec<CommandSpec> {
             false,
             updates
         ),
+        spec!(
+            "library.check",
+            "Check team libraries for updates",
+            "{library? (an enabled team library: id, name or link; all of this file's when omitted)}  asks their servers for the latest version in the background, and from then on checks them by themselves: opening a file doesn't contact the server of a team library you haven't added or checked → {checking}",
+            false,
+            check
+        ),
         spec!(
             "library.acceptUpdates",
             "Accept library updates",
@@ -524,6 +531,10 @@ fn add(s: &mut Session, p: &Value) -> Result<Value> {
     let raw = req_string(cmd, p, "path")?;
     let path = if is_server_link(&raw) { raw.trim().to_string() } else { absolute_path(raw.trim()) };
     let lib = s.load_library(&path)?;
+    if is_server_link(&path) {
+        // Added by you: its server may be checked for updates from now on (#1097).
+        s.allow_library(&path);
+    }
     let m = &lib.manifest;
     let doc = s.doc()?;
     if doc.published.as_ref().is_some_and(|me| me.manifest.id == m.id) {
@@ -549,6 +560,27 @@ fn add(s: &mut Session, p: &Value) -> Result<Value> {
     Ok(out)
 }
 
+/// `library.check`: the user lets this file's team libraries (or one) be checked; they are asked now.
+fn check(s: &mut Session, p: &Value) -> Result<Value> {
+    let cmd = "library.check";
+    let doc = s.doc()?;
+    let links: Vec<String> = match string(p, "library") {
+        Some(key) => {
+            let l = find_link(doc, &key).ok_or_else(|| bad(cmd, format!("no enabled library `{key}` (library.list shows them)")))?;
+            if !is_server_link(&l.path) {
+                return Err(bad(cmd, format!("`{key}` is a library file, not a team library on a server")));
+            }
+            vec![l.path.clone()]
+        }
+        None => doc.libraries.enabled.iter().filter(|l| is_server_link(&l.path)).map(|l| l.path.clone()).collect(),
+    };
+    for l in &links {
+        s.allow_library(l);
+    }
+    s.refresh_server_libraries();
+    Ok(json!({"checking": links.len()}))
+}
+
 fn remove(s: &mut Session, p: &Value) -> Result<Value> {
     let cmd = "library.remove";
     let key = req_string(cmd, p, "id")?;

crates/engine/src/extensions/plugins.rs

@@ -107,6 +107,8 @@ const DENIED: &[&str] = &[
     "plugins.",
     "libraries.",
     "library.add",
+    // Contacts the servers a file names, and remembers them as yours (#1097).
+    "library.check",
     // Uploads the open document to a server.
     "library.publishToServer",
     "edit.undo",

crates/engine/src/lib.rs

@@ -361,6 +361,8 @@ mod jam_open_tests;
 #[cfg(test)]
 mod keymap_tests;
 #[cfg(test)]
+mod library_beacon_tests;
+#[cfg(test)]
 mod library_path_tests;
 #[cfg(test)]
 mod library_range_styles_tests;

crates/engine/src/library_beacon_tests.rs

@@ -0,0 +1,117 @@
+//! Opening a file that names a team library on some server doesn't contact that server (#1097):
+//! only team libraries you added or checked are asked for updates by themselves.
+
+use std::sync::{Arc, Mutex};
+
+use figcraft_collab::message::{ClientMsg, LibraryReply, LibraryReq};
+use figcraft_doc::{Document, LibraryLink};
+use serde_json::json;
+
+use crate::live::{Event, HostOptions, LiveLink, NetRequest, Presence, SessionInfo};
+use crate::{Prefs, Session};
+
+const LINK: &str = "ws://tracker.example:9/?file=f&token=per-file-secret";
+
+/// Every request the app would send anywhere.
+#[derive(Default)]
+struct Wire {
+    sent: Vec<String>,
+}
+
+struct Link(Arc<Mutex<Wire>>);
+
+impl Link {
+    fn note(&self, what: &str) {
+        self.0.lock().unwrap().sent.push(what.into());
+    }
+}
+
+impl LiveLink for Link {
+    fn host(&mut self, _: u64, _: HostOptions) -> Result<SessionInfo, String> {
+        self.note("host");
+        Err("no".into())
+    }
+    fn join(&mut self, _: u64, _: &str, _: &str) -> Result<(), String> {
+        self.note("join");
+        Err("no".into())
+    }
+    fn leave(&mut self, _: u64) {}
+    fn presence(&mut self, _: u64, _: &Presence) {}
+    fn send(&mut self, _: u64, _: ClientMsg) {
+        self.note("send");
+    }
+    fn poll(&mut self) -> Vec<(u64, Event)> {
+        Vec::new()
+    }
+    fn library(&mut self, link: &str, _: LibraryReq) -> Result<LibraryReply, String> {
+        self.note(link);
+        Ok(LibraryReply::Empty)
+    }
+    fn library_background(&mut self, jobs: Vec<(String, LibraryReq)>) {
+        for (link, _) in jobs {
+            self.note(&link);
+        }
+    }
+    fn background(&mut self, _: u64, _: NetRequest) -> bool {
+        self.note("background");
+        false
+    }
+}
+
+/// A session with `prefs` that opened a file (from disk) whose libraries include [`LINK`].
+fn opened(prefs: Prefs, tag: &str) -> (Session, Arc<Mutex<Wire>>, std::path::PathBuf) {
+    let mut d = Document::new("Someone else's file");
+    d.libraries_mut().enabled.push(LibraryLink { id: "lib".into(), name: "Kit".into(), path: LINK.into(), version: 3 });
+    let path = std::env::temp_dir().join(format!("figcraft-beacon-{}-{tag}.figcraft", std::process::id()));
+    std::fs::write(&path, figcraft_format::save_bytes(&d, None).unwrap()).unwrap();
+    let mut s = Session::new();
+    s.prefs = prefs;
+    let wire = Arc::new(Mutex::new(Wire::default()));
+    s.live.install(Box::new(Link(wire.clone())));
+    s.execute("file.open", &json!({"path": path.to_string_lossy()})).unwrap();
+    assert_eq!(s.doc().unwrap().libraries.enabled.len(), 1);
+    (s, wire, path)
+}
+
+fn sent(w: &Arc<Mutex<Wire>>) -> Vec<String> {
+    w.lock().unwrap().sent.clone()
+}
+
+#[test]
+fn opening_a_file_naming_an_unknown_team_library_contacts_nothing() {
+    let (mut s, wire, path) = opened(Prefs::default(), "unknown");
+    // What the Assets panel does now and then while the file is shown.
+    for _ in 0..3 {
+        s.refresh_server_libraries();
+    }
+    assert!(sent(&wire).is_empty(), "no connection attempt: {:?}", sent(&wire));
+    assert!(!s.library_allowed(LINK));
+    // You check it: now it is asked, and from then on by itself.
+    assert_eq!(s.execute("library.check", &json!({})).unwrap()["checking"], 1);
+    assert_eq!(sent(&wire), vec![LINK.to_string()]);
+    assert!(s.library_allowed(LINK) && s.live.unsaved, "kept with the preferences");
+    s.refresh_server_libraries();
+    assert_eq!(sent(&wire).len(), 2);
+    // Kept as a hash: the preferences hold no token.
+    assert!(!serde_json::to_string(&s.prefs).unwrap().contains("per-file-secret"));
+    let _ = std::fs::remove_file(path);
+}
+
+#[test]
+fn a_team_library_you_checked_before_is_checked_when_a_file_names_it() {
+    let mut mine = Session::new();
+    mine.allow_library(LINK);
+    let (mut s, wire, path) = opened(mine.prefs.clone(), "checked");
+    s.refresh_server_libraries();
+    assert_eq!(sent(&wire), vec![LINK.to_string()]);
+    let _ = std::fs::remove_file(path);
+}
+
+#[test]
+fn checking_one_team_library_by_name() {
+    let (mut s, wire, path) = opened(Prefs::default(), "named");
+    assert!(s.execute("library.check", &json!({"library": "Nope"})).is_err());
+    assert_eq!(s.execute("library.check", &json!({"library": "Kit"})).unwrap()["checking"], 1);
+    assert_eq!(sent(&wire), vec![LINK.to_string()]);
+    let _ = std::fs::remove_file(path);
+}

crates/engine/src/prefs.rs

@@ -110,6 +110,12 @@ pub struct Prefs {
     /// `widget:<plugin>`.
     #[serde(skip_serializing_if = "Vec::is_empty")]
     pub recent_actions: Vec<String>,
+    /// The team libraries you added (`library.add`) or checked (`library.check`), as hashes of
+    /// their links (which carry tokens), newest first, at most
+    /// [`MAX_TEAM_LIBRARIES`](crate::library_server::MAX_TEAM_LIBRARIES): only these are checked
+    /// for updates by themselves. A file naming another one doesn't contact its server (#1097).
+    #[serde(skip_serializing_if = "Vec::is_empty")]
+    pub team_libraries: Vec<String>,
 }
 
 /// Recently run actions kept for the Actions palette (newest first).
@@ -150,6 +156,7 @@ impl Default for Prefs {
             view: BTreeMap::new(),
             board_grid: Default::default(),
             recent_actions: Vec::new(),
+            team_libraries: Vec::new(),
         }
     }
 }

crates/engine/src/sharing/live.rs

@@ -196,8 +196,8 @@ pub struct Lives {
     /// Registers the app as the opener of `figcraft://` invite links (the desktop app on Windows
     /// installs one; see [`crate::invite_links`]).
     pub links: Option<Box<dyn crate::invite_links::LinkRegistrar>>,
-    /// The recorded online files changed since the app last saved the preferences (it saves them
-    /// at once: a crash must not lose an owner token).
+    /// The recorded online files or your team libraries changed since the app last saved the
+    /// preferences (it saves them at once: a crash must not lose an owner token).
     pub unsaved: bool,
     /// Servers that couldn't say what became of your files at the last `collab.files {refresh}`.
     pub servers: crate::online::Problems,