From 1873043177fa9b8780d4bcd0a1cced59cfd8e855 Mon Sep 17 00:00:00 2001 From: elpidioassinaturas Date: Fri, 6 Nov 2026 11:45:46 -0300 Subject: [PATCH 1/5] feat(plugins): a plugin command's service.add reads off the frame on the desktop (#1319) The documents a plugin command's service.add reads are fetched on a background thread, under #1341's request numbers, and its features or coverage go through layer.addUrl as Add From Service's do. The command runs again when each has come and goes on with the layer, so the window answers meanwhile. - A run again sees the view it first ran with, so a view moved during the wait asks for no other area. - A click on the command waiting doesn't start it over; the status bar says it is running. - A failed answer is given once a run, in order, so a command trying again gets its next try. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/cmd/layer.rs | 6 +- crates/engine/src/cmd/plugin_fetch.rs | 176 ++++++++++++- crates/engine/src/cmd/plugins.rs | 5 + crates/engine/src/cmd/service.rs | 133 ++++++++-- crates/engine/src/lib.rs | 3 + crates/ui-egui/src/pluginpanel.rs | 343 ++++++++++++++++++++++++++ crates/ui-egui/src/pluginstart.rs | 35 ++- docs/plugins.md | 24 +- 8 files changed, 694 insertions(+), 31 deletions(-) diff --git a/crates/engine/src/cmd/layer.rs b/crates/engine/src/cmd/layer.rs index f34bd22e9..e15573817 100644 --- a/crates/engine/src/cmd/layer.rs +++ b/crates/engine/src/cmd/layer.rs @@ -3086,7 +3086,7 @@ fn add_url_as_given(s: &mut Session, p: &Value) -> CmdResult { // Answering it changes nothing (the layer is its own undo step). let wanted = s.asked_for(p); if s.leave_downloads - && let Some((_, answer, added)) = s.rerun_landed.iter().find(|(asked, ..)| super::plugins::fetch::same_request(asked, &wanted)) + && let Some((answer, added)) = s.rerun_answer(&wanted) { s.request_only = true; // Added to another map than the one shown now: the command's next steps would act on this @@ -3111,10 +3111,10 @@ fn add_url_as_given(s: &mut Session, p: &Value) -> CmdResult { if missing || added.gone(s.map()) { return Err(format!("{url}: the layer its download added was taken away (undone) before the command ran again")); } - if super::plugins::fetch::table_gone(answer, &s.project) { + if super::plugins::fetch::table_gone(&answer, &s.project) { return Err(format!("{url}: the table its download added was taken away (undone) before the command ran again")); } - return added.answer_now(s.map(), answer); + return added.answer_now(s.map(), &answer); } // An address read through a saved S3 connection is judged by the address it wraps; the whole // address stays what is fetched and kept. diff --git a/crates/engine/src/cmd/plugin_fetch.rs b/crates/engine/src/cmd/plugin_fetch.rs index b089161a5..13391830b 100644 --- a/crates/engine/src/cmd/plugin_fetch.rs +++ b/crates/engine/src/cmd/plugin_fetch.rs @@ -17,9 +17,13 @@ //! desktop app since #1170) runs again the same way once the app has added it (#1202): its //! `layer.addUrl` then answers what adding the download answered, so the command goes on with //! the layer (clips, renames or styles it). +//! +//! A plugin command's `service.add` on the desktop waits the same way (#1319): the documents it +//! reads are fetched off the frame ([`Session::left_document`]), and its features or coverage are +//! left to the app as `layer.addUrl`'s download is. use std::collections::VecDeque; -use std::sync::Mutex; +use std::sync::{Arc, Mutex}; use serde_json::{Value, json}; use web_time::{Duration, Instant}; @@ -281,10 +285,19 @@ impl Calls { /// with an error: one the app lost track of doesn't keep it waiting for ever. pub const REQUEST_WAIT: Duration = Duration::from_secs(10 * 60); +/// The most bytes a service's document read off the frame may have: it is kept until no command +/// waits for it, and read again each time the command runs. +pub const MAX_DOCUMENT: usize = 64 << 20; +/// How long a service's document read off the frame may take: what a command waiting for it had. +const DOCUMENT_TIMEOUT: Duration = Duration::from_secs(60); + /// A plugin command waiting for its fetches, or its downloads, before it runs again. pub(crate) struct Retry { id: String, params: Value, + /// The map shown when it first ran, and its view: the window answers while the command waits, + /// and a view moved meanwhile must not make the next run ask for another area (#1319). + view: (u64, crate::map::Camera), urls: Vec, /// The downloads it left to the app ([`Session::request_url`]), by request number. requests: Vec, @@ -313,6 +326,8 @@ pub(crate) struct UrlWaits { /// caller gave its number with the same address, then the app's own download came) isn't /// added twice. added: VecDeque<(u64, Value)>, + /// A service's documents being read off the frame, by request number. + reading: Vec<(u64, Arc, String>>>>)>, } /// What adding a landed download added, to tell when its command runs again whether it is still @@ -328,6 +343,8 @@ pub(crate) struct Added { /// The map it was added to (its instance id, which stays with it as maps are removed or /// moved), which may not be the one shown when the command runs again. pub(crate) map: u64, + /// A service's document read off the frame ([`Session::left_document`]), which adds nothing. + pub(crate) document: Option>, } impl Added { @@ -347,7 +364,7 @@ impl Added { (None, Some(i)) => usize::try_from(i).ok().and_then(|i| map.tiles.get(i)).cloned().into_iter().collect(), _ => Vec::new(), }; - Added { layers, tiles, map: at } + Added { layers, tiles, map: at, document: None } } /// `answer` as it stands in `map` now: a tile layer's place in the list, which moves as others @@ -526,6 +543,36 @@ impl UrlWaits { self.expired.push((n, asked)); } + /// Starts reading the document `asked` for off the frame, as request `n`. + pub(crate) fn read_document(&mut self, n: u64, asked: &Value) { + let slot: Arc, String>>>> = Arc::default(); + let land = slot.clone(); + crate::render::rustgis_tiles::cache::download_within( + asked["url"].as_str().unwrap_or(""), + MAX_DOCUMENT, + DOCUMENT_TIMEOUT, + Box::new(move |r| *land.lock().unwrap_or_else(|e| e.into_inner()) = Some(r)), + ); + self.reading.push((n, slot)); + } + + /// Lands the documents read by now. One whose command gave up is dropped, and forgotten. + fn documents_in(&mut self) { + for (n, slot) in std::mem::take(&mut self.reading) { + let answer = slot.lock().unwrap_or_else(|e| e.into_inner()).take(); + let Some(answer) = answer else { + self.reading.push((n, slot)); + continue; + }; + match answer { + Ok(bytes) => self.land(n, Ok(json!({})), Added { document: Some(bytes.into()), ..Added::default() }), + // Its command gets this as its answer: without the address's keys. + Err(e) => self.land(n, Err(crate::sources::redact_text(&e)), Added::default()), + } + self.refused(n); + } + } + /// Whether as many given-up downloads are still out as are remembered ([`MAX_EXPIRED`]). fn given_up_full(&self) -> bool { self.expired.len() >= MAX_EXPIRED @@ -558,6 +605,12 @@ pub(crate) fn without_request(p: &Value) -> Value { p } +/// Whether a download request asks for a service's document, which the session reads itself, +/// rather than data for the app to add. +pub(crate) fn is_document(p: &Value) -> bool { + p.get("document").and_then(Value::as_bool) == Some(true) +} + /// Whether two download requests ask for the same thing, whatever their numbers, and whatever /// spaces are around their addresses (the app downloads the address trimmed). pub(crate) fn same_request(a: &Value, b: &Value) -> bool { @@ -586,6 +639,51 @@ impl Session { asked } + /// What a command run again is answered for `wanted`: what adding the download answered, and + /// what it added. A failure answers once a run, and a call made again after it is a new try: + /// each run gets what the run before it got, in order, so a command that tries again after a + /// failure goes on to its next try, and doesn't find the same failure every time. + pub(crate) fn rerun_answer(&mut self, wanted: &Value) -> Option<(CmdResult, Added)> { + let told = &self.rerun_told; + let at = self + .rerun_landed + .iter() + .enumerate() + .position(|(i, (asked, answer, _))| same_request(asked, wanted) && (answer.is_ok() || !told.contains(&i)))?; + let (_, answer, added) = self.rerun_landed.get(at)?; + let answered = (answer.clone(), added.clone()); + if answered.0.is_err() { + self.rerun_told.push(at); + } + Some(answered) + } + + /// The service document at `url`, for a plugin command's `service.add` (#1319): what reading it + /// off the frame gave, or `None` when it is asked for now, and the command runs again once it + /// has come. Its waiting, replacing and giving up are a download's. + pub(crate) fn left_document(&mut self, url: &str) -> Option, String>> { + let asked = self.asked_for(&json!({"url": url, "document": true})); + if let Some((answer, added)) = self.rerun_answer(&asked) { + return Some(answer.and_then(|_| added.document.ok_or_else(|| format!("{url}: no document came")))); + } + let number = self.url_waits.in_flight(&asked).unwrap_or_else(|| { + self.next_request += 1; + self.next_request + }); + if let Some(asked_now) = self.call_requests.as_mut() + && !asked_now.iter().any(|(n, _)| *n == number) + { + asked_now.push((number, asked)); + } + None + } + + /// Whether a command run again has an answer for `wanted`. + pub(crate) fn rerun_has(&self, wanted: &Value) -> bool { + let wanted = self.asked_for(wanted); + self.rerun_landed.iter().any(|(asked, ..)| same_request(asked, &wanted)) + } + /// Keeps plugin command `id` to run again once `urls` (cache keys) have landed, and the /// downloads it left to the app (`requests`: their numbers and what was asked) have been /// added: its answer, which says so. @@ -659,6 +757,8 @@ impl Session { self.plugin_retries.push(Retry { id: id.to_string(), params: params.clone(), + // (A run again holds the view of its first, so this keeps it.) + view: (self.map().instance_id(), self.map().camera), urls, requests: numbers, answered, @@ -696,11 +796,23 @@ impl Session { !self.plugin_retries.is_empty() } + /// Whether plugin command `id` is waiting to run again with these parameters: asked for again, + /// it would only start over. + pub fn plugin_command_waits(&self, id: &str, params: &Value) -> bool { + self.plugin_retries.iter().any(|r| r.id == id && r.params == *params) + } + + /// The plugin command waiting longest for its fetches or downloads, for the front end to show. + pub fn plugin_command_waiting(&self) -> Option<&str> { + self.plugin_retries.first().map(|r| r.id.as_str()) + } + /// Runs again the plugin commands whose fetches have all landed and whose downloads the app /// has all added (see `host.fetch` and `layer.addUrl` in `docs/plugins.md`): each command and /// what it answered this time. Its `layer.addUrl` calls answer what adding each download /// answered. Errors are also given as notices, as for any command. pub fn poll_plugin_retries(&mut self) -> Vec<(String, CmdResult)> { + self.url_waits.documents_in(); if self.plugin_retries.is_empty() { return Vec::new(); } @@ -742,10 +854,18 @@ impl Session { self.rerun_landed = r.answered.iter().cloned().chain(mine).collect(); self.rerun_said = r.said; self.retry_tries = r.tries; + // It sees the view it first ran with; one it doesn't move itself goes back to what + // the user has moved to since. + let held = r.view.1; + let shown = (r.view.0 == self.map().instance_id()).then(|| std::mem::replace(&mut self.map_mut().camera, held)); // Run as the app runs a plugin's command: what it asks for now is left to the app. let res = self.run_leaving_downloads(&r.id, &r.params); + if let Some(shown) = shown.filter(|_| self.map().camera == held) { + self.map_mut().camera = shown; + } self.retry_tries = 0; self.rerun_landed.clear(); + self.rerun_told.clear(); self.rerun_said.clear(); (r.id, res) }) @@ -763,6 +883,58 @@ impl Session { mod tests { use super::*; + /// A failure answers once a run, in the order it came, and a call made again after it is a new + /// download: a command trying again gets its next try, not the same failure for ever. What was + /// added answers as often as it is asked for (#1319). + #[test] + fn a_failed_answer_is_given_once_a_run() { + let mut s = Session::new(crate::sample::world_project()); + s.leave_downloads = true; + let id = u64::from(s.map().layers[0].id); + let asked = json!({"url": "https://x.org/a.geojson"}); + let failed: CmdResult = Err("https://x.org/a.geojson: 502 Bad Gateway".into()); + s.rerun_landed = vec![(asked.clone(), failed.clone(), Added::default())]; + assert_eq!(s.run("layer.addUrl", &asked), failed); + assert_eq!(s.run("layer.addUrl", &asked).unwrap()["pending"], true, "the next try"); + assert_eq!(s.take_url_requests().len(), 1); + // The next run: that failure again, in its place, then what the try that followed gave. + s.rerun_told.clear(); + let answer: CmdResult = Ok(json!({"layer": id})); + s.rerun_landed.push((asked.clone(), answer.clone(), Added::of(s.map(), &answer))); + assert_eq!(s.run("layer.addUrl", &asked), failed); + for _ in 0..2 { + assert_eq!(s.run("layer.addUrl", &asked), answer); + } + assert!(s.take_url_requests().is_empty(), "nothing asked again"); + } + + /// A document read off the frame lands for the command waiting for it; one whose command gave + /// up is dropped and forgotten. An unread one stays out, and asks nothing of the app (#1319). + #[test] + fn documents_read_off_the_frame_land_or_are_dropped() { + let reading = |answer| -> Arc, String>>>> { Arc::new(Mutex::new(answer)) }; + let doc = |n: u32| json!({"url": format!("https://x.org/{n}"), "document": true}); + let mut waits = UrlWaits::default(); + waits.wait(1, doc(1)); + waits.wait(2, doc(2)); + waits.wait(3, doc(3)); + waits.remember_given_up(4, doc(4)); + waits.reading = vec![ + (1, reading(Some(Ok(b"caps".to_vec())))), + (2, reading(Some(Err("https://x.org/2?token=hunter2: 404 Not Found".into())))), + (3, reading(None)), + (4, reading(Some(Ok(b"late".to_vec())))), + ]; + waits.documents_in(); + let landed = |n: u64| waits.landed.iter().find(|(m, ..)| *m == n); + assert_eq!(landed(1).unwrap().3.document.as_deref(), Some(&b"caps"[..])); + let e = landed(2).unwrap().2.as_ref().unwrap_err(); + assert!(e.contains("404") && !e.contains("hunter2"), "{e}"); + assert!(landed(3).is_none() && waits.reading.len() == 1 && waits.reading[0].0 == 3); + assert!(landed(4).is_none() && !waits.given_up(4, &doc(4)), "given up, it is forgotten"); + assert!(is_document(&doc(1)) && !is_document(&json!({"url": "https://x.org/1"}))); + } + /// A command run again checks that what its download added is still there: a layer with its /// id but other data (one added after an undo), a tile layer taken away (another of its /// address left), or a table taken away, is said, not answered (#1202). diff --git a/crates/engine/src/cmd/plugins.rs b/crates/engine/src/cmd/plugins.rs index e9e96ca14..3f5b13967 100644 --- a/crates/engine/src/cmd/plugins.rs +++ b/crates/engine/src/cmd/plugins.rs @@ -730,6 +730,11 @@ impl Session { // Queued already: this number for this address (another caller may give the number). let taken = self.url_requests.iter().any(|q| q["request"].as_u64() == Some(*n) && fetch::same_request(q, asked)); if !taken && !self.url_waits.has(*n) { + // A service's document is read here: the app adds nothing for it. + if fetch::is_document(asked) { + self.url_waits.read_document(*n, asked); + continue; + } let mut q = asked.clone(); q["request"] = json!(n); self.url_requests.push(q); diff --git a/crates/engine/src/cmd/service.rs b/crates/engine/src/cmd/service.rs index b9a2bf1e0..ecc07eef2 100644 --- a/crates/engine/src/cmd/service.rs +++ b/crates/engine/src/cmd/service.rs @@ -111,7 +111,25 @@ impl Discovery { const MAX_SERVICE_DOCUMENTS: usize = 4; +/// Whether what this `service.add` would wait for is left off the frame: it is a plugin command's, +/// run by the desktop app, which runs the command again when it has come (#1319). +fn leaves_waits(s: &Session) -> bool { + cfg!(not(target_arch = "wasm32")) && s.leave_downloads && s.call_requests.is_some() +} + +/// What a `service.add` waiting for the document at `url` answers: its command runs again. +fn waiting_for(s: &mut Session, url: &str) -> CmdResult { + s.request_only = true; + Ok(json!({"pending": true, "url": url})) +} + fn capabilities(p: &Value) -> Result { + discover(p, None) +} + +/// Reads a service's documents, fetching what `p` doesn't give: waiting for them, or with `left`, +/// off the frame ([`Session::left_document`]), when a document not in yet is its `next`. +fn discover(p: &Value, mut left: Option<&mut Session>) -> Result { let original = url_of(p)?; let mut address = crate::render::rustgis_tiles::logins::inherit(original, &rustgis_data::ogc::capabilities_url(original, ps(p, "type"))); @@ -153,6 +171,12 @@ fn capabilities(p: &Value) -> Result { return Ok(result); }; rustgis_data::ogc::parse(&address, text.as_bytes()) + } else if let Some(s) = left.as_deref_mut() { + let Some(read) = s.left_document(&address) else { + result.next = Some(address); + return Ok(result); + }; + read.and_then(|bytes| rustgis_data::ogc::parse(&address, &bytes)).map_err(|e| format!("{original}: {e}")) } else { fetch_capabilities(original, &address) }; @@ -372,6 +396,25 @@ fn advertised_template(list: &str, vector: bool) -> Option { None } +/// [`advertised_template`] read off the frame: `Err` names the document it waits for. +fn left_template(s: &mut Session, list: &str, vector: bool) -> Result, String> { + use rustgis_data::ogc::{TilesStep, resolve_tiles}; + let mut doc = list.to_string(); + for _ in 0..2 { + if crate::render::rustgis_tiles::address::check(&doc, Some(list)).is_err() { + return Ok(None); + } + let Some(read) = s.left_document(&doc) else { return Err(doc) }; + let Ok(body) = read else { return Ok(None) }; + match resolve_tiles(list, &doc, &body, vector) { + Some(TilesStep::Template(t)) => return Ok(checked_template(list, &t).ok()), + Some(TilesStep::Next(next)) => doc = crate::render::rustgis_tiles::logins::inherit(&doc, &next), + None => return Ok(None), + } + } + Ok(None) +} + /// In the browser nothing waits: the app reads the template itself and passes it as `"tiles"`. #[cfg(target_arch = "wasm32")] fn advertised_template(_: &str, _: bool) -> Option { @@ -442,7 +485,12 @@ pub(crate) fn add(s: &mut Session, p: &Value) -> CmdResult { let p = bound.as_ref(); let url = url_of(p)?.to_string(); let want = ps(p, "layer").map(str::trim).filter(|l| !l.is_empty()).ok_or("layer is required")?; - let found = capabilities(p)?; + let leaving = leaves_waits(s); + let reads_off_frame = leaving && ps(p, "text").is_none(); + let found = discover(p, reads_off_frame.then_some(&mut *s))?; + if let Some(next) = found.next.as_deref().filter(|_| reads_off_frame) { + return waiting_for(s, next); + } let candidates = || { found.service.layers.iter().zip(&found.sources).filter(|(_, source)| ps(p, "source_url").is_none_or(|s| same_address(s, source))) }; @@ -516,6 +564,10 @@ pub(crate) fn add(s: &mut Session, p: &Value) -> CmdResult { // the usual path. let advertised = match (&layer.tilesets, ps(p, "tiles")) { (Some(_), Some(t)) => Some(checked_template(source, t)?), + (Some(list), None) if leaving => match left_template(s, list, matches!(layer.offer, Offer::VectorTiles { .. })) { + Ok(template) => template, + Err(doc) => return waiting_for(s, &doc), + }, (Some(list), None) => advertised_template(list, matches!(layer.offer, Offer::VectorTiles { .. })), _ => None, }; @@ -613,7 +665,19 @@ pub(crate) fn add(s: &mut Session, p: &Value) -> CmdResult { Err(format!("{url}: '{want}' is a folder or service the directory lists: list its layers (service.layers with url {open})")) } Offer::Coverage(c) => { - let (get, _) = coverage_request(s, p, c)?; + let described = crate::render::rustgis_tiles::logins::inherit(&c.endpoint, &rustgis_data::ogc::describe_coverage_url(c)); + let info = match ps(p, "describe") { + Some(text) => rustgis_data::ogc::parse_describe(c, text.as_bytes())?, + None if leaving => match s.left_document(&described) { + Some(read) => { + let bytes = read.map_err(|e| format!("{}: {e}", c.id))?; + rustgis_data::ogc::parse_describe(c, &bytes)? + } + None => return waiting_for(s, &described), + }, + None => fetch_describe(c, &described)?, + }; + let (get, _) = coverage_request_from(s, p, c, &info)?; #[cfg(target_arch = "wasm32")] { let _ = get; @@ -621,6 +685,9 @@ pub(crate) fn add(s: &mut Session, p: &Value) -> CmdResult { } #[cfg(not(target_arch = "wasm32"))] { + if leaving { + return super::layer::add_url(s, &json!({"url": get, "name": name, "coverage": true})); + } let bytes = crate::render::rustgis_tiles::cache::download_blocking(&get).map_err(|e| format!("{name}: {e}"))?; coverage_check(&name, &bytes)?; let r = super::raster::add_named_bytes(s, &json!({"name": name}), format!("{name}.tif"), bytes, None)?; @@ -641,24 +708,46 @@ pub(crate) fn add(s: &mut Session, p: &Value) -> CmdResult { // An area asked for, whether or not the request could carry it: a slow whole layer // then says so, and doesn't suggest the `bbox` already given. let given = p.get("bbox").is_some_and(|b| !b.is_null() && *b != "full"); - if given && asked.is_none() { - let why = if area.is_none() { "an area across the antimeridian" } else { "its request can't carry an area" }; - s.notify(format!("{name}: {why}, so the whole layer is asked for"), false); - } - let max = crate::render::rustgis_tiles::cache::MAX_DOWNLOAD; let from = asked.as_deref().unwrap_or(items); // An area request keeps the view until the requested box is applied below. let keep_view = asked.is_some().then_some(false); - let mut r = match download_layer(s, &name, from, max, true).map_err(|e| slow_whole_layer(e, given))? { - Downloaded::Bytes(bytes) => { - let bytes = features_answer(s, &name, bytes, layer.bbox); - super::raster::add_named_bytes(s, &json!({"name": name}), format!("{name}.geojson"), bytes, keep_view)? + // A plugin's command: downloaded off the frame as Add From Service does, added, and + // its command run again, which is answered here and goes on (#1319). + let left = leaving.then(|| { + let mut q = json!({"url": from, "name": name, "service": true}); + if let Some(bounds) = layer.bbox { + q["bounds"] = json!(bounds); + } + if let Some(zoom) = keep_view { + q["zoom"] = json!(zoom); + } + q + }); + // (By the run that has its answer: a run that waits is undone, and this goes with it.) + if given && asked.is_none() && left.as_ref().is_none_or(|q| s.rerun_has(q)) { + let why = if area.is_none() { "an area across the antimeridian" } else { "its request can't carry an area" }; + s.notify(format!("{name}: {why}, so the whole layer is asked for"), false); + } + let mut r = if let Some(q) = left { + let r = super::layer::add_url(s, &q).map_err(|e| slow_whole_layer(e, given))?; + if r.get("pending").is_some() { + return Ok(r); } - // Read in protocol buffer pages: their positions as the service's system - // places them, so there are no axes to check. - Downloaded::Read(read) => super::layer::add_read(s, &json!({"name": name}), *read, keep_view)?, + r + } else { + let max = crate::render::rustgis_tiles::cache::MAX_DOWNLOAD; + let r = match download_layer(s, &name, from, max, true).map_err(|e| slow_whole_layer(e, given))? { + Downloaded::Bytes(bytes) => { + let bytes = features_answer(s, &name, bytes, layer.bbox); + super::raster::add_named_bytes(s, &json!({"name": name}), format!("{name}.geojson"), bytes, keep_view)? + } + // Read in protocol buffer pages: their positions as the service's system + // places them, so there are no axes to check. + Downloaded::Read(read) => super::layer::add_read(s, &json!({"name": name}), *read, keep_view)?, + }; + mark_service_source(s, &r, from); + r }; - mark_service_source(s, &r, from); // Over an area, the features crossing it: the view stays, or goes to a box given // (the whole layer's bounds would show a country to fill one state). match (asked.is_some(), area, p.get("bbox")) { @@ -720,8 +809,18 @@ fn coverage_request(s: &Session, p: &Value, c: &Coverage) -> Result<(String, [f6 Some(text) => rustgis_data::ogc::parse_describe(c, text.as_bytes())?, None => fetch_describe(c, &describe)?, }; - let area = coverage_area(s, p, &info, &c.id)?; - let get = crate::render::rustgis_tiles::logins::inherit(&c.endpoint, &rustgis_data::ogc::get_coverage_url(c, &info, area)); + coverage_request_from(s, p, c, &info) +} + +/// [`coverage_request`] with its description read. +fn coverage_request_from( + s: &Session, + p: &Value, + c: &Coverage, + info: &rustgis_data::ogc::CoverageInfo, +) -> Result<(String, [f64; 4]), String> { + let area = coverage_area(s, p, info, &c.id)?; + let get = crate::render::rustgis_tiles::logins::inherit(&c.endpoint, &rustgis_data::ogc::get_coverage_url(c, info, area)); crate::render::rustgis_tiles::address::check(&get, Some(&c.endpoint))?; Ok((get, area)) } diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 3e3da2279..ed1e96895 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -731,6 +731,8 @@ pub struct Session { /// While a plugin command runs again for its downloads: what each one asked for, and what /// adding it answered, for its `layer.addUrl` to answer once. pub(crate) rerun_landed: Vec<(serde_json::Value, CmdResult, cmd::plugins::fetch::Added)>, + /// Which of those are failures already told in this run (see [`Session::rerun_answer`]). + pub(crate) rerun_told: Vec, /// While a plugin command runs again: the errors its earlier runs said, not said again. pub(crate) rerun_said: Vec, /// While plugin commands whose downloads are in run again: the downloads they wait for, still @@ -1038,6 +1040,7 @@ impl Session { next_request: 0, call_requests: None, rerun_landed: Vec::new(), + rerun_told: Vec::new(), rerun_said: Vec::new(), rerun_requests: Vec::new(), retry_tries: 0, diff --git a/crates/ui-egui/src/pluginpanel.rs b/crates/ui-egui/src/pluginpanel.rs index 08c586655..ed359ac51 100644 --- a/crates/ui-egui/src/pluginpanel.rs +++ b/crates/ui-egui/src/pluginpanel.rs @@ -686,6 +686,349 @@ mod tests { plugins::unload("ui-fail"); } + /// What a server was asked and hasn't answered yet. + type Held = std::sync::Arc>>; + + /// A service that answers only when told to ([`answer_held`]): what the window does meanwhile + /// can't be waiting for it. + fn hold_requests() -> Held { + let held = Held::default(); + let keep = held.clone(); + rustgis_engine::render::rustgis_tiles::cache::set_fetcher(move |url, done| keep.lock().unwrap().push((url, done))); + held + } + + /// Answers what was asked by `answer`, and says what that was. + fn answer_held(held: &Held, answer: &dyn Fn(&str) -> Result, String>) -> Vec { + let asked: Vec<_> = held.lock().unwrap().drain(..).collect(); + asked + .into_iter() + .map(|(url, done)| { + done(answer(&url)); + url + }) + .collect() + } + + /// What the frame loop does for a waiting command: starts what it asked for, adds what came + /// in, and runs it again when it can. + fn frame(app: &mut RustGisApp) -> Vec<(String, Result)> { + for p in app.session.take_url_requests() { + let _ = app.run("ui.addUrl", p); + } + app.poll_data_downloads(); + crate::serviceui::poll_pages(app, &egui::Context::default()); + app.session.poll_plugin_retries() + } + + /// Frames until the command stops waiting: every address asked for, in order, and how it ended. + fn run_out( + app: &mut RustGisApp, + held: &Held, + answer: &dyn Fn(&str) -> Result, String>, + ) -> (Vec, Result) { + let mut asked = Vec::new(); + let mut end = Err("it never ran again".to_string()); + for _ in 0..50 { + asked.extend(answer_held(held, answer)); + if let Some((_, r)) = frame(app).pop() { + end = r; + } + if !app.session.plugin_retries_waiting() && held.lock().unwrap().is_empty() { + return (asked, end); + } + } + panic!("it still waits: {asked:?} {end:?}"); + } + + const WFS: &str = r#" +solosSolosurn:ogc:def:crs:OGC:1.3:CRS84-74 -34-34 6"#; + const FEATURES: &str = r#"{"type":"FeatureCollection","features":[{"type":"Feature","geometry":{"type":"Point","coordinates":[-43,-5]},"properties":{"n":1}}]}"#; + + /// The WFS above: its capabilities, how many features it has, and its features. + fn wfs(url: &str) -> Result, String> { + Ok(if url.contains("GetCapabilities") { + WFS + } else if url.contains("RESULTTYPE=hits") { + r#""# + } else { + FEATURES + } + .as_bytes() + .to_vec()) + } + + fn solos(bbox: Value) -> Value { + json!({"op": "service.add", "params": {"url": "https://wfs.example.org/wfs", "type": "wfs", "layer": "solos", "name": "Solos", "bbox": bbox}}) + } + + /// The requests for a layer's features among `asked`, leaving out asking how many there are. + fn feature_requests(asked: &[String]) -> Vec<&String> { + asked.iter().filter(|u| u.contains("GetFeature") && !u.contains("RESULTTYPE=hits")).collect() + } + + fn whole_layer_notices(app: &RustGisApp, mark: u64) -> usize { + app.session.notices_since(mark).iter().filter(|n| n.text.contains("so the whole layer is asked for")).count() + } + + /// A plugin's `service.add` waits for nothing on the frame (#1319): its command answers with + /// the server still silent, the window says it is running, and the command runs again for each + /// thing that comes in, going on with the layer. An area its request carries is said nowhere. + #[test] + fn a_plugins_service_add_reads_off_the_frame() { + use rustgis_engine::plugins; + let _turn = crate::fetching::turn(); + let held = hold_requests(); + let pkg = plugins::Package::from_zip(&plugins::testing::package("ui-wfs", &["edit", "network"])).unwrap(); + plugins::load(pkg, "test").unwrap(); + let mut app = RustGisApp::with_sample(); + let mark = app.session.notice_mark(); + let layers = app.session.map().layers.len(); + let r = app.run("plugin:ui-wfs/f", solos(json!([-44, -6, -42, -4]))).unwrap(); + // Nothing has answered, and the command has. + assert_eq!(r["pending"], true, "{r}"); + assert!(r["downloading"][0].as_str().unwrap().contains("GetCapabilities"), "{r}"); + assert!(frame(&mut app).is_empty()); + assert!(frame(&mut app).is_empty(), "it waits, frame after frame"); + assert_eq!(held.lock().unwrap().len(), 1, "one request, for the capabilities"); + assert_eq!(app.session.map().layers.len(), layers); + assert!(crate::pluginstart::running(&app).is_some_and(|t| t.contains("Running"))); + // An error said meanwhile isn't hidden by it. + app.session.notify("something else went wrong".to_string(), true); + assert_eq!(crate::pluginstart::running(&app), None); + app.session.notify("back to normal".to_string(), false); + // The capabilities in, it asks for the features, off the frame too. + assert_eq!(answer_held(&held, &wfs).len(), 1); + let again = frame(&mut app); + assert_eq!(again.len(), 1, "{again:?}"); + assert_eq!(again[0].1.as_ref().unwrap()["pending"], true, "{again:?}"); + assert!(frame(&mut app).is_empty()); + assert_eq!(held.lock().unwrap().len(), 1, "one request, for the features"); + assert_eq!(app.session.map().layers.len(), layers, "not added before it comes"); + let (asked, end) = run_out(&mut app, &held, &wfs); + let r = end.unwrap(); + let features = feature_requests(&asked); + assert!(features.len() == 1 && features[0].contains("BBOX=-44,-6,-42,-4"), "{asked:?}"); + // It goes on with the layer as if `service.add` had waited: added once, known by its address. + let added = app.session.map().layer_by_name("Solos").unwrap(); + assert_eq!(r["layer"], json!(added.id), "{r}"); + assert_eq!(r["area"], json!([-44.0, -6.0, -42.0, -4.0]), "{r}"); + assert!(added.dataset.source.as_deref().is_some_and(|s| s.contains("GetFeature"))); + assert_eq!(app.session.map().layers.len(), layers + 1); + assert_eq!(whole_layer_notices(&app, mark), 0, "{:?}", app.session.notices_since(mark)); + assert_eq!(crate::pluginstart::running(&app), None); + // Its capabilities given by the command: only its features are asked for. + let mut given = solos(json!([-44, -6, -42, -4])); + given["params"]["text"] = json!(WFS); + given["params"]["name"] = json!("Given"); + let r = app.run("plugin:ui-wfs/f", given).unwrap(); + assert_eq!(r["pending"], true, "{r}"); + let (asked, end) = run_out(&mut app, &held, &wfs); + assert!(asked.iter().all(|u| !u.contains("GetCapabilities")) && feature_requests(&asked).len() == 1, "{asked:?}"); + assert!(end.is_ok() && app.session.map().layer_by_name("Given").is_some(), "{end:?}"); + plugins::unload("ui-wfs"); + } + + /// An area its request can't carry is said once, not once for each time the command runs. + #[test] + fn a_whole_layer_asked_for_is_said_once_by_a_command_run_again() { + use rustgis_engine::plugins; + let _turn = crate::fetching::turn(); + let held = hold_requests(); + let pkg = plugins::Package::from_zip(&plugins::testing::package("ui-whole", &["edit", "network"])).unwrap(); + plugins::load(pkg, "test").unwrap(); + let mut app = RustGisApp::with_sample(); + let mark = app.session.notice_mark(); + app.run("plugin:ui-whole/f", solos(json!([170, -6, -170, -4]))).unwrap(); + let (asked, end) = run_out(&mut app, &held, &wfs); + let r = end.unwrap(); + let features = feature_requests(&asked); + assert!(features.len() == 1 && !features[0].contains("BBOX"), "{asked:?}"); + assert!(r.get("area").is_none() && r["layer"].is_u64(), "{r}"); + assert_eq!(whole_layer_notices(&app, mark), 1, "{:?}", app.session.notices_since(mark)); + plugins::unload("ui-whole"); + } + + /// A service that fails answers the command at once, and that is said once: its capabilities, + /// and its features after them. + #[test] + fn a_service_that_fails_answers_a_plugins_service_add() { + use rustgis_engine::plugins; + let _turn = crate::fetching::turn(); + let held = hold_requests(); + let pkg = plugins::Package::from_zip(&plugins::testing::package("ui-down", &["edit", "network"])).unwrap(); + plugins::load(pkg, "test").unwrap(); + // Its features failing is said by the download, with the address, then by the command. + let down: [(&dyn Fn(&str) -> Result, String>, usize); 2] = [ + (&|_| Err("502 Bad Gateway".to_string()), 1), + (&|url| if url.contains("GetCapabilities") { wfs(url) } else { Err("502 Bad Gateway".to_string()) }, 2), + ]; + for (answer, said) in down { + let mut app = RustGisApp::with_sample(); + let mark = app.session.notice_mark(); + let layers = app.session.map().layers.len(); + app.run("plugin:ui-down/f", solos(json!([-44, -6, -42, -4]))).unwrap(); + let (_, end) = run_out(&mut app, &held, answer); + let e = end.unwrap_err(); + assert!(e.contains("502"), "{e}"); + assert_eq!(app.session.map().layers.len(), layers); + let notices = app.session.notices_since(mark); + let told = notices.iter().filter(|n| n.error && n.text.contains("502")).count(); + assert_eq!(told, said, "{notices:?}"); + assert!(frame(&mut app).is_empty(), "nothing left to run"); + } + plugins::unload("ui-down"); + } + + /// The window answers while a command waits, so the user may move the map: the command asking + /// for the view runs again with the one it began with, and asks for no other area. The user's + /// view stays, and the layer comes once. + #[test] + fn a_view_moved_while_a_command_waits_asks_for_no_other_area() { + use rustgis_engine::plugins; + let _turn = crate::fetching::turn(); + let held = hold_requests(); + let pkg = plugins::Package::from_zip(&plugins::testing::package("ui-view", &["edit", "network"])).unwrap(); + plugins::load(pkg, "test").unwrap(); + let mut app = RustGisApp::with_sample(); + let layers = app.session.map().layers.len(); + app.run("plugin:ui-view/f", solos(json!("view"))).unwrap(); + let mut moved = app.session.map().camera; + moved.center.x += 1_000_000.0; + app.session.map_mut().camera = moved; + let (asked, end) = run_out(&mut app, &held, &wfs); + let r = end.unwrap(); + let features = feature_requests(&asked); + assert!(features.len() == 1 && features[0].contains("BBOX"), "{asked:?}"); + assert!(r["layer"].is_u64(), "{r}"); + assert_eq!(app.session.map().layers.len(), layers + 1); + assert_eq!(app.session.map().camera, moved, "the user's view stays"); + plugins::unload("ui-view"); + } + + /// A box given moves the view when the layer is in, whatever the user did meanwhile: what the + /// command itself does to the view stands. + #[test] + fn a_box_given_frames_the_layer_once_it_is_in() { + use rustgis_engine::plugins; + let _turn = crate::fetching::turn(); + let held = hold_requests(); + let pkg = plugins::Package::from_zip(&plugins::testing::package("ui-box", &["edit", "network"])).unwrap(); + plugins::load(pkg, "test").unwrap(); + let mut app = RustGisApp::with_sample(); + let first = app.session.map().camera; + app.run("plugin:ui-box/f", solos(json!([-44, -6, -42, -4]))).unwrap(); + let (_, end) = run_out(&mut app, &held, &wfs); + end.unwrap(); + assert_ne!(app.session.map().camera, first); + plugins::unload("ui-box"); + } + + /// A map layer needs only its service's documents: its capabilities, and an OGC API layer's + /// tilesets, all off the frame; what it answers once they are in adds it. + #[test] + fn a_map_layer_of_a_plugins_service_add_needs_only_its_documents() { + use rustgis_engine::plugins; + const WMS: &str = r#" +Test WMS +image/png +EPSG:3857roadsRoads"#; + const API: &str = r#"{"collections": [{"id": "relief", "links": [{"rel": "tilesets-map", "href": "https://maps.example.com/relief/map/tiles"}]}]}"#; + const TILESETS: &str = r#"{"tilesets": [{"tileMatrixSetId": "WebMercatorQuad", "links": [{"rel": "self", "href": "https://maps.example.com/relief/map/tiles/smerc"}]}]}"#; + const TILESET: &str = r#"{"links": [{"rel": "item", "type": "image/png", "href": "https://maps.example.com/relief/{tileMatrix}/{tileRow}/{tileCol}.png"}]}"#; + let _turn = crate::fetching::turn(); + let held = hold_requests(); + let pkg = plugins::Package::from_zip(&plugins::testing::package("ui-map", &["edit", "network"])).unwrap(); + plugins::load(pkg, "test").unwrap(); + let mut app = RustGisApp::with_sample(); + let wms = json!({"op": "service.add", "params": {"url": "https://maps.example.com/wms", "type": "wms", "layer": "roads"}}); + assert_eq!(app.run("plugin:ui-map/f", wms).unwrap()["pending"], true); + assert!(app.session.map().tiles.iter().all(|t| t.name != "Roads")); + let (asked, end) = run_out(&mut app, &held, &|_| Ok(WMS.as_bytes().to_vec())); + assert_eq!(asked.len(), 1, "{asked:?}"); + assert!(end.is_ok() && app.session.map().tiles.iter().any(|t| t.name == "Roads"), "{end:?}"); + // An OGC API layer: its collections, the tileset list, then the tileset with its template. + let api = json!({"op": "service.add", "params": {"url": "https://maps.example.com/collections", "type": "ogcapi", "layer": "relief (map tiles)"}}); + assert_eq!(app.run("plugin:ui-map/f", api).unwrap()["pending"], true); + let (asked, end) = run_out(&mut app, &held, &|url| { + Ok(if url.contains("smerc") { + TILESET + } else if url.contains("/relief/") { + TILESETS + } else { + API + } + .as_bytes() + .to_vec()) + }); + assert_eq!(asked.len(), 3, "{asked:?}"); + assert!(end.is_ok(), "{end:?}"); + assert!(app.session.map().tiles.iter().any(|t| t.url == "https://maps.example.com/relief/{z}/{y}/{x}.png")); + plugins::unload("ui-map"); + } + + /// A coverage: its capabilities and description off the frame, then downloaded and added as + /// Add From Service does, and the command goes on with it; the service's error is its answer. + #[test] + fn a_coverage_of_a_plugins_service_add_reads_off_the_frame() { + use rustgis_engine::data::raster as r; + use rustgis_engine::plugins; + const WCS: &str = r#" +Soils +socSoil carbon +5 49.25.8 50 +"#; + const DESCRIBE: &str = r#" +49.2 550 5.8"#; + let raster = r::Raster::new( + "soc", + 8, + 8, + vec![vec![3.0; 64]], + r::GeoTransform { origin_x: 5.0, origin_y: 50.0, pixel_width: 0.1, pixel_height: -0.1 }, + r::Crs::LonLat, + None, + r::ColorHint::Gray, + "Float32", + ) + .unwrap(); + let tif = r::geotiff::write(&raster, 0).unwrap(); + let _turn = crate::fetching::turn(); + let held = hold_requests(); + let pkg = plugins::Package::from_zip(&plugins::testing::package("ui-wcs", &["edit", "network"])).unwrap(); + plugins::load(pkg, "test").unwrap(); + let mut app = RustGisApp::with_sample(); + let add = |name: &str| json!({"op": "service.add", "params": {"url": "https://wcs.example.org/wcs", "type": "wcs", "layer": "soc", "name": name, "bbox": [5, 49.2, 5.4, 50]}}); + assert_eq!(app.run("plugin:ui-wcs/f", add("Carbon")).unwrap()["pending"], true); + let doc = |url: &str| { + (if url.contains("GetCapabilities") { + Some(WCS) + } else if url.contains("DescribeCoverage") { + Some(DESCRIBE) + } else { + None + }) + .map(str::as_bytes) + .map(<[u8]>::to_vec) + }; + let (asked, end) = run_out(&mut app, &held, &|url| Ok(doc(url).unwrap_or_else(|| tif.clone()))); + let r = end.unwrap(); + assert!(asked.len() == 3 && asked[2].contains("GetCoverage"), "{asked:?}"); + let added = app.session.map().layer_by_name("Carbon").unwrap(); + assert!(added.is_raster() && r["layer"] == json!(added.id), "{r}"); + assert!(added.dataset.source.as_deref().is_some_and(|s| s.contains("GetCoverage"))); + // The service answers with its own error, not a GeoTIFF. + assert_eq!(app.run("plugin:ui-wcs/f", add("Refused")).unwrap()["pending"], true); + let (_, end) = + run_out( + &mut app, + &held, + &|url| Ok(doc(url).unwrap_or_else(|| b"no".to_vec())), + ); + assert!(end.is_err() && app.session.map().layer_by_name("Refused").is_none(), "{end:?}"); + plugins::unload("ui-wcs"); + } + /// A service's page that fails is what the plugin command that asked for it is answered, when /// it fails, not the wait running out (#1202). #[test] diff --git a/crates/ui-egui/src/pluginstart.rs b/crates/ui-egui/src/pluginstart.rs index 8d7571391..9bcfd3e0d 100644 --- a/crates/ui-egui/src/pluginstart.rs +++ b/crates/ui-egui/src/pluginstart.rs @@ -1,5 +1,6 @@ //! A plugin command the user asks for in the interface starts a frame or two later, so the window -//! first shows that it is running (#1319): the command may wait for a server on this thread, and +//! first shows that it is running (#1319): the command may wait for a server on this thread (unless +//! it is waiting in `layer.addUrl` or `service.add`, which leave that to the frame loop), and //! nothing is drawn until it has finished. Clicks made meanwhile, which the window system holds //! until then, are dropped, so a second click out of impatience doesn't run it again. Scripts, the //! AI Assistant and attached agents run commands at once ([`RustGisApp::run`]). @@ -39,8 +40,12 @@ pub struct State { } /// Leaves plugin command `id` to start once the window has shown it. The same command with the -/// same parameters, already waiting, isn't asked twice. +/// same parameters, already waiting to start or for what it asked of the web, isn't asked twice: +/// the status bar says it is running. pub(crate) fn ask(app: &mut RustGisApp, id: &str, params: Value) { + if app.session.plugin_command_waits(id, ¶ms) { + return; + } let opened = app.session.opened; let waiting = &mut app.ui.plugin_start.waiting; if !waiting.iter().any(|s| s.id == id && s.params == params && s.opened == opened) { @@ -106,10 +111,13 @@ pub(crate) fn frame_drawn(app: &mut RustGisApp, ctx: &egui::Context) { } } -/// What the status bar says while a command waits to start: `Running BrasilData: Solos…`. +/// What the status bar says while a command waits to start, or waits off the frame for what it +/// asked of the web: `Running BrasilData: Solos…`. An error said meanwhile goes first: a command +/// may wait for minutes, while the window takes other work. pub(crate) fn running(app: &RustGisApp) -> Option { - let title = title(app, &app.ui.plugin_start.waiting.front()?.id); - Some(i18n::fmt("Running {command}…", &[("command", &title)])) + let waiting = || app.session.plugin_command_waiting().filter(|_| !app.session.notices.last().is_some_and(|n| n.error)); + let id = app.ui.plugin_start.waiting.front().map(|s| s.id.as_str()).or_else(waiting)?; + Some(i18n::fmt("Running {command}…", &[("command", &title(app, id))])) } /// A plugin command as the user knows it: `BrasilData: Solos`. @@ -206,6 +214,23 @@ mod tests { plugins::unload("ui-turn"); } + /// The window answers while a command waits for the web, so a click can reach it: the same + /// command isn't started over, and another, or the same with other parameters, is (#1319). + #[test] + fn a_command_waiting_for_the_web_isnt_asked_again() { + let pkg = plugins::Package::from_zip(&plugins::testing::package("ui-again-asked", &["edit", "network"])).unwrap(); + plugins::load(pkg, "test").unwrap(); + let mut app = RustGisApp::with_sample(); + let (id, params) = ("plugin:ui-again-asked/f", json!({"op": "layer.addUrl", "params": {"url": "https://x.org/a.csv"}})); + app.run(id, params.clone()).unwrap(); + assert!(app.session.plugin_retries_waiting()); + ask(&mut app, id, params); + assert!(app.ui.plugin_start.waiting.is_empty()); + ask(&mut app, id, json!({"op": "layer.addUrl", "params": {"url": "https://x.org/b.csv"}})); + assert_eq!(app.ui.plugin_start.waiting.len(), 1); + plugins::unload("ui-again-asked"); + } + /// Clicks are dropped once, after a long run; what the pointer did otherwise stays. #[test] fn clicks_made_during_a_long_run_are_dropped_once() { diff --git a/docs/plugins.md b/docs/plugins.md index dd099e62b..5b11b1f7a 100644 --- a/docs/plugins.md +++ b/docs/plugins.md @@ -470,10 +470,26 @@ A call asked again while its download is under way (the button clicked twice) wa download, not another. A command runs again at most 64 times, and a download not added within 10 minutes runs it again with an error. -Other commands that wait for a server (`service.add`, say) still wait with the window held. A -command the user starts from the ribbon, command search or a panel first shows "Running …" in the -status bar with a busy cursor, and clicks made during a long run are dropped, so they don't run it -again. +`service.add` does the same on the desktop: it reads the service's documents off the frame, +answering `{"pending": true, "url": …}`, and leaves a feature layer or a coverage to download as +`layer.addUrl` does, so the same call answers what adding it answered once the command runs again +(with `"area"`, when asked for over one). The command may run several times on the way, once for +each document at most, and a map layer is added by the run that has them. In the web app +`service.add` still refuses to download. + +While a command waits the window answers, so the user can work and the project can change. The +status bar says the command is running, and asking for it again with the same parameters does +nothing. A run again sees the view the command first ran with, unless it moves the view itself; the +layers, the selection and the rest are as they are by then. A failed answer is given once a run, in +the order it came, so a call made again after it is a new try. Because everything before the call +runs again, a command replacing a layer removes the old one after the call: removed before it, the +run again finds its new layer gone, and says so. With the old name still in use the new layer came +in as `Name (2)`, so rename it once the old one is gone. + +Other commands that wait for a server (`service.layers`, `host.fetch`, say) still wait with the +window held. A command the user starts from the ribbon, command search or a panel first shows +"Running …" in the status bar with a busy cursor, and clicks made during a long run are dropped, +so they don't run it again. `rustgis-cli commands` lists every command id and its parameters. From a4750ba9ebcb2450a400d5dfc30ad82222d23763 Mon Sep 17 00:00:00 2001 From: elpidioassinaturas Date: Fri, 6 Nov 2026 14:53:13 -0300 Subject: [PATCH 2/5] fix(plugins): rerun_has isn't built for the browser, where nothing calls it The web app's build denies warnings, and the only caller is in service.add's native path. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/cmd/plugin_fetch.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/engine/src/cmd/plugin_fetch.rs b/crates/engine/src/cmd/plugin_fetch.rs index 13391830b..5a3771684 100644 --- a/crates/engine/src/cmd/plugin_fetch.rs +++ b/crates/engine/src/cmd/plugin_fetch.rs @@ -679,6 +679,7 @@ impl Session { } /// Whether a command run again has an answer for `wanted`. + #[cfg(not(target_arch = "wasm32"))] pub(crate) fn rerun_has(&self, wanted: &Value) -> bool { let wanted = self.asked_for(wanted); self.rerun_landed.iter().any(|(asked, ..)| same_request(asked, &wanted)) From 3d45bf4564ba1a2dc2f1f76cac86cdd074c08f11 Mon Sep 17 00:00:00 2001 From: elpidioassinaturas Date: Fri, 6 Nov 2026 15:46:17 -0300 Subject: [PATCH 3/5] fix(plugins): review round 1 of a plugin's service.add off the frame (#1319) - A run again holds the window's size with the view, and holds them whichever map is shown, so it asks for the area it began with. - A document asked for twice in a call is one read; up to 16 are read at a time; a refused run starts no read. - What a command waits for is named without its keys. - Tile sets not read off the frame are said. - A coverage is downloaded whole, not probed as a COG. - An error from before a command waits doesn't hide that it is running. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/cmd/plugin_fetch.rs | 95 ++++++++++++++++++++++----- crates/engine/src/cmd/plugins.rs | 10 ++- crates/engine/src/cmd/service.rs | 30 +++++++-- crates/ui-egui/src/pluginpanel.rs | 75 +++++++++++++++++++-- crates/ui-egui/src/pluginstart.rs | 7 +- docs/plugins.md | 10 +-- 6 files changed, 191 insertions(+), 36 deletions(-) diff --git a/crates/engine/src/cmd/plugin_fetch.rs b/crates/engine/src/cmd/plugin_fetch.rs index 5a3771684..55427f007 100644 --- a/crates/engine/src/cmd/plugin_fetch.rs +++ b/crates/engine/src/cmd/plugin_fetch.rs @@ -295,9 +295,12 @@ const DOCUMENT_TIMEOUT: Duration = Duration::from_secs(60); pub(crate) struct Retry { id: String, params: Value, - /// The map shown when it first ran, and its view: the window answers while the command waits, - /// and a view moved meanwhile must not make the next run ask for another area (#1319). - view: (u64, crate::map::Camera), + /// The view it first ran with, and the size of the window showing it: the window answers while + /// the command waits, and a view changed meanwhile must not make the next run ask for another + /// area (#1319). + view: (crate::map::Camera, (f64, f64)), + /// The notices given when it began to wait: one given since is about the time it waits. + notices: u64, urls: Vec, /// The downloads it left to the app ([`Session::request_url`]), by request number. requests: Vec, @@ -664,12 +667,25 @@ impl Session { pub(crate) fn left_document(&mut self, url: &str) -> Option, String>> { let asked = self.asked_for(&json!({"url": url, "document": true})); if let Some((answer, added)) = self.rerun_answer(&asked) { - return Some(answer.and_then(|_| added.document.ok_or_else(|| format!("{url}: no document came")))); + return Some(answer.and_then(|_| added.document.ok_or_else(|| "no document came".to_string()))); } - let number = self.url_waits.in_flight(&asked).unwrap_or_else(|| { - self.next_request += 1; - self.next_request - }); + // Asked for already in this call, or by a run before it: that read, not another. + let now = self.call_requests.as_deref().unwrap_or_default(); + let number = match now.iter().find(|(_, q)| same_request(q, &asked)).map(|(n, _)| *n).or_else(|| self.url_waits.in_flight(&asked)) { + Some(n) => n, + None => { + // Each one read stays in memory, so they wait in the numbers a download does. + let reading = self.url_waits.reading.len() + now.iter().filter(|(_, q)| is_document(q)).count(); + if reading >= crate::MAX_URL_REQUESTS { + return Some(Err(format!( + "{} documents are already being read; try again when they have come", + crate::MAX_URL_REQUESTS + ))); + } + self.next_request += 1; + self.next_request + } + }; if let Some(asked_now) = self.call_requests.as_mut() && !asked_now.iter().any(|(n, _)| *n == number) { @@ -759,7 +775,8 @@ impl Session { id: id.to_string(), params: params.clone(), // (A run again holds the view of its first, so this keeps it.) - view: (self.map().instance_id(), self.map().camera), + view: (self.map().camera, self.view_size), + notices: self.notice_mark(), urls, requests: numbers, answered, @@ -803,9 +820,13 @@ impl Session { self.plugin_retries.iter().any(|r| r.id == id && r.params == *params) } - /// The plugin command waiting longest for its fetches or downloads, for the front end to show. + /// The plugin command waiting longest for its fetches or downloads, for the front end to show: + /// not while the latest notice is an error given since it began to wait, which goes first. A + /// command may wait for minutes, while the window takes other work. pub fn plugin_command_waiting(&self) -> Option<&str> { - self.plugin_retries.first().map(|r| r.id.as_str()) + let first = self.plugin_retries.first()?; + let failed = self.notices_since(first.notices).last().is_some_and(|n| n.error); + (!failed).then_some(first.id.as_str()) } /// Runs again the plugin commands whose fetches have all landed and whose downloads the app @@ -855,14 +876,17 @@ impl Session { self.rerun_landed = r.answered.iter().cloned().chain(mine).collect(); self.rerun_said = r.said; self.retry_tries = r.tries; - // It sees the view it first ran with; one it doesn't move itself goes back to what - // the user has moved to since. - let held = r.view.1; - let shown = (r.view.0 == self.map().instance_id()).then(|| std::mem::replace(&mut self.map_mut().camera, held)); + // It sees the view it first ran with, whichever map is shown now, so it asks for the + // same area; one it doesn't move itself goes back to what the user has moved to since. + let (held, size) = r.view; + let map = self.map().instance_id(); + let shown = std::mem::replace(&mut self.map_mut().camera, held); + let shown_size = std::mem::replace(&mut self.view_size, size); // Run as the app runs a plugin's command: what it asks for now is left to the app. let res = self.run_leaving_downloads(&r.id, &r.params); - if let Some(shown) = shown.filter(|_| self.map().camera == held) { - self.map_mut().camera = shown; + self.view_size = shown_size; + if let Some(m) = self.project.maps.iter_mut().find(|m| m.instance_id() == map && m.camera == held) { + m.camera = shown; } self.retry_tries = 0; self.rerun_landed.clear(); @@ -909,6 +933,43 @@ mod tests { assert!(s.take_url_requests().is_empty(), "nothing asked again"); } + /// A document asked for twice in one call is one read, and they wait in the numbers a download + /// does: each stays in memory until its command has run (#1319). + #[test] + fn documents_asked_for_in_a_call_are_one_read_each_and_limited() { + let mut s = Session::new(crate::sample::world_project()); + s.call_requests = Some(Vec::new()); + assert!(s.left_document("https://x.org/same").is_none()); + assert!(s.left_document("https://x.org/same").is_none()); + assert_eq!(s.call_requests.as_ref().unwrap().len(), 1); + for i in 1..crate::MAX_URL_REQUESTS { + assert!(s.left_document(&format!("https://x.org/{i}")).is_none()); + } + let e = s.left_document("https://x.org/one-more").unwrap().unwrap_err(); + assert!(e.contains("already being read"), "{e}"); + assert_eq!(s.call_requests.as_ref().unwrap().len(), crate::MAX_URL_REQUESTS); + // One already asked for is still answered as waiting. + assert!(s.left_document("https://x.org/same").is_none()); + } + + /// A run refused for too many given-up downloads starts none of its documents' reads: with no + /// command waiting, nothing would take them (#1319). + #[test] + fn a_refused_run_starts_no_read() { + use rustgis_plugins as plugins; + let pkg = plugins::Package::from_zip(&rustgis_plugins::testing::package("refused-read", &["edit", "network"])).unwrap(); + plugins::load(pkg, "test").unwrap(); + let mut s = Session::new(crate::sample::world_project()); + for n in 0..MAX_EXPIRED as u64 { + s.url_waits.remember_given_up(n + 1000, json!({"url": format!("https://x.org/{n}")})); + } + let forwarded = json!({"op": "service.add", "params": {"url": "https://x.org/wfs", "type": "wfs", "layer": "a"}}); + let e = s.run_leaving_downloads("plugin:refused-read/f", &forwarded).unwrap_err(); + assert!(e.contains("still under way"), "{e}"); + assert!(s.url_waits.reading.is_empty()); + plugins::unload("refused-read"); + } + /// A document read off the frame lands for the command waiting for it; one whose command gave /// up is dropped and forgotten. An unread one stays out, and asks nothing of the app (#1319). #[test] diff --git a/crates/engine/src/cmd/plugins.rs b/crates/engine/src/cmd/plugins.rs index 3f5b13967..28040136c 100644 --- a/crates/engine/src/cmd/plugins.rs +++ b/crates/engine/src/cmd/plugins.rs @@ -726,13 +726,15 @@ impl Session { // The undone call took its download requests with it: they go to the app still, once, // unless the command gives up now (it has run again too often): then nothing waits for them. let giving_up = self.retry_tries >= fetch::MAX_RETRIES; + let mut documents = Vec::new(); for (n, asked) in requested.iter().filter(|_| !giving_up) { // Queued already: this number for this address (another caller may give the number). let taken = self.url_requests.iter().any(|q| q["request"].as_u64() == Some(*n) && fetch::same_request(q, asked)); if !taken && !self.url_waits.has(*n) { - // A service's document is read here: the app adds nothing for it. + // A service's document is read here, once the run is kept: the app adds nothing + // for it, and with no command waiting nothing would take it. if fetch::is_document(asked) { - self.url_waits.read_document(*n, asked); + documents.push((*n, asked.clone())); continue; } let mut q = asked.clone(); @@ -749,6 +751,10 @@ impl Session { self.rerun_said.truncate(earlier); if let Err(e) = &res { self.notify(e.clone(), true); + } else { + for (n, asked) in &documents { + self.url_waits.read_document(*n, asked); + } } res } diff --git a/crates/engine/src/cmd/service.rs b/crates/engine/src/cmd/service.rs index ecc07eef2..17cd3a55f 100644 --- a/crates/engine/src/cmd/service.rs +++ b/crates/engine/src/cmd/service.rs @@ -120,7 +120,8 @@ fn leaves_waits(s: &Session) -> bool { /// What a `service.add` waiting for the document at `url` answers: its command runs again. fn waiting_for(s: &mut Session, url: &str) -> CmdResult { s.request_only = true; - Ok(json!({"pending": true, "url": url})) + // (Without its keys: a saved login's may be in it, and the plugin gave only the service.) + Ok(json!({"pending": true, "url": crate::sources::redact(url)})) } fn capabilities(p: &Value) -> Result { @@ -405,11 +406,21 @@ fn left_template(s: &mut Session, list: &str, vector: bool) -> Result body, + Err(e) => { + s.notify(format!("{shown}: its tile sets couldn't be read, so the standard tile address is used: {e}"), false); + return Ok(None); + } + }; match resolve_tiles(list, &doc, &body, vector) { Some(TilesStep::Template(t)) => return Ok(checked_template(list, &t).ok()), Some(TilesStep::Next(next)) => doc = crate::render::rustgis_tiles::logins::inherit(&doc, &next), - None => return Ok(None), + None => { + s.notify(format!("{shown}: it names no tiles of this layer, so the standard tile address is used"), false); + return Ok(None); + } } } Ok(None) @@ -686,7 +697,8 @@ pub(crate) fn add(s: &mut Session, p: &Value) -> CmdResult { #[cfg(not(target_arch = "wasm32"))] { if leaving { - return super::layer::add_url(s, &json!({"url": get, "name": name, "coverage": true})); + // (Whole: a coverage's address may end in .tif, and it is read as a raster.) + return super::layer::add_url(s, &json!({"url": get, "name": name, "coverage": true, "whole": true})); } let bytes = crate::render::rustgis_tiles::cache::download_blocking(&get).map_err(|e| format!("{name}: {e}"))?; coverage_check(&name, &bytes)?; @@ -941,4 +953,14 @@ mod tests { assert_eq!(super::slow_whole_layer(slow.clone(), true), slow); assert_eq!(super::slow_whole_layer("404 Not Found".into(), false), "404 Not Found"); } + + /// What a command is told it waits for has no keys in it: a saved login's may be in the address. + #[test] + fn a_document_waited_for_is_named_without_its_keys() { + let mut s = crate::Session::new(crate::sample::world_project()); + let r = super::waiting_for(&mut s, "https://x.org/wfs?request=GetCapabilities&token=hunter2").unwrap(); + let url = r["url"].as_str().unwrap(); + assert!(url.starts_with("https://x.org/wfs") && !url.contains("hunter2"), "{r}"); + assert_eq!(r["pending"], true); + } } diff --git a/crates/ui-egui/src/pluginpanel.rs b/crates/ui-egui/src/pluginpanel.rs index ed359ac51..869246f80 100644 --- a/crates/ui-egui/src/pluginpanel.rs +++ b/crates/ui-egui/src/pluginpanel.rs @@ -767,6 +767,11 @@ mod tests { asked.iter().filter(|u| u.contains("GetFeature") && !u.contains("RESULTTYPE=hits")).collect() } + /// Whether `r` answers the area `e`, as near as it is written. + fn area_is(r: &Value, e: [f64; 4]) -> bool { + r["area"].as_array().is_some_and(|a| a.len() == 4 && a.iter().zip(e).all(|(v, x)| v.as_f64().is_some_and(|v| (v - x).abs() < 1e-3))) + } + fn whole_layer_notices(app: &RustGisApp, mark: u64) -> usize { app.session.notices_since(mark).iter().filter(|n| n.text.contains("so the whole layer is asked for")).count() } @@ -782,6 +787,8 @@ mod tests { let pkg = plugins::Package::from_zip(&plugins::testing::package("ui-wfs", &["edit", "network"])).unwrap(); plugins::load(pkg, "test").unwrap(); let mut app = RustGisApp::with_sample(); + // An error from before doesn't hide that it has started. + app.session.notify("an old error".to_string(), true); let mark = app.session.notice_mark(); let layers = app.session.map().layers.len(); let r = app.run("plugin:ui-wfs/f", solos(json!([-44, -6, -42, -4]))).unwrap(); @@ -880,9 +887,9 @@ mod tests { plugins::unload("ui-down"); } - /// The window answers while a command waits, so the user may move the map: the command asking - /// for the view runs again with the one it began with, and asks for no other area. The user's - /// view stays, and the layer comes once. + /// The window answers while a command waits, so the user may move the map or resize the window: + /// the command asking for the view runs again with the one it began with, and asks for no other + /// area. The user's view stays, and the layer comes once. #[test] fn a_view_moved_while_a_command_waits_asks_for_no_other_area() { use rustgis_engine::plugins; @@ -892,20 +899,54 @@ mod tests { plugins::load(pkg, "test").unwrap(); let mut app = RustGisApp::with_sample(); let layers = app.session.map().layers.len(); + app.session.run("map.zoomToExtent", &json!({"extent": [-44, -6, -42, -4]})).unwrap(); + let began = rustgis_engine::cmd::workspace::view_extent(&app.session); app.run("plugin:ui-view/f", solos(json!("view"))).unwrap(); let mut moved = app.session.map().camera; moved.center.x += 1_000_000.0; app.session.map_mut().camera = moved; + app.session.view_size = (600.0, 300.0); + assert_ne!(rustgis_engine::cmd::workspace::view_extent(&app.session), began); let (asked, end) = run_out(&mut app, &held, &wfs); let r = end.unwrap(); let features = feature_requests(&asked); assert!(features.len() == 1 && features[0].contains("BBOX"), "{asked:?}"); assert!(r["layer"].is_u64(), "{r}"); + assert!(area_is(&r, [began.min_x, began.min_y, began.max_x, began.max_y]), "{r} {began:?}"); assert_eq!(app.session.map().layers.len(), layers + 1); assert_eq!(app.session.map().camera, moved, "the user's view stays"); + assert_eq!(app.session.view_size, (600.0, 300.0)); plugins::unload("ui-view"); } + /// Another map shown while a command asking for the view waits: it asks for the area it began + /// with, once, and that map's view stays. The layer lands in the map shown, as a download does. + #[test] + fn another_map_shown_while_a_command_waits_asks_for_no_other_area() { + use rustgis_engine::plugins; + let _turn = crate::fetching::turn(); + let held = hold_requests(); + let pkg = plugins::Package::from_zip(&plugins::testing::package("ui-other-map", &["edit", "network"])).unwrap(); + plugins::load(pkg, "test").unwrap(); + let mut app = RustGisApp::with_sample(); + app.session.run("map.zoomToExtent", &json!({"extent": [-44, -6, -42, -4]})).unwrap(); + let began = rustgis_engine::cmd::workspace::view_extent(&app.session); + app.run("plugin:ui-other-map/f", solos(json!("view"))).unwrap(); + app.session.run("map.new", &json!({"name": "Second"})).unwrap(); + app.session.run("map.activate", &json!({"name": "Second"})).unwrap(); + let mut second = app.session.map().camera; + second.center.x += 1_000_000.0; + app.session.map_mut().camera = second; + let (asked, end) = run_out(&mut app, &held, &wfs); + let r = end.unwrap(); + assert_eq!(feature_requests(&asked).len(), 1, "{asked:?}"); + assert!(area_is(&r, [began.min_x, began.min_y, began.max_x, began.max_y]), "{r} {began:?}"); + assert_eq!(app.session.map().name, "Second"); + assert!(app.session.map().layer_by_name("Solos").is_some()); + assert_eq!(app.session.map().camera, second, "that map's view stays"); + plugins::unload("ui-other-map"); + } + /// A box given moves the view when the layer is in, whatever the user did meanwhile: what the /// command itself does to the view stands. #[test] @@ -916,11 +957,15 @@ mod tests { let pkg = plugins::Package::from_zip(&plugins::testing::package("ui-box", &["edit", "network"])).unwrap(); plugins::load(pkg, "test").unwrap(); let mut app = RustGisApp::with_sample(); - let first = app.session.map().camera; app.run("plugin:ui-box/f", solos(json!([-44, -6, -42, -4]))).unwrap(); + let mut moved = app.session.map().camera; + moved.center.x += 1_000_000.0; + app.session.map_mut().camera = moved; let (_, end) = run_out(&mut app, &held, &wfs); end.unwrap(); - assert_ne!(app.session.map().camera, first); + assert_ne!(app.session.map().camera, moved); + let center = app.session.map().camera.center_lonlat(); + assert!((center.x + 43.0).abs() < 0.01 && (center.y + 5.0).abs() < 0.01, "{center:?}"); plugins::unload("ui-box"); } @@ -964,6 +1009,19 @@ mod tests { assert_eq!(asked.len(), 3, "{asked:?}"); assert!(end.is_ok(), "{end:?}"); assert!(app.session.map().tiles.iter().any(|t| t.url == "https://maps.example.com/relief/{z}/{y}/{x}.png")); + let standard = |app: &RustGisApp, mark| { + app.session.notices_since(mark).iter().filter(|n| n.text.contains("the standard tile address is used")).count() + }; + assert_eq!(standard(&app, 0), 0, "the tiles it names are used"); + // Its tile sets not read: the standard address, and that is said, once. + let mark = app.session.notice_mark(); + let api = json!({"op": "service.add", "params": {"url": "https://maps.example.com/collections", "type": "ogcapi", "layer": "relief (map tiles)", "name": "Standard"}}); + assert_eq!(app.run("plugin:ui-map/f", api).unwrap()["pending"], true); + let (_, end) = run_out(&mut app, &held, &|url| { + if url.contains("/relief/") { Err("502 Bad Gateway".to_string()) } else { Ok(API.as_bytes().to_vec()) } + }); + assert!(end.is_ok(), "{end:?}"); + assert_eq!(standard(&app, mark), 1, "{:?}", app.session.notices_since(mark)); plugins::unload("ui-map"); } @@ -1017,6 +1075,13 @@ mod tests { let added = app.session.map().layer_by_name("Carbon").unwrap(); assert!(added.is_raster() && r["layer"] == json!(added.id), "{r}"); assert!(added.dataset.source.as_deref().is_some_and(|s| s.contains("GetCoverage"))); + // An address ending in .tif is a coverage too: downloaded whole, as a raster. + let tif_address = json!({"op": "service.add", "params": {"url": "https://wcs.example.org/soc.tif", "type": "wcs", "layer": "soc", "name": "Named", "bbox": [5, 49.2, 5.8, 50]}}); + assert_eq!(app.run("plugin:ui-wcs/f", tif_address).unwrap()["pending"], true); + let (asked, end) = run_out(&mut app, &held, &|url| Ok(doc(url).unwrap_or_else(|| tif.clone()))); + let r = end.unwrap(); + assert_eq!(asked.len(), 3, "the coverage asked for once, not probed: {asked:?}"); + assert!(app.session.map().layer_by_name("Named").is_some_and(|l| l.is_raster() && r["layer"] == json!(l.id)), "{r}"); // The service answers with its own error, not a GeoTIFF. assert_eq!(app.run("plugin:ui-wcs/f", add("Refused")).unwrap()["pending"], true); let (_, end) = diff --git a/crates/ui-egui/src/pluginstart.rs b/crates/ui-egui/src/pluginstart.rs index 9bcfd3e0d..8fdd8dbff 100644 --- a/crates/ui-egui/src/pluginstart.rs +++ b/crates/ui-egui/src/pluginstart.rs @@ -112,11 +112,10 @@ pub(crate) fn frame_drawn(app: &mut RustGisApp, ctx: &egui::Context) { } /// What the status bar says while a command waits to start, or waits off the frame for what it -/// asked of the web: `Running BrasilData: Solos…`. An error said meanwhile goes first: a command -/// may wait for minutes, while the window takes other work. +/// asked of the web: `Running BrasilData: Solos…`. While it waits off the frame, an error said +/// since it began to wait goes first. pub(crate) fn running(app: &RustGisApp) -> Option { - let waiting = || app.session.plugin_command_waiting().filter(|_| !app.session.notices.last().is_some_and(|n| n.error)); - let id = app.ui.plugin_start.waiting.front().map(|s| s.id.as_str()).or_else(waiting)?; + let id = app.ui.plugin_start.waiting.front().map(|s| s.id.as_str()).or_else(|| app.session.plugin_command_waiting())?; Some(i18n::fmt("Running {command}…", &[("command", &title(app, id))])) } diff --git a/docs/plugins.md b/docs/plugins.md index 5b11b1f7a..6a442119d 100644 --- a/docs/plugins.md +++ b/docs/plugins.md @@ -473,14 +473,16 @@ minutes runs it again with an error. `service.add` does the same on the desktop: it reads the service's documents off the frame, answering `{"pending": true, "url": …}`, and leaves a feature layer or a coverage to download as `layer.addUrl` does, so the same call answers what adding it answered once the command runs again -(with `"area"`, when asked for over one). The command may run several times on the way, once for -each document at most, and a map layer is added by the run that has them. In the web app +(a feature layer's with `"area"`, when asked for over one). The command may run several times on +the way, once for each document at most, and a map layer is added by the run that has them. Up to +16 documents are read at a time, and asking for more fails. In the web app `service.add` still refuses to download. While a command waits the window answers, so the user can work and the project can change. The status bar says the command is running, and asking for it again with the same parameters does -nothing. A run again sees the view the command first ran with, unless it moves the view itself; the -layers, the selection and the rest are as they are by then. A failed answer is given once a run, in +nothing. A run again sees the view the command first ran with, and the window's size then, on the +map shown by then, unless it moves the view itself; the layers, the selection and the rest are as +they are by then. A failed answer is given once a run, in the order it came, so a call made again after it is a new try. Because everything before the call runs again, a command replacing a layer removes the old one after the call: removed before it, the run again finds its new layer gone, and says so. With the old name still in use the new layer came From 0d1fc31fed451b4ff0e12cc069b561e06c71742e Mon Sep 17 00:00:00 2001 From: elpidioassinaturas Date: Fri, 6 Nov 2026 17:45:48 -0300 Subject: [PATCH 4/5] fix(plugins): layer.addUrl can't mark a download as a service's document (#1319) A download so marked was read for service.add, and no layer came, with nothing said. It is refused now. A test of a waiting command no longer uses the network. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/cmd/layer.rs | 4 ++++ crates/engine/src/cmd/plugin_fetch.rs | 14 ++++++++++++++ crates/ui-egui/src/pluginstart.rs | 3 +++ 3 files changed, 21 insertions(+) diff --git a/crates/engine/src/cmd/layer.rs b/crates/engine/src/cmd/layer.rs index e15573817..322dc88ee 100644 --- a/crates/engine/src/cmd/layer.rs +++ b/crates/engine/src/cmd/layer.rs @@ -3080,6 +3080,10 @@ fn add_url_as_given(s: &mut Session, p: &Value) -> CmdResult { if ["geolibreProject", "geolibreLayer", "geolibreZarr"].iter().any(|key| p.get(key).is_some()) { return Err("GeoLibre download state is internal; use project.importGeolibre".into()); } + // Marks a service's document, read and kept for `service.add`: as a download, no layer would come. + if super::plugins::fetch::is_document(p) { + return Err("\"document\" is internal to service.add: leave it out".into()); + } let url = ps(p, "url").map(str::trim).filter(|u| !u.is_empty()).ok_or("url is required")?; // A plugin command run again once the download it left to the app was added (#1202): what // adding it answered, as often as the run asks for it (the app downloaded and added it once). diff --git a/crates/engine/src/cmd/plugin_fetch.rs b/crates/engine/src/cmd/plugin_fetch.rs index 55427f007..f3650b85e 100644 --- a/crates/engine/src/cmd/plugin_fetch.rs +++ b/crates/engine/src/cmd/plugin_fetch.rs @@ -952,6 +952,20 @@ mod tests { assert!(s.left_document("https://x.org/same").is_none()); } + /// `layer.addUrl` can't mark its download as a service's document: nothing would add it, and + /// nothing would say so. A plugin's call and a direct one are refused, and nothing is read (#1319). + #[test] + fn a_download_cannot_be_marked_as_a_document() { + let mut s = Session::new(crate::sample::world_project()); + s.leave_downloads = true; + s.call_requests = Some(Vec::new()); + let marked = json!({"url": "https://x.org/a.csv", "document": true}); + assert!(s.run("layer.addUrl", &marked).unwrap_err().contains("internal")); + assert!(s.call_requests.as_ref().unwrap().is_empty() && s.url_waits.reading.is_empty()); + // Without the mark, it is a download as ever. + assert_eq!(s.run("layer.addUrl", &json!({"url": "https://x.org/a.csv"})).unwrap()["pending"], true); + } + /// A run refused for too many given-up downloads starts none of its documents' reads: with no /// command waiting, nothing would take them (#1319). #[test] diff --git a/crates/ui-egui/src/pluginstart.rs b/crates/ui-egui/src/pluginstart.rs index 8fdd8dbff..c66393ae6 100644 --- a/crates/ui-egui/src/pluginstart.rs +++ b/crates/ui-egui/src/pluginstart.rs @@ -217,6 +217,9 @@ mod tests { /// command isn't started over, and another, or the same with other parameters, is (#1319). #[test] fn a_command_waiting_for_the_web_isnt_asked_again() { + // A service that never answers: the command only has to be waiting. + let _turn = crate::fetching::turn(); + rustgis_engine::render::rustgis_tiles::cache::set_fetcher(|_, _| {}); let pkg = plugins::Package::from_zip(&plugins::testing::package("ui-again-asked", &["edit", "network"])).unwrap(); plugins::load(pkg, "test").unwrap(); let mut app = RustGisApp::with_sample(); From 9e3b2be0d6d816e43b8f48277e00e6c69f4091d5 Mon Sep 17 00:00:00 2001 From: elpidioassinaturas Date: Sat, 7 Nov 2026 00:22:33 -0300 Subject: [PATCH 5/5] test(plugins): a test's fetcher keeps no download of another test (#1319) The test of a command waiting for the web starts no download, so it needs neither a fetcher nor a turn. The fetcher it had dropped the `done` of any download another test started meanwhile, which left the in-flight count up for the rest of the process. `hold_requests` kept every download too, so one started by a test with no turn added to the request counts of the tests using it. It now keeps only its tests' `.example.` addresses and answers the others at once. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/pluginpanel.rs | 11 +++++++++-- crates/ui-egui/src/pluginstart.rs | 3 --- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/crates/ui-egui/src/pluginpanel.rs b/crates/ui-egui/src/pluginpanel.rs index 869246f80..42888cf06 100644 --- a/crates/ui-egui/src/pluginpanel.rs +++ b/crates/ui-egui/src/pluginpanel.rs @@ -690,11 +690,18 @@ mod tests { type Held = std::sync::Arc>>; /// A service that answers only when told to ([`answer_held`]): what the window does meanwhile - /// can't be waiting for it. + /// can't be waiting for it. A test with no turn may start a download meanwhile: that one isn't + /// counted as asked here, and is answered at once, as a fetcher must. fn hold_requests() -> Held { let held = Held::default(); let keep = held.clone(); - rustgis_engine::render::rustgis_tiles::cache::set_fetcher(move |url, done| keep.lock().unwrap().push((url, done))); + rustgis_engine::render::rustgis_tiles::cache::set_fetcher(move |url, done| { + if url.contains(".example.") { + keep.lock().unwrap().push((url, done)); + } else { + done(Err("not this test's service".into())); + } + }); held } diff --git a/crates/ui-egui/src/pluginstart.rs b/crates/ui-egui/src/pluginstart.rs index c66393ae6..8fdd8dbff 100644 --- a/crates/ui-egui/src/pluginstart.rs +++ b/crates/ui-egui/src/pluginstart.rs @@ -217,9 +217,6 @@ mod tests { /// command isn't started over, and another, or the same with other parameters, is (#1319). #[test] fn a_command_waiting_for_the_web_isnt_asked_again() { - // A service that never answers: the command only has to be waiting. - let _turn = crate::fetching::turn(); - rustgis_engine::render::rustgis_tiles::cache::set_fetcher(|_, _| {}); let pkg = plugins::Package::from_zip(&plugins::testing::package("ui-again-asked", &["edit", "network"])).unwrap(); plugins::load(pkg, "test").unwrap(); let mut app = RustGisApp::with_sample();