From 84d954784eef70ab223813cabd728dbe85cdef31 Mon Sep 17 00:00:00 2001 From: mavrick Date: Tue, 3 Nov 2026 12:38:02 -0600 Subject: [PATCH 01/10] feat(data): export into an existing GeoPackage as an added or replaced table share.exportLayer and share.exportTable take an optional mode (newFile, the default, still refuses an existing file; addTable; replaceTable) and tableName. Into an existing GeoPackage the export is one SQLite transaction: a table added, or a feature or attribute table of that name replaced with its spatial index and GeoPackage records, the file's other tables untouched and a failed export leaving the file as it was. Export Features offers the choice for GeoPackage on the desktop; Layer.to_file takes mode= and table_name=. Co-Authored-By: Claude Opus 5.5 --- ROADMAP.md | 4 +- .../rustgis-python/python/rustgis/__init__.py | 18 +- crates/data/src/export.rs | 38 ++- crates/data/src/export/tests.rs | 48 +++ crates/data/src/io.rs | 47 ++- crates/data/src/source/gpkg.rs | 290 +++++++++++------- crates/data/src/source/gpkg/add.rs | 204 ++++++++++++ crates/data/src/source/tests.rs | 141 +++++++++ crates/engine/src/cmd/share.rs | 75 ++++- crates/engine/src/tests.rs | 58 ++++ crates/ui-egui/src/dialogs.rs | 45 ++- crates/ui-egui/src/lib.rs | 29 ++ docs/python.md | 10 +- docs/user-guide/data-formats.md | 2 +- docs/user-guide/sharing.md | 20 +- 15 files changed, 894 insertions(+), 135 deletions(-) create mode 100644 crates/data/src/source/gpkg/add.rs diff --git a/ROADMAP.md b/ROADMAP.md index 010b2ce52..89456d417 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -92,8 +92,6 @@ And alongside it: ([#853](https://github.com/opengeos/RustGIS/issues/853)). _Coordinate first_ (it changes `FieldType`). - Editable standalone tables (they are read-only today). -- Export into an existing GeoPackage as an added or replaced table, leaving its other tables - ([#852](https://github.com/opengeos/RustGIS/issues/852); export refuses an existing file today). - Polygon editing: shared edges, where moving one boundary moves its neighbour's too. Coincident polygon vertices can already move together with Shared Vertices (`edit.moveSharedVertex`); different vertex densities and linked insertion/deletion remain @@ -149,6 +147,8 @@ Open questions rather than plans; discuss before building. ## Done +- Export into an existing GeoPackage on desktop: a layer or table added as one more table, or + replacing one of its tables, leaving the others as they were (`share.exportLayer` `mode`). - Polygon editing: Cut Polygon, Reshape (polygons and lines) and Trace along an edge while drawing, on the map and through `edit.cutPolygon`, `edit.reshape` and `edit.trace`. - Geometry conversions: Polygon To Line, Split Line At Vertices, Feature Envelope To Polygon diff --git a/apps/rustgis-python/python/rustgis/__init__.py b/apps/rustgis-python/python/rustgis/__init__.py index 82c9dc163..38329661b 100644 --- a/apps/rustgis-python/python/rustgis/__init__.py +++ b/apps/rustgis-python/python/rustgis/__init__.py @@ -268,7 +268,14 @@ def to_geopandas(self, selected: bool = False) -> Any: collection = self.to_geojson(selected) return gpd.GeoDataFrame.from_features(collection["features"], crs="EPSG:4326") - def to_file(self, path: PathLike, crs: Optional[str] = None) -> Any: + def to_file( + self, + path: PathLike, + crs: Optional[str] = None, + *, + mode: Optional[str] = None, + table_name: Optional[str] = None, + ) -> Any: """Writes the layer's features to a file (``share.exportLayer``). Args: @@ -276,10 +283,19 @@ def to_file(self, path: PathLike, crs: Optional[str] = None) -> Any: ``.gpkg``, ``.parquet``, ``.shp``, ``.zip``, ``.kml``, ``.kmz`` or ``.gpx``. crs: The coordinate system of a GeoPackage, Shapefile or FlatGeobuf, e.g. ``"EPSG:3857"`` (the one the layer was read in when left out). + mode: For a GeoPackage: ``"newFile"`` (the default; an existing file is refused), + ``"addTable"`` (one more table in an existing GeoPackage, or a new file when + there is none) or ``"replaceTable"`` (replacing its table of that name, which + must be there). + table_name: The GeoPackage table (the layer's name when left out). """ params: Dict[str, Any] = {"layer": self.id, "path": os.fspath(path)} if crs is not None: params["crs"] = crs + if mode is not None: + params["mode"] = mode + if table_name is not None: + params["tableName"] = table_name return self.session.run("share.exportLayer", params) def __eq__(self, other: object) -> bool: diff --git a/crates/data/src/export.rs b/crates/data/src/export.rs index 467cfd18b..c9509f3e8 100644 --- a/crates/data/src/export.rs +++ b/crates/data/src/export.rs @@ -6,7 +6,9 @@ use std::path::Path; -use crate::io::{CsvWriter, GeoJsonWriter, PROJECTED_FORMATS, check_projected, project_features, write_atomic_with}; +use crate::io::{ + CsvWriter, GeoJsonWriter, GpkgTable, GpkgWritten, PROJECTED_FORMATS, check_projected, project_features, write_atomic_with, +}; use crate::raster::Crs; use crate::source::FeatureSource; use crate::{DataError, Dataset, Feature, GeometryType, Result}; @@ -43,8 +45,32 @@ pub fn write_source_in(src: &dyn FeatureSource, layer: &Dataset, path: &Path, cr write_chunks_in(src, layer, path, crs, CHUNK_BYTES) } +/// As [`write_source_in`], a GeoPackage's table placed as `gpkg` says (see +/// [`crate::io::write_path_to`]): also returns the table written. +pub fn write_source_to( + src: &dyn FeatureSource, + layer: &Dataset, + path: &Path, + crs: Crs, + gpkg: &GpkgTable, +) -> Result<(Exported, Option)> { + write_chunks_to(src, layer, path, crs, gpkg, CHUNK_BYTES) +} + /// [`write_source_in`], `bytes` of features a chunk. fn write_chunks_in(src: &dyn FeatureSource, layer: &Dataset, path: &Path, crs: Crs, bytes: usize) -> Result { + write_chunks_to(src, layer, path, crs, &GpkgTable::default(), bytes).map(|(e, _)| e) +} + +/// [`write_source_to`], `bytes` of features a chunk. +fn write_chunks_to( + src: &dyn FeatureSource, + layer: &Dataset, + path: &Path, + crs: Crs, + gpkg: &GpkgTable, + bytes: usize, +) -> Result<(Exported, Option)> { let ext = path.extension().and_then(|e| e.to_str()).unwrap_or("").to_ascii_lowercase(); if !streams(&ext) { return Err(DataError::Unsupported(format!(".{ext} written a chunk at a time"))); @@ -80,14 +106,18 @@ fn write_chunks_in(src: &dyn FeatureSource, layer: &Dataset, path: &Path, crs: C skipped = batch.skipped; Ok(()) }; + let mut table = None; let left_out = match ext.as_str() { #[cfg(not(target_arch = "wasm32"))] "gpkg" => { - crate::source::gpkg::write_features_in(&layer.name, geometry_type, &layer.fields, path, crs, &mut feed)?; + table = Some(crate::source::gpkg::write_features_in(&layer.name, geometry_type, &layer.fields, path, crs, gpkg, &mut feed)?); 0 } #[cfg(target_arch = "wasm32")] - "gpkg" => return Err(DataError::Unsupported("GeoPackage in the browser".into())), + "gpkg" => { + let _ = (gpkg, &mut table); + return Err(DataError::Unsupported("GeoPackage in the browser".into())); + } "geojson" | "json" => write_atomic_with(path, |out| { let mut w = GeoJsonWriter::new(out, &layer.name, &layer.fields)?; feed(&mut |features| w.write(features))?; @@ -108,7 +138,7 @@ fn write_chunks_in(src: &dyn FeatureSource, layer: &Dataset, path: &Path, crs: C Ok::<_, DataError>(0) })?, }; - Ok(Exported { written: total.saturating_sub(left_out), left_out, skipped }) + Ok((Exported { written: total.saturating_sub(left_out), left_out, skipped }, table)) } /// The kind of a source's first geometry, read up to it; `None` for a source without any (read diff --git a/crates/data/src/export/tests.rs b/crates/data/src/export/tests.rs index 91220bc08..06aac7f70 100644 --- a/crates/data/src/export/tests.rs +++ b/crates/data/src/export/tests.rs @@ -278,3 +278,51 @@ fn a_panicking_writer_leaves_nothing_behind() { assert!(r.is_err()); assert_eq!(std::fs::read_dir(&d).unwrap().count(), 0); } + +/// Streamed into an existing GeoPackage, an export that starts again (a multipoint, then a +/// repeated id) rolls back what it wrote and writes the table once, as a whole export would; one +/// whose source fails leaves the file's bytes as they were. +#[test] +fn a_streamed_export_into_an_existing_geopackage_starts_again_inside_its_transaction() { + let d = folder("redo-existing"); + let county = d.join("county.gpkg"); + let mut parcels = Dataset::new("parcels", GeometryType::Point, vec![]); + parcels.push_next(Some(Geometry::Point(Coord::new(5.0, 5.0))), vec![]); + crate::io::write_path(&parcels, &county).unwrap(); + let fields = vec![Field::new("n", FieldType::Integer)]; + let mut both = Dataset::new("both", GeometryType::Point, fields.clone()); + for i in 0..20 { + let c = Coord::new(i as f64, 1.0); + let g = if i == 15 { Geometry::MultiPoint(vec![c, Coord::new(i as f64, 2.0)]) } else { Geometry::Point(c) }; + both.push_next(Some(g), vec![Value::Int(i)]); + } + let first = both.features.first().map(|f| f.id).unwrap(); + if let Some(f) = both.features.get_mut(18) { + f.id = first; + } + let layer = Dataset::new("both", GeometryType::Point, fields); + let add = GpkgTable { mode: crate::io::GpkgMode::AddTable, name: None }; + let src = Runs::new(&both, 4); + let (e, table) = write_chunks_to(&src, &layer, &county, Crs::LonLat, &add, 1).unwrap(); + assert_eq!(src.passes.load(std::sync::atomic::Ordering::Relaxed), 3, "written three times"); + assert_eq!((e.written, table), (20, Some(GpkgWritten { table: "both".into(), replaced: false }))); + let whole = d.join("whole.gpkg"); + crate::io::write_path(&both, &whole).unwrap(); + let read = + |p: &Path, t: &str| crate::source::open_path(p, Some(t)).unwrap().read(&Request { extent: None, limit: 100 }).unwrap().features; + assert_eq!(read(&county, "both"), read(&whole, "both")); + assert_eq!(read(&county, "parcels").len(), 1); + let c = rusqlite::Connection::open(&county).unwrap(); + let kind: String = + c.query_row("SELECT geometry_type_name FROM gpkg_geometry_columns WHERE table_name = 'both'", [], |r| r.get(0)).unwrap(); + assert_eq!(kind, "MULTIPOINT"); + drop(c); + // A source failing on the second pass: nothing of the table is left. + let before = std::fs::read(&county).unwrap(); + let mut layer2 = layer.clone(); + layer2.name = "again".into(); + let mut failing = Runs::new(&both, 4); + (failing.fail_after, failing.fail_pass) = (Some(2), Some(1)); + assert!(write_chunks_to(&failing, &layer2, &county, Crs::LonLat, &add, 1).is_err()); + assert_eq!(std::fs::read(&county).unwrap(), before); +} diff --git a/crates/data/src/io.rs b/crates/data/src/io.rs index 809dbdd87..5c7d8604b 100644 --- a/crates/data/src/io.rs +++ b/crates/data/src/io.rs @@ -480,9 +480,43 @@ pub fn write_path(ds: &Dataset, path: &Path) -> Result { write_path_in(ds, path, Crs::LonLat) } +/// How a GeoPackage export treats a GeoPackage already at its path. +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] +pub enum GpkgMode { + /// Writes a new file; an existing one is refused, as it may hold other tables. + #[default] + NewFile, + /// Adds the table to the GeoPackage (a new file when there is none); refuses a table of its + /// name. + AddTable, + /// Replaces the GeoPackage's feature or attribute table of its name, which must be there. + ReplaceTable, +} + +/// Where a GeoPackage export puts its table: how, and under what name (default: the layer's). +#[derive(Clone, Debug, Default, PartialEq, Eq)] +pub struct GpkgTable { + pub mode: GpkgMode, + pub name: Option, +} + +/// The table a GeoPackage export wrote: its name in the file (a reserved name is prefixed), and +/// whether it replaced one. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct GpkgWritten { + pub table: String, + pub replaced: bool, +} + /// As [`write_path`], in the coordinate system `crs` (a GeoPackage, Shapefile or FlatGeobuf; see /// [`write_bytes_in`]). pub fn write_path_in(ds: &Dataset, path: &Path, crs: Crs) -> Result { + write_path_to(ds, path, crs, &GpkgTable::default()).map(|(left_out, _)| left_out) +} + +/// As [`write_path_in`], a GeoPackage's table placed as `gpkg` says (other formats ignore it): +/// also returns the table written. +pub fn write_path_to(ds: &Dataset, path: &Path, crs: Crs, gpkg: &GpkgTable) -> Result<(usize, Option)> { let ext = path.extension().and_then(|e| e.to_str()).unwrap_or("").to_ascii_lowercase(); let projected; let ds = if crs == Crs::LonLat || !PROJECTED_FORMATS.contains(&ext.as_str()) { @@ -496,11 +530,14 @@ pub fn write_path_in(ds: &Dataset, path: &Path, crs: Crs) -> Result { "gpkg" => { #[cfg(not(target_arch = "wasm32"))] { - crate::source::gpkg::write_in(ds, path, crs)?; - Ok(0) + let table = crate::source::gpkg::write_table_in(ds, path, crs, gpkg)?; + Ok((0, Some(table))) } #[cfg(target_arch = "wasm32")] - Err(DataError::Unsupported("GeoPackage in the browser".into())) + { + let _ = gpkg; + Err(DataError::Unsupported("GeoPackage in the browser".into())) + } } "shp" => { let (parts, left_out) = shapefile_parts_in(ds, crs)?; @@ -570,7 +607,7 @@ pub fn write_path_in(ds: &Dataset, path: &Path, crs: Crs) -> Result { for stale in ["qix", "sbn", "sbx"] { let _ = std::fs::remove_file(path.with_extension(stale)); } - Ok(left_out) + Ok((left_out, None)) } _ if crs != Crs::LonLat && !PROJECTED_FORMATS.contains(&ext.as_str()) => { Err(DataError::Unsupported(format!(".{ext} in {} (it holds longitude/latitude only)", crs.label()))) @@ -578,7 +615,7 @@ pub fn write_path_in(ds: &Dataset, path: &Path, crs: Crs) -> Result { _ => { let (bytes, left_out) = projected_bytes(ds, &ext, crs)?; write_atomic(path, &bytes)?; - Ok(left_out) + Ok((left_out, None)) } } } diff --git a/crates/data/src/source/gpkg.rs b/crates/data/src/source/gpkg.rs index 16e33c912..e393dc855 100644 --- a/crates/data/src/source/gpkg.rs +++ b/crates/data/src/source/gpkg.rs @@ -13,8 +13,11 @@ use rustgis_geom::Extent; use std::path::{Path, PathBuf}; use std::sync::Mutex; +mod add; pub mod edit; +pub use crate::io::{GpkgMode, GpkgTable, GpkgWritten}; + /// Rows fetched by id per query. const CHUNK: usize = 500; @@ -533,14 +536,38 @@ pub fn write(ds: &crate::Dataset, path: &Path) -> Result<()> { /// Writes a dataset whose geometry is in `crs`'s coordinates as a new GeoPackage in that system. /// Existing destinations are refused, as with [`write`]. pub fn write_in(ds: &crate::Dataset, path: &Path, crs: Crs) -> Result<()> { + write_table_in(ds, path, crs, &GpkgTable::default()).map(|_| ()) +} + +/// As [`write_in`], the table placed as `target` says: in a new file, or added to (or replacing +/// a table of) the GeoPackage at `path` when it exists, its other tables left as they were. +pub fn write_table_in(ds: &crate::Dataset, path: &Path, crs: Crs, target: &GpkgTable) -> Result { + let name = target.name.as_deref().unwrap_or(&ds.name); + if let Some(replace) = into_existing(path, target)? { + return add::write_features(path, name, ds.geometry_type, &ds.fields, crs, replace, &mut |put| put(&ds.features)); + } check_new_destination(path)?; let tmp = temp_database_beside(path)?; let conn = Connection::open(&tmp).map_err(err)?; - build(&conn, ds, crs)?; - install(conn, tmp, path) + let table = build(&conn, ds, name, crs)?; + install(conn, tmp, path)?; + Ok(GpkgWritten { table, replaced: false }) +} + +/// Whether an export goes into the existing GeoPackage at `path` (anything there, which opening +/// it as one then checks) and, if so, whether it replaces a table: `None` for a new file. A +/// table is added to a new file when there is none; one can't be replaced in a file not there. +fn into_existing(path: &Path, target: &GpkgTable) -> Result> { + let exists = std::fs::symlink_metadata(path).is_ok(); + match target.mode { + GpkgMode::NewFile => Ok(None), + GpkgMode::AddTable => Ok(exists.then_some(false)), + GpkgMode::ReplaceTable if exists => Ok(Some(true)), + GpkgMode::ReplaceTable => Err(DataError::Format(format!("{}: no GeoPackage there to replace a table in", path.display()))), + } } -/// As [`write_in`], for a layer `name` of `geometry_type` with `fields`: the features `feed` +/// As [`write_table_in`], for a layer `name` of `geometry_type` with `fields`: the features `feed` /// hands to the function it is given, a run at a time, already in `crs`'s coordinates, so a /// whole layer needn't be held. A point layer's table type and whether ids are kept are chosen /// from the features so far; should a later one contradict that choice, the export starts @@ -551,49 +578,74 @@ pub(crate) fn write_features_in( fields: &[Field], path: &Path, crs: Crs, + target: &GpkgTable, feed: &mut dyn FnMut(&mut dyn FnMut(&[crate::Feature]) -> Result<()>) -> Result<()>, -) -> Result<()> { +) -> Result { + let name = target.name.as_deref().unwrap_or(name); + if let Some(replace) = into_existing(path, target)? { + return add::write_features(path, name, geometry_type, fields, crs, replace, feed); + } check_new_destination(path)?; let (mut multi_points, mut keep_ids) = (None, true); // Each choice changes at most once. for _ in 0..3 { let tmp = temp_database_beside(path)?; let conn = Connection::open(&tmp).map_err(err)?; - let (mut layout, mut extent, mut redo) = (None, Extent::EMPTY, None); - let fed = feed(&mut |features| { - if layout.is_none() { - let multi = - multi_points.unwrap_or_else(|| features.iter().any(|f| matches!(f.geometry, Some(crate::Geometry::MultiPoint(_))))); - layout = Some(create(&conn, name, geometry_type, fields, crs, multi)?); - } - let Some(l) = layout.as_ref() else { return Err(DataError::Format("GeoPackage: no table to write".into())) }; - for g in features.iter().filter_map(|f| f.geometry.as_ref()) { - extent = extent.union(&g.extent()); - } - redo = insert(&conn, l, features, keep_ids)?; - match redo { - // Stops the traversal; the export starts again below. - Some(_) => Err(DataError::Format("GeoPackage: starting again".into())), - None => Ok(()), - } - }); - match (fed, redo) { - (_, Some(Redo::MultiPoints)) => multi_points = Some(true), - (_, Some(Redo::Renumber)) => keep_ids = false, - (Err(e), None) => return Err(e), - (Ok(()), None) => { - let layout = match layout { - Some(l) => l, - None => create(&conn, name, geometry_type, fields, crs, multi_points.unwrap_or(false))?, - }; + let mut make = |multi| create(&conn, name, geometry_type, fields, crs, multi); + match fill(&conn, multi_points, keep_ids, &mut make, feed)? { + Err(Redo::MultiPoints) => multi_points = Some(true), + Err(Redo::Renumber) => keep_ids = false, + Ok((layout, extent)) => { finish(&conn, &layout, extent)?; - return install(conn, tmp, path); + install(conn, tmp, path)?; + return Ok(GpkgWritten { table: layout.table, replaced: false }); } } } Err(DataError::Format("GeoPackage: the export didn't settle on a table".into())) } +/// Feeds the features into the table `make` creates on `conn`, made once the first run shows +/// whether a point layer holds multipoints (unless `multi_points` already says): the table and +/// the extent of what went in, or what the export must start again for. With `keep_ids` each +/// feature keeps its id. +fn fill( + conn: &Connection, + multi_points: Option, + keep_ids: bool, + make: &mut dyn FnMut(bool) -> Result, + feed: &mut dyn FnMut(&mut dyn FnMut(&[crate::Feature]) -> Result<()>) -> Result<()>, +) -> Result> { + let (mut layout, mut extent, mut redo) = (None, Extent::EMPTY, None); + let fed = feed(&mut |features| { + if layout.is_none() { + let multi = multi_points.unwrap_or_else(|| features.iter().any(|f| matches!(f.geometry, Some(crate::Geometry::MultiPoint(_))))); + layout = Some(make(multi)?); + } + let Some(l) = layout.as_ref() else { return Err(DataError::Format("GeoPackage: no table to write".into())) }; + for g in features.iter().filter_map(|f| f.geometry.as_ref()) { + extent = extent.union(&g.extent()); + } + redo = insert(conn, l, features, keep_ids)?; + match redo { + // Stops the traversal; the export starts again. + Some(_) => Err(DataError::Format("GeoPackage: starting again".into())), + None => Ok(()), + } + }); + match (fed, redo) { + (_, Some(r)) => Ok(Err(r)), + (Err(e), None) => Err(e), + (Ok(()), None) => { + let layout = match layout { + Some(l) => l, + None => make(multi_points.unwrap_or(false))?, + }; + Ok(Ok((layout, extent))) + } + } +} + /// A new temporary file beside `path`, removed unless [`install`]ed. fn temp_database_beside(path: &Path) -> Result { let parent = path.parent().filter(|p| !p.as_os_str().is_empty()).unwrap_or_else(|| Path::new(".")); @@ -650,10 +702,11 @@ fn destination_error(path: &Path, error: std::io::Error) -> DataError { } } -/// Builds a whole layer's GeoPackage on `conn`, its table chosen from all its features. -fn build(conn: &Connection, ds: &crate::Dataset, crs: Crs) -> Result<()> { +/// Builds a whole layer's GeoPackage on `conn`, its table `name` chosen from all its features; +/// returns the table's name as written. +fn build(conn: &Connection, ds: &crate::Dataset, name: &str, crs: Crs) -> Result { let multi_points = ds.features.iter().any(|f| matches!(f.geometry, Some(crate::Geometry::MultiPoint(_)))); - let layout = create(conn, &ds.name, ds.geometry_type, &ds.fields, crs, multi_points)?; + let layout = create(conn, name, ds.geometry_type, &ds.fields, crs, multi_points)?; // Feature ids are kept when they are unique and fit; otherwise SQLite numbers the rows. let mut seen = std::collections::HashSet::new(); let keep_ids = ds.features.iter().all(|f| i64::try_from(f.id).is_ok() && seen.insert(f.id)); @@ -662,7 +715,8 @@ fn build(conn: &Connection, ds: &crate::Dataset, crs: Crs) -> Result<()> { if insert(conn, &layout, &ds.features, keep_ids)?.is_some() { return Err(DataError::Format("GeoPackage: a feature doesn't fit the table chosen for the layer".into())); } - finish(conn, &layout, ds.extent()) + finish(conn, &layout, ds.extent())?; + Ok(layout.table) } /// A feature table being written: its names, and what its rows hold. @@ -682,6 +736,25 @@ struct Layout { rtree_q: String, } +/// The GeoPackage's own tables, as a new file has them. +const METADATA: &str = "CREATE TABLE gpkg_spatial_ref_sys (srs_name TEXT NOT NULL, srs_id INTEGER PRIMARY KEY, organization TEXT NOT NULL, + organization_coordsys_id INTEGER NOT NULL, definition TEXT NOT NULL, description TEXT); + CREATE TABLE gpkg_contents (table_name TEXT NOT NULL PRIMARY KEY, data_type TEXT NOT NULL, identifier TEXT UNIQUE, + description TEXT DEFAULT '', last_change DATETIME NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ','now')), + min_x DOUBLE, min_y DOUBLE, max_x DOUBLE, max_y DOUBLE, srs_id INTEGER, + CONSTRAINT fk_gc_r_srs_id FOREIGN KEY (srs_id) REFERENCES gpkg_spatial_ref_sys(srs_id));"; + +/// The tables a GeoPackage holding a feature table with a spatial index needs beyond +/// [`METADATA`], made where missing (an existing GeoPackage may have only attribute tables). +const FEATURE_METADATA: &str = "CREATE TABLE IF NOT EXISTS gpkg_geometry_columns (table_name TEXT NOT NULL, column_name TEXT NOT NULL, + geometry_type_name TEXT NOT NULL, srs_id INTEGER NOT NULL, z TINYINT NOT NULL, m TINYINT NOT NULL, + CONSTRAINT pk_geom_cols PRIMARY KEY (table_name, column_name), + CONSTRAINT uk_gc_table_name UNIQUE (table_name), + CONSTRAINT fk_gc_tn FOREIGN KEY (table_name) REFERENCES gpkg_contents(table_name), + CONSTRAINT fk_gc_srs FOREIGN KEY (srs_id) REFERENCES gpkg_spatial_ref_sys (srs_id)); + CREATE TABLE IF NOT EXISTS gpkg_extensions (table_name TEXT, column_name TEXT, extension_name TEXT NOT NULL, + definition TEXT NOT NULL, scope TEXT NOT NULL, CONSTRAINT ge_tce UNIQUE (table_name, column_name, extension_name));"; + /// Creates the GeoPackage's tables and the empty feature table (and its spatial index) for a /// layer, in a transaction [`finish`] commits. A point layer holding any multipoint is a /// MULTIPOINT table, whose single points [`insert`] writes as one-point multipoints. @@ -691,7 +764,40 @@ fn create(conn: &Connection, name: &str, geometry_type: GeometryType, fields: &[ 0 => 100_000, c => i32::from(c), }; - // Not a reserved name, and never empty. + let layout = plan(name, geometry_type, fields, multi_points, srs_id); + conn.execute_batch(&format!( + "PRAGMA journal_mode = OFF; PRAGMA synchronous = OFF; + PRAGMA application_id = 1196444487; PRAGMA user_version = 10400; + {METADATA} {FEATURE_METADATA} BEGIN;" + )) + .map_err(err)?; + create_table(conn, &layout)?; + conn.execute( + "INSERT INTO gpkg_spatial_ref_sys VALUES + ('Undefined cartesian SRS', -1, 'NONE', -1, 'undefined', 'undefined cartesian coordinate reference system'), + ('Undefined geographic SRS', 0, 'NONE', 0, 'undefined', 'undefined geographic coordinate reference system'), + ('WGS 84 geodetic', 4326, 'EPSG', 4326, ?1, 'longitude/latitude coordinates in decimal degrees on the WGS 84 spheroid')", + [WGS84_WKT], + ) + .map_err(err)?; + if srs_id != 4326 { + // GeoPackage readers take an EPSG system by its code; a custom one has only its WKT + // (`undefined` when it has none). + let org = if crs.epsg() > 0 { "EPSG" } else { "NONE" }; + let wkt = crate::raster::wkt::to_wkt(crs); + conn.execute( + "INSERT INTO gpkg_spatial_ref_sys VALUES (?1, ?2, ?3, ?2, ?4, '')", + rusqlite::params![crs.label(), srs_id, org, wkt.as_deref().unwrap_or("undefined")], + ) + .map_err(err)?; + } + Ok(layout) +} + +/// The names a layer's feature table is written with, in spatial reference system `srs_id`: the +/// table named for the layer (not a reserved name, never empty), its id and geometry columns +/// clear of its fields, and its fields' columns unique without regard to case. +fn plan(name: &str, geometry_type: GeometryType, fields: &[Field], multi_points: bool, srs_id: i32) -> Layout { let mut table: String = name.chars().filter(|c| !c.is_control()).collect::().trim().to_string(); if table.is_empty() { table = "layer".into(); @@ -725,65 +831,8 @@ fn create(conn: &Connection, name: &str, geometry_type: GeometryType, fields: &[ GeometryType::None => "", }; let (t, g, i) = (quote(&table), quote(&geom), quote(&fid)); - let mut sql = String::from( - "PRAGMA journal_mode = OFF; PRAGMA synchronous = OFF; - PRAGMA application_id = 1196444487; PRAGMA user_version = 10400; - CREATE TABLE gpkg_spatial_ref_sys (srs_name TEXT NOT NULL, srs_id INTEGER PRIMARY KEY, organization TEXT NOT NULL, - organization_coordsys_id INTEGER NOT NULL, definition TEXT NOT NULL, description TEXT); - CREATE TABLE gpkg_contents (table_name TEXT NOT NULL PRIMARY KEY, data_type TEXT NOT NULL, identifier TEXT UNIQUE, - description TEXT DEFAULT '', last_change DATETIME NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ','now')), - min_x DOUBLE, min_y DOUBLE, max_x DOUBLE, max_y DOUBLE, srs_id INTEGER, - CONSTRAINT fk_gc_r_srs_id FOREIGN KEY (srs_id) REFERENCES gpkg_spatial_ref_sys(srs_id)); - CREATE TABLE gpkg_geometry_columns (table_name TEXT NOT NULL, column_name TEXT NOT NULL, geometry_type_name TEXT NOT NULL, - srs_id INTEGER NOT NULL, z TINYINT NOT NULL, m TINYINT NOT NULL, - CONSTRAINT pk_geom_cols PRIMARY KEY (table_name, column_name), - CONSTRAINT uk_gc_table_name UNIQUE (table_name), - CONSTRAINT fk_gc_tn FOREIGN KEY (table_name) REFERENCES gpkg_contents(table_name), - CONSTRAINT fk_gc_srs FOREIGN KEY (srs_id) REFERENCES gpkg_spatial_ref_sys (srs_id)); - CREATE TABLE gpkg_extensions (table_name TEXT, column_name TEXT, extension_name TEXT NOT NULL, definition TEXT NOT NULL, - scope TEXT NOT NULL, CONSTRAINT ge_tce UNIQUE (table_name, column_name, extension_name)); - BEGIN;", - ); - let mut cols = vec![format!("{i} INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL")]; - if has_geom { - cols.push(format!("{g} {geom_type}")); - } - for (f, col) in fields.iter().zip(&columns) { - let ty = match f.ty { - FieldType::Integer => "INTEGER", - FieldType::Double => "REAL", - FieldType::Boolean => "BOOLEAN", - FieldType::Text => "TEXT", - }; - cols.push(format!("{} {ty}", quote(col))); - } - sql.push_str(&format!("CREATE TABLE {t} ({});", cols.join(", "))); let rt = quote(&format!("rtree_{table}_{geom}")); - if has_geom { - sql.push_str(&format!("CREATE VIRTUAL TABLE {rt} USING rtree(id, minx, maxx, miny, maxy);")); - } - conn.execute_batch(&sql).map_err(err)?; - conn.execute( - "INSERT INTO gpkg_spatial_ref_sys VALUES - ('Undefined cartesian SRS', -1, 'NONE', -1, 'undefined', 'undefined cartesian coordinate reference system'), - ('Undefined geographic SRS', 0, 'NONE', 0, 'undefined', 'undefined geographic coordinate reference system'), - ('WGS 84 geodetic', 4326, 'EPSG', 4326, ?1, 'longitude/latitude coordinates in decimal degrees on the WGS 84 spheroid')", - [WGS84_WKT], - ) - .map_err(err)?; - if srs_id != 4326 { - // GeoPackage readers take an EPSG system by its code; a custom one has only its WKT - // (`undefined` when it has none). - let org = if crs.epsg() > 0 { "EPSG" } else { "NONE" }; - let wkt = crate::raster::wkt::to_wkt(crs); - conn.execute( - "INSERT INTO gpkg_spatial_ref_sys VALUES (?1, ?2, ?3, ?2, ?4, '')", - rusqlite::params![crs.label(), srs_id, org, wkt.as_deref().unwrap_or("undefined")], - ) - .map_err(err)?; - } - - Ok(Layout { + Layout { table, geom, columns, @@ -796,7 +845,30 @@ fn create(conn: &Connection, name: &str, geometry_type: GeometryType, fields: &[ geom_q: g, fid_q: i, rtree_q: rt, - }) + } +} + +/// Creates the empty feature table [`plan`] named, and its spatial index. +fn create_table(conn: &Connection, layout: &Layout) -> Result<()> { + let Layout { columns, fields, has_geom, geom_type, table_q: t, geom_q: g, fid_q: i, rtree_q: rt, .. } = layout; + let mut cols = vec![format!("{i} INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL")]; + if *has_geom { + cols.push(format!("{g} {geom_type}")); + } + for (f, col) in fields.iter().zip(columns) { + let ty = match f.ty { + FieldType::Integer => "INTEGER", + FieldType::Double => "REAL", + FieldType::Boolean => "BOOLEAN", + FieldType::Text => "TEXT", + }; + cols.push(format!("{} {ty}", quote(col))); + } + let mut sql = format!("CREATE TABLE {t} ({});", cols.join(", ")); + if *has_geom { + sql.push_str(&format!("CREATE VIRTUAL TABLE {rt} USING rtree(id, minx, maxx, miny, maxy);")); + } + conn.execute_batch(&sql).map_err(err) } /// What a streamed export found it must start again for, having chosen from the features so far. @@ -860,22 +932,34 @@ fn insert(conn: &Connection, layout: &Layout, features: &[crate::Feature], keep_ Ok(None) } -/// Records the table in the GeoPackage's contents with its `extent`, adds the triggers that keep -/// its spatial index in step with edits, and commits. +/// [`register`]s the table and commits. fn finish(conn: &Connection, layout: &Layout, extent: Extent) -> Result<()> { + register(conn, layout, extent)?; + conn.execute_batch("COMMIT;").map_err(err) +} + +/// Records the table in the GeoPackage's contents with its `extent` (named by its table name +/// unless another table already has that identifier) and adds the triggers that keep its +/// spatial index in step with edits. +fn register(conn: &Connection, layout: &Layout, extent: Extent) -> Result<()> { let Layout { table, geom, has_geom, geom_type, srs_id, table_q: t, geom_q: g, fid_q: i, rtree_q: rt, .. } = layout; let (has_geom, srs_id) = (*has_geom, *srs_id); let e = extent; let bounds: [Option; 4] = if e.is_empty() { [None; 4] } else { [Some(e.min_x), Some(e.min_y), Some(e.max_x), Some(e.max_y)] }; let srs: Option = has_geom.then_some(i64::from(srs_id)); conn.execute( - "INSERT INTO gpkg_contents (table_name, data_type, identifier, min_x, min_y, max_x, max_y, srs_id) VALUES (?1, ?2, ?1, ?3, ?4, ?5, ?6, ?7)", + "INSERT INTO gpkg_contents (table_name, data_type, identifier, last_change, min_x, min_y, max_x, max_y, srs_id) + VALUES (?1, ?2, CASE WHEN EXISTS (SELECT 1 FROM gpkg_contents WHERE identifier = ?1) THEN NULL ELSE ?1 END, + strftime('%Y-%m-%dT%H:%M:%fZ', 'now'), ?3, ?4, ?5, ?6, ?7)", rusqlite::params![table, if has_geom { "features" } else { "attributes" }, bounds[0], bounds[1], bounds[2], bounds[3], srs], ) .map_err(err)?; if has_geom { - conn.execute("INSERT INTO gpkg_geometry_columns VALUES (?1, ?2, ?3, ?4, 0, 0)", rusqlite::params![table, geom, geom_type, srs_id]) - .map_err(err)?; + conn.execute( + "INSERT INTO gpkg_geometry_columns (table_name, column_name, geometry_type_name, srs_id, z, m) VALUES (?1, ?2, ?3, ?4, 0, 0)", + rusqlite::params![table, geom, geom_type, srs_id], + ) + .map_err(err)?; // The GeoPackage 1.4 triggers that keep the index in step with edits. let trig = |s: &str| quote(&format!("rtree_{table}_{geom}_{s}")); conn.execute_batch(&format!( @@ -907,12 +991,12 @@ fn finish(conn: &Connection, layout: &Layout, extent: Extent) -> Result<()> { )) .map_err(err)?; conn.execute( - "INSERT INTO gpkg_extensions VALUES (?1, ?2, 'gpkg_rtree_index', 'http://www.geopackage.org/spec120/#extension_rtree', 'write-only')", + "INSERT INTO gpkg_extensions (table_name, column_name, extension_name, definition, scope) + VALUES (?1, ?2, 'gpkg_rtree_index', 'http://www.geopackage.org/spec120/#extension_rtree', 'write-only')", rusqlite::params![table, geom], ) .map_err(err)?; } - conn.execute_batch("COMMIT;").map_err(err)?; Ok(()) } diff --git a/crates/data/src/source/gpkg/add.rs b/crates/data/src/source/gpkg/add.rs new file mode 100644 index 000000000..09d5aa6e3 --- /dev/null +++ b/crates/data/src/source/gpkg/add.rs @@ -0,0 +1,204 @@ +//! Exports into an existing GeoPackage: one table added, or one of its feature or attribute +//! tables replaced, in a single transaction that leaves the file's other tables as they were. + +use super::{FEATURE_METADATA, GpkgWritten, Redo, WGS84_WKT, create_table, err, fill, plan, quote, register}; +use crate::raster::Crs; +use crate::{DataError, Field, GeometryType, Result}; +use rusqlite::config::DbConfig; +use rusqlite::{Connection, OpenFlags, OptionalExtension, TransactionBehavior}; +use std::cell::Cell; +use std::path::Path; +use std::time::Duration; + +/// Writes the features `feed` hands over as table `name` of the GeoPackage at `path` (see +/// [`super::write_features_in`]). A table of that name is refused, or, with `replace`, required: +/// a feature or attribute table, dropped first; nothing else in the file is touched. Every +/// change is one transaction, so a failed export leaves the file as it was. +pub(super) fn write_features( + path: &Path, + name: &str, + geometry_type: GeometryType, + fields: &[Field], + crs: Crs, + replace: bool, + feed: &mut dyn FnMut(&mut dyn FnMut(&[crate::Feature]) -> Result<()>) -> Result<()>, +) -> Result { + let file = path.file_name().unwrap_or_default().to_string_lossy().into_owned(); + // Never made here: the file must exist, as checked before. + let mut conn = Connection::open_with_flags(path, OpenFlags::SQLITE_OPEN_READ_WRITE | OpenFlags::SQLITE_OPEN_NO_MUTEX).map_err(err)?; + // The file may be hostile: what it defines (triggers, views) may not call functions, nor write + // to SQLite's own records. + conn.set_db_config(DbConfig::SQLITE_DBCONFIG_DEFENSIVE, true).map_err(err)?; + conn.set_db_config(DbConfig::SQLITE_DBCONFIG_TRUSTED_SCHEMA, false).map_err(err)?; + conn.busy_timeout(Duration::from_secs(5)).map_err(err)?; + let (mut multi_points, mut keep_ids) = (None, true); + // Each choice changes at most once; an export starting again rolls back what it wrote. + for _ in 0..3 { + let tx = conn.transaction_with_behavior(TransactionBehavior::Immediate).map_err(err)?; + check_geopackage(&tx, &file)?; + // An attribute table has no spatial reference system. + let srs_id = if geometry_type == GeometryType::None { 0 } else { srs_id(&tx, crs)? }; + let replaced = Cell::new(false); + let mut make = |multi| { + let layout = plan(name, geometry_type, fields, multi, srs_id); + replaced.set(make_room(&tx, &file, &layout.table, replace)?); + if layout.has_geom { + tx.execute_batch(FEATURE_METADATA).map_err(err)?; + let index = format!("rtree_{}_{}", layout.table, layout.geom); + if let Some(kind) = object(&tx, &index)? { + return Err(DataError::Format(format!( + "{file} already has {} named {index}, which the table's spatial index needs; choose another table name", + with_article(&kind) + ))); + } + } + create_table(&tx, &layout)?; + Ok(layout) + }; + match fill(&tx, multi_points, keep_ids, &mut make, feed)? { + Err(Redo::MultiPoints) => multi_points = Some(true), + Err(Redo::Renumber) => keep_ids = false, + Ok((layout, extent)) => { + register(&tx, &layout, extent)?; + tx.commit().map_err(err)?; + return Ok(GpkgWritten { table: layout.table, replaced: replaced.get() }); + } + } + } + Err(DataError::Format("GeoPackage: the export didn't settle on a table".into())) +} + +/// Refuses a database without a GeoPackage's contents and spatial reference systems. +fn check_geopackage(conn: &Connection, file: &str) -> Result<()> { + for table in ["gpkg_contents", "gpkg_spatial_ref_sys"] { + if object(conn, table)?.as_deref() != Some("table") { + return Err(DataError::Format(format!("{file}: not a GeoPackage (it has no {table} table)"))); + } + } + Ok(()) +} + +/// What kind of SQLite object (`table`, `view`, `index`, `trigger`) is named `name`, without +/// regard to case as SQLite names are. +fn object(conn: &Connection, name: &str) -> Result> { + conn.query_row("SELECT type FROM sqlite_master WHERE lower(name) = lower(?1) LIMIT 1", [name], |r| r.get(0)).optional().map_err(err) +} + +/// The GeoPackage's id for `crs`: an entry it has for the EPSG code, or with the same definition +/// for a system without one; else one added, numbered by its EPSG code where that is free and +/// otherwise from 100000 up, as a new file's custom system is. +fn srs_id(conn: &Connection, crs: Crs) -> Result { + let epsg = crs.epsg(); + let wkt = if epsg == 4326 { Some(WGS84_WKT.to_string()) } else { crate::raster::wkt::to_wkt(crs) }; + let found: Option = if epsg > 0 { + conn.query_row( + "SELECT srs_id FROM gpkg_spatial_ref_sys WHERE upper(organization) = 'EPSG' AND organization_coordsys_id = ?1 ORDER BY srs_id LIMIT 1", + [i64::from(epsg)], + |r| r.get(0), + ) + .optional() + .map_err(err)? + } else { + // A system without a WKT form is found by its name. + let (sql, key) = match &wkt { + Some(wkt) => ("SELECT srs_id FROM gpkg_spatial_ref_sys WHERE definition = ?1 ORDER BY srs_id LIMIT 1", wkt.clone()), + None => ( + "SELECT srs_id FROM gpkg_spatial_ref_sys WHERE srs_name = ?1 AND definition = 'undefined' AND srs_id >= 100000 ORDER BY srs_id LIMIT 1", + crs.label(), + ), + }; + conn.query_row(sql, [key], |r| r.get(0)).optional().map_err(err)? + }; + let id = match found { + Some(id) => id, + None => { + let preferred = if epsg > 0 { i64::from(epsg) } else { 100_000 }; + let taken: bool = conn + .query_row("SELECT EXISTS (SELECT 1 FROM gpkg_spatial_ref_sys WHERE srs_id = ?1)", [preferred], |r| r.get(0)) + .map_err(err)?; + let id = if taken { + conn.query_row("SELECT max(100000, max(srs_id) + 1) FROM gpkg_spatial_ref_sys", [], |r| r.get(0)).map_err(err)? + } else { + preferred + }; + let (org, code) = if epsg > 0 { ("EPSG", i64::from(epsg)) } else { ("NONE", id) }; + conn.execute( + "INSERT INTO gpkg_spatial_ref_sys (srs_name, srs_id, organization, organization_coordsys_id, definition, description) + VALUES (?1, ?2, ?3, ?4, ?5, '')", + rusqlite::params![crs.label(), id, org, code, wkt.as_deref().unwrap_or("undefined")], + ) + .map_err(err)?; + id + } + }; + i32::try_from(id).map_err(|_| DataError::Format(format!("GeoPackage: spatial reference system id {id} is out of range"))) +} + +/// Clears the way for table `table`: nothing of that name, or (with `replace`, which needs one) a +/// feature or attribute table of it, dropped with its spatial index and its GeoPackage records. +/// Returns whether a table was replaced; anything else of the name is refused. +fn make_room(conn: &Connection, file: &str, table: &str, replace: bool) -> Result { + let registered: Option<(String, String)> = conn + .query_row("SELECT table_name, data_type FROM gpkg_contents WHERE lower(table_name) = lower(?1)", [table], |r| { + Ok((r.get(0)?, r.get(1)?)) + }) + .optional() + .map_err(err)?; + let kind = object(conn, table)?; + match (kind.as_deref(), registered) { + // Replacing a table not there is most likely a misspelt name: the stale table would stay. + (None, None) if replace => { + Err(DataError::Format(format!("{file} has no table {table} to replace; check the name, or add the layer as a new table"))) + } + (None, None) => Ok(false), + (None, Some((old, _))) => { + Err(DataError::Format(format!("{file} lists a table {old} in its contents that it doesn't have; choose another table name"))) + } + (Some("table"), Some((old, data_type))) if matches!(data_type.as_str(), "features" | "attributes") => { + if !replace { + return Err(DataError::Format(format!( + "{file} already has a table {old}: choose another table name, or replace that table" + ))); + } + drop_table(conn, &old)?; + Ok(true) + } + (_, Some((old, data_type))) => Err(DataError::Format(format!( + "{file}: {old} is a {data_type} table, which an export doesn't replace; choose another table name" + ))), + (Some(kind), None) => Err(DataError::Format(format!( + "{file} already has {} named {table}, which an export doesn't replace; choose another table name", + with_article(kind) + ))), + } +} + +/// An SQLite object's kind with its article: "an index", "a view". +fn with_article(kind: &str) -> String { + if kind.starts_with(['a', 'e', 'i', 'o', 'u']) { format!("an {kind}") } else { format!("a {kind}") } +} + +/// Drops feature or attribute table `table`, its spatial indexes, and its rows in the GeoPackage +/// tables that describe it (the optional ones where the file has them). +fn drop_table(conn: &Connection, table: &str) -> Result<()> { + let has = |t: &str| object(conn, t).map(|k| k.as_deref() == Some("table")); + let mut sql = String::new(); + if has("gpkg_geometry_columns")? { + let mut st = conn.prepare("SELECT column_name FROM gpkg_geometry_columns WHERE lower(table_name) = lower(?1)").map_err(err)?; + let columns = st.query_map([table], |r| r.get::<_, String>(0)).map_err(err)?.collect::>>().map_err(err)?; + for c in columns { + sql.push_str(&format!("DROP TABLE IF EXISTS {};", quote(&format!("rtree_{table}_{c}")))); + } + } + sql.push_str(&format!("DROP TABLE {};", quote(table))); + conn.execute_batch(&sql).map_err(err)?; + // `gpkg_ogr_contents` is the feature count GDAL keeps, which would otherwise outlive the table. + for records in + ["gpkg_geometry_columns", "gpkg_extensions", "gpkg_data_columns", "gpkg_metadata_reference", "gpkg_ogr_contents", "gpkg_contents"] + { + if has(records)? { + conn.execute(&format!("DELETE FROM {records} WHERE lower(table_name) = lower(?1)"), [table]).map_err(err)?; + } + } + Ok(()) +} diff --git a/crates/data/src/source/tests.rs b/crates/data/src/source/tests.rs index d0496da6b..9443b1e79 100644 --- a/crates/data/src/source/tests.rs +++ b/crates/data/src/source/tests.rs @@ -2231,3 +2231,144 @@ fn flatgeobuf_binary_columns_are_said_and_left_out() { let src = open_path(&clean, None).unwrap(); assert!(src.info().notes.is_empty(), "{:?}", src.info().notes); } + +/// The tables of a GeoPackage, by name. +fn gpkg_tables(path: &Path) -> Vec { + let c = rusqlite::Connection::open(path).unwrap(); + let mut st = c.prepare("SELECT table_name FROM gpkg_contents ORDER BY table_name").unwrap(); + st.query_map([], |r| r.get(0)).unwrap().collect::>().unwrap() +} + +/// An export into an existing GeoPackage adds a table beside the others, refuses one whose name +/// is taken, and replaces a feature or attribute table (its spatial index too) when asked; the +/// other tables, an attribute table among them, are as they were. +#[test] +fn geopackage_export_adds_and_replaces_tables_and_keeps_the_others() { + use crate::io::{GpkgMode, GpkgTable, GpkgWritten, write_path_to}; + let workspace = tempfile::tempdir().unwrap(); + let path = workspace.path().join("county.gpkg"); + let mut parcels = Dataset::new("parcels", GeometryType::Point, vec![Field::new("apn", FieldType::Text)]); + parcels.push(Some(Geometry::Point(Coord::new(1.0, 2.0))), vec![Value::Text("001".into())]); + crate::io::write_path(&parcels, &path).unwrap(); + // An attribute table, and the feature counts GDAL keeps. + rusqlite::Connection::open(&path) + .unwrap() + .execute_batch( + "CREATE TABLE owners (name TEXT); INSERT INTO owners VALUES ('Shelby'); + INSERT INTO gpkg_contents (table_name, data_type, identifier) VALUES ('owners', 'attributes', 'owners'); + CREATE TABLE gpkg_ogr_contents (table_name TEXT NOT NULL PRIMARY KEY, feature_count INTEGER DEFAULT NULL); + INSERT INTO gpkg_ogr_contents VALUES ('parcels', 1);", + ) + .unwrap(); + let mut permits = Dataset::new("permits", GeometryType::Point, vec![Field::new("n", FieldType::Integer)]); + for i in 0..3 { + permits.push(Some(Geometry::Point(Coord::new(10.0 + i as f64, 20.0))), vec![Value::Int(i)]); + } + let to = |mode, name: Option<&str>| GpkgTable { mode, name: name.map(String::from) }; + let written = write_path_to(&permits, &path, Crs::LonLat, &to(GpkgMode::AddTable, None)).unwrap(); + assert_eq!(written, (0, Some(GpkgWritten { table: "permits".into(), replaced: false }))); + rusqlite::Connection::open(&path).unwrap().execute_batch("INSERT INTO gpkg_ogr_contents VALUES ('permits', 3)").unwrap(); + assert_eq!(gpkg_tables(&path), ["owners", "parcels", "permits"]); + let read = |t: &str, extent| open_path(&path, Some(t)).unwrap().read(&Request { extent, limit: 100 }).unwrap().features; + assert_eq!(read("permits", None).len(), 3); + // Its spatial index answers an extent query. + assert_eq!(read("permits", Some(Extent::new(10.5, 19.0, 11.5, 21.0))).len(), 1); + assert_eq!(read("parcels", None)[0].attrs, [Value::Text("001".into())]); + + // A name taken (whatever its case) is refused unless replacing, and the file is unchanged. + let before = std::fs::read(&path).unwrap(); + for name in [None, Some("PERMITS"), Some("owners_idx")] { + if name == Some("owners_idx") { + rusqlite::Connection::open(&path).unwrap().execute_batch("CREATE INDEX owners_idx ON owners (name)").unwrap(); + } + let e = write_path_to(&permits, &path, Crs::LonLat, &to(GpkgMode::AddTable, name)).unwrap_err().to_string(); + assert!(e.contains("already has"), "{name:?}: {e}"); + } + let before_index = std::fs::read(&path).unwrap(); + assert_ne!(before, before_index, "the index was added"); + + // Replacing: fewer features, in another system, and its old spatial index gone. + let mut fewer = permits.clone(); + fewer.features.truncate(1); + let replaced = write_path_to(&fewer, &path, Crs::WebMercator, &to(GpkgMode::ReplaceTable, None)).unwrap(); + assert_eq!(replaced.1, Some(GpkgWritten { table: "permits".into(), replaced: true })); + assert_eq!(gpkg_tables(&path), ["owners", "parcels", "permits"]); + let s = open_path(&path, Some("permits")).unwrap(); + assert_eq!(s.info().count, Some(1)); + assert_eq!(s.info().coordinate_system(), Some(Crs::WebMercator)); + let c = rusqlite::Connection::open(&path).unwrap(); + let count = |sql: &str| c.query_row(sql, [], |r| r.get::<_, i64>(0)).unwrap(); + assert_eq!(count("SELECT count(*) FROM rtree_permits_geom"), 1); + assert_eq!(count("SELECT count(*) FROM gpkg_geometry_columns WHERE table_name = 'permits'"), 1); + assert_eq!(count("SELECT count(*) FROM gpkg_extensions WHERE table_name = 'permits'"), 1); + assert_eq!(count("SELECT count(*) FROM gpkg_spatial_ref_sys WHERE srs_id = 3857"), 1); + // The old table's cached count is gone with it; the others' are kept. + assert_eq!(count("SELECT count(*) FROM gpkg_ogr_contents WHERE table_name = 'permits'"), 0); + assert_eq!(count("SELECT count(*) FROM gpkg_ogr_contents WHERE table_name = 'parcels'"), 1); + assert_eq!(c.query_row("SELECT name FROM owners", [], |r| r.get::<_, String>(0)).unwrap(), "Shelby"); + drop(c); + // A second table in that system reuses its entry. + let more = write_path_to(&fewer, &path, Crs::WebMercator, &to(GpkgMode::AddTable, Some("permits_2027"))).unwrap(); + assert_eq!(more.1, Some(GpkgWritten { table: "permits_2027".into(), replaced: false })); + // Replacing a table not there (a misspelt name) is refused rather than adding one. + let e = write_path_to(&fewer, &path, Crs::LonLat, &to(GpkgMode::ReplaceTable, Some("permit"))).unwrap_err().to_string(); + assert!(e.contains("no table permit to replace"), "{e}"); + let c = rusqlite::Connection::open(&path).unwrap(); + assert_eq!(c.query_row("SELECT count(*) FROM gpkg_spatial_ref_sys WHERE srs_id = 3857", [], |r| r.get::<_, i64>(0)).unwrap(), 1); + // A table without geometry replaces an attribute table, adding no spatial reference system. + let systems = || { + let c = rusqlite::Connection::open(&path).unwrap(); + c.query_row("SELECT count(*) FROM gpkg_spatial_ref_sys", [], |r| r.get::<_, i64>(0)).unwrap() + }; + let before_systems = systems(); + let mut owners = Dataset::new("owners", GeometryType::None, vec![Field::new("name", FieldType::Text)]); + owners.push(None, vec![Value::Text("Memphis".into())]); + owners.push(None, vec![Value::Text("Germantown".into())]); + drop(c); + write_path_to(&owners, &path, Crs::from_epsg(32616).unwrap(), &to(GpkgMode::ReplaceTable, None)).unwrap(); + assert_eq!(systems(), before_systems); + let c = rusqlite::Connection::open(&path).unwrap(); + assert_eq!(c.query_row("SELECT count(*) FROM owners", [], |r| r.get::<_, i64>(0)).unwrap(), 2); + drop(c); + assert_eq!(read("parcels", None).len(), 1, "untouched throughout"); +} + +/// An export into an existing GeoPackage never replaces what isn't a feature or attribute table +/// (a tile table, a view, an unregistered table) or a file that isn't there, and refuses a file +/// that isn't a GeoPackage; adding a table where there is no file makes a new one. +#[test] +fn geopackage_export_into_a_file_refuses_what_it_cannot_replace() { + use crate::io::{GpkgMode, GpkgTable, write_path_to}; + let workspace = tempfile::tempdir().unwrap(); + let path = workspace.path().join("county.gpkg"); + let replace = |name: &str| GpkgTable { mode: GpkgMode::ReplaceTable, name: Some(name.into()) }; + let mut ds = Dataset::new("permits", GeometryType::Point, vec![]); + ds.push(Some(Geometry::Point(Coord::new(1.0, 2.0))), vec![]); + // No file: nothing to replace in, but a table can be added to a new GeoPackage. + assert!(write_path_to(&ds, &path, Crs::LonLat, &replace("permits")).unwrap_err().to_string().contains("no GeoPackage there")); + assert!(!path.exists()); + let add = GpkgTable { mode: GpkgMode::AddTable, name: None }; + assert!(write_path_to(&ds, &path, Crs::LonLat, &add).unwrap().1.is_some_and(|t| !t.replaced)); + rusqlite::Connection::open(&path) + .unwrap() + .execute_batch( + "CREATE TABLE ortho (id INTEGER PRIMARY KEY, tile_data BLOB); + INSERT INTO gpkg_contents (table_name, data_type, identifier) VALUES ('ortho', 'tiles', 'ortho'); + CREATE VIEW recent AS SELECT * FROM permits; CREATE TABLE scratch (x);", + ) + .unwrap(); + let before = std::fs::read(&path).unwrap(); + for name in ["ortho", "recent", "scratch"] { + let e = write_path_to(&ds, &path, Crs::LonLat, &replace(name)).unwrap_err().to_string(); + assert!(e.contains("doesn't replace"), "{name}: {e}"); + assert_eq!(std::fs::read(&path).unwrap(), before, "{name}"); + } + let plain = workspace.path().join("plain.gpkg"); + rusqlite::Connection::open(&plain).unwrap().execute_batch("CREATE TABLE t (x)").unwrap(); + let e = write_path_to(&ds, &plain, Crs::LonLat, &replace("permits")).unwrap_err().to_string(); + assert!(e.contains("not a GeoPackage"), "{e}"); + let text = workspace.path().join("text.gpkg"); + std::fs::write(&text, b"not a database").unwrap(); + assert!(write_path_to(&ds, &text, Crs::LonLat, &replace("permits")).is_err()); + assert_eq!(std::fs::read(&text).unwrap(), b"not a database"); +} diff --git a/crates/engine/src/cmd/share.rs b/crates/engine/src/cmd/share.rs index d830fa05d..568afe80b 100644 --- a/crates/engine/src/cmd/share.rs +++ b/crates/engine/src/cmd/share.rs @@ -3,6 +3,7 @@ use std::path::Path; +use rustgis_data::io::{GpkgMode, GpkgTable, GpkgWritten}; use rustgis_data::raster::Crs; use rustgis_render::View; use serde_json::{Value, json}; @@ -18,34 +19,71 @@ pub fn commands() -> Vec { C::new("share.exportLayer", "Export Features", "Publish › Output", export_layer) .view() .when(need_layer) - .params(r#"{"layer"?, "path": "*.geojson|*.csv|*.fgb|*.gpkg|*.parquet|*.shp|*.zip|*.kml|*.kmz|*.gpx", "crs"?: "EPSG:… (or WKT) for a GeoPackage, Shapefile or FlatGeobuf (default: the system the layer was read in, else EPSG:4326)"}"#), + .params(r#"{"layer"?, "path": "*.geojson|*.csv|*.fgb|*.gpkg|*.parquet|*.shp|*.zip|*.kml|*.kmz|*.gpx", "crs"?: "EPSG:… (or WKT) for a GeoPackage, Shapefile or FlatGeobuf (default: the system the layer was read in, else EPSG:4326)", "mode"?: "newFile (default: refuses an existing file) | addTable | replaceTable, for a GeoPackage", "tableName"?: "the GeoPackage table (default: the layer's name)"}"#), C::new("share.exportTable", "Export Table", "Publish › Output", |s, p| { let path = ps(p, "path").ok_or("path is required")?; + let ext = Path::new(path).extension().and_then(|e| e.to_str()).unwrap_or("").to_ascii_lowercase(); + let gpkg = gpkg_table(p, &ext)?; let t = s.project.tables.get(s.table_index(p)?).ok_or("no such table")?; // Tables may retain geometry and its import losses, just as feature layers do. let notes = rustgis_data::export_notes([(&t.dataset, t.dataset.extent())], path, Crs::LonLat); - let ext = Path::new(path).extension().and_then(|e| e.to_str()).unwrap_or("").to_ascii_lowercase(); let mut dbf_notes = Vec::new(); if matches!(ext.as_str(), "shp" | "zip") { dbf_notes.extend(rustgis_data::io::shapefile_names_note(&t.dataset.fields, path)); dbf_notes.extend(rustgis_data::io::shapefile_values_notes(&t.dataset, path)); } - let left_out = rustgis_data::io::write_path(&t.dataset, Path::new(path)).map_err(|e| e.to_string())?; + let (left_out, table) = + rustgis_data::io::write_path_to(&t.dataset, Path::new(path), Crs::LonLat, &gpkg).map_err(|e| e.to_string())?; if left_out > 0 { s.notify(format!("{path}: {left_out} features without drawable geometry were left out"), false); } for note in notes.into_iter().chain(dbf_notes) { s.notify(note, false); } - Ok(json!({"path": path})) + let mut out = json!({"path": path}); + report_table(s, path, &gpkg, table, &mut out); + Ok(out) }) .params( - r#"{"table": name|id (a standalone table), "path": "*.geojson|*.csv|*.fgb|*.gpkg|*.parquet|*.shp|*.zip|*.kml|*.kmz|*.gpx"}"#, + r#"{"table": name|id (a standalone table), "path": "*.geojson|*.csv|*.fgb|*.gpkg|*.parquet|*.shp|*.zip|*.kml|*.kmz|*.gpx", "mode"?: "newFile (default: refuses an existing file) | addTable | replaceTable, for a GeoPackage", "tableName"?: "the GeoPackage table (default: the table's name)"}"#, ) .view(), ] } +/// A GeoPackage export's table, from `mode` and `tableName`, which only a GeoPackage (extension +/// `ext`) takes. +fn gpkg_table(p: &Value, ext: &str) -> Result { + let mode = match ps(p, "mode").map(str::trim) { + None | Some("newFile") => GpkgMode::NewFile, + Some("addTable") => GpkgMode::AddTable, + Some("replaceTable") => GpkgMode::ReplaceTable, + Some(other) => return Err(format!("unknown mode '{other}' (newFile, addTable or replaceTable)")), + }; + let name = ps(p, "tableName").map(str::trim).filter(|n| !n.is_empty()).map(String::from); + if ext != "gpkg" && (mode != GpkgMode::NewFile || name.is_some()) { + return Err("mode and tableName are for a GeoPackage (.gpkg) export".into()); + } + Ok(GpkgTable { mode, name }) +} + +/// Puts the table a GeoPackage export wrote, and whether it replaced one, in its result `out`; +/// an export into an existing GeoPackage also says so, and that a replaced table's own indexes, +/// triggers and column descriptions went with it. +fn report_table(s: &mut Session, path: &str, gpkg: &GpkgTable, table: Option, out: &mut Value) { + let Some(table) = table else { return }; + if gpkg.mode != GpkgMode::NewFile { + let note = if table.replaced { + format!("{path}: replaced its table {}, dropping the old table's indexes, triggers and column descriptions", table.table) + } else { + format!("{path}: added the table {}", table.table) + }; + s.notify(note, false); + } + out["table"] = json!(table.table); + out["replaced"] = json!(table.replaced); +} + /// Renders the active map at the current view (or a requested size) to PNG bytes. pub fn map_png(s: &mut Session, width: Option, height: Option, dpi: Option) -> Result, String> { let (vw, vh) = s.view_size; @@ -107,9 +145,14 @@ fn export_layer(s: &mut Session, p: &Value) -> CmdResult { let path = ps(p, "path").ok_or("path is required")?.to_string(); let l = s.map().layer(id).ok_or("no layer")?; let ext = Path::new(&path).extension().and_then(|e| e.to_str()).unwrap_or("").to_ascii_lowercase(); + let gpkg = gpkg_table(p, &ext)?; + // A layer read in place from the GeoPackage it goes into is read whole first: SQLite can't + // write a file while the same export is reading it. + let from_target = gpkg.mode != GpkgMode::NewFile + && l.source.as_ref().and_then(|src| src.path.as_deref()).is_some_and(|src| same_file(Path::new(src), Path::new(&path))); // A streamed layer exports its whole source, not the features read for the view: a chunk at // a time where the source and format allow, so its size isn't limited by memory. - let chunks = if l.is_streamed() && rustgis_data::export::streams(&ext) { l.chunk_export()? } else { None }; + let chunks = if l.is_streamed() && rustgis_data::export::streams(&ext) && !from_target { l.chunk_export()? } else { None }; let (whole, unread) = if l.is_streamed() && chunks.is_none() { l.load_all_counted().map(|(ds, n)| (Some(ds), n))? } else { (None, 0) }; let ds = chunks.as_ref().map(|(_, layer)| layer).or(whole.as_ref()).unwrap_or(&l.dataset); // GeoJSON, KML and GPX are longitude/latitude; the others keep the layer's own system unless @@ -118,14 +161,15 @@ fn export_layer(s: &mut Session, p: &Value) -> CmdResult { Some(text) => Crs::parse(text)?, None => rustgis_data::io::default_export_crs(ds, &ext), }; - let (written, left_out, skipped) = match &chunks { + let (written, left_out, skipped, table) = match &chunks { Some((src, layer)) => { - let e = rustgis_data::export::write_source_in(src.as_ref(), layer, Path::new(&path), crs).map_err(|e| e.to_string())?; - (e.written, e.left_out, e.skipped) + let (e, table) = + rustgis_data::export::write_source_to(src.as_ref(), layer, Path::new(&path), crs, &gpkg).map_err(|e| e.to_string())?; + (e.written, e.left_out, e.skipped, table) } None => { - let left_out = rustgis_data::io::write_path_in(ds, Path::new(&path), crs).map_err(|e| e.to_string())?; - (ds.features.len().saturating_sub(left_out), left_out, unread) + let (left_out, table) = rustgis_data::io::write_path_to(ds, Path::new(&path), crs, &gpkg).map_err(|e| e.to_string())?; + (ds.features.len().saturating_sub(left_out), left_out, unread, table) } }; // A streamed layer's extent is its whole source's. @@ -155,5 +199,12 @@ fn export_layer(s: &mut Session, p: &Value) -> CmdResult { s.notify(format!("{path}: no .prj written, as {} has no WKT form", crs.label()), false); } s.notify(format!("Exported {path}"), false); - Ok(json!({"path": path, "features": written, "leftOut": left_out, "crs": crs.label()})) + let mut out = json!({"path": path, "features": written, "leftOut": left_out, "crs": crs.label()}); + report_table(s, &path, &gpkg, table, &mut out); + Ok(out) +} + +/// Whether `a` and `b` are the same existing file. +fn same_file(a: &Path, b: &Path) -> bool { + matches!((std::fs::canonicalize(a), std::fs::canonicalize(b)), (Ok(a), Ok(b)) if a == b) } diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index f7a342f91..d4ecd722b 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -12229,6 +12229,64 @@ fn exporting_features_refuses_to_replace_an_existing_geopackage() { std::fs::remove_dir_all(dir).unwrap(); } +/// Export Features adds a layer to an existing GeoPackage as one more table, or replaces one of +/// its tables, saying which; a layer read in place from that GeoPackage can go back into it (its +/// filtered copy as another table). `mode` and `tableName` are for GeoPackages only. +#[test] +fn exporting_features_adds_or_replaces_a_table_in_an_existing_geopackage() { + let dir = std::env::temp_dir().join(format!("rustgis-engine-gpkg-add-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + let path = dir.join("county.gpkg"); + let mut s = world(); + s.run("share.exportLayer", &json!({"layer": "Cities", "path": path})).unwrap(); + let mark = s.notice_mark(); + let out = s.run("share.exportLayer", &json!({"layer": "Countries", "path": path, "mode": "addTable"})).unwrap(); + assert_eq!((out["table"].as_str(), out["replaced"].as_bool()), (Some("Countries"), Some(false))); + let texts: Vec = s.notices_since(mark).iter().map(|n| n.text.clone()).collect(); + assert!(texts.iter().any(|t| t.ends_with("added the table Countries")), "{texts:?}"); + let tables = rustgis_data::source::layers(&path).unwrap(); + assert_eq!(tables, ["Cities", "Countries"]); + // Taken: refused, unless replacing. + let e = s.run("share.exportLayer", &json!({"layer": "Cities", "path": path, "mode": "addTable"})).unwrap_err(); + assert!(e.to_string().contains("already has a table Cities"), "{e}"); + let cities = s.map().layer_by_name("Cities").map(|l| l.dataset.features.len()).unwrap(); + let mark = s.notice_mark(); + let out = + s.run("share.exportLayer", &json!({"layer": "Countries", "path": path, "mode": "replaceTable", "tableName": "Cities"})).unwrap(); + assert_eq!(out["replaced"].as_bool(), Some(true)); + assert!(s.notices_since(mark).iter().any(|n| n.text.contains("replaced its table Cities, dropping"))); + let countries = s.map().layer_by_name("Countries").map(|l| l.dataset.features.len()).unwrap(); + let count = |t: &str| rustgis_data::source::open_path(&path, Some(t)).unwrap().info().count; + assert_eq!(count("Cities"), Some(countries as u64)); + assert_ne!(cities, countries); + // Read in place from the GeoPackage, then exported back into it. + s.run("layer.addData", &json!({"path": path.display().to_string(), "layer": "Countries", "stream": true, "name": "InPlace"})).unwrap(); + s.run("share.exportLayer", &json!({"layer": "InPlace", "path": path, "mode": "addTable", "tableName": "copy"})).unwrap(); + assert_eq!(count("copy"), Some(countries as u64)); + // A standalone table goes in as an attribute table. + s.run("layer.addDataText", &json!({"fileName": "owners.csv", "text": "name,n\nShelby,1\n"})).unwrap(); + let table = s.project.tables.last().map(|t| t.id).unwrap(); + let out = s.run("share.exportTable", &json!({"table": table, "path": path, "mode": "addTable"})).unwrap(); + assert_eq!(out["table"].as_str(), Some("owners")); + let c = rusqlite::Connection::open(&path).unwrap(); + assert_eq!( + c.query_row("SELECT data_type FROM gpkg_contents WHERE table_name = 'owners'", [], |r| r.get::<_, String>(0)).unwrap(), + "attributes" + ); + drop(c); + // Other formats take neither; a mode not known is refused. + let geojson = dir.join("x.geojson"); + for p in [json!({"mode": "addTable"}), json!({"tableName": "x"})] { + let mut p = p; + p["path"] = json!(geojson); + assert!(s.run("share.exportLayer", &p).unwrap_err().to_string().contains("GeoPackage"), "{p}"); + } + assert!(s.run("share.exportLayer", &json!({"path": path, "mode": "append"})).is_err()); + assert!(!geojson.exists()); + std::fs::remove_dir_all(dir).unwrap(); +} + /// A layer whose Z values were dropped on import says so when added, read whole or in place, and /// again when exported or converted; a 2D layer's export says nothing of it. #[test] diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index 097e763a8..b0770a6d8 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -38,6 +38,8 @@ pub struct Dialogs { /// Add From Database: load the PostgreSQL table whole, to be edited and saved back. pub edit: bool, pub mode: &'static str, + /// Export Features' GeoPackage destination: one of [`GPKG_MODES`]' codes (empty: a new file). + pub gpkg_mode: &'static str, pub flag: bool, pub error: Option, pub info: Option, @@ -164,7 +166,12 @@ impl Dialogs { "renameLayer" => self.text = active.map(|l| l.name.clone()).unwrap_or_default(), // A file name can't hold a folder separator, which a project name may. "saveProject" => self.text = format!("{}.rgis", s.project.name.replace(['/', '\\'], "-")), - "exportLayer" => self.mode = "geojson", + "exportLayer" => { + self.mode = "geojson"; + self.gpkg_mode = "newFile"; + // The GeoPackage table's name. + self.text = active.map(|l| l.name.clone()).unwrap_or_default(); + } // The active layer when it is from PostGIS, else the first that is. "pgSave" | "pgDiscard" => { let pg = |l: &&rustgis_engine::map::Layer| l.postgis.is_some(); @@ -580,6 +587,10 @@ pub const EXPORT_FORMATS: &[(&str, &str)] = &[ ("GPX", "gpx"), ]; +/// Where Export Features puts a GeoPackage's table: (`share.exportLayer` mode, name). +pub const GPKG_MODES: &[(&str, &str)] = + &[("newFile", "A new GeoPackage"), ("addTable", "A new table in a GeoPackage"), ("replaceTable", "A table replaced in a GeoPackage")]; + /// The formats Export Features offers: all of [`EXPORT_FORMATS`] on the desktop. The web app /// leaves out GeoPackage, written with SQLite, and FlatGeobuf, whose writer needs a temporary /// file; the browser has neither. @@ -608,12 +619,42 @@ fn export_layer(app: &mut RustGisApp, ui: &mut egui::Ui) -> bool { .response .labelled_by(input_label.id); ui.end_row(); + // A GeoPackage can take the layer as one more table (desktop only: the browser offers none). + if app.dialogs.mode == "gpkg" && !web { + let into_label = ui.label(tl!("Into")); + let shown = GPKG_MODES.iter().find(|(m, _)| *m == app.dialogs.gpkg_mode).map_or(GPKG_MODES[0].1, |(_, l)| *l); + egui::ComboBox::from_id_salt("export_gpkg_mode") + .selected_text(tl!(shown)) + .width(220.0) + .show_ui(ui, |ui| { + for (mode, label) in GPKG_MODES { + ui.selectable_value(&mut app.dialogs.gpkg_mode, *mode, tl!(*label)); + } + }) + .response + .labelled_by(into_label.id); + ui.end_row(); + if !matches!(app.dialogs.gpkg_mode, "" | "newFile") { + let table_label = ui.label(tl!("Table")); + ui.add(egui::TextEdit::singleline(&mut app.dialogs.text).desired_width(220.0)).labelled_by(table_label.id); + ui.end_row(); + } + } }); if app.dialogs.mode == "shp" && web { ui.label(egui::RichText::new(tl!("The Shapefile's files download as one zip.")).small().weak()); } + let into_existing = app.dialogs.mode == "gpkg" && !web && !matches!(app.dialogs.gpkg_mode, "" | "newFile"); + if into_existing { + ui.label(egui::RichText::new(tl!("Pick the GeoPackage next. Its other tables are kept.")).small().weak()); + } buttons(app, ui, if web { "Download" } else { "Export…" }, |app| { - app.run("ui.exportLayer", json!({"format": app.dialogs.mode})).map(|_| ()) + let mut p = json!({"format": app.dialogs.mode}); + if into_existing { + p["mode"] = json!(app.dialogs.gpkg_mode); + p["tableName"] = json!(app.dialogs.text); + } + app.run("ui.exportLayer", p).map(|_| ()) }) } diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 55e7f9ae9..0ebfd26ec 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -3351,6 +3351,18 @@ impl RustGisApp { .find(|(_, e)| *e == ext) .map(|(l, _)| *l) .ok_or_else(|| format!("unknown format '{ext}'"))?; + // Into a GeoPackage that exists: picked to open, as it is kept, not replaced. + let mode = p.get("mode").and_then(Value::as_str).unwrap_or("newFile"); + if ext == "gpkg" && mode != "newFile" { + if let Some(path) = dialogs::pick_open(&["gpkg"], label) { + let mut args = json!({"path": path, "mode": mode}); + if let Some(table) = p.get("tableName").and_then(Value::as_str) { + args["tableName"] = json!(table); + } + return self.run("share.exportLayer", args); + } + return Ok(Value::Null); + } let name = self .session .active_layer @@ -4501,6 +4513,23 @@ mod download_service_tests { assert!(!app.session.notices.iter().any(|n| n.text.contains("254 bytes") || n.text.contains("left empty"))); } + /// Export Features into an existing GeoPackage: the file is picked to open, not to save over, + /// and the layer goes in as the table named in the dialog, beside the file's others. + #[test] + fn export_features_adds_a_table_to_a_picked_geopackage() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("county.gpkg"); + let mut app = RustGisApp::with_sample(); + app.run("share.exportLayer", json!({"layer": "Cities", "path": path})).unwrap(); + app.session.active_layer = app.session.map().layer_by_name("Countries").map(|l| l.id); + crate::dialogs::PICKED.with(|p| *p.borrow_mut() = Some(path.display().to_string())); + app.run("ui.exportLayer", json!({"format": "gpkg", "mode": "addTable", "tableName": "borders"})).unwrap(); + assert_eq!(rustgis_engine::data::source::layers(&path).unwrap(), ["Cities", "borders"]); + // Cancelling the pick writes nothing. + app.run("ui.exportLayer", json!({"format": "gpkg", "mode": "replaceTable", "tableName": "Cities"})).unwrap(); + assert_eq!(rustgis_engine::data::source::layers(&path).unwrap(), ["Cities", "borders"]); + } + #[test] fn direct_file_result_reports_delivery_instead_of_pending() { let pending = json!({"pending": true, "download": 7, "file": "cities.pmtiles"}); diff --git a/docs/python.md b/docs/python.md index 7d8e96725..e6dd9be6c 100644 --- a/docs/python.md +++ b/docs/python.md @@ -271,8 +271,14 @@ s["Cities"].to_geojson() # a FeatureCollection dict big.to_file("big-cities.gpkg", crs="EPSG:3857") # GeoJSON, CSV, FlatGeobuf, GeoParquet, … ``` -GeoPackage exports through `Layer.to_file` or `share.exportLayer` require a new filename. -An existing `.gpkg` is refused to preserve its tables; export does not append or replace tables. +A GeoPackage export through `Layer.to_file` or `share.exportLayer` writes a new file and refuses +an existing `.gpkg`, unless told to add a table to it or replace one of its tables; its other +tables are kept: + +```python +parcels.to_file("county.gpkg", mode="addTable", table_name="parcels_2026") +parcels.to_file("county.gpkg", mode="replaceTable") # its table named for the layer +``` `add_geojson` also takes any object with `__geo_interface__` (a Shapely geometry, a GeoSeries). `add_data`, `add_geojson` and `add_geopandas` return a `Layer` for a single layer, a list for diff --git a/docs/user-guide/data-formats.md b/docs/user-guide/data-formats.md index 747cb8aa5..76f347d2e 100644 --- a/docs/user-guide/data-formats.md +++ b/docs/user-guide/data-formats.md @@ -475,7 +475,7 @@ were rounded, and Export Raster, which writes the rounded values, says so again. | Format | How to write it | Notes | | --- | --- | --- | | GeoJSON (`.geojson`, `.json`), CSV | Data › Export (`share.exportLayer`) | The Export Features dialog picks the format; the chosen extension is appended when needed. In the web app it downloads any vector format below except GeoPackage and FlatGeobuf (a Shapefile as one zip). A streamed layer exports its whole source: on the desktop, to GeoJSON, CSV, FlatGeobuf, GeoPackage or GeoParquet a chunk at a time with no limit on its size (a GeoParquet source's row groups are read whole, so on the desktop each holds at most 16,777,216 rows), otherwise loaded whole (see [Streamed layers](data.md)); remote sources on the web must be downloaded first. | -| GeoPackage | Data › Export to `.gpkg` (desktop) | A new file with one feature table (an attributes table for a layer without geometry) in the layer's coordinate system (see below), with an R-tree and GeoPackage 1.4 triggers to maintain it. An existing destination is refused; adding or replacing tables in an existing GeoPackage is not supported. Unique feature ids that fit signed 64-bit integers are kept; otherwise SQLite assigns new ids. | +| GeoPackage | Data › Export to `.gpkg` (desktop) | A new file with one feature table (an attributes table for a layer without geometry) in the layer's coordinate system (see below), with an R-tree and GeoPackage 1.4 triggers to maintain it. An existing destination is refused unless the export adds a table to it or replaces one of its feature or attribute tables (`mode`, see [Exporting](sharing.md)), leaving its other tables as they were. Unique feature ids that fit signed 64-bit integers are kept; otherwise SQLite assigns new ids. | | GeoParquet (`.parquet`, `.geoparquet`) | Data › Export | GeoParquet 1.1: WKB geometry in WGS 84, Snappy compression, row groups of up to 65,536 features and a `bbox` covering column. A standalone table without geometry writes plain Parquet. | | Shapefile | Data › Export to `.shp` (or `.zip` for the files zipped) | Writes `.shp`, `.shx`, `.dbf`, `.prj` (the coordinate system, see below) and `.cpg` (UTF-8). Field names are cut to 10 bytes (made unique ignoring case, and reported), text to at most 254 bytes, and numbers that do not fit their dBase field are left empty. Export notices count shortened text, non-null values left empty (including incompatible types), and numbers rounded to the field's precision, and name the affected source fields; features without geometry, and lines of a single point, are left out (and reported). | | FlatGeobuf | Data › Export to `.fgb` | In the layer's coordinate system (see below), with a spatial index; features without geometry are left out (and reported). Desktop only: the writer needs a temporary file, which the browser doesn't have. | diff --git a/docs/user-guide/sharing.md b/docs/user-guide/sharing.md index 8da298bf8..b8bdb3fab 100644 --- a/docs/user-guide/sharing.md +++ b/docs/user-guide/sharing.md @@ -19,9 +19,23 @@ table. **Data › Export** also writes rasters to GeoTIFF, and **Data Interop** KML, GPX or CSV with WKT. Layouts, reports and presentations export from their own tabs and panes (see [Layouts and Reports](layouts.md)). -GeoPackage exports require a new filename. Exporting to an existing `.gpkg` is refused to -protect its tables and data, even if the save dialog offers to replace the file. Export does -not append a table to an existing GeoPackage or replace one of its tables. +A GeoPackage export writes a new file by default, and refuses an existing `.gpkg` to protect its +tables, even if the save dialog offers to replace the file. To put the layer into an existing +GeoPackage instead, choose GeoPackage in **Export Features**, then under **Into** either +**A new table in a GeoPackage** or **A table replaced in a GeoPackage**, name the table, and +pick the file (on the desktop). The GeoPackage's other tables stay as they were: one table is +added, or one feature or attribute table of that name is replaced with its spatial index, in a +single transaction, so a failed export changes nothing. A name already used is refused when +adding (with no file there, adding makes a new GeoPackage). Replacing needs a feature or +attribute table of that name, so a misspelt name changes nothing; tiles, views and other tables +are never replaced. A replaced table's own indexes, triggers and column descriptions go with it. +From the command line, `share.exportLayer` and `share.exportTable` take `"mode": "addTable"` or +`"replaceTable"` and an optional `"tableName"` (default: the layer's or table's name): + +```sh +rustgis-cli run --data permits.geojson \ + --cmd 'share.exportLayer={"layer":"permits","path":"county.gpkg","mode":"addTable"}' +``` Packages and project item files are described under [Project items](data.md#project-items). From 936ea4a16bbbe0d6806b4975ca112ab4d6fa4c84 Mon Sep 17 00:00:00 2001 From: mavrick Date: Tue, 3 Nov 2026 15:07:07 -0600 Subject: [PATCH 02/10] fix(data): guard exports into existing GeoPackages against their triggers and links Review follow-up for the GeoPackage add/replace export: the file's own triggers don't run while the export writes; a table in a related-tables relation isn't replaced; another application's database is refused; views and triggers elsewhere that name a replaced table are listed in a notice, which also names every kind of record dropped; mode and tableName must be strings. Docs: the Python example uses bound layers and the roadmap names share.exportTable too. Co-Authored-By: Claude Opus 5.5 --- ROADMAP.md | 3 +- crates/data/src/export/tests.rs | 2 +- crates/data/src/io.rs | 5 ++- crates/data/src/source/gpkg.rs | 4 +- crates/data/src/source/gpkg/add.rs | 61 ++++++++++++++++++++++++++---- crates/data/src/source/tests.rs | 35 ++++++++++++++--- crates/engine/src/cmd/share.rs | 31 ++++++++++++--- crates/engine/src/tests.rs | 8 ++++ crates/ui-egui/src/dialogs.rs | 2 +- docs/python.md | 4 +- docs/user-guide/sharing.md | 4 +- 11 files changed, 132 insertions(+), 27 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index 89456d417..1a664ed0f 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -148,7 +148,8 @@ Open questions rather than plans; discuss before building. ## Done - Export into an existing GeoPackage on desktop: a layer or table added as one more table, or - replacing one of its tables, leaving the others as they were (`share.exportLayer` `mode`). + replacing one of its tables, leaving the others as they were (`mode` on `share.exportLayer` and + `share.exportTable`). - Polygon editing: Cut Polygon, Reshape (polygons and lines) and Trace along an edge while drawing, on the map and through `edit.cutPolygon`, `edit.reshape` and `edit.trace`. - Geometry conversions: Polygon To Line, Split Line At Vertices, Feature Envelope To Polygon diff --git a/crates/data/src/export/tests.rs b/crates/data/src/export/tests.rs index 06aac7f70..24d067315 100644 --- a/crates/data/src/export/tests.rs +++ b/crates/data/src/export/tests.rs @@ -305,7 +305,7 @@ fn a_streamed_export_into_an_existing_geopackage_starts_again_inside_its_transac let src = Runs::new(&both, 4); let (e, table) = write_chunks_to(&src, &layer, &county, Crs::LonLat, &add, 1).unwrap(); assert_eq!(src.passes.load(std::sync::atomic::Ordering::Relaxed), 3, "written three times"); - assert_eq!((e.written, table), (20, Some(GpkgWritten { table: "both".into(), replaced: false }))); + assert_eq!((e.written, table), (20, Some(GpkgWritten { table: "both".into(), replaced: false, dependents: vec![] }))); let whole = d.join("whole.gpkg"); crate::io::write_path(&both, &whole).unwrap(); let read = diff --git a/crates/data/src/io.rs b/crates/data/src/io.rs index 5c7d8604b..890770369 100644 --- a/crates/data/src/io.rs +++ b/crates/data/src/io.rs @@ -500,12 +500,13 @@ pub struct GpkgTable { pub name: Option, } -/// The table a GeoPackage export wrote: its name in the file (a reserved name is prefixed), and -/// whether it replaced one. +/// The table a GeoPackage export wrote: its name in the file (a reserved name is prefixed), +/// whether it replaced one, and the views and triggers on other tables that name a replaced one. #[derive(Clone, Debug, PartialEq, Eq)] pub struct GpkgWritten { pub table: String, pub replaced: bool, + pub dependents: Vec, } /// As [`write_path`], in the coordinate system `crs` (a GeoPackage, Shapefile or FlatGeobuf; see diff --git a/crates/data/src/source/gpkg.rs b/crates/data/src/source/gpkg.rs index e393dc855..cb3325679 100644 --- a/crates/data/src/source/gpkg.rs +++ b/crates/data/src/source/gpkg.rs @@ -551,7 +551,7 @@ pub fn write_table_in(ds: &crate::Dataset, path: &Path, crs: Crs, target: &GpkgT let conn = Connection::open(&tmp).map_err(err)?; let table = build(&conn, ds, name, crs)?; install(conn, tmp, path)?; - Ok(GpkgWritten { table, replaced: false }) + Ok(GpkgWritten { table, replaced: false, dependents: Vec::new() }) } /// Whether an export goes into the existing GeoPackage at `path` (anything there, which opening @@ -598,7 +598,7 @@ pub(crate) fn write_features_in( Ok((layout, extent)) => { finish(&conn, &layout, extent)?; install(conn, tmp, path)?; - return Ok(GpkgWritten { table: layout.table, replaced: false }); + return Ok(GpkgWritten { table: layout.table, replaced: false, dependents: Vec::new() }); } } } diff --git a/crates/data/src/source/gpkg/add.rs b/crates/data/src/source/gpkg/add.rs index 09d5aa6e3..dd402efb9 100644 --- a/crates/data/src/source/gpkg/add.rs +++ b/crates/data/src/source/gpkg/add.rs @@ -30,6 +30,9 @@ pub(super) fn write_features( // to SQLite's own records. conn.set_db_config(DbConfig::SQLITE_DBCONFIG_DEFENSIVE, true).map_err(err)?; conn.set_db_config(DbConfig::SQLITE_DBCONFIG_TRUSTED_SCHEMA, false).map_err(err)?; + // Nor may its triggers run when the export writes its GeoPackage records: one could change + // other tables. The new table's own triggers are made after its rows are in. + conn.set_db_config(DbConfig::SQLITE_DBCONFIG_ENABLE_TRIGGER, false).map_err(err)?; conn.busy_timeout(Duration::from_secs(5)).map_err(err)?; let (mut multi_points, mut keep_ids) = (None, true); // Each choice changes at most once; an export starting again rolls back what it wrote. @@ -38,7 +41,8 @@ pub(super) fn write_features( check_geopackage(&tx, &file)?; // An attribute table has no spatial reference system. let srs_id = if geometry_type == GeometryType::None { 0 } else { srs_id(&tx, crs)? }; - let replaced = Cell::new(false); + // The views and triggers naming a replaced table, when one is. + let replaced = Cell::new(None); let mut make = |multi| { let layout = plan(name, geometry_type, fields, multi, srs_id); replaced.set(make_room(&tx, &file, &layout.table, replace)?); @@ -61,15 +65,22 @@ pub(super) fn write_features( Ok((layout, extent)) => { register(&tx, &layout, extent)?; tx.commit().map_err(err)?; - return Ok(GpkgWritten { table: layout.table, replaced: replaced.get() }); + let dependents = replaced.take(); + return Ok(GpkgWritten { table: layout.table, replaced: dependents.is_some(), dependents: dependents.unwrap_or_default() }); } } } Err(DataError::Format("GeoPackage: the export didn't settle on a table".into())) } -/// Refuses a database without a GeoPackage's contents and spatial reference systems. +/// Refuses a database without a GeoPackage's contents and spatial reference systems, or marked +/// as another application's (an unmarked one, as older programs write, is taken). fn check_geopackage(conn: &Connection, file: &str) -> Result<()> { + // "GPKG", and GeoPackage 1.0's and 1.1's "GP10" and "GP11". + let id: i64 = conn.query_row("PRAGMA application_id", [], |r| r.get(0)).map_err(err)?; + if !matches!(id, 0 | 0x4750_4B47 | 0x4750_3130 | 0x4750_3131) { + return Err(DataError::Format(format!("{file}: not a GeoPackage (its application id is {id:#010x})"))); + } for table in ["gpkg_contents", "gpkg_spatial_ref_sys"] { if object(conn, table)?.as_deref() != Some("table") { return Err(DataError::Format(format!("{file}: not a GeoPackage (it has no {table} table)"))); @@ -136,8 +147,10 @@ fn srs_id(conn: &Connection, crs: Crs) -> Result { /// Clears the way for table `table`: nothing of that name, or (with `replace`, which needs one) a /// feature or attribute table of it, dropped with its spatial index and its GeoPackage records. -/// Returns whether a table was replaced; anything else of the name is refused. -fn make_room(conn: &Connection, file: &str, table: &str, replace: bool) -> Result { +/// For a table replaced, returns the views and other tables' triggers whose SQL names it (they +/// read the new table, and break if a column they use is gone); anything else of the name is +/// refused. +fn make_room(conn: &Connection, file: &str, table: &str, replace: bool) -> Result>> { let registered: Option<(String, String)> = conn .query_row("SELECT table_name, data_type FROM gpkg_contents WHERE lower(table_name) = lower(?1)", [table], |r| { Ok((r.get(0)?, r.get(1)?)) @@ -150,7 +163,7 @@ fn make_room(conn: &Connection, file: &str, table: &str, replace: bool) -> Resul (None, None) if replace => { Err(DataError::Format(format!("{file} has no table {table} to replace; check the name, or add the layer as a new table"))) } - (None, None) => Ok(false), + (None, None) => Ok(None), (None, Some((old, _))) => { Err(DataError::Format(format!("{file} lists a table {old} in its contents that it doesn't have; choose another table name"))) } @@ -160,8 +173,14 @@ fn make_room(conn: &Connection, file: &str, table: &str, replace: bool) -> Resul "{file} already has a table {old}: choose another table name, or replace that table" ))); } + if related(conn, &old)? { + return Err(DataError::Format(format!( + "{file}: {old} is in a related-tables relation, whose links a replaced table would break; choose another table name" + ))); + } + let dependents = dependents(conn, &old)?; drop_table(conn, &old)?; - Ok(true) + Ok(Some(dependents)) } (_, Some((old, data_type))) => Err(DataError::Format(format!( "{file}: {old} is a {data_type} table, which an export doesn't replace; choose another table name" @@ -173,6 +192,34 @@ fn make_room(conn: &Connection, file: &str, table: &str, replace: bool) -> Resul } } +/// Whether the GeoPackage's related-tables extension names `table` as a base, related or mapping +/// table: its rows are linked by id, which a new table's rows don't keep. +fn related(conn: &Connection, table: &str) -> Result { + if object(conn, "gpkgext_relations")?.as_deref() != Some("table") { + return Ok(false); + } + conn.query_row( + "SELECT EXISTS (SELECT 1 FROM gpkgext_relations WHERE lower(?1) IN + (lower(base_table_name), lower(related_table_name), lower(mapping_table_name)))", + [table], + |r| r.get(0), + ) + .map_err(err) +} + +/// The views, and triggers on other tables, whose SQL mentions `table` (by a case-blind search, +/// so a longer name containing it counts too), by name. +fn dependents(conn: &Connection, table: &str) -> Result> { + let mut st = conn + .prepare( + "SELECT name FROM sqlite_master WHERE type IN ('view', 'trigger') AND lower(tbl_name) != lower(?1) + AND instr(lower(sql), lower(?1)) > 0 ORDER BY name", + ) + .map_err(err)?; + let names = st.query_map([table], |r| r.get::<_, String>(0)).map_err(err)?; + names.collect::>>().map_err(err) +} + /// An SQLite object's kind with its article: "an index", "a view". fn with_article(kind: &str) -> String { if kind.starts_with(['a', 'e', 'i', 'o', 'u']) { format!("an {kind}") } else { format!("a {kind}") } diff --git a/crates/data/src/source/tests.rs b/crates/data/src/source/tests.rs index 9443b1e79..9d9872583 100644 --- a/crates/data/src/source/tests.rs +++ b/crates/data/src/source/tests.rs @@ -2257,7 +2257,8 @@ fn geopackage_export_adds_and_replaces_tables_and_keeps_the_others() { "CREATE TABLE owners (name TEXT); INSERT INTO owners VALUES ('Shelby'); INSERT INTO gpkg_contents (table_name, data_type, identifier) VALUES ('owners', 'attributes', 'owners'); CREATE TABLE gpkg_ogr_contents (table_name TEXT NOT NULL PRIMARY KEY, feature_count INTEGER DEFAULT NULL); - INSERT INTO gpkg_ogr_contents VALUES ('parcels', 1);", + INSERT INTO gpkg_ogr_contents VALUES ('parcels', 1); + CREATE TRIGGER wipe AFTER INSERT ON gpkg_contents BEGIN DELETE FROM owners; END;", ) .unwrap(); let mut permits = Dataset::new("permits", GeometryType::Point, vec![Field::new("n", FieldType::Integer)]); @@ -2266,9 +2267,13 @@ fn geopackage_export_adds_and_replaces_tables_and_keeps_the_others() { } let to = |mode, name: Option<&str>| GpkgTable { mode, name: name.map(String::from) }; let written = write_path_to(&permits, &path, Crs::LonLat, &to(GpkgMode::AddTable, None)).unwrap(); - assert_eq!(written, (0, Some(GpkgWritten { table: "permits".into(), replaced: false }))); + assert_eq!(written, (0, Some(GpkgWritten { table: "permits".into(), replaced: false, dependents: vec![] }))); rusqlite::Connection::open(&path).unwrap().execute_batch("INSERT INTO gpkg_ogr_contents VALUES ('permits', 3)").unwrap(); assert_eq!(gpkg_tables(&path), ["owners", "parcels", "permits"]); + // The file's own trigger on its contents didn't run. + let owners = + |p: &Path| rusqlite::Connection::open(p).unwrap().query_row("SELECT count(*) FROM owners", [], |r| r.get::<_, i64>(0)).unwrap(); + assert_eq!(owners(&path), 1); let read = |t: &str, extent| open_path(&path, Some(t)).unwrap().read(&Request { extent, limit: 100 }).unwrap().features; assert_eq!(read("permits", None).len(), 3); // Its spatial index answers an extent query. @@ -2279,6 +2284,7 @@ fn geopackage_export_adds_and_replaces_tables_and_keeps_the_others() { let before = std::fs::read(&path).unwrap(); for name in [None, Some("PERMITS"), Some("owners_idx")] { if name == Some("owners_idx") { + assert_eq!(std::fs::read(&path).unwrap(), before, "the refusals changed nothing"); rusqlite::Connection::open(&path).unwrap().execute_batch("CREATE INDEX owners_idx ON owners (name)").unwrap(); } let e = write_path_to(&permits, &path, Crs::LonLat, &to(GpkgMode::AddTable, name)).unwrap_err().to_string(); @@ -2287,11 +2293,13 @@ fn geopackage_export_adds_and_replaces_tables_and_keeps_the_others() { let before_index = std::fs::read(&path).unwrap(); assert_ne!(before, before_index, "the index was added"); - // Replacing: fewer features, in another system, and its old spatial index gone. + // Replacing: fewer features, in another system, and its old spatial index gone. A view on the + // table is named, as it reads the new one. + rusqlite::Connection::open(&path).unwrap().execute_batch("CREATE VIEW recent_permits AS SELECT * FROM permits").unwrap(); let mut fewer = permits.clone(); fewer.features.truncate(1); let replaced = write_path_to(&fewer, &path, Crs::WebMercator, &to(GpkgMode::ReplaceTable, None)).unwrap(); - assert_eq!(replaced.1, Some(GpkgWritten { table: "permits".into(), replaced: true })); + assert_eq!(replaced.1, Some(GpkgWritten { table: "permits".into(), replaced: true, dependents: vec!["recent_permits".into()] })); assert_eq!(gpkg_tables(&path), ["owners", "parcels", "permits"]); let s = open_path(&path, Some("permits")).unwrap(); assert_eq!(s.info().count, Some(1)); @@ -2309,7 +2317,7 @@ fn geopackage_export_adds_and_replaces_tables_and_keeps_the_others() { drop(c); // A second table in that system reuses its entry. let more = write_path_to(&fewer, &path, Crs::WebMercator, &to(GpkgMode::AddTable, Some("permits_2027"))).unwrap(); - assert_eq!(more.1, Some(GpkgWritten { table: "permits_2027".into(), replaced: false })); + assert_eq!(more.1, Some(GpkgWritten { table: "permits_2027".into(), replaced: false, dependents: vec![] })); // Replacing a table not there (a misspelt name) is refused rather than adding one. let e = write_path_to(&fewer, &path, Crs::LonLat, &to(GpkgMode::ReplaceTable, Some("permit"))).unwrap_err().to_string(); assert!(e.contains("no table permit to replace"), "{e}"); @@ -2363,6 +2371,23 @@ fn geopackage_export_into_a_file_refuses_what_it_cannot_replace() { assert!(e.contains("doesn't replace"), "{name}: {e}"); assert_eq!(std::fs::read(&path).unwrap(), before, "{name}"); } + // A related-tables relation's table isn't replaced; another application's database isn't + // written to. + rusqlite::Connection::open(&path) + .unwrap() + .execute_batch( + "CREATE TABLE gpkgext_relations (id INTEGER PRIMARY KEY, base_table_name TEXT, base_primary_column TEXT, + related_table_name TEXT, related_primary_column TEXT, relation_name TEXT, mapping_table_name TEXT); + INSERT INTO gpkgext_relations VALUES (1, 'permits', 'fid', 'owners', 'id', 'attributes', 'permits_owners');", + ) + .unwrap(); + let e = write_path_to(&ds, &path, Crs::LonLat, &replace("permits")).unwrap_err().to_string(); + assert!(e.contains("related-tables relation"), "{e}"); + let other = workspace.path().join("other.gpkg"); + std::fs::copy(&path, &other).unwrap(); + rusqlite::Connection::open(&other).unwrap().execute_batch("PRAGMA application_id = 1297105496").unwrap(); + let add = GpkgTable { mode: GpkgMode::AddTable, name: Some("more".into()) }; + assert!(write_path_to(&ds, &other, Crs::LonLat, &add).unwrap_err().to_string().contains("application id")); let plain = workspace.path().join("plain.gpkg"); rusqlite::Connection::open(&plain).unwrap().execute_batch("CREATE TABLE t (x)").unwrap(); let e = write_path_to(&ds, &plain, Crs::LonLat, &replace("permits")).unwrap_err().to_string(); diff --git a/crates/engine/src/cmd/share.rs b/crates/engine/src/cmd/share.rs index 568afe80b..c13bf938f 100644 --- a/crates/engine/src/cmd/share.rs +++ b/crates/engine/src/cmd/share.rs @@ -54,13 +54,18 @@ pub fn commands() -> Vec { /// A GeoPackage export's table, from `mode` and `tableName`, which only a GeoPackage (extension /// `ext`) takes. fn gpkg_table(p: &Value, ext: &str) -> Result { - let mode = match ps(p, "mode").map(str::trim) { + // Not a string is refused, not taken for the default. + let text = |key: &str| match p.get(key) { + None | Some(Value::Null) => Ok(None), + Some(v) => v.as_str().map(|s| Some(s.trim())).ok_or_else(|| format!("{key} must be a string")), + }; + let mode = match text("mode")? { None | Some("newFile") => GpkgMode::NewFile, Some("addTable") => GpkgMode::AddTable, Some("replaceTable") => GpkgMode::ReplaceTable, Some(other) => return Err(format!("unknown mode '{other}' (newFile, addTable or replaceTable)")), }; - let name = ps(p, "tableName").map(str::trim).filter(|n| !n.is_empty()).map(String::from); + let name = text("tableName")?.filter(|n| !n.is_empty()).map(String::from); if ext != "gpkg" && (mode != GpkgMode::NewFile || name.is_some()) { return Err("mode and tableName are for a GeoPackage (.gpkg) export".into()); } @@ -68,17 +73,33 @@ fn gpkg_table(p: &Value, ext: &str) -> Result { } /// Puts the table a GeoPackage export wrote, and whether it replaced one, in its result `out`; -/// an export into an existing GeoPackage also says so, and that a replaced table's own indexes, -/// triggers and column descriptions went with it. +/// an export into an existing GeoPackage also says so, what went with a replaced table, and +/// which views and triggers elsewhere name it. fn report_table(s: &mut Session, path: &str, gpkg: &GpkgTable, table: Option, out: &mut Value) { let Some(table) = table else { return }; if gpkg.mode != GpkgMode::NewFile { let note = if table.replaced { - format!("{path}: replaced its table {}, dropping the old table's indexes, triggers and column descriptions", table.table) + format!( + "{path}: replaced its table {}, dropping the old table's indexes, triggers, column descriptions, metadata links and extension records", + table.table + ) } else { format!("{path}: added the table {}", table.table) }; s.notify(note, false); + if !table.dependents.is_empty() { + let n = table.dependents.len(); + s.notify( + format!( + "{path}: {n} view{} or trigger{} naming {} may no longer work if a column it uses is gone: {}", + if n == 1 { "" } else { "s" }, + if n == 1 { "" } else { "s" }, + table.table, + table.dependents.join(", ") + ), + false, + ); + } } out["table"] = json!(table.table); out["replaced"] = json!(table.replaced); diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index d4ecd722b..82eff1130 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -12283,7 +12283,15 @@ fn exporting_features_adds_or_replaces_a_table_in_an_existing_geopackage() { assert!(s.run("share.exportLayer", &p).unwrap_err().to_string().contains("GeoPackage"), "{p}"); } assert!(s.run("share.exportLayer", &json!({"path": path, "mode": "append"})).is_err()); + // Not strings: refused, not taken for the default. + for p in [json!({"mode": 1}), json!({"mode": "addTable", "tableName": ["x"]})] { + let mut p = p; + p["path"] = json!(path); + assert!(s.run("share.exportLayer", &p).unwrap_err().to_string().contains("must be a string"), "{p}"); + } assert!(!geojson.exists()); + // The layer read in place holds the file open, which Windows won't remove. + drop(s); std::fs::remove_dir_all(dir).unwrap(); } diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index b0770a6d8..8b4f82c27 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -587,7 +587,7 @@ pub const EXPORT_FORMATS: &[(&str, &str)] = &[ ("GPX", "gpx"), ]; -/// Where Export Features puts a GeoPackage's table: (`share.exportLayer` mode, name). +/// Where Export Features puts a GeoPackage's table: (`share.exportLayer` mode, label shown). pub const GPKG_MODES: &[(&str, &str)] = &[("newFile", "A new GeoPackage"), ("addTable", "A new table in a GeoPackage"), ("replaceTable", "A table replaced in a GeoPackage")]; diff --git a/docs/python.md b/docs/python.md index e6dd9be6c..bc43cf8ee 100644 --- a/docs/python.md +++ b/docs/python.md @@ -276,8 +276,8 @@ an existing `.gpkg`, unless told to add a table to it or replace one of its tabl tables are kept: ```python -parcels.to_file("county.gpkg", mode="addTable", table_name="parcels_2026") -parcels.to_file("county.gpkg", mode="replaceTable") # its table named for the layer +roads.to_file("big-cities.gpkg", mode="addTable", table_name="roads_2026") +big.to_file("big-cities.gpkg", mode="replaceTable") # its table named for the layer, "Big cities" ``` `add_geojson` also takes any object with `__geo_interface__` (a Shapely geometry, a GeoSeries). diff --git a/docs/user-guide/sharing.md b/docs/user-guide/sharing.md index b8bdb3fab..129b08024 100644 --- a/docs/user-guide/sharing.md +++ b/docs/user-guide/sharing.md @@ -28,7 +28,9 @@ added, or one feature or attribute table of that name is replaced with its spati single transaction, so a failed export changes nothing. A name already used is refused when adding (with no file there, adding makes a new GeoPackage). Replacing needs a feature or attribute table of that name, so a misspelt name changes nothing; tiles, views and other tables -are never replaced. A replaced table's own indexes, triggers and column descriptions go with it. +are never replaced, nor is a table in a related-tables relation. A replaced table's own indexes, +triggers and column descriptions go with it, and the export names any views or triggers elsewhere +that use it. The GeoPackage's own triggers don't run while the export writes. From the command line, `share.exportLayer` and `share.exportTable` take `"mode": "addTable"` or `"replaceTable"` and an optional `"tableName"` (default: the layer's or table's name): From 21b22e1b946a1f4d81487d5c93be1abc5dc46c50 Mon Sep 17 00:00:00 2001 From: mavrick Date: Tue, 3 Nov 2026 17:53:30 -0600 Subject: [PATCH 03/10] fix(data): drop only a real spatial index when replacing a GeoPackage table A table named as the replaced table's spatial index but not an R-tree is now kept, and the replace refused, rather than dropped. The docs say that a layer read in place from the GeoPackage it goes into is read whole first; the UI and data tests compare the file's bytes after a cancelled pick and after each refusal. Co-Authored-By: Claude Opus 5.5 --- crates/data/src/source/gpkg/add.rs | 16 ++++++++++++++-- crates/data/src/source/tests.rs | 18 +++++++++++++++++- crates/ui-egui/src/lib.rs | 3 ++- docs/user-guide/data-formats.md | 2 +- docs/user-guide/sharing.md | 4 +++- 5 files changed, 37 insertions(+), 6 deletions(-) diff --git a/crates/data/src/source/gpkg/add.rs b/crates/data/src/source/gpkg/add.rs index dd402efb9..abaa2b510 100644 --- a/crates/data/src/source/gpkg/add.rs +++ b/crates/data/src/source/gpkg/add.rs @@ -226,7 +226,8 @@ fn with_article(kind: &str) -> String { } /// Drops feature or attribute table `table`, its spatial indexes, and its rows in the GeoPackage -/// tables that describe it (the optional ones where the file has them). +/// tables that describe it (the optional ones where the file has them). A table named as its +/// spatial index that isn't an R-tree is left, and the new table's index then refused. fn drop_table(conn: &Connection, table: &str) -> Result<()> { let has = |t: &str| object(conn, t).map(|k| k.as_deref() == Some("table")); let mut sql = String::new(); @@ -234,7 +235,18 @@ fn drop_table(conn: &Connection, table: &str) -> Result<()> { let mut st = conn.prepare("SELECT column_name FROM gpkg_geometry_columns WHERE lower(table_name) = lower(?1)").map_err(err)?; let columns = st.query_map([table], |r| r.get::<_, String>(0)).map_err(err)?.collect::>>().map_err(err)?; for c in columns { - sql.push_str(&format!("DROP TABLE IF EXISTS {};", quote(&format!("rtree_{table}_{c}")))); + let index = format!("rtree_{table}_{c}"); + let rtree: bool = conn + .query_row( + "SELECT EXISTS (SELECT 1 FROM sqlite_master WHERE type = 'table' AND lower(name) = lower(?1) + AND lower(sql) LIKE 'create virtual table%using rtree%')", + [&index], + |r| r.get(0), + ) + .map_err(err)?; + if rtree { + sql.push_str(&format!("DROP TABLE {};", quote(&index))); + } } } sql.push_str(&format!("DROP TABLE {};", quote(table))); diff --git a/crates/data/src/source/tests.rs b/crates/data/src/source/tests.rs index 9d9872583..567394586 100644 --- a/crates/data/src/source/tests.rs +++ b/crates/data/src/source/tests.rs @@ -2282,15 +2282,17 @@ fn geopackage_export_adds_and_replaces_tables_and_keeps_the_others() { // A name taken (whatever its case) is refused unless replacing, and the file is unchanged. let before = std::fs::read(&path).unwrap(); + let mut before_index = before.clone(); for name in [None, Some("PERMITS"), Some("owners_idx")] { if name == Some("owners_idx") { assert_eq!(std::fs::read(&path).unwrap(), before, "the refusals changed nothing"); rusqlite::Connection::open(&path).unwrap().execute_batch("CREATE INDEX owners_idx ON owners (name)").unwrap(); + before_index = std::fs::read(&path).unwrap(); } let e = write_path_to(&permits, &path, Crs::LonLat, &to(GpkgMode::AddTable, name)).unwrap_err().to_string(); assert!(e.contains("already has"), "{name:?}: {e}"); } - let before_index = std::fs::read(&path).unwrap(); + assert_eq!(std::fs::read(&path).unwrap(), before_index, "the index refusal changed nothing"); assert_ne!(before, before_index, "the index was added"); // Replacing: fewer features, in another system, and its old spatial index gone. A view on the @@ -2383,6 +2385,20 @@ fn geopackage_export_into_a_file_refuses_what_it_cannot_replace() { .unwrap(); let e = write_path_to(&ds, &path, Crs::LonLat, &replace("permits")).unwrap_err().to_string(); assert!(e.contains("related-tables relation"), "{e}"); + // An ordinary table named as the spatial index is kept, and the replace refused. + let squatted = workspace.path().join("squatted.gpkg"); + write_path_to(&ds, &squatted, Crs::LonLat, &GpkgTable { mode: GpkgMode::AddTable, name: Some("sites".into()) }).unwrap(); + rusqlite::Connection::open(&squatted) + .unwrap() + .execute_batch( + "DROP TABLE rtree_sites_geom; CREATE TABLE rtree_sites_geom (note TEXT); INSERT INTO rtree_sites_geom VALUES ('keep');", + ) + .unwrap(); + let e = write_path_to(&ds, &squatted, Crs::LonLat, &replace("sites")).unwrap_err().to_string(); + assert!(e.contains("rtree_sites_geom"), "{e}"); + let kept: String = + rusqlite::Connection::open(&squatted).unwrap().query_row("SELECT note FROM rtree_sites_geom", [], |r| r.get(0)).unwrap(); + assert_eq!(kept, "keep"); let other = workspace.path().join("other.gpkg"); std::fs::copy(&path, &other).unwrap(); rusqlite::Connection::open(&other).unwrap().execute_batch("PRAGMA application_id = 1297105496").unwrap(); diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 0ebfd26ec..db5a9e8de 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -4526,8 +4526,9 @@ mod download_service_tests { app.run("ui.exportLayer", json!({"format": "gpkg", "mode": "addTable", "tableName": "borders"})).unwrap(); assert_eq!(rustgis_engine::data::source::layers(&path).unwrap(), ["Cities", "borders"]); // Cancelling the pick writes nothing. + let before = std::fs::read(&path).unwrap(); app.run("ui.exportLayer", json!({"format": "gpkg", "mode": "replaceTable", "tableName": "Cities"})).unwrap(); - assert_eq!(rustgis_engine::data::source::layers(&path).unwrap(), ["Cities", "borders"]); + assert_eq!(std::fs::read(&path).unwrap(), before); } #[test] diff --git a/docs/user-guide/data-formats.md b/docs/user-guide/data-formats.md index 76f347d2e..3dfcb4765 100644 --- a/docs/user-guide/data-formats.md +++ b/docs/user-guide/data-formats.md @@ -474,7 +474,7 @@ were rounded, and Export Raster, which writes the rounded values, says so again. | Format | How to write it | Notes | | --- | --- | --- | -| GeoJSON (`.geojson`, `.json`), CSV | Data › Export (`share.exportLayer`) | The Export Features dialog picks the format; the chosen extension is appended when needed. In the web app it downloads any vector format below except GeoPackage and FlatGeobuf (a Shapefile as one zip). A streamed layer exports its whole source: on the desktop, to GeoJSON, CSV, FlatGeobuf, GeoPackage or GeoParquet a chunk at a time with no limit on its size (a GeoParquet source's row groups are read whole, so on the desktop each holds at most 16,777,216 rows), otherwise loaded whole (see [Streamed layers](data.md)); remote sources on the web must be downloaded first. | +| GeoJSON (`.geojson`, `.json`), CSV | Data › Export (`share.exportLayer`) | The Export Features dialog picks the format; the chosen extension is appended when needed. In the web app it downloads any vector format below except GeoPackage and FlatGeobuf (a Shapefile as one zip). A streamed layer exports its whole source: on the desktop, to GeoJSON, CSV, FlatGeobuf, GeoPackage or GeoParquet a chunk at a time with no limit on its size (a GeoParquet source's row groups are read whole, so on the desktop each holds at most 16,777,216 rows; a layer read in place from the GeoPackage it goes into is read whole first), otherwise loaded whole (see [Streamed layers](data.md)); remote sources on the web must be downloaded first. | | GeoPackage | Data › Export to `.gpkg` (desktop) | A new file with one feature table (an attributes table for a layer without geometry) in the layer's coordinate system (see below), with an R-tree and GeoPackage 1.4 triggers to maintain it. An existing destination is refused unless the export adds a table to it or replaces one of its feature or attribute tables (`mode`, see [Exporting](sharing.md)), leaving its other tables as they were. Unique feature ids that fit signed 64-bit integers are kept; otherwise SQLite assigns new ids. | | GeoParquet (`.parquet`, `.geoparquet`) | Data › Export | GeoParquet 1.1: WKB geometry in WGS 84, Snappy compression, row groups of up to 65,536 features and a `bbox` covering column. A standalone table without geometry writes plain Parquet. | | Shapefile | Data › Export to `.shp` (or `.zip` for the files zipped) | Writes `.shp`, `.shx`, `.dbf`, `.prj` (the coordinate system, see below) and `.cpg` (UTF-8). Field names are cut to 10 bytes (made unique ignoring case, and reported), text to at most 254 bytes, and numbers that do not fit their dBase field are left empty. Export notices count shortened text, non-null values left empty (including incompatible types), and numbers rounded to the field's precision, and name the affected source fields; features without geometry, and lines of a single point, are left out (and reported). | diff --git a/docs/user-guide/sharing.md b/docs/user-guide/sharing.md index 129b08024..b315080cf 100644 --- a/docs/user-guide/sharing.md +++ b/docs/user-guide/sharing.md @@ -30,7 +30,9 @@ adding (with no file there, adding makes a new GeoPackage). Replacing needs a fe attribute table of that name, so a misspelt name changes nothing; tiles, views and other tables are never replaced, nor is a table in a related-tables relation. A replaced table's own indexes, triggers and column descriptions go with it, and the export names any views or triggers elsewhere -that use it. The GeoPackage's own triggers don't run while the export writes. +that use it. The GeoPackage's own triggers don't run while the export writes. A layer read in +place from the GeoPackage it is exported into is read whole first, rather than a chunk at a +time, as SQLite can't read and write one file in the same export. From the command line, `share.exportLayer` and `share.exportTable` take `"mode": "addTable"` or `"replaceTable"` and an optional `"tableName"` (default: the layer's or table's name): From 8f839d4af70a5d55f1f5a17f7eed595546193712 Mon Sep 17 00:00:00 2001 From: mavrick Date: Tue, 3 Nov 2026 21:25:40 -0600 Subject: [PATCH 04/10] fix(engine): reopen layers reading a replaced GeoPackage table A layer read in place from a table an export replaces now reopens its source (fields, geometry type and system read again) and the export says how many did; save-back still only resets the view. A table whose name another table already uses as identifier gets a numbered identifier rather than none. Only an R-tree (not rtree_i32) counts as the spatial index. Co-Authored-By: Claude Opus 5.5 --- crates/data/src/source/gpkg.rs | 30 +++++++++++++++++---- crates/data/src/source/gpkg/add.rs | 2 +- crates/data/src/source/tests.rs | 2 ++ crates/engine/src/cmd/gpkg.rs | 43 ++++++++++++++++++++---------- crates/engine/src/cmd/share.rs | 8 +++++- crates/engine/src/tests.rs | 11 ++++++++ crates/ui-egui/src/lib.rs | 2 ++ 7 files changed, 77 insertions(+), 21 deletions(-) diff --git a/crates/data/src/source/gpkg.rs b/crates/data/src/source/gpkg.rs index cb3325679..0dec2b6c5 100644 --- a/crates/data/src/source/gpkg.rs +++ b/crates/data/src/source/gpkg.rs @@ -938,8 +938,8 @@ fn finish(conn: &Connection, layout: &Layout, extent: Extent) -> Result<()> { conn.execute_batch("COMMIT;").map_err(err) } -/// Records the table in the GeoPackage's contents with its `extent` (named by its table name -/// unless another table already has that identifier) and adds the triggers that keep its +/// Records the table in the GeoPackage's contents with its `extent` (identified by its table +/// name, numbered if another table already has that identifier) and adds the triggers that keep its /// spatial index in step with edits. fn register(conn: &Connection, layout: &Layout, extent: Extent) -> Result<()> { let Layout { table, geom, has_geom, geom_type, srs_id, table_q: t, geom_q: g, fid_q: i, rtree_q: rt, .. } = layout; @@ -947,11 +947,31 @@ fn register(conn: &Connection, layout: &Layout, extent: Extent) -> Result<()> { let e = extent; let bounds: [Option; 4] = if e.is_empty() { [None; 4] } else { [Some(e.min_x), Some(e.min_y), Some(e.max_x), Some(e.max_y)] }; let srs: Option = has_geom.then_some(i64::from(srs_id)); + // The table's name, numbered when another table of an existing GeoPackage has it as its + // (unique) identifier. + let mut identifier = table.clone(); + for i in 2.. { + let taken: bool = conn + .query_row("SELECT EXISTS (SELECT 1 FROM gpkg_contents WHERE identifier = ?1)", [&identifier], |r| r.get(0)) + .map_err(err)?; + if !taken { + break; + } + identifier = format!("{table}_{i}"); + } conn.execute( "INSERT INTO gpkg_contents (table_name, data_type, identifier, last_change, min_x, min_y, max_x, max_y, srs_id) - VALUES (?1, ?2, CASE WHEN EXISTS (SELECT 1 FROM gpkg_contents WHERE identifier = ?1) THEN NULL ELSE ?1 END, - strftime('%Y-%m-%dT%H:%M:%fZ', 'now'), ?3, ?4, ?5, ?6, ?7)", - rusqlite::params![table, if has_geom { "features" } else { "attributes" }, bounds[0], bounds[1], bounds[2], bounds[3], srs], + VALUES (?1, ?2, ?3, strftime('%Y-%m-%dT%H:%M:%fZ', 'now'), ?4, ?5, ?6, ?7, ?8)", + rusqlite::params![ + table, + if has_geom { "features" } else { "attributes" }, + identifier, + bounds[0], + bounds[1], + bounds[2], + bounds[3], + srs + ], ) .map_err(err)?; if has_geom { diff --git a/crates/data/src/source/gpkg/add.rs b/crates/data/src/source/gpkg/add.rs index abaa2b510..f3ff3db6a 100644 --- a/crates/data/src/source/gpkg/add.rs +++ b/crates/data/src/source/gpkg/add.rs @@ -239,7 +239,7 @@ fn drop_table(conn: &Connection, table: &str) -> Result<()> { let rtree: bool = conn .query_row( "SELECT EXISTS (SELECT 1 FROM sqlite_master WHERE type = 'table' AND lower(name) = lower(?1) - AND lower(sql) LIKE 'create virtual table%using rtree%')", + AND (lower(sql) LIKE 'create virtual table%using rtree(%' OR lower(sql) LIKE 'create virtual table%using rtree (%'))", [&index], |r| r.get(0), ) diff --git a/crates/data/src/source/tests.rs b/crates/data/src/source/tests.rs index 567394586..71e730461 100644 --- a/crates/data/src/source/tests.rs +++ b/crates/data/src/source/tests.rs @@ -2394,8 +2394,10 @@ fn geopackage_export_into_a_file_refuses_what_it_cannot_replace() { "DROP TABLE rtree_sites_geom; CREATE TABLE rtree_sites_geom (note TEXT); INSERT INTO rtree_sites_geom VALUES ('keep');", ) .unwrap(); + let before = std::fs::read(&squatted).unwrap(); let e = write_path_to(&ds, &squatted, Crs::LonLat, &replace("sites")).unwrap_err().to_string(); assert!(e.contains("rtree_sites_geom"), "{e}"); + assert_eq!(std::fs::read(&squatted).unwrap(), before, "the failed replace changed nothing"); let kept: String = rusqlite::Connection::open(&squatted).unwrap().query_row("SELECT note FROM rtree_sites_geom", [], |r| r.get(0)).unwrap(); assert_eq!(kept, "keep"); diff --git a/crates/engine/src/cmd/gpkg.rs b/crates/engine/src/cmd/gpkg.rs index 0c3959dfb..a22311d32 100644 --- a/crates/engine/src/cmd/gpkg.rs +++ b/crates/engine/src/cmd/gpkg.rs @@ -132,6 +132,34 @@ fn add_layer(s: &mut Session, p: &Value) -> CmdResult { } } +/// Makes every layer read in place from `table` of the GeoPackage at `path` (or from that file +/// with no table named) read it again, in every map: its rows were written, or (with `reopen`) +/// the table was made anew, so its fields and system are read again too. Returns how many. +pub(crate) fn reset_sources(s: &mut Session, path: &std::path::Path, table: &str, reopen: bool) -> usize { + let mut n = 0; + let canonical = std::fs::canonicalize(path).ok(); + for map in &mut s.project.maps { + for l in map.layers.iter_mut() { + if let Some(source) = &mut l.source + && source.table.as_deref().is_none_or(|t| t.eq_ignore_ascii_case(table)) + && source.path.as_deref().is_some_and(|p| match (canonical.as_ref(), std::fs::canonicalize(p).ok()) { + (Some(a), Some(b)) => *a == b, + _ => std::path::Path::new(p) == path, + }) + { + n += 1; + if reopen { + // A table that can't be opened now says so when the layer is next drawn. + let _ = source.reopen(); + } else { + source.reset(); + } + } + } + } + n +} + fn save_edits(s: &mut Session, p: &Value) -> CmdResult { #[cfg(target_arch = "wasm32")] { @@ -242,20 +270,7 @@ fn save_edits(s: &mut Session, p: &Value) -> CmdResult { } s.saved_over.retain(rustgis_map::source::Versions::held_elsewhere); s.saved_over.push(edit.versions); - let canonical = std::fs::canonicalize(path).ok(); - for map in &mut s.project.maps { - for l in map.layers.iter_mut() { - if let Some(source) = &mut l.source - && source.table.as_deref().is_none_or(|t| t.eq_ignore_ascii_case(table)) - && source.path.as_deref().is_some_and(|p| match (canonical.as_ref(), std::fs::canonicalize(p).ok()) { - (Some(a), Some(b)) => *a == b, - _ => p == edit.path, - }) - { - source.reset(); - } - } - } + reset_sources(s, path, table, false); s.notify( format!( "{name}: saved to {table} in {}: {} new, {} changed, {} deleted", diff --git a/crates/engine/src/cmd/share.rs b/crates/engine/src/cmd/share.rs index c13bf938f..002bc59c7 100644 --- a/crates/engine/src/cmd/share.rs +++ b/crates/engine/src/cmd/share.rs @@ -74,10 +74,12 @@ fn gpkg_table(p: &Value, ext: &str) -> Result { /// Puts the table a GeoPackage export wrote, and whether it replaced one, in its result `out`; /// an export into an existing GeoPackage also says so, what went with a replaced table, and -/// which views and triggers elsewhere name it. +/// which views and triggers elsewhere name it. Layers read in place from a replaced table read +/// it again. fn report_table(s: &mut Session, path: &str, gpkg: &GpkgTable, table: Option, out: &mut Value) { let Some(table) = table else { return }; if gpkg.mode != GpkgMode::NewFile { + let reread = if table.replaced { super::gpkg::reset_sources(s, Path::new(path), &table.table, true) } else { 0 }; let note = if table.replaced { format!( "{path}: replaced its table {}, dropping the old table's indexes, triggers, column descriptions, metadata links and extension records", @@ -87,6 +89,10 @@ fn report_table(s: &mut Session, path: &str, gpkg: &GpkgTable, table: Option 0 { + let layers = if reread == 1 { "layer reads" } else { "layers read" }; + s.notify(format!("{path}: {reread} {layers} {} in place, now the new table", table.table), false); + } if !table.dependents.is_empty() { let n = table.dependents.len(); s.notify( diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 82eff1130..606c5a462 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -12264,6 +12264,17 @@ fn exporting_features_adds_or_replaces_a_table_in_an_existing_geopackage() { s.run("layer.addData", &json!({"path": path.display().to_string(), "layer": "Countries", "stream": true, "name": "InPlace"})).unwrap(); s.run("share.exportLayer", &json!({"layer": "InPlace", "path": path, "mode": "addTable", "tableName": "copy"})).unwrap(); assert_eq!(count("copy"), Some(countries as u64)); + // Replacing the table a layer reads in place makes it read the new table, and says so. + let kind = + |s: &Session| s.map().layer_by_name("InPlace").and_then(|l| l.source.as_ref()).and_then(|src| src.info()).map(|i| i.geometry_type); + s.map_mut().layers.iter_mut().for_each(|l| { + l.sync_source(&rustgis_geom::Extent::new(-20.0, -20.0, 20.0, 20.0), true); + }); + assert_eq!(kind(&s), Some(rustgis_data::GeometryType::Polygon)); + let mark = s.notice_mark(); + s.run("share.exportLayer", &json!({"layer": "Cities", "path": path, "mode": "replaceTable", "tableName": "Countries"})).unwrap(); + assert_eq!(kind(&s), Some(rustgis_data::GeometryType::Point), "the new table's"); + assert!(s.notices_since(mark).iter().any(|n| n.text.ends_with("1 layer reads Countries in place, now the new table"))); // A standalone table goes in as an attribute table. s.run("layer.addDataText", &json!({"fileName": "owners.csv", "text": "name,n\nShelby,1\n"})).unwrap(); let table = s.project.tables.last().map(|t| t.id).unwrap(); diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index db5a9e8de..4f65d4189 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -4525,6 +4525,8 @@ mod download_service_tests { crate::dialogs::PICKED.with(|p| *p.borrow_mut() = Some(path.display().to_string())); app.run("ui.exportLayer", json!({"format": "gpkg", "mode": "addTable", "tableName": "borders"})).unwrap(); assert_eq!(rustgis_engine::data::source::layers(&path).unwrap(), ["Cities", "borders"]); + let countries = app.session.map().layer_by_name("Countries").map(|l| l.dataset.features.len() as u64); + assert_eq!(rustgis_engine::data::source::open_path(&path, Some("borders")).unwrap().info().count, countries); // Cancelling the pick writes nothing. let before = std::fs::read(&path).unwrap(); app.run("ui.exportLayer", json!({"format": "gpkg", "mode": "replaceTable", "tableName": "Cities"})).unwrap(); From 88c8a1f908e7a6bb2142f7a93c8fedcfa675d703 Mon Sep 17 00:00:00 2001 From: mavrick Date: Tue, 3 Nov 2026 23:58:45 -0600 Subject: [PATCH 05/10] fix(data): refuse replacing a GeoPackage table others refer to by foreign key SQLite enforces foreign keys here, so dropping a table that another refers to would delete or empty that table's rows: such a replace is now refused, naming the tables. A layer that can't reopen a replaced table is reported, not counted as reading it. The spatial index is recognised however its declaration is spaced or quoted, still excluding rtree_i32. Co-Authored-By: Claude Opus 5.5 --- crates/data/src/source/gpkg/add.rs | 25 ++++++++++++++++++++++++- crates/data/src/source/tests.rs | 26 ++++++++++++++++++++++++++ crates/engine/src/cmd/gpkg.rs | 21 ++++++++++++--------- crates/engine/src/cmd/share.rs | 13 ++++++++++++- crates/engine/src/tests.rs | 2 ++ 5 files changed, 76 insertions(+), 11 deletions(-) diff --git a/crates/data/src/source/gpkg/add.rs b/crates/data/src/source/gpkg/add.rs index f3ff3db6a..47d847c48 100644 --- a/crates/data/src/source/gpkg/add.rs +++ b/crates/data/src/source/gpkg/add.rs @@ -173,6 +173,13 @@ fn make_room(conn: &Connection, file: &str, table: &str, replace: bool) -> Resul "{file} already has a table {old}: choose another table name, or replace that table" ))); } + let children = referencing(conn, &old)?; + if !children.is_empty() { + return Err(DataError::Format(format!( + "{file}: {} refer to {old} by foreign key, whose rows a replaced table would delete or break; choose another table name", + children.join(", ") + ))); + } if related(conn, &old)? { return Err(DataError::Format(format!( "{file}: {old} is in a related-tables relation, whose links a replaced table would break; choose another table name" @@ -192,6 +199,19 @@ fn make_room(conn: &Connection, file: &str, table: &str, replace: bool) -> Resul } } +/// The other tables with a foreign key to `table`. Dropping it would delete or empty their rows +/// (SQLite enforces foreign keys here), or leave them pointing at rows that aren't the same. +fn referencing(conn: &Connection, table: &str) -> Result> { + let mut st = conn + .prepare( + "SELECT DISTINCT m.name FROM sqlite_master m, pragma_foreign_key_list(m.name) f + WHERE m.type = 'table' AND lower(m.name) != lower(?1) AND lower(f.\"table\") = lower(?1) ORDER BY m.name", + ) + .map_err(err)?; + let names = st.query_map([table], |r| r.get::<_, String>(0)).map_err(err)?; + names.collect::>>().map_err(err) +} + /// Whether the GeoPackage's related-tables extension names `table` as a base, related or mapping /// table: its rows are linked by id, which a new table's rows don't keep. fn related(conn: &Connection, table: &str) -> Result { @@ -238,8 +258,11 @@ fn drop_table(conn: &Connection, table: &str) -> Result<()> { let index = format!("rtree_{table}_{c}"); let rtree: bool = conn .query_row( + // Its declaration without spaces, line breaks and quotes, so `USING "rtree" (` + // counts and `USING rtree_i32(` doesn't. "SELECT EXISTS (SELECT 1 FROM sqlite_master WHERE type = 'table' AND lower(name) = lower(?1) - AND (lower(sql) LIKE 'create virtual table%using rtree(%' OR lower(sql) LIKE 'create virtual table%using rtree (%'))", + AND replace(replace(replace(replace(replace(replace(lower(sql), ' ', ''), char(9), ''), char(10), ''), char(13), ''), '\"', ''), '`', '') + LIKE 'createvirtualtable%usingrtree(%')", [&index], |r| r.get(0), ) diff --git a/crates/data/src/source/tests.rs b/crates/data/src/source/tests.rs index 71e730461..7782b40f5 100644 --- a/crates/data/src/source/tests.rs +++ b/crates/data/src/source/tests.rs @@ -2385,6 +2385,19 @@ fn geopackage_export_into_a_file_refuses_what_it_cannot_replace() { .unwrap(); let e = write_path_to(&ds, &path, Crs::LonLat, &replace("permits")).unwrap_err().to_string(); assert!(e.contains("related-tables relation"), "{e}"); + // Nor is a table others refer to by foreign key: their rows would be deleted with it. + rusqlite::Connection::open(&path) + .unwrap() + .execute_batch( + "DELETE FROM gpkgext_relations; + CREATE TABLE inspections (id INTEGER PRIMARY KEY, permit INTEGER REFERENCES permits (fid) ON DELETE CASCADE); + INSERT INTO inspections VALUES (1, 1);", + ) + .unwrap(); + let e = write_path_to(&ds, &path, Crs::LonLat, &replace("permits")).unwrap_err().to_string(); + assert!(e.contains("inspections refer to permits"), "{e}"); + let kept: i64 = rusqlite::Connection::open(&path).unwrap().query_row("SELECT count(*) FROM inspections", [], |r| r.get(0)).unwrap(); + assert_eq!(kept, 1); // An ordinary table named as the spatial index is kept, and the replace refused. let squatted = workspace.path().join("squatted.gpkg"); write_path_to(&ds, &squatted, Crs::LonLat, &GpkgTable { mode: GpkgMode::AddTable, name: Some("sites".into()) }).unwrap(); @@ -2401,6 +2414,19 @@ fn geopackage_export_into_a_file_refuses_what_it_cannot_replace() { let kept: String = rusqlite::Connection::open(&squatted).unwrap().query_row("SELECT note FROM rtree_sites_geom", [], |r| r.get(0)).unwrap(); assert_eq!(kept, "keep"); + // A spatial index declared over lines, with the module quoted, is still the index; an + // `rtree_i32` table of that name isn't. + for (module, replaced) in [("\n \"rtree\" (", true), ("rtree_i32(", false)] { + let file = workspace.path().join(format!("index-{replaced}.gpkg")); + write_path_to(&ds, &file, Crs::LonLat, &GpkgTable { mode: GpkgMode::AddTable, name: Some("sites".into()) }).unwrap(); + rusqlite::Connection::open(&file) + .unwrap() + .execute_batch(&format!( + "DROP TABLE rtree_sites_geom; CREATE VIRTUAL TABLE rtree_sites_geom USING {module}id, minx, maxx, miny, maxy);" + )) + .unwrap(); + assert_eq!(write_path_to(&ds, &file, Crs::LonLat, &replace("sites")).is_ok(), replaced, "{module:?}"); + } let other = workspace.path().join("other.gpkg"); std::fs::copy(&path, &other).unwrap(); rusqlite::Connection::open(&other).unwrap().execute_batch("PRAGMA application_id = 1297105496").unwrap(); diff --git a/crates/engine/src/cmd/gpkg.rs b/crates/engine/src/cmd/gpkg.rs index a22311d32..800b2e090 100644 --- a/crates/engine/src/cmd/gpkg.rs +++ b/crates/engine/src/cmd/gpkg.rs @@ -134,9 +134,10 @@ fn add_layer(s: &mut Session, p: &Value) -> CmdResult { /// Makes every layer read in place from `table` of the GeoPackage at `path` (or from that file /// with no table named) read it again, in every map: its rows were written, or (with `reopen`) -/// the table was made anew, so its fields and system are read again too. Returns how many. -pub(crate) fn reset_sources(s: &mut Session, path: &std::path::Path, table: &str, reopen: bool) -> usize { - let mut n = 0; +/// the table was made anew, so its fields and system are read again too. Returns how many read +/// it again, and the layers that couldn't reopen it with why (each also keeps its error). +pub(crate) fn reset_sources(s: &mut Session, path: &std::path::Path, table: &str, reopen: bool) -> (usize, Vec) { + let (mut n, mut failed) = (0, Vec::new()); let canonical = std::fs::canonicalize(path).ok(); for map in &mut s.project.maps { for l in map.layers.iter_mut() { @@ -147,17 +148,19 @@ pub(crate) fn reset_sources(s: &mut Session, path: &std::path::Path, table: &str _ => std::path::Path::new(p) == path, }) { - n += 1; - if reopen { - // A table that can't be opened now says so when the layer is next drawn. - let _ = source.reopen(); - } else { + if !reopen { source.reset(); + n += 1; + } else { + match source.reopen() { + Ok(()) => n += 1, + Err(e) => failed.push(format!("{} ({e})", l.name)), + } } } } } - n + (n, failed) } fn save_edits(s: &mut Session, p: &Value) -> CmdResult { diff --git a/crates/engine/src/cmd/share.rs b/crates/engine/src/cmd/share.rs index 002bc59c7..ee52ffd4c 100644 --- a/crates/engine/src/cmd/share.rs +++ b/crates/engine/src/cmd/share.rs @@ -79,7 +79,8 @@ fn gpkg_table(p: &Value, ext: &str) -> Result { fn report_table(s: &mut Session, path: &str, gpkg: &GpkgTable, table: Option, out: &mut Value) { let Some(table) = table else { return }; if gpkg.mode != GpkgMode::NewFile { - let reread = if table.replaced { super::gpkg::reset_sources(s, Path::new(path), &table.table, true) } else { 0 }; + let (reread, failed) = + if table.replaced { super::gpkg::reset_sources(s, Path::new(path), &table.table, true) } else { (0, Vec::new()) }; let note = if table.replaced { format!( "{path}: replaced its table {}, dropping the old table's indexes, triggers, column descriptions, metadata links and extension records", @@ -93,6 +94,16 @@ fn report_table(s: &mut Session, path: &str, gpkg: &GpkgTable, table: Option Date: Wed, 4 Nov 2026 03:16:00 -0600 Subject: [PATCH 06/10] fix(ui): export the layer Export Features was opened on The dialog keeps its layer, so selecting another while it is open no longer makes a replace write that layer's features; ui.exportLayer refuses mode and tableName for other formats and passes tableName on as given. A replace names the layers loaded for editing that still show the old rows. The spatial index is recognised with a bracket-quoted module too. Co-Authored-By: Claude Opus 5.5 --- crates/data/src/source/gpkg/add.rs | 4 +-- crates/data/src/source/tests.rs | 4 +-- crates/engine/src/cmd/gpkg.rs | 15 ++++++++++ crates/engine/src/cmd/share.rs | 11 +++++++ crates/engine/src/tests.rs | 11 +++++++ crates/ui-egui/src/dialogs.rs | 8 +++++ crates/ui-egui/src/lib.rs | 48 ++++++++++++++++++++---------- 7 files changed, 81 insertions(+), 20 deletions(-) diff --git a/crates/data/src/source/gpkg/add.rs b/crates/data/src/source/gpkg/add.rs index 47d847c48..cbd923f85 100644 --- a/crates/data/src/source/gpkg/add.rs +++ b/crates/data/src/source/gpkg/add.rs @@ -258,10 +258,10 @@ fn drop_table(conn: &Connection, table: &str) -> Result<()> { let index = format!("rtree_{table}_{c}"); let rtree: bool = conn .query_row( - // Its declaration without spaces, line breaks and quotes, so `USING "rtree" (` + // Its declaration without spaces, line breaks and quotes of any kind, so `USING "rtree" (` // counts and `USING rtree_i32(` doesn't. "SELECT EXISTS (SELECT 1 FROM sqlite_master WHERE type = 'table' AND lower(name) = lower(?1) - AND replace(replace(replace(replace(replace(replace(lower(sql), ' ', ''), char(9), ''), char(10), ''), char(13), ''), '\"', ''), '`', '') + AND replace(replace(replace(replace(replace(replace(replace(replace(lower(sql), ' ', ''), char(9), ''), char(10), ''), char(13), ''), '\"', ''), '`', ''), '[', ''), ']', '') LIKE 'createvirtualtable%usingrtree(%')", [&index], |r| r.get(0), diff --git a/crates/data/src/source/tests.rs b/crates/data/src/source/tests.rs index f71781dbb..54b818764 100644 --- a/crates/data/src/source/tests.rs +++ b/crates/data/src/source/tests.rs @@ -2519,8 +2519,8 @@ fn geopackage_export_into_a_file_refuses_what_it_cannot_replace() { assert_eq!(kept, "keep"); // A spatial index declared over lines, with the module quoted, is still the index; an // `rtree_i32` table of that name isn't. - for (module, replaced) in [("\n \"rtree\" (", true), ("rtree_i32(", false)] { - let file = workspace.path().join(format!("index-{replaced}.gpkg")); + for (module, replaced) in [("\n \"rtree\" (", true), ("[rtree](", true), ("rtree_i32(", false)] { + let file = workspace.path().join(format!("index-{}.gpkg", module.len())); write_path_to(&ds, &file, Crs::LonLat, &GpkgTable { mode: GpkgMode::AddTable, name: Some("sites".into()) }).unwrap(); rusqlite::Connection::open(&file) .unwrap() diff --git a/crates/engine/src/cmd/gpkg.rs b/crates/engine/src/cmd/gpkg.rs index 800b2e090..c9404d408 100644 --- a/crates/engine/src/cmd/gpkg.rs +++ b/crates/engine/src/cmd/gpkg.rs @@ -163,6 +163,21 @@ pub(crate) fn reset_sources(s: &mut Session, path: &std::path::Path, table: &str (n, failed) } +/// The layers loaded for editing from `table` of the GeoPackage at `path`, by name, in every map: +/// after the table is replaced they still hold its old rows (with their unsaved edits). +pub(crate) fn editing(s: &Session, path: &std::path::Path, table: &str) -> Vec { + let canonical = std::fs::canonicalize(path).ok(); + let same = |p: &str| match (canonical.as_ref(), std::fs::canonicalize(p).ok()) { + (Some(a), Some(b)) => *a == b, + _ => std::path::Path::new(p) == path, + }; + let layers = s.project.maps.iter().flat_map(|m| m.layers.iter()); + layers + .filter(|l| l.gpkg.as_ref().is_some_and(|g| g.table.eq_ignore_ascii_case(table) && same(&g.path))) + .map(|l| l.name.clone()) + .collect() +} + fn save_edits(s: &mut Session, p: &Value) -> CmdResult { #[cfg(target_arch = "wasm32")] { diff --git a/crates/engine/src/cmd/share.rs b/crates/engine/src/cmd/share.rs index ee52ffd4c..7a9c688a3 100644 --- a/crates/engine/src/cmd/share.rs +++ b/crates/engine/src/cmd/share.rs @@ -94,6 +94,17 @@ fn report_table(s: &mut Session, path: &str, gpkg: &GpkgTable, table: Option, pub flag: bool, pub error: Option, pub info: Option, @@ -172,6 +175,7 @@ impl Dialogs { "exportLayer" => { self.mode = "geojson"; self.gpkg_mode = "newFile"; + self.export_layer = active.map(|l| (l.id, l.instance_id())); // The GeoPackage table's name. self.text = active.map(|l| l.name.clone()).unwrap_or_default(); } @@ -653,6 +657,10 @@ fn export_layer(app: &mut RustGisApp, ui: &mut egui::Ui) -> bool { } buttons(app, ui, if web { "Download" } else { "Export…" }, |app| { let mut p = json!({"format": app.dialogs.mode}); + if let Some((layer, instance)) = app.dialogs.export_layer { + p["layer"] = json!(layer); + p["instance"] = json!(instance); + } if into_existing { p["mode"] = json!(app.dialogs.gpkg_mode); p["tableName"] = json!(app.dialogs.text); diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index ec30e4af3..dd7652bcb 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -987,6 +987,20 @@ pub struct RustGisApp { } impl RustGisApp { + /// The layer to export: the one Export Features was opened on (`layer`, checked against its + /// `instance` so a reused id isn't taken for it), else the active layer. + fn export_target(&self, p: &Value) -> Result { + let Some(id) = p.get("layer").and_then(Value::as_u64) else { + return self.session.active_layer.ok_or_else(|| "select a layer in the Layers pane first".to_string()); + }; + let id = u32::try_from(id).map_err(|_| "no such layer".to_string())?; + let l = self.session.map().layer(id).ok_or("the layer Export Features was opened on is gone")?; + match p.get("instance").and_then(Value::as_u64) { + Some(instance) if instance != l.instance_id() => Err("the layer Export Features was opened on is gone".into()), + _ => Ok(id), + } + } + /// The Ctrl (⌘) shortcuts the keymap takes, as (Alt, Shift, key), the clipboard's left out: /// in a browser, what the web app keeps from the browser's own shortcuts (⌘D bookmarking /// the page, ⌘Y opening its history, ⌘= zooming it), as design tools in the browser do. @@ -3312,11 +3326,8 @@ impl RustGisApp { self.dialogs.open("exportLayer", &self.session)?; } "ui.exportLayer" if self.services.download.is_some() => { - let l = self - .session - .active_layer - .and_then(|id| self.session.map().layer(id)) - .ok_or("select a layer in the Layers pane first")?; + let id = self.export_target(p)?; + let l = self.session.map().layer(id).ok_or("select a layer in the Layers pane first")?; // Any format the dialog offers here (`dialogs::export_formats`): not GeoPackage or // FlatGeobuf, which the data crate refuses in the browser. let ext = p.get("format").and_then(Value::as_str).unwrap_or("geojson").to_ascii_lowercase(); @@ -3392,28 +3403,28 @@ impl RustGisApp { .ok_or_else(|| format!("unknown format '{ext}'"))?; // Into a GeoPackage that exists: picked to open, as it is kept, not replaced. let mode = p.get("mode").and_then(Value::as_str).unwrap_or("newFile"); + if ext != "gpkg" && (mode != "newFile" || p.get("tableName").is_some()) { + return Err(format!("mode and tableName are for a GeoPackage (.gpkg) export, not .{ext}")); + } + let id = self.export_target(p)?; if ext == "gpkg" && mode != "newFile" { if let Some(path) = dialogs::pick_open(&["gpkg"], label) { - let mut args = json!({"path": path, "mode": mode}); - if let Some(table) = p.get("tableName").and_then(Value::as_str) { - args["tableName"] = json!(table); + let mut args = json!({"layer": id, "path": path, "mode": mode}); + // Passed as given, so the command refuses one that isn't text. + if let Some(table) = p.get("tableName") { + args["tableName"] = table.clone(); } return self.run("share.exportLayer", args); } return Ok(Value::Null); } - let name = self - .session - .active_layer - .and_then(|id| self.session.map().layer(id)) - .map(|l| format!("{}.{ext}", l.name)) - .unwrap_or_default(); + let name = self.session.map().layer(id).map(|l| format!("{}.{ext}", l.name)).unwrap_or_default(); if let Some(path) = dialogs::pick_save(&name, &[ext.as_str()], label) { // The format chosen decides: its extension is added unless it was typed (a dot // in the name, as in "U.S. Cities", isn't taken for one). let typed = std::path::Path::new(&path).extension().is_some_and(|e| e.eq_ignore_ascii_case(&ext)); let path = if typed { path } else { format!("{path}.{ext}") }; - return self.run("share.exportLayer", json!({"path": path})); + return self.run("share.exportLayer", json!({"layer": id, "path": path})); } } // Add Zarr Variable, connected to a store (a link, a script). @@ -4565,8 +4576,13 @@ mod download_service_tests { let mut app = RustGisApp::with_sample(); app.run("share.exportLayer", json!({"layer": "Cities", "path": path})).unwrap(); app.session.active_layer = app.session.map().layer_by_name("Countries").map(|l| l.id); + app.run("ui.dialog", json!({"name": "exportLayer"})).unwrap(); + let (id, instance) = app.dialogs.export_layer.unwrap(); + // Another layer selected while the dialog is open: the dialog's is the one exported. + app.session.active_layer = app.session.map().layer_by_name("Cities").map(|l| l.id); crate::dialogs::PICKED.with(|p| *p.borrow_mut() = Some(path.display().to_string())); - app.run("ui.exportLayer", json!({"format": "gpkg", "mode": "addTable", "tableName": "borders"})).unwrap(); + let p = json!({"format": "gpkg", "mode": "addTable", "tableName": "borders", "layer": id, "instance": instance}); + app.run("ui.exportLayer", p).unwrap(); assert_eq!(rustgis_engine::data::source::layers(&path).unwrap(), ["Cities", "borders"]); let countries = app.session.map().layer_by_name("Countries").map(|l| l.dataset.features.len() as u64); assert_eq!(rustgis_engine::data::source::open_path(&path, Some("borders")).unwrap().info().count, countries); From 5bf6eab698a7d25a23c22020dabaa5ec10aaa0d6 Mon Sep 17 00:00:00 2001 From: mavrick Date: Wed, 4 Nov 2026 04:28:17 -0600 Subject: [PATCH 07/10] fix(data): keep foreign keys off while exporting into a GeoPackage The export's connection no longer enforces foreign keys, so deleting a replaced table's GeoPackage records can't cascade into a table referring to them (one referring to the replaced table itself is still refused). ui.exportLayer refuses a mode that isn't a string. Co-Authored-By: Claude Opus 5.5 --- crates/data/src/source/gpkg/add.rs | 3 +++ crates/data/src/source/tests.rs | 12 ++++++++++++ crates/ui-egui/src/lib.rs | 5 ++++- 3 files changed, 19 insertions(+), 1 deletion(-) diff --git a/crates/data/src/source/gpkg/add.rs b/crates/data/src/source/gpkg/add.rs index cbd923f85..a7f32f647 100644 --- a/crates/data/src/source/gpkg/add.rs +++ b/crates/data/src/source/gpkg/add.rs @@ -33,6 +33,9 @@ pub(super) fn write_features( // Nor may its triggers run when the export writes its GeoPackage records: one could change // other tables. The new table's own triggers are made after its rows are in. conn.set_db_config(DbConfig::SQLITE_DBCONFIG_ENABLE_TRIGGER, false).map_err(err)?; + // Nor may its foreign keys turn the export's deletes (of a replaced table's GeoPackage + // records) into deletes elsewhere; a table referring to the replaced one is refused instead. + conn.execute_batch("PRAGMA foreign_keys = OFF").map_err(err)?; conn.busy_timeout(Duration::from_secs(5)).map_err(err)?; let (mut multi_points, mut keep_ids) = (None, true); // Each choice changes at most once; an export starting again rolls back what it wrote. diff --git a/crates/data/src/source/tests.rs b/crates/data/src/source/tests.rs index 54b818764..531800ed6 100644 --- a/crates/data/src/source/tests.rs +++ b/crates/data/src/source/tests.rs @@ -2501,6 +2501,18 @@ fn geopackage_export_into_a_file_refuses_what_it_cannot_replace() { assert!(e.contains("inspections refer to permits"), "{e}"); let kept: i64 = rusqlite::Connection::open(&path).unwrap().query_row("SELECT count(*) FROM inspections", [], |r| r.get(0)).unwrap(); assert_eq!(kept, 1); + // A table referring to the GeoPackage's own records keeps its rows when a replace deletes them. + rusqlite::Connection::open(&path) + .unwrap() + .execute_batch( + "DROP TABLE inspections; + CREATE TABLE notes (layer TEXT REFERENCES gpkg_contents (table_name) ON DELETE CASCADE); + INSERT INTO notes VALUES ('permits');", + ) + .unwrap(); + write_path_to(&ds, &path, Crs::LonLat, &replace("permits")).unwrap(); + let notes: i64 = rusqlite::Connection::open(&path).unwrap().query_row("SELECT count(*) FROM notes", [], |r| r.get(0)).unwrap(); + assert_eq!(notes, 1); // An ordinary table named as the spatial index is kept, and the replace refused. let squatted = workspace.path().join("squatted.gpkg"); write_path_to(&ds, &squatted, Crs::LonLat, &GpkgTable { mode: GpkgMode::AddTable, name: Some("sites".into()) }).unwrap(); diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index dd7652bcb..ae021d9c5 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -3402,7 +3402,10 @@ impl RustGisApp { .map(|(l, _)| *l) .ok_or_else(|| format!("unknown format '{ext}'"))?; // Into a GeoPackage that exists: picked to open, as it is kept, not replaced. - let mode = p.get("mode").and_then(Value::as_str).unwrap_or("newFile"); + let mode = match p.get("mode") { + None | Some(Value::Null) => "newFile", + Some(v) => v.as_str().ok_or("mode must be a string")?, + }; if ext != "gpkg" && (mode != "newFile" || p.get("tableName").is_some()) { return Err(format!("mode and tableName are for a GeoPackage (.gpkg) export, not .{ext}")); } From 3215fc172a8f4deff2fb216b4d426dbe409193ed Mon Sep 17 00:00:00 2001 From: mavrick Date: Wed, 4 Nov 2026 05:40:10 -0600 Subject: [PATCH 08/10] fix(ui): check Export Features' layer and GeoPackage mode in the browser too ui.exportLayer takes its layer by id or name as commands do, refusing one that isn't there rather than exporting the active layer, and both its desktop and browser branches refuse mode and tableName for formats other than GeoPackage. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/lib.rs | 39 +++++++++++++++++++++++++++------------ 1 file changed, 27 insertions(+), 12 deletions(-) diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index ae021d9c5..4131c4a3f 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -987,13 +987,27 @@ pub struct RustGisApp { } impl RustGisApp { - /// The layer to export: the one Export Features was opened on (`layer`, checked against its - /// `instance` so a reused id isn't taken for it), else the active layer. + /// Export Features' GeoPackage `mode` (`newFile` when not given) for a file of extension `ext`: + /// it and `tableName` are refused for other formats, and a `mode` that isn't text, as the + /// command refuses them. + fn export_mode<'a>(p: &'a Value, ext: &str) -> Result<&'a str, String> { + let mode = match p.get("mode") { + None | Some(Value::Null) => "newFile", + Some(v) => v.as_str().ok_or("mode must be a string")?, + }; + if ext != "gpkg" && (mode != "newFile" || p.get("tableName").is_some()) { + return Err(format!("mode and tableName are for a GeoPackage (.gpkg) export, not .{ext}")); + } + Ok(mode) + } + + /// The layer to export: `layer` (an id or name, as commands take it; the dialog's is checked + /// against its `instance`, so a reused id isn't taken for it), else the active layer. fn export_target(&self, p: &Value) -> Result { - let Some(id) = p.get("layer").and_then(Value::as_u64) else { + if p.get("layer").is_none_or(Value::is_null) { return self.session.active_layer.ok_or_else(|| "select a layer in the Layers pane first".to_string()); - }; - let id = u32::try_from(id).map_err(|_| "no such layer".to_string())?; + } + let id = self.session.layer_id(p)?; let l = self.session.map().layer(id).ok_or("the layer Export Features was opened on is gone")?; match p.get("instance").and_then(Value::as_u64) { Some(instance) if instance != l.instance_id() => Err("the layer Export Features was opened on is gone".into()), @@ -3331,6 +3345,7 @@ impl RustGisApp { // Any format the dialog offers here (`dialogs::export_formats`): not GeoPackage or // FlatGeobuf, which the data crate refuses in the browser. let ext = p.get("format").and_then(Value::as_str).unwrap_or("geojson").to_ascii_lowercase(); + Self::export_mode(p, &ext)?; // A streamed layer exports its whole source, not the features read for the view. let whole = if l.is_streamed() { Some(l.load_all()?) } else { None }; // In the layer's own coordinate system, as share.exportLayer writes a file. @@ -3402,13 +3417,7 @@ impl RustGisApp { .map(|(l, _)| *l) .ok_or_else(|| format!("unknown format '{ext}'"))?; // Into a GeoPackage that exists: picked to open, as it is kept, not replaced. - let mode = match p.get("mode") { - None | Some(Value::Null) => "newFile", - Some(v) => v.as_str().ok_or("mode must be a string")?, - }; - if ext != "gpkg" && (mode != "newFile" || p.get("tableName").is_some()) { - return Err(format!("mode and tableName are for a GeoPackage (.gpkg) export, not .{ext}")); - } + let mode = Self::export_mode(p, &ext)?; let id = self.export_target(p)?; if ext == "gpkg" && mode != "newFile" { if let Some(path) = dialogs::pick_open(&["gpkg"], label) { @@ -4543,6 +4552,9 @@ mod download_service_tests { app }; let mut app = with(true); + // The browser's export refuses a GeoPackage mode for another format, as the desktop's does. + assert!(app.run("ui.exportLayer", json!({"format": "geojson", "mode": "replaceTable"})).is_err()); + assert_eq!(changed(&app), 0); app.run("ui.exportLayer", json!({"format": "shp"})).unwrap(); assert_eq!(changed(&app), 1); assert!(app.session.notices.iter().any(|n| n.text == "Sites.zip: 1 text value was cut to at most 254 bytes, in 'notes'")); @@ -4589,6 +4601,9 @@ mod download_service_tests { assert_eq!(rustgis_engine::data::source::layers(&path).unwrap(), ["Cities", "borders"]); let countries = app.session.map().layer_by_name("Countries").map(|l| l.dataset.features.len() as u64); assert_eq!(rustgis_engine::data::source::open_path(&path, Some("borders")).unwrap().info().count, countries); + // A layer by name is that layer; one not there is refused, not taken for the active one. + let e = app.run("ui.exportLayer", json!({"format": "gpkg", "mode": "addTable", "layer": "Nowhere"})).unwrap_err(); + assert!(e.contains("Nowhere"), "{e}"); // Cancelling the pick writes nothing. let before = std::fs::read(&path).unwrap(); app.run("ui.exportLayer", json!({"format": "gpkg", "mode": "replaceTable", "tableName": "Cities"})).unwrap(); From 60f43a03a7be298ff81bd5e0e839840ffc63244e Mon Sep 17 00:00:00 2001 From: mavrick Date: Wed, 4 Nov 2026 06:24:07 -0600 Subject: [PATCH 09/10] fix(ui): check Export Features' GeoPackage options as the command does ui.exportLayer checks mode and tableName with share.exportLayer's own parser before asking for a file (an unknown mode, a padded or empty value read as the command reads them) and passes them on as given, tableName with a new file too. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/cmd/share.rs | 4 +-- crates/ui-egui/src/lib.rs | 45 ++++++++++++++++------------------ 2 files changed, 23 insertions(+), 26 deletions(-) diff --git a/crates/engine/src/cmd/share.rs b/crates/engine/src/cmd/share.rs index 7a9c688a3..db7d8a83c 100644 --- a/crates/engine/src/cmd/share.rs +++ b/crates/engine/src/cmd/share.rs @@ -52,8 +52,8 @@ pub fn commands() -> Vec { } /// A GeoPackage export's table, from `mode` and `tableName`, which only a GeoPackage (extension -/// `ext`) takes. -fn gpkg_table(p: &Value, ext: &str) -> Result { +/// `ext`) takes; the front end checks a request with it before asking for a file. +pub fn gpkg_table(p: &Value, ext: &str) -> Result { // Not a string is refused, not taken for the default. let text = |key: &str| match p.get(key) { None | Some(Value::Null) => Ok(None), diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 4131c4a3f..00841a400 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -987,20 +987,6 @@ pub struct RustGisApp { } impl RustGisApp { - /// Export Features' GeoPackage `mode` (`newFile` when not given) for a file of extension `ext`: - /// it and `tableName` are refused for other formats, and a `mode` that isn't text, as the - /// command refuses them. - fn export_mode<'a>(p: &'a Value, ext: &str) -> Result<&'a str, String> { - let mode = match p.get("mode") { - None | Some(Value::Null) => "newFile", - Some(v) => v.as_str().ok_or("mode must be a string")?, - }; - if ext != "gpkg" && (mode != "newFile" || p.get("tableName").is_some()) { - return Err(format!("mode and tableName are for a GeoPackage (.gpkg) export, not .{ext}")); - } - Ok(mode) - } - /// The layer to export: `layer` (an id or name, as commands take it; the dialog's is checked /// against its `instance`, so a reused id isn't taken for it), else the active layer. fn export_target(&self, p: &Value) -> Result { @@ -3345,7 +3331,7 @@ impl RustGisApp { // Any format the dialog offers here (`dialogs::export_formats`): not GeoPackage or // FlatGeobuf, which the data crate refuses in the browser. let ext = p.get("format").and_then(Value::as_str).unwrap_or("geojson").to_ascii_lowercase(); - Self::export_mode(p, &ext)?; + rustgis_engine::cmd::share::gpkg_table(p, &ext)?; // A streamed layer exports its whole source, not the features read for the view. let whole = if l.is_streamed() { Some(l.load_all()?) } else { None }; // In the layer's own coordinate system, as share.exportLayer writes a file. @@ -3417,16 +3403,20 @@ impl RustGisApp { .map(|(l, _)| *l) .ok_or_else(|| format!("unknown format '{ext}'"))?; // Into a GeoPackage that exists: picked to open, as it is kept, not replaced. - let mode = Self::export_mode(p, &ext)?; + // Checked as the command checks them, before a file is asked for; then passed on. + let gpkg = rustgis_engine::cmd::share::gpkg_table(p, &ext)?; let id = self.export_target(p)?; - if ext == "gpkg" && mode != "newFile" { - if let Some(path) = dialogs::pick_open(&["gpkg"], label) { - let mut args = json!({"layer": id, "path": path, "mode": mode}); - // Passed as given, so the command refuses one that isn't text. - if let Some(table) = p.get("tableName") { - args["tableName"] = table.clone(); + let with_table = |mut args: Value| { + for key in ["mode", "tableName"] { + if let Some(v) = p.get(key) { + args[key] = v.clone(); } - return self.run("share.exportLayer", args); + } + args + }; + if gpkg.mode != rustgis_engine::data::io::GpkgMode::NewFile { + if let Some(path) = dialogs::pick_open(&["gpkg"], label) { + return self.run("share.exportLayer", with_table(json!({"layer": id, "path": path}))); } return Ok(Value::Null); } @@ -3436,7 +3426,7 @@ impl RustGisApp { // in the name, as in "U.S. Cities", isn't taken for one). let typed = std::path::Path::new(&path).extension().is_some_and(|e| e.eq_ignore_ascii_case(&ext)); let path = if typed { path } else { format!("{path}.{ext}") }; - return self.run("share.exportLayer", json!({"layer": id, "path": path})); + return self.run("share.exportLayer", with_table(json!({"layer": id, "path": path}))); } } // Add Zarr Variable, connected to a store (a link, a script). @@ -4555,6 +4545,9 @@ mod download_service_tests { // The browser's export refuses a GeoPackage mode for another format, as the desktop's does. assert!(app.run("ui.exportLayer", json!({"format": "geojson", "mode": "replaceTable"})).is_err()); assert_eq!(changed(&app), 0); + // As the command reads them: no table name given is none. + app.run("ui.exportLayer", json!({"format": "geojson", "mode": " newFile ", "tableName": ""})).unwrap(); + assert_eq!(changed(&app), 0, "GeoJSON keeps the names"); app.run("ui.exportLayer", json!({"format": "shp"})).unwrap(); assert_eq!(changed(&app), 1); assert!(app.session.notices.iter().any(|n| n.text == "Sites.zip: 1 text value was cut to at most 254 bytes, in 'notes'")); @@ -4601,6 +4594,10 @@ mod download_service_tests { assert_eq!(rustgis_engine::data::source::layers(&path).unwrap(), ["Cities", "borders"]); let countries = app.session.map().layer_by_name("Countries").map(|l| l.dataset.features.len() as u64); assert_eq!(rustgis_engine::data::source::open_path(&path, Some("borders")).unwrap().info().count, countries); + // An unknown mode is refused before a file is asked for (the pick isn't taken). + crate::dialogs::PICKED.with(|p| *p.borrow_mut() = Some(path.display().to_string())); + assert!(app.run("ui.exportLayer", json!({"format": "gpkg", "mode": "append"})).unwrap_err().contains("unknown mode")); + assert!(crate::dialogs::PICKED.with(|p| p.borrow_mut().take()).is_some()); // A layer by name is that layer; one not there is refused, not taken for the active one. let e = app.run("ui.exportLayer", json!({"format": "gpkg", "mode": "addTable", "layer": "Nowhere"})).unwrap_err(); assert!(e.contains("Nowhere"), "{e}"); From 760d9ae3369abc7e511430633217ecb8fd33484b Mon Sep 17 00:00:00 2001 From: mavrick Date: Wed, 4 Nov 2026 13:35:45 -0600 Subject: [PATCH 10/10] docs: say how a GeoPackage export names its table, and which tables have a spatial index Co-Authored-By: Claude Opus 5.5 --- docs/user-guide/sharing.md | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/docs/user-guide/sharing.md b/docs/user-guide/sharing.md index caccb22cd..efdffa363 100644 --- a/docs/user-guide/sharing.md +++ b/docs/user-guide/sharing.md @@ -31,8 +31,9 @@ tables, even if the save dialog offers to replace the file. To put the layer int GeoPackage instead, choose GeoPackage in **Export Features**, then under **Into** either **A new table in a GeoPackage** or **A table replaced in a GeoPackage**, name the table, and pick the file (on the desktop). The GeoPackage's other tables stay as they were: one table is -added, or one feature or attribute table of that name is replaced with its spatial index, in a -single transaction, so a failed export changes nothing. A name already used is refused when +added, or one feature or attribute table of that name is replaced (a feature table with its +spatial index; an attribute table has none), in a single transaction, so a failed export changes +nothing. A name already used is refused when adding (with no file there, adding makes a new GeoPackage). Replacing needs a feature or attribute table of that name, so a misspelt name changes nothing; tiles, views and other tables are never replaced, nor is a table in a related-tables relation. A replaced table's own indexes, @@ -41,7 +42,11 @@ that use it. The GeoPackage's own triggers don't run while the export writes. A place from the GeoPackage it is exported into is read whole first, rather than a chunk at a time, as SQLite can't read and write one file in the same export. From the command line, `share.exportLayer` and `share.exportTable` take `"mode": "addTable"` or -`"replaceTable"` and an optional `"tableName"` (default: the layer's or table's name): +`"replaceTable"` and an optional `"tableName"` (default: the layer's or table's name). The name is +written as a GeoPackage allows: control characters are removed, surrounding spaces trimmed, an +empty name becomes `layer`, and one starting with `gpkg_`, `sqlite_` or `rtree_` (reserved for the +GeoPackage's own tables) gets `layer_` in front. Adding and replacing look for that name, and the +command's result gives it as `table`: ```sh rustgis-cli run --data permits.geojson \