From 8f9af757c0088a484275c7b0920b4858b877af2b Mon Sep 17 00:00:00 2001 From: mgaux-dev Date: Fri, 30 Oct 2026 21:18:36 +0100 Subject: [PATCH 1/9] feat(services): Add From Service removes a layer it already added --- crates/ui-egui/src/dialogs.rs | 159 ++++++++++++++++++++++++++++++++-- 1 file changed, 154 insertions(+), 5 deletions(-) diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index f3d3bf3f1..c664c6a73 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -786,6 +786,57 @@ fn go_to(app: &mut RustGisApp, url: &str, kind: &str, way: Vec<(String, &'static /// How many of a service's layers Add From Service lists. const SHOWN_LAYERS: usize = 500; +/// An item of the active map whose `service.layers` offer is already there. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +enum AddedServiceItem { + /// A downloaded feature layer or coverage. + Layer(u32), + /// A WMS, WMTS, vector tile or style layer, drawn as tiles. + Tile(usize), +} + +/// Whether `name` is `title` as `service.add` names a layer, or a dated one (`"Roads 2024-01-01"`). +fn named_after(name: &str, title: &str) -> bool { + !title.is_empty() + && (name == title + || name + .strip_prefix(title) + .and_then(|rest| rest.strip_prefix(' ')) + .and_then(|date| date.chars().next()) + .is_some_and(|c| c.is_ascii_digit())) +} + +/// The item the service listing `l` already added to the active map, if it did: a tile layer by +/// its address (a dated address's `time_url` is the template the listing gives), or a feature or +/// coverage layer, which `service.add` names after the listed layer's title. +fn added_service_item(s: &rustgis_engine::Session, l: &Value) -> Option { + let offer = l.get("offer")?; + let kind = offer.get("type").and_then(Value::as_str)?; + let url = offer.get("url").and_then(Value::as_str).unwrap_or(""); + let title = l.get("title").and_then(Value::as_str).or_else(|| l.get("name").and_then(Value::as_str)).unwrap_or(""); + let map = s.map(); + match kind { + "tiles" | "vectorTiles" | "style" => { + // The address first: another service may have a layer of the same title. + let address = || (!url.is_empty()).then_some(url); + map.tiles + .iter() + .position(|t| address().is_some_and(|u| t.url == u || t.time_url.as_deref() == Some(u))) + .or_else(|| map.tiles.iter().position(|t| named_after(&t.name, title))) + .map(AddedServiceItem::Tile) + } + "features" | "coverage" => { + let address = || (!url.is_empty()).then_some(url); + map.layers + .iter() + .find(|layer| address().is_some_and(|u| layer.dataset.source.as_deref() == Some(u))) + .or_else(|| map.layers.iter().find(|layer| layer.name.eq_ignore_ascii_case(title))) + .map(|layer| AddedServiceItem::Layer(layer.id)) + } + _ => None, + } +} + /// Add From Service: a WMS, WMTS, WFS, WCS, OGC API Features or ArcGIS REST service by its address /// (or a catalogue searched for them, `cswui`). /// Connect fetches its capabilities in the background; its layers are then listed, each with an @@ -865,6 +916,7 @@ fn add_service(app: &mut RustGisApp, ui: &mut egui::Ui) -> bool { } // The service: connecting, an error, or its layers. let mut add: Option = None; + let mut remove: Option<(AddedServiceItem, String)> = None; let mut import = None; // A directory's folder or service to open, or Back to the directory before. let mut open: Option = None; @@ -1002,16 +1054,23 @@ fn add_service(app: &mut RustGisApp, ui: &mut egui::Ui) -> bool { ui.add_space(2.0); let kind = l.get("offer").and_then(|o| o.get("type")).and_then(Value::as_str); + let name = l.get("name").and_then(Value::as_str).unwrap_or(""); + let title = l.get("title").and_then(Value::as_str).unwrap_or(name); if kind == Some("service") { if ui.small_button(tl!("Open")).clicked() { open = l.get("offer").and_then(|o| o.get("url")).and_then(Value::as_str).map(str::to_string); } - } else if !(grouped && kind == Some("features")) && ui.small_button(tl!("Add")).clicked() { - add = Some(l.clone()); + } else if !(grouped && kind == Some("features")) { + match added_service_item(&app.session, l) { + Some(item) => { + if ui.small_button(tl!("Remove")).clicked() { + remove = Some((item, title.to_string())); + } + } + None if ui.small_button(tl!("Add")).clicked() => add = Some(l.clone()), + None => {} + } } - - let name = l.get("name").and_then(Value::as_str).unwrap_or(""); - let title = l.get("title").and_then(Value::as_str).unwrap_or(name); let what = match kind { Some("features") => tl!("features"), Some("style") => tl!("style"), @@ -1084,6 +1143,17 @@ fn add_service(app: &mut RustGisApp, ui: &mut egui::Ui) -> bool { { app.dialogs.error = Some(e); } + if let Some((item, title)) = remove { + let r = match item { + AddedServiceItem::Layer(id) => app.run("layer.remove", json!({"layer": id})), + AddedServiceItem::Tile(index) => app.run("map.removeTiles", json!({"index": index})), + }; + match r { + Ok(_) => app.dialogs.info = Some(crate::i18n::fmt("Removed {name}", &[("name", &title)])), + Err(e) => app.dialogs.error = Some(e), + } + app.map.invalidate(); + } // Into a directory's folder or service, keeping the way back; or back out of it. let here = app.ui.service.as_ref().map(|c| (c.url.clone(), c.kind)); if let (Some(url), Some(here)) = (open, here) { @@ -2218,4 +2288,83 @@ mod tests { assert_eq!(format_degrees(108.5), "108.5°"); assert_eq!(format_degrees(104.25), "104.25°"); } + + /// A downloaded feature layer keeps the listed title as its name, which tells the dialog it + /// is already there (and so offers Remove rather than Add). + #[test] + fn a_service_features_layer_is_known_once_added() { + let mut app = RustGisApp::with_sample(); + let listed = json!({ + "name": "places", + "title": "Service Places", + "offer": {"type": "features", "url": "https://example.com/collections/places/items"}, + }); + assert_eq!(added_service_item(&app.session, &listed), None); + app.run( + "layer.addDataText", + json!({ + "fileName": "places.geojson", + "name": "Service Places", + "text": r#"{"type":"FeatureCollection","features":[{"type":"Feature","properties":{},"geometry":{"type":"Point","coordinates":[0,0]}}]}"#, + }), + ) + .unwrap(); + let id = app.session.map().layer_by_name("Service Places").unwrap().id; + assert_eq!(added_service_item(&app.session, &listed), Some(AddedServiceItem::Layer(id))); + app.run("layer.remove", json!({"layer": id})).unwrap(); + assert_eq!(added_service_item(&app.session, &listed), None); + } + + /// Add From Service's row button says Remove for a service layer already on the map, and + /// removes it; once it is gone the row says Add again. + #[test] + fn a_service_layers_row_offers_to_remove_it_once_added() { + use egui_kittest::kittest::Queryable; + let mut app = RustGisApp::with_sample(); + let url = "https://example.com/service/{z}/{x}/{y}.png"; + app.run("map.addTiles", json!({"url": url, "name": "Service Roads", "overlay": true})).unwrap(); + app.ui.service = Some(crate::ServiceConn { + url: "https://example.com/service".into(), + kind: "wms", + slot: Default::default(), + text: None, + documents: Vec::new(), + pending: None, + listing: Some(json!({ + "kind": "WMS", + "title": "Roads", + "count": 1, + "layers": [{ + "name": "roads", + "title": "Service Roads", + "bbox": null, + "offer": {"type": "tiles", "url": url}, + }], + })), + error: None, + failure: None, + }); + app.run("ui.dialog", json!({"name": "addService"})).unwrap(); + app.dialogs.mode = "wms"; + let mut fonts = false; + let mut h = egui_kittest::Harness::builder().with_size(vec2(1200.0, 900.0)).build_ui_state( + move |ui, app: &mut RustGisApp| { + if !fonts { + crate::theme::install_fonts(ui.ctx(), &[], None); + fonts = true; + return; + } + let ctx = ui.ctx().clone(); + super::show(app, &ctx); + }, + app, + ); + h.run_steps(4); + assert!(h.query_by_label("Add").is_none(), "the layer is already on the map, so Add is not offered"); + h.get_by_label("Remove").click(); + h.run_steps(4); + assert!(h.state().session.map().tiles.iter().all(|t| t.url != url), "Remove took the tile layer off the map"); + assert!(h.query_by_label("Add").is_some()); + assert!(h.query_by_label("Remove").is_none(), "nothing is left to remove"); + } } From 1832f7063de13d8961727652cb9100a35eb07b28 Mon Sep 17 00:00:00 2001 From: mgaux-dev Date: Fri, 30 Oct 2026 22:51:50 +0100 Subject: [PATCH 2/9] fix(services): Add From Service matches a layer by its address, not its title --- crates/engine/src/cmd/clipboard.rs | 4 +- crates/engine/src/cmd/layer.rs | 9 +- crates/engine/src/cmd/mod.rs | 1 + crates/engine/src/cmd/service.rs | 28 ++- crates/engine/src/cmd/service_added.rs | 272 +++++++++++++++++++++++++ crates/engine/src/tests.rs | 21 ++ crates/ui-egui/src/dialogs.rs | 194 +++++++++--------- crates/ui-egui/src/lib.rs | 5 + crates/ui-egui/src/panes.rs | 3 +- docs/user-guide/data-formats.md | 4 + 10 files changed, 442 insertions(+), 99 deletions(-) create mode 100644 crates/engine/src/cmd/service_added.rs diff --git a/crates/engine/src/cmd/clipboard.rs b/crates/engine/src/cmd/clipboard.rs index 892a8650d..5d67bfd75 100644 --- a/crates/engine/src/cmd/clipboard.rs +++ b/crates/engine/src/cmd/clipboard.rs @@ -174,5 +174,7 @@ fn copy_path(s: &mut Session, p: &Value) -> CmdResult { let l = s.map().layer(id).ok_or("no such layer")?; let path = l.raster.as_ref().and_then(|r| r.path.clone()).or_else(|| l.dataset.source.clone()); let path = path.ok_or_else(|| format!("{} isn't stored in a file", l.name))?; - Ok(json!({"text": path})) + // A service layer's address is its source: copied without keys, tokens or its password, as + // the details pane shows it and a saved project keeps a web address. + Ok(json!({"text": crate::sources::redact(&path)})) } diff --git a/crates/engine/src/cmd/layer.rs b/crates/engine/src/cmd/layer.rs index 04e3e059a..1f8e53bbc 100644 --- a/crates/engine/src/cmd/layer.rs +++ b/crates/engine/src/cmd/layer.rs @@ -41,7 +41,7 @@ pub fn commands() -> Vec { r#"{"url": "https://…/data.geojson|.csv|.parquet|.fgb|.gpkg|.zip|.kml|.kmz|.gpx|.tif|.jp2 (JPEG 2000: desktop, through GDAL) (the format from the address, else the contents) | a .pmtiles archive", "name"?: text, "layer"?: "one layer of a zip, KML or GPX", "zoom"?: "bool: true zooms to what was added; false keeps the view of a GeoParquet or FlatGeobuf read in place (zoomed to only when the map was empty)", "x"?, "y"?, "wkt"?, "crs"?, "delimiter"?, "paperSpace"?, "stream"?, "as"? (as layer.addData), "whole"?: "bool: download a cloud-optimised GeoTIFF whole (as a raster layer) instead of reading it in place (an address without a file name is read in place when its first bytes are a cloud-optimised GeoTIFF)", "cog"?: "bool: the address is a cloud-optimised GeoTIFF whatever its name says (the app's probe found it so)", "start"?, "end"?, "unit"?, "interval"?, "dates"?: "a GeoTIFF address with date placeholders ({date:YYYY}, {YYYY}{MM}{DD}), one file per date: the dates it spans and steps through (as time.set), turning the time slider on"}"#, ), C::new("layer.addDownload", "Add Downloaded Data", "Home › Add", add_download) - .params(r#"{"id": a download the app finished (Home › Add › Add Data › From URL), "url": its address, "name"?: text, "layer"?: text, "zoom"?: bool, "x"?, "y"?, "wkt"?, "crs"?, "delimiter"?, "paperSpace"?, "stream"?, "as"? (as layer.addData), "service"?: bool (a service's features: their axes and paging are checked), "bounds"?: [west, south, east, north] (with service: the layer's longitude/latitude bounds from its capabilities, which its axes are checked against; without bounds they are left as they come; ignored without service), "coverage"?: bool (a WCS coverage: a GeoTIFF, else the service's error), "geolibreProject"?: bool (internal: downloaded project JSON), "geolibreLayer"?: {"token", "record"} (internal: reserved imported layer and its saved style), "geolibreZarr"?: bool (internal: an imported Zarr slice as GeoTIFF)}"#), + .params(r#"{"id": a download the app finished (Home › Add › Add Data › From URL), "url": its address, "name"?: text, "layer"?: text, "zoom"?: bool, "x"?, "y"?, "wkt"?, "crs"?, "delimiter"?, "paperSpace"?, "stream"?, "as"? (as layer.addData), "service"?: bool (a service's features: their axes and paging are checked, and the address is kept as the layer's source), "bounds"?: [west, south, east, north] (with service: the layer's longitude/latitude bounds from its capabilities, which its axes are checked against; without bounds they are left as they come; ignored without service), "coverage"?: bool (a WCS coverage: a GeoTIFF, else the service's error, and its request kept as the layer's source), "geolibreProject"?: bool (internal: downloaded project JSON), "geolibreLayer"?: {"token", "record"} (internal: reserved imported layer and its saved style), "geolibreZarr"?: bool (internal: an imported Zarr slice as GeoTIFF)}"#), C::new("layer.addEncodedPolyline", "Add Encoded Polyline", "Home › Add", add_encoded_polyline).params( r#"{"polyline": "encoded polyline text" | [texts] (one line each), "precision"?: 5 (Google) | 6 (OSRM, Valhalla), "name"?: text}"#, ), @@ -2905,13 +2905,18 @@ fn add_download(s: &mut Session, p: &Value) -> CmdResult { bytes }; // A WCS coverage (Add From Service): a GeoTIFF, or the service's error said as such. - let name = if super::b(p, "coverage").unwrap_or(false) { + let coverage = super::b(p, "coverage").unwrap_or(false); + let name = if coverage { super::service::coverage_check(ps(p, "name").unwrap_or(&url), &bytes)?; "coverage.tif".to_string() } else { url_file_name(&url, &bytes).map_err(|e| format!("{url}: {e}"))? }; let mut r = super::raster::add_named_bytes(s, p, name, bytes)?; + // A service's features or coverage: known by the address it came from (Add From Service). + if service || coverage { + super::service::mark_service_source(s, &r, &url); + } // `"zoom": true` zooms to everything added, whatever the map held (a link's data); `zoomed` // says whether there was anything to zoom to (a tile archive adds no layer). if b(p, "zoom") == Some(true) { diff --git a/crates/engine/src/cmd/mod.rs b/crates/engine/src/cmd/mod.rs index 9a7038ea6..f7c4efd28 100644 --- a/crates/engine/src/cmd/mod.rs +++ b/crates/engine/src/cmd/mod.rs @@ -42,6 +42,7 @@ pub mod s3; pub mod search; pub mod select; pub mod service; +pub mod service_added; pub mod share; pub mod sharing; pub mod sources; diff --git a/crates/engine/src/cmd/service.rs b/crates/engine/src/cmd/service.rs index 4adb7dc63..f0703887f 100644 --- a/crates/engine/src/cmd/service.rs +++ b/crates/engine/src/cmd/service.rs @@ -228,6 +228,26 @@ pub fn features_answer(s: &mut Session, name: &str, bytes: Vec, bounds: Opti rustgis_data::ogc::fix_axes(&bytes, bounds).unwrap_or(bytes) } +/// The layers an add's result `r` names (`layer`, `layers`) marked as read from the service +/// address `url`, not the file name their download was read under: Add From Service then knows +/// them as added, to remove. +pub(crate) fn mark_service_source(s: &mut Session, r: &Value, url: &str) { + let ids: Vec = r + .get("layers") + .and_then(Value::as_array) + .into_iter() + .flatten() + .chain(r.get("layer")) + .filter_map(Value::as_u64) + .filter_map(|i| u32::try_from(i).ok()) + .collect(); + for id in ids { + if let Some(l) = s.map_mut().layer_mut(id) { + l.dataset.source = Some(url.to_string()); + } + } +} + /// A layer's longitude/latitude bounds as a listing or a parameter gives them (`[west, south, /// east, north]`), for [`features_answer`]: none when absent or null. pub(crate) fn bounds_of(v: Option<&Value>) -> Result, String> { @@ -532,7 +552,9 @@ pub(crate) fn add(s: &mut Session, p: &Value) -> CmdResult { { let bytes = crate::render::rustgis_tiles::cache::download_blocking(&get).map_err(|e| format!("{name}: {e}"))?; coverage_check(&name, &bytes)?; - super::raster::add_named_bytes(s, &json!({"name": name}), format!("{name}.tif"), bytes) + let r = super::raster::add_named_bytes(s, &json!({"name": name}), format!("{name}.tif"), bytes)?; + mark_service_source(s, &r, &get); + Ok(r) } } Offer::Features { url: items } => { @@ -553,9 +575,11 @@ pub(crate) fn add(s: &mut Session, p: &Value) -> CmdResult { s.notify(format!("{name}: {why}, so the whole layer is asked for"), false); } let max = crate::render::rustgis_tiles::cache::MAX_DOWNLOAD; - let bytes = download_features(s, &name, asked.as_deref().unwrap_or(items), max).map_err(|e| slow_whole_layer(e, given))?; + let from = asked.as_deref().unwrap_or(items); + let bytes = download_features(s, &name, from, max).map_err(|e| slow_whole_layer(e, given))?; let bytes = features_answer(s, &name, bytes, layer.bbox); let mut r = super::raster::add_named_bytes(s, &json!({"name": name}), format!("{name}.geojson"), bytes)?; + 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")) { diff --git a/crates/engine/src/cmd/service_added.rs b/crates/engine/src/cmd/service_added.rs new file mode 100644 index 000000000..b286faf97 --- /dev/null +++ b/crates/engine/src/cmd/service_added.rs @@ -0,0 +1,272 @@ +//! Which layers of a service's listing (`service.layers`) are on the map already, and the command +//! that removes each (Add From Service's Remove). A listed layer is known by the address it came +//! from (`service::mark_service_source`, a tile layer's template), never by its name alone, so a +//! file's layer or another service's that shares the title is left alone. + +use rustgis_map::Map; +use serde_json::{Value, json}; + +/// A command and its parameters (`layer.remove {"layer"}`, `map.removeTiles {"index"}`). +pub type Removal = (&'static str, Value); + +/// For each layer of `listed` (a `service.layers` listing's `layers`) whose place is in `rows`, +/// the command that removes it from `map` when it is there; `None` for the others, and for every +/// row not asked about. Rows naming one layer each offer it; a tile layer is claimed by one OGC +/// API row at most (`claim_tilesets`). +pub fn added(map: &Map, listed: &[Value], rows: &[usize]) -> Vec> { + // Each layer's source as it is and without its keys, once: a project saved and opened again + // keeps its addresses without them (the listing's still carry them). + let sources: Vec> = + map.layers.iter().filter_map(|l| l.dataset.source.as_deref().map(|s| (l.id, s, crate::sources::redact(s)))).collect(); + let mut added = vec![None; listed.len()]; + for &i in rows { + if let (Some(l), Some(slot)) = (listed.get(i), added.get_mut(i)) { + *slot = removal(map, &sources, l); + } + } + claim_tilesets(map, listed, rows, &mut added); + added +} + +/// A map layer's id and source, as it is and without its keys. +type Source<'a> = (u32, &'a str, String); + +/// The command that removes the listed layer `l` from `map`, when it is there: matched by its +/// address as it is first, then without its keys (as a saved project keeps it), so a copy of the +/// same service added under another key is only taken when nothing matches exactly. +fn removal(map: &Map, sources: &[Source<'_>], l: &Value) -> Option { + let offer = l.get("offer")?; + let url = offer.get("url").and_then(Value::as_str).filter(|u| !u.is_empty())?; + let keyless = crate::sources::redact(url); + let remove_layer = |matches: &dyn Fn(&str, &str) -> bool| { + let exact = sources.iter().find(|(_, s, _)| matches(url, s)); + exact + .or_else(|| sources.iter().find(|(_, _, redacted)| matches(&keyless, redacted))) + .map(|(id, ..)| ("layer.remove", json!({"layer": id}))) + }; + match offer.get("type").and_then(Value::as_str) { + // Downloaded features keep the address they came from (an area only adds a query to it). + Some("features") => remove_layer(&from_address), + // A coverage keeps its GetCoverage request: the service's address, which its other + // coverages share, with this coverage's id. + Some("coverage") => { + let id = offer.get("describe").and_then(Value::as_str).and_then(coverage_id)?; + remove_layer(&|url, s| from_address(url, s) && coverage_id(s) == Some(id)) + } + // By position: a service's tile layers have no catalog id to tell them apart. Matched by + // the listed template here (a saved project keeps it without its keys); an OGC API + // layer's advertised one, by `claim_tilesets`. + Some("tiles" | "vectorTiles" | "style") => map + .tiles + .iter() + .position(|t| template_matches(url, &t.url)) + .or_else(|| map.tiles.iter().position(|t| template_matches(&keyless, &t.url))) + .map(|i| ("map.removeTiles", json!({"index": i}))), + // A folder or service (its layers are added from their rows) has nothing to remove. + _ => None, + } +} + +/// Gives the OGC API rows asked about (`rows`) that `added` doesn't match yet the tile layer their +/// tileset list advertised. Its template isn't in the listing, so it is found under that list: a +/// tile layer under one listed layer's list alone (counting every layer listed, shown or not), +/// that no row matched by its own template, is that row's. One under several lists is nobody's, +/// so Remove never takes another row's tiles. +fn claim_tilesets(map: &Map, listed: &[Value], rows: &[usize], added: &mut [Option]) { + let mut claimed: Vec = added + .iter() + .flatten() + .filter(|(command, _)| *command == "map.removeTiles") + .filter_map(|(_, p)| p.get("index").and_then(Value::as_u64)) + .collect(); + let lists: Vec<&str> = listed.iter().filter_map(tileset_list).collect(); + // How many listed layers' lists each tile layer is under, once. + let under: Vec = map.tiles.iter().map(|t| lists.iter().filter(|list| under_list(list, &t.url)).count()).collect(); + for &i in rows { + if added.get(i).is_none_or(Option::is_some) { + continue; + } + let Some(list) = listed.get(i).and_then(tileset_list) else { continue }; + let found = map + .tiles + .iter() + .zip(&under) + .zip(0u64..) + .find(|((t, under), n)| **under == 1 && !claimed.contains(n) && under_list(list, &t.url)) + .map(|(_, n)| n); + if let (Some(n), Some(slot)) = (found, added.get_mut(i)) { + *slot = Some(("map.removeTiles", json!({"index": n}))); + claimed.push(n); + } + } +} + +/// An OGC API layer's tileset list address, without its query. +fn tileset_list(l: &Value) -> Option<&str> { + l.get("tilesets").and_then(Value::as_str).and_then(|t| t.split(['?', '#']).next()).filter(|t| !t.is_empty()) +} + +/// Whether the tile template `url` is under the tileset list address `list` (its tilesets' paths). +fn under_list(list: &str, url: &str) -> bool { + url.strip_prefix(list).is_some_and(|rest| rest.starts_with('/')) +} + +/// Whether `source` is the address `url`, or `url` with a query or fragment after it (an area's +/// bounding box): never another address that only starts with the same letters (`…/items2` for +/// `…/items`), nor one further down its path (`…/items/2`, one feature added by hand). +fn from_address(url: &str, source: &str) -> bool { + source.strip_prefix(url).is_some_and(|rest| rest.is_empty() || url.ends_with(['?', '&']) || rest.starts_with(['?', '&', '#'])) +} + +/// A WCS request's coverage (`COVERAGEID`, or a WCS 1.0's `COVERAGE`), as its address gives it. +fn coverage_id(url: &str) -> Option<&str> { + let query = url.split('#').next()?.split_once('?')?.1; + query.split('&').find_map(|kv| { + let (k, v) = kv.split_once('=')?; + (k.eq_ignore_ascii_case("coverageid") || k.eq_ignore_ascii_case("coverage")).then_some(v).filter(|v| !v.is_empty()) + }) +} + +/// Whether `url` is `template` with its `{Time}` placeholders (a time-enabled layer's) filled in. +fn template_matches(template: &str, url: &str) -> bool { + let mut parts = template.split("{Time}"); + let Some(first) = parts.next() else { return false }; + let Some(mut rest) = url.strip_prefix(first) else { return false }; + // Each placeholder takes a date: one piece of the address, never a path or query. + let date = |time: &str| !time.is_empty() && !time.contains(['/', '?', '#']); + let mut parts = parts.peekable(); + while let Some(part) = parts.next() { + if parts.peek().is_none() { + return rest.strip_suffix(part).is_some_and(date); + } + let Some(at) = rest.find(part).filter(|&at| part.is_empty() || rest.get(..at).is_some_and(date)) else { return false }; + rest = rest.get(at + part.len()..).unwrap_or(""); + } + rest.is_empty() +} + +#[cfg(test)] +mod tests { + use super::*; + use rustgis_map::{TileKind, TileLayer}; + + fn tiles(urls: &[&str]) -> Map { + let mut map = Map::new("Map"); + map.tiles = urls.iter().map(|u| TileLayer::new("T", TileKind::Raster, *u)).collect(); + map + } + + #[test] + fn a_template_matches_only_its_own_address() { + let wms = "http://h/wms?layers=a&bbox={bbox-epsg-3857}"; + assert!(template_matches(wms, wms)); + assert!(!template_matches(wms, "http://h/wms?layers=b&bbox={bbox-epsg-3857}")); + assert!(!template_matches("", "http://h/x")); + let timed = "http://h/{Time}/{z}/{x}/{y}.png"; + assert!(template_matches(timed, "http://h/2024-06-01/{z}/{x}/{y}.png")); + assert!(!template_matches(timed, "http://other/2024-06-01/{z}/{x}/{y}.png")); + assert!(!template_matches(timed, "http://h/2024-06-01/{z}/{x}/{y}.jpg")); + // A trailing placeholder: a date, not every address under the prefix. + assert!(template_matches("http://h/tiles?time={Time}", "http://h/tiles?time=2024-06-01")); + assert!(!template_matches("http://h/{Time}", "http://h/other/{z}/{x}/{y}.png")); + assert!(!template_matches("http://h/{Time}", "http://h/")); + // A middle one: one piece of the path, not several. + assert!(!template_matches(timed, "http://h/a/b/{z}/{x}/{y}.png")); + assert!(!template_matches("http://h/{Time}/x/{Time}.png", "http://h/a/b/x/c.png")); + assert!(template_matches("http://h/{Time}/x/{Time}.png", "http://h/2024/x/2025.png")); + } + + #[test] + fn an_address_matches_only_itself_and_what_follows_it() { + let items = "https://h/items"; + assert!(from_address(items, items)); + assert!(from_address(items, "https://h/items?bbox=1,2,3,4")); + assert!(!from_address(items, "https://h/items/2"), "one feature further down the path"); + assert!(from_address(items, "https://h/items#top")); + assert!(!from_address(items, "https://h/items2"), "another service's address sharing the prefix"); + assert!(!from_address(items, "https://h/item")); + assert!(!from_address(items, "wfs.geojson")); + // An address ending in its query's start: what follows is that query. + assert!(from_address("https://h/wcs?", "https://h/wcs?SERVICE=WCS")); + // One ending in `/` is a path, not a prefix of every address under it. + assert!(!from_address("https://h/", "https://h/other")); + assert!(from_address("https://h/", "https://h/?bbox=1,2,3,4")); + } + + #[test] + fn a_coverage_is_told_by_its_id() { + let v2 = "https://h/wcs?SERVICE=WCS&VERSION=2.0.1&REQUEST=GetCoverage&COVERAGEID=dem&FORMAT=image%2Ftiff"; + assert_eq!(coverage_id(v2), Some("dem")); + assert_eq!(coverage_id("https://h/wcs?SERVICE=WCS&VERSION=1.0.0&REQUEST=DescribeCoverage&COVERAGE=soc"), Some("soc")); + assert_eq!(coverage_id("https://h/wcs?SERVICE=WCS&COVERAGEID="), None); + assert_eq!(coverage_id("https://h/wcs"), None); + } + + /// A saved project keeps a tile template without its key: the row still finds it. + #[test] + fn a_keyed_tile_layer_is_known_after_a_reopen() { + let offer = "https://h/wms?layers=a&api_key=K&bbox={bbox-epsg-3857}"; + let saved = crate::sources::redact(offer); + assert!(!saved.contains("api_key"), "{saved}"); + let map = tiles(&["https://elsewhere/{z}/{x}/{y}.png", &saved]); + let listed = [json!({"title": "A", "offer": {"type": "tiles", "url": offer}})]; + assert_eq!(added(&map, &listed, &[0]), [Some(("map.removeTiles", json!({"index": 1})))]); + } + + /// Two copies of one service under different keys: each row takes its own copy first. + #[test] + fn the_exact_address_wins_over_the_keyless_one() { + let mut map = Map::new("Map"); + for (id, key) in [(7, "A"), (8, "B")] { + let mut ds = rustgis_data::Dataset::new("Items", rustgis_data::GeometryType::Point, Vec::new()); + ds.source = Some(format!("https://h/items?f=json&api_key={key}")); + map.layers.push(rustgis_map::Layer::new(id, ds)); + } + let row = |key: &str| json!({"offer": {"type": "features", "url": format!("https://h/items?f=json&api_key={key}")}}); + let listed = [row("B"), row("A"), row("C")]; + let found = added(&map, &listed, &[0, 1, 2]); + assert_eq!(found[0], Some(("layer.remove", json!({"layer": 8})))); + assert_eq!(found[1], Some(("layer.remove", json!({"layer": 7})))); + // A key no copy has: the first copy without its key, as after a reopen. + assert_eq!(found[2], Some(("layer.remove", json!({"layer": 7})))); + } + + /// A layer with the listed title but another source is not the service's: its row keeps Add. + #[test] + fn a_layer_with_the_same_title_but_another_source_is_left_alone() { + let mut map = Map::new("Map"); + // A name that reads as a dated copy of the title (`Roads 2 Lanes`) is another layer too. + let mut ds = rustgis_data::Dataset::new("Roads 2 Lanes", rustgis_data::GeometryType::Point, Vec::new()); + ds.source = Some("C:/data/roads.shp".into()); + map.layers.push(rustgis_map::Layer::new(7, ds)); + let listed = [json!({"title": "Roads", "offer": {"type": "features", "url": "https://h/items"}})]; + assert_eq!(added(&map, &listed, &[0]), [None]); + let map = tiles(&["https://elsewhere/{z}/{x}/{y}.png"]); + let listed = [json!({"title": "T", "offer": {"type": "tiles", "url": "https://h/{z}/{x}/{y}.png"}})]; + assert_eq!(added(&map, &listed, &[0]), [None]); + } + + /// A coverage is known by its own id, not just its service's address. + #[test] + fn a_coverage_of_another_id_is_left_alone() { + let mut map = Map::new("Map"); + let mut ds = rustgis_data::Dataset::new("DEM", rustgis_data::GeometryType::None, Vec::new()); + ds.source = Some("https://h/wcs?SERVICE=WCS&REQUEST=GetCoverage&COVERAGEID=soc".into()); + map.layers.push(rustgis_map::Layer::new(7, ds)); + let row = |id: &str| json!({"title": "DEM", "offer": {"type": "coverage", "url": "https://h/wcs", "describe": format!("https://h/wcs?SERVICE=WCS&REQUEST=DescribeCoverage&COVERAGEID={id}")}}); + assert_eq!(added(&map, &[row("dem")], &[0]), [None]); + assert_eq!(added(&map, &[row("soc")], &[0]), [Some(("layer.remove", json!({"layer": 7})))]); + } + + /// An OGC API row's advertised tiles: claimed under its tileset list alone, counting the rows + /// a filter hides too, so a row never takes another's tiles. + #[test] + fn a_tileset_is_claimed_only_under_one_list() { + let map = tiles(&["https://h/tiles/B/{z}/{x}/{y}.png", "https://h/c/tiles/WebMercatorQuad/{z}/{y}/{x}"]); + let row = |id: &str, list: &str| json!({"offer": {"type": "tiles", "url": format!("https://h/offer/{id}")}, "tilesets": list}); + let listed = [row("a", "https://h/tiles"), row("b", "https://h/tiles"), row("c", "https://h/c/tiles?f=json")]; + // Rows a and b share a list: neither takes its tiles, even with b filtered out. + assert_eq!(added(&map, &listed, &[0]), [None, None, None]); + assert_eq!(added(&map, &listed, &[0, 1, 2]), [None, None, Some(("map.removeTiles", json!({"index": 1})))]); + } +} diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index cfb047fd1..6fa920cc9 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -9066,6 +9066,21 @@ fn a_failed_command_takes_its_request_back() { assert_eq!(left[0]["url"], "https://x.org/kept.geojson"); } +/// A service's features keep the address they came from, and Copy Path gives it without its key. +#[test] +fn a_service_layers_address_is_copied_without_its_key() { + let mut s = Session::new(Project::default()); + let id = s.download_started(); + assert!(s.download_landed(id, br#"{"type": "Point", "coordinates": [1, 2]}"#.to_vec())); + let url = "https://x.org/collections/a/items?f=json&api_key=SECRET"; + let r = s.run("layer.addDownload", &json!({"id": id, "url": url, "name": "Api", "service": true})).unwrap(); + let layer = r["layer"].as_u64().or_else(|| r["layers"][0].as_u64()).unwrap() as u32; + assert_eq!(s.map().layer(layer).and_then(|l| l.dataset.source.as_deref()), Some(url)); + let copied = s.run("layer.copyPath", &json!({"layer": layer})).unwrap(); + let text = copied["text"].as_str().unwrap(); + assert!(text.starts_with("https://x.org/collections/a/items?") && !text.contains("SECRET"), "{text}"); +} + /// The app's downloads land in the project they were started for, as bytes (no base64); one /// whose project was closed meanwhile is dropped. #[test] @@ -10096,6 +10111,9 @@ fn service_features_asked_for_over_an_area() { let r = add(&mut s, "1.1.0", Some(piaui.clone())); assert_eq!(r["features"], 1); assert_eq!(r["area"], piaui); + // The layer keeps the service address it came from, not its download's file name. + let source = s.map().layer_by_name("M0").and_then(|l| l.dataset.source.clone()); + assert!(source.as_deref().is_some_and(|src| src.starts_with(&format!("{url}?"))), "{source:?}"); let r = add(&mut s, "2.0.0", Some(json!("view"))); assert!(r["area"].is_array(), "{r}"); let r = add(&mut s, "2.0.0", None); @@ -10220,6 +10238,9 @@ fn wcs_coverages_and_catalogue_records() { let r = s.run("service.add", &p).unwrap(); let id = r["layer"].as_u64().unwrap() as u32; assert!(s.map().layer(id).is_some_and(|l| l.is_raster() && l.name == "Soil carbon"), "{r}"); + // The raster keeps its GetCoverage request, so Add From Service knows it as added. + let source = s.map().layer(id).and_then(|l| l.dataset.source.clone()); + assert!(source.as_deref().is_some_and(|src| src.contains("REQUEST=GetCoverage") && src.contains("=soc")), "{source:?}"); assert!( s.run("service.coverage", &json!({"url": url, "layer": "soc", "text": caps, "describe": describe, "bbox": [10, 10, 11, 11]})) .is_err() diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index c664c6a73..ca4f038e1 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -3,6 +3,7 @@ use egui::{Align2, vec2}; use rustgis_engine::Session; +use rustgis_engine::cmd::service_added::Removal; use serde_json::{Value, json}; use crate::RustGisApp; @@ -786,55 +787,33 @@ fn go_to(app: &mut RustGisApp, url: &str, kind: &str, way: Vec<(String, &'static /// How many of a service's layers Add From Service lists. const SHOWN_LAYERS: usize = 500; -/// An item of the active map whose `service.layers` offer is already there. -#[derive(Clone, Copy, Debug, PartialEq, Eq)] -enum AddedServiceItem { - /// A downloaded feature layer or coverage. - Layer(u32), - /// A WMS, WMTS, vector tile or style layer, drawn as tiles. - Tile(usize), -} - -/// Whether `name` is `title` as `service.add` names a layer, or a dated one (`"Roads 2024-01-01"`). -fn named_after(name: &str, title: &str) -> bool { - !title.is_empty() - && (name == title - || name - .strip_prefix(title) - .and_then(|rest| rest.strip_prefix(' ')) - .and_then(|date| date.chars().next()) - .is_some_and(|c| c.is_ascii_digit())) +/// The listed layers matching `filter` (words their title or name contains), with their place in +/// the listing. +fn matching_rows<'a>(layers: &'a [Value], filter: &str) -> Vec<(usize, &'a Value)> { + let words: Vec = filter.split_whitespace().map(str::to_lowercase).collect(); + let text = |l: &Value, k: &str| l.get(k).and_then(Value::as_str).unwrap_or("").to_lowercase(); + layers + .iter() + .enumerate() + .filter(|(_, l)| words.iter().all(|w| text(l, "title").contains(w.as_str()) || text(l, "name").contains(w.as_str()))) + .collect() } -/// The item the service listing `l` already added to the active map, if it did: a tile layer by -/// its address (a dated address's `time_url` is the template the listing gives), or a feature or -/// coverage layer, which `service.add` names after the listed layer's title. -fn added_service_item(s: &rustgis_engine::Session, l: &Value) -> Option { - let offer = l.get("offer")?; - let kind = offer.get("type").and_then(Value::as_str)?; - let url = offer.get("url").and_then(Value::as_str).unwrap_or(""); - let title = l.get("title").and_then(Value::as_str).or_else(|| l.get("name").and_then(Value::as_str)).unwrap_or(""); - let map = s.map(); - match kind { - "tiles" | "vectorTiles" | "style" => { - // The address first: another service may have a layer of the same title. - let address = || (!url.is_empty()).then_some(url); - map.tiles - .iter() - .position(|t| address().is_some_and(|u| t.url == u || t.time_url.as_deref() == Some(u))) - .or_else(|| map.tiles.iter().position(|t| named_after(&t.name, title))) - .map(AddedServiceItem::Tile) - } - "features" | "coverage" => { - let address = || (!url.is_empty()).then_some(url); - map.layers - .iter() - .find(|layer| address().is_some_and(|u| layer.dataset.source.as_deref() == Some(u))) - .or_else(|| map.layers.iter().find(|layer| layer.name.eq_ignore_ascii_case(title))) - .map(|layer| AddedServiceItem::Layer(layer.id)) - } - _ => None, +/// Which of the rows shown (the first [`SHOWN_LAYERS`] matching the filter) are on the map, by +/// their place in the listing (`service_added::added`), worked out again only when the session, +/// the listing or the filter changed, not on every frame. +fn refresh_added(app: &mut RustGisApp) { + let revision = app.session.revision; + let Some(conn) = app.ui.service.as_mut() else { return }; + let filter = &app.ui.service_browse.filter; + if conn.added.as_ref().is_some_and(|(r, f, _)| *r == revision && f == filter) { + return; } + let none = Vec::new(); + let layers = conn.listing.as_ref().and_then(|v| v.get("layers")).and_then(Value::as_array).unwrap_or(&none); + let shown: Vec = matching_rows(layers, filter).into_iter().take(SHOWN_LAYERS).map(|(i, _)| i).collect(); + let added = rustgis_engine::cmd::service_added::added(app.session.map(), layers, &shown); + conn.added = Some((revision, filter.clone(), added)); } /// Add From Service: a WMS, WMTS, WFS, WCS, OGC API Features or ArcGIS REST service by its address @@ -916,7 +895,7 @@ fn add_service(app: &mut RustGisApp, ui: &mut egui::Ui) -> bool { } // The service: connecting, an error, or its layers. let mut add: Option = None; - let mut remove: Option<(AddedServiceItem, String)> = None; + let mut remove: Option<(Removal, String)> = None; let mut import = None; // A directory's folder or service to open, or Back to the directory before. let mut open: Option = None; @@ -926,6 +905,7 @@ fn add_service(app: &mut RustGisApp, ui: &mut egui::Ui) -> bool { crate::connectionui::picked(&list, app.dialogs.connection.as_deref(), url).is_none().then(|| (url.to_string(), kind)) }); let mut keep_root = false; + refresh_added(app); if let Some(conn) = &app.ui.service { ui.separator(); if let Some(e) = &conn.error { @@ -1026,12 +1006,8 @@ fn add_service(app: &mut RustGisApp, ui: &mut egui::Ui) -> bool { } }); } - let words: Vec = app.ui.service_browse.filter.split_whitespace().map(str::to_lowercase).collect(); - let text = |l: &Value, k: &str| l.get(k).and_then(Value::as_str).unwrap_or("").to_lowercase(); - let matching: Vec<&Value> = layers - .iter() - .filter(|l| words.iter().all(|w| text(l, "title").contains(w.as_str()) || text(l, "name").contains(w.as_str()))) - .collect(); + let matching = matching_rows(layers, &app.ui.service_browse.filter); + let added = conn.added.as_ref(); if matching.len() > SHOWN_LAYERS { ui.label( egui::RichText::new(crate::i18n::fmt("The first {n} are listed.", &[("n", &SHOWN_LAYERS.to_string())])).small().weak(), @@ -1049,7 +1025,7 @@ fn add_service(app: &mut RustGisApp, ui: &mut egui::Ui) -> bool { .show(ui, |ui| { egui::ScrollArea::vertical().id_salt("add_service_layers_scroll").auto_shrink([false, false]).show(ui, |ui| { let style = ui.style().clone(); - for l in matching.into_iter().take(SHOWN_LAYERS) { + for (i, l) in matching.into_iter().take(SHOWN_LAYERS) { ui.horizontal(|ui| { ui.add_space(2.0); let kind = l.get("offer").and_then(|o| o.get("type")).and_then(Value::as_str); @@ -1061,14 +1037,13 @@ fn add_service(app: &mut RustGisApp, ui: &mut egui::Ui) -> bool { open = l.get("offer").and_then(|o| o.get("url")).and_then(Value::as_str).map(str::to_string); } } else if !(grouped && kind == Some("features")) { - match added_service_item(&app.session, l) { - Some(item) => { - if ui.small_button(tl!("Remove")).clicked() { - remove = Some((item, title.to_string())); - } + let removal = added.and_then(|(_, _, added)| added.get(i)).and_then(Option::as_ref); + let label = if removal.is_some() { tl!("Remove") } else { tl!("Add") }; + if ui.small_button(label).clicked() { + match removal { + Some((command, params)) => remove = Some(((command, params.clone()), title.to_string())), + None => add = Some(l.clone()), } - None if ui.small_button(tl!("Add")).clicked() => add = Some(l.clone()), - None => {} } } let what = match kind { @@ -1143,12 +1118,8 @@ fn add_service(app: &mut RustGisApp, ui: &mut egui::Ui) -> bool { { app.dialogs.error = Some(e); } - if let Some((item, title)) = remove { - let r = match item { - AddedServiceItem::Layer(id) => app.run("layer.remove", json!({"layer": id})), - AddedServiceItem::Tile(index) => app.run("map.removeTiles", json!({"index": index})), - }; - match r { + if let Some(((command, params), title)) = remove { + match app.run(command, params) { Ok(_) => app.dialogs.info = Some(crate::i18n::fmt("Removed {name}", &[("name", &title)])), Err(e) => app.dialogs.error = Some(e), } @@ -2289,32 +2260,6 @@ mod tests { assert_eq!(format_degrees(104.25), "104.25°"); } - /// A downloaded feature layer keeps the listed title as its name, which tells the dialog it - /// is already there (and so offers Remove rather than Add). - #[test] - fn a_service_features_layer_is_known_once_added() { - let mut app = RustGisApp::with_sample(); - let listed = json!({ - "name": "places", - "title": "Service Places", - "offer": {"type": "features", "url": "https://example.com/collections/places/items"}, - }); - assert_eq!(added_service_item(&app.session, &listed), None); - app.run( - "layer.addDataText", - json!({ - "fileName": "places.geojson", - "name": "Service Places", - "text": r#"{"type":"FeatureCollection","features":[{"type":"Feature","properties":{},"geometry":{"type":"Point","coordinates":[0,0]}}]}"#, - }), - ) - .unwrap(); - let id = app.session.map().layer_by_name("Service Places").unwrap().id; - assert_eq!(added_service_item(&app.session, &listed), Some(AddedServiceItem::Layer(id))); - app.run("layer.remove", json!({"layer": id})).unwrap(); - assert_eq!(added_service_item(&app.session, &listed), None); - } - /// Add From Service's row button says Remove for a service layer already on the map, and /// removes it; once it is gone the row says Add again. #[test] @@ -2341,6 +2286,7 @@ mod tests { "offer": {"type": "tiles", "url": url}, }], })), + added: None, error: None, failure: None, }); @@ -2367,4 +2313,66 @@ mod tests { assert!(h.query_by_label("Add").is_some()); assert!(h.query_by_label("Remove").is_none(), "nothing is left to remove"); } + + /// A service's feature layer, downloaded in the background, keeps the listed address as its + /// source even under another name: its row offers Remove, which takes it off the map and + /// offers Add again. + #[test] + fn a_service_feature_layer_is_known_once_added_by_its_address() { + use egui_kittest::kittest::Queryable; + let mut app = RustGisApp::with_sample(); + let url = "https://example.com/collections/places/items"; + let id = app.session.download_started(); + assert!(app.session.download_landed( + id, + br#"{"type":"FeatureCollection","features":[{"type":"Feature","properties":{},"geometry":{"type":"Point","coordinates":[0,0]}}]}"#.to_vec(), + )); + // A name of its own, so only the address it came from tells the dialog it is there. + app.run("layer.addDownload", json!({"id": id, "url": url, "name": "Places copy", "service": true})).unwrap(); + let layer = app.session.map().layer_by_name("Places copy").unwrap().id; + app.ui.service = Some(crate::ServiceConn { + url: "https://example.com/collections".into(), + kind: "wfs", + slot: Default::default(), + text: None, + documents: Vec::new(), + pending: None, + listing: Some(json!({ + "kind": "WFS", + "title": "Places", + "count": 1, + "layers": [{ + "name": "places", + "title": "Service Places", + "bbox": null, + "offer": {"type": "features", "url": url}, + }], + })), + added: None, + error: None, + failure: None, + }); + app.run("ui.dialog", json!({"name": "addService"})).unwrap(); + app.dialogs.mode = "wfs"; + let mut fonts = false; + let mut h = egui_kittest::Harness::builder().with_size(vec2(1200.0, 900.0)).build_ui_state( + move |ui, app: &mut RustGisApp| { + if !fonts { + crate::theme::install_fonts(ui.ctx(), &[], None); + fonts = true; + return; + } + let ctx = ui.ctx().clone(); + super::show(app, &ctx); + }, + app, + ); + h.run_steps(4); + assert!(h.query_by_label("Add").is_none(), "the layer is already on the map, so Add is not offered"); + h.get_by_label("Remove").click(); + h.run_steps(4); + assert!(h.state().session.map().layer(layer).is_none(), "Remove took the feature layer off the map"); + assert!(h.query_by_label("Add").is_some()); + assert!(h.query_by_label("Remove").is_none(), "nothing is left to remove"); + } } diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 551adc90d..1044234e3 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -352,6 +352,9 @@ pub struct ServiceConn { pub pending: Option, /// What `service.layers` made of it. pub listing: Option, + /// For each listed layer, the command that removes it when it is on the map, as of a session + /// revision and list filter (Add From Service's Remove; see `dialogs::refresh_added`). + pub added: Option<(u64, String, Vec>)>, pub error: Option, /// A capabilities request that failed, with what the server said (#509). pub failure: Option, @@ -2575,6 +2578,7 @@ impl RustGisApp { documents: Vec::new(), pending: Some(caps), listing: None, + added: None, error: None, failure: None, }); @@ -3652,6 +3656,7 @@ impl RustGisApp { conn.slot = fetch_detailed_into(next); } conn.listing = Some(v); + conn.added = None; } Err(e) => conn.error = Some(e), } diff --git a/crates/ui-egui/src/panes.rs b/crates/ui-egui/src/panes.rs index 668eca59c..e4b4bb80a 100644 --- a/crates/ui-egui/src/panes.rs +++ b/crates/ui-egui/src/panes.rs @@ -5542,7 +5542,8 @@ fn details(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens) { ("Fields", l.dataset.fields.len().to_string()), ("Selected", l.selection.len().to_string()), ("Symbology", tl!(l.renderer.kind_label()).to_string()), - ("Source", l.dataset.source.clone().unwrap_or_else(|| tl!("In project").into())), + // A web or database address without its keys, tokens or password. + ("Source", l.dataset.source.as_deref().map_or_else(|| tl!("In project").into(), rustgis_engine::sources::redact)), ("Source coordinate system", l.dataset.crs.map(|c| c.label()).unwrap_or_else(|| "EPSG:4326".into())), ("West", format!("{:.4}°", e.min_x)), ("East", format!("{:.4}°", e.max_x)), diff --git a/docs/user-guide/data-formats.md b/docs/user-guide/data-formats.md index a8f510d3c..d35c83275 100644 --- a/docs/user-guide/data-formats.md +++ b/docs/user-guide/data-formats.md @@ -139,6 +139,10 @@ its capabilities. For OGC API feature collections, enter a group name (initially and choose **Import All**. The group expands into ordinary point, line and polygon layers with the same legends, styling and attribute tables as file imports. Other offerings have individual **Add** buttons (`service.layers` and `service.add` from scripts, the command line and MCP). +A layer already on the active map shows **Remove** instead: the row removes it with +`layer.remove` (`map.removeTiles` for a tile layer), so the removal is undone like any other edit. +A layer is known by the address it was added from (a tile layer's template), not its title alone, +so one of yours or of another service with the same name is left alone. An OGC API landing page works directly: RustGIS follows its advertised JSON collections link, so you don't need to append `/collections`. If the landing page also offers map tiles, they remain listed alongside the collections. A failed collections request From 75060468307e47fc77c878e7e7a6669002227020 Mon Sep 17 00:00:00 2001 From: mgaux-dev Date: Fri, 30 Oct 2026 23:07:33 +0100 Subject: [PATCH 3/9] fix(ui): Add From Service caches its rows and their removals --- crates/ui-egui/src/dialogs.rs | 98 +++++++++++++++++++++++++++++---- crates/ui-egui/src/lib.rs | 21 ++++++- docs/user-guide/data-formats.md | 6 +- 3 files changed, 110 insertions(+), 15 deletions(-) diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index ca4f038e1..397195ee3 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -799,21 +799,24 @@ fn matching_rows<'a>(layers: &'a [Value], filter: &str) -> Vec<(usize, &'a Value .collect() } -/// Which of the rows shown (the first [`SHOWN_LAYERS`] matching the filter) are on the map, by -/// their place in the listing (`service_added::added`), worked out again only when the session, -/// the listing or the filter changed, not on every frame. +/// The rows Add From Service shows (the first [`SHOWN_LAYERS`] matching the filter), with the +/// command that removes each when it is on the map (`service_added::added`), worked out again only +/// when the session, the listing or the filter changed, not on every frame. fn refresh_added(app: &mut RustGisApp) { let revision = app.session.revision; let Some(conn) = app.ui.service.as_mut() else { return }; let filter = &app.ui.service_browse.filter; - if conn.added.as_ref().is_some_and(|(r, f, _)| *r == revision && f == filter) { + if conn.added.as_ref().is_some_and(|a| a.revision == revision && a.filter == *filter) { return; } let none = Vec::new(); let layers = conn.listing.as_ref().and_then(|v| v.get("layers")).and_then(Value::as_array).unwrap_or(&none); - let shown: Vec = matching_rows(layers, filter).into_iter().take(SHOWN_LAYERS).map(|(i, _)| i).collect(); - let added = rustgis_engine::cmd::service_added::added(app.session.map(), layers, &shown); - conn.added = Some((revision, filter.clone(), added)); + let matching = matching_rows(layers, filter); + let matched = matching.len(); + let shown: Vec = matching.into_iter().take(SHOWN_LAYERS).map(|(i, _)| i).collect(); + let removals = rustgis_engine::cmd::service_added::added(app.session.map(), layers, &shown); + let rows = shown.into_iter().map(|i| (i, removals.get(i).cloned().flatten())).collect(); + conn.added = Some(crate::ServiceAdded { revision, filter: filter.clone(), matched, rows }); } /// Add From Service: a WMS, WMTS, WFS, WCS, OGC API Features or ArcGIS REST service by its address @@ -1006,9 +1009,9 @@ fn add_service(app: &mut RustGisApp, ui: &mut egui::Ui) -> bool { } }); } - let matching = matching_rows(layers, &app.ui.service_browse.filter); let added = conn.added.as_ref(); - if matching.len() > SHOWN_LAYERS { + let rows: &[(usize, Option)] = added.map_or(&[], |a| a.rows.as_slice()); + if added.is_some_and(|a| a.matched > SHOWN_LAYERS) { ui.label( egui::RichText::new(crate::i18n::fmt("The first {n} are listed.", &[("n", &SHOWN_LAYERS.to_string())])).small().weak(), ); @@ -1025,7 +1028,8 @@ fn add_service(app: &mut RustGisApp, ui: &mut egui::Ui) -> bool { .show(ui, |ui| { egui::ScrollArea::vertical().id_salt("add_service_layers_scroll").auto_shrink([false, false]).show(ui, |ui| { let style = ui.style().clone(); - for (i, l) in matching.into_iter().take(SHOWN_LAYERS) { + for (i, removal) in rows { + let Some(l) = layers.get(*i) else { continue }; ui.horizontal(|ui| { ui.add_space(2.0); let kind = l.get("offer").and_then(|o| o.get("type")).and_then(Value::as_str); @@ -1037,7 +1041,7 @@ fn add_service(app: &mut RustGisApp, ui: &mut egui::Ui) -> bool { open = l.get("offer").and_then(|o| o.get("url")).and_then(Value::as_str).map(str::to_string); } } else if !(grouped && kind == Some("features")) { - let removal = added.and_then(|(_, _, added)| added.get(i)).and_then(Option::as_ref); + let removal = removal.as_ref(); let label = if removal.is_some() { tl!("Remove") } else { tl!("Add") }; if ui.small_button(label).clicked() { match removal { @@ -2375,4 +2379,76 @@ mod tests { assert!(h.query_by_label("Add").is_some()); assert!(h.query_by_label("Remove").is_none(), "nothing is left to remove"); } + + /// A coverage row is known by its GetCoverage request: the service's address and its own id. + /// Its Remove takes the layer off the map and offers Add again. + #[test] + fn a_service_coverage_is_known_once_added_by_its_request() { + use egui_kittest::kittest::Queryable; + let mut app = RustGisApp::with_sample(); + app.run( + "layer.addDataText", + json!({ + "fileName": "soil.geojson", + "name": "Soil carbon copy", + "text": r#"{"type":"FeatureCollection","features":[{"type":"Feature","properties":{},"geometry":{"type":"Point","coordinates":[0,0]}}]}"#, + }), + ) + .unwrap(); + let layer = app.session.map().layer_by_name("Soil carbon copy").unwrap().id; + // What a coverage download keeps (a feature layer stands in for the raster here: the + // dialog reads the layer's source and the GetCoverage request, not its kind). + let wcs = "https://example.com/wcs"; + let get = "https://example.com/wcs?SERVICE=WCS&VERSION=2.0.1&REQUEST=GetCoverage&COVERAGEID=soc&FORMAT=image%2Ftiff"; + app.session.map_mut().layer_mut(layer).unwrap().dataset.source = Some(get.into()); + app.ui.service = Some(crate::ServiceConn { + url: wcs.into(), + kind: "wcs", + slot: Default::default(), + text: None, + documents: Vec::new(), + pending: None, + listing: Some(json!({ + "kind": "WCS", + "title": "Soils", + "count": 1, + "layers": [{ + "name": "soc", + "title": "Soil carbon", + "bbox": null, + "offer": { + "type": "coverage", + "url": wcs, + "describe": "https://example.com/wcs?SERVICE=WCS&VERSION=2.0.1&REQUEST=DescribeCoverage&COVERAGEID=soc", + "version": "2.0.1", + }, + }], + })), + added: None, + error: None, + failure: None, + }); + app.run("ui.dialog", json!({"name": "addService"})).unwrap(); + app.dialogs.mode = "wcs"; + let mut fonts = false; + let mut h = egui_kittest::Harness::builder().with_size(vec2(1200.0, 900.0)).build_ui_state( + move |ui, app: &mut RustGisApp| { + if !fonts { + crate::theme::install_fonts(ui.ctx(), &[], None); + fonts = true; + return; + } + let ctx = ui.ctx().clone(); + super::show(app, &ctx); + }, + app, + ); + h.run_steps(4); + assert!(h.query_by_label("Add").is_none(), "the coverage is already on the map, so Add is not offered"); + h.get_by_label("Remove").click(); + h.run_steps(4); + assert!(h.state().session.map().layer(layer).is_none(), "Remove took the coverage off the map"); + assert!(h.query_by_label("Add").is_some()); + assert!(h.query_by_label("Remove").is_none(), "nothing is left to remove"); + } } diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 1044234e3..be8cc477a 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -92,6 +92,7 @@ mod widgets; mod workspacepanes; mod zarrui; +use rustgis_engine::cmd::service_added::Removal; use rustgis_engine::render::rustgis_tiles::failure::Failure; use rustgis_engine::{Project, Session}; use serde_json::{Value, json}; @@ -352,14 +353,28 @@ pub struct ServiceConn { pub pending: Option, /// What `service.layers` made of it. pub listing: Option, - /// For each listed layer, the command that removes it when it is on the map, as of a session - /// revision and list filter (Add From Service's Remove; see `dialogs::refresh_added`). - pub added: Option<(u64, String, Vec>)>, + /// The listing as Add From Service shows it, worked out when the session revision or the + /// filter changed (see `dialogs::refresh_added`). + pub added: Option, pub error: Option, /// A capabilities request that failed, with what the server said (#509). pub failure: Option, } +/// Add From Service's rows: the layers of a listing that match the filter, and for each the +/// command that removes it when it is on the map. +pub struct ServiceAdded { + /// The session revision this was worked out for. + pub revision: u64, + /// The listing filter this was worked out with. + pub filter: String, + /// How many layers match the filter (the first `dialogs::SHOWN_LAYERS` are shown). + pub matched: usize, + /// Each shown row: its place in the listing, and the command that removes it when it is on + /// the map (`None`: the row says Add). + pub rows: Vec<(usize, Option)>, +} + /// How Add From Service's list is browsed. #[derive(Default)] pub struct ServiceBrowse { diff --git a/docs/user-guide/data-formats.md b/docs/user-guide/data-formats.md index d35c83275..19945aea5 100644 --- a/docs/user-guide/data-formats.md +++ b/docs/user-guide/data-formats.md @@ -142,7 +142,11 @@ the same legends, styling and attribute tables as file imports. Other offerings A layer already on the active map shows **Remove** instead: the row removes it with `layer.remove` (`map.removeTiles` for a tile layer), so the removal is undone like any other edit. A layer is known by the address it was added from (a tile layer's template), not its title alone, -so one of yours or of another service with the same name is left alone. +so one of yours or of another service with the same name is left alone. A layer saved by an older +version, which kept the file name it was downloaded under rather than the service's address, +shows **Add**; RustGIS won't guess by name. A service's downloaded layers are grouped under the +service's host in the Layers pane's **Data Source** view, and **Copy Path** gives that address +without its keys or tokens. An OGC API landing page works directly: RustGIS follows its advertised JSON collections link, so you don't need to append `/collections`. If the landing page also offers map tiles, they remain listed alongside the collections. A failed collections request From 73e208791367b5007d8810241f7aa4ff2a1b396f Mon Sep 17 00:00:00 2001 From: mgaux-dev Date: Fri, 30 Oct 2026 23:31:31 +0100 Subject: [PATCH 4/9] fix(services): Add From Service marks imported collections and narrows tileset claims --- crates/engine/src/cmd/service_added.rs | 41 ++++++++++++-- crates/ui-egui/src/dialogs.rs | 76 ++++++++++++++++++++++++-- docs/user-guide/data-formats.md | 6 +- 3 files changed, 112 insertions(+), 11 deletions(-) diff --git a/crates/engine/src/cmd/service_added.rs b/crates/engine/src/cmd/service_added.rs index b286faf97..44d9c6c41 100644 --- a/crates/engine/src/cmd/service_added.rs +++ b/crates/engine/src/cmd/service_added.rs @@ -70,8 +70,10 @@ fn removal(map: &Map, sources: &[Source<'_>], l: &Value) -> Option { /// Gives the OGC API rows asked about (`rows`) that `added` doesn't match yet the tile layer their /// tileset list advertised. Its template isn't in the listing, so it is found under that list: a /// tile layer under one listed layer's list alone (counting every layer listed, shown or not), -/// that no row matched by its own template, is that row's. One under several lists is nobody's, -/// so Remove never takes another row's tiles. +/// under the listed layer's own template's start (its tile matrix set), and that no row matched by +/// its own template. One under several lists is nobody's, and another tileset's under the same +/// list (a layer added by hand from the same collection) isn't taken, so Remove never removes +/// another layer. fn claim_tilesets(map: &Map, listed: &[Value], rows: &[usize], added: &mut [Option]) { let mut claimed: Vec = added .iter() @@ -87,12 +89,19 @@ fn claim_tilesets(map: &Map, listed: &[Value], rows: &[usize], added: &mut [Opti continue; } let Some(list) = listed.get(i).and_then(tileset_list) else { continue }; + // The listed template's start (its address up to its tile matrix set), which the tileset + // advertises its tiles under. + let prefix = + listed.get(i).and_then(|l| l.get("offer")).and_then(|o| o.get("url")).and_then(Value::as_str).map(tile_start).unwrap_or(""); + if prefix.is_empty() { + continue; + } let found = map .tiles .iter() .zip(&under) .zip(0u64..) - .find(|((t, under), n)| **under == 1 && !claimed.contains(n) && under_list(list, &t.url)) + .find(|((t, under), n)| **under == 1 && !claimed.contains(n) && under_list(list, &t.url) && t.url.starts_with(prefix)) .map(|(_, n)| n); if let (Some(n), Some(slot)) = (found, added.get_mut(i)) { *slot = Some(("map.removeTiles", json!({"index": n}))); @@ -101,6 +110,12 @@ fn claim_tilesets(map: &Map, listed: &[Value], rows: &[usize], added: &mut [Opti } } +/// The address a tile template's tiles share: the template up to its first placeholder (`{z}`, +/// `{Time}`), which names the tile matrix set. +fn tile_start(template: &str) -> &str { + template.split_once('{').map_or(template, |(start, _)| start) +} + /// An OGC API layer's tileset list address, without its query. fn tileset_list(l: &Value) -> Option<&str> { l.get("tilesets").and_then(Value::as_str).and_then(|t| t.split(['?', '#']).next()).filter(|t| !t.is_empty()) @@ -263,10 +278,26 @@ mod tests { #[test] fn a_tileset_is_claimed_only_under_one_list() { let map = tiles(&["https://h/tiles/B/{z}/{x}/{y}.png", "https://h/c/tiles/WebMercatorQuad/{z}/{y}/{x}"]); - let row = |id: &str, list: &str| json!({"offer": {"type": "tiles", "url": format!("https://h/offer/{id}")}, "tilesets": list}); - let listed = [row("a", "https://h/tiles"), row("b", "https://h/tiles"), row("c", "https://h/c/tiles?f=json")]; + let row = |start: &str, list: &str| json!({"offer": {"type": "tiles", "url": format!("{start}/WebMercatorQuad/{{z}}/{{y}}/{{x}}")}, "tilesets": list}); + let listed = [ + row("https://h/tiles", "https://h/tiles"), + row("https://h/tiles", "https://h/tiles"), + row("https://h/c/tiles", "https://h/c/tiles?f=json"), + ]; // Rows a and b share a list: neither takes its tiles, even with b filtered out. assert_eq!(added(&map, &listed, &[0]), [None, None, None]); assert_eq!(added(&map, &listed, &[0, 1, 2]), [None, None, Some(("map.removeTiles", json!({"index": 1})))]); } + + /// A tile layer under the listed layer's tileset list but in another tile matrix set (a layer + /// added by hand from the same collection) is left alone. + #[test] + fn a_hand_added_tile_in_another_tileset_is_left_alone() { + let map = tiles(&["https://h/c/tiles/WorldCRS84Quad/{z}/{x}/{y}.png"]); + let row = json!({ + "offer": {"type": "tiles", "url": "https://h/c/tiles/WebMercatorQuad/{z}/{y}/{x}"}, + "tilesets": "https://h/c/tiles?f=json", + }); + assert_eq!(added(&map, &[row], &[0]), [None]); + } } diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index 397195ee3..e02f17d46 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -997,8 +997,8 @@ fn add_service(app: &mut RustGisApp, ui: &mut egui::Ui) -> bool { } if grouped && layers.iter().any(|l| l["offer"]["type"] == "features") { ui.horizontal(|ui| { - ui.label(tl!("Name")); - ui.text_edit_singleline(&mut app.ui.service_group_name); + let input_label = ui.label(tl!("Name")); + ui.add(egui::TextEdit::singleline(&mut app.ui.service_group_name)).labelled_by(input_label.id); let ready = conn.pending.is_none() && conn.error.is_none() && conn.failure.is_none() && app.ui.service_import.is_none(); if ui.add_enabled(ready, egui::Button::new(tl!("Import All"))).clicked() { let mut p = json!({"url":conn.url,"name":app.ui.service_group_name,"text":conn.text,"documents":conn.documents}); @@ -1036,12 +1036,18 @@ fn add_service(app: &mut RustGisApp, ui: &mut egui::Ui) -> bool { let name = l.get("name").and_then(Value::as_str).unwrap_or(""); let title = l.get("title").and_then(Value::as_str).unwrap_or(name); + let removal = removal.as_ref(); if kind == Some("service") { if ui.small_button(tl!("Open")).clicked() { open = l.get("offer").and_then(|o| o.get("url")).and_then(Value::as_str).map(str::to_string); } - } else if !(grouped && kind == Some("features")) { - let removal = removal.as_ref(); + } else if grouped && kind == Some("features") { + // Import All adds these as one group: the row says whether its + // collection is in it; the child is removed in the Layers pane. + if removal.is_some() { + ui.label(egui::RichText::new(tl!("On the map")).small().weak()); + } + } else { let label = if removal.is_some() { tl!("Remove") } else { tl!("Add") }; if ui.small_button(label).clicked() { match removal { @@ -2451,4 +2457,66 @@ mod tests { assert!(h.query_by_label("Add").is_some()); assert!(h.query_by_label("Remove").is_none(), "nothing is left to remove"); } + + /// A grouped OGC API collection imported with Import All says it is on the map; its row has no + /// Add or Remove (the group's child is removed in the Layers pane). + #[test] + fn an_imported_collection_says_it_is_on_the_map() { + use egui_kittest::kittest::Queryable; + let mut app = RustGisApp::with_sample(); + let url = "https://example.com/collections/places/items"; + app.run( + "layer.addDataText", + json!({ + "fileName": "places.geojson", + "name": "Places copy", + "text": r#"{"type":"FeatureCollection","features":[{"type":"Feature","properties":{},"geometry":{"type":"Point","coordinates":[0,0]}}]}"#, + }), + ) + .unwrap(); + let layer = app.session.map().layer_by_name("Places copy").unwrap().id; + // What `service.import` gives each collection's child. + app.session.map_mut().layer_mut(layer).unwrap().dataset.source = Some(url.into()); + app.ui.service = Some(crate::ServiceConn { + url: "https://example.com/collections".into(), + kind: "ogcapi", + slot: Default::default(), + text: None, + documents: Vec::new(), + pending: None, + listing: Some(json!({ + "kind": "OGC API Features", + "title": "Places", + "count": 1, + "layers": [{ + "name": "places", + "title": "Service Places", + "bbox": null, + "offer": {"type": "features", "url": url}, + }], + })), + added: None, + error: None, + failure: None, + }); + app.run("ui.dialog", json!({"name": "addService"})).unwrap(); + app.dialogs.mode = "ogcapi"; + let mut fonts = false; + let mut h = egui_kittest::Harness::builder().with_size(vec2(1200.0, 900.0)).build_ui_state( + move |ui, app: &mut RustGisApp| { + if !fonts { + crate::theme::install_fonts(ui.ctx(), &[], None); + fonts = true; + return; + } + let ctx = ui.ctx().clone(); + super::show(app, &ctx); + }, + app, + ); + h.run_steps(4); + assert!(h.query_by_label("On the map").is_some(), "the collection is in the imported group"); + assert!(h.query_by_label("Add").is_none(), "grouped features have no individual Add"); + assert!(h.query_by_label("Remove").is_none(), "grouped features are removed in the Layers pane"); + } } diff --git a/docs/user-guide/data-formats.md b/docs/user-guide/data-formats.md index 19945aea5..b6cd27518 100644 --- a/docs/user-guide/data-formats.md +++ b/docs/user-guide/data-formats.md @@ -137,8 +137,10 @@ FlatGeobuf whose header hasn't been fetched yet, use the From URL… dialog or a **Home › Add › Add Data › From Service…** connects to a service by its address, lists what it offers from its capabilities. For OGC API feature collections, enter a group name (initially **My OGC Layer**) and choose **Import All**. The group expands into ordinary point, line and polygon layers with -the same legends, styling and attribute tables as file imports. Other offerings have individual -**Add** buttons (`service.layers` and `service.add` from scripts, the command line and MCP). +the same legends, styling and attribute tables as file imports. A collection already imported +shows **On the map** in the listing; its layer is removed or refreshed from the group in the +Layers pane. Other offerings have individual **Add** buttons (`service.layers` and `service.add` +from scripts, the command line and MCP). A layer already on the active map shows **Remove** instead: the row removes it with `layer.remove` (`map.removeTiles` for a tile layer), so the removal is undone like any other edit. A layer is known by the address it was added from (a tile layer's template), not its title alone, From 46b874f37b858d6172fe002a505298a67600ca78 Mon Sep 17 00:00:00 2001 From: mgaux-dev Date: Fri, 30 Oct 2026 23:49:27 +0100 Subject: [PATCH 5/9] fix(services): Add From Service tells a collection's tile matrix sets apart --- crates/engine/src/cmd/service_added.rs | 82 ++++++++++++++++++------- crates/ui-egui/src/dialogs.rs | 83 +++++++------------------- 2 files changed, 79 insertions(+), 86 deletions(-) diff --git a/crates/engine/src/cmd/service_added.rs b/crates/engine/src/cmd/service_added.rs index 44d9c6c41..2a6361cde 100644 --- a/crates/engine/src/cmd/service_added.rs +++ b/crates/engine/src/cmd/service_added.rs @@ -70,10 +70,10 @@ fn removal(map: &Map, sources: &[Source<'_>], l: &Value) -> Option { /// Gives the OGC API rows asked about (`rows`) that `added` doesn't match yet the tile layer their /// tileset list advertised. Its template isn't in the listing, so it is found under that list: a /// tile layer under one listed layer's list alone (counting every layer listed, shown or not), -/// under the listed layer's own template's start (its tile matrix set), and that no row matched by -/// its own template. One under several lists is nobody's, and another tileset's under the same -/// list (a layer added by hand from the same collection) isn't taken, so Remove never removes -/// another layer. +/// starting with the listed layer's own template's start (its tile matrix set), and that no row +/// matched by its own template. Two rows of one collection may share a list: their tile matrix +/// sets tell their tiles apart. A tile layer under no listed layer's list, or another tileset's +/// under the same list (a layer added by hand), is nobody's, so Remove never removes another layer. fn claim_tilesets(map: &Map, listed: &[Value], rows: &[usize], added: &mut [Option]) { let mut claimed: Vec = added .iter() @@ -81,27 +81,44 @@ fn claim_tilesets(map: &Map, listed: &[Value], rows: &[usize], added: &mut [Opti .filter(|(command, _)| *command == "map.removeTiles") .filter_map(|(_, p)| p.get("index").and_then(Value::as_u64)) .collect(); - let lists: Vec<&str> = listed.iter().filter_map(tileset_list).collect(); - // How many listed layers' lists each tile layer is under, once. - let under: Vec = map.tiles.iter().map(|t| lists.iter().filter(|list| under_list(list, &t.url)).count()).collect(); + // Each listed layer's tileset list and the start of its own template (its tile matrix set). + let starts: Vec> = listed + .iter() + .map(|l| { + let list = tileset_list(l)?; + let start = l.get("offer")?.get("url")?.as_str().map(tile_start)?; + (!start.is_empty()).then_some((list, start)) + }) + .collect(); + // How many listed layers' tile matrix sets each tile layer is under, once: a row counts only + // for tiles that start with its own template's start, so the tile matrix sets of one + // collection sharing a tileset list are told apart (and the same start twice counts once). + let under: Vec = map + .tiles + .iter() + .map(|t| { + let mut starts: Vec<&str> = starts + .iter() + .flatten() + .filter(|(list, start)| under_list(list, &t.url) && t.url.starts_with(*start)) + .map(|(_, start)| *start) + .collect(); + starts.sort_unstable(); + starts.dedup(); + starts.len() + }) + .collect(); for &i in rows { if added.get(i).is_none_or(Option::is_some) { continue; } - let Some(list) = listed.get(i).and_then(tileset_list) else { continue }; - // The listed template's start (its address up to its tile matrix set), which the tileset - // advertises its tiles under. - let prefix = - listed.get(i).and_then(|l| l.get("offer")).and_then(|o| o.get("url")).and_then(Value::as_str).map(tile_start).unwrap_or(""); - if prefix.is_empty() { - continue; - } + let Some((list, start)) = starts.get(i).copied().flatten() else { continue }; let found = map .tiles .iter() .zip(&under) .zip(0u64..) - .find(|((t, under), n)| **under == 1 && !claimed.contains(n) && under_list(list, &t.url) && t.url.starts_with(prefix)) + .find(|((t, under), n)| **under == 1 && !claimed.contains(n) && under_list(list, &t.url) && t.url.starts_with(start)) .map(|(_, n)| n); if let (Some(n), Some(slot)) = (found, added.get_mut(i)) { *slot = Some(("map.removeTiles", json!({"index": n}))); @@ -273,20 +290,39 @@ mod tests { assert_eq!(added(&map, &[row("soc")], &[0]), [Some(("layer.remove", json!({"layer": 7})))]); } - /// An OGC API row's advertised tiles: claimed under its tileset list alone, counting the rows - /// a filter hides too, so a row never takes another's tiles. + /// An OGC API row's advertised tiles: its own row claims them, and one tile layer is claimed + /// by one row only, counting the rows a filter hides too. #[test] - fn a_tileset_is_claimed_only_under_one_list() { - let map = tiles(&["https://h/tiles/B/{z}/{x}/{y}.png", "https://h/c/tiles/WebMercatorQuad/{z}/{y}/{x}"]); + fn a_tileset_is_claimed_by_one_row_of_a_collection() { + let map = tiles(&["https://h/tiles/WebMercatorQuad/{z}/{y}/{x}.png", "https://h/c/tiles/WebMercatorQuad/{z}/{y}/{x}.png"]); let row = |start: &str, list: &str| json!({"offer": {"type": "tiles", "url": format!("{start}/WebMercatorQuad/{{z}}/{{y}}/{{x}}")}, "tilesets": list}); let listed = [ row("https://h/tiles", "https://h/tiles"), row("https://h/tiles", "https://h/tiles"), row("https://h/c/tiles", "https://h/c/tiles?f=json"), ]; - // Rows a and b share a list: neither takes its tiles, even with b filtered out. - assert_eq!(added(&map, &listed, &[0]), [None, None, None]); - assert_eq!(added(&map, &listed, &[0, 1, 2]), [None, None, Some(("map.removeTiles", json!({"index": 1})))]); + let remove = |i: u64| Some(("map.removeTiles", json!({"index": i}))); + // Rows a and b are one collection: a claims its tiles, and b doesn't take them too. It + // stays a's when only b is shown: the rows a filter hides count. + assert_eq!(added(&map, &listed, &[0]), [remove(0), None, None]); + assert_eq!(added(&map, &listed, &[1]), [None, remove(0), None]); + assert_eq!(added(&map, &listed, &[0, 1, 2]), [remove(0), None, remove(1)]); + } + + /// Two tile matrix sets of one collection may share a tileset list: each row claims its own + /// tiles, and a hand-added tile of another tile set under that list is left alone. + #[test] + fn tile_matrix_sets_sharing_a_list_are_told_apart() { + let map = tiles(&["https://h/c/tiles/WorldCRS84Quad/{z}/{x}/{y}.png", "https://h/c/tiles/WebMercatorQuad/{z}/{y}/{x}.png"]); + let row = |start: &str| json!({"offer": {"type": "tiles", "url": format!("{start}/{{z}}/{{y}}/{{x}}")}, "tilesets": "https://h/c/tiles?f=json"}); + let listed = [row("https://h/c/tiles/WebMercatorQuad"), row("https://h/c/tiles/WorldCRS84Quad")]; + // Each row takes the tile under its own template's start. + assert_eq!( + added(&map, &listed, &[0, 1]), + [Some(("map.removeTiles", json!({"index": 1}))), Some(("map.removeTiles", json!({"index": 0})))] + ); + // The WebMercatorQuad row alone doesn't take the WorldCRS84Quad tile. + assert_eq!(added(&map, &listed, &[0]), [Some(("map.removeTiles", json!({"index": 1}))), None]); } /// A tile layer under the listed layer's tileset list but in another tile matrix set (a layer diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index e02f17d46..a74ef32c7 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -2146,17 +2146,10 @@ fn crs_details(ui: &mut egui::Ui, chosen: &str, current: &str, t: &Tokens) { mod tests { use super::*; - /// The Coordinate System dialog re-centres a preset (opengeos/RustGIS#517): the central - /// meridian, set as assistive technology does, goes into the code that Apply uses. - #[test] - fn the_coordinate_system_dialog_sets_a_central_meridian() { - use egui::accesskit::{Action, ActionData, ActionRequest, Role}; - use egui_kittest::kittest::{NodeT, Queryable}; - let mut app = RustGisApp::with_sample(); - app.run("map.projection", json!({"type": "robinson"})).unwrap(); - app.run("ui.dialog", json!({"name": "coordinateSystem"})).unwrap(); + /// The app in a harness, its fonts installed on the first pass (the dialog draws on the next). + fn app_harness(app: RustGisApp) -> egui_kittest::Harness<'static, RustGisApp> { let mut fonts = false; - let mut h = egui_kittest::Harness::builder().with_size(vec2(1200.0, 900.0)).build_ui_state( + egui_kittest::Harness::builder().with_size(vec2(1200.0, 900.0)).build_ui_state( move |ui, app: &mut RustGisApp| { if !fonts { crate::theme::install_fonts(ui.ctx(), &[], None); @@ -2167,7 +2160,19 @@ mod tests { super::show(app, &ctx); }, app, - ); + ) + } + + /// The Coordinate System dialog re-centres a preset (opengeos/RustGIS#517): the central + /// meridian, set as assistive technology does, goes into the code that Apply uses. + #[test] + fn the_coordinate_system_dialog_sets_a_central_meridian() { + use egui::accesskit::{Action, ActionData, ActionRequest, Role}; + use egui_kittest::kittest::{NodeT, Queryable}; + let mut app = RustGisApp::with_sample(); + app.run("map.projection", json!({"type": "robinson"})).unwrap(); + app.run("ui.dialog", json!({"name": "coordinateSystem"})).unwrap(); + let mut h = app_harness(app); h.run_steps(3); // A world preset takes only its meridian. assert!(h.query_by_role_and_label(Role::SpinButton, "First standard parallel").is_none()); @@ -2302,19 +2307,7 @@ mod tests { }); app.run("ui.dialog", json!({"name": "addService"})).unwrap(); app.dialogs.mode = "wms"; - let mut fonts = false; - let mut h = egui_kittest::Harness::builder().with_size(vec2(1200.0, 900.0)).build_ui_state( - move |ui, app: &mut RustGisApp| { - if !fonts { - crate::theme::install_fonts(ui.ctx(), &[], None); - fonts = true; - return; - } - let ctx = ui.ctx().clone(); - super::show(app, &ctx); - }, - app, - ); + let mut h = app_harness(app); h.run_steps(4); assert!(h.query_by_label("Add").is_none(), "the layer is already on the map, so Add is not offered"); h.get_by_label("Remove").click(); @@ -2364,19 +2357,7 @@ mod tests { }); app.run("ui.dialog", json!({"name": "addService"})).unwrap(); app.dialogs.mode = "wfs"; - let mut fonts = false; - let mut h = egui_kittest::Harness::builder().with_size(vec2(1200.0, 900.0)).build_ui_state( - move |ui, app: &mut RustGisApp| { - if !fonts { - crate::theme::install_fonts(ui.ctx(), &[], None); - fonts = true; - return; - } - let ctx = ui.ctx().clone(); - super::show(app, &ctx); - }, - app, - ); + let mut h = app_harness(app); h.run_steps(4); assert!(h.query_by_label("Add").is_none(), "the layer is already on the map, so Add is not offered"); h.get_by_label("Remove").click(); @@ -2436,19 +2417,7 @@ mod tests { }); app.run("ui.dialog", json!({"name": "addService"})).unwrap(); app.dialogs.mode = "wcs"; - let mut fonts = false; - let mut h = egui_kittest::Harness::builder().with_size(vec2(1200.0, 900.0)).build_ui_state( - move |ui, app: &mut RustGisApp| { - if !fonts { - crate::theme::install_fonts(ui.ctx(), &[], None); - fonts = true; - return; - } - let ctx = ui.ctx().clone(); - super::show(app, &ctx); - }, - app, - ); + let mut h = app_harness(app); h.run_steps(4); assert!(h.query_by_label("Add").is_none(), "the coverage is already on the map, so Add is not offered"); h.get_by_label("Remove").click(); @@ -2501,19 +2470,7 @@ mod tests { }); app.run("ui.dialog", json!({"name": "addService"})).unwrap(); app.dialogs.mode = "ogcapi"; - let mut fonts = false; - let mut h = egui_kittest::Harness::builder().with_size(vec2(1200.0, 900.0)).build_ui_state( - move |ui, app: &mut RustGisApp| { - if !fonts { - crate::theme::install_fonts(ui.ctx(), &[], None); - fonts = true; - return; - } - let ctx = ui.ctx().clone(); - super::show(app, &ctx); - }, - app, - ); + let mut h = app_harness(app); h.run_steps(4); assert!(h.query_by_label("On the map").is_some(), "the collection is in the imported group"); assert!(h.query_by_label("Add").is_none(), "grouped features have no individual Add"); From 4be2df9da274a5bc7a0cb39611c19f5c49d6a055 Mon Sep 17 00:00:00 2001 From: mgaux-dev Date: Sat, 31 Oct 2026 00:26:21 +0100 Subject: [PATCH 6/9] fix(services): Add From Service matches only the addresses it added --- Cargo.lock | 1 + crates/engine/Cargo.toml | 2 + crates/engine/src/cmd/service_added.rs | 246 ++++++++++++++++++++----- crates/engine/src/tests.rs | 15 +- crates/ui-egui/src/dialogs.rs | 3 +- 5 files changed, 211 insertions(+), 56 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e69b80fc3..0b6f3db04 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4786,6 +4786,7 @@ dependencies = [ "serde_json", "sys-locale", "tiny-skia 0.12.0", + "url", "web-time", ] diff --git a/crates/engine/Cargo.toml b/crates/engine/Cargo.toml index 8dee7c482..ef29ef4c1 100644 --- a/crates/engine/Cargo.toml +++ b/crates/engine/Cargo.toml @@ -35,6 +35,8 @@ geolibre = ["dep:rustgis-geolibre"] rusqlite = { workspace = true } rustgis-plugins = { workspace = true, features = ["testing"] } tiny-skia = { workspace = true } +# Parsing the addresses commands ask for in tests. +url = { workspace = true } # Time slider animations as GIFs (exported by the desktop app only). [target.'cfg(not(target_arch = "wasm32"))'.dependencies] diff --git a/crates/engine/src/cmd/service_added.rs b/crates/engine/src/cmd/service_added.rs index 2a6361cde..cb224cbeb 100644 --- a/crates/engine/src/cmd/service_added.rs +++ b/crates/engine/src/cmd/service_added.rs @@ -1,5 +1,6 @@ -//! Which layers of a service's listing (`service.layers`) are on the map already, and the command -//! that removes each (Add From Service's Remove). A listed layer is known by the address it came +//! Add From Service's Remove: which layers of a service's listing (`service.layers`) are on the +//! map already, and the command that removes each. A helper for the dialog, not a command of its +//! own (`cmd::service` lists and adds the layers). A listed layer is known by the address it came //! from (`service::mark_service_source`, a tile layer's template), never by its name alone, so a //! file's layer or another service's that shares the title is left alone. @@ -14,27 +15,32 @@ pub type Removal = (&'static str, Value); /// row not asked about. Rows naming one layer each offer it; a tile layer is claimed by one OGC /// API row at most (`claim_tilesets`). pub fn added(map: &Map, listed: &[Value], rows: &[usize]) -> Vec> { - // Each layer's source as it is and without its keys, once: a project saved and opened again - // keeps its addresses without them (the listing's still carry them). + // Each layer's source and each tile layer's address as they are and without their keys, once: + // a project saved and opened again keeps its addresses without them (the listing's still carry + // them), and a copy of the same service added under another key is still known. let sources: Vec> = map.layers.iter().filter_map(|l| l.dataset.source.as_deref().map(|s| (l.id, s, crate::sources::redact(s)))).collect(); + let tiles: Vec> = map.tiles.iter().enumerate().map(|(i, t)| (i, t.url.as_str(), crate::sources::redact(&t.url))).collect(); let mut added = vec![None; listed.len()]; for &i in rows { if let (Some(l), Some(slot)) = (listed.get(i), added.get_mut(i)) { - *slot = removal(map, &sources, l); + *slot = removal(&sources, &tiles, l); } } - claim_tilesets(map, listed, rows, &mut added); + claim_tilesets(listed, rows, &tiles, &mut added); added } /// A map layer's id and source, as it is and without its keys. type Source<'a> = (u32, &'a str, String); -/// The command that removes the listed layer `l` from `map`, when it is there: matched by its +/// A tile layer's index, address, and address without its keys. +type Tile<'a> = (usize, &'a str, String); + +/// The command that removes the listed layer `l` from the map, when it is there: matched by its /// address as it is first, then without its keys (as a saved project keeps it), so a copy of the /// same service added under another key is only taken when nothing matches exactly. -fn removal(map: &Map, sources: &[Source<'_>], l: &Value) -> Option { +fn removal(sources: &[Source<'_>], tiles: &[Tile<'_>], l: &Value) -> Option { let offer = l.get("offer")?; let url = offer.get("url").and_then(Value::as_str).filter(|u| !u.is_empty())?; let keyless = crate::sources::redact(url); @@ -45,8 +51,9 @@ fn removal(map: &Map, sources: &[Source<'_>], l: &Value) -> Option { .map(|(id, ..)| ("layer.remove", json!({"layer": id}))) }; match offer.get("type").and_then(Value::as_str) { - // Downloaded features keep the address they came from (an area only adds a query to it). - Some("features") => remove_layer(&from_address), + // Downloaded features keep the address they came from, with only the area query the app + // adds to it. + Some("features") => remove_layer(&from_area), // A coverage keeps its GetCoverage request: the service's address, which its other // coverages share, with this coverage's id. Some("coverage") => { @@ -54,19 +61,29 @@ fn removal(map: &Map, sources: &[Source<'_>], l: &Value) -> Option { remove_layer(&|url, s| from_address(url, s) && coverage_id(s) == Some(id)) } // By position: a service's tile layers have no catalog id to tell them apart. Matched by - // the listed template here (a saved project keeps it without its keys); an OGC API - // layer's advertised one, by `claim_tilesets`. - Some("tiles" | "vectorTiles" | "style") => map - .tiles - .iter() - .position(|t| template_matches(url, &t.url)) - .or_else(|| map.tiles.iter().position(|t| template_matches(&keyless, &t.url))) - .map(|i| ("map.removeTiles", json!({"index": i}))), + // the listed template here; an OGC API layer's advertised one, by `claim_tilesets`. + Some("tiles" | "vectorTiles" | "style") => tile_match(tiles, l).map(|i| ("map.removeTiles", json!({"index": i}))), // A folder or service (its layers are added from their rows) has nothing to remove. _ => None, } } +/// The tile layer a listed layer's own template matches, if any: by its address as it is, then +/// without its keys (a saved project's, or a copy of the same service under another key). +fn tile_match(tiles: &[Tile<'_>], l: &Value) -> Option { + let offer = l.get("offer")?; + if !matches!(offer.get("type").and_then(Value::as_str), Some("tiles" | "vectorTiles" | "style")) { + return None; + } + let url = offer.get("url").and_then(Value::as_str).filter(|u| !u.is_empty())?; + let keyless = crate::sources::redact(url); + tiles + .iter() + .find(|(_, address, _)| template_matches(url, address)) + .or_else(|| tiles.iter().find(|(_, _, keyless_address)| template_matches(&keyless, keyless_address))) + .map(|(i, ..)| *i) +} + /// Gives the OGC API rows asked about (`rows`) that `added` doesn't match yet the tile layer their /// tileset list advertised. Its template isn't in the listing, so it is found under that list: a /// tile layer under one listed layer's list alone (counting every layer listed, shown or not), @@ -74,34 +91,39 @@ fn removal(map: &Map, sources: &[Source<'_>], l: &Value) -> Option { /// matched by its own template. Two rows of one collection may share a list: their tile matrix /// sets tell their tiles apart. A tile layer under no listed layer's list, or another tileset's /// under the same list (a layer added by hand), is nobody's, so Remove never removes another layer. -fn claim_tilesets(map: &Map, listed: &[Value], rows: &[usize], added: &mut [Option]) { - let mut claimed: Vec = added - .iter() - .flatten() - .filter(|(command, _)| *command == "map.removeTiles") - .filter_map(|(_, p)| p.get("index").and_then(Value::as_u64)) - .collect(); - // Each listed layer's tileset list and the start of its own template (its tile matrix set). - let starts: Vec> = listed +fn claim_tilesets(listed: &[Value], rows: &[usize], tiles: &[Tile<'_>], added: &mut [Option]) { + // Every listed row's own match, shown or not: a tile a row's own template matched is that + // row's, so an asked row's advertised template never takes it. + let mut claimed: Vec = listed.iter().filter_map(|l| tile_match(tiles, l)).map(|i| i as u64).collect(); + claimed.sort_unstable(); + claimed.dedup(); + // Each listed layer's tileset list and the start of its own template (its tile matrix set), + // as listed and without their keys: a saved project keeps its addresses without them. + let starts: Vec>> = listed .iter() .map(|l| { let list = tileset_list(l)?; let start = l.get("offer")?.get("url")?.as_str().map(tile_start)?; - (!start.is_empty()).then_some((list, start)) + (!start.is_empty()).then(|| Tileset { + list, + start, + list_keyless: crate::sources::redact(list), + start_keyless: crate::sources::redact(start), + }) }) .collect(); // How many listed layers' tile matrix sets each tile layer is under, once: a row counts only // for tiles that start with its own template's start, so the tile matrix sets of one // collection sharing a tileset list are told apart (and the same start twice counts once). - let under: Vec = map - .tiles + let under: Vec = tiles .iter() - .map(|t| { + .map(|(_, url, keyless)| { let mut starts: Vec<&str> = starts .iter() .flatten() - .filter(|(list, start)| under_list(list, &t.url) && t.url.starts_with(*start)) - .map(|(_, start)| *start) + .filter(|t| under_list(t.list, url) || under_list(&t.list_keyless, keyless)) + .filter(|t| url.starts_with(t.start) || keyless.starts_with(t.start_keyless.as_str())) + .map(|t| t.start_keyless.as_str()) .collect(); starts.sort_unstable(); starts.dedup(); @@ -112,13 +134,17 @@ fn claim_tilesets(map: &Map, listed: &[Value], rows: &[usize], added: &mut [Opti if added.get(i).is_none_or(Option::is_some) { continue; } - let Some((list, start)) = starts.get(i).copied().flatten() else { continue }; - let found = map - .tiles + let Some(tileset) = starts.get(i).and_then(Option::as_ref) else { continue }; + let found = tiles .iter() .zip(&under) .zip(0u64..) - .find(|((t, under), n)| **under == 1 && !claimed.contains(n) && under_list(list, &t.url) && t.url.starts_with(start)) + .find(|((tile, under), n)| { + **under == 1 + && !claimed.contains(n) + && (under_list(tileset.list, tile.1) || under_list(&tileset.list_keyless, &tile.2)) + && (tile.1.starts_with(tileset.start) || tile.2.starts_with(tileset.start_keyless.as_str())) + }) .map(|(_, n)| n); if let (Some(n), Some(slot)) = (found, added.get_mut(i)) { *slot = Some(("map.removeTiles", json!({"index": n}))); @@ -127,6 +153,14 @@ fn claim_tilesets(map: &Map, listed: &[Value], rows: &[usize], added: &mut [Opti } } +/// A listed OGC API layer's tileset list and template start, as listed and without their keys. +struct Tileset<'a> { + list: &'a str, + start: &'a str, + list_keyless: String, + start_keyless: String, +} + /// The address a tile template's tiles share: the template up to its first placeholder (`{z}`, /// `{Time}`), which names the tile matrix set. fn tile_start(template: &str) -> &str { @@ -143,13 +177,36 @@ fn under_list(list: &str, url: &str) -> bool { url.strip_prefix(list).is_some_and(|rest| rest.starts_with('/')) } -/// Whether `source` is the address `url`, or `url` with a query or fragment after it (an area's -/// bounding box): never another address that only starts with the same letters (`…/items2` for -/// `…/items`), nor one further down its path (`…/items/2`, one feature added by hand). +/// Whether `source` is a coverage's endpoint `url`, or `url` with a query or fragment after it +/// (its GetCoverage request): never another address that only starts with the same letters +/// (`…/wcs2` for `…/wcs`), nor one further down its path. fn from_address(url: &str, source: &str) -> bool { source.strip_prefix(url).is_some_and(|rest| rest.is_empty() || url.ends_with(['?', '&']) || rest.starts_with(['?', '&', '#'])) } +/// Whether `source` is the listed feature address `url` with only the area query the app appends +/// (`bbox`, a WFS's `BBOX`, or ArcGIS's geometry query): never a filter, another query of its own +/// or a fragment, so a filtered layer of the same service is left alone. +fn from_area(url: &str, source: &str) -> bool { + let Some(rest) = source.strip_prefix(url) else { return false }; + if rest.is_empty() { + return true; + } + let query = match url.ends_with(['?', '&']) { + true => rest, + false => match rest.strip_prefix('?').or_else(|| rest.strip_prefix('&')) { + Some(q) => q, + None => return false, + }, + }; + !query.is_empty() + && !query.contains('#') + && query.split('&').all(|kv| { + let key = kv.split_once('=').map_or(kv, |(k, _)| k); + matches!(key, "bbox" | "BBOX" | "geometry" | "geometryType" | "inSR" | "spatialRel") + }) +} + /// A WCS request's coverage (`COVERAGEID`, or a WCS 1.0's `COVERAGE`), as its address gives it. fn coverage_id(url: &str) -> Option<&str> { let query = url.split('#').next()?.split_once('?')?.1; @@ -165,7 +222,7 @@ fn template_matches(template: &str, url: &str) -> bool { let Some(first) = parts.next() else { return false }; let Some(mut rest) = url.strip_prefix(first) else { return false }; // Each placeholder takes a date: one piece of the address, never a path or query. - let date = |time: &str| !time.is_empty() && !time.contains(['/', '?', '#']); + let date = |time: &str| !time.is_empty() && !time.contains(['/', '?', '#', '&', '=']); let mut parts = parts.peekable(); while let Some(part) = parts.next() { if parts.peek().is_none() { @@ -200,6 +257,9 @@ mod tests { assert!(!template_matches(timed, "http://h/2024-06-01/{z}/{x}/{y}.jpg")); // A trailing placeholder: a date, not every address under the prefix. assert!(template_matches("http://h/tiles?time={Time}", "http://h/tiles?time=2024-06-01")); + // A placeholder takes a date, not the rest of a query (`&LAYERS=other` after it). + assert!(template_matches("http://h/tiles?TIME={Time}", "http://h/tiles?TIME=2024-06-01")); + assert!(!template_matches("http://h/tiles?TIME={Time}", "http://h/tiles?TIME=2024&LAYERS=other")); assert!(!template_matches("http://h/{Time}", "http://h/other/{z}/{x}/{y}.png")); assert!(!template_matches("http://h/{Time}", "http://h/")); // A middle one: one piece of the path, not several. @@ -208,16 +268,17 @@ mod tests { assert!(template_matches("http://h/{Time}/x/{Time}.png", "http://h/2024/x/2025.png")); } + /// A coverage endpoint matches its GetCoverage request after it, never another address that + /// only shares the prefix or is further down the path. #[test] - fn an_address_matches_only_itself_and_what_follows_it() { - let items = "https://h/items"; - assert!(from_address(items, items)); - assert!(from_address(items, "https://h/items?bbox=1,2,3,4")); - assert!(!from_address(items, "https://h/items/2"), "one feature further down the path"); - assert!(from_address(items, "https://h/items#top")); - assert!(!from_address(items, "https://h/items2"), "another service's address sharing the prefix"); - assert!(!from_address(items, "https://h/item")); - assert!(!from_address(items, "wfs.geojson")); + fn an_endpoint_matches_only_its_request() { + let endpoint = "https://h/wcs"; + assert!(from_address(endpoint, endpoint)); + assert!(from_address(endpoint, "https://h/wcs?SERVICE=WCS&REQUEST=GetCoverage")); + assert!(!from_address(endpoint, "https://h/wcs/2"), "a path under the endpoint"); + assert!(!from_address(endpoint, "https://h/wcs2"), "another address sharing the prefix"); + assert!(!from_address(endpoint, "https://h/wc")); + assert!(!from_address(endpoint, "wfs.geojson")); // An address ending in its query's start: what follows is that query. assert!(from_address("https://h/wcs?", "https://h/wcs?SERVICE=WCS")); // One ending in `/` is a path, not a prefix of every address under it. @@ -225,6 +286,28 @@ mod tests { assert!(from_address("https://h/", "https://h/?bbox=1,2,3,4")); } + /// A feature layer's address matches only the area query the app appends, never a filter or + /// fragment of its own. + #[test] + fn a_feature_address_matches_only_its_area_query() { + let items = "https://h/items"; + assert!(from_area(items, items)); + assert!(from_area(items, "https://h/items?bbox=1,2,3,4")); + assert!(from_area("https://h/items?f=json", "https://h/items?f=json&bbox=1,2,3,4")); + assert!(!from_area(items, "https://h/items?f=json&filter=private"), "a filtered layer is another layer"); + assert!(!from_area(items, "https://h/items?bbox=1,2,3,4&where=x")); + assert!(!from_area(items, "https://h/items#top"), "a fragment isn't an area"); + assert!(!from_area(items, "https://h/items/2"), "one feature further down the path"); + assert!(!from_area(items, "https://h/items2"), "another service's address sharing the prefix"); + // A WFS GetFeature address ending in its query's start, and ArcGIS's geometry query. + assert!(from_area("https://h/wfs?", "https://h/wfs?BBOX=1,2,3,4")); + assert!(from_area( + "https://h/FeatureServer/0/query", + "https://h/FeatureServer/0/query?geometry=1,2,3,4&geometryType=esriGeometryEnvelope&inSR=4326&spatialRel=esriSpatialRelIntersects" + )); + assert!(!from_area("https://h/FeatureServer/0/query", "https://h/FeatureServer/0/query?where=x")); + } + #[test] fn a_coverage_is_told_by_its_id() { let v2 = "https://h/wcs?SERVICE=WCS&VERSION=2.0.1&REQUEST=GetCoverage&COVERAGEID=dem&FORMAT=image%2Ftiff"; @@ -245,6 +328,17 @@ mod tests { assert_eq!(added(&map, &listed, &[0]), [Some(("map.removeTiles", json!({"index": 1})))]); } + /// A tile layer added under another key of the same service is found without its key, as a + /// feature layer is: the row doesn't offer Add and make a duplicate. + #[test] + fn a_tile_under_another_key_is_known() { + let offer = "https://h/wms?layers=a&api_key=A&bbox={bbox-epsg-3857}"; + let in_use = "https://h/wms?layers=a&api_key=B&bbox={bbox-epsg-3857}"; + let map = tiles(&[in_use]); + let listed = [json!({"title": "A", "offer": {"type": "tiles", "url": offer}})]; + assert_eq!(added(&map, &listed, &[0]), [Some(("map.removeTiles", json!({"index": 0})))]); + } + /// Two copies of one service under different keys: each row takes its own copy first. #[test] fn the_exact_address_wins_over_the_keyless_one() { @@ -325,6 +419,23 @@ mod tests { assert_eq!(added(&map, &listed, &[0]), [Some(("map.removeTiles", json!({"index": 1}))), None]); } + /// A tile a listed row's own template matches is that row's even when the filter hides it: an + /// OGC API row doesn't take it with its advertised template. + #[test] + fn a_tile_a_hidden_row_matches_is_not_claimed() { + let tile = "https://h/c/tiles/WebMercatorQuad/{z}/{y}/{x}.png"; + let map = tiles(&[tile]); + let listed = [ + json!({"offer": {"type": "tiles", "url": tile}}), + json!({ + "offer": {"type": "tiles", "url": "https://h/c/tiles/WebMercatorQuad/{z}/{y}/{x}"}, + "tilesets": "https://h/c/tiles?f=json", + }), + ]; + // The OGC API row is shown, the row whose template is the tile is hidden. + assert_eq!(added(&map, &listed, &[1]), [None, None]); + } + /// A tile layer under the listed layer's tileset list but in another tile matrix set (a layer /// added by hand from the same collection) is left alone. #[test] @@ -336,4 +447,39 @@ mod tests { }); assert_eq!(added(&map, &[row], &[0]), [None]); } + + /// A project saved and reopened keeps the tileset's addresses without their keys: the OGC API + /// row still claims its advertised tile. + #[test] + fn a_saved_tileset_still_claims_its_advertised_tile() { + let list = "https://user:pass@h/c/tiles?token=K&f=json"; + let offer = "https://user:pass@h/c/tiles/WebMercatorQuad/{z}/{y}/{x}"; + let saved_tile = "https://h/c/tiles/WebMercatorQuad/{z}/{y}/{x}.png"; + let map = tiles(&[saved_tile]); + let listed = [json!({"offer": {"type": "tiles", "url": offer}, "tilesets": list})]; + assert_eq!(added(&map, &listed, &[0]), [Some(("map.removeTiles", json!({"index": 0})))]); + } + + /// A coverage added as a real raster keeps its GetCoverage request, so the matcher knows it. + #[cfg(not(target_arch = "wasm32"))] + #[test] + fn a_real_rasters_coverage_is_known_by_its_request() { + use crate::{Project, Session}; + let mut s = Session::new(Project::default()); + let bytes = crate::tests::multiband_tiff(1, 1, &[vec![0.0]], (0.0, 0.0), 1.0, None); + let r = crate::cmd::raster::add_named_bytes(&mut s, &json!({"name": "Soil carbon"}), "soil.tif".into(), bytes).unwrap(); + let id = r["layer"].as_u64().unwrap() as u32; + let get = "https://h/wcs?SERVICE=WCS&VERSION=2.0.1&REQUEST=GetCoverage&COVERAGEID=soc&FORMAT=image%2Ftiff"; + crate::cmd::service::mark_service_source(&mut s, &r, get); + let listed = [json!({ + "title": "Soil carbon", + "offer": { + "type": "coverage", + "url": "https://h/wcs", + "describe": "https://h/wcs?SERVICE=WCS&VERSION=2.0.1&REQUEST=DescribeCoverage&COVERAGEID=soc", + "version": "2.0.1", + }, + })]; + assert_eq!(added(s.map(), &listed, &[0]), [Some(("layer.remove", json!({"layer": id})))]); + } } diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 6fa920cc9..006f9b7e2 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -10111,9 +10111,16 @@ fn service_features_asked_for_over_an_area() { let r = add(&mut s, "1.1.0", Some(piaui.clone())); assert_eq!(r["features"], 1); assert_eq!(r["area"], piaui); - // The layer keeps the service address it came from, not its download's file name. - let source = s.map().layer_by_name("M0").and_then(|l| l.dataset.source.clone()); - assert!(source.as_deref().is_some_and(|src| src.starts_with(&format!("{url}?"))), "{source:?}"); + // The layer keeps the service address it came from, not its download's file name: its + // GetFeature request for this type, with the area's box added. + let source = s.map().layer_by_name("M0").and_then(|l| l.dataset.source.clone()).unwrap(); + let requested = url::Url::parse(&source).unwrap(); + let query: std::collections::HashMap = requested.query_pairs().into_owned().collect(); + assert_eq!(query.get("SERVICE").map(String::as_str), Some("WFS"), "{source}"); + assert_eq!(query.get("VERSION").map(String::as_str), Some("1.1.0"), "{source}"); + assert_eq!(query.get("REQUEST").map(String::as_str), Some("GetFeature"), "{source}"); + assert_eq!(query.get("TYPENAME").map(String::as_str), Some("CGMAT:munic"), "{source}"); + assert!(query.contains_key("BBOX"), "{source}"); let r = add(&mut s, "2.0.0", Some(json!("view"))); assert!(r["area"].is_array(), "{r}"); let r = add(&mut s, "2.0.0", None); @@ -12222,7 +12229,7 @@ fn a_trend_over_a_data_cube() { /// A multi-band Float32 GeoTIFF in longitude/latitude, its bands' values interleaved, as GDAL /// writes it: `descriptions` in its GDAL_METADATA tag when given, and -9999 for no-data. #[cfg(not(target_arch = "wasm32"))] -fn multiband_tiff(w: u32, h: u32, bands: &[Vec], origin: (f64, f64), px: f64, descriptions: Option<&[String]>) -> Vec { +pub(crate) fn multiband_tiff(w: u32, h: u32, bands: &[Vec], origin: (f64, f64), px: f64, descriptions: Option<&[String]>) -> Vec { let n = bands.len() as u16; let mut out = vec![b'I', b'I', 42, 0, 0, 0, 0, 0]; for p in 0..(w * h) as usize { diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index a74ef32c7..a8f07ff04 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -809,8 +809,7 @@ fn refresh_added(app: &mut RustGisApp) { if conn.added.as_ref().is_some_and(|a| a.revision == revision && a.filter == *filter) { return; } - let none = Vec::new(); - let layers = conn.listing.as_ref().and_then(|v| v.get("layers")).and_then(Value::as_array).unwrap_or(&none); + let layers: &[Value] = conn.listing.as_ref().and_then(|v| v.get("layers")).and_then(Value::as_array).map_or(&[], Vec::as_slice); let matching = matching_rows(layers, filter); let matched = matching.len(); let shown: Vec = matching.into_iter().take(SHOWN_LAYERS).map(|(i, _)| i).collect(); From 0b7be8186e9346ac234d17aa39ab8d26bd1637f3 Mon Sep 17 00:00:00 2001 From: mgaux-dev Date: Sat, 31 Oct 2026 01:03:42 +0100 Subject: [PATCH 7/9] fix(ui): a service raster groups under the address it came from Add From Service's coverage downloads keep their GetCoverage request as the layer's source, but the Data Source view preferred the download's file name (coverage.tif). Prefer a web or database address over a raster's file name, as the docs and the feature layers already say, keep a layer with no raster on its dataset source, and leave local files grouped by their path. --- crates/ui-egui/src/panes.rs | 36 +++++++++++++++++++++++++++++++-- docs/user-guide/data-formats.md | 2 +- 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/crates/ui-egui/src/panes.rs b/crates/ui-egui/src/panes.rs index e4b4bb80a..cc6a558d1 100644 --- a/crates/ui-egui/src/panes.rs +++ b/crates/ui-egui/src/panes.rs @@ -985,8 +985,13 @@ fn path_name(path: &str) -> SourceName { /// The source a layer was read from (a raster's file, or the dataset's file, address or /// database), if any: layers made in the app (samples, drawings, tool outputs) have none. fn layer_source(l: &Layer) -> Option { - let raster = l.raster.as_ref().and_then(|r| r.path.as_ref().or(r.file_name.as_ref())); - raster.or(l.dataset.source.as_ref()).filter(|s| !s.trim().is_empty()).map(|s| source_name(s.as_str())) + // A service's downloaded raster keeps the address it came from as its dataset source, not + // the file name it was read under: group it by that address, as a feature layer is. A layer + // with no raster keeps its dataset source as it is (a database's name, say). + let source = l.dataset.source.as_deref(); + let address = source.filter(|s| s.split_once("://").is_some_and(|(scheme, _)| !scheme.eq_ignore_ascii_case("file"))); + let raster = l.raster.as_ref().and_then(|r| r.path.as_deref().or(r.file_name.as_deref())); + address.or(raster).or(source).filter(|s| !s.trim().is_empty()).map(source_name) } /// A group's heading in a Layers view, with optional hover text. @@ -6650,6 +6655,33 @@ mod tests { assert_eq!(source_name("file://user@localhost/x.gpkg").label, "x.gpkg"); } + /// A service's downloaded raster groups under the address it came from, not the file name its + /// download was read under; a local raster still groups by its file. + #[test] + fn a_service_rasters_source_is_its_address() { + use rustgis_engine::data::raster::{ColorHint, Crs, GeoTransform, Raster}; + let raster = || { + let gt = GeoTransform { origin_x: 0.0, origin_y: 1.0, pixel_width: 1.0, pixel_height: -1.0 }; + Raster::new("soil", 1, 1, vec![vec![1.0]], gt, Crs::LonLat, None, ColorHint::Gray, "Float32").unwrap() + }; + let mut ds = rustgis_engine::data::Dataset::new("Soil carbon", rustgis_engine::data::GeometryType::None, Vec::new()); + ds.source = Some("https://user:secret@h/wcs?SERVICE=WCS&COVERAGEID=soc&token=K".into()); + let mut l = Layer::new(7, ds); + l.raster = Some(rustgis_engine::map::RasterLayer::from_bytes(raster(), "coverage.tif", Vec::new())); + let src = layer_source(&l).unwrap(); + assert_eq!(src.label, "h"); + assert!(!src.key.contains("secret") && !src.hover.contains("secret")); + // A local file is grouped by its path, as before. + let mut local = Layer::new(8, rustgis_engine::data::Dataset::new("Local", rustgis_engine::data::GeometryType::None, Vec::new())); + local.raster = Some(rustgis_engine::map::RasterLayer::from_path(raster(), r"C:\GIS\soil.tif")); + assert_eq!(layer_source(&local).unwrap().label, "soil.tif"); + // A layer with no raster keeps its dataset source, even one that names no address scheme + // (a database connection kept by name). + let mut db = rustgis_engine::data::Dataset::new("Roads", rustgis_engine::data::GeometryType::Polyline, Vec::new()); + db.source = Some("osm".into()); + assert_eq!(layer_source(&Layer::new(9, db)).unwrap().label, "osm"); + } + /// #353: a row wider than the left pane (a long folder path in the Browser, a long favourite in /// Data Sources) widened the pane to its 480 px cap, and it stayed that wide on every pane. /// The pane keeps the width it was given; the row is cut off at its edge. diff --git a/docs/user-guide/data-formats.md b/docs/user-guide/data-formats.md index b6cd27518..8d61b088a 100644 --- a/docs/user-guide/data-formats.md +++ b/docs/user-guide/data-formats.md @@ -143,7 +143,7 @@ Layers pane. Other offerings have individual **Add** buttons (`service.layers` a from scripts, the command line and MCP). A layer already on the active map shows **Remove** instead: the row removes it with `layer.remove` (`map.removeTiles` for a tile layer), so the removal is undone like any other edit. -A layer is known by the address it was added from (a tile layer's template), not its title alone, +A layer is known by the address it was added from (a tile layer's template), never by its title, so one of yours or of another service with the same name is left alone. A layer saved by an older version, which kept the file name it was downloaded under rather than the service's address, shows **Add**; RustGIS won't guess by name. A service's downloaded layers are grouped under the From 50182eddbbe624e79c0d26fc334811f03a6d1520 Mon Sep 17 00:00:00 2001 From: mgaux-dev Date: Sat, 31 Oct 2026 01:03:43 +0100 Subject: [PATCH 8/9] fix(services): a tileset list ending in a slash still claims its tile An OGC API tilesets link may end with a slash; the listed tile's address is then under the same list without it. Treat the trailing slash as part of the same path, so the row offers Remove instead of Add. --- crates/engine/src/cmd/service_added.rs | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/crates/engine/src/cmd/service_added.rs b/crates/engine/src/cmd/service_added.rs index cb224cbeb..8b87f8209 100644 --- a/crates/engine/src/cmd/service_added.rs +++ b/crates/engine/src/cmd/service_added.rs @@ -172,9 +172,11 @@ fn tileset_list(l: &Value) -> Option<&str> { l.get("tilesets").and_then(Value::as_str).and_then(|t| t.split(['?', '#']).next()).filter(|t| !t.is_empty()) } -/// Whether the tile template `url` is under the tileset list address `list` (its tilesets' paths). +/// Whether the tile template `url` is under the tileset list address `list` (its tilesets' paths): +/// a trailing slash on the list names the same list. fn under_list(list: &str, url: &str) -> bool { - url.strip_prefix(list).is_some_and(|rest| rest.starts_with('/')) + let list = list.trim_end_matches('/'); + !list.is_empty() && url.strip_prefix(list).is_some_and(|rest| rest.starts_with('/')) } /// Whether `source` is a coverage's endpoint `url`, or `url` with a query or fragment after it @@ -419,6 +421,17 @@ mod tests { assert_eq!(added(&map, &listed, &[0]), [Some(("map.removeTiles", json!({"index": 1}))), None]); } + /// A tileset list address written with a trailing slash names the same list. + #[test] + fn a_tileset_list_with_a_trailing_slash_still_claims_its_tile() { + let map = tiles(&["https://h/c/tiles/WebMercatorQuad/{z}/{y}/{x}.png"]); + let row = json!({ + "offer": {"type": "tiles", "url": "https://h/c/tiles/WebMercatorQuad/{z}/{y}/{x}"}, + "tilesets": "https://h/c/tiles/?f=json", + }); + assert_eq!(added(&map, &[row], &[0]), [Some(("map.removeTiles", json!({"index": 0})))]); + } + /// A tile a listed row's own template matches is that row's even when the filter hides it: an /// OGC API row doesn't take it with its advertised template. #[test] From 9ecf8eebc838a45d968d10b7cc7457b95eac05d9 Mon Sep 17 00:00:00 2001 From: mgaux-dev Date: Sat, 31 Oct 2026 03:36:44 +0100 Subject: [PATCH 9/9] fix(services): a hand-added tile isn't claimed by a listed OGC API row Add From Service's Remove took any tile layer under an OGC API row's tile matrix set prefix as the row's advertised tile, so a tile added by hand in another path under that prefix showed Remove. A tile now has to follow the row's own template (its coordinates in the service's order, then an optional format extension), so only the advertised tileset is taken. Also from the review: assert the exact BBOX the WFS layer keeps as its source (not just that it has one), and say in the docs that the keyless match is meant to know a reopened project and the same service under another key, so Remove doesn't offer a duplicate Add. The coordinate-system test keeps its own harness; the new Add From Service tests share the helper. Screenshots (Add / Remove / On the map) rendered with ui_shot. --- crates/engine/src/cmd/service_added.rs | 78 ++++++++++++++++++++++---- crates/engine/src/tests.rs | 2 +- crates/ui-egui/src/dialogs.rs | 14 ++++- docs/user-guide/data-formats.md | 12 ++-- 4 files changed, 88 insertions(+), 18 deletions(-) diff --git a/crates/engine/src/cmd/service_added.rs b/crates/engine/src/cmd/service_added.rs index 8b87f8209..7c4fcf139 100644 --- a/crates/engine/src/cmd/service_added.rs +++ b/crates/engine/src/cmd/service_added.rs @@ -38,8 +38,10 @@ type Source<'a> = (u32, &'a str, String); type Tile<'a> = (usize, &'a str, String); /// The command that removes the listed layer `l` from the map, when it is there: matched by its -/// address as it is first, then without its keys (as a saved project keeps it), so a copy of the -/// same service added under another key is only taken when nothing matches exactly. +/// address as it is first, then without its keys (as a saved project keeps it). The keyless form +/// names the same layer: a project reopened from disk keeps no keys, and a copy of the same +/// service added under another key is that service's layer, so Remove takes it rather than +/// offering an Add that would duplicate it. An exact copy always wins. fn removal(sources: &[Source<'_>], tiles: &[Tile<'_>], l: &Value) -> Option { let offer = l.get("offer")?; let url = offer.get("url").and_then(Value::as_str).filter(|u| !u.is_empty())?; @@ -87,28 +89,33 @@ fn tile_match(tiles: &[Tile<'_>], l: &Value) -> Option { /// Gives the OGC API rows asked about (`rows`) that `added` doesn't match yet the tile layer their /// tileset list advertised. Its template isn't in the listing, so it is found under that list: a /// tile layer under one listed layer's list alone (counting every layer listed, shown or not), -/// starting with the listed layer's own template's start (its tile matrix set), and that no row -/// matched by its own template. Two rows of one collection may share a list: their tile matrix -/// sets tell their tiles apart. A tile layer under no listed layer's list, or another tileset's -/// under the same list (a layer added by hand), is nobody's, so Remove never removes another layer. +/// following the listed layer's own template (its tile matrix set, then the same address up to a +/// format extension), and that no row matched by its own template. Two rows of one collection may +/// share a list: their tile matrix sets tell their tiles apart. A tile layer under no listed +/// layer's list, or in another tile matrix set or another path under the same list (a layer added +/// by hand), is nobody's, so Remove never removes another layer. fn claim_tilesets(listed: &[Value], rows: &[usize], tiles: &[Tile<'_>], added: &mut [Option]) { // Every listed row's own match, shown or not: a tile a row's own template matched is that // row's, so an asked row's advertised template never takes it. let mut claimed: Vec = listed.iter().filter_map(|l| tile_match(tiles, l)).map(|i| i as u64).collect(); claimed.sort_unstable(); claimed.dedup(); - // Each listed layer's tileset list and the start of its own template (its tile matrix set), - // as listed and without their keys: a saved project keeps its addresses without them. + // Each listed layer's tileset list and its own template (its tile matrix set and the tiles + // under it), as listed and without their keys: a saved project keeps its addresses without + // them. let starts: Vec>> = listed .iter() .map(|l| { let list = tileset_list(l)?; - let start = l.get("offer")?.get("url")?.as_str().map(tile_start)?; + let template = l.get("offer")?.get("url")?.as_str()?; + let start = tile_start(template); (!start.is_empty()).then(|| Tileset { list, start, + template, list_keyless: crate::sources::redact(list), start_keyless: crate::sources::redact(start), + template_keyless: crate::sources::redact(template), }) }) .collect(); @@ -143,7 +150,7 @@ fn claim_tilesets(listed: &[Value], rows: &[usize], tiles: &[Tile<'_>], added: & **under == 1 && !claimed.contains(n) && (under_list(tileset.list, tile.1) || under_list(&tileset.list_keyless, &tile.2)) - && (tile.1.starts_with(tileset.start) || tile.2.starts_with(tileset.start_keyless.as_str())) + && (template_follows(tileset.template, tile.1) || template_follows(&tileset.template_keyless, &tile.2)) }) .map(|(_, n)| n); if let (Some(n), Some(slot)) = (found, added.get_mut(i)) { @@ -153,12 +160,15 @@ fn claim_tilesets(listed: &[Value], rows: &[usize], tiles: &[Tile<'_>], added: & } } -/// A listed OGC API layer's tileset list and template start, as listed and without their keys. +/// A listed OGC API layer's tileset list and template (and that template's start), as listed and +/// without their keys. struct Tileset<'a> { list: &'a str, start: &'a str, + template: &'a str, list_keyless: String, start_keyless: String, + template_keyless: String, } /// The address a tile template's tiles share: the template up to its first placeholder (`{z}`, @@ -236,6 +246,28 @@ fn template_matches(template: &str, url: &str) -> bool { rest.is_empty() } +/// Whether the tile address `url` follows the advertised template `template`: the same address, +/// up to the order the template writes its three coordinates in (`{z}/{y}/{x}`, or the service's +/// own `{z}/{x}/{y}`) and the tile's format extension after them (`…/{z}/{y}/{x}` and +/// `…/{z}/{y}/{x}.png`). A tile with anything else in between, or in another path under the +/// tile matrix set (`…/WebMercatorQuad/custom/…`), doesn't, so a layer added by hand is left alone. +fn template_follows(template: &str, url: &str) -> bool { + if template_matches(template, url) { + return true; + } + // What stays around the tile coordinates, which a service may write in another order: the + // path, the extension and the query must still be the template's. + let skeleton = |address: &str| address.replace("{z}", "").replace("{x}", "").replace("{y}", ""); + let (template, url) = (skeleton(template), skeleton(url)); + if url == template { + return true; + } + // The format's extension after the template: a short run of letters and digits after a dot at + // the address's end, never a path or a query. + url.rsplit_once('.') + .is_some_and(|(stem, ext)| !ext.is_empty() && ext.len() <= 8 && ext.chars().all(|c| c.is_ascii_alphanumeric()) && stem == template) +} + #[cfg(test)] mod tests { use super::*; @@ -461,6 +493,30 @@ mod tests { assert_eq!(added(&map, &[row], &[0]), [None]); } + /// A tile layer under the listed layer's tileset list and tile matrix set but in another path + /// (a layer added by hand) doesn't follow the advertised template: it is left alone. + #[test] + fn a_hand_added_tile_in_another_path_is_left_alone() { + let map = tiles(&["https://h/c/tiles/WebMercatorQuad/custom/{z}/{y}/{x}.png"]); + let row = json!({ + "offer": {"type": "tiles", "url": "https://h/c/tiles/WebMercatorQuad/{z}/{y}/{x}"}, + "tilesets": "https://h/c/tiles?f=json", + }); + assert_eq!(added(&map, &[row], &[0]), [None]); + } + + /// The tile's format extension after the advertised template (a template without one) doesn't + /// stop its row from claiming it. + #[test] + fn a_tiles_format_extension_follows_the_advertised_template() { + let map = tiles(&["https://h/c/tiles/WebMercatorQuad/{z}/{y}/{x}.png"]); + let row = json!({ + "offer": {"type": "tiles", "url": "https://h/c/tiles/WebMercatorQuad/{z}/{y}/{x}"}, + "tilesets": "https://h/c/tiles?f=json", + }); + assert_eq!(added(&map, &[row], &[0]), [Some(("map.removeTiles", json!({"index": 0})))]); + } + /// A project saved and reopened keeps the tileset's addresses without their keys: the OGC API /// row still claims its advertised tile. #[test] diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 006f9b7e2..65423d4e4 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -10120,7 +10120,7 @@ fn service_features_asked_for_over_an_area() { assert_eq!(query.get("VERSION").map(String::as_str), Some("1.1.0"), "{source}"); assert_eq!(query.get("REQUEST").map(String::as_str), Some("GetFeature"), "{source}"); assert_eq!(query.get("TYPENAME").map(String::as_str), Some("CGMAT:munic"), "{source}"); - assert!(query.contains_key("BBOX"), "{source}"); + assert_eq!(query.get("BBOX").map(String::as_str), Some("-10.93,-45.99,-2.74,-40.37,urn:ogc:def:crs:EPSG::4326"), "{source}"); let r = add(&mut s, "2.0.0", Some(json!("view"))); assert!(r["area"].is_array(), "{r}"); let r = add(&mut s, "2.0.0", None); diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index a8f07ff04..d14ffd3de 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -2171,7 +2171,19 @@ mod tests { let mut app = RustGisApp::with_sample(); app.run("map.projection", json!({"type": "robinson"})).unwrap(); app.run("ui.dialog", json!({"name": "coordinateSystem"})).unwrap(); - let mut h = app_harness(app); + let mut fonts = false; + let mut h = egui_kittest::Harness::builder().with_size(vec2(1200.0, 900.0)).build_ui_state( + move |ui, app: &mut RustGisApp| { + if !fonts { + crate::theme::install_fonts(ui.ctx(), &[], None); + fonts = true; + return; + } + let ctx = ui.ctx().clone(); + super::show(app, &ctx); + }, + app, + ); h.run_steps(3); // A world preset takes only its meridian. assert!(h.query_by_role_and_label(Role::SpinButton, "First standard parallel").is_none()); diff --git a/docs/user-guide/data-formats.md b/docs/user-guide/data-formats.md index 8d61b088a..594951cd2 100644 --- a/docs/user-guide/data-formats.md +++ b/docs/user-guide/data-formats.md @@ -144,11 +144,13 @@ from scripts, the command line and MCP). A layer already on the active map shows **Remove** instead: the row removes it with `layer.remove` (`map.removeTiles` for a tile layer), so the removal is undone like any other edit. A layer is known by the address it was added from (a tile layer's template), never by its title, -so one of yours or of another service with the same name is left alone. A layer saved by an older -version, which kept the file name it was downloaded under rather than the service's address, -shows **Add**; RustGIS won't guess by name. A service's downloaded layers are grouped under the -service's host in the Layers pane's **Data Source** view, and **Copy Path** gives that address -without its keys or tokens. +so one of yours or of another service with the same name is left alone. The same service added +under another key counts as the same layer, and so does a project reopened from disk, whose saved +address carries no keys: the row shows **Remove** rather than an **Add** that would duplicate it. +A layer saved by an older version, which kept the file name it was downloaded under rather than +the service's address, shows **Add**; RustGIS won't guess by name. A service's downloaded layers +are grouped under the service's host in the Layers pane's **Data Source** view, and **Copy Path** +gives that address without its keys or tokens. An OGC API landing page works directly: RustGIS follows its advertised JSON collections link, so you don't need to append `/collections`. If the landing page also offers map tiles, they remain listed alongside the collections. A failed collections request