test: UI tests take turns for the tile cache; a COG just probed is framed after a cache clear (#1499)

crates/engine/src/cmd/embeddings.rs

@@ -153,7 +153,8 @@ fn add(s: &mut Session, p: &Value) -> CmdResult {
             if !crate::render::rustgis_tiles::cog::probe_blocking(file)? {
                 return Err(format!("{file}: not a cloud-optimised GeoTIFF"));
             }
-            super::layer::add_cog(s, &json!({"name": name}), &crate::render::rustgis_tiles::cog::with_look(file, &look))
+            let look = crate::render::rustgis_tiles::cog::with_look(file, &look);
+            super::layer::add_cog(s, &json!({"name": name}), &look, cfg!(not(target_arch = "wasm32")))
         }
         "earth-index" => add_points(s, p, name),
         _ => add_tessera(s, p, d, name),

crates/engine/src/cmd/geolibre.rs

@@ -369,7 +369,7 @@ fn add_layer(s: &mut Session, r: &Value, dir: Option<&str>, report: &mut Report,
             return Ok(Added::Feature(id));
         }
         if kind == "cog" && remote {
-            super::layer::add_cog(s, &q, &address)?
+            super::layer::add_cog(s, &q, &address, false)?
         } else if kind == "zarr" {
             if let Some(v) = src.get("variable") {
                 q["variable"] = v.clone();

crates/engine/src/cmd/layer.rs

@@ -3148,7 +3148,7 @@ fn add_url_as_given(s: &mut Session, p: &Value) -> CmdResult {
         }
         let web = lower.starts_with("https://") || lower.starts_with("http://");
         return if is_tiff_url(url) && web {
-            add_cog(s, p, url)
+            add_cog(s, p, url, false)
         } else {
             Err(format!(
                 "{url}: dated addresses work for cloud-optimised GeoTIFFs on the web (http, https) here, and for tile layers (map.addTiles)"
@@ -3189,7 +3189,7 @@ fn add_url_as_given(s: &mut Session, p: &Value) -> CmdResult {
     {
         // Nothing may wait here: a GeoTIFF is taken to be cloud-optimised (the layer says if not).
         if tiff && !scan && !super::b(p, "whole").unwrap_or(false) {
-            return add_cog(s, p, url);
+            return add_cog(s, p, url, false);
         }
         // Downloaded by the app off the frame and added when it lands.
         s.request_url(p)
@@ -3213,7 +3213,7 @@ fn add_url_as_given(s: &mut Session, p: &Value) -> CmdResult {
                 Err(e) => return Err(named(e)),
             };
             if cog {
-                return add_cog(s, p, url);
+                return add_cog(s, p, url, true);
             }
         }
         let bytes = if rustgis_data::io::shapefile_part_urls(url).is_some() {
@@ -3533,8 +3533,10 @@ pub fn is_unnamed_url(url: &str) -> bool {
 }
 
 /// Adds a cloud-optimised GeoTIFF by its address as a layer read in place (its tiles by range,
-/// as the view needs them), framed when its header is in.
-pub(crate) fn add_cog(s: &mut Session, p: &Value, url: &str) -> CmdResult {
+/// as the view needs them), framed when its header is in. `probed`: its header was just read
+/// (natively), so it is waited for if something emptied the tile cache since (a login ended): the
+/// layer would go unframed. One not read yet isn't waited for (a project opening its files).
+pub(crate) fn add_cog(s: &mut Session, p: &Value, url: &str, probed: bool) -> CmdResult {
     let stem = cog_name(url);
     let name = ps(p, "name").map(str::trim).filter(|n| !n.is_empty()).unwrap_or(&stem);
     let empty_before = super::map_is_empty(s);
@@ -3546,17 +3548,36 @@ pub(crate) fn add_cog(s: &mut Session, p: &Value, url: &str) -> CmdResult {
         }
     }
     let mut r = super::tiles::add(s, &q)?;
-    // A dated address is framed on the file of the date it shows.
-    let url = match r.get("url").and_then(Value::as_str) {
+    // A plugin's command in the app doesn't wait (it runs on the frame), as in the browser.
+    let probed = probed && !s.leave_downloads;
+    // A dated address is framed on the file of the date it shows; why its file couldn't be read.
+    let (url, probed, mut why) = match r.get("url").and_then(Value::as_str) {
         Some(dated) => {
             // Its header read now (natively) as an undated address's is before it is added.
             #[cfg(not(target_arch = "wasm32"))]
-            let _ = crate::render::rustgis_tiles::cog::probe_blocking(dated);
-            dated.to_string()
+            let probe = if s.leave_downloads { Ok(false) } else { crate::render::rustgis_tiles::cog::probe_blocking(dated) };
+            #[cfg(target_arch = "wasm32")]
+            let probe: Result<bool, String> = Ok(false);
+            (dated.to_string(), probe == Ok(true), probe.err())
         }
-        None => url.to_string(),
+        None => (url.to_string(), probed, None),
     };
-    if let Ok(Some(info)) = crate::render::rustgis_tiles::cog::info(&url) {
+    #[cfg(not(target_arch = "wasm32"))]
+    let info = if probed {
+        crate::render::rustgis_tiles::cog::info_blocking(&url).map(Some)
+    } else {
+        crate::render::rustgis_tiles::cog::info(&url)
+    };
+    #[cfg(target_arch = "wasm32")]
+    let info = crate::render::rustgis_tiles::cog::info(&url);
+    let info = match info {
+        Ok(info) => info,
+        Err(e) => {
+            why.get_or_insert(e);
+            None
+        }
+    };
+    if let Some(info) = info {
         let [w, so, e, n] = info.bounds;
         // A cloud-optimised GeoTIFF read in place is framed when it is the first content on the
         // map, as other data is; `"zoom"` says otherwise (`true`, or `false` to keep the view).
@@ -3584,9 +3605,17 @@ pub(crate) fn add_cog(s: &mut Session, p: &Value, url: &str) -> CmdResult {
         for line in said {
             s.notify(format!("{shown}: {line}"), false);
         }
-    } else if r.get("dated").is_some() {
-        // Added, but its first date's file couldn't be read: the view stays where it was.
-        r["warning"] = json!(format!("{url} couldn't be read yet, so the map wasn't moved to it"));
+    } else if probed || why.is_some() || r.get("dated").is_some() {
+        // Added, but its (first date's) file couldn't be read: the view stays where it was, and
+        // the answer says why when the read said (its keys left out).
+        r["warning"] = json!(match why {
+            Some(e) => format!(
+                "{} couldn't be read ({}), so the map wasn't moved to it",
+                crate::sources::redact(&url),
+                crate::sources::redact_text(&e)
+            ),
+            None => format!("{url} couldn't be read yet, so the map wasn't moved to it"),
+        });
     }
     r["inPlace"] = json!(true);
     Ok(r)

crates/engine/src/lib.rs

@@ -856,7 +856,8 @@ fn fresh_source_ids(before: &Project, after: &mut Project) {
 /// layer holds only the window in view, so its other features aren't gone. It compares ids, so an
 /// id a single command both removes and hands out stays locked: only restoring from an archive can.
 /// A command that removes a layer removes its locks and line anchors too, or each frame would read
-/// them for nothing.
+/// them for nothing. It compares layer ids too: a layer id the map hands out again (after `u32::MAX`,
+/// or once a failed add rewinds the counter) keeps the old layer's entries.
 fn forget_label_state_of_removed_features(before: &Project, after: &mut Project) {
     for (i, m) in after.maps.iter_mut().enumerate() {
         let was = before.maps.get(i).filter(|o| o.name == m.name);

crates/engine/src/tests.rs

@@ -4160,6 +4160,10 @@ fn a_removed_layers_locked_labels_go_with_it() {
     assert_eq!(locked_layers(&s), (vec![a, b], vec![a, b]));
     s.run("edit.redo", &Value::Null).unwrap();
     assert_eq!(locked_layers(&s), (vec![b], vec![b]));
+    // The last locked layer gone, the lock holds, with nothing in it.
+    s.run("layer.remove", &json!({"layer": b})).unwrap();
+    assert!(s.map().label_lock.as_ref().is_some_and(Vec::is_empty));
+    assert!(s.map().label_lock_line_anchors.is_empty());
 }
 
 /// A deleted feature's lock entry and line anchor go with it, not those of another layer's feature
@@ -4176,6 +4180,36 @@ fn a_deleted_feature_leaves_another_layers_lock_alone() {
     assert_eq!(locked_layers(&s), (vec![b], vec![b]));
 }
 
+/// Deleting one of a layer's locked features keeps the lock entry and line anchor of the other.
+#[test]
+fn a_deleted_feature_leaves_its_neighbours_lock_alone() {
+    use rustgis_data::{Dataset, Field, FieldType, Geometry, GeometryType, Value as V};
+    let mut s = Session::new(Project::default());
+    let mut d = Dataset::new("A", GeometryType::Point, vec![Field::new("name", FieldType::Text)]);
+    for name in ["one", "two"] {
+        d.push(Some(Geometry::Point(rustgis_geom::Coord::new(0.0, 0.0))), vec![V::Text(name.into())]);
+    }
+    let a = s.map_mut().add_dataset(d);
+    let keys = [(a, 1, 0, 0), (a, 2, 0, 0)];
+    s.map_mut().label_lock = Some(keys.map(|k| (k, Default::default())).to_vec());
+    s.map_mut().label_lock_line_anchors = keys.map(|k| (k, Default::default())).into_iter().collect();
+    // The feature ids the lock and the anchors name, each sorted.
+    let ids = |s: &Session| {
+        let mut lock: Vec<u64> = s.map().label_lock.iter().flatten().map(|((_, f, ..), _)| *f).collect();
+        let mut anchors: Vec<u64> = s.map().label_lock_line_anchors.keys().map(|(_, f, ..)| *f).collect();
+        lock.sort_unstable();
+        anchors.sort_unstable();
+        (lock, anchors)
+    };
+    s.map_mut().layer_mut(a).unwrap().selection = vec![0];
+    s.run("edit.delete", &Value::Null).unwrap();
+    assert_eq!(ids(&s), (vec![2], vec![2]));
+    s.run("edit.undo", &Value::Null).unwrap();
+    assert_eq!(ids(&s), (vec![1, 2], vec![1, 2]));
+    s.run("edit.redo", &Value::Null).unwrap();
+    assert_eq!(ids(&s), (vec![2], vec![2]));
+}
+
 /// A streamed layer holds only the window in view: a lock's entry for a feature outside it stays
 /// when the window changes (#1135).
 #[test]
@@ -15613,6 +15647,56 @@ fn a_trend_over_a_stac_collection() {
     assert_eq!(s.map().layers.len(), layers);
 }
 
+/// A plugin's command in the app (`leave_downloads`) runs on the frame: a dated GeoTIFF's first
+/// file isn't waited for, and the answer says the map wasn't moved to it.
+#[cfg(not(target_arch = "wasm32"))]
+#[test]
+fn a_plugins_dated_cog_is_added_without_waiting() {
+    // A server that takes the connection and never answers: a probe would wait its minute.
+    let server = std::net::TcpListener::bind("127.0.0.1:0").unwrap();
+    let base = format!("http://{}", server.local_addr().unwrap());
+    let mut s = Session::new(Project::default());
+    let start = std::time::Instant::now();
+    let r =
+        s.run_leaving_downloads("layer.addUrl", &json!({"url": format!("{base}/{{date:YYYY}}.tif"), "start": 2020, "end": 2021})).unwrap();
+    assert!(start.elapsed() < std::time::Duration::from_secs(10), "waited {:?}", start.elapsed());
+    assert_eq!(r["inPlace"], true, "{r}");
+    assert!(r["warning"].as_str().is_some_and(|w| w.contains("wasn't moved")), "{r}");
+    drop(server);
+}
+
+/// A GeoTIFF whose header was just read is framed even if the cache forgot it since (a login
+/// ended), and one that can't be read again says why the map wasn't moved to it.
+#[cfg(not(target_arch = "wasm32"))]
+#[test]
+fn a_probed_cog_is_framed_after_the_cache_forgets_it() {
+    use crate::render::rustgis_tiles::{cache, cog};
+    let (base, _) = serve_ranges(rustgis_data::raster::cog::example(512, 128));
+    let url = format!("{base}/scene.tif");
+    assert_eq!(cog::probe_blocking(&url), Ok(true));
+    cache::forget(&url);
+    let mut s = Session::new(Project::default());
+    let r = crate::cmd::layer::add_cog(&mut s, &json!({}), &url, true).unwrap();
+    assert!(r["bounds"].is_array(), "{r}");
+    assert!(r.get("warning").is_none(), "{r}");
+    // Nothing answers at this address any more.
+    let gone = {
+        let server = std::net::TcpListener::bind("127.0.0.1:0").unwrap();
+        format!("http://{}/gone.tif", server.local_addr().unwrap())
+    };
+    let r = crate::cmd::layer::add_cog(&mut s, &json!({}), &gone, true).unwrap();
+    let warning = r["warning"].as_str().unwrap_or_default();
+    assert!(warning.contains("couldn't be read (") && warning.contains("wasn't moved"), "{r}");
+    // Not waited for, its failure (recorded, not yet tried again) is said too. Another test's
+    // login ending forgets the record, so it is made again for each try.
+    let said = (0..3).any(|_| {
+        let _ = crate::cmd::layer::add_cog(&mut s, &json!({}), &gone, true);
+        let r = crate::cmd::layer::add_cog(&mut s, &json!({}), &gone, false).unwrap();
+        r["warning"].as_str().is_some_and(|w| w.contains("couldn't be read (") && w.contains("wasn't moved"))
+    });
+    assert!(said, "a recorded failure went unsaid");
+}
+
 /// A cloud-optimised GeoTIFF by its address is read in place: its header, then only the tiles a
 /// map tile needs, by range; a GeoTIFF in strips is downloaded whole as a raster layer.
 #[test]

crates/tiles/src/cache.rs

@@ -140,6 +140,31 @@ pub fn clear_fetcher() {
     *fetcher().lock().unwrap_or_else(|e| e.into_inner()) = None;
 }
 
+/// Tests that fetch through the cache take turns. It has one fetcher for the whole process, so a
+/// test that replaces it ([`set_fetcher`]) would answer another's fetches. It also starts fetches
+/// on whichever thread is starting them already, so one test's fetch may run on another's thread,
+/// later. Tests that end a login take turns too, as that empties the cache ([`clear`]), and so do
+/// tests that wait for or plant what the cache holds (an archive read, a style resolved), which
+/// that would drop. The turn puts the native fetcher back when it ends.
+#[cfg(test)]
+pub(crate) fn turn() -> Turn {
+    static TURN: Mutex<()> = Mutex::new(());
+    Turn { _held: TURN.lock().unwrap_or_else(|e| e.into_inner()) }
+}
+
+/// A test's turn at the cache ([`turn`]).
+#[cfg(test)]
+pub(crate) struct Turn {
+    _held: std::sync::MutexGuard<'static, ()>,
+}
+
+#[cfg(test)]
+impl Drop for Turn {
+    fn drop(&mut self) {
+        clear_fetcher();
+    }
+}
+
 /// Replaces how tile pictures (PNG, JPEG, WebP) are decoded: the browser's own decoder works
 /// off the page's thread, where decoding them here held up every frame tiles arrived in. `done`
 /// must be called once, with the picture as [`RAW`] pixels, or with the bytes given back.
@@ -2396,6 +2421,7 @@ mod tests {
     #[cfg(not(target_arch = "wasm32"))]
     #[test]
     fn logins_headers_follow_same_origin_redirects() {
+        let _turn = crate::cache::turn();
         use crate::logins::{self, Method};
         let (base, asked) = serve_recorded(
             [
@@ -2418,6 +2444,7 @@ mod tests {
     #[cfg(not(target_arch = "wasm32"))]
     #[test]
     fn logins_redirects_never_pick_up_another_origins_login() {
+        let _turn = crate::cache::turn();
         use crate::logins::{self, Method};
         let (other, asked) = serve_recorded(vec![b"HTTP/1.1 200 OK\r\nContent-Length: 4\r\nConnection: close\r\n\r\ntile".to_vec()]);
         let base = serve(vec![format!("HTTP/1.1 302 Found\r\nLocation: {other}/tile\r\nContent-Length: 0\r\nConnection: close\r\n\r\n")]);
@@ -2437,6 +2464,7 @@ mod tests {
     #[cfg(not(target_arch = "wasm32"))]
     #[test]
     fn logins_diagnostics_never_record_the_binding_or_the_secret() {
+        let _turn = crate::cache::turn();
         use crate::failure::Reason;
         use crate::logins::{self, Method};
         use rustgis_data::diagnostics::{Filter, History};
@@ -2483,6 +2511,7 @@ mod tests {
     #[cfg(not(target_arch = "wasm32"))]
     #[test]
     fn delayed_head_refusal_after_cross_origin_redirect_is_status_only() {
+        let _turn = crate::cache::turn();
         use crate::logins::{self, Method};
         use rustgis_data::diagnostics::{Capture, Filter, History};
         use std::io::{Read, Write};
@@ -2546,6 +2575,7 @@ mod tests {
     #[cfg(not(target_arch = "wasm32"))]
     #[test]
     fn logins_head_and_post_carry_the_login() {
+        let _turn = crate::cache::turn();
         use crate::logins::{self, Method};
         let answer = b"HTTP/1.1 200 OK\r\nContent-Length: 0\r\nConnection: close\r\n\r\n".to_vec();
         let (base, asked) = serve_recorded(vec![answer.clone(), answer]);
@@ -2564,6 +2594,7 @@ mod tests {
     #[cfg(not(target_arch = "wasm32"))]
     #[test]
     fn live_connection_bindings_separate_requests_and_cache_entries() {
+        let _turn = crate::cache::turn();
         use crate::logins::{self, Method};
         let answer = b"HTTP/1.1 200 OK\r\nContent-Length: 2\r\nConnection: close\r\n\r\n{}".to_vec();
         let (base, asked) = serve_recorded(vec![answer.clone(), answer.clone(), answer]);
@@ -2595,6 +2626,7 @@ mod tests {
     #[cfg(not(target_arch = "wasm32"))]
     #[test]
     fn logout_during_pending_download_discards_old_authenticated_body() {
+        let _turn = crate::cache::turn();
         use crate::logins::{self, Method};
         use std::io::{Read, Write};
         let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap();
@@ -2634,6 +2666,7 @@ mod tests {
     #[cfg(not(target_arch = "wasm32"))]
     #[test]
     fn derived_style_tilejson_sprite_and_feature_requests_carry_the_binding() {
+        let _turn = crate::cache::turn();
         use crate::{
             logins::{self, Method},
             source,
@@ -2903,6 +2936,7 @@ mod tests {
 
     #[test]
     fn fetches_through_the_fetcher_once() {
+        let _turn = crate::cache::turn();
         static CALLS: AtomicUsize = AtomicUsize::new(0);
         set_fetcher(|url, done| {
             CALLS.fetch_add(1, Ordering::Relaxed);

crates/tiles/src/draw.rs

@@ -1285,6 +1285,7 @@ pub(crate) mod tests {
 
     #[test]
     fn picks_the_vector_tile_features_under_a_point() {
+        let _turn = crate::cache::turn();
         let tile = crate::mvt::encode(&crate::mvt::Tile {
             layers: vec![
                 mvt_layer("parks", GeomKind::Polygon, &[[1024.0, 1024.0], [3072.0, 1024.0], [3072.0, 3072.0], [1024.0, 3072.0]], "Central"),
@@ -1413,6 +1414,7 @@ pub(crate) mod tests {
 
     #[test]
     fn picks_a_feature_across_a_tile_edge() {
+        let _turn = crate::cache::turn();
         // Zoom 1: a point at the right edge of the western tile (1/0/0); the click is just over
         // the edge, in the eastern one (1/1/0).
         let tile = crate::mvt::encode(&crate::mvt::Tile { layers: vec![mvt_layer("pois", GeomKind::Point, &[[4094.0, 2048.0]], "Edge")] });

crates/tiles/src/logins.rs

@@ -417,6 +417,7 @@ mod tests {
 
     #[test]
     fn http_requires_permission_and_bindings_reject_scheme_host_and_port_changes() {
+        let _turn = crate::cache::turn();
         assert!(activate("http://insecure-login.test", "http", Method::Header("X-Key".into()), "short", false).is_err());
         let binding = activate("https://scoped-login.test", "scope", Method::Header("X-Key".into()), "scope-secret", false).unwrap();
         let bound = bind(&binding, "https://scoped-login.test/tile").unwrap();
@@ -432,6 +433,7 @@ mod tests {
 
     #[test]
     fn bindings_isolate_connections_and_revoke_generations() {
+        let _turn = crate::cache::turn();
         let target = "https://binding-login.example";
         let a = activate(target, "session-a/connection", Method::Header("X-Key".into()), "a", false).unwrap();
         let b = activate(target, "session-b/connection", Method::Header("X-Key".into()), "b", false).unwrap();
@@ -451,6 +453,7 @@ mod tests {
 
     #[test]
     fn failed_random_source_installs_nothing_and_preserves_a_live_login() {
+        let _turn = crate::cache::turn();
         let target = "https://random-failure-login.example";
         let old = activate(target, "random-replacement", Method::Header("X-Key".into()), "old-credential", false).unwrap();
         let address = bind(&old, &format!("{target}/tile")).unwrap();
@@ -473,6 +476,7 @@ mod tests {
 
     #[test]
     fn collisions_retry_without_overwriting_and_exhaust_after_four_attempts() {
+        let _turn = crate::cache::turn();
         let target = "https://collision-login.example";
         let method = || Method::Header("X-Key".into());
         let old = activate_with_random(target, "collision-old", method(), "old-key", false, |bytes| {
@@ -521,6 +525,7 @@ mod tests {
 
     #[test]
     fn scrubs_header_and_basic_payload() {
+        let _turn = crate::cache::turn();
         let value = basic("scrub-user", "scrub-password");
         let binding = activate("https://scrub-login.example", "scrub", Method::Header("Authorization".into()), &value, false).unwrap();
         let payload = value.strip_prefix("Basic ").unwrap();